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

Shrikant Raskar posted 2 patches 2 months, 1 week ago
[PATCH v2 1/2] dt-bindings: iio: max30100: Add pulse-width property
Posted by Shrikant Raskar 2 months, 1 week ago
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 <raskar.shree97@gmail.com>

Changes since v1:
Add unit suffix.
Drop redundant description.

Link to v1:
https://lore.kernel.org/all/20251004015623.7019-2-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..5c651a0151cc 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-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>;
         };
-- 
2.43.0
Re: [PATCH v2 1/2] dt-bindings: iio: max30100: Add pulse-width property
Posted by Jonathan Cameron 2 months ago
On Wed,  8 Oct 2025 08:47:36 +0530
Shrikant Raskar <raskar.shree97@gmail.com> wrote:

> 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 <raskar.shree97@gmail.com>
> 
> Changes since v1:
> Add unit suffix.
> Drop redundant description.
> 
> Link to v1:
> https://lore.kernel.org/all/20251004015623.7019-2-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..5c651a0151cc 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-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

> +    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
Posted by Shrikant 2 months ago
On Sun, Oct 12, 2025 at 10:36 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Wed,  8 Oct 2025 08:47:36 +0530
> Shrikant Raskar <raskar.shree97@gmail.com> wrote:
>
> > 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 <raskar.shree97@gmail.com>
> >
> > Changes since v1:
> > Add unit suffix.
> > Drop redundant description.
> >
> > Link to v1:
> > https://lore.kernel.org/all/20251004015623.7019-2-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..5c651a0151cc 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-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.
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
>
> > +    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
Posted by David Lechner 2 months, 1 week ago
On 10/7/25 10:17 PM, Shrikant Raskar wrote:
> 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 <raskar.shree97@gmail.com>
> 
> Changes since v1:
> Add unit suffix.
> Drop redundant description.
> 
> Link to v1:
> https://lore.kernel.org/all/20251004015623.7019-2-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..5c651a0151cc 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-us:
> +    maxItems: 1
> +    description: Pulse width in microseconds

Would be nice to add to the description which pulse width this is referring to.

> +    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

> +
>  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
Posted by Shrikant 2 months ago
On Fri, Oct 10, 2025 at 11:19 PM David Lechner <dlechner@baylibre.com> wrote:
>
> On 10/7/25 10:17 PM, Shrikant Raskar wrote:
> > 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 <raskar.shree97@gmail.com>
> >
> > Changes since v1:
> > Add unit suffix.
> > Drop redundant description.
> >
> > Link to v1:
> > https://lore.kernel.org/all/20251004015623.7019-2-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..5c651a0151cc 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-us:
> > +    maxItems: 1
> > +    description: Pulse width in microseconds
>
> Would 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.
> > +    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.
> 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
> > +
> >  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
Posted by Krzysztof Kozlowski 2 months, 1 week ago
On 08/10/2025 12:17, Shrikant Raskar wrote:
> 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 <raskar.shree97@gmail.com>
> 
> Changes since v1:
> Add unit suffix.
> Drop redundant description.
> 
> Link to v1:
> https://lore.kernel.org/all/20251004015623.7019-2-raskar.shree97@gmail.com/

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
Posted by Shrikant 2 months, 1 week ago
> > Signed-off-by: Shrikant Raskar <raskar.shree97@gmail.com>
> >
> > Changes since v1:
> > Add unit suffix.
> > Drop redundant description.
> >
> > Link to v1:
> > https://lore.kernel.org/all/20251004015623.7019-2-raskar.shree97@gmail.com/
>
> 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
Re: [PATCH v2 1/2] dt-bindings: iio: max30100: Add pulse-width property
Posted by Rob Herring (Arm) 2 months, 1 week ago
On Wed, 08 Oct 2025 08:47:36 +0530, Shrikant Raskar wrote:
> 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 <raskar.shree97@gmail.com>
> 
> Changes since v1:
> Add unit suffix.
> Drop redundant description.
> 
> Link to v1:
> https://lore.kernel.org/all/20251004015623.7019-2-raskar.shree97@gmail.com/
> ---
>  .../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/20251008031737.7321-2-raskar.shree97@gmail.com

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
Posted by Shrikant 2 months, 1 week ago
> 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/20251008031737.7321-2-raskar.shree97@gmail.com
>
> 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.
>
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