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

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

 



> From: linux-iio-owner@xxxxxxxxxxxxxxx <linux-iio-owner@xxxxxxxxxxxxxxx> On
> Behalf Of Andy Shevchenko
> Sent: Dienstag, 31. März 2020 19:40
> To: Sa, Nuno <Nuno.Sa@xxxxxxxxxx>
> Cc: linux-iio <linux-iio@xxxxxxxxxxxxxxx>; devicetree
> <devicetree@xxxxxxxxxxxxxxx>; Jonathan Cameron <jic23@xxxxxxxxxx>;
> Hartmut Knaack <knaack.h@xxxxxx>; Lars-Peter Clausen <lars@xxxxxxxxxx>;
> Peter Meerwald-Stadler <pmeerw@xxxxxxxxxx>; Rob Herring
> <robh+dt@xxxxxxxxxx>; Mark Rutland <mark.rutland@xxxxxxx>; Ardelean,
> Alexandru <alexandru.Ardelean@xxxxxxxxxx>; Hennerich, Michael
> <Michael.Hennerich@xxxxxxxxxx>
> Subject: Re: [PATCH v3 2/6] iio: imu: adis: Add irq mask variable
> 
> [External]
> 
> 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...

- Nuno Sá
> 
> --
> 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