On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna <omar.ramirez@xxxxxx> wrote:
> @@ -123,11 +123,10 @@ static int iommu_enable(struct omap_iommu *obj)
> if (!arch_iommu)
> return -ENODEV;
>
> - clk_enable(obj->clk);
> + pm_runtime_get_sync(obj->dev);
>
> err = arch_iommu->enable(obj);
>
> - clk_disable(obj->clk);
> return err;
> }
Hmm, this is called on omap_iommu_attach, and iommu_disable is not
called until omap_iommu_detach, so this means the device will never
sleep. Why don't you call pm_runtime_put() instead of clk_disable()?
All the rest of the calls to pm_runtime_get/put after this are
basically no-ops, because the count will never go below 1.
You mention some irq issues, but could it be due to some bad clocks in
the hwmod data?
> @@ -136,11 +135,9 @@ static void iommu_disable(struct omap_iommu *obj)
> if (!obj)
> return;
>
> - clk_enable(obj->clk);
> -
> arch_iommu->disable(obj);
>
> - clk_disable(obj->clk);
> + pm_runtime_put(obj->dev);
> }
>
> /*
> @@ -263,7 +260,7 @@ static int load_iotlb_entry(struct omap_iommu *obj, struct iotlb_entry *e)
> if (!obj || !obj->nr_tlb_entries || !e)
> return -EINVAL;
>
> - clk_enable(obj->clk);
> + pm_runtime_get_sync(obj->dev);
>
> iotlb_lock_get(obj, &l);
> if (l.base == obj->nr_tlb_entries) {
> @@ -293,7 +290,7 @@ static int load_iotlb_entry(struct omap_iommu *obj, struct iotlb_entry *e)
>
> cr = iotlb_alloc_cr(obj, e);
> if (IS_ERR(cr)) {
> - clk_disable(obj->clk);
> + pm_runtime_put(obj->dev);
> return PTR_ERR(cr);
> }
>
> @@ -307,7 +304,7 @@ static int load_iotlb_entry(struct omap_iommu *obj, struct iotlb_entry *e)
> l.vict = l.base;
> iotlb_lock_set(obj, &l);
> out:
> - clk_disable(obj->clk);
> + pm_runtime_put(obj->dev);
> return err;
> }
>
> @@ -337,7 +334,7 @@ static void flush_iotlb_page(struct omap_iommu *obj, u32 da)
> int i;
> struct cr_regs cr;
>
> - clk_enable(obj->clk);
> + pm_runtime_get_sync(obj->dev);
>
> for_each_iotlb_cr(obj, obj->nr_tlb_entries, i, cr) {
> u32 start;
> @@ -356,7 +353,7 @@ static void flush_iotlb_page(struct omap_iommu *obj, u32 da)
> iommu_write_reg(obj, 1, MMU_FLUSH_ENTRY);
> }
> }
> - clk_disable(obj->clk);
> + pm_runtime_put(obj->dev);
>
> if (i == obj->nr_tlb_entries)
> dev_dbg(obj->dev, "%s: no page for %08x\n", __func__, da);
> @@ -370,7 +367,7 @@ static void flush_iotlb_all(struct omap_iommu *obj)
> {
> struct iotlb_lock l;
>
> - clk_enable(obj->clk);
> + pm_runtime_get_sync(obj->dev);
>
> l.base = 0;
> l.vict = 0;
> @@ -378,7 +375,7 @@ static void flush_iotlb_all(struct omap_iommu *obj)
>
> iommu_write_reg(obj, 1, MMU_GFLUSH);
>
> - clk_disable(obj->clk);
> + pm_runtime_put(obj->dev);
> }
>
> #if defined(CONFIG_OMAP_IOMMU_DEBUG) || defined(CONFIG_OMAP_IOMMU_DEBUG_MODULE)
> @@ -388,11 +385,11 @@ ssize_t omap_iommu_dump_ctx(struct omap_iommu *obj, char *buf, ssize_t bytes)
> if (!obj || !buf)
> return -EINVAL;
>
> - clk_enable(obj->clk);
> + pm_runtime_get_sync(obj->dev);
>
> bytes = arch_iommu->dump_ctx(obj, buf, bytes);
>
> - clk_disable(obj->clk);
> + pm_runtime_put(obj->dev);
>
> return bytes;
> }
> @@ -406,7 +403,7 @@ __dump_tlb_entries(struct omap_iommu *obj, struct cr_regs *crs, int num)
> struct cr_regs tmp;
> struct cr_regs *p = crs;
>
> - clk_enable(obj->clk);
> + pm_runtime_get_sync(obj->dev);
> iotlb_lock_get(obj, &saved);
>
> for_each_iotlb_cr(obj, num, i, tmp) {
> @@ -416,7 +413,7 @@ __dump_tlb_entries(struct omap_iommu *obj, struct cr_regs *crs, int num)
> }
>
> iotlb_lock_set(obj, &saved);
> - clk_disable(obj->clk);
> + pm_runtime_put(obj->dev);
>
> return p - crs;
> }
> @@ -780,9 +777,7 @@ static irqreturn_t iommu_fault_handler(int irq, void *data)
> if (!obj->refcount)
> return IRQ_NONE;
>
> - clk_enable(obj->clk);
> errs = iommu_report_fault(obj, &da);
> - clk_disable(obj->clk);
Why?
> if (errs == 0)
> return IRQ_HANDLED;
>
> @@ -920,10 +915,6 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
> if (!obj)
> return -ENOMEM;
>
> - obj->clk = clk_get(&pdev->dev, pdata->clk_name);
> - if (IS_ERR(obj->clk))
> - goto err_clk;
> -
> obj->nr_tlb_entries = pdata->nr_tlb_entries;
> obj->name = pdata->name;
> obj->dev = &pdev->dev;
> @@ -966,6 +957,8 @@ static int __devinit omap_iommu_probe(struct platform_device *pdev)
> goto err_irq;
> platform_set_drvdata(pdev, obj);
>
> + pm_runtime_enable(obj->dev);
> +
> dev_info(&pdev->dev, "%s registered\n", obj->name);
> return 0;
>
> @@ -974,8 +967,6 @@ err_irq:
> err_ioremap:
> release_mem_region(res->start, resource_size(res));
> err_mem:
> - clk_put(obj->clk);
> -err_clk:
> kfree(obj);
> return err;
> }
> @@ -996,7 +987,8 @@ static int __devexit omap_iommu_remove(struct platform_device *pdev)
> release_mem_region(res->start, resource_size(res));
> iounmap(obj->regbase);
>
> - clk_put(obj->clk);
> + pm_runtime_disable(obj->dev);
I'm not sure if this is needed; you want to resume the driver? AFAICS
kfree will take care of that _without_ resuming.
> dev_info(&pdev->dev, "%s removed\n", obj->name);
> kfree(obj);
> return 0;
Cheers.
--
Felipe Contreras
--
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
