Similarily to commit 325253a6b2de ("backlight: Allow drivers to update
the core, and generate events on changes"), inform userspace about
brightness changes and allow drivers to request updates of the
brightness value.
Signed-off-by: Gabriele Mazzotta <[email protected]>
---
This is exactly like 325253a6b2de, but for the led subsystem. The only thing
I'm not entirely sure of is that uevents are sent even if the brightness
doesn't change, but this is how it's done in 325253a6b2de.
drivers/leds/led-class.c | 42 ++++++++++++++++++++++++++++++++++++++++++
include/linux/leds.h | 7 +++++++
2 files changed, 49 insertions(+)
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 326ee6e925a2..248ee0fca683 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -25,6 +25,27 @@
static struct class *leds_class;
+static void brightness_generate_event(struct led_classdev *led_cdev,
+ enum led_brightness_update_reason reason)
+{
+ char *envp[2];
+
+ switch (reason) {
+ case LED_BRIGHTNESS_UPDATE_SYSFS:
+ envp[0] = "SOURCE=sysfs";
+ break;
+ case LED_BRIGHTNESS_UPDATE_HOTKEY:
+ envp[0] = "SOURCE=hotkey";
+ break;
+ default:
+ envp[0] = "SOURCE=unknown";
+ break;
+ }
+ envp[1] = NULL;
+ kobject_uevent_env(&led_cdev->dev->kobj, KOBJ_CHANGE, envp);
+ sysfs_notify(&led_cdev->dev->kobj, NULL, "brightness");
+}
+
static ssize_t brightness_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -58,6 +79,8 @@ static ssize_t brightness_store(struct device *dev,
led_trigger_remove(led_cdev);
led_set_brightness(led_cdev, state);
+ brightness_generate_event(led_cdev, LED_BRIGHTNESS_UPDATE_SYSFS);
+
ret = size;
unlock:
mutex_unlock(&led_cdev->led_access);
@@ -129,6 +152,25 @@ void led_classdev_resume(struct led_classdev *led_cdev)
}
EXPORT_SYMBOL_GPL(led_classdev_resume);
+/**
+ * led_brightness_force_update - tell the backlight subsystem that hardware
+ * state has changed
+ * @led_cdev: the led device to update
+ *
+ * Updates the internal state of the backlight in response to a hardware event,
+ * and generate a uevent to notify userspace
+ */
+void led_brightness_force_update(struct led_classdev *led_cdev,
+ enum led_brightness_update_reason reason)
+{
+ mutex_lock(&led_cdev->led_access);
+ if (led_cdev->brightness_get)
+ led_cdev->brightness = led_cdev->brightness_get(led_cdev);
+ mutex_unlock(&led_cdev->led_access);
+ brightness_generate_event(led_cdev, reason);
+}
+EXPORT_SYMBOL(led_brightness_force_update);
+
#ifdef CONFIG_PM_SLEEP
static int led_suspend(struct device *dev)
{
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 569cb531094c..8d3faaf14ff1 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -31,6 +31,11 @@ enum led_brightness {
LED_FULL = 255,
};
+enum led_brightness_update_reason {
+ LED_BRIGHTNESS_UPDATE_HOTKEY,
+ LED_BRIGHTNESS_UPDATE_SYSFS,
+};
+
struct led_classdev {
const char *name;
enum led_brightness brightness;
@@ -123,6 +128,8 @@ extern void devm_led_classdev_unregister(struct device *parent,
struct led_classdev *led_cdev);
extern void led_classdev_suspend(struct led_classdev *led_cdev);
extern void led_classdev_resume(struct led_classdev *led_cdev);
+extern void led_brightness_force_update(struct led_classdev *led_cdev,
+ enum led_brightness_update_reason reason);
/**
* led_blink_set - set blinking with software fallback
--
2.11.0
Hi!
> Similarily to commit 325253a6b2de ("backlight: Allow drivers to update
> the core, and generate events on changes"), inform userspace about
> brightness changes and allow drivers to request updates of the
> brightness value.
First... we had similar patch in tree, and it caused problems, we are
now trying to figure out how to do it properly.
LED can be updated many times per second, uevent is probably _not_
good mechanism to achieve that.
Generating uevent for /sys changes does not make much sense, right?
> +extern void led_brightness_force_update(struct led_classdev *led_cdev,
> + enum led_brightness_update_reason reason);
I see this may make some sense, but there are no uses for this in this
patch.
My preffered solution would be ... for hardware that changes led
brightness itself, introduce a "trigger", so that userspace knows this
led is special, and then provide poll()able /sys fs file interested
parties can read.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On 27/12/2016 21:07, Pavel Machek wrote:
> Hi!
>
>> Similarily to commit 325253a6b2de ("backlight: Allow drivers to update
>> the core, and generate events on changes"), inform userspace about
>> brightness changes and allow drivers to request updates of the
>> brightness value.
>
> First... we had similar patch in tree, and it caused problems, we are
> now trying to figure out how to do it properly.
Oh, I didn't know that, sorry.
> LED can be updated many times per second, uevent is probably _not_
> good mechanism to achieve that.
Ummm, right, I didn't think of that.
> Generating uevent for /sys changes does not make much sense, right?
I planned to use this patch mostly for keyboard backlights, for
which some DEs provide a UI similar to the one for screen backlights.
Having uevents also for /sys changes means having the UI always in
sync with the kernel/hardware, as it happens for screen backlights.
In case of LEDs only the application changing the brightness is
aware of the change.
>> +extern void led_brightness_force_update(struct led_classdev *led_cdev,
>> + enum led_brightness_update_reason reason);
>
> I see this may make some sense, but there are no uses for this in this
> patch.
>
> My preffered solution would be ... for hardware that changes led
> brightness itself, introduce a "trigger", so that userspace knows this
> led is special, and then provide poll()able /sys fs file interested
> parties can read.
OK, I'll see if I can come up something good.
Regards,
Gabriele
> Best regards,
>
> Pavel
>
Hi!
> > Generating uevent for /sys changes does not make much sense, right?
>
> I planned to use this patch mostly for keyboard backlights, for
> which some DEs provide a UI similar to the one for screen backlights.
> Having uevents also for /sys changes means having the UI always in
> sync with the kernel/hardware, as it happens for screen backlights.
> In case of LEDs only the application changing the brightness is
> aware of the change.
>
> >> +extern void led_brightness_force_update(struct led_classdev *led_cdev,
> >> + enum led_brightness_update_reason reason);
> >
> > I see this may make some sense, but there are no uses for this in this
> > patch.
> >
> > My preffered solution would be ... for hardware that changes led
> > brightness itself, introduce a "trigger", so that userspace knows this
> > led is special, and then provide poll()able /sys fs file interested
> > parties can read.
>
> OK, I'll see if I can come up something good.
Please see this thread:
Date: Tue, 15 Nov 2016 13:06:14 +0100
From: Hans de Goede <[email protected]>
To: Pavel Machek <[email protected]>
Cc: Jacek Anaszewski <[email protected]>, Jacek Anaszewski
<[email protected]>, Tony Lindgren
<[email protected]>,
[email protected],
[email protected],
[email protected],
[email protected], Darren Hart
<[email protected]>, Hans de Goede <[email protected]>
Subject: Re: LEDs that change brightness "itself" -- that's a
trigger. Re: PM regression with LED changes in next-20161109
...and you may want to cc: Hans de Goede <[email protected]> . He is
apparently solving same problem.
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi Gabriele, Pavel,
On 12/27/2016 09:07 PM, Pavel Machek wrote:
> Hi!
>
>> Similarily to commit 325253a6b2de ("backlight: Allow drivers to update
>> the core, and generate events on changes"), inform userspace about
>> brightness changes and allow drivers to request updates of the
>> brightness value.
>
> First... we had similar patch in tree, and it caused problems, we are
> now trying to figure out how to do it properly.
>
> LED can be updated many times per second, uevent is probably _not_
> good mechanism to achieve that.
>
> Generating uevent for /sys changes does not make much sense, right?
>
>> +extern void led_brightness_force_update(struct led_classdev *led_cdev,
>> + enum led_brightness_update_reason reason);
>
> I see this may make some sense, but there are no uses for this in this
> patch.
>
> My preffered solution would be ... for hardware that changes led
> brightness itself, introduce a "trigger", so that userspace knows this
> led is special, and then provide poll()able /sys fs file interested
> parties can read.
I've recently submitted for a discussion the idea of "persistent
triggers" [0]. Any feedback is very much appreciated.
[0] https://www.spinics.net/lists/linux-leds/msg07332.html
--
Best regards,
Jacek Anaszewski