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 <[email protected]>
---
Cc: Marijn Suijten <[email protected]>
Cc: Dmitry Baryshkov <[email protected]>
Cc: Konrad Dybcio <[email protected]>
---
.../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
+ - 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
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 <[email protected]>
Reviewed-by: Marijn Suijten <[email protected]>
But...
> ---
>
> Cc: Marijn Suijten <[email protected]>
> Cc: Dmitry Baryshkov <[email protected]>
> Cc: Konrad Dybcio <[email protected]>
> ---
> .../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 <[email protected]>
>
> Reviewed-by: Marijn Suijten <[email protected]>
>
> But...
>
>> ---
>>
>> Cc: Marijn Suijten <[email protected]>
>> Cc: Dmitry Baryshkov <[email protected]>
>> Cc: Konrad Dybcio <[email protected]>
>> ---
>> .../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 <[email protected]>
> >
> > Reviewed-by: Marijn Suijten <[email protected]>
> >
> > But...
> >
> >> ---
> >>
> >> Cc: Marijn Suijten <[email protected]>
> >> Cc: Dmitry Baryshkov <[email protected]>
> >> Cc: Konrad Dybcio <[email protected]>
> >> ---
> >> .../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 <[email protected]>
>>>
>>> Reviewed-by: Marijn Suijten <[email protected]>
>>>
>>> But...
>>>
>>>> ---
>>>>
>>>> Cc: Marijn Suijten <[email protected]>
>>>> Cc: Dmitry Baryshkov <[email protected]>
>>>> Cc: Konrad Dybcio <[email protected]>
>>>> ---
>>>> .../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 <[email protected]>
>
> ---
>
> Cc: Marijn Suijten <[email protected]>
> Cc: Dmitry Baryshkov <[email protected]>
> Cc: Konrad Dybcio <[email protected]>
> ---
> .../devicetree/bindings/iommu/arm,smmu.yaml | 28 +++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
Acked-by: Rob Herring <[email protected]>
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 <[email protected]>
>
> ---
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 <[email protected]>
> >
> > ---
>
> 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,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev