On Fri, 4 May 2018 12:28:33 +0200
Thierry Reding <thierry.reding@xxxxxxxxx> wrote:
> On Thu, May 03, 2018 at 06:40:08PM +0200, Boris Brezillon wrote:
> > Having a device with a status property != "okay" in the DT is a valid
> > use case, and we should not prevent the registration of the DRM device
> > when the DSI device connected to the DSI controller is disabled.
> >
> > Consider the ENODEV return code as a valid result and do not expose the
> > DSI encoder/connector when it happens.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/vc4/vc4_dsi.c | 15 +++++++++++++--
> > 1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
> > index 8aa897835118..db2f137f8b7b 100644
> > --- a/drivers/gpu/drm/vc4/vc4_dsi.c
> > +++ b/drivers/gpu/drm/vc4/vc4_dsi.c
> > @@ -1606,8 +1606,18 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data)
> >
> > ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
> > &panel, &dsi->bridge);
> > - if (ret)
> > + if (ret) {
> > + /* If the bridge or panel pointed by dev->of_node is not
> > + * enabled, just return 0 here so that we don't prevent the DRM
> > + * dev from being registered. Of course that means the DSI
> > + * encoder won't be exposed, but that's not a problem since
> > + * nothing is connected to it.
> > + */
> > + if (ret == -ENODEV)
> > + return 0;
> > +
> > return ret;
> > + }
> >
> > if (panel) {
> > dsi->bridge = devm_drm_panel_bridge_add(dev, panel,
> > @@ -1652,7 +1662,8 @@ static void vc4_dsi_unbind(struct device *dev, struct device *master,
> > struct vc4_dev *vc4 = to_vc4_dev(drm);
> > struct vc4_dsi *dsi = dev_get_drvdata(dev);
> >
> > - pm_runtime_disable(dev);
> > + if (dsi->bridge)
> > + pm_runtime_disable(dev);
>
> Is this safe? This uses component/master, so dsi->bridge is going to
> remain valid until the driver's ->remove() is called. So technically you
> could have a situation where drm_of_find_panel_or_bridge() returned some
> error code that remains stored in dsi->bridge and cause the above
> condition to be incorrectly true.
No, because of_drm_find_bridge() (which is called from
drm_of_find_panel_or_bridge() returns either NULL or a valid bridge
pointer), so dsi->bridge either points to a valid bridge object or is
NULL. Am I missing something?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
