| Message ID | 20210729084731.79135-1-alexandru.tachici@analog.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | iio: adc: Fix flags in sigma-delta drivers | expand |
On Thu, 29 Jul 2021 11:47:28 +0300 <alexandru.tachici@analog.com> wrote: > From: Alexandru Tachici <alexandru.tachici@analog.com> > > Some sigma-delta drivers use wrong irq_flags specified in the > ad_sigma_delta_info struct. Add the flags corresponding to the > interrupt type specified in the data-sheets of each chip. > The complexity here is that we normally now leave this to the firmware description of the interrupt. The reason for that is people have an annoying habit of building boards where there is an inverter used as a cheap level converter on the interrupt line. It an also be somewhat 'fun' to identify from a datasheet if the signal is actually an edge interrupt or a level interrupt and in the case of devices that don't have any autonomous triggering (i.e. won't grab new data unless you tell the to) the difference is irrelevant. Take the ad7780 for example. It has an interrupt that remains low unless a) The data is read - normal case I'd imagine b) New data capture occurs (slowly at 10Hz) So we aren't dealing with a short pulse, or a situation where we have an interrupt that will stay set after the data is read (both of which would make this definitely an edge interrupt to avoid possibility of either missing or double triggering). Hence whilst it will work as an edge interrupt, it will also work fine with the existing level interrupt and in some ways level is more appropriate as the interrupt will remain set if you don't read it (for a while anyway and after that it's not safe to read). For extra fun / background for anyone else reading this thread, this interrupt line is also the data out line, so we absolutely 'must' keep the interrupt disabled until we are done with the read out, but that's handled in the ad_sigma_delta core. Conclusion, I'm not sure these are actually wrong, and if we were doing this today we wouldn't have them anyway... So have we seen any problems with current code? Jonathan > Alexandru Tachici (3): > iio: adc: ad7192: Fix IRQ flag > iio: adc: ad7780: Fix IRQ flag > iio: adc: ad7923: Fix IRQ flag > > drivers/iio/adc/ad7192.c | 1 + > drivers/iio/adc/ad7780.c | 2 +- > drivers/iio/adc/ad7793.c | 2 +- > 3 files changed, 3 insertions(+), 2 deletions(-) > > -- > 2.25.1
In Sigma-Delta devices the SDO line is also used as an interrupt. Leaving IRQ on level instead of falling might trigger a sample read when the IRQ is enabled, as the SDO line is already low. Not sure if SDO line will always imediately go high in ad_sd_buffer_postenable before the IRQ is enabled. Also the datasheets seem to explicitly say the falling edge of the SDO should be used as an interrupt. -Alexandru
On Tue, 3 Aug 2021 12:17:41 +0300 <alexandru.tachici@analog.com> wrote: > In Sigma-Delta devices the SDO line is also used as an interrupt. > Leaving IRQ on level instead of falling might trigger a sample read > when the IRQ is enabled, as the SDO line is already low. Not sure > if SDO line will always imediately go high in ad_sd_buffer_postenable > before the IRQ is enabled. ok. That last bit sounds like a good justification. It might do the wrong thing on initial startup. Please resend with that in the patch descriptions for those you are modifying. For those where it is being introduced, we probably want to leave the interrupt type to firmware (i.e. patch 1) > > Also the datasheets seem to explicitly say the falling edge of the SDO > should be used as an interrupt. Reference? > > -Alexandru
From: Alexandru Tachici <alexandru.tachici@analog.com> Some sigma-delta drivers use wrong irq_flags specified in the ad_sigma_delta_info struct. Add the flags corresponding to the interrupt type specified in the data-sheets of each chip. Alexandru Tachici (3): iio: adc: ad7192: Fix IRQ flag iio: adc: ad7780: Fix IRQ flag iio: adc: ad7923: Fix IRQ flag drivers/iio/adc/ad7192.c | 1 + drivers/iio/adc/ad7780.c | 2 +- drivers/iio/adc/ad7793.c | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) -- 2.25.1