Re: [PATCH v8 7/7] iio: accel: adxl345: complete the list of defines

Jonathan Cameron
2025-01-04T12:51:22Z

On Sat, 28 Dec 2024 16:48:11 +0100
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> On Sat, Dec 28, 2024 at 3:47 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Wed, 25 Dec 2024 18:13:38 +0000
> > Lothar Rubusch <l.rubusch@gmail.com> wrote:
> >
> > > Having interrupts events and FIFO available allows to evaluate the
> > > sensor events. Cover the list of interrupt based sensor events. Keep
> > > them in the header file for readability.
> > >
> > > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> >
> > One comment inline
> >
> > Thanks,
> >
> > Jonathan
> >
> > > #define ADXL345_REG_BW_RATE 0x2C
> > > #define ADXL345_REG_POWER_CTL 0x2D
> > > #define ADXL345_REG_INT_ENABLE 0x2E
> > > @@ -34,20 +59,40 @@
> > > #define ADXL345_FIFO_CTL_MODE(x) FIELD_PREP(GENMASK(7, 6), x)
> > >
> > > #define ADXL345_INT_DATA_READY BIT(7)
> > > +#define ADXL345_INT_SINGLE_TAP BIT(6)
> > > +#define ADXL345_INT_DOUBLE_TAP BIT(5)
> > > +#define ADXL345_INT_ACTIVITY BIT(4)
> > > +#define ADXL345_INT_INACTIVITY BIT(3)
> > > +#define ADXL345_INT_FREE_FALL BIT(2)
> > > #define ADXL345_INT_WATERMARK BIT(1)
> > > #define ADXL345_INT_OVERRUN BIT(0)
> > > +
> > > +#define ADXL345_S_TAP_MSK ADXL345_INT_SINGLE_TAP
> > > +#define ADXL345_D_TAP_MSK ADXL345_INT_DOUBLE_TAP
> >
> > Why have these?
> >
>
> To be honest, I'm unsure to keep this patch within this series now.
>
> Initially, ..... long story short, having FIFO and interrupt handling
> now, it is possible to apply and use those ADXL345_INT_ constants. On
> the other side, having this more and more separated out of other
> patches, it becomes clear there is still some implementation pending
> to really use them. I'd like to focus this series rather on FIFO and
> interrupt mechanism.
>
> Especially when it comes to the ADXL345_S_TAP_MSK defines, which
> probably make even less sense here, if I look at it now.
>
> What would you recommend - Keep it? Drop it? Add just the ADXL345_INT_
> fields w/o the _MSKs?

I don't mind that much either way so I'll go with what ever you did in
v9 (not looked yet :)

Jonathan

>
> >
> >