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

[Stefan Wahren]

Hello,

Am 13.03.25 um 20:04 schrieb 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.
Phew, that was over 4 years ago. To be honest, my only motivation back
then was to prepare support for the Raspberry Pi 4 (BCM2711). I did it
all in my spare time and never had access to any Broadcom documents. I
have no idea about all the other BCM chips, so a possible break of the
ABI for the BCM7268 was an accident. I don't know if Florian Fainelli or
Maxime Ripard can help here.

By the way the two schema maintainers have not been active at V3D for a
long time, so it would be good if someone could take over.

Regards
>
>>
>> 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
>