Re: [PATCH 02/30] backlight/gpio_backlight: Compare against struct fb_info.device

Hi

Am 06.06.23 um 09:49 schrieb Dan Carpenter:
> On Tue, Jun 06, 2023 at 09:24:48AM +0200, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 05.06.23 um 22:19 schrieb Ruhl, Michael J:
>>>> -----Original Message-----
>>>> From: dri-devel <[email protected]> On Behalf Of
>>>> Thomas Zimmermann
>>>> Sent: Monday, June 5, 2023 10:48 AM
>>>> To: [email protected]; [email protected]; [email protected];
>>>> [email protected]; [email protected]; [email protected];
>>>> [email protected]; [email protected]
>>>> Cc: [email protected]; Rich Felker <[email protected]>; linux-
>>>> [email protected]; [email protected]; dri-
>>>> [email protected]; Thomas Zimmermann
>>>> <[email protected]>; John Paul Adrian Glaubitz <[email protected]
>>>> berlin.de>; [email protected]
>>>> Subject: [PATCH 02/30] backlight/gpio_backlight: Compare against struct
>>>> fb_info.device
>>>>
>>>> Struct gpio_backlight_platform_data refers to a platform device within
>>>> the Linux device hierarchy. The test in gpio_backlight_check_fb()
>>>> compares it against the fbdev device in struct fb_info.dev, which
>>>> is different. Fix the test by comparing to struct fb_info.device.
>>>>
>>>> Fixes a bug in the backlight driver and prepares fbdev for making
>>>> struct fb_info.dev optional.
>>>
>>> I only see a rename from fbdev  to dev...
>>>
>>> Is there missing code?
>>
>> As Sam said, the compare operation used the wrong device from fb_info.
>>
>> I also changed the naming of a few fields in these backlight drivers. I
>> could move these renames into a separate patch if that makes things easier
>> for reviewers.
>>
>>>
>>> Would  a fixes: be useful?
>>
>> That would be commit 8b770e3c9824 ("backlight: Add GPIO-based backlight
>> driver") from 2013. Maybe a bit old already, but I can surely add it.
> 
> Don't add the Fixes tag to this one because it doesn't fix anything, it
> just renames stuff.  The real fix is later?  To be honest, it was kind
> of difficult to see where the actual fix was.
> 
> Fixes tags for old code is fine...  I like to know why bugs are
> introduced.  Was it adding a feature or part of fix for something else
> or a cleanup?

You're not the first to complain about the renaming. I'll split each 
backlight patch into a bug-fix and a rename patch then. The bugfix will 
get the Fixes tag.

Best regards
Thomas

> 
> regards,
> dan carpenter
> 

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