On 6 September 2018 at 09:43, Baolin Wang <baolin.wang@xxxxxxxxxx> wrote: > Hi Jacek, > > On 6 September 2018 at 03:14, Jacek Anaszewski > <jacek.anaszewski@xxxxxxxxx> wrote: >> Hi Baolin, >> >> Thanks for the v9. >> >> On 09/05/2018 09:20 AM, Baolin Wang wrote: >>> This patch implements the 'pattern_set'and 'pattern_clear' >>> interfaces to support SC27XX LED breathing mode. >>> >>> Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxx> >>> --- >>> Changes from v8: >>> - Optimize the ABI documentation file. >>> >>> Changes from v7: >>> - Add its own ABI documentation file. >>> >>> Changes from v6: >>> - None. >>> >>> Changes from v5: >>> - None. >>> >>> Changes from v4: >>> - None. >>> >>> Changes from v3: >>> - None. >>> >>> Changes from v2: >>> - None. >>> >>> Changes from v1: >>> - Remove pattern_get interface. >>> --- >>> .../ABI/testing/sysfs-class-led-driver-sc27xx | 20 +++++ >>> drivers/leds/leds-sc27xx-bltc.c | 93 ++++++++++++++++++++ >>> 2 files changed, 113 insertions(+) >>> create mode 100644 Documentation/ABI/testing/sysfs-class-led-driver-sc27xx >>> >>> diff --git a/Documentation/ABI/testing/sysfs-class-led-driver-sc27xx b/Documentation/ABI/testing/sysfs-class-led-driver-sc27xx >>> new file mode 100644 >>> index 0000000..391ca6e >>> --- /dev/null >>> +++ b/Documentation/ABI/testing/sysfs-class-led-driver-sc27xx >>> @@ -0,0 +1,20 @@ >>> +What: /sys/class/leds/<led>/hw_pattern >>> +Date: September 2018 >>> +KernelVersion: 4.20 >>> +Description: >>> + Specify a hardware pattern for the SC27XX LED. For the SC27XX >>> + LED controller, it only supports 4 stages to make a single >>> + hardware pattern, which is used to configure the rise time, >>> + high time, fall time and low time for the breathing mode. >>> + >>> + For the breathing mode, the SC27XX LED only expects one brightness >>> + for the high stage. To be compatible with the hardware pattern >>> + format, we should set brightness as 0 for rise stage, fall >>> + stage and low stage. >>> + >>> + Min stage duration: 1 >>> + Max stage duration: 255 >>> + Stage duration step: 125 ms >> >> It seems that min and max stage duration are given in device >> specific levels in contrary to the step which is given in ms. >> Please keep it consistent. If I'm getting it right then duration >> constraints should be given as follows: >> >> Min stage duration: 125 ms >> Max stage duration: 31875 ms > > This is not good for users. Since what we set into LED registers is > step counters, and each step is 125 ms. Which means it only support > 125ms, 250ms, 375ms, 500ms .... 31875ms. > > If user set stage duration to 200ms, what we actually set into LED > registers is 1 step counter (200ms / 125ms = 1), which means the > actual duration is 125ms. That will confuse the user. So I still like > to force user to set the step counter which can show the real > duration. Ah, sorry for misunderstanding, to keep consistent with the documentation, I will change the duration unit to ms and add more documentation for the duration step in next version. Thanks. -- Baolin Wang Best Regards