[PATCH] ARM: sun8i: Add Parrot Board DTS

2016-06-13 Thread Quentin Schulz
The Parrot Board is an evaluation board with an Allwinner R16 (assumed
to be close to an Allwinner A33), 4GB of NAND, 512MB of RAM, USB host
and OTG, a WiFi/Bluetooth combo chip, a micro SD Card reader, 2
controllable buttons, an LVDS port with separated backlight and
capacitive touch panel ports, an audio/microphone jack, a camera CSI
port, 2 sets of 22 GPIOs and an accelerometer.

Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
---
 arch/arm/boot/dts/Makefile |   1 +
 arch/arm/boot/dts/sun8i-r16-parrot.dts | 333 +
 2 files changed, 334 insertions(+)
 create mode 100644 arch/arm/boot/dts/sun8i-r16-parrot.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 06b6c2d..1149512 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -760,6 +760,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
sun8i-a33-ippo-q8h-v1.2.dtb \
sun8i-a33-q8-tablet.dtb \
sun8i-a33-sinlinx-sina33.dtb \
+   sun8i-r16-parrot.dtb \
sun8i-a83t-allwinner-h8homlet-v2.dtb \
sun8i-a83t-cubietruck-plus.dtb \
sun8i-h3-orangepi-2.dtb \
diff --git a/arch/arm/boot/dts/sun8i-r16-parrot.dts 
b/arch/arm/boot/dts/sun8i-r16-parrot.dts
new file mode 100644
index 000..75e2420
--- /dev/null
+++ b/arch/arm/boot/dts/sun8i-r16-parrot.dts
@@ -0,0 +1,333 @@
+/*
+ * Copyright 2015 Quentin Schulz
+ *
+ * Quentin Schulz <quentin.sch...@free-electrons.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file 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.
+ *
+ * This file 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.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "sun8i-a33.dtsi"
+#include "sunxi-common-regulators.dtsi"
+
+#include 
+#include 
+
+/ {
+   model = "Allwinner Parrot EVB R16";
+   compatible = "allwinner,parrot-evb-r16", "allwinner,sun8i-a33";
+
+   aliases {
+   serial0 = 
+   };
+
+   chosen {
+   stdout-path = "serial0:115200n8";
+   };
+
+   leds {
+   compatible = "gpio-leds";
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins_r16>;
+
+   led1 {
+   label = "r16:led1:usr";
+   gpio = < 4 17 GPIO_ACTIVE_HIGH>; /* PE17 */
+   };
+
+   led2 {
+   label = "r16:led2:usr";
+   gpio = < 4 16 GPIO_ACTIVE_HIGH>; /* PE16 */
+   };
+   };
+
+   wifi_pwrseq: wifi_pwrseq {
+   compatible = "mmc-pwrseq-simple";
+   reset-gpios = <_pio 0 6 GPIO_ACTIVE_LOW>; /* PL06 */
+   };
+
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins_a>;
+   status = "okay";
+};
+
+ {
+   vref-supply = <_aldo3>;
+   status = "okay";
+
+   button@0 {
+   label = "V+";
+   linux,code 

[PATCH] ARM: sun8i: Add Parrot Board DTS

2016-06-13 Thread Quentin Schulz
The Parrot Board is an evaluation board with an Allwinner R16 (assumed
to be close to an Allwinner A33), 4GB of NAND, 512MB of RAM, USB host
and OTG, a WiFi/Bluetooth combo chip, a micro SD Card reader, 2
controllable buttons, an LVDS port with separated backlight and
capacitive touch panel ports, an audio/microphone jack, a camera CSI
port, 2 sets of 22 GPIOs and an accelerometer.

This patch requires patches named "[v3] regulator: axp20x: Add support
for the (external) drivebus regulator" and "[PATCH 3/4] ARM: dts:
axp22x.dtsi: Add reg_drivebus node" from Hans de Goede or the micro USB
port would not be powered.

Quentin Schulz (1):
  ARM: sun8i: Add Parrot Board DTS

 arch/arm/boot/dts/Makefile |   1 +
 arch/arm/boot/dts/sun8i-r16-parrot.dts | 333 +
 2 files changed, 334 insertions(+)
 create mode 100644 arch/arm/boot/dts/sun8i-r16-parrot.dts

-- 
2.5.0



Re: [PATCH] ARM: sun8i: Add Parrot Board DTS

2016-06-14 Thread Quentin Schulz
Hi,

On 13/06/2016 15:04, Chen-Yu Tsai wrote:
> Hi,
> 
> On Mon, Jun 13, 2016 at 6:15 PM, Quentin Schulz
> <quentin.sch...@free-electrons.com> wrote:
>> The Parrot Board is an evaluation board with an Allwinner R16 (assumed
>> to be close to an Allwinner A33), 4GB of NAND, 512MB of RAM, USB host
> 
> You say NAND here, but you enable mmc2 for eMMC below. Please correct it.
> 

ACK.

>> and OTG, a WiFi/Bluetooth combo chip, a micro SD Card reader, 2
>> controllable buttons, an LVDS port with separated backlight and
>> capacitive touch panel ports, an audio/microphone jack, a camera CSI
>> port, 2 sets of 22 GPIOs and an accelerometer.
> 
> I assume the board is this one:
> 
> https://world.taobao.com/item/530374411673.htm
> 

Definitely looks like it.

>> Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
>> ---
>>  arch/arm/boot/dts/Makefile |   1 +
>>  arch/arm/boot/dts/sun8i-r16-parrot.dts | 333 
>> +
>>  2 files changed, 334 insertions(+)
>>  create mode 100644 arch/arm/boot/dts/sun8i-r16-parrot.dts
>>
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index 06b6c2d..1149512 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -760,6 +760,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
>> sun8i-a33-ippo-q8h-v1.2.dtb \
>> sun8i-a33-q8-tablet.dtb \
>> sun8i-a33-sinlinx-sina33.dtb \
>> +   sun8i-r16-parrot.dtb \
>> sun8i-a83t-allwinner-h8homlet-v2.dtb \
>> sun8i-a83t-cubietruck-plus.dtb \
>> sun8i-h3-orangepi-2.dtb \
>> diff --git a/arch/arm/boot/dts/sun8i-r16-parrot.dts 
>> b/arch/arm/boot/dts/sun8i-r16-parrot.dts
>> new file mode 100644
>> index 000..75e2420
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/sun8i-r16-parrot.dts
>> @@ -0,0 +1,333 @@
>> +/*
>> + * Copyright 2015 Quentin Schulz
>> + *
>> + * Quentin Schulz <quentin.sch...@free-electrons.com>
>> + *
>> + * This file is dual-licensed: you can use it either under the terms
>> + * of the GPL or the X11 license, at your option. Note that this dual
>> + * licensing only applies to this file, and not this project as a
>> + * whole.
>> + *
>> + *  a) This file 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.
>> + *
>> + * This file 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.
>> + *
>> + * Or, alternatively,
>> + *
>> + *  b) Permission is hereby granted, free of charge, to any person
>> + * obtaining a copy of this software and associated documentation
>> + * files (the "Software"), to deal in the Software without
>> + * restriction, including without limitation the rights to use,
>> + * copy, modify, merge, publish, distribute, sublicense, and/or
>> + * sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following
>> + * conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be
>> + * included in all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
>> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
>> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> + * OTHER DEALINGS IN THE SOFTWARE.
>> + */
>> +
>> +/dts-v1/;
>> +#include "sun8i-a33.dtsi"
>> +#include "sunxi-common-regulators.dtsi"
>> +
>> +#include 
>> +#include 
>> +
>> +/ {
>> +   model = "Allwinner Parrot EVB R16";
>> +   compatible = "allwinner,parrot-evb-r16", "allwinner,sun8i-a33";
>> +
>> +   aliases {
>> +

[PATCH] phy: phy-sun4i-usb: Add log when probing

2016-06-13 Thread Quentin Schulz
When phy-sun4i-usb's probing fails, it does not print the reason in
kernel log, forcing the developer to edit this driver to add info logs.
This commit makes the kernel print the reason of phy-sun4i-usb's probing
failure or a success message.

Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
---
 drivers/phy/phy-sun4i-usb.c | 28 ++--
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 45f01d6..117afa0 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -552,24 +552,32 @@ static int sun4i_usb_phy_probe(struct platform_device 
*pdev)
 
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy_ctrl");
data->base = devm_ioremap_resource(dev, res);
-   if (IS_ERR(data->base))
+   if (IS_ERR(data->base)) {
+   dev_err(dev, "Couldn't map our registers\n");
return PTR_ERR(data->base);
+   }
 
data->id_det_gpio = devm_gpiod_get_optional(dev, "usb0_id_det",
GPIOD_IN);
-   if (IS_ERR(data->id_det_gpio))
+   if (IS_ERR(data->id_det_gpio)) {
+   dev_err(dev, "Couldn't request ID GPIO\n");
return PTR_ERR(data->id_det_gpio);
+   }
 
data->vbus_det_gpio = devm_gpiod_get_optional(dev, "usb0_vbus_det",
  GPIOD_IN);
-   if (IS_ERR(data->vbus_det_gpio))
+   if (IS_ERR(data->vbus_det_gpio)) {
+   dev_err(dev, "Couldn't request VBUS detect GPIO\n");
return PTR_ERR(data->vbus_det_gpio);
+   }
 
if (of_find_property(np, "usb0_vbus_power-supply", NULL)) {
data->vbus_power_supply = devm_power_supply_get_by_phandle(dev,
 "usb0_vbus_power-supply");
-   if (IS_ERR(data->vbus_power_supply))
+   if (IS_ERR(data->vbus_power_supply)) {
+   dev_err(dev, "Couldn't get the VBUS power supply\n");
return PTR_ERR(data->vbus_power_supply);
+   }
 
if (!data->vbus_power_supply)
return -EPROBE_DEFER;
@@ -584,8 +592,10 @@ static int sun4i_usb_phy_probe(struct platform_device 
*pdev)
if (data->id_det_gpio) {
data->extcon = devm_extcon_dev_allocate(dev,
sun4i_usb_phy0_cable);
-   if (IS_ERR(data->extcon))
+   if (IS_ERR(data->extcon)) {
+   dev_err(dev, "Couldn't allocate our extcon device\n");
return PTR_ERR(data->extcon);
+   }
 
ret = devm_extcon_dev_register(dev, data->extcon);
if (ret) {
@@ -601,8 +611,12 @@ static int sun4i_usb_phy_probe(struct platform_device 
*pdev)
snprintf(name, sizeof(name), "usb%d_vbus", i);
phy->vbus = devm_regulator_get_optional(dev, name);
if (IS_ERR(phy->vbus)) {
-   if (PTR_ERR(phy->vbus) == -EPROBE_DEFER)
+   if (PTR_ERR(phy->vbus) == -EPROBE_DEFER) {
+   dev_err(dev, "Couldn't get regulator %s... 
Deferring probe\n",
+   name);
return -EPROBE_DEFER;
+   }
+
phy->vbus = NULL;
}
 
@@ -690,6 +704,8 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
return PTR_ERR(phy_provider);
}
 
+   dev_info(dev, "successfully loaded\n");
+
return 0;
 }
 
-- 
2.5.0



[PATCH] phy: phy-sun4i-usb: Fix optional gpios failing probe

2016-06-13 Thread Quentin Schulz
The interrupt 0 is not a valid interrupt number. In the event where the
retrieval of the vbus-det gpio would return null, the gpiod_to_irq
callback would return 0, while the current code makes the assumption
that it is a valid interrupt, and would go on calling request_irq.
Obviously, this would fail, preventing the driver from probing properly,
while the vbus and id gpios are optional.

Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
---
 drivers/phy/phy-sun4i-usb.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index bae54f7..45f01d6 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -645,11 +645,11 @@ static int sun4i_usb_phy_probe(struct platform_device 
*pdev)
 
data->id_det_irq = gpiod_to_irq(data->id_det_gpio);
data->vbus_det_irq = gpiod_to_irq(data->vbus_det_gpio);
-   if ((data->id_det_gpio && data->id_det_irq < 0) ||
-   (data->vbus_det_gpio && data->vbus_det_irq < 0))
+   if ((data->id_det_gpio && data->id_det_irq <= 0) ||
+   (data->vbus_det_gpio && data->vbus_det_irq <= 0))
data->phy0_poll = true;
 
-   if (data->id_det_irq >= 0) {
+   if (data->id_det_irq > 0) {
ret = devm_request_irq(dev, data->id_det_irq,
sun4i_usb_phy0_id_vbus_det_irq,
IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
@@ -660,7 +660,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
}
}
 
-   if (data->vbus_det_irq >= 0) {
+   if (data->vbus_det_irq > 0) {
ret = devm_request_irq(dev, data->vbus_det_irq,
sun4i_usb_phy0_id_vbus_det_irq,
IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
-- 
2.5.0



[PATCH v2] ARM: sun8i: Add Parrot Board DTS

2016-06-22 Thread Quentin Schulz
The Parrot Board is an evaluation board with an Allwinner R16 (assumed
to be close to an Allwinner A33), 4GB of eMMC, 512MB of RAM, USB host
and OTG, a WiFi/Bluetooth combo chip, a micro SD Card reader, 2
controllable buttons, an LVDS port with separated backlight and
capacitive touch panel ports, an audio/microphone jack, a camera CSI
port, 2 sets of 22 GPIOs and an accelerometer.

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

Patch dependencies:
 - regulator: axp20x: Add support for the (external) drivebus regulator
 - ARM: dts: axp22x.dtsi: Add reg_drivebus node
 - ARM: dts: axp22x.dtsi: Add usb_power_supply node
 - power: axp20x_usb: Add support for usb power-supply on axp22x pmics
 - mfd: axp20x: Extend axp22x_volatile_ranges
 - mfd: axp20x: Add axp20x-usb-power-supply for axp22x pmics
 - phy-sun4i-usb: Add workaround for missing Vbus det interrupts on A31
or the micro USB port would not be powered.

v2:
 - correct typos,
 - chose less generic name suffix for nodes (now_parrot_r16),
 - add comment for i2c childless node,
 - add comment for two regulators-powered WiFi chip,
 - add comment to explicit the use of some regulators,
 - add usb_vbus_power-supply property in usbphy node (this adds a
   dependance to an other patch),

 arch/arm/boot/dts/Makefile |   1 +
 arch/arm/boot/dts/sun8i-r16-parrot.dts | 356 +
 2 files changed, 357 insertions(+)
 create mode 100644 arch/arm/boot/dts/sun8i-r16-parrot.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 1ff5434..6c7581f 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -762,6 +762,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
sun8i-a33-ippo-q8h-v1.2.dtb \
sun8i-a33-q8-tablet.dtb \
sun8i-a33-sinlinx-sina33.dtb \
+   sun8i-r16-parrot.dtb \
sun8i-a83t-allwinner-h8homlet-v2.dtb \
sun8i-a83t-cubietruck-plus.dtb \
sun8i-h3-orangepi-2.dtb \
diff --git a/arch/arm/boot/dts/sun8i-r16-parrot.dts 
b/arch/arm/boot/dts/sun8i-r16-parrot.dts
new file mode 100644
index 000..cd68bfa
--- /dev/null
+++ b/arch/arm/boot/dts/sun8i-r16-parrot.dts
@@ -0,0 +1,356 @@
+/*
+ * Copyright 2016 Quentin Schulz
+ *
+ * Quentin Schulz <quentin.sch...@free-electrons.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file 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.
+ *
+ * This file 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.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "sun8i-a33.dtsi"
+#include "sunxi-common-regulators.dtsi"
+
+#include 
+#include 
+
+/ {
+   model = "Allwinner Parrot EVB R16";
+   compatible = "allwinner,parrot-evb-r16", "allwinner,sun8i-a33";
+
+   aliases {
+   serial0 = 
+   };
+
+   chosen {
+   stdout-path = "serial0:115200n8";
+   };
+
+   leds {
+   compatible = "gpio-leds";
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins_parrot_r16>;
+
+   led1 {
+   label = "parrot_r

[PATCH v3] ARM: sun8i: Add Parrot Board DTS

2016-06-24 Thread Quentin Schulz
The Parrot Board is an evaluation board with an Allwinner R16 (assumed
to be close to an Allwinner A33), 4GB of eMMC, 512MB of RAM, USB host
and OTG, a WiFi/Bluetooth combo chip, a micro SD Card reader, 2
controllable buttons, an LVDS port with separated backlight and
capacitive touch panel ports, an audio/microphone jack, a camera CSI
port, 2 sets of 22 GPIOs and an accelerometer.

Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
Acked-by: Chen-Yu Tsai <w...@csie.org>
---

Patch dependency:
 - phy-sun4i-usb: Add workaround for missing Vbus det interrupts on A31
or the micro USB port would not be powered.

v2:
 - correct typos,
 - chose less generic name suffix for nodes (now_parrot_r16),
 - add comment for i2c childless node,
 - add comment for two regulators-powered WiFi chip,
 - add comment to explicit the use of some regulators,
 - add usb_vbus_power-supply property in usbphy node (this adds a
   dependance to an other patch),

v3:
 - change compatible name from "allwinner,parrot-evb-r16" to
   "allwinner,parrot",
 - rename name suffix for nodes (now _parrot),
 - edited and removed comments,
 - removed dependencies that are already merged,
 - re-order alphabetically the DTB Makefile,

 arch/arm/boot/dts/Makefile |   3 +-
 arch/arm/boot/dts/sun8i-r16-parrot.dts | 348 +
 2 files changed, 350 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/sun8i-r16-parrot.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 2b6472c..bbabc58 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -777,7 +777,8 @@ dtb-$(CONFIG_MACH_SUN8I) += \
sun8i-h3-orangepi-one.dtb \
sun8i-h3-orangepi-pc.dtb \
sun8i-h3-orangepi-plus.dtb \
-   sun8i-h3-sinovoip-bpi-m2-plus.dtb
+   sun8i-h3-sinovoip-bpi-m2-plus.dtb \
+   sun8i-r16-parrot.dtb
 dtb-$(CONFIG_MACH_SUN9I) += \
sun9i-a80-optimus.dtb \
sun9i-a80-cubieboard4.dtb
diff --git a/arch/arm/boot/dts/sun8i-r16-parrot.dts 
b/arch/arm/boot/dts/sun8i-r16-parrot.dts
new file mode 100644
index 000..39c40be
--- /dev/null
+++ b/arch/arm/boot/dts/sun8i-r16-parrot.dts
@@ -0,0 +1,348 @@
+/*
+ * Copyright 2016 Quentin Schulz
+ *
+ * Quentin Schulz <quentin.sch...@free-electrons.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file 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.
+ *
+ * This file 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.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "sun8i-a33.dtsi"
+#include "sunxi-common-regulators.dtsi"
+
+#include 
+#include 
+
+/ {
+   model = "Allwinner R16 EVB (Parrot)";
+   compatible = "allwinner,parrot", "allwinner,sun8i-a33";
+
+   aliases {
+   serial0 = 
+   };
+
+   chosen {
+   stdout-path = "serial0:115200n8";
+   };
+
+   leds {
+   compatible = "gpio-leds";
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins_parrot>;
+
+   led1 {
+   

[PATCH 0/3] add support for Allwinner SoCs ADC

2016-06-28 Thread Quentin Schulz
The Allwinner SoCs all have an ADC that can also act as a touchscreen controller
and a thermal sensor. The first four channels can be used either for the ADC or
the touchscreen and the fifth channel is used for the thermal sensor. We
currently have a driver for the two latter functions in
drivers/input/touchscreen/sun4i-ts.c but we don't have access to the ADC feature
at all.

This adds initial support for Allwinner SoCs ADC with all features. Yet, the
touchscreen is not implemented but will be added later. To switch between
touchscreen and ADC modes, you need to poke few bits in registers and
(de)activate an interrupt (pen-up).
A MFD is provided to let the input driver activate the pen-up interrupt through
virtual interrupt, poke few bits via regmap and read data from the ADC driver
while both (and iio_hwmon) are probed by the MFD.

There exists slight modifications between the different SoCs ADC like the
address of some registers and the scale and offset to apply to thermal sensor
raw values. These modifications are done by drivers on different
platform_device_id passed by the MFD when probing subdrivers.

This also modifies iio-hwmon to allow probe deferring when no iio channel is
found. Currently when no iio channel is found, the probing of iio-hwmon fails.
This is problematic when iio-hwmon probes before the iio driver could register
iio channels to share.

Quentin Schulz (3):
  mfd: add support for Allwinner SoCs ADC
  iio: adc: add support for Allwinner SoCs ADC
  hwmon: iio_hwmon: defer probe when no channel is found

 drivers/hwmon/iio_hwmon.c   |   5 +-
 drivers/iio/adc/Kconfig |  12 ++
 drivers/iio/adc/Makefile|   1 +
 drivers/iio/adc/sunxi-gpadc-iio.c   | 371 
 drivers/mfd/Kconfig |  14 ++
 drivers/mfd/Makefile|   2 +
 drivers/mfd/sunxi-gpadc-mfd.c   | 188 ++
 include/linux/mfd/sunxi-gpadc-mfd.h |  14 ++
 8 files changed, 606 insertions(+), 1 deletion(-)
 create mode 100644 drivers/iio/adc/sunxi-gpadc-iio.c
 create mode 100644 drivers/mfd/sunxi-gpadc-mfd.c
 create mode 100644 include/linux/mfd/sunxi-gpadc-mfd.h

-- 
2.5.0



[PATCH 2/3] iio: adc: add support for Allwinner SoCs ADC

2016-06-28 Thread Quentin Schulz
The Allwinner SoCs all have an ADC that can also act as a touchscreen
controller and a thermal sensor. This patch adds the ADC driver which is
based on the MFD for the same SoCs ADC.

This also registers the thermal adc channel in the iio map array so
iio_hwmon could use it without modifying the Device Tree.

This driver probes on three different platform_device_id to take into
account slight differences between Allwinner SoCs ADCs.

Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
---
 drivers/iio/adc/Kconfig   |  12 ++
 drivers/iio/adc/Makefile  |   1 +
 drivers/iio/adc/sunxi-gpadc-iio.c | 371 ++
 3 files changed, 384 insertions(+)
 create mode 100644 drivers/iio/adc/sunxi-gpadc-iio.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 82c718c..b7b566a 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -328,6 +328,18 @@ config NAU7802
  To compile this driver as a module, choose M here: the
  module will be called nau7802.
 
+config SUNXI_ADC
+   tristate "ADC driver for sunxi platforms"
+   depends on IIO
+   depends on MFD_SUNXI_ADC
+   help
+ Say yes here to build support for Allwinner SoCs (A10, A13 and A31)
+ SoCs ADC. This ADC provides 4 channels which can be used as an ADC or
+ as a touchscreen input and one channel for thermal sensor.
+
+  To compile this driver as a module, choose M here: the
+  module will be called sunxi-gpadc-iio.
+
 config PALMAS_GPADC
tristate "TI Palmas General Purpose ADC"
depends on MFD_PALMAS
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 0cb7921..2996a5b 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_MCP3422) += mcp3422.o
 obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
 obj-$(CONFIG_MXS_LRADC) += mxs-lradc.o
 obj-$(CONFIG_NAU7802) += nau7802.o
+obj-$(CONFIG_SUNXI_ADC) += sunxi-gpadc-iio.o
 obj-$(CONFIG_PALMAS_GPADC) += palmas_gpadc.o
 obj-$(CONFIG_QCOM_SPMI_IADC) += qcom-spmi-iadc.o
 obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
diff --git a/drivers/iio/adc/sunxi-gpadc-iio.c 
b/drivers/iio/adc/sunxi-gpadc-iio.c
new file mode 100644
index 000..5840f43
--- /dev/null
+++ b/drivers/iio/adc/sunxi-gpadc-iio.c
@@ -0,0 +1,371 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define TP_CTRL0   0x00
+#define TP_CTRL1   0x04
+#define TP_CTRL2   0x08
+#define TP_CTRL3   0x0c
+#define TP_TPR 0x18
+#define TP_CDAT0x1c
+#define TEMP_DATA  0x20
+#define TP_DATA0x24
+
+/* TP_CTRL0 bits */
+#define ADC_FIRST_DLY(x)   ((x) << 24) /* 8 bits */
+#define ADC_FIRST_DLY_MODE BIT(23)
+#define ADC_CLK_SELECT BIT(22)
+#define ADC_CLK_DIVIDER(x) ((x) << 20) /* 2 bits */
+#define FS_DIV(x)  ((x) << 16) /* 4 bits */
+#define T_ACQ(x)   ((x) << 0)  /* 16 bits*/
+
+/* TP_CTRL1 bits */
+#define STYLUS_UP_DEBOUNCE(x)  ((x) << 12) /* 8 bits */
+#define STYLUS_UP_DEBOUNCE_EN  BIT(9)
+#define TOUCH_PAN_CALI_EN  BIT(6)
+#define TP_DUAL_EN BIT(5)
+#define TP_MODE_EN BIT(4)
+#define TP_ADC_SELECT  BIT(3)
+#define ADC_CHAN_SELECT(x) ((x) << 0)  /* 3 bits */
+
+/* TP_CTRL1 bits for sun6i SOCs */
+#define SUN6I_TOUCH_PAN_CALI_ENBIT(7)
+#define SUN6I_TP_DUAL_EN   BIT(6)
+#define SUN6I_TP_MODE_EN   BIT(5)
+#define SUN6I_TP_ADC_SELECTBIT(4)
+#define SUN6I_ADC_CHAN_SELECT(x)   BIT(x)  /* 4 bits */
+
+/* TP_CTRL2 bits */
+#define TP_SENSITIVE_ADJUST(x) ((x) << 28) /* 4 bits */
+#define TP_MODE_SELECT(x)  ((x) << 26) /* 2 bits */
+#define PRE_MEA_EN BIT(24)
+#define PRE_MEA_THRE_CNT(x)((x) << 0)  /* 24 bits*/
+
+/* TP_CTRL3 bits */
+#define FILTER_EN  BIT(2)
+#define FILTER_TYPE(x) ((x) << 0)  /* 2 bits */
+
+/* TP_INT_FIFOC irq and fifo mask / control bits */
+#define TEMP_IRQ_ENBIT(18)
+#define TP_OVERRUN_IRQ_EN  BIT(17)
+#define TP_DATA_IRQ_EN BIT(16)
+#define TP_DATA_XY_CHANGE  BIT(13)
+#define TP_FIFO_TRIG_LEVEL(x)  ((x) << 8)  /* 5 bits */
+#define TP_DATA_DRQ_EN BIT(7)
+#define TP_FIFO_FLUSH  BIT(4)
+#define TP_UP_IRQ_EN   BIT(1)
+#define TP_DOWN_IRQ_EN BIT(0)
+
+/* TP_INT_FIFOS irq and fifo status bits */
+#

[PATCH 3/3] hwmon: iio_hwmon: defer probe when no channel is found

2016-06-28 Thread Quentin Schulz
iio_channel_get_all returns -ENODEV when it cannot find either phandles and
properties in the Device Tree or channels whose consumer_dev_name matches
iio_hwmon in iio_map_list. The iio_map_list is filled in by iio drivers
which might be probed after iio_hwmon.

It is better to defer the probe of iio_hwmon if such error is returned by
iio_channel_get_all in order to let a chance to iio drivers to expose
channels in iio_map_list.

Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
---
 drivers/hwmon/iio_hwmon.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/iio_hwmon.c b/drivers/hwmon/iio_hwmon.c
index b550ba5..c0da4d9 100644
--- a/drivers/hwmon/iio_hwmon.c
+++ b/drivers/hwmon/iio_hwmon.c
@@ -73,8 +73,11 @@ static int iio_hwmon_probe(struct platform_device *pdev)
name = dev->of_node->name;
 
channels = iio_channel_get_all(dev);
-   if (IS_ERR(channels))
+   if (IS_ERR(channels)) {
+   if (PTR_ERR(channels) == -ENODEV)
+   return -EPROBE_DEFER;
return PTR_ERR(channels);
+   }
 
st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
if (st == NULL) {
-- 
2.5.0



[PATCH 1/3] mfd: add support for Allwinner SoCs ADC

2016-06-28 Thread Quentin Schulz
The Allwinner SoCs all have an ADC that can also act as a touchscreen
controller and a thermal sensor. For now, only the ADC and the thermal
sensor drivers are probed by the MFD, the touchscreen controller support
will be added later.

Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
---
 drivers/mfd/Kconfig |  14 +++
 drivers/mfd/Makefile|   2 +
 drivers/mfd/sunxi-gpadc-mfd.c   | 188 
 include/linux/mfd/sunxi-gpadc-mfd.h |  14 +++
 4 files changed, 218 insertions(+)
 create mode 100644 drivers/mfd/sunxi-gpadc-mfd.c
 create mode 100644 include/linux/mfd/sunxi-gpadc-mfd.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index eea61e3..1663db9 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -82,6 +82,20 @@ config MFD_ATMEL_FLEXCOM
  by the probe function of this MFD driver according to a device tree
  property.
 
+config MFD_SUNXI_ADC
+   tristate "ADC MFD core driver for sunxi platforms"
+   select MFD_CORE
+   select REGMAP_MMIO
+   help
+ Select this to get support for Allwinner SoCs (A10, A13 and A31) ADC.
+ This driver will only map the hardware interrupt and registers, you
+ have to select individual drivers based on this MFD to be able to use
+ the ADC or the thermal sensor. This will try to probe the ADC driver
+ sunxi-gpadc-iio and the hwmon driver iio_hwmon.
+
+ To compile this driver as a module, choose M here: the
+ module will be called sunxi-gpadc-mfd.
+
 config MFD_ATMEL_HLCDC
tristate "Atmel HLCDC (High-end LCD Controller)"
select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 5eaa6465d..b280d0a 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -199,6 +199,8 @@ obj-$(CONFIG_MFD_DLN2)  += dln2.o
 obj-$(CONFIG_MFD_RT5033)   += rt5033.o
 obj-$(CONFIG_MFD_SKY81452) += sky81452.o
 
+obj-$(CONFIG_MFD_SUNXI_ADC)+= sunxi-gpadc-mfd.o
+
 intel-soc-pmic-objs:= intel_soc_pmic_core.o intel_soc_pmic_crc.o
 intel-soc-pmic-$(CONFIG_INTEL_PMC_IPC) += intel_soc_pmic_bxtwc.o
 obj-$(CONFIG_INTEL_SOC_PMIC)   += intel-soc-pmic.o
diff --git a/drivers/mfd/sunxi-gpadc-mfd.c b/drivers/mfd/sunxi-gpadc-mfd.c
new file mode 100644
index 000..710e297
--- /dev/null
+++ b/drivers/mfd/sunxi-gpadc-mfd.c
@@ -0,0 +1,188 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define SUNXI_IRQ_FIFO_DATA0
+#define SUNXI_IRQ_TEMP_DATA1
+
+static struct resource adc_resources[] = {
+   {
+   .name   = "FIFO_DATA_PENDING",
+   .start  = SUNXI_IRQ_FIFO_DATA,
+   .end= SUNXI_IRQ_FIFO_DATA,
+   .flags  = IORESOURCE_IRQ,
+   }, {
+   .name   = "TEMP_DATA_PENDING",
+   .start  = SUNXI_IRQ_TEMP_DATA,
+   .end= SUNXI_IRQ_TEMP_DATA,
+   .flags  = IORESOURCE_IRQ,
+   },
+};
+
+static const struct regmap_irq sunxi_gpadc_mfd_regmap_irq[] = {
+   REGMAP_IRQ_REG(SUNXI_IRQ_FIFO_DATA, 0, BIT(16)),
+   REGMAP_IRQ_REG(SUNXI_IRQ_TEMP_DATA, 0, BIT(18)),
+};
+
+static const struct regmap_irq_chip sunxi_gpadc_mfd_regmap_irq_chip = {
+   .name = "sunxi_gpadc_mfd_irq_chip",
+   .status_base = TP_INT_FIFOS,
+   .ack_base = TP_INT_FIFOS,
+   .mask_base = TP_INT_FIFOC,
+   .init_ack_masked = true,
+   .mask_invert = true,
+   .irqs = sunxi_gpadc_mfd_regmap_irq,
+   .num_irqs = ARRAY_SIZE(sunxi_gpadc_mfd_regmap_irq),
+   .num_regs = 1,
+};
+
+static struct mfd_cell sun4i_gpadc_mfd_cells[] = {
+   {
+   .name   = "sun4i-a10-gpadc-iio",
+   .resources = adc_resources,
+   .num_resources = ARRAY_SIZE(adc_resources),
+   }, {
+   .name = "iio_hwmon",
+   }
+};
+
+static struct mfd_cell sun5i_gpadc_mfd_cells[] = {
+   {
+   .name   = "sun5i-a13-gpadc-iio",
+   .resources = adc_resources,
+   .num_resources = ARRAY_SIZE(adc_resources),
+   }, {
+   .name = "iio_hwmon",
+   },
+};
+
+static struct mfd_cell sun6i_gpadc_mfd_cells[] = {
+   {
+   .name   = "sun6i-a31-gpadc-iio",
+   .resources = adc_resources,
+   .num_resources = ARRAY_SIZE(adc_resources),
+   }, {
+   .name = "iio_hwmon",
+   },
+};
+
+static const struct regmap_config sunxi_gpadc_mfd_regmap_config = {
+   .reg_bits = 32,
+   .val_bits = 32,
+   .reg_stride = 4,
+   .fast_io = true,
+};
+
+static int sunxi_gpadc_mfd_probe(struct platform_device *pdev)
+{
+   struct sunxi_gpadc_mfd_dev *sunxi_gpadc_mfd_dev = NULL;
+   struct resource *mem = NULL;
+   unsigned int irq;
+   int ret;
+
+  

[PATCH v3 1/4] hwmon: iio_hwmon: delay probing with late_initcall

2016-07-26 Thread Quentin Schulz
iio_channel_get_all returns -ENODEV when it cannot find either phandles and
properties in the Device Tree or channels whose consumer_dev_name matches
iio_hwmon in iio_map_list. The iio_map_list is filled in by iio drivers
which might be probed after iio_hwmon.

This makes sure iio_hwmon is probed after all iio drivers which provides
channels to iio_hwmon are probed, be they present in the DT or using
iio_map_list.

This replaces module_platform_driver() by an explicit code variant which
calls late_initcall() install of module_init(), meaning it probes after
all the drivers using module_init() as their init.

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

v3:
 - use late_initcall instead of deferring probe,

 drivers/hwmon/iio_hwmon.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/iio_hwmon.c b/drivers/hwmon/iio_hwmon.c
index b550ba5..0a00bfb 100644
--- a/drivers/hwmon/iio_hwmon.c
+++ b/drivers/hwmon/iio_hwmon.c
@@ -192,7 +192,21 @@ static struct platform_driver __refdata iio_hwmon_driver = 
{
.remove = iio_hwmon_remove,
 };
 
-module_platform_driver(iio_hwmon_driver);
+static struct platform_driver * const drivers[] = {
+   _hwmon_driver,
+};
+
+static int __init iio_hwmon_late_init(void)
+{
+   return platform_register_drivers(drivers, ARRAY_SIZE(drivers));
+}
+late_initcall(iio_hwmon_late_init);
+
+static void __exit iio_hwmon_exit(void)
+{
+   platform_unregister_drivers(drivers, ARRAY_SIZE(drivers));
+}
+module_exit(iio_hwmon_exit);
 
 MODULE_AUTHOR("Jonathan Cameron <ji...@kernel.org>");
 MODULE_DESCRIPTION("IIO to hwmon driver");
-- 
2.5.0



[PATCH v3 0/4] add support for Allwinner SoCs ADC

2016-07-26 Thread Quentin Schulz
The Allwinner SoCs all have an ADC that can also act as a touchscreen
controller and a thermal sensor. The first four channels can be used either
for the ADC or the touchscreen and the fifth channel is used for the
thermal sensor. We currently have a driver for the two latter functions in
drivers/input/touchscreen/sun4i-ts.c but we don't have access to the ADC
feature at all. It is meant to replace the current driver by using MFD and
subdrivers.

This adds initial support for Allwinner SoCs ADC with all features. Yet,
the touchscreen is not implemented but will be added later. To switch
between touchscreen and ADC modes, you need to poke a few bits in registers
and (de)activate an interrupt (pen-up).
An MFD is provided to let the input driver activate the pen-up interrupt
through a virtual interrupt, poke a few bits via regmap and read data from
the ADC driver while both (and iio_hwmon) are probed by the MFD.

There are slight variations between the different SoCs ADC like the address
of some registers and the scale and offset to apply to raw thermal sensor
values. These variations are handled by using different platform_device_id,
passed to the sub-drivers when they are probed by the MFD.

Currently when no iio channel is found, the probing of iio-hwmon fails.
This is problematic when iio-hwmon probes before the iio driver could
register iio channels to share. Thus, this modifies iio-hwmon to probe late
with late_initcall making sure all drivers which provides iio channels have
probed before iio_hwmon.

When an MFD cell has an of_compatible (meaning it is present in the Device
Tree), other nodes can reference it using a phandle.
However when the MFD cell is not declared in the Device Tree, the only way
other nodes can reference it are by using a phandle to the MFD. Then when
this MFD cell tries to register itself in one framework, the registration
is denied by the framework because it is not matching the of_node of the
node which is referenced by the phandle in one of the other nodes.
This reattaches the of_node of the MFD to the MFD cell device structure
when the MFD cell has no of_compatible.
We need this modification to register the thermal sensor in the thermal
framework.

Removal of proposed patch for iio_hwmon's iio channel's label in v3. The
patch induces irreversible ABI changes and will be handled as a separate
patch since I think it is not absolutely necessary to have labels yet in
iio_hwmon.

Quentin Schulz (4):
  hwmon: iio_hwmon: delay probing with late_initcall
  mfd: add support for Allwinner SoCs ADC
  mfd: mfd-core: reattach mfd of_node to cells without of_compatible
  iio: adc: add support for Allwinner SoCs ADC

 drivers/hwmon/iio_hwmon.c   |  16 +-
 drivers/iio/adc/Kconfig |  12 +
 drivers/iio/adc/Makefile|   1 +
 drivers/iio/adc/sunxi-gpadc-iio.c   | 513 
 drivers/mfd/Kconfig |  15 ++
 drivers/mfd/Makefile|   2 +
 drivers/mfd/mfd-core.c  |  14 +-
 drivers/mfd/sunxi-gpadc-mfd.c   | 189 +
 include/linux/mfd/sunxi-gpadc-mfd.h |  94 +++
 9 files changed, 850 insertions(+), 6 deletions(-)
 create mode 100644 drivers/iio/adc/sunxi-gpadc-iio.c
 create mode 100644 drivers/mfd/sunxi-gpadc-mfd.c
 create mode 100644 include/linux/mfd/sunxi-gpadc-mfd.h

-- 
2.5.0



[PATCH v3 4/4] iio: adc: add support for Allwinner SoCs ADC

2016-07-26 Thread Quentin Schulz
The Allwinner SoCs all have an ADC that can also act as a touchscreen
controller and a thermal sensor. This patch adds the ADC driver which is
based on the MFD for the same SoCs ADC.

This also registers the thermal adc channel in the iio map array so
iio_hwmon could use it without modifying the Device Tree. This registers
the driver in the thermal framework.

This driver probes on three different platform_device_id to take into
account slight differences (registers bit and temperature computation)
between Allwinner SoCs ADCs.

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

I don't like how I get the temperature for the thermal framework
(sunxi_gpadc_get_temp). I am doing the exact same process as the iio_hwmon
but in this function. This is duplicated code. I could use
iio_read_channel_processed but it needs to have the iio_channel structure
of the thermal sensor which I can only get with iio_channel_get which
matches the device name with the consumer_dev_name in the iio_map array.
And frankly, I do not see myself declaring the driver both as the producer
and the consumer of IIO channels.

Moreover, since the thermal sensor is configured to throw an interrupt
every X seconds, only the first request within these X seconds will not
time out. This means that because the thermal framework regularly poll the
thermal sensor, it is really difficult to get a value from sysfs without
getting first a tonne of times out. I don't know if we can do something
about that?

v3:
 - correct wrapping,
 - add comment about thermal sensor inner working,
 - move defines in mfd header,
 - use structure to define SoC specific registers or behaviour,
 - attach this structure to the device according to of_device_id of the
   platform device,
 - use new mutex instead of iio_dev mutex,
 - use atomic flags to avoid race between request_irq and disable_irq in
   probe,
 - switch from processed value to raw, offset and scale values for
   temperature ADC channel,
 - remove faulty sentinel in iio_chan_spec array,
 - add pm_runtime support,
 - register thermal sensor in thermal framework (forgotten since the
   beginning whereas it is present in current sun4i-ts driver),
 - remove useless ret variables to store return value of regmap_reads,
 - move comments on thermal sensor acquisition period in code instead of
   header,
 - adding goto label to unregister iio_map_array when failing to register
   iio_dev,

v2:
 - add SUNXI_GPADC_ prefixes for defines,
 - correct typo in Kconfig,
 - reorder alphabetically includes, makefile,
 - add license header,
 - fix architecture variations not being handled in interrupt handlers or
   read raw functions,
 - fix unability to return negative values from thermal sensor,
 - add gotos to reduce code repetition,
 - fix irq variable being unsigned int instead of int,
 - remove useless dev_err and dev_info,
 - deactivate all interrupts if probe fails,
 - fix iio_device_register on NULL variable,
 - deactivate ADC in the IP when probe fails or when removing driver,

 drivers/iio/adc/Kconfig   |  12 +
 drivers/iio/adc/Makefile  |   1 +
 drivers/iio/adc/sunxi-gpadc-iio.c | 513 ++
 3 files changed, 526 insertions(+)
 create mode 100644 drivers/iio/adc/sunxi-gpadc-iio.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 25378c5..429ef16 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -384,6 +384,18 @@ config ROCKCHIP_SARADC
  To compile this driver as a module, choose M here: the
  module will be called rockchip_saradc.
 
+config SUN4I_GPADC
+   tristate "Support for the Allwinner SoCs GPADC"
+   depends on IIO
+   depends on MFD_SUN4I_GPADC
+   help
+ Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
+ GPADC. This ADC provides 4 channels which can be used as an ADC or as
+ a touchscreen input and one channel for thermal sensor.
+
+ To compile this driver as a module, choose M here: the module will be
+ called sunxi-gpadc-iio.
+
 config TI_ADC081C
tristate "Texas Instruments ADC081C/ADC101C/ADC121C family"
depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 38638d4..14d1739 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -37,6 +37,7 @@ obj-$(CONFIG_PALMAS_GPADC) += palmas_gpadc.o
 obj-$(CONFIG_QCOM_SPMI_IADC) += qcom-spmi-iadc.o
 obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
 obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
+obj-$(CONFIG_SUN4I_GPADC) += sunxi-gpadc-iio.o
 obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
 obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
 obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
diff --git a/drivers/iio/adc/sunxi-gpadc-iio.c 
b/drivers/iio/adc/sunxi-gpadc-iio.c
new file mode 100644
index 000..5647688
--- /dev/null
+++ b/drivers/iio/adc/sunxi-gpadc-iio.c
@@ -0,0 +1,513 @@
+/* ADC 

[PATCH v3 2/4] mfd: add support for Allwinner SoCs ADC

2016-07-26 Thread Quentin Schulz
The Allwinner SoCs all have an ADC that can also act as a touchscreen
controller and a thermal sensor. For now, only the ADC and the thermal
sensor drivers are probed by the MFD, the touchscreen controller support
will be added later.

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

v3:
 - use defines in regmap_irq instead of hard coded BITs,
 - use of_device_id data field to chose which MFD cells to add considering
   the compatible responsible of the MFD probe,
 - remove useless initializations,
 - disable all interrupts before adding them to regmap_irqchip,
 - add goto error label in probe,
 - correct wrapping in header license,
 - move defines from IIO driver to header,
 - use GENMASK to limit the size of the variable passed to a macro,
 - prefix register BIT defines with the name of the register,
 - reorder defines,

v2:
 - add license headers,
 - reorder alphabetically includes,
 - add SUNXI_GPADC_ prefixes for defines,

 drivers/mfd/Kconfig |  15 +++
 drivers/mfd/Makefile|   2 +
 drivers/mfd/sunxi-gpadc-mfd.c   | 189 
 include/linux/mfd/sunxi-gpadc-mfd.h |  94 ++
 4 files changed, 300 insertions(+)
 create mode 100644 drivers/mfd/sunxi-gpadc-mfd.c
 create mode 100644 include/linux/mfd/sunxi-gpadc-mfd.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 1bcf601..6180c2d 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -29,6 +29,21 @@ config MFD_ACT8945A
  linear regulators, along with a complete ActivePath battery
  charger.
 
+config MFD_SUN4I_GPADC
+   tristate "Allwinner sunxi platforms' GPADC MFD driver"
+   select MFD_CORE
+   select REGMAP_MMIO
+   depends on ARCH_SUNXI || COMPILE_TEST
+   help
+ Select this to get support for Allwinner SoCs (A10, A13 and A31) ADC.
+ This driver will only map the hardware interrupt and registers, you
+ have to select individual drivers based on this MFD to be able to use
+ the ADC or the thermal sensor. This will try to probe the ADC driver
+ sunxi-gpadc-iio and the hwmon driver iio_hwmon.
+
+ To compile this driver as a module, choose M here: the module will be
+ called sunxi-gpadc-mfd.
+
 config MFD_AS3711
bool "AMS AS3711"
select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 42a66e1..9dfd033 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -205,3 +205,5 @@ intel-soc-pmic-objs := intel_soc_pmic_core.o 
intel_soc_pmic_crc.o
 intel-soc-pmic-$(CONFIG_INTEL_PMC_IPC) += intel_soc_pmic_bxtwc.o
 obj-$(CONFIG_INTEL_SOC_PMIC)   += intel-soc-pmic.o
 obj-$(CONFIG_MFD_MT6397)   += mt6397-core.o
+
+obj-$(CONFIG_MFD_SUN4I_GPADC)  += sunxi-gpadc-mfd.o
diff --git a/drivers/mfd/sunxi-gpadc-mfd.c b/drivers/mfd/sunxi-gpadc-mfd.c
new file mode 100644
index 000..3d70af0
--- /dev/null
+++ b/drivers/mfd/sunxi-gpadc-mfd.c
@@ -0,0 +1,189 @@
+/* ADC MFD core driver for sunxi platforms
+ *
+ * Copyright (c) 2016 Quentin Schulz <quentin.schulz@free-electrons>
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+static struct resource adc_resources[] = {
+   {
+   .name   = "FIFO_DATA_PENDING",
+   .start  = SUNXI_IRQ_FIFO_DATA,
+   .end= SUNXI_IRQ_FIFO_DATA,
+   .flags  = IORESOURCE_IRQ,
+   }, {
+   .name   = "TEMP_DATA_PENDING",
+   .start  = SUNXI_IRQ_TEMP_DATA,
+   .end= SUNXI_IRQ_TEMP_DATA,
+   .flags  = IORESOURCE_IRQ,
+   },
+};
+
+static const struct regmap_irq sunxi_gpadc_mfd_regmap_irq[] = {
+   REGMAP_IRQ_REG(SUNXI_IRQ_FIFO_DATA, 0,
+  SUNXI_GPADC_TP_INT_FIFOC_TP_DATA_IRQ_EN),
+   REGMAP_IRQ_REG(SUNXI_IRQ_TEMP_DATA, 0,
+  SUNXI_GPADC_TP_INT_FIFOC_TEMP_IRQ_EN),
+};
+
+static const struct regmap_irq_chip sunxi_gpadc_mfd_regmap_irq_chip = {
+   .name = "sunxi_gpadc_mfd_irq_chip",
+   .status_base = SUNXI_GPADC_TP_INT_FIFOS,
+   .ack_base = SUNXI_GPADC_TP_INT_FIFOS,
+   .mask_base = SUNXI_GPADC_TP_INT_FIFOC,
+   .init_ack_masked = true,
+   .mask_invert = true,
+   .irqs = sunxi_gpadc_mfd_regmap_irq,
+   .num_irqs = ARRAY_SIZE(sunxi_gpadc_mfd_regmap_irq),
+   .num_regs = 1,
+};
+
+static struct mfd_cell sun4i_gpadc_mfd_cells[] = {
+   {
+   .name   = "sun4i-a10-gpadc-iio",
+   .resources = adc_resources,
+   .num_resources = ARRAY_SIZE(adc_resources),
+   }, {
+   .name = "iio_hwmon",
+   }
+};
+
+static struct m

Re: [PATCH v3 1/4] hwmon: iio_hwmon: delay probing with late_initcall

2016-07-26 Thread Quentin Schulz
Hi,

On 26/07/2016 09:48, Thomas Petazzoni wrote:
> Hello,
> 
> On Tue, 26 Jul 2016 09:43:44 +0200, Quentin Schulz wrote:
> 
>> -module_platform_driver(iio_hwmon_driver);
>> +static struct platform_driver * const drivers[] = {
>> +_hwmon_driver,
>> +};
>> +
>> +static int __init iio_hwmon_late_init(void)
>> +{
>> +return platform_register_drivers(drivers, ARRAY_SIZE(drivers));
> 
> Why not platform_register_driver() ?
> 
> This would avoid the need to declare an array with just one entry.

Actually, it is named platform_driver_register() as you just showed me
and that's the reason I didn't find it under platform_register_driver().

Thanks!

Quentin


[PATCH v3 3/4] mfd: mfd-core: reattach mfd of_node to cells without of_compatible

2016-07-26 Thread Quentin Schulz
When an MFD cell has an of_compatible (meaning it is present in the Device
Tree), other nodes can reference it using a phandle.

However when the MFD cell is not declared in the Device Tree, the only way
other nodes can reference it are by using a phandle to the MFD. Then when
this MFD cell tries to register itself in one framework, the registration
is denied by the framework because it is not matching the of_node of the
node which is referenced by the phandle in one of the other nodes.

This reattaches the of_node of the MFD to the MFD cell device structure
when the MFD cell has no of_compatible.

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

We need this modification to register the thermal sensor in the thermal
framework.

Added in v3.

 drivers/mfd/mfd-core.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index 3ac486a..0b19663 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -175,12 +175,16 @@ static int mfd_add_device(struct device *parent, int id,
if (ret < 0)
goto fail_res;
 
-   if (parent->of_node && cell->of_compatible) {
-   for_each_child_of_node(parent->of_node, np) {
-   if (of_device_is_compatible(np, cell->of_compatible)) {
-   pdev->dev.of_node = np;
-   break;
+   if (parent->of_node) {
+   if (cell->of_compatible) {
+   for_each_child_of_node(parent->of_node, np) {
+   if (of_device_is_compatible(np, 
cell->of_compatible)) {
+   pdev->dev.of_node = np;
+   break;
+   }
}
+   } else {
+   pdev->dev.of_node = parent->of_node;
}
}
 
-- 
2.5.0



Re: [PATCH v3 1/4] hwmon: iio_hwmon: delay probing with late_initcall

2016-07-26 Thread Quentin Schulz
On 26/07/2016 11:05, Alexander Stein wrote:
> On Tuesday 26 July 2016 10:24:48, Quentin Schulz wrote:
>> On 26/07/2016 10:21, Alexander Stein wrote:
>>> On Tuesday 26 July 2016 09:43:44, Quentin Schulz wrote:
>>>> iio_channel_get_all returns -ENODEV when it cannot find either phandles
>>>> and
>>>> properties in the Device Tree or channels whose consumer_dev_name matches
>>>> iio_hwmon in iio_map_list. The iio_map_list is filled in by iio drivers
>>>> which might be probed after iio_hwmon.
>>>
>>> Would it work if iio_channel_get_all returning ENODEV is used for
>>> returning
>>> EPROBE_DEFER in iio_channel_get_all? Using late initcalls for
>>> driver/device
>>> dependencies seems not right for me at this place.
>>
>> Then what if the iio_channel_get_all is called outside of the probe of a
>> driver? We'll have to change the error code, things we are apparently
>> trying to avoid (see v2 patches' discussions).
> 
> Maybe I didn't express my idea enough. I don't want to change the behavior of 
>  
> iio_channel_get_all at all. Just the result evaluation of iio_channel_get_all 
> in iio_hwmon_probe. I have something link the patch below in mind.
> 
> Best regards,
> Alexander
> ---
> diff --git a/drivers/hwmon/iio_hwmon.c b/drivers/hwmon/iio_hwmon.c
> index b550ba5..e32d150 100644
> --- a/drivers/hwmon/iio_hwmon.c
> +++ b/drivers/hwmon/iio_hwmon.c
> @@ -73,8 +73,12 @@ static int iio_hwmon_probe(struct platform_device *pdev)
> name = dev->of_node->name;
>  
> channels = iio_channel_get_all(dev);
> -   if (IS_ERR(channels))
> -   return PTR_ERR(channels);
> +   if (IS_ERR(channels)) {
> +   if (PTR_ERR(channels) == -ENODEV)
> +   return -EPROBE_DEFER;
> +   else
> +   return PTR_ERR(channels);
> +   }
>  
> st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
> if (st == NULL) {

Indeed, I misunderstood what you told me.

Actually, the patch you proposed is part of my v1
(https://lkml.org/lkml/2016/6/28/203) and v2
(https://lkml.org/lkml/2016/7/15/215).
Jonathan and Guenter didn't really like the idea of changing the -ENODEV
in -EPROBE_DEFER.

What I thought you were proposing was to change the -ENODEV return code
inside iio_channel_get_all. This cannot be an option since the function
might be called outside of a probe (it is not yet, but might be in the
future?).

Of what I understood, two possibilities are then possible (proposed
either by Guenter or Jonathan): either rework the iio framework to
register iio map array earlier or to use late_initcall instead of init
for the driver consuming the iio channels.

Thanks,
Quentin


[PATCH] hwmon: iio_hwmon: fix memory leak in name attribute

2016-07-26 Thread Quentin Schulz
The "name" variable's memory is now freed when the device is destructed
thanks to devm function.

Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
Reported-by: Guenter Roeck <li...@roeck-us.net>
---
 drivers/hwmon/iio_hwmon.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/hwmon/iio_hwmon.c b/drivers/hwmon/iio_hwmon.c
index 0a00bfb..777f2b5 100644
--- a/drivers/hwmon/iio_hwmon.c
+++ b/drivers/hwmon/iio_hwmon.c
@@ -110,24 +110,24 @@ static int iio_hwmon_probe(struct platform_device *pdev)
 
switch (type) {
case IIO_VOLTAGE:
-   a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
- "in%d_input",
- in_i++);
+   a->dev_attr.attr.name = devm_kasprintf(dev, GFP_KERNEL,
+  "in%d_input",
+  in_i++);
break;
case IIO_TEMP:
-   a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
- "temp%d_input",
- temp_i++);
+   a->dev_attr.attr.name = devm_kasprintf(dev, GFP_KERNEL,
+  "temp%d_input",
+  temp_i++);
break;
case IIO_CURRENT:
-   a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
- "curr%d_input",
- curr_i++);
+   a->dev_attr.attr.name = devm_kasprintf(dev, GFP_KERNEL,
+  "curr%d_input",
+  curr_i++);
break;
case IIO_HUMIDITYRELATIVE:
-   a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
- "humidity%d_input",
- humidity_i++);
+   a->dev_attr.attr.name = devm_kasprintf(dev, GFP_KERNEL,
+  
"humidity%d_input",
+  humidity_i++);
break;
default:
ret = -EINVAL;
-- 
2.5.0



Re: [PATCH v3 1/4] hwmon: iio_hwmon: delay probing with late_initcall

2016-07-26 Thread Quentin Schulz
On 26/07/2016 10:21, Alexander Stein wrote:
> On Tuesday 26 July 2016 09:43:44, Quentin Schulz wrote:
>> iio_channel_get_all returns -ENODEV when it cannot find either phandles and
>> properties in the Device Tree or channels whose consumer_dev_name matches
>> iio_hwmon in iio_map_list. The iio_map_list is filled in by iio drivers
>> which might be probed after iio_hwmon.
> 
> Would it work if iio_channel_get_all returning ENODEV is used for returning 
> EPROBE_DEFER in iio_channel_get_all? Using late initcalls for driver/device 
> dependencies seems not right for me at this place.

Then what if the iio_channel_get_all is called outside of the probe of a
driver? We'll have to change the error code, things we are apparently
trying to avoid (see v2 patches' discussions).

Thanks,
Quentin


Re: [PATCH v3 1/4] hwmon: iio_hwmon: delay probing with late_initcall

2016-07-26 Thread Quentin Schulz
On 26/07/2016 12:00, Alexander Stein wrote:
> On Tuesday 26 July 2016 11:33:59, Quentin Schulz wrote:
>> On 26/07/2016 11:05, Alexander Stein wrote:
>>> On Tuesday 26 July 2016 10:24:48, Quentin Schulz wrote:
>>>> On 26/07/2016 10:21, Alexander Stein wrote:
>>>>> On Tuesday 26 July 2016 09:43:44, Quentin Schulz wrote:
>>>>>> iio_channel_get_all returns -ENODEV when it cannot find either phandles
>>>>>> and
>>>>>> properties in the Device Tree or channels whose consumer_dev_name
>>>>>> matches
>>>>>> iio_hwmon in iio_map_list. The iio_map_list is filled in by iio drivers
>>>>>> which might be probed after iio_hwmon.
>>>>>
>>>>> Would it work if iio_channel_get_all returning ENODEV is used for
>>>>> returning
>>>>> EPROBE_DEFER in iio_channel_get_all? Using late initcalls for
>>>>> driver/device
>>>>> dependencies seems not right for me at this place.
>>>>
>>>> Then what if the iio_channel_get_all is called outside of the probe of a
>>>> driver? We'll have to change the error code, things we are apparently
>>>> trying to avoid (see v2 patches' discussions).
>>>
>>> Maybe I didn't express my idea enough. I don't want to change the behavior
>>> of iio_channel_get_all at all. Just the result evaluation of
>>> iio_channel_get_all in iio_hwmon_probe. I have something link the patch
>>> below in mind.
>>>
>>> Best regards,
>>> Alexander
>>> ---
>>> diff --git a/drivers/hwmon/iio_hwmon.c b/drivers/hwmon/iio_hwmon.c
>>> index b550ba5..e32d150 100644
>>> --- a/drivers/hwmon/iio_hwmon.c
>>> +++ b/drivers/hwmon/iio_hwmon.c
>>> @@ -73,8 +73,12 @@ static int iio_hwmon_probe(struct platform_device
>>> *pdev)
>>>
>>> name = dev->of_node->name;
>>> 
>>> channels = iio_channel_get_all(dev);
>>>
>>> -   if (IS_ERR(channels))
>>> -   return PTR_ERR(channels);
>>> +   if (IS_ERR(channels)) {
>>> +   if (PTR_ERR(channels) == -ENODEV)
>>> +   return -EPROBE_DEFER;
>>> +   else
>>> +   return PTR_ERR(channels);
>>> +   }
>>>
>>> st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
>>> if (st == NULL) {
>>
>> Indeed, I misunderstood what you told me.
>>
>> Actually, the patch you proposed is part of my v1
>> (https://lkml.org/lkml/2016/6/28/203) and v2
>> (https://lkml.org/lkml/2016/7/15/215).
>> Jonathan and Guenter didn't really like the idea of changing the -ENODEV
>> in -EPROBE_DEFER.
> 
> Thanks for the links.
> 
>> What I thought you were proposing was to change the -ENODEV return code
>> inside iio_channel_get_all. This cannot be an option since the function
>> might be called outside of a probe (it is not yet, but might be in the
>> future?).
> 
> AFAICS this is a helper function not knowing about device probing itself. And 
> it should stay at that.
> 
>> Of what I understood, two possibilities are then possible (proposed
>> either by Guenter or Jonathan): either rework the iio framework to
>> register iio map array earlier or to use late_initcall instead of init
>> for the driver consuming the iio channels.
> 
> Interestingly using this problem would not arise due to module dependencies. 
> But using late_initcall would mean this needs to be done on any driver using 
> iio channels? I would rather keep those consumers simple.

This would mean this needs to be done in any driver *using iio_map
array* to get iio channels. The other way of getting iio channels is
using properties in the Device Tree so no need for late_initcall in that
case.

Quentin


Re: [PATCH v3 4/4] iio: adc: add support for Allwinner SoCs ADC

2016-08-04 Thread Quentin Schulz
On 04/08/2016 11:56, Russell King - ARM Linux wrote:
> On Tue, Jul 26, 2016 at 09:43:47AM +0200, Quentin Schulz wrote:
>> +static int sunxi_gpadc_adc_read(struct iio_dev *indio_dev, int channel,
>> +int *val)
>> +{
>> +struct sunxi_gpadc_dev *info = iio_priv(indio_dev);
>> +int ret = 0;
>> +
>> +pm_runtime_get_sync(indio_dev->dev.parent);
>> +mutex_lock(>mutex);
>> +
>> +reinit_completion(>completion);
>> +regmap_write(info->regmap, SUNXI_GPADC_TP_CTRL1,
>> + info->soc_specific->tp_mode_en |
>> + info->soc_specific->tp_adc_select |
>> + info->soc_specific->adc_chan_select(channel));
>> +regmap_write(info->regmap, SUNXI_GPADC_TP_INT_FIFOC,
>> + SUNXI_GPADC_TP_INT_FIFOC_TP_FIFO_TRIG_LEVEL(1) |
>> + SUNXI_GPADC_TP_INT_FIFOC_TP_FIFO_FLUSH);
>> +enable_irq(info->fifo_data_irq);
> ^
> 
>> +
>> +if (!wait_for_completion_timeout(>completion,
>> + msecs_to_jiffies(100))) {
>> +ret = -ETIMEDOUT;
>> +goto out;
>> +}
>> +
>> +*val = info->adc_data;
>> +
>> +out:
>> +disable_irq(info->fifo_data_irq);
> ^^
> 
> I spotted this while skipping over the patch - and also noticed the
> below.
> 
> ...
>> +irq = platform_get_irq_byname(pdev, "TEMP_DATA_PENDING");
>> +if (irq < 0) {
>> +dev_err(>dev,
>> +"no TEMP_DATA_PENDING interrupt registered\n");
>> +ret = irq;
>> +goto err;
>> +}
>> +
>> +irq = regmap_irq_get_virq(sunxi_gpadc_mfd_dev->regmap_irqc, irq);
>> +ret = devm_request_any_context_irq(>dev, irq,
>> +   sunxi_gpadc_temp_data_irq_handler, 0,
>> +   "temp_data", info);
>> +if (ret < 0) {
>> +dev_err(>dev,
>> +"could not request TEMP_DATA_PENDING interrupt: %d\n",
>> +ret);
>> +goto err;
>> +}
>> +
>> +disable_irq(irq);
> ^^
> 
>> +info->temp_data_irq = irq;
>> +atomic_set(>ignore_temp_data_irq, 0);
>> +
>> +irq = platform_get_irq_byname(pdev, "FIFO_DATA_PENDING");
>> +if (irq < 0) {
>> +dev_err(>dev,
>> +"no FIFO_DATA_PENDING interrupt registered\n");
>> +ret = irq;
>> +goto err;
>> +}
>> +
>> +irq = regmap_irq_get_virq(sunxi_gpadc_mfd_dev->regmap_irqc, irq);
>> +ret = devm_request_any_context_irq(>dev, irq,
>> +   sunxi_gpadc_fifo_data_irq_handler, 0,
>> +   "fifo_data", info);
>> +if (ret < 0) {
>> +dev_err(>dev,
>> +"could not request FIFO_DATA_PENDING interrupt: %d\n",
>> +ret);
>> +goto err;
>> +}
>> +
>> +disable_irq(irq);
>> +info->fifo_data_irq = irq;
> 
> Firstly, claiming and then immediately disabling an interrupt handler
> looks very strange.  If you're disabling the interrupt because you're
> concerned that you may receive an unexpected interrupt, this is no
> good - consider what happens if the interrupt happens between you
> claiming and disabling it.

Indeed. This has been detected in v2
(https://lkml.org/lkml/2016/7/19/246) but since I only set values in
structures by reading registers defined beforehand, it is not a race.
However, like anything in the kernel, the driver might evolve and use
undefined variables in the interrupt handler which will introduce a
race. This potential race will be handled in v4 with atomic flags around
interrupt initializations (before requesting and after disabling). If an
interrupt occurs between the two instructions, reading the flag will
state if we need to ignore the interrupt.

> Secondly, interrupts asserted while disabled are recorded and replayed
> when you enable the interrupt, no matter when they happened (eg, they
> could occur immediately after you disabled the interrupt.)
> 
> I think you need to comment each of the sites in the driver, explaining
> why it's necessary to disable and enable the interrupt at the IRQ
> controller like this, or get rid of these enable/disable_irq() calls.

Comments for this is planned in v4.

Thanks,

Quentin


Re: [PATCH v3 4/4] iio: adc: add support for Allwinner SoCs ADC

2016-08-04 Thread Quentin Schulz
Hi Maxime,

On 29/07/2016 09:12, Maxime Ripard wrote:
> On Tue, Jul 26, 2016 at 09:43:47AM +0200, Quentin Schulz wrote:
[...]
>> +static int sunxi_gpadc_adc_read(struct iio_dev *indio_dev, int channel,
>> +int *val)
>> +{
>> +struct sunxi_gpadc_dev *info = iio_priv(indio_dev);
>> +int ret = 0;
>> +
>> +pm_runtime_get_sync(indio_dev->dev.parent);
>> +mutex_lock(>mutex);
>> +
>> +reinit_completion(>completion);
>> +regmap_write(info->regmap, SUNXI_GPADC_TP_CTRL1,
>> + info->soc_specific->tp_mode_en |
>> + info->soc_specific->tp_adc_select |
>> + info->soc_specific->adc_chan_select(channel));
>> +regmap_write(info->regmap, SUNXI_GPADC_TP_INT_FIFOC,
>> + SUNXI_GPADC_TP_INT_FIFOC_TP_FIFO_TRIG_LEVEL(1) |
>> + SUNXI_GPADC_TP_INT_FIFOC_TP_FIFO_FLUSH);
>> +enable_irq(info->fifo_data_irq);
>> +
>> +if (!wait_for_completion_timeout(>completion,
>> + msecs_to_jiffies(100))) {
>> +ret = -ETIMEDOUT;
>> +goto out;
>> +}
>> +
>> +*val = info->adc_data;
>> +
>> +out:
>> +disable_irq(info->fifo_data_irq);
>> +mutex_unlock(>mutex);
>> +pm_runtime_mark_last_busy(indio_dev->dev.parent);
>> +pm_runtime_put_autosuspend(indio_dev->dev.parent);
>> +
>> +return ret;
>> +}
>> +
>> +static int sunxi_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
>> +{
>> +struct sunxi_gpadc_dev *info = iio_priv(indio_dev);
>> +int ret = 0;
>> +
>> +pm_runtime_get_sync(indio_dev->dev.parent);
>> +mutex_lock(>mutex);
>> +
>> +reinit_completion(>completion);
>> +
>> +regmap_write(info->regmap, SUNXI_GPADC_TP_INT_FIFOC,
>> + SUNXI_GPADC_TP_INT_FIFOC_TP_FIFO_TRIG_LEVEL(1) |
>> + SUNXI_GPADC_TP_INT_FIFOC_TP_FIFO_FLUSH);
>> +/*
>> + * The temperature sensor returns valid data only when the ADC operates
>> + * in touchscreen mode.
>> + */
>> +regmap_write(info->regmap, SUNXI_GPADC_TP_CTRL1,
>> + info->soc_specific->tp_mode_en);
>> +enable_irq(info->temp_data_irq);
>> +
>> +if (!wait_for_completion_timeout(>completion,
>> + msecs_to_jiffies(100))) {
>> +ret = -ETIMEDOUT;
>> +goto out;
>> +}
>> +
>> +*val = info->temp_data;
>> +
>> +out:
>> +disable_irq(info->temp_data_irq);
>> +mutex_unlock(>mutex);
>> +pm_runtime_mark_last_busy(indio_dev->dev.parent);
>> +pm_runtime_put_autosuspend(indio_dev->dev.parent);
>> +
>> +return ret;
>> +}
> 
> This looks like the exact same function than above, can't that be
> factored (for example by passing the interrupt number as argument, and
> giving it a way to know if it's going to be used for the ADC or
> temperature as an argument?)

Yes it can. I could use the interrupt number to know in which mode to
operate since each interrupt is only activated in one mode (temperature
or ADC).

[...]
>> +static int sunxi_gpadc_runtime_suspend(struct device *dev)
>> +{
>> +struct sunxi_gpadc_dev *info = iio_priv(dev_get_drvdata(dev));
>> +
>> +mutex_lock(>mutex);
>> +
>> +/* Disable the ADC on IP */
>> +regmap_write(info->regmap, SUNXI_GPADC_TP_CTRL1, 0);
>> +/* Disable temperature sensor on IP */
>> +regmap_write(info->regmap, SUNXI_GPADC_TP_TPR, 0);
>> +
>> +mutex_unlock(>mutex);
> 
> Having some comments somewhere about what this mutex protects would be
> great (just like checkpatch tells you about).

ACK.

> However, I'm not sure this is even possible. Isn't the point of the
> runtime_pm precisely to not be called while you're using the device?

I agree on the principle but I am using runtime_pm functions (I am
mainly talking about the pm_runtime_put function) when probing or
removing the driver. Let's say we remove the mutex locks in runtime_pm
functions, what will happen if we are reading raw values from the ADC
when removing the ADC driver for example?

>> +
>> +return 0;
>> +}
>> +
>> +static int sunxi_gpadc_runtime_resume(struct device *dev)
>> +{
>> +struct sunxi_gpadc_dev *info = iio_priv(dev_get_drvdata(dev));
>> +
>> +mutex_lock(>mutex);
>> +
>>

Re: [PATCH v2 2/4] iio: adc: add support for Allwinner SoCs ADC

2016-07-21 Thread Quentin Schulz
On 20/07/2016 16:57, Jonathan Cameron wrote:
> On 19/07/16 09:33, Quentin Schulz wrote:
>> On 18/07/2016 15:18, Jonathan Cameron wrote:
>>> On 15/07/16 10:59, Quentin Schulz wrote:
[...]
>>>> +  enable_irq(info->temp_data_irq);
>>> Is this hardware spitting out extra irqs?  If not, much better to just
>>> leave it enabled all the time and control whether it can occur or not
>>> by controlling the device state..
>>
>> The temp_data_irq occurs every SUNXI_GPADC_TEMP_PERIOD(x) periods (in
>> the current state of the driver: 2s). What do you mean by controlling
>> the device state? Enabling or disabling the hardware part of the IP
>> responsible of getting the temperature
>> (SUNXI_GPADC_TP_TPR_TEMP_ENABLE(x) here)?
> Yes, or something along those lines if it wakes up fast enough.

The ADC wakes up fast enough but resets its internal time clock (I don't
know if it's the right term to use). Note that the temperature interrupt
occurs by period of X seconds in this IP.

This means that each time we disable the ADC on the hardware side, no
temperature interrupt will occur within the first X seconds. I don't
think this is what we want.

[...]

Quentin


[PATCH v2 2/4] iio: adc: add support for Allwinner SoCs ADC

2016-07-15 Thread Quentin Schulz
The Allwinner SoCs all have an ADC that can also act as a touchscreen
controller and a thermal sensor. This patch adds the ADC driver which is
based on the MFD for the same SoCs ADC.

This also registers the thermal adc channel in the iio map array so
iio_hwmon could use it without modifying the Device Tree.

This driver probes on three different platform_device_id to take into
account slight differences between Allwinner SoCs ADCs.

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

v2:
 - add SUNXI_GPADC_ prefixes for defines,
 - correct typo in Kconfig,
 - reorder alphabetically includes, makefile,
 - add license header,
 - fix architecture variations not being handled in interrupt handlers or
   read raw functions,
 - fix unability to return negative values from thermal sensor,
 - add gotos to reduce code repetition,
 - fix irq variable being unsigned int instead of int,
 - remove useless dev_err and dev_info,
 - deactivate all interrupts if probe fails,
 - fix iio_device_register on NULL variable,
 - deactivate ADC in the IP when probe fails or when removing driver,

 drivers/iio/adc/Kconfig   |  12 ++
 drivers/iio/adc/Makefile  |   1 +
 drivers/iio/adc/sunxi-gpadc-iio.c | 417 ++
 3 files changed, 430 insertions(+)
 create mode 100644 drivers/iio/adc/sunxi-gpadc-iio.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 25378c5..184856f 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -338,6 +338,18 @@ config NAU7802
  To compile this driver as a module, choose M here: the
  module will be called nau7802.
 
+config SUNXI_ADC
+   tristate "ADC driver for sunxi platforms"
+   depends on IIO
+   depends on MFD_SUNXI_ADC
+   help
+ Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
+ ADC. This ADC provides 4 channels which can be used as an ADC or as a
+ touchscreen input and one channel for thermal sensor.
+
+  To compile this driver as a module, choose M here: the module will be
+ called sunxi-gpadc-iio.
+
 config PALMAS_GPADC
tristate "TI Palmas General Purpose ADC"
depends on MFD_PALMAS
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 38638d4..3e60a1d 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -37,6 +37,7 @@ obj-$(CONFIG_PALMAS_GPADC) += palmas_gpadc.o
 obj-$(CONFIG_QCOM_SPMI_IADC) += qcom-spmi-iadc.o
 obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
 obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
+obj-$(CONFIG_SUNXI_ADC) += sunxi-gpadc-iio.o
 obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
 obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
 obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
diff --git a/drivers/iio/adc/sunxi-gpadc-iio.c 
b/drivers/iio/adc/sunxi-gpadc-iio.c
new file mode 100644
index 000..87cc913
--- /dev/null
+++ b/drivers/iio/adc/sunxi-gpadc-iio.c
@@ -0,0 +1,417 @@
+/* ADC driver for sunxi platforms
+ *
+ * Copyright (c) 2016 Quentin Schulz <quentin.schulz@free-electrons>
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#define SUNXI_GPADC_TP_CTRL0   0x00
+#define SUNXI_GPADC_TP_CTRL1   0x04
+#define SUNXI_GPADC_TP_CTRL2   0x08
+#define SUNXI_GPADC_TP_CTRL3   0x0c
+#define SUNXI_GPADC_TP_TPR 0x18
+#define SUNXI_GPADC_TP_CDAT0x1c
+#define SUNXI_GPADC_TEMP_DATA  0x20
+#define SUNXI_GPADC_TP_DATA0x24
+
+/* TP_CTRL0 bits */
+#define SUNXI_GPADC_ADC_FIRST_DLY(x)   ((x) << 24) /* 8 bits */
+#define SUNXI_GPADC_ADC_FIRST_DLY_MODE BIT(23)
+#define SUNXI_GPADC_ADC_CLK_SELECT BIT(22)
+#define SUNXI_GPADC_ADC_CLK_DIVIDER(x) ((x) << 20) /* 2 bits */
+#define SUNXI_GPADC_FS_DIV(x)  ((x) << 16) /* 4 bits */
+#define SUNXI_GPADC_T_ACQ(x)   ((x) << 0)  /* 16 bits */
+
+/* TP_CTRL1 bits */
+#define SUNXI_GPADC_STYLUS_UP_DEBOUNCE(x)  ((x) << 12) /* 8 bits */
+#define SUNXI_GPADC_STYLUS_UP_DEBOUNCE_EN  BIT(9)
+#define SUNXI_GPADC_TOUCH_PAN_CALI_EN  BIT(6)
+#define SUNXI_GPADC_TP_DUAL_EN BIT(5)
+#define SUNXI_GPADC_TP_MODE_EN BIT(4)
+#define SUNXI_GPADC_TP_ADC_SELECT  BIT(3)
+#define SUNXI_GPADC_ADC_CHAN_SELECT(x) ((x) << 0)  /* 3 bits */
+
+/* TP_CTRL1 bits for sun6i SOCs */
+#define SUNXI_GPADC_SUN6I_TOUCH_PAN_CALI_ENBIT(7)
+#define SUNXI_GPADC_SUN6I_TP_DUAL_EN   BIT(6)
+#define SUNXI_GPADC_SUN6I_TP_MODE_EN   

[PATCH v2 4/4] hwmon: iio: add label for channels read by iio_hwmon

2016-07-15 Thread Quentin Schulz
Currently, iio_hwmon only exposes values of the IIO channels it can read
but no label by channel is exposed.

This adds exposition of sysfs files containing label for IIO channels it
can read based on extended_name field of the iio_chan_spec of the channel.
If the extended_name field is empty, the sysfs file is not created by
iio_hwmon.

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

patch added in v2

 drivers/hwmon/iio_hwmon.c | 77 +--
 1 file changed, 68 insertions(+), 9 deletions(-)

diff --git a/drivers/hwmon/iio_hwmon.c b/drivers/hwmon/iio_hwmon.c
index c0da4d9..28d15b2 100644
--- a/drivers/hwmon/iio_hwmon.c
+++ b/drivers/hwmon/iio_hwmon.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /**
@@ -57,12 +58,26 @@ static ssize_t iio_hwmon_read_val(struct device *dev,
return sprintf(buf, "%d\n", result);
 }
 
+static ssize_t iio_hwmon_read_label(struct device *dev,
+   struct device_attribute *attr,
+   char *buf)
+{
+   struct sensor_device_attribute *sattr = to_sensor_dev_attr(attr);
+   struct iio_hwmon_state *state = dev_get_drvdata(dev);
+   const char *label = state->channels[sattr->index].channel->extend_name;
+
+   if (label)
+   return sprintf(buf, "%s\n", label);
+
+   return 0;
+}
+
 static int iio_hwmon_probe(struct platform_device *pdev)
 {
struct device *dev = >dev;
struct iio_hwmon_state *st;
-   struct sensor_device_attribute *a;
-   int ret, i;
+   struct sensor_device_attribute *a, *b;
+   int ret, i, j = 0;
int in_i = 1, temp_i = 1, curr_i = 1, humidity_i = 1;
enum iio_chan_type type;
struct iio_channel *channels;
@@ -92,7 +107,8 @@ static int iio_hwmon_probe(struct platform_device *pdev)
st->num_channels++;
 
st->attrs = devm_kzalloc(dev,
-sizeof(*st->attrs) * (st->num_channels + 1),
+sizeof(*st->attrs) *
+(2 * st->num_channels + 1),
 GFP_KERNEL);
if (st->attrs == NULL) {
ret = -ENOMEM;
@@ -107,6 +123,18 @@ static int iio_hwmon_probe(struct platform_device *pdev)
}
 
sysfs_attr_init(>dev_attr.attr);
+
+   b = NULL;
+   if (st->channels[i].channel->extend_name) {
+   b = devm_kzalloc(dev, sizeof(*b), GFP_KERNEL);
+   if (b == NULL) {
+   ret = -ENOMEM;
+   goto error_release_channels;
+   }
+
+   sysfs_attr_init(>dev_attr.attr);
+   }
+
ret = iio_get_channel_type(>channels[i], );
if (ret < 0)
goto error_release_channels;
@@ -115,35 +143,66 @@ static int iio_hwmon_probe(struct platform_device *pdev)
case IIO_VOLTAGE:
a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
  "in%d_input",
- in_i++);
+ in_i);
+   if (b)
+   b->dev_attr.attr.name = kasprintf(GFP_KERNEL,
+ "in%d_label",
+ in_i);
+   in_i++;
break;
case IIO_TEMP:
a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
  "temp%d_input",
- temp_i++);
+ temp_i);
+
+   if (b)
+   b->dev_attr.attr.name = kasprintf(GFP_KERNEL,
+ 
"temp%d_label",
+ temp_i);
+   temp_i++;
break;
case IIO_CURRENT:
a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
  "curr%d_input",
- curr_i++);
+ curr_i);
+
+   if (b)
+   b->dev_attr.attr.name = kasprintf(GFP_KERNEL,
+ 
"curr%d_label",
+

[PATCH v2 3/4] mfd: add support for Allwinner SoCs ADC

2016-07-15 Thread Quentin Schulz
The Allwinner SoCs all have an ADC that can also act as a touchscreen
controller and a thermal sensor. For now, only the ADC and the thermal
sensor drivers are probed by the MFD, the touchscreen controller support
will be added later.

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

v2:
 - add license headers,
 - reorder alphabetically includes,
 - add SUNXI_GPADC_ prefixes for defines,

 drivers/mfd/Kconfig |  14 +++
 drivers/mfd/Makefile|   2 +
 drivers/mfd/sunxi-gpadc-mfd.c   | 197 
 include/linux/mfd/sunxi-gpadc-mfd.h |  23 +
 4 files changed, 236 insertions(+)
 create mode 100644 drivers/mfd/sunxi-gpadc-mfd.c
 create mode 100644 include/linux/mfd/sunxi-gpadc-mfd.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 1bcf601..67b55d0 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -82,6 +82,20 @@ config MFD_ATMEL_FLEXCOM
  by the probe function of this MFD driver according to a device tree
  property.
 
+config MFD_SUNXI_ADC
+   tristate "ADC MFD core driver for sunxi platforms"
+   select MFD_CORE
+   select REGMAP_MMIO
+   help
+ Select this to get support for Allwinner SoCs (A10, A13 and A31) ADC.
+ This driver will only map the hardware interrupt and registers, you
+ have to select individual drivers based on this MFD to be able to use
+ the ADC or the thermal sensor. This will try to probe the ADC driver
+ sunxi-gpadc-iio and the hwmon driver iio_hwmon.
+
+ To compile this driver as a module, choose M here: the
+ module will be called sunxi-gpadc-mfd.
+
 config MFD_ATMEL_HLCDC
tristate "Atmel HLCDC (High-end LCD Controller)"
select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 42a66e1..dcf43cd 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -201,6 +201,8 @@ obj-$(CONFIG_MFD_DLN2)  += dln2.o
 obj-$(CONFIG_MFD_RT5033)   += rt5033.o
 obj-$(CONFIG_MFD_SKY81452) += sky81452.o
 
+obj-$(CONFIG_MFD_SUNXI_ADC)+= sunxi-gpadc-mfd.o
+
 intel-soc-pmic-objs:= intel_soc_pmic_core.o intel_soc_pmic_crc.o
 intel-soc-pmic-$(CONFIG_INTEL_PMC_IPC) += intel_soc_pmic_bxtwc.o
 obj-$(CONFIG_INTEL_SOC_PMIC)   += intel-soc-pmic.o
diff --git a/drivers/mfd/sunxi-gpadc-mfd.c b/drivers/mfd/sunxi-gpadc-mfd.c
new file mode 100644
index 000..f0005a6
--- /dev/null
+++ b/drivers/mfd/sunxi-gpadc-mfd.c
@@ -0,0 +1,197 @@
+/* ADC MFD core driver for sunxi platforms
+ *
+ * Copyright (c) 2016 Quentin Schulz <quentin.schulz@free-electrons>
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#define SUNXI_IRQ_FIFO_DATA0
+#define SUNXI_IRQ_TEMP_DATA1
+
+static struct resource adc_resources[] = {
+   {
+   .name   = "FIFO_DATA_PENDING",
+   .start  = SUNXI_IRQ_FIFO_DATA,
+   .end= SUNXI_IRQ_FIFO_DATA,
+   .flags  = IORESOURCE_IRQ,
+   }, {
+   .name   = "TEMP_DATA_PENDING",
+   .start  = SUNXI_IRQ_TEMP_DATA,
+   .end= SUNXI_IRQ_TEMP_DATA,
+   .flags  = IORESOURCE_IRQ,
+   },
+};
+
+static const struct regmap_irq sunxi_gpadc_mfd_regmap_irq[] = {
+   REGMAP_IRQ_REG(SUNXI_IRQ_FIFO_DATA, 0, BIT(16)),
+   REGMAP_IRQ_REG(SUNXI_IRQ_TEMP_DATA, 0, BIT(18)),
+};
+
+static const struct regmap_irq_chip sunxi_gpadc_mfd_regmap_irq_chip = {
+   .name = "sunxi_gpadc_mfd_irq_chip",
+   .status_base = SUNXI_GPADC_TP_INT_FIFOS,
+   .ack_base = SUNXI_GPADC_TP_INT_FIFOS,
+   .mask_base = SUNXI_GPADC_TP_INT_FIFOC,
+   .init_ack_masked = true,
+   .mask_invert = true,
+   .irqs = sunxi_gpadc_mfd_regmap_irq,
+   .num_irqs = ARRAY_SIZE(sunxi_gpadc_mfd_regmap_irq),
+   .num_regs = 1,
+};
+
+static struct mfd_cell sun4i_gpadc_mfd_cells[] = {
+   {
+   .name   = "sun4i-a10-gpadc-iio",
+   .resources = adc_resources,
+   .num_resources = ARRAY_SIZE(adc_resources),
+   }, {
+   .name = "iio_hwmon",
+   }
+};
+
+static struct mfd_cell sun5i_gpadc_mfd_cells[] = {
+   {
+   .name   = "sun5i-a13-gpadc-iio",
+   .resources = adc_resources,
+   .num_resources = ARRAY_SIZE(adc_resources),
+   }, {
+   .name = "iio_hwmon",
+   },
+};
+
+static struct mfd_cell sun6i_gpadc_mfd_cells[] = {
+   {
+   .name   = "sun6i-a31-gpadc-iio",
+   .resources = adc_resources,
+   .num_resources = ARRAY_SIZE(adc_r

[PATCH v2 1/4] hwmon: iio_hwmon: defer probe when no channel is found

2016-07-15 Thread Quentin Schulz
iio_channel_get_all returns -ENODEV when it cannot find either phandles and
properties in the Device Tree or channels whose consumer_dev_name matches
iio_hwmon in iio_map_list. The iio_map_list is filled in by iio drivers
which might be probed after iio_hwmon.

It is better to defer the probe of iio_hwmon if such error is returned by
iio_channel_get_all in order to let a chance to iio drivers to expose
channels in iio_map_list.

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

No modifications for this patch since we did not settled for a solution.
What should we do?

 drivers/hwmon/iio_hwmon.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/iio_hwmon.c b/drivers/hwmon/iio_hwmon.c
index b550ba5..c0da4d9 100644
--- a/drivers/hwmon/iio_hwmon.c
+++ b/drivers/hwmon/iio_hwmon.c
@@ -73,8 +73,11 @@ static int iio_hwmon_probe(struct platform_device *pdev)
name = dev->of_node->name;
 
channels = iio_channel_get_all(dev);
-   if (IS_ERR(channels))
+   if (IS_ERR(channels)) {
+   if (PTR_ERR(channels) == -ENODEV)
+   return -EPROBE_DEFER;
return PTR_ERR(channels);
+   }
 
st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
if (st == NULL) {
-- 
2.5.0



[PATCH v2 0/4] add support for Allwinner SoCs ADC

2016-07-15 Thread Quentin Schulz
The Allwinner SoCs all have an ADC that can also act as a touchscreen
controller and a thermal sensor. The first four channels can be used either
for the ADC or the touchscreen and the fifth channel is used for the
thermal sensor. We currently have a driver for the two latter functions in
drivers/input/touchscreen/sun4i-ts.c but we don't have access to the ADC
feature at all. It is meant to replace the current driver by using MFD and
subdrivers.

This adds initial support for Allwinner SoCs ADC with all features. Yet,
the touchscreen is not implemented but will be added later. To switch
between touchscreen and ADC modes, you need to poke a few bits in registers
and (de)activate an interrupt (pen-up).
An MFD is provided to let the input driver activate the pen-up interrupt
through a virtual interrupt, poke a few bits via regmap and read data from
the ADC driver while both (and iio_hwmon) are probed by the MFD.

There are slight variations between the different SoCs ADC like the address
of some registers and the scale and offset to apply to raw thermal sensor
values. These variations are handled by using different platform_device_id,
passed to the sub-drivers when they are probed by the MFD.

This also modifies iio-hwmon to allow probe deferring when no iio channel
is found. Currently when no iio channel is found, the probing of iio-hwmon
fails. This is problematic when iio-hwmon probes before the iio driver
could register iio channels to share.
It exposes labels for channels read by iio_hwmon. Until now, there were no
way to identify what the exposed channels are representing. Now, if a
channel has its extend_name field set, this value will be exposed in a
sysfs file suffixed by _label. If the field is empty, no file will be
created.

Quentin Schulz (4):
  hwmon: iio_hwmon: defer probe when no channel is found
  iio: adc: add support for Allwinner SoCs ADC
  mfd: add support for Allwinner SoCs ADC
  hwmon: iio: add label for channels read by iio_hwmon

 drivers/hwmon/iio_hwmon.c   |  82 ++-
 drivers/iio/adc/Kconfig |  12 ++
 drivers/iio/adc/Makefile|   1 +
 drivers/iio/adc/sunxi-gpadc-iio.c   | 417 
 drivers/mfd/Kconfig |  14 ++
 drivers/mfd/Makefile|   2 +
 drivers/mfd/sunxi-gpadc-mfd.c   | 197 +
 include/linux/mfd/sunxi-gpadc-mfd.h |  23 ++
 8 files changed, 738 insertions(+), 10 deletions(-)
 create mode 100644 drivers/iio/adc/sunxi-gpadc-iio.c
 create mode 100644 drivers/mfd/sunxi-gpadc-mfd.c
 create mode 100644 include/linux/mfd/sunxi-gpadc-mfd.h

-- 
2.5.0



Re: [PATCH v2 4/4] hwmon: iio: add label for channels read by iio_hwmon

2016-07-19 Thread Quentin Schulz
On 18/07/2016 14:24, Jonathan Cameron wrote:
> On 15/07/16 10:59, Quentin Schulz wrote:
>> Currently, iio_hwmon only exposes values of the IIO channels it can read
>> but no label by channel is exposed.
>>
>> This adds exposition of sysfs files containing label for IIO channels it
>> can read based on extended_name field of the iio_chan_spec of the channel.
>> If the extended_name field is empty, the sysfs file is not created by
>> iio_hwmon.
> Hmm. This is not the intent of extended name at all.  That exists to add
> a small amount of information to an constructed IIO channel name.
> Typically it's used to indicate physically wired stuff like:
> 
> in_voltage0_vdd_raw for cases where that channel of the ADC is hard wired
> to the vdd.  In this particular case the use might actually work as the
> vdd makes it clear it's a voltage - in general that's not the case.
> 
> Use of extended_name at all in IIO is only done after extensive review.
> It adds nasty custom ABI to a device, so the gain has to be considerable
> to use it.
> 
> When I read your original suggestion of adding labels, I was expecting the
> use of datasheet_name.  That has the advantage of being well defined by
> the datasheet (if not it should not be provided) + not being used in
> the construction of the IIO channel related attributes.  However, that
> may still not correspond well to the expected labelling in hwmon.

Good to know for extend_name use cases. While doing further testing, I
noticed the extend_name is also appended to the sysfs filename.. which
is definitely not what we want.

I've checked for datasheet_name and it is only used to be compared to
adc_channel_label from iio_map structure. Same for adc_channel_label
(which has to be the same as datasheet_name of the iio_chan_spec it is
linked to). So I could use this instead of extend_name to put a label on
a channel.
However, I thought of it to be more a way to identify the hardware in
the datasheet more than giving users a hint on what it is. That's what
"git grep adc_channel_label" told me. It's definitely better to use
datasheet_name over extend_name for channel labeling but I don't know if
it's really the good variable to use for labeling? In my understanding
of datasheet_name, in my case it would be more "temp_gpadc" than "SoC
temperature", that's what I mean.

> Thinking more on this, the label is going to often be a function of how
> the board is wired up...  Perhaps it should be a characteristic of the
> channel_map (hence from DT or similar) rather than part of the IIO driver
> itself?

Hmm.. I would not put a property in the DT only for labeling.

[...]


Re: [PATCH v2 2/4] iio: adc: add support for Allwinner SoCs ADC

2016-07-19 Thread Quentin Schulz
On 18/07/2016 14:57, Maxime Ripard wrote:
> On Fri, Jul 15, 2016 at 11:59:12AM +0200, Quentin Schulz wrote:
>> The Allwinner SoCs all have an ADC that can also act as a touchscreen
>> controller and a thermal sensor. This patch adds the ADC driver which is
>> based on the MFD for the same SoCs ADC.
>>
>> This also registers the thermal adc channel in the iio map array so
>> iio_hwmon could use it without modifying the Device Tree.
>>
>> This driver probes on three different platform_device_id to take into
>> account slight differences between Allwinner SoCs ADCs.
>>
>> Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
>> ---
>>
>> v2:
>>  - add SUNXI_GPADC_ prefixes for defines,
>>  - correct typo in Kconfig,
>>  - reorder alphabetically includes, makefile,
>>  - add license header,
>>  - fix architecture variations not being handled in interrupt handlers or
>>read raw functions,
>>  - fix unability to return negative values from thermal sensor,
>>  - add gotos to reduce code repetition,
>>  - fix irq variable being unsigned int instead of int,
>>  - remove useless dev_err and dev_info,
>>  - deactivate all interrupts if probe fails,
>>  - fix iio_device_register on NULL variable,
>>  - deactivate ADC in the IP when probe fails or when removing driver,
>>
>>  drivers/iio/adc/Kconfig   |  12 ++
>>  drivers/iio/adc/Makefile  |   1 +
>>  drivers/iio/adc/sunxi-gpadc-iio.c | 417 
>> ++
>>  3 files changed, 430 insertions(+)
>>  create mode 100644 drivers/iio/adc/sunxi-gpadc-iio.c
>>
>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> index 25378c5..184856f 100644
>> --- a/drivers/iio/adc/Kconfig
>> +++ b/drivers/iio/adc/Kconfig
>> @@ -338,6 +338,18 @@ config NAU7802
>>To compile this driver as a module, choose M here: the
>>module will be called nau7802.
>>  
>> +config SUNXI_ADC
> 
> We try to avoid the SUNXI prefix usually, otherwise this driver will
> have a generic name (or at least is implicitly saying that it supports
> all the sunxi SoCs), while it supports only a subset of those SoCs.
> 

ACK. Will be replaced by SUN4I_GPADC.

>> +tristate "ADC driver for sunxi platforms"
> 
> And you should also mention which ADC is supported, since we usually
> have several of them.
> 
> Something like "Support for the Allwinner SoCs GPADC"
> 

ACK.

>> +depends on IIO
>> +depends on MFD_SUNXI_ADC
> 
> The order of your patches is quite weird. You depend on an option that
> is not present yet?
> 

ACK. Will modify the order of patches to reflect the real order.

>> +help
>> +  Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
>> +  ADC. This ADC provides 4 channels which can be used as an ADC or as a
>> +  touchscreen input and one channel for thermal sensor.
>> +
>> +  To compile this driver as a module, choose M here: the module 
>> will be
> 
> Your indentation is weird here, and the wrapping is likely to be wrong
> too.
> 

ACK.

[...]
>> @@ -0,0 +1,417 @@
>> +/* ADC driver for sunxi platforms
>> + *
>> + * Copyright (c) 2016 Quentin Schulz <quentin.schulz@free-electrons>
>> + *
>> + * 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.
> 
> Your wrapping is wrong.
> 

ACK.

>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define SUNXI_GPADC_TP_CTRL00x00
>> +#define SUNXI_GPADC_TP_CTRL10x04
>> +#define SUNXI_GPADC_TP_CTRL20x08
>> +#define SUNXI_GPADC_TP_CTRL30x0c
>> +#define SUNXI_GPADC_TP_TPR  0x18
>> +#define SUNXI_GPADC_TP_CDAT 0x1c
>> +#define SUNXI_GPADC_TEMP_DATA   0x20
>> +#define SUNXI_GPADC_TP_DATA 0x24
>> +
>> +/* TP_CTRL0 bits */
>> +#define SUNXI_GPADC_ADC_FIRST_DLY(x)((x) << 24) /* 8 bits */
>> +#define SUNXI_GPADC_ADC_FIRST_DLY_MODE  BIT(23)
>> +#define SUNXI_GPADC_ADC_CLK_SELECT  BIT(22)
>> +#define SUNXI_GP

Re: [PATCH v2 3/4] mfd: add support for Allwinner SoCs ADC

2016-07-19 Thread Quentin Schulz
On 18/07/2016 15:25, Jonathan Cameron wrote:
> On 15/07/16 10:59, Quentin Schulz wrote:
>> The Allwinner SoCs all have an ADC that can also act as a touchscreen
>> controller and a thermal sensor. For now, only the ADC and the thermal
>> sensor drivers are probed by the MFD, the touchscreen controller support
>> will be added later.
>>
>> Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
> Hmm. Previous patch includes the header this one creates.  Ordering issue?
> The depends kind of prevents build failures by ensuring that can't be built
> until this one is in place, but it is certainly an ugly way to do it.
> 
> Few little bits innline.
[...]
>> +static int sunxi_gpadc_mfd_probe(struct platform_device *pdev)
>> +{
>> +struct sunxi_gpadc_mfd_dev *sunxi_gpadc_mfd_dev = NULL;
>> +struct resource *mem = NULL;
> Neither of the above assignments is necessary as both will be explicitly
> assigned before they are otherwise used.

ACK.

[...]
>> +dev_info(>dev, "successfully loaded\n");
> Seems like noise to me, but not my subsystem :)

ACK.
[...]


Re: [PATCH v2 2/4] iio: adc: add support for Allwinner SoCs ADC

2016-07-19 Thread Quentin Schulz
On 18/07/2016 15:18, Jonathan Cameron wrote:
> On 15/07/16 10:59, Quentin Schulz wrote:
>> The Allwinner SoCs all have an ADC that can also act as a touchscreen
>> controller and a thermal sensor. This patch adds the ADC driver which is
>> based on the MFD for the same SoCs ADC.
>>
>> This also registers the thermal adc channel in the iio map array so
>> iio_hwmon could use it without modifying the Device Tree.
>>
>> This driver probes on three different platform_device_id to take into
>> account slight differences between Allwinner SoCs ADCs.
>>
>> Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
> 
> Hi Quentin,
> 
> Various bits inline.  In particular the irq handling looks flakey / racey
> to me.  Definitely need some explanatory comments.
> 
> Also another note on the craziness that using extended_name to provide
> the hwmon labels will cause :)
> 
> Jonathan
[...]
>> +struct sunxi_gpadc_dev {
>> +void __iomem*regs;
>> +struct completion   completion;
>> +int temp_data;
>> +u32 adc_data;
>> +struct regmap   *regmap;
>> +unsigned intfifo_data_irq;
>> +unsigned inttemp_data_irq;
>> +unsigned intflags;
> I'd prefer something more explicit than this.  Right now only one
> bit can ever be set - indicating a particular chip family.  Why not
> just have an enum instead?

ACK.

[...]
>> +static const struct iio_chan_spec sunxi_gpadc_channels[] = {
>> +SUNXI_GPADC_ADC_CHANNEL(0, "adc_chan0"),
>> +SUNXI_GPADC_ADC_CHANNEL(1, "adc_chan1"),
>> +SUNXI_GPADC_ADC_CHANNEL(2, "adc_chan2"),
>> +SUNXI_GPADC_ADC_CHANNEL(3, "adc_chan3"),
>> +{
>> +.type = IIO_TEMP,
>> +.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
>> +.datasheet_name = "temp_adc",
>> +.extend_name = "SoC temperature",
> Just curious, did you look at the resultling sysfs entries?
> Going to be something like
> in_temp_SoC\ Temperature_input... Not good.

Just encountered this after further testing, not good as you said.

> If there is a strong enough reason (and there may be) to add a 'help string'
> type label to struct iio_chan_spec then my only real worries would be that
> we would be adding a whole pile of ABI that would be in the, near impossible
> to change in future, category...

I don't understand your "adding a whole pile of ABI" thing. Same as
datasheet_name variable: const char* in struct iio_chan_spec and used
whenever needed with some NULL checks. This is the easiest way to do it.
Or maybe you're thinking of adding an item to iio_chan_info_enum and all
its underlying modifications? This could allow us to expose a _label
sysfs file in the iio_device per type/channel.

I understand the "near impossible to change in future" concern though.

[...]
>> +enable_irq(info->temp_data_irq);
> Is this hardware spitting out extra irqs?  If not, much better to just
> leave it enabled all the time and control whether it can occur or not
> by controlling the device state..

The temp_data_irq occurs every SUNXI_GPADC_TEMP_PERIOD(x) periods (in
the current state of the driver: 2s). What do you mean by controlling
the device state? Enabling or disabling the hardware part of the IP
responsible of getting the temperature
(SUNXI_GPADC_TP_TPR_TEMP_ENABLE(x) here)?
Once the interrupt is activated, the IP periodically performs
conversions. We don't really want interrupts to be activated when not
needed.

[...]
>> +
>> +if (info->flags & SUNXI_GPADC_ARCH_SUN4I)
>> +*val = info->temp_data * 133 - 257000;
> Why report as processed?  I'd just report them as raw with the scale
> and offset provided.  It's not a big thing, but if we can leave it so
> that the conversion only occurs when desired, why not?
> 
> For in kernel users, this all happen 'automagically' anyway ;)
> 

ACK.

[...]
>> +mutex_unlock(_dev->mlock);
> mlock has a very specific purpose - locking the state changes of
> between 'buffered' (push) and poll modes. Don't use it for anything else.
> In fact it's almost always a bug to access it directly at all.  We have
> nice wrappers now for checking and locking the access mode.
> Just have a local lock in your iio_priv structure.

ACK. There is still a lot of drivers using iio_dev mlock though.

[...]
>> +static irqreturn_t sunxi_gpadc_fifo_data_irq_handler(int irq, void *dev_id)
>> +{
>> +struct sunxi_gpadc_dev *info =

Re: [PATCH v2 3/4] mfd: add support for Allwinner SoCs ADC

2016-07-19 Thread Quentin Schulz
On 18/07/2016 15:02, Maxime Ripard wrote:
> On Fri, Jul 15, 2016 at 11:59:13AM +0200, Quentin Schulz wrote:
>> The Allwinner SoCs all have an ADC that can also act as a touchscreen
>> controller and a thermal sensor. For now, only the ADC and the thermal
>> sensor drivers are probed by the MFD, the touchscreen controller support
>> will be added later.
>>
>> Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
>> ---
[...]
>> +config MFD_SUNXI_ADC
>> +tristate "ADC MFD core driver for sunxi platforms"
>> +select MFD_CORE
>> +select REGMAP_MMIO
> 
> It should also depends on the architectures supported (and probably 
> COMPILE_TEST)
> 

ACK.

[...]
>> +
>> +sunxi_gpadc_mfd_dev->regmap =
>> +devm_regmap_init_mmio(sunxi_gpadc_mfd_dev->dev,
>> +  sunxi_gpadc_mfd_dev->regs,
>> +  _gpadc_mfd_regmap_config);
> 
> This is usually on a single line (even if it exceeds 80 chars). Or
> maybe you can use a shorter variable name (like dev, or mfd).
> 

I'll go with a shorter name.

>> +if (IS_ERR(sunxi_gpadc_mfd_dev->regmap)) {
>> +ret = PTR_ERR(sunxi_gpadc_mfd_dev->regmap);
>> +dev_err(>dev, "failed to init regmap: %d\n", ret);
>> +return ret;
>> +}
>> +
>> +irq = platform_get_irq(pdev, 0);
>> +ret = regmap_add_irq_chip(sunxi_gpadc_mfd_dev->regmap, irq,
>> +  IRQF_ONESHOT, 0,
>> +  _gpadc_mfd_regmap_irq_chip,
>> +  _gpadc_mfd_dev->regmap_irqc);
>> +if (ret) {
>> +dev_err(>dev, "failed to add irq chip: %d\n", ret);
>> +return ret;
>> +}
> 
> You should probably make sure that you clear all the interrupts before
> enabling them.
> 

ACK. Thanks, didn't think of that.

>> +if (of_device_is_compatible(pdev->dev.of_node,
>> +"allwinner,sun4i-a10-ts"))
>> +ret = mfd_add_devices(sunxi_gpadc_mfd_dev->dev, 0,
>> +  sun4i_gpadc_mfd_cells,
>> +  ARRAY_SIZE(sun4i_gpadc_mfd_cells), NULL,
>> +  0, NULL);
>> +else if (of_device_is_compatible(pdev->dev.of_node,
>> + "allwinner,sun5i-a13-ts"))
>> +ret = mfd_add_devices(sunxi_gpadc_mfd_dev->dev, 0,
>> +  sun5i_gpadc_mfd_cells,
>> +  ARRAY_SIZE(sun5i_gpadc_mfd_cells), NULL,
>> +  0, NULL);
>> +else if (of_device_is_compatible(pdev->dev.of_node,
>> + "allwinner,sun6i-a31-ts"))
>> +ret = mfd_add_devices(sunxi_gpadc_mfd_dev->dev, 0,
>> +  sun6i_gpadc_mfd_cells,
>> +  ARRAY_SIZE(sun6i_gpadc_mfd_cells), NULL,
>> +  0, NULL);
> 
> This huge if / else can be removed by putting those structures in the
> data pointer of of_device_id.
> 

Indeed. It is what I am using for the ADC driver, don't know why I
didn't think of this for the MFD as well.

[...]
>> diff --git a/include/linux/mfd/sunxi-gpadc-mfd.h 
>> b/include/linux/mfd/sunxi-gpadc-mfd.h
>> new file mode 100644
>> index 000..7155845
>> --- /dev/null
>> +++ b/include/linux/mfd/sunxi-gpadc-mfd.h
>> @@ -0,0 +1,23 @@
>> +/* Header of ADC MFD core driver for sunxi platforms
>> + *
>> + * Copyright (c) 2016 Quentin Schulz <quentin.schulz@free-electrons>
>> + *
>> + * 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.
>> + */
>> +
>> +#ifndef __SUNXI_GPADC_MFD__H__
>> +#define __SUNXI_GPADC_MFD__H__
>> +
>> +#define SUNXI_GPADC_TP_INT_FIFOC0x10
>> +#define SUNXI_GPADC_TP_INT_FIFOS0x14
> 
> Why do you declare only these two registers there?
> 

Because these are used by the MFD while the others not. Maybe it's
better to put all register and bit defines in sunxi-gpadc-mfd.h? Anyway,
just found out it would be clearer to use the defines for the interrupts
rather than directly "BIT(x)". So that makes two more defines here.
Should we put everything in sunxi-gpadc-mfd.h since the MFD is needed
for the ADC, (future) touchscreen and iio_hwmon drivers?




signature.asc
Description: OpenPGP digital signature


[PATCH 2/5] mfd: sunxi-gpadc-mfd: add buffer structure

2016-07-20 Thread Quentin Schulz
This adds a buffer structure for files including the sunxi-gpadc-mfd
header. This structure has a buffer of 32 u32 values to store data from the
FIFO of the GPADC of Allwinner SoCs. A buff_size is provided in case the
buffer is not full.

Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
---
 include/linux/mfd/sunxi-gpadc-mfd.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/linux/mfd/sunxi-gpadc-mfd.h 
b/include/linux/mfd/sunxi-gpadc-mfd.h
index 7155845..f658299 100644
--- a/include/linux/mfd/sunxi-gpadc-mfd.h
+++ b/include/linux/mfd/sunxi-gpadc-mfd.h
@@ -20,4 +20,9 @@ struct sunxi_gpadc_mfd_dev {
void __iomem*regs;
 };
 
+struct sunxi_gpadc_buffer {
+   u32 buffer[32];
+   unsigned intbuff_size;
+};
+
 #endif
-- 
2.5.0



[PATCH 0/5] add resistive touchscreen support for new Allwinner SoCs' GPADC's driver

2016-07-20 Thread Quentin Schulz
The Allwinner SoCs all have an ADC that can also act as a touchscreen
controller and a thermal sensor. The first four channels can be used either
for the ADC or the touchscreen and the fifth channel is used for the
thermal sensor. We currently have a driver for the two latter functions in
drivers/input/touchscreen/sun4i-ts.c but we don't have access to the ADC
feature at all.

The temperature sensor returns valid values only when the GPADC is in
touchscreen mode.

This patch series is based on the patch series named "add support for
Allwinner SoCs ADC": https://lkml.org/lkml/2016/6/28/226

This adds the TP_UP_PENDING VIRQ in the MFD which occurs when the thing
(stylus or finger) touching the screen releases the touch. This VIRQ is
then handled in the touchscreen driver to notify the input framework of an
up event.
The MFD probes the touchscreen driver if the property
"allwinner,ts-attached" is set in rtp node of the DT.

The touchscreen driver needs data from the IIO ADC driver to retrieve touch
coordinates. The ADC driver exposes its four ADC channels to the
touchscreen driver. The touchscreen driver retrieves the four ADC channels,
associates a buffer with these channels and registers a callback to handle
buffer data. When the touchscreen driver is open, it starts the buffering
in the ADC driver and activates the callback. The ADC driver will be
notified a buffer is started and will then activate the FIFO_DATA_PENDING
interrupt which occurs when there is data to read in the FIFO. It also
selects the right mode (touchscreen or ADC). Then the FIFO_DATA_PENDING
VIRQ handler will continuously fill in a buffer with all data from the
hardware FIFO and send it to the touchscreen driver via the callback the
touchscreen driver registered previously when the VIRQ has been handled.
When in touchscreen mode, the hardware FIFO is filled in following this
pattern: X then Y coordinates of the first event, X and Y coordinates of
the second event, etc. This VIRQ is disabled only when the ADC driver is
notified a buffer is stopped. The touchscreen driver gets this buffer in
its defined callback and notifies the input framework of all events'
coordinates. When the touchscreen driver is closed, it stops the buffering
which deactivates the callback.

The hardware FIFO contains a maximum of 32 u32 values. Locations of the
first buffer retrieved after an up event are unreliable and are thus
dropped.

This patch replaces drivers/input/touchscreen/sun4i-ts.c by
drivers/input/touchscreen/sunxi-gpadc-ts.c, adding the ADC feature to
Allwinner SoCs' GPADC.

Quentin Schulz (5):
  mfd: sunxi-gpadc-mfd: add TP_UP_PENDING irq
  mfd: sunxi-gpadc-mfd: add buffer structure
  iio: adc: sunxi-gpadc-iio: enable iio_buffers
  input: touchscreen: support Allwinner SoCs' touchscreen
  mfd: sunxi-gpadc-mfd: probe sunxi-gpadc-ts driver

 drivers/iio/adc/Kconfig|   1 +
 drivers/iio/adc/sunxi-gpadc-iio.c  | 153 +--
 drivers/input/touchscreen/Kconfig  |  11 +-
 drivers/input/touchscreen/Makefile |   2 +-
 drivers/input/touchscreen/sun4i-ts.c   | 419 -
 drivers/input/touchscreen/sunxi-gpadc-ts.c | 195 ++
 drivers/mfd/sunxi-gpadc-mfd.c  | 127 ++---
 include/linux/mfd/sunxi-gpadc-mfd.h|   5 +
 8 files changed, 433 insertions(+), 480 deletions(-)
 delete mode 100644 drivers/input/touchscreen/sun4i-ts.c
 create mode 100644 drivers/input/touchscreen/sunxi-gpadc-ts.c

-- 
2.5.0



[PATCH 4/5] input: touchscreen: support Allwinner SoCs' touchscreen

2016-07-20 Thread Quentin Schulz
This adds support for Allwinner SoCs' (A10, A13 and A31) resistive
touchscreen. This driver is probed by the MFD sunxi-gpadc-mfd.

This driver uses ADC channels exposed through the IIO framework by
sunxi-gpadc-iio to get its data. When opening this input device, it will
start buffering in the ADC driver and enable a TP_UP_PENDING irq. The ADC
driver will fill in a buffer with all data and call the callback the input
device associated with this buffer. The input device will then read the
buffer two by two and send X and Y coordinates to the input framework based
on what it received from the ADC's buffer. When closing this input device,
the buffering is stopped.

Note that locations in the first received buffer after an TP_UP_PENDING irq
occurred are unreliable, thus dropped.

Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
---
 drivers/input/touchscreen/Kconfig  |  11 +-
 drivers/input/touchscreen/Makefile |   2 +-
 drivers/input/touchscreen/sun4i-ts.c   | 419 -
 drivers/input/touchscreen/sunxi-gpadc-ts.c | 195 ++
 4 files changed, 201 insertions(+), 426 deletions(-)
 delete mode 100644 drivers/input/touchscreen/sun4i-ts.c
 create mode 100644 drivers/input/touchscreen/sunxi-gpadc-ts.c

diff --git a/drivers/input/touchscreen/Kconfig 
b/drivers/input/touchscreen/Kconfig
index 8ecdc38..f16ce36 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -1072,14 +1072,13 @@ config TOUCHSCREEN_STMPE
 config TOUCHSCREEN_SUN4I
tristate "Allwinner sun4i resistive touchscreen controller support"
depends on ARCH_SUNXI || COMPILE_TEST
-   depends on HWMON
-   depends on THERMAL || !THERMAL_OF
+   depends on SUNXI_ADC
help
- This selects support for the resistive touchscreen controller
- found on Allwinner sunxi SoCs.
+ This selects support for the resistive touchscreen controller found
+ on Allwinner SoCs (A10, A13, A31).
 
- To compile this driver as a module, choose M here: the
- module will be called sun4i-ts.
+ To compile this driver as a module, choose M here: the module will be
+ called sunxi-gpadc-ts.
 
 config TOUCHSCREEN_SUR40
tristate "Samsung SUR40 (Surface 2.0/PixelSense) touchscreen"
diff --git a/drivers/input/touchscreen/Makefile 
b/drivers/input/touchscreen/Makefile
index f42975e..bdc1889 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -65,7 +65,7 @@ obj-$(CONFIG_TOUCHSCREEN_PIXCIR)  += pixcir_i2c_ts.o
 obj-$(CONFIG_TOUCHSCREEN_S3C2410)  += s3c2410_ts.o
 obj-$(CONFIG_TOUCHSCREEN_ST1232)   += st1232.o
 obj-$(CONFIG_TOUCHSCREEN_STMPE)+= stmpe-ts.o
-obj-$(CONFIG_TOUCHSCREEN_SUN4I)+= sun4i-ts.o
+obj-$(CONFIG_TOUCHSCREEN_SUN4I)+= sunxi-gpadc-ts.o
 obj-$(CONFIG_TOUCHSCREEN_SUR40)+= sur40.o
 obj-$(CONFIG_TOUCHSCREEN_TI_AM335X_TSC)+= ti_am335x_tsc.o
 obj-$(CONFIG_TOUCHSCREEN_TOUCHIT213)   += touchit213.o
diff --git a/drivers/input/touchscreen/sun4i-ts.c 
b/drivers/input/touchscreen/sun4i-ts.c
deleted file mode 100644
index d07dd29..000
--- a/drivers/input/touchscreen/sun4i-ts.c
+++ /dev/null
@@ -1,419 +0,0 @@
-/*
- * Allwinner sunxi resistive touchscreen controller driver
- *
- * Copyright (C) 2013 - 2014 Hans de Goede <hdego...@redhat.com>
- *
- * The hwmon parts are based on work by Corentin LABBE which is:
- * Copyright (C) 2013 Corentin LABBE <clabbe.montj...@gmail.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.
- *
- * 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.
- */
-
-/*
- * The sun4i-ts controller is capable of detecting a second touch, but when a
- * second touch is present then the accuracy becomes so bad the reported touch
- * location is not useable.
- *
- * The original android driver contains some complicated heuristics using the
- * aprox. distance between the 2 touches to see if the user is making a pinch
- * open / close movement, and then reports emulated multi-touch events around
- * the last touch coordinate (as the dual-touch coordinates are worthless).
- *
- * These kinds of heuristics are just asking for trouble (and don't belong
- * in the kernel). So this driver offers straight forward, reliable single
- * touch functionality only.
- *
- * s.a. A20 User Manual "1.15 TP" (Documentation/arm/sunxi/README)
- * (looks like the description in the A20 User Manual v1.3 is better
- * than the on

[PATCH 1/5] mfd: sunxi-gpadc-mfd: add TP_UP_PENDING irq

2016-07-20 Thread Quentin Schulz
This adds support for TP_UP_PENDING irq in Allwinner SoCs' GPADC's MFD.

This interrupt occurs when a touchscreen is attached and the thing (stylus,
finger) currently touching the touchscreen releases the touch.

Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
---
 drivers/mfd/sunxi-gpadc-mfd.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/mfd/sunxi-gpadc-mfd.c b/drivers/mfd/sunxi-gpadc-mfd.c
index f0005a6..05a000b 100644
--- a/drivers/mfd/sunxi-gpadc-mfd.c
+++ b/drivers/mfd/sunxi-gpadc-mfd.c
@@ -19,6 +19,8 @@
 
 #define SUNXI_IRQ_FIFO_DATA0
 #define SUNXI_IRQ_TEMP_DATA1
+#define SUNXI_IRQ_TP_UP2
+
 
 static struct resource adc_resources[] = {
{
@@ -34,9 +36,19 @@ static struct resource adc_resources[] = {
},
 };
 
+static struct resource ts_resources[] = {
+   {
+   .name   = "TP_UP_PENDING",
+   .start  = SUNXI_IRQ_TP_UP,
+   .end= SUNXI_IRQ_TP_UP,
+   .flags  = IORESOURCE_IRQ,
+   },
+};
+
 static const struct regmap_irq sunxi_gpadc_mfd_regmap_irq[] = {
REGMAP_IRQ_REG(SUNXI_IRQ_FIFO_DATA, 0, BIT(16)),
REGMAP_IRQ_REG(SUNXI_IRQ_TEMP_DATA, 0, BIT(18)),
+   REGMAP_IRQ_REG(SUNXI_IRQ_TP_UP, 0, BIT(1)),
 };
 
 static const struct regmap_irq_chip sunxi_gpadc_mfd_regmap_irq_chip = {
-- 
2.5.0



[PATCH 5/5] mfd: sunxi-gpadc-mfd: probe sunxi-gpadc-ts driver

2016-07-20 Thread Quentin Schulz
This probes the touchscreen driver for Allwinner SoCs (A10, A13 and A31)
when the property "allwinner,ts-attached" is set in the GPADC (rtp) node of
the DT.

Some comestic modifications done to shorten and increase readability of the
code.

Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
---
 drivers/mfd/sunxi-gpadc-mfd.c | 115 --
 1 file changed, 77 insertions(+), 38 deletions(-)

diff --git a/drivers/mfd/sunxi-gpadc-mfd.c b/drivers/mfd/sunxi-gpadc-mfd.c
index 05a000b..9b9ed0b 100644
--- a/drivers/mfd/sunxi-gpadc-mfd.c
+++ b/drivers/mfd/sunxi-gpadc-mfd.c
@@ -21,6 +21,11 @@
 #define SUNXI_IRQ_TEMP_DATA1
 #define SUNXI_IRQ_TP_UP2
 
+#define SUNXI_GPADC_MFD_CELL(_name, _resources, _num_resources) {  \
+   .name = _name,  \
+   .resources = _resources,\
+   .num_resources = _num_resources \
+}
 
 static struct resource adc_resources[] = {
{
@@ -64,33 +69,45 @@ static const struct regmap_irq_chip 
sunxi_gpadc_mfd_regmap_irq_chip = {
 };
 
 static struct mfd_cell sun4i_gpadc_mfd_cells[] = {
-   {
-   .name   = "sun4i-a10-gpadc-iio",
-   .resources = adc_resources,
-   .num_resources = ARRAY_SIZE(adc_resources),
-   }, {
-   .name = "iio_hwmon",
-   }
+   SUNXI_GPADC_MFD_CELL("sun4i-a10-gpadc-iio", adc_resources,
+ARRAY_SIZE(adc_resources)),
+   SUNXI_GPADC_MFD_CELL("iio_hwmon", NULL, 0),
 };
 
 static struct mfd_cell sun5i_gpadc_mfd_cells[] = {
-   {
-   .name   = "sun5i-a13-gpadc-iio",
-   .resources = adc_resources,
-   .num_resources = ARRAY_SIZE(adc_resources),
-   }, {
-   .name = "iio_hwmon",
-   },
+   SUNXI_GPADC_MFD_CELL("sun5i-a13-gpadc-iio", adc_resources,
+ARRAY_SIZE(adc_resources)),
+   SUNXI_GPADC_MFD_CELL("iio_hwmon", NULL, 0),
 };
 
 static struct mfd_cell sun6i_gpadc_mfd_cells[] = {
-   {
-   .name   = "sun6i-a31-gpadc-iio",
-   .resources = adc_resources,
-   .num_resources = ARRAY_SIZE(adc_resources),
-   }, {
-   .name = "iio_hwmon",
-   },
+   SUNXI_GPADC_MFD_CELL("sun6i-a31-gpadc-iio", adc_resources,
+ARRAY_SIZE(adc_resources)),
+   SUNXI_GPADC_MFD_CELL("iio_hwmon", NULL, 0),
+};
+
+static struct mfd_cell sun4i_gpadc_mfd_cells_ts[] = {
+   SUNXI_GPADC_MFD_CELL("sun6i-a31-gpadc-iio", adc_resources,
+ARRAY_SIZE(adc_resources)),
+   SUNXI_GPADC_MFD_CELL("iio_hwmon", NULL, 0),
+   SUNXI_GPADC_MFD_CELL("sunxi-gpadc-ts", ts_resources,
+ARRAY_SIZE(ts_resources)),
+};
+
+static struct mfd_cell sun5i_gpadc_mfd_cells_ts[] = {
+   SUNXI_GPADC_MFD_CELL("sun5i-a13-gpadc-iio", adc_resources,
+ARRAY_SIZE(adc_resources)),
+   SUNXI_GPADC_MFD_CELL("iio_hwmon", NULL, 0),
+   SUNXI_GPADC_MFD_CELL("sunxi-gpadc-ts", ts_resources,
+ARRAY_SIZE(ts_resources)),
+};
+
+static struct mfd_cell sun6i_gpadc_mfd_cells_ts[] = {
+   SUNXI_GPADC_MFD_CELL("sun6i-a31-gpadc-iio", adc_resources,
+ARRAY_SIZE(adc_resources)),
+   SUNXI_GPADC_MFD_CELL("iio_hwmon", NULL, 0),
+   SUNXI_GPADC_MFD_CELL("sunxi-gpadc-ts", ts_resources,
+ARRAY_SIZE(ts_resources)),
 };
 
 static const struct regmap_config sunxi_gpadc_mfd_regmap_config = {
@@ -142,23 +159,45 @@ static int sunxi_gpadc_mfd_probe(struct platform_device 
*pdev)
}
 
if (of_device_is_compatible(pdev->dev.of_node,
-   "allwinner,sun4i-a10-ts"))
-   ret = mfd_add_devices(sunxi_gpadc_mfd_dev->dev, 0,
- sun4i_gpadc_mfd_cells,
- ARRAY_SIZE(sun4i_gpadc_mfd_cells), NULL,
- 0, NULL);
-   else if (of_device_is_compatible(pdev->dev.of_node,
-"allwinner,sun5i-a13-ts"))
-   ret = mfd_add_devices(sunxi_gpadc_mfd_dev->dev, 0,
- sun5i_gpadc_mfd_cells,
- ARRAY_SIZE(sun5i_gpadc_mfd_cells), NULL,
- 0, NULL);
-   else if (of_device_is_compatible(pdev->dev.of_node,
-"allwinner,sun6i-a31-ts"))
-   ret = mfd_add_devices(sunxi

Re: [PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers

2016-07-20 Thread Quentin Schulz
On 20/07/2016 10:38, Peter Meerwald-Stadler wrote:
> 
>> This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
>> It also prepares the code which will be used by the touchscreen driver
>> named sunxi-gpadc-ts.
>>
>> The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
>> conversion's data. The GPADC uses the same ADC channels for the ADC and the
>> touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
>> consumer which will be in charge of reading data from these channels for
>> the input framework.
>>
>> The temperature can only be read when in touchscreen mode. This means if
>> the buffers are being used for the ADC, the temperature sensor cannot be
>> read.
>>
>> When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
>> and fill a buffer before sending it to the consumers which registered in
>> IIO for the ADC channels.
>>
>> When a consumer starts buffering ADC channels,
>> sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
>> irq and select the mode in which the GPADC should run (ADC or touchscreen)
>> depending on a property of the DT ("allwinner,ts-attached").
>> When the consumer stops buffering, it disables the same irq.
> 
> comments below
>  
[...]
>> @@ -101,19 +104,43 @@ struct sunxi_gpadc_dev {
>>  unsigned intfifo_data_irq;
>>  unsigned inttemp_data_irq;
>>  unsigned intflags;
>> +struct iio_dev  *indio_dev;
>> +struct sunxi_gpadc_buffer   buffer;
>> +boolts_attached;
>> +boolbuffered;
> 
> why add buffered, duplicate state and not query iio_buffer_enabled()?
> 
>>  };
>>  
>> -#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name) {  \
>> +#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name, _index) {  \
>>  .type = IIO_VOLTAGE,\
>>  .indexed = 1,   \
>>  .channel = _channel,\
>>  .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
>>  .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),   \
>>  .datasheet_name = _name,\
>> +.scan_index = _index,   \
>> +.scan_type = {  \
>> +.sign = 'u',\
>> +.realbits = 12, \
>> +.storagebits = 16,  \
>> +.shift = 0, \
> 
> shift not strictly needed
> 

ACK.

[...]
>>  static int sunxi_gpadc_read_raw(struct iio_dev *indio_dev,
>> @@ -219,15 +253,22 @@ static int sunxi_gpadc_read_raw(struct iio_dev 
>> *indio_dev,
>>  int *val, int *val2, long mask)
>>  {
>>  int ret;
>> +struct sunxi_gpadc_dev *info = iio_priv(indio_dev);
>>  
>>  switch (mask) {
>>  case IIO_CHAN_INFO_PROCESSED:
>> +if (info->buffered && !info->ts_attached)
>> +return -EBUSY;
> 
> there would be iio_device_claim_direct_mode()
> 

OK, iio_device_claim_direct_mode() and iio_device_release_direct_mode()
are new functions which are not yet in the Linux Cross Reference
(http://lxr.free-electrons.com/), I didn't know they existed. I'll use
that, iio_buffer_enabled() when needed and get rid of the buffered
boolean variable.

[...]
Thanks.

Quentin


[PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers

2016-07-20 Thread Quentin Schulz
This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
It also prepares the code which will be used by the touchscreen driver
named sunxi-gpadc-ts.

The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
conversion's data. The GPADC uses the same ADC channels for the ADC and the
touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
consumer which will be in charge of reading data from these channels for
the input framework.

The temperature can only be read when in touchscreen mode. This means if
the buffers are being used for the ADC, the temperature sensor cannot be
read.

When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
and fill a buffer before sending it to the consumers which registered in
IIO for the ADC channels.

When a consumer starts buffering ADC channels,
sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
irq and select the mode in which the GPADC should run (ADC or touchscreen)
depending on a property of the DT ("allwinner,ts-attached").
When the consumer stops buffering, it disables the same irq.

Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
---
 drivers/iio/adc/Kconfig   |   1 +
 drivers/iio/adc/sunxi-gpadc-iio.c | 153 ++
 2 files changed, 138 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 184856f..15e3b08 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -342,6 +342,7 @@ config SUNXI_ADC
tristate "ADC driver for sunxi platforms"
depends on IIO
depends on MFD_SUNXI_ADC
+   depends on IIO_BUFFER_CB
help
  Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
  ADC. This ADC provides 4 channels which can be used as an ADC or as a
diff --git a/drivers/iio/adc/sunxi-gpadc-iio.c 
b/drivers/iio/adc/sunxi-gpadc-iio.c
index 87cc913..2e44ca7 100644
--- a/drivers/iio/adc/sunxi-gpadc-iio.c
+++ b/drivers/iio/adc/sunxi-gpadc-iio.c
@@ -16,8 +16,9 @@
 #include 
 #include 
 
-#include 
+#include 
 #include 
+#include 
 #include 
 #include 
 
@@ -71,6 +72,7 @@
 #define SUNXI_GPADC_TP_DATA_XY_CHANGE  BIT(13)
 #define SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(x)  ((x) << 8)  /* 5 bits */
 #define SUNXI_GPADC_TP_DATA_DRQ_EN BIT(7)
+/* Be careful, flushing FIFO spawns SUNXI_GPADC_FIFO_DATA_PENDING interrupts */
 #define SUNXI_GPADC_TP_FIFO_FLUSH  BIT(4)
 #define SUNXI_GPADC_TP_UP_IRQ_EN   BIT(1)
 #define SUNXI_GPADC_TP_DOWN_IRQ_EN BIT(0)
@@ -79,6 +81,7 @@
 #define SUNXI_GPADC_TEMP_DATA_PENDING  BIT(18)
 #define SUNXI_GPADC_FIFO_OVERRUN_PENDING   BIT(17)
 #define SUNXI_GPADC_FIFO_DATA_PENDING  BIT(16)
+#define SUNXI_GPADC_RXA_CNTGENMASK(12, 8)
 #define SUNXI_GPADC_TP_IDLE_FLGBIT(2)
 #define SUNXI_GPADC_TP_UP_PENDING  BIT(1)
 #define SUNXI_GPADC_TP_DOWN_PENDINGBIT(0)
@@ -101,19 +104,43 @@ struct sunxi_gpadc_dev {
unsigned intfifo_data_irq;
unsigned inttemp_data_irq;
unsigned intflags;
+   struct iio_dev  *indio_dev;
+   struct sunxi_gpadc_buffer   buffer;
+   boolts_attached;
+   boolbuffered;
 };
 
-#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name) { \
+#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name, _index) { \
.type = IIO_VOLTAGE,\
.indexed = 1,   \
.channel = _channel,\
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),   \
.datasheet_name = _name,\
+   .scan_index = _index,   \
+   .scan_type = {  \
+   .sign = 'u',\
+   .realbits = 12, \
+   .storagebits = 16,  \
+   .shift = 0, \
+   .endianness = IIO_LE,   \
+   },  \
 }
 
 static struct iio_map sunxi_gpadc_hwmon_maps[] = {
{
+   .adc_channel_label = "adc_chan0",
+   .consumer_dev_name = "sunxi-gpadc-ts.0",
+   }, {
+   .adc_channel_label = "adc_chan1",
+   .consumer_dev_name = "sunxi-gpadc-ts.0",
+   }, {
+   .adc_channel_label = "adc_chan2",
+   .consumer_dev_name 

Re: [PATCH v2 2/4] iio: adc: add support for Allwinner SoCs ADC

2016-07-20 Thread Quentin Schulz
On 18/07/2016 15:18, Jonathan Cameron wrote:
[...]
>> +
>> +if (!wait_for_completion_timeout(>completion,
>> + msecs_to_jiffies(100))) {
>> +ret = -ETIMEDOUT;
>> +goto out;
>> +}
>> +
>> +if (info->flags & SUNXI_GPADC_ARCH_SUN4I)
>> +*val = info->temp_data * 133 - 257000;
> Why report as processed?  I'd just report them as raw with the scale
> and offset provided.  It's not a big thing, but if we can leave it so
> that the conversion only occurs when desired, why not?
> 
> For in kernel users, this all happen 'automagically' anyway ;)
> 

Mmmmh, in the code above we apply the scale on the raw value and then
the offset. While in iio_convert_raw_to_processed
(http://lxr.free-electrons.com/source/drivers/iio/inkern.c#L507), the
offset is applied before the scale.

The way would be to factorize the computation by scale:
Now: *val = raw * scale + offset
Then: *val = (raw + offset/scale) * scale

But the offset is an integer and offset/scale is therefore rounded.
Currently, we have the following values:
sun4i: -257000/133 = -1932.3308270676691
sun5i: -144700/100 = -1447
sun6i: -271000/167 = -1622.754491017964

Do we accept such rounding?

If not, we either stay with the processed value in read_raw or patch
inkern to add an offset to apply after having applied the scale to the
raw value (val2 from iio_channel_read is yet unused with
IIO_CHAN_INFO_OFFSET for example, we could use that to specify an
offset2 to apply after the switch(scale_type)-case).

[...]
Quentin



signature.asc
Description: OpenPGP digital signature


Re: [PATCH v2 4/4] hwmon: iio: add label for channels read by iio_hwmon

2016-07-15 Thread Quentin Schulz
On 15/07/2016 16:03, Guenter Roeck wrote:
> On 07/15/2016 02:59 AM, Quentin Schulz wrote:
[...]
>> +static ssize_t iio_hwmon_read_label(struct device *dev,
>> +struct device_attribute *attr,
>> +char *buf)
>> +{
>> +struct sensor_device_attribute *sattr = to_sensor_dev_attr(attr);
>> +struct iio_hwmon_state *state = dev_get_drvdata(dev);
>> +const char *label =
>> state->channels[sattr->index].channel->extend_name;
>> +
>> +if (label)
>> +return sprintf(buf, "%s\n", label);
>> +
> Can the name disappear on the fly, or be changed on the fly ?
> Then this is unusable. We should and can only provide labels
> if a name exists and is permanent. Otherwise all we do is
> to confuse user space.

It cannot, the extend_name field is const char* in the struct:
http://lxr.free-electrons.com/source/include/linux/iio/iio.h#L247

[...]
>> @@ -107,6 +123,18 @@ static int iio_hwmon_probe(struct platform_device
>> *pdev)
>>   }
>>
>>   sysfs_attr_init(>dev_attr.attr);
>> +
>> +b = NULL;
>> +if (st->channels[i].channel->extend_name) {
>> +b = devm_kzalloc(dev, sizeof(*b), GFP_KERNEL);
>> +if (b == NULL) {
>> +ret = -ENOMEM;
>> +goto error_release_channels;
>> +}
>> +
>> +sysfs_attr_init(>dev_attr.attr);
> 
> Why is this initialization here and not with the rest of the initialization
> of this attribute ?

I don't get your question. I've followed the exact same pattern as for
"a" variable's initialization.
The initialization is before the switch case because the name of the
exposed sysfs file depends on the type of the IIO channel. If I move the
initialization in the switch case, I'll have duplicated code.

>> +}
>> +
>>   ret = iio_get_channel_type(>channels[i], );
>>   if (ret < 0)
>>   goto error_release_channels;
>> @@ -115,35 +143,66 @@ static int iio_hwmon_probe(struct
>> platform_device *pdev)
>>   case IIO_VOLTAGE:
>>   a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
>> "in%d_input",
>> -  in_i++);
>> +  in_i);
>> +if (b)
>> +b->dev_attr.attr.name = kasprintf(GFP_KERNEL,
>> +  "in%d_label",
>> +  in_i);
>> +in_i++;
>>   break;
>>   case IIO_TEMP:
>>   a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
>> "temp%d_input",
>> -  temp_i++);
>> +  temp_i);
>> +
>> +if (b)
>> +b->dev_attr.attr.name = kasprintf(GFP_KERNEL,
>> +  "temp%d_label",
>> +  temp_i);
>> +temp_i++;
>>   break;
>>   case IIO_CURRENT:
>>   a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
>> "curr%d_input",
>> -  curr_i++);
>> +  curr_i);
>> +
>> +if (b)
>> +b->dev_attr.attr.name = kasprintf(GFP_KERNEL,
>> +  "curr%d_label",
>> +  curr_i);
>> +curr_i++;
>>   break;
>>   case IIO_HUMIDITYRELATIVE:
>>   a->dev_attr.attr.name = kasprintf(GFP_KERNEL,
>> "humidity%d_input",
>> -  humidity_i++);
>> +  humidity_i);
>> +
>> +if (b)
>> +b->dev_attr.attr.name = kasprintf(GFP_KERNEL,
>> +  "humidity%d_label",
>> +  humidity_i);
>> +humidity_i++;
>>   break;
>>   default:
>>   ret = -EINVAL;
>>   goto error_release_channels;
>>   }
>> -if (a->dev_attr.attr.name == NULL) {
>> +if (a->dev_attr.attr.name == NULL ||
>> +(b && b->dev_attr.attr.name == NULL)) {
>>   ret = -ENOMEM;
>> 

Re: [PATCH 2/3] iio: adc: add support for Allwinner SoCs ADC

2016-07-05 Thread Quentin Schulz
On 04/07/2016 18:29, Guenter Roeck wrote:
> On 07/04/2016 12:26 AM, Quentin Schulz wrote:
>> On 03/07/2016 17:43, Guenter Roeck wrote:
>>> On 07/03/2016 04:54 AM, Jonathan Cameron wrote:
>>>> On 28/06/16 09:18, Quentin Schulz wrote:
>>>>> The Allwinner SoCs all have an ADC that can also act as a touchscreen
>>>>> controller and a thermal sensor. This patch adds the ADC driver
>>>>> which is
>>>>> based on the MFD for the same SoCs ADC.
>>>>>
>>>>> This also registers the thermal adc channel in the iio map array so
>>>>> iio_hwmon could use it without modifying the Device Tree.
>>>>>
>>>>> This driver probes on three different platform_device_id to take into
>>>>> account slight differences between Allwinner SoCs ADCs.
>>>>>
>>>>> Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
>>>> Hi Quentin.
>>>>
>>>> I'm a bit in two minds about some of this.  That temperature sensor is
>>>> so obviously meant for hwmon purposes, I'm tempted to suggest it might
>>>> actually make sense to put it directly in hwmon rather than using the
>>>> bridge.  That obviously makes it less flexible in some ways (i.e. for
>>>> use within the thermal subsystem at some point).
>>>>
>>>> Guenter, what do you think?
>>>>
>>>
>>> With the upcoming new hwmon API, thermal registration is handled in the
>>> hwmon core, so that should not be an issue. Besides, other hwmon sensors
>>> already register with the thermal subsystem as well.
>>>
>>> This is difficult to evaluate without datasheet; I am not sure if
>>> the chip supports limits or trip points. If it supports trip points,
>>> thermal may be a better target.
>>>
>>> Overall it does look like the temperature sensor would warrant
>>> a separate driver. Only question is thermal or hwmon.
>>>
>>> Guenter
>>
>> Actually, the publicly available documentation for the temperature
>> sensor is almost inexistent. We only have the registers for it. For most
>> of the work on temperature sensor, it has been taken from the current
>> driver (sun4i-ts in input/touschreen) from Hans de Goede.
>>
> Does this mean that the temperature data will be exported twice, once
> through
> iio and once through the touchscreen driver ?
> 
> Guenter

Currently, the temperature data is exported once in thermal framework
from the sun4i-ts input driver.

In the proposed patch, the temperature is exported twice: once in iio
framework from the IIO driver and once in hwmon framework from iio_hwmon
driver (which takes data from a channel of the IIO driver).

Quentin

>> You can find the documentation here:
>> http://dl.linux-sunxi.org/A10/A10%20User%20manual%20V1.50.pdf
>> http://dl.linux-sunxi.org/A13/A13%20User%20Manual%20V1.30.pdf
>> http://dl.linux-sunxi.org/A31/A31%20User%20Manual%20V1.20.pdf
>>
>> The ADC is either called TP controller, Touch Panel controller or GPADC.
>> The temperature sensor's registers are only defined in the User Manual
>> of the A10 (first link).
>>
>> Nothing to be found for limits or trip points I think.
>>
>> Quentin
>>
>>>> I'm guessing detailed docs for this part aren't avaiable publicly? :(
>>>>
>>>> So the rest of my comments are kind of predicated on me having roughtly
>>>> understood how this device works from the structure of the driver.
>>>>
>>>> The temperature sensor is really effectively as separate ADC?
>>>> The main interest in this is for key detection?
>>>>
>>>> Anyhow, if the data flow for the temperatures sensor is not synced with
>>>> the other ADC channels, adding buffered (pushed) output from the
>>>> driver in
>>>> future will be fiddly and with a 250Hz device you'll probably want it.
>>>> Basically IIO buffered supports assumes each iio device will sample
>>>> data
>>>> at a particular frequency. If channels are not synchronized in that
>>>> fashion
>>>> then you have to register multiple devices or only pick a subset of
>>>> channels
>>>> to export.
>>>>
>>>> For the key detection you have already observed that IIO needs some
>>>> additions to be able to have consumers of what we term 'events' e.g.
>>>> threshold
>>>> interrupts.
>>>>
>>>> Looking at the lradc-k

Re: [PATCH 0/3] add support for Allwinner SoCs ADC

2016-07-01 Thread Quentin Schulz
On 29/06/2016 05:28, Chen-Yu Tsai wrote:
> Hi,
> 
> On Tue, Jun 28, 2016 at 4:45 PM, Quentin Schulz
> <quentin.sch...@free-electrons.com> wrote:
>> The Allwinner SoCs all have an ADC that can also act as a touchscreen 
>> controller
>> and a thermal sensor. The first four channels can be used either for the ADC 
>> or
>> the touchscreen and the fifth channel is used for the thermal sensor. We
>> currently have a driver for the two latter functions in
>> drivers/input/touchscreen/sun4i-ts.c but we don't have access to the ADC 
>> feature
>> at all.
>>
>> This adds initial support for Allwinner SoCs ADC with all features. Yet, the
>> touchscreen is not implemented but will be added later. To switch between
>> touchscreen and ADC modes, you need to poke few bits in registers and
>> (de)activate an interrupt (pen-up).
>> A MFD is provided to let the input driver activate the pen-up interrupt 
>> through
>> virtual interrupt, poke few bits via regmap and read data from the ADC driver
>> while both (and iio_hwmon) are probed by the MFD.
> 
> I take it that we are going to replace the original sun4i-ts driver
> with this new mfd one, and various sub device drivers?
> 

Yes, that's the spirit.

[...]

> One thing about iio-hwmon is that it doesn't support hwmon labels.
> Any chance you could improve this, so we see the same names in userspace?

Should be an easy patch: reading datasheet_name from iio_chan_spec of
the iio_channel in iio_hwmon_probe, if none then fall back to current
naming convention.

I don't think it is linked to this serie of patches so I prefer to look
at it later.

Thanks!


Re: [PATCH 2/3] iio: adc: add support for Allwinner SoCs ADC

2016-07-04 Thread Quentin Schulz
On 03/07/2016 17:43, Guenter Roeck wrote:
> On 07/03/2016 04:54 AM, Jonathan Cameron wrote:
>> On 28/06/16 09:18, Quentin Schulz wrote:
>>> The Allwinner SoCs all have an ADC that can also act as a touchscreen
>>> controller and a thermal sensor. This patch adds the ADC driver which is
>>> based on the MFD for the same SoCs ADC.
>>>
>>> This also registers the thermal adc channel in the iio map array so
>>> iio_hwmon could use it without modifying the Device Tree.
>>>
>>> This driver probes on three different platform_device_id to take into
>>> account slight differences between Allwinner SoCs ADCs.
>>>
>>> Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
>> Hi Quentin.
>>
>> I'm a bit in two minds about some of this.  That temperature sensor is
>> so obviously meant for hwmon purposes, I'm tempted to suggest it might
>> actually make sense to put it directly in hwmon rather than using the
>> bridge.  That obviously makes it less flexible in some ways (i.e. for
>> use within the thermal subsystem at some point).
>>
>> Guenter, what do you think?
>>
> 
> With the upcoming new hwmon API, thermal registration is handled in the
> hwmon core, so that should not be an issue. Besides, other hwmon sensors
> already register with the thermal subsystem as well.
> 
> This is difficult to evaluate without datasheet; I am not sure if
> the chip supports limits or trip points. If it supports trip points,
> thermal may be a better target.
> 
> Overall it does look like the temperature sensor would warrant
> a separate driver. Only question is thermal or hwmon.
> 
> Guenter

Actually, the publicly available documentation for the temperature
sensor is almost inexistent. We only have the registers for it. For most
of the work on temperature sensor, it has been taken from the current
driver (sun4i-ts in input/touschreen) from Hans de Goede.

You can find the documentation here:
http://dl.linux-sunxi.org/A10/A10%20User%20manual%20V1.50.pdf
http://dl.linux-sunxi.org/A13/A13%20User%20Manual%20V1.30.pdf
http://dl.linux-sunxi.org/A31/A31%20User%20Manual%20V1.20.pdf

The ADC is either called TP controller, Touch Panel controller or GPADC.
The temperature sensor's registers are only defined in the User Manual
of the A10 (first link).

Nothing to be found for limits or trip points I think.

Quentin

>> I'm guessing detailed docs for this part aren't avaiable publicly? :(
>>
>> So the rest of my comments are kind of predicated on me having roughtly
>> understood how this device works from the structure of the driver.
>>
>> The temperature sensor is really effectively as separate ADC?
>> The main interest in this is for key detection?
>>
>> Anyhow, if the data flow for the temperatures sensor is not synced with
>> the other ADC channels, adding buffered (pushed) output from the
>> driver in
>> future will be fiddly and with a 250Hz device you'll probably want it.
>> Basically IIO buffered supports assumes each iio device will sample data
>> at a particular frequency. If channels are not synchronized in that
>> fashion
>> then you have to register multiple devices or only pick a subset of
>> channels
>> to export.
>>
>> For the key detection you have already observed that IIO needs some
>> additions to be able to have consumers of what we term 'events' e.g.
>> threshold
>> interrupts.
>>
>> Looking at the lradc-keys driver in tree, it looks like we only really
>> have
>> really simple threshold interrups - configured to detect a very low
>> voltage?
>> + only one per channel.
>>
>> So not too nasty a case, but you are right some work is needed in IIO as
>> we simply don't have a means of passing these on as yet or configuring
>> them
>> from in kernel consumers.
>> If we take the easy route and don't demux incoming events then it
>> shouldn't
>> be too hard to add (demux can follow later).  Hence any client device
>> can try
>> to enable events it wants, but may get events that other client
>> devices wanted
>> as well.
>>
>> Config interface should be much the same as the write support for
>> channels.
>> Data flow marginally harder, but pretty much a list of callbacks within
>> iio_push_event.
>>
>> Not trivial, but not too tricky either.
>>
>> The events subsystem has a few 'limitations' we need to address long term
>> but as this is in kernel interface only, we can do this now and fix stuff
>> up in future without any ABI breakage. (limitations are things like only
>> one event of a give

Re: [PATCH v2 16/25] dt-bindings: power: supply: add AXP20X/AXP22X battery DT binding

2017-01-31 Thread Quentin Schulz
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=148411561025684=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=148411561725693=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



signature.asc
Description: OpenPGP digital signature


Re: [PATCH v9 3/3] iio: adc: add support for Allwinner SoCs ADC

2017-02-05 Thread Quentin Schulz
Hi Jonathan,

On 14/01/2017 20:28, Jonathan Cameron wrote:
> 
> 
> On 14 January 2017 19:19:58 GMT+00:00, Quentin Schulz 
> <quentin.sch...@free-electrons.com> wrote:
>> Hi Jonathan,
>>
>> On 08/01/2017 12:17, Jonathan Cameron wrote:
>>> On 30/12/16 14:40, Jonathan Cameron wrote:
>>>> On 13/12/16 14:33, Quentin Schulz wrote:
>>>>> The Allwinner SoCs all have an ADC that can also act as a
>> touchscreen
>>>>> controller and a thermal sensor. This patch adds the ADC driver
>> which is
>>>>> based on the MFD for the same SoCs ADC.
>>>>>
>>>>> This also registers the thermal adc channel in the iio map array so
>>>>> iio_hwmon could use it without modifying the Device Tree. This
>> registers
>>>>> the driver in the thermal framework.
>>>>>
>>>>> The thermal sensor requires the IP to be in touchscreen mode to
>> return
>>>>> correct values. Therefore, if the user is continuously reading the
>> ADC
>>>>> channel(s), the thermal framework in which the thermal sensor is
>>>>> registered will switch the IP in touchscreen mode to get a
>> temperature
>>>>> value and requires a delay of 100ms (because of the mode
>> switching),
>>>>> then the ADC will switch back to ADC mode and requires also a delay
>> of
>>>>> 100ms. If the ADC readings are critical to user and the SoC
>> temperature
>>>>> is not, this driver is capable of not registering the thermal
>> sensor in
>>>>> the thermal framework and thus, "quicken" the ADC readings.
>>>>>
>>>>> This driver probes on three different platform_device_id to take
>> into
>>>>> account slight differences (registers bit and temperature
>> computation)
>>>>> between Allwinner SoCs ADCs.
>>>>>
>>>>> Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
>>>>> Acked-by: Maxime Ripard <maxime.rip...@free-electrons.com>
>>>>> Acked-by: Jonathan Cameron <ji...@kernel.org>
>>>>> Acked-for-MFD-by: Lee Jones <lee.jo...@linaro.org>
>>>> One comment inline but not a blocker.
>>>>
>>>> I would ideally like an ack from the thermal side.  The relevant
>> code
>>>> is small, but best to be sure and keep them in the loop as well.
>>>>
>>>> It does feel a little convoluted to have both this directly
>> providing
>>>> a thermal zone and being able to create one indirectly through hwmon
>> as
>>>> well but this solution works for me I think...
>>>>
>>>> Cc'd Zang and Eduardo.
>>> Nothing seems to have come through on that front.
>>>
>>> I need to get a pull request out to Greg and rebase my tree before I
>> have
>>> the precursor patch in place. Give me a bump if you haven't heard
>> anything by
>>> the time next week.
>>>
>>
>> Kindly "giving you a bump" you as requested since I haven't heard from
>> you for a week.
> Greg hasn't pulled yet, so may be a few more days.
> 
> J

I haven't received any news from you on the merging of this patch series
for a month, so kindly pinging.

Thanks,
Quentin

>>
>> Thanks,
>> Quentin
>>
>>> Thanks,
>>>
>>> Jonathan
>>>>
>>>> Jonathan
>>>>> ---
>>>>>
>>>>> v9:
>>>>>  - clarify comment on why we have to use the parent node as node
>> for
>>>>>  registering in thermal framework, (backward compatibility)
>>>>>  - clarify comment on why we can disable CONFIG_THERMAL_OF,
>>>>>  - clarify Kconfig help to say that CONFIG_THERMAL_OF can be
>> disabled
>>>>>  but should not in most cases,
>>>>>  - make return value of devm_thermal_zone_of_sensor_register a
>> local
>>>>>  variable of the condition block,
>>>>>  - correct scale from _PLUS_MICRO to _PLUS_NANO for ADC raw
>> readings
>>>>>  scale,
>>>>>
>>>>> v8:
>>>>>  - remove Kconfig depends on !TOUCHSCREEN_SUN4I (moved to
>>>>> MFD_SUN4I_GPADC),
>>>>>  - fix return values of regmap_irq_get_virq and
>> platform_get_irq_byname
>>>>> stored in an unsigned int and then check if negative,
>>>>>  - fix uninitialized ret value when

Re: [PATCH 08/22] power: supply: add AC power supply driver for AXP20X and AXP22X PMICs

2017-01-26 Thread Quentin Schulz
Hi Sebastian,

On 17/01/2017 04:00, Sebastian Reichel wrote:
> Hi Quentin,
> 
> The driver looks mostly fine. I do have a two comments, though.
> 
> On Mon, Jan 02, 2017 at 05:37:08PM +0100, Quentin Schulz wrote:
>> [...]
>>
>> +static int axp20x_ac_power_probe(struct platform_device *pdev)
>> +{
>> +static const char * const axp20x_irq_names[] = { "ACIN_PLUGIN",
>> +"ACIN_REMOVAL", NULL };
>>
>> +static const char * const *irq_names;
>> +const struct power_supply_desc *ac_power_desc;
>> +int i, irq, ret;
>> +
>> +if (!of_device_is_available(pdev->dev.of_node))
>> +return -ENODEV;
>> +
>> +if (!axp20x) {
>> +dev_err(>dev, "Parent drvdata not set\n");
>> +return -EINVAL;
>> +}
> 
> axp20x will no longer be needed after implementing below
> comments.
> 
>> [...]
>> +irq_names = axp20x_irq_names;
> 
> Just rename axp20x_irq_names into irq_names, since its only used
> here.
> 
>> [...]
>>
>> +power->np = pdev->dev.of_node;
> 
> This can be dropped, it's not used at all.
> 
>> +power->regmap = axp20x->regmap;
> 
> power->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> 

ACK on everything above.

>> [...]
>> +/* 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(>dev, "No IRQ for %s: %d\n",
>> + irq_names[i], irq);
>> +continue;
>> +}
>> +irq = regmap_irq_get_virq(axp20x->regmap_irqc, irq);
> 
> The mapping should actually happen in the mfd driver, so that
> the platform resource contains a valid irq.
> 

I've come with this solution:


diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 012c064..117eacb 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -882,7 +882,7 @@ EXPORT_SYMBOL(axp20x_match_device);

 int axp20x_device_probe(struct axp20x_dev *axp20x)
 {
-   int ret;
+   int ret, irq_base;

ret = regmap_add_irq_chip(axp20x->regmap, axp20x->irq,
  IRQF_ONESHOT | IRQF_SHARED, -1,
@@ -893,8 +893,9 @@ int axp20x_device_probe(struct axp20x_dev *axp20x)
return ret;
}

+   irq_base = regmap_irq_chip_get_base(axp20x->regmap_irqc);
ret = mfd_add_devices(axp20x->dev, -1, axp20x->cells,
- axp20x->nr_cells, NULL, 0, NULL);
+ axp20x->nr_cells, NULL, irq_base, NULL);

if (ret) {
dev_err(axp20x->dev, "failed to add MFD devices: %d\n", ret);


However, this implies that all cells added by the mfd driver which are
requesting irqs will need to be changed in the same commit to remove the
regmap_irq_get_virq calls. If we don't modify the drivers, they will
purely fail to request the irqs.

The impacted drivers are the following:

 - drivers/extcon/extcon-axp288.c
 - drivers/input/misc/axp20x-pek.c
 - drivers/power/supply/axp20x_usb_power.c
 - drivers/power/supply/axp288_charger.c
 - drivers/power/supply/axp288_fuel_gauge.c

Is it really worth to do such a cleanup? I'm assuming that impacting
four different subsystems at the same time might require a bit of time
to make the patch into the kernel. I don't see also another way than
doing one single patch for all changes since the changes in the mfd
driver will break all aforementioned drivers.

Thanks,
Quentin

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


[PATCH v2 22/25] ARM: dtsi: axp209: add battery power supply subnode

2017-01-27 Thread Quentin Schulz
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.sch...@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



[PATCH v2 24/25] ARM: dts: sun8i: sina33: enable battery power supply subnode

2017-01-27 Thread Quentin Schulz
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.sch...@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";
 };
 
+_power_supply {
+   status = "okay";
+};
+
 _aldo1 {
regulator-always-on;
regulator-min-microvolt = <300>;
-- 
2.9.3



[PATCH v2 15/25] ARM: sun5i: chip: enable ACIN power supply subnode

2017-01-27 Thread Quentin Schulz
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.sch...@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"
 
+_power_supply {
+   status = "okay";
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins_a>;
-- 
2.9.3



[PATCH v2 18/25] mfd: axp20x: add V_OFF to writeable regs for AXP20X and AXP22X

2017-01-27 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-for-MFD-by: Lee Jones <lee.jo...@linaro.org>
Acked-by: Chen-Yu Tsai <w...@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



[PATCH v2 23/25] ARM: dtsi: axp22x: add battery power supply subnode

2017-01-27 Thread Quentin Schulz
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.sch...@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



[PATCH v2 19/25] iio: adc: axp20x_adc: map battery IIO channels

2017-01-27 Thread Quentin Schulz
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.sch...@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



[PATCH v2 25/25] ARM: sun5i: chip: enable battery power supply subnode

2017-01-27 Thread Quentin Schulz
The NextThing Co. CHIP has an AXP209 PMIC with battery connector.

This enables the battery power supply subnode.

Signed-off-by: Quentin Schulz <quentin.sch...@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";
 };
 
+_power_supply {
+   status = "okay";
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins_a>;
-- 
2.9.3



[PATCH v2 20/25] power: supply: add battery driver for AXP20X and AXP22X PMICs

2017-01-27 Thread Quentin Schulz
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.sch...@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 000..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.sch...@free-electrons.com>
+ *
+ * This driver is based on a previous upstreaming attempt by:
+ * Bruno Prémont <bonb...@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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#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_VALIDBIT(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)
+

Re: [PATCH v2 04/25] iio: adc: add support for X-Powers AXP20X and AXP22X PMICs ADCs

2017-01-28 Thread Quentin Schulz
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.sch...@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 = 10;
>> +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(>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(>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 = >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(>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 = ii

[PATCH v2 17/25] mfd: axp20x: add CHRG_CTRL1/2/3 to writeable regs for AXP20X/AXP22X

2017-01-27 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-for-MFD-by: Lee Jones <lee.jo...@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



[PATCH v2 02/25] mfd: axp20x: correct name of temperature data ADC registers

2017-01-27 Thread Quentin Schulz
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.sch...@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_H0x58
 #define AXP22X_TS_ADC_L0x59
 #define AXP22X_BATLOW_THRES1   0xe6
-- 
2.9.3



[PATCH v2 14/25] ARM: dts: sun8i: sina33: enable ACIN power supply subnode

2017-01-27 Thread Quentin Schulz
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.sch...@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"
 
+_power_supply {
+   status = "okay";
+};
+
 _aldo1 {
regulator-always-on;
regulator-min-microvolt = <300>;
-- 
2.9.3



[PATCH v2 07/25] ARM: dtsi: axp22x: add AXP22X ADC subnode

2017-01-27 Thread Quentin Schulz
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.sch...@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



[PATCH v2 06/25] ARM: dtsi: axp209: add AXP209 ADC subnode

2017-01-27 Thread Quentin Schulz
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.sch...@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



[PATCH v2 13/25] ARM: dtsi: axp22x: add AC power supply subnode

2017-01-27 Thread Quentin Schulz
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.sch...@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



[PATCH v2 08/25] dt-bindings: power: supply: add AXP20X/AXP22X AC power supply

2017-01-27 Thread Quentin Schulz
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.sch...@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 000..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:
+
+ {
+   ac_power_supply: ac-power-supply {
+   compatible = "x-powers,axp202-ac-power-supply";
+   };
+};
-- 
2.9.3



[PATCH v2 11/25] mfd: axp20x: add AC power supply cells for AXP22X PMICs

2017-01-27 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-for-MFD-by: Lee Jones <lee.jo...@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



[PATCH v2 12/25] ARM: dtsi: axp209: add AC power supply subnode

2017-01-27 Thread Quentin Schulz
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.sch...@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



[PATCH v2 05/25] mfd: axp20x: add ADC cells for AXP20X and AXP22X PMICs

2017-01-27 Thread Quentin Schulz
This adds the AXP20X/AXP22x ADCs driver to the mfd cells of the AXP209,
AXP221 and AXP223 MFD.

Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
Acked-for-MFD-by: Lee Jones <lee.jo...@linaro.org>
Acked-by: Chen-Yu Tsai <w...@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



[PATCH v2 09/25] iio: adc: axp20x_adc: map acin_i and acin_v

2017-01-27 Thread Quentin Schulz
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.sch...@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



[PATCH v2 10/25] power: supply: add AC power supply driver for AXP20X and AXP22X PMICs

2017-01-27 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-by: Jonathan Cameron <ji...@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 000..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.sch...@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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#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, );
+   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, );
+   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, );
+   if (ret)
+   return ret;
+
+  

[PATCH v2 01/25] dt-bindings: iio: adc: add AXP20X/AXP22X ADC DT binding

2017-01-27 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-by: Rob Herring <r...@kernel.org>
Acked-by: Chen-Yu Tsai <w...@csie.org>
Acked-by: Maxime Ripard <maxime.rip...@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 000..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_adc: adc {
+   compatible = "x-powers,axp209-adc";
+   };
+};
-- 
2.9.3



[PATCH v2 00/25] add support for AXP20X and AXP22X power supply drivers

2017-01-27 Thread Quentin Schulz
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 f

[PATCH v2 03/25] power: supply: axp20x_usb_power: use IIO channels when available

2017-01-27 Thread Quentin Schulz
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.sch...@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 
 #include 
 #include 
+#include 
 
 #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,
+>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,
+>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(>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(>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

[PATCH v2 04/25] iio: adc: add support for X-Powers AXP20X and AXP22X PMICs ADCs

2017-01-27 Thread Quentin Schulz
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.sch...@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 000..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.sch...@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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#define AXP20X_ADC_EN1_MASKGENMASK(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, 

[PATCH v2 16/25] dt-bindings: power: supply: add AXP20X/AXP22X battery DT binding

2017-01-27 Thread Quentin Schulz
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.sch...@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 000..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:
+
+ {
+   battery_power_supply: battery-power-supply {
+   compatible = "x-powers,axp209-battery-power-supply";
+   x-powers,constant-charge-current = <60>;
+   }
+};
-- 
2.9.3



[PATCH v2 21/25] mfd: axp20x: add MFD cells for AXP20X and AXP22X battery driver

2017-01-27 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-for-MFD-by: Lee Jones <lee.jo...@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



Re: [PATCH v3 11/18] dt-bindings: power: supply: add AXP20X/AXP22X battery DT binding

2017-02-21 Thread Quentin Schulz
Hi Chen-Yu,

On 21/02/2017 05:45, Chen-Yu Tsai wrote:
> On Tue, Feb 14, 2017 at 5:41 PM, Quentin Schulz
> <quentin.sch...@free-electrons.com> wrote:
>> 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.sch...@free-electrons.com>
>> Acked-by: Maxime Ripard <maxime.rip...@free-electrons.com>
>> ---
>>
>> v3:
>>  - removed constant charge current property, now should use the WIP
>>  battery framework,
> 
> IIRC this should also include a property for referencing the battery?
> 

Indeed.

Quentin

> Otherwise,
> 
> Acked-by: Chen-Yu Tsai <w...@csie.org>
> 
>>
>> 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 | 20 
>> 
>>  1 file changed, 20 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 000..c248866
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
>> @@ -0,0 +1,20 @@
>> +AXP20x and AXP22x battery power supply
>> +
>> +Required Properties:
>> + - compatible, one of:
>> +   "x-powers,axp209-battery-power-supply"
>> +   "x-powers,axp221-battery-power-supply"
>> +
>> +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:
>> +
>> + {
>> +   battery_power_supply: battery-power-supply {
>> +   compatible = "x-powers,axp209-battery-power-supply";
>> +   }
>> +};
>> --
>> 2.9.3
>>

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


Re: [PATCH v3 13/18] power: supply: add battery driver for AXP20X and AXP22X PMICs

2017-02-21 Thread Quentin Schulz
Hi Chen-Yu,

On 21/02/2017 05:44, Chen-Yu Tsai wrote:
> On Tue, Feb 14, 2017 at 5:41 PM, Quentin Schulz
> <quentin.sch...@free-electrons.com> 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.sch...@free-electrons.com>
>> Acked-by: Jonathan Cameron <ji...@kernel.org>
>> Acked-by: Maxime Ripard <maxime.rip...@free-electrons.com>
[...]
>> +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 - 30) / 10;
>> +   else
>> +   charge_current = (charge_current - 30) / 15;
>> +
>> +   if (charge_current > AXP20X_CHRG_CTRL1_TGT_CURR || charge_current < 
>> 0)
>> +   return -EINVAL;
> 
> I would add a check or warn if the value to be programmed exceeds that 
> returned
> by power_supply_get_battery_info.

Completely agree on a warning.

> A charge current exceeding the limits of the
> battery is potentially disastrous. The battery may be destroyed or even burst
> into flames and explode, taking the board and anything nearby with it. 
> Otherwise
> 

Yes, I understand. Maybe I'm seeing this ability to set the (max)
constant current charge the wrong way. Here is what I think:
If we limit the max constant charge current with a DT property, it would
require a DT rebuild when changing the battery (i.e. if an end-user
decides to change the battery with a bigger constant charge current, he
has to recompile the DT to change the DT value).

What I can suggest is the following:
 - set the max constant charge current and the default constant charge
current from the DT property,
 - allow the user to change the constant charge current via sysfs within
minimal-DT value range,
 - allow the user to set max constant charge current via sysfs (and
print a warning as well when setting it), then the user can set a higher
constant charge current,

That would require a two steps modification with a printed warning.
"Safer" but does not remove the ability to change the constant charge
current in the case of battery swapping/changing.

> Acked-by: Chen-Yu Tsai <w...@csie.org>
> 
> Speaking of power_supply_get_battery_info, is it merged or ready to be merged?
> 

v7 under way IIRC.

Thanks,
Quentin

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


Re: [PATCH v3 17/18] ARM: dts: sun8i: sina33: enable battery power supply subnode

2017-02-21 Thread Quentin Schulz
On 21/02/2017 05:50, Chen-Yu Tsai wrote:
> On Tue, Feb 14, 2017 at 5:41 PM, Quentin Schulz
> <quentin.sch...@free-electrons.com> wrote:
>> 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.sch...@free-electrons.com>
>> Acked-by: Maxime Ripard <maxime.rip...@free-electrons.com>
> 
> Battery charger enabled without any description of the associated battery...
> In this case I think the driver should use the lowest charge current to be
> safe?
> 

Seems fair.

Quentin

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


Re: [PATCH v3 04/18] iio: adc: add support for X-Powers AXP20X and AXP22X PMICs ADCs

2017-02-21 Thread Quentin Schulz
Hi Jonathan,

On 19/02/2017 13:40, Jonathan Cameron wrote:
> On 14/02/17 09:40, 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.sch...@free-electrons.com>
>> Acked-by: Maxime Ripard <maxime.rip...@free-electrons.com>
> Hi Quentin,
> 
> A few bits and bobs inline.  The bigest one is that I don't think
> you need to carry your struct axp_data pointer around in the iio_priv
> structure as it only seems to be used in probe.
> 
> Anyhow, tidy these little bits up (quite a few are optional in the
> sense that they are a matter or personal taste and I don't feel
> strongly about them) and you can add
> 
> Reviewed-by: Jonathan Cameron <ji...@kernel.org>
> 
> A nice driver.
> 
> Jonathan
[...]
>> +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(>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->dev.parent = >dev;
>> +indio_dev->dev.of_node = pdev->dev.of_node;
>> +indio_dev->modes = INDIO_DIRECT_MODE;
>> +
>> +info->data = (struct axp_data 
>> *)platform_get_device_id(pdev)->driver_data;
> I think this is only actually used in probe, so could be handled more
> neatly with a local variable rather than sticking it in info.
> 
[...]
>> +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);
> Reorder the two lines above this one and then you could just do this
> inline. (really trivial so feel free to not bother ;)
>> +
>> +iio_device_unregister(indio_dev);
>> +iio_map_array_unregister(indio_dev);
>> +
>> +    regmap_write(info->regmap, AXP20X_ADC_EN1, 0);
>> +
>> +if (info->data->adc_en2)

data is used here to know if I should disable misc ADCs (if supported by
the PMIC). So kinda need it only for that purpose, maybe I can use a
slightly simpler structure to only store regmap and this adc_en2 boolean.

Ack for everything else.

Thanks,
Quentin

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


[PATCH v3 11/18] dt-bindings: power: supply: add AXP20X/AXP22X battery DT binding

2017-02-14 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-by: Maxime Ripard <maxime.rip...@free-electrons.com>
---

v3:
 - removed constant charge current property, now should use the WIP
 battery framework,

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 | 20 
 1 file changed, 20 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 000..c248866
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
@@ -0,0 +1,20 @@
+AXP20x and AXP22x battery power supply
+
+Required Properties:
+ - compatible, one of:
+   "x-powers,axp209-battery-power-supply"
+   "x-powers,axp221-battery-power-supply"
+
+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:
+
+ {
+   battery_power_supply: battery-power-supply {
+   compatible = "x-powers,axp209-battery-power-supply";
+   }
+};
-- 
2.9.3



[PATCH v3 12/18] mfd: axp20x: add CHRG_CTRL1/2/3 to writeable regs for AXP20X/AXP22X

2017-02-14 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-by: Maxime Ripard <maxime.rip...@free-electrons.com>
Acked-by: Chen-Yu Tsai <w...@csie.org>
Acked-for-MFD-by: Lee Jones <lee.jo...@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 98abe4b..86bc1d5 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



[PATCH v3 17/18] ARM: dts: sun8i: sina33: enable battery power supply subnode

2017-02-14 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-by: Maxime Ripard <maxime.rip...@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";
 };
 
+_power_supply {
+   status = "okay";
+};
+
 _aldo1 {
regulator-always-on;
regulator-min-microvolt = <300>;
-- 
2.9.3



[PATCH v3 16/18] ARM: dtsi: axp22x: add battery power supply subnode

2017-02-14 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-by: Maxime Ripard <maxime.rip...@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 67331c5..87fb08e 100644
--- a/arch/arm/boot/dts/axp22x.dtsi
+++ b/arch/arm/boot/dts/axp22x.dtsi
@@ -57,6 +57,11 @@
status = "disabled";
};
 
+   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



[PATCH v3 18/18] ARM: sun5i: chip: enable battery power supply subnode

2017-02-14 Thread Quentin Schulz
The NextThing Co. CHIP has an AXP209 PMIC with battery connector.

This enables the battery power supply subnode.

Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
Acked-by: Maxime Ripard <maxime.rip...@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";
 };
 
+_power_supply {
+   status = "okay";
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins_a>;
-- 
2.9.3



[PATCH v3 14/18] mfd: axp20x: add MFD cells for AXP20X and AXP22X battery driver

2017-02-14 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-for-MFD-by: Lee Jones <lee.jo...@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 86bc1d5..b75d5c5 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -585,6 +585,9 @@ static struct mfd_cell axp20x_cells[] = {
}, {
.name   = "axp20x-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),
@@ -612,6 +615,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),
@@ -627,6 +633,9 @@ static struct mfd_cell axp223_cells[] = {
}, {
.name   = "axp22x-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



[PATCH v3 10/18] ARM: sun5i: chip: enable ACIN power supply subnode

2017-02-14 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-by: Chen-Yu Tsai <w...@csie.org>
Acked-by: Maxime Ripard <maxime.rip...@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"
 
+_power_supply {
+   status = "okay";
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins_a>;
-- 
2.9.3



[PATCH v3 06/18] mfd: axp20x: add AC power supply cells for AXP22X PMICs

2017-02-14 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-by: Maxime Ripard <maxime.rip...@free-electrons.com>
Acked-By: Sebastian Reichel <s...@kernel.org>
Acked-by: Chen-Yu Tsai <w...@csie.org>
Acked-for-MFD-by: Lee Jones <lee.jo...@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 a44a2fe..98abe4b 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -605,6 +605,11 @@ static struct mfd_cell axp221_cells[] = {
}, {
.name   = "axp22x-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),
@@ -622,6 +627,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



[PATCH v3 08/18] ARM: dtsi: axp22x: add AC power supply subnode

2017-02-14 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-by: Maxime Ripard <maxime.rip...@free-electrons.com>
Acked-by: Chen-Yu Tsai <w...@csie.org>
---

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 458b668..67331c5 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";
+   };
+
regulators {
/* Default work frequency for buck regulators */
x-powers,dcdc-freq = <3000>;
-- 
2.9.3



[PATCH v3 07/18] ARM: dtsi: axp209: add AC power supply subnode

2017-02-14 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-by: Chen-Yu Tsai <w...@csie.org>
Acked-by: Maxime Ripard <maxime.rip...@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 675bb0f..9677dd5 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";
+   };
+
axp_gpio: gpio {
compatible = "x-powers,axp209-gpio";
gpio-controller;
-- 
2.9.3



[PATCH v3 09/18] ARM: dts: sun8i: sina33: enable ACIN power supply subnode

2017-02-14 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-by: Chen-Yu Tsai <w...@csie.org>
Acked-by: Maxime Ripard <maxime.rip...@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"
 
+_power_supply {
+   status = "okay";
+};
+
 _aldo1 {
regulator-always-on;
regulator-min-microvolt = <300>;
-- 
2.9.3



[PATCH v3 01/18] dt-bindings: power: battery: add constant-charge-current property

2017-02-14 Thread Quentin Schulz
This adds the constant-charge-current property to the list of optional
properties of the battery.

The constant charge current is critical for batteries as they can't
handle all charge currents.

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

added in v3

 Documentation/devicetree/bindings/power/supply/battery.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt 
b/Documentation/devicetree/bindings/power/supply/battery.txt
index d78a4aa..3e830fe 100644
--- a/Documentation/devicetree/bindings/power/supply/battery.txt
+++ b/Documentation/devicetree/bindings/power/supply/battery.txt
@@ -7,6 +7,7 @@ Optional Properties:
  - voltage-min-design-microvolt: drained battery voltage
  - energy-full-design-microwatt-hours: battery design energy
  - charge-full-design-microamp-hours: battery design capacity
+ - constant-charge-current-microamp: battery constant charge current
 
 Future Properties must be named for the corresponding elements in
 enum power_supply_property, defined in include/linux/power_supply.h.
@@ -22,6 +23,7 @@ Example:
voltage-min-design-microvolt = <320>;
energy-full-design-microwatt-hours = <529>;
charge-full-design-microamp-hours = <143>;
+   constant-charge-current-microamp = <30>;
};
 
charger: charger@11 {
-- 
2.9.3



[PATCH v3 04/18] iio: adc: add support for X-Powers AXP20X and AXP22X PMICs ADCs

2017-02-14 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-by: Maxime Ripard <maxime.rip...@free-electrons.com>
---

v3:
 - moved from switch to if condition in axp20x_adc_raw and
 axp22x_adc_raw,
 - removed DT support as DT node has been dropped,
   - use of platform_device_id
 - correctly defined the name of the iio device (name used to probe the
 driver),
 - added goto for errors in probe,
 - added iio_map_array_unregister to the remove function,

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 | 606 +++
 3 files changed, 617 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 000..5ef6af8
--- /dev/null
+++ b/drivers/iio/adc/axp20x_adc.c
@@ -0,0 +1,606 @@
+/* ADC driver for AXP20X and AXP22X PMICs
+ *
+ * Copyright (c) 2016 Free Electrons NextThing Co.
+ * Quentin Schulz <quentin.sch...@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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#define AXP20X_ADC_EN1_MASKGENMASK(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) << 

[PATCH v3 15/18] ARM: dtsi: axp209: add battery power supply subnode

2017-02-14 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-by: Maxime Ripard <maxime.rip...@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 9677dd5..3c8fa26 100644
--- a/arch/arm/boot/dts/axp209.dtsi
+++ b/arch/arm/boot/dts/axp209.dtsi
@@ -64,6 +64,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



[PATCH v3 13/18] power: supply: add battery driver for AXP20X and AXP22X PMICs

2017-02-14 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-by: Jonathan Cameron <ji...@kernel.org>
Acked-by: Maxime Ripard <maxime.rip...@free-electrons.com>
---

v3:
 - added axp20x_set_voltage_min_design function so it can be reused,
 - used power_supply_get_battery_info for setting constant charge current
 instead of x-powers,constant-charge-current introduced in v2,
 - used power_supply_get_battery_info for setting voltage min design of
 the battery,

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 | 492 ++
 3 files changed, 505 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 000..bd16ac6
--- /dev/null
+++ b/drivers/power/supply/axp20x_battery.c
@@ -0,0 +1,492 @@
+/*
+ * Battery power supply driver for X-Powers AXP20X and AXP22X PMICs
+ *
+ * Copyright 2016 Free Electrons NextThing Co.
+ * Quentin Schulz <quentin.sch...@free-electrons.com>
+ *
+ * This driver is based on a previous upstreaming attempt by:
+ * Bruno Prémont <bonb...@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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#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_VALIDBIT(7)
+
+#define AXP20X_CHRG_CTRL

[PATCH v3 05/18] mfd: axp20x: add ADC cells for AXP20X and AXP22X PMICs

2017-02-14 Thread Quentin Schulz
This adds the AXP20X/AXP22x ADCs driver to the mfd cells of the AXP209,
AXP221 and AXP223 MFD.

Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
Acked-by: Maxime Ripard <maxime.rip...@free-electrons.com>
---

v3:
 - removed of_compatible as DT node has been removed,
 - use different names to probe the ADC driver,

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

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 9c2fd37..a44a2fe 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -581,6 +581,8 @@ static struct mfd_cell axp20x_cells[] = {
}, {
.name   = "axp20x-regulator",
}, {
+   .name   = "axp20x-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 +603,8 @@ static struct mfd_cell axp221_cells[] = {
}, {
.name   = "axp20x-regulator",
}, {
+   .name   = "axp22x-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 +618,8 @@ static struct mfd_cell axp223_cells[] = {
.num_resources  = ARRAY_SIZE(axp22x_pek_resources),
.resources  = axp22x_pek_resources,
}, {
+   .name   = "axp22x-adc",
+   }, {
.name   = "axp20x-regulator",
}, {
.name   = "axp20x-usb-power-supply",
-- 
2.9.3



Re: [PATCH v3 01/18] dt-bindings: power: battery: add constant-charge-current property

2017-02-15 Thread Quentin Schulz
Hi,

On 15/02/2017 01:46, Liam Breck wrote:
> 
> On Tue, 14 Feb 2017 10:40:55 +0100 Quentin Schulz wrote:
>> This adds the constant-charge-current property to the list of optional
>> properties of the battery.
>>
>> The constant charge current is critical for batteries as they can't
>> handle all charge currents.
>>
>> Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
>> ---
>>
>> added in v3
>>
>>  Documentation/devicetree/bindings/power/supply/battery.txt | 2 ++
> 
> Is constant-charge-current dependent on the battery (e.g. capacity, nominal 
> voltage, etc) or the 
> system (charger chip, input current/voltage, etc)?
> 
> It belongs in Doc.../power/supply/battery.txt if it's a characteristic of the 
> battery.
> 
> Note, this page asserts that constant-current charging applies to NiMH 
> batteries:
> http://power-topics.blogspot.com/2016/05/constant-voltage-constant-current.html
> 
> Related properties to be added to battery.txt near-future in a patchset for 
> the BQ24190 
> charger are as follows. These are not currently in enum 
> power_supply_property, so the actual names 
> are still to be decided.
> 
> precharge-current-microamp:
>maximum charge current during precharge phase (typically 20% of battery 
> capacity)
> 
> termination-current-microamp (or endcharge-current):
>a charge cycle terminates when the battery voltage is above recharge 
> threshold,
>and the current is below this setting (typically 10% of battery capacity)
> 

We have a client with a board whose battery accepts a maximum of 300mA
for charging. So depending on the battery, we cannot have any charging
current we want. The AXP PmMICs set constant charge current in a range
of 300mA-1800mA, so it is enforced by the charger but needs to be
adapted depending on the battery present in the system.

The AXP PMICs charge battery with constant current (Ichrg) between the
trickle voltage (Vtrkl which is ~3.0V) and the targeted voltage (Vtrgt;
which seems to be the voltage telling the battery is fully charged).

So if I understand correctly, "my" constant-charge-current would be
located in the charging cycle between your precharge-current-microamp
and the termination-current-microamp as it is the current for the
charging process as a whole.

See here[1] for the explanation in the datasheet (page 20).

That would definitely match what is explained in your link for constant
current.

[1] http://dl.linux-sunxi.org/AXP/AXP209_Datasheet_v1.0en.pdf

Let me know if something seems odd,
Thanks,
Quentin

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


[PATCH v3 00/18] add support for AXP20X and AXP22X power supply drivers

2017-02-14 Thread Quentin Schulz
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.

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_v3

v3:
 - Removed DT property for constant charge current in favor of the WIP
 battery framework as requested by Sebastian Reichel,
 - Using a simple if condition instead of a switch in the ADC driver,
 - Fixed error handling in ADC driver's probe,
 - Fixed missing call to iio_map_array_unregister in the ADC driver's
 remove,
 - Removed ADC driver's DT node and documentation,
 - Merged IIO channel mapping patches into the original ADC driver
 patch,
 - Removed `adding V-OFF to writeable reg' patch as it's already in
 writeable reg range,

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 (18):
  dt-bindings: power: battery: add constant-charge-current property
  power: supply: power_supply_core: add constant-current-charge optional
property
  mfd: axp20x: correct name of temperature data ADC registers
  iio: adc: add support for X-Powers AXP20X and AXP22X PMICs ADCs
  mfd: axp20x: add ADC cells 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
  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: enab

[PATCH v3 02/18] power: supply: power_supply_core: add constant-current-charge optional property

2017-02-14 Thread Quentin Schulz
This adds the constant-current-charge property to the list of optional
properties for the battery.

The constant charge current is critical for batteries as they can't
handle all charge currents.

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

added in v3

 drivers/power/supply/power_supply_core.c | 3 +++
 include/linux/power_supply.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/power/supply/power_supply_core.c 
b/drivers/power/supply/power_supply_core.c
index ced8fef..c844def 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -498,6 +498,7 @@ int power_supply_get_battery_info(struct power_supply *psy,
info->energy_full_design_uwh = -EINVAL;
info->charge_full_design_uah = -EINVAL;
info->voltage_min_design_uv  = -EINVAL;
+   info->constant_charge_current_ua  = -EINVAL;
 
if (!psy->of_node) {
dev_warn(>dev, "%s currently only supports devicetree\n",
@@ -522,6 +523,8 @@ int power_supply_get_battery_info(struct power_supply *psy,
 >charge_full_design_uah);
of_property_read_u32(battery_np, "voltage-min-design-microvolt",
 >voltage_min_design_uv);
+   of_property_read_u32(battery_np, "constant-charge-current-microamp",
+>constant_charge_current_ua);
 
return 0;
 }
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index e84f1d3..f723a8e 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -301,6 +301,7 @@ struct power_supply_battery_info {
int energy_full_design_uwh; /* microWatt-hours */
int charge_full_design_uah; /* microAmp-hours */
int voltage_min_design_uv;  /* microVolts */
+   int constant_charge_current_ua; /* microAmps */
 };
 
 extern struct atomic_notifier_head power_supply_notifier;
-- 
2.9.3



[PATCH v3 03/18] mfd: axp20x: correct name of temperature data ADC registers

2017-02-14 Thread Quentin Schulz
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.sch...@free-electrons.com>
Acked-by: Chen-Yu Tsai <w...@csie.org>
Acked-by: Maxime Ripard <maxime.rip...@free-electrons.com>
Acked-for-MFD-by: Lee Jones <lee.jo...@linaro.org>
---

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_H0x58
 #define AXP22X_TS_ADC_L0x59
 #define AXP22X_BATLOW_THRES1   0xe6
-- 
2.9.3



  1   2   3   4   5   6   7   8   9   10   >