* [PATCH v2 1/2] dmaengine: tegra210-adma: use devm_clk_*() helpers
@ 2019-03-13 5:43 Sameer Pujar
2019-03-13 5:43 ` [PATCH v2 2/2] dmaengine: tegra210-adma: update system sleep callbacks Sameer Pujar
0 siblings, 1 reply; 6+ messages in thread
From: Sameer Pujar @ 2019-03-13 5:43 UTC (permalink / raw)
To: dan.j.williams, vkoul
Cc: treding, jonathanh, dmaengine, linux-tegra, linux-kernel, Sameer Pujar
adma driver is using pm_clk_*() interface for managing clock resources.
With this it is observed that clocks remain ON always. This happens on
Tegra devices which use BPMP co-processor to manage clock resources,
where clocks are enabled during prepare phase. This is necessary because
clocks to BPMP are always blocking. When pm_clk_*() interface is used on
such Tegra devices, clock prepare count is not balanced till remove call
happens for the driver and hence clocks are seen ON always. Thus this
patch replaces pm_clk_*() with devm_clk_*() framework.
Suggested-by: Mohan Kumar D <mkumard@nvidia.com>
Reviewed-by: Jonathan Hunter <jonathanh@nvidia.com>
Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
drivers/dma/tegra210-adma.c | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
index 5ec0dd9..650cd9c 100644
--- a/drivers/dma/tegra210-adma.c
+++ b/drivers/dma/tegra210-adma.c
@@ -22,7 +22,6 @@
#include <linux/of_device.h>
#include <linux/of_dma.h>
#include <linux/of_irq.h>
-#include <linux/pm_clock.h>
#include <linux/pm_runtime.h>
#include <linux/slab.h>
@@ -141,6 +140,7 @@ struct tegra_adma {
struct dma_device dma_dev;
struct device *dev;
void __iomem *base_addr;
+ struct clk *ahub_clk;
unsigned int nr_channels;
unsigned long rx_requests_reserved;
unsigned long tx_requests_reserved;
@@ -637,8 +637,9 @@ static int tegra_adma_runtime_suspend(struct device *dev)
struct tegra_adma *tdma = dev_get_drvdata(dev);
tdma->global_cmd = tdma_read(tdma, ADMA_GLOBAL_CMD);
+ clk_disable_unprepare(tdma->ahub_clk);
- return pm_clk_suspend(dev);
+ return 0;
}
static int tegra_adma_runtime_resume(struct device *dev)
@@ -646,10 +647,11 @@ static int tegra_adma_runtime_resume(struct device *dev)
struct tegra_adma *tdma = dev_get_drvdata(dev);
int ret;
- ret = pm_clk_resume(dev);
- if (ret)
+ ret = clk_prepare_enable(tdma->ahub_clk);
+ if (ret) {
+ dev_err(dev, "ahub clk_enable failed: %d\n", ret);
return ret;
-
+ }
tdma_write(tdma, ADMA_GLOBAL_CMD, tdma->global_cmd);
return 0;
@@ -693,13 +695,11 @@ static int tegra_adma_probe(struct platform_device *pdev)
if (IS_ERR(tdma->base_addr))
return PTR_ERR(tdma->base_addr);
- ret = pm_clk_create(&pdev->dev);
- if (ret)
- return ret;
-
- ret = of_pm_clk_add_clk(&pdev->dev, "d_audio");
- if (ret)
- goto clk_destroy;
+ tdma->ahub_clk = devm_clk_get(&pdev->dev, "d_audio");
+ if (IS_ERR(tdma->ahub_clk)) {
+ dev_err(&pdev->dev, "Error: Missing ahub controller clock\n");
+ return PTR_ERR(tdma->ahub_clk);
+ }
pm_runtime_enable(&pdev->dev);
@@ -776,8 +776,6 @@ static int tegra_adma_probe(struct platform_device *pdev)
pm_runtime_put_sync(&pdev->dev);
rpm_disable:
pm_runtime_disable(&pdev->dev);
-clk_destroy:
- pm_clk_destroy(&pdev->dev);
return ret;
}
@@ -794,7 +792,6 @@ static int tegra_adma_remove(struct platform_device *pdev)
pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
- pm_clk_destroy(&pdev->dev);
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/2] dmaengine: tegra210-adma: update system sleep callbacks
2019-03-13 5:43 [PATCH v2 1/2] dmaengine: tegra210-adma: use devm_clk_*() helpers Sameer Pujar
@ 2019-03-13 5:43 ` Sameer Pujar
2019-03-13 10:28 ` Jon Hunter
0 siblings, 1 reply; 6+ messages in thread
From: Sameer Pujar @ 2019-03-13 5:43 UTC (permalink / raw)
To: dan.j.williams, vkoul
Cc: treding, jonathanh, dmaengine, linux-tegra, linux-kernel, Sameer Pujar
If the driver is active till late suspend, where runtime PM cannot run,
force suspend is essential in such case to put the device in low power
state. Thus pm_runtime_force_suspend and pm_runtime_force_resume are
used as system sleep callbacks during system wide PM transitions.
Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
drivers/dma/tegra210-adma.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
index 650cd9c..be29171 100644
--- a/drivers/dma/tegra210-adma.c
+++ b/drivers/dma/tegra210-adma.c
@@ -796,17 +796,11 @@ static int tegra_adma_remove(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_PM_SLEEP
-static int tegra_adma_pm_suspend(struct device *dev)
-{
- return pm_runtime_suspended(dev) == false;
-}
-#endif
-
static const struct dev_pm_ops tegra_adma_dev_pm_ops = {
SET_RUNTIME_PM_OPS(tegra_adma_runtime_suspend,
tegra_adma_runtime_resume, NULL)
- SET_SYSTEM_SLEEP_PM_OPS(tegra_adma_pm_suspend, NULL)
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
};
static struct platform_driver tegra_admac_driver = {
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/2] dmaengine: tegra210-adma: update system sleep callbacks
2019-03-13 5:43 ` [PATCH v2 2/2] dmaengine: tegra210-adma: update system sleep callbacks Sameer Pujar
@ 2019-03-13 10:28 ` Jon Hunter
2019-03-13 10:40 ` Sameer Pujar
0 siblings, 1 reply; 6+ messages in thread
From: Jon Hunter @ 2019-03-13 10:28 UTC (permalink / raw)
To: Sameer Pujar, dan.j.williams, vkoul
Cc: treding, dmaengine, linux-tegra, linux-kernel
On 13/03/2019 05:43, Sameer Pujar wrote:
> If the driver is active till late suspend, where runtime PM cannot run,
> force suspend is essential in such case to put the device in low power
> state. Thus pm_runtime_force_suspend and pm_runtime_force_resume are
> used as system sleep callbacks during system wide PM transitions.
>
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> ---
> drivers/dma/tegra210-adma.c | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
> index 650cd9c..be29171 100644
> --- a/drivers/dma/tegra210-adma.c
> +++ b/drivers/dma/tegra210-adma.c
> @@ -796,17 +796,11 @@ static int tegra_adma_remove(struct platform_device *pdev)
> return 0;
> }
>
> -#ifdef CONFIG_PM_SLEEP
> -static int tegra_adma_pm_suspend(struct device *dev)
> -{
> - return pm_runtime_suspended(dev) == false;
> -}
> -#endif
> -
> static const struct dev_pm_ops tegra_adma_dev_pm_ops = {
> SET_RUNTIME_PM_OPS(tegra_adma_runtime_suspend,
> tegra_adma_runtime_resume, NULL)
> - SET_SYSTEM_SLEEP_PM_OPS(tegra_adma_pm_suspend, NULL)
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> };
Looking at our downstream kernel we use LATE_SYSTEM_SLEEP for these. Any
reason why you changed this?
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/2] dmaengine: tegra210-adma: update system sleep callbacks
2019-03-13 10:28 ` Jon Hunter
@ 2019-03-13 10:40 ` Sameer Pujar
2019-03-13 10:49 ` Jon Hunter
0 siblings, 1 reply; 6+ messages in thread
From: Sameer Pujar @ 2019-03-13 10:40 UTC (permalink / raw)
To: Jon Hunter, dan.j.williams, vkoul
Cc: treding, dmaengine, linux-tegra, linux-kernel
On 3/13/2019 3:58 PM, Jon Hunter wrote:
> On 13/03/2019 05:43, Sameer Pujar wrote:
>> If the driver is active till late suspend, where runtime PM cannot run,
>> force suspend is essential in such case to put the device in low power
>> state. Thus pm_runtime_force_suspend and pm_runtime_force_resume are
>> used as system sleep callbacks during system wide PM transitions.
>>
>> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
>> ---
>> drivers/dma/tegra210-adma.c | 10 ++--------
>> 1 file changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
>> index 650cd9c..be29171 100644
>> --- a/drivers/dma/tegra210-adma.c
>> +++ b/drivers/dma/tegra210-adma.c
>> @@ -796,17 +796,11 @@ static int tegra_adma_remove(struct platform_device *pdev)
>> return 0;
>> }
>>
>> -#ifdef CONFIG_PM_SLEEP
>> -static int tegra_adma_pm_suspend(struct device *dev)
>> -{
>> - return pm_runtime_suspended(dev) == false;
>> -}
>> -#endif
>> -
>> static const struct dev_pm_ops tegra_adma_dev_pm_ops = {
>> SET_RUNTIME_PM_OPS(tegra_adma_runtime_suspend,
>> tegra_adma_runtime_resume, NULL)
>> - SET_SYSTEM_SLEEP_PM_OPS(tegra_adma_pm_suspend, NULL)
>> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>> + pm_runtime_force_resume)
>> };
> Looking at our downstream kernel we use LATE_SYSTEM_SLEEP for these. Any
> reason why you changed this?
I think, I just wanted to replace function calls for system sleep here
and probably did
not see exactly what we have in downstream kernel at that point. Looking
at the commit
log in downstream, it might qualify for separate patch.
Let me know if you think, its better to add here.
>
> Cheers
> Jon
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/2] dmaengine: tegra210-adma: update system sleep callbacks
2019-03-13 10:40 ` Sameer Pujar
@ 2019-03-13 10:49 ` Jon Hunter
2019-03-13 10:56 ` Sameer Pujar
0 siblings, 1 reply; 6+ messages in thread
From: Jon Hunter @ 2019-03-13 10:49 UTC (permalink / raw)
To: Sameer Pujar, dan.j.williams, vkoul
Cc: treding, dmaengine, linux-tegra, linux-kernel
On 13/03/2019 10:40, Sameer Pujar wrote:
>
> On 3/13/2019 3:58 PM, Jon Hunter wrote:
>> On 13/03/2019 05:43, Sameer Pujar wrote:
>>> If the driver is active till late suspend, where runtime PM cannot run,
>>> force suspend is essential in such case to put the device in low power
>>> state. Thus pm_runtime_force_suspend and pm_runtime_force_resume are
>>> used as system sleep callbacks during system wide PM transitions.
>>>
>>> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
>>> ---
>>> drivers/dma/tegra210-adma.c | 10 ++--------
>>> 1 file changed, 2 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
>>> index 650cd9c..be29171 100644
>>> --- a/drivers/dma/tegra210-adma.c
>>> +++ b/drivers/dma/tegra210-adma.c
>>> @@ -796,17 +796,11 @@ static int tegra_adma_remove(struct
>>> platform_device *pdev)
>>> return 0;
>>> }
>>> -#ifdef CONFIG_PM_SLEEP
>>> -static int tegra_adma_pm_suspend(struct device *dev)
>>> -{
>>> - return pm_runtime_suspended(dev) == false;
>>> -}
>>> -#endif
>>> -
>>> static const struct dev_pm_ops tegra_adma_dev_pm_ops = {
>>> SET_RUNTIME_PM_OPS(tegra_adma_runtime_suspend,
>>> tegra_adma_runtime_resume, NULL)
>>> - SET_SYSTEM_SLEEP_PM_OPS(tegra_adma_pm_suspend, NULL)
>>> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>>> + pm_runtime_force_resume)
>>> };
>> Looking at our downstream kernel we use LATE_SYSTEM_SLEEP for these. Any
>> reason why you changed this?
> I think, I just wanted to replace function calls for system sleep here
> and probably did
> not see exactly what we have in downstream kernel at that point. Looking
> at the commit
> log in downstream, it might qualify for separate patch.
> Let me know if you think, its better to add here.
I see no reason to change this from what we have been using and testing
downstream. I don't think that this warrants yet another patch for this.
Furthermore, the changelog references 'late' so it does not seem to
align with the change itself.
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/2] dmaengine: tegra210-adma: update system sleep callbacks
2019-03-13 10:49 ` Jon Hunter
@ 2019-03-13 10:56 ` Sameer Pujar
0 siblings, 0 replies; 6+ messages in thread
From: Sameer Pujar @ 2019-03-13 10:56 UTC (permalink / raw)
To: Jon Hunter, dan.j.williams, vkoul
Cc: treding, dmaengine, linux-tegra, linux-kernel
On 3/13/2019 4:19 PM, Jon Hunter wrote:
> On 13/03/2019 10:40, Sameer Pujar wrote:
>> On 3/13/2019 3:58 PM, Jon Hunter wrote:
>>> On 13/03/2019 05:43, Sameer Pujar wrote:
>>>> If the driver is active till late suspend, where runtime PM cannot run,
>>>> force suspend is essential in such case to put the device in low power
>>>> state. Thus pm_runtime_force_suspend and pm_runtime_force_resume are
>>>> used as system sleep callbacks during system wide PM transitions.
>>>>
>>>> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
>>>> ---
>>>> drivers/dma/tegra210-adma.c | 10 ++--------
>>>> 1 file changed, 2 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
>>>> index 650cd9c..be29171 100644
>>>> --- a/drivers/dma/tegra210-adma.c
>>>> +++ b/drivers/dma/tegra210-adma.c
>>>> @@ -796,17 +796,11 @@ static int tegra_adma_remove(struct
>>>> platform_device *pdev)
>>>> return 0;
>>>> }
>>>> -#ifdef CONFIG_PM_SLEEP
>>>> -static int tegra_adma_pm_suspend(struct device *dev)
>>>> -{
>>>> - return pm_runtime_suspended(dev) == false;
>>>> -}
>>>> -#endif
>>>> -
>>>> static const struct dev_pm_ops tegra_adma_dev_pm_ops = {
>>>> SET_RUNTIME_PM_OPS(tegra_adma_runtime_suspend,
>>>> tegra_adma_runtime_resume, NULL)
>>>> - SET_SYSTEM_SLEEP_PM_OPS(tegra_adma_pm_suspend, NULL)
>>>> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>>>> + pm_runtime_force_resume)
>>>> };
>>> Looking at our downstream kernel we use LATE_SYSTEM_SLEEP for these. Any
>>> reason why you changed this?
>> I think, I just wanted to replace function calls for system sleep here
>> and probably did
>> not see exactly what we have in downstream kernel at that point. Looking
>> at the commit
>> log in downstream, it might qualify for separate patch.
>> Let me know if you think, its better to add here.
> I see no reason to change this from what we have been using and testing
> downstream. I don't think that this warrants yet another patch for this.
> Furthermore, the changelog references 'late' so it does not seem to
> align with the change itself.
Agree, will update. Thanks.
>
> Cheers
> Jon
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-13 10:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-13 5:43 [PATCH v2 1/2] dmaengine: tegra210-adma: use devm_clk_*() helpers Sameer Pujar
2019-03-13 5:43 ` [PATCH v2 2/2] dmaengine: tegra210-adma: update system sleep callbacks Sameer Pujar
2019-03-13 10:28 ` Jon Hunter
2019-03-13 10:40 ` Sameer Pujar
2019-03-13 10:49 ` Jon Hunter
2019-03-13 10:56 ` Sameer Pujar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).