[PATCH v4] Fix IRQ issue by setting IRQ_DISABLE_UNLAZY flag

[PATCH v4] Fix IRQ issue by setting IRQ_DISABLE_UNLAZY flag

[Masahiro Honda]

The Sigma-Delta ADCs supported by this driver can use SDO as an interrupt
line to indicate the completion of a conversion. However, some devices
cannot properly detect the completion of a conversion by an interrupt.
This is for the reason mentioned in the following commit.

commit e9849777d0e2 ("genirq: Add flag to force mask in
                      disable_irq[_nosync]()")

A read operation is performed by an extra interrupt before the completion
of a conversion. This patch fixes the issue by setting IRQ_DISABLE_UNLAZY
flag.

Signed-off-by: Masahiro Honda <honda@mechatrax.com>
---
v4:
 - Remove the callback.
v3: https://lore.kernel.org/linux-iio/20230420102316.757-1-honda@mechatrax.com/
 - Remove the Kconfig option.
v2: https://lore.kernel.org/linux-iio/20230414102744.150-1-honda@mechatrax.com/
 - Rework commit message.
 - Add a new entry in the Kconfig.
 - Call irq_clear_status_flags(irq, IRQ_DISABLE_UNLAZY) when freeing the IRQ.
v1: https://lore.kernel.org/linux-iio/20230306044737.862-1-honda@mechatrax.com/

 drivers/iio/adc/ad_sigma_delta.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index d8570f6207..7e21928707 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -584,6 +584,10 @@ static int devm_ad_sd_probe_trigger(struct device *dev, struct iio_dev *indio_de
 	init_completion(&sigma_delta->completion);
 
 	sigma_delta->irq_dis = true;
+
+	/* the IRQ core clears IRQ_DISABLE_UNLAZY flag when freeing an IRQ */
+	irq_set_status_flags(sigma_delta->spi->irq, IRQ_DISABLE_UNLAZY);
+
 	ret = devm_request_irq(dev, sigma_delta->spi->irq,
 			       ad_sd_data_rdy_trig_poll,
 			       sigma_delta->info->irq_flags | IRQF_NO_AUTOEN,
-- 
2.34.1

Re: [PATCH v4] Fix IRQ issue by setting IRQ_DISABLE_UNLAZY flag

[Nuno Sá]

On Tue, 2023-05-02 at 19:29 +0900, Masahiro Honda wrote:
> The Sigma-Delta ADCs supported by this driver can use SDO as an interrupt
> line to indicate the completion of a conversion. However, some devices
> cannot properly detect the completion of a conversion by an interrupt.
> This is for the reason mentioned in the following commit.
> 
> commit e9849777d0e2 ("genirq: Add flag to force mask in
>                       disable_irq[_nosync]()")
> 
> A read operation is performed by an extra interrupt before the completion
> of a conversion. This patch fixes the issue by setting IRQ_DISABLE_UNLAZY
> flag.
> 
> Signed-off-by: Masahiro Honda <honda@mechatrax.com>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

> v4:
>  - Remove the callback.
> v3:
> https://lore.kernel.org/linux-iio/20230420102316.757-1-honda@mechatrax.com/
>  - Remove the Kconfig option.
> v2:
> https://lore.kernel.org/linux-iio/20230414102744.150-1-honda@mechatrax.com/
>  - Rework commit message.
>  - Add a new entry in the Kconfig.
>  - Call irq_clear_status_flags(irq, IRQ_DISABLE_UNLAZY) when freeing the IRQ.
> v1:
> https://lore.kernel.org/linux-iio/20230306044737.862-1-honda@mechatrax.com/
> 
>  drivers/iio/adc/ad_sigma_delta.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/iio/adc/ad_sigma_delta.c
> b/drivers/iio/adc/ad_sigma_delta.c
> index d8570f6207..7e21928707 100644
> --- a/drivers/iio/adc/ad_sigma_delta.c
> +++ b/drivers/iio/adc/ad_sigma_delta.c
> @@ -584,6 +584,10 @@ static int devm_ad_sd_probe_trigger(struct device *dev,
> struct iio_dev *indio_de
>         init_completion(&sigma_delta->completion);
>  
>         sigma_delta->irq_dis = true;
> +
> +       /* the IRQ core clears IRQ_DISABLE_UNLAZY flag when freeing an IRQ */
> +       irq_set_status_flags(sigma_delta->spi->irq, IRQ_DISABLE_UNLAZY);
> +
>         ret = devm_request_irq(dev, sigma_delta->spi->irq,
>                                ad_sd_data_rdy_trig_poll,
>                                sigma_delta->info->irq_flags | IRQF_NO_AUTOEN,

Re: [PATCH v4] Fix IRQ issue by setting IRQ_DISABLE_UNLAZY flag

[Jonathan Cameron]

On Wed, 03 May 2023 10:09:28 +0200
Nuno Sá <noname.nuno@gmail.com> wrote:

Patch title should be something like the following so it's easy to see what
is affected when looking at a long list of patches.

iio: adc: ad_sigma_delta: Fix IRQ issue by setting IRQ_DISABLE_UNLAZY flag.


> On Tue, 2023-05-02 at 19:29 +0900, Masahiro Honda wrote:
> > The Sigma-Delta ADCs supported by this driver can use SDO as an interrupt
> > line to indicate the completion of a conversion. However, some devices
> > cannot properly detect the completion of a conversion by an interrupt.
> > This is for the reason mentioned in the following commit.
> > 
> > commit e9849777d0e2 ("genirq: Add flag to force mask in
> >                       disable_irq[_nosync]()")
> > 
> > A read operation is performed by an extra interrupt before the completion
> > of a conversion. This patch fixes the issue by setting IRQ_DISABLE_UNLAZY
> > flag.
> > 
> > Signed-off-by: Masahiro Honda <honda@mechatrax.com>
> > ---  
> 
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>

Fixes tag?  We'll want to know how far to backport this.  I assume it's
limited by the above commit as these drivers are older than that. 

I'm also not totally sure what this 'looks like' for a user.  What happens?
Fail to read, wrong value, lock up or something else?  It would be helpful
to include that information in case anyone else runs into this.

Actual change looks right to me.

Thanks,

Jonathan


> 
> > v4:
> >  - Remove the callback.
> > v3:
> > https://lore.kernel.org/linux-iio/20230420102316.757-1-honda@mechatrax.com/
> >  - Remove the Kconfig option.
> > v2:
> > https://lore.kernel.org/linux-iio/20230414102744.150-1-honda@mechatrax.com/
> >  - Rework commit message.
> >  - Add a new entry in the Kconfig.
> >  - Call irq_clear_status_flags(irq, IRQ_DISABLE_UNLAZY) when freeing the IRQ.
> > v1:
> > https://lore.kernel.org/linux-iio/20230306044737.862-1-honda@mechatrax.com/
> > 
> >  drivers/iio/adc/ad_sigma_delta.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/iio/adc/ad_sigma_delta.c
> > b/drivers/iio/adc/ad_sigma_delta.c
> > index d8570f6207..7e21928707 100644
> > --- a/drivers/iio/adc/ad_sigma_delta.c
> > +++ b/drivers/iio/adc/ad_sigma_delta.c
> > @@ -584,6 +584,10 @@ static int devm_ad_sd_probe_trigger(struct device *dev,
> > struct iio_dev *indio_de
> >         init_completion(&sigma_delta->completion);
> >  
> >         sigma_delta->irq_dis = true;
> > +
> > +       /* the IRQ core clears IRQ_DISABLE_UNLAZY flag when freeing an IRQ */
> > +       irq_set_status_flags(sigma_delta->spi->irq, IRQ_DISABLE_UNLAZY);
> > +
> >         ret = devm_request_irq(dev, sigma_delta->spi->irq,
> >                                ad_sd_data_rdy_trig_poll,
> >                                sigma_delta->info->irq_flags | IRQF_NO_AUTOEN,  
> 

Re: [PATCH v4] Fix IRQ issue by setting IRQ_DISABLE_UNLAZY flag

[Masahiro Honda]

Hi Jonathan,

Thank you for your advice.

On Sun, May 7, 2023 at 11:45 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Wed, 03 May 2023 10:09:28 +0200
> Nuno Sá <noname.nuno@gmail.com> wrote:
>
> Patch title should be something like the following so it's easy to see what
> is affected when looking at a long list of patches.
>
> iio: adc: ad_sigma_delta: Fix IRQ issue by setting IRQ_DISABLE_UNLAZY flag.
>

I'll fix the title.

>
> > On Tue, 2023-05-02 at 19:29 +0900, Masahiro Honda wrote:
> > > The Sigma-Delta ADCs supported by this driver can use SDO as an interrupt
> > > line to indicate the completion of a conversion. However, some devices
> > > cannot properly detect the completion of a conversion by an interrupt.
> > > This is for the reason mentioned in the following commit.
> > >
> > > commit e9849777d0e2 ("genirq: Add flag to force mask in
> > >                       disable_irq[_nosync]()")
> > >
> > > A read operation is performed by an extra interrupt before the completion
> > > of a conversion. This patch fixes the issue by setting IRQ_DISABLE_UNLAZY
> > > flag.
> > >
> > > Signed-off-by: Masahiro Honda <honda@mechatrax.com>
> > > ---
> >
> > Reviewed-by: Nuno Sá <nuno.sa@analog.com>
>
> Fixes tag?  We'll want to know how far to backport this.  I assume it's
> limited by the above commit as these drivers are older than that.
>

I'll try to add a fixes tag.

> I'm also not totally sure what this 'looks like' for a user.  What happens?
> Fail to read, wrong value, lock up or something else?  It would be helpful
> to include that information in case anyone else runs into this.
>

I'll add information to the commit message.

Regards,

Masahiro


> Actual change looks right to me.
>
> Thanks,
>
> Jonathan
>
>
> >
> > > v4:
> > >  - Remove the callback.
> > > v3:
> > > https://lore.kernel.org/linux-iio/20230420102316.757-1-honda@mechatrax.com/
> > >  - Remove the Kconfig option.
> > > v2:
> > > https://lore.kernel.org/linux-iio/20230414102744.150-1-honda@mechatrax.com/
> > >  - Rework commit message.
> > >  - Add a new entry in the Kconfig.
> > >  - Call irq_clear_status_flags(irq, IRQ_DISABLE_UNLAZY) when freeing the IRQ.
> > > v1:
> > > https://lore.kernel.org/linux-iio/20230306044737.862-1-honda@mechatrax.com/
> > >
> > >  drivers/iio/adc/ad_sigma_delta.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/iio/adc/ad_sigma_delta.c
> > > b/drivers/iio/adc/ad_sigma_delta.c
> > > index d8570f6207..7e21928707 100644
> > > --- a/drivers/iio/adc/ad_sigma_delta.c
> > > +++ b/drivers/iio/adc/ad_sigma_delta.c
> > > @@ -584,6 +584,10 @@ static int devm_ad_sd_probe_trigger(struct device *dev,
> > > struct iio_dev *indio_de
> > >         init_completion(&sigma_delta->completion);
> > >
> > >         sigma_delta->irq_dis = true;
> > > +
> > > +       /* the IRQ core clears IRQ_DISABLE_UNLAZY flag when freeing an IRQ */
> > > +       irq_set_status_flags(sigma_delta->spi->irq, IRQ_DISABLE_UNLAZY);
> > > +
> > >         ret = devm_request_irq(dev, sigma_delta->spi->irq,
> > >                                ad_sd_data_rdy_trig_poll,
> > >                                sigma_delta->info->irq_flags | IRQF_NO_AUTOEN,
> >
>