Received-SPF: pass (google.com: domain of linux-kernel+bounces-188631-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <18dcd73685535685034346ca1d88fe0f36358dc5.camel@gmail.com>
Subject: Re: [PATCH v4 07/10] iio: imu: adis_trigger: Allow level interrupts
 for FIFO readings
From: Nuno =?ISO-8859-1?Q?S=E1?= <noname.nuno@gmail.com>
To: Ramona Gradinariu <ramona.bolboaca13@gmail.com>, 
	linux-kernel@vger.kernel.org, jic23@kernel.org, linux-iio@vger.kernel.org, 
	devicetree@vger.kernel.org, conor+dt@kernel.org, 
	krzysztof.kozlowski+dt@linaro.org, robh@kernel.org, nuno.sa@analog.com
Date: Fri, 24 May 2024 12:59:41 +0200
In-Reply-To: <20240524090030.336427-8-ramona.bolboaca13@gmail.com>
References: <20240524090030.336427-1-ramona.bolboaca13@gmail.com>
	 <20240524090030.336427-8-ramona.bolboaca13@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.52.1 (3.52.1-1.fc40) 
Precedence: bulk
MIME-Version: 1.0

On Fri, 2024-05-24 at 12:00 +0300, Ramona Gradinariu wrote:
> Currently, adis library allows configuration only for edge interrupts,
> needed for data ready sampling.
> This patch removes the restriction for level interrupts for devices
> which have FIFO support.
> Furthermore, in case of devices which have FIFO support,
> devm_request_threaded_irq is used for interrupt allocation, to avoid
> flooding the processor with the FIFO watermark level interrupt, which
> is active until enough data has been read from the FIFO.
>=20
> Signed-off-by: Ramona Gradinariu <ramona.bolboaca13@gmail.com>
> ---

Reviewed-by: Nuno Sa <nuno.sa@analog.com>
(much better, IMO, commit message :))

> =C2=A0drivers/iio/imu/adis_trigger.c | 37 ++++++++++++++++++++++++-------=
---
> =C2=A0include/linux/iio/imu/adis.h=C2=A0=C2=A0 |=C2=A0 1 +
> =C2=A02 files changed, 27 insertions(+), 11 deletions(-)
>=20
> diff --git a/drivers/iio/imu/adis_trigger.c b/drivers/iio/imu/adis_trigge=
r.c
> index f890bf842db8..a8740b043cfe 100644
> --- a/drivers/iio/imu/adis_trigger.c
> +++ b/drivers/iio/imu/adis_trigger.c
> @@ -34,17 +34,24 @@ static int adis_validate_irq_flag(struct adis *adis)
> =C2=A0	if (adis->data->unmasked_drdy)
> =C2=A0		adis->irq_flag |=3D IRQF_NO_AUTOEN;
> =C2=A0	/*
> -	 * Typically this devices have data ready either on the rising edge or
> -	 * on the falling edge of the data ready pin. This checks enforces that
> -	 * one of those is set in the drivers... It defaults to
> -	 * IRQF_TRIGGER_RISING for backward compatibility with devices that
> -	 * don't support changing the pin polarity.
> +	 * Typically adis devices without FIFO have data ready either on the
> +	 * rising edge or on the falling edge of the data ready pin.
> +	 * IMU devices with FIFO support have the watermark pin level driven
> +	 * either high or low when the FIFO is filled with the desired number
> +	 * of samples.
> +	 * It defaults to IRQF_TRIGGER_RISING for backward compatibility with
> +	 * devices that don't support changing the pin polarity.
> =C2=A0	 */
> =C2=A0	if (direction =3D=3D IRQF_TRIGGER_NONE) {
> =C2=A0		adis->irq_flag |=3D IRQF_TRIGGER_RISING;
> =C2=A0		return 0;
> =C2=A0	} else if (direction !=3D IRQF_TRIGGER_RISING &&
> -		=C2=A0=C2=A0 direction !=3D IRQF_TRIGGER_FALLING) {
> +		=C2=A0=C2=A0 direction !=3D IRQF_TRIGGER_FALLING && !adis->data->has_f=
ifo) {
> +		dev_err(&adis->spi->dev, "Invalid IRQ mask: %08lx\n",
> +			adis->irq_flag);
> +		return -EINVAL;
> +	} else if (direction !=3D IRQF_TRIGGER_HIGH &&
> +		=C2=A0=C2=A0 direction !=3D IRQF_TRIGGER_LOW && adis->data->has_fifo) =
{
> =C2=A0		dev_err(&adis->spi->dev, "Invalid IRQ mask: %08lx\n",
> =C2=A0			adis->irq_flag);
> =C2=A0		return -EINVAL;
> @@ -77,11 +84,19 @@ int devm_adis_probe_trigger(struct adis *adis, struct=
 iio_dev
> *indio_dev)
> =C2=A0	if (ret)
> =C2=A0		return ret;
> =C2=A0
> -	ret =3D devm_request_irq(&adis->spi->dev, adis->spi->irq,
> -			=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 &iio_trigger_generic_data_rdy_po=
ll,
> -			=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 adis->irq_flag,
> -			=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 indio_dev->name,
> -			=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 adis->trig);
> +	if (adis->data->has_fifo)
> +		ret =3D devm_request_threaded_irq(&adis->spi->dev, adis->spi->irq,
> +						NULL,
> +						&iio_trigger_generic_data_rdy_poll
> ,
> +						adis->irq_flag | IRQF_ONESHOT,
> +						indio_dev->name,
> +						adis->trig);
> +	else
> +		ret =3D devm_request_irq(&adis->spi->dev, adis->spi->irq,
> +				=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 &iio_trigger_generic_data_rdy_p=
oll,
> +				=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 adis->irq_flag,
> +				=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 indio_dev->name,
> +				=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 adis->trig);
> =C2=A0	if (ret)
> =C2=A0		return ret;
> =C2=A0
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index 8dda3cfa5773..4d4cdbe155b2 100644
> --- a/include/linux/iio/imu/adis.h
> +++ b/include/linux/iio/imu/adis.h
> @@ -85,6 +85,7 @@ struct adis_data {
> =C2=A0	bool unmasked_drdy;
> =C2=A0
> =C2=A0	bool has_paging;
> +	bool has_fifo;
> =C2=A0
> =C2=A0	unsigned int burst_reg_cmd;
> =C2=A0	unsigned int burst_len;