Message-ID: <20251012151130.797450e3@jic23-huawei>
Date: Sun, 12 Oct 2025 15:11:30 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Shrikant <raskar.shree97@...il.com>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 dlechner@...libre.com, nuno.sa@...log.com, andy@...nel.org,
 matt@...ostay.sg, skhan@...uxfoundation.org, david.hunter.linux@...il.com,
 linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-kernel-mentees@...ts.linux.dev
Subject: Re: [PATCH 1/2] dt-bindings: iio: max30100: Add pulse-width
 property

On Mon, 6 Oct 2025 08:34:03 +0530
Shrikant <raskar.shree97@...il.com> wrote:

> On Sat, Oct 4, 2025 at 6:42 PM Jonathan Cameron <jic23@...nel.org> wrote:
> >
> > On Sat,  4 Oct 2025 07:26:22 +0530
> > Shrikant Raskar <raskar.shree97@...il.com> wrote:
> >  
> > > The MAX30100 sensor supports multiple LED pulse widths (200us, 400us,
> > > 800us, 1600us). These settings affect measurement resolution and power
> > > consumption. Until now, the driver always defaulted to 1600us.
> > >
> > > Introduce a new device tree property `maxim,pulse-width` that allows
> > > users to select the desired pulse width in microseconds from device
> > > tree.
> > >
> > > Valid values are: 200, 400, 800, 1600.
> > >
> > > This prepares for driver changes that read this property and configure
> > > the SPO2 register accordingly.
> > >
> > > Signed-off-by: Shrikant Raskar <raskar.shree97@...il.com>  
> > Hi Shrikant,
> >
> > Explain why this is in some way related to characteristics of how the
> > system containing this chip is built (wiring, lenses etc).  Otherwise
> > this might instead be something that should be controlled from userspace
> > not firmware.
> >
> > Also, give a little more on why we care about controlling it at all.
> > Is there a usecase where power use of this chip matters?  Mostly I'd expect
> > it to be in measurement equipment with relatively short measuring periods.
> >
> > Jonathan  
> Hi Jonathan,
> 
> Thanks for the feedback.
> 
> The pulse width configuration is indeed dependent on the hardware integration
> of the MAX30100. It affects how much optical energy the LEDs emit per sample,
> which in turn depends on physical factors such as:
> 
>  - The type and thickness of the optical window or lens covering the sensor
>  - The distance between the LED and photodiode
>  - The reflectivity of the skin/contact surface
> 
> For example:
>  - A smartwatch/wearable ring with a thin glass window can operate
> with 200–400 µs pulses to
>    save power, while
>  - A medical-grade pulse oximeter or a sensor mounted behind a thicker
>    protective layer may require 800–1600 µs for reliable signal amplitude.
> 
> I believe it would be appropriate to describe these fixed,
> board-specific characteristics in the Device Tree,
> since they are determined by the hardware design rather than being
> runtime or user-controlled parameters.
> 
> Would it be okay if I send v2 of the patch with the above explanation
> included in the commit message?
Hi Shrikant,

I'd have this excellent detail + examples summarised in the patch description
and also a small amount of description in the actual binding even if that just says
something like
   Description:
     Pulse width in... . Appropriate pulse width is dependant on factors
     such as optical window absorption, distances and expected reflectivity
     of skin / contact surface.
That's just a quick mash up of what you have above, feel free to not use this
particular text! 

The inclusion of target surface reflectivity in there makes me thing that
for some applications we may also need userspace tweaking or some algorithmic
way to increase or decrease the value according to skin characteristics. However
I guess maybe it isn't that sensitive.

Jonathan

> 
> Thanks and regards,
> Shrikant

Powered by blists - more mailing lists