drivers: fbtft: Refactor backlight logic

Commit Message

qianfan Feb. 12, 2022, 4:27 a.m. UTC

From: qianfan Zhao <[e-mail]>

Control led gpios by using GPIO_ACTIVE flags, don't detect the polarity
by reading the gpio value when probe.

Signed-off-by: qianfan Zhao <[e-mail]>
---
 drivers/staging/fbtft/fbtft-core.c | 54 +++++++++++++++++-------------
 drivers/staging/fbtft/fbtft.h      |  1 -
 2 files changed, 30 insertions(+), 25 deletions(-)

Comments

Andy Shevchenko Feb. 14, 2022, 10:01 a.m. UTC | #1

On Sat, Feb 12, 2022 at 12:27:39PM +0800, [e-mail] wrote:
> From: qianfan Zhao <[e-mail]>
> 
> Control led gpios by using GPIO_ACTIVE flags, don't detect the polarity
> by reading the gpio value when probe.

...

> +	enum gpiod_flags flags = GPIOD_OUT_HIGH;
> +
> +	/* request and turn on backlight */
> +	return fbtft_request_one_gpio_with_flags(par, name, index, flags, gpiop);

But taking into consideration all possible polarities this is wrong
assumption. Depending on the flags in DT or ACPI this may give an
opposite effect.

P.S. Please, Cc all your fbtft patches to fbdev maintainer as well.

Patch

diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index ed992ca605eb..37548848af99 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -70,14 +70,14 @@  void fbtft_dbg_hex(const struct device *dev, int groupsize,
 }
 EXPORT_SYMBOL(fbtft_dbg_hex);
 
-static int fbtft_request_one_gpio(struct fbtft_par *par,
-				  const char *name, int index,
-				  struct gpio_desc **gpiop)
+static int fbtft_request_one_gpio_with_flags(struct fbtft_par *par,
+					     const char *name, int index,
+					     enum gpiod_flags flags,
+					     struct gpio_desc **gpiop)
 {
 	struct device *dev = par->info->device;
 
-	*gpiop = devm_gpiod_get_index_optional(dev, name, index,
-					       GPIOD_OUT_LOW);
+	*gpiop = devm_gpiod_get_index_optional(dev, name, index, flags);
 	if (IS_ERR(*gpiop))
 		return dev_err_probe(dev, PTR_ERR(*gpiop), "Failed to request %s GPIO\n", name);
 
@@ -87,6 +87,25 @@  static int fbtft_request_one_gpio(struct fbtft_par *par,
 	return 0;
 }
 
+static int fbtft_request_one_gpio(struct fbtft_par *par,
+				  const char *name, int index,
+				  struct gpio_desc **gpiop)
+{
+	enum gpiod_flags flags = GPIOD_OUT_LOW;
+
+	return fbtft_request_one_gpio_with_flags(par, name, index, flags, gpiop);
+}
+
+static int fbtft_request_led_gpio(struct fbtft_par *par,
+				  const char *name, int index,
+				  struct gpio_desc **gpiop)
+{
+	enum gpiod_flags flags = GPIOD_OUT_HIGH;
+
+	/* request and turn on backlight */
+	return fbtft_request_one_gpio_with_flags(par, name, index, flags, gpiop);
+}
+
 static int fbtft_request_gpios(struct fbtft_par *par)
 {
 	int i;
@@ -115,7 +134,7 @@  static int fbtft_request_gpios(struct fbtft_par *par)
 					     &par->gpio.db[i]);
 		if (ret)
 			return ret;
-		ret = fbtft_request_one_gpio(par, "led", i,
+		ret = fbtft_request_led_gpio(par, "led", i,
 					     &par->gpio.led[i]);
 		if (ret)
 			return ret;
@@ -132,17 +151,16 @@  static int fbtft_request_gpios(struct fbtft_par *par)
 static int fbtft_backlight_update_status(struct backlight_device *bd)
 {
 	struct fbtft_par *par = bl_get_data(bd);
-	bool polarity = par->polarity;
 
 	fbtft_par_dbg(DEBUG_BACKLIGHT, par,
-		      "%s: polarity=%d, power=%d, fb_blank=%d\n",
-		      __func__, polarity, bd->props.power, bd->props.fb_blank);
+		      "%s: power=%d, fb_blank=%d\n",
+		      __func__, bd->props.power, bd->props.fb_blank);
 
 	if ((bd->props.power == FB_BLANK_UNBLANK) &&
 	    (bd->props.fb_blank == FB_BLANK_UNBLANK))
-		gpiod_set_value(par->gpio.led[0], polarity);
+		gpiod_set_value(par->gpio.led[0], 1);
 	else
-		gpiod_set_value(par->gpio.led[0], !polarity);
+		gpiod_set_value(par->gpio.led[0], 0);
 
 	return 0;
 }
@@ -179,11 +197,7 @@  void fbtft_register_backlight(struct fbtft_par *par)
 	}
 
 	bl_props.type = BACKLIGHT_RAW;
-	/* Assume backlight is off, get polarity from current state of pin */
-	bl_props.power = FB_BLANK_POWERDOWN;
-	if (!gpiod_get_value(par->gpio.led[0]))
-		par->polarity = true;
-
+	bl_props.power = FB_BLANK_UNBLANK;
 	bd = backlight_device_register(dev_driver_string(par->info->device),
 				       par->info->device, par,
 				       &fbtft_bl_ops, &bl_props);
@@ -853,14 +867,6 @@  int fbtft_register_framebuffer(struct fb_info *fb_info)
 		 fb_info->fix.smem_len >> 10, text1,
 		 HZ / fb_info->fbdefio->delay, text2);
 
-#ifdef CONFIG_FB_BACKLIGHT
-	/* Turn on backlight if available */
-	if (fb_info->bl_dev) {
-		fb_info->bl_dev->props.power = FB_BLANK_UNBLANK;
-		fb_info->bl_dev->ops->update_status(fb_info->bl_dev);
-	}
-#endif
-
 	return 0;
 
 reg_fail:
diff --git a/drivers/staging/fbtft/fbtft.h b/drivers/staging/fbtft/fbtft.h
index 76f8c090a837..6751c129df4d 100644
--- a/drivers/staging/fbtft/fbtft.h
+++ b/drivers/staging/fbtft/fbtft.h
@@ -228,7 +228,6 @@  struct fbtft_par {
 	ktime_t update_time;
 	bool bgr;
 	void *extra;
-	bool polarity;
 };
 
 #define NUMARGS(...)  (sizeof((int[]){__VA_ARGS__}) / sizeof(int))