* [PATCH] imx: thermal: imx_get_temp might be called before the sensor clock ist prepared
@ 2014-10-06 18:37 Heiner Kallweit
2014-10-07 13:04 ` Philipp Zabel
0 siblings, 1 reply; 6+ messages in thread
From: Heiner Kallweit @ 2014-10-06 18:37 UTC (permalink / raw)
To: linux-arm-kernel
imx_get_temp might be called before the sensor clock ist 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>
---
drivers/thermal/imx_thermal.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 461bf3d..ef50df6 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -521,20 +521,6 @@ static int imx_thermal_probe(struct platform_device *pdev)
return ret;
}
- data->tz = thermal_zone_device_register("imx_thermal_zone",
- IMX_TRIP_NUM,
- BIT(IMX_TRIP_PASSIVE), data,
- &imx_tz_ops, NULL,
- IMX_PASSIVE_DELAY,
- IMX_POLLING_DELAY);
- if (IS_ERR(data->tz)) {
- ret = PTR_ERR(data->tz);
- dev_err(&pdev->dev,
- "failed to register thermal zone device %d\n", ret);
- 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");
@@ -551,6 +537,20 @@ static int imx_thermal_probe(struct platform_device *pdev)
dev_warn(&pdev->dev, "failed to enable thermal clk: %d\n", ret);
}
+ data->tz = thermal_zone_device_register("imx_thermal_zone",
+ IMX_TRIP_NUM,
+ BIT(IMX_TRIP_PASSIVE), data,
+ &imx_tz_ops, NULL,
+ IMX_PASSIVE_DELAY,
+ IMX_POLLING_DELAY);
+ if (IS_ERR(data->tz)) {
+ ret = PTR_ERR(data->tz);
+ dev_err(&pdev->dev,
+ "failed to register thermal zone device %d\n", ret);
+ cpufreq_cooling_unregister(data->cdev);
+ return 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] 6+ messages in thread* [PATCH] imx: thermal: imx_get_temp might be called before the sensor clock ist prepared
2014-10-06 18:37 [PATCH] imx: thermal: imx_get_temp might be called before the sensor clock ist prepared Heiner Kallweit
@ 2014-10-07 13:04 ` Philipp Zabel
2014-10-07 19:36 ` Heiner Kallweit
0 siblings, 1 reply; 6+ messages in thread
From: Philipp Zabel @ 2014-10-07 13:04 UTC (permalink / raw)
To: linux-arm-kernel
Hi Heiner,
Am Montag, den 06.10.2014, 20:37 +0200 schrieb Heiner Kallweit:
> imx_get_temp might be called before the sensor clock ist 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>
Looks good to me, just an issue with the error path - see below.
[...]
> @@ -551,6 +537,20 @@ static int imx_thermal_probe(struct platform_device *pdev)
> dev_warn(&pdev->dev, "failed to enable thermal clk: %d\n", ret);
> }
>
> + data->tz = thermal_zone_device_register("imx_thermal_zone",
> + IMX_TRIP_NUM,
> + BIT(IMX_TRIP_PASSIVE), data,
> + &imx_tz_ops, NULL,
> + IMX_PASSIVE_DELAY,
> + IMX_POLLING_DELAY);
> + if (IS_ERR(data->tz)) {
> + ret = PTR_ERR(data->tz);
> + dev_err(&pdev->dev,
> + "failed to register thermal zone device %d\n", ret);
If the thermal_clk was enabled before, clk_disable_unprepare() should be
called on it here.
> + cpufreq_cooling_unregister(data->cdev);
> + return 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
regards
Philipp
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] imx: thermal: imx_get_temp might be called before the sensor clock ist prepared
2014-10-07 13:04 ` Philipp Zabel
@ 2014-10-07 19:36 ` Heiner Kallweit
2014-10-07 19:42 ` Fabio Estevam
0 siblings, 1 reply; 6+ messages in thread
From: Heiner Kallweit @ 2014-10-07 19:36 UTC (permalink / raw)
To: linux-arm-kernel
Thanks for the hint, Philipp.
After thinking a little bit more about it I'm not sure about one thing:
If we can't get the clock or fail to enable it: Currently the code just throws
a warning and proceeds with trying to register the thermal device.
Does this make sense or shall we bail out instead (like in case of other error conditions
in imx_thermal_probe)? I'd tend to bail out ..
Regards, Heiner
Am 07.10.2014 um 15:04 schrieb Philipp Zabel:
> Hi Heiner,
>
> Am Montag, den 06.10.2014, 20:37 +0200 schrieb Heiner Kallweit:
>> imx_get_temp might be called before the sensor clock ist 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>
>
> Looks good to me, just an issue with the error path - see below.
>
> [...]
>> @@ -551,6 +537,20 @@ static int imx_thermal_probe(struct platform_device *pdev)
>> dev_warn(&pdev->dev, "failed to enable thermal clk: %d\n", ret);
>> }
>>
>> + data->tz = thermal_zone_device_register("imx_thermal_zone",
>> + IMX_TRIP_NUM,
>> + BIT(IMX_TRIP_PASSIVE), data,
>> + &imx_tz_ops, NULL,
>> + IMX_PASSIVE_DELAY,
>> + IMX_POLLING_DELAY);
>> + if (IS_ERR(data->tz)) {
>> + ret = PTR_ERR(data->tz);
>> + dev_err(&pdev->dev,
>> + "failed to register thermal zone device %d\n", ret);
>
> If the thermal_clk was enabled before, clk_disable_unprepare() should be
> called on it here.
>
>> + cpufreq_cooling_unregister(data->cdev);
>> + return 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
>
> regards
> Philipp
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] imx: thermal: imx_get_temp might be called before the sensor clock ist prepared
2014-10-07 19:36 ` Heiner Kallweit
@ 2014-10-07 19:42 ` Fabio Estevam
2014-10-08 5:27 ` Heiner Kallweit
0 siblings, 1 reply; 6+ messages in thread
From: Fabio Estevam @ 2014-10-07 19:42 UTC (permalink / raw)
To: linux-arm-kernel
Hi Heiner,
On Tue, Oct 7, 2014 at 4:36 PM, Heiner Kallweit <heiner.kallweit@web.de> wrote:
> Thanks for the hint, Philipp.
> After thinking a little bit more about it I'm not sure about one thing:
>
> If we can't get the clock or fail to enable it: Currently the code just throws
> a warning and proceeds with trying to register the thermal device.
> Does this make sense or shall we bail out instead (like in case of other error conditions
> in imx_thermal_probe)? I'd tend to bail out ..
Agree with you. We should do a 'return ret' if clk_prepare_enable()
fails. Thanks
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] imx: thermal: imx_get_temp might be called before the sensor clock ist prepared
2014-10-07 19:42 ` Fabio Estevam
@ 2014-10-08 5:27 ` Heiner Kallweit
0 siblings, 0 replies; 6+ messages in thread
From: Heiner Kallweit @ 2014-10-08 5:27 UTC (permalink / raw)
To: linux-arm-kernel
OK, I'll submit a revised patch with improved error path.
Regards, Heiner
Am 07.10.2014 um 21:42 schrieb Fabio Estevam:
> Hi Heiner,
>
> On Tue, Oct 7, 2014 at 4:36 PM, Heiner Kallweit <heiner.kallweit@web.de> wrote:
>> Thanks for the hint, Philipp.
>> After thinking a little bit more about it I'm not sure about one thing:
>>
>> If we can't get the clock or fail to enable it: Currently the code just throws
>> a warning and proceeds with trying to register the thermal device.
>> Does this make sense or shall we bail out instead (like in case of other error conditions
>> in imx_thermal_probe)? I'd tend to bail out ..
>
> Agree with you. We should do a 'return ret' if clk_prepare_enable()
> fails. Thanks
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Patch] imx: thermal: imx_get_temp might be called before the sensor clock ist prepared
@ 2014-10-06 18:34 Heiner Kallweit
0 siblings, 0 replies; 6+ messages in thread
From: Heiner Kallweit @ 2014-10-06 18:34 UTC (permalink / raw)
To: linux-arm-kernel
imx_get_temp might be called before the sensor clock ist 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>
---
drivers/thermal/imx_thermal.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 461bf3d..ef50df6 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -521,20 +521,6 @@ static int imx_thermal_probe(struct platform_device *pdev)
return ret;
}
- data->tz = thermal_zone_device_register("imx_thermal_zone",
- IMX_TRIP_NUM,
- BIT(IMX_TRIP_PASSIVE), data,
- &imx_tz_ops, NULL,
- IMX_PASSIVE_DELAY,
- IMX_POLLING_DELAY);
- if (IS_ERR(data->tz)) {
- ret = PTR_ERR(data->tz);
- dev_err(&pdev->dev,
- "failed to register thermal zone device %d\n", ret);
- 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");
@@ -551,6 +537,20 @@ static int imx_thermal_probe(struct platform_device *pdev)
dev_warn(&pdev->dev, "failed to enable thermal clk: %d\n", ret);
}
+ data->tz = thermal_zone_device_register("imx_thermal_zone",
+ IMX_TRIP_NUM,
+ BIT(IMX_TRIP_PASSIVE), data,
+ &imx_tz_ops, NULL,
+ IMX_PASSIVE_DELAY,
+ IMX_POLLING_DELAY);
+ if (IS_ERR(data->tz)) {
+ ret = PTR_ERR(data->tz);
+ dev_err(&pdev->dev,
+ "failed to register thermal zone device %d\n", ret);
+ cpufreq_cooling_unregister(data->cdev);
+ return 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] 6+ messages in thread
end of thread, other threads:[~2014-10-08 5:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-06 18:37 [PATCH] imx: thermal: imx_get_temp might be called before the sensor clock ist prepared Heiner Kallweit
2014-10-07 13:04 ` Philipp Zabel
2014-10-07 19:36 ` Heiner Kallweit
2014-10-07 19:42 ` Fabio Estevam
2014-10-08 5:27 ` Heiner Kallweit
-- strict thread matches above, loose matches on Subject: below --
2014-10-06 18:34 [Patch] " Heiner Kallweit
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).