ryu: improve inductive charging control

When inductive charging just starts, there might be a blip on
CHARGE_DONE signal and it'd cause our charging control logic to shut
down charging. Fix this by waiting for a second before we start
monitoring CHARGE_DONE.

Also, once we see CHARGE_DONE=1 and disable charging, CHARGE_DONE will
go low. Handle this by ignoring all subsequent CHARGE_DONE change until
the next time the lid is opened.

BRANCH=Ryu
BUG=None
TEST=Pass the updated unit test.
TEST=Charge a base on Ryu P3.

Change-Id: I9d911cd689d8e88ebcd66e6eca7c86dd70704880
Signed-off-by: Vic Yang <victoryang@google.com>
Reviewed-on: https://chromium-review.googlesource.com/243365
Tested-by: Vic Yang <victoryang@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
Commit-Queue: Vic Yang <victoryang@chromium.org>

3 files changed, 95 insertions, 15 deletions

diff --git a/board/ryu/board.h b/board/ryu/board.h
index 66b991a09a..7cdfb65dd1 100644
--- a/board/ryu/board.h
+++ b/board/ryu/board.h
@@ -102,7 +102,7 @@
 
 /* Maximum number of deferrable functions */
 #undef  DEFERRABLE_MAX_COUNT
-#define DEFERRABLE_MAX_COUNT 10
+#define DEFERRABLE_MAX_COUNT 11
 
 #ifndef __ASSEMBLER__
 
diff --git a/common/inductive_charging.c b/common/inductive_charging.c
index 0021cc3ac6..8b92394c8c 100644
--- a/common/inductive_charging.c
+++ b/common/inductive_charging.c
@@ -8,22 +8,74 @@
 #include "common.h"
 #include "gpio.h"
 #include "hooks.h"
+#include "inductive_charging.h"
 #include "lid_switch.h"
 #include "timer.h"
 
+/*
+ * The inductive charger is controlled with two signals:
+ *   - BASE_CHG_VDD_EN controls whether the charger is powered.
+ *   - CHARGE_EN controls whether to enable charging.
+ * Charging status is reported via CHARGE_DONE, but in a tricky way:
+ *   - It's 0 if:
+ *     + The charger is unpowered. (i.e. BASE_CHG_VDD_EN = 0)
+ *     + Or charging is disabled. (i.e. CHARGE_EN = 0)
+ *     + Or the charging current is small enough.
+ *   - Otherwise, it's 1.
+ */
+
+/* Whether we want to process interrupts on CHARGE_DONE or not. */
+static int monitor_charge_done;
+
+/*
+ * Start monitoring CHARGE_DONE and fires the interrupt once so that
+ * we react to the current value.
+ */
+static void inductive_charging_monitor_charge(void)
+{
+	monitor_charge_done = 1;
+	inductive_charging_interrupt(GPIO_CHARGE_DONE);
+}
+DECLARE_DEFERRED(inductive_charging_monitor_charge);
+
 void inductive_charging_interrupt(enum gpio_signal signal)
 {
 	int charger_enabled = gpio_get_level(GPIO_BASE_CHG_VDD_EN);
 	int charge_done = gpio_get_level(GPIO_CHARGE_DONE);
+	static int charge_already_done;
+
+	if (!monitor_charge_done && signal == GPIO_CHARGE_DONE)
+		return;
 
-	/* Always try to charge if the lid is just closed */
-	if (signal == GPIO_LID_OPEN)
+	if (signal == GPIO_LID_OPEN) {
+		/* The lid has been opened. Clear all states. */
 		charge_done = 0;
+		charge_already_done = 0;
+		monitor_charge_done = 0;
+	} else if (signal == GPIO_CHARGE_DONE) {
+		/*
+		 * Once we see CHARGE_DONE=1, we ignore any change on
+		 * CHARGE_DONE until the next time the lid is opened.
+		 */
+		if (charge_done == 1)
+			charge_already_done = 1;
+		else if (charge_already_done)
+			return;
+	}
 
-	if (!charger_enabled || charge_done)
+	if (!charger_enabled || charge_done) {
 		gpio_set_level(GPIO_CHARGE_EN, 0);
-	else
+	} else {
 		gpio_set_level(GPIO_CHARGE_EN, 1);
+		/*
+		 * When the charging is just enabled, there might be a
+		 * blip on CHARGE_DONE. Wait for a second before we start
+		 * looking at CHARGE_DONE.
+		 */
+		if (!monitor_charge_done)
+			hook_call_deferred(inductive_charging_monitor_charge,
+					   SECOND);
+	}
 }
 
 static void inductive_charging_deferred_update(void)
diff --git a/test/inductive_charging.c b/test/inductive_charging.c
index 9e9d1f8486..f3cc6cf908 100644
--- a/test/inductive_charging.c
+++ b/test/inductive_charging.c
@@ -16,7 +16,9 @@
 #include "util.h"
 
 #define START_CHARGE_DELAY 5000 /* ms */
-#define TEST_CHECK_CHARGE_DELAY (START_CHARGE_DELAY + 500) /* ms */
+#define MONITOR_CHARGE_DONE_DELAY 1000 /* ms */
+#define TEST_CHECK_CHARGE_DELAY (START_CHARGE_DELAY + \
+				 MONITOR_CHARGE_DONE_DELAY + 500) /* ms */
 
 static void wait_for_lid_debounce(void)
 {
@@ -38,7 +40,7 @@ static int test_lid(void)
 	TEST_ASSERT(gpio_get_level(GPIO_BASE_CHG_VDD_EN) == 0);
 
 	/*
-	 * Close the lid. The EC should wait for a second before
+	 * Close the lid. The EC should wait for 5 second before
 	 * enabling transmitter.
 	 */
 	set_lid_open(0);
@@ -71,10 +73,16 @@ static int test_charge_done(void)
 	TEST_ASSERT(gpio_get_level(GPIO_BASE_CHG_VDD_EN) == 1);
 	TEST_ASSERT(gpio_get_level(GPIO_CHARGE_EN) == 0);
 
-	/* Oops, need charging again. */
+	/* Oops, CHARGE_DONE changes again. We should ignore it. */
 	gpio_set_level(GPIO_CHARGE_DONE, 0);
 	TEST_ASSERT(gpio_get_level(GPIO_BASE_CHG_VDD_EN) == 1);
-	TEST_ASSERT(gpio_get_level(GPIO_CHARGE_EN) == 1);
+	TEST_ASSERT(gpio_get_level(GPIO_CHARGE_EN) == 0);
+
+	/* Open the lid. Charger should be turned off. */
+	set_lid_open(1);
+	msleep(TEST_CHECK_CHARGE_DELAY);
+	TEST_ASSERT(gpio_get_level(GPIO_BASE_CHG_VDD_EN) == 0);
+	TEST_ASSERT(gpio_get_level(GPIO_CHARGE_EN) == 0);
 
 	return EC_SUCCESS;
 }
@@ -105,27 +113,47 @@ static int test_lid_open_during_charging(void)
 	return EC_SUCCESS;
 }
 
-static int test_clear_charge_done(void)
+static int test_debounce_charge_done(void)
 {
-	/* Lid is open initially. CHARGE_DONE is set. */
+	/* Lid is open initially. */
 	set_lid_open(1);
+	gpio_set_level(GPIO_CHARGE_DONE, 0);
 	msleep(TEST_CHECK_CHARGE_DELAY);
-	gpio_set_level(GPIO_CHARGE_DONE, 1);
 	TEST_ASSERT(gpio_get_level(GPIO_BASE_CHG_VDD_EN) == 0);
 	TEST_ASSERT(gpio_get_level(GPIO_CHARGE_EN) == 0);
 
 	/* Close the lid. Charging should start. */
 	set_lid_open(0);
-	msleep(TEST_CHECK_CHARGE_DELAY);
+	msleep(START_CHARGE_DELAY + 100);
+	TEST_ASSERT(gpio_get_level(GPIO_BASE_CHG_VDD_EN) == 1);
+	TEST_ASSERT(gpio_get_level(GPIO_CHARGE_EN) == 1);
+
+	/* Within the first second, changes on CHARGE_DONE should be ignore. */
+	gpio_set_level(GPIO_CHARGE_DONE, 1);
+	TEST_ASSERT(gpio_get_level(GPIO_BASE_CHG_VDD_EN) == 1);
+	TEST_ASSERT(gpio_get_level(GPIO_CHARGE_EN) == 1);
+	msleep(100);
 	TEST_ASSERT(gpio_get_level(GPIO_BASE_CHG_VDD_EN) == 1);
 	TEST_ASSERT(gpio_get_level(GPIO_CHARGE_EN) == 1);
 	gpio_set_level(GPIO_CHARGE_DONE, 0);
+	TEST_ASSERT(gpio_get_level(GPIO_BASE_CHG_VDD_EN) == 1);
+	TEST_ASSERT(gpio_get_level(GPIO_CHARGE_EN) == 1);
+	msleep(100);
+	TEST_ASSERT(gpio_get_level(GPIO_BASE_CHG_VDD_EN) == 1);
+	TEST_ASSERT(gpio_get_level(GPIO_CHARGE_EN) == 1);
 
-	/* Charge is done. */
+	/* Changes on CHARGE_DONE after take effect. */
+	msleep(MONITOR_CHARGE_DONE_DELAY);
 	gpio_set_level(GPIO_CHARGE_DONE, 1);
 	TEST_ASSERT(gpio_get_level(GPIO_BASE_CHG_VDD_EN) == 1);
 	TEST_ASSERT(gpio_get_level(GPIO_CHARGE_EN) == 0);
 
+	/* Open the lid. Charger should be turned off. */
+	set_lid_open(1);
+	msleep(TEST_CHECK_CHARGE_DELAY);
+	TEST_ASSERT(gpio_get_level(GPIO_BASE_CHG_VDD_EN) == 0);
+	TEST_ASSERT(gpio_get_level(GPIO_CHARGE_EN) == 0);
+
 	return EC_SUCCESS;
 }
 
@@ -136,7 +164,7 @@ void run_test(void)
 	RUN_TEST(test_lid);
 	RUN_TEST(test_charge_done);
 	RUN_TEST(test_lid_open_during_charging);
-	RUN_TEST(test_clear_charge_done);
+	RUN_TEST(test_debounce_charge_done);
 
 	test_print_result();
 }