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

Lothar Rubusch
2024-12-28T15:48:48Z

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?

>
>