Message-ID:
 <FR3P281MB175722266C119B719FCE9E6CCE77A@FR3P281MB1757.DEUP281.PROD.OUTLOOK.COM>
Date: Fri, 13 Jun 2025 17:14:10 +0000
From: Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@....com>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
CC: Jonathan Cameron <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
        David Lechner <dlechner@...libre.com>,
        Nuno Sá
	<nuno.sa@...log.com>,
        Andy Shevchenko <andy@...nel.org>,
        "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 1/2] iio: imu: inv_icm42600: add WoM support

>
>
>________________________________________
>From: Andy Shevchenko <andriy.shevchenko@...el.com>
>Sent: Friday, June 13, 2025 17:01
>To: Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@....com>
>Cc: Jonathan Cameron <jic23@...nel.org>; Lars-Peter Clausen <lars@...afoo.de>; David Lechner <dlechner@...libre.com>; Nuno Sá <nuno.sa@...log.com>; Andy Shevchenko <andy@...nel.org>; linux-iio@...r.kernel.org <linux-iio@...r.kernel.org>; linux-kernel@...r.kernel.org <linux-kernel@...r.kernel.org>
>Subject: Re: [PATCH v4 1/2] iio: imu: inv_icm42600: add WoM support
> 
>This Message Is From an External Sender
>This message came from outside your organization.
> 
>On Fri, Jun 13, 2025 at 05:41:47PM +0300, Andy Shevchenko wrote:
>> On Fri, Jun 13, 2025 at 01:43:58PM +0000, Jean-Baptiste Maneyrol wrote:
>> > >From: Andy Shevchenko <andriy.shevchenko@...el.com>
>> > >Sent: Friday, June 13, 2025 14:54
>> > >On Fri, Jun 13, 2025 at 03:53:36PM +0300, Andy Shevchenko wrote:
>> > >> On Fri, Jun 13, 2025 at 12:46:46PM +0000, Jean-Baptiste Maneyrol wrote:
>> > >> > >From: Andy Shevchenko <andriy.shevchenko@...el.com>
>> > >> > >Sent: Friday, June 13, 2025 10:29
>> > >> > >On Fri, Jun 13, 2025 at 09:34:26AM +0200, Jean-Baptiste Maneyrol via B4 Relay wrote:
>
>...
>
>> > >> > >Overall, looking to this patch again, I think it would be better to prepend it
>> > >> > >by replacing *int*_t types by the respective uXX ones. Because in this patch
>> > >> > >we add dozens of new ones which increases an unneeded churn in the future.
>> > >> > >
>> > >> > In my opinion, to respect the rule don't mix *int*_t and uXX types, it is better
>> > >> > to keep *int*_t types. If it need to be changed, we can change afterward the
>> > >> > whole driver types with a replace tool and send it in a separate patch.
>> > >> 
>> > >> It will be never ending story, sorry. We need someone to solve this tech debt.
>> > >> And since this patch adds more than 3 new users of it, I think it's a candidate
>> > >> to embrace the burden.
>> > >
>> > >For your convenience I can mock-up a change...
>> > 
>> > It looks like there's something I don't understand in the kernel Documentation about
>> > types then.
>> > Quoting Documentation/process/coding-style.rst, section 5.d:
>> > ---
>> > New types which are identical to standard C99 types, in certain exceptional circumstances.
>> > 
>> > Although it would only take a short amount of time for the eyes and brain to become accustomed
>> > to the standard types like uint32_t, some people object to their use anyway.
>> > 
>> > Therefore, the Linux-specific u8/u16/u32/u64 types and their signed equivalents which are
>> > identical to standard types are permitted -- although they are not mandatory in new code
>> > of your own.
>> > 
>> > When editing existing code which already uses one or the other set of types, you should
>> > conform to the existing choices in that code.
>> > ---
>> > 
>> > My understanding is that uXX are not mandatory for new code. You can use types like *int*_t.
>> > But you need to conform afterward to the existing choice. That's why this driver was
>> > done initially with *int*_t types, and that patches are conforming to this choice.
>> 
>> This part of the documentation has a lot of room for different interpretations.
>> One [1] may consider this as uXX superior, another, like you, that it's okay
>> to use.  In any case Greg KH prefers uXX over uintXX_t. And he is also in
>> the chain of maintainers here. Feel free to amend the Documentation. But
>> be sure all stakeholders will see your proposal (like Greg KH and other
>> key maintainers).
>> 
>> > By looking at all Linux drivers, there are plenty of them using *int*_t, even
>> > inside iio:
>> 
>> $ git grep -l 'u\?int[0-9][0-9]\?_t' -- drivers/iio/ | wc -l
>> 59
>> 
>> $ git ls-files drivers/iio*.c | wc -l
>> 640
>> 
>> Less than 10%.
>> 
>> > Then, why it is mandatory to change this driver to use uXX instead?
>> 
>> TO be consistent. With the above wording in the documentation I may argue that
>> entire subsystem should be consistent and at least in IIO we have tons of patch
>> series that are against the whole subsystem to do one style change or another
>> (look at the recent memset() vs. {} for initialisation).
>> 
>> [1] https://urldefense.com/v3/__https://lore.kernel.org/all/20250409180953.398686-1-matchstick@neverthere.org/__;!!FtrhtPsWDhZ6tw!DVTvkgDsymM7132dB-wjei-s0JxYiivZxtzEHfWjsrn_6toqTXA__hm2nPUh7jmectCXcP9Z3OAh0hMm-WD6eQAHOtdiGbYQqsw$[lore[.]kernel[.]org]
>
>Oh, this [2] is golden!
>YUou may found support for your arguments and for mine in that thread, but the
>bottom line is: what do maintainers of IIO prefer? (Taking into account that it
>goes via Greg KH)
>
>
>[2]: https://urldefense.com/v3/__https://lore.kernel.org/all/20210423230609.13519-1-alx.manpages@gmail.com/__;!!FtrhtPsWDhZ6tw!DVTvkgDsymM7132dB-wjei-s0JxYiivZxtzEHfWjsrn_6toqTXA__hm2nPUh7jmectCXcP9Z3OAh0hMm-WD6eQAHOtdiuFc54eI$[lore[.]kernel[.]org]
>

If this is required, I can do it. I would just want to know if this is mandatory
since we already have a couple of drivers merged using standard types and
other drivers planned to be merged.

Can I do it in the same series or should it be in a separate patch before this
series?


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

Thanks,
JB

Powered by blists - more mailing lists