diff mbox

[v3] dt/bindings: qoriq-clock: Add binding for FMan clock mux

Message ID 1429008968-24707-1-git-send-email-igal.liberman@freescale.com (mailing list archive)
State Changes Requested
Delegated to: Scott Wood
Headers show

Commit Message

Igal.Liberman April 14, 2015, 10:56 a.m. UTC 
From: Igal Liberman <Igal.Liberman@freescale.com>

v3: Addressed feedback from Scott:
	- Removed clock specifier description.

v2: Addressed feedback from Scott:
	- Moved the "fman-clk-mux" clock provider details
	  under "clocks" property.

Signed-off-by: Igal Liberman <Igal.Liberman@freescale.com>
---
 .../devicetree/bindings/clock/qoriq-clock.txt      |   17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Comments

Scott Wood April 15, 2015, 5:35 p.m. UTC | #1 
On Tue, 2015-04-14 at 13:56 +0300, Igal.Liberman wrote:
> From: Igal Liberman <Igal.Liberman@freescale.com>
> 
> v3: Addressed feedback from Scott:
> 	- Removed clock specifier description.
> 
> v2: Addressed feedback from Scott:
> 	- Moved the "fman-clk-mux" clock provider details
> 	  under "clocks" property.
> 
> Signed-off-by: Igal Liberman <Igal.Liberman@freescale.com>
> ---
>  .../devicetree/bindings/clock/qoriq-clock.txt      |   17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qoriq-clock.txt b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
> index b0d7b73..2bb3b38 100644
> --- a/Documentation/devicetree/bindings/clock/qoriq-clock.txt
> +++ b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
> @@ -65,9 +65,10 @@ Required properties:
>  		It takes parent's clock-frequency as its clock.
>  	* "fsl,qoriq-platform-pll-1.0" for the platform PLL clock (v1.0)
>  	* "fsl,qoriq-platform-pll-2.0" for the platform PLL clock (v2.0)
> +	* "fsl,fman-clk-mux" for the Frame Manager clock.
>  - #clock-cells: From common clock binding. The number of cells in a
> -	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0"
> -	clocks, or <1> for "fsl,qoriq-core-pll-[1,2].0" clocks.
> +	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0" and
> +	"fsl,fman-clk-mux" clocks or <1> for "fsl,qoriq-core-pll-[1,2].0".
>  	For "fsl,qoriq-core-pll-1.0" clocks, the single
>  	clock-specifier cell may take the following values:
>  	* 0 - equal to the PLL frequency
> @@ -145,6 +146,18 @@ Example for clock block and clock provider:
>  			clocks = <&sysclk>;
>  			clock-output-names = "platform-pll", "platform-pll-div2";
>  		};
> +
> +		fm0clk: fm0-clk-mux {
> +			#clock-cells = <0>;
> +			reg = <0x10 4>
> +			compatible = "fsl,fman-clk-mux";
> +			clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>, <&pll0 3>,
> +				 <&platform_pll 0>, <&pll1 1>, <&pll1 2>;
> +			clock-names = "pll0", "pll0-div2", "pll0-div3",
> +				      "pll0-div4", "platform-pll", "pll1-div2",
> +				      "pll1-div3";
> +			clock-output-names = "fm0-clk";
> +		};
>  	};
>  };
>  

I don't see this register in the manuals for older DPAA chips, such as
p4080 or p3041.  Is it present but undocumented?  Should I be looking
somewhere other than "Clocking Memory Map"?

-Scott
Igal.Liberman April 16, 2015, 6:11 a.m. UTC | #2 
Regards,
Igal Liberman.

> -----Original Message-----

> From: Wood Scott-B07421

> Sent: Wednesday, April 15, 2015 8:36 PM

> To: Liberman Igal-B31950

> Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org

> Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan clock mux

> 

> On Tue, 2015-04-14 at 13:56 +0300, Igal.Liberman wrote:

> > From: Igal Liberman <Igal.Liberman@freescale.com>

> >

> > v3: Addressed feedback from Scott:

> > 	- Removed clock specifier description.

> >

> > v2: Addressed feedback from Scott:

> > 	- Moved the "fman-clk-mux" clock provider details

> > 	  under "clocks" property.

> >

> > Signed-off-by: Igal Liberman <Igal.Liberman@freescale.com>

> > ---

> >  .../devicetree/bindings/clock/qoriq-clock.txt      |   17 +++++++++++++++--

> >  1 file changed, 15 insertions(+), 2 deletions(-)

> >

> > diff --git a/Documentation/devicetree/bindings/clock/qoriq-clock.txt

> > b/Documentation/devicetree/bindings/clock/qoriq-clock.txt

> > index b0d7b73..2bb3b38 100644

> > --- a/Documentation/devicetree/bindings/clock/qoriq-clock.txt

> > +++ b/Documentation/devicetree/bindings/clock/qoriq-clock.txt

> > @@ -65,9 +65,10 @@ Required properties:

> >  		It takes parent's clock-frequency as its clock.

> >  	* "fsl,qoriq-platform-pll-1.0" for the platform PLL clock (v1.0)

> >  	* "fsl,qoriq-platform-pll-2.0" for the platform PLL clock (v2.0)

> > +	* "fsl,fman-clk-mux" for the Frame Manager clock.

> >  - #clock-cells: From common clock binding. The number of cells in a

> > -	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0"

> > -	clocks, or <1> for "fsl,qoriq-core-pll-[1,2].0" clocks.

> > +	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0" and

> > +	"fsl,fman-clk-mux" clocks or <1> for "fsl,qoriq-core-pll-[1,2].0".

> >  	For "fsl,qoriq-core-pll-1.0" clocks, the single

> >  	clock-specifier cell may take the following values:

> >  	* 0 - equal to the PLL frequency

> > @@ -145,6 +146,18 @@ Example for clock block and clock provider:

> >  			clocks = <&sysclk>;

> >  			clock-output-names = "platform-pll", "platform-pll-

> div2";

> >  		};

> > +

> > +		fm0clk: fm0-clk-mux {

> > +			#clock-cells = <0>;

> > +			reg = <0x10 4>

> > +			compatible = "fsl,fman-clk-mux";

> > +			clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>, <&pll0 3>,

> > +				 <&platform_pll 0>, <&pll1 1>, <&pll1 2>;

> > +			clock-names = "pll0", "pll0-div2", "pll0-div3",

> > +				      "pll0-div4", "platform-pll", "pll1-div2",

> > +				      "pll1-div3";

> > +			clock-output-names = "fm0-clk";

> > +		};

> >  	};

> >  };

> >

> 

> I don't see this register in the manuals for older DPAA chips, such as

> p4080 or p3041.  Is it present but undocumented?  Should I be looking

> somewhere other than "Clocking Memory Map"?

> 


It's available only in part of the new chips (T4, T2, B4).
In T1024/T1040 there's only one option for FMan clock so this register is not available.

> -Scott

> 


Igal.
Scott Wood April 17, 2015, 5:41 a.m. UTC | #3 
On Thu, 2015-04-16 at 01:11 -0500, Liberman Igal-B31950 wrote:
> 
> 
> Regards,
> Igal Liberman.
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Wednesday, April 15, 2015 8:36 PM
> > To: Liberman Igal-B31950
> > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> > Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan clock mux
> > 
> > On Tue, 2015-04-14 at 13:56 +0300, Igal.Liberman wrote:
> > > From: Igal Liberman <Igal.Liberman@freescale.com>
> > >
> > > v3: Addressed feedback from Scott:
> > > 	- Removed clock specifier description.
> > >
> > > v2: Addressed feedback from Scott:
> > > 	- Moved the "fman-clk-mux" clock provider details
> > > 	  under "clocks" property.
> > >
> > > Signed-off-by: Igal Liberman <Igal.Liberman@freescale.com>
> > > ---
> > >  .../devicetree/bindings/clock/qoriq-clock.txt      |   17 +++++++++++++++--
> > >  1 file changed, 15 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/clock/qoriq-clock.txt
> > > b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
> > > index b0d7b73..2bb3b38 100644
> > > --- a/Documentation/devicetree/bindings/clock/qoriq-clock.txt
> > > +++ b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
> > > @@ -65,9 +65,10 @@ Required properties:
> > >  		It takes parent's clock-frequency as its clock.
> > >  	* "fsl,qoriq-platform-pll-1.0" for the platform PLL clock (v1.0)
> > >  	* "fsl,qoriq-platform-pll-2.0" for the platform PLL clock (v2.0)
> > > +	* "fsl,fman-clk-mux" for the Frame Manager clock.
> > >  - #clock-cells: From common clock binding. The number of cells in a
> > > -	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0"
> > > -	clocks, or <1> for "fsl,qoriq-core-pll-[1,2].0" clocks.
> > > +	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0" and
> > > +	"fsl,fman-clk-mux" clocks or <1> for "fsl,qoriq-core-pll-[1,2].0".
> > >  	For "fsl,qoriq-core-pll-1.0" clocks, the single
> > >  	clock-specifier cell may take the following values:
> > >  	* 0 - equal to the PLL frequency
> > > @@ -145,6 +146,18 @@ Example for clock block and clock provider:
> > >  			clocks = <&sysclk>;
> > >  			clock-output-names = "platform-pll", "platform-pll-
> > div2";
> > >  		};
> > > +
> > > +		fm0clk: fm0-clk-mux {
> > > +			#clock-cells = <0>;
> > > +			reg = <0x10 4>
> > > +			compatible = "fsl,fman-clk-mux";
> > > +			clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>, <&pll0 3>,
> > > +				 <&platform_pll 0>, <&pll1 1>, <&pll1 2>;
> > > +			clock-names = "pll0", "pll0-div2", "pll0-div3",
> > > +				      "pll0-div4", "platform-pll", "pll1-div2",
> > > +				      "pll1-div3";
> > > +			clock-output-names = "fm0-clk";
> > > +		};
> > >  	};
> > >  };
> > >
> > 
> > I don't see this register in the manuals for older DPAA chips, such as
> > p4080 or p3041.  Is it present but undocumented?  Should I be looking
> > somewhere other than "Clocking Memory Map"?
> > 
> 
> It's available only in part of the new chips (T4, T2, B4).
> In T1024/T1040 there's only one option for FMan clock so this register is not available.

So it's part of the 2.0 chassis?  I'd stick a 2.0 in there, then.  Who
knows what we may see in the future.

-Scott
Igal.Liberman April 20, 2015, 11:07 a.m. UTC | #4 
Regards,
Igal Liberman.

> -----Original Message-----

> From: Wood Scott-B07421

> Sent: Friday, April 17, 2015 8:41 AM

> To: Liberman Igal-B31950

> Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org

> Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan clock mux

> 

> On Thu, 2015-04-16 at 01:11 -0500, Liberman Igal-B31950 wrote:

> >

> >

> > Regards,

> > Igal Liberman.

> >

> > > -----Original Message-----

> > > From: Wood Scott-B07421

> > > Sent: Wednesday, April 15, 2015 8:36 PM

> > > To: Liberman Igal-B31950

> > > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org

> > > Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan

> > > clock mux

> > >

> > > On Tue, 2015-04-14 at 13:56 +0300, Igal.Liberman wrote:

> > > > From: Igal Liberman <Igal.Liberman@freescale.com>

> > > >

> > > > v3: Addressed feedback from Scott:

> > > > 	- Removed clock specifier description.

> > > >

> > > > v2: Addressed feedback from Scott:

> > > > 	- Moved the "fman-clk-mux" clock provider details

> > > > 	  under "clocks" property.

> > > >

> > > > Signed-off-by: Igal Liberman <Igal.Liberman@freescale.com>

> > > > ---

> > > >  .../devicetree/bindings/clock/qoriq-clock.txt      |   17

> +++++++++++++++--

> > > >  1 file changed, 15 insertions(+), 2 deletions(-)

> > > >

> > > > diff --git

> > > > a/Documentation/devicetree/bindings/clock/qoriq-clock.txt

> > > > b/Documentation/devicetree/bindings/clock/qoriq-clock.txt

> > > > index b0d7b73..2bb3b38 100644

> > > > --- a/Documentation/devicetree/bindings/clock/qoriq-clock.txt

> > > > +++ b/Documentation/devicetree/bindings/clock/qoriq-clock.txt

> > > > @@ -65,9 +65,10 @@ Required properties:

> > > >  		It takes parent's clock-frequency as its clock.

> > > >  	* "fsl,qoriq-platform-pll-1.0" for the platform PLL clock (v1.0)

> > > >  	* "fsl,qoriq-platform-pll-2.0" for the platform PLL clock (v2.0)

> > > > +	* "fsl,fman-clk-mux" for the Frame Manager clock.

> > > >  - #clock-cells: From common clock binding. The number of cells in a

> > > > -	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0"

> > > > -	clocks, or <1> for "fsl,qoriq-core-pll-[1,2].0" clocks.

> > > > +	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0" and

> > > > +	"fsl,fman-clk-mux" clocks or <1> for "fsl,qoriq-core-pll-[1,2].0".

> > > >  	For "fsl,qoriq-core-pll-1.0" clocks, the single

> > > >  	clock-specifier cell may take the following values:

> > > >  	* 0 - equal to the PLL frequency @@ -145,6 +146,18 @@ Example

> > > > for clock block and clock provider:

> > > >  			clocks = <&sysclk>;

> > > >  			clock-output-names = "platform-pll", "platform-pll-

> > > div2";

> > > >  		};

> > > > +

> > > > +		fm0clk: fm0-clk-mux {

> > > > +			#clock-cells = <0>;

> > > > +			reg = <0x10 4>

> > > > +			compatible = "fsl,fman-clk-mux";

> > > > +			clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>, <&pll0 3>,

> > > > +				 <&platform_pll 0>, <&pll1 1>, <&pll1 2>;

> > > > +			clock-names = "pll0", "pll0-div2", "pll0-div3",

> > > > +				      "pll0-div4", "platform-pll", "pll1-div2",

> > > > +				      "pll1-div3";

> > > > +			clock-output-names = "fm0-clk";

> > > > +		};

> > > >  	};

> > > >  };

> > > >

> > >

> > > I don't see this register in the manuals for older DPAA chips, such

> > > as

> > > p4080 or p3041.  Is it present but undocumented?  Should I be

> > > looking somewhere other than "Clocking Memory Map"?

> > >

> >

> > It's available only in part of the new chips (T4, T2, B4).

> > In T1024/T1040 there's only one option for FMan clock so this register is not

> available.

> 

> So it's part of the 2.0 chassis?  I'd stick a 2.0 in there, then.  Who knows what

> we may see in the future.

> 


OK,
We can go with "fsl,fman-clk-mux-1/2-0.".
In that case, we need to update FMan nodes and the clock driver:
https://patchwork.ozlabs.org/patch/443973/
https://patchwork.ozlabs.org/patch/461813/
I will update those patches separately.

> -Scott

> 


Igal
Igal.Liberman April 20, 2015, 11:40 a.m. UTC | #5 
Regards,
Igal Liberman.

> -----Original Message-----

> From: Liberman Igal-B31950

> Sent: Monday, April 20, 2015 2:07 PM

> To: Wood Scott-B07421

> Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org

> Subject: RE: [v3] dt/bindings: qoriq-clock: Add binding for FMan clock mux

> 

> 

> 

> Regards,

> Igal Liberman.

> 

> > -----Original Message-----

> > From: Wood Scott-B07421

> > Sent: Friday, April 17, 2015 8:41 AM

> > To: Liberman Igal-B31950

> > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org

> > Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan clock

> > mux

> >

> > On Thu, 2015-04-16 at 01:11 -0500, Liberman Igal-B31950 wrote:

> > >

> > >

> > > Regards,

> > > Igal Liberman.

> > >

> > > > -----Original Message-----

> > > > From: Wood Scott-B07421

> > > > Sent: Wednesday, April 15, 2015 8:36 PM

> > > > To: Liberman Igal-B31950

> > > > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org

> > > > Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan

> > > > clock mux

> > > >

> > > > On Tue, 2015-04-14 at 13:56 +0300, Igal.Liberman wrote:

> > > > > From: Igal Liberman <Igal.Liberman@freescale.com>

> > > > >

> > > > > v3: Addressed feedback from Scott:

> > > > > 	- Removed clock specifier description.

> > > > >

> > > > > v2: Addressed feedback from Scott:

> > > > > 	- Moved the "fman-clk-mux" clock provider details

> > > > > 	  under "clocks" property.

> > > > >

> > > > > Signed-off-by: Igal Liberman <Igal.Liberman@freescale.com>

> > > > > ---

> > > > >  .../devicetree/bindings/clock/qoriq-clock.txt      |   17

> > +++++++++++++++--

> > > > >  1 file changed, 15 insertions(+), 2 deletions(-)

> > > > >

> > > > > diff --git

> > > > > a/Documentation/devicetree/bindings/clock/qoriq-clock.txt

> > > > > b/Documentation/devicetree/bindings/clock/qoriq-clock.txt

> > > > > index b0d7b73..2bb3b38 100644

> > > > > --- a/Documentation/devicetree/bindings/clock/qoriq-clock.txt

> > > > > +++ b/Documentation/devicetree/bindings/clock/qoriq-clock.txt

> > > > > @@ -65,9 +65,10 @@ Required properties:

> > > > >  		It takes parent's clock-frequency as its clock.

> > > > >  	* "fsl,qoriq-platform-pll-1.0" for the platform PLL clock (v1.0)

> > > > >  	* "fsl,qoriq-platform-pll-2.0" for the platform PLL clock

> > > > > (v2.0)

> > > > > +	* "fsl,fman-clk-mux" for the Frame Manager clock.

> > > > >  - #clock-cells: From common clock binding. The number of cells in a

> > > > > -	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0"

> > > > > -	clocks, or <1> for "fsl,qoriq-core-pll-[1,2].0" clocks.

> > > > > +	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0"

> and

> > > > > +	"fsl,fman-clk-mux" clocks or <1> for "fsl,qoriq-core-pll-

> [1,2].0".

> > > > >  	For "fsl,qoriq-core-pll-1.0" clocks, the single

> > > > >  	clock-specifier cell may take the following values:

> > > > >  	* 0 - equal to the PLL frequency @@ -145,6 +146,18 @@

> Example

> > > > > for clock block and clock provider:

> > > > >  			clocks = <&sysclk>;

> > > > >  			clock-output-names = "platform-pll",

> "platform-pll-

> > > > div2";

> > > > >  		};

> > > > > +

> > > > > +		fm0clk: fm0-clk-mux {

> > > > > +			#clock-cells = <0>;

> > > > > +			reg = <0x10 4>

> > > > > +			compatible = "fsl,fman-clk-mux";

> > > > > +			clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>,

> <&pll0 3>,

> > > > > +				 <&platform_pll 0>, <&pll1 1>, <&pll1

> 2>;

> > > > > +			clock-names = "pll0", "pll0-div2", "pll0-div3",

> > > > > +				      "pll0-div4", "platform-pll", "pll1-

> div2",

> > > > > +				      "pll1-div3";

> > > > > +			clock-output-names = "fm0-clk";

> > > > > +		};

> > > > >  	};

> > > > >  };

> > > > >

> > > >

> > > > I don't see this register in the manuals for older DPAA chips,

> > > > such as

> > > > p4080 or p3041.  Is it present but undocumented?  Should I be

> > > > looking somewhere other than "Clocking Memory Map"?

> > > >

> > >

> > > It's available only in part of the new chips (T4, T2, B4).

> > > In T1024/T1040 there's only one option for FMan clock so this

> > > register is not

> > available.

> >

> > So it's part of the 2.0 chassis?  I'd stick a 2.0 in there, then.  Who

> > knows what we may see in the future.

> >

> 

> OK,

> We can go with "fsl,fman-clk-mux-1/2-0.".

> In that case, we need to update FMan nodes and the clock driver:

> https://patchwork.ozlabs.org/patch/443973/

> https://patchwork.ozlabs.org/patch/461813/

> I will update those patches separately.

> 


Scott,
There are 2 options:
Use "fsl,fman-clk-mux-1.0" for SoC without CLKCGnHWACSR register.
Use "fsl,fman-clk-mux-2.0" for SoC with CLKCGnHWACSR register.
Or
Use "fsl,fman-clk-mux-1.0" for SoC which support FMan V2 (Pxxxx)
Use "fsl,fman-clk-mux-2.0" for SoC which support FMan V3 (B/T)

Using the 1st option might be confusing because core pll/mux 2.0 represents B/T devices and 1.0 represent Pxxxx.
In this case, T1040 uses "fsl,qoriq-core-pll/mux-2.0" and "fsl,fman-clk-mux-1.0".
On the other hand, the second option doesn't distinguishes between T4 and T1 (for example), as T1 doesn't have reg property while T4 has.

What do you think?
 
> > -Scott

> >

> 

> Igal


Igal
Scott Wood April 21, 2015, 12:51 a.m. UTC | #6 
On Mon, 2015-04-20 at 06:40 -0500, Liberman Igal-B31950 wrote:
> 
> 
> Regards,
> Igal Liberman.
> 
> > -----Original Message-----
> > From: Liberman Igal-B31950
> > Sent: Monday, April 20, 2015 2:07 PM
> > To: Wood Scott-B07421
> > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> > Subject: RE: [v3] dt/bindings: qoriq-clock: Add binding for FMan clock mux
> > 
> > 
> > 
> > Regards,
> > Igal Liberman.
> > 
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Friday, April 17, 2015 8:41 AM
> > > To: Liberman Igal-B31950
> > > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> > > Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan clock
> > > mux
> > >
> > > On Thu, 2015-04-16 at 01:11 -0500, Liberman Igal-B31950 wrote:
> > > >
> > > >
> > > > Regards,
> > > > Igal Liberman.
> > > >
> > > > > -----Original Message-----
> > > > > From: Wood Scott-B07421
> > > > > Sent: Wednesday, April 15, 2015 8:36 PM
> > > > > To: Liberman Igal-B31950
> > > > > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> > > > > Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan
> > > > > clock mux
> > > > >
> > > > > On Tue, 2015-04-14 at 13:56 +0300, Igal.Liberman wrote:
> > > > > > From: Igal Liberman <Igal.Liberman@freescale.com>
> > > > > >
> > > > > > v3: Addressed feedback from Scott:
> > > > > > 	- Removed clock specifier description.
> > > > > >
> > > > > > v2: Addressed feedback from Scott:
> > > > > > 	- Moved the "fman-clk-mux" clock provider details
> > > > > > 	  under "clocks" property.
> > > > > >
> > > > > > Signed-off-by: Igal Liberman <Igal.Liberman@freescale.com>
> > > > > > ---
> > > > > >  .../devicetree/bindings/clock/qoriq-clock.txt      |   17
> > > +++++++++++++++--
> > > > > >  1 file changed, 15 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git
> > > > > > a/Documentation/devicetree/bindings/clock/qoriq-clock.txt
> > > > > > b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
> > > > > > index b0d7b73..2bb3b38 100644
> > > > > > --- a/Documentation/devicetree/bindings/clock/qoriq-clock.txt
> > > > > > +++ b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
> > > > > > @@ -65,9 +65,10 @@ Required properties:
> > > > > >  		It takes parent's clock-frequency as its clock.
> > > > > >  	* "fsl,qoriq-platform-pll-1.0" for the platform PLL clock (v1.0)
> > > > > >  	* "fsl,qoriq-platform-pll-2.0" for the platform PLL clock
> > > > > > (v2.0)
> > > > > > +	* "fsl,fman-clk-mux" for the Frame Manager clock.
> > > > > >  - #clock-cells: From common clock binding. The number of cells in a
> > > > > > -	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0"
> > > > > > -	clocks, or <1> for "fsl,qoriq-core-pll-[1,2].0" clocks.
> > > > > > +	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0"
> > and
> > > > > > +	"fsl,fman-clk-mux" clocks or <1> for "fsl,qoriq-core-pll-
> > [1,2].0".
> > > > > >  	For "fsl,qoriq-core-pll-1.0" clocks, the single
> > > > > >  	clock-specifier cell may take the following values:
> > > > > >  	* 0 - equal to the PLL frequency @@ -145,6 +146,18 @@
> > Example
> > > > > > for clock block and clock provider:
> > > > > >  			clocks = <&sysclk>;
> > > > > >  			clock-output-names = "platform-pll",
> > "platform-pll-
> > > > > div2";
> > > > > >  		};
> > > > > > +
> > > > > > +		fm0clk: fm0-clk-mux {
> > > > > > +			#clock-cells = <0>;
> > > > > > +			reg = <0x10 4>
> > > > > > +			compatible = "fsl,fman-clk-mux";
> > > > > > +			clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>,
> > <&pll0 3>,
> > > > > > +				 <&platform_pll 0>, <&pll1 1>, <&pll1
> > 2>;
> > > > > > +			clock-names = "pll0", "pll0-div2", "pll0-div3",
> > > > > > +				      "pll0-div4", "platform-pll", "pll1-
> > div2",
> > > > > > +				      "pll1-div3";
> > > > > > +			clock-output-names = "fm0-clk";
> > > > > > +		};
> > > > > >  	};
> > > > > >  };
> > > > > >
> > > > >
> > > > > I don't see this register in the manuals for older DPAA chips,
> > > > > such as
> > > > > p4080 or p3041.  Is it present but undocumented?  Should I be
> > > > > looking somewhere other than "Clocking Memory Map"?
> > > > >
> > > >
> > > > It's available only in part of the new chips (T4, T2, B4).
> > > > In T1024/T1040 there's only one option for FMan clock so this
> > > > register is not
> > > available.
> > >
> > > So it's part of the 2.0 chassis?  I'd stick a 2.0 in there, then.  Who
> > > knows what we may see in the future.
> > >
> > 
> > OK,
> > We can go with "fsl,fman-clk-mux-1/2-0.".
> > In that case, we need to update FMan nodes and the clock driver:
> > https://patchwork.ozlabs.org/patch/443973/
> > https://patchwork.ozlabs.org/patch/461813/
> > I will update those patches separately.
> > 
> 
> Scott,
> There are 2 options:
> Use "fsl,fman-clk-mux-1.0" for SoC without CLKCGnHWACSR register.
> Use "fsl,fman-clk-mux-2.0" for SoC with CLKCGnHWACSR register.
> Or
> Use "fsl,fman-clk-mux-1.0" for SoC which support FMan V2 (Pxxxx)
> Use "fsl,fman-clk-mux-2.0" for SoC which support FMan V3 (B/T)

1.0/2.0 in the clockgen node refers to chassis version.  It has nothing
to do with FMan version.

In fact, fman should not be in the compatible because, now that I found
the documentation for this, I see that it's more generic than that.
"fman-clk-mux" should be replaced with "qoriq-hwacsr".

> Using the 1st option might be confusing because core pll/mux 2.0 represents B/T devices and 1.0 represent Pxxxx.
> In this case, T1040 uses "fsl,qoriq-core-pll/mux-2.0" and "fsl,fman-clk-mux-1.0".
> On the other hand, the second option doesn't distinguishes between T4 and T1 (for example), as T1 doesn't have reg property while T4 has.

How would t1040 have a so-called "fman-clk-mux" node at all if it
doesn't have this register?

I think we've made a mess of the clock bindings and this is only going
to make it worse.  We need to stay compatible with the mess we've made,
but I'm inclined to say that we shouldn't add more nodes to it.  

Instead, have the toplevel clockgen node have a chip-based compatible in
addition to version (e.g. compatible = "fsl,t4240-clockgen",
"fsl,qoriq-clockgen-2.0") and have the toplevel node export whatever
clocks are needed.  Put the logic to deal with all the different
dividers, register values, and such in code, not the device tree.  The
binding should be focused on how to encode the specifier of the exported
clocks.

-Scott
Igal.Liberman April 22, 2015, 10:47 a.m. UTC | #7 
Regards,
Igal Liberman.

> -----Original Message-----

> From: Wood Scott-B07421

> Sent: Tuesday, April 21, 2015 3:52 AM

> To: Liberman Igal-B31950

> Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Tang

> Yuantian-B29983

> Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan clock mux

> 

> On Mon, 2015-04-20 at 06:40 -0500, Liberman Igal-B31950 wrote:

> >

> >

> > Regards,

> > Igal Liberman.

> >

> > > -----Original Message-----

> > > From: Liberman Igal-B31950

> > > Sent: Monday, April 20, 2015 2:07 PM

> > > To: Wood Scott-B07421

> > > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org

> > > Subject: RE: [v3] dt/bindings: qoriq-clock: Add binding for FMan

> > > clock mux

> > >

> > >

> > >

> > > Regards,

> > > Igal Liberman.

> > >

> > > > -----Original Message-----

> > > > From: Wood Scott-B07421

> > > > Sent: Friday, April 17, 2015 8:41 AM

> > > > To: Liberman Igal-B31950

> > > > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org

> > > > Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan

> > > > clock mux

> > > >

> > > > On Thu, 2015-04-16 at 01:11 -0500, Liberman Igal-B31950 wrote:

> > > > >

> > > > >

> > > > > Regards,

> > > > > Igal Liberman.

> > > > >

> > > > > > -----Original Message-----

> > > > > > From: Wood Scott-B07421

> > > > > > Sent: Wednesday, April 15, 2015 8:36 PM

> > > > > > To: Liberman Igal-B31950

> > > > > > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org

> > > > > > Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for

> > > > > > FMan clock mux

> > > > > >

> > > > > > On Tue, 2015-04-14 at 13:56 +0300, Igal.Liberman wrote:

> > > > > > > From: Igal Liberman <Igal.Liberman@freescale.com>

> > > > > > >

> > > > > > > v3: Addressed feedback from Scott:

> > > > > > > 	- Removed clock specifier description.

> > > > > > >

> > > > > > > v2: Addressed feedback from Scott:

> > > > > > > 	- Moved the "fman-clk-mux" clock provider details

> > > > > > > 	  under "clocks" property.

> > > > > > >

> > > > > > > Signed-off-by: Igal Liberman <Igal.Liberman@freescale.com>

> > > > > > > ---

> > > > > > >  .../devicetree/bindings/clock/qoriq-clock.txt      |   17

> > > > +++++++++++++++--

> > > > > > >  1 file changed, 15 insertions(+), 2 deletions(-)

> > > > > > >

> > > > > > > diff --git

> > > > > > > a/Documentation/devicetree/bindings/clock/qoriq-clock.txt

> > > > > > > b/Documentation/devicetree/bindings/clock/qoriq-clock.txt

> > > > > > > index b0d7b73..2bb3b38 100644

> > > > > > > ---

> > > > > > > a/Documentation/devicetree/bindings/clock/qoriq-clock.txt

> > > > > > > +++ b/Documentation/devicetree/bindings/clock/qoriq-clock.tx

> > > > > > > +++ t

> > > > > > > @@ -65,9 +65,10 @@ Required properties:

> > > > > > >  		It takes parent's clock-frequency as its clock.

> > > > > > >  	* "fsl,qoriq-platform-pll-1.0" for the platform PLL clock (v1.0)

> > > > > > >  	* "fsl,qoriq-platform-pll-2.0" for the platform PLL clock

> > > > > > > (v2.0)

> > > > > > > +	* "fsl,fman-clk-mux" for the Frame Manager clock.

> > > > > > >  - #clock-cells: From common clock binding. The number of cells in

> a

> > > > > > > -	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0"

> > > > > > > -	clocks, or <1> for "fsl,qoriq-core-pll-[1,2].0" clocks.

> > > > > > > +	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0"

> > > and

> > > > > > > +	"fsl,fman-clk-mux" clocks or <1> for "fsl,qoriq-core-pll-

> > > [1,2].0".

> > > > > > >  	For "fsl,qoriq-core-pll-1.0" clocks, the single

> > > > > > >  	clock-specifier cell may take the following values:

> > > > > > >  	* 0 - equal to the PLL frequency @@ -145,6 +146,18 @@

> > > Example

> > > > > > > for clock block and clock provider:

> > > > > > >  			clocks = <&sysclk>;

> > > > > > >  			clock-output-names = "platform-pll",

> > > "platform-pll-

> > > > > > div2";

> > > > > > >  		};

> > > > > > > +

> > > > > > > +		fm0clk: fm0-clk-mux {

> > > > > > > +			#clock-cells = <0>;

> > > > > > > +			reg = <0x10 4>

> > > > > > > +			compatible = "fsl,fman-clk-mux";

> > > > > > > +			clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>,

> > > <&pll0 3>,

> > > > > > > +				 <&platform_pll 0>, <&pll1 1>, <&pll1

> > > 2>;

> > > > > > > +			clock-names = "pll0", "pll0-div2", "pll0-div3",

> > > > > > > +				      "pll0-div4", "platform-pll", "pll1-

> > > div2",

> > > > > > > +				      "pll1-div3";

> > > > > > > +			clock-output-names = "fm0-clk";

> > > > > > > +		};

> > > > > > >  	};

> > > > > > >  };

> > > > > > >

> > > > > >

> > > > > > I don't see this register in the manuals for older DPAA chips,

> > > > > > such as

> > > > > > p4080 or p3041.  Is it present but undocumented?  Should I be

> > > > > > looking somewhere other than "Clocking Memory Map"?

> > > > > >

> > > > >

> > > > > It's available only in part of the new chips (T4, T2, B4).

> > > > > In T1024/T1040 there's only one option for FMan clock so this

> > > > > register is not

> > > > available.

> > > >

> > > > So it's part of the 2.0 chassis?  I'd stick a 2.0 in there, then.

> > > > Who knows what we may see in the future.

> > > >

> > >

> > > OK,

> > > We can go with "fsl,fman-clk-mux-1/2-0.".

> > > In that case, we need to update FMan nodes and the clock driver:

> > > https://patchwork.ozlabs.org/patch/443973/

> > > https://patchwork.ozlabs.org/patch/461813/

> > > I will update those patches separately.

> > >

> >

> > Scott,

> > There are 2 options:

> > Use "fsl,fman-clk-mux-1.0" for SoC without CLKCGnHWACSR register.

> > Use "fsl,fman-clk-mux-2.0" for SoC with CLKCGnHWACSR register.

> > Or

> > Use "fsl,fman-clk-mux-1.0" for SoC which support FMan V2 (Pxxxx) Use

> > "fsl,fman-clk-mux-2.0" for SoC which support FMan V3 (B/T)

> 

> 1.0/2.0 in the clockgen node refers to chassis version.  It has nothing to do

> with FMan version.

> 


I know. However there's a match: 1.0 chassis used in SoC with FMan v2 and 2.0 chassis in SoC with FMan v3.

> In fact, fman should not be in the compatible because, now that I found the

> documentation for this, I see that it's more generic than that.

> "fman-clk-mux" should be replaced with "qoriq-hwacsr".

> 

> > Using the 1st option might be confusing because core pll/mux 2.0

> represents B/T devices and 1.0 represent Pxxxx.

> > In this case, T1040 uses "fsl,qoriq-core-pll/mux-2.0" and "fsl,fman-clk-mux-

> 1.0".

> > On the other hand, the second option doesn't distinguishes between T4

> and T1 (for example), as T1 doesn't have reg property while T4 has.

> 

> How would t1040 have a so-called "fman-clk-mux" node at all if it doesn't

> have this register?

> 

> I think we've made a mess of the clock bindings and this is only going to make

> it worse.  We need to stay compatible with the mess we've made, but I'm

> inclined to say that we shouldn't add more nodes to it.

> 

> Instead, have the toplevel clockgen node have a chip-based compatible in

> addition to version (e.g. compatible = "fsl,t4240-clockgen",

> "fsl,qoriq-clockgen-2.0") and have the toplevel node export whatever clocks

> are needed.  Put the logic to deal with all the different dividers, register

> values, and such in code, not the device tree.  The binding should be focused

> on how to encode the specifier of the exported clocks.

> 


We have 2 cases:
	- Devices (T2/T4/B4) with CLKCG1HWACSR register.
	- Devices (Pxxxx, T1) without CLKCG1HWACSR register (Pxxxx devices have many options, T1 has only one option)

For the first group, we can have " qoriq-hwacsr" property in the clock node.
Currently T4 FMan clock mux node is the following:
	fm1clk: fm1-clk-mux {
		#clock-cells = <0>;
		compatible = "fsl,fman-clk-mux";
		clocks = <&pll1 1>, <&pll1 2>, <&pll1 3>,
			 <&platform_pll 0>, <&pll0 1>, <&pll0 2>;
		clock-names = "pll1-div2", "pll1-div3", "pll1-div4",
			      "platform-pll", "pll0-div2", "pll0-div3";
		clock-output-names = "fm1-clk";
	};
As far as I understand we need to move the node to the top level clock node.
In addition we need to add reg property and change the name of the node and the compatible.
In that case, the driver can read this register instead of parsing the RCW.

What about the devices of the second group?
In this case we don't have a register to determine the source clock. So we need access to guts registers, like we have currently.
The suggestion above doesn’t suit for those devices. 

> -Scott

> 


Igal
Scott Wood April 30, 2015, 12:30 a.m. UTC | #8 
On Wed, 2015-04-22 at 05:47 -0500, Liberman Igal-B31950 wrote:
> 
> 
> Regards,
> Igal Liberman.
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Tuesday, April 21, 2015 3:52 AM
> > To: Liberman Igal-B31950
> > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Tang
> > Yuantian-B29983
> > Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan clock mux
> > 
> > On Mon, 2015-04-20 at 06:40 -0500, Liberman Igal-B31950 wrote:
> > >
> > >
> > > Regards,
> > > Igal Liberman.
> > >
> > > > -----Original Message-----
> > > > From: Liberman Igal-B31950
> > > > Sent: Monday, April 20, 2015 2:07 PM
> > > > To: Wood Scott-B07421
> > > > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> > > > Subject: RE: [v3] dt/bindings: qoriq-clock: Add binding for FMan
> > > > clock mux
> > > >
> > > >
> > > >
> > > > Regards,
> > > > Igal Liberman.
> > > >
> > > > > -----Original Message-----
> > > > > From: Wood Scott-B07421
> > > > > Sent: Friday, April 17, 2015 8:41 AM
> > > > > To: Liberman Igal-B31950
> > > > > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> > > > > Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan
> > > > > clock mux
> > > > >
> > > > > On Thu, 2015-04-16 at 01:11 -0500, Liberman Igal-B31950 wrote:
> > > > > >
> > > > > >
> > > > > > Regards,
> > > > > > Igal Liberman.
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Wood Scott-B07421
> > > > > > > Sent: Wednesday, April 15, 2015 8:36 PM
> > > > > > > To: Liberman Igal-B31950
> > > > > > > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> > > > > > > Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for
> > > > > > > FMan clock mux
> > > > > > >
> > > > > > > On Tue, 2015-04-14 at 13:56 +0300, Igal.Liberman wrote:
> > > > > > > > From: Igal Liberman <Igal.Liberman@freescale.com>
> > > > > > > >
> > > > > > > > v3: Addressed feedback from Scott:
> > > > > > > > 	- Removed clock specifier description.
> > > > > > > >
> > > > > > > > v2: Addressed feedback from Scott:
> > > > > > > > 	- Moved the "fman-clk-mux" clock provider details
> > > > > > > > 	  under "clocks" property.
> > > > > > > >
> > > > > > > > Signed-off-by: Igal Liberman <Igal.Liberman@freescale.com>
> > > > > > > > ---
> > > > > > > >  .../devicetree/bindings/clock/qoriq-clock.txt      |   17
> > > > > +++++++++++++++--
> > > > > > > >  1 file changed, 15 insertions(+), 2 deletions(-)
> > > > > > > >
> > > > > > > > diff --git
> > > > > > > > a/Documentation/devicetree/bindings/clock/qoriq-clock.txt
> > > > > > > > b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
> > > > > > > > index b0d7b73..2bb3b38 100644
> > > > > > > > ---
> > > > > > > > a/Documentation/devicetree/bindings/clock/qoriq-clock.txt
> > > > > > > > +++ b/Documentation/devicetree/bindings/clock/qoriq-clock.tx
> > > > > > > > +++ t
> > > > > > > > @@ -65,9 +65,10 @@ Required properties:
> > > > > > > >  		It takes parent's clock-frequency as its clock.
> > > > > > > >  	* "fsl,qoriq-platform-pll-1.0" for the platform PLL clock (v1.0)
> > > > > > > >  	* "fsl,qoriq-platform-pll-2.0" for the platform PLL clock
> > > > > > > > (v2.0)
> > > > > > > > +	* "fsl,fman-clk-mux" for the Frame Manager clock.
> > > > > > > >  - #clock-cells: From common clock binding. The number of cells in
> > a
> > > > > > > > -	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0"
> > > > > > > > -	clocks, or <1> for "fsl,qoriq-core-pll-[1,2].0" clocks.
> > > > > > > > +	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0"
> > > > and
> > > > > > > > +	"fsl,fman-clk-mux" clocks or <1> for "fsl,qoriq-core-pll-
> > > > [1,2].0".
> > > > > > > >  	For "fsl,qoriq-core-pll-1.0" clocks, the single
> > > > > > > >  	clock-specifier cell may take the following values:
> > > > > > > >  	* 0 - equal to the PLL frequency @@ -145,6 +146,18 @@
> > > > Example
> > > > > > > > for clock block and clock provider:
> > > > > > > >  			clocks = <&sysclk>;
> > > > > > > >  			clock-output-names = "platform-pll",
> > > > "platform-pll-
> > > > > > > div2";
> > > > > > > >  		};
> > > > > > > > +
> > > > > > > > +		fm0clk: fm0-clk-mux {
> > > > > > > > +			#clock-cells = <0>;
> > > > > > > > +			reg = <0x10 4>
> > > > > > > > +			compatible = "fsl,fman-clk-mux";
> > > > > > > > +			clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>,
> > > > <&pll0 3>,
> > > > > > > > +				 <&platform_pll 0>, <&pll1 1>, <&pll1
> > > > 2>;
> > > > > > > > +			clock-names = "pll0", "pll0-div2", "pll0-div3",
> > > > > > > > +				      "pll0-div4", "platform-pll", "pll1-
> > > > div2",
> > > > > > > > +				      "pll1-div3";
> > > > > > > > +			clock-output-names = "fm0-clk";
> > > > > > > > +		};
> > > > > > > >  	};
> > > > > > > >  };
> > > > > > > >
> > > > > > >
> > > > > > > I don't see this register in the manuals for older DPAA chips,
> > > > > > > such as
> > > > > > > p4080 or p3041.  Is it present but undocumented?  Should I be
> > > > > > > looking somewhere other than "Clocking Memory Map"?
> > > > > > >
> > > > > >
> > > > > > It's available only in part of the new chips (T4, T2, B4).
> > > > > > In T1024/T1040 there's only one option for FMan clock so this
> > > > > > register is not
> > > > > available.
> > > > >
> > > > > So it's part of the 2.0 chassis?  I'd stick a 2.0 in there, then.
> > > > > Who knows what we may see in the future.
> > > > >
> > > >
> > > > OK,
> > > > We can go with "fsl,fman-clk-mux-1/2-0.".
> > > > In that case, we need to update FMan nodes and the clock driver:
> > > > https://patchwork.ozlabs.org/patch/443973/
> > > > https://patchwork.ozlabs.org/patch/461813/
> > > > I will update those patches separately.
> > > >
> > >
> > > Scott,
> > > There are 2 options:
> > > Use "fsl,fman-clk-mux-1.0" for SoC without CLKCGnHWACSR register.
> > > Use "fsl,fman-clk-mux-2.0" for SoC with CLKCGnHWACSR register.
> > > Or
> > > Use "fsl,fman-clk-mux-1.0" for SoC which support FMan V2 (Pxxxx) Use
> > > "fsl,fman-clk-mux-2.0" for SoC which support FMan V3 (B/T)
> > 
> > 1.0/2.0 in the clockgen node refers to chassis version.  It has nothing to do
> > with FMan version.
> > 
> 
> I know. However there's a match: 1.0 chassis used in SoC with FMan v2 and 2.0 chassis in SoC with FMan v3.
> 
> > In fact, fman should not be in the compatible because, now that I found the
> > documentation for this, I see that it's more generic than that.
> > "fman-clk-mux" should be replaced with "qoriq-hwacsr".
> > 
> > > Using the 1st option might be confusing because core pll/mux 2.0
> > represents B/T devices and 1.0 represent Pxxxx.
> > > In this case, T1040 uses "fsl,qoriq-core-pll/mux-2.0" and "fsl,fman-clk-mux-
> > 1.0".
> > > On the other hand, the second option doesn't distinguishes between T4
> > and T1 (for example), as T1 doesn't have reg property while T4 has.
> > 
> > How would t1040 have a so-called "fman-clk-mux" node at all if it doesn't
> > have this register?
> > 
> > I think we've made a mess of the clock bindings and this is only going to make
> > it worse.  We need to stay compatible with the mess we've made, but I'm
> > inclined to say that we shouldn't add more nodes to it.
> > 
> > Instead, have the toplevel clockgen node have a chip-based compatible in
> > addition to version (e.g. compatible = "fsl,t4240-clockgen",
> > "fsl,qoriq-clockgen-2.0") and have the toplevel node export whatever clocks
> > are needed.  Put the logic to deal with all the different dividers, register
> > values, and such in code, not the device tree.  The binding should be focused
> > on how to encode the specifier of the exported clocks.
> > 
> 
> We have 2 cases:
> 	- Devices (T2/T4/B4) with CLKCG1HWACSR register.
> 	- Devices (Pxxxx, T1) without CLKCG1HWACSR register (Pxxxx devices have many options, T1 has only one option)
> 
> For the first group, we can have " qoriq-hwacsr" property in the clock node.

No, we're not going to describe every register with its own property.
Describe the chip and let the driver be the place with the knowledge of
what each chip is like.

> Currently T4 FMan clock mux node is the following:
> 	fm1clk: fm1-clk-mux {
> 		#clock-cells = <0>;
> 		compatible = "fsl,fman-clk-mux";
> 		clocks = <&pll1 1>, <&pll1 2>, <&pll1 3>,
> 			 <&platform_pll 0>, <&pll0 1>, <&pll0 2>;
> 		clock-names = "pll1-div2", "pll1-div3", "pll1-div4",
> 			      "platform-pll", "pll0-div2", "pll0-div3";
> 		clock-output-names = "fm1-clk";
> 	};
> As far as I understand we need to move the node to the top level clock node.
> In addition we need to add reg property and change the name of the node and the compatible.
> In that case, the driver can read this register instead of parsing the RCW.

I'm having a hard time following.  What I'm suggesting is eliminating
the above and having the clockgen node itself (which already has a reg
property) be a clock source with a clock specifier encoding that
distinguishes the fman clock from other clocks.

> What about the devices of the second group?
> In this case we don't have a register to determine the source clock. So we need access to guts registers, 

I never suggested taking away access to the guts registers.

> like we have currently.
> The suggestion above doesn’t suit for those devices. 

How is the clock determined on those chips?

-Scott
Igal.Liberman April 30, 2015, 2:28 p.m. UTC | #9 
Regards,
Igal Liberman.

> -----Original Message-----

> From: Wood Scott-B07421

> Sent: Thursday, April 30, 2015 3:31 AM

> To: Liberman Igal-B31950

> Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Tang

> Yuantian-B29983

> Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan clock mux

> 

> On Wed, 2015-04-22 at 05:47 -0500, Liberman Igal-B31950 wrote:

> >

> >

> > Regards,

> > Igal Liberman.

> >

> > > -----Original Message-----

> > > From: Wood Scott-B07421

> > > Sent: Tuesday, April 21, 2015 3:52 AM

> > > To: Liberman Igal-B31950

> > > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Tang

> > > Yuantian-B29983

> > > Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan

> > > clock mux

> > >

> > > On Mon, 2015-04-20 at 06:40 -0500, Liberman Igal-B31950 wrote:

> > > >

> > > >

> > > > Regards,

> > > > Igal Liberman.

> > > >

> > > > > -----Original Message-----

> > > > > From: Liberman Igal-B31950

> > > > > Sent: Monday, April 20, 2015 2:07 PM

> > > > > To: Wood Scott-B07421

> > > > > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org

> > > > > Subject: RE: [v3] dt/bindings: qoriq-clock: Add binding for FMan

> > > > > clock mux

> > > > >

> > > > >

> > > > >

> > > > > Regards,

> > > > > Igal Liberman.

> > > > >

> > > > > > -----Original Message-----

> > > > > > From: Wood Scott-B07421

> > > > > > Sent: Friday, April 17, 2015 8:41 AM

> > > > > > To: Liberman Igal-B31950

> > > > > > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org

> > > > > > Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for

> > > > > > FMan clock mux

> > > > > >

> > > > > > On Thu, 2015-04-16 at 01:11 -0500, Liberman Igal-B31950 wrote:

> > > > > > >

> > > > > > >

> > > > > > > Regards,

> > > > > > > Igal Liberman.

> > > > > > >

> > > > > > > > -----Original Message-----

> > > > > > > > From: Wood Scott-B07421

> > > > > > > > Sent: Wednesday, April 15, 2015 8:36 PM

> > > > > > > > To: Liberman Igal-B31950

> > > > > > > > Cc: devicetree@vger.kernel.org;

> > > > > > > > linuxppc-dev@lists.ozlabs.org

> > > > > > > > Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding

> > > > > > > > for FMan clock mux

> > > > > > > >

> > > > > > > > On Tue, 2015-04-14 at 13:56 +0300, Igal.Liberman wrote:

> > > > > > > > > From: Igal Liberman <Igal.Liberman@freescale.com>

> > > > > > > > >

> > > > > > > > > v3: Addressed feedback from Scott:

> > > > > > > > > 	- Removed clock specifier description.

> > > > > > > > >

> > > > > > > > > v2: Addressed feedback from Scott:

> > > > > > > > > 	- Moved the "fman-clk-mux" clock provider details

> > > > > > > > > 	  under "clocks" property.

> > > > > > > > >

> > > > > > > > > Signed-off-by: Igal Liberman

> > > > > > > > > <Igal.Liberman@freescale.com>

> > > > > > > > > ---

> > > > > > > > >  .../devicetree/bindings/clock/qoriq-clock.txt      |   17

> > > > > > +++++++++++++++--

> > > > > > > > >  1 file changed, 15 insertions(+), 2 deletions(-)

> > > > > > > > >

> > > > > > > > > diff --git

> > > > > > > > > a/Documentation/devicetree/bindings/clock/qoriq-clock.tx

> > > > > > > > > t

> > > > > > > > > b/Documentation/devicetree/bindings/clock/qoriq-clock.tx

> > > > > > > > > t index b0d7b73..2bb3b38 100644

> > > > > > > > > ---

> > > > > > > > > a/Documentation/devicetree/bindings/clock/qoriq-clock.tx

> > > > > > > > > t

> > > > > > > > > +++ b/Documentation/devicetree/bindings/clock/qoriq-cloc

> > > > > > > > > +++ k.tx

> > > > > > > > > +++ t

> > > > > > > > > @@ -65,9 +65,10 @@ Required properties:

> > > > > > > > >  		It takes parent's clock-frequency as its clock.

> > > > > > > > >  	* "fsl,qoriq-platform-pll-1.0" for the platform PLL clock (v1.0)

> > > > > > > > >  	* "fsl,qoriq-platform-pll-2.0" for the platform PLL

> > > > > > > > > clock

> > > > > > > > > (v2.0)

> > > > > > > > > +	* "fsl,fman-clk-mux" for the Frame Manager clock.

> > > > > > > > >  - #clock-cells: From common clock binding. The number

> > > > > > > > > of cells in

> > > a

> > > > > > > > > -	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0"

> > > > > > > > > -	clocks, or <1> for "fsl,qoriq-core-pll-[1,2].0" clocks.

> > > > > > > > > +	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-

> [1,2].0"

> > > > > and

> > > > > > > > > +	"fsl,fman-clk-mux" clocks or <1> for

> > > > > > > > > +"fsl,qoriq-core-pll-

> > > > > [1,2].0".

> > > > > > > > >  	For "fsl,qoriq-core-pll-1.0" clocks, the single

> > > > > > > > >  	clock-specifier cell may take the following values:

> > > > > > > > >  	* 0 - equal to the PLL frequency @@ -145,6 +146,18 @@

> > > > > Example

> > > > > > > > > for clock block and clock provider:

> > > > > > > > >  			clocks = <&sysclk>;

> > > > > > > > >  			clock-output-names = "platform-pll",

> > > > > "platform-pll-

> > > > > > > > div2";

> > > > > > > > >  		};

> > > > > > > > > +

> > > > > > > > > +		fm0clk: fm0-clk-mux {

> > > > > > > > > +			#clock-cells = <0>;

> > > > > > > > > +			reg = <0x10 4>

> > > > > > > > > +			compatible = "fsl,fman-clk-mux";

> > > > > > > > > +			clocks = <&pll0 0>, <&pll0 1>, <&pll0

> 2>,

> > > > > <&pll0 3>,

> > > > > > > > > +				 <&platform_pll 0>, <&pll1 1>,

> <&pll1

> > > > > 2>;

> > > > > > > > > +			clock-names = "pll0", "pll0-div2",

> "pll0-div3",

> > > > > > > > > +				      "pll0-div4", "platform-pll",

> "pll1-

> > > > > div2",

> > > > > > > > > +				      "pll1-div3";

> > > > > > > > > +			clock-output-names = "fm0-clk";

> > > > > > > > > +		};

> > > > > > > > >  	};

> > > > > > > > >  };

> > > > > > > > >

> > > > > > > >

> > > > > > > > I don't see this register in the manuals for older DPAA

> > > > > > > > chips, such as

> > > > > > > > p4080 or p3041.  Is it present but undocumented?  Should I

> > > > > > > > be looking somewhere other than "Clocking Memory Map"?

> > > > > > > >

> > > > > > >

> > > > > > > It's available only in part of the new chips (T4, T2, B4).

> > > > > > > In T1024/T1040 there's only one option for FMan clock so

> > > > > > > this register is not

> > > > > > available.

> > > > > >

> > > > > > So it's part of the 2.0 chassis?  I'd stick a 2.0 in there, then.

> > > > > > Who knows what we may see in the future.

> > > > > >

> > > > >

> > > > > OK,

> > > > > We can go with "fsl,fman-clk-mux-1/2-0.".

> > > > > In that case, we need to update FMan nodes and the clock driver:

> > > > > https://patchwork.ozlabs.org/patch/443973/

> > > > > https://patchwork.ozlabs.org/patch/461813/

> > > > > I will update those patches separately.

> > > > >

> > > >

> > > > Scott,

> > > > There are 2 options:

> > > > Use "fsl,fman-clk-mux-1.0" for SoC without CLKCGnHWACSR register.

> > > > Use "fsl,fman-clk-mux-2.0" for SoC with CLKCGnHWACSR register.

> > > > Or

> > > > Use "fsl,fman-clk-mux-1.0" for SoC which support FMan V2 (Pxxxx)

> > > > Use "fsl,fman-clk-mux-2.0" for SoC which support FMan V3 (B/T)

> > >

> > > 1.0/2.0 in the clockgen node refers to chassis version.  It has

> > > nothing to do with FMan version.

> > >

> >

> > I know. However there's a match: 1.0 chassis used in SoC with FMan v2 and

> 2.0 chassis in SoC with FMan v3.

> >

> > > In fact, fman should not be in the compatible because, now that I

> > > found the documentation for this, I see that it's more generic than that.

> > > "fman-clk-mux" should be replaced with "qoriq-hwacsr".

> > >

> > > > Using the 1st option might be confusing because core pll/mux 2.0

> > > represents B/T devices and 1.0 represent Pxxxx.

> > > > In this case, T1040 uses "fsl,qoriq-core-pll/mux-2.0" and

> > > > "fsl,fman-clk-mux-

> > > 1.0".

> > > > On the other hand, the second option doesn't distinguishes between

> > > > T4

> > > and T1 (for example), as T1 doesn't have reg property while T4 has.

> > >

> > > How would t1040 have a so-called "fman-clk-mux" node at all if it

> > > doesn't have this register?

> > >

> > > I think we've made a mess of the clock bindings and this is only

> > > going to make it worse.  We need to stay compatible with the mess

> > > we've made, but I'm inclined to say that we shouldn't add more nodes to

> it.

> > >

> > > Instead, have the toplevel clockgen node have a chip-based

> > > compatible in addition to version (e.g. compatible =

> > > "fsl,t4240-clockgen",

> > > "fsl,qoriq-clockgen-2.0") and have the toplevel node export whatever

> > > clocks are needed.  Put the logic to deal with all the different

> > > dividers, register values, and such in code, not the device tree.

> > > The binding should be focused on how to encode the specifier of the

> exported clocks.

> > >

> >

> > We have 2 cases:

> > 	- Devices (T2/T4/B4) with CLKCG1HWACSR register.

> > 	- Devices (Pxxxx, T1) without CLKCG1HWACSR register (Pxxxx

> devices

> > have many options, T1 has only one option)

> >

> > For the first group, we can have " qoriq-hwacsr" property in the clock node.

> 

> No, we're not going to describe every register with its own property.

> Describe the chip and let the driver be the place with the knowledge of what

> each chip is like.

> 


I think that FMan clock mux (as we defined it) is similar to other muxes available in our SoC.
If we take T4 as example, it has 3 muxes defined in the device tree (mux0, mux1 and mux2).
Each mux has its own reg property (and clock providers).
	mux2: mux2@40 {
		#clock-cells = <0>;
		reg = <0x40 0x4>;
		compatible = "fsl,qoriq-core-mux-2.0";
		clocks = <&pll3 0>, <&pll3 1>, <&pll3 2>,
			<&pll4 0>, <&pll4 1>, <&pll4 2>;
		clock-names = "pll3", "pll3-div2", "pll3-div4",
			"pll4", "pll4-div2", "pll4-div4";
		clock-output-names = "cmux2";
	};

I agree that "fm1-clk-mux" need to be moved from the "guts" node to "clockgen" node.
However, I'm not sure which changes you want to perform in the node (beside adding reg property for SoC which has it for FMan clock).

> > Currently T4 FMan clock mux node is the following:

> > 	fm1clk: fm1-clk-mux {

> > 		#clock-cells = <0>;

> > 		compatible = "fsl,fman-clk-mux";

> > 		clocks = <&pll1 1>, <&pll1 2>, <&pll1 3>,

> > 			 <&platform_pll 0>, <&pll0 1>, <&pll0 2>;

> > 		clock-names = "pll1-div2", "pll1-div3", "pll1-div4",

> > 			      "platform-pll", "pll0-div2", "pll0-div3";

> > 		clock-output-names = "fm1-clk";

> > 	};

> > As far as I understand we need to move the node to the top level clock

> node.

> > In addition we need to add reg property and change the name of the node

> and the compatible.

> > In that case, the driver can read this register instead of parsing the RCW.

> 

> I'm having a hard time following.  What I'm suggesting is eliminating the

> above and having the clockgen node itself (which already has a reg

> property) be a clock source with a clock specifier encoding that distinguishes

> the fman clock from other clocks.

> 

> > What about the devices of the second group?

> > In this case we don't have a register to determine the source clock.

> > So we need access to guts registers,

> 

> I never suggested taking away access to the guts registers.

> 


I didn't mention that you want to remove them.
Just tried to explain that for Pxxxx devices, we need to use the current method (explained later).

> > like we have currently.

> > The suggestion above doesn’t suit for those devices.

> 

> How is the clock determined on those chips?

> 


In those chips (and currently others), currently, in the device tree we have all optional FMan clock providers (different between each SoC).
The clock driver reads the RCW in order to determine the clock provider and saves the index, later get_fm_clk_parent returns the index.

> -Scott

> 


Igal
Scott Wood May 1, 2015, 11:42 p.m. UTC | #10 
On Thu, 2015-04-30 at 09:28 -0500, Liberman Igal-B31950 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Thursday, April 30, 2015 3:31 AM
> > To: Liberman Igal-B31950
> > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Tang
> > Yuantian-B29983
> > Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan clock mux
> > 
> > On Wed, 2015-04-22 at 05:47 -0500, Liberman Igal-B31950 wrote:
> > > We have 2 cases:
> > > 	- Devices (T2/T4/B4) with CLKCG1HWACSR register.
> > > 	- Devices (Pxxxx, T1) without CLKCG1HWACSR register (Pxxxx
> > devices
> > > have many options, T1 has only one option)
> > >
> > > For the first group, we can have " qoriq-hwacsr" property in the clock node.
> > 
> > No, we're not going to describe every register with its own property.
> > Describe the chip and let the driver be the place with the knowledge of what
> > each chip is like.
> > 
> 
> I think that FMan clock mux (as we defined it) is similar to other muxes available in our SoC.

I realize that.  I'm saying the way we described existing muxes seems to
be a mistake.  We're putting too much complexity in the device tree.
Better to put the complexity in a place that isn't stable ABI.

> If we take T4 as example, it has 3 muxes defined in the device tree (mux0, mux1 and mux2).
> Each mux has its own reg property (and clock providers).
> 	mux2: mux2@40 {
> 		#clock-cells = <0>;
> 		reg = <0x40 0x4>;
> 		compatible = "fsl,qoriq-core-mux-2.0";
> 		clocks = <&pll3 0>, <&pll3 1>, <&pll3 2>,
> 			<&pll4 0>, <&pll4 1>, <&pll4 2>;
> 		clock-names = "pll3", "pll3-div2", "pll3-div4",
> 			"pll4", "pll4-div2", "pll4-div4";
> 		clock-output-names = "cmux2";
> 	};
> 
> I agree that "fm1-clk-mux" need to be moved from the "guts" node to "clockgen" node.

That's not what I was saying.  I'm saying get rid of the node entirely,
in favor of having the clockgen node itself be a clock source with
multiple post-mux outputs.

> However, I'm not sure which changes you want to perform in the node (beside adding reg property for SoC which has it for FMan clock).

Again, the clockgen node already has a reg property.

-Scott
Igal.Liberman May 5, 2015, 9:02 p.m. UTC | #11 
Regards,
Igal Liberman.

> -----Original Message-----

> From: Wood Scott-B07421

> Sent: Saturday, May 02, 2015 2:43 AM

> To: Liberman Igal-B31950

> Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Tang

> Yuantian-B29983

> Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan clock mux

> 

> On Thu, 2015-04-30 at 09:28 -0500, Liberman Igal-B31950 wrote:

> > > -----Original Message-----

> > > From: Wood Scott-B07421

> > > Sent: Thursday, April 30, 2015 3:31 AM

> > > To: Liberman Igal-B31950

> > > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Tang

> > > Yuantian-B29983

> > > Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan

> > > clock mux

> > >

> > > On Wed, 2015-04-22 at 05:47 -0500, Liberman Igal-B31950 wrote:

> > > > We have 2 cases:

> > > > 	- Devices (T2/T4/B4) with CLKCG1HWACSR register.

> > > > 	- Devices (Pxxxx, T1) without CLKCG1HWACSR register (Pxxxx

> > > devices

> > > > have many options, T1 has only one option)

> > > >

> > > > For the first group, we can have " qoriq-hwacsr" property in the clock

> node.

> > >

> > > No, we're not going to describe every register with its own property.

> > > Describe the chip and let the driver be the place with the knowledge

> > > of what each chip is like.

> > >

> >

> > I think that FMan clock mux (as we defined it) is similar to other muxes

> available in our SoC.

> 

> I realize that.  I'm saying the way we described existing muxes seems to be a

> mistake.  We're putting too much complexity in the device tree.

> Better to put the complexity in a place that isn't stable ABI.

> 

> > If we take T4 as example, it has 3 muxes defined in the device tree (mux0,

> mux1 and mux2).

> > Each mux has its own reg property (and clock providers).

> > 	mux2: mux2@40 {

> > 		#clock-cells = <0>;

> > 		reg = <0x40 0x4>;

> > 		compatible = "fsl,qoriq-core-mux-2.0";

> > 		clocks = <&pll3 0>, <&pll3 1>, <&pll3 2>,

> > 			<&pll4 0>, <&pll4 1>, <&pll4 2>;

> > 		clock-names = "pll3", "pll3-div2", "pll3-div4",

> > 			"pll4", "pll4-div2", "pll4-div4";

> > 		clock-output-names = "cmux2";

> > 	};

> >

> > I agree that "fm1-clk-mux" need to be moved from the "guts" node to

> "clockgen" node.

> 

> That's not what I was saying.  I'm saying get rid of the node entirely, in favor

> of having the clockgen node itself be a clock source with multiple post-mux

> outputs.

> 


Scott,
Currently the clockgen node has a number of sub nodes (platform clock, sysclock, PLLs and muxes, the number of muxes/PLLs is SoC dependent).
The clockgen node has reg property, however, all other nodes have reg property too (The clockgen points to the clocking block address and the sub nodes point to the specific register in the clock block).
Do you want to change this structure completely?
I'm not sure I understand the exact way you want see those nodes (in the final state).

> > However, I'm not sure which changes you want to perform in the node

> (beside adding reg property for SoC which has it for FMan clock).

> 

> Again, the clockgen node already has a reg property.

> 

> -Scott

>
Scott Wood May 5, 2015, 9:16 p.m. UTC | #12 
On Tue, 2015-05-05 at 16:02 -0500, Liberman Igal-B31950 wrote:
> 
> 
> Regards,
> Igal Liberman.
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Saturday, May 02, 2015 2:43 AM
> > To: Liberman Igal-B31950
> > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Tang
> > Yuantian-B29983
> > Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan clock mux
> > 
> > On Thu, 2015-04-30 at 09:28 -0500, Liberman Igal-B31950 wrote:
> > > > -----Original Message-----
> > > > From: Wood Scott-B07421
> > > > Sent: Thursday, April 30, 2015 3:31 AM
> > > > To: Liberman Igal-B31950
> > > > Cc: devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Tang
> > > > Yuantian-B29983
> > > > Subject: Re: [v3] dt/bindings: qoriq-clock: Add binding for FMan
> > > > clock mux
> > > >
> > > > On Wed, 2015-04-22 at 05:47 -0500, Liberman Igal-B31950 wrote:
> > > > > We have 2 cases:
> > > > > 	- Devices (T2/T4/B4) with CLKCG1HWACSR register.
> > > > > 	- Devices (Pxxxx, T1) without CLKCG1HWACSR register (Pxxxx
> > > > devices
> > > > > have many options, T1 has only one option)
> > > > >
> > > > > For the first group, we can have " qoriq-hwacsr" property in the clock
> > node.
> > > >
> > > > No, we're not going to describe every register with its own property.
> > > > Describe the chip and let the driver be the place with the knowledge
> > > > of what each chip is like.
> > > >
> > >
> > > I think that FMan clock mux (as we defined it) is similar to other muxes
> > available in our SoC.
> > 
> > I realize that.  I'm saying the way we described existing muxes seems to be a
> > mistake.  We're putting too much complexity in the device tree.
> > Better to put the complexity in a place that isn't stable ABI.
> > 
> > > If we take T4 as example, it has 3 muxes defined in the device tree (mux0,
> > mux1 and mux2).
> > > Each mux has its own reg property (and clock providers).
> > > 	mux2: mux2@40 {
> > > 		#clock-cells = <0>;
> > > 		reg = <0x40 0x4>;
> > > 		compatible = "fsl,qoriq-core-mux-2.0";
> > > 		clocks = <&pll3 0>, <&pll3 1>, <&pll3 2>,
> > > 			<&pll4 0>, <&pll4 1>, <&pll4 2>;
> > > 		clock-names = "pll3", "pll3-div2", "pll3-div4",
> > > 			"pll4", "pll4-div2", "pll4-div4";
> > > 		clock-output-names = "cmux2";
> > > 	};
> > >
> > > I agree that "fm1-clk-mux" need to be moved from the "guts" node to
> > "clockgen" node.
> > 
> > That's not what I was saying.  I'm saying get rid of the node entirely, in favor
> > of having the clockgen node itself be a clock source with multiple post-mux
> > outputs.
> > 
> 
> Scott,
> Currently the clockgen node has a number of sub nodes (platform clock, sysclock, PLLs and muxes, the number of muxes/PLLs is SoC dependent).
> The clockgen node has reg property, however, all other nodes have reg property too (The clockgen points to the clocking block address and the sub nodes point to the specific register in the clock block).
> Do you want to change this structure completely?

Yes, I want to change it completely (while ensuring the kernel still
works, albeit without new functionality, with older device trees).

> I'm not sure I understand the exact way you want see those nodes (in the final state).

Something like:

clockgen: global-utilities@e1000 {
	compatible = "fsl,<whatever>-clockgen";
	clock-frequency = <...>;
	reg = <0xe1000 0x1000>;
	#clock-cells = <2>;
};

cpu0: PowerPC,e6500@0 {
	device_type = "cpu";
	reg = <0 1>;
	clocks = <&clockgen 0 0>;
	next-level-cache = <&L2_1>;
	fsl,portid-mapping = <0x80000000>;
};


cpu4: PowerPC,e6500@0 {
	device_type = "cpu";
	reg = <4 5>;
	clocks = <&clockgen 0 1>;
	next-level-cache = <&L2_1>;
	fsl,portid-mapping = <0x80000000>;
};

fman@... {
	...
	clocks = <&clockgen 1 0>;
	...
};

...with the clockgen binding explaining the format of the clock
specifier, and the clockgen driver containing the chip-specific muxing
details.

-Scott
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/clock/qoriq-clock.txt b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
index b0d7b73..2bb3b38 100644
--- a/Documentation/devicetree/bindings/clock/qoriq-clock.txt
+++ b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
@@ -65,9 +65,10 @@  Required properties:
 		It takes parent's clock-frequency as its clock.
 	* "fsl,qoriq-platform-pll-1.0" for the platform PLL clock (v1.0)
 	* "fsl,qoriq-platform-pll-2.0" for the platform PLL clock (v2.0)
+	* "fsl,fman-clk-mux" for the Frame Manager clock.
 - #clock-cells: From common clock binding. The number of cells in a
-	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0"
-	clocks, or <1> for "fsl,qoriq-core-pll-[1,2].0" clocks.
+	clock-specifier. Should be <0> for "fsl,qoriq-sysclk-[1,2].0" and
+	"fsl,fman-clk-mux" clocks or <1> for "fsl,qoriq-core-pll-[1,2].0".
 	For "fsl,qoriq-core-pll-1.0" clocks, the single
 	clock-specifier cell may take the following values:
 	* 0 - equal to the PLL frequency
@@ -145,6 +146,18 @@  Example for clock block and clock provider:
 			clocks = <&sysclk>;
 			clock-output-names = "platform-pll", "platform-pll-div2";
 		};
+
+		fm0clk: fm0-clk-mux {
+			#clock-cells = <0>;
+			reg = <0x10 4>
+			compatible = "fsl,fman-clk-mux";
+			clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>, <&pll0 3>,
+				 <&platform_pll 0>, <&pll1 1>, <&pll1 2>;
+			clock-names = "pll0", "pll0-div2", "pll0-div3",
+				      "pll0-div4", "platform-pll", "pll1-div2",
+				      "pll1-div3";
+			clock-output-names = "fm0-clk";
+		};
 	};
 };