* [PATCH v2] dma: qcom: bam_dma: Manage clocks when controlled_remotely is set
@ 2021-01-26 21:18 Thara Gopinath
2021-01-27 1:33 ` Shawn Guo
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Thara Gopinath @ 2021-01-26 21:18 UTC (permalink / raw)
To: agross, bjorn.andersson, dan.j.williams, vkoul
Cc: shawn.guo, srinivas.kandagatla, linux-arm-msm, dmaengine, linux-kernel
When bam dma is "controlled remotely", thus far clocks were not controlled
from the Linux. In this scenario, Linux was disabling runtime pm in bam dma
driver and not doing any clock management in suspend/resume hooks.
With introduction of crypto engine bam dma, the clock is a rpmh resource
that can be controlled from both Linux and TZ/remote side. Now bam dma
clock is getting enabled during probe even though the bam dma can be
"controlled remotely". But due to clocks not being handled properly,
bam_suspend generates a unbalanced clk_unprepare warning during system
suspend.
To fix the above issue and to enable proper clock-management, this patch
enables runtim-pm and handles bam dma clocks in suspend/resume hooks if
the clock node is present irrespective of controlled_remotely property.
Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
---
v1->v2:
- As per Shawn's suggestion, use devm_clk_get_optional to get the
bam clock if the "controlled_remotely" property is set so that
the clock code takes care of setting the bam clock to NULL if
not specified by dt.
- Remove the check for "controlled_remotely" property in
bam_dma_resume now that clock enable / disable is based on
whether bamclk is NULL or not.
- Rebased to v5.11-rc5
drivers/dma/qcom/bam_dma.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 88579857ca1d..c8a77b428b52 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -1270,13 +1270,13 @@ static int bam_dma_probe(struct platform_device *pdev)
dev_err(bdev->dev, "num-ees unspecified in dt\n");
}
- bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
- if (IS_ERR(bdev->bamclk)) {
- if (!bdev->controlled_remotely)
- return PTR_ERR(bdev->bamclk);
+ if (bdev->controlled_remotely)
+ bdev->bamclk = devm_clk_get_optional(bdev->dev, "bam_clk");
+ else
+ bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
- bdev->bamclk = NULL;
- }
+ if (IS_ERR(bdev->bamclk))
+ return PTR_ERR(bdev->bamclk);
ret = clk_prepare_enable(bdev->bamclk);
if (ret) {
@@ -1350,7 +1350,7 @@ static int bam_dma_probe(struct platform_device *pdev)
if (ret)
goto err_unregister_dma;
- if (bdev->controlled_remotely) {
+ if (!bdev->bamclk) {
pm_runtime_disable(&pdev->dev);
return 0;
}
@@ -1438,10 +1438,10 @@ static int __maybe_unused bam_dma_suspend(struct device *dev)
{
struct bam_device *bdev = dev_get_drvdata(dev);
- if (!bdev->controlled_remotely)
+ if (bdev->bamclk) {
pm_runtime_force_suspend(dev);
-
- clk_unprepare(bdev->bamclk);
+ clk_unprepare(bdev->bamclk);
+ }
return 0;
}
@@ -1451,12 +1451,13 @@ static int __maybe_unused bam_dma_resume(struct device *dev)
struct bam_device *bdev = dev_get_drvdata(dev);
int ret;
- ret = clk_prepare(bdev->bamclk);
- if (ret)
- return ret;
+ if (bdev->bamclk) {
+ ret = clk_prepare(bdev->bamclk);
+ if (ret)
+ return ret;
- if (!bdev->controlled_remotely)
pm_runtime_force_resume(dev);
+ }
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] dma: qcom: bam_dma: Manage clocks when controlled_remotely is set
2021-01-26 21:18 [PATCH v2] dma: qcom: bam_dma: Manage clocks when controlled_remotely is set Thara Gopinath
@ 2021-01-27 1:33 ` Shawn Guo
2021-01-27 5:33 ` Bjorn Andersson
2021-01-27 15:42 ` Vinod Koul
2 siblings, 0 replies; 4+ messages in thread
From: Shawn Guo @ 2021-01-27 1:33 UTC (permalink / raw)
To: Thara Gopinath
Cc: agross, bjorn.andersson, dan.j.williams, vkoul,
srinivas.kandagatla, linux-arm-msm, dmaengine, linux-kernel
On Tue, Jan 26, 2021 at 04:18:59PM -0500, Thara Gopinath wrote:
> When bam dma is "controlled remotely", thus far clocks were not controlled
> from the Linux. In this scenario, Linux was disabling runtime pm in bam dma
> driver and not doing any clock management in suspend/resume hooks.
>
> With introduction of crypto engine bam dma, the clock is a rpmh resource
> that can be controlled from both Linux and TZ/remote side. Now bam dma
> clock is getting enabled during probe even though the bam dma can be
> "controlled remotely". But due to clocks not being handled properly,
> bam_suspend generates a unbalanced clk_unprepare warning during system
> suspend.
>
> To fix the above issue and to enable proper clock-management, this patch
> enables runtim-pm and handles bam dma clocks in suspend/resume hooks if
> the clock node is present irrespective of controlled_remotely property.
>
> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
Reviewed-by: Shawn Guo <shawn.guo@linaro.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] dma: qcom: bam_dma: Manage clocks when controlled_remotely is set
2021-01-26 21:18 [PATCH v2] dma: qcom: bam_dma: Manage clocks when controlled_remotely is set Thara Gopinath
2021-01-27 1:33 ` Shawn Guo
@ 2021-01-27 5:33 ` Bjorn Andersson
2021-01-27 15:42 ` Vinod Koul
2 siblings, 0 replies; 4+ messages in thread
From: Bjorn Andersson @ 2021-01-27 5:33 UTC (permalink / raw)
To: Thara Gopinath
Cc: agross, dan.j.williams, vkoul, shawn.guo, srinivas.kandagatla,
linux-arm-msm, dmaengine, linux-kernel
On Tue 26 Jan 15:18 CST 2021, Thara Gopinath wrote:
> When bam dma is "controlled remotely", thus far clocks were not controlled
> from the Linux. In this scenario, Linux was disabling runtime pm in bam dma
> driver and not doing any clock management in suspend/resume hooks.
>
> With introduction of crypto engine bam dma, the clock is a rpmh resource
> that can be controlled from both Linux and TZ/remote side. Now bam dma
> clock is getting enabled during probe even though the bam dma can be
> "controlled remotely". But due to clocks not being handled properly,
> bam_suspend generates a unbalanced clk_unprepare warning during system
> suspend.
>
> To fix the above issue and to enable proper clock-management, this patch
> enables runtim-pm and handles bam dma clocks in suspend/resume hooks if
> the clock node is present irrespective of controlled_remotely property.
>
> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
And from John on IRC:
Tested-by: John Stultz <john.stultz@linaro.org>
Regards,
Bjorn
> ---
>
> v1->v2:
> - As per Shawn's suggestion, use devm_clk_get_optional to get the
> bam clock if the "controlled_remotely" property is set so that
> the clock code takes care of setting the bam clock to NULL if
> not specified by dt.
> - Remove the check for "controlled_remotely" property in
> bam_dma_resume now that clock enable / disable is based on
> whether bamclk is NULL or not.
> - Rebased to v5.11-rc5
>
> drivers/dma/qcom/bam_dma.c | 29 +++++++++++++++--------------
> 1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> index 88579857ca1d..c8a77b428b52 100644
> --- a/drivers/dma/qcom/bam_dma.c
> +++ b/drivers/dma/qcom/bam_dma.c
> @@ -1270,13 +1270,13 @@ static int bam_dma_probe(struct platform_device *pdev)
> dev_err(bdev->dev, "num-ees unspecified in dt\n");
> }
>
> - bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
> - if (IS_ERR(bdev->bamclk)) {
> - if (!bdev->controlled_remotely)
> - return PTR_ERR(bdev->bamclk);
> + if (bdev->controlled_remotely)
> + bdev->bamclk = devm_clk_get_optional(bdev->dev, "bam_clk");
> + else
> + bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
>
> - bdev->bamclk = NULL;
> - }
> + if (IS_ERR(bdev->bamclk))
> + return PTR_ERR(bdev->bamclk);
>
> ret = clk_prepare_enable(bdev->bamclk);
> if (ret) {
> @@ -1350,7 +1350,7 @@ static int bam_dma_probe(struct platform_device *pdev)
> if (ret)
> goto err_unregister_dma;
>
> - if (bdev->controlled_remotely) {
> + if (!bdev->bamclk) {
> pm_runtime_disable(&pdev->dev);
> return 0;
> }
> @@ -1438,10 +1438,10 @@ static int __maybe_unused bam_dma_suspend(struct device *dev)
> {
> struct bam_device *bdev = dev_get_drvdata(dev);
>
> - if (!bdev->controlled_remotely)
> + if (bdev->bamclk) {
> pm_runtime_force_suspend(dev);
> -
> - clk_unprepare(bdev->bamclk);
> + clk_unprepare(bdev->bamclk);
> + }
>
> return 0;
> }
> @@ -1451,12 +1451,13 @@ static int __maybe_unused bam_dma_resume(struct device *dev)
> struct bam_device *bdev = dev_get_drvdata(dev);
> int ret;
>
> - ret = clk_prepare(bdev->bamclk);
> - if (ret)
> - return ret;
> + if (bdev->bamclk) {
> + ret = clk_prepare(bdev->bamclk);
> + if (ret)
> + return ret;
>
> - if (!bdev->controlled_remotely)
> pm_runtime_force_resume(dev);
> + }
>
> return 0;
> }
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] dma: qcom: bam_dma: Manage clocks when controlled_remotely is set
2021-01-26 21:18 [PATCH v2] dma: qcom: bam_dma: Manage clocks when controlled_remotely is set Thara Gopinath
2021-01-27 1:33 ` Shawn Guo
2021-01-27 5:33 ` Bjorn Andersson
@ 2021-01-27 15:42 ` Vinod Koul
2 siblings, 0 replies; 4+ messages in thread
From: Vinod Koul @ 2021-01-27 15:42 UTC (permalink / raw)
To: Thara Gopinath
Cc: agross, bjorn.andersson, dan.j.williams, shawn.guo,
srinivas.kandagatla, linux-arm-msm, dmaengine, linux-kernel
On 26-01-21, 16:18, Thara Gopinath wrote:
> When bam dma is "controlled remotely", thus far clocks were not controlled
> from the Linux. In this scenario, Linux was disabling runtime pm in bam dma
> driver and not doing any clock management in suspend/resume hooks.
>
> With introduction of crypto engine bam dma, the clock is a rpmh resource
> that can be controlled from both Linux and TZ/remote side. Now bam dma
> clock is getting enabled during probe even though the bam dma can be
> "controlled remotely". But due to clocks not being handled properly,
> bam_suspend generates a unbalanced clk_unprepare warning during system
> suspend.
>
> To fix the above issue and to enable proper clock-management, this patch
> enables runtim-pm and handles bam dma clocks in suspend/resume hooks if
> the clock node is present irrespective of controlled_remotely property.
Applied after fixing subsystem name, thanks
--
~Vinod
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-01-27 15:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-26 21:18 [PATCH v2] dma: qcom: bam_dma: Manage clocks when controlled_remotely is set Thara Gopinath
2021-01-27 1:33 ` Shawn Guo
2021-01-27 5:33 ` Bjorn Andersson
2021-01-27 15:42 ` Vinod Koul
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).