mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ryan Brue <ryanbrue.dev@gmail.com>
To: Henrik Grimler <henrik.grimler@axis.com>
Cc: Sebastian Reichel <sre@kernel.org>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	linux-pm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] dt-bindings: power: supply: battery: allow longer ocv-capacity tables
Date: Sun, 20 Sep 2026 01:42:02 -0500	[thread overview]
Message-ID: <fff86d6b-e4e6-4d90-8373-3ee32432807a@gmail.com> (raw)
In-Reply-To: <20260918082921.GA1515928@pc67698-2615.sto.se.axis.com>

On 9/18/26 3:29 AM, Henrik Grimler wrote:
> Hi Ryan,
>
> On Fri, Sep 18, 2026 at 12:36:31AM -0500, Ryan Brue wrote:
>> ocv-capacity-table-N has been capped at 100 points since battery.txt was
>> converted to YAML, where the limit arrived without a stated reason.
>>
>> The MT6397 fuel gauge is characterised per temperature by a table the
>> Amazon Fire HD 10 (2017) vendor device tree carries with 126 points, of
>> which 122 are expressible here - the remainder are greater than 100%
>> discharged, so the binding excludes those points. Boards carrying this
>> PMIC fuel gauge would need more than 100 points to describe the pack with
>> the generic property. Raise the cap to 128.
>>
>> Assisted-by: LLM
>> Signed-off-by: Ryan Brue <ryanbrue.dev@gmail.com>
>> ---
>> No kernel change goes with this. power_supply_get_battery_info() sizes each
>> ocv-capacity-table-N from the property itself -- it reads the length with
>> fwnode_property_count_u32() and devm_kcalloc()s that many entries -- so
>> maxItems in the binding is the only cap on points per table.
>> POWER_SUPPLY_OCV_TEMP_MAX bounds the number of tables, not their length.
>>
>> The consumer that wants this is an MT6397 PMIC fuel gauge not yet posted;
>> its pack is characterised at 126 points per temperature in the vendor's
>> kernel (Amazon Fire OS, based on Linux 3.18), with 122 of those points
>> being expressible with the generic property (the rest are greater than
>> 100%).
> Allowing for points > 100 % could make sense, but why would you need
> 122 points up to 100 %? If the vendor kernel has several values at for
> example 20 %, then a better solution is probably to take the average
> of them.
>
> I think only reason to have multiple values for the same percentage
> would be if hysterersis (see for example this open-access article [1]
> for discussion about hysteresis) is taken into account, i.e. having
> one table for charge direction, and one table for discharge direction,
> but I don't think any driver uses multiple tables to handle something
> like that.
>
> [1] https://doi.org/10.1038/s41598-019-51474-5
>
> Best regards,
> Henrik Grimler
Hi Henrik,
Yeah, you're right. To be honest, I didn't think about that, and should 
have.

On why there's so many points: the vendor's table isn't indexed by 
percentage at all. A row is a fixed 54 mAh step of charge - 
step_of_qmax, which the meter converts to mAh directly - and the 
percentage column is just that rounded, round(i * 54 * 100 / Qmax), 
which fits every row of all five tables exactly. At 0.85% per row about 
one in six repeats, so the duplicates carry nothing. I should also 
correct the figures I sent: 126/122 is a different cell in the same 
vendor file. This unit's tables are 120 points and none of the points 
are above 100% in this one.

I don't need to model the vendor one to one. I measured what dropping 
resolution costs, and decimating to 100 points changes the capacity I 
report by at most 1% - so it fits the binding as it stands, and the 
justification I sent doesn't hold.

The only thing I can see still being worth raising is 101 rather than 
128. Capacity percent is capped at 100, and 0..100 inclusive is 101 
values, so if I'm not mistaken no board can express 1% granularity today 
if they have points at both 0 and 100. I'm not sure that's worth a patch 
on its own, so I'm fine dropping this, or doing a v2 allowing 101 values.

Best regards,
Ryan Brue

      reply	other threads:[~2026-09-20  6:42 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-18  5:36 Ryan Brue
2026-09-18  8:29 ` Henrik Grimler
2026-09-20  6:42   ` Ryan Brue [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fff86d6b-e4e6-4d90-8373-3ee32432807a@gmail.com \
    --to=ryanbrue.dev@gmail.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=henrik.grimler@axis.com \
    --cc=krzk+dt@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sre@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®