RE: [PATCH v2 4/5] crypto: starfive: Add sm3 support for JH8100

[JiaJie Ho]

> > Add driver support for SM3 hash/HMAC for JH8100 SoC. JH8100 contains a
> > separate SM algo engine and new dedicated dma that supports 64-bit
> > address access.
> >
> > Signed-off-by: Jia Jie Ho <jiajie.ho@xxxxxxxxxxxxxxxx>
> > ---
> >  drivers/crypto/starfive/Kconfig       |  25 +-
> >  drivers/crypto/starfive/Makefile      |   3 +
> >  drivers/crypto/starfive/jh7110-cryp.c |  48 ++-
> > drivers/crypto/starfive/jh7110-cryp.h |  59 +++
> > drivers/crypto/starfive/jh7110-hash.c |  20 +-
> > drivers/crypto/starfive/jh8100-sm3.c  | 532
> ++++++++++++++++++++++++++
> >  6 files changed, 677 insertions(+), 10 deletions(-)  create mode
> > 100644 drivers/crypto/starfive/jh8100-sm3.c
> >
> > diff --git a/drivers/crypto/starfive/Kconfig
> > b/drivers/crypto/starfive/Kconfig index 0fe389e9f932..e6bf02d0ed1f
> > 100644
> > --- a/drivers/crypto/starfive/Kconfig
> > +++ b/drivers/crypto/starfive/Kconfig
> > @@ -5,7 +5,7 @@
> >  config CRYPTO_DEV_JH7110
> >  	tristate "StarFive JH7110 cryptographic engine driver"
> >  	depends on (SOC_STARFIVE && AMBA_PL08X) || COMPILE_TEST
> > -	depends on HAS_DMA
> > +	depends on HAS_DMA && !CRYPTO_DEV_JH8100
> >  	select CRYPTO_ENGINE
> >  	select CRYPTO_HMAC
> >  	select CRYPTO_SHA256
> > @@ -24,3 +24,26 @@ config CRYPTO_DEV_JH7110
> >  	  skciphers, AEAD and hash functions.
> >
> >  	  If you choose 'M' here, this module will be called jh7110-crypto.
> > +
> > +config CRYPTO_DEV_JH8100
> > +	tristate "StarFive JH8100 cryptographic engine drivers"
> > +	depends on (SOC_STARFIVE && DW_AXI_DMAC) || COMPILE_TEST
> > +	depends on HAS_DMA
> > +	select CRYPTO_ENGINE
> > +	select CRYPTO_HMAC
> > +	select CRYPTO_SHA256
> > +	select CRYPTO_SHA512
> > +	select CRYPTO_SM3_GENERIC
> > +	select CRYPTO_RSA
> > +	select CRYPTO_AES
> > +	select CRYPTO_CCM
> > +	select CRYPTO_GCM
> > +	select CRYPTO_CBC
> > +	select CRYPTO_ECB
> > +	select CRYPTO_CTR
> > +	help
> > +	  Support for StarFive JH8100 crypto hardware acceleration engine.
> > +	  This module provides additional support for SM2 signature
> verification,
> > +	  SM3 hash/hmac functions and SM4 skcipher.
> > +
> > +	  If you choose 'M' here, this module will be called jh8100-crypto.
> > diff --git a/drivers/crypto/starfive/Makefile
> > b/drivers/crypto/starfive/Makefile
> > index 8c137afe58ad..67717fca3f5d 100644
> > --- a/drivers/crypto/starfive/Makefile
> > +++ b/drivers/crypto/starfive/Makefile
> > @@ -2,3 +2,6 @@
> >
> >  obj-$(CONFIG_CRYPTO_DEV_JH7110) += jh7110-crypto.o
> > jh7110-crypto-objs := jh7110-cryp.o jh7110-hash.o jh7110-rsa.o
> > jh7110-aes.o
> > +
> > +obj-$(CONFIG_CRYPTO_DEV_JH8100) += jh8100-crypto.o jh8100-crypto-
> objs
> > +:= jh7110-cryp.o jh7110-hash.o jh7110-rsa.o jh7110-aes.o jh8100-sm3.o
> > diff --git a/drivers/crypto/starfive/jh7110-cryp.c
> > b/drivers/crypto/starfive/jh7110-cryp.c
> > index fe33e87f25ab..fb7c19705fbf 100644
> > --- a/drivers/crypto/starfive/jh7110-cryp.c
> > +++ b/drivers/crypto/starfive/jh7110-cryp.c
> > @@ -106,6 +106,26 @@ static irqreturn_t starfive_cryp_irq(int irq, void
> *priv)
> >  	return IRQ_HANDLED;
> >  }
> >
> > +#ifdef CONFIG_CRYPTO_DEV_JH8100
> > +static irqreturn_t starfive_cryp_irq1(int irq, void *priv) {
> > +	u32 status;
> > +	u32 mask;
> > +	struct starfive_cryp_dev *cryp = (struct starfive_cryp_dev *)priv;
> > +
> > +	mask = readl(cryp->base + STARFIVE_SM_IE_MASK_OFFSET);
> > +	status = readl(cryp->base + STARFIVE_SM_IE_FLAG_OFFSET);
> > +
> > +	if (status & STARFIVE_SM_IE_FLAG_SM3_DONE) {
> > +		mask |= STARFIVE_SM_IE_MASK_SM3_DONE;
> > +		writel(mask, cryp->base + STARFIVE_SM_IE_MASK_OFFSET);
> > +		tasklet_schedule(&cryp->sm3_done);
> > +	}
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +#endif
> > +
> >  static int starfive_cryp_probe(struct platform_device *pdev)  {
> >  	struct starfive_cryp_dev *cryp;
> > @@ -156,6 +176,16 @@ static int starfive_cryp_probe(struct
> platform_device *pdev)
> >  		return dev_err_probe(&pdev->dev, ret,
> >  				     "Failed to register interrupt handler\n");
> >
> > +#ifdef CONFIG_CRYPTO_DEV_JH8100
> > +	tasklet_init(&cryp->sm3_done, starfive_sm3_done_task, (unsigned
> > +long)cryp);
> > +
> > +	irq = platform_get_irq(pdev, 1);
> > +	if (irq < 0)
> > +		return irq;
> > +
> > +	ret = devm_request_irq(&pdev->dev, irq, starfive_cryp_irq1, 0,
> > +			       pdev->name, (void *)cryp);
> > +#endif
> >  	clk_prepare_enable(cryp->hclk);
> >  	clk_prepare_enable(cryp->ahb);
> >  	reset_control_deassert(cryp->rst);
> > @@ -191,8 +221,17 @@ static int starfive_cryp_probe(struct
> platform_device *pdev)
> >  	if (ret)
> >  		goto err_algs_rsa;
> >
> > +#ifdef CONFIG_CRYPTO_DEV_JH8100
> > +	ret = starfive_sm3_register_algs();
> > +	if (ret)
> > +		goto err_algs_sm3;
> > +#endif
> >  	return 0;
> >
> > +#ifdef CONFIG_CRYPTO_DEV_JH8100
> > +err_algs_sm3:
> > +	starfive_rsa_unregister_algs();
> > +#endif
> >  err_algs_rsa:
> >  	starfive_hash_unregister_algs();
> >  err_algs_hash:
> > @@ -213,7 +252,9 @@ static int starfive_cryp_probe(struct
> platform_device *pdev)
> >  	reset_control_assert(cryp->rst);
> >
> >  	tasklet_kill(&cryp->hash_done);
> > -
> > +#ifdef CONFIG_CRYPTO_DEV_JH8100
> > +	tasklet_kill(&cryp->sm3_done);
> > +#endif
> 
> Please use if instead if ifdef where possible:
> 
> 	if (IS_ENABLED(CONFIG_CRYPTO_DEV_JH8100))
> 		tasklet_kill(&cryp->sm3_done);
> 

Hi Herbert,
Thanks for reviewing this.
I am using ifdef so unused codes would not be compiled into the drivers for unsupported device. 
Is the final driver size a concern in this case?

Best regards,
Jia Jie