| Message ID | 20220705202837.667-3-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | clk: qcom: Drop use of pxo/cxo_board for rpm devices | expand |
On Tue, 5 Jul 2022 at 23:56, Christian Marangi <ansuelsmth@gmail.com> wrote: > > Fix dtbs_check warning for new rpmcc Documentation changes and add the > required clocks. > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > arch/arm/boot/dts/qcom-apq8064.dtsi | 2 ++ > arch/arm/boot/dts/qcom-ipq8064.dtsi | 2 ++ > arch/arm/boot/dts/qcom-msm8660.dtsi | 4 +++- > 3 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi > index 91adcbd54b17..6a88e616ea01 100644 > --- a/arch/arm/boot/dts/qcom-apq8064.dtsi > +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi > @@ -853,6 +853,8 @@ rpm@108000 { > rpmcc: clock-controller { > compatible = "qcom,rpmcc-apq8064", "qcom,rpmcc"; > #clock-cells = <1>; > + clocks = <&pxo_board>, <&cxo_board>; > + clock-names = "pxo", "cxo"; > }; > > regulators { > diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi > index 4b475d98343c..1425a4e4283f 100644 > --- a/arch/arm/boot/dts/qcom-ipq8064.dtsi > +++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi > @@ -773,6 +773,8 @@ rpm: rpm@108000 { > rpmcc: clock-controller { > compatible = "qcom,rpmcc-ipq806x", "qcom,rpmcc"; > #clock-cells = <1>; > + clocks = <&pxo_board>; > + clock-names = "pxo"; > }; > }; > > diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi > index b9cded35b1cc..63a501c63cf8 100644 > --- a/arch/arm/boot/dts/qcom-msm8660.dtsi > +++ b/arch/arm/boot/dts/qcom-msm8660.dtsi > @@ -56,7 +56,7 @@ cxo_board { > clock-frequency = <19200000>; > }; > > - pxo_board { > + pxo_board: pxo_board { > compatible = "fixed-clock"; > #clock-cells = <0>; > clock-frequency = <27000000>; > @@ -411,6 +411,8 @@ rpm: rpm@104000 { > rpmcc: clock-controller { > compatible = "qcom,rpmcc-msm8660", "qcom,rpmcc"; > #clock-cells = <1>; > + clocks = <&pxo_board>; > + clock-names = "pxo"; > }; > > pm8901-regulators { > -- > 2.36.1 >
On 05/07/2022 22:28, Christian Marangi wrote: > Fix dtbs_check warning for new rpmcc Documentation changes and add the > required clocks. There is no warning in the kernel, right? So the commit is not correct. Only the second part of your sentence applies, but you should extend it. Best regards, Krzysztof
On Wed, Jul 06, 2022 at 09:44:04AM +0200, Krzysztof Kozlowski wrote: > On 05/07/2022 22:28, Christian Marangi wrote: > > Fix dtbs_check warning for new rpmcc Documentation changes and add the > > required clocks. > > There is no warning in the kernel, right? So the commit is not correct. > Oh ok, the warning is generated by the new Documentation. > Only the second part of your sentence applies, but you should extend it. > Ok will reword this. > > > Best regards, > Krzysztof
On 06/07/2022 12:20, Christian Marangi wrote: > On Wed, Jul 06, 2022 at 09:44:04AM +0200, Krzysztof Kozlowski wrote: >> On 05/07/2022 22:28, Christian Marangi wrote: >>> Fix dtbs_check warning for new rpmcc Documentation changes and add the >>> required clocks. >> >> There is no warning in the kernel, right? So the commit is not correct. >> > > Oh ok, the warning is generated by the new Documentation. Patches, especially DTS, might go via different trees, so the moment DTS is applied there might be no such warning. Best regards, Krzysztof
On Wed, Jul 06, 2022 at 05:07:12PM +0200, Krzysztof Kozlowski wrote: > On 06/07/2022 12:20, Christian Marangi wrote: > > On Wed, Jul 06, 2022 at 09:44:04AM +0200, Krzysztof Kozlowski wrote: > >> On 05/07/2022 22:28, Christian Marangi wrote: > >>> Fix dtbs_check warning for new rpmcc Documentation changes and add the > >>> required clocks. > >> > >> There is no warning in the kernel, right? So the commit is not correct. > >> > > > > Oh ok, the warning is generated by the new Documentation. > > Patches, especially DTS, might go via different trees, so the moment DTS > is applied there might be no such warning. > I'm still confused about this topic... With this kind of change, I notice I sent Documentation change and then rob bot complain about dtbs_check having warning... So the correct way is to send Documentation change and fix dtbs_check warning in the same commit OR keep what I'm doing with sending Documentation changes and fix DTS in a separate commit? I assume separate patch was the way to go but now I'm not sure anymore... > Best regards, > Krzysztof
On 06/07/2022 21:10, Christian Marangi wrote: > On Wed, Jul 06, 2022 at 05:07:12PM +0200, Krzysztof Kozlowski wrote: >> On 06/07/2022 12:20, Christian Marangi wrote: >>> On Wed, Jul 06, 2022 at 09:44:04AM +0200, Krzysztof Kozlowski wrote: >>>> On 05/07/2022 22:28, Christian Marangi wrote: >>>>> Fix dtbs_check warning for new rpmcc Documentation changes and add the >>>>> required clocks. >>>> >>>> There is no warning in the kernel, right? So the commit is not correct. >>>> >>> >>> Oh ok, the warning is generated by the new Documentation. >> >> Patches, especially DTS, might go via different trees, so the moment DTS >> is applied there might be no such warning. >> > > I'm still confused about this topic... > With this kind of change, I notice I sent Documentation change and then > rob bot complain about dtbs_check having warning... > > So the correct way is to send Documentation change and fix dtbs_check > warning in the same commit OR keep what I'm doing with sending > Documentation changes and fix DTS in a separate commit? Binding is almost always separate from DTS and always separate from driver. The order depends on what you're doing. If you bring ABI break change to bindings, then the order does not matter, because each order will be non-bisectable. Because you broke ABI. That's the case in this patchset. For other cases, usually bindings patches should be the first in patchset. How it goes via maintainer trees is not your problem here. Patches might go together or might go separate. Anyway it was not the topic of my comment. Comment was about not specific commit msg which does not fit the Linux kernel process and does not fit git history once applied by maintainer. It fits even less when backported to stable kernels, which you commit msg encourages to do. Best regards, Krzysztof
On Wed, Jul 06, 2022 at 10:09:05PM +0200, Krzysztof Kozlowski wrote: > On 06/07/2022 21:10, Christian Marangi wrote: > > On Wed, Jul 06, 2022 at 05:07:12PM +0200, Krzysztof Kozlowski wrote: > >> On 06/07/2022 12:20, Christian Marangi wrote: > >>> On Wed, Jul 06, 2022 at 09:44:04AM +0200, Krzysztof Kozlowski wrote: > >>>> On 05/07/2022 22:28, Christian Marangi wrote: > >>>>> Fix dtbs_check warning for new rpmcc Documentation changes and add the > >>>>> required clocks. > >>>> > >>>> There is no warning in the kernel, right? So the commit is not correct. > >>>> > >>> > >>> Oh ok, the warning is generated by the new Documentation. > >> > >> Patches, especially DTS, might go via different trees, so the moment DTS > >> is applied there might be no such warning. > >> > > > > I'm still confused about this topic... > > With this kind of change, I notice I sent Documentation change and then > > rob bot complain about dtbs_check having warning... > > > > So the correct way is to send Documentation change and fix dtbs_check > > warning in the same commit OR keep what I'm doing with sending > > Documentation changes and fix DTS in a separate commit? > > Binding is almost always separate from DTS and always separate from > driver. The order depends on what you're doing. If you bring ABI break > change to bindings, then the order does not matter, because each order > will be non-bisectable. Because you broke ABI. That's the case in this > patchset. > > For other cases, usually bindings patches should be the first in patchset. > > How it goes via maintainer trees is not your problem here. Patches might > go together or might go separate. > > Anyway it was not the topic of my comment. Comment was about not > specific commit msg which does not fit the Linux kernel process and does > not fit git history once applied by maintainer. It fits even less when > backported to stable kernels, which you commit msg encourages to do. > It was a more generic question so sorry for the OT. Will reword the commit description, thanks again for the clarification about this generic topic.
diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi index 91adcbd54b17..6a88e616ea01 100644 --- a/arch/arm/boot/dts/qcom-apq8064.dtsi +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi @@ -853,6 +853,8 @@ rpm@108000 { rpmcc: clock-controller { compatible = "qcom,rpmcc-apq8064", "qcom,rpmcc"; #clock-cells = <1>; + clocks = <&pxo_board>, <&cxo_board>; + clock-names = "pxo", "cxo"; }; regulators { diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi index 4b475d98343c..1425a4e4283f 100644 --- a/arch/arm/boot/dts/qcom-ipq8064.dtsi +++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi @@ -773,6 +773,8 @@ rpm: rpm@108000 { rpmcc: clock-controller { compatible = "qcom,rpmcc-ipq806x", "qcom,rpmcc"; #clock-cells = <1>; + clocks = <&pxo_board>; + clock-names = "pxo"; }; }; diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi index b9cded35b1cc..63a501c63cf8 100644 --- a/arch/arm/boot/dts/qcom-msm8660.dtsi +++ b/arch/arm/boot/dts/qcom-msm8660.dtsi @@ -56,7 +56,7 @@ cxo_board { clock-frequency = <19200000>; }; - pxo_board { + pxo_board: pxo_board { compatible = "fixed-clock"; #clock-cells = <0>; clock-frequency = <27000000>; @@ -411,6 +411,8 @@ rpm: rpm@104000 { rpmcc: clock-controller { compatible = "qcom,rpmcc-msm8660", "qcom,rpmcc"; #clock-cells = <1>; + clocks = <&pxo_board>; + clock-names = "pxo"; }; pm8901-regulators {
Fix dtbs_check warning for new rpmcc Documentation changes and add the required clocks. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- arch/arm/boot/dts/qcom-apq8064.dtsi | 2 ++ arch/arm/boot/dts/qcom-ipq8064.dtsi | 2 ++ arch/arm/boot/dts/qcom-msm8660.dtsi | 4 +++- 3 files changed, 7 insertions(+), 1 deletion(-)