2022-08-17 20:23:06

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v3 0/4] Fix dt-validate issues on qemu dtbdumps due to dt-bindings

From: Conor Dooley <[email protected]>

(correctly sent this time, without duplicate patches)

The device trees produced automatically for the virt and spike machines
fail dt-validate on several grounds. Some of these need to be fixed in
the linux kernel's dt-bindings, but others are caused by bugs in QEMU.

Patches been sent that fix the QEMU issues [0], but a couple of them
need to be fixed in the kernel's dt-bindings. The first patches add
compatibles for "riscv,{clint,plic}0" which are present in drivers and
the auto generated QEMU dtbs. The final patch should be ignored for all
serious purposes unless you want to wash your eyes out afterwards, but
JIC the versioned extensions ever come up, it's there.

Thanks to Rob Herring for reporting these issues [1],
Conor.

To reproduce the errors:
./build/qemu-system-riscv64 -nographic -machine virt,dumpdtb=qemu.dtb
dt-validate -p /path/to/linux/kernel/Documentation/devicetree/bindings/processed-schema.json qemu.dtb
(The processed schema needs to be generated first)

0 - https://lore.kernel.org/linux-riscv/[email protected]/
1 - https://lore.kernel.org/linux-riscv/[email protected]/

Changes since v2:
- removed the extra patches from the directory

Changes since v1:
- drop the "legacy systems" bit from the binding descriptions
- convert to a regex for the isa string

Conor Dooley (4):
dt-bindings: timer: sifive,clint: add legacy riscv compatible
dt-bindings: interrupt-controller: sifive,plic: add legacy riscv
compatible
dt-bindings: riscv: add new riscv,isa strings for emulators
dt-bindings: riscv: isa string bonus content

.../sifive,plic-1.0.0.yaml | 5 +++++
.../devicetree/bindings/riscv/cpus.yaml | 9 ++++++---
.../bindings/timer/sifive,clint.yaml | 18 ++++++++++++------
3 files changed, 23 insertions(+), 9 deletions(-)


base-commit: 568035b01cfb107af8d2e4bd2fb9aea22cf5b868
--
2.37.1


2022-08-17 20:24:46

by Conor Dooley

[permalink] [raw]
Subject: [NOT FOR INCLUSION v3 4/4] dt-bindings: riscv: isa string bonus content

From: Conor Dooley <[email protected]>

**NOT FOR INCLUSION**

I figured, sure why not add the strings for version number validation,
just in case we need them in the future. The commented out string is
considered by dt-schema to be "not a regex", but regex101 thinks it
is... Maybe dt-schema does not support named match groups, but they
are the only way that I could trivially find to make this somewhat
manageable. Either way, it is permissive so it allows combinations
of "M", "MpM" & no number.

Not-signed-off-by: Conor Dooley <[email protected]>
---
Documentation/devicetree/bindings/riscv/cpus.yaml | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
index c0e0bc5dce04..38a824453012 100644
--- a/Documentation/devicetree/bindings/riscv/cpus.yaml
+++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
@@ -80,7 +80,11 @@ properties:
insensitive, letters in the riscv,isa string must be all
lowercase to simplify parsing.
$ref: "/schemas/types.yaml#/definitions/string"
- pattern: ^rv(?:64|32)imaf?d?q?c?b?v?k?h?(?:(?:_[zsh][imafdqcbvksh]|_x)(?:[a-z])+)*$
+ oneOf:
+ - pattern: ^rv(?:64|32)imaf?d?q?c?b?v?k?h?(?:(?:_[zsh][imafdqcbvksh]|_x)(?:[a-z])+)*$
+ - pattern: ^rv(?:64|32)(?:i\d+)(?:m\d+)(?:a\d+)(?:f\d+)?(?:d\d+)?(?:q\d+)?(?:c\d+)?(?:b\d+)?(?:v\d+)?(?:k\d+)?(?:h\d+)?(?:(?:_[zsh][imafdqcbvksh]|_x)(?:[a-z])+\d+)*$
+ - pattern: ^rv(?:64|32)(?:i\d+p\d+)(?:m\d+p\d+)(?:a\d+p\d+)(?:f\d+p\d+)?(?:d\d+p\d+)?(?:q\d+p\d+)?(?:c\d+p\d+)?(?:b\d+p\d+)?(?:v\d+p\d+)?(?:k\d+p\d+)?(?:h\d+p\d+)?(?:(?:_[zsh][imafdqcbvksh]|_x)(?:[a-z])+(?:\d+p\d+))*$
+# - pattern: ^rv(?:64|32)(?:i(?<num>(?:\d+|\d+p\d+)?)?)(?:m(?:\k<num>)?)(?:a(?:\k<num>)?)(?:f(?:\k<num>)?)?(?:d(?:\k<num>)?)?(?:q(?:\k<num>)?)?(?:c(?:\k<num>)?)?(?:b(?:\k<num>)?)?(?:v(?:\k<num>)?)?(?:k(?:\k<num>)?)?(?:h(?:\k<num>)?)?(?:(?:_[zsh][imafdqcbvksh]|_x)(?:[a-z])*(?:\d+|\d+p\d+)?)+$

# RISC-V requires 'timebase-frequency' in /cpus, so disallow it here
timebase-frequency: false
--
2.37.1

2022-08-17 20:25:56

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v3 2/4] dt-bindings: interrupt-controller: sifive,plic: add legacy riscv compatible

From: Conor Dooley <[email protected]>

While "real" hardware might not use the compatible string "riscv,plic0"
it is present in the driver & QEMU uses it for automatically generated
virt machine dtbs. To avoid dt-validate problems with QEMU produced
dtbs, such as the following, add it to the binding.

riscv-virt.dtb: plic@c000000: compatible: 'oneOf' conditional failed, one must be fixed:
'sifive,plic-1.0.0' is not one of ['sifive,fu540-c000-plic', 'starfive,jh7100-plic', 'canaan,k210-plic']
'sifive,plic-1.0.0' is not one of ['allwinner,sun20i-d1-plic']
'sifive,plic-1.0.0' was expected
'thead,c900-plic' was expected
riscv-virt.dtb: plic@c000000: '#address-cells' is a required property

Reported-by: Rob Herring <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Signed-off-by: Conor Dooley <[email protected]>
---
.../bindings/interrupt-controller/sifive,plic-1.0.0.yaml | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
index 92e0f8c3eff2..99e01f4d0a69 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
@@ -66,6 +66,11 @@ properties:
- enum:
- allwinner,sun20i-d1-plic
- const: thead,c900-plic
+ - items:
+ - const: sifive,plic-1.0.0
+ - const: riscv,plic0
+ deprecated: true
+ description: For the QEMU virt machine only

reg:
maxItems: 1
--
2.37.1

2022-08-17 20:25:56

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v3 3/4] dt-bindings: riscv: add new riscv,isa strings for emulators

From: Conor Dooley <[email protected]>

The QEMU virt and spike machines currently export a riscv,isa string of
"rv64imafdcsuh",

While the RISC-V foundation has been ratifying a bunch of extenstions
etc, the kernel has remained relatively static with what hardware is
supported - but the same is not true of QEMU. Using the virt machine
and running dt-validate on the dumped dtb fails, partly due to the
unexpected isa string.

Rather than enumerate the many many possbilities, change the pattern
to a regex, with the following assumptions:
- the single letter order is fixed & we don't care about things that
can't even do "ima"
- the standard multi letter extensions are all in a "_z<foo>" format
where the first letter of <foo> is a valid single letter extension
- _s & _h are used for supervisor and hyper visor extensions.
- after the first two chars, a standard multi letter extension name
could be an english word (ifencei anyone?) so it's not worth
restricting the charset
- vendor ISA extensions begind with _x and have no charset restrictions
- we don't care about an e extension from an OS pov
- that attempting to validate the contents of the multiletter extensions
with dt-validate beyond the formatting is a futile, massively verbose
or unwieldy exercise at best.
- ima are required

The following limitations also apply:
- multi letter extension ordering is not enforced. dt-schema does not
appear to allow for named match groups, so the resulting regex would
be even more of a headache.
- ditto for the numbered extensions.

Finally, add me as a maintainer of the binding so that when it breaks
in the future, I can be held responsible!

Reported-by: Rob Herring <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Signed-off-by: Conor Dooley <[email protected]>
---
Palmer, feel free to drop the maintainer addition. I just mostly want
to clean up my own mess on this when they decide to ratify more
extensions & this comes back up again.
---
Documentation/devicetree/bindings/riscv/cpus.yaml | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
index 873dd12f6e89..c0e0bc5dce04 100644
--- a/Documentation/devicetree/bindings/riscv/cpus.yaml
+++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
@@ -9,6 +9,7 @@ title: RISC-V bindings for 'cpus' DT nodes
maintainers:
- Paul Walmsley <[email protected]>
- Palmer Dabbelt <[email protected]>
+ - Conor Dooley <[email protected]>

description: |
This document uses some terminology common to the RISC-V community
@@ -79,9 +80,7 @@ properties:
insensitive, letters in the riscv,isa string must be all
lowercase to simplify parsing.
$ref: "/schemas/types.yaml#/definitions/string"
- enum:
- - rv64imac
- - rv64imafdc
+ pattern: ^rv(?:64|32)imaf?d?q?c?b?v?k?h?(?:(?:_[zsh][imafdqcbvksh]|_x)(?:[a-z])+)*$

# RISC-V requires 'timebase-frequency' in /cpus, so disallow it here
timebase-frequency: false
--
2.37.1

2022-08-17 20:46:57

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v3 1/4] dt-bindings: timer: sifive,clint: add legacy riscv compatible

From: Conor Dooley <[email protected]>

While "real" hardware might not use the compatible string "riscv,clint0"
it is present in the driver & QEMU uses it for automatically generated
virt machine dtbs. To avoid dt-validate problems with QEMU produced
dtbs, such as the following, add it to the binding.

riscv-virt.dtb: clint@2000000: compatible:0: 'sifive,clint0' is not one of ['sifive,fu540-c000-clint', 'starfive,jh7100-clint', 'canaan,k210-clint']

Reported-by: Rob Herring <[email protected]>
Link: https://lore.kernel.org/linux-riscv/[email protected]/
Signed-off-by: Conor Dooley <[email protected]>
---
.../bindings/timer/sifive,clint.yaml | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.yaml b/Documentation/devicetree/bindings/timer/sifive,clint.yaml
index e64f46339079..bbad24165837 100644
--- a/Documentation/devicetree/bindings/timer/sifive,clint.yaml
+++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml
@@ -22,12 +22,18 @@ description:

properties:
compatible:
- items:
- - enum:
- - sifive,fu540-c000-clint
- - starfive,jh7100-clint
- - canaan,k210-clint
- - const: sifive,clint0
+ oneOf:
+ - items:
+ - enum:
+ - sifive,fu540-c000-clint
+ - starfive,jh7100-clint
+ - canaan,k210-clint
+ - const: sifive,clint0
+ - items:
+ - const: sifive,clint0
+ - const: riscv,clint0
+ deprecated: true
+ description: For the QEMU virt machine only

description:
Should be "<vendor>,<chip>-clint" and "sifive,clint<version>".
--
2.37.1

2022-08-18 01:50:23

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH v3 3/4] dt-bindings: riscv: add new riscv,isa strings for emulators

On Thu, Aug 18, 2022 at 4:12 AM Conor Dooley <[email protected]> wrote:
>
> From: Conor Dooley <[email protected]>
>
> The QEMU virt and spike machines currently export a riscv,isa string of
> "rv64imafdcsuh",
>
> While the RISC-V foundation has been ratifying a bunch of extenstions
> etc, the kernel has remained relatively static with what hardware is
> supported - but the same is not true of QEMU. Using the virt machine
> and running dt-validate on the dumped dtb fails, partly due to the
> unexpected isa string.
>
> Rather than enumerate the many many possbilities, change the pattern
> to a regex, with the following assumptions:
> - the single letter order is fixed & we don't care about things that
> can't even do "ima"
> - the standard multi letter extensions are all in a "_z<foo>" format
> where the first letter of <foo> is a valid single letter extension
> - _s & _h are used for supervisor and hyper visor extensions.
> - after the first two chars, a standard multi letter extension name
> could be an english word (ifencei anyone?) so it's not worth
> restricting the charset
> - vendor ISA extensions begind with _x and have no charset restrictions
> - we don't care about an e extension from an OS pov
> - that attempting to validate the contents of the multiletter extensions
> with dt-validate beyond the formatting is a futile, massively verbose
> or unwieldy exercise at best.
> - ima are required
>
> The following limitations also apply:
> - multi letter extension ordering is not enforced. dt-schema does not
> appear to allow for named match groups, so the resulting regex would
> be even more of a headache.
> - ditto for the numbered extensions.
>
> Finally, add me as a maintainer of the binding so that when it breaks
> in the future, I can be held responsible!
>
> Reported-by: Rob Herring <[email protected]>
> Link: https://lore.kernel.org/linux-riscv/[email protected]/
> Signed-off-by: Conor Dooley <[email protected]>
> ---
> Palmer, feel free to drop the maintainer addition. I just mostly want
> to clean up my own mess on this when they decide to ratify more
> extensions & this comes back up again.
> ---
> Documentation/devicetree/bindings/riscv/cpus.yaml | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> index 873dd12f6e89..c0e0bc5dce04 100644
> --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> @@ -9,6 +9,7 @@ title: RISC-V bindings for 'cpus' DT nodes
> maintainers:
> - Paul Walmsley <[email protected]>
> - Palmer Dabbelt <[email protected]>
> + - Conor Dooley <[email protected]>
Acked-by: Guo Ren <[email protected]>

>
> description: |
> This document uses some terminology common to the RISC-V community
> @@ -79,9 +80,7 @@ properties:
> insensitive, letters in the riscv,isa string must be all
> lowercase to simplify parsing.
> $ref: "/schemas/types.yaml#/definitions/string"
> - enum:
> - - rv64imac
> - - rv64imafdc
> + pattern: ^rv(?:64|32)imaf?d?q?c?b?v?k?h?(?:(?:_[zsh][imafdqcbvksh]|_x)(?:[a-z])+)*$
>
> # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here
> timebase-frequency: false
> --
> 2.37.1
>


--
Best Regards
Guo Ren

2022-08-18 05:44:43

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v3 3/4] dt-bindings: riscv: add new riscv,isa strings for emulators

On Wed, Aug 17, 2022 at 09:12:12PM +0100, Conor Dooley wrote:
> From: Conor Dooley <[email protected]>
>
> The QEMU virt and spike machines currently export a riscv,isa string of
> "rv64imafdcsuh",
>
> While the RISC-V foundation has been ratifying a bunch of extenstions
> etc, the kernel has remained relatively static with what hardware is
> supported - but the same is not true of QEMU. Using the virt machine
> and running dt-validate on the dumped dtb fails, partly due to the
> unexpected isa string.
>
> Rather than enumerate the many many possbilities, change the pattern
> to a regex, with the following assumptions:
> - the single letter order is fixed & we don't care about things that
> can't even do "ima"
> - the standard multi letter extensions are all in a "_z<foo>" format
> where the first letter of <foo> is a valid single letter extension
> - _s & _h are used for supervisor and hyper visor extensions.
> - after the first two chars, a standard multi letter extension name
> could be an english word (ifencei anyone?) so it's not worth
> restricting the charset
> - vendor ISA extensions begind with _x and have no charset restrictions
> - we don't care about an e extension from an OS pov
> - that attempting to validate the contents of the multiletter extensions
> with dt-validate beyond the formatting is a futile, massively verbose
> or unwieldy exercise at best.
> - ima are required
>
> The following limitations also apply:
> - multi letter extension ordering is not enforced. dt-schema does not
> appear to allow for named match groups, so the resulting regex would
> be even more of a headache.
> - ditto for the numbered extensions.
>
> Finally, add me as a maintainer of the binding so that when it breaks
> in the future, I can be held responsible!
>
> Reported-by: Rob Herring <[email protected]>
> Link: https://lore.kernel.org/linux-riscv/[email protected]/
> Signed-off-by: Conor Dooley <[email protected]>
> ---
> Palmer, feel free to drop the maintainer addition. I just mostly want
> to clean up my own mess on this when they decide to ratify more
> extensions & this comes back up again.
> ---
> Documentation/devicetree/bindings/riscv/cpus.yaml | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> index 873dd12f6e89..c0e0bc5dce04 100644
> --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> @@ -9,6 +9,7 @@ title: RISC-V bindings for 'cpus' DT nodes
> maintainers:
> - Paul Walmsley <[email protected]>
> - Palmer Dabbelt <[email protected]>
> + - Conor Dooley <[email protected]>
>
> description: |
> This document uses some terminology common to the RISC-V community
> @@ -79,9 +80,7 @@ properties:
> insensitive, letters in the riscv,isa string must be all
> lowercase to simplify parsing.
> $ref: "/schemas/types.yaml#/definitions/string"
> - enum:
> - - rv64imac
> - - rv64imafdc
> + pattern: ^rv(?:64|32)imaf?d?q?c?b?v?k?h?(?:(?:_[zsh][imafdqcbvksh]|_x)(?:[a-z])+)*$
>
> # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here
> timebase-frequency: false
> --
> 2.37.1
>

Reviewed-by: Andrew Jones <[email protected]>

2022-08-18 06:16:10

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v3 3/4] dt-bindings: riscv: add new riscv,isa strings for emulators

On Thu, Aug 18, 2022 at 07:40:14AM +0200, Andrew Jones wrote:
> On Wed, Aug 17, 2022 at 09:12:12PM +0100, Conor Dooley wrote:
> > From: Conor Dooley <[email protected]>
> >
> > The QEMU virt and spike machines currently export a riscv,isa string of
> > "rv64imafdcsuh",
> >
> > While the RISC-V foundation has been ratifying a bunch of extenstions
> > etc, the kernel has remained relatively static with what hardware is
> > supported - but the same is not true of QEMU. Using the virt machine
> > and running dt-validate on the dumped dtb fails, partly due to the
> > unexpected isa string.
> >
> > Rather than enumerate the many many possbilities, change the pattern
> > to a regex, with the following assumptions:
> > - the single letter order is fixed & we don't care about things that
> > can't even do "ima"
> > - the standard multi letter extensions are all in a "_z<foo>" format
> > where the first letter of <foo> is a valid single letter extension
> > - _s & _h are used for supervisor and hyper visor extensions.
> > - after the first two chars, a standard multi letter extension name
> > could be an english word (ifencei anyone?) so it's not worth
> > restricting the charset
> > - vendor ISA extensions begind with _x and have no charset restrictions
> > - we don't care about an e extension from an OS pov
> > - that attempting to validate the contents of the multiletter extensions
> > with dt-validate beyond the formatting is a futile, massively verbose
> > or unwieldy exercise at best.
> > - ima are required
> >
> > The following limitations also apply:
> > - multi letter extension ordering is not enforced. dt-schema does not
> > appear to allow for named match groups, so the resulting regex would
> > be even more of a headache.
> > - ditto for the numbered extensions.
> >
> > Finally, add me as a maintainer of the binding so that when it breaks
> > in the future, I can be held responsible!
> >
> > Reported-by: Rob Herring <[email protected]>
> > Link: https://lore.kernel.org/linux-riscv/[email protected]/
> > Signed-off-by: Conor Dooley <[email protected]>
> > ---
> > Palmer, feel free to drop the maintainer addition. I just mostly want
> > to clean up my own mess on this when they decide to ratify more
> > extensions & this comes back up again.
> > ---
> > Documentation/devicetree/bindings/riscv/cpus.yaml | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > index 873dd12f6e89..c0e0bc5dce04 100644
> > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > @@ -9,6 +9,7 @@ title: RISC-V bindings for 'cpus' DT nodes
> > maintainers:
> > - Paul Walmsley <[email protected]>
> > - Palmer Dabbelt <[email protected]>
> > + - Conor Dooley <[email protected]>
> >
> > description: |
> > This document uses some terminology common to the RISC-V community
> > @@ -79,9 +80,7 @@ properties:
> > insensitive, letters in the riscv,isa string must be all
> > lowercase to simplify parsing.
> > $ref: "/schemas/types.yaml#/definitions/string"
> > - enum:
> > - - rv64imac
> > - - rv64imafdc
> > + pattern: ^rv(?:64|32)imaf?d?q?c?b?v?k?h?(?:(?:_[zsh][imafdqcbvksh]|_x)(?:[a-z])+)*$

Actually we don't want S and H extensions to be treated like Z, but rather
like X. Only Z extensions have the category character convention. (And I'm
still tempted to just drop the enforcement from Z too, since it adds
additional maintenance and we've already settled for something less than
complete.)

Thanks,
drew

> >
> > # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here
> > timebase-frequency: false
> > --
> > 2.37.1
> >
>
> Reviewed-by: Andrew Jones <[email protected]>

2022-08-18 06:48:54

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 3/4] dt-bindings: riscv: add new riscv,isa strings for emulators

On 18/08/2022 06:48, Andrew Jones wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Thu, Aug 18, 2022 at 07:40:14AM +0200, Andrew Jones wrote:
>> On Wed, Aug 17, 2022 at 09:12:12PM +0100, Conor Dooley wrote:
>>> From: Conor Dooley <[email protected]>
>>>
>>> The QEMU virt and spike machines currently export a riscv,isa string of
>>> "rv64imafdcsuh",
>>>
>>> While the RISC-V foundation has been ratifying a bunch of extenstions
>>> etc, the kernel has remained relatively static with what hardware is
>>> supported - but the same is not true of QEMU. Using the virt machine
>>> and running dt-validate on the dumped dtb fails, partly due to the
>>> unexpected isa string.
>>>
>>> Rather than enumerate the many many possbilities, change the pattern
>>> to a regex, with the following assumptions:
>>> - the single letter order is fixed & we don't care about things that
>>> can't even do "ima"
>>> - the standard multi letter extensions are all in a "_z<foo>" format
>>> where the first letter of <foo> is a valid single letter extension
>>> - _s & _h are used for supervisor and hyper visor extensions.
>>> - after the first two chars, a standard multi letter extension name
>>> could be an english word (ifencei anyone?) so it's not worth
>>> restricting the charset
>>> - vendor ISA extensions begind with _x and have no charset restrictions
>>> - we don't care about an e extension from an OS pov
>>> - that attempting to validate the contents of the multiletter extensions
>>> with dt-validate beyond the formatting is a futile, massively verbose
>>> or unwieldy exercise at best.
>>> - ima are required
>>>
>>> The following limitations also apply:
>>> - multi letter extension ordering is not enforced. dt-schema does not
>>> appear to allow for named match groups, so the resulting regex would
>>> be even more of a headache.
>>> - ditto for the numbered extensions.
>>>
>>> Finally, add me as a maintainer of the binding so that when it breaks
>>> in the future, I can be held responsible!
>>>
>>> Reported-by: Rob Herring <[email protected]>
>>> Link: https://lore.kernel.org/linux-riscv/[email protected]/
>>> Signed-off-by: Conor Dooley <[email protected]>
>>> ---
>>> Palmer, feel free to drop the maintainer addition. I just mostly want
>>> to clean up my own mess on this when they decide to ratify more
>>> extensions & this comes back up again.
>>> ---
>>> Documentation/devicetree/bindings/riscv/cpus.yaml | 5 ++---
>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
>>> index 873dd12f6e89..c0e0bc5dce04 100644
>>> --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
>>> +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
>>> @@ -9,6 +9,7 @@ title: RISC-V bindings for 'cpus' DT nodes
>>> maintainers:
>>> - Paul Walmsley <[email protected]>
>>> - Palmer Dabbelt <[email protected]>
>>> + - Conor Dooley <[email protected]>
>>>
>>> description: |
>>> This document uses some terminology common to the RISC-V community
>>> @@ -79,9 +80,7 @@ properties:
>>> insensitive, letters in the riscv,isa string must be all
>>> lowercase to simplify parsing.
>>> $ref: "/schemas/types.yaml#/definitions/string"
>>> - enum:
>>> - - rv64imac
>>> - - rv64imafdc
>>> + pattern: ^rv(?:64|32)imaf?d?q?c?b?v?k?h?(?:(?:_[zsh][imafdqcbvksh]|_x)(?:[a-z])+)*$
>
> Actually we don't want S and H extensions to be treated like Z, but rather
> like X. Only Z extensions have the category character convention. (And I'm
> still tempted to just drop the enforcement from Z too, since it adds
> additional maintenance and we've already settled for something less than
> complete.)

Yeah, I think you are probably right there. I'll let this one sit for
a few days & if nothing else comes in I'll send a v4 with the charset
restriction dropped.

Thanks Drew,
Conor.

>
> Thanks,
> drew
>
>>>
>>> # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here
>>> timebase-frequency: false
>>> --
>>> 2.37.1
>>>
>>
>> Reviewed-by: Andrew Jones <[email protected]>

2022-08-18 16:44:10

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 2/4] dt-bindings: interrupt-controller: sifive,plic: add legacy riscv compatible

On Wed, Aug 17, 2022 at 09:12:11PM +0100, Conor Dooley wrote:
> From: Conor Dooley <[email protected]>
>
> While "real" hardware might not use the compatible string "riscv,plic0"
> it is present in the driver & QEMU uses it for automatically generated
> virt machine dtbs. To avoid dt-validate problems with QEMU produced
> dtbs, such as the following, add it to the binding.
>
> riscv-virt.dtb: plic@c000000: compatible: 'oneOf' conditional failed, one must be fixed:
> 'sifive,plic-1.0.0' is not one of ['sifive,fu540-c000-plic', 'starfive,jh7100-plic', 'canaan,k210-plic']
> 'sifive,plic-1.0.0' is not one of ['allwinner,sun20i-d1-plic']
> 'sifive,plic-1.0.0' was expected
> 'thead,c900-plic' was expected
> riscv-virt.dtb: plic@c000000: '#address-cells' is a required property
>
> Reported-by: Rob Herring <[email protected]>
> Link: https://lore.kernel.org/linux-riscv/[email protected]/
> Signed-off-by: Conor Dooley <[email protected]>
> ---
> .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml | 5 +++++
> 1 file changed, 5 insertions(+)

Reviewed-by: Rob Herring <[email protected]>

2022-08-18 17:03:32

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] dt-bindings: timer: sifive,clint: add legacy riscv compatible

On Wed, Aug 17, 2022 at 09:12:10PM +0100, Conor Dooley wrote:
> From: Conor Dooley <[email protected]>
>
> While "real" hardware might not use the compatible string "riscv,clint0"
> it is present in the driver & QEMU uses it for automatically generated
> virt machine dtbs. To avoid dt-validate problems with QEMU produced
> dtbs, such as the following, add it to the binding.
>
> riscv-virt.dtb: clint@2000000: compatible:0: 'sifive,clint0' is not one of ['sifive,fu540-c000-clint', 'starfive,jh7100-clint', 'canaan,k210-clint']
>
> Reported-by: Rob Herring <[email protected]>
> Link: https://lore.kernel.org/linux-riscv/[email protected]/
> Signed-off-by: Conor Dooley <[email protected]>
> ---
> .../bindings/timer/sifive,clint.yaml | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)

Reviewed-by: Rob Herring <[email protected]>