linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] bus: tegra-aconnect: use devm_clk_*() helpers
@ 2019-03-13  4:41 Sameer Pujar
  2019-03-13  4:41 ` [PATCH v2 2/2] bus: tegra-aconnect: add system sleep callbacks Sameer Pujar
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Sameer Pujar @ 2019-03-13  4:41 UTC (permalink / raw)
  To: treding, jonathanh; +Cc: linux-tegra, linux-kernel, Sameer Pujar

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 <mkumard@nvidia.com>
Reviewed-by: Jonathan Hunter <jonathanh@nvidia.com>
Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] bus: tegra-aconnect: add system sleep callbacks
  2019-03-13  4:41 [PATCH v2 1/2] bus: tegra-aconnect: use devm_clk_*() helpers Sameer Pujar
@ 2019-03-13  4:41 ` Sameer Pujar
  2019-03-13 10:30   ` Jon Hunter
  2019-03-27 11:10 ` [PATCH v2 1/2] bus: tegra-aconnect: use devm_clk_*() helpers Sameer Pujar
  2019-03-28 16:32 ` Thierry Reding
  2 siblings, 1 reply; 6+ messages in thread
From: Sameer Pujar @ 2019-03-13  4:41 UTC (permalink / raw)
  To: treding, jonathanh; +Cc: linux-tegra, linux-kernel, Sameer Pujar

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 <spujar@nvidia.com>
---
 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] bus: tegra-aconnect: add system sleep callbacks
  2019-03-13  4:41 ` [PATCH v2 2/2] bus: tegra-aconnect: add system sleep callbacks Sameer Pujar
@ 2019-03-13 10:30   ` Jon Hunter
  0 siblings, 0 replies; 6+ messages in thread
From: Jon Hunter @ 2019-03-13 10:30 UTC (permalink / raw)
  To: Sameer Pujar, treding; +Cc: linux-tegra, linux-kernel


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 <spujar@nvidia.com>
> ---
>  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 <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] bus: tegra-aconnect: use devm_clk_*() helpers
  2019-03-13  4:41 [PATCH v2 1/2] bus: tegra-aconnect: use devm_clk_*() helpers Sameer Pujar
  2019-03-13  4:41 ` [PATCH v2 2/2] bus: tegra-aconnect: add system sleep callbacks Sameer Pujar
@ 2019-03-27 11:10 ` Sameer Pujar
  2019-03-27 13:18   ` Jon Hunter
  2019-03-28 16:32 ` Thierry Reding
  2 siblings, 1 reply; 6+ messages in thread
From: Sameer Pujar @ 2019-03-27 11:10 UTC (permalink / raw)
  To: treding, jonathanh; +Cc: linux-tegra, linux-kernel

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 <mkumard@nvidia.com>
> Reviewed-by: Jonathan Hunter <jonathanh@nvidia.com>
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> ---
>   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 = {

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] bus: tegra-aconnect: use devm_clk_*() helpers
  2019-03-27 11:10 ` [PATCH v2 1/2] bus: tegra-aconnect: use devm_clk_*() helpers Sameer Pujar
@ 2019-03-27 13:18   ` Jon Hunter
  0 siblings, 0 replies; 6+ messages in thread
From: Jon Hunter @ 2019-03-27 13:18 UTC (permalink / raw)
  To: Sameer Pujar, treding; +Cc: linux-tegra, linux-kernel


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] bus: tegra-aconnect: use devm_clk_*() helpers
  2019-03-13  4:41 [PATCH v2 1/2] bus: tegra-aconnect: use devm_clk_*() helpers Sameer Pujar
  2019-03-13  4:41 ` [PATCH v2 2/2] bus: tegra-aconnect: add system sleep callbacks Sameer Pujar
  2019-03-27 11:10 ` [PATCH v2 1/2] bus: tegra-aconnect: use devm_clk_*() helpers Sameer Pujar
@ 2019-03-28 16:32 ` Thierry Reding
  2 siblings, 0 replies; 6+ messages in thread
From: Thierry Reding @ 2019-03-28 16:32 UTC (permalink / raw)
  To: Sameer Pujar; +Cc: treding, jonathanh, linux-tegra, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 986 bytes --]

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 <mkumard@nvidia.com>
> Reviewed-by: Jonathan Hunter <jonathanh@nvidia.com>
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> ---
>  drivers/bus/tegra-aconnect.c | 64 ++++++++++++++++++++++++++++++--------------
>  1 file changed, 44 insertions(+), 20 deletions(-)

Both patches applied to for-5.2/bus, thanks.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-03-28 16:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-13  4:41 [PATCH v2 1/2] bus: tegra-aconnect: use devm_clk_*() helpers Sameer Pujar
2019-03-13  4:41 ` [PATCH v2 2/2] bus: tegra-aconnect: add system sleep callbacks Sameer Pujar
2019-03-13 10:30   ` Jon Hunter
2019-03-27 11:10 ` [PATCH v2 1/2] bus: tegra-aconnect: use devm_clk_*() helpers Sameer Pujar
2019-03-27 13:18   ` Jon Hunter
2019-03-28 16:32 ` Thierry Reding

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).