bd99955: Improve interrupt / USB charger task wake scheme

Previously our charger ISR called a deferred task which woke our charger
task. We can skip the deferred task and just wake our charger task
directly.

The other meaningful change here is to assume that we're using the
charger for VBUS detection / BC1.2 if we have a usb_chg task, which
holds true for all of our current boards with this charger.

BUG=None
TEST=Manual on kevin with subsequent commit. Verify charger connect /
disconnect detection works properly on both ports, with zinger, donette
and generic DCP charger.
BRANCH=None

Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Change-Id: Iad4f3ea90947b50859c549b591675e325717209f
Reviewed-on: https://chromium-review.googlesource.com/352822
Commit-Ready: Shawn N <shawnn@chromium.org>
Tested-by: Shawn N <shawnn@chromium.org>
Reviewed-by: Vijay P Hiremath <vijay.p.hiremath@intel.com>
Reviewed-by: Mary Ruthven <mruthven@chromium.org>
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

3 files changed, 56 insertions, 83 deletions

diff --git a/common/usb_charger.c b/common/usb_charger.c
index 33754120a1..2cd9cfbcd3 100644
--- a/common/usb_charger.c
+++ b/common/usb_charger.c
@@ -59,14 +59,13 @@ int usb_charger_port_is_sourcing_vbus(int port)
 void usb_charger_vbus_change(int port, int vbus_level)
 {
 	/* If VBUS has transitioned low, notify PD module directly */
-	pd_vbus_low(port);
+	if (!vbus_level)
+		pd_vbus_low(port);
+
 	/* Update VBUS supplier and signal VBUS change to USB_CHG task */
 	update_vbus_supplier(port, vbus_level);
 
-#ifdef HAS_TASK_USB_CHG
-	/* USB Charger task */
-	task_set_event(TASK_ID_USB_CHG, (1 << port), 0);
-#else
+#ifdef HAS_TASK_USB_CHG_P0
 	/* USB Charger task(s) */
 	task_set_event(USB_CHG_PORT_TO_TASK_ID(port), USB_CHG_EVENT_VBUS, 0);
 #endif
diff --git a/driver/charger/bd99955.c b/driver/charger/bd99955.c
index 4e74823ebc..cf3ad64d95 100644
--- a/driver/charger/bd99955.c
+++ b/driver/charger/bd99955.c
@@ -177,6 +177,8 @@ static int bd99955_get_charger_op_status(int *status)
 }
 
 #ifdef HAS_TASK_USB_CHG
+static int bc12_detected_type[CONFIG_USB_PD_PORT_COUNT];
+
 static int bd99955_get_bc12_device_type(enum bd99955_charge_port port)
 {
 	int rv;
@@ -307,7 +309,6 @@ static void bd99955_bc12_detach(int port, int type)
 	pd_send_host_event(PD_EVENT_POWER_CHANGE);
 }
 
-#ifdef CONFIG_USB_PD_VBUS_DETECT_CHARGER
 static int bd99955_enable_vbus_detect_interrupts(int port, int enable)
 {
 	int reg;
@@ -350,7 +351,8 @@ static int bd99955_enable_vbus_detect_interrupts(int port, int enable)
 	return ch_raw_write16(port_reg, reg, BD99955_EXTENDED_COMMAND);
 }
 
-static int bd99955_get_vbus_detect_interrupts(int port, int get)
+/* Read + clear active interrupt bits for a given port */
+static int bd99955_get_vbus_detect_interrupts(int port)
 {
 	int rv;
 	int reg;
@@ -358,44 +360,35 @@ static int bd99955_get_vbus_detect_interrupts(int port, int get)
 
 	port_reg = (port == BD99955_CHARGE_PORT_VBUS) ?
 			BD99955_CMD_INT1_STATUS : BD99955_CMD_INT2_STATUS;
-	if (get) {
-		rv = ch_raw_read16(port_reg, &reg, BD99955_EXTENDED_COMMAND);
 
-		return rv ? 0 : reg &
-			(BD99955_CMD_INT_SET_RES | BD99955_CMD_INT_SET_DET);
-	} else
-		return ch_raw_write16(port_reg,
-			(BD99955_CMD_INT_SET_RES | BD99955_CMD_INT_SET_DET),
-			BD99955_EXTENDED_COMMAND);
-}
-#endif /* CONFIG_USB_PD_VBUS_DETECT_CHARGER */
+	rv = ch_raw_read16(port_reg, &reg, BD99955_EXTENDED_COMMAND);
 
-static void usb_charger_task_init(int *bc12_type)
-{
-	int port;
+	if (rv)
+		return 0;
 
-	for (port = 0; port < CONFIG_USB_PD_PORT_COUNT; port++) {
-		bc12_type[port] = CHARGE_SUPPLIER_NONE;
+	/* Clear the interrupt status bits we just read */
+	ch_raw_write16(port_reg, reg, BD99955_EXTENDED_COMMAND);
 
-#ifdef CONFIG_USB_PD_VBUS_DETECT_CHARGER
-		/* Clear any pending VBUS interrupts */
-		bd99955_get_vbus_detect_interrupts(port, 0);
-#endif
-	}
+	return reg;
 }
 
-static void usb_charger_process(enum bd99955_charge_port port, int *bc12_type)
+static void usb_charger_process(enum bd99955_charge_port port)
 {
-	int vbus_provided = pd_snk_is_vbus_provided(port);
+	int vbus_provided = bd99955_is_vbus_provided(port);
 
+	/* Inform other modules about VBUS level */
+	/* TODO: map charger port num to board port num, they may differ. */
+	usb_charger_vbus_change(port, vbus_provided);
+
+	/* Do BC1.2 detection */
 	if (vbus_provided) {
 		/* Charger/sync attached */
-		bc12_type[port] = bd99955_bc12_detect(port);
-	} else if (bc12_type[port] != CHARGE_SUPPLIER_NONE &&
+		bc12_detected_type[port] = bd99955_bc12_detect(port);
+	} else if (bc12_detected_type[port] != CHARGE_SUPPLIER_NONE &&
 		!vbus_provided) {
 		/* Charger/sync detached */
-		bd99955_bc12_detach(port, bc12_type[port]);
-		bc12_type[port] = CHARGE_SUPPLIER_NONE;
+		bd99955_bc12_detach(port, bc12_detected_type[port]);
+		bc12_detected_type[port] = CHARGE_SUPPLIER_NONE;
 	}
 }
 #endif /* HAS_TASK_USB_CHG */
@@ -661,11 +654,6 @@ static void bd99995_init(void)
 	reg &= ~BD99955_CMD_VM_CTRL_SET_EXTIADPEN;
 	ch_raw_write16(BD99955_CMD_VM_CTRL_SET, reg,
 		       BD99955_EXTENDED_COMMAND);
-
-#if defined(HAS_TASK_USB_CHG) && defined(CONFIG_USB_PD_VBUS_DETECT_CHARGER)
-	bd99955_enable_vbus_detect_interrupts(BD99955_CHARGE_PORT_VBUS, 1);
-	bd99955_enable_vbus_detect_interrupts(BD99955_CHARGE_PORT_VCC, 1);
-#endif
 }
 DECLARE_HOOK(HOOK_INIT, bd99995_init, HOOK_PRIO_INIT_EXTPOWER);
 
@@ -792,54 +780,46 @@ void usb_charger_set_switches(int port, enum usb_switch setting)
 	bd99955_enable_usb_switch(port, usb_switch_state[port]);
 }
 
-#ifdef CONFIG_USB_PD_VBUS_DETECT_CHARGER
-void bd99955_vbus_interrupt_deferred(void)
-{
-	int port;
-	int intr;
-
-	for (port = 0; port < CONFIG_USB_PD_PORT_COUNT; port++) {
-		/* Get the VBUS interrupt */
-		intr = bd99955_get_vbus_detect_interrupts(port, 1);
-		if (!intr)
-			continue;
-
-		if (intr & BD99955_CMD_INT_SET_RES)
-			usb_charger_vbus_change(port, 0);
-
-		if (intr & BD99955_CMD_INT_SET_DET)
-			usb_charger_vbus_change(port, 1);
-
-		/* Clear the VBUS interrupt */
-		bd99955_get_vbus_detect_interrupts(port, 0);
-	}
-}
-DECLARE_DEFERRED(bd99955_vbus_interrupt_deferred);
-
 void bd99955_vbus_interrupt(enum gpio_signal signal)
 {
-	hook_call_deferred(&bd99955_vbus_interrupt_deferred_data, 0);
+	task_wake(TASK_ID_USB_CHG);
 }
-#endif /* CONFIG_USB_PD_VBUS_DETECT_CHARGER */
 
 void usb_charger_task(void)
 {
-	int evt = USB_CHG_EVENT_VBUS_P0 | USB_CHG_EVENT_VBUS_P1;
-	int bc12_type[CONFIG_USB_PD_PORT_COUNT];
+	static int initialized;
+	int changed, port;
 
-	usb_charger_task_init(bc12_type);
+	for (port = 0; port < CONFIG_USB_PD_PORT_COUNT; port++)
+		bc12_detected_type[port] = CHARGE_SUPPLIER_NONE;
 
-	while (1) {
-		if (evt & USB_CHG_EVENT_VBUS_P0)
-			usb_charger_process(BD99955_CHARGE_PORT_VBUS,
-					&bc12_type[BD99955_CHARGE_PORT_VBUS]);
+	bd99955_enable_vbus_detect_interrupts(BD99955_CHARGE_PORT_VBUS, 1);
+	bd99955_enable_vbus_detect_interrupts(BD99955_CHARGE_PORT_VCC, 1);
 
-		if (evt & USB_CHG_EVENT_VBUS_P1)
-			usb_charger_process(BD99955_CHARGE_PORT_VCC,
-					&bc12_type[BD99955_CHARGE_PORT_VCC]);
+	while (1) {
+		changed = 0;
+		for (port = 0; port < CONFIG_USB_PD_PORT_COUNT; port++) {
+			/* Get the VBUS interrupt */
+			if (bd99955_get_vbus_detect_interrupts(port) ||
+			    !initialized) {
+				/* Detect based on current state of VBUS */
+				usb_charger_process(port);
+				changed = 1;
+			}
+		}
+
+		initialized = 1;
 
-		/* Wait for interrupt */
-		evt = task_wait_event(-1);
+		/*
+		 * Re-read interrupt registers immediately if we got an
+		 * interrupt. We're dealing with multiple independent
+		 * interrupt sources and the interrupt pin may have
+		 * never deasserted if both sources were not in clear
+		 * state simultaneously.
+		 */
+		if (!changed)
+			/* Wait for task wake */
+			task_wait_event(-1);
 	}
 }
 #endif /* HAS_TASK_USB_CHG */
diff --git a/include/usb_charge.h b/include/usb_charge.h
index 382f373f8a..34893eabf7 100644
--- a/include/usb_charge.h
+++ b/include/usb_charge.h
@@ -47,13 +47,7 @@ int usb_charge_set_mode(int usb_port_id, enum usb_charge_mode mode);
  */
 int usb_charge_ports_enabled(void);
 
-/* Events handled by the USB_CHG task */
-#ifdef HAS_TASK_USB_CHG
-/* One task for all the ports */
-#define USB_CHG_EVENT_VBUS_P0 TASK_EVENT_CUSTOM(1)
-#define USB_CHG_EVENT_VBUS_P1 TASK_EVENT_CUSTOM(2)
-#elif defined(HAS_TASK_USB_CHG_P0)
-/* One task per port */
+#ifdef HAS_TASK_USB_CHG_P0
 #define USB_CHG_EVENT_BC12 TASK_EVENT_CUSTOM(1)
 #define USB_CHG_EVENT_VBUS TASK_EVENT_CUSTOM(2)
 #define USB_CHG_EVENT_INTR TASK_EVENT_CUSTOM(4)