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@xxxxxxxxxxxxxxx; linuxppc-dev@xxxxxxxxxxxxxxxx
> > 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@xxxxxxxxxxxxxxx; linuxppc-dev@xxxxxxxxxxxxxxxx
> > > 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@xxxxxxxxxxxxxxx; linuxppc-dev@xxxxxxxxxxxxxxxx
> > > > > 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@xxxxxxxxxxxxx>
> > > > > >
> > > > > > 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@xxxxxxxxxxxxx>
> > > > > > ---
> > > > > > .../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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
