[PATCH v2 0/4] mfd/power: axp20x-usb-power driver

[PATCH v2 0/4] mfd/power: axp20x-usb-power driver

[Hans de Goede]

Hi All,

Here is v2 of my axp20x-usb-power driver, addressing all review remarks
from Lee Jones review of v1 (see individual commit messages for details).

I'm not resending the dts bits as those are unchanged. I'll resend them
to Maxime for merging once the axp20x-usb-power mfd bits and
power_supply-driver have landed.

Regards,

Hans

[PATCH v2 0/4] mfd/power: axp20x-usb-power driver

[Hans de Goede]

Hi All,

Here is v2 of my axp20x-usb-power driver, addressing all review remarks
from Lee Jones review of v1 (see individual commit messages for details).

I'm not resending the dts bits as those are unchanged. I'll resend them
to Maxime for merging once the axp20x-usb-power mfd bits and
power_supply-driver have landed.

Regards,

Hans

[PATCH v2 1/4] ARM: dts: Add binding documentation for AXP20x pmic usb power supply

[Hans de Goede]

Add binding documentation for the usb power supply part of the AXP20x pmic.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
Changes in v2:
-Split out into a separate patch from the actual driver commit
---
 .../bindings/power_supply/axp20x_usb_power.txt     | 34 ++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt

diff --git a/Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt b/Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt
new file mode 100644
index 0000000..a84d801
--- /dev/null
+++ b/Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt
@@ -0,0 +1,34 @@
+AXP20x USB power supply
+
+Required Properties:
+-compatible: "x-powers,axp202-usb-power-supply"
+
+This node is a subnode of the axp20x PMIC.
+
+Example:
+
+axp209: pmic@34 {
+	compatible = "x-powers,axp209";
+	reg = <0x34>;
+	interrupt-parent = <&nmi_intc>;
+	interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+	interrupt-controller;
+	#interrupt-cells = <1>;
+
+	regulators {
+		x-powers,dcdc-freq = <1500>;
+
+		vdd_cpu: dcdc2 {
+			regulator-always-on;
+			regulator-min-microvolt = <1000000>;
+			regulator-max-microvolt = <1450000>;
+			regulator-name = "vdd-cpu";
+		};
+
+		...
+	};
+
+	usb_power_supply: usb_power_supply {
+		compatible = "x-powers,axp202-usb-power-supply";
+	};
+};
-- 
2.4.3

[PATCH v2 1/4] ARM: dts: Add binding documentation for AXP20x pmic usb power supply

[Hans de Goede]

Add binding documentation for the usb power supply part of the AXP20x pmic.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Split out into a separate patch from the actual driver commit
---
 .../bindings/power_supply/axp20x_usb_power.txt     | 34 ++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt

diff --git a/Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt b/Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt
new file mode 100644
index 0000000..a84d801
--- /dev/null
+++ b/Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt
@@ -0,0 +1,34 @@
+AXP20x USB power supply
+
+Required Properties:
+-compatible: "x-powers,axp202-usb-power-supply"
+
+This node is a subnode of the axp20x PMIC.
+
+Example:
+
+axp209: pmic at 34 {
+	compatible = "x-powers,axp209";
+	reg = <0x34>;
+	interrupt-parent = <&nmi_intc>;
+	interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+	interrupt-controller;
+	#interrupt-cells = <1>;
+
+	regulators {
+		x-powers,dcdc-freq = <1500>;
+
+		vdd_cpu: dcdc2 {
+			regulator-always-on;
+			regulator-min-microvolt = <1000000>;
+			regulator-max-microvolt = <1450000>;
+			regulator-name = "vdd-cpu";
+		};
+
+		...
+	};
+
+	usb_power_supply: usb_power_supply {
+		compatible = "x-powers,axp202-usb-power-supply";
+	};
+};
-- 
2.4.3

[PATCH v2 2/4] mfd: axp20x: Add missing registers, and mark more registers volatile

[Hans de Goede]

From: Bruno Prémont <bonbons-ud5FBsm0p/xEiooADzr8i9i2O/JbrIOy@public.gmane.org>

Add an extra set of registers which is necessary tu support the PMICs
battery charger function, and mark registers which contain status bits,
gpio status, and adc readings as volatile.

Cc: Bruno Prémont <bonbons-ud5FBsm0p/xEiooADzr8i9i2O/JbrIOy@public.gmane.org>
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
Changes in v2:
-Add a AXP20X_OCV_MAX define
---
 drivers/mfd/axp20x.c       | 8 +++++++-
 include/linux/mfd/axp20x.h | 6 ++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 6df9155..f9a3c2d 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -39,10 +39,16 @@ static const char * const axp20x_model_names[] = {
 static const struct regmap_range axp20x_writeable_ranges[] = {
 	regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE),
 	regmap_reg_range(AXP20X_DCDC_MODE, AXP20X_FG_RES),
+	regmap_reg_range(AXP20X_RDC_H, AXP20X_OCV(15)),
 };
 
 static const struct regmap_range axp20x_volatile_ranges[] = {
+	regmap_reg_range(AXP20X_PWR_INPUT_STATUS, AXP20X_USB_OTG_STATUS),
+	regmap_reg_range(AXP20X_CHRG_CTRL1, AXP20X_CHRG_CTRL2),
 	regmap_reg_range(AXP20X_IRQ1_EN, AXP20X_IRQ5_STATE),
+	regmap_reg_range(AXP20X_ACIN_V_ADC_H, AXP20X_IPSOUT_V_HIGH_L),
+	regmap_reg_range(AXP20X_GPIO20_SS, AXP20X_GPIO3_CTRL),
+	regmap_reg_range(AXP20X_FG_RES, AXP20X_RDC_L),
 };
 
 static const struct regmap_access_table axp20x_writeable_table = {
@@ -159,7 +165,7 @@ static const struct regmap_config axp20x_regmap_config = {
 	.val_bits	= 8,
 	.wr_table	= &axp20x_writeable_table,
 	.volatile_table	= &axp20x_volatile_table,
-	.max_register	= AXP20X_FG_RES,
+	.max_register	= AXP20X_OCV(AXP20X_OCV_MAX),
 	.cache_type	= REGCACHE_RBTREE,
 };
 
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 5275423..6d8b39a 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -151,6 +151,12 @@ enum {
 #define AXP20X_CC_CTRL			0xb8
 #define AXP20X_FG_RES			0xb9
 
+/* OCV */
+#define AXP20X_RDC_H			0xba
+#define AXP20X_RDC_L			0xbb
+#define AXP20X_OCV(m)			(0xc0 + (m))
+#define AXP20X_OCV_MAX			0xf
+
 /* AXP22X specific registers */
 #define AXP22X_BATLOW_THRES1		0xe6
 
-- 
2.4.3

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

[PATCH v2 2/4] mfd: axp20x: Add missing registers, and mark more registers volatile

[Hans de Goede]

From: Bruno Pr?mont <bonbons@linux-vserver.org>

Add an extra set of registers which is necessary tu support the PMICs
battery charger function, and mark registers which contain status bits,
gpio status, and adc readings as volatile.

Cc: Bruno Pr?mont <bonbons@linux-vserver.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Add a AXP20X_OCV_MAX define
---
 drivers/mfd/axp20x.c       | 8 +++++++-
 include/linux/mfd/axp20x.h | 6 ++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 6df9155..f9a3c2d 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -39,10 +39,16 @@ static const char * const axp20x_model_names[] = {
 static const struct regmap_range axp20x_writeable_ranges[] = {
 	regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE),
 	regmap_reg_range(AXP20X_DCDC_MODE, AXP20X_FG_RES),
+	regmap_reg_range(AXP20X_RDC_H, AXP20X_OCV(15)),
 };
 
 static const struct regmap_range axp20x_volatile_ranges[] = {
+	regmap_reg_range(AXP20X_PWR_INPUT_STATUS, AXP20X_USB_OTG_STATUS),
+	regmap_reg_range(AXP20X_CHRG_CTRL1, AXP20X_CHRG_CTRL2),
 	regmap_reg_range(AXP20X_IRQ1_EN, AXP20X_IRQ5_STATE),
+	regmap_reg_range(AXP20X_ACIN_V_ADC_H, AXP20X_IPSOUT_V_HIGH_L),
+	regmap_reg_range(AXP20X_GPIO20_SS, AXP20X_GPIO3_CTRL),
+	regmap_reg_range(AXP20X_FG_RES, AXP20X_RDC_L),
 };
 
 static const struct regmap_access_table axp20x_writeable_table = {
@@ -159,7 +165,7 @@ static const struct regmap_config axp20x_regmap_config = {
 	.val_bits	= 8,
 	.wr_table	= &axp20x_writeable_table,
 	.volatile_table	= &axp20x_volatile_table,
-	.max_register	= AXP20X_FG_RES,
+	.max_register	= AXP20X_OCV(AXP20X_OCV_MAX),
 	.cache_type	= REGCACHE_RBTREE,
 };
 
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 5275423..6d8b39a 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -151,6 +151,12 @@ enum {
 #define AXP20X_CC_CTRL			0xb8
 #define AXP20X_FG_RES			0xb9
 
+/* OCV */
+#define AXP20X_RDC_H			0xba
+#define AXP20X_RDC_L			0xbb
+#define AXP20X_OCV(m)			(0xc0 + (m))
+#define AXP20X_OCV_MAX			0xf
+
 /* AXP22X specific registers */
 #define AXP22X_BATLOW_THRES1		0xe6
 
-- 
2.4.3

[PATCH v2 3/4] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Hans de Goede]

Add a cell for the usb power_supply part of the axp20x PMICs.

Note that this cell is only for the usb power_supply part and not the
ac-power / battery-charger / rtc-backup-bat-charger bits.

Depending on the board each of those must be enabled / disabled separately
in devicetree as most boards do not use all 4. So in dt each one needs its
own child-node of the axp20x node. Another reason for using separate child
nodes for each is so that other devicetree nodes can have a power-supply
property with a phandle referencing a node representing a single
power-supply.

The decision to use a separate devicetree node for each is reflected on
the kernel side by each getting its own mfd-cell / platform_device and
platform-driver.

Cc: Bruno Prémont <bonbons-ud5FBsm0p/xEiooADzr8i9i2O/JbrIOy@public.gmane.org>
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
Changes in v2:
-Use DEFINE_RES_IRQ_NAMED
-Change indentation of axp20x_cells initializers to avoid line wrapping
---
 drivers/mfd/axp20x.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index f9a3c2d..ca4a604 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -113,6 +113,13 @@ static struct resource axp20x_pek_resources[] = {
 	},
 };
 
+static struct resource axp20x_usb_power_supply_resources[] = {
+	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_PLUGIN, "VBUS_PLUGIN"),
+	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_REMOVAL, "VBUS_REMOVAL"),
+	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_VALID, "VBUS_VALID"),
+	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_NOT_VALID, "VBUS_NOT_VALID"),
+};
+
 static struct resource axp22x_pek_resources[] = {
 	{
 		.name   = "PEK_DBR",
@@ -363,11 +370,16 @@ static const struct regmap_irq_chip axp288_regmap_irq_chip = {
 
 static struct mfd_cell axp20x_cells[] = {
 	{
-		.name			= "axp20x-pek",
-		.num_resources		= ARRAY_SIZE(axp20x_pek_resources),
-		.resources		= axp20x_pek_resources,
+		.name		= "axp20x-pek",
+		.num_resources	= ARRAY_SIZE(axp20x_pek_resources),
+		.resources	= axp20x_pek_resources,
 	}, {
-		.name			= "axp20x-regulator",
+		.name		= "axp20x-regulator",
+	}, {
+		.name		= "axp20x-usb-power-supply",
+		.of_compatible	= "x-powers,axp202-usb-power-supply",
+		.num_resources	= ARRAY_SIZE(axp20x_usb_power_supply_resources),
+		.resources	= axp20x_usb_power_supply_resources,
 	},
 };
 
-- 
2.4.3

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

[PATCH v2 3/4] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Hans de Goede]

Add a cell for the usb power_supply part of the axp20x PMICs.

Note that this cell is only for the usb power_supply part and not the
ac-power / battery-charger / rtc-backup-bat-charger bits.

Depending on the board each of those must be enabled / disabled separately
in devicetree as most boards do not use all 4. So in dt each one needs its
own child-node of the axp20x node. Another reason for using separate child
nodes for each is so that other devicetree nodes can have a power-supply
property with a phandle referencing a node representing a single
power-supply.

The decision to use a separate devicetree node for each is reflected on
the kernel side by each getting its own mfd-cell / platform_device and
platform-driver.

Cc: Bruno Pr?mont <bonbons@linux-vserver.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Use DEFINE_RES_IRQ_NAMED
-Change indentation of axp20x_cells initializers to avoid line wrapping
---
 drivers/mfd/axp20x.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index f9a3c2d..ca4a604 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -113,6 +113,13 @@ static struct resource axp20x_pek_resources[] = {
 	},
 };
 
+static struct resource axp20x_usb_power_supply_resources[] = {
+	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_PLUGIN, "VBUS_PLUGIN"),
+	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_REMOVAL, "VBUS_REMOVAL"),
+	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_VALID, "VBUS_VALID"),
+	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_NOT_VALID, "VBUS_NOT_VALID"),
+};
+
 static struct resource axp22x_pek_resources[] = {
 	{
 		.name   = "PEK_DBR",
@@ -363,11 +370,16 @@ static const struct regmap_irq_chip axp288_regmap_irq_chip = {
 
 static struct mfd_cell axp20x_cells[] = {
 	{
-		.name			= "axp20x-pek",
-		.num_resources		= ARRAY_SIZE(axp20x_pek_resources),
-		.resources		= axp20x_pek_resources,
+		.name		= "axp20x-pek",
+		.num_resources	= ARRAY_SIZE(axp20x_pek_resources),
+		.resources	= axp20x_pek_resources,
 	}, {
-		.name			= "axp20x-regulator",
+		.name		= "axp20x-regulator",
+	}, {
+		.name		= "axp20x-usb-power-supply",
+		.of_compatible	= "x-powers,axp202-usb-power-supply",
+		.num_resources	= ARRAY_SIZE(axp20x_usb_power_supply_resources),
+		.resources	= axp20x_usb_power_supply_resources,
 	},
 };
 
-- 
2.4.3

[PATCH v2 4/4] power: Add an axp20x-usb-power driver

[Hans de Goede]

This adds a driver for the usb power_supply bits of the axp20x PMICs.

I initially started writing my own driver, before coming aware of
Bruno Prémont's excellent earlier RFC with a driver for this.

My driver was lacking CURRENT_MAX and VOLTAGE_MIN support Bruno's
drvier has, so I've copied the code for those from his driver.

Note that the AC-power-supply and battery charger bits will need separate
drivers. Each one needs its own devictree child-node so that other
devicetree nodes can reference the right power-supply, and thus each one
will get its own mfd-cell / platform_device and platform-driver.

Cc: Bruno Prémont <bonbons-ud5FBsm0p/xEiooADzr8i9i2O/JbrIOy@public.gmane.org>
Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
Changes in v2:
-Split out the dt-bindings documentation into a separate patch
-Renamed axp20x_read_16bit to axp20x_read_variable_width
-Use better local variable names inside axp20x_read_variable_width
---
 drivers/power/Kconfig            |   7 ++
 drivers/power/Makefile           |   1 +
 drivers/power/axp20x_usb_power.c | 243 +++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/axp20x.h       |  24 ++++
 4 files changed, 275 insertions(+)
 create mode 100644 drivers/power/axp20x_usb_power.c

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 4091fb0..1fee60c 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -439,6 +439,13 @@ config BATTERY_RT5033
 	  The fuelgauge calculates and determines the battery state of charge
 	  according to battery open circuit voltage.
 
+config AXP20X_POWER
+	tristate "AXP20x power supply driver"
+	depends on MFD_AXP20X
+	help
+	  This driver provides support for the power supply features of
+	  AXP20x PMIC.
+
 source "drivers/power/reset/Kconfig"
 
 endif # POWER_SUPPLY
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index b7b0181..ae0f27d 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_GENERIC_ADC_BATTERY)	+= generic-adc-battery.o
 
 obj-$(CONFIG_PDA_POWER)		+= pda_power.o
 obj-$(CONFIG_APM_POWER)		+= apm_power.o
+obj-$(CONFIG_AXP20X_POWER)	+= axp20x_usb_power.o
 obj-$(CONFIG_MAX8925_POWER)	+= max8925_power.o
 obj-$(CONFIG_WM831X_BACKUP)	+= wm831x_backup.o
 obj-$(CONFIG_WM831X_POWER)	+= wm831x_power.o
diff --git a/drivers/power/axp20x_usb_power.c b/drivers/power/axp20x_usb_power.c
new file mode 100644
index 0000000..09f388e
--- /dev/null
+++ b/drivers/power/axp20x_usb_power.c
@@ -0,0 +1,243 @@
+/*
+ * AXP20x PMIC USB power supply status driver
+ *
+ * Copyright (C) 2015 Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
+ * Copyright (C) 2014 Bruno Prémont <bonbons-ud5FBsm0p/xEiooADzr8i9i2O/JbrIOy@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under  the terms of the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/axp20x.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define DRVNAME "axp20x-usb-power-supply"
+
+#define AXP20X_PWR_STATUS_VBUS_PRESENT	BIT(5)
+#define AXP20X_PWR_STATUS_VBUS_USED	BIT(4)
+
+#define AXP20X_USB_STATUS_VBUS_VALID	BIT(2)
+
+#define AXP20X_VBUS_VHOLD_uV(b)		(4000000 + (((b) >> 3) & 7) * 100000)
+#define AXP20X_VBUS_CLIMIT_MASK		3
+#define AXP20X_VBUC_CLIMIT_900mA	0
+#define AXP20X_VBUC_CLIMIT_500mA	1
+#define AXP20X_VBUC_CLIMIT_100mA	2
+#define AXP20X_VBUC_CLIMIT_NONE		3
+
+#define AXP20X_ADC_EN1_VBUS_CURR	BIT(2)
+#define AXP20X_ADC_EN1_VBUS_VOLT	BIT(3)
+
+#define AXP20X_VBUS_MON_VBUS_VALID	BIT(3)
+
+struct axp20x_usb_power {
+	struct regmap *regmap;
+	struct power_supply *supply;
+};
+
+static irqreturn_t axp20x_usb_power_irq(int irq, void *devid)
+{
+	struct axp20x_usb_power *power = devid;
+
+	power_supply_changed(power->supply);
+
+	return IRQ_HANDLED;
+}
+
+static int axp20x_usb_power_get_property(struct power_supply *psy,
+	enum power_supply_property psp, union power_supply_propval *val)
+{
+	struct axp20x_usb_power *power = power_supply_get_drvdata(psy);
+	unsigned int input, v;
+	int r;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
+		r = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v);
+		if (r)
+			return r;
+
+		val->intval = AXP20X_VBUS_VHOLD_uV(v);
+		return 0;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		r = axp20x_read_variable_width(power->regmap,
+					       AXP20X_VBUS_V_ADC_H, 12);
+		if (r < 0)
+			return r;
+
+		val->intval = r * 1700; /* 1 step = 1.7 mV */
+		return 0;
+	case POWER_SUPPLY_PROP_CURRENT_MAX:
+		r = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v);
+		if (r)
+			return r;
+
+		switch (v & AXP20X_VBUS_CLIMIT_MASK) {
+		case AXP20X_VBUC_CLIMIT_100mA:
+			val->intval = 100000;
+			break;
+		case AXP20X_VBUC_CLIMIT_500mA:
+			val->intval = 500000;
+			break;
+		case AXP20X_VBUC_CLIMIT_900mA:
+			val->intval = 900000;
+			break;
+		case AXP20X_VBUC_CLIMIT_NONE:
+			val->intval = -1;
+			break;
+		}
+		return 0;
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		r = axp20x_read_variable_width(power->regmap,
+					       AXP20X_VBUS_I_ADC_H, 12);
+		if (r < 0)
+			return r;
+
+		val->intval = r * 375; /* 1 step = 0.375 mA */
+		return 0;
+	default:
+		break;
+	}
+
+	/* All the properties below need the input-status reg value */
+	r = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, &input);
+	if (r)
+		return r;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_HEALTH:
+		if (!(input & AXP20X_PWR_STATUS_VBUS_PRESENT)) {
+			val->intval = POWER_SUPPLY_HEALTH_UNKNOWN;
+			break;
+		}
+
+		r = regmap_read(power->regmap, AXP20X_USB_OTG_STATUS, &v);
+		if (r)
+			return r;
+
+		if (!(v & AXP20X_USB_STATUS_VBUS_VALID)) {
+			val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
+			break;
+		}
+
+		val->intval = POWER_SUPPLY_HEALTH_GOOD;
+		break;
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_PRESENT);
+		break;
+	case POWER_SUPPLY_PROP_ONLINE:
+		val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_USED);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static enum power_supply_property axp20x_usb_power_properties[] = {
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_MAX,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+};
+
+static const struct power_supply_desc axp20x_usb_power_desc = {
+	.name = "axp20x-usb",
+	.type = POWER_SUPPLY_TYPE_USB,
+	.properties = axp20x_usb_power_properties,
+	.num_properties = ARRAY_SIZE(axp20x_usb_power_properties),
+	.get_property = axp20x_usb_power_get_property,
+};
+
+static int axp20x_usb_power_probe(struct platform_device *pdev)
+{
+	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
+	struct power_supply_config psy_cfg = {};
+	struct axp20x_usb_power *power;
+	static const char * const irq_names[] = { "VBUS_PLUGIN",
+		"VBUS_REMOVAL", "VBUS_VALID", "VBUS_NOT_VALID" };
+	int i, irq, r;
+
+	if (!of_device_is_available(pdev->dev.of_node))
+		return -ENODEV;
+
+	power = devm_kzalloc(&pdev->dev, sizeof(*power), GFP_KERNEL);
+	if (!power)
+		return -ENOMEM;
+
+	power->regmap = axp20x->regmap;
+
+	/* Enable vbus valid checking */
+	r = regmap_update_bits(power->regmap, AXP20X_VBUS_MON,
+		    AXP20X_VBUS_MON_VBUS_VALID, AXP20X_VBUS_MON_VBUS_VALID);
+	if (r)
+		return r;
+
+	/* Enable vbus voltage and current measurement */
+	r = 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 (r)
+		return r;
+
+	psy_cfg.of_node = pdev->dev.of_node;
+	psy_cfg.drv_data = power;
+
+	power->supply = devm_power_supply_register(&pdev->dev,
+					&axp20x_usb_power_desc, &psy_cfg);
+	if (IS_ERR(power->supply))
+		return PTR_ERR(power->supply);
+
+	/* Request irqs after registering, as irqs may trigger immediately */
+	for (i = 0; i < ARRAY_SIZE(irq_names); i++) {
+		irq = platform_get_irq_byname(pdev, irq_names[i]);
+		if (irq < 0) {
+			dev_warn(&pdev->dev, "No IRQ for %s: %d\n",
+				 irq_names[i], irq);
+			continue;
+		}
+		irq = regmap_irq_get_virq(axp20x->regmap_irqc, irq);
+		r = devm_request_any_context_irq(&pdev->dev, irq,
+				axp20x_usb_power_irq, 0, DRVNAME, power);
+		if (r < 0)
+			dev_warn(&pdev->dev, "Error requesting %s IRQ: %d\n",
+				 irq_names[i], r);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id axp20x_usb_power_match[] = {
+	{ .compatible = "x-powers,axp202-usb-power-supply" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, axp20x_usb_power_match);
+
+static struct platform_driver axp20x_usb_power_driver = {
+	.probe = axp20x_usb_power_probe,
+	.driver = {
+		.name = DRVNAME,
+		.of_match_table = axp20x_usb_power_match,
+	},
+};
+
+module_platform_driver(axp20x_usb_power_driver);
+
+MODULE_AUTHOR("Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>");
+MODULE_DESCRIPTION("AXP20x PMIC USB power supply status driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 6d8b39a..8ec996e 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -11,6 +11,8 @@
 #ifndef __LINUX_MFD_AXP20X_H
 #define __LINUX_MFD_AXP20X_H
 
+#include <linux/regmap.h>
+
 enum {
 	AXP202_ID = 0,
 	AXP209_ID,
@@ -372,4 +374,26 @@ struct axp288_extcon_pdata {
 	struct gpio_desc *gpio_mux_cntl;
 };
 
+/* generic helper function for reading 9-16 bit wide regs */
+static inline int axp20x_read_variable_width(struct regmap *regmap,
+	unsigned int reg, unsigned int width)
+{
+	unsigned int reg_val, result;
+	int err;
+
+	err = regmap_read(regmap, reg, &reg_val);
+	if (err)
+		return err;
+
+	result = reg_val << (width - 8);
+
+	err = regmap_read(regmap, reg + 1, &reg_val);
+	if (err)
+		return err;
+
+	result |= reg_val;
+
+	return result;
+}
+
 #endif /* __LINUX_MFD_AXP20X_H */
-- 
2.4.3

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

[PATCH v2 4/4] power: Add an axp20x-usb-power driver

[Hans de Goede]

This adds a driver for the usb power_supply bits of the axp20x PMICs.

I initially started writing my own driver, before coming aware of
Bruno Pr?mont's excellent earlier RFC with a driver for this.

My driver was lacking CURRENT_MAX and VOLTAGE_MIN support Bruno's
drvier has, so I've copied the code for those from his driver.

Note that the AC-power-supply and battery charger bits will need separate
drivers. Each one needs its own devictree child-node so that other
devicetree nodes can reference the right power-supply, and thus each one
will get its own mfd-cell / platform_device and platform-driver.

Cc: Bruno Pr?mont <bonbons@linux-vserver.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-Split out the dt-bindings documentation into a separate patch
-Renamed axp20x_read_16bit to axp20x_read_variable_width
-Use better local variable names inside axp20x_read_variable_width
---
 drivers/power/Kconfig            |   7 ++
 drivers/power/Makefile           |   1 +
 drivers/power/axp20x_usb_power.c | 243 +++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/axp20x.h       |  24 ++++
 4 files changed, 275 insertions(+)
 create mode 100644 drivers/power/axp20x_usb_power.c

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 4091fb0..1fee60c 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -439,6 +439,13 @@ config BATTERY_RT5033
 	  The fuelgauge calculates and determines the battery state of charge
 	  according to battery open circuit voltage.
 
+config AXP20X_POWER
+	tristate "AXP20x power supply driver"
+	depends on MFD_AXP20X
+	help
+	  This driver provides support for the power supply features of
+	  AXP20x PMIC.
+
 source "drivers/power/reset/Kconfig"
 
 endif # POWER_SUPPLY
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index b7b0181..ae0f27d 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_GENERIC_ADC_BATTERY)	+= generic-adc-battery.o
 
 obj-$(CONFIG_PDA_POWER)		+= pda_power.o
 obj-$(CONFIG_APM_POWER)		+= apm_power.o
+obj-$(CONFIG_AXP20X_POWER)	+= axp20x_usb_power.o
 obj-$(CONFIG_MAX8925_POWER)	+= max8925_power.o
 obj-$(CONFIG_WM831X_BACKUP)	+= wm831x_backup.o
 obj-$(CONFIG_WM831X_POWER)	+= wm831x_power.o
diff --git a/drivers/power/axp20x_usb_power.c b/drivers/power/axp20x_usb_power.c
new file mode 100644
index 0000000..09f388e
--- /dev/null
+++ b/drivers/power/axp20x_usb_power.c
@@ -0,0 +1,243 @@
+/*
+ * AXP20x PMIC USB power supply status driver
+ *
+ * Copyright (C) 2015 Hans de Goede <hdegoede@redhat.com>
+ * Copyright (C) 2014 Bruno Pr?mont <bonbons@linux-vserver.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under  the terms of the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/axp20x.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#define DRVNAME "axp20x-usb-power-supply"
+
+#define AXP20X_PWR_STATUS_VBUS_PRESENT	BIT(5)
+#define AXP20X_PWR_STATUS_VBUS_USED	BIT(4)
+
+#define AXP20X_USB_STATUS_VBUS_VALID	BIT(2)
+
+#define AXP20X_VBUS_VHOLD_uV(b)		(4000000 + (((b) >> 3) & 7) * 100000)
+#define AXP20X_VBUS_CLIMIT_MASK		3
+#define AXP20X_VBUC_CLIMIT_900mA	0
+#define AXP20X_VBUC_CLIMIT_500mA	1
+#define AXP20X_VBUC_CLIMIT_100mA	2
+#define AXP20X_VBUC_CLIMIT_NONE		3
+
+#define AXP20X_ADC_EN1_VBUS_CURR	BIT(2)
+#define AXP20X_ADC_EN1_VBUS_VOLT	BIT(3)
+
+#define AXP20X_VBUS_MON_VBUS_VALID	BIT(3)
+
+struct axp20x_usb_power {
+	struct regmap *regmap;
+	struct power_supply *supply;
+};
+
+static irqreturn_t axp20x_usb_power_irq(int irq, void *devid)
+{
+	struct axp20x_usb_power *power = devid;
+
+	power_supply_changed(power->supply);
+
+	return IRQ_HANDLED;
+}
+
+static int axp20x_usb_power_get_property(struct power_supply *psy,
+	enum power_supply_property psp, union power_supply_propval *val)
+{
+	struct axp20x_usb_power *power = power_supply_get_drvdata(psy);
+	unsigned int input, v;
+	int r;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
+		r = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v);
+		if (r)
+			return r;
+
+		val->intval = AXP20X_VBUS_VHOLD_uV(v);
+		return 0;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		r = axp20x_read_variable_width(power->regmap,
+					       AXP20X_VBUS_V_ADC_H, 12);
+		if (r < 0)
+			return r;
+
+		val->intval = r * 1700; /* 1 step = 1.7 mV */
+		return 0;
+	case POWER_SUPPLY_PROP_CURRENT_MAX:
+		r = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v);
+		if (r)
+			return r;
+
+		switch (v & AXP20X_VBUS_CLIMIT_MASK) {
+		case AXP20X_VBUC_CLIMIT_100mA:
+			val->intval = 100000;
+			break;
+		case AXP20X_VBUC_CLIMIT_500mA:
+			val->intval = 500000;
+			break;
+		case AXP20X_VBUC_CLIMIT_900mA:
+			val->intval = 900000;
+			break;
+		case AXP20X_VBUC_CLIMIT_NONE:
+			val->intval = -1;
+			break;
+		}
+		return 0;
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		r = axp20x_read_variable_width(power->regmap,
+					       AXP20X_VBUS_I_ADC_H, 12);
+		if (r < 0)
+			return r;
+
+		val->intval = r * 375; /* 1 step = 0.375 mA */
+		return 0;
+	default:
+		break;
+	}
+
+	/* All the properties below need the input-status reg value */
+	r = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, &input);
+	if (r)
+		return r;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_HEALTH:
+		if (!(input & AXP20X_PWR_STATUS_VBUS_PRESENT)) {
+			val->intval = POWER_SUPPLY_HEALTH_UNKNOWN;
+			break;
+		}
+
+		r = regmap_read(power->regmap, AXP20X_USB_OTG_STATUS, &v);
+		if (r)
+			return r;
+
+		if (!(v & AXP20X_USB_STATUS_VBUS_VALID)) {
+			val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
+			break;
+		}
+
+		val->intval = POWER_SUPPLY_HEALTH_GOOD;
+		break;
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_PRESENT);
+		break;
+	case POWER_SUPPLY_PROP_ONLINE:
+		val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_USED);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static enum power_supply_property axp20x_usb_power_properties[] = {
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_MAX,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+};
+
+static const struct power_supply_desc axp20x_usb_power_desc = {
+	.name = "axp20x-usb",
+	.type = POWER_SUPPLY_TYPE_USB,
+	.properties = axp20x_usb_power_properties,
+	.num_properties = ARRAY_SIZE(axp20x_usb_power_properties),
+	.get_property = axp20x_usb_power_get_property,
+};
+
+static int axp20x_usb_power_probe(struct platform_device *pdev)
+{
+	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
+	struct power_supply_config psy_cfg = {};
+	struct axp20x_usb_power *power;
+	static const char * const irq_names[] = { "VBUS_PLUGIN",
+		"VBUS_REMOVAL", "VBUS_VALID", "VBUS_NOT_VALID" };
+	int i, irq, r;
+
+	if (!of_device_is_available(pdev->dev.of_node))
+		return -ENODEV;
+
+	power = devm_kzalloc(&pdev->dev, sizeof(*power), GFP_KERNEL);
+	if (!power)
+		return -ENOMEM;
+
+	power->regmap = axp20x->regmap;
+
+	/* Enable vbus valid checking */
+	r = regmap_update_bits(power->regmap, AXP20X_VBUS_MON,
+		    AXP20X_VBUS_MON_VBUS_VALID, AXP20X_VBUS_MON_VBUS_VALID);
+	if (r)
+		return r;
+
+	/* Enable vbus voltage and current measurement */
+	r = 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 (r)
+		return r;
+
+	psy_cfg.of_node = pdev->dev.of_node;
+	psy_cfg.drv_data = power;
+
+	power->supply = devm_power_supply_register(&pdev->dev,
+					&axp20x_usb_power_desc, &psy_cfg);
+	if (IS_ERR(power->supply))
+		return PTR_ERR(power->supply);
+
+	/* Request irqs after registering, as irqs may trigger immediately */
+	for (i = 0; i < ARRAY_SIZE(irq_names); i++) {
+		irq = platform_get_irq_byname(pdev, irq_names[i]);
+		if (irq < 0) {
+			dev_warn(&pdev->dev, "No IRQ for %s: %d\n",
+				 irq_names[i], irq);
+			continue;
+		}
+		irq = regmap_irq_get_virq(axp20x->regmap_irqc, irq);
+		r = devm_request_any_context_irq(&pdev->dev, irq,
+				axp20x_usb_power_irq, 0, DRVNAME, power);
+		if (r < 0)
+			dev_warn(&pdev->dev, "Error requesting %s IRQ: %d\n",
+				 irq_names[i], r);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id axp20x_usb_power_match[] = {
+	{ .compatible = "x-powers,axp202-usb-power-supply" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, axp20x_usb_power_match);
+
+static struct platform_driver axp20x_usb_power_driver = {
+	.probe = axp20x_usb_power_probe,
+	.driver = {
+		.name = DRVNAME,
+		.of_match_table = axp20x_usb_power_match,
+	},
+};
+
+module_platform_driver(axp20x_usb_power_driver);
+
+MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
+MODULE_DESCRIPTION("AXP20x PMIC USB power supply status driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 6d8b39a..8ec996e 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -11,6 +11,8 @@
 #ifndef __LINUX_MFD_AXP20X_H
 #define __LINUX_MFD_AXP20X_H
 
+#include <linux/regmap.h>
+
 enum {
 	AXP202_ID = 0,
 	AXP209_ID,
@@ -372,4 +374,26 @@ struct axp288_extcon_pdata {
 	struct gpio_desc *gpio_mux_cntl;
 };
 
+/* generic helper function for reading 9-16 bit wide regs */
+static inline int axp20x_read_variable_width(struct regmap *regmap,
+	unsigned int reg, unsigned int width)
+{
+	unsigned int reg_val, result;
+	int err;
+
+	err = regmap_read(regmap, reg, &reg_val);
+	if (err)
+		return err;
+
+	result = reg_val << (width - 8);
+
+	err = regmap_read(regmap, reg + 1, &reg_val);
+	if (err)
+		return err;
+
+	result |= reg_val;
+
+	return result;
+}
+
 #endif /* __LINUX_MFD_AXP20X_H */
-- 
2.4.3

Re: [PATCH v2 2/4] mfd: axp20x: Add missing registers, and mark more registers volatile

[Lee Jones]

On Sat, 13 Jun 2015, Hans de Goede wrote:

> From: Bruno Prémont <bonbons-ud5FBsm0p/xEiooADzr8i9i2O/JbrIOy@public.gmane.org>
> 
> Add an extra set of registers which is necessary tu support the PMICs
> battery charger function, and mark registers which contain status bits,
> gpio status, and adc readings as volatile.
> 
> Cc: Bruno Prémont <bonbons-ud5FBsm0p/xEiooADzr8i9i2O/JbrIOy@public.gmane.org>
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> Changes in v2:
> -Add a AXP20X_OCV_MAX define
> ---
>  drivers/mfd/axp20x.c       | 8 +++++++-
>  include/linux/mfd/axp20x.h | 6 ++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)

Patch looks good, but you're missing Bruno's SoB.

> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 6df9155..f9a3c2d 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -39,10 +39,16 @@ static const char * const axp20x_model_names[] = {
>  static const struct regmap_range axp20x_writeable_ranges[] = {
>  	regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE),
>  	regmap_reg_range(AXP20X_DCDC_MODE, AXP20X_FG_RES),
> +	regmap_reg_range(AXP20X_RDC_H, AXP20X_OCV(15)),
>  };
>  
>  static const struct regmap_range axp20x_volatile_ranges[] = {
> +	regmap_reg_range(AXP20X_PWR_INPUT_STATUS, AXP20X_USB_OTG_STATUS),
> +	regmap_reg_range(AXP20X_CHRG_CTRL1, AXP20X_CHRG_CTRL2),
>  	regmap_reg_range(AXP20X_IRQ1_EN, AXP20X_IRQ5_STATE),
> +	regmap_reg_range(AXP20X_ACIN_V_ADC_H, AXP20X_IPSOUT_V_HIGH_L),
> +	regmap_reg_range(AXP20X_GPIO20_SS, AXP20X_GPIO3_CTRL),
> +	regmap_reg_range(AXP20X_FG_RES, AXP20X_RDC_L),
>  };
>  
>  static const struct regmap_access_table axp20x_writeable_table = {
> @@ -159,7 +165,7 @@ static const struct regmap_config axp20x_regmap_config = {
>  	.val_bits	= 8,
>  	.wr_table	= &axp20x_writeable_table,
>  	.volatile_table	= &axp20x_volatile_table,
> -	.max_register	= AXP20X_FG_RES,
> +	.max_register	= AXP20X_OCV(AXP20X_OCV_MAX),
>  	.cache_type	= REGCACHE_RBTREE,
>  };
>  
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index 5275423..6d8b39a 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -151,6 +151,12 @@ enum {
>  #define AXP20X_CC_CTRL			0xb8
>  #define AXP20X_FG_RES			0xb9
>  
> +/* OCV */
> +#define AXP20X_RDC_H			0xba
> +#define AXP20X_RDC_L			0xbb
> +#define AXP20X_OCV(m)			(0xc0 + (m))
> +#define AXP20X_OCV_MAX			0xf
> +
>  /* AXP22X specific registers */
>  #define AXP22X_BATLOW_THRES1		0xe6
>  

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

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

[PATCH v2 2/4] mfd: axp20x: Add missing registers, and mark more registers volatile

[Lee Jones]

On Sat, 13 Jun 2015, Hans de Goede wrote:

> From: Bruno Pr?mont <bonbons@linux-vserver.org>
> 
> Add an extra set of registers which is necessary tu support the PMICs
> battery charger function, and mark registers which contain status bits,
> gpio status, and adc readings as volatile.
> 
> Cc: Bruno Pr?mont <bonbons@linux-vserver.org>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Add a AXP20X_OCV_MAX define
> ---
>  drivers/mfd/axp20x.c       | 8 +++++++-
>  include/linux/mfd/axp20x.h | 6 ++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)

Patch looks good, but you're missing Bruno's SoB.

> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 6df9155..f9a3c2d 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -39,10 +39,16 @@ static const char * const axp20x_model_names[] = {
>  static const struct regmap_range axp20x_writeable_ranges[] = {
>  	regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE),
>  	regmap_reg_range(AXP20X_DCDC_MODE, AXP20X_FG_RES),
> +	regmap_reg_range(AXP20X_RDC_H, AXP20X_OCV(15)),
>  };
>  
>  static const struct regmap_range axp20x_volatile_ranges[] = {
> +	regmap_reg_range(AXP20X_PWR_INPUT_STATUS, AXP20X_USB_OTG_STATUS),
> +	regmap_reg_range(AXP20X_CHRG_CTRL1, AXP20X_CHRG_CTRL2),
>  	regmap_reg_range(AXP20X_IRQ1_EN, AXP20X_IRQ5_STATE),
> +	regmap_reg_range(AXP20X_ACIN_V_ADC_H, AXP20X_IPSOUT_V_HIGH_L),
> +	regmap_reg_range(AXP20X_GPIO20_SS, AXP20X_GPIO3_CTRL),
> +	regmap_reg_range(AXP20X_FG_RES, AXP20X_RDC_L),
>  };
>  
>  static const struct regmap_access_table axp20x_writeable_table = {
> @@ -159,7 +165,7 @@ static const struct regmap_config axp20x_regmap_config = {
>  	.val_bits	= 8,
>  	.wr_table	= &axp20x_writeable_table,
>  	.volatile_table	= &axp20x_volatile_table,
> -	.max_register	= AXP20X_FG_RES,
> +	.max_register	= AXP20X_OCV(AXP20X_OCV_MAX),
>  	.cache_type	= REGCACHE_RBTREE,
>  };
>  
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index 5275423..6d8b39a 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -151,6 +151,12 @@ enum {
>  #define AXP20X_CC_CTRL			0xb8
>  #define AXP20X_FG_RES			0xb9
>  
> +/* OCV */
> +#define AXP20X_RDC_H			0xba
> +#define AXP20X_RDC_L			0xbb
> +#define AXP20X_OCV(m)			(0xc0 + (m))
> +#define AXP20X_OCV_MAX			0xf
> +
>  /* AXP22X specific registers */
>  #define AXP22X_BATLOW_THRES1		0xe6
>  

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

Re: [PATCH v2 3/4] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Lee Jones]

> Add a cell for the usb power_supply part of the axp20x PMICs.

Why are you duplicating the subject line?

> Note that this cell is only for the usb power_supply part and not the
> ac-power / battery-charger / rtc-backup-bat-charger bits.
> 
> Depending on the board each of those must be enabled / disabled separately
> in devicetree as most boards do not use all 4. So in dt each one needs its
> own child-node of the axp20x node. Another reason for using separate child
> nodes for each is so that other devicetree nodes can have a power-supply
> property with a phandle referencing a node representing a single
> power-supply.
> 
> The decision to use a separate devicetree node for each is reflected on
> the kernel side by each getting its own mfd-cell / platform_device and
> platform-driver.

You don't really need to say any of this, as this is the 'norm'.

What you didn't mention however, is that you're taking the opportunity
to fix some formatting issues and that there are no functional changes
in these lines.

> Cc: Bruno Prémont <bonbons-ud5FBsm0p/xEiooADzr8i9i2O/JbrIOy@public.gmane.org>
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> Changes in v2:
> -Use DEFINE_RES_IRQ_NAMED
> -Change indentation of axp20x_cells initializers to avoid line wrapping
> ---
>  drivers/mfd/axp20x.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)

Patch looks okay however:

Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index f9a3c2d..ca4a604 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -113,6 +113,13 @@ static struct resource axp20x_pek_resources[] = {
>  	},
>  };
>  
> +static struct resource axp20x_usb_power_supply_resources[] = {
> +	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_PLUGIN, "VBUS_PLUGIN"),
> +	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_REMOVAL, "VBUS_REMOVAL"),
> +	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_VALID, "VBUS_VALID"),
> +	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_NOT_VALID, "VBUS_NOT_VALID"),
> +};
> +
>  static struct resource axp22x_pek_resources[] = {
>  	{
>  		.name   = "PEK_DBR",
> @@ -363,11 +370,16 @@ static const struct regmap_irq_chip axp288_regmap_irq_chip = {
>  
>  static struct mfd_cell axp20x_cells[] = {
>  	{
> -		.name			= "axp20x-pek",
> -		.num_resources		= ARRAY_SIZE(axp20x_pek_resources),
> -		.resources		= axp20x_pek_resources,
> +		.name		= "axp20x-pek",
> +		.num_resources	= ARRAY_SIZE(axp20x_pek_resources),
> +		.resources	= axp20x_pek_resources,
>  	}, {
> -		.name			= "axp20x-regulator",
> +		.name		= "axp20x-regulator",
> +	}, {
> +		.name		= "axp20x-usb-power-supply",
> +		.of_compatible	= "x-powers,axp202-usb-power-supply",
> +		.num_resources	= ARRAY_SIZE(axp20x_usb_power_supply_resources),
> +		.resources	= axp20x_usb_power_supply_resources,
>  	},
>  };
>  

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

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

[PATCH v2 3/4] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Lee Jones]

> Add a cell for the usb power_supply part of the axp20x PMICs.

Why are you duplicating the subject line?

> Note that this cell is only for the usb power_supply part and not the
> ac-power / battery-charger / rtc-backup-bat-charger bits.
> 
> Depending on the board each of those must be enabled / disabled separately
> in devicetree as most boards do not use all 4. So in dt each one needs its
> own child-node of the axp20x node. Another reason for using separate child
> nodes for each is so that other devicetree nodes can have a power-supply
> property with a phandle referencing a node representing a single
> power-supply.
> 
> The decision to use a separate devicetree node for each is reflected on
> the kernel side by each getting its own mfd-cell / platform_device and
> platform-driver.

You don't really need to say any of this, as this is the 'norm'.

What you didn't mention however, is that you're taking the opportunity
to fix some formatting issues and that there are no functional changes
in these lines.

> Cc: Bruno Pr?mont <bonbons@linux-vserver.org>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Use DEFINE_RES_IRQ_NAMED
> -Change indentation of axp20x_cells initializers to avoid line wrapping
> ---
>  drivers/mfd/axp20x.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)

Patch looks okay however:

Acked-by: Lee Jones <lee.jones@linaro.org>

> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index f9a3c2d..ca4a604 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -113,6 +113,13 @@ static struct resource axp20x_pek_resources[] = {
>  	},
>  };
>  
> +static struct resource axp20x_usb_power_supply_resources[] = {
> +	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_PLUGIN, "VBUS_PLUGIN"),
> +	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_REMOVAL, "VBUS_REMOVAL"),
> +	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_VALID, "VBUS_VALID"),
> +	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_NOT_VALID, "VBUS_NOT_VALID"),
> +};
> +
>  static struct resource axp22x_pek_resources[] = {
>  	{
>  		.name   = "PEK_DBR",
> @@ -363,11 +370,16 @@ static const struct regmap_irq_chip axp288_regmap_irq_chip = {
>  
>  static struct mfd_cell axp20x_cells[] = {
>  	{
> -		.name			= "axp20x-pek",
> -		.num_resources		= ARRAY_SIZE(axp20x_pek_resources),
> -		.resources		= axp20x_pek_resources,
> +		.name		= "axp20x-pek",
> +		.num_resources	= ARRAY_SIZE(axp20x_pek_resources),
> +		.resources	= axp20x_pek_resources,
>  	}, {
> -		.name			= "axp20x-regulator",
> +		.name		= "axp20x-regulator",
> +	}, {
> +		.name		= "axp20x-usb-power-supply",
> +		.of_compatible	= "x-powers,axp202-usb-power-supply",
> +		.num_resources	= ARRAY_SIZE(axp20x_usb_power_supply_resources),
> +		.resources	= axp20x_usb_power_supply_resources,
>  	},
>  };
>  

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

Re: [PATCH v2 4/4] power: Add an axp20x-usb-power driver

[Lee Jones]

On Sat, 13 Jun 2015, Hans de Goede wrote:

> This adds a driver for the usb power_supply bits of the axp20x PMICs.
> 
> I initially started writing my own driver, before coming aware of
> Bruno Prémont's excellent earlier RFC with a driver for this.
> 
> My driver was lacking CURRENT_MAX and VOLTAGE_MIN support Bruno's
> drvier has, so I've copied the code for those from his driver.
> 
> Note that the AC-power-supply and battery charger bits will need separate
> drivers. Each one needs its own devictree child-node so that other
> devicetree nodes can reference the right power-supply, and thus each one
> will get its own mfd-cell / platform_device and platform-driver.
> 
> Cc: Bruno Prémont <bonbons-ud5FBsm0p/xEiooADzr8i9i2O/JbrIOy@public.gmane.org>
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> Changes in v2:
> -Split out the dt-bindings documentation into a separate patch
> -Renamed axp20x_read_16bit to axp20x_read_variable_width
> -Use better local variable names inside axp20x_read_variable_width
> ---
>  drivers/power/Kconfig            |   7 ++
>  drivers/power/Makefile           |   1 +
>  drivers/power/axp20x_usb_power.c | 243 +++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/axp20x.h       |  24 ++++

For the MFD part:

Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

>  4 files changed, 275 insertions(+)
>  create mode 100644 drivers/power/axp20x_usb_power.c
> 
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 4091fb0..1fee60c 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -439,6 +439,13 @@ config BATTERY_RT5033
>  	  The fuelgauge calculates and determines the battery state of charge
>  	  according to battery open circuit voltage.
>  
> +config AXP20X_POWER
> +	tristate "AXP20x power supply driver"
> +	depends on MFD_AXP20X
> +	help
> +	  This driver provides support for the power supply features of
> +	  AXP20x PMIC.
> +
>  source "drivers/power/reset/Kconfig"
>  
>  endif # POWER_SUPPLY
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index b7b0181..ae0f27d 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_GENERIC_ADC_BATTERY)	+= generic-adc-battery.o
>  
>  obj-$(CONFIG_PDA_POWER)		+= pda_power.o
>  obj-$(CONFIG_APM_POWER)		+= apm_power.o
> +obj-$(CONFIG_AXP20X_POWER)	+= axp20x_usb_power.o
>  obj-$(CONFIG_MAX8925_POWER)	+= max8925_power.o
>  obj-$(CONFIG_WM831X_BACKUP)	+= wm831x_backup.o
>  obj-$(CONFIG_WM831X_POWER)	+= wm831x_power.o
> diff --git a/drivers/power/axp20x_usb_power.c b/drivers/power/axp20x_usb_power.c
> new file mode 100644
> index 0000000..09f388e
> --- /dev/null
> +++ b/drivers/power/axp20x_usb_power.c
> @@ -0,0 +1,243 @@
> +/*
> + * AXP20x PMIC USB power supply status driver
> + *
> + * Copyright (C) 2015 Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> + * Copyright (C) 2014 Bruno Prémont <bonbons-ud5FBsm0p/xEiooADzr8i9i2O/JbrIOy@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under  the terms of the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/axp20x.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#define DRVNAME "axp20x-usb-power-supply"
> +
> +#define AXP20X_PWR_STATUS_VBUS_PRESENT	BIT(5)
> +#define AXP20X_PWR_STATUS_VBUS_USED	BIT(4)
> +
> +#define AXP20X_USB_STATUS_VBUS_VALID	BIT(2)
> +
> +#define AXP20X_VBUS_VHOLD_uV(b)		(4000000 + (((b) >> 3) & 7) * 100000)
> +#define AXP20X_VBUS_CLIMIT_MASK		3
> +#define AXP20X_VBUC_CLIMIT_900mA	0
> +#define AXP20X_VBUC_CLIMIT_500mA	1
> +#define AXP20X_VBUC_CLIMIT_100mA	2
> +#define AXP20X_VBUC_CLIMIT_NONE		3
> +
> +#define AXP20X_ADC_EN1_VBUS_CURR	BIT(2)
> +#define AXP20X_ADC_EN1_VBUS_VOLT	BIT(3)
> +
> +#define AXP20X_VBUS_MON_VBUS_VALID	BIT(3)
> +
> +struct axp20x_usb_power {
> +	struct regmap *regmap;
> +	struct power_supply *supply;
> +};
> +
> +static irqreturn_t axp20x_usb_power_irq(int irq, void *devid)
> +{
> +	struct axp20x_usb_power *power = devid;
> +
> +	power_supply_changed(power->supply);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int axp20x_usb_power_get_property(struct power_supply *psy,
> +	enum power_supply_property psp, union power_supply_propval *val)
> +{
> +	struct axp20x_usb_power *power = power_supply_get_drvdata(psy);
> +	unsigned int input, v;
> +	int r;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
> +		r = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v);
> +		if (r)
> +			return r;
> +
> +		val->intval = AXP20X_VBUS_VHOLD_uV(v);
> +		return 0;
> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		r = axp20x_read_variable_width(power->regmap,
> +					       AXP20X_VBUS_V_ADC_H, 12);
> +		if (r < 0)
> +			return r;
> +
> +		val->intval = r * 1700; /* 1 step = 1.7 mV */
> +		return 0;
> +	case POWER_SUPPLY_PROP_CURRENT_MAX:
> +		r = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v);
> +		if (r)
> +			return r;
> +
> +		switch (v & AXP20X_VBUS_CLIMIT_MASK) {
> +		case AXP20X_VBUC_CLIMIT_100mA:
> +			val->intval = 100000;
> +			break;
> +		case AXP20X_VBUC_CLIMIT_500mA:
> +			val->intval = 500000;
> +			break;
> +		case AXP20X_VBUC_CLIMIT_900mA:
> +			val->intval = 900000;
> +			break;
> +		case AXP20X_VBUC_CLIMIT_NONE:
> +			val->intval = -1;
> +			break;
> +		}
> +		return 0;
> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		r = axp20x_read_variable_width(power->regmap,
> +					       AXP20X_VBUS_I_ADC_H, 12);
> +		if (r < 0)
> +			return r;
> +
> +		val->intval = r * 375; /* 1 step = 0.375 mA */
> +		return 0;
> +	default:
> +		break;
> +	}
> +
> +	/* All the properties below need the input-status reg value */
> +	r = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, &input);
> +	if (r)
> +		return r;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_HEALTH:
> +		if (!(input & AXP20X_PWR_STATUS_VBUS_PRESENT)) {
> +			val->intval = POWER_SUPPLY_HEALTH_UNKNOWN;
> +			break;
> +		}
> +
> +		r = regmap_read(power->regmap, AXP20X_USB_OTG_STATUS, &v);
> +		if (r)
> +			return r;
> +
> +		if (!(v & AXP20X_USB_STATUS_VBUS_VALID)) {
> +			val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
> +			break;
> +		}
> +
> +		val->intval = POWER_SUPPLY_HEALTH_GOOD;
> +		break;
> +	case POWER_SUPPLY_PROP_PRESENT:
> +		val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_PRESENT);
> +		break;
> +	case POWER_SUPPLY_PROP_ONLINE:
> +		val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_USED);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static enum power_supply_property axp20x_usb_power_properties[] = {
> +	POWER_SUPPLY_PROP_HEALTH,
> +	POWER_SUPPLY_PROP_PRESENT,
> +	POWER_SUPPLY_PROP_ONLINE,
> +	POWER_SUPPLY_PROP_VOLTAGE_MIN,
> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +	POWER_SUPPLY_PROP_CURRENT_MAX,
> +	POWER_SUPPLY_PROP_CURRENT_NOW,
> +};
> +
> +static const struct power_supply_desc axp20x_usb_power_desc = {
> +	.name = "axp20x-usb",
> +	.type = POWER_SUPPLY_TYPE_USB,
> +	.properties = axp20x_usb_power_properties,
> +	.num_properties = ARRAY_SIZE(axp20x_usb_power_properties),
> +	.get_property = axp20x_usb_power_get_property,
> +};
> +
> +static int axp20x_usb_power_probe(struct platform_device *pdev)
> +{
> +	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
> +	struct power_supply_config psy_cfg = {};
> +	struct axp20x_usb_power *power;
> +	static const char * const irq_names[] = { "VBUS_PLUGIN",
> +		"VBUS_REMOVAL", "VBUS_VALID", "VBUS_NOT_VALID" };
> +	int i, irq, r;
> +
> +	if (!of_device_is_available(pdev->dev.of_node))
> +		return -ENODEV;
> +
> +	power = devm_kzalloc(&pdev->dev, sizeof(*power), GFP_KERNEL);
> +	if (!power)
> +		return -ENOMEM;
> +
> +	power->regmap = axp20x->regmap;
> +
> +	/* Enable vbus valid checking */
> +	r = regmap_update_bits(power->regmap, AXP20X_VBUS_MON,
> +		    AXP20X_VBUS_MON_VBUS_VALID, AXP20X_VBUS_MON_VBUS_VALID);
> +	if (r)
> +		return r;
> +
> +	/* Enable vbus voltage and current measurement */
> +	r = 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 (r)
> +		return r;
> +
> +	psy_cfg.of_node = pdev->dev.of_node;
> +	psy_cfg.drv_data = power;
> +
> +	power->supply = devm_power_supply_register(&pdev->dev,
> +					&axp20x_usb_power_desc, &psy_cfg);
> +	if (IS_ERR(power->supply))
> +		return PTR_ERR(power->supply);
> +
> +	/* Request irqs after registering, as irqs may trigger immediately */
> +	for (i = 0; i < ARRAY_SIZE(irq_names); i++) {
> +		irq = platform_get_irq_byname(pdev, irq_names[i]);
> +		if (irq < 0) {
> +			dev_warn(&pdev->dev, "No IRQ for %s: %d\n",
> +				 irq_names[i], irq);
> +			continue;
> +		}
> +		irq = regmap_irq_get_virq(axp20x->regmap_irqc, irq);
> +		r = devm_request_any_context_irq(&pdev->dev, irq,
> +				axp20x_usb_power_irq, 0, DRVNAME, power);
> +		if (r < 0)
> +			dev_warn(&pdev->dev, "Error requesting %s IRQ: %d\n",
> +				 irq_names[i], r);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id axp20x_usb_power_match[] = {
> +	{ .compatible = "x-powers,axp202-usb-power-supply" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, axp20x_usb_power_match);
> +
> +static struct platform_driver axp20x_usb_power_driver = {
> +	.probe = axp20x_usb_power_probe,
> +	.driver = {
> +		.name = DRVNAME,
> +		.of_match_table = axp20x_usb_power_match,
> +	},
> +};
> +
> +module_platform_driver(axp20x_usb_power_driver);
> +
> +MODULE_AUTHOR("Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>");
> +MODULE_DESCRIPTION("AXP20x PMIC USB power supply status driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index 6d8b39a..8ec996e 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -11,6 +11,8 @@
>  #ifndef __LINUX_MFD_AXP20X_H
>  #define __LINUX_MFD_AXP20X_H
>  
> +#include <linux/regmap.h>
> +
>  enum {
>  	AXP202_ID = 0,
>  	AXP209_ID,
> @@ -372,4 +374,26 @@ struct axp288_extcon_pdata {
>  	struct gpio_desc *gpio_mux_cntl;
>  };
>  
> +/* generic helper function for reading 9-16 bit wide regs */
> +static inline int axp20x_read_variable_width(struct regmap *regmap,
> +	unsigned int reg, unsigned int width)
> +{
> +	unsigned int reg_val, result;
> +	int err;
> +
> +	err = regmap_read(regmap, reg, &reg_val);
> +	if (err)
> +		return err;
> +
> +	result = reg_val << (width - 8);
> +
> +	err = regmap_read(regmap, reg + 1, &reg_val);
> +	if (err)
> +		return err;
> +
> +	result |= reg_val;
> +
> +	return result;
> +}
> +
>  #endif /* __LINUX_MFD_AXP20X_H */

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

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

[PATCH v2 4/4] power: Add an axp20x-usb-power driver

[Lee Jones]

On Sat, 13 Jun 2015, Hans de Goede wrote:

> This adds a driver for the usb power_supply bits of the axp20x PMICs.
> 
> I initially started writing my own driver, before coming aware of
> Bruno Pr?mont's excellent earlier RFC with a driver for this.
> 
> My driver was lacking CURRENT_MAX and VOLTAGE_MIN support Bruno's
> drvier has, so I've copied the code for those from his driver.
> 
> Note that the AC-power-supply and battery charger bits will need separate
> drivers. Each one needs its own devictree child-node so that other
> devicetree nodes can reference the right power-supply, and thus each one
> will get its own mfd-cell / platform_device and platform-driver.
> 
> Cc: Bruno Pr?mont <bonbons@linux-vserver.org>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -Split out the dt-bindings documentation into a separate patch
> -Renamed axp20x_read_16bit to axp20x_read_variable_width
> -Use better local variable names inside axp20x_read_variable_width
> ---
>  drivers/power/Kconfig            |   7 ++
>  drivers/power/Makefile           |   1 +
>  drivers/power/axp20x_usb_power.c | 243 +++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/axp20x.h       |  24 ++++

For the MFD part:

Acked-by: Lee Jones <lee.jones@linaro.org>

>  4 files changed, 275 insertions(+)
>  create mode 100644 drivers/power/axp20x_usb_power.c
> 
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 4091fb0..1fee60c 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -439,6 +439,13 @@ config BATTERY_RT5033
>  	  The fuelgauge calculates and determines the battery state of charge
>  	  according to battery open circuit voltage.
>  
> +config AXP20X_POWER
> +	tristate "AXP20x power supply driver"
> +	depends on MFD_AXP20X
> +	help
> +	  This driver provides support for the power supply features of
> +	  AXP20x PMIC.
> +
>  source "drivers/power/reset/Kconfig"
>  
>  endif # POWER_SUPPLY
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index b7b0181..ae0f27d 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_GENERIC_ADC_BATTERY)	+= generic-adc-battery.o
>  
>  obj-$(CONFIG_PDA_POWER)		+= pda_power.o
>  obj-$(CONFIG_APM_POWER)		+= apm_power.o
> +obj-$(CONFIG_AXP20X_POWER)	+= axp20x_usb_power.o
>  obj-$(CONFIG_MAX8925_POWER)	+= max8925_power.o
>  obj-$(CONFIG_WM831X_BACKUP)	+= wm831x_backup.o
>  obj-$(CONFIG_WM831X_POWER)	+= wm831x_power.o
> diff --git a/drivers/power/axp20x_usb_power.c b/drivers/power/axp20x_usb_power.c
> new file mode 100644
> index 0000000..09f388e
> --- /dev/null
> +++ b/drivers/power/axp20x_usb_power.c
> @@ -0,0 +1,243 @@
> +/*
> + * AXP20x PMIC USB power supply status driver
> + *
> + * Copyright (C) 2015 Hans de Goede <hdegoede@redhat.com>
> + * Copyright (C) 2014 Bruno Pr?mont <bonbons@linux-vserver.org>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under  the terms of the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/axp20x.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#define DRVNAME "axp20x-usb-power-supply"
> +
> +#define AXP20X_PWR_STATUS_VBUS_PRESENT	BIT(5)
> +#define AXP20X_PWR_STATUS_VBUS_USED	BIT(4)
> +
> +#define AXP20X_USB_STATUS_VBUS_VALID	BIT(2)
> +
> +#define AXP20X_VBUS_VHOLD_uV(b)		(4000000 + (((b) >> 3) & 7) * 100000)
> +#define AXP20X_VBUS_CLIMIT_MASK		3
> +#define AXP20X_VBUC_CLIMIT_900mA	0
> +#define AXP20X_VBUC_CLIMIT_500mA	1
> +#define AXP20X_VBUC_CLIMIT_100mA	2
> +#define AXP20X_VBUC_CLIMIT_NONE		3
> +
> +#define AXP20X_ADC_EN1_VBUS_CURR	BIT(2)
> +#define AXP20X_ADC_EN1_VBUS_VOLT	BIT(3)
> +
> +#define AXP20X_VBUS_MON_VBUS_VALID	BIT(3)
> +
> +struct axp20x_usb_power {
> +	struct regmap *regmap;
> +	struct power_supply *supply;
> +};
> +
> +static irqreturn_t axp20x_usb_power_irq(int irq, void *devid)
> +{
> +	struct axp20x_usb_power *power = devid;
> +
> +	power_supply_changed(power->supply);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int axp20x_usb_power_get_property(struct power_supply *psy,
> +	enum power_supply_property psp, union power_supply_propval *val)
> +{
> +	struct axp20x_usb_power *power = power_supply_get_drvdata(psy);
> +	unsigned int input, v;
> +	int r;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
> +		r = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v);
> +		if (r)
> +			return r;
> +
> +		val->intval = AXP20X_VBUS_VHOLD_uV(v);
> +		return 0;
> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		r = axp20x_read_variable_width(power->regmap,
> +					       AXP20X_VBUS_V_ADC_H, 12);
> +		if (r < 0)
> +			return r;
> +
> +		val->intval = r * 1700; /* 1 step = 1.7 mV */
> +		return 0;
> +	case POWER_SUPPLY_PROP_CURRENT_MAX:
> +		r = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v);
> +		if (r)
> +			return r;
> +
> +		switch (v & AXP20X_VBUS_CLIMIT_MASK) {
> +		case AXP20X_VBUC_CLIMIT_100mA:
> +			val->intval = 100000;
> +			break;
> +		case AXP20X_VBUC_CLIMIT_500mA:
> +			val->intval = 500000;
> +			break;
> +		case AXP20X_VBUC_CLIMIT_900mA:
> +			val->intval = 900000;
> +			break;
> +		case AXP20X_VBUC_CLIMIT_NONE:
> +			val->intval = -1;
> +			break;
> +		}
> +		return 0;
> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		r = axp20x_read_variable_width(power->regmap,
> +					       AXP20X_VBUS_I_ADC_H, 12);
> +		if (r < 0)
> +			return r;
> +
> +		val->intval = r * 375; /* 1 step = 0.375 mA */
> +		return 0;
> +	default:
> +		break;
> +	}
> +
> +	/* All the properties below need the input-status reg value */
> +	r = regmap_read(power->regmap, AXP20X_PWR_INPUT_STATUS, &input);
> +	if (r)
> +		return r;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_HEALTH:
> +		if (!(input & AXP20X_PWR_STATUS_VBUS_PRESENT)) {
> +			val->intval = POWER_SUPPLY_HEALTH_UNKNOWN;
> +			break;
> +		}
> +
> +		r = regmap_read(power->regmap, AXP20X_USB_OTG_STATUS, &v);
> +		if (r)
> +			return r;
> +
> +		if (!(v & AXP20X_USB_STATUS_VBUS_VALID)) {
> +			val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
> +			break;
> +		}
> +
> +		val->intval = POWER_SUPPLY_HEALTH_GOOD;
> +		break;
> +	case POWER_SUPPLY_PROP_PRESENT:
> +		val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_PRESENT);
> +		break;
> +	case POWER_SUPPLY_PROP_ONLINE:
> +		val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_USED);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static enum power_supply_property axp20x_usb_power_properties[] = {
> +	POWER_SUPPLY_PROP_HEALTH,
> +	POWER_SUPPLY_PROP_PRESENT,
> +	POWER_SUPPLY_PROP_ONLINE,
> +	POWER_SUPPLY_PROP_VOLTAGE_MIN,
> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +	POWER_SUPPLY_PROP_CURRENT_MAX,
> +	POWER_SUPPLY_PROP_CURRENT_NOW,
> +};
> +
> +static const struct power_supply_desc axp20x_usb_power_desc = {
> +	.name = "axp20x-usb",
> +	.type = POWER_SUPPLY_TYPE_USB,
> +	.properties = axp20x_usb_power_properties,
> +	.num_properties = ARRAY_SIZE(axp20x_usb_power_properties),
> +	.get_property = axp20x_usb_power_get_property,
> +};
> +
> +static int axp20x_usb_power_probe(struct platform_device *pdev)
> +{
> +	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
> +	struct power_supply_config psy_cfg = {};
> +	struct axp20x_usb_power *power;
> +	static const char * const irq_names[] = { "VBUS_PLUGIN",
> +		"VBUS_REMOVAL", "VBUS_VALID", "VBUS_NOT_VALID" };
> +	int i, irq, r;
> +
> +	if (!of_device_is_available(pdev->dev.of_node))
> +		return -ENODEV;
> +
> +	power = devm_kzalloc(&pdev->dev, sizeof(*power), GFP_KERNEL);
> +	if (!power)
> +		return -ENOMEM;
> +
> +	power->regmap = axp20x->regmap;
> +
> +	/* Enable vbus valid checking */
> +	r = regmap_update_bits(power->regmap, AXP20X_VBUS_MON,
> +		    AXP20X_VBUS_MON_VBUS_VALID, AXP20X_VBUS_MON_VBUS_VALID);
> +	if (r)
> +		return r;
> +
> +	/* Enable vbus voltage and current measurement */
> +	r = 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 (r)
> +		return r;
> +
> +	psy_cfg.of_node = pdev->dev.of_node;
> +	psy_cfg.drv_data = power;
> +
> +	power->supply = devm_power_supply_register(&pdev->dev,
> +					&axp20x_usb_power_desc, &psy_cfg);
> +	if (IS_ERR(power->supply))
> +		return PTR_ERR(power->supply);
> +
> +	/* Request irqs after registering, as irqs may trigger immediately */
> +	for (i = 0; i < ARRAY_SIZE(irq_names); i++) {
> +		irq = platform_get_irq_byname(pdev, irq_names[i]);
> +		if (irq < 0) {
> +			dev_warn(&pdev->dev, "No IRQ for %s: %d\n",
> +				 irq_names[i], irq);
> +			continue;
> +		}
> +		irq = regmap_irq_get_virq(axp20x->regmap_irqc, irq);
> +		r = devm_request_any_context_irq(&pdev->dev, irq,
> +				axp20x_usb_power_irq, 0, DRVNAME, power);
> +		if (r < 0)
> +			dev_warn(&pdev->dev, "Error requesting %s IRQ: %d\n",
> +				 irq_names[i], r);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id axp20x_usb_power_match[] = {
> +	{ .compatible = "x-powers,axp202-usb-power-supply" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, axp20x_usb_power_match);
> +
> +static struct platform_driver axp20x_usb_power_driver = {
> +	.probe = axp20x_usb_power_probe,
> +	.driver = {
> +		.name = DRVNAME,
> +		.of_match_table = axp20x_usb_power_match,
> +	},
> +};
> +
> +module_platform_driver(axp20x_usb_power_driver);
> +
> +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
> +MODULE_DESCRIPTION("AXP20x PMIC USB power supply status driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index 6d8b39a..8ec996e 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -11,6 +11,8 @@
>  #ifndef __LINUX_MFD_AXP20X_H
>  #define __LINUX_MFD_AXP20X_H
>  
> +#include <linux/regmap.h>
> +
>  enum {
>  	AXP202_ID = 0,
>  	AXP209_ID,
> @@ -372,4 +374,26 @@ struct axp288_extcon_pdata {
>  	struct gpio_desc *gpio_mux_cntl;
>  };
>  
> +/* generic helper function for reading 9-16 bit wide regs */
> +static inline int axp20x_read_variable_width(struct regmap *regmap,
> +	unsigned int reg, unsigned int width)
> +{
> +	unsigned int reg_val, result;
> +	int err;
> +
> +	err = regmap_read(regmap, reg, &reg_val);
> +	if (err)
> +		return err;
> +
> +	result = reg_val << (width - 8);
> +
> +	err = regmap_read(regmap, reg + 1, &reg_val);
> +	if (err)
> +		return err;
> +
> +	result |= reg_val;
> +
> +	return result;
> +}
> +
>  #endif /* __LINUX_MFD_AXP20X_H */

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

Re: [linux-sunxi] Re: [PATCH v2 3/4] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Hans de Goede]

Hi,

On 24-06-15 13:23, Lee Jones wrote:
>> Add a cell for the usb power_supply part of the axp20x PMICs.
>
> Why are you duplicating the subject line?

Heh, because some maintainers insist that the main-body part of the
commit message must stand by itself, without needing the subject to be
understandable...

>> Note that this cell is only for the usb power_supply part and not the
>> ac-power / battery-charger / rtc-backup-bat-charger bits.
>>
>> Depending on the board each of those must be enabled / disabled separately
>> in devicetree as most boards do not use all 4. So in dt each one needs its
>> own child-node of the axp20x node. Another reason for using separate child
>> nodes for each is so that other devicetree nodes can have a power-supply
>> property with a phandle referencing a node representing a single
>> power-supply.
>>
>> The decision to use a separate devicetree node for each is reflected on
>> the kernel side by each getting its own mfd-cell / platform_device and
>> platform-driver.
>
> You don't really need to say any of this, as this is the 'norm'.

I agree it should be the norm, but I'm not sure if it actually is, while
working on this I've seen several drivers which instantiate multiple
power-supply class devices from a single mfd-cell. And this is what
Bruno's original patches did, so I would prefer to keep this

> What you didn't mention however, is that you're taking the opportunity
> to fix some formatting issues and that there are no functional changes
> in these lines.

That is the end result of your request to change the indentation to
avoid line-wrapping :)

>
>> Cc: Bruno Prémont <bonbons@linux-vserver.org>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -Use DEFINE_RES_IRQ_NAMED
>> -Change indentation of axp20x_cells initializers to avoid line wrapping
>> ---
>>   drivers/mfd/axp20x.c | 20 ++++++++++++++++----
>>   1 file changed, 16 insertions(+), 4 deletions(-)
>
> Patch looks okay however:
>
> Acked-by: Lee Jones <lee.jones@linaro.org>

Thanks.

Regards,

Hans


>
>> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
>> index f9a3c2d..ca4a604 100644
>> --- a/drivers/mfd/axp20x.c
>> +++ b/drivers/mfd/axp20x.c
>> @@ -113,6 +113,13 @@ static struct resource axp20x_pek_resources[] = {
>>   	},
>>   };
>>
>> +static struct resource axp20x_usb_power_supply_resources[] = {
>> +	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_PLUGIN, "VBUS_PLUGIN"),
>> +	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_REMOVAL, "VBUS_REMOVAL"),
>> +	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_VALID, "VBUS_VALID"),
>> +	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_NOT_VALID, "VBUS_NOT_VALID"),
>> +};
>> +
>>   static struct resource axp22x_pek_resources[] = {
>>   	{
>>   		.name   = "PEK_DBR",
>> @@ -363,11 +370,16 @@ static const struct regmap_irq_chip axp288_regmap_irq_chip = {
>>
>>   static struct mfd_cell axp20x_cells[] = {
>>   	{
>> -		.name			= "axp20x-pek",
>> -		.num_resources		= ARRAY_SIZE(axp20x_pek_resources),
>> -		.resources		= axp20x_pek_resources,
>> +		.name		= "axp20x-pek",
>> +		.num_resources	= ARRAY_SIZE(axp20x_pek_resources),
>> +		.resources	= axp20x_pek_resources,
>>   	}, {
>> -		.name			= "axp20x-regulator",
>> +		.name		= "axp20x-regulator",
>> +	}, {
>> +		.name		= "axp20x-usb-power-supply",
>> +		.of_compatible	= "x-powers,axp202-usb-power-supply",
>> +		.num_resources	= ARRAY_SIZE(axp20x_usb_power_supply_resources),
>> +		.resources	= axp20x_usb_power_supply_resources,
>>   	},
>>   };
>>
>

[linux-sunxi] Re: [PATCH v2 3/4] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Hans de Goede]

Hi,

On 24-06-15 13:23, Lee Jones wrote:
>> Add a cell for the usb power_supply part of the axp20x PMICs.
>
> Why are you duplicating the subject line?

Heh, because some maintainers insist that the main-body part of the
commit message must stand by itself, without needing the subject to be
understandable...

>> Note that this cell is only for the usb power_supply part and not the
>> ac-power / battery-charger / rtc-backup-bat-charger bits.
>>
>> Depending on the board each of those must be enabled / disabled separately
>> in devicetree as most boards do not use all 4. So in dt each one needs its
>> own child-node of the axp20x node. Another reason for using separate child
>> nodes for each is so that other devicetree nodes can have a power-supply
>> property with a phandle referencing a node representing a single
>> power-supply.
>>
>> The decision to use a separate devicetree node for each is reflected on
>> the kernel side by each getting its own mfd-cell / platform_device and
>> platform-driver.
>
> You don't really need to say any of this, as this is the 'norm'.

I agree it should be the norm, but I'm not sure if it actually is, while
working on this I've seen several drivers which instantiate multiple
power-supply class devices from a single mfd-cell. And this is what
Bruno's original patches did, so I would prefer to keep this

> What you didn't mention however, is that you're taking the opportunity
> to fix some formatting issues and that there are no functional changes
> in these lines.

That is the end result of your request to change the indentation to
avoid line-wrapping :)

>
>> Cc: Bruno Pr?mont <bonbons@linux-vserver.org>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -Use DEFINE_RES_IRQ_NAMED
>> -Change indentation of axp20x_cells initializers to avoid line wrapping
>> ---
>>   drivers/mfd/axp20x.c | 20 ++++++++++++++++----
>>   1 file changed, 16 insertions(+), 4 deletions(-)
>
> Patch looks okay however:
>
> Acked-by: Lee Jones <lee.jones@linaro.org>

Thanks.

Regards,

Hans


>
>> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
>> index f9a3c2d..ca4a604 100644
>> --- a/drivers/mfd/axp20x.c
>> +++ b/drivers/mfd/axp20x.c
>> @@ -113,6 +113,13 @@ static struct resource axp20x_pek_resources[] = {
>>   	},
>>   };
>>
>> +static struct resource axp20x_usb_power_supply_resources[] = {
>> +	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_PLUGIN, "VBUS_PLUGIN"),
>> +	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_REMOVAL, "VBUS_REMOVAL"),
>> +	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_VALID, "VBUS_VALID"),
>> +	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_NOT_VALID, "VBUS_NOT_VALID"),
>> +};
>> +
>>   static struct resource axp22x_pek_resources[] = {
>>   	{
>>   		.name   = "PEK_DBR",
>> @@ -363,11 +370,16 @@ static const struct regmap_irq_chip axp288_regmap_irq_chip = {
>>
>>   static struct mfd_cell axp20x_cells[] = {
>>   	{
>> -		.name			= "axp20x-pek",
>> -		.num_resources		= ARRAY_SIZE(axp20x_pek_resources),
>> -		.resources		= axp20x_pek_resources,
>> +		.name		= "axp20x-pek",
>> +		.num_resources	= ARRAY_SIZE(axp20x_pek_resources),
>> +		.resources	= axp20x_pek_resources,
>>   	}, {
>> -		.name			= "axp20x-regulator",
>> +		.name		= "axp20x-regulator",
>> +	}, {
>> +		.name		= "axp20x-usb-power-supply",
>> +		.of_compatible	= "x-powers,axp202-usb-power-supply",
>> +		.num_resources	= ARRAY_SIZE(axp20x_usb_power_supply_resources),
>> +		.resources	= axp20x_usb_power_supply_resources,
>>   	},
>>   };
>>
>

Re: Re: [PATCH v2 3/4] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Lee Jones]

On Wed, 24 Jun 2015, Hans de Goede wrote:
> On 24-06-15 13:23, Lee Jones wrote:
> >>Add a cell for the usb power_supply part of the axp20x PMICs.
> >
> >Why are you duplicating the subject line?
> 
> Heh, because some maintainers insist that the main-body part of the
> commit message must stand by itself, without needing the subject to be
> understandable...

Some Maintainers are crazy. ;)

> >>Note that this cell is only for the usb power_supply part and not the
> >>ac-power / battery-charger / rtc-backup-bat-charger bits.
> >>
> >>Depending on the board each of those must be enabled / disabled separately
> >>in devicetree as most boards do not use all 4. So in dt each one needs its
> >>own child-node of the axp20x node. Another reason for using separate child
> >>nodes for each is so that other devicetree nodes can have a power-supply
> >>property with a phandle referencing a node representing a single
> >>power-supply.
> >>
> >>The decision to use a separate devicetree node for each is reflected on
> >>the kernel side by each getting its own mfd-cell / platform_device and
> >>platform-driver.
> >
> >You don't really need to say any of this, as this is the 'norm'.
> 
> I agree it should be the norm, but I'm not sure if it actually is, while
> working on this I've seen several drivers which instantiate multiple
> power-supply class devices from a single mfd-cell. And this is what
> Bruno's original patches did, so I would prefer to keep this

Up to you, but it's really not required.  No one else says this stuff.

> >What you didn't mention however, is that you're taking the opportunity
> >to fix some formatting issues and that there are no functional changes
> >in these lines.
> 
> That is the end result of your request to change the indentation to
> avoid line-wrapping :)

Fine, you can add my Suggested-by too then [don't actually do that],
but regardless of where the idea came from you should still tell us
what the patch does in the commit log.  I was looking for functional
differences within those formatting changes.  If I wanted to be really
religious about it I would [rightly] say "don't mix functional changes
with clean-ups", but instead I'll politely ask for a quick mention in
the commit log when you re-submit.  Aren't I nice?! ;)

> >>Cc: Bruno Prémont <bonbons-ud5FBsm0p/xEiooADzr8i9i2O/JbrIOy@public.gmane.org>
> >>Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >>---
> >>Changes in v2:
> >>-Use DEFINE_RES_IRQ_NAMED
> >>-Change indentation of axp20x_cells initializers to avoid line wrapping
> >>---
> >>  drivers/mfd/axp20x.c | 20 ++++++++++++++++----
> >>  1 file changed, 16 insertions(+), 4 deletions(-)
> >
> >Patch looks okay however:
> >
> >Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> Thanks.
> 
> Regards,
> 
> Hans
> 
> 
> >
> >>diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> >>index f9a3c2d..ca4a604 100644
> >>--- a/drivers/mfd/axp20x.c
> >>+++ b/drivers/mfd/axp20x.c
> >>@@ -113,6 +113,13 @@ static struct resource axp20x_pek_resources[] = {
> >>  	},
> >>  };
> >>
> >>+static struct resource axp20x_usb_power_supply_resources[] = {
> >>+	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_PLUGIN, "VBUS_PLUGIN"),
> >>+	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_REMOVAL, "VBUS_REMOVAL"),
> >>+	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_VALID, "VBUS_VALID"),
> >>+	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_NOT_VALID, "VBUS_NOT_VALID"),
> >>+};
> >>+
> >>  static struct resource axp22x_pek_resources[] = {
> >>  	{
> >>  		.name   = "PEK_DBR",
> >>@@ -363,11 +370,16 @@ static const struct regmap_irq_chip axp288_regmap_irq_chip = {
> >>
> >>  static struct mfd_cell axp20x_cells[] = {
> >>  	{
> >>-		.name			= "axp20x-pek",
> >>-		.num_resources		= ARRAY_SIZE(axp20x_pek_resources),
> >>-		.resources		= axp20x_pek_resources,
> >>+		.name		= "axp20x-pek",
> >>+		.num_resources	= ARRAY_SIZE(axp20x_pek_resources),
> >>+		.resources	= axp20x_pek_resources,
> >>  	}, {
> >>-		.name			= "axp20x-regulator",
> >>+		.name		= "axp20x-regulator",
> >>+	}, {
> >>+		.name		= "axp20x-usb-power-supply",
> >>+		.of_compatible	= "x-powers,axp202-usb-power-supply",
> >>+		.num_resources	= ARRAY_SIZE(axp20x_usb_power_supply_resources),
> >>+		.resources	= axp20x_usb_power_supply_resources,
> >>  	},
> >>  };
> >>
> >

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

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v2 3/4] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Lee Jones]

On Wed, 24 Jun 2015, Hans de Goede wrote:
> On 24-06-15 13:23, Lee Jones wrote:
> >>Add a cell for the usb power_supply part of the axp20x PMICs.
> >
> >Why are you duplicating the subject line?
> 
> Heh, because some maintainers insist that the main-body part of the
> commit message must stand by itself, without needing the subject to be
> understandable...

Some Maintainers are crazy. ;)

> >>Note that this cell is only for the usb power_supply part and not the
> >>ac-power / battery-charger / rtc-backup-bat-charger bits.
> >>
> >>Depending on the board each of those must be enabled / disabled separately
> >>in devicetree as most boards do not use all 4. So in dt each one needs its
> >>own child-node of the axp20x node. Another reason for using separate child
> >>nodes for each is so that other devicetree nodes can have a power-supply
> >>property with a phandle referencing a node representing a single
> >>power-supply.
> >>
> >>The decision to use a separate devicetree node for each is reflected on
> >>the kernel side by each getting its own mfd-cell / platform_device and
> >>platform-driver.
> >
> >You don't really need to say any of this, as this is the 'norm'.
> 
> I agree it should be the norm, but I'm not sure if it actually is, while
> working on this I've seen several drivers which instantiate multiple
> power-supply class devices from a single mfd-cell. And this is what
> Bruno's original patches did, so I would prefer to keep this

Up to you, but it's really not required.  No one else says this stuff.

> >What you didn't mention however, is that you're taking the opportunity
> >to fix some formatting issues and that there are no functional changes
> >in these lines.
> 
> That is the end result of your request to change the indentation to
> avoid line-wrapping :)

Fine, you can add my Suggested-by too then [don't actually do that],
but regardless of where the idea came from you should still tell us
what the patch does in the commit log.  I was looking for functional
differences within those formatting changes.  If I wanted to be really
religious about it I would [rightly] say "don't mix functional changes
with clean-ups", but instead I'll politely ask for a quick mention in
the commit log when you re-submit.  Aren't I nice?! ;)

> >>Cc: Bruno Pr?mont <bonbons@linux-vserver.org>
> >>Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>---
> >>Changes in v2:
> >>-Use DEFINE_RES_IRQ_NAMED
> >>-Change indentation of axp20x_cells initializers to avoid line wrapping
> >>---
> >>  drivers/mfd/axp20x.c | 20 ++++++++++++++++----
> >>  1 file changed, 16 insertions(+), 4 deletions(-)
> >
> >Patch looks okay however:
> >
> >Acked-by: Lee Jones <lee.jones@linaro.org>
> 
> Thanks.
> 
> Regards,
> 
> Hans
> 
> 
> >
> >>diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> >>index f9a3c2d..ca4a604 100644
> >>--- a/drivers/mfd/axp20x.c
> >>+++ b/drivers/mfd/axp20x.c
> >>@@ -113,6 +113,13 @@ static struct resource axp20x_pek_resources[] = {
> >>  	},
> >>  };
> >>
> >>+static struct resource axp20x_usb_power_supply_resources[] = {
> >>+	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_PLUGIN, "VBUS_PLUGIN"),
> >>+	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_REMOVAL, "VBUS_REMOVAL"),
> >>+	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_VALID, "VBUS_VALID"),
> >>+	DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_VBUS_NOT_VALID, "VBUS_NOT_VALID"),
> >>+};
> >>+
> >>  static struct resource axp22x_pek_resources[] = {
> >>  	{
> >>  		.name   = "PEK_DBR",
> >>@@ -363,11 +370,16 @@ static const struct regmap_irq_chip axp288_regmap_irq_chip = {
> >>
> >>  static struct mfd_cell axp20x_cells[] = {
> >>  	{
> >>-		.name			= "axp20x-pek",
> >>-		.num_resources		= ARRAY_SIZE(axp20x_pek_resources),
> >>-		.resources		= axp20x_pek_resources,
> >>+		.name		= "axp20x-pek",
> >>+		.num_resources	= ARRAY_SIZE(axp20x_pek_resources),
> >>+		.resources	= axp20x_pek_resources,
> >>  	}, {
> >>-		.name			= "axp20x-regulator",
> >>+		.name		= "axp20x-regulator",
> >>+	}, {
> >>+		.name		= "axp20x-usb-power-supply",
> >>+		.of_compatible	= "x-powers,axp202-usb-power-supply",
> >>+		.num_resources	= ARRAY_SIZE(axp20x_usb_power_supply_resources),
> >>+		.resources	= axp20x_usb_power_supply_resources,
> >>  	},
> >>  };
> >>
> >

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