Hi Christophe,
On Fri, 9 May 2025 09:48:44 +0200
Christophe Leroy <christophe.le...@csgroup.eu> wrote:
> When no post-completion processing is expected, don't waste time
> handling useless interrupts.
>
> Only set QMC_BD_[R/T]X_I and QMC_BD_[R/T]X_UB when a completion
> function is passed in.
>
> Signed-off-by: Christophe Leroy <christophe.le...@csgroup.eu>
> ---
> drivers/soc/fsl/qe/qmc.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
> index 36c0ccc06151..0a704fd0b1a0 100644
> --- a/drivers/soc/fsl/qe/qmc.c
> +++ b/drivers/soc/fsl/qe/qmc.c
> @@ -474,7 +474,9 @@ int qmc_chan_write_submit(struct qmc_chan *chan,
> dma_addr_t addr, size_t length,
> xfer_desc->context = context;
>
> /* Activate the descriptor */
> - ctrl |= (QMC_BD_TX_R | QMC_BD_TX_UB);
> + ctrl |= QMC_BD_TX_R;
> + if (complete)
> + ctrl |= QMC_BD_TX_I | QMC_BD_TX_UB;
Be careful, you don't set the UB bit for all descriptor anymore.
Your goal, is to have interrupts only on some descriptors (those where I
bit is set).
This can lead to issue in the function handling the interrupt.
This function, qmc_chan_write_done(), do the processing according to the
following:
/*
* R bit UB bit
* 0 0 : The BD is free
* 1 1 : The BD is in used, waiting for transfer
* 0 1 : The BD is in used, waiting for completion
* 1 0 : Should not append
*/
https://elixir.bootlin.com/linux/v6.15-rc5/source/drivers/soc/fsl/qe/qmc.c#L507
It considers R=0 / UB=0 as a free BD and R=1 / UB=0 as a case that should
not happen.
Both cases are no more correct with your modification.
Have the feeling that UB bit still has to be set even if I bit is not set
in order to have qmc_chan_write_done() looking at all descriptors.
Suppose:
desc 0, no interrupt
desc 1, no interrupt
desc 2, interrupt
When the interrupt for desc 2 is handled, desc 0 and desc 1 are seen with
R=0 and UB=0. As desc 0 is considered as free by qmc_chan_write_done(), it
will never look at desc 2.
> wmb(); /* Be sure to flush the descriptor before control update */
> qmc_write16(&bd->cbd_sc, ctrl);
>
> @@ -586,7 +588,9 @@ int qmc_chan_read_submit(struct qmc_chan *chan,
> dma_addr_t addr, size_t length,
> QMC_BD_RX_AB | QMC_BD_RX_CR);
>
> /* Activate the descriptor */
> - ctrl |= (QMC_BD_RX_E | QMC_BD_RX_UB);
> + ctrl |= QMC_BD_RX_E;
> + if (complete)
> + ctrl |= QMC_BD_RX_I | QMC_BD_RX_UB;
Exact same comment.
Best regards,
Hervé
