[v3,0/4] crypto: starfive - Add drivers for crypto engine

Message ID 20230313135646.2077707-1-jiajie.ho@starfivetech.com
Headers
Series crypto: starfive - Add drivers for crypto engine |

Message

JiaJie Ho March 13, 2023, 1:56 p.m. UTC
  This patch series adds kernel driver support for StarFive JH7110 crypto
engine. The first patch adds Documentations for the device and Patch 2
adds device probe and DMA init for the module. Patch 3 adds crypto and
DMA dts node for VisionFive 2 board. Patch 4 adds hash/hmac support to
the module.

Patch 3 needs to be applied on top of:
https://patchwork.kernel.org/project/linux-riscv/patch/20230221024645.127922-18-hal.feng@starfivetech.com/
https://patchwork.kernel.org/project/linux-riscv/cover/20230120024445.244345-1-xingyu.wu@starfivetech.com/

Changes v2->v3:
- Only implement digest and use fallback for other ops (Herbert)
- Use interrupt instead of polling for hash complete (Herbert)
- Remove manual data copy from out-of-bound memory location as it will
  be handled by DMA API. (Christoph & Herbert)

Changes v1->v2:
- Fixed yaml filename and format (Krzysztof)
- Removed unnecessary property names in yaml (Krzysztof)
- Moved of_device_id table close to usage (Krzysztof)
- Use dev_err_probe for error returns (Krzysztof)
- Dropped redundant readl and writel wrappers (Krzysztof)
- Updated commit signed offs (Conor)
- Dropped redundant node in dts, module set to on in dtsi (Conor)

Jia Jie Ho (4):
  dt-bindings: crypto: Add StarFive crypto module
  crypto: starfive - Add crypto engine support
  riscv: dts: starfive: Add crypto and DMA node for VisionFive 2
  crypto: starfive - Add hash and HMAC support

 .../crypto/starfive,jh7110-crypto.yaml        |   70 ++
 MAINTAINERS                                   |    7 +
 arch/riscv/boot/dts/starfive/jh7110.dtsi      |   28 +
 drivers/crypto/Kconfig                        |    1 +
 drivers/crypto/Makefile                       |    1 +
 drivers/crypto/starfive/Kconfig               |   21 +
 drivers/crypto/starfive/Makefile              |    4 +
 drivers/crypto/starfive/jh7110-cryp.c         |  239 ++++
 drivers/crypto/starfive/jh7110-cryp.h         |  134 +++
 drivers/crypto/starfive/jh7110-hash.c         | 1041 +++++++++++++++++
 10 files changed, 1546 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/crypto/starfive,jh7110-crypto.yaml
 create mode 100644 drivers/crypto/starfive/Kconfig
 create mode 100644 drivers/crypto/starfive/Makefile
 create mode 100644 drivers/crypto/starfive/jh7110-cryp.c
 create mode 100644 drivers/crypto/starfive/jh7110-cryp.h
 create mode 100644 drivers/crypto/starfive/jh7110-hash.c
  

Comments

JiaJie Ho March 14, 2023, 3:24 a.m. UTC | #1
> On 13 Mar 2023 21:56:46 +0800 Jia Jie Ho <jiajie.ho@starfivetech.com>
> > +static int starfive_hash_xmit_dma(struct starfive_cryp_ctx *ctx) {

[...]
> > +	dmaengine_slave_config(cryp->tx, &cryp->cfg_in);
> > +
> > +	in_desc = dmaengine_prep_slave_sg(cryp->tx, rctx->in_sg,
> > +					  rctx->in_sg_len,
> DMA_MEM_TO_DEV,
> > +					  DMA_PREP_INTERRUPT  |
> DMA_CTRL_ACK);
> > +
> > +	if (!in_desc)
> > +		return -EINVAL;
> > +
> > +	reinit_completion(&cryp->tx_comp);
> 
> What breaks without reinit?

Hi Hillf, 
So far, I don't see errors without the reinit.
I kept the reinit to reset the done field in case there is a prior transfer.
I'll remove this if it is not needed.

> > +
> > +	in_desc->callback = starfive_hash_dma_callback;
> > +	in_desc->callback_param = cryp;
> > +
> > +	cookie = dmaengine_submit(in_desc);
> > +	dma_async_issue_pending(cryp->tx);
> > +
> > +	if (!wait_for_completion_timeout(&cryp->tx_comp,
> > +					 msecs_to_jiffies(10000))) {
> > +		dev_err(cryp->dev, "wait_for_completion_timeout error,
> cookie = %x\n",
> > +			dma_async_is_tx_complete(cryp->rx, cookie,
> > +						 NULL, NULL));
> 
> What prevents wakeup from coming in case of timeout?

Usually, timeout is caused by failure to complete all data transfer.
It might be due to errors in dma chan setup or in case the hardware hangs.

Thanks for taking time looking into the patch.

Best regards,
Jia Jie
  
JiaJie Ho March 24, 2023, 4:51 a.m. UTC | #2
> -----Original Message-----
> From: Jia Jie Ho <jiajie.ho@starfivetech.com>
> Sent: 13 March, 2023 9:57 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>; Emil Renner Berthing
> <kernel@esmil.dk>; Conor Dooley <conor.dooley@microchip.com>
> Cc: linux-crypto@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-riscv@lists.infradead.org
> Subject: [PATCH v3 0/4] crypto: starfive - Add drivers for crypto engine
> 
> This patch series adds kernel driver support for StarFive JH7110 crypto engine.
> The first patch adds Documentations for the device and Patch 2 adds device
> probe and DMA init for the module. Patch 3 adds crypto and DMA dts node
> for VisionFive 2 board. Patch 4 adds hash/hmac support to the module.
> 
> Patch 3 needs to be applied on top of:
> https://patchwork.kernel.org/project/linux-
> riscv/patch/20230221024645.127922-18-hal.feng@starfivetech.com/
> https://patchwork.kernel.org/project/linux-
> riscv/cover/20230120024445.244345-1-xingyu.wu@starfivetech.com/
> 
> Changes v2->v3:
> - Only implement digest and use fallback for other ops (Herbert)
> - Use interrupt instead of polling for hash complete (Herbert)
> - Remove manual data copy from out-of-bound memory location as it will
>   be handled by DMA API. (Christoph & Herbert)
> 
> Changes v1->v2:
> - Fixed yaml filename and format (Krzysztof)
> - Removed unnecessary property names in yaml (Krzysztof)
> - Moved of_device_id table close to usage (Krzysztof)
> - Use dev_err_probe for error returns (Krzysztof)
> - Dropped redundant readl and writel wrappers (Krzysztof)
> - Updated commit signed offs (Conor)
> - Dropped redundant node in dts, module set to on in dtsi (Conor)
> 
> Jia Jie Ho (4):
>   dt-bindings: crypto: Add StarFive crypto module
>   crypto: starfive - Add crypto engine support
>   riscv: dts: starfive: Add crypto and DMA node for VisionFive 2
>   crypto: starfive - Add hash and HMAC support
> 
>  .../crypto/starfive,jh7110-crypto.yaml        |   70 ++
>  MAINTAINERS                                   |    7 +
>  arch/riscv/boot/dts/starfive/jh7110.dtsi      |   28 +
>  drivers/crypto/Kconfig                        |    1 +
>  drivers/crypto/Makefile                       |    1 +
>  drivers/crypto/starfive/Kconfig               |   21 +
>  drivers/crypto/starfive/Makefile              |    4 +
>  drivers/crypto/starfive/jh7110-cryp.c         |  239 ++++
>  drivers/crypto/starfive/jh7110-cryp.h         |  134 +++
>  drivers/crypto/starfive/jh7110-hash.c         | 1041 +++++++++++++++++
>  10 files changed, 1546 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/crypto/starfive,jh7110-crypto.yaml
>  create mode 100644 drivers/crypto/starfive/Kconfig  create mode 100644
> drivers/crypto/starfive/Makefile  create mode 100644
> drivers/crypto/starfive/jh7110-cryp.c
>  create mode 100644 drivers/crypto/starfive/jh7110-cryp.h
>  create mode 100644 drivers/crypto/starfive/jh7110-hash.c
> 
> --
> 2.25.1


Hi Herbert/David,

Could you please help review this patch series?
Thanks in advance.

Best regards,
Jia Jie