Messages in this thread | | | Subject | Re: [PATCH v2 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver | From | Jacek Anaszewski <> | Date | Wed, 9 May 2018 21:40:35 +0200 |
| |
Hi,
On 05/09/2018 04:25 PM, Pavel Machek wrote: > On Tue 2018-05-08 13:39:45, Baolin Wang wrote: >> From: Xiaotong Lu <xiaotong.lu@spreadtrum.com> >> >> This patch adds Spreadtrum SC27xx PMIC series breathing light controller >> driver, which can support 3 LEDs. Each LED can work at normal PWM mode >> and breathing mode. >> >> 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..22166fb >> --- /dev/null >> +++ b/Documentation/ABI/testing/sysfs-class-led-driver-sc27xx >> @@ -0,0 +1,19 @@ >> +What: /sys/class/leds/<led>/rise_time >> +What: /sys/class/leds/<led>/high_time >> +What: /sys/class/leds/<led>/fall_time >> +What: /sys/class/leds/<led>/low_time >> +Date: May 2018 >> +KernelVersion: 4.18 >> +Contact: Xiaotong Lu <xiaotong.lu@spreadtrum.com> >> +Description: >> + Set the pattern generator rise, high, fall and low >> + times (0..63). It's unit is 0.125s, it should be > 0. >> + >> + 1 - 125 ms >> + 2 - 250 ms >> + 3 - 375 ms >> + ... >> + ... >> + ... >> + 62 - 7.75 s >> + 63 - 7.875 s > > How does this interact with triggers? With manually setting > brightness? Are the pattern generators independend for the LEDs? > > Can you generate white breathing pattern? If so, how? > > How do you select between normal and breathing modes? > > I'd specify times in miliseconds or something, this is way too > hardware specific.
Agreed.
> Now... functionality like this is common between many LED > controllers. N900 could do this kind of "breathing", too, and it also > supports other patterns. > > I believe we need interface common between different LED controllers. > > And I guess it would be easiest if you dropped this part from initial > merge.
I disagree here. We already had the same discussion at the occasion of the patch [0] and it turned out to be a dead-end [1]. Now we have neither the driver nor the generic pattern interface.
We also already have some older LED class drivers that implement custom pattern interfaces (e.g. drivers/leds/leds-lm3533.c) and the same approach can be applied in this case.
Regarding interaction with triggers - we could disable pattern on setting any trigger and return -EBUSY from the pattern interface if led_cdev->trigger is not NULL.
[0] https://lkml.org/lkml/2017/3/23/33 [1] https://lkml.org/lkml/2017/11/15/27
-- Best regards, Jacek Anaszewski
| |