[PATCH v1 1/2] led: leds-lp50xx: Add max-brightness as in leds-pwm

[PATCH v1 1/2] led: leds-lp50xx: Add max-brightness as in leds-pwm

[Astrid Rost]

Add max-brightness in order to reduce the current on the connected LEDs.
Normally, the maximum brightness is determined by the hardware, and this
property is not required. This property is used to set a software limit.
It could happen that an LED is made so bright that it gets damaged or
causes damage due to restrictions in a specific system, such as mounting
conditions.

Signed-off-by: Astrid Rost <astrid.rost@axis.com>
---
 drivers/leds/leds-lp50xx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/leds/leds-lp50xx.c b/drivers/leds/leds-lp50xx.c
index 28d6b39fa72d..1cb54c0524a1 100644
--- a/drivers/leds/leds-lp50xx.c
+++ b/drivers/leds/leds-lp50xx.c
@@ -502,6 +502,8 @@ static int lp50xx_probe_dt(struct lp50xx *priv)
 		led->mc_cdev.num_colors = num_colors;
 		led->mc_cdev.subled_info = mc_led_info;
 		led_cdev = &led->mc_cdev.led_cdev;
+		fwnode_property_read_u32(child, "max-brightness",
+					 &led_cdev->max_brightness);
 		led_cdev->brightness_set_blocking = lp50xx_brightness_set;
 
 		ret = devm_led_classdev_multicolor_register_ext(priv->dev,
-- 
2.30.2

Re: [PATCH v1 1/2] led: leds-lp50xx: Add max-brightness as in leds-pwm

[Andy Shevchenko]

On Thu, Jun 29, 2023 at 03:47:21PM +0200, Astrid Rost wrote:
> Add max-brightness in order to reduce the current on the connected LEDs.
> Normally, the maximum brightness is determined by the hardware, and this
> property is not required. This property is used to set a software limit.
> It could happen that an LED is made so bright that it gets damaged or
> causes damage due to restrictions in a specific system, such as mounting
> conditions.

This will be 4th driver in the LED subsystem that will use this (optional)
property. Perhaps it's time to actually move it to the LED generic bindings
and call this from led_parse_fwnode_props()?

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1 2/2] dt: bindings: lp50xx: Add max-brightness as in leds-pwm

[Astrid Rost]

Add max-brightness in order to reduce the current on the connected LEDs.
Normally, the maximum brightness is determined by the hardware, and this
property is not required. This property is used to set a software limit.
It could happen that an LED is made so bright that it gets damaged or
causes damage due to restrictions in a specific system, such as mounting
conditions.

Signed-off-by: Astrid Rost <astrid.rost@axis.com>
---
 Documentation/devicetree/bindings/leds/leds-lp50xx.yaml | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
index 402c25424525..6a1425969cb4 100644
--- a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
+++ b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
@@ -73,6 +73,15 @@ patternProperties:
       '#size-cells':
         const: 0
 
+      max-brightness:
+        description:
+          Normally, the maximum brightness is determined by the hardware, and
+          this property is not required. This property is used to set a software
+          limit. It could happen that an LED is made so bright that it gets
+          damaged or causes damage due to restrictions in a specific system,
+          such as mounting conditions.
+        $ref: /schemas/types.yaml#definitions/uint32
+
     patternProperties:
       "^led@[0-9a-f]+$":
         type: object
-- 
2.30.2

Re: [PATCH v1 2/2] dt: bindings: lp50xx: Add max-brightness as in leds-pwm

[Rob Herring]

On Thu, Jun 29, 2023 at 03:47:22PM +0200, Astrid Rost wrote:
> Add max-brightness in order to reduce the current on the connected LEDs.
> Normally, the maximum brightness is determined by the hardware, and this
> property is not required. This property is used to set a software limit.
> It could happen that an LED is made so bright that it gets damaged or
> causes damage due to restrictions in a specific system, such as mounting
> conditions.
> 
> Signed-off-by: Astrid Rost <astrid.rost@axis.com>
> ---
>  Documentation/devicetree/bindings/leds/leds-lp50xx.yaml | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
> index 402c25424525..6a1425969cb4 100644
> --- a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
> +++ b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
> @@ -73,6 +73,15 @@ patternProperties:
>        '#size-cells':
>          const: 0
>  
> +      max-brightness:
> +        description:
> +          Normally, the maximum brightness is determined by the hardware, and
> +          this property is not required. This property is used to set a software
> +          limit. It could happen that an LED is made so bright that it gets
> +          damaged or causes damage due to restrictions in a specific system,
> +          such as mounting conditions.
> +        $ref: /schemas/types.yaml#definitions/uint32

We already have led-max-microamp. If this h/w works by controlling the 
current, then that is what you should use. "max-brightness" makes more 
sense for PWM based control.

If you do end up keeping this, it belongs in the 'led' nodes not the 
controller as brightness is a property of the LED, not the LED 
driver(controller). And it should be in common schema rather than 
defining the type yet again.

Rob

Re: [PATCH v1 2/2] dt: bindings: lp50xx: Add max-brightness as in leds-pwm

[Astrid Rost]

On 6/29/23 19:59, Rob Herring wrote:
> On Thu, Jun 29, 2023 at 03:47:22PM +0200, Astrid Rost wrote:
>> Add max-brightness in order to reduce the current on the connected LEDs.
>> Normally, the maximum brightness is determined by the hardware, and this
>> property is not required. This property is used to set a software limit.
>> It could happen that an LED is made so bright that it gets damaged or
>> causes damage due to restrictions in a specific system, such as mounting
>> conditions.
>>
>> Signed-off-by: Astrid Rost <astrid.rost@axis.com>
>> ---
>>   Documentation/devicetree/bindings/leds/leds-lp50xx.yaml | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
>> index 402c25424525..6a1425969cb4 100644
>> --- a/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
>> +++ b/Documentation/devicetree/bindings/leds/leds-lp50xx.yaml
>> @@ -73,6 +73,15 @@ patternProperties:
>>         '#size-cells':
>>           const: 0
>>   
>> +      max-brightness:
>> +        description:
>> +          Normally, the maximum brightness is determined by the hardware, and
>> +          this property is not required. This property is used to set a software
>> +          limit. It could happen that an LED is made so bright that it gets
>> +          damaged or causes damage due to restrictions in a specific system,
>> +          such as mounting conditions.
>> +        $ref: /schemas/types.yaml#definitions/uint32
> 
> We already have led-max-microamp. If this h/w works by controlling the
> current, then that is what you should use. "max-brightness" makes more
> sense for PWM based control.

Yes, I looked into this. This LED controller TI-LP5024 has only one flag 
as max current option, which allows to set between 35 mA and the default 
value 25.5 mA. This is not enough for our application.

> 
> If you do end up keeping this, it belongs in the 'led' nodes not the
> controller as brightness is a property of the LED, not the LED
> driver(controller). And it should be in common schema rather than
> defining the type yet again.

I will look moving it into the the LED generic bindings as Andy 
suggested. max_brightness is already there and handled correctly. It is 
just a matter of reading it from the devicetree.

> 
> Rob