iio: health: max30100: Add DT pulse-width support
Subsystems (2)
v2 cover iio: health: max30100: Add DT pulse-width support
Add support for configuring the LED pulse width of the MAX30100 sensor via a device tree property.
v2 changes:
dt-bindings: iio: max30100: Add pulse-width property:
Add unit suffix (-microseconds)
Drop redundant description
iio: health: max30100: Add pulse-width configuration via DT:
Improve default handling
Used FIELD_PREP() for pulse width
Use dev_err_probe() for error reporting
Fix signedness issue
Tested on: Raspberry Pi 3B + MAX30100 breakout
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 | 35 +++++++++++++++++-- 2 files changed, 39 insertions(+), 2 deletions(-)
Signature · 1 line
2.43.0
PATCH v2 1/2 dt-bindings: iio: max30100: Add pulse-width property
The appropriate LED pulse width for the MAX30100 depends on board-specific optical and mechanical design (lens, enclosure, LED-to-sensor distance) and the trade-off between measurement resolution and power consumption. Encoding it in Device Tree documents these platform choices and ensures consistent behavior.
Tested on: Raspberry Pi 3B + MAX30100 breakout board.
Changes since v1:
Add unit suffix.
Drop redundant description.
Link to v1:
https://lore.kernel.org/all/[email protected]/
--- .../devicetree/bindings/iio/health/maxim,max30100.yaml | 6 ++++++ 1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml +6 -0
@@ -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-us: + 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-us = <1600>; interrupt-parent = <&gpio1>; interrupts = <16 2>; };
Signature · 1 line
2.43.0
Rob Herring (Arm)
maintainerRe: [PATCH v2 1/2] dt-bindings: iio: max30100: Add pulse-width property
Quoted 14 lines · Shrikant Raskar
The appropriate LED pulse width for the MAX30100 depends on board-specific optical and mechanical design (lens, enclosure, LED-to-sensor distance) and the trade-off between measurement resolution and power consumption. Encoding it in Device Tree documents these platform choices and ensures consistent behavior.
Tested on: Raspberry Pi 3B + MAX30100 breakout board.
Signed-off-by Shrikant Raskar <[email protected]>Changes since v1:
Add unit suffix.
Drop redundant description.
Quoted 6 lines
Link to v1:
https://lore.kernel.org/all/[email protected]/--- .../devicetree/bindings/iio/health/maxim,max30100.yaml | 6 ++++++ 1 file changed, 6 insertions(+)
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml: properties:maxim,pulse-width-us: 'enum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
hint: Scalar and array keywords cannot be mixed
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml: properties:maxim,pulse-width-us: 'anyOf' conditional failed, one must be fixed:
'enum' is not one of ['maxItems', 'description', 'deprecated']
hint: Only "maxItems" is required for a single entry if there are no constraints defined for the values.
Additional properties are not allowed ('enum' was unexpected)
hint: Arrays must be described with a combination of minItems/maxItems/items
'maxItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
1 is less than the minimum of 2
hint: Arrays must be described with a combination of minItems/maxItems/items
hint: cell array properties must define how many entries and what the entries are when there is more than one entry.
from schema $id: http://devicetree.org/meta-schemas/core.yaml#
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Re: [PATCH v2 1/2] dt-bindings: iio: max30100: Add pulse-width property
Quoted 30 lines
My bot found errors running 'make
dt_binding_check' on your patch:yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml: properties:maxim,pulse-width-us: 'enum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
hint: Scalar and array keywords cannot be mixed
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml: properties:maxim,pulse-width-us: 'anyOf' conditional failed, one must be fixed:
'enum' is not one of ['maxItems', 'description', 'deprecated']
hint: Only "maxItems" is required for a single entry if there are no constraints defined for the values.
Additional properties are not allowed ('enum' was unexpected)
hint: Arrays must be described with a combination of minItems/maxItems/items
'maxItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
1 is less than the minimum of 2
hint: Arrays must be described with a combination of minItems/maxItems/items
hint: cell array properties must define how many entries and what the entries are when there is more than one entry.
from schema $id: http://devicetree.org/meta-schemas/core.yaml#doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.
If you already ran 'make
dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:
Quoted 6 lines
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note that
DT_SCHEMA_FILEScan be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Hi Rob,
Thank you for reviewing my patch and pointing out the dt-binding schema issues. I’ve updated the YAML to fix the reported warnings. I’ll verify with yamllint and dtschema
to confirm there are no remaining errors and submit a corrected v3 patch shortly.
Thanks for your guidance.
Regards,
Shrikant
Re: [PATCH v2 1/2] dt-bindings: iio: max30100: Add pulse-width property
In reply to Shrikant RaskarQuoted 8 lines · Shrikant Raskar
The appropriate LED pulse width for the MAX30100 depends on board-specific optical and mechanical design (lens, enclosure, LED-to-sensor distance) and the trade-off between measurement resolution and power consumption. Encoding it in Device Tree documents these platform choices and ensures consistent behavior.
Tested on: Raspberry Pi 3B + MAX30100 breakout board.
Quoted 8 lines
Signed-off-by Shrikant Raskar <[email protected]>Changes since v1:
Add unit suffix.
Drop redundant description.
This does not belong to commit msg but to changelog under ---.
See submitting patches.
You need to also start testing your patches BEFORE you send them.
Best regards,
Krzysztof
Re: [PATCH v2 1/2] dt-bindings: iio: max30100: Add pulse-width property
Quoted 6 lines
Quoted 6 lines
Signed-off-by Shrikant Raskar <[email protected]>Changes since v1:
Add unit suffix.
Drop redundant description.
Quoted 9 lines
Quoted 2 lines
This does not belong to commit msg but to changelog under ---.
See submitting patches.
You need to also start testing your patches BEFORE you send them.
Hello Krzysztof ,
Thanks for reviewing the patch and sharing your feedback.
I have removed the changelog from the commit message and added under ---. I will test and will share the updated version of the patch shortly.
Thanks for your guidance.
Regards,
Shrikant
David Lechner
reviewerRe: [PATCH v2 1/2] dt-bindings: iio: max30100: Add pulse-width property
In reply to Shrikant RaskarQuoted 25 lines · Shrikant Raskar
The appropriate LED pulse width for the MAX30100 depends on board-specific optical and mechanical design (lens, enclosure, LED-to-sensor distance) and the trade-off between measurement resolution and power consumption. Encoding it in Device Tree documents these platform choices and ensures consistent behavior.
Tested on: Raspberry Pi 3B + MAX30100 breakout board.
Signed-off-by Shrikant Raskar <[email protected]>Changes since v1:
Add unit suffix.
Drop redundant description.Link to v1:
https://lore.kernel.org/all/[email protected]/--- .../devicetree/bindings/iio/health/maxim,max30100.yaml | 6 ++++++ 1 file changed, 6 insertions(+)Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml +0
-0
Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml properties: · line 32
Quoted 6 lines
Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml +3
-0@@ -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-us: + maxItems: 1 + description: Pulse width in microseconds
Would be nice to add to the description which pulse width this is referring to.
Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml properties: · line 33
Quoted 1 line
Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml +1
-0+ enum: [200, 400, 800, 1600]
Properties with standard unit suffixes are u32 arrays, so I think this would fix the error and also make maxItems not necessary.
items:
- enum: [200, 400, 800, 1600]
And we want to know what the default is if this property is omitted.
default: 1600
Quoted 12 lines
Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml +2
-0+ additionalProperties: false required:@@ -44,6 +49,7 @@ examples: compatible = "maxim,max30100"; reg = <0x57>; maxim,led-current-microamp = <24000 50000>; + maxim,pulse-width-us = <1600>; interrupt-parent = <&gpio1>; interrupts = <16 2>; };
Re: [PATCH v2 1/2] dt-bindings: iio: max30100: Add pulse-width property
Quoted 30 lines · David Lechner
Quoted 28 lines · Shrikant Raskar
The appropriate LED pulse width for the MAX30100 depends on board-specific optical and mechanical design (lens, enclosure, LED-to-sensor distance) and the trade-off between measurement resolution and power consumption. Encoding it in Device Tree documents these platform choices and ensures consistent behavior.
Tested on: Raspberry Pi 3B + MAX30100 breakout board.
Signed-off-by Shrikant Raskar <[email protected]>Changes since v1:
Add unit suffix.
Drop redundant description.Link to v1:
https://lore.kernel.org/all/[email protected]/--- .../devicetree/bindings/iio/health/maxim,max30100.yaml | 6 ++++++ 1 file changed, 6 insertions(+)Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml +0
-0@@ -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.
Quoted 5 lines
Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml properties: · line 32
Quoted 3 lines
Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml +3
-0+ maxim,pulse-width-us: + maxItems: 1 + description: Pulse width in microsecondsWould be nice to add to the description which pulse width this is referring to.
Thanks for your review comment, I have updated the description and shared v3 patch for your review.
Quoted 8 lines
Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml properties: · line 33
Quoted 1 line
Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml +1
-0+ enum: [200, 400, 800, 1600]Properties with standard unit suffixes are u32 arrays, so I think this would fix the error and also make maxItems not necessary.
items:
- enum: [200, 400, 800, 1600]
Thanks for sharing the fix. I have tried it but 'dt_binding_check' complains as below: 'items' is not one of ['description', 'deprecated', 'const', 'enum','minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']. Schema expects it to be defined as a single u32, not an array. I have updated the patch accordingly.
Quoted 4 lines
And we want to know what the default is if this property is omitted.
default: 1600
Thanks for your feedback, I have added the default value and shared v3 patch for your review.
Thanks and Regards,
Shrikant
Quoted 13 lines
Quoted 12 lines
Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml +2
-0+ additionalProperties: false required:@@ -44,6 +49,7 @@ examples: compatible = "maxim,max30100"; reg = <0x57>; maxim,led-current-microamp = <24000 50000>; + maxim,pulse-width-us = <1600>; interrupt-parent = <&gpio1>; interrupts = <16 2>; };
Jonathan Cameron
maintainerRe: [PATCH v2 1/2] dt-bindings: iio: max30100: Add pulse-width property
In reply to Shrikant RaskarQuoted 25 lines · On Wed, 8 Oct 2025 08:47:36 +0530 Shrikant Raskar <[email protected]>…
The appropriate LED pulse width for the MAX30100 depends on board-specific optical and mechanical design (lens, enclosure, LED-to-sensor distance) and the trade-off between measurement resolution and power consumption. Encoding it in Device Tree documents these platform choices and ensures consistent behavior.
Tested on: Raspberry Pi 3B + MAX30100 breakout board.
Signed-off-by Shrikant Raskar <[email protected]>Changes since v1:
Add unit suffix.
Drop redundant description.Link to v1:
https://lore.kernel.org/all/[email protected]/--- .../devicetree/bindings/iio/health/maxim,max30100.yaml | 6 ++++++ 1 file changed, 6 insertions(+)Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml +0
-0
Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml properties: · line 32
Quoted 6 lines
Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml +3
-0@@ -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-us: + maxItems: 1 + description: Pulse width in microseconds
I continued the discussion on v1 but just to make sure it is not missed, add a bit more here briefly touching on factors that govern what the right value is here.
Thanks,
Jonathan
Quoted 13 lines
Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml +3
-0+ 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-us = <1600>; interrupt-parent = <&gpio1>; interrupts = <16 2>; };
Re: [PATCH v2 1/2] dt-bindings: iio: max30100: Add pulse-width property
Quoted 32 lines · Jonathan Cameron
Quoted 28 lines · On Wed, 8 Oct 2025 08:47:36 +0530 Shrikant Raskar <[email protected]>…
The appropriate LED pulse width for the MAX30100 depends on board-specific optical and mechanical design (lens, enclosure, LED-to-sensor distance) and the trade-off between measurement resolution and power consumption. Encoding it in Device Tree documents these platform choices and ensures consistent behavior.
Tested on: Raspberry Pi 3B + MAX30100 breakout board.
Signed-off-by Shrikant Raskar <[email protected]>Changes since v1:
Add unit suffix.
Drop redundant description.Link to v1:
https://lore.kernel.org/all/[email protected]/--- .../devicetree/bindings/iio/health/maxim,max30100.yaml | 6 ++++++ 1 file changed, 6 insertions(+)Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml +0
-0@@ -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.
Quoted 6 lines
Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml properties: · line 32
Quoted 3 lines
Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml +3
-0+ maxim,pulse-width-us: + maxItems: 1 + description: Pulse width in microsecondsI continued the discussion on v1 but just to make sure it is not missed, add a bit more here briefly touching on factors that govern what the right value is here.
I have considered the feedback from v1 and updated the description with suggested factors. I have shared the v3 patches with updates for your review.
Thanks and Regards,
Shrikant
Quoted 15 lines
Quoted 14 lines
Documentation/devicetree/bindings/iio/health/maxim,max30100.yaml +3
-0+ 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-us = <1600>; interrupt-parent = <&gpio1>; interrupts = <16 2>; };
PATCH v2 2/2 iio: health: max30100: Add pulse-width configuration via DT
In reply to Shrikant RaskarThe 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-us) 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()
Changes since v1:
Use FIELD_PREP() and define a pulse width bit mask. Initialize default pulse_us before property read. Use dev_err_probe() for error reporting.
Make pulse_width signed to handle negative return values.
Link to v1:
https://lore.kernel.org/all/[email protected]/
--- drivers/iio/health/max30100.c | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-)
drivers/iio/health/max30100.c +33 -2
@@ -5,7 +5,6 @@* Copyright (C) 2015, 2018 * Author: Matt Ranostay <[email protected]> * -* TODO: enable pulse length controls via device tree properties */ #include <linux/module.h>
@@ -54,6 +53,10 @@#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_PW_MASK GENMASK(1, 0) +#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 +309,47 @@ 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; +int pulse_width; +/* set default pulse-width-us to 1600us */ +unsigned int pulse_us =1600 ; +struct device *dev = &data->client->dev;/* setup LED current settings */ ret =max30100_led_init (data);if (ret)return ret; +/* Read pulse-width-us from DT */ +device_property_read_u32 (dev,"maxim,pulse-width-us" , &pulse_us); + + pulse_width =max30100_get_pulse_width (pulse_us); +if (pulse_width <0 ) +return dev_err_probe (dev, pulse_width,"invalid pulse-width %uus\n" , pulse_us); +/* 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 | +FIELD_PREP (MAX30100_REG_SPO2_CONFIG_PW_MASK , pulse_width));if (ret)return ret;
Signature · 1 line
2.43.0
Re: [PATCH v2 2/2] iio: health: max30100: Add pulse-width configuration via DT
Hi Shrikant,
Thanks for your patch.
Quoted 15 lines · 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-us) 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 inprobe()
ConfirmedSPO2_CONFIGregister written correctly usingregmap_read()Signed-off-by Shrikant Raskar <[email protected]>
Quoted 8 lines
Changes since v1:
UseFIELD_PREP()and define a pulse width bit mask. Initialize defaultpulse_usbefore property read. Usedev_err_probe()for error reporting.
Makepulse_widthsigned to handle negative return values.
As mentioned in the bindings patch, this is not place for changelog. With that fixed:
Quoted 78 lines
---
drivers/iio/health/max30100.c | 35 +++++++++++++++++++++++++++++++++--1 file changed, 33 insertions(+), 2 deletions(-)drivers/iio/health/max30100.c +0
-1@@ -5,7 +5,6 @@* Copyright (C) 2015, 2018 * Author: Matt Ranostay <[email protected]> * -* TODO: enable pulse length controls via device tree properties */
#include <linux/module.h>drivers/iio/health/max30100.c +4
-0@@ -54,6 +53,10 @@ #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_PW_MASK GENMASK(1, 0) +#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
#defineMAX30100_REG_LED_CONFIG0x09drivers/iio/health/max30100.c +0
-0@@ -306,19 +309,47 @@ static int max30100_led_init(struct max30100_data *data) MAX30100_REG_LED_CONFIG_LED_MASK, reg); }drivers/iio/health/max30100.c +16
-0+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;drivers/iio/health/max30100.c +4
-0+int pulse_width; +/* set default pulse-width-us to 1600us */ +unsigned int pulse_us =1600 ; +struct device *dev = &data->client->dev;
/* setup LED current settings */
ret =max30100_led_init(data);
if (ret)
return ret;drivers/iio/health/max30100.c +6
-0+/* Read pulse-width-us from DT */ +device_property_read_u32 (dev,"maxim,pulse-width-us" , &pulse_us); + + pulse_width =max30100_get_pulse_width (pulse_us); +if (pulse_width <0 ) + return dev_err_probe(dev, pulse_width, "invalid pulse-width%uus\n",
pulse_us);
+
/* enable hi-res SPO2 readings at 100Hz */
ret = regmap_write(data->regmap,MAX30100_REG_SPO2_CONFIG,MAX30100_REG_SPO2_CONFIG_HI_RES_EN|drivers/iio/health/max30100.c +2
-1- MAX30100_REG_SPO2_CONFIG_100HZ); +MAX30100_REG_SPO2_CONFIG_100HZ | + FIELD_PREP(MAX30100_REG_SPO2_CONFIG_PW_MASK,
pulse_width));
if (ret)
return ret;
Re: [PATCH v2 2/2] iio: health: max30100: Add pulse-width configuration via DT
Quoted 27 lines · Nuno Sá
Hi Shrikant,
Thanks for your patch.
Quoted 21 lines · 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-us) 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 inprobe()
ConfirmedSPO2_CONFIGregister written correctly usingregmap_read()Signed-off-by Shrikant Raskar <[email protected]>Changes since v1:
UseFIELD_PREP()and define a pulse width bit mask. Initialize defaultpulse_usbefore property read. Usedev_err_probe()for error reporting.
Makepulse_widthsigned to handle negative return values.
Quoted 7 lines
Quoted 2 lines
As mentioned in the bindings patch, this is not place for changelog. With that fixed:
Reviewed-by Nuno Sá <[email protected]>
Hello Nuno Sá,
Thanks for reviewing my patch.
I have removed the changelog from commit message and shared the v3 patch for review.
Thanks and Regards,
Shrikant
Quoted 79 lines
Quoted 79 lines
--- drivers/iio/health/max30100.c | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-)drivers/iio/health/max30100.c +33
-2@@ -5,7 +5,6 @@* Copyright (C) 2015, 2018 * Author: Matt Ranostay <[email protected]> * -* TODO: enable pulse length controls via device tree properties */ #include <linux/module.h> @@ -54,6 +53,10 @@#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_PW_MASK GENMASK(1, 0) +#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 +309,47 @@ 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; +int pulse_width; +/* set default pulse-width-us to 1600us */ +unsigned int pulse_us =1600 ; +struct device *dev = &data->client->dev;/* setup LED current settings */ ret =max30100_led_init (data);if (ret)return ret; +/* Read pulse-width-us from DT */ +device_property_read_u32 (dev,"maxim,pulse-width-us" , &pulse_us); + + pulse_width =max30100_get_pulse_width (pulse_us); +if (pulse_width <0 ) + return dev_err_probe(dev, pulse_width, "invalid pulse-width %uus\n", pulse_us); + /* 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 | +FIELD_PREP (MAX30100_REG_SPO2_CONFIG_PW_MASK , pulse_width));if (ret)return ret;
kernel test robot
build botRe: [PATCH v2 2/2] iio: health: max30100: Add pulse-width configuration via DT
In reply to Shrikant Raskarkernel test robot noticed the following build errors:
Hi Shrikant,
kernel test robot noticed the following build errors:
[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on robh/for-next v6.17]
[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/20251010-102537 base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg patch link: https://lore.kernel.org/r/20251008031737.7321-3-raskar.shree97%40gmail.com patch subject: [PATCH v2 2/2] iio: health: max30100: Add pulse-width configuration via DT config: csky-randconfig-001-20251010 (https://download.01.org/0day-ci/archive/20251011/[email protected]/config) compiler: csky-linux-gcc (GCC) 15.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251011/[email protected]/reproduce)
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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
drivers/iio/health/max30100.c: In function 'max30100_chip_init':
>> drivers/iio/health/max30100.c:352:34: error: implicit declaration of function 'FIELD_PREP' [-Wimplicit-function-declaration]
352 | FIELD_PREP(MAX30100_REG_SPO2_CONFIG_PW_MASK, pulse_width));
| ^~~~~~~~~~vim +/FIELD_PREP +352 drivers/iio/health/max30100.c
327
328 static int max30100_chip_init(struct max30100_data *data)
329 {
330 int ret;
331 int pulse_width;
332 /* set default pulse-width-us to 1600us */
333 unsigned int pulse_us = 1600;
334 struct device *dev = &data->client->dev;
335
336 /* setup LED current settings */
337 ret = max30100_led_init(data);
338 if (ret)
339 return ret;
340
341 /* Read pulse-width-us from DT */
342 device_property_read_u32(dev, "maxim,pulse-width-us", &pulse_us);
343
344 pulse_width = max30100_get_pulse_width(pulse_us);
345 if (pulse_width < 0)
346 return dev_err_probe(dev, pulse_width, "invalid pulse-width %uus\n", pulse_us);
347
348 /* enable hi-res SPO2 readings at 100Hz */
349 ret = regmap_write(data->regmap, MAX30100_REG_SPO2_CONFIG,
350 MAX30100_REG_SPO2_CONFIG_HI_RES_EN |
351 MAX30100_REG_SPO2_CONFIG_100HZ |
> 352 FIELD_PREP(MAX30100_REG_SPO2_CONFIG_PW_MASK, pulse_width));
353 if (ret)
354 return ret;
355
356 /* enable SPO2 mode */
357 ret = regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG,
358 MAX30100_REG_MODE_CONFIG_MODE_MASK,
359 MAX30100_REG_MODE_CONFIG_MODE_HR_EN |
360 MAX30100_REG_MODE_CONFIG_MODE_SPO2_EN);
361 if (ret)
362 return ret;
363
364 /* enable FIFO interrupt */
365 return regmap_update_bits(data->regmap, MAX30100_REG_INT_ENABLE,
366 MAX30100_REG_INT_ENABLE_MASK,
367 MAX30100_REG_INT_ENABLE_FIFO_EN
368 << MAX30100_REG_INT_ENABLE_MASK_SHIFT);
369 }
370
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
David Lechner
reviewerRe: [PATCH v2 2/2] iio: health: max30100: Add pulse-width configuration via DT
In reply to Shrikant RaskarOn 10/7/25 10:17 PM, Shrikant Raskar wrote:
...
Quoted 11 lines
drivers/iio/health/max30100.c +0
-1@@ -5,7 +5,6 @@* Copyright (C) 2015, 2018 * Author: Matt Ranostay <[email protected]> * -* TODO: enable pulse length controls via device tree properties */ #include <linux/module.h>
drivers/iio/health/max30100.c line 59
Quoted 7 lines
drivers/iio/health/max30100.c +3
-0@@ -54,6 +53,10 @@#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_PW_MASK GENMASK(1, 0)> +#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
Would make more sense to put this new code before BIT(2) to preserve the order of lowest to highest bits.