lkml.org 
[lkml]   [2010]   [Nov]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/3] msm: iommu: Rework clock logic and add IOMMU bus clock control
On 11/22/2010 3:51 PM, Daniel Walker wrote:
> On Fri, 2010-11-19 at 19:02 -0800, Stepan Moskovchenko wrote:
>> Clean up the clock control code in the probe calls, and add
>> 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
> You need to remove this Change-Id ..
Fixed in v2.

>
>> + pr_err("Could not request memory region: start=%p, len=%d\n",
>> + (void *) r->start, len);(void *) r->start, len);
> You usually just tab over till you get to the " like this,
>
> pr_err("Could not request memory region: start=%p, len=%d\n",
> (void *) r->start, len);
Fixed in v2.

>> drv = platform_get_drvdata(pdev);
>> 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));
> Do you really need the memset ?
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.

>> + if (ret)
>> + goto fail;
>> +
>> + if (drvdata->clk) {
>> + ret = clk_enable(drvdata->clk);
>> + if (ret) {
>> + clk_disable(drvdata->pclk);
>> + goto fail;
>> + }
>> + }
> You did this in a prior patch also, you could combine them into a single
> helper function. Maybe do the same for the disable side too.
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.

I will send out v2 in a few minutes.

Steve

---

Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.




\
 
 \ /
  Last update: 2010-11-23 04:09    [W:8.881 / U:0.360 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site