Re: [PATCH] AM33XX: Clock: Fix JTAG disconnect during kernel boot

2012-11-22 Thread Vaibhav Hiremath


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-hun...@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 majord...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] AM33XX: Clock: Fix JTAG disconnect during kernel boot

2012-11-22 Thread Joel A Fernandes
Hi Jon,

On Mon, Nov 19, 2012 at 10:15 AM, Jon Hunter jon-hun...@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-hun...@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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] AM33XX: Clock: Fix JTAG disconnect during kernel boot

2012-11-22 Thread Joel A Fernandes
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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] AM33XX: Clock: Fix JTAG disconnect during kernel boot

2012-11-19 Thread Jon Hunter

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-hun...@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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] AM33XX: Clock: Fix JTAG disconnect during kernel boot

2012-11-16 Thread Joel A Fernandes
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 joelag...@ti.com
Cc: Russ Dill russ.d...@ti.com
Cc: Matt Porter mpor...@ti.com
Cc: Steve Kipisz s-kipi...@ti.com
Cc: Jason Kridner jkrid...@beagleboard.org
Cc: Tony Lindgren t...@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

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] AM33XX: Clock: Fix JTAG disconnect during kernel boot

2012-11-16 Thread Joel A Fernandes
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 joelag...@ti.com
Cc: Russ Dill russ.d...@ti.com
Cc: Matt Porter mpor...@ti.com
Cc: Steve Kipisz s-kipi...@ti.com
Cc: Jason Kridner jkrid...@beagleboard.org
Cc: Tony Lindgren t...@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

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] AM33XX: Clock: Fix JTAG disconnect during kernel boot

2012-11-16 Thread Jon Hunter

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 joelag...@ti.com
 Cc: Russ Dill russ.d...@ti.com
 Cc: Matt Porter mpor...@ti.com
 Cc: Steve Kipisz s-kipi...@ti.com
 Cc: Jason Kridner jkrid...@beagleboard.org
 Cc: Tony Lindgren t...@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



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] AM33XX: Clock: Fix JTAG disconnect during kernel boot

2012-11-16 Thread Joel A Fernandes
Hi Jon,

On Fri, Nov 16, 2012 at 6:12 PM, Jon Hunter jon-hun...@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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html