mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Md Shofiqul Islam <shofiqtest@gmail.com>
To: linux-iio@vger.kernel.org
Cc: Md Shofiqul Islam <shofiqtest@gmail.com>,
	jic23@kernel.org, devicetree@vger.kernel.org, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org,
	andriy.shevchenko@intel.com, u.kleine-koenig@baylibre.com,
	joshua.crofts1@gmail.com, nuno.sa@analog.com,
	Michael.Hennerich@analog.com, dlechner@baylibre.com,
	linux@analog.com, linux-kernel@vger.kernel.org
Subject: [PATCH v10 0/2] iio: health: add MAX86150 ECG and PPG biosensor driver
Date: Fri, 17 Jul 2026 23:11:35 +0300	[thread overview]
Message-ID: <20260717201138.1078019-1-shofiqtest@gmail.com> (raw)

[-- 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

             reply	other threads:[~2026-07-17 20:11 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-17 20:11 Md Shofiqul Islam [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260717201138.1078019-1-shofiqtest@gmail.com \
    --to=shofiqtest@gmail.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=andriy.shevchenko@intel.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=joshua.crofts1@gmail.com \
    --cc=krzk+dt@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@analog.com \
    --cc=nuno.sa@analog.com \
    --cc=robh@kernel.org \
    --cc=u.kleine-koenig@baylibre.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox