[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMz4kuKJ73mHzYotwmx08h1sJJiKAZNCspo0_v7L-2Ec+5rEQg@mail.gmail.com>
Date: Tue, 9 Oct 2018 20:01:19 +0800
From: Baolin Wang <baolin.wang@...aro.org>
To: Jacek Anaszewski <jacek.anaszewski@...il.com>
Cc: Pavel Machek <pavel@....cz>, rteysseyre@...il.com,
Bjorn Andersson <bjorn.andersson@...aro.org>,
Mark Brown <broonie@...nel.org>,
Linux LED Subsystem <linux-leds@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v14 1/2] leds: core: Introduce LED pattern trigger
Hi Jacek and Pavel,
On 5 October 2018 at 04:00, Jacek Anaszewski <jacek.anaszewski@...il.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@...il.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@...il.com>
>>>> Signed-off-by: Baolin Wang <baolin.wang@...aro.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?
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.
--
Baolin Wang
Best Regards
Powered by blists - more mailing lists