Re: [PATCH V4 3/6] OMAPDSS: DSS: Cleanup cpu_is_xxxx checks
From: Mahapatra, Chandrabhanu <hidden>
Date: 2012-08-20 10:48:44
Also in:
linux-fbdev
On Mon, Aug 20, 2012 at 2:12 PM, Tomi Valkeinen [off-list ref] wrote:
On Fri, 2012-08-17 at 16:54 +0300, Tomi Valkeinen wrote:quoted
On Thu, 2012-08-16 at 16:48 +0530, Chandrabhanu Mahapatra wrote:quoted
All the cpu_is checks have been moved to dss_init_features function providing a much more generic and cleaner interface. The OMAP version and revision specific initializations in various functions are cleaned and the necessary data are moved to dss_features structure which is local to dss.c. Signed-off-by: Chandrabhanu Mahapatra <redacted>quoted
+static int __init dss_init_features(struct device *dev) +{ + dss.feat = devm_kzalloc(dev, sizeof(*dss.feat), GFP_KERNEL); + if (!dss.feat) { + dev_err(dev, "Failed to allocate local DSS Features\n"); + return -ENOMEM; + } + + if (cpu_is_omap24xx()) + dss.feat = &omap24xx_dss_features; + else if (cpu_is_omap34xx()) + dss.feat = &omap34xx_dss_features; + else if (cpu_is_omap3630()) + dss.feat = &omap3630_dss_features; + else if (cpu_is_omap44xx()) + dss.feat = &omap44xx_dss_features; + else + return -ENODEV; + + return 0; +}This is not correct (and same problem in dispc). You allocate the feat struct and assign the pointer to dss.feat, but then overwrite dss.feat pointer with the pointer to omap24xx_dss_features (which is freed later). You need to memcpy it.
Ok.
quoted
I also get a crash on omap3 overo board when loading omapdss:
During recent tests I never saw the crash happen. The kernel used to freeze during booting after printing "Uncompressing Linux... done, booting the kernel."
The crash happens because dss_get_clocks uses the feat stuff, but the dss_init_features is only called later. Did you test this? I can't see how that can work on any board.
Thanks for pointing that out. Correcting above fixed the problem.
Also, in the current place you have dss_init_features call, in case there's an error you can't just return, you need to release the resources. The same problem is in the dispc.c. Tomi
By releasing resources did you mean the dss.feat memory? I think dss_init_features call is wrongly placed and should be placed at the very beginning of the probe function. -- Chandrabhanu Mahapatra Texas Instruments India Pvt. Ltd.