[PATCH v4 4/7] dt-bindings: gpu: v3d: Add per-compatible register restrictions

[Maíra Canal]

+Cc Stefan

Hi Krzysztof,

On 13/03/25 12:03, Krzysztof Kozlowski wrote:
> On 13/03/2025 15:43, Maíra Canal wrote:
>> In order to enforce per-SoC register rules, add per-compatible
>> restrictions. V3D 3.3 (represented by brcm,7268-v3d) has a cache
>> controller (GCA), which is not present in other V3D generations.
>> Declaring these differences helps ensure the DTB accurately reflect
>> the hardware design.
>>
>> While not ideal, this commit keeps the register order flexible for
>> brcm,7268-v3d with the goal to keep the ABI backwards compatible.
>>
>> Signed-off-by: Maíra Canal <mcanal at igalia.com>
>> ---
>>   .../devicetree/bindings/gpu/brcm,bcm-v3d.yaml      | 73 ++++++++++++++++++----
>>   1 file changed, 61 insertions(+), 12 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>> index dc078ceeca9ac3447ba54a7c8830821f0b2a7f9f..9867b617c60c6fe34a0f88a3ee2f581a94b69a5c 100644
>> --- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>> +++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
>> @@ -22,20 +22,10 @@ properties:
>>         - brcm,7278-v3d
>>   
>>     reg:
>> -    items:
>> -      - description: hub register (required)
>> -      - description: core0 register (required)
>> -      - description: GCA cache controller register (if GCA controller present)
>> -      - description: bridge register (if no external reset controller)
>> -    minItems: 2
> 
> Widest constraints always stay here, so you cannot remove minItems.
> 
>> +    maxItems: 4
> 
>>   
>>     reg-names:
>> -    items:
>> -      - const: hub
>> -      - const: core0
>> -      - enum: [ bridge, gca ]
>> -      - enum: [ bridge, gca ]
>> -    minItems: 2
> 
> Same problem.
> 
>> +    maxItems: 4
>>   
>>     interrupts:
>>       items:
>> @@ -58,6 +48,65 @@ required:
>>     - reg-names
>>     - interrupts
> 
> ...
> 
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: brcm,7268-v3d
>> +    then:
>> +      properties:
>> +        reg:
>> +          items:
>> +            - description: hub register
>> +            - description: core0 register
>> +            - description: GCA cache controller register
>> +            - description: bridge register (if no external reset controller)
>> +          minItems: 3
>> +        reg-names:
>> +          items:
>> +            - const: hub
>> +            - const: core0
>> +            - enum: [ bridge, gca ]
> 
 > So GCA is always there? Then this should be just 'gca'. Your list for

GCA is always there for V3D 3.3, therefore it is always there for
brcm,7268-v3d.

> 'reg' already says that third item must be GCA. I understand that you do
> not want to affect the ABI, but it already kind of is with enforcing GCA
> in 'reg'.

I'm adding Stefan to the loop as he was the one that converted this DT
binding to YAML. Stefan, could you share your thoughts about breaking
the ABI for BCM7268? We would enforce the following order: hub, core0,
bridge, and gca.

> 
> I anyway do not understand why 'gca' or 'bridge' are supposed to be
> optional. Does the given SoC differ between boards? What is the external
> reset controller here? External like outside of SoC?

TBH I never saw BCM7268 or BCM7278 in the wild, but you are correct,
"bridge" shouldn't change for the same SoC. I'll do my diligence and
research more about those SoCs.

Best Regards,
- Maíra

> 
> Best regards,
> Krzysztof