Hi, On 3/23/23 12:36, Pavel Machek wrote: > On Thu 2023-03-23 12:29:29, Hans de Goede wrote: >> Hi, >> >> On 3/23/23 12:26, Pavel Machek wrote: >>> On Thu 2023-03-23 12:24:05, Hans de Goede wrote: >>>> Hi Pavel, >>>> >>>> On 3/23/23 12:15, Pavel Machek wrote: >>>>> Hi! >>>>> >>>>>> There are two LED controllers, LEDA indicator LED and LEDB flash LED for >>>>>> tps68470. LEDA can be enabled by setting TPS68470_ILEDCTL_ENA. Moreover, >>>>>> tps68470 provides four levels of power status for LEDB. If the >>>>>> properties called "ti,ledb-current" can be found, the current will be >>>>>> set according to the property values. These two LEDs can be controlled >>>>>> through the LED class of sysfs (tps68470-leda and tps68470-ledb). >>>>> >>>>> If the LED can have four different currents, should it have 4 >>>>> brightness levels? >>>> >>>> No this was already discussed with an earlier version. This is in >>>> indicator LED output. The current setting is a one time boot configure >>>> thing after which the indicator LED is either on or off. >>> >>> Current levels are exponential in that driver. That will result in >>> rather nice four level. Surely LED does not care if you set it during >>> boot or later? >> >> Well for one there is no guarantee the LED can continuously handle >> the maximum configurable LED current and as you rightly point out >> elsewhere in the thread we don't want to be blowing up hw. > > hw can support 16mA -> you expose 0, 2mA, 4mA, 8mA, 16mA levels. > > hw can support 4mA -> you expose 0, 2mA, 4mA. This is just not how this hw is intended to be used. If you look at the registers there are on/off bits for LEDA and LEDB in a single register and then there is a current-limit only for LEDB, where as LEDA has a fixed current. So clearly the intention here is on/off use with a fixed current and dimming something like a privacy-LED on a laptop camera makes no sense and will never be used since we disable userspace access of the LED when it is used as a privacy-LED. Regards, Hans