ROHM IIO fixes
v3 cover ROHM IIO fixes
Fix a few issues from IIO drivers for ROHM components.
I hired couple of gnomes to work for me. :) (Ran AI reviews). Unsurprizingly some bugs were spotted. Time to try fix mess (mostly) I have authored.
Bugs were found by AI review but fixes are made by hand. Hence no AI-tooling tags are added. Please, let me know if one is needed.
Revision history:
v2 => v3:
- Drop already applied patch (1/4 in v2)
- patch 1/3: Change integration time getter signature as suggested by Jonathan.
- patch 2/3: Improve unwinding goto label naming
v1 => v2:
- Drop already applied patches (except the 1/4 which is in testing)
- all: Fixes tag before SOB
- patch 2/4: Clarify units for minimum integration time
- patch 3/4: Fix the unwinding of fifo enable
- patch 4/4: Fix the broken commit message
---
NOTE: Only _very_ shallow testing is done. All reviewing and testing is appreciated as usual!
Matti Vaittinen (3):
iio: light: rohm-bu27034: Fix infinite delay on error
iio: accel: kionix-kx022a: Prevent memory leak and fix state
iio: accel: kionix-kx022a: Fix IPOL macro name
drivers/iio/accel/kionix-kx022a.c | 28 ++++++++++++--- drivers/iio/accel/kionix-kx022a.h | 2 +- drivers/iio/light/rohm-bu27034.c | 60 ++++++++++++++++++------------- 3 files changed, 60 insertions(+), 30 deletions(-)
base-commit: 183f05a300eab41e4578337eac59335730dfebf9
Signature · 1 line
2.55.0
PATCH v3 1/3 iio: light: rohm-bu27034: Fix infinite delay on error
From: Matti Vaittinen <[email protected]>
When reading an integration-time fails, the code will use error code to compute the sleep time.
Fix this by using the smallest integration time as a default if reading fails.
---
I am not happy how intrusive this patch is for a fix. I really believe what I suggested in discussion:
wait_ms = bu27034_get_int_time(data) / USEC_PER_MSEC; if (wait_ms < BU27034_INT_TIME_MIN_MS)
wait_ms = BU27034_INT_TIME_MIN_MS;
https://lore.kernel.org/all/[email protected]/
would be better as a fix. Well, if this is what reviewers prefer, then I can live with it.
Revision history:
v2 => v3:
- Changed the signature of the measurement time getter as suggested by Jonathan.
v1 => v2:
- Moved Fixes before SOB
- Clarified units for the smallest integration time as suggested by Andy
--- drivers/iio/light/rohm-bu27034.c | 60 +++++++++++++++++++------------- 1 file changed, 36 insertions(+), 24 deletions(-)
drivers/iio/light/rohm-bu27034.c +36 -24
@@ -137,6 +137,7 @@ static const struct iio_gain_sel_pair bu27034_gains[] = {#define BU27034_MEAS_MODE_200MS 2#define BU27034_MEAS_MODE_400MS 4 +#define BU27034_INT_TIME_US_MIN (55 * USEC_PER_MSEC)static const struct iio_itime_sel_mul bu27034_itimes[] = {GAIN_SCALE_ITIME_US (400000 ,BU27034_MEAS_MODE_400MS ,8 ),GAIN_SCALE_ITIME_US (200000 ,BU27034_MEAS_MODE_200MS ,4 ),
@@ -296,7 +297,7 @@ static int bu27034_get_gain(struct bu27034_data *data, int chan, int *gain)return 0 ; } -static int bu27034_get_int_time (struct bu27034_data *data) +static int bu27034_get_int_time (struct bu27034_data *data,int *itime) {int ret, sel;
@@ -304,24 +305,30 @@ static int bu27034_get_int_time(struct bu27034_data *data)if (ret)return ret; -return iio_gts_find_int_time_by_sel (&data->gts, - sel &BU27034_MASK_MEAS_MODE ); + ret =iio_gts_find_int_time_by_sel (&data->gts, + sel &BU27034_MASK_MEAS_MODE ); +if (ret <0 ) +return ret; + + *itime = ret; + +return 0 ; }static int _bu27034_get_scale (struct bu27034_data *data,int channel,int *val,int *val2) { -int gain, ret; +int gain, itime, ret; ret =bu27034_get_gain (data, channel, &gain);if (ret)return ret; - ret =bu27034_get_int_time (data); -if (ret <0 ) + ret =bu27034_get_int_time (data, &itime); +if (ret)return ret; -return iio_gts_get_scale (&data->gts, gain, ret, val, val2); +return iio_gts_get_scale (&data->gts, gain, itime, val, val2); } static int bu27034_get_scale(struct bu27034_data *data, int channel, int *val,
@@ -397,12 +404,10 @@ static int bu27034_try_set_int_time(struct bu27034_data *data, int time_us)int ret, int_time_old, i;guard (mutex)(&data->mutex); - ret =bu27034_get_int_time (data); -if (ret <0 ) + ret =bu27034_get_int_time (data, &int_time_old); +if (ret)return ret; - int_time_old = ret; -if (!iio_gts_valid_time (&data->gts, time_us)) {dev_err (data->dev,"Unsupported integration time %u\n" , time_us);
@@ -841,7 +846,7 @@ static int bu27034_meas_set(struct bu27034_data *data, bool en)static int bu27034_get_single_result (struct bu27034_data *data,int chan,int *val) { -int ret; +int ret, itime;if (chan <BU27034_CHAN_DATA0 || chan >BU27034_CHAN_DATA1 )return -EINVAL ;
@@ -850,11 +855,11 @@ static int bu27034_get_single_result(struct bu27034_data *data, int chan,if (ret)return ret; - ret =bu27034_get_int_time (data); -if (ret <0 ) + ret =bu27034_get_int_time (data, &itime); +if (ret)return ret; -msleep (ret /1000 ); +msleep (itime /1000 );return bu27034_read_result (data, chan, val); }
@@ -904,12 +909,10 @@ static int bu27034_calc_mlux(struct bu27034_data *data, __le16 *res, int *val)if (ret)return ret; - ret =bu27034_get_int_time (data); -if (ret <0 ) + ret =bu27034_get_int_time (data, &meastime); +if (ret)return ret; - meastime = ret; - d1_d0_ratio_scaled = (unsigned int )ch1 * (unsigned int )gain0 *100 ; helper64 = (u64 )ch1 * (u64 )gain0 *100LLU ;
@@ -970,9 +973,9 @@ static int bu27034_read_raw(struct iio_dev *idev,switch (mask) {case IIO_CHAN_INFO_INT_TIME : *val =0 ; - *val2 =bu27034_get_int_time (data); -if (*val2 <0 ) -return *val2; + ret =bu27034_get_int_time (data, val2); +if (ret) +return ret;return IIO_VAL_INT_PLUS_MICRO ;
@@ -1157,11 +1160,20 @@ static int bu27034_buffer_thread(void *arg) {struct iio_dev *idev = arg;struct bu27034_data *data; -int wait_ms; +int wait_ms, ret; data =iio_priv (idev); - wait_ms =bu27034_get_int_time (data); +/* +* If reading the integration time fails, default to the minimum so we +* don't lose samples. This may waste CPU cycles, but as a hardening +* against theoretical, once-in-a-blue-moon error, this should be Ok. +*/ + wait_ms =BU27034_INT_TIME_US_MIN ; + ret =bu27034_get_int_time (data, &wait_ms); +if (ret) +dev_warn (data->dev,"Failed to get integration time\n" ); + wait_ms /=1000 ; wait_ms -=BU27034_MEAS_WAIT_PREMATURE_MS ;
Signature · 1 line
2.55.0
Jonathan Cameron
maintainerRe: [PATCH v3 1/3] iio: light: rohm-bu27034: Fix infinite delay on error
Quoted 12 lines · On Wed, 2 Sep 2026 11:48:46 +0300 Matti Vaittinen <[email protected]>…
From: Matti Vaittinen <[email protected]>
When reading an integration-time fails, the code will use error code to compute the sleep time.
Fix this by using the smallest integration time as a default if reading fails.
Fixes e52afbd61039 ("iio: light: ROHM BU27034 Ambient Light Sensor")Suggested-by Jonathan Cameron <[email protected]>Signed-off-by Matti Vaittinen <[email protected]>
Quoted 8 lines
---
I am not happy how intrusive this patch is for a fix. I really believe what I suggested in discussion:
wait_ms=bu27034_get_int_time(data)/USEC_PER_MSEC; if (wait_ms<BU27034_INT_TIME_MIN_MS)wait_ms = BU27034_INT_TIME_MIN_MS;
I really dislike messing with error codes like that division is doing so I do prefer this though I get that it is a bit noisy.
So applied to the fixes-togreg branch of iio.git and marked for stable.
Thanks,
Jonathan
PATCH v3 2/3 iio: accel: kionix-kx022a: Prevent memory leak and fix state
In reply to Matti VaittinenFrom: Matti Vaittinen <[email protected]>
The driver allocates memory for samples at buffer enable path. If regmap operation fails in the kx022a_fifo_enable() at the buffer enable path, the allocated memory is never freed. Furthermore, the state information and previous hardware configuration(s) aren't undone, potentially leaving WMI interrupts and buffers enabled, or driver state flags wrong.
Free the memory and revert the hardware configuration and state flags on error path.
---
Revision history:
v2 => v3:
- Make all of the fifo_enable unwinding goto labels describe what clean-up action is to be taken.
v1 => v2:
- Fix unwinding the fifo enabling
- Move Fixes before the SOB.
--- drivers/iio/accel/kionix-kx022a.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-)
drivers/iio/accel/kionix-kx022a.c +23 -5
@@ -981,26 +981,44 @@ static int kx022a_fifo_enable(struct kx022a_data *data)guard (mutex)(&data->mutex); ret =__kx022a_turn_on_off (data, false);if (ret) -return ret; + goto err_free_out;/* Update watermark to HW */ ret =kx022a_fifo_set_wmi (data);if (ret) -return ret; + goto err_turn_on_out;/* Enable buffer */ ret =regmap_set_bits (data->regmap, data->chip_info->buf_cntl2,KX022A_MASK_BUF_EN );if (ret) -return ret; + goto err_turn_on_out; data->state |=KX022A_STATE_FIFO ; ret =regmap_set_bits (data->regmap, data->ien_reg,KX022A_MASK_WMI );if (ret) -return ret; + goto err_buf_disable_out; -return __kx022a_turn_on_off (data, true); + ret =__kx022a_turn_on_off (data, true); +if (ret) + goto err_wmi_clear_out; + +return ret; + +err_wmi_clear_out: +regmap_clear_bits (data->regmap, data->ien_reg, +KX022A_MASK_WMI ); +err_buf_disable_out: +regmap_clear_bits (data->regmap, data->chip_info->buf_cntl2, +KX022A_MASK_BUF_EN ); + data->state &= ~KX022A_STATE_FIFO ; +err_turn_on_out: +__kx022a_turn_on_off (data, true); +err_free_out: +kfree (data->fifo_buffer); + +return ret; }static int kx022a_buffer_postenable (struct iio_dev *idev)
Signature · 1 line
2.55.0
Jonathan Cameron
maintainerRe: [PATCH v3 2/3] iio: accel: kionix-kx022a: Prevent memory leak and fix state
Quoted 8 lines · On Wed, 2 Sep 2026 11:49:05 +0300 Matti Vaittinen <[email protected]>…
From: Matti Vaittinen <[email protected]>
The driver allocates memory for samples at buffer enable path. If regmap operation fails in the
kx022a_fifo_enable()at the buffer enable path, the allocated memory is never freed. Furthermore, the state information and previous hardware configuration(s) aren't undone, potentially leaving WMI interrupts and buffers enabled, or driver state flags wrong.
Quoted 6 lines
Free the memory and revert the hardware configuration and state flags on error path.
Fixes e7123a4dfcd7 ("iio: accel: kionix-kx022a: Refactor driver and add chip_info structure")Signed-off-by Matti Vaittinen <[email protected]>Reviewed-by Mehdi Djait <[email protected]>
Applied and marked for stable.
PATCH v3 3/3 iio: accel: kionix-kx022a: Fix IPOL macro name
In reply to Matti VaittinenFrom: Matti Vaittinen <[email protected]>
The "interrupt polarity high"-macro for KX022A variant is defined as:
"#define KX022A_MASK_IPOL KX022A_MASK_IPOL1"
However, the KX022A_MASK_IPOL1 is not defined anywhere, so actually using the KX022A_IPOL_HIGH would produce a compile error.
Fix the define by using correct mask.
---
Revision history:
v2 =>:
- No changes
v1 => v2:
- Fixed the commit message where line: "#define KX022A_MASK_IPOL KX022A_MASK_IPOL1" was lost, as git treated it as a comment :)
- Moved Fixes before SOB
--- drivers/iio/accel/kionix-kx022a.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/accel/kionix-kx022a.h +1 -1
@@ -65,7 +65,7 @@#define KX022A_MASK_IEN BIT(5)#define KX022A_MASK_IPOL BIT(4)#define KX022A_IPOL_LOW 0 -#define KX022A_IPOL_HIGH KX022A_MASK_IPOL1 +#define KX022A_IPOL_HIGH KX022A_MASK_IPOL#define KX022A_MASK_ITYP BIT(3)#define KX022A_ITYP_PULSE KX022A_MASK_ITYP#define KX022A_ITYP_LEVEL 0
Signature · 1 line
2.55.0
Jonathan Cameron
maintainerRe: [PATCH v3 3/3] iio: accel: kionix-kx022a: Fix IPOL macro name
Quoted 5 lines · On Wed, 2 Sep 2026 11:49:18 +0300 Matti Vaittinen <[email protected]>…
From: Matti Vaittinen <[email protected]>
The "interrupt polarity high"-macro for KX022A variant is defined as:
"#defineKX022A_MASK_IPOLKX022A_MASK_IPOL1"
Quoted 8 lines
However, the
KX022A_MASK_IPOL1is not defined anywhere, so actually using theKX022A_IPOL_HIGHwould produce a compile error.Fix the define by using correct mask.
Fixes 7c1d1677b322 ("iio: accel: Support Kionix/ROHM KX022A accelerometer")Signed-off-by Matti Vaittinen <[email protected]>Reviewed-by Mehdi Djait <[email protected]>
This one can go the slow way. Applied to the testing branch of iio.git. Thanks,
Jonathan