* [PATCH v3] thermal: imx: implement runtime PM support
@ 2021-11-17 10:34 Oleksij Rempel
2021-11-17 10:37 ` Oleksij Rempel
2022-02-17 4:41 ` [PATCH] thermal/drivers/imx: add missing pm_runtime_put on error path Dominique Martinet
0 siblings, 2 replies; 6+ messages in thread
From: Oleksij Rempel @ 2021-11-17 10:34 UTC (permalink / raw)
To: Shawn Guo, Sascha Hauer
Cc: Oleksij Rempel, Amit Kucheria, Andrzej Pietrasiewicz,
Daniel Lezcano, Fabio Estevam, NXP Linux Team,
Pengutronix Kernel Team, Zhang Rui, linux-arm-kernel,
linux-kernel, linux-pm, David Jander, Petr Benes,
Michal Vokáč
Starting with commit d92ed2c9d3ff ("thermal: imx: Use driver's local
data to decide whether to run a measurement") this driver stared using
irq_enabled flag to make decision to power on/off the thermal core. This
triggered a regression, where after reaching critical temperature, alarm
IRQ handler set irq_enabled to false, disabled thermal core and was not
able read temperature and disable cooling sequence.
In case the cooling device is "CPU/GPU freq", the system will run with
reduce performance until next reboot.
To solve this issue, we need to move all parts implementing hand made
runtime power management and let it handle actual runtime PM framework.
Fixes: d92ed2c9d3ff ("thermal: imx: Use driver's local data to decide whether to run a measurement")
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
drivers/thermal/imx_thermal.c | 145 +++++++++++++++++++++-------------
1 file changed, 91 insertions(+), 54 deletions(-)
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 2c7473d86a59..16663373b682 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -15,6 +15,7 @@
#include <linux/regmap.h>
#include <linux/thermal.h>
#include <linux/nvmem-consumer.h>
+#include <linux/pm_runtime.h>
#define REG_SET 0x4
#define REG_CLR 0x8
@@ -194,6 +195,7 @@ static struct thermal_soc_data thermal_imx7d_data = {
};
struct imx_thermal_data {
+ struct device *dev;
struct cpufreq_policy *policy;
struct thermal_zone_device *tz;
struct thermal_cooling_device *cdev;
@@ -252,44 +254,15 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
const struct thermal_soc_data *soc_data = data->socdata;
struct regmap *map = data->tempmon;
unsigned int n_meas;
- bool wait, run_measurement;
u32 val;
+ int ret;
- run_measurement = !data->irq_enabled;
- if (!run_measurement) {
- /* Check if a measurement is currently in progress */
- regmap_read(map, soc_data->temp_data, &val);
- wait = !(val & soc_data->temp_valid_mask);
- } else {
- /*
- * Every time we measure the temperature, we will power on the
- * temperature sensor, enable measurements, take a reading,
- * disable measurements, power off the temperature sensor.
- */
- regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
- soc_data->power_down_mask);
- regmap_write(map, soc_data->sensor_ctrl + REG_SET,
- soc_data->measure_temp_mask);
-
- wait = true;
- }
-
- /*
- * According to the temp sensor designers, it may require up to ~17us
- * to complete a measurement.
- */
- if (wait)
- usleep_range(20, 50);
+ ret = pm_runtime_resume_and_get(data->dev);
+ if (ret < 0)
+ return ret;
regmap_read(map, soc_data->temp_data, &val);
- if (run_measurement) {
- regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
- soc_data->measure_temp_mask);
- regmap_write(map, soc_data->sensor_ctrl + REG_SET,
- soc_data->power_down_mask);
- }
-
if ((val & soc_data->temp_valid_mask) == 0) {
dev_dbg(&tz->device, "temp measurement never finished\n");
return -EAGAIN;
@@ -328,6 +301,8 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
enable_irq(data->irq);
}
+ pm_runtime_put(data->dev);
+
return 0;
}
@@ -335,24 +310,16 @@ static int imx_change_mode(struct thermal_zone_device *tz,
enum thermal_device_mode mode)
{
struct imx_thermal_data *data = tz->devdata;
- struct regmap *map = data->tempmon;
- const struct thermal_soc_data *soc_data = data->socdata;
if (mode == THERMAL_DEVICE_ENABLED) {
- regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
- soc_data->power_down_mask);
- regmap_write(map, soc_data->sensor_ctrl + REG_SET,
- soc_data->measure_temp_mask);
+ pm_runtime_get(data->dev);
if (!data->irq_enabled) {
data->irq_enabled = true;
enable_irq(data->irq);
}
} else {
- regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
- soc_data->measure_temp_mask);
- regmap_write(map, soc_data->sensor_ctrl + REG_SET,
- soc_data->power_down_mask);
+ pm_runtime_put(data->dev);
if (data->irq_enabled) {
disable_irq(data->irq);
@@ -393,6 +360,11 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
int temp)
{
struct imx_thermal_data *data = tz->devdata;
+ int ret;
+
+ ret = pm_runtime_resume_and_get(data->dev);
+ if (ret < 0)
+ return ret;
/* do not allow changing critical threshold */
if (trip == IMX_TRIP_CRITICAL)
@@ -406,6 +378,8 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
imx_set_alarm_temp(data, temp);
+ pm_runtime_put(data->dev);
+
return 0;
}
@@ -681,6 +655,8 @@ static int imx_thermal_probe(struct platform_device *pdev)
if (!data)
return -ENOMEM;
+ data->dev = &pdev->dev;
+
map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "fsl,tempmon");
if (IS_ERR(map)) {
ret = PTR_ERR(map);
@@ -800,6 +776,16 @@ static int imx_thermal_probe(struct platform_device *pdev)
data->socdata->power_down_mask);
regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
data->socdata->measure_temp_mask);
+ /* After power up, we need a delay before first access can be done. */
+ usleep_range(20, 50);
+
+ /* the core was configured and enabled just before */
+ pm_runtime_set_active(&pdev->dev);
+ pm_runtime_enable(data->dev);
+
+ ret = pm_runtime_resume_and_get(data->dev);
+ if (ret < 0)
+ goto disable_runtime_pm;
data->irq_enabled = true;
ret = thermal_zone_device_enable(data->tz);
@@ -814,10 +800,15 @@ static int imx_thermal_probe(struct platform_device *pdev)
goto thermal_zone_unregister;
}
+ pm_runtime_put(data->dev);
+
return 0;
thermal_zone_unregister:
thermal_zone_device_unregister(data->tz);
+disable_runtime_pm:
+ pm_runtime_put_noidle(data->dev);
+ pm_runtime_disable(data->dev);
clk_disable:
clk_disable_unprepare(data->thermal_clk);
legacy_cleanup:
@@ -829,13 +820,9 @@ static int imx_thermal_probe(struct platform_device *pdev)
static int imx_thermal_remove(struct platform_device *pdev)
{
struct imx_thermal_data *data = platform_get_drvdata(pdev);
- struct regmap *map = data->tempmon;
- /* Disable measurements */
- regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
- data->socdata->power_down_mask);
- if (!IS_ERR(data->thermal_clk))
- clk_disable_unprepare(data->thermal_clk);
+ pm_runtime_put_noidle(data->dev);
+ pm_runtime_disable(data->dev);
thermal_zone_device_unregister(data->tz);
imx_thermal_unregister_legacy_cooling(data);
@@ -858,29 +845,79 @@ static int __maybe_unused imx_thermal_suspend(struct device *dev)
ret = thermal_zone_device_disable(data->tz);
if (ret)
return ret;
+
+ return pm_runtime_force_suspend(data->dev);
+}
+
+static int __maybe_unused imx_thermal_resume(struct device *dev)
+{
+ struct imx_thermal_data *data = dev_get_drvdata(dev);
+ int ret;
+
+ ret = pm_runtime_force_resume(data->dev);
+ if (ret)
+ return ret;
+ /* Enabled thermal sensor after resume */
+ return thermal_zone_device_enable(data->tz);
+}
+
+static int __maybe_unused imx_thermal_runtime_suspend(struct device *dev)
+{
+ struct imx_thermal_data *data = dev_get_drvdata(dev);
+ const struct thermal_soc_data *socdata = data->socdata;
+ struct regmap *map = data->tempmon;
+ int ret;
+
+ ret = regmap_write(map, socdata->sensor_ctrl + REG_CLR,
+ socdata->measure_temp_mask);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(map, socdata->sensor_ctrl + REG_SET,
+ socdata->power_down_mask);
+ if (ret)
+ return ret;
+
clk_disable_unprepare(data->thermal_clk);
return 0;
}
-static int __maybe_unused imx_thermal_resume(struct device *dev)
+static int __maybe_unused imx_thermal_runtime_resume(struct device *dev)
{
struct imx_thermal_data *data = dev_get_drvdata(dev);
+ const struct thermal_soc_data *socdata = data->socdata;
+ struct regmap *map = data->tempmon;
int ret;
ret = clk_prepare_enable(data->thermal_clk);
if (ret)
return ret;
- /* Enabled thermal sensor after resume */
- ret = thermal_zone_device_enable(data->tz);
+
+ ret = regmap_write(map, socdata->sensor_ctrl + REG_CLR,
+ socdata->power_down_mask);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(map, socdata->sensor_ctrl + REG_SET,
+ socdata->measure_temp_mask);
if (ret)
return ret;
+ /*
+ * According to the temp sensor designers, it may require up to ~17us
+ * to complete a measurement.
+ */
+ usleep_range(20, 50);
+
return 0;
}
-static SIMPLE_DEV_PM_OPS(imx_thermal_pm_ops,
- imx_thermal_suspend, imx_thermal_resume);
+static const struct dev_pm_ops imx_thermal_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(imx_thermal_suspend, imx_thermal_resume)
+ SET_RUNTIME_PM_OPS(imx_thermal_runtime_suspend,
+ imx_thermal_runtime_resume, NULL)
+};
static struct platform_driver imx_thermal = {
.driver = {
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] thermal: imx: implement runtime PM support
2021-11-17 10:34 [PATCH v3] thermal: imx: implement runtime PM support Oleksij Rempel
@ 2021-11-17 10:37 ` Oleksij Rempel
2021-11-18 10:05 ` Petr Benes
2022-02-17 4:41 ` [PATCH] thermal/drivers/imx: add missing pm_runtime_put on error path Dominique Martinet
1 sibling, 1 reply; 6+ messages in thread
From: Oleksij Rempel @ 2021-11-17 10:37 UTC (permalink / raw)
To: Shawn Guo, Sascha Hauer
Cc: Daniel Lezcano, Michal Vokáč,
linux-pm, Petr Benes, Amit Kucheria, linux-kernel,
Andrzej Pietrasiewicz, NXP Linux Team, Pengutronix Kernel Team,
David Jander, Zhang Rui, Fabio Estevam, linux-arm-kernel
Petr, Michal,
can you please test this patch.
On Wed, Nov 17, 2021 at 11:34:26AM +0100, Oleksij Rempel wrote:
> Starting with commit d92ed2c9d3ff ("thermal: imx: Use driver's local
> data to decide whether to run a measurement") this driver stared using
> irq_enabled flag to make decision to power on/off the thermal core. This
> triggered a regression, where after reaching critical temperature, alarm
> IRQ handler set irq_enabled to false, disabled thermal core and was not
> able read temperature and disable cooling sequence.
>
> In case the cooling device is "CPU/GPU freq", the system will run with
> reduce performance until next reboot.
>
> To solve this issue, we need to move all parts implementing hand made
> runtime power management and let it handle actual runtime PM framework.
>
> Fixes: d92ed2c9d3ff ("thermal: imx: Use driver's local data to decide whether to run a measurement")
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
> drivers/thermal/imx_thermal.c | 145 +++++++++++++++++++++-------------
> 1 file changed, 91 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 2c7473d86a59..16663373b682 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -15,6 +15,7 @@
> #include <linux/regmap.h>
> #include <linux/thermal.h>
> #include <linux/nvmem-consumer.h>
> +#include <linux/pm_runtime.h>
>
> #define REG_SET 0x4
> #define REG_CLR 0x8
> @@ -194,6 +195,7 @@ static struct thermal_soc_data thermal_imx7d_data = {
> };
>
> struct imx_thermal_data {
> + struct device *dev;
> struct cpufreq_policy *policy;
> struct thermal_zone_device *tz;
> struct thermal_cooling_device *cdev;
> @@ -252,44 +254,15 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
> const struct thermal_soc_data *soc_data = data->socdata;
> struct regmap *map = data->tempmon;
> unsigned int n_meas;
> - bool wait, run_measurement;
> u32 val;
> + int ret;
>
> - run_measurement = !data->irq_enabled;
> - if (!run_measurement) {
> - /* Check if a measurement is currently in progress */
> - regmap_read(map, soc_data->temp_data, &val);
> - wait = !(val & soc_data->temp_valid_mask);
> - } else {
> - /*
> - * Every time we measure the temperature, we will power on the
> - * temperature sensor, enable measurements, take a reading,
> - * disable measurements, power off the temperature sensor.
> - */
> - regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
> - soc_data->power_down_mask);
> - regmap_write(map, soc_data->sensor_ctrl + REG_SET,
> - soc_data->measure_temp_mask);
> -
> - wait = true;
> - }
> -
> - /*
> - * According to the temp sensor designers, it may require up to ~17us
> - * to complete a measurement.
> - */
> - if (wait)
> - usleep_range(20, 50);
> + ret = pm_runtime_resume_and_get(data->dev);
> + if (ret < 0)
> + return ret;
>
> regmap_read(map, soc_data->temp_data, &val);
>
> - if (run_measurement) {
> - regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
> - soc_data->measure_temp_mask);
> - regmap_write(map, soc_data->sensor_ctrl + REG_SET,
> - soc_data->power_down_mask);
> - }
> -
> if ((val & soc_data->temp_valid_mask) == 0) {
> dev_dbg(&tz->device, "temp measurement never finished\n");
> return -EAGAIN;
> @@ -328,6 +301,8 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
> enable_irq(data->irq);
> }
>
> + pm_runtime_put(data->dev);
> +
> return 0;
> }
>
> @@ -335,24 +310,16 @@ static int imx_change_mode(struct thermal_zone_device *tz,
> enum thermal_device_mode mode)
> {
> struct imx_thermal_data *data = tz->devdata;
> - struct regmap *map = data->tempmon;
> - const struct thermal_soc_data *soc_data = data->socdata;
>
> if (mode == THERMAL_DEVICE_ENABLED) {
> - regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
> - soc_data->power_down_mask);
> - regmap_write(map, soc_data->sensor_ctrl + REG_SET,
> - soc_data->measure_temp_mask);
> + pm_runtime_get(data->dev);
>
> if (!data->irq_enabled) {
> data->irq_enabled = true;
> enable_irq(data->irq);
> }
> } else {
> - regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
> - soc_data->measure_temp_mask);
> - regmap_write(map, soc_data->sensor_ctrl + REG_SET,
> - soc_data->power_down_mask);
> + pm_runtime_put(data->dev);
>
> if (data->irq_enabled) {
> disable_irq(data->irq);
> @@ -393,6 +360,11 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
> int temp)
> {
> struct imx_thermal_data *data = tz->devdata;
> + int ret;
> +
> + ret = pm_runtime_resume_and_get(data->dev);
> + if (ret < 0)
> + return ret;
>
> /* do not allow changing critical threshold */
> if (trip == IMX_TRIP_CRITICAL)
> @@ -406,6 +378,8 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
>
> imx_set_alarm_temp(data, temp);
>
> + pm_runtime_put(data->dev);
> +
> return 0;
> }
>
> @@ -681,6 +655,8 @@ static int imx_thermal_probe(struct platform_device *pdev)
> if (!data)
> return -ENOMEM;
>
> + data->dev = &pdev->dev;
> +
> map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "fsl,tempmon");
> if (IS_ERR(map)) {
> ret = PTR_ERR(map);
> @@ -800,6 +776,16 @@ static int imx_thermal_probe(struct platform_device *pdev)
> data->socdata->power_down_mask);
> regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
> data->socdata->measure_temp_mask);
> + /* After power up, we need a delay before first access can be done. */
> + usleep_range(20, 50);
> +
> + /* the core was configured and enabled just before */
> + pm_runtime_set_active(&pdev->dev);
> + pm_runtime_enable(data->dev);
> +
> + ret = pm_runtime_resume_and_get(data->dev);
> + if (ret < 0)
> + goto disable_runtime_pm;
>
> data->irq_enabled = true;
> ret = thermal_zone_device_enable(data->tz);
> @@ -814,10 +800,15 @@ static int imx_thermal_probe(struct platform_device *pdev)
> goto thermal_zone_unregister;
> }
>
> + pm_runtime_put(data->dev);
> +
> return 0;
>
> thermal_zone_unregister:
> thermal_zone_device_unregister(data->tz);
> +disable_runtime_pm:
> + pm_runtime_put_noidle(data->dev);
> + pm_runtime_disable(data->dev);
> clk_disable:
> clk_disable_unprepare(data->thermal_clk);
> legacy_cleanup:
> @@ -829,13 +820,9 @@ static int imx_thermal_probe(struct platform_device *pdev)
> static int imx_thermal_remove(struct platform_device *pdev)
> {
> struct imx_thermal_data *data = platform_get_drvdata(pdev);
> - struct regmap *map = data->tempmon;
>
> - /* Disable measurements */
> - regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
> - data->socdata->power_down_mask);
> - if (!IS_ERR(data->thermal_clk))
> - clk_disable_unprepare(data->thermal_clk);
> + pm_runtime_put_noidle(data->dev);
> + pm_runtime_disable(data->dev);
>
> thermal_zone_device_unregister(data->tz);
> imx_thermal_unregister_legacy_cooling(data);
> @@ -858,29 +845,79 @@ static int __maybe_unused imx_thermal_suspend(struct device *dev)
> ret = thermal_zone_device_disable(data->tz);
> if (ret)
> return ret;
> +
> + return pm_runtime_force_suspend(data->dev);
> +}
> +
> +static int __maybe_unused imx_thermal_resume(struct device *dev)
> +{
> + struct imx_thermal_data *data = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = pm_runtime_force_resume(data->dev);
> + if (ret)
> + return ret;
> + /* Enabled thermal sensor after resume */
> + return thermal_zone_device_enable(data->tz);
> +}
> +
> +static int __maybe_unused imx_thermal_runtime_suspend(struct device *dev)
> +{
> + struct imx_thermal_data *data = dev_get_drvdata(dev);
> + const struct thermal_soc_data *socdata = data->socdata;
> + struct regmap *map = data->tempmon;
> + int ret;
> +
> + ret = regmap_write(map, socdata->sensor_ctrl + REG_CLR,
> + socdata->measure_temp_mask);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(map, socdata->sensor_ctrl + REG_SET,
> + socdata->power_down_mask);
> + if (ret)
> + return ret;
> +
> clk_disable_unprepare(data->thermal_clk);
>
> return 0;
> }
>
> -static int __maybe_unused imx_thermal_resume(struct device *dev)
> +static int __maybe_unused imx_thermal_runtime_resume(struct device *dev)
> {
> struct imx_thermal_data *data = dev_get_drvdata(dev);
> + const struct thermal_soc_data *socdata = data->socdata;
> + struct regmap *map = data->tempmon;
> int ret;
>
> ret = clk_prepare_enable(data->thermal_clk);
> if (ret)
> return ret;
> - /* Enabled thermal sensor after resume */
> - ret = thermal_zone_device_enable(data->tz);
> +
> + ret = regmap_write(map, socdata->sensor_ctrl + REG_CLR,
> + socdata->power_down_mask);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(map, socdata->sensor_ctrl + REG_SET,
> + socdata->measure_temp_mask);
> if (ret)
> return ret;
>
> + /*
> + * According to the temp sensor designers, it may require up to ~17us
> + * to complete a measurement.
> + */
> + usleep_range(20, 50);
> +
> return 0;
> }
>
> -static SIMPLE_DEV_PM_OPS(imx_thermal_pm_ops,
> - imx_thermal_suspend, imx_thermal_resume);
> +static const struct dev_pm_ops imx_thermal_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(imx_thermal_suspend, imx_thermal_resume)
> + SET_RUNTIME_PM_OPS(imx_thermal_runtime_suspend,
> + imx_thermal_runtime_resume, NULL)
> +};
>
> static struct platform_driver imx_thermal = {
> .driver = {
> --
> 2.30.2
>
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] thermal: imx: implement runtime PM support
2021-11-17 10:37 ` Oleksij Rempel
@ 2021-11-18 10:05 ` Petr Benes
2021-11-19 9:18 ` Petr Benes
0 siblings, 1 reply; 6+ messages in thread
From: Petr Benes @ 2021-11-18 10:05 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Shawn Guo, Sascha Hauer, Daniel Lezcano, Michal Vokáč,
linux-pm, Amit Kucheria, linux-kernel, Andrzej Pietrasiewicz,
NXP Linux Team, Pengutronix Kernel Team, David Jander, Zhang Rui,
Fabio Estevam, linux-arm-kernel
Works for me! Well done.
On Wed, 17 Nov 2021 at 11:37, Oleksij Rempel <o.rempel@pengutronix.de> wrote:
>
> Petr, Michal,
>
> can you please test this patch.
>
> On Wed, Nov 17, 2021 at 11:34:26AM +0100, Oleksij Rempel wrote:
> > Starting with commit d92ed2c9d3ff ("thermal: imx: Use driver's local
> > data to decide whether to run a measurement") this driver stared using
> > irq_enabled flag to make decision to power on/off the thermal core. This
> > triggered a regression, where after reaching critical temperature, alarm
> > IRQ handler set irq_enabled to false, disabled thermal core and was not
> > able read temperature and disable cooling sequence.
> >
> > In case the cooling device is "CPU/GPU freq", the system will run with
> > reduce performance until next reboot.
> >
> > To solve this issue, we need to move all parts implementing hand made
> > runtime power management and let it handle actual runtime PM framework.
> >
> > Fixes: d92ed2c9d3ff ("thermal: imx: Use driver's local data to decide whether to run a measurement")
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> > drivers/thermal/imx_thermal.c | 145 +++++++++++++++++++++-------------
> > 1 file changed, 91 insertions(+), 54 deletions(-)
> >
> > diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> > index 2c7473d86a59..16663373b682 100644
> > --- a/drivers/thermal/imx_thermal.c
> > +++ b/drivers/thermal/imx_thermal.c
> > @@ -15,6 +15,7 @@
> > #include <linux/regmap.h>
> > #include <linux/thermal.h>
> > #include <linux/nvmem-consumer.h>
> > +#include <linux/pm_runtime.h>
> >
> > #define REG_SET 0x4
> > #define REG_CLR 0x8
> > @@ -194,6 +195,7 @@ static struct thermal_soc_data thermal_imx7d_data = {
> > };
> >
> > struct imx_thermal_data {
> > + struct device *dev;
> > struct cpufreq_policy *policy;
> > struct thermal_zone_device *tz;
> > struct thermal_cooling_device *cdev;
> > @@ -252,44 +254,15 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
> > const struct thermal_soc_data *soc_data = data->socdata;
> > struct regmap *map = data->tempmon;
> > unsigned int n_meas;
> > - bool wait, run_measurement;
> > u32 val;
> > + int ret;
> >
> > - run_measurement = !data->irq_enabled;
> > - if (!run_measurement) {
> > - /* Check if a measurement is currently in progress */
> > - regmap_read(map, soc_data->temp_data, &val);
> > - wait = !(val & soc_data->temp_valid_mask);
> > - } else {
> > - /*
> > - * Every time we measure the temperature, we will power on the
> > - * temperature sensor, enable measurements, take a reading,
> > - * disable measurements, power off the temperature sensor.
> > - */
> > - regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
> > - soc_data->power_down_mask);
> > - regmap_write(map, soc_data->sensor_ctrl + REG_SET,
> > - soc_data->measure_temp_mask);
> > -
> > - wait = true;
> > - }
> > -
> > - /*
> > - * According to the temp sensor designers, it may require up to ~17us
> > - * to complete a measurement.
> > - */
> > - if (wait)
> > - usleep_range(20, 50);
> > + ret = pm_runtime_resume_and_get(data->dev);
> > + if (ret < 0)
> > + return ret;
> >
> > regmap_read(map, soc_data->temp_data, &val);
> >
> > - if (run_measurement) {
> > - regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
> > - soc_data->measure_temp_mask);
> > - regmap_write(map, soc_data->sensor_ctrl + REG_SET,
> > - soc_data->power_down_mask);
> > - }
> > -
> > if ((val & soc_data->temp_valid_mask) == 0) {
> > dev_dbg(&tz->device, "temp measurement never finished\n");
> > return -EAGAIN;
> > @@ -328,6 +301,8 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
> > enable_irq(data->irq);
> > }
> >
> > + pm_runtime_put(data->dev);
> > +
> > return 0;
> > }
> >
> > @@ -335,24 +310,16 @@ static int imx_change_mode(struct thermal_zone_device *tz,
> > enum thermal_device_mode mode)
> > {
> > struct imx_thermal_data *data = tz->devdata;
> > - struct regmap *map = data->tempmon;
> > - const struct thermal_soc_data *soc_data = data->socdata;
> >
> > if (mode == THERMAL_DEVICE_ENABLED) {
> > - regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
> > - soc_data->power_down_mask);
> > - regmap_write(map, soc_data->sensor_ctrl + REG_SET,
> > - soc_data->measure_temp_mask);
> > + pm_runtime_get(data->dev);
> >
> > if (!data->irq_enabled) {
> > data->irq_enabled = true;
> > enable_irq(data->irq);
> > }
> > } else {
> > - regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
> > - soc_data->measure_temp_mask);
> > - regmap_write(map, soc_data->sensor_ctrl + REG_SET,
> > - soc_data->power_down_mask);
> > + pm_runtime_put(data->dev);
> >
> > if (data->irq_enabled) {
> > disable_irq(data->irq);
> > @@ -393,6 +360,11 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
> > int temp)
> > {
> > struct imx_thermal_data *data = tz->devdata;
> > + int ret;
> > +
> > + ret = pm_runtime_resume_and_get(data->dev);
> > + if (ret < 0)
> > + return ret;
> >
> > /* do not allow changing critical threshold */
> > if (trip == IMX_TRIP_CRITICAL)
> > @@ -406,6 +378,8 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
> >
> > imx_set_alarm_temp(data, temp);
> >
> > + pm_runtime_put(data->dev);
> > +
> > return 0;
> > }
> >
> > @@ -681,6 +655,8 @@ static int imx_thermal_probe(struct platform_device *pdev)
> > if (!data)
> > return -ENOMEM;
> >
> > + data->dev = &pdev->dev;
> > +
> > map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "fsl,tempmon");
> > if (IS_ERR(map)) {
> > ret = PTR_ERR(map);
> > @@ -800,6 +776,16 @@ static int imx_thermal_probe(struct platform_device *pdev)
> > data->socdata->power_down_mask);
> > regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
> > data->socdata->measure_temp_mask);
> > + /* After power up, we need a delay before first access can be done. */
> > + usleep_range(20, 50);
> > +
> > + /* the core was configured and enabled just before */
> > + pm_runtime_set_active(&pdev->dev);
> > + pm_runtime_enable(data->dev);
> > +
> > + ret = pm_runtime_resume_and_get(data->dev);
> > + if (ret < 0)
> > + goto disable_runtime_pm;
> >
> > data->irq_enabled = true;
> > ret = thermal_zone_device_enable(data->tz);
> > @@ -814,10 +800,15 @@ static int imx_thermal_probe(struct platform_device *pdev)
> > goto thermal_zone_unregister;
> > }
> >
> > + pm_runtime_put(data->dev);
> > +
> > return 0;
> >
> > thermal_zone_unregister:
> > thermal_zone_device_unregister(data->tz);
> > +disable_runtime_pm:
> > + pm_runtime_put_noidle(data->dev);
> > + pm_runtime_disable(data->dev);
> > clk_disable:
> > clk_disable_unprepare(data->thermal_clk);
> > legacy_cleanup:
> > @@ -829,13 +820,9 @@ static int imx_thermal_probe(struct platform_device *pdev)
> > static int imx_thermal_remove(struct platform_device *pdev)
> > {
> > struct imx_thermal_data *data = platform_get_drvdata(pdev);
> > - struct regmap *map = data->tempmon;
> >
> > - /* Disable measurements */
> > - regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
> > - data->socdata->power_down_mask);
> > - if (!IS_ERR(data->thermal_clk))
> > - clk_disable_unprepare(data->thermal_clk);
> > + pm_runtime_put_noidle(data->dev);
> > + pm_runtime_disable(data->dev);
> >
> > thermal_zone_device_unregister(data->tz);
> > imx_thermal_unregister_legacy_cooling(data);
> > @@ -858,29 +845,79 @@ static int __maybe_unused imx_thermal_suspend(struct device *dev)
> > ret = thermal_zone_device_disable(data->tz);
> > if (ret)
> > return ret;
> > +
> > + return pm_runtime_force_suspend(data->dev);
> > +}
> > +
> > +static int __maybe_unused imx_thermal_resume(struct device *dev)
> > +{
> > + struct imx_thermal_data *data = dev_get_drvdata(dev);
> > + int ret;
> > +
> > + ret = pm_runtime_force_resume(data->dev);
> > + if (ret)
> > + return ret;
> > + /* Enabled thermal sensor after resume */
> > + return thermal_zone_device_enable(data->tz);
> > +}
> > +
> > +static int __maybe_unused imx_thermal_runtime_suspend(struct device *dev)
> > +{
> > + struct imx_thermal_data *data = dev_get_drvdata(dev);
> > + const struct thermal_soc_data *socdata = data->socdata;
> > + struct regmap *map = data->tempmon;
> > + int ret;
> > +
> > + ret = regmap_write(map, socdata->sensor_ctrl + REG_CLR,
> > + socdata->measure_temp_mask);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_write(map, socdata->sensor_ctrl + REG_SET,
> > + socdata->power_down_mask);
> > + if (ret)
> > + return ret;
> > +
> > clk_disable_unprepare(data->thermal_clk);
> >
> > return 0;
> > }
> >
> > -static int __maybe_unused imx_thermal_resume(struct device *dev)
> > +static int __maybe_unused imx_thermal_runtime_resume(struct device *dev)
> > {
> > struct imx_thermal_data *data = dev_get_drvdata(dev);
> > + const struct thermal_soc_data *socdata = data->socdata;
> > + struct regmap *map = data->tempmon;
> > int ret;
> >
> > ret = clk_prepare_enable(data->thermal_clk);
> > if (ret)
> > return ret;
> > - /* Enabled thermal sensor after resume */
> > - ret = thermal_zone_device_enable(data->tz);
> > +
> > + ret = regmap_write(map, socdata->sensor_ctrl + REG_CLR,
> > + socdata->power_down_mask);
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_write(map, socdata->sensor_ctrl + REG_SET,
> > + socdata->measure_temp_mask);
> > if (ret)
> > return ret;
> >
> > + /*
> > + * According to the temp sensor designers, it may require up to ~17us
> > + * to complete a measurement.
> > + */
> > + usleep_range(20, 50);
> > +
> > return 0;
> > }
> >
> > -static SIMPLE_DEV_PM_OPS(imx_thermal_pm_ops,
> > - imx_thermal_suspend, imx_thermal_resume);
> > +static const struct dev_pm_ops imx_thermal_pm_ops = {
> > + SET_SYSTEM_SLEEP_PM_OPS(imx_thermal_suspend, imx_thermal_resume)
> > + SET_RUNTIME_PM_OPS(imx_thermal_runtime_suspend,
> > + imx_thermal_runtime_resume, NULL)
> > +};
> >
> > static struct platform_driver imx_thermal = {
> > .driver = {
> > --
> > 2.30.2
> >
> >
> >
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] thermal: imx: implement runtime PM support
2021-11-18 10:05 ` Petr Benes
@ 2021-11-19 9:18 ` Petr Benes
2021-11-19 9:28 ` Daniel Lezcano
0 siblings, 1 reply; 6+ messages in thread
From: Petr Benes @ 2021-11-19 9:18 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Shawn Guo, Sascha Hauer, Daniel Lezcano, Michal Vokáč,
linux-pm, Amit Kucheria, linux-kernel, Andrzej Pietrasiewicz,
NXP Linux Team, Pengutronix Kernel Team, David Jander, Zhang Rui,
Fabio Estevam, linux-arm-kernel
Hello everybody,
If this issue is about to close. Appending my tag.
Fixes: d92ed2c9d3ff ("thermal: imx: Use driver's local data to decide
whether to run a measurement")
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Tested-by: Petr Beneš <petr.benes@ysoft.com>
Thank you for the fix.
Petr
On Thu, 18 Nov 2021 at 11:05, Petr Benes <petrben@gmail.com> wrote:
>
> Works for me! Well done.
>
> On Wed, 17 Nov 2021 at 11:37, Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> >
> > Petr, Michal,
> >
> > can you please test this patch.
> >
> > On Wed, Nov 17, 2021 at 11:34:26AM +0100, Oleksij Rempel wrote:
> > > Starting with commit d92ed2c9d3ff ("thermal: imx: Use driver's local
> > > data to decide whether to run a measurement") this driver stared using
> > > irq_enabled flag to make decision to power on/off the thermal core. This
> > > triggered a regression, where after reaching critical temperature, alarm
> > > IRQ handler set irq_enabled to false, disabled thermal core and was not
> > > able read temperature and disable cooling sequence.
> > >
> > > In case the cooling device is "CPU/GPU freq", the system will run with
> > > reduce performance until next reboot.
> > >
> > > To solve this issue, we need to move all parts implementing hand made
> > > runtime power management and let it handle actual runtime PM framework.
> > >
> > > Fixes: d92ed2c9d3ff ("thermal: imx: Use driver's local data to decide whether to run a measurement")
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > ---
> > > drivers/thermal/imx_thermal.c | 145 +++++++++++++++++++++-------------
> > > 1 file changed, 91 insertions(+), 54 deletions(-)
> > >
> > > diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> > > index 2c7473d86a59..16663373b682 100644
> > > --- a/drivers/thermal/imx_thermal.c
> > > +++ b/drivers/thermal/imx_thermal.c
> > > @@ -15,6 +15,7 @@
> > > #include <linux/regmap.h>
> > > #include <linux/thermal.h>
> > > #include <linux/nvmem-consumer.h>
> > > +#include <linux/pm_runtime.h>
> > >
> > > #define REG_SET 0x4
> > > #define REG_CLR 0x8
> > > @@ -194,6 +195,7 @@ static struct thermal_soc_data thermal_imx7d_data = {
> > > };
> > >
> > > struct imx_thermal_data {
> > > + struct device *dev;
> > > struct cpufreq_policy *policy;
> > > struct thermal_zone_device *tz;
> > > struct thermal_cooling_device *cdev;
> > > @@ -252,44 +254,15 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
> > > const struct thermal_soc_data *soc_data = data->socdata;
> > > struct regmap *map = data->tempmon;
> > > unsigned int n_meas;
> > > - bool wait, run_measurement;
> > > u32 val;
> > > + int ret;
> > >
> > > - run_measurement = !data->irq_enabled;
> > > - if (!run_measurement) {
> > > - /* Check if a measurement is currently in progress */
> > > - regmap_read(map, soc_data->temp_data, &val);
> > > - wait = !(val & soc_data->temp_valid_mask);
> > > - } else {
> > > - /*
> > > - * Every time we measure the temperature, we will power on the
> > > - * temperature sensor, enable measurements, take a reading,
> > > - * disable measurements, power off the temperature sensor.
> > > - */
> > > - regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
> > > - soc_data->power_down_mask);
> > > - regmap_write(map, soc_data->sensor_ctrl + REG_SET,
> > > - soc_data->measure_temp_mask);
> > > -
> > > - wait = true;
> > > - }
> > > -
> > > - /*
> > > - * According to the temp sensor designers, it may require up to ~17us
> > > - * to complete a measurement.
> > > - */
> > > - if (wait)
> > > - usleep_range(20, 50);
> > > + ret = pm_runtime_resume_and_get(data->dev);
> > > + if (ret < 0)
> > > + return ret;
> > >
> > > regmap_read(map, soc_data->temp_data, &val);
> > >
> > > - if (run_measurement) {
> > > - regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
> > > - soc_data->measure_temp_mask);
> > > - regmap_write(map, soc_data->sensor_ctrl + REG_SET,
> > > - soc_data->power_down_mask);
> > > - }
> > > -
> > > if ((val & soc_data->temp_valid_mask) == 0) {
> > > dev_dbg(&tz->device, "temp measurement never finished\n");
> > > return -EAGAIN;
> > > @@ -328,6 +301,8 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
> > > enable_irq(data->irq);
> > > }
> > >
> > > + pm_runtime_put(data->dev);
> > > +
> > > return 0;
> > > }
> > >
> > > @@ -335,24 +310,16 @@ static int imx_change_mode(struct thermal_zone_device *tz,
> > > enum thermal_device_mode mode)
> > > {
> > > struct imx_thermal_data *data = tz->devdata;
> > > - struct regmap *map = data->tempmon;
> > > - const struct thermal_soc_data *soc_data = data->socdata;
> > >
> > > if (mode == THERMAL_DEVICE_ENABLED) {
> > > - regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
> > > - soc_data->power_down_mask);
> > > - regmap_write(map, soc_data->sensor_ctrl + REG_SET,
> > > - soc_data->measure_temp_mask);
> > > + pm_runtime_get(data->dev);
> > >
> > > if (!data->irq_enabled) {
> > > data->irq_enabled = true;
> > > enable_irq(data->irq);
> > > }
> > > } else {
> > > - regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
> > > - soc_data->measure_temp_mask);
> > > - regmap_write(map, soc_data->sensor_ctrl + REG_SET,
> > > - soc_data->power_down_mask);
> > > + pm_runtime_put(data->dev);
> > >
> > > if (data->irq_enabled) {
> > > disable_irq(data->irq);
> > > @@ -393,6 +360,11 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
> > > int temp)
> > > {
> > > struct imx_thermal_data *data = tz->devdata;
> > > + int ret;
> > > +
> > > + ret = pm_runtime_resume_and_get(data->dev);
> > > + if (ret < 0)
> > > + return ret;
> > >
> > > /* do not allow changing critical threshold */
> > > if (trip == IMX_TRIP_CRITICAL)
> > > @@ -406,6 +378,8 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
> > >
> > > imx_set_alarm_temp(data, temp);
> > >
> > > + pm_runtime_put(data->dev);
> > > +
> > > return 0;
> > > }
> > >
> > > @@ -681,6 +655,8 @@ static int imx_thermal_probe(struct platform_device *pdev)
> > > if (!data)
> > > return -ENOMEM;
> > >
> > > + data->dev = &pdev->dev;
> > > +
> > > map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "fsl,tempmon");
> > > if (IS_ERR(map)) {
> > > ret = PTR_ERR(map);
> > > @@ -800,6 +776,16 @@ static int imx_thermal_probe(struct platform_device *pdev)
> > > data->socdata->power_down_mask);
> > > regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
> > > data->socdata->measure_temp_mask);
> > > + /* After power up, we need a delay before first access can be done. */
> > > + usleep_range(20, 50);
> > > +
> > > + /* the core was configured and enabled just before */
> > > + pm_runtime_set_active(&pdev->dev);
> > > + pm_runtime_enable(data->dev);
> > > +
> > > + ret = pm_runtime_resume_and_get(data->dev);
> > > + if (ret < 0)
> > > + goto disable_runtime_pm;
> > >
> > > data->irq_enabled = true;
> > > ret = thermal_zone_device_enable(data->tz);
> > > @@ -814,10 +800,15 @@ static int imx_thermal_probe(struct platform_device *pdev)
> > > goto thermal_zone_unregister;
> > > }
> > >
> > > + pm_runtime_put(data->dev);
> > > +
> > > return 0;
> > >
> > > thermal_zone_unregister:
> > > thermal_zone_device_unregister(data->tz);
> > > +disable_runtime_pm:
> > > + pm_runtime_put_noidle(data->dev);
> > > + pm_runtime_disable(data->dev);
> > > clk_disable:
> > > clk_disable_unprepare(data->thermal_clk);
> > > legacy_cleanup:
> > > @@ -829,13 +820,9 @@ static int imx_thermal_probe(struct platform_device *pdev)
> > > static int imx_thermal_remove(struct platform_device *pdev)
> > > {
> > > struct imx_thermal_data *data = platform_get_drvdata(pdev);
> > > - struct regmap *map = data->tempmon;
> > >
> > > - /* Disable measurements */
> > > - regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
> > > - data->socdata->power_down_mask);
> > > - if (!IS_ERR(data->thermal_clk))
> > > - clk_disable_unprepare(data->thermal_clk);
> > > + pm_runtime_put_noidle(data->dev);
> > > + pm_runtime_disable(data->dev);
> > >
> > > thermal_zone_device_unregister(data->tz);
> > > imx_thermal_unregister_legacy_cooling(data);
> > > @@ -858,29 +845,79 @@ static int __maybe_unused imx_thermal_suspend(struct device *dev)
> > > ret = thermal_zone_device_disable(data->tz);
> > > if (ret)
> > > return ret;
> > > +
> > > + return pm_runtime_force_suspend(data->dev);
> > > +}
> > > +
> > > +static int __maybe_unused imx_thermal_resume(struct device *dev)
> > > +{
> > > + struct imx_thermal_data *data = dev_get_drvdata(dev);
> > > + int ret;
> > > +
> > > + ret = pm_runtime_force_resume(data->dev);
> > > + if (ret)
> > > + return ret;
> > > + /* Enabled thermal sensor after resume */
> > > + return thermal_zone_device_enable(data->tz);
> > > +}
> > > +
> > > +static int __maybe_unused imx_thermal_runtime_suspend(struct device *dev)
> > > +{
> > > + struct imx_thermal_data *data = dev_get_drvdata(dev);
> > > + const struct thermal_soc_data *socdata = data->socdata;
> > > + struct regmap *map = data->tempmon;
> > > + int ret;
> > > +
> > > + ret = regmap_write(map, socdata->sensor_ctrl + REG_CLR,
> > > + socdata->measure_temp_mask);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = regmap_write(map, socdata->sensor_ctrl + REG_SET,
> > > + socdata->power_down_mask);
> > > + if (ret)
> > > + return ret;
> > > +
> > > clk_disable_unprepare(data->thermal_clk);
> > >
> > > return 0;
> > > }
> > >
> > > -static int __maybe_unused imx_thermal_resume(struct device *dev)
> > > +static int __maybe_unused imx_thermal_runtime_resume(struct device *dev)
> > > {
> > > struct imx_thermal_data *data = dev_get_drvdata(dev);
> > > + const struct thermal_soc_data *socdata = data->socdata;
> > > + struct regmap *map = data->tempmon;
> > > int ret;
> > >
> > > ret = clk_prepare_enable(data->thermal_clk);
> > > if (ret)
> > > return ret;
> > > - /* Enabled thermal sensor after resume */
> > > - ret = thermal_zone_device_enable(data->tz);
> > > +
> > > + ret = regmap_write(map, socdata->sensor_ctrl + REG_CLR,
> > > + socdata->power_down_mask);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = regmap_write(map, socdata->sensor_ctrl + REG_SET,
> > > + socdata->measure_temp_mask);
> > > if (ret)
> > > return ret;
> > >
> > > + /*
> > > + * According to the temp sensor designers, it may require up to ~17us
> > > + * to complete a measurement.
> > > + */
> > > + usleep_range(20, 50);
> > > +
> > > return 0;
> > > }
> > >
> > > -static SIMPLE_DEV_PM_OPS(imx_thermal_pm_ops,
> > > - imx_thermal_suspend, imx_thermal_resume);
> > > +static const struct dev_pm_ops imx_thermal_pm_ops = {
> > > + SET_SYSTEM_SLEEP_PM_OPS(imx_thermal_suspend, imx_thermal_resume)
> > > + SET_RUNTIME_PM_OPS(imx_thermal_runtime_suspend,
> > > + imx_thermal_runtime_resume, NULL)
> > > +};
> > >
> > > static struct platform_driver imx_thermal = {
> > > .driver = {
> > > --
> > > 2.30.2
> > >
> > >
> > >
> >
> > --
> > Pengutronix e.K. | |
> > Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] thermal: imx: implement runtime PM support
2021-11-19 9:18 ` Petr Benes
@ 2021-11-19 9:28 ` Daniel Lezcano
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Lezcano @ 2021-11-19 9:28 UTC (permalink / raw)
To: Petr Benes, Oleksij Rempel
Cc: Shawn Guo, Sascha Hauer, Michal Vokáč,
linux-pm, Amit Kucheria, linux-kernel, Andrzej Pietrasiewicz,
NXP Linux Team, Pengutronix Kernel Team, David Jander, Zhang Rui,
Fabio Estevam, linux-arm-kernel
On 19/11/2021 10:18, Petr Benes wrote:
> Hello everybody,
>
> If this issue is about to close. Appending my tag.
>
> Fixes: d92ed2c9d3ff ("thermal: imx: Use driver's local data to decide
> whether to run a measurement")
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>
> Tested-by: Petr Beneš <petr.benes@ysoft.com>
Applied, thanks for testing.
In the future, please just add your tag, not the other ones otherwise
that puzzle the tools collecting them.
-- D.
[ ... ]
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] thermal/drivers/imx: add missing pm_runtime_put on error path
2021-11-17 10:34 [PATCH v3] thermal: imx: implement runtime PM support Oleksij Rempel
2021-11-17 10:37 ` Oleksij Rempel
@ 2022-02-17 4:41 ` Dominique Martinet
1 sibling, 0 replies; 6+ messages in thread
From: Dominique Martinet @ 2022-02-17 4:41 UTC (permalink / raw)
To: Oleksij Rempel, Daniel Lezcano
Cc: Rafael J . Wysocki, Amit Kucheria, Zhang Rui, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, linux-pm, linux-arm-kernel, linux-kernel,
Petr Beneš,
Dominique Martinet
We took a reference so need to put it back when imx_get_temp() returns
EAGAIN.
Fixes: 4cf2ddf16e17 ("thermal/drivers/imx: Implement runtime PM support")
Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
---
stumbled upon this merging our stable tree, but I don't actually use
imx-thermal so haven't tested it.
drivers/thermal/imx_thermal.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 16663373b682..641f90f9c750 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -265,6 +265,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
if ((val & soc_data->temp_valid_mask) == 0) {
dev_dbg(&tz->device, "temp measurement never finished\n");
+ pm_runtime_put(data->dev);
return -EAGAIN;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-17 4:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-17 10:34 [PATCH v3] thermal: imx: implement runtime PM support Oleksij Rempel
2021-11-17 10:37 ` Oleksij Rempel
2021-11-18 10:05 ` Petr Benes
2021-11-19 9:18 ` Petr Benes
2021-11-19 9:28 ` Daniel Lezcano
2022-02-17 4:41 ` [PATCH] thermal/drivers/imx: add missing pm_runtime_put on error path Dominique Martinet
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).