[4/4] drm/panel: jh057n0090: Add regulator support

Submitted by Guido Günther on June 25, 2019, 5:05 p.m.

Details

Message ID b239f1db7a1f67988a9bd1ed62f6a1cf1dce944c.1561482165.git.agx@sigxcpu.org
State New
Headers show
Series "drm/panel: jh057n0090: Add regulators and drop magic value in init" ( rev: 1 ) in DRI devel

Not browsing as part of any series.

Commit Message

Guido Günther June 25, 2019, 5:05 p.m.
Allow to specify regulators for vcc and iovcc. According to the data
sheet the panel wants vcc (2.8V) and iovcc (1.8V) and there's no startup
dependency between the two.

Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
 .../drm/panel/panel-rocktech-jh057n00900.c    | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
index b8a069055fbc..f8f6f087b9bc 100644
--- a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
+++ b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
@@ -15,6 +15,7 @@ 
 #include <linux/gpio/consumer.h>
 #include <linux/media-bus-format.h>
 #include <linux/module.h>
+#include <linux/regulator/consumer.h>
 #include <video/display_timing.h>
 #include <video/mipi_display.h>
 
@@ -47,6 +48,8 @@  struct jh057n {
 	struct drm_panel panel;
 	struct gpio_desc *reset_gpio;
 	struct backlight_device *backlight;
+	struct regulator *vcc;
+	struct regulator *iovcc;
 	bool prepared;
 
 	struct dentry *debugfs;
@@ -160,6 +163,8 @@  static int jh057n_unprepare(struct drm_panel *panel)
 		return 0;
 
 	mipi_dsi_dcs_set_display_off(dsi);
+	regulator_disable(ctx->iovcc);
+	regulator_disable(ctx->vcc);
 	ctx->prepared = false;
 
 	return 0;
@@ -174,6 +179,13 @@  static int jh057n_prepare(struct drm_panel *panel)
 		return 0;
 
 	DRM_DEV_DEBUG_DRIVER(ctx->dev, "Resetting the panel\n");
+	ret = regulator_enable(ctx->vcc);
+	if (ret < 0)
+		return ret;
+	ret = regulator_enable(ctx->iovcc);
+	if (ret < 0)
+		return ret;
+
 	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
 	usleep_range(20, 40);
 	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
@@ -301,6 +313,13 @@  static int jh057n_probe(struct mipi_dsi_device *dsi)
 	if (IS_ERR(ctx->backlight))
 		return PTR_ERR(ctx->backlight);
 
+	ctx->vcc = devm_regulator_get(dev, "vcc");
+	if (IS_ERR(ctx->vcc))
+		return PTR_ERR(ctx->vcc);
+	ctx->iovcc = devm_regulator_get(dev, "iovcc");
+	if (IS_ERR(ctx->iovcc))
+		return PTR_ERR(ctx->iovcc);
+
 	drm_panel_init(&ctx->panel);
 	ctx->panel.dev = dev;
 	ctx->panel.funcs = &jh057n_drm_funcs;

Comments

On Tue, Jun 25, 2019 at 07:05:19PM +0200, Guido Günther wrote:
> Allow to specify regulators for vcc and iovcc. According to the data
> sheet the panel wants vcc (2.8V) and iovcc (1.8V) and there's no startup
> dependency between the two.
s/jh057n0090/jh057n00900

> 
> Signed-off-by: Guido Günther <agx@sigxcpu.org>
> ---
>  .../drm/panel/panel-rocktech-jh057n00900.c    | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
> index b8a069055fbc..f8f6f087b9bc 100644
> --- a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
> +++ b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
> @@ -15,6 +15,7 @@
>  #include <linux/gpio/consumer.h>
>  #include <linux/media-bus-format.h>
>  #include <linux/module.h>
> +#include <linux/regulator/consumer.h>
>  #include <video/display_timing.h>
>  #include <video/mipi_display.h>
>  
> @@ -47,6 +48,8 @@ struct jh057n {
>  	struct drm_panel panel;
>  	struct gpio_desc *reset_gpio;
>  	struct backlight_device *backlight;
> +	struct regulator *vcc;
> +	struct regulator *iovcc;
>  	bool prepared;
>  
>  	struct dentry *debugfs;
> @@ -160,6 +163,8 @@ static int jh057n_unprepare(struct drm_panel *panel)
>  		return 0;
>  
>  	mipi_dsi_dcs_set_display_off(dsi);
> +	regulator_disable(ctx->iovcc);
> +	regulator_disable(ctx->vcc);
>  	ctx->prepared = false;
>  
>  	return 0;
> @@ -174,6 +179,13 @@ static int jh057n_prepare(struct drm_panel *panel)
>  		return 0;
>  
>  	DRM_DEV_DEBUG_DRIVER(ctx->dev, "Resetting the panel\n");
> +	ret = regulator_enable(ctx->vcc);
> +	if (ret < 0)
> +		return ret;
> +	ret = regulator_enable(ctx->iovcc);
> +	if (ret < 0)
> +		return ret;
> +
>  	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
>  	usleep_range(20, 40);
>  	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> @@ -301,6 +313,13 @@ static int jh057n_probe(struct mipi_dsi_device *dsi)
>  	if (IS_ERR(ctx->backlight))
>  		return PTR_ERR(ctx->backlight);
>  
> +	ctx->vcc = devm_regulator_get(dev, "vcc");
> +	if (IS_ERR(ctx->vcc))
> +		return PTR_ERR(ctx->vcc);
> +	ctx->iovcc = devm_regulator_get(dev, "iovcc");
> +	if (IS_ERR(ctx->iovcc))
> +		return PTR_ERR(ctx->iovcc);
> +
Consider to write an error message.
The regulators are now mandatory, but they be missing in some device
trees. So it would be good to help them to understand why it fails.

With this considered:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

	Sam
Hi Sam,
On Tue, Jun 25, 2019 at 11:24:19PM +0200, Sam Ravnborg wrote:
> On Tue, Jun 25, 2019 at 07:05:19PM +0200, Guido Günther wrote:
> > Allow to specify regulators for vcc and iovcc. According to the data
> > sheet the panel wants vcc (2.8V) and iovcc (1.8V) and there's no startup
> > dependency between the two.
> s/jh057n0090/jh057n00900
> 
> > 
> > Signed-off-by: Guido Günther <agx@sigxcpu.org>
> > ---
> >  .../drm/panel/panel-rocktech-jh057n00900.c    | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
> > index b8a069055fbc..f8f6f087b9bc 100644
> > --- a/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
> > +++ b/drivers/gpu/drm/panel/panel-rocktech-jh057n00900.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/gpio/consumer.h>
> >  #include <linux/media-bus-format.h>
> >  #include <linux/module.h>
> > +#include <linux/regulator/consumer.h>
> >  #include <video/display_timing.h>
> >  #include <video/mipi_display.h>
> >  
> > @@ -47,6 +48,8 @@ struct jh057n {
> >  	struct drm_panel panel;
> >  	struct gpio_desc *reset_gpio;
> >  	struct backlight_device *backlight;
> > +	struct regulator *vcc;
> > +	struct regulator *iovcc;
> >  	bool prepared;
> >  
> >  	struct dentry *debugfs;
> > @@ -160,6 +163,8 @@ static int jh057n_unprepare(struct drm_panel *panel)
> >  		return 0;
> >  
> >  	mipi_dsi_dcs_set_display_off(dsi);
> > +	regulator_disable(ctx->iovcc);
> > +	regulator_disable(ctx->vcc);
> >  	ctx->prepared = false;
> >  
> >  	return 0;
> > @@ -174,6 +179,13 @@ static int jh057n_prepare(struct drm_panel *panel)
> >  		return 0;
> >  
> >  	DRM_DEV_DEBUG_DRIVER(ctx->dev, "Resetting the panel\n");
> > +	ret = regulator_enable(ctx->vcc);
> > +	if (ret < 0)
> > +		return ret;
> > +	ret = regulator_enable(ctx->iovcc);
> > +	if (ret < 0)
> > +		return ret;
> > +
> >  	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> >  	usleep_range(20, 40);
> >  	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> > @@ -301,6 +313,13 @@ static int jh057n_probe(struct mipi_dsi_device *dsi)
> >  	if (IS_ERR(ctx->backlight))
> >  		return PTR_ERR(ctx->backlight);
> >  
> > +	ctx->vcc = devm_regulator_get(dev, "vcc");
> > +	if (IS_ERR(ctx->vcc))
> > +		return PTR_ERR(ctx->vcc);
> > +	ctx->iovcc = devm_regulator_get(dev, "iovcc");
> > +	if (IS_ERR(ctx->iovcc))
> > +		return PTR_ERR(ctx->iovcc);
> > +
> Consider to write an error message.
> The regulators are now mandatory, but they be missing in some device
> trees. So it would be good to help them to understand why it fails.

I've fixed this and your other comments in v2.
> 
> With this considered:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

Thanks! I've not added this yet since I made two more changes:

- also print an error when regulator_enable() fails
- disable vcc if enabling iovcc fails afterwards

Hope this looks sane now.

Cheers,
 -- Guido

> 
> 	Sam
>