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;
MIME-Version: 1.0
References: <20220420211105.14654-1-jagathjog1996@gmail.com>
 <20220420211105.14654-7-jagathjog1996@gmail.com> <20220501173110.67a18bf9@jic23-huawei>
In-Reply-To: <20220501173110.67a18bf9@jic23-huawei>
From:   Jagath Jog J <jagathjog1996@gmail.com>
Date:   Mon, 2 May 2022 01:57:30 +0530
Message-ID: <CAM+2EuL7Qcj2znsmXPd6Q7wT-EQrUa=y6idcH=rryQdSiGSdOg@mail.gmail.com>
Subject: Re: [PATCH v4 6/9] iio: accel: bma400: Add step change event
To:     Jonathan Cameron <jic23@kernel.org>
Cc:     Dan Robertson <dan@dlrobertson.com>,
        Andy Shevchenko <andy.shevchenko@gmail.com>,
        linux-iio <linux-iio@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Jonathan,

On Sun, May 1, 2022 at 9:52 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> 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?

Yeah, it is possible when multiple interrupts are enabled like data ready,
step and generic interrupts.

> 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.

Then I will push all the events that occurred and then in the end I will return
HANDLED so that none of the events are missed.
I will change this in the next version.

Thank you,
Jagath


>
> > +             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);
>