Received-SPF: pass (google.com: domain of linux-kernel+bounces-177627-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Mon, 13 May 2024 09:27:18 -0400
From: =?utf-8?B?TsOtY29sYXMgRi4gUi4gQS4=?= Prado <nfraprado@collabora.com>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: Hsin-Te Yuan <yuanhsinte@chromium.org>,
	Sebastian Reichel <sre@kernel.org>, kernel@collabora.com,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Pin-yen Lin <treapking@chromium.org>
Subject: Re: [PATCH v3] power: supply: sbs-battery: Handle unsupported
 PROP_TIME_TO_EMPTY_NOW
Message-ID: <1db95251-04bb-4d4f-b77b-3b78a8f497cd@notapiano>
References: <20240418-sbs-time-empty-now-error-v3-1-f286e29e3fca@collabora.com>
 <cf4d8131-4b63-4c7a-9f27-5a0847c656c4@notapiano>
 <CAHc4DNJ0prAQOw89Hvw8n9KhY+8xB3D77pJvoPfU-X7ZFDYu7Q@mail.gmail.com>
 <924db470-8163-4454-8f59-f7372a132186@notapiano>
 <c721f2b9-2b08-45f3-adb5-09b163924fbc@collabora.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <c721f2b9-2b08-45f3-adb5-09b163924fbc@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