| Message ID | 1303291509-2305-1-git-send-email-shweta.gulati@ti.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Paul Walmsley |
| Headers | show |
> -----Original Message----- > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap- > owner@vger.kernel.org] On Behalf Of Gulati, Shweta > Sent: Wednesday, April 20, 2011 2:55 PM > To: linux-omap@vger.kernel.org > Cc: linux-arm-kernel@lists.infradead.org; Gulati, Shweta; Nayak, Rajendra; > Paul Wamsley > Subject: [PATCH] OMAP: Added recalculation of clock rate in 'clk_set_rate' > > From: Gulati, Shweta <shweta.gulati@ti.com> > > Core Clk Tree shows incorrect Clk rates at OPP50, as > in commit e07f469d284ca3d1f5dcf5438c22982be98bc071 > calling of 'recalc' in API clk_set_rate is unintentionally > removed, because of which clock's tree rates get goofed > up when DVFS happens. This Patch adds recalc API back. I see that the patch is not adding API back. It is adding a call to the API. > > Tested on OMAP3630 SDP Board. > > Signed-off-by: Shweta Gulati <shweta.gulati@ti.com> > Cc: Rajendra Nayak <rnayak@ti.com> > Cc: Paul Wamsley <paul@pwsan.com> > --- > arch/arm/plat-omap/clock.c | 5 ++++- > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c > index c9122dd..5a0d06b 100644 > --- a/arch/arm/plat-omap/clock.c > +++ b/arch/arm/plat-omap/clock.c > @@ -130,8 +130,11 @@ int clk_set_rate(struct clk *clk, unsigned long rate) > > spin_lock_irqsave(&clockfw_lock, flags); > ret = arch_clock->clk_set_rate(clk, rate); > - if (ret == 0) > + if (ret == 0) { checking if (!ret) is an intelligent way. But it is an individual's choice. > + if (clk->recalc) > + clk->rate = clk->recalc(clk); > propagate_rate(clk); > + } > spin_unlock_irqrestore(&clockfw_lock, flags); > > return ret; > -- > 1.7.0.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" 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-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 20-04-2011 13:25, Shweta Gulati wrote: > From: Gulati, Shweta<shweta.gulati@ti.com> > Core Clk Tree shows incorrect Clk rates at OPP50, as > in commit e07f469d284ca3d1f5dcf5438c22982be98bc071 Please also specify that commit's summary -- for human readers. > calling of 'recalc' in API clk_set_rate is unintentionally > removed, because of which clock's tree rates get goofed > up when DVFS happens. This Patch adds recalc API back. > Tested on OMAP3630 SDP Board. > Signed-off-by: Shweta Gulati<shweta.gulati@ti.com> > Cc: Rajendra Nayak<rnayak@ti.com> > Cc: Paul Wamsley<paul@pwsan.com> WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi On Wed, 20 Apr 2011, Shweta Gulati wrote: > From: Gulati, Shweta <shweta.gulati@ti.com> > > Core Clk Tree shows incorrect Clk rates at OPP50, as > in commit e07f469d284ca3d1f5dcf5438c22982be98bc071 > calling of 'recalc' in API clk_set_rate is unintentionally > removed, That's intentional. struct clk.set_rate functions need to set the struct clk.rate before returning. If they don't do that, it's a bug in the struct clk's .set_rate function. > because of which clock's tree rates get goofed up when DVFS happens. Clearly this isn't on the mainline kernel... > This Patch adds recalc API back. > > Tested on OMAP3630 SDP Board. > > Signed-off-by: Shweta Gulati <shweta.gulati@ti.com> > Cc: Rajendra Nayak <rnayak@ti.com> > Cc: Paul Wamsley <paul@pwsan.com> > --- > arch/arm/plat-omap/clock.c | 5 ++++- > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c > index c9122dd..5a0d06b 100644 > --- a/arch/arm/plat-omap/clock.c > +++ b/arch/arm/plat-omap/clock.c > @@ -130,8 +130,11 @@ int clk_set_rate(struct clk *clk, unsigned long rate) > > spin_lock_irqsave(&clockfw_lock, flags); > ret = arch_clock->clk_set_rate(clk, rate); > - if (ret == 0) > + if (ret == 0) { > + if (clk->recalc) > + clk->rate = clk->recalc(clk); > propagate_rate(clk); > + } > spin_unlock_irqrestore(&clockfw_lock, flags); > > return ret; > -- > 1.7.0.4 > - Paul -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Wed, Apr 20, 2011 at 7:05 PM, Janorkar, Mayuresh <mayur@ti.com> wrote: > > >> -----Original Message----- >> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap- >> owner@vger.kernel.org] On Behalf Of Gulati, Shweta >> Sent: Wednesday, April 20, 2011 2:55 PM >> To: linux-omap@vger.kernel.org >> Cc: linux-arm-kernel@lists.infradead.org; Gulati, Shweta; Nayak, Rajendra; >> Paul Wamsley >> Subject: [PATCH] OMAP: Added recalculation of clock rate in 'clk_set_rate' >> >> From: Gulati, Shweta <shweta.gulati@ti.com> >> >> Core Clk Tree shows incorrect Clk rates at OPP50, as >> in commit e07f469d284ca3d1f5dcf5438c22982be98bc071 >> calling of 'recalc' in API clk_set_rate is unintentionally >> removed, because of which clock's tree rates get goofed >> up when DVFS happens. This Patch adds recalc API back. > > I see that the patch is not adding API back. > It is adding a call to the API. Will Modify Commit message. >> >> Tested on OMAP3630 SDP Board. >> >> Signed-off-by: Shweta Gulati <shweta.gulati@ti.com> >> Cc: Rajendra Nayak <rnayak@ti.com> >> Cc: Paul Wamsley <paul@pwsan.com> >> --- >> arch/arm/plat-omap/clock.c | 5 ++++- >> 1 files changed, 4 insertions(+), 1 deletions(-) >> >> diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c >> index c9122dd..5a0d06b 100644 >> --- a/arch/arm/plat-omap/clock.c >> +++ b/arch/arm/plat-omap/clock.c >> @@ -130,8 +130,11 @@ int clk_set_rate(struct clk *clk, unsigned long rate) >> >> spin_lock_irqsave(&clockfw_lock, flags); >> ret = arch_clock->clk_set_rate(clk, rate); >> - if (ret == 0) >> + if (ret == 0) { > > checking if (!ret) is an intelligent way. > But it is an individual's choice. It doesn't make much difference. >> + if (clk->recalc) >> + clk->rate = clk->recalc(clk); >> propagate_rate(clk); >> + } >> spin_unlock_irqrestore(&clockfw_lock, flags); >> >> return ret; >> -- >> 1.7.0.4 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-omap" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >
Hi, On Wed, Apr 20, 2011 at 9:12 PM, Sergei Shtylyov <sshtylyov@mvista.com> wrote: > Hello. > > On 20-04-2011 13:25, Shweta Gulati wrote: > >> From: Gulati, Shweta<shweta.gulati@ti.com> > >> Core Clk Tree shows incorrect Clk rates at OPP50, as >> in commit e07f469d284ca3d1f5dcf5438c22982be98bc071 > > Please also specify that commit's summary -- for human readers. Will do. >> calling of 'recalc' in API clk_set_rate is unintentionally >> removed, because of which clock's tree rates get goofed >> up when DVFS happens. This Patch adds recalc API back. > >> Tested on OMAP3630 SDP Board. > >> Signed-off-by: Shweta Gulati<shweta.gulati@ti.com> >> Cc: Rajendra Nayak<rnayak@ti.com> >> Cc: Paul Wamsley<paul@pwsan.com> > > WBR, Sergei >
Hi, On Thu, Apr 21, 2011 at 1:25 AM, Paul Walmsley <paul@pwsan.com> wrote: > Hi > > On Wed, 20 Apr 2011, Shweta Gulati wrote: > >> From: Gulati, Shweta <shweta.gulati@ti.com> >> >> Core Clk Tree shows incorrect Clk rates at OPP50, as >> in commit e07f469d284ca3d1f5dcf5438c22982be98bc071 >> calling of 'recalc' in API clk_set_rate is unintentionally >> removed, > > That's intentional. struct clk.set_rate functions need to set the struct > clk.rate before returning. If they don't do that, it's a bug in the > struct clk's .set_rate function. Yes, but in current code clk_set_rate of dpll3_m2 - 'omap3_core_dpll_m2_set_rate' doesn't update clk.rate, I will submit patch fixing that bug and will make sure that set_rate of all clocks should update clk.rate > >> because of which clock's tree rates get goofed up when DVFS happens. > > Clearly this isn't on the mainline kernel... If you check for OPP50, the core clocks would show wrong rates. Thanks for reviewing. > >> This Patch adds recalc API back. >> >> Tested on OMAP3630 SDP Board. >> >> Signed-off-by: Shweta Gulati <shweta.gulati@ti.com> >> Cc: Rajendra Nayak <rnayak@ti.com> >> Cc: Paul Wamsley <paul@pwsan.com> >> --- >> arch/arm/plat-omap/clock.c | 5 ++++- >> 1 files changed, 4 insertions(+), 1 deletions(-) >> >> diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c >> index c9122dd..5a0d06b 100644 >> --- a/arch/arm/plat-omap/clock.c >> +++ b/arch/arm/plat-omap/clock.c >> @@ -130,8 +130,11 @@ int clk_set_rate(struct clk *clk, unsigned long rate) >> >> spin_lock_irqsave(&clockfw_lock, flags); >> ret = arch_clock->clk_set_rate(clk, rate); >> - if (ret == 0) >> + if (ret == 0) { >> + if (clk->recalc) >> + clk->rate = clk->recalc(clk); >> propagate_rate(clk); >> + } >> spin_unlock_irqrestore(&clockfw_lock, flags); >> >> return ret; >> -- >> 1.7.0.4 >> > > > - Paul >
On Thu, 21 Apr 2011, Gulati, Shweta wrote: > Yes, but in current code clk_set_rate of dpll3_m2 - > 'omap3_core_dpll_m2_set_rate' doesn't update clk.rate, I will submit > patch fixing that bug and will make sure that set_rate of all clocks > should update clk.rate Yes, that's the right thing to do, please send that. Such a patch was originally part of the series for the last merge window; it seems I inadvertently dropped it. - Paul -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 21 Apr 2011, Gulati, Shweta wrote: > Yes, but in current code clk_set_rate of dpll3_m2 - > 'omap3_core_dpll_m2_set_rate' > doesn't update clk.rate, I will submit patch fixing that bug and will > make sure that > set_rate of all clocks should update clk.rate Ping. Do you plan to post this soon? This should be a trivial patch. - Paul -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Sat, May 7, 2011 at 3:19 AM, Paul Walmsley <paul@pwsan.com> wrote: > On Thu, 21 Apr 2011, Gulati, Shweta wrote: > >> Yes, but in current code clk_set_rate of dpll3_m2 - >> 'omap3_core_dpll_m2_set_rate' >> doesn't update clk.rate, I will submit patch fixing that bug and will >> make sure that >> set_rate of all clocks should update clk.rate > > Ping. Do you plan to post this soon? This should be a trivial patch. Sorry for the delay, I will post today. > - Paul >
On Tue, May 10, 2011 at 04:25:19PM +0530, Gulati, Shweta wrote: > Hi, > > On Sat, May 7, 2011 at 3:19 AM, Paul Walmsley <paul@pwsan.com> wrote: > > On Thu, 21 Apr 2011, Gulati, Shweta wrote: > > > >> Yes, but in current code clk_set_rate of dpll3_m2 - > >> 'omap3_core_dpll_m2_set_rate' > >> doesn't update clk.rate, I will submit patch fixing that bug and will > >> make sure that > >> set_rate of all clocks should update clk.rate > > > > Ping. Do you plan to post this soon? This should be a trivial patch. > Sorry for the delay, I will post today. Hi Shweta , To avoid delay, yesterday posted a patch for the same with your signed off by : http://www.spinics.net/lists/linux-omap/msg50776.html Please verify the patch and if anything is missing, please submit a new patch. thanks , - Avinash > > - Paul > > > > > > -- > Thanks, > Regards, > Shweta > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Tue, May 10, 2011 at 7:12 PM, Avinash.H.M. <avinashhm@ti.com> wrote: > On Tue, May 10, 2011 at 04:25:19PM +0530, Gulati, Shweta wrote: >> Hi, >> >> On Sat, May 7, 2011 at 3:19 AM, Paul Walmsley <paul@pwsan.com> wrote: >> > On Thu, 21 Apr 2011, Gulati, Shweta wrote: >> > >> >> Yes, but in current code clk_set_rate of dpll3_m2 - >> >> 'omap3_core_dpll_m2_set_rate' >> >> doesn't update clk.rate, I will submit patch fixing that bug and will >> >> make sure that >> >> set_rate of all clocks should update clk.rate >> > >> > Ping. Do you plan to post this soon? This should be a trivial patch. >> Sorry for the delay, I will post today. > > Hi Shweta , > > To avoid delay, yesterday posted a patch for the same with your signed > off by : > > http://www.spinics.net/lists/linux-omap/msg50776.html > > Please verify the patch and if anything is missing, please submit a new > patch. Thanks Avinash for posting the Patch on time. I have checked all clk's set_rate, other than 'omap3_dpll3_m2_set_rate' all set_rate APIs update clk rate, so the patch is complete. Acked. > thanks , > > - Avinash > > >> > - Paul >> > >> >> >> >> -- >> Thanks, >> Regards, >> Shweta >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c index c9122dd..5a0d06b 100644 --- a/arch/arm/plat-omap/clock.c +++ b/arch/arm/plat-omap/clock.c @@ -130,8 +130,11 @@ int clk_set_rate(struct clk *clk, unsigned long rate) spin_lock_irqsave(&clockfw_lock, flags); ret = arch_clock->clk_set_rate(clk, rate); - if (ret == 0) + if (ret == 0) { + if (clk->recalc) + clk->rate = clk->recalc(clk); propagate_rate(clk); + } spin_unlock_irqrestore(&clockfw_lock, flags); return ret;