Linux USB
 help / color / mirror / Atom feed
From: Amit Sunil Dhamne <amitsd@google.com>
To: Sebastian Reichel <sebastian.reichel@collabora.com>
Cc: "Badhri Jagan Sridharan" <badhri@google.com>,
	"Heikki Krogerus" <heikki.krogerus@linux.intel.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Hans de Goede" <hansg@kernel.org>,
	"Krzysztof Kozlowski" <krzk@kernel.org>,
	"Marek Szyprowski" <m.szyprowski@samsung.com>,
	"Sebastian Krzyszkowiak" <sebastian.krzyszkowiak@puri.sm>,
	"Purism Kernel Team" <kernel@puri.sm>,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-usb@vger.kernel.org,
	"André Draszik" <andre.draszik@linaro.org>,
	"Tudor Ambarus" <tudor.ambarus@linaro.org>,
	"Peter Griffin" <peter.griffin@linaro.org>,
	"RD Babiera" <rdbabiera@google.com>,
	"Kyle Tso" <kyletso@google.com>
Subject: Re: [PATCH v3 0/2] Add support for Battery Status AMS
Date: Mon, 8 Jun 2026 12:20:25 -0700	[thread overview]
Message-ID: <6ea3d71f-ec81-44b7-a0f7-d10adcb4c834@google.com> (raw)
In-Reply-To: <aiG1aJY7VURSJOZ2@venus>

Hi Sebastian,

On 6/4/26 10:36 AM, Sebastian Reichel wrote:
> Hi,
>
> On Tue, Jun 02, 2026 at 10:47:05PM +0000, Amit Sunil Dhamne via B4 Relay wrote:
>> PD 3.1 v1.8 Spec necessitates a response to Get_Battery_Status request
>> from the port partner (see "6.13.2 Applicability of Data Message").
>> This patchset adds support to get all the battery type power supplies
>> and query them to report the telemetry required to build a Battery
>> Status Message. Right now, this submission assumes all the battery type
>> power supplies that exist in the system are fixed (meaning cannot be hot
>> swapped).
>>
>> Previously, I had sent a patch series [1]. However there were some
>> concerns. Broadly:
>>   * No client drivers
>>   * Duplicating dt properties
>> To address the above issues, we now have Fuel Gauge and Charger drivers.
>> Also, I have rectified my approach to fetch information about batteries
>> from the power supply core.
>>
>> While, the original patch series [1] added support for Battery Caps as
>> well, this patch series only adds support for Battery Status. Therefore,
>> I am sending it as a new series while incorporating relevant feedback.
>>
>> [1] https://lore.kernel.org/all/20250507-batt_ops-v2-0-8d06130bffe6@google.com/
>>
>> Patches in series:
>> [A] "power: supply: Add helpers to get and put arrays of power supply handles"
>> [B] "usb: typec: tcpm: Add support for Battery Status response message"
>>
>> Technical dependency of patches:
>> [B] depends on [A] due to usage of `power_supply_get_battery_all` &
>> `power_supply_put_battery_all` APIs.
>>
>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>> ---
> I think you want to filter on batteries that are
> POWER_SUPPLY_SCOPE_SYSTEM? Otherwise this would also give you
> battery devices for something like a cordless mouse.

Thanks for pointing that out. Which one would you prefer?:

1. Bake this into the power_supply_get_battery_all() implementation by
either passing the scope as an argument or directly implementing the
filtering on POWER_SUPPLY_SCOPE_SYSTEM without the argument.

2. power_supply_get_battery_all() returns all battery type power
supplies but tcpm filters them. Note that tcpm still keeps all
references to all batteries because of the review comments in [2].


[2]
https://lore.kernel.org/all/4e4a63a4-9d07-4b77-a8dc-ba19c9a803f7@kernel.org/

> You mention that this is assuming batteries to be always present,
> but handle POWER_SUPPLY_PROP_PRESENT. So basically a battery, which
> has POWER_SUPPLY_PROP_PRESENT=0 would violate the PD spec as Fixed
> Batteries are not supposed to have this unset?

Just to clarify, I meant fixed and not "always present". The spec
defines fixed battery as:

"A Battery that is not easily removed or replaced by an end user e.g.,
requires a special tool to access or is soldered in."

Theoretically, you could still have a phone with the fixed battery
removed and the system powered by USB.

So, while this is more relevant to hot-swappable battery case (, which
is defined by the spec as: "A Battery that is easily accessible for a
user to remove or change for another Battery.") it wouldn't be totally
off the mark to add that check here.

So while I lean on having the check present, I am okay either way.


> Not sure if this is fixable, but you implicitly rely on the battery
> driver to be probed before TCPM reaches this.


Given the power supply architecture, the probe order is deterministic: a
power supplier driver (e.g., the TCPC) must probe before the supplied
devices (charger, fuel-gauge). Because of this, TCPM will naturally be
up and running before the fuel gauge during early boot.

Holding off TCPM interactions to wait for downstream devices to probe
isn't a viable option. TCPM operates under strict USB-PD timing
constraints. Stalling the state machine to wait for a fuel gauge could
violate these timings and cause the PD contract to fail entirely.
Furthermore, a fuel gauge is not required to establish a valid PD
contract; Battery Status and Battery Cap messages are purely telemetry.

The current approach handles this cleanly:
(1) If a request comes in during early boot before the battery is
available, we safely send an unsupported message.
(2) The port partner will continue to retry for battery status/caps
irrespective of the reply.
(3) Once the system is fully booted and the fuel gauge has probed, the
telemetry is reported correctly.

We initially considered a DT-property based approach so TCPM would know
the exact number of batteries (and their reference phandles) at probe
time, but we moved away from that to avoid duplicating DT properties
(based on feedback in [1]). The current implementation safely
prioritizes critical power negotiation over optional telemetry while
still fulfilling the PD requirements in the steady state.


Thanks,

Amit


> Greetings,
>
> -- Sebastian

  reply	other threads:[~2026-06-08 19:20 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-02 22:47 [PATCH v3 0/2] Add support for Battery Status AMS Amit Sunil Dhamne via B4 Relay
2026-06-02 22:47 ` [PATCH v3 1/2] power: supply: Add helpers to get and put arrays of power supply handles Amit Sunil Dhamne via B4 Relay
2026-06-02 22:47 ` [PATCH v3 2/2] usb: typec: tcpm: Add support for Battery Status response message Amit Sunil Dhamne via B4 Relay
2026-06-05 12:55   ` Heikki Krogerus
2026-06-04 17:36 ` [PATCH v3 0/2] Add support for Battery Status AMS Sebastian Reichel
2026-06-08 19:20   ` Amit Sunil Dhamne [this message]
2026-06-12 20:50     ` Amit Sunil Dhamne

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=6ea3d71f-ec81-44b7-a0f7-d10adcb4c834@google.com \
    --to=amitsd@google.com \
    --cc=andre.draszik@linaro.org \
    --cc=badhri@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hansg@kernel.org \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=kernel@puri.sm \
    --cc=krzk@kernel.org \
    --cc=kyletso@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=peter.griffin@linaro.org \
    --cc=rdbabiera@google.com \
    --cc=sebastian.krzyszkowiak@puri.sm \
    --cc=sebastian.reichel@collabora.com \
    --cc=tudor.ambarus@linaro.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