| Message ID | 20221114104222.36329-2-konrad.dybcio@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/9] dt-bindings: arm-smmu: Allow up to 3 power-domains | expand |
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 >>>
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 + maxItems: 3 nvidia,memory-controller: description: |
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(-)