Hi Kevin,
On Sat, Jan 8, 2011 at 5:13 AM, Kevin Hilman <khilman@xxxxxx> wrote:
> Sumit Semwal <sumit.semwal@xxxxxx> writes:
>
>> From: Senthilvadivu Guruswamy <svadivu@xxxxxx>
>>
>> Hwmod adaptation design requires each of the DSS HW IP to be a platform driver.
>> So a platform_driver of DSS is created and init exit methods are moved from core.c
>> to its driver probe,remove. pdev member has to be maintained by its own drivers.
>>
>> DSS platform driver is registered from inside omap_dss_probe, in the order desired.
>>
>> Signed-off-by: Senthilvadivu Guruswamy <svadivu@xxxxxx>
>> Signed-off-by: Sumit Semwal <sumit.semwal@xxxxxx>
>> ---
>> drivers/video/omap2/dss/core.c | 21 +++++++--------
>> drivers/video/omap2/dss/dss.c | 55 ++++++++++++++++++++++++++++++++++++++-
>> drivers/video/omap2/dss/dss.h | 4 +-
>> 3 files changed, 65 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
>> index 48d20d8..faca859 100644
>> --- a/drivers/video/omap2/dss/core.c
>> +++ b/drivers/video/omap2/dss/core.c
>> @@ -497,9 +497,9 @@ static inline void dss_uninitialize_debugfs(void)
>> static int omap_dss_probe(struct platform_device *pdev)
>> {
>> struct omap_dss_board_info *pdata = pdev->dev.platform_data;
>> - int skip_init = 0;
>> int r;
>> int i;
>> + int skip_init = 0;
>
> looks like unnessary move.
>
> maybe you meant to remove this var since its usage was removed from this
> function?
>
>> core.pdev = pdev;
>>
>> @@ -517,15 +517,9 @@ static int omap_dss_probe(struct platform_device *pdev)
>> core.ctx_id = dss_get_ctx_id();
>> DSSDBG("initial ctx id %u\n", core.ctx_id);
>>
>> -#ifdef CONFIG_FB_OMAP_BOOTLOADER_INIT
>> - /* DISPC_CONTROL */
>> - if (omap_readl(0x48050440) & 1) /* LCD enabled? */
>> - skip_init = 1;
>> -#endif
>> -
>> - r = dss_init(skip_init);
>> + r = dss_init_platform_driver();
>> if (r) {
>> - DSSERR("Failed to initialize DSS\n");
>> + DSSERR("Failed to initialize DSS platform driver\n");
>> goto err_dss;
>> }
>>
>> @@ -553,6 +547,11 @@ static int omap_dss_probe(struct platform_device *pdev)
>> goto err_venc;
>> }
>>
>> +#ifdef CONFIG_FB_OMAP_BOOTLOADER_INIT
>> + /* DISPC_CONTROL */
>> + if (omap_readl(0x48050440) & 1) /* LCD enabled? */
>> + skip_init = 1;
>> +#endif
>
> nope, you just moved it here.
>
> But it's also duplicated in dsshw_probe below. Is that needed?
Yes; these two are needed as both dss_init() (which is replaced with
dss_init_platform_driver() in this patch) and also by sdi_init().
sdi_init() is a software init, so it will not be moved to a
platform_driver. That's why we need the #ifdef.. #endif in both
dss_probe() and dsshw_probe().
>
>> if (cpu_is_omap34xx()) {
>> r = sdi_init(skip_init);
>> if (r) {
>> @@ -610,7 +609,7 @@ err_dispc:
>> err_dpi:
>> rfbi_exit();
>> err_rfbi:
>> - dss_exit();
>> + dss_deinit_platform_driver();
>
> s/deinit/uninit/ or uninitial
Sure, will change.
>> err_dss:
>> dss_clk_disable_all_no_ctx();
>> dss_put_clocks();
>> @@ -636,7 +635,7 @@ static int omap_dss_remove(struct platform_device *pdev)
>> sdi_exit();
>> }
>>
>> - dss_exit();
>> + dss_deinit_platform_driver();
>>
>> /* these should be removed at some point */
>> c = core.dss_ick->usecount;
>> diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
>> index 77c3621..ebb294a 100644
>> --- a/drivers/video/omap2/dss/dss.c
>> +++ b/drivers/video/omap2/dss/dss.c
>> @@ -59,6 +59,7 @@ struct dss_reg {
>> dss_write_reg(idx, FLD_MOD(dss_read_reg(idx), val, start, end))
>>
>> static struct {
>> + struct platform_device *pdev;
>> void __iomem *base;
>>
>> struct clk *dpll4_m4_ck;
>> @@ -549,7 +550,7 @@ void dss_set_dac_pwrdn_bgz(bool enable)
>> REG_FLD_MOD(DSS_CONTROL, enable, 5, 5); /* DAC Power-Down Control */
>> }
>>
>> -int dss_init(bool skip_init)
>> +static int dss_init(bool skip_init)
>> {
>> int r;
>> u32 rev;
>> @@ -629,7 +630,7 @@ fail0:
>> return r;
>> }
>>
>> -void dss_exit(void)
>> +static void dss_exit(void)
>> {
>> if (cpu_is_omap34xx())
>> clk_put(dss.dpll4_m4_ck);
>> @@ -639,3 +640,53 @@ void dss_exit(void)
>> iounmap(dss.base);
>> }
>>
>> +/* DSS HW IP initialisation */
>> +static int omap_dsshw_probe(struct platform_device *pdev)
>> +{
>> + int r;
>> + int skip_init = 0;
>> +
>> + dss.pdev = pdev;
>> +
>> +#ifdef CONFIG_FB_OMAP_BOOTLOADER_INIT
>> + /* DISPC_CONTROL */
>> + if (omap_readl(0x48050440) & 1) /* LCD enabled? */
>> + skip_init = 1;
>> +#endif
>> +
>> + r = dss_init(skip_init);
>> + if (r) {
>> + DSSERR("Failed to initialize DSS\n");
>> + goto err_dss;
>> + }
>> +
>> +err_dss:
>> +
>> + return r;
>> +}
>> +
>> +static int omap_dsshw_remove(struct platform_device *pdev)
>> +{
>> + dss_exit();
>> +
>> + return 0;
>> +}
>> +
>> +static struct platform_driver omap_dsshw_driver = {
>> + .probe = omap_dsshw_probe,
>> + .remove = omap_dsshw_remove,
>> + .driver = {
>> + .name = "omap_dss",
>> + .owner = THIS_MODULE,
>> + },
>> +};
>> +
>> +int dss_init_platform_driver(void)
>> +{
>> + return platform_driver_register(&omap_dsshw_driver);
>> +}
>> +
>> +void dss_deinit_platform_driver(void)
>> +{
>> + return platform_driver_unregister(&omap_dsshw_driver);
>> +}
>
> Is there a reason for these extra functions?
>
> Why can't the platform_driver APIs be called directly?
Of all these platform devices (in this and later patches of this
series), the initialization needs to be in a sequence. That's achieved
by calling the init_platform_driver / uninit_platform_driver centrally
from core.c's omap_dss_probe() in the required sequence. Since
platform driver data for each platform device is static within each
platform driver's file, these functions are required.
Best regards,
~Sumit.
>
>> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
>> index 5c7940d..50b4223 100644
>> --- a/drivers/video/omap2/dss/dss.h
>> +++ b/drivers/video/omap2/dss/dss.h
>> @@ -214,8 +214,8 @@ void dss_overlay_setup_l4_manager(struct omap_overlay_manager *mgr);
>> void dss_recheck_connections(struct omap_dss_device *dssdev, bool force);
>>
>> /* DSS */
>> -int dss_init(bool skip_init);
>> -void dss_exit(void);
>> +int dss_init_platform_driver(void);
>> +void dss_deinit_platform_driver(void);
>>
>> void dss_save_context(void);
>> void dss_restore_context(void);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
