Re: [PATCH -next 2/2] leds: add /sys/class/leds/<led>/current-trigger

From: Dan Murphy
Date: Wed Oct 02 2019 - 13:56:25 EST


Jacek

On 10/2/19 12:46 PM, Jacek Anaszewski wrote:
Dan,

On 10/2/19 5:47 PM, Dan Murphy wrote:
Akinobu

On 10/2/19 10:13 AM, Akinobu Mita wrote:
Reading /sys/class/leds/<led>/trigger returns all available LED triggers.
However, this violates the "one value per file" rule of sysfs.

This provides /sys/class/leds/<led>/current-trigger which is almost
identical to /sys/class/leds/<led>/trigger. The only difference is that
'current-trigger' only shows the current trigger name.

This new file follows the "one value per file" rule of sysfs.
We can find all available LED triggers by listing the
/sys/devices/virtual/led-trigger/ directory.

Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Cc: "Rafael J. Wysocki" <rafael@xxxxxxxxxx>
Cc: Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx>
Cc: Pavel Machek <pavel@xxxxxx>
Cc: Dan Murphy <dmurphy@xxxxxx>
Signed-off-by: Akinobu Mita <akinobu.mita@xxxxxxxxx>
---
 Documentation/ABI/testing/sysfs-class-led | 13 +++++++++++
 drivers/leds/led-class.c | 10 ++++++++
 drivers/leds/led-triggers.c | 38
+++++++++++++++++++++++++++----
 drivers/leds/leds.h | 5 ++++
 4 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-led
b/Documentation/ABI/testing/sysfs-class-led
index 5f67f7a..fdfed3f 100644
--- a/Documentation/ABI/testing/sysfs-class-led
+++ b/Documentation/ABI/testing/sysfs-class-led
@@ -61,3 +61,16 @@ Description:
ÂÂÂÂÂÂÂÂÂ gpio and backlight triggers. In case of the backlight trigger,
ÂÂÂÂÂÂÂÂÂ it is useful when driving a LED which is intended to indicate
ÂÂÂÂÂÂÂÂÂ a device in a standby like state.
+
+What:ÂÂÂÂÂÂÂ /sys/class/leds/<led>/current-trigger
+Date:ÂÂÂÂÂÂÂ September 2019
+KernelVersion:ÂÂÂ 5.5
+Contact:ÂÂÂ linux-leds@xxxxxxxxxxxxxxx
+Description:
+ÂÂÂÂÂÂÂ Set the trigger for this LED. A trigger is a kernel based source
+ÂÂÂÂÂÂÂ of LED events.
+ÂÂÂÂÂÂÂ Writing the trigger name to this file will change the current
+ÂÂÂÂÂÂÂ trigger. Trigger specific parameters can appear in
+ÂÂÂÂÂÂÂ /sys/class/leds/<led> once a given trigger is selected. For
+ÂÂÂÂÂÂÂ their documentation see sysfs-class-led-trigger-*.
+ÂÂÂÂÂÂÂ Reading this file will return the current LED trigger name.
Why do we need this new file can't we just update the current trigger
file implementation?
We can't change existing ABI. It doesn't matter if it is documented
or not - it's in place for very long time and you can't guarantee there
are no users relying on triggers file show format.

So if it has been in place for a very long time why do we need another ABI that does sorta the same thing?

This seems to be a bit confusing and extra.

Maybe this ABI should be RO where a user can read the current-trigger as a single value per file but writing the trigger still

is done through the old ABI.

Dan



I don't see any documentation that states that the read of the trigger
file will print a list of known triggers.

And writing to the trigger file still works so I would think the _show
just needs to be fixed.

Besides this patch does not fix the issue in the commit message that the
trigger file still violates the one value per file rule.

Dan