Hi!
This small series improves DRM bridges code by silencing a noisy error coming from of-graph code for the device-trees that are missing a display bridge graph.
graph: no port node found in ...
One example where this error happens is an older bridge-less DTB used in conjunction with a newer kernel which has a display controller driver that supports DRM bridges.
Changelog:
v9: - These two patches are factored out from [1] in order to ease applying of the patches.
- The of_graph_presents() is renamed to of_graph_is_present() like it was requested by Rob Herring in the review comment to [1].
- Added Rob's r-b.
[1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=184102
Dmitry Osipenko (2): of_graph: add of_graph_is_present() drm/of: Make drm_of_find_panel_or_bridge() to check graph's presence
drivers/gpu/drm/drm_of.c | 9 +++++++ drivers/of/property.c | 52 +++++++++++++++++++++++++++++++++------- include/linux/of_graph.h | 6 +++++ 3 files changed, 58 insertions(+), 9 deletions(-)
In some case, like a DRM display code for example, it's useful to silently check whether port node exists at all in a device-tree before proceeding with parsing of the graph.
This patch adds of_graph_is_present() which returns true if given device-tree node contains OF graph port.
Reviewed-by: Rob Herring robh@kernel.org Signed-off-by: Dmitry Osipenko digetx@gmail.com --- drivers/of/property.c | 52 +++++++++++++++++++++++++++++++++------- include/linux/of_graph.h | 6 +++++ 2 files changed, 49 insertions(+), 9 deletions(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c index 6a5760f0d6cd..e12b8b491837 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -29,6 +29,48 @@
#include "of_private.h"
+/** + * of_graph_get_first_local_port() - get first local port node + * @node: pointer to a local endpoint device_node + * + * Return: First local port node associated with local endpoint node linked + * to @node. Use of_node_put() on it when done. + */ +static struct device_node * +of_graph_get_first_local_port(const struct device_node *node) +{ + struct device_node *ports, *port; + + ports = of_get_child_by_name(node, "ports"); + if (ports) + node = ports; + + port = of_get_child_by_name(node, "port"); + of_node_put(ports); + + return port; +} + +/** + * of_graph_is_present() - check graph's presence + * @node: pointer to a device_node checked for the graph's presence + * + * Return: True if @node has a port or ports sub-node, false otherwise. + */ +bool of_graph_is_present(const struct device_node *node) +{ + struct device_node *local; + + local = of_graph_get_first_local_port(node); + if (!local) + return false; + + of_node_put(local); + + return true; +} +EXPORT_SYMBOL(of_graph_is_present); + /** * of_property_count_elems_of_size - Count the number of elements in a property * @@ -608,15 +650,7 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent, * parent port node. */ if (!prev) { - struct device_node *node; - - node = of_get_child_by_name(parent, "ports"); - if (node) - parent = node; - - port = of_get_child_by_name(parent, "port"); - of_node_put(node); - + port = of_graph_get_first_local_port(parent); if (!port) { pr_err("graph: no port node found in %pOF\n", parent); return NULL; diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h index 01038a6aade0..4d7756087b6b 100644 --- a/include/linux/of_graph.h +++ b/include/linux/of_graph.h @@ -38,6 +38,7 @@ struct of_endpoint { child = of_graph_get_next_endpoint(parent, child))
#ifdef CONFIG_OF +bool of_graph_is_present(const struct device_node *node); int of_graph_parse_endpoint(const struct device_node *node, struct of_endpoint *endpoint); int of_graph_get_endpoint_count(const struct device_node *np); @@ -56,6 +57,11 @@ struct device_node *of_graph_get_remote_node(const struct device_node *node, u32 port, u32 endpoint); #else
+static inline bool of_graph_is_present(const struct device_node *node) +{ + return false; +} + static inline int of_graph_parse_endpoint(const struct device_node *node, struct of_endpoint *endpoint) {
Hi Dmitry,
Thank you for the patch.
On Wed, Jul 01, 2020 at 05:16:16AM +0300, Dmitry Osipenko wrote:
In some case, like a DRM display code for example, it's useful to silently check whether port node exists at all in a device-tree before proceeding with parsing of the graph.
This patch adds of_graph_is_present() which returns true if given device-tree node contains OF graph port.
Reviewed-by: Rob Herring robh@kernel.org Signed-off-by: Dmitry Osipenko digetx@gmail.com
drivers/of/property.c | 52 +++++++++++++++++++++++++++++++++------- include/linux/of_graph.h | 6 +++++ 2 files changed, 49 insertions(+), 9 deletions(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c index 6a5760f0d6cd..e12b8b491837 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -29,6 +29,48 @@
#include "of_private.h"
+/**
- of_graph_get_first_local_port() - get first local port node
- @node: pointer to a local endpoint device_node
It's not an endpoint.
- Return: First local port node associated with local endpoint node linked
to @node. Use of_node_put() on it when done.
- */
+static struct device_node * +of_graph_get_first_local_port(const struct device_node *node) +{
- struct device_node *ports, *port;
- ports = of_get_child_by_name(node, "ports");
- if (ports)
node = ports;
- port = of_get_child_by_name(node, "port");
- of_node_put(ports);
- return port;
+}
+/**
- of_graph_is_present() - check graph's presence
- @node: pointer to a device_node checked for the graph's presence
- Return: True if @node has a port or ports sub-node, false otherwise.
- */
+bool of_graph_is_present(const struct device_node *node) +{
- struct device_node *local;
- local = of_graph_get_first_local_port(node);
- if (!local)
return false;
- of_node_put(local);
- return true;
+} +EXPORT_SYMBOL(of_graph_is_present);
/**
- of_property_count_elems_of_size - Count the number of elements in a property
@@ -608,15 +650,7 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent, * parent port node. */ if (!prev) {
struct device_node *node;
node = of_get_child_by_name(parent, "ports");
if (node)
parent = node;
port = of_get_child_by_name(parent, "port");
of_node_put(node);
port = of_graph_get_first_local_port(parent);
I think this introduces a bug below in the function, where parent is used and is expected to point to the ports node if available. I'd leave this part of the change out, and inline +of_graph_get_first_local_port() in of_graph_is_present().
if (!port) { pr_err("graph: no port node found in %pOF\n", parent); return NULL;
diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h index 01038a6aade0..4d7756087b6b 100644 --- a/include/linux/of_graph.h +++ b/include/linux/of_graph.h @@ -38,6 +38,7 @@ struct of_endpoint { child = of_graph_get_next_endpoint(parent, child))
#ifdef CONFIG_OF +bool of_graph_is_present(const struct device_node *node); int of_graph_parse_endpoint(const struct device_node *node, struct of_endpoint *endpoint); int of_graph_get_endpoint_count(const struct device_node *np); @@ -56,6 +57,11 @@ struct device_node *of_graph_get_remote_node(const struct device_node *node, u32 port, u32 endpoint); #else
+static inline bool of_graph_is_present(const struct device_node *node) +{
- return false;
+}
static inline int of_graph_parse_endpoint(const struct device_node *node, struct of_endpoint *endpoint) {
01.07.2020 08:45, Laurent Pinchart пишет:
Hi Dmitry,
Thank you for the patch.
On Wed, Jul 01, 2020 at 05:16:16AM +0300, Dmitry Osipenko wrote:
In some case, like a DRM display code for example, it's useful to silently check whether port node exists at all in a device-tree before proceeding with parsing of the graph.
This patch adds of_graph_is_present() which returns true if given device-tree node contains OF graph port.
Reviewed-by: Rob Herring robh@kernel.org Signed-off-by: Dmitry Osipenko digetx@gmail.com
drivers/of/property.c | 52 +++++++++++++++++++++++++++++++++------- include/linux/of_graph.h | 6 +++++ 2 files changed, 49 insertions(+), 9 deletions(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c index 6a5760f0d6cd..e12b8b491837 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -29,6 +29,48 @@
#include "of_private.h"
+/**
- of_graph_get_first_local_port() - get first local port node
- @node: pointer to a local endpoint device_node
It's not an endpoint.
Alright, somehow I was reading this as a "device_node that contains endpoint (of the graph)". But after re-reading twice I can see that it's not the case.
It should be a "pointer to device_node containing graph port", just like the of_graph_get_remote_node() says it. Thank you :)
- Return: First local port node associated with local endpoint node linked
to @node. Use of_node_put() on it when done.
- */
+static struct device_node * +of_graph_get_first_local_port(const struct device_node *node) +{
- struct device_node *ports, *port;
- ports = of_get_child_by_name(node, "ports");
- if (ports)
node = ports;
- port = of_get_child_by_name(node, "port");
- of_node_put(ports);
- return port;
+}
+/**
- of_graph_is_present() - check graph's presence
- @node: pointer to a device_node checked for the graph's presence
- Return: True if @node has a port or ports sub-node, false otherwise.
- */
+bool of_graph_is_present(const struct device_node *node) +{
- struct device_node *local;
- local = of_graph_get_first_local_port(node);
- if (!local)
return false;
- of_node_put(local);
- return true;
+} +EXPORT_SYMBOL(of_graph_is_present);
/**
- of_property_count_elems_of_size - Count the number of elements in a property
@@ -608,15 +650,7 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent, * parent port node. */ if (!prev) {
struct device_node *node;
node = of_get_child_by_name(parent, "ports");
if (node)
parent = node;
port = of_get_child_by_name(parent, "port");
of_node_put(node);
port = of_graph_get_first_local_port(parent);
I think this introduces a bug below in the function, where parent is used and is expected to point to the ports node if available. I'd leave this part of the change out, and inline +of_graph_get_first_local_port() in of_graph_is_present().
Good catch! I'll correct this.
Thank you for the review :)
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 the graph's presence in a device-tree before proceeding with parsing of the graph.
Reviewed-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Dmitry Osipenko digetx@gmail.com --- drivers/gpu/drm/drm_of.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c index b50b44e76279..fdb05fbf72a0 100644 --- a/drivers/gpu/drm/drm_of.c +++ b/drivers/gpu/drm/drm_of.c @@ -246,6 +246,15 @@ int drm_of_find_panel_or_bridge(const struct device_node *np, 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 whether graph presents in the + * device-tree node. + */ + if (!of_graph_is_present(np)) + return -ENODEV; + remote = of_graph_get_remote_node(np, port, endpoint); if (!remote) return -ENODEV;
dri-devel@lists.freedesktop.org