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

[Jonathan Cameron <jic23@kernel.org>]

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

> > -----Original Message-----
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Saturday, March 7, 2026 1:05 PM
> > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>
> > Cc: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael
> > <Michael.Hennerich@analog.com>; Schmitt, Marcelo
> > <Marcelo.Schmitt@analog.com>; Sa, Nuno <Nuno.Sa@analog.com>; David
> > Lechner <dlechner@baylibre.com>; Andy Shevchenko <andy@kernel.org>;
> > Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>;
> > Conor Dooley <conor+dt@kernel.org>; Petre Rodan
> > <petre.rodan@subdimension.ro>; Marques, Jorge
> > <Jorge.Marques@analog.com>; linux-iio@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> > 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@analog.com> 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@analog.com>  
> > 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
>