[PATCH 0/8] mfd/power/phy: Add support for otg vbus detection via axp pmic

[PATCH 0/8] mfd/power/phy: Add support for otg vbus detection via axp pmic

[Hans de Goede]

Hi All,

Here is a series which adds the beginning of power-supply support to
the axp20x pmic code. My primary reason for working on this is to
enable the use of the usb power-supply bits in the pmic to for vbus
detection on boards which do not have a vbus-det gpio, and instead
rely on the pmic for vbus detection.

After I had written most of the vbus power-supply driver code I
became aware of Bruno Prémont's (in the CC) previous work on this
our drivers are mostly the same, and I've borrowed some code from
his driver to add support for min-volt / max-curr properties.

The big difference between our 2 drivers is that mine driver uses
a devicetree child node / mfd cell per power-supply, so one for
each of the usb-power, ac-power / battery-charger and 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.

Regards,

Hans

[PATCH 0/8] mfd/power/phy: Add support for otg vbus detection via axp pmic

[Hans de Goede]

Hi All,

Here is a series which adds the beginning of power-supply support to
the axp20x pmic code. My primary reason for working on this is to
enable the use of the usb power-supply bits in the pmic to for vbus
detection on boards which do not have a vbus-det gpio, and instead
rely on the pmic for vbus detection.

After I had written most of the vbus power-supply driver code I
became aware of Bruno Pr?mont's (in the CC) previous work on this
our drivers are mostly the same, and I've borrowed some code from
his driver to add support for min-volt / max-curr properties.

The big difference between our 2 drivers is that mine driver uses
a devicetree child node / mfd cell per power-supply, so one for
each of the usb-power, ac-power / battery-charger and 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.

Regards,

Hans

[PATCH 1/8] 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>
---
 drivers/mfd/axp20x.c       | 6 ++++++
 include/linux/mfd/axp20x.h | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 6df9155..6ffbc11 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 = {
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 95568eb..f4290ae 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -151,6 +151,11 @@ 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))
+
 /* AXP22X specific registers */
 #define AXP22X_BATLOW_THRES1		0xe6
 
-- 
2.3.6

[PATCH 1/8] 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>
---
 drivers/mfd/axp20x.c       | 6 ++++++
 include/linux/mfd/axp20x.h | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 6df9155..6ffbc11 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 = {
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 95568eb..f4290ae 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -151,6 +151,11 @@ 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))
+
 /* AXP22X specific registers */
 #define AXP22X_BATLOW_THRES1		0xe6
 
-- 
2.3.6

[PATCH 2/8] 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>
---
 drivers/mfd/axp20x.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 6ffbc11..47ce233 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -113,6 +113,30 @@ static struct resource axp20x_pek_resources[] = {
 	},
 };
 
+static struct resource axp20x_usb_power_supply_resources[] = {
+	{
+		.name	= "VBUS_PLUGIN",
+		.start	= AXP20X_IRQ_VBUS_PLUGIN,
+		.end	= AXP20X_IRQ_VBUS_PLUGIN,
+		.flags	= IORESOURCE_IRQ,
+	}, {
+		.name	= "VBUS_REMOVAL",
+		.start	= AXP20X_IRQ_VBUS_REMOVAL,
+		.end	= AXP20X_IRQ_VBUS_REMOVAL,
+		.flags	= IORESOURCE_IRQ,
+	}, {
+		.name	= "VBUS_VALID",
+		.start	= AXP20X_IRQ_VBUS_VALID,
+		.end	= AXP20X_IRQ_VBUS_VALID,
+		.flags	= IORESOURCE_IRQ,
+	}, {
+		.name	= "VBUS_NOT_VALID",
+		.start	= AXP20X_IRQ_VBUS_NOT_VALID,
+		.end	= AXP20X_IRQ_VBUS_NOT_VALID,
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
 static struct resource axp22x_pek_resources[] = {
 	{
 		.name   = "PEK_DBR",
@@ -165,7 +189,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(15),
 	.cache_type	= REGCACHE_RBTREE,
 };
 
@@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = {
 		.resources		= axp20x_pek_resources,
 	}, {
 		.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.3.6

-- 
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 2/8] 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>
---
 drivers/mfd/axp20x.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 6ffbc11..47ce233 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -113,6 +113,30 @@ static struct resource axp20x_pek_resources[] = {
 	},
 };
 
+static struct resource axp20x_usb_power_supply_resources[] = {
+	{
+		.name	= "VBUS_PLUGIN",
+		.start	= AXP20X_IRQ_VBUS_PLUGIN,
+		.end	= AXP20X_IRQ_VBUS_PLUGIN,
+		.flags	= IORESOURCE_IRQ,
+	}, {
+		.name	= "VBUS_REMOVAL",
+		.start	= AXP20X_IRQ_VBUS_REMOVAL,
+		.end	= AXP20X_IRQ_VBUS_REMOVAL,
+		.flags	= IORESOURCE_IRQ,
+	}, {
+		.name	= "VBUS_VALID",
+		.start	= AXP20X_IRQ_VBUS_VALID,
+		.end	= AXP20X_IRQ_VBUS_VALID,
+		.flags	= IORESOURCE_IRQ,
+	}, {
+		.name	= "VBUS_NOT_VALID",
+		.start	= AXP20X_IRQ_VBUS_NOT_VALID,
+		.end	= AXP20X_IRQ_VBUS_NOT_VALID,
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
 static struct resource axp22x_pek_resources[] = {
 	{
 		.name   = "PEK_DBR",
@@ -165,7 +189,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(15),
 	.cache_type	= REGCACHE_RBTREE,
 };
 
@@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = {
 		.resources		= axp20x_pek_resources,
 	}, {
 		.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.3.6

[PATCH 3/8] power: Add devm_power_supply_get_by_phandle() helper function

[Hans de Goede]

This commit adds a resource-managed version of the
power_supply_get_by_phandle() function.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/power_supply_core.c | 39 +++++++++++++++++++++++++++++++++++++++
 include/linux/power_supply.h      |  5 +++++
 2 files changed, 44 insertions(+)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 2ed4a4a..59856bc 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -420,6 +420,45 @@ struct power_supply *power_supply_get_by_phandle(struct device_node *np,
 	return psy;
 }
 EXPORT_SYMBOL_GPL(power_supply_get_by_phandle);
+
+static void devm_power_supply_put(struct device *dev, void *res)
+{
+	struct power_supply **psy = res;
+
+	power_supply_put(*psy);
+}
+
+/**
+ * devm_power_supply_get_by_phandle() - Resource managed version of
+ *  power_supply_get_by_phandle()
+ * @dev: Pointer to device holding phandle property
+ * @phandle_name: Name of property holding a power supply phandle
+ *
+ * Return: On success returns a reference to a power supply with
+ * matching name equals to value under @property, NULL or ERR_PTR otherwise.
+ */
+struct power_supply *devm_power_supply_get_by_phandle(struct device *dev,
+						      const char *property)
+{
+	struct power_supply **ptr, *psy;
+
+	if (!dev->of_node)
+		return ERR_PTR(-ENODEV);
+
+	ptr = devres_alloc(devm_power_supply_put, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	psy = power_supply_get_by_phandle(dev->of_node, property);
+	if (IS_ERR_OR_NULL(psy)) {
+		devres_free(ptr);
+	} else {
+		*ptr = psy;
+		devres_add(dev, ptr);
+	}
+	return psy;
+}
+EXPORT_SYMBOL_GPL(devm_power_supply_get_by_phandle);
 #endif /* CONFIG_OF */
 
 int power_supply_get_property(struct power_supply *psy,
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 75a1dd8..a99cf4f 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -286,10 +286,15 @@ extern void power_supply_put(struct power_supply *psy);
 #ifdef CONFIG_OF
 extern struct power_supply *power_supply_get_by_phandle(struct device_node *np,
 							const char *property);
+extern struct power_supply *devm_power_supply_get_by_phandle(
+				    struct device *dev, const char *property);
 #else /* !CONFIG_OF */
 static inline struct power_supply *
 power_supply_get_by_phandle(struct device_node *np, const char *property)
 { return NULL; }
+static inline struct power_supply *
+devm_power_supply_get_by_phandle(struct device *dev, const char *property)
+{ return NULL; }
 #endif /* CONFIG_OF */
 extern void power_supply_changed(struct power_supply *psy);
 extern int power_supply_am_i_supplied(struct power_supply *psy);
-- 
2.3.6

[PATCH 3/8] power: Add devm_power_supply_get_by_phandle() helper function

[Hans de Goede]

This commit adds a resource-managed version of the
power_supply_get_by_phandle() function.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/power_supply_core.c | 39 +++++++++++++++++++++++++++++++++++++++
 include/linux/power_supply.h      |  5 +++++
 2 files changed, 44 insertions(+)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 2ed4a4a..59856bc 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -420,6 +420,45 @@ struct power_supply *power_supply_get_by_phandle(struct device_node *np,
 	return psy;
 }
 EXPORT_SYMBOL_GPL(power_supply_get_by_phandle);
+
+static void devm_power_supply_put(struct device *dev, void *res)
+{
+	struct power_supply **psy = res;
+
+	power_supply_put(*psy);
+}
+
+/**
+ * devm_power_supply_get_by_phandle() - Resource managed version of
+ *  power_supply_get_by_phandle()
+ * @dev: Pointer to device holding phandle property
+ * @phandle_name: Name of property holding a power supply phandle
+ *
+ * Return: On success returns a reference to a power supply with
+ * matching name equals to value under @property, NULL or ERR_PTR otherwise.
+ */
+struct power_supply *devm_power_supply_get_by_phandle(struct device *dev,
+						      const char *property)
+{
+	struct power_supply **ptr, *psy;
+
+	if (!dev->of_node)
+		return ERR_PTR(-ENODEV);
+
+	ptr = devres_alloc(devm_power_supply_put, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	psy = power_supply_get_by_phandle(dev->of_node, property);
+	if (IS_ERR_OR_NULL(psy)) {
+		devres_free(ptr);
+	} else {
+		*ptr = psy;
+		devres_add(dev, ptr);
+	}
+	return psy;
+}
+EXPORT_SYMBOL_GPL(devm_power_supply_get_by_phandle);
 #endif /* CONFIG_OF */
 
 int power_supply_get_property(struct power_supply *psy,
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 75a1dd8..a99cf4f 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -286,10 +286,15 @@ extern void power_supply_put(struct power_supply *psy);
 #ifdef CONFIG_OF
 extern struct power_supply *power_supply_get_by_phandle(struct device_node *np,
 							const char *property);
+extern struct power_supply *devm_power_supply_get_by_phandle(
+				    struct device *dev, const char *property);
 #else /* !CONFIG_OF */
 static inline struct power_supply *
 power_supply_get_by_phandle(struct device_node *np, const char *property)
 { return NULL; }
+static inline struct power_supply *
+devm_power_supply_get_by_phandle(struct device *dev, const char *property)
+{ return NULL; }
 #endif /* CONFIG_OF */
 extern void power_supply_changed(struct power_supply *psy);
 extern int power_supply_am_i_supplied(struct power_supply *psy);
-- 
2.3.6

[PATCH 4/8] 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>
---
 .../bindings/power_supply/axp20x_usb_power.txt     |  34 +++
 drivers/power/Kconfig                              |   7 +
 drivers/power/Makefile                             |   1 +
 drivers/power/axp20x_usb_power.c                   | 241 +++++++++++++++++++++
 include/linux/mfd/axp20x.h                         |  24 ++
 5 files changed, 307 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt
 create mode 100644 drivers/power/axp20x_usb_power.c

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";
+	};
+};
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..4be4c8b
--- /dev/null
+++ b/drivers/power/axp20x_usb_power.c
@@ -0,0 +1,241 @@
+/*
+ * 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_16bit(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_16bit(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;
+	const char *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 f4290ae..d7adb2e 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,
@@ -366,4 +368,26 @@ struct axp20x_fg_pdata {
 	int thermistor_curve[MAX_THERM_CURVE_SIZE][2];
 };
 
+/* generic helper function for reading 9-16 bit wide regs */
+static inline int axp20x_read_16bit(struct regmap *regmap,
+				    unsigned int reg, unsigned int width)
+{
+	unsigned int v, raw;
+	int r;
+
+	r = regmap_read(regmap, reg, &v);
+	if (r)
+		return r;
+
+	raw = v << (width - 8);
+
+	r = regmap_read(regmap, reg + 1, &v);
+	if (r)
+		return r;
+
+	raw |= v;
+
+	return raw;
+}
+
 #endif /* __LINUX_MFD_AXP20X_H */
-- 
2.3.6

--
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 4/8] 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>
---
 .../bindings/power_supply/axp20x_usb_power.txt     |  34 +++
 drivers/power/Kconfig                              |   7 +
 drivers/power/Makefile                             |   1 +
 drivers/power/axp20x_usb_power.c                   | 241 +++++++++++++++++++++
 include/linux/mfd/axp20x.h                         |  24 ++
 5 files changed, 307 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt
 create mode 100644 drivers/power/axp20x_usb_power.c

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";
+	};
+};
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..4be4c8b
--- /dev/null
+++ b/drivers/power/axp20x_usb_power.c
@@ -0,0 +1,241 @@
+/*
+ * 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_16bit(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_16bit(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;
+	const char *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 f4290ae..d7adb2e 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,
@@ -366,4 +368,26 @@ struct axp20x_fg_pdata {
 	int thermistor_curve[MAX_THERM_CURVE_SIZE][2];
 };
 
+/* generic helper function for reading 9-16 bit wide regs */
+static inline int axp20x_read_16bit(struct regmap *regmap,
+				    unsigned int reg, unsigned int width)
+{
+	unsigned int v, raw;
+	int r;
+
+	r = regmap_read(regmap, reg, &v);
+	if (r)
+		return r;
+
+	raw = v << (width - 8);
+
+	r = regmap_read(regmap, reg + 1, &v);
+	if (r)
+		return r;
+
+	raw |= v;
+
+	return raw;
+}
+
 #endif /* __LINUX_MFD_AXP20X_H */
-- 
2.3.6

[PATCH 5/8] phy-sun4i-usb: Add support for monitoring vbus via a power-supply

[Hans de Goede]

On some boards there is no vbus_det gpio pin, instead vbus-detection for
otg can be done via the pmic.

This commit adds support for monitoring vbus_det via the power_supply
exported by the pmic, enabling support for otg on these boards.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 .../devicetree/bindings/phy/sun4i-usb-phy.txt      |  1 +
 drivers/phy/phy-sun4i-usb.c                        | 68 +++++++++++++++++++---
 2 files changed, 62 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
index 5f48979..0cebf74 100644
--- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
@@ -29,6 +29,7 @@ Required properties:
 Optional properties:
 - usb0_id_det-gpios : gpio phandle for reading the otg id pin value
 - usb0_vbus_det-gpios : gpio phandle for detecting the presence of usb0 vbus
+- usb0_vbus_power-supply: power-supply phandle for usb0 vbus presence detect
 - usb0_vbus-supply : regulator phandle for controller usb0 vbus
 - usb1_vbus-supply : regulator phandle for controller usb1 vbus
 - usb2_vbus-supply : regulator phandle for controller usb2 vbus
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 4981041..85bb215 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -36,6 +36,7 @@
 #include <linux/phy/phy.h>
 #include <linux/phy/phy-sun4i-usb.h>
 #include <linux/platform_device.h>
+#include <linux/power_supply.h>
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 #include <linux/workqueue.h>
@@ -111,6 +112,8 @@ struct sun4i_usb_phy_data {
 	bool phy0_poll;
 	struct gpio_desc *id_det_gpio;
 	struct gpio_desc *vbus_det_gpio;
+	struct power_supply *vbus_power_supply;
+	struct notifier_block vbus_power_nb;
 	int id_det_irq;
 	int vbus_det_irq;
 	int id_det;
@@ -355,6 +358,30 @@ static struct phy_ops sun4i_usb_phy_ops = {
 	.owner		= THIS_MODULE,
 };
 
+static int sun4i_usb_phy0_get_vbus_det(struct sun4i_usb_phy_data *data)
+{
+	if (data->vbus_det_gpio)
+		return gpiod_get_value_cansleep(data->vbus_det_gpio);
+
+	if (data->vbus_power_supply) {
+		union power_supply_propval val;
+		int r;
+
+		r = power_supply_get_property(data->vbus_power_supply,
+					      POWER_SUPPLY_PROP_PRESENT, &val);
+		if (r == 0)
+			return val.intval;
+	}
+
+	/* Fallback: report vbus as high */
+	return 1;
+}
+
+static bool sun4i_usb_phy0_have_vbus_det(struct sun4i_usb_phy_data *data)
+{
+	return data->vbus_det_gpio || data->vbus_power_supply;
+}
+
 static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
 {
 	struct sun4i_usb_phy_data *data =
@@ -363,10 +390,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
 	int id_det, vbus_det, id_notify = 0, vbus_notify = 0;
 
 	id_det = gpiod_get_value_cansleep(data->id_det_gpio);
-	if (data->vbus_det_gpio)
-		vbus_det = gpiod_get_value_cansleep(data->vbus_det_gpio);
-	else
-		vbus_det = 1; /* Report vbus as high */
+	vbus_det = sun4i_usb_phy0_get_vbus_det(data);
 
 	mutex_lock(&phy0->mutex);
 
@@ -381,7 +405,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
 		 * without vbus detection report vbus low for long enough for
 		 * the musb-ip to end the current device session.
 		 */
-		if (!data->vbus_det_gpio && id_det == 0) {
+		if (!sun4i_usb_phy0_have_vbus_det(data) && id_det == 0) {
 			sun4i_usb_phy0_set_vbus_detect(phy0, 0);
 			msleep(200);
 			sun4i_usb_phy0_set_vbus_detect(phy0, 1);
@@ -408,7 +432,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
 		 * without vbus detection report vbus low for long enough to
 		 * the musb-ip to end the current host session.
 		 */
-		if (!data->vbus_det_gpio && id_det == 1) {
+		if (!sun4i_usb_phy0_have_vbus_det(data) && id_det == 1) {
 			mutex_lock(&phy0->mutex);
 			sun4i_usb_phy0_set_vbus_detect(phy0, 0);
 			msleep(1000);
@@ -436,6 +460,20 @@ static irqreturn_t sun4i_usb_phy0_id_vbus_det_irq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static int sun4i_usb_phy0_vbus_notify(struct notifier_block *nb,
+				      unsigned long val, void *v)
+{
+	struct sun4i_usb_phy_data *data = 
+		container_of(nb, struct sun4i_usb_phy_data, vbus_power_nb);
+	struct power_supply *psy = v;
+
+	/* Properties on the vbus_power_supply changed, scan vbus_det */
+	if (val == PSY_EVENT_PROP_CHANGED && psy == data->vbus_power_supply)
+		mod_delayed_work(system_wq, &data->detect, DEBOUNCE_TIME);
+
+	return NOTIFY_OK;
+}
+
 static struct phy *sun4i_usb_phy_xlate(struct device *dev,
 					struct of_phandle_args *args)
 {
@@ -512,8 +550,24 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
 		data->vbus_det_gpio = NULL;
 	}
 
+	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))
+			return PTR_ERR(data->vbus_power_supply);
+
+		if (!data->vbus_power_supply)
+			return -EPROBE_DEFER;
+
+		data->vbus_power_nb.notifier_call = sun4i_usb_phy0_vbus_notify;
+		data->vbus_power_nb.priority = 0;
+		ret = power_supply_reg_notifier(&data->vbus_power_nb);
+		if (ret)
+			return ret;
+	}
+
 	/* vbus_det without id_det makes no sense, and is not supported */
-	if (data->vbus_det_gpio && !data->id_det_gpio) {
+	if (sun4i_usb_phy0_have_vbus_det(data) && !data->id_det_gpio) {
 		dev_err(dev, "usb0_id_det missing or invalid\n");
 		return -ENODEV;
 	}
-- 
2.3.6

--
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 5/8] phy-sun4i-usb: Add support for monitoring vbus via a power-supply

[Hans de Goede]

On some boards there is no vbus_det gpio pin, instead vbus-detection for
otg can be done via the pmic.

This commit adds support for monitoring vbus_det via the power_supply
exported by the pmic, enabling support for otg on these boards.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../devicetree/bindings/phy/sun4i-usb-phy.txt      |  1 +
 drivers/phy/phy-sun4i-usb.c                        | 68 +++++++++++++++++++---
 2 files changed, 62 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
index 5f48979..0cebf74 100644
--- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
@@ -29,6 +29,7 @@ Required properties:
 Optional properties:
 - usb0_id_det-gpios : gpio phandle for reading the otg id pin value
 - usb0_vbus_det-gpios : gpio phandle for detecting the presence of usb0 vbus
+- usb0_vbus_power-supply: power-supply phandle for usb0 vbus presence detect
 - usb0_vbus-supply : regulator phandle for controller usb0 vbus
 - usb1_vbus-supply : regulator phandle for controller usb1 vbus
 - usb2_vbus-supply : regulator phandle for controller usb2 vbus
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 4981041..85bb215 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -36,6 +36,7 @@
 #include <linux/phy/phy.h>
 #include <linux/phy/phy-sun4i-usb.h>
 #include <linux/platform_device.h>
+#include <linux/power_supply.h>
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 #include <linux/workqueue.h>
@@ -111,6 +112,8 @@ struct sun4i_usb_phy_data {
 	bool phy0_poll;
 	struct gpio_desc *id_det_gpio;
 	struct gpio_desc *vbus_det_gpio;
+	struct power_supply *vbus_power_supply;
+	struct notifier_block vbus_power_nb;
 	int id_det_irq;
 	int vbus_det_irq;
 	int id_det;
@@ -355,6 +358,30 @@ static struct phy_ops sun4i_usb_phy_ops = {
 	.owner		= THIS_MODULE,
 };
 
+static int sun4i_usb_phy0_get_vbus_det(struct sun4i_usb_phy_data *data)
+{
+	if (data->vbus_det_gpio)
+		return gpiod_get_value_cansleep(data->vbus_det_gpio);
+
+	if (data->vbus_power_supply) {
+		union power_supply_propval val;
+		int r;
+
+		r = power_supply_get_property(data->vbus_power_supply,
+					      POWER_SUPPLY_PROP_PRESENT, &val);
+		if (r == 0)
+			return val.intval;
+	}
+
+	/* Fallback: report vbus as high */
+	return 1;
+}
+
+static bool sun4i_usb_phy0_have_vbus_det(struct sun4i_usb_phy_data *data)
+{
+	return data->vbus_det_gpio || data->vbus_power_supply;
+}
+
 static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
 {
 	struct sun4i_usb_phy_data *data =
@@ -363,10 +390,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
 	int id_det, vbus_det, id_notify = 0, vbus_notify = 0;
 
 	id_det = gpiod_get_value_cansleep(data->id_det_gpio);
-	if (data->vbus_det_gpio)
-		vbus_det = gpiod_get_value_cansleep(data->vbus_det_gpio);
-	else
-		vbus_det = 1; /* Report vbus as high */
+	vbus_det = sun4i_usb_phy0_get_vbus_det(data);
 
 	mutex_lock(&phy0->mutex);
 
@@ -381,7 +405,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
 		 * without vbus detection report vbus low for long enough for
 		 * the musb-ip to end the current device session.
 		 */
-		if (!data->vbus_det_gpio && id_det == 0) {
+		if (!sun4i_usb_phy0_have_vbus_det(data) && id_det == 0) {
 			sun4i_usb_phy0_set_vbus_detect(phy0, 0);
 			msleep(200);
 			sun4i_usb_phy0_set_vbus_detect(phy0, 1);
@@ -408,7 +432,7 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
 		 * without vbus detection report vbus low for long enough to
 		 * the musb-ip to end the current host session.
 		 */
-		if (!data->vbus_det_gpio && id_det == 1) {
+		if (!sun4i_usb_phy0_have_vbus_det(data) && id_det == 1) {
 			mutex_lock(&phy0->mutex);
 			sun4i_usb_phy0_set_vbus_detect(phy0, 0);
 			msleep(1000);
@@ -436,6 +460,20 @@ static irqreturn_t sun4i_usb_phy0_id_vbus_det_irq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static int sun4i_usb_phy0_vbus_notify(struct notifier_block *nb,
+				      unsigned long val, void *v)
+{
+	struct sun4i_usb_phy_data *data = 
+		container_of(nb, struct sun4i_usb_phy_data, vbus_power_nb);
+	struct power_supply *psy = v;
+
+	/* Properties on the vbus_power_supply changed, scan vbus_det */
+	if (val == PSY_EVENT_PROP_CHANGED && psy == data->vbus_power_supply)
+		mod_delayed_work(system_wq, &data->detect, DEBOUNCE_TIME);
+
+	return NOTIFY_OK;
+}
+
 static struct phy *sun4i_usb_phy_xlate(struct device *dev,
 					struct of_phandle_args *args)
 {
@@ -512,8 +550,24 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
 		data->vbus_det_gpio = NULL;
 	}
 
+	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))
+			return PTR_ERR(data->vbus_power_supply);
+
+		if (!data->vbus_power_supply)
+			return -EPROBE_DEFER;
+
+		data->vbus_power_nb.notifier_call = sun4i_usb_phy0_vbus_notify;
+		data->vbus_power_nb.priority = 0;
+		ret = power_supply_reg_notifier(&data->vbus_power_nb);
+		if (ret)
+			return ret;
+	}
+
 	/* vbus_det without id_det makes no sense, and is not supported */
-	if (data->vbus_det_gpio && !data->id_det_gpio) {
+	if (sun4i_usb_phy0_have_vbus_det(data) && !data->id_det_gpio) {
 		dev_err(dev, "usb0_id_det missing or invalid\n");
 		return -ENODEV;
 	}
-- 
2.3.6

[PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Hans de Goede]

Add a node representing the usb power supply part of the axp209 pmic, note
that the usb power supply and the (to be added later) ac power supply will
each have their own child-node, so that they can be separately specified
as power-supply for other nodes using a power-supply property with a
phandle pointing to the right axp209 child-node.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 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 24c935c..051ab3b 100644
--- a/arch/arm/boot/dts/axp209.dtsi
+++ b/arch/arm/boot/dts/axp209.dtsi
@@ -89,4 +89,9 @@
 			regulator-name = "ldo5";
 		};
 	};
+
+	usb_power_supply: usb_power_supply {
+		compatible = "x-powers,axp202-usb-power-supply";
+		status = "disabled";
+	};
 };
-- 
2.3.6

[PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Hans de Goede]

Add a node representing the usb power supply part of the axp209 pmic, note
that the usb power supply and the (to be added later) ac power supply will
each have their own child-node, so that they can be separately specified
as power-supply for other nodes using a power-supply property with a
phandle pointing to the right axp209 child-node.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 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 24c935c..051ab3b 100644
--- a/arch/arm/boot/dts/axp209.dtsi
+++ b/arch/arm/boot/dts/axp209.dtsi
@@ -89,4 +89,9 @@
 			regulator-name = "ldo5";
 		};
 	};
+
+	usb_power_supply: usb_power_supply {
+		compatible = "x-powers,axp202-usb-power-supply";
+		status = "disabled";
+	};
 };
-- 
2.3.6

[PATCH 7/8] ARM: dts: sun7i: Add regulator configuration to the bananapi dts file

[Hans de Goede]

Add regulator configuration to the bananapi dts file, this enables cpu
voltage scaling.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/boot/dts/sun7i-a20-bananapi.dts | 39 ++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
index 9f7b472..2a1188102 100644
--- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
+++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
@@ -92,6 +92,10 @@
 	status = "okay";
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &ehci0 {
 	status = "okay";
 };
@@ -119,16 +123,14 @@
 	status = "okay";
 
 	axp209: pmic@34 {
-		compatible = "x-powers,axp209";
 		reg = <0x34>;
 		interrupt-parent = <&nmi_intc>;
 		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
-
-		interrupt-controller;
-		#interrupt-cells = <1>;
 	};
 };
 
+#include "axp209.dtsi"
+
 &i2c2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c2_pins_a>;
@@ -182,6 +184,31 @@
 	};
 };
 
+&reg_dcdc2 {
+	regulator-always-on;
+	regulator-min-microvolt = <1000000>;
+	regulator-max-microvolt = <1500000>;
+	regulator-name = "vdd-cpu";
+};
+
+&reg_dcdc3 {
+	regulator-always-on;
+	regulator-min-microvolt = <1000000>;
+	regulator-max-microvolt = <1400000>;
+	regulator-name = "vdd-int-pll";
+};
+
+&reg_ldo1 {
+	regulator-name = "vdd-rtc";
+};
+
+&reg_ldo2 {
+	regulator-always-on;
+	regulator-min-microvolt = <3000000>;
+	regulator-max-microvolt = <3000000>;
+	regulator-name = "avcc";
+};
+
 &reg_usb1_vbus {
 	status = "okay";
 };
@@ -216,6 +243,10 @@
 	status = "okay";
 };
 
+&usb_power_supply {
+	status = "okay";
+};
+
 &usbphy {
 	usb1_vbus-supply = <&reg_usb1_vbus>;
 	usb2_vbus-supply = <&reg_usb2_vbus>;
-- 
2.3.6

--
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 7/8] ARM: dts: sun7i: Add regulator configuration to the bananapi dts file

[Hans de Goede]

Add regulator configuration to the bananapi dts file, this enables cpu
voltage scaling.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/boot/dts/sun7i-a20-bananapi.dts | 39 ++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
index 9f7b472..2a1188102 100644
--- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
+++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
@@ -92,6 +92,10 @@
 	status = "okay";
 };
 
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
 &ehci0 {
 	status = "okay";
 };
@@ -119,16 +123,14 @@
 	status = "okay";
 
 	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>;
 	};
 };
 
+#include "axp209.dtsi"
+
 &i2c2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c2_pins_a>;
@@ -182,6 +184,31 @@
 	};
 };
 
+&reg_dcdc2 {
+	regulator-always-on;
+	regulator-min-microvolt = <1000000>;
+	regulator-max-microvolt = <1500000>;
+	regulator-name = "vdd-cpu";
+};
+
+&reg_dcdc3 {
+	regulator-always-on;
+	regulator-min-microvolt = <1000000>;
+	regulator-max-microvolt = <1400000>;
+	regulator-name = "vdd-int-pll";
+};
+
+&reg_ldo1 {
+	regulator-name = "vdd-rtc";
+};
+
+&reg_ldo2 {
+	regulator-always-on;
+	regulator-min-microvolt = <3000000>;
+	regulator-max-microvolt = <3000000>;
+	regulator-name = "avcc";
+};
+
 &reg_usb1_vbus {
 	status = "okay";
 };
@@ -216,6 +243,10 @@
 	status = "okay";
 };
 
+&usb_power_supply {
+	status = "okay";
+};
+
 &usbphy {
 	usb1_vbus-supply = <&reg_usb1_vbus>;
 	usb2_vbus-supply = <&reg_usb2_vbus>;
-- 
2.3.6

[PATCH 8/8] ARM: dts: sun7i: Enable USB DRC on Bananapi

[Hans de Goede]

Enable the otg/drc usb controller on the Bananapi.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/boot/dts/sun7i-a20-bananapi.dts | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
index 2a1188102..d268c78 100644
--- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
+++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
@@ -161,7 +161,18 @@
 	status = "okay";
 };
 
+&otg_sram {
+	status = "okay";
+};
+
 &pio {
+	usb0_id_detect_pin: usb0_id_detect_pin@0 {
+		allwinner,pins = "PH4";
+		allwinner,function = "gpio_in";
+		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+		allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
+	};
+
 	mmc0_cd_pin_bananapi: mmc0_cd_pin@0 {
 		allwinner,pins = "PH10";
 		allwinner,function = "gpio_in";
@@ -209,6 +220,10 @@
 	regulator-name = "avcc";
 };
 
+&reg_usb0_vbus {
+	status = "okay";
+};
+
 &reg_usb1_vbus {
 	status = "okay";
 };
@@ -243,11 +258,21 @@
 	status = "okay";
 };
 
+&usb_otg {
+	dr_mode = "otg";
+	status = "okay";
+};
+
 &usb_power_supply {
 	status = "okay";
 };
 
 &usbphy {
+	pinctrl-names = "default";
+	pinctrl-0 = <&usb0_id_detect_pin>;
+	usb0_id_det-gpio = <&pio 7 4 GPIO_ACTIVE_HIGH>; /* PH4 */
+	usb0_vbus_power-supply = <&usb_power_supply>;
+	usb0_vbus-supply = <&reg_usb0_vbus>;
 	usb1_vbus-supply = <&reg_usb1_vbus>;
 	usb2_vbus-supply = <&reg_usb2_vbus>;
 	status = "okay";
-- 
2.3.6

[PATCH 8/8] ARM: dts: sun7i: Enable USB DRC on Bananapi

[Hans de Goede]

Enable the otg/drc usb controller on the Bananapi.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/boot/dts/sun7i-a20-bananapi.dts | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
index 2a1188102..d268c78 100644
--- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
+++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
@@ -161,7 +161,18 @@
 	status = "okay";
 };
 
+&otg_sram {
+	status = "okay";
+};
+
 &pio {
+	usb0_id_detect_pin: usb0_id_detect_pin at 0 {
+		allwinner,pins = "PH4";
+		allwinner,function = "gpio_in";
+		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+		allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
+	};
+
 	mmc0_cd_pin_bananapi: mmc0_cd_pin at 0 {
 		allwinner,pins = "PH10";
 		allwinner,function = "gpio_in";
@@ -209,6 +220,10 @@
 	regulator-name = "avcc";
 };
 
+&reg_usb0_vbus {
+	status = "okay";
+};
+
 &reg_usb1_vbus {
 	status = "okay";
 };
@@ -243,11 +258,21 @@
 	status = "okay";
 };
 
+&usb_otg {
+	dr_mode = "otg";
+	status = "okay";
+};
+
 &usb_power_supply {
 	status = "okay";
 };
 
 &usbphy {
+	pinctrl-names = "default";
+	pinctrl-0 = <&usb0_id_detect_pin>;
+	usb0_id_det-gpio = <&pio 7 4 GPIO_ACTIVE_HIGH>; /* PH4 */
+	usb0_vbus_power-supply = <&usb_power_supply>;
+	usb0_vbus-supply = <&reg_usb0_vbus>;
 	usb1_vbus-supply = <&reg_usb1_vbus>;
 	usb2_vbus-supply = <&reg_usb2_vbus>;
 	status = "okay";
-- 
2.3.6

Re: [PATCH 2/8] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Chen-Yu Tsai]

On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> 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>
> ---
>  drivers/mfd/axp20x.c | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 6ffbc11..47ce233 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -113,6 +113,30 @@ static struct resource axp20x_pek_resources[] = {
>         },
>  };
>
> +static struct resource axp20x_usb_power_supply_resources[] = {
> +       {
> +               .name   = "VBUS_PLUGIN",
> +               .start  = AXP20X_IRQ_VBUS_PLUGIN,
> +               .end    = AXP20X_IRQ_VBUS_PLUGIN,
> +               .flags  = IORESOURCE_IRQ,
> +       }, {
> +               .name   = "VBUS_REMOVAL",
> +               .start  = AXP20X_IRQ_VBUS_REMOVAL,
> +               .end    = AXP20X_IRQ_VBUS_REMOVAL,
> +               .flags  = IORESOURCE_IRQ,
> +       }, {
> +               .name   = "VBUS_VALID",
> +               .start  = AXP20X_IRQ_VBUS_VALID,
> +               .end    = AXP20X_IRQ_VBUS_VALID,
> +               .flags  = IORESOURCE_IRQ,
> +       }, {
> +               .name   = "VBUS_NOT_VALID",
> +               .start  = AXP20X_IRQ_VBUS_NOT_VALID,
> +               .end    = AXP20X_IRQ_VBUS_NOT_VALID,
> +               .flags  = IORESOURCE_IRQ,
> +       },
> +};
> +
>  static struct resource axp22x_pek_resources[] = {
>         {
>                 .name   = "PEK_DBR",
> @@ -165,7 +189,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(15),
>         .cache_type     = REGCACHE_RBTREE,
>  };
>
> @@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = {
>                 .resources              = axp20x_pek_resources,
>         }, {
>                 .name                   = "axp20x-regulator",
> +       }, {
> +               .name                   = "axp20x-usb-power-supply",

Could we use either "vbus-power-supply" to match the AXP datasheets,
or "otg-power-supply" which is slightly more obvious to board owners?

Thanks

ChenYu

> +               .of_compatible          = "x-powers,axp202-usb-power-supply",
> +               .num_resources          =
> +                               ARRAY_SIZE(axp20x_usb_power_supply_resources),
> +               .resources              = axp20x_usb_power_supply_resources,
>         },
>  };
>
> --
> 2.3.6
>
> --
> 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.

-- 
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] [PATCH 2/8] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Chen-Yu Tsai]

On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> 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>
> ---
>  drivers/mfd/axp20x.c | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 6ffbc11..47ce233 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -113,6 +113,30 @@ static struct resource axp20x_pek_resources[] = {
>         },
>  };
>
> +static struct resource axp20x_usb_power_supply_resources[] = {
> +       {
> +               .name   = "VBUS_PLUGIN",
> +               .start  = AXP20X_IRQ_VBUS_PLUGIN,
> +               .end    = AXP20X_IRQ_VBUS_PLUGIN,
> +               .flags  = IORESOURCE_IRQ,
> +       }, {
> +               .name   = "VBUS_REMOVAL",
> +               .start  = AXP20X_IRQ_VBUS_REMOVAL,
> +               .end    = AXP20X_IRQ_VBUS_REMOVAL,
> +               .flags  = IORESOURCE_IRQ,
> +       }, {
> +               .name   = "VBUS_VALID",
> +               .start  = AXP20X_IRQ_VBUS_VALID,
> +               .end    = AXP20X_IRQ_VBUS_VALID,
> +               .flags  = IORESOURCE_IRQ,
> +       }, {
> +               .name   = "VBUS_NOT_VALID",
> +               .start  = AXP20X_IRQ_VBUS_NOT_VALID,
> +               .end    = AXP20X_IRQ_VBUS_NOT_VALID,
> +               .flags  = IORESOURCE_IRQ,
> +       },
> +};
> +
>  static struct resource axp22x_pek_resources[] = {
>         {
>                 .name   = "PEK_DBR",
> @@ -165,7 +189,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(15),
>         .cache_type     = REGCACHE_RBTREE,
>  };
>
> @@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = {
>                 .resources              = axp20x_pek_resources,
>         }, {
>                 .name                   = "axp20x-regulator",
> +       }, {
> +               .name                   = "axp20x-usb-power-supply",

Could we use either "vbus-power-supply" to match the AXP datasheets,
or "otg-power-supply" which is slightly more obvious to board owners?

Thanks

ChenYu

> +               .of_compatible          = "x-powers,axp202-usb-power-supply",
> +               .num_resources          =
> +                               ARRAY_SIZE(axp20x_usb_power_supply_resources),
> +               .resources              = axp20x_usb_power_supply_resources,
>         },
>  };
>
> --
> 2.3.6
>
> --
> 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 at googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 0/8] mfd/power/phy: Add support for otg vbus detection via axp pmic

[Priit Laes]

On Tue, 2015-06-09 at 23:37 +0200, Hans de Goede wrote:
> Hi All,
> 
> Here is a series which adds the beginning of power-supply support to
> the axp20x pmic code. My primary reason for working on this is to
> enable the use of the usb power-supply bits in the pmic to for vbus
> detection on boards which do not have a vbus-det gpio, and instead
> rely on the pmic for vbus detection.
> 
> After I had written most of the vbus power-supply driver code I
> became aware of Bruno Prémont's (in the CC) previous work on this
> our drivers are mostly the same, and I've borrowed some code from
> his driver to add support for min-volt / max-curr properties.

This is one of the reasons why we try to keep our Mainlining Effort
page up-to-date:
http://linux-sunxi.org/Linux_mainlining_effort


> 
> The big difference between our 2 drivers is that mine driver uses
> a devicetree child node / mfd cell per power-supply, so one for
> each of the usb-power, ac-power / battery-charger and 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.
> 
> Regards,
> 
> Hans
> 

-- 
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] [PATCH 0/8] mfd/power/phy: Add support for otg vbus detection via axp pmic

[Priit Laes]

On Tue, 2015-06-09 at 23:37 +0200, Hans de Goede wrote:
> Hi All,
> 
> Here is a series which adds the beginning of power-supply support to
> the axp20x pmic code. My primary reason for working on this is to
> enable the use of the usb power-supply bits in the pmic to for vbus
> detection on boards which do not have a vbus-det gpio, and instead
> rely on the pmic for vbus detection.
> 
> After I had written most of the vbus power-supply driver code I
> became aware of Bruno Pr?mont's (in the CC) previous work on this
> our drivers are mostly the same, and I've borrowed some code from
> his driver to add support for min-volt / max-curr properties.

This is one of the reasons why we try to keep our Mainlining Effort
page up-to-date:
http://linux-sunxi.org/Linux_mainlining_effort


> 
> The big difference between our 2 drivers is that mine driver uses
> a devicetree child node / mfd cell per power-supply, so one for
> each of the usb-power, ac-power / battery-charger and 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.
> 
> Regards,
> 
> Hans
> 

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

[Lee Jones]

On Tue, 09 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>
> ---
>  .../bindings/power_supply/axp20x_usb_power.txt     |  34 +++

This needs to be submitted in a seperate patch.

>  drivers/power/Kconfig                              |   7 +
>  drivers/power/Makefile                             |   1 +
>  drivers/power/axp20x_usb_power.c                   | 241 +++++++++++++++++++++
>  include/linux/mfd/axp20x.h                         |  24 ++
>  5 files changed, 307 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt
>  create mode 100644 drivers/power/axp20x_usb_power.c

[...]

> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index f4290ae..d7adb2e 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,
> @@ -366,4 +368,26 @@ struct axp20x_fg_pdata {
>  	int thermistor_curve[MAX_THERM_CURVE_SIZE][2];
>  };
>  
> +/* generic helper function for reading 9-16 bit wide regs */
> +static inline int axp20x_read_16bit(struct regmap *regmap,

This is only used twice and only in one file.

Do you know of any other uses of this call that will be upstreamed
shortly?

> +				    unsigned int reg, unsigned int width)

The function name is a bit misleading.

> +{
> +	unsigned int v, raw;
> +	int r;

'v' and 'r' are not good variable names.

> +	r = regmap_read(regmap, reg, &v);
> +	if (r)
> +		return r;
> +
> +	raw = v << (width - 8);

So 'reg' is expected to be top end loaded?

E.g A value of say 0x0101 (257) in 9 bits will look like this:

reg          reg + 1
1000 0000b   0000 0001b

> +	r = regmap_read(regmap, reg + 1, &v);
> +	if (r)
> +		return r;
> +
> +	raw |= v;
> +
> +	return raw;
> +}
> +
>  #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

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

[Lee Jones]

On Tue, 09 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>
> ---
>  .../bindings/power_supply/axp20x_usb_power.txt     |  34 +++

This needs to be submitted in a seperate patch.

>  drivers/power/Kconfig                              |   7 +
>  drivers/power/Makefile                             |   1 +
>  drivers/power/axp20x_usb_power.c                   | 241 +++++++++++++++++++++
>  include/linux/mfd/axp20x.h                         |  24 ++
>  5 files changed, 307 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt
>  create mode 100644 drivers/power/axp20x_usb_power.c

[...]

> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index f4290ae..d7adb2e 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,
> @@ -366,4 +368,26 @@ struct axp20x_fg_pdata {
>  	int thermistor_curve[MAX_THERM_CURVE_SIZE][2];
>  };
>  
> +/* generic helper function for reading 9-16 bit wide regs */
> +static inline int axp20x_read_16bit(struct regmap *regmap,

This is only used twice and only in one file.

Do you know of any other uses of this call that will be upstreamed
shortly?

> +				    unsigned int reg, unsigned int width)

The function name is a bit misleading.

> +{
> +	unsigned int v, raw;
> +	int r;

'v' and 'r' are not good variable names.

> +	r = regmap_read(regmap, reg, &v);
> +	if (r)
> +		return r;
> +
> +	raw = v << (width - 8);

So 'reg' is expected to be top end loaded?

E.g A value of say 0x0101 (257) in 9 bits will look like this:

reg          reg + 1
1000 0000b   0000 0001b

> +	r = regmap_read(regmap, reg + 1, &v);
> +	if (r)
> +		return r;
> +
> +	raw |= v;
> +
> +	return raw;
> +}
> +
>  #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: [PATCH 2/8] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Lee Jones]

On Tue, 09 Jun 2015, Hans de Goede wrote:

> 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>
> ---
>  drivers/mfd/axp20x.c | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 6ffbc11..47ce233 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -113,6 +113,30 @@ static struct resource axp20x_pek_resources[] = {
>  	},
>  };
>  
> +static struct resource axp20x_usb_power_supply_resources[] = {
> +	{
> +		.name	= "VBUS_PLUGIN",
> +		.start	= AXP20X_IRQ_VBUS_PLUGIN,
> +		.end	= AXP20X_IRQ_VBUS_PLUGIN,
> +		.flags	= IORESOURCE_IRQ,
> +	}, {
> +		.name	= "VBUS_REMOVAL",
> +		.start	= AXP20X_IRQ_VBUS_REMOVAL,
> +		.end	= AXP20X_IRQ_VBUS_REMOVAL,
> +		.flags	= IORESOURCE_IRQ,
> +	}, {
> +		.name	= "VBUS_VALID",
> +		.start	= AXP20X_IRQ_VBUS_VALID,
> +		.end	= AXP20X_IRQ_VBUS_VALID,
> +		.flags	= IORESOURCE_IRQ,
> +	}, {
> +		.name	= "VBUS_NOT_VALID",
> +		.start	= AXP20X_IRQ_VBUS_NOT_VALID,
> +		.end	= AXP20X_IRQ_VBUS_NOT_VALID,
> +		.flags	= IORESOURCE_IRQ,
> +	},
> +};

Can you take a look at the DEFINE_RES_* macros in
include/linux/ioport.h please?

>  static struct resource axp22x_pek_resources[] = {
>  	{
>  		.name   = "PEK_DBR",
> @@ -165,7 +189,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(15),
>  	.cache_type	= REGCACHE_RBTREE,
>  };
>  
> @@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = {
>  		.resources		= axp20x_pek_resources,
>  	}, {
>  		.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),

This wrap is only necessary due to the extreme tabbing used to line up
the '='.  Please do something about that instead.

> +		.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 2/8] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Lee Jones]

On Tue, 09 Jun 2015, Hans de Goede wrote:

> 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>
> ---
>  drivers/mfd/axp20x.c | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 6ffbc11..47ce233 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -113,6 +113,30 @@ static struct resource axp20x_pek_resources[] = {
>  	},
>  };
>  
> +static struct resource axp20x_usb_power_supply_resources[] = {
> +	{
> +		.name	= "VBUS_PLUGIN",
> +		.start	= AXP20X_IRQ_VBUS_PLUGIN,
> +		.end	= AXP20X_IRQ_VBUS_PLUGIN,
> +		.flags	= IORESOURCE_IRQ,
> +	}, {
> +		.name	= "VBUS_REMOVAL",
> +		.start	= AXP20X_IRQ_VBUS_REMOVAL,
> +		.end	= AXP20X_IRQ_VBUS_REMOVAL,
> +		.flags	= IORESOURCE_IRQ,
> +	}, {
> +		.name	= "VBUS_VALID",
> +		.start	= AXP20X_IRQ_VBUS_VALID,
> +		.end	= AXP20X_IRQ_VBUS_VALID,
> +		.flags	= IORESOURCE_IRQ,
> +	}, {
> +		.name	= "VBUS_NOT_VALID",
> +		.start	= AXP20X_IRQ_VBUS_NOT_VALID,
> +		.end	= AXP20X_IRQ_VBUS_NOT_VALID,
> +		.flags	= IORESOURCE_IRQ,
> +	},
> +};

Can you take a look at the DEFINE_RES_* macros in
include/linux/ioport.h please?

>  static struct resource axp22x_pek_resources[] = {
>  	{
>  		.name   = "PEK_DBR",
> @@ -165,7 +189,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(15),
>  	.cache_type	= REGCACHE_RBTREE,
>  };
>  
> @@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = {
>  		.resources		= axp20x_pek_resources,
>  	}, {
>  		.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),

This wrap is only necessary due to the extreme tabbing used to line up
the '='.  Please do something about that instead.

> +		.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 2/8] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Lee Jones]

On Tue, 09 Jun 2015, Hans de Goede wrote:

> 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>
> ---
>  drivers/mfd/axp20x.c | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 6ffbc11..47ce233 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -113,6 +113,30 @@ static struct resource axp20x_pek_resources[] = {
>  	},
>  };
>  
> +static struct resource axp20x_usb_power_supply_resources[] = {
> +	{
> +		.name	= "VBUS_PLUGIN",
> +		.start	= AXP20X_IRQ_VBUS_PLUGIN,
> +		.end	= AXP20X_IRQ_VBUS_PLUGIN,
> +		.flags	= IORESOURCE_IRQ,
> +	}, {
> +		.name	= "VBUS_REMOVAL",
> +		.start	= AXP20X_IRQ_VBUS_REMOVAL,
> +		.end	= AXP20X_IRQ_VBUS_REMOVAL,
> +		.flags	= IORESOURCE_IRQ,
> +	}, {
> +		.name	= "VBUS_VALID",
> +		.start	= AXP20X_IRQ_VBUS_VALID,
> +		.end	= AXP20X_IRQ_VBUS_VALID,
> +		.flags	= IORESOURCE_IRQ,
> +	}, {
> +		.name	= "VBUS_NOT_VALID",
> +		.start	= AXP20X_IRQ_VBUS_NOT_VALID,
> +		.end	= AXP20X_IRQ_VBUS_NOT_VALID,
> +		.flags	= IORESOURCE_IRQ,
> +	},
> +};
> +
>  static struct resource axp22x_pek_resources[] = {
>  	{
>  		.name   = "PEK_DBR",
> @@ -165,7 +189,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(15),

Please define 15, as MAX_WHATEVER_REG or something.

>  	.cache_type	= REGCACHE_RBTREE,
>  };
>  
> @@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = {
>  		.resources		= axp20x_pek_resources,
>  	}, {
>  		.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 2/8] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Lee Jones]

On Tue, 09 Jun 2015, Hans de Goede wrote:

> 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>
> ---
>  drivers/mfd/axp20x.c | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 6ffbc11..47ce233 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -113,6 +113,30 @@ static struct resource axp20x_pek_resources[] = {
>  	},
>  };
>  
> +static struct resource axp20x_usb_power_supply_resources[] = {
> +	{
> +		.name	= "VBUS_PLUGIN",
> +		.start	= AXP20X_IRQ_VBUS_PLUGIN,
> +		.end	= AXP20X_IRQ_VBUS_PLUGIN,
> +		.flags	= IORESOURCE_IRQ,
> +	}, {
> +		.name	= "VBUS_REMOVAL",
> +		.start	= AXP20X_IRQ_VBUS_REMOVAL,
> +		.end	= AXP20X_IRQ_VBUS_REMOVAL,
> +		.flags	= IORESOURCE_IRQ,
> +	}, {
> +		.name	= "VBUS_VALID",
> +		.start	= AXP20X_IRQ_VBUS_VALID,
> +		.end	= AXP20X_IRQ_VBUS_VALID,
> +		.flags	= IORESOURCE_IRQ,
> +	}, {
> +		.name	= "VBUS_NOT_VALID",
> +		.start	= AXP20X_IRQ_VBUS_NOT_VALID,
> +		.end	= AXP20X_IRQ_VBUS_NOT_VALID,
> +		.flags	= IORESOURCE_IRQ,
> +	},
> +};
> +
>  static struct resource axp22x_pek_resources[] = {
>  	{
>  		.name   = "PEK_DBR",
> @@ -165,7 +189,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(15),

Please define 15, as MAX_WHATEVER_REG or something.

>  	.cache_type	= REGCACHE_RBTREE,
>  };
>  
> @@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = {
>  		.resources		= axp20x_pek_resources,
>  	}, {
>  		.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 1/8] mfd: axp20x: Add missing registers, and mark more registers volatile

[Lee Jones]

On Tue, 09 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>
> ---
>  drivers/mfd/axp20x.c       | 6 ++++++
>  include/linux/mfd/axp20x.h | 5 +++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 6df9155..6ffbc11 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)),

Define 15 please.

>  };
>  
>  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 = {
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index 95568eb..f4290ae 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -151,6 +151,11 @@ 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))
> +
>  /* 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 1/8] mfd: axp20x: Add missing registers, and mark more registers volatile

[Lee Jones]

On Tue, 09 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>
> ---
>  drivers/mfd/axp20x.c       | 6 ++++++
>  include/linux/mfd/axp20x.h | 5 +++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 6df9155..6ffbc11 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)),

Define 15 please.

>  };
>  
>  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 = {
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index 95568eb..f4290ae 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -151,6 +151,11 @@ 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))
> +
>  /* 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 2/8] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Hans de Goede]

Hi,

On 10-06-15 03:19, Chen-Yu Tsai wrote:
> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> 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>
>> ---
>>   drivers/mfd/axp20x.c | 32 +++++++++++++++++++++++++++++++-
>>   1 file changed, 31 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
>> index 6ffbc11..47ce233 100644
>> --- a/drivers/mfd/axp20x.c
>> +++ b/drivers/mfd/axp20x.c
>> @@ -113,6 +113,30 @@ static struct resource axp20x_pek_resources[] = {
>>          },
>>   };
>>
>> +static struct resource axp20x_usb_power_supply_resources[] = {
>> +       {
>> +               .name   = "VBUS_PLUGIN",
>> +               .start  = AXP20X_IRQ_VBUS_PLUGIN,
>> +               .end    = AXP20X_IRQ_VBUS_PLUGIN,
>> +               .flags  = IORESOURCE_IRQ,
>> +       }, {
>> +               .name   = "VBUS_REMOVAL",
>> +               .start  = AXP20X_IRQ_VBUS_REMOVAL,
>> +               .end    = AXP20X_IRQ_VBUS_REMOVAL,
>> +               .flags  = IORESOURCE_IRQ,
>> +       }, {
>> +               .name   = "VBUS_VALID",
>> +               .start  = AXP20X_IRQ_VBUS_VALID,
>> +               .end    = AXP20X_IRQ_VBUS_VALID,
>> +               .flags  = IORESOURCE_IRQ,
>> +       }, {
>> +               .name   = "VBUS_NOT_VALID",
>> +               .start  = AXP20X_IRQ_VBUS_NOT_VALID,
>> +               .end    = AXP20X_IRQ_VBUS_NOT_VALID,
>> +               .flags  = IORESOURCE_IRQ,
>> +       },
>> +};
>> +
>>   static struct resource axp22x_pek_resources[] = {
>>          {
>>                  .name   = "PEK_DBR",
>> @@ -165,7 +189,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(15),
>>          .cache_type     = REGCACHE_RBTREE,
>>   };
>>
>> @@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = {
>>                  .resources              = axp20x_pek_resources,
>>          }, {
>>                  .name                   = "axp20x-regulator",
>> +       }, {
>> +               .name                   = "axp20x-usb-power-supply",
>
> Could we use either "vbus-power-supply" to match the AXP datasheets,
> or "otg-power-supply" which is slightly more obvious to board owners?

I do not like the vbus name, since it does not indicate which bus
it is, OTOH you are right that is what it is called in the datasheet.

As for using otg, I think that usb is better then.

All in all I believe that the current usb name is best, but if others
disagree I'm open to renaming this.

So anyone else have an opinion on what would be a good name for the
cell and the compatible ?

Regards,

Hans

-- 
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] [PATCH 2/8] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Hans de Goede]

Hi,

On 10-06-15 03:19, Chen-Yu Tsai wrote:
> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> 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>
>> ---
>>   drivers/mfd/axp20x.c | 32 +++++++++++++++++++++++++++++++-
>>   1 file changed, 31 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
>> index 6ffbc11..47ce233 100644
>> --- a/drivers/mfd/axp20x.c
>> +++ b/drivers/mfd/axp20x.c
>> @@ -113,6 +113,30 @@ static struct resource axp20x_pek_resources[] = {
>>          },
>>   };
>>
>> +static struct resource axp20x_usb_power_supply_resources[] = {
>> +       {
>> +               .name   = "VBUS_PLUGIN",
>> +               .start  = AXP20X_IRQ_VBUS_PLUGIN,
>> +               .end    = AXP20X_IRQ_VBUS_PLUGIN,
>> +               .flags  = IORESOURCE_IRQ,
>> +       }, {
>> +               .name   = "VBUS_REMOVAL",
>> +               .start  = AXP20X_IRQ_VBUS_REMOVAL,
>> +               .end    = AXP20X_IRQ_VBUS_REMOVAL,
>> +               .flags  = IORESOURCE_IRQ,
>> +       }, {
>> +               .name   = "VBUS_VALID",
>> +               .start  = AXP20X_IRQ_VBUS_VALID,
>> +               .end    = AXP20X_IRQ_VBUS_VALID,
>> +               .flags  = IORESOURCE_IRQ,
>> +       }, {
>> +               .name   = "VBUS_NOT_VALID",
>> +               .start  = AXP20X_IRQ_VBUS_NOT_VALID,
>> +               .end    = AXP20X_IRQ_VBUS_NOT_VALID,
>> +               .flags  = IORESOURCE_IRQ,
>> +       },
>> +};
>> +
>>   static struct resource axp22x_pek_resources[] = {
>>          {
>>                  .name   = "PEK_DBR",
>> @@ -165,7 +189,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(15),
>>          .cache_type     = REGCACHE_RBTREE,
>>   };
>>
>> @@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = {
>>                  .resources              = axp20x_pek_resources,
>>          }, {
>>                  .name                   = "axp20x-regulator",
>> +       }, {
>> +               .name                   = "axp20x-usb-power-supply",
>
> Could we use either "vbus-power-supply" to match the AXP datasheets,
> or "otg-power-supply" which is slightly more obvious to board owners?

I do not like the vbus name, since it does not indicate which bus
it is, OTOH you are right that is what it is called in the datasheet.

As for using otg, I think that usb is better then.

All in all I believe that the current usb name is best, but if others
disagree I'm open to renaming this.

So anyone else have an opinion on what would be a good name for the
cell and the compatible ?

Regards,

Hans

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

[Hans de Goede]

Hi,

Thanks for the quick review I'll do a v2 addressing your concerns soonish.

On 10-06-15 09:29, Lee Jones wrote:
> On Tue, 09 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>
>> ---
>>   .../bindings/power_supply/axp20x_usb_power.txt     |  34 +++
>
> This needs to be submitted in a seperate patch.

Ok.

>
>>   drivers/power/Kconfig                              |   7 +
>>   drivers/power/Makefile                             |   1 +
>>   drivers/power/axp20x_usb_power.c                   | 241 +++++++++++++++++++++
>>   include/linux/mfd/axp20x.h                         |  24 ++
>>   5 files changed, 307 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt
>>   create mode 100644 drivers/power/axp20x_usb_power.c
>
> [...]
>
>> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
>> index f4290ae..d7adb2e 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,
>> @@ -366,4 +368,26 @@ struct axp20x_fg_pdata {
>>   	int thermistor_curve[MAX_THERM_CURVE_SIZE][2];
>>   };
>>
>> +/* generic helper function for reading 9-16 bit wide regs */
>> +static inline int axp20x_read_16bit(struct regmap *regmap,
>
> This is only used twice and only in one file.
>
> Do you know of any other uses of this call that will be upstreamed
> shortly?

Yes I plan to write drivers for the other 3 power_supply class
cells (ac-power, battery-charger, rtc-bat-charger) in the axp209,
and most of those need the same helper which I why I put it here.

>
>> +				    unsigned int reg, unsigned int width)
>
> The function name is a bit misleading.

How about: axp20x_read_multibyte_reg ?

>
>> +{
>> +	unsigned int v, raw;
>> +	int r;
>
> 'v' and 'r' are not good variable names.
>
>> +	r = regmap_read(regmap, reg, &v);
>> +	if (r)
>> +		return r;
>> +
>> +	raw = v << (width - 8);
>
> So 'reg' is expected to be top end loaded?
>
> E.g A value of say 0x0101 (257) in 9 bits will look like this:
>
> reg          reg + 1
> 1000 0000b   0000 0001b

Yes, I guess this was done so that you can get all the 8 msb-s
in a single read if you do not care about the lsb-s.

>
>> +	r = regmap_read(regmap, reg + 1, &v);
>> +	if (r)
>> +		return r;
>> +
>> +	raw |= v;
>> +
>> +	return raw;
>> +}
>> +
>>   #endif /* __LINUX_MFD_AXP20X_H */
>

Regards,

Hans

-- 
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 4/8] power: Add an axp20x-usb-power driver

[Hans de Goede]

Hi,

Thanks for the quick review I'll do a v2 addressing your concerns soonish.

On 10-06-15 09:29, Lee Jones wrote:
> On Tue, 09 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>
>> ---
>>   .../bindings/power_supply/axp20x_usb_power.txt     |  34 +++
>
> This needs to be submitted in a seperate patch.

Ok.

>
>>   drivers/power/Kconfig                              |   7 +
>>   drivers/power/Makefile                             |   1 +
>>   drivers/power/axp20x_usb_power.c                   | 241 +++++++++++++++++++++
>>   include/linux/mfd/axp20x.h                         |  24 ++
>>   5 files changed, 307 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt
>>   create mode 100644 drivers/power/axp20x_usb_power.c
>
> [...]
>
>> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
>> index f4290ae..d7adb2e 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,
>> @@ -366,4 +368,26 @@ struct axp20x_fg_pdata {
>>   	int thermistor_curve[MAX_THERM_CURVE_SIZE][2];
>>   };
>>
>> +/* generic helper function for reading 9-16 bit wide regs */
>> +static inline int axp20x_read_16bit(struct regmap *regmap,
>
> This is only used twice and only in one file.
>
> Do you know of any other uses of this call that will be upstreamed
> shortly?

Yes I plan to write drivers for the other 3 power_supply class
cells (ac-power, battery-charger, rtc-bat-charger) in the axp209,
and most of those need the same helper which I why I put it here.

>
>> +				    unsigned int reg, unsigned int width)
>
> The function name is a bit misleading.

How about: axp20x_read_multibyte_reg ?

>
>> +{
>> +	unsigned int v, raw;
>> +	int r;
>
> 'v' and 'r' are not good variable names.
>
>> +	r = regmap_read(regmap, reg, &v);
>> +	if (r)
>> +		return r;
>> +
>> +	raw = v << (width - 8);
>
> So 'reg' is expected to be top end loaded?
>
> E.g A value of say 0x0101 (257) in 9 bits will look like this:
>
> reg          reg + 1
> 1000 0000b   0000 0001b

Yes, I guess this was done so that you can get all the 8 msb-s
in a single read if you do not care about the lsb-s.

>
>> +	r = regmap_read(regmap, reg + 1, &v);
>> +	if (r)
>> +		return r;
>> +
>> +	raw |= v;
>> +
>> +	return raw;
>> +}
>> +
>>   #endif /* __LINUX_MFD_AXP20X_H */
>

Regards,

Hans

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

[Lee Jones]

On Wed, 10 Jun 2015, Hans de Goede wrote:

> Hi,
> 
> Thanks for the quick review I'll do a v2 addressing your concerns soonish.
> 
> On 10-06-15 09:29, Lee Jones wrote:
> >On Tue, 09 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>
> >>---
> >>  .../bindings/power_supply/axp20x_usb_power.txt     |  34 +++
> >
> >This needs to be submitted in a seperate patch.
> 
> Ok.
> 
> >
> >>  drivers/power/Kconfig                              |   7 +
> >>  drivers/power/Makefile                             |   1 +
> >>  drivers/power/axp20x_usb_power.c                   | 241 +++++++++++++++++++++
> >>  include/linux/mfd/axp20x.h                         |  24 ++
> >>  5 files changed, 307 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt
> >>  create mode 100644 drivers/power/axp20x_usb_power.c
> >
> >[...]
> >
> >>diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> >>index f4290ae..d7adb2e 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,
> >>@@ -366,4 +368,26 @@ struct axp20x_fg_pdata {
> >>  	int thermistor_curve[MAX_THERM_CURVE_SIZE][2];
> >>  };
> >>
> >>+/* generic helper function for reading 9-16 bit wide regs */
> >>+static inline int axp20x_read_16bit(struct regmap *regmap,
> >
> >This is only used twice and only in one file.
> >
> >Do you know of any other uses of this call that will be upstreamed
> >shortly?
> 
> Yes I plan to write drivers for the other 3 power_supply class
> cells (ac-power, battery-charger, rtc-bat-charger) in the axp209,
> and most of those need the same helper which I why I put it here.

Okay.

> >>+				    unsigned int reg, unsigned int width)
> >
> >The function name is a bit misleading.
> 
> How about: axp20x_read_multibyte_reg ?

axp20x_read_variable_width() ?

> >>+{
> >>+	unsigned int v, raw;
> >>+	int r;
> >
> >'v' and 'r' are not good variable names.
> >
> >>+	r = regmap_read(regmap, reg, &v);
> >>+	if (r)
> >>+		return r;
> >>+
> >>+	raw = v << (width - 8);
> >
> >So 'reg' is expected to be top end loaded?
> >
> >E.g A value of say 0x0101 (257) in 9 bits will look like this:
> >
> >reg          reg + 1
> >1000 0000b   0000 0001b
> 
> Yes, I guess this was done so that you can get all the 8 msb-s
> in a single read if you do not care about the lsb-s.

Odd, but okay.

> >>+	r = regmap_read(regmap, reg + 1, &v);
> >>+	if (r)
> >>+		return r;
> >>+
> >>+	raw |= v;
> >>+
> >>+	return raw;
> >>+}
> >>+
> >>  #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 4/8] power: Add an axp20x-usb-power driver

[Lee Jones]

On Wed, 10 Jun 2015, Hans de Goede wrote:

> Hi,
> 
> Thanks for the quick review I'll do a v2 addressing your concerns soonish.
> 
> On 10-06-15 09:29, Lee Jones wrote:
> >On Tue, 09 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>
> >>---
> >>  .../bindings/power_supply/axp20x_usb_power.txt     |  34 +++
> >
> >This needs to be submitted in a seperate patch.
> 
> Ok.
> 
> >
> >>  drivers/power/Kconfig                              |   7 +
> >>  drivers/power/Makefile                             |   1 +
> >>  drivers/power/axp20x_usb_power.c                   | 241 +++++++++++++++++++++
> >>  include/linux/mfd/axp20x.h                         |  24 ++
> >>  5 files changed, 307 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt
> >>  create mode 100644 drivers/power/axp20x_usb_power.c
> >
> >[...]
> >
> >>diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> >>index f4290ae..d7adb2e 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,
> >>@@ -366,4 +368,26 @@ struct axp20x_fg_pdata {
> >>  	int thermistor_curve[MAX_THERM_CURVE_SIZE][2];
> >>  };
> >>
> >>+/* generic helper function for reading 9-16 bit wide regs */
> >>+static inline int axp20x_read_16bit(struct regmap *regmap,
> >
> >This is only used twice and only in one file.
> >
> >Do you know of any other uses of this call that will be upstreamed
> >shortly?
> 
> Yes I plan to write drivers for the other 3 power_supply class
> cells (ac-power, battery-charger, rtc-bat-charger) in the axp209,
> and most of those need the same helper which I why I put it here.

Okay.

> >>+				    unsigned int reg, unsigned int width)
> >
> >The function name is a bit misleading.
> 
> How about: axp20x_read_multibyte_reg ?

axp20x_read_variable_width() ?

> >>+{
> >>+	unsigned int v, raw;
> >>+	int r;
> >
> >'v' and 'r' are not good variable names.
> >
> >>+	r = regmap_read(regmap, reg, &v);
> >>+	if (r)
> >>+		return r;
> >>+
> >>+	raw = v << (width - 8);
> >
> >So 'reg' is expected to be top end loaded?
> >
> >E.g A value of say 0x0101 (257) in 9 bits will look like this:
> >
> >reg          reg + 1
> >1000 0000b   0000 0001b
> 
> Yes, I guess this was done so that you can get all the 8 msb-s
> in a single read if you do not care about the lsb-s.

Odd, but okay.

> >>+	r = regmap_read(regmap, reg + 1, &v);
> >>+	if (r)
> >>+		return r;
> >>+
> >>+	raw |= v;
> >>+
> >>+	return raw;
> >>+}
> >>+
> >>  #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: [PATCH 3/8] power: Add devm_power_supply_get_by_phandle() helper function

[Sebastian Reichel]

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

Hi,

On Tue, Jun 09, 2015 at 11:37:56PM +0200, Hans de Goede wrote:
> This commit adds a resource-managed version of the
> power_supply_get_by_phandle() function.

Thanks, queued.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 3/8] power: Add devm_power_supply_get_by_phandle() helper function

[Sebastian Reichel]

Hi,

On Tue, Jun 09, 2015 at 11:37:56PM +0200, Hans de Goede wrote:
> This commit adds a resource-managed version of the
> power_supply_get_by_phandle() function.

Thanks, queued.

-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150610/66ec6051/attachment.sig>

Re: [PATCH 2/8] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Maxime Ripard]

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

On Wed, Jun 10, 2015 at 09:57:13AM +0200, Hans de Goede wrote:
> >>@@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = {
> >>                 .resources              = axp20x_pek_resources,
> >>         }, {
> >>                 .name                   = "axp20x-regulator",
> >>+       }, {
> >>+               .name                   = "axp20x-usb-power-supply",
> >
> >Could we use either "vbus-power-supply" to match the AXP datasheets,
> >or "otg-power-supply" which is slightly more obvious to board owners?
> 
> I do not like the vbus name, since it does not indicate which bus
> it is, OTOH you are right that is what it is called in the datasheet.
> 
> As for using otg, I think that usb is better then.
> 
> All in all I believe that the current usb name is best, but if others
> disagree I'm open to renaming this.
> 
> So anyone else have an opinion on what would be a good name for the
> cell and the compatible ?

I usually prefer to use the name mentionned in the datasheet, but if
that doesn't make sense, feel free to use an alternative like this
one.

Maxime

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

[linux-sunxi] [PATCH 2/8] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Maxime Ripard]

On Wed, Jun 10, 2015 at 09:57:13AM +0200, Hans de Goede wrote:
> >>@@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = {
> >>                 .resources              = axp20x_pek_resources,
> >>         }, {
> >>                 .name                   = "axp20x-regulator",
> >>+       }, {
> >>+               .name                   = "axp20x-usb-power-supply",
> >
> >Could we use either "vbus-power-supply" to match the AXP datasheets,
> >or "otg-power-supply" which is slightly more obvious to board owners?
> 
> I do not like the vbus name, since it does not indicate which bus
> it is, OTOH you are right that is what it is called in the datasheet.
> 
> As for using otg, I think that usb is better then.
> 
> All in all I believe that the current usb name is best, but if others
> disagree I'm open to renaming this.
> 
> So anyone else have an opinion on what would be a good name for the
> cell and the compatible ?

I usually prefer to use the name mentionned in the datasheet, but if
that doesn't make sense, feel free to use an alternative like this
one.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150613/3324ad71/attachment.sig>

Re: [PATCH 2/8] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Michal Suchanek]

Hello,

On 13 June 2015 at 15:50, Maxime Ripard
<maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> On Wed, Jun 10, 2015 at 09:57:13AM +0200, Hans de Goede wrote:
>> >>@@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = {
>> >>                 .resources              = axp20x_pek_resources,
>> >>         }, {
>> >>                 .name                   = "axp20x-regulator",
>> >>+       }, {
>> >>+               .name                   = "axp20x-usb-power-supply",
>> >
>> >Could we use either "vbus-power-supply" to match the AXP datasheets,
>> >or "otg-power-supply" which is slightly more obvious to board owners?
>>
>> I do not like the vbus name, since it does not indicate which bus
>> it is, OTOH you are right that is what it is called in the datasheet.
>>
>> As for using otg, I think that usb is better then.
>>
>> All in all I believe that the current usb name is best, but if others
>> disagree I'm open to renaming this.
>>
>> So anyone else have an opinion on what would be a good name for the
>> cell and the compatible ?
>
> I usually prefer to use the name mentionned in the datasheet, but if
> that doesn't make sense, feel free to use an alternative like this
> one.

The power source is known as USB power since the reference Allwinner
design connects this pin to the USB OTG connector and all known boards
that use the AXP copy the design.

If somebody gets creative with the vbus input or there is another
manufacturer using AXP with different design it might get problematic
naming it usb.

However, either name is pretty much arbitrary and there is no need for
the vbus to be connected to a bus connector. You could design a board
with two plain power jacks and the AXP would switch between those
according to its priority. If this priority is not configurable then I
think it would be most descriptive to name the inputs as primary and
secondary AC.

The most important part is adding proper documentation so whatever the
name the user can understand that this is the part that is called vbus
in datasheed and is typically wired to the usb otg port.

I only found very terse doc patch.
It adds a new file without a reference in the main axp20x file. IMHO
there should be a reference for the sub-nodes or they should be all in
one file. Spreading the text into multitude of files that contain 2
lines of explanation and an example seems counter-productive. The
examples can be combined in one.

Another nit is that the doc shows axp202-usb-power-supply compatible
while the usb power is available on all AXP regulators for which there
are drivers since axp152. IMHO this can be just
x-powers,usb-power-supply since it can only appear as subnode of the
axp, anyway.

Thanks

Michal

[linux-sunxi] [PATCH 2/8] mfd: axp20x: Add a cell for the usb power_supply part of the axp20x PMICs

[Michal Suchanek]

Hello,

On 13 June 2015 at 15:50, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Wed, Jun 10, 2015 at 09:57:13AM +0200, Hans de Goede wrote:
>> >>@@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = {
>> >>                 .resources              = axp20x_pek_resources,
>> >>         }, {
>> >>                 .name                   = "axp20x-regulator",
>> >>+       }, {
>> >>+               .name                   = "axp20x-usb-power-supply",
>> >
>> >Could we use either "vbus-power-supply" to match the AXP datasheets,
>> >or "otg-power-supply" which is slightly more obvious to board owners?
>>
>> I do not like the vbus name, since it does not indicate which bus
>> it is, OTOH you are right that is what it is called in the datasheet.
>>
>> As for using otg, I think that usb is better then.
>>
>> All in all I believe that the current usb name is best, but if others
>> disagree I'm open to renaming this.
>>
>> So anyone else have an opinion on what would be a good name for the
>> cell and the compatible ?
>
> I usually prefer to use the name mentionned in the datasheet, but if
> that doesn't make sense, feel free to use an alternative like this
> one.

The power source is known as USB power since the reference Allwinner
design connects this pin to the USB OTG connector and all known boards
that use the AXP copy the design.

If somebody gets creative with the vbus input or there is another
manufacturer using AXP with different design it might get problematic
naming it usb.

However, either name is pretty much arbitrary and there is no need for
the vbus to be connected to a bus connector. You could design a board
with two plain power jacks and the AXP would switch between those
according to its priority. If this priority is not configurable then I
think it would be most descriptive to name the inputs as primary and
secondary AC.

The most important part is adding proper documentation so whatever the
name the user can understand that this is the part that is called vbus
in datasheed and is typically wired to the usb otg port.

I only found very terse doc patch.
It adds a new file without a reference in the main axp20x file. IMHO
there should be a reference for the sub-nodes or they should be all in
one file. Spreading the text into multitude of files that contain 2
lines of explanation and an example seems counter-productive. The
examples can be combined in one.

Another nit is that the doc shows axp202-usb-power-supply compatible
while the usb power is available on all AXP regulators for which there
are drivers since axp152. IMHO this can be just
x-powers,usb-power-supply since it can only appear as subnode of the
axp, anyway.

Thanks

Michal

Re: [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Chen-Yu Tsai]

Hi Hans,

On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Add a node representing the usb power supply part of the axp209 pmic, note
> that the usb power supply and the (to be added later) ac power supply will
> each have their own child-node, so that they can be separately specified
> as power-supply for other nodes using a power-supply property with a
> phandle pointing to the right axp209 child-node.
>
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  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 24c935c..051ab3b 100644
> --- a/arch/arm/boot/dts/axp209.dtsi
> +++ b/arch/arm/boot/dts/axp209.dtsi
> @@ -89,4 +89,9 @@
>                         regulator-name = "ldo5";
>                 };
>         };
> +
> +       usb_power_supply: usb_power_supply {
> +               compatible = "x-powers,axp202-usb-power-supply";
> +               status = "disabled";

Is there any reason to have this disabled by default?
AFAIK this is not something configurable in hardware,
and the driver just gives readouts and status updates.

ChenYu

> +       };
>  };
> --
> 2.3.6
>
> --
> 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] [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Chen-Yu Tsai]

Hi Hans,

On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Add a node representing the usb power supply part of the axp209 pmic, note
> that the usb power supply and the (to be added later) ac power supply will
> each have their own child-node, so that they can be separately specified
> as power-supply for other nodes using a power-supply property with a
> phandle pointing to the right axp209 child-node.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  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 24c935c..051ab3b 100644
> --- a/arch/arm/boot/dts/axp209.dtsi
> +++ b/arch/arm/boot/dts/axp209.dtsi
> @@ -89,4 +89,9 @@
>                         regulator-name = "ldo5";
>                 };
>         };
> +
> +       usb_power_supply: usb_power_supply {
> +               compatible = "x-powers,axp202-usb-power-supply";
> +               status = "disabled";

Is there any reason to have this disabled by default?
AFAIK this is not something configurable in hardware,
and the driver just gives readouts and status updates.

ChenYu

> +       };
>  };
> --
> 2.3.6
>
> --
> 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 at googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Bruno Prémont]

Hi ChenYu,

On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
> Hi Hans,
> 
> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > Add a node representing the usb power supply part of the axp209 pmic, note
> > that the usb power supply and the (to be added later) ac power supply will
> > each have their own child-node, so that they can be separately specified
> > as power-supply for other nodes using a power-supply property with a
> > phandle pointing to the right axp209 child-node.
> >
> > Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> >  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 24c935c..051ab3b 100644
> > --- a/arch/arm/boot/dts/axp209.dtsi
> > +++ b/arch/arm/boot/dts/axp209.dtsi
> > @@ -89,4 +89,9 @@
> >                         regulator-name = "ldo5";
> >                 };
> >         };
> > +
> > +       usb_power_supply: usb_power_supply {
> > +               compatible = "x-powers,axp202-usb-power-supply";
> > +               status = "disabled";
> 
> Is there any reason to have this disabled by default?
> AFAIK this is not something configurable in hardware,
> and the driver just gives readouts and status updates.

There are some devices that only have 'AC-IN' instead of usb/otg power
(set-top boxes) so having the devices tell what power sources they
have makes sense (otherwise those that have ac-in instead of otg would
have to disable usb_power_supply) and avoids confusion by explicitly
stating presence in device dts.

NB: I should hopefully get my battery supply driver ported this WE,
ac-in/rtc are done, this WE. I'm going to default-off for them as well
(note that for some extra details should be provided in dts).

Bruno

> ChenYu
> 
> > +       };
> >  };
> > --
> > 2.3.6

[linux-sunxi] [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Bruno Prémont]

Hi ChenYu,

On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
> Hi Hans,
> 
> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> > Add a node representing the usb power supply part of the axp209 pmic, note
> > that the usb power supply and the (to be added later) ac power supply will
> > each have their own child-node, so that they can be separately specified
> > as power-supply for other nodes using a power-supply property with a
> > phandle pointing to the right axp209 child-node.
> >
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > ---
> >  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 24c935c..051ab3b 100644
> > --- a/arch/arm/boot/dts/axp209.dtsi
> > +++ b/arch/arm/boot/dts/axp209.dtsi
> > @@ -89,4 +89,9 @@
> >                         regulator-name = "ldo5";
> >                 };
> >         };
> > +
> > +       usb_power_supply: usb_power_supply {
> > +               compatible = "x-powers,axp202-usb-power-supply";
> > +               status = "disabled";
> 
> Is there any reason to have this disabled by default?
> AFAIK this is not something configurable in hardware,
> and the driver just gives readouts and status updates.

There are some devices that only have 'AC-IN' instead of usb/otg power
(set-top boxes) so having the devices tell what power sources they
have makes sense (otherwise those that have ac-in instead of otg would
have to disable usb_power_supply) and avoids confusion by explicitly
stating presence in device dts.

NB: I should hopefully get my battery supply driver ported this WE,
ac-in/rtc are done, this WE. I'm going to default-off for them as well
(note that for some extra details should be provided in dts).

Bruno

> ChenYu
> 
> > +       };
> >  };
> > --
> > 2.3.6

Re: [linux-sunxi] [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Chen-Yu Tsai]

On Fri, Jul 31, 2015 at 1:51 PM, Bruno Prémont
<bonbons@linux-vserver.org> wrote:
> Hi ChenYu,
>
> On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
>> Hi Hans,
>>
>> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> > Add a node representing the usb power supply part of the axp209 pmic, note
>> > that the usb power supply and the (to be added later) ac power supply will
>> > each have their own child-node, so that they can be separately specified
>> > as power-supply for other nodes using a power-supply property with a
>> > phandle pointing to the right axp209 child-node.
>> >
>> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> > ---
>> >  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 24c935c..051ab3b 100644
>> > --- a/arch/arm/boot/dts/axp209.dtsi
>> > +++ b/arch/arm/boot/dts/axp209.dtsi
>> > @@ -89,4 +89,9 @@
>> >                         regulator-name = "ldo5";
>> >                 };
>> >         };
>> > +
>> > +       usb_power_supply: usb_power_supply {
>> > +               compatible = "x-powers,axp202-usb-power-supply";
>> > +               status = "disabled";
>>
>> Is there any reason to have this disabled by default?
>> AFAIK this is not something configurable in hardware,
>> and the driver just gives readouts and status updates.
>
> There are some devices that only have 'AC-IN' instead of usb/otg power
> (set-top boxes) so having the devices tell what power sources they
> have makes sense (otherwise those that have ac-in instead of otg would
> have to disable usb_power_supply) and avoids confusion by explicitly
> stating presence in device dts.

That's true. But this is a bit of hardware inside the AXP.
If it's power isn't routed, it should report it, shouldn't it?

And normal users would probably expect to read the input
voltage/current/status through sysfs (or maybe hwmon?),
instead of not finding the device and having to look through
the DTS.

If you're using a distro, you might not have the kernel sources
around, and digging through the in-system device tree isn't that
straightforward if you don't know where to look.

> NB: I should hopefully get my battery supply driver ported this WE,
> ac-in/rtc are done, this WE. I'm going to default-off for them as well
> (note that for some extra details should be provided in dts).

I'm guessing we won't be using the axp288 one?

Thanks
ChenYu

> Bruno
>
>> ChenYu
>>
>> > +       };
>> >  };
>> > --
>> > 2.3.6

[linux-sunxi] [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Chen-Yu Tsai]

On Fri, Jul 31, 2015 at 1:51 PM, Bruno Pr?mont
<bonbons@linux-vserver.org> wrote:
> Hi ChenYu,
>
> On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
>> Hi Hans,
>>
>> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> > Add a node representing the usb power supply part of the axp209 pmic, note
>> > that the usb power supply and the (to be added later) ac power supply will
>> > each have their own child-node, so that they can be separately specified
>> > as power-supply for other nodes using a power-supply property with a
>> > phandle pointing to the right axp209 child-node.
>> >
>> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> > ---
>> >  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 24c935c..051ab3b 100644
>> > --- a/arch/arm/boot/dts/axp209.dtsi
>> > +++ b/arch/arm/boot/dts/axp209.dtsi
>> > @@ -89,4 +89,9 @@
>> >                         regulator-name = "ldo5";
>> >                 };
>> >         };
>> > +
>> > +       usb_power_supply: usb_power_supply {
>> > +               compatible = "x-powers,axp202-usb-power-supply";
>> > +               status = "disabled";
>>
>> Is there any reason to have this disabled by default?
>> AFAIK this is not something configurable in hardware,
>> and the driver just gives readouts and status updates.
>
> There are some devices that only have 'AC-IN' instead of usb/otg power
> (set-top boxes) so having the devices tell what power sources they
> have makes sense (otherwise those that have ac-in instead of otg would
> have to disable usb_power_supply) and avoids confusion by explicitly
> stating presence in device dts.

That's true. But this is a bit of hardware inside the AXP.
If it's power isn't routed, it should report it, shouldn't it?

And normal users would probably expect to read the input
voltage/current/status through sysfs (or maybe hwmon?),
instead of not finding the device and having to look through
the DTS.

If you're using a distro, you might not have the kernel sources
around, and digging through the in-system device tree isn't that
straightforward if you don't know where to look.

> NB: I should hopefully get my battery supply driver ported this WE,
> ac-in/rtc are done, this WE. I'm going to default-off for them as well
> (note that for some extra details should be provided in dts).

I'm guessing we won't be using the axp288 one?

Thanks
ChenYu

> Bruno
>
>> ChenYu
>>
>> > +       };
>> >  };
>> > --
>> > 2.3.6

Re: [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Bruno Prémont]

On Fri, 31 Jul 2015 14:14:28 +0800 Chen-Yu Tsai wrote:
> On Fri, Jul 31, 2015 at 1:51 PM, Bruno Prémont wrote:
> > Hi ChenYu,
> >
> > On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
> >> Hi Hans,
> >>
> >> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede wrote:
> >> > Add a node representing the usb power supply part of the axp209 pmic, note
> >> > that the usb power supply and the (to be added later) ac power supply will
> >> > each have their own child-node, so that they can be separately specified
> >> > as power-supply for other nodes using a power-supply property with a
> >> > phandle pointing to the right axp209 child-node.
> >> >
> >> > Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >> > ---
> >> >  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 24c935c..051ab3b 100644
> >> > --- a/arch/arm/boot/dts/axp209.dtsi
> >> > +++ b/arch/arm/boot/dts/axp209.dtsi
> >> > @@ -89,4 +89,9 @@
> >> >                         regulator-name = "ldo5";
> >> >                 };
> >> >         };
> >> > +
> >> > +       usb_power_supply: usb_power_supply {
> >> > +               compatible = "x-powers,axp202-usb-power-supply";
> >> > +               status = "disabled";
> >>
> >> Is there any reason to have this disabled by default?
> >> AFAIK this is not something configurable in hardware,
> >> and the driver just gives readouts and status updates.
> >
> > There are some devices that only have 'AC-IN' instead of usb/otg power
> > (set-top boxes) so having the devices tell what power sources they
> > have makes sense (otherwise those that have ac-in instead of otg would
> > have to disable usb_power_supply) and avoids confusion by explicitly
> > stating presence in device dts.
> 
> That's true. But this is a bit of hardware inside the AXP.
> If it's power isn't routed, it should report it, shouldn't it?

If the device does not use the input there are two cases:
- ac-in and usb/otg in short-circuited (there is a bit in AXP registers
  to provide this indication)
- the unused one is just not connected/connectable

In either case the voltage of the unused/unusable branch is not really
useful, nor is it useful to have it show up in sysfs.

> And normal users would probably expect to read the input
> voltage/current/status through sysfs (or maybe hwmon?),
> instead of not finding the device and having to look through
> the DTS.

Only those supplies effectively present (in-use or with connectors
to eventually use them) should be visible in sysfs.
Having more of them will just confuse the users, they would be searching
how they can connect the supply while there is no means to do so
except soldering to the tiny AXP pins.

hwmon makes more sense for output voltages of the AXP or temperature
of AXP chip itself, not so much for the input side if that side is
covered by power_supply devices (otherwise the information is being
duplicated and if so should only be via power_supply class, not
individual drivers).

> If you're using a distro, you might not have the kernel sources
> around, and digging through the in-system device tree isn't that
> straightforward if you don't know where to look.

If the dts is incomplete that's another issue.

> > NB: I should hopefully get my battery supply driver ported this WE,
> > ac-in/rtc are done, this WE. I'm going to default-off for them as well
> > (note that for some extra details should be provided in dts).
> 
> I'm guessing we won't be using the axp288 one?

In a first step I won't be using it.

There are a few reasons for that:
- axp288 is based on ACPI/platform to provide configuration information
  instead of dts
- it makes extensive use of OCV and registers that are possibly present
  on axp20x but undocumented there.

Bruno

-- 
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] [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Bruno Prémont]

On Fri, 31 Jul 2015 14:14:28 +0800 Chen-Yu Tsai wrote:
> On Fri, Jul 31, 2015 at 1:51 PM, Bruno Pr?mont wrote:
> > Hi ChenYu,
> >
> > On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
> >> Hi Hans,
> >>
> >> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede wrote:
> >> > Add a node representing the usb power supply part of the axp209 pmic, note
> >> > that the usb power supply and the (to be added later) ac power supply will
> >> > each have their own child-node, so that they can be separately specified
> >> > as power-supply for other nodes using a power-supply property with a
> >> > phandle pointing to the right axp209 child-node.
> >> >
> >> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> > ---
> >> >  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 24c935c..051ab3b 100644
> >> > --- a/arch/arm/boot/dts/axp209.dtsi
> >> > +++ b/arch/arm/boot/dts/axp209.dtsi
> >> > @@ -89,4 +89,9 @@
> >> >                         regulator-name = "ldo5";
> >> >                 };
> >> >         };
> >> > +
> >> > +       usb_power_supply: usb_power_supply {
> >> > +               compatible = "x-powers,axp202-usb-power-supply";
> >> > +               status = "disabled";
> >>
> >> Is there any reason to have this disabled by default?
> >> AFAIK this is not something configurable in hardware,
> >> and the driver just gives readouts and status updates.
> >
> > There are some devices that only have 'AC-IN' instead of usb/otg power
> > (set-top boxes) so having the devices tell what power sources they
> > have makes sense (otherwise those that have ac-in instead of otg would
> > have to disable usb_power_supply) and avoids confusion by explicitly
> > stating presence in device dts.
> 
> That's true. But this is a bit of hardware inside the AXP.
> If it's power isn't routed, it should report it, shouldn't it?

If the device does not use the input there are two cases:
- ac-in and usb/otg in short-circuited (there is a bit in AXP registers
  to provide this indication)
- the unused one is just not connected/connectable

In either case the voltage of the unused/unusable branch is not really
useful, nor is it useful to have it show up in sysfs.

> And normal users would probably expect to read the input
> voltage/current/status through sysfs (or maybe hwmon?),
> instead of not finding the device and having to look through
> the DTS.

Only those supplies effectively present (in-use or with connectors
to eventually use them) should be visible in sysfs.
Having more of them will just confuse the users, they would be searching
how they can connect the supply while there is no means to do so
except soldering to the tiny AXP pins.

hwmon makes more sense for output voltages of the AXP or temperature
of AXP chip itself, not so much for the input side if that side is
covered by power_supply devices (otherwise the information is being
duplicated and if so should only be via power_supply class, not
individual drivers).

> If you're using a distro, you might not have the kernel sources
> around, and digging through the in-system device tree isn't that
> straightforward if you don't know where to look.

If the dts is incomplete that's another issue.

> > NB: I should hopefully get my battery supply driver ported this WE,
> > ac-in/rtc are done, this WE. I'm going to default-off for them as well
> > (note that for some extra details should be provided in dts).
> 
> I'm guessing we won't be using the axp288 one?

In a first step I won't be using it.

There are a few reasons for that:
- axp288 is based on ACPI/platform to provide configuration information
  instead of dts
- it makes extensive use of OCV and registers that are possibly present
  on axp20x but undocumented there.

Bruno

Re: [linux-sunxi] [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Maxime Ripard]

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

On Fri, Jul 31, 2015 at 08:35:41AM +0200, Bruno Prémont wrote:
> > > NB: I should hopefully get my battery supply driver ported this WE,
> > > ac-in/rtc are done, this WE. I'm going to default-off for them as well
> > > (note that for some extra details should be provided in dts).
> > 
> > I'm guessing we won't be using the axp288 one?
> 
> In a first step I won't be using it.
> 
> There are a few reasons for that:
> - axp288 is based on ACPI/platform to provide configuration information
>   instead of dts

That's not an issue. You have an API to handle lookup in both DT and
ACPI (the fwnode_* stuff).

> - it makes extensive use of OCV and registers that are possibly present
>   on axp20x but undocumented there.

That will be more problematic. Still I don't see why we wouldn't use a
similar setup to what they did.

Maxime

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[linux-sunxi] [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Maxime Ripard]

On Fri, Jul 31, 2015 at 08:35:41AM +0200, Bruno Pr?mont wrote:
> > > NB: I should hopefully get my battery supply driver ported this WE,
> > > ac-in/rtc are done, this WE. I'm going to default-off for them as well
> > > (note that for some extra details should be provided in dts).
> > 
> > I'm guessing we won't be using the axp288 one?
> 
> In a first step I won't be using it.
> 
> There are a few reasons for that:
> - axp288 is based on ACPI/platform to provide configuration information
>   instead of dts

That's not an issue. You have an API to handle lookup in both DT and
ACPI (the fwnode_* stuff).

> - it makes extensive use of OCV and registers that are possibly present
>   on axp20x but undocumented there.

That will be more problematic. Still I don't see why we wouldn't use a
similar setup to what they did.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150731/796bb5e1/attachment.sig>

Re: [linux-sunxi] [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Hans de Goede]

Hi,

On 31-07-15 08:35, Bruno Prémont wrote:
> On Fri, 31 Jul 2015 14:14:28 +0800 Chen-Yu Tsai wrote:
>> On Fri, Jul 31, 2015 at 1:51 PM, Bruno Prémont wrote:
>>> Hi ChenYu,
>>>
>>> On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
>>>> Hi Hans,
>>>>
>>>> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede wrote:
>>>>> Add a node representing the usb power supply part of the axp209 pmic, note
>>>>> that the usb power supply and the (to be added later) ac power supply will
>>>>> each have their own child-node, so that they can be separately specified
>>>>> as power-supply for other nodes using a power-supply property with a
>>>>> phandle pointing to the right axp209 child-node.
>>>>>
>>>>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>>> ---
>>>>>   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 24c935c..051ab3b 100644
>>>>> --- a/arch/arm/boot/dts/axp209.dtsi
>>>>> +++ b/arch/arm/boot/dts/axp209.dtsi
>>>>> @@ -89,4 +89,9 @@
>>>>>                          regulator-name = "ldo5";
>>>>>                  };
>>>>>          };
>>>>> +
>>>>> +       usb_power_supply: usb_power_supply {
>>>>> +               compatible = "x-powers,axp202-usb-power-supply";
>>>>> +               status = "disabled";
>>>>
>>>> Is there any reason to have this disabled by default?
>>>> AFAIK this is not something configurable in hardware,
>>>> and the driver just gives readouts and status updates.
>>>
>>> There are some devices that only have 'AC-IN' instead of usb/otg power
>>> (set-top boxes) so having the devices tell what power sources they
>>> have makes sense (otherwise those that have ac-in instead of otg would
>>> have to disable usb_power_supply) and avoids confusion by explicitly
>>> stating presence in device dts.
>>
>> That's true. But this is a bit of hardware inside the AXP.
>> If it's power isn't routed, it should report it, shouldn't it?
>
> If the device does not use the input there are two cases:
> - ac-in and usb/otg in short-circuited (there is a bit in AXP registers
>    to provide this indication)
> - the unused one is just not connected/connectable
>
> In either case the voltage of the unused/unusable branch is not really
> useful, nor is it useful to have it show up in sysfs.

Exactly my thinking on this, +1 .

>> And normal users would probably expect to read the input
>> voltage/current/status through sysfs (or maybe hwmon?),
>> instead of not finding the device and having to look through
>> the DTS.
>
> Only those supplies effectively present (in-use or with connectors
> to eventually use them) should be visible in sysfs.
> Having more of them will just confuse the users, they would be searching
> how they can connect the supply while there is no means to do so
> except soldering to the tiny AXP pins.

+1

Regards,

Hans
--
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

[linux-sunxi] [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Hans de Goede]

Hi,

On 31-07-15 08:35, Bruno Pr?mont wrote:
> On Fri, 31 Jul 2015 14:14:28 +0800 Chen-Yu Tsai wrote:
>> On Fri, Jul 31, 2015 at 1:51 PM, Bruno Pr?mont wrote:
>>> Hi ChenYu,
>>>
>>> On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
>>>> Hi Hans,
>>>>
>>>> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede wrote:
>>>>> Add a node representing the usb power supply part of the axp209 pmic, note
>>>>> that the usb power supply and the (to be added later) ac power supply will
>>>>> each have their own child-node, so that they can be separately specified
>>>>> as power-supply for other nodes using a power-supply property with a
>>>>> phandle pointing to the right axp209 child-node.
>>>>>
>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>> ---
>>>>>   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 24c935c..051ab3b 100644
>>>>> --- a/arch/arm/boot/dts/axp209.dtsi
>>>>> +++ b/arch/arm/boot/dts/axp209.dtsi
>>>>> @@ -89,4 +89,9 @@
>>>>>                          regulator-name = "ldo5";
>>>>>                  };
>>>>>          };
>>>>> +
>>>>> +       usb_power_supply: usb_power_supply {
>>>>> +               compatible = "x-powers,axp202-usb-power-supply";
>>>>> +               status = "disabled";
>>>>
>>>> Is there any reason to have this disabled by default?
>>>> AFAIK this is not something configurable in hardware,
>>>> and the driver just gives readouts and status updates.
>>>
>>> There are some devices that only have 'AC-IN' instead of usb/otg power
>>> (set-top boxes) so having the devices tell what power sources they
>>> have makes sense (otherwise those that have ac-in instead of otg would
>>> have to disable usb_power_supply) and avoids confusion by explicitly
>>> stating presence in device dts.
>>
>> That's true. But this is a bit of hardware inside the AXP.
>> If it's power isn't routed, it should report it, shouldn't it?
>
> If the device does not use the input there are two cases:
> - ac-in and usb/otg in short-circuited (there is a bit in AXP registers
>    to provide this indication)
> - the unused one is just not connected/connectable
>
> In either case the voltage of the unused/unusable branch is not really
> useful, nor is it useful to have it show up in sysfs.

Exactly my thinking on this, +1 .

>> And normal users would probably expect to read the input
>> voltage/current/status through sysfs (or maybe hwmon?),
>> instead of not finding the device and having to look through
>> the DTS.
>
> Only those supplies effectively present (in-use or with connectors
> to eventually use them) should be visible in sysfs.
> Having more of them will just confuse the users, they would be searching
> how they can connect the supply while there is no means to do so
> except soldering to the tiny AXP pins.

+1

Regards,

Hans

Re: [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Chen-Yu Tsai]

On Fri, Jul 31, 2015 at 4:31 PM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Hi,
>
>
> On 31-07-15 08:35, Bruno Prémont wrote:
>>
>> On Fri, 31 Jul 2015 14:14:28 +0800 Chen-Yu Tsai wrote:
>>>
>>> On Fri, Jul 31, 2015 at 1:51 PM, Bruno Prémont wrote:
>>>>
>>>> Hi ChenYu,
>>>>
>>>> On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
>>>>>
>>>>> Hi Hans,
>>>>>
>>>>> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede wrote:
>>>>>>
>>>>>> Add a node representing the usb power supply part of the axp209 pmic,
>>>>>> note
>>>>>> that the usb power supply and the (to be added later) ac power supply
>>>>>> will
>>>>>> each have their own child-node, so that they can be separately
>>>>>> specified
>>>>>> as power-supply for other nodes using a power-supply property with a
>>>>>> phandle pointing to the right axp209 child-node.
>>>>>>
>>>>>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>>>> ---
>>>>>>   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 24c935c..051ab3b 100644
>>>>>> --- a/arch/arm/boot/dts/axp209.dtsi
>>>>>> +++ b/arch/arm/boot/dts/axp209.dtsi
>>>>>> @@ -89,4 +89,9 @@
>>>>>>                          regulator-name = "ldo5";
>>>>>>                  };
>>>>>>          };
>>>>>> +
>>>>>> +       usb_power_supply: usb_power_supply {
>>>>>> +               compatible = "x-powers,axp202-usb-power-supply";
>>>>>> +               status = "disabled";
>>>>>
>>>>>
>>>>> Is there any reason to have this disabled by default?
>>>>> AFAIK this is not something configurable in hardware,
>>>>> and the driver just gives readouts and status updates.
>>>>
>>>>
>>>> There are some devices that only have 'AC-IN' instead of usb/otg power
>>>> (set-top boxes) so having the devices tell what power sources they
>>>> have makes sense (otherwise those that have ac-in instead of otg would
>>>> have to disable usb_power_supply) and avoids confusion by explicitly
>>>> stating presence in device dts.
>>>
>>>
>>> That's true. But this is a bit of hardware inside the AXP.
>>> If it's power isn't routed, it should report it, shouldn't it?
>>
>>
>> If the device does not use the input there are two cases:
>> - ac-in and usb/otg in short-circuited (there is a bit in AXP registers
>>    to provide this indication)
>> - the unused one is just not connected/connectable
>>
>> In either case the voltage of the unused/unusable branch is not really
>> useful, nor is it useful to have it show up in sysfs.
>
>
> Exactly my thinking on this, +1 .
>
>>> And normal users would probably expect to read the input
>>> voltage/current/status through sysfs (or maybe hwmon?),
>>> instead of not finding the device and having to look through
>>> the DTS.
>>
>>
>> Only those supplies effectively present (in-use or with connectors
>> to eventually use them) should be visible in sysfs.
>> Having more of them will just confuse the users, they would be searching
>> how they can connect the supply while there is no means to do so
>> except soldering to the tiny AXP pins.
>
>
> +1

OK. So mostly tablets and dev boards would benefit from having both
power supplies. Is there a preference for usb-power-supply over
vbus-det-gpio if both are available? Or just list them both and
let the driver sort it out?

Thanks.


ChenYu

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

[linux-sunxi] [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Chen-Yu Tsai]

On Fri, Jul 31, 2015 at 4:31 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 31-07-15 08:35, Bruno Pr?mont wrote:
>>
>> On Fri, 31 Jul 2015 14:14:28 +0800 Chen-Yu Tsai wrote:
>>>
>>> On Fri, Jul 31, 2015 at 1:51 PM, Bruno Pr?mont wrote:
>>>>
>>>> Hi ChenYu,
>>>>
>>>> On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
>>>>>
>>>>> Hi Hans,
>>>>>
>>>>> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede wrote:
>>>>>>
>>>>>> Add a node representing the usb power supply part of the axp209 pmic,
>>>>>> note
>>>>>> that the usb power supply and the (to be added later) ac power supply
>>>>>> will
>>>>>> each have their own child-node, so that they can be separately
>>>>>> specified
>>>>>> as power-supply for other nodes using a power-supply property with a
>>>>>> phandle pointing to the right axp209 child-node.
>>>>>>
>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>>> ---
>>>>>>   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 24c935c..051ab3b 100644
>>>>>> --- a/arch/arm/boot/dts/axp209.dtsi
>>>>>> +++ b/arch/arm/boot/dts/axp209.dtsi
>>>>>> @@ -89,4 +89,9 @@
>>>>>>                          regulator-name = "ldo5";
>>>>>>                  };
>>>>>>          };
>>>>>> +
>>>>>> +       usb_power_supply: usb_power_supply {
>>>>>> +               compatible = "x-powers,axp202-usb-power-supply";
>>>>>> +               status = "disabled";
>>>>>
>>>>>
>>>>> Is there any reason to have this disabled by default?
>>>>> AFAIK this is not something configurable in hardware,
>>>>> and the driver just gives readouts and status updates.
>>>>
>>>>
>>>> There are some devices that only have 'AC-IN' instead of usb/otg power
>>>> (set-top boxes) so having the devices tell what power sources they
>>>> have makes sense (otherwise those that have ac-in instead of otg would
>>>> have to disable usb_power_supply) and avoids confusion by explicitly
>>>> stating presence in device dts.
>>>
>>>
>>> That's true. But this is a bit of hardware inside the AXP.
>>> If it's power isn't routed, it should report it, shouldn't it?
>>
>>
>> If the device does not use the input there are two cases:
>> - ac-in and usb/otg in short-circuited (there is a bit in AXP registers
>>    to provide this indication)
>> - the unused one is just not connected/connectable
>>
>> In either case the voltage of the unused/unusable branch is not really
>> useful, nor is it useful to have it show up in sysfs.
>
>
> Exactly my thinking on this, +1 .
>
>>> And normal users would probably expect to read the input
>>> voltage/current/status through sysfs (or maybe hwmon?),
>>> instead of not finding the device and having to look through
>>> the DTS.
>>
>>
>> Only those supplies effectively present (in-use or with connectors
>> to eventually use them) should be visible in sysfs.
>> Having more of them will just confuse the users, they would be searching
>> how they can connect the supply while there is no means to do so
>> except soldering to the tiny AXP pins.
>
>
> +1

OK. So mostly tablets and dev boards would benefit from having both
power supplies. Is there a preference for usb-power-supply over
vbus-det-gpio if both are available? Or just list them both and
let the driver sort it out?

Thanks.


ChenYu

Re: [linux-sunxi] [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Hans de Goede]

Hi,

On 31-07-15 11:00, Chen-Yu Tsai wrote:
> On Fri, Jul 31, 2015 at 4:31 PM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> Hi,
>>
>>
>> On 31-07-15 08:35, Bruno Prémont wrote:
>>>
>>> On Fri, 31 Jul 2015 14:14:28 +0800 Chen-Yu Tsai wrote:
>>>>
>>>> On Fri, Jul 31, 2015 at 1:51 PM, Bruno Prémont wrote:
>>>>>
>>>>> Hi ChenYu,
>>>>>
>>>>> On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
>>>>>>
>>>>>> Hi Hans,
>>>>>>
>>>>>> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede wrote:
>>>>>>>
>>>>>>> Add a node representing the usb power supply part of the axp209 pmic,
>>>>>>> note
>>>>>>> that the usb power supply and the (to be added later) ac power supply
>>>>>>> will
>>>>>>> each have their own child-node, so that they can be separately
>>>>>>> specified
>>>>>>> as power-supply for other nodes using a power-supply property with a
>>>>>>> phandle pointing to the right axp209 child-node.
>>>>>>>
>>>>>>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>>>>> ---
>>>>>>>    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 24c935c..051ab3b 100644
>>>>>>> --- a/arch/arm/boot/dts/axp209.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/axp209.dtsi
>>>>>>> @@ -89,4 +89,9 @@
>>>>>>>                           regulator-name = "ldo5";
>>>>>>>                   };
>>>>>>>           };
>>>>>>> +
>>>>>>> +       usb_power_supply: usb_power_supply {
>>>>>>> +               compatible = "x-powers,axp202-usb-power-supply";
>>>>>>> +               status = "disabled";
>>>>>>
>>>>>>
>>>>>> Is there any reason to have this disabled by default?
>>>>>> AFAIK this is not something configurable in hardware,
>>>>>> and the driver just gives readouts and status updates.
>>>>>
>>>>>
>>>>> There are some devices that only have 'AC-IN' instead of usb/otg power
>>>>> (set-top boxes) so having the devices tell what power sources they
>>>>> have makes sense (otherwise those that have ac-in instead of otg would
>>>>> have to disable usb_power_supply) and avoids confusion by explicitly
>>>>> stating presence in device dts.
>>>>
>>>>
>>>> That's true. But this is a bit of hardware inside the AXP.
>>>> If it's power isn't routed, it should report it, shouldn't it?
>>>
>>>
>>> If the device does not use the input there are two cases:
>>> - ac-in and usb/otg in short-circuited (there is a bit in AXP registers
>>>     to provide this indication)
>>> - the unused one is just not connected/connectable
>>>
>>> In either case the voltage of the unused/unusable branch is not really
>>> useful, nor is it useful to have it show up in sysfs.
>>
>>
>> Exactly my thinking on this, +1 .
>>
>>>> And normal users would probably expect to read the input
>>>> voltage/current/status through sysfs (or maybe hwmon?),
>>>> instead of not finding the device and having to look through
>>>> the DTS.
>>>
>>>
>>> Only those supplies effectively present (in-use or with connectors
>>> to eventually use them) should be visible in sysfs.
>>> Having more of them will just confuse the users, they would be searching
>>> how they can connect the supply while there is no means to do so
>>> except soldering to the tiny AXP pins.
>>
>>
>> +1
>
> OK. So mostly tablets and dev boards would benefit from having both
> power supplies. Is there a preference for usb-power-supply over
> vbus-det-gpio if both are available? Or just list them both and
> let the driver sort it out?

So far I've simply stuck with whatever the fex file uses, if both
are specified the driver will prefer the gpio, but that is not
documented behavior. IMHO it would be better to pick and specify
only one.

Regards,

Hans
--
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

[linux-sunxi] [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Hans de Goede]

Hi,

On 31-07-15 11:00, Chen-Yu Tsai wrote:
> On Fri, Jul 31, 2015 at 4:31 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>>
>> On 31-07-15 08:35, Bruno Pr?mont wrote:
>>>
>>> On Fri, 31 Jul 2015 14:14:28 +0800 Chen-Yu Tsai wrote:
>>>>
>>>> On Fri, Jul 31, 2015 at 1:51 PM, Bruno Pr?mont wrote:
>>>>>
>>>>> Hi ChenYu,
>>>>>
>>>>> On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
>>>>>>
>>>>>> Hi Hans,
>>>>>>
>>>>>> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede wrote:
>>>>>>>
>>>>>>> Add a node representing the usb power supply part of the axp209 pmic,
>>>>>>> note
>>>>>>> that the usb power supply and the (to be added later) ac power supply
>>>>>>> will
>>>>>>> each have their own child-node, so that they can be separately
>>>>>>> specified
>>>>>>> as power-supply for other nodes using a power-supply property with a
>>>>>>> phandle pointing to the right axp209 child-node.
>>>>>>>
>>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>>>> ---
>>>>>>>    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 24c935c..051ab3b 100644
>>>>>>> --- a/arch/arm/boot/dts/axp209.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/axp209.dtsi
>>>>>>> @@ -89,4 +89,9 @@
>>>>>>>                           regulator-name = "ldo5";
>>>>>>>                   };
>>>>>>>           };
>>>>>>> +
>>>>>>> +       usb_power_supply: usb_power_supply {
>>>>>>> +               compatible = "x-powers,axp202-usb-power-supply";
>>>>>>> +               status = "disabled";
>>>>>>
>>>>>>
>>>>>> Is there any reason to have this disabled by default?
>>>>>> AFAIK this is not something configurable in hardware,
>>>>>> and the driver just gives readouts and status updates.
>>>>>
>>>>>
>>>>> There are some devices that only have 'AC-IN' instead of usb/otg power
>>>>> (set-top boxes) so having the devices tell what power sources they
>>>>> have makes sense (otherwise those that have ac-in instead of otg would
>>>>> have to disable usb_power_supply) and avoids confusion by explicitly
>>>>> stating presence in device dts.
>>>>
>>>>
>>>> That's true. But this is a bit of hardware inside the AXP.
>>>> If it's power isn't routed, it should report it, shouldn't it?
>>>
>>>
>>> If the device does not use the input there are two cases:
>>> - ac-in and usb/otg in short-circuited (there is a bit in AXP registers
>>>     to provide this indication)
>>> - the unused one is just not connected/connectable
>>>
>>> In either case the voltage of the unused/unusable branch is not really
>>> useful, nor is it useful to have it show up in sysfs.
>>
>>
>> Exactly my thinking on this, +1 .
>>
>>>> And normal users would probably expect to read the input
>>>> voltage/current/status through sysfs (or maybe hwmon?),
>>>> instead of not finding the device and having to look through
>>>> the DTS.
>>>
>>>
>>> Only those supplies effectively present (in-use or with connectors
>>> to eventually use them) should be visible in sysfs.
>>> Having more of them will just confuse the users, they would be searching
>>> how they can connect the supply while there is no means to do so
>>> except soldering to the tiny AXP pins.
>>
>>
>> +1
>
> OK. So mostly tablets and dev boards would benefit from having both
> power supplies. Is there a preference for usb-power-supply over
> vbus-det-gpio if both are available? Or just list them both and
> let the driver sort it out?

So far I've simply stuck with whatever the fex file uses, if both
are specified the driver will prefer the gpio, but that is not
documented behavior. IMHO it would be better to pick and specify
only one.

Regards,

Hans

Re: [linux-sunxi] [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Hans de Goede]

Hi,

On 31-07-15 11:00, Chen-Yu Tsai wrote:
> On Fri, Jul 31, 2015 at 4:31 PM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> Hi,
>>
>>
>> On 31-07-15 08:35, Bruno Prémont wrote:
>>>
>>> On Fri, 31 Jul 2015 14:14:28 +0800 Chen-Yu Tsai wrote:
>>>>
>>>> On Fri, Jul 31, 2015 at 1:51 PM, Bruno Prémont wrote:
>>>>>
>>>>> Hi ChenYu,
>>>>>
>>>>> On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
>>>>>>
>>>>>> Hi Hans,
>>>>>>
>>>>>> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede wrote:
>>>>>>>
>>>>>>> Add a node representing the usb power supply part of the axp209 pmic,
>>>>>>> note
>>>>>>> that the usb power supply and the (to be added later) ac power supply
>>>>>>> will
>>>>>>> each have their own child-node, so that they can be separately
>>>>>>> specified
>>>>>>> as power-supply for other nodes using a power-supply property with a
>>>>>>> phandle pointing to the right axp209 child-node.
>>>>>>>
>>>>>>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>>>>> ---
>>>>>>>    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 24c935c..051ab3b 100644
>>>>>>> --- a/arch/arm/boot/dts/axp209.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/axp209.dtsi
>>>>>>> @@ -89,4 +89,9 @@
>>>>>>>                           regulator-name = "ldo5";
>>>>>>>                   };
>>>>>>>           };
>>>>>>> +
>>>>>>> +       usb_power_supply: usb_power_supply {
>>>>>>> +               compatible = "x-powers,axp202-usb-power-supply";
>>>>>>> +               status = "disabled";
>>>>>>
>>>>>>
>>>>>> Is there any reason to have this disabled by default?
>>>>>> AFAIK this is not something configurable in hardware,
>>>>>> and the driver just gives readouts and status updates.
>>>>>
>>>>>
>>>>> There are some devices that only have 'AC-IN' instead of usb/otg power
>>>>> (set-top boxes) so having the devices tell what power sources they
>>>>> have makes sense (otherwise those that have ac-in instead of otg would
>>>>> have to disable usb_power_supply) and avoids confusion by explicitly
>>>>> stating presence in device dts.
>>>>
>>>>
>>>> That's true. But this is a bit of hardware inside the AXP.
>>>> If it's power isn't routed, it should report it, shouldn't it?
>>>
>>>
>>> If the device does not use the input there are two cases:
>>> - ac-in and usb/otg in short-circuited (there is a bit in AXP registers
>>>     to provide this indication)
>>> - the unused one is just not connected/connectable
>>>
>>> In either case the voltage of the unused/unusable branch is not really
>>> useful, nor is it useful to have it show up in sysfs.
>>
>>
>> Exactly my thinking on this, +1 .
>>
>>>> And normal users would probably expect to read the input
>>>> voltage/current/status through sysfs (or maybe hwmon?),
>>>> instead of not finding the device and having to look through
>>>> the DTS.
>>>
>>>
>>> Only those supplies effectively present (in-use or with connectors
>>> to eventually use them) should be visible in sysfs.
>>> Having more of them will just confuse the users, they would be searching
>>> how they can connect the supply while there is no means to do so
>>> except soldering to the tiny AXP pins.
>>
>>
>> +1
>
> OK. So mostly tablets and dev boards would benefit from having both
> power supplies. Is there a preference for usb-power-supply over
> vbus-det-gpio if both are available? Or just list them both and
> let the driver sort it out?

p.s.

Note that the driver can even deal with not having a vbus-det at
all. It will then simulate a vbus drop for long enough to terminate
the current session when ever the id pin changes. This is necessary
for the original cubieboard which has its vbus wired hard to 5V.

This MUST only be used on boards woth broken vbus-det though, having
vbus-det is important to avoid feeding power to the outside when
a charger is plugged in (a charge cable should never pull the id
pin low, but you never know).

Regards,

Hans
--
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

[linux-sunxi] [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Hans de Goede]

Hi,

On 31-07-15 11:00, Chen-Yu Tsai wrote:
> On Fri, Jul 31, 2015 at 4:31 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>>
>> On 31-07-15 08:35, Bruno Pr?mont wrote:
>>>
>>> On Fri, 31 Jul 2015 14:14:28 +0800 Chen-Yu Tsai wrote:
>>>>
>>>> On Fri, Jul 31, 2015 at 1:51 PM, Bruno Pr?mont wrote:
>>>>>
>>>>> Hi ChenYu,
>>>>>
>>>>> On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
>>>>>>
>>>>>> Hi Hans,
>>>>>>
>>>>>> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede wrote:
>>>>>>>
>>>>>>> Add a node representing the usb power supply part of the axp209 pmic,
>>>>>>> note
>>>>>>> that the usb power supply and the (to be added later) ac power supply
>>>>>>> will
>>>>>>> each have their own child-node, so that they can be separately
>>>>>>> specified
>>>>>>> as power-supply for other nodes using a power-supply property with a
>>>>>>> phandle pointing to the right axp209 child-node.
>>>>>>>
>>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>>>> ---
>>>>>>>    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 24c935c..051ab3b 100644
>>>>>>> --- a/arch/arm/boot/dts/axp209.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/axp209.dtsi
>>>>>>> @@ -89,4 +89,9 @@
>>>>>>>                           regulator-name = "ldo5";
>>>>>>>                   };
>>>>>>>           };
>>>>>>> +
>>>>>>> +       usb_power_supply: usb_power_supply {
>>>>>>> +               compatible = "x-powers,axp202-usb-power-supply";
>>>>>>> +               status = "disabled";
>>>>>>
>>>>>>
>>>>>> Is there any reason to have this disabled by default?
>>>>>> AFAIK this is not something configurable in hardware,
>>>>>> and the driver just gives readouts and status updates.
>>>>>
>>>>>
>>>>> There are some devices that only have 'AC-IN' instead of usb/otg power
>>>>> (set-top boxes) so having the devices tell what power sources they
>>>>> have makes sense (otherwise those that have ac-in instead of otg would
>>>>> have to disable usb_power_supply) and avoids confusion by explicitly
>>>>> stating presence in device dts.
>>>>
>>>>
>>>> That's true. But this is a bit of hardware inside the AXP.
>>>> If it's power isn't routed, it should report it, shouldn't it?
>>>
>>>
>>> If the device does not use the input there are two cases:
>>> - ac-in and usb/otg in short-circuited (there is a bit in AXP registers
>>>     to provide this indication)
>>> - the unused one is just not connected/connectable
>>>
>>> In either case the voltage of the unused/unusable branch is not really
>>> useful, nor is it useful to have it show up in sysfs.
>>
>>
>> Exactly my thinking on this, +1 .
>>
>>>> And normal users would probably expect to read the input
>>>> voltage/current/status through sysfs (or maybe hwmon?),
>>>> instead of not finding the device and having to look through
>>>> the DTS.
>>>
>>>
>>> Only those supplies effectively present (in-use or with connectors
>>> to eventually use them) should be visible in sysfs.
>>> Having more of them will just confuse the users, they would be searching
>>> how they can connect the supply while there is no means to do so
>>> except soldering to the tiny AXP pins.
>>
>>
>> +1
>
> OK. So mostly tablets and dev boards would benefit from having both
> power supplies. Is there a preference for usb-power-supply over
> vbus-det-gpio if both are available? Or just list them both and
> let the driver sort it out?

p.s.

Note that the driver can even deal with not having a vbus-det at
all. It will then simulate a vbus drop for long enough to terminate
the current session when ever the id pin changes. This is necessary
for the original cubieboard which has its vbus wired hard to 5V.

This MUST only be used on boards woth broken vbus-det though, having
vbus-det is important to avoid feeding power to the outside when
a charger is plugged in (a charge cable should never pull the id
pin low, but you never know).

Regards,

Hans

Re: [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Chen-Yu Tsai]

On Fri, Jul 31, 2015 at 5:11 PM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Hi,
>
>
> On 31-07-15 11:00, Chen-Yu Tsai wrote:
>>
>> On Fri, Jul 31, 2015 at 4:31 PM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> wrote:
>>>
>>> Hi,
>>>
>>>
>>> On 31-07-15 08:35, Bruno Prémont wrote:
>>>>
>>>>
>>>> On Fri, 31 Jul 2015 14:14:28 +0800 Chen-Yu Tsai wrote:
>>>>>
>>>>>
>>>>> On Fri, Jul 31, 2015 at 1:51 PM, Bruno Prémont wrote:
>>>>>>
>>>>>>
>>>>>> Hi ChenYu,
>>>>>>
>>>>>> On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
>>>>>>>
>>>>>>>
>>>>>>> Hi Hans,
>>>>>>>
>>>>>>> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> Add a node representing the usb power supply part of the axp209
>>>>>>>> pmic,
>>>>>>>> note
>>>>>>>> that the usb power supply and the (to be added later) ac power
>>>>>>>> supply
>>>>>>>> will
>>>>>>>> each have their own child-node, so that they can be separately
>>>>>>>> specified
>>>>>>>> as power-supply for other nodes using a power-supply property with a
>>>>>>>> phandle pointing to the right axp209 child-node.
>>>>>>>>
>>>>>>>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>>>>>> ---
>>>>>>>>    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 24c935c..051ab3b 100644
>>>>>>>> --- a/arch/arm/boot/dts/axp209.dtsi
>>>>>>>> +++ b/arch/arm/boot/dts/axp209.dtsi
>>>>>>>> @@ -89,4 +89,9 @@
>>>>>>>>                           regulator-name = "ldo5";
>>>>>>>>                   };
>>>>>>>>           };
>>>>>>>> +
>>>>>>>> +       usb_power_supply: usb_power_supply {
>>>>>>>> +               compatible = "x-powers,axp202-usb-power-supply";
>>>>>>>> +               status = "disabled";
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Is there any reason to have this disabled by default?
>>>>>>> AFAIK this is not something configurable in hardware,
>>>>>>> and the driver just gives readouts and status updates.
>>>>>>
>>>>>>
>>>>>>
>>>>>> There are some devices that only have 'AC-IN' instead of usb/otg power
>>>>>> (set-top boxes) so having the devices tell what power sources they
>>>>>> have makes sense (otherwise those that have ac-in instead of otg would
>>>>>> have to disable usb_power_supply) and avoids confusion by explicitly
>>>>>> stating presence in device dts.
>>>>>
>>>>>
>>>>>
>>>>> That's true. But this is a bit of hardware inside the AXP.
>>>>> If it's power isn't routed, it should report it, shouldn't it?
>>>>
>>>>
>>>>
>>>> If the device does not use the input there are two cases:
>>>> - ac-in and usb/otg in short-circuited (there is a bit in AXP registers
>>>>     to provide this indication)
>>>> - the unused one is just not connected/connectable
>>>>
>>>> In either case the voltage of the unused/unusable branch is not really
>>>> useful, nor is it useful to have it show up in sysfs.
>>>
>>>
>>>
>>> Exactly my thinking on this, +1 .
>>>
>>>>> And normal users would probably expect to read the input
>>>>> voltage/current/status through sysfs (or maybe hwmon?),
>>>>> instead of not finding the device and having to look through
>>>>> the DTS.
>>>>
>>>>
>>>>
>>>> Only those supplies effectively present (in-use or with connectors
>>>> to eventually use them) should be visible in sysfs.
>>>> Having more of them will just confuse the users, they would be searching
>>>> how they can connect the supply while there is no means to do so
>>>> except soldering to the tiny AXP pins.
>>>
>>>
>>>
>>> +1
>>
>>
>> OK. So mostly tablets and dev boards would benefit from having both
>> power supplies. Is there a preference for usb-power-supply over
>> vbus-det-gpio if both are available? Or just list them both and
>> let the driver sort it out?
>
>
> So far I've simply stuck with whatever the fex file uses, if both
> are specified the driver will prefer the gpio, but that is not
> documented behavior. IMHO it would be better to pick and specify
> only one.

>From a functional standpoint, if usb-power-supply is available,
using it would be better because:

a) VBUS detection should be much better (IIRC cutoff at 4.4V)
b) Controllable max current draw
c) Can see the current draw

Of course not all boards would work. I think I even have a board
that is only half powered on when only USB VBUS is used.

ChenYu

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

[linux-sunxi] [PATCH 6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

[Chen-Yu Tsai]

On Fri, Jul 31, 2015 at 5:11 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 31-07-15 11:00, Chen-Yu Tsai wrote:
>>
>> On Fri, Jul 31, 2015 at 4:31 PM, Hans de Goede <hdegoede@redhat.com>
>> wrote:
>>>
>>> Hi,
>>>
>>>
>>> On 31-07-15 08:35, Bruno Pr?mont wrote:
>>>>
>>>>
>>>> On Fri, 31 Jul 2015 14:14:28 +0800 Chen-Yu Tsai wrote:
>>>>>
>>>>>
>>>>> On Fri, Jul 31, 2015 at 1:51 PM, Bruno Pr?mont wrote:
>>>>>>
>>>>>>
>>>>>> Hi ChenYu,
>>>>>>
>>>>>> On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
>>>>>>>
>>>>>>>
>>>>>>> Hi Hans,
>>>>>>>
>>>>>>> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> Add a node representing the usb power supply part of the axp209
>>>>>>>> pmic,
>>>>>>>> note
>>>>>>>> that the usb power supply and the (to be added later) ac power
>>>>>>>> supply
>>>>>>>> will
>>>>>>>> each have their own child-node, so that they can be separately
>>>>>>>> specified
>>>>>>>> as power-supply for other nodes using a power-supply property with a
>>>>>>>> phandle pointing to the right axp209 child-node.
>>>>>>>>
>>>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>>>>> ---
>>>>>>>>    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 24c935c..051ab3b 100644
>>>>>>>> --- a/arch/arm/boot/dts/axp209.dtsi
>>>>>>>> +++ b/arch/arm/boot/dts/axp209.dtsi
>>>>>>>> @@ -89,4 +89,9 @@
>>>>>>>>                           regulator-name = "ldo5";
>>>>>>>>                   };
>>>>>>>>           };
>>>>>>>> +
>>>>>>>> +       usb_power_supply: usb_power_supply {
>>>>>>>> +               compatible = "x-powers,axp202-usb-power-supply";
>>>>>>>> +               status = "disabled";
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Is there any reason to have this disabled by default?
>>>>>>> AFAIK this is not something configurable in hardware,
>>>>>>> and the driver just gives readouts and status updates.
>>>>>>
>>>>>>
>>>>>>
>>>>>> There are some devices that only have 'AC-IN' instead of usb/otg power
>>>>>> (set-top boxes) so having the devices tell what power sources they
>>>>>> have makes sense (otherwise those that have ac-in instead of otg would
>>>>>> have to disable usb_power_supply) and avoids confusion by explicitly
>>>>>> stating presence in device dts.
>>>>>
>>>>>
>>>>>
>>>>> That's true. But this is a bit of hardware inside the AXP.
>>>>> If it's power isn't routed, it should report it, shouldn't it?
>>>>
>>>>
>>>>
>>>> If the device does not use the input there are two cases:
>>>> - ac-in and usb/otg in short-circuited (there is a bit in AXP registers
>>>>     to provide this indication)
>>>> - the unused one is just not connected/connectable
>>>>
>>>> In either case the voltage of the unused/unusable branch is not really
>>>> useful, nor is it useful to have it show up in sysfs.
>>>
>>>
>>>
>>> Exactly my thinking on this, +1 .
>>>
>>>>> And normal users would probably expect to read the input
>>>>> voltage/current/status through sysfs (or maybe hwmon?),
>>>>> instead of not finding the device and having to look through
>>>>> the DTS.
>>>>
>>>>
>>>>
>>>> Only those supplies effectively present (in-use or with connectors
>>>> to eventually use them) should be visible in sysfs.
>>>> Having more of them will just confuse the users, they would be searching
>>>> how they can connect the supply while there is no means to do so
>>>> except soldering to the tiny AXP pins.
>>>
>>>
>>>
>>> +1
>>
>>
>> OK. So mostly tablets and dev boards would benefit from having both
>> power supplies. Is there a preference for usb-power-supply over
>> vbus-det-gpio if both are available? Or just list them both and
>> let the driver sort it out?
>
>
> So far I've simply stuck with whatever the fex file uses, if both
> are specified the driver will prefer the gpio, but that is not
> documented behavior. IMHO it would be better to pick and specify
> only one.

>From a functional standpoint, if usb-power-supply is available,
using it would be better because:

a) VBUS detection should be much better (IIRC cutoff at 4.4V)
b) Controllable max current draw
c) Can see the current draw

Of course not all boards would work. I think I even have a board
that is only half powered on when only USB VBUS is used.

ChenYu