| Message ID | 20250206154033.697495-13-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
| Series | backlight, lcd, led: Remove fbdev dependencies | expand |
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@suse.de> > --- > 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? > { > 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.c > index 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? / > + 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? > + 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.h > index 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 >
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@suse.de> >> --- >> 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.c >> index 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.h >> index 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 >>
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) { 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.c index 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) + if (info->blank == FB_BLANK_UNBLANK) + ledtrig_backlight_blank(true); + 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.h index 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. */
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@suse.de> --- 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(-)