| Message ID | 20161023194652.24335-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi Hans, Thanks for the update. Could you please rebase your patches onto recent linux-leds.git, for-next branch? Also it would be convenient to have sample application for polling brightness changes in tools/leds. Thanks, Jacek Anaszewski On 10/23/2016 09:46 PM, Hans de Goede wrote: > Add support for userspace using poll() for POLL_PRI on the sysfs brightness > attr to watch for brightness changes. > > This commit adds a led_notify_brightness_change helper function for > waking up any poll() waiters; and calls this after any brightness changes > (after any led_classdev->brightness_set[_blocking] calls have completed). > > In some use-cases led hardware may autonomously change its brightness, > e.g. the keyboard backlight used on some laptops is controlled by a > hardwired (firmware handled) hotkey. led_notify_brightness_change is > exported for use by drivers which can detect such autonomous changes. > > This commit also updates the Documentation/ABI/testing/sysfs-class-led > documentation to document that userspace may now poll on the brightness > attribute. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > Changes in v2: > -Wakeup / notify userspace on any brightness changes, not just on > autonomous changes done by the hw > --- > Documentation/ABI/testing/sysfs-class-led | 8 ++++++-- > drivers/leds/led-class.c | 9 +++++++++ > drivers/leds/led-core.c | 16 +++++++++++++++- > include/linux/leds.h | 12 ++++++++++++ > 4 files changed, 42 insertions(+), 3 deletions(-) > > diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led > index 86ace28..0685551 100644 > --- a/Documentation/ABI/testing/sysfs-class-led > +++ b/Documentation/ABI/testing/sysfs-class-led > @@ -1,12 +1,16 @@ > What: /sys/class/leds/<led>/brightness > -Date: March 2006 > -KernelVersion: 2.6.17 > +Date: March 2006 (poll October 2016) > +KernelVersion: 2.6.17 (poll since 4.10) > Contact: Richard Purdie <rpurdie@rpsys.net> > Description: > Set the brightness of the LED. Most LEDs don't > have hardware brightness support so will just be turned on for > non-zero brightness settings. The value is between 0 and > /sys/class/leds/<led>/max_brightness. > + The file supports poll() to detect brightness changes, in > + some cases the hardware / firmware may change the brightness > + autonomously, poll() should be woken up in this case too, > + but not all drivers may support this. > > What: /sys/class/leds/<led>/max_brightness > Date: March 2006 > diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c > index aa84e5b..3427a65 100644 > --- a/drivers/leds/led-class.c > +++ b/drivers/leds/led-class.c > @@ -203,6 +203,14 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev) > dev_warn(parent, "Led %s renamed to %s due to name collision", > led_cdev->name, dev_name(led_cdev->dev)); > > + led_cdev->brightness_kn = sysfs_get_dirent(led_cdev->dev->kobj.sd, > + "brightness"); > + if (!led_cdev->brightness_kn) { > + dev_err(led_cdev->dev, "Error getting brightness kernfs_node\n"); > + device_unregister(led_cdev->dev); > + return -ENODEV; > + } > + > #ifdef CONFIG_LEDS_TRIGGERS > init_rwsem(&led_cdev->trigger_lock); > #endif > @@ -254,6 +262,7 @@ void led_classdev_unregister(struct led_classdev *led_cdev) > > flush_work(&led_cdev->set_brightness_work); > > + sysfs_put(led_cdev->brightness_kn); > device_unregister(led_cdev->dev); > > down_write(&leds_list_lock); > diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c > index 3bce448..e2e5cc7 100644 > --- a/drivers/leds/led-core.c > +++ b/drivers/leds/led-core.c > @@ -33,16 +33,24 @@ static int __led_set_brightness(struct led_classdev *led_cdev, > > led_cdev->brightness_set(led_cdev, value); > > + led_notify_brightness_change(led_cdev); > + > return 0; > } > > static int __led_set_brightness_blocking(struct led_classdev *led_cdev, > enum led_brightness value) > { > + int ret; > + > if (!led_cdev->brightness_set_blocking) > return -ENOTSUPP; > > - return led_cdev->brightness_set_blocking(led_cdev, value); > + ret = led_cdev->brightness_set_blocking(led_cdev, value); > + if (ret >= 0) > + led_notify_brightness_change(led_cdev); > + > + return ret; > } > > static void led_timer_function(unsigned long data) > @@ -308,6 +316,12 @@ int led_update_brightness(struct led_classdev *led_cdev) > } > EXPORT_SYMBOL_GPL(led_update_brightness); > > +void led_notify_brightness_change(struct led_classdev *led_cdev) > +{ > + sysfs_notify_dirent(led_cdev->brightness_kn); > +} > +EXPORT_SYMBOL_GPL(led_notify_brightness_change); > + > /* Caller must ensure led_cdev->led_access held */ > void led_sysfs_disable(struct led_classdev *led_cdev) > { > diff --git a/include/linux/leds.h b/include/linux/leds.h > index ddfcb2d..eebcd8c 100644 > --- a/include/linux/leds.h > +++ b/include/linux/leds.h > @@ -13,6 +13,7 @@ > #define __LINUX_LEDS_H_INCLUDED > > #include <linux/device.h> > +#include <linux/kernfs.h> > #include <linux/list.h> > #include <linux/mutex.h> > #include <linux/rwsem.h> > @@ -94,6 +95,8 @@ struct led_classdev { > struct work_struct set_brightness_work; > int delayed_set_value; > > + struct kernfs_node *brightness_kn; > + > #ifdef CONFIG_LEDS_TRIGGERS > /* Protects the trigger data below */ > struct rw_semaphore trigger_lock; > @@ -193,6 +196,15 @@ extern int led_set_brightness_sync(struct led_classdev *led_cdev, > extern int led_update_brightness(struct led_classdev *led_cdev); > > /** > + * led_notify_brightness_change - Notify userspace of brightness changes > + * @led_cdev: the LED to do the notify on > + * > + * Let any users waiting for POLL_PRI on the led's brightness sysfs > + * atrribute know that the brightness has been changed. > + */ > +extern void led_notify_brightness_change(struct led_classdev *led_cdev); > + > +/** > * led_sysfs_disable - disable LED sysfs interface > * @led_cdev: the LED to set > * > -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 24-10-16 22:43, Jacek Anaszewski wrote: > Hi Hans, > > Thanks for the update. > Could you please rebase your patches onto recent linux-leds.git, > for-next branch? Done, I will send a v3 soon (today or tomorrow). > Also it would be convenient to have sample application for polling > brightness changes in tools/leds. I'm sorry but I do not have time to create such a sample application. Regards, Hans > > Thanks, > Jacek Anaszewski > > On 10/23/2016 09:46 PM, Hans de Goede wrote: >> Add support for userspace using poll() for POLL_PRI on the sysfs brightness >> attr to watch for brightness changes. >> >> This commit adds a led_notify_brightness_change helper function for >> waking up any poll() waiters; and calls this after any brightness changes >> (after any led_classdev->brightness_set[_blocking] calls have completed). >> >> In some use-cases led hardware may autonomously change its brightness, >> e.g. the keyboard backlight used on some laptops is controlled by a >> hardwired (firmware handled) hotkey. led_notify_brightness_change is >> exported for use by drivers which can detect such autonomous changes. >> >> This commit also updates the Documentation/ABI/testing/sysfs-class-led >> documentation to document that userspace may now poll on the brightness >> attribute. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> Changes in v2: >> -Wakeup / notify userspace on any brightness changes, not just on >> autonomous changes done by the hw >> --- >> Documentation/ABI/testing/sysfs-class-led | 8 ++++++-- >> drivers/leds/led-class.c | 9 +++++++++ >> drivers/leds/led-core.c | 16 +++++++++++++++- >> include/linux/leds.h | 12 ++++++++++++ >> 4 files changed, 42 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led >> index 86ace28..0685551 100644 >> --- a/Documentation/ABI/testing/sysfs-class-led >> +++ b/Documentation/ABI/testing/sysfs-class-led >> @@ -1,12 +1,16 @@ >> What: /sys/class/leds/<led>/brightness >> -Date: March 2006 >> -KernelVersion: 2.6.17 >> +Date: March 2006 (poll October 2016) >> +KernelVersion: 2.6.17 (poll since 4.10) >> Contact: Richard Purdie <rpurdie@rpsys.net> >> Description: >> Set the brightness of the LED. Most LEDs don't >> have hardware brightness support so will just be turned on for >> non-zero brightness settings. The value is between 0 and >> /sys/class/leds/<led>/max_brightness. >> + The file supports poll() to detect brightness changes, in >> + some cases the hardware / firmware may change the brightness >> + autonomously, poll() should be woken up in this case too, >> + but not all drivers may support this. >> >> What: /sys/class/leds/<led>/max_brightness >> Date: March 2006 >> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c >> index aa84e5b..3427a65 100644 >> --- a/drivers/leds/led-class.c >> +++ b/drivers/leds/led-class.c >> @@ -203,6 +203,14 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev) >> dev_warn(parent, "Led %s renamed to %s due to name collision", >> led_cdev->name, dev_name(led_cdev->dev)); >> >> + led_cdev->brightness_kn = sysfs_get_dirent(led_cdev->dev->kobj.sd, >> + "brightness"); >> + if (!led_cdev->brightness_kn) { >> + dev_err(led_cdev->dev, "Error getting brightness kernfs_node\n"); >> + device_unregister(led_cdev->dev); >> + return -ENODEV; >> + } >> + >> #ifdef CONFIG_LEDS_TRIGGERS >> init_rwsem(&led_cdev->trigger_lock); >> #endif >> @@ -254,6 +262,7 @@ void led_classdev_unregister(struct led_classdev *led_cdev) >> >> flush_work(&led_cdev->set_brightness_work); >> >> + sysfs_put(led_cdev->brightness_kn); >> device_unregister(led_cdev->dev); >> >> down_write(&leds_list_lock); >> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c >> index 3bce448..e2e5cc7 100644 >> --- a/drivers/leds/led-core.c >> +++ b/drivers/leds/led-core.c >> @@ -33,16 +33,24 @@ static int __led_set_brightness(struct led_classdev *led_cdev, >> >> led_cdev->brightness_set(led_cdev, value); >> >> + led_notify_brightness_change(led_cdev); >> + >> return 0; >> } >> >> static int __led_set_brightness_blocking(struct led_classdev *led_cdev, >> enum led_brightness value) >> { >> + int ret; >> + >> if (!led_cdev->brightness_set_blocking) >> return -ENOTSUPP; >> >> - return led_cdev->brightness_set_blocking(led_cdev, value); >> + ret = led_cdev->brightness_set_blocking(led_cdev, value); >> + if (ret >= 0) >> + led_notify_brightness_change(led_cdev); >> + >> + return ret; >> } >> >> static void led_timer_function(unsigned long data) >> @@ -308,6 +316,12 @@ int led_update_brightness(struct led_classdev *led_cdev) >> } >> EXPORT_SYMBOL_GPL(led_update_brightness); >> >> +void led_notify_brightness_change(struct led_classdev *led_cdev) >> +{ >> + sysfs_notify_dirent(led_cdev->brightness_kn); >> +} >> +EXPORT_SYMBOL_GPL(led_notify_brightness_change); >> + >> /* Caller must ensure led_cdev->led_access held */ >> void led_sysfs_disable(struct led_classdev *led_cdev) >> { >> diff --git a/include/linux/leds.h b/include/linux/leds.h >> index ddfcb2d..eebcd8c 100644 >> --- a/include/linux/leds.h >> +++ b/include/linux/leds.h >> @@ -13,6 +13,7 @@ >> #define __LINUX_LEDS_H_INCLUDED >> >> #include <linux/device.h> >> +#include <linux/kernfs.h> >> #include <linux/list.h> >> #include <linux/mutex.h> >> #include <linux/rwsem.h> >> @@ -94,6 +95,8 @@ struct led_classdev { >> struct work_struct set_brightness_work; >> int delayed_set_value; >> >> + struct kernfs_node *brightness_kn; >> + >> #ifdef CONFIG_LEDS_TRIGGERS >> /* Protects the trigger data below */ >> struct rw_semaphore trigger_lock; >> @@ -193,6 +196,15 @@ extern int led_set_brightness_sync(struct led_classdev *led_cdev, >> extern int led_update_brightness(struct led_classdev *led_cdev); >> >> /** >> + * led_notify_brightness_change - Notify userspace of brightness changes >> + * @led_cdev: the LED to do the notify on >> + * >> + * Let any users waiting for POLL_PRI on the led's brightness sysfs >> + * atrribute know that the brightness has been changed. >> + */ >> +extern void led_notify_brightness_change(struct led_classdev *led_cdev); >> + >> +/** >> * led_sysfs_disable - disable LED sysfs interface >> * @led_cdev: the LED to set >> * >> -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 10/26/2016 05:18 PM, Hans de Goede wrote: > Hi, > > On 24-10-16 22:43, Jacek Anaszewski wrote: >> Hi Hans, >> >> Thanks for the update. >> Could you please rebase your patches onto recent linux-leds.git, >> for-next branch? > > Done, I will send a v3 soon (today or tomorrow). > >> Also it would be convenient to have sample application for polling >> brightness changes in tools/leds. > > I'm sorry but I do not have time to create such a sample application. You must have tested your solution somehow. Just polish the code a bit, add a Makefile and that's it. Compare tools/leds/uledmon.c - it doesn't need to be anything big. If you're adding a feature that interacts with userspace, then I'd like to have a sample code that shows how to use it. Just for a reference.
Hi, On 27-10-16 08:58, Jacek Anaszewski wrote: > Hi, > > On 10/26/2016 05:18 PM, Hans de Goede wrote: >> Hi, >> >> On 24-10-16 22:43, Jacek Anaszewski wrote: >>> Hi Hans, >>> >>> Thanks for the update. >>> Could you please rebase your patches onto recent linux-leds.git, >>> for-next branch? >> >> Done, I will send a v3 soon (today or tomorrow). >> >>> Also it would be convenient to have sample application for polling >>> brightness changes in tools/leds. >> >> I'm sorry but I do not have time to create such a sample application. > > You must have tested your solution somehow. Yes, as is made clear by the other patches in this set, I'm using it with keyboard backlight control led interfaces. Which in userspace are handled by upower which provides a dbus interface which is used by gnome-settings-daemon. > Just polish the code a bit, > add a Makefile and that's it. Compare tools/leds/uledmon.c - it doesn't > need to be anything big. upower uses gio which is really not suitable for this, so I would need to create something from scratch. Regards, Hans > > If you're adding a feature that interacts with userspace, then I'd like > to have a sample code that shows how to use it. Just for a reference. > -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/27/2016 09:33 AM, Hans de Goede wrote: > Hi, > > On 27-10-16 08:58, Jacek Anaszewski wrote: >> Hi, >> >> On 10/26/2016 05:18 PM, Hans de Goede wrote: >>> Hi, >>> >>> On 24-10-16 22:43, Jacek Anaszewski wrote: >>>> Hi Hans, >>>> >>>> Thanks for the update. >>>> Could you please rebase your patches onto recent linux-leds.git, >>>> for-next branch? >>> >>> Done, I will send a v3 soon (today or tomorrow). >>> >>>> Also it would be convenient to have sample application for polling >>>> brightness changes in tools/leds. >>> >>> I'm sorry but I do not have time to create such a sample application. >> >> You must have tested your solution somehow. > > Yes, as is made clear by the other patches in this set, I'm using > it with keyboard backlight control led interfaces. Which in userspace > are handled by upower which provides a dbus interface which is used > by gnome-settings-daemon. > >> Just polish the code a bit, >> add a Makefile and that's it. Compare tools/leds/uledmon.c - it doesn't >> need to be anything big. > > upower uses gio which is really not suitable for this, so I would need > to create something from scratch. Okay, so I'll have to do it myself, when I'll find a moment.
diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led index 86ace28..0685551 100644 --- a/Documentation/ABI/testing/sysfs-class-led +++ b/Documentation/ABI/testing/sysfs-class-led @@ -1,12 +1,16 @@ What: /sys/class/leds/<led>/brightness -Date: March 2006 -KernelVersion: 2.6.17 +Date: March 2006 (poll October 2016) +KernelVersion: 2.6.17 (poll since 4.10) Contact: Richard Purdie <rpurdie@rpsys.net> Description: Set the brightness of the LED. Most LEDs don't have hardware brightness support so will just be turned on for non-zero brightness settings. The value is between 0 and /sys/class/leds/<led>/max_brightness. + The file supports poll() to detect brightness changes, in + some cases the hardware / firmware may change the brightness + autonomously, poll() should be woken up in this case too, + but not all drivers may support this. What: /sys/class/leds/<led>/max_brightness Date: March 2006 diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c index aa84e5b..3427a65 100644 --- a/drivers/leds/led-class.c +++ b/drivers/leds/led-class.c @@ -203,6 +203,14 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev) dev_warn(parent, "Led %s renamed to %s due to name collision", led_cdev->name, dev_name(led_cdev->dev)); + led_cdev->brightness_kn = sysfs_get_dirent(led_cdev->dev->kobj.sd, + "brightness"); + if (!led_cdev->brightness_kn) { + dev_err(led_cdev->dev, "Error getting brightness kernfs_node\n"); + device_unregister(led_cdev->dev); + return -ENODEV; + } + #ifdef CONFIG_LEDS_TRIGGERS init_rwsem(&led_cdev->trigger_lock); #endif @@ -254,6 +262,7 @@ void led_classdev_unregister(struct led_classdev *led_cdev) flush_work(&led_cdev->set_brightness_work); + sysfs_put(led_cdev->brightness_kn); device_unregister(led_cdev->dev); down_write(&leds_list_lock); diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c index 3bce448..e2e5cc7 100644 --- a/drivers/leds/led-core.c +++ b/drivers/leds/led-core.c @@ -33,16 +33,24 @@ static int __led_set_brightness(struct led_classdev *led_cdev, led_cdev->brightness_set(led_cdev, value); + led_notify_brightness_change(led_cdev); + return 0; } static int __led_set_brightness_blocking(struct led_classdev *led_cdev, enum led_brightness value) { + int ret; + if (!led_cdev->brightness_set_blocking) return -ENOTSUPP; - return led_cdev->brightness_set_blocking(led_cdev, value); + ret = led_cdev->brightness_set_blocking(led_cdev, value); + if (ret >= 0) + led_notify_brightness_change(led_cdev); + + return ret; } static void led_timer_function(unsigned long data) @@ -308,6 +316,12 @@ int led_update_brightness(struct led_classdev *led_cdev) } EXPORT_SYMBOL_GPL(led_update_brightness); +void led_notify_brightness_change(struct led_classdev *led_cdev) +{ + sysfs_notify_dirent(led_cdev->brightness_kn); +} +EXPORT_SYMBOL_GPL(led_notify_brightness_change); + /* Caller must ensure led_cdev->led_access held */ void led_sysfs_disable(struct led_classdev *led_cdev) { diff --git a/include/linux/leds.h b/include/linux/leds.h index ddfcb2d..eebcd8c 100644 --- a/include/linux/leds.h +++ b/include/linux/leds.h @@ -13,6 +13,7 @@ #define __LINUX_LEDS_H_INCLUDED #include <linux/device.h> +#include <linux/kernfs.h> #include <linux/list.h> #include <linux/mutex.h> #include <linux/rwsem.h> @@ -94,6 +95,8 @@ struct led_classdev { struct work_struct set_brightness_work; int delayed_set_value; + struct kernfs_node *brightness_kn; + #ifdef CONFIG_LEDS_TRIGGERS /* Protects the trigger data below */ struct rw_semaphore trigger_lock; @@ -193,6 +196,15 @@ extern int led_set_brightness_sync(struct led_classdev *led_cdev, extern int led_update_brightness(struct led_classdev *led_cdev); /** + * led_notify_brightness_change - Notify userspace of brightness changes + * @led_cdev: the LED to do the notify on + * + * Let any users waiting for POLL_PRI on the led's brightness sysfs + * atrribute know that the brightness has been changed. + */ +extern void led_notify_brightness_change(struct led_classdev *led_cdev); + +/** * led_sysfs_disable - disable LED sysfs interface * @led_cdev: the LED to set *
Add support for userspace using poll() for POLL_PRI on the sysfs brightness attr to watch for brightness changes. This commit adds a led_notify_brightness_change helper function for waking up any poll() waiters; and calls this after any brightness changes (after any led_classdev->brightness_set[_blocking] calls have completed). In some use-cases led hardware may autonomously change its brightness, e.g. the keyboard backlight used on some laptops is controlled by a hardwired (firmware handled) hotkey. led_notify_brightness_change is exported for use by drivers which can detect such autonomous changes. This commit also updates the Documentation/ABI/testing/sysfs-class-led documentation to document that userspace may now poll on the brightness attribute. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Changes in v2: -Wakeup / notify userspace on any brightness changes, not just on autonomous changes done by the hw --- Documentation/ABI/testing/sysfs-class-led | 8 ++++++-- drivers/leds/led-class.c | 9 +++++++++ drivers/leds/led-core.c | 16 +++++++++++++++- include/linux/leds.h | 12 ++++++++++++ 4 files changed, 42 insertions(+), 3 deletions(-)