Let's switch to the new devm MIPI-DSI function to register and attach our secondary device. This also avoids leaking the device when we detach the bridge.

Acked-by: Sam Ravnborg sam@ravnborg.org Tested-by: John Stultz john.stultz@linaro.org Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/adv7511/adv7511.h | 1 - drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 -- drivers/gpu/drm/bridge/adv7511/adv7533.c | 20 ++++---------------- 3 files changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 05e3abb5a0c9..592ecfcf00ca 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h @@ -401,7 +401,6 @@ void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode *mode); int adv7533_patch_registers(struct adv7511 *adv); int adv7533_patch_cec_registers(struct adv7511 *adv); int adv7533_attach_dsi(struct adv7511 *adv); -void adv7533_detach_dsi(struct adv7511 *adv); int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv);

#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 76555ae64e9c..9e3585f23cf1 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -1307,8 +1307,6 @@ static int adv7511_remove(struct i2c_client *i2c) { struct adv7511 *adv7511 = i2c_get_clientdata(i2c);

- if (adv7511->type == ADV7533 || adv7511->type == ADV7535) - adv7533_detach_dsi(adv7511); i2c_unregister_device(adv7511->i2c_cec); clk_disable_unprepare(adv7511->cec_clk);

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c index 59d718bde8c4..eb7579dec40a 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c @@ -153,11 +153,10 @@ int adv7533_attach_dsi(struct adv7511 *adv) return -EPROBE_DEFER; }

- dsi = mipi_dsi_device_register_full(host, &info); + 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); - goto err_dsi_device; + return PTR_ERR(dsi); }

adv->dsi = dsi; @@ -167,24 +166,13 @@ int adv7533_attach_dsi(struct adv7511 *adv) dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE | MIPI_DSI_MODE_NO_EOT_PACKET | MIPI_DSI_MODE_VIDEO_HSE;

- ret = mipi_dsi_attach(dsi); + ret = devm_mipi_dsi_attach(dev, dsi); if (ret < 0) { dev_err(dev, "failed to attach dsi to host\n"); - goto err_dsi_attach; + return ret; }

return 0; - -err_dsi_attach: - mipi_dsi_device_unregister(dsi); -err_dsi_device: - return ret; -} - -void adv7533_detach_dsi(struct adv7511 *adv) -{ - mipi_dsi_detach(adv->dsi); - mipi_dsi_device_unregister(adv->dsi); }

int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv)

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.

Acked-by: Sam Ravnborg sam@ravnborg.org Tested-by: John Stultz john.stultz@linaro.org Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 9e3585f23cf1..f8e5da148599 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -910,9 +910,6 @@ static int adv7511_bridge_attach(struct drm_bridge *bridge, return ret; }

- if (adv->type == ADV7533 || adv->type == ADV7535) - ret = adv7533_attach_dsi(adv); - if (adv->i2c_main->irq) regmap_write(adv->regmap, ADV7511_REG_INT_ENABLE(0), ADV7511_INT0_HPD); @@ -1288,8 +1285,18 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) drm_bridge_add(&adv7511->bridge);

adv7511_audio_init(dev, adv7511); + + if (adv7511->type == ADV7533 || adv7511->type == ADV7535) { + ret = adv7533_attach_dsi(adv7511); + if (ret) + goto err_unregister_audio; + } + return 0;

+err_unregister_audio: + adv7511_audio_exit(adv7511); + drm_bridge_remove(&adv7511->bridge); err_unregister_cec: i2c_unregister_device(adv7511->i2c_cec); clk_disable_unprepare(adv7511->cec_clk);

Let's switch to the new devm MIPI-DSI function to register and attach our secondary device.

Acked-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/analogix/anx7625.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 1a871f6b6822..4adeb2bad03a 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -1316,6 +1316,7 @@ static int anx7625_attach_dsi(struct anx7625_data *ctx) .channel = 0, .node = NULL, }; + int ret;

DRM_DEV_DEBUG_DRIVER(dev, "attach dsi\n");

@@ -1325,7 +1326,7 @@ static int anx7625_attach_dsi(struct anx7625_data *ctx) return -EINVAL; }

- dsi = mipi_dsi_device_register_full(host, &info); + dsi = devm_mipi_dsi_device_register_full(dev, host, &info); if (IS_ERR(dsi)) { DRM_DEV_ERROR(dev, "fail to create dsi device.\n"); return -EINVAL; @@ -1337,10 +1338,10 @@ static int anx7625_attach_dsi(struct anx7625_data *ctx) MIPI_DSI_MODE_VIDEO_SYNC_PULSE | MIPI_DSI_MODE_VIDEO_HSE;

- if (mipi_dsi_attach(dsi) < 0) { + ret = devm_mipi_dsi_attach(dev, dsi); + if (ret) { DRM_DEV_ERROR(dev, "fail to attach dsi to host.\n"); - mipi_dsi_device_unregister(dsi); - return -EINVAL; + return ret; }

ctx->dsi = dsi; @@ -1350,16 +1351,6 @@ static int anx7625_attach_dsi(struct anx7625_data *ctx) return 0; }

-static void anx7625_bridge_detach(struct drm_bridge *bridge) -{ - struct anx7625_data *ctx = bridge_to_anx7625(bridge); - - if (ctx->dsi) { - mipi_dsi_detach(ctx->dsi); - mipi_dsi_device_unregister(ctx->dsi); - } -} - static int anx7625_bridge_attach(struct drm_bridge *bridge, enum drm_bridge_attach_flags flags) { @@ -1624,7 +1615,6 @@ static struct edid *anx7625_bridge_get_edid(struct drm_bridge *bridge,

static const struct drm_bridge_funcs anx7625_bridge_funcs = { .attach = anx7625_bridge_attach, - .detach = anx7625_bridge_detach, .disable = anx7625_bridge_disable, .mode_valid = anx7625_bridge_mode_valid, .mode_set = anx7625_bridge_mode_set,

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.

Acked-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/analogix/anx7625.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 4adeb2bad03a..d0317651cd75 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -1367,12 +1367,6 @@ static int anx7625_bridge_attach(struct drm_bridge *bridge, return -ENODEV; }

- err = anx7625_attach_dsi(ctx); - if (err) { - DRM_DEV_ERROR(dev, "Fail to attach to dsi : %d\n", err); - return err; - } - if (ctx->pdata.panel_bridge) { err = drm_bridge_attach(bridge->encoder, ctx->pdata.panel_bridge, @@ -1845,10 +1839,24 @@ static int anx7625_i2c_probe(struct i2c_client *client, platform->bridge.type = DRM_MODE_CONNECTOR_eDP; drm_bridge_add(&platform->bridge);

+ ret = anx7625_attach_dsi(platform); + if (ret) { + DRM_DEV_ERROR(dev, "Fail to attach to dsi : %d\n", ret); + goto unregister_bridge; + } + DRM_DEV_DEBUG_DRIVER(dev, "probe done\n");

return 0;

+unregister_bridge: + drm_bridge_remove(&platform->bridge); + + if (!platform->pdata.low_power_mode) + pm_runtime_put_sync_suspend(&client->dev); + + anx7625_unregister_i2c_dummy_clients(platform); + free_wq: if (platform->workqueue) destroy_workqueue(platform->workqueue);

Let's switch to the new devm MIPI-DSI function to register and attach our secondary device.

Acked-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/lontium-lt8912b.c | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c b/drivers/gpu/drm/bridge/lontium-lt8912b.c index 1b0c7eaf6c84..cc968d65936b 100644 --- a/drivers/gpu/drm/bridge/lontium-lt8912b.c +++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c @@ -472,11 +472,11 @@ static int lt8912_attach_dsi(struct lt8912 *lt) return -EPROBE_DEFER; }

- dsi = mipi_dsi_device_register_full(host, &info); + dsi = devm_mipi_dsi_device_register_full(dev, host, &info); if (IS_ERR(dsi)) { ret = PTR_ERR(dsi); dev_err(dev, "failed to create dsi device (%d)\n", ret); - goto err_dsi_device; + return ret; }

lt->dsi = dsi; @@ -489,24 +489,13 @@ static int lt8912_attach_dsi(struct lt8912 *lt) MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET;

- ret = mipi_dsi_attach(dsi); + ret = devm_mipi_dsi_attach(dev, dsi); if (ret < 0) { dev_err(dev, "failed to attach dsi to host\n"); - goto err_dsi_attach; + return ret; }

return 0; - -err_dsi_attach: - mipi_dsi_device_unregister(dsi); -err_dsi_device: - return ret; -} - -static void lt8912_detach_dsi(struct lt8912 *lt) -{ - mipi_dsi_detach(lt->dsi); - mipi_dsi_device_unregister(lt->dsi); }

static int lt8912_bridge_connector_init(struct drm_bridge *bridge) @@ -573,7 +562,6 @@ static void lt8912_bridge_detach(struct drm_bridge *bridge) struct lt8912 *lt = bridge_to_lt8912(bridge);

if (lt->is_attached) { - lt8912_detach_dsi(lt); lt8912_hard_power_off(lt); drm_connector_unregister(&lt->connector); drm_connector_cleanup(&lt->connector);

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.

Acked-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/lontium-lt8912b.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt8912b.c b/drivers/gpu/drm/bridge/lontium-lt8912b.c index cc968d65936b..c642d1e02b2f 100644 --- a/drivers/gpu/drm/bridge/lontium-lt8912b.c +++ b/drivers/gpu/drm/bridge/lontium-lt8912b.c @@ -544,10 +544,6 @@ static int lt8912_bridge_attach(struct drm_bridge *bridge, if (ret) goto error;

- ret = lt8912_attach_dsi(lt); - if (ret) - goto error; - lt->is_attached = true;

return 0; @@ -706,8 +702,15 @@ static int lt8912_probe(struct i2c_client *client,

drm_bridge_add(&lt->bridge);

+ ret = lt8912_attach_dsi(lt); + if (ret) + goto err_attach; + return 0;

+err_attach: + drm_bridge_remove(&lt->bridge); + lt8912_free_i2c(lt); err_i2c: lt8912_put_dt(lt); err_dt_parse:

Let's switch to the new devm MIPI-DSI function to register and attach our secondary device.

Acked-by: Sam Ravnborg sam@ravnborg.org Tested-by: John Stultz john.stultz@linaro.org Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/lontium-lt9611.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c index 29b1ce2140ab..654131aca5ed 100644 --- a/drivers/gpu/drm/bridge/lontium-lt9611.c +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c @@ -760,6 +760,7 @@ static struct mipi_dsi_device *lt9611_attach_dsi(struct lt9611 *lt9611, const struct mipi_dsi_device_info info = { "lt9611", 0, NULL }; struct mipi_dsi_device *dsi; struct mipi_dsi_host *host; + struct device *dev = lt9611->dev; int ret;

host = of_find_mipi_dsi_host_by_node(dsi_node); @@ -768,7 +769,7 @@ static struct mipi_dsi_device *lt9611_attach_dsi(struct lt9611 *lt9611, return ERR_PTR(-EPROBE_DEFER); }

- dsi = mipi_dsi_device_register_full(host, &info); + dsi = devm_mipi_dsi_device_register_full(dev, host, &info); if (IS_ERR(dsi)) { dev_err(lt9611->dev, "failed to create dsi device\n"); return dsi; @@ -779,29 +780,15 @@ static struct mipi_dsi_device *lt9611_attach_dsi(struct lt9611 *lt9611, dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE | MIPI_DSI_MODE_VIDEO_HSE;

- ret = mipi_dsi_attach(dsi); + ret = devm_mipi_dsi_attach(dev, dsi); if (ret < 0) { - dev_err(lt9611->dev, "failed to attach dsi to host\n"); - mipi_dsi_device_unregister(dsi); + dev_err(dev, "failed to attach dsi to host\n"); return ERR_PTR(ret); }

return dsi; }

-static void lt9611_bridge_detach(struct drm_bridge *bridge) -{ - struct lt9611 *lt9611 = bridge_to_lt9611(bridge); - - if (lt9611->dsi1) { - mipi_dsi_detach(lt9611->dsi1); - mipi_dsi_device_unregister(lt9611->dsi1); - } - - mipi_dsi_detach(lt9611->dsi0); - mipi_dsi_device_unregister(lt9611->dsi0); -} - static int lt9611_connector_init(struct drm_bridge *bridge, struct lt9611 *lt9611) { int ret; @@ -855,9 +842,7 @@ static int lt9611_bridge_attach(struct drm_bridge *bridge, return 0;

err_unregister_dsi0: - lt9611_bridge_detach(bridge); drm_connector_cleanup(&lt9611->connector); - mipi_dsi_device_unregister(lt9611->dsi0);

return ret; } @@ -952,7 +937,6 @@ static void lt9611_bridge_hpd_enable(struct drm_bridge *bridge)

static const struct drm_bridge_funcs lt9611_bridge_funcs = { .attach = lt9611_bridge_attach, - .detach = lt9611_bridge_detach, .mode_valid = lt9611_bridge_mode_valid, .enable = lt9611_bridge_enable, .disable = lt9611_bridge_disable,

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.

Acked-by: Sam Ravnborg sam@ravnborg.org Tested-by: John Stultz john.stultz@linaro.org Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/lontium-lt9611.c | 38 ++++++++++++------------- 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c index 654131aca5ed..d2f45a0f79c8 100644 --- a/drivers/gpu/drm/bridge/lontium-lt9611.c +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c @@ -825,26 +825,7 @@ static int lt9611_bridge_attach(struct drm_bridge *bridge, return ret; }

- /* Attach primary DSI */ - lt9611->dsi0 = lt9611_attach_dsi(lt9611, lt9611->dsi0_node); - if (IS_ERR(lt9611->dsi0)) - return PTR_ERR(lt9611->dsi0); - - /* Attach secondary DSI, if specified */ - if (lt9611->dsi1_node) { - lt9611->dsi1 = lt9611_attach_dsi(lt9611, lt9611->dsi1_node); - if (IS_ERR(lt9611->dsi1)) { - ret = PTR_ERR(lt9611->dsi1); - goto err_unregister_dsi0; - } - } - return 0; - -err_unregister_dsi0: - drm_connector_cleanup(&lt9611->connector); - - return ret; }

static enum drm_mode_status lt9611_bridge_mode_valid(struct drm_bridge *bridge, @@ -1165,10 +1146,29 @@ static int lt9611_probe(struct i2c_client *client,

drm_bridge_add(&lt9611->bridge);

+ /* Attach primary DSI */ + lt9611->dsi0 = lt9611_attach_dsi(lt9611, lt9611->dsi0_node); + if (IS_ERR(lt9611->dsi0)) { + ret = PTR_ERR(lt9611->dsi0); + goto err_remove_bridge; + } + + /* Attach secondary DSI, if specified */ + if (lt9611->dsi1_node) { + lt9611->dsi1 = lt9611_attach_dsi(lt9611, lt9611->dsi1_node); + if (IS_ERR(lt9611->dsi1)) { + ret = PTR_ERR(lt9611->dsi1); + goto err_remove_bridge; + } + } + lt9611_enable_hpd_interrupts(lt9611);

return lt9611_audio_init(dev, lt9611);

+err_remove_bridge: + drm_bridge_remove(&lt9611->bridge); + err_disable_regulators: regulator_bulk_disable(ARRAY_SIZE(lt9611->supplies), lt9611->supplies);

Let's switch to the new devm MIPI-DSI function to register and attach our secondary device.

Acked-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 38 +++++----------------- 1 file changed, 8 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c index 010657ea7af7..ab1a0c00aad8 100644 --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c @@ -258,17 +258,18 @@ static struct mipi_dsi_device *lt9611uxc_attach_dsi(struct lt9611uxc *lt9611uxc, const struct mipi_dsi_device_info info = { "lt9611uxc", 0, NULL }; struct mipi_dsi_device *dsi; struct mipi_dsi_host *host; + struct device *dev = lt9611uxc->dev; int ret;

host = of_find_mipi_dsi_host_by_node(dsi_node); if (!host) { - dev_err(lt9611uxc->dev, "failed to find dsi host\n"); + dev_err(dev, "failed to find dsi host\n"); return ERR_PTR(-EPROBE_DEFER); }

- dsi = mipi_dsi_device_register_full(host, &info); + dsi = devm_mipi_dsi_device_register_full(dev, host, &info); if (IS_ERR(dsi)) { - dev_err(lt9611uxc->dev, "failed to create dsi device\n"); + dev_err(dev, "failed to create dsi device\n"); return dsi; }

@@ -277,10 +278,9 @@ static struct mipi_dsi_device *lt9611uxc_attach_dsi(struct lt9611uxc *lt9611uxc, dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE | MIPI_DSI_MODE_VIDEO_HSE;

- ret = mipi_dsi_attach(dsi); + ret = devm_mipi_dsi_attach(dev, dsi); if (ret < 0) { - dev_err(lt9611uxc->dev, "failed to attach dsi to host\n"); - mipi_dsi_device_unregister(dsi); + dev_err(dev, "failed to attach dsi to host\n"); return ERR_PTR(ret); }

@@ -355,19 +355,6 @@ static int lt9611uxc_connector_init(struct drm_bridge *bridge, struct lt9611uxc return drm_connector_attach_encoder(&lt9611uxc->connector, bridge->encoder); }

-static void lt9611uxc_bridge_detach(struct drm_bridge *bridge) -{ - struct lt9611uxc *lt9611uxc = bridge_to_lt9611uxc(bridge); - - if (lt9611uxc->dsi1) { - mipi_dsi_detach(lt9611uxc->dsi1); - mipi_dsi_device_unregister(lt9611uxc->dsi1); - } - - mipi_dsi_detach(lt9611uxc->dsi0); - mipi_dsi_device_unregister(lt9611uxc->dsi0); -} - static int lt9611uxc_bridge_attach(struct drm_bridge *bridge, enum drm_bridge_attach_flags flags) { @@ -388,19 +375,11 @@ static int lt9611uxc_bridge_attach(struct drm_bridge *bridge, /* Attach secondary DSI, if specified */ if (lt9611uxc->dsi1_node) { lt9611uxc->dsi1 = lt9611uxc_attach_dsi(lt9611uxc, lt9611uxc->dsi1_node); - if (IS_ERR(lt9611uxc->dsi1)) { - ret = PTR_ERR(lt9611uxc->dsi1); - goto err_unregister_dsi0; - } + if (IS_ERR(lt9611uxc->dsi1)) + return PTR_ERR(lt9611uxc->dsi1); }

return 0; - -err_unregister_dsi0: - mipi_dsi_detach(lt9611uxc->dsi0); - mipi_dsi_device_unregister(lt9611uxc->dsi0); - - return ret; }

static enum drm_mode_status @@ -544,7 +523,6 @@ static struct edid *lt9611uxc_bridge_get_edid(struct drm_bridge *bridge,

static const struct drm_bridge_funcs lt9611uxc_bridge_funcs = { .attach = lt9611uxc_bridge_attach, - .detach = lt9611uxc_bridge_detach, .mode_valid = lt9611uxc_bridge_mode_valid, .mode_set = lt9611uxc_bridge_mode_set, .detect = lt9611uxc_bridge_detect,

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.

Acked-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 31 +++++++++++++--------- 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c index ab1a0c00aad8..33f9716da0ee 100644 --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c @@ -367,18 +367,6 @@ static int lt9611uxc_bridge_attach(struct drm_bridge *bridge, return ret; }

- /* Attach primary DSI */ - lt9611uxc->dsi0 = lt9611uxc_attach_dsi(lt9611uxc, lt9611uxc->dsi0_node); - if (IS_ERR(lt9611uxc->dsi0)) - return PTR_ERR(lt9611uxc->dsi0); - - /* Attach secondary DSI, if specified */ - if (lt9611uxc->dsi1_node) { - lt9611uxc->dsi1 = lt9611uxc_attach_dsi(lt9611uxc, lt9611uxc->dsi1_node); - if (IS_ERR(lt9611uxc->dsi1)) - return PTR_ERR(lt9611uxc->dsi1); - } - return 0; }

@@ -958,8 +946,27 @@ static int lt9611uxc_probe(struct i2c_client *client,

drm_bridge_add(&lt9611uxc->bridge);

+ /* Attach primary DSI */ + lt9611uxc->dsi0 = lt9611uxc_attach_dsi(lt9611uxc, lt9611uxc->dsi0_node); + if (IS_ERR(lt9611uxc->dsi0)) { + ret = PTR_ERR(lt9611uxc->dsi0); + goto err_remove_bridge; + } + + /* Attach secondary DSI, if specified */ + if (lt9611uxc->dsi1_node) { + lt9611uxc->dsi1 = lt9611uxc_attach_dsi(lt9611uxc, lt9611uxc->dsi1_node); + if (IS_ERR(lt9611uxc->dsi1)) { + ret = PTR_ERR(lt9611uxc->dsi1); + goto err_remove_bridge; + } + } + return lt9611uxc_audio_init(dev, lt9611uxc);

+err_remove_bridge: + drm_bridge_remove(&lt9611uxc->bridge); + err_disable_regulators: regulator_bulk_disable(ARRAY_SIZE(lt9611uxc->supplies), lt9611uxc->supplies);

Let's switch to the new devm MIPI-DSI function to register and attach our secondary device. This also avoids leaking the device on removal.

Acked-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/parade-ps8640.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c index 3aaa90913bf8..5ae15fc407c5 100644 --- a/drivers/gpu/drm/bridge/parade-ps8640.c +++ b/drivers/gpu/drm/bridge/parade-ps8640.c @@ -428,7 +428,7 @@ static int ps8640_bridge_attach(struct drm_bridge *bridge, if (!host) return -ENODEV;

- dsi = mipi_dsi_device_register_full(host, &info); + 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); @@ -442,27 +442,22 @@ static int ps8640_bridge_attach(struct drm_bridge *bridge, MIPI_DSI_MODE_VIDEO_SYNC_PULSE; dsi->format = MIPI_DSI_FMT_RGB888; dsi->lanes = NUM_MIPI_LANES; - ret = mipi_dsi_attach(dsi); + + ret = devm_mipi_dsi_attach(dev, dsi); if (ret) { dev_err(dev, "failed to attach dsi device: %d\n", ret); - goto err_dsi_attach; + return ret; }

ret = drm_dp_aux_register(&ps_bridge->aux); if (ret) { dev_err(dev, "failed to register DP AUX channel: %d\n", ret); - goto err_aux_register; + return ret; }

/* Attach the panel-bridge to the dsi bridge */ return drm_bridge_attach(bridge->encoder, ps_bridge->panel_bridge, &ps_bridge->bridge, flags); - -err_aux_register: - mipi_dsi_detach(dsi); -err_dsi_attach: - mipi_dsi_device_unregister(dsi); - return ret; }

static void ps8640_bridge_detach(struct drm_bridge *bridge)

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.

Acked-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/parade-ps8640.c | 98 +++++++++++++++----------- 1 file changed, 55 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c index 5ae15fc407c5..4b36e4dc78f1 100644 --- a/drivers/gpu/drm/bridge/parade-ps8640.c +++ b/drivers/gpu/drm/bridge/parade-ps8640.c @@ -401,54 +401,11 @@ static int ps8640_bridge_attach(struct drm_bridge *bridge, { 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 = NUM_MIPI_LANES; - - ret = devm_mipi_dsi_attach(dev, dsi); - if (ret) { - dev_err(dev, "failed to attach dsi device: %d\n", ret); - return ret; - } - ret = drm_dp_aux_register(&ps_bridge->aux); if (ret) { dev_err(dev, "failed to register DP AUX channel: %d\n", ret); @@ -507,6 +464,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 = NUM_MIPI_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; @@ -584,7 +588,15 @@ static int ps8640_probe(struct i2c_client *client)

drm_bridge_add(&ps_bridge->bridge);

+ ret = ps8640_bridge_host_attach(dev, ps_bridge); + if (ret) + goto err_bridge_remove; + return 0; + +err_bridge_remove: + drm_bridge_remove(&ps_bridge->bridge); + return ret; }

static int ps8640_remove(struct i2c_client *client)

Commit 24417d5b0c00 ("drm/bridge: ti-sn65dsi83: Implement .detach callback") moved the unregistration of the bridge DSI device and bridge itself to the detach callback.

While this is correct for the DSI device detach and unregistration, the bridge is added in the driver probe, and should thus be removed as part of its remove callback.

Acked-by: Sam Ravnborg sam@ravnborg.org Reviewed-by: Marek Vasut marex@denx.de Fixes: 24417d5b0c00 ("drm/bridge: ti-sn65dsi83: Implement .detach callback") Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/ti-sn65dsi83.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c index 52030a82f3e1..3bfd07caf8d7 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c @@ -297,7 +297,6 @@ static void sn65dsi83_detach(struct drm_bridge *bridge)

mipi_dsi_detach(ctx->dsi); mipi_dsi_device_unregister(ctx->dsi); - drm_bridge_remove(&ctx->bridge); ctx->dsi = NULL; }

@@ -693,6 +692,7 @@ static int sn65dsi83_remove(struct i2c_client *client) { struct sn65dsi83 *ctx = i2c_get_clientdata(client);

+ drm_bridge_remove(&ctx->bridge); of_node_put(ctx->host_node);

return 0;

Let's switch to the new devm MIPI-DSI function to register and attach our secondary device. This also avoids leaking the device when we detach the bridge but don't remove its driver.

Acked-by: Sam Ravnborg sam@ravnborg.org Tested-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/ti-sn65dsi83.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c index 3bfd07caf8d7..539edd2c19f5 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c @@ -262,7 +262,7 @@ static int sn65dsi83_attach(struct drm_bridge *bridge, return -EPROBE_DEFER; }

- dsi = mipi_dsi_device_register_full(host, &info); + dsi = devm_mipi_dsi_device_register_full(dev, host, &info); if (IS_ERR(dsi)) { return dev_err_probe(dev, PTR_ERR(dsi), "failed to create dsi device\n"); @@ -274,18 +274,14 @@ static int sn65dsi83_attach(struct drm_bridge *bridge, dsi->format = MIPI_DSI_FMT_RGB888; dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST;

- ret = mipi_dsi_attach(dsi); + ret = devm_mipi_dsi_attach(dev, dsi); if (ret < 0) { dev_err(dev, "failed to attach dsi to host\n"); - goto err_dsi_attach; + return ret; }

return drm_bridge_attach(bridge->encoder, ctx->panel_bridge, &ctx->bridge, flags); - -err_dsi_attach: - mipi_dsi_device_unregister(dsi); - return ret; }

static void sn65dsi83_detach(struct drm_bridge *bridge) @@ -295,8 +291,6 @@ static void sn65dsi83_detach(struct drm_bridge *bridge) if (!ctx->dsi) return;

- mipi_dsi_detach(ctx->dsi); - mipi_dsi_device_unregister(ctx->dsi); ctx->dsi = NULL; }

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.

Acked-by: Sam Ravnborg sam@ravnborg.org Tested-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/ti-sn65dsi83.c | 80 +++++++++++++++------------ 1 file changed, 46 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c index 539edd2c19f5..945f08de45f1 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c @@ -245,40 +245,6 @@ static int sn65dsi83_attach(struct drm_bridge *bridge, enum drm_bridge_attach_flags flags) { struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge); - struct device *dev = ctx->dev; - struct mipi_dsi_device *dsi; - struct mipi_dsi_host *host; - int ret = 0; - - const struct mipi_dsi_device_info info = { - .type = "sn65dsi83", - .channel = 0, - .node = NULL, - }; - - host = of_find_mipi_dsi_host_by_node(ctx->host_node); - if (!host) { - dev_err(dev, "failed to find dsi host\n"); - return -EPROBE_DEFER; - } - - dsi = devm_mipi_dsi_device_register_full(dev, host, &info); - if (IS_ERR(dsi)) { - return dev_err_probe(dev, PTR_ERR(dsi), - "failed to create dsi device\n"); - } - - ctx->dsi = dsi; - - dsi->lanes = ctx->dsi_lanes; - dsi->format = MIPI_DSI_FMT_RGB888; - dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST; - - ret = devm_mipi_dsi_attach(dev, dsi); - if (ret < 0) { - dev_err(dev, "failed to attach dsi to host\n"); - return ret; - }

return drm_bridge_attach(bridge->encoder, ctx->panel_bridge, &ctx->bridge, flags); @@ -636,6 +602,44 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model) return 0; }

+static int sn65dsi83_host_attach(struct sn65dsi83 *ctx) +{ + struct device *dev = ctx->dev; + struct mipi_dsi_device *dsi; + struct mipi_dsi_host *host; + const struct mipi_dsi_device_info info = { + .type = "sn65dsi83", + .channel = 0, + .node = NULL, + }; + int ret; + + host = of_find_mipi_dsi_host_by_node(ctx->host_node); + if (!host) { + dev_err(dev, "failed to find dsi host\n"); + return -EPROBE_DEFER; + } + + dsi = devm_mipi_dsi_device_register_full(dev, host, &info); + if (IS_ERR(dsi)) + return dev_err_probe(dev, PTR_ERR(dsi), + "failed to create dsi device\n"); + + ctx->dsi = dsi; + + dsi->lanes = ctx->dsi_lanes; + dsi->format = MIPI_DSI_FMT_RGB888; + dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST; + + ret = devm_mipi_dsi_attach(dev, dsi); + if (ret < 0) { + dev_err(dev, "failed to attach dsi to host: %d\n", ret); + return ret; + } + + return 0; +} + static int sn65dsi83_probe(struct i2c_client *client, const struct i2c_device_id *id) { @@ -679,7 +683,15 @@ static int sn65dsi83_probe(struct i2c_client *client, ctx->bridge.of_node = dev->of_node; drm_bridge_add(&ctx->bridge);

+ ret = sn65dsi83_host_attach(ctx); + if (ret) + goto err_remove_bridge; + return 0; + +err_remove_bridge: + drm_bridge_remove(&ctx->bridge); + return ret; }

static int sn65dsi83_remove(struct i2c_client *client)

Let's switch to the new devm MIPI-DSI function to register and attach our secondary device. This also avoids leaking the device when we detach the bridge.

Acked-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/ti-sn65dsi86.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c index 6154bed0af5b..36a82e3d17ab 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c @@ -662,6 +662,7 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge, struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge); struct mipi_dsi_host *host; struct mipi_dsi_device *dsi; + struct device *dev = pdata->dev; const struct mipi_dsi_device_info info = { .type = "ti_sn_bridge", .channel = 0, .node = NULL, @@ -701,7 +702,7 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge, goto err_dsi_host; }

- dsi = mipi_dsi_device_register_full(host, &info); + dsi = devm_mipi_dsi_device_register_full(dev, host, &info); if (IS_ERR(dsi)) { DRM_ERROR("failed to create dsi device\n"); ret = PTR_ERR(dsi); @@ -714,16 +715,16 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge, dsi->mode_flags = MIPI_DSI_MODE_VIDEO;

/* check if continuous dsi clock is required or not */ - pm_runtime_get_sync(pdata->dev); + pm_runtime_get_sync(dev); regmap_read(pdata->regmap, SN_DPPLL_SRC_REG, &val); - pm_runtime_put_autosuspend(pdata->dev); + pm_runtime_put_autosuspend(dev); if (!(val & DPPLL_CLK_SRC_DSICLK)) dsi->mode_flags |= MIPI_DSI_CLOCK_NON_CONTINUOUS;

- ret = mipi_dsi_attach(dsi); + ret = devm_mipi_dsi_attach(dev, dsi); if (ret < 0) { DRM_ERROR("failed to attach dsi to host\n"); - goto err_dsi_attach; + goto err_dsi_host; } pdata->dsi = dsi;

@@ -734,14 +735,10 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge, ret = drm_bridge_attach(bridge->encoder, pdata->next_bridge, &pdata->bridge, flags); if (ret < 0) - goto err_dsi_detach; + goto err_dsi_host;

return 0;

-err_dsi_detach: - mipi_dsi_detach(dsi); -err_dsi_attach: - mipi_dsi_device_unregister(dsi); err_dsi_host: drm_connector_cleanup(&pdata->connector); err_conn_init: @@ -1237,11 +1234,6 @@ static void ti_sn_bridge_remove(struct auxiliary_device *adev) if (!pdata) return;

- if (pdata->dsi) { - mipi_dsi_detach(pdata->dsi); - mipi_dsi_device_unregister(pdata->dsi); - } - drm_bridge_remove(&pdata->bridge);

of_node_put(pdata->host_node);

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.

Acked-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/ti-sn65dsi86.c | 77 ++++++++++++++------------- 1 file changed, 40 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c index 36a82e3d17ab..b6ce6776cdf1 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c @@ -655,58 +655,27 @@ static struct ti_sn65dsi86 *bridge_to_ti_sn65dsi86(struct drm_bridge *bridge) return container_of(bridge, struct ti_sn65dsi86, bridge); }

-static int ti_sn_bridge_attach(struct drm_bridge *bridge, - enum drm_bridge_attach_flags flags) +static int ti_sn_attach_host(struct ti_sn65dsi86 *pdata) { int ret, val; - struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge); struct mipi_dsi_host *host; struct mipi_dsi_device *dsi; struct device *dev = pdata->dev; const struct mipi_dsi_device_info info = { .type = "ti_sn_bridge", .channel = 0, .node = NULL, - }; + };

- if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) { - DRM_ERROR("Fix bridge driver to make connector optional!"); - return -EINVAL; - } - - pdata->aux.drm_dev = bridge->dev; - ret = drm_dp_aux_register(&pdata->aux); - if (ret < 0) { - drm_err(bridge->dev, "Failed to register DP AUX channel: %d\n", ret); - return ret; - } - - ret = ti_sn_bridge_connector_init(pdata); - if (ret < 0) - goto err_conn_init; - - /* - * TODO: ideally finding host resource and dsi dev registration needs - * to be done in bridge probe. But some existing DSI host drivers will - * wait for any of the drm_bridge/drm_panel to get added to the global - * bridge/panel list, before completing their probe. So if we do the - * dsi dev registration part in bridge probe, before populating in - * the global bridge list, then it will cause deadlock as dsi host probe - * will never complete, neither our bridge probe. So keeping it here - * will satisfy most of the existing host drivers. Once the host driver - * is fixed we can move the below code to bridge probe safely. - */ host = of_find_mipi_dsi_host_by_node(pdata->host_node); if (!host) { DRM_ERROR("failed to find dsi host\n"); - ret = -ENODEV; - goto err_dsi_host; + return -ENODEV; }

dsi = devm_mipi_dsi_device_register_full(dev, host, &info); if (IS_ERR(dsi)) { DRM_ERROR("failed to create dsi device\n"); - ret = PTR_ERR(dsi); - goto err_dsi_host; + return PTR_ERR(dsi); }

/* TODO: setting to 4 MIPI lanes always for now */ @@ -721,12 +690,38 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge, if (!(val & DPPLL_CLK_SRC_DSICLK)) dsi->mode_flags |= MIPI_DSI_CLOCK_NON_CONTINUOUS;

+ pdata->dsi = dsi; + ret = devm_mipi_dsi_attach(dev, dsi); if (ret < 0) { DRM_ERROR("failed to attach dsi to host\n"); - goto err_dsi_host; + return ret; } - pdata->dsi = dsi; + + return 0; +} + +static int ti_sn_bridge_attach(struct drm_bridge *bridge, + enum drm_bridge_attach_flags flags) +{ + struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge); + int ret; + + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) { + DRM_ERROR("Fix bridge driver to make connector optional!"); + return -EINVAL; + } + + pdata->aux.drm_dev = bridge->dev; + ret = drm_dp_aux_register(&pdata->aux); + if (ret < 0) { + drm_err(bridge->dev, "Failed to register DP AUX channel: %d\n", ret); + return ret; + } + + ret = ti_sn_bridge_connector_init(pdata); + if (ret < 0) + goto err_conn_init;

/* We never want the next bridge to *also* create a connector: */ flags |= DRM_BRIDGE_ATTACH_NO_CONNECTOR; @@ -1224,7 +1219,15 @@ static int ti_sn_bridge_probe(struct auxiliary_device *adev,

drm_bridge_add(&pdata->bridge);

+ ret = ti_sn_attach_host(pdata); + if (ret) + goto err_remove_bridge; + return 0; + +err_remove_bridge: + drm_bridge_remove(&pdata->bridge); + return ret; }

static void ti_sn_bridge_remove(struct auxiliary_device *adev)

Let's switch to the new devm MIPI-DSI function to register and attach our secondary device. This also avoids leaking the device when we detach the bridge.

Acked-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/tc358775.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c index 2272adcc5b4a..35e66d1b6456 100644 --- a/drivers/gpu/drm/bridge/tc358775.c +++ b/drivers/gpu/drm/bridge/tc358775.c @@ -610,11 +610,10 @@ static int tc_bridge_attach(struct drm_bridge *bridge, return -EPROBE_DEFER; }

- dsi = mipi_dsi_device_register_full(host, &info); + 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); - goto err_dsi_device; + return PTR_ERR(dsi); }

tc->dsi = dsi; @@ -623,19 +622,15 @@ static int tc_bridge_attach(struct drm_bridge *bridge, dsi->format = MIPI_DSI_FMT_RGB888; dsi->mode_flags = MIPI_DSI_MODE_VIDEO;

- ret = mipi_dsi_attach(dsi); + ret = devm_mipi_dsi_attach(dev, dsi); if (ret < 0) { dev_err(dev, "failed to attach dsi to host\n"); - goto err_dsi_attach; + return ret; }

/* Attach the panel-bridge to the dsi bridge */ return drm_bridge_attach(bridge->encoder, tc->panel_bridge, &tc->bridge, flags); -err_dsi_attach: - mipi_dsi_device_unregister(dsi); -err_dsi_device: - return ret; }

static const struct drm_bridge_funcs tc_bridge_funcs = {

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.

Acked-by: Sam Ravnborg sam@ravnborg.org Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/bridge/tc358775.c | 37 +++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c index 35e66d1b6456..2c76331b251d 100644 --- a/drivers/gpu/drm/bridge/tc358775.c +++ b/drivers/gpu/drm/bridge/tc358775.c @@ -594,11 +594,26 @@ static int tc_bridge_attach(struct drm_bridge *bridge, enum drm_bridge_attach_flags flags) { struct tc_data *tc = bridge_to_tc(bridge); + + /* Attach the panel-bridge to the dsi bridge */ + return drm_bridge_attach(bridge->encoder, tc->panel_bridge, + &tc->bridge, flags); +} + +static const struct drm_bridge_funcs tc_bridge_funcs = { + .attach = tc_bridge_attach, + .pre_enable = tc_bridge_pre_enable, + .enable = tc_bridge_enable, + .mode_valid = tc_mode_valid, + .post_disable = tc_bridge_post_disable, +}; + +static int tc_attach_host(struct tc_data *tc) +{ struct device *dev = &tc->i2c->dev; struct mipi_dsi_host *host; struct mipi_dsi_device *dsi; int ret; - const struct mipi_dsi_device_info info = { .type = "tc358775", .channel = 0, .node = NULL, @@ -628,19 +643,9 @@ static int tc_bridge_attach(struct drm_bridge *bridge, return ret; }

- /* Attach the panel-bridge to the dsi bridge */ - return drm_bridge_attach(bridge->encoder, tc->panel_bridge, - &tc->bridge, flags); + return 0; }

-static const struct drm_bridge_funcs tc_bridge_funcs = { - .attach = tc_bridge_attach, - .pre_enable = tc_bridge_pre_enable, - .enable = tc_bridge_enable, - .mode_valid = tc_mode_valid, - .post_disable = tc_bridge_post_disable, -}; - static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) { struct device *dev = &client->dev; @@ -704,7 +709,15 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id)

i2c_set_clientdata(client, tc);

+ ret = tc_attach_host(tc); + if (ret) + goto err_bridge_remove; + return 0; + +err_bridge_remove: + drm_bridge_remove(&tc->bridge); + return ret; }

static int tc_remove(struct i2c_client *client)

Without proper care and an agreement between how DSI hosts and devices drivers register their MIPI-DSI entities and potential components, we can end up in a situation where the drivers can never probe.

Most drivers were taking evasive maneuvers to try to workaround this, but not all of them were following the same conventions, resulting in various incompatibilities between DSI hosts and devices.

Now that we have a sequence agreed upon and documented, let's convert kirin to it.

Tested-by: John Stultz john.stultz@linaro.org Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 52 +++++++++++++------- 1 file changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c index 952cfdb1961d..1d556482bb46 100644 --- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c +++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c @@ -81,7 +81,7 @@ struct dsi_hw_ctx {

struct dw_dsi { struct drm_encoder encoder; - struct drm_bridge *bridge; + struct device *dev; struct mipi_dsi_host host; struct drm_display_mode cur_mode; struct dsi_hw_ctx *ctx; @@ -720,10 +720,13 @@ static int dw_drm_encoder_init(struct device *dev, return 0; }

+static const struct component_ops dsi_ops; static int dsi_host_attach(struct mipi_dsi_host *host, struct mipi_dsi_device *mdsi) { struct dw_dsi *dsi = host_to_dsi(host); + struct device *dev = host->dev; + int ret;

if (mdsi->lanes < 1 || mdsi->lanes > 4) { DRM_ERROR("dsi device params invalid\n"); @@ -734,13 +737,20 @@ static int dsi_host_attach(struct mipi_dsi_host *host, dsi->format = mdsi->format; dsi->mode_flags = mdsi->mode_flags;

+ ret = component_add(dev, &dsi_ops); + if (ret) + return ret; + return 0; }

static int dsi_host_detach(struct mipi_dsi_host *host, struct mipi_dsi_device *mdsi) { - /* do nothing */ + struct device *dev = host->dev; + + component_del(dev, &dsi_ops); + return 0; }

@@ -768,7 +778,17 @@ static int dsi_host_init(struct device *dev, struct dw_dsi *dsi) static int dsi_bridge_init(struct drm_device *dev, struct dw_dsi *dsi) { struct drm_encoder *encoder = &dsi->encoder; - struct drm_bridge *bridge = dsi->bridge; + struct drm_bridge *bridge; + struct device_node *np = dsi->dev->of_node; + int ret; + + /* + * Get the endpoint node. In our case, dsi has one output port1 + * to which the external HDMI bridge is connected. + */ + ret = drm_of_find_panel_or_bridge(np, 1, 0, NULL, &bridge); + if (ret) + return ret;

/* associate the bridge to dsi encoder */ return drm_bridge_attach(encoder, bridge, NULL, 0); @@ -785,10 +805,6 @@ static int dsi_bind(struct device *dev, struct device *master, void *data) if (ret) return ret;

- ret = dsi_host_init(dev, dsi); - if (ret) - return ret; - ret = dsi_bridge_init(drm_dev, dsi); if (ret) return ret; @@ -809,17 +825,7 @@ static const struct component_ops dsi_ops = { static int dsi_parse_dt(struct platform_device *pdev, struct dw_dsi *dsi) { struct dsi_hw_ctx *ctx = dsi->ctx; - struct device_node *np = pdev->dev.of_node; struct resource *res; - int ret; - - /* - * Get the endpoint node. In our case, dsi has one output port1 - * to which the external HDMI bridge is connected. - */ - ret = drm_of_find_panel_or_bridge(np, 1, 0, NULL, &dsi->bridge); - if (ret) - return ret;

ctx->pclk = devm_clk_get(&pdev->dev, "pclk"); if (IS_ERR(ctx->pclk)) { @@ -852,6 +858,7 @@ static int dsi_probe(struct platform_device *pdev) dsi = &data->dsi; ctx = &data->ctx; dsi->ctx = ctx; + dsi->dev = &pdev->dev;

ret = dsi_parse_dt(pdev, dsi); if (ret) @@ -859,12 +866,19 @@ static int dsi_probe(struct platform_device *pdev)

platform_set_drvdata(pdev, data);

- return component_add(&pdev->dev, &dsi_ops); + ret = dsi_host_init(&pdev->dev, dsi); + if (ret) + return ret; + + return 0; }

static int dsi_remove(struct platform_device *pdev) { - component_del(&pdev->dev, &dsi_ops); + struct dsi_data *data = platform_get_drvdata(pdev); + struct dw_dsi *dsi = &data->dsi; + + mipi_dsi_host_unregister(&dsi->host);

return 0; }