| Message ID | 1381300704-4238-1-git-send-email-Yuantian.Tang@freescale.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Scott Wood |
| Headers | show |
On Wed, Oct 09, 2013 at 07:38:24AM +0100, Yuantian.Tang@freescale.com wrote: > From: Tang Yuantian <yuantian.tang@freescale.com> > > The following SoCs will be affected: p2041, p3041, p4080, > p5020, p5040, b4420, b4860, t4240 > > Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com> > Signed-off-by: Li Yang <leoli@freescale.com> > --- > v5: > - refine the binding document > - update the compatible string > v4: > - add binding document > - update compatible string > - update the reg property > v3: > - fix typo > v2: > - add t4240, b4420, b4860 support > - remove pll/4 clock from p2041, p3041 and p5020 board > > .../devicetree/bindings/clock/corenet-clock.txt | 111 ++++++++++++++++++++ > arch/powerpc/boot/dts/fsl/b4420si-post.dtsi | 35 +++++++ > arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi | 2 + > arch/powerpc/boot/dts/fsl/b4860si-post.dtsi | 35 +++++++ > arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi | 4 + > arch/powerpc/boot/dts/fsl/p2041si-post.dtsi | 60 +++++++++++ > arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi | 4 + > arch/powerpc/boot/dts/fsl/p3041si-post.dtsi | 60 +++++++++++ > arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi | 4 + > arch/powerpc/boot/dts/fsl/p4080si-post.dtsi | 112 +++++++++++++++++++++ > arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi | 8 ++ > arch/powerpc/boot/dts/fsl/p5020si-post.dtsi | 42 ++++++++ > arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi | 2 + > arch/powerpc/boot/dts/fsl/p5040si-post.dtsi | 60 +++++++++++ > arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi | 4 + > arch/powerpc/boot/dts/fsl/t4240si-post.dtsi | 85 ++++++++++++++++ > arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi | 12 +++ > 17 files changed, 640 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/corenet-clock.txt > > diff --git a/Documentation/devicetree/bindings/clock/corenet-clock.txt b/Documentation/devicetree/bindings/clock/corenet-clock.txt > new file mode 100644 > index 0000000..8efc62d > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/corenet-clock.txt > @@ -0,0 +1,111 @@ > +* Clock Block on Freescale CoreNet Platforms > + > +Freescale CoreNet chips take primary clocking input from the external > +SYSCLK signal. The SYSCLK input (frequency) is multiplied using > +multiple phase locked loops (PLL) to create a variety of frequencies > +which can then be passed to a variety of internal logic, including > +cores and peripheral IP blocks. > +Please refer to the Reference Manual for details. > + > +1. Clock Block Binding > + > +Required properties: > +- compatible: Should include one or more of the following: > + - "fsl,<chip>-clockgen": for chip specific clock block > + - "fsl,qoriq-clockgen-[1,2].x": for chassis 1.x and 2.x clock While I can see that "fsl,<chip>-clockgen" might have a large set of strings that we may never deal with in th kernel, I'd prefer that the basic strings (i.e. all the "fsl,qoriq-clockgen-[1,2].x" variants) were listed explicitly here. Given they only seem to be "fsl,qoriq-clockgen-1.0" and "fsl,qoriq-clockgen-2.0" this shouldn't be too difficult to list and describe. > +- reg: Offset and length of the clock register set > +- clock-frequency: Indicates input clock frequency of clock block. > + Will be set by u-boot Why does the fact this is set by u-boot matter to the binding? > + > +Recommended properties: > +- #ddress-cells: Specifies the number of cells used to represent > + physical base addresses. Must be present if the device has > + sub-nodes and set to 1 if present Typo: #address-cells In the example it looks like the address cells of child nodes are offsets within the unit, rather than absolute physical addresses. Could the code not treat them as absolute addresses? Then we'd only need to document that #address-cells, #size-cells and ranges must be present and have values suitable for mapping child nodes into the address space of the parent. > +- #size-cells: Specifies the number of cells used to represent > + the size of an address. Must be present if the device has > + sub-nodes and set to 1 if present It's not really the size of an address, it's the size of a region identified by a reg entry. I think this can be simplified by my suggestion above. > + > +2. Clock Provider/Consumer Binding > + > +Most of the binding are from the common clock binding[1]. > + [1] Documentation/devicetree/bindings/clock/clock-bindings.txt > + > +Required properties: > +- compatible : Should include one or more of the following: > + - "fsl,qoriq-core-pll-[1,2].x": Indicates a core PLL clock device > + - "fsl,qoriq-core-mux-[1,2].x": Indicates a core multiplexer clock > + device; divided from the core PLL clock As above, I'd prefer a complete list of the basic strings we expect. > + - "fixed-clock": From common clock binding; indicates output clock > + of oscillator > + - "fsl,qoriq-sysclk-[1,2].x": Indicates input system clock Here too. > +- #clock-cells: From common clock binding; indicates the number of > + output clock. 0 is for one output clock; 1 for more than one clock If a clock source has multiple outputs, what those outputs are and what values in clock-cells they correspond to should be described here. > + > +Recommended properties: > +- clocks: Should be the phandle of input parent clock > +- clock-names: From common clock binding, indicates the clock name That description's a bit opaque. What's the name of the clock input on these units? That's what clock-names should contain, and that should be documented. Do we not _always_ need the parent clock? If we have a clock do we need a clock-names entry for it? > +- clock-output-names: From common clock binding, indicates the names of > + output clocks > +- reg: Should be the offset and length of clock block base address. > + The length should be 4. > + > +Example for clock block and clock provider: > +/ { > + clockgen: global-utilities@e1000 { > + compatible = "fsl,p5020-clockgen", "fsl,qoriq-clockgen-1.0"; > + reg = <0xe1000 0x1000>; > + clock-frequency = <0>; That looks odd. > + #address-cells = <1>; > + #size-cells = <1>; > + > + sysclk: sysclk { > + #clock-cells = <0>; > + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; We didn't mention in the binding that "fsl,qoriq-sysclk-1.0" was compatible with "fixed-clock" and should have "fixed-clock" in the compatible list... > + clock-output-names = "sysclk"; > + } > + > + pll0: pll0@800 { > + #clock-cells = <1>; > + reg = <0x800 0x4>; > + compatible = "fsl,qoriq-core-pll-1.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll0", "pll0-div2"; > + }; > + > + pll1: pll1@820 { > + #clock-cells = <1>; > + reg = <0x820 0x4>; > + compatible = "fsl,qoriq-core-pll-1.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll1", "pll1-div2"; > + }; > + > + mux0: mux0@0 { > + #clock-cells = <0>; > + reg = <0x0 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux0"; > + }; > + > + mux1: mux1@20 { > + #clock-cells = <0>; > + reg = <0x20 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux1"; How does the mux choose which input clock to use at a point in time? Cheers, Mark.
Thanks for your review. See my reply inline > -----Original Message----- > From: Mark Rutland [mailto:mark.rutland@arm.com] > Sent: 2013年10月10日 星期四 18:04 > To: Tang Yuantian-B29983 > Cc: galak@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org; > devicetree@vger.kernel.org; Li Yang-Leo-R58472 > Subject: Re: [PATCH v5] powerpc/mpc85xx: Update the clock nodes in device > tree > > On Wed, Oct 09, 2013 at 07:38:24AM +0100, Yuantian.Tang@freescale.com > wrote: > > From: Tang Yuantian <yuantian.tang@freescale.com> > > > > The following SoCs will be affected: p2041, p3041, p4080, p5020, > > p5040, b4420, b4860, t4240 > > > > Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com> > > Signed-off-by: Li Yang <leoli@freescale.com> > > --- > > v5: > > - refine the binding document > > - update the compatible string > > v4: > > - add binding document > > - update compatible string > > - update the reg property > > v3: > > - fix typo > > v2: > > - add t4240, b4420, b4860 support > > - remove pll/4 clock from p2041, p3041 and p5020 board > > > > .../devicetree/bindings/clock/corenet-clock.txt | 111 > ++++++++++++++++++++ > > arch/powerpc/boot/dts/fsl/b4420si-post.dtsi | 35 +++++++ > > arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi | 2 + > > arch/powerpc/boot/dts/fsl/b4860si-post.dtsi | 35 +++++++ > > arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi | 4 + > > arch/powerpc/boot/dts/fsl/p2041si-post.dtsi | 60 +++++++++++ > > arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi | 4 + > > arch/powerpc/boot/dts/fsl/p3041si-post.dtsi | 60 +++++++++++ > > arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi | 4 + > > arch/powerpc/boot/dts/fsl/p4080si-post.dtsi | 112 > +++++++++++++++++++++ > > arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi | 8 ++ > > arch/powerpc/boot/dts/fsl/p5020si-post.dtsi | 42 ++++++++ > > arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi | 2 + > > arch/powerpc/boot/dts/fsl/p5040si-post.dtsi | 60 +++++++++++ > > arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi | 4 + > > arch/powerpc/boot/dts/fsl/t4240si-post.dtsi | 85 > ++++++++++++++++ > > arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi | 12 +++ > > 17 files changed, 640 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/clock/corenet-clock.txt > > > > diff --git a/Documentation/devicetree/bindings/clock/corenet-clock.txt > > b/Documentation/devicetree/bindings/clock/corenet-clock.txt > > new file mode 100644 > > index 0000000..8efc62d > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/clock/corenet-clock.txt > > @@ -0,0 +1,111 @@ > > +* Clock Block on Freescale CoreNet Platforms > > + > > +Freescale CoreNet chips take primary clocking input from the external > > +SYSCLK signal. The SYSCLK input (frequency) is multiplied using > > +multiple phase locked loops (PLL) to create a variety of frequencies > > +which can then be passed to a variety of internal logic, including > > +cores and peripheral IP blocks. > > +Please refer to the Reference Manual for details. > > + > > +1. Clock Block Binding > > + > > +Required properties: > > +- compatible: Should include one or more of the following: > > + - "fsl,<chip>-clockgen": for chip specific clock block > > + - "fsl,qoriq-clockgen-[1,2].x": for chassis 1.x and 2.x clock > > While I can see that "fsl,<chip>-clockgen" might have a large set of > strings that we may never deal with in th kernel, I'd prefer that the > basic strings (i.e. all the "fsl,qoriq-clockgen-[1,2].x" variants) were > listed explicitly here. > > Given they only seem to be "fsl,qoriq-clockgen-1.0" and "fsl,qoriq- > clockgen-2.0" this shouldn't be too difficult to list and describe. > OK, I will list them all. > > +- reg: Offset and length of the clock register set > > +- clock-frequency: Indicates input clock frequency of clock block. > > + Will be set by u-boot > > Why does the fact this is set by u-boot matter to the binding? > OK, I will remove it. > > + > > +Recommended properties: > > +- #ddress-cells: Specifies the number of cells used to represent > > + physical base addresses. Must be present if the device has > > + sub-nodes and set to 1 if present > > Typo: #address-cells > > In the example it looks like the address cells of child nodes are offsets > within the unit, rather than absolute physical addresses. Could the code > not treat them as absolute addresses? Then we'd only need to document > that #address-cells, #size-cells and ranges must be present and have > values suitable for mapping child nodes into the address space of the > parent. > OK, thanks. > > +- #size-cells: Specifies the number of cells used to represent > > + the size of an address. Must be present if the device has > > + sub-nodes and set to 1 if present > > It's not really the size of an address, it's the size of a region > identified by a reg entry. > > I think this can be simplified by my suggestion above. > Yes > > + > > +2. Clock Provider/Consumer Binding > > + > > +Most of the binding are from the common clock binding[1]. > > + [1] Documentation/devicetree/bindings/clock/clock-bindings.txt > > + > > +Required properties: > > +- compatible : Should include one or more of the following: > > + - "fsl,qoriq-core-pll-[1,2].x": Indicates a core PLL clock > device > > + - "fsl,qoriq-core-mux-[1,2].x": Indicates a core multiplexer > clock > > + device; divided from the core PLL clock > > As above, I'd prefer a complete list of the basic strings we expect. > There is no list here, just *-mux-1.x and *-mux-2.x What kind of list do you prefer? > > + - "fixed-clock": From common clock binding; indicates output > clock > > + of oscillator > > + - "fsl,qoriq-sysclk-[1,2].x": Indicates input system clock > > Here too. > > > +- #clock-cells: From common clock binding; indicates the number of > > + output clock. 0 is for one output clock; 1 for more than one > > +clock > > If a clock source has multiple outputs, what those outputs are and what > values in clock-cells they correspond to should be described here. > There is no way to describe the values of multiple outputs here. This property is the type of BOOL. See the clock-bindings.txt. > > + > > +Recommended properties: > > +- clocks: Should be the phandle of input parent clock > > +- clock-names: From common clock binding, indicates the clock name > > That description's a bit opaque. > > What's the name of the clock input on these units? That's what clock- > names should contain, and that should be documented. > Is it necessary to document these names since they are totally used by clock provider and clock consumer has no idea about them? > Do we not _always_ need the parent clock? > Not for fixed-clock that its parent clock is oscillator :) > If we have a clock do we need a clock-names entry for it? > Technically yes, but I don't bother to add it if the clock node has only one clocks. > > +- clock-output-names: From common clock binding, indicates the names > of > > + output clocks > > +- reg: Should be the offset and length of clock block base address. > > + The length should be 4. > > + > > +Example for clock block and clock provider: > > +/ { > > + clockgen: global-utilities@e1000 { > > + compatible = "fsl,p5020-clockgen", "fsl,qoriq-clockgen- > 1.0"; > > + reg = <0xe1000 0x1000>; > > + clock-frequency = <0>; > > That looks odd. > Yes, but it has already existed here before this patch. Can I move it to sysclk clock node since it is not used yet? > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + sysclk: sysclk { > > + #clock-cells = <0>; > > + compatible = "fsl,qoriq-sysclk-1.0", > > + "fixed-clock"; > > We didn't mention in the binding that "fsl,qoriq-sysclk-1.0" was > compatible with "fixed-clock" and should have "fixed-clock" in the > compatible list... > OK, will fix it. > > + clock-output-names = "sysclk"; > > + } > > + > > + pll0: pll0@800 { > > + #clock-cells = <1>; > > + reg = <0x800 0x4>; > > + compatible = "fsl,qoriq-core-pll-1.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll0", "pll0-div2"; > > + }; > > + > > + pll1: pll1@820 { > > + #clock-cells = <1>; > > + reg = <0x820 0x4>; > > + compatible = "fsl,qoriq-core-pll-1.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll1", "pll1-div2"; > > + }; > > + > > + mux0: mux0@0 { > > + #clock-cells = <0>; > > + reg = <0x0 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, > <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", > "pll1_1"; > > + clock-output-names = "cmux0"; > > + }; > > + > > + mux1: mux1@20 { > > + #clock-cells = <0>; > > + reg = <0x20 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, > <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", > "pll1_1"; > > + clock-output-names = "cmux1"; > > How does the mux choose which input clock to use at a point in time? > That is decided at runtime. Different input clock will lead to the different Clock frequency that the CPUs work on. Thanks, Yuantian > Cheers, > Mark.
On Fri, Oct 11, 2013 at 04:18:18AM +0100, Tang Yuantian-B29983 wrote: > Thanks for your review. > See my reply inline > > > -----Original Message----- > > From: Mark Rutland [mailto:mark.rutland@arm.com] > > Sent: 2013年10月10日 星期四 18:04 > > To: Tang Yuantian-B29983 > > Cc: galak@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org; > > devicetree@vger.kernel.org; Li Yang-Leo-R58472 > > Subject: Re: [PATCH v5] powerpc/mpc85xx: Update the clock nodes in device > > tree > > > > On Wed, Oct 09, 2013 at 07:38:24AM +0100, Yuantian.Tang@freescale.com > > wrote: > > > From: Tang Yuantian <yuantian.tang@freescale.com> > > > > > > The following SoCs will be affected: p2041, p3041, p4080, p5020, > > > p5040, b4420, b4860, t4240 > > > > > > Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com> > > > Signed-off-by: Li Yang <leoli@freescale.com> > > > --- > > > v5: > > > - refine the binding document > > > - update the compatible string > > > v4: > > > - add binding document > > > - update compatible string > > > - update the reg property > > > v3: > > > - fix typo > > > v2: > > > - add t4240, b4420, b4860 support > > > - remove pll/4 clock from p2041, p3041 and p5020 board > > > > > > .../devicetree/bindings/clock/corenet-clock.txt | 111 > > ++++++++++++++++++++ > > > arch/powerpc/boot/dts/fsl/b4420si-post.dtsi | 35 +++++++ > > > arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi | 2 + > > > arch/powerpc/boot/dts/fsl/b4860si-post.dtsi | 35 +++++++ > > > arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi | 4 + > > > arch/powerpc/boot/dts/fsl/p2041si-post.dtsi | 60 +++++++++++ > > > arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi | 4 + > > > arch/powerpc/boot/dts/fsl/p3041si-post.dtsi | 60 +++++++++++ > > > arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi | 4 + > > > arch/powerpc/boot/dts/fsl/p4080si-post.dtsi | 112 > > +++++++++++++++++++++ > > > arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi | 8 ++ > > > arch/powerpc/boot/dts/fsl/p5020si-post.dtsi | 42 ++++++++ > > > arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi | 2 + > > > arch/powerpc/boot/dts/fsl/p5040si-post.dtsi | 60 +++++++++++ > > > arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi | 4 + > > > arch/powerpc/boot/dts/fsl/t4240si-post.dtsi | 85 > > ++++++++++++++++ > > > arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi | 12 +++ > > > 17 files changed, 640 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/clock/corenet-clock.txt > > > > > > diff --git a/Documentation/devicetree/bindings/clock/corenet-clock.txt > > > b/Documentation/devicetree/bindings/clock/corenet-clock.txt > > > new file mode 100644 > > > index 0000000..8efc62d > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/clock/corenet-clock.txt > > > @@ -0,0 +1,111 @@ > > > +* Clock Block on Freescale CoreNet Platforms > > > + > > > +Freescale CoreNet chips take primary clocking input from the external > > > +SYSCLK signal. The SYSCLK input (frequency) is multiplied using > > > +multiple phase locked loops (PLL) to create a variety of frequencies > > > +which can then be passed to a variety of internal logic, including > > > +cores and peripheral IP blocks. > > > +Please refer to the Reference Manual for details. > > > + > > > +1. Clock Block Binding > > > + > > > +Required properties: > > > +- compatible: Should include one or more of the following: > > > + - "fsl,<chip>-clockgen": for chip specific clock block > > > + - "fsl,qoriq-clockgen-[1,2].x": for chassis 1.x and 2.x clock > > > > While I can see that "fsl,<chip>-clockgen" might have a large set of > > strings that we may never deal with in th kernel, I'd prefer that the > > basic strings (i.e. all the "fsl,qoriq-clockgen-[1,2].x" variants) were > > listed explicitly here. > > > > Given they only seem to be "fsl,qoriq-clockgen-1.0" and "fsl,qoriq- > > clockgen-2.0" this shouldn't be too difficult to list and describe. > > > OK, I will list them all. Thanks. > > > > +- reg: Offset and length of the clock register set > > > +- clock-frequency: Indicates input clock frequency of clock block. > > > + Will be set by u-boot > > > > Why does the fact this is set by u-boot matter to the binding? > > > OK, I will remove it. > > > > + > > > +Recommended properties: > > > +- #ddress-cells: Specifies the number of cells used to represent > > > + physical base addresses. Must be present if the device has > > > + sub-nodes and set to 1 if present > > > > Typo: #address-cells > > > > In the example it looks like the address cells of child nodes are offsets > > within the unit, rather than absolute physical addresses. Could the code > > not treat them as absolute addresses? Then we'd only need to document > > that #address-cells, #size-cells and ranges must be present and have > > values suitable for mapping child nodes into the address space of the > > parent. > > > OK, thanks. > > > > +- #size-cells: Specifies the number of cells used to represent > > > + the size of an address. Must be present if the device has > > > + sub-nodes and set to 1 if present > > > > It's not really the size of an address, it's the size of a region > > identified by a reg entry. > > > > I think this can be simplified by my suggestion above. > > > Yes Aah, I see that this is already in use, so it's a bit late to change this... > > > > + > > > +2. Clock Provider/Consumer Binding > > > + > > > +Most of the binding are from the common clock binding[1]. > > > + [1] Documentation/devicetree/bindings/clock/clock-bindings.txt > > > + > > > +Required properties: > > > +- compatible : Should include one or more of the following: I didn't spot this earlier, but why "one or more"? are the 2.0 variants backwards compatible with the 1.0 variants. > > > + - "fsl,qoriq-core-pll-[1,2].x": Indicates a core PLL clock > > device > > > + - "fsl,qoriq-core-mux-[1,2].x": Indicates a core multiplexer > > clock > > > + device; divided from the core PLL clock > > > > As above, I'd prefer a complete list of the basic strings we expect. > > > There is no list here, just *-mux-1.x and *-mux-2.x > What kind of list do you prefer? Something like: - compatible: Should include at least one of: * "fsl,qoriq-core-pll-1.0" for core PLL clocks (v1.0) * "fsl,qoriq-core-pll-2.0" for core PLL clocks (v2.0) * "fsl,qoriq-core-mux-1.0" for core mux clocks (v1.0) * "fsl,qoriq-core-mux-2.0" for core mux clocks (v2.0) The *-2.0 variants are backwards compatible with the *-1.0 variants, so for compatiblity a *-1.0 variant string should be in the list. > > > > + - "fixed-clock": From common clock binding; indicates output > > clock > > > + of oscillator > > > + - "fsl,qoriq-sysclk-[1,2].x": Indicates input system clock > > > > Here too. > > > > > +- #clock-cells: From common clock binding; indicates the number of > > > + output clock. 0 is for one output clock; 1 for more than one > > > +clock > > > > If a clock source has multiple outputs, what those outputs are and what > > values in clock-cells they correspond to should be described here. > > > There is no way to describe the values of multiple outputs here. > This property is the type of BOOL. See the clock-bindings.txt. Sorry? #clock-cells is most definitely _not_ a bool: 17: #clock-cells: Number of cells in a clock specifier; Typically 0 for nodes 18: with a single clock output and 1 for nodes with multiple 19: clock outputs. And neither are the clock-specifiers encoded with those cells. Consider: pll0: pll0@800 { #clock-cells = <1>; reg = <0x800 0x4>; compatible = "fsl,qoriq-core-pll-1.0"; clocks = <&sysclk>; clock-output-names = "pll0", "pll0-div2"; }; Here the value of the cells in a clock-specifier seem to be: 0: pll0 1: pll0-div2 So in a consumer, the valid values of the cells in a clock-specifier are: consumer: device { compatible = "vendor,some-device"; clocks = <&pll0 0>, /* pll0 */ <&pll0 1>; /* pll0-div2 */ }; There must be some meaning assigned to the values of the cells in the clock-specifier (e.g. linear index of the clock output in the hardware). It would be good to describe this, other clock bindings do. > > > > + > > > +Recommended properties: > > > +- clocks: Should be the phandle of input parent clock > > > +- clock-names: From common clock binding, indicates the clock name > > > > That description's a bit opaque. > > > > What's the name of the clock input on these units? That's what clock- > > names should contain, and that should be documented. > > > Is it necessary to document these names since they are totally used > by clock provider and clock consumer has no idea about them? I'm not sure I follow -- clocks and clock-names are used by consumers. They define which clocks are inputs to a consumer, and the names of the clock inputs on the consumer: consumer@0xffff0000 { reg = <0xffff0000 0x1000>; compatible = "vendor,some-consumer"; clocks = <&pl011 0>, <&otherclock 43 22>, <&pl011 1>; clock-names = "apb_pclk", "pixel_clk", "scanout_clk"; }; Here the set of clock-names would be defined in the binding of the consumer, based on the clock input names in the IP documentation -- they tell the consumer which clock inputs on the consumer the clocks described in clocks are wired in to, and describe nothing about output of the provider. Using clock-names allows the set of clocks on an IP to change over revisions and for some clock inputs to be optional. > > > Do we not _always_ need the parent clock? > > > Not for fixed-clock that its parent clock is oscillator :) Certainly fixed-clock doesn't need any parent clock, but I guess PLLs and muxes always do. > > > If we have a clock do we need a clock-names entry for it? > > > Technically yes, but I don't bother to add it if the clock node has > only one clocks. Ok. Where we only have one input clock, this doesn't need to be described. Do the mux clocks always take 3 input clocks (judging by the examples they do)? > > > > +- clock-output-names: From common clock binding, indicates the names > > of > > > + output clocks > > > +- reg: Should be the offset and length of clock block base address. > > > + The length should be 4. > > > + > > > +Example for clock block and clock provider: > > > +/ { > > > + clockgen: global-utilities@e1000 { > > > + compatible = "fsl,p5020-clockgen", "fsl,qoriq-clockgen- > > 1.0"; > > > + reg = <0xe1000 0x1000>; > > > + clock-frequency = <0>; > > > > That looks odd. > > > Yes, but it has already existed here before this patch. > Can I move it to sysclk clock node since it is not used yet? I hadn't realised there were DTS with this already. Why is there a clock with clock-frequency = <0> at all? Surely that isn't useful... > > > > + #address-cells = <1>; > > > + #size-cells = <1>; > > > + > > > + sysclk: sysclk { > > > + #clock-cells = <0>; > > > + compatible = "fsl,qoriq-sysclk-1.0", > > > + "fixed-clock"; > > > > We didn't mention in the binding that "fsl,qoriq-sysclk-1.0" was > > compatible with "fixed-clock" and should have "fixed-clock" in the > > compatible list... > > > OK, will fix it. Cheers. Also, doesn't a fixed-clock require a clock-frequency? > > > > + clock-output-names = "sysclk"; > > > + } > > > + > > > + pll0: pll0@800 { > > > + #clock-cells = <1>; > > > + reg = <0x800 0x4>; > > > + compatible = "fsl,qoriq-core-pll-1.0"; > > > + clocks = <&sysclk>; > > > + clock-output-names = "pll0", "pll0-div2"; > > > + }; > > > + > > > + pll1: pll1@820 { > > > + #clock-cells = <1>; > > > + reg = <0x820 0x4>; > > > + compatible = "fsl,qoriq-core-pll-1.0"; > > > + clocks = <&sysclk>; > > > + clock-output-names = "pll1", "pll1-div2"; > > > + }; > > > + > > > + mux0: mux0@0 { > > > + #clock-cells = <0>; > > > + reg = <0x0 0x4>; > > > + compatible = "fsl,qoriq-core-mux-1.0"; > > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, > > <&pll1 1>; > > > + clock-names = "pll0_0", "pll0_1", "pll1_0", > > "pll1_1"; > > > + clock-output-names = "cmux0"; > > > + }; > > > + > > > + mux1: mux1@20 { > > > + #clock-cells = <0>; > > > + reg = <0x20 0x4>; > > > + compatible = "fsl,qoriq-core-mux-1.0"; > > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, > > <&pll1 1>; > > > + clock-names = "pll0_0", "pll0_1", "pll1_0", > > "pll1_1"; I didn't spot this last time, but the clock-names here seem to be the names of the outputs from the provider, rather than the input names of the consumer. This goes against the intended purpose of clock-names. > > > + clock-output-names = "cmux1"; > > > > How does the mux choose which input clock to use at a point in time? > > > That is decided at runtime. Different input clock will lead to the different > Clock frequency that the CPUs work on. Ok. Cheers, Mark.
On Fri, 2013-10-11 at 10:25 +0100, Mark Rutland wrote: > On Fri, Oct 11, 2013 at 04:18:18AM +0100, Tang Yuantian-B29983 wrote: > > Thanks for your review. > > See my reply inline > > > > > -----Original Message----- > > > From: Mark Rutland [mailto:mark.rutland@arm.com] > > > Sent: 2013年10月10日 星期四 18:04 > > > To: Tang Yuantian-B29983 > > > Cc: galak@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org; > > > devicetree@vger.kernel.org; Li Yang-Leo-R58472 > > > Subject: Re: [PATCH v5] powerpc/mpc85xx: Update the clock nodes in device > > > tree > > > > > > On Wed, Oct 09, 2013 at 07:38:24AM +0100, Yuantian.Tang@freescale.com > > > wrote: > > > > From: Tang Yuantian <yuantian.tang@freescale.com> > > > > > > > > The following SoCs will be affected: p2041, p3041, p4080, p5020, > > > > p5040, b4420, b4860, t4240 > > > > > > > > Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com> > > > > Signed-off-by: Li Yang <leoli@freescale.com> > > > > --- > > > > v5: > > > > - refine the binding document > > > > - update the compatible string > > > > v4: > > > > - add binding document > > > > - update compatible string > > > > - update the reg property > > > > v3: > > > > - fix typo > > > > v2: > > > > - add t4240, b4420, b4860 support > > > > - remove pll/4 clock from p2041, p3041 and p5020 board > > > > > > > > .../devicetree/bindings/clock/corenet-clock.txt | 111 > > > ++++++++++++++++++++ > > > > arch/powerpc/boot/dts/fsl/b4420si-post.dtsi | 35 +++++++ > > > > arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi | 2 + > > > > arch/powerpc/boot/dts/fsl/b4860si-post.dtsi | 35 +++++++ > > > > arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi | 4 + > > > > arch/powerpc/boot/dts/fsl/p2041si-post.dtsi | 60 +++++++++++ > > > > arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi | 4 + > > > > arch/powerpc/boot/dts/fsl/p3041si-post.dtsi | 60 +++++++++++ > > > > arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi | 4 + > > > > arch/powerpc/boot/dts/fsl/p4080si-post.dtsi | 112 > > > +++++++++++++++++++++ > > > > arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi | 8 ++ > > > > arch/powerpc/boot/dts/fsl/p5020si-post.dtsi | 42 ++++++++ > > > > arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi | 2 + > > > > arch/powerpc/boot/dts/fsl/p5040si-post.dtsi | 60 +++++++++++ > > > > arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi | 4 + > > > > arch/powerpc/boot/dts/fsl/t4240si-post.dtsi | 85 > > > ++++++++++++++++ > > > > arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi | 12 +++ > > > > 17 files changed, 640 insertions(+) > > > > create mode 100644 > > > > Documentation/devicetree/bindings/clock/corenet-clock.txt > > > > > > > > diff --git a/Documentation/devicetree/bindings/clock/corenet-clock.txt > > > > b/Documentation/devicetree/bindings/clock/corenet-clock.txt > > > > new file mode 100644 > > > > index 0000000..8efc62d > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/clock/corenet-clock.txt > > > > @@ -0,0 +1,111 @@ > > > > +* Clock Block on Freescale CoreNet Platforms > > > > + > > > > +Freescale CoreNet chips take primary clocking input from the external > > > > +SYSCLK signal. The SYSCLK input (frequency) is multiplied using > > > > +multiple phase locked loops (PLL) to create a variety of frequencies > > > > +which can then be passed to a variety of internal logic, including > > > > +cores and peripheral IP blocks. > > > > +Please refer to the Reference Manual for details. > > > > + > > > > +1. Clock Block Binding > > > > + > > > > +Required properties: > > > > +- compatible: Should include one or more of the following: > > > > + - "fsl,<chip>-clockgen": for chip specific clock block > > > > + - "fsl,qoriq-clockgen-[1,2].x": for chassis 1.x and 2.x clock > > > > > > While I can see that "fsl,<chip>-clockgen" might have a large set of > > > strings that we may never deal with in th kernel, I'd prefer that the > > > basic strings (i.e. all the "fsl,qoriq-clockgen-[1,2].x" variants) were > > > listed explicitly here. > > > > > > Given they only seem to be "fsl,qoriq-clockgen-1.0" and "fsl,qoriq- > > > clockgen-2.0" this shouldn't be too difficult to list and describe. > > > > > OK, I will list them all. > > Thanks. > > > > > > > +- reg: Offset and length of the clock register set > > > > +- clock-frequency: Indicates input clock frequency of clock block. > > > > + Will be set by u-boot > > > > > > Why does the fact this is set by u-boot matter to the binding? > > > > > OK, I will remove it. > > > > > > + > > > > +Recommended properties: > > > > +- #ddress-cells: Specifies the number of cells used to represent > > > > + physical base addresses. Must be present if the device has > > > > + sub-nodes and set to 1 if present > > > > > > Typo: #address-cells > > > > > > In the example it looks like the address cells of child nodes are offsets > > > within the unit, rather than absolute physical addresses. Could the code > > > not treat them as absolute addresses? Then we'd only need to document > > > that #address-cells, #size-cells and ranges must be present and have > > > values suitable for mapping child nodes into the address space of the > > > parent. > > > > > OK, thanks. > > > > > > +- #size-cells: Specifies the number of cells used to represent > > > > + the size of an address. Must be present if the device has > > > > + sub-nodes and set to 1 if present > > > > > > It's not really the size of an address, it's the size of a region > > > identified by a reg entry. > > > > > > I think this can be simplified by my suggestion above. > > > > > Yes > > Aah, I see that this is already in use, so it's a bit late to change > this... > > > > > > > + > > > > +2. Clock Provider/Consumer Binding > > > > + > > > > +Most of the binding are from the common clock binding[1]. > > > > + [1] Documentation/devicetree/bindings/clock/clock-bindings.txt > > > > + > > > > +Required properties: > > > > +- compatible : Should include one or more of the following: > > I didn't spot this earlier, but why "one or more"? are the 2.0 variants > backwards compatible with the 1.0 variants. > > > > > + - "fsl,qoriq-core-pll-[1,2].x": Indicates a core PLL clock > > > device > > > > + - "fsl,qoriq-core-mux-[1,2].x": Indicates a core multiplexer > > > clock > > > > + device; divided from the core PLL clock > > > > > > As above, I'd prefer a complete list of the basic strings we expect. > > > > > There is no list here, just *-mux-1.x and *-mux-2.x > > What kind of list do you prefer? > > Something like: > > - compatible: Should include at least one of: > * "fsl,qoriq-core-pll-1.0" for core PLL clocks (v1.0) > * "fsl,qoriq-core-pll-2.0" for core PLL clocks (v2.0) > * "fsl,qoriq-core-mux-1.0" for core mux clocks (v1.0) > * "fsl,qoriq-core-mux-2.0" for core mux clocks (v2.0) > The *-2.0 variants are backwards compatible with the *-1.0 variants, > so for compatiblity a *-1.0 variant string should be in the list. I'm not sure that they're 100% compatible. 1.0 seems to have a "KILL" bit in the PLL register that 2.0 doesn't have (unless it's a documentation glitch). > > > > +- clock-output-names: From common clock binding, indicates the names > > > of > > > > + output clocks > > > > +- reg: Should be the offset and length of clock block base address. > > > > + The length should be 4. > > > > + > > > > +Example for clock block and clock provider: > > > > +/ { > > > > + clockgen: global-utilities@e1000 { > > > > + compatible = "fsl,p5020-clockgen", "fsl,qoriq-clockgen- > > > 1.0"; > > > > + reg = <0xe1000 0x1000>; > > > > + clock-frequency = <0>; > > > > > > That looks odd. > > > > > Yes, but it has already existed here before this patch. > > Can I move it to sysclk clock node since it is not used yet? > > I hadn't realised there were DTS with this already. Why is there a clock > with clock-frequency = <0> at all? Surely that isn't useful... The actual frequency is patched in by U-Boot -- and moving it to a different node would break this. > > > > + #address-cells = <1>; > > > > + #size-cells = <1>; > > > > + > > > > + sysclk: sysclk { > > > > + #clock-cells = <0>; > > > > + compatible = "fsl,qoriq-sysclk-1.0", > > > > + "fixed-clock"; > > > > > > We didn't mention in the binding that "fsl,qoriq-sysclk-1.0" was > > > compatible with "fixed-clock" and should have "fixed-clock" in the > > > compatible list... > > > > > OK, will fix it. > > Cheers. Also, doesn't a fixed-clock require a clock-frequency? Why isn't the global-utilities node, that has the clock-frequency, acting as the fixed-clock? I thought that's what was in earlier revisions... If it absolutely must be a separate node for some reason, I suppose you could remove the "fixed-clock" and have a tiny "driver" that looks up the frequency in the parent node. This would be an instance of a non-"fixed-clock" that doesn't have a parent clock described in the device tree, because the information comes from some other mechanism. > > > > + mux1: mux1@20 { > > > > + #clock-cells = <0>; > > > > + reg = <0x20 0x4>; > > > > + compatible = "fsl,qoriq-core-mux-1.0"; > > > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, > > > <&pll1 1>; > > > > + clock-names = "pll0_0", "pll0_1", "pll1_0", > > > "pll1_1"; > > I didn't spot this last time, but the clock-names here seem to be the > names of the outputs from the provider, rather than the input names of > the consumer. This goes against the intended purpose of clock-names. As far as "pll0", "pll1", etc. goes, that appears to be the input name. It's all on one chip, so the virtual pins are documented based on what they're connected to. I'm not sure I understand the "_0"/"_1" part, though. Doesn't each PLL just have one output, which the consumer may choose to divide by 2 (or in some cases 4)? Why does that division need to be exposed in the device tree as separate connections to the parent clock? -Scott
On Wed, 2013-10-09 at 14:38 +0800, Yuantian.Tang@freescale.com wrote: > From: Tang Yuantian <yuantian.tang@frovider: > +/ { > + clockgen: global-utilities@e1000 { > + compatible = "fsl,p5020-clockgen", "fsl,qoriq-clockgen-1.0"; > + reg = <0xe1000 0x1000>; > + clock-frequency = <0>; > + #address-cells = <1>; > + #size-cells = <1>; > + > + sysclk: sysclk { > + #clock-cells = <0>; > + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; > + clock-output-names = "sysclk"; > + } > + > + pll0: pll0@800 { > + #clock-cells = <1>; > + reg = <0x800 0x4>; > + compatible = "fsl,qoriq-core-pll-1.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll0", "pll0-div2"; > + }; Where is "ranges" in the global-utilities node? -Scott
Thanks for your review. > -----Original Message----- > From: Wood Scott-B07421 > Sent: 2013年10月12日 星期六 3:07 > To: Mark Rutland > Cc: Tang Yuantian-B29983; devicetree@vger.kernel.org; linuxppc- > dev@lists.ozlabs.org; Li Yang-Leo-R58472 > Subject: Re: [PATCH v5] powerpc/mpc85xx: Update the clock nodes in device > tree > > On Fri, 2013-10-11 at 10:25 +0100, Mark Rutland wrote: > > On Fri, Oct 11, 2013 at 04:18:18AM +0100, Tang Yuantian-B29983 wrote: > > > Thanks for your review. > > > See my reply inline > > > > > > > -----Original Message----- > > > > From: Mark Rutland [mailto:mark.rutland@arm.com] > > > > Sent: 2013年10月10日 星期四 18:04 > > > > To: Tang Yuantian-B29983 > > > > Cc: galak@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org; > > > > devicetree@vger.kernel.org; Li Yang-Leo-R58472 > > > > Subject: Re: [PATCH v5] powerpc/mpc85xx: Update the clock nodes in > > > > device tree > > > > > > > > On Wed, Oct 09, 2013 at 07:38:24AM +0100, > > > > Yuantian.Tang@freescale.com > > > > wrote: > > > > > From: Tang Yuantian <yuantian.tang@freescale.com> > > > > > > > > > > The following SoCs will be affected: p2041, p3041, p4080, p5020, > > > > > p5040, b4420, b4860, t4240 > > > > > > > > > > Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com> > > > > > Signed-off-by: Li Yang <leoli@freescale.com> > > > > > --- > > > > > v5: > > > > > - refine the binding document > > > > > - update the compatible string > > > > > v4: > > > > > - add binding document > > > > > - update compatible string > > > > > - update the reg property > > > > > v3: > > > > > - fix typo > > > > > v2: > > > > > - add t4240, b4420, b4860 support > > > > > - remove pll/4 clock from p2041, p3041 and p5020 board > > > > > > > > > > .../devicetree/bindings/clock/corenet-clock.txt | 111 > > > > ++++++++++++++++++++ > > > > > arch/powerpc/boot/dts/fsl/b4420si-post.dtsi | 35 +++++++ > > > > > arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi | 2 + > > > > > arch/powerpc/boot/dts/fsl/b4860si-post.dtsi | 35 +++++++ > > > > > arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi | 4 + > > > > > arch/powerpc/boot/dts/fsl/p2041si-post.dtsi | 60 > +++++++++++ > > > > > arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi | 4 + > > > > > arch/powerpc/boot/dts/fsl/p3041si-post.dtsi | 60 > +++++++++++ > > > > > arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi | 4 + > > > > > arch/powerpc/boot/dts/fsl/p4080si-post.dtsi | 112 > > > > +++++++++++++++++++++ > > > > > arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi | 8 ++ > > > > > arch/powerpc/boot/dts/fsl/p5020si-post.dtsi | 42 > ++++++++ > > > > > arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi | 2 + > > > > > arch/powerpc/boot/dts/fsl/p5040si-post.dtsi | 60 > +++++++++++ > > > > > arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi | 4 + > > > > > arch/powerpc/boot/dts/fsl/t4240si-post.dtsi | 85 > > > > ++++++++++++++++ > > > > > arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi | 12 +++ > > > > > 17 files changed, 640 insertions(+) create mode 100644 > > > > > Documentation/devicetree/bindings/clock/corenet-clock.txt > > > > > > > > > > diff --git > > > > > a/Documentation/devicetree/bindings/clock/corenet-clock.txt > > > > > b/Documentation/devicetree/bindings/clock/corenet-clock.txt > > > > > new file mode 100644 > > > > > index 0000000..8efc62d > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/clock/corenet-clock.txt > > > > > @@ -0,0 +1,111 @@ > > > > > +* Clock Block on Freescale CoreNet Platforms > > > > > + > > > > > +Freescale CoreNet chips take primary clocking input from the > > > > > +external SYSCLK signal. The SYSCLK input (frequency) is > > > > > +multiplied using multiple phase locked loops (PLL) to create a > > > > > +variety of frequencies which can then be passed to a variety of > > > > > +internal logic, including cores and peripheral IP blocks. > > > > > +Please refer to the Reference Manual for details. > > > > > + > > > > > +1. Clock Block Binding > > > > > + > > > > > +Required properties: > > > > > +- compatible: Should include one or more of the following: > > > > > + - "fsl,<chip>-clockgen": for chip specific clock block > > > > > + - "fsl,qoriq-clockgen-[1,2].x": for chassis 1.x and 2.x > > > > > +clock > > > > > > > > While I can see that "fsl,<chip>-clockgen" might have a large set > > > > of strings that we may never deal with in th kernel, I'd prefer > > > > that the basic strings (i.e. all the "fsl,qoriq-clockgen-[1,2].x" > > > > variants) were listed explicitly here. > > > > > > > > Given they only seem to be "fsl,qoriq-clockgen-1.0" and > > > > "fsl,qoriq- clockgen-2.0" this shouldn't be too difficult to list > and describe. > > > > > > > OK, I will list them all. > > > > Thanks. > > > > > > > > > > +- reg: Offset and length of the clock register set > > > > > +- clock-frequency: Indicates input clock frequency of clock > block. > > > > > + Will be set by u-boot > > > > > > > > Why does the fact this is set by u-boot matter to the binding? > > > > > > > OK, I will remove it. > > > > > > > > + > > > > > +Recommended properties: > > > > > +- #ddress-cells: Specifies the number of cells used to represent > > > > > + physical base addresses. Must be present if the device > has > > > > > + sub-nodes and set to 1 if present > > > > > > > > Typo: #address-cells > > > > > > > > In the example it looks like the address cells of child nodes are > > > > offsets within the unit, rather than absolute physical addresses. > > > > Could the code not treat them as absolute addresses? Then we'd > > > > only need to document that #address-cells, #size-cells and ranges > > > > must be present and have values suitable for mapping child nodes > > > > into the address space of the parent. > > > > > > > OK, thanks. > > > > > > > > +- #size-cells: Specifies the number of cells used to represent > > > > > + the size of an address. Must be present if the device has > > > > > + sub-nodes and set to 1 if present > > > > > > > > It's not really the size of an address, it's the size of a region > > > > identified by a reg entry. > > > > > > > > I think this can be simplified by my suggestion above. > > > > > > > Yes > > > > Aah, I see that this is already in use, so it's a bit late to change > > this... > > > > > > > > > > + > > > > > +2. Clock Provider/Consumer Binding > > > > > + > > > > > +Most of the binding are from the common clock binding[1]. > > > > > + [1] Documentation/devicetree/bindings/clock/clock-bindings.txt > > > > > + > > > > > +Required properties: > > > > > +- compatible : Should include one or more of the following: > > > > I didn't spot this earlier, but why "one or more"? are the 2.0 > > variants backwards compatible with the 1.0 variants. > > > > > > > + - "fsl,qoriq-core-pll-[1,2].x": Indicates a core PLL > > > > > + clock > > > > device > > > > > + - "fsl,qoriq-core-mux-[1,2].x": Indicates a core > > > > > + multiplexer > > > > clock > > > > > + device; divided from the core PLL clock > > > > > > > > As above, I'd prefer a complete list of the basic strings we expect. > > > > > > > There is no list here, just *-mux-1.x and *-mux-2.x What kind of > > > list do you prefer? > > > > Something like: > > > > - compatible: Should include at least one of: > > * "fsl,qoriq-core-pll-1.0" for core PLL clocks (v1.0) > > * "fsl,qoriq-core-pll-2.0" for core PLL clocks (v2.0) > > * "fsl,qoriq-core-mux-1.0" for core mux clocks (v1.0) > > * "fsl,qoriq-core-mux-2.0" for core mux clocks (v2.0) > > The *-2.0 variants are backwards compatible with the *-1.0 variants, > > so for compatiblity a *-1.0 variant string should be in the list. > > I'm not sure that they're 100% compatible. 1.0 seems to have a "KILL" > bit in the PLL register that 2.0 doesn't have (unless it's a > documentation glitch). > > > > > > +- clock-output-names: From common clock binding, indicates the > > > > > +names > > > > of > > > > > + output clocks > > > > > +- reg: Should be the offset and length of clock block base > address. > > > > > + The length should be 4. > > > > > + > > > > > +Example for clock block and clock provider: > > > > > +/ { > > > > > + clockgen: global-utilities@e1000 { > > > > > + compatible = "fsl,p5020-clockgen", > > > > > +"fsl,qoriq-clockgen- > > > > 1.0"; > > > > > + reg = <0xe1000 0x1000>; > > > > > + clock-frequency = <0>; > > > > > > > > That looks odd. > > > > > > > Yes, but it has already existed here before this patch. > > > Can I move it to sysclk clock node since it is not used yet? > > > > I hadn't realised there were DTS with this already. Why is there a > > clock with clock-frequency = <0> at all? Surely that isn't useful... > > The actual frequency is patched in by U-Boot -- and moving it to a > different node would break this. > > > > > > + #address-cells = <1>; > > > > > + #size-cells = <1>; > > > > > + > > > > > + sysclk: sysclk { > > > > > + #clock-cells = <0>; > > > > > + compatible = "fsl,qoriq-sysclk-1.0", > > > > > + "fixed-clock"; > > > > > > > > We didn't mention in the binding that "fsl,qoriq-sysclk-1.0" was > > > > compatible with "fixed-clock" and should have "fixed-clock" in the > > > > compatible list... > > > > > > > OK, will fix it. > > > > Cheers. Also, doesn't a fixed-clock require a clock-frequency? > > Why isn't the global-utilities node, that has the clock-frequency, acting > as the fixed-clock? I thought that's what was in earlier revisions... > As you pointed out before, we can't combine the global-utilities node and the fixed-clock node into one node, because these two nodes have different properties which are not compatible each other. > If it absolutely must be a separate node for some reason, I suppose you > could remove the "fixed-clock" and have a tiny "driver" that looks up the > frequency in the parent node. This would be an instance of a non-"fixed- > clock" that doesn't have a parent clock described in the device tree, > because the information comes from some other mechanism. > This tiny driver is still needed to get the clock frequency from parent node. That is what I am going to change with current driver. But we still need to add the fixed-clock node in order to provide clock source to PLL nodes. > > > > > + mux1: mux1@20 { > > > > > + #clock-cells = <0>; > > > > > + reg = <0x20 0x4>; > > > > > + compatible = "fsl,qoriq-core-mux-1.0"; > > > > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 > > > > > + 0>, > > > > <&pll1 1>; > > > > > + clock-names = "pll0_0", "pll0_1", > > > > > + "pll1_0", > > > > "pll1_1"; > > > > I didn't spot this last time, but the clock-names here seem to be the > > names of the outputs from the provider, rather than the input names of > > the consumer. This goes against the intended purpose of clock-names. > > As far as "pll0", "pll1", etc. goes, that appears to be the input name. > It's all on one chip, so the virtual pins are documented based on what > they're connected to. > > I'm not sure I understand the "_0"/"_1" part, though. Doesn't each PLL > just have one output, which the consumer may choose to divide by 2 (or in > some cases 4)? Why does that division need to be exposed in the device > tree as separate connections to the parent clock? > The device tree makes that quite clear. Each PLL has several output which MUX node can take from. It is not a runtime decision. Regards, Yuantian > -Scott >
Thanks for your review. > -----Original Message----- > From: Wood Scott-B07421 > Sent: 2013年10月12日 星期六 3:08 > To: Tang Yuantian-B29983 > Cc: galak@kernel.crashing.org; devicetree@vger.kernel.org; linuxppc- > dev@lists.ozlabs.org > Subject: Re: [PATCH v5] powerpc/mpc85xx: Update the clock nodes in device > tree > > On Wed, 2013-10-09 at 14:38 +0800, Yuantian.Tang@freescale.com wrote: > > From: Tang Yuantian <yuantian.tang@frovider: > > +/ { > > + clockgen: global-utilities@e1000 { > > + compatible = "fsl,p5020-clockgen", "fsl,qoriq-clockgen-1.0"; > > + reg = <0xe1000 0x1000>; > > + clock-frequency = <0>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + sysclk: sysclk { > > + #clock-cells = <0>; > > + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; > > + clock-output-names = "sysclk"; > > + } > > + > > + pll0: pll0@800 { > > + #clock-cells = <1>; > > + reg = <0x800 0x4>; > > + compatible = "fsl,qoriq-core-pll-1.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll0", "pll0-div2"; > > + }; > > Where is "ranges" in the global-utilities node? > Will add it thanks. Regards, Yuantian > -Scott >
Thanks for your review. > > > > > > > +- reg: Offset and length of the clock register set > > > > +- clock-frequency: Indicates input clock frequency of clock block. > > > > + Will be set by u-boot > > > > > > Why does the fact this is set by u-boot matter to the binding? > > > > > OK, I will remove it. > > > > > > + > > > > +Recommended properties: > > > > +- #ddress-cells: Specifies the number of cells used to represent > > > > + physical base addresses. Must be present if the device has > > > > + sub-nodes and set to 1 if present > > > > > > Typo: #address-cells > > > > > > In the example it looks like the address cells of child nodes are > > > offsets within the unit, rather than absolute physical addresses. > > > Could the code not treat them as absolute addresses? Then we'd only > > > need to document that #address-cells, #size-cells and ranges must be > > > present and have values suitable for mapping child nodes into the > > > address space of the parent. > > > > > OK, thanks. > > > > > > +- #size-cells: Specifies the number of cells used to represent > > > > + the size of an address. Must be present if the device has > > > > + sub-nodes and set to 1 if present > > > > > > It's not really the size of an address, it's the size of a region > > > identified by a reg entry. > > > > > > I think this can be simplified by my suggestion above. > > > > > Yes > > Aah, I see that this is already in use, so it's a bit late to change > this... > I will modify the driver anyway once the binding gets done. > > > > > > + > > > > +2. Clock Provider/Consumer Binding > > > > + > > > > +Most of the binding are from the common clock binding[1]. > > > > + [1] Documentation/devicetree/bindings/clock/clock-bindings.txt > > > > + > > > > +Required properties: > > > > +- compatible : Should include one or more of the following: > > I didn't spot this earlier, but why "one or more"? are the 2.0 variants > backwards compatible with the 1.0 variants. > One or more for fixed-clock which is compatible with qoriq-sysclk-*. > > > > + - "fsl,qoriq-core-pll-[1,2].x": Indicates a core PLL clock > > > device > > > > + - "fsl,qoriq-core-mux-[1,2].x": Indicates a core > > > > + multiplexer > > > clock > > > > + device; divided from the core PLL clock > > > > > > As above, I'd prefer a complete list of the basic strings we expect. > > > > > There is no list here, just *-mux-1.x and *-mux-2.x What kind of list > > do you prefer? > > Something like: > > - compatible: Should include at least one of: > * "fsl,qoriq-core-pll-1.0" for core PLL clocks (v1.0) > * "fsl,qoriq-core-pll-2.0" for core PLL clocks (v2.0) > * "fsl,qoriq-core-mux-1.0" for core mux clocks (v1.0) > * "fsl,qoriq-core-mux-2.0" for core mux clocks (v2.0) > The *-2.0 variants are backwards compatible with the *-1.0 variants, > so for compatiblity a *-1.0 variant string should be in the list. > See the comment by Scott wood. > > > > > > + - "fixed-clock": From common clock binding; indicates > > > > + output > > > clock > > > > + of oscillator > > > > + - "fsl,qoriq-sysclk-[1,2].x": Indicates input system clock > > > > > > Here too. > > > > > > > +- #clock-cells: From common clock binding; indicates the number of > > > > + output clock. 0 is for one output clock; 1 for more than > > > > +one clock > > > > > > If a clock source has multiple outputs, what those outputs are and > > > what values in clock-cells they correspond to should be described > here. > > > > > There is no way to describe the values of multiple outputs here. > > This property is the type of BOOL. See the clock-bindings.txt. > > Sorry? #clock-cells is most definitely _not_ a bool: > ACTs like bool. > 17: #clock-cells: Number of cells in a clock specifier; Typically 0 > for nodes > 18: with a single clock output and 1 for nodes with > multiple > 19: clock outputs. > > And neither are the clock-specifiers encoded with those cells. Consider: > > pll0: pll0@800 { > #clock-cells = <1>; > reg = <0x800 0x4>; > compatible = "fsl,qoriq-core-pll-1.0"; > clocks = <&sysclk>; > clock-output-names = "pll0", "pll0-div2"; > }; > > Here the value of the cells in a clock-specifier seem to be: > 0: pll0 > 1: pll0-div2 > > So in a consumer, the valid values of the cells in a clock-specifier > are: > > consumer: device { > compatible = "vendor,some-device"; > clocks = <&pll0 0>, /* pll0 */ > <&pll0 1>; /* pll0-div2 */ > }; > > There must be some meaning assigned to the values of the cells in the > clock-specifier (e.g. linear index of the clock output in the hardware). > It would be good to describe this, other clock bindings do. > Sorry, I still get what you mean. There is no INDEX at all. 0 is for one output, 1 for multiple output. Just like that. What the " other clock bindings" did you refer to? > > > > > > + > > > > +Recommended properties: > > > > +- clocks: Should be the phandle of input parent clock > > > > +- clock-names: From common clock binding, indicates the clock > > > > +name > > > > > > That description's a bit opaque. > > > > > > What's the name of the clock input on these units? That's what > > > clock- names should contain, and that should be documented. > > > > > Is it necessary to document these names since they are totally used by > > clock provider and clock consumer has no idea about them? > > I'm not sure I follow -- clocks and clock-names are used by consumers. > They define which clocks are inputs to a consumer, and the names of the > clock inputs on the consumer: > > consumer@0xffff0000 { > reg = <0xffff0000 0x1000>; > compatible = "vendor,some-consumer"; > clocks = <&pl011 0>, > <&otherclock 43 22>, > <&pl011 1>; > clock-names = "apb_pclk", > "pixel_clk", > "scanout_clk"; > }; > > Here the set of clock-names would be defined in the binding of the > consumer, based on the clock input names in the IP documentation -- they > tell the consumer which clock inputs on the consumer the clocks described > in clocks are wired in to, and describe nothing about output of the > provider. Using clock-names allows the set of clocks on an IP to change > over revisions and for some clock inputs to be optional. > OK, I get it. Will name the clock-names better, and add it if missed. Thanks, > > > > > Do we not _always_ need the parent clock? > > > > > Not for fixed-clock that its parent clock is oscillator :) > > Certainly fixed-clock doesn't need any parent clock, but I guess PLLs and > muxes always do. > > > > > > If we have a clock do we need a clock-names entry for it? > > > > > Technically yes, but I don't bother to add it if the clock node has > > only one clocks. > > Ok. Where we only have one input clock, this doesn't need to be described. > > Do the mux clocks always take 3 input clocks (judging by the examples > they do)? > > > > > > > +- clock-output-names: From common clock binding, indicates the > > > > +names > > > of > > > > + output clocks > > > > +- reg: Should be the offset and length of clock block base address. > > > > + The length should be 4. > > > > + > > > > +Example for clock block and clock provider: > > > > +/ { > > > > + clockgen: global-utilities@e1000 { > > > > + compatible = "fsl,p5020-clockgen", > > > > +"fsl,qoriq-clockgen- > > > 1.0"; > > > > + reg = <0xe1000 0x1000>; > > > > + clock-frequency = <0>; > > > > > > That looks odd. > > > > > Yes, but it has already existed here before this patch. > > Can I move it to sysclk clock node since it is not used yet? > > I hadn't realised there were DTS with this already. Why is there a clock > with clock-frequency = <0> at all? Surely that isn't useful... > See comments by Scott Wood, Thanks you Scoot. > > > > > > + #address-cells = <1>; > > > > + #size-cells = <1>; > > > > + > > > > + sysclk: sysclk { > > > > + #clock-cells = <0>; > > > > + compatible = "fsl,qoriq-sysclk-1.0", > > > > + "fixed-clock"; > > > > > > We didn't mention in the binding that "fsl,qoriq-sysclk-1.0" was > > > compatible with "fixed-clock" and should have "fixed-clock" in the > > > compatible list... > > > > > OK, will fix it. > > Cheers. Also, doesn't a fixed-clock require a clock-frequency? > Yes, it should be. But we got the clock-frequency from parent node because There is a clock-frequency already there before this patch. > > > > > > + clock-output-names = "sysclk"; > > > > + } > > > > + > > > > + pll0: pll0@800 { > > > > + #clock-cells = <1>; > > > > + reg = <0x800 0x4>; > > > > + compatible = "fsl,qoriq-core-pll-1.0"; > > > > + clocks = <&sysclk>; > > > > + clock-output-names = "pll0", "pll0-div2"; > > > > + }; > > > > + > > > > + pll1: pll1@820 { > > > > + #clock-cells = <1>; > > > > + reg = <0x820 0x4>; > > > > + compatible = "fsl,qoriq-core-pll-1.0"; > > > > + clocks = <&sysclk>; > > > > + clock-output-names = "pll1", "pll1-div2"; > > > > + }; > > > > + > > > > + mux0: mux0@0 { > > > > + #clock-cells = <0>; > > > > + reg = <0x0 0x4>; > > > > + compatible = "fsl,qoriq-core-mux-1.0"; > > > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, > > > <&pll1 1>; > > > > + clock-names = "pll0_0", "pll0_1", > > > > + "pll1_0", > > > "pll1_1"; > > > > + clock-output-names = "cmux0"; > > > > + }; > > > > + > > > > + mux1: mux1@20 { > > > > + #clock-cells = <0>; > > > > + reg = <0x20 0x4>; > > > > + compatible = "fsl,qoriq-core-mux-1.0"; > > > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, > > > <&pll1 1>; > > > > + clock-names = "pll0_0", "pll0_1", > > > > + "pll1_0", > > > "pll1_1"; > > I didn't spot this last time, but the clock-names here seem to be the > names of the outputs from the provider, rather than the input names of > the consumer. This goes against the intended purpose of clock-names. > I am confused here with provider/consumer because some nodes can both be consumer and provider. The clock-names are corresponding to clocks one by one, what's wrong with it? Regards, Yuantian > > > > + clock-output-names = "cmux1"; > > > > > > How does the mux choose which input clock to use at a point in time? > > > > > That is decided at runtime. Different input clock will lead to the > > different Clock frequency that the CPUs work on. > > Ok. > > Cheers, > Mark.
On Fri, 2013-10-11 at 21:52 -0500, Tang Yuantian-B29983 wrote: > Thanks for your review. > > > -----Original Message----- > > From: Wood Scott-B07421 > > Sent: 2013年10月12日 星期六 3:07 > > To: Mark Rutland > > Cc: Tang Yuantian-B29983; devicetree@vger.kernel.org; linuxppc- > > dev@lists.ozlabs.org; Li Yang-Leo-R58472 > > Subject: Re: [PATCH v5] powerpc/mpc85xx: Update the clock nodes in device > > tree > > > > I'm not sure I understand the "_0"/"_1" part, though. Doesn't each PLL > > just have one output, which the consumer may choose to divide by 2 (or in > > some cases 4)? Why does that division need to be exposed in the device > > tree as separate connections to the parent clock? > > > The device tree makes that quite clear. You chose to model it that way in the device tree; that doesn't make it clear that the hardware works that way or that it's a good way to model it. > Each PLL has several output which MUX node can take from. Point out where in the hardware documentation it says this. What I see is a PLL that has one output, and a MUX register that can choose from multiple PLL and divider options. > It is not a runtime decision. Hmm? It's a register you write to. -Scott
> -----Original Message----- > From: Wood Scott-B07421 > Sent: 2013年10月15日 星期二 6:13 > To: Tang Yuantian-B29983 > Cc: Wood Scott-B07421; Mark Rutland; devicetree@vger.kernel.org; > linuxppc-dev@lists.ozlabs.org; Li Yang-Leo-R58472 > Subject: Re: [PATCH v5] powerpc/mpc85xx: Update the clock nodes in device > tree > > On Fri, 2013-10-11 at 21:52 -0500, Tang Yuantian-B29983 wrote: > > Thanks for your review. > > > > > -----Original Message----- > > > From: Wood Scott-B07421 > > > Sent: 2013年10月12日 星期六 3:07 > > > To: Mark Rutland > > > Cc: Tang Yuantian-B29983; devicetree@vger.kernel.org; linuxppc- > > > dev@lists.ozlabs.org; Li Yang-Leo-R58472 > > > Subject: Re: [PATCH v5] powerpc/mpc85xx: Update the clock nodes in > > > device tree > > > > > > I'm not sure I understand the "_0"/"_1" part, though. Doesn't each > > > PLL just have one output, which the consumer may choose to divide by > > > 2 (or in some cases 4)? Why does that division need to be exposed > > > in the device tree as separate connections to the parent clock? > > > > > The device tree makes that quite clear. > > You chose to model it that way in the device tree; that doesn't make it > clear that the hardware works that way or that it's a good way to model > it. > > > Each PLL has several output which MUX node can take from. > > Point out where in the hardware documentation it says this. What I see > is a PLL that has one output, and a MUX register that can choose from > multiple PLL and divider options. > Take T4240 for example: see section 4.6.5.1 , (Page 141) in T4240RM Rev. D, 09/2012. > > It is not a runtime decision. > > Hmm? It's a register you write to. > I mean the number of PLL output or the division of PLL is not a runtime decision. Which output of PLL the MUX can take is a runtime decision. Regards, Yuantian > -Scott >
On Mon, 2013-10-14 at 20:59 -0500, Tang Yuantian-B29983 wrote: > > -----Original Message----- > > From: Wood Scott-B07421 > > Sent: 2013年10月15日 星期二 6:13 > > To: Tang Yuantian-B29983 > > Cc: Wood Scott-B07421; Mark Rutland; devicetree@vger.kernel.org; > > linuxppc-dev@lists.ozlabs.org; Li Yang-Leo-R58472 > > Subject: Re: [PATCH v5] powerpc/mpc85xx: Update the clock nodes in device > > tree > > > > On Fri, 2013-10-11 at 21:52 -0500, Tang Yuantian-B29983 wrote: > > > Thanks for your review. > > > > > > > -----Original Message----- > > > > From: Wood Scott-B07421 > > > > Sent: 2013年10月12日 星期六 3:07 > > > > To: Mark Rutland > > > > Cc: Tang Yuantian-B29983; devicetree@vger.kernel.org; linuxppc- > > > > dev@lists.ozlabs.org; Li Yang-Leo-R58472 > > > > Subject: Re: [PATCH v5] powerpc/mpc85xx: Update the clock nodes in > > > > device tree > > > > > > > > I'm not sure I understand the "_0"/"_1" part, though. Doesn't each > > > > PLL just have one output, which the consumer may choose to divide by > > > > 2 (or in some cases 4)? Why does that division need to be exposed > > > > in the device tree as separate connections to the parent clock? > > > > > > > The device tree makes that quite clear. > > > > You chose to model it that way in the device tree; that doesn't make it > > clear that the hardware works that way or that it's a good way to model > > it. > > > > > Each PLL has several output which MUX node can take from. > > > > Point out where in the hardware documentation it says this. What I see > > is a PLL that has one output, and a MUX register that can choose from > > multiple PLL and divider options. > > > Take T4240 for example: see section 4.6.5.1 , (Page 141) in T4240RM Rev. D, 09/2012. That shows the dividers as being somewhere in between the PLL and the MUX. The MUX is where the divider is selected. There's nothing in the PLL's programming interface that relates to the dividers. As such it's simpler to model it as being part of the MUX. -Scott
> > > > > > > > > The device tree makes that quite clear. > > > > > > You chose to model it that way in the device tree; that doesn't make > > > it clear that the hardware works that way or that it's a good way to > > > model it. > > > > > > > Each PLL has several output which MUX node can take from. > > > > > > Point out where in the hardware documentation it says this. What I > > > see is a PLL that has one output, and a MUX register that can choose > > > from multiple PLL and divider options. > > > > > Take T4240 for example: see section 4.6.5.1 , (Page 141) in T4240RM Rev. > D, 09/2012. > > That shows the dividers as being somewhere in between the PLL and the MUX. > The MUX is where the divider is selected. There's nothing in the PLL's > programming interface that relates to the dividers. As such it's simpler > to model it as being part of the MUX. > > -Scott > I don't know whether it is simpler, but "modeling divider as being part of the MUX" is your guess, right? If the "divider" is included in MUX, the MUX would not be called "MUX". I don't know whether "divider" module exists or not. If it exists, it should be part of PLL or between PLL and MUX. wherever it was, the device tree binding is appropriate. The P3041RM shows exactly each PLL has 2 outputs which definitely have no "divider" at all. Regards, Yuantian
On Tue, 2013-10-15 at 21:57 -0500, Tang Yuantian-B29983 wrote: > > > > > > > > > > > The device tree makes that quite clear. > > > > > > > > You chose to model it that way in the device tree; that doesn't make > > > > it clear that the hardware works that way or that it's a good way to > > > > model it. > > > > > > > > > Each PLL has several output which MUX node can take from. > > > > > > > > Point out where in the hardware documentation it says this. What I > > > > see is a PLL that has one output, and a MUX register that can choose > > > > from multiple PLL and divider options. > > > > > > > Take T4240 for example: see section 4.6.5.1 , (Page 141) in T4240RM Rev. > > D, 09/2012. > > > > That shows the dividers as being somewhere in between the PLL and the MUX. > > The MUX is where the divider is selected. There's nothing in the PLL's > > programming interface that relates to the dividers. As such it's simpler > > to model it as being part of the MUX. > > > > -Scott > > > I don't know whether it is simpler, but "modeling divider as being part of the MUX" > is your guess, right? > If the "divider" is included in MUX, the MUX would not be called "MUX". It's still selecting from multiple PLLs. > I don't know whether "divider" module exists or not. If it exists, it should be part > of PLL or between PLL and MUX. wherever it was, the device tree binding is appropriate. The device tree binding is unnecessarily complicated. > The P3041RM shows exactly each PLL has 2 outputs which definitely have no "divider" at all. That diagram is a bit weird -- one of the outputs is used as is, and the other is split into 1/2 and 1/4. It doesn't really matter though; the end result is the same. We're describing the programming interface, not artwork choices in the manual. -Scott
> > > That shows the dividers as being somewhere in between the PLL and the > MUX. > > > The MUX is where the divider is selected. There's nothing in the > > > PLL's programming interface that relates to the dividers. As such > > > it's simpler to model it as being part of the MUX. > > > > > > -Scott > > > > > I don't know whether it is simpler, but "modeling divider as being part > of the MUX" > > is your guess, right? > > If the "divider" is included in MUX, the MUX would not be called "MUX". > > It's still selecting from multiple PLLs. > > > I don't know whether "divider" module exists or not. If it exists, it > > should be part of PLL or between PLL and MUX. wherever it was, the > device tree binding is appropriate. > > The device tree binding is unnecessarily complicated. > > > The P3041RM shows exactly each PLL has 2 outputs which definitely have > no "divider" at all. > > That diagram is a bit weird -- one of the outputs is used as is, and the > other is split into 1/2 and 1/4. It doesn't really matter though; the > end result is the same. We're describing the programming interface, not > artwork choices in the manual. > > -Scott > If the device tree needs to be modified, could you give me your suggestions? Regards, Yuantian
On Wed, 2013-10-16 at 21:08 -0500, Tang Yuantian-B29983 wrote: > > > > That shows the dividers as being somewhere in between the PLL and the > > MUX. > > > > The MUX is where the divider is selected. There's nothing in the > > > > PLL's programming interface that relates to the dividers. As such > > > > it's simpler to model it as being part of the MUX. > > > > > > > > -Scott > > > > > > > I don't know whether it is simpler, but "modeling divider as being part > > of the MUX" > > > is your guess, right? > > > If the "divider" is included in MUX, the MUX would not be called "MUX". > > > > It's still selecting from multiple PLLs. > > > > > I don't know whether "divider" module exists or not. If it exists, it > > > should be part of PLL or between PLL and MUX. wherever it was, the > > device tree binding is appropriate. > > > > The device tree binding is unnecessarily complicated. > > > > > The P3041RM shows exactly each PLL has 2 outputs which definitely have > > no "divider" at all. > > > > That diagram is a bit weird -- one of the outputs is used as is, and the > > other is split into 1/2 and 1/4. It doesn't really matter though; the > > end result is the same. We're describing the programming interface, not > > artwork choices in the manual. > > > > -Scott > > > If the device tree needs to be modified, could you give me your suggestions? My suggestion is to have only one output per PLL node. The MUX node would have one input per PLL. Dividers would be handled internally to the MUX driver. -Scott
> On Wed, 2013-10-16 at 21:08 -0500, Tang Yuantian-B29983 wrote: > > > > > That shows the dividers as being somewhere in between the PLL > > > > > and the > > > MUX. > > > > > The MUX is where the divider is selected. There's nothing in > > > > > the PLL's programming interface that relates to the dividers. > > > > > As such it's simpler to model it as being part of the MUX. > > > > > > > > > > -Scott > > > > > > > > > I don't know whether it is simpler, but "modeling divider as being > > > > part > > > of the MUX" > > > > is your guess, right? > > > > If the "divider" is included in MUX, the MUX would not be called > "MUX". > > > > > > It's still selecting from multiple PLLs. > > > > > > > I don't know whether "divider" module exists or not. If it exists, > > > > it should be part of PLL or between PLL and MUX. wherever it was, > > > > the > > > device tree binding is appropriate. > > > > > > The device tree binding is unnecessarily complicated. > > > > > > > The P3041RM shows exactly each PLL has 2 outputs which definitely > > > > have > > > no "divider" at all. > > > > > > That diagram is a bit weird -- one of the outputs is used as is, and > > > the other is split into 1/2 and 1/4. It doesn't really matter > > > though; the end result is the same. We're describing the > > > programming interface, not artwork choices in the manual. > > > > > > -Scott > > > > > If the device tree needs to be modified, could you give me your > suggestions? > > My suggestion is to have only one output per PLL node. The MUX node > would have one input per PLL. Dividers would be handled internally to > the MUX driver. > > -Scott > I don't think your suggestion is constructive. Your suggestion is on the premise of that the "divider" is part of MUX, And I think it should be part of PLL. MUX can't CREATE clock which PLL can. So my thought is more reasonable. If the device tree documents like what you said, it would have few help for driver. The reason is obvious that the driver is still going to deal with the "divider" detail which varies from platform to platform. I will document as it was unless you prove your suggestion. Thanks, Yuantian
On Thu, 2013-10-17 at 21:06 -0500, Tang Yuantian-B29983 wrote: > > On Wed, 2013-10-16 at 21:08 -0500, Tang Yuantian-B29983 wrote: > > > > > > That shows the dividers as being somewhere in between the PLL > > > > > > and the > > > > MUX. > > > > > > The MUX is where the divider is selected. There's nothing in > > > > > > the PLL's programming interface that relates to the dividers. > > > > > > As such it's simpler to model it as being part of the MUX. > > > > > > > > > > > > -Scott > > > > > > > > > > > I don't know whether it is simpler, but "modeling divider as being > > > > > part > > > > of the MUX" > > > > > is your guess, right? > > > > > If the "divider" is included in MUX, the MUX would not be called > > "MUX". > > > > > > > > It's still selecting from multiple PLLs. > > > > > > > > > I don't know whether "divider" module exists or not. If it exists, > > > > > it should be part of PLL or between PLL and MUX. wherever it was, > > > > > the > > > > device tree binding is appropriate. > > > > > > > > The device tree binding is unnecessarily complicated. > > > > > > > > > The P3041RM shows exactly each PLL has 2 outputs which definitely > > > > > have > > > > no "divider" at all. > > > > > > > > That diagram is a bit weird -- one of the outputs is used as is, and > > > > the other is split into 1/2 and 1/4. It doesn't really matter > > > > though; the end result is the same. We're describing the > > > > programming interface, not artwork choices in the manual. > > > > > > > > -Scott > > > > > > > If the device tree needs to be modified, could you give me your > > suggestions? > > > > My suggestion is to have only one output per PLL node. The MUX node > > would have one input per PLL. Dividers would be handled internally to > > the MUX driver. > > > > -Scott > > > I don't think your suggestion is constructive. Hmm? > Your suggestion is on the premise of that the "divider" is part of MUX, > And I think it should be part of PLL. > MUX can't CREATE clock which PLL can. So my thought is more reasonable. > If the device tree documents like what you said, it would have few help for driver. > The reason is obvious that the driver is still going to deal with the "divider" > detail which varies from platform to platform. > I will document as it was unless you prove your suggestion. I can't follow this. My point is that my suggestion better matches the programming model, and is simpler. -Scott
> > > > > > > > > > It's still selecting from multiple PLLs. > > > > > > > > > > > I don't know whether "divider" module exists or not. If it > > > > > > exists, it should be part of PLL or between PLL and MUX. > > > > > > wherever it was, the > > > > > device tree binding is appropriate. > > > > > > > > > > The device tree binding is unnecessarily complicated. > > > > > > > > > > > The P3041RM shows exactly each PLL has 2 outputs which > > > > > > definitely have > > > > > no "divider" at all. > > > > > > > > > > That diagram is a bit weird -- one of the outputs is used as is, > > > > > and the other is split into 1/2 and 1/4. It doesn't really > > > > > matter though; the end result is the same. We're describing the > > > > > programming interface, not artwork choices in the manual. > > > > > > > > > > -Scott > > > > > > > > > If the device tree needs to be modified, could you give me your > > > suggestions? > > > > > > My suggestion is to have only one output per PLL node. The MUX node > > > would have one input per PLL. Dividers would be handled internally > > > to the MUX driver. > > > > > > -Scott > > > > > I don't think your suggestion is constructive. > > Hmm? > > > Your suggestion is on the premise of that the "divider" is part of > > MUX, And I think it should be part of PLL. > > MUX can't CREATE clock which PLL can. So my thought is more reasonable. > > If the device tree documents like what you said, it would have few help > for driver. > > The reason is obvious that the driver is still going to deal with the > "divider" > > detail which varies from platform to platform. > > I will document as it was unless you prove your suggestion. > > I can't follow this. My point is that my suggestion better matches the > programming model, and is simpler. > > -Scott > I didn't see how your suggestion is a better matching. OSC ----> PLL1 ----> mux ----> CPU | | |--> PLL2 --| ........ As your suggestion, the clock tree looks like the above. In this case, the MUX driver will not know the divider details(/2, /4, or /3). I think the MUX should act like "switch" which choose one of the input clock as a output clock. It should not CREATE clock(like PLL1/2, PLL1/4). The purpose of clock driver is to establish the clock tree. The clock tree will not be established in your suggestion because the divider is missing, we don't know where PLL/2 comes from. If you really like your proposal, it should be changed to this: OSC ------> PLL1 -----> PLL1 /1 ---------> MUX ------->CPU | |___> PLL1 /2 _______| | | |____> PLL2 -----> PLL2 /2 -------| |___> PLL2/ 4 _______| (it is possible that PLLs have different divider). Regards, Yuantian
On Sat, Oct 12, 2013 at 04:40:06AM +0100, Tang Yuantian-B29983 wrote: > Thanks for your review. > > > > > > > > > > > +- reg: Offset and length of the clock register set > > > > > +- clock-frequency: Indicates input clock frequency of clock block. > > > > > + Will be set by u-boot > > > > > > > > Why does the fact this is set by u-boot matter to the binding? > > > > > > > OK, I will remove it. > > > > > > > > + > > > > > +Recommended properties: > > > > > +- #ddress-cells: Specifies the number of cells used to represent > > > > > + physical base addresses. Must be present if the device has > > > > > + sub-nodes and set to 1 if present > > > > > > > > Typo: #address-cells > > > > > > > > In the example it looks like the address cells of child nodes are > > > > offsets within the unit, rather than absolute physical addresses. > > > > Could the code not treat them as absolute addresses? Then we'd only > > > > need to document that #address-cells, #size-cells and ranges must be > > > > present and have values suitable for mapping child nodes into the > > > > address space of the parent. > > > > > > > OK, thanks. > > > > > > > > +- #size-cells: Specifies the number of cells used to represent > > > > > + the size of an address. Must be present if the device has > > > > > + sub-nodes and set to 1 if present > > > > > > > > It's not really the size of an address, it's the size of a region > > > > identified by a reg entry. > > > > > > > > I think this can be simplified by my suggestion above. > > > > > > > Yes > > > > Aah, I see that this is already in use, so it's a bit late to change > > this... > > > I will modify the driver anyway once the binding gets done. Won't this break existing users DTBs? > > > > > > > > > + > > > > > +2. Clock Provider/Consumer Binding > > > > > + > > > > > +Most of the binding are from the common clock binding[1]. > > > > > + [1] Documentation/devicetree/bindings/clock/clock-bindings.txt > > > > > + > > > > > +Required properties: > > > > > +- compatible : Should include one or more of the following: > > > > I didn't spot this earlier, but why "one or more"? are the 2.0 variants > > backwards compatible with the 1.0 variants. > > > One or more for fixed-clock which is compatible with qoriq-sysclk-*. This may be somewhat pedantic, but an element from the list plus fixed-clock isn't one or more of the following. It's one of the following plus fixed-clock. It may be better to explicitly state that the compatible list must include one of the following, plus fixed-clock. > > > > > > + - "fsl,qoriq-core-pll-[1,2].x": Indicates a core PLL clock > > > > device > > > > > + - "fsl,qoriq-core-mux-[1,2].x": Indicates a core > > > > > + multiplexer > > > > clock > > > > > + device; divided from the core PLL clock > > > > > > > > As above, I'd prefer a complete list of the basic strings we expect. > > > > > > > There is no list here, just *-mux-1.x and *-mux-2.x What kind of list > > > do you prefer? > > > > Something like: > > > > - compatible: Should include at least one of: > > * "fsl,qoriq-core-pll-1.0" for core PLL clocks (v1.0) > > * "fsl,qoriq-core-pll-2.0" for core PLL clocks (v2.0) > > * "fsl,qoriq-core-mux-1.0" for core mux clocks (v1.0) > > * "fsl,qoriq-core-mux-2.0" for core mux clocks (v2.0) > > The *-2.0 variants are backwards compatible with the *-1.0 variants, > > so for compatiblity a *-1.0 variant string should be in the list. > > > See the comment by Scott wood. OK. The note at the bottom cna go, but I'd still prefer an explicit list with a description of each entry. > > > > > > > > > + - "fixed-clock": From common clock binding; indicates > > > > > + output > > > > clock > > > > > + of oscillator > > > > > + - "fsl,qoriq-sysclk-[1,2].x": Indicates input system clock > > > > > > > > Here too. > > > > > > > > > +- #clock-cells: From common clock binding; indicates the number of > > > > > + output clock. 0 is for one output clock; 1 for more than > > > > > +one clock > > > > > > > > If a clock source has multiple outputs, what those outputs are and > > > > what values in clock-cells they correspond to should be described > > here. > > > > > > > There is no way to describe the values of multiple outputs here. > > > This property is the type of BOOL. See the clock-bindings.txt. > > > > Sorry? #clock-cells is most definitely _not_ a bool: > > > ACTs like bool. No it does not act like boo, a nd only appears to if you do not consider the general casel. In general #clock-cells could be any arbitrarily large number, not just <0> or <1>. It represents the number of cells in a clock-specifier, not simply that there is a clock-specifier. It's perfectly possible to have #clock-cells = <2>, or more. It' perfectly possible to have a DT like the below (with some properties omitted for brevity): clk_2: some-clock { compatible = "vendor,banked-clock"; #clock-cells = <2>; }; clk_5: other-clock { #clock-cells = <5>; }; clk_0: another-clock { #clock-cells = <0>; }; consumer { clocks = <&clk5 0 17 53 91 0>, <&clk_0>, <&clk2 3 17>; }; In all of these cases, the cells in the clock-specifier must mean something. They uniquely identify a clock output of the clock provider, and there must be a way of mapping them to a particular clock. The meaning of the cells in the clock specifier should be specified. Consider "vendor,banked-clock". It's binding could look something like: - compatible: should contain * "vendor,banked-clock" for v1 banked clock designs - #clock-cells: Should be <2> * The first cell selects the internal clock bank, indexed [1,3] * The second cell selects a clock within the bank, indexed [0,25] A clock might have a set of named outputs: - #clock-cells: should be <1>, a single cell which may be one of the following: * 0 - REFCLKOUT * 1 - PLLCLKOUT * 5 - LOWPWRCLKOUT A clock provider might have a contiguous (or discontiguous) set of clock indexes: - #clock-cells: should be <1>. The clock index as per the documentation, in the range [0,15]. I hope this clears up the confusion on what I am asking for. > > > 17: #clock-cells: Number of cells in a clock specifier; Typically 0 > > for nodes > > 18: with a single clock output and 1 for nodes with > > multiple > > 19: clock outputs. > > > > And neither are the clock-specifiers encoded with those cells. Consider: > > > > pll0: pll0@800 { > > #clock-cells = <1>; > > reg = <0x800 0x4>; > > compatible = "fsl,qoriq-core-pll-1.0"; > > clocks = <&sysclk>; > > clock-output-names = "pll0", "pll0-div2"; > > }; > > > > Here the value of the cells in a clock-specifier seem to be: > > 0: pll0 > > 1: pll0-div2 > > > > So in a consumer, the valid values of the cells in a clock-specifier > > are: > > > > consumer: device { > > compatible = "vendor,some-device"; > > clocks = <&pll0 0>, /* pll0 */ > > <&pll0 1>; /* pll0-div2 */ > > }; > > > > There must be some meaning assigned to the values of the cells in the > > clock-specifier (e.g. linear index of the clock output in the hardware). > > It would be good to describe this, other clock bindings do. > > > Sorry, I still get what you mean. There is no INDEX at all. > 0 is for one output, 1 for multiple output. Just like that. > What the " other clock bindings" did you refer to? Take a look at Documentation/devicetree/bindings/clock/imx23-clock.txt, which specifies each clock output by name. You can also take a look at the tegra clock bindings, which define the set of clocks by reference to a device tree header file. > > > > > > > > > + > > > > > +Recommended properties: > > > > > +- clocks: Should be the phandle of input parent clock > > > > > +- clock-names: From common clock binding, indicates the clock > > > > > +name > > > > > > > > That description's a bit opaque. > > > > > > > > What's the name of the clock input on these units? That's what > > > > clock- names should contain, and that should be documented. > > > > > > > Is it necessary to document these names since they are totally used by > > > clock provider and clock consumer has no idea about them? > > > > I'm not sure I follow -- clocks and clock-names are used by consumers. > > They define which clocks are inputs to a consumer, and the names of the > > clock inputs on the consumer: > > > > consumer@0xffff0000 { > > reg = <0xffff0000 0x1000>; > > compatible = "vendor,some-consumer"; > > clocks = <&pl011 0>, > > <&otherclock 43 22>, > > <&pl011 1>; > > clock-names = "apb_pclk", > > "pixel_clk", > > "scanout_clk"; > > }; > > > > Here the set of clock-names would be defined in the binding of the > > consumer, based on the clock input names in the IP documentation -- they > > tell the consumer which clock inputs on the consumer the clocks described > > in clocks are wired in to, and describe nothing about output of the > > provider. Using clock-names allows the set of clocks on an IP to change > > over revisions and for some clock inputs to be optional. > > > OK, I get it. Will name the clock-names better, and add it if missed. > Thanks, Cheers. [...] > > > > > + #address-cells = <1>; > > > > > + #size-cells = <1>; > > > > > + > > > > > + sysclk: sysclk { > > > > > + #clock-cells = <0>; > > > > > + compatible = "fsl,qoriq-sysclk-1.0", > > > > > + "fixed-clock"; > > > > > > > > We didn't mention in the binding that "fsl,qoriq-sysclk-1.0" was > > > > compatible with "fixed-clock" and should have "fixed-clock" in the > > > > compatible list... > > > > > > > OK, will fix it. > > > > Cheers. Also, doesn't a fixed-clock require a clock-frequency? > > > Yes, it should be. But we got the clock-frequency from parent node because > There is a clock-frequency already there before this patch. OK. Should we not place it there for future? We're not compatible with fixed-clock if we don't fulfil the minimum requirements of the fixed-clock binding... > > > > > > > > > + clock-output-names = "sysclk"; > > > > > + } > > > > > + > > > > > + pll0: pll0@800 { > > > > > + #clock-cells = <1>; > > > > > + reg = <0x800 0x4>; > > > > > + compatible = "fsl,qoriq-core-pll-1.0"; > > > > > + clocks = <&sysclk>; > > > > > + clock-output-names = "pll0", "pll0-div2"; > > > > > + }; > > > > > + > > > > > + pll1: pll1@820 { > > > > > + #clock-cells = <1>; > > > > > + reg = <0x820 0x4>; > > > > > + compatible = "fsl,qoriq-core-pll-1.0"; > > > > > + clocks = <&sysclk>; > > > > > + clock-output-names = "pll1", "pll1-div2"; > > > > > + }; > > > > > + > > > > > + mux0: mux0@0 { > > > > > + #clock-cells = <0>; > > > > > + reg = <0x0 0x4>; > > > > > + compatible = "fsl,qoriq-core-mux-1.0"; > > > > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, > > > > <&pll1 1>; > > > > > + clock-names = "pll0_0", "pll0_1", > > > > > + "pll1_0", > > > > "pll1_1"; > > > > > + clock-output-names = "cmux0"; > > > > > + }; > > > > > + > > > > > + mux1: mux1@20 { > > > > > + #clock-cells = <0>; > > > > > + reg = <0x20 0x4>; > > > > > + compatible = "fsl,qoriq-core-mux-1.0"; > > > > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, > > > > <&pll1 1>; > > > > > + clock-names = "pll0_0", "pll0_1", > > > > > + "pll1_0", > > > > "pll1_1"; > > > > I didn't spot this last time, but the clock-names here seem to be the > > names of the outputs from the provider, rather than the input names of > > the consumer. This goes against the intended purpose of clock-names. > > > I am confused here with provider/consumer because some nodes can both be consumer and provider. > The clock-names are corresponding to clocks one by one, what's wrong with it? We have clock-names for a consumer's names of its inputs, and clock-output-names for a provider's names of it's outputs. Consider a PLL device which takes a clock input (REFCLK) and outputs a higher frequency clock (OUTCLK). The PLL's specification only talks in terms of REFCLK and OUTCLK, but these are not the names of the clocks as they are output from the original clock, or provided to the final consumer. Consider a sequence of these PLLs plugged together. The OUTCLK of one feeds into the REFCLK of the next: fixed: fixed-clock { compatible = "fixed-clock"; clock-frequency = <50>; }; pll_0: pll { compatible = "vendor,pll"; clocks = <&fixed>; clock-names = "refclk"; clock-output-names = "outclk"; }; pll_1: another-pll { compatible = "vendor,pll"; clocks = <&pll_0>; clock-names = "refclk"; clock-output-names = "outclk"; }; consumer { compatible = "vendor,clock-consumer"; clocks = <&pll_1>, <&pll_0>; clock-names = "halfclk", "fullclk"; }; In each case, clock-names describes the clock inputs from the view of the PLL they are input to, not from the provider they came from, or the consumer some outputs are going to. Giving the inputs names in this way makes it possible to describe situations where onlya subset of clock inputs are wired up -- we could just have "fullclk" on the final consumer, with only pll_0 as an input, and the driver could figure out what to do. Does that help? Thanks, Mark.
Thanks for your review. > -----Original Message----- > From: Mark Rutland [mailto:mark.rutland@arm.com] > Sent: 2013年10月21日 星期一 17:15 > To: Tang Yuantian-B29983 > Cc: galak@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org; > devicetree@vger.kernel.org; Li Yang-Leo-R58472 > Subject: Re: [PATCH v5] powerpc/mpc85xx: Update the clock nodes in device > tree > > On Sat, Oct 12, 2013 at 04:40:06AM +0100, Tang Yuantian-B29983 wrote: > > Thanks for your review. > > > > > > > > > > > > > > > +- reg: Offset and length of the clock register set > > > > > > +- clock-frequency: Indicates input clock frequency of clock > block. > > > > > > + Will be set by u-boot > > > > > > > > > > Why does the fact this is set by u-boot matter to the binding? > > > > > > > > > OK, I will remove it. > > > > > > > > > > + > > > > > > +Recommended properties: > > > > > > +- #ddress-cells: Specifies the number of cells used to > represent > > > > > > + physical base addresses. Must be present if the device > has > > > > > > + sub-nodes and set to 1 if present > > > > > > > > > > Typo: #address-cells > > > > > > > > > > In the example it looks like the address cells of child nodes > > > > > are offsets within the unit, rather than absolute physical > addresses. > > > > > Could the code not treat them as absolute addresses? Then we'd > > > > > only need to document that #address-cells, #size-cells and > > > > > ranges must be present and have values suitable for mapping > > > > > child nodes into the address space of the parent. > > > > > > > > > OK, thanks. > > > > > > > > > > +- #size-cells: Specifies the number of cells used to represent > > > > > > + the size of an address. Must be present if the device > has > > > > > > + sub-nodes and set to 1 if present > > > > > > > > > > It's not really the size of an address, it's the size of a > > > > > region identified by a reg entry. > > > > > > > > > > I think this can be simplified by my suggestion above. > > > > > > > > > Yes > > > > > > Aah, I see that this is already in use, so it's a bit late to change > > > this... > > > > > I will modify the driver anyway once the binding gets done. > > Won't this break existing users DTBs? > This binding and DT should be merged first. For some reasons, the driver has already been merged. > > > > > > > > > > > > + > > > > > > +2. Clock Provider/Consumer Binding > > > > > > + > > > > > > +Most of the binding are from the common clock binding[1]. > > > > > > + [1] > > > > > > +Documentation/devicetree/bindings/clock/clock-bindings.txt > > > > > > + > > > > > > +Required properties: > > > > > > +- compatible : Should include one or more of the following: > > > > > > I didn't spot this earlier, but why "one or more"? are the 2.0 > > > variants backwards compatible with the 1.0 variants. > > > > > One or more for fixed-clock which is compatible with qoriq-sysclk-*. > > This may be somewhat pedantic, but an element from the list plus fixed- > clock isn't one or more of the following. It's one of the following plus > fixed-clock. > > It may be better to explicitly state that the compatible list must > include one of the following, plus fixed-clock. > Since the qoriq-sysclk-* is not 100% compatible with fixed-clock(no clock-frequency property), I will remove fixed-clock. > > > > > > > > + - "fsl,qoriq-core-pll-[1,2].x": Indicates a core PLL > > > > > > + clock > > > > > device > > > > > > + - "fsl,qoriq-core-mux-[1,2].x": Indicates a core > > > > > > + multiplexer > > > > > clock > > > > > > + device; divided from the core PLL clock > > > > > > > > > > As above, I'd prefer a complete list of the basic strings we > expect. > > > > > > > > > There is no list here, just *-mux-1.x and *-mux-2.x What kind of > > > > list do you prefer? > > > > > > Something like: > > > > > > - compatible: Should include at least one of: > > > * "fsl,qoriq-core-pll-1.0" for core PLL clocks (v1.0) > > > * "fsl,qoriq-core-pll-2.0" for core PLL clocks (v2.0) > > > * "fsl,qoriq-core-mux-1.0" for core mux clocks (v1.0) > > > * "fsl,qoriq-core-mux-2.0" for core mux clocks (v2.0) > > > The *-2.0 variants are backwards compatible with the *-1.0 variants, > > > so for compatiblity a *-1.0 variant string should be in the list. > > > > > See the comment by Scott wood. > > OK. The note at the bottom cna go, but I'd still prefer an explicit list > with a description of each entry. > OK, will give a list for them. > > > > > > > > > > > > + - "fixed-clock": From common clock binding; indicates > > > > > > + output > > > > > clock > > > > > > + of oscillator > > > > > > + - "fsl,qoriq-sysclk-[1,2].x": Indicates input system > > > > > > + clock > > > > > > > > > > Here too. > > > > > > > > > > > +- #clock-cells: From common clock binding; indicates the > number of > > > > > > + output clock. 0 is for one output clock; 1 for more > > > > > > +than one clock > > > > > > > > > > If a clock source has multiple outputs, what those outputs are > > > > > and what values in clock-cells they correspond to should be > > > > > described > > > here. > > > > > > > > > There is no way to describe the values of multiple outputs here. > > > > This property is the type of BOOL. See the clock-bindings.txt. > > > > > > Sorry? #clock-cells is most definitely _not_ a bool: > > > > > ACTs like bool. > > No it does not act like boo, a nd only appears to if you do not consider > the general casel. In general #clock-cells could be any arbitrarily large > number, not just <0> or <1>. It represents the number of cells in a > clock-specifier, not simply that there is a clock-specifier. > > It's perfectly possible to have #clock-cells = <2>, or more. It' > perfectly possible to have a DT like the below (with some properties > omitted for > brevity): > > clk_2: some-clock { > compatible = "vendor,banked-clock"; > #clock-cells = <2>; > }; > > clk_5: other-clock { > #clock-cells = <5>; > }; > > clk_0: another-clock { > #clock-cells = <0>; > }; > > consumer { > clocks = <&clk5 0 17 53 91 0>, > <&clk_0>, > <&clk2 3 17>; > }; > > In all of these cases, the cells in the clock-specifier must mean > something. > They uniquely identify a clock output of the clock provider, and there > must be a way of mapping them to a particular clock. > > The meaning of the cells in the clock specifier should be specified. > Consider "vendor,banked-clock". It's binding could look something like: > > - compatible: should contain > * "vendor,banked-clock" for v1 banked clock designs > - #clock-cells: Should be <2> > * The first cell selects the internal clock bank, indexed [1,3] > * The second cell selects a clock within the bank, indexed [0,25] > > A clock might have a set of named outputs: > > - #clock-cells: should be <1>, a single cell which may be one of the > following: > * 0 - REFCLKOUT > * 1 - PLLCLKOUT > * 5 - LOWPWRCLKOUT > > A clock provider might have a contiguous (or discontiguous) set of clock > indexes: > > - #clock-cells: should be <1>. The clock index as per the documentation, > in the range [0,15]. > > I hope this clears up the confusion on what I am asking for. > I understand now. There could be a clock-cells with the value of more than 1. That is just rarely used. In my case, #clock-cells should be <1> and I will add a list of values. > > > > > 17: #clock-cells: Number of cells in a clock specifier; > Typically 0 > > > for nodes > > > 18: with a single clock output and 1 for nodes > with > > > multiple > > > 19: clock outputs. > > > > > > And neither are the clock-specifiers encoded with those cells. > Consider: > > > > > > pll0: pll0@800 { > > > #clock-cells = <1>; > > > reg = <0x800 0x4>; > > > compatible = "fsl,qoriq-core-pll-1.0"; > > > clocks = <&sysclk>; > > > clock-output-names = "pll0", "pll0-div2"; > > > }; > > > > > > Here the value of the cells in a clock-specifier seem to be: > > > 0: pll0 > > > 1: pll0-div2 > > > > > > So in a consumer, the valid values of the cells in a clock-specifier > > > are: > > > > > > consumer: device { > > > compatible = "vendor,some-device"; > > > clocks = <&pll0 0>, /* pll0 */ > > > <&pll0 1>; /* pll0-div2 */ > > > }; > > > > > > There must be some meaning assigned to the values of the cells in > > > the clock-specifier (e.g. linear index of the clock output in the > hardware). > > > It would be good to describe this, other clock bindings do. > > > > > Sorry, I still get what you mean. There is no INDEX at all. > > 0 is for one output, 1 for multiple output. Just like that. > > What the " other clock bindings" did you refer to? > > Take a look at Documentation/devicetree/bindings/clock/imx23-clock.txt, > which specifies each clock output by name. You can also take a look at > the tegra clock bindings, which define the set of clocks by reference to > a device tree header file. > Got it. > > > > > > > > > > > > + > > > > > > +Recommended properties: > > > > > > +- clocks: Should be the phandle of input parent clock > > > > > > +- clock-names: From common clock binding, indicates the clock > > > > > > +name > > > > > > > > > > That description's a bit opaque. > > > > > > > > > > What's the name of the clock input on these units? That's what > > > > > clock- names should contain, and that should be documented. > > > > > > > > > Is it necessary to document these names since they are totally > > > > used by clock provider and clock consumer has no idea about them? > > > > > > I'm not sure I follow -- clocks and clock-names are used by consumers. > > > They define which clocks are inputs to a consumer, and the names of > > > the clock inputs on the consumer: > > > > > > consumer@0xffff0000 { > > > reg = <0xffff0000 0x1000>; > > > compatible = "vendor,some-consumer"; > > > clocks = <&pl011 0>, > > > <&otherclock 43 22>, > > > <&pl011 1>; > > > clock-names = "apb_pclk", > > > "pixel_clk", > > > "scanout_clk"; > > > }; > > > > > > Here the set of clock-names would be defined in the binding of the > > > consumer, based on the clock input names in the IP documentation -- > > > they tell the consumer which clock inputs on the consumer the clocks > > > described in clocks are wired in to, and describe nothing about > > > output of the provider. Using clock-names allows the set of clocks > > > on an IP to change over revisions and for some clock inputs to be > optional. > > > > > OK, I get it. Will name the clock-names better, and add it if missed. > > Thanks, > > Cheers. > > [...] > > > > > > > + #address-cells = <1>; > > > > > > + #size-cells = <1>; > > > > > > + > > > > > > + sysclk: sysclk { > > > > > > + #clock-cells = <0>; > > > > > > + compatible = "fsl,qoriq-sysclk-1.0", > > > > > > + "fixed-clock"; > > > > > > > > > > We didn't mention in the binding that "fsl,qoriq-sysclk-1.0" was > > > > > compatible with "fixed-clock" and should have "fixed-clock" in > > > > > the compatible list... > > > > > > > > > OK, will fix it. > > > > > > Cheers. Also, doesn't a fixed-clock require a clock-frequency? > > > > > Yes, it should be. But we got the clock-frequency from parent node > > because There is a clock-frequency already there before this patch. > > OK. Should we not place it there for future? We're not compatible with > fixed-clock if we don't fulfil the minimum requirements of the fixed- > clock binding... > > > > > > > > > > > > > + clock-output-names = "sysclk"; > > > > > > + } > > > > > > + > > > > > > + pll0: pll0@800 { > > > > > > + #clock-cells = <1>; > > > > > > + reg = <0x800 0x4>; > > > > > > + compatible = "fsl,qoriq-core-pll-1.0"; > > > > > > + clocks = <&sysclk>; > > > > > > + clock-output-names = "pll0", "pll0- > div2"; > > > > > > + }; > > > > > > + > > > > > > + pll1: pll1@820 { > > > > > > + #clock-cells = <1>; > > > > > > + reg = <0x820 0x4>; > > > > > > + compatible = "fsl,qoriq-core-pll-1.0"; > > > > > > + clocks = <&sysclk>; > > > > > > + clock-output-names = "pll1", "pll1- > div2"; > > > > > > + }; > > > > > > + > > > > > > + mux0: mux0@0 { > > > > > > + #clock-cells = <0>; > > > > > > + reg = <0x0 0x4>; > > > > > > + compatible = "fsl,qoriq-core-mux-1.0"; > > > > > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 > > > > > > + 0>, > > > > > <&pll1 1>; > > > > > > + clock-names = "pll0_0", "pll0_1", > > > > > > + "pll1_0", > > > > > "pll1_1"; > > > > > > + clock-output-names = "cmux0"; > > > > > > + }; > > > > > > + > > > > > > + mux1: mux1@20 { > > > > > > + #clock-cells = <0>; > > > > > > + reg = <0x20 0x4>; > > > > > > + compatible = "fsl,qoriq-core-mux-1.0"; > > > > > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 > > > > > > + 0>, > > > > > <&pll1 1>; > > > > > > + clock-names = "pll0_0", "pll0_1", > > > > > > + "pll1_0", > > > > > "pll1_1"; > > > > > > I didn't spot this last time, but the clock-names here seem to be > > > the names of the outputs from the provider, rather than the input > > > names of the consumer. This goes against the intended purpose of > clock-names. > > > > > I am confused here with provider/consumer because some nodes can both > be consumer and provider. > > The clock-names are corresponding to clocks one by one, what's wrong > with it? > > We have clock-names for a consumer's names of its inputs, and clock- > output-names for a provider's names of it's outputs. > > Consider a PLL device which takes a clock input (REFCLK) and outputs a > higher frequency clock (OUTCLK). The PLL's specification only talks in > terms of REFCLK and OUTCLK, but these are not the names of the clocks as > they are output from the original clock, or provided to the final > consumer. > > Consider a sequence of these PLLs plugged together. The OUTCLK of one > feeds into the REFCLK of the next: > > fixed: fixed-clock { > compatible = "fixed-clock"; > clock-frequency = <50>; > }; > > pll_0: pll { > compatible = "vendor,pll"; > clocks = <&fixed>; > clock-names = "refclk"; > clock-output-names = "outclk"; > }; > > pll_1: another-pll { > compatible = "vendor,pll"; > clocks = <&pll_0>; > clock-names = "refclk"; > clock-output-names = "outclk"; > }; > > consumer { > compatible = "vendor,clock-consumer"; > clocks = <&pll_1>, > <&pll_0>; > clock-names = "halfclk", "fullclk"; }; > > In each case, clock-names describes the clock inputs from the view of the > PLL they are input to, not from the provider they came from, or the > consumer some outputs are going to. Giving the inputs names in this way > makes it possible to describe situations where onlya subset of clock > inputs are wired up -- we could just have "fullclk" on the final consumer, > with only pll_0 as an input, and the driver could figure out what to do. > > Does that help? > Yes, it is very helpful. I will name clock-names better. Regards, Yuantian
On Wed, 9 Oct 2013 14:38:24 +0800, <Yuantian.Tang@freescale.com> wrote: > From: Tang Yuantian <yuantian.tang@freescale.com> > > The following SoCs will be affected: p2041, p3041, p4080, > p5020, p5040, b4420, b4860, t4240 > > Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com> > Signed-off-by: Li Yang <leoli@freescale.com> > --- > v5: > - refine the binding document > - update the compatible string > v4: > - add binding document > - update compatible string > - update the reg property > v3: > - fix typo > v2: > - add t4240, b4420, b4860 support > - remove pll/4 clock from p2041, p3041 and p5020 board > > .../devicetree/bindings/clock/corenet-clock.txt | 111 ++++++++++++++++++++ > arch/powerpc/boot/dts/fsl/b4420si-post.dtsi | 35 +++++++ > arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi | 2 + > arch/powerpc/boot/dts/fsl/b4860si-post.dtsi | 35 +++++++ > arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi | 4 + > arch/powerpc/boot/dts/fsl/p2041si-post.dtsi | 60 +++++++++++ > arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi | 4 + > arch/powerpc/boot/dts/fsl/p3041si-post.dtsi | 60 +++++++++++ > arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi | 4 + > arch/powerpc/boot/dts/fsl/p4080si-post.dtsi | 112 +++++++++++++++++++++ > arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi | 8 ++ > arch/powerpc/boot/dts/fsl/p5020si-post.dtsi | 42 ++++++++ > arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi | 2 + > arch/powerpc/boot/dts/fsl/p5040si-post.dtsi | 60 +++++++++++ > arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi | 4 + > arch/powerpc/boot/dts/fsl/t4240si-post.dtsi | 85 ++++++++++++++++ > arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi | 12 +++ > 17 files changed, 640 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/corenet-clock.txt > > diff --git a/Documentation/devicetree/bindings/clock/corenet-clock.txt b/Documentation/devicetree/bindings/clock/corenet-clock.txt > new file mode 100644 > index 0000000..8efc62d > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/corenet-clock.txt > @@ -0,0 +1,111 @@ > +* Clock Block on Freescale CoreNet Platforms > + > +Freescale CoreNet chips take primary clocking input from the external > +SYSCLK signal. The SYSCLK input (frequency) is multiplied using > +multiple phase locked loops (PLL) to create a variety of frequencies > +which can then be passed to a variety of internal logic, including > +cores and peripheral IP blocks. > +Please refer to the Reference Manual for details. > + > +1. Clock Block Binding > + > +Required properties: > +- compatible: Should include one or more of the following: > + - "fsl,<chip>-clockgen": for chip specific clock block > + - "fsl,qoriq-clockgen-[1,2].x": for chassis 1.x and 2.x clock > +- reg: Offset and length of the clock register set > +- clock-frequency: Indicates input clock frequency of clock block. > + Will be set by u-boot > + > +Recommended properties: > +- #ddress-cells: Specifies the number of cells used to represent typo > + physical base addresses. Must be present if the device has > + sub-nodes and set to 1 if present > +- #size-cells: Specifies the number of cells used to represent > + the size of an address. Must be present if the device has > + sub-nodes and set to 1 if present > + > +2. Clock Provider/Consumer Binding > + > +Most of the binding are from the common clock binding[1]. > + [1] Documentation/devicetree/bindings/clock/clock-bindings.txt > + > +Required properties: > +- compatible : Should include one or more of the following: > + - "fsl,qoriq-core-pll-[1,2].x": Indicates a core PLL clock device > + - "fsl,qoriq-core-mux-[1,2].x": Indicates a core multiplexer clock > + device; divided from the core PLL clock > + - "fixed-clock": From common clock binding; indicates output clock > + of oscillator > + - "fsl,qoriq-sysclk-[1,2].x": Indicates input system clock > +- #clock-cells: From common clock binding; indicates the number of > + output clock. 0 is for one output clock; 1 for more than one clock > + > +Recommended properties: > +- clocks: Should be the phandle of input parent clock > +- clock-names: From common clock binding, indicates the clock name > +- clock-output-names: From common clock binding, indicates the names of > + output clocks > +- reg: Should be the offset and length of clock block base address. > + The length should be 4. Binding looks reasonable to me. g. > + > +Example for clock block and clock provider: > +/ { > + clockgen: global-utilities@e1000 { > + compatible = "fsl,p5020-clockgen", "fsl,qoriq-clockgen-1.0"; > + reg = <0xe1000 0x1000>; > + clock-frequency = <0>; > + #address-cells = <1>; > + #size-cells = <1>; > + > + sysclk: sysclk { > + #clock-cells = <0>; > + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; > + clock-output-names = "sysclk"; > + } > + > + pll0: pll0@800 { > + #clock-cells = <1>; > + reg = <0x800 0x4>; > + compatible = "fsl,qoriq-core-pll-1.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll0", "pll0-div2"; > + }; > + > + pll1: pll1@820 { > + #clock-cells = <1>; > + reg = <0x820 0x4>; > + compatible = "fsl,qoriq-core-pll-1.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll1", "pll1-div2"; > + }; > + > + mux0: mux0@0 { > + #clock-cells = <0>; > + reg = <0x0 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux0"; > + }; > + > + mux1: mux1@20 { > + #clock-cells = <0>; > + reg = <0x20 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux1"; > + }; > + }; > + } > + > +Example for clock consumer: > + > +/ { > + cpu0: PowerPC,e5500@0 { > + ... > + clocks = <&mux0>; > + ... > + }; > + } > diff --git a/arch/powerpc/boot/dts/fsl/b4420si-post.dtsi b/arch/powerpc/boot/dts/fsl/b4420si-post.dtsi > index 5a6615d..e910e82 100644 > --- a/arch/powerpc/boot/dts/fsl/b4420si-post.dtsi > +++ b/arch/powerpc/boot/dts/fsl/b4420si-post.dtsi > @@ -86,6 +86,41 @@ > > clockgen: global-utilities@e1000 { > compatible = "fsl,b4420-clockgen", "fsl,qoriq-clockgen-2.0"; > + #address-cells = <1>; > + #size-cells = <1>; > + > + sysclk: sysclk { > + #clock-cells = <0>; > + compatible = "fsl,qoriq-sysclk-2.0", "fixed-clock"; > + clock-output-names = "sysclk"; > + } > + > + pll0: pll0@800 { > + #clock-cells = <1>; > + reg = <0x800 0x4>; > + compatible = "fsl,qoriq-core-pll-2.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll0", "pll0-div2", "pll0-div4"; > + }; > + > + pll1: pll1@820 { > + #clock-cells = <1>; > + reg = <0x820 0x4>; > + compatible = "fsl,qoriq-core-pll-2.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll1", "pll1-div2", "pll1-div4"; > + }; > + > + mux0: mux0@0 { > + #clock-cells = <0>; > + reg = <0x0 0x4>; > + compatible = "fsl,qoriq-core-mux-2.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>, > + <&pll1 0>, <&pll1 1>, <&pll1 2>; > + clock-names = "pll0_0", "pll0_1", "pll0_2", > + "pll1_0", "pll1_1", "pll1_2"; > + clock-output-names = "cmux0"; > + }; > }; > > rcpm: global-utilities@e2000 { > diff --git a/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi b/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi > index 7b4426e..a11126b 100644 > --- a/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi > +++ b/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi > @@ -62,11 +62,13 @@ > cpu0: PowerPC,e6500@0 { > device_type = "cpu"; > reg = <0 1>; > + clocks = <&mux0>; > next-level-cache = <&L2>; > }; > cpu1: PowerPC,e6500@2 { > device_type = "cpu"; > reg = <2 3>; > + clocks = <&mux0>; > next-level-cache = <&L2>; > }; > }; > diff --git a/arch/powerpc/boot/dts/fsl/b4860si-post.dtsi b/arch/powerpc/boot/dts/fsl/b4860si-post.dtsi > index e5cf6c8..5cfcfe4 100644 > --- a/arch/powerpc/boot/dts/fsl/b4860si-post.dtsi > +++ b/arch/powerpc/boot/dts/fsl/b4860si-post.dtsi > @@ -130,6 +130,41 @@ > > clockgen: global-utilities@e1000 { > compatible = "fsl,b4860-clockgen", "fsl,qoriq-clockgen-2.0"; > + #address-cells = <1>; > + #size-cells = <1>; > + > + sysclk: sysclk { > + #clock-cells = <0>; > + compatible = "fsl,qoriq-sysclk-2.0", "fixed-clock"; > + clock-output-names = "sysclk"; > + } > + > + pll0: pll0@800 { > + #clock-cells = <1>; > + reg = <0x800 0x4>; > + compatible = "fsl,qoriq-core-pll-2.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll0", "pll0-div2", "pll0-div4"; > + }; > + > + pll1: pll1@820 { > + #clock-cells = <1>; > + reg = <0x820 0x4>; > + compatible = "fsl,qoriq-core-pll-2.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll1", "pll1-div2", "pll1-div4"; > + }; > + > + mux0: mux0@0 { > + #clock-cells = <0>; > + reg = <0x0 0x4>; > + compatible = "fsl,qoriq-core-mux-2.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>, > + <&pll1 0>, <&pll1 1>, <&pll1 2>; > + clock-names = "pll0_0", "pll0_1", "pll0_2", > + "pll1_0", "pll1_1", "pll1_2"; > + clock-output-names = "cmux0"; > + }; > }; > > rcpm: global-utilities@e2000 { > diff --git a/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi b/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi > index 5263fa4..185a231 100644 > --- a/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi > +++ b/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi > @@ -62,21 +62,25 @@ > cpu0: PowerPC,e6500@0 { > device_type = "cpu"; > reg = <0 1>; > + clocks = <&mux0>; > next-level-cache = <&L2>; > }; > cpu1: PowerPC,e6500@2 { > device_type = "cpu"; > reg = <2 3>; > + clocks = <&mux0>; > next-level-cache = <&L2>; > }; > cpu2: PowerPC,e6500@4 { > device_type = "cpu"; > reg = <4 5>; > + clocks = <&mux0>; > next-level-cache = <&L2>; > }; > cpu3: PowerPC,e6500@6 { > device_type = "cpu"; > reg = <6 7>; > + clocks = <&mux0>; > next-level-cache = <&L2>; > }; > }; > diff --git a/arch/powerpc/boot/dts/fsl/p2041si-post.dtsi b/arch/powerpc/boot/dts/fsl/p2041si-post.dtsi > index dc6cc5a..f3f7f65 100644 > --- a/arch/powerpc/boot/dts/fsl/p2041si-post.dtsi > +++ b/arch/powerpc/boot/dts/fsl/p2041si-post.dtsi > @@ -308,6 +308,66 @@ > compatible = "fsl,p2041-clockgen", "fsl,qoriq-clockgen-1.0"; > reg = <0xe1000 0x1000>; > clock-frequency = <0>; > + #address-cells = <1>; > + #size-cells = <1>; > + > + sysclk: sysclk { > + #clock-cells = <0>; > + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; > + clock-output-names = "sysclk"; > + } > + > + pll0: pll0@800 { > + #clock-cells = <1>; > + reg = <0x800 0x4>; > + compatible = "fsl,qoriq-core-pll-1.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll0", "pll0-div2"; > + }; > + > + pll1: pll1@820 { > + #clock-cells = <1>; > + reg = <0x820 0x4>; > + compatible = "fsl,qoriq-core-pll-1.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll1", "pll1-div2"; > + }; > + > + mux0: mux0@0 { > + #clock-cells = <0>; > + reg = <0x0 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux0"; > + }; > + > + mux1: mux1@20 { > + #clock-cells = <0>; > + reg = <0x20 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux1"; > + }; > + > + mux2: mux2@40 { > + #clock-cells = <0>; > + reg = <0x40 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux2"; > + }; > + > + mux3: mux3@60 { > + #clock-cells = <0>; > + reg = <0x60 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux3"; > + }; > }; > > rcpm: global-utilities@e2000 { > diff --git a/arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi b/arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi > index 7a2697d..22f3b14 100644 > --- a/arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi > +++ b/arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi > @@ -81,6 +81,7 @@ > cpu0: PowerPC,e500mc@0 { > device_type = "cpu"; > reg = <0>; > + clocks = <&mux0>; > next-level-cache = <&L2_0>; > L2_0: l2-cache { > next-level-cache = <&cpc>; > @@ -89,6 +90,7 @@ > cpu1: PowerPC,e500mc@1 { > device_type = "cpu"; > reg = <1>; > + clocks = <&mux1>; > next-level-cache = <&L2_1>; > L2_1: l2-cache { > next-level-cache = <&cpc>; > @@ -97,6 +99,7 @@ > cpu2: PowerPC,e500mc@2 { > device_type = "cpu"; > reg = <2>; > + clocks = <&mux2>; > next-level-cache = <&L2_2>; > L2_2: l2-cache { > next-level-cache = <&cpc>; > @@ -105,6 +108,7 @@ > cpu3: PowerPC,e500mc@3 { > device_type = "cpu"; > reg = <3>; > + clocks = <&mux3>; > next-level-cache = <&L2_3>; > L2_3: l2-cache { > next-level-cache = <&cpc>; > diff --git a/arch/powerpc/boot/dts/fsl/p3041si-post.dtsi b/arch/powerpc/boot/dts/fsl/p3041si-post.dtsi > index 3fa1e22..9bab9c9 100644 > --- a/arch/powerpc/boot/dts/fsl/p3041si-post.dtsi > +++ b/arch/powerpc/boot/dts/fsl/p3041si-post.dtsi > @@ -335,6 +335,66 @@ > compatible = "fsl,p3041-clockgen", "fsl,qoriq-clockgen-1.0"; > reg = <0xe1000 0x1000>; > clock-frequency = <0>; > + #address-cells = <1>; > + #size-cells = <1>; > + > + sysclk: sysclk { > + #clock-cells = <0>; > + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; > + clock-output-names = "sysclk"; > + } > + > + pll0: pll0@800 { > + #clock-cells = <1>; > + reg = <0x800 0x4>; > + compatible = "fsl,qoriq-core-pll-1.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll0", "pll0-div2"; > + }; > + > + pll1: pll1@820 { > + #clock-cells = <1>; > + reg = <0x820 0x4>; > + compatible = "fsl,qoriq-core-pll-1.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll1", "pll1-div2"; > + }; > + > + mux0: mux0@0 { > + #clock-cells = <0>; > + reg = <0x0 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux0"; > + }; > + > + mux1: mux1@20 { > + #clock-cells = <0>; > + reg = <0x20 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux1"; > + }; > + > + mux2: mux2@40 { > + #clock-cells = <0>; > + reg = <0x40 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux2"; > + }; > + > + mux3: mux3@60 { > + #clock-cells = <0>; > + reg = <0x60 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux3"; > + }; > }; > > rcpm: global-utilities@e2000 { > diff --git a/arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi b/arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi > index c9ca2c3..468e8be 100644 > --- a/arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi > +++ b/arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi > @@ -82,6 +82,7 @@ > cpu0: PowerPC,e500mc@0 { > device_type = "cpu"; > reg = <0>; > + clocks = <&mux0>; > next-level-cache = <&L2_0>; > L2_0: l2-cache { > next-level-cache = <&cpc>; > @@ -90,6 +91,7 @@ > cpu1: PowerPC,e500mc@1 { > device_type = "cpu"; > reg = <1>; > + clocks = <&mux1>; > next-level-cache = <&L2_1>; > L2_1: l2-cache { > next-level-cache = <&cpc>; > @@ -98,6 +100,7 @@ > cpu2: PowerPC,e500mc@2 { > device_type = "cpu"; > reg = <2>; > + clocks = <&mux2>; > next-level-cache = <&L2_2>; > L2_2: l2-cache { > next-level-cache = <&cpc>; > @@ -106,6 +109,7 @@ > cpu3: PowerPC,e500mc@3 { > device_type = "cpu"; > reg = <3>; > + clocks = <&mux3>; > next-level-cache = <&L2_3>; > L2_3: l2-cache { > next-level-cache = <&cpc>; > diff --git a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi > index 34769a7..2108269 100644 > --- a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi > +++ b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi > @@ -355,6 +355,118 @@ > compatible = "fsl,p4080-clockgen", "fsl,qoriq-clockgen-1.0"; > reg = <0xe1000 0x1000>; > clock-frequency = <0>; > + #address-cells = <1>; > + #size-cells = <1>; > + > + sysclk: sysclk { > + #clock-cells = <0>; > + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; > + clock-output-names = "sysclk"; > + } > + > + pll0: pll0@800 { > + #clock-cells = <1>; > + reg = <0x800 0x4>; > + compatible = "fsl,qoriq-core-pll-1.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll0", "pll0-div2"; > + }; > + > + pll1: pll1@820 { > + #clock-cells = <1>; > + reg = <0x820 0x4>; > + compatible = "fsl,qoriq-core-pll-1.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll1", "pll1-div2"; > + }; > + > + pll2: pll2@840 { > + #clock-cells = <1>; > + reg = <0x840 0x4>; > + compatible = "fsl,qoriq-core-pll-1.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll2", "pll2-div2"; > + }; > + > + pll3: pll3@860 { > + #clock-cells = <1>; > + reg = <0x860 0x4>; > + compatible = "fsl,qoriq-core-pll-1.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll3", "pll3-div2"; > + }; > + > + mux0: mux0@0 { > + #clock-cells = <0>; > + reg = <0x0 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux0"; > + }; > + > + mux1: mux1@20 { > + #clock-cells = <0>; > + reg = <0x20 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux1"; > + }; > + > + mux2: mux2@40 { > + #clock-cells = <0>; > + reg = <0x40 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux2"; > + }; > + > + mux3: mux3@60 { > + #clock-cells = <0>; > + reg = <0x60 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux3"; > + }; > + > + mux4: mux4@80 { > + #clock-cells = <0>; > + reg = <0x80 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll2 0>, <&pll2 1>, <&pll3 0>, <&pll3 1>; > + clock-names = "pll2_0", "pll2_1", "pll3_0", "pll3_1"; > + clock-output-names = "cmux4"; > + }; > + > + mux5: mux5@a0 { > + #clock-cells = <0>; > + reg = <0xa0 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll2 0>, <&pll2 1>, <&pll3 0>, <&pll3 1>; > + clock-names = "pll2_0", "pll2_1", "pll3_0", "pll3_1"; > + clock-output-names = "cmux5"; > + }; > + > + mux6: mux6@c0 { > + #clock-cells = <0>; > + reg = <0xc0 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll2 0>, <&pll2 1>, <&pll3 0>, <&pll3 1>; > + clock-names = "pll2_0", "pll2_1", "pll3_0", "pll3_1"; > + clock-output-names = "cmux6"; > + }; > + > + mux7: mux7@e0 { > + #clock-cells = <0>; > + reg = <0xe0 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll2 0>, <&pll2 1>, <&pll3 0>, <&pll3 1>; > + clock-names = "pll2_0", "pll2_1", "pll3_0", "pll3_1"; > + clock-output-names = "cmux7"; > + }; > }; > > rcpm: global-utilities@e2000 { > diff --git a/arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi b/arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi > index 493d9a0..0040b5a 100644 > --- a/arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi > +++ b/arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi > @@ -81,6 +81,7 @@ > cpu0: PowerPC,e500mc@0 { > device_type = "cpu"; > reg = <0>; > + clocks = <&mux0>; > next-level-cache = <&L2_0>; > L2_0: l2-cache { > next-level-cache = <&cpc>; > @@ -89,6 +90,7 @@ > cpu1: PowerPC,e500mc@1 { > device_type = "cpu"; > reg = <1>; > + clocks = <&mux1>; > next-level-cache = <&L2_1>; > L2_1: l2-cache { > next-level-cache = <&cpc>; > @@ -97,6 +99,7 @@ > cpu2: PowerPC,e500mc@2 { > device_type = "cpu"; > reg = <2>; > + clocks = <&mux2>; > next-level-cache = <&L2_2>; > L2_2: l2-cache { > next-level-cache = <&cpc>; > @@ -105,6 +108,7 @@ > cpu3: PowerPC,e500mc@3 { > device_type = "cpu"; > reg = <3>; > + clocks = <&mux3>; > next-level-cache = <&L2_3>; > L2_3: l2-cache { > next-level-cache = <&cpc>; > @@ -113,6 +117,7 @@ > cpu4: PowerPC,e500mc@4 { > device_type = "cpu"; > reg = <4>; > + clocks = <&mux4>; > next-level-cache = <&L2_4>; > L2_4: l2-cache { > next-level-cache = <&cpc>; > @@ -121,6 +126,7 @@ > cpu5: PowerPC,e500mc@5 { > device_type = "cpu"; > reg = <5>; > + clocks = <&mux5>; > next-level-cache = <&L2_5>; > L2_5: l2-cache { > next-level-cache = <&cpc>; > @@ -129,6 +135,7 @@ > cpu6: PowerPC,e500mc@6 { > device_type = "cpu"; > reg = <6>; > + clocks = <&mux6>; > next-level-cache = <&L2_6>; > L2_6: l2-cache { > next-level-cache = <&cpc>; > @@ -137,6 +144,7 @@ > cpu7: PowerPC,e500mc@7 { > device_type = "cpu"; > reg = <7>; > + clocks = <&mux7>; > next-level-cache = <&L2_7>; > L2_7: l2-cache { > next-level-cache = <&cpc>; > diff --git a/arch/powerpc/boot/dts/fsl/p5020si-post.dtsi b/arch/powerpc/boot/dts/fsl/p5020si-post.dtsi > index bc3ae5a..e09f8cd 100644 > --- a/arch/powerpc/boot/dts/fsl/p5020si-post.dtsi > +++ b/arch/powerpc/boot/dts/fsl/p5020si-post.dtsi > @@ -340,6 +340,48 @@ > compatible = "fsl,p5020-clockgen", "fsl,qoriq-clockgen-1.0"; > reg = <0xe1000 0x1000>; > clock-frequency = <0>; > + #address-cells = <1>; > + #size-cells = <1>; > + > + sysclk: sysclk { > + #clock-cells = <0>; > + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; > + clock-output-names = "sysclk"; > + } > + > + pll0: pll0@800 { > + #clock-cells = <1>; > + reg = <0x800 0x4>; > + compatible = "fsl,qoriq-core-pll-1.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll0", "pll0-div2"; > + }; > + > + pll1: pll1@820 { > + #clock-cells = <1>; > + reg = <0x820 0x4>; > + compatible = "fsl,qoriq-core-pll-1.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll1", "pll1-div2"; > + }; > + > + mux0: mux0@0 { > + #clock-cells = <0>; > + reg = <0x0 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux0"; > + }; > + > + mux1: mux1@20 { > + #clock-cells = <0>; > + reg = <0x20 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux1"; > + }; > }; > > rcpm: global-utilities@e2000 { > diff --git a/arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi b/arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi > index 8df47fc..fe1a2e6 100644 > --- a/arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi > +++ b/arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi > @@ -88,6 +88,7 @@ > cpu0: PowerPC,e5500@0 { > device_type = "cpu"; > reg = <0>; > + clocks = <&mux0>; > next-level-cache = <&L2_0>; > L2_0: l2-cache { > next-level-cache = <&cpc>; > @@ -96,6 +97,7 @@ > cpu1: PowerPC,e5500@1 { > device_type = "cpu"; > reg = <1>; > + clocks = <&mux1>; > next-level-cache = <&L2_1>; > L2_1: l2-cache { > next-level-cache = <&cpc>; > diff --git a/arch/powerpc/boot/dts/fsl/p5040si-post.dtsi b/arch/powerpc/boot/dts/fsl/p5040si-post.dtsi > index a91897f..109f132 100644 > --- a/arch/powerpc/boot/dts/fsl/p5040si-post.dtsi > +++ b/arch/powerpc/boot/dts/fsl/p5040si-post.dtsi > @@ -300,6 +300,66 @@ > compatible = "fsl,p5040-clockgen", "fsl,qoriq-clockgen-1.0"; > reg = <0xe1000 0x1000>; > clock-frequency = <0>; > + #address-cells = <1>; > + #size-cells = <1>; > + > + sysclk: sysclk { > + #clock-cells = <0>; > + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; > + clock-output-names = "sysclk"; > + } > + > + pll0: pll0@800 { > + #clock-cells = <1>; > + reg = <0x800 0x4>; > + compatible = "fsl,qoriq-core-pll-1.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll0", "pll0-div2"; > + }; > + > + pll1: pll1@820 { > + #clock-cells = <1>; > + reg = <0x820 0x4>; > + compatible = "fsl,qoriq-core-pll-1.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll1", "pll1-div2"; > + }; > + > + mux0: mux0@0 { > + #clock-cells = <0>; > + reg = <0x0 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux0"; > + }; > + > + mux1: mux1@20 { > + #clock-cells = <0>; > + reg = <0x20 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux1"; > + }; > + > + mux2: mux2@40 { > + #clock-cells = <0>; > + reg = <0x40 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux2"; > + }; > + > + mux3: mux3@60 { > + #clock-cells = <0>; > + reg = <0x60 0x4>; > + compatible = "fsl,qoriq-core-mux-1.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > + clock-output-names = "cmux3"; > + }; > }; > > rcpm: global-utilities@e2000 { > diff --git a/arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi b/arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi > index 40ca943..3674686 100644 > --- a/arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi > +++ b/arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi > @@ -81,6 +81,7 @@ > cpu0: PowerPC,e5500@0 { > device_type = "cpu"; > reg = <0>; > + clocks = <&mux0>; > next-level-cache = <&L2_0>; > L2_0: l2-cache { > next-level-cache = <&cpc>; > @@ -89,6 +90,7 @@ > cpu1: PowerPC,e5500@1 { > device_type = "cpu"; > reg = <1>; > + clocks = <&mux1>; > next-level-cache = <&L2_1>; > L2_1: l2-cache { > next-level-cache = <&cpc>; > @@ -97,6 +99,7 @@ > cpu2: PowerPC,e5500@2 { > device_type = "cpu"; > reg = <2>; > + clocks = <&mux2>; > next-level-cache = <&L2_2>; > L2_2: l2-cache { > next-level-cache = <&cpc>; > @@ -105,6 +108,7 @@ > cpu3: PowerPC,e5500@3 { > device_type = "cpu"; > reg = <3>; > + clocks = <&mux3>; > next-level-cache = <&L2_3>; > L2_3: l2-cache { > next-level-cache = <&cpc>; > diff --git a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi > index 510afa3..d45434f 100644 > --- a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi > +++ b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi > @@ -370,6 +370,91 @@ > clockgen: global-utilities@e1000 { > compatible = "fsl,t4240-clockgen", "fsl,qoriq-clockgen-2.0"; > reg = <0xe1000 0x1000>; > + #address-cells = <1>; > + #size-cells = <1>; > + > + sysclk: sysclk { > + #clock-cells = <0>; > + compatible = "fsl,qoriq-sysclk-2.0", "fixed-clock"; > + clock-output-names = "sysclk"; > + } > + > + pll0: pll0@800 { > + #clock-cells = <1>; > + reg = <0x800 0x4>; > + compatible = "fsl,qoriq-core-pll-2.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll0", "pll0-div2", "pll0-div4"; > + }; > + > + pll1: pll1@820 { > + #clock-cells = <1>; > + reg = <0x820 0x4>; > + compatible = "fsl,qoriq-core-pll-2.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll1", "pll1-div2", "pll1-div4"; > + }; > + > + pll2: pll2@840 { > + #clock-cells = <1>; > + reg = <0x840 0x4>; > + compatible = "fsl,qoriq-core-pll-2.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll2", "pll2-div2", "pll2-div4"; > + }; > + > + pll3: pll3@860 { > + #clock-cells = <1>; > + reg = <0x860 0x4>; > + compatible = "fsl,qoriq-core-pll-2.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll3", "pll3-div2", "pll3-div4"; > + }; > + > + pll4: pll4@880 { > + #clock-cells = <1>; > + reg = <0x880 0x4>; > + compatible = "fsl,qoriq-core-pll-2.0"; > + clocks = <&sysclk>; > + clock-output-names = "pll4", "pll4-div2", "pll4-div4"; > + }; > + > + mux0: mux0@0 { > + #clock-cells = <0>; > + reg = <0x0 0x4>; > + compatible = "fsl,qoriq-core-mux-2.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>, > + <&pll1 0>, <&pll1 1>, <&pll1 2>, > + <&pll2 0>, <&pll2 1>, <&pll2 2>; > + clock-names = "pll0_0", "pll0_1", "pll0_2", > + "pll1_0", "pll1_1", "pll1_2", > + "pll2_0", "pll2_1", "pll2_2"; > + clock-output-names = "cmux0"; > + }; > + > + mux1: mux1@20 { > + #clock-cells = <0>; > + reg = <0x20 0x4>; > + compatible = "fsl,qoriq-core-mux-2.0"; > + clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>, > + <&pll1 0>, <&pll1 1>, <&pll1 2>, > + <&pll2 0>, <&pll2 1>, <&pll2 2>; > + clock-names = "pll0_0", "pll0_1", "pll0_2", > + "pll1_0", "pll1_1", "pll1_2", > + "pll2_0", "pll2_1", "pll2_2"; > + clock-output-names = "cmux1"; > + }; > + > + 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_0", "pll3_1", "pll3_2", > + "pll4_0", "pll4_1", "pll4_2"; > + clock-output-names = "cmux2"; > + }; > }; > > rcpm: global-utilities@e2000 { > diff --git a/arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi b/arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi > index a93c55a..0b8ccc5 100644 > --- a/arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi > +++ b/arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi > @@ -67,61 +67,73 @@ > cpu0: PowerPC,e6500@0 { > device_type = "cpu"; > reg = <0 1>; > + clocks = <&mux0>; > next-level-cache = <&L2_1>; > }; > cpu1: PowerPC,e6500@2 { > device_type = "cpu"; > reg = <2 3>; > + clocks = <&mux0>; > next-level-cache = <&L2_1>; > }; > cpu2: PowerPC,e6500@4 { > device_type = "cpu"; > reg = <4 5>; > + clocks = <&mux0>; > next-level-cache = <&L2_1>; > }; > cpu3: PowerPC,e6500@6 { > device_type = "cpu"; > reg = <6 7>; > + clocks = <&mux0>; > next-level-cache = <&L2_1>; > }; > cpu4: PowerPC,e6500@8 { > device_type = "cpu"; > reg = <8 9>; > + clocks = <&mux1>; > next-level-cache = <&L2_2>; > }; > cpu5: PowerPC,e6500@10 { > device_type = "cpu"; > reg = <10 11>; > + clocks = <&mux1>; > next-level-cache = <&L2_2>; > }; > cpu6: PowerPC,e6500@12 { > device_type = "cpu"; > reg = <12 13>; > + clocks = <&mux1>; > next-level-cache = <&L2_2>; > }; > cpu7: PowerPC,e6500@14 { > device_type = "cpu"; > reg = <14 15>; > + clocks = <&mux1>; > next-level-cache = <&L2_2>; > }; > cpu8: PowerPC,e6500@16 { > device_type = "cpu"; > reg = <16 17>; > + clocks = <&mux2>; > next-level-cache = <&L2_3>; > }; > cpu9: PowerPC,e6500@18 { > device_type = "cpu"; > reg = <18 19>; > + clocks = <&mux2>; > next-level-cache = <&L2_3>; > }; > cpu10: PowerPC,e6500@20 { > device_type = "cpu"; > reg = <20 21>; > + clocks = <&mux2>; > next-level-cache = <&L2_3>; > }; > cpu11: PowerPC,e6500@22 { > device_type = "cpu"; > reg = <22 23>; > + clocks = <&mux2>; > next-level-cache = <&L2_3>; > }; > }; > -- > 1.8.0 > > > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev
> > +1. Clock Block Binding > > + > > +Required properties: > > +- compatible: Should include one or more of the following: > > + - "fsl,<chip>-clockgen": for chip specific clock block > > + - "fsl,qoriq-clockgen-[1,2].x": for chassis 1.x and 2.x clock > > +- reg: Offset and length of the clock register set > > +- clock-frequency: Indicates input clock frequency of clock block. > > + Will be set by u-boot > > + > > +Recommended properties: > > +- #ddress-cells: Specifies the number of cells used to represent > > typo Thanks, someone else already pointed out it. Will fix in next patch. Regards, Yuantian > > > + physical base addresses. Must be present if the device has > > + sub-nodes and set to 1 if present > > +- #size-cells: Specifies the number of cells used to represent > > + the size of an address. Must be present if the device has > > + sub-nodes and set to 1 if present > > + > > +2. Clock Provider/Consumer Binding > > + > > +Most of the binding are from the common clock binding[1]. > > + [1] Documentation/devicetree/bindings/clock/clock-bindings.txt > > + > > +Required properties: > > +- compatible : Should include one or more of the following: > > + - "fsl,qoriq-core-pll-[1,2].x": Indicates a core PLL clock device > > + - "fsl,qoriq-core-mux-[1,2].x": Indicates a core multiplexer clock > > + device; divided from the core PLL clock > > + - "fixed-clock": From common clock binding; indicates output clock > > + of oscillator > > + - "fsl,qoriq-sysclk-[1,2].x": Indicates input system clock > > +- #clock-cells: From common clock binding; indicates the number of > > + output clock. 0 is for one output clock; 1 for more than one clock > > + > > +Recommended properties: > > +- clocks: Should be the phandle of input parent clock > > +- clock-names: From common clock binding, indicates the clock name > > +- clock-output-names: From common clock binding, indicates the names > of > > + output clocks > > +- reg: Should be the offset and length of clock block base address. > > + The length should be 4. > > Binding looks reasonable to me. > > g. > > > + > > +Example for clock block and clock provider: > > +/ { > > + clockgen: global-utilities@e1000 { > > + compatible = "fsl,p5020-clockgen", "fsl,qoriq-clockgen-1.0"; > > + reg = <0xe1000 0x1000>; > > + clock-frequency = <0>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + sysclk: sysclk { > > + #clock-cells = <0>; > > + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; > > + clock-output-names = "sysclk"; > > + } > > + > > + pll0: pll0@800 { > > + #clock-cells = <1>; > > + reg = <0x800 0x4>; > > + compatible = "fsl,qoriq-core-pll-1.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll0", "pll0-div2"; > > + }; > > + > > + pll1: pll1@820 { > > + #clock-cells = <1>; > > + reg = <0x820 0x4>; > > + compatible = "fsl,qoriq-core-pll-1.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll1", "pll1-div2"; > > + }; > > + > > + mux0: mux0@0 { > > + #clock-cells = <0>; > > + reg = <0x0 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux0"; > > + }; > > + > > + mux1: mux1@20 { > > + #clock-cells = <0>; > > + reg = <0x20 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux1"; > > + }; > > + }; > > + } > > + > > +Example for clock consumer: > > + > > +/ { > > + cpu0: PowerPC,e5500@0 { > > + ... > > + clocks = <&mux0>; > > + ... > > + }; > > + } > > diff --git a/arch/powerpc/boot/dts/fsl/b4420si-post.dtsi > > b/arch/powerpc/boot/dts/fsl/b4420si-post.dtsi > > index 5a6615d..e910e82 100644 > > --- a/arch/powerpc/boot/dts/fsl/b4420si-post.dtsi > > +++ b/arch/powerpc/boot/dts/fsl/b4420si-post.dtsi > > @@ -86,6 +86,41 @@ > > > > clockgen: global-utilities@e1000 { > > compatible = "fsl,b4420-clockgen", "fsl,qoriq-clockgen-2.0"; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + sysclk: sysclk { > > + #clock-cells = <0>; > > + compatible = "fsl,qoriq-sysclk-2.0", "fixed-clock"; > > + clock-output-names = "sysclk"; > > + } > > + > > + pll0: pll0@800 { > > + #clock-cells = <1>; > > + reg = <0x800 0x4>; > > + compatible = "fsl,qoriq-core-pll-2.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll0", "pll0-div2", "pll0-div4"; > > + }; > > + > > + pll1: pll1@820 { > > + #clock-cells = <1>; > > + reg = <0x820 0x4>; > > + compatible = "fsl,qoriq-core-pll-2.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll1", "pll1-div2", "pll1-div4"; > > + }; > > + > > + mux0: mux0@0 { > > + #clock-cells = <0>; > > + reg = <0x0 0x4>; > > + compatible = "fsl,qoriq-core-mux-2.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>, > > + <&pll1 0>, <&pll1 1>, <&pll1 2>; > > + clock-names = "pll0_0", "pll0_1", "pll0_2", > > + "pll1_0", "pll1_1", "pll1_2"; > > + clock-output-names = "cmux0"; > > + }; > > }; > > > > rcpm: global-utilities@e2000 { > > diff --git a/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi > > b/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi > > index 7b4426e..a11126b 100644 > > --- a/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi > > +++ b/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi > > @@ -62,11 +62,13 @@ > > cpu0: PowerPC,e6500@0 { > > device_type = "cpu"; > > reg = <0 1>; > > + clocks = <&mux0>; > > next-level-cache = <&L2>; > > }; > > cpu1: PowerPC,e6500@2 { > > device_type = "cpu"; > > reg = <2 3>; > > + clocks = <&mux0>; > > next-level-cache = <&L2>; > > }; > > }; > > diff --git a/arch/powerpc/boot/dts/fsl/b4860si-post.dtsi > > b/arch/powerpc/boot/dts/fsl/b4860si-post.dtsi > > index e5cf6c8..5cfcfe4 100644 > > --- a/arch/powerpc/boot/dts/fsl/b4860si-post.dtsi > > +++ b/arch/powerpc/boot/dts/fsl/b4860si-post.dtsi > > @@ -130,6 +130,41 @@ > > > > clockgen: global-utilities@e1000 { > > compatible = "fsl,b4860-clockgen", "fsl,qoriq-clockgen-2.0"; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + sysclk: sysclk { > > + #clock-cells = <0>; > > + compatible = "fsl,qoriq-sysclk-2.0", "fixed-clock"; > > + clock-output-names = "sysclk"; > > + } > > + > > + pll0: pll0@800 { > > + #clock-cells = <1>; > > + reg = <0x800 0x4>; > > + compatible = "fsl,qoriq-core-pll-2.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll0", "pll0-div2", "pll0-div4"; > > + }; > > + > > + pll1: pll1@820 { > > + #clock-cells = <1>; > > + reg = <0x820 0x4>; > > + compatible = "fsl,qoriq-core-pll-2.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll1", "pll1-div2", "pll1-div4"; > > + }; > > + > > + mux0: mux0@0 { > > + #clock-cells = <0>; > > + reg = <0x0 0x4>; > > + compatible = "fsl,qoriq-core-mux-2.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>, > > + <&pll1 0>, <&pll1 1>, <&pll1 2>; > > + clock-names = "pll0_0", "pll0_1", "pll0_2", > > + "pll1_0", "pll1_1", "pll1_2"; > > + clock-output-names = "cmux0"; > > + }; > > }; > > > > rcpm: global-utilities@e2000 { > > diff --git a/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi > > b/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi > > index 5263fa4..185a231 100644 > > --- a/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi > > +++ b/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi > > @@ -62,21 +62,25 @@ > > cpu0: PowerPC,e6500@0 { > > device_type = "cpu"; > > reg = <0 1>; > > + clocks = <&mux0>; > > next-level-cache = <&L2>; > > }; > > cpu1: PowerPC,e6500@2 { > > device_type = "cpu"; > > reg = <2 3>; > > + clocks = <&mux0>; > > next-level-cache = <&L2>; > > }; > > cpu2: PowerPC,e6500@4 { > > device_type = "cpu"; > > reg = <4 5>; > > + clocks = <&mux0>; > > next-level-cache = <&L2>; > > }; > > cpu3: PowerPC,e6500@6 { > > device_type = "cpu"; > > reg = <6 7>; > > + clocks = <&mux0>; > > next-level-cache = <&L2>; > > }; > > }; > > diff --git a/arch/powerpc/boot/dts/fsl/p2041si-post.dtsi > > b/arch/powerpc/boot/dts/fsl/p2041si-post.dtsi > > index dc6cc5a..f3f7f65 100644 > > --- a/arch/powerpc/boot/dts/fsl/p2041si-post.dtsi > > +++ b/arch/powerpc/boot/dts/fsl/p2041si-post.dtsi > > @@ -308,6 +308,66 @@ > > compatible = "fsl,p2041-clockgen", "fsl,qoriq-clockgen-1.0"; > > reg = <0xe1000 0x1000>; > > clock-frequency = <0>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + sysclk: sysclk { > > + #clock-cells = <0>; > > + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; > > + clock-output-names = "sysclk"; > > + } > > + > > + pll0: pll0@800 { > > + #clock-cells = <1>; > > + reg = <0x800 0x4>; > > + compatible = "fsl,qoriq-core-pll-1.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll0", "pll0-div2"; > > + }; > > + > > + pll1: pll1@820 { > > + #clock-cells = <1>; > > + reg = <0x820 0x4>; > > + compatible = "fsl,qoriq-core-pll-1.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll1", "pll1-div2"; > > + }; > > + > > + mux0: mux0@0 { > > + #clock-cells = <0>; > > + reg = <0x0 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux0"; > > + }; > > + > > + mux1: mux1@20 { > > + #clock-cells = <0>; > > + reg = <0x20 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux1"; > > + }; > > + > > + mux2: mux2@40 { > > + #clock-cells = <0>; > > + reg = <0x40 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux2"; > > + }; > > + > > + mux3: mux3@60 { > > + #clock-cells = <0>; > > + reg = <0x60 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux3"; > > + }; > > }; > > > > rcpm: global-utilities@e2000 { > > diff --git a/arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi > > b/arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi > > index 7a2697d..22f3b14 100644 > > --- a/arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi > > +++ b/arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi > > @@ -81,6 +81,7 @@ > > cpu0: PowerPC,e500mc@0 { > > device_type = "cpu"; > > reg = <0>; > > + clocks = <&mux0>; > > next-level-cache = <&L2_0>; > > L2_0: l2-cache { > > next-level-cache = <&cpc>; > > @@ -89,6 +90,7 @@ > > cpu1: PowerPC,e500mc@1 { > > device_type = "cpu"; > > reg = <1>; > > + clocks = <&mux1>; > > next-level-cache = <&L2_1>; > > L2_1: l2-cache { > > next-level-cache = <&cpc>; > > @@ -97,6 +99,7 @@ > > cpu2: PowerPC,e500mc@2 { > > device_type = "cpu"; > > reg = <2>; > > + clocks = <&mux2>; > > next-level-cache = <&L2_2>; > > L2_2: l2-cache { > > next-level-cache = <&cpc>; > > @@ -105,6 +108,7 @@ > > cpu3: PowerPC,e500mc@3 { > > device_type = "cpu"; > > reg = <3>; > > + clocks = <&mux3>; > > next-level-cache = <&L2_3>; > > L2_3: l2-cache { > > next-level-cache = <&cpc>; > > diff --git a/arch/powerpc/boot/dts/fsl/p3041si-post.dtsi > > b/arch/powerpc/boot/dts/fsl/p3041si-post.dtsi > > index 3fa1e22..9bab9c9 100644 > > --- a/arch/powerpc/boot/dts/fsl/p3041si-post.dtsi > > +++ b/arch/powerpc/boot/dts/fsl/p3041si-post.dtsi > > @@ -335,6 +335,66 @@ > > compatible = "fsl,p3041-clockgen", "fsl,qoriq-clockgen-1.0"; > > reg = <0xe1000 0x1000>; > > clock-frequency = <0>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + sysclk: sysclk { > > + #clock-cells = <0>; > > + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; > > + clock-output-names = "sysclk"; > > + } > > + > > + pll0: pll0@800 { > > + #clock-cells = <1>; > > + reg = <0x800 0x4>; > > + compatible = "fsl,qoriq-core-pll-1.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll0", "pll0-div2"; > > + }; > > + > > + pll1: pll1@820 { > > + #clock-cells = <1>; > > + reg = <0x820 0x4>; > > + compatible = "fsl,qoriq-core-pll-1.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll1", "pll1-div2"; > > + }; > > + > > + mux0: mux0@0 { > > + #clock-cells = <0>; > > + reg = <0x0 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux0"; > > + }; > > + > > + mux1: mux1@20 { > > + #clock-cells = <0>; > > + reg = <0x20 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux1"; > > + }; > > + > > + mux2: mux2@40 { > > + #clock-cells = <0>; > > + reg = <0x40 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux2"; > > + }; > > + > > + mux3: mux3@60 { > > + #clock-cells = <0>; > > + reg = <0x60 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux3"; > > + }; > > }; > > > > rcpm: global-utilities@e2000 { > > diff --git a/arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi > > b/arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi > > index c9ca2c3..468e8be 100644 > > --- a/arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi > > +++ b/arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi > > @@ -82,6 +82,7 @@ > > cpu0: PowerPC,e500mc@0 { > > device_type = "cpu"; > > reg = <0>; > > + clocks = <&mux0>; > > next-level-cache = <&L2_0>; > > L2_0: l2-cache { > > next-level-cache = <&cpc>; > > @@ -90,6 +91,7 @@ > > cpu1: PowerPC,e500mc@1 { > > device_type = "cpu"; > > reg = <1>; > > + clocks = <&mux1>; > > next-level-cache = <&L2_1>; > > L2_1: l2-cache { > > next-level-cache = <&cpc>; > > @@ -98,6 +100,7 @@ > > cpu2: PowerPC,e500mc@2 { > > device_type = "cpu"; > > reg = <2>; > > + clocks = <&mux2>; > > next-level-cache = <&L2_2>; > > L2_2: l2-cache { > > next-level-cache = <&cpc>; > > @@ -106,6 +109,7 @@ > > cpu3: PowerPC,e500mc@3 { > > device_type = "cpu"; > > reg = <3>; > > + clocks = <&mux3>; > > next-level-cache = <&L2_3>; > > L2_3: l2-cache { > > next-level-cache = <&cpc>; > > diff --git a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi > > b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi > > index 34769a7..2108269 100644 > > --- a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi > > +++ b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi > > @@ -355,6 +355,118 @@ > > compatible = "fsl,p4080-clockgen", "fsl,qoriq-clockgen-1.0"; > > reg = <0xe1000 0x1000>; > > clock-frequency = <0>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + sysclk: sysclk { > > + #clock-cells = <0>; > > + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; > > + clock-output-names = "sysclk"; > > + } > > + > > + pll0: pll0@800 { > > + #clock-cells = <1>; > > + reg = <0x800 0x4>; > > + compatible = "fsl,qoriq-core-pll-1.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll0", "pll0-div2"; > > + }; > > + > > + pll1: pll1@820 { > > + #clock-cells = <1>; > > + reg = <0x820 0x4>; > > + compatible = "fsl,qoriq-core-pll-1.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll1", "pll1-div2"; > > + }; > > + > > + pll2: pll2@840 { > > + #clock-cells = <1>; > > + reg = <0x840 0x4>; > > + compatible = "fsl,qoriq-core-pll-1.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll2", "pll2-div2"; > > + }; > > + > > + pll3: pll3@860 { > > + #clock-cells = <1>; > > + reg = <0x860 0x4>; > > + compatible = "fsl,qoriq-core-pll-1.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll3", "pll3-div2"; > > + }; > > + > > + mux0: mux0@0 { > > + #clock-cells = <0>; > > + reg = <0x0 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux0"; > > + }; > > + > > + mux1: mux1@20 { > > + #clock-cells = <0>; > > + reg = <0x20 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux1"; > > + }; > > + > > + mux2: mux2@40 { > > + #clock-cells = <0>; > > + reg = <0x40 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux2"; > > + }; > > + > > + mux3: mux3@60 { > > + #clock-cells = <0>; > > + reg = <0x60 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux3"; > > + }; > > + > > + mux4: mux4@80 { > > + #clock-cells = <0>; > > + reg = <0x80 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll2 0>, <&pll2 1>, <&pll3 0>, <&pll3 1>; > > + clock-names = "pll2_0", "pll2_1", "pll3_0", "pll3_1"; > > + clock-output-names = "cmux4"; > > + }; > > + > > + mux5: mux5@a0 { > > + #clock-cells = <0>; > > + reg = <0xa0 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll2 0>, <&pll2 1>, <&pll3 0>, <&pll3 1>; > > + clock-names = "pll2_0", "pll2_1", "pll3_0", "pll3_1"; > > + clock-output-names = "cmux5"; > > + }; > > + > > + mux6: mux6@c0 { > > + #clock-cells = <0>; > > + reg = <0xc0 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll2 0>, <&pll2 1>, <&pll3 0>, <&pll3 1>; > > + clock-names = "pll2_0", "pll2_1", "pll3_0", "pll3_1"; > > + clock-output-names = "cmux6"; > > + }; > > + > > + mux7: mux7@e0 { > > + #clock-cells = <0>; > > + reg = <0xe0 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll2 0>, <&pll2 1>, <&pll3 0>, <&pll3 1>; > > + clock-names = "pll2_0", "pll2_1", "pll3_0", "pll3_1"; > > + clock-output-names = "cmux7"; > > + }; > > }; > > > > rcpm: global-utilities@e2000 { > > diff --git a/arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi > > b/arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi > > index 493d9a0..0040b5a 100644 > > --- a/arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi > > +++ b/arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi > > @@ -81,6 +81,7 @@ > > cpu0: PowerPC,e500mc@0 { > > device_type = "cpu"; > > reg = <0>; > > + clocks = <&mux0>; > > next-level-cache = <&L2_0>; > > L2_0: l2-cache { > > next-level-cache = <&cpc>; > > @@ -89,6 +90,7 @@ > > cpu1: PowerPC,e500mc@1 { > > device_type = "cpu"; > > reg = <1>; > > + clocks = <&mux1>; > > next-level-cache = <&L2_1>; > > L2_1: l2-cache { > > next-level-cache = <&cpc>; > > @@ -97,6 +99,7 @@ > > cpu2: PowerPC,e500mc@2 { > > device_type = "cpu"; > > reg = <2>; > > + clocks = <&mux2>; > > next-level-cache = <&L2_2>; > > L2_2: l2-cache { > > next-level-cache = <&cpc>; > > @@ -105,6 +108,7 @@ > > cpu3: PowerPC,e500mc@3 { > > device_type = "cpu"; > > reg = <3>; > > + clocks = <&mux3>; > > next-level-cache = <&L2_3>; > > L2_3: l2-cache { > > next-level-cache = <&cpc>; > > @@ -113,6 +117,7 @@ > > cpu4: PowerPC,e500mc@4 { > > device_type = "cpu"; > > reg = <4>; > > + clocks = <&mux4>; > > next-level-cache = <&L2_4>; > > L2_4: l2-cache { > > next-level-cache = <&cpc>; > > @@ -121,6 +126,7 @@ > > cpu5: PowerPC,e500mc@5 { > > device_type = "cpu"; > > reg = <5>; > > + clocks = <&mux5>; > > next-level-cache = <&L2_5>; > > L2_5: l2-cache { > > next-level-cache = <&cpc>; > > @@ -129,6 +135,7 @@ > > cpu6: PowerPC,e500mc@6 { > > device_type = "cpu"; > > reg = <6>; > > + clocks = <&mux6>; > > next-level-cache = <&L2_6>; > > L2_6: l2-cache { > > next-level-cache = <&cpc>; > > @@ -137,6 +144,7 @@ > > cpu7: PowerPC,e500mc@7 { > > device_type = "cpu"; > > reg = <7>; > > + clocks = <&mux7>; > > next-level-cache = <&L2_7>; > > L2_7: l2-cache { > > next-level-cache = <&cpc>; > > diff --git a/arch/powerpc/boot/dts/fsl/p5020si-post.dtsi > > b/arch/powerpc/boot/dts/fsl/p5020si-post.dtsi > > index bc3ae5a..e09f8cd 100644 > > --- a/arch/powerpc/boot/dts/fsl/p5020si-post.dtsi > > +++ b/arch/powerpc/boot/dts/fsl/p5020si-post.dtsi > > @@ -340,6 +340,48 @@ > > compatible = "fsl,p5020-clockgen", "fsl,qoriq-clockgen-1.0"; > > reg = <0xe1000 0x1000>; > > clock-frequency = <0>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + sysclk: sysclk { > > + #clock-cells = <0>; > > + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; > > + clock-output-names = "sysclk"; > > + } > > + > > + pll0: pll0@800 { > > + #clock-cells = <1>; > > + reg = <0x800 0x4>; > > + compatible = "fsl,qoriq-core-pll-1.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll0", "pll0-div2"; > > + }; > > + > > + pll1: pll1@820 { > > + #clock-cells = <1>; > > + reg = <0x820 0x4>; > > + compatible = "fsl,qoriq-core-pll-1.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll1", "pll1-div2"; > > + }; > > + > > + mux0: mux0@0 { > > + #clock-cells = <0>; > > + reg = <0x0 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux0"; > > + }; > > + > > + mux1: mux1@20 { > > + #clock-cells = <0>; > > + reg = <0x20 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux1"; > > + }; > > }; > > > > rcpm: global-utilities@e2000 { > > diff --git a/arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi > > b/arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi > > index 8df47fc..fe1a2e6 100644 > > --- a/arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi > > +++ b/arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi > > @@ -88,6 +88,7 @@ > > cpu0: PowerPC,e5500@0 { > > device_type = "cpu"; > > reg = <0>; > > + clocks = <&mux0>; > > next-level-cache = <&L2_0>; > > L2_0: l2-cache { > > next-level-cache = <&cpc>; > > @@ -96,6 +97,7 @@ > > cpu1: PowerPC,e5500@1 { > > device_type = "cpu"; > > reg = <1>; > > + clocks = <&mux1>; > > next-level-cache = <&L2_1>; > > L2_1: l2-cache { > > next-level-cache = <&cpc>; > > diff --git a/arch/powerpc/boot/dts/fsl/p5040si-post.dtsi > > b/arch/powerpc/boot/dts/fsl/p5040si-post.dtsi > > index a91897f..109f132 100644 > > --- a/arch/powerpc/boot/dts/fsl/p5040si-post.dtsi > > +++ b/arch/powerpc/boot/dts/fsl/p5040si-post.dtsi > > @@ -300,6 +300,66 @@ > > compatible = "fsl,p5040-clockgen", "fsl,qoriq-clockgen-1.0"; > > reg = <0xe1000 0x1000>; > > clock-frequency = <0>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + sysclk: sysclk { > > + #clock-cells = <0>; > > + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; > > + clock-output-names = "sysclk"; > > + } > > + > > + pll0: pll0@800 { > > + #clock-cells = <1>; > > + reg = <0x800 0x4>; > > + compatible = "fsl,qoriq-core-pll-1.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll0", "pll0-div2"; > > + }; > > + > > + pll1: pll1@820 { > > + #clock-cells = <1>; > > + reg = <0x820 0x4>; > > + compatible = "fsl,qoriq-core-pll-1.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll1", "pll1-div2"; > > + }; > > + > > + mux0: mux0@0 { > > + #clock-cells = <0>; > > + reg = <0x0 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux0"; > > + }; > > + > > + mux1: mux1@20 { > > + #clock-cells = <0>; > > + reg = <0x20 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux1"; > > + }; > > + > > + mux2: mux2@40 { > > + #clock-cells = <0>; > > + reg = <0x40 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux2"; > > + }; > > + > > + mux3: mux3@60 { > > + #clock-cells = <0>; > > + reg = <0x60 0x4>; > > + compatible = "fsl,qoriq-core-mux-1.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; > > + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; > > + clock-output-names = "cmux3"; > > + }; > > }; > > > > rcpm: global-utilities@e2000 { > > diff --git a/arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi > > b/arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi > > index 40ca943..3674686 100644 > > --- a/arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi > > +++ b/arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi > > @@ -81,6 +81,7 @@ > > cpu0: PowerPC,e5500@0 { > > device_type = "cpu"; > > reg = <0>; > > + clocks = <&mux0>; > > next-level-cache = <&L2_0>; > > L2_0: l2-cache { > > next-level-cache = <&cpc>; > > @@ -89,6 +90,7 @@ > > cpu1: PowerPC,e5500@1 { > > device_type = "cpu"; > > reg = <1>; > > + clocks = <&mux1>; > > next-level-cache = <&L2_1>; > > L2_1: l2-cache { > > next-level-cache = <&cpc>; > > @@ -97,6 +99,7 @@ > > cpu2: PowerPC,e5500@2 { > > device_type = "cpu"; > > reg = <2>; > > + clocks = <&mux2>; > > next-level-cache = <&L2_2>; > > L2_2: l2-cache { > > next-level-cache = <&cpc>; > > @@ -105,6 +108,7 @@ > > cpu3: PowerPC,e5500@3 { > > device_type = "cpu"; > > reg = <3>; > > + clocks = <&mux3>; > > next-level-cache = <&L2_3>; > > L2_3: l2-cache { > > next-level-cache = <&cpc>; > > diff --git a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi > > b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi > > index 510afa3..d45434f 100644 > > --- a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi > > +++ b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi > > @@ -370,6 +370,91 @@ > > clockgen: global-utilities@e1000 { > > compatible = "fsl,t4240-clockgen", "fsl,qoriq-clockgen-2.0"; > > reg = <0xe1000 0x1000>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + sysclk: sysclk { > > + #clock-cells = <0>; > > + compatible = "fsl,qoriq-sysclk-2.0", "fixed-clock"; > > + clock-output-names = "sysclk"; > > + } > > + > > + pll0: pll0@800 { > > + #clock-cells = <1>; > > + reg = <0x800 0x4>; > > + compatible = "fsl,qoriq-core-pll-2.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll0", "pll0-div2", "pll0-div4"; > > + }; > > + > > + pll1: pll1@820 { > > + #clock-cells = <1>; > > + reg = <0x820 0x4>; > > + compatible = "fsl,qoriq-core-pll-2.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll1", "pll1-div2", "pll1-div4"; > > + }; > > + > > + pll2: pll2@840 { > > + #clock-cells = <1>; > > + reg = <0x840 0x4>; > > + compatible = "fsl,qoriq-core-pll-2.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll2", "pll2-div2", "pll2-div4"; > > + }; > > + > > + pll3: pll3@860 { > > + #clock-cells = <1>; > > + reg = <0x860 0x4>; > > + compatible = "fsl,qoriq-core-pll-2.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll3", "pll3-div2", "pll3-div4"; > > + }; > > + > > + pll4: pll4@880 { > > + #clock-cells = <1>; > > + reg = <0x880 0x4>; > > + compatible = "fsl,qoriq-core-pll-2.0"; > > + clocks = <&sysclk>; > > + clock-output-names = "pll4", "pll4-div2", "pll4-div4"; > > + }; > > + > > + mux0: mux0@0 { > > + #clock-cells = <0>; > > + reg = <0x0 0x4>; > > + compatible = "fsl,qoriq-core-mux-2.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>, > > + <&pll1 0>, <&pll1 1>, <&pll1 2>, > > + <&pll2 0>, <&pll2 1>, <&pll2 2>; > > + clock-names = "pll0_0", "pll0_1", "pll0_2", > > + "pll1_0", "pll1_1", "pll1_2", > > + "pll2_0", "pll2_1", "pll2_2"; > > + clock-output-names = "cmux0"; > > + }; > > + > > + mux1: mux1@20 { > > + #clock-cells = <0>; > > + reg = <0x20 0x4>; > > + compatible = "fsl,qoriq-core-mux-2.0"; > > + clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>, > > + <&pll1 0>, <&pll1 1>, <&pll1 2>, > > + <&pll2 0>, <&pll2 1>, <&pll2 2>; > > + clock-names = "pll0_0", "pll0_1", "pll0_2", > > + "pll1_0", "pll1_1", "pll1_2", > > + "pll2_0", "pll2_1", "pll2_2"; > > + clock-output-names = "cmux1"; > > + }; > > + > > + 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_0", "pll3_1", "pll3_2", > > + "pll4_0", "pll4_1", "pll4_2"; > > + clock-output-names = "cmux2"; > > + }; > > }; > > > > rcpm: global-utilities@e2000 { > > diff --git a/arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi > > b/arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi > > index a93c55a..0b8ccc5 100644 > > --- a/arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi > > +++ b/arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi > > @@ -67,61 +67,73 @@ > > cpu0: PowerPC,e6500@0 { > > device_type = "cpu"; > > reg = <0 1>; > > + clocks = <&mux0>; > > next-level-cache = <&L2_1>; > > }; > > cpu1: PowerPC,e6500@2 { > > device_type = "cpu"; > > reg = <2 3>; > > + clocks = <&mux0>; > > next-level-cache = <&L2_1>; > > }; > > cpu2: PowerPC,e6500@4 { > > device_type = "cpu"; > > reg = <4 5>; > > + clocks = <&mux0>; > > next-level-cache = <&L2_1>; > > }; > > cpu3: PowerPC,e6500@6 { > > device_type = "cpu"; > > reg = <6 7>; > > + clocks = <&mux0>; > > next-level-cache = <&L2_1>; > > }; > > cpu4: PowerPC,e6500@8 { > > device_type = "cpu"; > > reg = <8 9>; > > + clocks = <&mux1>; > > next-level-cache = <&L2_2>; > > }; > > cpu5: PowerPC,e6500@10 { > > device_type = "cpu"; > > reg = <10 11>; > > + clocks = <&mux1>; > > next-level-cache = <&L2_2>; > > }; > > cpu6: PowerPC,e6500@12 { > > device_type = "cpu"; > > reg = <12 13>; > > + clocks = <&mux1>; > > next-level-cache = <&L2_2>; > > }; > > cpu7: PowerPC,e6500@14 { > > device_type = "cpu"; > > reg = <14 15>; > > + clocks = <&mux1>; > > next-level-cache = <&L2_2>; > > }; > > cpu8: PowerPC,e6500@16 { > > device_type = "cpu"; > > reg = <16 17>; > > + clocks = <&mux2>; > > next-level-cache = <&L2_3>; > > }; > > cpu9: PowerPC,e6500@18 { > > device_type = "cpu"; > > reg = <18 19>; > > + clocks = <&mux2>; > > next-level-cache = <&L2_3>; > > }; > > cpu10: PowerPC,e6500@20 { > > device_type = "cpu"; > > reg = <20 21>; > > + clocks = <&mux2>; > > next-level-cache = <&L2_3>; > > }; > > cpu11: PowerPC,e6500@22 { > > device_type = "cpu"; > > reg = <22 23>; > > + clocks = <&mux2>; > > next-level-cache = <&L2_3>; > > }; > > }; > > -- > > 1.8.0 > > > > > > _______________________________________________ > > Linuxppc-dev mailing list > > Linuxppc-dev@lists.ozlabs.org > > https://lists.ozlabs.org/listinfo/linuxppc-dev >
On Sun, 2013-10-20 at 21:55 -0500, Tang Yuantian-B29983 wrote: > I didn't see how your suggestion is a better matching. > > OSC ----> PLL1 ----> mux ----> CPU > | | > |--> PLL2 --| > ........ > As your suggestion, the clock tree looks like the above. > In this case, the MUX driver will not know the divider > details(/2, /4, or /3). When is there ever a /3? > I think the MUX should act like "switch" which choose one > of the input clock as a output clock. It should not CREATE > clock(like PLL1/2, PLL1/4). > The purpose of clock driver is to establish the clock tree. > The clock tree will not be established in your suggestion > because the divider is missing, we don't know where PLL/2 comes from. > > If you really like your proposal, it should be changed to this: > > OSC ------> PLL1 -----> PLL1 /1 ---------> MUX ------->CPU > | |___> PLL1 /2 _______| > | | > |____> PLL2 -----> PLL2 /2 -------| > |___> PLL2/ 4 _______| > > (it is possible that PLLs have different divider). Do we actually have (or expect) a situation where the PLLs have different dividers, or even where the same bit setting in the MUX register means a different divider from one chip to another (within the same MUX compatible string)? If so, then I agree that we should go with your approach. The way Freescale documents things in chip manuals rather than in block manuals, with little bits of information different in each chip manual, makes it hard to figure out this sort of thing. From the examples I looked at, it seemed pretty consistent that the low 2 bits of CLKSEL in the MUX were the log2 of the divider. Are there any chips that don't adhere to this? -Scott
Thanks for your review. > -----Original Message----- > From: Wood Scott-B07421 > Sent: 2013年10月29日 星期二 11:26 > To: Tang Yuantian-B29983 > Cc: Wood Scott-B07421; Mark Rutland; devicetree@vger.kernel.org; > linuxppc-dev@lists.ozlabs.org; Li Yang-Leo-R58472 > Subject: Re: [PATCH v5] powerpc/mpc85xx: Update the clock nodes in device > tree > > On Sun, 2013-10-20 at 21:55 -0500, Tang Yuantian-B29983 wrote: > > I didn't see how your suggestion is a better matching. > > > > OSC ----> PLL1 ----> mux ----> CPU > > | | > > |--> PLL2 --| > > ........ > > As your suggestion, the clock tree looks like the above. > > In this case, the MUX driver will not know the divider details(/2, /4, > > or /3). > > When is there ever a /3? > For T4, there is a /3, but it is used by PME not CPU. > > I think the MUX should act like "switch" which choose one of the input > > clock as a output clock. It should not CREATE clock(like PLL1/2, > > PLL1/4). > > The purpose of clock driver is to establish the clock tree. > > The clock tree will not be established in your suggestion because the > > divider is missing, we don't know where PLL/2 comes from. > > > > If you really like your proposal, it should be changed to this: > > > > OSC ------> PLL1 -----> PLL1 /1 ---------> MUX ------->CPU > > | |___> PLL1 /2 _______| > > | | > > |____> PLL2 -----> PLL2 /2 -------| > > |___> PLL2/ 4 _______| > > > > (it is possible that PLLs have different divider). > > Do we actually have (or expect) a situation where the PLLs have different > dividers, or even where the same bit setting in the MUX register means a > different divider from one chip to another (within the same MUX > compatible string)? If so, then I agree that we should go with your > approach. > For a specific chip, the dividers are same for each PLL. But CPU may use ONLY one of the dividers of a PLL. For example, on p4080, core0-3 may use PLL3/1, but do not use PLL3/2. (I didn't deal with this situation in driver either because there are limitations to use it). > The way Freescale documents things in chip manuals rather than in block > manuals, with little bits of information different in each chip manual, > makes it hard to figure out this sort of thing. From the examples I > looked at, it seemed pretty consistent that the low 2 bits of CLKSEL in > the MUX were the log2 of the divider. Are there any chips that don't > adhere to this? > Your observation is correct until then. But there is no rule on this officially. Now I want to summary the pros and cons of your suggestion: The pros: 1. the device tree would be simpler. The cons: 1. cannot get the whole clock tree picture because dividers are hidden in driver. 2. no way to deal with the use case on p4080 3. need to deal with each <chip>-clockgen compatible string and pass a parameter to tell it what the divider is. 4. the "log2 rule" could be incorrect in future, say, we have a divider of /5. Any thoughts on this? Regards, Yuantian
diff --git a/Documentation/devicetree/bindings/clock/corenet-clock.txt b/Documentation/devicetree/bindings/clock/corenet-clock.txt new file mode 100644 index 0000000..8efc62d --- /dev/null +++ b/Documentation/devicetree/bindings/clock/corenet-clock.txt @@ -0,0 +1,111 @@ +* Clock Block on Freescale CoreNet Platforms + +Freescale CoreNet chips take primary clocking input from the external +SYSCLK signal. The SYSCLK input (frequency) is multiplied using +multiple phase locked loops (PLL) to create a variety of frequencies +which can then be passed to a variety of internal logic, including +cores and peripheral IP blocks. +Please refer to the Reference Manual for details. + +1. Clock Block Binding + +Required properties: +- compatible: Should include one or more of the following: + - "fsl,<chip>-clockgen": for chip specific clock block + - "fsl,qoriq-clockgen-[1,2].x": for chassis 1.x and 2.x clock +- reg: Offset and length of the clock register set +- clock-frequency: Indicates input clock frequency of clock block. + Will be set by u-boot + +Recommended properties: +- #ddress-cells: Specifies the number of cells used to represent + physical base addresses. Must be present if the device has + sub-nodes and set to 1 if present +- #size-cells: Specifies the number of cells used to represent + the size of an address. Must be present if the device has + sub-nodes and set to 1 if present + +2. Clock Provider/Consumer Binding + +Most of the binding are from the common clock binding[1]. + [1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : Should include one or more of the following: + - "fsl,qoriq-core-pll-[1,2].x": Indicates a core PLL clock device + - "fsl,qoriq-core-mux-[1,2].x": Indicates a core multiplexer clock + device; divided from the core PLL clock + - "fixed-clock": From common clock binding; indicates output clock + of oscillator + - "fsl,qoriq-sysclk-[1,2].x": Indicates input system clock +- #clock-cells: From common clock binding; indicates the number of + output clock. 0 is for one output clock; 1 for more than one clock + +Recommended properties: +- clocks: Should be the phandle of input parent clock +- clock-names: From common clock binding, indicates the clock name +- clock-output-names: From common clock binding, indicates the names of + output clocks +- reg: Should be the offset and length of clock block base address. + The length should be 4. + +Example for clock block and clock provider: +/ { + clockgen: global-utilities@e1000 { + compatible = "fsl,p5020-clockgen", "fsl,qoriq-clockgen-1.0"; + reg = <0xe1000 0x1000>; + clock-frequency = <0>; + #address-cells = <1>; + #size-cells = <1>; + + sysclk: sysclk { + #clock-cells = <0>; + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; + clock-output-names = "sysclk"; + } + + pll0: pll0@800 { + #clock-cells = <1>; + reg = <0x800 0x4>; + compatible = "fsl,qoriq-core-pll-1.0"; + clocks = <&sysclk>; + clock-output-names = "pll0", "pll0-div2"; + }; + + pll1: pll1@820 { + #clock-cells = <1>; + reg = <0x820 0x4>; + compatible = "fsl,qoriq-core-pll-1.0"; + clocks = <&sysclk>; + clock-output-names = "pll1", "pll1-div2"; + }; + + mux0: mux0@0 { + #clock-cells = <0>; + reg = <0x0 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux0"; + }; + + mux1: mux1@20 { + #clock-cells = <0>; + reg = <0x20 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux1"; + }; + }; + } + +Example for clock consumer: + +/ { + cpu0: PowerPC,e5500@0 { + ... + clocks = <&mux0>; + ... + }; + } diff --git a/arch/powerpc/boot/dts/fsl/b4420si-post.dtsi b/arch/powerpc/boot/dts/fsl/b4420si-post.dtsi index 5a6615d..e910e82 100644 --- a/arch/powerpc/boot/dts/fsl/b4420si-post.dtsi +++ b/arch/powerpc/boot/dts/fsl/b4420si-post.dtsi @@ -86,6 +86,41 @@ clockgen: global-utilities@e1000 { compatible = "fsl,b4420-clockgen", "fsl,qoriq-clockgen-2.0"; + #address-cells = <1>; + #size-cells = <1>; + + sysclk: sysclk { + #clock-cells = <0>; + compatible = "fsl,qoriq-sysclk-2.0", "fixed-clock"; + clock-output-names = "sysclk"; + } + + pll0: pll0@800 { + #clock-cells = <1>; + reg = <0x800 0x4>; + compatible = "fsl,qoriq-core-pll-2.0"; + clocks = <&sysclk>; + clock-output-names = "pll0", "pll0-div2", "pll0-div4"; + }; + + pll1: pll1@820 { + #clock-cells = <1>; + reg = <0x820 0x4>; + compatible = "fsl,qoriq-core-pll-2.0"; + clocks = <&sysclk>; + clock-output-names = "pll1", "pll1-div2", "pll1-div4"; + }; + + mux0: mux0@0 { + #clock-cells = <0>; + reg = <0x0 0x4>; + compatible = "fsl,qoriq-core-mux-2.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>, + <&pll1 0>, <&pll1 1>, <&pll1 2>; + clock-names = "pll0_0", "pll0_1", "pll0_2", + "pll1_0", "pll1_1", "pll1_2"; + clock-output-names = "cmux0"; + }; }; rcpm: global-utilities@e2000 { diff --git a/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi b/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi index 7b4426e..a11126b 100644 --- a/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi +++ b/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi @@ -62,11 +62,13 @@ cpu0: PowerPC,e6500@0 { device_type = "cpu"; reg = <0 1>; + clocks = <&mux0>; next-level-cache = <&L2>; }; cpu1: PowerPC,e6500@2 { device_type = "cpu"; reg = <2 3>; + clocks = <&mux0>; next-level-cache = <&L2>; }; }; diff --git a/arch/powerpc/boot/dts/fsl/b4860si-post.dtsi b/arch/powerpc/boot/dts/fsl/b4860si-post.dtsi index e5cf6c8..5cfcfe4 100644 --- a/arch/powerpc/boot/dts/fsl/b4860si-post.dtsi +++ b/arch/powerpc/boot/dts/fsl/b4860si-post.dtsi @@ -130,6 +130,41 @@ clockgen: global-utilities@e1000 { compatible = "fsl,b4860-clockgen", "fsl,qoriq-clockgen-2.0"; + #address-cells = <1>; + #size-cells = <1>; + + sysclk: sysclk { + #clock-cells = <0>; + compatible = "fsl,qoriq-sysclk-2.0", "fixed-clock"; + clock-output-names = "sysclk"; + } + + pll0: pll0@800 { + #clock-cells = <1>; + reg = <0x800 0x4>; + compatible = "fsl,qoriq-core-pll-2.0"; + clocks = <&sysclk>; + clock-output-names = "pll0", "pll0-div2", "pll0-div4"; + }; + + pll1: pll1@820 { + #clock-cells = <1>; + reg = <0x820 0x4>; + compatible = "fsl,qoriq-core-pll-2.0"; + clocks = <&sysclk>; + clock-output-names = "pll1", "pll1-div2", "pll1-div4"; + }; + + mux0: mux0@0 { + #clock-cells = <0>; + reg = <0x0 0x4>; + compatible = "fsl,qoriq-core-mux-2.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>, + <&pll1 0>, <&pll1 1>, <&pll1 2>; + clock-names = "pll0_0", "pll0_1", "pll0_2", + "pll1_0", "pll1_1", "pll1_2"; + clock-output-names = "cmux0"; + }; }; rcpm: global-utilities@e2000 { diff --git a/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi b/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi index 5263fa4..185a231 100644 --- a/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi +++ b/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi @@ -62,21 +62,25 @@ cpu0: PowerPC,e6500@0 { device_type = "cpu"; reg = <0 1>; + clocks = <&mux0>; next-level-cache = <&L2>; }; cpu1: PowerPC,e6500@2 { device_type = "cpu"; reg = <2 3>; + clocks = <&mux0>; next-level-cache = <&L2>; }; cpu2: PowerPC,e6500@4 { device_type = "cpu"; reg = <4 5>; + clocks = <&mux0>; next-level-cache = <&L2>; }; cpu3: PowerPC,e6500@6 { device_type = "cpu"; reg = <6 7>; + clocks = <&mux0>; next-level-cache = <&L2>; }; }; diff --git a/arch/powerpc/boot/dts/fsl/p2041si-post.dtsi b/arch/powerpc/boot/dts/fsl/p2041si-post.dtsi index dc6cc5a..f3f7f65 100644 --- a/arch/powerpc/boot/dts/fsl/p2041si-post.dtsi +++ b/arch/powerpc/boot/dts/fsl/p2041si-post.dtsi @@ -308,6 +308,66 @@ compatible = "fsl,p2041-clockgen", "fsl,qoriq-clockgen-1.0"; reg = <0xe1000 0x1000>; clock-frequency = <0>; + #address-cells = <1>; + #size-cells = <1>; + + sysclk: sysclk { + #clock-cells = <0>; + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; + clock-output-names = "sysclk"; + } + + pll0: pll0@800 { + #clock-cells = <1>; + reg = <0x800 0x4>; + compatible = "fsl,qoriq-core-pll-1.0"; + clocks = <&sysclk>; + clock-output-names = "pll0", "pll0-div2"; + }; + + pll1: pll1@820 { + #clock-cells = <1>; + reg = <0x820 0x4>; + compatible = "fsl,qoriq-core-pll-1.0"; + clocks = <&sysclk>; + clock-output-names = "pll1", "pll1-div2"; + }; + + mux0: mux0@0 { + #clock-cells = <0>; + reg = <0x0 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux0"; + }; + + mux1: mux1@20 { + #clock-cells = <0>; + reg = <0x20 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux1"; + }; + + mux2: mux2@40 { + #clock-cells = <0>; + reg = <0x40 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux2"; + }; + + mux3: mux3@60 { + #clock-cells = <0>; + reg = <0x60 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux3"; + }; }; rcpm: global-utilities@e2000 { diff --git a/arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi b/arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi index 7a2697d..22f3b14 100644 --- a/arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi +++ b/arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi @@ -81,6 +81,7 @@ cpu0: PowerPC,e500mc@0 { device_type = "cpu"; reg = <0>; + clocks = <&mux0>; next-level-cache = <&L2_0>; L2_0: l2-cache { next-level-cache = <&cpc>; @@ -89,6 +90,7 @@ cpu1: PowerPC,e500mc@1 { device_type = "cpu"; reg = <1>; + clocks = <&mux1>; next-level-cache = <&L2_1>; L2_1: l2-cache { next-level-cache = <&cpc>; @@ -97,6 +99,7 @@ cpu2: PowerPC,e500mc@2 { device_type = "cpu"; reg = <2>; + clocks = <&mux2>; next-level-cache = <&L2_2>; L2_2: l2-cache { next-level-cache = <&cpc>; @@ -105,6 +108,7 @@ cpu3: PowerPC,e500mc@3 { device_type = "cpu"; reg = <3>; + clocks = <&mux3>; next-level-cache = <&L2_3>; L2_3: l2-cache { next-level-cache = <&cpc>; diff --git a/arch/powerpc/boot/dts/fsl/p3041si-post.dtsi b/arch/powerpc/boot/dts/fsl/p3041si-post.dtsi index 3fa1e22..9bab9c9 100644 --- a/arch/powerpc/boot/dts/fsl/p3041si-post.dtsi +++ b/arch/powerpc/boot/dts/fsl/p3041si-post.dtsi @@ -335,6 +335,66 @@ compatible = "fsl,p3041-clockgen", "fsl,qoriq-clockgen-1.0"; reg = <0xe1000 0x1000>; clock-frequency = <0>; + #address-cells = <1>; + #size-cells = <1>; + + sysclk: sysclk { + #clock-cells = <0>; + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; + clock-output-names = "sysclk"; + } + + pll0: pll0@800 { + #clock-cells = <1>; + reg = <0x800 0x4>; + compatible = "fsl,qoriq-core-pll-1.0"; + clocks = <&sysclk>; + clock-output-names = "pll0", "pll0-div2"; + }; + + pll1: pll1@820 { + #clock-cells = <1>; + reg = <0x820 0x4>; + compatible = "fsl,qoriq-core-pll-1.0"; + clocks = <&sysclk>; + clock-output-names = "pll1", "pll1-div2"; + }; + + mux0: mux0@0 { + #clock-cells = <0>; + reg = <0x0 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux0"; + }; + + mux1: mux1@20 { + #clock-cells = <0>; + reg = <0x20 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux1"; + }; + + mux2: mux2@40 { + #clock-cells = <0>; + reg = <0x40 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux2"; + }; + + mux3: mux3@60 { + #clock-cells = <0>; + reg = <0x60 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux3"; + }; }; rcpm: global-utilities@e2000 { diff --git a/arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi b/arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi index c9ca2c3..468e8be 100644 --- a/arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi +++ b/arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi @@ -82,6 +82,7 @@ cpu0: PowerPC,e500mc@0 { device_type = "cpu"; reg = <0>; + clocks = <&mux0>; next-level-cache = <&L2_0>; L2_0: l2-cache { next-level-cache = <&cpc>; @@ -90,6 +91,7 @@ cpu1: PowerPC,e500mc@1 { device_type = "cpu"; reg = <1>; + clocks = <&mux1>; next-level-cache = <&L2_1>; L2_1: l2-cache { next-level-cache = <&cpc>; @@ -98,6 +100,7 @@ cpu2: PowerPC,e500mc@2 { device_type = "cpu"; reg = <2>; + clocks = <&mux2>; next-level-cache = <&L2_2>; L2_2: l2-cache { next-level-cache = <&cpc>; @@ -106,6 +109,7 @@ cpu3: PowerPC,e500mc@3 { device_type = "cpu"; reg = <3>; + clocks = <&mux3>; next-level-cache = <&L2_3>; L2_3: l2-cache { next-level-cache = <&cpc>; diff --git a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi index 34769a7..2108269 100644 --- a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi +++ b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi @@ -355,6 +355,118 @@ compatible = "fsl,p4080-clockgen", "fsl,qoriq-clockgen-1.0"; reg = <0xe1000 0x1000>; clock-frequency = <0>; + #address-cells = <1>; + #size-cells = <1>; + + sysclk: sysclk { + #clock-cells = <0>; + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; + clock-output-names = "sysclk"; + } + + pll0: pll0@800 { + #clock-cells = <1>; + reg = <0x800 0x4>; + compatible = "fsl,qoriq-core-pll-1.0"; + clocks = <&sysclk>; + clock-output-names = "pll0", "pll0-div2"; + }; + + pll1: pll1@820 { + #clock-cells = <1>; + reg = <0x820 0x4>; + compatible = "fsl,qoriq-core-pll-1.0"; + clocks = <&sysclk>; + clock-output-names = "pll1", "pll1-div2"; + }; + + pll2: pll2@840 { + #clock-cells = <1>; + reg = <0x840 0x4>; + compatible = "fsl,qoriq-core-pll-1.0"; + clocks = <&sysclk>; + clock-output-names = "pll2", "pll2-div2"; + }; + + pll3: pll3@860 { + #clock-cells = <1>; + reg = <0x860 0x4>; + compatible = "fsl,qoriq-core-pll-1.0"; + clocks = <&sysclk>; + clock-output-names = "pll3", "pll3-div2"; + }; + + mux0: mux0@0 { + #clock-cells = <0>; + reg = <0x0 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux0"; + }; + + mux1: mux1@20 { + #clock-cells = <0>; + reg = <0x20 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux1"; + }; + + mux2: mux2@40 { + #clock-cells = <0>; + reg = <0x40 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux2"; + }; + + mux3: mux3@60 { + #clock-cells = <0>; + reg = <0x60 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux3"; + }; + + mux4: mux4@80 { + #clock-cells = <0>; + reg = <0x80 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll2 0>, <&pll2 1>, <&pll3 0>, <&pll3 1>; + clock-names = "pll2_0", "pll2_1", "pll3_0", "pll3_1"; + clock-output-names = "cmux4"; + }; + + mux5: mux5@a0 { + #clock-cells = <0>; + reg = <0xa0 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll2 0>, <&pll2 1>, <&pll3 0>, <&pll3 1>; + clock-names = "pll2_0", "pll2_1", "pll3_0", "pll3_1"; + clock-output-names = "cmux5"; + }; + + mux6: mux6@c0 { + #clock-cells = <0>; + reg = <0xc0 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll2 0>, <&pll2 1>, <&pll3 0>, <&pll3 1>; + clock-names = "pll2_0", "pll2_1", "pll3_0", "pll3_1"; + clock-output-names = "cmux6"; + }; + + mux7: mux7@e0 { + #clock-cells = <0>; + reg = <0xe0 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll2 0>, <&pll2 1>, <&pll3 0>, <&pll3 1>; + clock-names = "pll2_0", "pll2_1", "pll3_0", "pll3_1"; + clock-output-names = "cmux7"; + }; }; rcpm: global-utilities@e2000 { diff --git a/arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi b/arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi index 493d9a0..0040b5a 100644 --- a/arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi +++ b/arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi @@ -81,6 +81,7 @@ cpu0: PowerPC,e500mc@0 { device_type = "cpu"; reg = <0>; + clocks = <&mux0>; next-level-cache = <&L2_0>; L2_0: l2-cache { next-level-cache = <&cpc>; @@ -89,6 +90,7 @@ cpu1: PowerPC,e500mc@1 { device_type = "cpu"; reg = <1>; + clocks = <&mux1>; next-level-cache = <&L2_1>; L2_1: l2-cache { next-level-cache = <&cpc>; @@ -97,6 +99,7 @@ cpu2: PowerPC,e500mc@2 { device_type = "cpu"; reg = <2>; + clocks = <&mux2>; next-level-cache = <&L2_2>; L2_2: l2-cache { next-level-cache = <&cpc>; @@ -105,6 +108,7 @@ cpu3: PowerPC,e500mc@3 { device_type = "cpu"; reg = <3>; + clocks = <&mux3>; next-level-cache = <&L2_3>; L2_3: l2-cache { next-level-cache = <&cpc>; @@ -113,6 +117,7 @@ cpu4: PowerPC,e500mc@4 { device_type = "cpu"; reg = <4>; + clocks = <&mux4>; next-level-cache = <&L2_4>; L2_4: l2-cache { next-level-cache = <&cpc>; @@ -121,6 +126,7 @@ cpu5: PowerPC,e500mc@5 { device_type = "cpu"; reg = <5>; + clocks = <&mux5>; next-level-cache = <&L2_5>; L2_5: l2-cache { next-level-cache = <&cpc>; @@ -129,6 +135,7 @@ cpu6: PowerPC,e500mc@6 { device_type = "cpu"; reg = <6>; + clocks = <&mux6>; next-level-cache = <&L2_6>; L2_6: l2-cache { next-level-cache = <&cpc>; @@ -137,6 +144,7 @@ cpu7: PowerPC,e500mc@7 { device_type = "cpu"; reg = <7>; + clocks = <&mux7>; next-level-cache = <&L2_7>; L2_7: l2-cache { next-level-cache = <&cpc>; diff --git a/arch/powerpc/boot/dts/fsl/p5020si-post.dtsi b/arch/powerpc/boot/dts/fsl/p5020si-post.dtsi index bc3ae5a..e09f8cd 100644 --- a/arch/powerpc/boot/dts/fsl/p5020si-post.dtsi +++ b/arch/powerpc/boot/dts/fsl/p5020si-post.dtsi @@ -340,6 +340,48 @@ compatible = "fsl,p5020-clockgen", "fsl,qoriq-clockgen-1.0"; reg = <0xe1000 0x1000>; clock-frequency = <0>; + #address-cells = <1>; + #size-cells = <1>; + + sysclk: sysclk { + #clock-cells = <0>; + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; + clock-output-names = "sysclk"; + } + + pll0: pll0@800 { + #clock-cells = <1>; + reg = <0x800 0x4>; + compatible = "fsl,qoriq-core-pll-1.0"; + clocks = <&sysclk>; + clock-output-names = "pll0", "pll0-div2"; + }; + + pll1: pll1@820 { + #clock-cells = <1>; + reg = <0x820 0x4>; + compatible = "fsl,qoriq-core-pll-1.0"; + clocks = <&sysclk>; + clock-output-names = "pll1", "pll1-div2"; + }; + + mux0: mux0@0 { + #clock-cells = <0>; + reg = <0x0 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux0"; + }; + + mux1: mux1@20 { + #clock-cells = <0>; + reg = <0x20 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux1"; + }; }; rcpm: global-utilities@e2000 { diff --git a/arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi b/arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi index 8df47fc..fe1a2e6 100644 --- a/arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi +++ b/arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi @@ -88,6 +88,7 @@ cpu0: PowerPC,e5500@0 { device_type = "cpu"; reg = <0>; + clocks = <&mux0>; next-level-cache = <&L2_0>; L2_0: l2-cache { next-level-cache = <&cpc>; @@ -96,6 +97,7 @@ cpu1: PowerPC,e5500@1 { device_type = "cpu"; reg = <1>; + clocks = <&mux1>; next-level-cache = <&L2_1>; L2_1: l2-cache { next-level-cache = <&cpc>; diff --git a/arch/powerpc/boot/dts/fsl/p5040si-post.dtsi b/arch/powerpc/boot/dts/fsl/p5040si-post.dtsi index a91897f..109f132 100644 --- a/arch/powerpc/boot/dts/fsl/p5040si-post.dtsi +++ b/arch/powerpc/boot/dts/fsl/p5040si-post.dtsi @@ -300,6 +300,66 @@ compatible = "fsl,p5040-clockgen", "fsl,qoriq-clockgen-1.0"; reg = <0xe1000 0x1000>; clock-frequency = <0>; + #address-cells = <1>; + #size-cells = <1>; + + sysclk: sysclk { + #clock-cells = <0>; + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock"; + clock-output-names = "sysclk"; + } + + pll0: pll0@800 { + #clock-cells = <1>; + reg = <0x800 0x4>; + compatible = "fsl,qoriq-core-pll-1.0"; + clocks = <&sysclk>; + clock-output-names = "pll0", "pll0-div2"; + }; + + pll1: pll1@820 { + #clock-cells = <1>; + reg = <0x820 0x4>; + compatible = "fsl,qoriq-core-pll-1.0"; + clocks = <&sysclk>; + clock-output-names = "pll1", "pll1-div2"; + }; + + mux0: mux0@0 { + #clock-cells = <0>; + reg = <0x0 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux0"; + }; + + mux1: mux1@20 { + #clock-cells = <0>; + reg = <0x20 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux1"; + }; + + mux2: mux2@40 { + #clock-cells = <0>; + reg = <0x40 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux2"; + }; + + mux3: mux3@60 { + #clock-cells = <0>; + reg = <0x60 0x4>; + compatible = "fsl,qoriq-core-mux-1.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>; + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1"; + clock-output-names = "cmux3"; + }; }; rcpm: global-utilities@e2000 { diff --git a/arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi b/arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi index 40ca943..3674686 100644 --- a/arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi +++ b/arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi @@ -81,6 +81,7 @@ cpu0: PowerPC,e5500@0 { device_type = "cpu"; reg = <0>; + clocks = <&mux0>; next-level-cache = <&L2_0>; L2_0: l2-cache { next-level-cache = <&cpc>; @@ -89,6 +90,7 @@ cpu1: PowerPC,e5500@1 { device_type = "cpu"; reg = <1>; + clocks = <&mux1>; next-level-cache = <&L2_1>; L2_1: l2-cache { next-level-cache = <&cpc>; @@ -97,6 +99,7 @@ cpu2: PowerPC,e5500@2 { device_type = "cpu"; reg = <2>; + clocks = <&mux2>; next-level-cache = <&L2_2>; L2_2: l2-cache { next-level-cache = <&cpc>; @@ -105,6 +108,7 @@ cpu3: PowerPC,e5500@3 { device_type = "cpu"; reg = <3>; + clocks = <&mux3>; next-level-cache = <&L2_3>; L2_3: l2-cache { next-level-cache = <&cpc>; diff --git a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi index 510afa3..d45434f 100644 --- a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi +++ b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi @@ -370,6 +370,91 @@ clockgen: global-utilities@e1000 { compatible = "fsl,t4240-clockgen", "fsl,qoriq-clockgen-2.0"; reg = <0xe1000 0x1000>; + #address-cells = <1>; + #size-cells = <1>; + + sysclk: sysclk { + #clock-cells = <0>; + compatible = "fsl,qoriq-sysclk-2.0", "fixed-clock"; + clock-output-names = "sysclk"; + } + + pll0: pll0@800 { + #clock-cells = <1>; + reg = <0x800 0x4>; + compatible = "fsl,qoriq-core-pll-2.0"; + clocks = <&sysclk>; + clock-output-names = "pll0", "pll0-div2", "pll0-div4"; + }; + + pll1: pll1@820 { + #clock-cells = <1>; + reg = <0x820 0x4>; + compatible = "fsl,qoriq-core-pll-2.0"; + clocks = <&sysclk>; + clock-output-names = "pll1", "pll1-div2", "pll1-div4"; + }; + + pll2: pll2@840 { + #clock-cells = <1>; + reg = <0x840 0x4>; + compatible = "fsl,qoriq-core-pll-2.0"; + clocks = <&sysclk>; + clock-output-names = "pll2", "pll2-div2", "pll2-div4"; + }; + + pll3: pll3@860 { + #clock-cells = <1>; + reg = <0x860 0x4>; + compatible = "fsl,qoriq-core-pll-2.0"; + clocks = <&sysclk>; + clock-output-names = "pll3", "pll3-div2", "pll3-div4"; + }; + + pll4: pll4@880 { + #clock-cells = <1>; + reg = <0x880 0x4>; + compatible = "fsl,qoriq-core-pll-2.0"; + clocks = <&sysclk>; + clock-output-names = "pll4", "pll4-div2", "pll4-div4"; + }; + + mux0: mux0@0 { + #clock-cells = <0>; + reg = <0x0 0x4>; + compatible = "fsl,qoriq-core-mux-2.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>, + <&pll1 0>, <&pll1 1>, <&pll1 2>, + <&pll2 0>, <&pll2 1>, <&pll2 2>; + clock-names = "pll0_0", "pll0_1", "pll0_2", + "pll1_0", "pll1_1", "pll1_2", + "pll2_0", "pll2_1", "pll2_2"; + clock-output-names = "cmux0"; + }; + + mux1: mux1@20 { + #clock-cells = <0>; + reg = <0x20 0x4>; + compatible = "fsl,qoriq-core-mux-2.0"; + clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>, + <&pll1 0>, <&pll1 1>, <&pll1 2>, + <&pll2 0>, <&pll2 1>, <&pll2 2>; + clock-names = "pll0_0", "pll0_1", "pll0_2", + "pll1_0", "pll1_1", "pll1_2", + "pll2_0", "pll2_1", "pll2_2"; + clock-output-names = "cmux1"; + }; + + 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_0", "pll3_1", "pll3_2", + "pll4_0", "pll4_1", "pll4_2"; + clock-output-names = "cmux2"; + }; }; rcpm: global-utilities@e2000 { diff --git a/arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi b/arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi index a93c55a..0b8ccc5 100644 --- a/arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi +++ b/arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi @@ -67,61 +67,73 @@ cpu0: PowerPC,e6500@0 { device_type = "cpu"; reg = <0 1>; + clocks = <&mux0>; next-level-cache = <&L2_1>; }; cpu1: PowerPC,e6500@2 { device_type = "cpu"; reg = <2 3>; + clocks = <&mux0>; next-level-cache = <&L2_1>; }; cpu2: PowerPC,e6500@4 { device_type = "cpu"; reg = <4 5>; + clocks = <&mux0>; next-level-cache = <&L2_1>; }; cpu3: PowerPC,e6500@6 { device_type = "cpu"; reg = <6 7>; + clocks = <&mux0>; next-level-cache = <&L2_1>; }; cpu4: PowerPC,e6500@8 { device_type = "cpu"; reg = <8 9>; + clocks = <&mux1>; next-level-cache = <&L2_2>; }; cpu5: PowerPC,e6500@10 { device_type = "cpu"; reg = <10 11>; + clocks = <&mux1>; next-level-cache = <&L2_2>; }; cpu6: PowerPC,e6500@12 { device_type = "cpu"; reg = <12 13>; + clocks = <&mux1>; next-level-cache = <&L2_2>; }; cpu7: PowerPC,e6500@14 { device_type = "cpu"; reg = <14 15>; + clocks = <&mux1>; next-level-cache = <&L2_2>; }; cpu8: PowerPC,e6500@16 { device_type = "cpu"; reg = <16 17>; + clocks = <&mux2>; next-level-cache = <&L2_3>; }; cpu9: PowerPC,e6500@18 { device_type = "cpu"; reg = <18 19>; + clocks = <&mux2>; next-level-cache = <&L2_3>; }; cpu10: PowerPC,e6500@20 { device_type = "cpu"; reg = <20 21>; + clocks = <&mux2>; next-level-cache = <&L2_3>; }; cpu11: PowerPC,e6500@22 { device_type = "cpu"; reg = <22 23>; + clocks = <&mux2>; next-level-cache = <&L2_3>; }; };