Hi Julian,
On 1/5/20 7:00 AM, Julian Calaby wrote:
On Sun, Jan 5, 2020 at 12:24 PM Samuel Holland samuel@sholland.org wrote:
AXP803/AXP813 have a flag that enables/disables the AC power supply input. This flag does not affect the status bits in PWR_INPUT_STATUS. Its effect can be verified by checking the battery charge/discharge state (bit 2 of PWR_INPUT_STATUS), or by examining the current draw on the AC input.
Take this flag into account when getting the ONLINE property of the AC input, on PMICs where this flag is present.
Fixes: 7693b5643fd2 ("power: supply: add AC power supply driver for AXP813") Cc: stable@vger.kernel.org Signed-off-by: Samuel Holland samuel@sholland.org
drivers/power/supply/axp20x_ac_power.c | 31 +++++++++++++++++++++----- 1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/drivers/power/supply/axp20x_ac_power.c b/drivers/power/supply/axp20x_ac_power.c index 0d34a932b6d5..ca0a28f72a27 100644 --- a/drivers/power/supply/axp20x_ac_power.c +++ b/drivers/power/supply/axp20x_ac_power.c @@ -23,6 +23,8 @@ #define AXP20X_PWR_STATUS_ACIN_PRESENT BIT(7) #define AXP20X_PWR_STATUS_ACIN_AVAIL BIT(6)
+#define AXP813_ACIN_PATH_SEL BIT(7)
#define AXP813_VHOLD_MASK GENMASK(5, 3) #define AXP813_VHOLD_UV_TO_BIT(x) ((((x) / 100000) - 40) << 3) #define AXP813_VHOLD_REG_TO_UV(x) \ @@ -40,6 +42,7 @@ struct axp20x_ac_power { struct power_supply *supply; struct iio_channel *acin_v; struct iio_channel *acin_i;
bool has_acin_path_sel;};
static irqreturn_t axp20x_ac_power_irq(int irq, void *devid) @@ -86,6 +89,17 @@ static int axp20x_ac_power_get_property(struct power_supply *psy, return ret;
val->intval = !!(reg & AXP20X_PWR_STATUS_ACIN_AVAIL);
/* ACIN_PATH_SEL disables ACIN even if ACIN_AVAIL is set. */if (power->has_acin_path_sel) {Do we need to check this bit if ACIN_AVAIL is not set?
No, we don't. However due to regcache this won't actually cause another read from the device. If I send a v3, I'll move the && to the if statement.
ret = regmap_read(power->regmap, AXP813_ACIN_PATH_CTRL,®);if (ret)return ret;val->intval &= !!(reg & AXP813_ACIN_PATH_SEL);If we only check this bit if ACIN_AVAIL is set, then we don't need the "&" in the "&=". (I'm assuming that val->intval is an int, not a bool, otherwise this is the wrong operator)
val->intval is an int, but it only ever takes the values 0 or 1. The !! expression coerces an integer to the range of a boolean. So the two ways of deriving the value ("&=" here vs "&& val->intval" in the if statement) are equivalent.
Thanks,
Thanks! Samuel