[PATCH v3 0/2] riscv,isa fixups

[PATCH v3 0/2] riscv,isa fixups

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

I noticed ~today~ while looking at the isa manual that I had not
accounted for another couple of edge cases with my regex. As before, I
think attempting to validate the canonical order for multiletter stuff
makes no sense - but we should totally try to avoid false-positives for
combinations that are known to be valid.

All I've changed for v2 was collecting tags & adding in the missing
commit reference that Heiko pointed out.

v3 fixes an issue Jess spotted - it's *any* multi-letter extension that
can come immediately after the single-letter ones, not just ones
starting with Z.

@Palmer, either you can take this once the DT folks have ACKed it if you
like, or I will take onto some v6.2-rcN fixes branch. I don't think that
there is any urgency :)

Thanks,
Conor.

CC: Jessica Clarke <jrtc27@jrtc27.com>
CC: Conor Dooley <conor@kernel.org>
CC: Rob Herring <robh+dt@kernel.org>
CC: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
CC: Paul Walmsley <paul.walmsley@sifive.com>
CC: Palmer Dabbelt <palmer@dabbelt.com>
CC: Albert Ou <aou@eecs.berkeley.edu>
CC: Heiko Stuebner <heiko@sntech.de>
CC: Andrew Jones <ajones@ventanamicro.com>
CC: Guo Ren <guoren@kernel.org>
CC: linux-riscv@lists.infradead.org
CC: devicetree@vger.kernel.org
CC: linux-kernel@vger.kernel.org

Conor Dooley (2):
  dt-bindings: riscv: fix underscore requirement for multi-letter
    extensions
  dt-bindings: riscv: fix single letter canonical order

 Documentation/devicetree/bindings/riscv/cpus.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.38.1

[PATCH v3 1/2] dt-bindings: riscv: fix underscore requirement for multi-letter extensions

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

The RISC-V ISA Manual allows the first multi-letter extension to avoid
a leading underscore. Underscores are only required between multi-letter
extensions.

The dt-binding does not validate that a multi-letter extension is
canonically ordered, as that'd need an even worse regex than is here,
but it should not fail validation for valid ISA strings.

Allow the first multi-letter extension to appear immediately after
the single-letter extensions.

Link: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-unpriv-pdf-from-asciidoc-15112022 # Chapter 29.5
Fixes: 299824e68bd0 ("dt-bindings: riscv: add new riscv,isa strings for emulators")
Acked-by: Guo Ren <guoren@kernel.org>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 Documentation/devicetree/bindings/riscv/cpus.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
index 90a7cabf58fe..97659bb71811 100644
--- a/Documentation/devicetree/bindings/riscv/cpus.yaml
+++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
@@ -80,7 +80,7 @@ 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?(?:_[hsxz](?:[a-z])+)*$
+    pattern: ^rv(?:64|32)imaf?d?q?c?b?v?k?h?(?:[hsxz](?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$
 
   # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here
   timebase-frequency: false
-- 
2.38.1

[PATCH v3 2/2] dt-bindings: riscv: fix single letter canonical order

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

I used the wikipedia table for ordering extensions when updating the
pattern here in commit 299824e68bd0 ("dt-bindings: riscv: add new
riscv,isa strings for emulators").

Unfortunately that table did not match canonical order, as defined by
the RISC-V ISA Manual, which defines extension ordering in (what is
currently) Table 41, "Standard ISA extension names". Fix things up by
re-sorting v (vector) and adding p (packed-simd) & j (dynamic
languages). The e (reduced integer) and g (general) extensions are still
intentionally left out.

Link: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-unpriv-pdf-from-asciidoc-15112022 # Chapter 29.5
Fixes: 299824e68bd0 ("dt-bindings: riscv: add new riscv,isa strings for emulators")
Acked-by: Guo Ren <guoren@kernel.org>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 Documentation/devicetree/bindings/riscv/cpus.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
index 97659bb71811..d4148418350c 100644
--- a/Documentation/devicetree/bindings/riscv/cpus.yaml
+++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
@@ -80,7 +80,7 @@ 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?(?:[hsxz](?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$
+    pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:[hsxz](?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$
 
   # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here
   timebase-frequency: false
-- 
2.38.1

Re: [PATCH v3 1/2] dt-bindings: riscv: fix underscore requirement for multi-letter extensions

[Rob Herring]

On Mon, 05 Dec 2022 17:44:59 +0000, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> The RISC-V ISA Manual allows the first multi-letter extension to avoid
> a leading underscore. Underscores are only required between multi-letter
> extensions.
> 
> The dt-binding does not validate that a multi-letter extension is
> canonically ordered, as that'd need an even worse regex than is here,
> but it should not fail validation for valid ISA strings.
> 
> Allow the first multi-letter extension to appear immediately after
> the single-letter extensions.
> 
> Link: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-unpriv-pdf-from-asciidoc-15112022 # Chapter 29.5
> Fixes: 299824e68bd0 ("dt-bindings: riscv: add new riscv,isa strings for emulators")
> Acked-by: Guo Ren <guoren@kernel.org>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  Documentation/devicetree/bindings/riscv/cpus.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 2/2] dt-bindings: riscv: fix single letter canonical order

[Rob Herring]

On Mon, 05 Dec 2022 17:45:00 +0000, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> I used the wikipedia table for ordering extensions when updating the
> pattern here in commit 299824e68bd0 ("dt-bindings: riscv: add new
> riscv,isa strings for emulators").
> 
> Unfortunately that table did not match canonical order, as defined by
> the RISC-V ISA Manual, which defines extension ordering in (what is
> currently) Table 41, "Standard ISA extension names". Fix things up by
> re-sorting v (vector) and adding p (packed-simd) & j (dynamic
> languages). The e (reduced integer) and g (general) extensions are still
> intentionally left out.
> 
> Link: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-unpriv-pdf-from-asciidoc-15112022 # Chapter 29.5
> Fixes: 299824e68bd0 ("dt-bindings: riscv: add new riscv,isa strings for emulators")
> Acked-by: Guo Ren <guoren@kernel.org>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
> Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  Documentation/devicetree/bindings/riscv/cpus.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 0/2] riscv,isa fixups

[Palmer Dabbelt]

On Mon, 05 Dec 2022 09:44:58 PST (-0800), Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> I noticed ~today~ while looking at the isa manual that I had not
> accounted for another couple of edge cases with my regex. As before, I
> think attempting to validate the canonical order for multiletter stuff
> makes no sense - but we should totally try to avoid false-positives for
> combinations that are known to be valid.
>
> All I've changed for v2 was collecting tags & adding in the missing
> commit reference that Heiko pointed out.
>
> v3 fixes an issue Jess spotted - it's *any* multi-letter extension that
> can come immediately after the single-letter ones, not just ones
> starting with Z.
>
> @Palmer, either you can take this once the DT folks have ACKed it if you
> like, or I will take onto some v6.2-rcN fixes branch. I don't think that
> there is any urgency :)

Sorry I missed these.  

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>

We can just do a shared tag if you have stuff that depends on them?  
That'll let me keep fixes clean, as I'm running the DT stuff too now.  
Though maybe it doesn't matter so much because I'm not really taking any 
DT stuff.

> Thanks,
> Conor.
>
> CC: Jessica Clarke <jrtc27@jrtc27.com>
> CC: Conor Dooley <conor@kernel.org>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> CC: Paul Walmsley <paul.walmsley@sifive.com>
> CC: Palmer Dabbelt <palmer@dabbelt.com>
> CC: Albert Ou <aou@eecs.berkeley.edu>
> CC: Heiko Stuebner <heiko@sntech.de>
> CC: Andrew Jones <ajones@ventanamicro.com>
> CC: Guo Ren <guoren@kernel.org>
> CC: linux-riscv@lists.infradead.org
> CC: devicetree@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
>
> Conor Dooley (2):
>   dt-bindings: riscv: fix underscore requirement for multi-letter
>     extensions
>   dt-bindings: riscv: fix single letter canonical order
>
>  Documentation/devicetree/bindings/riscv/cpus.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Re: [PATCH v3 0/2] riscv,isa fixups

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1566 bytes --]

On Fri, Jan 06, 2023 at 10:27:48AM -0800, Palmer Dabbelt wrote:
> On Mon, 05 Dec 2022 09:44:58 PST (-0800), Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley@microchip.com>
> > 
> > I noticed ~today~ while looking at the isa manual that I had not
> > accounted for another couple of edge cases with my regex. As before, I
> > think attempting to validate the canonical order for multiletter stuff
> > makes no sense - but we should totally try to avoid false-positives for
> > combinations that are known to be valid.
> > 
> > All I've changed for v2 was collecting tags & adding in the missing
> > commit reference that Heiko pointed out.
> > 
> > v3 fixes an issue Jess spotted - it's *any* multi-letter extension that
> > can come immediately after the single-letter ones, not just ones
> > starting with Z.
> > 
> > @Palmer, either you can take this once the DT folks have ACKed it if you
> > like, or I will take onto some v6.2-rcN fixes branch. I don't think that
> > there is any urgency :)
> 
> Sorry I missed these.
> 
> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
> Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
> 
> We can just do a shared tag if you have stuff that depends on them?  That'll
> let me keep fixes clean, as I'm running the DT stuff too now.  Though maybe
> it doesn't matter so much because I'm not really taking any DT stuff.

Nah, shared tag is not needed.
Noone's got a DT with multiletter stuff that needs to be merged yet.
Feel free to take on on fixes or for-next.

Thanks,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 0/2] riscv,isa fixups

[patchwork-bot+linux-riscv]

Hello:

This series was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <palmer@rivosinc.com>:

On Mon,  5 Dec 2022 17:44:58 +0000 you wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> I noticed ~today~ while looking at the isa manual that I had not
> accounted for another couple of edge cases with my regex. As before, I
> think attempting to validate the canonical order for multiletter stuff
> makes no sense - but we should totally try to avoid false-positives for
> combinations that are known to be valid.
> 
> [...]

Here is the summary with links:
  - [v3,1/2] dt-bindings: riscv: fix underscore requirement for multi-letter extensions
    https://git.kernel.org/riscv/c/ec64efc4966e
  - [v3,2/2] dt-bindings: riscv: fix single letter canonical order
    https://git.kernel.org/riscv/c/a943385aa801

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html