From: Bartosz Golaszewski bgolaszewski@baylibre.com
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.
First two patches contain minor fixes. Third patch makes the driver explicitly drive the GPIO line. Fourth patch adds all necessary data structures to ecovec24. Patch 5/9 unifies much of the code for both pdata and non-pdata cases. Patches 6-7/9 remove unused platform data fields. Last two patches contain additional improvements for the GPIO backlight driver while we're already modifying it.
I don't have access to this HW but hopefully this works. Only compile tested.
[1] https://lkml.org/lkml/2019/6/25/900
v1 -> v2: - rebased on top of v5.3-rc1 and adjusted to the recent changes from Andy - added additional two patches with minor improvements
v2 -> v3: - in patch 7/7: used initializers to set values for pdata and dev local vars
v3 -> v4: - rebased on top of v5.4-rc1 - removed changes that are no longer relevant after commit ec665b756e6f ("backlight: gpio-backlight: Correct initial power state handling") - added patch 7/7
v4 -> v5: - in patch 7/7: added a comment replacing the name of the function being pulled into probe()
v5 -> v6: - added a patch making the driver explicitly set the direction of the GPIO to output - added a patch removing a redundant newline
Bartosz Golaszewski (9): backlight: gpio: remove unneeded include backlight: gpio: remove stray newline backlight: gpio: explicitly set the direction of the GPIO 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 backlight: gpio: use a helper variable for &pdev->dev backlight: gpio: pull gpio_backlight_initial_power_state() into probe
arch/sh/boards/mach-ecovec24/setup.c | 33 +++-- drivers/video/backlight/gpio_backlight.c | 128 +++++++------------ include/linux/platform_data/gpio_backlight.h | 3 - 3 files changed, 69 insertions(+), 95 deletions(-)
From: Bartosz Golaszewski bgolaszewski@baylibre.com
We no longer use any symbols from of_gpio.h. Remove this include.
Signed-off-by: Bartosz Golaszewski bgolaszewski@baylibre.com Reviewed-by: Linus Walleij linus.walleij@linaro.org Reviewed-by: Daniel Thompson daniel.thompson@linaro.org Reviewed-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- drivers/video/backlight/gpio_backlight.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 18e053e4716c..7e1990199fae 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -12,7 +12,6 @@ #include <linux/kernel.h> #include <linux/module.h> #include <linux/of.h> -#include <linux/of_gpio.h> #include <linux/platform_data/gpio_backlight.h> #include <linux/platform_device.h> #include <linux/property.h>
From: Bartosz Golaszewski bgolaszewski@baylibre.com
Remove a double newline from the driver.
Signed-off-by: Bartosz Golaszewski bgolaszewski@baylibre.com --- drivers/video/backlight/gpio_backlight.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 7e1990199fae..3955b513f2f8 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -91,7 +91,6 @@ static int gpio_backlight_initial_power_state(struct gpio_backlight *gbl) return FB_BLANK_UNBLANK; }
- static int gpio_backlight_probe(struct platform_device *pdev) { struct gpio_backlight_platform_data *pdata =
On Sat, Oct 19, 2019 at 12:58 PM Bartosz Golaszewski brgl@bgdev.pl wrote:
From: Bartosz Golaszewski bgolaszewski@baylibre.com
Remove a double newline from the driver.
Reviewed-by: Andy Shevchenko andy.shevchenko@gmail.com
Signed-off-by: Bartosz Golaszewski bgolaszewski@baylibre.com
drivers/video/backlight/gpio_backlight.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 7e1990199fae..3955b513f2f8 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -91,7 +91,6 @@ static int gpio_backlight_initial_power_state(struct gpio_backlight *gbl) return FB_BLANK_UNBLANK; }
static int gpio_backlight_probe(struct platform_device *pdev) { struct gpio_backlight_platform_data *pdata = -- 2.23.0
On Sat, Oct 19, 2019 at 10:35:49AM +0200, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski bgolaszewski@baylibre.com
Remove a double newline from the driver.
Signed-off-by: Bartosz Golaszewski bgolaszewski@baylibre.com
Reviewed-by: Daniel Thompson daniel.thompson@linaro.org
(wow! that one was easy ;-) )
drivers/video/backlight/gpio_backlight.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 7e1990199fae..3955b513f2f8 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -91,7 +91,6 @@ static int gpio_backlight_initial_power_state(struct gpio_backlight *gbl) return FB_BLANK_UNBLANK; }
static int gpio_backlight_probe(struct platform_device *pdev) { struct gpio_backlight_platform_data *pdata = -- 2.23.0
From: Bartosz Golaszewski bgolaszewski@baylibre.com
The GPIO backlight driver currently requests the line 'as is', without acively setting its direction. This can lead to problems: if the line is in input mode by default, we won't be able to drive it later when updating the status and also reading its initial value doesn't make sense for backlight setting.
Request the line 'as is' initially, so that we can read its value without affecting it but then change the direction to output explicitly when setting the initial brightness.
Also: check the current direction and only read the value if it's output.
Signed-off-by: Bartosz Golaszewski bgolaszewski@baylibre.com --- drivers/video/backlight/gpio_backlight.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 3955b513f2f8..a36ac3a45b81 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -25,9 +25,8 @@ struct gpio_backlight { int def_value; };
-static int gpio_backlight_update_status(struct backlight_device *bl) +static int gpio_backlight_get_curr_brightness(struct backlight_device *bl) { - struct gpio_backlight *gbl = bl_get_data(bl); int brightness = bl->props.brightness;
if (bl->props.power != FB_BLANK_UNBLANK || @@ -35,6 +34,14 @@ static int gpio_backlight_update_status(struct backlight_device *bl) bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK)) brightness = 0;
+ return brightness; +} + +static int gpio_backlight_update_status(struct backlight_device *bl) +{ + struct gpio_backlight *gbl = bl_get_data(bl); + int brightness = gpio_backlight_get_curr_brightness(bl); + gpiod_set_value_cansleep(gbl->gpiod, brightness);
return 0; @@ -85,7 +92,8 @@ static int gpio_backlight_initial_power_state(struct gpio_backlight *gbl) return gbl->def_value ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN;
/* if the enable GPIO is disabled, do not enable the backlight */ - if (gpiod_get_value_cansleep(gbl->gpiod) == 0) + if (gpiod_get_direction(gbl->gpiod) == 0 && + gpiod_get_value_cansleep(gbl->gpiod) == 0) return FB_BLANK_POWERDOWN;
return FB_BLANK_UNBLANK; @@ -98,7 +106,7 @@ static int gpio_backlight_probe(struct platform_device *pdev) struct backlight_properties props; struct backlight_device *bl; struct gpio_backlight *gbl; - int ret; + int ret, init_brightness;
gbl = devm_kzalloc(&pdev->dev, sizeof(*gbl), GFP_KERNEL); if (gbl == NULL) @@ -151,7 +159,12 @@ static int gpio_backlight_probe(struct platform_device *pdev) bl->props.power = gpio_backlight_initial_power_state(gbl); bl->props.brightness = 1;
- backlight_update_status(bl); + init_brightness = gpio_backlight_get_curr_brightness(bl); + ret = gpiod_direction_output(gbl->gpiod, init_brightness); + if (ret) { + dev_err(&pdev->dev, "failed to set initial brightness\n"); + return ret; + }
platform_set_drvdata(pdev, bl); return 0;
On Sat, Oct 19, 2019 at 10:35:50AM +0200, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski bgolaszewski@baylibre.com
The GPIO backlight driver currently requests the line 'as is', without acively setting its direction. This can lead to problems: if the line is in input mode by default, we won't be able to drive it later when updating the status and also reading its initial value doesn't make sense for backlight setting.
Request the line 'as is' initially, so that we can read its value without affecting it but then change the direction to output explicitly when setting the initial brightness.
Also: check the current direction and only read the value if it's output.
Signed-off-by: Bartosz Golaszewski bgolaszewski@baylibre.com
Intent looks good to me but...
drivers/video/backlight/gpio_backlight.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 3955b513f2f8..a36ac3a45b81 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -25,9 +25,8 @@ struct gpio_backlight { int def_value; };
-static int gpio_backlight_update_status(struct backlight_device *bl) +static int gpio_backlight_get_curr_brightness(struct backlight_device *bl)
This function does not get the current brightness (e.g. what the hardware is currently doing). Given we've just nuked the function that *did* get the current brightness from the hardware this isn't an acceptable name.
Would like something like calc_brightness() or get_next_brightness().
Daniel.
pon., 21 paź 2019 o 12:45 Daniel Thompson daniel.thompson@linaro.org napisał(a):
On Sat, Oct 19, 2019 at 10:35:50AM +0200, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski bgolaszewski@baylibre.com
The GPIO backlight driver currently requests the line 'as is', without acively setting its direction. This can lead to problems: if the line is in input mode by default, we won't be able to drive it later when updating the status and also reading its initial value doesn't make sense for backlight setting.
Request the line 'as is' initially, so that we can read its value without affecting it but then change the direction to output explicitly when setting the initial brightness.
Also: check the current direction and only read the value if it's output.
Signed-off-by: Bartosz Golaszewski bgolaszewski@baylibre.com
Intent looks good to me but...
drivers/video/backlight/gpio_backlight.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 3955b513f2f8..a36ac3a45b81 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -25,9 +25,8 @@ struct gpio_backlight { int def_value; };
-static int gpio_backlight_update_status(struct backlight_device *bl) +static int gpio_backlight_get_curr_brightness(struct backlight_device *bl)
This function does not get the current brightness (e.g. what the hardware is currently doing). Given we've just nuked the function that *did* get the current brightness from the hardware this isn't an acceptable name.
Would like something like calc_brightness() or get_next_brightness().
Fair enough, the latter sounds good in this case.
Bart
From: Bartosz Golaszewski bgolaszewski@baylibre.com
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 bgolaszewski@baylibre.com Reviewed-by: Andy Shevchenko andriy.shevchenko@linux.intel.com Reviewed-by: Linus Walleij linus.walleij@linaro.org --- 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 acaa97459531..aaa8ea62636f 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) @@ -1462,6 +1476,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)); }
From: Bartosz Golaszewski bgolaszewski@baylibre.com
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 bgolaszewski@baylibre.com Reviewed-by: Linus Walleij linus.walleij@linaro.org Reviewed-by: Daniel Thompson daniel.thompson@linaro.org Reviewed-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- drivers/video/backlight/gpio_backlight.c | 62 +++++------------------- 1 file changed, 11 insertions(+), 51 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index a36ac3a45b81..c829b82f5372 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -6,7 +6,6 @@ #include <linux/backlight.h> #include <linux/err.h> #include <linux/fb.h> -#include <linux/gpio.h> /* Only for legacy support */ #include <linux/gpio/consumer.h> #include <linux/init.h> #include <linux/kernel.h> @@ -61,28 +60,6 @@ static const struct backlight_ops gpio_backlight_ops = { .check_fb = gpio_backlight_check_fb, };
-static int gpio_backlight_probe_dt(struct platform_device *pdev, - struct gpio_backlight *gbl) -{ - struct device *dev = &pdev->dev; - int ret; - - gbl->def_value = device_property_read_bool(dev, "default-on"); - - gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS); - 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; - } - - return 0; -} - static int gpio_backlight_initial_power_state(struct gpio_backlight *gbl) { struct device_node *node = gbl->dev->of_node; @@ -114,35 +91,18 @@ static int gpio_backlight_probe(struct platform_device *pdev)
gbl->dev = &pdev->dev;
- if (pdev->dev.fwnode) { - ret = gpio_backlight_probe_dt(pdev, gbl); - if (ret) - return ret; - } else 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(&pdev->dev, "unable to request GPIO\n"); - return ret; - } - gbl->gpiod = gpio_to_desc(pdata->gpio); - if (!gbl->gpiod) - return -EINVAL; - } else { - dev_err(&pdev->dev, - "failed to find platform data or device tree node.\n"); - return -ENODEV; + + gbl->def_value = device_property_read_bool(&pdev->dev, "default-on"); + + gbl->gpiod = devm_gpiod_get(&pdev->dev, NULL, GPIOD_ASIS); + if (IS_ERR(gbl->gpiod)) { + ret = PTR_ERR(gbl->gpiod); + if (ret != -EPROBE_DEFER) + dev_err(&pdev->dev, + "Error: The gpios parameter is missing or invalid.\n"); + return ret; }
memset(&props, 0, sizeof(props));
From: Bartosz Golaszewski bgolaszewski@baylibre.com
Platform data fields other than fbdev are no longer used by the backlight driver. Remove them.
Signed-off-by: Bartosz Golaszewski bgolaszewski@baylibre.com Reviewed-by: Andy Shevchenko andriy.shevchenko@linux.intel.com Reviewed-by: Linus Walleij linus.walleij@linaro.org --- 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 aaa8ea62636f..dd427bac5cde 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 = {
From: Bartosz Golaszewski bgolaszewski@baylibre.com
Remove the platform data fields that nobody uses.
Signed-off-by: Bartosz Golaszewski bgolaszewski@baylibre.com Reviewed-by: Andy Shevchenko andriy.shevchenko@linux.intel.com Reviewed-by: Linus Walleij linus.walleij@linaro.org Reviewed-by: Daniel Thompson daniel.thompson@linaro.org --- 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
From: Bartosz Golaszewski bgolaszewski@baylibre.com
Instead of dereferencing pdev each time, use a helper variable for the associated device pointer.
Signed-off-by: Bartosz Golaszewski bgolaszewski@baylibre.com Reviewed-by: Linus Walleij linus.walleij@linaro.org Reviewed-by: Daniel Thompson daniel.thompson@linaro.org Reviewed-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- drivers/video/backlight/gpio_backlight.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index c829b82f5372..7b411f6ee15a 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -78,29 +78,29 @@ static int gpio_backlight_initial_power_state(struct gpio_backlight *gbl)
static int gpio_backlight_probe(struct platform_device *pdev) { - struct gpio_backlight_platform_data *pdata = - dev_get_platdata(&pdev->dev); + struct device *dev = &pdev->dev; + struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev); struct backlight_properties props; struct backlight_device *bl; struct gpio_backlight *gbl; int ret, init_brightness;
- gbl = devm_kzalloc(&pdev->dev, sizeof(*gbl), GFP_KERNEL); + gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL); if (gbl == NULL) return -ENOMEM;
- gbl->dev = &pdev->dev; + gbl->dev = dev;
if (pdata) gbl->fbdev = pdata->fbdev;
- gbl->def_value = device_property_read_bool(&pdev->dev, "default-on"); + gbl->def_value = device_property_read_bool(dev, "default-on");
- gbl->gpiod = devm_gpiod_get(&pdev->dev, NULL, GPIOD_ASIS); + gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS); if (IS_ERR(gbl->gpiod)) { ret = PTR_ERR(gbl->gpiod); if (ret != -EPROBE_DEFER) - dev_err(&pdev->dev, + dev_err(dev, "Error: The gpios parameter is missing or invalid.\n"); return ret; } @@ -108,11 +108,10 @@ static int gpio_backlight_probe(struct platform_device *pdev) memset(&props, 0, sizeof(props)); props.type = BACKLIGHT_RAW; props.max_brightness = 1; - bl = devm_backlight_device_register(&pdev->dev, dev_name(&pdev->dev), - &pdev->dev, gbl, &gpio_backlight_ops, - &props); + bl = devm_backlight_device_register(dev, dev_name(dev), dev, gbl, + &gpio_backlight_ops, &props); if (IS_ERR(bl)) { - dev_err(&pdev->dev, "failed to register backlight\n"); + dev_err(dev, "failed to register backlight\n"); return PTR_ERR(bl); }
@@ -122,7 +121,7 @@ static int gpio_backlight_probe(struct platform_device *pdev) init_brightness = gpio_backlight_get_curr_brightness(bl); ret = gpiod_direction_output(gbl->gpiod, init_brightness); if (ret) { - dev_err(&pdev->dev, "failed to set initial brightness\n"); + dev_err(dev, "failed to set initial brightness\n"); return ret; }
From: Bartosz Golaszewski bgolaszewski@baylibre.com
The probe function in the gpio-backlight driver is quite short. If we pull gpio_backlight_initial_power_state() into probe we can drop two more fields from struct gpio_backlight and shrink the driver code.
Signed-off-by: Bartosz Golaszewski bgolaszewski@baylibre.com Acked-by: Daniel Thompson daniel.thompson@linaro.org --- drivers/video/backlight/gpio_backlight.c | 38 +++++++++--------------- 1 file changed, 14 insertions(+), 24 deletions(-)
diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 7b411f6ee15a..4336db6bf6b5 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -17,11 +17,8 @@ #include <linux/slab.h>
struct gpio_backlight { - struct device *dev; struct device *fbdev; - struct gpio_desc *gpiod; - int def_value; };
static int gpio_backlight_get_curr_brightness(struct backlight_device *bl) @@ -60,41 +57,24 @@ static const struct backlight_ops gpio_backlight_ops = { .check_fb = gpio_backlight_check_fb, };
-static int gpio_backlight_initial_power_state(struct gpio_backlight *gbl) -{ - struct device_node *node = gbl->dev->of_node; - - /* Not booted with device tree or no phandle link to the node */ - if (!node || !node->phandle) - return gbl->def_value ? FB_BLANK_UNBLANK : FB_BLANK_POWERDOWN; - - /* if the enable GPIO is disabled, do not enable the backlight */ - if (gpiod_get_direction(gbl->gpiod) == 0 && - gpiod_get_value_cansleep(gbl->gpiod) == 0) - return FB_BLANK_POWERDOWN; - - return FB_BLANK_UNBLANK; -} - static int gpio_backlight_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct gpio_backlight_platform_data *pdata = dev_get_platdata(dev); + struct device_node *of_node = dev->of_node; struct backlight_properties props; struct backlight_device *bl; struct gpio_backlight *gbl; - int ret, init_brightness; + int ret, init_brightness, def_value;
gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL); if (gbl == NULL) return -ENOMEM;
- gbl->dev = dev; - if (pdata) gbl->fbdev = pdata->fbdev;
- gbl->def_value = device_property_read_bool(dev, "default-on"); + def_value = device_property_read_bool(dev, "default-on");
gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS); if (IS_ERR(gbl->gpiod)) { @@ -115,7 +95,17 @@ static int gpio_backlight_probe(struct platform_device *pdev) return PTR_ERR(bl); }
- bl->props.power = gpio_backlight_initial_power_state(gbl); + /* Set the initial power state */ + if (!of_node || !of_node->phandle) + /* Not booted with device tree or no phandle link to the node */ + bl->props.power = def_value ? FB_BLANK_UNBLANK + : FB_BLANK_POWERDOWN; + else if (gpiod_get_direction(gbl->gpiod) == 0 && + gpiod_get_value_cansleep(gbl->gpiod) == 0) + bl->props.power = FB_BLANK_POWERDOWN; + else + bl->props.power = FB_BLANK_UNBLANK; + bl->props.brightness = 1;
init_brightness = gpio_backlight_get_curr_brightness(bl);
dri-devel@lists.freedesktop.org
-
Andy Shevchenko -
Bartosz Golaszewski -
Daniel Thompson