Re: [PATCH v3 1/2] leds: core: Introduce generic pattern interface

From: Jacek Anaszewski
Date: Mon Jul 16 2018 - 16:29:19 EST

Hi David,

On 07/16/2018 03:00 AM, David Lechner wrote:
On 07/15/2018 07:22 AM, Jacek Anaszewski wrote:
On 07/15/2018 12:39 AM, Pavel Machek wrote:
On Sun 2018-07-15 00:29:25, Pavel Machek wrote:
On Sun 2018-07-15 00:02:57, Jacek Anaszewski wrote:
Hi Pavel,

On 07/14/2018 11:20 PM, Pavel Machek wrote:
Hi!

It also drew my attention to the issue of desired pattern sysfs
interface semantics on uninitialized pattern. In your implementation
user seems to be unable to determine if the pattern is activated
or not. We should define the semantics for this use case and
describe it in the documentation. Possibly pattern could
return alone new line character then.

Let me take a step back: we have triggers.. like LED blinking.

How is that going to interact with patterns? We probably want the
patterns to be ignored in that case...?

Which suggest to me that we should treat patterns as a trigger. I
believe we do something similar with blinking already.

Then it is easy to determine if pattern is active, and pattern
vs. trigger issue is solved automatically.

I'm all for it. I proposed this approach during the previous
discussions related to possible pattern interface implementations,
but you seemed not to be so enthusiastic in [0].

[0] https://lkml.org/lkml/2017/4/7/350

Hmm. Reading my own email now, I can't decipher it.

I believe I meant "changing patterns from kernel in response to events
is probably overkill"... or something like that.

Anyway -- to clean up the confusion -- I'd like to see

echo pattern > trigger
echo "1 2 3 4 5 6 7 8" > somewhere

s/somewhere/pattern/

pattern trigger should create "pattern" file similarly how ledtrig-timer
creates delay_{on|off} files.


I don't think this is the best way. For example, if you want more than one
LED to have the same pattern, then the patterns will not be synchronized
between the LEDs. The same things happens now with many of the existing
triggers. For example, if I have two LEDs side-by-side using the heartbeat
trigger, they may blink at the same time or they may not, which is not
very nice. I think we can make something better.

It is virtually impossible to enforce synchronous blinking for the
LEDs driven by different hardware due to:

- different hardware means via which brightness is set (MMIO, I2C, SPI,
PWM and other pulsed fashion based protocols),
- the need for deferring brightness setting to a workqueue task to
allow for setting LED brightness from atomic context,
- contention on locks

For the LEDs driven by the same chip it would make more sense
to allow for synchronization, but it can be achieved on driver
level, with help of some subsystem level interface to indicate
which LEDs should be synchronized.

However, when we start to pretend that we can synchronize the
devices, we must answer how accurate we can be. The accuracy
will decrease as blink frequency rises. We'd need to define
reliability limit.

For different devices, this limit will be different, and it will also
depend on the CPU speed.

We've had few attempts of approaching the subject of synchronized
blinking but none of them proved to be good enough to be merged.

Frankly speaking I doubt it is good task for the system like Linux.

Perhaps a way to do this would be to use configfs to create a pattern
trigger that can be shared by multiple LEDs. Like this:

    mkdir /sys/kernel/config/leds/triggers/my-nice-pattern
    echo "1 2 3 4" > /sys/kernel/config/leds/triggers/my-nice-pattern/pattern
    echo my-nice-pattern > /sys/class/leds/led0/trigger
    echo my-nice-pattern > /sys/class/leds/led1/trigger


Please CC me on any future revisions of this series. I would like to test it.


--
Best regards,
Jacek Anaszewski