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@xxxxxxxxxxx>
> > ---
> > .../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@xxxxxxxxxxxx>
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
>
