* Re: [PATCH] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series
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
2013-10-12 2:28 ` Tomasz Figa
2013-10-15 6:23 ` [PATCH v2] " Naveen Krishna Chatradhi
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Tomasz Figa @ 2013-10-12 2:22 UTC (permalink / raw)
To: Naveen Krishna Chatradhi
Cc: linux-i2c, linux-kernel, linux-samsung-soc, wsa, khali,
ben-linux, grant.likely, devicetree-discuss, sjg,
naveenkrishna.ch, broonie, diander, cpgs
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series
2013-10-12 2:22 ` Tomasz Figa
@ 2013-10-12 2:28 ` Tomasz Figa
2013-10-12 5:42 ` Tomasz Figa
0 siblings, 1 reply; 14+ messages in thread
From: Tomasz Figa @ 2013-10-12 2:28 UTC (permalink / raw)
To: Naveen Krishna Chatradhi, sjg
Cc: linux-i2c, linux-kernel, linux-samsung-soc, wsa, khali,
ben-linux, grant.likely, naveenkrishna.ch, broonie, dianders,
cpgs
[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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series
2013-10-12 2:28 ` Tomasz Figa
@ 2013-10-12 5:42 ` Tomasz Figa
2013-10-12 6:36 ` Naveen Krishna Ch
0 siblings, 1 reply; 14+ messages in thread
From: Tomasz Figa @ 2013-10-12 5:42 UTC (permalink / raw)
To: Naveen Krishna Chatradhi
Cc: sjg, linux-i2c, linux-kernel, linux-samsung-soc, wsa, khali,
ben-linux, grant.likely, naveenkrishna.ch, broonie, dianders,
cpgs
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series
2013-10-12 5:42 ` Tomasz Figa
@ 2013-10-12 6:36 ` Naveen Krishna Ch
0 siblings, 0 replies; 14+ messages in thread
From: Naveen Krishna Ch @ 2013-10-12 6:36 UTC (permalink / raw)
To: Tomasz Figa
Cc: Naveen Krishna Chatradhi, sjg, linux-i2c, linux-kernel,
linux-samsung-soc, wsa, khali, Ben Dooks, grant.likely, broonie,
dianders, cpgs
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
>
--
Shine bright,
(: Nav :)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series
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
@ 2013-10-15 6:23 ` Naveen Krishna Chatradhi
2013-10-15 6:56 ` Kyungmin Park
2013-11-19 6:18 ` [PATCH v3] " Naveen Krishna Chatradhi
2013-11-26 4:22 ` [PATCH v4] " Naveen Krishna Chatradhi
3 siblings, 1 reply; 14+ messages in thread
From: Naveen Krishna Chatradhi @ 2013-10-15 6:23 UTC (permalink / raw)
To: linux-i2c
Cc: t.figa, sjg, linux-kernel, linux-samsung-soc, wsa, ben-linux,
grant.likely, broonie, dianders, cpgs
For Exynos4 and Exynos5 SoCs from Samsung the 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 if CONFIG_CPU_FREQ_S3C24XX is enabled.
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>
---
Changes since v1:
Use CONFIG_CPU_FREQ_S3C24XX instead of (CONFIG_CPU_FREQ & !CONFIG_EXYNOS)
As commented by Tomasz
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..97f14f7 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_S3C24XX)
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_S3C24XX)
#define freq_to_i2c(_n) container_of(_n, struct s3c24xx_i2c, freq_transition)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series
2013-10-15 6:23 ` [PATCH v2] " Naveen Krishna Chatradhi
@ 2013-10-15 6:56 ` Kyungmin Park
2013-10-15 15:38 ` Doug Anderson
0 siblings, 1 reply; 14+ messages in thread
From: Kyungmin Park @ 2013-10-15 6:56 UTC (permalink / raw)
To: Naveen Krishna Chatradhi
Cc: linux-i2c, t.figa, sjg, linux-kernel, linux-samsung-soc, wsa,
ben-linux, grant.likely, broonie, dianders, cpgs
On Tue, Oct 15, 2013 at 3:23 PM, Naveen Krishna Chatradhi
<ch.naveen@samsung.com> wrote:
> For Exynos4 and Exynos5 SoCs from Samsung the 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 if CONFIG_CPU_FREQ_S3C24XX is enabled.
>
> 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>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> Changes since v1:
> Use CONFIG_CPU_FREQ_S3C24XX instead of (CONFIG_CPU_FREQ & !CONFIG_EXYNOS)
> As commented by Tomasz
>
> 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..97f14f7 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_S3C24XX)
> 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_S3C24XX)
>
> #define freq_to_i2c(_n) container_of(_n, struct s3c24xx_i2c, freq_transition)
>
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series
2013-10-15 6:56 ` Kyungmin Park
@ 2013-10-15 15:38 ` Doug Anderson
0 siblings, 0 replies; 14+ messages in thread
From: Doug Anderson @ 2013-10-15 15:38 UTC (permalink / raw)
To: Kyungmin Park
Cc: Naveen Krishna Chatradhi, linux-i2c, Tomasz Figa, Simon Glass,
linux-kernel, linux-samsung-soc, Wolfram Sang, Ben Dooks,
Grant Likely, broonie, cpgs .
Hi,
On Mon, Oct 14, 2013 at 11:56 PM, Kyungmin Park <kmpark@infradead.org> wrote:
> On Tue, Oct 15, 2013 at 3:23 PM, Naveen Krishna Chatradhi
> <ch.naveen@samsung.com> wrote:
>> For Exynos4 and Exynos5 SoCs from Samsung the 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 if CONFIG_CPU_FREQ_S3C24XX is enabled.
>>
>> 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>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>> Changes since v1:
>> Use CONFIG_CPU_FREQ_S3C24XX instead of (CONFIG_CPU_FREQ & !CONFIG_EXYNOS)
>> As commented by Tomasz
>>
>> drivers/i2c/busses/i2c-s3c2410.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
Looks great to me. Thank you for the suggestions Tomasz, as always.
Reviewed-by: Doug Anderson <dianders@chromium.org>
We need to come up with a solution for the CPU_FREQ stuff in
s3c2410_wdt too. We could use a similar solution but since the
CPU_FREQ stuff in s3c2410_wdt is more than just an optimization it
means that it's not good if S3C24XX is included in a multiplatform
kernel. (For the watchdog it's more than just an optimization since
every frequency transition actually pets the watchdog, making it
useless when you transition several times per second).
-Doug
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series
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
2013-10-15 6:23 ` [PATCH v2] " Naveen Krishna Chatradhi
@ 2013-11-19 6:18 ` Naveen Krishna Chatradhi
2013-11-25 23:41 ` Doug Anderson
2013-11-26 4:22 ` [PATCH v4] " Naveen Krishna Chatradhi
3 siblings, 1 reply; 14+ messages in thread
From: Naveen Krishna Chatradhi @ 2013-11-19 6:18 UTC (permalink / raw)
To: linux-i2c
Cc: sjg, linux-kernel, linux-samsung-soc, wsa, ben-linux,
grant.likely, broonie, dianders, cpgs, t.figa
For Exynos4 and Exynos5 SoCs from Samsung the 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 if CONFIG_CPU_FREQ_S3C24XX is enabled.
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>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Reviewed-by: Doug Anderson <dianders@chromium.org>
---
Changes since v2:
None, Rebased on for-next of linux-i2c git repo.
Changes since v1:
Use CONFIG_CPU_FREQ_S3C24XX instead of (CONFIG_CPU_FREQ & !CONFIG_EXYNOS)
As commented by Tomasz
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 bf8fb94..fa51dff 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_S3C24XX)
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_S3C24XX)
#define freq_to_i2c(_n) container_of(_n, struct s3c24xx_i2c, freq_transition)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v3] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series
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
0 siblings, 1 reply; 14+ messages in thread
From: Doug Anderson @ 2013-11-25 23:41 UTC (permalink / raw)
To: Naveen Krishna Chatradhi
Cc: linux-i2c, Simon Glass, linux-kernel, linux-samsung-soc,
Wolfram Sang, Ben Dooks, Grant Likely, broonie, cpgs .,
Tomasz Figa
Naveen,
On Mon, Nov 18, 2013 at 10:18 PM, Naveen Krishna Chatradhi
<ch.naveen@samsung.com> wrote:
> For Exynos4 and Exynos5 SoCs from Samsung the 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 if CONFIG_CPU_FREQ_S3C24XX is enabled.
>
> 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>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Reviewed-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes since v2:
> None, Rebased on for-next of linux-i2c git repo.
>
> Changes since v1:
> Use CONFIG_CPU_FREQ_S3C24XX instead of (CONFIG_CPU_FREQ & !CONFIG_EXYNOS)
> As commented by Tomasz
>
> drivers/i2c/busses/i2c-s3c2410.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Can you please spin this with comments from
<https://patchwork.kernel.org/patch/3235091/>? Thanks!
-Doug
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series
2013-11-25 23:41 ` Doug Anderson
@ 2013-11-26 4:07 ` Naveen Krishna Ch
0 siblings, 0 replies; 14+ messages in thread
From: Naveen Krishna Ch @ 2013-11-26 4:07 UTC (permalink / raw)
To: Doug Anderson
Cc: Naveen Krishna Chatradhi, linux-i2c, Simon Glass, linux-kernel,
linux-samsung-soc, Wolfram Sang, Ben Dooks, Grant Likely,
broonie, cpgs .,
Tomasz Figa
Hello Doug,
On 26 November 2013 05:11, Doug Anderson <dianders@chromium.org> wrote:
> Naveen,
>
> On Mon, Nov 18, 2013 at 10:18 PM, Naveen Krishna Chatradhi
> <ch.naveen@samsung.com> wrote:
>> For Exynos4 and Exynos5 SoCs from Samsung the 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 if CONFIG_CPU_FREQ_S3C24XX is enabled.
>>
>> 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>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> Reviewed-by: Doug Anderson <dianders@chromium.org>
>> ---
>> Changes since v2:
>> None, Rebased on for-next of linux-i2c git repo.
>>
>> Changes since v1:
>> Use CONFIG_CPU_FREQ_S3C24XX instead of (CONFIG_CPU_FREQ & !CONFIG_EXYNOS)
>> As commented by Tomasz
>>
>> drivers/i2c/busses/i2c-s3c2410.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Can you please spin this with comments from
> <https://patchwork.kernel.org/patch/3235091/>? Thanks!
Thanks for pointing out for me
To summarize, Post f023f8dd59 commit we should be using
ARM_S3C24XX_CPUFREQ instead of ARM_S3C24XX_CPUFREQ right.
Will re-spin with this changes.
>
> -Doug
> --
> 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
--
Thanks & Regards,
(: Nav :)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series
2013-10-11 11:26 [PATCH] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series Naveen Krishna Chatradhi
` (2 preceding siblings ...)
2013-11-19 6:18 ` [PATCH v3] " Naveen Krishna Chatradhi
@ 2013-11-26 4:22 ` Naveen Krishna Chatradhi
2013-12-09 16:09 ` Tomasz Figa
2014-01-03 16:23 ` Wolfram Sang
3 siblings, 2 replies; 14+ messages in thread
From: Naveen Krishna Chatradhi @ 2013-11-26 4:22 UTC (permalink / raw)
To: linux-i2c
Cc: sjg, linux-kernel, linux-samsung-soc, wsa, ben-linux,
grant.likely, broonie, dianders, cpgs, t.figa
For Exynos4 and Exynos5 SoCs from Samsung the 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 if CONFIG_CPU_FREQ_S3C24XX is enabled.
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>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Reviewed-by: Doug Anderson <dianders@chromium.org>
---
Changes since v3:
As per discussion at https://patchwork.kernel.org/patch/3235091/
Post f023f8dd59 commit we should be using
ARM_S3C24XX_CPUFREQ instead of CPU_FREQ_S3C24XX
upstrea linux kernel and linuxsamsung already has the commit
f023f8dd59. Hence, rebasing the patch.
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 bf8fb94..a77ce13 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_ARM_S3C24XX_CPUFREQ)
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_ARM_S3C24XX_CPUFREQ)
#define freq_to_i2c(_n) container_of(_n, struct s3c24xx_i2c, freq_transition)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series
2013-11-26 4:22 ` [PATCH v4] " Naveen Krishna Chatradhi
@ 2013-12-09 16:09 ` Tomasz Figa
2014-01-03 16:23 ` Wolfram Sang
1 sibling, 0 replies; 14+ messages in thread
From: Tomasz Figa @ 2013-12-09 16:09 UTC (permalink / raw)
To: Naveen Krishna Chatradhi, wsa
Cc: linux-i2c, sjg, linux-kernel, linux-samsung-soc, ben-linux,
grant.likely, broonie, dianders, cpgs
Hi Naveen,
On Tuesday 26 of November 2013 09:52:46 Naveen Krishna Chatradhi wrote:
> For Exynos4 and Exynos5 SoCs from Samsung the 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 if CONFIG_CPU_FREQ_S3C24XX is enabled.
>
> 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>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Reviewed-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes since v3:
> As per discussion at https://patchwork.kernel.org/patch/3235091/
> Post f023f8dd59 commit we should be using
> ARM_S3C24XX_CPUFREQ instead of CPU_FREQ_S3C24XX
>
> upstrea linux kernel and linuxsamsung already has the commit
> f023f8dd59. Hence, rebasing the patch.
>
> drivers/i2c/busses/i2c-s3c2410.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Wolfram, are you okay with this patch?
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series
2013-11-26 4:22 ` [PATCH v4] " Naveen Krishna Chatradhi
2013-12-09 16:09 ` Tomasz Figa
@ 2014-01-03 16:23 ` Wolfram Sang
1 sibling, 0 replies; 14+ messages in thread
From: Wolfram Sang @ 2014-01-03 16:23 UTC (permalink / raw)
To: Naveen Krishna Chatradhi
Cc: linux-i2c, sjg, linux-kernel, linux-samsung-soc, ben-linux,
grant.likely, broonie, dianders, cpgs, t.figa
[-- Attachment #1: Type: text/plain, Size: 766 bytes --]
On Tue, Nov 26, 2013 at 09:52:46AM +0530, Naveen Krishna Chatradhi wrote:
> For Exynos4 and Exynos5 SoCs from Samsung the 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 if CONFIG_CPU_FREQ_S3C24XX is enabled.
>
> 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>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Reviewed-by: Doug Anderson <dianders@chromium.org>
Applied to for-next, thanks!
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread