Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <6d58a58cd7e8a7cbb91b2658e9a85b44b34b64dc.camel@gmail.com>
Subject: Re: iio: ad7192: Pending IRQ on SDO/RDY causing retrigger of
 interrupt handler and missing every sample
From:   Nuno =?ISO-8859-1?Q?S=E1?= <noname.nuno@gmail.com>
To:     Fabrizio Lamarque <fl.scratchpad@gmail.com>,
        alexandru.tachici@analog.com, linux-kernel@vger.kernel.org
Cc:     Jonathan Cameron <jic23@kernel.org>
Date:   Wed, 05 Apr 2023 12:32:45 +0200
In-Reply-To: <CAPJMGm4GaSjD6bdqMwCr2EVZGenWzT-nCCf3BMRaD1TSfAabpA@mail.gmail.com>
References: <CAPJMGm4GaSjD6bdqMwCr2EVZGenWzT-nCCf3BMRaD1TSfAabpA@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.4 
MIME-Version: 1.0
Precedence: bulk

Hi Fabrizio,

Thanks for reporting this...

On Wed, 2023-04-05 at 11:53 +0200, Fabrizio Lamarque wrote:
> Link:
> https://lore.kernel.org/all/20210906065630.16325-3-alexandru.tachici@anal=
og.com/
>=20
> This commit broke the driver functionality, i.e. cat in_voltage1_raw
> triggers a correct sampling sequence only the first time, then it
> always returns the first sampled value.
>=20
> Following the sequence of ad_sigma_delta_single_conversion() within
> ad_sigma_delta.c, this is due to:
>=20
> - IRQ resend mechanism is always enabled for ARM cores
> (CONFIG_HARDIRQS_SW_RESEND)
> - Edge IRQs are always made pending when a corresponding event
> happens, even after disable_irq(). This is intentional and designed to
> prevent missing signal edges.
> - Level IRQs are not impacted by IRQ resend (i.e. IRQ_PENDING is
> always cleared).
> - SPI communication causes the IRQ to be set pending (even if
> corresponding interrupt is disabled)
> - The second time ad_sigma_delta_single_conversion() is called,
> enable_irq() will trigger the interrupt immediately, even if RDY line
> is high.
> - In turn, this causes the call
> wait_for_completion_interruptible_timeout() to exit immediately, with
> RDY line still high.
> - Right after the SPI register read, the MODE register is written with
> AD_SD_MODE_IDLE, and pending conversion is stopped. Hence DATA
> register is never updated.
>=20
> I would suggest to revert this commit or set the flag with
> IRQF_TRIGGER_LOW instead of IRQF_TRIGGER_FALLING, but I am not sure
> about the problem solved by this commit and how to replicate it.
> Another option would be to keep IRQ flags within bindings only.
>=20

I'm starting to think that what's stated in that commit=C2=A0

"Leaving IRQ on level instead of falling might trigger a sample read
when the IRQ is enabled, as the SDO line is already low. "

might actually be related with something else...

> As a side note, AD7192 datasheet says that the falling edge on SDO
> line _can_ be used as an interrupt to processor, but it also states
> that the _level_ on SDO/RDY line indicates the sample is ready. What
> happens on SDO/RDY interrupt line before the ADC conversion is
> triggered should be ignored.
>=20

Well, from the datasheet (as you already know):

"
In addition, DOUT/RDY operates as a data ready pin, going low to indicate
the completion of a conversion. If the data is not read after the conversio=
n,
the pin goes high before the next
update occurs. The DOUT/RDY falling edge can be used as an interrupt to a
processor, indicating that valid
data is available.
"

So I really read IRQF_TRIGGER_FALLING in the above and all the other sigma
delta=C2=A0devices have the same setting (I think). So the fact that it wor=
ks with
level IRQs might just be lucky instead of fixing the real problem. Can you =
try
the below patch (I'm starting to think it might be related):


https://lore.kernel.org/linux-iio/20230306044737.862-1-honda@mechatrax.com/


- Nuno S=C3=A1