| 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: <edb0ba399467359e16e50c89a1671b7f@manjaro.org> Date: Fri, 01 Mar 2024 09:21:04 +0100 From: Dragan Simic <dsimic@...jaro.org> To: Alexey Charkov <alchark@...il.com> Cc: Rob Herring <robh+dt@...nel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>, Heiko Stuebner <heiko@...ech.de>, Daniel Lezcano <daniel.lezcano@...aro.org>, Viresh Kumar <viresh.kumar@...aro.org>, Chen-Yu Tsai <wens@...nel.org>, devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH v3 1/5] arm64: dts: rockchip: enable built-in thermal monitoring on RK3588 On 2024-03-01 08:51, Alexey Charkov wrote: > On Fri, Mar 1, 2024 at 10:14 AM Dragan Simic <dsimic@...jaro.org> > wrote: >> On 2024-03-01 06:20, Alexey Charkov wrote: >> > On Fri, Mar 1, 2024 at 1:11 AM Dragan Simic <dsimic@...jaro.org> wrote: >> >> See, I'm not a native English speaker, but I've spent a lot of time >> >> and effort improving my English skills. Thus, perhaps these comments >> >> may or may not seem like unnecessary nitpicking, depending on how much >> >> someone pays attention to writing style in general, but I'll risk to >> >> be annoying and state these comments anyway. :) >> >> >> >> The comment above could be written in a much more condensed form like >> >> this, which would also be a bit more accurate: >> >> >> >> >> >> /* IPA threshold, when IPA governor is >> >> used */ >> >> >> >> IOW, we're writing all this for someone to read later, but we should >> >> (and can) perfectly reasonably expect some already existing background >> >> knowledge from the readers. In other words, we should be as concise >> >> as possible. >> > >> > In fact, the power allocation governor code itself doesn't call those >> > trips threshold or target as your suggested wording would imply. >> > Instead, it calls them "switch on temperature" and "maximum desired >> > temperature" [1]. Maybe we can call them that in the comments (and >> > also avoid calling the governor IPA, because upstream code only calls >> > it a "power allocator"). >> >> Hmm, but "IPA" is still mentioned in exactly three places in the files >> under drivers/thermal. I think that warrants the use of "IPA", which >> is also widely used pretty much everywhere. >> >> Perhaps a win-win would be to have only the very first of the comments >> like this, to introduce "IPA" as an acronym: >> >> /* Power allocator (IPA) thermal >> governor */ >> /* switch-on point, when IPA >> governor >> is used */ > > Yes, good point, thanks! I'm glad that you agree. :) >> Next, "the target temperature" is mentioned more than a few times in >> drivers/thermal/gov_power_allocator.c, which I believe makes the use >> of "IPA target" perfectly valid. Actually, let's use "IPA target >> temperature", if you agree, to make it self descriptive. > > Or perhaps simply "target temperature"? Stepwise governor will also > use this trip as its target, so it's not IPA specific, unlike the > switch-on point. I also had similar thoughts about the shared nature. I agree, just "/* target temperature */" would be fine. >> Finally, the threshold... Based on >> drivers/thermal/gov_power_allocator.c, >> I think "IPA switch-on point" would be a good choice, which I already >> used above in the proposed opening comment. > > Agreed, that sounds good to me, will reflect in the next iteration. > Thanks for bringing it up! Great, thanks!
Powered by blists - more mailing lists