[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cf4d8131-4b63-4c7a-9f27-5a0847c656c4@notapiano>
Date: Fri, 19 Apr 2024 12:03:02 -0400
From: Nícolas F. R. A. Prado <nfraprado@...labora.com>
To: Sebastian Reichel <sre@...nel.org>
Cc: kernel@...labora.com, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org, Pin-yen Lin <treapking@...omium.org>,
Hsin-Te Yuan <yuanhsinte@...omium.org>
Subject: Re: [PATCH v3] power: supply: sbs-battery: Handle unsupported
PROP_TIME_TO_EMPTY_NOW
On Thu, Apr 18, 2024 at 01:34:23PM -0400, Nícolas F. R. A. Prado wrote:
> Despite the RunTimeToEmpty() (0x11) function being defined in the SBS
> specification as required, it seems that not all batteries implement it.
> On platforms with such batteries, reading the property will cause an
> error to be printed:
>
> power_supply sbs-8-000b: driver failed to report `time_to_empty_now' property: -5
>
> This not only pollutes the log, distracting from real problems on the
> device, but also prevents the uevent file from being read since it
> contains all properties, including the faulty one.
>
> The following table summarizes the findings for a handful of platforms:
>
> Platform Status Manufacturer Model
> ------------------------------------------------------------------------
> mt8186-corsola-steelix-sku131072 OK BYD L22B3PG0
> mt8195-cherry-tomato-r2 NOT OK PANASON AP16L5J
> mt8192-asurada-spherion-r0 NOT OK PANASON AP15O5L
> mt8183-kukui-jacuzzi-juniper-sku16 NOT OK LGC KT0 AP16L8J
> mt8173-elm-hana OK Sunwoda L18D3PG1
> sc7180-trogdor-lazor-limozeen-nots-r5 NOT OK Murata AP18C4K
> sc7180-trogdor-kingoftown NOT OK 333-AC-0D-A GG02047XL
> rk3399-gru-kevin OK SDI 4352D51
>
> Detect if this is one of the quirky batteries during presence update, so
> that hot-plugging works as expected, and if so report -ENODATA for
> POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW, which removes it from uevent and
> prevents throwing errors.
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@...labora.com>
> ---
Hi,
I'm coming back with more information after some more testing has been done.
Most importantly, in the meantime, a parallel investigation uncovered that the
time_to_empty_now issue was actually in the EC firmware:
https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/5465747
So the other faulty properties (which I'll mention below) could also be due to
the EC firmware. These are the EC firmware version for the platforms with
additional issues:
* RW version: juniper_v2.0.2509-9101a0730
* RW version: lazor_v2.0.6519-9923041f79
Hsin-Te, do you have information on whether it's an EC issue in this case as
well?
The following table shows all the faulty properties per platform:
Platform Manufacturer Model Faulty properties
---------------------------------------------------------------------------------
mt8186-corsola-steelix-sku131072 BYD L22B3PG0 -
mt8195-cherry-tomato-r2 PANASON AP16L5J time_to_empty_now
mt8192-asurada-spherion-r0 PANASON AP15O5L time_to_empty_now
mt8183-kukui-jacuzzi-juniper-sku16 LGC KT0 AP16L8J time_to_empty_now
capacity_error_margin
constant_charge_current_max
constant_charge_voltage_max
current_avg
technology
manufacture_year
manufacture_month
manufacture_day
SPEC_INFO
mt8173-elm-hana Sunwoda L18D3PG1 -
sc7180-trogdor-lazor-limozeen-nots-r5 Murata AP18C4K time_to_empty_now
capacity_error_margin
constant_charge_current_max
constant_charge_voltage_max
current_avg
sc7180-trogdor-kingoftown 333-AC-0D-A GG02047XL time_to_empty_now
rk3399-gru-kevin SDI 4352D51 -
If it turns out to not be an EC issue for the properties other than the
time_to_empty_now, then quirks will need to be added for them. As for SPEC_INFO
it's fine to keep it the way it is, as it already fails gracefully by falling
back to disabled PEC. However it does mean sbs_update_quirks() would need to be
moved up in sbs_update_presence(), or it will never run when SPEC_INFO fails.
Also, the battery vendor for limozeen is actually "Murata ", with a trailing
space...
While at it, I also tested whether PEC was broken on all platforms (which have
the SBS battery behind the EC I2C tunnel) to see if it could have any relation
with the faulty properties:
PEC
Platform Manufacturer Model Status
------------------------------------------------------------------------
mt8186-corsola-steelix-sku131072 BYD L22B3PG0 NOT SUPPORTED
mt8195-cherry-tomato-r2 PANASON AP16L5J NOT SUPPORTED
mt8192-asurada-spherion-r0 PANASON AP15O5L NOT SUPPORTED
mt8183-kukui-jacuzzi-juniper-sku16 LGC KT0 AP16L8J NOT SUPPORTED
mt8173-elm-hana Sunwoda L18D3PG1 BROKEN
sc7180-trogdor-lazor-limozeen-nots-r5 Murata AP18C4K NOT SUPPORTED
sc7180-trogdor-kingoftown 333-AC-0D-A GG02047XL NOT SUPPORTED
rk3399-gru-kevin SDI 4352D51 BROKEN
Where on the platforms marked BROKEN all properties would fail like so:
power_supply sbs-9-000b: driver failed to report `status' property: -74
Those platforms indeed had PEC enabled:
<6>[ 18.109211] sbs-battery 9-000b: PEC: enabled
and I verified the reported SBS version was SBS_VERSION_1_1_WITH_PEC.
Meanwhile, all the other platforms, marked NOT SUPPORTED, didn't actually have
PEC enabled:
<6>[ 14.563070] sbs-battery 8-000b: PEC: disabled
which I verified was due to version SBS_VERSION_1_0 being reported (except for
jacuzzi, which fails to report a version).
So all platforms that had batteries that support PEC, have broken PEC, but most
don't support it. In any case there doesn't seem to be a correlation with the
properties that the batteries support, so it looks to be an orthogonal issue.
Thanks,
Nícolas
Powered by blists - more mailing lists