[PATCH 0/2] ARM: OMAP AM33XX: clock data: Enable clkout2 output on SoC pad

[PATCH 0/2] ARM: OMAP AM33XX: clock data: Enable clkout2 output on SoC pad

[Vaibhav Hiremath]

'clkout2' comes out on the device pad and is being used by various
external on-board peripherals like, Audio codecs, Wilink and stuff.
So enable the clkout2 by default during init sequence itself
along with right pinmux configuration clkout2 output.

Also, add the missing entry of "clkout2_ck" to the AM33xx clock table.

As far as enabling of clkout2 during init is concerned,
we can argue that it can be handled using DT property and enable
the clock only when specified; but not until OMAP clock-tree
migration to DT.

Vaibhav Hiremath (2):
  ARM: OMAP AM33XX: clock data: Enable clkout2 as part of init
  ARM: dts: AM33XX: Set pinmux for clkout2 pad used for clock output

 arch/arm/boot/dts/am335x-bone.dts     |    8 +++++++-
 arch/arm/boot/dts/am335x-evm.dts      |    8 +++++++-
 arch/arm/boot/dts/am335x-evmsk.dts    |    8 +++++++-
 arch/arm/mach-omap2/cclock33xx_data.c |    2 ++
 4 files changed, 23 insertions(+), 3 deletions(-)

[PATCH 1/2] ARM: OMAP AM33XX: clock data: Enable clkout2 as part of init

[Vaibhav Hiremath]

clkout2 comes out on the pad and is being used by various
external on-board peripherals like, Audio codecs and stuff.
So enable the clkout2 by default during init sequence itself.

Also, add the missing entry of "clkout2_ck" to the clock table.

Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
---
 arch/arm/mach-omap2/cclock33xx_data.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/cclock33xx_data.c b/arch/arm/mach-omap2/cclock33xx_data.c
index 6fd0ed1..a8140b6 100644
--- a/arch/arm/mach-omap2/cclock33xx_data.c
+++ b/arch/arm/mach-omap2/cclock33xx_data.c
@@ -979,6 +979,7 @@ static struct omap_clk am33xx_clks[] = {
 	CLK(NULL,	"trace_pmd_clk_mux_ck",	&trace_pmd_clk_mux_ck),
 	CLK(NULL,	"stm_clk_div_ck",	&stm_clk_div_ck),
 	CLK(NULL,	"trace_clk_div_ck",	&trace_clk_div_ck),
+	CLK(NULL,	"clkout2_ck",		&clkout2_ck),
 };
 
 
@@ -989,6 +990,7 @@ static const char *enable_init_clks[] = {
 	"l4hs_gclk",
 	"l4fw_gclk",
 	"l4ls_gclk",
+	"clkout2_ck",	/* Required for external peripherals like, Audio codecs */
 };
 
 int __init am33xx_clk_init(void)
-- 
1.7.0.4

[PATCH 1/2] ARM: OMAP AM33XX: clock data: Enable clkout2 as part of init

[Paul Walmsley]

On Tue, 7 May 2013, Vaibhav Hiremath wrote:

> clkout2 comes out on the pad and is being used by various
> external on-board peripherals like, Audio codecs and stuff.
> So enable the clkout2 by default during init sequence itself.

I don't like this: the clock should be enabled by the drivers for those 
external peripherals, not enabled by default.  So I think you should 
reconsider the part of the patch that enables it upon init.  But if you 
really want to do this, I'm not inclined to stand in the way; you can add 
my ack.

- Paul

> 
> Also, add the missing entry of "clkout2_ck" to the clock table.
> 
> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> ---
>  arch/arm/mach-omap2/cclock33xx_data.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/cclock33xx_data.c b/arch/arm/mach-omap2/cclock33xx_data.c
> index 6fd0ed1..a8140b6 100644
> --- a/arch/arm/mach-omap2/cclock33xx_data.c
> +++ b/arch/arm/mach-omap2/cclock33xx_data.c
> @@ -979,6 +979,7 @@ static struct omap_clk am33xx_clks[] = {
>  	CLK(NULL,	"trace_pmd_clk_mux_ck",	&trace_pmd_clk_mux_ck),
>  	CLK(NULL,	"stm_clk_div_ck",	&stm_clk_div_ck),
>  	CLK(NULL,	"trace_clk_div_ck",	&trace_clk_div_ck),
> +	CLK(NULL,	"clkout2_ck",		&clkout2_ck),
>  };
>  
>  
> @@ -989,6 +990,7 @@ static const char *enable_init_clks[] = {
>  	"l4hs_gclk",
>  	"l4fw_gclk",
>  	"l4ls_gclk",
> +	"clkout2_ck",	/* Required for external peripherals like, Audio codecs */
>  };
>  
>  int __init am33xx_clk_init(void)
> -- 
> 1.7.0.4
> 


- Paul

[PATCH 1/2] ARM: OMAP AM33XX: clock data: Enable clkout2 as part of init

[Hiremath, Vaibhav]

> -----Original Message-----
> From: Paul Walmsley [mailto:paul at pwsan.com]
> Sent: Monday, May 20, 2013 8:20 AM
> To: Hiremath, Vaibhav
> Cc: linux-omap at vger.kernel.org; tony at atomide.com; Cousson, Benoit;
> linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH 1/2] ARM: OMAP AM33XX: clock data: Enable clkout2
> as part of init
> 
> On Tue, 7 May 2013, Vaibhav Hiremath wrote:
> 
> > clkout2 comes out on the pad and is being used by various
> > external on-board peripherals like, Audio codecs and stuff.
> > So enable the clkout2 by default during init sequence itself.
> 
> I don't like this: the clock should be enabled by the drivers for those
> external peripherals, not enabled by default.  

Neither do I. And certainly respective driver should make sure that
he enables all required clocks.

The gap here is, we only support DT only boot and currently clock bindings
Is not coming from DT. Once we get there this needs to change.

Also, note that, I do not expect impact on PM (atleast on AM335x).

> So I think you should
> reconsider the part of the patch that enables it upon init.  But if you
> really want to do this, I'm not inclined to stand in the way; you can
> add
> my ack.
> 

Thanks for your ack, I will send out next version shortly.

Thanks,
Vaibhav

[PATCH 2/2] ARM: dts: AM33XX: Set pinmux for clkout2 pad used for clock output

[Vaibhav Hiremath]

xdma_event_intr1.clkout2 pad can be used to source clock
from either 32K OSC or any of the PLL (except MPU) outputs.
On the existing AM335x based boards (EVM, EVM-SK and Bone),
this pad is used to feed the clock to audio codes.

So, this patch configures the pinmux to get clkout2 on the pad.

Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
---
 arch/arm/boot/dts/am335x-bone.dts  |    8 +++++++-
 arch/arm/boot/dts/am335x-evm.dts   |    8 +++++++-
 arch/arm/boot/dts/am335x-evmsk.dts |    8 +++++++-
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts
index bfba6fc..f4630a3 100644
--- a/arch/arm/boot/dts/am335x-bone.dts
+++ b/arch/arm/boot/dts/am335x-bone.dts
@@ -26,7 +26,7 @@
 
 	am33xx_pinmux: pinmux at 44e10800 {
 		pinctrl-names = "default";
-		pinctrl-0 = <>;
+		pinctrl-0 = <&clkout2_pin>;
 
 		user_leds_s0: user_leds_s0 {
 			pinctrl-single,pins = <
@@ -50,6 +50,12 @@
 				0x174 0x00	/* uart0_txd.uart0_txd PULLDOWN | MODE0 */
 			>;
 		};
+
+		clkout2_pin: pinumx_clkout2_pin {
+			pinctrl-single,pins = <
+				0x1b4 0x03	/* xdma_event_intr1.clkout2 OMAP_MUX_MODE3 | AM33XX_PIN_OUTPUT */
+			>;
+		};
 	};
 
 	ocp {
diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index f598ed2..0673308 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -26,7 +26,7 @@
 
 	am33xx_pinmux: pinmux at 44e10800 {
 		pinctrl-names = "default";
-		pinctrl-0 = <&matrix_keypad_s0 &volume_keys_s0>;
+		pinctrl-0 = <&matrix_keypad_s0 &volume_keys_s0 &clkout2_pin>;
 
 		matrix_keypad_s0: matrix_keypad_s0 {
 			pinctrl-single,pins = <
@@ -65,6 +65,12 @@
 				0x174 0x00	/* uart0_txd.uart0_txd PULLDOWN | MODE0 */
 			>;
 		};
+
+		clkout2_pin: pinumx_clkout2_pin {
+			pinctrl-single,pins = <
+				0x1b4 0x03	/* xdma_event_intr1.clkout2 OMAP_MUX_MODE3 | AM33XX_PIN_OUTPUT */
+			>;
+		};
 	};
 
 	ocp {
diff --git a/arch/arm/boot/dts/am335x-evmsk.dts b/arch/arm/boot/dts/am335x-evmsk.dts
index 0eec644..a559389 100644
--- a/arch/arm/boot/dts/am335x-evmsk.dts
+++ b/arch/arm/boot/dts/am335x-evmsk.dts
@@ -32,7 +32,7 @@
 
 	am33xx_pinmux: pinmux at 44e10800 {
 		pinctrl-names = "default";
-		pinctrl-0 = <&gpio_keys_s0>;
+		pinctrl-0 = <&gpio_keys_s0 &clkout2_pin>;
 
 		user_leds_s0: user_leds_s0 {
 			pinctrl-single,pins = <
@@ -65,6 +65,12 @@
 				0x174 0x00	/* uart0_txd.uart0_txd PULLDOWN | MODE0 */
 			>;
 		};
+
+		clkout2_pin: pinumx_clkout2_pin {
+			pinctrl-single,pins = <
+				0x1b4 0x03	/* xdma_event_intr1.clkout2 OMAP_MUX_MODE3 | AM33XX_PIN_OUTPUT */
+			>;
+		};
 	};
 
 	ocp {
-- 
1.7.0.4

[PATCH 2/2] ARM: dts: AM33XX: Set pinmux for clkout2 pad used for clock output

[Paul Walmsley]

Hi

something that you should fix:

On Tue, 7 May 2013, Vaibhav Hiremath wrote:

> xdma_event_intr1.clkout2 pad can be used to source clock
> from either 32K OSC or any of the PLL (except MPU) outputs.
> On the existing AM335x based boards (EVM, EVM-SK and Bone),
> this pad is used to feed the clock to audio codes.
> 
> So, this patch configures the pinmux to get clkout2 on the pad.
> 
> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> ---
>  arch/arm/boot/dts/am335x-bone.dts  |    8 +++++++-
>  arch/arm/boot/dts/am335x-evm.dts   |    8 +++++++-
>  arch/arm/boot/dts/am335x-evmsk.dts |    8 +++++++-
>  3 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts
> index bfba6fc..f4630a3 100644
> --- a/arch/arm/boot/dts/am335x-bone.dts
> +++ b/arch/arm/boot/dts/am335x-bone.dts
> @@ -26,7 +26,7 @@
>  
>  	am33xx_pinmux: pinmux at 44e10800 {
>  		pinctrl-names = "default";
> -		pinctrl-0 = <>;
> +		pinctrl-0 = <&clkout2_pin>;
>  
>  		user_leds_s0: user_leds_s0 {
>  			pinctrl-single,pins = <
> @@ -50,6 +50,12 @@
>  				0x174 0x00	/* uart0_txd.uart0_txd PULLDOWN | MODE0 */
>  			>;
>  		};
> +
> +		clkout2_pin: pinumx_clkout2_pin {

"pinmux" is misspelled here and in several other parts of this file.




- Paul

[PATCH 2/2] ARM: dts: AM33XX: Set pinmux for clkout2 pad used for clock output

[Hiremath, Vaibhav]

> -----Original Message-----
> From: Paul Walmsley [mailto:paul at pwsan.com]
> Sent: Monday, May 20, 2013 8:21 AM
> To: Hiremath, Vaibhav
> Cc: linux-omap at vger.kernel.org; tony at atomide.com; Cousson, Benoit;
> linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH 2/2] ARM: dts: AM33XX: Set pinmux for clkout2 pad
> used for clock output
> 
> Hi
> 
> something that you should fix:
> 
> On Tue, 7 May 2013, Vaibhav Hiremath wrote:
> 
> > xdma_event_intr1.clkout2 pad can be used to source clock
> > from either 32K OSC or any of the PLL (except MPU) outputs.
> > On the existing AM335x based boards (EVM, EVM-SK and Bone),
> > this pad is used to feed the clock to audio codes.
> >
> > So, this patch configures the pinmux to get clkout2 on the pad.
> >
> > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> > ---
> >  arch/arm/boot/dts/am335x-bone.dts  |    8 +++++++-
> >  arch/arm/boot/dts/am335x-evm.dts   |    8 +++++++-
> >  arch/arm/boot/dts/am335x-evmsk.dts |    8 +++++++-
> >  3 files changed, 21 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/am335x-bone.dts
> b/arch/arm/boot/dts/am335x-bone.dts
> > index bfba6fc..f4630a3 100644
> > --- a/arch/arm/boot/dts/am335x-bone.dts
> > +++ b/arch/arm/boot/dts/am335x-bone.dts
> > @@ -26,7 +26,7 @@
> >
> >  	am33xx_pinmux: pinmux at 44e10800 {
> >  		pinctrl-names = "default";
> > -		pinctrl-0 = <>;
> > +		pinctrl-0 = <&clkout2_pin>;
> >
> >  		user_leds_s0: user_leds_s0 {
> >  			pinctrl-single,pins = <
> > @@ -50,6 +50,12 @@
> >  				0x174 0x00	/* uart0_txd.uart0_txd PULLDOWN |
> MODE0 */
> >  			>;
> >  		};
> > +
> > +		clkout2_pin: pinumx_clkout2_pin {
> 
> "pinmux" is misspelled here and in several other parts of this file.
> 

Very good catch. Once misspelled and it gets copy-pasted everywhere :)

Will send next version shortly.

Thanks,
Vaibhav

[PATCH 0/2] ARM: OMAP AM33XX: clock data: Enable clkout2 output on SoC pad

[Hiremath, Vaibhav]

> -----Original Message-----
> From: Hiremath, Vaibhav
> Sent: Tuesday, May 07, 2013 3:10 PM
> To: linux-omap at vger.kernel.org
> Cc: tony at atomide.com; Cousson, Benoit; paul at pwsan.com; linux-arm-
> kernel at lists.infradead.org; Hiremath, Vaibhav
> Subject: [PATCH 0/2] ARM: OMAP AM33XX: clock data: Enable clkout2
> output on SoC pad
> 
> 'clkout2' comes out on the device pad and is being used by various
> external on-board peripherals like, Audio codecs, Wilink and stuff.
> So enable the clkout2 by default during init sequence itself
> along with right pinmux configuration clkout2 output.
> 
> Also, add the missing entry of "clkout2_ck" to the AM33xx clock table.
> 
> As far as enabling of clkout2 during init is concerned,
> we can argue that it can be handled using DT property and enable
> the clock only when specified; but not until OMAP clock-tree
> migration to DT.
> 
> Vaibhav Hiremath (2):
>   ARM: OMAP AM33XX: clock data: Enable clkout2 as part of init
>   ARM: dts: AM33XX: Set pinmux for clkout2 pad used for clock output
> 
>  arch/arm/boot/dts/am335x-bone.dts     |    8 +++++++-
>  arch/arm/boot/dts/am335x-evm.dts      |    8 +++++++-
>  arch/arm/boot/dts/am335x-evmsk.dts    |    8 +++++++-
>  arch/arm/mach-omap2/cclock33xx_data.c |    2 ++
>  4 files changed, 23 insertions(+), 3 deletions(-)

Tony, Paul and Benoit,

Any update on this series?

Thanks,
Vaibhav

[PATCH 0/2] ARM: OMAP AM33XX: clock data: Enable clkout2 output on SoC pad

[Paul Walmsley]

On Fri, 17 May 2013, Hiremath, Vaibhav wrote:

> Tony, Paul and Benoit,
> 
> Any update on this series?

Most of the changes are DT-related, so it should probably go in via 
Beno?t.  I'll ack the clock side, but have a comment on patch 2, to be 
sent shortly.


- Paul

[PATCH 0/2] ARM: OMAP AM33XX: clock data: Enable clkout2 output on SoC pad

[Hiremath, Vaibhav]

> -----Original Message-----
> From: Paul Walmsley [mailto:paul at pwsan.com]
> Sent: Monday, May 20, 2013 8:17 AM
> To: Hiremath, Vaibhav
> Cc: linux-omap at vger.kernel.org; tony at atomide.com; Cousson, Benoit;
> linux-arm-kernel at lists.infradead.org
> Subject: RE: [PATCH 0/2] ARM: OMAP AM33XX: clock data: Enable clkout2
> output on SoC pad
> 
> On Fri, 17 May 2013, Hiremath, Vaibhav wrote:
> 
> > Tony, Paul and Benoit,
> >
> > Any update on this series?
> 
> Most of the changes are DT-related, so it should probably go in via
> Beno?t.  I'll ack the clock side, but have a comment on patch 2, to be
> sent shortly.
> 
> 

Thanks Paul for the review, I will next version shortly with your ack.


Thanks,
Vaibhav

[PATCH 0/2] ARM: OMAP AM33XX: clock data: Enable clkout2 output on SoC pad

[Benoit Cousson]

Salut Paul,

On 05/20/2013 04:47 AM, Paul Walmsley wrote:
> On Fri, 17 May 2013, Hiremath, Vaibhav wrote:
> 
>> Tony, Paul and Benoit,
>>
>> Any update on this series?
> 
> Most of the changes are DT-related, so it should probably go in via 
> Beno?t.  I'll ack the clock side, but have a comment on patch 2, to be 
> sent shortly.

I've tried to apply that patch, but unfortunately, it does depend on
that one:

https://patchwork.kernel.org/patch/2212081/

If you did not already pulled the first one I can take both with your
acked-by.
Otherwise, I can take only the DTS ones.

Is that OK?

Thanks,
Benoit

[PATCH 0/2] ARM: OMAP AM33XX: clock data: Enable clkout2 output on SoC pad

[Paul Walmsley]

Salut Beno?t,

On Mon, 27 May 2013, Benoit Cousson wrote:

> On 05/20/2013 04:47 AM, Paul Walmsley wrote:
> > On Fri, 17 May 2013, Hiremath, Vaibhav wrote:
> > 
> >> Tony, Paul and Benoit,
> >>
> >> Any update on this series?
> > 
> > Most of the changes are DT-related, so it should probably go in via 
> > Beno?t.  I'll ack the clock side, but have a comment on patch 2, to be 
> > sent shortly.
> 
> I've tried to apply that patch, but unfortunately, it does depend on
> that one:
> 
> https://patchwork.kernel.org/patch/2212081/
>
> If you did not already pulled the first one I can take both with your
> acked-by.
> Otherwise, I can take only the DTS ones.
> 
> Is that OK?

Will ack that first patch now.  

Vaibhav, looks like that first series is still in RFC form; you might want 
to repost it as a non-RFC series.


- Paul

[PATCH 0/2] ARM: OMAP AM33XX: clock data: Enable clkout2 output on SoC pad

[Benoit Cousson]

Salut Paul,

On 05/29/2013 09:06 PM, Paul Walmsley wrote:
> Salut Beno?t,
> 
> On Mon, 27 May 2013, Benoit Cousson wrote:
> 
>> On 05/20/2013 04:47 AM, Paul Walmsley wrote:
>>> On Fri, 17 May 2013, Hiremath, Vaibhav wrote:
>>>
>>>> Tony, Paul and Benoit,
>>>>
>>>> Any update on this series?
>>>
>>> Most of the changes are DT-related, so it should probably go in via 
>>> Beno?t.  I'll ack the clock side, but have a comment on patch 2, to be 
>>> sent shortly.
>>
>> I've tried to apply that patch, but unfortunately, it does depend on
>> that one:
>>
>> https://patchwork.kernel.org/patch/2212081/
>>
>> If you did not already pulled the first one I can take both with your
>> acked-by.
>> Otherwise, I can take only the DTS ones.
>>
>> Is that OK?
> 
> Will ack that first patch now.  

Cool, Thanks Paul.

Regards,
Benoit