linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Song Bao Hua (Barry Song)" <song.bao.hua@hisilicon.com>
To: Jonathan Cameron <jic23@kernel.org>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>
Cc: Jonathan Cameron <jonathan.cameron@huawei.com>,
	Alexandru Ardelean <ardeleanalex@gmail.com>,
	Nuno Sa <Nuno.Sa@analog.com>,
	"tiantao (H)" <tiantao6@hisilicon.com>
Subject: RE: [PATCH 6/7] iio:imu:adis: Use IRQF_NO_AUTOEN instead of irq request then disable
Date: Fri, 2 Apr 2021 20:37:37 +0000	[thread overview]
Message-ID: <a733aa9b11684bb990f0bc6a50b101e5@hisilicon.com> (raw)
In-Reply-To: <20210402184544.488862-7-jic23@kernel.org>



> -----Original Message-----
> From: Jonathan Cameron [mailto:jic23@kernel.org]
> Sent: Saturday, April 3, 2021 7:46 AM
> To: linux-iio@vger.kernel.org
> Cc: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; Alexandru Ardelean <ardeleanalex@gmail.com>;
> Nuno Sa <Nuno.Sa@analog.com>
> Subject: [PATCH 6/7] iio:imu:adis: Use IRQF_NO_AUTOEN instead of irq request
> then disable
> 
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> 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@huawei.com>
> Cc: Alexandru Ardelean <ardeleanalex@gmail.com>
> Cc: Nuno Sa <Nuno.Sa@analog.com>
> ---
>  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@hisilicon.com>

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


  reply	other threads:[~2021-04-02 20:37 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-02 18:45 [PATCH 0/7] iio: Use IRQF_NO_AUTOEN Jonathan Cameron
2021-04-02 18:45 ` [PATCH 1/7] iio:adc:ad7766: Use new IRQF_NO_AUTOEN to reduce boilerplate Jonathan Cameron
2021-04-02 20:10   ` Song Bao Hua (Barry Song)
2021-04-05 14:27     ` Jonathan Cameron
2021-04-02 18:45 ` [PATCH 2/7] iio:adc:exynos-adc: Use new IRQF_NO_AUTOEN flag rather than separate irq_disable() Jonathan Cameron
2021-04-02 20:11   ` Song Bao Hua (Barry Song)
2021-04-04 19:20   ` Krzysztof Kozlowski
2021-04-05 14:29     ` Jonathan Cameron
2021-04-02 18:45 ` [PATCH 3/7] iio:adc:nau7802: Use IRQF_NO_AUTOEN instead of request then disable Jonathan Cameron
2021-04-02 20:12   ` Song Bao Hua (Barry Song)
2021-04-05 14:33     ` Jonathan Cameron
2021-04-02 18:45 ` [PATCH 4/7] iio:adc:sun4i-gpadc: Use new IRQF_NO_AUTOEN flag " Jonathan Cameron
2021-04-02 20:13   ` Song Bao Hua (Barry Song)
2021-04-05 14:35     ` Jonathan Cameron
2021-04-02 18:45 ` [PATCH 5/7] iio:chemical:scd30: Use IRQF_NO_AUTOEN to avoid irq " Jonathan Cameron
2021-04-02 20:14   ` Song Bao Hua (Barry Song)
2021-04-05 14:36     ` Jonathan Cameron
2021-04-02 18:45 ` [PATCH 6/7] iio:imu:adis: Use IRQF_NO_AUTOEN instead of " Jonathan Cameron
2021-04-02 20:37   ` Song Bao Hua (Barry Song) [this message]
2021-04-05 14:40     ` Jonathan Cameron
2021-04-06 11:48       ` Sa, Nuno
2021-04-02 18:45 ` [PATCH 7/7] iio:adc:ad_sigma_delta: Use IRQF_NO_AUTOEN rather than request and disable Jonathan Cameron
2021-04-02 20:30   ` Song Bao Hua (Barry Song)
2021-04-05 14:51     ` Jonathan Cameron
2021-04-05 22:56       ` Song Bao Hua (Barry Song)
2021-04-02 19:00 ` [PATCH 0/7] iio: Use IRQF_NO_AUTOEN Andy Shevchenko
2021-04-03 11:45 ` Lars-Peter Clausen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a733aa9b11684bb990f0bc6a50b101e5@hisilicon.com \
    --to=song.bao.hua@hisilicon.com \
    --cc=Nuno.Sa@analog.com \
    --cc=ardeleanalex@gmail.com \
    --cc=jic23@kernel.org \
    --cc=jonathan.cameron@huawei.com \
    --cc=linux-iio@vger.kernel.org \
    --cc=tiantao6@hisilicon.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).