Re: [PATCH] leds: gpio: support multi-level brightness

[Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx>]

On 10/9/19 4:43 PM, Akinobu Mita wrote:
> 2019年10月9日(水) 5:53 Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx>:
>>
>> On 10/6/19 4:11 PM, Akinobu Mita wrote:
>>> 2019年10月6日(日) 4:17 Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx>:
>>>>
>>>> On 10/5/19 3:20 PM, Akinobu Mita wrote:
>>>>> 2019年10月5日(土) 6:17 Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx>:
>>>>>>
>>>>>> Hi Akinobu,
>>>>>>
>>>>>> Why do you think this change is needed? Does it solve
>>>>>> some use case for you?
>>>>>
>>>>> It can be useful when using with an LED trigger that could set the
>>>>> brightness values other than LED_FULL or LED_OFF.
>>>>>
>>>>> The LED CPU trigger for all CPUs (not per CPU) sets the brightness value
>>>>> depending on the number of active CPUs.  We can define the multi brightness
>>>>> level gpio LED with fewer number of GPIO LEDs than the total number of
>>>>> CPUs, and the LEDs can be viewed as a level meter.
>>>>
>>>> Can't you achieve exactly the same effect by creating separate LED class
>>>> device for each GPIO LED and registering each of them for separate cpuN
>>>> trigger?
>>>
>>> If there are GPIO LEDs as many as the total number of CPUs, we can.
>>> However, if there are only two GPIO LEDs and six CPUs, we can only know
>>> the CPU activity for two CPUs out of six CPUs with cpuN trigger.
>>> So it's different from using cpu (all) trigger with multi level (2-level)
>>> brightness GPIO LED.
>>
>> OK, that's a reasonable argument. However, this is clearly
>> trigger-specific functionality and we should not delegate this
>> logic down to the driver.
>>
>> What you propose should be a responsibility of a trigger that would
>> allow registering multiple LEDs for its disposal. This would have to
>> be different from existing LED Trigger mechanism, that blindly
>> applies trigger event to all LEDs that have registered for it.
>>
>> Such a trigger would have to be a separate LED (pattern?) class device.
>> It would need to be told how many LEDs it is going to manage
>> and create files for filling the LED names. This design could be also
>> used for defining patterns spanning on multiple LEDs. Just a rough idea.
>> We can dwell on it if it catches.
> 
> What do you think about introducing a new led driver and describing a
> level meter in the following DT node.
> 
> led-level-meter {
>         compatible = "led-level-meter";
>         leds = <&led0>, <&led1>, <&led2>, <&led3>;
> };
> 
> Setting the brightness of led-level-meter to LED_HALF turns on led0 and
> led1 with max brightness and turns off led2 and led3.
> 
> This is inspired by led-backlight driver patchset and Bjorn Andersson's
> reply in this thread.

Looks interesting, I like the idea.

-- 
Best regards,
Jacek Anaszewski