| Message ID | 20201126184559.3052375-14-angelogioacchino.delregno@somainline.org |
|---|---|
| State | New |
| Headers | show |
| Series | Enable CPRh/3/4, CPU Scaling on various QCOM SoCs | expand |
On Thu, Nov 26, 2020 at 07:45:59PM +0100, AngeloGioacchino Del Regno wrote: > The OSM programming addition has been done under the > qcom,cpufreq-hw-8998 compatible name: specify the requirement > of two additional register spaces for this functionality. > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> > --- > .../bindings/cpufreq/qcom,cpufreq-hw.yaml | 31 ++++++++++++++++--- > 1 file changed, 27 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/cpufreq/qcom,cpufreq-hw.yaml b/Documentation/devicetree/bindings/cpufreq/qcom,cpufreq-hw.yaml > index 94a56317b14b..f64cea73037e 100644 > --- a/Documentation/devicetree/bindings/cpufreq/qcom,cpufreq-hw.yaml > +++ b/Documentation/devicetree/bindings/cpufreq/qcom,cpufreq-hw.yaml > @@ -23,17 +23,21 @@ properties: > - qcom,cpufreq-epss > > reg: > + description: Base address and size of the RBCPR register region That doesn't make sense given you have 2 regions. > minItems: 2 > maxItems: 2 maxItems: 4 > > reg-names: > description: > - Frequency domain register region for each domain. > - items: > - - const: "freq-domain0" > - - const: "freq-domain1" > + Frequency domain register region for each domain. If OSM programming > + does not happen in the bootloader and has to be done in this driver, > + then also the OSM domain region osm-domain[0-1] has to be provided. Don't write free form text for what can be expressed as schema. > + minItems: 2 > + maxItems: 2 You obviously haven't tried this change with 8998. It will fail with more than 2. What you need here is: minItems: 2 maxItems: 4 items: - const: "freq-domain0" - const: "freq-domain1" - const: "osm-domain0" - const: "osm-domain1" And then... > > clock-names: > + minItems: 2 > + maxItems: 2 > - const: xo > - const: ref > > @@ -53,9 +57,28 @@ properties: > property with phandle to a cpufreq_hw followed by the Domain ID(0/1) > in the CPU DT node. > > +allOf: > + - if: > + properties: > + reg-names: > + contains: > + const: qcom,cpufreq-hw-8998 > + then: > + properties: > + reg: > + minItems: 4 > + maxItems: 4 > + reg-names: ...here just: minItems: 4 And you'll need an 'else' clause with 'maxItems: 2' for reg and reg-names. > + items: > + - const: "freq-domain0" > + - const: "freq-domain1" > + - const: "osm-domain0" > + - const: "osm-domain1" > + > required: > - compatible > - reg > + - reg-names You can't make something that was optional now required. (Unless it was a mistake and all existing users always had 'reg-names'.) > - clock-names > - clocks > - "#freq-domain-cells" > -- > 2.29.2 >
Il 08/12/20 19:11, Rob Herring ha scritto: Hello! Replying very late seem to be obligatory for me nowadays so for this and for any other late replies: I'm sorry! > On Thu, Nov 26, 2020 at 07:45:59PM +0100, AngeloGioacchino Del Regno wrote: >> The OSM programming addition has been done under the >> qcom,cpufreq-hw-8998 compatible name: specify the requirement >> of two additional register spaces for this functionality. >> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> >> --- >> .../bindings/cpufreq/qcom,cpufreq-hw.yaml | 31 ++++++++++++++++--- >> 1 file changed, 27 insertions(+), 4 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/cpufreq/qcom,cpufreq-hw.yaml b/Documentation/devicetree/bindings/cpufreq/qcom,cpufreq-hw.yaml >> index 94a56317b14b..f64cea73037e 100644 >> --- a/Documentation/devicetree/bindings/cpufreq/qcom,cpufreq-hw.yaml >> +++ b/Documentation/devicetree/bindings/cpufreq/qcom,cpufreq-hw.yaml >> @@ -23,17 +23,21 @@ properties: >> - qcom,cpufreq-epss >> >> reg: >> + description: Base address and size of the RBCPR register region > > That doesn't make sense given you have 2 regions. > >> minItems: 2 >> maxItems: 2 > > maxItems: 4 > Indeed it doesn't make sense. >> >> reg-names: >> description: >> - Frequency domain register region for each domain. >> - items: >> - - const: "freq-domain0" >> - - const: "freq-domain1" >> + Frequency domain register region for each domain. If OSM programming >> + does not happen in the bootloader and has to be done in this driver, >> + then also the OSM domain region osm-domain[0-1] has to be provided. > > Don't write free form text for what can be expressed as schema. > I guess the later 'if' for 8998 is sufficient to express that, then... right? >> + minItems: 2 >> + maxItems: 2 > > You obviously haven't tried this change with 8998. It will fail with > more than 2. What you need here is: > My testing methodology must be flawed. Or perhaps I just need some more sleep... probably the latter. > minItems: 2 > maxItems: 4 > > items: > - const: "freq-domain0" > - const: "freq-domain1" > - const: "osm-domain0" > - const: "osm-domain1" > > And then... > >> >> clock-names: >> + minItems: 2 >> + maxItems: 2 >> - const: xo >> - const: ref >> >> @@ -53,9 +57,28 @@ properties: >> property with phandle to a cpufreq_hw followed by the Domain ID(0/1) >> in the CPU DT node. >> >> +allOf: >> + - if: >> + properties: >> + reg-names: >> + contains: >> + const: qcom,cpufreq-hw-8998 >> + then: >> + properties: >> + reg: >> + minItems: 4 >> + maxItems: 4 >> + reg-names: > > ...here just: > > minItems: 4 > > And you'll need an 'else' clause with 'maxItems: 2' for reg and > reg-names. > Big thank you for that!!! >> + items: >> + - const: "freq-domain0" >> + - const: "freq-domain1" >> + - const: "osm-domain0" >> + - const: "osm-domain1" >> + >> required: >> - compatible >> - reg >> + - reg-names > > You can't make something that was optional now required. (Unless it was > a mistake and all existing users always had 'reg-names'.) > Well, yes. All existing users are already declaring reg-names, no DT changes to do for them. >> - clock-names >> - clocks >> - "#freq-domain-cells" >> -- >> 2.29.2 >> Thanks for the review. A V2 of the entire series will come soon! -- Angelo
diff --git a/Documentation/devicetree/bindings/cpufreq/qcom,cpufreq-hw.yaml b/Documentation/devicetree/bindings/cpufreq/qcom,cpufreq-hw.yaml index 94a56317b14b..f64cea73037e 100644 --- a/Documentation/devicetree/bindings/cpufreq/qcom,cpufreq-hw.yaml +++ b/Documentation/devicetree/bindings/cpufreq/qcom,cpufreq-hw.yaml @@ -23,17 +23,21 @@ properties: - qcom,cpufreq-epss reg: + description: Base address and size of the RBCPR register region minItems: 2 maxItems: 2 reg-names: description: - Frequency domain register region for each domain. - items: - - const: "freq-domain0" - - const: "freq-domain1" + Frequency domain register region for each domain. If OSM programming + does not happen in the bootloader and has to be done in this driver, + then also the OSM domain region osm-domain[0-1] has to be provided. + minItems: 2 + maxItems: 2 clock-names: + minItems: 2 + maxItems: 2 - const: xo - const: ref @@ -53,9 +57,28 @@ properties: property with phandle to a cpufreq_hw followed by the Domain ID(0/1) in the CPU DT node. +allOf: + - if: + properties: + reg-names: + contains: + const: qcom,cpufreq-hw-8998 + then: + properties: + reg: + minItems: 4 + maxItems: 4 + reg-names: + items: + - const: "freq-domain0" + - const: "freq-domain1" + - const: "osm-domain0" + - const: "osm-domain1" + required: - compatible - reg + - reg-names - clock-names - clocks - "#freq-domain-cells"
The OSM programming addition has been done under the qcom,cpufreq-hw-8998 compatible name: specify the requirement of two additional register spaces for this functionality. Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> --- .../bindings/cpufreq/qcom,cpufreq-hw.yaml | 31 ++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-)