Hi Jyri,
On Tuesday, 11 December 2018 15:01:52 EET Jyri Sarha wrote:
> On 06/12/2018 22:26, Laurent Pinchart wrote:
> > The TFP410 has a powerdown pin that can be connected to a GPIO to
> > control power saving. The DT bindings define a corresponding property,
> > but the driver doesn't implement support for it. Fix that.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> > ---
>
> I find the naming of the the gpio bit backwards, but I guess it si fine
> since it is also the name of the pin.
I would have gone for "enable-gpios", but given that we have DTs in the wild
using the "powerdown-gpios" property (with the omapdrm-specific TFP410 driver
which uses the same compatible string as the drm_bridge-based driver) I think
we need to stick to it. As you mentioned this is the pin's name, so it's not
too big of a deal.
I'll now wait for your review of patch 5/5 before merging these changes :-)
The plan is to get them in drm-misc for v4.22, please let me know if that
causes any problem for you. Also, please feel free to review 1/5 and 2/5 if
you have time.
> Reviewed-by: Jyri Sarha <jsa...@ti.com>
>
> > drivers/gpu/drm/bridge/ti-tfp410.c | 24 ++++++++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c
> > b/drivers/gpu/drm/bridge/ti-tfp410.c index e4280f5af9f5..d25d23cfe3f5
> > 100644
> > --- a/drivers/gpu/drm/bridge/ti-tfp410.c
> > +++ b/drivers/gpu/drm/bridge/ti-tfp410.c
> > @@ -32,6 +32,7 @@ struct tfp410 {
> > struct i2c_adapter *ddc;
> > struct gpio_desc *hpd;
> > struct delayed_work hpd_work;
> > + struct gpio_desc *powerdown;
> > struct device *dev;
> > };
> >
> > @@ -139,8 +140,24 @@ static int tfp410_attach(struct drm_bridge *bridge)
> > return 0;
> > }
> >
> > +static void tfp410_enable(struct drm_bridge *bridge)
> > +{
> > + struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
> > +
> > + gpiod_set_value_cansleep(dvi->powerdown, 0);
> > +}
> > +
> > +static void tfp410_disable(struct drm_bridge *bridge)
> > +{
> > + struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
> > +
> > + gpiod_set_value_cansleep(dvi->powerdown, 1);
> > +}
> > +
> > static const struct drm_bridge_funcs tfp410_bridge_funcs = {
> > .attach = tfp410_attach,
> > + .enable = tfp410_enable,
> > + .disable = tfp410_disable,
> > };
> >
> > static void tfp410_hpd_work_func(struct work_struct *work)
> > @@ -229,6 +246,13 @@ static int tfp410_init(struct device *dev)
> > if (ret)
> > goto fail;
> >
> > + dvi->powerdown = devm_gpiod_get_optional(dev, "powerdown",
> > + GPIOD_OUT_HIGH);
> > + if (IS_ERR(dvi->powerdown)) {
> > + dev_err(dev, "failed to parse powerdown gpio\n");
> > + return PTR_ERR(dvi->powerdown);
> > + }
> > +
> > if (dvi->hpd) {
> > INIT_DELAYED_WORK(&dvi->hpd_work, tfp410_hpd_work_func);
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
