Re: [PATCH 13/13] dt-bindings: cpufreq: qcom-hw: Add bindings for 8998

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxx>
---
  .../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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux