Skip to contentSkip to conversation

dt-bindings: connector: Add self-powered property

PATCH In review Badhri Jagan Sridharan 3 participants 10 messages Oct 1, 2018 – Nov 6, 2018

Subsystems (4)
lore
Share & tools
lore

Badhri Jagan Sridharan

author maintainer
#0 lore raw

PATCH v3 1/3 dt-bindings: connector: Add self-powered property

Quoted 1 line

From USB_PD_R3_0

7.1.5 Response to Hard Resets
Device operation during and after a Hard Reset is defined as follows:
Self-powered devices Should Not disconnect from USB during a Hard Reset (see Section 9.1.2).
Bus powered devices will disconnect from USB during a Hard Reset due to the loss of their power source.

Therefore it is necessary to know whether the port belongs to a device which is self powered or bus powered. This change adds "self-powered" flag to the connector class which present indicates that the port belongs to a device that is self powered. Else it is bus powered usb device.

-------
Changes is v3:
- Rebase on top of usb-next
- no change w.r.t to this patch. Same as previous versions.

No v2 version as the patch was introduced there.

---
 Documentation/devicetree/bindings/connector/usb-connector.txt | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/connector/usb-connector.txt +2 -0
index d90e17e2428b..a9a2f2fc44f2 100644
@@ -14,6 +14,8 @@ Optional properties:
 - label: symbolic name for the connector,
 - type: size of the connector, should be specified in case of USB-A, USB-B
   non-fullsize connectors: "mini", "micro".
+- self-powered: Set this property if the usb device that has its own power
+  source.
 
 Optional properties for usb-c-connector:
 - power-role: should be one of "source", "sink" or "dual"(DRP) if typec
Signature · 1 line
2.19.0.605.g01d371f741-goog
author maintainer
#1 lore raw

PATCH v3 2/3 usb: typec: tcpm: Do not disconnect link for self powered devices

During HARD_RESET the data link is disconnected.
For self powered device, the spec is advising against doing that.

Quoted 1 line

From USB_PD_R3_0

7.1.5 Response to Hard Resets
Device operation during and after a Hard Reset is defined as follows:
Self-powered devices Should Not disconnect from USB during a Hard Reset (see Section 9.1.2).
Bus powered devices will disconnect from USB during a Hard Reset due to the loss of their power source.

Tackle this by letting TCPM know whether the device is self or bus powered.

This overcomes unnecessary port disconnections from hard reset. Also, speeds up the enumeration time when connected to Type-A ports.

---------
Version history:
V3:
Rebase on top of usb-next

V2:
Based on feedback from [email protected]
- self_powered added to the struct tcpm_port which is populated from a. "connector" node of the device tree in tcpm_fw_get_caps() b. "self_powered" node of the tcpc_config in tcpm_copy_caps

Based on feedbase from [email protected]
- Code was refactored
- SRC_HARD_RESET_VBUS_OFF sets the link state to false based on self_powered flag

V1 located here:
https://lkml.org/lkml/2018/9/13/94

---
 drivers/usb/typec/tcpm/tcpm.c | 12 ++++++++++--
 include/linux/usb/tcpm.h      |  1 +
 2 files changed, 11 insertions(+), 2 deletions(-)
drivers/usb/typec/tcpm/tcpm.c +10 -2
index 4f1f4215f3d6..c3ac0e46106b 100644
@@ -317,6 +317,9 @@ struct tcpm_port {
 	/* Deadline in jiffies to exit src_try_wait state */
 	unsigned long max_wait;
 
+	/* port belongs to a self powered device */
+	bool self_powered;
+
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *dentry;
 	struct mutex logbuffer_lock;	/* log buffer access lock */
@@ -3257,7 +3260,8 @@ static void run_state_machine(struct tcpm_port *port)
 	case SRC_HARD_RESET_VBUS_OFF:
 		tcpm_set_vconn(port, true);
 		tcpm_set_vbus(port, false);
-		tcpm_set_roles(port, false, TYPEC_SOURCE, TYPEC_HOST);
+		tcpm_set_roles(port, port->self_powered, TYPEC_SOURCE,
+			       TYPEC_HOST);
 		tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
 		break;
 	case SRC_HARD_RESET_VBUS_ON:
@@ -3270,7 +3274,8 @@ static void run_state_machine(struct tcpm_port *port)
 		memset(&port->pps_data, 0, sizeof(port->pps_data));
 		tcpm_set_vconn(port, false);
 		tcpm_set_charge(port, false);
-		tcpm_set_roles(port, false, TYPEC_SINK, TYPEC_DEVICE);
+		tcpm_set_roles(port, port->self_powered, TYPEC_SINK,
+			       TYPEC_DEVICE);
 		/*
 		 * VBUS may or may not toggle, depending on the adapter.
 		 * If it doesn't toggle, transition to SNK_HARD_RESET_SINK_ON
@@ -4409,6 +4414,8 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
 		return -EINVAL;
 	port->operating_snk_mw = mw / 1000;
 
+	port->self_powered = fwnode_property_read_bool(fwnode, "self-powered");
+
 	return 0;
 }
 
@@ -4717,6 +4724,7 @@ static int tcpm_copy_caps(struct tcpm_port *port,
 	port->typec_caps.prefer_role = tcfg->default_role;
 	port->typec_caps.type = tcfg->type;
 	port->typec_caps.data = tcfg->data;
+	port->self_powered = port->tcpc->config->self_powered;
 
 	return 0;
 }
include/linux/usb/tcpm.h +1 -0
index 7e7fbfb84e8e..50c74a77db55 100644
@@ -89,6 +89,7 @@ struct tcpc_config {
 	enum typec_port_data data;
 	enum typec_role default_role;
 	bool try_role_hw;	/* try.{src,snk} implemented in hardware */
+	bool self_powered;	/* port belongs to a self powered device */
 
 	const struct typec_altmode_desc *alt_modes;
 };
Signature · 1 line
2.19.0.605.g01d371f741-goog
maintainer Reviewed-by on v3 2/3
#2 lore raw

Re: [PATCH v3 2/3] usb: typec: tcpm: Do not disconnect link for self powered devices

Quoted 11 lines · Badhri Jagan Sridharan

During HARD_RESET the data link is disconnected.
For self powered device, the spec is advising against doing that.

Quoted 1 line

From USB_PD_R3_0

7.1.5 Response to Hard Resets
Device operation during and after a Hard Reset is defined as follows:
Self-powered devices Should Not disconnect from USB during a Hard Reset (see Section 9.1.2).
Bus powered devices will disconnect from USB during a Hard Reset due to the loss of their power source.

Quoted 6 lines

Tackle this by letting TCPM know whether the device is self or bus powered.

This overcomes unnecessary port disconnections from hard reset. Also, speeds up the enumeration time when connected to Type-A ports.

Quoted 88 lines

---------
Version history:
V3:
Rebase on top of usb-next

V2:
Based on feedback from [email protected]
- self_powered added to the struct tcpm_port which is populated from a. "connector" node of the device tree in tcpm_fw_get_caps() b. "self_powered" node of the tcpc_config in tcpm_copy_caps

Based on feedbase from [email protected]
- Code was refactored
- SRC_HARD_RESET_VBUS_OFF sets the link state to false based on self_powered flag

V1 located here:
https://lkml.org/lkml/2018/9/13/94

---
 drivers/usb/typec/tcpm/tcpm.c | 12 ++++++++++--
 include/linux/usb/tcpm.h      |  1 +
 2 files changed, 11 insertions(+), 2 deletions(-)
drivers/usb/typec/tcpm/tcpm.c +10 -2
index 4f1f4215f3d6..c3ac0e46106b 100644
@@ -317,6 +317,9 @@ struct tcpm_port {
 	/* Deadline in jiffies to exit src_try_wait state */
 	unsigned long max_wait;
 
+	/* port belongs to a self powered device */
+	bool self_powered;
+
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *dentry;
 	struct mutex logbuffer_lock;	/* log buffer access lock */
@@ -3257,7 +3260,8 @@ static void run_state_machine(struct tcpm_port *port)
 	case SRC_HARD_RESET_VBUS_OFF:
 		tcpm_set_vconn(port, true);
 		tcpm_set_vbus(port, false);
-		tcpm_set_roles(port, false, TYPEC_SOURCE, TYPEC_HOST);
+		tcpm_set_roles(port, port->self_powered, TYPEC_SOURCE,
+			       TYPEC_HOST);
 		tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
 		break;
 	case SRC_HARD_RESET_VBUS_ON:
@@ -3270,7 +3274,8 @@ static void run_state_machine(struct tcpm_port *port)
 		memset(&port->pps_data, 0, sizeof(port->pps_data));
 		tcpm_set_vconn(port, false);
 		tcpm_set_charge(port, false);
-		tcpm_set_roles(port, false, TYPEC_SINK, TYPEC_DEVICE);
+		tcpm_set_roles(port, port->self_powered, TYPEC_SINK,
+			       TYPEC_DEVICE);
 		/*
 		 * VBUS may or may not toggle, depending on the adapter.
 		 * If it doesn't toggle, transition to SNK_HARD_RESET_SINK_ON
@@ -4409,6 +4414,8 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
 		return -EINVAL;
 	port->operating_snk_mw = mw / 1000;
 
+	port->self_powered = fwnode_property_read_bool(fwnode, "self-powered");
+
 	return 0;
 }
 
@@ -4717,6 +4724,7 @@ static int tcpm_copy_caps(struct tcpm_port *port,
 	port->typec_caps.prefer_role = tcfg->default_role;
 	port->typec_caps.type = tcfg->type;
 	port->typec_caps.data = tcfg->data;
+	port->self_powered = port->tcpc->config->self_powered;
 
 	return 0;
 }
include/linux/usb/tcpm.h +1 -0
index 7e7fbfb84e8e..50c74a77db55 100644
@@ -89,6 +89,7 @@ struct tcpc_config {
 	enum typec_port_data data;
 	enum typec_role default_role;
 	bool try_role_hw;	/* try.{src,snk} implemented in hardware */
+	bool self_powered;	/* port belongs to a self powered device */
 
 	const struct typec_altmode_desc *alt_modes;
 };
Signature · 1 line
2.19.0.605.g01d371f741-goog

thanks,

Signature · 1 line
heikki
author maintainer
#3 lore raw

PATCH v3 3/3 usb: typec: tcpm: charge current handling for sink during hard reset

In reply to Badhri Jagan Sridharan

During the initial connect to a non-pd port, sink would hard reset twice before deeming that the port partner is non-pd. TCPM sets the the charge path to false during the hard reset. This causes unnecessary connects/disconnects of charge path and makes port take longer to charge from the non-pd ports. Avoid this by not setting the charge path to false unless the partner has already identified to be pd capable.

When partner is a pd port, set the charge path to false in SNK_HARD_RESET_SINK_OFF. Set the current limits to default value based of CC pull up and resume the charge path when port enters SNK_HARD_RESET_SINK_ON.

--------
Changes in V3:
Rebase on top of usb-next

Changes in V2:
Based on feedback of [email protected]
- vsafe_5v_hard_reset flag from tcpc_config is removed
- Patch only differentiates between pd port partner and non-pd port partner

V1 version of the patch is here:
https://lkml.org/lkml/2018/9/14/11

---
 drivers/usb/typec/tcpm/tcpm.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/usb/typec/tcpm/tcpm.c +8 -1
index c3ac0e46106b..c25a69922ee6 100644
@@ -3273,7 +3273,8 @@ static void run_state_machine(struct tcpm_port *port)
 	case SNK_HARD_RESET_SINK_OFF:
 		memset(&port->pps_data, 0, sizeof(port->pps_data));
 		tcpm_set_vconn(port, false);
-		tcpm_set_charge(port, false);
+		if (port->pd_capable)
+			tcpm_set_charge(port, false);
 		tcpm_set_roles(port, port->self_powered, TYPEC_SINK,
 			       TYPEC_DEVICE);
 		/*
@@ -3305,6 +3306,12 @@ static void run_state_machine(struct tcpm_port *port)
 		 * Similar, dual-mode ports in source mode should transition
 		 * to PE_SNK_Transition_to_default.
 		 */
+		if (port->pd_capable) {
+			tcpm_set_current_limit(port,
+					       tcpm_get_current_limit(port),
+					       5000);
+			tcpm_set_charge(port, true);
+		}
 		tcpm_set_attached_state(port, true);
 		tcpm_set_state(port, SNK_STARTUP, 0);
 		break;
Signature · 1 line
2.19.0.605.g01d371f741-goog
maintainer Reviewed-by on v3 3/3
#4 lore raw

Re: [PATCH v3 3/3] usb: typec: tcpm: charge current handling for sink during hard reset

Quoted 24 lines · Badhri Jagan Sridharan

During the initial connect to a non-pd port, sink would hard reset twice before deeming that the port partner is non-pd. TCPM sets the the charge path to false during the hard reset. This causes unnecessary connects/disconnects of charge path and makes port take longer to charge from the non-pd ports. Avoid this by not setting the charge path to false unless the partner has already identified to be pd capable.

When partner is a pd port, set the charge path to false in SNK_HARD_RESET_SINK_OFF. Set the current limits to default value based of CC pull up and resume the charge path when port enters SNK_HARD_RESET_SINK_ON.

--------
Changes in V3:
Rebase on top of usb-next

Changes in V2:
Based on feedback of [email protected]
- vsafe_5v_hard_reset flag from tcpc_config is removed
- Patch only differentiates between pd port partner and non-pd port partner

Quoted 6 lines

V1 version of the patch is here:
https://lkml.org/lkml/2018/9/14/11

---
 drivers/usb/typec/tcpm/tcpm.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
maintainer Reviewed-by on v3 3/3
#5 lore raw

Re: [PATCH v3 3/3] usb: typec: tcpm: charge current handling for sink during hard reset

In reply to Badhri Jagan Sridharan
Quoted 7 lines · Badhri Jagan Sridharan

During the initial connect to a non-pd port, sink would hard reset twice before deeming that the port partner is non-pd. TCPM sets the the charge path to false during the hard reset. This causes unnecessary connects/disconnects of charge path and makes port take longer to charge from the non-pd ports. Avoid this by not setting the charge path to false unless the partner has already identified to be pd capable.

Quoted 6 lines

When partner is a pd port, set the charge path to false in SNK_HARD_RESET_SINK_OFF. Set the current limits to default value based of CC pull up and resume the charge path when port enters SNK_HARD_RESET_SINK_ON.

Quoted 45 lines

--------
Changes in V3:
Rebase on top of usb-next

Changes in V2:
Based on feedback of [email protected]
- vsafe_5v_hard_reset flag from tcpc_config is removed
- Patch only differentiates between pd port partner and non-pd port partner

V1 version of the patch is here:
https://lkml.org/lkml/2018/9/14/11

---
 drivers/usb/typec/tcpm/tcpm.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/usb/typec/tcpm/tcpm.c +8 -1
index c3ac0e46106b..c25a69922ee6 100644
@@ -3273,7 +3273,8 @@ static void run_state_machine(struct tcpm_port *port)
 	case SNK_HARD_RESET_SINK_OFF:
 		memset(&port->pps_data, 0, sizeof(port->pps_data));
 		tcpm_set_vconn(port, false);
-		tcpm_set_charge(port, false);
+		if (port->pd_capable)
+			tcpm_set_charge(port, false);
 		tcpm_set_roles(port, port->self_powered, TYPEC_SINK,
 			       TYPEC_DEVICE);
 		/*
@@ -3305,6 +3306,12 @@ static void run_state_machine(struct tcpm_port *port)
 		 * Similar, dual-mode ports in source mode should transition
 		 * to PE_SNK_Transition_to_default.
 		 */
+		if (port->pd_capable) {
+			tcpm_set_current_limit(port,
+					       tcpm_get_current_limit(port),
+					       5000);
+			tcpm_set_charge(port, true);
+		}
 		tcpm_set_attached_state(port, true);
 		tcpm_set_state(port, SNK_STARTUP, 0);
 		break;
Signature · 1 line
2.19.0.605.g01d371f741-goog

thanks,

Signature · 1 line
heikki
author maintainer
#6 lore raw

Re: [PATCH v3 3/3] usb: typec: tcpm: charge current handling for sink during hard reset

Hi Heikki, Rob,

Thanks for reviewing !
Should I update the patchset with the Reviewed-by tag and bumpup the patchset version ?
Or is that not necessary?

Thanks,
Badhri.

Quoted 68 lines · Heikki Krogerus
Quoted 7 lines · Badhri Jagan Sridharan

During the initial connect to a non-pd port, sink would hard reset twice before deeming that the port partner is non-pd. TCPM sets the the charge path to false during the hard reset. This causes unnecessary connects/disconnects of charge path and makes port take longer to charge from the non-pd ports. Avoid this by not setting the charge path to false unless the partner has already identified to be pd capable.

Quoted 6 lines

When partner is a pd port, set the charge path to false in SNK_HARD_RESET_SINK_OFF. Set the current limits to default value based of CC pull up and resume the charge path when port enters SNK_HARD_RESET_SINK_ON.

Quoted 44 lines

--------
Changes in V3:
Rebase on top of usb-next

Changes in V2:
Based on feedback of [email protected]
- vsafe_5v_hard_reset flag from tcpc_config is removed
- Patch only differentiates between pd port partner and non-pd port partner

V1 version of the patch is here:
https://lkml.org/lkml/2018/9/14/11

---
 drivers/usb/typec/tcpm/tcpm.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/usb/typec/tcpm/tcpm.c +8 -2
index c3ac0e46106b..c25a69922ee6 100644
@@ -3273,7 +3273,8 @@ static void run_state_machine(struct tcpm_port *port)
      case SNK_HARD_RESET_SINK_OFF:
              memset(&port->pps_data, 0, sizeof(port->pps_data));
              tcpm_set_vconn(port, false);
-             tcpm_set_charge(port, false);
+             if (port->pd_capable)
+                     tcpm_set_charge(port, false);
              tcpm_set_roles(port, port->self_powered, TYPEC_SINK,
                             TYPEC_DEVICE);
              /*
@@ -3305,6 +3306,12 @@ static void run_state_machine(struct tcpm_port *port)
               * Similar, dual-mode ports in source mode should transition
               * to PE_SNK_Transition_to_default.
               */
+             if (port->pd_capable) {
+                     tcpm_set_current_limit(port,
+                                            tcpm_get_current_limit(port),
+                                            5000);
+                     tcpm_set_charge(port, true);
+             }
              tcpm_set_attached_state(port, true);
              tcpm_set_state(port, SNK_STARTUP, 0);
              break;
--
Quoted 1 line

2.19.0.605.g01d371f741-goog

thanks,

--
heikki

maintainer
#7 lore raw

Re: [PATCH v3 3/3] usb: typec: tcpm: charge current handling for sink during hard reset

Quoted 6 lines · Badhri Jagan Sridharan

Hi Heikki, Rob,

Thanks for reviewing !
Should I update the patchset with the Reviewed-by tag and bumpup the patchset version ?
Or is that not necessary?

No it's not necessary. Greg will eventually pick these to his queue (assuming there are no other comments before that), and he'll take care of adding the tags.

thanks,

Signature · 1 line
heikki
maintainer Reviewed-by on v3 1/3
#8 lore raw
In reply to Badhri Jagan Sridharan
Quoted 8 lines · Badhri Jagan Sridharan
Quoted 1 line

From USB_PD_R3_0

7.1.5 Response to Hard Resets
Device operation during and after a Hard Reset is defined as follows:
Self-powered devices Should Not disconnect from USB during a Hard Reset (see Section 9.1.2).
Bus powered devices will disconnect from USB during a Hard Reset due to the loss of their power source.

Quoted 7 lines

Therefore it is necessary to know whether the port belongs to a device which is self powered or bus powered. This change adds "self-powered" flag to the connector class which present indicates that the port belongs to a device that is self powered. Else it is bus powered usb device.

Rob, I can see you acked one of the patches modifying tcpm.c in this series, but did you mean to put the tag to this patch instead?

FWIW:

Quoted 26 lines

-------
Changes is v3:
- Rebase on top of usb-next
- no change w.r.t to this patch. Same as previous versions.

No v2 version as the patch was introduced there.

---
 Documentation/devicetree/bindings/connector/usb-connector.txt | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/connector/usb-connector.txt +2 -0
index d90e17e2428b..a9a2f2fc44f2 100644
@@ -14,6 +14,8 @@ Optional properties:
 - label: symbolic name for the connector,
 - type: size of the connector, should be specified in case of USB-A, USB-B
   non-fullsize connectors: "mini", "micro".
+- self-powered: Set this property if the usb device that has its own power
+  source.
 
 Optional properties for usb-c-connector:
 - power-role: should be one of "source", "sink" or "dual"(DRP) if typec
Signature · 1 line
2.19.0.605.g01d371f741-goog

thanks,

Signature · 1 line
heikki
maintainer Reviewed-by
#9 lore raw
Quoted 16 lines · Heikki Krogerus
Quoted 14 lines · Badhri Jagan Sridharan
Quoted 1 line

From USB_PD_R3_0

7.1.5 Response to Hard Resets
Device operation during and after a Hard Reset is defined as follows:
Self-powered devices Should Not disconnect from USB during a Hard Reset (see Section 9.1.2).
Bus powered devices will disconnect from USB during a Hard Reset due to the loss of their power source.

Therefore it is necessary to know whether the port belongs to a device which is self powered or bus powered. This change adds "self-powered" flag to the connector class which present indicates that the port belongs to a device that is self powered. Else it is bus powered usb device.

Quoted 4 lines
Quoted 1 line

Rob, I can see you acked one of the patches modifying tcpm.c in this series, but did you mean to put the tag to this patch instead?

Hum, indeed. Looks like a scripting fail in my reply. The R-by was for this one.

Rob