| Message ID | 20210907024038.871299-1-marex@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: ti-sn65dsi83: Implement .detach callback | expand |
> On Tue, 7 Sept 2021 at 04:40, Marek Vasut <marex@denx.de> wrote: > > Move detach implementation from sn65dsi83_remove() to dedicated .detach callback. There is no functional change to the code, but > that detach is now in the correct location. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Jagan Teki <jagan@amarulasolutions.com> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Robert Foss <robert.foss@linaro.org> > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: dri-devel@lists.freedesktop.org > --- > drivers/gpu/drm/bridge/ti-sn65dsi83.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c > index 4ea71d7f0bfbc..13ee313daba96 100644 > --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c > @@ -288,6 +288,19 @@ static int sn65dsi83_attach(struct drm_bridge *bridge, > return ret; > } > > +static void sn65dsi83_detach(struct drm_bridge *bridge) > +{ > + struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge); > + > + if (!ctx->dsi) > + return; > + > + mipi_dsi_detach(ctx->dsi); > + mipi_dsi_device_unregister(ctx->dsi); > + drm_bridge_remove(&ctx->bridge); > + ctx->dsi = NULL; Is this assignment necessary? I'm not seeing it in the other drivers. WIth this cleared up feel free to add my r-b. Reviewed-by: Robert Foss <robert.foss@linaro.org>
On 10/6/21 11:47 AM, Robert Foss wrote: >> > On Tue, 7 Sept 2021 at 04:40, Marek Vasut <marex@denx.de> wrote: >> >> Move detach implementation from sn65dsi83_remove() to dedicated > .detach callback. There is no functional change to the code, but >> that detach is now in the correct location. >> >> Signed-off-by: Marek Vasut <marex@denx.de> >> Cc: Jagan Teki <jagan@amarulasolutions.com> >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Cc: Linus Walleij <linus.walleij@linaro.org> >> Cc: Robert Foss <robert.foss@linaro.org> >> Cc: Sam Ravnborg <sam@ravnborg.org> >> Cc: dri-devel@lists.freedesktop.org >> --- >> drivers/gpu/drm/bridge/ti-sn65dsi83.c | 17 ++++++++++++++--- >> 1 file changed, 14 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c >> index 4ea71d7f0bfbc..13ee313daba96 100644 >> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c >> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c >> @@ -288,6 +288,19 @@ static int sn65dsi83_attach(struct drm_bridge *bridge, >> return ret; >> } >> >> +static void sn65dsi83_detach(struct drm_bridge *bridge) >> +{ >> + struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge); >> + >> + if (!ctx->dsi) >> + return; >> + >> + mipi_dsi_detach(ctx->dsi); >> + mipi_dsi_device_unregister(ctx->dsi); >> + drm_bridge_remove(&ctx->bridge); >> + ctx->dsi = NULL; > > Is this assignment necessary? I'm not seeing it in the other drivers. > > WIth this cleared up feel free to add my r-b. > Reviewed-by: Robert Foss <robert.foss@linaro.org> It works in tandem with the if (!ctx->dsi) return; at the beginning to prevent crash in case the detach callback was called multiple times for whatever reason.
Applied to drm-misc-next On Thu, 7 Oct 2021 at 21:51, Marek Vasut <marex@denx.de> wrote: > > On 10/6/21 11:47 AM, Robert Foss wrote: > >> > > On Tue, 7 Sept 2021 at 04:40, Marek Vasut <marex@denx.de> wrote: > >> > >> Move detach implementation from sn65dsi83_remove() to dedicated > > .detach callback. There is no functional change to the code, but > >> that detach is now in the correct location. > >> > >> Signed-off-by: Marek Vasut <marex@denx.de> > >> Cc: Jagan Teki <jagan@amarulasolutions.com> > >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> Cc: Linus Walleij <linus.walleij@linaro.org> > >> Cc: Robert Foss <robert.foss@linaro.org> > >> Cc: Sam Ravnborg <sam@ravnborg.org> > >> Cc: dri-devel@lists.freedesktop.org > >> --- > >> drivers/gpu/drm/bridge/ti-sn65dsi83.c | 17 ++++++++++++++--- > >> 1 file changed, 14 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c > >> index 4ea71d7f0bfbc..13ee313daba96 100644 > >> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c > >> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c > >> @@ -288,6 +288,19 @@ static int sn65dsi83_attach(struct drm_bridge *bridge, > >> return ret; > >> } > >> > >> +static void sn65dsi83_detach(struct drm_bridge *bridge) > >> +{ > >> + struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge); > >> + > >> + if (!ctx->dsi) > >> + return; > >> + > >> + mipi_dsi_detach(ctx->dsi); > >> + mipi_dsi_device_unregister(ctx->dsi); > >> + drm_bridge_remove(&ctx->bridge); > >> + ctx->dsi = NULL; > > > > Is this assignment necessary? I'm not seeing it in the other drivers. > > > > WIth this cleared up feel free to add my r-b. > > Reviewed-by: Robert Foss <robert.foss@linaro.org> > > It works in tandem with the if (!ctx->dsi) return; at the beginning to > prevent crash in case the detach callback was called multiple times for > whatever reason.
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c index 4ea71d7f0bfbc..13ee313daba96 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c @@ -288,6 +288,19 @@ static int sn65dsi83_attach(struct drm_bridge *bridge, return ret; } +static void sn65dsi83_detach(struct drm_bridge *bridge) +{ + struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge); + + if (!ctx->dsi) + return; + + mipi_dsi_detach(ctx->dsi); + mipi_dsi_device_unregister(ctx->dsi); + drm_bridge_remove(&ctx->bridge); + ctx->dsi = NULL; +} + static void sn65dsi83_atomic_pre_enable(struct drm_bridge *bridge, struct drm_bridge_state *old_bridge_state) { @@ -588,6 +601,7 @@ sn65dsi83_atomic_get_input_bus_fmts(struct drm_bridge *bridge, static const struct drm_bridge_funcs sn65dsi83_funcs = { .attach = sn65dsi83_attach, + .detach = sn65dsi83_detach, .atomic_pre_enable = sn65dsi83_atomic_pre_enable, .atomic_enable = sn65dsi83_atomic_enable, .atomic_disable = sn65dsi83_atomic_disable, @@ -702,9 +716,6 @@ static int sn65dsi83_remove(struct i2c_client *client) { struct sn65dsi83 *ctx = i2c_get_clientdata(client); - mipi_dsi_detach(ctx->dsi); - mipi_dsi_device_unregister(ctx->dsi); - drm_bridge_remove(&ctx->bridge); of_node_put(ctx->host_node); return 0;
Move detach implementation from sn65dsi83_remove() to dedicated .detach callback. There is no functional change to the code, but that detach is now in the correct location. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Jagan Teki <jagan@amarulasolutions.com> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Robert Foss <robert.foss@linaro.org> Cc: Sam Ravnborg <sam@ravnborg.org> Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/bridge/ti-sn65dsi83.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)