| Message ID | 20250304143152.1799966-1-vladimir.zapolskiy@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | dt-bindings: clock: qcom,x1e80100-camcc: Fix the list of required-opps | expand |
On Tue, Mar 04, 2025 at 04:31:52PM +0200, Vladimir Zapolskiy wrote: > The switch to multiple power domains implies that the required-opps > property shall be updated accordingly, a record in one property > corresponds to a record in another one. > > Fixes: 7ec95ff9abf4 ("dt-bindings: clock: move qcom,x1e80100-camcc to its own file") > Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> > --- > .../devicetree/bindings/clock/qcom,x1e80100-camcc.yaml | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/qcom,x1e80100-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,x1e80100-camcc.yaml > index 5bbbaa15a260..938a2f1ff3fc 100644 > --- a/Documentation/devicetree/bindings/clock/qcom,x1e80100-camcc.yaml > +++ b/Documentation/devicetree/bindings/clock/qcom,x1e80100-camcc.yaml > @@ -40,9 +40,9 @@ properties: > - description: A phandle to the MMCX power-domain > > required-opps: > - maxItems: 1 > - description: > - A phandle to an OPP node describing MMCX performance points. > + items: > + - description: A phandle to an OPP node describing MXC performance points > + - description: A phandle to an OPP node describing MMCX performance points If rewriting this, then: s/A phandle to an OPP node describing// because it is redundant (this cannot be anything else). But more important is that you introduced ABI break, without actual reason. Switch to multiple power domains does not look like fix so neither should this be. Reverse the items to keep the ABI intact... unless ABI was broken earlier and this just keeps doing it to make code consistent. But then please explain it in commit msg why we need to break it second time. Best regards, Krzysztof
On 3/5/25 09:56, Krzysztof Kozlowski wrote: > On Tue, Mar 04, 2025 at 04:31:52PM +0200, Vladimir Zapolskiy wrote: >> The switch to multiple power domains implies that the required-opps >> property shall be updated accordingly, a record in one property >> corresponds to a record in another one. >> >> Fixes: 7ec95ff9abf4 ("dt-bindings: clock: move qcom,x1e80100-camcc to its own file") >> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> >> --- >> .../devicetree/bindings/clock/qcom,x1e80100-camcc.yaml | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/clock/qcom,x1e80100-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,x1e80100-camcc.yaml >> index 5bbbaa15a260..938a2f1ff3fc 100644 >> --- a/Documentation/devicetree/bindings/clock/qcom,x1e80100-camcc.yaml >> +++ b/Documentation/devicetree/bindings/clock/qcom,x1e80100-camcc.yaml >> @@ -40,9 +40,9 @@ properties: >> - description: A phandle to the MMCX power-domain >> >> required-opps: >> - maxItems: 1 >> - description: >> - A phandle to an OPP node describing MMCX performance points. >> + items: >> + - description: A phandle to an OPP node describing MXC performance points >> + - description: A phandle to an OPP node describing MMCX performance points > > If rewriting this, then: > s/A phandle to an OPP node describing// > because it is redundant (this cannot be anything else). > > But more important is that you introduced ABI break, without actual > reason. Switch to multiple power domains does not look like fix so > neither should this be. Reverse the items to keep the ABI intact... To the best of my knowledge there is no users of this compatible in the Linux kernel, so likely there is no ABI break. > unless ABI was broken earlier and this just keeps doing it to make code > consistent. But then please explain it in commit msg why we need to > break it second time. > https://lore.kernel.org/all/CAA8EJprP9Z181VDCT=xfyrBipzgiB0tfb8M_XZ4H=yOrvEnB0w@mail.gmail.com/ -- Best wishes, Vladimir
diff --git a/Documentation/devicetree/bindings/clock/qcom,x1e80100-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,x1e80100-camcc.yaml index 5bbbaa15a260..938a2f1ff3fc 100644 --- a/Documentation/devicetree/bindings/clock/qcom,x1e80100-camcc.yaml +++ b/Documentation/devicetree/bindings/clock/qcom,x1e80100-camcc.yaml @@ -40,9 +40,9 @@ properties: - description: A phandle to the MMCX power-domain required-opps: - maxItems: 1 - description: - A phandle to an OPP node describing MMCX performance points. + items: + - description: A phandle to an OPP node describing MXC performance points + - description: A phandle to an OPP node describing MMCX performance points required: - compatible @@ -66,7 +66,8 @@ examples: <&sleep_clk>; power-domains = <&rpmhpd RPMHPD_MXC>, <&rpmhpd RPMHPD_MMCX>; - required-opps = <&rpmhpd_opp_low_svs>; + required-opps = <&rpmhpd_opp_low_svs>, + <&rpmhpd_opp_low_svs>; #clock-cells = <1>; #reset-cells = <1>; #power-domain-cells = <1>;
The switch to multiple power domains implies that the required-opps property shall be updated accordingly, a record in one property corresponds to a record in another one. Fixes: 7ec95ff9abf4 ("dt-bindings: clock: move qcom,x1e80100-camcc to its own file") Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> --- .../devicetree/bindings/clock/qcom,x1e80100-camcc.yaml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)