Re: [PATCH v2 1/9] dt-bindings: arm-smmu: Allow up to 3 power-domains

From: Konrad Dybcio
Date: Tue Nov 15 2022 - 10:04:51 EST



On 15/11/2022 14:43, Robin Murphy wrote:
On 2022-11-15 13:06, Konrad Dybcio wrote:


On 15/11/2022 14:00, Krzysztof Kozlowski wrote:
On 15/11/2022 13:54, Konrad Dybcio wrote:


On 14/11/2022 17:58, Krzysztof Kozlowski wrote:
On 14/11/2022 16:53, Konrad Dybcio wrote:

On 14/11/2022 14:00, Krzysztof Kozlowski wrote:
On 14/11/2022 12:17, Konrad Dybcio wrote:
On 14/11/2022 12:01, Krzysztof Kozlowski wrote:
On 14/11/2022 11:42, Konrad Dybcio wrote:
Some SMMUs require that a vote is held on as much as 3 separate PDs
(hello Qualcomm). Allow it in bindings.

Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
---
Changes since v1:
- Add minItems

     Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 3 ++-
     1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
index 9066e6df1ba1..82bc696de662 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
@@ -159,7 +159,8 @@ properties:
               through the TCU's programming interface.
       power-domains:
-    maxItems: 1
+    minItems: 0
It cannot be 0.

minItems: 1

Anyway you still need to restrict it per variant, as I said in previous
version.
Hm.. I'm not entirely sure what you mean.. Should I add a list of
compatibles
Yes and limit it to maxItems: 1 for "else".

I tried adding:



     - if:
         properties:
           compatible:
             contains:
               enum:
                 - qcom,sm6375-smmu-500
       then:
         properties:
           power-domains:
             minItems: 3
             maxItems: 3
       else:
         properties:
           power-domains:
             maxItems: 1


Right under the nvidia reg if-else in the allOf, but dtbs_check throws
errors like:


/home/konrad/linux/arch/arm64/boot/dts/qcom/msm8998-sony-xperia-yoshino-poplar.dtb:
iommu@5040000: 'power-domains' does not match any of the regexes:
'pinctrl-[0-9]+'


Any clues as to why?

I don't know what code do you have there, but generic pattern is:

https://elixir.bootlin.com/linux/v5.19-rc6/source/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml#L38

I tried many things, but I still don't seem to get a hang of it.. Here's
my current diff rebased on top of Dmitry's recent cleanups (available at
[1])


diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
index 28f5720824cd..55759aebc4a0 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
@@ -200,7 +200,7 @@ properties:
       maxItems: 7

     power-domains:

As I mentioned before - minItems: 1.
But not all SMMUs require a power domain :/

Right, so it's not a required property. However if it *is* present, then it needs to reference at least one power domain, because having an empty property, i.e.:

    power-domains = <>;

or
    power-domains;

makes no sense whatsoever.

Thanks,
Robin.
OHHHH!

That was the missing piece that made it click for me!
Thanks Krzysztof, Robin for guiding me through this.

Konrad



Just like the link I gave you.

-    maxItems: 1
+    maxItems: 3

     nvidia,memory-controller:
       description: |
@@ -364,6 +364,26 @@ allOf:
               - description: interface clock required to access smmu's
registers
                   through the TCU's programming interface.

+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,sm6375-smmu-500
+    then:
+      properties:
+        power-domains:
+          items:
+            - description: SNoC MMU TBU RT GDSC
+            - description: SNoC MMU TBU NRT GDSC
+            - description: SNoC TURING MMU TBU0 GDSC
+
+      required:
+        - power-domains
+    else:
+      properties:
+        power-domains:
+          maxItems: 1
+
   examples:
     - |+
       /* SMMU with stream matching or stream indexing */


In my eyes, this should work, but I still get errors like:

/home/konrad/linux/arch/arm64/boot/dts/qcom/sm8250-hdk.dtb:
iommu@3da0000: power-domains: [[108, 0]] is too short

as if the else: path was never taken..

It was, but the top-level property said that minItems=3 (implicitly), so
it is too short.
So the top-level properties take precedence over the ones that come from the if-then-else?? Ugh.

Konrad

Best regards,
Krzysztof