Re: [PATCH 1/4] arm64: dts: imx8mm-venice-gw700x: override thermal cfg for industrial temp

From: Frieder Schrempf <frieder.schrempf@kontron.de>
To: Tim Harvey <tharvey@gateworks.com>
Cc: Rob Herring <robh+dt@kernel.org>, Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Device Tree Mailing List <devicetree@vger.kernel.org>,
	Linux ARM Mailing List <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/4] arm64: dts: imx8mm-venice-gw700x: override thermal cfg for industrial temp
Date: Mon, 7 Jun 2021 09:20:39 +0200	[thread overview]
Message-ID: <5eba4438-239c-71d1-56ec-4edd70e87ec3@kontron.de> (raw)
In-Reply-To: <CAJ+vNU0mBQJ_ZLsKOiY0k7+drYvmw+bsHfeX4+Mg=s4_fxSUYA@mail.gmail.com>

On 04.06.21 17:42, Tim Harvey wrote:
> On Wed, Jun 2, 2021 at 12:11 AM Frieder Schrempf
> <frieder.schrempf@kontron.de> wrote:
>>
>> On 01.06.21 19:49, Tim Harvey wrote:
>>> Override the default temperature alert/crit for Industrial temp IMX8M
>>> Mini.
>>>
>>> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
>>> ---
>>>  .../boot/dts/freescale/imx8mm-venice-gw700x.dtsi     | 12 ++++++++++++
>>>  1 file changed, 12 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-venice-gw700x.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw700x.dtsi
>>> index c769fadbd008..512b76cd7c3b 100644
>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-venice-gw700x.dtsi
>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-venice-gw700x.dtsi
>>> @@ -493,3 +493,15 @@
>>>               >;
>>>       };
>>>  };
>>> +
>>> +&cpu_alert0 {
>>> +     temperature = <95000>;
>>> +     hysteresis = <2000>;
>>> +     type = "passive";
>>> +};
>>> +
>>> +&cpu_crit0 {
>>> +     temperature = <105000>;
>>> +     hysteresis = <2000>;
>>> +     type = "critical";
>>> +};
>>
>> As this is not really board-specific, I think the proper way to handle this for all boards is to let the thermal driver read the temperature grading from the OTP fuses and set the trip-points accordingly, similar to what is done on i.MX6 [1].
>>
>> [1] https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fdrivers%2Fthermal%2Fimx_thermal.c%3Fh%3Dv5.13-rc4%23n508&amp;data=04%7C01%7Cfrieder.schrempf%40kontron.de%7C7635e5c10c3e4d3fba3208d9276f5426%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C637584181370793415%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=1Ed1rcaki%2FIPpFmii%2FEVc%2FAzPkzTzS9m8nplyFS90s8%3D&amp;reserved=0
> 
> Frieder,
> 
> Yes, I thought about adding that kind of support to imx8mm_thermal.c
> but the difference is that imx8mm has alerts defined by dt and imx6
> does not so is it right to override dt alerts on imx8m? What if
> someone designs a board that they specifically want a lower alert than
> the cpu grade they are using based on something else on the board?
> 
> My approach to this was to eventually actually adjust the imx8m dt
> alerts in boot firmware based on some boot firmware setting or
> specific board support and leave the kernel alone.

Allowing board-specific trip points sounds like a valid request, but I still think we need a way to handle the temperature grading in the driver if no board-specific trip-points are given.

What if we just set the temperature property in the trip nodes in imx8mm.dtsi to zero? The thermal driver would detect this and setup the correct values according to the grading. If the dt already provides non-zero temperature values (through the board dts) the driver will just leave the values and disregard the grading.

I think this solution would be covering all needs.

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