Re: [PATCH v2 20/28] fbdev: clps711x-fb: Replace check_fb in favor of struct fb_info.lcd_dev

[Thomas Zimmermann <tzimmermann@suse.de>]

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

-- 
--
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)