<br>On Tue, Nov 20, 2012 at 5:03 PM, Prathyush K <<a href="mailto:prathyush@chromium.org">prathyush@chromium.org</a>> wrote:<br>> On Tue, Nov 20, 2012 at 1:00 PM, Cho KyongHo <<a href="mailto:pullip.cho@samsung.com">pullip.cho@samsung.com</a>> wrote:<br>
>><br>>> This change enables the client device drivers not to care about<br>>> the state of System MMU since the internal state of System MMU<br>>> is controlled by the runtime PM and suspend/resume callback functions.<br>
>><br>>> Signed-off-by: KyongHo Cho <<a href="mailto:pullip.cho@samsung.com">pullip.cho@samsung.com</a>><br>>> ---<br>>> drivers/iommu/exynos-iommu.c | 175<br>>> ++++++++++++++++++++++---------------------<br>
>> 1 file changed, 89 insertions(+), 86 deletions(-)<br>>><br>>> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c<br>>> index 8f9239c..169f56e 100644<br>>> --- a/drivers/iommu/exynos-iommu.c<br>
>> +++ b/drivers/iommu/exynos-iommu.c<br>>> @@ -208,6 +208,7 @@ struct sysmmu_drvdata {<br>>> struct iommu_domain *domain;<br>>> sysmmu_fault_handler_t fault_handler;<br>>> unsigned long pgtable;<br>
>> + bool runtime_active;<br>>> void __iomem *sfrbases[0];<br>>> };<br>>><br>>> @@ -477,7 +478,8 @@ static bool __sysmmu_disable(struct sysmmu_drvdata<br>>> *drvdata)<br>
>> drvdata->pgtable = 0;<br>>> drvdata->domain = NULL;<br>>><br>>> - __sysmmu_disable_nocount(drvdata);<br>>> + if (drvdata->runtime_active)<br>
>> + __sysmmu_disable_nocount(drvdata);<br>>><br>>> dev_dbg(drvdata->sysmmu, "Disabled\n");<br>>> } else {<br>>> @@ -490,30 +492,6 @@ static bool __sysmmu_disable(struct sysmmu_drvdata<br>
>> *drvdata)<br>>> return disabled;<br>>> }<br>>><br>>> -static bool __exynos_sysmmu_disable(struct device *dev)<br>>> -{<br>>> - unsigned long flags;<br>>> - bool disabled = true;<br>
>> - struct exynos_iommu_owner *owner = dev->archdata.iommu;<br>>> - struct device *sysmmu;<br>>> -<br>>> - BUG_ON(!has_sysmmu(dev));<br>>> -<br>>> - spin_lock_irqsave(&owner->lock, flags);<br>
>> -<br>>> - /* Every call to __sysmmu_disable() must return same result */<br>>> - for_each_sysmmu(dev, sysmmu) {<br>>> - struct sysmmu_drvdata *drvdata = dev_get_drvdata(sysmmu);<br>
>> - disabled = __sysmmu_disable(drvdata);<br>>> - if (disabled)<br>>> - drvdata->master = NULL;<br>>> - }<br>>> -<br>>> - spin_unlock_irqrestore(&owner->lock, flags);<br>
>> -<br>>> - return disabled;<br>>> -}<br>>> -<br>>> static void __sysmmu_enable_nocount(struct sysmmu_drvdata *drvdata)<br>>> {<br>>> int i;<br>>> @@ -554,7 +532,8 @@ static int __sysmmu_enable(struct sysmmu_drvdata<br>
>> *drvdata,<br>>> drvdata->pgtable = pgtable;<br>>> drvdata->domain = domain;<br>>><br>>> - __sysmmu_enable_nocount(drvdata);<br>>> + if (drvdata->runtime_active)<br>
>> + __sysmmu_enable_nocount(drvdata);<br>>><br>>> dev_dbg(drvdata->sysmmu, "Enabled\n");<br>>> } else {<br>>> @@ -610,42 +589,31 @@ static int __exynos_sysmmu_enable(struct device<br>
>> *dev, unsigned long pgtable,<br>>><br>>> int exynos_sysmmu_enable(struct device *dev, unsigned long pgtable)<br>>> {<br>>> - int ret;<br>>> - struct device *sysmmu;<br>
>> -<br>>> BUG_ON(!memblock_is_memory(pgtable));<br>>><br>>> - for_each_sysmmu(dev, sysmmu) {<br>>> - ret = pm_runtime_get_sync(sysmmu);<br>>> - if (ret < 0)<br>
>> - break;<br>>> - }<br>>> -<br>>> - if (ret < 0) {<br>>> - struct device *start;<br>>> - for_each_sysmmu_until(dev, start, sysmmu)<br>
>> - pm_runtime_put(start);<br>>> -<br>>> - return ret;<br>>> - }<br>>> -<br>>> - ret = __exynos_sysmmu_enable(dev, pgtable, NULL);<br>>> - if (ret < 0)<br>
>> - for_each_sysmmu(dev, sysmmu)<br>>> - pm_runtime_put(sysmmu);<br>>> -<br>>> - return ret;<br>>> + return __exynos_sysmmu_enable(dev, pgtable, NULL);<br>
>> }<br>>><br>>> bool exynos_sysmmu_disable(struct device *dev)<br>>> {<br>>> - bool disabled;<br>>> + unsigned long flags;<br>>> + bool disabled = true;<br>>> + struct exynos_iommu_owner *owner = dev->archdata.iommu;<br>
>> struct device *sysmmu;<br>>><br>>> - disabled = __exynos_sysmmu_disable(dev);<br>>> + BUG_ON(!has_sysmmu(dev));<br>>><br>>> - for_each_sysmmu(dev, sysmmu)<br>
>> - pm_runtime_put(sysmmu);<br>>> + spin_lock_irqsave(&owner->lock, flags);<br>>> +<br>>> + /* Every call to __sysmmu_disable() must return same result */<br>>> + for_each_sysmmu(dev, sysmmu) {<br>
>> + struct sysmmu_drvdata *drvdata = dev_get_drvdata(sysmmu);<br>>> + disabled = __sysmmu_disable(drvdata);<br>>> + if (disabled)<br>>> + drvdata->master = NULL;<br>
>> + }<br>>> +<br>>> + spin_unlock_irqrestore(&owner->lock, flags);<br>>><br>>> return disabled;<br>>> }<br>>> @@ -661,7 +629,8 @@ static void sysmmu_tlb_invalidate_entry(struct device<br>
>> *dev, unsigned long iova)<br>>> data = dev_get_drvdata(sysmmu);<br>>><br>>> spin_lock_irqsave(&data->lock, flags);<br>>> - if (is_sysmmu_active(data)) {<br>
>> + if (is_sysmmu_active(data) &&<br>>> + data->runtime_active) {<br>>> int i;<br>>> for (i = 0; i < data->nsfrs; i++) {<br>
>> if (sysmmu_block(data->sfrbases[i])) {<br>>> @@ -899,6 +868,7 @@ static int __init exynos_sysmmu_probe(struct<br>>> platform_device *pdev)<br>>><br>>> ret = __sysmmu_setup(dev, data);<br>
>> if (!ret) {<br>>> + data->runtime_active = !pm_runtime_enabled(dev);<br>>> data->sysmmu = dev;<br>>> spin_lock_init(&data->lock);<br>
>><br>>> @@ -911,6 +881,64 @@ static int __init exynos_sysmmu_probe(struct<br>>> platform_device *pdev)<br>>> return ret;<br>>> }<br>>><br>>> +#ifdef CONFIG_PM_SLEEP<br>>> +static int sysmmu_suspend(struct device *dev)<br>
>> +{<br>>> + struct sysmmu_drvdata *drvdata = dev_get_drvdata(dev);<br>>> + unsigned long flags;<br>>> + spin_lock_irqsave(&drvdata->lock, flags);<br>>> + if (is_sysmmu_active(drvdata) &&<br>
>> + (!pm_runtime_enabled(dev) || drvdata->runtime_active))<br>>> + __sysmmu_disable_nocount(drvdata);<br>>> + spin_unlock_irqrestore(&drvdata->lock, flags);<br>
>> + return 0;<br>>> +}<br>>> +<br>>> +static int sysmmu_resume(struct device *dev)<br>>> +{<br>>> + struct sysmmu_drvdata *drvdata = dev_get_drvdata(dev);<br>>> + unsigned long flags;<br>
>> + spin_lock_irqsave(&drvdata->lock, flags);<br>>> + if (is_sysmmu_active(drvdata) &&<br>>> + (!pm_runtime_enabled(dev) || drvdata->runtime_active)) {<br>>> + __sysmmu_enable_nocount(drvdata);<br>
>> + }<br>>> + spin_unlock_irqrestore(&drvdata->lock, flags);<br>>> + return 0;<br>>> +}<br>>> +#endif<br>>> +<br>>> +#ifdef CONFIG_PM_RUNTIME<br>>> +static int sysmmu_runtime_suspend(struct device *dev)<br>
>> +{<br>>> + struct sysmmu_drvdata *drvdata = dev_get_drvdata(dev);<br>>> + unsigned long flags;<br>>> + spin_lock_irqsave(&drvdata->lock, flags);<br>>> + if (is_sysmmu_active(drvdata))<br>
>> + __sysmmu_disable_nocount(drvdata);<br>>> + drvdata->runtime_active = false;<br>>> + spin_unlock_irqrestore(&drvdata->lock, flags);<br>>> + return 0;<br>
>> +}<br>>> +<br>>> +static int sysmmu_runtime_resume(struct device *dev)<br>>> +{<br>>> + struct sysmmu_drvdata *drvdata = dev_get_drvdata(dev);<br>>> + unsigned long flags;<br>
>> + spin_lock_irqsave(&drvdata->lock, flags);<br>>> + drvdata->runtime_active = true;<br>>> + if (is_sysmmu_active(drvdata))<br>>> + __sysmmu_enable_nocount(drvdata);<br>
>> + spin_unlock_irqrestore(&drvdata->lock, flags);<br>>> + return 0;<br>>> +}<br>>> +#endif<br>>> +<br>>> +static const struct dev_pm_ops __pm_ops = {<br>>> + SET_SYSTEM_SLEEP_PM_OPS(sysmmu_suspend, sysmmu_resume)<br>
>> + SET_RUNTIME_PM_OPS(sysmmu_runtime_resume, sysmmu_runtime_suspend,<br>>> NULL)<br>><br>><br>> The runtime PM ops are not set correctly. The suspend_fn should be first,<br>> followed<br>> by resume_fn as per SET_RUNTIME_PM_OPS.<br>
><br>> Thanks,<br>> Prathyush<br>><br><br>Thank you.<br>I missed that. It will be fixed in the next version of this patch.<br><br>Cho KyongHo.<br>