linux-kernel - Re: [PATCH v2] arm64: dts: qcom: sc8280xp: Introduce additional tsens instances

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZbdyBgbX2Su80eln@hovoldconsulting.com>
Date: Mon, 29 Jan 2024 10:38:14 +0100
From: Johan Hovold <johan@...nel.org>
To: Konrad Dybcio <konrad.dybcio@...aro.org>
Cc: Bjorn Andersson <quic_bjorande@...cinc.com>,
	Bjorn Andersson <andersson@...nel.org>,
	Rob Herring <robh+dt@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>, linux-arm-msm@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] arm64: dts: qcom: sc8280xp: Introduce additional
 tsens instances

On Fri, Jan 26, 2024 at 10:23:41PM +0100, Konrad Dybcio wrote:
> On 26.01.2024 18:00, Johan Hovold wrote:
> > On Fri, Jan 26, 2024 at 08:51:13AM -0800, Bjorn Andersson wrote:
> >> On Fri, Jan 26, 2024 at 05:36:10PM +0100, Johan Hovold wrote:
> > 
> >>> Shall you submit a follow-on patch to set the polling delays to zero
> >>> for the other thermal zones (cpu, cluster, mem) so that we don't poll
> >>> for those?
> >>
> >> I optimistically interpreted Konrad's response as a promise by him to do
> >> so ;)
> >>
> >> I do like his patch which remove the poll-properties for non-polling
> >> mode. Would be nice to not first change the values to 0 and then remove
> >> the properties...
> 
> That was my intention as well..
> 
> > 
> > No, that should not be an issue as it allows us to get rid of the
> > polling without waiting for a binding update which may or may not
> > materialise in 6.9-rc1.
> 
> If you really insist, I may do that, but if the thermal guys act on it
> quickly and we negotiate an immutable branch, we can simply but atop it,
> saving the submitter timeof(patchset), the reviewers timeof(verify), the
> build bots timeof(builds) and the applier timeof(pick-build-push)..

Why would introduce such a dependency for really no good reason?

Submit a patch based on the current binding, then when/if your proposed
binding update hits mainline, you can send a *single* patch dropping the
parameters from all qualcomm dtsi.

Updating the binding is a separate and lower priority task. In fact, it
may not even be desirable at all as an omission of adding these
parameters could then lead to broken thermal management on platforms
where the interrupts do not work. Having an explicit poll-delay of zero
at least gives people a reason to think about it when merging a new
platform.

But again, that's a separate discussion. Don't make this patch depend on
that.

> > But whoever updates those properties need to do some proper testing to
> > make sure that those interrupts really work.
> 
> They seem to, check /proc/interrupts before and after adding an e.g. 45degC
> trip point on one of the CPU thermal zones, they fire aplenty.

That's not proper testing. Add/enable debugging in the thermal driver
and make sure that you trigger precisely once when passing the threshold
in both directions.

Johan