Re: [PATCH v2 1/2] iio: accel: adxl345: propagate errors when reading event configuration

[Wenqiang Zhao]

> Cleanest solution is probably to pass a bool * in as a parameter and
> use that to 'return' the boolean rather than mixing up bool and error
> codes as it currently does.

Thanks, Jonathan. I have reworked both the enable-state and coupling
queries to return status separately from bool output parameters, and
updated the event-read, disable and link-bit callers. The 534 host
function-chain cases pass with UBSan; no full kernel build or hardware
test has been performed. I will send v3 after the final review.

Thanks,
Wenqiang


On Sat, 17 Oct 2026 19:13:10 +0100, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> On Fri, 16 Oct 2026 19:39:08 -0700
> 赵文强 <berringtoni36@xxxxxxxxx> wrote:
>
> > adxl345_is_act_inact_en() returns either a boolean state or a negative
>
> Returning true or false to a function that reports to return integers
> is rather nasty and is probably what lead to this bug in the first
> place!  Would you mind also changing that as part of this fix?
>
> Cleanest solution is probably to pass a bool * in as a parameter and
> use that to 'return' the boolean rather than mixing up bool and error
> codes as it currently does.  That will keep the return value for whether
> the query succeeded or not.
>
> Thanks,
>
> Jonathan
>
>
> > regmap error. Applying !! in adxl345_read_mag_config() converts every
> > negative error to 1, so a failed register read is reported to userspace
> > as an enabled activity or inactivity event.
> >
> > Return the helper result directly. Its successful paths already return
> > 0 or 1, and the IIO event attribute read path propagates negative errors.
> > This also covers the adaptive magnitude events using the same helper.
> >
> > Fixes: 66a1af6d8a63 ("iio: accel: adxl345: add activity event feature")
> > Assisted-by: LLM
> > Signed-off-by: 赵文强 <berringtoni36@xxxxxxxxx>
>
> > ---
> > Validation on macOS ARM64 with Apple clang 17:
> > - Compiled the three relevant driver functions verbatim with the driver
> >   header, using a host regmap stub for deterministic read failures.
> > - The original code failed 36 error cases; the patch passed all 53 cases.
> > - Covered activity/inactivity and DC/AC coupling, each of three regmap
> >   reads failing with -EIO, -ETIMEDOUT and -EREMOTEIO, enabled/disabled
> >   states, mismatched coupling and an unsupported direction.
> > - Checked read order, early exit and unchanged register state.
> > - Used -Wall -Wextra -Werror, AddressSanitizer and UndefinedBehaviorSanitizer.
> > - git diff --check and checkpatch.pl --no-tree --no-signoff --strict
> >   on the code diff passed with no errors, warnings or checks.
> > - The full patch draft also passed checkpatch with no errors, warnings
> >   or checks after fetching the introducing commit for Fixes validation.
> >
> > Also compiled adxl345_core.o, adxl345_spi.o and adxl345_i2c.o as
> > x86_64 ELF relocatable objects using clang and unpacked Debian Linux 6.1
> > headers supplemented by upstream IIO/regmap headers. This checks syntax
> > and types; it is not a full Kbuild/modpost build.
> >
> > No in-kernel KUnit execution or hardware sensor test was performed.
> > The checkout is sparse; --no-tree disables checkpatch's tree layout
> > check. Initial draft checks used --no-signoff before human review.
> > The final signed email also passed checkpatch without --no-signoff.
> >
> > AI assistance: Codex reviewed the event-read path, identified the errno
> > conversion, drafted this two-line fix, the reproducer and changelog.
> > The request was to prepare a driver/BSP/kernel contribution related to an
> > ADXL345 SPI/IIO vibration-monitoring project, with reproduction and checks.
> >
> >  drivers/iio/accel/adxl345_core.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> > index 6c9080d..5983fc7 100644
> > --- a/drivers/iio/accel/adxl345_core.c
> > +++ b/drivers/iio/accel/adxl345_core.c
> > @@ -1176,9 +1176,9 @@ static int adxl345_read_mag_config(struct
> > adxl345_state *st,
> >  {
> >  	switch (dir) {
> >  	case IIO_EV_DIR_RISING:
> > -		return !!adxl345_is_act_inact_en(st, type_act);
> > +		return adxl345_is_act_inact_en(st, type_act);
> >  	case IIO_EV_DIR_FALLING:
> > -		return !!adxl345_is_act_inact_en(st, type_inact);
> > +		return adxl345_is_act_inact_en(st, type_inact);
> >  	default:
> >  		return -EINVAL;
> >  	}
> >
> > base-commit: 9ee8306121495d2a25aa5d1bfd519f2748786b83