diff mbox series

[v2,2/5] dt-bindings: mmc: sdhci-msm: constrain reg-names perp variants

Message ID 20220711082940.39539-3-krzysztof.kozlowski@linaro.org
State Superseded
Headers show
Series dt-bindings: mmc: / ARM: qcom: correct reg-names and clock entries | expand

Commit Message

Krzysztof Kozlowski July 11, 2022, 8:29 a.m. UTC
The entries in arrays must have fixed order, so the bindings and Linux
driver expecting various combinations of 'reg' addresses was never
actually conforming to guidelines.

The 'core' reg entry is valid only for SDCC v4 and lower, so disallow it
in SDCC v5.  SDCC v4 supports CQE and ICE, so allow them, even though
the qcom,sdhci-msm-v4 compatible is used also for earlier SoCs with SDCC
v2 or v3, so it is not entirely accurate.

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

---

Changes since v1:
1. Rework the patch based on Doug's feedback.
---
 .../devicetree/bindings/mmc/sdhci-msm.yaml    | 61 ++++++++++++-------
 1 file changed, 38 insertions(+), 23 deletions(-)

Comments

Doug Anderson July 11, 2022, 2:52 p.m. UTC | #1
Hi

On Mon, Jul 11, 2022 at 1:29 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> The entries in arrays must have fixed order, so the bindings and Linux
> driver expecting various combinations of 'reg' addresses was never
> actually conforming to guidelines.
>
> The 'core' reg entry is valid only for SDCC v4 and lower, so disallow it
> in SDCC v5.  SDCC v4 supports CQE and ICE, so allow them, even though
> the qcom,sdhci-msm-v4 compatible is used also for earlier SoCs with SDCC
> v2 or v3, so it is not entirely accurate.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> ---
>
> Changes since v1:
> 1. Rework the patch based on Doug's feedback.
> ---
>  .../devicetree/bindings/mmc/sdhci-msm.yaml    | 61 ++++++++++++-------
>  1 file changed, 38 insertions(+), 23 deletions(-)

In the ${SUBJECT} I'm not sure what a "perp variant" is. Is that a
typo or just a phrase I'm not aware of?


> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> index fc6e5221985a..2f0fdd65e908 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> @@ -49,33 +49,11 @@ properties:
>
>    reg:
>      minItems: 1
> -    items:
> -      - description: Host controller register map
> -      - description: SD Core register map
> -      - description: CQE register map
> -      - description: Inline Crypto Engine register map
> +    maxItems: 4
>
>    reg-names:
>      minItems: 1
>      maxItems: 4
> -    oneOf:
> -      - items:
> -          - const: hc
> -      - items:
> -          - const: hc
> -          - const: core
> -      - items:
> -          - const: hc
> -          - const: cqhci
> -      - items:
> -          - const: hc
> -          - const: cqhci
> -          - const: ice
> -      - items:
> -          - const: hc
> -          - const: core
> -          - const: cqhci
> -          - const: ice
>
>    clocks:
>      minItems: 3
> @@ -177,6 +155,43 @@ required:
>  allOf:
>    - $ref: mmc-controller.yaml#
>
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sdhci-msm-v4
> +    then:
> +      properties:
> +        reg:
> +          minItems: 2
> +          items:
> +            - description: Host controller register map
> +            - description: SD Core register map
> +            - description: CQE register map
> +            - description: Inline Crypto Engine register map
> +        reg-names:
> +          minItems: 2
> +          items:
> +            - const: hc
> +            - const: core
> +            - const: cqhci
> +            - const: ice
> +    else:
> +      properties:
> +        reg:
> +          minItems: 1
> +          items:
> +            - description: Host controller register map
> +            - description: CQE register map
> +            - description: Inline Crypto Engine register map
> +        reg-names:
> +          minItems: 1
> +          items:
> +            - const: hc
> +            - const: cqhci
> +            - const: ice

Do you need to set "maxItems" here? If you don't then will it inherit
the maxItems of 4 from above?

-Doug
Krzysztof Kozlowski July 11, 2022, 2:53 p.m. UTC | #2
On 11/07/2022 16:52, Doug Anderson wrote:
> Hi
> 
> On Mon, Jul 11, 2022 at 1:29 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> The entries in arrays must have fixed order, so the bindings and Linux
>> driver expecting various combinations of 'reg' addresses was never
>> actually conforming to guidelines.
>>
>> The 'core' reg entry is valid only for SDCC v4 and lower, so disallow it
>> in SDCC v5.  SDCC v4 supports CQE and ICE, so allow them, even though
>> the qcom,sdhci-msm-v4 compatible is used also for earlier SoCs with SDCC
>> v2 or v3, so it is not entirely accurate.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>
>> ---
>>
>> Changes since v1:
>> 1. Rework the patch based on Doug's feedback.
>> ---
>>  .../devicetree/bindings/mmc/sdhci-msm.yaml    | 61 ++++++++++++-------
>>  1 file changed, 38 insertions(+), 23 deletions(-)
> 
> In the ${SUBJECT} I'm not sure what a "perp variant" is. Is that a
> typo or just a phrase I'm not aware of?

Should be:
"per variants"

> 
> 
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>> index fc6e5221985a..2f0fdd65e908 100644
>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>> @@ -49,33 +49,11 @@ properties:
>>
>>    reg:
>>      minItems: 1
>> -    items:
>> -      - description: Host controller register map
>> -      - description: SD Core register map
>> -      - description: CQE register map
>> -      - description: Inline Crypto Engine register map
>> +    maxItems: 4
>>
>>    reg-names:
>>      minItems: 1
>>      maxItems: 4
>> -    oneOf:
>> -      - items:
>> -          - const: hc
>> -      - items:
>> -          - const: hc
>> -          - const: core
>> -      - items:
>> -          - const: hc
>> -          - const: cqhci
>> -      - items:
>> -          - const: hc
>> -          - const: cqhci
>> -          - const: ice
>> -      - items:
>> -          - const: hc
>> -          - const: core
>> -          - const: cqhci
>> -          - const: ice
>>
>>    clocks:
>>      minItems: 3
>> @@ -177,6 +155,43 @@ required:
>>  allOf:
>>    - $ref: mmc-controller.yaml#
>>
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,sdhci-msm-v4
>> +    then:
>> +      properties:
>> +        reg:
>> +          minItems: 2
>> +          items:
>> +            - description: Host controller register map
>> +            - description: SD Core register map
>> +            - description: CQE register map
>> +            - description: Inline Crypto Engine register map
>> +        reg-names:
>> +          minItems: 2
>> +          items:
>> +            - const: hc
>> +            - const: core
>> +            - const: cqhci
>> +            - const: ice
>> +    else:
>> +      properties:
>> +        reg:
>> +          minItems: 1
>> +          items:
>> +            - description: Host controller register map
>> +            - description: CQE register map
>> +            - description: Inline Crypto Engine register map
>> +        reg-names:
>> +          minItems: 1
>> +          items:
>> +            - const: hc
>> +            - const: cqhci
>> +            - const: ice
> 
> Do you need to set "maxItems" here? If you don't then will it inherit
> the maxItems of 4 from above?

No, items determine the size instead.


Best regards,
Krzysztof
Krzysztof Kozlowski July 12, 2022, 7:02 a.m. UTC | #3
On 11/07/2022 17:11, Doug Anderson wrote:
> Hi,
> 
> On Mon, Jul 11, 2022 at 7:53 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 11/07/2022 16:52, Doug Anderson wrote:
>>> Hi
>>>
>>> On Mon, Jul 11, 2022 at 1:29 AM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> The entries in arrays must have fixed order, so the bindings and Linux
>>>> driver expecting various combinations of 'reg' addresses was never
>>>> actually conforming to guidelines.
>>>>
>>>> The 'core' reg entry is valid only for SDCC v4 and lower, so disallow it
>>>> in SDCC v5.  SDCC v4 supports CQE and ICE, so allow them, even though
>>>> the qcom,sdhci-msm-v4 compatible is used also for earlier SoCs with SDCC
>>>> v2 or v3, so it is not entirely accurate.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>
>>>> ---
>>>>
>>>> Changes since v1:
>>>> 1. Rework the patch based on Doug's feedback.
>>>> ---
>>>>  .../devicetree/bindings/mmc/sdhci-msm.yaml    | 61 ++++++++++++-------
>>>>  1 file changed, 38 insertions(+), 23 deletions(-)
>>>
>>> In the ${SUBJECT} I'm not sure what a "perp variant" is. Is that a
>>> typo or just a phrase I'm not aware of?
>>
>> Should be:
>> "per variants"
>>
>>>
>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>>>> index fc6e5221985a..2f0fdd65e908 100644
>>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>>>> @@ -49,33 +49,11 @@ properties:
>>>>
>>>>    reg:
>>>>      minItems: 1
>>>> -    items:
>>>> -      - description: Host controller register map
>>>> -      - description: SD Core register map
>>>> -      - description: CQE register map
>>>> -      - description: Inline Crypto Engine register map
>>>> +    maxItems: 4
>>>>
>>>>    reg-names:
>>>>      minItems: 1
>>>>      maxItems: 4
>>>> -    oneOf:
>>>> -      - items:
>>>> -          - const: hc
>>>> -      - items:
>>>> -          - const: hc
>>>> -          - const: core
>>>> -      - items:
>>>> -          - const: hc
>>>> -          - const: cqhci
>>>> -      - items:
>>>> -          - const: hc
>>>> -          - const: cqhci
>>>> -          - const: ice
>>>> -      - items:
>>>> -          - const: hc
>>>> -          - const: core
>>>> -          - const: cqhci
>>>> -          - const: ice
>>>>
>>>>    clocks:
>>>>      minItems: 3
>>>> @@ -177,6 +155,43 @@ required:
>>>>  allOf:
>>>>    - $ref: mmc-controller.yaml#
>>>>
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            enum:
>>>> +              - qcom,sdhci-msm-v4
>>>> +    then:
>>>> +      properties:
>>>> +        reg:
>>>> +          minItems: 2
>>>> +          items:
>>>> +            - description: Host controller register map
>>>> +            - description: SD Core register map
>>>> +            - description: CQE register map
>>>> +            - description: Inline Crypto Engine register map
>>>> +        reg-names:
>>>> +          minItems: 2
>>>> +          items:
>>>> +            - const: hc
>>>> +            - const: core
>>>> +            - const: cqhci
>>>> +            - const: ice
>>>> +    else:
>>>> +      properties:
>>>> +        reg:
>>>> +          minItems: 1
>>>> +          items:
>>>> +            - description: Host controller register map
>>>> +            - description: CQE register map
>>>> +            - description: Inline Crypto Engine register map
>>>> +        reg-names:
>>>> +          minItems: 1
>>>> +          items:
>>>> +            - const: hc
>>>> +            - const: cqhci
>>>> +            - const: ice
>>>
>>> Do you need to set "maxItems" here? If you don't then will it inherit
>>> the maxItems of 4 from above?
>>
>> No, items determine the size instead.
> 
> Can you just remove the "maxItems" from above then? Does it buy us anything?

There is no maxItems directly here...

> 
> In any case, with the ${SUBJECT} fixed:
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>


Best regards,
Krzysztof
Doug Anderson July 12, 2022, 2:29 p.m. UTC | #4
Hi,

On Tue, Jul 12, 2022 at 12:02 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 11/07/2022 17:11, Doug Anderson wrote:
> > Hi,
> >
> > On Mon, Jul 11, 2022 at 7:53 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 11/07/2022 16:52, Doug Anderson wrote:
> >>> Hi
> >>>
> >>> On Mon, Jul 11, 2022 at 1:29 AM Krzysztof Kozlowski
> >>> <krzysztof.kozlowski@linaro.org> wrote:
> >>>>
> >>>> The entries in arrays must have fixed order, so the bindings and Linux
> >>>> driver expecting various combinations of 'reg' addresses was never
> >>>> actually conforming to guidelines.
> >>>>
> >>>> The 'core' reg entry is valid only for SDCC v4 and lower, so disallow it
> >>>> in SDCC v5.  SDCC v4 supports CQE and ICE, so allow them, even though
> >>>> the qcom,sdhci-msm-v4 compatible is used also for earlier SoCs with SDCC
> >>>> v2 or v3, so it is not entirely accurate.
> >>>>
> >>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>>>
> >>>> ---
> >>>>
> >>>> Changes since v1:
> >>>> 1. Rework the patch based on Doug's feedback.
> >>>> ---
> >>>>  .../devicetree/bindings/mmc/sdhci-msm.yaml    | 61 ++++++++++++-------
> >>>>  1 file changed, 38 insertions(+), 23 deletions(-)
> >>>
> >>> In the ${SUBJECT} I'm not sure what a "perp variant" is. Is that a
> >>> typo or just a phrase I'm not aware of?
> >>
> >> Should be:
> >> "per variants"
> >>
> >>>
> >>>
> >>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> >>>> index fc6e5221985a..2f0fdd65e908 100644
> >>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> >>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> >>>> @@ -49,33 +49,11 @@ properties:
> >>>>
> >>>>    reg:
> >>>>      minItems: 1
> >>>> -    items:
> >>>> -      - description: Host controller register map
> >>>> -      - description: SD Core register map
> >>>> -      - description: CQE register map
> >>>> -      - description: Inline Crypto Engine register map
> >>>> +    maxItems: 4
> >>>>
> >>>>    reg-names:
> >>>>      minItems: 1
> >>>>      maxItems: 4
> >>>> -    oneOf:
> >>>> -      - items:
> >>>> -          - const: hc
> >>>> -      - items:
> >>>> -          - const: hc
> >>>> -          - const: core
> >>>> -      - items:
> >>>> -          - const: hc
> >>>> -          - const: cqhci
> >>>> -      - items:
> >>>> -          - const: hc
> >>>> -          - const: cqhci
> >>>> -          - const: ice
> >>>> -      - items:
> >>>> -          - const: hc
> >>>> -          - const: core
> >>>> -          - const: cqhci
> >>>> -          - const: ice
> >>>>
> >>>>    clocks:
> >>>>      minItems: 3
> >>>> @@ -177,6 +155,43 @@ required:
> >>>>  allOf:
> >>>>    - $ref: mmc-controller.yaml#
> >>>>
> >>>> +  - if:
> >>>> +      properties:
> >>>> +        compatible:
> >>>> +          contains:
> >>>> +            enum:
> >>>> +              - qcom,sdhci-msm-v4
> >>>> +    then:
> >>>> +      properties:
> >>>> +        reg:
> >>>> +          minItems: 2
> >>>> +          items:
> >>>> +            - description: Host controller register map
> >>>> +            - description: SD Core register map
> >>>> +            - description: CQE register map
> >>>> +            - description: Inline Crypto Engine register map
> >>>> +        reg-names:
> >>>> +          minItems: 2
> >>>> +          items:
> >>>> +            - const: hc
> >>>> +            - const: core
> >>>> +            - const: cqhci
> >>>> +            - const: ice
> >>>> +    else:
> >>>> +      properties:
> >>>> +        reg:
> >>>> +          minItems: 1
> >>>> +          items:
> >>>> +            - description: Host controller register map
> >>>> +            - description: CQE register map
> >>>> +            - description: Inline Crypto Engine register map
> >>>> +        reg-names:
> >>>> +          minItems: 1
> >>>> +          items:
> >>>> +            - const: hc
> >>>> +            - const: cqhci
> >>>> +            - const: ice
> >>>
> >>> Do you need to set "maxItems" here? If you don't then will it inherit
> >>> the maxItems of 4 from above?
> >>
> >> No, items determine the size instead.
> >
> > Can you just remove the "maxItems" from above then? Does it buy us anything?
>
> There is no maxItems directly here...

Sorry, I mean above in the schema. After your patch the schema is effectively:

reg:
  minItems: 1
  maxItems: 4
reg-names:
  minItems: 1
  maxItems: 4

...

allOf:
  - if:
      blah-blah-blah
    then:
      properties:
        reg:
          minItems: 2
          items:
            - description: ...
            - description: ...
            - description: ...
            - description: ...
        reg-names:
          blah-blah-blah
    else:
      blah-blah-blah

I'm asking about the maxItems _above_, AKA in the section:

reg:
  minItems: 1
  maxItems: 4
reg-names:
  minItems: 1
  maxItems: 4

Can we remove the "maxItems: 4" from the above and have it just be:

reg:
  minItems: 1
reg-names:
  minItems: 1

-Doug
Krzysztof Kozlowski July 12, 2022, 2:38 p.m. UTC | #5
On 12/07/2022 16:29, Doug Anderson wrote:
> Hi,
> 
> On Tue, Jul 12, 2022 at 12:02 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 11/07/2022 17:11, Doug Anderson wrote:
>>> Hi,
>>>
>>> On Mon, Jul 11, 2022 at 7:53 AM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 11/07/2022 16:52, Doug Anderson wrote:
>>>>> Hi
>>>>>
>>>>> On Mon, Jul 11, 2022 at 1:29 AM Krzysztof Kozlowski
>>>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>>>
>>>>>> The entries in arrays must have fixed order, so the bindings and Linux
>>>>>> driver expecting various combinations of 'reg' addresses was never
>>>>>> actually conforming to guidelines.
>>>>>>
>>>>>> The 'core' reg entry is valid only for SDCC v4 and lower, so disallow it
>>>>>> in SDCC v5.  SDCC v4 supports CQE and ICE, so allow them, even though
>>>>>> the qcom,sdhci-msm-v4 compatible is used also for earlier SoCs with SDCC
>>>>>> v2 or v3, so it is not entirely accurate.
>>>>>>
>>>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> Changes since v1:
>>>>>> 1. Rework the patch based on Doug's feedback.
>>>>>> ---
>>>>>>  .../devicetree/bindings/mmc/sdhci-msm.yaml    | 61 ++++++++++++-------
>>>>>>  1 file changed, 38 insertions(+), 23 deletions(-)
>>>>>
>>>>> In the ${SUBJECT} I'm not sure what a "perp variant" is. Is that a
>>>>> typo or just a phrase I'm not aware of?
>>>>
>>>> Should be:
>>>> "per variants"
>>>>
>>>>>
>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>>>>>> index fc6e5221985a..2f0fdd65e908 100644
>>>>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
>>>>>> @@ -49,33 +49,11 @@ properties:
>>>>>>
>>>>>>    reg:
>>>>>>      minItems: 1
>>>>>> -    items:
>>>>>> -      - description: Host controller register map
>>>>>> -      - description: SD Core register map
>>>>>> -      - description: CQE register map
>>>>>> -      - description: Inline Crypto Engine register map
>>>>>> +    maxItems: 4
>>>>>>
>>>>>>    reg-names:
>>>>>>      minItems: 1
>>>>>>      maxItems: 4
>>>>>> -    oneOf:
>>>>>> -      - items:
>>>>>> -          - const: hc
>>>>>> -      - items:
>>>>>> -          - const: hc
>>>>>> -          - const: core
>>>>>> -      - items:
>>>>>> -          - const: hc
>>>>>> -          - const: cqhci
>>>>>> -      - items:
>>>>>> -          - const: hc
>>>>>> -          - const: cqhci
>>>>>> -          - const: ice
>>>>>> -      - items:
>>>>>> -          - const: hc
>>>>>> -          - const: core
>>>>>> -          - const: cqhci
>>>>>> -          - const: ice
>>>>>>
>>>>>>    clocks:
>>>>>>      minItems: 3
>>>>>> @@ -177,6 +155,43 @@ required:
>>>>>>  allOf:
>>>>>>    - $ref: mmc-controller.yaml#
>>>>>>
>>>>>> +  - if:
>>>>>> +      properties:
>>>>>> +        compatible:
>>>>>> +          contains:
>>>>>> +            enum:
>>>>>> +              - qcom,sdhci-msm-v4
>>>>>> +    then:
>>>>>> +      properties:
>>>>>> +        reg:
>>>>>> +          minItems: 2
>>>>>> +          items:
>>>>>> +            - description: Host controller register map
>>>>>> +            - description: SD Core register map
>>>>>> +            - description: CQE register map
>>>>>> +            - description: Inline Crypto Engine register map
>>>>>> +        reg-names:
>>>>>> +          minItems: 2
>>>>>> +          items:
>>>>>> +            - const: hc
>>>>>> +            - const: core
>>>>>> +            - const: cqhci
>>>>>> +            - const: ice
>>>>>> +    else:
>>>>>> +      properties:
>>>>>> +        reg:
>>>>>> +          minItems: 1
>>>>>> +          items:
>>>>>> +            - description: Host controller register map
>>>>>> +            - description: CQE register map
>>>>>> +            - description: Inline Crypto Engine register map
>>>>>> +        reg-names:
>>>>>> +          minItems: 1
>>>>>> +          items:
>>>>>> +            - const: hc
>>>>>> +            - const: cqhci
>>>>>> +            - const: ice
>>>>>
>>>>> Do you need to set "maxItems" here? If you don't then will it inherit
>>>>> the maxItems of 4 from above?
>>>>
>>>> No, items determine the size instead.
>>>
>>> Can you just remove the "maxItems" from above then? Does it buy us anything?
>>
>> There is no maxItems directly here...
> 
> Sorry, I mean above in the schema. After your patch the schema is effectively:
> 
> reg:
>   minItems: 1
>   maxItems: 4
> reg-names:
>   minItems: 1
>   maxItems: 4
> 
> ...
> 
> allOf:
>   - if:
>       blah-blah-blah
>     then:
>       properties:
>         reg:
>           minItems: 2
>           items:
>             - description: ...
>             - description: ...
>             - description: ...
>             - description: ...
>         reg-names:
>           blah-blah-blah
>     else:
>       blah-blah-blah
> 
> I'm asking about the maxItems _above_, AKA in the section:
> 
> reg:
>   minItems: 1
>   maxItems: 4
> reg-names:
>   minItems: 1
>   maxItems: 4
> 
> Can we remove the "maxItems: 4" from the above and have it just be:
> 
> reg:
>   minItems: 1
> reg-names:
>   minItems: 1
> 


Yes, we can, but preferred is to have it because it makes the broad
constraints easily visible. You don't need to check each if:else branch
to find upper bounds or check if maxItems are defined at all. This also
matches pattern used in bindings without allOf:if:then - each time you
are expected to see array types constraint in the list of properties.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
index fc6e5221985a..2f0fdd65e908 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
+++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
@@ -49,33 +49,11 @@  properties:
 
   reg:
     minItems: 1
-    items:
-      - description: Host controller register map
-      - description: SD Core register map
-      - description: CQE register map
-      - description: Inline Crypto Engine register map
+    maxItems: 4
 
   reg-names:
     minItems: 1
     maxItems: 4
-    oneOf:
-      - items:
-          - const: hc
-      - items:
-          - const: hc
-          - const: core
-      - items:
-          - const: hc
-          - const: cqhci
-      - items:
-          - const: hc
-          - const: cqhci
-          - const: ice
-      - items:
-          - const: hc
-          - const: core
-          - const: cqhci
-          - const: ice
 
   clocks:
     minItems: 3
@@ -177,6 +155,43 @@  required:
 allOf:
   - $ref: mmc-controller.yaml#
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sdhci-msm-v4
+    then:
+      properties:
+        reg:
+          minItems: 2
+          items:
+            - description: Host controller register map
+            - description: SD Core register map
+            - description: CQE register map
+            - description: Inline Crypto Engine register map
+        reg-names:
+          minItems: 2
+          items:
+            - const: hc
+            - const: core
+            - const: cqhci
+            - const: ice
+    else:
+      properties:
+        reg:
+          minItems: 1
+          items:
+            - description: Host controller register map
+            - description: CQE register map
+            - description: Inline Crypto Engine register map
+        reg-names:
+          minItems: 1
+          items:
+            - const: hc
+            - const: cqhci
+            - const: ice
+
 unevaluatedProperties: false
 
 examples: