From mboxrd@z Thu Jan 1 00:00:00 1970 From: Archit Taneja Date: Wed, 07 Mar 2012 18:23:49 +0000 Subject: Re: [PATCH 08/21] OMAPDSS: clean up the omapdss platform data mess Message-Id: <4F57A4E5.6070201@ti.com> List-Id: References: <1331124290-6285-1-git-send-email-tomi.valkeinen@ti.com> <1331124290-6285-9-git-send-email-tomi.valkeinen@ti.com> In-Reply-To: <1331124290-6285-9-git-send-email-tomi.valkeinen@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Tomi Valkeinen Cc: linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org, archit@ti.com On Wednesday 07 March 2012 06:14 PM, Tomi Valkeinen wrote: > The omapdss pdata handling is a mess. This is more evident when trying > to use device tree for DSS, as we don't have platform data anymore in > that case. This patch cleans the pdata handling by: > > - Remove struct omap_display_platform_data. It was used just as a > wrapper for struct omap_dss_board_info. > - Pass the platform data only to omapdss device. The drivers for omap > dss hwmods do not need the platform data. This should also work better > for DT, as we can create omapdss device programmatically in generic omap > boot code, and thus we can pass the pdata to it. > - Create dss functions for get_ctx_loss_count and dsi_enable/disable_pads > that the dss hwmod drivers can call. > > Signed-off-by: Tomi Valkeinen > --- > arch/arm/mach-omap2/display.c | 37 ++++++++++++++++++------------------- > drivers/video/omap2/dss/core.c | 35 +++++++++++++++++++++++++++++++++++ > drivers/video/omap2/dss/dispc.c | 21 ++------------------- > drivers/video/omap2/dss/dsi.c | 17 +++-------------- > drivers/video/omap2/dss/dss.h | 3 +++ > drivers/video/omap2/dss/hdmi.c | 2 -- > include/video/omapdss.h | 5 ----- > 7 files changed, 61 insertions(+), 59 deletions(-) > > diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c > index 3677b1f..279c124 100644 > --- a/arch/arm/mach-omap2/display.c > +++ b/arch/arm/mach-omap2/display.c > @@ -185,10 +185,23 @@ int __init omap_display_init(struct omap_dss_board_info *board_data) > struct omap_hwmod *oh; > struct platform_device *pdev; > int i, oh_count; > - struct omap_display_platform_data pdata; > const struct omap_dss_hwmod_data *curr_dss_hwmod; > > - memset(&pdata, 0, sizeof(pdata)); > + /* create omapdss device */ > + > + board_data->dsi_enable_pads = omap_dsi_enable_pads; > + board_data->dsi_disable_pads = omap_dsi_disable_pads; > + board_data->get_context_loss_count = omap_pm_get_dev_context_loss_count; Why are the checks for board data being NULL removed here? > + > + omap_display_device.dev.platform_data = board_data; > + > + r = platform_device_register(&omap_display_device); > + if (r< 0) { > + pr_err("Unable to register omapdss device\n"); > + return r; > + } > + > + /* create devices for dss hwmods */ > > if (cpu_is_omap24xx()) { > curr_dss_hwmod = omap2_dss_hwmod_data; > @@ -201,15 +214,6 @@ int __init omap_display_init(struct omap_dss_board_info *board_data) > oh_count = ARRAY_SIZE(omap4_dss_hwmod_data); > } > > - if (board_data->dsi_enable_pads = NULL) > - board_data->dsi_enable_pads = omap_dsi_enable_pads; > - if (board_data->dsi_disable_pads = NULL) > - board_data->dsi_disable_pads = omap_dsi_disable_pads; > - > - pdata.board_data = board_data; > - pdata.board_data->get_context_loss_count > - omap_pm_get_dev_context_loss_count; > - > for (i = 0; i< oh_count; i++) { > oh = omap_hwmod_lookup(curr_dss_hwmod[i].oh_name); > if (!oh) { > @@ -219,21 +223,16 @@ int __init omap_display_init(struct omap_dss_board_info *board_data) > } > > pdev = omap_device_build(curr_dss_hwmod[i].dev_name, > - curr_dss_hwmod[i].id, oh,&pdata, > - sizeof(struct omap_display_platform_data), > + curr_dss_hwmod[i].id, oh, > + NULL, 0, > NULL, 0, 0); > > if (WARN((IS_ERR(pdev)), "Could not build omap_device for %s\n", > curr_dss_hwmod[i].oh_name)) > return -ENODEV; > } > - omap_display_device.dev.platform_data = board_data; > > - r = platform_device_register(&omap_display_device); > - if (r< 0) > - printk(KERN_ERR "Unable to register OMAP-Display device\n"); > - > - return r; > + return 0; > } > > static void dispc_disable_outputs(void) > diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c > index 8613f86..3efd473 100644 > --- a/drivers/video/omap2/dss/core.c > +++ b/drivers/video/omap2/dss/core.c > @@ -87,6 +87,41 @@ struct regulator *dss_get_vdds_sdi(void) > return reg; > } > > +int dss_get_ctx_loss_count(struct device *dev) > +{ > + struct omap_dss_board_info *board_data = core.pdev->dev.platform_data; > + int cnt; > + > + if (!board_data || !board_data->get_context_loss_count) > + return -ENOENT; why do we check board_data being NULL here and not in omap_display_init()? Archit > + > + cnt = board_data->get_context_loss_count(dev); > + > + WARN_ONCE(cnt< 0, "get_context_loss_count failed: %d\n", cnt); > + > + return cnt; > +} > + > +int dss_dsi_enable_pads(int dsi_id, unsigned lane_mask) > +{ > + struct omap_dss_board_info *board_data = core.pdev->dev.platform_data; > + > + if (!board_data || !board_data->dsi_enable_pads) > + return -ENOENT; > + > + return board_data->dsi_enable_pads(dsi_id, lane_mask); > +} > + > +void dss_dsi_disable_pads(int dsi_id, unsigned lane_mask) > +{ > + struct omap_dss_board_info *board_data = core.pdev->dev.platform_data; > + > + if (!board_data || !board_data->dsi_enable_pads) > + return; > + > + return board_data->dsi_disable_pads(dsi_id, lane_mask); > +} > + > #if defined(CONFIG_DEBUG_FS)&& defined(CONFIG_OMAP2_DSS_DEBUG_SUPPORT) > static int dss_debug_show(struct seq_file *s, void *unused) > { > diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c > index bddd64b..703bb20 100644 > --- a/drivers/video/omap2/dss/dispc.c > +++ b/drivers/video/omap2/dss/dispc.c > @@ -131,23 +131,6 @@ static inline u32 dispc_read_reg(const u16 idx) > return __raw_readl(dispc.base + idx); > } > > -static int dispc_get_ctx_loss_count(void) > -{ > - struct device *dev =&dispc.pdev->dev; > - struct omap_display_platform_data *pdata = dev->platform_data; > - struct omap_dss_board_info *board_data = pdata->board_data; > - int cnt; > - > - if (!board_data->get_context_loss_count) > - return -ENOENT; > - > - cnt = board_data->get_context_loss_count(dev); > - > - WARN_ONCE(cnt< 0, "get_context_loss_count failed: %d\n", cnt); > - > - return cnt; > -} > - > #define SR(reg) \ > dispc.ctx[DISPC_##reg / sizeof(u32)] = dispc_read_reg(DISPC_##reg) > #define RR(reg) \ > @@ -251,7 +234,7 @@ static void dispc_save_context(void) > if (dss_has_feature(FEAT_CORE_CLK_DIV)) > SR(DIVISOR); > > - dispc.ctx_loss_cnt = dispc_get_ctx_loss_count(); > + dispc.ctx_loss_cnt = dss_get_ctx_loss_count(&dispc.pdev->dev); > dispc.ctx_valid = true; > > DSSDBG("context saved, ctx_loss_count %d\n", dispc.ctx_loss_cnt); > @@ -266,7 +249,7 @@ static void dispc_restore_context(void) > if (!dispc.ctx_valid) > return; > > - ctx = dispc_get_ctx_loss_count(); > + ctx = dss_get_ctx_loss_count(&dispc.pdev->dev); > > if (ctx>= 0&& ctx = dispc.ctx_loss_cnt) > return; > diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c > index 3e656be..4e2f7ff 100644 > --- a/drivers/video/omap2/dss/dsi.c > +++ b/drivers/video/omap2/dss/dsi.c > @@ -261,9 +261,6 @@ struct dsi_data { > struct clk *dss_clk; > struct clk *sys_clk; > > - int (*enable_pads)(int dsi_id, unsigned lane_mask); > - void (*disable_pads)(int dsi_id, unsigned lane_mask); > - > struct dsi_clock_info current_cinfo; > > bool vdds_dsi_enabled; > @@ -2396,7 +2393,7 @@ static int dsi_cio_init(struct omap_dss_device *dssdev) > > DSSDBGF(); > > - r = dsi->enable_pads(dsi_get_dsidev_id(dsidev), dsi_get_lane_mask(dssdev)); > + r = dss_dsi_enable_pads(dsi_get_dsidev_id(dsidev), dsi_get_lane_mask(dssdev)); > if (r) > return r; > > @@ -2506,21 +2503,20 @@ err_cio_pwr: > dsi_cio_disable_lane_override(dsidev); > err_scp_clk_dom: > dsi_disable_scp_clk(dsidev); > - dsi->disable_pads(dsi_get_dsidev_id(dsidev), dsi_get_lane_mask(dssdev)); > + dss_dsi_disable_pads(dsi_get_dsidev_id(dsidev), dsi_get_lane_mask(dssdev)); > return r; > } > > static void dsi_cio_uninit(struct omap_dss_device *dssdev) > { > struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev); > - struct dsi_data *dsi = dsi_get_dsidrv_data(dsidev); > > /* DDR_CLK_ALWAYS_ON */ > REG_FLD_MOD(dsidev, DSI_CLK_CTRL, 0, 13, 13); > > dsi_cio_power(dsidev, DSI_COMPLEXIO_POWER_OFF); > dsi_disable_scp_clk(dsidev); > - dsi->disable_pads(dsi_get_dsidev_id(dsidev), dsi_get_lane_mask(dssdev)); > + dss_dsi_disable_pads(dsi_get_dsidev_id(dsidev), dsi_get_lane_mask(dssdev)); > } > > static void dsi_config_tx_fifo(struct platform_device *dsidev, > @@ -4680,8 +4676,6 @@ static void dsi_put_clocks(struct platform_device *dsidev) > /* DSI1 HW IP initialisation */ > static int omap_dsihw_probe(struct platform_device *dsidev) > { > - struct omap_display_platform_data *dss_plat_data; > - struct omap_dss_board_info *board_info; > u32 rev; > int r, i, dsi_module = dsi_get_dsidev_id(dsidev); > struct resource *dsi_mem; > @@ -4695,11 +4689,6 @@ static int omap_dsihw_probe(struct platform_device *dsidev) > dsi_pdev_map[dsi_module] = dsidev; > dev_set_drvdata(&dsidev->dev, dsi); > > - dss_plat_data = dsidev->dev.platform_data; > - board_info = dss_plat_data->board_data; > - dsi->enable_pads = board_info->dsi_enable_pads; > - dsi->disable_pads = board_info->dsi_disable_pads; > - > spin_lock_init(&dsi->irq_lock); > spin_lock_init(&dsi->errors_lock); > dsi->errors = 0; > diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h > index d4b3dff..d37ed80 100644 > --- a/drivers/video/omap2/dss/dss.h > +++ b/drivers/video/omap2/dss/dss.h > @@ -162,6 +162,9 @@ struct platform_device; > struct bus_type *dss_get_bus(void); > struct regulator *dss_get_vdds_dsi(void); > struct regulator *dss_get_vdds_sdi(void); > +int dss_get_ctx_loss_count(struct device *dev); > +int dss_dsi_enable_pads(int dsi_id, unsigned lane_mask); > +void dss_dsi_disable_pads(int dsi_id, unsigned lane_mask); > > /* apply */ > void dss_apply_init(void); > diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c > index c4b4f69..cacf856 100644 > --- a/drivers/video/omap2/dss/hdmi.c > +++ b/drivers/video/omap2/dss/hdmi.c > @@ -63,7 +63,6 @@ > > static struct { > struct mutex lock; > - struct omap_display_platform_data *pdata; > struct platform_device *pdev; > struct hdmi_ip_data ip_data; > > @@ -796,7 +795,6 @@ static int omapdss_hdmihw_probe(struct platform_device *pdev) > struct resource *hdmi_mem; > int r; > > - hdmi.pdata = pdev->dev.platform_data; > hdmi.pdev = pdev; > > mutex_init(&hdmi.lock); > diff --git a/include/video/omapdss.h b/include/video/omapdss.h > index 483f67c..b499ccb 100644 > --- a/include/video/omapdss.h > +++ b/include/video/omapdss.h > @@ -316,11 +316,6 @@ extern int omap_display_init(struct omap_dss_board_info *board_data); > /* HDMI mux init*/ > extern int omap_hdmi_init(enum omap_hdmi_flags flags); > > -struct omap_display_platform_data { > - struct omap_dss_board_info *board_data; > - /* TODO: Additional members to be added when PM is considered */ > -}; > - > struct omap_video_timings { > /* Unit: pixels */ > u16 x_res;