diff mbox series

[07/12] memory: stm32-fmc2-ebi: add runtime PM support

Message ID 20240212174822.77734-8-christophe.kerello@foss.st.com
State New
Headers show
Series Add MP25 FMC2 support | expand

Commit Message

Christophe Kerello Feb. 12, 2024, 5:48 p.m. UTC
Add runtime PM support in FMC2 ebi driver to be able to manage GENPD
support when it will be enabled.

Signed-off-by: Christophe Kerello <christophe.kerello@foss.st.com>
---
 drivers/memory/stm32-fmc2-ebi.c | 40 ++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 8 deletions(-)

Comments

Krzysztof Kozlowski Feb. 13, 2024, 7:59 a.m. UTC | #1
On 12/02/2024 18:48, Christophe Kerello wrote:
> Add runtime PM support in FMC2 ebi driver to be able to manage GENPD
> support when it will be enabled.

You don't do any real runtime PM here (turning on PM domain on probe is
not real PM), so please explain what is the goal of it and say that it
is basic RPM for keeping domain on.

> 
> Signed-off-by: Christophe Kerello <christophe.kerello@foss.st.com>
> ---
>  drivers/memory/stm32-fmc2-ebi.c | 40 ++++++++++++++++++++++++++-------
>  1 file changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/memory/stm32-fmc2-ebi.c b/drivers/memory/stm32-fmc2-ebi.c
> index 04248c15832f..8c30e56be3b0 100644
> --- a/drivers/memory/stm32-fmc2-ebi.c
> +++ b/drivers/memory/stm32-fmc2-ebi.c
> @@ -11,6 +11,7 @@
>  #include <linux/of_platform.h>
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
>  #include <linux/reset.h>
>  
> @@ -1381,6 +1382,7 @@ static int stm32_fmc2_ebi_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	ebi->dev = dev;
> +	platform_set_drvdata(pdev, ebi);

Does not look related.


Best regards,
Krzysztof
Christophe Kerello Feb. 13, 2024, 1:31 p.m. UTC | #2
On 2/13/24 08:59, Krzysztof Kozlowski wrote:
> On 12/02/2024 18:48, Christophe Kerello wrote:
>> Add runtime PM support in FMC2 ebi driver to be able to manage GENPD
>> support when it will be enabled.
> 

Hi Krzysztof,

> You don't do any real runtime PM here (turning on PM domain on probe is
> not real PM), so please explain what is the goal of it and say that it
> is basic RPM for keeping domain on.
> 

Yes, you are true, I will modify the commit message.

>>
>> Signed-off-by: Christophe Kerello <christophe.kerello@foss.st.com>
>> ---
>>   drivers/memory/stm32-fmc2-ebi.c | 40 ++++++++++++++++++++++++++-------
>>   1 file changed, 32 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/memory/stm32-fmc2-ebi.c b/drivers/memory/stm32-fmc2-ebi.c
>> index 04248c15832f..8c30e56be3b0 100644
>> --- a/drivers/memory/stm32-fmc2-ebi.c
>> +++ b/drivers/memory/stm32-fmc2-ebi.c
>> @@ -11,6 +11,7 @@
>>   #include <linux/of_platform.h>
>>   #include <linux/pinctrl/consumer.h>
>>   #include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>>   #include <linux/regmap.h>
>>   #include <linux/reset.h>
>>   
>> @@ -1381,6 +1382,7 @@ static int stm32_fmc2_ebi_probe(struct platform_device *pdev)
>>   		return -ENOMEM;
>>   
>>   	ebi->dev = dev;
>> +	platform_set_drvdata(pdev, ebi);
> 
> Does not look related.
> 

With pm_runtime_resume_and_get API now called, 
stm32_fmc2_ebi_runtime_resume needs ebi data to enable the clock. It
means that the platform data has to be set before this call.

Regards,
Christophe Kerello.

> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Feb. 13, 2024, 1:33 p.m. UTC | #3
On 13/02/2024 14:31, Christophe Kerello wrote:
>>>   
>>> @@ -1381,6 +1382,7 @@ static int stm32_fmc2_ebi_probe(struct platform_device *pdev)
>>>   		return -ENOMEM;
>>>   
>>>   	ebi->dev = dev;
>>> +	platform_set_drvdata(pdev, ebi);
>>
>> Does not look related.
>>
> 
> With pm_runtime_resume_and_get API now called, 
> stm32_fmc2_ebi_runtime_resume needs ebi data to enable the clock. It
> means that the platform data has to be set before this call.
> 

Ah, OK.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/memory/stm32-fmc2-ebi.c b/drivers/memory/stm32-fmc2-ebi.c
index 04248c15832f..8c30e56be3b0 100644
--- a/drivers/memory/stm32-fmc2-ebi.c
+++ b/drivers/memory/stm32-fmc2-ebi.c
@@ -11,6 +11,7 @@ 
 #include <linux/of_platform.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/reset.h>
 
@@ -1381,6 +1382,7 @@  static int stm32_fmc2_ebi_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	ebi->dev = dev;
+	platform_set_drvdata(pdev, ebi);
 
 	ebi->data = of_device_get_match_data(dev);
 	if (!ebi->data)
@@ -1398,10 +1400,14 @@  static int stm32_fmc2_ebi_probe(struct platform_device *pdev)
 	if (PTR_ERR(rstc) == -EPROBE_DEFER)
 		return -EPROBE_DEFER;
 
-	ret = clk_prepare_enable(ebi->clk);
+	ret = devm_pm_runtime_enable(dev);
 	if (ret)
 		return ret;
 
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0)
+		return ret;
+
 	if (!IS_ERR(rstc)) {
 		reset_control_assert(rstc);
 		reset_control_deassert(rstc);
@@ -1432,7 +1438,6 @@  static int stm32_fmc2_ebi_probe(struct platform_device *pdev)
 		goto err_release;
 
 	stm32_fmc2_ebi_save_setup(ebi);
-	platform_set_drvdata(pdev, ebi);
 
 	return 0;
 
@@ -1440,7 +1445,7 @@  static int stm32_fmc2_ebi_probe(struct platform_device *pdev)
 	stm32_fmc2_ebi_disable_banks(ebi);
 	stm32_fmc2_ebi_disable(ebi);
 	stm32_fmc2_ebi_put_sems(ebi);
-	clk_disable_unprepare(ebi->clk);
+	pm_runtime_put_sync_suspend(dev);
 
 	return ret;
 }
@@ -1453,7 +1458,23 @@  static void stm32_fmc2_ebi_remove(struct platform_device *pdev)
 	stm32_fmc2_ebi_disable_banks(ebi);
 	stm32_fmc2_ebi_disable(ebi);
 	stm32_fmc2_ebi_put_sems(ebi);
+	pm_runtime_put_sync_suspend(&pdev->dev);
+}
+
+static int __maybe_unused stm32_fmc2_ebi_runtime_suspend(struct device *dev)
+{
+	struct stm32_fmc2_ebi *ebi = dev_get_drvdata(dev);
+
 	clk_disable_unprepare(ebi->clk);
+
+	return 0;
+}
+
+static int __maybe_unused stm32_fmc2_ebi_runtime_resume(struct device *dev)
+{
+	struct stm32_fmc2_ebi *ebi = dev_get_drvdata(dev);
+
+	return clk_prepare_enable(ebi->clk);
 }
 
 static int __maybe_unused stm32_fmc2_ebi_suspend(struct device *dev)
@@ -1462,7 +1483,7 @@  static int __maybe_unused stm32_fmc2_ebi_suspend(struct device *dev)
 
 	stm32_fmc2_ebi_disable(ebi);
 	stm32_fmc2_ebi_put_sems(ebi);
-	clk_disable_unprepare(ebi->clk);
+	pm_runtime_put_sync_suspend(dev);
 	pinctrl_pm_select_sleep_state(dev);
 
 	return 0;
@@ -1475,8 +1496,8 @@  static int __maybe_unused stm32_fmc2_ebi_resume(struct device *dev)
 
 	pinctrl_pm_select_default_state(dev);
 
-	ret = clk_prepare_enable(ebi->clk);
-	if (ret)
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0)
 		return ret;
 
 	stm32_fmc2_ebi_get_sems(ebi);
@@ -1486,8 +1507,11 @@  static int __maybe_unused stm32_fmc2_ebi_resume(struct device *dev)
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(stm32_fmc2_ebi_pm_ops, stm32_fmc2_ebi_suspend,
-			 stm32_fmc2_ebi_resume);
+static const struct dev_pm_ops stm32_fmc2_ebi_pm_ops = {
+	SET_RUNTIME_PM_OPS(stm32_fmc2_ebi_runtime_suspend,
+			   stm32_fmc2_ebi_runtime_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(stm32_fmc2_ebi_suspend, stm32_fmc2_ebi_resume)
+};
 
 static const struct stm32_fmc2_ebi_data stm32_fmc2_ebi_mp1_data = {
 	.rnb_for_nand = false,