Re: [PATCH] drm/bridge: ti-sn65dsi86: Associate DSI device lifetime with auxiliary device

[Neil Armstrong <neil.armstrong@linaro.org>]

On 03/10/2023 01:54, Stephen Boyd wrote:
> The kernel produces a warning splat and the DSI device fails to register
> in this driver if the i2c driver probes, populates child auxiliary
> devices, and then somewhere in ti_sn_bridge_probe() a function call
> returns -EPROBE_DEFER. When the auxiliary driver probe defers, the dsi
> device created by devm_mipi_dsi_device_register_full() is left
> registered because the devm managed device used to manage the lifetime
> of the DSI device is the parent i2c device, not the auxiliary device
> that is being probed.
> 
> Associate the DSI device created and managed by this driver to the
> lifetime of the auxiliary device, not the i2c device, so that the DSI
> device is removed when the auxiliary driver unbinds. Similarly change
> the device pointer used for dev_err_probe() so the deferred probe errors
> are associated with the auxiliary device instead of the parent i2c
> device so we can narrow down future problems faster.
> 
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Maxime Ripard <maxime@cerno.tech>
> Fixes: c3b75d4734cb ("drm/bridge: sn65dsi86: Register and attach our DSI device at probe")
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>   drivers/gpu/drm/bridge/ti-sn65dsi86.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index f448b903e190..84148a79414b 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -692,7 +692,7 @@ static struct ti_sn65dsi86 *bridge_to_ti_sn65dsi86(struct drm_bridge *bridge)
>   	return container_of(bridge, struct ti_sn65dsi86, bridge);
>   }
>   
> -static int ti_sn_attach_host(struct ti_sn65dsi86 *pdata)
> +static int ti_sn_attach_host(struct auxiliary_device *adev, struct ti_sn65dsi86 *pdata)
>   {
>   	int val;
>   	struct mipi_dsi_host *host;
> @@ -707,7 +707,7 @@ static int ti_sn_attach_host(struct ti_sn65dsi86 *pdata)
>   	if (!host)
>   		return -EPROBE_DEFER;
>   
> -	dsi = devm_mipi_dsi_device_register_full(dev, host, &info);
> +	dsi = devm_mipi_dsi_device_register_full(&adev->dev, host, &info);
>   	if (IS_ERR(dsi))
>   		return PTR_ERR(dsi);
>   
> @@ -725,7 +725,7 @@ static int ti_sn_attach_host(struct ti_sn65dsi86 *pdata)
>   
>   	pdata->dsi = dsi;
>   
> -	return devm_mipi_dsi_attach(dev, dsi);
> +	return devm_mipi_dsi_attach(&adev->dev, dsi);
>   }
>   
>   static int ti_sn_bridge_attach(struct drm_bridge *bridge,
> @@ -1298,9 +1298,9 @@ static int ti_sn_bridge_probe(struct auxiliary_device *adev,
>   	struct device_node *np = pdata->dev->of_node;
>   	int ret;
>   
> -	pdata->next_bridge = devm_drm_of_get_bridge(pdata->dev, np, 1, 0);
> +	pdata->next_bridge = devm_drm_of_get_bridge(&adev->dev, np, 1, 0);
>   	if (IS_ERR(pdata->next_bridge))
> -		return dev_err_probe(pdata->dev, PTR_ERR(pdata->next_bridge),
> +		return dev_err_probe(&adev->dev, PTR_ERR(pdata->next_bridge),
>   				     "failed to create panel bridge\n");
>   
>   	ti_sn_bridge_parse_lanes(pdata, np);
> @@ -1319,9 +1319,9 @@ static int ti_sn_bridge_probe(struct auxiliary_device *adev,
>   
>   	drm_bridge_add(&pdata->bridge);
>   
> -	ret = ti_sn_attach_host(pdata);
> +	ret = ti_sn_attach_host(adev, pdata);
>   	if (ret) {
> -		dev_err_probe(pdata->dev, ret, "failed to attach dsi host\n");
> +		dev_err_probe(&adev->dev, ret, "failed to attach dsi host\n");
>   		goto err_remove_bridge;
>   	}
>   
> 
> base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d

This looks reasonable

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>