| Message ID | 20230120124645.819910-1-nuno.sa@analog.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | iio: adc: ad7791: fix IRQ flags | expand |
On Fri, 20 Jan 2023 13:46:45 +0100 Nuno Sá <nuno.sa@analog.com> wrote: > The interrupt is triggered on the falling edge rather than being a level > low interrupt. > > Fixes: da4d3d6bb9f6 ("iio: adc: ad-sigma-delta: Allow custom IRQ flags") > Signed-off-by: Nuno Sá <nuno.sa@analog.com> What are the symptoms of this? Given the ad_sigma_delta.c irq handler disables the interrupt until after the data read is done (at which point the level is presumably high again), I don't immediately see why the change here has any impact - either we trigger on the fall, or on the fact it has become low.. Or is there a board other there that only does end interrupts that is causing problems? Jonathan > --- > drivers/iio/adc/ad7791.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iio/adc/ad7791.c b/drivers/iio/adc/ad7791.c > index fee8d129a5f0..86effe8501b4 100644 > --- a/drivers/iio/adc/ad7791.c > +++ b/drivers/iio/adc/ad7791.c > @@ -253,7 +253,7 @@ static const struct ad_sigma_delta_info ad7791_sigma_delta_info = { > .has_registers = true, > .addr_shift = 4, > .read_mask = BIT(3), > - .irq_flags = IRQF_TRIGGER_LOW, > + .irq_flags = IRQF_TRIGGER_FALLING, > }; > > static int ad7791_read_raw(struct iio_dev *indio_dev,
On Sat, 2023-01-21 at 16:58 +0000, Jonathan Cameron wrote: > On Fri, 20 Jan 2023 13:46:45 +0100 > Nuno Sá <nuno.sa@analog.com> wrote: > > > The interrupt is triggered on the falling edge rather than being a > > level > > low interrupt. > > > > Fixes: da4d3d6bb9f6 ("iio: adc: ad-sigma-delta: Allow custom IRQ > > flags") > > Signed-off-by: Nuno Sá <nuno.sa@analog.com> > > What are the symptoms of this? Given the ad_sigma_delta.c irq > handler > disables the interrupt until after the data read is done (at which > point the > level is presumably high again), I don't immediately see why the > change > here has any impact - either we trigger on the fall, or on the fact > it > has become low.. > > Honestly I did not checked this in any HW. This was just by inspecting the datasheet and confirming that the LOW IRQ is not coherent with what we have in other sigma delta ADCs. However, after some git blaming, I found this [1] which shows that this might be an issue... Hmm, maybe makes sense to add a link to the bellow patch in the commit description... [1]:https://lore.kernel.org/linux-iio/20200113102653.20900-3-alexandru.tachici@analog.com/ - Nuno Sá
On Mon, 2023-01-30 at 10:01 +0100, Nuno Sá wrote: > On Sat, 2023-01-21 at 16:58 +0000, Jonathan Cameron wrote: > > On Fri, 20 Jan 2023 13:46:45 +0100 > > Nuno Sá <nuno.sa@analog.com> wrote: > > > > > The interrupt is triggered on the falling edge rather than being > > > a > > > level > > > low interrupt. > > > > > > Fixes: da4d3d6bb9f6 ("iio: adc: ad-sigma-delta: Allow custom IRQ > > > flags") > > > Signed-off-by: Nuno Sá <nuno.sa@analog.com> > > > > What are the symptoms of this? Given the ad_sigma_delta.c irq > > handler > > disables the interrupt until after the data read is done (at which > > point the > > level is presumably high again), I don't immediately see why the > > change > > here has any impact - either we trigger on the fall, or on the fact > > it > > has become low.. > > > > > > Honestly I did not checked this in any HW. This was just by > inspecting > the datasheet and confirming that the LOW IRQ is not coherent with > what > we have in other sigma delta ADCs. > > However, after some git blaming, I found this [1] which shows that > this > might be an issue... > > Hmm, maybe makes sense to add a link to the bellow patch in the > commit > description... > > [1]: > https://lore.kernel.org/linux-iio/20200113102653.20900-3-alexandru.tachici@analog.com > / > > - Nuno Sá Hi Jonathan, Anything that I should do in this one? As I did not tested it, it might not be a real issue but I still think the patch is good even though it might not deserve a Fixes tag... - Nuno Sá
On Mon, 06 Feb 2023 12:13:50 +0100 Nuno Sá <noname.nuno@gmail.com> wrote: > On Mon, 2023-01-30 at 10:01 +0100, Nuno Sá wrote: > > On Sat, 2023-01-21 at 16:58 +0000, Jonathan Cameron wrote: > > > On Fri, 20 Jan 2023 13:46:45 +0100 > > > Nuno Sá <nuno.sa@analog.com> wrote: > > > > > > > The interrupt is triggered on the falling edge rather than being > > > > a > > > > level > > > > low interrupt. > > > > > > > > Fixes: da4d3d6bb9f6 ("iio: adc: ad-sigma-delta: Allow custom IRQ > > > > flags") > > > > Signed-off-by: Nuno Sá <nuno.sa@analog.com> > > > > > > What are the symptoms of this? Given the ad_sigma_delta.c irq > > > handler > > > disables the interrupt until after the data read is done (at which > > > point the > > > level is presumably high again), I don't immediately see why the > > > change > > > here has any impact - either we trigger on the fall, or on the fact > > > it > > > has become low.. > > > > > > > > > > Honestly I did not checked this in any HW. This was just by > > inspecting > > the datasheet and confirming that the LOW IRQ is not coherent with > > what > > we have in other sigma delta ADCs. > > > > However, after some git blaming, I found this [1] which shows that > > this > > might be an issue... > > > > Hmm, maybe makes sense to add a link to the bellow patch in the > > commit > > description... > > > > [1]: > > https://lore.kernel.org/linux-iio/20200113102653.20900-3-alexandru.tachici@analog.com > > / > > > > - Nuno Sá > > Hi Jonathan, > > Anything that I should do in this one? As I did not tested it, it might > not be a real issue but I still think the patch is good even though it > might not deserve a Fixes tag... > > - Nuno Sá Applied to the fixes-togreg rbanch of iio.git. I left the fixes tag but just to be awkward didn't mark it for stable (it'll get picked up anyway probably but I didn't want to imply there was any rush in doing so ;) Thanks, Jonathan
diff --git a/drivers/iio/adc/ad7791.c b/drivers/iio/adc/ad7791.c index fee8d129a5f0..86effe8501b4 100644 --- a/drivers/iio/adc/ad7791.c +++ b/drivers/iio/adc/ad7791.c @@ -253,7 +253,7 @@ static const struct ad_sigma_delta_info ad7791_sigma_delta_info = { .has_registers = true, .addr_shift = 4, .read_mask = BIT(3), - .irq_flags = IRQF_TRIGGER_LOW, + .irq_flags = IRQF_TRIGGER_FALLING, }; static int ad7791_read_raw(struct iio_dev *indio_dev,
The interrupt is triggered on the falling edge rather than being a level low interrupt. Fixes: da4d3d6bb9f6 ("iio: adc: ad-sigma-delta: Allow custom IRQ flags") Signed-off-by: Nuno Sá <nuno.sa@analog.com> --- drivers/iio/adc/ad7791.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)