IIO: Tidying up timestamp alignment markings.
Subsystems (30)
Jonathan Cameron
maintainercover IIO: Tidying up timestamp alignment markings.
From: Jonathan Cameron <[email protected]>
I haven't +CC everyone relevant as that would be too long a list and these are mostly entirely mechanical changes that just need someone to sanity check I didn't do anything stupid.
This combines several related types of change:
- Some 'fixes'. These aren't actual bugs but more places where the code relies on factors it shouldn't such as what architecture the driver is running on, or padding of the wrong element happening to always make enough room.
- Now we have aligned_s64, use that to replace all the remaining s64 timestamp __aligned(8) instances in IIO.
This is both cleaner and hopefully less prone to confusing static analysis tools.
- Change the timestamp type in iio_push_to_buffers_with_timestamp() to s64. This is an entirely in kernel interface. It's data is pushed to user space in many case but by a rather indirect route, so I think it is more appropriate to switch this to s64 which is the type of almost all the values passed into it. Note a follow up series may tidy up some local variables that are of the int64_t type.
- Change the few int64_t timestamp __aligned(8) cases to aligned_s64. This is mostly a consistency thing to avoid confusion if a new author is trying to figure out what types they should use. Better to have one answer than a mixture.
There is one other IIO driver hiding over in media that will be modified by a separate patch because I want to add some more info to that and a more specific CC list.
Jonathan Cameron (20):
iio: adc: ad7944: Fix sign and use aligned_s64 for timestamp.
io: adc: ina2xx-adc: Fix sign and use aligned_s64 for timestamp.
iio: temperature: tmp006: Use aligned_s64 instead of open coding
alignment.
iio: resolver: ad2s1210: Use aligned_s64 instead of open coding
alignment.
iio: proximity: Use aligned_s64 instead of open coding alignment.
iio: pressure: Use aligned_s64 instead of open coding alignment.
iio: magnetometer: Use aligned_s64 instead of open coding alignment.
iio: light: Use aligned_s64 instead of open coding alignment.
iio: imu: Use aligned_s64 instead of open coding alignment.
iio: humidity: Use aligned_s64 instead of open coding alignment.
iio: gyro: Use aligned_s64 instead of open coding alignment.
iio: chemical: Use aligned_s64 instead of open coding alignment.
iio: adc: Use aligned_s64 instead of open coding alignment.
iio: accel: bma220: Use aligned_s64 instead of open coding alignment.
iio: buffer: Make timestamp s64 in
iio_push_to_buffers_with_timestamp()
iio: adc: ti-lmp92064: Switch timestamp type from int64_t __aligned(8)
to aligned_s64
iio: chemical: scd4x: witch timestamp type from int64_t __aligned(8)
to aligned_s64
iio: imu: inv_icm42600: witch timestamp type from int64_t __aligned(8)
to aligned_s64
iio: adc: mt6360: Correct marking of timestamp alignment.
iio: adc: rockchip: correct alignment of timestamp
drivers/iio/accel/bma220_spi.c | 2 +- drivers/iio/adc/ad4000.c | 2 +- drivers/iio/adc/ad7944.c | 2 +- drivers/iio/adc/ina2xx-adc.c | 2 +- drivers/iio/adc/max1118.c | 2 +- drivers/iio/adc/max11410.c | 2 +- drivers/iio/adc/mcp3911.c | 2 +- drivers/iio/adc/mt6360-adc.c | 4 ++-- drivers/iio/adc/pac1921.c | 2 +- drivers/iio/adc/rockchip_saradc.c | 2 +- drivers/iio/adc/rtq6056.c | 2 +- drivers/iio/adc/ti-adc081c.c | 2 +- drivers/iio/adc/ti-adc084s021.c | 2 +- drivers/iio/adc/ti-ads1015.c | 2 +- drivers/iio/adc/ti-ads1119.c | 2 +- drivers/iio/adc/ti-ads131e08.c | 2 +- drivers/iio/adc/ti-lmp92064.c | 2 +- drivers/iio/adc/ti-tsc2046.c | 2 +- drivers/iio/adc/vf610_adc.c | 2 +- drivers/iio/chemical/ccs811.c | 2 +- drivers/iio/chemical/ens160_core.c | 2 +- drivers/iio/chemical/scd30_core.c | 2 +- drivers/iio/chemical/scd4x.c | 2 +- drivers/iio/gyro/adxrs290.c | 2 +- drivers/iio/gyro/bmg160_core.c | 2 +- drivers/iio/gyro/itg3200_buffer.c | 2 +- drivers/iio/gyro/mpu3050-core.c | 2 +- drivers/iio/humidity/am2315.c | 2 +- drivers/iio/humidity/hdc100x.c | 2 +- drivers/iio/humidity/hts221.h | 2 +- drivers/iio/imu/bmi323/bmi323_core.c | 2 +- drivers/iio/imu/bno055/bno055.c | 2 +- drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 2 +- drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c | 2 +- drivers/iio/light/adjd_s311.c | 2 +- drivers/iio/light/as73211.c | 2 +- drivers/iio/light/bh1745.c | 2 +- drivers/iio/light/isl29125.c | 2 +- drivers/iio/light/ltr501.c | 2 +- drivers/iio/light/max44000.c | 2 +- drivers/iio/light/rohm-bu27034.c | 2 +- drivers/iio/light/rpr0521.c | 2 +- drivers/iio/light/st_uvis25.h | 2 +- drivers/iio/light/tcs3414.c | 2 +- drivers/iio/light/tcs3472.c | 2 +- drivers/iio/magnetometer/af8133j.c | 2 +- drivers/iio/magnetometer/ak8974.c | 2 +- drivers/iio/magnetometer/ak8975.c | 2 +- drivers/iio/magnetometer/bmc150_magn.c | 2 +- drivers/iio/magnetometer/hmc5843.h | 2 +- drivers/iio/magnetometer/mag3110.c | 2 +- drivers/iio/magnetometer/yamaha-yas530.c | 2 +- drivers/iio/pressure/hsc030pa.h | 2 +- drivers/iio/pressure/ms5611_core.c | 2 +- drivers/iio/pressure/rohm-bm1390.c | 2 +- drivers/iio/proximity/as3935.c | 2 +- drivers/iio/proximity/hx9023s.c | 2 +- drivers/iio/proximity/mb1232.c | 2 +- drivers/iio/proximity/pulsedlight-lidar-lite-v2.c | 2 +- drivers/iio/proximity/srf08.c | 2 +- drivers/iio/proximity/sx_common.h | 2 +- drivers/iio/resolver/ad2s1210.c | 2 +- drivers/iio/temperature/tmp006.c | 2 +- include/linux/iio/buffer.h | 6 +++--- 64 files changed, 67 insertions(+), 67 deletions(-)
Signature · 1 line
2.47.1
Jonathan Cameron
maintainerPATCH 1/20 iio: adc: ad7944: Fix sign and use aligned_s64 for timestamp.
From: Jonathan Cameron <[email protected]>
Whilst it doesn't actually make any difference because the code that fills this field doesn't care, timestamps are all signed. Use the new aligned_s64 instead of open coding alignment to avoid confusing static analyzers and give slightly cleaner code.
--- drivers/iio/adc/ad7944.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/adc/ad7944.c +1 -1
@@ -75,7 +75,7 @@ struct ad7944_adc {u16 u16;u32 u32; } raw; -u64 timestamp__aligned(8) ; +aligned_s64 timestamp; } sample__aligned(IIO_DMA_MINALIGN) ; };
Signature · 1 line
2.47.1
Re: [PATCH 01/20] iio: adc: ad7944: Fix sign and use aligned_s64 for timestamp.
Quoted 9 lines · Jonathan Cameron
From: Jonathan Cameron <[email protected]>
Whilst it doesn't actually make any difference because the code that fills this field doesn't care, timestamps are all signed. Use the new
aligned_s64instead of open coding alignment to avoid confusing static analyzers and give slightly cleaner code.Signed-off-by Jonathan Cameron <[email protected]>---
Jonathan Cameron
maintainerPATCH 2/20 io: adc: ina2xx-adc: Fix sign and use aligned_s64 for timestamp.
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
Whilst it doesn't actually make any difference because the code that fills this field doesn't care, timestamps are all signed. Use the new aligned_s64 instead of open coding alignment to avoid confusing static analyzers and give slightly cleaner code.
--- drivers/iio/adc/ina2xx-adc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/adc/ina2xx-adc.c +1 -1
@@ -150,7 +150,7 @@ struct ina2xx_chip_info {/* data buffer needs space for channel data and timestamp */ struct {u16 chan[4 ]; -u64 ts__aligned(8) ; +aligned_s64 ts; } scan; };
Signature · 1 line
2.47.1
Re: [PATCH 02/20] io: adc: ina2xx-adc: Fix sign and use aligned_s64 for timestamp.
Hello,
this patch is in next as 1b54068b5934a871f1895adc5e5ca4355781eeb7.
Maybe it's already to late for that, but if not:
$Subject ~= s/io:/iio:/
Best regards
Uwe
Re: [PATCH 02/20] io: adc: ina2xx-adc: Fix sign and use aligned_s64 for timestamp.
Quoted 11 lines · On Thu, 30 Jan 2025 16:40:04 +0100 Uwe Kleine-König…
Hello,
this patch is in next as
1b54068b5934a871f1895adc5e5ca4355781eeb7.Maybe it's already to late for that, but if not:
$Subject ~= s/io:/iio:/Best regards
Uwe
Long gone :( It's in mainline.
Jonathan
Jonathan Cameron
maintainerPATCH 3/20 iio: temperature: tmp006: Use aligned_s64 instead of open coding alignment.
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
Use this new type to both slightly simplify the code and avoid confusing static analysis tools. Mostly this series is about consistency to avoid this code pattern getting copied into more drivers.
--- drivers/iio/temperature/tmp006.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/temperature/tmp006.c +1 -1
@@ -248,7 +248,7 @@ static irqreturn_t tmp006_trigger_handler(int irq, void *p)struct tmp006_data *data =iio_priv (indio_dev);struct {s16 channels[2 ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan;s32 ret;
Signature · 1 line
2.47.1
Jonathan Cameron
maintainerPATCH 4/20 iio: resolver: ad2s1210: Use aligned_s64 instead of open coding alignment.
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
Use this new type to both slightly simplify the code and avoid confusing static analysis tools. Mostly this series is about consistency to avoid this code pattern getting copied into more drivers.
--- drivers/iio/resolver/ad2s1210.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/resolver/ad2s1210.c +1 -1
@@ -164,7 +164,7 @@ struct ad2s1210_state {struct {__be16 chan[2 ];/* Ensure timestamp is naturally aligned. */ -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan;/** SPI transmit buffer. */ u8 rx[2 ];
Signature · 1 line
2.47.1
Jonathan Cameron
maintainerPATCH 5/20 iio: proximity: Use aligned_s64 instead of open coding alignment.
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
Use this new type to both slightly simplify the code and avoid confusing static analysis tools. Mostly this series is about consistency to avoid this code pattern getting copied into more drivers.
--- drivers/iio/proximity/as3935.c | 2 +- drivers/iio/proximity/hx9023s.c | 2 +- drivers/iio/proximity/mb1232.c | 2 +- drivers/iio/proximity/pulsedlight-lidar-lite-v2.c | 2 +- drivers/iio/proximity/srf08.c | 2 +- drivers/iio/proximity/sx_common.h | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-)
drivers/iio/proximity/as3935.c +1 -1
@@ -63,7 +63,7 @@ struct as3935_state {/* Ensure timestamp is naturally aligned */ struct {u8 chan; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan;u8 buf[2 ]__aligned(IIO_DMA_MINALIGN) ; };
drivers/iio/proximity/hx9023s.c +1 -1
@@ -134,7 +134,7 @@ struct hx9023s_data {struct {__le16 channels[HX9023S_CH_NUM ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } buffer;/*
drivers/iio/proximity/mb1232.c +1 -1
@@ -45,7 +45,7 @@ struct mb1232_data {/* Ensure correct alignment of data to push to IIO buffer */ struct {s16 distance; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan; };
drivers/iio/proximity/pulsedlight-lidar-lite-v2.c +1 -1
@@ -47,7 +47,7 @@ struct lidar_data {/* Ensure timestamp is naturally aligned */ struct {u16 chan; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan; };
drivers/iio/proximity/srf08.c +1 -1
@@ -66,7 +66,7 @@ struct srf08_data {/* Ensure timestamp is naturally aligned */ struct {s16 chan; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan;/* Sensor-Type */
drivers/iio/proximity/sx_common.h +1 -1
@@ -125,7 +125,7 @@ struct sx_common_data {/* Ensure correct alignment of timestamp when present. */ struct {__be16 channels[SX_COMMON_MAX_NUM_CHANNELS ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } buffer;unsigned int suspend_ctrl;
Signature · 1 line
2.47.1
Jonathan Cameron
maintainerPATCH 6/20 iio: pressure: Use aligned_s64 instead of open coding alignment.
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
Use this new type to both slightly simplify the code and avoid confusing static analysis tools. Mostly this series is about consistency to avoid this code pattern getting copied into more drivers.
--- drivers/iio/pressure/hsc030pa.h | 2 +- drivers/iio/pressure/ms5611_core.c | 2 +- drivers/iio/pressure/rohm-bm1390.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
drivers/iio/pressure/hsc030pa.h +1 -1
@@ -58,7 +58,7 @@ struct hsc_data {s32 p_offset_dec;struct {__be16 chan[2 ]; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan;u8 buffer[HSC_REG_MEASUREMENT_RD_SIZE ]__aligned(IIO_DMA_MINALIGN) ; };
drivers/iio/pressure/ms5611_core.c +1 -1
@@ -213,7 +213,7 @@ static irqreturn_t ms5611_trigger_handler(int irq, void *p)/* Ensure buffer elements are naturally aligned */ struct {s32 channels[2 ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan;int ret;
drivers/iio/pressure/rohm-bm1390.c +1 -1
@@ -139,7 +139,7 @@ enum {struct bm1390_data_buf {u32 pressure;__be16 temp; -s64 ts__aligned(8) ; +aligned_s64 ts; };/* BM1390 has FIFO for 4 pressure samples */
Signature · 1 line
2.47.1
Re: [PATCH 06/20] iio: pressure: Use aligned_s64 instead of open coding alignment.
Quoted 9 lines · Jonathan Cameron
From: Jonathan Cameron <[email protected]>
Use this new type to both slightly simplify the code and avoid confusing static analysis tools. Mostly this series is about consistency to avoid this code pattern getting copied into more drivers.
Signed-off-by Jonathan Cameron <[email protected]>--- drivers/iio/pressure/rohm-bm1390.c | 2 +-
For the BD1390:
Yours,
-- Matti
Jonathan Cameron
maintainerPATCH 7/20 iio: magnetometer: Use aligned_s64 instead of open coding alignment.
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
Use this new type to both slightly simplify the code and avoid confusing static analysis tools. Mostly this series is about consistency to avoid this code pattern getting copied into more drivers.
--- drivers/iio/magnetometer/af8133j.c | 2 +- drivers/iio/magnetometer/ak8974.c | 2 +- drivers/iio/magnetometer/ak8975.c | 2 +- drivers/iio/magnetometer/bmc150_magn.c | 2 +- drivers/iio/magnetometer/hmc5843.h | 2 +- drivers/iio/magnetometer/mag3110.c | 2 +- drivers/iio/magnetometer/yamaha-yas530.c | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-)
drivers/iio/magnetometer/af8133j.c +1 -1
@@ -360,7 +360,7 @@ static irqreturn_t af8133j_trigger_handler(int irq, void *p)s64 timestamp =iio_get_time_ns (indio_dev);struct {__le16 values[3 ]; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } sample;int ret;
drivers/iio/magnetometer/ak8974.c +1 -1
@@ -197,7 +197,7 @@ struct ak8974 {/* Ensure timestamp is naturally aligned */ struct {__le16 channels[3 ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan; };
drivers/iio/magnetometer/ak8975.c +1 -1
@@ -426,7 +426,7 @@ struct ak8975_data {/* Ensure natural alignment of timestamp */ struct {s16 channels[3 ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan; };
drivers/iio/magnetometer/bmc150_magn.c +1 -1
@@ -140,7 +140,7 @@ struct bmc150_magn_data {/* Ensure timestamp is naturally aligned */ struct {s32 chans[3 ]; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan;struct iio_trigger *dready_trig;bool dready_trigger_on;
drivers/iio/magnetometer/hmc5843.h +1 -1
@@ -44,7 +44,7 @@ struct hmc5843_data {struct iio_mount_matrix orientation;struct {__be16 chans[3 ]; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan; };
drivers/iio/magnetometer/mag3110.c +1 -1
@@ -60,7 +60,7 @@ struct mag3110_data {struct {__be16 channels[3 ];u8 temperature; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan; };
drivers/iio/magnetometer/yamaha-yas530.c +1 -1
@@ -236,7 +236,7 @@ struct yas5xx {*/ struct {s32 channels[4 ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan; };
Signature · 1 line
2.47.1
Jonathan Cameron
maintainerPATCH 8/20 iio: light: Use aligned_s64 instead of open coding alignment.
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
Use this new type to both slightly simplify the code and avoid confusing static analysis tools. Mostly this series is about consistency to avoid this code pattern getting copied into more drivers.
--- drivers/iio/light/adjd_s311.c | 2 +- drivers/iio/light/as73211.c | 2 +- drivers/iio/light/bh1745.c | 2 +- drivers/iio/light/isl29125.c | 2 +- drivers/iio/light/ltr501.c | 2 +- drivers/iio/light/max44000.c | 2 +- drivers/iio/light/rohm-bu27034.c | 2 +- drivers/iio/light/rpr0521.c | 2 +- drivers/iio/light/st_uvis25.h | 2 +- drivers/iio/light/tcs3414.c | 2 +- drivers/iio/light/tcs3472.c | 2 +- 11 files changed, 11 insertions(+), 11 deletions(-)
drivers/iio/light/adjd_s311.c +1 -1
@@ -56,7 +56,7 @@ struct adjd_s311_data {struct i2c_client *client;struct {s16 chans[4 ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan; };
drivers/iio/light/as73211.c +1 -1
@@ -636,7 +636,7 @@ static irqreturn_t as73211_trigger_handler(int irq __always_unused, void *p)struct as73211_data *data =iio_priv (indio_dev);struct {__le16 chan[4 ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan;int data_result, ret;
drivers/iio/light/bh1745.c +1 -1
@@ -739,7 +739,7 @@ static irqreturn_t bh1745_trigger_handler(int interrupt, void *p)struct bh1745_data *data =iio_priv (indio_dev);struct {u16 chans[4 ]; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan;u16 value;int ret;
drivers/iio/light/isl29125.c +1 -1
@@ -54,7 +54,7 @@ struct isl29125_data {/* Ensure timestamp is naturally aligned */ struct {u16 chans[3 ]; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan; };
drivers/iio/light/ltr501.c +1 -1
@@ -1280,7 +1280,7 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p)struct ltr501_data *data =iio_priv (indio_dev);struct {u16 channels[3 ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan;__le16 als_buf[2 ];u8 mask =0 ;
drivers/iio/light/max44000.c +1 -1
@@ -78,7 +78,7 @@ struct max44000_data {/* Ensure naturally aligned timestamp */ struct {u16 channels[2 ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan; };
drivers/iio/light/rohm-bu27034.c +1 -1
@@ -206,7 +206,7 @@ struct bu27034_data {struct {u32 mlux;__le16 channels[BU27034_NUM_HW_DATA_CHANS ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan; };
drivers/iio/light/rpr0521.c +1 -1
@@ -203,7 +203,7 @@ struct rpr0521_data {struct {__le16 channels[3 ];u8 garbage; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan; };
drivers/iio/light/st_uvis25.h +1 -1
@@ -30,7 +30,7 @@ struct st_uvis25_hw {/* Ensure timestamp is naturally aligned */ struct {u8 chan; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan; };
drivers/iio/light/tcs3414.c +1 -1
@@ -56,7 +56,7 @@ struct tcs3414_data {/* Ensure timestamp is naturally aligned */ struct {u16 chans[4 ]; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan; };
drivers/iio/light/tcs3472.c +1 -1
@@ -67,7 +67,7 @@ struct tcs3472_data {/* Ensure timestamp is naturally aligned */ struct {u16 chans[4 ]; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan; };
Signature · 1 line
2.47.1
Re: [PATCH 08/20] iio: light: Use aligned_s64 instead of open coding alignment.
Quoted 10 lines · Jonathan Cameron
From: Jonathan Cameron <[email protected]>
Use this new type to both slightly simplify the code and avoid confusing static analysis tools. Mostly this series is about consistency to avoid this code pattern getting copied into more drivers.
Signed-off-by Jonathan Cameron <[email protected]>--- drivers/iio/light/rohm-bu27034.c | 2 +- drivers/iio/light/rpr0521.c | 2 +-
FWIW:
For these two (although I'm not maintaining the RPR driver)
Yours,
-- Matti
Jonathan Cameron
maintainerPATCH 9/20 iio: imu: Use aligned_s64 instead of open coding alignment.
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
Use this new type to both slightly simplify the code and avoid confusing static analysis tools. Mostly this series is about consistency to avoid this code pattern getting copied into more drivers.
--- drivers/iio/imu/bmi323/bmi323_core.c | 2 +- drivers/iio/imu/bno055/bno055.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/iio/imu/bmi323/bmi323_core.c +1 -1
@@ -174,7 +174,7 @@ struct bmi323_data {__le16 fifo_buff[BMI323_FIFO_FULL_IN_WORDS ]__aligned(IIO_DMA_MINALIGN) ;struct {__le16 channels[BMI323_CHAN_MAX ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } buffer;__le16 steps_count[BMI323_STEP_LEN ]; };
drivers/iio/imu/bno055/bno055.c +1 -1
@@ -207,7 +207,7 @@ struct bno055_priv {bool sw_reset;struct {__le16 chans[BNO055_SCAN_CH_COUNT ]; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } buf;struct dentry *debugfs; };
Signature · 1 line
2.47.1
Jonathan Cameron
maintainerPATCH 10/20 iio: humidity: Use aligned_s64 instead of open coding alignment.
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
Use this new type to both slightly simplify the code and avoid confusing static analysis tools. Mostly this series is about consistency to avoid this code pattern getting copied into more drivers.
--- drivers/iio/humidity/am2315.c | 2 +- drivers/iio/humidity/hdc100x.c | 2 +- drivers/iio/humidity/hts221.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
drivers/iio/humidity/am2315.c +1 -1
@@ -35,7 +35,7 @@ struct am2315_data {/* Ensure timestamp is naturally aligned */ struct {s16 chans[2 ]; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan; };
drivers/iio/humidity/hdc100x.c +1 -1
@@ -44,7 +44,7 @@ struct hdc100x_data {/* Ensure natural alignment of timestamp */ struct {__be16 channels[2 ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan; };
drivers/iio/humidity/hts221.h +1 -1
@@ -40,7 +40,7 @@ struct hts221_hw {/* Ensure natural alignment of timestamp */ struct {__le16 channels[2 ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan; };
Signature · 1 line
2.47.1
Jonathan Cameron
maintainerPATCH 11/20 iio: gyro: Use aligned_s64 instead of open coding alignment.
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
Use this new type to both slightly simplify the code and avoid confusing static analysis tools. Mostly this series is about consistency to avoid this code pattern getting copied into more drivers.
--- drivers/iio/gyro/adxrs290.c | 2 +- drivers/iio/gyro/bmg160_core.c | 2 +- drivers/iio/gyro/itg3200_buffer.c | 2 +- drivers/iio/gyro/mpu3050-core.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-)
drivers/iio/gyro/adxrs290.c +1 -1
@@ -75,7 +75,7 @@ struct adxrs290_state {/* Ensure correct alignment of timestamp when present */ struct {s16 channels[3 ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } buffer; };
drivers/iio/gyro/bmg160_core.c +1 -1
@@ -99,7 +99,7 @@ struct bmg160_data {/* Ensure naturally aligned timestamp */ struct {s16 chans[3 ]; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan;u32 dps_range;int ev_enable_state;
drivers/iio/gyro/itg3200_buffer.c +1 -1
@@ -52,7 +52,7 @@ static irqreturn_t itg3200_trigger_handler(int irq, void *p)*/ struct {__be16 buf[ITG3200_SCAN_ELEMENTS ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan;int ret =itg3200_read_all_channels (st->i2c, scan.buf);
drivers/iio/gyro/mpu3050-core.c +1 -1
@@ -474,7 +474,7 @@ static irqreturn_t mpu3050_trigger_handler(int irq, void *p)int ret;struct {__be16 chans[4 ]; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan;s64 timestamp;unsigned int datums_from_fifo =0 ;
Signature · 1 line
2.47.1
Jonathan Cameron
maintainerPATCH 12/20 iio: chemical: Use aligned_s64 instead of open coding alignment.
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
Use this new type to both slightly simplify the code and avoid confusing static analysis tools. Mostly this series is about consistency to avoid this code pattern getting copied into more drivers.
--- drivers/iio/chemical/ccs811.c | 2 +- drivers/iio/chemical/ens160_core.c | 2 +- drivers/iio/chemical/scd30_core.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
drivers/iio/chemical/ccs811.c +1 -1
@@ -81,7 +81,7 @@ struct ccs811_data {/* Ensures correct alignment of timestamp if present */ struct {s16 channels[2 ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan; };
drivers/iio/chemical/ens160_core.c +1 -1
@@ -60,7 +60,7 @@ struct ens160_data {struct mutex mutex;struct {__le16 chans[2 ]; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan__aligned(IIO_DMA_MINALIGN) ;u8 fw_version[3 ];__le16 buf;
drivers/iio/chemical/scd30_core.c +1 -1
@@ -594,7 +594,7 @@ static irqreturn_t scd30_trigger_handler(int irq, void *p)struct scd30_state *state =iio_priv (indio_dev);struct {int data[SCD30_MEAS_COUNT ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan;int ret;
Signature · 1 line
2.47.1
Jonathan Cameron
maintainerPATCH 13/20 iio: adc: Use aligned_s64 instead of open coding alignment.
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
Use this new type to both slightly simplify the code and avoid confusing static analysis tools. Mostly this series is about consistency to avoid this code pattern getting copied into more drivers.
--- drivers/iio/adc/ad4000.c | 2 +- drivers/iio/adc/max1118.c | 2 +- drivers/iio/adc/max11410.c | 2 +- drivers/iio/adc/mcp3911.c | 2 +- drivers/iio/adc/pac1921.c | 2 +- drivers/iio/adc/rtq6056.c | 2 +- drivers/iio/adc/ti-adc081c.c | 2 +- drivers/iio/adc/ti-adc084s021.c | 2 +- drivers/iio/adc/ti-ads1015.c | 2 +- drivers/iio/adc/ti-ads1119.c | 2 +- drivers/iio/adc/ti-ads131e08.c | 2 +- drivers/iio/adc/ti-tsc2046.c | 2 +- drivers/iio/adc/vf610_adc.c | 2 +- 13 files changed, 13 insertions(+), 13 deletions(-)
drivers/iio/adc/ad4000.c +1 -1
@@ -414,7 +414,7 @@ struct ad4000_state {__be16 sample_buf16;__be32 sample_buf32; } data; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan__aligned(IIO_DMA_MINALIGN) ;u8 tx_buf[2 ];u8 rx_buf[2 ];
drivers/iio/adc/max1118.c +1 -1
@@ -39,7 +39,7 @@ struct max1118 {/* Ensure natural alignment of buffer elements */ struct {u8 channels[2 ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan;u8 data__aligned(IIO_DMA_MINALIGN) ;
drivers/iio/adc/max11410.c +1 -1
@@ -143,7 +143,7 @@ struct max11410_state {int irq;struct {u32 data__aligned(IIO_DMA_MINALIGN) ; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan; };
drivers/iio/adc/mcp3911.c +1 -1
@@ -122,7 +122,7 @@ struct mcp3911 {const struct mcp3911_chip_info *chip;struct {u32 channels[MCP39XX_MAX_NUM_CHANNELS ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan;u8 tx_buf__aligned(IIO_DMA_MINALIGN) ;
drivers/iio/adc/pac1921.c +1 -1
@@ -209,7 +209,7 @@ struct pac1921_priv {struct {u16 chan[PAC1921_NUM_MEAS_CHANS ]; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan; };
drivers/iio/adc/rtq6056.c +1 -1
@@ -634,7 +634,7 @@ static irqreturn_t rtq6056_buffer_trigger_handler(int irq, void *p)struct device *dev = priv->dev;struct {u16 vals[RTQ6056_MAX_CHANNEL ]; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } data;unsigned int raw;int i =0 , bit, ret;
drivers/iio/adc/ti-adc081c.c +1 -1
@@ -37,7 +37,7 @@ struct adc081c {/* Ensure natural alignment of buffer elements */ struct {u16 channel; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan; };
drivers/iio/adc/ti-adc084s021.c +1 -1
@@ -29,7 +29,7 @@ struct adc084s021 {/* Buffer used to align data */ struct {__be16 channels[4 ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } scan;/* * DMA (thus cache coherency maintenance) may require the
drivers/iio/adc/ti-ads1015.c +1 -1
@@ -448,7 +448,7 @@ static irqreturn_t ads1015_trigger_handler(int irq, void *p)/* Ensure natural alignment of timestamp */ struct {s16 chan; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan;int chan, ret, res;
drivers/iio/adc/ti-ads1119.c +1 -1
@@ -501,7 +501,7 @@ static irqreturn_t ads1119_trigger_handler(int irq, void *private)struct ads1119_state *st =iio_priv (indio_dev);struct {unsigned int sample; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan;unsigned int index;int ret;
drivers/iio/adc/ti-ads131e08.c +1 -1
@@ -102,7 +102,7 @@ struct ads131e08_state {struct completion completion;struct {u8 data[ADS131E08_NUM_DATA_BYTES_MAX ]; -s64 ts__aligned(8) ; +aligned_s64 ts; } tmp_buf;u8 tx_buf[3 ]__aligned(IIO_DMA_MINALIGN) ;
drivers/iio/adc/ti-tsc2046.c +1 -1
@@ -157,7 +157,7 @@ struct tsc2046_adc_priv {/* Scan data for each channel */ u16 data[TI_TSC2046_MAX_CHAN ];/* Timestamp */ -s64 ts__aligned(8) ; +aligned_s64 ts; } scan_buf;/*
drivers/iio/adc/vf610_adc.c +1 -1
@@ -173,7 +173,7 @@ struct vf610_adc {/* Ensure the timestamp is naturally aligned */ struct {u16 chan; -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan; };
Signature · 1 line
2.47.1
Re: [PATCH 13/20] iio: adc: Use aligned_s64 instead of open coding alignment.
Quoted 8 lines · Jonathan Cameron
From: Jonathan Cameron <[email protected]>
Use this new type to both slightly simplify the code and avoid confusing static analysis tools. Mostly this series is about consistency to avoid this code pattern getting copied into more drivers.
Signed-off-by Jonathan Cameron <[email protected]>---
Thanks
Jonathan Cameron
maintainerPATCH 14/20 iio: accel: bma220: Use aligned_s64 instead of open coding alignment.
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
Use this new type to both slightly simplify the code and avoid confusing static analysis tools. Mostly this series is about consistency to avoid this code pattern getting copied into more drivers.
--- drivers/iio/accel/bma220_spi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/accel/bma220_spi.c +1 -1
@@ -66,7 +66,7 @@ struct bma220_data {struct {s8 chans[3 ];/* Ensure timestamp is naturally aligned. */ -s64 timestamp__aligned(8) ; +aligned_s64 timestamp; } scan;u8 tx_buf[2 ]__aligned(IIO_DMA_MINALIGN) ; };
Signature · 1 line
2.47.1
Jonathan Cameron
maintainerPATCH 15/20 iio: buffer: Make timestamp s64 in iio_push_to_buffers_with_timestamp()
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
This is a bit of a corner case for selecting between the in kernel types and standard c integer types we tend to prefer for userspace interfaces. The interface is entirely within the kernel but in many cases the data ultimately ends up in userspace (via some time in a kfifo). On balance the value passed is almost always an s64, so standardize on that. Main reason to change this is that it has led to some inconsistency in the storage type used. The majority use aligned_s64 rather than int64_t __aligned(8) and this will ensure there is one obvious choice.
--- include/linux/iio/buffer.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
include/linux/iio/buffer.h +3 -3
@@ -35,11 +35,11 @@ int iio_pop_from_buffer(struct iio_buffer *buffer, void *data);* Returns 0 on success, a negative error code otherwise. */ static inline int iio_push_to_buffers_with_timestamp (struct iio_dev *indio_dev, -void *data,int64_t timestamp) +void *data,s64 timestamp) {if (indio_dev->scan_timestamp) { -size_t ts_offset = indio_dev->scan_bytes /sizeof (int64_t ) -1 ; - ((int64_t *)data)[ts_offset] = timestamp; +size_t ts_offset = indio_dev->scan_bytes /sizeof (s64) -1 ; + ((s64 *)data)[ts_offset] = timestamp; }return iio_push_to_buffers (indio_dev, data);
Signature · 1 line
2.47.1
Re: [PATCH 15/20] iio: buffer: Make timestamp s64 in iio_push_to_buffers_with_timestamp()
Quoted 4 lines · Jonathan Cameron
From: Jonathan Cameron <[email protected]>
This is a bit of a corner case for selecting between the in kernel types and standard c integer types we tend to prefer for userspace interfaces.
s/c/C/
Quoted 6 lines
The interface is entirely within the kernel but in many cases the data ultimately ends up in userspace (via some time in a kfifo). On balance the value passed is almost always an s64, so standardize on that. Main reason to change this is that it has led to some inconsistency in the storage type used. The majority use
aligned_s64rather thanint64_t__aligned(8)and this will ensure there is one obvious choice.
...
include/linux/iio/buffer.h int iio_pop_from_buffer(struct iio_buffer *buffer, void *data); · line 38
Quoted 3 lines
static inline int iio_push_to_buffers_with_timestamp(struct
iio_dev*indio_dev,include/linux/iio/buffer.h +1
-1- void *data, int64_t timestamp) + void *data, s64 timestamp)
Hmm... Is it the indentation used for other static inline definitions there? Otherwise I would fix it to follow standard pattern (use same column as the first argument).
...
include/linux/iio/buffer.h int iio_pop_from_buffer(struct iio_buffer *buffer, void *data); · line 41
Quoted 4 lines
include/linux/iio/buffer.h +1
-2if (indio_dev->scan_timestamp) { -size_t ts_offset = indio_dev->scan_bytes /sizeof (int64_t ) -1 ; - ((int64_t *)data)[ts_offset] = timestamp; +size_t ts_offset = indio_dev->scan_bytes /sizeof (s64) -1 ;
sizeof(timestamp) ?
Quoted 2 lines
include/linux/iio/buffer.h +1
-0+ ((s64 *)data)[ts_offset] = timestamp; }
Signature · 2 lines
With Best Regards, Andy Shevchenko
Jonathan Cameron
maintainerRe: [PATCH 15/20] iio: buffer: Make timestamp s64 in iio_push_to_buffers_with_timestamp()
Quoted 17 lines · On Sun, 15 Dec 2024 23:12:13 +0200 Andy Shevchenko…
Quoted 4 lines · Jonathan Cameron
From: Jonathan Cameron <[email protected]>
This is a bit of a corner case for selecting between the in kernel types and standard c integer types we tend to prefer for userspace interfaces.
s/c/C/
Quoted 6 lines
The interface is entirely within the kernel but in many cases the data ultimately ends up in userspace (via some time in a kfifo). On balance the value passed is almost always an s64, so standardize on that. Main reason to change this is that it has led to some inconsistency in the storage type used. The majority use
aligned_s64rather thanint64_t__aligned(8)and this will ensure there is one obvious choice....
Quoted 7 lines
include/linux/iio/buffer.h int iio_pop_from_buffer(struct iio_buffer *buffer, void *data); · line 38
Quoted 3 lines
static inline int iio_push_to_buffers_with_timestamp(struct
iio_dev*indio_dev,include/linux/iio/buffer.h +1
-1- void *data, int64_t timestamp) + void *data, s64 timestamp)Hmm... Is it the indentation used for other static inline definitions there? Otherwise I would fix it to follow standard pattern (use same column as the first argument).
Makes sense.
Quoted 9 lines
...
include/linux/iio/buffer.h int iio_pop_from_buffer(struct iio_buffer *buffer, void *data); · line 41
Quoted 4 lines
include/linux/iio/buffer.h +1
-2if (indio_dev->scan_timestamp) { -size_t ts_offset = indio_dev->scan_bytes /sizeof (int64_t ) -1 ; - ((int64_t *)data)[ts_offset] = timestamp; +size_t ts_offset = indio_dev->scan_bytes /sizeof (s64) -1 ;sizeof(timestamp) ?
I think not on this one. It's about the type of the cast on the next line more than timestamp type. If timestamp were an s32 and we were writing it into a 64 bit location then sizeof(timestamp) would be the wrong size but whereas the proposal here would still work. If we were doing a memcpy rather than a simple assignment the sizes would have to be the same and your suggestion would make sense.
Could introduce a local variable to make this more obvious and provide a parameter for the sizeof()
s64 *datas64 = (s64 *)data; size_t ts_offset = indio-dev->scan_bytes / sizeof(*datas64) - 1; datas64[ts_offset] = timestamp;
Do you think that is worth doing or just adding complexity we don't need? I don't see it as much more readable, so think on balance sticking to original proposal in this patch makes more sense.
Note this needed a rebase anyway as scan_timestamp is now private.
Quoted 4 lines
Quoted 3 lines
include/linux/iio/buffer.h +1
-0+ ((s64 *)data)[ts_offset] = timestamp; }
Jonathan Cameron
maintainerPATCH 16/20 iio: adc: ti-lmp92064: Switch timestamp type from int64_t __aligned(8) to aligned_s64
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
The vast majority of IIO drivers use aligned_s64 for the type of the timestamp field. It is not a bug to use int64_t and until this series iio_push_to_buffers_with_timestamp() took and int64_t timestamp, it is inconsistent. This change is to remove that inconsistency and ensure there is one obvious choice for future drivers.
--- drivers/iio/adc/ti-lmp92064.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/adc/ti-lmp92064.c +1 -1
@@ -199,7 +199,7 @@ static irqreturn_t lmp92064_trigger_handler(int irq, void *p)struct lmp92064_adc_priv *priv =iio_priv (indio_dev);struct {u16 values[2 ]; -int64_t timestamp__aligned(8) ; +aligned_s64 timestamp; } data;int ret;
Signature · 1 line
2.47.1
Jonathan Cameron
maintainerPATCH 17/20 iio: chemical: scd4x: switch timestamp type from int64_t __aligned(8) to aligned_s64
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
The vast majority of IIO drivers use aligned_s64 for the type of the timestamp field. It is not a bug to use int64_t and until this series iio_push_to_buffers_with_timestamp() took and int64_t timestamp, it is inconsistent. This change is to remove that inconsistency and ensure there is one obvious choice for future drivers.
--- drivers/iio/chemical/scd4x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/chemical/scd4x.c +1 -1
@@ -665,7 +665,7 @@ static irqreturn_t scd4x_trigger_handler(int irq, void *p)struct scd4x_state *state =iio_priv (indio_dev);struct {uint16_t data[3 ]; -int64_t ts__aligned(8) ; +aligned_s64 ts; } scan;int ret;
Signature · 1 line
2.47.1
Jonathan Cameron
maintainerPATCH 18/20 iio: imu: inv_icm42600: switch timestamp type from int64_t __aligned(8) to aligned_s64
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
The vast majority of IIO drivers use aligned_s64 for the type of the timestamp field. It is not a bug to use int64_t and until this series iio_push_to_buffers_with_timestamp() took and int64_t timestamp, it is inconsistent. This change is to remove that inconsistency and ensure there is one obvious choice for future drivers.
--- drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 2 +- drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c +1 -1
@@ -178,7 +178,7 @@ static const struct iio_chan_spec inv_icm42600_accel_channels[] = {struct inv_icm42600_accel_buffer {struct inv_icm42600_fifo_sensor_data accel;int16_t temp; -int64_t timestamp__aligned(8) ; +aligned_s64 timestamp; };#define INV_ICM42600_SCAN_MASK_ACCEL_3AXIS \
drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c +1 -1
@@ -78,7 +78,7 @@ static const struct iio_chan_spec inv_icm42600_gyro_channels[] = {struct inv_icm42600_gyro_buffer {struct inv_icm42600_fifo_sensor_data gyro;int16_t temp; -int64_t timestamp__aligned(8) ; +aligned_s64 timestamp; };#define INV_ICM42600_SCAN_MASK_GYRO_3AXIS \
Signature · 1 line
2.47.1
Re: [PATCH 18/20] iio: imu: inv_icm42600: switch timestamp type from int64_t __aligned(8) to aligned_s64
Hello,
Thanks,
JB
________________________________________
From: Jonathan Cameron <[email protected]>
Sent: Sunday, December 15, 2024 19:29
To: [email protected] <[email protected]>; Andy Shevchenko <[email protected]> Cc: David Lechner <[email protected]>; Heiko Stuebner <[email protected]>; Jonathan Cameron <[email protected]> Subject: [PATCH 18/20] iio: imu: inv_icm42600: switch timestamp type from int64_t __aligned(8) to aligned_s64
This Message Is From an External Sender
This message came from outside your organization.
From: Jonathan Cameron <[email protected]>
The vast majority of IIO drivers use aligned_s64 for the type of the timestamp field. It is not a bug to use int64_t and until this series iio_push_to_buffers_with_timestamp() took and int64_t timestamp, it is inconsistent. This change is to remove that inconsistency and ensure there is one obvious choice for future drivers.
--- drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 2 +- drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c +1 -1
@@ -178,7 +178,7 @@ static const struct iio_chan_spec inv_icm42600_accel_channels[] = {struct inv_icm42600_accel_buffer {struct inv_icm42600_fifo_sensor_data accel;int16_t temp; -int64_t timestamp__aligned(8) ; +aligned_s64 timestamp; };#define INV_ICM42600_SCAN_MASK_ACCEL_3AXIS \
drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c +1 -1
@@ -78,7 +78,7 @@ static const struct iio_chan_spec inv_icm42600_gyro_channels[] = {struct inv_icm42600_gyro_buffer {struct inv_icm42600_fifo_sensor_data gyro;int16_t temp; -int64_t timestamp__aligned(8) ; +aligned_s64 timestamp; };#define INV_ICM42600_SCAN_MASK_GYRO_3AXIS \
Signature · 1 line
2.47.1
Jonathan Cameron
maintainerPATCH 19/20 iio: adc: mt6360: Correct marking of timestamp alignment.
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
This is sort of a fix as I assume this device only exists on architectures that do 8 byte alignment of 8 byte types. However, drivers should not be written with that assumption and should allow for the 4 byte alignment of these types as seen for example on x86_32. As such, we need to force the alignment of the timestamp field itself not the containing structure, which will also be aligned appropriate given C structure alignment guarantees alignment to at least the alignment of structure member that needs highest alignment.
This is even more in the 'sort of' category because if we did have insufficient alignment of the timestamp, then marking the structure __aligned(8) would result in padding after the timestamp field. iio_push_to_buffers_with_timestamp() doesn't actually use the field, but instead where it is expected to be so will place the timestamp in the last 8 bytes, whether or not the structure definition puts it there. Hence it all works before the change. The reasoning is too subtle, just mark the right element aligned and use the new aligned_s64 to do so also clearing up the inconsistency of an int64_t timestamp relative to most other IIO drivers.
--- drivers/iio/adc/mt6360-adc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/iio/adc/mt6360-adc.c +2 -2
@@ -263,8 +263,8 @@ static irqreturn_t mt6360_adc_trigger_handler(int irq, void *p)struct mt6360_adc_data *mad =iio_priv (indio_dev);struct {u16 values[MT6360_CHAN_MAX ]; -int64_t timestamp; - } data__aligned(8) ; +aligned_s64 timestamp; + } data;int i =0 , bit, val, ret;memset (&data,0 ,sizeof (data));
Signature · 1 line
2.47.1
Jonathan Cameron
maintainerPATCH 20/20 iio: adc: rockchip: correct alignment of timestamp
In reply to Jonathan CameronFrom: Jonathan Cameron <[email protected]>
I assume this device is only used on architectures where a 8 byte integer type is always 8 byte aligned. However, I would prefer IIO drivers to never make that assumption as the code gets copied into new drivers which are not so tightly couple to one driver and those can run on architectures that align these types to only 4 bytes in which case this structure may be 4 byte to small leading to a buffer overrun.
--- drivers/iio/adc/rockchip_saradc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/adc/rockchip_saradc.c +1 -1
@@ -363,7 +363,7 @@ static irqreturn_t rockchip_saradc_trigger_handler(int irq, void *p)*/ struct {u16 values[SARADC_MAX_CHANNELS ]; -int64_t timestamp; +aligned_s64 timestamp; } data;int ret;int i, j =0 ;
Signature · 1 line
2.47.1
Quoted 27 lines · Jonathan Cameron
From: Jonathan Cameron <[email protected]>
I haven't +CC everyone relevant as that would be too long a list and these are mostly entirely mechanical changes that just need someone to sanity check I didn't do anything stupid.
This combines several related types of change:
- Some 'fixes'. These aren't actual bugs but more places where the code relies on factors it shouldn't such as what architecture the driver is running on, or padding of the wrong element happening to always make enough room.
- Now we havealigned_s64, use that to replace all the remaining s64 timestamp__aligned(8)instances in IIO.
This is both cleaner and hopefully less prone to confusing static analysis tools.
- Change the timestamp type iniio_push_to_buffers_with_timestamp()to s64. This is an entirely in kernel interface. It's data is pushed to user space in many case but by a rather indirect route, so I think it is more appropriate to switch this to s64 which is the type of almost all the values passed into it. Note a follow up series may tidy up some local variables that are of theint64_ttype.
- Change the fewint64_ttimestamp__aligned(8)cases toaligned_s64. This is mostly a consistency thing to avoid confusion if a new author is trying to figure out what types they should use. Better to have one answer than a mixture.
Quoted 3 lines
There is one other IIO driver hiding over in media that will be modified by a separate patch because I want to add some more info to that and a more specific CC list.
I briefly looked at most of the patches and haven't noticed any potential issues, all seems like a straightforward conversion. Hence
for all, except 15 and 19.
Patch 15 has got a few minor comments (and to accent on that I'm not sure about my offer for sizeof(), using direct variable might make it inconsistent in this case, because the following line uses explicit type instead of typeof(), and using the latter makes the code unreadable).
Patch 19 seems correct to me, but I'm not going to give a tag to be on the safe side.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Jonathan Cameron
maintainerQuoted 38 lines · On Sun, 15 Dec 2024 23:18:14 +0200 Andy Shevchenko…
Quoted 27 lines · Jonathan Cameron
From: Jonathan Cameron <[email protected]>
I haven't +CC everyone relevant as that would be too long a list and these are mostly entirely mechanical changes that just need someone to sanity check I didn't do anything stupid.
This combines several related types of change:
- Some 'fixes'. These aren't actual bugs but more places where the code relies on factors it shouldn't such as what architecture the driver is running on, or padding of the wrong element happening to always make enough room.
- Now we havealigned_s64, use that to replace all the remaining s64 timestamp__aligned(8)instances in IIO.
This is both cleaner and hopefully less prone to confusing static analysis tools.
- Change the timestamp type iniio_push_to_buffers_with_timestamp()to s64. This is an entirely in kernel interface. It's data is pushed to user space in many case but by a rather indirect route, so I think it is more appropriate to switch this to s64 which is the type of almost all the values passed into it. Note a follow up series may tidy up some local variables that are of theint64_ttype.
- Change the fewint64_ttimestamp__aligned(8)cases toaligned_s64. This is mostly a consistency thing to avoid confusion if a new author is trying to figure out what types they should use. Better to have one answer than a mixture.Quoted 3 lines
There is one other IIO driver hiding over in media that will be modified by a separate patch because I want to add some more info to that and a more specific CC list.
I briefly looked at most of the patches and haven't noticed any potential issues, all seems like a straightforward conversion. Hence
Reviewed-by Andy Shevchenko <[email protected]>for all, except 15 and 19.
Quoted 7 lines
Patch 15 has got a few minor comments (and to accent on that I'm not sure about my offer for
sizeof(), using direct variable might make it inconsistent in this case, because the following line uses explicit type instead oftypeof(), and using the latter makes the code unreadable).Patch 19 seems correct to me, but I'm not going to give a tag to be on the safe side.
Thanks, I've applied all but 15 and 19 (which could do with more eyes and will be more obvious in a v2 with just two patches!)
Will tidy up 15 and send out a v2 shortly.
Thanks,
Jonathan