| Message ID | 20220515064126.1424-7-linux.amoon@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Exynos Thermal code inprovement | expand |
On 15/05/2022 08:41, Anand Moon wrote:
> Add runtime power management for exynos thermal driver.
First of all - why? Second, I do not see it being added. Where are the
runtime callbacks?
Best regards,
Krzysztof
Hi Krzysztof, On Sun, 15 May 2022 at 15:18, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 15/05/2022 08:41, Anand Moon wrote: > > Add runtime power management for exynos thermal driver. > > First of all - why? Second, I do not see it being added. Where are the > runtime callbacks? > To control runtime control PMU, did I miss something? I looked into imx thermal driver # drivers/thermal/imx_thermal.c to enable run-time power management for exynos driver. > > Best regards, > Krzysztof Thanks & Regards -Anand
On 17/05/2022 20:45, Anand Moon wrote: > Hi Krzysztof, > > On Sun, 15 May 2022 at 15:18, Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 15/05/2022 08:41, Anand Moon wrote: >>> Add runtime power management for exynos thermal driver. >> >> First of all - why? Second, I do not see it being added. Where are the >> runtime callbacks? >> > > To control runtime control PMU, did I miss something? Controlling runtime PM by itself is not a goal. What does it change if it is enabled? > I looked into imx thermal driver # drivers/thermal/imx_thermal.c > to enable run-time power management for exynos driver. So you have runtime PM enabled and then what happens? Where is the power saving? Since you did not implement the callbacks, all this should be explained in commit msg. Best regards, Krzysztof
Hi Krzysztof, On Wed, 18 May 2022 at 12:49, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 17/05/2022 20:45, Anand Moon wrote: > > Hi Krzysztof, > > > > On Sun, 15 May 2022 at 15:18, Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> > >> On 15/05/2022 08:41, Anand Moon wrote: > >>> Add runtime power management for exynos thermal driver. > >> > >> First of all - why? Second, I do not see it being added. Where are the > >> runtime callbacks? > >> > > > > To control runtime control PMU, did I miss something? > > Controlling runtime PM by itself is not a goal. What does it change if > it is enabled? > It means we could have efficient power management for this driver. as per my understanding, it controls runtime sleep and improves power efficiency > > I looked into imx thermal driver # drivers/thermal/imx_thermal.c > > to enable run-time power management for exynos driver. > > So you have runtime PM enabled and then what happens? Where is the power > saving? Since you did not implement the callbacks, all this should be > explained in commit msg. > Ok, As per the original code, it just registers the SIMPLE_DEV_PM_OPS with .pm = &exynos_tmu_pm So I have made sure that suspend resume feature works correctly with these changes on SBC Odroid U3 and XU4. I will try to look into setting RUNTIME_PM_OPS or use UNIVERSAL_DEV_PM_OPS instead of SIMPLE_DEV_PM_OPS any thought on this? > > Best regards, > Krzysztof Thanks and Regards -Anand
On 21/05/2022 11:52, Anand Moon wrote: > Hi Krzysztof, > > On Wed, 18 May 2022 at 12:49, Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 17/05/2022 20:45, Anand Moon wrote: >>> Hi Krzysztof, >>> >>> On Sun, 15 May 2022 at 15:18, Krzysztof Kozlowski >>> <krzysztof.kozlowski@linaro.org> wrote: >>>> >>>> On 15/05/2022 08:41, Anand Moon wrote: >>>>> Add runtime power management for exynos thermal driver. >>>> >>>> First of all - why? Second, I do not see it being added. Where are the >>>> runtime callbacks? >>>> >>> >>> To control runtime control PMU, did I miss something? >> >> Controlling runtime PM by itself is not a goal. What does it change if >> it is enabled? >> > It means we could have efficient power management for this driver. > as per my understanding, it controls runtime sleep and improves power efficiency How? I asked - what is being changed after enabling PM - and you answered without any specifics. Where exactly is the power saving? Please be specific, very specific. > >>> I looked into imx thermal driver # drivers/thermal/imx_thermal.c >>> to enable run-time power management for exynos driver. >> >> So you have runtime PM enabled and then what happens? Where is the power >> saving? Since you did not implement the callbacks, all this should be >> explained in commit msg. >> > Ok, As per the original code, it just registers the SIMPLE_DEV_PM_OPS > with .pm = &exynos_tmu_pm And does nothing else, right? No benefits? > So I have made sure that suspend resume feature works correctly > with these changes on SBC Odroid U3 and XU4. How is suspend/resume related to runtime PM? Are you talking about system suspend? What do you mean now? > > I will try to look into setting RUNTIME_PM_OPS > or use UNIVERSAL_DEV_PM_OPS instead of SIMPLE_DEV_PM_OPS > any thought on this? Why looking at them? You avoid giving any specific answer, so we are repeating the same and the same. Just to be sure - maybe I don't see the obvious stuff, so please explain also this obvious things. Please come with specifics, because otherwise I see it as a waste of our time. Best regards, Krzysztof
diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c index f8a527f19383..be9b98caf2ba 100644 --- a/drivers/thermal/samsung/exynos_tmu.c +++ b/drivers/thermal/samsung/exynos_tmu.c @@ -20,6 +20,7 @@ #include <linux/of_irq.h> #include <linux/platform_device.h> #include <linux/regulator/consumer.h> +#include <linux/pm_runtime.h> #include <dt-bindings/thermal/thermal_exynos.h> @@ -1106,6 +1107,15 @@ static int exynos_tmu_probe(struct platform_device *pdev) goto err_thermal; } + pm_runtime_set_active(&pdev->dev); + pm_runtime_set_autosuspend_delay(&pdev->dev, 1000); + pm_runtime_use_autosuspend(&pdev->dev); + pm_runtime_enable(&pdev->dev); + + ret = pm_runtime_resume_and_get(&pdev->dev); + if (ret < 0) + goto disable_runtime_pm; + ret = devm_request_irq(&pdev->dev, data->irq, exynos_tmu_irq, IRQF_TRIGGER_RISING | IRQF_SHARED, dev_name(&pdev->dev), data); if (ret) { @@ -1113,11 +1123,16 @@ static int exynos_tmu_probe(struct platform_device *pdev) goto err_thermal; } + pm_runtime_put(&pdev->dev); + exynos_tmu_control(pdev, true); return 0; err_thermal: thermal_zone_of_sensor_unregister(&pdev->dev, data->tzd); +disable_runtime_pm: + pm_runtime_put_noidle(&pdev->dev); + pm_runtime_disable(&pdev->dev); err_clk_sec: clk_disable_unprepare(data->clk_sec); err_sclk: @@ -1143,6 +1158,9 @@ static int exynos_tmu_remove(struct platform_device *pdev) clk_disable_unprepare(data->clk); clk_disable_unprepare(data->clk_sec); + pm_runtime_put_noidle(&pdev->dev); + pm_runtime_disable(&pdev->dev); + if (!IS_ERR(data->regulator)) regulator_disable(data->regulator); @@ -1151,18 +1169,25 @@ static int exynos_tmu_remove(struct platform_device *pdev) static int __maybe_unused exynos_tmu_suspend(struct device *dev) { - exynos_tmu_control(to_platform_device(dev), false); + struct platform_device *pdev = to_platform_device(dev); - return 0; + exynos_tmu_control(pdev, false); + + return pm_runtime_force_suspend(&pdev->dev); } static int __maybe_unused exynos_tmu_resume(struct device *dev) { struct platform_device *pdev = to_platform_device(dev); + int ret; exynos_tmu_initialize(pdev); exynos_tmu_control(pdev, true); + ret = pm_runtime_force_resume(&pdev->dev); + if (ret) + return ret; + return 0; }
Add runtime power management for exynos thermal driver. Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- v1: new patch in this series. --- drivers/thermal/samsung/exynos_tmu.c | 29 ++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-)