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

[PATCH v2 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_battery_all` &
`power_supply_put_array` APIs.

Signed-off-by: Amit Sunil Dhamne <amitsd@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 | 137 ++++++++++++++++++++++++++++
 drivers/usb/typec/tcpm/tcpm.c            | 151 ++++++++++++++++++++++++++++++-
 include/linux/power_supply.h             |  12 +++
 include/linux/usb/pd.h                   |  29 ++++++
 4 files changed, 326 insertions(+), 3 deletions(-)
---
base-commit: 57b8e2d666a31fa201432d58f5fe3469a0dd83ba
change-id: 20260501-batt-status-16b6761c0bb1

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

[PATCH v2 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_battery_all() to allow drivers to obtain a list of
registered battery type power supply references in the system. Also add
power_supply_put_array() to release references to the power supplies
when no longer needed.

Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
---
 drivers/power/supply/power_supply_core.c | 137 +++++++++++++++++++++++++++++++
 include/linux/power_supply.h             |  12 +++
 2 files changed, 149 insertions(+)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index a446d3d086fc..04e5c1083e8f 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -482,6 +482,127 @@ struct power_supply *power_supply_get_by_name(const char *name)
 }
 EXPORT_SYMBOL_GPL(power_supply_get_by_name);
 
+static int __power_supply_get_num_battery(struct power_supply *epsy, void *data)
+{
+	int *count = data;
+
+	if (epsy->desc->type == POWER_SUPPLY_TYPE_BATTERY)
+		(*count)++;
+
+	return 0;
+}
+
+static int power_supply_get_num_battery(struct device *dev)
+{
+	int ret, count = 0;
+
+	ret = power_supply_for_each_psy(&count, __power_supply_get_num_battery);
+
+	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_battery_array(struct power_supply *epsy,
+						 void *_data)
+{
+	struct psy_get_supplies_data *data = _data;
+
+	if (epsy->desc->type == POWER_SUPPLY_TYPE_BATTERY) {
+		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_battery_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_battery_array);
+
+	dev_dbg(dev, "%s Found %d batteries with array size %d ret %d\n",
+		__func__, data.cnt, data.size, ret);
+
+	if (ret < 0) {
+		power_supply_put_array(batteries, data.cnt);
+		return ret;
+	}
+
+	return data.cnt;
+}
+
+/**
+ * power_supply_get_battery_all() - Fetches references to all battery type power
+ *                                  supplies.
+ * @dev: Pointer to device requesting the power supply refs.
+ * @psys: Pointer to an array of power supply refs.
+ *
+ * Helper function to get handles to all battery type power supplies.
+ * 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.
+ *
+ * The caller should invoke power_supply_put_array() on the returned array
+ * of psy refs and free the array returned by this function using kfree()
+ * after use.
+ */
+int __must_check power_supply_get_battery_all(struct device *dev,
+					      struct power_supply ***psys)
+{
+	int ret;
+
+	if (!psys)
+		return -EINVAL;
+
+	ret = power_supply_get_num_battery(dev);
+	if (ret < 0)
+		return ret;
+
+	if (!ret) {
+		*psys = NULL;
+		return 0;
+	}
+
+	*psys = kzalloc_objs(**psys, ret);
+	if (!*psys)
+		return -ENOMEM;
+
+	ret = power_supply_populate_battery_array(dev, ret, *psys);
+	if (ret <= 0) {
+		kfree(*psys);
+		*psys = NULL;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(power_supply_get_battery_all);
+
 /**
  * power_supply_put() - Drop reference obtained with power_supply_get_by_name
  * @psy: Reference to put
@@ -496,6 +617,22 @@ void power_supply_put(struct power_supply *psy)
 }
 EXPORT_SYMBOL_GPL(power_supply_put);
 
+/**
+ * power_supply_put_array() - Drop references to an array of power supplies.
+ * @psys: Array of power supply references to put.
+ * @count: Number of elements in the array.
+ */
+void power_supply_put_array(struct power_supply **psys, int count)
+{
+	int i;
+
+	for (i = 0; i < count; i++) {
+		if (psys[i])
+			power_supply_put(psys[i]);
+	}
+}
+EXPORT_SYMBOL_GPL(power_supply_put_array);
+
 static int power_supply_match_device_fwnode(struct device *dev, const void *data)
 {
 	return dev->parent && dev_fwnode(dev->parent) == data;
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 7a5e4c3242a0..026412164a88 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -806,11 +806,23 @@ 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_battery_all(struct device *dev,
+						     struct power_supply ***psys);
+extern void power_supply_put_array(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_battery_all(struct device *dev,
+							    struct power_supply ***psys)
+{
+	if (psys)
+		*psys = NULL;
+	return 0;
+}
+
+static inline void power_supply_put_array(struct power_supply **psys, int count) {}
 #endif
 extern struct power_supply *power_supply_get_by_reference(struct fwnode_handle *fwnode,
 							  const char *property);

-- 
2.54.0.746.g67dd491aae-goog

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

[Hans de Goede]

Hi,

Thank you for the new version.

On 27-May-26 02:02, Amit Sunil Dhamne via B4 Relay wrote:
> From: Amit Sunil Dhamne <amitsd@google.com>
> 
> Add power_supply_get_battery_all() to allow drivers to obtain a list of
> registered battery type power supply references in the system. Also add
> power_supply_put_array() to release references to the power supplies
> when no longer needed.
> 
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> ---
>  drivers/power/supply/power_supply_core.c | 137 +++++++++++++++++++++++++++++++
>  include/linux/power_supply.h             |  12 +++
>  2 files changed, 149 insertions(+)
> 
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index a446d3d086fc..04e5c1083e8f 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -482,6 +482,127 @@ struct power_supply *power_supply_get_by_name(const char *name)
>  }
>  EXPORT_SYMBOL_GPL(power_supply_get_by_name);
>  
> +static int __power_supply_get_num_battery(struct power_supply *epsy, void *data)
> +{
> +	int *count = data;
> +
> +	if (epsy->desc->type == POWER_SUPPLY_TYPE_BATTERY)
> +		(*count)++;
> +
> +	return 0;
> +}
> +
> +static int power_supply_get_num_battery(struct device *dev)
> +{
> +	int ret, count = 0;
> +
> +	ret = power_supply_for_each_psy(&count, __power_supply_get_num_battery);
> +
> +	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_battery_array(struct power_supply *epsy,
> +						 void *_data)
> +{
> +	struct psy_get_supplies_data *data = _data;
> +
> +	if (epsy->desc->type == POWER_SUPPLY_TYPE_BATTERY) {
> +		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_battery_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_battery_array);
> +
> +	dev_dbg(dev, "%s Found %d batteries with array size %d ret %d\n",
> +		__func__, data.cnt, data.size, ret);
> +
> +	if (ret < 0) {
> +		power_supply_put_array(batteries, data.cnt);
> +		return ret;
> +	}
> +
> +	return data.cnt;
> +}
> +
> +/**
> + * power_supply_get_battery_all() - Fetches references to all battery type power
> + *                                  supplies.
> + * @dev: Pointer to device requesting the power supply refs.
> + * @psys: Pointer to an array of power supply refs.
> + *
> + * Helper function to get handles to all battery type power supplies.
> + * 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.
> + *
> + * The caller should invoke power_supply_put_array() on the returned array
> + * of psy refs and free the array returned by this function using kfree()
> + * after use.
> + */
> +int __must_check power_supply_get_battery_all(struct device *dev,
> +					      struct power_supply ***psys)
> +{
> +	int ret;
> +
> +	if (!psys)
> +		return -EINVAL;
> +
> +	ret = power_supply_get_num_battery(dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (!ret) {
> +		*psys = NULL;
> +		return 0;
> +	}
> +
> +	*psys = kzalloc_objs(**psys, ret);
> +	if (!*psys)
> +		return -ENOMEM;
> +
> +	ret = power_supply_populate_battery_array(dev, ret, *psys);
> +	if (ret <= 0) {
> +		kfree(*psys);
> +		*psys = NULL;
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(power_supply_get_battery_all);
> +
>  /**
>   * power_supply_put() - Drop reference obtained with power_supply_get_by_name
>   * @psy: Reference to put
> @@ -496,6 +617,22 @@ void power_supply_put(struct power_supply *psy)
>  }
>  EXPORT_SYMBOL_GPL(power_supply_put);
>  
> +/**
> + * power_supply_put_array() - Drop references to an array of power supplies.
> + * @psys: Array of power supply references to put.
> + * @count: Number of elements in the array.
> + */
> +void power_supply_put_array(struct power_supply **psys, int count)
> +{
> +	int i;
> +
> +	for (i = 0; i < count; i++) {
> +		if (psys[i])
> +			power_supply_put(psys[i]);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(power_supply_put_array);
> +

I think it would be better to call this: power_supply_put_battery_all()
and move it to directly below power_supply_get_battery_all()and also
have it call kfree(psys) so that it does all the teardown of what
power_supply_get_battery_all() has done.

Regards,

Hans



>  static int power_supply_match_device_fwnode(struct device *dev, const void *data)
>  {
>  	return dev->parent && dev_fwnode(dev->parent) == data;
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index 7a5e4c3242a0..026412164a88 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -806,11 +806,23 @@ 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_battery_all(struct device *dev,
> +						     struct power_supply ***psys);
> +extern void power_supply_put_array(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_battery_all(struct device *dev,
> +							    struct power_supply ***psys)
> +{
> +	if (psys)
> +		*psys = NULL;
> +	return 0;
> +}
> +
> +static inline void power_supply_put_array(struct power_supply **psys, int count) {}
>  #endif
>  extern struct power_supply *power_supply_get_by_reference(struct fwnode_handle *fwnode,
>  							  const char *property);
> 

[PATCH v2 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>
---
 drivers/usb/typec/tcpm/tcpm.c | 151 +++++++++++++++++++++++++++++++++++++++++-
 include/linux/usb/pd.h        |  29 ++++++++
 2 files changed, 177 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index dfbb94ddc98a..ee45d90dbf98 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -232,7 +232,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 {
@@ -389,6 +390,14 @@ struct pd_timings {
 /* Convert microwatt to watt */
 #define UW_TO_W(pow)					((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
  * @vid: Vendor ID (assigned by USB-IF)
@@ -683,6 +692,9 @@ struct tcpm_port {
 
 	struct pd_identifier pd_ident;
 	struct sink_caps_ext_data sink_caps_ext;
+	struct power_supply *fixed_batt[MAX_NUM_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 */
@@ -1391,6 +1403,37 @@ static int tcpm_pd_send_revision(struct tcpm_port *port)
 	return tcpm_pd_transmit(port, TCPC_TX_SOP, &msg);
 }
 
+static void tcpm_init_fixed_batt(struct tcpm_port *port)
+{
+	struct power_supply **batteries;
+	int ret, i;
+
+	if (port->fixed_batt_cnt > 0)
+		return;
+
+	ret = power_supply_get_battery_all(port->dev, &batteries);
+	if (ret < 0) {
+		tcpm_log(port, "Failed to fetch batteries refs ret=%d", ret);
+		return;
+	}
+
+	for (i = 0; i < ret; i++) {
+		if (!batteries[i])
+			continue;
+
+		if (port->fixed_batt_cnt < MAX_NUM_FIXED_BATT)
+			port->fixed_batt[port->fixed_batt_cnt++] = batteries[i];
+		else
+			break;
+	}
+
+	if (i < ret)
+		power_supply_put_array(batteries + i, ret - i);
+
+	if (ret)
+		kfree(batteries);
+}
+
 static int tcpm_pd_send_source_caps(struct tcpm_port *port)
 {
 	struct pd_message msg;
@@ -1476,6 +1519,8 @@ static int tcpm_pd_send_sink_cap_ext(struct tcpm_port *port)
 
 	if (!port->self_powered)
 		data->spr_op_pdp = operating_snk_watt;
+	else
+		tcpm_init_fixed_batt(port);
 
 	/*
 	 * SPR Sink Minimum PDP indicates the minimum power required to operate
@@ -1502,6 +1547,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 = port->fixed_batt_cnt;
 	skedb.modes = data->modes;
 	skedb.spr_min_pdp = data->spr_min_pdp;
 	skedb.spr_op_pdp = data->spr_op_pdp;
@@ -1520,6 +1566,87 @@ 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;
+
+	memset(&msg, 0, sizeof(msg));
+
+	if (batt_id >= port->fixed_batt_cnt || !port->fixed_batt[batt_id])
+		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 = ((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);
 }
 
@@ -3854,6 +3981,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)) {
@@ -3862,7 +3990,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;
@@ -3887,9 +4015,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:
@@ -4100,6 +4236,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;
 		}
@@ -8597,6 +8741,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_array(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.54.0.746.g67dd491aae-goog

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

[Hans de Goede]

Hi,

On 27-May-26 02:02, 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>
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 151 +++++++++++++++++++++++++++++++++++++++++-
>  include/linux/usb/pd.h        |  29 ++++++++
>  2 files changed, 177 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index dfbb94ddc98a..ee45d90dbf98 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -232,7 +232,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 {
> @@ -389,6 +390,14 @@ struct pd_timings {
>  /* Convert microwatt to watt */
>  #define UW_TO_W(pow)					((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
>   * @vid: Vendor ID (assigned by USB-IF)
> @@ -683,6 +692,9 @@ struct tcpm_port {
>  
>  	struct pd_identifier pd_ident;
>  	struct sink_caps_ext_data sink_caps_ext;
> +	struct power_supply *fixed_batt[MAX_NUM_FIXED_BATT];
> +	u32 fixed_batt_cnt;

Why not just have a struct power_supply **fixed_batt here
and directly pass that to power_supply_get_battery_all() ?

And then call power_supply_put_battery_all() at cleanup /
driver-remove time ?


> +	u32 batt_request_id;
>  #ifdef CONFIG_DEBUG_FS
>  	struct dentry *dentry;
>  	struct mutex logbuffer_lock;	/* log buffer access lock */
> @@ -1391,6 +1403,37 @@ static int tcpm_pd_send_revision(struct tcpm_port *port)
>  	return tcpm_pd_transmit(port, TCPC_TX_SOP, &msg);
>  }
>  
> +static void tcpm_init_fixed_batt(struct tcpm_port *port)
> +{
> +	struct power_supply **batteries;
> +	int ret, i;
> +
> +	if (port->fixed_batt_cnt > 0)
> +		return;
> +
> +	ret = power_supply_get_battery_all(port->dev, &batteries);
> +	if (ret < 0) {
> +		tcpm_log(port, "Failed to fetch batteries refs ret=%d", ret);
> +		return;
> +	}
> +
> +	for (i = 0; i < ret; i++) {
> +		if (!batteries[i])
> +			continue;
> +
> +		if (port->fixed_batt_cnt < MAX_NUM_FIXED_BATT)
> +			port->fixed_batt[port->fixed_batt_cnt++] = batteries[i];
> +		else
> +			break;
> +	}
> +
> +	if (i < ret)
> +		power_supply_put_array(batteries + i, ret - i);
> +
> +	if (ret)
> +		kfree(batteries);
> +}
> +

If you do as I suggest above this entire function can go away.

>  static int tcpm_pd_send_source_caps(struct tcpm_port *port)
>  {
>  	struct pd_message msg;
> @@ -1476,6 +1519,8 @@ static int tcpm_pd_send_sink_cap_ext(struct tcpm_port *port)
>  
>  	if (!port->self_powered)
>  		data->spr_op_pdp = operating_snk_watt;
> +	else
> +		tcpm_init_fixed_batt(port);

And you can just call power_supply_get_battery_all() here and store
count int fixed_batt_cnt (make fixed_batt_cnt an int so that negative
errors don't turn into a big number.

>  
>  	/*
>  	 * SPR Sink Minimum PDP indicates the minimum power required to operate
> @@ -1502,6 +1547,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 = port->fixed_batt_cnt;
>  	skedb.modes = data->modes;
>  	skedb.spr_min_pdp = data->spr_min_pdp;
>  	skedb.spr_op_pdp = data->spr_op_pdp;
> @@ -1520,6 +1566,87 @@ 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;
> +
> +	memset(&msg, 0, sizeof(msg));
> +
> +	if (batt_id >= port->fixed_batt_cnt || !port->fixed_batt[batt_id])
> +		goto send_status;

And here you can then additionally check against MAX_NUM_FIXED_BATT .

Also drop the NULL check here, power_supply_get_battery_all() never
leaves holes / NULL entries in the array.


> +
> +	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 = ((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);
>  }
>  
> @@ -3854,6 +3981,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)) {
> @@ -3862,7 +3990,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;
> @@ -3887,9 +4015,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:
> @@ -4100,6 +4236,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;
>  		}
> @@ -8597,6 +8741,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_array(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 */
> 


Regards,

Hans

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

[Amit Sunil Dhamne]

Hi Hans,

Thanks for the review!

On 5/27/26 3:15 AM, Hans de Goede wrote:
> Hi,
>
> On 27-May-26 02:02, 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>
>> ---
>>   drivers/usb/typec/tcpm/tcpm.c | 151 +++++++++++++++++++++++++++++++++++++++++-
>>   include/linux/usb/pd.h        |  29 ++++++++
>>   2 files changed, 177 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
>> index dfbb94ddc98a..ee45d90dbf98 100644
>> --- a/drivers/usb/typec/tcpm/tcpm.c
>> +++ b/drivers/usb/typec/tcpm/tcpm.c
>> @@ -232,7 +232,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 {
>> @@ -389,6 +390,14 @@ struct pd_timings {
>>   /* Convert microwatt to watt */
>>   #define UW_TO_W(pow)					((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
>>    * @vid: Vendor ID (assigned by USB-IF)
>> @@ -683,6 +692,9 @@ struct tcpm_port {
>>   
>>   	struct pd_identifier pd_ident;
>>   	struct sink_caps_ext_data sink_caps_ext;
>> +	struct power_supply *fixed_batt[MAX_NUM_FIXED_BATT];
>> +	u32 fixed_batt_cnt;
> Why not just have a struct power_supply **fixed_batt here
> and directly pass that to power_supply_get_battery_all() ?
>
> And then call power_supply_put_battery_all() at cleanup /
> driver-remove time ?

Comment addressed below.


>
>
>> +	u32 batt_request_id;
>>   #ifdef CONFIG_DEBUG_FS
>>   	struct dentry *dentry;
>>   	struct mutex logbuffer_lock;	/* log buffer access lock */
>> @@ -1391,6 +1403,37 @@ static int tcpm_pd_send_revision(struct tcpm_port *port)
>>   	return tcpm_pd_transmit(port, TCPC_TX_SOP, &msg);
>>   }
>>   
>> +static void tcpm_init_fixed_batt(struct tcpm_port *port)
>> +{
>> +	struct power_supply **batteries;
>> +	int ret, i;
>> +
>> +	if (port->fixed_batt_cnt > 0)
>> +		return;
>> +
>> +	ret = power_supply_get_battery_all(port->dev, &batteries);
>> +	if (ret < 0) {
>> +		tcpm_log(port, "Failed to fetch batteries refs ret=%d", ret);
>> +		return;
>> +	}
>> +
>> +	for (i = 0; i < ret; i++) {
>> +		if (!batteries[i])
>> +			continue;
>> +
>> +		if (port->fixed_batt_cnt < MAX_NUM_FIXED_BATT)
>> +			port->fixed_batt[port->fixed_batt_cnt++] = batteries[i];
>> +		else
>> +			break;
>> +	}
>> +
>> +	if (i < ret)
>> +		power_supply_put_array(batteries + i, ret - i);
>> +
>> +	if (ret)
>> +		kfree(batteries);
>> +}
>> +
> If you do as I suggest above this entire function can go away.
The reason for implementing it this way was because the USB PD protocol 
only caters to a maximum of 4 batteries. I wanted to avoid unnecessarily 
holding references to any extra ones if a platform theoretically had 5 
or more.

However, I agree that this is overthinking it for a practical scenario, 
and your approach is much cleaner. I will drop the helper function and 
switch to dynamically holding the array pointer directly as you suggested.


>
>>   static int tcpm_pd_send_source_caps(struct tcpm_port *port)
>>   {
>>   	struct pd_message msg;
>> @@ -1476,6 +1519,8 @@ static int tcpm_pd_send_sink_cap_ext(struct tcpm_port *port)
>>   
>>   	if (!port->self_powered)
>>   		data->spr_op_pdp = operating_snk_watt;
>> +	else
>> +		tcpm_init_fixed_batt(port);
> And you can just call power_supply_get_battery_all() here and store
> count int fixed_batt_cnt (make fixed_batt_cnt an int so that negative
> errors don't turn into a big number.
>
>>   
>>   	/*
>>   	 * SPR Sink Minimum PDP indicates the minimum power required to operate
>> @@ -1502,6 +1547,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 = port->fixed_batt_cnt;
>>   	skedb.modes = data->modes;
>>   	skedb.spr_min_pdp = data->spr_min_pdp;
>>   	skedb.spr_op_pdp = data->spr_op_pdp;
>> @@ -1520,6 +1566,87 @@ 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;
>> +
>> +	memset(&msg, 0, sizeof(msg));
>> +
>> +	if (batt_id >= port->fixed_batt_cnt || !port->fixed_batt[batt_id])
>> +		goto send_status;
> And here you can then additionally check against MAX_NUM_FIXED_BATT .
>
> Also drop the NULL check here, power_supply_get_battery_all() never
> leaves holes / NULL entries in the array.

Ack.


BR,

Amit

>
>
>> +
>> +	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 = ((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);
>>   }
>>   
>> @@ -3854,6 +3981,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)) {
>> @@ -3862,7 +3990,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;
>> @@ -3887,9 +4015,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:
>> @@ -4100,6 +4236,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;
>>   		}
>> @@ -8597,6 +8741,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_array(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 */
>>
>
> Regards,
>
> Hans
>
>