raa489000: never report ACOK when sourcing VBUS

This chip sometimes ACOK when sourcing VBUS because the ACOK status bit
actually indicates whether VBUS is above approximately 3.6V and the
charger is not overcurrent or over-temperature, though it also seems to
report ACOK correctly when the port comes up as a source rather than
going through a PRS. This change makes raa489000_is_acok return a correct
value (not-OK) in all situations when sourcing VBUS.

BUG=b:263691951,b:262663436
TEST=On Nivviks, `hibernate` now correctly hibernates following fast role
     swap from sink to source on port C0 (with C1 disconnected), where
     previously it refused to hibernate with the "AC on, skip hibernate"
     message.
BRANCH=nissa

Change-Id: I314988d7dfcc8b4d576224f934cd5dc693e7f54f
Signed-off-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4222772
Reviewed-by: Adam Mills <adamjmills@google.com>
Reviewed-by: Arthur Lin <arthur.lin@lcfc.corp-partner.google.com>

1 files changed, 22 insertions, 1 deletions

diff --git a/driver/charger/isl923x.c b/driver/charger/isl923x.c
index f8336a1f50..a5fae9329d 100644
--- a/driver/charger/isl923x.c
+++ b/driver/charger/isl923x.c
@@ -887,7 +887,28 @@ enum ec_error_list raa489000_is_acok(int chgnum, bool *acok)
 	rv = raw_read16(chgnum, ISL9238_REG_INFO2, &regval);
 	if (rv != EC_SUCCESS)
 		return rv;
-	*acok = (regval & RAA489000_INFO2_ACOK);
+
+	/*
+	 * ACOK can sometimes be asserted when RAA489000 is sourcing VBUS in OTG
+	 * mode, because that bit is derived from the VBUS comparator. If the
+	 * charger reports it's in OTG mode, always say ACOK is false because we
+	 * can't be running from a charger if we're also sourcing VBUS and the
+	 * ACOK bit may be untrustworthy.
+	 *
+	 * This may sometimes report incorrectly because the state bits of
+	 * the Information2 register take a small amount of time to update on
+	 * a state change. In most cases the event hooks used to trigger
+	 * raa489000_check_ac_present are good indications of a state change,
+	 * but during power role swaps there may be no hooks executed so it's
+	 * most consistent to use the charger's reported state only (otherwise
+	 * some situations could use the EC's view of the current state and
+	 * others would require asking the charger).
+	 */
+	if (((regval >> RAA489000_INFO2_STATE_SHIFT) &
+	     RAA489000_INFO2_STATE_MASK) == RAA489000_INFO2_STATE_OTG)
+		*acok = false;
+	else
+		*acok = (regval & RAA489000_INFO2_ACOK) != 0;
 
 	return EC_SUCCESS;
 }