[PATCH v2 3/3] fbdev: omap2: panel-dpi: implement support for a vcc regulator

[PATCH v2 3/3] fbdev: omap2: panel-dpi: implement support for a vcc regulator

[Uwe Kleine-König]

To allow supporting displays that need some logic to enable power to the
display try to get a vcc-supply property from the device tree and drive
the resulting regulator accordingly.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Changes since (implicit) v1, sent with
Message-Id: 1449753107-11410-5-git-send-email-uwe@kleine-koenig.org:

 - split out dt binding changes
 - reword commit log
---
 drivers/video/fbdev/omap2/displays-new/panel-dpi.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/video/fbdev/omap2/displays-new/panel-dpi.c b/drivers/video/fbdev/omap2/displays-new/panel-dpi.c
index 201a1c1a6f42..8c3f31ebff00 100644
--- a/drivers/video/fbdev/omap2/displays-new/panel-dpi.c
+++ b/drivers/video/fbdev/omap2/displays-new/panel-dpi.c
@@ -15,6 +15,7 @@
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
+#include <linux/regulator/consumer.h>
 
 #include <video/omapdss.h>
 #include <video/omap-panel-data.h>
@@ -32,6 +33,7 @@ struct panel_drv_data {
 	int backlight_gpio;
 
 	struct gpio_desc *enable_gpio;
+	struct regulator *vcc_supply;
 };
 
 #define to_panel_data(p) container_of(p, struct panel_drv_data, dssdev)
@@ -83,6 +85,12 @@ static int panel_dpi_enable(struct omap_dss_device *dssdev)
 	if (r)
 		return r;
 
+	r = regulator_enable(ddata->vcc_supply);
+	if (r) {
+		in->ops.dpi->disable(in);
+		return r;
+	}
+
 	gpiod_set_value_cansleep(ddata->enable_gpio, 1);
 
 	if (gpio_is_valid(ddata->backlight_gpio))
@@ -105,6 +113,7 @@ static void panel_dpi_disable(struct omap_dss_device *dssdev)
 		gpio_set_value_cansleep(ddata->backlight_gpio, 0);
 
 	gpiod_set_value_cansleep(ddata->enable_gpio, 0);
+	regulator_disable(ddata->vcc_supply);
 
 	in->ops.dpi->disable(in);
 
@@ -223,6 +232,10 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
 	if (IS_ERR(gpio))
 		return PTR_ERR(gpio);
 
+	ddata->vcc_supply = devm_regulator_get(&pdev->dev, "vcc");
+	if (IS_ERR(ddata->vcc_supply))
+		return PTR_ERR(ddata->vcc_supply);
+
 	ddata->backlight_gpio = -ENOENT;
 
 	r = of_get_display_timing(node, "panel-timing", &timing);
-- 
2.6.2

Re: [PATCH v2 3/3] fbdev: omap2: panel-dpi: implement support for a vcc regulator

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 2311 bytes --]



On 20/12/15 13:13, Uwe Kleine-König wrote:
> To allow supporting displays that need some logic to enable power to the
> display try to get a vcc-supply property from the device tree and drive
> the resulting regulator accordingly.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Changes since (implicit) v1, sent with
> Message-Id: 1449753107-11410-5-git-send-email-uwe@kleine-koenig.org:
> 
>  - split out dt binding changes
>  - reword commit log
> ---
>  drivers/video/fbdev/omap2/displays-new/panel-dpi.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/video/fbdev/omap2/displays-new/panel-dpi.c b/drivers/video/fbdev/omap2/displays-new/panel-dpi.c
> index 201a1c1a6f42..8c3f31ebff00 100644
> --- a/drivers/video/fbdev/omap2/displays-new/panel-dpi.c
> +++ b/drivers/video/fbdev/omap2/displays-new/panel-dpi.c
> @@ -15,6 +15,7 @@
>  #include <linux/slab.h>
>  #include <linux/of.h>
>  #include <linux/of_gpio.h>
> +#include <linux/regulator/consumer.h>
>  
>  #include <video/omapdss.h>
>  #include <video/omap-panel-data.h>
> @@ -32,6 +33,7 @@ struct panel_drv_data {
>  	int backlight_gpio;
>  
>  	struct gpio_desc *enable_gpio;
> +	struct regulator *vcc_supply;
>  };
>  
>  #define to_panel_data(p) container_of(p, struct panel_drv_data, dssdev)
> @@ -83,6 +85,12 @@ static int panel_dpi_enable(struct omap_dss_device *dssdev)
>  	if (r)
>  		return r;
>  
> +	r = regulator_enable(ddata->vcc_supply);
> +	if (r) {
> +		in->ops.dpi->disable(in);
> +		return r;
> +	}
> +
>  	gpiod_set_value_cansleep(ddata->enable_gpio, 1);
>  
>  	if (gpio_is_valid(ddata->backlight_gpio))
> @@ -105,6 +113,7 @@ static void panel_dpi_disable(struct omap_dss_device *dssdev)
>  		gpio_set_value_cansleep(ddata->backlight_gpio, 0);
>  
>  	gpiod_set_value_cansleep(ddata->enable_gpio, 0);
> +	regulator_disable(ddata->vcc_supply);
>  
>  	in->ops.dpi->disable(in);
>  
> @@ -223,6 +232,10 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
>  	if (IS_ERR(gpio))
>  		return PTR_ERR(gpio);
>  
> +	ddata->vcc_supply = devm_regulator_get(&pdev->dev, "vcc");
> +	if (IS_ERR(ddata->vcc_supply))
> +		return PTR_ERR(ddata->vcc_supply);

devm_regulator_get_optional()?

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 3/3] fbdev: omap2: panel-dpi: implement support for a vcc regulator

[Uwe Kleine-König]

Hello Tomi,

On Tue, Feb 16, 2016 at 03:00:00PM +0200, Tomi Valkeinen wrote:
> On 20/12/15 13:13, Uwe Kleine-König wrote:
> > @@ -223,6 +232,10 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
> >  	if (IS_ERR(gpio))
> >  		return PTR_ERR(gpio);
> >  
> > +	ddata->vcc_supply = devm_regulator_get(&pdev->dev, "vcc");
> > +	if (IS_ERR(ddata->vcc_supply))
> > +		return PTR_ERR(ddata->vcc_supply);
> 
> devm_regulator_get_optional()?

That would make the specification of a vcc supply obligatory IIUC. (Yes,
the semantic difference between regulator_get vs. regulator_get_optional
is reversed when comparing to gpio_get vs. gpio_get_optional.)

Best regards
Uwe


-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH v2 3/3] fbdev: omap2: panel-dpi: implement support for a vcc regulator

[Tomi Valkeinen]

[-- Attachment #1.1: Type: text/plain, Size: 1116 bytes --]

On 16/02/16 15:48, Uwe Kleine-König wrote:
> Hello Tomi,
> 
> On Tue, Feb 16, 2016 at 03:00:00PM +0200, Tomi Valkeinen wrote:
>> On 20/12/15 13:13, Uwe Kleine-König wrote:
>>> @@ -223,6 +232,10 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
>>>  	if (IS_ERR(gpio))
>>>  		return PTR_ERR(gpio);
>>>  
>>> +	ddata->vcc_supply = devm_regulator_get(&pdev->dev, "vcc");
>>> +	if (IS_ERR(ddata->vcc_supply))
>>> +		return PTR_ERR(ddata->vcc_supply);
>>
>> devm_regulator_get_optional()?
> 
> That would make the specification of a vcc supply obligatory IIUC. (Yes,
> the semantic difference between regulator_get vs. regulator_get_optional
> is reversed when comparing to gpio_get vs. gpio_get_optional.)

This is a bit confusing...

So, is regulator_get_optional() meant for cases where the device can
truly function without the supply in question? And regulator_get()
should be used when the device needs the supply, even if the supply
cannot be controlled via SW, and thus we may not even have the supply
visible in the SW side (leading to using dummy regulator)?

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 3/3] fbdev: omap2: panel-dpi: implement support for a vcc regulator

[Uwe Kleine-König]

Hello,

Cc += Liam Girdwood, Mark Brown

On Fri, Feb 26, 2016 at 12:13:07PM +0200, Tomi Valkeinen wrote:
> On 16/02/16 15:48, Uwe Kleine-König wrote:
> > Hello Tomi,
> > 
> > On Tue, Feb 16, 2016 at 03:00:00PM +0200, Tomi Valkeinen wrote:
> >> On 20/12/15 13:13, Uwe Kleine-König wrote:
> >>> @@ -223,6 +232,10 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
> >>>  	if (IS_ERR(gpio))
> >>>  		return PTR_ERR(gpio);
> >>>  
> >>> +	ddata->vcc_supply = devm_regulator_get(&pdev->dev, "vcc");
> >>> +	if (IS_ERR(ddata->vcc_supply))
> >>> +		return PTR_ERR(ddata->vcc_supply);
> >>
> >> devm_regulator_get_optional()?
> > 
> > That would make the specification of a vcc supply obligatory IIUC. (Yes,
> > the semantic difference between regulator_get vs. regulator_get_optional
> > is reversed when comparing to gpio_get vs. gpio_get_optional.)
> 
> This is a bit confusing...
> 
> So, is regulator_get_optional() meant for cases where the device can
> truly function without the supply in question? And regulator_get()
> should be used when the device needs the supply, even if the supply
> cannot be controlled via SW, and thus we may not even have the supply
> visible in the SW side (leading to using dummy regulator)?

Yeah, I think this is the right picture.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH v2 3/3] fbdev: omap2: panel-dpi: implement support for a vcc regulator

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1145 bytes --]

On Fri, Feb 26, 2016 at 01:10:08PM +0100, Uwe Kleine-König wrote:
> On Fri, Feb 26, 2016 at 12:13:07PM +0200, Tomi Valkeinen wrote:

> > > That would make the specification of a vcc supply obligatory IIUC. (Yes,
> > > the semantic difference between regulator_get vs. regulator_get_optional
> > > is reversed when comparing to gpio_get vs. gpio_get_optional.)

Not really, in both cases the _optional() case is for the case where the
requested resource may reasonably be absent.

> > So, is regulator_get_optional() meant for cases where the device can
> > truly function without the supply in question? And regulator_get()
> > should be used when the device needs the supply, even if the supply
> > cannot be controlled via SW, and thus we may not even have the supply
> > visible in the SW side (leading to using dummy regulator)?

> Yeah, I think this is the right picture.

Yes, exactly.  Even in cases where supplies could potentially be
software controlled frequently system constraints make it impossible to
actually change their state so the consumer should not normally assume
that a disable will actually happen.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]