* [PATCH V2] dmaengine: tegra-apb: Really fix runtime-pm usage
@ 2017-06-06 12:49 Jon Hunter
[not found] ` <1496753369-5356-1-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Jon Hunter @ 2017-06-06 12:49 UTC (permalink / raw)
To: Laxman Dewangan, Vinod Koul
Cc: Thierry Reding, dmaengine-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA, Jon Hunter
Commit edd3bdbe9db1 ("dmaengine: tegra-apb: Correct runtime-pm usage")
added pm_runtime_get/put() calls to the tegra-apb DMA system suspend
callbacks. Runtime PM is disabled during system suspend and so these
APIs cannot be used. Fix the suspend handling for the tegra-apb DMA by
moving the save and restore of the DMA register context into the
runtime PM suspend and resume callbacks, and then use the
pm_runtime_force_suspend/resume() APIs to invoke the runtime PM
callbacks during system suspend.
Fixes: edd3bdbe9db1 ("dmaengine: tegra-apb: Correct runtime-pm usage")
Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
Changes since V1:
- Drop the custom suspend/resume callbacks and use
pm_runtime_force_suspend/resume directly in SET_SYSTEM_SLEEP_PM_OPS.
drivers/dma/tegra20-apb-dma.c | 50 +++++++++----------------------------------
1 file changed, 10 insertions(+), 40 deletions(-)
diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index 3722b9d8d9fe..b9d75a54c896 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -1494,35 +1494,7 @@ static int tegra_dma_remove(struct platform_device *pdev)
static int tegra_dma_runtime_suspend(struct device *dev)
{
struct tegra_dma *tdma = dev_get_drvdata(dev);
-
- clk_disable_unprepare(tdma->dma_clk);
- return 0;
-}
-
-static int tegra_dma_runtime_resume(struct device *dev)
-{
- struct tegra_dma *tdma = dev_get_drvdata(dev);
- int ret;
-
- ret = clk_prepare_enable(tdma->dma_clk);
- if (ret < 0) {
- dev_err(dev, "clk_enable failed: %d\n", ret);
- return ret;
- }
- return 0;
-}
-
-#ifdef CONFIG_PM_SLEEP
-static int tegra_dma_pm_suspend(struct device *dev)
-{
- struct tegra_dma *tdma = dev_get_drvdata(dev);
int i;
- int ret;
-
- /* Enable clock before accessing register */
- ret = pm_runtime_get_sync(dev);
- if (ret < 0)
- return ret;
tdma->reg_gen = tdma_read(tdma, TEGRA_APBDMA_GENERAL);
for (i = 0; i < tdma->chip_data->nr_channels; i++) {
@@ -1543,21 +1515,21 @@ static int tegra_dma_pm_suspend(struct device *dev)
TEGRA_APBDMA_CHAN_WCOUNT);
}
- /* Disable clock */
- pm_runtime_put(dev);
+ clk_disable_unprepare(tdma->dma_clk);
+
return 0;
}
-static int tegra_dma_pm_resume(struct device *dev)
+static int tegra_dma_runtime_resume(struct device *dev)
{
struct tegra_dma *tdma = dev_get_drvdata(dev);
- int i;
- int ret;
+ int i, ret;
- /* Enable clock before accessing register */
- ret = pm_runtime_get_sync(dev);
- if (ret < 0)
+ ret = clk_prepare_enable(tdma->dma_clk);
+ if (ret < 0) {
+ dev_err(dev, "clk_enable failed: %d\n", ret);
return ret;
+ }
tdma_write(tdma, TEGRA_APBDMA_GENERAL, tdma->reg_gen);
tdma_write(tdma, TEGRA_APBDMA_CONTROL, 0);
@@ -1582,16 +1554,14 @@ static int tegra_dma_pm_resume(struct device *dev)
(ch_reg->csr & ~TEGRA_APBDMA_CSR_ENB));
}
- /* Disable clock */
- pm_runtime_put(dev);
return 0;
}
-#endif
static const struct dev_pm_ops tegra_dma_dev_pm_ops = {
SET_RUNTIME_PM_OPS(tegra_dma_runtime_suspend, tegra_dma_runtime_resume,
NULL)
- SET_SYSTEM_SLEEP_PM_OPS(tegra_dma_pm_suspend, tegra_dma_pm_resume)
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
};
static const struct of_device_id tegra_dma_of_match[] = {
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V2] dmaengine: tegra-apb: Really fix runtime-pm usage
[not found] ` <1496753369-5356-1-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2017-06-06 14:48 ` Thierry Reding
2017-06-27 11:44 ` Jon Hunter
2017-06-30 5:44 ` Vinod Koul
1 sibling, 1 reply; 5+ messages in thread
From: Thierry Reding @ 2017-06-06 14:48 UTC (permalink / raw)
To: Jon Hunter
Cc: Laxman Dewangan, Vinod Koul, dmaengine-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 3821 bytes --]
On Tue, Jun 06, 2017 at 01:49:29PM +0100, Jon Hunter wrote:
> Commit edd3bdbe9db1 ("dmaengine: tegra-apb: Correct runtime-pm usage")
> added pm_runtime_get/put() calls to the tegra-apb DMA system suspend
> callbacks. Runtime PM is disabled during system suspend and so these
> APIs cannot be used. Fix the suspend handling for the tegra-apb DMA by
> moving the save and restore of the DMA register context into the
> runtime PM suspend and resume callbacks, and then use the
> pm_runtime_force_suspend/resume() APIs to invoke the runtime PM
> callbacks during system suspend.
>
> Fixes: edd3bdbe9db1 ("dmaengine: tegra-apb: Correct runtime-pm usage")
>
> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>
> Changes since V1:
> - Drop the custom suspend/resume callbacks and use
> pm_runtime_force_suspend/resume directly in SET_SYSTEM_SLEEP_PM_OPS.
>
> drivers/dma/tegra20-apb-dma.c | 50 +++++++++----------------------------------
> 1 file changed, 10 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
> index 3722b9d8d9fe..b9d75a54c896 100644
> --- a/drivers/dma/tegra20-apb-dma.c
> +++ b/drivers/dma/tegra20-apb-dma.c
> @@ -1494,35 +1494,7 @@ static int tegra_dma_remove(struct platform_device *pdev)
> static int tegra_dma_runtime_suspend(struct device *dev)
> {
> struct tegra_dma *tdma = dev_get_drvdata(dev);
> -
> - clk_disable_unprepare(tdma->dma_clk);
> - return 0;
> -}
> -
> -static int tegra_dma_runtime_resume(struct device *dev)
> -{
> - struct tegra_dma *tdma = dev_get_drvdata(dev);
> - int ret;
> -
> - ret = clk_prepare_enable(tdma->dma_clk);
> - if (ret < 0) {
> - dev_err(dev, "clk_enable failed: %d\n", ret);
> - return ret;
> - }
> - return 0;
> -}
> -
> -#ifdef CONFIG_PM_SLEEP
> -static int tegra_dma_pm_suspend(struct device *dev)
> -{
> - struct tegra_dma *tdma = dev_get_drvdata(dev);
> int i;
> - int ret;
> -
> - /* Enable clock before accessing register */
> - ret = pm_runtime_get_sync(dev);
> - if (ret < 0)
> - return ret;
>
> tdma->reg_gen = tdma_read(tdma, TEGRA_APBDMA_GENERAL);
> for (i = 0; i < tdma->chip_data->nr_channels; i++) {
> @@ -1543,21 +1515,21 @@ static int tegra_dma_pm_suspend(struct device *dev)
> TEGRA_APBDMA_CHAN_WCOUNT);
> }
>
> - /* Disable clock */
> - pm_runtime_put(dev);
> + clk_disable_unprepare(tdma->dma_clk);
> +
> return 0;
> }
>
> -static int tegra_dma_pm_resume(struct device *dev)
> +static int tegra_dma_runtime_resume(struct device *dev)
> {
> struct tegra_dma *tdma = dev_get_drvdata(dev);
> - int i;
> - int ret;
> + int i, ret;
>
> - /* Enable clock before accessing register */
> - ret = pm_runtime_get_sync(dev);
> - if (ret < 0)
> + ret = clk_prepare_enable(tdma->dma_clk);
> + if (ret < 0) {
> + dev_err(dev, "clk_enable failed: %d\n", ret);
> return ret;
> + }
>
> tdma_write(tdma, TEGRA_APBDMA_GENERAL, tdma->reg_gen);
> tdma_write(tdma, TEGRA_APBDMA_CONTROL, 0);
> @@ -1582,16 +1554,14 @@ static int tegra_dma_pm_resume(struct device *dev)
> (ch_reg->csr & ~TEGRA_APBDMA_CSR_ENB));
> }
>
> - /* Disable clock */
> - pm_runtime_put(dev);
> return 0;
> }
> -#endif
>
> static const struct dev_pm_ops tegra_dma_dev_pm_ops = {
> SET_RUNTIME_PM_OPS(tegra_dma_runtime_suspend, tegra_dma_runtime_resume,
> NULL)
> - SET_SYSTEM_SLEEP_PM_OPS(tegra_dma_pm_suspend, tegra_dma_pm_resume)
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
Is that even necessary? I thought runtime PM was going to be triggered
for system sleep anyway, but it looks like there are other examples of
this usage, so maybe I'm mistaken.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V2] dmaengine: tegra-apb: Really fix runtime-pm usage
2017-06-06 14:48 ` Thierry Reding
@ 2017-06-27 11:44 ` Jon Hunter
[not found] ` <f336e72a-d966-11e4-0885-e324843d5c00-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Jon Hunter @ 2017-06-27 11:44 UTC (permalink / raw)
To: Thierry Reding, Vinod Koul
Cc: Laxman Dewangan, dmaengine-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA
On 06/06/17 15:48, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Tue, Jun 06, 2017 at 01:49:29PM +0100, Jon Hunter wrote:
>> Commit edd3bdbe9db1 ("dmaengine: tegra-apb: Correct runtime-pm usage")
>> added pm_runtime_get/put() calls to the tegra-apb DMA system suspend
>> callbacks. Runtime PM is disabled during system suspend and so these
>> APIs cannot be used. Fix the suspend handling for the tegra-apb DMA by
>> moving the save and restore of the DMA register context into the
>> runtime PM suspend and resume callbacks, and then use the
>> pm_runtime_force_suspend/resume() APIs to invoke the runtime PM
>> callbacks during system suspend.
>>
>> Fixes: edd3bdbe9db1 ("dmaengine: tegra-apb: Correct runtime-pm usage")
>>
>> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>>
>> Changes since V1:
>> - Drop the custom suspend/resume callbacks and use
>> pm_runtime_force_suspend/resume directly in SET_SYSTEM_SLEEP_PM_OPS.
>>
>> drivers/dma/tegra20-apb-dma.c | 50 +++++++++----------------------------------
>> 1 file changed, 10 insertions(+), 40 deletions(-)
>>
>> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
>> index 3722b9d8d9fe..b9d75a54c896 100644
>> --- a/drivers/dma/tegra20-apb-dma.c
>> +++ b/drivers/dma/tegra20-apb-dma.c
>> @@ -1494,35 +1494,7 @@ static int tegra_dma_remove(struct platform_device *pdev)
>> static int tegra_dma_runtime_suspend(struct device *dev)
>> {
>> struct tegra_dma *tdma = dev_get_drvdata(dev);
>> -
>> - clk_disable_unprepare(tdma->dma_clk);
>> - return 0;
>> -}
>> -
>> -static int tegra_dma_runtime_resume(struct device *dev)
>> -{
>> - struct tegra_dma *tdma = dev_get_drvdata(dev);
>> - int ret;
>> -
>> - ret = clk_prepare_enable(tdma->dma_clk);
>> - if (ret < 0) {
>> - dev_err(dev, "clk_enable failed: %d\n", ret);
>> - return ret;
>> - }
>> - return 0;
>> -}
>> -
>> -#ifdef CONFIG_PM_SLEEP
>> -static int tegra_dma_pm_suspend(struct device *dev)
>> -{
>> - struct tegra_dma *tdma = dev_get_drvdata(dev);
>> int i;
>> - int ret;
>> -
>> - /* Enable clock before accessing register */
>> - ret = pm_runtime_get_sync(dev);
>> - if (ret < 0)
>> - return ret;
>>
>> tdma->reg_gen = tdma_read(tdma, TEGRA_APBDMA_GENERAL);
>> for (i = 0; i < tdma->chip_data->nr_channels; i++) {
>> @@ -1543,21 +1515,21 @@ static int tegra_dma_pm_suspend(struct device *dev)
>> TEGRA_APBDMA_CHAN_WCOUNT);
>> }
>>
>> - /* Disable clock */
>> - pm_runtime_put(dev);
>> + clk_disable_unprepare(tdma->dma_clk);
>> +
>> return 0;
>> }
>>
>> -static int tegra_dma_pm_resume(struct device *dev)
>> +static int tegra_dma_runtime_resume(struct device *dev)
>> {
>> struct tegra_dma *tdma = dev_get_drvdata(dev);
>> - int i;
>> - int ret;
>> + int i, ret;
>>
>> - /* Enable clock before accessing register */
>> - ret = pm_runtime_get_sync(dev);
>> - if (ret < 0)
>> + ret = clk_prepare_enable(tdma->dma_clk);
>> + if (ret < 0) {
>> + dev_err(dev, "clk_enable failed: %d\n", ret);
>> return ret;
>> + }
>>
>> tdma_write(tdma, TEGRA_APBDMA_GENERAL, tdma->reg_gen);
>> tdma_write(tdma, TEGRA_APBDMA_CONTROL, 0);
>> @@ -1582,16 +1554,14 @@ static int tegra_dma_pm_resume(struct device *dev)
>> (ch_reg->csr & ~TEGRA_APBDMA_CSR_ENB));
>> }
>>
>> - /* Disable clock */
>> - pm_runtime_put(dev);
>> return 0;
>> }
>> -#endif
>>
>> static const struct dev_pm_ops tegra_dma_dev_pm_ops = {
>> SET_RUNTIME_PM_OPS(tegra_dma_runtime_suspend, tegra_dma_runtime_resume,
>> NULL)
>> - SET_SYSTEM_SLEEP_PM_OPS(tegra_dma_pm_suspend, tegra_dma_pm_resume)
>> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>> + pm_runtime_force_resume)
>
> Is that even necessary? I thought runtime PM was going to be triggered
> for system sleep anyway, but it looks like there are other examples of
> this usage, so maybe I'm mistaken.
Yes this is necessary. No RPM is not automatically trigger by system
suspend AFAICT.
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V2] dmaengine: tegra-apb: Really fix runtime-pm usage
[not found] ` <f336e72a-d966-11e4-0885-e324843d5c00-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2017-06-30 5:42 ` Vinod Koul
0 siblings, 0 replies; 5+ messages in thread
From: Vinod Koul @ 2017-06-30 5:42 UTC (permalink / raw)
To: Jon Hunter
Cc: Thierry Reding, Laxman Dewangan,
dmaengine-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA
On Tue, Jun 27, 2017 at 12:44:58PM +0100, Jon Hunter wrote:
> >> static const struct dev_pm_ops tegra_dma_dev_pm_ops = {
> >> SET_RUNTIME_PM_OPS(tegra_dma_runtime_suspend, tegra_dma_runtime_resume,
> >> NULL)
> >> - SET_SYSTEM_SLEEP_PM_OPS(tegra_dma_pm_suspend, tegra_dma_pm_resume)
> >> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> >> + pm_runtime_force_resume)
> >
> > Is that even necessary? I thought runtime PM was going to be triggered
> > for system sleep anyway, but it looks like there are other examples of
> > this usage, so maybe I'm mistaken.
>
> Yes this is necessary. No RPM is not automatically trigger by system
> suspend AFAICT.
Yes I was earlier under the same impression but later did realize that the
behaviour seems to be arch specific and we don't have guarantee on this
--
~Vinod
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V2] dmaengine: tegra-apb: Really fix runtime-pm usage
[not found] ` <1496753369-5356-1-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2017-06-06 14:48 ` Thierry Reding
@ 2017-06-30 5:44 ` Vinod Koul
1 sibling, 0 replies; 5+ messages in thread
From: Vinod Koul @ 2017-06-30 5:44 UTC (permalink / raw)
To: Jon Hunter
Cc: Laxman Dewangan, Thierry Reding,
dmaengine-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA
On Tue, Jun 06, 2017 at 01:49:29PM +0100, Jon Hunter wrote:
> Commit edd3bdbe9db1 ("dmaengine: tegra-apb: Correct runtime-pm usage")
> added pm_runtime_get/put() calls to the tegra-apb DMA system suspend
> callbacks. Runtime PM is disabled during system suspend and so these
> APIs cannot be used. Fix the suspend handling for the tegra-apb DMA by
> moving the save and restore of the DMA register context into the
> runtime PM suspend and resume callbacks, and then use the
> pm_runtime_force_suspend/resume() APIs to invoke the runtime PM
> callbacks during system suspend.
Applied, thanks
--
~Vinod
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-06-30 5:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-06 12:49 [PATCH V2] dmaengine: tegra-apb: Really fix runtime-pm usage Jon Hunter
[not found] ` <1496753369-5356-1-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2017-06-06 14:48 ` Thierry Reding
2017-06-27 11:44 ` Jon Hunter
[not found] ` <f336e72a-d966-11e4-0885-e324843d5c00-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2017-06-30 5:42 ` Vinod Koul
2017-06-30 5:44 ` Vinod Koul
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.