* Re: [PATCH v2 0/4] Introduce HONOR MagicBook Art 14 device tree
From: Valentin Manea @ 2026-06-23 18:27 UTC (permalink / raw)
To: mail
Cc: andersson, conor+dt, devicetree, konradybcio, krzk+dt,
linux-arm-msm, linux-kernel, robh
In-Reply-To: <20260515172926.16597-1-mail@etehtsea.me>
Hi
> I'm using it with an external display connected to the HDMI port as > a daily driver for quite some time now. > > eDP support is broken
(screen is eigher grey or grey with noise), > thus, keeping it disabled
for now.
The reason for the eDP problems is that the firmware enables DSC for
the panel at boot, but the msm driver does not yet support it. There are
some patches floating around to add the functionality but they seem to
have some problems. In the repo with my name that you quoted I have
integrated them for testing, but as they are they already have been
rejected for mainline, so I think we just need to wait for qualcomm or
linaro to finish their work.
Now I have a simpler solution for this problem, I just made a copy of the
one of the eDP panel drivers and do a force disable of DSC on resume,
which seems to work perfectly fine, there is enough bandwidth to
drive the panel even at 120Hz even with DSC disabled. You can find my
patch in the quoted repository in the qcom-x1e-7.0 branch.
If this is an acceptable solution I will cleanup the patch more and
post it here. Perhaps the other solution is for the msm driver to
disable DSC explicitly for panels that advertise it, because it might
be enabled by bootloaders/firmware.
Valentin
^ permalink raw reply
* Re: [PATCH v2 05/12] nvmem: microchip-otpc: add tag-based packet lookup
From: Andy Shevchenko @ 2026-06-23 18:31 UTC (permalink / raw)
To: Varshini Rajendran
Cc: ehristev, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt,
nicolas.ferre, alexandre.belloni, claudiu.beznea, srini,
linux-iio, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20260623105944.128840-6-varshini.rajendran@microchip.com>
On Tue, Jun 23, 2026 at 04:29:37PM +0530, Varshini Rajendran wrote:
> Add support for accessing OTP packets by their 4-byte ASCII tag while
> preserving backward compatibility with the existing ID-based lookup.
>
> The OTP memory layout can vary across devices and may change over time,
> making the packet ID approach unreliable when the memory map is not
> known in advance. The packet tag provides a reliable way to identify
> and access packets without prior knowledge of the OTP memory layout.
>
> Two offset encoding are now supported:
> 1. Legacy ID-based: offset = OTP_PKT(id) = id * 4
> Used in DT as: reg = <OTP_PKT(1) 76>;
> 2. TAG-based: offset = 4-byte ASCII packet tag
> Used in DT as: reg = <0x41435354 0x4c>; (tag "ACST")
>
> The driver resolves offsets matching valid legacy selectors (multiples
> of 4 within the packet count) through ID lookup, falling back to tag
> lookup for other values. This ensures existing device trees continue
> to work while enabling new tag-based access.
>
> During probe, packet meta data including the tag is read and cached.
> The driver also validates OTP memory accessibility and emulation mode
> status. When the boot packet is not configured, emulation mode allows
> access to the other packets. When both are not available an
> informational message is logged.
>
> The stride of the nvmem memory is set to 1 in order to support tag based
> offsets, comment in the header file is updated accordingly.
...
> #define MCHP_OTPC_SIZE (11 * 1024)
Side note: At some point maybe (11 * SZ_1K) with help of sizes.h?
...
> +/**
> + * mchp_otpc_resolve_packet() - resolve offset to packet
> + * @otpc: OTPC private data
> + * @off: NVMEM offset (legacy ID-based or TAG-based)
> + *
> + * Legacy offsets (multiples of 4 within valid ID range) are resolved
> + * through ID lookup. Other offsets are treated as 4-byte ASCII tags.
> + *
> + * Return: pointer to packet if found, NULL otherwise
> + */
> +static struct mchp_otpc_packet *mchp_otpc_resolve_packet(struct mchp_otpc *otpc,
> + u32 off)
> +{
> + /*
> + * Legacy id based packet access: offset = id * 4
> + * Inside the driver we use continuous unsigned integer numbers
> + * for packet id, thus divide off by 4 before passing it to
> + * mchp_otpc_id_to_packet().
> + */
> +
> + if (!(off % 4) && (off / 4) < otpc->npackets)
> + return mchp_otpc_id_to_packet(otpc, off / 4);
Hmm... I was thinking about something like temporary variables for these two.
Note, in some cases the compiler may issue a single instruction when it sees
both together. That's why in many GPIO drivers we use something like
unsigned int offset = foo / 8;
unsigned int shift = foo % 8;
or similar.
> + /*
> + * TAG-based packet access: offset is a 4-byte ASCII tag
> + */
> + return mchp_otpc_tag_to_packet(otpc, off);
> +}
...
> {
> struct nvmem_device *nvmem;
> struct mchp_otpc *otpc;
> - u32 size;
> + u32 size, tmp;
> int ret;
> + bool emul_enable;
Perhaps keep the reversed xmas tree order?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 03/12] iio: adc: at91-sama5d2_adc: adapt the driver for sama7d65
From: Andy Shevchenko @ 2026-06-23 18:22 UTC (permalink / raw)
To: Varshini Rajendran
Cc: ehristev, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt,
nicolas.ferre, alexandre.belloni, claudiu.beznea, srini,
linux-iio, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20260623105944.128840-4-varshini.rajendran@microchip.com>
On Tue, Jun 23, 2026 at 04:29:35PM +0530, Varshini Rajendran wrote:
> Add support for sama7d65 ADC. The differences are highlighted with the
> compatible. The calibration data layout is the main difference.
Do you need to update a Kconfig help text?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re:[PATCH 1/2] dt-bindings: hwmon: chipcap2: Add label property
From: Flaviu Nistor @ 2026-06-23 18:16 UTC (permalink / raw)
To: Javier Carrasco
Cc: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet, Shuah Khan, Flaviu Nistor, linux-hwmon,
linux-kernel, devicetree, linux-doc
In-Reply-To: <DJFPYCV2FXW7.1BFG9DURPZRCC@gmail.com>
On Mon Jun 22, 2026 at 7:29 PM CEST, Javier Carrasco wrote:
>On Mon Jun 22, 2026 at 2:21 PM CEST, Flaviu Nistor wrote:
>> Add support for an optional label property similar to other hwmon devices
>> This allows, in case of boards with multiple CHIPCAP2 sensors, to assign
>> distinct names to each instance.
>>
>> Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
>> ---
>> .../devicetree/bindings/hwmon/amphenol,chipcap2.yaml | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.ya=
>ml b/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
>> index 17351fdbefce..f00b5a4b14dd 100644
>> --- a/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
>> +++ b/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
>> @@ -33,6 +33,10 @@ properties:
>> reg:
>> maxItems: 1
>>
>> + label:
>> + description:
>> + A descriptive name for this channel, like "ambient" or "psu".
>> +
>> interrupts:
>> items:
>> - description: measurement ready indicator
>> @@ -72,6 +76,7 @@ examples:
>> <5 IRQ_TYPE_EDGE_RISING>,
>> <6 IRQ_TYPE_EDGE_RISING>;
>> interrupt-names =3D "ready", "low", "high";
>> + label =3D "somelabel";
>> vdd-supply =3D <®_vdd>;
>> };
> };
>
>Hello Falviu, thank you for your patch.
>
Hello Javier, thanks for your reply.
>Should we not add a reference to hwmon-common.yaml (with
>unevelautedProperties instead of additionalProperties), as label is
>defined there? I believe that Krzysztof Kozlowski did something similar
>for the shunt-resistor-micro-ohms property. Could we follow suit here?
>
This is a good question and I am happy you asked. I also thought a lot
about this and the reason I decided to go for this approach is that by using
$ref: hwmon-common.yaml#, I would have to change additionalProperties: false
to unevaluatedProperties: false, which will evaluate in case it is used, also
shunt-resistor-micro-ohms property which does not apply to this sensor. At
least this is my understanding, but of course I can be wrong (I see lm75 binding
also uses $ref: hwmon-common.yaml# but shunt-resistor-micro-ohms does not apply).
>I am also not a big fan of a name like "somelabel", and a more
>meaningful name from a "real" example would look better. I know that
>some examples have already used "somelabel" as an example, but others
>have used more meaningful names too.
>
I will have to send a v2 since for the label property description I used
"channel" instead of "sensor" (detected by Sashiko AI review), so I can
use in the example section a more meaningful name like "Room" if no other
suggestion.
>Best regards,
>Javier Carrasco
Best regards,
Flaviu Nistor
^ permalink raw reply
* Re: [PATCH v2 02/12] iio: adc: at91-sama5d2_adc: rework temp calibration layout handling
From: Andy Shevchenko @ 2026-06-23 18:15 UTC (permalink / raw)
To: Varshini Rajendran
Cc: ehristev, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt,
nicolas.ferre, alexandre.belloni, claudiu.beznea, srini,
linux-iio, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20260623105944.128840-3-varshini.rajendran@microchip.com>
On Tue, Jun 23, 2026 at 04:29:34PM +0530, Varshini Rajendran wrote:
> Extend support to handle different temperature calibration layouts.
>
> Add a temperature calibration data layout structure to describe indexes
> of the factors P1, P4, P6, tag, minimum length of the packet and the
> scaling factors for P1 (mul, div) which are SoC-specific instead of the
> older non scalable id structure. This helps handle the differences in the
> same function flow and prepare the calibration data to be applied. Add
> additional condition to validate the calibration data read from the
> NVMEM cell using the TAG of the packet.
>
> Use cleanup helpers for NVMEM data buffer wherever applicable.
> +#define AT91_TEMP_CALIB_TAG_ACST 0x41435354
This seems like FourCC that can be represented in ASCII (I suppose "ACST").
Can you add a short comment on top to decode that?
/* ...blablabla... in ASCII "ACST" */
...
> struct at91_adc_platform {
> const struct at91_adc_reg_layout *layout;
> @@ -481,6 +504,7 @@ struct at91_adc_platform {
> unsigned int chan_realbits;
> unsigned int temp_chan;
> bool temp_sensor;
> + const struct at91_adc_temp_calib_layout *temp_calib_layout;
> };
Is this the best placement in accordance with `pahole` tool?
...
> - u32 *buf;
> + u32 *buf __free(kfree) = NULL;
This looks like a separate change. Why is it in this patch?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH V13 5/9] iio: imu: inv_icm42607: Add PM support for icm42607
From: Chris Morgan @ 2026-06-23 18:08 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Chris Morgan, linux-iio, andy, nuno.sa, dlechner,
jean-baptiste.maneyrol, linux-rockchip, devicetree, heiko,
conor+dt, krzk+dt, robh, andriy.shevchenko
In-Reply-To: <ajnwk_zWevY14mEO@wintermute.localhost.fail>
On Mon, Jun 22, 2026 at 09:34:10PM -0500, Chris Morgan wrote:
> On Sun, Jun 21, 2026 at 06:19:48PM +0100, Jonathan Cameron wrote:
> > On Mon, 15 Jun 2026 12:25:48 -0500
> > Chris Morgan <macroalpha82@gmail.com> wrote:
> >
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > >
> > > Add power management support for the ICM42607 device driver.
> > >
> > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > A few things from taking a look at the sashiko report:
> > https://sashiko.dev/#/patchset/20260615172554.160910-1-macroalpha82%40gmail.com
> >
> > > ---
> > > drivers/iio/imu/inv_icm42607/inv_icm42607.h | 18 +++
> > > .../iio/imu/inv_icm42607/inv_icm42607_core.c | 139 ++++++++++++++++++
> > > .../iio/imu/inv_icm42607/inv_icm42607_i2c.c | 1 +
> > > .../iio/imu/inv_icm42607/inv_icm42607_spi.c | 1 +
> > > 4 files changed, 159 insertions(+)
> > >
> > > diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607.h b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> > > index a6a58571935f..4f4f541027dc 100644
> > > --- a/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> > > +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> >
> > > @@ -334,11 +345,18 @@ struct inv_icm42607_state {
> > > #define INV_ICM42607_GYRO_STOP_TIME_MS 45
> > > #define INV_ICM42607_TEMP_STARTUP_TIME_MS 77
> > >
> > > +/*
> > > + * Suspend delay assumed from other icm42600 series device, not
> > > + * documented in datasheet.
> > > + */
> > > +#define INV_ICM42607_SUSPEND_DELAY_MS (2 * USEC_PER_MSEC)
> >
> > Sashiko had a valid comment on this. MSEC_PER_SEC seems more
> > appropriate given this is 2 seconds in milli seconds.
> >
> > > +
> > > typedef int (*inv_icm42607_bus_setup)(struct inv_icm42607_state *);
> > >
> > > extern const struct regmap_config inv_icm42607_regmap_config;
> > > extern const struct inv_icm42607_hw inv_icm42607_hw_data;
> > > extern const struct inv_icm42607_hw inv_icm42607p_hw_data;
> > > +extern const struct dev_pm_ops inv_icm42607_pm_ops;
> > >
> > > int inv_icm42607_core_probe(struct regmap *regmap,
> > > const struct inv_icm42607_hw *hw,
> > > diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> > > index 4b8e19091786..64f5d263de4f 100644
> > > --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> > > +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> > > @@ -4,6 +4,7 @@
> > > */
> > >
> > > #include <linux/bitfield.h>
> > > +#include <linux/cleanup.h>
> > > #include <linux/delay.h>
> > > #include <linux/dev_printk.h>
> > > #include <linux/device/devres.h>
> > > @@ -11,6 +12,7 @@
> > > #include <linux/iio/iio.h>
> > > #include <linux/module.h>
> > > #include <linux/mutex.h>
> > > +#include <linux/pm_runtime.h>
> > > #include <linux/regmap.h>
> > > #include <linux/regulator/consumer.h>
> > > #include <linux/time.h>
> > > @@ -103,6 +105,63 @@ const struct inv_icm42607_hw inv_icm42607p_hw_data = {
> > > };
> > > EXPORT_SYMBOL_NS_GPL(inv_icm42607p_hw_data, "IIO_ICM42607");
> > >
> > > +static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
> > > + enum inv_icm42607_sensor_mode gyro,
> > > + enum inv_icm42607_sensor_mode accel,
> > > + bool temp, unsigned int *sleep_ms)
> > > +{
> > > + enum inv_icm42607_sensor_mode oldaccel = st->conf.accel.mode;
> > > + enum inv_icm42607_sensor_mode oldgyro = st->conf.gyro.mode;
> > > + bool oldtemp = st->conf.temp_en;
> > > + unsigned int sleepval_ms;
> > > + unsigned int val;
> > > + int ret;
> > > +
> > > + if (gyro == oldgyro && accel == oldaccel && temp == oldtemp)
> > > + return 0;
> > > +
> > > + /*
> > > + * Datasheet on page 14.26 says we need to ensure the gyro sensor is on
> > > + * for a minimum of 45ms. So if we transition from an on state to an
> > > + * off state wait 45ms to ensure a sufficient pause before power off.
> >
> > Sashiko commented on this.. I think what we could do with adding to the
> > comment is what the path is that didn't pass through this function which would
> > ensure we have been on for 30 of this msecs already.
>
> I'm going to track whatever time the gyro started, and then if less
> than 45ms has elapsed just pause the remaining amount of time.
>
> >
> > > + */
> > > + if (!gyro && oldgyro)
> > > + fsleep(INV_ICM42607_GYRO_STOP_TIME_MS * USEC_PER_MSEC);
> > > +
> > > + val = FIELD_PREP(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, gyro);
> > > + val |= FIELD_PREP(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, accel);
> > > + ret = regmap_write(st->map, INV_ICM42607_REG_PWR_MGMT0, val);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + st->conf.gyro.mode = gyro;
> > > + st->conf.accel.mode = accel;
> > > + st->conf.temp_en = temp;
> > > +
> > > + /*
> > > + * If a state change occurs from off to on, sleep for the startup
> > > + * time of the sensor, unless a sleep_ms is specified. Since more
> > > + * than one sensor can be transitioned from off to on, select the
> > > + * maximum time from each of the sensors changing from off to on.
> > > + */
> > > + sleepval_ms = 0;
> > > + if (temp && !oldtemp)
> > > + sleepval_ms = max(sleepval_ms, INV_ICM42607_TEMP_STARTUP_TIME_MS);
> > > +
> > > + if (accel && !oldaccel)
> > > + sleepval_ms = max(sleepval_ms, INV_ICM42607_ACCEL_STARTUP_TIME_MS);
> > > +
> > > + if (gyro && !oldgyro)
> > > + sleepval_ms = max(sleepval_ms, INV_ICM42607_GYRO_STARTUP_TIME_MS);
> > > +
> > > + if (sleep_ms)
> > > + *sleep_ms = sleepval_ms;
> > > + else if (sleepval_ms)
> > > + fsleep(sleepval_ms * USEC_PER_MSEC);
> > > +
> > > + return 0;
> > > +}
> >
> > >
> > > int inv_icm42607_core_probe(struct regmap *regmap,
> > > @@ -236,6 +305,8 @@ int inv_icm42607_core_probe(struct regmap *regmap,
> > > if (!st)
> > > return -ENOMEM;
> > >
> > > + dev_set_drvdata(dev, st);
> > > +
> > > ret = devm_mutex_init(dev, &st->lock);
> > > if (ret)
> > > return ret;
> > > @@ -271,10 +342,78 @@ int inv_icm42607_core_probe(struct regmap *regmap,
> > > if (ret)
> > > return ret;
> > >
> > > + ret = devm_pm_runtime_set_active_enabled(dev);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + pm_runtime_set_autosuspend_delay(dev, INV_ICM42607_SUSPEND_DELAY_MS);
> > > + pm_runtime_use_autosuspend(dev);
> > Sashiko does put out some stuff here. Please take a look and work out or
> > test if it is right (I think not but haven't checked that carefully!)
> > From a quick look I think that the auto disabling of autosuspend does a
> > rpm_idle() that should result in it suspending...
> >
>
> I see a few other drivers adding one more call to
> devm_pm_runtime_enable() so I'm going to see how that works out.
Obviously that didn't work, but digging in much deeper into the PM code
I see that devm_pm_runtime_set_active_enabled() returns
devm_pm_runtime_enable(), which sets devm_add_action_or_reset() on
pm_runtime_disable_action(), which calls
pm_runtime_dont_use_autosuspend() and pm_runtime_disable(). Shouldn't
this work, or am I missing something? Basically when the driver
detaches shouldn't this chain end up calling
pm_runtime_dont_use_autosuspend()?
Thank you,
Chris
>
> >
> > > +
> > > return 0;
> > > }
> > > EXPORT_SYMBOL_NS_GPL(inv_icm42607_core_probe, "IIO_ICM42607");
>
> Thank you,
> Chris
^ permalink raw reply
* Re: [PATCH 5/5] arm64: dts: qcom: hamoa: Extend QMPPHY description for USB4
From: Dmitry Baryshkov @ 2026-06-23 17:58 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, linux-kernel, linux-phy,
linux-arm-msm, devicetree, usb4-upstream, Raghavendra Thoorpu,
Mika Westerberg, Sven Peter, Konrad Dybcio
In-Reply-To: <20260518-topic-usb4phy-v1-5-71d827c49dca@oss.qualcomm.com>
On Mon, May 18, 2026 at 12:29:52PM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> The USB4 part of the QMPPHY requires that one more GCC clock (P2RR2P -
> PHY-to-Router, Router-to-PHY) is enabled for the PHY to initialize
> successfully. Describe that.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
> arch/arm64/boot/dts/qcom/hamoa.dtsi | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 4/5] phy: qualcomm: qmp-combo: Add USB4/TBT3 configuration data for Hamoa
From: Dmitry Baryshkov @ 2026-06-23 17:58 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, linux-kernel, linux-phy,
linux-arm-msm, devicetree, usb4-upstream, Raghavendra Thoorpu,
Mika Westerberg, Sven Peter, Konrad Dybcio
In-Reply-To: <20260518-topic-usb4phy-v1-4-71d827c49dca@oss.qualcomm.com>
On Mon, May 18, 2026 at 12:29:51PM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> Add the offsets and configuration tables to support USB4 and
> Thunderbolt 3 operation on the USB4-capable PHYs found on Hamoa chips.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
> drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 200 ++++++++++++++++++++-
> drivers/phy/qualcomm/phy-qcom-qmp-pcs-aon-v6.h | 2 +
> drivers/phy/qualcomm/phy-qcom-qmp-pcs-usb-v6.h | 15 ++
> .../phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h | 45 +++++
> 4 files changed, 256 insertions(+), 6 deletions(-)
>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v4 2/3] iio: health: add MAX86150 ECG and PPG biosensor driver
From: sashiko-bot @ 2026-06-23 17:57 UTC (permalink / raw)
To: Md Shofiqul Islam; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260623174600.17100-3-shofiqtest@gmail.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] The driver fails to assign `indio_dev->trig`, causing `iio_trigger_validate_own_device` to unconditionally reject the trigger.
- [High] Using a hard IRQ handler for the IIO trigger without interrupt masking causes a system-locking interrupt storm for level-triggered interrupts.
- [Medium] Timestamp reconstruction assigns inaccurate timestamps into the past when the hardware FIFO overflows because it ignores the `OVF_COUNTER` dropped sample count.
- [Medium] The I2C DMA buffer `fifo_raw` shares a cacheline with the pushed CPU buffer `buf`, violating DMA API constraints.
- [Medium] A blind sleep in `read_raw` risks reading an empty FIFO if the internal oscillator or device wake-up is slower than nominal.
--
commit 44deffd1bb30e8d4384d605ba5d156fb29678ad5
Author: Md Shofiqul Islam <shofiqtest@gmail.com>
iio: health: add MAX86150 ECG and PPG biosensor driver
The MAX86150 (Maxim/Analog Devices) combines two PPG optical channels
(Red/IR LED) and one ECG biopotential channel in a single I2C device with
a 32-entry hardware FIFO.
> diff --git a/drivers/iio/health/max86150.c b/drivers/iio/health/max86150.c
> new file mode 100644
> index 0000000000000..0140c93bd4ec9
> --- /dev/null
> +++ b/drivers/iio/health/max86150.c
[ ... ]
> +struct max86150_data {
> + struct regmap *regmap;
> + struct device *dev;
> + struct iio_trigger *trig;
> + u32 sample_period_ns;
> + u8 fifo_raw[MAX86150_SAMPLE_BYTES] __aligned(ARCH_DMA_MINALIGN);
> + s32 buf[6] __aligned(8);
> +};
[Severity: Medium]
Since fifo_raw is 9 bytes long, does the __aligned macro only align the
start of the array, leaving the buf array packed into the same DMA cacheline?
Mapping an unpadded buffer that shares a cacheline with CPU-accessed data
might violate kernel DMA mapping constraints and trigger
CONFIG_DMA_API_DEBUG warnings.
[ ... ]
> +static int max86150_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
[ ... ]
> + /* Wait for one complete sample period at 100 Hz (<= 10 ms) */
> + usleep_range(11000, 13000);
> +
> + ret = max86150_read_one_sample(data, &ppg_red, &ppg_ir, &ecg);
[Severity: Medium]
Is it possible for the hardware read to execute while the FIFO is still
empty if the internal oscillator or device wake-up is slower than nominal?
Would it be safer to use regmap_read_poll_timeout on the PPG_RDY status
bit to ensure a sample is actually ready?
[ ... ]
> +static irqreturn_t max86150_trigger_handler(int irq, void *p)
> +{
[ ... ]
> + /*
> + * Anchor timestamps to the A_FULL IRQ capture time: sample index
> + * (MAX86150_A_FULL_SAMPLES - 1) corresponds to pf->timestamp.
> + * Samples that accumulated between the IRQ and handler execution
> + * receive timestamps in the future relative to the IRQ, eliminating
> + * load-dependent jitter in multi-sample drains.
> + */
> + for (i = 0; i < n_avail; i++) {
> + s64 ts = pf->timestamp +
> + (s64)(i - (MAX86150_A_FULL_SAMPLES - 1)) *
> + data->sample_period_ns;
[Severity: Medium]
When the FIFO overflows (ovf > 0), the hardware overwrites the oldest
samples. Does this calculation assign inaccurate timestamps into the past
because it ignores the OVF_COUNTER dropped sample count?
The remaining 32 samples are chronologically shifted forward by ovf relative
to the original A_FULL IRQ event.
[ ... ]
> +static int max86150_probe(struct i2c_client *client)
> +{
[ ... ]
> + /*
> + * If the device tree provides an interrupt, set up a hardware
> + * trigger so userspace can use the FIFO almost-full signal to
> + * drive capture without polling.
> + */
> + if (client->irq > 0) {
> + unsigned long irq_trig;
> +
> + data->trig = devm_iio_trigger_alloc(&client->dev,
> + "%s-dev%d",
> + indio_dev->name,
> + iio_device_id(indio_dev));
> + if (!data->trig)
> + return -ENOMEM;
> +
> + data->trig->ops = &max86150_trigger_ops;
> + iio_trigger_set_drvdata(data->trig, indio_dev);
[Severity: High]
Does the driver fail to assign indio_dev->trig = data->trig after
allocating the trigger?
Since validate_trigger is set to iio_trigger_validate_own_device, it looks
like missing this assignment will cause any attempt to write to the
current_trigger sysfs node to unconditionally fail with -EINVAL.
> +
> + /*
> + * Honour the interrupt trigger type from the device tree.
> + * Fall back to falling-edge if the DT does not specify one.
> + */
> + irq_trig = irq_get_trigger_type(client->irq);
> + if (!irq_trig)
> + irq_trig = IRQF_TRIGGER_FALLING;
> +
> + ret = devm_request_irq(&client->dev, client->irq,
> + iio_trigger_generic_data_rdy_poll,
> + irq_trig,
> + "max86150", data->trig);
[Severity: High]
Will using a hard IRQ handler for the IIO trigger without interrupt masking
cause an interrupt storm for level-triggered interrupts?
For level-triggered interrupts, the hard handler wakes the consumer thread
and returns IRQ_HANDLED without clearing the physical interrupt via I2C.
The IRQ core then unmasks the still-asserted interrupt line, causing it to
immediately fire again and preventing the threaded consumer from ever
running to clear the source.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260623174600.17100-1-shofiqtest@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH 3/5] phy: qualcomm: qmp-combo: Add preliminary USB4 support
From: Dmitry Baryshkov @ 2026-06-23 17:56 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Konrad Dybcio, Vinod Koul, Neil Armstrong, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, linux-kernel,
linux-phy, linux-arm-msm, devicetree, usb4-upstream,
Raghavendra Thoorpu, Mika Westerberg, Sven Peter
In-Reply-To: <3972248c-acfc-4b31-8c99-69bfdba34b8c@oss.qualcomm.com>
On Tue, Jun 16, 2026 at 01:44:48PM +0200, Konrad Dybcio wrote:
> On 5/28/26 10:00 AM, Dmitry Baryshkov wrote:
> > On Fri, May 22, 2026 at 02:05:14PM +0200, Konrad Dybcio wrote:
> >> On 5/20/26 5:06 PM, Dmitry Baryshkov wrote:
> >>> On Tue, May 19, 2026 at 10:12:06AM +0200, Konrad Dybcio wrote:
> >>>> On 5/18/26 5:38 PM, Dmitry Baryshkov wrote:
> >>>>> On Mon, May 18, 2026 at 04:15:16PM +0200, Konrad Dybcio wrote:
> >>>>>> On 5/18/26 3:57 PM, Dmitry Baryshkov wrote:
> >>>>>>> On Mon, May 18, 2026 at 12:29:50PM +0200, Konrad Dybcio wrote:
> >>>>>>>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> >>>>>>>>
> >>>>>>>> Some Combo PHYs (so far only on SC8280XP, X1E80100 and Glymur), come in
> >>>>>>>> a flavor called USB43DP, which as the name implies, features USB4, USB3
> >>>>>>>> and DP signal processing capabilities. In that architecture, USB3 and
> >>>>>>>> USB4 PHYs share the same USB_PLL while featuring separate logic spaces.
> >>>>>>>> The DP part is roughly the same as on the instances without USB4.
> >>>>>>>>
> >>>>>>>> The USB4 and USB3/DP operation modes of the PHY are mutually exclusive.
> >>>>>>>> Only one USB protocol (and flavor of pipe clock) can be active at a
> >>>>>>>> given moment (not to be confused with USB3 not being able to be
> >>>>>>>> tunneled as USB4 packets - that of course remains possible).
> >>>>>>>> The DP PLL is still used for clocking tunneled DP links. It may be
> >>>>>>>> turned off to save power when no tunnels are active, but that's left as
> >>>>>>>> a TODO item for now.
> >>>>>>>>
> >>>>>>>> Due to the nature of USB4, the Type-C handling happens entirely inside
> >>>>>>>> the Host Router, and as such the QMPPHY's mux_set() function is
> >>>>>>>> nullified for the period when USB4 PHY remains active. This is strictly
> >>>>>>>> necessary, as the Host Router driver is going to excercise manual
> >>>>>>>> control over the USB4 PHY's power state, which is needed by the suspend
> >>>>>>>> and resume flows. Failure to control that synchronously with other
> >>>>>>>> parts of the code results in a SoC crash by unlocked access.
> >>>>>>>>
> >>>>>>>> Because of that, a new struct phy is spawned to expose the USB4 mode,
> >>>>>>>> along with a .set_mode callback to allow toggling between USB4 and TBT3
> >>>>>>>> submodes.
> >>>>>>>>
> >>>>>>>> Thunderbolt 3, having a number of differences vs USB4, requires a
> >>>>>>>> couple specific overrides, pertaining to electrical characteristics,
> >>>>>>>> which are easily accommodated for.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> >>>>>>>> ---
> >>>>>>>> drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 392 ++++++++++++++++++++++++------
> >>>>>>>> 1 file changed, 322 insertions(+), 70 deletions(-)
> >>>>>>>>
> >>>>>>>
> >>>>>>> Overall it looks good. The major question (after looking at TODOs), do
> >>>>>>> we need a separate submode for USB+DP / TBT+DP?
> >>>>>>
> >>>>>> The problem space is as follows:
> >>>>>>
> >>>>>> After a TBT (collectively TBT3+ and USB4) link has been established and
> >>>>>> we have a link partner, we may (based on the HW capabilities and user
> >>>>>> config, such as kernel params but not only) start or stop a DP tunnel at
> >>>>>> runtime. On Qualcomm hardware, the PHY is kept in USB4 mode and its DP
> >>>>>> AUX lines are not used (instead, the encapsulated DP AUX packets are r/w
> >>>>>> entirely within the USB4 subsystem via a pair of FIFOs that Linux sees
> >>>>>> as a separate DP AUX host)
> >>>>>
> >>>>> So far so good. But I still don't grok if having a DP-over-USB4 is a
> >>>>> separate submode or not. I.e. I see code (and TODOs) to detect and
> >>>>> handle DP going on and off. Would it be better if we specify that
> >>>>> explicitly?
> >>>>
> >>>> I really don't want to end up in a situation like we have with:
> >>>>
> >>>> $ rg _USB include/linux/phy/phy.h
> >>>> 29: PHY_MODE_USB_HOST,
> >>>> 30: PHY_MODE_USB_HOST_LS,
> >>>> 31: PHY_MODE_USB_HOST_FS,
> >>>> 32: PHY_MODE_USB_HOST_HS,
> >>>> 33: PHY_MODE_USB_HOST_SS,
> >>>> 34: PHY_MODE_USB_DEVICE,
> >>>> 35: PHY_MODE_USB_DEVICE_LS,
> >>>> 36: PHY_MODE_USB_DEVICE_FS,
> >>>> 37: PHY_MODE_USB_DEVICE_HS,
> >>>> 38: PHY_MODE_USB_DEVICE_SS,
> >>>> 39: PHY_MODE_USB_OTG,
> >>>>
> >>>>>> Then, on hamoa/glymur specifically, any of the 3 USB4-capable DP hosts
> >>>>>> can be muxed to either of the 2 DPIN ports on any of the 3 USB4 routers
> >>>>>> (and each of these routers is hardwired to one of the PHYs).
> >>>>>>
> >>>>>> To underline, we have 3 DP producers and 6 consumers. If there's e.g. a
> >>>>>> super high-res display at one of the physical ports, or a long
> >>>>>> daisy-chain, we may need to use 2 DPTXes to service 1 receptacle. Then,
> >>>>>> we would only need one of the PHYs (associated with the router that's
> >>>>>> wired to that port) to provide a DP clock.
> >>>>>>
> >>>>>> This, along with the normal (logical or physical) present/absent status
> >>>>>> can change at runtime. My plan is to use phy_set_opts(dp_tunelling=true)
> >>>>>> or something along those lines to toggle that bit as necessary
> >>>>>
> >>>>> I don't see phy_set_opts(). So maybe a submode then...
> >>>>
> >>>> Sorry, I misremembered the name. The function is phy_configure(), and it
> >>>> takes a union phy_configure_opts, hence the confusion
> >>>
> >>> So, phy_configure() will be called for the DP PHY to set the DP opts,
> >>> but how do you plan to determine if DP is on or not? Or do you plan to
> >>> add phy_tbt_configure_opts ?
> >>>
> >>> Another obvious option would be to set the flag if DP PHY is being tuned
> >>> on / off. I don't know if that fulfills your needs.
> >>
> >> Either this or tbt_configure_opts. We still have the muxing question to
> >> chew through.
> >>
> >> The bottom line is that all AUX traffic happens between the "AUX adapters"
> >> within USB4SS, talking over thunderbolt to other AUX adapters on the LTTPRs
> >> and the far-end device (and anything inbetween in a chained topology) meaning
> >> we only need to engage the DP host itself (and therefore the PHY) after we've
> >> already performed the capability negotiations
> >
> > I hope you mean USB link capabilities. DP host still needs to ping LTTPRs
> > and read all the DP properties on its own. I don't think we want to leak
> > that to the other layers.
>
> I must crush your hopes.
>
> There's some preliminary TBT-layer setup (handled by the tbt driver in
> Linux), followed by the expected DPCD (and alike) r/w accesses, which on
> our hw must happen through the DP adapters housed inside USB4SS (again,
> because the DPTX's auxbus is NOPed out). Think of it as just another
> i2c_aux provider.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v3 1/3] dt-bindings: iio: health: add maxim,max86150
From: Conor Dooley @ 2026-06-23 17:53 UTC (permalink / raw)
To: Md Shofiqul Islam
Cc: linux-iio, jic23, lars, robh, conor+dt, krzk+dt, devicetree
In-Reply-To: <20260623164309.16452-2-shofiqtest@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3950 bytes --]
On Tue, Jun 23, 2026 at 07:43:07PM +0300, Md Shofiqul Islam wrote:
> Add YAML binding schema for the Maxim MAX86150 combined ECG and PPG
> biosensor. The device exposes two PPG optical channels (Red and IR LED)
> for heart rate and SpO2 measurement, and one ECG biopotential channel, all
> accessible over I2C at up to 400 kHz.
>
> An optional active-low interrupt line connects to the 32-entry hardware
> FIFO almost-full output. Two optional regulator supplies (vdd for the
> digital core and leds for the LED anodes) cover boards that require
> explicit power sequencing.
>
> Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
> ---
> .../bindings/iio/health/maxim,max86150.yaml | 67 +++++++++++++++++++
> 1 file changed, 67 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/health/maxim,max86150.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/health/maxim,max86150.yaml b/Documentation/devicetree/bindings/iio/health/maxim,max86150.yaml
> new file mode 100644
> index 000000000000..1bf10fd1a3d2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/health/maxim,max86150.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/health/maxim,max86150.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Maxim MAX86150 ECG and PPG Biosensor
> +
> +maintainers:
> + - Md Shofiqul Islam <shofiqtest@gmail.com>
> +
> +description: |
> + The MAX86150 is an integrated biosensor SoC that combines:
> + - Two PPG (photoplethysmography) channels: Red LED and IR LED,
> + for heart rate and blood-oxygen saturation (SpO2) measurement.
> + - One ECG (electrocardiogram) channel for biopotential recording.
> +
> + The device communicates over I2C at up to 400 kHz and raises an
> + active-low interrupt when the 32-entry hardware FIFO reaches its
> + configurable almost-full threshold.
> +
> + Datasheet:
> + https://www.analog.com/media/en/technical-documentation/data-sheets/MAX86150.pdf
> +
> +properties:
> + compatible:
> + const: maxim,max86150
ADI would like the prefix for newly added former maxim products to be
"adi,".
> +
> + reg:
> + maxItems: 1
> + description: I2C device address, always 0x5E.
> +
> + interrupts:
> + maxItems: 1
> + description: |
> + Active-low interrupt line. Asserted when the FIFO almost-full
> + threshold is reached or when a new PPG sample is ready.
> +
> + vdd-supply:
> + description: Digital core supply, 1.71 V to 1.89 V.
This is a 1.8v supply, the tolerances on it don't need to be mentioned
here.
> + leds-supply:
> + description: LED anode supply, 3.0 V to 5.5 V.
Datasheet cites 5v as the max for this, not 5.5v, with a typical value of
3.3v. Makes me think that 3.3 is the intended value, since using a 5v
supply could easily fluctuate over the maximum rated value.
Looks like there's more supplies than this?
I see:
Vdd_ana
Vdd_dig
Vref
Vled
Additionally, these supplies should be mandatory.
pw-bot: changes-requested
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + biosensor@5e {
Seems to be prior art for calling things like this "heart-rate",
although that is somewhat limited compared to what this device can do.
Cheers,
Conor.
> + compatible = "maxim,max86150";
> + reg = <0x5e>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <5 IRQ_TYPE_EDGE_FALLING>;
> + vdd-supply = <&vdd_1v8>;
> + leds-supply = <&vdd_3v3>;
> + };
> + };
> --
> 2.51.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH v4 3/3] MAINTAINERS: add entry for MAX86150 IIO health driver
From: Md Shofiqul Islam @ 2026-06-23 17:46 UTC (permalink / raw)
To: linux-iio
Cc: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, lars,
devicetree, linux-kernel, Md Shofiqul Islam
In-Reply-To: <20260623174600.17100-1-shofiqtest@gmail.com>
Add a MAINTAINERS entry for the new MAX86150 ECG and PPG biosensor driver,
covering both the driver and its DT binding schema.
Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
---
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 3115538ce829..a441ec44bb27 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15863,6 +15863,13 @@ S: Supported
F: Documentation/devicetree/bindings/power/supply/maxim,max77976.yaml
F: drivers/power/supply/max77976_charger.c
+MAXIM MAX86150 ECG AND PPG BIOSENSOR DRIVER
+M: Md Shofiqul Islam <shofiqtest@gmail.com>
+L: linux-iio@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/iio/health/maxim,max86150.yaml
+F: drivers/iio/health/max86150.c
+
MAXIM MUIC CHARGER DRIVERS FOR EXYNOS BASED BOARDS
M: Krzysztof Kozlowski <krzk@kernel.org>
L: linux-pm@vger.kernel.org
--
2.51.1
^ permalink raw reply related
* [PATCH v4 2/3] iio: health: add MAX86150 ECG and PPG biosensor driver
From: Md Shofiqul Islam @ 2026-06-23 17:45 UTC (permalink / raw)
To: linux-iio
Cc: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, lars,
devicetree, linux-kernel, Md Shofiqul Islam
In-Reply-To: <20260623174600.17100-1-shofiqtest@gmail.com>
The MAX86150 (Maxim/Analog Devices) combines two PPG optical channels
(Red/IR LED) and one ECG biopotential channel in a single I2C device with
a 32-entry hardware FIFO.
Driver features:
- Three IIO channels: in_intensityred_raw, in_intensityir_raw,
in_voltage0_raw
- Triggered buffer path driven by the FIFO almost-full interrupt;
set_trigger_state enables/disables the interrupt only while the buffer
is active and FIFO is flushed before capture starts
- validate_trigger = iio_trigger_validate_own_device prevents
incompatible external triggers from being attached
- IRQ trigger type taken from irq_get_trigger_type() to honour DT
configuration; falls back to falling-edge if unspecified
- fifo_raw burst-read buffer is heap-allocated in struct max86150_data
and aligned to ARCH_DMA_MINALIGN to satisfy DMA mapping requirements
of I2C host controllers that use DMA for burst reads
- SYS_SHDN asserted at end of chip_init so LED drivers draw no current
when capture is inactive; set_trigger_state() and read_raw() wake and
sleep the device on demand
- Per-sample timestamps anchored to the A_FULL IRQ capture time: the
sample at index (A_FULL_SAMPLES - 1) maps to pf->timestamp, samples
accumulated after the IRQ receive future timestamps, eliminating
load-dependent jitter
- FIFO empty vs exactly-full disambiguation: when wr_ptr == rd_ptr with
OVF_COUNTER == 0, the A_FULL status bit distinguishes a pointer
wrap-around (full) from a genuinely empty FIFO
- devm_regulator_get_enable_optional() for the two optional supplies;
-ENODEV is treated as success (supply absent, not an error)
- Powerdown devm action disables interrupts and asserts SYS_SHDN
Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
---
drivers/iio/health/Kconfig | 23 ++
drivers/iio/health/Makefile | 1 +
drivers/iio/health/max86150.c | 677 ++++++++++++++++++++++++++++++++++
3 files changed, 701 insertions(+)
create mode 100644 drivers/iio/health/max86150.c
diff --git a/drivers/iio/health/Kconfig b/drivers/iio/health/Kconfig
index a89f3abf11f4..6496cf55290c 100644
--- a/drivers/iio/health/Kconfig
+++ b/drivers/iio/health/Kconfig
@@ -13,6 +13,7 @@ config AFE4403
depends on SPI_MASTER
select REGMAP_SPI
select IIO_BUFFER
+ select IIO_TRIGGER
select IIO_TRIGGERED_BUFFER
help
Say yes to choose the Texas Instruments AFE4403
@@ -26,6 +27,7 @@ config AFE4404
depends on I2C
select REGMAP_I2C
select IIO_BUFFER
+ select IIO_TRIGGER
select IIO_TRIGGERED_BUFFER
help
Say yes to choose the Texas Instruments AFE4404
@@ -39,6 +41,7 @@ config MAX30100
depends on I2C
select REGMAP_I2C
select IIO_BUFFER
+ select IIO_TRIGGER
select IIO_KFIFO_BUF
help
Say Y here to build I2C interface support for the Maxim
@@ -52,6 +55,7 @@ config MAX30102
depends on I2C
select REGMAP_I2C
select IIO_BUFFER
+ select IIO_TRIGGER
select IIO_KFIFO_BUF
help
Say Y here to build I2C interface support for the Maxim
@@ -62,4 +66,23 @@ config MAX30102
endmenu
+
+config MAX86150
+ tristate "MAX86150 ECG and PPG biosensor"
+ depends on I2C
+ select IIO_BUFFER
+ select IIO_TRIGGER
+ select IIO_TRIGGERED_BUFFER
+ select REGMAP_I2C
+ help
+ Say Y here to enable support for the Maxim MAX86150 combined
+ ECG and photoplethysmography (PPG) biosensor.
+
+ The driver exposes three IIO channels: two PPG optical channels
+ (Red and IR LED) for heart rate and SpO2 monitoring, and one
+ ECG channel for biopotential recording.
+
+ This driver can also be built as a module. If so, the module
+ will be called max86150.
+
endmenu
diff --git a/drivers/iio/health/Makefile b/drivers/iio/health/Makefile
index 910817112258..04fc73c58444 100644
--- a/drivers/iio/health/Makefile
+++ b/drivers/iio/health/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_AFE4403) += afe4403.o
obj-$(CONFIG_AFE4404) += afe4404.o
obj-$(CONFIG_MAX30100) += max30100.o
obj-$(CONFIG_MAX30102) += max30102.o
+obj-$(CONFIG_MAX86150) += max86150.o
diff --git a/drivers/iio/health/max86150.c b/drivers/iio/health/max86150.c
new file mode 100644
index 000000000000..0140c93bd4ec
--- /dev/null
+++ b/drivers/iio/health/max86150.c
@@ -0,0 +1,677 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * MAX86150 combined ECG and PPG biosensor driver
+ *
+ * Copyright (C) 2026 Md Shofiqul Islam <shofiqtest@gmail.com>
+ *
+ * The MAX86150 integrates two PPG optical channels (Red/IR LED) and one
+ * ECG biopotential channel in a single I2C device. Data is captured
+ * through a 32-entry hardware FIFO with a configurable almost-full
+ * interrupt, making it well-suited for continuous monitoring with a
+ * low-power host.
+ *
+ * Datasheet:
+ * https://www.analog.com/media/en/technical-documentation/data-sheets/MAX86150.pdf
+ */
+
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/irq.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+
+/* Register addresses */
+#define MAX86150_REG_INT_STATUS1 0x00
+#define MAX86150_REG_INT_STATUS2 0x01
+#define MAX86150_REG_INT_ENABLE1 0x02
+#define MAX86150_REG_INT_ENABLE2 0x03
+#define MAX86150_REG_FIFO_WR_PTR 0x04
+#define MAX86150_REG_OVF_COUNTER 0x05
+#define MAX86150_REG_FIFO_RD_PTR 0x06
+#define MAX86150_REG_FIFO_DATA 0x07
+#define MAX86150_REG_FIFO_CONFIG 0x08
+#define MAX86150_REG_FIFO_DCTRL1 0x09 /* FD1[3:0] FD2[7:4] */
+#define MAX86150_REG_FIFO_DCTRL2 0x0A /* FD3[3:0] FD4[7:4] */
+#define MAX86150_REG_SYS_CTRL 0x0D
+#define MAX86150_REG_PPG_CONFIG1 0x10
+#define MAX86150_REG_PPG_CONFIG2 0x11
+#define MAX86150_REG_LED1_PA 0x14 /* Red LED pulse amplitude */
+#define MAX86150_REG_LED2_PA 0x15 /* IR LED pulse amplitude */
+#define MAX86150_REG_ECG_CONFIG1 0x3C
+#define MAX86150_REG_ECG_CONFIG3 0x3E
+#define MAX86150_REG_PART_ID 0xFF
+
+/* Field masks */
+#define MAX86150_PART_ID_VAL 0x1E
+
+/* INT_STATUS1 / INT_ENABLE1 */
+#define MAX86150_INT_A_FULL BIT(7) /* FIFO almost full */
+#define MAX86150_INT_PPG_RDY BIT(6) /* new PPG sample ready */
+
+/* SYS_CTRL */
+#define MAX86150_SYS_SHDN BIT(1)
+#define MAX86150_SYS_RESET BIT(0)
+
+/* FIFO_CONFIG */
+#define MAX86150_FIFO_SMP_AVE GENMASK(7, 5)
+#define MAX86150_FIFO_ROLLOVER_EN BIT(4)
+#define MAX86150_FIFO_A_FULL GENMASK(3, 0)
+
+/* FIFO slot data-type codes */
+#define MAX86150_FD_NONE 0x0
+#define MAX86150_FD_LED1 0x1 /* Red PPG */
+#define MAX86150_FD_LED2 0x2 /* IR PPG */
+#define MAX86150_FD_ECG 0x9
+#define MAX86150_FIFO_FD1 GENMASK(3, 0)
+#define MAX86150_FIFO_FD2 GENMASK(7, 4)
+#define MAX86150_FIFO_FD3 GENMASK(3, 0)
+#define MAX86150_FIFO_FD4 GENMASK(7, 4)
+
+/* PPG_CONFIG1 */
+#define MAX86150_PPG_ADC_RGE GENMASK(7, 6)
+#define MAX86150_PPG_SR GENMASK(5, 1)
+
+/* Geometry */
+#define MAX86150_FIFO_DEPTH 32
+#define MAX86150_BYTES_PER_SLOT 3 /* 24-bit word per slot */
+#define MAX86150_NUM_SLOTS 3 /* Red, IR, ECG */
+#define MAX86150_SAMPLE_BYTES (MAX86150_NUM_SLOTS * MAX86150_BYTES_PER_SLOT)
+/* Samples present in the FIFO when the A_FULL interrupt fires */
+#define MAX86150_A_FULL_SAMPLES (MAX86150_FIFO_DEPTH - MAX86150_FIFO_A_FULL_VAL)
+
+/* Default hardware configuration */
+#define MAX86150_LED_PA_DEFAULT 0x3F /* ~50 mA */
+#define MAX86150_PPG_SR_100HZ 4 /* PPG_SR field value for 100 Hz */
+#define MAX86150_PPG_ADC_RGE_16384 2 /* 16384 nA full scale */
+/* Fire A_FULL when 17 slots remain (32 - 15 = 17 samples in FIFO) */
+#define MAX86150_FIFO_A_FULL_VAL 15
+
+/* Scan element indices */
+enum max86150_scan_idx {
+ MAX86150_IDX_PPG_RED = 0,
+ MAX86150_IDX_PPG_IR = 1,
+ MAX86150_IDX_ECG = 2,
+ MAX86150_IDX_TS,
+};
+
+/**
+ * struct max86150_data - driver private state
+ * @regmap: register map for this device
+ * @dev: parent device (for dev_err logging)
+ * @trig: IIO hardware trigger backed by the device interrupt line
+ * @sample_period_ns: sample period in nanoseconds (set from configured rate)
+ * @fifo_raw: DMA-safe buffer for regmap_noinc_read() FIFO bursts;
+ * must be in struct (heap) not on the stack to satisfy
+ * DMA mapping requirements of some I2C host controllers
+ * @buf: IIO push buffer sized for worst-case (all 3 channels
+ * active): 3 x s32 (12 bytes) + 4-byte pad + s64
+ * timestamp = 24 bytes. __aligned(8) satisfies
+ * iio_push_to_buffers_with_timestamp().
+ */
+struct max86150_data {
+ struct regmap *regmap;
+ struct device *dev;
+ struct iio_trigger *trig;
+ u32 sample_period_ns;
+ u8 fifo_raw[MAX86150_SAMPLE_BYTES] __aligned(ARCH_DMA_MINALIGN);
+ s32 buf[6] __aligned(8);
+};
+
+/* IIO channel specification */
+
+static const struct iio_chan_spec max86150_channels[] = {
+ {
+ /* PPG Red LED - optical intensity, 19-bit unsigned */
+ .type = IIO_INTENSITY,
+ .modified = 1,
+ .channel2 = IIO_MOD_LIGHT_RED,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .scan_index = MAX86150_IDX_PPG_RED,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 19,
+ .storagebits = 32,
+ .endianness = IIO_CPU,
+ },
+ },
+ {
+ /* PPG IR LED - optical intensity, 19-bit unsigned */
+ .type = IIO_INTENSITY,
+ .modified = 1,
+ .channel2 = IIO_MOD_LIGHT_IR,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .scan_index = MAX86150_IDX_PPG_IR,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 19,
+ .storagebits = 32,
+ .endianness = IIO_CPU,
+ },
+ },
+ {
+ /* ECG biopotential - voltage, 18-bit signed two's complement */
+ .type = IIO_VOLTAGE,
+ .channel = 0,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .scan_index = MAX86150_IDX_ECG,
+ .scan_type = {
+ .sign = 's',
+ .realbits = 18,
+ .storagebits = 32,
+ .endianness = IIO_CPU,
+ },
+ },
+ IIO_CHAN_SOFT_TIMESTAMP(MAX86150_IDX_TS),
+};
+
+/* Regmap configuration */
+
+static const struct regmap_config max86150_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = MAX86150_REG_PART_ID,
+};
+
+/* FIFO helper */
+
+/**
+ * max86150_read_one_sample - burst-read one complete 3-slot FIFO entry
+ * @data: driver state
+ * @ppg_red: out - 19-bit PPG Red ADC value (unsigned)
+ * @ppg_ir: out - 19-bit PPG IR ADC value (unsigned)
+ * @ecg: out - 18-bit ECG ADC value (sign-extended to s32)
+ *
+ * Each FIFO entry is 9 bytes (3 slots x 3 bytes). FIFO_DATA is a
+ * streaming register - the address does not auto-increment on each
+ * byte, so regmap_noinc_read() is used instead of regmap_bulk_read().
+ *
+ * Byte layout in the 24-bit FIFO word (MSB first):
+ * PPG 19-bit unsigned: bits [18:0], top 5 bits are always zero
+ * ECG 18-bit signed: bits [17:0], top 6 bits are sign extension
+ */
+static int max86150_read_one_sample(struct max86150_data *data,
+ u32 *ppg_red, u32 *ppg_ir, s32 *ecg)
+{
+ int ret;
+
+ /*
+ * Use data->fifo_raw (heap memory) not a local array so the buffer is
+ * DMA-mappable for I2C host controllers that use DMA for burst reads.
+ */
+ ret = regmap_noinc_read(data->regmap, MAX86150_REG_FIFO_DATA,
+ data->fifo_raw, sizeof(data->fifo_raw));
+ if (ret)
+ return ret;
+
+ /* Bytes [0..2]: PPG Red - 19-bit value in bits [18:0] */
+ *ppg_red = (u32)(data->fifo_raw[0] & 0x07) << 16 |
+ (u32)data->fifo_raw[1] << 8 | data->fifo_raw[2];
+
+ /* Bytes [3..5]: PPG IR - same format */
+ *ppg_ir = (u32)(data->fifo_raw[3] & 0x07) << 16 |
+ (u32)data->fifo_raw[4] << 8 | data->fifo_raw[5];
+
+ /* Bytes [6..8]: ECG - 18-bit signed, sign-extend to s32 */
+ *ecg = sign_extend32((u32)(data->fifo_raw[6] & 0x03) << 16 |
+ (u32)data->fifo_raw[7] << 8 | data->fifo_raw[8], 17);
+
+ return 0;
+}
+
+/* IIO read_raw */
+
+static int max86150_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct max86150_data *data = iio_priv(indio_dev);
+ u32 ppg_red, ppg_ir;
+ s32 ecg;
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ /*
+ * Claim direct mode to prevent concurrent sysfs reads from
+ * corrupting the FIFO pointers while a triggered buffer
+ * capture is active.
+ */
+ if (!iio_device_claim_direct(indio_dev))
+ return -EBUSY;
+
+ /*
+ * Single-shot path: wake the device, flush stale FIFO data,
+ * wait one sample period, read, then return to shutdown so
+ * the LEDs are not drawing current when idle.
+ */
+ ret = regmap_update_bits(data->regmap, MAX86150_REG_SYS_CTRL,
+ MAX86150_SYS_SHDN, 0);
+ if (!ret)
+ ret = regmap_write(data->regmap,
+ MAX86150_REG_FIFO_WR_PTR, 0);
+ if (!ret)
+ ret = regmap_write(data->regmap,
+ MAX86150_REG_OVF_COUNTER, 0);
+ if (!ret)
+ ret = regmap_write(data->regmap,
+ MAX86150_REG_FIFO_RD_PTR, 0);
+ if (ret) {
+ regmap_update_bits(data->regmap, MAX86150_REG_SYS_CTRL,
+ MAX86150_SYS_SHDN, MAX86150_SYS_SHDN);
+ iio_device_release_direct(indio_dev);
+ return ret;
+ }
+
+ /* Wait for one complete sample period at 100 Hz (<= 10 ms) */
+ usleep_range(11000, 13000);
+
+ ret = max86150_read_one_sample(data, &ppg_red, &ppg_ir, &ecg);
+ regmap_update_bits(data->regmap, MAX86150_REG_SYS_CTRL,
+ MAX86150_SYS_SHDN, MAX86150_SYS_SHDN);
+ iio_device_release_direct(indio_dev);
+ if (ret)
+ return ret;
+
+ switch (chan->scan_index) {
+ case MAX86150_IDX_PPG_RED:
+ *val = ppg_red;
+ break;
+ case MAX86150_IDX_PPG_IR:
+ *val = ppg_ir;
+ break;
+ case MAX86150_IDX_ECG:
+ *val = ecg;
+ break;
+ default:
+ return -EINVAL;
+ }
+ return IIO_VAL_INT;
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_info max86150_iio_info = {
+ .read_raw = max86150_read_raw,
+ .validate_trigger = iio_trigger_validate_own_device,
+};
+
+/* Trigger */
+
+/*
+ * Control device power and the FIFO almost-full interrupt when the IIO
+ * triggered buffer is started (state=true) or stopped (state=false).
+ *
+ * On start: wake from shutdown, flush stale FIFO data so the first
+ * samples pushed to userspace are from after buffer enable, then
+ * unmask the A_FULL interrupt.
+ *
+ * On stop: mask the interrupt, then return to shutdown so the LED
+ * drivers do not draw current while capture is inactive.
+ */
+static int max86150_set_trigger_state(struct iio_trigger *trig, bool state)
+{
+ struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
+ struct max86150_data *data = iio_priv(indio_dev);
+ int ret;
+
+ if (!state) {
+ ret = regmap_write(data->regmap, MAX86150_REG_INT_ENABLE1, 0);
+ regmap_update_bits(data->regmap, MAX86150_REG_SYS_CTRL,
+ MAX86150_SYS_SHDN, MAX86150_SYS_SHDN);
+ return ret;
+ }
+
+ ret = regmap_update_bits(data->regmap, MAX86150_REG_SYS_CTRL,
+ MAX86150_SYS_SHDN, 0);
+ if (!ret)
+ ret = regmap_write(data->regmap, MAX86150_REG_FIFO_WR_PTR, 0);
+ if (!ret)
+ ret = regmap_write(data->regmap, MAX86150_REG_OVF_COUNTER, 0);
+ if (!ret)
+ ret = regmap_write(data->regmap, MAX86150_REG_FIFO_RD_PTR, 0);
+ if (!ret)
+ ret = regmap_write(data->regmap, MAX86150_REG_INT_ENABLE1,
+ MAX86150_INT_A_FULL);
+ if (ret)
+ regmap_update_bits(data->regmap, MAX86150_REG_SYS_CTRL,
+ MAX86150_SYS_SHDN, MAX86150_SYS_SHDN);
+ return ret;
+}
+
+static const struct iio_trigger_ops max86150_trigger_ops = {
+ .set_trigger_state = max86150_set_trigger_state,
+};
+
+/* Triggered buffer */
+
+/**
+ * max86150_trigger_handler - threaded IRQ handler for FIFO almost-full
+ *
+ * Called by the IIO buffer infrastructure when the hardware trigger fires.
+ * Reads INT_STATUS1 to de-assert the interrupt, then drains all available
+ * FIFO samples into the IIO push buffer, packing only the channels that
+ * are currently enabled in active_scan_mask.
+ */
+static irqreturn_t max86150_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *idev = pf->indio_dev;
+ struct max86150_data *data = iio_priv(idev);
+ unsigned int status, wr_ptr, rd_ptr, ovf;
+ u32 ppg_red, ppg_ir;
+ s32 ecg;
+ int ret, n_avail, i, j;
+
+ /*
+ * Reading INT_STATUS1 clears the interrupt. Do this before touching
+ * the FIFO so the pin is de-asserted while we drain samples.
+ */
+ ret = regmap_read(data->regmap, MAX86150_REG_INT_STATUS1, &status);
+ if (ret)
+ goto done;
+
+ ret = regmap_read(data->regmap, MAX86150_REG_FIFO_WR_PTR, &wr_ptr);
+ if (ret)
+ goto done;
+ ret = regmap_read(data->regmap, MAX86150_REG_FIFO_RD_PTR, &rd_ptr);
+ if (ret)
+ goto done;
+
+ /*
+ * OVF_COUNTER: if non-zero the FIFO overflowed; drain all 32 slots.
+ * When wr_ptr == rd_ptr with no overflow the FIFO could be empty OR
+ * hold exactly MAX86150_FIFO_DEPTH entries (pointer wrap-around).
+ * Use the A_FULL status bit to disambiguate: if the IRQ fired for
+ * A_FULL but the computed count is zero, the FIFO wrapped to full.
+ */
+ ret = regmap_read(data->regmap, MAX86150_REG_OVF_COUNTER, &ovf);
+ if (ret)
+ goto done;
+
+ if (ovf > 0) {
+ n_avail = MAX86150_FIFO_DEPTH;
+ } else {
+ n_avail = (wr_ptr - rd_ptr) & (MAX86150_FIFO_DEPTH - 1);
+ if (n_avail == 0 && (status & MAX86150_INT_A_FULL))
+ n_avail = MAX86150_FIFO_DEPTH;
+ }
+
+ /*
+ * Anchor timestamps to the A_FULL IRQ capture time: sample index
+ * (MAX86150_A_FULL_SAMPLES - 1) corresponds to pf->timestamp.
+ * Samples that accumulated between the IRQ and handler execution
+ * receive timestamps in the future relative to the IRQ, eliminating
+ * load-dependent jitter in multi-sample drains.
+ */
+ for (i = 0; i < n_avail; i++) {
+ s64 ts = pf->timestamp +
+ (s64)(i - (MAX86150_A_FULL_SAMPLES - 1)) *
+ data->sample_period_ns;
+
+ ret = max86150_read_one_sample(data, &ppg_red, &ppg_ir, &ecg);
+ if (ret)
+ break;
+
+ /*
+ * Zero the entire buffer before packing so padding bytes
+ * between enabled channels do not leak previous sample data
+ * to userspace when fewer than 3 channels are active.
+ */
+ memset(data->buf, 0, sizeof(data->buf));
+
+ /*
+ * Pack only active channels at consecutive positions [0..j-1].
+ * iio_push_to_buffers_with_timestamp() uses scan_bytes (which
+ * accounts for the active channel count) to place the timestamp,
+ * so static indexing would misplace it when fewer than 3
+ * channels are enabled.
+ */
+ j = 0;
+ if (test_bit(MAX86150_IDX_PPG_RED, idev->active_scan_mask))
+ data->buf[j++] = ppg_red;
+ if (test_bit(MAX86150_IDX_PPG_IR, idev->active_scan_mask))
+ data->buf[j++] = ppg_ir;
+ if (test_bit(MAX86150_IDX_ECG, idev->active_scan_mask))
+ data->buf[j++] = ecg;
+
+ iio_push_to_buffers_with_timestamp(idev, data->buf, ts);
+ }
+
+done:
+ iio_trigger_notify_done(idev->trig);
+ return IRQ_HANDLED;
+}
+
+/* Chip initialisation / teardown */
+
+static void max86150_powerdown(void *arg)
+{
+ struct max86150_data *data = arg;
+
+ regmap_write(data->regmap, MAX86150_REG_INT_ENABLE1, 0);
+ regmap_write(data->regmap, MAX86150_REG_SYS_CTRL, MAX86150_SYS_SHDN);
+}
+
+static int max86150_chip_init(struct max86150_data *data)
+{
+ int ret;
+
+ /* Software reset; the bit self-clears within 1 ms */
+ ret = regmap_write(data->regmap, MAX86150_REG_SYS_CTRL,
+ MAX86150_SYS_RESET);
+ if (ret)
+ return ret;
+ usleep_range(1000, 2000);
+
+ /*
+ * FIFO: no sample averaging, rollover enabled, assert A_FULL when
+ * 17 samples are in the FIFO (32 - 15 = 17 available to read).
+ */
+ ret = regmap_write(data->regmap, MAX86150_REG_FIFO_CONFIG,
+ MAX86150_FIFO_ROLLOVER_EN |
+ FIELD_PREP(MAX86150_FIFO_A_FULL,
+ MAX86150_FIFO_A_FULL_VAL));
+ if (ret)
+ return ret;
+
+ /* Slot 1 = PPG Red (LED1), Slot 2 = PPG IR (LED2) */
+ ret = regmap_write(data->regmap, MAX86150_REG_FIFO_DCTRL1,
+ FIELD_PREP(MAX86150_FIFO_FD1, MAX86150_FD_LED1) |
+ FIELD_PREP(MAX86150_FIFO_FD2, MAX86150_FD_LED2));
+ if (ret)
+ return ret;
+
+ /* Slot 3 = ECG, Slot 4 = disabled */
+ ret = regmap_write(data->regmap, MAX86150_REG_FIFO_DCTRL2,
+ FIELD_PREP(MAX86150_FIFO_FD3, MAX86150_FD_ECG) |
+ FIELD_PREP(MAX86150_FIFO_FD4, MAX86150_FD_NONE));
+ if (ret)
+ return ret;
+
+ /* PPG: 100 Hz sample rate, 16384 nA ADC full-scale range */
+ ret = regmap_write(data->regmap, MAX86150_REG_PPG_CONFIG1,
+ FIELD_PREP(MAX86150_PPG_ADC_RGE,
+ MAX86150_PPG_ADC_RGE_16384) |
+ FIELD_PREP(MAX86150_PPG_SR,
+ MAX86150_PPG_SR_100HZ));
+ if (ret)
+ return ret;
+
+ /* LED pulse amplitudes (~50 mA) */
+ ret = regmap_write(data->regmap, MAX86150_REG_LED1_PA,
+ MAX86150_LED_PA_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(data->regmap, MAX86150_REG_LED2_PA,
+ MAX86150_LED_PA_DEFAULT);
+ if (ret)
+ return ret;
+
+ /*
+ * Record sample period for timestamp reconstruction in the trigger
+ * handler. The PPG_SR field is fixed to 100 Hz in this driver.
+ */
+ data->sample_period_ns = 10000000; /* 100 Hz = 10 ms */
+
+ /*
+ * Assert SYS_SHDN so the LED drivers do not draw current while
+ * the driver is bound but no capture is active.
+ * set_trigger_state() clears SHDN when the IIO buffer is enabled
+ * and re-asserts it when disabled. read_raw() wakes and sleeps
+ * the device around each single-shot read.
+ */
+ return regmap_write(data->regmap, MAX86150_REG_SYS_CTRL,
+ MAX86150_SYS_SHDN);
+}
+
+/* Probe */
+
+static int max86150_probe(struct i2c_client *client)
+{
+ struct iio_dev *indio_dev;
+ struct max86150_data *data;
+ unsigned int part_id;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ data = iio_priv(indio_dev);
+ data->dev = &client->dev;
+
+ /*
+ * Enable power supplies before any I2C access. Both supplies are
+ * optional in the device tree; use _optional variant so probing
+ * succeeds on boards that power the device from fixed rails with no
+ * DT regulator node.
+ */
+ ret = devm_regulator_get_enable_optional(&client->dev, "vdd");
+ if (ret && ret != -ENODEV)
+ return dev_err_probe(&client->dev, ret,
+ "Failed to get/enable vdd supply\n");
+
+ ret = devm_regulator_get_enable_optional(&client->dev, "leds");
+ if (ret && ret != -ENODEV)
+ return dev_err_probe(&client->dev, ret,
+ "Failed to get/enable leds supply\n");
+
+ data->regmap = devm_regmap_init_i2c(client, &max86150_regmap_config);
+ if (IS_ERR(data->regmap))
+ return dev_err_probe(&client->dev, PTR_ERR(data->regmap),
+ "Failed to initialise regmap\n");
+
+ ret = regmap_read(data->regmap, MAX86150_REG_PART_ID, &part_id);
+ if (ret)
+ return dev_err_probe(&client->dev, ret,
+ "Cannot read part ID\n");
+
+ if (part_id != MAX86150_PART_ID_VAL)
+ return dev_err_probe(&client->dev, -ENODEV,
+ "Unexpected part ID 0x%02x (expected 0x%02x)\n",
+ part_id, MAX86150_PART_ID_VAL);
+
+ ret = max86150_chip_init(data);
+ if (ret)
+ return dev_err_probe(&client->dev, ret,
+ "Chip initialisation failed\n");
+
+ ret = devm_add_action_or_reset(&client->dev, max86150_powerdown, data);
+ if (ret)
+ return ret;
+
+ indio_dev->name = "max86150";
+ indio_dev->channels = max86150_channels;
+ indio_dev->num_channels = ARRAY_SIZE(max86150_channels);
+ indio_dev->info = &max86150_iio_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ /*
+ * If the device tree provides an interrupt, set up a hardware
+ * trigger so userspace can use the FIFO almost-full signal to
+ * drive capture without polling.
+ */
+ if (client->irq > 0) {
+ unsigned long irq_trig;
+
+ data->trig = devm_iio_trigger_alloc(&client->dev,
+ "%s-dev%d",
+ indio_dev->name,
+ iio_device_id(indio_dev));
+ if (!data->trig)
+ return -ENOMEM;
+
+ data->trig->ops = &max86150_trigger_ops;
+ iio_trigger_set_drvdata(data->trig, indio_dev);
+
+ /*
+ * Honour the interrupt trigger type from the device tree.
+ * Fall back to falling-edge if the DT does not specify one.
+ */
+ irq_trig = irq_get_trigger_type(client->irq);
+ if (!irq_trig)
+ irq_trig = IRQF_TRIGGER_FALLING;
+
+ ret = devm_request_irq(&client->dev, client->irq,
+ iio_trigger_generic_data_rdy_poll,
+ irq_trig,
+ "max86150", data->trig);
+ if (ret)
+ return dev_err_probe(&client->dev, ret,
+ "Cannot request IRQ %d\n",
+ client->irq);
+
+ ret = devm_iio_trigger_register(&client->dev, data->trig);
+ if (ret)
+ return dev_err_probe(&client->dev, ret,
+ "Cannot register trigger\n");
+ }
+
+ ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev,
+ iio_pollfunc_store_time,
+ max86150_trigger_handler,
+ NULL);
+ if (ret)
+ return dev_err_probe(&client->dev, ret,
+ "Cannot setup triggered buffer\n");
+
+ return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+/* I2C driver table */
+
+static const struct i2c_device_id max86150_id[] = {
+ { "max86150" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, max86150_id);
+
+static const struct of_device_id max86150_of_match[] = {
+ { .compatible = "maxim,max86150" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, max86150_of_match);
+
+static struct i2c_driver max86150_driver = {
+ .driver = {
+ .name = "max86150",
+ .of_match_table = max86150_of_match,
+ },
+ .probe = max86150_probe,
+ .id_table = max86150_id,
+};
+module_i2c_driver(max86150_driver);
+
+MODULE_AUTHOR("Md Shofiqul Islam <shofiqtest@gmail.com>");
+MODULE_DESCRIPTION("MAX86150 ECG and PPG biosensor driver");
+MODULE_LICENSE("GPL");
--
2.51.1
^ permalink raw reply related
* [PATCH v4 1/3] dt-bindings: iio: health: add maxim,max86150
From: Md Shofiqul Islam @ 2026-06-23 17:45 UTC (permalink / raw)
To: linux-iio
Cc: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, lars,
devicetree, linux-kernel, Md Shofiqul Islam
In-Reply-To: <20260623174600.17100-1-shofiqtest@gmail.com>
Add YAML binding schema for the Maxim MAX86150 combined ECG and PPG
biosensor. The device exposes two PPG optical channels (Red and IR LED)
for heart rate and SpO2 measurement, and one ECG biopotential channel, all
accessible over I2C at up to 400 kHz.
An optional active-low interrupt line connects to the 32-entry hardware
FIFO almost-full output. Two optional regulator supplies (vdd for the
digital core and leds for the LED anodes) cover boards that require
explicit power sequencing.
Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
---
.../bindings/iio/health/maxim,max86150.yaml | 67 +++++++++++++++++++
1 file changed, 67 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/health/maxim,max86150.yaml
diff --git a/Documentation/devicetree/bindings/iio/health/maxim,max86150.yaml b/Documentation/devicetree/bindings/iio/health/maxim,max86150.yaml
new file mode 100644
index 000000000000..1bf10fd1a3d2
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/health/maxim,max86150.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/health/maxim,max86150.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim MAX86150 ECG and PPG Biosensor
+
+maintainers:
+ - Md Shofiqul Islam <shofiqtest@gmail.com>
+
+description: |
+ The MAX86150 is an integrated biosensor SoC that combines:
+ - Two PPG (photoplethysmography) channels: Red LED and IR LED,
+ for heart rate and blood-oxygen saturation (SpO2) measurement.
+ - One ECG (electrocardiogram) channel for biopotential recording.
+
+ The device communicates over I2C at up to 400 kHz and raises an
+ active-low interrupt when the 32-entry hardware FIFO reaches its
+ configurable almost-full threshold.
+
+ Datasheet:
+ https://www.analog.com/media/en/technical-documentation/data-sheets/MAX86150.pdf
+
+properties:
+ compatible:
+ const: maxim,max86150
+
+ reg:
+ maxItems: 1
+ description: I2C device address, always 0x5E.
+
+ interrupts:
+ maxItems: 1
+ description: |
+ Active-low interrupt line. Asserted when the FIFO almost-full
+ threshold is reached or when a new PPG sample is ready.
+
+ vdd-supply:
+ description: Digital core supply, 1.71 V to 1.89 V.
+
+ leds-supply:
+ description: LED anode supply, 3.0 V to 5.5 V.
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ biosensor@5e {
+ compatible = "maxim,max86150";
+ reg = <0x5e>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <5 IRQ_TYPE_EDGE_FALLING>;
+ vdd-supply = <&vdd_1v8>;
+ leds-supply = <&vdd_3v3>;
+ };
+ };
--
2.51.1
^ permalink raw reply related
* [PATCH v4 0/3] iio: health: add MAX86150 ECG and PPG biosensor driver
From: Md Shofiqul Islam @ 2026-06-23 17:45 UTC (permalink / raw)
To: linux-iio
Cc: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, lars,
devicetree, linux-kernel, Md Shofiqul Islam
In-Reply-To: <20260623155556.13701-1-shofiqtest@gmail.com>
Changes in v4 (addressing Sashiko review of v3):
- [High] Fix optional regulator probe failure: treat -ENODEV from
devm_regulator_get_enable_optional() as success (supply absent,
not an error).
- [High] Align fifo_raw to ARCH_DMA_MINALIGN to satisfy DMA mapping
requirements of I2C host controllers that use DMA for burst transfers.
- [High] Disambiguate FIFO empty vs exactly-full: when wr_ptr == rd_ptr
with OVF_COUNTER == 0, consult the A_FULL interrupt status bit to
determine whether the FIFO pointer wrapped to full or is truly empty.
- [High] Remove iio_trigger_get() in probe: the incremented refcount
leaks on the error path when devm_iio_device_register() fails because
iio_device_unregister() (and its paired iio_trigger_put()) never runs.
Users set the trigger via the current_trigger sysfs attribute as normal.
- [High] Assert SYS_SHDN in chip_init() so the LED drivers draw no
current while capture is inactive. set_trigger_state() clears SHDN
when the buffer is enabled and re-asserts it when disabled.
read_raw() wakes and sleeps the device around each single-shot read.
- [Medium] Replace IRQF_TRIGGER_FALLING with irq_get_trigger_type() to
honour the interrupt trigger type from the device tree; falls back to
falling-edge if the DT does not specify one.
- [Medium] Add .validate_trigger = iio_trigger_validate_own_device to
prevent incompatible external triggers from being attached.
- [Medium] Fix per-sample timestamp jitter: anchor timestamps to the
A_FULL IRQ capture time. The sample at index (A_FULL_SAMPLES - 1)
corresponds to pf->timestamp; samples accumulated between the IRQ and
handler execution receive future timestamps, eliminating scheduling-
latency-dependent jitter in multi-sample drains.
Link: https://lore.kernel.org/linux-iio/20260623155556.13701-1-shofiqtest@gmail.com/
v3 cover letter
Md Shofiqul Islam (3):
dt-bindings: iio: health: add maxim,max86150
iio: health: add MAX86150 ECG and PPG biosensor driver
MAINTAINERS: add entry for MAX86150 IIO health driver
--
2.49.0
^ permalink raw reply
* Re: [PATCH v4 5/5] clk: rockchip: rk3588: add GATE_GRF clocks for I2S MCLK output to IO
From: Diederik de Haas @ 2026-06-23 17:42 UTC (permalink / raw)
To: Daniele Briguglio, Diederik de Haas, Heiko Stuebner,
Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Nicolas Frattaroli, linux-clk, devicetree, linux-arm-kernel,
linux-rockchip, linux-kernel, Ricardo Pardini
In-Reply-To: <20260623140958.4181297-1-hello@superkali.me>
Hi,
On Tue Jun 23, 2026 at 4:10 PM CEST, Daniele Briguglio wrote:
>> So IIUC that means I'd be testing both variants.
>
> Right, that covers both: the mux path and the consumer path. Looking
> forward to the results.
Analog audio works on both. Plus with _TO_IO and LTS without in their
respective DTS's. So I guess CLK_IGNORE_UNUSED works.
Whether it's a good/right fix, I'll leave up to others.
Cheers,
Diederik
^ permalink raw reply
* Re: [PATCH 2/3] dt-bindings: hwmon: pmbus: Support for onsemi's FD5121
From: Conor Dooley @ 2026-06-23 17:36 UTC (permalink / raw)
To: Selvamani.Rajagopal
Cc: Guenter Roeck, Jonathan Corbet, Shuah Khan, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-hwmon, linux-doc,
linux-kernel, devicetree
In-Reply-To: <20260622-support-fd5121-from-onsemi-v1-2-b31767689c65@onsemi.com>
[-- Attachment #1: Type: text/plain, Size: 2269 bytes --]
On Mon, Jun 22, 2026 at 10:55:38PM -0700, Selvamani Rajagopal via B4 Relay wrote:
> From: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>
>
> Add devicetree schema for onsemi FD5121, FD5123, and
> FD5125 dual rail, multi-phase digital controllers.
>
> Signed-off-by: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>
> ---
> .../bindings/hwmon/pmbus/onnn,fd5121.yaml | 41 ++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/onnn,fd5121.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/onnn,fd5121.yaml
> new file mode 100644
> index 000000000000..b0453b0634f0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/onnn,fd5121.yaml
> @@ -0,0 +1,41 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/pmbus/onnn,fd5121.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: onsemi's multi-phase digital controllers
Can someone explain to me what a "digital controller" actually is?
Seems very generi and that a word may have been left out, were it not
for the fact that this wording is used several times in the patch.
> +
> +maintainers:
> + - Selvamani Rajagopal <selvamani.rajagopal@onsemi.com>
> +
> +description:
> + onsemi multi-phase digital controllers with PMBus.
> +
> +properties:
> + compatible:
> + enum:
> + - onnn,fd5121
> + - onnn,fd5123
> + - onnn,fd5125
Your /OF/ match data in your driver suggests that you intended to permit
fallback compatibles here?
|+#ifdef CONFIG_OF
|+static const struct of_device_id fd5121_of_match[] = {
|+ { .compatible = "onnn,fd5121" },
|+ { }
|+};
|+MODULE_DEVICE_TABLE(of, fd5121_of_match);
|+#endif
pw-bot: changes-requested
Cheers,
Conor.
> +
> + reg:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + fd5121@50 {
> + compatible = "onnn,fd5121";
> + reg = <0x50>;
> + };
> + };
>
> --
> 2.43.0
>
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH] dt-bindings: clock: renesas,versaclock7: Update maintainer
From: Geert Uytterhoeven @ 2026-06-23 17:27 UTC (permalink / raw)
To: Biju
Cc: Alex Helms, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm, Biju Das,
Brian Masney, linux-renesas-soc, linux-clk, devicetree,
linux-kernel, Prabhakar Mahadev Lad
In-Reply-To: <20260623162039.153291-1-biju.das.jz@bp.renesas.com>
On Tue, 23 Jun 2026 at 18:20, Biju <biju.das.au@gmail.com> wrote:
> From: Biju Das <biju.das.jz@bp.renesas.com>
>
> Alex's email is bouncing. Update the maintainers list with my contact
> details to take over the schema maintenance.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-clk for v7.3.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v8 1/3] dt-bindings: pwm: dwc: Document optional resets property
From: Conor Dooley @ 2026-06-23 17:32 UTC (permalink / raw)
To: dongxuyang
Cc: ukleinek, robh, krzk+dt, conor+dt, ben-linux, ben.dooks, p.zabel,
linux-pwm, devicetree, linux-kernel, ningyu, linmin, xuxiang,
wangguosheng, pinkesh.vaghela
In-Reply-To: <20260623071416.2092-1-dongxuyang@eswincomputing.com>
[-- Attachment #1: Type: text/plain, Size: 75 bytes --]
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v8 2/3] dt-bindings: pwm: dwc: Add eswin compatible
From: Conor Dooley @ 2026-06-23 17:32 UTC (permalink / raw)
To: dongxuyang
Cc: ukleinek, robh, krzk+dt, conor+dt, ben-linux, ben.dooks, p.zabel,
linux-pwm, devicetree, linux-kernel, ningyu, linmin, xuxiang,
wangguosheng, pinkesh.vaghela
In-Reply-To: <20260623071438.2147-1-dongxuyang@eswincomputing.com>
[-- Attachment #1: Type: text/plain, Size: 75 bytes --]
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2 04/12] dt-bindings: nvmem: microchip,sama7g5-otpc: add sama7d65 and dt node example
From: Conor Dooley @ 2026-06-23 17:28 UTC (permalink / raw)
To: Varshini Rajendran
Cc: ehristev, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt,
nicolas.ferre, alexandre.belloni, claudiu.beznea, srini,
linux-iio, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20260623105944.128840-5-varshini.rajendran@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 1995 bytes --]
On Tue, Jun 23, 2026 at 04:29:36PM +0530, Varshini Rajendran wrote:
> Add support for sama7d65 and a dt node example that shows tag can be used
> to reference a packet stored in the OTP memory.
>
> Signed-off-by: Varshini Rajendran <varshini.rajendran@microchip.com>
> ---
> .../nvmem/microchip,sama7g5-otpc.yaml | 28 +++++++++++++++++--
> 1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml b/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
> index cc25f2927682..3cc16b0044a6 100644
> --- a/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
> @@ -20,9 +20,15 @@ allOf:
>
> properties:
> compatible:
> - items:
> - - const: microchip,sama7g5-otpc
> - - const: syscon
> + oneOf:
> + - items:
> + - const: microchip,sama7g5-otpc
> + - const: syscon
> + - items:
> + - enum:
> + - microchip,sama7d65-otpc
> + - const: microchip,sama7g5-otpc
> + - const: syscon
>
> reg:
> maxItems: 1
> @@ -48,4 +54,20 @@ examples:
> };
> };
>
> + - |
> + otp_controller: efuse@e8c00000 {
Drop the unused label since you have to respin the series.
With it gone
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
Cheers,
Conor.
> + compatible = "microchip,sama7d65-otpc", "microchip,sama7g5-otpc", "syscon";
> + reg = <0xe8c00000 0x100>;
> +
> + nvmem-layout {
> + compatible = "fixed-layout";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + temp_calib: calib@41435354 {
> + reg = <0x41435354 0x2c>; /* Temp calib data packet TAG */
> + };
> + };
> + };
> +
> ...
> --
> 2.34.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2 01/12] dt-bindings: iio: adc: at91-sama5d2: document sama7d65
From: Conor Dooley @ 2026-06-23 17:26 UTC (permalink / raw)
To: Varshini Rajendran
Cc: ehristev, jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt,
nicolas.ferre, alexandre.belloni, claudiu.beznea, srini,
linux-iio, devicetree, linux-arm-kernel, linux-kernel,
Krzysztof Kozlowski
In-Reply-To: <20260623105944.128840-2-varshini.rajendran@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 1071 bytes --]
On Tue, Jun 23, 2026 at 04:29:33PM +0530, Varshini Rajendran wrote:
> Add dt-binding documentation for sama7d65 ADC.
Commit message is missing an explanation of why a fallback is not
suitable.
pw-bot: changes-requested
Thanks,
Conor
>
> Signed-off-by: Varshini Rajendran <varshini.rajendran@microchip.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> Documentation/devicetree/bindings/iio/adc/atmel,sama5d2-adc.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/atmel,sama5d2-adc.yaml b/Documentation/devicetree/bindings/iio/adc/atmel,sama5d2-adc.yaml
> index 4817b840977a..e8a65fdcd018 100644
> --- a/Documentation/devicetree/bindings/iio/adc/atmel,sama5d2-adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/atmel,sama5d2-adc.yaml
> @@ -15,6 +15,7 @@ properties:
> - atmel,sama5d2-adc
> - microchip,sam9x60-adc
> - microchip,sama7g5-adc
> + - microchip,sama7d65-adc
>
> reg:
> maxItems: 1
> --
> 2.34.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH] of_numa: fix return -EINVAL when numa-node-id is not found in last node
From: Sang-Heon Jeon @ 2026-06-23 17:08 UTC (permalink / raw)
To: Rob Herring
Cc: Saravana Kannan, Andrew Morton, Dan Williams, David Hildenbrand,
devicetree, Mike Rapoport (Microsoft)
In-Reply-To: <CAL_JsqJ9Aedo1HA7mrb=99eAQPJ2L5G7P9inL5D4AHfqrvuxBw@mail.gmail.com>
On Tue, Jun 23, 2026 at 9:53 PM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Jun 23, 2026 at 7:08 AM Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
> >
> > On Tue, Jun 23, 2026 at 10:42 AM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Sun, Jun 21, 2026 at 11:39:18PM +0900, Sang-Heon Jeon wrote:
> > > > When the numa-node-id property is not found in the last memory node,
> > > > of_property_read_u32() returns -EINVAL, which then becomes the return
> > > > value of of_numa_parse_memory_nodes(), even though earlier memory nodes
> > > > were parsed successfully.
> > > >
> > > > Commit 7e488677a54a ("of, numa: return -EINVAL when no numa-node-id is
> > > > found") meant -EINVAL to be returned only when the numa-node-id property
> > > > is not found at all, not when it is found in an earlier memory node but
> > > > not in the last.
> > > >
> > > > Check whether at least one memory node was parsed successfully, and return
> > > > 0 in that case, -EINVAL otherwise, so the return value of
> > > > of_property_read_u32() for the last memory node no longer corrupts the
> > > > overall return value.
> > >
> > > IDK, it's arguable that an incomplete DT isn't valid and something we
> > > need to support. Is missing numa-node-id valid or it's just better to
> > > have at least partially
> > > configured NUMA nodes?
> >
> > You're right.
> >
> > Since the first implementation 298535c00a2c ("of, numa: Add NUMA of
> > binding implementation.") skips memory nodes without `numa-node-id`,
> > I only try to fix the error case introduced by 7e488677a54a ("of,
> > numa: return -EINVAL when no numa-node-id is found").
> >
> > IMHO, if changing the logic is acceptable, rejecting DT with an
> > incomplete NUMA configuration also seems reasonable.
> > We already call memblock_validate_numa_coverage() after
> > of_numa_parse_memory_nodes(),
> > so such an incomplete configuration would most likely fail from that
> > validation anyway.
> >
> > So, would it be OK to reject a DT with an incomplete NUMA
> > configuration? If so, I'll make a patch in that direction instead.
>
> Reject? As in panic()? No, because why would we make a system that
> boots not boot and a panic at that stage won't be seen because your
> console is not up yet. If you want to add a warning that the NUMA
> information is incomplete, that would be fine.
Sorry for the confusion. Previous "reject" meant returning a non-zero
value from of_numa_parse_memory_nodes().
When of_numa_parse_memory_nodes() returns non-zero, NUMA
initialization from DT is considered as failed and dummy_numa_init()
is called,
which leaves the system with a single node covering all memory.
In conclusion, the direction that i meant is returning -EINVAL instead
of skipping no `numa-node-id` node,
> Also, are we sure it's not valid to have some memory associated with a
> numa node and some that is not?
As I described above, we already call
memblock_validate_numa_coverage() after of_numa_parse_memory_nodes()
success.
The threshold bytes of no node assigned memory is 0(arm64 / riscv) or
1MB(x86, introduced for e820/SRAT mismatch, but shared with DTB path
as well),
so in practice existing logic already doesn't allow no node assigned
memory, which corresponds to a missing `numa-node-id` in DT.
So as far as I know, the only case that passes
memblock_validate_numa_coverage() with a DT missing `numa-node-id` is
when numa_cleanup_meminfo() join two blocks of the same node (same
scenario as QEMU test case). An unassigned region exists between two
blocks of the same node, and no other node's memory exists in it.
So, in my opinion, at least for now, partial existence of
`numa-node-id` DT is not valid.
But maybe, somewhere, someone might want to skip `numa-node-id` for
the memory in the middle of the same node. So I can't be 100% sure.
Since neither of us is sure whether such a DT is valid, I'd rather
keep the current patch's direction because it doesn't change existing
policy.
> Rob
And always, thanks for taking the time to review.
Best Regards,
Sang-Heon Jeon
^ permalink raw reply
* Re: [PATCH] dt-bindings: clock: renesas,versaclock7: Update maintainer
From: Brian Masney @ 2026-06-23 17:00 UTC (permalink / raw)
To: Biju
Cc: Geert Uytterhoeven, Alex Helms, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm,
Biju Das, linux-renesas-soc, linux-clk, devicetree, linux-kernel,
Prabhakar Mahadev Lad
In-Reply-To: <20260623162039.153291-1-biju.das.jz@bp.renesas.com>
On Tue, Jun 23, 2026 at 05:20:37PM +0100, Biju wrote:
> From: Biju Das <biju.das.jz@bp.renesas.com>
>
> Alex's email is bouncing. Update the maintainers list with my contact
> details to take over the schema maintenance.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> Ref [1]
> [1] https://lore.kernel.org/all/ajqWevofEJ3fv856@redhat.com/
Reviewed-by: Brian Masney <bmasney@redhat.com>
Thanks for submitting this quickly.
Brian
^ permalink raw reply
* Re: [PATCH v3 2/3] iio: health: add MAX86150 ECG and PPG biosensor driver
From: sashiko-bot @ 2026-06-23 16:57 UTC (permalink / raw)
To: Md Shofiqul Islam; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260623164309.16452-3-shofiqtest@gmail.com>
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [High] Probe fails unconditionally when optional regulators are absent.
- [High] The `fifo_raw` buffer is not cacheline aligned, violating DMA API rules and risking memory corruption.
- [High] FIFO pointer math cannot distinguish an empty FIFO from an exactly full FIFO, permanently stalling capture.
- [High] Trigger reference leak on error path if triggered buffer setup fails.
- [High] The biosensor is fully powered on and pulsing its LEDs continuously while the driver is bound, causing excessive power drain.
- [Medium] The driver hardcodes `IRQF_TRIGGER_FALLING`, overriding the device tree interrupt configuration.
- [Medium] The driver lacks a `validate_trigger` callback, allowing incompatible external software triggers to corrupt FIFO reads.
- [Medium] Per-sample timestamps are incorrectly reconstructed backwards from the most recent sample, introducing load-dependent jitter.
--
commit b7b1aa92d36a3970970aa6f40becae60ad1ec8c5
Author: Md Shofiqul Islam <shofiqtest@gmail.com>
Subject: iio: health: add MAX86150 ECG and PPG biosensor driver
This commit adds an I2C driver for the MAX86150 ECG and PPG biosensor,
exposing three IIO channels. It includes a hardware FIFO triggered buffer
path and devm regulator support.
> diff --git a/drivers/iio/health/max86150.c b/drivers/iio/health/max86150.c
> --- /dev/null
> +++ b/drivers/iio/health/max86150.c
> @@ -113,8 +113,8 @@ struct max86150_data {
> struct regmap *regmap;
> struct device *dev;
> struct iio_trigger *trig;
> u32 sample_period_ns;
> u8 fifo_raw[MAX86150_SAMPLE_BYTES];
> s32 buf[6] __aligned(8);
> };
[Severity: High]
Does this struct layout risk memory corruption during DMA burst reads?
Since data->fifo_raw is passed to regmap_noinc_read(), which may perform DMA
burst transfers on certain I2C host controllers, it lacks cacheline alignment.
If the CPU flushes dirty cache lines encompassing the adjacent buf during the
DMA transfer, wouldn't the flush clobber the incoming DMA data? Could we add
__aligned(IIO_DMA_MINALIGN) to fifo_raw?
[ ... ]
> @@ -286,4 +286,4 @@ static int max86150_read_raw(struct iio_dev *indio_dev,
> static const struct iio_info max86150_iio_info = {
> .read_raw = max86150_read_raw,
> };
[Severity: Medium]
Should max86150_iio_info implement a validate_trigger callback?
This device relies on its hardware FIFO being correctly flushed and configured
only when its own hardware trigger is enabled via .set_trigger_state. Without
a validate_trigger check like iio_validate_own_trigger, userspace could bind
arbitrary software triggers (like sysfs or hrtimer).
If an external trigger is used, .set_trigger_state is bypassed, the hardware
interrupt is never enabled, and wouldn't the driver blindly read unaligned,
duplicated, or stale samples from the continuously running FIFO?
[ ... ]
> @@ -375,6 +375,6 @@ static irqreturn_t max86150_trigger_handler(int irq, void *p)
> if (ovf > 0)
> n_avail = MAX86150_FIFO_DEPTH;
> else
> n_avail = (wr_ptr - rd_ptr) & (MAX86150_FIFO_DEPTH - 1);
[Severity: High]
Does this logic correctly handle an exactly full FIFO?
If the FIFO reaches exactly 32 samples (completely full) but has not yet
overflowed (ovf == 0), wr_ptr will wrap and equal rd_ptr. This calculation
evaluates to n_avail = 0, causing the driver to skip reading any samples.
Because the interrupt is configured as edge-triggered, the line won't assert
again until the FIFO drops below the almost-full threshold. Will this
permanently stall the triggered buffer capture?
> @@ -387,4 +387,4 @@ static irqreturn_t max86150_trigger_handler(int irq, void *p)
> for (i = 0; i < n_avail; i++) {
> s64 ts = pf->timestamp -
> (s64)(n_avail - 1 - i) * data->sample_period_ns;
[Severity: Medium]
Are the per-sample timestamps aligned correctly?
iio_pollfunc_store_time() records the interrupt capture time in pf->timestamp
when the FIFO hits the almost-full watermark (17 samples). If the threaded
IRQ handler is delayed by system load, additional samples may accumulate.
This loop assigns pf->timestamp to the newest sample (i = n_avail - 1)
instead of the watermark sample, backdating the rest. Doesn't this variable
scheduling latency shift all timestamps backwards and corrupt their regular
spacing?
[ ... ]
> @@ -441,7 +441,7 @@ static int max86150_chip_init(struct max86150_data *data)
>
> /* PPG: 100 Hz sample rate, 16384 nA ADC full-scale range */
> ret = regmap_write(data->regmap, MAX86150_REG_PPG_CONFIG1,
> FIELD_PREP(MAX86150_PPG_ADC_RGE,
> MAX86150_PPG_ADC_RGE_16384) |
> FIELD_PREP(MAX86150_PPG_SR,
> MAX86150_PPG_SR_100HZ));
[Severity: High]
Is the biosensor intentionally left continuously powered on?
Because the driver doesn't implement IIO buffer setup_ops (e.g., preenable
and postdisable) or Runtime PM, the hardware remains active permanently after
initialization.
Won't the optical LEDs continuously pulse at 100 Hz (drawing ~50 mA) even
when no userspace process is capturing data?
[ ... ]
> @@ -528,10 +528,10 @@ static int max86150_probe(struct i2c_client *client)
> * DT regulator node.
> */
> ret = devm_regulator_get_enable_optional(&client->dev, "vdd");
> if (ret)
> return dev_err_probe(&client->dev, ret,
> "Failed to get/enable vdd supply\n");
>
> ret = devm_regulator_get_enable_optional(&client->dev, "leds");
> if (ret)
> return dev_err_probe(&client->dev, ret,
> "Failed to get/enable leds supply\n");
[Severity: High]
Does the driver fail to probe on valid configurations without regulators?
If a board supplies power via fixed rails and omits the DT regulator nodes,
devm_regulator_get_enable_optional() correctly returns -ENODEV.
By directly forwarding ret to dev_err_probe() and aborting, doesn't this
prevent the driver from binding on hardware that intentionally omits these
optional regulators?
[ ... ]
> @@ -575,6 +575,6 @@ static int max86150_probe(struct i2c_client *client)
> ret = devm_request_irq(&client->dev, client->irq,
> iio_trigger_generic_data_rdy_poll,
> IRQF_TRIGGER_FALLING,
> "max86150", data->trig);
[Severity: Medium]
Is it safe to hardcode IRQF_TRIGGER_FALLING here?
Statically setting the IRQF_TRIGGER_FALLING flag overrides any trigger type
specified by the firmware in the device tree's interrupts property.
Will this break compatibility with board layouts that use an inverter or
level-shifter on the interrupt line? Should it pass 0 or IRQF_ONESHOT
instead to allow the core to apply the DT-defined polarity?
[ ... ]
> @@ -602,8 +602,8 @@ static int max86150_probe(struct i2c_client *client)
> * the iio_dev is freed. No additional devm action needed.
> */
> indio_dev->trig = iio_trigger_get(data->trig);
> }
>
> ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev,
> iio_pollfunc_store_time,
> max86150_trigger_handler,
> NULL);
[Severity: High]
Is there a trigger reference leak if the buffer setup fails?
iio_trigger_get() increments the default trigger's reference count. The IIO
core only balances this in iio_dev_release() if the INDIO_ALL_TRIGGERED_MODES
mask flag is set in indio_dev->modes.
Since this flag is set inside devm_iio_triggered_buffer_setup(), if the
buffer setup fails, the flag is never set. Consequently, the devm cleanup path
will skip iio_device_unregister_trigger_consumer(). Doesn't this permanently
leak the trigger device memory and its module reference?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260623164309.16452-1-shofiqtest@gmail.com?part=2
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox