On Fri, Feb 9, 2024 at 6:28 AM Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > On Thu, Feb 08, 2024 at 10:02:38PM -0800, Abhishek Pandit-Subedi wrote: > > Between UCSI 1.2 and UCSI 2.0, the size of the MESSAGE_IN region was > > increased from 16 to 256. In order to avoid overflowing reads for older > > systems, add a mechanism to use the read UCSI version to truncate read > > sizes on UCSI v1.2. > > ... > > > + if (ucsi->version <= UCSI_VERSION_1_2) > > + buf_size = min_t(size_t, 16, buf_size); > > Please, avoid using min_t(). Here the clamp() can be used. I think this is likely the 4th time I've been tripped up by an undocumented practice in this patch series. <linux/minmax.h> says nothing about avoiding min_t -- why prefer clamp()? Please add the recommendation here (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/minmax.h#n10) and I am more than happy to change it after. > Shouldn't magic number be defined? The comment right above this line documents the number. As this is the only use right now, I don't see a need to make it a macro/constant yet. > > -- > With Best Regards, > Andy Shevchenko > > Cheers, Abhishek