Message ID | 1512100685-4015-2-git-send-email-nickey.yang@rock-chips.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Dear Nickey, Many thanks for your patch. I am sorry to say that but you can not add my "Acked-by" to this patch because this code is different from the "original" one from Brian (which got my "Acked-by"). Sometimes it is not an issue because differences are not important but in this particular case, the code is really different: you have remove platform_set_drvdata() & platform_get_drvdata() in the stm part. Could you please go back to the original code or propose me an updated version of this code. Many thanks, Philippe :-) On 12/01/2017 04:58 AM, Nickey Yang wrote: > From: Brian Norris <briannorris@chromium.org> > > Bridge drivers/helpers shouldn't be clobbering the drvdata, since a > parent driver might need to own this. Instead, let's return our > 'dw_mipi_dsi' object and have callers pass that back to us for removal. > > Signed-off-by: Brian Norris <briannorris@chromium.org> > Signed-off-by: Nickey Yang <nickey.yang@rock-chips.com> > Reviewed-by: Matthias Kaehlcke <mka@chromium.org> > Reviewed-by: Archit Taneja <architt@codeaurora.org> > Acked-by: Philippe Cornu <philippe.cornu@st.com> > Link:https://patchwork.kernel.org/patch/10078493/ > > --- > Changes in v4: > - Add From tag,update subject line > - keep patch "drm/stm: dsi: Adjust dw_mipi_dsi_probe and remove" > in this piece together. > > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 36 ++++++++++----------------- > drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 8 +++--- > include/drm/bridge/dw_mipi_dsi.h | 17 ++++++++----- > 3 files changed, 29 insertions(+), 32 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > index d9cca4f..c39c7dc 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > @@ -922,8 +922,6 @@ static int dw_mipi_dsi_bridge_attach(struct drm_bridge *bridge) > dsi->bridge.of_node = pdev->dev.of_node; > #endif > > - dev_set_drvdata(dev, dsi); > - > return dsi; > } > > @@ -935,23 +933,16 @@ static void __dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi) > /* > * Probe/remove API, used from platforms based on the DRM bridge API. > */ > -int dw_mipi_dsi_probe(struct platform_device *pdev, > - const struct dw_mipi_dsi_plat_data *plat_data) > +struct dw_mipi_dsi * > +dw_mipi_dsi_probe(struct platform_device *pdev, > + const struct dw_mipi_dsi_plat_data *plat_data) > { > - struct dw_mipi_dsi *dsi; > - > - dsi = __dw_mipi_dsi_probe(pdev, plat_data); > - if (IS_ERR(dsi)) > - return PTR_ERR(dsi); > - > - return 0; > + return __dw_mipi_dsi_probe(pdev, plat_data); > } > EXPORT_SYMBOL_GPL(dw_mipi_dsi_probe); > > -void dw_mipi_dsi_remove(struct platform_device *pdev) > +void dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi) > { > - struct dw_mipi_dsi *dsi = platform_get_drvdata(pdev); > - > mipi_dsi_host_unregister(&dsi->dsi_host); > > __dw_mipi_dsi_remove(dsi); > @@ -961,31 +952,30 @@ void dw_mipi_dsi_remove(struct platform_device *pdev) > /* > * Bind/unbind API, used from platforms based on the component framework. > */ > -int dw_mipi_dsi_bind(struct platform_device *pdev, struct drm_encoder *encoder, > - const struct dw_mipi_dsi_plat_data *plat_data) > +struct dw_mipi_dsi * > +dw_mipi_dsi_bind(struct platform_device *pdev, struct drm_encoder *encoder, > + const struct dw_mipi_dsi_plat_data *plat_data) > { > struct dw_mipi_dsi *dsi; > int ret; > > dsi = __dw_mipi_dsi_probe(pdev, plat_data); > if (IS_ERR(dsi)) > - return PTR_ERR(dsi); > + return dsi; > > ret = drm_bridge_attach(encoder, &dsi->bridge, NULL); > if (ret) { > - dw_mipi_dsi_remove(pdev); > + dw_mipi_dsi_remove(dsi); > DRM_ERROR("Failed to initialize bridge with drm\n"); > - return ret; > + return ERR_PTR(ret); > } > > - return 0; > + return dsi; > } > EXPORT_SYMBOL_GPL(dw_mipi_dsi_bind); > > -void dw_mipi_dsi_unbind(struct device *dev) > +void dw_mipi_dsi_unbind(struct dw_mipi_dsi *dsi) > { > - struct dw_mipi_dsi *dsi = dev_get_drvdata(dev); > - > __dw_mipi_dsi_remove(dsi); > } > EXPORT_SYMBOL_GPL(dw_mipi_dsi_unbind); > diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > index e5b6310..80f9950 100644 > --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > @@ -66,6 +66,7 @@ enum dsi_color { > struct dw_mipi_dsi_stm { > void __iomem *base; > struct clk *pllref_clk; > + struct dw_mipi_dsi *dmd; > }; > > static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val) > @@ -318,10 +319,11 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev) > dw_mipi_dsi_stm_plat_data.base = dsi->base; > dw_mipi_dsi_stm_plat_data.priv_data = dsi; > > - ret = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data); > - if (ret) { > + dsi->dmd = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data); > + if (IS_ERR(dsi->dmd)) { > DRM_ERROR("Failed to initialize mipi dsi host\n"); > clk_disable_unprepare(dsi->pllref_clk); > + return PTR_ERR(dsi->dmd); > } > > return ret; > @@ -332,7 +334,7 @@ static int dw_mipi_dsi_stm_remove(struct platform_device *pdev) > struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data; > > clk_disable_unprepare(dsi->pllref_clk); > - dw_mipi_dsi_remove(pdev); > + dw_mipi_dsi_remove(dsi->dmd); > > return 0; > } > diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h > index 9b30fec..d9c6d54 100644 > --- a/include/drm/bridge/dw_mipi_dsi.h > +++ b/include/drm/bridge/dw_mipi_dsi.h > @@ -10,6 +10,8 @@ > #ifndef __DW_MIPI_DSI__ > #define __DW_MIPI_DSI__ > > +struct dw_mipi_dsi; > + > struct dw_mipi_dsi_phy_ops { > int (*init)(void *priv_data); > int (*get_lane_mbps)(void *priv_data, struct drm_display_mode *mode, > @@ -29,11 +31,14 @@ struct dw_mipi_dsi_plat_data { > void *priv_data; > }; > > -int dw_mipi_dsi_probe(struct platform_device *pdev, > - const struct dw_mipi_dsi_plat_data *plat_data); > -void dw_mipi_dsi_remove(struct platform_device *pdev); > -int dw_mipi_dsi_bind(struct platform_device *pdev, struct drm_encoder *encoder, > - const struct dw_mipi_dsi_plat_data *plat_data); > -void dw_mipi_dsi_unbind(struct device *dev); > +struct dw_mipi_dsi *dw_mipi_dsi_probe(struct platform_device *pdev, > + const struct dw_mipi_dsi_plat_data > + *plat_data); > +void dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi); > +struct dw_mipi_dsi *dw_mipi_dsi_bind(struct platform_device *pdev, > + struct drm_encoder *encoder, > + const struct dw_mipi_dsi_plat_data > + *plat_data); > +void dw_mipi_dsi_unbind(struct dw_mipi_dsi *dsi); > > #endif /* __DW_MIPI_DSI__ */ >
Hi Philippe, On 2017年12月01日 16:32, Philippe CORNU wrote: > Dear Nickey, > > Many thanks for your patch. > > I am sorry to say that but you can not add my "Acked-by" to this patch > because this code is different from the "original" one from Brian (which > got my "Acked-by"). I'm sorry I didn't think much about it, Thank you for correcting me. > Sometimes it is not an issue because differences are not important but > in this particular case, the code is really different: you have remove > platform_set_drvdata() & platform_get_drvdata() in the stm part. > > Could you please go back to the original code or propose me an updated > version of this code. Could you help update new version of this code(stm part) and then test on stm platform? Thanks, Nickey. > > Many thanks, > > Philippe :-) > > > On 12/01/2017 04:58 AM, Nickey Yang wrote: >> From: Brian Norris <briannorris@chromium.org> >> >> Bridge drivers/helpers shouldn't be clobbering the drvdata, since a >> parent driver might need to own this. Instead, let's return our >> 'dw_mipi_dsi' object and have callers pass that back to us for removal. >> >> Signed-off-by: Brian Norris <briannorris@chromium.org> >> Signed-off-by: Nickey Yang <nickey.yang@rock-chips.com> >> Reviewed-by: Matthias Kaehlcke <mka@chromium.org> >> Reviewed-by: Archit Taneja <architt@codeaurora.org> >> Acked-by: Philippe Cornu <philippe.cornu@st.com> >> Link:https://patchwork.kernel.org/patch/10078493/ >> >> --- >> Changes in v4: >> - Add From tag,update subject line >> - keep patch "drm/stm: dsi: Adjust dw_mipi_dsi_probe and remove" >> in this piece together. >> >> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 36 ++++++++++----------------- >> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 8 +++--- >> include/drm/bridge/dw_mipi_dsi.h | 17 ++++++++----- >> 3 files changed, 29 insertions(+), 32 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c >> index d9cca4f..c39c7dc 100644 >> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c >> @@ -922,8 +922,6 @@ static int dw_mipi_dsi_bridge_attach(struct drm_bridge *bridge) >> dsi->bridge.of_node = pdev->dev.of_node; >> #endif >> >> - dev_set_drvdata(dev, dsi); >> - >> return dsi; >> } >> >> @@ -935,23 +933,16 @@ static void __dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi) >> /* >> * Probe/remove API, used from platforms based on the DRM bridge API. >> */ >> -int dw_mipi_dsi_probe(struct platform_device *pdev, >> - const struct dw_mipi_dsi_plat_data *plat_data) >> +struct dw_mipi_dsi * >> +dw_mipi_dsi_probe(struct platform_device *pdev, >> + const struct dw_mipi_dsi_plat_data *plat_data) >> { >> - struct dw_mipi_dsi *dsi; >> - >> - dsi = __dw_mipi_dsi_probe(pdev, plat_data); >> - if (IS_ERR(dsi)) >> - return PTR_ERR(dsi); >> - >> - return 0; >> + return __dw_mipi_dsi_probe(pdev, plat_data); >> } >> EXPORT_SYMBOL_GPL(dw_mipi_dsi_probe); >> >> -void dw_mipi_dsi_remove(struct platform_device *pdev) >> +void dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi) >> { >> - struct dw_mipi_dsi *dsi = platform_get_drvdata(pdev); >> - >> mipi_dsi_host_unregister(&dsi->dsi_host); >> >> __dw_mipi_dsi_remove(dsi); >> @@ -961,31 +952,30 @@ void dw_mipi_dsi_remove(struct platform_device *pdev) >> /* >> * Bind/unbind API, used from platforms based on the component framework. >> */ >> -int dw_mipi_dsi_bind(struct platform_device *pdev, struct drm_encoder *encoder, >> - const struct dw_mipi_dsi_plat_data *plat_data) >> +struct dw_mipi_dsi * >> +dw_mipi_dsi_bind(struct platform_device *pdev, struct drm_encoder *encoder, >> + const struct dw_mipi_dsi_plat_data *plat_data) >> { >> struct dw_mipi_dsi *dsi; >> int ret; >> >> dsi = __dw_mipi_dsi_probe(pdev, plat_data); >> if (IS_ERR(dsi)) >> - return PTR_ERR(dsi); >> + return dsi; >> >> ret = drm_bridge_attach(encoder, &dsi->bridge, NULL); >> if (ret) { >> - dw_mipi_dsi_remove(pdev); >> + dw_mipi_dsi_remove(dsi); >> DRM_ERROR("Failed to initialize bridge with drm\n"); >> - return ret; >> + return ERR_PTR(ret); >> } >> >> - return 0; >> + return dsi; >> } >> EXPORT_SYMBOL_GPL(dw_mipi_dsi_bind); >> >> -void dw_mipi_dsi_unbind(struct device *dev) >> +void dw_mipi_dsi_unbind(struct dw_mipi_dsi *dsi) >> { >> - struct dw_mipi_dsi *dsi = dev_get_drvdata(dev); >> - >> __dw_mipi_dsi_remove(dsi); >> } >> EXPORT_SYMBOL_GPL(dw_mipi_dsi_unbind); >> diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c >> index e5b6310..80f9950 100644 >> --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c >> +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c >> @@ -66,6 +66,7 @@ enum dsi_color { >> struct dw_mipi_dsi_stm { >> void __iomem *base; >> struct clk *pllref_clk; >> + struct dw_mipi_dsi *dmd; >> }; >> >> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val) >> @@ -318,10 +319,11 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev) >> dw_mipi_dsi_stm_plat_data.base = dsi->base; >> dw_mipi_dsi_stm_plat_data.priv_data = dsi; >> >> - ret = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data); >> - if (ret) { >> + dsi->dmd = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data); >> + if (IS_ERR(dsi->dmd)) { >> DRM_ERROR("Failed to initialize mipi dsi host\n"); >> clk_disable_unprepare(dsi->pllref_clk); >> + return PTR_ERR(dsi->dmd); >> } >> >> return ret; >> @@ -332,7 +334,7 @@ static int dw_mipi_dsi_stm_remove(struct platform_device *pdev) >> struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data; >> >> clk_disable_unprepare(dsi->pllref_clk); >> - dw_mipi_dsi_remove(pdev); >> + dw_mipi_dsi_remove(dsi->dmd); >> >> return 0; >> } >> diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h >> index 9b30fec..d9c6d54 100644 >> --- a/include/drm/bridge/dw_mipi_dsi.h >> +++ b/include/drm/bridge/dw_mipi_dsi.h >> @@ -10,6 +10,8 @@ >> #ifndef __DW_MIPI_DSI__ >> #define __DW_MIPI_DSI__ >> >> +struct dw_mipi_dsi; >> + >> struct dw_mipi_dsi_phy_ops { >> int (*init)(void *priv_data); >> int (*get_lane_mbps)(void *priv_data, struct drm_display_mode *mode, >> @@ -29,11 +31,14 @@ struct dw_mipi_dsi_plat_data { >> void *priv_data; >> }; >> >> -int dw_mipi_dsi_probe(struct platform_device *pdev, >> - const struct dw_mipi_dsi_plat_data *plat_data); >> -void dw_mipi_dsi_remove(struct platform_device *pdev); >> -int dw_mipi_dsi_bind(struct platform_device *pdev, struct drm_encoder *encoder, >> - const struct dw_mipi_dsi_plat_data *plat_data); >> -void dw_mipi_dsi_unbind(struct device *dev); >> +struct dw_mipi_dsi *dw_mipi_dsi_probe(struct platform_device *pdev, >> + const struct dw_mipi_dsi_plat_data >> + *plat_data); >> +void dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi); >> +struct dw_mipi_dsi *dw_mipi_dsi_bind(struct platform_device *pdev, >> + struct drm_encoder *encoder, >> + const struct dw_mipi_dsi_plat_data >> + *plat_data); >> +void dw_mipi_dsi_unbind(struct dw_mipi_dsi *dsi); >> >> #endif /* __DW_MIPI_DSI__ */
Hi Nickey, On 12/01/2017 10:11 AM, Nickey Yang wrote: > Hi Philippe, > > > On 2017年12月01日 16:32, Philippe CORNU wrote: >> Dear Nickey, >> >> Many thanks for your patch. >> >> I am sorry to say that but you can not add my "Acked-by" to this patch >> because this code is different from the "original" one from Brian (which >> got my "Acked-by"). > I'm sorry I didn't think much about it, Thank you for correcting me. >> Sometimes it is not an issue because differences are not important but >> in this particular case, the code is really different: you have remove >> platform_set_drvdata() & platform_get_drvdata() in the stm part. >> >> Could you please go back to the original code or propose me an updated >> version of this code. > Could you help update new version of this code(stm part) and then test on > stm platform? I think you can simply goes back to the original version from Brian (see the discussion thread in https://patchwork.kernel.org/patch/10078493/) unless you have specific/good reasons for modifying the code as you did. Many thanks, Philippe :) > > Thanks, > Nickey. >> >> Many thanks, >> >> Philippe :-) >> >> >> On 12/01/2017 04:58 AM, Nickey Yang wrote: >>> From: Brian Norris<briannorris@chromium.org> >>> >>> Bridge drivers/helpers shouldn't be clobbering the drvdata, since a >>> parent driver might need to own this. Instead, let's return our >>> 'dw_mipi_dsi' object and have callers pass that back to us for removal. >>> >>> Signed-off-by: Brian Norris<briannorris@chromium.org> >>> Signed-off-by: Nickey Yang<nickey.yang@rock-chips.com> >>> Reviewed-by: Matthias Kaehlcke<mka@chromium.org> >>> Reviewed-by: Archit Taneja<architt@codeaurora.org> >>> Acked-by: Philippe Cornu<philippe.cornu@st.com> >>> Link:https://patchwork.kernel.org/patch/10078493/ >>> >>> --- >>> Changes in v4: >>> - Add From tag,update subject line >>> - keep patch "drm/stm: dsi: Adjust dw_mipi_dsi_probe and remove" >>> in this piece together. >>> >>> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 36 ++++++++++----------------- >>> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 8 +++--- >>> include/drm/bridge/dw_mipi_dsi.h | 17 ++++++++----- >>> 3 files changed, 29 insertions(+), 32 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c >>> index d9cca4f..c39c7dc 100644 >>> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c >>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c >>> @@ -922,8 +922,6 @@ static int dw_mipi_dsi_bridge_attach(struct drm_bridge *bridge) >>> dsi->bridge.of_node = pdev->dev.of_node; >>> #endif >>> >>> - dev_set_drvdata(dev, dsi); >>> - >>> return dsi; >>> } >>> >>> @@ -935,23 +933,16 @@ static void __dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi) >>> /* >>> * Probe/remove API, used from platforms based on the DRM bridge API. >>> */ >>> -int dw_mipi_dsi_probe(struct platform_device *pdev, >>> - const struct dw_mipi_dsi_plat_data *plat_data) >>> +struct dw_mipi_dsi * >>> +dw_mipi_dsi_probe(struct platform_device *pdev, >>> + const struct dw_mipi_dsi_plat_data *plat_data) >>> { >>> - struct dw_mipi_dsi *dsi; >>> - >>> - dsi = __dw_mipi_dsi_probe(pdev, plat_data); >>> - if (IS_ERR(dsi)) >>> - return PTR_ERR(dsi); >>> - >>> - return 0; >>> + return __dw_mipi_dsi_probe(pdev, plat_data); >>> } >>> EXPORT_SYMBOL_GPL(dw_mipi_dsi_probe); >>> >>> -void dw_mipi_dsi_remove(struct platform_device *pdev) >>> +void dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi) >>> { >>> - struct dw_mipi_dsi *dsi = platform_get_drvdata(pdev); >>> - >>> mipi_dsi_host_unregister(&dsi->dsi_host); >>> >>> __dw_mipi_dsi_remove(dsi); >>> @@ -961,31 +952,30 @@ void dw_mipi_dsi_remove(struct platform_device *pdev) >>> /* >>> * Bind/unbind API, used from platforms based on the component framework. >>> */ >>> -int dw_mipi_dsi_bind(struct platform_device *pdev, struct drm_encoder *encoder, >>> - const struct dw_mipi_dsi_plat_data *plat_data) >>> +struct dw_mipi_dsi * >>> +dw_mipi_dsi_bind(struct platform_device *pdev, struct drm_encoder *encoder, >>> + const struct dw_mipi_dsi_plat_data *plat_data) >>> { >>> struct dw_mipi_dsi *dsi; >>> int ret; >>> >>> dsi = __dw_mipi_dsi_probe(pdev, plat_data); >>> if (IS_ERR(dsi)) >>> - return PTR_ERR(dsi); >>> + return dsi; >>> >>> ret = drm_bridge_attach(encoder, &dsi->bridge, NULL); >>> if (ret) { >>> - dw_mipi_dsi_remove(pdev); >>> + dw_mipi_dsi_remove(dsi); >>> DRM_ERROR("Failed to initialize bridge with drm\n"); >>> - return ret; >>> + return ERR_PTR(ret); >>> } >>> >>> - return 0; >>> + return dsi; >>> } >>> EXPORT_SYMBOL_GPL(dw_mipi_dsi_bind); >>> >>> -void dw_mipi_dsi_unbind(struct device *dev) >>> +void dw_mipi_dsi_unbind(struct dw_mipi_dsi *dsi) >>> { >>> - struct dw_mipi_dsi *dsi = dev_get_drvdata(dev); >>> - >>> __dw_mipi_dsi_remove(dsi); >>> } >>> EXPORT_SYMBOL_GPL(dw_mipi_dsi_unbind); >>> diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c >>> index e5b6310..80f9950 100644 >>> --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c >>> +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c >>> @@ -66,6 +66,7 @@ enum dsi_color { >>> struct dw_mipi_dsi_stm { >>> void __iomem *base; >>> struct clk *pllref_clk; >>> + struct dw_mipi_dsi *dmd; >>> }; >>> >>> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val) >>> @@ -318,10 +319,11 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev) >>> dw_mipi_dsi_stm_plat_data.base = dsi->base; >>> dw_mipi_dsi_stm_plat_data.priv_data = dsi; >>> >>> - ret = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data); >>> - if (ret) { >>> + dsi->dmd = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data); >>> + if (IS_ERR(dsi->dmd)) { >>> DRM_ERROR("Failed to initialize mipi dsi host\n"); >>> clk_disable_unprepare(dsi->pllref_clk); >>> + return PTR_ERR(dsi->dmd); >>> } >>> >>> return ret; >>> @@ -332,7 +334,7 @@ static int dw_mipi_dsi_stm_remove(struct platform_device *pdev) >>> struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data; >>> >>> clk_disable_unprepare(dsi->pllref_clk); >>> - dw_mipi_dsi_remove(pdev); >>> + dw_mipi_dsi_remove(dsi->dmd); >>> >>> return 0; >>> } >>> diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h >>> index 9b30fec..d9c6d54 100644 >>> --- a/include/drm/bridge/dw_mipi_dsi.h >>> +++ b/include/drm/bridge/dw_mipi_dsi.h >>> @@ -10,6 +10,8 @@ >>> #ifndef __DW_MIPI_DSI__ >>> #define __DW_MIPI_DSI__ >>> >>> +struct dw_mipi_dsi; >>> + >>> struct dw_mipi_dsi_phy_ops { >>> int (*init)(void *priv_data); >>> int (*get_lane_mbps)(void *priv_data, struct drm_display_mode *mode, >>> @@ -29,11 +31,14 @@ struct dw_mipi_dsi_plat_data { >>> void *priv_data; >>> }; >>> >>> -int dw_mipi_dsi_probe(struct platform_device *pdev, >>> - const struct dw_mipi_dsi_plat_data *plat_data); >>> -void dw_mipi_dsi_remove(struct platform_device *pdev); >>> -int dw_mipi_dsi_bind(struct platform_device *pdev, struct drm_encoder *encoder, >>> - const struct dw_mipi_dsi_plat_data *plat_data); >>> -void dw_mipi_dsi_unbind(struct device *dev); >>> +struct dw_mipi_dsi *dw_mipi_dsi_probe(struct platform_device *pdev, >>> + const struct dw_mipi_dsi_plat_data >>> + *plat_data); >>> +void dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi); >>> +struct dw_mipi_dsi *dw_mipi_dsi_bind(struct platform_device *pdev, >>> + struct drm_encoder *encoder, >>> + const struct dw_mipi_dsi_plat_data >>> + *plat_data); >>> +void dw_mipi_dsi_unbind(struct dw_mipi_dsi *dsi); >>> >>> #endif /* __DW_MIPI_DSI__ */ >
On 1 December 2017 at 08:32, Philippe CORNU <philippe.cornu@st.com> wrote: > Dear Nickey, > > Many thanks for your patch. > > I am sorry to say that but you can not add my "Acked-by" to this patch > because this code is different from the "original" one from Brian (which > got my "Acked-by"). > Small idea: I've seen people mention the revision that got the r-b/ack. Although in such cases the changelog tends to be part of the commit message. For example: drm: address ... Determine X based on A. v2: - Some changelog Acked-by: Some Person <fancy@email.com> (v1) -EMil
Hi Philippe, On 2017年12月01日 18:07, Philippe CORNU wrote: > Hi Nickey, > > On 12/01/2017 10:11 AM, Nickey Yang wrote: >> Hi Philippe, >> >> >> On 2017年12月01日 16:32, Philippe CORNU wrote: >>> Dear Nickey, >>> >>> Many thanks for your patch. >>> >>> I am sorry to say that but you can not add my "Acked-by" to this patch >>> because this code is different from the "original" one from Brian (which >>> got my "Acked-by"). >> I'm sorry I didn't think much about it, Thank you for correcting me. >>> Sometimes it is not an issue because differences are not important but >>> in this particular case, the code is really different: you have remove >>> platform_set_drvdata() & platform_get_drvdata() in the stm part. >>> >>> Could you please go back to the original code or propose me an updated >>> version of this code. >> Could you help update new version of this code(stm part) and then test on >> stm platform? > I think you can simply goes back to the original version from Brian (see > the discussion thread in https://patchwork.kernel.org/patch/10078493/) > unless you have specific/good reasons for modifying the code as you did. mmm,I'm sorry, I feel a little puzzled. Do you means we should abandon Brian's patch (https://patchwork.kernel.org/patch/10078493/)? I think we need to adjust stm part because dw_mipi_dsi_stm.c calls bridge's drivers if we want merge Brian's patch. Thanks > Many thanks, > Philippe :) > >> Thanks, >> Nickey. >>> Many thanks, >>> >>> Philippe :-) >>> >>> >>> On 12/01/2017 04:58 AM, Nickey Yang wrote: >>>> From: Brian Norris<briannorris@chromium.org> >>>> >>>> Bridge drivers/helpers shouldn't be clobbering the drvdata, since a >>>> parent driver might need to own this. Instead, let's return our >>>> 'dw_mipi_dsi' object and have callers pass that back to us for removal. >>>> >>>> Signed-off-by: Brian Norris<briannorris@chromium.org> >>>> Signed-off-by: Nickey Yang<nickey.yang@rock-chips.com> >>>> Reviewed-by: Matthias Kaehlcke<mka@chromium.org> >>>> Reviewed-by: Archit Taneja<architt@codeaurora.org> >>>> Acked-by: Philippe Cornu<philippe.cornu@st.com> >>>> Link:https://patchwork.kernel.org/patch/10078493/ >>>> >>>> --- >>>> Changes in v4: >>>> - Add From tag,update subject line >>>> - keep patch "drm/stm: dsi: Adjust dw_mipi_dsi_probe and remove" >>>> in this piece together. >>>> >>>> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 36 ++++++++++----------------- >>>> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 8 +++--- >>>> include/drm/bridge/dw_mipi_dsi.h | 17 ++++++++----- >>>> 3 files changed, 29 insertions(+), 32 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c >>>> index d9cca4f..c39c7dc 100644 >>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c >>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c >>>> @@ -922,8 +922,6 @@ static int dw_mipi_dsi_bridge_attach(struct drm_bridge *bridge) >>>> dsi->bridge.of_node = pdev->dev.of_node; >>>> #endif >>>> >>>> - dev_set_drvdata(dev, dsi); >>>> - >>>> return dsi; >>>> } >>>> >>>> @@ -935,23 +933,16 @@ static void __dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi) >>>> /* >>>> * Probe/remove API, used from platforms based on the DRM bridge API. >>>> */ >>>> -int dw_mipi_dsi_probe(struct platform_device *pdev, >>>> - const struct dw_mipi_dsi_plat_data *plat_data) >>>> +struct dw_mipi_dsi * >>>> +dw_mipi_dsi_probe(struct platform_device *pdev, >>>> + const struct dw_mipi_dsi_plat_data *plat_data) >>>> { >>>> - struct dw_mipi_dsi *dsi; >>>> - >>>> - dsi = __dw_mipi_dsi_probe(pdev, plat_data); >>>> - if (IS_ERR(dsi)) >>>> - return PTR_ERR(dsi); >>>> - >>>> - return 0; >>>> + return __dw_mipi_dsi_probe(pdev, plat_data); >>>> } >>>> EXPORT_SYMBOL_GPL(dw_mipi_dsi_probe); >>>> >>>> -void dw_mipi_dsi_remove(struct platform_device *pdev) >>>> +void dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi) >>>> { >>>> - struct dw_mipi_dsi *dsi = platform_get_drvdata(pdev); >>>> - >>>> mipi_dsi_host_unregister(&dsi->dsi_host); >>>> >>>> __dw_mipi_dsi_remove(dsi); >>>> @@ -961,31 +952,30 @@ void dw_mipi_dsi_remove(struct platform_device *pdev) >>>> /* >>>> * Bind/unbind API, used from platforms based on the component framework. >>>> */ >>>> -int dw_mipi_dsi_bind(struct platform_device *pdev, struct drm_encoder *encoder, >>>> - const struct dw_mipi_dsi_plat_data *plat_data) >>>> +struct dw_mipi_dsi * >>>> +dw_mipi_dsi_bind(struct platform_device *pdev, struct drm_encoder *encoder, >>>> + const struct dw_mipi_dsi_plat_data *plat_data) >>>> { >>>> struct dw_mipi_dsi *dsi; >>>> int ret; >>>> >>>> dsi = __dw_mipi_dsi_probe(pdev, plat_data); >>>> if (IS_ERR(dsi)) >>>> - return PTR_ERR(dsi); >>>> + return dsi; >>>> >>>> ret = drm_bridge_attach(encoder, &dsi->bridge, NULL); >>>> if (ret) { >>>> - dw_mipi_dsi_remove(pdev); >>>> + dw_mipi_dsi_remove(dsi); >>>> DRM_ERROR("Failed to initialize bridge with drm\n"); >>>> - return ret; >>>> + return ERR_PTR(ret); >>>> } >>>> >>>> - return 0; >>>> + return dsi; >>>> } >>>> EXPORT_SYMBOL_GPL(dw_mipi_dsi_bind); >>>> >>>> -void dw_mipi_dsi_unbind(struct device *dev) >>>> +void dw_mipi_dsi_unbind(struct dw_mipi_dsi *dsi) >>>> { >>>> - struct dw_mipi_dsi *dsi = dev_get_drvdata(dev); >>>> - >>>> __dw_mipi_dsi_remove(dsi); >>>> } >>>> EXPORT_SYMBOL_GPL(dw_mipi_dsi_unbind); >>>> diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c >>>> index e5b6310..80f9950 100644 >>>> --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c >>>> +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c >>>> @@ -66,6 +66,7 @@ enum dsi_color { >>>> struct dw_mipi_dsi_stm { >>>> void __iomem *base; >>>> struct clk *pllref_clk; >>>> + struct dw_mipi_dsi *dmd; >>>> }; >>>> >>>> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val) >>>> @@ -318,10 +319,11 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev) >>>> dw_mipi_dsi_stm_plat_data.base = dsi->base; >>>> dw_mipi_dsi_stm_plat_data.priv_data = dsi; >>>> >>>> - ret = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data); >>>> - if (ret) { >>>> + dsi->dmd = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data); >>>> + if (IS_ERR(dsi->dmd)) { >>>> DRM_ERROR("Failed to initialize mipi dsi host\n"); >>>> clk_disable_unprepare(dsi->pllref_clk); >>>> + return PTR_ERR(dsi->dmd); >>>> } >>>> >>>> return ret; >>>> @@ -332,7 +334,7 @@ static int dw_mipi_dsi_stm_remove(struct platform_device *pdev) >>>> struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data; >>>> >>>> clk_disable_unprepare(dsi->pllref_clk); >>>> - dw_mipi_dsi_remove(pdev); >>>> + dw_mipi_dsi_remove(dsi->dmd); >>>> >>>> return 0; >>>> } >>>> diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h >>>> index 9b30fec..d9c6d54 100644 >>>> --- a/include/drm/bridge/dw_mipi_dsi.h >>>> +++ b/include/drm/bridge/dw_mipi_dsi.h >>>> @@ -10,6 +10,8 @@ >>>> #ifndef __DW_MIPI_DSI__ >>>> #define __DW_MIPI_DSI__ >>>> >>>> +struct dw_mipi_dsi; >>>> + >>>> struct dw_mipi_dsi_phy_ops { >>>> int (*init)(void *priv_data); >>>> int (*get_lane_mbps)(void *priv_data, struct drm_display_mode *mode, >>>> @@ -29,11 +31,14 @@ struct dw_mipi_dsi_plat_data { >>>> void *priv_data; >>>> }; >>>> >>>> -int dw_mipi_dsi_probe(struct platform_device *pdev, >>>> - const struct dw_mipi_dsi_plat_data *plat_data); >>>> -void dw_mipi_dsi_remove(struct platform_device *pdev); >>>> -int dw_mipi_dsi_bind(struct platform_device *pdev, struct drm_encoder *encoder, >>>> - const struct dw_mipi_dsi_plat_data *plat_data); >>>> -void dw_mipi_dsi_unbind(struct device *dev); >>>> +struct dw_mipi_dsi *dw_mipi_dsi_probe(struct platform_device *pdev, >>>> + const struct dw_mipi_dsi_plat_data >>>> + *plat_data); >>>> +void dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi); >>>> +struct dw_mipi_dsi *dw_mipi_dsi_bind(struct platform_device *pdev, >>>> + struct drm_encoder *encoder, >>>> + const struct dw_mipi_dsi_plat_data >>>> + *plat_data); >>>> +void dw_mipi_dsi_unbind(struct dw_mipi_dsi *dsi); >>>> >>>> #endif /* __DW_MIPI_DSI__ */
Hi Nickey, On Tue, Dec 05, 2017 at 05:14:11PM +0800, Nickey Yang wrote: > On 2017年12月01日 18:07, Philippe CORNU wrote: > >On 12/01/2017 10:11 AM, Nickey Yang wrote: > >>On 2017年12月01日 16:32, Philippe CORNU wrote: > >>>I am sorry to say that but you can not add my "Acked-by" to this patch > >>>because this code is different from the "original" one from Brian (which > >>>got my "Acked-by"). > >>I'm sorry I didn't think much about it, Thank you for correcting me. > >>>Sometimes it is not an issue because differences are not important but > >>>in this particular case, the code is really different: you have remove > >>>platform_set_drvdata() & platform_get_drvdata() in the stm part. > >>> > >>>Could you please go back to the original code or propose me an updated > >>>version of this code. > >>Could you help update new version of this code(stm part) and then test on > >>stm platform? > >I think you can simply goes back to the original version from Brian (see > >the discussion thread in https://patchwork.kernel.org/patch/10078493/) > >unless you have specific/good reasons for modifying the code as you did. > mmm,I'm sorry, I feel a little puzzled. Do you means we should abandon > Brian's patch (https://patchwork.kernel.org/patch/10078493/)? > I think we need to adjust stm part because dw_mipi_dsi_stm.c calls > bridge's drivers if we want merge Brian's patch. It's really simple. Your code is different from the patch I sent, and in a way that Philippe did not like. I'll highlight it again below: > >>>>diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > >>>>index e5b6310..80f9950 100644 > >>>>--- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > >>>>+++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > >>>>@@ -66,6 +66,7 @@ enum dsi_color { > >>>> struct dw_mipi_dsi_stm { > >>>> void __iomem *base; > >>>> struct clk *pllref_clk; > >>>>+ struct dw_mipi_dsi *dmd; > >>>> }; > >>>> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val) > >>>>@@ -318,10 +319,11 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev) > >>>> dw_mipi_dsi_stm_plat_data.base = dsi->base; > >>>> dw_mipi_dsi_stm_plat_data.priv_data = dsi; > >>>>- ret = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data); > >>>>- if (ret) { > >>>>+ dsi->dmd = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data); > >>>>+ if (IS_ERR(dsi->dmd)) { > >>>> DRM_ERROR("Failed to initialize mipi dsi host\n"); > >>>> clk_disable_unprepare(dsi->pllref_clk); > >>>>+ return PTR_ERR(dsi->dmd); > >>>> } > >>>> return ret; > >>>>@@ -332,7 +334,7 @@ static int dw_mipi_dsi_stm_remove(struct platform_device *pdev) > >>>> struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data; > >>>> clk_disable_unprepare(dsi->pllref_clk); > >>>>- dw_mipi_dsi_remove(pdev); > >>>>+ dw_mipi_dsi_remove(dsi->dmd); > >>>> return 0; > >>>> } Above is your diff for dw_mipi_dsi-stm.c. Particularly, notice that remove() is directly referencing the static dw_mipi_dsi_stm_plat_data struct. If you look back at my patch [1] you'll see that you're missing hunks like this: static int dw_mipi_dsi_stm_remove(struct platform_device *pdev) { - struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data; + struct dw_mipi_dsi_stm *dsi = platform_get_drvdata(pdev); clk_disable_unprepare(dsi->pllref_clk); [...] Brian [1] https://patchwork.kernel.org/patch/10078493/
Hi Brian, On 2017年12月06日 02:56, Brian Norris wrote: > Hi Nickey, > > On Tue, Dec 05, 2017 at 05:14:11PM +0800, Nickey Yang wrote: >> On 2017年12月01日 18:07, Philippe CORNU wrote: >>> On 12/01/2017 10:11 AM, Nickey Yang wrote: >>>> On 2017年12月01日 16:32, Philippe CORNU wrote: >>>>> I am sorry to say that but you can not add my "Acked-by" to this patch >>>>> because this code is different from the "original" one from Brian (which >>>>> got my "Acked-by"). >>>> I'm sorry I didn't think much about it, Thank you for correcting me. >>>>> Sometimes it is not an issue because differences are not important but >>>>> in this particular case, the code is really different: you have remove >>>>> platform_set_drvdata() & platform_get_drvdata() in the stm part. >>>>> >>>>> Could you please go back to the original code or propose me an updated >>>>> version of this code. >>>> Could you help update new version of this code(stm part) and then test on >>>> stm platform? >>> I think you can simply goes back to the original version from Brian (see >>> the discussion thread in https://patchwork.kernel.org/patch/10078493/) >>> unless you have specific/good reasons for modifying the code as you did. >> mmm,I'm sorry, I feel a little puzzled. Do you means we should abandon >> Brian's patch (https://patchwork.kernel.org/patch/10078493/)? >> I think we need to adjust stm part because dw_mipi_dsi_stm.c calls >> bridge's drivers if we want merge Brian's patch. > It's really simple. Your code is different from the patch I sent, and in > a way that Philippe did not like. I'll highlight it again below: > >>>>>> diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c >>>>>> index e5b6310..80f9950 100644 >>>>>> --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c >>>>>> +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c >>>>>> @@ -66,6 +66,7 @@ enum dsi_color { >>>>>> struct dw_mipi_dsi_stm { >>>>>> void __iomem *base; >>>>>> struct clk *pllref_clk; >>>>>> + struct dw_mipi_dsi *dmd; >>>>>> }; >>>>>> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val) >>>>>> @@ -318,10 +319,11 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev) >>>>>> dw_mipi_dsi_stm_plat_data.base = dsi->base; >>>>>> dw_mipi_dsi_stm_plat_data.priv_data = dsi; >>>>>> - ret = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data); >>>>>> - if (ret) { >>>>>> + dsi->dmd = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data); >>>>>> + if (IS_ERR(dsi->dmd)) { >>>>>> DRM_ERROR("Failed to initialize mipi dsi host\n"); >>>>>> clk_disable_unprepare(dsi->pllref_clk); >>>>>> + return PTR_ERR(dsi->dmd); >>>>>> } >>>>>> return ret; >>>>>> @@ -332,7 +334,7 @@ static int dw_mipi_dsi_stm_remove(struct platform_device *pdev) >>>>>> struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data; >>>>>> clk_disable_unprepare(dsi->pllref_clk); >>>>>> - dw_mipi_dsi_remove(pdev); >>>>>> + dw_mipi_dsi_remove(dsi->dmd); >>>>>> return 0; >>>>>> } > > Above is your diff for dw_mipi_dsi-stm.c. Particularly, notice that > remove() is directly referencing the static dw_mipi_dsi_stm_plat_data > struct. > > If you look back at my patch [1] you'll see that you're missing hunks > like this: > Thank you for pointing out my mistake. I will fix this in next version. Nickey. > static int dw_mipi_dsi_stm_remove(struct platform_device *pdev) > { > - struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data; > + struct dw_mipi_dsi_stm *dsi = platform_get_drvdata(pdev); > > clk_disable_unprepare(dsi->pllref_clk); > [...] > > Brian > > [1] https://patchwork.kernel.org/patch/10078493/ > > >
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c index d9cca4f..c39c7dc 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c @@ -922,8 +922,6 @@ static int dw_mipi_dsi_bridge_attach(struct drm_bridge *bridge) dsi->bridge.of_node = pdev->dev.of_node; #endif - dev_set_drvdata(dev, dsi); - return dsi; } @@ -935,23 +933,16 @@ static void __dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi) /* * Probe/remove API, used from platforms based on the DRM bridge API. */ -int dw_mipi_dsi_probe(struct platform_device *pdev, - const struct dw_mipi_dsi_plat_data *plat_data) +struct dw_mipi_dsi * +dw_mipi_dsi_probe(struct platform_device *pdev, + const struct dw_mipi_dsi_plat_data *plat_data) { - struct dw_mipi_dsi *dsi; - - dsi = __dw_mipi_dsi_probe(pdev, plat_data); - if (IS_ERR(dsi)) - return PTR_ERR(dsi); - - return 0; + return __dw_mipi_dsi_probe(pdev, plat_data); } EXPORT_SYMBOL_GPL(dw_mipi_dsi_probe); -void dw_mipi_dsi_remove(struct platform_device *pdev) +void dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi) { - struct dw_mipi_dsi *dsi = platform_get_drvdata(pdev); - mipi_dsi_host_unregister(&dsi->dsi_host); __dw_mipi_dsi_remove(dsi); @@ -961,31 +952,30 @@ void dw_mipi_dsi_remove(struct platform_device *pdev) /* * Bind/unbind API, used from platforms based on the component framework. */ -int dw_mipi_dsi_bind(struct platform_device *pdev, struct drm_encoder *encoder, - const struct dw_mipi_dsi_plat_data *plat_data) +struct dw_mipi_dsi * +dw_mipi_dsi_bind(struct platform_device *pdev, struct drm_encoder *encoder, + const struct dw_mipi_dsi_plat_data *plat_data) { struct dw_mipi_dsi *dsi; int ret; dsi = __dw_mipi_dsi_probe(pdev, plat_data); if (IS_ERR(dsi)) - return PTR_ERR(dsi); + return dsi; ret = drm_bridge_attach(encoder, &dsi->bridge, NULL); if (ret) { - dw_mipi_dsi_remove(pdev); + dw_mipi_dsi_remove(dsi); DRM_ERROR("Failed to initialize bridge with drm\n"); - return ret; + return ERR_PTR(ret); } - return 0; + return dsi; } EXPORT_SYMBOL_GPL(dw_mipi_dsi_bind); -void dw_mipi_dsi_unbind(struct device *dev) +void dw_mipi_dsi_unbind(struct dw_mipi_dsi *dsi) { - struct dw_mipi_dsi *dsi = dev_get_drvdata(dev); - __dw_mipi_dsi_remove(dsi); } EXPORT_SYMBOL_GPL(dw_mipi_dsi_unbind); diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c index e5b6310..80f9950 100644 --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c @@ -66,6 +66,7 @@ enum dsi_color { struct dw_mipi_dsi_stm { void __iomem *base; struct clk *pllref_clk; + struct dw_mipi_dsi *dmd; }; static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val) @@ -318,10 +319,11 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev) dw_mipi_dsi_stm_plat_data.base = dsi->base; dw_mipi_dsi_stm_plat_data.priv_data = dsi; - ret = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data); - if (ret) { + dsi->dmd = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data); + if (IS_ERR(dsi->dmd)) { DRM_ERROR("Failed to initialize mipi dsi host\n"); clk_disable_unprepare(dsi->pllref_clk); + return PTR_ERR(dsi->dmd); } return ret; @@ -332,7 +334,7 @@ static int dw_mipi_dsi_stm_remove(struct platform_device *pdev) struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data; clk_disable_unprepare(dsi->pllref_clk); - dw_mipi_dsi_remove(pdev); + dw_mipi_dsi_remove(dsi->dmd); return 0; } diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h index 9b30fec..d9c6d54 100644 --- a/include/drm/bridge/dw_mipi_dsi.h +++ b/include/drm/bridge/dw_mipi_dsi.h @@ -10,6 +10,8 @@ #ifndef __DW_MIPI_DSI__ #define __DW_MIPI_DSI__ +struct dw_mipi_dsi; + struct dw_mipi_dsi_phy_ops { int (*init)(void *priv_data); int (*get_lane_mbps)(void *priv_data, struct drm_display_mode *mode, @@ -29,11 +31,14 @@ struct dw_mipi_dsi_plat_data { void *priv_data; }; -int dw_mipi_dsi_probe(struct platform_device *pdev, - const struct dw_mipi_dsi_plat_data *plat_data); -void dw_mipi_dsi_remove(struct platform_device *pdev); -int dw_mipi_dsi_bind(struct platform_device *pdev, struct drm_encoder *encoder, - const struct dw_mipi_dsi_plat_data *plat_data); -void dw_mipi_dsi_unbind(struct device *dev); +struct dw_mipi_dsi *dw_mipi_dsi_probe(struct platform_device *pdev, + const struct dw_mipi_dsi_plat_data + *plat_data); +void dw_mipi_dsi_remove(struct dw_mipi_dsi *dsi); +struct dw_mipi_dsi *dw_mipi_dsi_bind(struct platform_device *pdev, + struct drm_encoder *encoder, + const struct dw_mipi_dsi_plat_data + *plat_data); +void dw_mipi_dsi_unbind(struct dw_mipi_dsi *dsi); #endif /* __DW_MIPI_DSI__ */