Re: [PATCH v3] power: supply: sbs-battery: Handle unsupported PROP_TIME_TO_EMPTY_NOW

["Nícolas F. R. A. Prado" <nfraprado@collabora.com>]

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@collabora.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