[PATCH] regulator: axp20x: Add support for the (external) drivebus regulator

[PATCH] regulator: axp20x: Add support for the (external) drivebus regulator

[Hans de Goede]

The axp20x pmics have 2 power inputs, one called ACIN which is intended
for to be supplied via a powerbarrel on the board and one called VBUS
which is intended to be supplied via an otg connector.

In the VBUS case the pmic needs to know if the board is supplying power
to the otg connector, because then it should not take any power from
its VBUS pin. The axp209 pmic has a N_VBUSEN input pin via which the
board can signal to the pmic whether the board is supplying power to the
otg connector or not.

On the axp221/axp223 this pin can alternatively be used as an output
which controls an external regulator which (optionally) supplies
power to the otg connector from the board. When the pin is used as
output it is called DRIVEBUS in the datasheet.

This commit adds support for the DRIVEBUS pin as an extra pmic
controlled regulator. Since this is optional a new x-powers,drivebus dt
property is added. When this is present the misc-control register is
written to change the N_VBUSEN input pin to DRIVEBUS output pin mode and
the extra drivebus regulator is registered with the regulator subsystem.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 Documentation/devicetree/bindings/mfd/axp20x.txt |  5 ++++
 drivers/regulator/axp20x-regulator.c             | 30 ++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
index d20b103..7a88479 100644
--- a/Documentation/devicetree/bindings/mfd/axp20x.txt
+++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
@@ -22,6 +22,11 @@ Optional properties:
 		      AXP152/20X: range:  750-1875, Default: 1.5 MHz
 		      AXP22X/80X: range: 1800-4050, Default: 3   MHz
 
+- x-powers,drivebus: axp221 / axp223 only boolean, set this when the N_VBUSEN
+                  pin is used as an output pin to control an external regulator
+                  to drive the OTG VBus, rather then as an input pin which
+                  signals whether the board is driving OTG VBus or not.
+
 - <input>-supply: a phandle to the regulator supply node. May be omitted if
 		  inputs are unregulated, such as using the IPSOUT output
 		  from the PMIC.
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 514a5e8..2422ddd 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -36,6 +36,8 @@
 
 #define AXP20X_FREQ_DCDC_MASK		0x0f
 
+#define AXP22X_MISC_N_VBUSEN_FUNC	BIT(4)
+
 #define AXP_DESC_IO(_family, _id, _match, _supply, _min, _max, _step, _vreg,	\
 		    _vmask, _ereg, _emask, _enable_val, _disable_val)		\
 	[_family##_##_id] = {							\
@@ -230,6 +232,18 @@ static const struct regulator_desc axp22x_regulators[] = {
 	AXP_DESC_FIXED(AXP22X, RTC_LDO, "rtc_ldo", "ips", 3000),
 };
 
+static const struct regulator_desc axp22x_drivebus_regulator = {
+	.name		= "drivebus",
+	.supply_name	= "ips",
+	.of_match	= of_match_ptr("drivebus"),
+	.regulators_node = of_match_ptr("regulators"),
+	.type		= REGULATOR_VOLTAGE,
+	.owner		= THIS_MODULE,
+	.enable_reg	= AXP20X_VBUS_IPSOUT_MGMT,
+	.enable_mask	= BIT(2),
+	.ops		= &axp20x_ops_sw,
+};
+
 static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
 {
 	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
@@ -354,6 +368,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 	u32 workmode;
 	const char *axp22x_dc1_name = axp22x_regulators[AXP22X_DCDC1].name;
 	const char *axp22x_dc5_name = axp22x_regulators[AXP22X_DCDC5].name;
+	bool drivebus = false;
 
 	switch (axp20x->variant) {
 	case AXP202_ID:
@@ -365,6 +380,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 	case AXP223_ID:
 		regulators = axp22x_regulators;
 		nregulators = AXP22X_REG_ID_MAX;
+		drivebus = of_property_read_bool(pdev->dev.parent->of_node,
+						 "x-powers,drivebus");
 		break;
 	default:
 		dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
@@ -439,6 +456,19 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		}
 	}
 
+	if (drivebus) {
+		/* Change N_VBUSEN sense pin to DRIVEBUS output pin */
+		regmap_update_bits(axp20x->regmap, AXP20X_OVER_TMP,
+				   AXP22X_MISC_N_VBUSEN_FUNC, 0);
+		rdev = devm_regulator_register(&pdev->dev,
+					       &axp22x_drivebus_regulator,
+					       &config);
+		if (IS_ERR(rdev)) {
+			dev_err(&pdev->dev, "Failed to register drivebus\n");
+			return PTR_ERR(rdev);
+		}
+	}
+
 	return 0;
 }
 
-- 
2.7.4

[PATCH] regulator: axp20x: Add support for the (external) drivebus regulator

[Hans de Goede]

The axp20x pmics have 2 power inputs, one called ACIN which is intended
for to be supplied via a powerbarrel on the board and one called VBUS
which is intended to be supplied via an otg connector.

In the VBUS case the pmic needs to know if the board is supplying power
to the otg connector, because then it should not take any power from
its VBUS pin. The axp209 pmic has a N_VBUSEN input pin via which the
board can signal to the pmic whether the board is supplying power to the
otg connector or not.

On the axp221/axp223 this pin can alternatively be used as an output
which controls an external regulator which (optionally) supplies
power to the otg connector from the board. When the pin is used as
output it is called DRIVEBUS in the datasheet.

This commit adds support for the DRIVEBUS pin as an extra pmic
controlled regulator. Since this is optional a new x-powers,drivebus dt
property is added. When this is present the misc-control register is
written to change the N_VBUSEN input pin to DRIVEBUS output pin mode and
the extra drivebus regulator is registered with the regulator subsystem.

Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 Documentation/devicetree/bindings/mfd/axp20x.txt |  5 ++++
 drivers/regulator/axp20x-regulator.c             | 30 ++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
index d20b103..7a88479 100644
--- a/Documentation/devicetree/bindings/mfd/axp20x.txt
+++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
@@ -22,6 +22,11 @@ Optional properties:
 		      AXP152/20X: range:  750-1875, Default: 1.5 MHz
 		      AXP22X/80X: range: 1800-4050, Default: 3   MHz
 
+- x-powers,drivebus: axp221 / axp223 only boolean, set this when the N_VBUSEN
+                  pin is used as an output pin to control an external regulator
+                  to drive the OTG VBus, rather then as an input pin which
+                  signals whether the board is driving OTG VBus or not.
+
 - <input>-supply: a phandle to the regulator supply node. May be omitted if
 		  inputs are unregulated, such as using the IPSOUT output
 		  from the PMIC.
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 514a5e8..2422ddd 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -36,6 +36,8 @@
 
 #define AXP20X_FREQ_DCDC_MASK		0x0f
 
+#define AXP22X_MISC_N_VBUSEN_FUNC	BIT(4)
+
 #define AXP_DESC_IO(_family, _id, _match, _supply, _min, _max, _step, _vreg,	\
 		    _vmask, _ereg, _emask, _enable_val, _disable_val)		\
 	[_family##_##_id] = {							\
@@ -230,6 +232,18 @@ static const struct regulator_desc axp22x_regulators[] = {
 	AXP_DESC_FIXED(AXP22X, RTC_LDO, "rtc_ldo", "ips", 3000),
 };
 
+static const struct regulator_desc axp22x_drivebus_regulator = {
+	.name		= "drivebus",
+	.supply_name	= "ips",
+	.of_match	= of_match_ptr("drivebus"),
+	.regulators_node = of_match_ptr("regulators"),
+	.type		= REGULATOR_VOLTAGE,
+	.owner		= THIS_MODULE,
+	.enable_reg	= AXP20X_VBUS_IPSOUT_MGMT,
+	.enable_mask	= BIT(2),
+	.ops		= &axp20x_ops_sw,
+};
+
 static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
 {
 	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
@@ -354,6 +368,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 	u32 workmode;
 	const char *axp22x_dc1_name = axp22x_regulators[AXP22X_DCDC1].name;
 	const char *axp22x_dc5_name = axp22x_regulators[AXP22X_DCDC5].name;
+	bool drivebus = false;
 
 	switch (axp20x->variant) {
 	case AXP202_ID:
@@ -365,6 +380,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 	case AXP223_ID:
 		regulators = axp22x_regulators;
 		nregulators = AXP22X_REG_ID_MAX;
+		drivebus = of_property_read_bool(pdev->dev.parent->of_node,
+						 "x-powers,drivebus");
 		break;
 	default:
 		dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
@@ -439,6 +456,19 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		}
 	}
 
+	if (drivebus) {
+		/* Change N_VBUSEN sense pin to DRIVEBUS output pin */
+		regmap_update_bits(axp20x->regmap, AXP20X_OVER_TMP,
+				   AXP22X_MISC_N_VBUSEN_FUNC, 0);
+		rdev = devm_regulator_register(&pdev->dev,
+					       &axp22x_drivebus_regulator,
+					       &config);
+		if (IS_ERR(rdev)) {
+			dev_err(&pdev->dev, "Failed to register drivebus\n");
+			return PTR_ERR(rdev);
+		}
+	}
+
 	return 0;
 }
 
-- 
2.7.4

--
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] regulator: axp20x: Add support for the (external) drivebus regulator

[Hans de Goede]

The axp20x pmics have 2 power inputs, one called ACIN which is intended
for to be supplied via a powerbarrel on the board and one called VBUS
which is intended to be supplied via an otg connector.

In the VBUS case the pmic needs to know if the board is supplying power
to the otg connector, because then it should not take any power from
its VBUS pin. The axp209 pmic has a N_VBUSEN input pin via which the
board can signal to the pmic whether the board is supplying power to the
otg connector or not.

On the axp221/axp223 this pin can alternatively be used as an output
which controls an external regulator which (optionally) supplies
power to the otg connector from the board. When the pin is used as
output it is called DRIVEBUS in the datasheet.

This commit adds support for the DRIVEBUS pin as an extra pmic
controlled regulator. Since this is optional a new x-powers,drivebus dt
property is added. When this is present the misc-control register is
written to change the N_VBUSEN input pin to DRIVEBUS output pin mode and
the extra drivebus regulator is registered with the regulator subsystem.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 Documentation/devicetree/bindings/mfd/axp20x.txt |  5 ++++
 drivers/regulator/axp20x-regulator.c             | 30 ++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
index d20b103..7a88479 100644
--- a/Documentation/devicetree/bindings/mfd/axp20x.txt
+++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
@@ -22,6 +22,11 @@ Optional properties:
 		      AXP152/20X: range:  750-1875, Default: 1.5 MHz
 		      AXP22X/80X: range: 1800-4050, Default: 3   MHz
 
+- x-powers,drivebus: axp221 / axp223 only boolean, set this when the N_VBUSEN
+                  pin is used as an output pin to control an external regulator
+                  to drive the OTG VBus, rather then as an input pin which
+                  signals whether the board is driving OTG VBus or not.
+
 - <input>-supply: a phandle to the regulator supply node. May be omitted if
 		  inputs are unregulated, such as using the IPSOUT output
 		  from the PMIC.
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 514a5e8..2422ddd 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -36,6 +36,8 @@
 
 #define AXP20X_FREQ_DCDC_MASK		0x0f
 
+#define AXP22X_MISC_N_VBUSEN_FUNC	BIT(4)
+
 #define AXP_DESC_IO(_family, _id, _match, _supply, _min, _max, _step, _vreg,	\
 		    _vmask, _ereg, _emask, _enable_val, _disable_val)		\
 	[_family##_##_id] = {							\
@@ -230,6 +232,18 @@ static const struct regulator_desc axp22x_regulators[] = {
 	AXP_DESC_FIXED(AXP22X, RTC_LDO, "rtc_ldo", "ips", 3000),
 };
 
+static const struct regulator_desc axp22x_drivebus_regulator = {
+	.name		= "drivebus",
+	.supply_name	= "ips",
+	.of_match	= of_match_ptr("drivebus"),
+	.regulators_node = of_match_ptr("regulators"),
+	.type		= REGULATOR_VOLTAGE,
+	.owner		= THIS_MODULE,
+	.enable_reg	= AXP20X_VBUS_IPSOUT_MGMT,
+	.enable_mask	= BIT(2),
+	.ops		= &axp20x_ops_sw,
+};
+
 static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
 {
 	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
@@ -354,6 +368,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 	u32 workmode;
 	const char *axp22x_dc1_name = axp22x_regulators[AXP22X_DCDC1].name;
 	const char *axp22x_dc5_name = axp22x_regulators[AXP22X_DCDC5].name;
+	bool drivebus = false;
 
 	switch (axp20x->variant) {
 	case AXP202_ID:
@@ -365,6 +380,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 	case AXP223_ID:
 		regulators = axp22x_regulators;
 		nregulators = AXP22X_REG_ID_MAX;
+		drivebus = of_property_read_bool(pdev->dev.parent->of_node,
+						 "x-powers,drivebus");
 		break;
 	default:
 		dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
@@ -439,6 +456,19 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		}
 	}
 
+	if (drivebus) {
+		/* Change N_VBUSEN sense pin to DRIVEBUS output pin */
+		regmap_update_bits(axp20x->regmap, AXP20X_OVER_TMP,
+				   AXP22X_MISC_N_VBUSEN_FUNC, 0);
+		rdev = devm_regulator_register(&pdev->dev,
+					       &axp22x_drivebus_regulator,
+					       &config);
+		if (IS_ERR(rdev)) {
+			dev_err(&pdev->dev, "Failed to register drivebus\n");
+			return PTR_ERR(rdev);
+		}
+	}
+
 	return 0;
 }
 
-- 
2.7.4

Re: [PATCH] regulator: axp20x: Add support for the (external) drivebus regulator

[Rob Herring]

On Sat, May 14, 2016 at 07:55:45PM +0200, Hans de Goede wrote:
> The axp20x pmics have 2 power inputs, one called ACIN which is intended
> for to be supplied via a powerbarrel on the board and one called VBUS
> which is intended to be supplied via an otg connector.
> 
> In the VBUS case the pmic needs to know if the board is supplying power
> to the otg connector, because then it should not take any power from
> its VBUS pin. The axp209 pmic has a N_VBUSEN input pin via which the
> board can signal to the pmic whether the board is supplying power to the
> otg connector or not.
> 
> On the axp221/axp223 this pin can alternatively be used as an output
> which controls an external regulator which (optionally) supplies
> power to the otg connector from the board. When the pin is used as
> output it is called DRIVEBUS in the datasheet.
> 
> This commit adds support for the DRIVEBUS pin as an extra pmic
> controlled regulator. Since this is optional a new x-powers,drivebus dt
> property is added. When this is present the misc-control register is
> written to change the N_VBUSEN input pin to DRIVEBUS output pin mode and
> the extra drivebus regulator is registered with the regulator subsystem.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  Documentation/devicetree/bindings/mfd/axp20x.txt |  5 ++++
>  drivers/regulator/axp20x-regulator.c             | 30 ++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
> index d20b103..7a88479 100644
> --- a/Documentation/devicetree/bindings/mfd/axp20x.txt
> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
> @@ -22,6 +22,11 @@ Optional properties:
>  		      AXP152/20X: range:  750-1875, Default: 1.5 MHz
>  		      AXP22X/80X: range: 1800-4050, Default: 3   MHz
>  
> +- x-powers,drivebus: axp221 / axp223 only boolean, set this when the N_VBUSEN

x-powers,drive-vbus-en

> +                  pin is used as an output pin to control an external regulator
> +                  to drive the OTG VBus, rather then as an input pin which
> +                  signals whether the board is driving OTG VBus or not.
> +
>  - <input>-supply: a phandle to the regulator supply node. May be omitted if
>  		  inputs are unregulated, such as using the IPSOUT output
>  		  from the PMIC.

Re: [PATCH] regulator: axp20x: Add support for the (external) drivebus regulator

[Rob Herring]

On Sat, May 14, 2016 at 07:55:45PM +0200, Hans de Goede wrote:
> The axp20x pmics have 2 power inputs, one called ACIN which is intended
> for to be supplied via a powerbarrel on the board and one called VBUS
> which is intended to be supplied via an otg connector.
> 
> In the VBUS case the pmic needs to know if the board is supplying power
> to the otg connector, because then it should not take any power from
> its VBUS pin. The axp209 pmic has a N_VBUSEN input pin via which the
> board can signal to the pmic whether the board is supplying power to the
> otg connector or not.
> 
> On the axp221/axp223 this pin can alternatively be used as an output
> which controls an external regulator which (optionally) supplies
> power to the otg connector from the board. When the pin is used as
> output it is called DRIVEBUS in the datasheet.
> 
> This commit adds support for the DRIVEBUS pin as an extra pmic
> controlled regulator. Since this is optional a new x-powers,drivebus dt
> property is added. When this is present the misc-control register is
> written to change the N_VBUSEN input pin to DRIVEBUS output pin mode and
> the extra drivebus regulator is registered with the regulator subsystem.
> 
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/mfd/axp20x.txt |  5 ++++
>  drivers/regulator/axp20x-regulator.c             | 30 ++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
> index d20b103..7a88479 100644
> --- a/Documentation/devicetree/bindings/mfd/axp20x.txt
> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
> @@ -22,6 +22,11 @@ Optional properties:
>  		      AXP152/20X: range:  750-1875, Default: 1.5 MHz
>  		      AXP22X/80X: range: 1800-4050, Default: 3   MHz
>  
> +- x-powers,drivebus: axp221 / axp223 only boolean, set this when the N_VBUSEN

x-powers,drive-vbus-en

> +                  pin is used as an output pin to control an external regulator
> +                  to drive the OTG VBus, rather then as an input pin which
> +                  signals whether the board is driving OTG VBus or not.
> +
>  - <input>-supply: a phandle to the regulator supply node. May be omitted if
>  		  inputs are unregulated, such as using the IPSOUT output
>  		  from the PMIC.
--
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] regulator: axp20x: Add support for the (external) drivebus regulator

[Rob Herring]

On Sat, May 14, 2016 at 07:55:45PM +0200, Hans de Goede wrote:
> The axp20x pmics have 2 power inputs, one called ACIN which is intended
> for to be supplied via a powerbarrel on the board and one called VBUS
> which is intended to be supplied via an otg connector.
> 
> In the VBUS case the pmic needs to know if the board is supplying power
> to the otg connector, because then it should not take any power from
> its VBUS pin. The axp209 pmic has a N_VBUSEN input pin via which the
> board can signal to the pmic whether the board is supplying power to the
> otg connector or not.
> 
> On the axp221/axp223 this pin can alternatively be used as an output
> which controls an external regulator which (optionally) supplies
> power to the otg connector from the board. When the pin is used as
> output it is called DRIVEBUS in the datasheet.
> 
> This commit adds support for the DRIVEBUS pin as an extra pmic
> controlled regulator. Since this is optional a new x-powers,drivebus dt
> property is added. When this is present the misc-control register is
> written to change the N_VBUSEN input pin to DRIVEBUS output pin mode and
> the extra drivebus regulator is registered with the regulator subsystem.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  Documentation/devicetree/bindings/mfd/axp20x.txt |  5 ++++
>  drivers/regulator/axp20x-regulator.c             | 30 ++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
> index d20b103..7a88479 100644
> --- a/Documentation/devicetree/bindings/mfd/axp20x.txt
> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
> @@ -22,6 +22,11 @@ Optional properties:
>  		      AXP152/20X: range:  750-1875, Default: 1.5 MHz
>  		      AXP22X/80X: range: 1800-4050, Default: 3   MHz
>  
> +- x-powers,drivebus: axp221 / axp223 only boolean, set this when the N_VBUSEN

x-powers,drive-vbus-en

> +                  pin is used as an output pin to control an external regulator
> +                  to drive the OTG VBus, rather then as an input pin which
> +                  signals whether the board is driving OTG VBus or not.
> +
>  - <input>-supply: a phandle to the regulator supply node. May be omitted if
>  		  inputs are unregulated, such as using the IPSOUT output
>  		  from the PMIC.

Re: [PATCH] regulator: axp20x: Add support for the (external) drivebus regulator

[Chen-Yu Tsai]

Hi,

On Sun, May 15, 2016 at 1:55 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> The axp20x pmics have 2 power inputs, one called ACIN which is intended
> for to be supplied via a powerbarrel on the board and one called VBUS
> which is intended to be supplied via an otg connector.
>
> In the VBUS case the pmic needs to know if the board is supplying power
> to the otg connector, because then it should not take any power from
> its VBUS pin. The axp209 pmic has a N_VBUSEN input pin via which the
> board can signal to the pmic whether the board is supplying power to the
> otg connector or not.
>
> On the axp221/axp223 this pin can alternatively be used as an output
> which controls an external regulator which (optionally) supplies
> power to the otg connector from the board. When the pin is used as
> output it is called DRIVEBUS in the datasheet.

The datasheet actually calls it DRIVEVBUS. DRIVEBUS seems to be some
typo Allwinner has in their code.

>
> This commit adds support for the DRIVEBUS pin as an extra pmic
> controlled regulator. Since this is optional a new x-powers,drivebus dt
> property is added. When this is present the misc-control register is
> written to change the N_VBUSEN input pin to DRIVEBUS output pin mode and
> the extra drivebus regulator is registered with the regulator subsystem.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  Documentation/devicetree/bindings/mfd/axp20x.txt |  5 ++++
>  drivers/regulator/axp20x-regulator.c             | 30 ++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
> index d20b103..7a88479 100644
> --- a/Documentation/devicetree/bindings/mfd/axp20x.txt
> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
> @@ -22,6 +22,11 @@ Optional properties:
>                       AXP152/20X: range:  750-1875, Default: 1.5 MHz
>                       AXP22X/80X: range: 1800-4050, Default: 3   MHz
>
> +- x-powers,drivebus: axp221 / axp223 only boolean, set this when the N_VBUSEN
> +                  pin is used as an output pin to control an external regulator
> +                  to drive the OTG VBus, rather then as an input pin which
> +                  signals whether the board is driving OTG VBus or not.
> +

You should also add it to the regulators table further down in the binding.
And, as Rob mentioned, x-powers,drive-vbus-en (or drivevbus-en) is better
for a boolean property.

>  - <input>-supply: a phandle to the regulator supply node. May be omitted if
>                   inputs are unregulated, such as using the IPSOUT output
>                   from the PMIC.
> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
> index 514a5e8..2422ddd 100644
> --- a/drivers/regulator/axp20x-regulator.c
> +++ b/drivers/regulator/axp20x-regulator.c
> @@ -36,6 +36,8 @@
>
>  #define AXP20X_FREQ_DCDC_MASK          0x0f
>
> +#define AXP22X_MISC_N_VBUSEN_FUNC      BIT(4)
> +
>  #define AXP_DESC_IO(_family, _id, _match, _supply, _min, _max, _step, _vreg,   \
>                     _vmask, _ereg, _emask, _enable_val, _disable_val)           \
>         [_family##_##_id] = {                                                   \
> @@ -230,6 +232,18 @@ static const struct regulator_desc axp22x_regulators[] = {
>         AXP_DESC_FIXED(AXP22X, RTC_LDO, "rtc_ldo", "ips", 3000),
>  };
>
> +static const struct regulator_desc axp22x_drivebus_regulator = {
> +       .name           = "drivebus",
> +       .supply_name    = "ips",

Given that you are approximating a gpio (in this case, the
N_VBUSEN/DRIVEVBUS pin)
controlled fixed voltage regulator, it would be better if you could:

a) have some way to specify the output voltage, so it looks like a fixed voltage
   regulator that can be turned on/off.

And

b) have a separate supply name for this so it properly propagates enable/disable
   calls. It would also pass the upstream voltage, thus behaving like a switch
   you intended with the axp20x_ops_sw below.


Regards
ChenYu

> +       .of_match       = of_match_ptr("drivebus"),
> +       .regulators_node = of_match_ptr("regulators"),
> +       .type           = REGULATOR_VOLTAGE,
> +       .owner          = THIS_MODULE,
> +       .enable_reg     = AXP20X_VBUS_IPSOUT_MGMT,
> +       .enable_mask    = BIT(2),
> +       .ops            = &axp20x_ops_sw,
> +};
> +
>  static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
>  {
>         struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
> @@ -354,6 +368,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>         u32 workmode;
>         const char *axp22x_dc1_name = axp22x_regulators[AXP22X_DCDC1].name;
>         const char *axp22x_dc5_name = axp22x_regulators[AXP22X_DCDC5].name;
> +       bool drivebus = false;
>
>         switch (axp20x->variant) {
>         case AXP202_ID:
> @@ -365,6 +380,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>         case AXP223_ID:
>                 regulators = axp22x_regulators;
>                 nregulators = AXP22X_REG_ID_MAX;
> +               drivebus = of_property_read_bool(pdev->dev.parent->of_node,
> +                                                "x-powers,drivebus");
>                 break;
>         default:
>                 dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
> @@ -439,6 +456,19 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>                 }
>         }
>
> +       if (drivebus) {
> +               /* Change N_VBUSEN sense pin to DRIVEBUS output pin */
> +               regmap_update_bits(axp20x->regmap, AXP20X_OVER_TMP,
> +                                  AXP22X_MISC_N_VBUSEN_FUNC, 0);
> +               rdev = devm_regulator_register(&pdev->dev,
> +                                              &axp22x_drivebus_regulator,
> +                                              &config);
> +               if (IS_ERR(rdev)) {
> +                       dev_err(&pdev->dev, "Failed to register drivebus\n");
> +                       return PTR_ERR(rdev);
> +               }
> +       }
> +
>         return 0;
>  }
>
> --
> 2.7.4
>

Re: [PATCH] regulator: axp20x: Add support for the (external) drivebus regulator

[Chen-Yu Tsai]

Hi,

On Sun, May 15, 2016 at 1:55 AM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> The axp20x pmics have 2 power inputs, one called ACIN which is intended
> for to be supplied via a powerbarrel on the board and one called VBUS
> which is intended to be supplied via an otg connector.
>
> In the VBUS case the pmic needs to know if the board is supplying power
> to the otg connector, because then it should not take any power from
> its VBUS pin. The axp209 pmic has a N_VBUSEN input pin via which the
> board can signal to the pmic whether the board is supplying power to the
> otg connector or not.
>
> On the axp221/axp223 this pin can alternatively be used as an output
> which controls an external regulator which (optionally) supplies
> power to the otg connector from the board. When the pin is used as
> output it is called DRIVEBUS in the datasheet.

The datasheet actually calls it DRIVEVBUS. DRIVEBUS seems to be some
typo Allwinner has in their code.

>
> This commit adds support for the DRIVEBUS pin as an extra pmic
> controlled regulator. Since this is optional a new x-powers,drivebus dt
> property is added. When this is present the misc-control register is
> written to change the N_VBUSEN input pin to DRIVEBUS output pin mode and
> the extra drivebus regulator is registered with the regulator subsystem.
>
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/mfd/axp20x.txt |  5 ++++
>  drivers/regulator/axp20x-regulator.c             | 30 ++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
> index d20b103..7a88479 100644
> --- a/Documentation/devicetree/bindings/mfd/axp20x.txt
> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
> @@ -22,6 +22,11 @@ Optional properties:
>                       AXP152/20X: range:  750-1875, Default: 1.5 MHz
>                       AXP22X/80X: range: 1800-4050, Default: 3   MHz
>
> +- x-powers,drivebus: axp221 / axp223 only boolean, set this when the N_VBUSEN
> +                  pin is used as an output pin to control an external regulator
> +                  to drive the OTG VBus, rather then as an input pin which
> +                  signals whether the board is driving OTG VBus or not.
> +

You should also add it to the regulators table further down in the binding.
And, as Rob mentioned, x-powers,drive-vbus-en (or drivevbus-en) is better
for a boolean property.

>  - <input>-supply: a phandle to the regulator supply node. May be omitted if
>                   inputs are unregulated, such as using the IPSOUT output
>                   from the PMIC.
> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
> index 514a5e8..2422ddd 100644
> --- a/drivers/regulator/axp20x-regulator.c
> +++ b/drivers/regulator/axp20x-regulator.c
> @@ -36,6 +36,8 @@
>
>  #define AXP20X_FREQ_DCDC_MASK          0x0f
>
> +#define AXP22X_MISC_N_VBUSEN_FUNC      BIT(4)
> +
>  #define AXP_DESC_IO(_family, _id, _match, _supply, _min, _max, _step, _vreg,   \
>                     _vmask, _ereg, _emask, _enable_val, _disable_val)           \
>         [_family##_##_id] = {                                                   \
> @@ -230,6 +232,18 @@ static const struct regulator_desc axp22x_regulators[] = {
>         AXP_DESC_FIXED(AXP22X, RTC_LDO, "rtc_ldo", "ips", 3000),
>  };
>
> +static const struct regulator_desc axp22x_drivebus_regulator = {
> +       .name           = "drivebus",
> +       .supply_name    = "ips",

Given that you are approximating a gpio (in this case, the
N_VBUSEN/DRIVEVBUS pin)
controlled fixed voltage regulator, it would be better if you could:

a) have some way to specify the output voltage, so it looks like a fixed voltage
   regulator that can be turned on/off.

And

b) have a separate supply name for this so it properly propagates enable/disable
   calls. It would also pass the upstream voltage, thus behaving like a switch
   you intended with the axp20x_ops_sw below.


Regards
ChenYu

> +       .of_match       = of_match_ptr("drivebus"),
> +       .regulators_node = of_match_ptr("regulators"),
> +       .type           = REGULATOR_VOLTAGE,
> +       .owner          = THIS_MODULE,
> +       .enable_reg     = AXP20X_VBUS_IPSOUT_MGMT,
> +       .enable_mask    = BIT(2),
> +       .ops            = &axp20x_ops_sw,
> +};
> +
>  static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
>  {
>         struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
> @@ -354,6 +368,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>         u32 workmode;
>         const char *axp22x_dc1_name = axp22x_regulators[AXP22X_DCDC1].name;
>         const char *axp22x_dc5_name = axp22x_regulators[AXP22X_DCDC5].name;
> +       bool drivebus = false;
>
>         switch (axp20x->variant) {
>         case AXP202_ID:
> @@ -365,6 +380,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>         case AXP223_ID:
>                 regulators = axp22x_regulators;
>                 nregulators = AXP22X_REG_ID_MAX;
> +               drivebus = of_property_read_bool(pdev->dev.parent->of_node,
> +                                                "x-powers,drivebus");
>                 break;
>         default:
>                 dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
> @@ -439,6 +456,19 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>                 }
>         }
>
> +       if (drivebus) {
> +               /* Change N_VBUSEN sense pin to DRIVEBUS output pin */
> +               regmap_update_bits(axp20x->regmap, AXP20X_OVER_TMP,
> +                                  AXP22X_MISC_N_VBUSEN_FUNC, 0);
> +               rdev = devm_regulator_register(&pdev->dev,
> +                                              &axp22x_drivebus_regulator,
> +                                              &config);
> +               if (IS_ERR(rdev)) {
> +                       dev_err(&pdev->dev, "Failed to register drivebus\n");
> +                       return PTR_ERR(rdev);
> +               }
> +       }
> +
>         return 0;
>  }
>
> --
> 2.7.4
>

[PATCH] regulator: axp20x: Add support for the (external) drivebus regulator

[Chen-Yu Tsai]

Hi,

On Sun, May 15, 2016 at 1:55 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> The axp20x pmics have 2 power inputs, one called ACIN which is intended
> for to be supplied via a powerbarrel on the board and one called VBUS
> which is intended to be supplied via an otg connector.
>
> In the VBUS case the pmic needs to know if the board is supplying power
> to the otg connector, because then it should not take any power from
> its VBUS pin. The axp209 pmic has a N_VBUSEN input pin via which the
> board can signal to the pmic whether the board is supplying power to the
> otg connector or not.
>
> On the axp221/axp223 this pin can alternatively be used as an output
> which controls an external regulator which (optionally) supplies
> power to the otg connector from the board. When the pin is used as
> output it is called DRIVEBUS in the datasheet.

The datasheet actually calls it DRIVEVBUS. DRIVEBUS seems to be some
typo Allwinner has in their code.

>
> This commit adds support for the DRIVEBUS pin as an extra pmic
> controlled regulator. Since this is optional a new x-powers,drivebus dt
> property is added. When this is present the misc-control register is
> written to change the N_VBUSEN input pin to DRIVEBUS output pin mode and
> the extra drivebus regulator is registered with the regulator subsystem.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  Documentation/devicetree/bindings/mfd/axp20x.txt |  5 ++++
>  drivers/regulator/axp20x-regulator.c             | 30 ++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
> index d20b103..7a88479 100644
> --- a/Documentation/devicetree/bindings/mfd/axp20x.txt
> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
> @@ -22,6 +22,11 @@ Optional properties:
>                       AXP152/20X: range:  750-1875, Default: 1.5 MHz
>                       AXP22X/80X: range: 1800-4050, Default: 3   MHz
>
> +- x-powers,drivebus: axp221 / axp223 only boolean, set this when the N_VBUSEN
> +                  pin is used as an output pin to control an external regulator
> +                  to drive the OTG VBus, rather then as an input pin which
> +                  signals whether the board is driving OTG VBus or not.
> +

You should also add it to the regulators table further down in the binding.
And, as Rob mentioned, x-powers,drive-vbus-en (or drivevbus-en) is better
for a boolean property.

>  - <input>-supply: a phandle to the regulator supply node. May be omitted if
>                   inputs are unregulated, such as using the IPSOUT output
>                   from the PMIC.
> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
> index 514a5e8..2422ddd 100644
> --- a/drivers/regulator/axp20x-regulator.c
> +++ b/drivers/regulator/axp20x-regulator.c
> @@ -36,6 +36,8 @@
>
>  #define AXP20X_FREQ_DCDC_MASK          0x0f
>
> +#define AXP22X_MISC_N_VBUSEN_FUNC      BIT(4)
> +
>  #define AXP_DESC_IO(_family, _id, _match, _supply, _min, _max, _step, _vreg,   \
>                     _vmask, _ereg, _emask, _enable_val, _disable_val)           \
>         [_family##_##_id] = {                                                   \
> @@ -230,6 +232,18 @@ static const struct regulator_desc axp22x_regulators[] = {
>         AXP_DESC_FIXED(AXP22X, RTC_LDO, "rtc_ldo", "ips", 3000),
>  };
>
> +static const struct regulator_desc axp22x_drivebus_regulator = {
> +       .name           = "drivebus",
> +       .supply_name    = "ips",

Given that you are approximating a gpio (in this case, the
N_VBUSEN/DRIVEVBUS pin)
controlled fixed voltage regulator, it would be better if you could:

a) have some way to specify the output voltage, so it looks like a fixed voltage
   regulator that can be turned on/off.

And

b) have a separate supply name for this so it properly propagates enable/disable
   calls. It would also pass the upstream voltage, thus behaving like a switch
   you intended with the axp20x_ops_sw below.


Regards
ChenYu

> +       .of_match       = of_match_ptr("drivebus"),
> +       .regulators_node = of_match_ptr("regulators"),
> +       .type           = REGULATOR_VOLTAGE,
> +       .owner          = THIS_MODULE,
> +       .enable_reg     = AXP20X_VBUS_IPSOUT_MGMT,
> +       .enable_mask    = BIT(2),
> +       .ops            = &axp20x_ops_sw,
> +};
> +
>  static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
>  {
>         struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
> @@ -354,6 +368,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>         u32 workmode;
>         const char *axp22x_dc1_name = axp22x_regulators[AXP22X_DCDC1].name;
>         const char *axp22x_dc5_name = axp22x_regulators[AXP22X_DCDC5].name;
> +       bool drivebus = false;
>
>         switch (axp20x->variant) {
>         case AXP202_ID:
> @@ -365,6 +380,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>         case AXP223_ID:
>                 regulators = axp22x_regulators;
>                 nregulators = AXP22X_REG_ID_MAX;
> +               drivebus = of_property_read_bool(pdev->dev.parent->of_node,
> +                                                "x-powers,drivebus");
>                 break;
>         default:
>                 dev_err(&pdev->dev, "Unsupported AXP variant: %ld\n",
> @@ -439,6 +456,19 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
>                 }
>         }
>
> +       if (drivebus) {
> +               /* Change N_VBUSEN sense pin to DRIVEBUS output pin */
> +               regmap_update_bits(axp20x->regmap, AXP20X_OVER_TMP,
> +                                  AXP22X_MISC_N_VBUSEN_FUNC, 0);
> +               rdev = devm_regulator_register(&pdev->dev,
> +                                              &axp22x_drivebus_regulator,
> +                                              &config);
> +               if (IS_ERR(rdev)) {
> +                       dev_err(&pdev->dev, "Failed to register drivebus\n");
> +                       return PTR_ERR(rdev);
> +               }
> +       }
> +
>         return 0;
>  }
>
> --
> 2.7.4
>

Re: [linux-sunxi] Re: [PATCH] regulator: axp20x: Add support for the (external) drivebus regulator

[Hans de Goede]

Hi,

On 16-05-16 19:06, Chen-Yu Tsai wrote:
> Hi,
>
> On Sun, May 15, 2016 at 1:55 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> The axp20x pmics have 2 power inputs, one called ACIN which is intended
>> for to be supplied via a powerbarrel on the board and one called VBUS
>> which is intended to be supplied via an otg connector.
>>
>> In the VBUS case the pmic needs to know if the board is supplying power
>> to the otg connector, because then it should not take any power from
>> its VBUS pin. The axp209 pmic has a N_VBUSEN input pin via which the
>> board can signal to the pmic whether the board is supplying power to the
>> otg connector or not.
>>
>> On the axp221/axp223 this pin can alternatively be used as an output
>> which controls an external regulator which (optionally) supplies
>> power to the otg connector from the board. When the pin is used as
>> output it is called DRIVEBUS in the datasheet.
>
> The datasheet actually calls it DRIVEVBUS. DRIVEBUS seems to be some
> typo Allwinner has in their code.

Fixed for v2.

>> This commit adds support for the DRIVEBUS pin as an extra pmic
>> controlled regulator. Since this is optional a new x-powers,drivebus dt
>> property is added. When this is present the misc-control register is
>> written to change the N_VBUSEN input pin to DRIVEBUS output pin mode and
>> the extra drivebus regulator is registered with the regulator subsystem.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  Documentation/devicetree/bindings/mfd/axp20x.txt |  5 ++++
>>  drivers/regulator/axp20x-regulator.c             | 30 ++++++++++++++++++++++++
>>  2 files changed, 35 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
>> index d20b103..7a88479 100644
>> --- a/Documentation/devicetree/bindings/mfd/axp20x.txt
>> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
>> @@ -22,6 +22,11 @@ Optional properties:
>>                       AXP152/20X: range:  750-1875, Default: 1.5 MHz
>>                       AXP22X/80X: range: 1800-4050, Default: 3   MHz
>>
>> +- x-powers,drivebus: axp221 / axp223 only boolean, set this when the N_VBUSEN
>> +                  pin is used as an output pin to control an external regulator
>> +                  to drive the OTG VBus, rather then as an input pin which
>> +                  signals whether the board is driving OTG VBus or not.
>> +
>
> You should also add it to the regulators table further down in the binding.
> And, as Rob mentioned, x-powers,drive-vbus-en (or drivevbus-en) is better
> for a boolean property.

Both fixed for v2.

>>  - <input>-supply: a phandle to the regulator supply node. May be omitted if
>>                   inputs are unregulated, such as using the IPSOUT output
>>                   from the PMIC.
>> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
>> index 514a5e8..2422ddd 100644
>> --- a/drivers/regulator/axp20x-regulator.c
>> +++ b/drivers/regulator/axp20x-regulator.c
>> @@ -36,6 +36,8 @@
>>
>>  #define AXP20X_FREQ_DCDC_MASK          0x0f
>>
>> +#define AXP22X_MISC_N_VBUSEN_FUNC      BIT(4)
>> +
>>  #define AXP_DESC_IO(_family, _id, _match, _supply, _min, _max, _step, _vreg,   \
>>                     _vmask, _ereg, _emask, _enable_val, _disable_val)           \
>>         [_family##_##_id] = {                                                   \
>> @@ -230,6 +232,18 @@ static const struct regulator_desc axp22x_regulators[] = {
>>         AXP_DESC_FIXED(AXP22X, RTC_LDO, "rtc_ldo", "ips", 3000),
>>  };
>>
>> +static const struct regulator_desc axp22x_drivebus_regulator = {
>> +       .name           = "drivebus",
>> +       .supply_name    = "ips",
>
> Given that you are approximating a gpio (in this case, the
> N_VBUSEN/DRIVEVBUS pin)
> controlled fixed voltage regulator, it would be better if you could:
>
> a) have some way to specify the output voltage, so it looks like a fixed voltage
>    regulator that can be turned on/off.

This is fixed by your suggestion b) actually, if no get_voltage* ops are specified,
it will inherit the voltage of it supply.

> And
>
> b) have a separate supply name for this so it properly propagates enable/disable
>    calls. It would also pass the upstream voltage, thus behaving like a switch
>    you intended with the axp20x_ops_sw below.

Ack, fixed for v2.

Thanks for the review.

Regards,

Hans

Re: Re: [PATCH] regulator: axp20x: Add support for the (external) drivebus regulator

[Hans de Goede]

Hi,

On 16-05-16 19:06, Chen-Yu Tsai wrote:
> Hi,
>
> On Sun, May 15, 2016 at 1:55 AM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> The axp20x pmics have 2 power inputs, one called ACIN which is intended
>> for to be supplied via a powerbarrel on the board and one called VBUS
>> which is intended to be supplied via an otg connector.
>>
>> In the VBUS case the pmic needs to know if the board is supplying power
>> to the otg connector, because then it should not take any power from
>> its VBUS pin. The axp209 pmic has a N_VBUSEN input pin via which the
>> board can signal to the pmic whether the board is supplying power to the
>> otg connector or not.
>>
>> On the axp221/axp223 this pin can alternatively be used as an output
>> which controls an external regulator which (optionally) supplies
>> power to the otg connector from the board. When the pin is used as
>> output it is called DRIVEBUS in the datasheet.
>
> The datasheet actually calls it DRIVEVBUS. DRIVEBUS seems to be some
> typo Allwinner has in their code.

Fixed for v2.

>> This commit adds support for the DRIVEBUS pin as an extra pmic
>> controlled regulator. Since this is optional a new x-powers,drivebus dt
>> property is added. When this is present the misc-control register is
>> written to change the N_VBUSEN input pin to DRIVEBUS output pin mode and
>> the extra drivebus regulator is registered with the regulator subsystem.
>>
>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/mfd/axp20x.txt |  5 ++++
>>  drivers/regulator/axp20x-regulator.c             | 30 ++++++++++++++++++++++++
>>  2 files changed, 35 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
>> index d20b103..7a88479 100644
>> --- a/Documentation/devicetree/bindings/mfd/axp20x.txt
>> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
>> @@ -22,6 +22,11 @@ Optional properties:
>>                       AXP152/20X: range:  750-1875, Default: 1.5 MHz
>>                       AXP22X/80X: range: 1800-4050, Default: 3   MHz
>>
>> +- x-powers,drivebus: axp221 / axp223 only boolean, set this when the N_VBUSEN
>> +                  pin is used as an output pin to control an external regulator
>> +                  to drive the OTG VBus, rather then as an input pin which
>> +                  signals whether the board is driving OTG VBus or not.
>> +
>
> You should also add it to the regulators table further down in the binding.
> And, as Rob mentioned, x-powers,drive-vbus-en (or drivevbus-en) is better
> for a boolean property.

Both fixed for v2.

>>  - <input>-supply: a phandle to the regulator supply node. May be omitted if
>>                   inputs are unregulated, such as using the IPSOUT output
>>                   from the PMIC.
>> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
>> index 514a5e8..2422ddd 100644
>> --- a/drivers/regulator/axp20x-regulator.c
>> +++ b/drivers/regulator/axp20x-regulator.c
>> @@ -36,6 +36,8 @@
>>
>>  #define AXP20X_FREQ_DCDC_MASK          0x0f
>>
>> +#define AXP22X_MISC_N_VBUSEN_FUNC      BIT(4)
>> +
>>  #define AXP_DESC_IO(_family, _id, _match, _supply, _min, _max, _step, _vreg,   \
>>                     _vmask, _ereg, _emask, _enable_val, _disable_val)           \
>>         [_family##_##_id] = {                                                   \
>> @@ -230,6 +232,18 @@ static const struct regulator_desc axp22x_regulators[] = {
>>         AXP_DESC_FIXED(AXP22X, RTC_LDO, "rtc_ldo", "ips", 3000),
>>  };
>>
>> +static const struct regulator_desc axp22x_drivebus_regulator = {
>> +       .name           = "drivebus",
>> +       .supply_name    = "ips",
>
> Given that you are approximating a gpio (in this case, the
> N_VBUSEN/DRIVEVBUS pin)
> controlled fixed voltage regulator, it would be better if you could:
>
> a) have some way to specify the output voltage, so it looks like a fixed voltage
>    regulator that can be turned on/off.

This is fixed by your suggestion b) actually, if no get_voltage* ops are specified,
it will inherit the voltage of it supply.

> And
>
> b) have a separate supply name for this so it properly propagates enable/disable
>    calls. It would also pass the upstream voltage, thus behaving like a switch
>    you intended with the axp20x_ops_sw below.

Ack, fixed for v2.

Thanks for the review.

Regards,

Hans

[linux-sunxi] Re: [PATCH] regulator: axp20x: Add support for the (external) drivebus regulator

[Hans de Goede]

Hi,

On 16-05-16 19:06, Chen-Yu Tsai wrote:
> Hi,
>
> On Sun, May 15, 2016 at 1:55 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> The axp20x pmics have 2 power inputs, one called ACIN which is intended
>> for to be supplied via a powerbarrel on the board and one called VBUS
>> which is intended to be supplied via an otg connector.
>>
>> In the VBUS case the pmic needs to know if the board is supplying power
>> to the otg connector, because then it should not take any power from
>> its VBUS pin. The axp209 pmic has a N_VBUSEN input pin via which the
>> board can signal to the pmic whether the board is supplying power to the
>> otg connector or not.
>>
>> On the axp221/axp223 this pin can alternatively be used as an output
>> which controls an external regulator which (optionally) supplies
>> power to the otg connector from the board. When the pin is used as
>> output it is called DRIVEBUS in the datasheet.
>
> The datasheet actually calls it DRIVEVBUS. DRIVEBUS seems to be some
> typo Allwinner has in their code.

Fixed for v2.

>> This commit adds support for the DRIVEBUS pin as an extra pmic
>> controlled regulator. Since this is optional a new x-powers,drivebus dt
>> property is added. When this is present the misc-control register is
>> written to change the N_VBUSEN input pin to DRIVEBUS output pin mode and
>> the extra drivebus regulator is registered with the regulator subsystem.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  Documentation/devicetree/bindings/mfd/axp20x.txt |  5 ++++
>>  drivers/regulator/axp20x-regulator.c             | 30 ++++++++++++++++++++++++
>>  2 files changed, 35 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
>> index d20b103..7a88479 100644
>> --- a/Documentation/devicetree/bindings/mfd/axp20x.txt
>> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
>> @@ -22,6 +22,11 @@ Optional properties:
>>                       AXP152/20X: range:  750-1875, Default: 1.5 MHz
>>                       AXP22X/80X: range: 1800-4050, Default: 3   MHz
>>
>> +- x-powers,drivebus: axp221 / axp223 only boolean, set this when the N_VBUSEN
>> +                  pin is used as an output pin to control an external regulator
>> +                  to drive the OTG VBus, rather then as an input pin which
>> +                  signals whether the board is driving OTG VBus or not.
>> +
>
> You should also add it to the regulators table further down in the binding.
> And, as Rob mentioned, x-powers,drive-vbus-en (or drivevbus-en) is better
> for a boolean property.

Both fixed for v2.

>>  - <input>-supply: a phandle to the regulator supply node. May be omitted if
>>                   inputs are unregulated, such as using the IPSOUT output
>>                   from the PMIC.
>> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
>> index 514a5e8..2422ddd 100644
>> --- a/drivers/regulator/axp20x-regulator.c
>> +++ b/drivers/regulator/axp20x-regulator.c
>> @@ -36,6 +36,8 @@
>>
>>  #define AXP20X_FREQ_DCDC_MASK          0x0f
>>
>> +#define AXP22X_MISC_N_VBUSEN_FUNC      BIT(4)
>> +
>>  #define AXP_DESC_IO(_family, _id, _match, _supply, _min, _max, _step, _vreg,   \
>>                     _vmask, _ereg, _emask, _enable_val, _disable_val)           \
>>         [_family##_##_id] = {                                                   \
>> @@ -230,6 +232,18 @@ static const struct regulator_desc axp22x_regulators[] = {
>>         AXP_DESC_FIXED(AXP22X, RTC_LDO, "rtc_ldo", "ips", 3000),
>>  };
>>
>> +static const struct regulator_desc axp22x_drivebus_regulator = {
>> +       .name           = "drivebus",
>> +       .supply_name    = "ips",
>
> Given that you are approximating a gpio (in this case, the
> N_VBUSEN/DRIVEVBUS pin)
> controlled fixed voltage regulator, it would be better if you could:
>
> a) have some way to specify the output voltage, so it looks like a fixed voltage
>    regulator that can be turned on/off.

This is fixed by your suggestion b) actually, if no get_voltage* ops are specified,
it will inherit the voltage of it supply.

> And
>
> b) have a separate supply name for this so it properly propagates enable/disable
>    calls. It would also pass the upstream voltage, thus behaving like a switch
>    you intended with the axp20x_ops_sw below.

Ack, fixed for v2.

Thanks for the review.

Regards,

Hans