Hi Am 11.02.25 um 14:57 schrieb Lee Jones:
On Thu, 06 Feb 2025, Thomas Zimmermann wrote:Remove support for fb events from the led backlight trigger. Provide the helper ledtrig_backlight_blank() instead. Call it from fbdev to inform the trigger of changes to a display's blank state. Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> --- drivers/leds/trigger/ledtrig-backlight.c | 31 +++++------------------- drivers/video/fbdev/core/fbmem.c | 21 +++++++++------- include/linux/leds.h | 6 +++++ 3 files changed, 24 insertions(+), 34 deletions(-) diff --git a/drivers/leds/trigger/ledtrig-backlight.c b/drivers/leds/trigger/ledtrig-backlight.c index f9317f93483b..e3ae4adc29e3 100644 --- a/drivers/leds/trigger/ledtrig-backlight.c +++ b/drivers/leds/trigger/ledtrig-backlight.c @@ -10,7 +10,6 @@ #include <linux/kernel.h> #include <linux/slab.h> #include <linux/init.h> -#include <linux/fb.h> #include <linux/leds.h> #include "../leds.h"@@ -21,7 +20,6 @@ struct bl_trig_notifier {struct led_classdev *led; int brightness; int old_status; - struct notifier_block notifier; unsigned invert;struct list_head entry;@@ -30,7 +28,7 @@ struct bl_trig_notifier { static struct list_head ledtrig_backlight_list; static struct mutex ledtrig_backlight_list_mutex;-static void ledtrig_backlight_blank(struct bl_trig_notifier *n, bool on)+static void __ledtrig_backlight_blank(struct bl_trig_notifier *n, bool on)I'm confused. Didn't you just introduce this?
It's being renamed here; probably something to avoid.
{ struct led_classdev *led = n->led; int new_status = !on ? BLANK : UNBLANK; @@ -48,23 +46,14 @@ static void ledtrig_backlight_blank(struct bl_trig_notifier *n, bool on) n->old_status = new_status; }-static int fb_notifier_callback(struct notifier_block *p,- unsigned long event, void *data) +void ledtrig_backlight_blank(bool on) { - struct bl_trig_notifier *n = container_of(p, - struct bl_trig_notifier, notifier); - struct fb_event *fb_event = data; - int *blank; - - /* If we aren't interested in this event, skip it immediately ... */ - if (event != FB_EVENT_BLANK) - return 0; - - blank = fb_event->data; + struct bl_trig_notifier *n;- ledtrig_backlight_blank(n, !blank[0]);+ guard(mutex)(&ledtrig_backlight_list_mutex);- return 0;+ list_for_each_entry(n, &ledtrig_backlight_list, entry) + __ledtrig_backlight_blank(n, on); }static ssize_t bl_trig_invert_show(struct device *dev,@@ -110,8 +99,6 @@ ATTRIBUTE_GROUPS(bl_trig);static int bl_trig_activate(struct led_classdev *led){ - int ret; - struct bl_trig_notifier *n;n = kzalloc(sizeof(struct bl_trig_notifier), GFP_KERNEL);@@ -122,11 +109,6 @@ static int bl_trig_activate(struct led_classdev *led) n->led = led; n->brightness = led->brightness; n->old_status = UNBLANK; - n->notifier.notifier_call = fb_notifier_callback; - - ret = fb_register_client(&n->notifier); - if (ret) - dev_err(led->dev, "unable to register backlight trigger\n");mutex_lock(&ledtrig_backlight_list_mutex);list_add(&n->entry, &ledtrig_backlight_list); @@ -143,7 +125,6 @@ static void bl_trig_deactivate(struct led_classdev *led) list_del(&n->entry); mutex_unlock(&ledtrig_backlight_list_mutex);- fb_unregister_client(&n->notifier);kfree(n); }diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.cindex fb7ca143a996..92c3fe2a7aff 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -16,6 +16,7 @@ #include <linux/fb.h> #include <linux/fbcon.h> #include <linux/lcd.h> +#include <linux/leds.h>#include <video/nomodeset.h> @@ -373,11 +374,19 @@ static void fb_lcd_notify_blank(struct fb_info *info)#endif }+static void fb_ledtrig_backlight_notify_blank(struct fb_info *info)+{ +#if IS_REACHABLE(CONFIG_LEDS_TRIGGER_BACKLIGHT)#iferry is generally discouraged in C files. Does is_defined() work for you?
I don't think so. This ifdef covers the case that fbdev is built in, but led is a module (i.e., fbdev=y && led=m).
/+ if (info->blank == FB_BLANK_UNBLANK) + ledtrig_backlight_blank(true);If !CONFIG_LEDS_TRIGGER_BACKLIGHT(), then ledtrig_backlight_blank() is a noop anyway, right? So why encompass it in the #if at all?
Because of (fbdev=y && led=m) again. ledtrig_backlight_blank() would be defined then, but not linkable from fbdev. Preferably, I'd rather leave out the ifdef in the led header file.
Best regards Thomas
+ else + ledtrig_backlight_blank(false); +#endif +} + int fb_blank(struct fb_info *info, int blank) { int old_blank = info->blank; - struct fb_event event; - int data[2]; int ret;if (!info->fbops->fb_blank)@@ -386,11 +395,6 @@ int fb_blank(struct fb_info *info, int blank) if (blank > FB_BLANK_POWERDOWN) blank = FB_BLANK_POWERDOWN;- data[0] = blank;- data[1] = old_blank; - event.info = info; - event.data = data; - info->blank = blank;ret = info->fbops->fb_blank(blank, info);@@ -399,8 +403,7 @@ int fb_blank(struct fb_info *info, int blank)fb_bl_notify_blank(info, old_blank);fb_lcd_notify_blank(info); - - fb_notifier_call_chain(FB_EVENT_BLANK, &event); + fb_ledtrig_backlight_notify_blank(info);return 0; diff --git a/include/linux/leds.h b/include/linux/leds.hindex 98f9719c924c..8c7c41888f7d 100644 --- a/include/linux/leds.h +++ b/include/linux/leds.h @@ -640,6 +640,12 @@ static inline void ledtrig_flash_ctrl(bool on) {} static inline void ledtrig_torch_ctrl(bool on) {} #endif+#if IS_ENABLED(CONFIG_LEDS_TRIGGER_BACKLIGHT)+void ledtrig_backlight_blank(bool on); +#else +static inline void ledtrig_backlight_blank(bool on) {} +#endif + /* * Generic LED platform data for describing LED names and default triggers. */ -- 2.48.1
-- -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
