1. Patchew
  2. linux
  3. [PATCH 0/2] iio: health: max30100: Add DT pulse-width support
  4. View patch

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

Shrikant Raskar posted 2 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH 2/2] iio: health: max30100: Add pulse-width configuration via DT
Posted by Shrikant Raskar 2 months, 2 weeks ago 
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 2/2] iio: health: max30100: Add pulse-width configuration via DT
Posted by kernel test robot 2 months, 1 week ago 
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 2/2] iio: health: max30100: Add pulse-width configuration via DT
Posted by Jonathan Cameron 2 months, 2 weeks ago 
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;
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.