* [PATCH v3] imx: thermal: imx_get_temp might be called before sensor clock is prepared
@ 2014-10-13 20:51 Heiner Kallweit
2014-11-07 18:18 ` Eduardo Valentin
0 siblings, 1 reply; 4+ messages in thread
From: Heiner Kallweit @ 2014-10-13 20:51 UTC (permalink / raw)
To: linux-pm; +Cc: Zhang Rui <rui.zhang@intel.com>; Eduardo Valentin
imx_get_temp might be called before the sensor clock is prepared
thus resulting in a timeout of the first attempt to read temp:
thermal thermal_zone0: failed to read out thermal zone 0
Happened to me on a Utilite Standard with IMX6 Dual SoC.
Reason is that in imx_thermal_probe thermal_zone_device_register
is called before the sensor clock is prepared.
thermal_zone_device_register however calls
thermal_zone_device_update which eventually calls imx_get_temp.
Fix this by preparing the clock before calling
thermal_zone_device_register.
Signed-off-by: Heiner Kallweit <heiner.kallweit@web.de>
---
v2: revised error path. Bail out and tidy up properly if we can't
get the clock or fail to enable it
v3: don't print error message if getting clock returns EPROBE_DEFER
---
drivers/thermal/imx_thermal.c | 41 +++++++++++++++++++++++++----------------
1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 461bf3d..0e8ef55 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -521,6 +521,30 @@ static int imx_thermal_probe(struct platform_device *pdev)
return ret;
}
+ data->thermal_clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(data->thermal_clk)) {
+ ret = PTR_ERR(data->thermal_clk);
+ if (ret != -EPROBE_DEFER)
+ dev_err(&pdev->dev,
+ "failed to get thermal clk: %d\n", ret);
+ cpufreq_cooling_unregister(data->cdev);
+ return ret;
+ }
+
+ /*
+ * Thermal sensor needs clk on to get correct value, normally
+ * we should enable its clk before taking measurement and disable
+ * clk after measurement is done, but if alarm function is enabled,
+ * hardware will auto measure the temperature periodically, so we
+ * need to keep the clk always on for alarm function.
+ */
+ ret = clk_prepare_enable(data->thermal_clk);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to enable thermal clk: %d\n", ret);
+ cpufreq_cooling_unregister(data->cdev);
+ return ret;
+ }
+
data->tz = thermal_zone_device_register("imx_thermal_zone",
IMX_TRIP_NUM,
BIT(IMX_TRIP_PASSIVE), data,
@@ -531,26 +555,11 @@ static int imx_thermal_probe(struct platform_device *pdev)
ret = PTR_ERR(data->tz);
dev_err(&pdev->dev,
"failed to register thermal zone device %d\n", ret);
+ clk_disable_unprepare(data->thermal_clk);
cpufreq_cooling_unregister(data->cdev);
return ret;
}
- data->thermal_clk = devm_clk_get(&pdev->dev, NULL);
- if (IS_ERR(data->thermal_clk)) {
- dev_warn(&pdev->dev, "failed to get thermal clk!\n");
- } else {
- /*
- * Thermal sensor needs clk on to get correct value, normally
- * we should enable its clk before taking measurement and disable
- * clk after measurement is done, but if alarm function is enabled,
- * hardware will auto measure the temperature periodically, so we
- * need to keep the clk always on for alarm function.
- */
- ret = clk_prepare_enable(data->thermal_clk);
- if (ret)
- dev_warn(&pdev->dev, "failed to enable thermal clk: %d\n", ret);
- }
-
/* Enable measurements at ~ 10 Hz */
regmap_write(map, TEMPSENSE1 + REG_CLR, TEMPSENSE1_MEASURE_FREQ);
measure_freq = DIV_ROUND_UP(32768, 10); /* 10 Hz */
--
2.1.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v3] imx: thermal: imx_get_temp might be called before sensor clock is prepared 2014-10-13 20:51 [PATCH v3] imx: thermal: imx_get_temp might be called before sensor clock is prepared Heiner Kallweit @ 2014-11-07 18:18 ` Eduardo Valentin 2014-11-07 22:43 ` Heiner Kallweit 0 siblings, 1 reply; 4+ messages in thread From: Eduardo Valentin @ 2014-11-07 18:18 UTC (permalink / raw) To: Heiner Kallweit; +Cc: linux-pm [-- Attachment #1: Type: text/plain, Size: 4375 bytes --] Hello Heiner, On Mon, Oct 13, 2014 at 10:51:23PM +0200, Heiner Kallweit wrote: > imx_get_temp might be called before the sensor clock is prepared > thus resulting in a timeout of the first attempt to read temp: > thermal thermal_zone0: failed to read out thermal zone 0 > Happened to me on a Utilite Standard with IMX6 Dual SoC. > > Reason is that in imx_thermal_probe thermal_zone_device_register > is called before the sensor clock is prepared. > thermal_zone_device_register however calls > thermal_zone_device_update which eventually calls imx_get_temp. > > Fix this by preparing the clock before calling > thermal_zone_device_register. > > Signed-off-by: Heiner Kallweit <heiner.kallweit@web.de> > --- > v2: revised error path. Bail out and tidy up properly if we can't > get the clock or fail to enable it > v3: don't print error message if getting clock returns EPROBE_DEFER > --- > drivers/thermal/imx_thermal.c | 41 +++++++++++++++++++++++++---------------- > 1 file changed, 25 insertions(+), 16 deletions(-) > > diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c > index 461bf3d..0e8ef55 100644 > --- a/drivers/thermal/imx_thermal.c > +++ b/drivers/thermal/imx_thermal.c > @@ -521,6 +521,30 @@ static int imx_thermal_probe(struct platform_device *pdev) > return ret; > } > > + data->thermal_clk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(data->thermal_clk)) { > + ret = PTR_ERR(data->thermal_clk); > + if (ret != -EPROBE_DEFER) > + dev_err(&pdev->dev, > + "failed to get thermal clk: %d\n", ret); > + cpufreq_cooling_unregister(data->cdev); > + return ret; > + } > + > + /* > + * Thermal sensor needs clk on to get correct value, normally > + * we should enable its clk before taking measurement and disable > + * clk after measurement is done, but if alarm function is enabled, > + * hardware will auto measure the temperature periodically, so we > + * need to keep the clk always on for alarm function. > + */ > + ret = clk_prepare_enable(data->thermal_clk); > + if (ret) { > + dev_err(&pdev->dev, "failed to enable thermal clk: %d\n", ret); > + cpufreq_cooling_unregister(data->cdev); > + return ret; > + } > + > data->tz = thermal_zone_device_register("imx_thermal_zone", > IMX_TRIP_NUM, > BIT(IMX_TRIP_PASSIVE), data, > @@ -531,26 +555,11 @@ static int imx_thermal_probe(struct platform_device *pdev) > ret = PTR_ERR(data->tz); > dev_err(&pdev->dev, > "failed to register thermal zone device %d\n", ret); > + clk_disable_unprepare(data->thermal_clk); > cpufreq_cooling_unregister(data->cdev); > return ret; > } > > - data->thermal_clk = devm_clk_get(&pdev->dev, NULL); > - if (IS_ERR(data->thermal_clk)) { > - dev_warn(&pdev->dev, "failed to get thermal clk!\n"); > - } else { > - /* > - * Thermal sensor needs clk on to get correct value, normally > - * we should enable its clk before taking measurement and disable > - * clk after measurement is done, but if alarm function is enabled, > - * hardware will auto measure the temperature periodically, so we > - * need to keep the clk always on for alarm function. > - */ > - ret = clk_prepare_enable(data->thermal_clk); > - if (ret) > - dev_warn(&pdev->dev, "failed to enable thermal clk: %d\n", ret); > - } > - > /* Enable measurements at ~ 10 Hz */ > regmap_write(map, TEMPSENSE1 + REG_CLR, TEMPSENSE1_MEASURE_FREQ); > measure_freq = DIV_ROUND_UP(32768, 10); /* 10 Hz */ While here, do you need to move up also the configuration of the measurements at ~ 10 Hz? Or would still the first readings be correct while at the reset value? Cheers, Eduardo Valentin > -- > 2.1.2 [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] imx: thermal: imx_get_temp might be called before sensor clock is prepared 2014-11-07 18:18 ` Eduardo Valentin @ 2014-11-07 22:43 ` Heiner Kallweit 2014-11-07 23:47 ` Eduardo Valentin 0 siblings, 1 reply; 4+ messages in thread From: Heiner Kallweit @ 2014-11-07 22:43 UTC (permalink / raw) To: Eduardo Valentin; +Cc: linux-pm Am 07.11.2014 um 19:18 schrieb Eduardo Valentin: > > Hello Heiner, > > On Mon, Oct 13, 2014 at 10:51:23PM +0200, Heiner Kallweit wrote: >> imx_get_temp might be called before the sensor clock is prepared >> thus resulting in a timeout of the first attempt to read temp: >> thermal thermal_zone0: failed to read out thermal zone 0 >> Happened to me on a Utilite Standard with IMX6 Dual SoC. >> >> Reason is that in imx_thermal_probe thermal_zone_device_register >> is called before the sensor clock is prepared. >> thermal_zone_device_register however calls >> thermal_zone_device_update which eventually calls imx_get_temp. >> >> Fix this by preparing the clock before calling >> thermal_zone_device_register. >> >> Signed-off-by: Heiner Kallweit <heiner.kallweit@web.de> >> --- >> v2: revised error path. Bail out and tidy up properly if we can't >> get the clock or fail to enable it >> v3: don't print error message if getting clock returns EPROBE_DEFER >> --- >> drivers/thermal/imx_thermal.c | 41 +++++++++++++++++++++++++---------------- >> 1 file changed, 25 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c >> index 461bf3d..0e8ef55 100644 >> --- a/drivers/thermal/imx_thermal.c >> +++ b/drivers/thermal/imx_thermal.c >> @@ -521,6 +521,30 @@ static int imx_thermal_probe(struct platform_device *pdev) >> return ret; >> } >> >> + data->thermal_clk = devm_clk_get(&pdev->dev, NULL); >> + if (IS_ERR(data->thermal_clk)) { >> + ret = PTR_ERR(data->thermal_clk); >> + if (ret != -EPROBE_DEFER) >> + dev_err(&pdev->dev, >> + "failed to get thermal clk: %d\n", ret); >> + cpufreq_cooling_unregister(data->cdev); >> + return ret; >> + } >> + >> + /* >> + * Thermal sensor needs clk on to get correct value, normally >> + * we should enable its clk before taking measurement and disable >> + * clk after measurement is done, but if alarm function is enabled, >> + * hardware will auto measure the temperature periodically, so we >> + * need to keep the clk always on for alarm function. >> + */ >> + ret = clk_prepare_enable(data->thermal_clk); >> + if (ret) { >> + dev_err(&pdev->dev, "failed to enable thermal clk: %d\n", ret); >> + cpufreq_cooling_unregister(data->cdev); >> + return ret; >> + } >> + >> data->tz = thermal_zone_device_register("imx_thermal_zone", >> IMX_TRIP_NUM, >> BIT(IMX_TRIP_PASSIVE), data, >> @@ -531,26 +555,11 @@ static int imx_thermal_probe(struct platform_device *pdev) >> ret = PTR_ERR(data->tz); >> dev_err(&pdev->dev, >> "failed to register thermal zone device %d\n", ret); >> + clk_disable_unprepare(data->thermal_clk); >> cpufreq_cooling_unregister(data->cdev); >> return ret; >> } >> >> - data->thermal_clk = devm_clk_get(&pdev->dev, NULL); >> - if (IS_ERR(data->thermal_clk)) { >> - dev_warn(&pdev->dev, "failed to get thermal clk!\n"); >> - } else { >> - /* >> - * Thermal sensor needs clk on to get correct value, normally >> - * we should enable its clk before taking measurement and disable >> - * clk after measurement is done, but if alarm function is enabled, >> - * hardware will auto measure the temperature periodically, so we >> - * need to keep the clk always on for alarm function. >> - */ >> - ret = clk_prepare_enable(data->thermal_clk); >> - if (ret) >> - dev_warn(&pdev->dev, "failed to enable thermal clk: %d\n", ret); >> - } >> - >> /* Enable measurements at ~ 10 Hz */ >> regmap_write(map, TEMPSENSE1 + REG_CLR, TEMPSENSE1_MEASURE_FREQ); >> measure_freq = DIV_ROUND_UP(32768, 10); /* 10 Hz */ > > > While here, do you need to move up also the configuration of the > measurements at ~ 10 Hz? Or would still the first readings be correct > while at the reset value? When imx_get_temp is called from thermal_zone_device_register data->mode still is THERMAL_DEVICE_DISABLED. imx_get_temp therefore will power on the sensor and trigger a single measurement. IMHO this is fully ok and I don't think we have to relocate the setup of the scheduled measurements. > > Cheers, > > Eduardo Valentin >> -- >> 2.1.2 Rgds, Heiner ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] imx: thermal: imx_get_temp might be called before sensor clock is prepared 2014-11-07 22:43 ` Heiner Kallweit @ 2014-11-07 23:47 ` Eduardo Valentin 0 siblings, 0 replies; 4+ messages in thread From: Eduardo Valentin @ 2014-11-07 23:47 UTC (permalink / raw) To: Heiner Kallweit; +Cc: linux-pm [-- Attachment #1: Type: text/plain, Size: 5408 bytes --] Heiner, On Fri, Nov 07, 2014 at 11:43:15PM +0100, Heiner Kallweit wrote: > Am 07.11.2014 um 19:18 schrieb Eduardo Valentin: > > > > Hello Heiner, > > > > On Mon, Oct 13, 2014 at 10:51:23PM +0200, Heiner Kallweit wrote: > >> imx_get_temp might be called before the sensor clock is prepared > >> thus resulting in a timeout of the first attempt to read temp: > >> thermal thermal_zone0: failed to read out thermal zone 0 > >> Happened to me on a Utilite Standard with IMX6 Dual SoC. > >> > >> Reason is that in imx_thermal_probe thermal_zone_device_register > >> is called before the sensor clock is prepared. > >> thermal_zone_device_register however calls > >> thermal_zone_device_update which eventually calls imx_get_temp. > >> > >> Fix this by preparing the clock before calling > >> thermal_zone_device_register. > >> > >> Signed-off-by: Heiner Kallweit <heiner.kallweit@web.de> > >> --- > >> v2: revised error path. Bail out and tidy up properly if we can't > >> get the clock or fail to enable it > >> v3: don't print error message if getting clock returns EPROBE_DEFER > >> --- > >> drivers/thermal/imx_thermal.c | 41 +++++++++++++++++++++++++---------------- > >> 1 file changed, 25 insertions(+), 16 deletions(-) > >> > >> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c > >> index 461bf3d..0e8ef55 100644 > >> --- a/drivers/thermal/imx_thermal.c > >> +++ b/drivers/thermal/imx_thermal.c > >> @@ -521,6 +521,30 @@ static int imx_thermal_probe(struct platform_device *pdev) > >> return ret; > >> } > >> > >> + data->thermal_clk = devm_clk_get(&pdev->dev, NULL); > >> + if (IS_ERR(data->thermal_clk)) { > >> + ret = PTR_ERR(data->thermal_clk); > >> + if (ret != -EPROBE_DEFER) > >> + dev_err(&pdev->dev, > >> + "failed to get thermal clk: %d\n", ret); > >> + cpufreq_cooling_unregister(data->cdev); > >> + return ret; > >> + } > >> + > >> + /* > >> + * Thermal sensor needs clk on to get correct value, normally > >> + * we should enable its clk before taking measurement and disable > >> + * clk after measurement is done, but if alarm function is enabled, > >> + * hardware will auto measure the temperature periodically, so we > >> + * need to keep the clk always on for alarm function. > >> + */ > >> + ret = clk_prepare_enable(data->thermal_clk); > >> + if (ret) { > >> + dev_err(&pdev->dev, "failed to enable thermal clk: %d\n", ret); > >> + cpufreq_cooling_unregister(data->cdev); > >> + return ret; > >> + } > >> + > >> data->tz = thermal_zone_device_register("imx_thermal_zone", > >> IMX_TRIP_NUM, > >> BIT(IMX_TRIP_PASSIVE), data, > >> @@ -531,26 +555,11 @@ static int imx_thermal_probe(struct platform_device *pdev) > >> ret = PTR_ERR(data->tz); > >> dev_err(&pdev->dev, > >> "failed to register thermal zone device %d\n", ret); > >> + clk_disable_unprepare(data->thermal_clk); > >> cpufreq_cooling_unregister(data->cdev); > >> return ret; > >> } > >> > >> - data->thermal_clk = devm_clk_get(&pdev->dev, NULL); > >> - if (IS_ERR(data->thermal_clk)) { > >> - dev_warn(&pdev->dev, "failed to get thermal clk!\n"); > >> - } else { > >> - /* > >> - * Thermal sensor needs clk on to get correct value, normally > >> - * we should enable its clk before taking measurement and disable > >> - * clk after measurement is done, but if alarm function is enabled, > >> - * hardware will auto measure the temperature periodically, so we > >> - * need to keep the clk always on for alarm function. > >> - */ > >> - ret = clk_prepare_enable(data->thermal_clk); > >> - if (ret) > >> - dev_warn(&pdev->dev, "failed to enable thermal clk: %d\n", ret); > >> - } > >> - > >> /* Enable measurements at ~ 10 Hz */ > >> regmap_write(map, TEMPSENSE1 + REG_CLR, TEMPSENSE1_MEASURE_FREQ); > >> measure_freq = DIV_ROUND_UP(32768, 10); /* 10 Hz */ > > > > > > While here, do you need to move up also the configuration of the > > measurements at ~ 10 Hz? Or would still the first readings be correct > > while at the reset value? > When imx_get_temp is called from thermal_zone_device_register data->mode still is > THERMAL_DEVICE_DISABLED. imx_get_temp therefore will power on the sensor and > trigger a single measurement. > IMHO this is fully ok and I don't think we have to relocate the setup of the > scheduled measurements. OK. I see. In this case, can you please re-post this patch on top of my -fixes patches? I am planing to send it in the -rc cycles. I've just merged another fix on imx driver and looks like your patch needs refreshing now. Thanks, Eduardo Valentin > > > > Cheers, > > > > Eduardo Valentin > >> -- > >> 2.1.2 > Rgds, Heiner > [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-11-07 23:48 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-10-13 20:51 [PATCH v3] imx: thermal: imx_get_temp might be called before sensor clock is prepared Heiner Kallweit 2014-11-07 18:18 ` Eduardo Valentin 2014-11-07 22:43 ` Heiner Kallweit 2014-11-07 23:47 ` Eduardo Valentin
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).