SN5S330: treat interrupts as level-sensitive

The SN5S330 PPC will pull its /INT pin low until all interrupts are
cleared.  Since the interrupt pin is treated as edge-sensitive, its
handler needs to provide level-checking before exiting.  Otherwise, if
not all interrupts are cleared before the handler exits, the EC won't
see another edge to call the handler again.

Boards which share the PPC interrupt pin with other sources may choose
to implement their own callback, if they are able to determine which
chip was the source of the interrupt.

BUG=b:118846062
BRANCH=None
TEST=performed several power swaps and unplugs on a pair of Careenas,
verifying that in instances where the handler had to loop around we
correctly cleared the interrupts and the "ectool usbpdpower" output was
normal

Change-Id: Iccbe40976a746d109d67b9a91f8fbd81898f9b3f
Signed-off-by: Diana Z <dzigterman@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1327123
Reviewed-by: Scott Collyer <scollyer@chromium.org>
Reviewed-by: Edward Hill <ecgh@chromium.org>
Reviewed-by: Jett Rink <jettrink@chromium.org>

8 files changed, 79 insertions, 22 deletions

diff --git a/baseboard/grunt/baseboard.c b/baseboard/grunt/baseboard.c
index ba8b18f104..8591501303 100644
--- a/baseboard/grunt/baseboard.c
+++ b/baseboard/grunt/baseboard.c
@@ -147,6 +147,15 @@ struct ppc_config_t ppc_chips[] = {
 };
 unsigned int ppc_cnt = ARRAY_SIZE(ppc_chips);
 
+int ppc_get_alert_status(int port)
+{
+	if (port == 0)
+		return gpio_get_level(GPIO_USB_C0_SWCTL_INT_ODL) == 0;
+	else
+		return gpio_get_level(GPIO_USB_C1_SWCTL_INT_ODL) == 0;
+}
+
+
 /* BC 1.2 chip Configuration */
 const struct max14637_config_t max14637_config[CONFIG_USB_PD_PORT_COUNT] = {
 	[USB_PD_PORT_ANX74XX] = {
diff --git a/baseboard/grunt/baseboard.h b/baseboard/grunt/baseboard.h
index f0df8796ff..98c2163576 100644
--- a/baseboard/grunt/baseboard.h
+++ b/baseboard/grunt/baseboard.h
@@ -120,6 +120,7 @@
 #define CONFIG_USB_PD_TRY_SRC
 #define CONFIG_USB_PD_VBUS_DETECT_PPC
 #define CONFIG_USBC_PPC_SN5S330
+#define CONFIG_USBC_PPC_DEDICATED_INT
 #define CONFIG_USBC_SS_MUX
 #define CONFIG_USBC_SS_MUX_DFP_ONLY
 #define CONFIG_USBC_VCONN
diff --git a/baseboard/octopus/baseboard.h b/baseboard/octopus/baseboard.h
index f582fe77f5..af34b348a0 100644
--- a/baseboard/octopus/baseboard.h
+++ b/baseboard/octopus/baseboard.h
@@ -176,6 +176,7 @@
 	#define CONFIG_USB_PD_TCPM_PS8751	/* C1 Mux: PS8751 */
 	#define CONFIG_USBC_PPC_SN5S330		/* C0 & C1 PPC: each SN5S330 */
 	#define CONFIG_USBC_PPC_VCONN
+	#define CONFIG_USBC_PPC_DEDICATED_INT
 #else
 	#error Must define a VARIANT_OCTOPUS_USBC
 #endif /* VARIANT_OCTOPUS_USBC */
diff --git a/board/ampton/board.c b/board/ampton/board.c
index 63e124fa79..9282df416f 100644
--- a/board/ampton/board.c
+++ b/board/ampton/board.c
@@ -49,6 +49,14 @@ static void ppc_interrupt(enum gpio_signal signal)
 		sn5s330_interrupt(1);
 }
 
+int ppc_get_alert_status(int port)
+{
+	if (port == 0)
+		return gpio_get_level(GPIO_USB_C0_PD_INT_ODL) == 0;
+	else
+		return gpio_get_level(GPIO_USB_C1_PD_INT_ODL) == 0;
+}
+
 #include "gpio_list.h" /* Must come after other header files. */
 
 /******************************************************************************/
diff --git a/board/bip/board.c b/board/bip/board.c
index f82d0c41d9..318a5bc178 100644
--- a/board/bip/board.c
+++ b/board/bip/board.c
@@ -42,6 +42,14 @@ static void ppc_interrupt(enum gpio_signal signal)
 		sn5s330_interrupt(1);
 }
 
+int ppc_get_alert_status(int port)
+{
+	if (port == 0)
+		return gpio_get_level(GPIO_USB_C0_PD_INT_ODL) == 0;
+	else
+		return gpio_get_level(GPIO_USB_C1_PD_INT_ODL) == 0;
+}
+
 #include "gpio_list.h" /* Must come after other header files. */
 
 /******************************************************************************/
diff --git a/driver/ppc/sn5s330.c b/driver/ppc/sn5s330.c
index 66ffa2678b..b90605a3e1 100644
--- a/driver/ppc/sn5s330.c
+++ b/driver/ppc/sn5s330.c
@@ -614,37 +614,56 @@ static int sn5s330_vbus_source_enable(int port, int enable)
 
 static void sn5s330_handle_interrupt(int port)
 {
-	int rise = 0;
-	int fall = 0;
+	int attempt = 0;
 
 	/*
-	 * The only interrupts that should be enabled are the PP1 overcurrent
-	 * condition, and for VBUS_GOOD if PPC is being used to detect VBUS.
+	 * SN5S330's /INT pin is level, so process interrupts until it
+	 * deasserts if the chip has a dedicated interrupt pin.
 	 */
-	read_reg(port, SN5S330_INT_TRIP_RISE_REG1, &rise);
-	read_reg(port, SN5S330_INT_TRIP_FALL_REG1, &fall);
+#ifdef CONFIG_USBC_PPC_DEDICATED_INT
+	while (ppc_get_alert_status(port))
+#endif
+	{
+		int rise = 0;
+		int fall = 0;
 
-	/* Let the board know about the overcurrent event. */
-	if (rise & SN5S330_ILIM_PP1_MASK)
-		board_overcurrent_event(port);
+		attempt++;
 
-	/* Clear the interrupt sources. */
-	write_reg(port, SN5S330_INT_TRIP_RISE_REG1, rise);
-	write_reg(port, SN5S330_INT_TRIP_FALL_REG1, fall);
+		if (attempt > 1)
+			CPRINTS("ppc p%d: Could not clear interrupts on first "
+				"try, retrying", port);
 
-#if defined(CONFIG_USB_PD_VBUS_DETECT_PPC) && defined(CONFIG_USB_CHARGER)
-	read_reg(port, SN5S330_INT_TRIP_RISE_REG3, &rise);
-	read_reg(port, SN5S330_INT_TRIP_FALL_REG3, &fall);
+		/*
+		 * The only interrupts that should be enabled are the PP1
+		 * overcurrent condition, and for VBUS_GOOD if PPC is being
+		 * used to detect VBUS.
+		 */
+		read_reg(port, SN5S330_INT_TRIP_RISE_REG1, &rise);
+		read_reg(port, SN5S330_INT_TRIP_FALL_REG1, &fall);
 
-	/* Inform other modules about VBUS level */
-	if (rise & SN5S330_VBUS_GOOD_MASK
-	    || fall & SN5S330_VBUS_GOOD_MASK)
-		usb_charger_vbus_change(port, sn5s330_is_vbus_present(port));
+		/* Let the board know about the overcurrent event. */
+		if (rise & SN5S330_ILIM_PP1_MASK)
+			board_overcurrent_event(port);
 
-	/* Clear the interrupt sources. */
-	write_reg(port, SN5S330_INT_TRIP_RISE_REG3, rise);
-	write_reg(port, SN5S330_INT_TRIP_FALL_REG3, fall);
+		/* Clear the interrupt sources. */
+		write_reg(port, SN5S330_INT_TRIP_RISE_REG1, rise);
+		write_reg(port, SN5S330_INT_TRIP_FALL_REG1, fall);
+
+#if defined(CONFIG_USB_PD_VBUS_DETECT_PPC) && defined(CONFIG_USB_CHARGER)
+		read_reg(port, SN5S330_INT_TRIP_RISE_REG3, &rise);
+		read_reg(port, SN5S330_INT_TRIP_FALL_REG3, &fall);
+
+		/* Inform other modules about VBUS level */
+		if (rise & SN5S330_VBUS_GOOD_MASK
+		    || fall & SN5S330_VBUS_GOOD_MASK)
+			usb_charger_vbus_change(port,
+						sn5s330_is_vbus_present(port));
+
+		/* Clear the interrupt sources. */
+		write_reg(port, SN5S330_INT_TRIP_RISE_REG3, rise);
+		write_reg(port, SN5S330_INT_TRIP_FALL_REG3, fall);
 #endif  /* CONFIG_USB_PD_VBUS_DETECT_PPC && CONFIG_USB_CHARGER */
+	}
 }
 
 static void sn5s330_irq_deferred(void)
diff --git a/include/config.h b/include/config.h
index 5e9533e492..6f283416e4 100644
--- a/include/config.h
+++ b/include/config.h
@@ -3338,6 +3338,9 @@
 /* PPC is capable of providing VCONN */
 #undef CONFIG_USBC_PPC_VCONN
 
+/* PPC has level interrupts and has a dedicated interrupt pin to check */
+#undef CONFIG_USBC_PPC_DEDICATED_INT
+
 /* Support for USB type-c superspeed mux */
 #undef CONFIG_USBC_SS_MUX
 
diff --git a/include/usbc_ppc.h b/include/usbc_ppc.h
index a93f7e7109..06c2ff3f2f 100644
--- a/include/usbc_ppc.h
+++ b/include/usbc_ppc.h
@@ -222,4 +222,12 @@ void board_overcurrent_event(int port);
  */
 int ppc_enter_low_power_mode(int port);
 
+/**
+ * Board specific callback to check if the PPC interrupt is still asserted
+ *
+ * @param port: The Type-C port number to check
+ * @return 0 if interrupt is cleared, 1 if it is still on
+ */
+int ppc_get_alert_status(int port);
+
 #endif /* !defined(__CROS_EC_USBC_PPC_H) */