Silence missing-graph error for DRM bridges
Subsystems (4)
v9 cover Silence missing-graph error for DRM bridges
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(-)
Signature · 1 line
2.26.0
PATCH v9 1/2 of_graph: add of_graph_is_present()
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.
--- drivers/of/property.c | 52 +++++++++++++++++++++++++++++++++------- include/linux/of_graph.h | 6 +++++ 2 files changed, 49 insertions(+), 9 deletions(-)
drivers/of/property.c +43 -9
@@ -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 ;
include/linux/of_graph.h +6 -0
@@ -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) {
Signature · 1 line
2.26.0
Re: [PATCH v9 1/2] of_graph: add of_graph_is_present()
Hi Dmitry,
Thank you for the patch.
Quoted 19 lines · Dmitry Osipenko
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 <[email protected]>Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/of/property.c | 52 +++++++++++++++++++++++++++++++++------- include/linux/of_graph.h | 6 +++++ 2 files changed, 49 insertions(+), 9 deletions(-)drivers/of/property.c +0
-0
drivers/of/property.c line 34
Quoted 6 lines
drivers/of/property.c +3
-0@@ -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.
Quoted 46 lines
drivers/of/property.c +39
-0+* +* 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) {
drivers/of/property.c struct device_node *of_graph_get_next_endpoint(const struct device_node *parent, · line 653
Quoted 10 lines
drivers/of/property.c +1
-9-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().
Quoted 27 lines
drivers/of/property.c +0
-0if (!port) {pr_err ("graph: no port node found in %pOF\n" , parent);return NULL ;include/linux/of_graph.h +6
-0@@ -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) {
Signature · 3 lines
Regards, Laurent Pinchart
Re: [PATCH v9 1/2] of_graph: add of_graph_is_present()
01.07.2020 08:45, Laurent Pinchart пишет:
Quoted 25 lines
Hi Dmitry,
Thank you for the patch.
Quoted 20 lines · Dmitry Osipenko
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 <[email protected]>Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/of/property.c | 52 +++++++++++++++++++++++++++++++++------- include/linux/of_graph.h | 6 +++++ 2 files changed, 49 insertions(+), 9 deletions(-)drivers/of/property.c +0
-0@@ -29,6 +29,48 @@
Quoted 7 lines
drivers/of/property.c line 34
Quoted 5 lines
drivers/of/property.c +3
-0#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 :)
Quoted 53 lines
Quoted 53 lines
drivers/of/property.c +39
-7+* +* 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" );
Quoted 8 lines
drivers/of/property.c struct device_node *of_graph_get_next_endpoint(const struct device_node *parent, · line 653
Quoted 3 lines
drivers/of/property.c +1
-2-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 :)
PATCH v9 2/2 drm/of: Make drm_of_find_panel_or_bridge() to check graph's presence
In reply to Dmitry OsipenkoWhen 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.
--- drivers/gpu/drm/drm_of.c | 9 +++++++++ 1 file changed, 9 insertions(+)
drivers/gpu/drm/drm_of.c +9 -0
@@ -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 ;
Signature · 1 line
2.26.0