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

[Jonathan Cameron <jic23@kernel.org>]

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

> 
> >
> >