> Subject: Re: [PATCH 1/4] arm64: dts: imx8mm-venice-gw700x: override > thermal cfg for industrial temp > > On 07.06.21 09:30, Jacky Bai wrote: > >> Subject: Re: [PATCH 1/4] arm64: dts: imx8mm-venice-gw700x: override > >> thermal cfg for industrial temp > >> > >> On 04.06.21 17:42, Tim Harvey wrote: > >>> On Wed, Jun 2, 2021 at 12:11 AM Frieder Schrempf > >>> <frieder.schrempf@xxxxxxxxxx> 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@xxxxxxxxxxxxx> > >>>>> --- > >>>>> .../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]. > >>>> > > ... > >>> > >>> 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. > > > > I thought to add the grading check in the imx8mm_thermal.c to > > dynamically set the trip points temp, but it seems hard to do it due > > to the fact of_thermal is used, as no helper API is exported by of_thermal, > no better way to override the trip point temp. > > > > glad to see any good suggestions. > > Right, the driver doesn't handle the trip-points directly. This is all hidden in the > framework. So this might not be so easy to implement. > > What about this other approach: Adding all the possible trip-points for the > different gradings to the SoC-devicetree and then let the thermal driver > remove the trip nodes from the dt that are not valid for the detected grading, > just before the driver registers the sensor/zone. It is more reasonable for the firmware/bootloader to handle this by checking the grading. BR Jacky Bai