* [PATCH] drm/bridge/synopsis: stop clobbering drvdata
@ 2017-11-28 1:05 ` Brian Norris
0 siblings, 0 replies; 18+ messages in thread
From: Brian Norris @ 2017-11-28 1:05 UTC (permalink / raw)
To: Archit Taneja, Andrzej Hajda, Laurent Pinchart
Cc: David Airlie, Yannick Fertre, Philippe Cornu, Benjamin Gaignard,
Vincent Abriou, dri-devel, linux-kernel, Sean Paul, Nickey Yang,
hl, linux-rockchip, mka, Brian Norris
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>
---
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 36 ++++++++++-----------------
drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 14 +++++++----
include/drm/bridge/dw_mipi_dsi.h | 17 ++++++++-----
3 files changed, 33 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index d9cca4fd66ec..c39c7dce20ed 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 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
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 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
/*
* 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 e5b6310240fe..7ed0ef7f6ec2 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 *dsi;
};
static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val)
@@ -318,21 +319,24 @@ 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) {
+ platform_set_drvdata(pdev, dsi);
+
+ dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
+ if (IS_ERR(dsi->dsi)) {
DRM_ERROR("Failed to initialize mipi dsi host\n");
clk_disable_unprepare(dsi->pllref_clk);
+ return PTR_ERR(dsi->dsi);
}
- return ret;
+ return 0;
}
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);
- dw_mipi_dsi_remove(pdev);
+ dw_mipi_dsi_remove(dsi->dsi);
return 0;
}
diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h
index 9b30fec302c8..d9c6d549f971 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__ */
--
2.15.0.417.g466bffb3ac-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH] drm/bridge/synopsis: stop clobbering drvdata
@ 2017-11-28 1:05 ` Brian Norris
0 siblings, 0 replies; 18+ messages in thread
From: Brian Norris @ 2017-11-28 1:05 UTC (permalink / raw)
To: Archit Taneja, Andrzej Hajda, Laurent Pinchart
Cc: hl, David Airlie, Brian Norris, Philippe Cornu, dri-devel,
linux-kernel, Yannick Fertre, linux-rockchip, Nickey Yang, mka,
Vincent Abriou
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>
---
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 36 ++++++++++-----------------
drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 14 +++++++----
include/drm/bridge/dw_mipi_dsi.h | 17 ++++++++-----
3 files changed, 33 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index d9cca4fd66ec..c39c7dce20ed 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 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
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 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
/*
* 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 e5b6310240fe..7ed0ef7f6ec2 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 *dsi;
};
static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val)
@@ -318,21 +319,24 @@ 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) {
+ platform_set_drvdata(pdev, dsi);
+
+ dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
+ if (IS_ERR(dsi->dsi)) {
DRM_ERROR("Failed to initialize mipi dsi host\n");
clk_disable_unprepare(dsi->pllref_clk);
+ return PTR_ERR(dsi->dsi);
}
- return ret;
+ return 0;
}
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);
- dw_mipi_dsi_remove(pdev);
+ dw_mipi_dsi_remove(dsi->dsi);
return 0;
}
diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h
index 9b30fec302c8..d9c6d549f971 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__ */
--
2.15.0.417.g466bffb3ac-goog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/bridge/synopsis: stop clobbering drvdata
2017-11-28 1:05 ` Brian Norris
@ 2017-11-28 2:25 ` Matthias Kaehlcke
-1 siblings, 0 replies; 18+ messages in thread
From: Matthias Kaehlcke @ 2017-11-28 2:25 UTC (permalink / raw)
To: Brian Norris
Cc: Archit Taneja, Andrzej Hajda, Laurent Pinchart, David Airlie,
Yannick Fertre, Philippe Cornu, Benjamin Gaignard,
Vincent Abriou, dri-devel, linux-kernel, Sean Paul, Nickey Yang,
hl, linux-rockchip
El Mon, Nov 27, 2017 at 05:05:38PM -0800 Brian Norris ha dit:
> 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>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 36 ++++++++++-----------------
> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 14 +++++++----
> include/drm/bridge/dw_mipi_dsi.h | 17 ++++++++-----
> 3 files changed, 33 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index d9cca4fd66ec..c39c7dce20ed 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 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> 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 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
> /*
> * 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 e5b6310240fe..7ed0ef7f6ec2 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 *dsi;
> };
>
> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val)
> @@ -318,21 +319,24 @@ 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) {
> + platform_set_drvdata(pdev, dsi);
> +
> + dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
> + if (IS_ERR(dsi->dsi)) {
> DRM_ERROR("Failed to initialize mipi dsi host\n");
> clk_disable_unprepare(dsi->pllref_clk);
> + return PTR_ERR(dsi->dsi);
> }
>
> - return ret;
> + return 0;
> }
>
> 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);
> - dw_mipi_dsi_remove(pdev);
> + dw_mipi_dsi_remove(dsi->dsi);
>
> return 0;
> }
> diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h
> index 9b30fec302c8..d9c6d549f971 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__ */
I don't claim to have expertise in this subsystem, but FWIW:
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/bridge/synopsis: stop clobbering drvdata
@ 2017-11-28 2:25 ` Matthias Kaehlcke
0 siblings, 0 replies; 18+ messages in thread
From: Matthias Kaehlcke @ 2017-11-28 2:25 UTC (permalink / raw)
To: Brian Norris
Cc: hl, linux-rockchip, David Airlie, Philippe Cornu, dri-devel,
linux-kernel, Yannick Fertre, Nickey Yang, Laurent Pinchart,
Vincent Abriou
El Mon, Nov 27, 2017 at 05:05:38PM -0800 Brian Norris ha dit:
> 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>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 36 ++++++++++-----------------
> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 14 +++++++----
> include/drm/bridge/dw_mipi_dsi.h | 17 ++++++++-----
> 3 files changed, 33 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index d9cca4fd66ec..c39c7dce20ed 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 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> 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 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
> /*
> * 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 e5b6310240fe..7ed0ef7f6ec2 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 *dsi;
> };
>
> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val)
> @@ -318,21 +319,24 @@ 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) {
> + platform_set_drvdata(pdev, dsi);
> +
> + dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
> + if (IS_ERR(dsi->dsi)) {
> DRM_ERROR("Failed to initialize mipi dsi host\n");
> clk_disable_unprepare(dsi->pllref_clk);
> + return PTR_ERR(dsi->dsi);
> }
>
> - return ret;
> + return 0;
> }
>
> 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);
> - dw_mipi_dsi_remove(pdev);
> + dw_mipi_dsi_remove(dsi->dsi);
>
> return 0;
> }
> diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h
> index 9b30fec302c8..d9c6d549f971 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__ */
I don't claim to have expertise in this subsystem, but FWIW:
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/bridge/synopsis: stop clobbering drvdata
2017-11-28 1:05 ` Brian Norris
(?)
(?)
@ 2017-11-28 6:27 ` Archit Taneja
-1 siblings, 0 replies; 18+ messages in thread
From: Archit Taneja @ 2017-11-28 6:27 UTC (permalink / raw)
To: Brian Norris, Andrzej Hajda, Laurent Pinchart
Cc: David Airlie, Yannick Fertre, Philippe Cornu, Benjamin Gaignard,
Vincent Abriou, dri-devel, linux-kernel, Sean Paul, Nickey Yang,
hl, linux-rockchip, mka
On 11/28/2017 06:35 AM, Brian Norris wrote:
> 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.
>
Reviewed-by: Archit Taneja <architt@codeaurora.org>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 36 ++++++++++-----------------
> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 14 +++++++----
> include/drm/bridge/dw_mipi_dsi.h | 17 ++++++++-----
> 3 files changed, 33 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index d9cca4fd66ec..c39c7dce20ed 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 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> 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 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
> /*
> * 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 e5b6310240fe..7ed0ef7f6ec2 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 *dsi;
> };
>
> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val)
> @@ -318,21 +319,24 @@ 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) {
> + platform_set_drvdata(pdev, dsi);
> +
> + dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
> + if (IS_ERR(dsi->dsi)) {
> DRM_ERROR("Failed to initialize mipi dsi host\n");
> clk_disable_unprepare(dsi->pllref_clk);
> + return PTR_ERR(dsi->dsi);
> }
>
> - return ret;
> + return 0;
> }
>
> 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);
> - dw_mipi_dsi_remove(pdev);
> + dw_mipi_dsi_remove(dsi->dsi);
>
> return 0;
> }
> diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h
> index 9b30fec302c8..d9c6d549f971 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__ */
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/bridge/synopsis: stop clobbering drvdata
2017-11-28 1:05 ` Brian Norris
@ 2017-11-28 9:34 ` Philippe CORNU
-1 siblings, 0 replies; 18+ messages in thread
From: Philippe CORNU @ 2017-11-28 9:34 UTC (permalink / raw)
To: Brian Norris, Archit Taneja, Andrzej Hajda, Laurent Pinchart
Cc: David Airlie, Yannick FERTRE, Benjamin Gaignard, Vincent ABRIOU,
dri-devel, linux-kernel, Sean Paul, Nickey Yang, hl,
linux-rockchip, mka
Hi Brian,
On 11/28/2017 02:05 AM, Brian Norris wrote:
> 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.
And many thanks for this cleanup.
(please update the headline with "synopsys")
Successfully tested on stm.
Acked-by: Philippe Cornu <philippe.cornu@st.com>
Many thanks,
Philippe :-)
>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 36 ++++++++++-----------------
> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 14 +++++++----
> include/drm/bridge/dw_mipi_dsi.h | 17 ++++++++-----
> 3 files changed, 33 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index d9cca4fd66ec..c39c7dce20ed 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 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> 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 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
> /*
> * 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 e5b6310240fe..7ed0ef7f6ec2 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 *dsi;
> };
>
> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val)
> @@ -318,21 +319,24 @@ 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) {
> + platform_set_drvdata(pdev, dsi);
> +
> + dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
> + if (IS_ERR(dsi->dsi)) {
> DRM_ERROR("Failed to initialize mipi dsi host\n");
> clk_disable_unprepare(dsi->pllref_clk);
> + return PTR_ERR(dsi->dsi);
> }
>
> - return ret;
> + return 0;
> }
>
> 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);
> - dw_mipi_dsi_remove(pdev);
> + dw_mipi_dsi_remove(dsi->dsi);
>
> return 0;
> }
> diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h
> index 9b30fec302c8..d9c6d549f971 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__ */
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/bridge/synopsis: stop clobbering drvdata
@ 2017-11-28 9:34 ` Philippe CORNU
0 siblings, 0 replies; 18+ messages in thread
From: Philippe CORNU @ 2017-11-28 9:34 UTC (permalink / raw)
To: Brian Norris, Archit Taneja, Andrzej Hajda, Laurent Pinchart
Cc: hl, David Airlie, linux-kernel, dri-devel, Yannick FERTRE,
linux-rockchip, Nickey Yang, mka, Vincent ABRIOU
Hi Brian,
On 11/28/2017 02:05 AM, Brian Norris wrote:
> 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.
And many thanks for this cleanup.
(please update the headline with "synopsys")
Successfully tested on stm.
Acked-by: Philippe Cornu <philippe.cornu@st.com>
Many thanks,
Philippe :-)
>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 36 ++++++++++-----------------
> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 14 +++++++----
> include/drm/bridge/dw_mipi_dsi.h | 17 ++++++++-----
> 3 files changed, 33 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index d9cca4fd66ec..c39c7dce20ed 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 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> 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 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
> /*
> * 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 e5b6310240fe..7ed0ef7f6ec2 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 *dsi;
> };
>
> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val)
> @@ -318,21 +319,24 @@ 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) {
> + platform_set_drvdata(pdev, dsi);
> +
> + dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
> + if (IS_ERR(dsi->dsi)) {
> DRM_ERROR("Failed to initialize mipi dsi host\n");
> clk_disable_unprepare(dsi->pllref_clk);
> + return PTR_ERR(dsi->dsi);
> }
>
> - return ret;
> + return 0;
> }
>
> 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);
> - dw_mipi_dsi_remove(pdev);
> + dw_mipi_dsi_remove(dsi->dsi);
>
> return 0;
> }
> diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h
> index 9b30fec302c8..d9c6d549f971 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__ */
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/bridge/synopsis: stop clobbering drvdata
2017-11-28 1:05 ` Brian Norris
` (3 preceding siblings ...)
(?)
@ 2017-11-28 12:51 ` Laurent Pinchart
2017-11-28 18:21 ` Brian Norris
-1 siblings, 1 reply; 18+ messages in thread
From: Laurent Pinchart @ 2017-11-28 12:51 UTC (permalink / raw)
To: Brian Norris
Cc: Archit Taneja, Andrzej Hajda, David Airlie, Yannick Fertre,
Philippe Cornu, Benjamin Gaignard, Vincent Abriou, dri-devel,
linux-kernel, Sean Paul, Nickey Yang, hl, linux-rockchip, mka
Hi Brian,
Thank you for the patch.
I'd mention dw-mipi-dsi in the subject line as the directory contains the dw-
hdmi driver as well that this patch doesn't touch.
On Tuesday, 28 November 2017 03:05:38 EET Brian Norris wrote:
> Bridge drivers/helpers shouldn't be clobbering the drvdata, since a
> parent driver might need to own this.
By parent driver I assume you mean a glue driver that binds to the SoC-
specific compatible string for the DSI transmitter.
> 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>
Wouldn't it be cleaner to embed the dw_mipi_dsi structure in the parent-
specific data structure (struct dw_mipi_dsi_stm and struct
dw_mipi_dsi_rockchip when the "[PATCH v3 0/5] Update ROCKCHIP DSI driver that
uses dw-mipi-dsi bridge" patch series will land) instead of allocating it
dynamically ? We would then have a single object to track.
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 36 ++++++++++--------------
> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 14 +++++++----
> include/drm/bridge/dw_mipi_dsi.h | 17 ++++++++-----
> 3 files changed, 33 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c index
> d9cca4fd66ec..c39c7dce20ed 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 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> 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 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
> /*
> * 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 e5b6310240fe..7ed0ef7f6ec2
> 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 *dsi;
> };
>
> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val)
> @@ -318,21 +319,24 @@ 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) {
> + platform_set_drvdata(pdev, dsi);
> +
> + dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
> + if (IS_ERR(dsi->dsi)) {
> DRM_ERROR("Failed to initialize mipi dsi host\n");
> clk_disable_unprepare(dsi->pllref_clk);
> + return PTR_ERR(dsi->dsi);
> }
>
> - return ret;
> + return 0;
> }
>
> 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);
> - dw_mipi_dsi_remove(pdev);
> + dw_mipi_dsi_remove(dsi->dsi);
>
> return 0;
> }
> diff --git a/include/drm/bridge/dw_mipi_dsi.h
> b/include/drm/bridge/dw_mipi_dsi.h index 9b30fec302c8..d9c6d549f971 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__ */
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/bridge/synopsis: stop clobbering drvdata
2017-11-28 12:51 ` Laurent Pinchart
@ 2017-11-28 18:21 ` Brian Norris
0 siblings, 0 replies; 18+ messages in thread
From: Brian Norris @ 2017-11-28 18:21 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Archit Taneja, Andrzej Hajda, David Airlie, Yannick Fertre,
Philippe Cornu, Benjamin Gaignard, Vincent Abriou, dri-devel,
linux-kernel, Sean Paul, Nickey Yang, hl, linux-rockchip, mka,
Jeffy Chen, Doug Anderson
Hi Laurent,
On Tue, Nov 28, 2017 at 02:51:46PM +0200, Laurent Pinchart wrote:
> Hi Brian,
>
> Thank you for the patch.
>
> I'd mention dw-mipi-dsi in the subject line as the directory contains the dw-
> hdmi driver as well that this patch doesn't touch.
Yep. Does it need another tag in the subject? e.g., '.../dw-mipi-dsi:'?
> On Tuesday, 28 November 2017 03:05:38 EET Brian Norris wrote:
> > Bridge drivers/helpers shouldn't be clobbering the drvdata, since a
> > parent driver might need to own this.
>
> By parent driver I assume you mean a glue driver that binds to the SoC-
> specific compatible string for the DSI transmitter.
Indeed. Nickey picked this up for his Rockchip driver submission, but
maybe we should reword the commit message a bit.
> > 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>
>
> Wouldn't it be cleaner to embed the dw_mipi_dsi structure in the parent-
> specific data structure (struct dw_mipi_dsi_stm and struct
> dw_mipi_dsi_rockchip when the "[PATCH v3 0/5] Update ROCKCHIP DSI driver that
> uses dw-mipi-dsi bridge" patch series will land) instead of allocating it
> dynamically ? We would then have a single object to track.
I suppose we could do that too. But that would require exposing the
whole layout of 'struct dw_mipi_dsi' to users. Do we want to sacrifice
the enforced separation for a little bit of nicer object handling?
Also, this was modeled a bit after the similar rework needed to untangle
the drvdata handling in the Rockchip analogix DP driver vs. the analogix
bridge DP code:
[PATCH v6 03/10] drm/bridge: analogix: Do not use device's drvdata
https://patchwork.kernel.org/patch/10015875/
Brian
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/bridge/synopsis: stop clobbering drvdata
@ 2017-11-28 18:21 ` Brian Norris
0 siblings, 0 replies; 18+ messages in thread
From: Brian Norris @ 2017-11-28 18:21 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Doug Anderson, hl, linux-rockchip, David Airlie, Jeffy Chen,
Philippe Cornu, dri-devel, linux-kernel, Yannick Fertre,
Nickey Yang, mka, Vincent Abriou
Hi Laurent,
On Tue, Nov 28, 2017 at 02:51:46PM +0200, Laurent Pinchart wrote:
> Hi Brian,
>
> Thank you for the patch.
>
> I'd mention dw-mipi-dsi in the subject line as the directory contains the dw-
> hdmi driver as well that this patch doesn't touch.
Yep. Does it need another tag in the subject? e.g., '.../dw-mipi-dsi:'?
> On Tuesday, 28 November 2017 03:05:38 EET Brian Norris wrote:
> > Bridge drivers/helpers shouldn't be clobbering the drvdata, since a
> > parent driver might need to own this.
>
> By parent driver I assume you mean a glue driver that binds to the SoC-
> specific compatible string for the DSI transmitter.
Indeed. Nickey picked this up for his Rockchip driver submission, but
maybe we should reword the commit message a bit.
> > 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>
>
> Wouldn't it be cleaner to embed the dw_mipi_dsi structure in the parent-
> specific data structure (struct dw_mipi_dsi_stm and struct
> dw_mipi_dsi_rockchip when the "[PATCH v3 0/5] Update ROCKCHIP DSI driver that
> uses dw-mipi-dsi bridge" patch series will land) instead of allocating it
> dynamically ? We would then have a single object to track.
I suppose we could do that too. But that would require exposing the
whole layout of 'struct dw_mipi_dsi' to users. Do we want to sacrifice
the enforced separation for a little bit of nicer object handling?
Also, this was modeled a bit after the similar rework needed to untangle
the drvdata handling in the Rockchip analogix DP driver vs. the analogix
bridge DP code:
[PATCH v6 03/10] drm/bridge: analogix: Do not use device's drvdata
https://patchwork.kernel.org/patch/10015875/
Brian
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/bridge/synopsis: stop clobbering drvdata
2017-11-28 9:34 ` Philippe CORNU
@ 2018-01-09 13:01 ` Philippe CORNU
-1 siblings, 0 replies; 18+ messages in thread
From: Philippe CORNU @ 2018-01-09 13:01 UTC (permalink / raw)
To: Brian Norris, Archit Taneja, Andrzej Hajda, Laurent Pinchart
Cc: David Airlie, Yannick FERTRE, Benjamin Gaignard, Vincent ABRIOU,
dri-devel, linux-kernel, Sean Paul, Nickey Yang, hl,
linux-rockchip, mka, Nickey Yang
Hi Archit, Andrzej & Laurent,
Regarding this patch from Brian, I think it could be nice to merge it
(1xAcked-by, 2xReviewed-by).
Could you please have a look?
Only the small "typo" in the headline needs to be changed.
Many thanks,
Philippe :-)
On 11/28/2017 10:34 AM, Philippe CORNU wrote:
> Hi Brian,
>
> On 11/28/2017 02:05 AM, Brian Norris wrote:
>> 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.
>
> And many thanks for this cleanup.
> (please update the headline with "synopsys")
>
> Successfully tested on stm.
>
> Acked-by: Philippe Cornu <philippe.cornu@st.com>
>
> Many thanks,
> Philippe :-)
>>
>> Signed-off-by: Brian Norris <briannorris@chromium.org>
>> ---
>> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 36
>> ++++++++++-----------------
>> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 14 +++++++----
>> include/drm/bridge/dw_mipi_dsi.h | 17 ++++++++-----
>> 3 files changed, 33 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>> index d9cca4fd66ec..c39c7dce20ed 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 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
>> 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 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
>> /*
>> * 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 e5b6310240fe..7ed0ef7f6ec2 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 *dsi;
>> };
>> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg,
>> u32 val)
>> @@ -318,21 +319,24 @@ 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) {
>> + platform_set_drvdata(pdev, dsi);
>> +
>> + dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
>> + if (IS_ERR(dsi->dsi)) {
>> DRM_ERROR("Failed to initialize mipi dsi host\n");
>> clk_disable_unprepare(dsi->pllref_clk);
>> + return PTR_ERR(dsi->dsi);
>> }
>> - return ret;
>> + return 0;
>> }
>> 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);
>> - dw_mipi_dsi_remove(pdev);
>> + dw_mipi_dsi_remove(dsi->dsi);
>> return 0;
>> }
>> diff --git a/include/drm/bridge/dw_mipi_dsi.h
>> b/include/drm/bridge/dw_mipi_dsi.h
>> index 9b30fec302c8..d9c6d549f971 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__ */
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/bridge/synopsis: stop clobbering drvdata
@ 2018-01-09 13:01 ` Philippe CORNU
0 siblings, 0 replies; 18+ messages in thread
From: Philippe CORNU @ 2018-01-09 13:01 UTC (permalink / raw)
To: Brian Norris, Archit Taneja, Andrzej Hajda, Laurent Pinchart
Cc: hl, David Airlie, linux-kernel, dri-devel, Yannick FERTRE,
linux-rockchip, Nickey Yang, mka, Vincent ABRIOU
Hi Archit, Andrzej & Laurent,
Regarding this patch from Brian, I think it could be nice to merge it
(1xAcked-by, 2xReviewed-by).
Could you please have a look?
Only the small "typo" in the headline needs to be changed.
Many thanks,
Philippe :-)
On 11/28/2017 10:34 AM, Philippe CORNU wrote:
> Hi Brian,
>
> On 11/28/2017 02:05 AM, Brian Norris wrote:
>> 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.
>
> And many thanks for this cleanup.
> (please update the headline with "synopsys")
>
> Successfully tested on stm.
>
> Acked-by: Philippe Cornu <philippe.cornu@st.com>
>
> Many thanks,
> Philippe :-)
>>
>> Signed-off-by: Brian Norris <briannorris@chromium.org>
>> ---
>> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 36
>> ++++++++++-----------------
>> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 14 +++++++----
>> include/drm/bridge/dw_mipi_dsi.h | 17 ++++++++-----
>> 3 files changed, 33 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>> index d9cca4fd66ec..c39c7dce20ed 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 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
>> 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 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
>> /*
>> * 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 e5b6310240fe..7ed0ef7f6ec2 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 *dsi;
>> };
>> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg,
>> u32 val)
>> @@ -318,21 +319,24 @@ 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) {
>> + platform_set_drvdata(pdev, dsi);
>> +
>> + dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
>> + if (IS_ERR(dsi->dsi)) {
>> DRM_ERROR("Failed to initialize mipi dsi host\n");
>> clk_disable_unprepare(dsi->pllref_clk);
>> + return PTR_ERR(dsi->dsi);
>> }
>> - return ret;
>> + return 0;
>> }
>> 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);
>> - dw_mipi_dsi_remove(pdev);
>> + dw_mipi_dsi_remove(dsi->dsi);
>> return 0;
>> }
>> diff --git a/include/drm/bridge/dw_mipi_dsi.h
>> b/include/drm/bridge/dw_mipi_dsi.h
>> index 9b30fec302c8..d9c6d549f971 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__ */
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/bridge/synopsis: stop clobbering drvdata
2017-11-28 18:21 ` Brian Norris
@ 2018-01-09 13:36 ` Laurent Pinchart
-1 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2018-01-09 13:36 UTC (permalink / raw)
To: dri-devel
Cc: Brian Norris, Doug Anderson, hl, linux-rockchip, David Airlie,
Jeffy Chen, Philippe Cornu, linux-kernel, Yannick Fertre,
Nickey Yang, mka, Vincent Abriou
Hi Brian,
On Tuesday, 28 November 2017 20:21:23 EET Brian Norris wrote:
> On Tue, Nov 28, 2017 at 02:51:46PM +0200, Laurent Pinchart wrote:
> > Hi Brian,
> >
> > Thank you for the patch.
> >
> > I'd mention dw-mipi-dsi in the subject line as the directory contains the
> > dw-hdmi driver as well that this patch doesn't touch.
>
> Yep. Does it need another tag in the subject? e.g., '.../dw-mipi-dsi:'?
>
> > On Tuesday, 28 November 2017 03:05:38 EET Brian Norris wrote:
> >> Bridge drivers/helpers shouldn't be clobbering the drvdata, since a
> >> parent driver might need to own this.
> >
> > By parent driver I assume you mean a glue driver that binds to the SoC-
> > specific compatible string for the DSI transmitter.
>
> Indeed. Nickey picked this up for his Rockchip driver submission, but
> maybe we should reword the commit message a bit.
How about "drm: dw-mipi-dsi: Stop clobbering drvdata" ?
> >> 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>
> >
> > Wouldn't it be cleaner to embed the dw_mipi_dsi structure in the parent-
> > specific data structure (struct dw_mipi_dsi_stm and struct
> > dw_mipi_dsi_rockchip when the "[PATCH v3 0/5] Update ROCKCHIP DSI driver
> > that uses dw-mipi-dsi bridge" patch series will land) instead of
> > allocating it dynamically ? We would then have a single object to track.
>
> I suppose we could do that too. But that would require exposing the
> whole layout of 'struct dw_mipi_dsi' to users. Do we want to sacrifice
> the enforced separation for a little bit of nicer object handling?
I certainly don't think we should go for spaghetti code with all objects
accessing each other :) On the other hand, we're talking about C code, and we
thus have no way to enforce access restrictions in the compiler, so it's a
lost battle anyway. I don't see an issue with exposing the object in the sense
of moving its definition to a header file if it results in cleaner code. I
think we need to trust developers not to abuse internal APIs, and if they do,
catch it during review.
> Also, this was modeled a bit after the similar rework needed to untangle
> the drvdata handling in the Rockchip analogix DP driver vs. the analogix
> bridge DP code:
>
> [PATCH v6 03/10] drm/bridge: analogix: Do not use device's drvdata
> https://patchwork.kernel.org/patch/10015875/
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/bridge/synopsis: stop clobbering drvdata
@ 2018-01-09 13:36 ` Laurent Pinchart
0 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2018-01-09 13:36 UTC (permalink / raw)
To: dri-devel
Cc: Brian Norris, hl, David Airlie, linux-kernel, Jeffy Chen,
Doug Anderson, Philippe Cornu, Yannick Fertre, linux-rockchip,
Nickey Yang, mka, Vincent Abriou
Hi Brian,
On Tuesday, 28 November 2017 20:21:23 EET Brian Norris wrote:
> On Tue, Nov 28, 2017 at 02:51:46PM +0200, Laurent Pinchart wrote:
> > Hi Brian,
> >
> > Thank you for the patch.
> >
> > I'd mention dw-mipi-dsi in the subject line as the directory contains the
> > dw-hdmi driver as well that this patch doesn't touch.
>
> Yep. Does it need another tag in the subject? e.g., '.../dw-mipi-dsi:'?
>
> > On Tuesday, 28 November 2017 03:05:38 EET Brian Norris wrote:
> >> Bridge drivers/helpers shouldn't be clobbering the drvdata, since a
> >> parent driver might need to own this.
> >
> > By parent driver I assume you mean a glue driver that binds to the SoC-
> > specific compatible string for the DSI transmitter.
>
> Indeed. Nickey picked this up for his Rockchip driver submission, but
> maybe we should reword the commit message a bit.
How about "drm: dw-mipi-dsi: Stop clobbering drvdata" ?
> >> 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>
> >
> > Wouldn't it be cleaner to embed the dw_mipi_dsi structure in the parent-
> > specific data structure (struct dw_mipi_dsi_stm and struct
> > dw_mipi_dsi_rockchip when the "[PATCH v3 0/5] Update ROCKCHIP DSI driver
> > that uses dw-mipi-dsi bridge" patch series will land) instead of
> > allocating it dynamically ? We would then have a single object to track.
>
> I suppose we could do that too. But that would require exposing the
> whole layout of 'struct dw_mipi_dsi' to users. Do we want to sacrifice
> the enforced separation for a little bit of nicer object handling?
I certainly don't think we should go for spaghetti code with all objects
accessing each other :) On the other hand, we're talking about C code, and we
thus have no way to enforce access restrictions in the compiler, so it's a
lost battle anyway. I don't see an issue with exposing the object in the sense
of moving its definition to a header file if it results in cleaner code. I
think we need to trust developers not to abuse internal APIs, and if they do,
catch it during review.
> Also, this was modeled a bit after the similar rework needed to untangle
> the drvdata handling in the Rockchip analogix DP driver vs. the analogix
> bridge DP code:
>
> [PATCH v6 03/10] drm/bridge: analogix: Do not use device's drvdata
> https://patchwork.kernel.org/patch/10015875/
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/bridge/synopsis: stop clobbering drvdata
2018-01-09 13:01 ` Philippe CORNU
@ 2018-01-09 13:37 ` Laurent Pinchart
-1 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2018-01-09 13:37 UTC (permalink / raw)
To: Philippe CORNU
Cc: Brian Norris, Archit Taneja, Andrzej Hajda, David Airlie,
Yannick FERTRE, Benjamin Gaignard, Vincent ABRIOU, dri-devel,
linux-kernel, Sean Paul, Nickey Yang, hl, linux-rockchip, mka
Hi Philippe,
On Tuesday, 9 January 2018 15:01:02 EET Philippe CORNU wrote:
> Hi Archit, Andrzej & Laurent,
>
> Regarding this patch from Brian, I think it could be nice to merge it
> (1xAcked-by, 2xReviewed-by).
>
> Could you please have a look?
>
> Only the small "typo" in the headline needs to be changed.
>
> Many thanks,
I've just replied to Brian in this mail thread. Sorry for the delay, it
slipped through the cracks. Thank you for pinging me.
> On 11/28/2017 10:34 AM, Philippe CORNU wrote:
>
> > Hi Brian,
> >
> > On 11/28/2017 02:05 AM, Brian Norris wrote:
> >
> >> 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.
> >
> >
> > And many thanks for this cleanup.
> > (please update the headline with "synopsys")
> >
> > Successfully tested on stm.
> >
> > Acked-by: Philippe Cornu <philippe.cornu@st.com>
> >
> > Many thanks,
> > Philippe :-)
> >
> >>
> >>
> >> Signed-off-by: Brian Norris <briannorris@chromium.org>
> >> ---
> >> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 36
> >> ++++++++++-----------------
> >> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 14 +++++++----
> >> include/drm/bridge/dw_mipi_dsi.h | 17 ++++++++-----
> >> 3 files changed, 33 insertions(+), 34 deletions(-)
> >>
> >>
> >>
> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> >> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> >> index d9cca4fd66ec..c39c7dce20ed 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 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> >> 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 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
> >> /*
> >> * 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 e5b6310240fe..7ed0ef7f6ec2 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 *dsi;
> >> };
> >> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg,
> >> u32 val)
> >> @@ -318,21 +319,24 @@ 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) {
> >> + platform_set_drvdata(pdev, dsi);
> >> +
> >> + dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
> >> + if (IS_ERR(dsi->dsi)) {
> >> DRM_ERROR("Failed to initialize mipi dsi host\n");
> >> clk_disable_unprepare(dsi->pllref_clk);
> >> + return PTR_ERR(dsi->dsi);
> >> }
> >> - return ret;
> >> + return 0;
> >> }
> >> 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);
> >> - dw_mipi_dsi_remove(pdev);
> >> + dw_mipi_dsi_remove(dsi->dsi);
> >> return 0;
> >> }
> >> diff --git a/include/drm/bridge/dw_mipi_dsi.h
> >> b/include/drm/bridge/dw_mipi_dsi.h
> >> index 9b30fec302c8..d9c6d549f971 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__ */
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/bridge/synopsis: stop clobbering drvdata
@ 2018-01-09 13:37 ` Laurent Pinchart
0 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2018-01-09 13:37 UTC (permalink / raw)
To: Philippe CORNU
Cc: hl, linux-rockchip, David Airlie, Brian Norris, linux-kernel,
dri-devel, Yannick FERTRE, Nickey Yang, mka, Vincent ABRIOU
Hi Philippe,
On Tuesday, 9 January 2018 15:01:02 EET Philippe CORNU wrote:
> Hi Archit, Andrzej & Laurent,
>
> Regarding this patch from Brian, I think it could be nice to merge it
> (1xAcked-by, 2xReviewed-by).
>
> Could you please have a look?
>
> Only the small "typo" in the headline needs to be changed.
>
> Many thanks,
I've just replied to Brian in this mail thread. Sorry for the delay, it
slipped through the cracks. Thank you for pinging me.
> On 11/28/2017 10:34 AM, Philippe CORNU wrote:
>
> > Hi Brian,
> >
> > On 11/28/2017 02:05 AM, Brian Norris wrote:
> >
> >> 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.
> >
> >
> > And many thanks for this cleanup.
> > (please update the headline with "synopsys")
> >
> > Successfully tested on stm.
> >
> > Acked-by: Philippe Cornu <philippe.cornu@st.com>
> >
> > Many thanks,
> > Philippe :-)
> >
> >>
> >>
> >> Signed-off-by: Brian Norris <briannorris@chromium.org>
> >> ---
> >> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 36
> >> ++++++++++-----------------
> >> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 14 +++++++----
> >> include/drm/bridge/dw_mipi_dsi.h | 17 ++++++++-----
> >> 3 files changed, 33 insertions(+), 34 deletions(-)
> >>
> >>
> >>
> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> >> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> >> index d9cca4fd66ec..c39c7dce20ed 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 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> >> 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 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
> >> /*
> >> * 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 e5b6310240fe..7ed0ef7f6ec2 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 *dsi;
> >> };
> >> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg,
> >> u32 val)
> >> @@ -318,21 +319,24 @@ 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) {
> >> + platform_set_drvdata(pdev, dsi);
> >> +
> >> + dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
> >> + if (IS_ERR(dsi->dsi)) {
> >> DRM_ERROR("Failed to initialize mipi dsi host\n");
> >> clk_disable_unprepare(dsi->pllref_clk);
> >> + return PTR_ERR(dsi->dsi);
> >> }
> >> - return ret;
> >> + return 0;
> >> }
> >> 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);
> >> - dw_mipi_dsi_remove(pdev);
> >> + dw_mipi_dsi_remove(dsi->dsi);
> >> return 0;
> >> }
> >> diff --git a/include/drm/bridge/dw_mipi_dsi.h
> >> b/include/drm/bridge/dw_mipi_dsi.h
> >> index 9b30fec302c8..d9c6d549f971 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__ */
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/bridge/synopsis: stop clobbering drvdata
2018-01-09 13:37 ` Laurent Pinchart
@ 2018-01-09 13:45 ` Andrzej Hajda
-1 siblings, 0 replies; 18+ messages in thread
From: Andrzej Hajda @ 2018-01-09 13:45 UTC (permalink / raw)
To: Laurent Pinchart, Philippe CORNU
Cc: Brian Norris, Archit Taneja, David Airlie, Yannick FERTRE,
Benjamin Gaignard, Vincent ABRIOU, dri-devel, linux-kernel,
Sean Paul, Nickey Yang, hl, linux-rockchip, mka
On 09.01.2018 14:37, Laurent Pinchart wrote:
> Hi Philippe,
>
> On Tuesday, 9 January 2018 15:01:02 EET Philippe CORNU wrote:
>> Hi Archit, Andrzej & Laurent,
>>
>> Regarding this patch from Brian, I think it could be nice to merge it
>> (1xAcked-by, 2xReviewed-by).
>>
>> Could you please have a look?
>>
>> Only the small "typo" in the headline needs to be changed.
>>
>> Many thanks,
> I've just replied to Brian in this mail thread. Sorry for the delay, it
> slipped through the cracks. Thank you for pinging me.
I have just pushed the patch to drm-misc-next. I am sorry, if you wanted
to polish it more, from the thread it looked it can be merged as is.
Regards
Andrzej
>> On 11/28/2017 10:34 AM, Philippe CORNU wrote:
>>
>>> Hi Brian,
>>>
>>> On 11/28/2017 02:05 AM, Brian Norris wrote:
>>>
>>>> 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.
>>>
>>> And many thanks for this cleanup.
>>> (please update the headline with "synopsys")
>>>
>>> Successfully tested on stm.
>>>
>>> Acked-by: Philippe Cornu <philippe.cornu@st.com>
>>>
>>> Many thanks,
>>> Philippe :-)
>>>
>>>>
>>>> Signed-off-by: Brian Norris <briannorris@chromium.org>
>>>> ---
>>>> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 36
>>>> ++++++++++-----------------
>>>> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 14 +++++++----
>>>> include/drm/bridge/dw_mipi_dsi.h | 17 ++++++++-----
>>>> 3 files changed, 33 insertions(+), 34 deletions(-)
>>>>
>>>>
>>>>
>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>>>> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>>>> index d9cca4fd66ec..c39c7dce20ed 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 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
>>>> 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 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
>>>> /*
>>>> * 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 e5b6310240fe..7ed0ef7f6ec2 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 *dsi;
>>>> };
>>>> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg,
>>>> u32 val)
>>>> @@ -318,21 +319,24 @@ 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) {
>>>> + platform_set_drvdata(pdev, dsi);
>>>> +
>>>> + dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
>>>> + if (IS_ERR(dsi->dsi)) {
>>>> DRM_ERROR("Failed to initialize mipi dsi host\n");
>>>> clk_disable_unprepare(dsi->pllref_clk);
>>>> + return PTR_ERR(dsi->dsi);
>>>> }
>>>> - return ret;
>>>> + return 0;
>>>> }
>>>> 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);
>>>> - dw_mipi_dsi_remove(pdev);
>>>> + dw_mipi_dsi_remove(dsi->dsi);
>>>> return 0;
>>>> }
>>>> diff --git a/include/drm/bridge/dw_mipi_dsi.h
>>>> b/include/drm/bridge/dw_mipi_dsi.h
>>>> index 9b30fec302c8..d9c6d549f971 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__ */
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] drm/bridge/synopsis: stop clobbering drvdata
@ 2018-01-09 13:45 ` Andrzej Hajda
0 siblings, 0 replies; 18+ messages in thread
From: Andrzej Hajda @ 2018-01-09 13:45 UTC (permalink / raw)
To: Laurent Pinchart, Philippe CORNU
Cc: hl, David Airlie, Brian Norris, linux-kernel, dri-devel,
Yannick FERTRE, linux-rockchip, Nickey Yang, mka, Vincent ABRIOU
On 09.01.2018 14:37, Laurent Pinchart wrote:
> Hi Philippe,
>
> On Tuesday, 9 January 2018 15:01:02 EET Philippe CORNU wrote:
>> Hi Archit, Andrzej & Laurent,
>>
>> Regarding this patch from Brian, I think it could be nice to merge it
>> (1xAcked-by, 2xReviewed-by).
>>
>> Could you please have a look?
>>
>> Only the small "typo" in the headline needs to be changed.
>>
>> Many thanks,
> I've just replied to Brian in this mail thread. Sorry for the delay, it
> slipped through the cracks. Thank you for pinging me.
I have just pushed the patch to drm-misc-next. I am sorry, if you wanted
to polish it more, from the thread it looked it can be merged as is.
Regards
Andrzej
>> On 11/28/2017 10:34 AM, Philippe CORNU wrote:
>>
>>> Hi Brian,
>>>
>>> On 11/28/2017 02:05 AM, Brian Norris wrote:
>>>
>>>> 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.
>>>
>>> And many thanks for this cleanup.
>>> (please update the headline with "synopsys")
>>>
>>> Successfully tested on stm.
>>>
>>> Acked-by: Philippe Cornu <philippe.cornu@st.com>
>>>
>>> Many thanks,
>>> Philippe :-)
>>>
>>>>
>>>> Signed-off-by: Brian Norris <briannorris@chromium.org>
>>>> ---
>>>> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 36
>>>> ++++++++++-----------------
>>>> drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 14 +++++++----
>>>> include/drm/bridge/dw_mipi_dsi.h | 17 ++++++++-----
>>>> 3 files changed, 33 insertions(+), 34 deletions(-)
>>>>
>>>>
>>>>
>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>>>> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>>>> index d9cca4fd66ec..c39c7dce20ed 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 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
>>>> 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 @@ EXPORT_SYMBOL_GPL(dw_mipi_dsi_remove);
>>>> /*
>>>> * 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 e5b6310240fe..7ed0ef7f6ec2 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 *dsi;
>>>> };
>>>> static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg,
>>>> u32 val)
>>>> @@ -318,21 +319,24 @@ 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) {
>>>> + platform_set_drvdata(pdev, dsi);
>>>> +
>>>> + dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
>>>> + if (IS_ERR(dsi->dsi)) {
>>>> DRM_ERROR("Failed to initialize mipi dsi host\n");
>>>> clk_disable_unprepare(dsi->pllref_clk);
>>>> + return PTR_ERR(dsi->dsi);
>>>> }
>>>> - return ret;
>>>> + return 0;
>>>> }
>>>> 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);
>>>> - dw_mipi_dsi_remove(pdev);
>>>> + dw_mipi_dsi_remove(dsi->dsi);
>>>> return 0;
>>>> }
>>>> diff --git a/include/drm/bridge/dw_mipi_dsi.h
>>>> b/include/drm/bridge/dw_mipi_dsi.h
>>>> index 9b30fec302c8..d9c6d549f971 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__ */
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-01-09 13:45 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-28 1:05 [PATCH] drm/bridge/synopsis: stop clobbering drvdata Brian Norris
2017-11-28 1:05 ` Brian Norris
2017-11-28 2:25 ` Matthias Kaehlcke
2017-11-28 2:25 ` Matthias Kaehlcke
2017-11-28 6:27 ` Archit Taneja
2017-11-28 9:34 ` Philippe CORNU
2017-11-28 9:34 ` Philippe CORNU
2018-01-09 13:01 ` Philippe CORNU
2018-01-09 13:01 ` Philippe CORNU
2018-01-09 13:37 ` Laurent Pinchart
2018-01-09 13:37 ` Laurent Pinchart
2018-01-09 13:45 ` Andrzej Hajda
2018-01-09 13:45 ` Andrzej Hajda
2017-11-28 12:51 ` Laurent Pinchart
2017-11-28 18:21 ` Brian Norris
2017-11-28 18:21 ` Brian Norris
2018-01-09 13:36 ` Laurent Pinchart
2018-01-09 13:36 ` Laurent Pinchart
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.