| Message ID | 20220620110739.1598514-4-dmitry.baryshkov@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/4] dt-bindings: clock: qcom,gcc-msm8660: separate GCC bindings for MSM8660 | expand |
On 20/06/2022 13:07, Dmitry Baryshkov wrote: > Add pxo/cxo clocks to the GCC device tree node. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > arch/arm/boot/dts/qcom-msm8660.dtsi | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi > index 47b97daecef1..61e3ab0ebfd3 100644 > --- a/arch/arm/boot/dts/qcom-msm8660.dtsi > +++ b/arch/arm/boot/dts/qcom-msm8660.dtsi > @@ -50,13 +50,13 @@ cpu-pmu { > }; > > clocks { > - cxo_board { > + cxo_board: cxo_board { If you touch this line, please correct the naming for node - no underscores and preferred some prefix or suffix, so: cxo-board-clk clk-cxo-board > compatible = "fixed-clock"; > #clock-cells = <0>; > clock-frequency = <19200000>; > }; > > - pxo_board { > + pxo_board: pxo_board { The same > compatible = "fixed-clock"; > #clock-cells = <0>; > clock-frequency = <27000000>; > @@ -129,6 +129,8 @@ gcc: clock-controller@900000 { > #power-domain-cells = <1>; > #reset-cells = <1>; > reg = <0x900000 0x4000>; > + clocks = <&pxo_board>, <&cxo_board>; > + clock-names = "pxo", "cxo"; > }; > > gsbi6: gsbi@16500000 { Best regards, Krzysztof
On Mon, 20 Jun 2022 at 14:17, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 20/06/2022 13:07, Dmitry Baryshkov wrote: > > Add pxo/cxo clocks to the GCC device tree node. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > arch/arm/boot/dts/qcom-msm8660.dtsi | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi > > index 47b97daecef1..61e3ab0ebfd3 100644 > > --- a/arch/arm/boot/dts/qcom-msm8660.dtsi > > +++ b/arch/arm/boot/dts/qcom-msm8660.dtsi > > @@ -50,13 +50,13 @@ cpu-pmu { > > }; > > > > clocks { > > - cxo_board { > > + cxo_board: cxo_board { > > If you touch this line, please correct the naming for node - no > underscores and preferred some prefix or suffix, so: > cxo-board-clk > clk-cxo-board Unfortunately, I don't think it's possible. We are bound by backwards compatibility. Node name is used as a clock name. And other drivers might reference the clock using the name. Thus, if you check other DT files, we also have similar clock node names. > > compatible = "fixed-clock"; > > #clock-cells = <0>; > > clock-frequency = <19200000>; > > }; > > > > - pxo_board { > > + pxo_board: pxo_board { > > The same > > > compatible = "fixed-clock"; > > #clock-cells = <0>; > > clock-frequency = <27000000>;
On 20/06/2022 13:25, Dmitry Baryshkov wrote: > On Mon, 20 Jun 2022 at 14:17, Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 20/06/2022 13:07, Dmitry Baryshkov wrote: >>> Add pxo/cxo clocks to the GCC device tree node. >>> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>> --- >>> arch/arm/boot/dts/qcom-msm8660.dtsi | 6 ++++-- >>> 1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi >>> index 47b97daecef1..61e3ab0ebfd3 100644 >>> --- a/arch/arm/boot/dts/qcom-msm8660.dtsi >>> +++ b/arch/arm/boot/dts/qcom-msm8660.dtsi >>> @@ -50,13 +50,13 @@ cpu-pmu { >>> }; >>> >>> clocks { >>> - cxo_board { >>> + cxo_board: cxo_board { >> >> If you touch this line, please correct the naming for node - no >> underscores and preferred some prefix or suffix, so: >> cxo-board-clk >> clk-cxo-board > > Unfortunately, I don't think it's possible. We are bound by backwards > compatibility. Node name is used as a clock name. And other drivers > might reference the clock using the name. Thus, if you check other DT > files, we also have similar clock node names. Hm, right, we would need clock-output-names which makes the change a bit bigger than just adding label. Best regards, Krzysztof
On 20/06/2022 13:07, Dmitry Baryshkov wrote: > Add pxo/cxo clocks to the GCC device tree node. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > arch/arm/boot/dts/qcom-msm8660.dtsi | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On Mon 20 Jun 06:07 CDT 2022, Dmitry Baryshkov wrote: > Add pxo/cxo clocks to the GCC device tree node. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > arch/arm/boot/dts/qcom-msm8660.dtsi | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi > index 47b97daecef1..61e3ab0ebfd3 100644 > --- a/arch/arm/boot/dts/qcom-msm8660.dtsi > +++ b/arch/arm/boot/dts/qcom-msm8660.dtsi > @@ -50,13 +50,13 @@ cpu-pmu { > }; > > clocks { > - cxo_board { > + cxo_board: cxo_board { As requested by Krzysztof, please use clock-output-names to specify the name of the clock, and rename the node "cxo-board-clk". Thanks, Bjorn > compatible = "fixed-clock"; > #clock-cells = <0>; > clock-frequency = <19200000>; > }; > > - pxo_board { > + pxo_board: pxo_board { > compatible = "fixed-clock"; > #clock-cells = <0>; > clock-frequency = <27000000>; > @@ -129,6 +129,8 @@ gcc: clock-controller@900000 { > #power-domain-cells = <1>; > #reset-cells = <1>; > reg = <0x900000 0x4000>; > + clocks = <&pxo_board>, <&cxo_board>; > + clock-names = "pxo", "cxo"; > }; > > gsbi6: gsbi@16500000 { > -- > 2.35.1 >
On 14/07/2022 00:29, Bjorn Andersson wrote: > On Mon 20 Jun 06:07 CDT 2022, Dmitry Baryshkov wrote: > >> Add pxo/cxo clocks to the GCC device tree node. >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> arch/arm/boot/dts/qcom-msm8660.dtsi | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi >> index 47b97daecef1..61e3ab0ebfd3 100644 >> --- a/arch/arm/boot/dts/qcom-msm8660.dtsi >> +++ b/arch/arm/boot/dts/qcom-msm8660.dtsi >> @@ -50,13 +50,13 @@ cpu-pmu { >> }; >> >> clocks { >> - cxo_board { >> + cxo_board: cxo_board { > > As requested by Krzysztof, please use clock-output-names to specify the > name of the clock, and rename the node "cxo-board-clk". Actually I believe Krzysztof agreed (and acked) this change, as it follows the example of existing boards. > > Thanks, > Bjorn > >> compatible = "fixed-clock"; >> #clock-cells = <0>; >> clock-frequency = <19200000>; >> }; >> >> - pxo_board { >> + pxo_board: pxo_board { >> compatible = "fixed-clock"; >> #clock-cells = <0>; >> clock-frequency = <27000000>; >> @@ -129,6 +129,8 @@ gcc: clock-controller@900000 { >> #power-domain-cells = <1>; >> #reset-cells = <1>; >> reg = <0x900000 0x4000>; >> + clocks = <&pxo_board>, <&cxo_board>; >> + clock-names = "pxo", "cxo"; >> }; >> >> gsbi6: gsbi@16500000 { >> -- >> 2.35.1 >>
On 14/07/2022 09:41, Dmitry Baryshkov wrote: > On 14/07/2022 00:29, Bjorn Andersson wrote: >> On Mon 20 Jun 06:07 CDT 2022, Dmitry Baryshkov wrote: >> >>> Add pxo/cxo clocks to the GCC device tree node. >>> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>> --- >>> arch/arm/boot/dts/qcom-msm8660.dtsi | 6 ++++-- >>> 1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi >>> index 47b97daecef1..61e3ab0ebfd3 100644 >>> --- a/arch/arm/boot/dts/qcom-msm8660.dtsi >>> +++ b/arch/arm/boot/dts/qcom-msm8660.dtsi >>> @@ -50,13 +50,13 @@ cpu-pmu { >>> }; >>> >>> clocks { >>> - cxo_board { >>> + cxo_board: cxo_board { >> >> As requested by Krzysztof, please use clock-output-names to specify the >> name of the clock, and rename the node "cxo-board-clk". > > Actually I believe Krzysztof agreed (and acked) this change, as it > follows the example of existing boards. Yeah, because this would be out of scope of this change. In the long term fixing the node name would be still useful, because DTC W=2 complains. Best regards, Krzysztof
diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi index 47b97daecef1..61e3ab0ebfd3 100644 --- a/arch/arm/boot/dts/qcom-msm8660.dtsi +++ b/arch/arm/boot/dts/qcom-msm8660.dtsi @@ -50,13 +50,13 @@ cpu-pmu { }; clocks { - cxo_board { + cxo_board: cxo_board { compatible = "fixed-clock"; #clock-cells = <0>; clock-frequency = <19200000>; }; - pxo_board { + pxo_board: pxo_board { compatible = "fixed-clock"; #clock-cells = <0>; clock-frequency = <27000000>; @@ -129,6 +129,8 @@ gcc: clock-controller@900000 { #power-domain-cells = <1>; #reset-cells = <1>; reg = <0x900000 0x4000>; + clocks = <&pxo_board>, <&cxo_board>; + clock-names = "pxo", "cxo"; }; gsbi6: gsbi@16500000 {
Add pxo/cxo clocks to the GCC device tree node. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- arch/arm/boot/dts/qcom-msm8660.dtsi | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)