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

Thomas Zimmermann
2023-06-06T08:05:31Z

[-- Attachment #1.1: Type: text/plain, Size: 2721 bytes --]

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 <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
>>>> Thomas Zimmermann
>>>> Sent: Monday, June 5, 2023 10:48 AM
>>>> To: daniel@ffwll.ch; javierm@redhat.com; sam@ravnborg.org;
>>>> deller@gmx.de; geert+renesas@glider.be; lee@kernel.org;
>>>> daniel.thompson@linaro.org; jingoohan1@gmail.com
>>>> Cc: linux-fbdev@vger.kernel.org; Rich Felker <dalias@libc.org>; linux-
>>>> sh@vger.kernel.org; linux-staging@lists.linux.dev; dri-
>>>> devel@lists.freedesktop.org; Thomas Zimmermann
>>>> <tzimmermann@suse.de>; John Paul Adrian Glaubitz <glaubitz@physik.fu-
>>>> berlin.de>; linux-omap@vger.kernel.org
>>>> 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)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]