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: [email protected]
Signed-off-by: Samuel Holland <[email protected]>
---
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) {
+ ret = regmap_read(power->regmap, AXP813_ACIN_PATH_CTRL,
+ ®);
+ if (ret)
+ return ret;
+
+ val->intval &= !!(reg & AXP813_ACIN_PATH_SEL);
+ }
+
return 0;
case POWER_SUPPLY_PROP_VOLTAGE_NOW:
@@ -224,21 +238,25 @@ static const struct power_supply_desc axp813_ac_power_desc = {
struct axp_data {
const struct power_supply_desc *power_desc;
bool acin_adc;
+ bool acin_path_sel;
};
static const struct axp_data axp20x_data = {
- .power_desc = &axp20x_ac_power_desc,
- .acin_adc = true,
+ .power_desc = &axp20x_ac_power_desc,
+ .acin_adc = true,
+ .acin_path_sel = false,
};
static const struct axp_data axp22x_data = {
- .power_desc = &axp22x_ac_power_desc,
- .acin_adc = false,
+ .power_desc = &axp22x_ac_power_desc,
+ .acin_adc = false,
+ .acin_path_sel = false,
};
static const struct axp_data axp813_data = {
- .power_desc = &axp813_ac_power_desc,
- .acin_adc = false,
+ .power_desc = &axp813_ac_power_desc,
+ .acin_adc = false,
+ .acin_path_sel = true,
};
static int axp20x_ac_power_probe(struct platform_device *pdev)
@@ -282,6 +300,7 @@ static int axp20x_ac_power_probe(struct platform_device *pdev)
}
power->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ power->has_acin_path_sel = axp_data->acin_path_sel;
platform_set_drvdata(pdev, power);
--
2.23.0
On Sun, Jan 5, 2020 at 9:24 AM Samuel Holland <[email protected]> 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: [email protected]
> Signed-off-by: Samuel Holland <[email protected]>
Reviewed-by: Chen-Yu Tsai <[email protected]>
Hi Samuel,
On Sun, Jan 5, 2020 at 12:24 PM Samuel Holland <[email protected]> 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: [email protected]
> Signed-off-by: Samuel Holland <[email protected]>
> ---
> 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?
> + 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)
Thanks,
--
Julian Calaby
Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
Hi Julian,
On 1/5/20 7:00 AM, Julian Calaby wrote:
> On Sun, Jan 5, 2020 at 12:24 PM Samuel Holland <[email protected]> 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: [email protected]
>> Signed-off-by: Samuel Holland <[email protected]>
>> ---
>> 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