| Message ID | 20240221094324.27436-2-tzimmermann@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | backlight: Replace struct fb_info in interfaces | expand |
On Wed, Feb 21, 2024 at 5:45 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: > Am 21.02.24 um 15:34 schrieb Andy Shevchenko: > > On Wed, Feb 21, 2024 at 10:41:28AM +0100, Thomas Zimmermann wrote: > >> Framebuffer drivers for devices with dedicated backlight are supposed > >> to set struct fb_info.bl_dev to the backlight's respective device. Use > >> the value to match backlight and framebuffer in the backlight core code. ... > >> if (!bd->ops) > >> goto out; > >> - if (bd->ops->check_fb && !bd->ops->check_fb(bd, evdata->info)) > >> + else if (bd->ops->check_fb && !bd->ops->check_fb(bd, info)) > > What's the point of adding redundant 'else'? > > > >> goto out; > >> +#if IS_ENABLED(CONFIG_FB_BACKLIGHT) > >> + else if (info->bl_dev && info->bl_dev != bd) > > Ditto. > > They group these tests into one single block of code; signaling that > these tests serve the same purpose. Commit message has nothing about this. Also if needed, it should be a separate change. > >> + goto out; > >> +#endif
On Wed, 21 Feb 2024, Thomas Zimmermann wrote: > Framebuffer drivers for devices with dedicated backlight are supposed > to set struct fb_info.bl_dev to the backlight's respective device. Use > the value to match backlight and framebuffer in the backlight core code. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org> > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> > --- > drivers/video/backlight/backlight.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c > index 86e1cdc8e3697..48844a4f28ad3 100644 > --- a/drivers/video/backlight/backlight.c > +++ b/drivers/video/backlight/backlight.c > @@ -98,7 +98,8 @@ static int fb_notifier_callback(struct notifier_block *self, > { > struct backlight_device *bd; > struct fb_event *evdata = data; > - int node = evdata->info->node; > + struct fb_info *info = evdata->info; > + int node = info->node; > int fb_blank = 0; > > /* If we aren't interested in this event, skip it immediately ... */ > @@ -110,8 +111,12 @@ static int fb_notifier_callback(struct notifier_block *self, > > if (!bd->ops) > goto out; > - if (bd->ops->check_fb && !bd->ops->check_fb(bd, evdata->info)) > + else if (bd->ops->check_fb && !bd->ops->check_fb(bd, info)) > goto out; > +#if IS_ENABLED(CONFIG_FB_BACKLIGHT) I don't want #ifery in C files. Please find another way. > + else if (info->bl_dev && info->bl_dev != bd) > + goto out; > +#endif > > fb_blank = *(int *)evdata->data; > if (fb_blank == FB_BLANK_UNBLANK && !bd->fb_bl_on[node]) { > -- > 2.43.0 >
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c index 86e1cdc8e3697..48844a4f28ad3 100644 --- a/drivers/video/backlight/backlight.c +++ b/drivers/video/backlight/backlight.c @@ -98,7 +98,8 @@ static int fb_notifier_callback(struct notifier_block *self, { struct backlight_device *bd; struct fb_event *evdata = data; - int node = evdata->info->node; + struct fb_info *info = evdata->info; + int node = info->node; int fb_blank = 0; /* If we aren't interested in this event, skip it immediately ... */ @@ -110,8 +111,12 @@ static int fb_notifier_callback(struct notifier_block *self, if (!bd->ops) goto out; - if (bd->ops->check_fb && !bd->ops->check_fb(bd, evdata->info)) + else if (bd->ops->check_fb && !bd->ops->check_fb(bd, info)) goto out; +#if IS_ENABLED(CONFIG_FB_BACKLIGHT) + else if (info->bl_dev && info->bl_dev != bd) + goto out; +#endif fb_blank = *(int *)evdata->data; if (fb_blank == FB_BLANK_UNBLANK && !bd->fb_bl_on[node]) {