Hi Krzysztof,
On 3/10/25 14:34, Krzysztof Kozlowski wrote:
On 10/03/2025 14:15, Maíra Canal wrote:
Hi Krzysztof,
On 3/10/25 09:55, Krzysztof Kozlowski wrote:
On 10/03/2025 12:57, Maíra Canal wrote:
Signed-off-by: Maíra Canal <mcanal@xxxxxxxxxx>
---
.../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.
I'm sorry, I don't usually write DTBs. I believe what I need is to
specify the reg items for each compatible, right?
You need to list four items in 'reg' and last items' description would
need to describe two different sets.
And commit msg must explain why now this device needs to use sms, not
gca. Why you cannot use the gca range instead of sms? So many questions.
I believe I understood the issue now. From my point of view, the idea is
preserving the semantics of the register bank names. GCA is a cache
controller that it is only available in V3D 3.3 (so, only brcm,7268-v3d
has it). SMS is a power controller that it is only available in V3D 7.1
(so, only brcm,2712-v3d has it).
Each one of those compatibles represent different V3D generations (3.3,
4.1, 4.2, 7.1).
From my understanding, I'm keeping the ABI preserved, as brcm,7268-v3d
needs to have a GCA register (otherwise, you won't be able to flush the
cache) and brcm,2711-v3d doesn't even have this piece of hardware.
I understand that now I'm imposing per-compatible restrictions that
didn't exist in the spec, but the new restrictions are compatible to the
hardware specification. I'd like to understand if I can:
1. Use two register items (gca and sms) to preserve the semantics of the
register names.
2. Can I add per-compatible restrictions to the DT even if the original
bindings didn't do it? If not, can I just add a new register to the
register list? Like:
reg-names:
items:
- const: hub
- const: core0
- - enum: [ bridge, gca ]
- - enum: [ bridge, gca ]
+ - enum: [ bridge, gca, sms ]
+ - enum: [ bridge, gca, sms ]
+ - enum: [ bridge, gca, sms ]
minItems: 2
Best Regards,
- Maíra
bcm
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.
>
I used `make dt_binding_check` to test it and it didn't catch any
errors.
So change the example to have one list with two items and second list
with three items. Is it correct DTS? No. Does this pass tests? Yes.
+ - 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.
I'm trying to keep backwards compatibility. This binding exists for many
years and it always used "enum: [ bridge, gca ]".
But it is now sms, not gca,, so I do not see how the ABI is preserved.
Best regards,
Krzysztof
