[RFC PATCH 0/7] AXP803 AC/Battery support

[RFC PATCH 0/7] AXP803 AC/Battery support

[Icenowy Zheng]

The AXP803 PMIC, used by most Allwinner A64 boards, features 3 power inputs:
AC, USB and Battery.

This patchset adds support for the AC and Battery supplies, which is useful
for the boards from Pine64 (Pine64, SoPine w/ baseboard model A, Pinebook).

The USB supply is not yet supported in this patchset because it's not
present on Pine series boards.

In order to enable battery monitoring the ADC for battery is also enabled
for AXs.

In order to enable battery monitoring the ADC for battery is also enabled
for AXP803.

Icenowy Zheng (7):
  dt-bindings: add compatibles for AXP803 Battery/USB power supplies
  iio: adc: axp20x-adc: allow to skip ADC rate setup now
  iio: adc: axp20x-adc: add support for AXP803
  power: supply: axp20x-battery: support AXP803
  mfd: axp20x: add cells for AXP803 ADC/AC Power/Battery
  arm64: allwinner: a64: add power supply nodes in AXP803 DTSI
  arm64: allwinner: a64: enable AC and Battery for Pine64

 .../bindings/power/supply/axp20x_battery.txt       |   1 +
 .../bindings/power/supply/axp20x_usb_power.txt     |   1 +
 arch/arm64/boot/dts/allwinner/axp803.dtsi          |  15 +++
 .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts |   8 ++
 drivers/iio/adc/axp20x_adc.c                       | 114 ++++++++++++++++++++-
 drivers/mfd/axp20x.c                               |  11 ++
 drivers/power/supply/axp20x_battery.c              |  88 ++++++++++++++--
 7 files changed, 226 insertions(+), 12 deletions(-)

-- 
2.13.5

[RFC PATCH 4/7] power: supply: axp20x-battery: support AXP803

[Icenowy Zheng]

The AXP803 PMIC has battery support like other AXP PMICs, but with
different definition of max target charging voltage and constant
charging current.

Add support for AXP803 battery in axp20x-battery driver.

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

diff --git a/drivers/power/supply/axp20x_battery.c b/drivers/power/supply/axp20x_battery.c
index 7494f0f0eadb..c9a9fb320c92 100644
--- a/drivers/power/supply/axp20x_battery.c
+++ b/drivers/power/supply/axp20x_battery.c
@@ -49,6 +49,8 @@
 #define AXP22X_CHRG_CTRL1_TGT_4_22V	(1 << 5)
 #define AXP22X_CHRG_CTRL1_TGT_4_24V	(3 << 5)
 
+#define AXP803_CHRG_CTRL1_TGT_4_35V	(3 << 5)
+
 #define AXP20X_CHRG_CTRL1_TGT_CURR	GENMASK(3, 0)
 
 #define AXP20X_V_OFF_MASK		GENMASK(2, 0)
@@ -123,20 +125,71 @@ static int axp22x_battery_get_max_voltage(struct axp20x_batt_ps *axp20x_batt,
 	return 0;
 }
 
+static int axp803_battery_get_max_voltage(struct axp20x_batt_ps *axp20x_batt,
+					  int *val)
+{
+	int ret, reg;
+
+	ret = regmap_read(axp20x_batt->regmap, AXP20X_CHRG_CTRL1, &reg);
+	if (ret)
+		return ret;
+
+	switch (reg & AXP20X_CHRG_CTRL1_TGT_VOLT) {
+	case AXP20X_CHRG_CTRL1_TGT_4_1V:
+		*val = 4100000;
+		break;
+	case AXP20X_CHRG_CTRL1_TGT_4_15V:
+		*val = 4150000;
+		break;
+	case AXP20X_CHRG_CTRL1_TGT_4_2V:
+		*val = 4200000;
+		break;
+	case AXP803_CHRG_CTRL1_TGT_4_35V:
+		*val = 4350000;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static void raw_to_constant_charge_current(struct axp20x_batt_ps *axp, int *val)
 {
-	if (axp->axp_id == AXP209_ID)
+	switch (axp->axp_id) {
+	case AXP209_ID:
 		*val = *val * 100000 + 300000;
-	else
+		break;
+	case AXP221_ID:
 		*val = *val * 150000 + 300000;
+		break;
+	case AXP803_ID:
+		*val = *val * 200000 + 200000;
+		break;
+	}
 }
 
 static void constant_charge_current_to_raw(struct axp20x_batt_ps *axp, int *val)
 {
-	if (axp->axp_id == AXP209_ID)
+	switch (axp->axp_id) {
+	case AXP209_ID:
 		*val = (*val - 300000) / 100000;
-	else
+		break;
+	case AXP221_ID:
 		*val = (*val - 300000) / 150000;
+		break;
+	case AXP803_ID:
+		*val = (*val - 200000) / 200000;
+		/*
+		 * The maximum charge current on AXP803 is 2.8A, and the
+		 * datasheet says "1110-1111 reserved" in this part.
+		 * So we return an invalid value -1 in this situation,
+		 * which will be dealed by the caller of this function,
+		 */
+		if (*val > 13)
+			*val = -1;
+		break;
+	}
 }
 
 static int axp20x_get_constant_charge_current(struct axp20x_batt_ps *axp,
@@ -269,9 +322,13 @@ static int axp20x_battery_get_prop(struct power_supply *psy,
 		if (ret)
 			return ret;
 
-		if (axp20x_batt->axp_id == AXP221_ID &&
-		    !(reg & AXP22X_FG_VALID))
-			return -EINVAL;
+		switch (axp20x_batt->axp_id) {
+		case AXP221_ID:
+		case AXP803_ID:
+			if (!(reg & AXP22X_FG_VALID))
+				return -EINVAL;
+			break;
+		};
 
 		/*
 		 * Fuel Gauge data takes 7 bits but the stored value seems to be
@@ -281,11 +338,19 @@ static int axp20x_battery_get_prop(struct power_supply *psy,
 		break;
 
 	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
-		if (axp20x_batt->axp_id == AXP209_ID)
+		switch (axp20x_batt->axp_id) {
+		case AXP209_ID:
 			return axp20x_battery_get_max_voltage(axp20x_batt,
 							      &val->intval);
-		return axp22x_battery_get_max_voltage(axp20x_batt,
-						      &val->intval);
+		case AXP221_ID:
+			return axp22x_battery_get_max_voltage(axp20x_batt,
+							      &val->intval);
+		case AXP803_ID:
+			return axp803_battery_get_max_voltage(axp20x_batt,
+							      &val->intval);
+		default:
+			return -EINVAL;
+		}
 
 	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
 		ret = regmap_read(axp20x_batt->regmap, AXP20X_V_OFF, &reg);
@@ -467,6 +532,9 @@ static const struct of_device_id axp20x_battery_ps_id[] = {
 	}, {
 		.compatible = "x-powers,axp221-battery-power-supply",
 		.data = (void *)AXP221_ID,
+	}, {
+		.compatible = "x-powers,axp803-battery-power-supply",
+		.data = (void *)AXP803_ID,
 	}, { /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, axp20x_battery_ps_id);
-- 
2.13.5

Re: [RFC PATCH 4/7] power: supply: axp20x-battery: support AXP803

[Jonathan Cameron]

On Wed, 20 Sep 2017 23:18:11 +0800
Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote:

> The AXP803 PMIC has battery support like other AXP PMICs, but with
> different definition of max target charging voltage and constant
> charging current.
> 
> Add support for AXP803 battery in axp20x-battery driver.
> 
> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>

This looks fine to me, but I haven't dived into the precise values
etc.

Jonathan

> ---
>  drivers/power/supply/axp20x_battery.c | 88 +++++++++++++++++++++++++++++++----
>  1 file changed, 78 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/power/supply/axp20x_battery.c b/drivers/power/supply/axp20x_battery.c
> index 7494f0f0eadb..c9a9fb320c92 100644
> --- a/drivers/power/supply/axp20x_battery.c
> +++ b/drivers/power/supply/axp20x_battery.c
> @@ -49,6 +49,8 @@
>  #define AXP22X_CHRG_CTRL1_TGT_4_22V	(1 << 5)
>  #define AXP22X_CHRG_CTRL1_TGT_4_24V	(3 << 5)
>  
> +#define AXP803_CHRG_CTRL1_TGT_4_35V	(3 << 5)
> +
>  #define AXP20X_CHRG_CTRL1_TGT_CURR	GENMASK(3, 0)
>  
>  #define AXP20X_V_OFF_MASK		GENMASK(2, 0)
> @@ -123,20 +125,71 @@ static int axp22x_battery_get_max_voltage(struct axp20x_batt_ps *axp20x_batt,
>  	return 0;
>  }
>  
> +static int axp803_battery_get_max_voltage(struct axp20x_batt_ps *axp20x_batt,
> +					  int *val)
> +{
> +	int ret, reg;
> +
> +	ret = regmap_read(axp20x_batt->regmap, AXP20X_CHRG_CTRL1, &reg);
> +	if (ret)
> +		return ret;
> +
> +	switch (reg & AXP20X_CHRG_CTRL1_TGT_VOLT) {
> +	case AXP20X_CHRG_CTRL1_TGT_4_1V:
> +		*val = 4100000;
> +		break;
> +	case AXP20X_CHRG_CTRL1_TGT_4_15V:
> +		*val = 4150000;
> +		break;
> +	case AXP20X_CHRG_CTRL1_TGT_4_2V:
> +		*val = 4200000;
> +		break;
> +	case AXP803_CHRG_CTRL1_TGT_4_35V:
> +		*val = 4350000;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static void raw_to_constant_charge_current(struct axp20x_batt_ps *axp, int *val)
>  {
> -	if (axp->axp_id == AXP209_ID)
> +	switch (axp->axp_id) {
> +	case AXP209_ID:
>  		*val = *val * 100000 + 300000;
> -	else
> +		break;
> +	case AXP221_ID:
>  		*val = *val * 150000 + 300000;
> +		break;
> +	case AXP803_ID:
> +		*val = *val * 200000 + 200000;
> +		break;
> +	}
>  }
>  
>  static void constant_charge_current_to_raw(struct axp20x_batt_ps *axp, int *val)
>  {
> -	if (axp->axp_id == AXP209_ID)
> +	switch (axp->axp_id) {
> +	case AXP209_ID:
>  		*val = (*val - 300000) / 100000;
> -	else
> +		break;
> +	case AXP221_ID:
>  		*val = (*val - 300000) / 150000;
> +		break;
> +	case AXP803_ID:
> +		*val = (*val - 200000) / 200000;
> +		/*
> +		 * The maximum charge current on AXP803 is 2.8A, and the
> +		 * datasheet says "1110-1111 reserved" in this part.
> +		 * So we return an invalid value -1 in this situation,
> +		 * which will be dealed by the caller of this function,
> +		 */
> +		if (*val > 13)
> +			*val = -1;
> +		break;
> +	}
>  }
>  
>  static int axp20x_get_constant_charge_current(struct axp20x_batt_ps *axp,
> @@ -269,9 +322,13 @@ static int axp20x_battery_get_prop(struct power_supply *psy,
>  		if (ret)
>  			return ret;
>  
> -		if (axp20x_batt->axp_id == AXP221_ID &&
> -		    !(reg & AXP22X_FG_VALID))
> -			return -EINVAL;
> +		switch (axp20x_batt->axp_id) {
> +		case AXP221_ID:
> +		case AXP803_ID:
> +			if (!(reg & AXP22X_FG_VALID))
> +				return -EINVAL;
> +			break;
> +		};
>  
>  		/*
>  		 * Fuel Gauge data takes 7 bits but the stored value seems to be
> @@ -281,11 +338,19 @@ static int axp20x_battery_get_prop(struct power_supply *psy,
>  		break;
>  
>  	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
> -		if (axp20x_batt->axp_id == AXP209_ID)
> +		switch (axp20x_batt->axp_id) {
> +		case AXP209_ID:
>  			return axp20x_battery_get_max_voltage(axp20x_batt,
>  							      &val->intval);
> -		return axp22x_battery_get_max_voltage(axp20x_batt,
> -						      &val->intval);
> +		case AXP221_ID:
> +			return axp22x_battery_get_max_voltage(axp20x_batt,
> +							      &val->intval);
> +		case AXP803_ID:
> +			return axp803_battery_get_max_voltage(axp20x_batt,
> +							      &val->intval);
> +		default:
> +			return -EINVAL;
> +		}
>  
>  	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
>  		ret = regmap_read(axp20x_batt->regmap, AXP20X_V_OFF, &reg);
> @@ -467,6 +532,9 @@ static const struct of_device_id axp20x_battery_ps_id[] = {
>  	}, {
>  		.compatible = "x-powers,axp221-battery-power-supply",
>  		.data = (void *)AXP221_ID,
> +	}, {
> +		.compatible = "x-powers,axp803-battery-power-supply",
> +		.data = (void *)AXP803_ID,
>  	}, { /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, axp20x_battery_ps_id);

Re: [RFC PATCH 4/7] power: supply: axp20x-battery: support AXP803

[Quentin Schulz]

Hi Icenowy,

On 20/09/2017 17:18, Icenowy Zheng wrote:
> The AXP803 PMIC has battery support like other AXP PMICs, but with
> different definition of max target charging voltage and constant
> charging current.
> 
> Add support for AXP803 battery in axp20x-battery driver.
> 
> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
> ---
>  drivers/power/supply/axp20x_battery.c | 88 +++++++++++++++++++++++++++++++----
>  1 file changed, 78 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/power/supply/axp20x_battery.c b/drivers/power/supply/axp20x_battery.c
> index 7494f0f0eadb..c9a9fb320c92 100644
> --- a/drivers/power/supply/axp20x_battery.c
> +++ b/drivers/power/supply/axp20x_battery.c
> @@ -49,6 +49,8 @@
[...]
>  static void constant_charge_current_to_raw(struct axp20x_batt_ps *axp, int *val)
>  {
> -	if (axp->axp_id == AXP209_ID)
> +	switch (axp->axp_id) {
> +	case AXP209_ID:
>  		*val = (*val - 300000) / 100000;
> -	else
> +		break;
> +	case AXP221_ID:
>  		*val = (*val - 300000) / 150000;
> +		break;
> +	case AXP803_ID:
> +		*val = (*val - 200000) / 200000;
> +		/*
> +		 * The maximum charge current on AXP803 is 2.8A, and the
> +		 * datasheet says "1110-1111 reserved" in this part.
> +		 * So we return an invalid value -1 in this situation,
> +		 * which will be dealed by the caller of this function,
> +		 */

Good, we could do that for the two others compatible as well. They are
not explicitly marked as reserved but it stops at 1100 for AXP223/AXP221
for example.

> +		if (*val > 13)
> +			*val = -1;
> +		break;
> +	}
>  }
>  
>  static int axp20x_get_constant_charge_current(struct axp20x_batt_ps *axp,
> @@ -269,9 +322,13 @@ static int axp20x_battery_get_prop(struct power_supply *psy,
>  		if (ret)
>  			return ret;
>  
> -		if (axp20x_batt->axp_id == AXP221_ID &&
> -		    !(reg & AXP22X_FG_VALID))
> -			return -EINVAL;
> +		switch (axp20x_batt->axp_id) {
> +		case AXP221_ID:
> +		case AXP803_ID:
> +			if (!(reg & AXP22X_FG_VALID))
> +				return -EINVAL;
> +			break;
> +		};

Looks weird to me.

if (axp20x_batt->axp_id != AXP209_ID && !(reg & AXP22X_FG_VALID))

would be a better match?
[...]

Thanks,
Quentin

-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[RFC PATCH 1/7] dt-bindings: add compatibles for AXP803 Battery/USB power supplies

[Icenowy Zheng]

The AXP803 PMIC has different Battery and USB power supplies than the
AXP series PMICs already supported by the kernel, but the AC power
supply is the same as AXP22x (as it can only detect the present/online
state of the AC power supply on both AXP22x and AXP803).

Add compatible strings for the AXP803 Battery/USB power supplies. For AC
power supply the one on AXP803 is compatible with the one on AXP22x.

Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
---
 Documentation/devicetree/bindings/power/supply/axp20x_battery.txt   | 1 +
 Documentation/devicetree/bindings/power/supply/axp20x_usb_power.txt | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt b/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
index c24886676a60..091e5471a8c6 100644
--- a/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
+++ b/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
@@ -4,6 +4,7 @@ Required Properties:
  - compatible, one of:
 			"x-powers,axp209-battery-power-supply"
 			"x-powers,axp221-battery-power-supply"
+			"x-powers,axp803-battery-power-supply"
 
 This node is a subnode of the axp20x/axp22x PMIC.
 
diff --git a/Documentation/devicetree/bindings/power/supply/axp20x_usb_power.txt b/Documentation/devicetree/bindings/power/supply/axp20x_usb_power.txt
index ba8d35f66cbe..f30e3bf8d23f 100644
--- a/Documentation/devicetree/bindings/power/supply/axp20x_usb_power.txt
+++ b/Documentation/devicetree/bindings/power/supply/axp20x_usb_power.txt
@@ -4,6 +4,7 @@ Required Properties:
 -compatible: One of: "x-powers,axp202-usb-power-supply"
                      "x-powers,axp221-usb-power-supply"
                      "x-powers,axp223-usb-power-supply"
+                     "x-powers,axp803-usb-power-supply"
 
 The AXP223 PMIC shares most of its behaviour with the AXP221 but has slight
 variations such as the former being able to set the VBUS power supply max
-- 
2.13.5

Re: [RFC PATCH 1/7] dt-bindings: add compatibles for AXP803 Battery/USB power supplies

[Quentin Schulz]

Hi Icenowy,

On 20/09/2017 17:18, Icenowy Zheng wrote:
> The AXP803 PMIC has different Battery and USB power supplies than the
> AXP series PMICs already supported by the kernel, but the AC power
> supply is the same as AXP22x (as it can only detect the present/online
> state of the AC power supply on both AXP22x and AXP803).
> 
> Add compatible strings for the AXP803 Battery/USB power supplies. For AC
> power supply the one on AXP803 is compatible with the one on AXP22x.
> 
> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/power/supply/axp20x_battery.txt   | 1 +
>  Documentation/devicetree/bindings/power/supply/axp20x_usb_power.txt | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt b/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
> index c24886676a60..091e5471a8c6 100644
> --- a/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
> +++ b/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
> @@ -4,6 +4,7 @@ Required Properties:
>   - compatible, one of:
>  			"x-powers,axp209-battery-power-supply"
>  			"x-powers,axp221-battery-power-supply"
> +			"x-powers,axp803-battery-power-supply"
>  
>  This node is a subnode of the axp20x/axp22x PMIC.
>  
> diff --git a/Documentation/devicetree/bindings/power/supply/axp20x_usb_power.txt b/Documentation/devicetree/bindings/power/supply/axp20x_usb_power.txt
> index ba8d35f66cbe..f30e3bf8d23f 100644
> --- a/Documentation/devicetree/bindings/power/supply/axp20x_usb_power.txt
> +++ b/Documentation/devicetree/bindings/power/supply/axp20x_usb_power.txt
> @@ -4,6 +4,7 @@ Required Properties:
>  -compatible: One of: "x-powers,axp202-usb-power-supply"
>                       "x-powers,axp221-usb-power-supply"
>                       "x-powers,axp223-usb-power-supply"
> +                     "x-powers,axp803-usb-power-supply"

No. You are adding support for AC and battery power supply in this
patchset, not for USB.

Quentin
-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[RFC PATCH 2/7] iio: adc: axp20x-adc: allow to skip ADC rate setup now

[Icenowy Zheng]

The ADC rate setup on AXP803 is more complex than AXP20x/22x.

As it's not a necessary setup, allow it to be skipped, to allow simpler
AXP803 support now.

Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
---
 drivers/iio/adc/axp20x_adc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
index 11e177180ea0..93dd6b80059e 100644
--- a/drivers/iio/adc/axp20x_adc.c
+++ b/drivers/iio/adc/axp20x_adc.c
@@ -556,8 +556,10 @@ static int axp20x_probe(struct platform_device *pdev)
 				   AXP20X_ADC_EN2_MASK, AXP20X_ADC_EN2_MASK);
 
 	/* Configure ADCs rate */
-	regmap_update_bits(info->regmap, AXP20X_ADC_RATE, AXP20X_ADC_RATE_MASK,
-			   info->data->adc_rate(100));
+	if (info->data->adc_rate)
+		regmap_update_bits(info->regmap, AXP20X_ADC_RATE,
+				   AXP20X_ADC_RATE_MASK,
+				   info->data->adc_rate(100));
 
 	ret = iio_map_array_register(indio_dev, info->data->maps);
 	if (ret < 0) {
-- 
2.13.5

Re: [RFC PATCH 2/7] iio: adc: axp20x-adc: allow to skip ADC rate setup now

[Quentin Schulz]

Hi Icenowy,

On 20/09/2017 17:18, Icenowy Zheng wrote:
> The ADC rate setup on AXP803 is more complex than AXP20x/22x.
> 

Can you elaborate?

I can see two rate settings in register 0x85.

Bits 7-6 => TS/GPIO ADC speed setting (25, 50, 100, 200 Hz) ((ilog2(x) /
25) << 6, same as AXP20X).
Bits 5-4 => Vol/Cur ADC speed setting (100, 200, 400, 800 Hz) ((ilog2(x)
/ 100) << 4, same as AXP22X except the bit shift).

Just set both to 100 or 200 by default (common rate). If someone wants
to add a more specific rate setting, he could do it in the future.

Setting a default is safer as we don't know if anything before the
kernel does anything to this register.

Thanks,
Quentin

> As it's not a necessary setup, allow it to be skipped, to allow simpler
> AXP803 support now.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> ---
>  drivers/iio/adc/axp20x_adc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
> index 11e177180ea0..93dd6b80059e 100644
> --- a/drivers/iio/adc/axp20x_adc.c
> +++ b/drivers/iio/adc/axp20x_adc.c
> @@ -556,8 +556,10 @@ static int axp20x_probe(struct platform_device *pdev)
>  				   AXP20X_ADC_EN2_MASK, AXP20X_ADC_EN2_MASK);
>  
>  	/* Configure ADCs rate */
> -	regmap_update_bits(info->regmap, AXP20X_ADC_RATE, AXP20X_ADC_RATE_MASK,
> -			   info->data->adc_rate(100));
> +	if (info->data->adc_rate)
> +		regmap_update_bits(info->regmap, AXP20X_ADC_RATE,
> +				   AXP20X_ADC_RATE_MASK,
> +				   info->data->adc_rate(100));
>  
>  	ret = iio_map_array_register(indio_dev, info->data->maps);
>  	if (ret < 0) {
> 

-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[RFC PATCH 3/7] iio: adc: axp20x-adc: add support for AXP803

[Icenowy Zheng]

AXP803 SoC features an ADC part including these channels: GPADC (GPIO0)
and TS pins, PMIC internal temperature sensor, battery voltage, battery
charge/discharge current.

Add support for the battery-related channels and internal temperature
channel in order to allow battery monitoring. The TS and GPADC channels
are complex and will be support after more investigation.

Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
---
 drivers/iio/adc/axp20x_adc.c | 108 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)

diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
index 93dd6b80059e..4f0cd98cf6ea 100644
--- a/drivers/iio/adc/axp20x_adc.c
+++ b/drivers/iio/adc/axp20x_adc.c
@@ -28,6 +28,8 @@
 
 #define AXP20X_ADC_EN2_MASK			(GENMASK(3, 2) | BIT(7))
 #define AXP22X_ADC_EN1_MASK			(GENMASK(7, 5) | BIT(0))
+/* TODO: Enable TS and GPADC when supporting them */
+#define AXP803_ADC_EN1_MASK			GENMASK(7, 5)
 
 #define AXP20X_GPIO10_IN_RANGE_GPIO0		BIT(0)
 #define AXP20X_GPIO10_IN_RANGE_GPIO1		BIT(1)
@@ -95,6 +97,17 @@ enum axp22x_adc_channel_i {
 	AXP22X_BATT_DISCHRG_I,
 };
 
+enum axp803_adc_channel_v {
+	AXP803_TS_IN = 0,
+	AXP803_GPADC_IN,
+	AXP803_BATT_V,
+};
+
+enum axp803_adc_channel_i {
+	AXP803_BATT_CHRG_I = 2,
+	AXP803_BATT_DISCHRG_I,
+};
+
 static struct iio_map axp20x_maps[] = {
 	{
 		.consumer_dev_name = "axp20x-usb-power-supply",
@@ -144,6 +157,11 @@ static struct iio_map axp22x_maps[] = {
 };
 
 /*
+ * AXP803 shares the same consumer map with AXP22x, as it has no ADC for
+ * VBUS and ACIN inputs either.
+ */
+
+/*
  * Channels are mapped by physical system. Their channels share the same index.
  * i.e. acin_i is in_current0_raw and acin_v is in_voltage0_raw.
  * The only exception is for the battery. batt_v will be in_voltage6_raw and
@@ -197,6 +215,23 @@ static const struct iio_chan_spec axp22x_adc_channels[] = {
 			   AXP20X_BATT_DISCHRG_I_H),
 };
 
+static const struct iio_chan_spec axp803_adc_channels[] = {
+	{
+		.type = IIO_TEMP,
+		.address = AXP288_PMIC_ADC_H,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE) |
+				      BIT(IIO_CHAN_INFO_OFFSET),
+		.datasheet_name = "pmic_temp",
+	},
+	AXP20X_ADC_CHANNEL(AXP803_BATT_V, "batt_v", IIO_VOLTAGE,
+			   AXP20X_BATT_V_H),
+	AXP20X_ADC_CHANNEL(AXP803_BATT_CHRG_I, "batt_chrg_i", IIO_CURRENT,
+			   AXP20X_BATT_CHRG_I_H),
+	AXP20X_ADC_CHANNEL(AXP803_BATT_DISCHRG_I, "batt_dischrg_i", IIO_CURRENT,
+			   AXP20X_BATT_DISCHRG_I_H),
+};
+
 static int axp20x_adc_raw(struct iio_dev *indio_dev,
 			  struct iio_chan_spec const *chan, int *val)
 {
@@ -243,6 +278,19 @@ static int axp22x_adc_raw(struct iio_dev *indio_dev,
 	return IIO_VAL_INT;
 }
 
+static int axp803_adc_raw(struct iio_dev *indio_dev,
+			  struct iio_chan_spec const *chan, int *val)
+{
+	struct axp20x_adc_iio *info = iio_priv(indio_dev);
+
+	/* All channels on AXP803 are stored on 12 bits. */
+	*val = axp20x_read_variable_width(info->regmap, chan->address, 12);
+	if (*val < 0)
+		return *val;
+
+	return IIO_VAL_INT;
+}
+
 static int axp20x_adc_scale_voltage(int channel, int *val, int *val2)
 {
 	switch (channel) {
@@ -342,6 +390,31 @@ static int axp22x_adc_scale(struct iio_chan_spec const *chan, int *val,
 	}
 }
 
+static int axp803_adc_scale(struct iio_chan_spec const *chan, int *val,
+			    int *val2)
+{
+	switch (chan->type) {
+	case IIO_VOLTAGE:
+		if (chan->channel != AXP803_BATT_V)
+			return -EINVAL;
+
+		*val = 1;
+		*val2 = 100000;
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case IIO_CURRENT:
+		*val = 1;
+		return IIO_VAL_INT;
+
+	case IIO_TEMP:
+		*val = 106;
+		return IIO_VAL_INT;
+
+	default:
+		return -EINVAL;
+	}
+}
+
 static int axp20x_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
 				     int *val)
 {
@@ -425,6 +498,26 @@ static int axp22x_read_raw(struct iio_dev *indio_dev,
 	}
 }
 
+static int axp803_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan, int *val,
+			   int *val2, long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_OFFSET:
+		*val = -2525;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		return axp803_adc_scale(chan, val, val2);
+
+	case IIO_CHAN_INFO_RAW:
+		return axp803_adc_raw(indio_dev, chan, val);
+
+	default:
+		return -EINVAL;
+	}
+}
+
 static int axp20x_write_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan, int val, int val2,
 			    long mask)
@@ -472,6 +565,11 @@ static const struct iio_info axp22x_adc_iio_info = {
 	.driver_module = THIS_MODULE,
 };
 
+static const struct iio_info axp803_adc_iio_info = {
+	.read_raw = axp803_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
 static int axp20x_adc_rate(int rate)
 {
 	return AXP20X_ADC_RATE_HZ(rate);
@@ -512,9 +610,19 @@ static const struct axp_data axp22x_data = {
 	.maps = axp22x_maps,
 };
 
+static const struct axp_data axp803_data = {
+	.iio_info = &axp803_adc_iio_info,
+	.num_channels = ARRAY_SIZE(axp803_adc_channels),
+	.channels = axp803_adc_channels,
+	.adc_en1_mask = AXP803_ADC_EN1_MASK,
+	.adc_en2 = false,
+	.maps = axp22x_maps,
+};
+
 static const struct platform_device_id axp20x_adc_id_match[] = {
 	{ .name = "axp20x-adc", .driver_data = (kernel_ulong_t)&axp20x_data, },
 	{ .name = "axp22x-adc", .driver_data = (kernel_ulong_t)&axp22x_data, },
+	{ .name = "axp803-adc", .driver_data = (kernel_ulong_t)&axp803_data, },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(platform, axp20x_adc_id_match);
-- 
2.13.5

Re: [RFC PATCH 3/7] iio: adc: axp20x-adc: add support for AXP803

[Jonathan Cameron]

On Wed, 20 Sep 2017 23:18:10 +0800
Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote:

> AXP803 SoC features an ADC part including these channels: GPADC (GPIO0)
> and TS pins, PMIC internal temperature sensor, battery voltage, battery
> charge/discharge current.
> 
> Add support for the battery-related channels and internal temperature
> channel in order to allow battery monitoring. The TS and GPADC channels
> are complex and will be support after more investigation.
> 
> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>

A few comments inline but this looks good to me.

I will want to leave plenty of time for others to comment however, particularly
Quentin.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/axp20x_adc.c | 108 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 108 insertions(+)
> 
> diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
> index 93dd6b80059e..4f0cd98cf6ea 100644
> --- a/drivers/iio/adc/axp20x_adc.c
> +++ b/drivers/iio/adc/axp20x_adc.c
> @@ -28,6 +28,8 @@
>  
>  #define AXP20X_ADC_EN2_MASK			(GENMASK(3, 2) | BIT(7))
>  #define AXP22X_ADC_EN1_MASK			(GENMASK(7, 5) | BIT(0))
> +/* TODO: Enable TS and GPADC when supporting them */
> +#define AXP803_ADC_EN1_MASK			GENMASK(7, 5)
>  
>  #define AXP20X_GPIO10_IN_RANGE_GPIO0		BIT(0)
>  #define AXP20X_GPIO10_IN_RANGE_GPIO1		BIT(1)
> @@ -95,6 +97,17 @@ enum axp22x_adc_channel_i {
>  	AXP22X_BATT_DISCHRG_I,
>  };
>  
> +enum axp803_adc_channel_v {
> +	AXP803_TS_IN = 0,
> +	AXP803_GPADC_IN,
> +	AXP803_BATT_V,
> +};
> +
> +enum axp803_adc_channel_i {
> +	AXP803_BATT_CHRG_I = 2,
> +	AXP803_BATT_DISCHRG_I,
> +};
> +
>  static struct iio_map axp20x_maps[] = {
>  	{
>  		.consumer_dev_name = "axp20x-usb-power-supply",
> @@ -144,6 +157,11 @@ static struct iio_map axp22x_maps[] = {
>  };
>  
>  /*
> + * AXP803 shares the same consumer map with AXP22x, as it has no ADC for
> + * VBUS and ACIN inputs either.
> + */
> +
> +/*
>   * Channels are mapped by physical system. Their channels share the same index.
>   * i.e. acin_i is in_current0_raw and acin_v is in_voltage0_raw.
>   * The only exception is for the battery. batt_v will be in_voltage6_raw and
> @@ -197,6 +215,23 @@ static const struct iio_chan_spec axp22x_adc_channels[] = {
>  			   AXP20X_BATT_DISCHRG_I_H),
>  };
>  
> +static const struct iio_chan_spec axp803_adc_channels[] = {
> +	{
> +		.type = IIO_TEMP,
> +		.address = AXP288_PMIC_ADC_H,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE) |
> +				      BIT(IIO_CHAN_INFO_OFFSET),
> +		.datasheet_name = "pmic_temp",
> +	},
> +	AXP20X_ADC_CHANNEL(AXP803_BATT_V, "batt_v", IIO_VOLTAGE,
> +			   AXP20X_BATT_V_H),
> +	AXP20X_ADC_CHANNEL(AXP803_BATT_CHRG_I, "batt_chrg_i", IIO_CURRENT,
> +			   AXP20X_BATT_CHRG_I_H),
> +	AXP20X_ADC_CHANNEL(AXP803_BATT_DISCHRG_I, "batt_dischrg_i", IIO_CURRENT,
> +			   AXP20X_BATT_DISCHRG_I_H),
> +};
> +
>  static int axp20x_adc_raw(struct iio_dev *indio_dev,
>  			  struct iio_chan_spec const *chan, int *val)
>  {
> @@ -243,6 +278,19 @@ static int axp22x_adc_raw(struct iio_dev *indio_dev,
>  	return IIO_VAL_INT;
>  }
>  
> +static int axp803_adc_raw(struct iio_dev *indio_dev,
> +			  struct iio_chan_spec const *chan, int *val)
> +{
> +	struct axp20x_adc_iio *info = iio_priv(indio_dev);
> +
> +	/* All channels on AXP803 are stored on 12 bits. */
> +	*val = axp20x_read_variable_width(info->regmap, chan->address, 12);
> +	if (*val < 0)
> +		return *val;
> +
> +	return IIO_VAL_INT;
> +}
> +
>  static int axp20x_adc_scale_voltage(int channel, int *val, int *val2)
>  {
>  	switch (channel) {
> @@ -342,6 +390,31 @@ static int axp22x_adc_scale(struct iio_chan_spec const *chan, int *val,
>  	}
>  }
>  
> +static int axp803_adc_scale(struct iio_chan_spec const *chan, int *val,
> +			    int *val2)
> +{
> +	switch (chan->type) {
> +	case IIO_VOLTAGE:
> +		if (chan->channel != AXP803_BATT_V)
> +			return -EINVAL;
> +
> +		*val = 1;
> +		*val2 = 100000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	case IIO_CURRENT:
> +		*val = 1;

A scale of 1 is assumed so you could drop providing this attribute.
However, given there are scales for all other channels I guess that
would feel weird.  There is nothing in our ABI saying you can't
specify things that are the default so it makes sense to me to keep
this here.

> +		return IIO_VAL_INT;
> +
> +	case IIO_TEMP:
> +		*val = 106;
> +		return IIO_VAL_INT;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  static int axp20x_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
>  				     int *val)
>  {
> @@ -425,6 +498,26 @@ static int axp22x_read_raw(struct iio_dev *indio_dev,
>  	}
>  }
>  
> +static int axp803_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan, int *val,
> +			   int *val2, long mask)
> +{
> +	switch (mask) {
> +	case IIO_CHAN_INFO_OFFSET:

I know it is impossible to get here unless we have a temperature channel,
but it still feels like this should be made apparent here.

Perhaps a comment rather than an explicit check in the code?

> +		*val = -2525;
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		return axp803_adc_scale(chan, val, val2);
> +
> +	case IIO_CHAN_INFO_RAW:
> +		return axp803_adc_raw(indio_dev, chan, val);
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  static int axp20x_write_raw(struct iio_dev *indio_dev,
>  			    struct iio_chan_spec const *chan, int val, int val2,
>  			    long mask)
> @@ -472,6 +565,11 @@ static const struct iio_info axp22x_adc_iio_info = {
>  	.driver_module = THIS_MODULE,
>  };
>  
> +static const struct iio_info axp803_adc_iio_info = {
> +	.read_raw = axp803_read_raw,
> +	.driver_module = THIS_MODULE,

.driver_module is now gone from this structure, but as it hasn't gone
upstream from my tree yet I'll clean these up if they are still there
once we get to the point of applying this patch.

> +};
> +
>  static int axp20x_adc_rate(int rate)
>  {
>  	return AXP20X_ADC_RATE_HZ(rate);
> @@ -512,9 +610,19 @@ static const struct axp_data axp22x_data = {
>  	.maps = axp22x_maps,
>  };
>  
> +static const struct axp_data axp803_data = {
> +	.iio_info = &axp803_adc_iio_info,
> +	.num_channels = ARRAY_SIZE(axp803_adc_channels),
> +	.channels = axp803_adc_channels,
> +	.adc_en1_mask = AXP803_ADC_EN1_MASK,
> +	.adc_en2 = false,
> +	.maps = axp22x_maps,
> +};
> +
>  static const struct platform_device_id axp20x_adc_id_match[] = {
>  	{ .name = "axp20x-adc", .driver_data = (kernel_ulong_t)&axp20x_data, },
>  	{ .name = "axp22x-adc", .driver_data = (kernel_ulong_t)&axp22x_data, },
> +	{ .name = "axp803-adc", .driver_data = (kernel_ulong_t)&axp803_data, },
>  	{ /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(platform, axp20x_adc_id_match);

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 3/7] iio: adc: axp20x-adc: add support for AXP803

[Quentin Schulz]

Hi Icenowy,

On 20/09/2017 17:18, Icenowy Zheng wrote:
> AXP803 SoC features an ADC part including these channels: GPADC (GPIO0)
> and TS pins, PMIC internal temperature sensor, battery voltage, battery
> charge/discharge current.
> 
> Add support for the battery-related channels and internal temperature
> channel in order to allow battery monitoring. The TS and GPADC channels
> are complex and will be support after more investigation.
> 
> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
> ---
>  drivers/iio/adc/axp20x_adc.c | 108 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 108 insertions(+)
> 
> diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
> index 93dd6b80059e..4f0cd98cf6ea 100644
> --- a/drivers/iio/adc/axp20x_adc.c
> +++ b/drivers/iio/adc/axp20x_adc.c
> @@ -28,6 +28,8 @@
[...]>  /*
> + * AXP803 shares the same consumer map with AXP22x, as it has no ADC for
> + * VBUS and ACIN inputs either.
> + */
> +
> +/*

Put that in the commit log?
If we add a comment for each newly supported PMIC we will end up with
more comments than code :)

[...]>
> +static const struct axp_data axp803_data = {
> +	.iio_info = &axp803_adc_iio_info,
> +	.num_channels = ARRAY_SIZE(axp803_adc_channels),
> +	.channels = axp803_adc_channels,
> +	.adc_en1_mask = AXP803_ADC_EN1_MASK,
> +	.adc_en2 = false,

Not required I guess, by default it is false.

Thanks,
Quentin
-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[RFC PATCH 5/7] mfd: axp20x: add cells for AXP803 ADC/AC Power/Battery

[Icenowy Zheng]

As we have now support for AXP803 ADC/Battery, and the AC Power part of
AXP803 is the same as AXP22x, add MFD cells for these drivers.

Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
---
 drivers/mfd/axp20x.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 336de66ca408..91be5fe1c5de 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -850,7 +850,18 @@ static struct mfd_cell axp803_cells[] = {
 		.num_resources		= ARRAY_SIZE(axp803_pek_resources),
 		.resources		= axp803_pek_resources,
 	},
+	{	.name			= "axp803-adc" },
 	{	.name			= "axp20x-regulator" },
+	{
+		.name		= "axp20x-ac-power-supply",
+		.of_compatible	= "x-powers,axp221-ac-power-supply",
+		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
+		.resources	= axp20x_ac_power_supply_resources,
+	},
+	{
+		.name		= "axp20x-battery-power-supply",
+		.of_compatible	= "x-powers,axp803-battery-power-supply",
+	},
 };
 
 static struct mfd_cell axp806_cells[] = {
-- 
2.13.5

[RFC PATCH 6/7] arm64: allwinner: a64: add power supply nodes in AXP803 DTSI

[Icenowy Zheng]

AXP803 PMIC features AC/USB/Battery power supplies.

As we have now the device tree bindings for them, add device tree
nodes for them.

Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
---
 arch/arm64/boot/dts/allwinner/axp803.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/axp803.dtsi b/arch/arm64/boot/dts/allwinner/axp803.dtsi
index ff8af52743ff..3a8615231b7c 100644
--- a/arch/arm64/boot/dts/allwinner/axp803.dtsi
+++ b/arch/arm64/boot/dts/allwinner/axp803.dtsi
@@ -49,6 +49,16 @@
 	interrupt-controller;
 	#interrupt-cells = <1>;
 
+	ac_power_supply: ac-power-supply {
+		compatible = "x-powers,axp221-ac-power-supply";
+		status = "disabled";
+	};
+
+	battery_power_supply: battery-power-supply {
+		compatible = "x-powers,axp803-battery-power-supply";
+		status = "disabled";
+	};
+
 	regulators {
 		/* Default work frequency for buck regulators */
 		x-powers,dcdc-freq = <3000>;
@@ -147,4 +157,9 @@
 			regulator-name = "rtc-ldo";
 		};
 	};
+
+	usb_power_supply: usb_power_supply {
+		compatible = "x-powers,axp803-usb-power-supply";
+		status = "disabled";
+	};
 };
-- 
2.13.5

Re: [RFC PATCH 6/7] arm64: allwinner: a64: add power supply nodes in AXP803 DTSI

[Quentin Schulz]

Hi Icenowy,

On 20/09/2017 17:18, Icenowy Zheng wrote:
> AXP803 PMIC features AC/USB/Battery power supplies.
> 
> As we have now the device tree bindings for them, add device tree
> nodes for them.
> 
> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
> ---
>  arch/arm64/boot/dts/allwinner/axp803.dtsi | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/axp803.dtsi b/arch/arm64/boot/dts/allwinner/axp803.dtsi
> index ff8af52743ff..3a8615231b7c 100644
> --- a/arch/arm64/boot/dts/allwinner/axp803.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/axp803.dtsi
> @@ -49,6 +49,16 @@
>  	interrupt-controller;
>  	#interrupt-cells = <1>;
>  
> +	ac_power_supply: ac-power-supply {
> +		compatible = "x-powers,axp221-ac-power-supply";
> +		status = "disabled";
> +	};
> +
> +	battery_power_supply: battery-power-supply {
> +		compatible = "x-powers,axp803-battery-power-supply";
> +		status = "disabled";
> +	};
> +
>  	regulators {
>  		/* Default work frequency for buck regulators */
>  		x-powers,dcdc-freq = <3000>;
> @@ -147,4 +157,9 @@
>  			regulator-name = "rtc-ldo";
>  		};
>  	};
> +
> +	usb_power_supply: usb_power_supply {
> +		compatible = "x-powers,axp803-usb-power-supply";
> +		status = "disabled";
> +	};

No. You have added support for the AC and battery power supply drivers
in this patchset, not for USB.

Quentin

-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [RFC PATCH 6/7] arm64: allwinner: a64: add power supply nodes in AXP803 DTSI

[Icenowy Zheng]

于 2017年9月25日 GMT+08:00 下午5:11:57, Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> 写到:
>Hi Icenowy,
>
>On 20/09/2017 17:18, Icenowy Zheng wrote:
>> AXP803 PMIC features AC/USB/Battery power supplies.
>> 
>> As we have now the device tree bindings for them, add device tree
>> nodes for them.
>> 
>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
>> ---
>>  arch/arm64/boot/dts/allwinner/axp803.dtsi | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>> 
>> diff --git a/arch/arm64/boot/dts/allwinner/axp803.dtsi
>b/arch/arm64/boot/dts/allwinner/axp803.dtsi
>> index ff8af52743ff..3a8615231b7c 100644
>> --- a/arch/arm64/boot/dts/allwinner/axp803.dtsi
>> +++ b/arch/arm64/boot/dts/allwinner/axp803.dtsi
>> @@ -49,6 +49,16 @@
>>  	interrupt-controller;
>>  	#interrupt-cells = <1>;
>>  
>> +	ac_power_supply: ac-power-supply {
>> +		compatible = "x-powers,axp221-ac-power-supply";
>> +		status = "disabled";
>> +	};
>> +
>> +	battery_power_supply: battery-power-supply {
>> +		compatible = "x-powers,axp803-battery-power-supply";
>> +		status = "disabled";
>> +	};
>> +
>>  	regulators {
>>  		/* Default work frequency for buck regulators */
>>  		x-powers,dcdc-freq = <3000>;
>> @@ -147,4 +157,9 @@
>>  			regulator-name = "rtc-ldo";
>>  		};
>>  	};
>> +
>> +	usb_power_supply: usb_power_supply {
>> +		compatible = "x-powers,axp803-usb-power-supply";
>> +		status = "disabled";
>> +	};
>
>No. You have added support for the AC and battery power supply drivers
>in this patchset, not for USB.

But I added its device tree binding.

>
>Quentin

-- 
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-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: Re: [RFC PATCH 6/7] arm64: allwinner: a64: add power supply nodes in AXP803 DTSI

[Chen-Yu Tsai]

On Mon, Sep 25, 2017 at 5:14 PM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote:
>
>
> 于 2017年9月25日 GMT+08:00 下午5:11:57, Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> 写到:
>>Hi Icenowy,
>>
>>On 20/09/2017 17:18, Icenowy Zheng wrote:
>>> AXP803 PMIC features AC/USB/Battery power supplies.
>>>
>>> As we have now the device tree bindings for them, add device tree
>>> nodes for them.
>>>
>>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
>>> ---
>>>  arch/arm64/boot/dts/allwinner/axp803.dtsi | 15 +++++++++++++++
>>>  1 file changed, 15 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/allwinner/axp803.dtsi
>>b/arch/arm64/boot/dts/allwinner/axp803.dtsi
>>> index ff8af52743ff..3a8615231b7c 100644
>>> --- a/arch/arm64/boot/dts/allwinner/axp803.dtsi
>>> +++ b/arch/arm64/boot/dts/allwinner/axp803.dtsi
>>> @@ -49,6 +49,16 @@
>>>      interrupt-controller;
>>>      #interrupt-cells = <1>;
>>>
>>> +    ac_power_supply: ac-power-supply {
>>> +            compatible = "x-powers,axp221-ac-power-supply";
>>> +            status = "disabled";
>>> +    };
>>> +
>>> +    battery_power_supply: battery-power-supply {
>>> +            compatible = "x-powers,axp803-battery-power-supply";
>>> +            status = "disabled";
>>> +    };
>>> +
>>>      regulators {
>>>              /* Default work frequency for buck regulators */
>>>              x-powers,dcdc-freq = <3000>;
>>> @@ -147,4 +157,9 @@
>>>                      regulator-name = "rtc-ldo";
>>>              };
>>>      };
>>> +
>>> +    usb_power_supply: usb_power_supply {
>>> +            compatible = "x-powers,axp803-usb-power-supply";
>>> +            status = "disabled";
>>> +    };
>>
>>No. You have added support for the AC and battery power supply drivers
>>in this patchset, not for USB.
>
> But I added its device tree binding.

Please do both at the same time. If you only add the binding without
the driver, how can you be sure the binding would be a proper fit?
Moreover, no one can actually test it.

ChenYu

-- 
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-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: [RFC PATCH 6/7] arm64: allwinner: a64: add power supply nodes in AXP803 DTSI

[Quentin Schulz]

Hi Icenowy,

On 25/09/2017 11:14, Icenowy Zheng wrote:
> 
> 
> 于 2017年9月25日 GMT+08:00 下午5:11:57, Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> 写到:
>> Hi Icenowy,
>>
>> On 20/09/2017 17:18, Icenowy Zheng wrote:
>>> AXP803 PMIC features AC/USB/Battery power supplies.
>>>
>>> As we have now the device tree bindings for them, add device tree
>>> nodes for them.
>>>
>>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
>>> ---
>>>  arch/arm64/boot/dts/allwinner/axp803.dtsi | 15 +++++++++++++++
>>>  1 file changed, 15 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/allwinner/axp803.dtsi
>> b/arch/arm64/boot/dts/allwinner/axp803.dtsi
>>> index ff8af52743ff..3a8615231b7c 100644
>>> --- a/arch/arm64/boot/dts/allwinner/axp803.dtsi
>>> +++ b/arch/arm64/boot/dts/allwinner/axp803.dtsi
>>> @@ -49,6 +49,16 @@
>>>  	interrupt-controller;
>>>  	#interrupt-cells = <1>;
>>>  
>>> +	ac_power_supply: ac-power-supply {
>>> +		compatible = "x-powers,axp221-ac-power-supply";
>>> +		status = "disabled";
>>> +	};
>>> +
>>> +	battery_power_supply: battery-power-supply {
>>> +		compatible = "x-powers,axp803-battery-power-supply";
>>> +		status = "disabled";
>>> +	};
>>> +
>>>  	regulators {
>>>  		/* Default work frequency for buck regulators */
>>>  		x-powers,dcdc-freq = <3000>;
>>> @@ -147,4 +157,9 @@
>>>  			regulator-name = "rtc-ldo";
>>>  		};
>>>  	};
>>> +
>>> +	usb_power_supply: usb_power_supply {
>>> +		compatible = "x-powers,axp803-usb-power-supply";
>>> +		status = "disabled";
>>> +	};
>>
>> No. You have added support for the AC and battery power supply drivers
>> in this patchset, not for USB.
> 
> But I added its device tree binding.

Yes and that is wrong. That would mislead users into thinking the usb
power supply is supported (since the dt binding is here) while it's not.

I would add the dt binding and the DT node only once it is supported.

Quentin
-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
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-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

[RFC PATCH 7/7] arm64: allwinner: a64: enable AC and Battery for Pine64

[Icenowy Zheng]

The Pine64 boards (including the Plus variant) have a Micro-USB jack
with it's 5V connected to the ACIN of the AXP803 PMIC and a battery
connector connected to the battery pins of the AXP803 PMIC.

Enable AC and Battery power supplies for Pine64, in order to monitor
them in the system.

Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
index d06e34b5d192..955f392af6a2 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
@@ -113,6 +113,14 @@
 
 #include "axp803.dtsi"
 
+&ac_power_supply {
+	status = "okay";
+};
+
+&battery_power_supply {
+	status = "okay";
+};
+
 &reg_aldo2 {
 	regulator-always-on;
 	regulator-min-microvolt = <1800000>;
-- 
2.13.5

Re: [RFC PATCH 0/7] AXP803 AC/Battery support

[Jonathan Cameron]

On Wed, 20 Sep 2017 23:18:07 +0800
Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote:

> The AXP803 PMIC, used by most Allwinner A64 boards, features 3 power inputs:
> AC, USB and Battery.
> 
> This patchset adds support for the AC and Battery supplies, which is useful
> for the boards from Pine64 (Pine64, SoPine w/ baseboard model A, Pinebook).
> 
> The USB supply is not yet supported in this patchset because it's not
> present on Pine series boards.
> 
> In order to enable battery monitoring the ADC for battery is also enabled
> for AXs.
> 
> In order to enable battery monitoring the ADC for battery is also enabled
> for AXP803.

I'll go with the obvious question...

Why an RFC rather than a standard patch submission? I'm not immediately
seeing what is controversial!

Jonathan

> 
> Icenowy Zheng (7):
>   dt-bindings: add compatibles for AXP803 Battery/USB power supplies
>   iio: adc: axp20x-adc: allow to skip ADC rate setup now
>   iio: adc: axp20x-adc: add support for AXP803
>   power: supply: axp20x-battery: support AXP803
>   mfd: axp20x: add cells for AXP803 ADC/AC Power/Battery
>   arm64: allwinner: a64: add power supply nodes in AXP803 DTSI
>   arm64: allwinner: a64: enable AC and Battery for Pine64
> 
>  .../bindings/power/supply/axp20x_battery.txt       |   1 +
>  .../bindings/power/supply/axp20x_usb_power.txt     |   1 +
>  arch/arm64/boot/dts/allwinner/axp803.dtsi          |  15 +++
>  .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts |   8 ++
>  drivers/iio/adc/axp20x_adc.c                       | 114 ++++++++++++++++++++-
>  drivers/mfd/axp20x.c                               |  11 ++
>  drivers/power/supply/axp20x_battery.c              |  88 ++++++++++++++--
>  7 files changed, 226 insertions(+), 12 deletions(-)
> 

Re: [RFC PATCH 0/7] AXP803 AC/Battery support

[Icenowy Zheng]

于 2017年9月21日 GMT+08:00 下午10:46:21, Jonathan Cameron <Jonathan.Cameron-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> 写到:
>On Wed, 20 Sep 2017 23:18:07 +0800
>Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote:
>
>> The AXP803 PMIC, used by most Allwinner A64 boards, features 3 power
>inputs:
>> AC, USB and Battery.
>> 
>> This patchset adds support for the AC and Battery supplies, which is
>useful
>> for the boards from Pine64 (Pine64, SoPine w/ baseboard model A,
>Pinebook).
>> 
>> The USB supply is not yet supported in this patchset because it's not
>> present on Pine series boards.
>> 
>> In order to enable battery monitoring the ADC for battery is also
>enabled
>> for AXs.
>> 
>> In order to enable battery monitoring the ADC for battery is also
>enabled
>> for AXP803.
>
>I'll go with the obvious question...
>
>Why an RFC rather than a standard patch submission? I'm not immediately
>seeing what is controversial!

Oh I am just not confident about this patchset,
especially the IIO part.

>
>Jonathan
>
>> 
>> Icenowy Zheng (7):
>>   dt-bindings: add compatibles for AXP803 Battery/USB power supplies
>>   iio: adc: axp20x-adc: allow to skip ADC rate setup now
>>   iio: adc: axp20x-adc: add support for AXP803
>>   power: supply: axp20x-battery: support AXP803
>>   mfd: axp20x: add cells for AXP803 ADC/AC Power/Battery
>>   arm64: allwinner: a64: add power supply nodes in AXP803 DTSI
>>   arm64: allwinner: a64: enable AC and Battery for Pine64
>> 
>>  .../bindings/power/supply/axp20x_battery.txt       |   1 +
>>  .../bindings/power/supply/axp20x_usb_power.txt     |   1 +
>>  arch/arm64/boot/dts/allwinner/axp803.dtsi          |  15 +++
>>  .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts |   8 ++
>>  drivers/iio/adc/axp20x_adc.c                       | 114
>++++++++++++++++++++-
>>  drivers/mfd/axp20x.c                               |  11 ++
>>  drivers/power/supply/axp20x_battery.c              |  88
>++++++++++++++--
>>  7 files changed, 226 insertions(+), 12 deletions(-)
>> 
>
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/7] AXP803 AC/Battery support

[Jonathan Cameron]

On Thu, 21 Sep 2017 23:20:11 +0800
Icenowy Zheng <icenowy@aosc.io> wrote:

> 于 2017年9月21日 GMT+08:00 下午10:46:21, Jonathan Cameron <Jonathan.Cameron@huawei.com> 写到:
> >On Wed, 20 Sep 2017 23:18:07 +0800
> >Icenowy Zheng <icenowy@aosc.io> wrote:
> >  
> >> The AXP803 PMIC, used by most Allwinner A64 boards, features 3 power  
> >inputs:  
> >> AC, USB and Battery.
> >> 
> >> This patchset adds support for the AC and Battery supplies, which is  
> >useful  
> >> for the boards from Pine64 (Pine64, SoPine w/ baseboard model A,  
> >Pinebook).  
> >> 
> >> The USB supply is not yet supported in this patchset because it's not
> >> present on Pine series boards.
> >> 
> >> In order to enable battery monitoring the ADC for battery is also  
> >enabled  
> >> for AXs.
> >> 
> >> In order to enable battery monitoring the ADC for battery is also  
> >enabled  
> >> for AXP803.  
> >
> >I'll go with the obvious question...
> >
> >Why an RFC rather than a standard patch submission? I'm not immediately
> >seeing what is controversial!  
> 
> Oh I am just not confident about this patchset,
> especially the IIO part.

It all looks fine to me.  I would imagine that, once everyone is
happy, this will go through the mfd tree, but Lee may have other ideas!

Jonathan
> 
> >
> >Jonathan
> >  
> >> 
> >> Icenowy Zheng (7):
> >>   dt-bindings: add compatibles for AXP803 Battery/USB power supplies
> >>   iio: adc: axp20x-adc: allow to skip ADC rate setup now
> >>   iio: adc: axp20x-adc: add support for AXP803
> >>   power: supply: axp20x-battery: support AXP803
> >>   mfd: axp20x: add cells for AXP803 ADC/AC Power/Battery
> >>   arm64: allwinner: a64: add power supply nodes in AXP803 DTSI
> >>   arm64: allwinner: a64: enable AC and Battery for Pine64
> >> 
> >>  .../bindings/power/supply/axp20x_battery.txt       |   1 +
> >>  .../bindings/power/supply/axp20x_usb_power.txt     |   1 +
> >>  arch/arm64/boot/dts/allwinner/axp803.dtsi          |  15 +++
> >>  .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts |   8 ++
> >>  drivers/iio/adc/axp20x_adc.c                       | 114  
> >++++++++++++++++++++-  
> >>  drivers/mfd/axp20x.c                               |  11 ++
> >>  drivers/power/supply/axp20x_battery.c              |  88  
> >++++++++++++++--  
> >>  7 files changed, 226 insertions(+), 12 deletions(-)
> >>   
> >
> >
> >_______________________________________________
> >linux-arm-kernel mailing list
> >linux-arm-kernel@lists.infradead.org
> >http://lists.infradead.org/mailman/listinfo/linux-arm-kernel  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 0/7] AXP803 AC/Battery support

[Quentin Schulz]

Hi Icenowy,

On 24/09/2017 16:36, Jonathan Cameron wrote:
> On Thu, 21 Sep 2017 23:20:11 +0800
> Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote:
> 
>> 于 2017年9月21日 GMT+08:00 下午10:46:21, Jonathan Cameron <Jonathan.Cameron-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> 写到:
>>> On Wed, 20 Sep 2017 23:18:07 +0800
>>> Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote:
>>>  
>>>> The AXP803 PMIC, used by most Allwinner A64 boards, features 3 power  
>>> inputs:  
>>>> AC, USB and Battery.
>>>>
>>>> This patchset adds support for the AC and Battery supplies, which is  
>>> useful  
>>>> for the boards from Pine64 (Pine64, SoPine w/ baseboard model A,  
>>> Pinebook).  
>>>>
>>>> The USB supply is not yet supported in this patchset because it's not
>>>> present on Pine series boards.
>>>>
>>>> In order to enable battery monitoring the ADC for battery is also  
>>> enabled  
>>>> for AXs.
>>>>
>>>> In order to enable battery monitoring the ADC for battery is also  
>>> enabled  
>>>> for AXP803.  
>>>
>>> I'll go with the obvious question...
>>>
>>> Why an RFC rather than a standard patch submission? I'm not immediately
>>> seeing what is controversial!  
>>
>> Oh I am just not confident about this patchset,
>> especially the IIO part.
> 
> It all looks fine to me.  I would imagine that, once everyone is
> happy, this will go through the mfd tree, but Lee may have other ideas!
> 

Small modifications to make but I definitely agree with Jonathan that
you did not need to send an RFC.

Quentin

> Jonathan
>>
>>>
>>> Jonathan
>>>  
>>>>
>>>> Icenowy Zheng (7):
>>>>   dt-bindings: add compatibles for AXP803 Battery/USB power supplies
>>>>   iio: adc: axp20x-adc: allow to skip ADC rate setup now
>>>>   iio: adc: axp20x-adc: add support for AXP803
>>>>   power: supply: axp20x-battery: support AXP803
>>>>   mfd: axp20x: add cells for AXP803 ADC/AC Power/Battery
>>>>   arm64: allwinner: a64: add power supply nodes in AXP803 DTSI
>>>>   arm64: allwinner: a64: enable AC and Battery for Pine64
>>>>
>>>>  .../bindings/power/supply/axp20x_battery.txt       |   1 +
>>>>  .../bindings/power/supply/axp20x_usb_power.txt     |   1 +
>>>>  arch/arm64/boot/dts/allwinner/axp803.dtsi          |  15 +++
>>>>  .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts |   8 ++
>>>>  drivers/iio/adc/axp20x_adc.c                       | 114  
>>> ++++++++++++++++++++-  
>>>>  drivers/mfd/axp20x.c                               |  11 ++
>>>>  drivers/power/supply/axp20x_battery.c              |  88  
>>> ++++++++++++++--  
>>>>  7 files changed, 226 insertions(+), 12 deletions(-)
>>>>   
>>>
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel  
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
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-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.