| Message ID | 20230612053922.3284394-4-dmitry.baryshkov@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | viresh kumar |
| Headers | show |
| Series | ARM: qcom: apq8064: support CPU frequency scaling | expand |
On 12/06/2023 07:39, Dmitry Baryshkov wrote: > The SAW2 device can optionally provide a voltage regulator supplying the > CPU core, cluster or L2 cache. Describe it in the device bindings. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > .../devicetree/bindings/soc/qcom/qcom,saw2.yaml | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml > index a016242367b9..b809a9cc0916 100644 > --- a/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml > @@ -47,6 +47,10 @@ properties: > - description: Base address and size of the alias register region > minItems: 1 > > + regulator: > + $ref: /schemas/regulator/regulator.yaml# There was such property in the binding (and DTS!) but a bool. Previous patch silently dropped it, so re-introducing it with different type is confusing. Best regards, Krzysztof
On 14/06/2023 19:05, Krzysztof Kozlowski wrote: > On 12/06/2023 07:39, Dmitry Baryshkov wrote: >> The SAW2 device can optionally provide a voltage regulator supplying the >> CPU core, cluster or L2 cache. Describe it in the device bindings. >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> .../devicetree/bindings/soc/qcom/qcom,saw2.yaml | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml >> index a016242367b9..b809a9cc0916 100644 >> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml >> @@ -47,6 +47,10 @@ properties: >> - description: Base address and size of the alias register region >> minItems: 1 >> >> + regulator: >> + $ref: /schemas/regulator/regulator.yaml# > > There was such property in the binding (and DTS!) but a bool. Previous > patch silently dropped it, so re-introducing it with different type is > confusing. Could you please propose a better name here? saw-regulator? Or maybe regulator-saw? (as we might get regulator-avs at some point).
On 15/06/2023 00:49, Dmitry Baryshkov wrote: > On 14/06/2023 19:05, Krzysztof Kozlowski wrote: >> On 12/06/2023 07:39, Dmitry Baryshkov wrote: >>> The SAW2 device can optionally provide a voltage regulator supplying the >>> CPU core, cluster or L2 cache. Describe it in the device bindings. >>> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>> --- >>> .../devicetree/bindings/soc/qcom/qcom,saw2.yaml | 17 +++++++++++++++++ >>> 1 file changed, 17 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml >>> index a016242367b9..b809a9cc0916 100644 >>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml >>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml >>> @@ -47,6 +47,10 @@ properties: >>> - description: Base address and size of the alias register region >>> minItems: 1 >>> >>> + regulator: >>> + $ref: /schemas/regulator/regulator.yaml# >> >> There was such property in the binding (and DTS!) but a bool. Previous >> patch silently dropped it, so re-introducing it with different type is >> confusing. > > Could you please propose a better name here? saw-regulator? Or maybe > regulator-saw? (as we might get regulator-avs at some point). regulator name is OK for me, but any ABI change should be: 1. Clearly expressed with rationale, 2. Done probably in one DT commit, not two. IOW, first silently dropping a property and then adding a new one like nothing happened is not good. It should be clear that old property is wrong because foo bar and we make it now different with breaking all the DTS because foo bar. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml index a016242367b9..b809a9cc0916 100644 --- a/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,saw2.yaml @@ -47,6 +47,10 @@ properties: - description: Base address and size of the alias register region minItems: 1 + regulator: + $ref: /schemas/regulator/regulator.yaml# + description: corresponding core, cluster or cache voltage supply regulator + required: - compatible - reg @@ -92,4 +96,17 @@ examples: reg = <0x17912000 0x1000>; }; + - | + /* + * Example 3: SAW2 with the bundled regulator definition. + */ + power-manager@2089000 { + compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2"; + reg = <0x02089000 0x1000>, <0x02009000 0x1000>; + + regulator { + regulator-min-microvolt = <850000>; + regulator-max-microvolt = <1300000>; + }; + }; ...
The SAW2 device can optionally provide a voltage regulator supplying the CPU core, cluster or L2 cache. Describe it in the device bindings. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- .../devicetree/bindings/soc/qcom/qcom,saw2.yaml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)