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

From: "Sa, Nuno" <Nuno.Sa@analog.com>
To: "jic23@kernel.org" <jic23@kernel.org>
Cc: "linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"Ardelean, Alexandru" <alexandru.Ardelean@analog.com>,
	"pmeerw@pmeerw.net" <pmeerw@pmeerw.net>,
	"lars@metafoo.de" <lars@metafoo.de>,
	"knaack.h@gmx.de" <knaack.h@gmx.de>,
	"Hennerich, Michael" <Michael.Hennerich@analog.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>
Subject: Re: [PATCH 2/5] iio: imu: adis: Add irq mask variable
Date: Wed, 4 Mar 2020 17:29:24 +0000	[thread overview]
Message-ID: <0eb6b0373b13457ee6db321369c1d6217b6a6142.camel@analog.com> (raw)
In-Reply-To: <20200303204041.36a1bc6a@archlinux>

On Tue, 2020-03-03 at 20:40 +0000, Jonathan Cameron wrote:
> 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.

Got it.

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