From:   JiaJie Ho <jiajie.ho@starfivetech.com>
To:     Hillf Danton <hdanton@sina.com>
CC:     Herbert Xu <herbert@gondor.apana.org.au>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Emil Renner Berthing <kernel@esmil.dk>,
        Conor Dooley <conor.dooley@microchip.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-riscv@lists.infradead.org" <linux-riscv@lists.infradead.org>
Subject: RE: [PATCH v3 4/4] crypto: starfive - Add hash and HMAC support
Thread-Topic: [PATCH v3 4/4] crypto: starfive - Add hash and HMAC support
Thread-Index: AQHZVbQZd9+lAcQFW0yunvw0rKV1eK7440YAgAC0knA=
Date:   Tue, 14 Mar 2023 03:24:39 +0000
Message-ID: <a00427747c094f16acc78b536508bd16@EXMBX068.cuchost.com>
References: <20230313135646.2077707-1-jiajie.ho@starfivetech.com>
 <20230314001744.1911-1-hdanton@sina.com>
In-Reply-To: <20230314001744.1911-1-hdanton@sina.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Precedence: bulk

> 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