[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <270924f7-8e92-a99a-a185-c814cdf1fe01@ti.com>
Date: Mon, 6 Feb 2017 10:31:15 +0200
From: Jyri Sarha <jsarha@...com>
To: Rob Herring <robh@...nel.org>, David Airlie <airlied@...ux.ie>,
Daniel Vetter <daniel.vetter@...el.com>,
Sean Paul <seanpaul@...omium.org>
CC: <dri-devel@...ts.freedesktop.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>,
Frank Rowand <frowand.list@...il.com>,
Boris Brezillon <boris.brezillon@...e-electrons.com>,
Archit Taneja <architt@...eaurora.org>,
Jingoo Han <jingoohan1@...il.com>,
Inki Dae <inki.dae@...sung.com>,
Joonyoung Shim <jy0922.shim@...sung.com>,
Seung-Woo Kim <sw0312.kim@...sung.com>,
Kyungmin Park <kyungmin.park@...sung.com>,
Kukjin Kim <kgene@...nel.org>,
Krzysztof Kozlowski <krzk@...nel.org>,
Javier Martinez Canillas <javier@....samsung.com>,
Stefan Agner <stefan@...er.ch>,
Alison Wang <alison.wang@...escale.com>,
Xinliang Liu <z.liuxinliang@...ilicon.com>,
Rongrong Zou <zourongrong@...il.com>,
Xinwei Kong <kong.kongxinwei@...ilicon.com>,
Chen Feng <puck.chen@...ilicon.com>,
Philipp Zabel <p.zabel@...gutronix.de>,
CK Hu <ck.hu@...iatek.com>,
Matthias Brugger <matthias.bgg@...il.com>,
Marek Vasut <marex@...x.de>,
Mark Yao <mark.yao@...k-chips.com>,
Heiko Stuebner <heiko@...ech.de>,
Maxime Ripard <maxime.ripard@...e-electrons.com>,
Chen-Yu Tsai <wens@...e.org>,
Liviu Dudau <liviu.dudau@....com>,
Mali DP Maintainers <malidp@...s.arm.com>,
Neil Armstrong <narmstrong@...libre.com>,
Carlo Caione <carlo@...one.org>,
Kevin Hilman <khilman@...libre.com>,
Rob Clark <robdclark@...il.com>,
Tomi Valkeinen <tomi.valkeinen@...com>,
Eric Anholt <eric@...olt.net>,
Russell King <rmk+kernel@...linux.org.uk>
Subject: Re: [PATCH 3/5] drm: convert drivers to use of_graph_get_remote_node
Thanks Rob, for nice cleanup, but ...
On 02/04/17 05:36, Rob Herring wrote:
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index 6dfdb145f3bb..e74cc236a79b 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -1013,16 +1013,7 @@ int tilcdc_crtc_create(struct drm_device *dev)
> drm_crtc_helper_add(crtc, &tilcdc_crtc_helper_funcs);
>
> if (priv->is_componentized) {
> - struct device_node *ports =
> - of_get_child_by_name(dev->dev->of_node, "ports");
> -
> - if (ports) {
> - crtc->port = of_get_child_by_name(ports, "port");
> - of_node_put(ports);
> - } else {
> - crtc->port =
> - of_get_child_by_name(dev->dev->of_node, "port");
> - }
> + crtc->port = of_graph_get_port_by_id(dev->dev->of_node, 0, 0);
> if (!crtc->port) { /* This should never happen */
> dev_err(dev->dev, "Port node not found in %s\n",
> dev->dev->of_node->full_name);
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> index c67d7cd7d57e..b7523dce4e8a 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c
> @@ -187,39 +187,6 @@ int tilcdc_attach_bridge(struct drm_device *ddev, struct drm_bridge *bridge)
> return ret;
> }
>
> -static int tilcdc_node_has_port(struct device_node *dev_node)
> -{
> - struct device_node *node;
> -
> - node = of_get_child_by_name(dev_node, "ports");
> - if (!node)
> - node = of_get_child_by_name(dev_node, "port");
> - if (!node)
> - return 0;
> - of_node_put(node);
> -
> - return 1;
> -}
> -
> -static
> -struct device_node *tilcdc_get_remote_node(struct device_node *node)
> -{
> - struct device_node *ep;
> - struct device_node *parent;
> -
> - if (!tilcdc_node_has_port(node))
> - return NULL;
> -
> - ep = of_graph_get_next_endpoint(node, NULL);
> - if (!ep)
> - return NULL;
> -
> - parent = of_graph_get_remote_port_parent(ep);
> - of_node_put(ep);
> -
> - return parent;
> -}
> -
> int tilcdc_attach_external_device(struct drm_device *ddev)
> {
> struct tilcdc_drm_private *priv = ddev->dev_private;
> @@ -227,7 +194,7 @@ int tilcdc_attach_external_device(struct drm_device *ddev)
> struct drm_bridge *bridge;
> int ret;
>
> - remote_node = tilcdc_get_remote_node(ddev->dev->of_node);
> + remote_node = of_graph_get_remote_node(ddev->dev->of_node, 0, 0);
> if (!remote_node)
> return 0;
>
> @@ -266,35 +233,18 @@ int tilcdc_get_external_components(struct device *dev,
> struct component_match **match)
> {
> struct device_node *node;
> - struct device_node *ep = NULL;
> - int count = 0;
> - int ret = 0;
>
> - if (!tilcdc_node_has_port(dev->of_node))
> + if (!match)
> return 0;
This will break tilcdc on setups that use the old tilcdc internal panel-
or tfp410- support. The driver uses tilcdc_get_external_components()
with match == NULL to check if the setup uses graph binding or legacy
internal panel/tfp410 support.
My intention is to get rid off the legacy internal encoder and connector
support (I even did that once already, but DRM had lot of infrastructure
changes in that area and my changes became obsolete), but we still need it.
>
> - while ((ep = of_graph_get_next_endpoint(dev->of_node, ep))) {
> - node = of_graph_get_remote_port_parent(ep);
> - if (!node || !of_device_is_available(node)) {
> - of_node_put(node);
> - continue;
> - }
> -
> - dev_dbg(dev, "Subdevice node '%s' found\n", node->name);
> -
> - if (of_device_is_compatible(node, "nxp,tda998x")) {
> - if (match)
> - drm_of_component_match_add(dev, match,
> - dev_match_of, node);
> - ret = 1;
> - }
> + node = of_graph_get_remote_node(dev->of_node, 0, 0);
>
> + if (!of_device_is_compatible(node, "nxp,tda998x")) {
> of_node_put(node);
> - if (count++ > 1) {
> - dev_err(dev, "Only one port is supported\n");
> - return -EINVAL;
> - }
> + return 0;
> }
>
> - return ret;
Simply remove the above mentioned if statement add and this here:
if (match)
> + drm_of_component_match_add(dev, match, dev_match_of, node);
> + of_node_put(node);
> + return 1;
> }
Best regards,
Jyri
Powered by blists - more mailing lists