Re: [PATCH] spi: spi-fsl-lpspi: fix watermark truncation caused by type cast

[david laight <david.laight@runbox.com>]

On Fri, 21 Nov 2025 11:19:34 -0500
Frank Li <Frank.li@nxp.com> wrote:

> On Mon, Nov 17, 2025 at 11:03:55AM +0800, carlos.song@nxp.com wrote:
> > From: Carlos Song <carlos.song@nxp.com>
> >
> > 't->len' is an unsigned integer, while 'watermark' and 'txfifosize' are
> > u8. Using min_t with typeof(watermark) forces both values to be cast to
> > u8, which truncates len when it exceeds 255. For example, len = 4096
> > becomes 0 after casting, resulting in an incorrect watermark value.
> >
> > Use a wider type in min_t to avoid truncation and ensure the correct
> > minimum value is applied.
> >
> > Fixes: a750050349ea ("spi: spi-fsl-lpspi: use min_t() to improve code")
> > Signed-off-by: Carlos Song <carlos.song@nxp.com>
> > ---
> >  drivers/spi/spi-fsl-lpspi.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c
> > index 8da66e101386..065456aba2ae 100644
> > --- a/drivers/spi/spi-fsl-lpspi.c
> > +++ b/drivers/spi/spi-fsl-lpspi.c
> > @@ -486,7 +486,13 @@ static int fsl_lpspi_setup_transfer(struct spi_controller *controller,
> >  		fsl_lpspi->tx = fsl_lpspi_buf_tx_u32;
> >  	}
> >
> > -	fsl_lpspi->watermark = min_t(typeof(fsl_lpspi->watermark),
> > +	/*
> > +	 * t->len is 'unsigned' and txfifosize and watermrk is 'u8', force
> > +	 * type cast is inevitable. When len > 255, len will be truncated in min_t(),
> > +	 * it caused wrong watermark set. 'unsigned int' is as the designated type
> > +	 * for min_t() to avoid truncation.
> > +	 */
> > +	fsl_lpspi->watermark = min_t(unsigned int,
> >  				     fsl_lpspi->txfifosize,
> >  				     t->len);  
> 
> There are thread discussion about min() and min_t()
> 
> https://lore.kernel.org/all/20251119224140.8616-1-david.laight.linux@gmail.com/

The big comment even carefully explains that the two types are unsigned ones.
So a simple min() is absolutely fine (and the comment can go away).

The old typecheck in min was just so stupid.
In this case the 'u8' variable is promoted to 'int' (they always are)
and then converted to 'unsigned int' to match the other type.
Even though there is an implicit 'int' => 'unsigned int' cast it is
impossible for a negative value to become a large positive on
(which is the only justification for the type check).

I'd check the file for other uses on min_t() as well.

	David


> 
> Frank
> 
> >
> > --
> > 2.34.1
> >  
>