Hi Adrian, Thanks for the patch. This is nice consolidation work. I'm Ccing Heiko for the Rockchip part.
See below for some comments. On Mon, 2019-12-02 at 21:33 +0200, AdrianAdrian Ratiu wrote: > Convert the common bridge code and the two rockchip & stm drivers > which currently use it to the regmap API in anticipation for further > changes to make it more generic and add older DSI host controller > support as found on i.mx6 based devices. > > The regmap becomes an internal state of the bridge. No functional > changes other than requiring the platform drivers to use the > pre-configured regmap supplied by the bridge after its probe() call > instead of ioremp'ing the registers themselves. > > In subsequent commits the bridge will become able to detect the > DSI host core version and init the regmap with different register > layouts. The platform drivers will continue to use the regmap without > modifications or worrying about the specific layout in use (in other > words the layout is abstracted away via the regmap). > > Suggested-by: Boris Brezillon <boris.brezil...@collabora.com> > Reviewed-by: Neil Armstrong <narmstr...@baylibre.com> > Reviewed-by: Emil Velikov <emil.veli...@collabora.com> > Signed-off-by: Adrian Ratiu <adrian.ra...@collabora.com> > --- > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 215 ++++++++++-------- > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 17 +- At least for Rockchip, I'd rather see this done in two steps: first some regmap infrastructure introduced, and then in a follow-up patch, the rockchip driver moved to it. It's safer, and better from a bisection POV, and from a first look it seems doable. > drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 34 ++- It would be good to do try the same for STM. It's also simpler to review that way. > include/drm/bridge/dw_mipi_dsi.h | 2 +- > 4 files changed, 145 insertions(+), 123 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > index b6e793bb653c..6cb57807f3f9 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > @@ -15,6 +15,7 @@ > #include <linux/module.h> > #include <linux/of_device.h> > #include <linux/pm_runtime.h> > +#include <linux/regmap.h> > #include <linux/reset.h> > > #include <video/mipi_display.h> > @@ -226,7 +227,7 @@ struct dw_mipi_dsi { > struct mipi_dsi_host dsi_host; > struct drm_bridge *panel_bridge; > struct device *dev; > - void __iomem *base; > + struct regmap *regs; > You have the regmap here... > > struct clk *pclk; > [..] > @@ -954,7 +952,6 @@ static int dw_mipi_dsi_rockchip_probe(struct > platform_device *pdev) > } > > dsi->dev = dev; > - dsi->pdata.base = dsi->base; > dsi->pdata.max_data_lanes = dsi->cdata->max_data_lanes; > dsi->pdata.phy_ops = &dw_mipi_dsi_rockchip_phy_ops; > dsi->pdata.host_ops = &dw_mipi_dsi_rockchip_host_ops; > @@ -970,6 +967,8 @@ static int dw_mipi_dsi_rockchip_probe(struct > platform_device *pdev) > goto err_clkdisable; > } > > + dsi->regs = dsi->pdata.regs; > + ... and this goes for both STM and Rockchip: I don't think you need neither the struct dw_mipi_dsi_plat_data.regs nor the structdw_mipi_dsi_{rockchip, stm}.regs. You should be able to just access the regmap via the struct dw_mipi_dsi. [..] > > err_dsi_probe: > @@ -474,7 +472,7 @@ static struct platform_driver dw_mipi_dsi_stm_driver = { > .remove = dw_mipi_dsi_stm_remove, > .driver = { > .of_match_table = dw_mipi_dsi_stm_dt_ids, > - .name = "stm32-display-dsi", > + .name = DRIVER_NAME, Unrelated change, please drop it. > .pm = &dw_mipi_dsi_stm_pm_ops, > }, > }; Thanks, Ezequiel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel