Messages in this thread |  | | From | Carlos Song <> | | Subject | Re: [PATCH] spi: spi-fsl-lpspi: fix watermark truncation caused by type cast | | Date | Wed, 26 Nov 2025 10:02:23 +0000 |
| |
> -----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 >
|  |