linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tomasz Figa <t.figa@samsung.com>
To: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-samsung-soc@vger.kernel.org, wsa@the-dreams.de,
	khali@linux-fr.org, ben-linux@fluff.org,
	grant.likely@secretlab.ca, devicetree-discuss@lists.ozlabs.org,
	sjg@chromium.org, naveenkrishna.ch@gmail.com,
	broonie@opensource.wolfsonmicro.com, diander@google.com,
	cpgs@samsung.com
Subject: Re: [PATCH] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series
Date: Sat, 12 Oct 2013 04:22:04 +0200	[thread overview]
Message-ID: <3245175.os31ym2KFi@amdn883> (raw)
In-Reply-To: <1381490814-20890-1-git-send-email-ch.naveen@samsung.com>

Hi Naveen,

On Friday 11 of October 2013 16:56:54 Naveen Krishna Chatradhi wrote:
> The exynos5 i2c clock is based on a fixed 66 MHz peripheral clock, and
> therefore is completely independent of the cpu frequency.
> Thus, registering for a CPU freq notifier is very wasteful.
> 
> This patch modifes the code such that, i2c bus registers to
> cpu_freq_transition only for non Exynos SoCs.
> 
> This change should save a bunch of cpufreq transitions calls
> which does not apply to exynos SoCs.

The idea is fine, although...

> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
> ---
>  drivers/i2c/busses/i2c-s3c2410.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
> index cab1c91..d062fa7 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -123,7 +123,7 @@ struct s3c24xx_i2c {
>  	struct s3c2410_platform_i2c	*pdata;
>  	int			gpios[2];
>  	struct pinctrl          *pctrl;
> -#ifdef CONFIG_CPU_FREQ
> +#if defined(CONFIG_CPU_FREQ) && !defined(CONFIG_ARCH_EXYNOS)

...this is not a good coding practice, especially when already having
multiplatform kernels in sight.

The best way would be to check on which SoC we are running at runtime,
but since this might need changing a lot of code, then at least I would
change this from !defined(EXYNOS) to defined(S3C24XX), so it is not being
compiled in when S3C24XX support is not enabled and if it's enabled then
the notifier will be registered as a safe fallback that will run correctly
on all platforms.

Best regards,
Tomasz


  reply	other threads:[~2013-10-12  2:22 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-11 11:26 [PATCH] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series Naveen Krishna Chatradhi
2013-10-12  2:22 ` Tomasz Figa [this message]
2013-10-12  2:28   ` Tomasz Figa
2013-10-12  5:42     ` Tomasz Figa
2013-10-12  6:36       ` Naveen Krishna Ch
2013-10-15  6:23 ` [PATCH v2] " Naveen Krishna Chatradhi
2013-10-15  6:56   ` Kyungmin Park
2013-10-15 15:38     ` Doug Anderson
2013-11-19  6:18 ` [PATCH v3] " Naveen Krishna Chatradhi
2013-11-25 23:41   ` Doug Anderson
2013-11-26  4:07     ` Naveen Krishna Ch
2013-11-26  4:22 ` [PATCH v4] " Naveen Krishna Chatradhi
2013-12-09 16:09   ` Tomasz Figa
2014-01-03 16:23   ` Wolfram Sang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3245175.os31ym2KFi@amdn883 \
    --to=t.figa@samsung.com \
    --cc=ben-linux@fluff.org \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=ch.naveen@samsung.com \
    --cc=cpgs@samsung.com \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=diander@google.com \
    --cc=grant.likely@secretlab.ca \
    --cc=khali@linux-fr.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=naveenkrishna.ch@gmail.com \
    --cc=sjg@chromium.org \
    --cc=wsa@the-dreams.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).