sm5803: add tests for set_vsys_compensation

Two bugs in the driver were exposed by this test and are fixed:
 * All 8 bits of IR_COMP_REG2 are used as resistance value (in addition
   to bits 6 and 7 of IR_COMP_REG1), but the driver incorrectly masked
   off bit 7 which would cause incorrect values to be programmed when
   resistance is greater than 212 mΩ.
 * sm5803_set_vsys_compensation always returned an error, which apparently
   goes unchecked by regular charger code.

BUG=b:242544165
TEST=./twister -ci -T zephyr/test/drivers/ -s drivers.sm5803
BRANCH=none

Change-Id: I761c3523d9903c3498cfe30d93ddb56b004cc4ef
Signed-off-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4482275
Reviewed-by: Tristan Honscheid <honscheid@google.com>

4 files changed, 64 insertions, 2 deletions

diff --git a/driver/charger/sm5803.c b/driver/charger/sm5803.c
index ec7132fb35..f480d520e0 100644
--- a/driver/charger/sm5803.c
+++ b/driver/charger/sm5803.c
@@ -1941,7 +1941,7 @@ static enum ec_error_list sm5803_set_vsys_compensation(int chgnum,
 	/* Set IR drop compensation */
 	r = ocpc->combined_rsys_rbatt_mo * 100 / 167; /* 1.67mOhm steps */
 	r = MAX(0, r);
-	rv = chg_write8(chgnum, SM5803_REG_IR_COMP2, r & 0x7F);
+	rv = chg_write8(chgnum, SM5803_REG_IR_COMP2, r & 0xFF);
 	rv |= chg_read8(chgnum, SM5803_REG_IR_COMP1, &regval);
 	regval &= ~SM5803_IR_COMP_RES_SET_MSB;
 	r = r >> 8; /* Bits 9:8 */
@@ -1952,7 +1952,7 @@ static enum ec_error_list sm5803_set_vsys_compensation(int chgnum,
 	if (rv)
 		return EC_ERROR_UNKNOWN;
 
-	return EC_ERROR_UNIMPLEMENTED;
+	return EC_SUCCESS;
 }
 
 /* Hardware current ramping (aka DPM: Dynamic Power Management) */
diff --git a/zephyr/emul/emul_sm5803.c b/zephyr/emul/emul_sm5803.c
index 39dda70598..d41dd48f09 100644
--- a/zephyr/emul/emul_sm5803.c
+++ b/zephyr/emul/emul_sm5803.c
@@ -56,6 +56,8 @@ struct sm5803_emul_data {
 	uint8_t pre_fast_conf[6];
 	/** Raw value of GPIO0_CTRL register */
 	uint8_t gpio_ctrl;
+	/** Raw values of IR_COMP_REG1 and 2 */
+	uint8_t ir_comp1, ir_comp2;
 };
 
 struct sm5803_emul_cfg {
@@ -241,6 +243,13 @@ uint8_t sm5803_emul_get_gpio_ctrl(const struct emul *emul)
 	return data->gpio_ctrl;
 }
 
+uint16_t sm5803_emul_get_ir_comp(const struct emul *emul)
+{
+	struct sm5803_emul_data *data = emul->data;
+
+	return (data->ir_comp1 << 8) | data->ir_comp2;
+}
+
 static void sm5803_emul_reset(const struct emul *emul)
 {
 	struct sm5803_emul_data *data = emul->data;
@@ -285,6 +294,8 @@ static void sm5803_emul_reset(const struct emul *emul)
 	data->disch_conf5 = 0;
 	memset(data->pre_fast_conf, 0, sizeof(data->pre_fast_conf));
 	data->gpio_ctrl = 0x04;
+	data->ir_comp1 = 1;
+	data->ir_comp2 = 1;
 
 	/* Interrupt pin deasserted */
 	gpio_emul_input_set(cfg->interrupt_gpio.port, cfg->interrupt_gpio.pin,
@@ -409,6 +420,12 @@ static int sm5803_chg_read_byte(const struct emul *target, int reg,
 	case SM5803_REG_PRE_FAST_CONF_REG1 + 5:
 		*val = data->pre_fast_conf[reg - SM5803_REG_PRE_FAST_CONF_REG1];
 		return 0;
+	case SM5803_REG_IR_COMP1:
+		*val = data->ir_comp1;
+		return 0;
+	case SM5803_REG_IR_COMP2:
+		*val = data->ir_comp2;
+		return 0;
 	case SM5803_REG_LOG2:
 		*val = ((data->ibus * ADC_CURRENT_LSB_MA) >
 			(data->input_current_limit * ICL_LSB_MA))
@@ -454,6 +471,12 @@ static int sm5803_chg_write_byte(const struct emul *target, int reg,
 	case SM5803_REG_PRE_FAST_CONF_REG1 + 5:
 		data->pre_fast_conf[reg - SM5803_REG_PRE_FAST_CONF_REG1] = val;
 		return 0;
+	case SM5803_REG_IR_COMP1:
+		data->ir_comp1 = val;
+		return 0;
+	case SM5803_REG_IR_COMP2:
+		data->ir_comp2 = val;
+		return 0;
 	case SM5803_REG_PHOT1:
 		data->phot1 = val;
 		return 0;
diff --git a/zephyr/include/emul/emul_sm5803.h b/zephyr/include/emul/emul_sm5803.h
index 006fe1d0c7..481644640d 100644
--- a/zephyr/include/emul/emul_sm5803.h
+++ b/zephyr/include/emul/emul_sm5803.h
@@ -75,3 +75,10 @@ void sm5803_emul_set_irqs(const struct emul *emul, uint8_t irq1, uint8_t irq2,
 
 /** Get the value of the GPIO_CTRL_1 register, which controls GPIO0. */
 uint8_t sm5803_emul_get_gpio_ctrl(const struct emul *emul);
+
+/**
+ * Get the values of the IR_COMP1 and IR_COMP2 registers.
+ *
+ * Register values are concatenated, with COMP1 as MSB and COMP2 as LSB.
+ */
+uint16_t sm5803_emul_get_ir_comp(const struct emul *emul);
diff --git a/zephyr/test/drivers/sm5803/src/sm5803.c b/zephyr/test/drivers/sm5803/src/sm5803.c
index 159f91bc71..e6db27b47d 100644
--- a/zephyr/test/drivers/sm5803/src/sm5803.c
+++ b/zephyr/test/drivers/sm5803/src/sm5803.c
@@ -707,3 +707,35 @@ ZTEST(sm5803, test_gpio)
 	zassert_not_equal(sm5803_set_gpio0_level(CHARGER_NUM, 0), 0);
 	zassert_not_equal(sm5803_configure_chg_det_od(CHARGER_NUM, 1), 0);
 }
+
+ZTEST(sm5803, test_vsys_compensation)
+{
+	struct ocpc_data ocpc;
+
+	/* Minimum possible resistance */
+	ocpc.combined_rsys_rbatt_mo = 0;
+	zassert_ok(sm5803_drv.set_vsys_compensation(CHARGER_NUM, &ocpc, 0, 0));
+	zassert_equal(sm5803_emul_get_ir_comp(SM5803_EMUL), 0x2100,
+		      "actual IR_COMP value was %#x",
+		      sm5803_emul_get_ir_comp(SM5803_EMUL));
+
+	/* Maximum resistance supported by SM5803 (1.67 mOhm * 0x3FF) */
+	ocpc.combined_rsys_rbatt_mo = 1709;
+	zassert_ok(sm5803_drv.set_vsys_compensation(CHARGER_NUM, &ocpc, 0, 0));
+	zassert_equal(sm5803_emul_get_ir_comp(SM5803_EMUL), 0xE1FF,
+		      "actual IR_COMP value was %#x",
+		      sm5803_emul_get_ir_comp(SM5803_EMUL));
+
+	/* Typical actual resistance */
+	ocpc.combined_rsys_rbatt_mo = 42;
+	zassert_ok(sm5803_drv.set_vsys_compensation(CHARGER_NUM, &ocpc, 0, 0));
+	zassert_equal(sm5803_emul_get_ir_comp(SM5803_EMUL), 0x2119,
+		      "actual IR_COMP value was %#x",
+		      sm5803_emul_get_ir_comp(SM5803_EMUL));
+
+	/* Communication errors bubble up */
+	i2c_common_emul_set_read_fail_reg(sm5803_emul_get_i2c_chg(SM5803_EMUL),
+					  SM5803_REG_IR_COMP1);
+	zassert_not_equal(
+		sm5803_drv.set_vsys_compensation(CHARGER_NUM, &ocpc, 0, 0), 0);
+}