Re: [PATCH] drivers: iio: accel: fix ADX355 startup race condition

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Thu, 11 Sep 2025 11:33:49 +0200
Andrej Valek <andrej.v@skyrain.eu> wrote:

> Hi Jonathan,
> 
> First, I would like to thanks for your feedback.
> 
> On 10.09.2025 20:30, Jonathan Cameron wrote:
> > On Tue,  9 Sep 2025 10:55:28 +0200
> > Andrej Valek <andrej.v@skyrain.eu> wrote:
> >  
> >> From: Valek Andrej <andrej.v@skyrain.eu>  
> > Hi Valek,
> >
> > Thanks for the patch. Small thing on patch title, don't include drivers.
> > It's a pain but you need to look at other patches to a given subsystem
> > to find out the preferred style.  
> Valek is my surname 🙂.

Oops. Sorry! 

Hi Andrej,

> >> Signed-off-by: Valek Andrej <andrej.v@skyrain.eu>
> >> Signed-off-by: Kessler Markus <markus.kessler@hilti.com>
> >> ---
> >>   drivers/iio/accel/adxl355_core.c | 48 ++++++++++++++++++++++++++++----
> >>   1 file changed, 43 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/iio/accel/adxl355_core.c b/drivers/iio/accel/adxl355_core.c
> >> index 2e00fd51b4d51..5386cd4766def 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,9 @@ 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];  
> > Needs to be a DMA safe buffer.  We can't assume that regmap will always
> > bounce the data through one before passing it to the SPI controllers
> > that do sometimes require DMA safe buffers.    Add a buffer to end of
> > struct adxl355_data where you can take advantage of the forcing of appropriate
> > padding that is already going on there.  

> I see, but I don't like extending the adxl355_data just for one time 
> usage. The suggested approach is more similar to what is done with the 
> ID checking.
> | if (regval != ADXL355_DEVID_MST_VAL) {
> |  dev_err(data->dev, "Invalid MEMS ID 0x%02x\n", regval);
> |  return -ENODEV;
> | }

That's a single read.  Regmap always bounces those so a local variable should
be fine. Not so for a bulk read (or at least no one guarantees it)

https://events19.linuxfoundation.org/wp-content/uploads/2017/12/20181023-Wolfram-Sang-ELCE18-safe_dma_buffers.pdf
Slide 11 (in general this slide deck is a good introduction to the fun of DMA safety).

The path for a single read goes through:
https://elixir.bootlin.com/linux/v6.16.7/source/drivers/base/regmap/regmap.c#L2779
_regmap_bus_read() which uses map->work_buf which is DMA safe.

Jonathan