| Message ID | 1381490814-20890-1-git-send-email-ch.naveen@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Fixing incorrent mail addresses and dropping the old DT ML.] On Saturday 12 of October 2013 04:22:04 Tomasz Figa wrote: > 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 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Saturday 12 of October 2013 04:28:51 Tomasz Figa wrote: > [Fixing incorrent mail addresses and dropping the old DT ML.] > > On Saturday 12 of October 2013 04:22:04 Tomasz Figa wrote: > > 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. Actually you can simply check for CONFIG_CPU_FREQ_S3C24XX here. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12 October 2013 11:12, Tomasz Figa <t.figa@samsung.com> wrote: > On Saturday 12 of October 2013 04:28:51 Tomasz Figa wrote: >> [Fixing incorrent mail addresses and dropping the old DT ML.] >> >> On Saturday 12 of October 2013 04:22:04 Tomasz Figa wrote: >> > 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. > > Actually you can simply check for CONFIG_CPU_FREQ_S3C24XX here. sure, this makes it more meaningful. will make the modifications and resubmit. > > Best regards, > Tomasz >
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) struct notifier_block freq_transition; #endif }; @@ -843,7 +843,7 @@ static int s3c24xx_i2c_clockrate(struct s3c24xx_i2c *i2c, unsigned int *got) return 0; } -#ifdef CONFIG_CPU_FREQ +#if defined(CONFIG_CPU_FREQ) && !defined(CONFIG_ARCH_EXYNOS) #define freq_to_i2c(_n) container_of(_n, struct s3c24xx_i2c, freq_transition)
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. Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com> --- drivers/i2c/busses/i2c-s3c2410.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)