Re: [PATCH v4 4/4] crypto: starfive - Add hash and HMAC support

From: Herbert Xu
Date: Mon Apr 10 2023 - 03:51:34 EST


On Mon, Apr 10, 2023 at 03:37:52PM +0800, Jia Jie Ho wrote:
>
> +static void starfive_hash_start(void *param)
> +{
> + struct starfive_cryp_ctx *ctx = param;
> + struct starfive_cryp_request_ctx *rctx = ctx->rctx;
> + struct starfive_cryp_dev *cryp = ctx->cryp;
> + union starfive_alg_cr alg_cr;
> + union starfive_hash_csr csr;
> +
> + dma_unmap_sg(cryp->dev, rctx->in_sg, rctx->in_sg_len, DMA_TO_DEVICE);
> +
> + alg_cr.v = 0;
> + alg_cr.clear = 1;
> +
> + writel(alg_cr.v, cryp->base + STARFIVE_ALG_CR_OFFSET);
> +
> + csr.v = readl(cryp->base + STARFIVE_HASH_SHACSR);
> + csr.firstb = 0;
> + csr.final = 1;
> +
> + reinit_completion(&cryp->hash_done);
> + writel(~STARFIVE_IE_MASK_HASH_DONE, cryp->base + STARFIVE_IE_MASK_OFFSET);
> + writel(csr.v, cryp->base + STARFIVE_HASH_SHACSR);
> +}

Why are you still using a completion? The callback function should
invoke the crypto_engine finalize_request call directly.

> +static int starfive_hash_xmit(struct starfive_cryp_ctx *ctx)
> +{
> + struct starfive_cryp_request_ctx *rctx = ctx->rctx;
> + struct starfive_cryp_dev *cryp = ctx->cryp;
> + int ret;
> +
> + rctx->csr.hash.v = 0;
> + rctx->csr.hash.reset = 1;
> + writel(rctx->csr.hash.v, cryp->base + STARFIVE_HASH_SHACSR);
> +
> + if (starfive_hash_wait_busy(ctx))
> + return dev_err_probe(cryp->dev, -ETIMEDOUT, "Error resetting engine.\n");
> +
> + rctx->csr.hash.v = 0;
> + rctx->csr.hash.mode = ctx->hash_mode & STARFIVE_HASH_MODE_MASK;
> + rctx->csr.hash.ie = 1;
> +
> + if (ctx->hash_mode & STARFIVE_HASH_HMAC_FLAGS) {
> + ret = starfive_hash_hmac_key(ctx);
> + if (ret)
> + return ret;
> + } else {
> + rctx->csr.hash.start = 1;
> + rctx->csr.hash.firstb = 1;
> + writel(rctx->csr.hash.v, cryp->base + STARFIVE_HASH_SHACSR);
> + }
> +
> + ret = starfive_hash_xmit_dma(ctx);
> + if (ret)
> + return ret;
> +
> + if (!wait_for_completion_timeout(&cryp->hash_done, msecs_to_jiffies(10000)))
> + return dev_err_probe(cryp->dev, -ETIMEDOUT, "Timeout waiting for hash done\n");

There is no point in waiting for completion. Just return 0 and
you're done.

Cheers,
--
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt