mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Henrik Grimler <henrik.grimler@axis.com>
To: Ryan Brue <ryanbrue.dev@gmail.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: Tue, 22 Sep 2026 08:51:21 +0200	[thread overview]
Message-ID: <20260922065121.GA70591@lap5cd525d1j1.sto.se.axis.com> (raw)
In-Reply-To: <fff86d6b-e4e6-4d90-8373-3ee32432807a@gmail.com>

Hi Ryan,

On Sun, Sep 20, 2026 at 01:42:02AM -0500, Ryan Brue wrote:
> 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 see, thanks for explaining.

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

Interpolating the vendor table to fit 100 points would be the way to
go in my opinion.

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

Changing the limit to 101, and adding a comment about why, sound good
to me!

> Best regards,
> Ryan Brue

Best regards,
Henrik Grimler

      reply	other threads:[~2026-09-22  6:51 UTC|newest]

Thread overview: 4+ 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
2026-09-22  6:51     ` Henrik Grimler [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=20260922065121.GA70591@lap5cd525d1j1.sto.se.axis.com \
    --to=henrik.grimler@axis.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=ryanbrue.dev@gmail.com \
    --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®