Re: [PATCH v5] DT: leds: Improve description of flash LEDs related properties

[Jacek Anaszewski <j.anaszewski@samsung.com>]

On 04/08/2015 05:15 PM, Sylwester Nawrocki wrote:
> On 08/04/15 16:31, Jacek Anaszewski wrote:
>> Properties defining maximum values for LED currents and timeout should
>> be mandatory to avoid the risk of hardware damage. This patch fixes
>> the issue.
>>
>> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>
>> ---
>>   Documentation/devicetree/bindings/leds/common.txt |   19 +++++++++++--------
>>   1 file changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
>> index 747c538..e478ac6 100644
>> --- a/Documentation/devicetree/bindings/leds/common.txt
>> +++ b/Documentation/devicetree/bindings/leds/common.txt
>> @@ -10,6 +10,17 @@ can influence the way of the LED device initialization, the LED components
>>   have to be tightly coupled with the LED device binding. They are represented
>>   by child nodes of the parent LED device binding.
>>
>> +Required properties for child nodes:
>
> These properties are mandatory only for LEDs with Flash/Torch capabilities,
> aren't they?

flash-max-microamp and flash-timeout-us properties description mention
that they refer to flash LEDs. Perhaps this should be made indeed more
explicit.

 > Requiring those properties for all led nodes would make all
> current dtses not compliant with the DT binding specification AFAICT.

I was also worrying about making led-max-microamp required, but others
didn't share my objections. I think that we have to reexamine this.

Please refer to the discussion under [PATCH v4]. The role of this
led-max-microamp property would be preventing hardware damage.


> How about:
>
> "Required properties for child nodes for LEDs with Flash/Torch capabilities:" ?
>
>> +- led-max-microamp : Maximum LED supply current in microamperes
>> +                     (torch LED for flash devices). Controllers that have no
>> +                     configurable current can omit this property.
>> +- flash-max-microamp : Maximum flash LED supply current in microamperes.
>> +- flash-timeout-us : Timeout in microseconds after which the flash
>> +                     LED is turned off.
>> +
>
>> +Above properties determine a LED driver IC settings required for safe
>> +operation. They should be also used as the initial settings for the IC.
>
> Shouldn't "Controllers that have no configurable current can omit this
> property" refer to both led-max-microamp and flash-max-microamp?
 >
> I would drop the "Above...for the IC." paragraph and instead add something like:
>
> "For controllers that have no configurable current the led-max-microamp,
> flash-max-microamp properties respectively can be omitted. For controllers that
> have no configurable timeout the flash-timeout-us property can be omitted."

Are we going to avoid mentioning about their role in preventing
hardware damage (former "for safe operation" sequence)?

-- 
Best Regards,
Jacek Anaszewski