From 06168a64b1ae346816fcd0a0c3ea5276c778408b Mon Sep 17 00:00:00 2001 From: Bhushan Shah Date: Fri, 23 Jun 2017 09:24:33 +0530 Subject: [PATCH 1/4] backlight: lm3630a: Bump REG_MAX value to 0x50 instead of 0x1F In the lm3630a_chip_init we try to write to 0x50 register, which is higher value then the max_register value, this resulted in regmap_write return -EIO. Fix this by bumping REG_MAX value to 0x50. This code was introduced with the chip revision in commit 28e64a68a2ef, however setting filter strength was failing silently because it used unsigned int for storing and comparing the return values. Bug related to signedness was fixed in 2a0c316bf3cc, which made it error out correctly instead of failing silently. I found this issue by using this driver on LGE Nexus 5 (hammerhead). After this commit lm3630a_chip_init succeeds instead of failing with -EIO. Fixes: 28e64a68a2ef ("backlight: lm3630: apply chip revision") Fixes: 2a0c316bf3cc ("drivers/video/backlight/lm3630a_bl.c: fix signedness bug in lm3630a_chip_init()") Suggested-by: Bjorn Andersson Signed-off-by: Bhushan Shah Acked-by: Daniel Thompson Signed-off-by: Lee Jones --- drivers/video/backlight/lm3630a_bl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c index 60d6c2ac87aa..2030a6b77a09 100644 --- a/drivers/video/backlight/lm3630a_bl.c +++ b/drivers/video/backlight/lm3630a_bl.c @@ -31,7 +31,8 @@ #define REG_FAULT 0x0B #define REG_PWM_OUTLOW 0x12 #define REG_PWM_OUTHIGH 0x13 -#define REG_MAX 0x1F +#define REG_FILTER_STRENGTH 0x50 +#define REG_MAX 0x50 #define INT_DEBOUNCE_MSEC 10 struct lm3630a_chip { @@ -80,7 +81,7 @@ static int lm3630a_chip_init(struct lm3630a_chip *pchip) usleep_range(1000, 2000); /* set Filter Strength Register */ - rval = lm3630a_write(pchip, 0x50, 0x03); + rval = lm3630a_write(pchip, REG_FILTER_STRENGTH, 0x03); /* set Cofig. register */ rval |= lm3630a_update(pchip, REG_CONFIG, 0x07, pdata->pwm_ctrl); /* set boost control */ From 62cdfe658535c39003d4d1b9d7f9b5a23a0db6bf Mon Sep 17 00:00:00 2001 From: Arvind Yadav Date: Tue, 20 Jun 2017 13:22:15 +0530 Subject: [PATCH 2/4] backlight: pwm_bl: Make of_device_ids const of_device_ids are not supposed to change at runtime. All functions working with of_device_ids provided by work with const of_device_ids. So mark the non-const structs as const. Signed-off-by: Arvind Yadav Acked-by: Thierry Reding Acked-by: Daniel Thompson Reviewed-by: Laurent Pinchart Signed-off-by: Lee Jones --- drivers/video/backlight/pwm_bl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index 002f1ce22bd0..9bd17682655a 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -178,7 +178,7 @@ static int pwm_backlight_parse_dt(struct device *dev, return 0; } -static struct of_device_id pwm_backlight_of_match[] = { +static const struct of_device_id pwm_backlight_of_match[] = { { .compatible = "pwm-backlight" }, { } }; From de7389003ab77fc63296b2adee4cd9c48dc41d5a Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Tue, 30 May 2017 13:48:21 +0200 Subject: [PATCH 3/4] backlight: gpio_backlight: Convert to use GPIO descriptor This driver is predominantly used by device tree systems, all of which can deal with modern GPIO descriptors. The legacy GPIO API is only used by one SH board so make the GPIO descriptor the default way to deal with it. As an intended side effect we do not need to look around in the device tree for the inversion flag since the GPIO descriptors will intrinsically deal with this. Signed-off-by: Linus Walleij Acked-by: Daniel Thompson Reviewed-by: Laurent Pinchart Signed-off-by: Lee Jones --- drivers/video/backlight/gpio_backlight.c | 75 ++++++++++++++---------- 1 file changed, 45 insertions(+), 30 deletions(-) diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 18134416b154..5ffaff1e4142 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -9,7 +9,8 @@ #include #include #include -#include +#include /* Only for legacy support */ +#include #include #include #include @@ -23,7 +24,7 @@ struct gpio_backlight { struct device *dev; struct device *fbdev; - int gpio; + struct gpio_desc *gpiod; int active; int def_value; }; @@ -38,8 +39,8 @@ static int gpio_backlight_update_status(struct backlight_device *bl) bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK)) brightness = 0; - gpio_set_value_cansleep(gbl->gpio, - brightness ? gbl->active : !gbl->active); + gpiod_set_value_cansleep(gbl->gpiod, + brightness ? gbl->active : !gbl->active); return 0; } @@ -61,22 +62,26 @@ static const struct backlight_ops gpio_backlight_ops = { static int gpio_backlight_probe_dt(struct platform_device *pdev, struct gpio_backlight *gbl) { - struct device_node *np = pdev->dev.of_node; - enum of_gpio_flags gpio_flags; - - gbl->gpio = of_get_gpio_flags(np, 0, &gpio_flags); - - if (!gpio_is_valid(gbl->gpio)) { - if (gbl->gpio != -EPROBE_DEFER) { - dev_err(&pdev->dev, - "Error: The gpios parameter is missing or invalid.\n"); - } - return gbl->gpio; - } - - gbl->active = (gpio_flags & OF_GPIO_ACTIVE_LOW) ? 0 : 1; + struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node; + enum gpiod_flags flags; + int ret; gbl->def_value = of_property_read_bool(np, "default-on"); + flags = gbl->def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW; + /* GPIO descriptors keep track of inversion */ + gbl->active = 1; + + 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; + } return 0; } @@ -89,7 +94,6 @@ static int gpio_backlight_probe(struct platform_device *pdev) struct backlight_device *bl; struct gpio_backlight *gbl; struct device_node *np = pdev->dev.of_node; - unsigned long flags = GPIOF_DIR_OUT; int ret; if (!pdata && !np) { @@ -109,22 +113,33 @@ static int gpio_backlight_probe(struct platform_device *pdev) if (ret) return ret; } else { + /* + * 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; + gbl->fbdev = pdata->fbdev; - gbl->gpio = pdata->gpio; gbl->active = pdata->active_low ? 0 : 1; gbl->def_value = pdata->def_value; - } - if (gbl->active) - flags |= gbl->def_value ? GPIOF_INIT_HIGH : GPIOF_INIT_LOW; - else - flags |= gbl->def_value ? GPIOF_INIT_LOW : GPIOF_INIT_HIGH; + if (gbl->active) + flags |= gbl->def_value ? + GPIOF_INIT_HIGH : GPIOF_INIT_LOW; + else + flags |= gbl->def_value ? + GPIOF_INIT_LOW : GPIOF_INIT_HIGH; - ret = devm_gpio_request_one(gbl->dev, gbl->gpio, flags, - pdata ? pdata->name : "backlight"); - if (ret < 0) { - dev_err(&pdev->dev, "unable to request GPIO\n"); - return ret; + 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; } memset(&props, 0, sizeof(props)); From 2606706e4d7b89ebf13f35895a7dfe00e394e782 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Tue, 30 May 2017 13:48:22 +0200 Subject: [PATCH 4/4] backlight: gpio_backlight: Delete pdata inversion The option to invert the output of the GPIO (active low) is not used by the only platform still using platform data to set up a GPIO backlight (one SH board). Delete the option as we do not expect to expand the use of board files for this driver, and GPIO descriptors intrinsically keep track of any signal inversion. Signed-off-by: Linus Walleij Acked-by: Daniel Thompson Reviewed-by: Laurent Pinchart Signed-off-by: Lee Jones --- drivers/video/backlight/gpio_backlight.c | 15 ++------------- include/linux/platform_data/gpio_backlight.h | 1 - 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index 5ffaff1e4142..e470da95d806 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -25,7 +25,6 @@ struct gpio_backlight { struct device *fbdev; struct gpio_desc *gpiod; - int active; int def_value; }; @@ -39,8 +38,7 @@ static int gpio_backlight_update_status(struct backlight_device *bl) bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK)) brightness = 0; - gpiod_set_value_cansleep(gbl->gpiod, - brightness ? gbl->active : !gbl->active); + gpiod_set_value_cansleep(gbl->gpiod, brightness); return 0; } @@ -69,8 +67,6 @@ static int gpio_backlight_probe_dt(struct platform_device *pdev, gbl->def_value = of_property_read_bool(np, "default-on"); flags = gbl->def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW; - /* GPIO descriptors keep track of inversion */ - gbl->active = 1; gbl->gpiod = devm_gpiod_get(dev, NULL, flags); if (IS_ERR(gbl->gpiod)) { @@ -121,15 +117,8 @@ static int gpio_backlight_probe(struct platform_device *pdev) unsigned long flags = GPIOF_DIR_OUT; gbl->fbdev = pdata->fbdev; - gbl->active = pdata->active_low ? 0 : 1; gbl->def_value = pdata->def_value; - - if (gbl->active) - flags |= gbl->def_value ? - GPIOF_INIT_HIGH : GPIOF_INIT_LOW; - else - flags |= gbl->def_value ? - GPIOF_INIT_LOW : GPIOF_INIT_HIGH; + flags |= gbl->def_value ? GPIOF_INIT_HIGH : GPIOF_INIT_LOW; ret = devm_gpio_request_one(gbl->dev, pdata->gpio, flags, pdata ? pdata->name : "backlight"); diff --git a/include/linux/platform_data/gpio_backlight.h b/include/linux/platform_data/gpio_backlight.h index 5ae0d9c80d4d..683d90453c41 100644 --- a/include/linux/platform_data/gpio_backlight.h +++ b/include/linux/platform_data/gpio_backlight.h @@ -14,7 +14,6 @@ struct gpio_backlight_platform_data { struct device *fbdev; int gpio; int def_value; - bool active_low; const char *name; };