linux-kernel - Re: [PATCH v3] power: supply: sbs-battery: Handle unsupported PROP_TIME_TO_EMPTY

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: <1db95251-04bb-4d4f-b77b-3b78a8f497cd@notapiano>
Date: Mon, 13 May 2024 09:27:18 -0400
From: Nícolas F. R. A. Prado <nfraprado@...labora.com>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: Hsin-Te Yuan <yuanhsinte@...omium.org>,
	Sebastian Reichel <sre@...nel.org>, kernel@...labora.com,
	linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
	Pin-yen Lin <treapking@...omium.org>
Subject: Re: [PATCH v3] power: supply: sbs-battery: Handle unsupported
 PROP_TIME_TO_EMPTY_NOW

On Thu, May 09, 2024 at 05:43:42PM +0200, AngeloGioacchino Del Regno wrote:
> Il 09/05/24 17:25, Nícolas F. R. A. Prado ha scritto:
> > On Mon, Apr 22, 2024 at 04:10:23PM +0800, Hsin-Te Yuan wrote:
> > > On Sat, Apr 20, 2024 at 12:03 AM Nícolas F. R. A. Prado
> > > <nfraprado@...labora.com> wrote:
> > > > 
> > > > On Thu, Apr 18, 2024 at 01:34:23PM -0400, Nícolas F. R. A. Prado wrote:
[..]
> > 
> > Getting back on this, we were finally able to update the EC firmware for both
> > juniper and limozeen and all the issues were fixed. I have added the logs below
> > just for reference. So I guess the only change we could have upstream would be a
> > message suggesting the user to update the EC firmware in case the SBS is behind
> > the CrosEC and it starts throwing errors. I'll prepare a patch for that.
> > 
> 
> ...yes, but then you can't do that in the sbs-battery driver, but rather in the
> CrOS EC - so you'd have to link this and the other driver (beware: I'm not
> proposing to do that!), which wouldn't be the cleanest of options.

I *was* actually thinking of adding the log in the sbs driver by checking the
parent's compatible, since that's already done elsewhere in that driver to
disable PEC:

	if (of_device_is_compatible(client->dev.parent->of_node, "google,cros-ec-i2c-tunnel")

But now that you mention it, indeed if we're only printing a warning, it would
be best to do it in the EC i2c tunnel driver. And that's all that I'm proposing
to do: log a warning telling the user to update their EC firmware, as that
should fix the readouts, and not add any quirk to the driver.

Thanks,
Nícolas

> 
> Perhaps we could check "how many times *in a row, from boot*" the readout is
> failing and dynamically add the quirk with a big pr_warn().
> 
> I guess that could work but, at the same time, that's code to engineer very
> carefully, or we'd risk breaking machines that would get that reading to work,
> for example, only after a suspend-resume cycle (which is bad, yes, but still)
> or other oddities...
> 
> Any other ideas?
> 
> Cheers,
> Angelo