nissa: process shared BC1.2 interrupts alongside other PD interrupts

BC1.2 runs at a low priority and can cause the same kinds of priority
inversion as the charger can. On devices using PS8745 in particular,
we've found that BC1.2 interrupts can block timely processing of TCPC
interrupts which results in the TCPM violating the TRANSMIT register
requirements in TCPCI specification section 4.4.15 where the TCPM "shall
clear the resulting alert from a prior TRANSMIT write before writing to
the TRANSMIT register again" with some particular communication patterns,
causing an I2CInterfaceError on the PS8745 that is never cleared (which
causes the affected port to stop responding completely).

Running BC1.2 interrupts synchronously and polling the shared IRQ GPIO
to retrigger PD interrupts ensures that PD interrupts will always be
handled at a higher priority than regular PD communication, preventing
this problematic delayed interrupt processing.

BUG=b:269064073
TEST=The USB-C1 interrupt on Yaviks no longer gets stuck asserted when
     connected to an HP U28 or M27 monitor; charging still works as
     expected on Craask for both ports.
BRANCH=nissa

Change-Id: I2d573fcbe3b471b9f93a98fc3b401141c26daf7d
Signed-off-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4335462
Reviewed-by: Kangheui Won <khwon@chromium.org>

1 files changed, 18 insertions, 57 deletions

diff --git a/zephyr/program/nissa/nereid/src/usbc.c b/zephyr/program/nissa/nereid/src/usbc.c
index 8c50afeccb..3ce489e357 100644
--- a/zephyr/program/nissa/nereid/src/usbc.c
+++ b/zephyr/program/nissa/nereid/src/usbc.c
@@ -282,69 +282,21 @@ void usb_c0_interrupt(enum gpio_signal s)
 }
 
 /* C1 interrupt line shared by BC 1.2, TCPC, and charger */
-static void check_c1_line(void);
-DECLARE_DEFERRED(check_c1_line);
-
-static void notify_c1_chips(void)
-{
-	schedule_deferred_pd_interrupt(1);
-	usb_charger_task_set_event(1, USB_CHG_EVENT_BC12);
-	/* Charger is handled in board_process_pd_alert */
-}
-
-static void check_c1_line(void)
-{
-	/*
-	 * If line is still being held low, see if there's more to process from
-	 * one of the chips.
-	 */
-	if (!gpio_pin_get_dt(GPIO_DT_FROM_ALIAS(gpio_usb_c1_int_odl))) {
-		notify_c1_chips();
-		hook_call_deferred(&check_c1_line_data, INT_RECHECK_US);
-	}
-}
-
 void usb_c1_interrupt(enum gpio_signal s)
 {
-	/* Cancel any previous calls to check the interrupt line */
-	hook_call_deferred(&check_c1_line_data, -1);
-
-	/* Notify all chips using this line that an interrupt came in */
-	notify_c1_chips();
-
-	/* Check the line again in 5ms */
-	hook_call_deferred(&check_c1_line_data, INT_RECHECK_US);
-}
-
-/*
- * Check state of IRQ lines at startup, ensuring an IRQ that happened before
- * the EC started up won't get lost (leaving the IRQ line asserted and blocking
- * any further interrupts on the port).
- *
- * Although the PD task will check for pending TCPC interrupts on startup,
- * the charger sharing the IRQ will not be polled automatically.
- */
-void board_handle_initial_typec_irq(void)
-{
-	check_c0_line();
-	if (board_get_usb_pd_port_count() == 2)
-		check_c1_line();
+	/* Charger and BC1.2 are handled in board_process_pd_alert */
+	schedule_deferred_pd_interrupt(1);
 }
-/*
- * This must run after sub-board detection (which happens in EC main()),
- * but isn't depended on by anything else either.
- */
-DECLARE_HOOK(HOOK_INIT, board_handle_initial_typec_irq, HOOK_PRIO_LAST);
 
 /*
  * Handle charger interrupts in the PD task. Not doing so can lead to a priority
  * inversion where we fail to respond to TCPC alerts quickly enough because we
- * don't get another edge on a shared IRQ until the charger interrupt is cleared
- * (or the IRQ is polled again), which happens in the low-priority charger task:
- * the high-priority type-C handler is thus blocked on the lower-priority
- * charger.
+ * don't get another edge on a shared IRQ until the other interrupt is cleared
+ * (or the IRQ is polled again), which happens in lower-priority tasks: the
+ * high-priority type-C handler is thus blocked on the lower-priority one(s).
  *
- * To avoid that, we run charger interrupts at the same priority.
+ * To avoid that, we run charger and BC1.2 interrupts synchronously alongside
+ * PD interrupts so they have the same priority.
  */
 void board_process_pd_alert(int port)
 {
@@ -352,10 +304,19 @@ void board_process_pd_alert(int port)
 	 * Port 0 doesn't use an external TCPC, so its interrupts don't need
 	 * this special handling.
 	 */
-	if (port == 1 &&
-	    !gpio_pin_get_dt(GPIO_DT_FROM_ALIAS(gpio_usb_c1_int_odl))) {
+	if (port != 1)
+		return;
+
+	if (!gpio_pin_get_dt(GPIO_DT_FROM_ALIAS(gpio_usb_c1_int_odl))) {
 		sm5803_handle_interrupt(port);
+		usb_charger_task_set_event_sync(1, USB_CHG_EVENT_BC12);
 	}
+	/*
+	 * Immediately schedule another TCPC interrupt if it seems we haven't
+	 * cleared all pending interrupts.
+	 */
+	if (!gpio_pin_get_dt(GPIO_DT_FROM_ALIAS(gpio_usb_c1_int_odl)))
+		schedule_deferred_pd_interrupt(port);
 }
 
 int pd_snk_is_vbus_provided(int port)