RE: [PATCH 6/7] iio:imu:adis: Use IRQF_NO_AUTOEN instead of irq request then disable

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

 




> -----Original Message-----
> From: Jonathan Cameron [mailto:jic23@xxxxxxxxxx]
> Sent: Saturday, April 3, 2021 7:46 AM
> To: linux-iio@xxxxxxxxxxxxxxx
> Cc: Song Bao Hua (Barry Song) <song.bao.hua@xxxxxxxxxxxxx>; Jonathan Cameron
> <jonathan.cameron@xxxxxxxxxx>; Alexandru Ardelean <ardeleanalex@xxxxxxxxx>;
> Nuno Sa <Nuno.Sa@xxxxxxxxxx>
> Subject: [PATCH 6/7] iio:imu:adis: Use IRQF_NO_AUTOEN instead of irq request
> then disable
> 
> From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> 
> This is a bit involved as the adis library code already has some
> sanity checking of the flags of the requested irq that we need
> to ensure is happy to pass through the IRQF_NO_AUTOEN flag untouched.
> 
> Using this flag avoids us autoenabling the irq in the adis16460 and
> adis16475 drivers which cover parts that don't have any means of
> masking the interrupt on the device end.
> 
> Note, compile tested only!
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> Cc: Alexandru Ardelean <ardeleanalex@xxxxxxxxx>
> Cc: Nuno Sa <Nuno.Sa@xxxxxxxxxx>
> ---
>  drivers/iio/imu/adis16460.c    |  4 ++--
>  drivers/iio/imu/adis16475.c    |  5 +++--
>  drivers/iio/imu/adis_trigger.c | 11 ++++++-----
>  3 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c
> index 74a161e39733..73bf45e859b8 100644
> --- a/drivers/iio/imu/adis16460.c
> +++ b/drivers/iio/imu/adis16460.c
> @@ -403,12 +403,12 @@ static int adis16460_probe(struct spi_device *spi)
>  	if (ret)
>  		return ret;
> 
> +	/* We cannot mask the interrupt, so ensure it isn't auto enabled */
> +	st->adis.irq_flag |= IRQF_NO_AUTOEN;
>  	ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL);
>  	if (ret)
>  		return ret;
> 
> -	adis16460_enable_irq(&st->adis, 0);
> -
>  	ret = __adis_initial_startup(&st->adis);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
> index 8f6bea4b6608..1de62fc79e0f 100644
> --- a/drivers/iio/imu/adis16475.c
> +++ b/drivers/iio/imu/adis16475.c
> @@ -1258,6 +1258,9 @@ static int adis16475_config_irq_pin(struct adis16475 *st)
>  		return -EINVAL;
>  	}
> 
> +	/* We cannot mask the interrupt so ensure it's not enabled at request */
> +	st->adis.irq_flag |= IRQF_NO_AUTOEN;
> +
>  	val = ADIS16475_MSG_CTRL_DR_POL(polarity);
>  	ret = __adis_update_bits(&st->adis, ADIS16475_REG_MSG_CTRL,
>  				 ADIS16475_MSG_CTRL_DR_POL_MASK, val);
> @@ -1362,8 +1365,6 @@ static int adis16475_probe(struct spi_device *spi)
>  	if (ret)
>  		return ret;
> 
> -	adis16475_enable_irq(&st->adis, false);
> -
>  	ret = devm_iio_device_register(&spi->dev, indio_dev);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/iio/imu/adis_trigger.c b/drivers/iio/imu/adis_trigger.c
> index 0f29e56200af..39b50d6fdb6a 100644
> --- a/drivers/iio/imu/adis_trigger.c
> +++ b/drivers/iio/imu/adis_trigger.c
> @@ -29,18 +29,19 @@ static const struct iio_trigger_ops adis_trigger_ops = {
> 
>  static int adis_validate_irq_flag(struct adis *adis)
>  {
> +	unsigned long direction = adis->irq_flag & IRQF_TRIGGER_MASK;
>  	/*
>  	 * 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 wiht devices that
> +	 * IRQF_TRIGGER_RISING for backward compatibility with devices that
>  	 * don't support changing the pin polarity.
>  	 */
> -	if (!adis->irq_flag) {
> -		adis->irq_flag = IRQF_TRIGGER_RISING;
> +	if (!direction) {

Reviewed-by: Barry Song <song.bao.hua@xxxxxxxxxxxxx>

Though a better way might be:

if (direction == IRQF_TRIGGER_NONE)

> +		adis->irq_flag |= IRQF_TRIGGER_RISING;
>  		return 0;
> -	} else if (adis->irq_flag != IRQF_TRIGGER_RISING &&
> -		   adis->irq_flag != IRQF_TRIGGER_FALLING) {
> +	} else if (direction != IRQF_TRIGGER_RISING &&
> +		   direction != IRQF_TRIGGER_FALLING) {
>  		dev_err(&adis->spi->dev, "Invalid IRQ mask: %08lx\n",
>  			adis->irq_flag);
>  		return -EINVAL;
> --
> 2.31.1

Thanks
Barry




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux