| Message ID | 20240906075439.98476-21-tzimmermann@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | backlight: lcd: Remove fbdev dependencies | expand |
Op 06-09-2024 om 09:52 schreef Thomas Zimmermann: > Store the lcd device in struct fb_info.lcd_dev. The lcd subsystem can > now detect the lcd's fbdev device from this field. > > This makes the implementation of check_fb in clps711x_lcd_ops obsolete. > Remove it. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org> > --- > drivers/video/fbdev/clps711x-fb.c | 23 ++++++++++------------- > 1 file changed, 10 insertions(+), 13 deletions(-) > > diff --git a/drivers/video/fbdev/clps711x-fb.c b/drivers/video/fbdev/clps711x-fb.c > index 6171a98a48fd..4340ea3b9660 100644 > --- a/drivers/video/fbdev/clps711x-fb.c > +++ b/drivers/video/fbdev/clps711x-fb.c > @@ -162,13 +162,6 @@ static const struct fb_ops clps711x_fb_ops = { > .fb_blank = clps711x_fb_blank, > }; > > -static int clps711x_lcd_check_fb(struct lcd_device *lcddev, struct fb_info *fi) > -{ > - struct clps711x_fb_info *cfb = dev_get_drvdata(&lcddev->dev); > - > - return (!fi || fi->par == cfb) ? 1 : 0; > -} > - > static int clps711x_lcd_get_power(struct lcd_device *lcddev) > { > struct clps711x_fb_info *cfb = dev_get_drvdata(&lcddev->dev); > @@ -198,7 +191,6 @@ static int clps711x_lcd_set_power(struct lcd_device *lcddev, int blank) > } > > static const struct lcd_ops clps711x_lcd_ops = { > - .check_fb = clps711x_lcd_check_fb, > .get_power = clps711x_lcd_get_power, > .set_power = clps711x_lcd_set_power, > }; > @@ -325,16 +317,21 @@ static int clps711x_fb_probe(struct platform_device *pdev) > if (ret) > goto out_fb_dealloc_cmap; > > + lcd = devm_lcd_device_register(dev, "clps711x-lcd", dev, cfb, > + &clps711x_lcd_ops); > + if (IS_ERR(lcd)) { > + ret = PTR_ERR(lcd); > + goto out_fb_dealloc_cmap; > + } > + > + info->lcd_dev = lcd; > + > ret = register_framebuffer(info); > if (ret) > goto out_fb_dealloc_cmap; > > - lcd = devm_lcd_device_register(dev, "clps711x-lcd", dev, cfb, > - &clps711x_lcd_ops); > - if (!IS_ERR(lcd)) > - return 0; > + return 0; > > - ret = PTR_ERR(lcd); > unregister_framebuffer(info); > > out_fb_dealloc_cmap: Something is not right here. With the current patch you'll make the unregister_framebuffer(info) unreachable, because there is a return 0 in front. Please check again.
Hi Am 03.10.24 um 20:33 schrieb Kees Bakker: > Op 06-09-2024 om 09:52 schreef Thomas Zimmermann: >> Store the lcd device in struct fb_info.lcd_dev. The lcd subsystem can >> now detect the lcd's fbdev device from this field. >> >> This makes the implementation of check_fb in clps711x_lcd_ops obsolete. >> Remove it. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org> >> --- >> drivers/video/fbdev/clps711x-fb.c | 23 ++++++++++------------- >> 1 file changed, 10 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/video/fbdev/clps711x-fb.c >> b/drivers/video/fbdev/clps711x-fb.c >> index 6171a98a48fd..4340ea3b9660 100644 >> --- a/drivers/video/fbdev/clps711x-fb.c >> +++ b/drivers/video/fbdev/clps711x-fb.c >> @@ -162,13 +162,6 @@ static const struct fb_ops clps711x_fb_ops = { >> .fb_blank = clps711x_fb_blank, >> }; >> -static int clps711x_lcd_check_fb(struct lcd_device *lcddev, struct >> fb_info *fi) >> -{ >> - struct clps711x_fb_info *cfb = dev_get_drvdata(&lcddev->dev); >> - >> - return (!fi || fi->par == cfb) ? 1 : 0; >> -} >> - >> static int clps711x_lcd_get_power(struct lcd_device *lcddev) >> { >> struct clps711x_fb_info *cfb = dev_get_drvdata(&lcddev->dev); >> @@ -198,7 +191,6 @@ static int clps711x_lcd_set_power(struct >> lcd_device *lcddev, int blank) >> } >> static const struct lcd_ops clps711x_lcd_ops = { >> - .check_fb = clps711x_lcd_check_fb, >> .get_power = clps711x_lcd_get_power, >> .set_power = clps711x_lcd_set_power, >> }; >> @@ -325,16 +317,21 @@ static int clps711x_fb_probe(struct >> platform_device *pdev) >> if (ret) >> goto out_fb_dealloc_cmap; >> + lcd = devm_lcd_device_register(dev, "clps711x-lcd", dev, cfb, >> + &clps711x_lcd_ops); >> + if (IS_ERR(lcd)) { >> + ret = PTR_ERR(lcd); >> + goto out_fb_dealloc_cmap; >> + } >> + >> + info->lcd_dev = lcd; >> + >> ret = register_framebuffer(info); >> if (ret) >> goto out_fb_dealloc_cmap; >> - lcd = devm_lcd_device_register(dev, "clps711x-lcd", dev, cfb, >> - &clps711x_lcd_ops); >> - if (!IS_ERR(lcd)) >> - return 0; >> + return 0; >> - ret = PTR_ERR(lcd); >> unregister_framebuffer(info); >> out_fb_dealloc_cmap: > Something is not right here. With the current patch you'll make the > unregister_framebuffer(info) > unreachable, because there is a return 0 in front. > Please check again. See https://lore.kernel.org/linux-fbdev/20241004014349.435006-1-qianqiang.liu@163.com/T/#u This line used to be code for error rollback, but is now unnecessary AFAICT. Best regards Thomas
Fri, Sep 06, 2024 at 09:52:34AM +0200, Thomas Zimmermann kirjoitti: > Store the lcd device in struct fb_info.lcd_dev. The lcd subsystem can > now detect the lcd's fbdev device from this field. > > This makes the implementation of check_fb in clps711x_lcd_ops obsolete. > Remove it. ... > + lcd = devm_lcd_device_register(dev, "clps711x-lcd", dev, cfb, > + &clps711x_lcd_ops); > + if (IS_ERR(lcd)) { > + ret = PTR_ERR(lcd); > + goto out_fb_dealloc_cmap; > + } > + > + info->lcd_dev = lcd; > + > ret = register_framebuffer(info); > if (ret) > goto out_fb_dealloc_cmap; > > - lcd = devm_lcd_device_register(dev, "clps711x-lcd", dev, cfb, > - &clps711x_lcd_ops); > - if (!IS_ERR(lcd)) > - return 0; > + return 0; > > - ret = PTR_ERR(lcd); > unregister_framebuffer(info); Haven't you got a dead code warning here? > > out_fb_dealloc_cmap:
diff --git a/drivers/video/fbdev/clps711x-fb.c b/drivers/video/fbdev/clps711x-fb.c index 6171a98a48fd..4340ea3b9660 100644 --- a/drivers/video/fbdev/clps711x-fb.c +++ b/drivers/video/fbdev/clps711x-fb.c @@ -162,13 +162,6 @@ static const struct fb_ops clps711x_fb_ops = { .fb_blank = clps711x_fb_blank, }; -static int clps711x_lcd_check_fb(struct lcd_device *lcddev, struct fb_info *fi) -{ - struct clps711x_fb_info *cfb = dev_get_drvdata(&lcddev->dev); - - return (!fi || fi->par == cfb) ? 1 : 0; -} - static int clps711x_lcd_get_power(struct lcd_device *lcddev) { struct clps711x_fb_info *cfb = dev_get_drvdata(&lcddev->dev); @@ -198,7 +191,6 @@ static int clps711x_lcd_set_power(struct lcd_device *lcddev, int blank) } static const struct lcd_ops clps711x_lcd_ops = { - .check_fb = clps711x_lcd_check_fb, .get_power = clps711x_lcd_get_power, .set_power = clps711x_lcd_set_power, }; @@ -325,16 +317,21 @@ static int clps711x_fb_probe(struct platform_device *pdev) if (ret) goto out_fb_dealloc_cmap; + lcd = devm_lcd_device_register(dev, "clps711x-lcd", dev, cfb, + &clps711x_lcd_ops); + if (IS_ERR(lcd)) { + ret = PTR_ERR(lcd); + goto out_fb_dealloc_cmap; + } + + info->lcd_dev = lcd; + ret = register_framebuffer(info); if (ret) goto out_fb_dealloc_cmap; - lcd = devm_lcd_device_register(dev, "clps711x-lcd", dev, cfb, - &clps711x_lcd_ops); - if (!IS_ERR(lcd)) - return 0; + return 0; - ret = PTR_ERR(lcd); unregister_framebuffer(info); out_fb_dealloc_cmap: