Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <b8dfa3de-9f84-27be-fce3-e63d68f99ccc@samsung.com>
Date:   Tue, 24 Aug 2021 15:14:12 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:92.0)
        Gecko/20100101 Thunderbird/92.0
Subject: Re: [PATCH v3 6/8] drm/bridge: ps8640: Register and attach our DSI
 device at probe
Content-Language: en-GB
To:     Maxime Ripard <maxime@cerno.tech>, Jonas Karlman <jonas@kwiboo.se>,
        Sam Ravnborg <sam@ravnborg.org>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        Thierry Reding <thierry.reding@gmail.com>,
        Daniel Vetter <daniel.vetter@intel.com>,
        David Airlie <airlied@linux.ie>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        Neil Armstrong <narmstrong@baylibre.com>,
        Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
        Robert Foss <robert.foss@linaro.org>
Cc:     linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org
From:   Andrzej Hajda <a.hajda@samsung.com>
In-Reply-To: <20210823084723.1493908-7-maxime@cerno.tech>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <20210823084723.1493908-1-maxime@cerno.tech>
        <CGME20210823084802eucas1p2a46bef1b5dd44f1738761f1ae605722c@eucas1p2.samsung.com>
        <20210823084723.1493908-7-maxime@cerno.tech>
Precedence: bulk


W dniu 23.08.2021 o 10:47, Maxime Ripard pisze:
> In order to avoid any probe ordering issue, the best practice is to move
> the secondary MIPI-DSI device registration and attachment to the
> MIPI-DSI host at probe time. Let's do this.
>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>   drivers/gpu/drm/bridge/parade-ps8640.c | 93 ++++++++++++++------------
>   1 file changed, 51 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
> index 794c9516b05d..37f7d166a3c6 100644
> --- a/drivers/gpu/drm/bridge/parade-ps8640.c
> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> @@ -213,52 +213,10 @@ static int ps8640_bridge_attach(struct drm_bridge *bridge,
>   				enum drm_bridge_attach_flags flags)
>   {
>   	struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
> -	struct device *dev = &ps_bridge->page[0]->dev;
> -	struct device_node *in_ep, *dsi_node;
> -	struct mipi_dsi_device *dsi;
> -	struct mipi_dsi_host *host;
> -	int ret;
> -	const struct mipi_dsi_device_info info = { .type = "ps8640",
> -						   .channel = 0,
> -						   .node = NULL,
> -						 };
>   
>   	if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR))
>   		return -EINVAL;
>   
> -	/* port@0 is ps8640 dsi input port */
> -	in_ep = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1);
> -	if (!in_ep)
> -		return -ENODEV;
> -
> -	dsi_node = of_graph_get_remote_port_parent(in_ep);
> -	of_node_put(in_ep);
> -	if (!dsi_node)
> -		return -ENODEV;
> -
> -	host = of_find_mipi_dsi_host_by_node(dsi_node);
> -	of_node_put(dsi_node);
> -	if (!host)
> -		return -ENODEV;
> -
> -	dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
> -	if (IS_ERR(dsi)) {
> -		dev_err(dev, "failed to create dsi device\n");
> -		ret = PTR_ERR(dsi);
> -		return ret;
> -	}
> -
> -	ps_bridge->dsi = dsi;
> -
> -	dsi->host = host;
> -	dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
> -			  MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> -	dsi->format = MIPI_DSI_FMT_RGB888;
> -	dsi->lanes = DP_NUM_LANES;
> -	ret = devm_mipi_dsi_attach(dev, dsi);
> -	if (ret)
> -		return ret;
> -
>   	/* Attach the panel-bridge to the dsi bridge */
>   	return drm_bridge_attach(bridge->encoder, ps_bridge->panel_bridge,
>   				 &ps_bridge->bridge, flags);
> @@ -305,6 +263,53 @@ static const struct drm_bridge_funcs ps8640_bridge_funcs = {
>   	.pre_enable = ps8640_pre_enable,
>   };
>   
> +static int ps8640_bridge_host_attach(struct device *dev, struct ps8640 *ps_bridge)
> +{
> +	struct device_node *in_ep, *dsi_node;
> +	struct mipi_dsi_device *dsi;
> +	struct mipi_dsi_host *host;
> +	int ret;
> +	const struct mipi_dsi_device_info info = { .type = "ps8640",
> +						   .channel = 0,
> +						   .node = NULL,
> +						 };
> +
> +	/* port@0 is ps8640 dsi input port */
> +	in_ep = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1);
> +	if (!in_ep)
> +		return -ENODEV;
> +
> +	dsi_node = of_graph_get_remote_port_parent(in_ep);
> +	of_node_put(in_ep);
> +	if (!dsi_node)
> +		return -ENODEV;
> +
> +	host = of_find_mipi_dsi_host_by_node(dsi_node);
> +	of_node_put(dsi_node);
> +	if (!host)
> +		return -EPROBE_DEFER;
> +
> +	dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
> +	if (IS_ERR(dsi)) {
> +		dev_err(dev, "failed to create dsi device\n");
> +		return PTR_ERR(dsi);
> +	}
> +
> +	ps_bridge->dsi = dsi;
> +
> +	dsi->host = host;
> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO |
> +			  MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->lanes = DP_NUM_LANES;
> +
> +	ret = devm_mipi_dsi_attach(dev, dsi);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
>   static int ps8640_probe(struct i2c_client *client)
>   {
>   	struct device *dev = &client->dev;
> @@ -371,6 +376,10 @@ static int ps8640_probe(struct i2c_client *client)
>   
>   	drm_bridge_add(&ps_bridge->bridge);
>   
> +	ret = ps8640_bridge_host_attach(dev, ps_bridge);
> +	if (ret)
> +		return ret;


I do not see drm_bridge_remove on error path, the same for sn65dsi83.


Regards

Andrzej


> +
>   	return 0;
>   }
>