Re: [PATCH] i2c: s3c2410: dont need CPU_FREQ transitions for exynosseries

From: Tomasz Figa
Date: Fri Oct 11 2013 - 22:22:25 EST


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@xxxxxxxxxxx>
> ---
> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/