On 11/04/2024 13:53, Conor Dooley wrote:
On Thu, Apr 11, 2024 at 11:08:21AM +0200, Clément Léger wrote:
If we consider to have potentially broken isa string (ie extensions dependencies not correctly handled), then we'll need some way to validate this within the kernel.
No, the DT passed to the kernel should be correct and we by and large we should not have to do validation of it. What I meant above was writing the binding so that something invalid will not pass dtbs_check.
Acked, I was mainly answering Deepak question about dependencies wrt to using __RISCV_ISA_EXT_SUPERSET() which does not seems to be relevant since we expect a correct isa string to be passed.
Ahh, okay.
But as you stated, DT validation clearly make sense. I think a lot of extensions strings would benefit such support (All the Zv* depends on V, etc).
I think it is actually as simple something like this, which makes it invalid to have "d" without "f":
| diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml | index 468c646247aa..594828700cbe 100644 | --- a/Documentation/devicetree/bindings/riscv/extensions.yaml | +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml | @@ -484,5 +484,20 @@ properties: | Registers in the AX45MP datasheet. | https://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.... | | +allOf: | + - if: | + properties: | + riscv,isa-extensions: | + contains: | + const: "d" | + not: | + contains: | + const: "f" | + then: | + properties: | + riscv,isa-extensions: | + false | + | + | additionalProperties: true | ...
If you do have d without f, the checker will say: cpu@2: riscv,isa-extensions: False schema does not allow ['i', 'm', 'a', 'd', 'c']
At least that's readable, even though not clear about what to do. I wish
That looks really readable indeed but the messages that result from errors are not so informative.
It tried playing with various constructs and found this one to yield a comprehensive message:
+allOf: + - if: + properties: + riscv,isa-extensions: + contains: + const: zcf + not: + contains: + const: zca + then: + properties: + riscv,isa-extensions: + items: + anyOf: + - const: zca
arch/riscv/boot/dts/allwinner/sun20i-d1-dongshan-nezha-stu.dtb: cpu@0: riscv,isa-extensions:10: 'anyOf' conditional failed, one must be fixed: 'zca' was expected from schema $id: http://devicetree.org/schemas/riscv/extensions.yaml
Even though dt-bindings-check passed, not sure if this is totally a valid construct though...
Thanks,
Clément
the former could be said about the wall of text you get for /each/ undocumented entry in the string.