diff mbox series

[1/2] dt-bindings: arm-smmu: Document SM61[12]5 GPU SMMU

Message ID 20230315-topic-kamorta_adrsmmu-v1-1-d1c0dea90bd9@linaro.org
State New
Headers
Series SM6115 GPU SMMU |

Commit Message

Konrad Dybcio March 15, 2023, 10:52 a.m. UTC 
  Both of these SoCs have a Qualcomm MMU500 implementation of SMMU
in front of their GPUs that expect 3 clocks. Both of them also have
an APPS SMMU that expects no clocks. Remove qcom,sm61[12]5-smmu-500
from the "no clocks" list (intentionally 'breaking' the schema checks
of APPS SMMU, as now it *can* accept clocks - with the current
structure of this file it would have taken a wastefully-long time to
sort this out properly..) and add necessary yaml to describe the
clocks required by the GPU SMMUs.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 .../devicetree/bindings/iommu/arm,smmu.yaml        | 28 ++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

Comments

Krzysztof Kozlowski March 16, 2023, 7:29 p.m. UTC | #1 
On 15/03/2023 11:52, Konrad Dybcio wrote:
> Both of these SoCs have a Qualcomm MMU500 implementation of SMMU
> in front of their GPUs that expect 3 clocks. Both of them also have
> an APPS SMMU that expects no clocks. Remove qcom,sm61[12]5-smmu-500
> from the "no clocks" list (intentionally 'breaking' the schema checks
> of APPS SMMU, as now it *can* accept clocks - with the current
> structure of this file it would have taken a wastefully-long time to
> sort this out properly..) and add necessary yaml to describe the
> clocks required by the GPU SMMUs.


> +      properties:
> +        compatible:
> +          items:
> +            - enum:
> +                - qcom,sm6115-smmu-500
> +                - qcom,sm6125-smmu-500
> +            - const: qcom,adreno-smmu
> +            - const: qcom,smmu-500
> +            - const: arm,mmu-500

If you drop the hunk later (from allOf:if), then what clocks do you
expect for non-GPU SMMU?

> +    then:
> +      properties:
> +        clock-names:
> +          items:
> +            - const: mem
> +            - const: hlos
> +            - const: iface
> +
> +        clocks:
> +          items:
> +            - description: GPU memory bus clock
> +            - description: Voter clock required for HLOS SMMU access
> +            - description: Interface clock required for register access
> +
>    # Disallow clocks for all other platforms with specific compatibles
>    - if:
>        properties:
> @@ -394,8 +420,6 @@ allOf:
>                - qcom,sdm845-smmu-500
>                - qcom,sdx55-smmu-500
>                - qcom,sdx65-smmu-500
> -              - qcom,sm6115-smmu-500
> -              - qcom,sm6125-smmu-500
>                - qcom,sm6350-smmu-500
>                - qcom,sm6375-smmu-500
>                - qcom,sm8350-smmu-500
> 

Best regards,
Krzysztof
Konrad Dybcio March 16, 2023, 9:59 p.m. UTC | #2 
On 16.03.2023 20:29, Krzysztof Kozlowski wrote:
> On 15/03/2023 11:52, Konrad Dybcio wrote:
>> Both of these SoCs have a Qualcomm MMU500 implementation of SMMU
>> in front of their GPUs that expect 3 clocks. Both of them also have
>> an APPS SMMU that expects no clocks. Remove qcom,sm61[12]5-smmu-500
>> from the "no clocks" list (intentionally 'breaking' the schema checks
>> of APPS SMMU, as now it *can* accept clocks - with the current
>> structure of this file it would have taken a wastefully-long time to
>> sort this out properly..) and add necessary yaml to describe the
>> clocks required by the GPU SMMUs.
> 
> 
>> +      properties:
>> +        compatible:
>> +          items:
>> +            - enum:
>> +                - qcom,sm6115-smmu-500
>> +                - qcom,sm6125-smmu-500
>> +            - const: qcom,adreno-smmu
>> +            - const: qcom,smmu-500
>> +            - const: arm,mmu-500
> 
> If you drop the hunk later (from allOf:if), then what clocks do you
> expect for non-GPU SMMU?
Both 6115 and 6125 require no clocks under the APPS (non-GPU) SMMU.
However, the list below uses a `contains:` which means I'd have
to add a whole another hunk like

	- items:
            - enum:
                - qcom,sm6115-smmu-500
                - qcom,sm6125-smmu-500
            - const: qcom,smmu-500
            - const: arm,mmu-500

and add another level of indentation to the previous one

I figured skipping that was less messy (I think we discussed this
once as well), but if you prefer to keep it strict, I can.

Konrad
	
> 
>> +    then:
>> +      properties:
>> +        clock-names:
>> +          items:
>> +            - const: mem
>> +            - const: hlos
>> +            - const: iface
>> +
>> +        clocks:
>> +          items:
>> +            - description: GPU memory bus clock
>> +            - description: Voter clock required for HLOS SMMU access
>> +            - description: Interface clock required for register access
>> +
>>    # Disallow clocks for all other platforms with specific compatibles
>>    - if:
>>        properties:
>> @@ -394,8 +420,6 @@ allOf:
>>                - qcom,sdm845-smmu-500
>>                - qcom,sdx55-smmu-500
>>                - qcom,sdx65-smmu-500
>> -              - qcom,sm6115-smmu-500
>> -              - qcom,sm6125-smmu-500
>>                - qcom,sm6350-smmu-500
>>                - qcom,sm6375-smmu-500
>>                - qcom,sm8350-smmu-500
>>
> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski March 17, 2023, 8:32 a.m. UTC | #3 
On 16/03/2023 22:59, Konrad Dybcio wrote:
> 
> 
> On 16.03.2023 20:29, Krzysztof Kozlowski wrote:
>> On 15/03/2023 11:52, Konrad Dybcio wrote:
>>> Both of these SoCs have a Qualcomm MMU500 implementation of SMMU
>>> in front of their GPUs that expect 3 clocks. Both of them also have
>>> an APPS SMMU that expects no clocks. Remove qcom,sm61[12]5-smmu-500
>>> from the "no clocks" list (intentionally 'breaking' the schema checks
>>> of APPS SMMU, as now it *can* accept clocks - with the current
>>> structure of this file it would have taken a wastefully-long time to
>>> sort this out properly..) and add necessary yaml to describe the
>>> clocks required by the GPU SMMUs.
>>
>>
>>> +      properties:
>>> +        compatible:
>>> +          items:
>>> +            - enum:
>>> +                - qcom,sm6115-smmu-500
>>> +                - qcom,sm6125-smmu-500
>>> +            - const: qcom,adreno-smmu
>>> +            - const: qcom,smmu-500
>>> +            - const: arm,mmu-500
>>
>> If you drop the hunk later (from allOf:if), then what clocks do you
>> expect for non-GPU SMMU?
> Both 6115 and 6125 require no clocks under the APPS (non-GPU) SMMU.
> However, the list below uses a `contains:` which means I'd have
> to add a whole another hunk like
> 
> 	- items:
>             - enum:
>                 - qcom,sm6115-smmu-500
>                 - qcom,sm6125-smmu-500
>             - const: qcom,smmu-500
>             - const: arm,mmu-500
> 
> and add another level of indentation to the previous one
> 
> I figured skipping that was less messy (I think we discussed this
> once as well), but if you prefer to keep it strict, I can.

Nah, ok, it's fine.

Best regards,
Krzysztof
Krzysztof Kozlowski March 17, 2023, 8:32 a.m. UTC | #4 
On 15/03/2023 11:52, Konrad Dybcio wrote:
> Both of these SoCs have a Qualcomm MMU500 implementation of SMMU
> in front of their GPUs that expect 3 clocks. Both of them also have
> an APPS SMMU that expects no clocks. Remove qcom,sm61[12]5-smmu-500
> from the "no clocks" list (intentionally 'breaking' the schema checks
> of APPS SMMU, as now it *can* accept clocks - with the current
> structure of this file it would have taken a wastefully-long time to
> sort this out properly..) and add necessary yaml to describe the
> clocks required by the GPU SMMUs.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
index a6224b7e5310..62c7a5ff148e 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
@@ -79,6 +79,8 @@  properties:
         items:
           - enum:
               - qcom,sc7280-smmu-500
+              - qcom,sm6115-smmu-500
+              - qcom,sm6125-smmu-500
               - qcom,sm8150-smmu-500
               - qcom,sm8250-smmu-500
               - qcom,sm8350-smmu-500
@@ -375,6 +377,30 @@  allOf:
             - description: interface clock required to access smmu's registers
                 through the TCU's programming interface.
 
+  - if:
+      properties:
+        compatible:
+          items:
+            - enum:
+                - qcom,sm6115-smmu-500
+                - qcom,sm6125-smmu-500
+            - const: qcom,adreno-smmu
+            - const: qcom,smmu-500
+            - const: arm,mmu-500
+    then:
+      properties:
+        clock-names:
+          items:
+            - const: mem
+            - const: hlos
+            - const: iface
+
+        clocks:
+          items:
+            - description: GPU memory bus clock
+            - description: Voter clock required for HLOS SMMU access
+            - description: Interface clock required for register access
+
   # Disallow clocks for all other platforms with specific compatibles
   - if:
       properties:
@@ -394,8 +420,6 @@  allOf:
               - qcom,sdm845-smmu-500
               - qcom,sdx55-smmu-500
               - qcom,sdx65-smmu-500
-              - qcom,sm6115-smmu-500
-              - qcom,sm6125-smmu-500
               - qcom,sm6350-smmu-500
               - qcom,sm6375-smmu-500
               - qcom,sm8350-smmu-500