Re: [PATCH v2] iio: accel: fix ADXL355 startup race condition

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 15 Sep 2025 13:58:18 +0200
Andrej Valek <andrej.v@skyrain.eu> wrote:

> From: Valek Andrej <andrej.v@skyrain.eu>
> 
> There is an race-condition where device is not full working after SW reset.
> Therefore it's necessary to wait some time after reset and verify shadow
> registers values by reading and comparing the values before/after reset.
> This mechanism is described in datasheet at least from revision D.
> 
> Signed-off-by: Valek Andrej <andrej.v@skyrain.eu>
> Signed-off-by: Kessler Markus <markus.kessler@hilti.com>
> ---
There should be a change lot here.

As per the reply I just sent, even though today regmap doesn't need a dma
safe buffer for bulk reads that is undocumented detail and may not remain
true in the long run.  Hence use a dma safe buffer.

>  drivers/iio/accel/adxl355_core.c | 41 ++++++++++++++++++++++++++++----
>  1 file changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl355_core.c b/drivers/iio/accel/adxl355_core.c
> index 2e00fd51b4d51..d688841f374da 100644
> --- a/drivers/iio/accel/adxl355_core.c
> +++ b/drivers/iio/accel/adxl355_core.c
> @@ -56,6 +56,8 @@
>  #define  ADXL355_POWER_CTL_DRDY_MSK	BIT(2)
>  #define ADXL355_SELF_TEST_REG		0x2E
>  #define ADXL355_RESET_REG		0x2F
> +#define ADXL355_BASE_ADDR_SHADOW_REG	0x50
> +#define ADXL355_SHADOW_REG_COUNT	5
>  
>  #define ADXL355_DEVID_AD_VAL		0xAD
>  #define ADXL355_DEVID_MST_VAL		0x1D
> @@ -294,6 +296,8 @@ static void adxl355_fill_3db_frequency_table(struct adxl355_data *data)
>  static int adxl355_setup(struct adxl355_data *data)
>  {
>  	unsigned int regval;
> +	u8 shadow_regs[ADXL355_SHADOW_REG_COUNT];
As above. This needs to be DMA safe. If you want to keep it local use
	u8 *shadow_regs __free(kfree) = kzalloc(ADXL355_SHADOW_REG_COUNT, GFP_KERNEL);
or kcalloc if you prefer.  


> +	int retries = 5; /* the number is chosen based on empirical reasons */
>  	int ret;
>  
>  	ret = regmap_read(data->regmap, ADXL355_DEVID_AD_REG, &regval);
> @@ -321,14 +325,41 @@ static int adxl355_setup(struct adxl355_data *data)
>  	if (regval != ADXL355_PARTID_VAL)
>  		dev_warn(data->dev, "Invalid DEV ID 0x%02x\n", regval);
>  
> -	/*
> -	 * Perform a software reset to make sure the device is in a consistent
> -	 * state after start-up.
> -	 */
> -	ret = regmap_write(data->regmap, ADXL355_RESET_REG, ADXL355_RESET_CODE);
> +	/* Read shadow registers to be compared after reset */
> +	ret = regmap_bulk_read(data->regmap,
> +			       ADXL355_BASE_ADDR_SHADOW_REG,
> +			       shadow_regs, ADXL355_SHADOW_REG_COUNT);

Here is the non dma safe buffer usage.

>  	if (ret)
>  		return ret;
>  
> +	do {
> +		if (--retries == 0) {
> +			dev_err(data->dev, "Shadow registers mismatch\n");
> +			return -EIO;
> +		}
> +
> +		/*
> +		 * Perform a software reset to make sure the device is in a consistent
> +		 * state after start-up.
> +		 */
> +		ret = regmap_write(data->regmap, ADXL355_RESET_REG,
> +				   ADXL355_RESET_CODE);
> +		if (ret)
> +			return ret;
> +
> +		/* Wait at least 5ms after software reset */
> +		usleep_range(5000, 10000);
> +
> +		/* Read shadow registers for comparison */
> +		ret = regmap_bulk_read(data->regmap,
> +				       ADXL355_BASE_ADDR_SHADOW_REG,
> +				       data->buffer.buf,
> +				       ADXL355_SHADOW_REG_COUNT);
> +		if (ret)
> +			return ret;
> +	} while (memcmp(shadow_regs, data->buffer.buf,
> +			ADXL355_SHADOW_REG_COUNT));
> +
>  	ret = regmap_update_bits(data->regmap, ADXL355_POWER_CTL_REG,
>  				 ADXL355_POWER_CTL_DRDY_MSK,
>  				 FIELD_PREP(ADXL355_POWER_CTL_DRDY_MSK, 1));