Messages in this thread | | | Subject | Re: [PATCH -next 2/2] leds: add /sys/class/leds/<led>/current-trigger | From | Dan Murphy <> | Date | Wed, 2 Oct 2019 12:57:01 -0500 |
| |
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@linuxfoundation.org> >>> Cc: "Rafael J. Wysocki" <rafael@kernel.org> >>> Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com> >>> Cc: Pavel Machek <pavel@ucw.cz> >>> Cc: Dan Murphy <dmurphy@ti.com> >>> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> >>> --- >>> 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@vger.kernel.org >>> +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 >> >>
| |