[PATCH v3 0/2] Add lcd driver for panels with gpio controlled panel reset

[PATCH v3 0/2] Add lcd driver for panels with gpio controlled panel reset

[Thomas Abraham]

Changes since v2:
- Reworked as per comments[1] from Mark Brown.
  - removed the setting of regulator voltage.

[1] http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg08743.html

Changes since v1:
- Fixed gpio leak, added MODULE_DEVICE_TABLE and dev_pm_ops.
- Changed device tree bindings.

This patchset adds support for passive panels that use a nRESET line which is
controlled by a GPIO of the host system and which do not use a serial comamnd
interface (i2c or spi) or memory-mapped-io interface. Platforms that do not
require any additional control apart from the nRESET line can use this driver
for lcd power control.

The first patch adds driver for passive raster-type lcd's with gpio controlled
panel reset. It includes support for both dt and non-dt platforms. The second
patch adds support for using this driver on Origen board.

Thomas Abraham (2):
  backlight: lcd: add driver for raster-type lcd's with gpio controlled panel reset
  ARM: Exynos: Use lcd power control driver for lcd panel

 .../devicetree/bindings/lcd/lcd-pwrctrl.txt        |   36 +++
 arch/arm/mach-exynos/mach-origen.c                 |   29 +--
 drivers/video/backlight/Kconfig                    |    7 +
 drivers/video/backlight/Makefile                   |    1 +
 drivers/video/backlight/lcd_pwrctrl.c              |  226 ++++++++++++++++++++
 include/video/lcd_pwrctrl.h                        |   24 ++
 6 files changed, 300 insertions(+), 23 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/lcd/lcd-pwrctrl.txt
 create mode 100644 drivers/video/backlight/lcd_pwrctrl.c
 create mode 100644 include/video/lcd_pwrctrl.h

[PATCH v3 1/2] backlight: lcd: add driver for raster-type lcd's with gpio controlled panel reset

[Thomas Abraham]

Add a lcd panel driver for simple raster-type lcd's which uses a gpio
controlled panel reset. The driver controls the nRESET line of the panel
using a gpio connected from the host system. The Vcc supply to the panel
is (optionally) controlled using a voltage regulator. This driver excludes
support for lcd panels that use a serial command interface or direct
memory mapped IO interface.

Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
 .../devicetree/bindings/lcd/lcd-pwrctrl.txt        |   36 +++
 drivers/video/backlight/Kconfig                    |    7 +
 drivers/video/backlight/Makefile                   |    1 +
 drivers/video/backlight/lcd_pwrctrl.c              |  226 ++++++++++++++++++++
 include/video/lcd_pwrctrl.h                        |   24 ++
 5 files changed, 294 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/lcd/lcd-pwrctrl.txt
 create mode 100644 drivers/video/backlight/lcd_pwrctrl.c
 create mode 100644 include/video/lcd_pwrctrl.h

diff --git a/Documentation/devicetree/bindings/lcd/lcd-pwrctrl.txt b/Documentation/devicetree/bindings/lcd/lcd-pwrctrl.txt
new file mode 100644
index 0000000..22604a2
--- /dev/null
+++ b/Documentation/devicetree/bindings/lcd/lcd-pwrctrl.txt
@@ -0,0 +1,36 @@
+* Power controller for simple lcd panels
+
+Some LCD panels provide a simple control interface for the host system. The
+control mechanism would include a nRESET line connected to a gpio of the host
+system and a Vcc supply line which the host can optionally be controlled using
+a voltage regulator. Such simple panels do not support serial command
+interface (such as i2c or spi) or memory-mapped-io interface.
+
+Required properties:
+- compatible: should be 'lcd-powercontrol'
+
+- lcd-reset-gpio: The GPIO number of the host system used to control the
+  nRESET line. The format of the gpio specifier depends on the gpio controller
+  of the host system.
+
+Optional properties:
+- lcd-reset-active-high: When the nRESET line is asserted low, the lcd panel
+  is reset and stays in reset mode as long as the nRESET line is asserted low.
+  This is the default behaviour of most lcd panels. If a lcd panel requires the
+  nRESET line to be asserted high for panel reset, then this property is used.
+  Note: Some platforms might allow inverting the polarity of the gpio output
+  in the 'lcd-reset-gpio' gpio specifier. On such platforms, if the polarity
+  is used to control the output of the gpio, then this property should not be
+  used.
+
+- vcc-lcd-supply: phandle of the regulator that controls the vcc supply to
+  the lcd panel.
+
+Example:
+
+	lcd_pwrctrl {
+		compatible = "lcd-powercontrol";
+		lcd-reset-gpio = <&gpe0 4 1 0 0>;
+		lcd-reset-active-high;
+		lcd-vcc-supply = <&regulator7>;
+	};
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 681b369..9b52ea8 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -86,6 +86,13 @@ config LCD_PLATFORM
 	  This driver provides a platform-device registered LCD power
 	  control interface.
 
+config LCD_PWRCTRL
+	tristate "LCD panel power control"
+	help
+	  Say y here, if you have a lcd panel that allows reset and vcc to be
+	  controlled by the host system, and which does not use a serial command
+	  interface (such as i2c or spi) or memory-mapped-io interface.
+
 config LCD_TOSA
 	tristate "Sharp SL-6000 LCD Driver"
 	depends on SPI && MACH_TOSA
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index af5cf65..d85c8d2 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_LCD_LMS283GF05)	   += lms283gf05.o
 obj-$(CONFIG_LCD_LTV350QV)	   += ltv350qv.o
 obj-$(CONFIG_LCD_ILI9320)	   += ili9320.o
 obj-$(CONFIG_LCD_PLATFORM)	   += platform_lcd.o
+obj-$(CONFIG_LCD_PWRCTRL)	   += lcd_pwrctrl.o
 obj-$(CONFIG_LCD_VGG2432A4)	   += vgg2432a4.o
 obj-$(CONFIG_LCD_TDO24M)	   += tdo24m.o
 obj-$(CONFIG_LCD_TOSA)		   += tosa_lcd.o
diff --git a/drivers/video/backlight/lcd_pwrctrl.c b/drivers/video/backlight/lcd_pwrctrl.c
new file mode 100644
index 0000000..917d842
--- /dev/null
+++ b/drivers/video/backlight/lcd_pwrctrl.c
@@ -0,0 +1,226 @@
+/*
+ * Simple lcd panel power control driver.
+ *
+ * Copyright (c) 2011-2012 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2011-2012 Linaro Ltd.
+ *
+ * This driver is for controlling power for raster type lcd panels that requires
+ * its nRESET interface line to be connected and controlled by a GPIO of the
+ * host system and the Vcc line controlled by a voltage regulator.  This
+ * excludes support for lcd panels that use a serial command interface or direct
+ * memory mapped IO interface.
+ *
+ * The nRESET interface line of the panel should be connected to a gpio of the
+ * host system. The Vcc pin is controlled using a external volatage regulator.
+ * Panel backlight is not controlled by this driver.
+ *
+ * This driver is derived from platform-lcd.c which was written by
+ * Ben Dooks <ben@simtec.co.uk>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+*/
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/fb.h>
+#include <linux/lcd.h>
+#include <linux/gpio.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/regulator/consumer.h>
+#include <video/lcd_pwrctrl.h>
+
+struct lcd_pwrctrl {
+	struct device		*dev;
+	struct lcd_device	*lcd;
+	struct lcd_pwrctrl_data	*pdata;
+	struct regulator	*regulator;
+	unsigned int		power;
+	bool			suspended;
+	bool			pwr_en;
+};
+
+static int lcd_pwrctrl_get_power(struct lcd_device *lcd)
+{
+	struct lcd_pwrctrl *lp = lcd_get_data(lcd);
+	return lp->power;
+}
+
+static int lcd_pwrctrl_set_power(struct lcd_device *lcd, int power)
+{
+	struct lcd_pwrctrl *lp = lcd_get_data(lcd);
+	struct lcd_pwrctrl_data *pd = lp->pdata;
+	bool lcd_enable;
+	int lcd_reset, ret = 0;
+
+	lcd_enable = (power = FB_BLANK_POWERDOWN || lp->suspended) ? 0 : 1;
+	lcd_reset = (pd->invert) ? !lcd_enable : lcd_enable;
+
+	if (lp->pwr_en = lcd_enable)
+		return 0;
+
+	if (!IS_ERR_OR_NULL(lp->regulator)) {
+		if (lcd_enable) {
+			if (regulator_enable(lp->regulator)) {
+				dev_info(lp->dev, "regulator enable failed\n");
+				ret = -EPERM;
+			}
+		} else {
+			if (regulator_disable(lp->regulator)) {
+				dev_info(lp->dev, "regulator disable failed\n");
+				ret = -EPERM;
+			}
+		}
+	}
+
+	gpio_direction_output(lp->pdata->gpio, lcd_reset);
+	lp->power = power;
+	lp->pwr_en = lcd_enable;
+	return ret;
+}
+
+static int lcd_pwrctrl_check_fb(struct lcd_device *lcd, struct fb_info *info)
+{
+	struct lcd_pwrctrl *lp = lcd_get_data(lcd);
+	return lp->dev->parent = info->device;
+}
+
+static struct lcd_ops lcd_pwrctrl_ops = {
+	.get_power	= lcd_pwrctrl_get_power,
+	.set_power	= lcd_pwrctrl_set_power,
+	.check_fb	= lcd_pwrctrl_check_fb,
+};
+
+#ifdef CONFIG_OF
+static void __devinit lcd_pwrctrl_parse_dt(struct device *dev,
+					struct lcd_pwrctrl_data *pdata)
+{
+	struct device_node *np = dev->of_node;
+
+	pdata->gpio = of_get_named_gpio(np, "lcd-reset-gpio", 0);
+	if (of_get_property(np, "lcd-reset-active-high", NULL))
+		pdata->invert = true;
+}
+#endif
+
+static int __devinit lcd_pwrctrl_probe(struct platform_device *pdev)
+{
+	struct lcd_pwrctrl *lp;
+	struct lcd_pwrctrl_data *pdata = pdev->dev.platform_data;
+	struct device *dev = &pdev->dev;
+	int err;
+
+#ifdef CONFIG_OF
+	if (dev->of_node) {
+		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+		if (!pdata) {
+			dev_err(dev, "memory allocation for pdata failed\n");
+			return -ENOMEM;
+		}
+		lcd_pwrctrl_parse_dt(dev, pdata);
+	}
+#endif
+
+	if (!pdata) {
+		dev_err(dev, "platform data not available\n");
+		return -EINVAL;
+	}
+
+	lp = devm_kzalloc(dev, sizeof(struct lcd_pwrctrl), GFP_KERNEL);
+	if (!lp) {
+		dev_err(dev, "memory allocation failed for private data\n");
+		return -ENOMEM;
+	}
+
+	err = gpio_request(pdata->gpio, "LCD-nRESET");
+	if (err) {
+		dev_err(dev, "gpio [%d] request failed\n", pdata->gpio);
+		return err;
+	}
+
+	/*
+	 * If power to lcd and/or lcd interface is controlled using a regulator,
+	 * get the handle to the regulator for later use during power switching.
+	 */
+	lp->regulator = devm_regulator_get(dev, "vcc-lcd");
+	if (IS_ERR(lp->regulator))
+		dev_info(dev, "could not find regulator\n");
+
+	lp->dev = dev;
+	lp->pdata = pdata;
+	lp->lcd = lcd_device_register(dev_name(dev), dev, lp, &lcd_pwrctrl_ops);
+	if (IS_ERR(lp->lcd)) {
+		dev_err(dev, "cannot register lcd device\n");
+		gpio_free(pdata->gpio);
+		return PTR_ERR(lp->lcd);
+	}
+
+	platform_set_drvdata(pdev, lp);
+	lcd_pwrctrl_set_power(lp->lcd, FB_BLANK_NORMAL);
+	return 0;
+}
+
+static int __devexit lcd_pwrctrl_remove(struct platform_device *pdev)
+{
+	struct lcd_pwrctrl *lp = platform_get_drvdata(pdev);
+	lcd_device_unregister(lp->lcd);
+	gpio_free(lp->pdata->gpio);
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int lcd_pwrctrl_suspend(struct device *dev)
+{
+	struct lcd_pwrctrl *lp = dev_get_drvdata(dev);
+
+	lp->suspended = true;
+	lcd_pwrctrl_set_power(lp->lcd, FB_BLANK_POWERDOWN);
+	return 0;
+}
+
+static int lcd_pwrctrl_resume(struct device *dev)
+{
+	struct lcd_pwrctrl *lp = dev_get_drvdata(dev);
+
+	lp->suspended = false;
+	lcd_pwrctrl_set_power(lp->lcd, FB_BLANK_UNBLANK);
+	return 0;
+}
+
+static const struct dev_pm_ops lcd_pwrctrl_dev_pm_ops = {
+	.suspend	= lcd_pwrctrl_suspend,
+	.resume		= lcd_pwrctrl_resume,
+};
+
+#define LCD_PWRCTRL_DEV_PM_OPS	(&lcd_pwrctrl_dev_pm_ops)
+#else
+#define LCD_PWRCTRL_DEV_PM_OPS	NULL
+#endif /* CONFIG_PM */
+
+#ifdef CONFIG_OF
+static const struct of_device_id lcd_pwrctrl_match[] = {
+	{ .compatible = "lcd-powercontrol", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, lcd_pwrctrl_match);
+#endif
+
+static struct platform_driver lcd_pwrctrl_driver = {
+	.driver		= {
+		.name	= "lcd-pwrctrl",
+		.owner	= THIS_MODULE,
+		.pm	= LCD_PWRCTRL_DEV_PM_OPS,
+		.of_match_table	= of_match_ptr(lcd_pwrctrl_match),
+	},
+	.probe		= lcd_pwrctrl_probe,
+	.remove		= lcd_pwrctrl_remove,
+};
+
+module_platform_driver(lcd_pwrctrl_driver);
+
+MODULE_AUTHOR("Thomas Abraham <thomas.ab@samsung.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:lcd-pwrctrl");
diff --git a/include/video/lcd_pwrctrl.h b/include/video/lcd_pwrctrl.h
new file mode 100644
index 0000000..924bfd2
--- /dev/null
+++ b/include/video/lcd_pwrctrl.h
@@ -0,0 +1,24 @@
+/*
+ * Simple lcd panel power control driver.
+ *
+ * Copyright (c) 2011-2012 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2011-2012 Linaro Ltd.
+ *
+ * This driver is derived from platform-lcd.h which was written by
+ * Ben Dooks <ben@simtec.co.uk>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+*/
+
+/**
+ * struct lcd_pwrctrl_data - platform data for lcd_pwrctrl driver.
+ * @gpio: GPIO number of the host system that connects to nRESET line.
+ * @invert: True, if output of gpio connected to nRESET should be inverted.
+ */
+struct lcd_pwrctrl_data {
+	int		gpio;
+	bool		invert;
+};
-- 
1.6.6.rc2

[PATCH v3 2/2] ARM: Exynos: Use lcd power control driver for lcd panel

[Thomas Abraham]

The Hydis hv070wsa lcd panel used with the Origen board uses a gpio
for reset and the Vcc supply to the panel can be controlled using
a voltage regulator. Switch to using the lcd power control driver
for controlling the power to the lcd panel.

Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
 arch/arm/mach-exynos/mach-origen.c |   29 ++++++-----------------------
 1 files changed, 6 insertions(+), 23 deletions(-)

diff --git a/arch/arm/mach-exynos/mach-origen.c b/arch/arm/mach-exynos/mach-origen.c
index d3b2e9d..6c4997c 100644
--- a/arch/arm/mach-exynos/mach-origen.c
+++ b/arch/arm/mach-exynos/mach-origen.c
@@ -26,7 +26,7 @@
 #include <asm/hardware/gic.h>
 #include <asm/mach-types.h>
 
-#include <video/platform_lcd.h>
+#include <video/lcd_pwrctrl.h>
 
 #include <plat/regs-serial.h>
 #include <plat/regs-fb-v4.h>
@@ -129,7 +129,7 @@ static struct regulator_consumer_supply __initdata buck3_consumer[] = {
 	REGULATOR_SUPPLY("vdd_g3d", "mali_drm"), /* G3D */
 };
 static struct regulator_consumer_supply __initdata buck7_consumer[] = {
-	REGULATOR_SUPPLY("vcc", "platform-lcd"), /* LCD */
+	REGULATOR_SUPPLY("vcc-lcd", "lcd-pwrctrl.0"), /* LCD */
 };
 
 static struct regulator_init_data __initdata max8997_ldo1_data = {
@@ -384,7 +384,7 @@ static struct regulator_init_data __initdata max8997_buck7_data = {
 		.name		= "VDD_LCD_3.3V",
 		.min_uV		= 3300000,
 		.max_uV		= 3300000,
-		.boot_on	= 1,
+		.boot_on	= 0,
 		.apply_uV	= 1,
 		.valid_ops_mask	= REGULATOR_CHANGE_STATUS,
 		.state_mem	= {
@@ -553,29 +553,12 @@ static struct platform_device origen_device_gpiokeys = {
 	},
 };
 
-static void lcd_hv070wsa_set_power(struct plat_lcd_data *pd, unsigned int power)
-{
-	int ret;
-
-	if (power)
-		ret = gpio_request_one(EXYNOS4_GPE3(4),
-					GPIOF_OUT_INIT_HIGH, "GPE3_4");
-	else
-		ret = gpio_request_one(EXYNOS4_GPE3(4),
-					GPIOF_OUT_INIT_LOW, "GPE3_4");
-
-	gpio_free(EXYNOS4_GPE3(4));
-
-	if (ret)
-		pr_err("failed to request gpio for LCD power: %d\n", ret);
-}
-
-static struct plat_lcd_data origen_lcd_hv070wsa_data = {
-	.set_power = lcd_hv070wsa_set_power,
+static struct lcd_pwrctrl_data origen_lcd_hv070wsa_data = {
+	.gpio	= EXYNOS4_GPE3(4),
 };
 
 static struct platform_device origen_lcd_hv070wsa = {
-	.name			= "platform-lcd",
+	.name			= "lcd-pwrctrl",
 	.dev.parent		= &s5p_device_fimd0.dev,
 	.dev.platform_data	= &origen_lcd_hv070wsa_data,
 };
-- 
1.6.6.rc2

Re: [PATCH v3 1/2] backlight: lcd: add driver for raster-type lcd's with gpio controlled panel reset

[Andrew Morton]

> Cc: rpurdie@rpsys.net, florianSchandinat@gmx.de, devicetree-discuss@lists.ozlabs.org, linux-fbdev@vger.kernel.org, linux-samsung-soc@vger.kernel.org, grant.likely@secretlab.ca, rob.herring@calxeda.com, kgene.kim@samsung.com, jg1.han@samsung.com, broonie@opensource.wolfsonmicro.com, kyungmin.park@samsung.com, augulis.darius@gmail.com, ben-linux@fluff.org, lars@metafoo.de, patches@linaro.org

Poor me.  When someone sends a patch like this, I need to go and hunt
down everyone's real names to nicely add them to the changelog's Cc:
list.  I prefer that you do this ;)

You can add Cc:'s to the changelog yourself, of course.  Often that
works out better than having me try to work out who might be
interested in the patch.

On Mon, 26 Mar 2012 14:16:39 +0530
Thomas Abraham <thomas.abraham@linaro.org> wrote:

> Add a lcd panel driver for simple raster-type lcd's which uses a gpio
> controlled panel reset. The driver controls the nRESET line of the panel
> using a gpio connected from the host system. The Vcc supply to the panel
> is (optionally) controlled using a voltage regulator. This driver excludes
> support for lcd panels that use a serial command interface or direct
> memory mapped IO interface.
> 
>
> ...
>
> +struct lcd_pwrctrl {
> +	struct device		*dev;
> +	struct lcd_device	*lcd;
> +	struct lcd_pwrctrl_data	*pdata;
> +	struct regulator	*regulator;
> +	unsigned int		power;
> +	bool			suspended;
> +	bool			pwr_en;

Generally kernel code will avoid these unpronounceable abbreviations. 
So we do

pwr -> power
en -> enable
ctrl -> control

etc.  It results in longer identifiers, but the code is more readable
and, more importantly, more *rememberable*.  

> +};
> +
> +static int lcd_pwrctrl_get_power(struct lcd_device *lcd)
> +{
> +	struct lcd_pwrctrl *lp = lcd_get_data(lcd);
> +	return lp->power;
> +}
> +
> +static int lcd_pwrctrl_set_power(struct lcd_device *lcd, int power)

See, shouldn't that have been lcd_pwrctrl_set_pwr?

If we avoid the abbreviations, such issues do not arise.

> +{
> +	struct lcd_pwrctrl *lp = lcd_get_data(lcd);
> +	struct lcd_pwrctrl_data *pd = lp->pdata;
> +	bool lcd_enable;
> +	int lcd_reset, ret = 0;
> +
> +	lcd_enable = (power = FB_BLANK_POWERDOWN || lp->suspended) ? 0 : 1;

This isn't how to use `bool'.  We can use

	lcd_enable = (power = FB_BLANK_POWERDOWN) || lp->suspended;


> +	lcd_reset = (pd->invert) ? !lcd_enable : lcd_enable;
> +
> +	if (lp->pwr_en = lcd_enable)
> +		return 0;
> +
> +	if (!IS_ERR_OR_NULL(lp->regulator)) {
> +		if (lcd_enable) {
> +			if (regulator_enable(lp->regulator)) {
> +				dev_info(lp->dev, "regulator enable failed\n");
> +				ret = -EPERM;
> +			}
> +		} else {
> +			if (regulator_disable(lp->regulator)) {
> +				dev_info(lp->dev, "regulator disable failed\n");
> +				ret = -EPERM;
> +			}
> +		}
> +	}
> +
> +	gpio_direction_output(lp->pdata->gpio, lcd_reset);
> +	lp->power = power;
> +	lp->pwr_en = lcd_enable;

Again, this could have been any of

	lp->[power|pwr] = [power|pwr];
	lp->[power|pwr]_[en|enable] = lcd_[en|enable];

zillions of combinations!  If we just avoid the abbreviations, there is
only one combination.

> +	return ret;
> +}
> +
>
> ...
>
> +static int __devinit lcd_pwrctrl_probe(struct platform_device *pdev)
> +{
> +	struct lcd_pwrctrl *lp;
> +	struct lcd_pwrctrl_data *pdata = pdev->dev.platform_data;
> +	struct device *dev = &pdev->dev;
> +	int err;
> +
> +#ifdef CONFIG_OF
> +	if (dev->of_node) {
> +		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +		if (!pdata) {
> +			dev_err(dev, "memory allocation for pdata failed\n");
> +			return -ENOMEM;
> +		}
> +		lcd_pwrctrl_parse_dt(dev, pdata);
> +	}
> +#endif
> +
> +	if (!pdata) {
> +		dev_err(dev, "platform data not available\n");
> +		return -EINVAL;
> +	}
> +
> +	lp = devm_kzalloc(dev, sizeof(struct lcd_pwrctrl), GFP_KERNEL);

Nit: I prefer sizeof(*lp) here, so I don't have to scroll back and
check the type of lp.

> +	if (!lp) {
> +		dev_err(dev, "memory allocation failed for private data\n");
> +		return -ENOMEM;
> +	}
> +
> +	err = gpio_request(pdata->gpio, "LCD-nRESET");
> +	if (err) {
> +		dev_err(dev, "gpio [%d] request failed\n", pdata->gpio);
> +		return err;
> +	}
> +
>
> ...
>

The code looks OK to me, but I do think the naming decisions should be
revisited, please.