mbox series

[v3,00/10] iio: adc: ad7124: Various fixes

Message ID 20241122113322.242875-12-u.kleine-koenig@baylibre.com (mailing list archive)
Headers show
Series iio: adc: ad7124: Various fixes | expand

Message

Uwe Kleine-König Nov. 22, 2024, 11:33 a.m. UTC
Hello,

this series contains all the yet unapplied patches I created to improve
support for the ad7124. I hope collecting them all in a single series is
fine and simplifies application. It superseeds the following series:

	https://lore.kernel.org/linux-iio/cover.1731404695.git.u.kleine-koenig@baylibre.com
	https://lore.kernel.org/linux-iio/20241119183611.56820-2-u.kleine-koenig@baylibre.com
	https://lore.kernel.org/linux-iio/20241028160748.489596-6-u.kleine-koenig@baylibre.com

It also benefits the other ADCs making use of the ad_sigma_delta
helpers. v6.12 is used as a base plus a fix that is already in next as
64612ec9b909b699293b7220c634f67a9fc12e06.

Changes since the previous submissions:

 - Rebase to v6.12 (was v6.12-rc1 before)
 - Rewording of some commit logs to match the actual changes
 - The last five patches are new.

The rdy-gpio patch was discussed controversially before, but I'm still
convinced that it's correct to do it this way. IMHO this is also
confirmed in the discussion with tglx at
https://lore.kernel.org/all/io53lznz3qp3jd5rohqsjhosnmdzd6d44sdbwu5jcfrs3rz2a2@orquwgflrtyc/
. Also while this fix doesn't work on all platforms (because not all irq
controllers can expose the DOUT/̅R̅D̅Y line as a GPIO in addition to
triggering the irq), it is definitively useful on the platforms where it
does work. The alternative is polling instead of using the irq which I'm
sure comes at a performance cost. (And even for polling, having the GPIO
reading the DOUT/̅R̅D̅Y line is beneficial as polling a GPIO is easier and
cheaper than polling a register bit via spi.)

The patch "Fix a race condition" is supposed to cure the following
warning that triggered twice for me:

	[   86.807282] ------------[ cut here ]------------
	[   86.811902] WARNING: CPU: 1 PID: 291 at kernel/irq/manage.c:790 __enable_irq+0x68/0x8c
	[   86.819819] Unbalanced enable for IRQ 35
	[   86.823729] Modules linked in:
	[   86.826784] CPU: 1 UID: 0 PID: 291 Comm: iio_readdev Not tainted 6.12.0-rc1+ #63
	[   86.834158] Hardware name: Altera SOCFPGA
	[   86.838156] Call trace: 
	[   86.838166]  unwind_backtrace from show_stack+0x18/0x1c
	[   86.845924]  show_stack from dump_stack_lvl+0x54/0x68
	[   86.850985]  dump_stack_lvl from __warn+0x88/0x11c
	[   86.855792]  __warn from warn_slowpath_fmt+0x128/0x194
	[   86.860937]  warn_slowpath_fmt from __enable_irq+0x68/0x8c
	[   86.866430]  __enable_irq from enable_irq+0x54/0xac
	[   86.871313]  enable_irq from ad_sd_buffer_postenable+0x120/0x18c
	[   86.877321]  ad_sd_buffer_postenable from __iio_update_buffers+0x688/0xa24
	[   86.884187]  __iio_update_buffers from enable_store+0x8c/0xd4
	[   86.889925]  enable_store from kernfs_fop_write_iter+0x130/0x1fc
	[   86.895928]  kernfs_fop_write_iter from vfs_write+0x258/0x404
	[   86.901671]  vfs_write from ksys_write+0x78/0xfc
	[   86.906287]  ksys_write from ret_fast_syscall+0x0/0x1c
	[   86.911420] Exception stack(0xf0a45fa8 to 0xf0a45ff0)
	[   86.916460] 5fa0:                   00000002 00b98f80 00000005 00b98f80 00000002 00000000
	[   86.924611] 5fc0: 00000002 00b98f80 00000005 00000004 b6ee3b94 00b97960 00000002 bee7442c
	[   86.932758] 5fe0: 00000004 bee73f20 b6e613e9 b6dd2776
	[   86.937793] ---[ end trace 0000000000000000 ]---

While I didn't find a way to reproduce this problem, I'm convinced the
patch fixes real race conditions like the one above.

The patch "Check for previous ready signals" fixes a similar problem as
the rdy-gpios patch. In that case however the immediate irq is triggered
by a previous measurement that didn't complete in time (e.g. because the
sysfs read was Ctrl-C'd). This one cannot be detected with the rdy-gpio
approach because in this case the device actually reports a pending irq.

The last patch isn't a fix but provides (part of) the motivation for
this series. Adding temperature support in half a work day was my
initial plan, all the fixes were found while implementing and testing
that one on a de10-nano board.

I didn't mark the patches for application to stable, because I think
this should be a maintainer decision, but if you ask me: backport all
but the last one. (Patches 3 and 7 are not fixes, but prerequisites for
the following patches.)

Best regards
Uwe

Uwe Kleine-König (10):
  iio: adc: ad7124: Don't create more channels than the driver can handle
  iio: adc: ad7124: Refuse invalid input specifiers
  dt-bindings: iio: adc: adi,ad7124: Allow specifications of a gpio for irq line
  iio: adc: ad_sigma_delta: Add support for reading irq status using a GPIO
  iio: adc: ad_sigma_delta: Handle CS assertion as intended in ad_sd_read_reg_raw()
  iio: adc: ad_sigma_delta: Fix a race condition
  iio: adc: ad_sigma_delta: Store information about reset sequence length
  iio: adc: ad_sigma_delta: Check for previous ready signals
  iio: adc: ad7124: Add error reporting during probe
  iio: adc: ad7124: Implement temperature measurement

 .../bindings/iio/adc/adi,ad7124.yaml          |   8 +
 drivers/iio/adc/ad7124.c                      | 219 +++++++++++++-----
 drivers/iio/adc/ad7173.c                      |   1 +
 drivers/iio/adc/ad7192.c                      |   4 +-
 drivers/iio/adc/ad7791.c                      |   1 +
 drivers/iio/adc/ad7793.c                      |   3 +-
 drivers/iio/adc/ad_sigma_delta.c              | 184 ++++++++++++---
 include/linux/iio/adc/ad_sigma_delta.h        |   7 +-
 8 files changed, 338 insertions(+), 89 deletions(-)

base-commit: adc218676eef25575469234709c2d87185ca223a
prerequisite-patch-id: 617af17fc377a984762c61893b9f2a92ae62213a