linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers
@ 2017-01-27  8:54 Quentin Schulz
  2017-01-27  8:54 ` [PATCH v2 01/25] dt-bindings: iio: adc: add AXP20X/AXP22X ADC DT binding Quentin Schulz
                   ` (25 more replies)
  0 siblings, 26 replies; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The X-Powers AXP20X and AXP22X PMICs have multiple ADCs. They expose
information and data of the various power supplies they support such as
ACIN, battery and VBUS. For example, they expose the current battery
voltage, charge or discharge, as well as ACIN and VBUS current voltages
and currents, internal PMIC temperature and ADC on 2 different GPIOs
when in the right mode (for the AXP209 only).

The ACIN power supply driver is added by this patch. The AXP20X and
AXP22X can both read the status and the "usability" of the power supply
but only the AXP209 will be able to tell the current current and voltage
of the power supply by reading ADC channels. It is simply not supported
by the AXP22X PMICs.

The battery power supply driver is also added by this patch. The AXP20X
and AXP22X share most of their behaviour but have slight variations. The
allowed target voltages for battery charging are not the same, the
AXP22X PMIC are able to tell if the battery percentage computed by the
PMIC is trustworthy and they have different formulas for computing max
current for battery power supply. The driver is able to give the current
voltage and current of the battery (be it charging or discharging), the
maximal and minimal voltage and maximal current allowed for the battery,
whether the battery is present and usable and its capacity. It will get
the battery current current and voltage by reading the ADC channels. The
PMIC allows maximal voltages (4.36V for AXP20X and 4.22V and 4.24V for
AXP22X) that should not be used with Lithium-based batteries and since
this PMIC is supposed to be used with Lithium-based batteries, they have
been disabled. The values returned by the ADC driver are multipled by
1000 to scale from the mV returned by the ADC to the uV expected by the
power supply framework.

This series of patch adds DT bindings for ACIN power supply, ADC and
battery power supply drivers for AXP20X and AXP22X PMICs and their
documentation. It also enables the supported power supplies for the
Nextthing Co. CHIP and Sinlinx SinA33 boards.

The different drivers are also added to the MFD cells of the AXP20X and
AXP22X cells and the writeable and volatile regs updated to work with
the newly added drivers.

VBUS driver has intentionally not been modified to use the ADC channels
because a DT binding already exists for this driver. Migrating the
driver would mean to add an iio_map to map the ADC channels to the VBUS
driver (so we can use iio_channel_get and iio_read_channel_processed
functions). This slightly complexifies the VBUS driver only for
"cosmetic" changes. Feel free to give your two cents on the matter.

This series of patch is based on a previous upstreaming attempt done by
Bruno Prémont few months ago. It differs in three points: the ADC
driver does not tell the battery temperature (TS_IN) as I do not have a
board to test it with, it does not tell the instantaneous battery power
as it returns crazy values for me and finally no support for OCV curves
for the battery.

You can test these patches from this repo and branch:
https://github.com/QSchulz/linux/tree/axp2xx_adc_batt_ac_v2

v2:
 - Some registers' name have been changed to better reflect their
 purpose,
 - Make VBUS power supply driver use IIO channels when AXP ADC driver is
 enabled, but fall back on previous behavior when disabled. This is made
 to avoid the ADC driver overwritting registers for VBUS power supply
 ADC when removed,
 - Removed useless adding of data registers to volatile registers,
 - Reordered IIO channels, now grouped by same part of the PMIC (e.g.
 voltage and current of the battery have the same index in different
 IIO types),
 - Added structures for specific data instead of matching on IDs,
 - Switched from DT IIO channels mapping to iio_map structures IIO
 channels mapping,

Quentin

Quentin Schulz (25):
  dt-bindings: iio: adc: add AXP20X/AXP22X ADC DT binding
  mfd: axp20x: correct name of temperature data ADC registers
  power: supply: axp20x_usb_power: use IIO channels when available
  iio: adc: add support for X-Powers AXP20X and AXP22X PMICs ADCs
  mfd: axp20x: add ADC cells for AXP20X and AXP22X PMICs
  ARM: dtsi: axp209: add AXP209 ADC subnode
  ARM: dtsi: axp22x: add AXP22X ADC subnode
  dt-bindings: power: supply: add AXP20X/AXP22X AC power supply
  iio: adc: axp20x_adc: map acin_i and acin_v
  power: supply: add AC power supply driver for AXP20X and AXP22X PMICs
  mfd: axp20x: add AC power supply cells for AXP22X PMICs
  ARM: dtsi: axp209: add AC power supply subnode
  ARM: dtsi: axp22x: add AC power supply subnode
  ARM: dts: sun8i: sina33: enable ACIN power supply subnode
  ARM: sun5i: chip: enable ACIN power supply subnode
  dt-bindings: power: supply: add AXP20X/AXP22X battery DT binding
  mfd: axp20x: add CHRG_CTRL1/2/3 to writeable regs for AXP20X/AXP22X
  mfd: axp20x: add V_OFF to writeable regs for AXP20X and AXP22X
  iio: adc: axp20x_adc: map battery IIO channels
  power: supply: add battery driver for AXP20X and AXP22X PMICs
  mfd: axp20x: add MFD cells for AXP20X and AXP22X battery driver
  ARM: dtsi: axp209: add battery power supply subnode
  ARM: dtsi: axp22x: add battery power supply subnode
  ARM: dts: sun8i: sina33: enable battery power supply subnode
  ARM: sun5i: chip: enable battery power supply subnode

 .../devicetree/bindings/iio/adc/axp20x_adc.txt     |  22 +
 .../bindings/power/supply/axp20x_ac_power.txt      |  22 +
 .../bindings/power/supply/axp20x_battery.txt       |  25 +
 arch/arm/boot/dts/axp209.dtsi                      |  14 +
 arch/arm/boot/dts/axp22x.dtsi                      |  14 +
 arch/arm/boot/dts/sun5i-r8-chip.dts                |   8 +
 arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts     |   8 +
 drivers/iio/adc/Kconfig                            |  10 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/axp20x_adc.c                       | 609 +++++++++++++++++++++
 drivers/mfd/axp20x.c                               |  36 +-
 drivers/power/supply/Kconfig                       |  24 +
 drivers/power/supply/Makefile                      |   2 +
 drivers/power/supply/axp20x_ac_power.c             | 250 +++++++++
 drivers/power/supply/axp20x_battery.c              | 477 ++++++++++++++++
 drivers/power/supply/axp20x_usb_power.c            |  70 ++-
 include/linux/mfd/axp20x.h                         |   4 +-
 17 files changed, 1587 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt
 create mode 100644 Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
 create mode 100644 Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
 create mode 100644 drivers/iio/adc/axp20x_adc.c
 create mode 100644 drivers/power/supply/axp20x_ac_power.c
 create mode 100644 drivers/power/supply/axp20x_battery.c

-- 
2.9.3

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

* [PATCH v2 01/25] dt-bindings: iio: adc: add AXP20X/AXP22X ADC DT binding
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-27  8:54 ` [PATCH v2 02/25] mfd: axp20x: correct name of temperature data ADC registers Quentin Schulz
                   ` (24 subsequent siblings)
  25 siblings, 0 replies; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The X-Powers AXP20X and AXP22X PMICs have multiple ADCs. They expose the
battery voltage, battery charge and discharge currents, AC-in and VBUS
voltages and currents, 2 GPIOs muxable in ADC mode and PMIC temperature.

This adds the device tree binding documentation for the X-Powers AXP20X
and AXP22X PMICs ADCs.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Chen-Yu Tsai <wens@csie.org>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---

v2:
 - removing io-channels from required properties,
 - update AXP ADC DT node name from axp209_adc to simply adc,

 .../devicetree/bindings/iio/adc/axp20x_adc.txt     | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt

diff --git a/Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt b/Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt
new file mode 100644
index 0000000..a071247
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt
@@ -0,0 +1,22 @@
+X-Powers AXP20X and AXP22X PMIC Analog to Digital Converter (ADC)
+
+The X-Powers AXP20X and AXP22X PMICs have multiple ADCs. They expose the
+battery voltage, battery charge and discharge currents, AC-in and VBUS
+voltages and currents, 2 GPIOs muxable in ADC mode and PMIC temperature.
+
+The AXP22X PMICs do not have all ADCs of the AXP20X though.
+
+Required properties:
+ - compatible, one of:
+			"x-powers,axp209-adc"
+			"x-powers,axp221-adc"
+
+This is a subnode of the AXP20X PMIC.
+
+Example:
+
+&axp209 {
+	axp209_adc: adc {
+		compatible = "x-powers,axp209-adc";
+	};
+};
-- 
2.9.3

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

* [PATCH v2 02/25] mfd: axp20x: correct name of temperature data ADC registers
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
  2017-01-27  8:54 ` [PATCH v2 01/25] dt-bindings: iio: adc: add AXP20X/AXP22X ADC DT binding Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-27  9:30   ` Chen-Yu Tsai
  2017-02-08  9:26   ` Lee Jones
  2017-01-27  8:54 ` [PATCH v2 03/25] power: supply: axp20x_usb_power: use IIO channels when available Quentin Schulz
                   ` (23 subsequent siblings)
  25 siblings, 2 replies; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The registers 0x56 and 0x57 of AXP22X PMIC store the value of the
internal temperature of the PMIC.

This patch modifies the name of these registers from AXP22X_PMIC_ADC_H/L
to AXP22X_PMIC_TEMP_H/L so their purpose is clearer.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---

added in v2

 drivers/mfd/axp20x.c       | 2 +-
 include/linux/mfd/axp20x.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 619a83e..9c2fd37 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -102,7 +102,7 @@ static const struct regmap_range axp22x_volatile_ranges[] = {
 	regmap_reg_range(AXP20X_VBUS_IPSOUT_MGMT, AXP20X_VBUS_IPSOUT_MGMT),
 	regmap_reg_range(AXP20X_IRQ1_EN, AXP20X_IRQ5_STATE),
 	regmap_reg_range(AXP22X_GPIO_STATE, AXP22X_GPIO_STATE),
-	regmap_reg_range(AXP22X_PMIC_ADC_H, AXP20X_IPSOUT_V_HIGH_L),
+	regmap_reg_range(AXP22X_PMIC_TEMP_H, AXP20X_IPSOUT_V_HIGH_L),
 	regmap_reg_range(AXP20X_FG_RES, AXP20X_FG_RES),
 };
 
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index a4860bc..5ecadbb 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -228,8 +228,8 @@ enum {
 #define AXP20X_OCV_MAX			0xf
 
 /* AXP22X specific registers */
-#define AXP22X_PMIC_ADC_H		0x56
-#define AXP22X_PMIC_ADC_L		0x57
+#define AXP22X_PMIC_TEMP_H		0x56
+#define AXP22X_PMIC_TEMP_L		0x57
 #define AXP22X_TS_ADC_H			0x58
 #define AXP22X_TS_ADC_L			0x59
 #define AXP22X_BATLOW_THRES1		0xe6
-- 
2.9.3

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

* [PATCH v2 03/25] power: supply: axp20x_usb_power: use IIO channels when available
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
  2017-01-27  8:54 ` [PATCH v2 01/25] dt-bindings: iio: adc: add AXP20X/AXP22X ADC DT binding Quentin Schulz
  2017-01-27  8:54 ` [PATCH v2 02/25] mfd: axp20x: correct name of temperature data ADC registers Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-28 14:36   ` Jonathan Cameron
  2017-01-29 16:21   ` Sebastian Reichel
  2017-01-27  8:54 ` [PATCH v2 04/25] iio: adc: add support for X-Powers AXP20X and AXP22X PMICs ADCs Quentin Schulz
                   ` (22 subsequent siblings)
  25 siblings, 2 replies; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The X-Powers AXP20X PMIC exposes the current current and voltage
measures via an internal ADC.

This adds the possibility to read IIO channels directly for processed
values rather than reading the registers and computing the value.

For backward compatibility purpose, if the IIO driver is not compiled,
this driver will fall back on previous behaviour which is direct
register readings.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---

added in v2

 drivers/power/supply/axp20x_usb_power.c | 70 +++++++++++++++++++++++++++++++--
 1 file changed, 66 insertions(+), 4 deletions(-)

diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c
index 1bcb025..d8b1dc6 100644
--- a/drivers/power/supply/axp20x_usb_power.c
+++ b/drivers/power/supply/axp20x_usb_power.c
@@ -22,6 +22,7 @@
 #include <linux/power_supply.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
+#include <linux/iio/consumer.h>
 
 #define DRVNAME "axp20x-usb-power-supply"
 
@@ -49,6 +50,8 @@ struct axp20x_usb_power {
 	struct regmap *regmap;
 	struct power_supply *supply;
 	int axp20x_id;
+	struct iio_channel *vbus_v;
+	struct iio_channel *vbus_i;
 };
 
 static irqreturn_t axp20x_usb_power_irq(int irq, void *devid)
@@ -76,6 +79,20 @@ static int axp20x_usb_power_get_property(struct power_supply *psy,
 		val->intval = AXP20X_VBUS_VHOLD_uV(v);
 		return 0;
 	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		if (IS_ENABLED(CONFIG_AXP20X_ADC)) {
+			ret = iio_read_channel_processed(power->vbus_v,
+							 &val->intval);
+			if (ret)
+				return ret;
+
+			/*
+			 * IIO framework gives mV but Power Supply framework
+			 * gives uV.
+			 */
+			val->intval *= 1000;
+			return 0;
+		}
+
 		ret = axp20x_read_variable_width(power->regmap,
 						 AXP20X_VBUS_V_ADC_H, 12);
 		if (ret < 0)
@@ -107,6 +124,20 @@ static int axp20x_usb_power_get_property(struct power_supply *psy,
 		}
 		return 0;
 	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		if (IS_ENABLED(CONFIG_AXP20X_ADC)) {
+			ret = iio_read_channel_processed(power->vbus_i,
+							 &val->intval);
+			if (ret)
+				return ret;
+
+			/*
+			 * IIO framework gives mA but Power Supply framework
+			 * gives uA.
+			 */
+			val->intval *= 1000;
+			return 0;
+		}
+
 		ret = axp20x_read_variable_width(power->regmap,
 						 AXP20X_VBUS_I_ADC_H, 12);
 		if (ret < 0)
@@ -269,6 +300,36 @@ static const struct power_supply_desc axp22x_usb_power_desc = {
 	.set_property = axp20x_usb_power_set_property,
 };
 
+static int configure_iio_channels(struct platform_device *pdev,
+				  struct axp20x_usb_power *power)
+{
+	power->vbus_v = devm_iio_channel_get(&pdev->dev, "vbus_v");
+	if (IS_ERR(power->vbus_v)) {
+		if (PTR_ERR(power->vbus_v) == -ENODEV)
+			return -EPROBE_DEFER;
+		return PTR_ERR(power->vbus_v);
+	}
+
+	power->vbus_i = devm_iio_channel_get(&pdev->dev, "vbus_i");
+	if (IS_ERR(power->vbus_i)) {
+		if (PTR_ERR(power->vbus_i) == -ENODEV)
+			return -EPROBE_DEFER;
+		return PTR_ERR(power->vbus_i);
+	}
+
+	return 0;
+}
+
+static int configure_adc_registers(struct axp20x_usb_power *power)
+{
+	/* Enable vbus voltage and current measurement */
+	return regmap_update_bits(power->regmap, AXP20X_ADC_EN1,
+				  AXP20X_ADC_EN1_VBUS_CURR |
+				  AXP20X_ADC_EN1_VBUS_VOLT,
+				  AXP20X_ADC_EN1_VBUS_CURR |
+				  AXP20X_ADC_EN1_VBUS_VOLT);
+}
+
 static int axp20x_usb_power_probe(struct platform_device *pdev)
 {
 	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
@@ -307,10 +368,11 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
 		if (ret)
 			return ret;
 
-		/* Enable vbus voltage and current measurement */
-		ret = regmap_update_bits(power->regmap, AXP20X_ADC_EN1,
-			AXP20X_ADC_EN1_VBUS_CURR | AXP20X_ADC_EN1_VBUS_VOLT,
-			AXP20X_ADC_EN1_VBUS_CURR | AXP20X_ADC_EN1_VBUS_VOLT);
+		if (IS_ENABLED(CONFIG_AXP20X_ADC))
+			ret = configure_iio_channels(pdev, power);
+		else
+			ret = configure_adc_registers(power);
+
 		if (ret)
 			return ret;
 
-- 
2.9.3

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

* [PATCH v2 04/25] iio: adc: add support for X-Powers AXP20X and AXP22X PMICs ADCs
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (2 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 03/25] power: supply: axp20x_usb_power: use IIO channels when available Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-28 14:49   ` Jonathan Cameron
  2017-01-27  8:54 ` [PATCH v2 05/25] mfd: axp20x: add ADC cells for AXP20X and AXP22X PMICs Quentin Schulz
                   ` (21 subsequent siblings)
  25 siblings, 1 reply; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The X-Powers AXP20X and AXP22X PMICs have multiple ADCs. They expose the
battery voltage, battery charge and discharge currents, AC-in and VBUS
voltages and currents, 2 GPIOs muxable in ADC mode and PMIC temperature.

This adds support for most of AXP20X and AXP22X ADCs.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---

v2:
 - removed unused defines,
 - changed BIT(x) to 1 << x when describing bits purpose for which 2 <<
 x or 3 << x exists, to be consistent,
 - changed ADC rate defines to macro formulas,
 - reordered IIO channels, now different measures (current/voltage) of
 the same part of the PMIC (e.g. battery), have the same IIO channel in
 their respective IIO type. When a part of the PMIC have only one
 measure, a number is jumped,
 - left IIO channel mapping in DT to use iio_map structure,
 - removed indexing of ADC internal temperature,
 - removed unused iio_dev structure in axp20x_adc_iio,
 - added a structure for data specific to AXP20X or AXP22X PMICs instead
 of using an ID and an if condition when needing to separate the
 behaviour of both,
 - added a comment on batt_chrg_i really being on 12bits rather than
 what the Chinese datasheets say (13 bits),
 - corrected the offset for AXP22X PMIC temperature,
 - set the ADC rate to a value (100Hz) shared by the AXP20X and AXP22X,
 - created macro formulas to compute the ADC rate for each,
 - added a condition on presence of ADC_EN2 reg before setting/resetting
 it,
 - switched from devm_iio_device_unregister to the non-devm function
 because of the need for a remove function,
 - removed some dead code,

 drivers/iio/adc/Kconfig      |  10 +
 drivers/iio/adc/Makefile     |   1 +
 drivers/iio/adc/axp20x_adc.c | 572 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 583 insertions(+)
 create mode 100644 drivers/iio/adc/axp20x_adc.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 9c8b558..ed17fe1 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -154,6 +154,16 @@ config AT91_SAMA5D2_ADC
 	  To compile this driver as a module, choose M here: the module will be
 	  called at91-sama5d2_adc.
 
+config AXP20X_ADC
+	tristate "X-Powers AXP20X and AXP22X ADC driver"
+	depends on MFD_AXP20X
+	help
+	  Say yes here to have support for X-Powers power management IC (PMIC)
+	  AXP20X and AXP22X ADC devices.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called axp20x_adc.
+
 config AXP288_ADC
 	tristate "X-Powers AXP288 ADC driver"
 	depends on MFD_AXP20X
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index d36c4be..f5c28a5 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_AD7887) += ad7887.o
 obj-$(CONFIG_AD799X) += ad799x.o
 obj-$(CONFIG_AT91_ADC) += at91_adc.o
 obj-$(CONFIG_AT91_SAMA5D2_ADC) += at91-sama5d2_adc.o
+obj-$(CONFIG_AXP20X_ADC) += axp20x_adc.o
 obj-$(CONFIG_AXP288_ADC) += axp288_adc.o
 obj-$(CONFIG_BCM_IPROC_ADC) += bcm_iproc_adc.o
 obj-$(CONFIG_BERLIN2_ADC) += berlin2-adc.o
diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
new file mode 100644
index 0000000..bacde92
--- /dev/null
+++ b/drivers/iio/adc/axp20x_adc.c
@@ -0,0 +1,572 @@
+/* ADC driver for AXP20X and AXP22X PMICs
+ *
+ * Copyright (c) 2016 Free Electrons NextThing Co.
+ *	Quentin Schulz <quentin.schulz@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License version 2 as published by the
+ * Free Software Foundation.
+ */
+
+#include <linux/completion.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/thermal.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/driver.h>
+#include <linux/iio/machine.h>
+#include <linux/mfd/axp20x.h>
+
+#define AXP20X_ADC_EN1_MASK			GENMASK(7, 0)
+
+#define AXP20X_ADC_EN2_MASK			(GENMASK(3, 2) | BIT(7))
+#define AXP22X_ADC_EN1_MASK			(GENMASK(7, 5) | BIT(0))
+
+#define AXP20X_GPIO10_IN_RANGE_GPIO0		BIT(0)
+#define AXP20X_GPIO10_IN_RANGE_GPIO1		BIT(1)
+#define AXP20X_GPIO10_IN_RANGE_GPIO0_VAL(x)	((x) & BIT(0))
+#define AXP20X_GPIO10_IN_RANGE_GPIO1_VAL(x)	(((x) & BIT(0)) << 1)
+
+#define AXP20X_ADC_RATE_MASK			GENMASK(7, 6)
+#define AXP20X_ADC_RATE_HZ(x)			((ilog2((x) / 25) << 6) & AXP20X_ADC_RATE_MASK)
+#define AXP22X_ADC_RATE_HZ(x)			((ilog2((x) / 100) << 6) & AXP20X_ADC_RATE_MASK)
+
+#define AXP20X_ADC_CHANNEL(_channel, _name, _type, _reg)	\
+	{							\
+		.type = _type,					\
+		.indexed = 1,					\
+		.channel = _channel,				\
+		.address = _reg,				\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
+				      BIT(IIO_CHAN_INFO_SCALE),	\
+		.datasheet_name = _name,			\
+	}
+
+#define AXP20X_ADC_CHANNEL_OFFSET(_channel, _name, _type, _reg) \
+	{							\
+		.type = _type,					\
+		.indexed = 1,					\
+		.channel = _channel,				\
+		.address = _reg,				\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
+				      BIT(IIO_CHAN_INFO_SCALE) |\
+				      BIT(IIO_CHAN_INFO_OFFSET),\
+		.datasheet_name = _name,			\
+	}
+
+struct axp_data;
+
+struct axp20x_adc_iio {
+	struct regmap		*regmap;
+	struct axp_data		*data;
+};
+
+enum axp20x_adc_channel_v {
+	AXP20X_ACIN_V = 0,
+	AXP20X_VBUS_V,
+	AXP20X_TS_IN,
+	AXP20X_GPIO0_V,
+	AXP20X_GPIO1_V,
+	AXP20X_IPSOUT_V,
+	AXP20X_BATT_V,
+};
+
+enum axp20x_adc_channel_i {
+	AXP20X_ACIN_I = 0,
+	AXP20X_VBUS_I,
+	AXP20X_BATT_CHRG_I,
+	AXP20X_BATT_DISCHRG_I,
+};
+
+enum axp22x_adc_channel_v {
+	AXP22X_TS_IN = 0,
+	AXP22X_BATT_V,
+};
+
+enum axp22x_adc_channel_i {
+	AXP22X_BATT_CHRG_I = 1,
+	AXP22X_BATT_DISCHRG_I,
+};
+
+static struct iio_map axp20x_maps[] = {
+	{
+		.consumer_dev_name = "axp20x-usb-power-supply",
+		.consumer_channel = "vbus_v",
+		.adc_channel_label = "vbus_v",
+	}, {
+		.consumer_dev_name = "axp20x-usb-power-supply",
+		.consumer_channel = "vbus_i",
+		.adc_channel_label = "vbus_i",
+	}, { /* sentinel */ }
+};
+
+/*
+ * 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
+ * charge current in_current6_raw and discharge current will be in_current7_raw.
+ */
+static const struct iio_chan_spec axp20x_adc_channels[] = {
+	AXP20X_ADC_CHANNEL(AXP20X_ACIN_V, "acin_v", IIO_VOLTAGE,
+			   AXP20X_ACIN_V_ADC_H),
+	AXP20X_ADC_CHANNEL(AXP20X_ACIN_I, "acin_i", IIO_CURRENT,
+			   AXP20X_ACIN_I_ADC_H),
+	AXP20X_ADC_CHANNEL(AXP20X_VBUS_V, "vbus_v", IIO_VOLTAGE,
+			   AXP20X_VBUS_V_ADC_H),
+	AXP20X_ADC_CHANNEL(AXP20X_VBUS_I, "vbus_i", IIO_CURRENT,
+			   AXP20X_VBUS_I_ADC_H),
+	{
+		.type = IIO_TEMP,
+		.address = AXP20X_TEMP_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_OFFSET(AXP20X_GPIO0_V, "gpio0_v", IIO_VOLTAGE,
+				  AXP20X_GPIO0_V_ADC_H),
+	AXP20X_ADC_CHANNEL_OFFSET(AXP20X_GPIO1_V, "gpio1_v", IIO_VOLTAGE,
+				  AXP20X_GPIO1_V_ADC_H),
+	AXP20X_ADC_CHANNEL(AXP20X_IPSOUT_V, "ipsout_v", IIO_VOLTAGE,
+			   AXP20X_IPSOUT_V_HIGH_H),
+	AXP20X_ADC_CHANNEL(AXP20X_BATT_V, "batt_v", IIO_VOLTAGE,
+			   AXP20X_BATT_V_H),
+	AXP20X_ADC_CHANNEL(AXP20X_BATT_CHRG_I, "batt_chrg_i", IIO_CURRENT,
+			   AXP20X_BATT_CHRG_I_H),
+	AXP20X_ADC_CHANNEL(AXP20X_BATT_DISCHRG_I, "batt_dischrg_i", IIO_CURRENT,
+			   AXP20X_BATT_DISCHRG_I_H),
+};
+
+static const struct iio_chan_spec axp22x_adc_channels[] = {
+	{
+		.type = IIO_TEMP,
+		.address = AXP22X_PMIC_TEMP_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(AXP22X_BATT_V, "batt_v", IIO_VOLTAGE,
+			   AXP20X_BATT_V_H),
+	AXP20X_ADC_CHANNEL(AXP22X_BATT_CHRG_I, "batt_chrg_i", IIO_CURRENT,
+			   AXP20X_BATT_CHRG_I_H),
+	AXP20X_ADC_CHANNEL(AXP22X_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)
+{
+	struct axp20x_adc_iio *info = iio_priv(indio_dev);
+	int size = 12;
+
+	switch (chan->type) {
+	case IIO_CURRENT:
+		/*
+		 * Unlike the Chinese datasheets tell, the charging current is
+		 * stored on 12 bits, not 13 bits.
+		 */
+		if (chan->channel == AXP20X_BATT_DISCHRG_I)
+			size = 13;
+	case IIO_VOLTAGE:
+	case IIO_TEMP:
+		*val = axp20x_read_variable_width(info->regmap, chan->address,
+						  size);
+		if (*val < 0)
+			return *val;
+
+		return IIO_VAL_INT;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int axp22x_adc_raw(struct iio_dev *indio_dev,
+			  struct iio_chan_spec const *chan, int *val)
+{
+	struct axp20x_adc_iio *info = iio_priv(indio_dev);
+	int size = 12;
+
+	switch (chan->type) {
+	case IIO_CURRENT:
+		/*
+		 * Unlike the Chinese datasheets tell, the charging current is
+		 * stored on 12 bits, not 13 bits.
+		 */
+		if (chan->channel == AXP22X_BATT_DISCHRG_I)
+			size = 13;
+	case IIO_VOLTAGE:
+	case IIO_TEMP:
+		*val = axp20x_read_variable_width(info->regmap, chan->address,
+						  size);
+		if (*val < 0)
+			return *val;
+
+		return IIO_VAL_INT;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int axp20x_adc_scale_voltage(int channel, int *val, int *val2)
+{
+	switch (channel) {
+	case AXP20X_ACIN_V:
+	case AXP20X_VBUS_V:
+		*val = 1;
+		*val2 = 700000;
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case AXP20X_GPIO0_V:
+	case AXP20X_GPIO1_V:
+		*val = 0;
+		*val2 = 500000;
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case AXP20X_BATT_V:
+		*val = 1;
+		*val2 = 100000;
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case AXP20X_IPSOUT_V:
+		*val = 1;
+		*val2 = 400000;
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int axp20x_adc_scale_current(int channel, int *val, int *val2)
+{
+	switch (channel) {
+	case AXP20X_ACIN_I:
+		*val = 0;
+		*val2 = 625000;
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case AXP20X_VBUS_I:
+		*val = 0;
+		*val2 = 375000;
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case AXP20X_BATT_DISCHRG_I:
+	case AXP20X_BATT_CHRG_I:
+		*val = 0;
+		*val2 = 500000;
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int axp20x_adc_scale(struct iio_chan_spec const *chan, int *val,
+			    int *val2)
+{
+	switch (chan->type) {
+	case IIO_VOLTAGE:
+		return axp20x_adc_scale_voltage(chan->channel, val, val2);
+
+	case IIO_CURRENT:
+		return axp20x_adc_scale_current(chan->channel, val, val2);
+
+	case IIO_TEMP:
+		*val = 100;
+		return IIO_VAL_INT;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int axp22x_adc_scale(struct iio_chan_spec const *chan, int *val,
+			    int *val2)
+{
+	switch (chan->type) {
+	case IIO_VOLTAGE:
+		if (chan->channel != AXP22X_BATT_V)
+			return -EINVAL;
+
+		*val = 1;
+		*val2 = 100000;
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case IIO_CURRENT:
+		*val = 0;
+		*val2 = 500000;
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case IIO_TEMP:
+		*val = 100;
+		return IIO_VAL_INT;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int axp20x_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
+				     int *val)
+{
+	struct axp20x_adc_iio *info = iio_priv(indio_dev);
+	int ret, reg;
+
+	if (channel != AXP20X_GPIO0_V && channel != AXP20X_GPIO1_V)
+		return -EINVAL;
+
+	ret = regmap_read(info->regmap, AXP20X_GPIO10_IN_RANGE, &reg);
+	if (ret < 0)
+		return ret;
+
+	if (channel == AXP20X_GPIO0_V)
+		*val = reg & AXP20X_GPIO10_IN_RANGE_GPIO0;
+	else
+		*val = reg & AXP20X_GPIO10_IN_RANGE_GPIO1;
+
+	*val = !!(*val) * 700000;
+
+	return IIO_VAL_INT;
+}
+
+static int axp20x_adc_offset(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan, int *val)
+{
+	switch (chan->type) {
+	case IIO_VOLTAGE:
+		return axp20x_adc_offset_voltage(indio_dev, chan->channel, val);
+
+	case IIO_TEMP:
+		*val = -1447;
+		return IIO_VAL_INT;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int axp20x_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:
+		return axp20x_adc_offset(indio_dev, chan, val);
+
+	case IIO_CHAN_INFO_SCALE:
+		return axp20x_adc_scale(chan, val, val2);
+
+	case IIO_CHAN_INFO_RAW:
+		return axp20x_adc_raw(indio_dev, chan, val);
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int axp22x_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 = -2677;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		return axp22x_adc_scale(chan, val, val2);
+
+	case IIO_CHAN_INFO_RAW:
+		return axp22x_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)
+{
+	struct axp20x_adc_iio *info = iio_priv(indio_dev);
+
+	/*
+	 * The AXP20X PMIC allows the user to choose between 0V and 0.7V offsets
+	 * for (independently) GPIO0 and GPIO1 when in ADC mode.
+	 */
+	if (mask != IIO_CHAN_INFO_OFFSET)
+		return -EINVAL;
+
+	if (chan->channel != AXP20X_GPIO0_V && chan->channel != AXP20X_GPIO1_V)
+		return -EINVAL;
+
+	if (val != 0 && val != 700000)
+		return -EINVAL;
+
+	if (chan->channel == AXP20X_GPIO0_V)
+		return regmap_update_bits(info->regmap, AXP20X_GPIO10_IN_RANGE,
+					  AXP20X_GPIO10_IN_RANGE_GPIO0,
+					  AXP20X_GPIO10_IN_RANGE_GPIO0_VAL(!!val));
+
+	return regmap_update_bits(info->regmap, AXP20X_GPIO10_IN_RANGE,
+				  AXP20X_GPIO10_IN_RANGE_GPIO1,
+				  AXP20X_GPIO10_IN_RANGE_GPIO1_VAL(!!val));
+}
+
+static const struct iio_info axp20x_adc_iio_info = {
+	.read_raw = axp20x_read_raw,
+	.write_raw = axp20x_write_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static const struct iio_info axp22x_adc_iio_info = {
+	.read_raw = axp22x_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static int axp20x_adc_rate(int rate)
+{
+	return AXP20X_ADC_RATE_HZ(rate);
+}
+
+static int axp22x_adc_rate(int rate)
+{
+	return AXP22X_ADC_RATE_HZ(rate);
+}
+
+struct axp_data {
+	const struct iio_info		*iio_info;
+	int				num_channels;
+	struct iio_chan_spec const	*channels;
+	unsigned long			adc_en1_mask;
+	int				(*adc_rate)(int rate);
+	bool				adc_en2;
+	struct iio_map			*maps;
+};
+
+static const struct axp_data axp20x_data = {
+	.iio_info = &axp20x_adc_iio_info,
+	.num_channels = ARRAY_SIZE(axp20x_adc_channels),
+	.channels = axp20x_adc_channels,
+	.adc_en1_mask = AXP20X_ADC_EN1_MASK,
+	.adc_rate = axp20x_adc_rate,
+	.adc_en2 = true,
+	.maps = axp20x_maps,
+};
+
+static const struct axp_data axp22x_data = {
+	.iio_info = &axp22x_adc_iio_info,
+	.num_channels = ARRAY_SIZE(axp22x_adc_channels),
+	.channels = axp22x_adc_channels,
+	.adc_en1_mask = AXP22X_ADC_EN1_MASK,
+	.adc_rate = axp22x_adc_rate,
+	.adc_en2 = false,
+};
+
+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, },
+	{ /* sentinel */ },
+};
+
+static int axp20x_probe(struct platform_device *pdev)
+{
+	struct axp20x_adc_iio *info;
+	struct iio_dev *indio_dev;
+	struct axp20x_dev *axp20x_dev;
+	int ret;
+
+	axp20x_dev = dev_get_drvdata(pdev->dev.parent);
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	info = iio_priv(indio_dev);
+	platform_set_drvdata(pdev, indio_dev);
+
+	info->regmap = axp20x_dev->regmap;
+	indio_dev->name = dev_name(&pdev->dev);
+	indio_dev->dev.parent = &pdev->dev;
+	indio_dev->dev.of_node = pdev->dev.of_node;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	info->data = (struct axp_data *)of_device_get_match_data(&pdev->dev);
+
+	indio_dev->info = info->data->iio_info;
+	indio_dev->num_channels = info->data->num_channels;
+	indio_dev->channels = info->data->channels;
+
+	/* Enable the ADCs on IP */
+	regmap_write(info->regmap, AXP20X_ADC_EN1, info->data->adc_en1_mask);
+
+	if (info->data->adc_en2)
+		/* Enable GPIO0/1 and internal temperature ADCs */
+		regmap_update_bits(info->regmap, AXP20X_ADC_EN2,
+				   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));
+
+	ret = iio_map_array_register(indio_dev, info->data->maps);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to register IIO maps: %d\n", ret);
+		return ret;
+	}
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "could not register the device\n");
+		regmap_write(info->regmap, AXP20X_ADC_EN1, 0);
+
+		if (info->data->adc_en2)
+			regmap_write(info->regmap, AXP20X_ADC_EN2, 0);
+
+		return ret;
+	}
+
+	return 0;
+}
+
+static int axp20x_remove(struct platform_device *pdev)
+{
+	struct axp20x_adc_iio *info;
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+
+	info = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+
+	regmap_write(info->regmap, AXP20X_ADC_EN1, 0);
+
+	if (info->data->adc_en2)
+		regmap_write(info->regmap, AXP20X_ADC_EN2, 0);
+
+	return 0;
+}
+
+static struct platform_driver axp20x_adc_driver = {
+	.driver = {
+		.name = "axp20x-adc",
+		.of_match_table = axp20x_adc_of_match,
+	},
+	.probe = axp20x_probe,
+	.remove = axp20x_remove,
+};
+
+module_platform_driver(axp20x_adc_driver);
+
+MODULE_DESCRIPTION("ADC driver for AXP20X and AXP22X PMICs");
+MODULE_AUTHOR("Quentin Schulz <quentin.schulz@free-electrons.com>");
+MODULE_LICENSE("GPL");
-- 
2.9.3

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

* [PATCH v2 05/25] mfd: axp20x: add ADC cells for AXP20X and AXP22X PMICs
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (3 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 04/25] iio: adc: add support for X-Powers AXP20X and AXP22X PMICs ADCs Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-27  8:54 ` [PATCH v2 06/25] ARM: dtsi: axp209: add AXP209 ADC subnode Quentin Schulz
                   ` (20 subsequent siblings)
  25 siblings, 0 replies; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

This adds the AXP20X/AXP22x ADCs driver to the mfd cells of the AXP209,
AXP221 and AXP223 MFD.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
Acked-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/mfd/axp20x.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 9c2fd37..cdc729d 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -581,6 +581,9 @@ static struct mfd_cell axp20x_cells[] = {
 	}, {
 		.name		= "axp20x-regulator",
 	}, {
+		.name		= "axp20x-adc",
+		.of_compatible	= "x-powers,axp209-adc",
+	}, {
 		.name		= "axp20x-ac-power-supply",
 		.of_compatible	= "x-powers,axp202-ac-power-supply",
 		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
@@ -601,6 +604,9 @@ static struct mfd_cell axp221_cells[] = {
 	}, {
 		.name		= "axp20x-regulator",
 	}, {
+		.name		= "axp20x-adc",
+		.of_compatible	= "x-powers,axp221-adc"
+	}, {
 		.name		= "axp20x-usb-power-supply",
 		.of_compatible	= "x-powers,axp221-usb-power-supply",
 		.num_resources	= ARRAY_SIZE(axp22x_usb_power_supply_resources),
@@ -614,6 +620,9 @@ static struct mfd_cell axp223_cells[] = {
 		.num_resources		= ARRAY_SIZE(axp22x_pek_resources),
 		.resources		= axp22x_pek_resources,
 	}, {
+		.name		= "axp20x-adc",
+		.of_compatible	= "x-powers,axp221-adc"
+	}, {
 		.name			= "axp20x-regulator",
 	}, {
 		.name		= "axp20x-usb-power-supply",
-- 
2.9.3

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

* [PATCH v2 06/25] ARM: dtsi: axp209: add AXP209 ADC subnode
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (4 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 05/25] mfd: axp20x: add ADC cells for AXP20X and AXP22X PMICs Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-31  4:08   ` Chen-Yu Tsai
  2017-01-27  8:54 ` [PATCH v2 07/25] ARM: dtsi: axp22x: add AXP22X " Quentin Schulz
                   ` (19 subsequent siblings)
  25 siblings, 1 reply; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

X-Powers AXP209 PMIC has multiple ADCs, each one exposing data from the
different power supplies connected to the PMIC.

This adds the ADC subnode for AXP20X PMIC.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---

v2:
 - removed #io-channels property (the IIO channels mapping is done by
 using iio_map structure in the ADC driver),

 arch/arm/boot/dts/axp209.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
index 675bb0f..00f6ff5 100644
--- a/arch/arm/boot/dts/axp209.dtsi
+++ b/arch/arm/boot/dts/axp209.dtsi
@@ -53,6 +53,10 @@
 	interrupt-controller;
 	#interrupt-cells = <1>;
 
+	axp209_adc: adc {
+		compatible = "x-powers,axp209-adc";
+	};
+
 	axp_gpio: gpio {
 		compatible = "x-powers,axp209-gpio";
 		gpio-controller;
-- 
2.9.3

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

* [PATCH v2 07/25] ARM: dtsi: axp22x: add AXP22X ADC subnode
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (5 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 06/25] ARM: dtsi: axp209: add AXP209 ADC subnode Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-27  8:54 ` [PATCH v2 08/25] dt-bindings: power: supply: add AXP20X/AXP22X AC power supply Quentin Schulz
                   ` (18 subsequent siblings)
  25 siblings, 0 replies; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

X-Powers AXP22X PMIC has multiple ADCs, each one exposing data from the
different power supplies connected to the PMIC.

This adds the ADC subnode for AXP22X PMIC.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---

v2:
 - removed #io-channels property (the IIO channels mapping is done by
 using iio_map structure in the ADC driver),

 arch/arm/boot/dts/axp22x.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/axp22x.dtsi b/arch/arm/boot/dts/axp22x.dtsi
index 458b668..abeec81 100644
--- a/arch/arm/boot/dts/axp22x.dtsi
+++ b/arch/arm/boot/dts/axp22x.dtsi
@@ -52,6 +52,10 @@
 	interrupt-controller;
 	#interrupt-cells = <1>;
 
+	axp221_adc: adc {
+		compatible = "x-powers,axp221-adc";
+	};
+
 	regulators {
 		/* Default work frequency for buck regulators */
 		x-powers,dcdc-freq = <3000>;
-- 
2.9.3

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

* [PATCH v2 08/25] dt-bindings: power: supply: add AXP20X/AXP22X AC power supply
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (6 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 07/25] ARM: dtsi: axp22x: add AXP22X " Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-29 16:22   ` Sebastian Reichel
  2017-01-27  8:54 ` [PATCH v2 09/25] iio: adc: axp20x_adc: map acin_i and acin_v Quentin Schulz
                   ` (17 subsequent siblings)
  25 siblings, 1 reply; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The X-Powers AXP20X and AXP22X PMICs have an AC entry to supply power to
the board. They have a few registers dedicated to the status of the AC
power supply.

This adds the DT binding documentation for the AC power supply for
AXP20X and AXP22X PMICs.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---

v2:
 - removed #io-channels property,

 .../bindings/power/supply/axp20x_ac_power.txt      | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt

diff --git a/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt b/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
new file mode 100644
index 0000000..826e8a8
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
@@ -0,0 +1,22 @@
+AXP20X and AXP22X PMICs' AC power supply
+
+Required Properties:
+ - compatible: One of:
+			"x-powers,axp202-ac-power-supply"
+			"x-powers,axp221-ac-power-supply"
+
+This node is a subnode of the axp20x PMIC.
+
+The AXP20X can read the current current and voltage supplied by AC by
+reading ADC channels from the AXP20X ADC.
+
+The AXP22X is only able to tell if an AC power supply is present and
+usable.
+
+Example:
+
+&axp209 {
+	ac_power_supply: ac-power-supply {
+		compatible = "x-powers,axp202-ac-power-supply";
+	};
+};
-- 
2.9.3

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

* [PATCH v2 09/25] iio: adc: axp20x_adc: map acin_i and acin_v
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (7 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 08/25] dt-bindings: power: supply: add AXP20X/AXP22X AC power supply Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-28 14:51   ` Jonathan Cameron
  2017-01-29 16:31   ` Sebastian Reichel
  2017-01-27  8:54 ` [PATCH v2 10/25] power: supply: add AC power supply driver for AXP20X and AXP22X PMICs Quentin Schulz
                   ` (16 subsequent siblings)
  25 siblings, 2 replies; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

This maps the IIO channels acin_i and acin_v (respectively exposing the
current current and voltage measures of the AC power supply) to the AC
power supply driver.

Only the AXP20X PMICs have these ADC channels and thus they are only
mapped for this version of the PMIC.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---

added in v2

 drivers/iio/adc/axp20x_adc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
index bacde92..14f4ec0 100644
--- a/drivers/iio/adc/axp20x_adc.c
+++ b/drivers/iio/adc/axp20x_adc.c
@@ -104,6 +104,14 @@ static struct iio_map axp20x_maps[] = {
 		.consumer_dev_name = "axp20x-usb-power-supply",
 		.consumer_channel = "vbus_i",
 		.adc_channel_label = "vbus_i",
+	}, {
+		.consumer_dev_name = "axp20x-ac-power-supply",
+		.consumer_channel = "acin_v",
+		.adc_channel_label = "acin_v",
+	}, {
+		.consumer_dev_name = "axp20x-ac-power-supply",
+		.consumer_channel = "acin_i",
+		.adc_channel_label = "acin_i",
 	}, { /* sentinel */ }
 };
 
-- 
2.9.3

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

* [PATCH v2 10/25] power: supply: add AC power supply driver for AXP20X and AXP22X PMICs
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (8 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 09/25] iio: adc: axp20x_adc: map acin_i and acin_v Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-29 16:25   ` Sebastian Reichel
  2017-01-27  8:54 ` [PATCH v2 11/25] mfd: axp20x: add AC power supply cells for " Quentin Schulz
                   ` (15 subsequent siblings)
  25 siblings, 1 reply; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The X-Powers AXP20X and AXP22X PMICs expose the status of AC power
supply.

Moreover, the AXP20X can also expose the current current and voltage
values of the AC power supply.

This adds the driver which exposes the status of the AC power supply of
the AXP20X and AXP22X PMICs.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
Acked-by: Jonathan Cameron <jic23@kernel.org>
---

v2:
 - replaced µ character by a common u for micro units to make checkpatch
 happy,
 - use of structure for specific data instead of an ID and if
 condiftions,
 - use dev_get_regmap instead of manually looking for it in the parent
 drvdata,

 drivers/power/supply/Kconfig           |  12 ++
 drivers/power/supply/Makefile          |   1 +
 drivers/power/supply/axp20x_ac_power.c | 250 +++++++++++++++++++++++++++++++++
 3 files changed, 263 insertions(+)
 create mode 100644 drivers/power/supply/axp20x_ac_power.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 76806a0..c552b4b 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -214,6 +214,18 @@ config BATTERY_DA9150
 	  This driver can also be built as a module. If so, the module will be
 	  called da9150-fg.
 
+config CHARGER_AXP20X
+	tristate "X-Powers AXP20X and AXP22X AC power supply driver"
+	depends on MFD_AXP20X
+	depends on AXP20X_ADC
+	depends on IIO
+	help
+	  Say Y here to enable support for X-Powers AXP20X and AXP22X PMICs' AC
+	  power supply.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called axp20x_ac_power.
+
 config AXP288_CHARGER
 	tristate "X-Powers AXP288 Charger"
 	depends on MFD_AXP20X && EXTCON_AXP288
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 36c599d..7d22417 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_TEST_POWER)	+= test_power.o
 
 obj-$(CONFIG_BATTERY_88PM860X)	+= 88pm860x_battery.o
 obj-$(CONFIG_BATTERY_ACT8945A)	+= act8945a_charger.o
+obj-$(CONFIG_CHARGER_AXP20X)	+= axp20x_ac_power.o
 obj-$(CONFIG_BATTERY_DS2760)	+= ds2760_battery.o
 obj-$(CONFIG_BATTERY_DS2780)	+= ds2780_battery.o
 obj-$(CONFIG_BATTERY_DS2781)	+= ds2781_battery.o
diff --git a/drivers/power/supply/axp20x_ac_power.c b/drivers/power/supply/axp20x_ac_power.c
new file mode 100644
index 0000000..59ab4f2
--- /dev/null
+++ b/drivers/power/supply/axp20x_ac_power.c
@@ -0,0 +1,250 @@
+/*
+ * AXP20X and AXP22X PMICs' ACIN power supply driver
+ *
+ * Copyright (C) 2016 Free Electrons
+ *	Quentin Schulz <quentin.schulz@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under  the terms of the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/axp20x.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/iio/consumer.h>
+
+#define AXP20X_PWR_STATUS_ACIN_PRESENT	BIT(7)
+#define AXP20X_PWR_STATUS_ACIN_AVAIL	BIT(6)
+
+#define DRVNAME "axp20x-ac-power-supply"
+
+struct axp20x_ac_power {
+	struct device_node *np;
+	struct regmap *regmap;
+	struct power_supply *supply;
+	int axp20x_id;
+	struct iio_channel *acin_v;
+	struct iio_channel *acin_i;
+};
+
+static irqreturn_t axp20x_ac_power_irq(int irq, void *devid)
+{
+	struct axp20x_ac_power *power = devid;
+
+	power_supply_changed(power->supply);
+
+	return IRQ_HANDLED;
+}
+
+static int axp20x_ac_power_get_property(struct power_supply *psy,
+					enum power_supply_property psp,
+					union power_supply_propval *val)
+{
+	struct axp20x_ac_power *power = power_supply_get_drvdata(psy);
+	int ret, reg;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_HEALTH:
+		ret = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, &reg);
+		if (ret)
+			return ret;
+
+		if (reg & AXP20X_PWR_STATUS_ACIN_PRESENT) {
+			val->intval = POWER_SUPPLY_HEALTH_GOOD;
+			return 0;
+		}
+
+		val->intval = POWER_SUPPLY_HEALTH_UNKNOWN;
+		return 0;
+
+	case POWER_SUPPLY_PROP_PRESENT:
+		ret = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, &reg);
+		if (ret)
+			return ret;
+
+		val->intval = !!(reg & AXP20X_PWR_STATUS_ACIN_PRESENT);
+		return 0;
+
+	case POWER_SUPPLY_PROP_ONLINE:
+		ret = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, &reg);
+		if (ret)
+			return ret;
+
+		val->intval = !!(reg & AXP20X_PWR_STATUS_ACIN_AVAIL);
+		return 0;
+
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		ret = iio_read_channel_processed(power->acin_v, &val->intval);
+		if (ret)
+			return ret;
+
+		/* IIO framework gives mV but Power Supply framework gives uV */
+		val->intval *= 1000;
+
+		return 0;
+
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		ret = iio_read_channel_processed(power->acin_i, &val->intval);
+		if (ret)
+			return ret;
+
+		/* IIO framework gives mA but Power Supply framework gives uA */
+		val->intval *= 1000;
+
+		return 0;
+
+	default:
+		return -EINVAL;
+	}
+
+	return -EINVAL;
+}
+
+static enum power_supply_property axp20x_ac_power_properties[] = {
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+};
+
+static enum power_supply_property axp22x_ac_power_properties[] = {
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_ONLINE,
+};
+
+static const struct power_supply_desc axp20x_ac_power_desc = {
+	.name = "axp20x-ac",
+	.type = POWER_SUPPLY_TYPE_MAINS,
+	.properties = axp20x_ac_power_properties,
+	.num_properties = ARRAY_SIZE(axp20x_ac_power_properties),
+	.get_property = axp20x_ac_power_get_property,
+};
+
+static const struct power_supply_desc axp22x_ac_power_desc = {
+	.name = "axp22x-ac",
+	.type = POWER_SUPPLY_TYPE_MAINS,
+	.properties = axp22x_ac_power_properties,
+	.num_properties = ARRAY_SIZE(axp22x_ac_power_properties),
+	.get_property = axp20x_ac_power_get_property,
+};
+
+struct axp_data {
+	const struct power_supply_desc	*power_desc;
+	bool				acin_adc;
+};
+
+static const struct axp_data axp20x_data = {
+	.power_desc = &axp20x_ac_power_desc,
+	.acin_adc = true,
+};
+
+static const struct axp_data axp22x_data = {
+	.power_desc = &axp22x_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);
+	struct power_supply_config psy_cfg = {};
+	struct axp20x_ac_power *power;
+	struct axp_data *axp_data;
+	static const char * const irq_names[] = { "ACIN_PLUGIN", "ACIN_REMOVAL",
+		NULL };
+	int i, irq, ret;
+
+	if (!of_device_is_available(pdev->dev.of_node))
+		return -ENODEV;
+
+	if (!axp20x) {
+		dev_err(&pdev->dev, "Parent drvdata not set\n");
+		return -EINVAL;
+	}
+
+	power = devm_kzalloc(&pdev->dev, sizeof(*power), GFP_KERNEL);
+	if (!power)
+		return -ENOMEM;
+
+	axp_data = (struct axp_data *)of_device_get_match_data(&pdev->dev);
+
+	if (axp_data->acin_adc) {
+		power->acin_v = devm_iio_channel_get(&pdev->dev, "acin_v");
+		if (IS_ERR(power->acin_v)) {
+			if (PTR_ERR(power->acin_v) == -ENODEV)
+				return -EPROBE_DEFER;
+			return PTR_ERR(power->acin_v);
+		}
+
+		power->acin_i = devm_iio_channel_get(&pdev->dev, "acin_i");
+		if (IS_ERR(power->acin_i)) {
+			if (PTR_ERR(power->acin_i) == -ENODEV)
+				return -EPROBE_DEFER;
+			return PTR_ERR(power->acin_i);
+		}
+	}
+
+	power->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+
+	platform_set_drvdata(pdev, power);
+
+	psy_cfg.of_node = pdev->dev.of_node;
+	psy_cfg.drv_data = power;
+
+	power->supply = devm_power_supply_register(&pdev->dev,
+						   axp_data->power_desc,
+						   &psy_cfg);
+	if (IS_ERR(power->supply))
+		return PTR_ERR(power->supply);
+
+	/* Request irqs after registering, as irqs may trigger immediately */
+	for (i = 0; irq_names[i]; i++) {
+		irq = platform_get_irq_byname(pdev, irq_names[i]);
+		if (irq < 0) {
+			dev_warn(&pdev->dev, "No IRQ for %s: %d\n",
+				 irq_names[i], irq);
+			continue;
+		}
+		irq = regmap_irq_get_virq(axp20x->regmap_irqc, irq);
+		ret = devm_request_any_context_irq(&pdev->dev, irq,
+						   axp20x_ac_power_irq, 0,
+						   DRVNAME, power);
+		if (ret < 0)
+			dev_warn(&pdev->dev, "Error requesting %s IRQ: %d\n",
+				 irq_names[i], ret);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id axp20x_ac_power_match[] = {
+	{
+		.compatible = "x-powers,axp202-ac-power-supply",
+		.data = (void *)&axp20x_data,
+	}, {
+		.compatible = "x-powers,axp221-ac-power-supply",
+		.data = (void *)&axp22x_data,
+	}, { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, axp20x_ac_power_match);
+
+static struct platform_driver axp20x_ac_power_driver = {
+	.probe = axp20x_ac_power_probe,
+	.driver = {
+		.name = DRVNAME,
+		.of_match_table = axp20x_ac_power_match,
+	},
+};
+
+module_platform_driver(axp20x_ac_power_driver);
+
+MODULE_AUTHOR("Quentin Schulz <quentin.schulz@free-electrons.com>");
+MODULE_DESCRIPTION("AXP20X and AXP22X PMICs' AC power supply driver");
+MODULE_LICENSE("GPL");
-- 
2.9.3

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

* [PATCH v2 11/25] mfd: axp20x: add AC power supply cells for AXP22X PMICs
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (9 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 10/25] power: supply: add AC power supply driver for AXP20X and AXP22X PMICs Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-29 16:35   ` Sebastian Reichel
  2017-01-27  8:54 ` [PATCH v2 12/25] ARM: dtsi: axp209: add AC power supply subnode Quentin Schulz
                   ` (14 subsequent siblings)
  25 siblings, 1 reply; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The X-Powers AXP20X and AXP22X PMICs expose the status of AC power
supply.

This adds the AC power supply driver to the MFD cells of the AXP22X
PMICs.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/axp20x.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index cdc729d..7607c30 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -607,6 +607,11 @@ static struct mfd_cell axp221_cells[] = {
 		.name		= "axp20x-adc",
 		.of_compatible	= "x-powers,axp221-adc"
 	}, {
+		.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-usb-power-supply",
 		.of_compatible	= "x-powers,axp221-usb-power-supply",
 		.num_resources	= ARRAY_SIZE(axp22x_usb_power_supply_resources),
@@ -625,6 +630,11 @@ static struct mfd_cell axp223_cells[] = {
 	}, {
 		.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-usb-power-supply",
 		.of_compatible	= "x-powers,axp223-usb-power-supply",
 		.num_resources	= ARRAY_SIZE(axp22x_usb_power_supply_resources),
-- 
2.9.3

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

* [PATCH v2 12/25] ARM: dtsi: axp209: add AC power supply subnode
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (10 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 11/25] mfd: axp20x: add AC power supply cells for " Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-31  4:12   ` Chen-Yu Tsai
  2017-01-27  8:54 ` [PATCH v2 13/25] ARM: dtsi: axp22x: " Quentin Schulz
                   ` (13 subsequent siblings)
  25 siblings, 1 reply; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The X-Powers AXP20X PMIC exposes the status of AC power supply, the
current current and voltage supplied to the board by the AC power
supply.

This adds the AC power supply subnode for AXP20X PMIC.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---

v2:
 - changed DT node name from ac_power_supply to ac-power-supply,
 - removed io-channels and io-channel-names from DT (the IIO mapping is
 done in the IIO ADC driver now),

 arch/arm/boot/dts/axp209.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
index 00f6ff5..8675c696 100644
--- a/arch/arm/boot/dts/axp209.dtsi
+++ b/arch/arm/boot/dts/axp209.dtsi
@@ -53,6 +53,11 @@
 	interrupt-controller;
 	#interrupt-cells = <1>;
 
+	ac_power_supply: ac-power-supply {
+		compatible = "x-powers,axp202-ac-power-supply";
+		status = "disabled";
+	};
+
 	axp209_adc: adc {
 		compatible = "x-powers,axp209-adc";
 	};
-- 
2.9.3

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

* [PATCH v2 13/25] ARM: dtsi: axp22x: add AC power supply subnode
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (11 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 12/25] ARM: dtsi: axp209: add AC power supply subnode Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-31  4:13   ` Chen-Yu Tsai
  2017-01-27  8:54 ` [PATCH v2 14/25] ARM: dts: sun8i: sina33: enable ACIN " Quentin Schulz
                   ` (12 subsequent siblings)
  25 siblings, 1 reply; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The X-Powers AXP22X PMIC exposes the status of AC power supply.

This adds the AC power supply subnode for the AXP22X PMIC.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---

v2:
 - changed DT node name from ac_power_supply to ac-power-supply,
 - removed io-channels and io-channel-names from DT (the IIO mapping is
 done in the IIO ADC driver now),

 arch/arm/boot/dts/axp22x.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/axp22x.dtsi b/arch/arm/boot/dts/axp22x.dtsi
index abeec81..f2835b4 100644
--- a/arch/arm/boot/dts/axp22x.dtsi
+++ b/arch/arm/boot/dts/axp22x.dtsi
@@ -52,6 +52,11 @@
 	interrupt-controller;
 	#interrupt-cells = <1>;
 
+	ac_power_supply: ac-power-supply {
+		compatible = "x-powers,axp221-ac-power-supply";
+		status = "disabled";
+	};
+
 	axp221_adc: adc {
 		compatible = "x-powers,axp221-adc";
 	};
-- 
2.9.3

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

* [PATCH v2 14/25] ARM: dts: sun8i: sina33: enable ACIN power supply subnode
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (12 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 13/25] ARM: dtsi: axp22x: " Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-31  5:03   ` Chen-Yu Tsai
  2017-01-27  8:54 ` [PATCH v2 15/25] ARM: sun5i: chip: " Quentin Schulz
                   ` (11 subsequent siblings)
  25 siblings, 1 reply; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The Sinlinx SinA33 has an AXP223 PMIC and an ACIN connector, thus, we
enable the ACIN power supply in its Device Tree.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
 arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts b/arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts
index 28c58c5..bf53408 100644
--- a/arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts
+++ b/arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts
@@ -147,6 +147,10 @@
 
 #include "axp223.dtsi"
 
+&ac_power_supply {
+	status = "okay";
+};
+
 &reg_aldo1 {
 	regulator-always-on;
 	regulator-min-microvolt = <3000000>;
-- 
2.9.3

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

* [PATCH v2 15/25] ARM: sun5i: chip: enable ACIN power supply subnode
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (13 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 14/25] ARM: dts: sun8i: sina33: enable ACIN " Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-31  5:03   ` Chen-Yu Tsai
  2017-01-27  8:54 ` [PATCH v2 16/25] dt-bindings: power: supply: add AXP20X/AXP22X battery DT binding Quentin Schulz
                   ` (10 subsequent siblings)
  25 siblings, 1 reply; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The NextThing Co. CHIP has an AXP209 PMIC and can be power-supplied by
ACIN via the CHG-IN pin.

This enables the ACIN power supply subnode in the DT.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
 arch/arm/boot/dts/sun5i-r8-chip.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/sun5i-r8-chip.dts b/arch/arm/boot/dts/sun5i-r8-chip.dts
index c6da5ad..6011757 100644
--- a/arch/arm/boot/dts/sun5i-r8-chip.dts
+++ b/arch/arm/boot/dts/sun5i-r8-chip.dts
@@ -128,6 +128,10 @@
 
 #include "axp209.dtsi"
 
+&ac_power_supply {
+	status = "okay";
+};
+
 &i2c1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c1_pins_a>;
-- 
2.9.3

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

* [PATCH v2 16/25] dt-bindings: power: supply: add AXP20X/AXP22X battery DT binding
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (14 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 15/25] ARM: sun5i: chip: " Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-29 16:47   ` Sebastian Reichel
  2017-01-27  8:54 ` [PATCH v2 17/25] mfd: axp20x: add CHRG_CTRL1/2/3 to writeable regs for AXP20X/AXP22X Quentin Schulz
                   ` (9 subsequent siblings)
  25 siblings, 1 reply; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The X-Powers AXP20X and AXP22X PMICs can have a battery as power supply.

This patch adds the DT binding documentation for the battery power
supply which gets various data from the PMIC, such as the battery status
(charging, discharging, full, dead), current max limit, current current,
battery capacity (in percentage), voltage max and min limits, current
voltage and battery capacity (in Ah).

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---

v2:
 - changed DT node name from ac_power_supply to ac-power-supply,
 - removed io-channels and io-channel-names from DT (the IIO mapping is
 done in the IIO ADC driver now),
 - added x-powers,constant-charge-current property to set the maximal
 default constant current charge of the battery,

 .../bindings/power/supply/axp20x_battery.txt       | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/axp20x_battery.txt

diff --git a/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt b/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
new file mode 100644
index 0000000..6f7eff2
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
@@ -0,0 +1,25 @@
+AXP20x and AXP22x battery power supply
+
+Required Properties:
+ - compatible, one of:
+			"x-powers,axp209-battery-power-supply"
+			"x-powers,axp221-battery-power-supply"
+
+Optional properties:
+ - x-powers,constant-charge-current: its value in uA gives the PMIC the default
+ maximal allowed value for the constant charge current of the battery.
+
+This node is a subnode of the axp20x/axp22x PMIC.
+
+The AXP20X and AXP22X can read the battery voltage, charge and discharge
+currents of the battery by reading ADC channels from the AXP20X/AXP22X
+ADC.
+
+Example:
+
+&axp209 {
+	battery_power_supply: battery-power-supply {
+		compatible = "x-powers,axp209-battery-power-supply";
+		x-powers,constant-charge-current = <600000>;
+	}
+};
-- 
2.9.3

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

* [PATCH v2 17/25] mfd: axp20x: add CHRG_CTRL1/2/3 to writeable regs for AXP20X/AXP22X
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (15 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 16/25] dt-bindings: power: supply: add AXP20X/AXP22X battery DT binding Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-31  5:05   ` Chen-Yu Tsai
  2017-01-27  8:54 ` [PATCH v2 18/25] mfd: axp20x: add V_OFF to writeable regs for AXP20X and AXP22X Quentin Schulz
                   ` (8 subsequent siblings)
  25 siblings, 1 reply; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The CHRG_CTRL1 and CHRG_CTRL2 registers are made for controlling
different battery charging settings such as the constant current charge
value.

The AXP22X also have a third register CHRG_CTRL3 which has settings for
battery charging too.

This adds the CHRG_CTRL1, CHRG_CTRL2 and CHRG_CTRL3 registers to the
list of writeable registers for AXP20X and AXP22X PMICs.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---

v2:
 - added AXP20X_CHRG_CTRL2 and AXP20X_CHRG_CTRL3 to the writeable
 registers table,
 - removed added reg range for ADC data in volatile regs range,

 drivers/mfd/axp20x.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 7607c30..8210623 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -66,6 +66,7 @@ static const struct regmap_access_table axp152_volatile_table = {
 static const struct regmap_range axp20x_writeable_ranges[] = {
 	regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE),
 	regmap_reg_range(AXP20X_VBUS_IPSOUT_MGMT, AXP20X_VBUS_IPSOUT_MGMT),
+	regmap_reg_range(AXP20X_CHRG_CTRL1, AXP20X_CHRG_CTRL2),
 	regmap_reg_range(AXP20X_DCDC_MODE, AXP20X_FG_RES),
 	regmap_reg_range(AXP20X_RDC_H, AXP20X_OCV(AXP20X_OCV_MAX)),
 };
@@ -94,6 +95,7 @@ static const struct regmap_access_table axp20x_volatile_table = {
 static const struct regmap_range axp22x_writeable_ranges[] = {
 	regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE),
 	regmap_reg_range(AXP20X_VBUS_IPSOUT_MGMT, AXP20X_VBUS_IPSOUT_MGMT),
+	regmap_reg_range(AXP20X_CHRG_CTRL1, AXP22X_CHRG_CTRL3),
 	regmap_reg_range(AXP20X_DCDC_MODE, AXP22X_BATLOW_THRES1),
 };
 
-- 
2.9.3

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

* [PATCH v2 18/25] mfd: axp20x: add V_OFF to writeable regs for AXP20X and AXP22X
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (16 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 17/25] mfd: axp20x: add CHRG_CTRL1/2/3 to writeable regs for AXP20X/AXP22X Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-27  8:54 ` [PATCH v2 19/25] iio: adc: axp20x_adc: map battery IIO channels Quentin Schulz
                   ` (7 subsequent siblings)
  25 siblings, 0 replies; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The V_OFF register has its first 3 read-write bits for the minimal
voltage (Voff) of the battery before the system is automatically shut
down due to the power being too low.

This adds V_OFF register to the writeable registers of AXP20X and AXP22X
PMICs.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
Acked-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/mfd/axp20x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 8210623..223dffe 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -65,7 +65,7 @@ static const struct regmap_access_table axp152_volatile_table = {
 
 static const struct regmap_range axp20x_writeable_ranges[] = {
 	regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE),
-	regmap_reg_range(AXP20X_VBUS_IPSOUT_MGMT, AXP20X_VBUS_IPSOUT_MGMT),
+	regmap_reg_range(AXP20X_VBUS_IPSOUT_MGMT, AXP20X_V_OFF),
 	regmap_reg_range(AXP20X_CHRG_CTRL1, AXP20X_CHRG_CTRL2),
 	regmap_reg_range(AXP20X_DCDC_MODE, AXP20X_FG_RES),
 	regmap_reg_range(AXP20X_RDC_H, AXP20X_OCV(AXP20X_OCV_MAX)),
@@ -94,7 +94,7 @@ static const struct regmap_access_table axp20x_volatile_table = {
 /* AXP22x ranges are shared with the AXP809, as they cover the same range */
 static const struct regmap_range axp22x_writeable_ranges[] = {
 	regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE),
-	regmap_reg_range(AXP20X_VBUS_IPSOUT_MGMT, AXP20X_VBUS_IPSOUT_MGMT),
+	regmap_reg_range(AXP20X_VBUS_IPSOUT_MGMT, AXP20X_V_OFF),
 	regmap_reg_range(AXP20X_CHRG_CTRL1, AXP22X_CHRG_CTRL3),
 	regmap_reg_range(AXP20X_DCDC_MODE, AXP22X_BATLOW_THRES1),
 };
-- 
2.9.3

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

* [PATCH v2 19/25] iio: adc: axp20x_adc: map battery IIO channels
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (17 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 18/25] mfd: axp20x: add V_OFF to writeable regs for AXP20X and AXP22X Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-28 14:52   ` Jonathan Cameron
  2017-01-27  8:54 ` [PATCH v2 20/25] power: supply: add battery driver for AXP20X and AXP22X PMICs Quentin Schulz
                   ` (6 subsequent siblings)
  25 siblings, 1 reply; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

This maps the IIO channels batt_v, batt_chrg_i and batt_dischrg_i
(respectively exposing the current charging and discharging currents and
current voltage measures of the battery power supply) to the battery
power supply driver.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---

added in v2

 drivers/iio/adc/axp20x_adc.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
index 14f4ec0..64c7c75 100644
--- a/drivers/iio/adc/axp20x_adc.c
+++ b/drivers/iio/adc/axp20x_adc.c
@@ -112,6 +112,34 @@ static struct iio_map axp20x_maps[] = {
 		.consumer_dev_name = "axp20x-ac-power-supply",
 		.consumer_channel = "acin_i",
 		.adc_channel_label = "acin_i",
+	}, {
+		.consumer_dev_name = "axp20x-battery-power-supply",
+		.consumer_channel = "batt_v",
+		.adc_channel_label = "batt_v",
+	}, {
+		.consumer_dev_name = "axp20x-battery-power-supply",
+		.consumer_channel = "batt_chrg_i",
+		.adc_channel_label = "batt_chrg_i",
+	}, {
+		.consumer_dev_name = "axp20x-battery-power-supply",
+		.consumer_channel = "batt_dischrg_i",
+		.adc_channel_label = "batt_dischrg_i",
+	}, { /* sentinel */ }
+};
+
+static struct iio_map axp22x_maps[] = {
+	{
+		.consumer_dev_name = "axp20x-battery-power-supply",
+		.consumer_channel = "batt_v",
+		.adc_channel_label = "batt_v",
+	}, {
+		.consumer_dev_name = "axp20x-battery-power-supply",
+		.consumer_channel = "batt_chrg_i",
+		.adc_channel_label = "batt_chrg_i",
+	}, {
+		.consumer_dev_name = "axp20x-battery-power-supply",
+		.consumer_channel = "batt_dischrg_i",
+		.adc_channel_label = "batt_dischrg_i",
 	}, { /* sentinel */ }
 };
 
@@ -479,6 +507,7 @@ static const struct axp_data axp22x_data = {
 	.adc_en1_mask = AXP22X_ADC_EN1_MASK,
 	.adc_rate = axp22x_adc_rate,
 	.adc_en2 = false,
+	.maps = axp22x_maps,
 };
 
 static const struct of_device_id axp20x_adc_of_match[] = {
-- 
2.9.3

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

* [PATCH v2 20/25] power: supply: add battery driver for AXP20X and AXP22X PMICs
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (18 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 19/25] iio: adc: axp20x_adc: map battery IIO channels Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-28 14:54   ` Jonathan Cameron
  2017-01-27  8:54 ` [PATCH v2 21/25] mfd: axp20x: add MFD cells for AXP20X and AXP22X battery driver Quentin Schulz
                   ` (5 subsequent siblings)
  25 siblings, 1 reply; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The X-Powers AXP20X and AXP22X PMICs can have a battery as power supply.

This patch adds the battery power supply driver to get various data from
the PMIC, such as the battery status (charging, discharging, full,
dead), current max limit, current current, battery capacity (in
percentage), voltage max and min limits, current voltage and battery
capacity (in Ah).

This battery driver uses the AXP20X/AXP22X ADC driver as PMIC data
provider.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---

v2:
 - changed BIT(x) to 1 << x when describing bits purpose for which 2 <<
 x or 3 << x exists, to be consistent,
 - switched from POWER_SUPPLY_PROP_CURRENT_MAX to
 POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
 - added POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX to the list of
 readable properties,
 - replaced µ character by a common u for micro units to make checkpatch
 happy,
 - factorized code in axp20x_battery_set_max_voltage,
 - added a axp20x_set_constant_charge_current function to be used when
 setting the value from sysfs and from the DT,
 - removed some dead code,
 - added a DT property to set constant current charge of the battery
 (x-powers,constant-charge-current),
 - migrated to dev_get_regmap instead of manually looking for the regmap
 in the drvdata of the parent,
 - switched from int to uintptr_t cast to make sure the cast is always
 for the same size type (make build on 64bits platforms happy mainly),

 drivers/power/supply/Kconfig          |  12 +
 drivers/power/supply/Makefile         |   1 +
 drivers/power/supply/axp20x_battery.c | 477 ++++++++++++++++++++++++++++++++++
 3 files changed, 490 insertions(+)
 create mode 100644 drivers/power/supply/axp20x_battery.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index c552b4b..48619de 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -226,6 +226,18 @@ config CHARGER_AXP20X
 	  This driver can also be built as a module. If so, the module will be
 	  called axp20x_ac_power.
 
+config BATTERY_AXP20X
+	tristate "X-Powers AXP20X battery driver"
+	depends on MFD_AXP20X
+	depends on AXP20X_ADC
+	depends on IIO
+	help
+	  Say Y here to enable support for X-Powers AXP20X PMICs' battery power
+	  supply.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called axp20x_battery.
+
 config AXP288_CHARGER
 	tristate "X-Powers AXP288 Charger"
 	depends on MFD_AXP20X && EXTCON_AXP288
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 7d22417..5a217b2 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_TEST_POWER)	+= test_power.o
 
 obj-$(CONFIG_BATTERY_88PM860X)	+= 88pm860x_battery.o
 obj-$(CONFIG_BATTERY_ACT8945A)	+= act8945a_charger.o
+obj-$(CONFIG_BATTERY_AXP20X)	+= axp20x_battery.o
 obj-$(CONFIG_CHARGER_AXP20X)	+= axp20x_ac_power.o
 obj-$(CONFIG_BATTERY_DS2760)	+= ds2760_battery.o
 obj-$(CONFIG_BATTERY_DS2780)	+= ds2780_battery.o
diff --git a/drivers/power/supply/axp20x_battery.c b/drivers/power/supply/axp20x_battery.c
new file mode 100644
index 0000000..dd040b8
--- /dev/null
+++ b/drivers/power/supply/axp20x_battery.c
@@ -0,0 +1,477 @@
+/*
+ * Battery power supply driver for X-Powers AXP20X and AXP22X PMICs
+ *
+ * Copyright 2016 Free Electrons NextThing Co.
+ *	Quentin Schulz <quentin.schulz@free-electrons.com>
+ *
+ * This driver is based on a previous upstreaming attempt by:
+ *	Bruno Prémont <bonbons@linux-vserver.org>
+ *
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License. See the file "COPYING" in the main directory of this
+ * archive for more details.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/time.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/consumer.h>
+#include <linux/mfd/axp20x.h>
+
+#define AXP20X_PWR_STATUS_BAT_CHARGING	BIT(2)
+
+#define AXP20X_PWR_OP_BATT_PRESENT	BIT(5)
+#define AXP20X_PWR_OP_BATT_ACTIVATED	BIT(3)
+
+#define AXP209_FG_PERCENT		GENMASK(6, 0)
+#define AXP22X_FG_VALID			BIT(7)
+
+#define AXP20X_CHRG_CTRL1_TGT_VOLT	GENMASK(6, 5)
+#define AXP20X_CHRG_CTRL1_TGT_4_1V	(0 << 5)
+#define AXP20X_CHRG_CTRL1_TGT_4_15V	(1 << 5)
+#define AXP20X_CHRG_CTRL1_TGT_4_2V	(2 << 5)
+#define AXP20X_CHRG_CTRL1_TGT_4_36V	(3 << 5)
+
+#define AXP22X_CHRG_CTRL1_TGT_4_22V	(1 << 5)
+#define AXP22X_CHRG_CTRL1_TGT_4_24V	(3 << 5)
+
+#define AXP20X_CHRG_CTRL1_TGT_CURR	GENMASK(3, 0)
+
+#define AXP20X_V_OFF_MASK		GENMASK(2, 0)
+
+struct axp20x_batt_ps {
+	struct regmap *regmap;
+	struct power_supply *batt;
+	struct axp20x_dev *axp20x;
+	struct iio_channel *batt_chrg_i;
+	struct iio_channel *batt_dischrg_i;
+	struct iio_channel *batt_v;
+	u8 axp_id;
+};
+
+static int axp20x_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 AXP20X_CHRG_CTRL1_TGT_4_36V:
+		*val = 4360000;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int axp22x_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_2V:
+		*val = 4200000;
+		break;
+	case AXP22X_CHRG_CTRL1_TGT_4_22V:
+		*val = 4220000;
+		break;
+	case AXP22X_CHRG_CTRL1_TGT_4_24V:
+		*val = 4240000;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int axp20x_battery_get_prop(struct power_supply *psy,
+				   enum power_supply_property psp,
+				   union power_supply_propval *val)
+{
+	struct axp20x_batt_ps *axp20x_batt = power_supply_get_drvdata(psy);
+	struct iio_channel *chan;
+	int ret = 0, reg, val1;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_PRESENT:
+	case POWER_SUPPLY_PROP_ONLINE:
+		ret = regmap_read(axp20x_batt->regmap, AXP20X_PWR_OP_MODE,
+				  &reg);
+		if (ret)
+			return ret;
+
+		val->intval = !!(reg & AXP20X_PWR_OP_BATT_PRESENT);
+		break;
+
+	case POWER_SUPPLY_PROP_STATUS:
+		ret = regmap_read(axp20x_batt->regmap, AXP20X_PWR_INPUT_STATUS,
+				  &reg);
+		if (ret)
+			return ret;
+
+		if (reg & AXP20X_PWR_STATUS_BAT_CHARGING) {
+			val->intval = POWER_SUPPLY_STATUS_CHARGING;
+			return 0;
+		}
+
+		ret = iio_read_channel_processed(axp20x_batt->batt_dischrg_i,
+						 &val1);
+		if (ret)
+			return ret;
+
+		if (val1) {
+			val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+			return 0;
+		}
+
+		ret = regmap_read(axp20x_batt->regmap, AXP20X_FG_RES, &val1);
+		if (ret)
+			return ret;
+
+		/*
+		 * Fuel Gauge data takes 7 bits but the stored value seems to be
+		 * directly the raw percentage without any scaling to 7 bits.
+		 */
+		if ((val1 & AXP209_FG_PERCENT) == 100)
+			val->intval = POWER_SUPPLY_STATUS_FULL;
+		else
+			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
+		break;
+
+	case POWER_SUPPLY_PROP_HEALTH:
+		ret = regmap_read(axp20x_batt->regmap, AXP20X_PWR_OP_MODE,
+				  &val1);
+		if (ret)
+			return ret;
+
+		if (val1 & AXP20X_PWR_OP_BATT_ACTIVATED) {
+			val->intval = POWER_SUPPLY_HEALTH_DEAD;
+			return 0;
+		}
+
+		val->intval = POWER_SUPPLY_HEALTH_GOOD;
+		break;
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
+		ret = regmap_read(axp20x_batt->regmap, AXP20X_CHRG_CTRL1, &reg);
+		if (ret)
+			return ret;
+
+		reg &= AXP20X_CHRG_CTRL1_TGT_CURR;
+		val->intval = reg * 100000 + 300000;
+		break;
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
+		val->intval = AXP20X_CHRG_CTRL1_TGT_CURR * 100000 + 300000;
+		break;
+
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		ret = regmap_read(axp20x_batt->regmap, AXP20X_PWR_INPUT_STATUS,
+				  &reg);
+		if (ret)
+			return ret;
+
+		if (reg & AXP20X_PWR_STATUS_BAT_CHARGING)
+			chan = axp20x_batt->batt_chrg_i;
+		else
+			chan = axp20x_batt->batt_dischrg_i;
+
+		ret = iio_read_channel_processed(chan, &val->intval);
+		if (ret)
+			return ret;
+
+		/* IIO framework gives mA but Power Supply framework gives uA */
+		val->intval *= 1000;
+		break;
+
+	case POWER_SUPPLY_PROP_CAPACITY:
+		/* When no battery is present, return capacity is 100% */
+		ret = regmap_read(axp20x_batt->regmap, AXP20X_PWR_OP_MODE,
+				  &reg);
+		if (ret)
+			return ret;
+
+		if (!(reg & AXP20X_PWR_OP_BATT_PRESENT)) {
+			val->intval = 100;
+			return 0;
+		}
+
+		ret = regmap_read(axp20x_batt->regmap, AXP20X_FG_RES, &reg);
+		if (ret)
+			return ret;
+
+		if (axp20x_batt->axp_id == AXP221_ID &&
+		    !(reg & AXP22X_FG_VALID))
+			return -EINVAL;
+
+		/*
+		 * Fuel Gauge data takes 7 bits but the stored value seems to be
+		 * directly the raw percentage without any scaling to 7 bits.
+		 */
+		val->intval = reg & AXP209_FG_PERCENT;
+		break;
+
+	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
+		if (axp20x_batt->axp_id == AXP209_ID)
+			return axp20x_battery_get_max_voltage(axp20x_batt,
+							      &val->intval);
+		return axp22x_battery_get_max_voltage(axp20x_batt,
+						      &val->intval);
+
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
+		ret = regmap_read(axp20x_batt->regmap, AXP20X_V_OFF, &reg);
+		if (ret)
+			return ret;
+
+		val->intval = 2600000 + 100000 * (reg & AXP20X_V_OFF_MASK);
+		break;
+
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		ret = iio_read_channel_processed(axp20x_batt->batt_v,
+						 &val->intval);
+		if (ret)
+			return ret;
+
+		/* IIO framework gives mV but Power Supply framework gives uV */
+		val->intval *= 1000;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int axp20x_battery_set_max_voltage(struct axp20x_batt_ps *axp20x_batt,
+					  int val)
+{
+	switch (val) {
+	case 4100000:
+		val = AXP20X_CHRG_CTRL1_TGT_4_1V;
+		break;
+
+	case 4150000:
+		if (axp20x_batt->axp_id == AXP221_ID)
+			return -EINVAL;
+
+		val = AXP20X_CHRG_CTRL1_TGT_4_15V;
+		break;
+
+	case 4200000:
+		val = AXP20X_CHRG_CTRL1_TGT_4_2V;
+		break;
+
+	default:
+		/*
+		 * AXP20x max voltage can be set to 4.36V and AXP22X max voltage
+		 * can be set to 4.22V and 4.24V, but these voltages are too
+		 * high for Lithium based batteries (AXP PMICs are supposed to
+		 * be used with these kinds of battery).
+		 */
+		return -EINVAL;
+	}
+
+	return regmap_update_bits(axp20x_batt->regmap, AXP20X_CHRG_CTRL1,
+				  AXP20X_CHRG_CTRL1_TGT_VOLT, val);
+}
+
+static int axp20x_set_constant_charge_current(struct axp20x_batt_ps *axp_batt,
+					      int charge_current)
+{
+	if (axp_batt->axp_id == AXP209_ID)
+		charge_current = (charge_current - 300000) / 100000;
+	else
+		charge_current = (charge_current - 300000) / 150000;
+
+	if (charge_current > AXP20X_CHRG_CTRL1_TGT_CURR || charge_current < 0)
+		return -EINVAL;
+
+	return regmap_update_bits(axp_batt->regmap, AXP20X_CHRG_CTRL1,
+				  AXP20X_CHRG_CTRL1_TGT_CURR, charge_current);
+}
+
+static int axp20x_battery_set_prop(struct power_supply *psy,
+				   enum power_supply_property psp,
+				   const union power_supply_propval *val)
+{
+	struct axp20x_batt_ps *axp20x_batt = power_supply_get_drvdata(psy);
+	int val1;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
+		val1 = (val->intval - 2600000) / 100000;
+		if (val1 < 0 || val1 > AXP20X_V_OFF_MASK)
+			return -EINVAL;
+
+		return regmap_update_bits(axp20x_batt->regmap, AXP20X_V_OFF,
+					  AXP20X_V_OFF_MASK, val1);
+
+	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
+		return axp20x_battery_set_max_voltage(axp20x_batt, val->intval);
+
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
+		return axp20x_set_constant_charge_current(axp20x_batt,
+							  val->intval);
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static enum power_supply_property axp20x_battery_props[] = {
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
+	POWER_SUPPLY_PROP_CAPACITY,
+};
+
+static int axp20x_battery_prop_writeable(struct power_supply *psy,
+					 enum power_supply_property psp)
+{
+	return psp == POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN ||
+	       psp == POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN ||
+	       psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT;
+}
+
+static const struct power_supply_desc axp20x_batt_ps_desc = {
+	.name = "axp20x-battery",
+	.type = POWER_SUPPLY_TYPE_BATTERY,
+	.properties = axp20x_battery_props,
+	.num_properties = ARRAY_SIZE(axp20x_battery_props),
+	.property_is_writeable = axp20x_battery_prop_writeable,
+	.get_property = axp20x_battery_get_prop,
+	.set_property = axp20x_battery_set_prop,
+};
+
+static const struct of_device_id axp20x_battery_ps_id[] = {
+	{
+		.compatible = "x-powers,axp209-battery-power-supply",
+		.data = (void *)AXP209_ID,
+	}, {
+		.compatible = "x-powers,axp221-battery-power-supply",
+		.data = (void *)AXP221_ID,
+	}, { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, axp20x_battery_ps_id);
+
+static int axp20x_power_probe(struct platform_device *pdev)
+{
+	struct axp20x_batt_ps *axp20x_batt;
+	struct power_supply_config psy_cfg = {};
+	int ret;
+	u32 const_charge_current;
+
+	if (!of_device_is_available(pdev->dev.of_node))
+		return -ENODEV;
+
+	axp20x_batt = devm_kzalloc(&pdev->dev, sizeof(*axp20x_batt),
+				   GFP_KERNEL);
+	if (!axp20x_batt)
+		return -ENOMEM;
+
+	axp20x_batt->batt_v = devm_iio_channel_get(&pdev->dev, "batt_v");
+	if (IS_ERR(axp20x_batt->batt_v)) {
+		if (PTR_ERR(axp20x_batt->batt_v) == -ENODEV)
+			return -EPROBE_DEFER;
+		return PTR_ERR(axp20x_batt->batt_v);
+	}
+
+	axp20x_batt->batt_chrg_i = devm_iio_channel_get(&pdev->dev,
+							"batt_chrg_i");
+	if (IS_ERR(axp20x_batt->batt_chrg_i)) {
+		if (PTR_ERR(axp20x_batt->batt_chrg_i) == -ENODEV)
+			return -EPROBE_DEFER;
+		return PTR_ERR(axp20x_batt->batt_chrg_i);
+	}
+
+	axp20x_batt->batt_dischrg_i = devm_iio_channel_get(&pdev->dev,
+							   "batt_dischrg_i");
+	if (IS_ERR(axp20x_batt->batt_dischrg_i)) {
+		if (PTR_ERR(axp20x_batt->batt_dischrg_i) == -ENODEV)
+			return -EPROBE_DEFER;
+		return PTR_ERR(axp20x_batt->batt_dischrg_i);
+	}
+
+	axp20x_batt->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	platform_set_drvdata(pdev, axp20x_batt);
+
+	psy_cfg.drv_data = axp20x_batt;
+	psy_cfg.of_node = pdev->dev.of_node;
+
+	axp20x_batt->axp_id = (uintptr_t)of_device_get_match_data(&pdev->dev);
+
+	if (!device_property_read_u32(&pdev->dev,
+				      "x-powers,constant-charge-current",
+				      &const_charge_current)) {
+		ret = axp20x_set_constant_charge_current(axp20x_batt,
+							 const_charge_current);
+		if (ret)
+			dev_err(&pdev->dev,
+				"couldn't set constant charge current: %d\n",
+				ret);
+	}
+
+	axp20x_batt->batt = devm_power_supply_register(&pdev->dev,
+						       &axp20x_batt_ps_desc,
+						       &psy_cfg);
+	return PTR_ERR_OR_ZERO(axp20x_batt->batt);
+}
+
+static struct platform_driver axp20x_batt_driver = {
+	.probe    = axp20x_power_probe,
+	.driver   = {
+		.name  = "axp20x-battery-power-supply",
+		.of_match_table = axp20x_battery_ps_id,
+	},
+};
+
+module_platform_driver(axp20x_batt_driver);
+
+MODULE_DESCRIPTION("Battery power supply driver for AXP20X and AXP22X PMICs");
+MODULE_AUTHOR("Quentin Schulz <quentin.schulz@free-electrons.com>");
+MODULE_LICENSE("GPL");
-- 
2.9.3

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

* [PATCH v2 21/25] mfd: axp20x: add MFD cells for AXP20X and AXP22X battery driver
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (19 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 20/25] power: supply: add battery driver for AXP20X and AXP22X PMICs Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-27  8:54 ` [PATCH v2 22/25] ARM: dtsi: axp209: add battery power supply subnode Quentin Schulz
                   ` (4 subsequent siblings)
  25 siblings, 0 replies; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The X-Powers AXP20X and AXP22X PMICs can have a battery as power supply.

This patch adds the AXP20X/AXP22X battery driver to the MFD cells of the
AXP209, AXP221 and AXP223 MFD.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/axp20x.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 223dffe..e79ff98 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -586,6 +586,9 @@ static struct mfd_cell axp20x_cells[] = {
 		.name		= "axp20x-adc",
 		.of_compatible	= "x-powers,axp209-adc",
 	}, {
+		.name		= "axp20x-battery-power-supply",
+		.of_compatible	= "x-powers,axp209-battery-power-supply",
+	}, {
 		.name		= "axp20x-ac-power-supply",
 		.of_compatible	= "x-powers,axp202-ac-power-supply",
 		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
@@ -614,6 +617,9 @@ static struct mfd_cell axp221_cells[] = {
 		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
 		.resources	= axp20x_ac_power_supply_resources,
 	}, {
+		.name		= "axp20x-battery-power-supply",
+		.of_compatible	= "x-powers,axp221-battery-power-supply",
+	}, {
 		.name		= "axp20x-usb-power-supply",
 		.of_compatible	= "x-powers,axp221-usb-power-supply",
 		.num_resources	= ARRAY_SIZE(axp22x_usb_power_supply_resources),
@@ -630,6 +636,9 @@ static struct mfd_cell axp223_cells[] = {
 		.name		= "axp20x-adc",
 		.of_compatible	= "x-powers,axp221-adc"
 	}, {
+		.name		= "axp20x-battery-power-supply",
+		.of_compatible	= "x-powers,axp221-battery-power-supply",
+	}, {
 		.name			= "axp20x-regulator",
 	}, {
 		.name		= "axp20x-ac-power-supply",
-- 
2.9.3

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

* [PATCH v2 22/25] ARM: dtsi: axp209: add battery power supply subnode
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (20 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 21/25] mfd: axp20x: add MFD cells for AXP20X and AXP22X battery driver Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-27  8:54 ` [PATCH v2 23/25] ARM: dtsi: axp22x: " Quentin Schulz
                   ` (3 subsequent siblings)
  25 siblings, 0 replies; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The X-Powers AXP209 PMIC exposes battery supply various data such as
the battery status (charging, discharging, full, dead), current max
limit, current current, battery capacity (in percentage), voltage max
and min limits, current voltage, and battery capacity (in Ah).

This adds the battery power supply subnode for AXP20X PMIC.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---

v2:
 - changed DT node name from battery_power_supply to
 battery-power-supply,
 - removed io-channels and io-channel-names from DT (the IIO mapping is
 done in the IIO ADC driver now),

 arch/arm/boot/dts/axp209.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
index 8675c696..51810aa 100644
--- a/arch/arm/boot/dts/axp209.dtsi
+++ b/arch/arm/boot/dts/axp209.dtsi
@@ -68,6 +68,11 @@
 		#gpio-cells = <2>;
 	};
 
+	battery_power_supply: battery-power-supply {
+		compatible = "x-powers,axp209-battery-power-supply";
+		status = "disabled";
+	};
+
 	regulators {
 		/* Default work frequency for buck regulators */
 		x-powers,dcdc-freq = <1500>;
-- 
2.9.3

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

* [PATCH v2 23/25] ARM: dtsi: axp22x: add battery power supply subnode
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (21 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 22/25] ARM: dtsi: axp209: add battery power supply subnode Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-27  8:54 ` [PATCH v2 24/25] ARM: dts: sun8i: sina33: enable " Quentin Schulz
                   ` (2 subsequent siblings)
  25 siblings, 0 replies; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The X-Powers AXP22X PMIC exposes battery supply various data such as
the battery status (charging, discharging, full, dead), current max
limit, current current, battery capacity (in percentage), voltage max
limit, current voltage, and battery capacity (in Ah).

This adds the battery power supply subnode for AXP22X PMIC.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---

v2:
 - changed DT node name from battery_power_supply to
 battery-power-supply,
 - removed io-channels and io-channel-names from DT (the IIO mapping is
 done in the IIO ADC driver now),

 arch/arm/boot/dts/axp22x.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/axp22x.dtsi b/arch/arm/boot/dts/axp22x.dtsi
index f2835b4..bcfab6e 100644
--- a/arch/arm/boot/dts/axp22x.dtsi
+++ b/arch/arm/boot/dts/axp22x.dtsi
@@ -61,6 +61,11 @@
 		compatible = "x-powers,axp221-adc";
 	};
 
+	battery_power_supply: battery-power-supply {
+		compatible = "x-powers,axp221-battery-power-supply";
+		status = "disabled";
+	};
+
 	regulators {
 		/* Default work frequency for buck regulators */
 		x-powers,dcdc-freq = <3000>;
-- 
2.9.3

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

* [PATCH v2 24/25] ARM: dts: sun8i: sina33: enable battery power supply subnode
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (22 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 23/25] ARM: dtsi: axp22x: " Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-27  8:54 ` [PATCH v2 25/25] ARM: sun5i: chip: " Quentin Schulz
  2017-01-27  9:19 ` [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Maxime Ripard
  25 siblings, 0 replies; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The Sinlinx SinA33 has an AXP223 PMIC and a battery connector, thus, we
enable the battery power supply subnode in its Device Tree.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
 arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts b/arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts
index bf53408..2fe9299 100644
--- a/arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts
+++ b/arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts
@@ -151,6 +151,10 @@
 	status = "okay";
 };
 
+&battery_power_supply {
+	status = "okay";
+};
+
 &reg_aldo1 {
 	regulator-always-on;
 	regulator-min-microvolt = <3000000>;
-- 
2.9.3

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

* [PATCH v2 25/25] ARM: sun5i: chip: enable battery power supply subnode
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (23 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 24/25] ARM: dts: sun8i: sina33: enable " Quentin Schulz
@ 2017-01-27  8:54 ` Quentin Schulz
  2017-01-27  9:19 ` [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Maxime Ripard
  25 siblings, 0 replies; 51+ messages in thread
From: Quentin Schulz @ 2017-01-27  8:54 UTC (permalink / raw)
  To: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, lee.jones
  Cc: Quentin Schulz, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

The NextThing Co. CHIP has an AXP209 PMIC with battery connector.

This enables the battery power supply subnode.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
 arch/arm/boot/dts/sun5i-r8-chip.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/sun5i-r8-chip.dts b/arch/arm/boot/dts/sun5i-r8-chip.dts
index 6011757..d4332b1 100644
--- a/arch/arm/boot/dts/sun5i-r8-chip.dts
+++ b/arch/arm/boot/dts/sun5i-r8-chip.dts
@@ -132,6 +132,10 @@
 	status = "okay";
 };
 
+&battery_power_supply {
+	status = "okay";
+};
+
 &i2c1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c1_pins_a>;
-- 
2.9.3

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

* Re: [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers
  2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
                   ` (24 preceding siblings ...)
  2017-01-27  8:54 ` [PATCH v2 25/25] ARM: sun5i: chip: " Quentin Schulz
@ 2017-01-27  9:19 ` Maxime Ripard
  25 siblings, 0 replies; 51+ messages in thread
From: Maxime Ripard @ 2017-01-27  9:19 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	lee.jones, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

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

On Fri, Jan 27, 2017 at 09:54:33AM +0100, Quentin Schulz wrote:
> The X-Powers AXP20X and AXP22X PMICs have multiple ADCs. They expose
> information and data of the various power supplies they support such as
> ACIN, battery and VBUS. For example, they expose the current battery
> voltage, charge or discharge, as well as ACIN and VBUS current voltages
> and currents, internal PMIC temperature and ADC on 2 different GPIOs
> when in the right mode (for the AXP209 only).
> 
> The ACIN power supply driver is added by this patch. The AXP20X and
> AXP22X can both read the status and the "usability" of the power supply
> but only the AXP209 will be able to tell the current current and voltage
> of the power supply by reading ADC channels. It is simply not supported
> by the AXP22X PMICs.
> 
> The battery power supply driver is also added by this patch. The AXP20X
> and AXP22X share most of their behaviour but have slight variations. The
> allowed target voltages for battery charging are not the same, the
> AXP22X PMIC are able to tell if the battery percentage computed by the
> PMIC is trustworthy and they have different formulas for computing max
> current for battery power supply. The driver is able to give the current
> voltage and current of the battery (be it charging or discharging), the
> maximal and minimal voltage and maximal current allowed for the battery,
> whether the battery is present and usable and its capacity. It will get
> the battery current current and voltage by reading the ADC channels. The
> PMIC allows maximal voltages (4.36V for AXP20X and 4.22V and 4.24V for
> AXP22X) that should not be used with Lithium-based batteries and since
> this PMIC is supposed to be used with Lithium-based batteries, they have
> been disabled. The values returned by the ADC driver are multipled by
> 1000 to scale from the mV returned by the ADC to the uV expected by the
> power supply framework.
> 
> This series of patch adds DT bindings for ACIN power supply, ADC and
> battery power supply drivers for AXP20X and AXP22X PMICs and their
> documentation. It also enables the supported power supplies for the
> Nextthing Co. CHIP and Sinlinx SinA33 boards.
> 
> The different drivers are also added to the MFD cells of the AXP20X and
> AXP22X cells and the writeable and volatile regs updated to work with
> the newly added drivers.
> 
> VBUS driver has intentionally not been modified to use the ADC channels
> because a DT binding already exists for this driver. Migrating the
> driver would mean to add an iio_map to map the ADC channels to the VBUS
> driver (so we can use iio_channel_get and iio_read_channel_processed
> functions). This slightly complexifies the VBUS driver only for
> "cosmetic" changes. Feel free to give your two cents on the matter.
> 
> This series of patch is based on a previous upstreaming attempt done by
> Bruno Prémont few months ago. It differs in three points: the ADC
> driver does not tell the battery temperature (TS_IN) as I do not have a
> board to test it with, it does not tell the instantaneous battery power
> as it returns crazy values for me and finally no support for OCV curves
> for the battery.
> 
> You can test these patches from this repo and branch:
> https://github.com/QSchulz/linux/tree/axp2xx_adc_batt_ac_v2

For the whole serie,
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Maxime

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

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

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

* Re: [PATCH v2 02/25] mfd: axp20x: correct name of temperature data ADC registers
  2017-01-27  8:54 ` [PATCH v2 02/25] mfd: axp20x: correct name of temperature data ADC registers Quentin Schulz
@ 2017-01-27  9:30   ` Chen-Yu Tsai
  2017-02-08  9:26   ` Lee Jones
  1 sibling, 0 replies; 51+ messages in thread
From: Chen-Yu Tsai @ 2017-01-27  9:30 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: knaack.h, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Rob Herring, Mark Rutland, Chen-Yu Tsai, Sebastian Reichel,
	Russell King, Maxime Ripard, Lee Jones, linux-iio, devicetree,
	linux-kernel, open list:THERMAL, linux-arm-kernel,
	Thomas Petazzoni, Icenowy Zheng, Bruno Prémont

On Fri, Jan 27, 2017 at 4:54 PM, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> The registers 0x56 and 0x57 of AXP22X PMIC store the value of the
> internal temperature of the PMIC.
>
> This patch modifies the name of these registers from AXP22X_PMIC_ADC_H/L
> to AXP22X_PMIC_TEMP_H/L so their purpose is clearer.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

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

* Re: [PATCH v2 03/25] power: supply: axp20x_usb_power: use IIO channels when available
  2017-01-27  8:54 ` [PATCH v2 03/25] power: supply: axp20x_usb_power: use IIO channels when available Quentin Schulz
@ 2017-01-28 14:36   ` Jonathan Cameron
  2017-01-29 16:21   ` Sebastian Reichel
  1 sibling, 0 replies; 51+ messages in thread
From: Jonathan Cameron @ 2017-01-28 14:36 UTC (permalink / raw)
  To: Quentin Schulz, knaack.h, lars, pmeerw, robh+dt, mark.rutland,
	wens, sre, linux, maxime.ripard, lee.jones
  Cc: linux-iio, devicetree, linux-kernel, linux-pm, linux-arm-kernel,
	thomas.petazzoni, icenowy, bonbons

On 27/01/17 08:54, Quentin Schulz wrote:
> The X-Powers AXP20X PMIC exposes the current current and voltage
> measures via an internal ADC.
> 
> This adds the possibility to read IIO channels directly for processed
> values rather than reading the registers and computing the value.
> 
> For backward compatibility purpose, if the IIO driver is not compiled,
> this driver will fall back on previous behaviour which is direct
> register readings.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
Acked-by: Jonathan Cameron <jic23@kernel.org>

I (or someone else) really needs to find time to deal with deferred probing
properly in the IIO core. It's a mess as we need to revisit how the
channel map stuff works to make sure it's always there...

Anyhow, it's still on the todo list and we can tidy this up if it ever
happens!

Jonathan
> ---
> 
> added in v2
> 
>  drivers/power/supply/axp20x_usb_power.c | 70 +++++++++++++++++++++++++++++++--
>  1 file changed, 66 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c
> index 1bcb025..d8b1dc6 100644
> --- a/drivers/power/supply/axp20x_usb_power.c
> +++ b/drivers/power/supply/axp20x_usb_power.c
> @@ -22,6 +22,7 @@
>  #include <linux/power_supply.h>
>  #include <linux/regmap.h>
>  #include <linux/slab.h>
> +#include <linux/iio/consumer.h>
>  
>  #define DRVNAME "axp20x-usb-power-supply"
>  
> @@ -49,6 +50,8 @@ struct axp20x_usb_power {
>  	struct regmap *regmap;
>  	struct power_supply *supply;
>  	int axp20x_id;
> +	struct iio_channel *vbus_v;
> +	struct iio_channel *vbus_i;
>  };
>  
>  static irqreturn_t axp20x_usb_power_irq(int irq, void *devid)
> @@ -76,6 +79,20 @@ static int axp20x_usb_power_get_property(struct power_supply *psy,
>  		val->intval = AXP20X_VBUS_VHOLD_uV(v);
>  		return 0;
>  	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		if (IS_ENABLED(CONFIG_AXP20X_ADC)) {
> +			ret = iio_read_channel_processed(power->vbus_v,
> +							 &val->intval);
> +			if (ret)
> +				return ret;
> +
> +			/*
> +			 * IIO framework gives mV but Power Supply framework
> +			 * gives uV.
> +			 */
> +			val->intval *= 1000;
> +			return 0;
> +		}
> +
>  		ret = axp20x_read_variable_width(power->regmap,
>  						 AXP20X_VBUS_V_ADC_H, 12);
>  		if (ret < 0)
> @@ -107,6 +124,20 @@ static int axp20x_usb_power_get_property(struct power_supply *psy,
>  		}
>  		return 0;
>  	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		if (IS_ENABLED(CONFIG_AXP20X_ADC)) {
> +			ret = iio_read_channel_processed(power->vbus_i,
> +							 &val->intval);
> +			if (ret)
> +				return ret;
> +
> +			/*
> +			 * IIO framework gives mA but Power Supply framework
> +			 * gives uA.
> +			 */
> +			val->intval *= 1000;
> +			return 0;
> +		}
> +
>  		ret = axp20x_read_variable_width(power->regmap,
>  						 AXP20X_VBUS_I_ADC_H, 12);
>  		if (ret < 0)
> @@ -269,6 +300,36 @@ static const struct power_supply_desc axp22x_usb_power_desc = {
>  	.set_property = axp20x_usb_power_set_property,
>  };
>  
> +static int configure_iio_channels(struct platform_device *pdev,
> +				  struct axp20x_usb_power *power)
> +{
> +	power->vbus_v = devm_iio_channel_get(&pdev->dev, "vbus_v");
> +	if (IS_ERR(power->vbus_v)) {
> +		if (PTR_ERR(power->vbus_v) == -ENODEV)
> +			return -EPROBE_DEFER;
> +		return PTR_ERR(power->vbus_v);
> +	}
> +
> +	power->vbus_i = devm_iio_channel_get(&pdev->dev, "vbus_i");
> +	if (IS_ERR(power->vbus_i)) {
> +		if (PTR_ERR(power->vbus_i) == -ENODEV)
> +			return -EPROBE_DEFER;
> +		return PTR_ERR(power->vbus_i);
> +	}
> +
> +	return 0;
> +}
> +
> +static int configure_adc_registers(struct axp20x_usb_power *power)
> +{
> +	/* Enable vbus voltage and current measurement */
> +	return regmap_update_bits(power->regmap, AXP20X_ADC_EN1,
> +				  AXP20X_ADC_EN1_VBUS_CURR |
> +				  AXP20X_ADC_EN1_VBUS_VOLT,
> +				  AXP20X_ADC_EN1_VBUS_CURR |
> +				  AXP20X_ADC_EN1_VBUS_VOLT);
> +}
> +
>  static int axp20x_usb_power_probe(struct platform_device *pdev)
>  {
>  	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
> @@ -307,10 +368,11 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
>  		if (ret)
>  			return ret;
>  
> -		/* Enable vbus voltage and current measurement */
> -		ret = regmap_update_bits(power->regmap, AXP20X_ADC_EN1,
> -			AXP20X_ADC_EN1_VBUS_CURR | AXP20X_ADC_EN1_VBUS_VOLT,
> -			AXP20X_ADC_EN1_VBUS_CURR | AXP20X_ADC_EN1_VBUS_VOLT);
> +		if (IS_ENABLED(CONFIG_AXP20X_ADC))
> +			ret = configure_iio_channels(pdev, power);
> +		else
> +			ret = configure_adc_registers(power);
> +
>  		if (ret)
>  			return ret;
>  
> 

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

* Re: [PATCH v2 04/25] iio: adc: add support for X-Powers AXP20X and AXP22X PMICs ADCs
  2017-01-27  8:54 ` [PATCH v2 04/25] iio: adc: add support for X-Powers AXP20X and AXP22X PMICs ADCs Quentin Schulz
@ 2017-01-28 14:49   ` Jonathan Cameron
  2017-01-28 15:12     ` Quentin Schulz
  0 siblings, 1 reply; 51+ messages in thread
From: Jonathan Cameron @ 2017-01-28 14:49 UTC (permalink / raw)
  To: Quentin Schulz, knaack.h, lars, pmeerw, robh+dt, mark.rutland,
	wens, sre, linux, maxime.ripard, lee.jones
  Cc: linux-iio, devicetree, linux-kernel, linux-pm, linux-arm-kernel,
	thomas.petazzoni, icenowy, bonbons

On 27/01/17 08:54, Quentin Schulz wrote:
> The X-Powers AXP20X and AXP22X PMICs have multiple ADCs. They expose the
> battery voltage, battery charge and discharge currents, AC-in and VBUS
> voltages and currents, 2 GPIOs muxable in ADC mode and PMIC temperature.
> 
> This adds support for most of AXP20X and AXP22X ADCs.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
Pretty good, but not everything seems to be cleaned up on error paths
in probe.

A few other suggestions / questions inline.

Jonathan
> ---
> 
> v2:
>  - removed unused defines,
>  - changed BIT(x) to 1 << x when describing bits purpose for which 2 <<
>  x or 3 << x exists, to be consistent,
>  - changed ADC rate defines to macro formulas,
>  - reordered IIO channels, now different measures (current/voltage) of
>  the same part of the PMIC (e.g. battery), have the same IIO channel in
>  their respective IIO type. When a part of the PMIC have only one
>  measure, a number is jumped,
>  - left IIO channel mapping in DT to use iio_map structure,
>  - removed indexing of ADC internal temperature,
>  - removed unused iio_dev structure in axp20x_adc_iio,
>  - added a structure for data specific to AXP20X or AXP22X PMICs instead
>  of using an ID and an if condition when needing to separate the
>  behaviour of both,
>  - added a comment on batt_chrg_i really being on 12bits rather than
>  what the Chinese datasheets say (13 bits),
>  - corrected the offset for AXP22X PMIC temperature,
>  - set the ADC rate to a value (100Hz) shared by the AXP20X and AXP22X,
>  - created macro formulas to compute the ADC rate for each,
>  - added a condition on presence of ADC_EN2 reg before setting/resetting
>  it,
>  - switched from devm_iio_device_unregister to the non-devm function
>  because of the need for a remove function,
>  - removed some dead code,
> 
>  drivers/iio/adc/Kconfig      |  10 +
>  drivers/iio/adc/Makefile     |   1 +
>  drivers/iio/adc/axp20x_adc.c | 572 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 583 insertions(+)
>  create mode 100644 drivers/iio/adc/axp20x_adc.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 9c8b558..ed17fe1 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -154,6 +154,16 @@ config AT91_SAMA5D2_ADC
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called at91-sama5d2_adc.
>  
> +config AXP20X_ADC
> +	tristate "X-Powers AXP20X and AXP22X ADC driver"
> +	depends on MFD_AXP20X
> +	help
> +	  Say yes here to have support for X-Powers power management IC (PMIC)
> +	  AXP20X and AXP22X ADC devices.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called axp20x_adc.
> +
>  config AXP288_ADC
>  	tristate "X-Powers AXP288 ADC driver"
>  	depends on MFD_AXP20X
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index d36c4be..f5c28a5 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_AD7887) += ad7887.o
>  obj-$(CONFIG_AD799X) += ad799x.o
>  obj-$(CONFIG_AT91_ADC) += at91_adc.o
>  obj-$(CONFIG_AT91_SAMA5D2_ADC) += at91-sama5d2_adc.o
> +obj-$(CONFIG_AXP20X_ADC) += axp20x_adc.o
>  obj-$(CONFIG_AXP288_ADC) += axp288_adc.o
>  obj-$(CONFIG_BCM_IPROC_ADC) += bcm_iproc_adc.o
>  obj-$(CONFIG_BERLIN2_ADC) += berlin2-adc.o
> diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
> new file mode 100644
> index 0000000..bacde92
> --- /dev/null
> +++ b/drivers/iio/adc/axp20x_adc.c
> @@ -0,0 +1,572 @@
> +/* ADC driver for AXP20X and AXP22X PMICs
> + *
> + * Copyright (c) 2016 Free Electrons NextThing Co.
> + *	Quentin Schulz <quentin.schulz@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it under
> + * the terms of the GNU General Public License version 2 as published by the
> + * Free Software Foundation.
> + */
> +
> +#include <linux/completion.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/thermal.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/driver.h>
> +#include <linux/iio/machine.h>
> +#include <linux/mfd/axp20x.h>
> +
> +#define AXP20X_ADC_EN1_MASK			GENMASK(7, 0)
> +
> +#define AXP20X_ADC_EN2_MASK			(GENMASK(3, 2) | BIT(7))
> +#define AXP22X_ADC_EN1_MASK			(GENMASK(7, 5) | BIT(0))
> +
> +#define AXP20X_GPIO10_IN_RANGE_GPIO0		BIT(0)
> +#define AXP20X_GPIO10_IN_RANGE_GPIO1		BIT(1)
> +#define AXP20X_GPIO10_IN_RANGE_GPIO0_VAL(x)	((x) & BIT(0))
> +#define AXP20X_GPIO10_IN_RANGE_GPIO1_VAL(x)	(((x) & BIT(0)) << 1)
> +
> +#define AXP20X_ADC_RATE_MASK			GENMASK(7, 6)
> +#define AXP20X_ADC_RATE_HZ(x)			((ilog2((x) / 25) << 6) & AXP20X_ADC_RATE_MASK)
> +#define AXP22X_ADC_RATE_HZ(x)			((ilog2((x) / 100) << 6) & AXP20X_ADC_RATE_MASK)
> +
> +#define AXP20X_ADC_CHANNEL(_channel, _name, _type, _reg)	\
> +	{							\
> +		.type = _type,					\
> +		.indexed = 1,					\
> +		.channel = _channel,				\
> +		.address = _reg,				\
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
> +				      BIT(IIO_CHAN_INFO_SCALE),	\
> +		.datasheet_name = _name,			\
> +	}
> +
> +#define AXP20X_ADC_CHANNEL_OFFSET(_channel, _name, _type, _reg) \
> +	{							\
> +		.type = _type,					\
> +		.indexed = 1,					\
> +		.channel = _channel,				\
> +		.address = _reg,				\
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
> +				      BIT(IIO_CHAN_INFO_SCALE) |\
> +				      BIT(IIO_CHAN_INFO_OFFSET),\
> +		.datasheet_name = _name,			\
> +	}
> +
> +struct axp_data;
> +
> +struct axp20x_adc_iio {
> +	struct regmap		*regmap;
> +	struct axp_data		*data;
> +};
> +
> +enum axp20x_adc_channel_v {
> +	AXP20X_ACIN_V = 0,
> +	AXP20X_VBUS_V,
> +	AXP20X_TS_IN,
> +	AXP20X_GPIO0_V,
> +	AXP20X_GPIO1_V,
> +	AXP20X_IPSOUT_V,
> +	AXP20X_BATT_V,
> +};
> +
> +enum axp20x_adc_channel_i {
> +	AXP20X_ACIN_I = 0,
> +	AXP20X_VBUS_I,
> +	AXP20X_BATT_CHRG_I,
> +	AXP20X_BATT_DISCHRG_I,
> +};
> +
> +enum axp22x_adc_channel_v {
> +	AXP22X_TS_IN = 0,
> +	AXP22X_BATT_V,
> +};
> +
> +enum axp22x_adc_channel_i {
> +	AXP22X_BATT_CHRG_I = 1,
> +	AXP22X_BATT_DISCHRG_I,
> +};
> +
> +static struct iio_map axp20x_maps[] = {
> +	{
> +		.consumer_dev_name = "axp20x-usb-power-supply",
> +		.consumer_channel = "vbus_v",
> +		.adc_channel_label = "vbus_v",
> +	}, {
> +		.consumer_dev_name = "axp20x-usb-power-supply",
> +		.consumer_channel = "vbus_i",
> +		.adc_channel_label = "vbus_i",
> +	}, { /* sentinel */ }
> +};
> +
> +/*
> + * 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
> + * charge current in_current6_raw and discharge current will be in_current7_raw.
> + */
> +static const struct iio_chan_spec axp20x_adc_channels[] = {
> +	AXP20X_ADC_CHANNEL(AXP20X_ACIN_V, "acin_v", IIO_VOLTAGE,
> +			   AXP20X_ACIN_V_ADC_H),
> +	AXP20X_ADC_CHANNEL(AXP20X_ACIN_I, "acin_i", IIO_CURRENT,
> +			   AXP20X_ACIN_I_ADC_H),
> +	AXP20X_ADC_CHANNEL(AXP20X_VBUS_V, "vbus_v", IIO_VOLTAGE,
> +			   AXP20X_VBUS_V_ADC_H),
> +	AXP20X_ADC_CHANNEL(AXP20X_VBUS_I, "vbus_i", IIO_CURRENT,
> +			   AXP20X_VBUS_I_ADC_H),
> +	{
> +		.type = IIO_TEMP,
> +		.address = AXP20X_TEMP_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_OFFSET(AXP20X_GPIO0_V, "gpio0_v", IIO_VOLTAGE,
> +				  AXP20X_GPIO0_V_ADC_H),
> +	AXP20X_ADC_CHANNEL_OFFSET(AXP20X_GPIO1_V, "gpio1_v", IIO_VOLTAGE,
> +				  AXP20X_GPIO1_V_ADC_H),
> +	AXP20X_ADC_CHANNEL(AXP20X_IPSOUT_V, "ipsout_v", IIO_VOLTAGE,
> +			   AXP20X_IPSOUT_V_HIGH_H),
> +	AXP20X_ADC_CHANNEL(AXP20X_BATT_V, "batt_v", IIO_VOLTAGE,
> +			   AXP20X_BATT_V_H),
> +	AXP20X_ADC_CHANNEL(AXP20X_BATT_CHRG_I, "batt_chrg_i", IIO_CURRENT,
> +			   AXP20X_BATT_CHRG_I_H),
> +	AXP20X_ADC_CHANNEL(AXP20X_BATT_DISCHRG_I, "batt_dischrg_i", IIO_CURRENT,
> +			   AXP20X_BATT_DISCHRG_I_H),
> +};
> +
> +static const struct iio_chan_spec axp22x_adc_channels[] = {
> +	{
> +		.type = IIO_TEMP,
> +		.address = AXP22X_PMIC_TEMP_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(AXP22X_BATT_V, "batt_v", IIO_VOLTAGE,
> +			   AXP20X_BATT_V_H),
> +	AXP20X_ADC_CHANNEL(AXP22X_BATT_CHRG_I, "batt_chrg_i", IIO_CURRENT,
> +			   AXP20X_BATT_CHRG_I_H),
> +	AXP20X_ADC_CHANNEL(AXP22X_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)
> +{
> +	struct axp20x_adc_iio *info = iio_priv(indio_dev);
> +	int size = 12;
> +
> +	switch (chan->type) {
> +	case IIO_CURRENT:
> +		/*
> +		 * Unlike the Chinese datasheets tell, the charging current is
> +		 * stored on 12 bits, not 13 bits.
> +		 */
> +		if (chan->channel == AXP20X_BATT_DISCHRG_I)
> +			size = 13;
Given I don't think you can get here without it being current, voltage or temp;
couldn't this be done more cleanly with
if ((chan->type == IIO_CURRENT) && (chan->channel == AXP20X_BAT_DISCHRG_I))
   size = 13;

and have the rest in the normal code flow?

> +	case IIO_VOLTAGE:
> +	case IIO_TEMP:
> +		*val = axp20x_read_variable_width(info->regmap, chan->address,
> +						  size);
> +		if (*val < 0)
> +			return *val;
> +
> +		return IIO_VAL_INT;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int axp22x_adc_raw(struct iio_dev *indio_dev,
> +			  struct iio_chan_spec const *chan, int *val)
> +{
> +	struct axp20x_adc_iio *info = iio_priv(indio_dev);
> +	int size = 12;
> +
> +	switch (chan->type) {
> +	case IIO_CURRENT:
> +		/*
> +		 * Unlike the Chinese datasheets tell, the charging current is
> +		 * stored on 12 bits, not 13 bits.
> +		 */
> +		if (chan->channel == AXP22X_BATT_DISCHRG_I)
> +			size = 13;
> +	case IIO_VOLTAGE:
> +	case IIO_TEMP:
> +		*val = axp20x_read_variable_width(info->regmap, chan->address,
> +						  size);
> +		if (*val < 0)
> +			return *val;
> +
> +		return IIO_VAL_INT;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int axp20x_adc_scale_voltage(int channel, int *val, int *val2)
> +{
> +	switch (channel) {
> +	case AXP20X_ACIN_V:
> +	case AXP20X_VBUS_V:
> +		*val = 1;
> +		*val2 = 700000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	case AXP20X_GPIO0_V:
> +	case AXP20X_GPIO1_V:
> +		*val = 0;
> +		*val2 = 500000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	case AXP20X_BATT_V:
> +		*val = 1;
> +		*val2 = 100000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	case AXP20X_IPSOUT_V:
> +		*val = 1;
> +		*val2 = 400000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int axp20x_adc_scale_current(int channel, int *val, int *val2)
> +{
> +	switch (channel) {
> +	case AXP20X_ACIN_I:
> +		*val = 0;
> +		*val2 = 625000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	case AXP20X_VBUS_I:
> +		*val = 0;
> +		*val2 = 375000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	case AXP20X_BATT_DISCHRG_I:
> +	case AXP20X_BATT_CHRG_I:
> +		*val = 0;
> +		*val2 = 500000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int axp20x_adc_scale(struct iio_chan_spec const *chan, int *val,
> +			    int *val2)
> +{
> +	switch (chan->type) {
> +	case IIO_VOLTAGE:
> +		return axp20x_adc_scale_voltage(chan->channel, val, val2);
> +
> +	case IIO_CURRENT:
> +		return axp20x_adc_scale_current(chan->channel, val, val2);
> +
> +	case IIO_TEMP:
> +		*val = 100;
> +		return IIO_VAL_INT;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int axp22x_adc_scale(struct iio_chan_spec const *chan, int *val,
> +			    int *val2)
> +{
> +	switch (chan->type) {
> +	case IIO_VOLTAGE:
> +		if (chan->channel != AXP22X_BATT_V)
> +			return -EINVAL;
> +
> +		*val = 1;
> +		*val2 = 100000;
> +		return IIO_VAL_INT_PLUS_MICRO;
A fixed scale of 1.1x? (just checking)
> +
> +	case IIO_CURRENT:
> +		*val = 0;
> +		*val2 = 500000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	case IIO_TEMP:
> +		*val = 100;
> +		return IIO_VAL_INT;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int axp20x_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
> +				     int *val)
> +{
> +	struct axp20x_adc_iio *info = iio_priv(indio_dev);
> +	int ret, reg;
> +
> +	if (channel != AXP20X_GPIO0_V && channel != AXP20X_GPIO1_V)
> +		return -EINVAL;
> +
> +	ret = regmap_read(info->regmap, AXP20X_GPIO10_IN_RANGE, &reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (channel == AXP20X_GPIO0_V)
> +		*val = reg & AXP20X_GPIO10_IN_RANGE_GPIO0;
> +	else
> +		*val = reg & AXP20X_GPIO10_IN_RANGE_GPIO1;
> +
> +	*val = !!(*val) * 700000;
> +
> +	return IIO_VAL_INT;
> +}
> +
> +static int axp20x_adc_offset(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan, int *val)
> +{
> +	switch (chan->type) {
> +	case IIO_VOLTAGE:
> +		return axp20x_adc_offset_voltage(indio_dev, chan->channel, val);
> +
> +	case IIO_TEMP:
> +		*val = -1447;
> +		return IIO_VAL_INT;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int axp20x_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:
> +		return axp20x_adc_offset(indio_dev, chan, val);
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		return axp20x_adc_scale(chan, val, val2);
> +
> +	case IIO_CHAN_INFO_RAW:
> +		return axp20x_adc_raw(indio_dev, chan, val);
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int axp22x_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 = -2677;
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		return axp22x_adc_scale(chan, val, val2);
> +
> +	case IIO_CHAN_INFO_RAW:
> +		return axp22x_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)
> +{
> +	struct axp20x_adc_iio *info = iio_priv(indio_dev);
> +
> +	/*
> +	 * The AXP20X PMIC allows the user to choose between 0V and 0.7V offsets
> +	 * for (independently) GPIO0 and GPIO1 when in ADC mode.
> +	 */
> +	if (mask != IIO_CHAN_INFO_OFFSET)
> +		return -EINVAL;
> +
> +	if (chan->channel != AXP20X_GPIO0_V && chan->channel != AXP20X_GPIO1_V)
> +		return -EINVAL;
> +
> +	if (val != 0 && val != 700000)
> +		return -EINVAL;
> +
> +	if (chan->channel == AXP20X_GPIO0_V)
> +		return regmap_update_bits(info->regmap, AXP20X_GPIO10_IN_RANGE,
> +					  AXP20X_GPIO10_IN_RANGE_GPIO0,
> +					  AXP20X_GPIO10_IN_RANGE_GPIO0_VAL(!!val));
> +
> +	return regmap_update_bits(info->regmap, AXP20X_GPIO10_IN_RANGE,
> +				  AXP20X_GPIO10_IN_RANGE_GPIO1,
> +				  AXP20X_GPIO10_IN_RANGE_GPIO1_VAL(!!val));
> +}
> +
> +static const struct iio_info axp20x_adc_iio_info = {
> +	.read_raw = axp20x_read_raw,
> +	.write_raw = axp20x_write_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static const struct iio_info axp22x_adc_iio_info = {
> +	.read_raw = axp22x_read_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static int axp20x_adc_rate(int rate)
> +{
> +	return AXP20X_ADC_RATE_HZ(rate);
> +}
> +
> +static int axp22x_adc_rate(int rate)
> +{
> +	return AXP22X_ADC_RATE_HZ(rate);
> +}
> +
> +struct axp_data {
> +	const struct iio_info		*iio_info;
> +	int				num_channels;
> +	struct iio_chan_spec const	*channels;
> +	unsigned long			adc_en1_mask;
> +	int				(*adc_rate)(int rate);
> +	bool				adc_en2;
> +	struct iio_map			*maps;
> +};
> +
> +static const struct axp_data axp20x_data = {
> +	.iio_info = &axp20x_adc_iio_info,
> +	.num_channels = ARRAY_SIZE(axp20x_adc_channels),
> +	.channels = axp20x_adc_channels,
> +	.adc_en1_mask = AXP20X_ADC_EN1_MASK,
> +	.adc_rate = axp20x_adc_rate,
> +	.adc_en2 = true,
> +	.maps = axp20x_maps,
> +};
> +
> +static const struct axp_data axp22x_data = {
> +	.iio_info = &axp22x_adc_iio_info,
> +	.num_channels = ARRAY_SIZE(axp22x_adc_channels),
> +	.channels = axp22x_adc_channels,
> +	.adc_en1_mask = AXP22X_ADC_EN1_MASK,
> +	.adc_rate = axp22x_adc_rate,
> +	.adc_en2 = false,
> +};
> +
> +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, },
> +	{ /* sentinel */ },
> +};
> +
> +static int axp20x_probe(struct platform_device *pdev)
> +{
> +	struct axp20x_adc_iio *info;
> +	struct iio_dev *indio_dev;
> +	struct axp20x_dev *axp20x_dev;
> +	int ret;
> +
> +	axp20x_dev = dev_get_drvdata(pdev->dev.parent);
> +
> +	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	info = iio_priv(indio_dev);
> +	platform_set_drvdata(pdev, indio_dev);
> +
> +	info->regmap = axp20x_dev->regmap;
> +	indio_dev->name = dev_name(&pdev->dev);
Not sure on this name - what does end up as?  Expected to be
a description of the part so in this case something like axp209-adc.
I've been lax at picking up on this in the past and it's led to some
crazy naming that is no use at all to userspace.  Basically this
name just provides a convenient user readable name for userspace apps to
use.

> +	indio_dev->dev.parent = &pdev->dev;
> +	indio_dev->dev.of_node = pdev->dev.of_node;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	info->data = (struct axp_data *)of_device_get_match_data(&pdev->dev);
> +
> +	indio_dev->info = info->data->iio_info;
> +	indio_dev->num_channels = info->data->num_channels;
> +	indio_dev->channels = info->data->channels;
> +
> +	/* Enable the ADCs on IP */
> +	regmap_write(info->regmap, AXP20X_ADC_EN1, info->data->adc_en1_mask);
> +
> +	if (info->data->adc_en2)
> +		/* Enable GPIO0/1 and internal temperature ADCs */
> +		regmap_update_bits(info->regmap, AXP20X_ADC_EN2,
> +				   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));
> +
> +	ret = iio_map_array_register(indio_dev, info->data->maps);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to register IIO maps: %d\n", ret);
This should be disabling channels.
> +		return ret;
> +	}
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "could not register the device\n");
> +		regmap_write(info->regmap, AXP20X_ADC_EN1, 0);
> +
> +		if (info->data->adc_en2)
> +			regmap_write(info->regmap, AXP20X_ADC_EN2, 0);
I'd expect to see a complete unwind of what has been done earlier in probe
including iio_map_array_unregister.

The traditional goto error* approach is probably worth having here to
make sure the unwind makes sense.
> +
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int axp20x_remove(struct platform_device *pdev)
> +{
> +	struct axp20x_adc_iio *info;
> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +
> +	info = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
iio_map_array_unregister?
> +
> +	regmap_write(info->regmap, AXP20X_ADC_EN1, 0);
> +
> +	if (info->data->adc_en2)
> +		regmap_write(info->regmap, AXP20X_ADC_EN2, 0);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver axp20x_adc_driver = {
> +	.driver = {
> +		.name = "axp20x-adc",
> +		.of_match_table = axp20x_adc_of_match,
> +	},
> +	.probe = axp20x_probe,
> +	.remove = axp20x_remove,
> +};
> +
> +module_platform_driver(axp20x_adc_driver);
> +
> +MODULE_DESCRIPTION("ADC driver for AXP20X and AXP22X PMICs");
> +MODULE_AUTHOR("Quentin Schulz <quentin.schulz@free-electrons.com>");
> +MODULE_LICENSE("GPL");
> 

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

* Re: [PATCH v2 09/25] iio: adc: axp20x_adc: map acin_i and acin_v
  2017-01-27  8:54 ` [PATCH v2 09/25] iio: adc: axp20x_adc: map acin_i and acin_v Quentin Schulz
@ 2017-01-28 14:51   ` Jonathan Cameron
  2017-01-29 16:31   ` Sebastian Reichel
  1 sibling, 0 replies; 51+ messages in thread
From: Jonathan Cameron @ 2017-01-28 14:51 UTC (permalink / raw)
  To: Quentin Schulz, knaack.h, lars, pmeerw, robh+dt, mark.rutland,
	wens, sre, linux, maxime.ripard, lee.jones
  Cc: linux-iio, devicetree, linux-kernel, linux-pm, linux-arm-kernel,
	thomas.petazzoni, icenowy, bonbons

On 27/01/17 08:54, Quentin Schulz wrote:
> This maps the IIO channels acin_i and acin_v (respectively exposing the
> current current and voltage measures of the AC power supply) to the AC
> power supply driver.
> 
> Only the AXP20X PMICs have these ADC channels and thus they are only
> mapped for this version of the PMIC.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
> 
> added in v2
> 
>  drivers/iio/adc/axp20x_adc.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
> index bacde92..14f4ec0 100644
> --- a/drivers/iio/adc/axp20x_adc.c
> +++ b/drivers/iio/adc/axp20x_adc.c
> @@ -104,6 +104,14 @@ static struct iio_map axp20x_maps[] = {
>  		.consumer_dev_name = "axp20x-usb-power-supply",
>  		.consumer_channel = "vbus_i",
>  		.adc_channel_label = "vbus_i",
> +	}, {
> +		.consumer_dev_name = "axp20x-ac-power-supply",
> +		.consumer_channel = "acin_v",
> +		.adc_channel_label = "acin_v",
> +	}, {
> +		.consumer_dev_name = "axp20x-ac-power-supply",
> +		.consumer_channel = "acin_i",
> +		.adc_channel_label = "acin_i",
>  	}, { /* sentinel */ }
>  };
>  
> 

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

* Re: [PATCH v2 19/25] iio: adc: axp20x_adc: map battery IIO channels
  2017-01-27  8:54 ` [PATCH v2 19/25] iio: adc: axp20x_adc: map battery IIO channels Quentin Schulz
@ 2017-01-28 14:52   ` Jonathan Cameron
  2017-01-31  5:06     ` Chen-Yu Tsai
  0 siblings, 1 reply; 51+ messages in thread
From: Jonathan Cameron @ 2017-01-28 14:52 UTC (permalink / raw)
  To: Quentin Schulz, knaack.h, lars, pmeerw, robh+dt, mark.rutland,
	wens, sre, linux, maxime.ripard, lee.jones
  Cc: linux-iio, devicetree, linux-kernel, linux-pm, linux-arm-kernel,
	thomas.petazzoni, icenowy, bonbons

On 27/01/17 08:54, Quentin Schulz wrote:
> This maps the IIO channels batt_v, batt_chrg_i and batt_dischrg_i
> (respectively exposing the current charging and discharging currents and
> current voltage measures of the battery power supply) to the battery
> power supply driver.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
> 
> added in v2
> 
>  drivers/iio/adc/axp20x_adc.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
> index 14f4ec0..64c7c75 100644
> --- a/drivers/iio/adc/axp20x_adc.c
> +++ b/drivers/iio/adc/axp20x_adc.c
> @@ -112,6 +112,34 @@ static struct iio_map axp20x_maps[] = {
>  		.consumer_dev_name = "axp20x-ac-power-supply",
>  		.consumer_channel = "acin_i",
>  		.adc_channel_label = "acin_i",
> +	}, {
> +		.consumer_dev_name = "axp20x-battery-power-supply",
> +		.consumer_channel = "batt_v",
> +		.adc_channel_label = "batt_v",
> +	}, {
> +		.consumer_dev_name = "axp20x-battery-power-supply",
> +		.consumer_channel = "batt_chrg_i",
> +		.adc_channel_label = "batt_chrg_i",
> +	}, {
> +		.consumer_dev_name = "axp20x-battery-power-supply",
> +		.consumer_channel = "batt_dischrg_i",
> +		.adc_channel_label = "batt_dischrg_i",
> +	}, { /* sentinel */ }
> +};
> +
> +static struct iio_map axp22x_maps[] = {
> +	{
> +		.consumer_dev_name = "axp20x-battery-power-supply",
> +		.consumer_channel = "batt_v",
> +		.adc_channel_label = "batt_v",
> +	}, {
> +		.consumer_dev_name = "axp20x-battery-power-supply",
> +		.consumer_channel = "batt_chrg_i",
> +		.adc_channel_label = "batt_chrg_i",
> +	}, {
> +		.consumer_dev_name = "axp20x-battery-power-supply",
> +		.consumer_channel = "batt_dischrg_i",
> +		.adc_channel_label = "batt_dischrg_i",
>  	}, { /* sentinel */ }
>  };
>  
> @@ -479,6 +507,7 @@ static const struct axp_data axp22x_data = {
>  	.adc_en1_mask = AXP22X_ADC_EN1_MASK,
>  	.adc_rate = axp22x_adc_rate,
>  	.adc_en2 = false,
> +	.maps = axp22x_maps,
>  };
>  
>  static const struct of_device_id axp20x_adc_of_match[] = {
> 

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

* Re: [PATCH v2 20/25] power: supply: add battery driver for AXP20X and AXP22X PMICs
  2017-01-27  8:54 ` [PATCH v2 20/25] power: supply: add battery driver for AXP20X and AXP22X PMICs Quentin Schulz
@ 2017-01-28 14:54   ` Jonathan Cameron
  0 siblings, 0 replies; 51+ messages in thread
From: Jonathan Cameron @ 2017-01-28 14:54 UTC (permalink / raw)
  To: Quentin Schulz, knaack.h, lars, pmeerw, robh+dt, mark.rutland,
	wens, sre, linux, maxime.ripard, lee.jones
  Cc: linux-iio, devicetree, linux-kernel, linux-pm, linux-arm-kernel,
	thomas.petazzoni, icenowy, bonbons

On 27/01/17 08:54, Quentin Schulz wrote:
> The X-Powers AXP20X and AXP22X PMICs can have a battery as power supply.
> 
> This patch adds the battery power supply driver to get various data from
> the PMIC, such as the battery status (charging, discharging, full,
> dead), current max limit, current current, battery capacity (in
> percentage), voltage max and min limits, current voltage and battery
> capacity (in Ah).
> 
> This battery driver uses the AXP20X/AXP22X ADC driver as PMIC data
> provider.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
For IIO bits
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
> 
> v2:
>  - changed BIT(x) to 1 << x when describing bits purpose for which 2 <<
>  x or 3 << x exists, to be consistent,
>  - switched from POWER_SUPPLY_PROP_CURRENT_MAX to
>  POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
>  - added POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX to the list of
>  readable properties,
>  - replaced µ character by a common u for micro units to make checkpatch
>  happy,
>  - factorized code in axp20x_battery_set_max_voltage,
>  - added a axp20x_set_constant_charge_current function to be used when
>  setting the value from sysfs and from the DT,
>  - removed some dead code,
>  - added a DT property to set constant current charge of the battery
>  (x-powers,constant-charge-current),
>  - migrated to dev_get_regmap instead of manually looking for the regmap
>  in the drvdata of the parent,
>  - switched from int to uintptr_t cast to make sure the cast is always
>  for the same size type (make build on 64bits platforms happy mainly),
> 
>  drivers/power/supply/Kconfig          |  12 +
>  drivers/power/supply/Makefile         |   1 +
>  drivers/power/supply/axp20x_battery.c | 477 ++++++++++++++++++++++++++++++++++
>  3 files changed, 490 insertions(+)
>  create mode 100644 drivers/power/supply/axp20x_battery.c
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index c552b4b..48619de 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -226,6 +226,18 @@ config CHARGER_AXP20X
>  	  This driver can also be built as a module. If so, the module will be
>  	  called axp20x_ac_power.
>  
> +config BATTERY_AXP20X
> +	tristate "X-Powers AXP20X battery driver"
> +	depends on MFD_AXP20X
> +	depends on AXP20X_ADC
> +	depends on IIO
> +	help
> +	  Say Y here to enable support for X-Powers AXP20X PMICs' battery power
> +	  supply.
> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called axp20x_battery.
> +
>  config AXP288_CHARGER
>  	tristate "X-Powers AXP288 Charger"
>  	depends on MFD_AXP20X && EXTCON_AXP288
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 7d22417..5a217b2 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_TEST_POWER)	+= test_power.o
>  
>  obj-$(CONFIG_BATTERY_88PM860X)	+= 88pm860x_battery.o
>  obj-$(CONFIG_BATTERY_ACT8945A)	+= act8945a_charger.o
> +obj-$(CONFIG_BATTERY_AXP20X)	+= axp20x_battery.o
>  obj-$(CONFIG_CHARGER_AXP20X)	+= axp20x_ac_power.o
>  obj-$(CONFIG_BATTERY_DS2760)	+= ds2760_battery.o
>  obj-$(CONFIG_BATTERY_DS2780)	+= ds2780_battery.o
> diff --git a/drivers/power/supply/axp20x_battery.c b/drivers/power/supply/axp20x_battery.c
> new file mode 100644
> index 0000000..dd040b8
> --- /dev/null
> +++ b/drivers/power/supply/axp20x_battery.c
> @@ -0,0 +1,477 @@
> +/*
> + * Battery power supply driver for X-Powers AXP20X and AXP22X PMICs
> + *
> + * Copyright 2016 Free Electrons NextThing Co.
> + *	Quentin Schulz <quentin.schulz@free-electrons.com>
> + *
> + * This driver is based on a previous upstreaming attempt by:
> + *	Bruno Prémont <bonbons@linux-vserver.org>
> + *
> + * This file is subject to the terms and conditions of the GNU General
> + * Public License. See the file "COPYING" in the main directory of this
> + * archive for more details.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/time.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/consumer.h>
> +#include <linux/mfd/axp20x.h>
> +
> +#define AXP20X_PWR_STATUS_BAT_CHARGING	BIT(2)
> +
> +#define AXP20X_PWR_OP_BATT_PRESENT	BIT(5)
> +#define AXP20X_PWR_OP_BATT_ACTIVATED	BIT(3)
> +
> +#define AXP209_FG_PERCENT		GENMASK(6, 0)
> +#define AXP22X_FG_VALID			BIT(7)
> +
> +#define AXP20X_CHRG_CTRL1_TGT_VOLT	GENMASK(6, 5)
> +#define AXP20X_CHRG_CTRL1_TGT_4_1V	(0 << 5)
> +#define AXP20X_CHRG_CTRL1_TGT_4_15V	(1 << 5)
> +#define AXP20X_CHRG_CTRL1_TGT_4_2V	(2 << 5)
> +#define AXP20X_CHRG_CTRL1_TGT_4_36V	(3 << 5)
> +
> +#define AXP22X_CHRG_CTRL1_TGT_4_22V	(1 << 5)
> +#define AXP22X_CHRG_CTRL1_TGT_4_24V	(3 << 5)
> +
> +#define AXP20X_CHRG_CTRL1_TGT_CURR	GENMASK(3, 0)
> +
> +#define AXP20X_V_OFF_MASK		GENMASK(2, 0)
> +
> +struct axp20x_batt_ps {
> +	struct regmap *regmap;
> +	struct power_supply *batt;
> +	struct axp20x_dev *axp20x;
> +	struct iio_channel *batt_chrg_i;
> +	struct iio_channel *batt_dischrg_i;
> +	struct iio_channel *batt_v;
> +	u8 axp_id;
> +};
> +
> +static int axp20x_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 AXP20X_CHRG_CTRL1_TGT_4_36V:
> +		*val = 4360000;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int axp22x_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_2V:
> +		*val = 4200000;
> +		break;
> +	case AXP22X_CHRG_CTRL1_TGT_4_22V:
> +		*val = 4220000;
> +		break;
> +	case AXP22X_CHRG_CTRL1_TGT_4_24V:
> +		*val = 4240000;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int axp20x_battery_get_prop(struct power_supply *psy,
> +				   enum power_supply_property psp,
> +				   union power_supply_propval *val)
> +{
> +	struct axp20x_batt_ps *axp20x_batt = power_supply_get_drvdata(psy);
> +	struct iio_channel *chan;
> +	int ret = 0, reg, val1;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_PRESENT:
> +	case POWER_SUPPLY_PROP_ONLINE:
> +		ret = regmap_read(axp20x_batt->regmap, AXP20X_PWR_OP_MODE,
> +				  &reg);
> +		if (ret)
> +			return ret;
> +
> +		val->intval = !!(reg & AXP20X_PWR_OP_BATT_PRESENT);
> +		break;
> +
> +	case POWER_SUPPLY_PROP_STATUS:
> +		ret = regmap_read(axp20x_batt->regmap, AXP20X_PWR_INPUT_STATUS,
> +				  &reg);
> +		if (ret)
> +			return ret;
> +
> +		if (reg & AXP20X_PWR_STATUS_BAT_CHARGING) {
> +			val->intval = POWER_SUPPLY_STATUS_CHARGING;
> +			return 0;
> +		}
> +
> +		ret = iio_read_channel_processed(axp20x_batt->batt_dischrg_i,
> +						 &val1);
> +		if (ret)
> +			return ret;
> +
> +		if (val1) {
> +			val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
> +			return 0;
> +		}
> +
> +		ret = regmap_read(axp20x_batt->regmap, AXP20X_FG_RES, &val1);
> +		if (ret)
> +			return ret;
> +
> +		/*
> +		 * Fuel Gauge data takes 7 bits but the stored value seems to be
> +		 * directly the raw percentage without any scaling to 7 bits.
> +		 */
> +		if ((val1 & AXP209_FG_PERCENT) == 100)
> +			val->intval = POWER_SUPPLY_STATUS_FULL;
> +		else
> +			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
> +		break;
> +
> +	case POWER_SUPPLY_PROP_HEALTH:
> +		ret = regmap_read(axp20x_batt->regmap, AXP20X_PWR_OP_MODE,
> +				  &val1);
> +		if (ret)
> +			return ret;
> +
> +		if (val1 & AXP20X_PWR_OP_BATT_ACTIVATED) {
> +			val->intval = POWER_SUPPLY_HEALTH_DEAD;
> +			return 0;
> +		}
> +
> +		val->intval = POWER_SUPPLY_HEALTH_GOOD;
> +		break;
> +
> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
> +		ret = regmap_read(axp20x_batt->regmap, AXP20X_CHRG_CTRL1, &reg);
> +		if (ret)
> +			return ret;
> +
> +		reg &= AXP20X_CHRG_CTRL1_TGT_CURR;
> +		val->intval = reg * 100000 + 300000;
> +		break;
> +
> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
> +		val->intval = AXP20X_CHRG_CTRL1_TGT_CURR * 100000 + 300000;
> +		break;
> +
> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		ret = regmap_read(axp20x_batt->regmap, AXP20X_PWR_INPUT_STATUS,
> +				  &reg);
> +		if (ret)
> +			return ret;
> +
> +		if (reg & AXP20X_PWR_STATUS_BAT_CHARGING)
> +			chan = axp20x_batt->batt_chrg_i;
> +		else
> +			chan = axp20x_batt->batt_dischrg_i;
> +
> +		ret = iio_read_channel_processed(chan, &val->intval);
> +		if (ret)
> +			return ret;
> +
> +		/* IIO framework gives mA but Power Supply framework gives uA */
> +		val->intval *= 1000;
> +		break;
> +
> +	case POWER_SUPPLY_PROP_CAPACITY:
> +		/* When no battery is present, return capacity is 100% */
> +		ret = regmap_read(axp20x_batt->regmap, AXP20X_PWR_OP_MODE,
> +				  &reg);
> +		if (ret)
> +			return ret;
> +
> +		if (!(reg & AXP20X_PWR_OP_BATT_PRESENT)) {
> +			val->intval = 100;
> +			return 0;
> +		}
> +
> +		ret = regmap_read(axp20x_batt->regmap, AXP20X_FG_RES, &reg);
> +		if (ret)
> +			return ret;
> +
> +		if (axp20x_batt->axp_id == AXP221_ID &&
> +		    !(reg & AXP22X_FG_VALID))
> +			return -EINVAL;
> +
> +		/*
> +		 * Fuel Gauge data takes 7 bits but the stored value seems to be
> +		 * directly the raw percentage without any scaling to 7 bits.
> +		 */
> +		val->intval = reg & AXP209_FG_PERCENT;
> +		break;
> +
> +	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
> +		if (axp20x_batt->axp_id == AXP209_ID)
> +			return axp20x_battery_get_max_voltage(axp20x_batt,
> +							      &val->intval);
> +		return axp22x_battery_get_max_voltage(axp20x_batt,
> +						      &val->intval);
> +
> +	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
> +		ret = regmap_read(axp20x_batt->regmap, AXP20X_V_OFF, &reg);
> +		if (ret)
> +			return ret;
> +
> +		val->intval = 2600000 + 100000 * (reg & AXP20X_V_OFF_MASK);
> +		break;
> +
> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		ret = iio_read_channel_processed(axp20x_batt->batt_v,
> +						 &val->intval);
> +		if (ret)
> +			return ret;
> +
> +		/* IIO framework gives mV but Power Supply framework gives uV */
> +		val->intval *= 1000;
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int axp20x_battery_set_max_voltage(struct axp20x_batt_ps *axp20x_batt,
> +					  int val)
> +{
> +	switch (val) {
> +	case 4100000:
> +		val = AXP20X_CHRG_CTRL1_TGT_4_1V;
> +		break;
> +
> +	case 4150000:
> +		if (axp20x_batt->axp_id == AXP221_ID)
> +			return -EINVAL;
> +
> +		val = AXP20X_CHRG_CTRL1_TGT_4_15V;
> +		break;
> +
> +	case 4200000:
> +		val = AXP20X_CHRG_CTRL1_TGT_4_2V;
> +		break;
> +
> +	default:
> +		/*
> +		 * AXP20x max voltage can be set to 4.36V and AXP22X max voltage
> +		 * can be set to 4.22V and 4.24V, but these voltages are too
> +		 * high for Lithium based batteries (AXP PMICs are supposed to
> +		 * be used with these kinds of battery).
> +		 */
> +		return -EINVAL;
> +	}
> +
> +	return regmap_update_bits(axp20x_batt->regmap, AXP20X_CHRG_CTRL1,
> +				  AXP20X_CHRG_CTRL1_TGT_VOLT, val);
> +}
> +
> +static int axp20x_set_constant_charge_current(struct axp20x_batt_ps *axp_batt,
> +					      int charge_current)
> +{
> +	if (axp_batt->axp_id == AXP209_ID)
> +		charge_current = (charge_current - 300000) / 100000;
> +	else
> +		charge_current = (charge_current - 300000) / 150000;
> +
> +	if (charge_current > AXP20X_CHRG_CTRL1_TGT_CURR || charge_current < 0)
> +		return -EINVAL;
> +
> +	return regmap_update_bits(axp_batt->regmap, AXP20X_CHRG_CTRL1,
> +				  AXP20X_CHRG_CTRL1_TGT_CURR, charge_current);
> +}
> +
> +static int axp20x_battery_set_prop(struct power_supply *psy,
> +				   enum power_supply_property psp,
> +				   const union power_supply_propval *val)
> +{
> +	struct axp20x_batt_ps *axp20x_batt = power_supply_get_drvdata(psy);
> +	int val1;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
> +		val1 = (val->intval - 2600000) / 100000;
> +		if (val1 < 0 || val1 > AXP20X_V_OFF_MASK)
> +			return -EINVAL;
> +
> +		return regmap_update_bits(axp20x_batt->regmap, AXP20X_V_OFF,
> +					  AXP20X_V_OFF_MASK, val1);
> +
> +	case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
> +		return axp20x_battery_set_max_voltage(axp20x_batt, val->intval);
> +
> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
> +		return axp20x_set_constant_charge_current(axp20x_batt,
> +							  val->intval);
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static enum power_supply_property axp20x_battery_props[] = {
> +	POWER_SUPPLY_PROP_PRESENT,
> +	POWER_SUPPLY_PROP_ONLINE,
> +	POWER_SUPPLY_PROP_STATUS,
> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +	POWER_SUPPLY_PROP_CURRENT_NOW,
> +	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
> +	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
> +	POWER_SUPPLY_PROP_HEALTH,
> +	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
> +	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
> +	POWER_SUPPLY_PROP_CAPACITY,
> +};
> +
> +static int axp20x_battery_prop_writeable(struct power_supply *psy,
> +					 enum power_supply_property psp)
> +{
> +	return psp == POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN ||
> +	       psp == POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN ||
> +	       psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT;
> +}
> +
> +static const struct power_supply_desc axp20x_batt_ps_desc = {
> +	.name = "axp20x-battery",
> +	.type = POWER_SUPPLY_TYPE_BATTERY,
> +	.properties = axp20x_battery_props,
> +	.num_properties = ARRAY_SIZE(axp20x_battery_props),
> +	.property_is_writeable = axp20x_battery_prop_writeable,
> +	.get_property = axp20x_battery_get_prop,
> +	.set_property = axp20x_battery_set_prop,
> +};
> +
> +static const struct of_device_id axp20x_battery_ps_id[] = {
> +	{
> +		.compatible = "x-powers,axp209-battery-power-supply",
> +		.data = (void *)AXP209_ID,
> +	}, {
> +		.compatible = "x-powers,axp221-battery-power-supply",
> +		.data = (void *)AXP221_ID,
> +	}, { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, axp20x_battery_ps_id);
> +
> +static int axp20x_power_probe(struct platform_device *pdev)
> +{
> +	struct axp20x_batt_ps *axp20x_batt;
> +	struct power_supply_config psy_cfg = {};
> +	int ret;
> +	u32 const_charge_current;
> +
> +	if (!of_device_is_available(pdev->dev.of_node))
> +		return -ENODEV;
> +
> +	axp20x_batt = devm_kzalloc(&pdev->dev, sizeof(*axp20x_batt),
> +				   GFP_KERNEL);
> +	if (!axp20x_batt)
> +		return -ENOMEM;
> +
> +	axp20x_batt->batt_v = devm_iio_channel_get(&pdev->dev, "batt_v");
> +	if (IS_ERR(axp20x_batt->batt_v)) {
> +		if (PTR_ERR(axp20x_batt->batt_v) == -ENODEV)
> +			return -EPROBE_DEFER;
> +		return PTR_ERR(axp20x_batt->batt_v);
> +	}
> +
> +	axp20x_batt->batt_chrg_i = devm_iio_channel_get(&pdev->dev,
> +							"batt_chrg_i");
> +	if (IS_ERR(axp20x_batt->batt_chrg_i)) {
> +		if (PTR_ERR(axp20x_batt->batt_chrg_i) == -ENODEV)
> +			return -EPROBE_DEFER;
> +		return PTR_ERR(axp20x_batt->batt_chrg_i);
> +	}
> +
> +	axp20x_batt->batt_dischrg_i = devm_iio_channel_get(&pdev->dev,
> +							   "batt_dischrg_i");
> +	if (IS_ERR(axp20x_batt->batt_dischrg_i)) {
> +		if (PTR_ERR(axp20x_batt->batt_dischrg_i) == -ENODEV)
> +			return -EPROBE_DEFER;
> +		return PTR_ERR(axp20x_batt->batt_dischrg_i);
> +	}
> +
> +	axp20x_batt->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	platform_set_drvdata(pdev, axp20x_batt);
> +
> +	psy_cfg.drv_data = axp20x_batt;
> +	psy_cfg.of_node = pdev->dev.of_node;
> +
> +	axp20x_batt->axp_id = (uintptr_t)of_device_get_match_data(&pdev->dev);
> +
> +	if (!device_property_read_u32(&pdev->dev,
> +				      "x-powers,constant-charge-current",
> +				      &const_charge_current)) {
> +		ret = axp20x_set_constant_charge_current(axp20x_batt,
> +							 const_charge_current);
> +		if (ret)
> +			dev_err(&pdev->dev,
> +				"couldn't set constant charge current: %d\n",
> +				ret);
> +	}
> +
> +	axp20x_batt->batt = devm_power_supply_register(&pdev->dev,
> +						       &axp20x_batt_ps_desc,
> +						       &psy_cfg);
> +	return PTR_ERR_OR_ZERO(axp20x_batt->batt);
> +}
> +
> +static struct platform_driver axp20x_batt_driver = {
> +	.probe    = axp20x_power_probe,
> +	.driver   = {
> +		.name  = "axp20x-battery-power-supply",
> +		.of_match_table = axp20x_battery_ps_id,
> +	},
> +};
> +
> +module_platform_driver(axp20x_batt_driver);
> +
> +MODULE_DESCRIPTION("Battery power supply driver for AXP20X and AXP22X PMICs");
> +MODULE_AUTHOR("Quentin Schulz <quentin.schulz@free-electrons.com>");
> +MODULE_LICENSE("GPL");
> 

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

* Re: [PATCH v2 04/25] iio: adc: add support for X-Powers AXP20X and AXP22X PMICs ADCs
  2017-01-28 14:49   ` Jonathan Cameron
@ 2017-01-28 15:12     ` Quentin Schulz
  2017-01-28 15:34       ` Jonathan Cameron
  0 siblings, 1 reply; 51+ messages in thread
From: Quentin Schulz @ 2017-01-28 15:12 UTC (permalink / raw)
  To: Jonathan Cameron, knaack.h, lars, pmeerw, robh+dt, mark.rutland,
	wens, sre, linux, maxime.ripard, lee.jones
  Cc: linux-iio, devicetree, linux-kernel, linux-pm, linux-arm-kernel,
	thomas.petazzoni, icenowy, bonbons

Hi Jonathan

On 28/01/2017 15:49, Jonathan Cameron wrote:
> On 27/01/17 08:54, Quentin Schulz wrote:
>> The X-Powers AXP20X and AXP22X PMICs have multiple ADCs. They expose the
>> battery voltage, battery charge and discharge currents, AC-in and VBUS
>> voltages and currents, 2 GPIOs muxable in ADC mode and PMIC temperature.
>>
>> This adds support for most of AXP20X and AXP22X ADCs.
>>
>> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> Pretty good, but not everything seems to be cleaned up on error paths
> in probe.
> 
> A few other suggestions / questions inline.
> 
> Jonathan
>> ---
[...]
>> +static int axp20x_adc_raw(struct iio_dev *indio_dev,
>> +			  struct iio_chan_spec const *chan, int *val)
>> +{
>> +	struct axp20x_adc_iio *info = iio_priv(indio_dev);
>> +	int size = 12;
>> +
>> +	switch (chan->type) {
>> +	case IIO_CURRENT:
>> +		/*
>> +		 * Unlike the Chinese datasheets tell, the charging current is
>> +		 * stored on 12 bits, not 13 bits.
>> +		 */
>> +		if (chan->channel == AXP20X_BATT_DISCHRG_I)
>> +			size = 13;
> Given I don't think you can get here without it being current, voltage or temp;
> couldn't this be done more cleanly with
> if ((chan->type == IIO_CURRENT) && (chan->channel == AXP20X_BAT_DISCHRG_I))
>    size = 13;
> 
> and have the rest in the normal code flow?
> 

Indeed.

>> +	case IIO_VOLTAGE:
>> +	case IIO_TEMP:
>> +		*val = axp20x_read_variable_width(info->regmap, chan->address,
>> +						  size);
>> +		if (*val < 0)
>> +			return *val;
>> +
>> +		return IIO_VAL_INT;
>> +
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +}
[...]
>> +static int axp22x_adc_scale(struct iio_chan_spec const *chan, int *val,
>> +			    int *val2)
>> +{
>> +	switch (chan->type) {
>> +	case IIO_VOLTAGE:
>> +		if (chan->channel != AXP22X_BATT_V)
>> +			return -EINVAL;
>> +
>> +		*val = 1;
>> +		*val2 = 100000;
>> +		return IIO_VAL_INT_PLUS_MICRO;
> A fixed scale of 1.1x? (just checking)

Yes, there is only one voltage exposed for AXP22X PMICs: the battery
voltage which has a scale of 1.1 (for mV).

[...]
>> +static int axp20x_probe(struct platform_device *pdev)
>> +{
>> +	struct axp20x_adc_iio *info;
>> +	struct iio_dev *indio_dev;
>> +	struct axp20x_dev *axp20x_dev;
>> +	int ret;
>> +
>> +	axp20x_dev = dev_get_drvdata(pdev->dev.parent);
>> +
>> +	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
>> +	if (!indio_dev)
>> +		return -ENOMEM;
>> +
>> +	info = iio_priv(indio_dev);
>> +	platform_set_drvdata(pdev, indio_dev);
>> +
>> +	info->regmap = axp20x_dev->regmap;
>> +	indio_dev->name = dev_name(&pdev->dev);
> Not sure on this name - what does end up as?  Expected to be
> a description of the part so in this case something like axp209-adc.
> I've been lax at picking up on this in the past and it's led to some
> crazy naming that is no use at all to userspace.  Basically this
> name just provides a convenient user readable name for userspace apps to
> use.
> 

ACK. Should we have a different name for AXP20X and AXP22X PMICs?

>> +	indio_dev->dev.parent = &pdev->dev;
>> +	indio_dev->dev.of_node = pdev->dev.of_node;
>> +	indio_dev->modes = INDIO_DIRECT_MODE;
>> +
>> +	info->data = (struct axp_data *)of_device_get_match_data(&pdev->dev);
>> +
>> +	indio_dev->info = info->data->iio_info;
>> +	indio_dev->num_channels = info->data->num_channels;
>> +	indio_dev->channels = info->data->channels;
>> +
>> +	/* Enable the ADCs on IP */
>> +	regmap_write(info->regmap, AXP20X_ADC_EN1, info->data->adc_en1_mask);
>> +
>> +	if (info->data->adc_en2)
>> +		/* Enable GPIO0/1 and internal temperature ADCs */
>> +		regmap_update_bits(info->regmap, AXP20X_ADC_EN2,
>> +				   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));
>> +
>> +	ret = iio_map_array_register(indio_dev, info->data->maps);
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev, "failed to register IIO maps: %d\n", ret);
> This should be disabling channels.

You mean disabling ADCs as it is done in the remove? If so, indeed.

>> +		return ret;
>> +	}
>> +
>> +	ret = iio_device_register(indio_dev);
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev, "could not register the device\n");
>> +		regmap_write(info->regmap, AXP20X_ADC_EN1, 0);
>> +
>> +		if (info->data->adc_en2)
>> +			regmap_write(info->regmap, AXP20X_ADC_EN2, 0);
> I'd expect to see a complete unwind of what has been done earlier in probe
> including iio_map_array_unregister.
> 
> The traditional goto error* approach is probably worth having here to
> make sure the unwind makes sense.

Indeed.

>> +
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int axp20x_remove(struct platform_device *pdev)
>> +{
>> +	struct axp20x_adc_iio *info;
>> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>> +
>> +	info = iio_priv(indio_dev);
>> +
>> +	iio_device_unregister(indio_dev);
> iio_map_array_unregister?

Yes. I see iio_device_unregister already disable all buffers, why not
unregistering the map array as well in this function?

[...]
Thanks,
Quentin

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

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

* Re: [PATCH v2 04/25] iio: adc: add support for X-Powers AXP20X and AXP22X PMICs ADCs
  2017-01-28 15:12     ` Quentin Schulz
@ 2017-01-28 15:34       ` Jonathan Cameron
  0 siblings, 0 replies; 51+ messages in thread
From: Jonathan Cameron @ 2017-01-28 15:34 UTC (permalink / raw)
  To: Quentin Schulz, knaack.h, lars, pmeerw, robh+dt, mark.rutland,
	wens, sre, linux, maxime.ripard, lee.jones
  Cc: linux-iio, devicetree, linux-kernel, linux-pm, linux-arm-kernel,
	thomas.petazzoni, icenowy, bonbons

On 28/01/17 15:12, Quentin Schulz wrote:
> Hi Jonathan
> 
> On 28/01/2017 15:49, Jonathan Cameron wrote:
>> On 27/01/17 08:54, Quentin Schulz wrote:
>>> The X-Powers AXP20X and AXP22X PMICs have multiple ADCs. They expose the
>>> battery voltage, battery charge and discharge currents, AC-in and VBUS
>>> voltages and currents, 2 GPIOs muxable in ADC mode and PMIC temperature.
>>>
>>> This adds support for most of AXP20X and AXP22X ADCs.
>>>
>>> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
>> Pretty good, but not everything seems to be cleaned up on error paths
>> in probe.
>>
>> A few other suggestions / questions inline.
>>
>> Jonathan
>>> ---
> [...]
>>> +static int axp20x_adc_raw(struct iio_dev *indio_dev,
>>> +			  struct iio_chan_spec const *chan, int *val)
>>> +{
>>> +	struct axp20x_adc_iio *info = iio_priv(indio_dev);
>>> +	int size = 12;
>>> +
>>> +	switch (chan->type) {
>>> +	case IIO_CURRENT:
>>> +		/*
>>> +		 * Unlike the Chinese datasheets tell, the charging current is
>>> +		 * stored on 12 bits, not 13 bits.
>>> +		 */
>>> +		if (chan->channel == AXP20X_BATT_DISCHRG_I)
>>> +			size = 13;
>> Given I don't think you can get here without it being current, voltage or temp;
>> couldn't this be done more cleanly with
>> if ((chan->type == IIO_CURRENT) && (chan->channel == AXP20X_BAT_DISCHRG_I))
>>    size = 13;
>>
>> and have the rest in the normal code flow?
>>
> 
> Indeed.
> 
>>> +	case IIO_VOLTAGE:
>>> +	case IIO_TEMP:
>>> +		*val = axp20x_read_variable_width(info->regmap, chan->address,
>>> +						  size);
>>> +		if (*val < 0)
>>> +			return *val;
>>> +
>>> +		return IIO_VAL_INT;
>>> +
>>> +	default:
>>> +		return -EINVAL;
>>> +	}
>>> +}
> [...]
>>> +static int axp22x_adc_scale(struct iio_chan_spec const *chan, int *val,
>>> +			    int *val2)
>>> +{
>>> +	switch (chan->type) {
>>> +	case IIO_VOLTAGE:
>>> +		if (chan->channel != AXP22X_BATT_V)
>>> +			return -EINVAL;
>>> +
>>> +		*val = 1;
>>> +		*val2 = 100000;
>>> +		return IIO_VAL_INT_PLUS_MICRO;
>> A fixed scale of 1.1x? (just checking)
> 
> Yes, there is only one voltage exposed for AXP22X PMICs: the battery
> voltage which has a scale of 1.1 (for mV).
Fair enough - just seemed an oddly specific number!
> 
> [...]
>>> +static int axp20x_probe(struct platform_device *pdev)
>>> +{
>>> +	struct axp20x_adc_iio *info;
>>> +	struct iio_dev *indio_dev;
>>> +	struct axp20x_dev *axp20x_dev;
>>> +	int ret;
>>> +
>>> +	axp20x_dev = dev_get_drvdata(pdev->dev.parent);
>>> +
>>> +	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
>>> +	if (!indio_dev)
>>> +		return -ENOMEM;
>>> +
>>> +	info = iio_priv(indio_dev);
>>> +	platform_set_drvdata(pdev, indio_dev);
>>> +
>>> +	info->regmap = axp20x_dev->regmap;
>>> +	indio_dev->name = dev_name(&pdev->dev);
>> Not sure on this name - what does end up as?  Expected to be
>> a description of the part so in this case something like axp209-adc.
>> I've been lax at picking up on this in the past and it's led to some
>> crazy naming that is no use at all to userspace.  Basically this
>> name just provides a convenient user readable name for userspace apps to
>> use.
>>
> 
> ACK. Should we have a different name for AXP20X and AXP22X PMICs?
ideally yes.
> 
>>> +	indio_dev->dev.parent = &pdev->dev;
>>> +	indio_dev->dev.of_node = pdev->dev.of_node;
>>> +	indio_dev->modes = INDIO_DIRECT_MODE;
>>> +
>>> +	info->data = (struct axp_data *)of_device_get_match_data(&pdev->dev);
>>> +
>>> +	indio_dev->info = info->data->iio_info;
>>> +	indio_dev->num_channels = info->data->num_channels;
>>> +	indio_dev->channels = info->data->channels;
>>> +
>>> +	/* Enable the ADCs on IP */
>>> +	regmap_write(info->regmap, AXP20X_ADC_EN1, info->data->adc_en1_mask);
>>> +
>>> +	if (info->data->adc_en2)
>>> +		/* Enable GPIO0/1 and internal temperature ADCs */
>>> +		regmap_update_bits(info->regmap, AXP20X_ADC_EN2,
>>> +				   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));
>>> +
>>> +	ret = iio_map_array_register(indio_dev, info->data->maps);
>>> +	if (ret < 0) {
>>> +		dev_err(&pdev->dev, "failed to register IIO maps: %d\n", ret);
>> This should be disabling channels.
> 
> You mean disabling ADCs as it is done in the remove? If so, indeed.
yes.
> 
>>> +		return ret;
>>> +	}
>>> +
>>> +	ret = iio_device_register(indio_dev);
>>> +	if (ret < 0) {
>>> +		dev_err(&pdev->dev, "could not register the device\n");
>>> +		regmap_write(info->regmap, AXP20X_ADC_EN1, 0);
>>> +
>>> +		if (info->data->adc_en2)
>>> +			regmap_write(info->regmap, AXP20X_ADC_EN2, 0);
>> I'd expect to see a complete unwind of what has been done earlier in probe
>> including iio_map_array_unregister.
>>
>> The traditional goto error* approach is probably worth having here to
>> make sure the unwind makes sense.
> 
> Indeed.
> 
>>> +
>>> +		return ret;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int axp20x_remove(struct platform_device *pdev)
>>> +{
>>> +	struct axp20x_adc_iio *info;
>>> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>>> +
>>> +	info = iio_priv(indio_dev);
>>> +
>>> +	iio_device_unregister(indio_dev);
>> iio_map_array_unregister?
> 
> Yes. I see iio_device_unregister already disable all buffers, why not
> unregistering the map array as well in this function?
Mostly about symmetry - it's easier to be sure things are right
if there are explicit calls to set stuff up and then to unwind it.

Jonathan
> 
> [...]
> Thanks,
> Quentin
> 

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

* Re: [PATCH v2 03/25] power: supply: axp20x_usb_power: use IIO channels when available
  2017-01-27  8:54 ` [PATCH v2 03/25] power: supply: axp20x_usb_power: use IIO channels when available Quentin Schulz
  2017-01-28 14:36   ` Jonathan Cameron
@ 2017-01-29 16:21   ` Sebastian Reichel
  1 sibling, 0 replies; 51+ messages in thread
From: Sebastian Reichel @ 2017-01-29 16:21 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, linux,
	maxime.ripard, lee.jones, linux-iio, devicetree, linux-kernel,
	linux-pm, linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

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

Hi,

On Fri, Jan 27, 2017 at 09:54:36AM +0100, Quentin Schulz wrote:
> The X-Powers AXP20X PMIC exposes the current current and voltage
> measures via an internal ADC.
> 
> This adds the possibility to read IIO channels directly for processed
> values rather than reading the registers and computing the value.
> 
> For backward compatibility purpose, if the IIO driver is not compiled,
> this driver will fall back on previous behaviour which is direct
> register readings.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>

I queued this into power-supply's for next branch.

-- Sebastian

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

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

* Re: [PATCH v2 08/25] dt-bindings: power: supply: add AXP20X/AXP22X AC power supply
  2017-01-27  8:54 ` [PATCH v2 08/25] dt-bindings: power: supply: add AXP20X/AXP22X AC power supply Quentin Schulz
@ 2017-01-29 16:22   ` Sebastian Reichel
  0 siblings, 0 replies; 51+ messages in thread
From: Sebastian Reichel @ 2017-01-29 16:22 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, linux,
	maxime.ripard, lee.jones, linux-iio, devicetree, linux-kernel,
	linux-pm, linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

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

Hi,

On Fri, Jan 27, 2017 at 09:54:41AM +0100, Quentin Schulz wrote:
> The X-Powers AXP20X and AXP22X PMICs have an AC entry to supply power to
> the board. They have a few registers dedicated to the status of the AC
> power supply.
> 
> This adds the DT binding documentation for the AC power supply for
> AXP20X and AXP22X PMICs.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>

Thanks, I queued this into power-supply's for-next branch.

-- Sebastian

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

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

* Re: [PATCH v2 10/25] power: supply: add AC power supply driver for AXP20X and AXP22X PMICs
  2017-01-27  8:54 ` [PATCH v2 10/25] power: supply: add AC power supply driver for AXP20X and AXP22X PMICs Quentin Schulz
@ 2017-01-29 16:25   ` Sebastian Reichel
  0 siblings, 0 replies; 51+ messages in thread
From: Sebastian Reichel @ 2017-01-29 16:25 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, linux,
	maxime.ripard, lee.jones, linux-iio, devicetree, linux-kernel,
	linux-pm, linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

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

Hi,

On Fri, Jan 27, 2017 at 09:54:43AM +0100, Quentin Schulz wrote:
> The X-Powers AXP20X and AXP22X PMICs expose the status of AC power
> supply.
> 
> Moreover, the AXP20X can also expose the current current and voltage
> values of the AC power supply.
> 
> This adds the driver which exposes the status of the AC power supply of
> the AXP20X and AXP22X PMICs.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> Acked-by: Jonathan Cameron <jic23@kernel.org>

Thanks, I queued this into power-supply's for-next branch. I
removed "struct device_node *np" and "int axp20x_id" from
"struct axp20x_ac_power", since they were unused.

-- Sebastian

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

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

* Re: [PATCH v2 09/25] iio: adc: axp20x_adc: map acin_i and acin_v
  2017-01-27  8:54 ` [PATCH v2 09/25] iio: adc: axp20x_adc: map acin_i and acin_v Quentin Schulz
  2017-01-28 14:51   ` Jonathan Cameron
@ 2017-01-29 16:31   ` Sebastian Reichel
  1 sibling, 0 replies; 51+ messages in thread
From: Sebastian Reichel @ 2017-01-29 16:31 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, linux,
	maxime.ripard, lee.jones, linux-iio, devicetree, linux-kernel,
	linux-pm, linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

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

Hi,

On Fri, Jan 27, 2017 at 09:54:42AM +0100, Quentin Schulz wrote:
> This maps the IIO channels acin_i and acin_v (respectively exposing the
> current current and voltage measures of the AC power supply) to the AC
> power supply driver.
> 
> Only the AXP20X PMICs have these ADC channels and thus they are only
> mapped for this version of the PMIC.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> ---
> 
> added in v2
> 
>  drivers/iio/adc/axp20x_adc.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
> index bacde92..14f4ec0 100644
> --- a/drivers/iio/adc/axp20x_adc.c
> +++ b/drivers/iio/adc/axp20x_adc.c
> @@ -104,6 +104,14 @@ static struct iio_map axp20x_maps[] = {
>  		.consumer_dev_name = "axp20x-usb-power-supply",
>  		.consumer_channel = "vbus_i",
>  		.adc_channel_label = "vbus_i",
> +	}, {
> +		.consumer_dev_name = "axp20x-ac-power-supply",
> +		.consumer_channel = "acin_v",
> +		.adc_channel_label = "acin_v",
> +	}, {
> +		.consumer_dev_name = "axp20x-ac-power-supply",
> +		.consumer_channel = "acin_i",
> +		.adc_channel_label = "acin_i",
>  	}, { /* sentinel */ }
>  };

I suggest to merge this into Patch 4 before resending the series.

-- Sebastian

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

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

* Re: [PATCH v2 11/25] mfd: axp20x: add AC power supply cells for AXP22X PMICs
  2017-01-27  8:54 ` [PATCH v2 11/25] mfd: axp20x: add AC power supply cells for " Quentin Schulz
@ 2017-01-29 16:35   ` Sebastian Reichel
  2017-01-31  4:11     ` Chen-Yu Tsai
  0 siblings, 1 reply; 51+ messages in thread
From: Sebastian Reichel @ 2017-01-29 16:35 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, linux,
	maxime.ripard, lee.jones, linux-iio, devicetree, linux-kernel,
	linux-pm, linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

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

Hi,

On Fri, Jan 27, 2017 at 09:54:44AM +0100, Quentin Schulz wrote:
> The X-Powers AXP20X and AXP22X PMICs expose the status of AC power
> supply.
> 
> This adds the AC power supply driver to the MFD cells of the AXP22X
> PMICs.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

Acked-By: Sebastian Reichel <sre@kernel.org>

-- Sebastian

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

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

* Re: [PATCH v2 16/25] dt-bindings: power: supply: add AXP20X/AXP22X battery DT binding
  2017-01-27  8:54 ` [PATCH v2 16/25] dt-bindings: power: supply: add AXP20X/AXP22X battery DT binding Quentin Schulz
@ 2017-01-29 16:47   ` Sebastian Reichel
  2017-01-31  7:59     ` Quentin Schulz
  0 siblings, 1 reply; 51+ messages in thread
From: Sebastian Reichel @ 2017-01-29 16:47 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, linux,
	maxime.ripard, lee.jones, linux-iio, devicetree, linux-kernel,
	linux-pm, linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

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

Hi,

On Fri, Jan 27, 2017 at 09:54:49AM +0100, Quentin Schulz wrote:
>  - added x-powers,constant-charge-current property to set the
>  maximal default constant current charge of the battery,

Since this is information about the battery and not the fuel-gauge,
it should use the WIP "framework" for information about batteries.

Have a look at the following patchset:

http://marc.info/?l=linux-pm&m=148411561025684&w=2

-- Sebastian

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

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

* Re: [PATCH v2 06/25] ARM: dtsi: axp209: add AXP209 ADC subnode
  2017-01-27  8:54 ` [PATCH v2 06/25] ARM: dtsi: axp209: add AXP209 ADC subnode Quentin Schulz
@ 2017-01-31  4:08   ` Chen-Yu Tsai
  0 siblings, 0 replies; 51+ messages in thread
From: Chen-Yu Tsai @ 2017-01-31  4:08 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: knaack.h, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Rob Herring, Mark Rutland, Chen-Yu Tsai, Sebastian Reichel,
	Russell King, Maxime Ripard, Lee Jones, linux-iio, devicetree,
	linux-kernel, open list:THERMAL, linux-arm-kernel,
	Thomas Petazzoni, Icenowy Zheng, Bruno Prémont

On Fri, Jan 27, 2017 at 4:54 PM, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> X-Powers AXP209 PMIC has multiple ADCs, each one exposing data from the
> different power supplies connected to the PMIC.
>
> This adds the ADC subnode for AXP20X PMIC.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>

Given the device node no longer exports anything, we might
as well drop it from the device tree.

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

* Re: [PATCH v2 11/25] mfd: axp20x: add AC power supply cells for AXP22X PMICs
  2017-01-29 16:35   ` Sebastian Reichel
@ 2017-01-31  4:11     ` Chen-Yu Tsai
  0 siblings, 0 replies; 51+ messages in thread
From: Chen-Yu Tsai @ 2017-01-31  4:11 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Quentin Schulz, knaack.h, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Rob Herring, Mark Rutland, Chen-Yu Tsai,
	Russell King, Maxime Ripard, Lee Jones, linux-iio, devicetree,
	linux-kernel, open list:THERMAL, linux-arm-kernel,
	Thomas Petazzoni, Icenowy Zheng, Bruno Prémont

On Mon, Jan 30, 2017 at 12:35 AM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> On Fri, Jan 27, 2017 at 09:54:44AM +0100, Quentin Schulz wrote:
>> The X-Powers AXP20X and AXP22X PMICs expose the status of AC power
>> supply.
>>
>> This adds the AC power supply driver to the MFD cells of the AXP22X
>> PMICs.
>>
>> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
>> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
>
> Acked-By: Sebastian Reichel <sre@kernel.org>

Acked-by: Chen-Yu Tsai <wens@csie.org>

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

* Re: [PATCH v2 12/25] ARM: dtsi: axp209: add AC power supply subnode
  2017-01-27  8:54 ` [PATCH v2 12/25] ARM: dtsi: axp209: add AC power supply subnode Quentin Schulz
@ 2017-01-31  4:12   ` Chen-Yu Tsai
  0 siblings, 0 replies; 51+ messages in thread
From: Chen-Yu Tsai @ 2017-01-31  4:12 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: knaack.h, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Rob Herring, Mark Rutland, Chen-Yu Tsai, Sebastian Reichel,
	Russell King, Maxime Ripard, Lee Jones, linux-iio, devicetree,
	linux-kernel, open list:THERMAL, linux-arm-kernel,
	Thomas Petazzoni, Icenowy Zheng, Bruno Prémont

On Fri, Jan 27, 2017 at 4:54 PM, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> The X-Powers AXP20X PMIC exposes the status of AC power supply, the
> current current and voltage supplied to the board by the AC power
> supply.
>
> This adds the AC power supply subnode for AXP20X PMIC.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

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

* Re: [PATCH v2 13/25] ARM: dtsi: axp22x: add AC power supply subnode
  2017-01-27  8:54 ` [PATCH v2 13/25] ARM: dtsi: axp22x: " Quentin Schulz
@ 2017-01-31  4:13   ` Chen-Yu Tsai
  0 siblings, 0 replies; 51+ messages in thread
From: Chen-Yu Tsai @ 2017-01-31  4:13 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: knaack.h, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Rob Herring, Mark Rutland, Chen-Yu Tsai, Sebastian Reichel,
	Russell King, Maxime Ripard, Lee Jones, linux-iio, devicetree,
	linux-kernel, open list:THERMAL, linux-arm-kernel,
	Thomas Petazzoni, Icenowy Zheng, Bruno Prémont

On Fri, Jan 27, 2017 at 4:54 PM, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> The X-Powers AXP22X PMIC exposes the status of AC power supply.
>
> This adds the AC power supply subnode for the AXP22X PMIC.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

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

* Re: [PATCH v2 14/25] ARM: dts: sun8i: sina33: enable ACIN power supply subnode
  2017-01-27  8:54 ` [PATCH v2 14/25] ARM: dts: sun8i: sina33: enable ACIN " Quentin Schulz
@ 2017-01-31  5:03   ` Chen-Yu Tsai
  0 siblings, 0 replies; 51+ messages in thread
From: Chen-Yu Tsai @ 2017-01-31  5:03 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: knaack.h, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Rob Herring, Mark Rutland, Chen-Yu Tsai, Sebastian Reichel,
	Russell King, Maxime Ripard, Lee Jones, linux-iio, devicetree,
	linux-kernel, open list:THERMAL, linux-arm-kernel,
	Thomas Petazzoni, Icenowy Zheng, Bruno Prémont

On Fri, Jan 27, 2017 at 4:54 PM, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> The Sinlinx SinA33 has an AXP223 PMIC and an ACIN connector, thus, we
> enable the ACIN power supply in its Device Tree.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

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

* Re: [PATCH v2 15/25] ARM: sun5i: chip: enable ACIN power supply subnode
  2017-01-27  8:54 ` [PATCH v2 15/25] ARM: sun5i: chip: " Quentin Schulz
@ 2017-01-31  5:03   ` Chen-Yu Tsai
  0 siblings, 0 replies; 51+ messages in thread
From: Chen-Yu Tsai @ 2017-01-31  5:03 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: knaack.h, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Rob Herring, Mark Rutland, Chen-Yu Tsai, Sebastian Reichel,
	Russell King, Maxime Ripard, Lee Jones, linux-iio, devicetree,
	linux-kernel, open list:THERMAL, linux-arm-kernel,
	Thomas Petazzoni, Icenowy Zheng, Bruno Prémont

On Fri, Jan 27, 2017 at 4:54 PM, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> The NextThing Co. CHIP has an AXP209 PMIC and can be power-supplied by
> ACIN via the CHG-IN pin.
>
> This enables the ACIN power supply subnode in the DT.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

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

* Re: [PATCH v2 17/25] mfd: axp20x: add CHRG_CTRL1/2/3 to writeable regs for AXP20X/AXP22X
  2017-01-27  8:54 ` [PATCH v2 17/25] mfd: axp20x: add CHRG_CTRL1/2/3 to writeable regs for AXP20X/AXP22X Quentin Schulz
@ 2017-01-31  5:05   ` Chen-Yu Tsai
  0 siblings, 0 replies; 51+ messages in thread
From: Chen-Yu Tsai @ 2017-01-31  5:05 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: knaack.h, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Rob Herring, Mark Rutland, Chen-Yu Tsai, Sebastian Reichel,
	Russell King, Maxime Ripard, Lee Jones, linux-iio, devicetree,
	linux-kernel, open list:THERMAL, linux-arm-kernel,
	Thomas Petazzoni, Icenowy Zheng, Bruno Prémont

On Fri, Jan 27, 2017 at 4:54 PM, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> The CHRG_CTRL1 and CHRG_CTRL2 registers are made for controlling
> different battery charging settings such as the constant current charge
> value.
>
> The AXP22X also have a third register CHRG_CTRL3 which has settings for
> battery charging too.
>
> This adds the CHRG_CTRL1, CHRG_CTRL2 and CHRG_CTRL3 registers to the
> list of writeable registers for AXP20X and AXP22X PMICs.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Chen-Yu Tsai <wens@csie.org>

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

* Re: [PATCH v2 19/25] iio: adc: axp20x_adc: map battery IIO channels
  2017-01-28 14:52   ` Jonathan Cameron
@ 2017-01-31  5:06     ` Chen-Yu Tsai
  0 siblings, 0 replies; 51+ messages in thread
From: Chen-Yu Tsai @ 2017-01-31  5:06 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Quentin Schulz, knaack.h, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Rob Herring, Mark Rutland, Chen-Yu Tsai,
	Sebastian Reichel, Russell King, Maxime Ripard, Lee Jones,
	linux-iio, devicetree, linux-kernel, open list:THERMAL,
	linux-arm-kernel, Thomas Petazzoni, Icenowy Zheng,
	Bruno Prémont

On Sat, Jan 28, 2017 at 10:52 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 27/01/17 08:54, Quentin Schulz wrote:
>> This maps the IIO channels batt_v, batt_chrg_i and batt_dischrg_i
>> (respectively exposing the current charging and discharging currents and
>> current voltage measures of the battery power supply) to the battery
>> power supply driver.
>>
>> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> Acked-by: Jonathan Cameron <jic23@kernel.org>

Acked-by: Chen-Yu Tsai <wens@csie.org>

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

* Re: [PATCH v2 16/25] dt-bindings: power: supply: add AXP20X/AXP22X battery DT binding
  2017-01-29 16:47   ` Sebastian Reichel
@ 2017-01-31  7:59     ` Quentin Schulz
  0 siblings, 0 replies; 51+ messages in thread
From: Quentin Schulz @ 2017-01-31  7:59 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, linux,
	maxime.ripard, lee.jones, linux-iio, devicetree, linux-kernel,
	linux-pm, linux-arm-kernel, thomas.petazzoni, icenowy, bonbons


[-- Attachment #1.1: Type: text/plain, Size: 1645 bytes --]

Hi,

On 29/01/2017 17:47, Sebastian Reichel wrote:
> Hi,
> 
> On Fri, Jan 27, 2017 at 09:54:49AM +0100, Quentin Schulz wrote:
>>  - added x-powers,constant-charge-current property to set the
>>  maximal default constant current charge of the battery,
> 
> Since this is information about the battery and not the fuel-gauge,
> it should use the WIP "framework" for information about batteries.
> 
> Have a look at the following patchset:
> 
> http://marc.info/?l=linux-pm&m=148411561025684&w=2
> 

OK. So what you propose is to have a fourth property in this new
structure named design-max-constant-charge-current that gives the
maximal input amperage the battery can receive?

Then, I set the charger to output a maximum of this amperage by default
and let the user the possibility to choose between the minimum allowed
by the PMIC and the maximum allowed by the battery from sysfs. That
makes more sense than what I do here in the way that I didn't protect a
possible over-amperage of the battery, thing that Chen-Yu was afraid
some users would do.

I've a comment on the linked patches however. Though the three
properties are listed as optional in the binding-dt, the implementation
is saying the opposite:

http://marc.info/?l=linux-pm&m=148411561725693&w=2

If I'm not mistaken, if `nominal-microvolt' or `design-microwatt-hours'
is not a property of the DT node, power_supply_get_battery_info will
return without parsing the other properties and even return a negative
error.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [PATCH v2 02/25] mfd: axp20x: correct name of temperature data ADC registers
  2017-01-27  8:54 ` [PATCH v2 02/25] mfd: axp20x: correct name of temperature data ADC registers Quentin Schulz
  2017-01-27  9:30   ` Chen-Yu Tsai
@ 2017-02-08  9:26   ` Lee Jones
  1 sibling, 0 replies; 51+ messages in thread
From: Lee Jones @ 2017-02-08  9:26 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: knaack.h, lars, pmeerw, robh+dt, mark.rutland, wens, sre, linux,
	maxime.ripard, linux-iio, devicetree, linux-kernel, linux-pm,
	linux-arm-kernel, thomas.petazzoni, icenowy, bonbons

On Fri, 27 Jan 2017, Quentin Schulz wrote:

> The registers 0x56 and 0x57 of AXP22X PMIC store the value of the
> internal temperature of the PMIC.
> 
> This patch modifies the name of these registers from AXP22X_PMIC_ADC_H/L
> to AXP22X_PMIC_TEMP_H/L so their purpose is clearer.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> ---
> 
> added in v2
> 
>  drivers/mfd/axp20x.c       | 2 +-
>  include/linux/mfd/axp20x.h | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
  
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 619a83e..9c2fd37 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -102,7 +102,7 @@ static const struct regmap_range axp22x_volatile_ranges[] = {
>  	regmap_reg_range(AXP20X_VBUS_IPSOUT_MGMT, AXP20X_VBUS_IPSOUT_MGMT),
>  	regmap_reg_range(AXP20X_IRQ1_EN, AXP20X_IRQ5_STATE),
>  	regmap_reg_range(AXP22X_GPIO_STATE, AXP22X_GPIO_STATE),
> -	regmap_reg_range(AXP22X_PMIC_ADC_H, AXP20X_IPSOUT_V_HIGH_L),
> +	regmap_reg_range(AXP22X_PMIC_TEMP_H, AXP20X_IPSOUT_V_HIGH_L),
>  	regmap_reg_range(AXP20X_FG_RES, AXP20X_FG_RES),
>  };
>  
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index a4860bc..5ecadbb 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -228,8 +228,8 @@ enum {
>  #define AXP20X_OCV_MAX			0xf
>  
>  /* AXP22X specific registers */
> -#define AXP22X_PMIC_ADC_H		0x56
> -#define AXP22X_PMIC_ADC_L		0x57
> +#define AXP22X_PMIC_TEMP_H		0x56
> +#define AXP22X_PMIC_TEMP_L		0x57
>  #define AXP22X_TS_ADC_H			0x58
>  #define AXP22X_TS_ADC_L			0x59
>  #define AXP22X_BATLOW_THRES1		0xe6

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

end of thread, other threads:[~2017-02-08  9:27 UTC | newest]

Thread overview: 51+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-27  8:54 [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Quentin Schulz
2017-01-27  8:54 ` [PATCH v2 01/25] dt-bindings: iio: adc: add AXP20X/AXP22X ADC DT binding Quentin Schulz
2017-01-27  8:54 ` [PATCH v2 02/25] mfd: axp20x: correct name of temperature data ADC registers Quentin Schulz
2017-01-27  9:30   ` Chen-Yu Tsai
2017-02-08  9:26   ` Lee Jones
2017-01-27  8:54 ` [PATCH v2 03/25] power: supply: axp20x_usb_power: use IIO channels when available Quentin Schulz
2017-01-28 14:36   ` Jonathan Cameron
2017-01-29 16:21   ` Sebastian Reichel
2017-01-27  8:54 ` [PATCH v2 04/25] iio: adc: add support for X-Powers AXP20X and AXP22X PMICs ADCs Quentin Schulz
2017-01-28 14:49   ` Jonathan Cameron
2017-01-28 15:12     ` Quentin Schulz
2017-01-28 15:34       ` Jonathan Cameron
2017-01-27  8:54 ` [PATCH v2 05/25] mfd: axp20x: add ADC cells for AXP20X and AXP22X PMICs Quentin Schulz
2017-01-27  8:54 ` [PATCH v2 06/25] ARM: dtsi: axp209: add AXP209 ADC subnode Quentin Schulz
2017-01-31  4:08   ` Chen-Yu Tsai
2017-01-27  8:54 ` [PATCH v2 07/25] ARM: dtsi: axp22x: add AXP22X " Quentin Schulz
2017-01-27  8:54 ` [PATCH v2 08/25] dt-bindings: power: supply: add AXP20X/AXP22X AC power supply Quentin Schulz
2017-01-29 16:22   ` Sebastian Reichel
2017-01-27  8:54 ` [PATCH v2 09/25] iio: adc: axp20x_adc: map acin_i and acin_v Quentin Schulz
2017-01-28 14:51   ` Jonathan Cameron
2017-01-29 16:31   ` Sebastian Reichel
2017-01-27  8:54 ` [PATCH v2 10/25] power: supply: add AC power supply driver for AXP20X and AXP22X PMICs Quentin Schulz
2017-01-29 16:25   ` Sebastian Reichel
2017-01-27  8:54 ` [PATCH v2 11/25] mfd: axp20x: add AC power supply cells for " Quentin Schulz
2017-01-29 16:35   ` Sebastian Reichel
2017-01-31  4:11     ` Chen-Yu Tsai
2017-01-27  8:54 ` [PATCH v2 12/25] ARM: dtsi: axp209: add AC power supply subnode Quentin Schulz
2017-01-31  4:12   ` Chen-Yu Tsai
2017-01-27  8:54 ` [PATCH v2 13/25] ARM: dtsi: axp22x: " Quentin Schulz
2017-01-31  4:13   ` Chen-Yu Tsai
2017-01-27  8:54 ` [PATCH v2 14/25] ARM: dts: sun8i: sina33: enable ACIN " Quentin Schulz
2017-01-31  5:03   ` Chen-Yu Tsai
2017-01-27  8:54 ` [PATCH v2 15/25] ARM: sun5i: chip: " Quentin Schulz
2017-01-31  5:03   ` Chen-Yu Tsai
2017-01-27  8:54 ` [PATCH v2 16/25] dt-bindings: power: supply: add AXP20X/AXP22X battery DT binding Quentin Schulz
2017-01-29 16:47   ` Sebastian Reichel
2017-01-31  7:59     ` Quentin Schulz
2017-01-27  8:54 ` [PATCH v2 17/25] mfd: axp20x: add CHRG_CTRL1/2/3 to writeable regs for AXP20X/AXP22X Quentin Schulz
2017-01-31  5:05   ` Chen-Yu Tsai
2017-01-27  8:54 ` [PATCH v2 18/25] mfd: axp20x: add V_OFF to writeable regs for AXP20X and AXP22X Quentin Schulz
2017-01-27  8:54 ` [PATCH v2 19/25] iio: adc: axp20x_adc: map battery IIO channels Quentin Schulz
2017-01-28 14:52   ` Jonathan Cameron
2017-01-31  5:06     ` Chen-Yu Tsai
2017-01-27  8:54 ` [PATCH v2 20/25] power: supply: add battery driver for AXP20X and AXP22X PMICs Quentin Schulz
2017-01-28 14:54   ` Jonathan Cameron
2017-01-27  8:54 ` [PATCH v2 21/25] mfd: axp20x: add MFD cells for AXP20X and AXP22X battery driver Quentin Schulz
2017-01-27  8:54 ` [PATCH v2 22/25] ARM: dtsi: axp209: add battery power supply subnode Quentin Schulz
2017-01-27  8:54 ` [PATCH v2 23/25] ARM: dtsi: axp22x: " Quentin Schulz
2017-01-27  8:54 ` [PATCH v2 24/25] ARM: dts: sun8i: sina33: enable " Quentin Schulz
2017-01-27  8:54 ` [PATCH v2 25/25] ARM: sun5i: chip: " Quentin Schulz
2017-01-27  9:19 ` [PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers Maxime Ripard

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