On Fri, 2010-11-19 at 19:02 -0800, Stepan Moskovchenko wrote:Fixed in v2.Clean up the clock control code in the probe calls, and addYou need to remove this Change-Id ..
support for controlling the clock for the IOMMU bus
interconnect. With the (proper) clock driver in place, the
clock control logic in the probe function can be made much
cleaner since it does not have to deal with the placeholder
driver anymore.
Change-Id: I1040bc4e18f4ab4b7cc0dd5fe667f9df83b9f1f5
Fixed in v2.
+ pr_err("Could not request memory region: start=%p, len=%d\n",You usually just tab over till you get to the " like this,
+ (void *) r->start, len);(void *) r->start, len);
pr_err("Could not request memory region: start=%p, len=%d\n",
(void *) r->start, len);
I guess not.. it seemed like good practice to poison the memory in case someone else had a stale reference to it. I guess I can remove them.drv = platform_get_drvdata(pdev);Do you really need the memset ?
if (drv) {
- memset(drv, 0, sizeof(struct msm_iommu_drvdata));
+ if (drv->clk)
+ clk_put(drv->clk);
+ clk_put(drv->pclk);
+ memset(drv, 0, sizeof(*drv));
That was in a different file (where it gets used much more extensively than just in the one case here). I would rather not make a bunch of my internal stuff visible to the global namespace if I can help it, and it's a trivial enough operation to enable two clocks.+ if (ret)You did this in a prior patch also, you could combine them into a single
+ goto fail;
+
+ if (drvdata->clk) {
+ ret = clk_enable(drvdata->clk);
+ if (ret) {
+ clk_disable(drvdata->pclk);
+ goto fail;
+ }
+ }
helper function. Maybe do the same for the disable side too.