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

[Jonathan Cameron]

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