Re: [PATCH 6/8] leds: tps68470: Support the WLED driver

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 3/23/23 17:25, Dan Scally wrote:
> Hi Pavel, thanks for taking a look
> 
> On 23/03/2023 11:22, Pavel Machek wrote:
>> On Wed 2023-03-22 16:09:24, Daniel Scally wrote:
>>> The TPS68470 PMIC provides a third LED driver in addition to the two
>>> indicator LEDs. Add support for the WLED. To ensure the LED is active
>>> for as long as the kernel instructs it to be we need to re-trigger it
>>> periodically to avoid the IC's internal timeouts.
>> Wow. No!
>>
>> If hardware does not wart you to burn the LED, it is not okay to just
>> work around that. These are not designed for continuous operation.
>>
>>> diff --git a/drivers/leds/leds-tps68470.c b/drivers/leds/leds-tps68470.c
>>> index 44df175d25de..abcd3494b1a8 100644
>> Fun sha1 hash ;-).
> 
> 
> heh yeh
> 
>>
>>> @@ -52,11 +61,33 @@ enum ctrlb_current {
>>>       CTRLB_16MA    = 3,
>>>   };
>>>   +/*
>>> + * The WLED can operate in different modes, including a Flash and Torch mode. In
>>> + * each mode there's a timeout which ranges from a matter of milliseconds to up
>>> + * to 13 seconds. We don't want that timeout to apply though because the LED
>>> + * should be lit until we say that it should no longer be lit, re-trigger the
>>> + * LED periodically to keep it alive.
>>> + */
>> We don't want the LED to overheat. That takes precedence.
>>
>> Find out what are the maximum limits for on time at various current
>> levels. LED framework should be used for torch mode, with current set
>> such that unlimited operation is safe. V4L2 should be used for flash
>> mode.
> 
> 
> I did it this way because this is how the IC operates on my device whilst it's booted to Windows...but I suppose given they don't expose the LED outside of their Hello auth thing they can guarantee it's not being lit for too long - I confess that hadn't occurred to me. Anyway; I'll update this to re-trigger if the IC is in torch mode within the timeout (which the datasheet explicitly says you can do in torch mode; the current is much more heavily limited in that mode) and in the flash mode to update the brightness setting to 0 once the timeout expires so it reflects the actual state of the LED. Does that sound ok?

That sounds reasonable to me.

Regards,

Hans