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

From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: "Nuno Sá" <nuno.sa@analog.com>
Cc: linux-iio <linux-iio@vger.kernel.org>,
	devicetree <devicetree@vger.kernel.org>,
	Jonathan Cameron <jic23@kernel.org>,
	Hartmut Knaack <knaack.h@gmx.de>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexandru Ardelean <alexandru.Ardelean@analog.com>,
	Michael Hennerich <Michael.Hennerich@analog.com>
Subject: Re: [PATCH v3 2/6] iio: imu: adis: Add irq mask variable
Date: Tue, 31 Mar 2020 20:40:02 +0300	[thread overview]
Message-ID: <CAHp75VfXS1P-EJaKERojK=-3erYi5MxyDMjfrELyY2X3PZEN1A@mail.gmail.com> (raw)
In-Reply-To: <20200331114811.7978-3-nuno.sa@analog.com>

On Tue, Mar 31, 2020 at 2:49 PM Nuno Sá <nuno.sa@analog.com> 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?

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

-- 
With Best Regards,
Andy Shevchenko