[PATCH 0/2] iio: health: max30100: Add DT pulse-width support

[PATCH 0/2] iio: health: max30100: Add DT pulse-width support

[Shrikant Raskar]

Add device tree and driver support for configuring the MAX30100
pulse width, addressing a long-standing TODO in the driver.

Tested on Raspberry Pi 3B with MAX30100 breakout module.

Shrikant Raskar (2):
  dt-bindings: iio: max30100: Add pulse-width property
  iio: health: max30100: Add pulse-width configuration via DT

 .../bindings/iio/health/maxim,max30100.yaml   |  6 +++
 drivers/iio/health/max30100.c                 | 39 ++++++++++++++++++-
 2 files changed, 43 insertions(+), 2 deletions(-)

-- 
2.43.0

[PATCH 1/2] dt-bindings: iio: max30100: Add pulse-width property

[Shrikant Raskar]

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@gmail.com>
---
 .../devicetree/bindings/iio/health/maxim,max30100.yaml      | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml b/Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml
index 967778fb0ce8..55aaf2ff919b 100644
--- a/Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml
+++ b/Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml
@@ -27,6 +27,11 @@ properties:
       LED current whilst the engine is running. First indexed value is
       the configuration for the RED LED, and second value is for the IR LED.
 
+  maxim,pulse-width:
+    maxItems: 1
+    description: Pulse width in microseconds
+    enum: [200, 400, 800, 1600]
+
 additionalProperties: false
 
 required:
@@ -44,6 +49,7 @@ examples:
             compatible = "maxim,max30100";
             reg = <0x57>;
             maxim,led-current-microamp = <24000 50000>;
+            maxim,pulse-width = <1600>;
             interrupt-parent = <&gpio1>;
             interrupts = <16 2>;
         };
-- 
2.43.0

[PATCH 2/2] iio: health: max30100: Add pulse-width configuration via DT

[Shrikant Raskar]

The MAX30100 driver previously hardcoded the SPO2 pulse width to
1600us. This patch adds support for reading the pulse width from
device tree (`maxim,pulse-width`) and programming it into the SPO2
configuration register.

If no property is provided, the driver falls back to 1600us to
preserve existing behavior.

Testing:
Hardware: Raspberry Pi 3B + MAX30100 breakout
Verified DT property read in probe()
Confirmed SPO2_CONFIG register written correctly using regmap_read()

Signed-off-by: Shrikant Raskar <raskar.shree97@gmail.com>
---
 drivers/iio/health/max30100.c | 39 +++++++++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/health/max30100.c b/drivers/iio/health/max30100.c
index 814f521e47ae..2b3348c75beb 100644
--- a/drivers/iio/health/max30100.c
+++ b/drivers/iio/health/max30100.c
@@ -5,7 +5,6 @@
  * Copyright (C) 2015, 2018
  * Author: Matt Ranostay <matt.ranostay@konsulko.com>
  *
- * TODO: enable pulse length controls via device tree properties
  */
 
 #include <linux/module.h>
@@ -54,6 +53,9 @@
 #define MAX30100_REG_SPO2_CONFIG		0x07
 #define MAX30100_REG_SPO2_CONFIG_100HZ		BIT(2)
 #define MAX30100_REG_SPO2_CONFIG_HI_RES_EN	BIT(6)
+#define MAX30100_REG_SPO2_CONFIG_200US		0x0
+#define MAX30100_REG_SPO2_CONFIG_400US		0x1
+#define MAX30100_REG_SPO2_CONFIG_800US		0x2
 #define MAX30100_REG_SPO2_CONFIG_1600US		0x3
 
 #define MAX30100_REG_LED_CONFIG			0x09
@@ -306,19 +308,52 @@ static int max30100_led_init(struct max30100_data *data)
 		MAX30100_REG_LED_CONFIG_LED_MASK, reg);
 }
 
+static int max30100_get_pulse_width(unsigned int pwidth_us)
+{
+	switch (pwidth_us) {
+	case 200:
+		return MAX30100_REG_SPO2_CONFIG_200US;
+	case 400:
+		return MAX30100_REG_SPO2_CONFIG_400US;
+	case 800:
+		return MAX30100_REG_SPO2_CONFIG_800US;
+	case 1600:
+		return MAX30100_REG_SPO2_CONFIG_1600US;
+	default:
+		return -EINVAL;
+	}
+}
+
 static int max30100_chip_init(struct max30100_data *data)
 {
 	int ret;
+	unsigned int pulse_us;
+	unsigned int pulse_width;
+	struct device *dev = &data->client->dev;
 
 	/* setup LED current settings */
 	ret = max30100_led_init(data);
 	if (ret)
 		return ret;
 
+	/* Get pulse width from DT, default = 1600us */
+	ret = device_property_read_u32(dev, "maxim,pulse-width", &pulse_us);
+	if (ret) {
+		dev_warn(dev, "no pulse-width defined, defaulting to 1600us\n");
+		pulse_width = MAX30100_REG_SPO2_CONFIG_1600US;
+	} else {
+		pulse_width = max30100_get_pulse_width(pulse_us);
+		if (pulse_width < 0) {
+			dev_err(dev, "invalid pulse-width %u\n", pulse_us);
+			return pulse_width;
+		}
+	}
+
 	/* enable hi-res SPO2 readings at 100Hz */
 	ret = regmap_write(data->regmap, MAX30100_REG_SPO2_CONFIG,
 				 MAX30100_REG_SPO2_CONFIG_HI_RES_EN |
-				 MAX30100_REG_SPO2_CONFIG_100HZ);
+				 MAX30100_REG_SPO2_CONFIG_100HZ |
+				 pulse_width);
 	if (ret)
 		return ret;
 
-- 
2.43.0

Re: [PATCH 1/2] dt-bindings: iio: max30100: Add pulse-width property

[Bhanu Seshu Kumar Valluri]

On 04/10/25 07:26, Shrikant Raskar 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@gmail.com>

The subject prefix [PATCH 1/2] says it's a two part patch series. But I think
you send all changes in a single patch. If single patch use [PATCH] instead
of [PATCH 1/2].

Re: [PATCH 1/2] dt-bindings: iio: max30100: Add pulse-width property

[Shrikant]

On Sat, Oct 4, 2025 at 8:52 AM Bhanu Seshu Kumar Valluri
<bhanuseshukumar@gmail.com> wrote:
>
> On 04/10/25 07:26, Shrikant Raskar 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@gmail.com>
>
> The subject prefix [PATCH 1/2] says it's a two part patch series. But I think
> you send all changes in a single patch. If single patch use [PATCH] instead
> of [PATCH 1/2].
>
>
Thanks for your feedback.
I have shared 2 patches, one for device tree property update and
respective driver update in
MAX30100 driver. Can you please check the patch with subject ' iio:
health: max30100: Add pulse-width configuration via DT' ?

Re: [PATCH 1/2] dt-bindings: iio: max30100: Add pulse-width property

[Jonathan Cameron]

On Sat,  4 Oct 2025 07:26:22 +0530
Shrikant Raskar <raskar.shree97@gmail.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@gmail.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

> ---
>  .../devicetree/bindings/iio/health/maxim,max30100.yaml      | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml b/Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml
> index 967778fb0ce8..55aaf2ff919b 100644
> --- a/Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml
> +++ b/Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml
> @@ -27,6 +27,11 @@ properties:
>        LED current whilst the engine is running. First indexed value is
>        the configuration for the RED LED, and second value is for the IR LED.
>  
> +  maxim,pulse-width:
> +    maxItems: 1
> +    description: Pulse width in microseconds
> +    enum: [200, 400, 800, 1600]
> +
>  additionalProperties: false
>  
>  required:
> @@ -44,6 +49,7 @@ examples:
>              compatible = "maxim,max30100";
>              reg = <0x57>;
>              maxim,led-current-microamp = <24000 50000>;
> +            maxim,pulse-width = <1600>;
>              interrupt-parent = <&gpio1>;
>              interrupts = <16 2>;
>          };

Re: [PATCH 2/2] iio: health: max30100: Add pulse-width configuration via DT

[Jonathan Cameron]

On Sat,  4 Oct 2025 07:26:23 +0530
Shrikant Raskar <raskar.shree97@gmail.com> wrote:

> The MAX30100 driver previously hardcoded the SPO2 pulse width to
> 1600us. This patch adds support for reading the pulse width from
> device tree (`maxim,pulse-width`) and programming it into the SPO2
> configuration register.
> 
> If no property is provided, the driver falls back to 1600us to
> preserve existing behavior.
> 
> Testing:
> Hardware: Raspberry Pi 3B + MAX30100 breakout
> Verified DT property read in probe()
> Confirmed SPO2_CONFIG register written correctly using regmap_read()
A few minor comments inline.
Thanks,

Jonathan

> 
> Signed-off-by: Shrikant Raskar <raskar.shree97@gmail.com>
> ---
>  drivers/iio/health/max30100.c | 39 +++++++++++++++++++++++++++++++++--
>  1 file changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/health/max30100.c b/drivers/iio/health/max30100.c
> index 814f521e47ae..2b3348c75beb 100644
> --- a/drivers/iio/health/max30100.c
> +++ b/drivers/iio/health/max30100.c
> @@ -5,7 +5,6 @@
>   * Copyright (C) 2015, 2018
>   * Author: Matt Ranostay <matt.ranostay@konsulko.com>
>   *
> - * TODO: enable pulse length controls via device tree properties
>   */
>  
>  #include <linux/module.h>
> @@ -54,6 +53,9 @@
>  #define MAX30100_REG_SPO2_CONFIG		0x07
>  #define MAX30100_REG_SPO2_CONFIG_100HZ		BIT(2)
>  #define MAX30100_REG_SPO2_CONFIG_HI_RES_EN	BIT(6)
> +#define MAX30100_REG_SPO2_CONFIG_200US		0x0
> +#define MAX30100_REG_SPO2_CONFIG_400US		0x1
> +#define MAX30100_REG_SPO2_CONFIG_800US		0x2
>  #define MAX30100_REG_SPO2_CONFIG_1600US		0x3
>  
>  #define MAX30100_REG_LED_CONFIG			0x09
> @@ -306,19 +308,52 @@ static int max30100_led_init(struct max30100_data *data)
>  		MAX30100_REG_LED_CONFIG_LED_MASK, reg);
>  }
>  
> +static int max30100_get_pulse_width(unsigned int pwidth_us)
> +{
> +	switch (pwidth_us) {
> +	case 200:
> +		return MAX30100_REG_SPO2_CONFIG_200US;
> +	case 400:
> +		return MAX30100_REG_SPO2_CONFIG_400US;
> +	case 800:
> +		return MAX30100_REG_SPO2_CONFIG_800US;
> +	case 1600:
> +		return MAX30100_REG_SPO2_CONFIG_1600US;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  static int max30100_chip_init(struct max30100_data *data)
>  {
>  	int ret;
> +	unsigned int pulse_us;
> +	unsigned int pulse_width;
> +	struct device *dev = &data->client->dev;
>  
>  	/* setup LED current settings */
>  	ret = max30100_led_init(data);
>  	if (ret)
>  		return ret;
>  
> +	/* Get pulse width from DT, default = 1600us */
> +	ret = device_property_read_u32(dev, "maxim,pulse-width", &pulse_us);
> +	if (ret) {
> +		dev_warn(dev, "no pulse-width defined, defaulting to 1600us\n");
> +		pulse_width = MAX30100_REG_SPO2_CONFIG_1600US;

Usual trick for these is to set pulse_us to 1600 before calling the
device_property_read_u32(). If that fails then the default value will remain
in the variable and we can just call the code below without needing
to explicitly handle two cases.

> +	} else {
> +		pulse_width = max30100_get_pulse_width(pulse_us);
> +		if (pulse_width < 0) {
> +			dev_err(dev, "invalid pulse-width %u\n", pulse_us);

Only happens in probe() so prefer use of return dev_err_probe()
for compactness in this case.

> +			return pulse_width;
> +		}
> +	}
> +
>  	/* enable hi-res SPO2 readings at 100Hz */
>  	ret = regmap_write(data->regmap, MAX30100_REG_SPO2_CONFIG,
>  				 MAX30100_REG_SPO2_CONFIG_HI_RES_EN |
> -				 MAX30100_REG_SPO2_CONFIG_100HZ);
> +				 MAX30100_REG_SPO2_CONFIG_100HZ |
> +				 pulse_width);

Even though it's the lowest field in this register. I'd prefer
a mask being defined and FIELD_PREP() used to set it.
That way we don't need to know it is the lowest field when looking at this
code.

>  	if (ret)
>  		return ret;
>  

Re: [PATCH 1/2] dt-bindings: iio: max30100: Add pulse-width property

[Shrikant]

On Sat, Oct 4, 2025 at 6:42 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Sat,  4 Oct 2025 07:26:22 +0530
> Shrikant Raskar <raskar.shree97@gmail.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@gmail.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?

Thanks and regards,
Shrikant

Re: [PATCH 1/2] dt-bindings: iio: max30100: Add pulse-width property

[Krzysztof Kozlowski]

On 04/10/2025 10:56, Shrikant Raskar 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.

Don't describe the contents of patch. We see the contents from the diff.
Above two paragraphs are completely redundant, drop.

> 
> This prepares for driver changes that read this property and configure
> the SPO2 register accordingly.

Drop, redundant. Not really relevant.

> 
> Signed-off-by: Shrikant Raskar <raskar.shree97@gmail.com>
> ---
>  .../devicetree/bindings/iio/health/maxim,max30100.yaml      | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml b/Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml
> index 967778fb0ce8..55aaf2ff919b 100644
> --- a/Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml
> +++ b/Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml
> @@ -27,6 +27,11 @@ properties:
>        LED current whilst the engine is running. First indexed value is
>        the configuration for the RED LED, and second value is for the IR LED.
>  
> +  maxim,pulse-width:

You miss unit suffix and testing, most likely.

> +    maxItems: 1
> +    description: Pulse width in microseconds
> +    enum: [200, 400, 800, 1600]x


Best regards,
Krzysztof

Re: [PATCH 2/2] iio: health: max30100: Add pulse-width configuration via DT

[kernel test robot]

Hi Shrikant,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on robh/for-next linus/master v6.17 next-20251008]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Shrikant-Raskar/dt-bindings-iio-max30100-Add-pulse-width-property/20251004-095849
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20251004015623.7019-3-raskar.shree97%40gmail.com
patch subject: [PATCH 2/2] iio: health: max30100: Add pulse-width configuration via DT
config: arm-randconfig-r073-20251004 (https://download.01.org/0day-ci/archive/20251009/202510092124.rc01eF4I-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 8.5.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510092124.rc01eF4I-lkp@intel.com/

smatch warnings:
drivers/iio/health/max30100.c:346 max30100_chip_init() warn: unsigned 'pulse_width' is never less than zero.
drivers/iio/health/max30100.c:346 max30100_chip_init() warn: error code type promoted to positive: 'pulse_width'

vim +/pulse_width +346 drivers/iio/health/max30100.c

   326	
   327	static int max30100_chip_init(struct max30100_data *data)
   328	{
   329		int ret;
   330		unsigned int pulse_us;
   331		unsigned int pulse_width;
   332		struct device *dev = &data->client->dev;
   333	
   334		/* setup LED current settings */
   335		ret = max30100_led_init(data);
   336		if (ret)
   337			return ret;
   338	
   339		/* Get pulse width from DT, default = 1600us */
   340		ret = device_property_read_u32(dev, "maxim,pulse-width", &pulse_us);
   341		if (ret) {
   342			dev_warn(dev, "no pulse-width defined, defaulting to 1600us\n");
   343			pulse_width = MAX30100_REG_SPO2_CONFIG_1600US;
   344		} else {
   345			pulse_width = max30100_get_pulse_width(pulse_us);
 > 346			if (pulse_width < 0) {
   347				dev_err(dev, "invalid pulse-width %u\n", pulse_us);
   348				return pulse_width;
   349			}
   350		}
   351	
   352		/* enable hi-res SPO2 readings at 100Hz */
   353		ret = regmap_write(data->regmap, MAX30100_REG_SPO2_CONFIG,
   354					 MAX30100_REG_SPO2_CONFIG_HI_RES_EN |
   355					 MAX30100_REG_SPO2_CONFIG_100HZ |
   356					 pulse_width);
   357		if (ret)
   358			return ret;
   359	
   360		/* enable SPO2 mode */
   361		ret = regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG,
   362					 MAX30100_REG_MODE_CONFIG_MODE_MASK,
   363					 MAX30100_REG_MODE_CONFIG_MODE_HR_EN |
   364					 MAX30100_REG_MODE_CONFIG_MODE_SPO2_EN);
   365		if (ret)
   366			return ret;
   367	
   368		/* enable FIFO interrupt */
   369		return regmap_update_bits(data->regmap, MAX30100_REG_INT_ENABLE,
   370					 MAX30100_REG_INT_ENABLE_MASK,
   371					 MAX30100_REG_INT_ENABLE_FIFO_EN
   372					 << MAX30100_REG_INT_ENABLE_MASK_SHIFT);
   373	}
   374	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/2] dt-bindings: iio: max30100: Add pulse-width property

[Jonathan Cameron]

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

> On Sat, Oct 4, 2025 at 6:42 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Sat,  4 Oct 2025 07:26:22 +0530
> > Shrikant Raskar <raskar.shree97@gmail.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@gmail.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

Re: [PATCH 1/2] dt-bindings: iio: max30100: Add pulse-width property

[Shrikant]

On Sun, Oct 12, 2025 at 7:41 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon, 6 Oct 2025 08:34:03 +0530
> Shrikant <raskar.shree97@gmail.com> wrote:
>
> > On Sat, Oct 4, 2025 at 6:42 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > >
> > > On Sat,  4 Oct 2025 07:26:22 +0530
> > > Shrikant Raskar <raskar.shree97@gmail.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@gmail.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!
>
Thanks for your response. Sure, I will update the description in the
binding and will
update the commit message describing the details and examples. I will share the
updated version of the patch shortly.
> 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.
Need to check on this point.

Thanks and regards,
 Shrikant