Message-ID: <c0ccb3021324883fce5d87e27f5abdcccc96503a.camel@icenowy.me>
Date:   Tue, 01 Aug 2023 22:10:34 +0800
From:   Icenowy Zheng <uwu@...nowy.me>
To:     Chen-Yu Tsai <wenst@...omium.org>,
        Daniel Lezcano <daniel.lezcano@...aro.org>
Cc:     Mark Brown <broonie@...nel.org>, Amit Kucheria <amitk@...nel.org>,
        Zhang Rui <rui.zhang@...el.com>, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-sunxi@...ts.linux.dev
Subject: Re: [PATCH RESEND RESEND] thermal/of: support thermal zones w/o
 trips subnode

在 2023-07-24星期一的 12:25 +0800，Chen-Yu Tsai写道：
> On Sun, Jul 23, 2023 at 12:12:49PM +0200, Daniel Lezcano wrote:
> > 
> > Hi Mark,
> > 
> > On 22/07/2023 22:11, Mark Brown wrote:
> > > On Sat, Jul 22, 2023 at 08:25:34PM +0800, Icenowy Zheng wrote:
> > > > From: Icenowy Zheng <uwu@...nowy.me>
> > > > 
> > > > Although the current device tree binding of thermal zones
> > > > require the
> > > > trips subnode, the binding in kernel v5.15 does not require it,
> > > > and many
> > > > device trees shipped with the kernel, for example,
> > > > allwinner/sun50i-a64.dtsi and mediatek/mt8183-kukui.dtsi in
> > > > ARM64, still
> > > > comply to the old binding and contain no trips subnode.
> > > > 
> > > > Allow the code to successfully register thermal zones w/o trips
> > > > subnode
> > > > for DT binding compatibility now.
> > > > 
> > > > Furtherly, the inconsistency between DTs and bindings should be
> > > > resolved
> > > > by either adding empty trips subnode or dropping the trips
> > > > subnode
> > > > requirement.
> > > 
> > > This makes sense to me - it allows people to see the reported
> > > temperature even if there's no trips defined which seems more
> > > helpful than refusing to register.
> > 
> > The binding describes the trip points as required and that since
> > the
> > beginning.
> 
> Not really. It was made optional in the v5.15 kernel release by
> commit
> 
>     22fc857538c3 dt-bindings: thermal: Make trips node optional

I agree, this is why I send this patch (and why I say 'for DT binding
compatibility now' in the commit message). Further discussion could be
performed, but this patch should be applied regardless of the result of
further discussion.

DT binding compatibility is the unbreakable law.

> 
> > What changed is now the code reflects the required property while
> > before it
> > was permissive, that was an oversight.
> > 
> > Just a reminder about the thermal framework goals:
> > 
> >   1. It protects the silicon (thus critical and hot trip points)
> > 
> >   2. It mitigates the temperature (thus cooling device bound to
> > trip points)
> > 
> >   3. It notifies the userspace when a trip point is crossed
> > 
> > So if the thermal zone is described but without any of this goal
> > above, it
> > is pointless.
> > 
> > If the goal is to report the temperature only, then hwmon should be
> > used
> > instead.
> 
> What about thermal sensors with multiple channels? Some of the
> channels
> are indeed tied to important hardware blocks like the CPU cores and
> should be tied into the thermal tripping. However other channels
> might
> only be used for temperature read-out and have no such requirement.
> 
> Should we be mixing thermal and hwmon APIs in the driver?

Well you have no right to decide which sensor should be used for
throttling and which not. So the only way to make the semantic correct
is just rip every sensor driver out of thermal API to hwmon API, and
let thermal framework to use hwmon's.

> 
> > If the goal is to mitigate by userspace, then the trip point *must*
> > be used
> > to prevent the userspace polling the temperature. With the trip
> > point the
> > sensor will be set to fire an interrupt at the given trip
> > temperature.
> > 
> > IOW, trip points are not optional
> 
> for measurement points that are used for thermal throttling /
> mitigation.
> 
> ChenYu
> 

Powered by blists - more mailing lists