[PATCH v2 4/6] dt-bindings: gpu: v3d: Add SMS to the registers' list

[Krzysztof Kozlowski]

On 10/03/2025 12:57, Maíra Canal wrote:
>>
>>> Signed-off-by: Maíra Canal <mcanal at igalia.com>
>>> ---
>>>   .../devicetree/bindings/gpu/brcm,bcm-v3d.yaml      | 60 ++++++++++++++++++++--
>>>   1 file changed, 55 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>>> index dc078ceeca9ac3447ba54a7c8830821f0b2a7f9f..c0caee055e8c18dbcac0e51aa192951996545695 100644
>>> --- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>>> +++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>>> @@ -27,15 +27,12 @@ properties:
>>>         - description: core0 register (required)
>>>         - description: GCA cache controller register (if GCA controller present)
>>>         - description: bridge register (if no external reset controller)
>>> +      - description: SMS register (if SMS controller present)
>>
>> This lists five items, but you say you have max 4?
> 
> V3D 3.1 uses hub, core0, gca, and bridge (optional)
> V3D 4.1 and 4.2 uses hub, core, and bridge (optional)
> V3D 7.1 uses hub, core0, sms, and bridge (optional)
> 
> Therefore, for a given DT, you will have 4 items max.

And how many items do you have here?

> 
>>
>>>       minItems: 2
>>>   
>>>     reg-names:
>>> -    items:
>>> -      - const: hub
>>> -      - const: core0
>>> -      - enum: [ bridge, gca ]
>>> -      - enum: [ bridge, gca ]
>>>       minItems: 2
>>> +    maxItems: 4
>>
>> So here 4, but earlier 5? These must come in sync.
> 
> I added maxItems for reg in the allOf section.

I don't think you answer the comments. I said you listed earlier 5 items
and then you answered with saying devices have 4 items. Here I said
these properties must be synced and you said why you added maxItems...
Not related, read again the feedback.


> 
>>
>>>   
>>>     interrupts:
>>>       items:
>>> @@ -60,6 +57,59 @@ required:
>>>   
>>>   additionalProperties: false
>>>   
>>> +allOf:
>>
>> This goes above additionalProperties.
> 
> Got it.
> 
>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - brcm,2711-v3d
>>> +              - brcm,7278-v3d
>>> +    then:
>>> +      properties:
>>> +        reg:
>>> +          minItems: 2
>>> +          maxItems: 3
>>> +        reg-names:
>>> +          items:
>>> +            - const: hub
>>> +            - const: core0
>>> +            - const: bridge
>>
>> Again un-synced lists.
> 
> Sorry, what do you mean by un-synced lists?

xxx and xxx-names must have the same constraints. They do not have here.
You have two different constraints and you can test your DTS to see that.

> 
>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: brcm,2712-v3d
>>> +    then:
>>> +      properties:
>>> +        reg:
>>> +          minItems: 3
>>> +          maxItems: 4
>>> +        reg-names:
>>> +          items:
>>> +            - const: hub
>>> +            - const: core0
>>> +            - enum: [ bridge, sms ]
>>> +            - enum: [ bridge, sms ]
>>> +          minItems: 3
>>
>> Why is this flexible?
> 
> I cannot guarantee the order and bridge is optional.

Hm? You must guarantee the order and I do not understand why this needs
some sort of exception from all other bindings that only here you cannot
guarantee the order.


Best regards,
Krzysztof