charger/isl923x: Implement charger_get_system_power from PSYS

On ISL923x, PSYS output is always enabled when the AP is on
(provided CONFIG_CHARGER_PSYS is enabled).

We add support for charger_get_system_power function, reading PSYS
value, when CONFIG_CHARGER_PSYS_READ is defined. This will be used
by the charging algorithm on lux.

We also rename CONFIG_CMD_CHARGER_PSYS to CONFIG_CHARGER_PSYS_READ
as CONFIG_CHARGER_PSYS_READ provides both "psys" console command
and the new function. We also cleanup unneeded undefs in board
files.

Note that this does not implement the function on bd9995x, but this
could be done without too much effort.

BRANCH=none
BUG=b:71520677
TEST=On lux, without AC connected, check that "psys" output roughly
     matches the output current from the battery.

Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>

Change-Id: Ie1ce8e0ac103daacc5a08b8ccae604d1d83551b8
Reviewed-on: https://chromium-review.googlesource.com/848487
Commit-Ready: Nicolas Boichat <drinkcat@chromium.org>
Tested-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>

12 files changed, 67 insertions, 52 deletions

diff --git a/board/chell/board.h b/board/chell/board.h
index f4c1de0ab9..9f3e93cda9 100644
--- a/board/chell/board.h
+++ b/board/chell/board.h
@@ -136,7 +136,6 @@
 
 /* Modules we want to exclude */
 #undef CONFIG_CMD_BATTFAKE
-#undef CONFIG_CMD_CHARGER_PSYS
 #undef CONFIG_CMD_FLASH
 #undef CONFIG_CMD_HASH
 #undef CONFIG_CMD_HCDEBUG
diff --git a/board/coral/board.h b/board/coral/board.h
index d10081f0a0..b73770f6dc 100644
--- a/board/coral/board.h
+++ b/board/coral/board.h
@@ -27,7 +27,7 @@
 #define BD9995X_IOUT_GAIN_SELECT \
 		BD9995X_CMD_PMON_IOUT_CTRL_SET_IOUT_GAIN_SET_20V
 
-#define CONFIG_CMD_CHARGER_PSYS
+#define CONFIG_CHARGER_PSYS_READ
 #define BD9995X_PSYS_GAIN_SELECT \
 		BD9995X_CMD_PMON_IOUT_CTRL_SET_PMON_GAIN_SET_02UAW
 
diff --git a/board/eve/board.h b/board/eve/board.h
index 12e791944f..2e34294d47 100644
--- a/board/eve/board.h
+++ b/board/eve/board.h
@@ -60,7 +60,6 @@
 #define CONFIG_CMD_ACCEL_INFO
 #define CONFIG_CMD_BATT_MFG_ACCESS
 #define CONFIG_CMD_CHARGER_ADC_AMON_BMON
-#define CONFIG_CMD_CHARGER_PSYS
 #define CONFIG_CMD_PD_CONTROL
 
 /* SOC */
@@ -99,6 +98,7 @@
 #define CONFIG_CHARGER_MAINTAIN_VBAT
 #define CONFIG_CHARGER_MIN_BAT_PCT_FOR_POWER_ON 1
 #define CONFIG_CHARGER_PROFILE_OVERRIDE
+#define CONFIG_CHARGER_PSYS_READ
 #define CONFIG_CHARGER_SENSE_RESISTOR 10
 #define CONFIG_CHARGER_SENSE_RESISTOR_AC 10
 #define BD9995X_IOUT_GAIN_SELECT \
diff --git a/board/kahlee/board.h b/board/kahlee/board.h
index d43356c8af..733c8d75e3 100644
--- a/board/kahlee/board.h
+++ b/board/kahlee/board.h
@@ -22,7 +22,7 @@
 #define CONFIG_CMD_BATT_MFG_ACCESS
 #define CONFIG_CHARGER_SENSE_RESISTOR		10
 #define CONFIG_CHARGER_SENSE_RESISTOR_AC	10
-#define CONFIG_CMD_CHARGER_PSYS
+#define CONFIG_CHARGER_PSYS_READ
 
 /* Battery */
 #define CONFIG_BATTERY_DEVICE_CHEMISTRY  "LION"
diff --git a/board/kevin/board.h b/board/kevin/board.h
index 856accd01b..abebc67d4b 100644
--- a/board/kevin/board.h
+++ b/board/kevin/board.h
@@ -188,7 +188,6 @@
 
 /* Modules we want to exclude */
 #undef CONFIG_CMD_BATTFAKE
-#undef CONFIG_CMD_CHARGER_PSYS
 #undef CONFIG_CMD_FLASH
 #undef CONFIG_CMD_HASH
 #undef CONFIG_CMD_HCDEBUG
diff --git a/board/reef/board.h b/board/reef/board.h
index f67994c2b8..9d6b3f31d7 100644
--- a/board/reef/board.h
+++ b/board/reef/board.h
@@ -27,7 +27,7 @@
 #define BD9995X_IOUT_GAIN_SELECT \
 		BD9995X_CMD_PMON_IOUT_CTRL_SET_IOUT_GAIN_SET_20V
 
-#define CONFIG_CMD_CHARGER_PSYS
+#define CONFIG_CHARGER_PSYS_READ
 #define BD9995X_PSYS_GAIN_SELECT \
 		BD9995X_CMD_PMON_IOUT_CTRL_SET_PMON_GAIN_SET_02UAW
 
diff --git a/board/reef_it8320/board.h b/board/reef_it8320/board.h
index a7983e03b2..099cfe273d 100644
--- a/board/reef_it8320/board.h
+++ b/board/reef_it8320/board.h
@@ -24,7 +24,7 @@
 #define BD9995X_IOUT_GAIN_SELECT \
 		BD9995X_CMD_PMON_IOUT_CTRL_SET_IOUT_GAIN_SET_20V
 
-#define CONFIG_CMD_CHARGER_PSYS
+#define CONFIG_CHARGER_PSYS_READ
 #define BD9995X_PSYS_GAIN_SELECT \
 		BD9995X_CMD_PMON_IOUT_CTRL_SET_PMON_GAIN_SET_02UAW
 
diff --git a/board/scarlet/board.h b/board/scarlet/board.h
index 6c3478cdaf..2788d4bd79 100644
--- a/board/scarlet/board.h
+++ b/board/scarlet/board.h
@@ -148,7 +148,6 @@
 
 /* Modules we want to exclude */
 #undef CONFIG_CMD_BATTFAKE
-#undef CONFIG_CMD_CHARGER_PSYS
 #undef CONFIG_CMD_FLASH
 #undef CONFIG_CMD_HASH
 #undef CONFIG_CMD_I2C_SCAN
diff --git a/driver/charger/bd9995x.c b/driver/charger/bd9995x.c
index cd9b60d759..9ab2f55a47 100644
--- a/driver/charger/bd9995x.c
+++ b/driver/charger/bd9995x.c
@@ -1328,7 +1328,7 @@ DECLARE_CONSOLE_COMMAND(bd9995x, console_command_bd9995x,
 			"Read or write a charger register");
 #endif /* CONFIG_CMD_CHARGER */
 
-#ifdef CONFIG_CMD_CHARGER_PSYS
+#ifdef CONFIG_CHARGER_PSYS_READ
 static int bd9995x_psys_charger_adc(void)
 {
 	int i;
@@ -1375,6 +1375,11 @@ static int bd9995x_enable_psys(void)
 
 /**
  * Get system power.
+ *
+ * TODO(b:71520677): Implement charger_get_system_power, disable psys readout
+ * when not needed (the code below leaves it enabled after the first access),
+ * update "psys" console command to use charger_get_system_power and move it
+ * to some common code.
  */
 static int console_command_psys(int argc, char **argv)
 {
@@ -1392,7 +1397,7 @@ static int console_command_psys(int argc, char **argv)
 DECLARE_CONSOLE_COMMAND(psys, console_command_psys,
 			NULL,
 			"Get the system power in mW");
-#endif /* CONFIG_CMD_CHARGER_PSYS */
+#endif /* CONFIG_CHARGER_PSYS_READ */
 
 #ifdef CONFIG_CMD_CHARGER_ADC_AMON_BMON
 static int bd9995x_amon_bmon_chg_adc(void)
diff --git a/driver/charger/isl923x.c b/driver/charger/isl923x.c
index e246f05ade..0117ad74d4 100644
--- a/driver/charger/isl923x.c
+++ b/driver/charger/isl923x.c
@@ -452,6 +452,8 @@ int chg_ramp_get_current_limit(void)
 
 
 #ifdef CONFIG_CHARGER_PSYS
+static int psys_enabled;
+
 static void charger_enable_psys(void)
 {
 	int val;
@@ -461,11 +463,17 @@ static void charger_enable_psys(void)
 	/*
 	 * enable system power monitor PSYS function
 	 */
-	if (!raw_read16(ISL923X_REG_CONTROL1, &val)) {
-		val |= ISL923X_C1_ENABLE_PSYS;
-		raw_write16(ISL923X_REG_CONTROL1, val);
-	}
+	if (raw_read16(ISL923X_REG_CONTROL1, &val))
+		goto out;
+
+	val |= ISL923X_C1_ENABLE_PSYS;
+
+	if (raw_write16(ISL923X_REG_CONTROL1, val))
+		goto out;
 
+	psys_enabled = 1;
+
+out:
 	mutex_unlock(&control1_mutex);
 }
 DECLARE_HOOK(HOOK_CHIPSET_STARTUP, charger_enable_psys, HOOK_PRIO_DEFAULT);
@@ -479,61 +487,52 @@ static void charger_disable_psys(void)
 	/*
 	 * disable system power monitor PSYS function
 	 */
-	if (!raw_read16(ISL923X_REG_CONTROL1, &val)) {
-		val &= ~ISL923X_C1_ENABLE_PSYS;
-		raw_write16(ISL923X_REG_CONTROL1, val);
-	}
+	if (raw_read16(ISL923X_REG_CONTROL1, &val))
+		goto out;
+
+	val &= ~ISL923X_C1_ENABLE_PSYS;
+
+	if (raw_write16(ISL923X_REG_CONTROL1, val))
+		goto out;
+
+	psys_enabled = 0;
 
+out:
 	mutex_unlock(&control1_mutex);
 }
 DECLARE_HOOK(HOOK_CHIPSET_SHUTDOWN, charger_disable_psys, HOOK_PRIO_DEFAULT);
 
-#ifdef CONFIG_CMD_CHARGER_PSYS
-/* TODO(crosbug.com/p/61166): This dead code is ISL9237-specific. */
-#define PSYS_ADC_READ_COUNT 100
-static int charger_get_system_power(void)
+#ifdef CONFIG_CHARGER_PSYS_READ
+int charger_get_system_power(void)
 {
-	int adc = 0;
-	int i;
-	int ret;
-	int val;
-
-	ret = raw_read16(ISL923X_REG_CONTROL2, &val);
-	if (ret)
-		return ret;
+	int adc;
 
-	/* Read ADC */
-	for (i = 0; i < PSYS_ADC_READ_COUNT; i++) {
-		adc += adc_read_channel(ADC_PSYS);
-		usleep(10);
-	}
+	/*
+	 * If PSYS is not enabled, AP is probably off, and the value is usually
+	 * too small to be measured acurately anyway.
+	 */
+	if (!psys_enabled)
+		return -1;
 
 	/*
-	 * Calculate the power in mW (Power = adc * gain)
-	 *
-	 * System power monitor PSYS output gain
-	 * [0]: 0 = 1.44 uA/W
-	 *      1 = 0.36 uA/W
-	 *
-	 * Do not divide the constants first to ensure precision is not lost.
+	 * We assume that the output gain is always left to the default
+	 * 1.44 uA/W, and that the ADC scaling values are setup accordingly in
+	 * board file, so that the value is indicated in uW.
 	 */
-	if (val & ISL923X_C2_PSYS_GAIN)
-		return ((adc * ISL923X_C2_PSYS_GAIN_0_36) /
-				PSYS_ADC_READ_COUNT);
-	else
-		return ((adc * ISL923X_C2_PSYS_GAIN_1_44) /
-				PSYS_ADC_READ_COUNT);
+	adc = adc_read_channel(ADC_PSYS);
+
+	return adc;
 }
 
 static int console_command_psys(int argc, char **argv)
 {
-	CPRINTF("system power = %d mW\n", charger_get_system_power());
+	ccprintf("PSYS = %d uW\n", charger_get_system_power());
 	return 0;
 }
 DECLARE_CONSOLE_COMMAND(psys, console_command_psys,
 			NULL,
 			"Get the system power in mW");
-#endif /* CONFIG_CMD_CHARGER_PSYS */
+#endif /* CONFIG_CHARGER_PSYS_READ */
 #endif /* CONFIG_CHARGER_PSYS */
 
 #ifdef CONFIG_CMD_CHARGER_ADC_AMON_BMON
diff --git a/include/charger.h b/include/charger.h
index 16f7c3cbd0..c66faa0a97 100644
--- a/include/charger.h
+++ b/include/charger.h
@@ -121,6 +121,12 @@ int charger_get_vbus_voltage(int port);
 /* Custom board function to discharge battery when on AC power */
 int board_discharge_on_ac(int enable);
 
+/*
+ * Read the current total system power in uW (usually from PSYS).
+ * Can be negative if the PSYS output is not currently enabled (e.g. AP is off).
+ */
+int charger_get_system_power(void);
+
 /* Other parameters that may be charger-specific, but are common so far. */
 int charger_set_input_current(int input_current);
 int charger_get_input_current(int *input_current);
diff --git a/include/config.h b/include/config.h
index 5b2b7d2a29..3e8d2d6935 100644
--- a/include/config.h
+++ b/include/config.h
@@ -521,10 +521,19 @@
  */
 #undef CONFIG_CHARGER_CURRENT_LIMIT
 
-/* Enable/disable system power monitor PSYS function */
+/*
+ * Enable/disable system power monitor PSYS function: this enables output
+ * from charger chip to SoC.
+ */
 #undef CONFIG_CHARGER_PSYS
 
 /*
+ * Enable reading PSYS (system power) value, either via "psys" console command,
+ * or via charger_get_system_power function.
+ */
+#undef CONFIG_CHARGER_PSYS_READ
+
+/*
  * Board specific charging current termination limit, in mA.  If defined and
  * charger supports setting termination current it should be set during charger
  * init.
@@ -729,7 +738,6 @@
 #undef  CONFIG_CMD_CHARGER_ADC_AMON_BMON
 #undef  CONFIG_CMD_CHARGER_PROFILE_OVERRIDE
 #undef  CONFIG_CMD_CHARGER_PROFILE_OVERRIDE_TEST
-#undef  CONFIG_CMD_CHARGER_PSYS
 #define CONFIG_CMD_CHARGE_SUPPLIER_INFO
 #undef  CONFIG_CMD_CHGRAMP
 #undef  CONFIG_CMD_CLOCKGATES