| Message ID | 20200614172234.8856-3-digetx@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support DRM bridges on NVIDIA Tegra | expand |
Hi Dmitry, Thank you for the patch. On Sun, Jun 14, 2020 at 08:22:30PM +0300, Dmitry Osipenko wrote: > When graph isn't defined in a device-tree, the of_graph_get_remote_node() > prints a noisy error message, telling that port node is not found. This is > undesirable behaviour in our case because absence of a panel/bridge graph > is a valid case. Let's check presence of the local port in a device-tree > before proceeding with parsing the graph. > > Reviewed-by: Sam Ravnborg <sam@ravnborg.org> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/gpu/drm/drm_of.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c > index b50b44e76279..e0652c38f357 100644 > --- a/drivers/gpu/drm/drm_of.c > +++ b/drivers/gpu/drm/drm_of.c > @@ -239,13 +239,24 @@ int drm_of_find_panel_or_bridge(const struct device_node *np, > struct drm_bridge **bridge) > { > int ret = -EPROBE_DEFER; > - struct device_node *remote; > + struct device_node *local, *remote; > > if (!panel && !bridge) > return -EINVAL; > if (panel) > *panel = NULL; > > + /* > + * of_graph_get_remote_node() produces a noisy error message if port > + * node isn't found and the absence of the port is a legit case here, > + * so at first we silently check presence of the local port. > + */ > + local = of_graph_get_local_port(np); > + if (!local) > + return -ENODEV; > + > + of_node_put(local); > + The code looks fine, but you may want to take into account my proposal in 1/7 to instead create a of_graph_has_port() function. The could would be simpler here. > remote = of_graph_get_remote_node(np, port, endpoint); > if (!remote) > return -ENODEV;
16.06.2020 04:26, Laurent Pinchart пишет: > Hi Dmitry, > > Thank you for the patch. > > On Sun, Jun 14, 2020 at 08:22:30PM +0300, Dmitry Osipenko wrote: >> When graph isn't defined in a device-tree, the of_graph_get_remote_node() >> prints a noisy error message, telling that port node is not found. This is >> undesirable behaviour in our case because absence of a panel/bridge graph >> is a valid case. Let's check presence of the local port in a device-tree >> before proceeding with parsing the graph. >> >> Reviewed-by: Sam Ravnborg <sam@ravnborg.org> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> drivers/gpu/drm/drm_of.c | 13 ++++++++++++- >> 1 file changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c >> index b50b44e76279..e0652c38f357 100644 >> --- a/drivers/gpu/drm/drm_of.c >> +++ b/drivers/gpu/drm/drm_of.c >> @@ -239,13 +239,24 @@ int drm_of_find_panel_or_bridge(const struct device_node *np, >> struct drm_bridge **bridge) >> { >> int ret = -EPROBE_DEFER; >> - struct device_node *remote; >> + struct device_node *local, *remote; >> >> if (!panel && !bridge) >> return -EINVAL; >> if (panel) >> *panel = NULL; >> >> + /* >> + * of_graph_get_remote_node() produces a noisy error message if port >> + * node isn't found and the absence of the port is a legit case here, >> + * so at first we silently check presence of the local port. >> + */ >> + local = of_graph_get_local_port(np); >> + if (!local) >> + return -ENODEV; >> + >> + of_node_put(local); >> + > > The code looks fine, but you may want to take into account my proposal > in 1/7 to instead create a of_graph_has_port() function. The could would > be simpler here. > >> remote = of_graph_get_remote_node(np, port, endpoint); >> if (!remote) >> return -ENODEV; > I like yours proposals and will prepare v8 based on them, thanks!
diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c index b50b44e76279..e0652c38f357 100644 --- a/drivers/gpu/drm/drm_of.c +++ b/drivers/gpu/drm/drm_of.c @@ -239,13 +239,24 @@ int drm_of_find_panel_or_bridge(const struct device_node *np, struct drm_bridge **bridge) { int ret = -EPROBE_DEFER; - struct device_node *remote; + struct device_node *local, *remote; if (!panel && !bridge) return -EINVAL; if (panel) *panel = NULL; + /* + * of_graph_get_remote_node() produces a noisy error message if port + * node isn't found and the absence of the port is a legit case here, + * so at first we silently check presence of the local port. + */ + local = of_graph_get_local_port(np); + if (!local) + return -ENODEV; + + of_node_put(local); + remote = of_graph_get_remote_node(np, port, endpoint); if (!remote) return -ENODEV;