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

  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@linaro.org>
---
Changes since v1:
- Add minItems

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

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@linaro.org>
> ---
> 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.

Best regards,
Krzysztof
  

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@linaro.org>
>> ---
>> 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
that are allowed to have 3 power-domains and leave it as it was before 
in the
'else' case?


Konrad

>
> Best regards,
> Krzysztof
>
  

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@linaro.org>
>>> ---
>>> 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".

> that are allowed to have 3 power-domains and leave it as it was before 
> in the
> 'else' case?

Best regards,
Krzysztof
  

On Mon, 14 Nov 2022 11:42:14 +0100, 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@linaro.org>
> ---
> Changes since v1:
> - Add minItems
> 
>  Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: properties:power-domains:minItems: 0 is less than the minimum of 1
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iommu/arm,smmu.yaml: properties:power-domains: 'anyOf' conditional failed, one must be fixed:
	'minItems' is not one of ['maxItems', 'description', 'deprecated']
		hint: Only "maxItems" is required for a single entry if there are no constraints defined for the values.
	'minItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
	'maxItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
	1 was expected
		hint: Only "maxItems" is required for a single entry if there are no constraints defined for the values.
	0 is less than the minimum of 1
		hint: Arrays must be described with a combination of minItems/maxItems/items
	hint: cell array properties must define how many entries and what the entries are when there is more than one entry.
	from schema $id: http://devicetree.org/meta-schemas/power-domain.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
  

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@linaro.org>
>>>> ---
>>>> 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?


Konrad

>
>> that are allowed to have 3 power-domains and leave it as it was before
>> in the
>> 'else' case?
> Best regards,
> Krzysztof
>
  

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@linaro.org>
>>>>> ---
>>>>> 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

Best regards,
Krzysztof
  

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@linaro.org>
>>>>>> ---
>>>>>> 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:
-    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..

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-joerg/arm-smmu/bindings

Konrad
> Best regards,
> Krzysztof
>
  

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@linaro.org>
>>>>>>> ---
>>>>>>> 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.

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.

Best regards,
Krzysztof
  

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@linaro.org>
>>>>>>>> ---
>>>>>>>> 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 :/

> 
> 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
>
  

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@linaro.org>
>>>>>>>>> ---
>>>>>>>>> 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.

> 
>>
>> 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
>>
  

On 15/11/2022 14:06, Konrad Dybcio wrote:>> 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 :/

It does not matter. This does not require power-domains.

> 
>>
>> 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.

It's a sum of them. Top level is expected to define the widest
constraints and if-then-else narrows them per variants.

Best regards,
Krzysztof
  

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@linaro.org>
>>>>>>>>>> ---
>>>>>>>>>> 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
>>>