| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <DU0PR04MB9417D574603B54AEF76480AE88499@DU0PR04MB9417.eurprd04.prod.outlook.com>
Date: Thu, 1 Jun 2023 09:52:01 +0000
From: Peng Fan <peng.fan@....com>
To: Daniel Lezcano <daniel.lezcano@...aro.org>,
"Peng Fan (OSS)" <peng.fan@....nxp.com>,
"rafael@...nel.org" <rafael@...nel.org>,
"shawnguo@...nel.org" <shawnguo@...nel.org>,
"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>
CC: "amitk@...nel.org" <amitk@...nel.org>,
"rui.zhang@...el.com" <rui.zhang@...el.com>,
"andrew.smirnov@...il.com" <andrew.smirnov@...il.com>,
"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"kernel@...gutronix.de" <kernel@...gutronix.de>,
"festevam@...il.com" <festevam@...il.com>,
dl-linux-imx <linux-imx@....com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
Alice Guo <alice.guo@....com>
Subject: RE: [PATCH 2/3] thermal: qoriq_thermal: only enable supported sensors
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@....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@....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
Powered by blists - more mailing lists