RE: [PATCH 2/3] thermal: qoriq_thermal: only enable supported sensors

quoted

Hi Daniel,

Subject: RE: [PATCH 2/3] thermal: qoriq_thermal: only enable supported
sensors

Subject: Re: [PATCH 2/3] thermal: qoriq_thermal: only enable supported
sensors

On 31/05/2023 14:05, Peng Fan wrote:
Subject: Re: [PATCH 2/3] thermal: qoriq_thermal: only enable
supported sensors

On 16/05/2023 10:37, Peng Fan (OSS) wrote:
From: Peng Fan <peng.fan@nxp.com>

There are MAX 16 sensors, but not all of them supported. Such as
i.MX8MQ, there are only 3 sensors. Enabling all 16 sensors will
touch reserved bits from i.MX8MQ reference mannual, and TMU will
stuck, temperature will not update anymore.

Fixes: 45038e03d633 ("thermal: qoriq: Enable all sensors before
registering them")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
   drivers/thermal/qoriq_thermal.c | 30 +++++++++++++++++++----------
-
   1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/thermal/qoriq_thermal.c
b/drivers/thermal/qoriq_thermal.c index
b806a0929459..53748c4a5be1
100644
--- a/drivers/thermal/qoriq_thermal.c
+++ b/drivers/thermal/qoriq_thermal.c
@@ -31,7 +31,6 @@
   #define TMR_DISABLE	0x0
   #define TMR_ME		0x80000000
   #define TMR_ALPF	0x0c000000
-#define TMR_MSITE_ALL	GENMASK(15, 0)

   #define REGS_TMTMIR	0x008	/* Temperature measurement
interval Register */
   #define TMTMIR_DEFAULT	0x0000000f
@@ -105,6 +104,11 @@ static int tmu_get_temp(struct
thermal_zone_device *tz, int *temp)
   	 * within sensor range. TEMP is an 9 bit value representing
   	 * temperature in KelVin.
   	 */
+
+	regmap_read(qdata->regmap, REGS_TMR, &val);
+	if (!(val & TMR_ME))
+		return -EAGAIN;
How is this change related to what is described in the changelog?
devm_thermal_zone_of_sensor_register will invoke get temp, since we
reverted the 45038e03d633 did, we need to check TMR_ME to avoid
return
invalid temperature.

 From a higher perspective if the sensor won't be enabled, then the
thermal zone should not be registered, the get_temp won't happen on a
disabled sensor and this test won't be necessary, no ?
After thinking more, I'd prefer current logic.

We rely on devm_thermal_of_zone_register's return value to know
whether there is a valid zone, then set sites bit, and after collected
all site bits, we enable the thermal IP.

If move the enabling thermal IP before devm_thermal_of_zone_register,
We need check dtb thermal zone, to know which zone is valid for current
thermal IP. This would complicate the design.

So just checking the enabling bit in get temperature would be much
simpler, and there just a small window before enabling thermal IP.

Thanks,
Peng.

Agree, let me move the temp sensor enabling before register thermal zone.

Thanks,
Peng.

--
<https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
w.linaro.org%2F&data=05%7C01%7Cpeng.fan%40nxp.com%7C2a5070b5049
14957160008db61d37861%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
C0%7C638211333111330055%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%
7C%7C%7C&sdata=aJBF2UEqaCAqcAbHcKzbGReVv6pbYlyQ25riVxEdG08%3D
&reserved=0> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:
<https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
w.facebook.com%2Fpages%2FLinaro&data=05%7C01%7Cpeng.fan%40nxp.c
om%7C2a5070b504914957160008db61d37861%7C686ea1d3bc2b4c6fa92cd
99c5c301635%7C0%7C0%7C638211333111330055%7CUnknown%7CTWFpb
GZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI
6Mn0%3D%7C3000%7C%7C%7C&sdata=9l627puhL7hQgMlPWaKkCIDkQKGX
TH49rEM0NFipvDs%3D&reserved=0> Facebook |
<https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwit
t
er.com%2F%23!%2Flinaroorg&data=05%7C01%7Cpeng.fan%40nxp.com%7C
2a5070b504914957160008db61d37861%7C686ea1d3bc2b4c6fa92cd99c5c30
1635%7C0%7C0%7C638211333111330055%7CUnknown%7CTWFpbGZsb3d8
eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
D%7C3000%7C%7C%7C&sdata=%2B0Oa%2BrxHmGPga0%2BGQjOOX6Dxaiuj
oPJhzwqBjjO%2B2Qo%3D&reserved=0> Twitter |
<https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
w.linaro.org%2Flinaro-
blog%2F&data=05%7C01%7Cpeng.fan%40nxp.com%7C2a5070b5049149571
60008db61d37861%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C
638211333111330055%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C
%7C&sdata=2lfwzPj3p%2BjowtVDc03plo1Ds%2BnT%2B2eSwdTQ9yQosfo
%3
D&reserved=0> Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help