[PATCH v5 0/2] Add support for Battery Status AMS

[PATCH v5 0/2] Add support for Battery Status AMS

[Amit Sunil Dhamne via B4 Relay]

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_system_batteries` &
`power_supply_put_system_batteries` APIs.

Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
---
Changes in v5:
- Use kernel helpers for division instead of '/' operator. (flagged by
  LKP test bot for 32 bit arch like nios32, i386). Validated build
  passes (on i386).
- Link to v4: https://lore.kernel.org/r/20260701-batt-status-v4-0-a31d97b1ae57@google.com

Changes in v4:
- Filter battery retrieval based on POWER_SUPPLY_SCOPE_SYSTEM
  (suggested by Sebastian Reichel)
- For the rest of the comments, I have replied in thread in previous
  submission.
- Link to v3: https://lore.kernel.org/r/20260602-batt-status-v3-0-a7c1c271f3b0@google.com

Changes in v3:
- Suggested by Hans:
  - s/power_supply_put_array/power_supply_put_battery_all.
  - Call kfree in power_supply_put_battery_all().
  - Change the datatype for fixed_batt from an array of pointers to a
    double pointer.
  - Directly store the power supply pointers returned by
    power_supply_get_battery_all() into fixed_batt.
  - As we are directly storing all battery type power supplies returned
    by *_get_battery_all() but as per the USB PD specification, tcpm
    only supports reporting of up to 4 fixed batteries, we add
    additional checks to satisfy the latter condition.
  - It was suggested that fixed_batt_cnt be changed to int to store the
    return value of *_get_battery_all(). However, I am leaving it as is
    because: in case the retval is < 0, we don't care for the error
    value beyond the scope of the function it's called in. It's sufficient
    to just have fixed_batt_cnt to stay 0. Lmk if you still want it to
    be int and I will change it.

- Suggested by Badhri (internal review)
  - It's not necessary from a PD protocol perspective that a Sink Cap
    Extended AMS should always precede the Battery Status AMS.
    Therefore, we should also get battery references during Battery
    Status AMS in case fixed_batteries was unintialized.
- Link to v2: https://lore.kernel.org/r/20260527-batt-status-v2-0-4282985165f3@google.com

Changes in v2:
- Suggested by Hans De Goede:
  - Added helper power_supply_put_array() to release array of psy refs.
  - Modified tcpm implementation to use the above.
  - Drop "power: supply: max17042: add handler for energy_now property"
    commit.
  - Offload energy_now calculation to tcpm.
- Minor changes to description comments of
  power_supply_get_battery_all() to improve brevity.
- Link to v1: https://lore.kernel.org/r/20260515-batt-status-v1-0-fed6b7d8cea7@google.com

---
Amit Sunil Dhamne (2):
      power: supply: Add helpers to get and put arrays of power supply handles
      usb: typec: tcpm: Add support for Battery Status response message

 drivers/power/supply/power_supply_core.c | 153 +++++++++++++++++++++++++++++++
 drivers/usb/typec/tcpm/tcpm.c            | 138 +++++++++++++++++++++++++++-
 include/linux/power_supply.h             |  15 +++
 include/linux/usb/pd.h                   |  29 ++++++
 4 files changed, 331 insertions(+), 4 deletions(-)
---
base-commit: 0e35b9b6ec0ffcc5e23cbdec09f5c622ad532b53
change-id: 20260501-batt-status-16b6761c0bb1

Best regards,
-- 
Amit Sunil Dhamne <amitsd@google.com>

[PATCH v5 1/2] power: supply: Add helpers to get and put arrays of power supply handles

[Amit Sunil Dhamne via B4 Relay]

From: Amit Sunil Dhamne <amitsd@google.com>

Add power_supply_get_system_batteries() to allow drivers to obtain
a list of registered battery type power supply references in the
system. Also add power_supply_put_system_batteries() to perform
cleanup after the former function is called.

Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
 drivers/power/supply/power_supply_core.c | 153 +++++++++++++++++++++++++++++++
 include/linux/power_supply.h             |  15 +++
 2 files changed, 168 insertions(+)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 2532e221b2e1..70f35f64cc72 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -477,6 +477,159 @@ struct power_supply *power_supply_get_by_name(const char *name)
 }
 EXPORT_SYMBOL_GPL(power_supply_get_by_name);
 
+static bool power_supply_is_system_battery(struct power_supply *psy)
+{
+	union power_supply_propval val;
+
+	if (psy->desc->type != POWER_SUPPLY_TYPE_BATTERY)
+		return false;
+
+	if (!power_supply_get_property_direct(psy, POWER_SUPPLY_PROP_SCOPE,
+					      &val))
+		if (val.intval == POWER_SUPPLY_SCOPE_DEVICE)
+			return false;
+
+	return true;
+}
+
+static int __power_supply_get_num_system_batteries(struct power_supply *epsy,
+						   void *data)
+{
+	int *count = data;
+
+	if (power_supply_is_system_battery(epsy))
+		(*count)++;
+
+	return 0;
+}
+
+static int power_supply_get_num_system_batteries(struct device *dev)
+{
+	int ret, count = 0;
+
+	ret = power_supply_for_each_psy(&count,
+					__power_supply_get_num_system_batteries);
+
+	dev_dbg(dev, "%s: count: %d ret %d\n", __func__, count, ret);
+
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+struct psy_get_supplies_data {
+	int cnt;
+	int size;
+	struct power_supply **psys;
+};
+
+static int
+__power_supply_populate_system_batteries_array(struct power_supply *epsy,
+					       void *_data)
+{
+	struct psy_get_supplies_data *data = _data;
+
+	if (power_supply_is_system_battery(epsy)) {
+		if (data->size <= data->cnt)
+			return -EOVERFLOW;
+
+		get_device(&epsy->dev);
+		data->psys[data->cnt] = epsy;
+		atomic_inc(&epsy->use_cnt);
+		data->cnt++;
+	}
+
+	return 0;
+}
+
+static int
+power_supply_populate_system_batteries_array(struct device *dev, int size,
+					     struct power_supply **batteries)
+{
+	int ret;
+
+	struct psy_get_supplies_data data = {
+		.cnt = 0,
+		.size = size,
+		.psys = batteries,
+	};
+
+	ret = power_supply_for_each_psy(&data,
+					__power_supply_populate_system_batteries_array);
+
+	dev_dbg(dev, "%s Found %d batteries with array size %d ret %d\n",
+		__func__, data.cnt, data.size, ret);
+
+	if (ret < 0 || !data.cnt) {
+		power_supply_put_system_batteries(batteries, data.cnt);
+		return ret;
+	}
+
+	return data.cnt;
+}
+
+/**
+ * power_supply_get_system_batteries() - Fetches references to battery type
+ *                                       power supplies in the system.
+ * @dev: Pointer to device requesting the power supply refs.
+ * @psys: Pointer to an array of power supply refs.
+ *
+ * Helper function to get handles to battery type power supplies in the system.
+ * If acquiring a ref to a power supply fails, then the search for battery type
+ * power supplies will abort and the acquired power supply references will be
+ * released.
+ *
+ * Return: Indicates the number of battery type power supplies returned on
+ * success or a negative error code on failure.
+ *
+ * Call power_supply_put_system_batteries() after use to cleanup resources.
+ */
+int __must_check power_supply_get_system_batteries(struct device *dev,
+						   struct power_supply ***psys)
+{
+	int ret;
+
+	if (!psys)
+		return -EINVAL;
+
+	ret = power_supply_get_num_system_batteries(dev);
+	if (ret <= 0) {
+		*psys = NULL;
+		return ret;
+	}
+
+	*psys = kzalloc_objs(**psys, ret);
+	if (!*psys)
+		return -ENOMEM;
+
+	ret = power_supply_populate_system_batteries_array(dev, ret, *psys);
+	if (ret <= 0)
+		*psys = NULL;
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(power_supply_get_system_batteries);
+
+/**
+ * power_supply_put_system_batteries() - Cleanup resources allocated by
+ *                                       power_supply_get_system_batteries()
+ * @psys: Array of power supply references to release and free.
+ * @count: Number of elements in the array.
+ */
+void power_supply_put_system_batteries(struct power_supply **psys, int count)
+{
+	int i;
+
+	for (i = 0; i < count; i++) {
+		if (psys[i])
+			power_supply_put(psys[i]);
+	}
+
+	kfree(psys);
+}
+EXPORT_SYMBOL_GPL(power_supply_put_system_batteries);
+
 /**
  * power_supply_put() - Drop reference obtained with power_supply_get_by_name
  * @psy: Reference to put
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 7a5e4c3242a0..9167a33d074b 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -806,11 +806,26 @@ extern int power_supply_reg_notifier(struct notifier_block *nb);
 extern void power_supply_unreg_notifier(struct notifier_block *nb);
 #if IS_ENABLED(CONFIG_POWER_SUPPLY)
 extern struct power_supply *power_supply_get_by_name(const char *name);
+extern int __must_check power_supply_get_system_batteries(struct device *dev,
+							  struct power_supply ***psys);
+extern void power_supply_put_system_batteries(struct power_supply **psys, int count);
 extern void power_supply_put(struct power_supply *psy);
 #else
 static inline void power_supply_put(struct power_supply *psy) {}
 static inline struct power_supply *power_supply_get_by_name(const char *name)
 { return NULL; }
+static inline int __must_check power_supply_get_system_batteries(struct device *dev,
+								 struct power_supply ***psys)
+{
+	if (psys)
+		*psys = NULL;
+	return 0;
+}
+
+static inline void power_supply_put_system_batteries(struct power_supply **psys,
+						     int count)
+{
+}
 #endif
 extern struct power_supply *power_supply_get_by_reference(struct fwnode_handle *fwnode,
 							  const char *property);

-- 
2.55.0.141.g00534a21ce-goog

[PATCH v5 2/2] usb: typec: tcpm: Add support for Battery Status response message

[Amit Sunil Dhamne via B4 Relay]

From: Amit Sunil Dhamne <amitsd@google.com>

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@google.com>
Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 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
+
+#define BATTERY_PROPERTY_UNKNOWN			0xffff
 
 /*
  * struct pd_identifier - Contains info about PD identifiers
@@ -683,6 +693,9 @@ struct tcpm_port {
 
 	struct pd_identifier pd_ident;
 	struct sink_caps_ext_data sink_caps_ext;
+	struct power_supply **fixed_batt;
+	u32 fixed_batt_cnt;
+	u32 batt_request_id;
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *dentry;
 	struct mutex logbuffer_lock;	/* log buffer access lock */
@@ -1470,6 +1483,20 @@ static int tcpm_pd_send_sink_caps(struct tcpm_port *port)
 	return tcpm_pd_transmit(port, TCPC_TX_SOP, &msg);
 }
 
+static void tcpm_get_fixed_batt(struct tcpm_port *port)
+{
+	int ret;
+
+	if (!port->self_powered || port->fixed_batt_cnt > 0)
+		return;
+
+	ret = power_supply_get_system_batteries(port->dev, &port->fixed_batt);
+	if (ret < 0)
+		tcpm_log(port, "Failed to get battery array, ret=%d", ret);
+	else
+		port->fixed_batt_cnt = ret;
+}
+
 static int tcpm_pd_send_sink_cap_ext(struct tcpm_port *port)
 {
 	u16 operating_snk_watt = port->operating_snk_mw / 1000;
@@ -1482,6 +1509,8 @@ static int tcpm_pd_send_sink_cap_ext(struct tcpm_port *port)
 	if (!port->self_powered)
 		data->spr_op_pdp = operating_snk_watt;
 
+	tcpm_get_fixed_batt(port);
+
 	/*
 	 * SPR Sink Minimum PDP indicates the minimum power required to operate
 	 * a sink device in its lowest level of functionality without requiring
@@ -1507,6 +1536,7 @@ static int tcpm_pd_send_sink_cap_ext(struct tcpm_port *port)
 	skedb.load_step = data->load_step;
 	skedb.load_char = cpu_to_le16(data->load_char);
 	skedb.compliance = data->compliance;
+	skedb.batt_info = min(port->fixed_batt_cnt, MAX_NUM_FIXED_BATT);
 	skedb.modes = data->modes;
 	skedb.spr_min_pdp = data->spr_min_pdp;
 	skedb.spr_op_pdp = data->spr_op_pdp;
@@ -1525,6 +1555,88 @@ static int tcpm_pd_send_sink_cap_ext(struct tcpm_port *port)
 					   port->message_id,
 					   data_obj_cnt,
 					   1 /* Denotes if ext header */));
+
+	return tcpm_pd_transmit(port, TCPC_TX_SOP, &msg);
+}
+
+static int tcpm_pd_send_batt_status(struct tcpm_port *port)
+{
+	u16 present_charge = BATTERY_PROPERTY_UNKNOWN;
+	bool batt_present = false, invalid_ref = true;
+	u32 batt_id = port->batt_request_id;
+	union power_supply_propval val;
+	struct power_supply *batt;
+	u8 charging_status = 0;
+	struct pd_message msg;
+	int ret, charge_now;
+	u64 energy_now;
+	u32 bsdo;
+
+	tcpm_get_fixed_batt(port);
+	memset(&msg, 0, sizeof(msg));
+
+	if (batt_id >= port->fixed_batt_cnt || batt_id >= MAX_NUM_FIXED_BATT)
+		goto send_status;
+
+	invalid_ref = false;
+	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);
+		}
+	}
+
+	ret = power_supply_get_property(batt, POWER_SUPPLY_PROP_STATUS, &val);
+	if (!ret) {
+		switch (val.intval) {
+		case POWER_SUPPLY_STATUS_CHARGING:
+			charging_status = BSDO_BATTERY_INFO_CHARGING;
+			break;
+		case POWER_SUPPLY_STATUS_DISCHARGING:
+			charging_status = BSDO_BATTERY_INFO_DISCHARGING;
+			break;
+		case POWER_SUPPLY_STATUS_NOT_CHARGING:
+		case POWER_SUPPLY_STATUS_FULL:
+			charging_status = BSDO_BATTERY_INFO_IDLE;
+			break;
+		default:
+			charging_status = BSDO_BATTERY_INFO_RSVD;
+			break;
+		}
+	}
+
+send_status:
+
+	bsdo = BSDO(present_charge, charging_status, batt_present, invalid_ref);
+	msg.payload[0] = cpu_to_le32(bsdo);
+	msg.header = PD_HEADER_LE(PD_DATA_BATT_STATUS,
+				  port->pwr_role,
+				  port->data_role,
+				  port->negotiated_rev,
+				  port->message_id,
+				  1);
+
 	return tcpm_pd_transmit(port, TCPC_TX_SOP, &msg);
 }
 
@@ -3886,6 +3998,7 @@ static void tcpm_pd_ext_msg_request(struct tcpm_port *port,
 {
 	enum pd_ext_msg_type type = pd_header_type_le(msg->header);
 	unsigned int data_size = pd_ext_header_data_size_le(msg->ext_msg.header);
+	const struct pd_chunked_ext_message_data *ext_msg = &msg->ext_msg;
 
 	/* stopping VDM state machine if interrupted by other Messages */
 	if (tcpm_vdm_ams(port)) {
@@ -3894,7 +4007,7 @@ static void tcpm_pd_ext_msg_request(struct tcpm_port *port,
 		mod_vdm_delayed_work(port, 0);
 	}
 
-	if (!(le16_to_cpu(msg->ext_msg.header) & PD_EXT_HDR_CHUNKED)) {
+	if (!(le16_to_cpu(ext_msg->header) & PD_EXT_HDR_CHUNKED)) {
 		tcpm_pd_handle_msg(port, PD_MSG_CTRL_NOT_SUPP, NONE_AMS);
 		tcpm_log(port, "Unchunked extended messages unsupported");
 		return;
@@ -3919,9 +4032,17 @@ static void tcpm_pd_ext_msg_request(struct tcpm_port *port,
 					     NONE_AMS, 0);
 		}
 		break;
+	case PD_EXT_GET_BATT_STATUS:
+		if (data_size >= 1) {
+			port->batt_request_id = ext_msg->data[0];
+			tcpm_pd_handle_msg(port, PD_MSG_DATA_BATT_STATUS,
+					   GETTING_BATTERY_STATUS);
+		} else {
+			tcpm_set_state(port, SOFT_RESET_SEND, 0);
+		}
+		break;
 	case PD_EXT_SOURCE_CAP_EXT:
 	case PD_EXT_GET_BATT_CAP:
-	case PD_EXT_GET_BATT_STATUS:
 	case PD_EXT_BATT_CAP:
 	case PD_EXT_GET_MANUFACTURER_INFO:
 	case PD_EXT_MANUFACTURER_INFO:
@@ -4132,6 +4253,14 @@ static bool tcpm_send_queued_message(struct tcpm_port *port)
 					 ret);
 			tcpm_ams_finish(port);
 			break;
+		case PD_MSG_DATA_BATT_STATUS:
+			ret = tcpm_pd_send_batt_status(port);
+			if (ret)
+				tcpm_log(port,
+					 "Failed to send battery status ret=%d",
+					 ret);
+			tcpm_ams_finish(port);
+			break;
 		default:
 			break;
 		}
@@ -8633,6 +8762,7 @@ void tcpm_unregister_port(struct tcpm_port *port)
 	hrtimer_cancel(&port->vdm_state_machine_timer);
 	hrtimer_cancel(&port->state_machine_timer);
 
+	power_supply_put_system_batteries(port->fixed_batt, port->fixed_batt_cnt);
 	tcpm_reset_port(port);
 
 	tcpm_port_unregister_pd(port);
diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
index 337a5485af7c..afb9c2c65588 100644
--- a/include/linux/usb/pd.h
+++ b/include/linux/usb/pd.h
@@ -724,4 +724,33 @@ void usb_power_delivery_unlink_device(struct usb_power_delivery *pd, struct devi
 
 #endif /* CONFIG_TYPEC */
 
+/* Battery Status Data Object */
+#define BSDO_PRESENT_CAPACITY				GENMASK(31, 16)
+#define BSDO_CHG_STATUS					GENMASK(11, 10)
+#define BSDO_BATTERY_PRESENT				BIT(9)
+#define BSDO_INVALID_BATTERY_REFERENCE			BIT(8)
+
+/*
+ * Battery Charge Status: Battery Charging Status Values as defined in
+ * "USB PD Spec Rev3.1 Ver1.8", "Table 6-46 Battery Status Data Object (BSDO)".
+ */
+#define BSDO_BATTERY_INFO_CHARGING			0x0
+#define BSDO_BATTERY_INFO_DISCHARGING			0x1
+#define BSDO_BATTERY_INFO_IDLE				0x2
+#define BSDO_BATTERY_INFO_RSVD				0x3
+
+/**
+ * BSDO() - Pack data into Battery Status Data Object format.
+ * @batt_charge: Battery's present state of charge in 0.1WH increment.
+ * @chg_status: Battery charge status.
+ * @batt_present: Indicates that battery is present/attached when set else absent when unset.
+ * @invalid_ref: Indicates that an invalid battery reference was made in the Get_Battery_Status
+ *		 request.
+ */
+#define BSDO(batt_charge, chg_status, batt_present, invalid_ref)	\
+	((FIELD_PREP(BSDO_PRESENT_CAPACITY, batt_charge)) |		\
+	 (FIELD_PREP(BSDO_CHG_STATUS, chg_status)) |			\
+	 ((batt_present) ? BSDO_BATTERY_PRESENT : 0) |			\
+	 ((invalid_ref) ? BSDO_INVALID_BATTERY_REFERENCE : 0))
+
 #endif /* __LINUX_USB_PD_H */

-- 
2.55.0.141.g00534a21ce-goog

Re: [PATCH v5 2/2] usb: typec: tcpm: Add support for Battery Status response message

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 3597 bytes --]

Hi,

On Tue, Jul 14, 2026 at 09:10:53PM +0000, Amit Sunil Dhamne via B4 Relay wrote:
> From: Amit Sunil Dhamne <amitsd@google.com>
> 
> 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@google.com>
> Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
> Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  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.

> [...]
> +	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?

P.S.: Sorry for slow review.

Greetings,

-- Sebastian


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v5 2/2] usb: typec: tcpm: Add support for Battery Status response message

[Amit Sunil Dhamne]

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@google.com>
>>
>> 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@google.com>
>> Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
>> Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> ---
>>  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
> 

Re: [PATCH v5 2/2] usb: typec: tcpm: Add support for Battery Status response message

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 3675 bytes --]

Hi,

On Wed, Jul 29, 2026 at 05:56:29PM -0700, Amit Sunil Dhamne wrote:
> [...]
>
> > > +/*
> > > + * 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.

You tested only the fixed variant, but your implementation is in the
generic TCPM and does not check if it runs on a Pixel 6. If you
touch generic code you need to think generic :)

Generally I would expect it is "safer" to expose batteries as
hot-swappable when they are fixed than the other way around.
FWIW the kernel's power-supply framework has no concept of fixed
batteries.

> > > [...]
> > > +	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?

That sounds sensible to me.

Greetings,

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: (subset) [PATCH v5 0/2] Add support for Battery Status AMS

[Sebastian Reichel]

On Tue, 14 Jul 2026 21:10:51 +0000, Amit Sunil Dhamne 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).
> 
> [...]

Applied, thanks!

[1/2] power: supply: Add helpers to get and put arrays of power supply handles
      commit: 0962125d9001077e91bc90656245f009e3c6c129

Best regards,
-- 
Sebastian Reichel <sebastian.reichel@collabora.com>

Re: (subset) [PATCH v5 0/2] Add support for Battery Status AMS

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1897 bytes --]

Hi,

On Sun, Jul 26, 2026 at 02:01:23AM +0200, Sebastian Reichel wrote:
> 
> On Tue, 14 Jul 2026 21:10:51 +0000, Amit Sunil Dhamne 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).
> > 
> > [...]
> 
> Applied, thanks!
> 
> [1/2] power: supply: Add helpers to get and put arrays of power supply handles
>       commit: 0962125d9001077e91bc90656245f009e3c6c129

Here is a signed immutable branch/tag for the USB subsystem to
handle patch 2 (I also left some review feedback for that one):

The following changes since commit dc59e4fea9d83f03bad6bddf3fa2e52491777482:

  Linux 7.2-rc1 (2026-06-28 12:01:31 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git tags/ib-psy-array-helpers-for-v7.3-signed

for you to fetch changes up to 0962125d9001077e91bc90656245f009e3c6c129:

  power: supply: Add helpers to get and put arrays of power supply handles (2026-07-26 01:31:43 +0200)

----------------------------------------------------------------
Immutable branch for battery array helpers for power-supply and USB

----------------------------------------------------------------
Amit Sunil Dhamne (1):
      power: supply: Add helpers to get and put arrays of power supply handles

 drivers/power/supply/power_supply_core.c | 153 +++++++++++++++++++++++++++++++
 include/linux/power_supply.h             |  15 +++
 2 files changed, 168 insertions(+)

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]