aconnect bus driver is using pm_clk_*() interface for managing clocks.
With this, clocks seem to be always ON. 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 calls to
BPMP are always blocking. When pm_clk_*() interface is used on such
Tegra devices, clock prepare count is not balanced till driver remove()
gets executed and hence clocks are seen ON always. Thus this patch
replaces pm_clk_*() with devm_clk_*() framework.
Suggested-by: Mohan Kumar D <[email protected]>
Reviewed-by: Jonathan Hunter <[email protected]>
Signed-off-by: Sameer Pujar <[email protected]>
---
drivers/bus/tegra-aconnect.c | 64 ++++++++++++++++++++++++++++++--------------
1 file changed, 44 insertions(+), 20 deletions(-)
diff --git a/drivers/bus/tegra-aconnect.c b/drivers/bus/tegra-aconnect.c
index 084ae28..9349157 100644
--- a/drivers/bus/tegra-aconnect.c
+++ b/drivers/bus/tegra-aconnect.c
@@ -12,28 +12,38 @@
#include <linux/module.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
-#include <linux/pm_clock.h>
#include <linux/pm_runtime.h>
+struct tegra_aconnect {
+ struct clk *ape_clk;
+ struct clk *apb2ape_clk;
+};
+
static int tegra_aconnect_probe(struct platform_device *pdev)
{
- int ret;
+ struct tegra_aconnect *aconnect;
if (!pdev->dev.of_node)
return -EINVAL;
- ret = pm_clk_create(&pdev->dev);
- if (ret)
- return ret;
+ aconnect = devm_kzalloc(&pdev->dev, sizeof(struct tegra_aconnect),
+ GFP_KERNEL);
+ if (!aconnect)
+ return -ENOMEM;
- ret = of_pm_clk_add_clk(&pdev->dev, "ape");
- if (ret)
- goto clk_destroy;
+ aconnect->ape_clk = devm_clk_get(&pdev->dev, "ape");
+ if (IS_ERR(aconnect->ape_clk)) {
+ dev_err(&pdev->dev, "Can't retrieve ape clock\n");
+ return PTR_ERR(aconnect->ape_clk);
+ }
- ret = of_pm_clk_add_clk(&pdev->dev, "apb2ape");
- if (ret)
- goto clk_destroy;
+ aconnect->apb2ape_clk = devm_clk_get(&pdev->dev, "apb2ape");
+ if (IS_ERR(aconnect->apb2ape_clk)) {
+ dev_err(&pdev->dev, "Can't retrieve apb2ape clock\n");
+ return PTR_ERR(aconnect->apb2ape_clk);
+ }
+ dev_set_drvdata(&pdev->dev, aconnect);
pm_runtime_enable(&pdev->dev);
of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
@@ -41,30 +51,44 @@ static int tegra_aconnect_probe(struct platform_device *pdev)
dev_info(&pdev->dev, "Tegra ACONNECT bus registered\n");
return 0;
-
-clk_destroy:
- pm_clk_destroy(&pdev->dev);
-
- return ret;
}
static int tegra_aconnect_remove(struct platform_device *pdev)
{
pm_runtime_disable(&pdev->dev);
- pm_clk_destroy(&pdev->dev);
-
return 0;
}
static int tegra_aconnect_runtime_resume(struct device *dev)
{
- return pm_clk_resume(dev);
+ struct tegra_aconnect *aconnect = dev_get_drvdata(dev);
+ int ret;
+
+ ret = clk_prepare_enable(aconnect->ape_clk);
+ if (ret) {
+ dev_err(dev, "ape clk_enable failed: %d\n", ret);
+ return ret;
+ }
+
+ ret = clk_prepare_enable(aconnect->apb2ape_clk);
+ if (ret) {
+ clk_disable_unprepare(aconnect->ape_clk);
+ dev_err(dev, "apb2ape clk_enable failed: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
}
static int tegra_aconnect_runtime_suspend(struct device *dev)
{
- return pm_clk_suspend(dev);
+ struct tegra_aconnect *aconnect = dev_get_drvdata(dev);
+
+ clk_disable_unprepare(aconnect->ape_clk);
+ clk_disable_unprepare(aconnect->apb2ape_clk);
+
+ return 0;
}
static const struct dev_pm_ops tegra_aconnect_pm_ops = {
--
2.7.4
pm_runtime_force_suspend() and pm_runtime_force_resume() are used as system
sleep noirq suspend and resume callbacks. If the driver is active till late
suspend, where runtime PM cannot run, force suspend is essential for the
device. This makes sure that the device is put into low power state during
system wide PM transitions to sleep states.
Signed-off-by: Sameer Pujar <[email protected]>
---
drivers/bus/tegra-aconnect.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/bus/tegra-aconnect.c b/drivers/bus/tegra-aconnect.c
index 9349157..ac58142 100644
--- a/drivers/bus/tegra-aconnect.c
+++ b/drivers/bus/tegra-aconnect.c
@@ -94,6 +94,8 @@ static int tegra_aconnect_runtime_suspend(struct device *dev)
static const struct dev_pm_ops tegra_aconnect_pm_ops = {
SET_RUNTIME_PM_OPS(tegra_aconnect_runtime_suspend,
tegra_aconnect_runtime_resume, NULL)
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
};
static const struct of_device_id tegra_aconnect_of_match[] = {
--
2.7.4
On 13/03/2019 04:41, Sameer Pujar wrote:
> pm_runtime_force_suspend() and pm_runtime_force_resume() are used as system
> sleep noirq suspend and resume callbacks. If the driver is active till late
> suspend, where runtime PM cannot run, force suspend is essential for the
> device. This makes sure that the device is put into low power state during
> system wide PM transitions to sleep states.
>
> Signed-off-by: Sameer Pujar <[email protected]>
> ---
> drivers/bus/tegra-aconnect.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/bus/tegra-aconnect.c b/drivers/bus/tegra-aconnect.c
> index 9349157..ac58142 100644
> --- a/drivers/bus/tegra-aconnect.c
> +++ b/drivers/bus/tegra-aconnect.c
> @@ -94,6 +94,8 @@ static int tegra_aconnect_runtime_suspend(struct device *dev)
> static const struct dev_pm_ops tegra_aconnect_pm_ops = {
> SET_RUNTIME_PM_OPS(tegra_aconnect_runtime_suspend,
> tegra_aconnect_runtime_resume, NULL)
> + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> };
>
> static const struct of_device_id tegra_aconnect_of_match[] = {
Acked-by: Jon Hunter <[email protected]>
Cheers
Jon
--
nvpublic
Hi Reviewers,
Request for review and approval.
Thanks,
Sameer.
On 3/13/2019 10:11 AM, Sameer Pujar wrote:
> aconnect bus driver is using pm_clk_*() interface for managing clocks.
> With this, clocks seem to be always ON. 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 calls to
> BPMP are always blocking. When pm_clk_*() interface is used on such
> Tegra devices, clock prepare count is not balanced till driver remove()
> gets executed and hence clocks are seen ON always. Thus this patch
> replaces pm_clk_*() with devm_clk_*() framework.
>
> Suggested-by: Mohan Kumar D <[email protected]>
> Reviewed-by: Jonathan Hunter <[email protected]>
> Signed-off-by: Sameer Pujar <[email protected]>
> ---
> drivers/bus/tegra-aconnect.c | 64 ++++++++++++++++++++++++++++++--------------
> 1 file changed, 44 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/bus/tegra-aconnect.c b/drivers/bus/tegra-aconnect.c
> index 084ae28..9349157 100644
> --- a/drivers/bus/tegra-aconnect.c
> +++ b/drivers/bus/tegra-aconnect.c
> @@ -12,28 +12,38 @@
> #include <linux/module.h>
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> -#include <linux/pm_clock.h>
> #include <linux/pm_runtime.h>
>
> +struct tegra_aconnect {
> + struct clk *ape_clk;
> + struct clk *apb2ape_clk;
> +};
> +
> static int tegra_aconnect_probe(struct platform_device *pdev)
> {
> - int ret;
> + struct tegra_aconnect *aconnect;
>
> if (!pdev->dev.of_node)
> return -EINVAL;
>
> - ret = pm_clk_create(&pdev->dev);
> - if (ret)
> - return ret;
> + aconnect = devm_kzalloc(&pdev->dev, sizeof(struct tegra_aconnect),
> + GFP_KERNEL);
> + if (!aconnect)
> + return -ENOMEM;
>
> - ret = of_pm_clk_add_clk(&pdev->dev, "ape");
> - if (ret)
> - goto clk_destroy;
> + aconnect->ape_clk = devm_clk_get(&pdev->dev, "ape");
> + if (IS_ERR(aconnect->ape_clk)) {
> + dev_err(&pdev->dev, "Can't retrieve ape clock\n");
> + return PTR_ERR(aconnect->ape_clk);
> + }
>
> - ret = of_pm_clk_add_clk(&pdev->dev, "apb2ape");
> - if (ret)
> - goto clk_destroy;
> + aconnect->apb2ape_clk = devm_clk_get(&pdev->dev, "apb2ape");
> + if (IS_ERR(aconnect->apb2ape_clk)) {
> + dev_err(&pdev->dev, "Can't retrieve apb2ape clock\n");
> + return PTR_ERR(aconnect->apb2ape_clk);
> + }
>
> + dev_set_drvdata(&pdev->dev, aconnect);
> pm_runtime_enable(&pdev->dev);
>
> of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
> @@ -41,30 +51,44 @@ static int tegra_aconnect_probe(struct platform_device *pdev)
> dev_info(&pdev->dev, "Tegra ACONNECT bus registered\n");
>
> return 0;
> -
> -clk_destroy:
> - pm_clk_destroy(&pdev->dev);
> -
> - return ret;
> }
>
> static int tegra_aconnect_remove(struct platform_device *pdev)
> {
> pm_runtime_disable(&pdev->dev);
>
> - pm_clk_destroy(&pdev->dev);
> -
> return 0;
> }
>
> static int tegra_aconnect_runtime_resume(struct device *dev)
> {
> - return pm_clk_resume(dev);
> + struct tegra_aconnect *aconnect = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = clk_prepare_enable(aconnect->ape_clk);
> + if (ret) {
> + dev_err(dev, "ape clk_enable failed: %d\n", ret);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(aconnect->apb2ape_clk);
> + if (ret) {
> + clk_disable_unprepare(aconnect->ape_clk);
> + dev_err(dev, "apb2ape clk_enable failed: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> }
>
> static int tegra_aconnect_runtime_suspend(struct device *dev)
> {
> - return pm_clk_suspend(dev);
> + struct tegra_aconnect *aconnect = dev_get_drvdata(dev);
> +
> + clk_disable_unprepare(aconnect->ape_clk);
> + clk_disable_unprepare(aconnect->apb2ape_clk);
> +
> + return 0;
> }
>
> static const struct dev_pm_ops tegra_aconnect_pm_ops = {
On 27/03/2019 11:10, Sameer Pujar wrote:
> Hi Reviewers,
>
> Request for review and approval.
Fine with me. I think you have my reviewed/acked-by.
If there are any changes we need to make, we could also use the clk_bulk
APIs here too. However, not critical.
Cheers
Jon
--
nvpublic
On Wed, Mar 13, 2019 at 10:11:58AM +0530, Sameer Pujar wrote:
> aconnect bus driver is using pm_clk_*() interface for managing clocks.
> With this, clocks seem to be always ON. 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 calls to
> BPMP are always blocking. When pm_clk_*() interface is used on such
> Tegra devices, clock prepare count is not balanced till driver remove()
> gets executed and hence clocks are seen ON always. Thus this patch
> replaces pm_clk_*() with devm_clk_*() framework.
>
> Suggested-by: Mohan Kumar D <[email protected]>
> Reviewed-by: Jonathan Hunter <[email protected]>
> Signed-off-by: Sameer Pujar <[email protected]>
> ---
> drivers/bus/tegra-aconnect.c | 64 ++++++++++++++++++++++++++++++--------------
> 1 file changed, 44 insertions(+), 20 deletions(-)
Both patches applied to for-5.2/bus, thanks.
Thierry