Re: [PATCH v3 2/6] iio: imu: adis: Add irq mask variable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Apr 1, 2020 at 10:22 AM Sa, Nuno <Nuno.Sa@xxxxxxxxxx> wrote:
> > From: linux-iio-owner@xxxxxxxxxxxxxxx <linux-iio-owner@xxxxxxxxxxxxxxx> On
> > Behalf Of Andy Shevchenko
> > Sent: Dienstag, 31. März 2020 19:40
> > On Tue, Mar 31, 2020 at 2:49 PM Nuno Sá <nuno.sa@xxxxxxxxxx> wrote:
> > >
> > > There are some ADIS devices that can configure the data ready pin
> > > polarity. Hence, we cannot hardcode our IRQ mask as
> > IRQF_TRIGGER_RISING
> > > since we might want to have it as IRQF_TRIGGER_FALLING.

...

> > > +static int adis_validate_irq_mask(struct adis *adis)
> > > +{
> > > +       if (!adis->irq_mask) {
> > > +               adis->irq_mask = IRQF_TRIGGER_RISING;
> > > +               return 0;
> >
> > > +       } else if (adis->irq_mask != IRQF_TRIGGER_RISING &&
> >
> > 'else' is redundant.
> >
> > > +                  adis->irq_mask != IRQF_TRIGGER_FALLING) {
> >
> > But this condition rises questions. Why i can't configure both?
> > Why I can't configure other flags there?
>
> Both you can't because it is just how these type of devices work. Data is ready either
> on the rising edge or on the falling edge (if supported by the device)...
> I agree this could check if only one of the flags are set instead of directly comparing the
> values (invalidating other flags) but I would prefer to keep this simple for now...
>
> >
> > > +               dev_err(&adis->spi->dev, "Invalid IRQ mask: %08lx\n",
> > > +                       adis->irq_mask);
> > > +               return -EINVAL;
> > > +       }
> >
> > > +       return 0;
> > > +}
> >
> > And actually name of the function is not exactly what it does. It
> > validates *or* initializes.
>
> Well, yes. It just sets the mask to the default value to keep backward compatibility
> with all the other devices that don't support/use this variable...

Perhaps documentation in a comment form should be added.
Moreover, I realized that you added to variable and function mask
suffix while it's actually flag.

-- 
With Best Regards,
Andy Shevchenko




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux