Messages in this thread | | | Date | Fri, 25 Feb 2011 14:09:57 -0800 | From | Stepan Moskovchenko <> | Subject | Re: [PATCH 2/4] msm: iommu: Rework clock logic and add IOMMU bus clock control |
| |
On 2/24/2011 11:29 PM, Trilok Soni wrote: > Hi Steve,
Hello
>> @@ -130,117 +131,134 @@ static int msm_iommu_probe(struct platform_device *pdev) >> { >> struct resource *r, *r2; >> struct clk *iommu_clk; >> + struct clk *iommu_pclk; >> struct msm_iommu_drvdata *drvdata; >> struct msm_iommu_dev *iommu_dev = pdev->dev.platform_data; > const please. >
I am not clear on what you mean. Please be more specific here.
>> + iommu_pclk = clk_get(NULL, "smmu_pclk"); >> + if (IS_ERR(iommu_pclk)) { >> + ret = -ENODEV; >> + goto fail; >> + } > I am not a big fan of this when you have the "device" around. You should just do > > iommu_pclk = clk_get(&pdev->dev, NULL); > > ...error logic... > > iommu_clk = clk_get(&pdev->dev, "iommu_clk"); > > ...error logic... >
The pclk is a "special" bus clock and does not have a specific device instance associated with it, so passing a device would not be appropriate in this case. I pass the device for other clocks that are indeed associated with devices, but this is not one of them. I suppose we could create 11 or 12 aliases for the pclk and associate it with all the IOMMU devices, but I would prefer to avoid doing that as I believe the current approach is cleaner.
>> - ret = -EBUSY; >> - goto fail; >> - } >> + len = r->end - r->start + 1; > > resource_size please. >
Ok
| |