[PATCH v4] iio: accel: fix ADXL355 startup race condition

[PATCH v4] iio: accel: fix ADXL355 startup race condition

[Andrej Valek]

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>
---
 drivers/iio/accel/adxl355_core.c | 44 ++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/accel/adxl355_core.c b/drivers/iio/accel/adxl355_core.c
index 2e00fd51b4d5..5fc7f814b907 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,7 +296,12 @@ static void adxl355_fill_3db_frequency_table(struct adxl355_data *data)
 static int adxl355_setup(struct adxl355_data *data)
 {
 	unsigned int regval;
+	int retries = 5; /* the number is chosen based on empirical reasons */
 	int ret;
+	u8 *shadow_regs __free(kfree) = kzalloc(ADXL355_SHADOW_REG_COUNT, GFP_KERNEL);
+
+	if (!shadow_regs)
+		return -ENOMEM;
 
 	ret = regmap_read(data->regmap, ADXL355_DEVID_AD_REG, &regval);
 	if (ret)
@@ -321,14 +328,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);
 	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));
-- 
2.34.1

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

[Jonathan Cameron]

On Mon,  6 Oct 2025 11:58:12 +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>
See submitting-patches.rst in documentation for how to format it, but as
requested on previous version please reply to this thread with
fixes tag.  No need to send a new version.


> ---
Change log missing

Otherwise looks good to me.

Jonathan

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

[Andrej Valek]

Hello Jonathan,



On 12.10.2025 17:26, Jonathan Cameron wrote:
> On Mon,  6 Oct 2025 11:58:12 +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>
> See submitting-patches.rst in documentation for how to format it, but as
> requested on previous version please reply to this thread with
> fixes tag.  No need to send a new version.
I looked into this document, and to be honest didn't find any "issues" 
in my patch. I used checkpatch and get_maintainer scripts and passed 
without errors. So what's missing then?
>
>
>> ---
> Change log missing
What do you mean by this? I didn't find any special document for this. 
Or do you mean the commit message, if yes, what's missing?
>
> Otherwise looks good to me.
>
> Jonathan
>
>
BR,
Andy

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

[David Lechner]

On 10/13/25 2:07 AM, Andrej Valek wrote:
> Hello Jonathan,
> 
> 
> 
> On 12.10.2025 17:26, Jonathan Cameron wrote:
>> On Mon,  6 Oct 2025 11:58:12 +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>
>> See submitting-patches.rst in documentation for how to format it, but as
>> requested on previous version please reply to this thread with
>> fixes tag.  No need to send a new version.
> I looked into this document, and to be honest didn't find any "issues" in my patch. I used checkpatch and get_maintainer scripts and passed without errors. So what's missing then?

This patch fixes a bug, so it needs a Fixes: tag that describes
the commit that introduced the bug. This information is used to
automatically select the patch for backports to stable kernels
that have the same bug.

So Jonathan was just asking you to reply with the following line...

Fixes: 12ed27863ea3 ("iio: accel: Add driver support for ADXL355")

See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes



>>
>>
>>> ---
>> Change log missing
> What do you mean by this? I didn't find any special document for this. Or do you mean the commit message, if yes, what's missing?


Each time you submit a new revision of a patch/series it should include
a changelog of what was change compared to the previous revisions that
were submitted. And preferably a link to the previous submission as well.
Tools like b4 can automate parts of this for you.

See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#commentary

>>
>> Otherwise looks good to me.
>>
>> Jonathan
>>
>>
> BR,
> Andy

[PATCH v4] iio: accel: fix ADXL355 startup race condition

[Andrej Valek]

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.

Fixes: 12ed27863ea3 ("iio: accel: Add driver support for ADXL355")
Signed-off-by: Valek Andrej <andrej.v@skyrain.eu>
Signed-off-by: Kessler Markus <markus.kessler@hilti.com>
---
V3 -> V4: DMA-safe buffers are used finally
V2 -> V3: Second tryout to use DMA-save buffers
V1 -> V2: First tryout to use DMA-safe buffers after review comments
---
 drivers/iio/accel/adxl355_core.c | 44 ++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/accel/adxl355_core.c b/drivers/iio/accel/adxl355_core.c
index 2e00fd51b4d5..5fc7f814b907 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,7 +296,12 @@ static void adxl355_fill_3db_frequency_table(struct adxl355_data *data)
 static int adxl355_setup(struct adxl355_data *data)
 {
 	unsigned int regval;
+	int retries = 5; /* the number is chosen based on empirical reasons */
 	int ret;
+	u8 *shadow_regs __free(kfree) = kzalloc(ADXL355_SHADOW_REG_COUNT, GFP_KERNEL);
+
+	if (!shadow_regs)
+		return -ENOMEM;
 
 	ret = regmap_read(data->regmap, ADXL355_DEVID_AD_REG, &regval);
 	if (ret)
@@ -321,14 +328,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);
 	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));
-- 
2.34.1

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

[Jonathan Cameron]

On Tue, 14 Oct 2025 09:13:44 +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.
> 
> Fixes: 12ed27863ea3 ("iio: accel: Add driver support for ADXL355")
> Signed-off-by: Valek Andrej <andrej.v@skyrain.eu>
> Signed-off-by: Kessler Markus <markus.kessler@hilti.com>
It doesn't hugely matter but I was only asking for a reply with the single
line David sent. Not a new posting of the driver.

Definitely don't ever send two v4 with any changes at all as that makes
it uncertain what automation will pick up.  Standard b4 string to grab this
patch grabs me two patches with two patches as it can't figure out they are
the same.

Anyhow, applied carefully by hand to get just this one and marked for
stable inclusion.

Thanks,

Jonathan

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

[Andrej Valek]

Hello Jonathan,

On 18.10.2025 15:08, Jonathan Cameron wrote:
> On Tue, 14 Oct 2025 09:13:44 +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.
>>
>> Fixes: 12ed27863ea3 ("iio: accel: Add driver support for ADXL355")
>> Signed-off-by: Valek Andrej <andrej.v@skyrain.eu>
>> Signed-off-by: Kessler Markus <markus.kessler@hilti.com>
> It doesn't hugely matter but I was only asking for a reply with the single
> line David sent. Not a new posting of the driver.
Ah ok, I misunderstood it then.
>
> Definitely don't ever send two v4 with any changes at all as that makes
> it uncertain what automation will pick up.  Standard b4 string to grab this
> patch grabs me two patches with two patches as it can't figure out they are
> the same.
Sorry about that.
>
> Anyhow, applied carefully by hand to get just this one and marked for
> stable inclusion.
>
> Thanks,
>
> Jonathan
>
Ok, good to know and thanks for the guiding. Will try to do it better in 
the future.

Btw, you swapped my name and surname again, at least in the commit 
(author) :).

BR,
Andy

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

[Jonathan Cameron]

On Sun, 19 Oct 2025 12:45:27 +0200
Andrej Valek <andrej.v@skyrain.eu> wrote:

> Hello Jonathan,
> 
> On 18.10.2025 15:08, Jonathan Cameron wrote:
> > On Tue, 14 Oct 2025 09:13:44 +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.
> >>
> >> Fixes: 12ed27863ea3 ("iio: accel: Add driver support for ADXL355")
> >> Signed-off-by: Valek Andrej <andrej.v@skyrain.eu>
> >> Signed-off-by: Kessler Markus <markus.kessler@hilti.com>  
> > It doesn't hugely matter but I was only asking for a reply with the single
> > line David sent. Not a new posting of the driver.  
> Ah ok, I misunderstood it then.
> >
> > Definitely don't ever send two v4 with any changes at all as that makes
> > it uncertain what automation will pick up.  Standard b4 string to grab this
> > patch grabs me two patches with two patches as it can't figure out they are
> > the same.  
> Sorry about that.
> >
> > Anyhow, applied carefully by hand to get just this one and marked for
> > stable inclusion.
> >
> > Thanks,
> >
> > Jonathan
> >  
> Ok, good to know and thanks for the guiding. Will try to do it better in 
> the future.
> 
> Btw, you swapped my name and surname again, at least in the commit 
> (author) :).

I'm confused.  Your
From: Valek Andrej <andrej.v@skyrain.eu>
line is at at the top of the patch.

If you have one of those it will ignore whatever email address the mail came from.
https://lore.kernel.org/all/20251014071344.151914-1-andrej.v@skyrain.eu/

So I think this is a git config issue your end if you'd prefer it listed as
Andrej Valek.

Jonathan

> 
> BR,
> Andy