On Sat, 28 Dec 2024 17:11:58 +0100 Lothar Rubusch <l.rubusch@xxxxxxxxx> wrote: > On Sat, Dec 28, 2024 at 3:45 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > > > On Wed, 25 Dec 2024 18:13:37 +0000 > > Lothar Rubusch <l.rubusch@xxxxxxxxx> wrote: > > > > > Add a basic setup for FIFO with configurable watermark. Add a handler > > > for watermark interrupt events and extend the channel for the > > > scan_index needed for the iio channel. The sensor is configurable to use > > > a FIFO_BYPASSED mode or a FIFO_STREAM mode. For the FIFO_STREAM mode now > > > a watermark can be configured, or disabled by setting 0. Further features > > > require a working FIFO setup. > > > > > > Signed-off-by: Lothar Rubusch <l.rubusch@xxxxxxxxx> > > > --- > > > drivers/iio/accel/adxl345.h | 27 ++- > > > drivers/iio/accel/adxl345_core.c | 305 ++++++++++++++++++++++++++++++- > > > 2 files changed, 321 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/iio/accel/adxl345.h b/drivers/iio/accel/adxl345.h > > > index 6f39f16d3..bf9e86cff 100644 > > > --- a/drivers/iio/accel/adxl345.h > > > +++ b/drivers/iio/accel/adxl345.h > > > @@ -15,18 +15,32 @@ > > > #define ADXL345_REG_OFS_AXIS(index) (ADXL345_REG_OFSX + (index)) > > > #define ADXL345_REG_BW_RATE 0x2C > > > #define ADXL345_REG_POWER_CTL 0x2D > > > +#define ADXL345_REG_INT_ENABLE 0x2E > > > +#define ADXL345_REG_INT_MAP 0x2F > > > +#define ADXL345_REG_INT_SOURCE 0x30 > > > +#define ADXL345_REG_INT_SOURCE_MSK 0xFF > > > #define ADXL345_REG_DATA_FORMAT 0x31 > > > -#define ADXL345_REG_DATAX0 0x32 > > > -#define ADXL345_REG_DATAY0 0x34 > > > -#define ADXL345_REG_DATAZ0 0x36 > > > -#define ADXL345_REG_DATA_AXIS(index) \ > > > - (ADXL345_REG_DATAX0 + (index) * sizeof(__le16)) > > > +#define ADXL345_REG_XYZ_BASE 0x32 > > > +#define ADXL345_REG_DATA_AXIS(index) \ > > > + (ADXL345_REG_XYZ_BASE + (index) * sizeof(__le16)) > > > > > > +#define ADXL345_REG_FIFO_CTL 0x38 > > > +#define ADXL345_REG_FIFO_STATUS 0x39 > > > +#define ADXL345_REG_FIFO_STATUS_MSK 0x3F > > > + > > > +#define ADXL345_FIFO_CTL_SAMPLES(x) FIELD_PREP(GENMASK(4, 0), x) > > These need linux/bitfield.h to be included. > > > > Sure, I can do this. > > > However, that got me looking closer at this and it should be done > > differently. > > > > Just define the masks in here and put the FIELD_PREP() inline in the > > c file. Same for the various other FIELD_PREP macros in here. > > > > It may seem convenient to wrap all this up here, but in general > > I'd rather see that these are simple FIELD_PREP() calls where they > > are used inline. > > > > Ok, I understand, the masks stay in the adxl345.h and the FIELD_RPEP() > macros better go to where they are used, i.e. the adxl345_core.c. > > Thank you for reviewing this. I was a bit unsure about using the > FIELD_PREP() / GENMASK() combinations correctly. The code seems to > work for me, though. For future usage of those macros: Generally my > usage of those macros is correct? Yes, the usage is fine otherwise. > > Do I understand correctly, on the long run it would generally be > cleaner, also to migrate the defines to the adxl345_core.c? IMHO, > probably most of the constants in the adxl345.h can be private to > adxl345_core.c, and probably better should. Exactly. Thanks, Jonathan > > Best regards, > L
