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

From: Jonathan Cameron <jic23@kernel.org>
To: "Nuno Sá" <nuno.sa@analog.com>
Cc: <linux-iio@vger.kernel.org>, <devicetree@vger.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 2/5] iio: imu: adis: Add irq mask variable
Date: Tue, 3 Mar 2020 20:40:41 +0000	[thread overview]
Message-ID: <20200303204041.36a1bc6a@archlinux> (raw)
In-Reply-To: <20200225124152.270914-3-nuno.sa@analog.com>

On Tue, 25 Feb 2020 13:41:49 +0100
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.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>

Missing docs for the addition to struct adis.

Otherwise, looks good to me.

thanks,

Jonathan

> ---
>  drivers/iio/imu/adis_trigger.c | 26 ++++++++++++++++++++++++--
>  include/linux/iio/imu/adis.h   |  1 +
>  2 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/imu/adis_trigger.c b/drivers/iio/imu/adis_trigger.c
> index a07dcc365c18..ae5a4f66752f 100644
> --- a/drivers/iio/imu/adis_trigger.c
> +++ b/drivers/iio/imu/adis_trigger.c
> @@ -34,6 +34,20 @@ static inline void adis_trigger_setup(struct adis *adis)
>  	iio_trigger_set_drvdata(adis->trig, adis);
>  }
>  
> +static inline 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 &&
> +		   adis->irq_mask != IRQF_TRIGGER_FALLING) {
> +		dev_err(&adis->spi->dev, "Invalid IRQ mask:%08lx\n",
> +			adis->irq_mask);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
>  /**
>   * adis_probe_trigger() - Sets up trigger for a adis device
>   * @adis: The adis device
> @@ -54,9 +68,13 @@ int adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev)
>  
>  	adis_trigger_setup(adis);
>  
> +	ret = __adis_validate_irq_mask(adis);
> +	if (ret)
> +		return ret;
> +
>  	ret = request_irq(adis->spi->irq,
>  			  &iio_trigger_generic_data_rdy_poll,
> -			  IRQF_TRIGGER_RISING,
> +			  adis->irq_mask,
>  			  indio_dev->name,
>  			  adis->trig);
>  	if (ret)
> @@ -95,9 +113,13 @@ int devm_adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev)
>  
>  	adis_trigger_setup(adis);
>  
> +	ret = __adis_validate_irq_mask(adis);
> +	if (ret)
> +		return ret;
> +
>  	ret = devm_request_irq(&adis->spi->dev, adis->spi->irq,
>  			       &iio_trigger_generic_data_rdy_poll,
> -			       IRQF_TRIGGER_RISING,
> +			       adis->irq_mask,
>  			       indio_dev->name,
>  			       adis->trig);
>  	if (ret)
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index 741512b28aaa..b4c35d137e2a 100644
> --- a/include/linux/iio/imu/adis.h
> +++ b/include/linux/iio/imu/adis.h
> @@ -84,6 +84,7 @@ struct adis {
>  	struct spi_message	msg;
>  	struct spi_transfer	*xfer;
>  	unsigned int		current_page;
> +	unsigned long		irq_mask;

This structure has kernel-doc. Please add this new element.

>  	void			*buffer;
>  
>  	uint8_t			tx[10] ____cacheline_aligned;