Messages in this thread | | | Date | Tue, 3 Oct 2023 15:22:19 +0200 | From | Neil Armstrong <> | Subject | Re: [PATCH] drm/bridge: ti-sn65dsi86: Associate DSI device lifetime with auxiliary device |
| |
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>
| |