* [PATCH] AM33XX: Clock: Fix JTAG disconnect during kernel boot @ 2012-11-16 23:44 Joel A Fernandes 2012-11-17 0:12 ` Jon Hunter 0 siblings, 1 reply; 8+ messages in thread From: Joel A Fernandes @ 2012-11-16 23:44 UTC (permalink / raw) To: linux-omap Cc: Russ Dill, Matt Porter, Steve Kipisz, Jason Kridner, Tony Lindgren Set ENABLE_ON_INIT for debugss clock so that it is on during init and the usecount is incremented. Not doing so causes kernel to disable debugss clock to save power when CONFIG_OMAP_RESET_CLOCKS is set, causing a JTAG disconnect. Signed-off-by: Joel A Fernandes <joelagnel@ti.com> Cc: Russ Dill <Russ.Dill@ti.com> Cc: Matt Porter <mporter@ti.com> Cc: Steve Kipisz <s-kipisz2@ti.com> Cc: Jason Kridner <jkridner@beagleboard.org> Cc: Tony Lindgren <tony@atomide.com> --- Resending, as Cc list was dropped. arch/arm/mach-omap2/clock33xx_data.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/mach-omap2/clock33xx_data.c b/arch/arm/mach-omap2/clock33xx_data.c index b7b7995..d0b4313 100644 --- a/arch/arm/mach-omap2/clock33xx_data.c +++ b/arch/arm/mach-omap2/clock33xx_data.c @@ -585,6 +585,7 @@ static struct clk debugss_ick = { .clkdm_name = "l3_aon_clkdm", .parent = &dpll_core_m4_ck, .ops = &clkops_omap2_dflt, + .flags = ENABLE_ON_INIT, .enable_reg = AM33XX_CM_WKUP_DEBUGSS_CLKCTRL, .enable_bit = AM33XX_MODULEMODE_SWCTRL, .recalc = &followparent_recalc, -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] AM33XX: Clock: Fix JTAG disconnect during kernel boot 2012-11-16 23:44 [PATCH] AM33XX: Clock: Fix JTAG disconnect during kernel boot Joel A Fernandes @ 2012-11-17 0:12 ` Jon Hunter 2012-11-17 2:44 ` Joel A Fernandes 0 siblings, 1 reply; 8+ messages in thread From: Jon Hunter @ 2012-11-17 0:12 UTC (permalink / raw) To: Joel A Fernandes Cc: linux-omap, Russ Dill, Matt Porter, Steve Kipisz, Jason Kridner, Tony Lindgren, Paul Walmsley On 11/16/2012 05:44 PM, Joel A Fernandes wrote: > Set ENABLE_ON_INIT for debugss clock so that it is on during init and the usecount is incremented. > Not doing so causes kernel to disable debugss clock to save power when CONFIG_OMAP_RESET_CLOCKS > is set, causing a JTAG disconnect. > > Signed-off-by: Joel A Fernandes <joelagnel@ti.com> > Cc: Russ Dill <Russ.Dill@ti.com> > Cc: Matt Porter <mporter@ti.com> > Cc: Steve Kipisz <s-kipisz2@ti.com> > Cc: Jason Kridner <jkridner@beagleboard.org> > Cc: Tony Lindgren <tony@atomide.com> > --- > Resending, as Cc list was dropped. > > arch/arm/mach-omap2/clock33xx_data.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/mach-omap2/clock33xx_data.c b/arch/arm/mach-omap2/clock33xx_data.c > index b7b7995..d0b4313 100644 > --- a/arch/arm/mach-omap2/clock33xx_data.c > +++ b/arch/arm/mach-omap2/clock33xx_data.c > @@ -585,6 +585,7 @@ static struct clk debugss_ick = { > .clkdm_name = "l3_aon_clkdm", > .parent = &dpll_core_m4_ck, > .ops = &clkops_omap2_dflt, > + .flags = ENABLE_ON_INIT, > .enable_reg = AM33XX_CM_WKUP_DEBUGSS_CLKCTRL, > .enable_bit = AM33XX_MODULEMODE_SWCTRL, > .recalc = &followparent_recalc, Does this mean this clock will always be enabled, even when not using JTAG? If so, is that what you want? I was not sure if you are concerned about power at all. Or if having that always enabled can inhibit low power states for suspend/idle etc. May be worth adding more details to the changelog about if there are side-effects to this change or not. Adding Paul as he is the maintainer of clocks and clock data. Cheers Jon ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] AM33XX: Clock: Fix JTAG disconnect during kernel boot 2012-11-17 0:12 ` Jon Hunter @ 2012-11-17 2:44 ` Joel A Fernandes 2012-11-19 16:15 ` Jon Hunter 0 siblings, 1 reply; 8+ messages in thread From: Joel A Fernandes @ 2012-11-17 2:44 UTC (permalink / raw) To: Jon Hunter Cc: Joel A Fernandes, linux-omap, Russ Dill, Matt Porter, Steve Kipisz, Jason Kridner, Tony Lindgren, Paul Walmsley Hi Jon, On Fri, Nov 16, 2012 at 6:12 PM, Jon Hunter <jon-hunter@ti.com> wrote: > > diff --git a/arch/arm/mach-omap2/clock33xx_data.c > > b/arch/arm/mach-omap2/clock33xx_data.c > > index b7b7995..d0b4313 100644 > > --- a/arch/arm/mach-omap2/clock33xx_data.c > > +++ b/arch/arm/mach-omap2/clock33xx_data.c > > @@ -585,6 +585,7 @@ static struct clk debugss_ick = { > > .clkdm_name = "l3_aon_clkdm", > > .parent = &dpll_core_m4_ck, > > .ops = &clkops_omap2_dflt, > > + .flags = ENABLE_ON_INIT, > > .enable_reg = AM33XX_CM_WKUP_DEBUGSS_CLKCTRL, > > .enable_bit = AM33XX_MODULEMODE_SWCTRL, > > .recalc = &followparent_recalc, > > Does this mean this clock will always be enabled, even when not using > JTAG? If so, is that what you want? Yes, the clock will always be on, because some times we connect JTAG on a running Linux system without powering it down. I can't comment without doing measurements if keeping the clock on has significant impact on power, but I see what you mean. Definitely low power is a parameter and folks who want lowest possible power might need to turn off the clock and not have JTAG in production, but that depends on the user's requirement. I personally don't care about power as much as I care about not being able to JTAG, and I think there are other folks who suffer from the same JTAG problem who wouldn't necessarily want lowest power. > I was not sure if you are concerned about power at all. Or if having > that always enabled can inhibit low power states for suspend/idle etc. Maybe, suspend/resume code can turn off clock on suspend and enable it on resume. > May be worth adding more details to the changelog about if there are > side-effects to this change or not. Sure, will do after waiting for any other review comments. Thanks, Joel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] AM33XX: Clock: Fix JTAG disconnect during kernel boot 2012-11-17 2:44 ` Joel A Fernandes @ 2012-11-19 16:15 ` Jon Hunter 2012-11-22 5:30 ` Vaibhav Hiremath 2012-11-22 21:01 ` Joel A Fernandes 0 siblings, 2 replies; 8+ messages in thread From: Jon Hunter @ 2012-11-19 16:15 UTC (permalink / raw) To: Joel A Fernandes Cc: Joel A Fernandes, linux-omap, Russ Dill, Matt Porter, Steve Kipisz, Jason Kridner, Tony Lindgren, Paul Walmsley On 11/16/2012 08:44 PM, Joel A Fernandes wrote: > Hi Jon, > > On Fri, Nov 16, 2012 at 6:12 PM, Jon Hunter <jon-hunter@ti.com> wrote: >>> diff --git a/arch/arm/mach-omap2/clock33xx_data.c >>> b/arch/arm/mach-omap2/clock33xx_data.c >>> index b7b7995..d0b4313 100644 >>> --- a/arch/arm/mach-omap2/clock33xx_data.c >>> +++ b/arch/arm/mach-omap2/clock33xx_data.c >>> @@ -585,6 +585,7 @@ static struct clk debugss_ick = { >>> .clkdm_name = "l3_aon_clkdm", >>> .parent = &dpll_core_m4_ck, >>> .ops = &clkops_omap2_dflt, >>> + .flags = ENABLE_ON_INIT, >>> .enable_reg = AM33XX_CM_WKUP_DEBUGSS_CLKCTRL, >>> .enable_bit = AM33XX_MODULEMODE_SWCTRL, >>> .recalc = &followparent_recalc, >> >> Does this mean this clock will always be enabled, even when not using >> JTAG? If so, is that what you want? > > Yes, the clock will always be on, because some times we connect JTAG > on a running Linux system without powering it down. I can't comment > without doing measurements if keeping the clock on has significant > impact on power, but I see what you mean. Definitely low power is a > parameter and folks who want lowest possible power might need to turn > off the clock and not have JTAG in production, but that depends on the > user's requirement. I personally don't care about power as much as I > care about not being able to JTAG, and I think there are other folks > who suffer from the same JTAG problem who wouldn't necessarily want > lowest power. I can't say I am that familiar with the AM33xx architecture, but I know that on OMAP this would keep on the debugss clock and power domain. Do you need this change to connect JTAG after the kernel has booted or just to maintain JTAG during kernel boot? I am wondering if you should implement a new flag such as "DONT_DISABLE_ON_INIT" so if the clock is on it is not disabled, but if it is not enabled, you don't enabled it. Then that would be the best of both worlds. >> I was not sure if you are concerned about power at all. Or if having >> that always enabled can inhibit low power states for suspend/idle etc. > > Maybe, suspend/resume code can turn off clock on suspend and enable it > on resume. Yes, but not ideal for a product assuming they are power sensitive. Cheers Jon ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] AM33XX: Clock: Fix JTAG disconnect during kernel boot 2012-11-19 16:15 ` Jon Hunter @ 2012-11-22 5:30 ` Vaibhav Hiremath 2012-11-22 21:04 ` Joel A Fernandes 2012-11-22 21:01 ` Joel A Fernandes 1 sibling, 1 reply; 8+ messages in thread From: Vaibhav Hiremath @ 2012-11-22 5:30 UTC (permalink / raw) To: Jon Hunter Cc: Joel A Fernandes, Joel A Fernandes, linux-omap, Russ Dill, Matt Porter, Steve Kipisz, Jason Kridner, Tony Lindgren, Paul Walmsley On 11/19/2012 9:45 PM, Jon Hunter wrote: > > On 11/16/2012 08:44 PM, Joel A Fernandes wrote: >> Hi Jon, >> >> On Fri, Nov 16, 2012 at 6:12 PM, Jon Hunter <jon-hunter@ti.com> wrote: >>>> diff --git a/arch/arm/mach-omap2/clock33xx_data.c >>>> b/arch/arm/mach-omap2/clock33xx_data.c >>>> index b7b7995..d0b4313 100644 >>>> --- a/arch/arm/mach-omap2/clock33xx_data.c >>>> +++ b/arch/arm/mach-omap2/clock33xx_data.c >>>> @@ -585,6 +585,7 @@ static struct clk debugss_ick = { >>>> .clkdm_name = "l3_aon_clkdm", >>>> .parent = &dpll_core_m4_ck, >>>> .ops = &clkops_omap2_dflt, >>>> + .flags = ENABLE_ON_INIT, >>>> .enable_reg = AM33XX_CM_WKUP_DEBUGSS_CLKCTRL, >>>> .enable_bit = AM33XX_MODULEMODE_SWCTRL, >>>> .recalc = &followparent_recalc, >>> >>> Does this mean this clock will always be enabled, even when not using >>> JTAG? If so, is that what you want? >> >> Yes, the clock will always be on, because some times we connect JTAG >> on a running Linux system without powering it down. I can't comment >> without doing measurements if keeping the clock on has significant >> impact on power, but I see what you mean. Definitely low power is a >> parameter and folks who want lowest possible power might need to turn >> off the clock and not have JTAG in production, but that depends on the >> user's requirement. I personally don't care about power as much as I >> care about not being able to JTAG, and I think there are other folks >> who suffer from the same JTAG problem who wouldn't necessarily want >> lowest power. > > I can't say I am that familiar with the AM33xx architecture, but I know > that on OMAP this would keep on the debugss clock and power domain. > > Do you need this change to connect JTAG after the kernel has booted or > just to maintain JTAG during kernel boot? > > I am wondering if you should implement a new flag such as > "DONT_DISABLE_ON_INIT" so if the clock is on it is not disabled, but if > it is not enabled, you don't enabled it. Then that would be the best of > both worlds. > Joel, The it should be handled is, on kernel_debug option we should keep it enabled and default it should be disabled. Something similar to, diff --git a/arch/arm/mach-omap2/clock33xx_data.c b/arch/arm/mach-omap2/clock33xx_data.c index 17e3de5..60e0b53 100644 --- a/arch/arm/mach-omap2/clock33xx_data.c +++ b/arch/arm/mach-omap2/clock33xx_data.c @@ -584,6 +584,9 @@ static struct clk debugss_ick = { .clkdm_name = "l3_aon_clkdm", .parent = &dpll_core_m4_ck, .ops = &clkops_omap2_dflt, +#ifdef CONFIG_DEBUG_KERNEL + .flags = ENABLE_ON_INIT, +#endif .enable_reg = AM33XX_CM_WKUP_DEBUGSS_CLKCTRL, .enable_bit = AM33XX_MODULEMODE_SWCTRL, .recalc = &followparent_recalc, Can you please test above change?? Thanks, Vaibhav >>> I was not sure if you are concerned about power at all. Or if having >>> that always enabled can inhibit low power states for suspend/idle etc. >> >> Maybe, suspend/resume code can turn off clock on suspend and enable it >> on resume. > > Yes, but not ideal for a product assuming they are power sensitive. > > Cheers > Jon > -- > 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 > ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] AM33XX: Clock: Fix JTAG disconnect during kernel boot 2012-11-22 5:30 ` Vaibhav Hiremath @ 2012-11-22 21:04 ` Joel A Fernandes 0 siblings, 0 replies; 8+ messages in thread From: Joel A Fernandes @ 2012-11-22 21:04 UTC (permalink / raw) To: Vaibhav Hiremath Cc: Jon Hunter, Joel A Fernandes, linux-omap, Russ Dill, Matt Porter, Steve Kipisz, Jason Kridner, Tony Lindgren, Paul Walmsley Hi Vaibhav, > The it should be handled is, on kernel_debug option we should keep it > enabled and default it should be disabled. > > Something similar to, > > > > diff --git a/arch/arm/mach-omap2/clock33xx_data.c > b/arch/arm/mach-omap2/clock33xx_data.c > index 17e3de5..60e0b53 100644 > --- a/arch/arm/mach-omap2/clock33xx_data.c > +++ b/arch/arm/mach-omap2/clock33xx_data.c > @@ -584,6 +584,9 @@ static struct clk debugss_ick = { > .clkdm_name = "l3_aon_clkdm", > .parent = &dpll_core_m4_ck, > .ops = &clkops_omap2_dflt, > +#ifdef CONFIG_DEBUG_KERNEL > + .flags = ENABLE_ON_INIT, > +#endif Yes, looks like its a good idea to wrap the flag around a config option, but I see you posted another patch where hwmod data is changed for debugss_ick and Igor reported it to be working. Either approach is ok with me. Regards, Joel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] AM33XX: Clock: Fix JTAG disconnect during kernel boot 2012-11-19 16:15 ` Jon Hunter 2012-11-22 5:30 ` Vaibhav Hiremath @ 2012-11-22 21:01 ` Joel A Fernandes 1 sibling, 0 replies; 8+ messages in thread From: Joel A Fernandes @ 2012-11-22 21:01 UTC (permalink / raw) To: Jon Hunter Cc: Joel A Fernandes, linux-omap, Russ Dill, Matt Porter, Steve Kipisz, Jason Kridner, Tony Lindgren, Paul Walmsley Hi Jon, On Mon, Nov 19, 2012 at 10:15 AM, Jon Hunter <jon-hunter@ti.com> wrote: > > On 11/16/2012 08:44 PM, Joel A Fernandes wrote: >> Hi Jon, >> >> On Fri, Nov 16, 2012 at 6:12 PM, Jon Hunter <jon-hunter@ti.com> wrote: >>>> diff --git a/arch/arm/mach-omap2/clock33xx_data.c >>>> b/arch/arm/mach-omap2/clock33xx_data.c >>>> index b7b7995..d0b4313 100644 >>>> --- a/arch/arm/mach-omap2/clock33xx_data.c >>>> +++ b/arch/arm/mach-omap2/clock33xx_data.c >>>> @@ -585,6 +585,7 @@ static struct clk debugss_ick = { >>>> .clkdm_name = "l3_aon_clkdm", >>>> .parent = &dpll_core_m4_ck, >>>> .ops = &clkops_omap2_dflt, >>>> + .flags = ENABLE_ON_INIT, >>>> .enable_reg = AM33XX_CM_WKUP_DEBUGSS_CLKCTRL, >>>> .enable_bit = AM33XX_MODULEMODE_SWCTRL, >>>> .recalc = &followparent_recalc, >>> >>> Does this mean this clock will always be enabled, even when not using >>> JTAG? If so, is that what you want? >> >> Yes, the clock will always be on, because some times we connect JTAG >> on a running Linux system without powering it down. I can't comment >> without doing measurements if keeping the clock on has significant >> impact on power, but I see what you mean. Definitely low power is a >> parameter and folks who want lowest possible power might need to turn >> off the clock and not have JTAG in production, but that depends on the >> user's requirement. I personally don't care about power as much as I >> care about not being able to JTAG, and I think there are other folks >> who suffer from the same JTAG problem who wouldn't necessarily want >> lowest power. > > I can't say I am that familiar with the AM33xx architecture, but I know > that on OMAP this would keep on the debugss clock and power domain. > > Do you need this change to connect JTAG after the kernel has booted or > just to maintain JTAG during kernel boot? Actually both, only early stages of boot will have JTAG otherwise and it shuts off pretty quickly. > I am wondering if you should implement a new flag such as > "DONT_DISABLE_ON_INIT" so if the clock is on it is not disabled, but if > it is not enabled, you don't enabled it. Then that would be the best of > both worlds. I agree this is better. But the usecount element in the clock structure would need to increment so it is known that the clock is in use. >>> I was not sure if you are concerned about power at all. Or if having >>> that always enabled can inhibit low power states for suspend/idle etc. >> >> Maybe, suspend/resume code can turn off clock on suspend and enable it >> on resume. > > Yes, but not ideal for a product assuming they are power sensitive. Yes, true. It should be turned off when not required. Vaibhav made suggestion where flag is wrapped in a kernel debug config option. Regards, Joel ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] AM33XX: Clock: Fix JTAG disconnect during kernel boot @ 2012-11-16 23:30 Joel A Fernandes 0 siblings, 0 replies; 8+ messages in thread From: Joel A Fernandes @ 2012-11-16 23:30 UTC (permalink / raw) To: linux-omap Set ENABLE_ON_INIT for debugss clock so that it is on during init and the usecount is incremented. Not doing so causes kernel to disable debugss clock to save power when CONFIG_OMAP_RESET_CLOCKS is set, causing a JTAG disconnect. Signed-off-by: Joel A Fernandes <joelagnel@ti.com> Cc: Russ Dill <Russ.Dill@ti.com> Cc: Matt Porter <mporter@ti.com> Cc: Steve Kipisz <s-kipisz2@ti.com> Cc: Jason Kridner <jkridner@beagleboard.org> Cc: Tony Lindgren <tony@atomide.com> --- arch/arm/mach-omap2/clock33xx_data.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/mach-omap2/clock33xx_data.c b/arch/arm/mach-omap2/clock33xx_data.c index b7b7995..d0b4313 100644 --- a/arch/arm/mach-omap2/clock33xx_data.c +++ b/arch/arm/mach-omap2/clock33xx_data.c @@ -585,6 +585,7 @@ static struct clk debugss_ick = { .clkdm_name = "l3_aon_clkdm", .parent = &dpll_core_m4_ck, .ops = &clkops_omap2_dflt, + .flags = ENABLE_ON_INIT, .enable_reg = AM33XX_CM_WKUP_DEBUGSS_CLKCTRL, .enable_bit = AM33XX_MODULEMODE_SWCTRL, .recalc = &followparent_recalc, -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-11-22 21:04 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-11-16 23:44 [PATCH] AM33XX: Clock: Fix JTAG disconnect during kernel boot Joel A Fernandes 2012-11-17 0:12 ` Jon Hunter 2012-11-17 2:44 ` Joel A Fernandes 2012-11-19 16:15 ` Jon Hunter 2012-11-22 5:30 ` Vaibhav Hiremath 2012-11-22 21:04 ` Joel A Fernandes 2012-11-22 21:01 ` Joel A Fernandes -- strict thread matches above, loose matches on Subject: below -- 2012-11-16 23:30 Joel A Fernandes
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox