diff mbox series

[RFC,v2,6/6] riscv: dts: thead: convert isa detection to new properties

Message ID 20230922081351.30239-8-conor@kernel.org (mailing list archive)
State Superseded
Commit 0804f3bec9e9a3e9f3b5431ac9a11417041bc4c2
Delegated to: Geert Uytterhoeven
Headers show
Series riscv,isa-extensions additions | expand

Commit Message

Conor Dooley Sept. 22, 2023, 8:13 a.m. UTC
From: Conor Dooley <conor.dooley@microchip.com>

Convert the th1520 devicetrees to use the new properties
"riscv,isa-base" & "riscv,isa-extensions".
For compatibility with other projects, "riscv,isa" remains.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/boot/dts/thead/th1520.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Guo Ren Sept. 23, 2023, 7:50 a.m. UTC | #1
On Fri, Sep 22, 2023 at 4:16 PM Conor Dooley <conor@kernel.org> wrote:
>
> From: Conor Dooley <conor.dooley@microchip.com>
>
> Convert the th1520 devicetrees to use the new properties
> "riscv,isa-base" & "riscv,isa-extensions".
> For compatibility with other projects, "riscv,isa" remains.
>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  arch/riscv/boot/dts/thead/th1520.dtsi | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> index ce708183b6f6..723f65487246 100644
> --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> @@ -20,6 +20,9 @@ c910_0: cpu@0 {
>                         compatible = "thead,c910", "riscv";
>                         device_type = "cpu";
>                         riscv,isa = "rv64imafdc";
> +                       riscv,isa-base = "rv64i";
Why not riscv,isa-base = "rv64"? I saw "i" in the riscv,isa-extensions.

Maybe this question should be in this mailing thread and a little late:
https://lore.kernel.org/linux-riscv/20230713-tablet-jimmy-987fea0eb2e1@wendy/

> +                       riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
> +                                              "zifencei", "zihpm";
>                         reg = <0>;
>                         i-cache-block-size = <64>;
>                         i-cache-size = <65536>;
> @@ -41,6 +44,9 @@ c910_1: cpu@1 {
>                         compatible = "thead,c910", "riscv";
>                         device_type = "cpu";
>                         riscv,isa = "rv64imafdc";
> +                       riscv,isa-base = "rv64i";
> +                       riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
> +                                              "zifencei", "zihpm";
>                         reg = <1>;
>                         i-cache-block-size = <64>;
>                         i-cache-size = <65536>;
> @@ -62,6 +68,9 @@ c910_2: cpu@2 {
>                         compatible = "thead,c910", "riscv";
>                         device_type = "cpu";
>                         riscv,isa = "rv64imafdc";
> +                       riscv,isa-base = "rv64i";
> +                       riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
> +                                              "zifencei", "zihpm";
>                         reg = <2>;
>                         i-cache-block-size = <64>;
>                         i-cache-size = <65536>;
> @@ -83,6 +92,9 @@ c910_3: cpu@3 {
>                         compatible = "thead,c910", "riscv";
>                         device_type = "cpu";
>                         riscv,isa = "rv64imafdc";
> +                       riscv,isa-base = "rv64i";
> +                       riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
> +                                              "zifencei", "zihpm";
>                         reg = <3>;
>                         i-cache-block-size = <64>;
>                         i-cache-size = <65536>;
> --
> 2.41.0
>


--
Best Regards
 Guo Ren
Conor Dooley Sept. 23, 2023, 10:25 a.m. UTC | #2
On Sat, Sep 23, 2023 at 03:50:36PM +0800, Guo Ren wrote:
> On Fri, Sep 22, 2023 at 4:16 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > From: Conor Dooley <conor.dooley@microchip.com>
> >
> > Convert the th1520 devicetrees to use the new properties
> > "riscv,isa-base" & "riscv,isa-extensions".
> > For compatibility with other projects, "riscv,isa" remains.
> >
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> >  arch/riscv/boot/dts/thead/th1520.dtsi | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> > index ce708183b6f6..723f65487246 100644
> > --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> > +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> > @@ -20,6 +20,9 @@ c910_0: cpu@0 {
> >                         compatible = "thead,c910", "riscv";
> >                         device_type = "cpu";
> >                         riscv,isa = "rv64imafdc";
> > +                       riscv,isa-base = "rv64i";

> Why not riscv,isa-base = "rv64"? I saw "i" in the riscv,isa-extensions.

I did it that way as a hedge against things changing in the future. I
have little trust in that part of the ISA specifications.
Icenowy Zheng Sept. 23, 2023, 11:22 p.m. UTC | #3
在 2023-09-22星期五的 09:13 +0100,Conor Dooley写道:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> Convert the th1520 devicetrees to use the new properties
> "riscv,isa-base" & "riscv,isa-extensions".
> For compatibility with other projects, "riscv,isa" remains.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  arch/riscv/boot/dts/thead/th1520.dtsi | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi
> b/arch/riscv/boot/dts/thead/th1520.dtsi
> index ce708183b6f6..723f65487246 100644
> --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> @@ -20,6 +20,9 @@ c910_0: cpu@0 {
>                         compatible = "thead,c910", "riscv";
>                         device_type = "cpu";
>                         riscv,isa = "rv64imafdc";
> +                       riscv,isa-base = "rv64i";
> +                       riscv,isa-extensions = "i", "m", "a", "f",
> "d", "c", "zicntr", "zicsr",
> +                                              "zifencei", "zihpm";

Zfh is supported by T-Head C9xx with float too.

In addition, should X extensions get listed here?

>                         reg = <0>;
>                         i-cache-block-size = <64>;
>                         i-cache-size = <65536>;
> @@ -41,6 +44,9 @@ c910_1: cpu@1 {
>                         compatible = "thead,c910", "riscv";
>                         device_type = "cpu";
>                         riscv,isa = "rv64imafdc";
> +                       riscv,isa-base = "rv64i";
> +                       riscv,isa-extensions = "i", "m", "a", "f",
> "d", "c", "zicntr", "zicsr",
> +                                              "zifencei", "zihpm";
>                         reg = <1>;
>                         i-cache-block-size = <64>;
>                         i-cache-size = <65536>;
> @@ -62,6 +68,9 @@ c910_2: cpu@2 {
>                         compatible = "thead,c910", "riscv";
>                         device_type = "cpu";
>                         riscv,isa = "rv64imafdc";
> +                       riscv,isa-base = "rv64i";
> +                       riscv,isa-extensions = "i", "m", "a", "f",
> "d", "c", "zicntr", "zicsr",
> +                                              "zifencei", "zihpm";
>                         reg = <2>;
>                         i-cache-block-size = <64>;
>                         i-cache-size = <65536>;
> @@ -83,6 +92,9 @@ c910_3: cpu@3 {
>                         compatible = "thead,c910", "riscv";
>                         device_type = "cpu";
>                         riscv,isa = "rv64imafdc";
> +                       riscv,isa-base = "rv64i";
> +                       riscv,isa-extensions = "i", "m", "a", "f",
> "d", "c", "zicntr", "zicsr",
> +                                              "zifencei", "zihpm";
>                         reg = <3>;
>                         i-cache-block-size = <64>;
>                         i-cache-size = <65536>;
Conor Dooley Sept. 25, 2023, 3:59 p.m. UTC | #4
On Sun, Sep 24, 2023 at 07:22:30AM +0800, Icenowy Zheng wrote:
> 在 2023-09-22星期五的 09:13 +0100,Conor Dooley写道:
> > From: Conor Dooley <conor.dooley@microchip.com>
> > 
> > Convert the th1520 devicetrees to use the new properties
> > "riscv,isa-base" & "riscv,isa-extensions".
> > For compatibility with other projects, "riscv,isa" remains.
> > 
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> >  arch/riscv/boot/dts/thead/th1520.dtsi | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi
> > b/arch/riscv/boot/dts/thead/th1520.dtsi
> > index ce708183b6f6..723f65487246 100644
> > --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> > +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> > @@ -20,6 +20,9 @@ c910_0: cpu@0 {
> >                         compatible = "thead,c910", "riscv";
> >                         device_type = "cpu";
> >                         riscv,isa = "rv64imafdc";
> > +                       riscv,isa-base = "rv64i";
> > +                       riscv,isa-extensions = "i", "m", "a", "f",
> > "d", "c", "zicntr", "zicsr",
> > +                                              "zifencei", "zihpm";
> 
> Zfh is supported by T-Head C9xx with float too.

You say xx, so just to be sure: Is it always supported, or only with
some config for the IP (I wanna know if I need to look out for it while
reviewing other SoCs etc)?
Also, do you have a link to the documentation for it? English is the
only relevant language I speak, so if the doc is in Chinese, I'll need
some help!

> In addition, should X extensions get listed here?

Yes, but someone who cares about documenting these extensions should do
it ;)

Thanks,
Conor.
Icenowy Zheng Sept. 26, 2023, 3:15 a.m. UTC | #5
在 2023-09-25星期一的 16:59 +0100,Conor Dooley写道:
> On Sun, Sep 24, 2023 at 07:22:30AM +0800, Icenowy Zheng wrote:
> > 在 2023-09-22星期五的 09:13 +0100,Conor Dooley写道:
> > > From: Conor Dooley <conor.dooley@microchip.com>
> > > 
> > > Convert the th1520 devicetrees to use the new properties
> > > "riscv,isa-base" & "riscv,isa-extensions".
> > > For compatibility with other projects, "riscv,isa" remains.
> > > 
> > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > ---
> > >  arch/riscv/boot/dts/thead/th1520.dtsi | 12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi
> > > b/arch/riscv/boot/dts/thead/th1520.dtsi
> > > index ce708183b6f6..723f65487246 100644
> > > --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> > > +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> > > @@ -20,6 +20,9 @@ c910_0: cpu@0 {
> > >                         compatible = "thead,c910", "riscv";
> > >                         device_type = "cpu";
> > >                         riscv,isa = "rv64imafdc";
> > > +                       riscv,isa-base = "rv64i";
> > > +                       riscv,isa-extensions = "i", "m", "a",
> > > "f",
> > > "d", "c", "zicntr", "zicsr",
> > > +                                              "zifencei",
> > > "zihpm";
> > 
> > Zfh is supported by T-Head C9xx with float too.
> 
> You say xx, so just to be sure: Is it always supported, or only with
> some config for the IP (I wanna know if I need to look out for it
> while
> reviewing other SoCs etc)?

I think it's grouped with FD.

> Also, do you have a link to the documentation for it? English is the
> only relevant language I speak, so if the doc is in Chinese, I'll
> need
> some help!

Sorry, but T-Head's official document [1] is only in Chinese. In
addition, in this document half-float is listed as a "T-Head
extension", but the encoding matches Zfh. See 15.6 Appendix B-6 Half
Float (15.6 附录 B-6 浮点半精度指令术语) .

[1]
https://github.com/T-head-Semi/openc910/blob/main/doc/%E7%8E%84%E9%93%81C910%E7%94%A8%E6%88%B7%E6%89%8B%E5%86%8C.pdf

> 
> > In addition, should X extensions get listed here?
> 
> Yes, but someone who cares about documenting these extensions should
> do
> it ;)

Well at least a bunch of Xthead's are now documented.

Maybe they will get appended after this patchset get introduced.

By the way, how to deal with the draft V of C9xx?

> 
> Thanks,
> Conor.
Conor Dooley Sept. 26, 2023, 9:14 a.m. UTC | #6
On Tue, Sep 26, 2023 at 11:15:27AM +0800, Icenowy Zheng wrote:
> 在 2023-09-25星期一的 16:59 +0100,Conor Dooley写道:
> > On Sun, Sep 24, 2023 at 07:22:30AM +0800, Icenowy Zheng wrote:
> > > 在 2023-09-22星期五的 09:13 +0100,Conor Dooley写道:
> > > > From: Conor Dooley <conor.dooley@microchip.com>
> > > > 
> > > > Convert the th1520 devicetrees to use the new properties
> > > > "riscv,isa-base" & "riscv,isa-extensions".
> > > > For compatibility with other projects, "riscv,isa" remains.
> > > > 
> > > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > > ---
> > > >  arch/riscv/boot/dts/thead/th1520.dtsi | 12 ++++++++++++
> > > >  1 file changed, 12 insertions(+)
> > > > 
> > > > diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi
> > > > b/arch/riscv/boot/dts/thead/th1520.dtsi
> > > > index ce708183b6f6..723f65487246 100644
> > > > --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> > > > +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> > > > @@ -20,6 +20,9 @@ c910_0: cpu@0 {
> > > >                         compatible = "thead,c910", "riscv";
> > > >                         device_type = "cpu";
> > > >                         riscv,isa = "rv64imafdc";
> > > > +                       riscv,isa-base = "rv64i";
> > > > +                       riscv,isa-extensions = "i", "m", "a",
> > > > "f",
> > > > "d", "c", "zicntr", "zicsr",
> > > > +                                              "zifencei",
> > > > "zihpm";
> > > 
> > > Zfh is supported by T-Head C9xx with float too.
> > 
> > You say xx, so just to be sure: Is it always supported, or only with
> > some config for the IP (I wanna know if I need to look out for it
> > while
> > reviewing other SoCs etc)?
> 
> I think it's grouped with FD.
> 
> > Also, do you have a link to the documentation for it? English is the
> > only relevant language I speak, so if the doc is in Chinese, I'll
> > need
> > some help!
> 
> Sorry, but T-Head's official document [1] is only in Chinese. In
> addition, in this document half-float is listed as a "T-Head
> extension", but the encoding matches Zfh. See 15.6 Appendix B-6 Half
> Float (15.6 附录 B-6 浮点半精度指令术语) .

Hmm, I'm not sure that I want to do this. Since it is only an "I think"
and it's not *actually* advertised as being Zfh but apparently has
matching encoding, I would rather someone who is capable of understanding
that document submitted the patch.

> [1]
> https://github.com/T-head-Semi/openc910/blob/main/doc/%E7%8E%84%E9%93%81C910%E7%94%A8%E6%88%B7%E6%89%8B%E5%86%8C.pdf
> 
> > 
> > > In addition, should X extensions get listed here?
> > 
> > Yes, but someone who cares about documenting these extensions should
> > do
> > it ;)
> 
> Well at least a bunch of Xthead's are now documented.

I meant documented in the dt-binding, not documented in a pdf on GitHub.

> Maybe they will get appended after this patchset get introduced.
> 
> By the way, how to deal with the draft V of C9xx?

Same deal, it'll have to be defined as a vendor extension in the
dt-binding.

Cheers,
Conor.
Jisheng Zhang Oct. 4, 2023, 12:13 p.m. UTC | #7
On Fri, Sep 22, 2023 at 09:13:51AM +0100, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> Convert the th1520 devicetrees to use the new properties
> "riscv,isa-base" & "riscv,isa-extensions".
> For compatibility with other projects, "riscv,isa" remains.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Reviewed-by: Jisheng Zhang <jszhang@kernel.org>
> ---
>  arch/riscv/boot/dts/thead/th1520.dtsi | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> index ce708183b6f6..723f65487246 100644
> --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> @@ -20,6 +20,9 @@ c910_0: cpu@0 {
>  			compatible = "thead,c910", "riscv";
>  			device_type = "cpu";
>  			riscv,isa = "rv64imafdc";
> +			riscv,isa-base = "rv64i";
> +			riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
> +					       "zifencei", "zihpm";
>  			reg = <0>;
>  			i-cache-block-size = <64>;
>  			i-cache-size = <65536>;
> @@ -41,6 +44,9 @@ c910_1: cpu@1 {
>  			compatible = "thead,c910", "riscv";
>  			device_type = "cpu";
>  			riscv,isa = "rv64imafdc";
> +			riscv,isa-base = "rv64i";
> +			riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
> +					       "zifencei", "zihpm";
>  			reg = <1>;
>  			i-cache-block-size = <64>;
>  			i-cache-size = <65536>;
> @@ -62,6 +68,9 @@ c910_2: cpu@2 {
>  			compatible = "thead,c910", "riscv";
>  			device_type = "cpu";
>  			riscv,isa = "rv64imafdc";
> +			riscv,isa-base = "rv64i";
> +			riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
> +					       "zifencei", "zihpm";
>  			reg = <2>;
>  			i-cache-block-size = <64>;
>  			i-cache-size = <65536>;
> @@ -83,6 +92,9 @@ c910_3: cpu@3 {
>  			compatible = "thead,c910", "riscv";
>  			device_type = "cpu";
>  			riscv,isa = "rv64imafdc";
> +			riscv,isa-base = "rv64i";
> +			riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
> +					       "zifencei", "zihpm";
>  			reg = <3>;
>  			i-cache-block-size = <64>;
>  			i-cache-size = <65536>;
> -- 
> 2.41.0
>
Guo Ren Oct. 9, 2023, 1:01 a.m. UTC | #8
On Sat, Sep 23, 2023 at 6:25 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Sat, Sep 23, 2023 at 03:50:36PM +0800, Guo Ren wrote:
> > On Fri, Sep 22, 2023 at 4:16 PM Conor Dooley <conor@kernel.org> wrote:
> > >
> > > From: Conor Dooley <conor.dooley@microchip.com>
> > >
> > > Convert the th1520 devicetrees to use the new properties
> > > "riscv,isa-base" & "riscv,isa-extensions".
> > > For compatibility with other projects, "riscv,isa" remains.
> > >
> > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > ---
> > >  arch/riscv/boot/dts/thead/th1520.dtsi | 12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> > >
> > > diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> > > index ce708183b6f6..723f65487246 100644
> > > --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> > > +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> > > @@ -20,6 +20,9 @@ c910_0: cpu@0 {
> > >                         compatible = "thead,c910", "riscv";
> > >                         device_type = "cpu";
> > >                         riscv,isa = "rv64imafdc";
> > > +                       riscv,isa-base = "rv64i";
>
> > Why not riscv,isa-base = "rv64"? I saw "i" in the riscv,isa-extensions.
>
> I did it that way as a hedge against things changing in the future. I
> have little trust in that part of the ISA specifications.
Okay, that's clear enough.

Acked-by: Guo Ren <guoren@kernel.org>
diff mbox series

Patch

diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index ce708183b6f6..723f65487246 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -20,6 +20,9 @@  c910_0: cpu@0 {
 			compatible = "thead,c910", "riscv";
 			device_type = "cpu";
 			riscv,isa = "rv64imafdc";
+			riscv,isa-base = "rv64i";
+			riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
+					       "zifencei", "zihpm";
 			reg = <0>;
 			i-cache-block-size = <64>;
 			i-cache-size = <65536>;
@@ -41,6 +44,9 @@  c910_1: cpu@1 {
 			compatible = "thead,c910", "riscv";
 			device_type = "cpu";
 			riscv,isa = "rv64imafdc";
+			riscv,isa-base = "rv64i";
+			riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
+					       "zifencei", "zihpm";
 			reg = <1>;
 			i-cache-block-size = <64>;
 			i-cache-size = <65536>;
@@ -62,6 +68,9 @@  c910_2: cpu@2 {
 			compatible = "thead,c910", "riscv";
 			device_type = "cpu";
 			riscv,isa = "rv64imafdc";
+			riscv,isa-base = "rv64i";
+			riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
+					       "zifencei", "zihpm";
 			reg = <2>;
 			i-cache-block-size = <64>;
 			i-cache-size = <65536>;
@@ -83,6 +92,9 @@  c910_3: cpu@3 {
 			compatible = "thead,c910", "riscv";
 			device_type = "cpu";
 			riscv,isa = "rv64imafdc";
+			riscv,isa-base = "rv64i";
+			riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
+					       "zifencei", "zihpm";
 			reg = <3>;
 			i-cache-block-size = <64>;
 			i-cache-size = <65536>;