[PATCH v3 5/7] dt-bindings: gpu: v3d: Add SMS register to BCM2712 compatible

[Maíra Canal]

Hi Rob,

On 3/11/25 17:23, Rob Herring wrote:
> On Tue, Mar 11, 2025 at 03:13:47PM -0300, Maíra Canal wrote:
>> V3D 7.1 exposes a new register block, called V3D_SMS. As BCM2712 has a
>> V3D 7.1 core, add a new register item to its compatible. Similar to the
>> GCA, which is specific for V3D 3.3, SMS is optional and should only be
>> added for V3D 7.1 variants (such as brcm,2712-v3d).
> 
> Based on the schema, that is not what optional means for bindings.
> Optional is within a specific compatible, not present for one compatible
> and not present for another compatible.

Sorry for the poor writing! I'll improve it in the next version.

> 
>>
>> Signed-off-by: Maíra Canal <mcanal at igalia.com>
>> ---
>>   .../devicetree/bindings/gpu/brcm,bcm-v3d.yaml      | 22 +++++++++++++++++++++-
>>   1 file changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>> index 141f2ed540bb4ddb85a933d7d44a4078c386ba39..7349347da1c0034a8849deaa6d64dde6d9d5a81a 100644
>> --- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>> +++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>> @@ -57,7 +57,6 @@ allOf:
>>             contains:
>>               enum:
>>                 - brcm,2711-v3d
>> -              - brcm,2712-v3d
>>                 - brcm,7278-v3d
>>       then:
>>         properties:
>> @@ -71,6 +70,27 @@ allOf:
>>               - const: hub
>>               - const: core0
>>               - const: bridge
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: brcm,2712-v3d
>> +    then:
>> +      properties:
>> +        reg:
>> +          items:
>> +            - description: hub register (required)
>> +            - description: core0 register (required)
>> +            - description: SMS register (required)
> 
> minItems tells us these are all 'required'.
> 
>> +            - description: bridge register (if no external reset controller)
>> +          minItems: 3
>> +        reg-names:
>> +          items:
>> +            - const: hub
>> +            - const: core0
>> +            - const: sms
>> +            - const: bridge
> 
> This is an ABI change because previously the 3rd entry was bridge and
> you moved it. Put new entries on the end to keep compatibility. If
> there's no users yet, then that's fine, but the commit message needs to
> say that.
> 

Again, I'm sorry about the naive question, but "bridge" is an optional
register. AFAIU if I add "sms" in the end, I won't be able to make
"bridge" optional. Am I missing something?

Initially, I thought about using "enum: [ bridge, sms ]", but from
Krzysztof’s feedback on v2, I'm not sure if it is the correct approach.

Thanks for your review!

Best Regards,
- Maíra

>> +          minItems: 3
>>     - if:
>>         properties:
>>           compatible:
>>
>> -- 
>> Git-154)
>>