Re: [PATCH 08/21] OMAPDSS: clean up the omapdss platform data mess

Archit Taneja
2012-03-08T08:18:03Z

On Thursday 08 March 2012 01:32 PM, Tomi Valkeinen wrote:
> On Wed, 2012-03-07 at 23:41 +0530, Archit Taneja wrote:
>> 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<tomi.valkeinen@ti.com>
>>> ---
>>> 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?
>
> We didn't check if board_data is NULL in the earlier version either. And
> I don't think there's need to check that, because if the board file
> calls this function, it should also give the board data.
>
> However, the earlier version didn't set the func pointers if the func
> pointer in the board_data was != NULL. Did you mean that? I removed that
> check, as I don't see a need for it. The func pointers should be set by
> this function, and I don't see why the board file would need to use its
> own versions.

Yes, I had meant the function pointers being != NULL, yes it doesn't
make sense for the board file to populate these.

>
>>> 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()?
>
> I added it for DT case, because then we don't have board_data for the
> devices defined in the DT data. However, for now we always have the
> board_data, and in this patch I should just move the code. So I'll
> remove the check, and add it later with DT code if needed.

Ok. When DT will be in use, would omap_display_init() be called or not?

>
> (and actually, I don' think the check is needed in DT case either...)

I don't know how things would work in DT case. So I'm not sure what's
happening here.

Archit

>
> Tomi
>
>
>
>