lkml.org 
[lkml]   [2018]   [Oct]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v14 1/2] leds: core: Introduce LED pattern trigger
Hi Jacek,

On 10 October 2018 at 02:37, Jacek Anaszewski
<jacek.anaszewski@gmail.com> wrote:
> Hi Baolin,
>
> On 10/09/2018 02:01 PM, Baolin Wang wrote:
>> Hi Jacek and Pavel,
>>
>> On 5 October 2018 at 04:00, Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
>>> Hi Baolin,
>>>
>>> On 10/03/2018 03:21 AM, Baolin Wang wrote:
>>>> Hi Jacek,
>>>>
>>>> On 3 October 2018 at 04:25, Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
>>>>> Hi Baolin,
>>>>>
>>>>> Thank you for the v14. We'll probably need v15, though :-)
>>>>>
>>>>> I added the comments in the code below.
>>>>>
>>>>> On 10/02/2018 05:43 PM, Baolin Wang wrote:
>>>>>> This patch adds one new led trigger that LED device can configure
>>>>>> the software or hardware pattern and trigger it.
>>>>>>
>>>>>> Consumers can write 'pattern' file to enable the software pattern
>>>>>> which alters the brightness for the specified duration with one
>>>>>> software timer.
>>>>>>
>>>>>> Moreover consumers can write 'hw_pattern' file to enable the hardware
>>>>>> pattern for some LED controllers which can autonomously control
>>>>>> brightness over time, according to some preprogrammed hardware
>>>>>> patterns.
>>>>>>
>>>>>> Signed-off-by: Raphael Teysseyre <rteysseyre@gmail.com>
>>>>>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>>>>>> ---
>>>>>> Changes from v13:
>>>>>> - Add duration validation for gradual dimming.
>>>>>> - Coding style optimization.
>>>>>>
>>>>>> Changes from v12:
>>>>>> - Add gradual dimming support for software pattern.
>>>>>>
>>>>>> Changes from v11:
>>>>>> - Change -1 means repeat indefinitely.
>>>>>>
>>>>>> Changes from v10:
>>>>>> - Change 'int' to 'u32' for delta_t field.
>>>>>>
>>>>>> Changes from v9:
>>>>>> - None.
>>>>>>
>>>>>> Changes from v8:
>>>>>> - None.
>>>>>>
>>>>>> Changes from v7:
>>>>>> - Move the SC27XX hardware patterns description into its own ABI file.
>>>>>>
>>>>>> Changes from v6:
>>>>>> - Improve commit message.
>>>>>> - Optimize the description of the hw_pattern file.
>>>>>> - Simplify some logics.
>>>>>>
>>>>>> Changes from v5:
>>>>>> - Add one 'hw_pattern' file for hardware patterns.
>>>>>>
>>>>>> Changes from v4:
>>>>>> - Change the repeat file to return the originally written number.
>>>>>> - Improve comments.
>>>>>> - Fix some build warnings.
>>>>>>
>>>>>> Changes from v3:
>>>>>> - Reset pattern number to 0 if user provides incorrect pattern string.
>>>>>> - Support one pattern.
>>>>>>
>>>>>> Changes from v2:
>>>>>> - Remove hardware_pattern boolen.
>>>>>> - Chnage the pattern string format.
>>>>>>
>>>>>> Changes from v1:
>>>>>> - Use ATTRIBUTE_GROUPS() to define attributes.
>>>>>> - Introduce hardware_pattern flag to determine if software pattern
>>>>>> or hardware pattern.
>>>>>> - Re-implement pattern_trig_store_pattern() function.
>>>>>> - Remove pattern_get() interface.
>>>>>> - Improve comments.
>>>>>> - Other small optimization.
>>>>>> ---
>>>>>> .../ABI/testing/sysfs-class-led-trigger-pattern | 76 ++++
>>>>>> drivers/leds/trigger/Kconfig | 7 +
>>>>>> drivers/leds/trigger/Makefile | 1 +
>>>>>> drivers/leds/trigger/ledtrig-pattern.c | 431 +++++++++++++++++++++
>>>>>> include/linux/leds.h | 15 +
>>>>>> 5 files changed, 530 insertions(+)
>>>>>> create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-pattern
>>>>>> create mode 100644 drivers/leds/trigger/ledtrig-pattern.c
>>>>>>
>>>>>> diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-pattern b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern
>>>>>> new file mode 100644
>>>>>> index 0000000..22d7af7
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern
>>>>>> @@ -0,0 +1,76 @@
>>>>>> +What: /sys/class/leds/<led>/pattern
>>>>>> +Date: September 2018
>>>>>> +KernelVersion: 4.20
>>>>>> +Description:
>>>>>> + Specify a software pattern for the LED, that supports altering
>>>>>> + the brightness for the specified duration with one software
>>>>>> + timer. It can do gradual dimming and constant brightness.
>>>>>> +
>>>>>> + The pattern is given by a series of tuples, of brightness and
>>>>>> + duration (ms). The LED is expected to traverse the series and
>>>>>> + each brightness value for the specified duration. Duration of
>>>>>> + 0 means brightness should immediately change to new value.
>>>>>> +
>>>>>> + 1. When doing gradual dimming, the led brightness will be updated
>>>>>> + every 50 milliseconds, so the duration of each step should not
>>>>>> + less than 50 milliseconds.
>>>>>
>>>>> I'd like to avoid this constraint. Lowest supported delta_t should be 1.
>>>>>
>>>>> We should only prevent entering dimming mode if current delta_t
>>>>> is lower than UPDATE_INTERVAL.
>>>>
>>>> I do not think so. If the pattern format is used for dimming and the
>>>> delta_t is lower than UPDATE_INTERVAL, we should return errors. Since
>>>> in this case, we can not change to constant mode.
>>>
>>> I'll play with the code at weekend and will let you know my findings.
>>
>> Thanks Jacek. Do you have any more suggestion?
>
> Sorry to say, but I had really busy weekend and failed to carve out
> time for looking into that. It's queued up with the highest priority
> on my todo list of things needing more than five minutes of attention.

Ah, okay, no hurry :)

>> BTW, we have not get a consensus about adding one 'dimming_interval'
>> file to set the dimming interval instead of fixed 50 ms.
>> Pavel, I want to hear your suggestion before I submit new patch set? Thanks.
>
> I think that this feature belongs to the "nice to have" category,
> and lack of it will not prevent us from merging the patch set.

OK. Thanks.

--
Baolin Wang
Best Regards

\
 
 \ /
  Last update: 2018-10-10 04:16    [W:0.055 / U:0.492 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site