Re: [PATCH v5 2/2] usb: typec: tcpm: Add support for Battery Status response message
From: Amit Sunil Dhamne
Date: Wed Jul 29 2026 - 20:57:05 EST
Hi Sebastian,
On 7/25/26 5:01 PM, Sebastian Reichel wrote:
Hi,
On Tue, Jul 14, 2026 at 09:10:53PM +0000, Amit Sunil Dhamne via B4 Relay wrote:
From: Amit Sunil Dhamne <amitsd@xxxxxxxxxx>
Add support for responding to a Get_Battery_Status request with a
Battery_Status message. The port partner shall request the status of a
port's battery by providing an index in the Get_Battery_Status AMS. In
case of failure to identify the battery, the port shall reply with an
appropriate message indicating so.
Support for Battery_Status message is required for sinks that contain
battery as specified in USB PD Rev3.1 v1.8
("Applicability of Data Messages" section).
Signed-off-by: Amit Sunil Dhamne <amitsd@xxxxxxxxxx>
Reviewed-by: Badhri Jagan Sridharan <badhri@xxxxxxxxxx>
Acked-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
---
drivers/usb/typec/tcpm/tcpm.c | 138 ++++++++++++++++++++++++++++++++++++++++--
include/linux/usb/pd.h | 29 +++++++++
2 files changed, 163 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 7ef746a90a17..cd33ee131ebd 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -12,6 +12,7 @@
#include <linux/jiffies.h>
#include <linux/kernel.h>
#include <linux/kthread.h>
+#include <linux/math64.h>
#include <linux/minmax.h>
#include <linux/module.h>
#include <linux/mutex.h>
@@ -232,7 +233,8 @@ enum pd_msg_request {
PD_MSG_DATA_SINK_CAP,
PD_MSG_DATA_SOURCE_CAP,
PD_MSG_DATA_REV,
- PD_MSG_EXT_SINK_CAP_EXT
+ PD_MSG_EXT_SINK_CAP_EXT,
+ PD_MSG_DATA_BATT_STATUS
};
enum adev_actions {
@@ -387,7 +389,15 @@ struct pd_timings {
};
/* Convert microwatt to watt */
-#define UW_TO_W(pow) ((pow) / 1000000)
+#define UW_TO_W(pow) (div_u64((pow), 1000000))
+
+/*
+ * As per USB PD Spec Rev 3.18 (Sec. 6.5.13.11), the number of fixed batteries
+ * that a port can be queried is restricted to 4.
+ */
+#define MAX_NUM_FIXED_BATT 4
If I understand the spec correctly, the presence of a fixed battery
should never change for fixed batteries. I guess the rationale is,
that one only has to the battery capabilities once for these kind of
batteries. But for the Linux kernel this concept does not exist and
all batteries are potentialle hot-swappable. For real hardware with
TCPM and hot-swappable battery, this code will now incorrectly
expose them as fixed battery and violate the spec. I think this should
at least be mentioned in the commit message.
You are completely right. I was approaching this primarily from the
smartphone side (Pixel 6), where batteries are effectively fixed and
inaccessible to the user. Because I don't have a setup with TCPM +
hot-swappable (in the context of the spec) batteries to test with, I only implemented the fixed case.
I mentioned this constraint in the cover letter, but I agree it could
have been in the commit message as well. Since Greg has already picked
this series up into his tree, I can't amend the commit message now.
However, if you think it's necessary, I can send a small incremental
patch to add a comment in the code clarifying this assumption.
Otherwise, we can leave it as-is until someone has the hardware to
properly implement and test the hot-swappable support. Let me know what
you prefer.
[...]
+ batt = port->fixed_batt[batt_id];
+ ret = power_supply_get_property(batt, POWER_SUPPLY_PROP_PRESENT, &val);
+ if (ret)
+ tcpm_log(port,
+ "Failed to fetch power_supply_prop_present ret %d",
+ ret);
+ else
+ batt_present = val.intval > 0;
+
+ ret = power_supply_get_property(batt, POWER_SUPPLY_PROP_CHARGE_NOW,
+ &val);
+ if (!ret) {
+ charge_now = val.intval;
+ ret = power_supply_get_property(batt,
+ POWER_SUPPLY_PROP_VOLTAGE_AVG,
+ &val);
+ if (!ret) {
+ energy_now = div_u64((u64)charge_now * val.intval,
+ 1000000);
+
+ /*
+ * Battery Present Charge is reported in
+ * increments of 0.1WH.
+ */
+ present_charge = (u16)UW_TO_W(energy_now * 10);
+ }
+ }
[...]
What about fuel gauges, which expose POWER_SUPPLY_PROP_ENERGY_NOW
instead of POWER_SUPPLY_PROP_CHARGE_NOW?
Good point. Our fuel gauge uses charge_* properties, so charge_now was
sufficient for our immediate use case, but it makes sense to support
energy_now natively for other users.
Since the original patch is already merged, I could write an incremental
follow-up patch that checks POWER_SUPPLY_PROP_ENERGY_NOW first, and if
it's not supported, falls back to calculating it via CHARGE_NOW *
VOLTAGE_AVG. Please let me know if this works?
P.S.: Sorry for slow review.
No worries, thanks for your feedback!
Regards,
Amit
Greetings,
-- Sebastian