Re: [PATCH v4 1/2] iio: imu: inv_icm42600: add WoM support

[Jean-Baptiste Maneyrol]

>________________________________________
>From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
>Sent: Friday, June 13, 2025 14:54
>To: Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@xxxxxxx>
>Cc: Jonathan Cameron <jic23@xxxxxxxxxx>; Lars-Peter Clausen <lars@xxxxxxxxxx>; David Lechner <dlechner@xxxxxxxxxxxx>; Nuno Sá <nuno.sa@xxxxxxxxxx>; Andy Shevchenko <andy@xxxxxxxxxx>; linux-iio@xxxxxxxxxxxxxxx <linux-iio@xxxxxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>
>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 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@xxxxxxxxx>
>> > >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.

By looking at all Linux drivers, there are plenty of them using *int*_t, even
inside iio:
adc/xilinx-xadc.h:	uint16_t threshold[16];
adc/xilinx-xadc.h:	uint16_t temp_hysteresis;
adc/xilinx-xadc.h:	uint16_t *data;
adc/xilinx-xadc.h:	int (*read)(struct xadc *xadc, unsigned int reg, uint16_t *val);
adc/xilinx-xadc.h:	int (*write)(struct xadc *xadc, unsigned int reg, uint16_t val);
adc/xilinx-xadc.h:	uint16_t *val)
adc/xilinx-xadc.h:	uint16_t val)
adc/xilinx-xadc.h:	uint16_t *val)
adc/xilinx-xadc.h:	uint16_t val)
adc/xilinx-xadc-events.c:	uint16_t cfg, old_cfg;
adc/xilinx-xadc-core.c:	uint16_t val)
adc/xilinx-xadc-core.c:	uint16_t *val)
adc/xilinx-xadc-core.c:	uint16_t *val)
adc/xilinx-xadc-core.c:	uint16_t val)
adc/xilinx-xadc-core.c:	uint16_t mask, uint16_t val)
adc/xilinx-xadc-core.c:	uint16_t tmp;
adc/xilinx-xadc-core.c:	uint16_t mask, uint16_t val)
adc/xilinx-xadc-core.c:	uint16_t val;
adc/xilinx-xadc-core.c:	uint16_t val16;
adc/xilinx-xadc-core.c:	uint16_t val16;
chemical/scd4x.c:static int scd4x_write(struct scd4x_state *state, enum scd4x_cmd cmd, uint16_t arg)
chemical/scd4x.c:				uint16_t arg, void *response, int response_sz)
chemical/scd4x.c:static int scd4x_read_meas(struct scd4x_state *state, uint16_t *meas)
chemical/scd4x.c:		uint16_t val;
chemical/scd4x.c:static int scd4x_read_poll(struct scd4x_state *state, uint16_t *buf)
chemical/scd4x.c:	uint16_t buf[3];
chemical/scd4x.c:	uint16_t value;
chemical/scd4x.c:	uint16_t val, arg;
chemical/scd4x.c:		uint16_t data[3];
dac/ad7303.c:	uint16_t config;
dac/ti-dac7612.c:	uint16_t cache[2];
dac/ad5766.c:	uint16_t val;
dac/ad5766.c:	uint16_t val;
dac/ad5766.c:	uint16_t val;
dac/ad5449.c:	uint16_t dac_cache[AD5449_MAX_CHANNELS];
dac/ad8460.c:	uint16_t sym;
gyro/adis16136.c:	uint16_t lot1, lot2, lot3, serial;
gyro/adis16136.c:	uint16_t flash_count;
gyro/adis16136.c:	uint16_t t;
gyro/adis16136.c:	uint16_t val16;
gyro/adis16136.c:	uint16_t prod_id;
humidity/ens210.c:	uint16_t part_id;
imu/adis16400.c:	uint16_t prod_id;
imu/adis16400.c:	uint16_t flash_count;
imu/adis16400.c:	uint16_t t;
imu/adis16400.c:	uint16_t t;
imu/adis16400.c:	uint16_t val16;
imu/adis16400.c:	uint16_t prod_id, smp_prd;
imu/adis16400.c:	int16_t val16;
imu/adis16460.c:	uint16_t t;
...


Then, why it is mandatory to change this driver to use uXX instead?

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

Thanks,
JB