dt-bindings: arm-smmu: disallow clocks when not used

  Disallow clocks for variants other than:
1. SMMUs with platform-specific compatibles which list explicit clocks
   and clock-names,
2. SMMUs using only generic compatibles, e.g. arm,mmu-500, which have a
   variable clocks on different implementations.

This requires such variants with platform-specific compatible, to
explicitly list the clocks or omit them, making the binding more
constraint.

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

---

Cc: Marijn Suijten <marijn.suijten@somainline.org>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 .../devicetree/bindings/iommu/arm,smmu.yaml   | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)
  

Is this missing a cc to linux-arm-msm?

On 2022-12-22 10:23:55, Krzysztof Kozlowski wrote:
> Disallow clocks for variants other than:
> 1. SMMUs with platform-specific compatibles which list explicit clocks
>    and clock-names,
> 2. SMMUs using only generic compatibles, e.g. arm,mmu-500, which have a
>    variable clocks on different implementations.
> 
> This requires such variants with platform-specific compatible, to
> explicitly list the clocks or omit them, making the binding more
> constraint.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>

But...

> ---
> 
> Cc: Marijn Suijten <marijn.suijten@somainline.org>
> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  .../devicetree/bindings/iommu/arm,smmu.yaml   | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> index 895ec8418465..0d88395e43ad 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> @@ -367,6 +367,34 @@ allOf:
>              - description: interface clock required to access smmu's registers
>                  through the TCU's programming interface.
>  
> +  # Disallow clocks for all other platforms with specific compatibles
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - cavium,smmu-v2
> +              - marvell,ap806-smmu-500
> +              - nvidia,smmu-500
> +              - qcom,qcm2290-smmu-500
> +              - qcom,qdu1000-smmu-500
> +              - qcom,sc7180-smmu-500

Hmm, sc7280 has two SMMUs.  The one for Adreno has clocks and a PD, the
one for APPS has neither.  Same story on sm8[12]50.  Aren't those going
to trip up the other `if` that requires clocks in both scenarios?

Note that the Adreno SMMUs have (or will get when we/Konrad submit
support for it) the "qcom,adreno-smmu" compatible to distinguish them.

- Marijn

> +              - qcom,sc8180x-smmu-500
> +              - qcom,sc8280xp-smmu-500
> +              - qcom,sdm670-smmu-500
> +              - qcom,sdm845-smmu-500
> +              - qcom,sdx55-smmu-500
> +              - qcom,sdx65-smmu-500
> +              - qcom,sm6115-smmu-500
> +              - qcom,sm6350-smmu-500
> +              - qcom,sm6375-smmu-500
> +              - qcom,sm8350-smmu-500
> +              - qcom,sm8450-smmu-500
> +    then:
> +      properties:
> +        clock-names: false
> +        clocks: false
> +
>    - if:
>        properties:
>          compatible:
> -- 
> 2.34.1
>
  

On 22/12/2022 11:16, Marijn Suijten wrote:
> Is this missing a cc to linux-arm-msm?

No, it is not (or maybe but then fix MAINTAINERS). The policy is to use
get_maintainers.pl to CC people.

> 
> On 2022-12-22 10:23:55, Krzysztof Kozlowski wrote:
>> Disallow clocks for variants other than:
>> 1. SMMUs with platform-specific compatibles which list explicit clocks
>>    and clock-names,
>> 2. SMMUs using only generic compatibles, e.g. arm,mmu-500, which have a
>>    variable clocks on different implementations.
>>
>> This requires such variants with platform-specific compatible, to
>> explicitly list the clocks or omit them, making the binding more
>> constraint.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> 
> But...
> 
>> ---
>>
>> Cc: Marijn Suijten <marijn.suijten@somainline.org>
>> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>>  .../devicetree/bindings/iommu/arm,smmu.yaml   | 28 +++++++++++++++++++
>>  1 file changed, 28 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> index 895ec8418465..0d88395e43ad 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> @@ -367,6 +367,34 @@ allOf:
>>              - description: interface clock required to access smmu's registers
>>                  through the TCU's programming interface.
>>  
>> +  # Disallow clocks for all other platforms with specific compatibles
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - cavium,smmu-v2
>> +              - marvell,ap806-smmu-500
>> +              - nvidia,smmu-500
>> +              - qcom,qcm2290-smmu-500
>> +              - qcom,qdu1000-smmu-500
>> +              - qcom,sc7180-smmu-500
> 
> Hmm, sc7280 has two SMMUs.  The one for Adreno has clocks and a PD, the

sc7280 is not here, so what is the mistake you see?

> one for APPS has neither.  Same story on sm8[12]50.  Aren't those going
> to trip up the other `if` that requires clocks in both scenarios?

They are not here either, so what is the error?

> 
> Note that the Adreno SMMUs have (or will get when we/Konrad submit
> support for it) the "qcom,adreno-smmu" compatible to distinguish them.
> 
> - Marijn
> 


Best regards,
Krzysztof
  

On 2022-12-22 11:36:16, Krzysztof Kozlowski wrote:
> On 22/12/2022 11:16, Marijn Suijten wrote:
> > Is this missing a cc to linux-arm-msm?
> 
> No, it is not (or maybe but then fix MAINTAINERS). The policy is to use
> get_maintainers.pl to CC people.

Yes, that is the question: is it in MANTAINERS and if not, why not?

> > On 2022-12-22 10:23:55, Krzysztof Kozlowski wrote:
> >> Disallow clocks for variants other than:
> >> 1. SMMUs with platform-specific compatibles which list explicit clocks
> >>    and clock-names,
> >> 2. SMMUs using only generic compatibles, e.g. arm,mmu-500, which have a
> >>    variable clocks on different implementations.
> >>
> >> This requires such variants with platform-specific compatible, to
> >> explicitly list the clocks or omit them, making the binding more
> >> constraint.
> >>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > 
> > Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> > 
> > But...
> > 
> >> ---
> >>
> >> Cc: Marijn Suijten <marijn.suijten@somainline.org>
> >> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >> Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
> >> ---
> >>  .../devicetree/bindings/iommu/arm,smmu.yaml   | 28 +++++++++++++++++++
> >>  1 file changed, 28 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> >> index 895ec8418465..0d88395e43ad 100644
> >> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> >> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> >> @@ -367,6 +367,34 @@ allOf:
> >>              - description: interface clock required to access smmu's registers
> >>                  through the TCU's programming interface.
> >>  
> >> +  # Disallow clocks for all other platforms with specific compatibles
> >> +  - if:
> >> +      properties:
> >> +        compatible:
> >> +          contains:
> >> +            enum:
> >> +              - cavium,smmu-v2
> >> +              - marvell,ap806-smmu-500
> >> +              - nvidia,smmu-500
> >> +              - qcom,qcm2290-smmu-500
> >> +              - qcom,qdu1000-smmu-500
> >> +              - qcom,sc7180-smmu-500
> > 
> > Hmm, sc7280 has two SMMUs.  The one for Adreno has clocks and a PD, the
> 
> sc7280 is not here, so what is the mistake you see?

sc7280 has two IOMMU nodes.  One with clocks (should not be in this
list), the other doesn't have clocks (should be in this list).

How do you want to address that?

> > one for APPS has neither.  Same story on sm8[12]50.  Aren't those going
> > to trip up the other `if` that requires clocks in both scenarios?
> 
> They are not here either, so what is the error?

Ditto.

> > Note that the Adreno SMMUs have (or will get when we/Konrad submit
> > support for it) the "qcom,adreno-smmu" compatible to distinguish them.

- Marijn
  

On 22/12/2022 14:33, Marijn Suijten wrote:
> On 2022-12-22 11:36:16, Krzysztof Kozlowski wrote:
>> On 22/12/2022 11:16, Marijn Suijten wrote:
>>> Is this missing a cc to linux-arm-msm?
>>
>> No, it is not (or maybe but then fix MAINTAINERS). The policy is to use
>> get_maintainers.pl to CC people.
> 
> Yes, that is the question: is it in MANTAINERS and if not, why not?

You can check by yourself if it is there.

Why not? I don't know. Could be that no one ever added it there.

> 
>>> On 2022-12-22 10:23:55, Krzysztof Kozlowski wrote:
>>>> Disallow clocks for variants other than:
>>>> 1. SMMUs with platform-specific compatibles which list explicit clocks
>>>>    and clock-names,
>>>> 2. SMMUs using only generic compatibles, e.g. arm,mmu-500, which have a
>>>>    variable clocks on different implementations.
>>>>
>>>> This requires such variants with platform-specific compatible, to
>>>> explicitly list the clocks or omit them, making the binding more
>>>> constraint.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>
>>> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
>>>
>>> But...
>>>
>>>> ---
>>>>
>>>> Cc: Marijn Suijten <marijn.suijten@somainline.org>
>>>> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>> Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
>>>> ---
>>>>  .../devicetree/bindings/iommu/arm,smmu.yaml   | 28 +++++++++++++++++++
>>>>  1 file changed, 28 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>>>> index 895ec8418465..0d88395e43ad 100644
>>>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>>>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>>>> @@ -367,6 +367,34 @@ allOf:
>>>>              - description: interface clock required to access smmu's registers
>>>>                  through the TCU's programming interface.
>>>>  
>>>> +  # Disallow clocks for all other platforms with specific compatibles
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            enum:
>>>> +              - cavium,smmu-v2
>>>> +              - marvell,ap806-smmu-500
>>>> +              - nvidia,smmu-500
>>>> +              - qcom,qcm2290-smmu-500
>>>> +              - qcom,qdu1000-smmu-500
>>>> +              - qcom,sc7180-smmu-500
>>>
>>> Hmm, sc7280 has two SMMUs.  The one for Adreno has clocks and a PD, the
>>
>> sc7280 is not here, so what is the mistake you see?
> 
> sc7280 has two IOMMU nodes.  One with clocks (should not be in this
> list), the other doesn't have clocks (should be in this list).
> 
> How do you want to address that?

No, because it is the same compatible.

Best regards,
Krzysztof
  

On 2022-12-22 15:03:20, Krzysztof Kozlowski wrote:
> On 22/12/2022 14:33, Marijn Suijten wrote:
> > On 2022-12-22 11:36:16, Krzysztof Kozlowski wrote:
> >> On 22/12/2022 11:16, Marijn Suijten wrote:
> >>> Is this missing a cc to linux-arm-msm?
> >>
> >> No, it is not (or maybe but then fix MAINTAINERS). The policy is to use
> >> get_maintainers.pl to CC people.
> > 
> > Yes, that is the question: is it in MANTAINERS and if not, why not?
> 
> You can check by yourself if it is there.

It's not there.

> Why not? I don't know. Could be that no one ever added it there.

Let's leave it like that then :)

<snip>

> > sc7280 has two IOMMU nodes.  One with clocks (should not be in this
> > list), the other doesn't have clocks (should be in this list).
> > 
> > How do you want to address that?
> 
> No, because it is the same compatible.

That is the point.  We can tell them apart based on the presence of
"qcom,adreno-smmu" though.  But if it is not spitting out any errors
right now, let's not bother.

- Marijn
  

On Thu, 22 Dec 2022 10:23:55 +0100, Krzysztof Kozlowski wrote:
> Disallow clocks for variants other than:
> 1. SMMUs with platform-specific compatibles which list explicit clocks
>    and clock-names,
> 2. SMMUs using only generic compatibles, e.g. arm,mmu-500, which have a
>    variable clocks on different implementations.
> 
> This requires such variants with platform-specific compatible, to
> explicitly list the clocks or omit them, making the binding more
> constraint.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---
> 
> Cc: Marijn Suijten <marijn.suijten@somainline.org>
> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  .../devicetree/bindings/iommu/arm,smmu.yaml   | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>
  

On 22/12/2022 10:23, Krzysztof Kozlowski wrote:
> Disallow clocks for variants other than:
> 1. SMMUs with platform-specific compatibles which list explicit clocks
>    and clock-names,
> 2. SMMUs using only generic compatibles, e.g. arm,mmu-500, which have a
>    variable clocks on different implementations.
> 
> This requires such variants with platform-specific compatible, to
> explicitly list the clocks or omit them, making the binding more
> constraint.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---

Will, Robin, Joerg,

Anyone willing to pick up this patch?

Best regards,
Krzysztof
  

On Thu, Jan 12, 2023 at 02:53:20PM +0100, Krzysztof Kozlowski wrote:
> On 22/12/2022 10:23, Krzysztof Kozlowski wrote:
> > Disallow clocks for variants other than:
> > 1. SMMUs with platform-specific compatibles which list explicit clocks
> >    and clock-names,
> > 2. SMMUs using only generic compatibles, e.g. arm,mmu-500, which have a
> >    variable clocks on different implementations.
> > 
> > This requires such variants with platform-specific compatible, to
> > explicitly list the clocks or omit them, making the binding more
> > constraint.
> > 
> > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > 
> > ---
> 
> Will, Robin, Joerg,
> 
> Anyone willing to pick up this patch?

Sure, I'll do an SMMU bindings pass shortly for 6.3

Will
  

On Thu, 22 Dec 2022 10:23:55 +0100, Krzysztof Kozlowski wrote:
> Disallow clocks for variants other than:
> 1. SMMUs with platform-specific compatibles which list explicit clocks
>    and clock-names,
> 2. SMMUs using only generic compatibles, e.g. arm,mmu-500, which have a
>    variable clocks on different implementations.
> 
> This requires such variants with platform-specific compatible, to
> explicitly list the clocks or omit them, making the binding more
> constraint.
> 
> [...]

Applied to will (for-joerg/arm-smmu/bindings), thanks!

[1/1] dt-bindings: arm-smmu: disallow clocks when not used
      https://git.kernel.org/will/c/3a3f20bae0ce

Cheers,