Re: [PATCH v4 5/7] power: supply: add AC power supply driver for AXP813

From: Chen-Yu Tsai
Date: Wed Oct 17 2018 - 02:13:21 EST

On Sat, Oct 13, 2018 at 4:09 PM Oskari Lemmela <oskari@xxxxxxxxxxx> wrote:
>
> AXP813 and AXP803 PMICs can control input current and
> minimum voltage.
>
> Both of these values are configurable.
>
> Signed-off-by: Oskari Lemmela <oskari@xxxxxxxxxxx>
> Reviewed-by: Quentin Schulz <quentin.schulz@xxxxxxxxxxx>
> ---
> drivers/power/supply/axp20x_ac_power.c | 92 ++++++++++++++++++++++++++
> include/linux/mfd/axp20x.h | 1 +
> 2 files changed, 93 insertions(+)
>
> diff --git a/drivers/power/supply/axp20x_ac_power.c b/drivers/power/supply/axp20x_ac_power.c
> index 0771f951b11f..059a97d6e14c 100644
> --- a/drivers/power/supply/axp20x_ac_power.c
> +++ b/drivers/power/supply/axp20x_ac_power.c
> @@ -27,6 +27,16 @@
> #define AXP20X_PWR_STATUS_ACIN_PRESENT BIT(7)
> #define AXP20X_PWR_STATUS_ACIN_AVAIL BIT(6)
>
> +#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) \
> + (((((x) & AXP813_VHOLD_MASK) >> 3) + 40) * 100000)
> +
> +#define AXP813_CURR_LIMIT_MASK GENMASK(2, 0)
> +#define AXP813_CURR_LIMIT_UA_TO_BIT(x) (((x) / 500000) - 3)
> +#define AXP813_CURR_LIMIT_REG_TO_UA(x) \
> + ((((x) & AXP813_CURR_LIMIT_MASK) + 3) * 500000)
> +
> #define DRVNAME "axp20x-ac-power-supply"
>
> struct axp20x_ac_power {
> @@ -102,6 +112,55 @@ static int axp20x_ac_power_get_property(struct power_supply *psy,
>
> return 0;
>
> + case POWER_SUPPLY_PROP_VOLTAGE_MIN:
> + ret = regmap_read(power->regmap, AXP813_ACIN_PATH_CTRL, &reg);
> + if (ret)
> + return ret;
> +
> + val->intval = AXP813_VHOLD_REG_TO_UV(reg);
> +
> + return 0;
> +
> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> + ret = regmap_read(power->regmap, AXP813_ACIN_PATH_CTRL, &reg);
> + if (ret)
> + return ret;
> +
> + val->intval = AXP813_CURR_LIMIT_REG_TO_UA(reg);

This doesn't handle the case where the register value is set to 0x6 or 0x7,
which should still mean 4000 mA.

> +
> + return 0;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int axp813_ac_power_set_property(struct power_supply *psy,
> + enum power_supply_property psp,
> + const union power_supply_propval *val)
> +{
> + struct axp20x_ac_power *power = power_supply_get_drvdata(psy);
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_VOLTAGE_MIN:
> + if (val->intval < 4000000 || val->intval > 4700000)
> + return -EINVAL;
> +
> + return regmap_update_bits(power->regmap, AXP813_ACIN_PATH_CTRL,
> + AXP813_VHOLD_MASK,
> + AXP813_VHOLD_UV_TO_BIT(val->intval));
> +
> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> + if (val->intval < 1500000 || val->intval > 4000000)
> + return -EINVAL;
> +
> + return regmap_update_bits(power->regmap, AXP813_ACIN_PATH_CTRL,
> + AXP813_CURR_LIMIT_MASK,
> + AXP813_CURR_LIMIT_UA_TO_BIT
> + (val->intval));

Nit, don't wrap this part. Even if it exceeds the 80 character limit, it's
still easier to read that way.

ChenYu

> +
> default:
> return -EINVAL;
> }
> @@ -109,6 +168,13 @@ static int axp20x_ac_power_get_property(struct power_supply *psy,
> return -EINVAL;
> }
>
> +static int axp813_ac_power_prop_writeable(struct power_supply *psy,
> + enum power_supply_property psp)
> +{
> + return psp == POWER_SUPPLY_PROP_VOLTAGE_MIN ||
> + psp == POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT;
> +}
> +
> static enum power_supply_property axp20x_ac_power_properties[] = {
> POWER_SUPPLY_PROP_HEALTH,
> POWER_SUPPLY_PROP_PRESENT,
> @@ -123,6 +189,14 @@ static enum power_supply_property axp22x_ac_power_properties[] = {
> POWER_SUPPLY_PROP_ONLINE,
> };
>
> +static enum power_supply_property axp813_ac_power_properties[] = {
> + POWER_SUPPLY_PROP_HEALTH,
> + POWER_SUPPLY_PROP_PRESENT,
> + POWER_SUPPLY_PROP_ONLINE,
> + POWER_SUPPLY_PROP_VOLTAGE_MIN,
> + POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> +};
> +
> static const struct power_supply_desc axp20x_ac_power_desc = {
> .name = "axp20x-ac",
> .type = POWER_SUPPLY_TYPE_MAINS,
> @@ -139,6 +213,16 @@ static const struct power_supply_desc axp22x_ac_power_desc = {
> .get_property = axp20x_ac_power_get_property,
> };
>
> +static const struct power_supply_desc axp813_ac_power_desc = {
> + .name = "axp813-ac",
> + .type = POWER_SUPPLY_TYPE_MAINS,
> + .properties = axp813_ac_power_properties,
> + .num_properties = ARRAY_SIZE(axp813_ac_power_properties),
> + .property_is_writeable = axp813_ac_power_prop_writeable,
> + .get_property = axp20x_ac_power_get_property,
> + .set_property = axp813_ac_power_set_property,
> +};
> +
> struct axp_data {
> const struct power_supply_desc *power_desc;
> bool acin_adc;
> @@ -154,6 +238,11 @@ static const struct axp_data axp22x_data = {
> .acin_adc = false,
> };
>
> +static const struct axp_data axp813_data = {
> + .power_desc = &axp813_ac_power_desc,
> + .acin_adc = false,
> +};
> +
> static int axp20x_ac_power_probe(struct platform_device *pdev)
> {
> struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
> @@ -234,6 +323,9 @@ static const struct of_device_id axp20x_ac_power_match[] = {
> }, {
> .compatible = "x-powers,axp221-ac-power-supply",
> .data = &axp22x_data,
> + }, {
> + .compatible = "x-powers,axp813-ac-power-supply",
> + .data = &axp813_data,
> }, { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, axp20x_ac_power_match);
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index 517e60eecbcb..2302b620d238 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -266,6 +266,7 @@ enum axp20x_variants {
> #define AXP288_RT_BATT_V_H 0xa0
> #define AXP288_RT_BATT_V_L 0xa1
>
> +#define AXP813_ACIN_PATH_CTRL 0x3a
> #define AXP813_ADC_RATE 0x85
>
> /* Fuel Gauge */
> --
> 2.17.1
>