| Message ID | 1552455794-24946-2-git-send-email-spujar@nvidia.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2,1/2] dmaengine: tegra210-adma: use devm_clk_*() helpers | expand |
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
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 >
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
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 >
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 = {
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(-)