On Mon, Feb 06, 2017 at 11:42:48AM +0100, Philipp Zabel wrote:
> On Fri, 2017-02-03 at 21:36 -0600, Rob Herring wrote:
> > Many drivers have a common pattern of searching the OF graph for either an
> > attached panel or bridge and then finding the DRM struct for the panel
> > or bridge. Also, most drivers need to handle deferred probing when the
> > DRM device is not yet instantiated. Create a common function,
> > drm_of_find_panel_or_bridge, to find the connected node and the
> > associated DRM panel or bridge device.
[...]
> > +int drm_of_find_panel_or_bridge(const struct device_node *np,
> > + int port, int endpoint,
> > + struct drm_panel **panel,
> > + struct drm_bridge **bridge)
> > +{
> > + int ret = -ENODEV;
>
> This is only returned if !panel && !bridge. I'd consider this invalid
> usage of this function, so maybe use -EINVAL?
Yes.
> > + struct device_node *remote;
> > +
> > + remote = of_graph_get_remote_node(np, port, endpoint);
> > + if (!remote)
> > + return -ENODEV;
> > +
> > + if (bridge)
> > + *bridge = NULL;
>
> I would move this ^ ...
>
> > + if (panel) {
> > + *panel = of_drm_find_panel(remote);
> > + if (*panel) {
>
> ... here.
Okay.
> > + ret = 0;
> > + goto out_put;
> > + }
> > + ret = -EPROBE_DEFER;
> > + }
> > +
> > + if (bridge) {
> > + *bridge = of_drm_find_bridge(remote);
> > + if (*bridge)
> > + ret = 0;
> > + else
> > + ret = -EPROBE_DEFER;
> > + }
> > +out_put:
> > + of_node_put(remote);
> > + return ret;
> > +}
I've ended up re-writing things a bit getting rid of the goto and the
result looks like this:
int drm_of_find_panel_or_bridge(const struct device_node *np,
int port, int endpoint,
struct drm_panel **panel,
struct drm_bridge **bridge)
{
int ret = -EPROBE_DEFER;
struct device_node *remote;
if (!panel && !bridge)
return -EINVAL;
remote = of_graph_get_remote_node(np, port, endpoint);
if (!remote)
return -ENODEV;
if (panel) {
*panel = of_drm_find_panel(remote);
if (*panel) {
if (bridge)
*bridge = NULL;
ret = 0;
}
}
/* No panel found yet, check for a bridge next. */
if (ret && bridge) {
*bridge = of_drm_find_bridge(remote);
if (*bridge)
ret = 0;
}
of_node_put(remote);
return ret;
}
--
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
