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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---
  .../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






[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux