| Message ID | 20221130055214.2416888-1-jiajie.ho@starfivetech.com |
|---|---|
| Headers | show |
| Series | crypto: starfive: Add driver for cryptographic engine | expand |
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 <jiajie.ho@starfivetech.com> > Signed-off-by: Huan Feng <huan.feng@starfivetech.com> > --- > .../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
> -----Original Message----- > From: Conor.Dooley@microchip.com <Conor.Dooley@microchip.com> > Sent: Wednesday, November 30, 2022 5:31 PM > To: JiaJie Ho <jiajie.ho@starfivetech.com> > Cc: robh+dt@kernel.org; herbert@gondor.apana.org.au; linux- > crypto@vger.kernel.org; kernel@esmil.dk; davem@davemloft.net; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux- > riscv@lists.infradead.org; krzysztof.kozlowski+dt@linaro.org > 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 jiajie.ho@starfivetech.com. 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 <jiajie.ho@starfivetech.com> > > Signed-off-by: Huan Feng <huan.feng@starfivetech.com> > > 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 > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Wednesday, November 30, 2022 9:22 PM > To: JiaJie Ho <jiajie.ho@starfivetech.com>; Herbert Xu > <herbert@gondor.apana.org.au>; David S . Miller <davem@davemloft.net>; > Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@linaro.org> > Cc: linux-crypto@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-riscv@lists.infradead.org > 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 <jiajie.ho@starfivetech.com> > > Signed-off-by: Huan Feng <huan.feng@starfivetech.com> > > --- > > .../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
On Thu, Dec 01, 2022 at 06:17:26AM +0000, JiaJie Ho wrote: > > -----Original Message----- > > From: Conor.Dooley@microchip.com <Conor.Dooley@microchip.com> > > Hey Jia Jie Ho, > > > > On 30/11/2022 05:52, Jia Jie Ho wrote: > > > [You don't often get email from jiajie.ho@starfivetech.com. 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 <jiajie.ho@starfivetech.com> > > > Signed-off-by: Huan Feng <huan.feng@starfivetech.com> > > > > 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 <huan.feng@starfivetech.com> Signed-off-by: Huan Feng <huan.feng@starfivetech.com> Signed-off-by: Jia Jie Ho <jiajie.ho@starfivetech.com> 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.
> -----Original Message----- > From: Conor Dooley <conor@kernel.org> > Sent: Friday, December 2, 2022 2:04 AM > To: JiaJie Ho <jiajie.ho@starfivetech.com> > Cc: Conor.Dooley@microchip.com; robh+dt@kernel.org; > herbert@gondor.apana.org.au; linux-crypto@vger.kernel.org; > kernel@esmil.dk; davem@davemloft.net; devicetree@vger.kernel.org; > linux-kernel@vger.kernel.org; linux-riscv@lists.infradead.org; > krzysztof.kozlowski+dt@linaro.org > 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: Conor.Dooley@microchip.com <Conor.Dooley@microchip.com> > > In that case, the SoB block should look like: > > Co-developed-by: Huan Feng <huan.feng@starfivetech.com> > Signed-off-by: Huan Feng <huan.feng@starfivetech.com> > Signed-off-by: Jia Jie Ho <jiajie.ho@starfivetech.com> > > 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
> -----Original Message----- > From: JiaJie Ho <jiajie.ho@starfivetech.com> > Sent: Wednesday, November 30, 2022 1:52 PM > To: Herbert Xu <herbert@gondor.apana.org.au>; David S . Miller > <davem@davemloft.net>; Rob Herring <robh+dt@kernel.org>; Krzysztof > Kozlowski <krzysztof.kozlowski+dt@linaro.org> > Cc: linux-crypto@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-riscv@lists.infradead.org; JiaJie Ho > <jiajie.ho@starfivetech.com> > Subject: [PATCH 0/6] crypto: starfive: Add driver for cryptographic engine > > This patch series adds kernel driver support for Starfive crypto engine. > The engine supports hardware acceleration for HMAC/hash functions, AES > block cipher operations and RSA. The first patch adds basic driver for device > probe and DMA init. The subsequent patches adds supported crypto > primitives to the driver which include hash functions, AES and RSA. Patch 5 > adds documentation to describe device tree bindings and the last patch adds > device node to VisionFive 2 dts. > > The driver has been tested with crypto selftest and additional test. > > This patch series depends on the following patches: > https://patchwork.kernel.org/project/linux- > riscv/cover/20221118010627.70576-1-hal.feng@starfivetech.com/ > https://patchwork.kernel.org/project/linux- > riscv/cover/20221118011714.70877-1-hal.feng@starfivetech.com/ > > Jia Jie Ho (6): > crypto: starfive - Add StarFive crypto engine support > crypto: starfive - Add hash and HMAC support > crypto: starfive - Add AES skcipher and aead support > crypto: starfive - Add Public Key algo support > dt-bindings: crypto: Add bindings for Starfive crypto driver > riscv: dts: starfive: Add crypto and DMA node for VisionFive 2 > Hi Herbert/David, Could you please help to review and provide comments on this patch series? Thank you in advance. Best regards, Jia Jie
On 08/12/2022 10:09, JiaJie Ho wrote: >> >> The driver has been tested with crypto selftest and additional test. >> >> This patch series depends on the following patches: >> https://patchwork.kernel.org/project/linux- >> riscv/cover/20221118010627.70576-1-hal.feng@starfivetech.com/ >> https://patchwork.kernel.org/project/linux- >> riscv/cover/20221118011714.70877-1-hal.feng@starfivetech.com/ >> >> Jia Jie Ho (6): >> crypto: starfive - Add StarFive crypto engine support >> crypto: starfive - Add hash and HMAC support >> crypto: starfive - Add AES skcipher and aead support >> crypto: starfive - Add Public Key algo support >> dt-bindings: crypto: Add bindings for Starfive crypto driver >> riscv: dts: starfive: Add crypto and DMA node for VisionFive 2 >> > > Hi Herbert/David, > > Could you please help to review and provide comments on this patch series? > Thank you in advance. You received some comments so the expectation is to send a v2. Best regards, Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Thursday, December 8, 2022 5:28 PM > To: JiaJie Ho <jiajie.ho@starfivetech.com>; Herbert Xu > <herbert@gondor.apana.org.au>; David S . Miller <davem@davemloft.net>; > Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@linaro.org> > Cc: linux-crypto@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-riscv@lists.infradead.org > Subject: Re: [PATCH 0/6] crypto: starfive: Add driver for cryptographic engine > > On 08/12/2022 10:09, JiaJie Ho wrote: > > > > Hi Herbert/David, > > > > Could you please help to review and provide comments on this patch series? > > Thank you in advance. > > You received some comments so the expectation is to send a v2. > Sure, I'll do that then. Thanks Jia Jie
On Thu, 08 Dec 2022 01:35:10 PST (-0800), jiajie.ho@starfivetech.com wrote: > > >> -----Original Message----- >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> Sent: Thursday, December 8, 2022 5:28 PM >> To: JiaJie Ho <jiajie.ho@starfivetech.com>; Herbert Xu >> <herbert@gondor.apana.org.au>; David S . Miller <davem@davemloft.net>; >> Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski >> <krzysztof.kozlowski+dt@linaro.org> >> Cc: linux-crypto@vger.kernel.org; devicetree@vger.kernel.org; linux- >> kernel@vger.kernel.org; linux-riscv@lists.infradead.org >> Subject: Re: [PATCH 0/6] crypto: starfive: Add driver for cryptographic engine >> >> On 08/12/2022 10:09, JiaJie Ho wrote: >> > >> > Hi Herbert/David, >> > >> > Could you please help to review and provide comments on this patch series? >> > Thank you in advance. >> >> You received some comments so the expectation is to send a v2. >> > > Sure, I'll do that then. Not sure if I missed it, but I can't find a v2.
> -----Original Message----- > From: Palmer Dabbelt <palmer@dabbelt.com> > Sent: Tuesday, December 13, 2022 2:20 PM > To: JiaJie Ho <jiajie.ho@starfivetech.com> > Cc: krzysztof.kozlowski@linaro.org; herbert@gondor.apana.org.au; > davem@davemloft.net; robh+dt@kernel.org; > krzysztof.kozlowski+dt@linaro.org; linux-crypto@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux- > riscv@lists.infradead.org > Subject: RE: [PATCH 0/6] crypto: starfive: Add driver for cryptographic engine > > >> > >> You received some comments so the expectation is to send a v2. > >> > > > > Sure, I'll do that then. > > Not sure if I missed it, but I can't find a v2. Hi Palmer, I'm still working on the patches. I'll send them out soon. Thanks, Jia Jie