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

[Thomas Zimmermann <tzimmermann@suse.de>]

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

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.

Best regards
Thomas

> 
> M
> 
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: Rich Felker <dalias@libc.org>
>> Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
>> Cc: Lee Jones <lee@kernel.org>
>> Cc: Daniel Thompson <daniel.thompson@linaro.org>
>> Cc: Jingoo Han <jingoohan1@gmail.com>
>> Cc: linux-sh@vger.kernel.org
>> ---
>> arch/sh/boards/mach-ecovec24/setup.c         | 2 +-
>> drivers/video/backlight/gpio_backlight.c     | 6 +++---
>> include/linux/platform_data/gpio_backlight.h | 2 +-
>> 3 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-
>> ecovec24/setup.c
>> index 674da7ebd8b7..310513646c9b 100644
>> --- a/arch/sh/boards/mach-ecovec24/setup.c
>> +++ b/arch/sh/boards/mach-ecovec24/setup.c
>> @@ -386,7 +386,7 @@ static struct property_entry gpio_backlight_props[] = {
>> };
>>
>> static struct gpio_backlight_platform_data gpio_backlight_data = {
>> -	.fbdev = &lcdc_device.dev,
>> +	.dev = &lcdc_device.dev,
>> };
>>
>> static const struct platform_device_info gpio_backlight_device_info = {
>> diff --git a/drivers/video/backlight/gpio_backlight.c
>> b/drivers/video/backlight/gpio_backlight.c
>> index 6f78d928f054..d3bea42407f1 100644
>> --- a/drivers/video/backlight/gpio_backlight.c
>> +++ b/drivers/video/backlight/gpio_backlight.c
>> @@ -17,7 +17,7 @@
>> #include <linux/slab.h>
>>
>> struct gpio_backlight {
>> -	struct device *fbdev;
>> +	struct device *dev;
>> 	struct gpio_desc *gpiod;
>> };
>>
>> @@ -35,7 +35,7 @@ static int gpio_backlight_check_fb(struct
>> backlight_device *bl,
>> {
>> 	struct gpio_backlight *gbl = bl_get_data(bl);
>>
>> -	return gbl->fbdev == NULL || gbl->fbdev == info->dev;
>> +	return !gbl->dev || gbl->dev == info->device;
>> }
>>
>> static const struct backlight_ops gpio_backlight_ops = {
>> @@ -59,7 +59,7 @@ static int gpio_backlight_probe(struct platform_device
>> *pdev)
>> 		return -ENOMEM;
>>
>> 	if (pdata)
>> -		gbl->fbdev = pdata->fbdev;
>> +		gbl->dev = pdata->dev;
>>
>> 	def_value = device_property_read_bool(dev, "default-on");
>>
>> diff --git a/include/linux/platform_data/gpio_backlight.h
>> b/include/linux/platform_data/gpio_backlight.h
>> index 1a8b5b1946fe..323fbf5f7613 100644
>> --- a/include/linux/platform_data/gpio_backlight.h
>> +++ b/include/linux/platform_data/gpio_backlight.h
>> @@ -8,7 +8,7 @@
>> struct device;
>>
>> struct gpio_backlight_platform_data {
>> -	struct device *fbdev;
>> +	struct device *dev;
>> };
>>
>> #endif
>> --
>> 2.40.1
> 

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