Re: [PATCH v2 4/4] iio: accel: adxl372: add support for ADXL371

[Jonathan Cameron]

On Mon, 9 Mar 2026 13:08:26 +0000
"Miclaus, Antoniu" <Antoniu.Miclaus@xxxxxxxxxx> wrote:

> > -----Original Message-----
> > From: Jonathan Cameron <jic23@xxxxxxxxxx>
> > Sent: Saturday, March 7, 2026 1:05 PM
> > To: Miclaus, Antoniu <Antoniu.Miclaus@xxxxxxxxxx>
> > Cc: Lars-Peter Clausen <lars@xxxxxxxxxx>; Hennerich, Michael
> > <Michael.Hennerich@xxxxxxxxxx>; Schmitt, Marcelo
> > <Marcelo.Schmitt@xxxxxxxxxx>; Sa, Nuno <Nuno.Sa@xxxxxxxxxx>; David
> > Lechner <dlechner@xxxxxxxxxxxx>; Andy Shevchenko <andy@xxxxxxxxxx>;
> > Rob Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>;
> > Conor Dooley <conor+dt@xxxxxxxxxx>; Petre Rodan
> > <petre.rodan@xxxxxxxxxxxxxxx>; Marques, Jorge
> > <Jorge.Marques@xxxxxxxxxx>; linux-iio@xxxxxxxxxxxxxxx;
> > devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> > Subject: Re: [PATCH v2 4/4] iio: accel: adxl372: add support for ADXL371
> > 
> > [External]
> > 
> > On Fri, 6 Mar 2026 17:18:24 +0200
> > Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx> wrote:
> >   
> > > Add support for the Analog Devices ADXL371, a +-200g 3-axis MEMS
> > > accelerometer sharing the same register map as the ADXL372 but with
> > > different ODR values (320/640/1280/2560/5120 Hz vs  
> > 400/800/1600/3200/  
> > > 6400 Hz), different bandwidth values, and different timer scale
> > > factors for activity/inactivity detection.
> > >
> > > Due to a silicon anomaly (er001) causing FIFO data misalignment on
> > > all current ADXL371 silicon, FIFO and triggered buffer support is
> > > disabled for the ADXL371 - only direct mode reads are supported.
> > >
> > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx>  
> > A couple of small formatting things. Otherwise looks good to me.
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> > >
> > > diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> > > index adb9e42653f1..7a1ee2fef618 100644
> > > --- a/drivers/iio/accel/adxl372.c
> > > +++ b/drivers/iio/accel/adxl372.c
> > > @@ -1,6 +1,6 @@
> > >  // SPDX-License-Identifier: GPL-2.0+
> > >  /*
> > > - * ADXL372 3-Axis Digital Accelerometer core driver
> > > + * ADXL371/ADXL372 3-Axis Digital Accelerometer core driver
> > >   *
> > >   * Copyright 2018 Analog Devices Inc.
> > >   */
> > > @@ -182,6 +182,14 @@ enum adxl372_odr {
> > >  	ADXL372_ODR_6400HZ,
> > >  };
> > >
> > > +enum adxl371_odr {
> > > +	ADXL371_ODR_320HZ,
> > > +	ADXL371_ODR_640HZ,
> > > +	ADXL371_ODR_1280HZ,
> > > +	ADXL371_ODR_2560HZ,
> > > +	ADXL371_ODR_5120HZ,  
> > Might be worth a
> > 	ADXL371_ODR_NUM
> > entry so you can size the array from it below.
> >   
> > > +};
> > > +
> > >  enum adxl372_bandwidth {
> > >  	ADXL372_BW_200HZ,
> > >  	ADXL372_BW_400HZ,
> > > @@ -222,6 +230,37 @@ static const int adxl372_bw_freq_tbl[5] = {
> > >  	200, 400, 800, 1600, 3200,
> > >  };
> > >
> > > +static const int adxl371_samp_freq_tbl[5] = {
> > > +	[ADXL371_ODR_320HZ] = 320,
> > > +	[ADXL371_ODR_640HZ] = 640,
> > > +	[ADXL371_ODR_1280HZ] = 1280,
> > > +	[ADXL371_ODR_2560HZ] = 2560,
> > > +	[ADXL371_ODR_5120HZ] = 5120,
> > > +};
> > > +
> > > +static const int adxl371_bw_freq_tbl[5] = {
> > > +	[ADXL371_ODR_320HZ] = 160,
> > > +	[ADXL371_ODR_640HZ] = 320,
> > > +	[ADXL371_ODR_1280HZ] = 640,
> > > +	[ADXL371_ODR_2560HZ] = 1280,
> > > +	[ADXL371_ODR_5120HZ] = 2560,
> > > +};  
> > Style wise, why not do the same for adxl372_bw_freq_tbl[] as here?
> > I slightly prefer this style, but key is consistency so if you'd
> > gone the other way for both that would have been fine as well.  
> 
> 
> I will do the same for adxl372_bw_freq_tbl[] as here.
> Where do you think is the best place to put that, in patch 1? (introduce chip_info structure)
Probably a precursor patch given the tables already exist.

Jonathan

> 
> Thanks
>