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

From: Thomas Zimmermann <tzimmermann@suse.de>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: "Ruhl, Michael J" <michael.j.ruhl@intel.com>,
	"daniel@ffwll.ch" <daniel@ffwll.ch>,
	"javierm@redhat.com" <javierm@redhat.com>,
	"sam@ravnborg.org" <sam@ravnborg.org>,
	"deller@gmx.de" <deller@gmx.de>,
	"geert+renesas@glider.be" <geert+renesas@glider.be>,
	"lee@kernel.org" <lee@kernel.org>,
	"daniel.thompson@linaro.org" <daniel.thompson@linaro.org>,
	"jingoohan1@gmail.com" <jingoohan1@gmail.com>,
	"linux-fbdev@vger.kernel.org" <linux-fbdev@vger.kernel.org>,
	Rich Felker <dalias@libc.org>,
	"linux-sh@vger.kernel.org" <linux-sh@vger.kernel.org>,
	"linux-staging@lists.linux.dev" <linux-staging@lists.linux.dev>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>,
	"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>
Subject: Re: [PATCH 02/30] backlight/gpio_backlight: Compare against struct fb_info.device
Date: Tue, 6 Jun 2023 10:05:29 +0200	[thread overview]
Message-ID: <37860c28-8b9b-448f-98d3-80ad427d1315@suse.de> (raw)
In-Reply-To: <9ed217b1-3a05-4daf-9e6f-52453a8b5f3e@kadam.mountain>

[-- 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 --]