linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Enable USB BC detection to raise AXP813 Vbus current
@ 2019-10-02 11:25 Icenowy Zheng
  2019-10-02 11:25 ` [PATCH 1/2] power: supply: axp20x_usb_power: enable USB BC detection on AXP813 Icenowy Zheng
  2019-10-02 11:25 ` [PATCH 2/2] power: supply: axp20x_usb_power: add applied max Vbus support for AXP813 Icenowy Zheng
  0 siblings, 2 replies; 8+ messages in thread
From: Icenowy Zheng @ 2019-10-02 11:25 UTC (permalink / raw)
  To: Sebastian Reichel, Chen-Yu Tsai
  Cc: linux-pm, linux-kernel, linux-sunxi, Icenowy Zheng

Unlike previous AXP PMICs, the AXP813 PMIC (and AXP803) supports port
detection defined in USB Battery Charging Specification 1.2, and sets
the real Vbus current based on a pre-defined value (which is the
original Vbus current limitation field) and the port status. However,
the detection needs manual activision. If it's not active, the PMIC will
assume a SDP and limit the Vbus current to 500mA.

This patchset contains two patches, one enables the USB BC 1.2
detection, the other exports the real applied Vbus limitation.

Icenowy Zheng (2):
  power: supply: axp20x_usb_power: enable USB BC detection on AXP813
  power: supply: axp20x_usb_power: add applied max Vbus support for
    AXP813

 drivers/power/supply/axp20x_usb_power.c | 140 +++++++++++++++++++++++-
 1 file changed, 137 insertions(+), 3 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] power: supply: axp20x_usb_power: enable USB BC detection on AXP813
  2019-10-02 11:25 [PATCH 0/2] Enable USB BC detection to raise AXP813 Vbus current Icenowy Zheng
@ 2019-10-02 11:25 ` Icenowy Zheng
  2019-10-14  4:22   ` Sebastian Reichel
  2019-10-02 11:25 ` [PATCH 2/2] power: supply: axp20x_usb_power: add applied max Vbus support for AXP813 Icenowy Zheng
  1 sibling, 1 reply; 8+ messages in thread
From: Icenowy Zheng @ 2019-10-02 11:25 UTC (permalink / raw)
  To: Sebastian Reichel, Chen-Yu Tsai
  Cc: linux-pm, linux-kernel, linux-sunxi, Icenowy Zheng

The AXP813 PMIC has support for detection of USB Battery Charging
specification, and it will limit the current to 500mA by default when
the detection is not enabled or the detection result is SDP.

Enable the BC detection to allow correctly selection of the current.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
 drivers/power/supply/axp20x_usb_power.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c
index dc4c316eff81..5f0a5722b19e 100644
--- a/drivers/power/supply/axp20x_usb_power.c
+++ b/drivers/power/supply/axp20x_usb_power.c
@@ -48,6 +48,8 @@
 
 #define AXP20X_VBUS_MON_VBUS_VALID	BIT(3)
 
+#define AXP813_BC_EN		BIT(0)
+
 /*
  * Note do not raise the debounce time, we must report Vusb high within
  * 100ms otherwise we get Vbus errors in musb.
@@ -495,6 +497,12 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	if (power->axp20x_id == AXP813_ID) {
+		/* Enable USB Battery Charging specification detection */
+		regmap_update_bits(axp20x->regmap, AXP288_BC_GLOBAL,
+				   AXP813_BC_EN, AXP813_BC_EN);
+	}
+
 	psy_cfg.of_node = pdev->dev.of_node;
 	psy_cfg.drv_data = power;
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] power: supply: axp20x_usb_power: add applied max Vbus support for AXP813
  2019-10-02 11:25 [PATCH 0/2] Enable USB BC detection to raise AXP813 Vbus current Icenowy Zheng
  2019-10-02 11:25 ` [PATCH 1/2] power: supply: axp20x_usb_power: enable USB BC detection on AXP813 Icenowy Zheng
@ 2019-10-02 11:25 ` Icenowy Zheng
  2019-10-07 16:07   ` [linux-sunxi] " Chen-Yu Tsai
  1 sibling, 1 reply; 8+ messages in thread
From: Icenowy Zheng @ 2019-10-02 11:25 UTC (permalink / raw)
  To: Sebastian Reichel, Chen-Yu Tsai
  Cc: linux-pm, linux-kernel, linux-sunxi, Icenowy Zheng

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 <icenowy@aosc.io>
---
 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 = 100000;
+		break;
+	case AXP813_VBUS_CLIMIT_REAL_500mA:
+		*val = 500000;
+		break;
+	case AXP813_VBUS_CLIMIT_REAL_900mA:
+		*val = 900000;
+		break;
+	case AXP813_VBUS_CLIMIT_REAL_1500mA:
+		*val = 1500000;
+		break;
+	case AXP813_VBUS_CLIMIT_REAL_2000mA:
+		*val = 2000000;
+		break;
+	case AXP813_VBUS_CLIMIT_REAL_2500mA:
+		*val = 2500000;
+		break;
+	case AXP813_VBUS_CLIMIT_REAL_3000mA:
+		*val = 3000000;
+		break;
+	case AXP813_VBUS_CLIMIT_REAL_3500mA:
+		*val = 3500000;
+		break;
+	default:
+		/* All other cases are 4000mA */
+		*val = 4000000;
+		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 100000:
+		val = AXP813_VBUS_CLIMIT_REAL_100mA;
+		break;
+	case 500000:
+		val = AXP813_VBUS_CLIMIT_REAL_500mA;
+		break;
+	case 900000:
+		val = AXP813_VBUS_CLIMIT_REAL_900mA;
+		break;
+	case 1500000:
+		val = AXP813_VBUS_CLIMIT_REAL_1500mA;
+		break;
+	case 2000000:
+		val = AXP813_VBUS_CLIMIT_REAL_2000mA;
+		break;
+	case 2500000:
+		val = AXP813_VBUS_CLIMIT_REAL_2500mA;
+		break;
+	case 3000000:
+		val = AXP813_VBUS_CLIMIT_REAL_3000mA;
+		break;
+	case 3500000:
+		val = AXP813_VBUS_CLIMIT_REAL_3500mA;
+		break;
+	case 4000000:
+		val = AXP813_VBUS_CLIMIT_REAL_4000mA;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return regmap_update_bits(power->regmap,
+				  AXP22X_CHRG_CTRL3,
+				  AXP813_VBUS_CLIMIT_REAL_MASK, val);
+}
+
 static int axp20x_usb_power_set_property(struct power_supply *psy,
 					 enum power_supply_property psp,
 					 const union power_supply_propval *val)
@@ -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);
+
 	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;
+
+		/* Enable USB Battery Charging specification detection */
+		regmap_update_bits(axp20x->regmap, AXP288_BC_GLOBAL,
+				   AXP813_BC_EN, AXP813_BC_EN);
 	} else {
 		dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
 			axp20x->variant);
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [linux-sunxi] [PATCH 2/2] power: supply: axp20x_usb_power: add applied max Vbus support for AXP813
  2019-10-02 11:25 ` [PATCH 2/2] power: supply: axp20x_usb_power: add applied max Vbus support for AXP813 Icenowy Zheng
@ 2019-10-07 16:07   ` Chen-Yu Tsai
  2019-10-08  3:09     ` Icenowy Zheng
  2019-10-14  4:24     ` Sebastian Reichel
  0 siblings, 2 replies; 8+ messages in thread
From: Chen-Yu Tsai @ 2019-10-07 16:07 UTC (permalink / raw)
  To: Icenowy Zheng, Sebastian Reichel
  Cc: open list:THERMAL, linux-kernel, linux-sunxi

Hi,

On Wed, Oct 2, 2019 at 7:27 PM Icenowy Zheng <icenowy@aosc.io> 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 <icenowy@aosc.io>
> ---
>  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;
> +
> +               /* Enable USB Battery Charging specification detection */
> +               regmap_update_bits(axp20x->regmap, AXP288_BC_GLOBAL,
> +                                  AXP813_BC_EN, AXP813_BC_EN);

This seems like a duplicate of

>         } else {
>                 dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
>                         axp20x->variant);
> --
> 2.21.0
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20191002112545.58481-3-icenowy%40aosc.io.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [linux-sunxi] [PATCH 2/2] power: supply: axp20x_usb_power: add applied max Vbus support for AXP813
  2019-10-07 16:07   ` [linux-sunxi] " Chen-Yu Tsai
@ 2019-10-08  3:09     ` Icenowy Zheng
  2019-10-08  3:14       ` Chen-Yu Tsai
  2019-10-14  4:24     ` Sebastian Reichel
  1 sibling, 1 reply; 8+ messages in thread
From: Icenowy Zheng @ 2019-10-08  3:09 UTC (permalink / raw)
  To: Chen-Yu Tsai, Sebastian Reichel
  Cc: open list:THERMAL, linux-kernel, linux-sunxi



于 2019年10月8日 GMT+08:00 上午12:07:05, Chen-Yu Tsai <wens@csie.org> 写到:
>Hi,
>
>On Wed, Oct 2, 2019 at 7:27 PM Icenowy Zheng <icenowy@aosc.io> 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 <icenowy@aosc.io>
>> ---
>>  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 == 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;
>> +
>> +               /* Enable USB Battery Charging specification
>detection */
>> +               regmap_update_bits(axp20x->regmap, AXP288_BC_GLOBAL,
>> +                                  AXP813_BC_EN, AXP813_BC_EN);
>
>This seems like a duplicate of
>
>>         } else {
>>                 dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
>>                         axp20x->variant);
>> --
>> 2.21.0
>>
>> --
>> You received this message because you are subscribed to the Google
>Groups "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it,
>send an email to linux-sunxi+unsubscribe@googlegroups.com.
>> To view this discussion on the web, visit
>https://groups.google.com/d/msgid/linux-sunxi/20191002112545.58481-3-icenowy%40aosc.io.

-- 
使用 K-9 Mail 发送自我的Android设备。

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [linux-sunxi] [PATCH 2/2] power: supply: axp20x_usb_power: add applied max Vbus support for AXP813
  2019-10-08  3:09     ` Icenowy Zheng
@ 2019-10-08  3:14       ` Chen-Yu Tsai
  0 siblings, 0 replies; 8+ messages in thread
From: Chen-Yu Tsai @ 2019-10-08  3:14 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Sebastian Reichel, open list:THERMAL, linux-kernel, linux-sunxi

On Tue, Oct 8, 2019 at 11:09 AM Icenowy Zheng <icenowy@aosc.io> wrote:
> 于 2019年10月8日 GMT+08:00 上午12:07:05, Chen-Yu Tsai <wens@csie.org> 写到:
> >Hi,
> >
> >On Wed, Oct 2, 2019 at 7:27 PM Icenowy Zheng <icenowy@aosc.io> 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 <icenowy@aosc.io>
> >> ---
> >>  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_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;
> >> +
> >> +               /* Enable USB Battery Charging specification
> >detection */
> >> +               regmap_update_bits(axp20x->regmap, AXP288_BC_GLOBAL,
> >> +                                  AXP813_BC_EN, AXP813_BC_EN);
> >
> >This seems like a duplicate of
> >
> >>         } else {
> >>                 dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
> >>                         axp20x->variant);
> >> --
> >> 2.21.0
> >>
> >> --
> >> You received this message because you are subscribed to the Google
> >Groups "linux-sunxi" group.
> >> To unsubscribe from this group and stop receiving emails from it,
> >send an email to linux-sunxi+unsubscribe@googlegroups.com.
> >> To view this discussion on the web, visit
> >https://groups.google.com/d/msgid/linux-sunxi/20191002112545.58481-3-icenowy%40aosc.io.
>
> --
> 使用 K-9 Mail 发送自我的Android设备。
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/A6B1EF32-5CDE-4AAE-A7DA-F9A636BE7BF3%40aosc.io.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] power: supply: axp20x_usb_power: enable USB BC detection on AXP813
  2019-10-02 11:25 ` [PATCH 1/2] power: supply: axp20x_usb_power: enable USB BC detection on AXP813 Icenowy Zheng
@ 2019-10-14  4:22   ` Sebastian Reichel
  0 siblings, 0 replies; 8+ messages in thread
From: Sebastian Reichel @ 2019-10-14  4:22 UTC (permalink / raw)
  To: Icenowy Zheng; +Cc: Chen-Yu Tsai, linux-pm, linux-kernel, linux-sunxi

[-- Attachment #1: Type: text/plain, Size: 1495 bytes --]

Hi,

On Wed, Oct 02, 2019 at 07:25:44PM +0800, Icenowy Zheng wrote:
> The AXP813 PMIC has support for detection of USB Battery Charging
> specification, and it will limit the current to 500mA by default when
> the detection is not enabled or the detection result is SDP.
> 
> Enable the BC detection to allow correctly selection of the current.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---

Thanks, queued to power-supply-next.

-- Sebastian

>  drivers/power/supply/axp20x_usb_power.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c
> index dc4c316eff81..5f0a5722b19e 100644
> --- a/drivers/power/supply/axp20x_usb_power.c
> +++ b/drivers/power/supply/axp20x_usb_power.c
> @@ -48,6 +48,8 @@
>  
>  #define AXP20X_VBUS_MON_VBUS_VALID	BIT(3)
>  
> +#define AXP813_BC_EN		BIT(0)
> +
>  /*
>   * Note do not raise the debounce time, we must report Vusb high within
>   * 100ms otherwise we get Vbus errors in musb.
> @@ -495,6 +497,12 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> +	if (power->axp20x_id == AXP813_ID) {
> +		/* Enable USB Battery Charging specification detection */
> +		regmap_update_bits(axp20x->regmap, AXP288_BC_GLOBAL,
> +				   AXP813_BC_EN, AXP813_BC_EN);
> +	}
> +
>  	psy_cfg.of_node = pdev->dev.of_node;
>  	psy_cfg.drv_data = power;
>  
> -- 
> 2.21.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [linux-sunxi] [PATCH 2/2] power: supply: axp20x_usb_power: add applied max Vbus support for AXP813
  2019-10-07 16:07   ` [linux-sunxi] " Chen-Yu Tsai
  2019-10-08  3:09     ` Icenowy Zheng
@ 2019-10-14  4:24     ` Sebastian Reichel
  1 sibling, 0 replies; 8+ messages in thread
From: Sebastian Reichel @ 2019-10-14  4:24 UTC (permalink / raw)
  To: Chen-Yu Tsai; +Cc: Icenowy Zheng, open list:THERMAL, linux-kernel, linux-sunxi

[-- Attachment #1: Type: text/plain, Size: 412 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-10-14 21:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-02 11:25 [PATCH 0/2] Enable USB BC detection to raise AXP813 Vbus current Icenowy Zheng
2019-10-02 11:25 ` [PATCH 1/2] power: supply: axp20x_usb_power: enable USB BC detection on AXP813 Icenowy Zheng
2019-10-14  4:22   ` Sebastian Reichel
2019-10-02 11:25 ` [PATCH 2/2] power: supply: axp20x_usb_power: add applied max Vbus support for AXP813 Icenowy Zheng
2019-10-07 16:07   ` [linux-sunxi] " Chen-Yu Tsai
2019-10-08  3:09     ` Icenowy Zheng
2019-10-08  3:14       ` Chen-Yu Tsai
2019-10-14  4:24     ` Sebastian Reichel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).