Re: [PATCH 1/3] spi: imx: Fix precedence bug in spi_imx_dma_max_wml_find()

[Frank Li <Frank.li@nxp.com>]

On Fri, May 01, 2026 at 01:59:49PM +0000, John Madieu wrote:
> The watermark search in spi_imx_dma_max_wml_find() reads:
>
> 	if (!dma_data->dma_len % (i * bytes_per_word))
> 		break;
>
> The unary ! binds tighter than %, so this parses as:
>
> 	if ((!dma_data->dma_len) % (i * bytes_per_word))
> 		break;
>
> !dma_data->dma_len is 0 or 1, and `0 % x == 0` for any x; `1 % x` is
> 0 unless x == 1. The condition is therefore false in every case
> except dma_len != 0 with i * bytes_per_word == 1, i.e. i == 1 and
> bytes_per_word == 1.
>
> The loop almost always falls through to its end, leaving i == 0,
> which the post-loop fallback rewrites to 1:
>
> 	if (i == 0)
> 		i = 1;
>
> So spi_imx->wml ends up at 1 for essentially every DMA transfer,
> defeating the entire purpose of the function. The DMA engine then
> requests service after every single FIFO word instead of using
> multi-word bursts, hurting throughput on every DMA-capable variant.
>
> Add the missing parentheses so the modulo is computed first, then
> negated:
>
> 	if (!(dma_data->dma_len % (i * bytes_per_word)))
> 		break;
>
> Fixes: faa8e404ad8e ("spi: imx: support dynamic burst length for ECSPI DMA mode")
> Signed-off-by: John Madieu <john.madieu@gmail.com>
> ---

Thank you for find this.

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/spi/spi-imx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> index e5c907c45b87..7ae8078c10ef 100644
> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -1836,7 +1836,7 @@ static void spi_imx_dma_max_wml_find(struct spi_imx_data *spi_imx,
>  	unsigned int i;
>
>  	for (i = spi_imx->devtype_data->fifo_size / 2; i > 0; i--) {
> -		if (!dma_data->dma_len % (i * bytes_per_word))
> +		if (!(dma_data->dma_len % (i * bytes_per_word)))
>  			break;
>  	}
>  	/* Use 1 as wml in case no available burst length got */
> --
> 2.25.1
>