Skip to contentSkip to conversation

usb: typec: tcpm: Introduce vsafe0v for vbus

PATCH Superseded Badhri Jagan Sridharan 2 participants 9 messages Dec 1, 2020 – Dec 2, 2020

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Badhri Jagan Sridharan

author maintainer
#0 lore raw

PATCH 1/3 usb: typec: tcpm: Introduce vsafe0v for vbus

TCPM at present lacks the notion of VSAFE0V. There are three vbus threshold levels that are critical to track:
a. vSafe5V - VBUS “5 volts” as defined by the USB
PD specification.
b. vSinkDisconnect - Threshold used for transition from
Attached.SNK to Unattached.SNK.
c. vSafe0V - VBUS “0 volts” as defined by the USB
PD specification.

Tracking vSafe0V is crucial for entry into Try.SNK and Attached.SRC and turning vbus back on by the source in response to hard reset.

From "4.5.2.2.8.2 Exiting from AttachWait.SRC State" section in the Type-C spec:

"The port shall transition to Attached.SRC when VBUS is at vSafe0V and the SRC.Rd state is detected on exactly one of the CC1 or CC2 pins for at least tCCDebounce."

"A DRP that strongly prefers the Sink role may optionally transition to Try.SNK instead of Attached.SRC when VBUS is at vSafe0V and the SRC.Rd state is detected on exactly one of the CC1 or CC2 pins for at least tCCDebounce."

From "7.1.5 Response to Hard Resets" section in the PD spec:

"After establishing the vSafe0V voltage condition on VBUS, the Source Shall wait tSrcRecover before re-applying VCONN and restoring VBUS to vSafe5V."

vbus_present in the TCPM code tracks vSafe5V(vbus_present is true) and vSinkDisconnect(vbus_present is false).

This change adds is_vbus_vsafe0v callback which when set makes TCPM query for vSafe0V voltage level when needed.

Since not all TCPC controllers might have the capability to report vSafe0V, TCPM assumes that vSafe0V is same as vSinkDisconnect when is_vbus_vsafe0v callback is not set. This allows TCPM to continue to support controllers which don't have the support for reporting vSafe0V.

Introducing vSafe0V helps fix the failure reported at "Step 15. CVS verifies PUT remains in AttachWait.SRC for 500ms" of "TD 4.7.2 Try. SNK DRP Connect DRP Test" of "Universal Serial Bus Type-C (USB Type-C) Functional Test Specification Chapters 4 and 5". Here the compliance tester intentionally maintains vbus at greater than vSafe0V and expects the Product under test to stay in AttachWait.SRC till vbus drops to vSafe0V.

---
 drivers/usb/typec/tcpm/tcpm.c | 63 +++++++++++++++++++++++++++++------
 include/linux/usb/tcpm.h      |  6 ++++
 2 files changed, 58 insertions(+), 11 deletions(-)
drivers/usb/typec/tcpm/tcpm.c +52 -11
index 3bbc1f10af49..10a065eef73e 100644
@@ -258,7 +258,19 @@ struct tcpm_port {
 	bool attached;
 	bool connected;
 	enum typec_port_type port_type;
+
+	/*
+	 * Set to true when vbus is greater than VSAFE5V min.
+	 * Set to false when vbus falls below vSinkDisconnect max threshold.
+	 */
 	bool vbus_present;
+
+	/*
+	 * Set to true when vbus is less than VSAFE0V max.
+	 * Set to false when vbus is greater than VSAFE0V max.
+	 */
+	bool vbus_vsafe0v;
+
 	bool vbus_never_low;
 	bool vbus_source;
 	bool vbus_charge;
@@ -3094,7 +3106,7 @@ static void run_state_machine(struct tcpm_port *port)
 		else if (tcpm_port_is_audio(port))
 			tcpm_set_state(port, AUDIO_ACC_ATTACHED,
 				       PD_T_CC_DEBOUNCE);
-		else if (tcpm_port_is_source(port))
+		else if (tcpm_port_is_source(port) && port->vbus_vsafe0v)
 			tcpm_set_state(port,
 				       tcpm_try_snk(port) ? SNK_TRY
 							  : SRC_ATTACHED,
@@ -4097,6 +4109,12 @@ static void _tcpm_pd_vbus_on(struct tcpm_port *port)
 {
 	tcpm_log_force(port, "VBUS on");
 	port->vbus_present = true;
+	/*
+	 * When vbus_present is true i.e. Voltage at VBUS is greater than VSAFE5V implicitly
+	 * states that vbus is not at VSAFE0V, hence clear the vbus_vsafe0v flag here.
+	 */
+	port->vbus_vsafe0v = false;
+
 	switch (port->state) {
 	case SNK_TRANSITION_SINK_VBUS:
 		port->explicit_contract = true;
@@ -4186,16 +4204,8 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)
 	case SNK_HARD_RESET_SINK_OFF:
 		tcpm_set_state(port, SNK_HARD_RESET_WAIT_VBUS, 0);
 		break;
-	case SRC_HARD_RESET_VBUS_OFF:
-		/*
-		 * After establishing the vSafe0V voltage condition on VBUS, the Source Shall wait
-		 * tSrcRecover before re-applying VCONN and restoring VBUS to vSafe5V.
-		 */
-		tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
-		break;
 	case HARD_RESET_SEND:
 		break;
-
 	case SNK_TRY:
 		/* Do nothing, waiting for timeout */
 		break;
@@ -4266,6 +4276,28 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)
 	}
 }
 
+static void _tcpm_pd_vbus_vsafe0v(struct tcpm_port *port)
+{
+	tcpm_log_force(port, "VBUS VSAFE0V");
+	port->vbus_vsafe0v = true;
+	switch (port->state) {
+	case SRC_HARD_RESET_VBUS_OFF:
+		/*
+		 * After establishing the vSafe0V voltage condition on VBUS, the Source Shall wait
+		 * tSrcRecover before re-applying VCONN and restoring VBUS to vSafe5V.
+		 */
+		tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
+		break;
+	case SRC_ATTACH_WAIT:
+		if (tcpm_port_is_source(port))
+			tcpm_set_state(port, tcpm_try_snk(port) ? SNK_TRY : SRC_ATTACHED,
+				       PD_T_CC_DEBOUNCE);
+		break;
+	default:
+		break;
+	}
+}
+
 static void _tcpm_pd_hard_reset(struct tcpm_port *port)
 {
 	tcpm_log_force(port, "Received hard reset");
@@ -4301,10 +4333,19 @@ static void tcpm_pd_event_handler(struct kthread_work *work)
 			bool vbus;
 
 			vbus = port->tcpc->get_vbus(port->tcpc);
-			if (vbus)
+			if (vbus) {
 				_tcpm_pd_vbus_on(port);
-			else
+			} else {
 				_tcpm_pd_vbus_off(port);
+				/*
+				 * When TCPC does not support detecting vsafe0v voltage level,
+				 * treat vbus absent as vsafe0v. Else invoke is_vbus_vsafe0v
+				 * to see if vbus has discharge to VSAFE0V.
+				 */
+				if (!port->tcpc->is_vbus_vsafe0v ||
+				    port->tcpc->is_vbus_vsafe0v(port->tcpc) > 0)
+					_tcpm_pd_vbus_vsafe0v(port);
+			}
 		}
 		if (events & TCPM_CC_EVENT) {
 			enum typec_cc_status cc1, cc2;
include/linux/usb/tcpm.h +6 -0
index e68aaa12886f..615d4532c028 100644
@@ -98,6 +98,11 @@ enum tcpm_transmit_type {
  *		will be turned on. requested_vbus_voltage is set to 0 when vbus
  *		is going to disappear knowingly i.e. during PR_SWAP and
  *		HARD_RESET etc.
+ * @is_vbus_vsafe0v:
+ *		Optional; TCPCI spec based TCPC implementations are expected to
+ *		detect VSAFE0V voltage level at vbus. When detection of VSAFE0V
+ *		is supported by TCPC, set this callback for TCPM to query
+ *		whether vbus is at VSAFE0V when needed.
  */
 struct tcpc_dev {
 	struct fwnode_handle *fwnode;
@@ -128,6 +133,7 @@ struct tcpc_dev {
 	int (*enable_auto_vbus_discharge)(struct tcpc_dev *dev, bool enable);
 	int (*set_auto_vbus_discharge_threshold)(struct tcpc_dev *dev, enum typec_pwr_opmode mode,
 						 bool pps_active, u32 requested_vbus_voltage);
+	int (*is_vbus_vsafe0v)(struct tcpc_dev *dev);
 };
 
 struct tcpm_port;
Signature · 1 line
2.29.2.454.gaff20da3a2-goog
author maintainer
#1 lore raw

PATCH 2/3 usb: typec: tcpci: Add support to report vSafe0V

This change adds vbus_vsafe0v which when set, makes TCPM query for VSAFE0V by assigning the tcpc.is_vbus_vsafe0v callback. Also enables ALERT.ExtendedStatus which is triggered when status of EXTENDED_STATUS.vSafe0V changes. EXTENDED_STATUS.vSafe0V is set when vbus is at vSafe0V and cleared otherwise.

---
 drivers/usb/typec/tcpm/tcpci.c | 55 ++++++++++++++++++++++++++--------
 drivers/usb/typec/tcpm/tcpci.h |  6 ++++
 2 files changed, 49 insertions(+), 12 deletions(-)
drivers/usb/typec/tcpm/tcpci.c +43 -12
index 12d983a75510..e281b8bee4db 100644
@@ -402,6 +402,19 @@ static int tcpci_get_vbus(struct tcpc_dev *tcpc)
 	return !!(reg & TCPC_POWER_STATUS_VBUS_PRES);
 }
 
+static int tcpci_is_vbus_vsafe0v(struct tcpc_dev *tcpc)
+{
+	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(tcpci->regmap, TCPC_EXTENDED_STATUS, &reg);
+	if (ret < 0)
+		return ret;
+
+	return !!(reg & TCPC_EXTENDED_STATUS_VSAFE0V);
+}
+
 static int tcpci_set_vbus(struct tcpc_dev *tcpc, bool source, bool sink)
 {
 	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
@@ -554,12 +567,22 @@ static int tcpci_init(struct tcpc_dev *tcpc)
 		TCPC_ALERT_RX_HARD_RST | TCPC_ALERT_CC_STATUS;
 	if (tcpci->controls_vbus)
 		reg |= TCPC_ALERT_POWER_STATUS;
+	/* Enable VSAFE0V status interrupt when detecting VSAFE0V is supported */
+	if (tcpci->data->vbus_vsafe0v) {
+		reg |= TCPC_ALERT_EXTENDED_STATUS;
+		ret = regmap_write(tcpci->regmap, TCPC_EXTENDED_STATUS_MASK,
+				   TCPC_EXTENDED_STATUS_VSAFE0V);
+		if (ret < 0)
+			return ret;
+	}
 	return tcpci_write16(tcpci, TCPC_ALERT_MASK, reg);
 }
 
 irqreturn_t tcpci_irq(struct tcpci *tcpci)
 {
 	u16 status;
+	int ret;
+	unsigned int raw;
 
 	tcpci_read16(tcpci, TCPC_ALERT, &status);
 
@@ -575,18 +598,17 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
 		tcpm_cc_change(tcpci->port);
 
 	if (status & TCPC_ALERT_POWER_STATUS) {
-		unsigned int reg;
-
-		regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, &reg);
-
-		/*
-		 * If power status mask has been reset, then the TCPC
-		 * has reset.
-		 */
-		if (reg == 0xff)
-			tcpm_tcpc_reset(tcpci->port);
-		else
-			tcpm_vbus_change(tcpci->port);
+		ret = regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, &raw);
+		if (ret >= 0) {
+			/*
+			 * If power status mask has been reset, then the TCPC
+			 * has reset.
+			 */
+			if (raw == 0xff)
+				tcpm_tcpc_reset(tcpci->port);
+			else
+				tcpm_vbus_change(tcpci->port);
+		}
 	}
 
 	if (status & TCPC_ALERT_RX_STATUS) {
@@ -622,6 +644,12 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
 		tcpm_pd_receive(tcpci->port, &msg);
 	}
 
+	if (status & TCPC_ALERT_EXTENDED_STATUS) {
+		ret = regmap_read(tcpci->regmap, TCPC_EXTENDED_STATUS, &raw);
+		if (ret >= 0 && (raw & TCPC_EXTENDED_STATUS_VSAFE0V))
+			tcpm_vbus_change(tcpci->port);
+	}
+
 	if (status & TCPC_ALERT_RX_HARD_RST)
 		tcpm_pd_hard_reset(tcpci->port);
 
@@ -699,6 +727,9 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
 			tcpci_set_auto_vbus_discharge_threshold;
 	}
 
+	if (tcpci->data->vbus_vsafe0v)
+		tcpci->tcpc.is_vbus_vsafe0v = tcpci_is_vbus_vsafe0v;
+
 	err = tcpci_parse_config(tcpci);
 	if (err < 0)
 		return ERR_PTR(err);
drivers/usb/typec/tcpm/tcpci.h +6 -0
index 3fe313655f0c..116a69c85e38 100644
@@ -49,6 +49,9 @@
 #define TCPC_TCPC_CTRL_ORIENTATION	BIT(0)
 #define TCPC_TCPC_CTRL_BIST_TM		BIT(1)
 
+#define TCPC_EXTENDED_STATUS		0x20
+#define TCPC_EXTENDED_STATUS_VSAFE0V	BIT(0)
+
 #define TCPC_ROLE_CTRL			0x1a
 #define TCPC_ROLE_CTRL_DRP		BIT(6)
 #define TCPC_ROLE_CTRL_RP_VAL_SHIFT	4
@@ -155,11 +158,14 @@ struct tcpci;
  *		is sourcing vbus.
  * @auto_discharge_disconnect:
  *		Optional; Enables TCPC to autonously discharge vbus on disconnect.
+ * @vbus_vsafe0v:
+ *		optional; Set when TCPC can detect whether vbus is at VSAFE0V.
  */
 struct tcpci_data {
 	struct regmap *regmap;
 	unsigned char TX_BUF_BYTE_x_hidden:1;
 	unsigned char auto_discharge_disconnect:1;
+	unsigned char vbus_vsafe0v:1;
 
 	int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
 	int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
Signature · 1 line
2.29.2.454.gaff20da3a2-goog
#2 lore raw

Re: [PATCH v1 2/3] usb: typec: tcpci: Add support to report vSafe0V

Quoted 77 lines · Badhri Jagan Sridharan

This change adds vbus_vsafe0v which when set, makes TCPM query for VSAFE0V by assigning the tcpc.is_vbus_vsafe0v callback. Also enables ALERT.ExtendedStatus which is triggered when status of EXTENDED_STATUS.vSafe0V changes. EXTENDED_STATUS.vSafe0V is set when vbus is at vSafe0V and cleared otherwise.

---
 drivers/usb/typec/tcpm/tcpci.c | 55 ++++++++++++++++++++++++++--------
 drivers/usb/typec/tcpm/tcpci.h |  6 ++++
 2 files changed, 49 insertions(+), 12 deletions(-)
drivers/usb/typec/tcpm/tcpci.c +24 -12
index 12d983a75510..e281b8bee4db 100644
@@ -402,6 +402,19 @@ static int tcpci_get_vbus(struct tcpc_dev *tcpc)
 	return !!(reg & TCPC_POWER_STATUS_VBUS_PRES);
 }
 
+static int tcpci_is_vbus_vsafe0v(struct tcpc_dev *tcpc)
+{
+	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(tcpci->regmap, TCPC_EXTENDED_STATUS, &reg);
+	if (ret < 0)
+		return ret;
+
+	return !!(reg & TCPC_EXTENDED_STATUS_VSAFE0V);
+}
+
 static int tcpci_set_vbus(struct tcpc_dev *tcpc, bool source, bool sink)
 {
 	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
@@ -554,12 +567,22 @@ static int tcpci_init(struct tcpc_dev *tcpc)
 		TCPC_ALERT_RX_HARD_RST | TCPC_ALERT_CC_STATUS;
 	if (tcpci->controls_vbus)
 		reg |= TCPC_ALERT_POWER_STATUS;
+	/* Enable VSAFE0V status interrupt when detecting VSAFE0V is supported */
+	if (tcpci->data->vbus_vsafe0v) {
+		reg |= TCPC_ALERT_EXTENDED_STATUS;
+		ret = regmap_write(tcpci->regmap, TCPC_EXTENDED_STATUS_MASK,
+				   TCPC_EXTENDED_STATUS_VSAFE0V);
+		if (ret < 0)
+			return ret;
+	}
 	return tcpci_write16(tcpci, TCPC_ALERT_MASK, reg);
 }
 
 irqreturn_t tcpci_irq(struct tcpci *tcpci)
 {
 	u16 status;
+	int ret;
+	unsigned int raw;
 
 	tcpci_read16(tcpci, TCPC_ALERT, &status);
 
@@ -575,18 +598,17 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
 		tcpm_cc_change(tcpci->port);
 
 	if (status & TCPC_ALERT_POWER_STATUS) {
-		unsigned int reg;
-
-		regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, &reg);
-
-		/*
-		 * If power status mask has been reset, then the TCPC
-		 * has reset.
-		 */
-		if (reg == 0xff)
-			tcpm_tcpc_reset(tcpci->port);
-		else
-			tcpm_vbus_change(tcpci->port);
+		ret = regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, &raw);

drivers/usb/typec/tcpm/tcpci.c irqreturn_t tcpci_irq(struct tcpci *tcpci) · line 611

Quoted 10 lines
drivers/usb/typec/tcpm/tcpci.c +10 -0
+		if (ret >= 0) {
+			/*
+			 * If power status mask has been reset, then the TCPC
+			 * has reset.
+			 */
+			if (raw == 0xff)
+				tcpm_tcpc_reset(tcpci->port);
+			else
+				tcpm_vbus_change(tcpci->port);
+		}

This change seems unrelated to this patch. Besides that, are you sure that ignoring an error from regmap_read() is sensible here ?

Overall, it may make sense to improve error handling in this driver, but I think it should be done in a separate patch.

Quoted 58 lines
drivers/usb/typec/tcpm/tcpci.c +9 -0
 	}
 
 	if (status & TCPC_ALERT_RX_STATUS) {
@@ -622,6 +644,12 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
 		tcpm_pd_receive(tcpci->port, &msg);
 	}
 
+	if (status & TCPC_ALERT_EXTENDED_STATUS) {
+		ret = regmap_read(tcpci->regmap, TCPC_EXTENDED_STATUS, &raw);
+		if (ret >= 0 && (raw & TCPC_EXTENDED_STATUS_VSAFE0V))
+			tcpm_vbus_change(tcpci->port);
+	}
+
 	if (status & TCPC_ALERT_RX_HARD_RST)
 		tcpm_pd_hard_reset(tcpci->port);
 
@@ -699,6 +727,9 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
 			tcpci_set_auto_vbus_discharge_threshold;
 	}
 
+	if (tcpci->data->vbus_vsafe0v)
+		tcpci->tcpc.is_vbus_vsafe0v = tcpci_is_vbus_vsafe0v;
+
 	err = tcpci_parse_config(tcpci);
 	if (err < 0)
 		return ERR_PTR(err);
drivers/usb/typec/tcpm/tcpci.h +6 -0
index 3fe313655f0c..116a69c85e38 100644
@@ -49,6 +49,9 @@
 #define TCPC_TCPC_CTRL_ORIENTATION	BIT(0)
 #define TCPC_TCPC_CTRL_BIST_TM		BIT(1)
 
+#define TCPC_EXTENDED_STATUS		0x20
+#define TCPC_EXTENDED_STATUS_VSAFE0V	BIT(0)
+
 #define TCPC_ROLE_CTRL			0x1a
 #define TCPC_ROLE_CTRL_DRP		BIT(6)
 #define TCPC_ROLE_CTRL_RP_VAL_SHIFT	4
@@ -155,11 +158,14 @@ struct tcpci;
  *		is sourcing vbus.
  * @auto_discharge_disconnect:
  *		Optional; Enables TCPC to autonously discharge vbus on disconnect.
+ * @vbus_vsafe0v:
+ *		optional; Set when TCPC can detect whether vbus is at VSAFE0V.
  */
 struct tcpci_data {
 	struct regmap *regmap;
 	unsigned char TX_BUF_BYTE_x_hidden:1;
 	unsigned char auto_discharge_disconnect:1;
+	unsigned char vbus_vsafe0v:1;
 
 	int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
 	int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
Signature · 1 line
2.29.2.454.gaff20da3a2-goog
author maintainer
#3 lore raw

Re: [PATCH v1 2/3] usb: typec: tcpci: Add support to report vSafe0V

Quoted 84 lines · Guenter Roeck
Quoted 82 lines · Badhri Jagan Sridharan

This change adds vbus_vsafe0v which when set, makes TCPM query for VSAFE0V by assigning the tcpc.is_vbus_vsafe0v callback. Also enables ALERT.ExtendedStatus which is triggered when status of EXTENDED_STATUS.vSafe0V changes. EXTENDED_STATUS.vSafe0V is set when vbus is at vSafe0V and cleared otherwise.

---
 drivers/usb/typec/tcpm/tcpci.c | 55 ++++++++++++++++++++++++++--------
 drivers/usb/typec/tcpm/tcpci.h |  6 ++++
 2 files changed, 49 insertions(+), 12 deletions(-)
drivers/usb/typec/tcpm/tcpci.c +29 -12
index 12d983a75510..e281b8bee4db 100644
@@ -402,6 +402,19 @@ static int tcpci_get_vbus(struct tcpc_dev *tcpc)
      return !!(reg & TCPC_POWER_STATUS_VBUS_PRES);
 }
 
+static int tcpci_is_vbus_vsafe0v(struct tcpc_dev *tcpc)
+{
+     struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
+     unsigned int reg;
+     int ret;
+
+     ret = regmap_read(tcpci->regmap, TCPC_EXTENDED_STATUS, &reg);
+     if (ret < 0)
+             return ret;
+
+     return !!(reg & TCPC_EXTENDED_STATUS_VSAFE0V);
+}
+
 static int tcpci_set_vbus(struct tcpc_dev *tcpc, bool source, bool sink)
 {
      struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
@@ -554,12 +567,22 @@ static int tcpci_init(struct tcpc_dev *tcpc)
              TCPC_ALERT_RX_HARD_RST | TCPC_ALERT_CC_STATUS;
      if (tcpci->controls_vbus)
              reg |= TCPC_ALERT_POWER_STATUS;
+     /* Enable VSAFE0V status interrupt when detecting VSAFE0V is supported */
+     if (tcpci->data->vbus_vsafe0v) {
+             reg |= TCPC_ALERT_EXTENDED_STATUS;
+             ret = regmap_write(tcpci->regmap, TCPC_EXTENDED_STATUS_MASK,
+                                TCPC_EXTENDED_STATUS_VSAFE0V);
+             if (ret < 0)
+                     return ret;
+     }
      return tcpci_write16(tcpci, TCPC_ALERT_MASK, reg);
 }
 
 irqreturn_t tcpci_irq(struct tcpci *tcpci)
 {
      u16 status;
+     int ret;
+     unsigned int raw;
 
      tcpci_read16(tcpci, TCPC_ALERT, &status);
 
@@ -575,18 +598,17 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
              tcpm_cc_change(tcpci->port);
 
      if (status & TCPC_ALERT_POWER_STATUS) {
-             unsigned int reg;
-
-             regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, &reg);
-
-             /*
-              * If power status mask has been reset, then the TCPC
-              * has reset.
-              */
-             if (reg == 0xff)
-                     tcpm_tcpc_reset(tcpci->port);
-             else
-                     tcpm_vbus_change(tcpci->port);
+             ret = regmap_read(tcpci->regmap, TCPC_POWER_STATUS_MASK, &raw);
+             if (ret >= 0) {
+                     /*
+                      * If power status mask has been reset, then the TCPC
+                      * has reset.
+                      */
Quoted 8 lines

drivers/usb/typec/tcpm/tcpci.c irqreturn_t tcpci_irq(struct tcpci *tcpci) · line 611

Quoted 5 lines
drivers/usb/typec/tcpm/tcpci.c +5 -0
+                     if (raw == 0xff)
+                             tcpm_tcpc_reset(tcpci->port);
+                     else
+                             tcpm_vbus_change(tcpci->port);
+             }

This change seems unrelated to this patch. Besides that, are you sure that ignoring an error from regmap_read() is sensible here ?

Sorry should have split that into a separate patch. I was actually intending to do the following where tcpm calls are not made if TCPC_POWER_STATUS_MASK read returns error. The code was previously ignoring the error.

               if (!ret) {
                        /*
                         * If power status mask has been reset, then the TCPC
                         * has reset.
                         */
                        if (raw == 0xff)
                                tcpm_tcpc_reset(tcpci->port);
                        else
                                tcpm_vbus_change(tcpci->port);
             }

This is reasonable right ?

                }
Quoted 62 lines

Overall, it may make sense to improve error handling in this driver, but I think it should be done in a separate patch.

Quoted 58 lines
drivers/usb/typec/tcpm/tcpci.c +9 -0
      }
 
      if (status & TCPC_ALERT_RX_STATUS) {
@@ -622,6 +644,12 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
              tcpm_pd_receive(tcpci->port, &msg);
      }
 
+     if (status & TCPC_ALERT_EXTENDED_STATUS) {
+             ret = regmap_read(tcpci->regmap, TCPC_EXTENDED_STATUS, &raw);
+             if (ret >= 0 && (raw & TCPC_EXTENDED_STATUS_VSAFE0V))
+                     tcpm_vbus_change(tcpci->port);
+     }
+
      if (status & TCPC_ALERT_RX_HARD_RST)
              tcpm_pd_hard_reset(tcpci->port);
 
@@ -699,6 +727,9 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
                      tcpci_set_auto_vbus_discharge_threshold;
      }
 
+     if (tcpci->data->vbus_vsafe0v)
+             tcpci->tcpc.is_vbus_vsafe0v = tcpci_is_vbus_vsafe0v;
+
      err = tcpci_parse_config(tcpci);
      if (err < 0)
              return ERR_PTR(err);
drivers/usb/typec/tcpm/tcpci.h +6 -1
index 3fe313655f0c..116a69c85e38 100644
@@ -49,6 +49,9 @@
 #define TCPC_TCPC_CTRL_ORIENTATION   BIT(0)
 #define TCPC_TCPC_CTRL_BIST_TM               BIT(1)
 
+#define TCPC_EXTENDED_STATUS         0x20
+#define TCPC_EXTENDED_STATUS_VSAFE0V BIT(0)
+
 #define TCPC_ROLE_CTRL                       0x1a
 #define TCPC_ROLE_CTRL_DRP           BIT(6)
 #define TCPC_ROLE_CTRL_RP_VAL_SHIFT  4
@@ -155,11 +158,14 @@ struct tcpci;
  *           is sourcing vbus.
  * @auto_discharge_disconnect:
  *           Optional; Enables TCPC to autonously discharge vbus on disconnect.
+ * @vbus_vsafe0v:
+ *           optional; Set when TCPC can detect whether vbus is at VSAFE0V.
  */
 struct tcpci_data {
      struct regmap *regmap;
      unsigned char TX_BUF_BYTE_x_hidden:1;
      unsigned char auto_discharge_disconnect:1;
+     unsigned char vbus_vsafe0v:1;
 
      int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
      int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
--

2.29.2.454.gaff20da3a2-goog

author maintainer
#4 lore raw

PATCH 3/3 usb: typec: tcpci_maxim: Enable VSAFE0V signalling

In reply to Badhri Jagan Sridharan

Unmask EXTENDED_STATUS_MASK.vSafe0V, ALERT.Extended_Status and set vbus_vsafe0v to enable VSAFE0V signalling.

---
 drivers/usb/typec/tcpm/tcpci_maxim.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
drivers/usb/typec/tcpm/tcpci_maxim.c +15 -1
index c1797239bf08..1aabe6abd819 100644
@@ -112,11 +112,18 @@ static void max_tcpci_init_regs(struct max_tcpci_chip *chip)
 		return;
 	}
 
+	/* Enable VSAFE0V detection */
+	ret = max_tcpci_write8(chip, TCPC_EXTENDED_STATUS_MASK, TCPC_EXTENDED_STATUS_VSAFE0V);
+	if (ret < 0) {
+		dev_err(chip->dev, "Unable to unmask TCPC_EXTENDED_STATUS_VSAFE0V ret:%d\n", ret);
+		return;
+	}
+
 	alert_mask = TCPC_ALERT_TX_SUCCESS | TCPC_ALERT_TX_DISCARDED | TCPC_ALERT_TX_FAILED |
 		TCPC_ALERT_RX_HARD_RST | TCPC_ALERT_RX_STATUS | TCPC_ALERT_CC_STATUS |
 		TCPC_ALERT_VBUS_DISCNCT | TCPC_ALERT_RX_BUF_OVF | TCPC_ALERT_POWER_STATUS |
 		/* Enable Extended alert for detecting Fast Role Swap Signal */
-		TCPC_ALERT_EXTND;
+		TCPC_ALERT_EXTND | TCPC_ALERT_EXTENDED_STATUS;
 
 	ret = max_tcpci_write16(chip, TCPC_ALERT_MASK, alert_mask);
 	if (ret < 0) {
@@ -315,6 +322,12 @@ static irqreturn_t _max_tcpci_irq(struct max_tcpci_chip *chip, u16 status)
 		}
 	}
 
+	if (status & TCPC_ALERT_EXTENDED_STATUS) {
+		ret = max_tcpci_read8(chip, TCPC_EXTENDED_STATUS, (u8 *)&reg_status);
+		if (ret >= 0 && (reg_status & TCPC_EXTENDED_STATUS_VSAFE0V))
+			tcpm_vbus_change(chip->port);
+	}
+
 	if (status & TCPC_ALERT_RX_STATUS)
 		process_rx(chip, status);
 
@@ -442,6 +455,7 @@ static int max_tcpci_probe(struct i2c_client *client, const struct i2c_device_id
 	chip->data.init = tcpci_init;
 	chip->data.frs_sourcing_vbus = max_tcpci_frs_sourcing_vbus;
 	chip->data.auto_discharge_disconnect = true;
+	chip->data.vbus_vsafe0v = 1;
 
 	max_tcpci_init_regs(chip);
 	chip->tcpci = tcpci_register_port(chip->dev, &chip->data);
Signature · 1 line
2.29.2.454.gaff20da3a2-goog
#5 lore raw

Re: [PATCH v1 3/3] usb: typec: tcpci_maxim: Enable VSAFE0V signalling

Quoted 46 lines · Badhri Jagan Sridharan

Unmask EXTENDED_STATUS_MASK.vSafe0V, ALERT.Extended_Status and set vbus_vsafe0v to enable VSAFE0V signalling.

---
 drivers/usb/typec/tcpm/tcpci_maxim.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
drivers/usb/typec/tcpm/tcpci_maxim.c +14 -1
index c1797239bf08..1aabe6abd819 100644
@@ -112,11 +112,18 @@ static void max_tcpci_init_regs(struct max_tcpci_chip *chip)
 		return;
 	}
 
+	/* Enable VSAFE0V detection */
+	ret = max_tcpci_write8(chip, TCPC_EXTENDED_STATUS_MASK, TCPC_EXTENDED_STATUS_VSAFE0V);
+	if (ret < 0) {
+		dev_err(chip->dev, "Unable to unmask TCPC_EXTENDED_STATUS_VSAFE0V ret:%d\n", ret);
+		return;
+	}
+
 	alert_mask = TCPC_ALERT_TX_SUCCESS | TCPC_ALERT_TX_DISCARDED | TCPC_ALERT_TX_FAILED |
 		TCPC_ALERT_RX_HARD_RST | TCPC_ALERT_RX_STATUS | TCPC_ALERT_CC_STATUS |
 		TCPC_ALERT_VBUS_DISCNCT | TCPC_ALERT_RX_BUF_OVF | TCPC_ALERT_POWER_STATUS |
 		/* Enable Extended alert for detecting Fast Role Swap Signal */
-		TCPC_ALERT_EXTND;
+		TCPC_ALERT_EXTND | TCPC_ALERT_EXTENDED_STATUS;
 
 	ret = max_tcpci_write16(chip, TCPC_ALERT_MASK, alert_mask);
 	if (ret < 0) {
@@ -315,6 +322,12 @@ static irqreturn_t _max_tcpci_irq(struct max_tcpci_chip *chip, u16 status)
 		}
 	}
 
+	if (status & TCPC_ALERT_EXTENDED_STATUS) {
+		ret = max_tcpci_read8(chip, TCPC_EXTENDED_STATUS, (u8 *)&reg_status);
+		if (ret >= 0 && (reg_status & TCPC_EXTENDED_STATUS_VSAFE0V))
+			tcpm_vbus_change(chip->port);
+	}
+
 	if (status & TCPC_ALERT_RX_STATUS)
 		process_rx(chip, status);
 

drivers/usb/typec/tcpm/tcpci_maxim.c static int max_tcpci_probe(struct i2c_client *client, const struct i2c_device_id · line 458

Quoted 4 lines
drivers/usb/typec/tcpm/tcpci_maxim.c +1 -0
@@ -442,6 +455,7 @@ static int max_tcpci_probe(struct i2c_client *client, const struct i2c_device_id
 	chip->data.init = tcpci_init;
 	chip->data.frs_sourcing_vbus = max_tcpci_frs_sourcing_vbus;
 	chip->data.auto_discharge_disconnect = true;
+	chip->data.vbus_vsafe0v = 1;

Why not use true/false ?

Quoted 6 lines
drivers/usb/typec/tcpm/tcpci_maxim.c +0 -0
 	max_tcpci_init_regs(chip);
 	chip->tcpci = tcpci_register_port(chip->dev, &chip->data);
Signature · 1 line
2.29.2.454.gaff20da3a2-goog
author maintainer
#6 lore raw

Re: [PATCH v1 3/3] usb: typec: tcpci_maxim: Enable VSAFE0V signalling

Quoted 47 lines · Guenter Roeck
Quoted 45 lines · Badhri Jagan Sridharan

Unmask EXTENDED_STATUS_MASK.vSafe0V, ALERT.Extended_Status and set vbus_vsafe0v to enable VSAFE0V signalling.

---
 drivers/usb/typec/tcpm/tcpci_maxim.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
drivers/usb/typec/tcpm/tcpci_maxim.c +14 -1
index c1797239bf08..1aabe6abd819 100644
@@ -112,11 +112,18 @@ static void max_tcpci_init_regs(struct max_tcpci_chip *chip)
              return;
      }
 
+     /* Enable VSAFE0V detection */
+     ret = max_tcpci_write8(chip, TCPC_EXTENDED_STATUS_MASK, TCPC_EXTENDED_STATUS_VSAFE0V);
+     if (ret < 0) {
+             dev_err(chip->dev, "Unable to unmask TCPC_EXTENDED_STATUS_VSAFE0V ret:%d\n", ret);
+             return;
+     }
+
      alert_mask = TCPC_ALERT_TX_SUCCESS | TCPC_ALERT_TX_DISCARDED | TCPC_ALERT_TX_FAILED |
              TCPC_ALERT_RX_HARD_RST | TCPC_ALERT_RX_STATUS | TCPC_ALERT_CC_STATUS |
              TCPC_ALERT_VBUS_DISCNCT | TCPC_ALERT_RX_BUF_OVF | TCPC_ALERT_POWER_STATUS |
              /* Enable Extended alert for detecting Fast Role Swap Signal */
-             TCPC_ALERT_EXTND;
+             TCPC_ALERT_EXTND | TCPC_ALERT_EXTENDED_STATUS;
 
      ret = max_tcpci_write16(chip, TCPC_ALERT_MASK, alert_mask);
      if (ret < 0) {
@@ -315,6 +322,12 @@ static irqreturn_t _max_tcpci_irq(struct max_tcpci_chip *chip, u16 status)
              }
      }
 
+     if (status & TCPC_ALERT_EXTENDED_STATUS) {
+             ret = max_tcpci_read8(chip, TCPC_EXTENDED_STATUS, (u8 *)&reg_status);
+             if (ret >= 0 && (reg_status & TCPC_EXTENDED_STATUS_VSAFE0V))
+                     tcpm_vbus_change(chip->port);
+     }
+
      if (status & TCPC_ALERT_RX_STATUS)
              process_rx(chip, status);
 
Quoted 7 lines

drivers/usb/typec/tcpm/tcpci_maxim.c static int max_tcpci_probe(struct i2c_client *client, const struct i2c_device_id · line 458

Quoted 5 lines
drivers/usb/typec/tcpm/tcpci_maxim.c +1 -0
@@ -442,6 +455,7 @@ static int max_tcpci_probe(struct i2c_client *client, const struct i2c_device_id
      chip->data.init = tcpci_init;
      chip->data.frs_sourcing_vbus = max_tcpci_frs_sourcing_vbus;
      chip->data.auto_discharge_disconnect = true;
+     chip->data.vbus_vsafe0v = 1;

Why not use true/false ?

Sure. Fixing in v2.

Quoted 7 lines
Quoted 7 lines
drivers/usb/typec/tcpm/tcpci_maxim.c +0 -1
      max_tcpci_init_regs(chip);
      chip->tcpci = tcpci_register_port(chip->dev, &chip->data);
--

2.29.2.454.gaff20da3a2-goog

#7 lore raw
In reply to Badhri Jagan Sridharan
Quoted 177 lines · Badhri Jagan Sridharan

TCPM at present lacks the notion of VSAFE0V. There are three vbus threshold levels that are critical to track:
a. vSafe5V - VBUS “5 volts” as defined by the USB
PD specification.
b. vSinkDisconnect - Threshold used for transition from
Attached.SNK to Unattached.SNK.
c. vSafe0V - VBUS “0 volts” as defined by the USB
PD specification.

Tracking vSafe0V is crucial for entry into Try.SNK and Attached.SRC and turning vbus back on by the source in response to hard reset.

From "4.5.2.2.8.2 Exiting from AttachWait.SRC State" section in the Type-C spec:

"The port shall transition to Attached.SRC when VBUS is at vSafe0V and the SRC.Rd state is detected on exactly one of the CC1 or CC2 pins for at least tCCDebounce."

"A DRP that strongly prefers the Sink role may optionally transition to Try.SNK instead of Attached.SRC when VBUS is at vSafe0V and the SRC.Rd state is detected on exactly one of the CC1 or CC2 pins for at least tCCDebounce."

From "7.1.5 Response to Hard Resets" section in the PD spec:

"After establishing the vSafe0V voltage condition on VBUS, the Source Shall wait tSrcRecover before re-applying VCONN and restoring VBUS to vSafe5V."

vbus_present in the TCPM code tracks vSafe5V(vbus_present is true) and vSinkDisconnect(vbus_present is false).

This change adds is_vbus_vsafe0v callback which when set makes TCPM query for vSafe0V voltage level when needed.

Since not all TCPC controllers might have the capability to report vSafe0V, TCPM assumes that vSafe0V is same as vSinkDisconnect when is_vbus_vsafe0v callback is not set. This allows TCPM to continue to support controllers which don't have the support for reporting vSafe0V.

Introducing vSafe0V helps fix the failure reported at "Step 15. CVS verifies PUT remains in AttachWait.SRC for 500ms" of "TD 4.7.2 Try. SNK DRP Connect DRP Test" of "Universal Serial Bus Type-C (USB Type-C) Functional Test Specification Chapters 4 and 5". Here the compliance tester intentionally maintains vbus at greater than vSafe0V and expects the Product under test to stay in AttachWait.SRC till vbus drops to vSafe0V.

---
 drivers/usb/typec/tcpm/tcpm.c | 63 +++++++++++++++++++++++++++++------
 include/linux/usb/tcpm.h      |  6 ++++
 2 files changed, 58 insertions(+), 11 deletions(-)
drivers/usb/typec/tcpm/tcpm.c +52 -11
index 3bbc1f10af49..10a065eef73e 100644
@@ -258,7 +258,19 @@ struct tcpm_port {
 	bool attached;
 	bool connected;
 	enum typec_port_type port_type;
+
+	/*
+	 * Set to true when vbus is greater than VSAFE5V min.
+	 * Set to false when vbus falls below vSinkDisconnect max threshold.
+	 */
 	bool vbus_present;
+
+	/*
+	 * Set to true when vbus is less than VSAFE0V max.
+	 * Set to false when vbus is greater than VSAFE0V max.
+	 */
+	bool vbus_vsafe0v;
+
 	bool vbus_never_low;
 	bool vbus_source;
 	bool vbus_charge;
@@ -3094,7 +3106,7 @@ static void run_state_machine(struct tcpm_port *port)
 		else if (tcpm_port_is_audio(port))
 			tcpm_set_state(port, AUDIO_ACC_ATTACHED,
 				       PD_T_CC_DEBOUNCE);
-		else if (tcpm_port_is_source(port))
+		else if (tcpm_port_is_source(port) && port->vbus_vsafe0v)
 			tcpm_set_state(port,
 				       tcpm_try_snk(port) ? SNK_TRY
 							  : SRC_ATTACHED,
@@ -4097,6 +4109,12 @@ static void _tcpm_pd_vbus_on(struct tcpm_port *port)
 {
 	tcpm_log_force(port, "VBUS on");
 	port->vbus_present = true;
+	/*
+	 * When vbus_present is true i.e. Voltage at VBUS is greater than VSAFE5V implicitly
+	 * states that vbus is not at VSAFE0V, hence clear the vbus_vsafe0v flag here.
+	 */
+	port->vbus_vsafe0v = false;
+
 	switch (port->state) {
 	case SNK_TRANSITION_SINK_VBUS:
 		port->explicit_contract = true;
@@ -4186,16 +4204,8 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)
 	case SNK_HARD_RESET_SINK_OFF:
 		tcpm_set_state(port, SNK_HARD_RESET_WAIT_VBUS, 0);
 		break;
-	case SRC_HARD_RESET_VBUS_OFF:
-		/*
-		 * After establishing the vSafe0V voltage condition on VBUS, the Source Shall wait
-		 * tSrcRecover before re-applying VCONN and restoring VBUS to vSafe5V.
-		 */
-		tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
-		break;
 	case HARD_RESET_SEND:
 		break;
-
 	case SNK_TRY:
 		/* Do nothing, waiting for timeout */
 		break;
@@ -4266,6 +4276,28 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)
 	}
 }
 
+static void _tcpm_pd_vbus_vsafe0v(struct tcpm_port *port)
+{
+	tcpm_log_force(port, "VBUS VSAFE0V");
+	port->vbus_vsafe0v = true;
+	switch (port->state) {
+	case SRC_HARD_RESET_VBUS_OFF:
+		/*
+		 * After establishing the vSafe0V voltage condition on VBUS, the Source Shall wait
+		 * tSrcRecover before re-applying VCONN and restoring VBUS to vSafe5V.
+		 */
+		tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
+		break;
+	case SRC_ATTACH_WAIT:
+		if (tcpm_port_is_source(port))
+			tcpm_set_state(port, tcpm_try_snk(port) ? SNK_TRY : SRC_ATTACHED,
+				       PD_T_CC_DEBOUNCE);
+		break;
+	default:
+		break;
+	}
+}
+
 static void _tcpm_pd_hard_reset(struct tcpm_port *port)
 {
 	tcpm_log_force(port, "Received hard reset");
@@ -4301,10 +4333,19 @@ static void tcpm_pd_event_handler(struct kthread_work *work)
 			bool vbus;
 
 			vbus = port->tcpc->get_vbus(port->tcpc);
-			if (vbus)
+			if (vbus) {
 				_tcpm_pd_vbus_on(port);
-			else
+			} else {
 				_tcpm_pd_vbus_off(port);
+				/*
+				 * When TCPC does not support detecting vsafe0v voltage level,
+				 * treat vbus absent as vsafe0v. Else invoke is_vbus_vsafe0v
+				 * to see if vbus has discharge to VSAFE0V.
+				 */
+				if (!port->tcpc->is_vbus_vsafe0v ||
+				    port->tcpc->is_vbus_vsafe0v(port->tcpc) > 0)
+					_tcpm_pd_vbus_vsafe0v(port);
+			}
 		}
 		if (events & TCPM_CC_EVENT) {
 			enum typec_cc_status cc1, cc2;
include/linux/usb/tcpm.h +0 -0
index e68aaa12886f..615d4532c028 100644

include/linux/usb/tcpm.h enum tcpm_transmit_type { · line 105

Quoted 8 lines
include/linux/usb/tcpm.h +5 -0
@@ -98,6 +98,11 @@ enum tcpm_transmit_type {
  *		will be turned on. requested_vbus_voltage is set to 0 when vbus
  *		is going to disappear knowingly i.e. during PR_SWAP and
  *		HARD_RESET etc.
+ * @is_vbus_vsafe0v:
+ *		Optional; TCPCI spec based TCPC implementations are expected to
+ *		detect VSAFE0V voltage level at vbus. When detection of VSAFE0V
+ *		is supported by TCPC, set this callback for TCPM to query
+ *		whether vbus is at VSAFE0V when needed.

Return values ?

include/linux/usb/tcpm.h struct tcpc_dev { · line 136

Quoted 8 lines
include/linux/usb/tcpm.h +1 -0
  */
 struct tcpc_dev {
 	struct fwnode_handle *fwnode;
@@ -128,6 +133,7 @@ struct tcpc_dev {
 	int (*enable_auto_vbus_discharge)(struct tcpc_dev *dev, bool enable);
 	int (*set_auto_vbus_discharge_threshold)(struct tcpc_dev *dev, enum typec_pwr_opmode mode,
 						 bool pps_active, u32 requested_vbus_voltage);
+	int (*is_vbus_vsafe0v)(struct tcpc_dev *dev);

I don't really see the point of returning an int here. If an error is returned, it should be checked. However, that is not currently the case; an error is treated as "false" return. As such, the function may as well return true/false (and false if an error is observed).

Quoted 6 lines
include/linux/usb/tcpm.h +0 -0
 };
 
 struct tcpm_port;
Signature · 1 line
2.29.2.454.gaff20da3a2-goog
author maintainer
#8 lore raw
Quoted 181 lines · Guenter Roeck
Quoted 179 lines · Badhri Jagan Sridharan

TCPM at present lacks the notion of VSAFE0V. There are three vbus threshold levels that are critical to track:
a. vSafe5V - VBUS “5 volts” as defined by the USB
PD specification.
b. vSinkDisconnect - Threshold used for transition from
Attached.SNK to Unattached.SNK.
c. vSafe0V - VBUS “0 volts” as defined by the USB
PD specification.

Tracking vSafe0V is crucial for entry into Try.SNK and Attached.SRC and turning vbus back on by the source in response to hard reset.

From "4.5.2.2.8.2 Exiting from AttachWait.SRC State" section in the Type-C spec:

"The port shall transition to Attached.SRC when VBUS is at vSafe0V and the SRC.Rd state is detected on exactly one of the CC1 or CC2 pins for at least tCCDebounce."

"A DRP that strongly prefers the Sink role may optionally transition to Try.SNK instead of Attached.SRC when VBUS is at vSafe0V and the SRC.Rd state is detected on exactly one of the CC1 or CC2 pins for at least tCCDebounce."

From "7.1.5 Response to Hard Resets" section in the PD spec:

"After establishing the vSafe0V voltage condition on VBUS, the Source Shall wait tSrcRecover before re-applying VCONN and restoring VBUS to vSafe5V."

vbus_present in the TCPM code tracks vSafe5V(vbus_present is true) and vSinkDisconnect(vbus_present is false).

This change adds is_vbus_vsafe0v callback which when set makes TCPM query for vSafe0V voltage level when needed.

Since not all TCPC controllers might have the capability to report vSafe0V, TCPM assumes that vSafe0V is same as vSinkDisconnect when is_vbus_vsafe0v callback is not set. This allows TCPM to continue to support controllers which don't have the support for reporting vSafe0V.

Introducing vSafe0V helps fix the failure reported at "Step 15. CVS verifies PUT remains in AttachWait.SRC for 500ms" of "TD 4.7.2 Try. SNK DRP Connect DRP Test" of "Universal Serial Bus Type-C (USB Type-C) Functional Test Specification Chapters 4 and 5". Here the compliance tester intentionally maintains vbus at greater than vSafe0V and expects the Product under test to stay in AttachWait.SRC till vbus drops to vSafe0V.

---
 drivers/usb/typec/tcpm/tcpm.c | 63 +++++++++++++++++++++++++++++------
 include/linux/usb/tcpm.h      |  6 ++++
 2 files changed, 58 insertions(+), 11 deletions(-)
drivers/usb/typec/tcpm/tcpm.c +52 -11
index 3bbc1f10af49..10a065eef73e 100644
@@ -258,7 +258,19 @@ struct tcpm_port {
      bool attached;
      bool connected;
      enum typec_port_type port_type;
+
+     /*
+      * Set to true when vbus is greater than VSAFE5V min.
+      * Set to false when vbus falls below vSinkDisconnect max threshold.
+      */
      bool vbus_present;
+
+     /*
+      * Set to true when vbus is less than VSAFE0V max.
+      * Set to false when vbus is greater than VSAFE0V max.
+      */
+     bool vbus_vsafe0v;
+
      bool vbus_never_low;
      bool vbus_source;
      bool vbus_charge;
@@ -3094,7 +3106,7 @@ static void run_state_machine(struct tcpm_port *port)
              else if (tcpm_port_is_audio(port))
                      tcpm_set_state(port, AUDIO_ACC_ATTACHED,
                                     PD_T_CC_DEBOUNCE);
-             else if (tcpm_port_is_source(port))
+             else if (tcpm_port_is_source(port) && port->vbus_vsafe0v)
                      tcpm_set_state(port,
                                     tcpm_try_snk(port) ? SNK_TRY
                                                        : SRC_ATTACHED,
@@ -4097,6 +4109,12 @@ static void _tcpm_pd_vbus_on(struct tcpm_port *port)
 {
      tcpm_log_force(port, "VBUS on");
      port->vbus_present = true;
+     /*
+      * When vbus_present is true i.e. Voltage at VBUS is greater than VSAFE5V implicitly
+      * states that vbus is not at VSAFE0V, hence clear the vbus_vsafe0v flag here.
+      */
+     port->vbus_vsafe0v = false;
+
      switch (port->state) {
      case SNK_TRANSITION_SINK_VBUS:
              port->explicit_contract = true;
@@ -4186,16 +4204,8 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)
      case SNK_HARD_RESET_SINK_OFF:
              tcpm_set_state(port, SNK_HARD_RESET_WAIT_VBUS, 0);
              break;
-     case SRC_HARD_RESET_VBUS_OFF:
-             /*
-              * After establishing the vSafe0V voltage condition on VBUS, the Source Shall wait
-              * tSrcRecover before re-applying VCONN and restoring VBUS to vSafe5V.
-              */
-             tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
-             break;
      case HARD_RESET_SEND:
              break;
-
      case SNK_TRY:
              /* Do nothing, waiting for timeout */
              break;
@@ -4266,6 +4276,28 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)
      }
 }
 
+static void _tcpm_pd_vbus_vsafe0v(struct tcpm_port *port)
+{
+     tcpm_log_force(port, "VBUS VSAFE0V");
+     port->vbus_vsafe0v = true;
+     switch (port->state) {
+     case SRC_HARD_RESET_VBUS_OFF:
+             /*
+              * After establishing the vSafe0V voltage condition on VBUS, the Source Shall wait
+              * tSrcRecover before re-applying VCONN and restoring VBUS to vSafe5V.
+              */
+             tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
+             break;
+     case SRC_ATTACH_WAIT:
+             if (tcpm_port_is_source(port))
+                     tcpm_set_state(port, tcpm_try_snk(port) ? SNK_TRY : SRC_ATTACHED,
+                                    PD_T_CC_DEBOUNCE);
+             break;
+     default:
+             break;
+     }
+}
+
 static void _tcpm_pd_hard_reset(struct tcpm_port *port)
 {
      tcpm_log_force(port, "Received hard reset");
@@ -4301,10 +4333,19 @@ static void tcpm_pd_event_handler(struct kthread_work *work)
                      bool vbus;
 
                      vbus = port->tcpc->get_vbus(port->tcpc);
-                     if (vbus)
+                     if (vbus) {
                              _tcpm_pd_vbus_on(port);
-                     else
+                     } else {
                              _tcpm_pd_vbus_off(port);
+                             /*
+                              * When TCPC does not support detecting vsafe0v voltage level,
+                              * treat vbus absent as vsafe0v. Else invoke is_vbus_vsafe0v
+                              * to see if vbus has discharge to VSAFE0V.
+                              */
+                             if (!port->tcpc->is_vbus_vsafe0v ||
+                                 port->tcpc->is_vbus_vsafe0v(port->tcpc) > 0)
+                                     _tcpm_pd_vbus_vsafe0v(port);
+                     }
              }
              if (events & TCPM_CC_EVENT) {
                      enum typec_cc_status cc1, cc2;
include/linux/usb/tcpm.h +0 -0
index e68aaa12886f..615d4532c028 100644
@@ -98,6 +98,11 @@ enum tcpm_transmit_type {
  *           will be turned on. requested_vbus_voltage is set to 0 when vbus
  *           is going to disappear knowingly i.e. during PR_SWAP and
Quoted 8 lines

include/linux/usb/tcpm.h enum tcpm_transmit_type { · line 105

Quoted 6 lines
include/linux/usb/tcpm.h +5 -0
  *           HARD_RESET etc.
+ * @is_vbus_vsafe0v:
+ *           Optional; TCPCI spec based TCPC implementations are expected to
+ *           detect VSAFE0V voltage level at vbus. When detection of VSAFE0V
+ *           is supported by TCPC, set this callback for TCPM to query
+ *           whether vbus is at VSAFE0V when needed.

Return values ?

Fixing in v2.

Quoted 14 lines

include/linux/usb/tcpm.h struct tcpc_dev { · line 136

Quoted 9 lines
include/linux/usb/tcpm.h +1 -0
  */
 struct tcpc_dev {
      struct fwnode_handle *fwnode;
@@ -128,6 +133,7 @@ struct tcpc_dev {
      int (*enable_auto_vbus_discharge)(struct tcpc_dev *dev, bool enable);
      int (*set_auto_vbus_discharge_threshold)(struct tcpc_dev *dev, enum typec_pwr_opmode mode,
                                               bool pps_active, u32 requested_vbus_voltage);
+     int (*is_vbus_vsafe0v)(struct tcpc_dev *dev);

I don't really see the point of returning an int here. If an error is returned, it should be checked. However, that is not currently the case; an error is treated as "false" return. As such, the function may as well return true/false (and false if an error is observed).

Sure. Changing the callback return to boolean in v2.

Quoted 7 lines
Quoted 7 lines
include/linux/usb/tcpm.h +0 -1
 };
 
 struct tcpm_port;
--

2.29.2.454.gaff20da3a2-goog