linux-kernel - Re: [PATCH v4 6/9] iio: accel: bma400: Add step change event

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAM+2EuL7Qcj2znsmXPd6Q7wT-EQrUa=y6idcH=rryQdSiGSdOg@mail.gmail.com>
Date:   Mon, 2 May 2022 01:57:30 +0530
From:   Jagath Jog J <jagathjog1996@...il.com>
To:     Jonathan Cameron <jic23@...nel.org>
Cc:     Dan Robertson <dan@...obertson.com>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        linux-iio <linux-iio@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 6/9] iio: accel: bma400: Add step change event

Hi Jonathan,

On Sun, May 1, 2022 at 9:52 PM Jonathan Cameron <jic23@...nel.org> wrote:
>
> On Thu, 21 Apr 2022 02:41:02 +0530
> Jagath Jog J <jagathjog1996@...il.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@...il.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);
>