fwk: dogwood: 5vsb entering/exit improvements

Lowered trip points so system does not brown out.

Added hysteris (0.3A delta) so any noise will have a harder time
dancing around the trip points.

Even with hysteris, added a timeout so it makes sure the load
doesn't return soon. Timeout increases exponentially so it gets more
and more quieter.

Improve debugging so all of the above can be tested better.

BUG=Nirav's system
BUG=https://app.clickup.com/t/86etqaz6v
BUG=https://app.clickup.com/t/86etqb6qx
TEST=Make Windows go to sleep, no more annoying clicking
TEST=With a 5V USB load, try to find the trip point, there is not
a ton of clicking anymore due to the hysterisis and timeout
TEST=System does not brown out due to lower trip points
TEST=While doing above, look at the console, there should be nicer logs
BRANCH=fwk-dogwood-27111

Signed-off-by: Josh Tsai <Josh_Tsai@compal.com>

Author: Josh-Tsai

zephyr/program/framework/dogwood/src/power_monitor.c

@@ -23,9 +23,6 @@
 #define INA236_ADC_RANGE	2500 /* 2500 nV */
 #define INA236_ALERT_LIMIT(x) ((x * INA236_SHUNT_RESISTOR) * 1000 / INA236_ADC_RANGE)
 
-#define INA236_MONITOR_12V_CURRENT 30000 /* 30 A */
-#define INA236_MONITOR_5V_CURRENT  2400  /* 2.4 A */
-
 /**
  * We measured the timing between EC turns on the ps_on and recevies the pok_l signal.
  * The average timing is 400 ~ 420 ms.
@@ -89,7 +86,7 @@ static void power_monitor_update_ina236(int idx)
 	if (idx == INA236_IDX_PSU_12V)
 		current = INA236_MONITOR_12V_CURRENT;
 	else if (idx == INA236_IDX_PSU_5V)
-		current = INA236_MONITOR_5V_CURRENT;
+		current = INA236_MONITOR_5V_UPPER_CURRENT_MA;
 	else {
 		CPRINTS("%s gets invalid index: %d", __func__, idx);
 		return;
@@ -100,17 +97,24 @@ static void power_monitor_update_ina236(int idx)
 	if (rv != EC_SUCCESS)
 		CPRINTS("INA236 index %d write config fail", idx);
 
-	rv = ina2xx_set_alert(idx, INA236_ALERT_LIMIT(current));
-
-	if (rv != EC_SUCCESS)
-		CPRINTS("INA236 index %d write alert fail", idx);
+	power_monitor_set_alert_current(idx, current);
 
 	rv = ina2xx_set_mask(idx, INA2XX_MASK_EN_SOL);
 
 	if (rv != EC_SUCCESS)
 		CPRINTS("INA236 index %d mask fail", idx);
 }
 
+void power_monitor_set_alert_current(int idx, int ma)
+{
+	int rv;
+
+	rv = ina2xx_set_alert(idx, INA236_ALERT_LIMIT(ma));
+
+	if (rv != EC_SUCCESS)
+		CPRINTS("INA236 index %d write alert fail", idx);
+}
+
 void power_monitor_init(void)
 {
 	int index;
@@ -143,6 +147,8 @@ static void power_monitor_resume(void)
 		/* Clear the alert status before disabling the interrupt */
 		power_monitor_set_5vsb_alert(false);
 		power_monitor_disable_interrupt(INA236_IDX_PSU_5V);
+		power_monitor_set_alert_current(INA236_IDX_PSU_5V,
+				INA236_MONITOR_5V_UPPER_CURRENT_MA);
 	}
 }
 DECLARE_HOOK(HOOK_CHIPSET_RESUME, power_monitor_resume, HOOK_PRIO_DEFAULT);

zephyr/program/framework/dogwood/src/power_sequence.c

@@ -44,6 +44,19 @@
  */
 #define TIMEOUT_VS_POWER_TURN_OFF (20 * MSEC)
 
+/*
+ * Time to wait until turning off the power supply in low power
+ * usage (like suspend).
+ *
+ * _MIN start at this timeout,
+ * After every low-power entry we'll *= by _INCREASE_FACTOR.
+ * for a max of _MAX
+ */
+#define TIMEOUT_5VSB_MIN (3 * MINUTE)
+#define TIMEOUT_5VSB_INCREASE_FACTOR 2
+#define TIMEOUT_5VSB_MAX (30 * MINUTE)
+static int timeout_5vsb = TIMEOUT_5VSB_MIN;
+
 static bool power_s5_up;		/* Chipset is sequencing up or down */
 static int force_shutdown_flags;
 static int d3cold_is_entry;	/* check the d3cold status */
@@ -62,6 +75,7 @@ static bool power_enable_psu(bool enable)
 	gpio_pin_set_dt(GPIO_DT_FROM_NODELABEL(gpio_ps_on), enable);
 
 	if (enable) {
+		CPRINTS("PS_ON was sent, waiting for POK");
 		/**
 		 * Wait for the PSU power good.
 		 * According to the waveform, EC needs to delay 500ms to
@@ -355,7 +369,8 @@ void power_5vsb_enter(void)
 	if (force_enable_psu)
 		return;
 
-	CPRINTS("power 5vsb enter");
+	CPRINTS("current was low (<%dmA) for a long time, switching to 5vsb",
+			INA236_MONITOR_5V_LOWER_CURRENT_MA);
 
 	gpio_pin_set_dt(GPIO_DT_FROM_NODELABEL(gpio_en_s0ix), 1);
 
@@ -364,21 +379,40 @@ void power_5vsb_enter(void)
 
 	k_msleep(10);
 	power_enable_psu(0);
+
+	/* Set the upper current limit */
+	power_monitor_set_alert_current(INA236_IDX_PSU_5V,
+			INA236_MONITOR_5V_UPPER_CURRENT_MA);
 }
 
+static void power_5vsb_enter_deferred(void)
+{
+	power_5vsb_enter();
+	timeout_5vsb *= TIMEOUT_5VSB_INCREASE_FACTOR;
+	if (timeout_5vsb > TIMEOUT_5VSB_MAX)
+		timeout_5vsb = TIMEOUT_5VSB_MAX;
+}
+DECLARE_DEFERRED(power_5vsb_enter_deferred);
+
 bool power_5vsb_exit(void)
 {
-	CPRINTS("power 5vsb exit");
+	CPRINTS("5vsb is not enough (>%dmA), turning on PSU",
+			INA236_MONITOR_5V_UPPER_CURRENT_MA);
 
 	if (!power_enable_psu(1))
 		return false;
 
 	k_msleep(10);
 	gpio_pin_set_dt(GPIO_DT_FROM_NODELABEL(gpio_en_rvsp_l), 1);
+	CPRINTS("switched to high power rails");
 
 	k_msleep(10);
 	gpio_pin_set_dt(GPIO_DT_FROM_NODELABEL(gpio_en_s0ix), 0);
 
+	/* Set the lower current limit */
+	power_monitor_set_alert_current(INA236_IDX_PSU_5V,
+			INA236_MONITOR_5V_LOWER_CURRENT_MA);
+
 	return true;
 }
 
@@ -428,7 +462,6 @@ static void diagnostic_power_glitch_detect(void)
 enum power_state power_handle_state(enum power_state state)
 {
 	int s5_exit_tries = 0;	/* For global reset to wait SLP_S5 signal de-asserts */
-
 	if (run_diagnostics == 1)
 		diagnostic_power_glitch_detect();
 
@@ -570,16 +603,22 @@ enum power_state power_handle_state(enum power_state state)
 				!gpio_pin_get_dt(GPIO_DT_FROM_NODELABEL(gpio_en_s0ix));
 			bool has_alert = power_monitor_get_5vsb_alert();
 
-			if (has_alert && !has_exited_5vsb) {
-				if (!power_5vsb_exit()) {
+			if (has_alert) {
+				/* clear the enter 5VSB timer if over upper current */
+				hook_call_deferred(&power_5vsb_enter_deferred_data, -1);
+				if (!has_exited_5vsb && !power_5vsb_exit()) {
 					resume_ms_flag = 0;
 					enter_ms_flag = 0;
 					system_in_s0ix = 0;
 					chipset_force_shutdown(CHIPSET_SHUTDOWN_POWERFAIL);
 					return POWER_S3S5;
 				}
-			} else if (!has_alert && has_exited_5vsb)
-				power_5vsb_enter();
+			} else if (!has_alert && has_exited_5vsb) {
+				hook_call_deferred(&power_5vsb_enter_deferred_data, timeout_5vsb);
+				CPRINTS("5V current under %dmA, don't need power supply, but waiting another %d min",
+						INA236_MONITOR_5V_LOWER_CURRENT_MA,
+						timeout_5vsb / MINUTE);
+			}
 		}
 
 		break;
@@ -635,6 +674,8 @@ enum power_state power_handle_state(enum power_state state)
 		return POWER_S0;
 
 	case POWER_S0:
+		/* Reset the timeout so we're power efficient entering suspend. */
+		timeout_5vsb = TIMEOUT_5VSB_MIN;
 
 		if (gpio_pin_get_dt(GPIO_DT_FROM_NODELABEL(gpio_slp_s3_l)) == 0 ||
 			force_shutdown_flags) {
@@ -702,13 +743,7 @@ enum power_state power_handle_state(enum power_state state)
 				return POWER_S0;
 			}
 
-			/**
-			 * Don't convert the 5VALW to 5VSB if the 5V current is over 2.4A.
-			 */
-			if (!power_monitor_get_5vsb_alert()) {
-				k_msleep(10);
-				power_5vsb_enter();
-			}
+			hook_call_deferred(&power_5vsb_enter_deferred_data, timeout_5vsb);
 		}
 
 		return POWER_S3;
@@ -717,8 +752,12 @@ enum power_state power_handle_state(enum power_state state)
 		/* Enable power for CPU check system */
 		k_msleep(10);
 		if (board_get_version() >= BOARD_VERSION_8) {
+			bool has_exited_5vsb =
+				!gpio_pin_get_dt(GPIO_DT_FROM_NODELABEL(gpio_en_s0ix));
 
-			if (!power_5vsb_exit()) {
+			/* clear the enter 5VSB timer if resume to s0ix */
+			hook_call_deferred(&power_5vsb_enter_deferred_data, -1);
+			if (!has_exited_5vsb && !power_5vsb_exit()) {
 				resume_ms_flag = 0;
 				enter_ms_flag = 0;
 				system_in_s0ix = 0;
@@ -776,7 +815,6 @@ enum power_state power_handle_state(enum power_state state)
 
 		lpc_s0ix_resume_restore_masks();
 		hook_notify(HOOK_CHIPSET_RESUME);
-
 		return POWER_S0;
 
 		break;

zephyr/program/framework/include/dogwood/power_monitor.h

@@ -9,11 +9,23 @@
 #define INA236_IDX_PSU_12V INA236_INDEX_ADD_PIN_GND
 #define INA236_IDX_PSU_5V  INA236_INDEX_ADD_PIN_VS
 
+#define INA236_MONITOR_12V_CURRENT 30000 /* 30 A */
+#define INA236_MONITOR_5V_UPPER_CURRENT_MA  1000
+#define INA236_MONITOR_5V_LOWER_CURRENT_MA   700
+
 /**
  * Return the alert flag
  *
  * @return true when the EC detects the alert
  */
 bool power_monitor_get_5vsb_alert(void);
 
+/**
+ * Set the alert current
+ *
+ * @param idx the index of the ina236
+ * @param current set the alert current
+ */
+void power_monitor_set_alert_current(int idx, int ma);
+
 #endif /* __CROS_EC_POWER_MONITOR_H */