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

[Alper Nebi Yasak <alpernebiyasak@gmail.com>]

Hi,

On 2024-05-13 16:27 +03:00, Nícolas F. R. A. Prado wrote:
> 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.

I still see this error on a cozmo, even after doing a ChromeOS recovery 
to upgrade EC firmware. (Also, some properties sometimes error with -6). 
Looks like Google did not release an updated version with that patch:

  $ sudo ectool version
  RO version:    cozmo_v2.0.9006-689870d95c
  RW version:    cozmo_v2.0.9006-689870d95c
  Firmware copy: RW
  Build info:    cozmo_v2.0.9006-689870d95c 2022-06-14 10:16:42 @chromeos-ci-firmware-us-central1-b-x32-0-he51

  $ sudo ectool battery
  Battery info:
    OEM name:               PANASON
    Model number:           AP19B5K
    Chemistry   :           LION
    Serial number:          38D5
    Design capacity:        3440 mAh
    Last full charge:       2558 mAh
    Design output voltage   11550 mV
    Cycle count             19
    Present voltage         11607 mV
    Present current         243 mA
    Remaining capacity      2142 mAh
    Flags                   0x06 BATT_PRESENT DISCHARGING

I hope you can ping someone to release a new firmware build (probably 
firmware-icarus-12574.B)? I checked `chromeos-firmwareupdate --manifest` 
as well, but mine matches the version there.

But upgrading firmware would be an ordeal for people who replaced 
ChromeOS with an ordinary Linux distro on the internal disk. ChromeOS 
recovery would wipe their non-ChromeOS system, so they might need to 
figure out how to safely manually upgrade firmware (thinking VPD and A/B 
flags).

Even then, the RO EC firmware will forever carry the EC bug on 
most devices. For example, one of my hana boards goes into a bootloop 
failing to sync EC firmware, where I had to disable that and only 
use RO EC firmware. And we will eventually have non-ChromeOS firmware 
for these devices, which might not properly handle EC firmware.

Please consider fixing it in the kernel as well.