charge_manager: Minimize log spew

Previously, we tried to minimize log spew by keeping track of previous
log entries and not writing new entries in some cases. Instead, we can
write a log on the following events only:

1. A port becomes active or
2. A port becomes inactive or
3. The active charge port power limit changes or
4. Any supplier change on an inactive port

Also, make charge_manager_save_log a non-static charge manager API
function, so that other modules can record a log, if they have reason to
believe a port has changed outside of a charge manager change.

BUG=chrome-os-partner:33248
TEST=Manual on Samus. Make various power actions and observe logging.
BRANCH=Samus

Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Change-Id: I5d5d3e186e85fdb1c59797ffbfb2f5a6ec04d94d
Reviewed-on: https://chromium-review.googlesource.com/247891
Reviewed-by: Alec Berg <alecaberg@chromium.org>

2 files changed, 33 insertions, 18 deletions

diff --git a/common/charge_manager.c b/common/charge_manager.c
index 7ee86081a6..35a570d2e6 100644
--- a/common/charge_manager.c
+++ b/common/charge_manager.c
@@ -35,6 +35,11 @@ static int charge_ceil[PD_PORT_COUNT];
 /* Dual-role capability of attached partner port */
 static enum dualrole_capabilities dualrole_capability[PD_PORT_COUNT];
 
+#ifdef CONFIG_USB_PD_LOGGING
+/* Mark port as dirty when making changes, for later logging */
+static int save_log[PD_PORT_COUNT];
+#endif
+
 /* Store current state of port enable / charge current. */
 static int charge_port = CHARGE_PORT_NONE;
 static int charge_current = CHARGE_CURRENT_UNINITIALIZED;
@@ -189,14 +194,12 @@ static void charge_manager_fill_power_info(int port,
 /**
  * Saves a power state log entry with the current info about the passed port.
  */
-static void charge_manager_save_log(int port)
+void charge_manager_save_log(int port)
 {
 	uint16_t flags = 0;
 	struct ec_response_usb_pd_power_info pinfo;
-	uint16_t voltage_now;
-	static uint16_t last_voltage[PD_PORT_COUNT];
-	static uint16_t last_flags[PD_PORT_COUNT];
 
+	save_log[port] = 0;
 	charge_manager_fill_power_info(port, &pinfo);
 
 	/* Flags are stored in the data field */
@@ -207,18 +210,6 @@ static void charge_manager_save_log(int port)
 	flags |= pinfo.role | (pinfo.type << CHARGE_FLAGS_TYPE_SHIFT) |
 		 (pinfo.dualrole ? CHARGE_FLAGS_DUAL_ROLE : 0);
 
-	/*
-	 * Check for a log change, not considering timestamp. Also, ignore
-	 * voltage_now fluctuations of < 500mV.
-	 */
-	voltage_now = pinfo.meas.voltage_now;
-	if (last_flags[port] == flags &&
-	    voltage_now < last_voltage[port] + 500 &&
-	    last_voltage[port] < voltage_now + 500)
-		return;
-	last_voltage[port] = voltage_now;
-	last_flags[port] = flags;
-
 	pd_log_event(PD_EVENT_MCU_CHARGE,
 		     PD_LOG_PORT_SIZE(port, sizeof(pinfo.meas)),
 		     flags, &pinfo.meas);
@@ -421,9 +412,25 @@ static void charge_manager_refresh(void)
 	charge_port = new_port;
 
 #ifdef CONFIG_USB_PD_LOGGING
-	/* Log possible charge state changes. */
+	/*
+	 * Write a log under the following conditions:
+	 *  1. A port becomes active or
+	 *  2. A port becomes inactive or
+	 *  3. The active charge port power limit changes or
+	 *  4. Any supplier change on an inactive port
+	 */
+	if (updated_new_port != CHARGE_PORT_NONE)
+		save_log[updated_new_port] = 1;
+	/* Don't log non-meaningful changes on charge port */
+	else if (charge_port != CHARGE_PORT_NONE)
+		save_log[charge_port] = 0;
+
+	if (updated_old_port != CHARGE_PORT_NONE)
+		save_log[updated_old_port] = 1;
+
 	for (i = 0; i < PD_PORT_COUNT; ++i)
-		charge_manager_save_log(i);
+		if (save_log[i])
+			charge_manager_save_log(i);
 #endif
 
 	/* New power requests must be set only after updating the globals. */
@@ -454,6 +461,9 @@ static void charge_manager_make_change(enum charge_manager_change_type change,
 		if (charge->current > 0 &&
 		    available_charge[supplier][port].current == 0)
 			clear_override = 1;
+#ifdef CONFIG_USB_PD_LOGGING
+		save_log[port] = 1;
+#endif
 		break;
 	case CHANGE_DUALROLE:
 		/*
diff --git a/include/charge_manager.h b/include/charge_manager.h
index 0b5c4f7cb4..d761f131fc 100644
--- a/include/charge_manager.h
+++ b/include/charge_manager.h
@@ -46,6 +46,11 @@ int charge_manager_get_override(void);
 /* Returns the current active charge port, as determined by charge manager */
 int charge_manager_get_active_charge_port(void);
 
+#ifdef CONFIG_USB_PD_LOGGING
+/* Save power state log entry for the given port */
+void charge_manager_save_log(int port);
+#endif
+
 /* Board-level callback functions */
 
 /*