[PATCH] iio: health: max30100: add config for LED current

[PATCH] iio: health: max30100: add config for LED current

[Matt Ranostay]

Allow the current for both RED and IR LEDs to be set via an device tree
property setting.

This is an optional setting that is useful for applications that have a
known glass attenuation factor.

Signed-off-by: Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/iio/health/max30100.txt     |  7 +++++++
 drivers/iio/health/max30100.c                       | 21 +++++++++++++++------
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/health/max30100.txt b/Documentation/devicetree/bindings/iio/health/max30100.txt
index f6fbac6..009314c 100644
--- a/Documentation/devicetree/bindings/iio/health/max30100.txt
+++ b/Documentation/devicetree/bindings/iio/health/max30100.txt
@@ -11,11 +11,18 @@ Required properties:
   Refer to interrupt-controller/interrupts.txt for generic
   interrupt client node bindings.
 
+Optional properties:
+  - max30100,led-current: configuration for LED current (in mA) while the engine
+    is running. Upper nibble is for the RED LED, and lower nibble the IR LED.
+
+    Refer to the datasheet for the current mapping values.
+
 Example:
 
 max30100@057 {
 	compatible = "maxim,max30100";
 	reg = <57>;
+	max30100,led-current = <0x7f>;
 	interrupt-parent = <&gpio1>;
 	interrupts = <16 2>;
 };
diff --git a/drivers/iio/health/max30100.c b/drivers/iio/health/max30100.c
index 9d1c81f..37e8d7e 100644
--- a/drivers/iio/health/max30100.c
+++ b/drivers/iio/health/max30100.c
@@ -13,7 +13,7 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
  * GNU General Public License for more details.
  *
- * TODO: allow LED current and pulse length controls via device tree properties
+ * TODO: enable pulse length controls via device tree properties
  */
 
 #include <linux/module.h>
@@ -24,6 +24,7 @@
 #include <linux/irq.h>
 #include <linux/i2c.h>
 #include <linux/mutex.h>
+#include <linux/of.h>
 #include <linux/regmap.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
@@ -245,13 +246,21 @@ static irqreturn_t max30100_interrupt_handler(int irq, void *private)
 
 static int max30100_chip_init(struct max30100_data *data)
 {
+	struct device *dev = &data->client->dev;
+	struct device_node *np = dev->of_node;
 	int ret;
+	u32 val;
 
-	/* RED IR LED = 24mA, IR LED = 50mA */
-	ret = regmap_write(data->regmap, MAX30100_REG_LED_CONFIG,
-				(MAX30100_REG_LED_CONFIG_24MA <<
-				 MAX30100_REG_LED_CONFIG_RED_LED_SHIFT) |
-				 MAX30100_REG_LED_CONFIG_50MA);
+	ret = of_property_read_u32(np, "max30100,led-current", &val);
+	if (ret) {
+		/* Default to 24 mA RED LED, 50 mA IR LED */
+		val = (MAX30100_REG_LED_CONFIG_24MA <<
+			MAX30100_REG_LED_CONFIG_RED_LED_SHIFT) |
+			MAX30100_REG_LED_CONFIG_50MA;
+		dev_warn(dev, "no led-current set, defaulting to %d", val);
+	}
+
+	ret = regmap_write(data->regmap, MAX30100_REG_LED_CONFIG, val & 0xFF);
 	if (ret)
 		return ret;
 
-- 
2.6.4

Re: [PATCH] iio: health: max30100: add config for LED current

[Rob Herring]

On Sat, Dec 26, 2015 at 08:38:24PM -0800, Matt Ranostay wrote:
> Allow the current for both RED and IR LEDs to be set via an device tree
> property setting.
> 
> This is an optional setting that is useful for applications that have a
> known glass attenuation factor.
> 
> Signed-off-by: Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  .../devicetree/bindings/iio/health/max30100.txt     |  7 +++++++
>  drivers/iio/health/max30100.c                       | 21 +++++++++++++++------
>  2 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/health/max30100.txt b/Documentation/devicetree/bindings/iio/health/max30100.txt
> index f6fbac6..009314c 100644
> --- a/Documentation/devicetree/bindings/iio/health/max30100.txt
> +++ b/Documentation/devicetree/bindings/iio/health/max30100.txt
> @@ -11,11 +11,18 @@ Required properties:
>    Refer to interrupt-controller/interrupts.txt for generic
>    interrupt client node bindings.
>  
> +Optional properties:
> +  - max30100,led-current: configuration for LED current (in mA) while the engine
> +    is running. Upper nibble is for the RED LED, and lower nibble the IR LED.

Can you use the common LED binding and led-max-microamp instead?

Otherwise, please add "-microamp" suffix and just split to 2 cells for 
RED and IR LEDs. Also, "max30100" is not a vendor prefix. Use maxim.


> +	ret = of_property_read_u32(np, "max30100,led-current", &val);
> +	if (ret) {
> +		/* Default to 24 mA RED LED, 50 mA IR LED */
> +		val = (MAX30100_REG_LED_CONFIG_24MA <<
> +			MAX30100_REG_LED_CONFIG_RED_LED_SHIFT) |
> +			MAX30100_REG_LED_CONFIG_50MA;
> +		dev_warn(dev, "no led-current set, defaulting to %d", val);
> +	}
> +
> +	ret = regmap_write(data->regmap, MAX30100_REG_LED_CONFIG, val & 0xFF);

Huh? You say the units are milliamps, but then you use the DT value 
directly?

Rob

Re: [PATCH] iio: health: max30100: add config for LED current

[Matt Ranostay]

On Tue, Dec 29, 2015 at 10:34 AM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Sat, Dec 26, 2015 at 08:38:24PM -0800, Matt Ranostay wrote:
>> Allow the current for both RED and IR LEDs to be set via an device tree
>> property setting.
>>
>> This is an optional setting that is useful for applications that have a
>> known glass attenuation factor.
>>
>> Signed-off-by: Matt Ranostay <mranostay-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  .../devicetree/bindings/iio/health/max30100.txt     |  7 +++++++
>>  drivers/iio/health/max30100.c                       | 21 +++++++++++++++------
>>  2 files changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/health/max30100.txt b/Documentation/devicetree/bindings/iio/health/max30100.txt
>> index f6fbac6..009314c 100644
>> --- a/Documentation/devicetree/bindings/iio/health/max30100.txt
>> +++ b/Documentation/devicetree/bindings/iio/health/max30100.txt
>> @@ -11,11 +11,18 @@ Required properties:
>>    Refer to interrupt-controller/interrupts.txt for generic
>>    interrupt client node bindings.
>>
>> +Optional properties:
>> +  - max30100,led-current: configuration for LED current (in mA) while the engine
>> +    is running. Upper nibble is for the RED LED, and lower nibble the IR LED.
>
> Can you use the common LED binding and led-max-microamp instead?
>
> Otherwise, please add "-microamp" suffix and just split to 2 cells for
> RED and IR LEDs. Also, "max30100" is not a vendor prefix. Use maxim.

Ok.

>
>
>> +     ret = of_property_read_u32(np, "max30100,led-current", &val);
>> +     if (ret) {
>> +             /* Default to 24 mA RED LED, 50 mA IR LED */
>> +             val = (MAX30100_REG_LED_CONFIG_24MA <<
>> +                     MAX30100_REG_LED_CONFIG_RED_LED_SHIFT) |
>> +                     MAX30100_REG_LED_CONFIG_50MA;
>> +             dev_warn(dev, "no led-current set, defaulting to %d", val);
>> +     }
>> +
>> +     ret = regmap_write(data->regmap, MAX30100_REG_LED_CONFIG, val & 0xFF);
>
> Huh? You say the units are milliamps, but then you use the DT value
> directly?

Yeah they are direct index values in the register. Probably should
make the documentation more clear.

>
> Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html