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

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

[carlos.song]

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);
 
-- 
2.34.1

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

[Carlos Song]

Gently ping to avoid missing...

> -----Original Message-----
> From: carlos.song@nxp.com <carlos.song@nxp.com>
> Sent: Monday, November 17, 2025 11:04 AM
> To: Frank Li <frank.li@nxp.com>; broonie@kernel.org; rongqianfeng@vivo.com
> Cc: linux-spi@vger.kernel.org; imx@lists.linux.dev; linux-kernel@vger.kernel.org
> Subject: [EXT] [PATCH] spi: spi-fsl-lpspi: fix watermark truncation caused by type
> cast
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report this
> email' button
> 
> 
> 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);
> 
> --
> 2.34.1
> 

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 866 bytes --]

On Fri, Nov 21, 2025 at 08:18:17AM +0000, Carlos Song wrote:
> 
> Gently ping to avoid missing...

Please don't send content free pings and please allow a reasonable time
for review.  People get busy, go on holiday, attend conferences and so 
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review.  If there have been
review comments then people may be waiting for those to be addressed.

Sending content free pings adds to the mail volume (if they are seen at
all) which is often the problem and since they can't be reviewed
directly if something has gone wrong you'll have to resend the patches
anyway, so sending again is generally a better approach though there are
some other maintainers who like them - if in doubt look at how patches
for the subsystem are normally handled.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Daniel Baluta]

On Mon, Nov 17, 2025 at 5:06 AM <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>

LGTM,

Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>

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

[Frank Li]

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>
> ---

Reviewed-by: Frank Li <Frank.Li@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);
>
> --
> 2.34.1
>

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

[Frank Li]

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 mit_t()

https://lore.kernel.org/all/20251119224140.8616-1-david.laight.linux@gmail.com/

Frank

>
> --
> 2.34.1
>

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

[david laight]

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
> >  
> 

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

[Andy Shevchenko]

On Sat, Nov 22, 2025 at 10:57:16AM +0000, david laight wrote:
> 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:

...

> > > +	/*
> > > +	 * 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.

Just came to this thread to echoing what David said. +1 to the above, please
convert to simple min(). The use cases for min_t() and max_t() should be rare
really.

-- 
With Best Regards,
Andy Shevchenko

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

[Carlos Song]

> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@intel.com>
> Sent: Tuesday, November 25, 2025 11:11 PM
> To: david laight <david.laight@runbox.com>
> Cc: Frank Li <frank.li@nxp.com>; Carlos Song <carlos.song@nxp.com>;
> broonie@kernel.org; rongqianfeng@vivo.com; linux-spi@vger.kernel.org;
> imx@lists.linux.dev; linux-kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH] spi: spi-fsl-lpspi: fix watermark truncation caused by
> type cast
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report this
> email' button
> 
> 
> On Sat, Nov 22, 2025 at 10:57:16AM +0000, david laight wrote:
> > 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:
> 
> ...
> 
> > > > + /*
> > > > +  * 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo
> > > re.kernel.org%2Fall%2F20251119224140.8616-1-david.laight.linux%40gma
> > >
> il.com%2F&data=05%7C02%7Ccarlos.song%40nxp.com%7C24c955c5ab414a26
> 730
> > >
> a08de2c34dda3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6389
> 96802
> > >
> 735067934%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiO
> iIwLj
> > >
> AuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%
> 7C
> > > %7C&sdata=NEBirtRBR5RXz9cUXuqWVtHT6b3memVhsRe5mA7AIAQ%3D&
> reserved=0
> >
> > 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.
> 
> Just came to this thread to echoing what David said. +1 to the above, please
> convert to simple min(). The use cases for min_t() and max_t() should be rare
> really.
> 

Hi,

Thank you for all ack about this patch.
From my points, min() ot min_t(unsigned, x, x) both are ok.
This patch has been picked, do I need to do a new patch to use min() instead this patch?

Carlos
> --
> With Best Regards,
> Andy Shevchenko
> 

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

[Andy Shevchenko]

On Wed, Nov 26, 2025 at 02:10:25AM +0000, Carlos Song wrote:
> > -----Original Message-----
> > From: Andy Shevchenko <andriy.shevchenko@intel.com>
> > Sent: Tuesday, November 25, 2025 11:11 PM
> > On Sat, Nov 22, 2025 at 10:57:16AM +0000, david laight wrote:
> > > 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:

...

> > > > > + /*
> > > > > +  * 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo
> > > > re.kernel.org%2Fall%2F20251119224140.8616-1-david.laight.linux%40gma
> > > >
> > il.com%2F&data=05%7C02%7Ccarlos.song%40nxp.com%7C24c955c5ab414a26
> > 730
> > > >
> > a08de2c34dda3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6389
> > 96802
> > > >
> > 735067934%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiO
> > iIwLj
> > > >
> > AuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%
> > 7C
> > > > %7C&sdata=NEBirtRBR5RXz9cUXuqWVtHT6b3memVhsRe5mA7AIAQ%3D&
> > reserved=0
> > >
> > > 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.
> > 
> > Just came to this thread to echoing what David said. +1 to the above, please
> > convert to simple min(). The use cases for min_t() and max_t() should be rare
> > really.
> 
> Thank you for all ack about this patch.
> From my points, min() ot min_t(unsigned, x, x) both are ok.
> This patch has been picked, do I need to do a new patch to use min() instead this patch?

It's already applied, so send a followup that moves from min_t() to min().

-- 
With Best Regards,
Andy Shevchenko

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

[Carlos Song]

> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@intel.com>
> Sent: Wednesday, November 26, 2025 2:43 PM
> To: Carlos Song <carlos.song@nxp.com>
> Cc: david laight <david.laight@runbox.com>; Frank Li <frank.li@nxp.com>;
> broonie@kernel.org; rongqianfeng@vivo.com; linux-spi@vger.kernel.org;
> imx@lists.linux.dev; linux-kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH] spi: spi-fsl-lpspi: fix watermark truncation caused by
> type cast
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report this
> email' button
> 
> 
> On Wed, Nov 26, 2025 at 02:10:25AM +0000, Carlos Song wrote:
> > > -----Original Message-----
> > > From: Andy Shevchenko <andriy.shevchenko@intel.com>
> > > Sent: Tuesday, November 25, 2025 11:11 PM On Sat, Nov 22, 2025 at
> > > 10:57:16AM +0000, david laight wrote:
> > > > 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:
> 
> ...
> 
> > > > > > + /*
> > > > > > +  * 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%
> > > > >
> 2Flo%2F&data=05%7C02%7Ccarlos.song%40nxp.com%7Ce0e6b6d0ae23480ed
> > > > >
> 8c008de2cb70f79%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638
> > > > >
> 997361915264343%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydW
> Us
> > > > > IlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3
> > > > >
> D%3D%7C0%7C%7C%7C&sdata=%2B4ARYhKtpMih2eUExScbLgjTTbm2%2FmkR
> tF%2
> > > > > FesH0M5ZQ%3D&reserved=0
> > > > > re.kernel.org%2Fall%2F20251119224140.8616-1-david.laight.linux%4
> > > > > 0gma
> > > > >
> > >
> il.com%2F&data=05%7C02%7Ccarlos.song%40nxp.com%7C24c955c5ab414a26
> > > 730
> > > > >
> > >
> a08de2c34dda3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6389
> > > 96802
> > > > >
> > >
> 735067934%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiO
> > > iIwLj
> > > > >
> > >
> AuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%
> > > 7C
> > > > > %7C&sdata=NEBirtRBR5RXz9cUXuqWVtHT6b3memVhsRe5mA7AIAQ%3
> D&
> > > reserved=0
> > > >
> > > > 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.
> > >
> > > Just came to this thread to echoing what David said. +1 to the
> > > above, please convert to simple min(). The use cases for min_t() and
> > > max_t() should be rare really.
> >
> > Thank you for all ack about this patch.
> > From my points, min() ot min_t(unsigned, x, x) both are ok.
> > This patch has been picked, do I need to do a new patch to use min() instead
> this patch?
> 
> It's already applied, so send a followup that moves from min_t() to min().
> 
Will do this then. Thank you.
> --
> With Best Regards,
> Andy Shevchenko
> 

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

[Mark Brown]

On Mon, 17 Nov 2025 11:03:55 +0800, carlos.song@nxp.com wrote:
> '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.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: spi-fsl-lpspi: fix watermark truncation caused by type cast
      commit: 9f0c21bac5a8089e74b21d007e26fb4594b10d73

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

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

[david laight]

On Mon, 17 Nov 2025 11:03:55 +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.

Also see my big patch that will detect this...

It usual that a simple min() will work fine.
(After the relaxation of the type comparison in min().)
If one of the values is a signed type (but positive) use umin(a,b).

This is a typical example of the type of the destination being used in min_t().
The real fix is to remove min_t() completely.

After all if the code had been:
	sl_lpspi->watermark = min(sl_lpspi->txfifosize,
		(typeof(fsl_lpspi->watermark))t->len);
a reviewer would have been likely to pick up the truncation.

Hiding the cast inside min_t() is a 'really bad idea' (tm).

I've found a pile of cases of:
	min_t(u8, value_32 / 2, 255)
in one driver (and the variable is pretty much called 'value_32'.

Looks like I've tried to compile this file yet.
'git diff' is currently outputting 8395 files for 358 files.

	David

> 
> 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);
>