Re: [PATCH 1/2] video: exynos_dp: Add parsing of gpios pins to exynos-dp driver

Re: [PATCH 1/2] video: exynos_dp: Add parsing of gpios pins to exynos-dp driver

[한진구]

Tuesday, May 14, 2013 11:17 PM, Vikas Sajjan wrote:
> 
> Hi Vikas,
> 
> On Tuesday 14 of May 2013 18:25:51 Vikas Sajjan wrote:
> >  Adds GPIO parsing functionality for "LCD backlight" and "LCD enable"
> >  GPIO pins of exynos dp controller.
> >
> > Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
> > ---
> >  drivers/video/exynos/exynos_dp_core.c |   45
> > +++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)
> >
> 
> I don't think that Exynos DP driver is right place for such code. Backlight
> and LCD drivers are responsible for backlight and LCD power control using
> backlight and LCD subsystems.
> 
> IMHO the correct solution would be to either extend existing backlight/lcd
> drivers found in drivers/video/backlight to support direct GPIO control and
> parse GPIO pins from device tree or create new gpio_bl and gpio_lcd drivers.

Hi Vikas Sajian,

I agree with Tomasz Figa's opinion.
Backlight/LCD framework should be used.
eDP panel backlight on SMDK5210 board can be controlled by PWM;
thus, pwm-backlight driver should be used.
Also, eDP panel reset pin should be controlled by using
platform-lcd driver.

> 
> CCing Richard, Florian and linux-fbdev.

Also, I have been doing backlight reviews instead of Richard,
please do CC'ing me.

Best regards,
Jingoo Han

> 
> Best regards,
> Tomasz

Re: [PATCH 1/2] video: exynos_dp: Add parsing of gpios pins to exynos-dp driver

[Tomasz Figa]

Hi Jingoo,

On Thursday 16 of May 2013 02:03:59 한진구 wrote:
> Tuesday, May 14, 2013 11:17 PM, Vikas Sajjan wrote:
> 
> > 
> > Hi Vikas,
> > 
> > On Tuesday 14 of May 2013 18:25:51 Vikas Sajjan wrote:
> > 
> > >  Adds GPIO parsing functionality for "LCD backlight" and "LCD enable"
> > >  GPIO pins of exynos dp controller.
> > >
> > >
> > >
> > > Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
> > > ---
> > > 
> > >  drivers/video/exynos/exynos_dp_core.c |   45
> > > 
> > > +++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)
> > >
> > >
> > 
> > 
> > I don't think that Exynos DP driver is right place for such code.
> > Backlight
 and LCD drivers are responsible for backlight and LCD power
> > control using backlight and LCD subsystems.
> > 
> > IMHO the correct solution would be to either extend existing
> > backlight/lcd
> > drivers found in drivers/video/backlight to support direct GPIO control
> > and
 parse GPIO pins from device tree or create new gpio_bl and gpio_lcd
> > drivers.
> 
> Hi Vikas Sajian,
> 
> I agree with Tomasz Figa's opinion.
> Backlight/LCD framework should be used.
> eDP panel backlight on SMDK5210 board can be controlled by PWM;
> thus, pwm-backlight driver should be used.
> Also, eDP panel reset pin should be controlled by using
> platform-lcd driver.
> 
> 
> > 
> > CCing Richard, Florian and linux-fbdev.
> 
> 
> Also, I have been doing backlight reviews instead of Richard,
> please do CC'ing me.

OK. I used get_maintainers script, but it seems like the result was a bit off 
in this case. Will remember for future.

Best regards,
-- 
Tomasz Figa
Linux Kernel Developer
Samsung R&D Institute Poland
Samsung Electronics

[PATCH 0/2] Add LCD backlight and LCD enable gpios pins to dp-controller DT node

[Vikas Sajjan]

This patch series adds LCD backlight and LCD enable gpios pins to dp-controller
DT node of exynos5250-smdk5250 and parsing of these gpio pins in exynos-dp driver

tested on exynos5250-smdk5250 Board.

rebased on kgene-next branch of 
https://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/

Vikas Sajjan (2):
  video: exynos_dp: Add parsing of gpios pins to exynos-dp driver
  ARM: dts: Add LCD backlight and LCD enable gpios pins to
    dp-controller DT node

 arch/arm/boot/dts/exynos5250-smdk5250.dts |    3 ++
 drivers/video/exynos/exynos_dp_core.c     |   45 +++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

-- 
1.7.9.5

[PATCH 1/2] video: exynos_dp: Add parsing of gpios pins to exynos-dp driver

[Vikas Sajjan]

 Adds GPIO parsing functionality for "LCD backlight" and "LCD enable"
 GPIO pins of exynos dp controller.

Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
---
 drivers/video/exynos/exynos_dp_core.c |   45 +++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index de9d4da..242a708 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -19,6 +19,7 @@
 #include <linux/interrupt.h>
 #include <linux/delay.h>
 #include <linux/of.h>
+#include <linux/of_gpio.h>
 
 #include <video/exynos_dp.h>
 
@@ -895,11 +896,51 @@ static void exynos_dp_hotplug(struct work_struct *work)
 }
 
 #ifdef CONFIG_OF
+
+static int exynos_parse_gpio(struct device *dev)
+{
+	int gpio = -1;
+	struct device_node *np = dev->of_node;
+	enum of_gpio_flags flags;
+	u32 value;
+	int ret = -1;
+
+	if (!of_find_property(np, "lcd_bl_gpio", &value)) {
+                dev_err(dev, "no bl gpio property found\n");
+		return -1;
+        }
+
+	gpio = of_get_named_gpio_flags(np, "lcd_bl_gpio", 0, &flags);
+        if (gpio_is_valid(gpio)) {
+                ret = gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH, "exynos4-fb");
+                if (ret < 0) {
+                        return ret;
+		}
+        }
+	gpio_set_value(gpio, 1);
+
+	if (!of_find_property(np, "lcd_en_gpio", &value)) {
+                dev_err(dev, "no bl gpio property found\n");
+		return -1;
+        }
+        gpio = of_get_named_gpio_flags(np, "lcd_en_gpio", 0, &flags);
+        if (gpio_is_valid(gpio)) {
+                ret = gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH, "exynos4-fb");
+                if (ret < 0) {
+                        return ret;
+		}
+        }
+	gpio_set_value(gpio, 1);
+
+	return 0;
+}
+
 static struct exynos_dp_platdata *exynos_dp_dt_parse_pdata(struct device *dev)
 {
 	struct device_node *dp_node = dev->of_node;
 	struct exynos_dp_platdata *pd;
 	struct video_info *dp_video_config;
+	int ret = -1;
 
 	pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
 	if (!pd) {
@@ -960,6 +1001,10 @@ static struct exynos_dp_platdata *exynos_dp_dt_parse_pdata(struct device *dev)
 		return ERR_PTR(-EINVAL);
 	}
 
+	ret = exynos_parse_gpio(dev);
+	if (ret != 0)
+		return NULL;
+
 	return pd;
 }
 
-- 
1.7.9.5

Re: [PATCH 1/2] video: exynos_dp: Add parsing of gpios pins to exynos-dp driver

[Tomasz Figa]

Hi Vikas,

On Tuesday 14 of May 2013 18:25:51 Vikas Sajjan wrote:
>  Adds GPIO parsing functionality for "LCD backlight" and "LCD enable"
>  GPIO pins of exynos dp controller.
> 
> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
> ---
>  drivers/video/exynos/exynos_dp_core.c |   45
> +++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)
> 

I don't think that Exynos DP driver is right place for such code. Backlight 
and LCD drivers are responsible for backlight and LCD power control using 
backlight and LCD subsystems.

IMHO the correct solution would be to either extend existing backlight/lcd 
drivers found in drivers/video/backlight to support direct GPIO control and 
parse GPIO pins from device tree or create new gpio_bl and gpio_lcd drivers.

CCing Richard, Florian and linux-fbdev.

Best regards,
Tomasz