Re: [PATCH v1 07/12] iio: accel: adxl345: show tap status and direction

[Jonathan Cameron]

On Tue, 28 Jan 2025 12:00:55 +0000
Lothar Rubusch <l.rubusch@xxxxxxxxx> wrote:

> Provide information in the iio tap event about the tap direction. This
> can be verified using 'iio_event_monior adxl345'. Reading out the
> ACT_TAP_STATUS register is also in preparation for activity events.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@xxxxxxxxx>

So, this confuses me a little.  You have separate controls on
enabling the directions but only a single bit to indicate that a tap
was detected. Can only one be enabled at time?  If not how does
reporting only one direction make sense?

That aside, why not squash this with the previous patch?


Jonathan


> ---
>  drivers/iio/accel/adxl345_core.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 0d991f3ec10c..7831ec511941 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -800,17 +800,26 @@ static const struct iio_buffer_setup_ops adxl345_buffer_ops = {
>  	.predisable = adxl345_buffer_predisable,
>  };
>  
> -static int adxl345_get_status(struct adxl345_state *st, unsigned int *int_stat)
> +static int adxl345_get_status(struct adxl345_state *st, unsigned int *int_stat,
> +			      enum iio_modifier *act_tap_dir)
>  {
>  	unsigned int regval;
>  	bool check_tap_stat;
>  
> +	*act_tap_dir = IIO_NO_MOD;
>  	check_tap_stat = FIELD_GET(ADXL345_REG_TAP_AXIS_MSK, st->tap_axis_ctrl) > 0;
>  
>  	if (check_tap_stat) {
>  		/* ACT_TAP_STATUS should be read before clearing the interrupt */
>  		if (regmap_read(st->regmap, ADXL345_REG_ACT_TAP_STATUS, &regval))
>  			return -EINVAL;
> +
> +		if (FIELD_GET(ADXL345_Z_EN, regval) > 0)
> +			*act_tap_dir = IIO_MOD_Z;
> +		else if (FIELD_GET(ADXL345_Y_EN, regval) > 0)
> +			*act_tap_dir = IIO_MOD_Y;
> +		else if (FIELD_GET(ADXL345_X_EN, regval) > 0)
> +			*act_tap_dir = IIO_MOD_X;
>  	}
>  
>  	return regmap_read(st->regmap, ADXL345_REG_INT_SOURCE, int_stat);
> @@ -835,7 +844,8 @@ static int adxl345_fifo_push(struct iio_dev *indio_dev,
>  	return 0;
>  }
>  
> -static int adxl345_push_event(struct iio_dev *indio_dev, int int_stat)
> +static int adxl345_push_event(struct iio_dev *indio_dev, int int_stat,
> +			      enum iio_modifier act_tap_dir)
>  {
>  	s64 ts = iio_get_time_ns(indio_dev);
>  	int ret;
> @@ -843,7 +853,7 @@ static int adxl345_push_event(struct iio_dev *indio_dev, int int_stat)
>  	if (FIELD_GET(ADXL345_INT_SINGLE_TAP, int_stat)) {
>  		ret = iio_push_event(indio_dev,
>  				     IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
> -							IIO_MOD_X_OR_Y_OR_Z,
> +							act_tap_dir,
>  							IIO_EV_TYPE_GESTURE,
>  							IIO_EV_DIR_SINGLETAP),
>  				     ts);
> @@ -866,12 +876,13 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
>  	struct iio_dev *indio_dev = p;
>  	struct adxl345_state *st = iio_priv(indio_dev);
>  	int int_stat;
> +	enum iio_modifier act_tap_dir;
>  	int samples;
>  
> -	if (adxl345_get_status(st, &int_stat))
> +	if (adxl345_get_status(st, &int_stat, &act_tap_dir))
>  		return IRQ_NONE;
>  
> -	if (adxl345_push_event(indio_dev, int_stat) == 0)
> +	if (adxl345_push_event(indio_dev, int_stat, act_tap_dir) == 0)
>  		return IRQ_HANDLED;
>  
>  	if (FIELD_GET(ADXL345_INT_WATERMARK, int_stat)) {