Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Sun, 1 May 2022 17:31:10 +0100
From:   Jonathan Cameron <jic23@kernel.org>
To:     Jagath Jog J <jagathjog1996@gmail.com>
Cc:     dan@dlrobertson.com, andy.shevchenko@gmail.com,
        linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 6/9] iio: accel: bma400: Add step change event
Message-ID: <20220501173110.67a18bf9@jic23-huawei>
In-Reply-To: <20220420211105.14654-7-jagathjog1996@gmail.com>
References: <20220420211105.14654-1-jagathjog1996@gmail.com>
        <20220420211105.14654-7-jagathjog1996@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 21 Apr 2022 02:41:02 +0530
Jagath Jog J <jagathjog1996@gmail.com> wrote:

> Added support for event when there is a detection of step change.
> INT1 pin is used to interrupt and event is pushed to userspace.
> 
> Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
Hi Jagath,

A query about handling of multiple interrupts...

> ---
>  drivers/iio/accel/bma400.h      |  2 +
>  drivers/iio/accel/bma400_core.c | 75 +++++++++++++++++++++++++++++++++
>  2 files changed, 77 insertions(+)
> 
>   * Read-write configuration registers
> diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
> index aafb5a40944d..fe101df7b773 100644
> --- a/drivers/iio/accel/bma400_core.c
> +++ b/drivers/iio/accel/bma400_core.c

>  
>  static const struct iio_trigger_ops bma400_trigger_ops = {
> @@ -971,6 +1035,7 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
>  {
>  	struct iio_dev *indio_dev = private;
>  	struct bma400_data *data = iio_priv(indio_dev);
> +	s64 timestamp = iio_get_time_ns(indio_dev);
>  	int ret;
>  
>  	/* Lock to protect the data->status */
> @@ -981,6 +1046,16 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
>  	if (ret)
>  		goto unlock_err;
>  
> +	if (FIELD_GET(BMA400_STEP_STAT_MASK, le16_to_cpu(data->status))) {
> +		iio_push_event(indio_dev,
> +			       IIO_EVENT_CODE(IIO_STEPS, 0, IIO_NO_MOD,
> +					      IIO_EV_DIR_NONE,
> +					      IIO_EV_TYPE_CHANGE, 0, 0, 0),
> +			       timestamp);
> +		mutex_unlock(&data->mutex);

Is it possible for two interrupt sources to be active at the same time?
Given the device is clearing interrupts on read (which is unusual enough to
make me check that on the datasheet) you will loose any other events.

Normal trick is to act on all set bits and if any of them were acted on
return HANDLED.

> +		return IRQ_HANDLED;
> +	}
> +
>  	if (FIELD_GET(BMA400_INT_DRDY_MSK, le16_to_cpu(data->status))) {
>  		mutex_unlock(&data->mutex);
>  		iio_trigger_poll_chained(data->trig);