Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1547728 > unrolled thread

[PATCH] leds: Allow drivers to update the core, and generate events on changes

Back to article view | Back to linux.kernel

Contents

  [PATCH] leds: Allow drivers to update the core, and generate events on changes Gabriele Mazzotta <gabriele.mzt@gmail.com> - 2016-12-27 20:20 +0100
    Re: [PATCH] leds: Allow drivers to update the core, and generate  events on changes Pavel Machek <pavel@ucw.cz> - 2016-12-27 21:10 +0100
      Re: [PATCH] leds: Allow drivers to update the core, and generate  events on changes Gabriele Mazzotta <gabriele.mzt@gmail.com> - 2016-12-27 21:30 +0100
        Re: [PATCH] leds: Allow drivers to update the core, and generate  events on changes Pavel Machek <pavel@ucw.cz> - 2016-12-27 22:20 +0100
      Re: [PATCH] leds: Allow drivers to update the core, and generate  events on changes Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2016-12-28 00:00 +0100

#1547728 — [PATCH] leds: Allow drivers to update the core, and generate events on changes

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 <gabriele.mzt@gmail.com>
---
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

[toc] | [next] | [standalone]

#1547742 — Re: [PATCH] leds: Allow drivers to update the core, and generate events on changes

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

#1547745 — Re: [PATCH] leds: Allow drivers to update the core, and generate events on changes

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
> 

[toc] | [prev] | [next] | [standalone]

#1547755 — Re: [PATCH] leds: Allow drivers to update the core, and generate events on changes

[Multipart message — attachments visible in raw view] — view raw

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 <hdegoede@redhat.com>
To: Pavel Machek <pavel@ucw.cz>
Cc: Jacek Anaszewski <j.anaszewski@samsung.com>, Jacek Anaszewski
        <jacek.anaszewski@gmail.com>, Tony Lindgren
	<tony@atomide.com>,
 	linux-leds@vger.kernel.org,
 	linux-omap@vger.kernel.org,
 	linux-arm-kernel@lists.infradead.org,
 	linux-kernel@vger.kernel.org, Darren Hart
 	<dvhart@infradead.org>, Hans de Goede <hdegoede@redhat.com>
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 <hdegoede@redhat.com> . 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

[toc] | [prev] | [next] | [standalone]

#1547789 — Re: [PATCH] leds: Allow drivers to update the core, and generate events on changes

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web