Message ID | 20180919073906.34187-2-andy.tang@nxp.com (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers | show |
Series | [1/3] clk: qoriq: add t1023 soc support | expand |
On Wed, 2018-09-19 at 15:39 +0800, andy.tang@nxp.com wrote: > +clockgen: global-utilities@e1000 { > + compatible = "fsl,qoriq-clockgen"; Where does this compatible string come from? > + compatible = "fsl,t1023-clockgen"; > }; And here you overwrite it with only the chip-specific compatible? Is t1023 incompatible with both fsl,qoriq-clockgen-1.0 and fsl,qoriq-clockgen- 2.0? The existing dts says 2.0; is that wrong? BTW, assuming it is 2.0 compatible and thus the use of qoriq-clockgen2.dtsi is correct, the best course of action is probably to to remove the legacy stuff from all fsl chips, rather than introduce a new dtsi. In fact it'd be nice to see it all removed in any case. :-) Also, please post any patches that you want me to apply to the linuxppc-dev mailing list. -Scott
Hi Scott, Please see my reply inline. > -----Original Message----- > From: Scott Wood <oss@buserror.net> > Sent: 2018年10月21日 7:54 > To: Andy Tang <andy.tang@nxp.com>; mturquette@baylibre.com > Cc: sboyd@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com; > linux-clk@vger.kernel.org; devicetree@vger.kernel.org > Subject: Re: [PATCH 2/3] powerpc: t102x: upgrade the legacy clock node > > On Wed, 2018-09-19 at 15:39 +0800, andy.tang@nxp.com wrote: > > +clockgen: global-utilities@e1000 { > > + compatible = "fsl,qoriq-clockgen"; > > Where does this compatible string come from? > > > + compatible = "fsl,t1023-clockgen"; > > }; > > And here you overwrite it with only the chip-specific compatible? > > Is t1023 incompatible with both fsl,qoriq-clockgen-1.0 and > fsl,qoriq-clockgen- 2.0? The existing dts says 2.0; is that wrong? > > BTW, assuming it is 2.0 compatible and thus the use of > qoriq-clockgen2.dtsi is correct, the best course of action is probably to to > remove the legacy stuff from all fsl chips, rather than introduce a new dtsi. > In fact it'd be nice to see it all removed in any case. :-) qoriq-clockgen*.dtsi are used by legacy bindings. The contents are all of legacy bindings. To use new framework, I introduce a new dtsi which contains new bindings and used for all PPC soc. A chip-specific compatible is needed because driver will use it to get chip-specific clock tree information. The clock information was defined in driver not in dts in new framework, remember? This patch set is the first step to convert it to using new framework. After all the chips have been updated, the legacy stuff will be removed. Including qoriq-clockgen*.dtsi. > > Also, please post any patches that you want me to apply to the > linuxppc-dev mailing list. Sure, no problem. BR, Andy > > -Scott
On Mon, 2018-10-22 at 01:34 +0000, Andy Tang wrote: > Hi Scott, > > Please see my reply inline. > > > -----Original Message----- > > From: Scott Wood <oss@buserror.net> > > Sent: 2018年10月21日 7:54 > > To: Andy Tang <andy.tang@nxp.com>; mturquette@baylibre.com > > Cc: sboyd@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com; > > linux-clk@vger.kernel.org; devicetree@vger.kernel.org > > Subject: Re: [PATCH 2/3] powerpc: t102x: upgrade the legacy clock node > > > > On Wed, 2018-09-19 at 15:39 +0800, andy.tang@nxp.com wrote: > > > +clockgen: global-utilities@e1000 { > > > + compatible = "fsl,qoriq-clockgen"; > > > > Where does this compatible string come from? > > > > > + compatible = "fsl,t1023-clockgen"; > > > }; > > > > And here you overwrite it with only the chip-specific compatible? > > > > Is t1023 incompatible with both fsl,qoriq-clockgen-1.0 and > > fsl,qoriq-clockgen- 2.0? The existing dts says 2.0; is that wrong? > > > > BTW, assuming it is 2.0 compatible and thus the use of > > qoriq-clockgen2.dtsi is correct, the best course of action is probably to > > to > > remove the legacy stuff from all fsl chips, rather than introduce a new > > dtsi. > > In fact it'd be nice to see it all removed in any case. :-) > > qoriq-clockgen*.dtsi are used by legacy bindings. The contents are all of > legacy bindings. > To use new framework, I introduce a new dtsi which contains new bindings and > used for all PPC soc. > A chip-specific compatible is needed because driver will use it to get chip- > specific clock tree information. > The clock information was defined in driver not in dts in new framework, > remember? I'm aware that a chip-specific compatible is required. What is unusual (on PPC) is providing *only* the chip-specific compatible. I was expecting something more along the lines of https://patchwork.ozlabs.org/patch/486565/ Granted, the driver requiring two different compatibles to be present is odd, and was a convenience based on what was already in the device trees, but the fsl,qoriq-clockgen-2.0 compatible is part of the new binding (albeit an optional part). I guess I don't mind no longer relying on it, but at least remove the "fsl,qoriq-clockgen" string. -Scott
Hi Scott, Please see my reply inline. > -----Original Message----- > From: Scott Wood <oss@buserror.net> > Sent: 2018年10月22日 13:21 > To: Andy Tang <andy.tang@nxp.com>; mturquette@baylibre.com > Cc: sboyd@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com; > linux-clk@vger.kernel.org; devicetree@vger.kernel.org > Subject: Re: [PATCH 2/3] powerpc: t102x: upgrade the legacy clock node > > On Mon, 2018-10-22 at 01:34 +0000, Andy Tang wrote: > > Hi Scott, > > > > Please see my reply inline. > > > > > -----Original Message----- > > > From: Scott Wood <oss@buserror.net> > > > Sent: 2018年10月21日 7:54 > > > To: Andy Tang <andy.tang@nxp.com>; mturquette@baylibre.com > > > Cc: sboyd@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com; > > > linux-clk@vger.kernel.org; devicetree@vger.kernel.org > > > Subject: Re: [PATCH 2/3] powerpc: t102x: upgrade the legacy clock > > > node > > > > > > On Wed, 2018-09-19 at 15:39 +0800, andy.tang@nxp.com wrote: > > > > +clockgen: global-utilities@e1000 { > > > > + compatible = "fsl,qoriq-clockgen"; > > > > > > Where does this compatible string come from? > > > > > > > + compatible = "fsl,t1023-clockgen"; > > > > }; > > > > > > And here you overwrite it with only the chip-specific compatible? > > > > > > Is t1023 incompatible with both fsl,qoriq-clockgen-1.0 and > > > fsl,qoriq-clockgen- 2.0? The existing dts says 2.0; is that wrong? > > > > > > BTW, assuming it is 2.0 compatible and thus the use of > > > qoriq-clockgen2.dtsi is correct, the best course of action is > > > probably to to remove the legacy stuff from all fsl chips, rather > > > than introduce a new dtsi. > > > In fact it'd be nice to see it all removed in any case. :-) > > > > qoriq-clockgen*.dtsi are used by legacy bindings. The contents are all > > of legacy bindings. > > To use new framework, I introduce a new dtsi which contains new > > bindings and used for all PPC soc. > > A chip-specific compatible is needed because driver will use it to get > > chip- specific clock tree information. > > The clock information was defined in driver not in dts in new > > framework, remember? > > I'm aware that a chip-specific compatible is required. What is unusual (on > PPC) is providing *only* the chip-specific compatible. I was expecting > something more along the lines of > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp > atchwork.ozlabs.org%2Fpatch%2F486565%2F&data=02%7C01%7Can > dy.tang%40nxp.com%7Ceaf46e43620b45a19eb408d637de20cb%7C686e > a1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636757824505667539 > &sdata=T5yRSWtHYO2yLMjsZRIxZT%2BM82xuGQm2VGX7MRB8MJA% > 3D&reserved=0 > > Granted, the driver requiring two different compatibles to be present is > odd, and was a convenience based on what was already in the device > trees, but the > fsl,qoriq-clockgen-2.0 compatible is part of the new binding (albeit an > optional part). I guess I don't mind no longer relying on it, but at least > remove the "fsl,qoriq-clockgen" string. After saw your previous patch, I plan to work out a similar patch. The fsl,qoriq-clockgen-2.0 and fsl,qoriq-clockgen-1.0 will be retained. Thanks, Andy > > -Scott
diff --git a/arch/powerpc/boot/dts/fsl/qoriq-clockgen.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-clockgen.dtsi new file mode 100644 index 0000000..49c6f86 --- /dev/null +++ b/arch/powerpc/boot/dts/fsl/qoriq-clockgen.dtsi @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * Device Tree Include file for NXP PowerPC family SoC. + * + * Copyright 2017 NXP + * + * Tang Yuantian <andy.tang@nxp.com> + * + */ + +sysclk: sysclk { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <100000000>; + clock-output-names = "sysclk"; +}; + +clockgen: global-utilities@e1000 { + compatible = "fsl,qoriq-clockgen"; + reg = <0xe1000 0x1000>; + #clock-cells = <2>; + clocks = <&sysclk>; +}; diff --git a/arch/powerpc/boot/dts/fsl/t1023si-post.dtsi b/arch/powerpc/boot/dts/fsl/t1023si-post.dtsi index 4908af5..c77d3e9 100644 --- a/arch/powerpc/boot/dts/fsl/t1023si-post.dtsi +++ b/arch/powerpc/boot/dts/fsl/t1023si-post.dtsi @@ -342,25 +342,9 @@ fsl,liodn-bits = <12>; }; -/include/ "qoriq-clockgen2.dtsi" +/include/ "qoriq-clockgen.dtsi" global-utilities@e1000 { - compatible = "fsl,t1023-clockgen", "fsl,qoriq-clockgen-2.0"; - mux0: mux0@0 { - #clock-cells = <0>; - reg = <0x0 4>; - compatible = "fsl,core-mux-clock"; - clocks = <&pll0 0>, <&pll0 1>; - clock-names = "pll0_0", "pll0_1"; - clock-output-names = "cmux0"; - }; - mux1: mux1@20 { - #clock-cells = <0>; - reg = <0x20 4>; - compatible = "fsl,core-mux-clock"; - clocks = <&pll0 0>, <&pll0 1>; - clock-names = "pll0_0", "pll0_1"; - clock-output-names = "cmux1"; - }; + compatible = "fsl,t1023-clockgen"; }; rcpm: global-utilities@e2000 { diff --git a/arch/powerpc/boot/dts/fsl/t102xsi-pre.dtsi b/arch/powerpc/boot/dts/fsl/t102xsi-pre.dtsi index 9d08a36..d87ea13 100644 --- a/arch/powerpc/boot/dts/fsl/t102xsi-pre.dtsi +++ b/arch/powerpc/boot/dts/fsl/t102xsi-pre.dtsi @@ -74,7 +74,7 @@ cpu0: PowerPC,e5500@0 { device_type = "cpu"; reg = <0>; - clocks = <&mux0>; + clocks = <&clockgen 1 0>; next-level-cache = <&L2_1>; #cooling-cells = <2>; L2_1: l2-cache { @@ -84,7 +84,7 @@ cpu1: PowerPC,e5500@1 { device_type = "cpu"; reg = <1>; - clocks = <&mux1>; + clocks = <&clockgen 1 1>; next-level-cache = <&L2_2>; #cooling-cells = <2>; L2_2: l2-cache {