pd: wait tSnkTransition after ACCEPT to change voltage

Ensure that the PD source changes the output voltage after
tSnkTransition delay after having sent the ACCEPT message
(rather than before).

Signed-off-by: Vincent Palatin <vpalatin@chromium.org>

BRANCH=samus
BUG=chrome-os-partner:33684
TEST=connect Zinger to a PD power sink and monitor VBUS and CC while
doing a 20V to 5V transition.

Change-Id: If86f59eec67630491f4e8dc13a52015ac2de918a
Reviewed-on: https://chromium-review.googlesource.com/230805
Reviewed-by: Alec Berg <alecaberg@chromium.org>
Commit-Queue: Vincent Palatin <vpalatin@chromium.org>
Tested-by: Vincent Palatin <vpalatin@chromium.org>

13 files changed, 101 insertions, 36 deletions

diff --git a/board/dingdong/usb_pd_policy.c b/board/dingdong/usb_pd_policy.c
index 63223d8927..2743eb12fd 100644
--- a/board/dingdong/usb_pd_policy.c
+++ b/board/dingdong/usb_pd_policy.c
@@ -96,12 +96,16 @@ void pd_set_max_voltage(unsigned mv)
 	select_mv = mv;
 }
 
-int requested_voltage_idx;
-int pd_request_voltage(uint32_t rdo)
+int pd_check_requested_voltage(uint32_t rdo)
 {
 	return EC_SUCCESS;
 }
 
+void pd_transition_voltage(int idx)
+{
+	/* No operation: sink only */
+}
+
 int pd_set_power_supply_ready(int port)
 {
 	return EC_SUCCESS;
diff --git a/board/firefly/usb_pd_policy.c b/board/firefly/usb_pd_policy.c
index 4a04988290..07b8e7745d 100644
--- a/board/firefly/usb_pd_policy.c
+++ b/board/firefly/usb_pd_policy.c
@@ -97,12 +97,17 @@ void pd_set_max_voltage(unsigned mv)
 	select_mv = mv;
 }
 
-int pd_request_voltage(uint32_t rdo)
+int pd_check_requested_voltage(uint32_t rdo)
 {
 	/* Never acting as a source */
 	return EC_ERROR_INVAL;
 }
 
+void pd_transition_voltage(int idx)
+{
+	/* No operation: sink only */
+}
+
 int pd_set_power_supply_ready(int port)
 {
 	/* Never acting as a source */
diff --git a/board/fruitpie/usb_pd_policy.c b/board/fruitpie/usb_pd_policy.c
index fcc25d1345..52adce5677 100644
--- a/board/fruitpie/usb_pd_policy.c
+++ b/board/fruitpie/usb_pd_policy.c
@@ -98,7 +98,7 @@ void pd_set_max_voltage(unsigned mv)
 	max_mv = mv;
 }
 
-int pd_request_voltage(uint32_t rdo)
+int pd_check_requested_voltage(uint32_t rdo)
 {
 	int op_ma = rdo & 0x3FF;
 	int max_ma = (rdo >> 10) & 0x3FF;
@@ -117,13 +117,18 @@ int pd_request_voltage(uint32_t rdo)
 	if (max_ma > pdo_ma)
 		return EC_ERROR_INVAL; /* too much max current */
 
-	CPRINTF("Switch to %d V %d mA (for %d/%d mA)\n",
+	CPRINTF("Requested %d V %d mA (for %d/%d mA)\n",
 		 ((pdo >> 10) & 0x3ff) * 50, (pdo & 0x3ff) * 10,
 		 ((rdo >> 10) & 0x3ff) * 10, (rdo & 0x3ff) * 10);
 
 	return EC_SUCCESS;
 }
 
+void pd_transition_voltage(int idx)
+{
+	/* No-operation: we are always 5V */
+}
+
 int pd_set_power_supply_ready(int port)
 {
 	/* provide VBUS */
diff --git a/board/hoho/usb_pd_policy.c b/board/hoho/usb_pd_policy.c
index 6098c790c1..a2d35f6e58 100644
--- a/board/hoho/usb_pd_policy.c
+++ b/board/hoho/usb_pd_policy.c
@@ -96,12 +96,16 @@ void pd_set_max_voltage(unsigned mv)
 	select_mv = mv;
 }
 
-int requested_voltage_idx;
-int pd_request_voltage(uint32_t rdo)
+int pd_check_requested_voltage(uint32_t rdo)
 {
 	return EC_SUCCESS;
 }
 
+void pd_transition_voltage(int idx)
+{
+	/* No operation: sink only */
+}
+
 int pd_set_power_supply_ready(int port)
 {
 	return EC_SUCCESS;
diff --git a/board/host/usb_pd_policy.c b/board/host/usb_pd_policy.c
index 6432e0ae7b..423add3543 100644
--- a/board/host/usb_pd_policy.c
+++ b/board/host/usb_pd_policy.c
@@ -97,7 +97,7 @@ void pd_set_max_voltage(unsigned mv)
 	max_mv = mv;
 }
 
-int pd_request_voltage(uint32_t rdo)
+int pd_check_requested_voltage(uint32_t rdo)
 {
 	int op_ma = rdo & 0x3FF;
 	int max_ma = (rdo >> 10) & 0x3FF;
@@ -116,13 +116,18 @@ int pd_request_voltage(uint32_t rdo)
 	if (max_ma > pdo_ma)
 		return EC_ERROR_INVAL; /* too much max current */
 
-	CPRINTF("Switch to %d V %d mA (for %d/%d mA)\n",
+	CPRINTF("Requested %d V %d mA (for %d/%d mA)\n",
 		((pdo >> 10) & 0x3ff) * 50, (pdo & 0x3ff) * 10,
 		((rdo >> 10) & 0x3ff) * 10, (rdo & 0x3ff) * 10);
 
 	return EC_SUCCESS;
 }
 
+void pd_transition_voltage(int idx)
+{
+	/* Not implemented */
+}
+
 int pd_set_power_supply_ready(int port)
 {
 	/* Not implemented */
diff --git a/board/plankton/usb_pd_policy.c b/board/plankton/usb_pd_policy.c
index b2fb708888..ad691f6187 100644
--- a/board/plankton/usb_pd_policy.c
+++ b/board/plankton/usb_pd_policy.c
@@ -117,8 +117,7 @@ void pd_set_max_voltage(unsigned mv)
 	select_mv = mv;
 }
 
-int requested_voltage_idx;
-int pd_request_voltage(uint32_t rdo)
+int pd_check_requested_voltage(uint32_t rdo)
 {
 	int op_ma = rdo & 0x3FF;
 	int max_ma = (rdo >> 10) & 0x3FF;
@@ -137,21 +136,23 @@ int pd_request_voltage(uint32_t rdo)
 	if (max_ma > pdo_ma)
 		return EC_ERROR_INVAL; /* too much max current */
 
-	CPRINTF("Switch to %d V %d mA (for %d/%d mA)\n",
+	CPRINTF("Requested %d V %d mA (for %d/%d mA)\n",
 		((pdo >> 10) & 0x3ff) * 50, (pdo & 0x3ff) * 10,
 		((rdo >> 10) & 0x3ff) * 10, (rdo & 0x3ff) * 10);
 
-	requested_voltage_idx = idx;
-
 	return EC_SUCCESS;
 }
 
+void pd_transition_voltage(int idx)
+{
+	gpio_set_level(GPIO_USBC_VSEL_0, idx >= 2);
+	gpio_set_level(GPIO_USBC_VSEL_1, idx >= 3);
+}
+
 int pd_set_power_supply_ready(int port)
 {
 	/* Output the correct voltage */
 	gpio_set_level(GPIO_VBUS_CHARGER_EN, 1);
-	gpio_set_level(GPIO_USBC_VSEL_0, requested_voltage_idx >= 2);
-	gpio_set_level(GPIO_USBC_VSEL_1, requested_voltage_idx >= 3);
 
 	return EC_SUCCESS;
 }
@@ -159,7 +160,6 @@ int pd_set_power_supply_ready(int port)
 void pd_power_supply_reset(int port)
 {
 	/* Kill VBUS */
-	requested_voltage_idx = 0;
 	gpio_set_level(GPIO_VBUS_CHARGER_EN, 0);
 	gpio_set_level(GPIO_USBC_VSEL_0, 0);
 	gpio_set_level(GPIO_USBC_VSEL_1, 0);
diff --git a/board/ryu/usb_pd_policy.c b/board/ryu/usb_pd_policy.c
index aea73188d4..cc8979e6f8 100644
--- a/board/ryu/usb_pd_policy.c
+++ b/board/ryu/usb_pd_policy.c
@@ -113,7 +113,7 @@ void pd_set_max_voltage(unsigned mv)
 	max_mv = mv;
 }
 
-int pd_request_voltage(uint32_t rdo)
+int pd_check_requested_voltage(uint32_t rdo)
 {
 	int op_ma = rdo & 0x3FF;
 	int max_ma = (rdo >> 10) & 0x3FF;
@@ -132,13 +132,18 @@ int pd_request_voltage(uint32_t rdo)
 	if (max_ma > pdo_ma)
 		return EC_ERROR_INVAL; /* too much max current */
 
-	CPRINTF("Switch to %d V %d mA (for %d/%d mA)\n",
+	CPRINTF("Requested %d V %d mA (for %d/%d mA)\n",
 		((pdo >> 10) & 0x3ff) * 50, (pdo & 0x3ff) * 10,
 		((rdo >> 10) & 0x3ff) * 10, (rdo & 0x3ff) * 10);
 
 	return EC_SUCCESS;
 }
 
+void pd_transition_voltage(int idx)
+{
+	/* No-operation: we are always 5V */
+}
+
 int pd_set_power_supply_ready(int port)
 {
 	/* provide VBUS */
diff --git a/board/ryu_p1/usb_pd_policy.c b/board/ryu_p1/usb_pd_policy.c
index 9ee61dde65..d98d17c11c 100644
--- a/board/ryu_p1/usb_pd_policy.c
+++ b/board/ryu_p1/usb_pd_policy.c
@@ -113,7 +113,7 @@ void pd_set_max_voltage(unsigned mv)
 	max_mv = mv;
 }
 
-int pd_request_voltage(uint32_t rdo)
+int pd_check_requested_voltage(uint32_t rdo)
 {
 	int op_ma = rdo & 0x3FF;
 	int max_ma = (rdo >> 10) & 0x3FF;
@@ -132,13 +132,18 @@ int pd_request_voltage(uint32_t rdo)
 	if (max_ma > pdo_ma)
 		return EC_ERROR_INVAL; /* too much max current */
 
-	CPRINTF("Switch to %d V %d mA (for %d/%d mA)\n",
+	CPRINTF("Requested %d V %d mA (for %d/%d mA)\n",
 		((pdo >> 10) & 0x3ff) * 50, (pdo & 0x3ff) * 10,
 		((rdo >> 10) & 0x3ff) * 10, (rdo & 0x3ff) * 10);
 
 	return EC_SUCCESS;
 }
 
+void pd_transition_voltage(int idx)
+{
+	/* No-operation: we are always 5V */
+}
+
 int pd_set_power_supply_ready(int port)
 {
 	/* provide VBUS */
diff --git a/board/samus_pd/usb_pd_policy.c b/board/samus_pd/usb_pd_policy.c
index 036653af9a..b87c880ded 100644
--- a/board/samus_pd/usb_pd_policy.c
+++ b/board/samus_pd/usb_pd_policy.c
@@ -131,7 +131,7 @@ void pd_set_max_voltage(unsigned mv)
 	max_mv = mv;
 }
 
-int pd_request_voltage(uint32_t rdo)
+int pd_check_requested_voltage(uint32_t rdo)
 {
 	int op_ma = rdo & 0x3FF;
 	int max_ma = (rdo >> 10) & 0x3FF;
@@ -150,13 +150,18 @@ int pd_request_voltage(uint32_t rdo)
 	if (max_ma > pdo_ma)
 		return EC_ERROR_INVAL; /* too much max current */
 
-	CPRINTF("Switch to %d V %d mA (for %d/%d mA)\n",
+	CPRINTF("Requested %d V %d mA (for %d/%d mA)\n",
 		 ((pdo >> 10) & 0x3ff) * 50, (pdo & 0x3ff) * 10,
 		 ((rdo >> 10) & 0x3ff) * 10, (rdo & 0x3ff) * 10);
 
 	return EC_SUCCESS;
 }
 
+void pd_transition_voltage(int idx)
+{
+	/* No-operation: we are always 5V */
+}
+
 int pd_set_power_supply_ready(int port)
 {
 	/* provide VBUS */
diff --git a/board/twinkie/usb_pd_policy.c b/board/twinkie/usb_pd_policy.c
index ec59b82769..31856c907c 100644
--- a/board/twinkie/usb_pd_policy.c
+++ b/board/twinkie/usb_pd_policy.c
@@ -116,7 +116,7 @@ void pd_set_max_voltage(unsigned mv)
 	max_mv = mv;
 }
 
-int pd_request_voltage(uint32_t rdo)
+int pd_check_requested_voltage(uint32_t rdo)
 {
 	int op_ma = rdo & 0x3FF;
 	int max_ma = (rdo >> 10) & 0x3FF;
@@ -135,13 +135,17 @@ int pd_request_voltage(uint32_t rdo)
 	if (max_ma > pdo_ma)
 		return EC_ERROR_INVAL; /* too much max current */
 
-	CPRINTF("Switch to %d V %d mA (for %d/%d mA)\n",
+	CPRINTF("Requested %d V %d mA (for %d/%d mA)\n",
 		((pdo >> 10) & 0x3ff) * 50, (pdo & 0x3ff) * 10,
 		((rdo >> 10) & 0x3ff) * 10, (rdo & 0x3ff) * 10);
 
 	return EC_SUCCESS;
 }
 
+void pd_transition_voltage(int idx)
+{
+}
+
 int pd_set_power_supply_ready(int port)
 {
 	return EC_SUCCESS; /* we are ready */
diff --git a/board/zinger/usb_pd_policy.c b/board/zinger/usb_pd_policy.c
index 2c609eca2b..de8104fea5 100644
--- a/board/zinger/usb_pd_policy.c
+++ b/board/zinger/usb_pd_policy.c
@@ -180,7 +180,7 @@ static int last_volt_idx;
 /* output current measurement */
 int vbus_amp;
 
-int pd_request_voltage(uint32_t rdo)
+int pd_check_requested_voltage(uint32_t rdo)
 {
 	int op_ma = rdo & 0x3FF;
 	int max_ma = (rdo >> 10) & 0x3FF;
@@ -203,10 +203,15 @@ int pd_request_voltage(uint32_t rdo)
 	if (max_ma > pdo_ma)
 		return EC_ERROR_INVAL; /* too much max current */
 
-	debug_printf("Switch to %d V %d mA (for %d/%d mA)\n",
+	debug_printf("Requested %d V %d mA (for %d/%d mA)\n",
 		     ((pdo >> 10) & 0x3ff) * 50, (pdo & 0x3ff) * 10,
 		     ((rdo >> 10) & 0x3ff) * 10, (rdo & 0x3ff) * 10);
+	/* Accept the requested voltage */
+	return EC_SUCCESS;
+}
 
+void pd_transition_voltage(int idx)
+{
 	if (idx - 1 < volt_idx) { /* down voltage transition */
 		/* Stop OCP monitoring */
 		adc_disable_watchdog();
@@ -224,8 +229,6 @@ int pd_request_voltage(uint32_t rdo)
 	last_volt_idx = volt_idx;
 	volt_idx = idx - 1;
 	set_output_voltage(voltages[volt_idx].select);
-
-	return EC_SUCCESS;
 }
 
 int pd_set_power_supply_ready(int port)
diff --git a/common/usb_pd_protocol.c b/common/usb_pd_protocol.c
index 7073064c2d..465e2394c4 100644
--- a/common/usb_pd_protocol.c
+++ b/common/usb_pd_protocol.c
@@ -263,6 +263,8 @@ static struct pd_protocol {
 	/* Time for source recovery after hard reset */
 	uint64_t src_recover;
 
+	/* last requested voltage PDO index */
+	int requested_idx;
 #ifdef CONFIG_USB_PD_DUAL_ROLE
 	/* Current limit / voltage based on the last request message */
 	uint32_t curr_limit;
@@ -857,7 +859,8 @@ static void handle_data_request(int port, uint16_t head,
 #endif /* CONFIG_USB_PD_DUAL_ROLE */
 	case PD_DATA_REQUEST:
 		if ((pd[port].power_role == PD_ROLE_SOURCE) && (cnt == 1))
-			if (!pd_request_voltage(payload[0])) {
+			if (!pd_check_requested_voltage(payload[0])) {
+				pd[port].requested_idx = payload[0] >> 28;
 				send_control(port, PD_CTRL_ACCEPT);
 				set_state(port, PD_STATE_SRC_ACCEPTED);
 				return;
@@ -1634,17 +1637,26 @@ void pd_task(void)
 			timeout = 500*MSEC;
 			break;
 		case PD_STATE_SRC_ACCEPTED:
-			/* Accept sent, wait for the end of transition */
+			/* Accept sent, wait for enabling the new voltage */
 			if (pd[port].last_state != pd[port].task_state)
 				set_state_timeout(
 					port,
 					get_time().val +
+					PD_T_SINK_TRANSITION,
+					PD_STATE_SRC_POWERED);
+			break;
+		case PD_STATE_SRC_POWERED:
+			/* Switch to the new requested voltage */
+			if (pd[port].last_state != pd[port].task_state) {
+				pd_transition_voltage(pd[port].requested_idx);
+				set_state_timeout(
+					port,
+					get_time().val +
 					PD_POWER_SUPPLY_TRANSITION_DELAY,
 					PD_STATE_SRC_TRANSITION);
+			}
 			break;
 		case PD_STATE_SRC_TRANSITION:
-			res = pd_set_power_supply_ready(port);
-			/* TODO error fallback */
 			/* the voltage output is good, notify the source */
 			res = send_control(port, PD_CTRL_PS_RDY);
 			if (res >= 0) {
@@ -2458,8 +2470,8 @@ static int command_pd(int argc, char **argv)
 			"SNK_SWAP_COMPLETE",
 #endif /* CONFIG_USB_PD_DUAL_ROLE */
 			"SRC_DISCONNECTED", "SRC_STARTUP", "SRC_DISCOVERY",
-			"SRC_NEGOCIATE", "SRC_ACCEPTED", "SRC_TRANSITION",
-			"SRC_READY", "SRC_DR_SWAP",
+			"SRC_NEGOCIATE", "SRC_ACCEPTED", "SRC_POWERED",
+			"SRC_TRANSITION", "SRC_READY", "SRC_DR_SWAP",
 #ifdef CONFIG_USB_PD_DUAL_ROLE
 			"SRC_SWAP_INIT", "SRC_SWAP_SNK_DISABLE",
 			"SRC_SWAP_SRC_DISABLE", "SRC_SWAP_STANDBY",
diff --git a/include/usb_pd.h b/include/usb_pd.h
index b743324566..7c276f5ed4 100644
--- a/include/usb_pd.h
+++ b/include/usb_pd.h
@@ -537,6 +537,7 @@ enum pd_states {
 	PD_STATE_SRC_DISCOVERY,
 	PD_STATE_SRC_NEGOCIATE,
 	PD_STATE_SRC_ACCEPTED,
+	PD_STATE_SRC_POWERED,
 	PD_STATE_SRC_TRANSITION,
 	PD_STATE_SRC_READY,
 	PD_STATE_SRC_DR_SWAP,
@@ -685,7 +686,14 @@ void pd_set_max_voltage(unsigned mv);
  * @param rdo  Request Data Object with the selected operating point.
  * @return EC_SUCCESS if we can get the requested voltage/OP, <0 else.
  */
-int pd_request_voltage(uint32_t rdo);
+int pd_check_requested_voltage(uint32_t rdo);
+
+/**
+ * Select a new output voltage.
+ *
+ * param idx index of the new voltage in the source PDO table.
+ */
+void pd_transition_voltage(int idx);
 
 /**
  * Go back to the default/safe state of the power supply