Message-ID: <CAHp75VeS8XQbcTaDFLUYTcd4SEdfoVOd4-mht6NGk__exSD0Vg@mail.gmail.com>
Date: Sat, 14 Jun 2025 00:02:52 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@....com>
Cc: Andy Shevchenko <andriy.shevchenko@...el.com>, 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

On Fri, Jun 13, 2025 at 8:14 PM Jean-Baptiste Maneyrol
<Jean-Baptiste.Maneyrol@....com> wrote:
 >From: Andy Shevchenko <andriy.shevchenko@...el.com>
> >Sent: Friday, June 13, 2025 17:01
> >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!
> >You 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.

Let's wait for others to speak up. Especially maintainers.

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

Same series, just a prerequisite patch. Note, I have one locally, I
just need to send it and you can use it, but the reason why I haven't
sent is the same — I want to know the official position of the IIO
subsystem about this.

-- 
With Best Regards,
Andy Shevchenko

Powered by blists - more mailing lists