* [PATCH v2 0/5] Add support for Battery Status & Battery Caps AMS in TCPM
@ 2025-05-08 1:00 Amit Sunil Dhamne via B4 Relay
2025-05-08 1:00 ` [PATCH v2 1/5] dt-bindings: connector: extend ports property to model power connections Amit Sunil Dhamne via B4 Relay
` (4 more replies)
0 siblings, 5 replies; 20+ messages in thread
From: Amit Sunil Dhamne via B4 Relay @ 2025-05-08 1:00 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek
Cc: Kyle Tso, devicetree, linux-kernel, linux-usb, linux-pm,
Amit Sunil Dhamne
Support for Battery Status & Battery Caps messages in response to
Get_Battery_Status & Get_Battery_Cap request is required by USB PD devices
powered by battery, as per "USB PD R3.1 V1.8 Spec", "6.13 Message
Applicability" section. This patchset adds support for these AMSes
to achieve greater compliance with the spec.
Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
---
Changes in v2:
- Instead of introducing new "fixed-batteries" property to pass
reference to FG, use OF graph by extending "ports" property
definition. (suggested by Krzysztof).
- Demonstration of binding usage in gs101-oriole will be in a
different patchset.
---
Amit Sunil Dhamne (5):
dt-bindings: connector: extend ports property to model power connections
power: supply: core: add helper to get power supply given a fwnode
usb: typec: tcpm: Add support for Battery Status response message
power: supply: core: add vendor and product id properties
usb: typec: tcpm: Add support for Battery Cap response message
Documentation/ABI/testing/sysfs-class-power | 19 +-
.../bindings/connector/usb-connector.yaml | 20 +-
.../devicetree/bindings/usb/maxim,max33359.yaml | 25 +++
Documentation/power/power_supply_class.rst | 11 ++
drivers/power/supply/power_supply_core.c | 30 +++
drivers/power/supply/power_supply_sysfs.c | 2 +
drivers/usb/typec/tcpm/tcpm.c | 208 ++++++++++++++++++++-
include/linux/power_supply.h | 5 +
include/linux/usb/pd.h | 65 +++++++
9 files changed, 371 insertions(+), 14 deletions(-)
---
base-commit: 80e54e84911a923c40d7bee33a34c1b4be148d7a
change-id: 20250311-batt_ops-be1bd71ca254
Best regards,
--
Amit Sunil Dhamne <amitsd@google.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/5] dt-bindings: connector: extend ports property to model power connections
2025-05-08 1:00 [PATCH v2 0/5] Add support for Battery Status & Battery Caps AMS in TCPM Amit Sunil Dhamne via B4 Relay
@ 2025-05-08 1:00 ` Amit Sunil Dhamne via B4 Relay
2025-05-08 2:08 ` Rob Herring (Arm)
2025-05-14 19:42 ` Rob Herring
2025-05-08 1:00 ` [PATCH v2 2/5] power: supply: core: add helper to get power supply given a fwnode Amit Sunil Dhamne via B4 Relay
` (3 subsequent siblings)
4 siblings, 2 replies; 20+ messages in thread
From: Amit Sunil Dhamne via B4 Relay @ 2025-05-08 1:00 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek
Cc: Kyle Tso, devicetree, linux-kernel, linux-usb, linux-pm,
Amit Sunil Dhamne
From: Amit Sunil Dhamne <amitsd@google.com>
Extend ports property to model power lines going between connector to
charger or battery/batteries. As an example, connector VBUS can supply
power in & out of the battery for a DRP.
Additionally, add ports property to maxim,max33359 controller example.
Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
---
.../bindings/connector/usb-connector.yaml | 20 +++++++++++------
.../devicetree/bindings/usb/maxim,max33359.yaml | 25 ++++++++++++++++++++++
2 files changed, 38 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
index 11e40d225b9f3a0d0aeea7bf764f1c00a719d615..706094f890026d324e6ece8b0c1e831d04d51eb7 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
+++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
@@ -181,16 +181,16 @@ properties:
port:
$ref: /schemas/graph.yaml#/properties/port
- description: OF graph bindings modeling a data bus to the connector, e.g.
- there is a single High Speed (HS) port present in this connector. If there
- is more than one bus (several port, with 'reg' property), they can be grouped
- under 'ports'.
+ description: OF graph binding to model a logical connection between a device
+ and connector. This connection may represent a data bus or power line. For
+ e.g. a High Speed (HS) data port present in this connector or VBUS line.
+ If there is more than one connection (several port, with 'reg' property),
+ they can be grouped under 'ports'.
ports:
$ref: /schemas/graph.yaml#/properties/ports
- description: OF graph bindings modeling any data bus to the connector
- unless the bus is between parent node and the connector. Since a single
- connector can have multiple data buses every bus has an assigned OF graph
+ description: OF graph bindings to model multiple "port". Since a connector
+ may have multiple logical connections each one has an assigned OF graph
port number as described below.
properties:
@@ -207,6 +207,12 @@ properties:
description: Sideband Use (SBU), present in USB-C. This describes the
alternate mode connection of which SBU is a part.
+ port@3:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: VBUS/VCHGIN present in USB-C connector to model power line
+ going in and/or out of the charger/battery. If there are multiple
+ batteries then this port should contain those many endpoints.
+
required:
- port@0
diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
index 3de4dc40b79192b60443421b557bd2fb18683bf7..730d5c1cc9ddf1ddeff055c00ee172745297633d 100644
--- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
+++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
@@ -75,6 +75,31 @@ examples:
PDO_FIXED(9000, 2000, 0)>;
sink-bc12-completion-time-ms = <500>;
pd-revision = /bits/ 8 <0x03 0x01 0x01 0x08>;
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ usbc0_orien_sw: endpoint {
+ remote-endpoint = <&usbdrd31_phy_orien_switch>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ usbc0_role_sw: endpoint {
+ remote-endpoint = <&usbdrd31_dwc3_role_switch>;
+ };
+ };
+
+ port@3 {
+ reg = <3>;
+ vbus_batt: endpoint {
+ remote-endpoint = <&max17201_fg>;
+ };
+ };
+ };
};
};
};
--
2.49.0.987.g0cc8ee98dc-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 2/5] power: supply: core: add helper to get power supply given a fwnode
2025-05-08 1:00 [PATCH v2 0/5] Add support for Battery Status & Battery Caps AMS in TCPM Amit Sunil Dhamne via B4 Relay
2025-05-08 1:00 ` [PATCH v2 1/5] dt-bindings: connector: extend ports property to model power connections Amit Sunil Dhamne via B4 Relay
@ 2025-05-08 1:00 ` Amit Sunil Dhamne via B4 Relay
2025-06-23 21:21 ` Sebastian Reichel
2025-05-08 1:00 ` [PATCH v2 3/5] usb: typec: tcpm: Add support for Battery Status response message Amit Sunil Dhamne via B4 Relay
` (2 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Amit Sunil Dhamne via B4 Relay @ 2025-05-08 1:00 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek
Cc: Kyle Tso, devicetree, linux-kernel, linux-usb, linux-pm,
Amit Sunil Dhamne
From: Amit Sunil Dhamne <amitsd@google.com>
Add a helper function power_supply_get_by_fwnode() to retrieve
power_supply given a valid fwnode reference.
Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
---
drivers/power/supply/power_supply_core.c | 30 ++++++++++++++++++++++++++++++
include/linux/power_supply.h | 3 +++
2 files changed, 33 insertions(+)
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 76c340b38015af0a67a0d91305e6242a8646bf53..ef6ba22b837b0b9463f9a3065425e2720f40b9eb 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -496,6 +496,36 @@ struct power_supply *power_supply_get_by_name(const char *name)
}
EXPORT_SYMBOL_GPL(power_supply_get_by_name);
+static int power_supply_match_device_by_fwnode(struct device *dev, const void *data)
+{
+ return dev->parent && dev_fwnode(dev->parent) == data;
+}
+
+/**
+ * power_supply_get_by_fwnode() - Search for power supply given a fwnode ref.
+ * @fwnode: fwnode reference
+ *
+ * If power supply was found, it increases reference count for the internal
+ * power supply's device. The user should power_supply_put() after use.
+ *
+ * Return: Reference to power_supply node matching the fwnode on success or
+ * NULL on failure.
+ */
+struct power_supply *power_supply_get_by_fwnode(struct fwnode_handle *fwnode)
+{
+ struct power_supply *psy = NULL;
+ struct device *dev = class_find_device(&power_supply_class, NULL, fwnode,
+ power_supply_match_device_by_fwnode);
+
+ if (dev) {
+ psy = dev_get_drvdata(dev);
+ atomic_inc(&psy->use_cnt);
+ }
+
+ return psy;
+}
+EXPORT_SYMBOL_GPL(power_supply_get_by_fwnode);
+
/**
* 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 6ed53b292162469d7b357734d5589bff18a201d0..a35b08bd368e9305554e1a608dc8e526983cfa12 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -801,10 +801,13 @@ 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 void power_supply_put(struct power_supply *psy);
+extern struct power_supply *power_supply_get_by_fwnode(struct fwnode_handle *fwnode);
#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 struct power_supply *power_supply_get_by_fwnode(struct fwnode_handle *fwnode)
+{ return NULL; }
#endif
#ifdef CONFIG_OF
extern struct power_supply *power_supply_get_by_phandle(struct device_node *np,
--
2.49.0.987.g0cc8ee98dc-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 3/5] usb: typec: tcpm: Add support for Battery Status response message
2025-05-08 1:00 [PATCH v2 0/5] Add support for Battery Status & Battery Caps AMS in TCPM Amit Sunil Dhamne via B4 Relay
2025-05-08 1:00 ` [PATCH v2 1/5] dt-bindings: connector: extend ports property to model power connections Amit Sunil Dhamne via B4 Relay
2025-05-08 1:00 ` [PATCH v2 2/5] power: supply: core: add helper to get power supply given a fwnode Amit Sunil Dhamne via B4 Relay
@ 2025-05-08 1:00 ` Amit Sunil Dhamne via B4 Relay
2025-06-23 21:27 ` Sebastian Reichel
2025-05-08 1:00 ` [PATCH v2 4/5] power: supply: core: add vendor and product id properties Amit Sunil Dhamne via B4 Relay
2025-05-08 1:00 ` [PATCH v2 5/5] usb: typec: tcpm: Add support for Battery Cap response message Amit Sunil Dhamne via B4 Relay
4 siblings, 1 reply; 20+ messages in thread
From: Amit Sunil Dhamne via B4 Relay @ 2025-05-08 1:00 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek
Cc: Kyle Tso, devicetree, linux-kernel, linux-usb, linux-pm,
Amit Sunil Dhamne
From: Amit Sunil Dhamne <amitsd@google.com>
Add support for responding to Get_Battery_Status (extended) request with
a Battery_Status (data) msg. The requester shall request the status of
an individual battery by providing an index in Get_Battery_Status. In
case of failure to identify battery, the responder shall reply with an
appropriate message indicating so.
Battery status support is only provided for fixed batteries indexed from
0 - 3.
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>
Reviewed-by: Kyle Tso <kyletso@google.com>
---
drivers/usb/typec/tcpm/tcpm.c | 118 +++++++++++++++++++++++++++++++++++++++++-
include/linux/usb/pd.h | 34 ++++++++++++
2 files changed, 150 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 6bf1a22c785aff6b1ad77a20d85e22580527f5b1..98df0c7ce00e43f6c95ab49237a414e1b4413369 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -228,6 +228,7 @@ enum pd_msg_request {
PD_MSG_DATA_SINK_CAP,
PD_MSG_DATA_SOURCE_CAP,
PD_MSG_DATA_REV,
+ PD_MSG_DATA_BATT_STATUS
};
enum adev_actions {
@@ -332,6 +333,17 @@ struct pd_timings {
u32 snk_bc12_cmpletion_time;
};
+/*
+ * As per USB PD Spec Rev 3.18 (Sec. 6.5.13.11), a sink can have a maximum
+ * of 4 fixed batteries indexed [0, 3].
+ */
+#define MAX_NUM_FIXED_BATT 4
+
+#define BATTERY_PROPERTY_UNKNOWN 0xffff
+
+/* Convert microwatt to watt */
+#define UWH_TO_WH(pow) ((pow) / 1000000)
+
struct tcpm_port {
struct device *dev;
@@ -580,6 +592,15 @@ struct tcpm_port {
/* Indicates maximum (revision, version) supported */
struct pd_revision_info pd_rev;
+
+ struct power_supply *fixed_batt[MAX_NUM_FIXED_BATT];
+ u8 fixed_batt_cnt;
+
+ /*
+ * Variable used to store battery_ref from the Get_Battery_Status
+ * request to process Battery_Status messages.
+ */
+ u8 batt_request;
#ifdef CONFIG_DEBUG_FS
struct dentry *dentry;
struct mutex logbuffer_lock; /* log buffer access lock */
@@ -1339,6 +1360,60 @@ static int tcpm_pd_send_sink_caps(struct tcpm_port *port)
return tcpm_pd_transmit(port, TCPC_TX_SOP, &msg);
}
+static int tcpm_pd_send_batt_status(struct tcpm_port *port)
+{
+ struct pd_message msg;
+ struct power_supply *batt;
+ u32 bsdo;
+ u32 batt_id = port->batt_request;
+ union power_supply_propval val;
+ int ret;
+ bool batt_present = false;
+ u8 charging_status = BSDO_BATTERY_INFO_RSVD;
+ u16 present_charge = BATTERY_PROPERTY_UNKNOWN;
+
+ memset(&msg, 0, sizeof(msg));
+ if (batt_id < MAX_NUM_FIXED_BATT && port->fixed_batt[batt_id]) {
+ batt_present = true;
+ batt = port->fixed_batt[batt_id];
+ ret = power_supply_get_property(batt, POWER_SUPPLY_PROP_ENERGY_NOW, &val);
+ /* Battery Present Charge is reported in increments of 0.1WH */
+ if (!ret)
+ present_charge = (u16)UWH_TO_WH(val.intval * 10);
+
+ ret = power_supply_get_property(batt, POWER_SUPPLY_PROP_STATUS,
+ &val);
+ if (!ret) {
+ switch (val.intval) {
+ case POWER_SUPPLY_STATUS_CHARGING:
+ case POWER_SUPPLY_STATUS_FULL:
+ 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:
+ charging_status = BSDO_BATTERY_INFO_IDLE;
+ break;
+ default:
+ charging_status = BSDO_BATTERY_INFO_RSVD;
+ break;
+ }
+ }
+ }
+
+ bsdo = BSDO(present_charge, batt_present ? charging_status : 0,
+ batt_present, !batt_present);
+ 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);
+}
+
static void mod_tcpm_delayed_work(struct tcpm_port *port, unsigned int delay_ms)
{
if (delay_ms) {
@@ -3597,6 +3672,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)) {
@@ -3605,7 +3681,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;
@@ -3630,9 +3706,13 @@ static void tcpm_pd_ext_msg_request(struct tcpm_port *port,
NONE_AMS, 0);
}
break;
+ case PD_EXT_GET_BATT_STATUS:
+ port->batt_request = ext_msg->data[0];
+ tcpm_pd_handle_msg(port, PD_MSG_DATA_BATT_STATUS,
+ GETTING_BATTERY_STATUS);
+ 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:
@@ -3833,6 +3913,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;
}
@@ -7164,6 +7252,26 @@ static void tcpm_fw_get_timings(struct tcpm_port *port, struct fwnode_handle *fw
port->timings.snk_bc12_cmpletion_time = val;
}
+static void tcpm_fw_get_batt(struct tcpm_port *port, struct fwnode_handle *fwnode)
+{
+ struct fwnode_handle *ep = NULL, *fwnode_batt;
+ struct power_supply *psy;
+
+ fwnode_graph_for_each_endpoint(fwnode, ep) {
+ fwnode_batt = fwnode_graph_get_remote_port_parent(ep);
+ if (!fwnode_batt)
+ continue;
+
+ psy = power_supply_get_by_fwnode(fwnode_batt);
+ if (psy && psy->desc->type == POWER_SUPPLY_TYPE_BATTERY)
+ port->fixed_batt[port->fixed_batt_cnt++] = psy;
+
+ fwnode_handle_put(fwnode_batt);
+ if (port->fixed_batt_cnt == MAX_NUM_FIXED_BATT)
+ break;
+ }
+}
+
static int tcpm_fw_get_caps(struct tcpm_port *port, struct fwnode_handle *fwnode)
{
struct fwnode_handle *capabilities, *child, *caps = NULL;
@@ -7746,6 +7854,7 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
tcpm_fw_get_timings(port, tcpc->fwnode);
tcpm_fw_get_pd_revision(port, tcpc->fwnode);
+ tcpm_fw_get_batt(port, tcpc->fwnode);
port->try_role = port->typec_caps.prefer_role;
@@ -7827,6 +7936,11 @@ void tcpm_unregister_port(struct tcpm_port *port)
hrtimer_cancel(&port->vdm_state_machine_timer);
hrtimer_cancel(&port->state_machine_timer);
+ for (i = 0; i < port->fixed_batt_cnt; i++) {
+ if (port->fixed_batt[i])
+ power_supply_put(port->fixed_batt[i]);
+ }
+
tcpm_reset_port(port);
tcpm_port_unregister_pd(port);
diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
index 3068c3084eb6176d7d9184c3959a4110282a9fa0..4efa7bfd9863915dfc8048da264116d9b05a447b 100644
--- a/include/linux/usb/pd.h
+++ b/include/linux/usb/pd.h
@@ -553,4 +553,38 @@ void usb_power_delivery_unlink_device(struct usb_power_delivery *pd, struct devi
#endif /* CONFIG_TYPEC */
+/* Battery Status Data Object */
+#define BSDO_PRESENT_CAPACITY_SHIFT 16
+#define BSDO_PRESENT_CAPACITY_MASK GENMASK(31, 16)
+#define BSDO_CHG_STATUS_SHIFT 10
+#define BSDO_CHG_STATUS_MASK 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: When set, this indicates battery is present/attached.
+ * Otherwise:
+ * - Non hot-swappable battery: Indicates absence of battery
+ * - Hot-swappable battery: Indicates battery is unattached
+ * @invalid_ref: Set when invalid battery reference is made in
+ * Get_Battery_Status request, else 0
+ */
+#define BSDO(batt_charge, chg_status, batt_present, invalid_ref) \
+ ((((batt_charge) << BSDO_PRESENT_CAPACITY_SHIFT) & BSDO_PRESENT_CAPACITY_MASK) | \
+ (((chg_status) << BSDO_CHG_STATUS_SHIFT) & BSDO_CHG_STATUS_MASK) | \
+ ((batt_present) ? BSDO_BATTERY_PRESENT : 0) | \
+ ((invalid_ref) ? BSDO_INVALID_BATTERY_REFERENCE : 0))
+
#endif /* __LINUX_USB_PD_H */
--
2.49.0.987.g0cc8ee98dc-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 4/5] power: supply: core: add vendor and product id properties
2025-05-08 1:00 [PATCH v2 0/5] Add support for Battery Status & Battery Caps AMS in TCPM Amit Sunil Dhamne via B4 Relay
` (2 preceding siblings ...)
2025-05-08 1:00 ` [PATCH v2 3/5] usb: typec: tcpm: Add support for Battery Status response message Amit Sunil Dhamne via B4 Relay
@ 2025-05-08 1:00 ` Amit Sunil Dhamne via B4 Relay
2025-06-23 21:44 ` Sebastian Reichel
2025-05-08 1:00 ` [PATCH v2 5/5] usb: typec: tcpm: Add support for Battery Cap response message Amit Sunil Dhamne via B4 Relay
4 siblings, 1 reply; 20+ messages in thread
From: Amit Sunil Dhamne via B4 Relay @ 2025-05-08 1:00 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek
Cc: Kyle Tso, devicetree, linux-kernel, linux-usb, linux-pm,
Amit Sunil Dhamne
From: Amit Sunil Dhamne <amitsd@google.com>
Add the following properties:
* Vendor Identifier (VID): Assigned to the battery manufacturer by USB
Implementers Forum (USB-IF).
* Product Identifier (PID) assigned by the manufacturer to the
battery.
This info is required by USB Type-C PD devices containing batteries.
This enables the USB Type C devices to respond to a Battery capacity
request from the port partner by querying for the PID & VID assigned to
the batteries. Refer to "USB Power Delivery Specification Rev3.1 v1.8"
Chapter 5.5 Battery_Capabilities Message.
Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
---
Documentation/ABI/testing/sysfs-class-power | 19 +++++++++++++++----
Documentation/power/power_supply_class.rst | 11 +++++++++++
drivers/power/supply/power_supply_sysfs.c | 2 ++
include/linux/power_supply.h | 2 ++
4 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
index 2a5c1a09a28f91beec6b18ca7b4492093026bc81..5495e82885b2294cdfd5ace0e7e5fcbeadfccb5f 100644
--- a/Documentation/ABI/testing/sysfs-class-power
+++ b/Documentation/ABI/testing/sysfs-class-power
@@ -814,11 +814,22 @@ Description:
Access: Read
Valid values: 1-31
-What: /sys/class/power_supply/<supply_name>/extensions/<extension_name>
-Date: March 2025
+What: /sys/class/power_supply/<supply_name>/usbif_vendor_id
+Date: May 2025
Contact: linux-pm@vger.kernel.org
Description:
- Reports the extensions registered to the power supply.
- Each entry is a link to the device which registered the extension.
+ Reports the vendor id assigned to the battery manufacturer by USB
+ Implementers Forum (USB-IF).
Access: Read
+ Valid values: 0x0-0xffff
+
+What: /sys/class/power_supply/<supply_name>/usbif_product_id
+Date: May 2025
+Contact: linux-pm@vger.kernel.org
+Description:
+ Reports the product id assigned to the battery by the manufacturer
+ (associated with usbif_vendor_id).
+
+ Access: Read
+ Valid values: 0x0-0xffff
diff --git a/Documentation/power/power_supply_class.rst b/Documentation/power/power_supply_class.rst
index da8e275a14ffb9f84bae9ae1efc4720a55ea3010..6d0a6bcf501e719fa4454845b583a8b38d371bb4 100644
--- a/Documentation/power/power_supply_class.rst
+++ b/Documentation/power/power_supply_class.rst
@@ -213,6 +213,17 @@ TIME_TO_FULL
seconds left for battery to be considered full
(i.e. while battery is charging)
+USBIF_VENDOR_ID
+ Vendor ID (VID) assigned to manufacturer or device vendor associated with the
+ battery by USB Implementers Forum (USB-IF). This property is described in
+ "USB Power Delivery Specification Rev3.1 V1.8" Chapter 6.5.5 Battery
+ Capabilities, Section 6.5.5.1 Vendor ID (VID).
+USBIF_PRODUCT_ID
+ Product ID (PID) assigned to the battery, such that if the VID belongs to the
+ manufacturer then the PID will be designated by it. Similarly if the VID
+ belongs to the device vendor then the PID will be designated by it. This
+ property is described in "USB Power Delivery Specification Rev3.1 V1.8"
+ Chapter 6.5.5 Battery Capabilities, Section 6.5.5.2 Product ID (PID).
Battery <-> external power supply interaction
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index edb058c19c9c44ad9ad97a626fc8f59e3d3735a6..534ed3cd049866fa747455bb6dae1ec2dc5e2da6 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -211,6 +211,8 @@ static struct power_supply_attr power_supply_attrs[] __ro_after_init = {
POWER_SUPPLY_ATTR(TIME_TO_EMPTY_AVG),
POWER_SUPPLY_ATTR(TIME_TO_FULL_NOW),
POWER_SUPPLY_ATTR(TIME_TO_FULL_AVG),
+ POWER_SUPPLY_ATTR(USBIF_VENDOR_ID),
+ POWER_SUPPLY_ATTR(USBIF_PRODUCT_ID),
POWER_SUPPLY_ENUM_ATTR(TYPE),
POWER_SUPPLY_ENUM_ATTR(USB_TYPE),
POWER_SUPPLY_ENUM_ATTR(SCOPE),
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index a35b08bd368e9305554e1a608dc8e526983cfa12..100eb559dcede938595ffbf83bc5ef3645a5a172 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -165,6 +165,8 @@ enum power_supply_property {
POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
POWER_SUPPLY_PROP_TIME_TO_FULL_NOW,
POWER_SUPPLY_PROP_TIME_TO_FULL_AVG,
+ POWER_SUPPLY_PROP_USBIF_VENDOR_ID,
+ POWER_SUPPLY_PROP_USBIF_PRODUCT_ID,
POWER_SUPPLY_PROP_TYPE, /* use power_supply.type instead */
POWER_SUPPLY_PROP_USB_TYPE,
POWER_SUPPLY_PROP_SCOPE,
--
2.49.0.987.g0cc8ee98dc-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 5/5] usb: typec: tcpm: Add support for Battery Cap response message
2025-05-08 1:00 [PATCH v2 0/5] Add support for Battery Status & Battery Caps AMS in TCPM Amit Sunil Dhamne via B4 Relay
` (3 preceding siblings ...)
2025-05-08 1:00 ` [PATCH v2 4/5] power: supply: core: add vendor and product id properties Amit Sunil Dhamne via B4 Relay
@ 2025-05-08 1:00 ` Amit Sunil Dhamne via B4 Relay
2025-06-23 21:51 ` Sebastian Reichel
4 siblings, 1 reply; 20+ messages in thread
From: Amit Sunil Dhamne via B4 Relay @ 2025-05-08 1:00 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Sebastian Reichel,
Heikki Krogerus, Rafael J. Wysocki, Len Brown, Pavel Machek
Cc: Kyle Tso, devicetree, linux-kernel, linux-usb, linux-pm,
Amit Sunil Dhamne
From: Amit Sunil Dhamne <amitsd@google.com>
Add support for responding to Get_Battery_Cap (extended) request with a
a Battery_Capabilities (extended) msg. The requester will request
Battery Cap for a specific battery using an index in Get_Battery_Cap. In
case of failure to identify battery, TCPM shall reply with an
appropriate message indicating so.
Support has been added only for fixed batteries and not hot swappable
ones.
As the Battery Cap Data Block size is 9 Bytes (lesser than
MaxExtendedMsgChunkLen of 26B), only a single chunk is required to
complete the AMS.
Support for Battery_Capabilities 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>
Reviewed-by: Kyle Tso <kyletso@google.com>
---
drivers/usb/typec/tcpm/tcpm.c | 96 +++++++++++++++++++++++++++++++++++++++++--
include/linux/usb/pd.h | 31 ++++++++++++++
2 files changed, 123 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 98df0c7ce00e43f6c95ab49237a414e1b4413369..4731126fbf19a50178be0cf8867b3fe08595724c 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -228,7 +228,8 @@ enum pd_msg_request {
PD_MSG_DATA_SINK_CAP,
PD_MSG_DATA_SOURCE_CAP,
PD_MSG_DATA_REV,
- PD_MSG_DATA_BATT_STATUS
+ PD_MSG_DATA_BATT_STATUS,
+ PD_MSG_EXT_BATT_CAP,
};
enum adev_actions {
@@ -597,8 +598,8 @@ struct tcpm_port {
u8 fixed_batt_cnt;
/*
- * Variable used to store battery_ref from the Get_Battery_Status
- * request to process Battery_Status messages.
+ * Variable used to store battery_ref from the Get_Battery_Status &
+ * Get_Battery_Caps request to process Battery_Status messages.
*/
u8 batt_request;
#ifdef CONFIG_DEBUG_FS
@@ -1414,6 +1415,81 @@ static int tcpm_pd_send_batt_status(struct tcpm_port *port)
return tcpm_pd_transmit(port, TCPC_TX_SOP, &msg);
}
+static int tcpm_pd_send_batt_cap(struct tcpm_port *port)
+{
+ struct pd_message msg;
+ struct power_supply *batt;
+ struct batt_cap_ext_msg bcdb;
+ u32 batt_id = port->batt_request;
+ int ret;
+ union power_supply_propval val;
+ bool batt_present = false;
+ u16 batt_design_cap = BATTERY_PROPERTY_UNKNOWN;
+ u16 batt_charge_cap = BATTERY_PROPERTY_UNKNOWN;
+ u8 data_obj_cnt;
+ /*
+ * As per USB PD Rev3.1 v1.8, if battery reference is incorrect,
+ * then set the VID field to 0xffff.
+ * If VID field is set to 0xffff, always set the PID field to
+ * 0x0000.
+ */
+ u16 vid = BATTERY_PROPERTY_UNKNOWN;
+ u16 pid = 0x0;
+
+ memset(&msg, 0, sizeof(msg));
+
+ if (batt_id < MAX_NUM_FIXED_BATT && port->fixed_batt[batt_id]) {
+ batt_present = true;
+ batt = port->fixed_batt[batt_id];
+ ret = power_supply_get_property(batt,
+ POWER_SUPPLY_PROP_USBIF_VENDOR_ID,
+ &val);
+ if (!ret)
+ vid = val.intval;
+
+ if (vid != BATTERY_PROPERTY_UNKNOWN) {
+ ret = power_supply_get_property(batt,
+ POWER_SUPPLY_PROP_USBIF_PRODUCT_ID,
+ &val);
+ if (!ret)
+ pid = val.intval;
+ }
+
+ ret = power_supply_get_property(batt,
+ POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
+ &val);
+ if (!ret)
+ batt_design_cap = (u16)UWH_TO_WH(val.intval * 10);
+
+ ret = power_supply_get_property(batt,
+ POWER_SUPPLY_PROP_ENERGY_FULL,
+ &val);
+ if (!ret)
+ batt_charge_cap = (u16)UWH_TO_WH(val.intval * 10);
+ }
+
+ bcdb.vid = cpu_to_le16(vid);
+ bcdb.pid = cpu_to_le16(pid);
+ bcdb.batt_design_cap = cpu_to_le16(batt_design_cap);
+ bcdb.batt_last_chg_cap = cpu_to_le16(batt_charge_cap);
+ bcdb.batt_type = !batt_present ? BATT_CAP_BATT_TYPE_INVALID_REF : 0;
+ memcpy(msg.ext_msg.data, &bcdb, sizeof(bcdb));
+ msg.ext_msg.header = PD_EXT_HDR_LE(sizeof(bcdb),
+ 0, /* Denotes if request chunk */
+ 0, /* Chunk number */
+ 1 /* Chunked */);
+
+ data_obj_cnt = count_chunked_data_objs(sizeof(bcdb));
+ msg.header = cpu_to_le16(PD_HEADER(PD_EXT_BATT_CAP,
+ port->pwr_role,
+ port->data_role,
+ port->negotiated_rev,
+ port->message_id,
+ data_obj_cnt,
+ 1 /* Denotes if ext header */));
+ return tcpm_pd_transmit(port, TCPC_TX_SOP, &msg);
+}
+
static void mod_tcpm_delayed_work(struct tcpm_port *port, unsigned int delay_ms)
{
if (delay_ms) {
@@ -3711,8 +3787,12 @@ static void tcpm_pd_ext_msg_request(struct tcpm_port *port,
tcpm_pd_handle_msg(port, PD_MSG_DATA_BATT_STATUS,
GETTING_BATTERY_STATUS);
break;
- case PD_EXT_SOURCE_CAP_EXT:
case PD_EXT_GET_BATT_CAP:
+ port->batt_request = ext_msg->data[0];
+ tcpm_pd_handle_msg(port, PD_MSG_EXT_BATT_CAP,
+ GETTING_BATTERY_CAPABILITIES);
+ break;
+ case PD_EXT_SOURCE_CAP_EXT:
case PD_EXT_BATT_CAP:
case PD_EXT_GET_MANUFACTURER_INFO:
case PD_EXT_MANUFACTURER_INFO:
@@ -3921,6 +4001,14 @@ static bool tcpm_send_queued_message(struct tcpm_port *port)
ret);
tcpm_ams_finish(port);
break;
+ case PD_MSG_EXT_BATT_CAP:
+ ret = tcpm_pd_send_batt_cap(port);
+ if (ret)
+ tcpm_log(port,
+ "Failed to send battery cap ret=%d",
+ ret);
+ tcpm_ams_finish(port);
+ break;
default:
break;
}
diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
index 4efa7bfd9863915dfc8048da264116d9b05a447b..c89594177da57f4398b75c89c1991b4937614a70 100644
--- a/include/linux/usb/pd.h
+++ b/include/linux/usb/pd.h
@@ -204,6 +204,37 @@ struct pd_message {
};
} __packed;
+/*
+ * count_chunked_data_objs: Helper to calculate number of Data Objects on a 4
+ * byte boundary.
+ * @size: Size of data block for extended message. Should *not* include extended
+ * header size.
+ */
+static inline u8 count_chunked_data_objs(u32 size)
+{
+ size += offsetof(struct pd_chunked_ext_message_data, data);
+ return ((size / 4) + (size % 4 ? 1 : 0));
+}
+
+/**
+ * batt_cap_ext_msg - Battery capability extended PD message
+ * @vid: Battery Vendor ID (assigned by USB-IF)
+ * @pid: Battery Product ID (assigned by battery or device vendor)
+ * @batt_design_cap: Battery design capacity in 0.1Wh
+ * @batt_last_chg_cap: Battery last full charge capacity in 0.1Wh
+ * @batt_type: Battery Type. bit0 when set indicates invalid battery reference.
+ * Rest of the bits are reserved.
+ */
+struct batt_cap_ext_msg {
+ __le16 vid;
+ __le16 pid;
+ __le16 batt_design_cap;
+ __le16 batt_last_chg_cap;
+ u8 batt_type;
+} __packed;
+
+#define BATT_CAP_BATT_TYPE_INVALID_REF BIT(0)
+
/* PDO: Power Data Object */
#define PDO_MAX_OBJECTS 7
--
2.49.0.987.g0cc8ee98dc-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/5] dt-bindings: connector: extend ports property to model power connections
2025-05-08 1:00 ` [PATCH v2 1/5] dt-bindings: connector: extend ports property to model power connections Amit Sunil Dhamne via B4 Relay
@ 2025-05-08 2:08 ` Rob Herring (Arm)
2025-05-13 5:10 ` Amit Sunil Dhamne
2025-05-14 19:42 ` Rob Herring
1 sibling, 1 reply; 20+ messages in thread
From: Rob Herring (Arm) @ 2025-05-08 2:08 UTC (permalink / raw)
To: Amit Sunil Dhamne
Cc: Len Brown, Kyle Tso, linux-pm, linux-usb, Krzysztof Kozlowski,
Greg Kroah-Hartman, Conor Dooley, Heikki Krogerus, linux-kernel,
Rafael J. Wysocki, devicetree, Sebastian Reichel,
Badhri Jagan Sridharan, Pavel Machek
On Wed, 07 May 2025 18:00:22 -0700, Amit Sunil Dhamne wrote:
> Extend ports property to model power lines going between connector to
> charger or battery/batteries. As an example, connector VBUS can supply
> power in & out of the battery for a DRP.
>
> Additionally, add ports property to maxim,max33359 controller example.
>
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> ---
> .../bindings/connector/usb-connector.yaml | 20 +++++++++++------
> .../devicetree/bindings/usb/maxim,max33359.yaml | 25 ++++++++++++++++++++++
> 2 files changed, 38 insertions(+), 7 deletions(-)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250507-batt_ops-v2-1-8d06130bffe6@google.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/5] dt-bindings: connector: extend ports property to model power connections
2025-05-08 2:08 ` Rob Herring (Arm)
@ 2025-05-13 5:10 ` Amit Sunil Dhamne
0 siblings, 0 replies; 20+ messages in thread
From: Amit Sunil Dhamne @ 2025-05-13 5:10 UTC (permalink / raw)
To: Rob Herring (Arm)
Cc: Len Brown, Kyle Tso, linux-pm, linux-usb, Krzysztof Kozlowski,
Greg Kroah-Hartman, Conor Dooley, Heikki Krogerus, linux-kernel,
Rafael J. Wysocki, devicetree, Sebastian Reichel,
Badhri Jagan Sridharan, Pavel Machek
Hi Rob,
On 5/7/25 7:08 PM, Rob Herring (Arm) wrote:
> On Wed, 07 May 2025 18:00:22 -0700, Amit Sunil Dhamne wrote:
>> Extend ports property to model power lines going between connector to
>> charger or battery/batteries. As an example, connector VBUS can supply
>> power in & out of the battery for a DRP.
>>
>> Additionally, add ports property to maxim,max33359 controller example.
>>
>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>> ---
>> .../bindings/connector/usb-connector.yaml | 20 +++++++++++------
>> .../devicetree/bindings/usb/maxim,max33359.yaml | 25 ++++++++++++++++++++++
>> 2 files changed, 38 insertions(+), 7 deletions(-)
>>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> yamllint warnings/errors:
I ran this and didn't see any errors on my side.
> dtschema/dtc warnings/errors:
>
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250507-batt_ops-v2-1-8d06130bffe6@google.com
Even the build logs don't show any error log.
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
My patchset is based on v6.14-rc6 and I tested it on that.
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
>
I did all of the above but make dt_binding_check still passes.
(.venv) amitsd@amitsd-gti:~/linaro-p6-image/src/linux$ make
dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/connector/usb-connector.yaml
SCHEMA Documentation/devicetree/bindings/processed-schema.json
/usr/local/google/home/amitsd/linaro-p6-image/src/linux/Documentation/devicetree/bindings/net/snps,dwmac.yaml:
mac-mode: missing type definition
^ This is not newly introduced jfyi.
CHKDT ./Documentation/devicetree/bindings
LINT ./Documentation/devicetree/bindings
DTEX
Documentation/devicetree/bindings/connector/usb-connector.example.dts
DTC [C]
Documentation/devicetree/bindings/connector/usb-connector.example.dtb
(.venv) amitsd@amitsd-gti:~/linaro-p6-image/src/linux$ make
dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/usb/maxim,max33359.yaml
CHKDT ./Documentation/devicetree/bindings
LINT ./Documentation/devicetree/bindings
DTEX Documentation/devicetree/bindings/usb/maxim,max33359.example.dts
DTC [C] Documentation/devicetree/bindings/usb/maxim,max33359.example.dtb
Please can you advise on what I may be missing?
Thanks,
Amit
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/5] dt-bindings: connector: extend ports property to model power connections
2025-05-08 1:00 ` [PATCH v2 1/5] dt-bindings: connector: extend ports property to model power connections Amit Sunil Dhamne via B4 Relay
2025-05-08 2:08 ` Rob Herring (Arm)
@ 2025-05-14 19:42 ` Rob Herring
2025-05-20 20:10 ` Amit Sunil Dhamne
1 sibling, 1 reply; 20+ messages in thread
From: Rob Herring @ 2025-05-14 19:42 UTC (permalink / raw)
To: Amit Sunil Dhamne
Cc: Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
Badhri Jagan Sridharan, Sebastian Reichel, Heikki Krogerus,
Rafael J. Wysocki, Len Brown, Pavel Machek, Kyle Tso, devicetree,
linux-kernel, linux-usb, linux-pm
On Wed, May 07, 2025 at 06:00:22PM -0700, Amit Sunil Dhamne wrote:
> Extend ports property to model power lines going between connector to
> charger or battery/batteries. As an example, connector VBUS can supply
> power in & out of the battery for a DRP.
>
> Additionally, add ports property to maxim,max33359 controller example.
>
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> ---
> .../bindings/connector/usb-connector.yaml | 20 +++++++++++------
> .../devicetree/bindings/usb/maxim,max33359.yaml | 25 ++++++++++++++++++++++
> 2 files changed, 38 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> index 11e40d225b9f3a0d0aeea7bf764f1c00a719d615..706094f890026d324e6ece8b0c1e831d04d51eb7 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> @@ -181,16 +181,16 @@ properties:
>
> port:
> $ref: /schemas/graph.yaml#/properties/port
> - description: OF graph bindings modeling a data bus to the connector, e.g.
> - there is a single High Speed (HS) port present in this connector. If there
> - is more than one bus (several port, with 'reg' property), they can be grouped
> - under 'ports'.
> + description: OF graph binding to model a logical connection between a device
> + and connector. This connection may represent a data bus or power line. For
> + e.g. a High Speed (HS) data port present in this connector or VBUS line.
> + If there is more than one connection (several port, with 'reg' property),
> + they can be grouped under 'ports'.
'port' and 'port@0' are equivalent. So you can't be changing its
definition.
I'm not sure showing a power connection with the graph is the right
approach. We have a binding for that already with the regulator binding.
Perhaps the connector needs to be a supply. It's already using that
binding in the supplying power to the connector case.
Rob
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/5] dt-bindings: connector: extend ports property to model power connections
2025-05-14 19:42 ` Rob Herring
@ 2025-05-20 20:10 ` Amit Sunil Dhamne
2025-05-28 18:42 ` Amit Sunil Dhamne
2025-06-23 22:08 ` Sebastian Reichel
0 siblings, 2 replies; 20+ messages in thread
From: Amit Sunil Dhamne @ 2025-05-20 20:10 UTC (permalink / raw)
To: Rob Herring
Cc: Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
Badhri Jagan Sridharan, Sebastian Reichel, Heikki Krogerus,
Rafael J. Wysocki, Len Brown, Pavel Machek, Kyle Tso, devicetree,
linux-kernel, linux-usb, linux-pm
Hi Rob,
Thanks for your response!
On 5/14/25 12:42 PM, Rob Herring wrote:
> On Wed, May 07, 2025 at 06:00:22PM -0700, Amit Sunil Dhamne wrote:
>> Extend ports property to model power lines going between connector to
>> charger or battery/batteries. As an example, connector VBUS can supply
>> power in & out of the battery for a DRP.
>>
>> Additionally, add ports property to maxim,max33359 controller example.
>>
>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>> ---
>> .../bindings/connector/usb-connector.yaml | 20 +++++++++++------
>> .../devicetree/bindings/usb/maxim,max33359.yaml | 25 ++++++++++++++++++++++
>> 2 files changed, 38 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> index 11e40d225b9f3a0d0aeea7bf764f1c00a719d615..706094f890026d324e6ece8b0c1e831d04d51eb7 100644
>> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> @@ -181,16 +181,16 @@ properties:
>>
>> port:
>> $ref: /schemas/graph.yaml#/properties/port
>> - description: OF graph bindings modeling a data bus to the connector, e.g.
>> - there is a single High Speed (HS) port present in this connector. If there
>> - is more than one bus (several port, with 'reg' property), they can be grouped
>> - under 'ports'.
>> + description: OF graph binding to model a logical connection between a device
>> + and connector. This connection may represent a data bus or power line. For
>> + e.g. a High Speed (HS) data port present in this connector or VBUS line.
>> + If there is more than one connection (several port, with 'reg' property),
>> + they can be grouped under 'ports'.
> 'port' and 'port@0' are equivalent. So you can't be changing its
> definition.
Noted!
> I'm not sure showing a power connection with the graph is the right
> approach.
I want to provide some more context and rationale behind using this design.
From a hardware perspective:
The max77759/max33359 IC has Type-C port controller, charger, fuel gauge
(FG) ICs. The Vbus from the connector goes to/from the TCPC and connects
with the charger IP via circuitry & from there on to the battery. The FG
is connected to the battery in parallel. As it can be seen that while
these IPs are interconnected, there's no direct connection of the fuel
gauge & the connector.
For this feature, I am interested in getting the reference to the FG. As
per graph description: "...These common bindings do not contain any
information about the direction or type of the connections, they just
map their existence." This works for my case because I just want the
connector to be aware of the Fuel gauge device without imposing a
specific directionality in terms of power supplier/supplied. This is
also the reason why I didn't use
"/schemas/power/supply/power-supply.yaml#power-supplies" binding.
> We have a binding for that already with the regulator binding.
I haven't explored the option of using regulator bindings. But in my
case I am interested in fuel gauge and unfortunately, they're modeled as
power_supply devices.
>
> Perhaps the connector needs to be a supply. It's already using that
> binding in the supplying power to the connector case.
Want to clarify, in this case you mean
/schemas/regulator/regulator.yaml#*-supply$ right?
Adding to my response above, the reason I don't want to impose a
directionality in terms of supplier/supplied is that in case of USB Dual
Role Port they're dynamic i.e., when USB is source, the power is
supplied out of the battery (battery/FG will be supplier) and in case
USB is sink, battery is supplied power. Whether the connector port is in
source or sink role is determined on a connection to connection basis.
Also, the knowledge of the supply direction is of no consequence for
this feature.
Please let me know what you think.
Thanks,
Amit
> Rob
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/5] dt-bindings: connector: extend ports property to model power connections
2025-05-20 20:10 ` Amit Sunil Dhamne
@ 2025-05-28 18:42 ` Amit Sunil Dhamne
2025-06-23 22:08 ` Sebastian Reichel
1 sibling, 0 replies; 20+ messages in thread
From: Amit Sunil Dhamne @ 2025-05-28 18:42 UTC (permalink / raw)
To: Rob Herring
Cc: Krzysztof Kozlowski, Conor Dooley, Greg Kroah-Hartman,
Badhri Jagan Sridharan, Sebastian Reichel, Heikki Krogerus,
Rafael J. Wysocki, Len Brown, Pavel Machek, Kyle Tso, devicetree,
linux-kernel, linux-usb, linux-pm, tudor.ambarus, andre.draszik,
peter.griffin
Hi,
On 5/20/25 1:10 PM, Amit Sunil Dhamne wrote:
> Hi Rob,
>
> Thanks for your response!
>
> On 5/14/25 12:42 PM, Rob Herring wrote:
>> On Wed, May 07, 2025 at 06:00:22PM -0700, Amit Sunil Dhamne wrote:
>>> Extend ports property to model power lines going between connector to
>>> charger or battery/batteries. As an example, connector VBUS can supply
>>> power in & out of the battery for a DRP.
>>>
>>> Additionally, add ports property to maxim,max33359 controller example.
>>>
>>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>>> ---
>>> .../bindings/connector/usb-connector.yaml | 20 +++++++++++------
>>> .../devicetree/bindings/usb/maxim,max33359.yaml | 25 ++++++++++++++++++++++
>>> 2 files changed, 38 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>> index 11e40d225b9f3a0d0aeea7bf764f1c00a719d615..706094f890026d324e6ece8b0c1e831d04d51eb7 100644
>>> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>> @@ -181,16 +181,16 @@ properties:
>>>
>>> port:
>>> $ref: /schemas/graph.yaml#/properties/port
>>> - description: OF graph bindings modeling a data bus to the connector, e.g.
>>> - there is a single High Speed (HS) port present in this connector. If there
>>> - is more than one bus (several port, with 'reg' property), they can be grouped
>>> - under 'ports'.
>>> + description: OF graph binding to model a logical connection between a device
>>> + and connector. This connection may represent a data bus or power line. For
>>> + e.g. a High Speed (HS) data port present in this connector or VBUS line.
>>> + If there is more than one connection (several port, with 'reg' property),
>>> + they can be grouped under 'ports'.
>> 'port' and 'port@0' are equivalent. So you can't be changing its
>> definition.
> Noted!
>
>
>> I'm not sure showing a power connection with the graph is the right
>> approach.
> I want to provide some more context and rationale behind using this design.
>
> From a hardware perspective:
>
> The max77759/max33359 IC has Type-C port controller, charger, fuel gauge
> (FG) ICs. The Vbus from the connector goes to/from the TCPC and connects
> with the charger IP via circuitry & from there on to the battery. The FG
> is connected to the battery in parallel. As it can be seen that while
> these IPs are interconnected, there's no direct connection of the fuel
> gauge & the connector.
>
> For this feature, I am interested in getting the reference to the FG. As
> per graph description: "...These common bindings do not contain any
> information about the direction or type of the connections, they just
> map their existence." This works for my case because I just want the
> connector to be aware of the Fuel gauge device without imposing a
> specific directionality in terms of power supplier/supplied. This is
> also the reason why I didn't use
> "/schemas/power/supply/power-supply.yaml#power-supplies" binding.
>
>> We have a binding for that already with the regulator binding.
> I haven't explored the option of using regulator bindings. But in my
> case I am interested in fuel gauge and unfortunately, they're modeled as
> power_supply devices.
>
>
>>
>> Perhaps the connector needs to be a supply. It's already using that
>> binding in the supplying power to the connector case.
> Want to clarify, in this case you mean
> /schemas/regulator/regulator.yaml#*-supply$ right?
>
> Adding to my response above, the reason I don't want to impose a
> directionality in terms of supplier/supplied is that in case of USB Dual
> Role Port they're dynamic i.e., when USB is source, the power is
> supplied out of the battery (battery/FG will be supplier) and in case
> USB is sink, battery is supplied power. Whether the connector port is in
> source or sink role is determined on a connection to connection basis.
> Also, the knowledge of the supply direction is of no consequence for
> this feature.
>
>
> Please let me know what you think.
>
> Thanks,
>
> Amit
I wanted to follow up on my previous responses. Please let me know if
you have any further questions or concerns.
Thanks,
Amit
>
>> Rob
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/5] power: supply: core: add helper to get power supply given a fwnode
2025-05-08 1:00 ` [PATCH v2 2/5] power: supply: core: add helper to get power supply given a fwnode Amit Sunil Dhamne via B4 Relay
@ 2025-06-23 21:21 ` Sebastian Reichel
2025-07-08 0:53 ` Amit Sunil Dhamne
0 siblings, 1 reply; 20+ messages in thread
From: Sebastian Reichel @ 2025-06-23 21:21 UTC (permalink / raw)
To: amitsd
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Heikki Krogerus,
Rafael J. Wysocki, Len Brown, Pavel Machek, Kyle Tso, devicetree,
linux-kernel, linux-usb, linux-pm
[-- Attachment #1: Type: text/plain, Size: 3368 bytes --]
Hi,
On Wed, May 07, 2025 at 06:00:23PM -0700, Amit Sunil Dhamne via B4 Relay wrote:
> From: Amit Sunil Dhamne <amitsd@google.com>
>
> Add a helper function power_supply_get_by_fwnode() to retrieve
> power_supply given a valid fwnode reference.
>
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> ---
> drivers/power/supply/power_supply_core.c | 30 ++++++++++++++++++++++++++++++
> include/linux/power_supply.h | 3 +++
> 2 files changed, 33 insertions(+)
>
> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> index 76c340b38015af0a67a0d91305e6242a8646bf53..ef6ba22b837b0b9463f9a3065425e2720f40b9eb 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -496,6 +496,36 @@ struct power_supply *power_supply_get_by_name(const char *name)
> }
> EXPORT_SYMBOL_GPL(power_supply_get_by_name);
>
> +static int power_supply_match_device_by_fwnode(struct device *dev, const void *data)
> +{
> + return dev->parent && dev_fwnode(dev->parent) == data;
> +}
This already exists as power_supply_match_device_fwnode().
> +
> +/**
> + * power_supply_get_by_fwnode() - Search for power supply given a fwnode ref.
> + * @fwnode: fwnode reference
> + *
> + * If power supply was found, it increases reference count for the internal
> + * power supply's device. The user should power_supply_put() after use.
> + *
> + * Return: Reference to power_supply node matching the fwnode on success or
> + * NULL on failure.
> + */
> +struct power_supply *power_supply_get_by_fwnode(struct fwnode_handle *fwnode)
> +{
> + struct power_supply *psy = NULL;
> + struct device *dev = class_find_device(&power_supply_class, NULL, fwnode,
> + power_supply_match_device_by_fwnode);
> +
> + if (dev) {
> + psy = dev_get_drvdata(dev);
> + atomic_inc(&psy->use_cnt);
> + }
> +
> + return psy;
> +}
> +EXPORT_SYMBOL_GPL(power_supply_get_by_fwnode);
And this is a 50% of power_supply_get_by_reference(), so the
function should be updated to use power_supply_get_by_fwnode().
Greetings,
-- Sebastian
> /**
> * 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 6ed53b292162469d7b357734d5589bff18a201d0..a35b08bd368e9305554e1a608dc8e526983cfa12 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -801,10 +801,13 @@ 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 void power_supply_put(struct power_supply *psy);
> +extern struct power_supply *power_supply_get_by_fwnode(struct fwnode_handle *fwnode);
> #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 struct power_supply *power_supply_get_by_fwnode(struct fwnode_handle *fwnode)
> +{ return NULL; }
> #endif
> #ifdef CONFIG_OF
> extern struct power_supply *power_supply_get_by_phandle(struct device_node *np,
>
> --
> 2.49.0.987.g0cc8ee98dc-goog
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/5] usb: typec: tcpm: Add support for Battery Status response message
2025-05-08 1:00 ` [PATCH v2 3/5] usb: typec: tcpm: Add support for Battery Status response message Amit Sunil Dhamne via B4 Relay
@ 2025-06-23 21:27 ` Sebastian Reichel
2025-07-08 0:55 ` Amit Sunil Dhamne
0 siblings, 1 reply; 20+ messages in thread
From: Sebastian Reichel @ 2025-06-23 21:27 UTC (permalink / raw)
To: amitsd
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Heikki Krogerus,
Rafael J. Wysocki, Len Brown, Pavel Machek, Kyle Tso, devicetree,
linux-kernel, linux-usb, linux-pm
[-- Attachment #1: Type: text/plain, Size: 1551 bytes --]
Hi,
On Wed, May 07, 2025 at 06:00:24PM -0700, Amit Sunil Dhamne via B4 Relay wrote:
> From: Amit Sunil Dhamne <amitsd@google.com>
>
> Add support for responding to Get_Battery_Status (extended) request with
> a Battery_Status (data) msg. The requester shall request the status of
> an individual battery by providing an index in Get_Battery_Status. In
> case of failure to identify battery, the responder shall reply with an
> appropriate message indicating so.
>
> Battery status support is only provided for fixed batteries indexed from
> 0 - 3.
>
> 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>
> Reviewed-by: Kyle Tso <kyletso@google.com>
> ---
(partial review)
> +static int tcpm_pd_send_batt_status(struct tcpm_port *port)
> +{
> + struct pd_message msg;
> + struct power_supply *batt;
> + u32 bsdo;
> + u32 batt_id = port->batt_request;
> + union power_supply_propval val;
> + int ret;
> + bool batt_present = false;
> + u8 charging_status = BSDO_BATTERY_INFO_RSVD;
> + u16 present_charge = BATTERY_PROPERTY_UNKNOWN;
> +
> + memset(&msg, 0, sizeof(msg));
> + if (batt_id < MAX_NUM_FIXED_BATT && port->fixed_batt[batt_id]) {
> + batt_present = true;
power_supply_get_property(batt, POWER_SUPPLY_PROP_PRESENT, &batt_present);
> ...
Greetings,
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 4/5] power: supply: core: add vendor and product id properties
2025-05-08 1:00 ` [PATCH v2 4/5] power: supply: core: add vendor and product id properties Amit Sunil Dhamne via B4 Relay
@ 2025-06-23 21:44 ` Sebastian Reichel
2025-07-08 1:05 ` Amit Sunil Dhamne
0 siblings, 1 reply; 20+ messages in thread
From: Sebastian Reichel @ 2025-06-23 21:44 UTC (permalink / raw)
To: amitsd
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Heikki Krogerus,
Rafael J. Wysocki, Len Brown, Pavel Machek, Kyle Tso, devicetree,
linux-kernel, linux-usb, linux-pm
[-- Attachment #1: Type: text/plain, Size: 5516 bytes --]
Hi,
On Wed, May 07, 2025 at 06:00:25PM -0700, Amit Sunil Dhamne via B4 Relay wrote:
> From: Amit Sunil Dhamne <amitsd@google.com>
>
> Add the following properties:
> * Vendor Identifier (VID): Assigned to the battery manufacturer by USB
> Implementers Forum (USB-IF).
> * Product Identifier (PID) assigned by the manufacturer to the
> battery.
>
> This info is required by USB Type-C PD devices containing batteries.
> This enables the USB Type C devices to respond to a Battery capacity
> request from the port partner by querying for the PID & VID assigned to
> the batteries. Refer to "USB Power Delivery Specification Rev3.1 v1.8"
> Chapter 5.5 Battery_Capabilities Message.
>
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> ---
> Documentation/ABI/testing/sysfs-class-power | 19 +++++++++++++++----
> Documentation/power/power_supply_class.rst | 11 +++++++++++
> drivers/power/supply/power_supply_sysfs.c | 2 ++
> include/linux/power_supply.h | 2 ++
> 4 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
> index 2a5c1a09a28f91beec6b18ca7b4492093026bc81..5495e82885b2294cdfd5ace0e7e5fcbeadfccb5f 100644
> --- a/Documentation/ABI/testing/sysfs-class-power
> +++ b/Documentation/ABI/testing/sysfs-class-power
> @@ -814,11 +814,22 @@ Description:
> Access: Read
> Valid values: 1-31
>
> -What: /sys/class/power_supply/<supply_name>/extensions/<extension_name>
> -Date: March 2025
Why does this remove existing Documentation?
> +What: /sys/class/power_supply/<supply_name>/usbif_vendor_id
I think we can use USB_VENDOR_ID and USB_PRODUCT_ID for this like
everyone else?
> +Date: May 2025
> Contact: linux-pm@vger.kernel.org
> Description:
> - Reports the extensions registered to the power supply.
> - Each entry is a link to the device which registered the extension.
> + Reports the vendor id assigned to the battery manufacturer by USB
> + Implementers Forum (USB-IF).
>
> Access: Read
> + Valid values: 0x0-0xffff
If I haven't missed something the formatting will be in decimal. I
think the hex format is more sensible, so this needs some extra
handling in power_supply_format_property() in power_supply_sysfs.c.
> +
> +What: /sys/class/power_supply/<supply_name>/usbif_product_id
> +Date: May 2025
> +Contact: linux-pm@vger.kernel.org
> +Description:
> + Reports the product id assigned to the battery by the manufacturer
> + (associated with usbif_vendor_id).
> +
> + Access: Read
> + Valid values: 0x0-0xffff
> diff --git a/Documentation/power/power_supply_class.rst b/Documentation/power/power_supply_class.rst
> index da8e275a14ffb9f84bae9ae1efc4720a55ea3010..6d0a6bcf501e719fa4454845b583a8b38d371bb4 100644
> --- a/Documentation/power/power_supply_class.rst
> +++ b/Documentation/power/power_supply_class.rst
> @@ -213,6 +213,17 @@ TIME_TO_FULL
> seconds left for battery to be considered full
> (i.e. while battery is charging)
>
> +USBIF_VENDOR_ID
> + Vendor ID (VID) assigned to manufacturer or device vendor associated with the
> + battery by USB Implementers Forum (USB-IF). This property is described in
> + "USB Power Delivery Specification Rev3.1 V1.8" Chapter 6.5.5 Battery
> + Capabilities, Section 6.5.5.1 Vendor ID (VID).
> +USBIF_PRODUCT_ID
> + Product ID (PID) assigned to the battery, such that if the VID belongs to the
> + manufacturer then the PID will be designated by it. Similarly if the VID
> + belongs to the device vendor then the PID will be designated by it. This
> + property is described in "USB Power Delivery Specification Rev3.1 V1.8"
> + Chapter 6.5.5 Battery Capabilities, Section 6.5.5.2 Product ID (PID).
>
> Battery <-> external power supply interaction
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index edb058c19c9c44ad9ad97a626fc8f59e3d3735a6..534ed3cd049866fa747455bb6dae1ec2dc5e2da6 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -211,6 +211,8 @@ static struct power_supply_attr power_supply_attrs[] __ro_after_init = {
> POWER_SUPPLY_ATTR(TIME_TO_EMPTY_AVG),
> POWER_SUPPLY_ATTR(TIME_TO_FULL_NOW),
> POWER_SUPPLY_ATTR(TIME_TO_FULL_AVG),
> + POWER_SUPPLY_ATTR(USBIF_VENDOR_ID),
> + POWER_SUPPLY_ATTR(USBIF_PRODUCT_ID),
> POWER_SUPPLY_ENUM_ATTR(TYPE),
> POWER_SUPPLY_ENUM_ATTR(USB_TYPE),
> POWER_SUPPLY_ENUM_ATTR(SCOPE),
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index a35b08bd368e9305554e1a608dc8e526983cfa12..100eb559dcede938595ffbf83bc5ef3645a5a172 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -165,6 +165,8 @@ enum power_supply_property {
> POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
> POWER_SUPPLY_PROP_TIME_TO_FULL_NOW,
> POWER_SUPPLY_PROP_TIME_TO_FULL_AVG,
> + POWER_SUPPLY_PROP_USBIF_VENDOR_ID,
> + POWER_SUPPLY_PROP_USBIF_PRODUCT_ID,
> POWER_SUPPLY_PROP_TYPE, /* use power_supply.type instead */
> POWER_SUPPLY_PROP_USB_TYPE,
> POWER_SUPPLY_PROP_SCOPE,
Neither this series, nor the Pixel 6 one seems to have any user for
these new properties? This becomes part of the kernel ABI, so we do
not add new properties without a user.
Greetings,
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 5/5] usb: typec: tcpm: Add support for Battery Cap response message
2025-05-08 1:00 ` [PATCH v2 5/5] usb: typec: tcpm: Add support for Battery Cap response message Amit Sunil Dhamne via B4 Relay
@ 2025-06-23 21:51 ` Sebastian Reichel
0 siblings, 0 replies; 20+ messages in thread
From: Sebastian Reichel @ 2025-06-23 21:51 UTC (permalink / raw)
To: amitsd
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Heikki Krogerus,
Rafael J. Wysocki, Len Brown, Pavel Machek, Kyle Tso, devicetree,
linux-kernel, linux-usb, linux-pm
[-- Attachment #1: Type: text/plain, Size: 7427 bytes --]
Hi,
On Wed, May 07, 2025 at 06:00:26PM -0700, Amit Sunil Dhamne via B4 Relay wrote:
> From: Amit Sunil Dhamne <amitsd@google.com>
>
> Add support for responding to Get_Battery_Cap (extended) request with a
> a Battery_Capabilities (extended) msg. The requester will request
> Battery Cap for a specific battery using an index in Get_Battery_Cap. In
> case of failure to identify battery, TCPM shall reply with an
> appropriate message indicating so.
>
> Support has been added only for fixed batteries and not hot swappable
> ones.
>
> As the Battery Cap Data Block size is 9 Bytes (lesser than
> MaxExtendedMsgChunkLen of 26B), only a single chunk is required to
> complete the AMS.
>
> Support for Battery_Capabilities 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>
> Reviewed-by: Kyle Tso <kyletso@google.com>
> ---
> drivers/usb/typec/tcpm/tcpm.c | 96 +++++++++++++++++++++++++++++++++++++++++--
> include/linux/usb/pd.h | 31 ++++++++++++++
> 2 files changed, 123 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 98df0c7ce00e43f6c95ab49237a414e1b4413369..4731126fbf19a50178be0cf8867b3fe08595724c 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -228,7 +228,8 @@ enum pd_msg_request {
> PD_MSG_DATA_SINK_CAP,
> PD_MSG_DATA_SOURCE_CAP,
> PD_MSG_DATA_REV,
> - PD_MSG_DATA_BATT_STATUS
> + PD_MSG_DATA_BATT_STATUS,
> + PD_MSG_EXT_BATT_CAP,
> };
>
> enum adev_actions {
> @@ -597,8 +598,8 @@ struct tcpm_port {
> u8 fixed_batt_cnt;
>
> /*
> - * Variable used to store battery_ref from the Get_Battery_Status
> - * request to process Battery_Status messages.
> + * Variable used to store battery_ref from the Get_Battery_Status &
> + * Get_Battery_Caps request to process Battery_Status messages.
> */
> u8 batt_request;
> #ifdef CONFIG_DEBUG_FS
> @@ -1414,6 +1415,81 @@ static int tcpm_pd_send_batt_status(struct tcpm_port *port)
> return tcpm_pd_transmit(port, TCPC_TX_SOP, &msg);
> }
>
> +static int tcpm_pd_send_batt_cap(struct tcpm_port *port)
> +{
> + struct pd_message msg;
> + struct power_supply *batt;
> + struct batt_cap_ext_msg bcdb;
> + u32 batt_id = port->batt_request;
> + int ret;
> + union power_supply_propval val;
> + bool batt_present = false;
> + u16 batt_design_cap = BATTERY_PROPERTY_UNKNOWN;
> + u16 batt_charge_cap = BATTERY_PROPERTY_UNKNOWN;
> + u8 data_obj_cnt;
> + /*
> + * As per USB PD Rev3.1 v1.8, if battery reference is incorrect,
> + * then set the VID field to 0xffff.
> + * If VID field is set to 0xffff, always set the PID field to
> + * 0x0000.
> + */
> + u16 vid = BATTERY_PROPERTY_UNKNOWN;
> + u16 pid = 0x0;
> +
> + memset(&msg, 0, sizeof(msg));
> +
> + if (batt_id < MAX_NUM_FIXED_BATT && port->fixed_batt[batt_id]) {
> + batt_present = true;
This should also handle POWER_SUPPLY_PROP_PRESENT.
Greetings,
-- Sebastian
> + batt = port->fixed_batt[batt_id];
> + ret = power_supply_get_property(batt,
> + POWER_SUPPLY_PROP_USBIF_VENDOR_ID,
> + &val);
> + if (!ret)
> + vid = val.intval;
> +
> + if (vid != BATTERY_PROPERTY_UNKNOWN) {
> + ret = power_supply_get_property(batt,
> + POWER_SUPPLY_PROP_USBIF_PRODUCT_ID,
> + &val);
> + if (!ret)
> + pid = val.intval;
> + }
> +
> + ret = power_supply_get_property(batt,
> + POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
> + &val);
> + if (!ret)
> + batt_design_cap = (u16)UWH_TO_WH(val.intval * 10);
> +
> + ret = power_supply_get_property(batt,
> + POWER_SUPPLY_PROP_ENERGY_FULL,
> + &val);
> + if (!ret)
> + batt_charge_cap = (u16)UWH_TO_WH(val.intval * 10);
> + }
> +
> + bcdb.vid = cpu_to_le16(vid);
> + bcdb.pid = cpu_to_le16(pid);
> + bcdb.batt_design_cap = cpu_to_le16(batt_design_cap);
> + bcdb.batt_last_chg_cap = cpu_to_le16(batt_charge_cap);
> + bcdb.batt_type = !batt_present ? BATT_CAP_BATT_TYPE_INVALID_REF : 0;
> + memcpy(msg.ext_msg.data, &bcdb, sizeof(bcdb));
> + msg.ext_msg.header = PD_EXT_HDR_LE(sizeof(bcdb),
> + 0, /* Denotes if request chunk */
> + 0, /* Chunk number */
> + 1 /* Chunked */);
> +
> + data_obj_cnt = count_chunked_data_objs(sizeof(bcdb));
> + msg.header = cpu_to_le16(PD_HEADER(PD_EXT_BATT_CAP,
> + port->pwr_role,
> + port->data_role,
> + port->negotiated_rev,
> + port->message_id,
> + data_obj_cnt,
> + 1 /* Denotes if ext header */));
> + return tcpm_pd_transmit(port, TCPC_TX_SOP, &msg);
> +}
> +
> static void mod_tcpm_delayed_work(struct tcpm_port *port, unsigned int delay_ms)
> {
> if (delay_ms) {
> @@ -3711,8 +3787,12 @@ static void tcpm_pd_ext_msg_request(struct tcpm_port *port,
> tcpm_pd_handle_msg(port, PD_MSG_DATA_BATT_STATUS,
> GETTING_BATTERY_STATUS);
> break;
> - case PD_EXT_SOURCE_CAP_EXT:
> case PD_EXT_GET_BATT_CAP:
> + port->batt_request = ext_msg->data[0];
> + tcpm_pd_handle_msg(port, PD_MSG_EXT_BATT_CAP,
> + GETTING_BATTERY_CAPABILITIES);
> + break;
> + case PD_EXT_SOURCE_CAP_EXT:
> case PD_EXT_BATT_CAP:
> case PD_EXT_GET_MANUFACTURER_INFO:
> case PD_EXT_MANUFACTURER_INFO:
> @@ -3921,6 +4001,14 @@ static bool tcpm_send_queued_message(struct tcpm_port *port)
> ret);
> tcpm_ams_finish(port);
> break;
> + case PD_MSG_EXT_BATT_CAP:
> + ret = tcpm_pd_send_batt_cap(port);
> + if (ret)
> + tcpm_log(port,
> + "Failed to send battery cap ret=%d",
> + ret);
> + tcpm_ams_finish(port);
> + break;
> default:
> break;
> }
> diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> index 4efa7bfd9863915dfc8048da264116d9b05a447b..c89594177da57f4398b75c89c1991b4937614a70 100644
> --- a/include/linux/usb/pd.h
> +++ b/include/linux/usb/pd.h
> @@ -204,6 +204,37 @@ struct pd_message {
> };
> } __packed;
>
> +/*
> + * count_chunked_data_objs: Helper to calculate number of Data Objects on a 4
> + * byte boundary.
> + * @size: Size of data block for extended message. Should *not* include extended
> + * header size.
> + */
> +static inline u8 count_chunked_data_objs(u32 size)
> +{
> + size += offsetof(struct pd_chunked_ext_message_data, data);
> + return ((size / 4) + (size % 4 ? 1 : 0));
> +}
> +
> +/**
> + * batt_cap_ext_msg - Battery capability extended PD message
> + * @vid: Battery Vendor ID (assigned by USB-IF)
> + * @pid: Battery Product ID (assigned by battery or device vendor)
> + * @batt_design_cap: Battery design capacity in 0.1Wh
> + * @batt_last_chg_cap: Battery last full charge capacity in 0.1Wh
> + * @batt_type: Battery Type. bit0 when set indicates invalid battery reference.
> + * Rest of the bits are reserved.
> + */
> +struct batt_cap_ext_msg {
> + __le16 vid;
> + __le16 pid;
> + __le16 batt_design_cap;
> + __le16 batt_last_chg_cap;
> + u8 batt_type;
> +} __packed;
> +
> +#define BATT_CAP_BATT_TYPE_INVALID_REF BIT(0)
> +
> /* PDO: Power Data Object */
> #define PDO_MAX_OBJECTS 7
>
>
> --
> 2.49.0.987.g0cc8ee98dc-goog
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/5] dt-bindings: connector: extend ports property to model power connections
2025-05-20 20:10 ` Amit Sunil Dhamne
2025-05-28 18:42 ` Amit Sunil Dhamne
@ 2025-06-23 22:08 ` Sebastian Reichel
2025-07-08 20:55 ` Amit Sunil Dhamne
1 sibling, 1 reply; 20+ messages in thread
From: Sebastian Reichel @ 2025-06-23 22:08 UTC (permalink / raw)
To: Amit Sunil Dhamne
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Heikki Krogerus,
Rafael J. Wysocki, Len Brown, Pavel Machek, Kyle Tso, devicetree,
linux-kernel, linux-usb, linux-pm
[-- Attachment #1: Type: text/plain, Size: 5425 bytes --]
Hi,
On Tue, May 20, 2025 at 01:10:25PM -0700, Amit Sunil Dhamne wrote:
> Hi Rob,
>
> Thanks for your response!
>
> On 5/14/25 12:42 PM, Rob Herring wrote:
> > On Wed, May 07, 2025 at 06:00:22PM -0700, Amit Sunil Dhamne wrote:
> >> Extend ports property to model power lines going between connector to
> >> charger or battery/batteries. As an example, connector VBUS can supply
> >> power in & out of the battery for a DRP.
> >>
> >> Additionally, add ports property to maxim,max33359 controller example.
> >>
> >> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> >> ---
> >> .../bindings/connector/usb-connector.yaml | 20 +++++++++++------
> >> .../devicetree/bindings/usb/maxim,max33359.yaml | 25 ++++++++++++++++++++++
> >> 2 files changed, 38 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> >> index 11e40d225b9f3a0d0aeea7bf764f1c00a719d615..706094f890026d324e6ece8b0c1e831d04d51eb7 100644
> >> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> >> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> >> @@ -181,16 +181,16 @@ properties:
> >>
> >> port:
> >> $ref: /schemas/graph.yaml#/properties/port
> >> - description: OF graph bindings modeling a data bus to the connector, e.g.
> >> - there is a single High Speed (HS) port present in this connector. If there
> >> - is more than one bus (several port, with 'reg' property), they can be grouped
> >> - under 'ports'.
> >> + description: OF graph binding to model a logical connection between a device
> >> + and connector. This connection may represent a data bus or power line. For
> >> + e.g. a High Speed (HS) data port present in this connector or VBUS line.
> >> + If there is more than one connection (several port, with 'reg' property),
> >> + they can be grouped under 'ports'.
> > 'port' and 'port@0' are equivalent. So you can't be changing its
> > definition.
>
> Noted!
>
>
> > I'm not sure showing a power connection with the graph is the right
> > approach.
>
> I want to provide some more context and rationale behind using this design.
>
> From a hardware perspective:
>
> The max77759/max33359 IC has Type-C port controller, charger, fuel gauge
> (FG) ICs. The Vbus from the connector goes to/from the TCPC and connects
> with the charger IP via circuitry & from there on to the battery. The FG
> is connected to the battery in parallel. As it can be seen that while
> these IPs are interconnected, there's no direct connection of the fuel
> gauge & the connector.
>
> For this feature, I am interested in getting the reference to the FG. As
> per graph description: "...These common bindings do not contain any
> information about the direction or type of the connections, they just
> map their existence." This works for my case because I just want the
> connector to be aware of the Fuel gauge device without imposing a
> specific directionality in terms of power supplier/supplied. This is
> also the reason why I didn't use
> "/schemas/power/supply/power-supply.yaml#power-supplies" binding.
>
> > We have a binding for that already with the regulator binding.
>
> I haven't explored the option of using regulator bindings. But in my
> case I am interested in fuel gauge and unfortunately, they're modeled as
> power_supply devices.
From hardware point of view there is no direct connection at all
between the fuel gauge and the connector. The usual hardware
connection is
connector -> charger -> battery
With the charger potentially supporting reverse operation to provide
energy from the battery to the connector (with "battery" I assume
a "smart" battery, so the raw cells and some kind of fuel gauge).
Thus the following example should properly document the hardware
connections:
---------------------------------------
typec-connector {
/* ... */
};
charger {
/* ... */
power-supplies = <&connector>;
};
fuel-gauge {
/* ... */
power-supplies = <&charger>;
};
---------------------------------------
It means instead of the direct graph lookup for the fuel gauge,
you would need a function walking through the graph build by the
power-supplies phandles. But it also means that the DT properly
describes the hardware instead of adding random graph connections.
Greetings,
-- Sebastian
> > Perhaps the connector needs to be a supply. It's already using that
> > binding in the supplying power to the connector case.
>
> Want to clarify, in this case you mean
> /schemas/regulator/regulator.yaml#*-supply$ right?
>
> Adding to my response above, the reason I don't want to impose a
> directionality in terms of supplier/supplied is that in case of USB Dual
> Role Port they're dynamic i.e., when USB is source, the power is
> supplied out of the battery (battery/FG will be supplier) and in case
> USB is sink, battery is supplied power. Whether the connector port is in
> source or sink role is determined on a connection to connection basis.
> Also, the knowledge of the supply direction is of no consequence for
> this feature.
>
>
> Please let me know what you think.
>
> Thanks,
>
> Amit
>
>
> > Rob
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/5] power: supply: core: add helper to get power supply given a fwnode
2025-06-23 21:21 ` Sebastian Reichel
@ 2025-07-08 0:53 ` Amit Sunil Dhamne
0 siblings, 0 replies; 20+ messages in thread
From: Amit Sunil Dhamne @ 2025-07-08 0:53 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Heikki Krogerus,
Rafael J. Wysocki, Len Brown, Pavel Machek, Kyle Tso, devicetree,
linux-kernel, linux-usb, linux-pm
Hi Sebastian,
On 6/23/25 2:21 PM, Sebastian Reichel wrote:
> Hi,
>
> On Wed, May 07, 2025 at 06:00:23PM -0700, Amit Sunil Dhamne via B4 Relay wrote:
>> From: Amit Sunil Dhamne <amitsd@google.com>
>>
>> Add a helper function power_supply_get_by_fwnode() to retrieve
>> power_supply given a valid fwnode reference.
>>
>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>> ---
>> drivers/power/supply/power_supply_core.c | 30 ++++++++++++++++++++++++++++++
>> include/linux/power_supply.h | 3 +++
>> 2 files changed, 33 insertions(+)
>>
>> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
>> index 76c340b38015af0a67a0d91305e6242a8646bf53..ef6ba22b837b0b9463f9a3065425e2720f40b9eb 100644
>> --- a/drivers/power/supply/power_supply_core.c
>> +++ b/drivers/power/supply/power_supply_core.c
>> @@ -496,6 +496,36 @@ struct power_supply *power_supply_get_by_name(const char *name)
>> }
>> EXPORT_SYMBOL_GPL(power_supply_get_by_name);
>>
>> +static int power_supply_match_device_by_fwnode(struct device *dev, const void *data)
>> +{
>> + return dev->parent && dev_fwnode(dev->parent) == data;
>> +}
> This already exists as power_supply_match_device_fwnode().
Thanks for your review! I will update in the next revision
>
>> +
>> +/**
>> + * power_supply_get_by_fwnode() - Search for power supply given a fwnode ref.
>> + * @fwnode: fwnode reference
>> + *
>> + * If power supply was found, it increases reference count for the internal
>> + * power supply's device. The user should power_supply_put() after use.
>> + *
>> + * Return: Reference to power_supply node matching the fwnode on success or
>> + * NULL on failure.
>> + */
>> +struct power_supply *power_supply_get_by_fwnode(struct fwnode_handle *fwnode)
>> +{
>> + struct power_supply *psy = NULL;
>> + struct device *dev = class_find_device(&power_supply_class, NULL, fwnode,
>> + power_supply_match_device_by_fwnode);
>> +
>> + if (dev) {
>> + psy = dev_get_drvdata(dev);
>> + atomic_inc(&psy->use_cnt);
>> + }
>> +
>> + return psy;
>> +}
>> +EXPORT_SYMBOL_GPL(power_supply_get_by_fwnode);
> And this is a 50% of power_supply_get_by_reference(), so the
> function should be updated to use power_supply_get_by_fwnode().
Agreed.
Thanks,
Amit
>
> Greetings,
>
> -- Sebastian
>
>> /**
>> * 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 6ed53b292162469d7b357734d5589bff18a201d0..a35b08bd368e9305554e1a608dc8e526983cfa12 100644
>> --- a/include/linux/power_supply.h
>> +++ b/include/linux/power_supply.h
>> @@ -801,10 +801,13 @@ 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 void power_supply_put(struct power_supply *psy);
>> +extern struct power_supply *power_supply_get_by_fwnode(struct fwnode_handle *fwnode);
>> #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 struct power_supply *power_supply_get_by_fwnode(struct fwnode_handle *fwnode)
>> +{ return NULL; }
>> #endif
>> #ifdef CONFIG_OF
>> extern struct power_supply *power_supply_get_by_phandle(struct device_node *np,
>>
>> --
>> 2.49.0.987.g0cc8ee98dc-goog
>>
>>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/5] usb: typec: tcpm: Add support for Battery Status response message
2025-06-23 21:27 ` Sebastian Reichel
@ 2025-07-08 0:55 ` Amit Sunil Dhamne
0 siblings, 0 replies; 20+ messages in thread
From: Amit Sunil Dhamne @ 2025-07-08 0:55 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Heikki Krogerus,
Rafael J. Wysocki, Len Brown, Pavel Machek, Kyle Tso, devicetree,
linux-kernel, linux-usb, linux-pm
On 6/23/25 2:27 PM, Sebastian Reichel wrote:
> Hi,
>
> On Wed, May 07, 2025 at 06:00:24PM -0700, Amit Sunil Dhamne via B4 Relay wrote:
>> From: Amit Sunil Dhamne <amitsd@google.com>
>>
>> Add support for responding to Get_Battery_Status (extended) request with
>> a Battery_Status (data) msg. The requester shall request the status of
>> an individual battery by providing an index in Get_Battery_Status. In
>> case of failure to identify battery, the responder shall reply with an
>> appropriate message indicating so.
>>
>> Battery status support is only provided for fixed batteries indexed from
>> 0 - 3.
>>
>> 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>
>> Reviewed-by: Kyle Tso <kyletso@google.com>
>> ---
> (partial review)
>
>> +static int tcpm_pd_send_batt_status(struct tcpm_port *port)
>> +{
>> + struct pd_message msg;
>> + struct power_supply *batt;
>> + u32 bsdo;
>> + u32 batt_id = port->batt_request;
>> + union power_supply_propval val;
>> + int ret;
>> + bool batt_present = false;
>> + u8 charging_status = BSDO_BATTERY_INFO_RSVD;
>> + u16 present_charge = BATTERY_PROPERTY_UNKNOWN;
>> +
>> + memset(&msg, 0, sizeof(msg));
>> + if (batt_id < MAX_NUM_FIXED_BATT && port->fixed_batt[batt_id]) {
>> + batt_present = true;
> power_supply_get_property(batt, POWER_SUPPLY_PROP_PRESENT, &batt_present);
Will update in the next revision.
Thanks,
Amit
>
>> ...
> Greetings,
>
> -- Sebastian
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 4/5] power: supply: core: add vendor and product id properties
2025-06-23 21:44 ` Sebastian Reichel
@ 2025-07-08 1:05 ` Amit Sunil Dhamne
0 siblings, 0 replies; 20+ messages in thread
From: Amit Sunil Dhamne @ 2025-07-08 1:05 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Heikki Krogerus,
Rafael J. Wysocki, Len Brown, Pavel Machek, Kyle Tso, devicetree,
linux-kernel, linux-usb, linux-pm
Hi Sebastian,
On 6/23/25 2:44 PM, Sebastian Reichel wrote:
> Hi,
>
> On Wed, May 07, 2025 at 06:00:25PM -0700, Amit Sunil Dhamne via B4 Relay wrote:
>> From: Amit Sunil Dhamne <amitsd@google.com>
>>
>> Add the following properties:
>> * Vendor Identifier (VID): Assigned to the battery manufacturer by USB
>> Implementers Forum (USB-IF).
>> * Product Identifier (PID) assigned by the manufacturer to the
>> battery.
>>
>> This info is required by USB Type-C PD devices containing batteries.
>> This enables the USB Type C devices to respond to a Battery capacity
>> request from the port partner by querying for the PID & VID assigned to
>> the batteries. Refer to "USB Power Delivery Specification Rev3.1 v1.8"
>> Chapter 5.5 Battery_Capabilities Message.
>>
>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>> ---
>> Documentation/ABI/testing/sysfs-class-power | 19 +++++++++++++++----
>> Documentation/power/power_supply_class.rst | 11 +++++++++++
>> drivers/power/supply/power_supply_sysfs.c | 2 ++
>> include/linux/power_supply.h | 2 ++
>> 4 files changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
>> index 2a5c1a09a28f91beec6b18ca7b4492093026bc81..5495e82885b2294cdfd5ace0e7e5fcbeadfccb5f 100644
>> --- a/Documentation/ABI/testing/sysfs-class-power
>> +++ b/Documentation/ABI/testing/sysfs-class-power
>> @@ -814,11 +814,22 @@ Description:
>> Access: Read
>> Valid values: 1-31
>>
>> -What: /sys/class/power_supply/<supply_name>/extensions/<extension_name>
>> -Date: March 2025
> Why does this remove existing Documentation?
Oops. I think that probably happened while resolving merge conflicts I
guess. Will fix.
>
>> +What: /sys/class/power_supply/<supply_name>/usbif_vendor_id
> I think we can use USB_VENDOR_ID and USB_PRODUCT_ID for this like
> everyone else?
Agreed.
>
>> +Date: May 2025
>> Contact: linux-pm@vger.kernel.org
>> Description:
>> - Reports the extensions registered to the power supply.
>> - Each entry is a link to the device which registered the extension.
>> + Reports the vendor id assigned to the battery manufacturer by USB
>> + Implementers Forum (USB-IF).
>>
>> Access: Read
>> + Valid values: 0x0-0xffff
> If I haven't missed something the formatting will be in decimal. I
> think the hex format is more sensible, so this needs some extra
> handling in power_supply_format_property() in power_supply_sysfs.c.
I see. I will add special handling for these in
power_supply_format_property().
>
>> +
>> +What: /sys/class/power_supply/<supply_name>/usbif_product_id
>> +Date: May 2025
>> +Contact: linux-pm@vger.kernel.org
>> +Description:
>> + Reports the product id assigned to the battery by the manufacturer
>> + (associated with usbif_vendor_id).
>> +
>> + Access: Read
>> + Valid values: 0x0-0xffff
>> diff --git a/Documentation/power/power_supply_class.rst b/Documentation/power/power_supply_class.rst
>> index da8e275a14ffb9f84bae9ae1efc4720a55ea3010..6d0a6bcf501e719fa4454845b583a8b38d371bb4 100644
>> --- a/Documentation/power/power_supply_class.rst
>> +++ b/Documentation/power/power_supply_class.rst
>> @@ -213,6 +213,17 @@ TIME_TO_FULL
>> seconds left for battery to be considered full
>> (i.e. while battery is charging)
>>
>> +USBIF_VENDOR_ID
>> + Vendor ID (VID) assigned to manufacturer or device vendor associated with the
>> + battery by USB Implementers Forum (USB-IF). This property is described in
>> + "USB Power Delivery Specification Rev3.1 V1.8" Chapter 6.5.5 Battery
>> + Capabilities, Section 6.5.5.1 Vendor ID (VID).
>> +USBIF_PRODUCT_ID
>> + Product ID (PID) assigned to the battery, such that if the VID belongs to the
>> + manufacturer then the PID will be designated by it. Similarly if the VID
>> + belongs to the device vendor then the PID will be designated by it. This
>> + property is described in "USB Power Delivery Specification Rev3.1 V1.8"
>> + Chapter 6.5.5 Battery Capabilities, Section 6.5.5.2 Product ID (PID).
>>
>> Battery <-> external power supply interaction
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
>> index edb058c19c9c44ad9ad97a626fc8f59e3d3735a6..534ed3cd049866fa747455bb6dae1ec2dc5e2da6 100644
>> --- a/drivers/power/supply/power_supply_sysfs.c
>> +++ b/drivers/power/supply/power_supply_sysfs.c
>> @@ -211,6 +211,8 @@ static struct power_supply_attr power_supply_attrs[] __ro_after_init = {
>> POWER_SUPPLY_ATTR(TIME_TO_EMPTY_AVG),
>> POWER_SUPPLY_ATTR(TIME_TO_FULL_NOW),
>> POWER_SUPPLY_ATTR(TIME_TO_FULL_AVG),
>> + POWER_SUPPLY_ATTR(USBIF_VENDOR_ID),
>> + POWER_SUPPLY_ATTR(USBIF_PRODUCT_ID),
>> POWER_SUPPLY_ENUM_ATTR(TYPE),
>> POWER_SUPPLY_ENUM_ATTR(USB_TYPE),
>> POWER_SUPPLY_ENUM_ATTR(SCOPE),
>> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
>> index a35b08bd368e9305554e1a608dc8e526983cfa12..100eb559dcede938595ffbf83bc5ef3645a5a172 100644
>> --- a/include/linux/power_supply.h
>> +++ b/include/linux/power_supply.h
>> @@ -165,6 +165,8 @@ enum power_supply_property {
>> POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
>> POWER_SUPPLY_PROP_TIME_TO_FULL_NOW,
>> POWER_SUPPLY_PROP_TIME_TO_FULL_AVG,
>> + POWER_SUPPLY_PROP_USBIF_VENDOR_ID,
>> + POWER_SUPPLY_PROP_USBIF_PRODUCT_ID,
>> POWER_SUPPLY_PROP_TYPE, /* use power_supply.type instead */
>> POWER_SUPPLY_PROP_USB_TYPE,
>> POWER_SUPPLY_PROP_SCOPE,
> Neither this series, nor the Pixel 6 one seems to have any user for
> these new properties? This becomes part of the kernel ABI, so we do
> not add new properties without a user.
I realize that there's no current driver that supplies these values atm.
I plan to add it for
https://lore.kernel.org/all/20250523-b4-gs101_max77759_fg-v4-0-b49904e35a34@uclouvain.be/
driver once it gets accepted.
Thanks,
Amit
>
> Greetings,
>
> -- Sebastian
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/5] dt-bindings: connector: extend ports property to model power connections
2025-06-23 22:08 ` Sebastian Reichel
@ 2025-07-08 20:55 ` Amit Sunil Dhamne
0 siblings, 0 replies; 20+ messages in thread
From: Amit Sunil Dhamne @ 2025-07-08 20:55 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Greg Kroah-Hartman, Badhri Jagan Sridharan, Heikki Krogerus,
Rafael J. Wysocki, Len Brown, Pavel Machek, Kyle Tso, devicetree,
linux-kernel, linux-usb, linux-pm
Hi Sebastian,
On 6/23/25 3:08 PM, Sebastian Reichel wrote:
> Hi,
>
> On Tue, May 20, 2025 at 01:10:25PM -0700, Amit Sunil Dhamne wrote:
>> Hi Rob,
>>
>> Thanks for your response!
>>
>> On 5/14/25 12:42 PM, Rob Herring wrote:
>>> On Wed, May 07, 2025 at 06:00:22PM -0700, Amit Sunil Dhamne wrote:
>>>> Extend ports property to model power lines going between connector to
>>>> charger or battery/batteries. As an example, connector VBUS can supply
>>>> power in & out of the battery for a DRP.
>>>>
>>>> Additionally, add ports property to maxim,max33359 controller example.
>>>>
>>>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>>>> ---
>>>> .../bindings/connector/usb-connector.yaml | 20 +++++++++++------
>>>> .../devicetree/bindings/usb/maxim,max33359.yaml | 25 ++++++++++++++++++++++
>>>> 2 files changed, 38 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>>> index 11e40d225b9f3a0d0aeea7bf764f1c00a719d615..706094f890026d324e6ece8b0c1e831d04d51eb7 100644
>>>> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>>>> @@ -181,16 +181,16 @@ properties:
>>>>
>>>> port:
>>>> $ref: /schemas/graph.yaml#/properties/port
>>>> - description: OF graph bindings modeling a data bus to the connector, e.g.
>>>> - there is a single High Speed (HS) port present in this connector. If there
>>>> - is more than one bus (several port, with 'reg' property), they can be grouped
>>>> - under 'ports'.
>>>> + description: OF graph binding to model a logical connection between a device
>>>> + and connector. This connection may represent a data bus or power line. For
>>>> + e.g. a High Speed (HS) data port present in this connector or VBUS line.
>>>> + If there is more than one connection (several port, with 'reg' property),
>>>> + they can be grouped under 'ports'.
>>> 'port' and 'port@0' are equivalent. So you can't be changing its
>>> definition.
>> Noted!
>>
>>
>>> I'm not sure showing a power connection with the graph is the right
>>> approach.
>> I want to provide some more context and rationale behind using this design.
>>
>> From a hardware perspective:
>>
>> The max77759/max33359 IC has Type-C port controller, charger, fuel gauge
>> (FG) ICs. The Vbus from the connector goes to/from the TCPC and connects
>> with the charger IP via circuitry & from there on to the battery. The FG
>> is connected to the battery in parallel. As it can be seen that while
>> these IPs are interconnected, there's no direct connection of the fuel
>> gauge & the connector.
>>
>> For this feature, I am interested in getting the reference to the FG. As
>> per graph description: "...These common bindings do not contain any
>> information about the direction or type of the connections, they just
>> map their existence." This works for my case because I just want the
>> connector to be aware of the Fuel gauge device without imposing a
>> specific directionality in terms of power supplier/supplied. This is
>> also the reason why I didn't use
>> "/schemas/power/supply/power-supply.yaml#power-supplies" binding.
>>
>>> We have a binding for that already with the regulator binding.
>> I haven't explored the option of using regulator bindings. But in my
>> case I am interested in fuel gauge and unfortunately, they're modeled as
>> power_supply devices.
> From hardware point of view there is no direct connection at all
> between the fuel gauge and the connector. The usual hardware
> connection is
>
> connector -> charger -> battery
>
> With the charger potentially supporting reverse operation to provide
> energy from the battery to the connector (with "battery" I assume
> a "smart" battery, so the raw cells and some kind of fuel gauge).
>
> Thus the following example should properly document the hardware
> connections:
>
> ---------------------------------------
> typec-connector {
> /* ... */
> };
>
> charger {
> /* ... */
> power-supplies = <&connector>;
> };
>
> fuel-gauge {
> /* ... */
> power-supplies = <&charger>;
> };
> ---------------------------------------
The hardware description is unambiguous for single power role Type-C
devices such as Sink only & Source only device (demonstrated by
inverting the relationship given in the above example).
For DRP power role, the above relationship feels semantically incorrect
because the illustrated relationship would not hold if the Type-C device
is Source for a given Type-C connection lifecycle.
I'll add a note somewhere mentioning that for DRP, the relationship can
be demonstrated either like a sink or source to make it less ambiguous.
> It means instead of the direct graph lookup for the fuel gauge,
> you would need a function walking through the graph build by the
> power-supplies phandles. But it also means that the DT properly
> describes the hardware instead of adding random graph connections.
Okay, will follow this approach.
Thanks,
Amit
>
> Greetings,
>
> -- Sebastian
>
>>> Perhaps the connector needs to be a supply. It's already using that
>>> binding in the supplying power to the connector case.
>> Want to clarify, in this case you mean
>> /schemas/regulator/regulator.yaml#*-supply$ right?
>>
>> Adding to my response above, the reason I don't want to impose a
>> directionality in terms of supplier/supplied is that in case of USB Dual
>> Role Port they're dynamic i.e., when USB is source, the power is
>> supplied out of the battery (battery/FG will be supplier) and in case
>> USB is sink, battery is supplied power. Whether the connector port is in
>> source or sink role is determined on a connection to connection basis.
>> Also, the knowledge of the supply direction is of no consequence for
>> this feature.
>>
>>
>> Please let me know what you think.
>>
>> Thanks,
>>
>> Amit
>>
>>
>>> Rob
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2025-07-08 20:55 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-08 1:00 [PATCH v2 0/5] Add support for Battery Status & Battery Caps AMS in TCPM Amit Sunil Dhamne via B4 Relay
2025-05-08 1:00 ` [PATCH v2 1/5] dt-bindings: connector: extend ports property to model power connections Amit Sunil Dhamne via B4 Relay
2025-05-08 2:08 ` Rob Herring (Arm)
2025-05-13 5:10 ` Amit Sunil Dhamne
2025-05-14 19:42 ` Rob Herring
2025-05-20 20:10 ` Amit Sunil Dhamne
2025-05-28 18:42 ` Amit Sunil Dhamne
2025-06-23 22:08 ` Sebastian Reichel
2025-07-08 20:55 ` Amit Sunil Dhamne
2025-05-08 1:00 ` [PATCH v2 2/5] power: supply: core: add helper to get power supply given a fwnode Amit Sunil Dhamne via B4 Relay
2025-06-23 21:21 ` Sebastian Reichel
2025-07-08 0:53 ` Amit Sunil Dhamne
2025-05-08 1:00 ` [PATCH v2 3/5] usb: typec: tcpm: Add support for Battery Status response message Amit Sunil Dhamne via B4 Relay
2025-06-23 21:27 ` Sebastian Reichel
2025-07-08 0:55 ` Amit Sunil Dhamne
2025-05-08 1:00 ` [PATCH v2 4/5] power: supply: core: add vendor and product id properties Amit Sunil Dhamne via B4 Relay
2025-06-23 21:44 ` Sebastian Reichel
2025-07-08 1:05 ` Amit Sunil Dhamne
2025-05-08 1:00 ` [PATCH v2 5/5] usb: typec: tcpm: Add support for Battery Cap response message Amit Sunil Dhamne via B4 Relay
2025-06-23 21:51 ` Sebastian Reichel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).