Devicetree
 help / color / mirror / Atom feed
* [PATCH v10 0/2] iio: health: add MAX86150 ECG and PPG biosensor driver
@ 2026-07-17 20:11 Md Shofiqul Islam
  2026-07-17 20:11 ` [PATCH v10 1/2] dt-bindings: iio: health: add adi,max86150 Md Shofiqul Islam
  2026-07-17 20:11 ` [PATCH v10 2/2] iio: health: add MAX86150 ECG and PPG biosensor driver Md Shofiqul Islam
  0 siblings, 2 replies; 5+ messages in thread
From: Md Shofiqul Islam @ 2026-07-17 20:11 UTC (permalink / raw)
  To: linux-iio
  Cc: Md Shofiqul Islam, jic23, devicetree, robh, krzk+dt, conor+dt,
	andriy.shevchenko, u.kleine-koenig, joshua.crofts1, nuno.sa,
	Michael.Hennerich, dlechner, linux, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 7047 bytes --]

Sorry for the long gap since v9. Between then and now the driver's
buffer implementation was rewritten from the kfifo-based design onto
the standard IIO hardware-trigger and triggered-buffer framework, and
I read through every open comment from every reviewer before sending
this rather than sending again too quickly.

I used AI assistance (Claude, Sonnet 5) while working through this, for
understanding kernel conventions and cross-checking against subsystem
patterns; that's disclosed via Assisted-by: Claude:claude-sonnet-5 tags
on both patches per Documentation/process/coding-assistants.rst, and
the design decisions and responsibility for what's here are mine.

Both patches now build and link cleanly (make M=drivers/iio/health),
checkpatch.pl reports 0 errors/0 warnings, and pahole confirms the
driver's private struct is 56 bytes/1 cacheline with only the expected
padding for timestamp alignment.

Testing: booted a kernel with this driver under virtme-ng/QEMU against
an i2c-stub device. Confirmed: probe correctly aborts with -ENODEV on
a part-ID mismatch (stub registers default to 0) and correctly binds
once the stub's PART_ID register is preset to 0x1E; the resulting
IIO device exposes the expected in_intensity_red_raw/in_intensity_ir_raw/
in_voltage_raw channels; direct reads on those channels time out
cleanly through the IIO_DEV_ACQUIRE_DIRECT_MODE()/regmap_read_poll_timeout()
path (i2c-stub never asserts PPG_RDY, so a ~25ms ETIMEDOUT is expected,
not a hang or crash); and module removal runs devm cleanup correctly
(SYS_CTRL read back with SHDN set after unbind, clean rmmod).

Not tested: i2c-stub has no interrupt line, so the actual
interrupt-driven path (max86150_trigger_enable(), the threaded
max86150_trigger_handler() FIFO drain, and buffer enable/disable) was
not exercised. That's the part of this series with the most new logic
and I don't have real hardware on hand to test it against; flagging
this explicitly rather than implying full coverage.

Changes since v9:
- Rewrote the buffer implementation around devm_iio_trigger_alloc() +
  devm_iio_triggered_buffer_setup() instead of the kfifo buffer and
  manual postenable/predisable pair. The hard-irq handler now only
  reads and clears INT_STATUS1 before calling iio_trigger_poll();
  the threaded trigger handler does the FIFO drain. This relies on
  the trigger core's own attach/detach synchronization instead of an
  explicit iio_buffer_enabled() guard or synchronize_irq(), which the
  old design needed to avoid a NULL active_scan_mask race on teardown
  (Sashiko, both HIGH severity findings)
- Clear the stale A_FULL status bit before arming INT_ENABLE1, and
  clear stale PPG_RDY before polling in the direct-read path; both
  prevent a previously-latched flag from firing against garbage state
  (Sashiko HIGH; Joshua)
- Use iio_get_time_ns(indio_dev) instead of ktime_get_ns() so
  timestamps respect the configured IIO clock source (Sashiko MEDIUM)
- Zero the scan buffer before each push so disabled channels don't
  leak stale data (Sashiko LOW)
- Added array_size.h, err.h, types.h; added a blank line before the
  iio/ include group (Andy Shevchenko, Joshua)
- Dropped the free-standing FIFO_A_FULL_VAL/PPG_SR_100HZ/ADC_RGE_16384
  single-value defines; inlined the FIFO threshold at its point of use
  with an explanatory comment, and listed the full set of PPG_SR
  (naming the single-/double-pulse variants) and PPG_ADC_RGE register
  values instead of just the one in use (Jonathan Cameron)
- Removed the unneeded __aligned(IIO_DMA_MINALIGN) on the plain I2C
  FIFO read buffer -- that alignment is for the IIO DMA buffer path,
  not a bounce-buffer requirement I2C opts into (Jonathan Cameron)
- Switched read_raw() to IIO_DEV_ACQUIRE_DIRECT_MODE()/
  IIO_DEV_ACQUIRE_FAILED(), with the read logic pulled into a
  max86150_do_read_raw() helper (Jonathan Cameron)
- Dropped irq_get_trigger_type(): the device has no register to
  reconfigure interrupt polarity or type, so IRQF_ONESHOT alone is
  sufficient (Jonathan Cameron)
- Checked max86150_powerdown()'s regmap calls and log failures with
  dev_warn(); fixed it to use regmap_set_bits() instead of a plain
  regmap_write() for the shutdown bit so it no longer clobbers the
  rest of SYS_CTRL (Joshua)
- Added blank lines between each regmap write + error-check block for
  readability (Joshua)
- Declared the FIFO-drain loop variable at point of use (Joshua)
- Added a datasheet-referenced comment and blank line around the
  SYS_RESET self-clear delay (Joshua)
- Moved the sample_period_ns assignment next to the PPG_CONFIG1 write
  it's derived from, instead of sitting among unrelated LED_PA writes
  (Jonathan Cameron)
- Trimmed the redundant "get/" from regulator failure messages, since
  devm_regulator_get_enable() is a single combined call (Joshua)
- Fixed the MAINTAINERS entry to sort alphabetically among the other
  MAX-prefixed entries (Sashiko LOW)
- Removed the Kconfig paragraph describing the driver's channels
  instead of the hardware (Joshua)
- Added the avdd-supply and vref-supply properties and renamed
  vled-supply to leds-supply, to cover all four independent supply
  rails in the datasheet power tree (found while re-verifying the
  binding against the driver)
- Fixed the DT binding example's interrupt flag from
  IRQ_TYPE_EDGE_FALLING to IRQ_TYPE_LEVEL_LOW to match the driver's
  actual level-triggered interrupt handling
- Fixed .validate_trigger in iio_info to iio_validate_own_trigger();
  the previous iio_trigger_validate_own_device() has its arguments in
  the opposite order and belongs on iio_trigger_ops.validate_device,
  where it's used correctly. Caught by actually building the module,
  not just reading the diff.
- Threaded this series properly with git send-email --thread this
  time (Andy Shevchenko, Krzysztof Kozlowski)
- Added Nuno Sá, Michael Hennerich and David Lechner to Cc, per
  scripts/get_maintainer.pl -- this is an Analog Devices part and I'd
  missed CCing ADI's own IIO maintainers on every version so far
- Part-ID mismatch now aborts probe with dev_err_probe(dev, -ENODEV,
  ...) instead of logging and continuing. This goes beyond the
  dev_info() vs dev_warn() log-level question raised in review: a
  mismatched part ID means we're either talking to the wrong device or
  something on the bus is wrong, and running chip_init() writes
  against an unverified chip seemed like the riskier default (Joshua)

Md Shofiqul Islam (2):
  dt-bindings: iio: health: add adi,max86150
  iio: health: add MAX86150 ECG and PPG biosensor driver

 .../bindings/iio/health/adi,max86150.yaml     |  78 ++
 MAINTAINERS                                   |   7 +
 drivers/iio/health/Kconfig                    |  13 +
 drivers/iio/health/Makefile                   |   1 +
 drivers/iio/health/max86150.c                 | 691 ++++++++++++++++++
 5 files changed, 790 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/health/adi,max86150.yaml
 create mode 100644 drivers/iio/health/max86150.c

--
2.51.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-17 20:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-17 20:11 [PATCH v10 0/2] iio: health: add MAX86150 ECG and PPG biosensor driver Md Shofiqul Islam
2026-07-17 20:11 ` [PATCH v10 1/2] dt-bindings: iio: health: add adi,max86150 Md Shofiqul Islam
2026-07-17 20:20   ` sashiko-bot
2026-07-17 20:11 ` [PATCH v10 2/2] iio: health: add MAX86150 ECG and PPG biosensor driver Md Shofiqul Islam
2026-07-17 20:30   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox