2022-11-30 05:56:41

by Jia Jie Ho

[permalink] [raw]
Subject: [PATCH 6/6] riscv: dts: starfive: Add crypto and DMA node for VisionFive 2

Adding StarFive crypto IP and DMA controller node
to VisionFive 2 SoC.

Signed-off-by: Jia Jie Ho <[email protected]>
Signed-off-by: Huan Feng <[email protected]>
---
.../jh7110-starfive-visionfive-v2.dts | 8 +++++
arch/riscv/boot/dts/starfive/jh7110.dtsi | 36 +++++++++++++++++++
2 files changed, 44 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
index 450e920236a5..da2aa4d597f3 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
@@ -115,3 +115,11 @@ &tdm_ext {
&mclk_ext {
clock-frequency = <49152000>;
};
+
+&sec_dma {
+ status = "okay";
+};
+
+&crypto {
+ status = "okay";
+};
diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index 4ac159d79d66..745a5650882c 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -455,5 +455,41 @@ uart5: serial@12020000 {
reg-shift = <2>;
status = "disabled";
};
+
+ sec_dma: sec_dma@16008000 {
+ compatible = "arm,pl080", "arm,primecell";
+ arm,primecell-periphid = <0x00041080>;
+ reg = <0x0 0x16008000 0x0 0x4000>;
+ reg-names = "sec_dma";
+ interrupts = <29>;
+ clocks = <&stgcrg JH7110_STGCLK_SEC_HCLK>,
+ <&stgcrg JH7110_STGCLK_SEC_MISCAHB>;
+ clock-names = "sec_hclk","apb_pclk";
+ resets = <&stgcrg JH7110_STGRST_SEC_TOP_HRESETN>;
+ reset-names = "sec_hre";
+ lli-bus-interface-ahb1;
+ mem-bus-interface-ahb1;
+ memcpy-burst-size = <256>;
+ memcpy-bus-width = <32>;
+ #dma-cells = <2>;
+ status = "disabled";
+ };
+
+ crypto: crypto@16000000 {
+ compatible = "starfive,jh7110-crypto";
+ reg = <0x0 0x16000000 0x0 0x4000>;
+ reg-names = "secreg";
+ clocks = <&stgcrg JH7110_STGCLK_SEC_HCLK>,
+ <&stgcrg JH7110_STGCLK_SEC_MISCAHB>;
+ clock-names = "sec_hclk","sec_ahb";
+ resets = <&stgcrg JH7110_STGRST_SEC_TOP_HRESETN>;
+ reset-names = "sec_hre";
+ enable-side-channel-mitigation;
+ enable-dma;
+ dmas = <&sec_dma 1 2>,
+ <&sec_dma 0 2>;
+ dma-names = "sec_m","sec_p";
+ status = "disabled";
+ };
};
};
--
2.25.1


2022-11-30 09:33:56

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 6/6] riscv: dts: starfive: Add crypto and DMA node for VisionFive 2

Hey Jia Jie Ho,

On 30/11/2022 05:52, Jia Jie Ho wrote:
> [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Adding StarFive crypto IP and DMA controller node
> to VisionFive 2 SoC.
>
> Signed-off-by: Jia Jie Ho <[email protected]>
> Signed-off-by: Huan Feng <[email protected]>

Out of curiosity, what was Huan Feng's contribution to this patch?
Did they co-develop it, or is there some other reason?

> ---
> .../jh7110-starfive-visionfive-v2.dts | 8 +++++
> arch/riscv/boot/dts/starfive/jh7110.dtsi | 36 +++++++++++++++++++

I figure Emil will likely see anyway, but whenever you get actual
review comments and send a v2 - please don't drop people that
get_maintainer.pl tells you are responsible for the code in
question.

> 2 files changed, 44 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> index 450e920236a5..da2aa4d597f3 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> @@ -115,3 +115,11 @@ &tdm_ext {
> &mclk_ext {
> clock-frequency = <49152000>;
> };
> +
> +&sec_dma {
> + status = "okay";
> +};
> +
> +&crypto {
> + status = "okay";
> +};

In what scenario would you not want to have these enabled?

Thanks,
Conor.

> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> index 4ac159d79d66..745a5650882c 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -455,5 +455,41 @@ uart5: serial@12020000 {
> reg-shift = <2>;
> status = "disabled";
> };
> +
> + sec_dma: sec_dma@16008000 {
> + compatible = "arm,pl080", "arm,primecell";
> + arm,primecell-periphid = <0x00041080>;
> + reg = <0x0 0x16008000 0x0 0x4000>;
> + reg-names = "sec_dma";
> + interrupts = <29>;
> + clocks = <&stgcrg JH7110_STGCLK_SEC_HCLK>,
> + <&stgcrg JH7110_STGCLK_SEC_MISCAHB>;
> + clock-names = "sec_hclk","apb_pclk";
> + resets = <&stgcrg JH7110_STGRST_SEC_TOP_HRESETN>;
> + reset-names = "sec_hre";
> + lli-bus-interface-ahb1;
> + mem-bus-interface-ahb1;
> + memcpy-burst-size = <256>;
> + memcpy-bus-width = <32>;
> + #dma-cells = <2>;
> + status = "disabled";
> + };
> +
> + crypto: crypto@16000000 {
> + compatible = "starfive,jh7110-crypto";
> + reg = <0x0 0x16000000 0x0 0x4000>;
> + reg-names = "secreg";
> + clocks = <&stgcrg JH7110_STGCLK_SEC_HCLK>,
> + <&stgcrg JH7110_STGCLK_SEC_MISCAHB>;
> + clock-names = "sec_hclk","sec_ahb";
> + resets = <&stgcrg JH7110_STGRST_SEC_TOP_HRESETN>;
> + reset-names = "sec_hre";
> + enable-side-channel-mitigation;
> + enable-dma;
> + dmas = <&sec_dma 1 2>,
> + <&sec_dma 0 2>;
> + dma-names = "sec_m","sec_p";
> + status = "disabled";
> + };
> };
> };
> --
> 2.25.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2022-11-30 13:24:34

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 6/6] riscv: dts: starfive: Add crypto and DMA node for VisionFive 2

On 30/11/2022 06:52, Jia Jie Ho wrote:
> Adding StarFive crypto IP and DMA controller node
> to VisionFive 2 SoC.
>
> Signed-off-by: Jia Jie Ho <[email protected]>
> Signed-off-by: Huan Feng <[email protected]>
> ---
> .../jh7110-starfive-visionfive-v2.dts | 8 +++++
> arch/riscv/boot/dts/starfive/jh7110.dtsi | 36 +++++++++++++++++++
> 2 files changed, 44 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> index 450e920236a5..da2aa4d597f3 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> @@ -115,3 +115,11 @@ &tdm_ext {
> &mclk_ext {
> clock-frequency = <49152000>;
> };
> +
> +&sec_dma {
> + status = "okay";
> +};
> +
> +&crypto {
> + status = "okay";
> +};
> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> index 4ac159d79d66..745a5650882c 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -455,5 +455,41 @@ uart5: serial@12020000 {
> reg-shift = <2>;
> status = "disabled";
> };
> +
> + sec_dma: sec_dma@16008000 {

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

No underscores in node names.

Does not look like you tested the DTS against bindings. Please run `make
dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst
for instructions).


Best regards,
Krzysztof

2022-12-01 06:22:44

by Jia Jie Ho

[permalink] [raw]
Subject: RE: [PATCH 6/6] riscv: dts: starfive: Add crypto and DMA node for VisionFive 2



> -----Original Message-----
> From: [email protected] <[email protected]>
> Sent: Wednesday, November 30, 2022 5:31 PM
> To: JiaJie Ho <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH 6/6] riscv: dts: starfive: Add crypto and DMA node for
> VisionFive 2
>
> Hey Jia Jie Ho,
>
> On 30/11/2022 05:52, Jia Jie Ho wrote:
> > [You don't often get email from [email protected]. Learn why
> > this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know
> > the content is safe
> >
> > Adding StarFive crypto IP and DMA controller node to VisionFive 2 SoC.
> >
> > Signed-off-by: Jia Jie Ho <[email protected]>
> > Signed-off-by: Huan Feng <[email protected]>
>
> Out of curiosity, what was Huan Feng's contribution to this patch?
> Did they co-develop it, or is there some other reason?
>
Hi Conor,
Yes, Huan Feng co-developed this driver.

> > ---
> > .../jh7110-starfive-visionfive-v2.dts | 8 +++++
> > arch/riscv/boot/dts/starfive/jh7110.dtsi | 36 +++++++++++++++++++
>
> I figure Emil will likely see anyway, but whenever you get actual review
> comments and send a v2 - please don't drop people that get_maintainer.pl
> tells you are responsible for the code in question.
>
I will include everyone involved when sending the new patch series.

> > 2 files changed, 44 insertions(+)
> >
> > diff --git
> > a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> > b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> > index 450e920236a5..da2aa4d597f3 100644
> > --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> > +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> > @@ -115,3 +115,11 @@ &tdm_ext {
> > &mclk_ext {
> > clock-frequency = <49152000>;
> > };
> > +
> > +&sec_dma {
> > + status = "okay";
> > +};
> > +
> > +&crypto {
> > + status = "okay";
> > +};
>
> In what scenario would you not want to have these enabled?
>
> Thanks,
> Conor.
>
These drivers are always enabled.
Is everything ok with the dts node entries?
Thank you for spending time reviewing and providing suggestions for this patch.

Regards,
Jia Jie

> > diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > index 4ac159d79d66..745a5650882c 100644
> > --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > @@ -455,5 +455,41 @@ uart5: serial@12020000 {
> > reg-shift = <2>;
> > status = "disabled";
> > };
> > +
> > + sec_dma: sec_dma@16008000 {
> > + compatible = "arm,pl080", "arm,primecell";
> > + arm,primecell-periphid = <0x00041080>;
> > + reg = <0x0 0x16008000 0x0 0x4000>;
> > + reg-names = "sec_dma";
> > + interrupts = <29>;
> > + clocks = <&stgcrg JH7110_STGCLK_SEC_HCLK>,
> > + <&stgcrg JH7110_STGCLK_SEC_MISCAHB>;
> > + clock-names = "sec_hclk","apb_pclk";
> > + resets = <&stgcrg JH7110_STGRST_SEC_TOP_HRESETN>;
> > + reset-names = "sec_hre";
> > + lli-bus-interface-ahb1;
> > + mem-bus-interface-ahb1;
> > + memcpy-burst-size = <256>;
> > + memcpy-bus-width = <32>;
> > + #dma-cells = <2>;
> > + status = "disabled";
> > + };
> > +
> > + crypto: crypto@16000000 {
> > + compatible = "starfive,jh7110-crypto";
> > + reg = <0x0 0x16000000 0x0 0x4000>;
> > + reg-names = "secreg";
> > + clocks = <&stgcrg JH7110_STGCLK_SEC_HCLK>,
> > + <&stgcrg JH7110_STGCLK_SEC_MISCAHB>;
> > + clock-names = "sec_hclk","sec_ahb";
> > + resets = <&stgcrg JH7110_STGRST_SEC_TOP_HRESETN>;
> > + reset-names = "sec_hre";
> > + enable-side-channel-mitigation;
> > + enable-dma;
> > + dmas = <&sec_dma 1 2>,
> > + <&sec_dma 0 2>;
> > + dma-names = "sec_m","sec_p";
> > + status = "disabled";
> > + };
> > };
> > };
> > --
> > 2.25.1
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

2022-12-01 07:26:27

by Jia Jie Ho

[permalink] [raw]
Subject: RE: [PATCH 6/6] riscv: dts: starfive: Add crypto and DMA node for VisionFive 2



> -----Original Message-----
> From: Krzysztof Kozlowski <[email protected]>
> Sent: Wednesday, November 30, 2022 9:22 PM
> To: JiaJie Ho <[email protected]>; Herbert Xu
> <[email protected]>; David S . Miller <[email protected]>;
> Rob Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH 6/6] riscv: dts: starfive: Add crypto and DMA node for
> VisionFive 2
>
> On 30/11/2022 06:52, Jia Jie Ho wrote:
> > Adding StarFive crypto IP and DMA controller node to VisionFive 2 SoC.
> >
> > Signed-off-by: Jia Jie Ho <[email protected]>
> > Signed-off-by: Huan Feng <[email protected]>
> > ---
> > .../jh7110-starfive-visionfive-v2.dts | 8 +++++
> > arch/riscv/boot/dts/starfive/jh7110.dtsi | 36 +++++++++++++++++++
> > 2 files changed, 44 insertions(+)
> >
> > diff --git
> > a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> > b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> > index 450e920236a5..da2aa4d597f3 100644
> > --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> > +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> > @@ -115,3 +115,11 @@ &tdm_ext {
> > &mclk_ext {
> > clock-frequency = <49152000>;
> > };
> > +
> > +&sec_dma {
> > + status = "okay";
> > +};
> > +
> > +&crypto {
> > + status = "okay";
> > +};
> > diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > index 4ac159d79d66..745a5650882c 100644
> > --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > @@ -455,5 +455,41 @@ uart5: serial@12020000 {
> > reg-shift = <2>;
> > status = "disabled";
> > };
> > +
> > + sec_dma: sec_dma@16008000 {
>
> Node names should be generic.
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-
> devicetree-basics.html#generic-names-recommendation
>
> No underscores in node names.
>
> Does not look like you tested the DTS against bindings. Please run `make
> dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst
> for instructions).

I'll fix the node name and run dtbs_check for the revised series.
Really appreciate your time and effort reviewing this patch.

Thanks,
Jia Jie

2022-12-01 18:12:15

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 6/6] riscv: dts: starfive: Add crypto and DMA node for VisionFive 2

On Thu, Dec 01, 2022 at 06:17:26AM +0000, JiaJie Ho wrote:
> > -----Original Message-----
> > From: [email protected] <[email protected]>

> > Hey Jia Jie Ho,
> >
> > On 30/11/2022 05:52, Jia Jie Ho wrote:
> > > [You don't often get email from [email protected]. Learn why
> > > this is important at https://aka.ms/LearnAboutSenderIdentification ]
> > >
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you know
> > > the content is safe
> > >
> > > Adding StarFive crypto IP and DMA controller node to VisionFive 2 SoC.
> > >
> > > Signed-off-by: Jia Jie Ho <[email protected]>
> > > Signed-off-by: Huan Feng <[email protected]>
> >
> > Out of curiosity, what was Huan Feng's contribution to this patch?
> > Did they co-develop it, or is there some other reason?
> >
> Hi Conor,
> Yes, Huan Feng co-developed this driver.

In that case, the SoB block should look like:

Co-developed-by: Huan Feng <[email protected]>
Signed-off-by: Huan Feng <[email protected]>
Signed-off-by: Jia Jie Ho <[email protected]>

Similarly for any other patches they may have co-developed :)

> > >
> > > diff --git
> > > a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> > > b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> > > index 450e920236a5..da2aa4d597f3 100644
> > > --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> > > +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> > > @@ -115,3 +115,11 @@ &tdm_ext {
> > > &mclk_ext {
> > > clock-frequency = <49152000>;
> > > };
> > > +
> > > +&sec_dma {
> > > + status = "okay";
> > > +};
> > > +
> > > +&crypto {
> > > + status = "okay";
> > > +};
> >
> > In what scenario would you not want to have these enabled?

> These drivers are always enabled.
> Is everything ok with the dts node entries?

If the hardware is always present, why not drop the "disabled" in
jh7110.dtsi & these two entries marking them as "okay" in the .dts?

> > > diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > > b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > > index 4ac159d79d66..745a5650882c 100644
> > > --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > > +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> > > @@ -455,5 +455,41 @@ uart5: serial@12020000 {
> > > reg-shift = <2>;
> > > status = "disabled";
> > > };
> > > +
> > > + sec_dma: sec_dma@16008000 {

> > > + status = "disabled";

> > > + };
> > > +
> > > + crypto: crypto@16000000 {

> > > + status = "disabled";

> > > + };
> > > };
> > > };

Thanks,
Conor.

2022-12-06 03:57:06

by Jia Jie Ho

[permalink] [raw]
Subject: RE: [PATCH 6/6] riscv: dts: starfive: Add crypto and DMA node for VisionFive 2



> -----Original Message-----
> From: Conor Dooley <[email protected]>
> Sent: Friday, December 2, 2022 2:04 AM
> To: JiaJie Ho <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH 6/6] riscv: dts: starfive: Add crypto and DMA node for
> VisionFive 2
>
> On Thu, Dec 01, 2022 at 06:17:26AM +0000, JiaJie Ho wrote:
> > > -----Original Message-----
> > > From: [email protected] <[email protected]>
>
> In that case, the SoB block should look like:
>
> Co-developed-by: Huan Feng <[email protected]>
> Signed-off-by: Huan Feng <[email protected]>
> Signed-off-by: Jia Jie Ho <[email protected]>
>
> Similarly for any other patches they may have co-developed :)
>

I'll add these in the v2.

> If the hardware is always present, why not drop the "disabled" in jh7110.dtsi
> & these two entries marking them as "okay" in the .dts?
>

Okay, I'll update these too.
Thanks again for the suggestions.

Best regards,
Jia Jie