[PATCH RFT 0/4] backlight: gpio: simplify the driver

From: Bartosz Golaszewski <redacted>

While working on my other series related to gpio-backlight[1] I noticed
that we could simplify the driver if we made the only user of platform
data use GPIO lookups and device properties. This series tries to do
that.

The first patch sets up all the required structures in the board file,
the second modifies the backlight driver, the third and fourth remove
the leftovers.

This series depends on the three first patches from [1].

I don't have access to this HW but hopefully this works. Only compile
tested.

[1] https://lkml.org/lkml/2019/6/25/900

Bartosz Golaszewski (4):
  sh: ecovec24: add additional properties to the backlight device
  backlight: gpio: simplify the platform data handling
  sh: ecovec24: don't set unused fields in platform data
  backlight: gpio: remove unused fields from platform data

 arch/sh/boards/mach-ecovec24/setup.c         | 33 ++++++++++----
 drivers/video/backlight/gpio_backlight.c     | 46 ++++++--------------
 include/linux/platform_data/gpio_backlight.h |  3 --
 3 files changed, 38 insertions(+), 44 deletions(-)

-- 
2.21.0

From: Bartosz Golaszewski <redacted>

Platform data fields other than fbdev are no longer used by the
backlight driver. Remove them.

Signed-off-by: Bartosz Golaszewski <redacted>
---
 arch/sh/boards/mach-ecovec24/setup.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
index 6926bb3865b9..64a5a1662b6d 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c

@@ -386,9 +386,6 @@ static struct property_entry gpio_backlight_props[] = {
 
 static struct gpio_backlight_platform_data gpio_backlight_data = {
 	.fbdev = &lcdc_device.dev,
-	.gpio = GPIO_PTR1,
-	.def_value = 1,
-	.name = "backlight",
 };
 
 static const struct platform_device_info gpio_backlight_device_info = {

-- 
2.21.0

From: Bartosz Golaszewski <redacted>

Remove the platform data fields that nobody uses.

Signed-off-by: Bartosz Golaszewski <redacted>
---
 include/linux/platform_data/gpio_backlight.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/linux/platform_data/gpio_backlight.h b/include/linux/platform_data/gpio_backlight.h
index 34179d600360..1a8b5b1946fe 100644
--- a/include/linux/platform_data/gpio_backlight.h
+++ b/include/linux/platform_data/gpio_backlight.h

@@ -9,9 +9,6 @@ struct device;
 
 struct gpio_backlight_platform_data {
 	struct device *fbdev;
-	int gpio;
-	int def_value;
-	const char *name;
 };
 
 #endif

-- 
2.21.0

From: Bartosz Golaszewski <redacted>

Add a GPIO lookup entry and a device property for GPIO backlight to the
board file. Tie them to the platform device which is now registered using
platform_device_register_full() because of the properties. These changes
are inactive now but will be used once the gpio backlight driver is
modified.

Signed-off-by: Bartosz Golaszewski <redacted>
---
 arch/sh/boards/mach-ecovec24/setup.c | 30 +++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
index f402aa741bf3..6926bb3865b9 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c

@@ -371,6 +371,19 @@ static struct platform_device lcdc_device = {
 	},
 };
 
+static struct gpiod_lookup_table gpio_backlight_lookup = {
+	.dev_id		= "gpio-backlight.0",
+	.table = {
+		GPIO_LOOKUP("sh7724_pfc", GPIO_PTR1, NULL, GPIO_ACTIVE_HIGH),
+		{ }
+	},
+};
+
+static struct property_entry gpio_backlight_props[] = {
+	PROPERTY_ENTRY_BOOL("default-on"),
+	{ }
+};
+
 static struct gpio_backlight_platform_data gpio_backlight_data = {
 	.fbdev = &lcdc_device.dev,
 	.gpio = GPIO_PTR1,

@@ -378,13 +391,15 @@ static struct gpio_backlight_platform_data gpio_backlight_data = {
 	.name = "backlight",
 };
 
-static struct platform_device gpio_backlight_device = {
+static const struct platform_device_info gpio_backlight_device_info = {
 	.name = "gpio-backlight",
-	.dev = {
-		.platform_data = &gpio_backlight_data,
-	},
+	.data = &gpio_backlight_data,
+	.size_data = sizeof(gpio_backlight_data),
+	.properties = gpio_backlight_props,
 };
 
+static struct platform_device *gpio_backlight_device;
+
 /* CEU0 */
 static struct ceu_platform_data ceu0_pdata = {
 	.num_subdevs			= 2,

@@ -1006,7 +1021,6 @@ static struct platform_device *ecovec_devices[] __initdata = {
 	&usb1_common_device,
 	&usbhs_device,
 	&lcdc_device,
-	&gpio_backlight_device,
 	&keysc_device,
 	&cn12_power,
 #if defined(CONFIG_MMC_SDHI) || defined(CONFIG_MMC_SDHI_MODULE)

@@ -1464,6 +1478,12 @@ static int __init arch_setup(void)
 #endif
 #endif
 
+	gpiod_add_lookup_table(&gpio_backlight_lookup);
+	gpio_backlight_device = platform_device_register_full(
+					&gpio_backlight_device_info);
+	if (IS_ERR(gpio_backlight_device))
+		return PTR_ERR(gpio_backlight_device);
+
 	return platform_add_devices(ecovec_devices,
 				    ARRAY_SIZE(ecovec_devices));
 }

-- 
2.21.0

From: Bartosz Golaszewski <redacted>

Now that the last user of platform data (sh ecovec24) defines a proper
GPIO lookup and sets the 'default-on' device property, we can drop the
platform_data-specific GPIO handling and unify a big chunk of code.

The only field used from the platform data is now the fbdev pointer.

Signed-off-by: Bartosz Golaszewski <redacted>
---
 drivers/video/backlight/gpio_backlight.c | 46 +++++++-----------------
 1 file changed, 13 insertions(+), 33 deletions(-)

diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
index 89e10bccfd3c..b20d2d5d5190 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c

@@ -71,41 +71,21 @@ static int gpio_backlight_probe(struct platform_device *pdev)
 
 	gbl->dev = dev;
 
-	if (pdata) {
-		/*
-		 * Legacy platform data GPIO retrieveal. Do not expand
-		 * the use of this code path, currently only used by one
-		 * SH board.
-		 */
-		unsigned long flags = GPIOF_DIR_OUT;
-
+	if (pdata)
 		gbl->fbdev = pdata->fbdev;
-		gbl->def_value = pdata->def_value;
-		flags |= gbl->def_value ? GPIOF_INIT_HIGH : GPIOF_INIT_LOW;
-
-		ret = devm_gpio_request_one(gbl->dev, pdata->gpio, flags,
-					    pdata ? pdata->name : "backlight");
-		if (ret < 0) {
-			dev_err(dev, "unable to request GPIO\n");
-			return ret;
-		}
-		gbl->gpiod = gpio_to_desc(pdata->gpio);
-		if (!gbl->gpiod)
-			return -EINVAL;
-	} else {
-		gbl->def_value = device_property_read_bool(dev, "default-on");
-		flags = gbl->def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
-
-		gbl->gpiod = devm_gpiod_get(dev, NULL, flags);
-		if (IS_ERR(gbl->gpiod)) {
-			ret = PTR_ERR(gbl->gpiod);
-
-			if (ret != -EPROBE_DEFER) {
-				dev_err(dev,
-					"Error: The gpios parameter is missing or invalid.\n");
-			}
-			return ret;
+
+	gbl->def_value = device_property_read_bool(dev, "default-on");
+	flags = gbl->def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW;
+
+	gbl->gpiod = devm_gpiod_get(dev, NULL, flags);
+	if (IS_ERR(gbl->gpiod)) {
+		ret = PTR_ERR(gbl->gpiod);
+
+		if (ret != -EPROBE_DEFER) {
+			dev_err(dev,
+				"Error: The gpios parameter is missing or invalid.\n");
 		}
+		return ret;
 	}
 
 	memset(&props, 0, sizeof(props));

-- 
2.21.0

On Fri, Jun 28, 2019 at 11:03 AM Bartosz Golaszewski [off-list ref] wrote:

From: Bartosz Golaszewski <redacted>

While working on my other series related to gpio-backlight[1] I noticed
that we could simplify the driver if we made the only user of platform
data use GPIO lookups and device properties. This series tries to do
that.

The first patch sets up all the required structures in the board file,
the second modifies the backlight driver, the third and fourth remove
the leftovers.

This series depends on the three first patches from [1].

I don't have access to this HW but hopefully this works. Only compile
tested.

This series:
Reviewed-by: Linus Walleij <redacted>

Excellent work!

Yours,
Linus Walleij

On Fri, Jun 28, 2019 at 11:15:10AM +0100, Linus Walleij wrote:

On Fri, Jun 28, 2019 at 11:03 AM Bartosz Golaszewski [off-list ref] wrote:

quoted

From: Bartosz Golaszewski <redacted>

While working on my other series related to gpio-backlight[1] I noticed
that we could simplify the driver if we made the only user of platform
data use GPIO lookups and device properties. This series tries to do
that.

The first patch sets up all the required structures in the board file,
the second modifies the backlight driver, the third and fourth remove
the leftovers.

This series depends on the three first patches from [1].

I don't have access to this HW but hopefully this works. Only compile
tested.

This series:
Reviewed-by: Linus Walleij <redacted>

Excellent work!

Ditto!

Hope to see this come around again alongside the other GPIO clean ups.


Daniel.