Re: [PATCH] iio: accel: adxl345: update period cache after successful write

[Nuno Sá]

On Sun, Oct 11, 2026 at 06:18:11AM -0400, 赵文强 wrote:
> The ADXL345 event-period read path returns st->inact_time_ms. The setters
> for inactivity, free-fall and the ODR-derived default currently update
> this cache before writing TIME_INACT or TIME_FF. If the regmap write
> fails, the sysfs write reports an error but a subsequent read returns
> the requested period even though it was not programmed.
> 
> Update the cached period only after the corresponding regmap write
> succeeds.
> 
> Fixes: 8a1536cc4a1c ("iio: accel: adxl345: add inactivity feature")
> Fixes: f6f22950d925 ("iio: accel: adxl345: extend inactivity time for
> less than 1s")

No line break

> Assisted-by: LLM
> Signed-off-by: 赵文强 <berringtoni36@xxxxxxxxx>

Not sure about the above non rendered characters on the name... Would
guess it's not ok.

> ---
> Validation:
> - A host harness extracted the production setter and period-read helpers
>   and injected -EIO at regmap_write(). The baseline failed all three
>   failure cases (3/6 passed); the patch passed all six cases, including
>   successful writes.
> - Compiled the full source to an x86_64 object with Apple clang 17 and
>   Debian Linux 6.1 headers plus upstream IIO/regmap headers and a cleanup
>   compatibility shim. This checks syntax and types with a mixed header
>   set; it is not a full current-tree Kbuild/modpost or target-board build.
> - No in-kernel KUnit or hardware test was performed.
> 
> AI assistance: Codex drafted the fix, changelog and fault-injection
> harness for a contribution related to an ADXL345 SPI/IIO project.

Seems like a lot of trouble for a simple fix :). The change makes sense but
incomplete. Looks to me that the driver is completely not safe in terms
of multiple userspace callers. Arguably a different issue though but
seems that a mutex is definitely needed.

- Nuno Sá

> 
>  drivers/iio/accel/adxl345_core.c | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 6c9080d..cd58304 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -391,30 +391,47 @@ static int adxl345_set_default_time(struct
> adxl345_state *st)
>  	odr = FIELD_GET(ADXL345_BW_RATE_MSK, regval);
>  	val = clamp(max_boundary - adxl345_odr_tbl[odr][0],
>  		    min_boundary, max_boundary);
> -	st->inact_time_ms = MILLI * val;
> 
>  	/* Inactivity time in s */
> -	return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val);
> +	ret = regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val);
> +	if (ret)
> +		return ret;
> +
> +	st->inact_time_ms = MILLI * val;
> +
> +	return 0;
>  }
> 
>  static int adxl345_set_inactivity_time(struct adxl345_state *st, u32 val_int)
>  {
> +	int ret;
> +
> +	ret = regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val_int);
> +	if (ret)
> +		return ret;
> +
>  	st->inact_time_ms = MILLI * val_int;
> 
> -	return regmap_write(st->regmap, ADXL345_REG_TIME_INACT, val_int);
> +	return 0;
>  }
> 
>  static int adxl345_set_freefall_time(struct adxl345_state *st, u32 val_fract)
>  {
> +	int ret;
> +
>  	/*
>  	 * Datasheet max. value is 255 * 5000 us = 1.275000 seconds.
>  	 *
>  	 * Recommended values between 100ms and 350ms (0x14 to 0x46)
>  	 */
> +	ret = regmap_write(st->regmap, ADXL345_REG_TIME_FF,
> +			   DIV_ROUND_CLOSEST(val_fract, 5));
> +	if (ret)
> +		return ret;
> +
>  	st->inact_time_ms = DIV_ROUND_UP(val_fract, MILLI);
> 
> -	return regmap_write(st->regmap, ADXL345_REG_TIME_FF,
> -			    DIV_ROUND_CLOSEST(val_fract, 5));
> +	return 0;
>  }
> 
>  /**
> base-commit: 9ee8306121495d2a25aa5d1bfd519f2748786b83
> -- 
> 2.50.1 (Apple Git-155)