linux-kernel - Re: [PATCH v2] thermal: core: Call monitor_thermal

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <5ad6a075-7280-4280-a797-cf04a1df0830@linaro.org>
Date: Thu, 4 Jul 2024 10:24:00 +0200
From: neil.armstrong@...aro.org
To: Daniel Lezcano <daniel.lezcano@...aro.org>,
 "Rafael J. Wysocki" <rjw@...ysocki.net>, Linux PM <linux-pm@...r.kernel.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Lukasz Luba <lukasz.luba@....com>,
 Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
 Zhang Rui <rui.zhang@...el.com>,
 linux-arm-msm <linux-arm-msm@...r.kernel.org>
Subject: Re: [PATCH v2] thermal: core: Call monitor_thermal_zone() if zone
 temperature is invalid

On 04/07/2024 09:57, Daniel Lezcano wrote:
> On 04/07/2024 09:39, neil.armstrong@...aro.org wrote:
> 
> [ ... ]
> 
>> OK I just found out, it's the `qcom-battmgr-bat` thermal zone, and in CI we do not have the firmwares so the
>> temperature is never available, this is why it fails in a loop.
>>
>> Before this patch it would fail silently, but would be useless if we start the firmware too late.
>>
>> So since it's firmware based, valid data could arrive very late in the boot stage, and sending an
>> error message in a loop until the firmware isn't started doesn't seem right.
> 
> Yeah, there was a similar bug with iwlwifi. They fixed it by registering the thermal zone after the firmware was successfully loaded.
> 
> Is that possible to do the same ?

The thermal zone is indirect, it's registered via power_supply_core.

A tentative was done to delay registering the power supply , since it caused issues in suspend/resume,
but it was reverted because it would require much more work:
https://lore.kernel.org/all/20240123160053.18331-1-johan+linaro@kernel.org/

Seems we should instead return -EAGAIN instead of -ENODEV in qcom_battmgr_bat_get_property(),

But I think power_supply_read_temp() should return -EAGAIN on -ENODEV, since it's the return
code for when a power supply isn't initialized.

Neil

> 
>> I think Rafael's new patch is good, but perhaps it should send an error when it finally stops monitoring.
> 
> 
>