[PATCH 0/4] AXP8x3 AC and battery power supply support

[PATCH 0/4] AXP8x3 AC and battery power supply support

[Oskari Lemmela]

AXP803 AC and battery power supply support.
AXP803/AXP813 AC power supply support with input current and
voltage limiting support.

Oskari Lemmela (4):
  dt-bindings: add compatibles for AXP803 AC and battery power supplies
  ARM: dtsi: axp81x: add AC power supply subnode
  arm64: allwinner: a64: add battery and AC power supply support
  power: supply: add AXP803/AXP813 AC and battery power supply support

 .../devicetree/bindings/gpio/gpio-axp209.txt  |   3 +-
 .../bindings/iio/adc/axp20x_adc.txt           |   3 +-
 .../bindings/power/supply/axp20x_ac_power.txt |   4 +
 .../bindings/power/supply/axp20x_battery.txt  |   1 +
 arch/arm/boot/dts/axp81x.dtsi                 |   5 +
 arch/arm64/boot/dts/allwinner/axp803.dtsi     |  31 +++
 .../allwinner/sun50i-a64-sopine-baseboard.dts |   8 +
 drivers/iio/adc/axp20x_adc.c                  |   1 +
 drivers/mfd/axp20x.c                          |  22 +-
 drivers/pinctrl/pinctrl-axp209.c              |   1 +
 drivers/power/supply/axp20x_ac_power.c        | 194 ++++++++++++++++++
 drivers/power/supply/axp20x_battery.c         |   3 +
 include/linux/mfd/axp20x.h                    |   1 +
 13 files changed, 274 insertions(+), 3 deletions(-)

-- 
2.17.1

[PATCH 1/4] dt-bindings: add compatibles for AXP803 AC and battery power supplies

[Oskari Lemmela]

AXP803 PMIC is register compatible with AXP813.

AXP803/AXP813 are capable to limit input current and supply voltage.

Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
---
 Documentation/devicetree/bindings/gpio/gpio-axp209.txt        | 3 ++-
 Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt      | 3 ++-
 .../devicetree/bindings/power/supply/axp20x_ac_power.txt      | 4 ++++
 .../devicetree/bindings/power/supply/axp20x_battery.txt       | 1 +
 4 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-axp209.txt b/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
index fc42b2caa06d..da3c0efb5e76 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
@@ -11,6 +11,7 @@ This driver employs the per-pin muxing pattern.
 Required properties:
 - compatible: Should be one of:
 	- "x-powers,axp209-gpio"
+	- "x-powers,axp803-gpio"
 	- "x-powers,axp813-gpio"
 - #gpio-cells: Should be two. The first cell is the pin number and the
   second is the GPIO flags.
@@ -67,7 +68,7 @@ GPIO0	|	gpio_in, gpio_out, ldo, adc
 GPIO1	|	gpio_in, gpio_out, ldo, adc
 GPIO2	|	gpio_in, gpio_out
 
-axp813
+axp8x3
 ------
 GPIO	|	Functions
 ------------------------
diff --git a/Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt b/Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt
index 7a6313913923..247616099171 100644
--- a/Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt
@@ -4,6 +4,7 @@ Required properties:
   - compatible: should be one of:
     - "x-powers,axp209-adc",
     - "x-powers,axp221-adc",
+    - "x-powers,axp803-adc",
     - "x-powers,axp813-adc",
   - #io-channel-cells: should be 1,
 
@@ -39,7 +40,7 @@ AXP22x
  2 | batt_chrg_i
  3 | batt_dischrg_i
 
-AXP813
+AXP8x3
 ------
  0 | pmic_temp
  1 | gpio0_v
diff --git a/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt b/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
index 826e8a879121..97276a71e961 100644
--- a/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
+++ b/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
@@ -4,6 +4,8 @@ Required Properties:
  - compatible: One of:
 			"x-powers,axp202-ac-power-supply"
 			"x-powers,axp221-ac-power-supply"
+			"x-powers,axp803-ac-power-supply"
+			"x-powers,axp813-ac-power-supply"
 
 This node is a subnode of the axp20x PMIC.
 
@@ -13,6 +15,8 @@ reading ADC channels from the AXP20X ADC.
 The AXP22X is only able to tell if an AC power supply is present and
 usable.
 
+The AXP8X3 is able to limit current and supply voltage
+
 Example:
 
 &axp209 {
diff --git a/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt b/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
index 41916f69902c..780ebd7e3b84 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"
 			"x-powers,axp813-battery-power-supply"
 
 This node is a subnode of its respective PMIC DT node.
-- 
2.17.1

Re: [PATCH 1/4] dt-bindings: add compatibles for AXP803 AC and battery power supplies

[Jonathan Cameron]

On Thu,  4 Oct 2018 22:34:07 +0300
Oskari Lemmela <oskari@lemmela.net> wrote:

> AXP803 PMIC is register compatible with AXP813.
> 
> AXP803/AXP813 are capable to limit input current and supply voltage.
> 
> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
Hi Oskari,

It would be better to split this up as different aspects are going
to ideally be applied through different kernel trees.

I know that already got commented on for the later patch but just wanted
to point out it applies here as well.

One comment inline about avoiding wildcards given the propensity of
hardware manufacturers to confuse us on their naming schemes.

Jonathan

> ---
>  Documentation/devicetree/bindings/gpio/gpio-axp209.txt        | 3 ++-
>  Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt      | 3 ++-
>  .../devicetree/bindings/power/supply/axp20x_ac_power.txt      | 4 ++++
>  .../devicetree/bindings/power/supply/axp20x_battery.txt       | 1 +
>  4 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-axp209.txt b/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
> index fc42b2caa06d..da3c0efb5e76 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
> @@ -11,6 +11,7 @@ This driver employs the per-pin muxing pattern.
>  Required properties:
>  - compatible: Should be one of:
>  	- "x-powers,axp209-gpio"
> +	- "x-powers,axp803-gpio"
>  	- "x-powers,axp813-gpio"
>  - #gpio-cells: Should be two. The first cell is the pin number and the
>    second is the GPIO flags.
> @@ -67,7 +68,7 @@ GPIO0	|	gpio_in, gpio_out, ldo, adc
>  GPIO1	|	gpio_in, gpio_out, ldo, adc
>  GPIO2	|	gpio_in, gpio_out
>  
> -axp813
> +axp8x3
Avoid wild cards like this.   It if far too common to have a hardware
manufacturer not keep to what seems like an established naming convention
and introduce something entirely different in the middle of an existing
series of names.

>  ------
>  GPIO	|	Functions
>  ------------------------
> diff --git a/Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt b/Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt
> index 7a6313913923..247616099171 100644
> --- a/Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt
> @@ -4,6 +4,7 @@ Required properties:
>    - compatible: should be one of:
>      - "x-powers,axp209-adc",
>      - "x-powers,axp221-adc",
> +    - "x-powers,axp803-adc",
>      - "x-powers,axp813-adc",
>    - #io-channel-cells: should be 1,
>  
> @@ -39,7 +40,7 @@ AXP22x
>   2 | batt_chrg_i
>   3 | batt_dischrg_i
>  
> -AXP813
> +AXP8x3
>  ------
>   0 | pmic_temp
>   1 | gpio0_v
> diff --git a/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt b/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
> index 826e8a879121..97276a71e961 100644
> --- a/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
> +++ b/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
> @@ -4,6 +4,8 @@ Required Properties:
>   - compatible: One of:
>  			"x-powers,axp202-ac-power-supply"
>  			"x-powers,axp221-ac-power-supply"
> +			"x-powers,axp803-ac-power-supply"
> +			"x-powers,axp813-ac-power-supply"
>  
>  This node is a subnode of the axp20x PMIC.
>  
> @@ -13,6 +15,8 @@ reading ADC channels from the AXP20X ADC.
>  The AXP22X is only able to tell if an AC power supply is present and
>  usable.
>  
> +The AXP8X3 is able to limit current and supply voltage
> +
>  Example:
>  
>  &axp209 {
> diff --git a/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt b/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
> index 41916f69902c..780ebd7e3b84 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"
>  			"x-powers,axp813-battery-power-supply"
>  
>  This node is a subnode of its respective PMIC DT node.

[PATCH 2/4] ARM: dtsi: axp81x: add AC power supply subnode

[Oskari Lemmela]

Add AC power supply subnode for AXP81X PMIC.

Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
---
 arch/arm/boot/dts/axp81x.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/axp81x.dtsi b/arch/arm/boot/dts/axp81x.dtsi
index 043c717dcef1..67c17151b584 100644
--- a/arch/arm/boot/dts/axp81x.dtsi
+++ b/arch/arm/boot/dts/axp81x.dtsi
@@ -69,6 +69,11 @@
 		};
 	};
 
+	ac_power_supply: ac-power-supply {
+		compatible = "x-powers,axp813-ac-power-supply";
+		status = "disabled";
+	};
+
 	battery_power_supply: battery-power-supply {
 		compatible = "x-powers,axp813-battery-power-supply";
 		status = "disabled";
-- 
2.17.1

[PATCH 3/4] arm64: allwinner: a64: add battery and AC power supply support

[Oskari Lemmela]

Add AXP803 DT nodes for AC and battery power supplies.
Enable AC and battery power supplies for sopine-baseboard.

Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
---
 arch/arm64/boot/dts/allwinner/axp803.dtsi     | 31 +++++++++++++++++++
 .../allwinner/sun50i-a64-sopine-baseboard.dts |  8 +++++
 2 files changed, 39 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/axp803.dtsi b/arch/arm64/boot/dts/allwinner/axp803.dtsi
index e5eae8bafc42..d23e5d1e9310 100644
--- a/arch/arm64/boot/dts/allwinner/axp803.dtsi
+++ b/arch/arm64/boot/dts/allwinner/axp803.dtsi
@@ -49,6 +49,37 @@
 	interrupt-controller;
 	#interrupt-cells = <1>;
 
+	axp_adc: adc {
+		compatible = "x-powers,axp803-adc";
+		#io-channel-cells = <1>;
+	};
+
+	axp_gpio: gpio {
+		compatible = "x-powers,axp803-gpio";
+		gpio-controller;
+		#gpio-cells = <2>;
+
+		gpio0_ldo: gpio0-ldo {
+			pins = "GPIO0";
+			function = "ldo";
+		};
+
+		gpio1_ldo: gpio1-ldo {
+			pins = "GPIO1";
+			function = "ldo";
+		};
+	};
+
+	ac_power_supply: ac-power-supply {
+		compatible = "x-powers,axp803-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>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
index c21f2331add6..335cf2263d19 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
@@ -69,6 +69,14 @@
 	};
 };
 
+&ac_power_supply {
+	status = "okay";
+};
+
+&battery_power_supply {
+	status = "okay";
+};
+
 &ehci0 {
 	status = "okay";
 };
-- 
2.17.1

[PATCH 4/4] power: supply: add AXP803/AXP813 AC and battery power supply support

[Oskari Lemmela]

AXP803 PMIC is register compatible with AXP813.

Added support for AXP803/AXP813 AC power supply.
AXP8x3 is capable to limit input current and minimum input voltage.
Both of these register values are writeable.

Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
---
 drivers/iio/adc/axp20x_adc.c           |   1 +
 drivers/mfd/axp20x.c                   |  22 ++-
 drivers/pinctrl/pinctrl-axp209.c       |   1 +
 drivers/power/supply/axp20x_ac_power.c | 194 +++++++++++++++++++++++++
 drivers/power/supply/axp20x_battery.c  |   3 +
 include/linux/mfd/axp20x.h             |   1 +
 6 files changed, 221 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
index 5be789269353..f919a16a8533 100644
--- a/drivers/iio/adc/axp20x_adc.c
+++ b/drivers/iio/adc/axp20x_adc.c
@@ -641,6 +641,7 @@ static const struct axp_data axp813_data = {
 static const struct of_device_id axp20x_adc_of_match[] = {
 	{ .compatible = "x-powers,axp209-adc", .data = (void *)&axp20x_data, },
 	{ .compatible = "x-powers,axp221-adc", .data = (void *)&axp22x_data, },
+	{ .compatible = "x-powers,axp803-adc", .data = (void *)&axp813_data, },
 	{ .compatible = "x-powers,axp813-adc", .data = (void *)&axp813_data, },
 	{ /* sentinel */ }
 };
diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 0be511dd93d0..c540f17549d5 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -731,8 +731,23 @@ static const struct mfd_cell axp803_cells[] = {
 		.name			= "axp221-pek",
 		.num_resources		= ARRAY_SIZE(axp803_pek_resources),
 		.resources		= axp803_pek_resources,
+	}, {
+		.name			= "axp20x-regulator"
+	}, {
+		.name			= "axp20x-gpio",
+		.of_compatible		= "x-powers,axp803-gpio",
+	}, {
+		.name			= "axp813-adc",
+		.of_compatible		= "x-powers,axp803-adc",
+	}, {
+		.name		= "axp20x-battery-power-supply",
+		.of_compatible	= "x-powers,axp803-battery-power-supply",
+	}, {
+		.name		= "axp20x-ac-power-supply",
+		.of_compatible	= "x-powers,axp803-ac-power-supply",
+		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
+		.resources	= axp20x_ac_power_supply_resources,
 	},
-	{	.name			= "axp20x-regulator" },
 };
 
 static const struct mfd_cell axp806_self_working_cells[] = {
@@ -778,6 +793,11 @@ static const struct mfd_cell axp813_cells[] = {
 	}, {
 		.name		= "axp20x-battery-power-supply",
 		.of_compatible	= "x-powers,axp813-battery-power-supply",
+	}, {
+		.name		= "axp20x-ac-power-supply",
+		.of_compatible	= "x-powers,axp813-ac-power-supply",
+		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
+		.resources	= axp20x_ac_power_supply_resources,
 	},
 };
 
diff --git a/drivers/pinctrl/pinctrl-axp209.c b/drivers/pinctrl/pinctrl-axp209.c
index afd0b533c40a..21859579e0a2 100644
--- a/drivers/pinctrl/pinctrl-axp209.c
+++ b/drivers/pinctrl/pinctrl-axp209.c
@@ -387,6 +387,7 @@ static int axp20x_build_funcs_groups(struct platform_device *pdev)
 
 static const struct of_device_id axp20x_pctl_match[] = {
 	{ .compatible = "x-powers,axp209-gpio", .data = &axp20x_data, },
+	{ .compatible = "x-powers,axp803-gpio", .data = &axp813_data, },
 	{ .compatible = "x-powers,axp813-gpio", .data = &axp813_data, },
 	{ }
 };
diff --git a/drivers/power/supply/axp20x_ac_power.c b/drivers/power/supply/axp20x_ac_power.c
index 0771f951b11f..3247efb81cd1 100644
--- a/drivers/power/supply/axp20x_ac_power.c
+++ b/drivers/power/supply/axp20x_ac_power.c
@@ -27,6 +27,23 @@
 #define AXP20X_PWR_STATUS_ACIN_PRESENT	BIT(7)
 #define AXP20X_PWR_STATUS_ACIN_AVAIL	BIT(6)
 
+#define AXP8X3_PWR_ACIN_VHOLD		GENMASK(5, 3)
+#define AXP8X3_PWR_ACIN_VHOLD_4_0V	(0 << 3)
+#define AXP8X3_PWR_ACIN_VHOLD_4_1V	(1 << 3)
+#define AXP8X3_PWR_ACIN_VHOLD_4_2V	(2 << 3)
+#define AXP8X3_PWR_ACIN_VHOLD_4_3V	(3 << 3)
+#define AXP8X3_PWR_ACIN_VHOLD_4_4V	(4 << 3)
+#define AXP8X3_PWR_ACIN_VHOLD_4_5V	(5 << 3)
+#define AXP8X3_PWR_ACIN_VHOLD_4_6V	(6 << 3)
+#define AXP8X3_PWR_ACIN_VHOLD_4_7V	(7 << 3)
+#define AXP8X3_PWR_ACIN_CURR_LIMIT	GENMASK(2, 0)
+#define AXP8X3_PWR_ACIN_CURR_1_5A	0
+#define AXP8X3_PWR_ACIN_CURR_2_0A	1
+#define AXP8X3_PWR_ACIN_CURR_2_5A	2
+#define AXP8X3_PWR_ACIN_CURR_3_0A	3
+#define AXP8X3_PWR_ACIN_CURR_3_5A	4
+#define AXP8X3_PWR_ACIN_CURR_4_0A	5
+
 #define DRVNAME "axp20x-ac-power-supply"
 
 struct axp20x_ac_power {
@@ -102,6 +119,72 @@ static int axp20x_ac_power_get_property(struct power_supply *psy,
 
 		return 0;
 
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
+		ret = regmap_read(power->regmap, AXP803_ACIN_PATH_CTRL, &reg);
+		if (ret)
+			return ret;
+
+		switch (reg & AXP8X3_PWR_ACIN_VHOLD) {
+		case AXP8X3_PWR_ACIN_VHOLD_4_0V:
+			val->intval = 4000000;
+			break;
+		case AXP8X3_PWR_ACIN_VHOLD_4_1V:
+			val->intval = 4100000;
+			break;
+		case AXP8X3_PWR_ACIN_VHOLD_4_2V:
+			val->intval = 4200000;
+			break;
+		case AXP8X3_PWR_ACIN_VHOLD_4_3V:
+			val->intval = 4300000;
+			break;
+		case AXP8X3_PWR_ACIN_VHOLD_4_4V:
+			val->intval = 4400000;
+			break;
+		case AXP8X3_PWR_ACIN_VHOLD_4_5V:
+			val->intval = 4500000;
+			break;
+		case AXP8X3_PWR_ACIN_VHOLD_4_6V:
+			val->intval = 4600000;
+			break;
+		case AXP8X3_PWR_ACIN_VHOLD_4_7V:
+			val->intval = 4700000;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		return 0;
+
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+		ret = regmap_read(power->regmap, AXP803_ACIN_PATH_CTRL, &reg);
+		if (ret)
+			return ret;
+
+		switch (reg & AXP8X3_PWR_ACIN_CURR_LIMIT) {
+		case AXP8X3_PWR_ACIN_CURR_1_5A:
+			val->intval = 1500000;
+			break;
+		case AXP8X3_PWR_ACIN_CURR_2_0A:
+			val->intval = 2000000;
+			break;
+		case AXP8X3_PWR_ACIN_CURR_2_5A:
+			val->intval = 2500000;
+			break;
+		case AXP8X3_PWR_ACIN_CURR_3_0A:
+			val->intval = 3000000;
+			break;
+		case AXP8X3_PWR_ACIN_CURR_3_5A:
+			val->intval = 3500000;
+			break;
+		case AXP8X3_PWR_ACIN_CURR_4_0A:
+			val->intval = 4000000;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		return 0;
+
 	default:
 		return -EINVAL;
 	}
@@ -109,6 +192,88 @@ static int axp20x_ac_power_get_property(struct power_supply *psy,
 	return -EINVAL;
 }
 
+static int axp20x_ac_power_set_property(struct power_supply *psy,
+					enum power_supply_property psp,
+					const union power_supply_propval *val)
+{
+	struct axp20x_ac_power *power = power_supply_get_drvdata(psy);
+	int setval;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
+		switch (val->intval) {
+		case 4000000:
+			setval = AXP8X3_PWR_ACIN_VHOLD_4_0V;
+			break;
+		case 4100000:
+			setval = AXP8X3_PWR_ACIN_VHOLD_4_1V;
+			break;
+		case 4200000:
+			setval = AXP8X3_PWR_ACIN_VHOLD_4_2V;
+			break;
+		case 4300000:
+			setval = AXP8X3_PWR_ACIN_VHOLD_4_3V;
+			break;
+		case 4400000:
+			setval = AXP8X3_PWR_ACIN_VHOLD_4_4V;
+			break;
+		case 4500000:
+			setval = AXP8X3_PWR_ACIN_VHOLD_4_5V;
+			break;
+		case 4600000:
+			setval = AXP8X3_PWR_ACIN_VHOLD_4_6V;
+			break;
+		case 4700000:
+			setval = AXP8X3_PWR_ACIN_VHOLD_4_7V;
+			break;
+
+		default:
+			return -EINVAL;
+		}
+		return regmap_update_bits(power->regmap, AXP803_ACIN_PATH_CTRL,
+					  AXP8X3_PWR_ACIN_VHOLD, setval);
+
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+
+		switch (val->intval) {
+		case 1500000:
+			setval = AXP8X3_PWR_ACIN_CURR_1_5A;
+			break;
+		case 2000000:
+			setval = AXP8X3_PWR_ACIN_CURR_2_0A;
+			break;
+		case 2500000:
+			setval = AXP8X3_PWR_ACIN_CURR_2_5A;
+			break;
+		case 3000000:
+			setval = AXP8X3_PWR_ACIN_CURR_3_0A;
+			break;
+		case 3500000:
+			setval = AXP8X3_PWR_ACIN_CURR_3_5A;
+			break;
+		case 4000000:
+			setval = AXP8X3_PWR_ACIN_CURR_4_0A;
+			break;
+		default:
+			return -EINVAL;
+		}
+		return regmap_update_bits(power->regmap, AXP803_ACIN_PATH_CTRL,
+					  AXP8X3_PWR_ACIN_CURR_LIMIT, setval);
+
+	default:
+		return -EINVAL;
+	}
+
+	return -EINVAL;
+}
+
+static int axp20x_ac_power_prop_writeable(struct power_supply *psy,
+					  enum power_supply_property psp)
+{
+	return psp == POWER_SUPPLY_PROP_VOLTAGE_MIN ||
+	       psp == POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT;
+}
+
 static enum power_supply_property axp20x_ac_power_properties[] = {
 	POWER_SUPPLY_PROP_HEALTH,
 	POWER_SUPPLY_PROP_PRESENT,
@@ -123,6 +288,14 @@ static enum power_supply_property axp22x_ac_power_properties[] = {
 	POWER_SUPPLY_PROP_ONLINE,
 };
 
+static enum power_supply_property axp8x3_ac_power_properties[] = {
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN,
+	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+};
+
 static const struct power_supply_desc axp20x_ac_power_desc = {
 	.name = "axp20x-ac",
 	.type = POWER_SUPPLY_TYPE_MAINS,
@@ -139,6 +312,16 @@ static const struct power_supply_desc axp22x_ac_power_desc = {
 	.get_property = axp20x_ac_power_get_property,
 };
 
+static const struct power_supply_desc axp8x3_ac_power_desc = {
+	.name = "axp8x3-ac",
+	.type = POWER_SUPPLY_TYPE_MAINS,
+	.properties = axp8x3_ac_power_properties,
+	.num_properties = ARRAY_SIZE(axp8x3_ac_power_properties),
+	.property_is_writeable = axp20x_ac_power_prop_writeable,
+	.get_property = axp20x_ac_power_get_property,
+	.set_property = axp20x_ac_power_set_property,
+};
+
 struct axp_data {
 	const struct power_supply_desc	*power_desc;
 	bool				acin_adc;
@@ -154,6 +337,11 @@ static const struct axp_data axp22x_data = {
 	.acin_adc = false,
 };
 
+static const struct axp_data axp8x3_data = {
+	.power_desc = &axp8x3_ac_power_desc,
+	.acin_adc = false,
+};
+
 static int axp20x_ac_power_probe(struct platform_device *pdev)
 {
 	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
@@ -234,6 +422,12 @@ static const struct of_device_id axp20x_ac_power_match[] = {
 	}, {
 		.compatible = "x-powers,axp221-ac-power-supply",
 		.data = &axp22x_data,
+	}, {
+		.compatible = "x-powers,axp803-ac-power-supply",
+		.data = &axp8x3_data,
+	}, {
+		.compatible = "x-powers,axp813-ac-power-supply",
+		.data = &axp8x3_data,
 	}, { /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, axp20x_ac_power_match);
diff --git a/drivers/power/supply/axp20x_battery.c b/drivers/power/supply/axp20x_battery.c
index e84b6e4da14a..932027f4a993 100644
--- a/drivers/power/supply/axp20x_battery.c
+++ b/drivers/power/supply/axp20x_battery.c
@@ -537,6 +537,9 @@ static const struct of_device_id axp20x_battery_ps_id[] = {
 	}, {
 		.compatible = "x-powers,axp221-battery-power-supply",
 		.data = (void *)&axp221_data,
+	}, {
+		.compatible = "x-powers,axp803-battery-power-supply",
+		.data = (void *)&axp813_data,
 	}, {
 		.compatible = "x-powers,axp813-battery-power-supply",
 		.data = (void *)&axp813_data,
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 517e60eecbcb..23d58e243d01 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -266,6 +266,7 @@ enum axp20x_variants {
 #define AXP288_RT_BATT_V_H		0xa0
 #define AXP288_RT_BATT_V_L		0xa1
 
+#define AXP803_ACIN_PATH_CTRL		0x3a
 #define AXP813_ADC_RATE			0x85
 
 /* Fuel Gauge */
-- 
2.17.1

Re: [PATCH 4/4] power: supply: add AXP803/AXP813 AC and battery power supply support

[Quentin Schulz]

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

Hi Oskari,

On Thu, Oct 04, 2018 at 10:34:10PM +0300, Oskari Lemmela wrote:
> AXP803 PMIC is register compatible with AXP813.
> 
> Added support for AXP803/AXP813 AC power supply.
> AXP8x3 is capable to limit input current and minimum input voltage.
> Both of these register values are writeable.
> 
> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
> ---
>  drivers/iio/adc/axp20x_adc.c           |   1 +
>  drivers/mfd/axp20x.c                   |  22 ++-
>  drivers/pinctrl/pinctrl-axp209.c       |   1 +
>  drivers/power/supply/axp20x_ac_power.c | 194 +++++++++++++++++++++++++
>  drivers/power/supply/axp20x_battery.c  |   3 +
>  include/linux/mfd/axp20x.h             |   1 +
>  6 files changed, 221 insertions(+), 1 deletion(-)
> 

We usually want one commit per logical change. e.g. here, we would want
*at least* one commit for the ADC, one for the MFD, one for the pinctrl,
one for the battery, one for the AC.

> diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
> index 5be789269353..f919a16a8533 100644
> --- a/drivers/iio/adc/axp20x_adc.c
> +++ b/drivers/iio/adc/axp20x_adc.c
> @@ -641,6 +641,7 @@ static const struct axp_data axp813_data = {
>  static const struct of_device_id axp20x_adc_of_match[] = {
>  	{ .compatible = "x-powers,axp209-adc", .data = (void *)&axp20x_data, },
>  	{ .compatible = "x-powers,axp221-adc", .data = (void *)&axp22x_data, },
> +	{ .compatible = "x-powers,axp803-adc", .data = (void *)&axp813_data, },
>  	{ .compatible = "x-powers,axp813-adc", .data = (void *)&axp813_data, },
>  	{ /* sentinel */ }
>  };

Then it means AXP813 and AXP803 ADCs are identical. Use the compatible
of the AXP813, no need to add a compatible for each and every PMIC's ADC
that'll be compatible with one that already exists.

> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 0be511dd93d0..c540f17549d5 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -731,8 +731,23 @@ static const struct mfd_cell axp803_cells[] = {
>  		.name			= "axp221-pek",
>  		.num_resources		= ARRAY_SIZE(axp803_pek_resources),
>  		.resources		= axp803_pek_resources,
> +	}, {
> +		.name			= "axp20x-regulator"
> +	}, {
> +		.name			= "axp20x-gpio",
> +		.of_compatible		= "x-powers,axp803-gpio",
> +	}, {
> +		.name			= "axp813-adc",
> +		.of_compatible		= "x-powers,axp803-adc",
> +	}, {
> +		.name		= "axp20x-battery-power-supply",
> +		.of_compatible	= "x-powers,axp803-battery-power-supply",
> +	}, {
> +		.name		= "axp20x-ac-power-supply",
> +		.of_compatible	= "x-powers,axp803-ac-power-supply",
> +		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
> +		.resources	= axp20x_ac_power_supply_resources,
>  	},
> -	{	.name			= "axp20x-regulator" },
>  };
>  
>  static const struct mfd_cell axp806_self_working_cells[] = {
> @@ -778,6 +793,11 @@ static const struct mfd_cell axp813_cells[] = {
>  	}, {
>  		.name		= "axp20x-battery-power-supply",
>  		.of_compatible	= "x-powers,axp813-battery-power-supply",
> +	}, {
> +		.name		= "axp20x-ac-power-supply",
> +		.of_compatible	= "x-powers,axp813-ac-power-supply",
> +		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
> +		.resources	= axp20x_ac_power_supply_resources,
>  	},
>  };
>  
> diff --git a/drivers/pinctrl/pinctrl-axp209.c b/drivers/pinctrl/pinctrl-axp209.c
> index afd0b533c40a..21859579e0a2 100644
> --- a/drivers/pinctrl/pinctrl-axp209.c
> +++ b/drivers/pinctrl/pinctrl-axp209.c
> @@ -387,6 +387,7 @@ static int axp20x_build_funcs_groups(struct platform_device *pdev)
>  
>  static const struct of_device_id axp20x_pctl_match[] = {
>  	{ .compatible = "x-powers,axp209-gpio", .data = &axp20x_data, },
> +	{ .compatible = "x-powers,axp803-gpio", .data = &axp813_data, },
>  	{ .compatible = "x-powers,axp813-gpio", .data = &axp813_data, },
>  	{ }
>  };

Same here. No need for a new compatible.

> diff --git a/drivers/power/supply/axp20x_ac_power.c b/drivers/power/supply/axp20x_ac_power.c
> index 0771f951b11f..3247efb81cd1 100644
> --- a/drivers/power/supply/axp20x_ac_power.c
> +++ b/drivers/power/supply/axp20x_ac_power.c
> @@ -27,6 +27,23 @@
>  #define AXP20X_PWR_STATUS_ACIN_PRESENT	BIT(7)
>  #define AXP20X_PWR_STATUS_ACIN_AVAIL	BIT(6)
>  
> +#define AXP8X3_PWR_ACIN_VHOLD		GENMASK(5, 3)

I would add _MASK at the end to be extra explicit.

> +#define AXP8X3_PWR_ACIN_VHOLD_4_0V	(0 << 3)
> +#define AXP8X3_PWR_ACIN_VHOLD_4_1V	(1 << 3)
> +#define AXP8X3_PWR_ACIN_VHOLD_4_2V	(2 << 3)
> +#define AXP8X3_PWR_ACIN_VHOLD_4_3V	(3 << 3)
> +#define AXP8X3_PWR_ACIN_VHOLD_4_4V	(4 << 3)
> +#define AXP8X3_PWR_ACIN_VHOLD_4_5V	(5 << 3)
> +#define AXP8X3_PWR_ACIN_VHOLD_4_6V	(6 << 3)
> +#define AXP8X3_PWR_ACIN_VHOLD_4_7V	(7 << 3)

You could define a macro for that, e.g.:
#define AXP8X3_PWR_ACIN_VHOLD_mV_reg(x)	((((x) - 4000) / 100) << 3)
#define AXP8X3_PWR_ACIN_VHOLD_reg_mV(x)	(((x) >> 3) * 100 + 4000)

> +#define AXP8X3_PWR_ACIN_CURR_LIMIT	GENMASK(2, 0)

I would add _MASK at the end to be extra explicit.

> +#define AXP8X3_PWR_ACIN_CURR_1_5A	0
> +#define AXP8X3_PWR_ACIN_CURR_2_0A	1
> +#define AXP8X3_PWR_ACIN_CURR_2_5A	2
> +#define AXP8X3_PWR_ACIN_CURR_3_0A	3
> +#define AXP8X3_PWR_ACIN_CURR_3_5A	4
> +#define AXP8X3_PWR_ACIN_CURR_4_0A	5
> +

#define AXP8X3_PWR_ACIN_CURR_mA_reg(x)	(((x) / 500) - 3)
#define AXP8X3_PWR_ACIN_CURR_reg_mA(x)	(((x) + 3 ) * 500)

They are not very pretty macro names but I'll let you figure out one
that are easier to understand :)

Also, you might want to use µA/µV so that it returns directly the value
wanted by the power-supply subsystem.

>  #define DRVNAME "axp20x-ac-power-supply"
>  
>  struct axp20x_ac_power {
> @@ -102,6 +119,72 @@ static int axp20x_ac_power_get_property(struct power_supply *psy,
>  
>  		return 0;
>  
> +	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
> +		ret = regmap_read(power->regmap, AXP803_ACIN_PATH_CTRL, &reg);
> +		if (ret)
> +			return ret;
> +

val->intval = AXP8X3_PWR_ACIN_VHOLD_reg_mV(reg & AXP8X3_PWR_ACIN_VHOLD) * 1000;

return 0;

> +		switch (reg & AXP8X3_PWR_ACIN_VHOLD) {
> +		case AXP8X3_PWR_ACIN_VHOLD_4_0V:
> +			val->intval = 4000000;
> +			break;
> +		case AXP8X3_PWR_ACIN_VHOLD_4_1V:
> +			val->intval = 4100000;
> +			break;
> +		case AXP8X3_PWR_ACIN_VHOLD_4_2V:
> +			val->intval = 4200000;
> +			break;
> +		case AXP8X3_PWR_ACIN_VHOLD_4_3V:
> +			val->intval = 4300000;
> +			break;
> +		case AXP8X3_PWR_ACIN_VHOLD_4_4V:
> +			val->intval = 4400000;
> +			break;
> +		case AXP8X3_PWR_ACIN_VHOLD_4_5V:
> +			val->intval = 4500000;
> +			break;
> +		case AXP8X3_PWR_ACIN_VHOLD_4_6V:
> +			val->intval = 4600000;
> +			break;
> +		case AXP8X3_PWR_ACIN_VHOLD_4_7V:
> +			val->intval = 4700000;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +
> +		return 0;
> +
> +	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> +		ret = regmap_read(power->regmap, AXP803_ACIN_PATH_CTRL, &reg);
> +		if (ret)
> +			return ret;
> +

val->intval = AXP8X3_PWR_ACIN_CURR_reg_mA(reg & AXP8X3_PWR_ACIN_CURR_LIMIT) * 1000;

return 0;

> +		switch (reg & AXP8X3_PWR_ACIN_CURR_LIMIT) {
> +		case AXP8X3_PWR_ACIN_CURR_1_5A:
> +			val->intval = 1500000;
> +			break;
> +		case AXP8X3_PWR_ACIN_CURR_2_0A:
> +			val->intval = 2000000;
> +			break;
> +		case AXP8X3_PWR_ACIN_CURR_2_5A:
> +			val->intval = 2500000;
> +			break;
> +		case AXP8X3_PWR_ACIN_CURR_3_0A:
> +			val->intval = 3000000;
> +			break;
> +		case AXP8X3_PWR_ACIN_CURR_3_5A:
> +			val->intval = 3500000;
> +			break;
> +		case AXP8X3_PWR_ACIN_CURR_4_0A:
> +			val->intval = 4000000;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +
> +		return 0;
> +
>  	default:
>  		return -EINVAL;
>  	}
> @@ -109,6 +192,88 @@ static int axp20x_ac_power_get_property(struct power_supply *psy,
>  	return -EINVAL;
>  }
>  
> +static int axp20x_ac_power_set_property(struct power_supply *psy,
> +					enum power_supply_property psp,
> +					const union power_supply_propval *val)
> +{
> +	struct axp20x_ac_power *power = power_supply_get_drvdata(psy);
> +	int setval;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
> +		switch (val->intval) {
> +		case 4000000:
> +			setval = AXP8X3_PWR_ACIN_VHOLD_4_0V;
> +			break;
> +		case 4100000:
> +			setval = AXP8X3_PWR_ACIN_VHOLD_4_1V;
> +			break;
> +		case 4200000:
> +			setval = AXP8X3_PWR_ACIN_VHOLD_4_2V;
> +			break;
> +		case 4300000:
> +			setval = AXP8X3_PWR_ACIN_VHOLD_4_3V;
> +			break;
> +		case 4400000:
> +			setval = AXP8X3_PWR_ACIN_VHOLD_4_4V;
> +			break;
> +		case 4500000:
> +			setval = AXP8X3_PWR_ACIN_VHOLD_4_5V;
> +			break;
> +		case 4600000:
> +			setval = AXP8X3_PWR_ACIN_VHOLD_4_6V;
> +			break;
> +		case 4700000:
> +			setval = AXP8X3_PWR_ACIN_VHOLD_4_7V;
> +			break;
> +
> +		default:
> +			return -EINVAL;
> +		}
> +		return regmap_update_bits(power->regmap, AXP803_ACIN_PATH_CTRL,
> +					  AXP8X3_PWR_ACIN_VHOLD, setval);
> +

return regmap_update_bits(power->regmap, AXP803_ACIN_PATH_CTRL,
			  AXP8X3_PWR_ACIN_VHOLD,
			  AXP8X3_PWR_ACIN_VHOLD_mV_reg(val->intval / 1000));

> +	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> +
> +		switch (val->intval) {
> +		case 1500000:
> +			setval = AXP8X3_PWR_ACIN_CURR_1_5A;
> +			break;
> +		case 2000000:
> +			setval = AXP8X3_PWR_ACIN_CURR_2_0A;
> +			break;
> +		case 2500000:
> +			setval = AXP8X3_PWR_ACIN_CURR_2_5A;
> +			break;
> +		case 3000000:
> +			setval = AXP8X3_PWR_ACIN_CURR_3_0A;
> +			break;
> +		case 3500000:
> +			setval = AXP8X3_PWR_ACIN_CURR_3_5A;
> +			break;
> +		case 4000000:
> +			setval = AXP8X3_PWR_ACIN_CURR_4_0A;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		return regmap_update_bits(power->regmap, AXP803_ACIN_PATH_CTRL,
> +					  AXP8X3_PWR_ACIN_CURR_LIMIT, setval);
return regmap_update_bits(power->regmap, AXP803_ACIN_PATH_CTRL,
			  AXP8X3_PWR_ACIN_CURR_LIMIT,
			  AXP8X3_PWR_ACIN_CURR_LIMIT_mA_reg(val->intval / 1000));
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int axp20x_ac_power_prop_writeable(struct power_supply *psy,
> +					  enum power_supply_property psp)
> +{
> +	return psp == POWER_SUPPLY_PROP_VOLTAGE_MIN ||
> +	       psp == POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT;
> +}
> +
>  static enum power_supply_property axp20x_ac_power_properties[] = {
>  	POWER_SUPPLY_PROP_HEALTH,
>  	POWER_SUPPLY_PROP_PRESENT,
> @@ -123,6 +288,14 @@ static enum power_supply_property axp22x_ac_power_properties[] = {
>  	POWER_SUPPLY_PROP_ONLINE,
>  };
>  
> +static enum power_supply_property axp8x3_ac_power_properties[] = {

I'll let Maxime or Chen-Yu confirm but I think we use the name of the
first PMIC in the family, so it would be axp813_ac_power_properties
even if it applies to multiple variants of the PMIC.

> +	POWER_SUPPLY_PROP_HEALTH,
> +	POWER_SUPPLY_PROP_PRESENT,
> +	POWER_SUPPLY_PROP_ONLINE,
> +	POWER_SUPPLY_PROP_VOLTAGE_MIN,
> +	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> +};
> +
>  static const struct power_supply_desc axp20x_ac_power_desc = {
>  	.name = "axp20x-ac",
>  	.type = POWER_SUPPLY_TYPE_MAINS,
> @@ -139,6 +312,16 @@ static const struct power_supply_desc axp22x_ac_power_desc = {
>  	.get_property = axp20x_ac_power_get_property,
>  };
>  
> +static const struct power_supply_desc axp8x3_ac_power_desc = {
> +	.name = "axp8x3-ac",
> +	.type = POWER_SUPPLY_TYPE_MAINS,
> +	.properties = axp8x3_ac_power_properties,
> +	.num_properties = ARRAY_SIZE(axp8x3_ac_power_properties),

Naming convention here for the above.

> +	.property_is_writeable = axp20x_ac_power_prop_writeable,

I think we would want axp813_ac_power_prop_writeable if it's applicable
only for the AXP813 and not the AXP20X.

> +	.get_property = axp20x_ac_power_get_property,
> +	.set_property = axp20x_ac_power_set_property,
> +};
> +
>  struct axp_data {
>  	const struct power_supply_desc	*power_desc;
>  	bool				acin_adc;
> @@ -154,6 +337,11 @@ static const struct axp_data axp22x_data = {
>  	.acin_adc = false,
>  };
>  
> +static const struct axp_data axp8x3_data = {
> +	.power_desc = &axp8x3_ac_power_desc,
> +	.acin_adc = false,
> +};
> +
>  static int axp20x_ac_power_probe(struct platform_device *pdev)
>  {
>  	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
> @@ -234,6 +422,12 @@ static const struct of_device_id axp20x_ac_power_match[] = {
>  	}, {
>  		.compatible = "x-powers,axp221-ac-power-supply",
>  		.data = &axp22x_data,
> +	}, {
> +		.compatible = "x-powers,axp803-ac-power-supply",
> +		.data = &axp8x3_data,
> +	}, {
> +		.compatible = "x-powers,axp813-ac-power-supply",
> +		.data = &axp8x3_data,

So AXP813 and AXP803 AC drivers are identical, use the same compatible,
no need to add two.

>  	}, { /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, axp20x_ac_power_match);
> diff --git a/drivers/power/supply/axp20x_battery.c b/drivers/power/supply/axp20x_battery.c
> index e84b6e4da14a..932027f4a993 100644
> --- a/drivers/power/supply/axp20x_battery.c
> +++ b/drivers/power/supply/axp20x_battery.c
> @@ -537,6 +537,9 @@ static const struct of_device_id axp20x_battery_ps_id[] = {
>  	}, {
>  		.compatible = "x-powers,axp221-battery-power-supply",
>  		.data = (void *)&axp221_data,
> +	}, {
> +		.compatible = "x-powers,axp803-battery-power-supply",
> +		.data = (void *)&axp813_data,

Ditto.

>  	}, {
>  		.compatible = "x-powers,axp813-battery-power-supply",
>  		.data = (void *)&axp813_data,
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index 517e60eecbcb..23d58e243d01 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -266,6 +266,7 @@ enum axp20x_variants {
>  #define AXP288_RT_BATT_V_H		0xa0
>  #define AXP288_RT_BATT_V_L		0xa1
>  
> +#define AXP803_ACIN_PATH_CTRL		0x3a
>  #define AXP813_ADC_RATE			0x85
>  

Basically, the only changes you need to keep are those in
drivers/power/supply/axp20x_ac_power.c and include/linux/mfd/axp20x.h.

Thanks,
Quentin

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

Re: [PATCH 4/4] power: supply: add AXP803/AXP813 AC and battery power supply support

[Oskari Lemmelä]

Hi Quentin,

On 05.10.2018 11:29, Quentin Schulz wrote:
> Hi Oskari,
>
> On Thu, Oct 04, 2018 at 10:34:10PM +0300, Oskari Lemmela wrote:
>> AXP803 PMIC is register compatible with AXP813.
>>
>> Added support for AXP803/AXP813 AC power supply.
>> AXP8x3 is capable to limit input current and minimum input voltage.
>> Both of these register values are writeable.
>>
>> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
>> ---
>>  drivers/iio/adc/axp20x_adc.c           |   1 +
>>  drivers/mfd/axp20x.c                   |  22 ++-
>>  drivers/pinctrl/pinctrl-axp209.c       |   1 +
>>  drivers/power/supply/axp20x_ac_power.c | 194 +++++++++++++++++++++++++
>>  drivers/power/supply/axp20x_battery.c  |   3 +
>>  include/linux/mfd/axp20x.h             |   1 +
>>  6 files changed, 221 insertions(+), 1 deletion(-)
>>
> We usually want one commit per logical change. e.g. here, we would want
> *at least* one commit for the ADC, one for the MFD, one for the pinctrl,
> one for the battery, one for the AC.
This was my very first kernel patch. I was thinking based on git log that
I should split commits even more. Now I know and do this in next versions.
>> diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
>> index 5be789269353..f919a16a8533 100644
>> --- a/drivers/iio/adc/axp20x_adc.c
>> +++ b/drivers/iio/adc/axp20x_adc.c
>> @@ -641,6 +641,7 @@ static const struct axp_data axp813_data = {
>>  static const struct of_device_id axp20x_adc_of_match[] = {
>>  	{ .compatible = "x-powers,axp209-adc", .data = (void *)&axp20x_data, },
>>  	{ .compatible = "x-powers,axp221-adc", .data = (void *)&axp22x_data, },
>> +	{ .compatible = "x-powers,axp803-adc", .data = (void *)&axp813_data, },
>>  	{ .compatible = "x-powers,axp813-adc", .data = (void *)&axp813_data, },
>>  	{ /* sentinel */ }
>>  };
> Then it means AXP813 and AXP803 ADCs are identical. Use the compatible
> of the AXP813, no need to add a compatible for each and every PMIC's ADC
> that'll be compatible with one that already exists.
Understood. I suppose if some feature doesn't work for both models then
we have to do new compatible?
>> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
>> index 0be511dd93d0..c540f17549d5 100644
>> --- a/drivers/mfd/axp20x.c
>> +++ b/drivers/mfd/axp20x.c
>> @@ -731,8 +731,23 @@ static const struct mfd_cell axp803_cells[] = {
>>  		.name			= "axp221-pek",
>>  		.num_resources		= ARRAY_SIZE(axp803_pek_resources),
>>  		.resources		= axp803_pek_resources,
>> +	}, {
>> +		.name			= "axp20x-regulator"
>> +	}, {
>> +		.name			= "axp20x-gpio",
>> +		.of_compatible		= "x-powers,axp803-gpio",
>> +	}, {
>> +		.name			= "axp813-adc",
>> +		.of_compatible		= "x-powers,axp803-adc",
>> +	}, {
>> +		.name		= "axp20x-battery-power-supply",
>> +		.of_compatible	= "x-powers,axp803-battery-power-supply",
>> +	}, {
>> +		.name		= "axp20x-ac-power-supply",
>> +		.of_compatible	= "x-powers,axp803-ac-power-supply",
>> +		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
>> +		.resources	= axp20x_ac_power_supply_resources,
>>  	},
>> -	{	.name			= "axp20x-regulator" },
>>  };
>>  
>>  static const struct mfd_cell axp806_self_working_cells[] = {
>> @@ -778,6 +793,11 @@ static const struct mfd_cell axp813_cells[] = {
>>  	}, {
>>  		.name		= "axp20x-battery-power-supply",
>>  		.of_compatible	= "x-powers,axp813-battery-power-supply",
>> +	}, {
>> +		.name		= "axp20x-ac-power-supply",
>> +		.of_compatible	= "x-powers,axp813-ac-power-supply",
>> +		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
>> +		.resources	= axp20x_ac_power_supply_resources,
>>  	},
>>  };
>>  
>> diff --git a/drivers/pinctrl/pinctrl-axp209.c b/drivers/pinctrl/pinctrl-axp209.c
>> index afd0b533c40a..21859579e0a2 100644
>> --- a/drivers/pinctrl/pinctrl-axp209.c
>> +++ b/drivers/pinctrl/pinctrl-axp209.c
>> @@ -387,6 +387,7 @@ static int axp20x_build_funcs_groups(struct platform_device *pdev)
>>  
>>  static const struct of_device_id axp20x_pctl_match[] = {
>>  	{ .compatible = "x-powers,axp209-gpio", .data = &axp20x_data, },
>> +	{ .compatible = "x-powers,axp803-gpio", .data = &axp813_data, },
>>  	{ .compatible = "x-powers,axp813-gpio", .data = &axp813_data, },
>>  	{ }
>>  };
> Same here. No need for a new compatible.
>
>> diff --git a/drivers/power/supply/axp20x_ac_power.c b/drivers/power/supply/axp20x_ac_power.c
>> index 0771f951b11f..3247efb81cd1 100644
>> --- a/drivers/power/supply/axp20x_ac_power.c
>> +++ b/drivers/power/supply/axp20x_ac_power.c
>> @@ -27,6 +27,23 @@
>>  #define AXP20X_PWR_STATUS_ACIN_PRESENT	BIT(7)
>>  #define AXP20X_PWR_STATUS_ACIN_AVAIL	BIT(6)
>>  
>> +#define AXP8X3_PWR_ACIN_VHOLD		GENMASK(5, 3)
> I would add _MASK at the end to be extra explicit.
Good idea. I'll change that in both places.
>> +#define AXP8X3_PWR_ACIN_VHOLD_4_0V	(0 << 3)
>> +#define AXP8X3_PWR_ACIN_VHOLD_4_1V	(1 << 3)
>> +#define AXP8X3_PWR_ACIN_VHOLD_4_2V	(2 << 3)
>> +#define AXP8X3_PWR_ACIN_VHOLD_4_3V	(3 << 3)
>> +#define AXP8X3_PWR_ACIN_VHOLD_4_4V	(4 << 3)
>> +#define AXP8X3_PWR_ACIN_VHOLD_4_5V	(5 << 3)
>> +#define AXP8X3_PWR_ACIN_VHOLD_4_6V	(6 << 3)
>> +#define AXP8X3_PWR_ACIN_VHOLD_4_7V	(7 << 3)
> You could define a macro for that, e.g.:
> #define AXP8X3_PWR_ACIN_VHOLD_mV_reg(x)	((((x) - 4000) / 100) << 3)
> #define AXP8X3_PWR_ACIN_VHOLD_reg_mV(x)	(((x) >> 3) * 100 + 4000)
>
>> +#define AXP8X3_PWR_ACIN_CURR_LIMIT	GENMASK(2, 0)
> I would add _MASK at the end to be extra explicit.
>
>> +#define AXP8X3_PWR_ACIN_CURR_1_5A	0
>> +#define AXP8X3_PWR_ACIN_CURR_2_0A	1
>> +#define AXP8X3_PWR_ACIN_CURR_2_5A	2
>> +#define AXP8X3_PWR_ACIN_CURR_3_0A	3
>> +#define AXP8X3_PWR_ACIN_CURR_3_5A	4
>> +#define AXP8X3_PWR_ACIN_CURR_4_0A	5
>> +
> #define AXP8X3_PWR_ACIN_CURR_mA_reg(x)	(((x) / 500) - 3)
> #define AXP8X3_PWR_ACIN_CURR_reg_mA(x)	(((x) + 3 ) * 500)
>
> They are not very pretty macro names but I'll let you figure out one
> that are easier to understand :)
>
> Also, you might want to use µA/µV so that it returns directly the value
> wanted by the power-supply subsystem.
Thanks. This way code will be much more compact. I'll try think
better macro names.
>>  #define DRVNAME "axp20x-ac-power-supply"
>>  
>>  struct axp20x_ac_power {
>> @@ -102,6 +119,72 @@ static int axp20x_ac_power_get_property(struct power_supply *psy,
>>  
>>  		return 0;
>>  
>> +	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
>> +		ret = regmap_read(power->regmap, AXP803_ACIN_PATH_CTRL, &reg);
>> +		if (ret)
>> +			return ret;
>> +
> val->intval = AXP8X3_PWR_ACIN_VHOLD_reg_mV(reg & AXP8X3_PWR_ACIN_VHOLD) * 1000;
>
> return 0;
>
>> +		switch (reg & AXP8X3_PWR_ACIN_VHOLD) {
>> +		case AXP8X3_PWR_ACIN_VHOLD_4_0V:
>> +			val->intval = 4000000;
>> +			break;
>> +		case AXP8X3_PWR_ACIN_VHOLD_4_1V:
>> +			val->intval = 4100000;
>> +			break;
>> +		case AXP8X3_PWR_ACIN_VHOLD_4_2V:
>> +			val->intval = 4200000;
>> +			break;
>> +		case AXP8X3_PWR_ACIN_VHOLD_4_3V:
>> +			val->intval = 4300000;
>> +			break;
>> +		case AXP8X3_PWR_ACIN_VHOLD_4_4V:
>> +			val->intval = 4400000;
>> +			break;
>> +		case AXP8X3_PWR_ACIN_VHOLD_4_5V:
>> +			val->intval = 4500000;
>> +			break;
>> +		case AXP8X3_PWR_ACIN_VHOLD_4_6V:
>> +			val->intval = 4600000;
>> +			break;
>> +		case AXP8X3_PWR_ACIN_VHOLD_4_7V:
>> +			val->intval = 4700000;
>> +			break;
>> +		default:
>> +			return -EINVAL;
>> +		}
>> +
>> +		return 0;
>> +
>> +	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
>> +		ret = regmap_read(power->regmap, AXP803_ACIN_PATH_CTRL, &reg);
>> +		if (ret)
>> +			return ret;
>> +
> val->intval = AXP8X3_PWR_ACIN_CURR_reg_mA(reg & AXP8X3_PWR_ACIN_CURR_LIMIT) * 1000;
>
> return 0;
>
>> +		switch (reg & AXP8X3_PWR_ACIN_CURR_LIMIT) {
>> +		case AXP8X3_PWR_ACIN_CURR_1_5A:
>> +			val->intval = 1500000;
>> +			break;
>> +		case AXP8X3_PWR_ACIN_CURR_2_0A:
>> +			val->intval = 2000000;
>> +			break;
>> +		case AXP8X3_PWR_ACIN_CURR_2_5A:
>> +			val->intval = 2500000;
>> +			break;
>> +		case AXP8X3_PWR_ACIN_CURR_3_0A:
>> +			val->intval = 3000000;
>> +			break;
>> +		case AXP8X3_PWR_ACIN_CURR_3_5A:
>> +			val->intval = 3500000;
>> +			break;
>> +		case AXP8X3_PWR_ACIN_CURR_4_0A:
>> +			val->intval = 4000000;
>> +			break;
>> +		default:
>> +			return -EINVAL;
>> +		}
>> +
>> +		return 0;
>> +
>>  	default:
>>  		return -EINVAL;
>>  	}
>> @@ -109,6 +192,88 @@ static int axp20x_ac_power_get_property(struct power_supply *psy,
>>  	return -EINVAL;
>>  }
>>  
>> +static int axp20x_ac_power_set_property(struct power_supply *psy,
>> +					enum power_supply_property psp,
>> +					const union power_supply_propval *val)
>> +{
>> +	struct axp20x_ac_power *power = power_supply_get_drvdata(psy);
>> +	int setval;
>> +
>> +	switch (psp) {
>> +	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
>> +		switch (val->intval) {
>> +		case 4000000:
>> +			setval = AXP8X3_PWR_ACIN_VHOLD_4_0V;
>> +			break;
>> +		case 4100000:
>> +			setval = AXP8X3_PWR_ACIN_VHOLD_4_1V;
>> +			break;
>> +		case 4200000:
>> +			setval = AXP8X3_PWR_ACIN_VHOLD_4_2V;
>> +			break;
>> +		case 4300000:
>> +			setval = AXP8X3_PWR_ACIN_VHOLD_4_3V;
>> +			break;
>> +		case 4400000:
>> +			setval = AXP8X3_PWR_ACIN_VHOLD_4_4V;
>> +			break;
>> +		case 4500000:
>> +			setval = AXP8X3_PWR_ACIN_VHOLD_4_5V;
>> +			break;
>> +		case 4600000:
>> +			setval = AXP8X3_PWR_ACIN_VHOLD_4_6V;
>> +			break;
>> +		case 4700000:
>> +			setval = AXP8X3_PWR_ACIN_VHOLD_4_7V;
>> +			break;
>> +
>> +		default:
>> +			return -EINVAL;
>> +		}
>> +		return regmap_update_bits(power->regmap, AXP803_ACIN_PATH_CTRL,
>> +					  AXP8X3_PWR_ACIN_VHOLD, setval);
>> +
> return regmap_update_bits(power->regmap, AXP803_ACIN_PATH_CTRL,
> 			  AXP8X3_PWR_ACIN_VHOLD,
> 			  AXP8X3_PWR_ACIN_VHOLD_mV_reg(val->intval / 1000));
>
>> +	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
>> +
>> +		switch (val->intval) {
>> +		case 1500000:
>> +			setval = AXP8X3_PWR_ACIN_CURR_1_5A;
>> +			break;
>> +		case 2000000:
>> +			setval = AXP8X3_PWR_ACIN_CURR_2_0A;
>> +			break;
>> +		case 2500000:
>> +			setval = AXP8X3_PWR_ACIN_CURR_2_5A;
>> +			break;
>> +		case 3000000:
>> +			setval = AXP8X3_PWR_ACIN_CURR_3_0A;
>> +			break;
>> +		case 3500000:
>> +			setval = AXP8X3_PWR_ACIN_CURR_3_5A;
>> +			break;
>> +		case 4000000:
>> +			setval = AXP8X3_PWR_ACIN_CURR_4_0A;
>> +			break;
>> +		default:
>> +			return -EINVAL;
>> +		}
>> +		return regmap_update_bits(power->regmap, AXP803_ACIN_PATH_CTRL,
>> +					  AXP8X3_PWR_ACIN_CURR_LIMIT, setval);
> return regmap_update_bits(power->regmap, AXP803_ACIN_PATH_CTRL,
> 			  AXP8X3_PWR_ACIN_CURR_LIMIT,
> 			  AXP8X3_PWR_ACIN_CURR_LIMIT_mA_reg(val->intval / 1000));
>> +
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static int axp20x_ac_power_prop_writeable(struct power_supply *psy,
>> +					  enum power_supply_property psp)
>> +{
>> +	return psp == POWER_SUPPLY_PROP_VOLTAGE_MIN ||
>> +	       psp == POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT;
>> +}
>> +
>>  static enum power_supply_property axp20x_ac_power_properties[] = {
>>  	POWER_SUPPLY_PROP_HEALTH,
>>  	POWER_SUPPLY_PROP_PRESENT,
>> @@ -123,6 +288,14 @@ static enum power_supply_property axp22x_ac_power_properties[] = {
>>  	POWER_SUPPLY_PROP_ONLINE,
>>  };
>>  
>> +static enum power_supply_property axp8x3_ac_power_properties[] = {
> I'll let Maxime or Chen-Yu confirm but I think we use the name of the
> first PMIC in the family, so it would be axp813_ac_power_properties
> even if it applies to multiple variants of the PMIC.
Makes sense. I'll replace all axp803,axp8x3 strings with axp813 ones.
>> +	POWER_SUPPLY_PROP_HEALTH,
>> +	POWER_SUPPLY_PROP_PRESENT,
>> +	POWER_SUPPLY_PROP_ONLINE,
>> +	POWER_SUPPLY_PROP_VOLTAGE_MIN,
>> +	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
>> +};
>> +
>>  static const struct power_supply_desc axp20x_ac_power_desc = {
>>  	.name = "axp20x-ac",
>>  	.type = POWER_SUPPLY_TYPE_MAINS,
>> @@ -139,6 +312,16 @@ static const struct power_supply_desc axp22x_ac_power_desc = {
>>  	.get_property = axp20x_ac_power_get_property,
>>  };
>>  
>> +static const struct power_supply_desc axp8x3_ac_power_desc = {
>> +	.name = "axp8x3-ac",
>> +	.type = POWER_SUPPLY_TYPE_MAINS,
>> +	.properties = axp8x3_ac_power_properties,
>> +	.num_properties = ARRAY_SIZE(axp8x3_ac_power_properties),
> Naming convention here for the above.
>
>> +	.property_is_writeable = axp20x_ac_power_prop_writeable,
> I think we would want axp813_ac_power_prop_writeable if it's applicable
> only for the AXP813 and not the AXP20X.
Noted.
>> +	.get_property = axp20x_ac_power_get_property,
>> +	.set_property = axp20x_ac_power_set_property,
>> +};
>> +
>>  struct axp_data {
>>  	const struct power_supply_desc	*power_desc;
>>  	bool				acin_adc;
>> @@ -154,6 +337,11 @@ static const struct axp_data axp22x_data = {
>>  	.acin_adc = false,
>>  };
>>  
>> +static const struct axp_data axp8x3_data = {
>> +	.power_desc = &axp8x3_ac_power_desc,
>> +	.acin_adc = false,
>> +};
>> +
>>  static int axp20x_ac_power_probe(struct platform_device *pdev)
>>  {
>>  	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
>> @@ -234,6 +422,12 @@ static const struct of_device_id axp20x_ac_power_match[] = {
>>  	}, {
>>  		.compatible = "x-powers,axp221-ac-power-supply",
>>  		.data = &axp22x_data,
>> +	}, {
>> +		.compatible = "x-powers,axp803-ac-power-supply",
>> +		.data = &axp8x3_data,
>> +	}, {
>> +		.compatible = "x-powers,axp813-ac-power-supply",
>> +		.data = &axp8x3_data,
> So AXP813 and AXP803 AC drivers are identical, use the same compatible,
> no need to add two.
>
>>  	}, { /* sentinel */ }
>>  };
>>  MODULE_DEVICE_TABLE(of, axp20x_ac_power_match);
>> diff --git a/drivers/power/supply/axp20x_battery.c b/drivers/power/supply/axp20x_battery.c
>> index e84b6e4da14a..932027f4a993 100644
>> --- a/drivers/power/supply/axp20x_battery.c
>> +++ b/drivers/power/supply/axp20x_battery.c
>> @@ -537,6 +537,9 @@ static const struct of_device_id axp20x_battery_ps_id[] = {
>>  	}, {
>>  		.compatible = "x-powers,axp221-battery-power-supply",
>>  		.data = (void *)&axp221_data,
>> +	}, {
>> +		.compatible = "x-powers,axp803-battery-power-supply",
>> +		.data = (void *)&axp813_data,
> Ditto.
>
>>  	}, {
>>  		.compatible = "x-powers,axp813-battery-power-supply",
>>  		.data = (void *)&axp813_data,
>> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
>> index 517e60eecbcb..23d58e243d01 100644
>> --- a/include/linux/mfd/axp20x.h
>> +++ b/include/linux/mfd/axp20x.h
>> @@ -266,6 +266,7 @@ enum axp20x_variants {
>>  #define AXP288_RT_BATT_V_H		0xa0
>>  #define AXP288_RT_BATT_V_L		0xa1
>>  
>> +#define AXP803_ACIN_PATH_CTRL		0x3a
>>  #define AXP813_ADC_RATE			0x85
>>  
> Basically, the only changes you need to keep are those in
> drivers/power/supply/axp20x_ac_power.c and include/linux/mfd/axp20x.h.
>
> Thanks,
> Quentin
I'll refactor code and commits based on these comments.

Thanks,
Oskari

[PATCH v2 0/6] AXP8x3 AC and battery power supply support

[Oskari Lemmela]

AXP813 AC power supply support with input current and
voltage limiting support.

AXP803 AC and battery power supply support.

Changes in v2:
* Reuse axp813 compatibles for axp803
* Refactor axp20x_ac_power.c

Oskari Lemmela (6):
  dt-bindings: power: supply: axp20x: add AXP813 AC power DT binding
  ARM: dts: axp81x: add AC power supply subnode
  arm64: dts: allwinner: axp803: add AC and battery power supplies
  arm64: dts: allwinner: a64: sopine: enable power supplies
  mfd: axp20x: add support AXP803 AC and battery power supplies
  power: supply: add AC power supply driver for AXP813

 .../bindings/power/supply/axp20x_ac_power.txt |  3 +
 arch/arm/boot/dts/axp81x.dtsi                 |  5 +
 arch/arm64/boot/dts/allwinner/axp803.dtsi     | 31 +++++++
 .../allwinner/sun50i-a64-sopine-baseboard.dts |  8 ++
 drivers/mfd/axp20x.c                          | 22 ++++-
 drivers/power/supply/axp20x_ac_power.c        | 92 +++++++++++++++++++
 include/linux/mfd/axp20x.h                    |  1 +
 7 files changed, 161 insertions(+), 1 deletion(-)

-- 
2.17.1

[PATCH v2 1/6] dt-bindings: power: supply: axp20x: add AXP813 AC power DT binding

[Oskari Lemmela]

The AXP803/AXP813 AC power supply can limit input current and voltage.

Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
---
 .../devicetree/bindings/power/supply/axp20x_ac_power.txt       | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt b/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
index 826e8a879121..7a1fb532abe5 100644
--- a/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
+++ b/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
@@ -4,6 +4,7 @@ Required Properties:
  - compatible: One of:
 			"x-powers,axp202-ac-power-supply"
 			"x-powers,axp221-ac-power-supply"
+			"x-powers,axp813-ac-power-supply"
 
 This node is a subnode of the axp20x PMIC.
 
@@ -13,6 +14,8 @@ reading ADC channels from the AXP20X ADC.
 The AXP22X is only able to tell if an AC power supply is present and
 usable.
 
+AXP813/AXP803 are able to limit current and supply voltage
+
 Example:
 
 &axp209 {
-- 
2.17.1

[PATCH v2 2/6] ARM: dts: axp81x: add AC power supply subnode

[Oskari Lemmela]

Add AC power supply subnode for AXP81X PMIC.

Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
---
 arch/arm/boot/dts/axp81x.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/axp81x.dtsi b/arch/arm/boot/dts/axp81x.dtsi
index 043c717dcef1..67c17151b584 100644
--- a/arch/arm/boot/dts/axp81x.dtsi
+++ b/arch/arm/boot/dts/axp81x.dtsi
@@ -69,6 +69,11 @@
 		};
 	};
 
+	ac_power_supply: ac-power-supply {
+		compatible = "x-powers,axp813-ac-power-supply";
+		status = "disabled";
+	};
+
 	battery_power_supply: battery-power-supply {
 		compatible = "x-powers,axp813-battery-power-supply";
 		status = "disabled";
-- 
2.17.1

Re: [PATCH v2 2/6] ARM: dts: axp81x: add AC power supply subnode

[Quentin Schulz]

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

Hi Oskari,

On Sun, Oct 07, 2018 at 12:18:32AM +0300, Oskari Lemmela wrote:
> Add AC power supply subnode for AXP81X PMIC.
> 
> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
> ---
>  arch/arm/boot/dts/axp81x.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/axp81x.dtsi b/arch/arm/boot/dts/axp81x.dtsi
> index 043c717dcef1..67c17151b584 100644
> --- a/arch/arm/boot/dts/axp81x.dtsi
> +++ b/arch/arm/boot/dts/axp81x.dtsi
> @@ -69,6 +69,11 @@
>  		};
>  	};
>  
> +	ac_power_supply: ac-power-supply {
> +		compatible = "x-powers,axp813-ac-power-supply";
> +		status = "disabled";
> +	};
> +

IIRC, when it's a DT node of something that has no address (register or
bus address), we order alphabetically the DT nodes within the same
parent node. So here, we would put it before axp_adc.

Thanks,
Quentin

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

[PATCH v2 3/6] arm64: dts: allwinner: axp803: add AC and battery power supplies

[Oskari Lemmela]

AXP803 is compatible with AXP813. Add DT nodes ADC, GPIO,
AC and battery power supplies.

Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
---
 arch/arm64/boot/dts/allwinner/axp803.dtsi | 31 +++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/axp803.dtsi b/arch/arm64/boot/dts/allwinner/axp803.dtsi
index e5eae8bafc42..533987b2fe7a 100644
--- a/arch/arm64/boot/dts/allwinner/axp803.dtsi
+++ b/arch/arm64/boot/dts/allwinner/axp803.dtsi
@@ -49,6 +49,37 @@
 	interrupt-controller;
 	#interrupt-cells = <1>;
 
+	axp_adc: adc {
+		compatible = "x-powers,axp813-adc";
+		#io-channel-cells = <1>;
+	};
+
+	axp_gpio: gpio {
+		compatible = "x-powers,axp813-gpio";
+		gpio-controller;
+		#gpio-cells = <2>;
+
+		gpio0_ldo: gpio0-ldo {
+			pins = "GPIO0";
+			function = "ldo";
+		};
+
+		gpio1_ldo: gpio1-ldo {
+			pins = "GPIO1";
+			function = "ldo";
+		};
+	};
+
+	ac_power_supply: ac-power-supply {
+		compatible = "x-powers,axp813-ac-power-supply";
+		status = "disabled";
+	};
+
+	battery_power_supply: battery-power-supply {
+		compatible = "x-powers,axp813-battery-power-supply";
+		status = "disabled";
+	};
+
 	regulators {
 		/* Default work frequency for buck regulators */
 		x-powers,dcdc-freq = <3000>;
-- 
2.17.1

Re: [PATCH v2 3/6] arm64: dts: allwinner: axp803: add AC and battery power supplies

[Quentin Schulz]

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

Hi Oskari,

On Sun, Oct 07, 2018 at 12:18:33AM +0300, Oskari Lemmela wrote:
> AXP803 is compatible with AXP813. Add DT nodes ADC, GPIO,
> AC and battery power supplies.
> 
> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
> ---
>  arch/arm64/boot/dts/allwinner/axp803.dtsi | 31 +++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/axp803.dtsi b/arch/arm64/boot/dts/allwinner/axp803.dtsi
> index e5eae8bafc42..533987b2fe7a 100644
> --- a/arch/arm64/boot/dts/allwinner/axp803.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/axp803.dtsi
> @@ -49,6 +49,37 @@
>  	interrupt-controller;
>  	#interrupt-cells = <1>;
>  
> +	axp_adc: adc {
> +		compatible = "x-powers,axp813-adc";
> +		#io-channel-cells = <1>;
> +	};
> +
> +	axp_gpio: gpio {
> +		compatible = "x-powers,axp813-gpio";
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +
> +		gpio0_ldo: gpio0-ldo {
> +			pins = "GPIO0";
> +			function = "ldo";
> +		};
> +
> +		gpio1_ldo: gpio1-ldo {
> +			pins = "GPIO1";
> +			function = "ldo";
> +		};
> +	};
> +
> +	ac_power_supply: ac-power-supply {
> +		compatible = "x-powers,axp813-ac-power-supply";
> +		status = "disabled";
> +	};
> +

Same as the patch before, I'd put it before axp_adc.

Thanks,
Quentin

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

[PATCH v2 4/6] arm64: dts: allwinner: a64: sopine: enable power supplies

[Oskari Lemmela]

Sopine baseboard have ACIN and battery connectors.

Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
---
 .../boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts    | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
index c21f2331add6..335cf2263d19 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
@@ -69,6 +69,14 @@
 	};
 };
 
+&ac_power_supply {
+	status = "okay";
+};
+
+&battery_power_supply {
+	status = "okay";
+};
+
 &ehci0 {
 	status = "okay";
 };
-- 
2.17.1

[PATCH v2 5/6] mfd: axp20x: add support AXP803 AC and battery power supplies

[Oskari Lemmela]

AXP803 is compatible with AXP813.
Adding needed cells for AC and battery power supplies.

AXP813 AC power supply cell added.

Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
---
 drivers/mfd/axp20x.c       | 22 +++++++++++++++++++++-
 include/linux/mfd/axp20x.h |  1 +
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 0be511dd93d0..215f0b009728 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -731,8 +731,23 @@ static const struct mfd_cell axp803_cells[] = {
 		.name			= "axp221-pek",
 		.num_resources		= ARRAY_SIZE(axp803_pek_resources),
 		.resources		= axp803_pek_resources,
+	}, {
+		.name			= "axp20x-regulator"
+	}, {
+		.name			= "axp20x-gpio",
+		.of_compatible		= "x-powers,axp813-gpio",
+	}, {
+		.name			= "axp813-adc",
+		.of_compatible		= "x-powers,axp813-adc",
+	}, {
+		.name		= "axp20x-battery-power-supply",
+		.of_compatible	= "x-powers,axp813-battery-power-supply",
+	}, {
+		.name           = "axp20x-ac-power-supply",
+		.of_compatible  = "x-powers,axp813-ac-power-supply",
+		.num_resources  = ARRAY_SIZE(axp20x_ac_power_supply_resources),
+		.resources      = axp20x_ac_power_supply_resources,
 	},
-	{	.name			= "axp20x-regulator" },
 };
 
 static const struct mfd_cell axp806_self_working_cells[] = {
@@ -778,6 +793,11 @@ static const struct mfd_cell axp813_cells[] = {
 	}, {
 		.name		= "axp20x-battery-power-supply",
 		.of_compatible	= "x-powers,axp813-battery-power-supply",
+	}, {
+		.name           = "axp20x-ac-power-supply",
+		.of_compatible  = "x-powers,axp813-ac-power-supply",
+		.num_resources  = ARRAY_SIZE(axp20x_ac_power_supply_resources),
+		.resources      = axp20x_ac_power_supply_resources,
 	},
 };
 
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 517e60eecbcb..2302b620d238 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -266,6 +266,7 @@ enum axp20x_variants {
 #define AXP288_RT_BATT_V_H		0xa0
 #define AXP288_RT_BATT_V_L		0xa1
 
+#define AXP813_ACIN_PATH_CTRL		0x3a
 #define AXP813_ADC_RATE			0x85
 
 /* Fuel Gauge */
-- 
2.17.1

Re: [PATCH v2 5/6] mfd: axp20x: add support AXP803 AC and battery power supplies

[Quentin Schulz]

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

Hi Oskari,

On Sun, Oct 07, 2018 at 12:18:35AM +0300, Oskari Lemmela wrote:
> AXP803 is compatible with AXP813.
> Adding needed cells for AC and battery power supplies.
> 
> AXP813 AC power supply cell added.
> 
> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
> ---
>  drivers/mfd/axp20x.c       | 22 +++++++++++++++++++++-
>  include/linux/mfd/axp20x.h |  1 +
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 0be511dd93d0..215f0b009728 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -731,8 +731,23 @@ static const struct mfd_cell axp803_cells[] = {
>  		.name			= "axp221-pek",
>  		.num_resources		= ARRAY_SIZE(axp803_pek_resources),
>  		.resources		= axp803_pek_resources,
> +	}, {
> +		.name			= "axp20x-regulator"
> +	}, {
> +		.name			= "axp20x-gpio",
> +		.of_compatible		= "x-powers,axp813-gpio",
> +	}, {
> +		.name			= "axp813-adc",
> +		.of_compatible		= "x-powers,axp813-adc",
> +	}, {
> +		.name		= "axp20x-battery-power-supply",
> +		.of_compatible	= "x-powers,axp813-battery-power-supply",
> +	}, {
> +		.name           = "axp20x-ac-power-supply",
> +		.of_compatible  = "x-powers,axp813-ac-power-supply",
> +		.num_resources  = ARRAY_SIZE(axp20x_ac_power_supply_resources),
> +		.resources      = axp20x_ac_power_supply_resources,
>  	},
> -	{	.name			= "axp20x-regulator" },
>  };
>  
>  static const struct mfd_cell axp806_self_working_cells[] = {
> @@ -778,6 +793,11 @@ static const struct mfd_cell axp813_cells[] = {
>  	}, {
>  		.name		= "axp20x-battery-power-supply",
>  		.of_compatible	= "x-powers,axp813-battery-power-supply",
> +	}, {
> +		.name           = "axp20x-ac-power-supply",
> +		.of_compatible  = "x-powers,axp813-ac-power-supply",
> +		.num_resources  = ARRAY_SIZE(axp20x_ac_power_supply_resources),
> +		.resources      = axp20x_ac_power_supply_resources,

I'll let Maxime or Chen-Yu tell you what they want but I think we could
have two different commits, one for AXP803 and one for AXP813. I don't
think we care about having one commit per added resource for the axp803
though.

>  	},
>  };
>  
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index 517e60eecbcb..2302b620d238 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -266,6 +266,7 @@ enum axp20x_variants {
>  #define AXP288_RT_BATT_V_H		0xa0
>  #define AXP288_RT_BATT_V_L		0xa1
>  
> +#define AXP813_ACIN_PATH_CTRL		0x3a

You're not using it in this patch, please keep this change in the commit
where its use is introduced.

Thanks,
Quentin

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

[PATCH v2 6/6] power: supply: add AC power supply driver for AXP813

[Oskari Lemmela]

AXP813 and AXP803 PMICs can control input current and
minimum voltage.

Both of these values are configurable.

Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
---
 drivers/power/supply/axp20x_ac_power.c | 92 ++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/drivers/power/supply/axp20x_ac_power.c b/drivers/power/supply/axp20x_ac_power.c
index 0771f951b11f..92a92354f6f0 100644
--- a/drivers/power/supply/axp20x_ac_power.c
+++ b/drivers/power/supply/axp20x_ac_power.c
@@ -27,6 +27,16 @@
 #define AXP20X_PWR_STATUS_ACIN_PRESENT	BIT(7)
 #define AXP20X_PWR_STATUS_ACIN_AVAIL	BIT(6)
 
+#define AXP813_VHOLD_MASK		GENMASK(5, 3)
+#define AXP813_VHOLD_UV_TO_BIT(x)	((((x) / 100000) - 40) << 3)
+#define AXP813_VHOLD_REG_TO_UV(x)	\
+	(((((x) & AXP813_VHOLD_MASK) >> 3) + 40) * 100000)
+
+#define AXP813_CURR_LIMIT_MASK		GENMASK(2, 0)
+#define AXP813_CURR_LIMIT_UA_TO_BIT(x)	(((x) / 500000) - 3)
+#define AXP813_CURR_LIMIT_REG_TO_UA(x)	\
+	((((x) & AXP813_CURR_LIMIT_MASK) + 3) * 500000)
+
 #define DRVNAME "axp20x-ac-power-supply"
 
 struct axp20x_ac_power {
@@ -102,6 +112,55 @@ static int axp20x_ac_power_get_property(struct power_supply *psy,
 
 		return 0;
 
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
+		ret = regmap_read(power->regmap, AXP813_ACIN_PATH_CTRL, &reg);
+		if (ret)
+			return ret;
+
+		val->intval = AXP813_VHOLD_REG_TO_UV(reg);
+
+		return 0;
+
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+		ret = regmap_read(power->regmap, AXP813_ACIN_PATH_CTRL, &reg);
+		if (ret)
+			return ret;
+
+		val->intval = AXP813_CURR_LIMIT_REG_TO_UA(reg);
+
+		return 0;
+
+	default:
+		return -EINVAL;
+	}
+
+	return -EINVAL;
+}
+
+static int axp20x_ac_power_set_property(struct power_supply *psy,
+					enum power_supply_property psp,
+					const union power_supply_propval *val)
+{
+	struct axp20x_ac_power *power = power_supply_get_drvdata(psy);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
+		if (val->intval < 4000000 || val->intval > 4700000)
+			return -EINVAL;
+
+		return regmap_update_bits(power->regmap, AXP813_ACIN_PATH_CTRL,
+					  AXP813_VHOLD_MASK,
+					  AXP813_VHOLD_UV_TO_BIT(val->intval));
+
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+		if (val->intval < 1500000 || val->intval > 4000000)
+			return -EINVAL;
+
+		return regmap_update_bits(power->regmap, AXP813_ACIN_PATH_CTRL,
+					  AXP813_CURR_LIMIT_MASK,
+					  AXP813_CURR_LIMIT_UA_TO_BIT
+					  (val->intval));
+
 	default:
 		return -EINVAL;
 	}
@@ -109,6 +168,13 @@ static int axp20x_ac_power_get_property(struct power_supply *psy,
 	return -EINVAL;
 }
 
+static int axp813_ac_power_prop_writeable(struct power_supply *psy,
+					  enum power_supply_property psp)
+{
+	return psp == POWER_SUPPLY_PROP_VOLTAGE_MIN ||
+	       psp == POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT;
+}
+
 static enum power_supply_property axp20x_ac_power_properties[] = {
 	POWER_SUPPLY_PROP_HEALTH,
 	POWER_SUPPLY_PROP_PRESENT,
@@ -123,6 +189,14 @@ static enum power_supply_property axp22x_ac_power_properties[] = {
 	POWER_SUPPLY_PROP_ONLINE,
 };
 
+static enum power_supply_property axp813_ac_power_properties[] = {
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN,
+	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+};
+
 static const struct power_supply_desc axp20x_ac_power_desc = {
 	.name = "axp20x-ac",
 	.type = POWER_SUPPLY_TYPE_MAINS,
@@ -139,6 +213,16 @@ static const struct power_supply_desc axp22x_ac_power_desc = {
 	.get_property = axp20x_ac_power_get_property,
 };
 
+static const struct power_supply_desc axp813_ac_power_desc = {
+	.name = "axp813-ac",
+	.type = POWER_SUPPLY_TYPE_MAINS,
+	.properties = axp813_ac_power_properties,
+	.num_properties = ARRAY_SIZE(axp813_ac_power_properties),
+	.property_is_writeable = axp813_ac_power_prop_writeable,
+	.get_property = axp20x_ac_power_get_property,
+	.set_property = axp20x_ac_power_set_property,
+};
+
 struct axp_data {
 	const struct power_supply_desc	*power_desc;
 	bool				acin_adc;
@@ -154,6 +238,11 @@ static const struct axp_data axp22x_data = {
 	.acin_adc = false,
 };
 
+static const struct axp_data axp813_data = {
+	.power_desc = &axp813_ac_power_desc,
+	.acin_adc = false,
+};
+
 static int axp20x_ac_power_probe(struct platform_device *pdev)
 {
 	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
@@ -234,6 +323,9 @@ static const struct of_device_id axp20x_ac_power_match[] = {
 	}, {
 		.compatible = "x-powers,axp221-ac-power-supply",
 		.data = &axp22x_data,
+	}, {
+		.compatible = "x-powers,axp813-ac-power-supply",
+		.data = &axp813_data,
 	}, { /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, axp20x_ac_power_match);
-- 
2.17.1

Re: [PATCH v2 6/6] power: supply: add AC power supply driver for AXP813

[Quentin Schulz]

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

Hi Oskari,

On Sun, Oct 07, 2018 at 12:18:36AM +0300, Oskari Lemmela wrote:
> AXP813 and AXP803 PMICs can control input current and
> minimum voltage.
> 
> Both of these values are configurable.
> 
> Signed-off-by: Oskari Lemmela <oskari@lemmela.net>
> ---
>  drivers/power/supply/axp20x_ac_power.c | 92 ++++++++++++++++++++++++++
>  1 file changed, 92 insertions(+)
> 
> diff --git a/drivers/power/supply/axp20x_ac_power.c b/drivers/power/supply/axp20x_ac_power.c
> index 0771f951b11f..92a92354f6f0 100644
> --- a/drivers/power/supply/axp20x_ac_power.c
> +++ b/drivers/power/supply/axp20x_ac_power.c
> @@ -27,6 +27,16 @@
>  #define AXP20X_PWR_STATUS_ACIN_PRESENT	BIT(7)
>  #define AXP20X_PWR_STATUS_ACIN_AVAIL	BIT(6)
>  
> +#define AXP813_VHOLD_MASK		GENMASK(5, 3)
> +#define AXP813_VHOLD_UV_TO_BIT(x)	((((x) / 100000) - 40) << 3)
> +#define AXP813_VHOLD_REG_TO_UV(x)	\
> +	(((((x) & AXP813_VHOLD_MASK) >> 3) + 40) * 100000)
> +
> +#define AXP813_CURR_LIMIT_MASK		GENMASK(2, 0)
> +#define AXP813_CURR_LIMIT_UA_TO_BIT(x)	(((x) / 500000) - 3)
> +#define AXP813_CURR_LIMIT_REG_TO_UA(x)	\
> +	((((x) & AXP813_CURR_LIMIT_MASK) + 3) * 500000)
> +
>  #define DRVNAME "axp20x-ac-power-supply"
>  
>  struct axp20x_ac_power {
> @@ -102,6 +112,55 @@ static int axp20x_ac_power_get_property(struct power_supply *psy,
>  
>  		return 0;
>  
> +	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
> +		ret = regmap_read(power->regmap, AXP813_ACIN_PATH_CTRL, &reg);
> +		if (ret)
> +			return ret;
> +
> +		val->intval = AXP813_VHOLD_REG_TO_UV(reg);
> +
> +		return 0;
> +
> +	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> +		ret = regmap_read(power->regmap, AXP813_ACIN_PATH_CTRL, &reg);
> +		if (ret)
> +			return ret;
> +
> +		val->intval = AXP813_CURR_LIMIT_REG_TO_UA(reg);
> +
> +		return 0;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int axp20x_ac_power_set_property(struct power_supply *psy,
> +					enum power_supply_property psp,
> +					const union power_supply_propval *val)
> +{

Argh, missed this one in the first version. Since you're introducing it
with the AXP813 and it isn't used with the AXP20X, I'd name it
axp813_ac_power_set_property. I'll let Maxime or Chen-Yu confirm though.

With the modification in the header from the previous patch in this
patch,

Reviewed-by: Quentin Schulz <quentin.schulz@bootlin.com>

Thanks!
Quentin

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

Re: [PATCH v2 0/6] AXP8x3 AC and battery power supply support

[Quentin Schulz]

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

Hi Oskari,

On Sun, Oct 07, 2018 at 12:18:30AM +0300, Oskari Lemmela wrote:
> AXP813 AC power supply support with input current and
> voltage limiting support.
> 
> AXP803 AC and battery power supply support.
> 

Great to see a new developer taking interest in those PMICs :)

I received the v2 patch series as answers of the v1 patch series, you
would want to fix that by sending a separate patch series. Using the
name of the cover letter of the patch series and the author, it's
usually good enough to find previous versions if we need to, you don't
need to send the new patch series as answers of the first patch series.

Thanks,
Quentin

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