Re: [linux-sunxi] [PATCH 2/2] power: supply: axp20x_usb_power: add applied max Vbus support for AXP813
Hi, On Tue, Oct 08, 2019 at 12:07:05AM +0800, Chen-Yu Tsai wrote: > As far as the sysfs documents go, CURRENT_MAX is read-only, and should refer > to > the hard limit the hardware can support, i.e. maximum power ratings. > INPUT_CURRENT_LIMIT and INPUT_VOLTAGE_LIMIT are for configurable upper and > lower > limits respectively. > > Sebastian, is my understanding of this correct? Yes. -- Sebastian signature.asc Description: PGP signature
Re: [linux-sunxi] [PATCH 2/2] power: supply: axp20x_usb_power: add applied max Vbus support for AXP813
On Tue, Oct 8, 2019 at 11:09 AM Icenowy Zheng wrote:
> 于 2019年10月8日 GMT+08:00 上午12:07:05, Chen-Yu Tsai 写到:
> >Hi,
> >
> >On Wed, Oct 2, 2019 at 7:27 PM Icenowy Zheng wrote:
> >>
> >> AXP813 PMIC has two Vbus maximum value settings -- one is the default
> >> value, which is currently the only supported one; the other is the
> >> really applied value, which is set according to the default value if
> >the
> >> BC detection module detected a charging port, or 500mA if no charging
> >> port is detected.
> >>
> >> Add support for reading and writing of the really applied Vbus
> >maxmium
> >> value. Interestingly it has a larger range than the default value.
> >>
> >> Signed-off-by: Icenowy Zheng
> >> ---
> >> drivers/power/supply/axp20x_usb_power.c | 132
> >+++-
> >> 1 file changed, 129 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/power/supply/axp20x_usb_power.c
> >b/drivers/power/supply/axp20x_usb_power.c
> >> index 5f0a5722b19e..905668a2727f 100644
> >> --- a/drivers/power/supply/axp20x_usb_power.c
> >> +++ b/drivers/power/supply/axp20x_usb_power.c
> >
> >[...]
> >
> >> @@ -354,6 +451,9 @@ static int axp20x_usb_power_set_property(struct
> >power_supply *psy,
> >>
> >val->intval);
> >> return axp20x_usb_power_set_current_max(power,
> >val->intval);
> >>
> >> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> >> + return
> >axp20x_usb_power_set_input_current_limit(power, val->intval);
> >> +
> >
> >So I think there are two things that should be adjusted.
> >
> >First, we should be using POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT for all
> >PMICs.
> >As far as the sysfs documents go, CURRENT_MAX is read-only, and should
> >refer to
> >the hard limit the hardware can support, i.e. maximum power ratings.
> >INPUT_CURRENT_LIMIT and INPUT_VOLTAGE_LIMIT are for configurable upper
> >and lower
> >limits respectively.
> >
> >Sebastian, is my understanding of this correct?
> >
> >We already use INPUT_CURRENT_LIMIT for the AXP813 in the axp20x-ac
> >driver, and
> >it would be nice to have both drivers expose the same attributes.
> >
> >Second, since the value set in register 0x35 is the one that actually
> >has an
> >effect, as opposed to just being a default, we should just use that
> >one.
>
> However, that default value is also important, otherwise users will
> get dropped back to 500mAh each time they re-insert USB jack.
>
> Is there a property to export the default value?
Not that I know of. I suppose you could piggy back on INPUT_CURRENT_LIMIT
and just set both values at the same time. Of course the default value
has a smaller range, so you would end up setting the highest value for
actual values above its range.
> BTW, if possible, apply patch 1 first, because it can raise current to 1.5A
> in the default situation.
Agreed.
ChenYu
> >Could you restructure the series based on what I described, with a new
> >patch 1
> >switching from CURRENT_MAX to INPUT_CURRENT_LIMIT, and then this patch
> >as patch 2?
> >And both patches should have Fixes tags and possibly CC stable so they
> >get backported
> >for people that are using stable kernels? And then the original patch
> >2 as patch 3.
> >
> >ChenYu
> >
> >> default:
> >> return -EINVAL;
> >> }
> >> @@ -365,7 +465,8 @@ static int axp20x_usb_power_prop_writeable(struct
> >power_supply *psy,
> >>enum power_supply_property
> >psp)
> >> {
> >> return psp == POWER_SUPPLY_PROP_VOLTAGE_MIN ||
> >> - psp == POWER_SUPPLY_PROP_CURRENT_MAX;
> >> + psp == POWER_SUPPLY_PROP_CURRENT_MAX ||
> >> + psp == POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT;
> >> }
> >>
> >> static enum power_supply_property axp20x_usb_power_properties[] = {
> >> @@ -386,6 +487,15 @@ static enum power_supply_property
> >axp22x_usb_power_properties[] = {
> >> POWER_SUPPLY_PROP_CURRENT_MAX,
> >> };
> >>
> >> +static enum power_supply_property axp813_usb_power_properties[] = {
> >> + POWER_SUPPLY_PROP_HEALTH,
> >> + POWER_SUPPLY_PROP_PRESENT,
> >> + POWER_SUPPLY_PROP_ONLINE,
> >> + POWER_SUPPLY_PROP_VOLTAGE_MIN,
> >> + POWER_SUPPLY_PROP_CURRENT_MAX,
> >> + POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> >> +};
> >> +
> >> static const struct power_supply_desc axp20x_usb_power_desc = {
> >> .name = "axp20x-usb",
> >> .type = POWER_SUPPLY_TYPE_USB,
> >> @@ -406,6 +516,16 @@ static const struct power_supply_desc
> >axp22x_usb_power_desc = {
> >> .set_property = axp20x_usb_power_set_property,
> >> };
> >>
> >> +static const struct power_supply_desc axp813_usb_power_desc = {
> >> + .name = "axp20x-usb",
> >> + .type = POWER_SUPPLY_TYPE_USB,
> >> + .properties = axp813_usb_power_properties,
> >> + .num_properties = ARRAY_SIZE(axp813_usb_power_properties),
> >> + .property_is_writeable = axp20x_usb_power_prop_writeable,
> >> + .get_
Re: [linux-sunxi] [PATCH 2/2] power: supply: axp20x_usb_power: add applied max Vbus support for AXP813
于 2019年10月8日 GMT+08:00 上午12:07:05, Chen-Yu Tsai 写到:
>Hi,
>
>On Wed, Oct 2, 2019 at 7:27 PM Icenowy Zheng wrote:
>>
>> AXP813 PMIC has two Vbus maximum value settings -- one is the default
>> value, which is currently the only supported one; the other is the
>> really applied value, which is set according to the default value if
>the
>> BC detection module detected a charging port, or 500mA if no charging
>> port is detected.
>>
>> Add support for reading and writing of the really applied Vbus
>maxmium
>> value. Interestingly it has a larger range than the default value.
>>
>> Signed-off-by: Icenowy Zheng
>> ---
>> drivers/power/supply/axp20x_usb_power.c | 132
>+++-
>> 1 file changed, 129 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/power/supply/axp20x_usb_power.c
>b/drivers/power/supply/axp20x_usb_power.c
>> index 5f0a5722b19e..905668a2727f 100644
>> --- a/drivers/power/supply/axp20x_usb_power.c
>> +++ b/drivers/power/supply/axp20x_usb_power.c
>
>[...]
>
>> @@ -354,6 +451,9 @@ static int axp20x_usb_power_set_property(struct
>power_supply *psy,
>>
>val->intval);
>> return axp20x_usb_power_set_current_max(power,
>val->intval);
>>
>> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
>> + return
>axp20x_usb_power_set_input_current_limit(power, val->intval);
>> +
>
>So I think there are two things that should be adjusted.
>
>First, we should be using POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT for all
>PMICs.
>As far as the sysfs documents go, CURRENT_MAX is read-only, and should
>refer to
>the hard limit the hardware can support, i.e. maximum power ratings.
>INPUT_CURRENT_LIMIT and INPUT_VOLTAGE_LIMIT are for configurable upper
>and lower
>limits respectively.
>
>Sebastian, is my understanding of this correct?
>
>We already use INPUT_CURRENT_LIMIT for the AXP813 in the axp20x-ac
>driver, and
>it would be nice to have both drivers expose the same attributes.
>
>Second, since the value set in register 0x35 is the one that actually
>has an
>effect, as opposed to just being a default, we should just use that
>one.
However, that default value is also important, otherwise users will
get dropped back to 500mAh each time they re-insert USB jack.
Is there a property to export the default value?
BTW, if possible, apply patch 1 first, because it can raise current to 1.5A
in the default situation.
>
>Could you restructure the series based on what I described, with a new
>patch 1
>switching from CURRENT_MAX to INPUT_CURRENT_LIMIT, and then this patch
>as patch 2?
>And both patches should have Fixes tags and possibly CC stable so they
>get backported
>for people that are using stable kernels? And then the original patch
>2 as patch 3.
>
>ChenYu
>
>> default:
>> return -EINVAL;
>> }
>> @@ -365,7 +465,8 @@ static int axp20x_usb_power_prop_writeable(struct
>power_supply *psy,
>>enum power_supply_property
>psp)
>> {
>> return psp == POWER_SUPPLY_PROP_VOLTAGE_MIN ||
>> - psp == POWER_SUPPLY_PROP_CURRENT_MAX;
>> + psp == POWER_SUPPLY_PROP_CURRENT_MAX ||
>> + psp == POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT;
>> }
>>
>> static enum power_supply_property axp20x_usb_power_properties[] = {
>> @@ -386,6 +487,15 @@ static enum power_supply_property
>axp22x_usb_power_properties[] = {
>> POWER_SUPPLY_PROP_CURRENT_MAX,
>> };
>>
>> +static enum power_supply_property axp813_usb_power_properties[] = {
>> + POWER_SUPPLY_PROP_HEALTH,
>> + POWER_SUPPLY_PROP_PRESENT,
>> + POWER_SUPPLY_PROP_ONLINE,
>> + POWER_SUPPLY_PROP_VOLTAGE_MIN,
>> + POWER_SUPPLY_PROP_CURRENT_MAX,
>> + POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
>> +};
>> +
>> static const struct power_supply_desc axp20x_usb_power_desc = {
>> .name = "axp20x-usb",
>> .type = POWER_SUPPLY_TYPE_USB,
>> @@ -406,6 +516,16 @@ static const struct power_supply_desc
>axp22x_usb_power_desc = {
>> .set_property = axp20x_usb_power_set_property,
>> };
>>
>> +static const struct power_supply_desc axp813_usb_power_desc = {
>> + .name = "axp20x-usb",
>> + .type = POWER_SUPPLY_TYPE_USB,
>> + .properties = axp813_usb_power_properties,
>> + .num_properties = ARRAY_SIZE(axp813_usb_power_properties),
>> + .property_is_writeable = axp20x_usb_power_prop_writeable,
>> + .get_property = axp20x_usb_power_get_property,
>> + .set_property = axp20x_usb_power_set_property,
>> +};
>> +
>> static int configure_iio_channels(struct platform_device *pdev,
>> struct axp20x_usb_power *power)
>> {
>> @@ -487,10 +607,16 @@ static int axp20x_usb_power_probe(struct
>platform_device *pdev)
>> usb_power_desc = &axp20x_usb_power_desc;
>> irq_names = axp20x_irq_names;
>> } else if (power->axp20x_id == AX
Re: [linux-sunxi] [PATCH 2/2] power: supply: axp20x_usb_power: add applied max Vbus support for AXP813
Hi,
On Wed, Oct 2, 2019 at 7:27 PM Icenowy Zheng wrote:
>
> AXP813 PMIC has two Vbus maximum value settings -- one is the default
> value, which is currently the only supported one; the other is the
> really applied value, which is set according to the default value if the
> BC detection module detected a charging port, or 500mA if no charging
> port is detected.
>
> Add support for reading and writing of the really applied Vbus maxmium
> value. Interestingly it has a larger range than the default value.
>
> Signed-off-by: Icenowy Zheng
> ---
> drivers/power/supply/axp20x_usb_power.c | 132 +++-
> 1 file changed, 129 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/power/supply/axp20x_usb_power.c
> b/drivers/power/supply/axp20x_usb_power.c
> index 5f0a5722b19e..905668a2727f 100644
> --- a/drivers/power/supply/axp20x_usb_power.c
> +++ b/drivers/power/supply/axp20x_usb_power.c
[...]
> @@ -354,6 +451,9 @@ static int axp20x_usb_power_set_property(struct
> power_supply *psy,
> val->intval);
> return axp20x_usb_power_set_current_max(power, val->intval);
>
> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> + return axp20x_usb_power_set_input_current_limit(power,
> val->intval);
> +
So I think there are two things that should be adjusted.
First, we should be using POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT for all PMICs.
As far as the sysfs documents go, CURRENT_MAX is read-only, and should refer to
the hard limit the hardware can support, i.e. maximum power ratings.
INPUT_CURRENT_LIMIT and INPUT_VOLTAGE_LIMIT are for configurable upper and lower
limits respectively.
Sebastian, is my understanding of this correct?
We already use INPUT_CURRENT_LIMIT for the AXP813 in the axp20x-ac driver, and
it would be nice to have both drivers expose the same attributes.
Second, since the value set in register 0x35 is the one that actually has an
effect, as opposed to just being a default, we should just use that one.
Could you restructure the series based on what I described, with a new patch 1
switching from CURRENT_MAX to INPUT_CURRENT_LIMIT, and then this patch
as patch 2?
And both patches should have Fixes tags and possibly CC stable so they
get backported
for people that are using stable kernels? And then the original patch
2 as patch 3.
ChenYu
> default:
> return -EINVAL;
> }
> @@ -365,7 +465,8 @@ static int axp20x_usb_power_prop_writeable(struct
> power_supply *psy,
>enum power_supply_property psp)
> {
> return psp == POWER_SUPPLY_PROP_VOLTAGE_MIN ||
> - psp == POWER_SUPPLY_PROP_CURRENT_MAX;
> + psp == POWER_SUPPLY_PROP_CURRENT_MAX ||
> + psp == POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT;
> }
>
> static enum power_supply_property axp20x_usb_power_properties[] = {
> @@ -386,6 +487,15 @@ static enum power_supply_property
> axp22x_usb_power_properties[] = {
> POWER_SUPPLY_PROP_CURRENT_MAX,
> };
>
> +static enum power_supply_property axp813_usb_power_properties[] = {
> + POWER_SUPPLY_PROP_HEALTH,
> + POWER_SUPPLY_PROP_PRESENT,
> + POWER_SUPPLY_PROP_ONLINE,
> + POWER_SUPPLY_PROP_VOLTAGE_MIN,
> + POWER_SUPPLY_PROP_CURRENT_MAX,
> + POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> +};
> +
> static const struct power_supply_desc axp20x_usb_power_desc = {
> .name = "axp20x-usb",
> .type = POWER_SUPPLY_TYPE_USB,
> @@ -406,6 +516,16 @@ static const struct power_supply_desc
> axp22x_usb_power_desc = {
> .set_property = axp20x_usb_power_set_property,
> };
>
> +static const struct power_supply_desc axp813_usb_power_desc = {
> + .name = "axp20x-usb",
> + .type = POWER_SUPPLY_TYPE_USB,
> + .properties = axp813_usb_power_properties,
> + .num_properties = ARRAY_SIZE(axp813_usb_power_properties),
> + .property_is_writeable = axp20x_usb_power_prop_writeable,
> + .get_property = axp20x_usb_power_get_property,
> + .set_property = axp20x_usb_power_set_property,
> +};
> +
> static int configure_iio_channels(struct platform_device *pdev,
> struct axp20x_usb_power *power)
> {
> @@ -487,10 +607,16 @@ static int axp20x_usb_power_probe(struct
> platform_device *pdev)
> usb_power_desc = &axp20x_usb_power_desc;
> irq_names = axp20x_irq_names;
> } else if (power->axp20x_id == AXP221_ID ||
> - power->axp20x_id == AXP223_ID ||
> - power->axp20x_id == AXP813_ID) {
> + power->axp20x_id == AXP223_ID) {
> usb_power_desc = &axp22x_usb_power_desc;
> irq_names = axp22x_irq_names;
> + } else if (power->axp20x_id == AXP813_ID) {
> + usb_power_desc = &axp813_usb_power_desc;
> + irq_names = axp22x_irq_names;
> +
> +
[PATCH 2/2] power: supply: axp20x_usb_power: add applied max Vbus support for AXP813
AXP813 PMIC has two Vbus maximum value settings -- one is the default
value, which is currently the only supported one; the other is the
really applied value, which is set according to the default value if the
BC detection module detected a charging port, or 500mA if no charging
port is detected.
Add support for reading and writing of the really applied Vbus maxmium
value. Interestingly it has a larger range than the default value.
Signed-off-by: Icenowy Zheng
---
drivers/power/supply/axp20x_usb_power.c | 132 +++-
1 file changed, 129 insertions(+), 3 deletions(-)
diff --git a/drivers/power/supply/axp20x_usb_power.c
b/drivers/power/supply/axp20x_usb_power.c
index 5f0a5722b19e..905668a2727f 100644
--- a/drivers/power/supply/axp20x_usb_power.c
+++ b/drivers/power/supply/axp20x_usb_power.c
@@ -50,6 +50,18 @@
#define AXP813_BC_EN BIT(0)
+#define AXP813_VBUS_CLIMIT_REAL_MASK GENMASK(7, 4)
+#define AXP813_VBUS_CLIMIT_REAL_100mA (0 << 4)
+#define AXP813_VBUS_CLIMIT_REAL_500mA (1 << 4)
+#define AXP813_VBUS_CLIMIT_REAL_900mA (2 << 4)
+#define AXP813_VBUS_CLIMIT_REAL_1500mA (3 << 4)
+#define AXP813_VBUS_CLIMIT_REAL_2000mA (4 << 4)
+#define AXP813_VBUS_CLIMIT_REAL_2500mA (5 << 4)
+#define AXP813_VBUS_CLIMIT_REAL_3000mA (6 << 4)
+#define AXP813_VBUS_CLIMIT_REAL_3500mA (7 << 4)
+#define AXP813_VBUS_CLIMIT_REAL_4000mA (8 << 4)
+/* The remaining values are all 4000mA according to the datasheet */
+
/*
* Note do not raise the debounce time, we must report Vusb high within
* 100ms otherwise we get Vbus errors in musb.
@@ -159,6 +171,47 @@ static int axp813_get_current_max(struct axp20x_usb_power
*power, int *val)
return 0;
}
+static int axp813_get_input_current_limit(struct axp20x_usb_power *power, int
*val)
+{
+ unsigned int v;
+ int ret = regmap_read(power->regmap, AXP22X_CHRG_CTRL3, &v);
+
+ if (ret)
+ return ret;
+
+ switch (v & AXP813_VBUS_CLIMIT_REAL_MASK) {
+ case AXP813_VBUS_CLIMIT_REAL_100mA:
+ *val = 10;
+ break;
+ case AXP813_VBUS_CLIMIT_REAL_500mA:
+ *val = 50;
+ break;
+ case AXP813_VBUS_CLIMIT_REAL_900mA:
+ *val = 90;
+ break;
+ case AXP813_VBUS_CLIMIT_REAL_1500mA:
+ *val = 150;
+ break;
+ case AXP813_VBUS_CLIMIT_REAL_2000mA:
+ *val = 200;
+ break;
+ case AXP813_VBUS_CLIMIT_REAL_2500mA:
+ *val = 250;
+ break;
+ case AXP813_VBUS_CLIMIT_REAL_3000mA:
+ *val = 300;
+ break;
+ case AXP813_VBUS_CLIMIT_REAL_3500mA:
+ *val = 350;
+ break;
+ default:
+ /* All other cases are 4000mA */
+ *val = 400;
+ break;
+ }
+ return 0;
+}
+
static int axp20x_usb_power_get_property(struct power_supply *psy,
enum power_supply_property psp, union power_supply_propval *val)
{
@@ -200,6 +253,8 @@ static int axp20x_usb_power_get_property(struct
power_supply *psy,
if (power->axp20x_id == AXP813_ID)
return axp813_get_current_max(power, &val->intval);
return axp20x_get_current_max(power, &val->intval);
+ case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ return axp813_get_input_current_limit(power, &val->intval);
case POWER_SUPPLY_PROP_CURRENT_NOW:
if (IS_ENABLED(CONFIG_AXP20X_ADC)) {
ret = iio_read_channel_processed(power->vbus_i,
@@ -338,6 +393,48 @@ static int axp20x_usb_power_set_current_max(struct
axp20x_usb_power *power,
return -EINVAL;
}
+static int axp20x_usb_power_set_input_current_limit(struct axp20x_usb_power
*power,
+ int intval)
+{
+ int val;
+
+ switch (intval) {
+ case 10:
+ val = AXP813_VBUS_CLIMIT_REAL_100mA;
+ break;
+ case 50:
+ val = AXP813_VBUS_CLIMIT_REAL_500mA;
+ break;
+ case 90:
+ val = AXP813_VBUS_CLIMIT_REAL_900mA;
+ break;
+ case 150:
+ val = AXP813_VBUS_CLIMIT_REAL_1500mA;
+ break;
+ case 200:
+ val = AXP813_VBUS_CLIMIT_REAL_2000mA;
+ break;
+ case 250:
+ val = AXP813_VBUS_CLIMIT_REAL_2500mA;
+ break;
+ case 300:
+ val = AXP813_VBUS_CLIMIT_REAL_3000mA;
+ break;
+ case 350:
+ val = AXP813_VBUS_CLIMIT_REAL_3500mA;
+ break;
+ case 400:
+ val = AXP813_VBUS_CLIMIT_REAL_4000mA;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return regmap_update_bits(power->regmap,
+
