Re: [PATCH v2 17/60] drm/omap: dss: Rework output lookup by port node
Hi,
On Sat, May 26, 2018 at 08:24:35PM +0300, Laurent Pinchart wrote:
> The omap_dss_find_output_by_port_node() function defined in output.c
> looks up an output from its port node. To do so it needs to call helper
> functions from dss-of.c to lookup the port parent and the port number.
> As omap_dss_find_output_by_port_node() is only called by
> omapdss_of_find_source_for_first_ep() from dss-of.c this goes back and
> forth between the to source files and isn't very clear.
>
> Simplify the code by passing both the parent and the port number to
> omap_dss_find_output_by_port_node() instead of the port node, and rename
> the function to omap_dss_find_output_by_port().
>
> Signed-off-by: Laurent Pinchart
> ---
Reviewed-by: Sebastian Reichel
-- Sebastian
> drivers/gpu/drm/omapdrm/dss/dss-of.c | 39
> ---
> drivers/gpu/drm/omapdrm/dss/omapdss.h | 6 ++
> drivers/gpu/drm/omapdrm/dss/output.c | 17 +++
> 3 files changed, 23 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/dss-of.c
> b/drivers/gpu/drm/omapdrm/dss/dss-of.c
> index 4602a79c6c44..b51af09e9111 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dss-of.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dss-of.c
> @@ -21,7 +21,8 @@
>
> #include "omapdss.h"
>
> -struct device_node *dss_of_port_get_parent_device(struct device_node *port)
> +static struct device_node *
> +dss_of_port_get_parent_device(struct device_node *port)
> {
> struct device_node *np;
> int i;
> @@ -45,40 +46,36 @@ struct device_node *dss_of_port_get_parent_device(struct
> device_node *port)
> return NULL;
> }
>
> -u32 dss_of_port_get_port_number(struct device_node *port)
> -{
> - int r;
> - u32 reg;
> -
> - r = of_property_read_u32(port, "reg", ®);
> - if (r)
> - reg = 0;
> -
> - return reg;
> -}
> -
> struct omap_dss_device *
> omapdss_of_find_source_for_first_ep(struct device_node *node)
> {
> - struct device_node *ep;
> + struct device_node *src_node;
> struct device_node *src_port;
> + struct device_node *ep;
> struct omap_dss_device *src;
> + u32 port_number = 0;
>
> + /* Get the endpoint... */
> ep = of_graph_get_endpoint_by_regs(node, 0, 0);
> if (!ep)
> return ERR_PTR(-EINVAL);
>
> + /* ... and its remote port... */
> src_port = of_graph_get_remote_port(ep);
> - if (!src_port) {
> - of_node_put(ep);
> - return ERR_PTR(-EINVAL);
> - }
> -
> of_node_put(ep);
> + if (!src_port)
> + return ERR_PTR(-EINVAL);
>
> - src = omap_dss_find_output_by_port_node(src_port);
> -
> + /* ... and the remote port's number and parent... */
> + of_property_read_u32(src_port, "reg", &port_number);
> + src_node = dss_of_port_get_parent_device(src_port);
> of_node_put(src_port);
> + if (!src_node)
> + return NULL;
> +
> + /* ... and finally the source. */
> + src = omap_dss_find_output_by_port(src_node, port_number);
> + of_node_put(src_node);
>
> return src ? src : ERR_PTR(-EPROBE_DEFER);
> }
> diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> index 1ccf0c67d308..ff0f603bce76 100644
> --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> @@ -576,7 +576,8 @@ int omap_dss_get_num_overlays(void);
> int omapdss_register_output(struct omap_dss_device *output);
> void omapdss_unregister_output(struct omap_dss_device *output);
> struct omap_dss_device *omap_dss_get_output(enum omap_dss_output_id id);
> -struct omap_dss_device *omap_dss_find_output_by_port_node(struct device_node
> *port);
> +struct omap_dss_device *omap_dss_find_output_by_port(struct device_node *src,
> + unsigned int port);
> int omapdss_output_set_device(struct omap_dss_device *out,
> struct omap_dss_device *dssdev);
> int omapdss_output_unset_device(struct omap_dss_device *out);
> @@ -603,9 +604,6 @@ static inline bool omapdss_device_is_enabled(struct
> omap_dss_device *dssdev)
> struct omap_dss_device *
> omapdss_of_find_source_for_first_ep(struct device_node *node);
>
> -struct device_node *dss_of_port_get_parent_device(struct device_node *port);
> -u32 dss_of_port_get_port_number(struct device_node *port);
> -
> enum dss_writeback_channel {
> DSS_WB_LCD1_MGR = 0,
> DSS_WB_LCD2_MGR = 1,
> diff --git a/drivers/gpu/drm/omapdrm/dss/output.c
> b/drivers/gpu/drm/omapdrm/dss/output.c
> index 7f18153a1bde..be254ea42e08 100644
> --- a/drivers/gpu/drm/omapdrm/dss/output.c
> +++ b/drivers/gpu/drm/omapdrm/dss/output.c
> @@ -122,27 +122,16 @@ struct omap_dss_device *omap_dss_get_output(enum
> omap_dss_output_id id)
> }
> EXPORT_SYMBOL(omap_dss_get_output);
>
> -struct omap_dss_device *omap_dss_find_output_by_port_node(struct device_node
> *port)
>
[PATCH v2 17/60] drm/omap: dss: Rework output lookup by port node
The omap_dss_find_output_by_port_node() function defined in output.c
looks up an output from its port node. To do so it needs to call helper
functions from dss-of.c to lookup the port parent and the port number.
As omap_dss_find_output_by_port_node() is only called by
omapdss_of_find_source_for_first_ep() from dss-of.c this goes back and
forth between the to source files and isn't very clear.
Simplify the code by passing both the parent and the port number to
omap_dss_find_output_by_port_node() instead of the port node, and rename
the function to omap_dss_find_output_by_port().
Signed-off-by: Laurent Pinchart
---
drivers/gpu/drm/omapdrm/dss/dss-of.c | 39 ---
drivers/gpu/drm/omapdrm/dss/omapdss.h | 6 ++
drivers/gpu/drm/omapdrm/dss/output.c | 17 +++
3 files changed, 23 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/dss/dss-of.c
b/drivers/gpu/drm/omapdrm/dss/dss-of.c
index 4602a79c6c44..b51af09e9111 100644
--- a/drivers/gpu/drm/omapdrm/dss/dss-of.c
+++ b/drivers/gpu/drm/omapdrm/dss/dss-of.c
@@ -21,7 +21,8 @@
#include "omapdss.h"
-struct device_node *dss_of_port_get_parent_device(struct device_node *port)
+static struct device_node *
+dss_of_port_get_parent_device(struct device_node *port)
{
struct device_node *np;
int i;
@@ -45,40 +46,36 @@ struct device_node *dss_of_port_get_parent_device(struct
device_node *port)
return NULL;
}
-u32 dss_of_port_get_port_number(struct device_node *port)
-{
- int r;
- u32 reg;
-
- r = of_property_read_u32(port, "reg", ®);
- if (r)
- reg = 0;
-
- return reg;
-}
-
struct omap_dss_device *
omapdss_of_find_source_for_first_ep(struct device_node *node)
{
- struct device_node *ep;
+ struct device_node *src_node;
struct device_node *src_port;
+ struct device_node *ep;
struct omap_dss_device *src;
+ u32 port_number = 0;
+ /* Get the endpoint... */
ep = of_graph_get_endpoint_by_regs(node, 0, 0);
if (!ep)
return ERR_PTR(-EINVAL);
+ /* ... and its remote port... */
src_port = of_graph_get_remote_port(ep);
- if (!src_port) {
- of_node_put(ep);
- return ERR_PTR(-EINVAL);
- }
-
of_node_put(ep);
+ if (!src_port)
+ return ERR_PTR(-EINVAL);
- src = omap_dss_find_output_by_port_node(src_port);
-
+ /* ... and the remote port's number and parent... */
+ of_property_read_u32(src_port, "reg", &port_number);
+ src_node = dss_of_port_get_parent_device(src_port);
of_node_put(src_port);
+ if (!src_node)
+ return NULL;
+
+ /* ... and finally the source. */
+ src = omap_dss_find_output_by_port(src_node, port_number);
+ of_node_put(src_node);
return src ? src : ERR_PTR(-EPROBE_DEFER);
}
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h
b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index 1ccf0c67d308..ff0f603bce76 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -576,7 +576,8 @@ int omap_dss_get_num_overlays(void);
int omapdss_register_output(struct omap_dss_device *output);
void omapdss_unregister_output(struct omap_dss_device *output);
struct omap_dss_device *omap_dss_get_output(enum omap_dss_output_id id);
-struct omap_dss_device *omap_dss_find_output_by_port_node(struct device_node
*port);
+struct omap_dss_device *omap_dss_find_output_by_port(struct device_node *src,
+unsigned int port);
int omapdss_output_set_device(struct omap_dss_device *out,
struct omap_dss_device *dssdev);
int omapdss_output_unset_device(struct omap_dss_device *out);
@@ -603,9 +604,6 @@ static inline bool omapdss_device_is_enabled(struct
omap_dss_device *dssdev)
struct omap_dss_device *
omapdss_of_find_source_for_first_ep(struct device_node *node);
-struct device_node *dss_of_port_get_parent_device(struct device_node *port);
-u32 dss_of_port_get_port_number(struct device_node *port);
-
enum dss_writeback_channel {
DSS_WB_LCD1_MGR = 0,
DSS_WB_LCD2_MGR = 1,
diff --git a/drivers/gpu/drm/omapdrm/dss/output.c
b/drivers/gpu/drm/omapdrm/dss/output.c
index 7f18153a1bde..be254ea42e08 100644
--- a/drivers/gpu/drm/omapdrm/dss/output.c
+++ b/drivers/gpu/drm/omapdrm/dss/output.c
@@ -122,27 +122,16 @@ struct omap_dss_device *omap_dss_get_output(enum
omap_dss_output_id id)
}
EXPORT_SYMBOL(omap_dss_get_output);
-struct omap_dss_device *omap_dss_find_output_by_port_node(struct device_node
*port)
+struct omap_dss_device *omap_dss_find_output_by_port(struct device_node *src,
+unsigned int port)
{
- struct device_node *src_node;
struct omap_dss_device *out;
- u32 reg;
-
- src_node = dss_of_port_get_parent_device(p
