[v2,2/2] ARM: dts: Append clock bindings for sai2 on VF610 platform
diff mbox

Message ID d09da3eb629143abd3f61ae037af4ddaf7286a83.1396432924.git.Guangyu.Chen@freescale.com
State New, archived
Headers show

Commit Message

Nicolin Chen April 2, 2014, 10:10 a.m. UTC
Since we added fours clock to the DT binding, we should update the current
SAI dts/dtsi so as not to break their functions.

Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
---
 arch/arm/boot/dts/vf610.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Shawn Guo April 2, 2014, 1:03 p.m. UTC | #1
On Wed, Apr 02, 2014 at 06:10:20PM +0800, Nicolin Chen wrote:
> Since we added fours clock to the DT binding, we should update the current
> SAI dts/dtsi so as not to break their functions.
> 
> Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
> ---
>  arch/arm/boot/dts/vf610.dtsi | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
> index d31ce1b..9fd0007 100644
> --- a/arch/arm/boot/dts/vf610.dtsi
> +++ b/arch/arm/boot/dts/vf610.dtsi
> @@ -139,8 +139,10 @@
>  				compatible = "fsl,vf610-sai";
>  				reg = <0x40031000 0x1000>;
>  				interrupts = <0 86 0x04>;
> -				clocks = <&clks VF610_CLK_SAI2>;
> -				clock-names = "sai";
> +				clocks = <&clks VF610_CLK_SAI2>,
> +				       <&clks VF610_CLK_SAI2>,
> +				       <&clks 0>, <&clks 0>;

So it seems that SAI on vf610 does work with only one clock.  So the
driver change will break old DTB for vf610?  If that's case, we will
have to need a new compatible for cases where 4 clocks are needed.

Shawn

> +				clock-names = "bus", "mclk1", "mclk2", "mclk3";
>  				status = "disabled";
>  			};
>  
> -- 
> 1.8.4
> 
>
Nicolin Chen April 2, 2014, 1:39 p.m. UTC | #2
Hi Shawn,

   Thanks for the comments, but...

On Wed, Apr 02, 2014 at 09:03:04PM +0800, Shawn Guo wrote:
> On Wed, Apr 02, 2014 at 06:10:20PM +0800, Nicolin Chen wrote:
> > Since we added fours clock to the DT binding, we should update the current
> > SAI dts/dtsi so as not to break their functions.
> > 
> > Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
> > ---
> >  arch/arm/boot/dts/vf610.dtsi | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
> > index d31ce1b..9fd0007 100644
> > --- a/arch/arm/boot/dts/vf610.dtsi
> > +++ b/arch/arm/boot/dts/vf610.dtsi
> > @@ -139,8 +139,10 @@
> >  				compatible = "fsl,vf610-sai";
> >  				reg = <0x40031000 0x1000>;
> >  				interrupts = <0 86 0x04>;
> > -				clocks = <&clks VF610_CLK_SAI2>;
> > -				clock-names = "sai";
> > +				clocks = <&clks VF610_CLK_SAI2>,
> > +				       <&clks VF610_CLK_SAI2>,
> > +				       <&clks 0>, <&clks 0>;
> 
> So it seems that SAI on vf610 does work with only one clock.  So the
> driver change will break old DTB for vf610?  If that's case, we will
> have to need a new compatible for cases where 4 clocks are needed.

According to Vybrid's RM Chapter 9.11.12 SAI clocking, the SoC actually
connects SAI with two clocks: SAI_CLK and Platform Bus Clock. So the DT
binding here still needs to be corrected even if ignoring driver change.

Besides, I've checked both SAI on imx and vf610 and found that they are
seemly identical, especially for the clock part -- "The transmitter and
receiver can independently select between the bus clock and up to three
audio master clocks to generate the bit clock." And the driver that was
designed for vf610 already contains the code to switch the clock between
Bus Clock and Three MCLKs. What I want to say is, even if SAI on vf610
does work with only one clock, it still doesn't have the full function
on vf610 -- driving clock from Platform Bus Clock unless we make this
improvement to the DT binding.

So I think it's fair to complete the code here for both platforms, even
though we might take the risk of merging conflict. And I understand
your point to avoid function break on those platform both of us aren't
convenient to test. But I've already involved Xiubo in the list. And
we can wait for his test result.

Hope you can understand the circumstance,
Nicolin
Xiubo Li April 4, 2014, 9:52 a.m. UTC | #3
> Subject: Re: [PATCH v2 2/2] ARM: dts: Append clock bindings for sai2 on VF610
> platform
> 
> Hi Shawn,
> 
>    Thanks for the comments, but...
> 
> On Wed, Apr 02, 2014 at 09:03:04PM +0800, Shawn Guo wrote:
> > On Wed, Apr 02, 2014 at 06:10:20PM +0800, Nicolin Chen wrote:
> > > Since we added fours clock to the DT binding, we should update the current
> > > SAI dts/dtsi so as not to break their functions.
> > >
> > > Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
> > > ---
> > >  arch/arm/boot/dts/vf610.dtsi | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
> > > index d31ce1b..9fd0007 100644
> > > --- a/arch/arm/boot/dts/vf610.dtsi
> > > +++ b/arch/arm/boot/dts/vf610.dtsi
> > > @@ -139,8 +139,10 @@
> > >  				compatible = "fsl,vf610-sai";
> > >  				reg = <0x40031000 0x1000>;
> > >  				interrupts = <0 86 0x04>;
> > > -				clocks = <&clks VF610_CLK_SAI2>;
> > > -				clock-names = "sai";
> > > +				clocks = <&clks VF610_CLK_SAI2>,
> > > +				       <&clks VF610_CLK_SAI2>,
> > > +				       <&clks 0>, <&clks 0>;
> >
> > So it seems that SAI on vf610 does work with only one clock.  So the
> > driver change will break old DTB for vf610?  If that's case, we will
> > have to need a new compatible for cases where 4 clocks are needed.
> 
> According to Vybrid's RM Chapter 9.11.12 SAI clocking, the SoC actually
> connects SAI with two clocks: SAI_CLK and Platform Bus Clock. So the DT
> binding here still needs to be corrected even if ignoring driver change.
> 
> Besides, I've checked both SAI on imx and vf610 and found that they are
> seemly identical, especially for the clock part -- "The transmitter and
> receiver can independently select between the bus clock and up to three
> audio master clocks to generate the bit clock." And the driver that was
> designed for vf610 already contains the code to switch the clock between
> Bus Clock and Three MCLKs. What I want to say is, even if SAI on vf610
> does work with only one clock, it still doesn't have the full function
> on vf610 -- driving clock from Platform Bus Clock unless we make this
> improvement to the DT binding.
> 
> So I think it's fair to complete the code here for both platforms, even
> though we might take the risk of merging conflict. And I understand
> your point to avoid function break on those platform both of us aren't
> convenient to test. But I've already involved Xiubo in the list. And
> we can wait for his test result.
> 

This has been test on my Vybird-TWR board and it's fine.

Thanks,

Brs
Xiubo



> Hope you can understand the circumstance,
> Nicolin
Mark Brown April 14, 2014, 8:38 p.m. UTC | #4
On Wed, Apr 02, 2014 at 06:10:20PM +0800, Nicolin Chen wrote:
> Since we added fours clock to the DT binding, we should update the current
> SAI dts/dtsi so as not to break their functions.

This doesn't apply against v3.15-rc1, can you please check and resend?
Nicolin Chen April 15, 2014, 2:28 a.m. UTC | #5
On Mon, Apr 14, 2014 at 09:38:51PM +0100, Mark Brown wrote:
> On Wed, Apr 02, 2014 at 06:10:20PM +0800, Nicolin Chen wrote:
> > Since we added fours clock to the DT binding, we should update the current
> > SAI dts/dtsi so as not to break their functions.
> 
> This doesn't apply against v3.15-rc1, can you please check and resend?

Please disregard this patch, since my v6 patch was compatible with the old
binding, the patch here is provisionally useless and I can later send it
via Shawn's tree.

Thank you,
Nicolin

Patch
diff mbox

diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
index d31ce1b..9fd0007 100644
--- a/arch/arm/boot/dts/vf610.dtsi
+++ b/arch/arm/boot/dts/vf610.dtsi
@@ -139,8 +139,10 @@ 
 				compatible = "fsl,vf610-sai";
 				reg = <0x40031000 0x1000>;
 				interrupts = <0 86 0x04>;
-				clocks = <&clks VF610_CLK_SAI2>;
-				clock-names = "sai";
+				clocks = <&clks VF610_CLK_SAI2>,
+				       <&clks VF610_CLK_SAI2>,
+				       <&clks 0>, <&clks 0>;
+				clock-names = "bus", "mclk1", "mclk2", "mclk3";
 				status = "disabled";
 			};