mkbp_event: Always mark MKBP interrupt as inactive before setting it again

This patch fixes issue with sensors data not flowing to the AP
after ChromeOS reboot.

Issue occurs when ChromeOS doesn't disable sensor FIFO interrupt.
During ChromeOS reboot, EC sends MKBP event to host, but event can't be
served. Hook task calls force_mkbp_if_events(), scheduled by
activate_mkbp_with_events(). Called function sets state.interrupt to
INTERRUPT_INACTIVE and calls active_mkbp_with_events() to set MKBP
interrupt again (again force_mkbp_if_events() is scheduled). This is
repeated 2 times only (limited by state.failed_attempts).

After 3 attempts to generate MKBP interrupt force_mkbp_if_events()
function is not changing state.interrupt to INTERRUPT_INACTIVE. As a
result calling activate_mkbp_with_events() doesn't set MKBP interrupt
and doesn't schedule force_mkbp_if_events() to be called by hook task.

When ChromeOS is running, EC is not able to send MKBP interrupt again,
because it is blocked by INTERRUPT_ACTIVE state. The only way to change
interrupt state is call EC_CMD_GET_NEXT_EVENT host command which will
eventually call set_inactive_if_no_events(). This host command is called
by host only when it receives MKBP interrupt.

This patch fixes vicious circle by setting state.interrupt to
INTERRUPT_INACTIVE in force_mkbp_if_events() always and calling
activate_mkbp_with_events() only when failed_attempts counter doesn't
exceed limit.

It is safe to mark interrupt state as INACTIVE, because
force_mkbp_with_events() function can be only scheduled by
activate_mkbp_with_event() which will set interrupt state to ACTIVE (if
there are still events waiting). Furthermore, failed_attempts counter is
only reset when AP actually pulls all of the events or number of
attempts is exceeded.

BUG=b:162254118
BRANCH=none
TEST=Flash EC ToT on eve. Boot ChromeOS then reboot it.
     Switch to developer console and check if sensors
     works by calling following command:
     'local_test_runner -waituntilready false hardware.SensorRing'

Signed-off-by: Patryk Duda <pdk@semihalf.com>
Change-Id: Ic6acf44770aba8d5d00f070de256751128171205
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2527171
Reviewed-by: Jett Rink <jettrink@chromium.org>

1 files changed, 47 insertions, 3 deletions

diff --git a/common/mkbp_event.c b/common/mkbp_event.c
index a5fe5635c6..800a95fc16 100644
--- a/common/mkbp_event.c
+++ b/common/mkbp_event.c
@@ -264,12 +264,55 @@ static void activate_mkbp_with_events(uint32_t events_to_add)
 static void force_mkbp_if_events(void)
 {
 	int toggled = 0;
+	int send_mkbp_interrupt = 0;
 
 	mutex_lock(&state.lock);
-	if (state.interrupt == INTERRUPT_ACTIVE) {
+	if (state.interrupt == INTERRUPT_INACTIVE) {
+		/*
+		 * When this function is called with state of interrupt set
+		 * to INACTIVE, it means that EC failed to send MKBP interrupt
+		 * to AP. In this case we are going to send interrupt once
+		 * again (without limits).
+		 */
+		send_mkbp_interrupt = 1;
+	} else if (state.interrupt == INTERRUPT_ACTIVE) {
+		/*
+		 * When this function is called with state of interrupt set
+		 * to ACTIVE, it means that AP failed to respond.
+		 *
+		 * It is safe to mark interrupt state as INACTIVE, because
+		 * force_mkbp_with_events() function can be only scheduled by
+		 * activate_mkbp_with_event() which will set interrupt state
+		 * to ACTIVE (and allow to increment failed_attempts counter).
+		 * After 3 attempts, we are setting interrupt state to INACTIVE
+		 * but we are not going to call activate_mkbp_with_events().
+		 * This was meant to unblock MKBP interrupt mechanism for new
+		 * events.
+		 */
+		state.interrupt = INTERRUPT_INACTIVE;
+		/*
+		 * Failed attempts counter is cleared only when AP pulls all
+		 * of events or we exceed number of attempts, so marking
+		 * interrupt as INACTIVE doesn't affect failed_attempts counter.
+		 * If we need to send interrupt once again
+		 * activate_mkbp_with_events() will set interrupt state to ACTIVE
+		 * before this function will be called.
+		 */
 		if (++state.failed_attempts < 3) {
-			state.interrupt = INTERRUPT_INACTIVE;
+			send_mkbp_interrupt = 1;
 			toggled = 1;
+		} else {
+			/*
+			 * If we exceed maximum number of failed attempts we
+			 * will stop trying to send MKBP interrupt for current
+			 * event (send_mkbp_interrupt == 0), but leaving
+			 * possibility to send MKBP interrupts for future
+			 * events (state of interrupt makred as inactive).
+			 * Future events should have a chance to be sent
+			 * 3 times, so we should clear failed attempts
+			 * counter now
+			 */
+			state.failed_attempts = 0;
 		}
 	}
 	mutex_unlock(&state.lock);
@@ -277,7 +320,8 @@ static void force_mkbp_if_events(void)
 	if (toggled)
 		CPRINTS("MKBP not cleared within threshold, toggling.");
 
-	activate_mkbp_with_events(0);
+	if (send_mkbp_interrupt)
+		activate_mkbp_with_events(0);
 }
 
 test_mockable int mkbp_send_event(uint8_t event_type)