diff mbox

[6/7] regulator: AXP20x: Add support for regulators subsystem

Message ID 1393692352-10839-7-git-send-email-carlo@caione.org (mailing list archive)
State New, archived
Headers show

Commit Message

Carlo Caione March 1, 2014, 4:45 p.m. UTC
AXP202 and AXP209 come with two synchronous step-down DC-DCs and five
LDOs. This patch introduces basic support for those regulators.

Signed-off-by: Carlo Caione <carlo@caione.org>
---
 arch/arm/configs/sunxi_defconfig     |   1 +
 drivers/regulator/Kconfig            |   7 +
 drivers/regulator/Makefile           |   1 +
 drivers/regulator/axp20x-regulator.c | 349 +++++++++++++++++++++++++++++++++++
 4 files changed, 358 insertions(+)
 create mode 100644 drivers/regulator/axp20x-regulator.c

Comments

Mark Brown March 3, 2014, 1:56 a.m. UTC | #1
On Sat, Mar 01, 2014 at 05:45:51PM +0100, Carlo Caione wrote:

> index d59c826..0cef101 100644
> --- a/arch/arm/configs/sunxi_defconfig
> +++ b/arch/arm/configs/sunxi_defconfig
> @@ -69,3 +69,4 @@ CONFIG_NFS_FS=y
>  CONFIG_ROOT_NFS=y
>  CONFIG_NLS=y
>  CONFIG_PRINTK_TIME=y
> +CONFIG_REGULATOR_AXP20X=y

If you want to update the defconfig do it in a separate patch.

> +static int axp20x_set_suspend_voltage(struct regulator_dev *rdev, int uV)
> +{
> +	return regulator_set_voltage_sel_regmap(rdev, 0);
> +}

I'm not entirely clear what this is supposed to do but it's broken - it
both ignores the voltage that is passed in and immediately writes to the
normal voltage select register which will presumably distrupt the system.

> +static int axp20x_io_enable_regmap(struct regulator_dev *rdev)
> +{
> +	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
> +				  rdev->desc->enable_mask, AXP20X_IO_ENABLED);
> +}

Please make some helpers for this (or extend the existing ones to cope
with more than a single bit control) - there's several other devices
which have similar multi-bit controls so we should factor this out
rather than open coding.

> +	np = of_node_get(pdev->dev.parent->of_node);
> +	if (!np)
> +		return 0;
> +
> +	regulators = of_find_node_by_name(np, "regulators");
> +	if (!regulators) {
> +		dev_err(&pdev->dev, "regulators node not found\n");
> +		return -EINVAL;
> +	}

The driver should be able to start up with no configuration provided at
all except for the device being registered - the user won't be able to
change anything but they will be able to read the current state of the
hardware which can be useful for diagnostics.

> +static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 workmode)
> +{
> +	unsigned int mask = AXP20X_WORKMODE_DCDC2_MASK;
> +
> +	if ((id != AXP20X_DCDC2) && (id != AXP20X_DCDC3))
> +		return -EINVAL;
> +
> +	if (id == AXP20X_DCDC3)
> +		mask = AXP20X_WORKMODE_DCDC3_MASK;
> +
> +	return regmap_update_bits(rdev->regmap, AXP20X_DCDC_MODE, mask, workmode);
> +}

What is a workmode?

> +		pmic->rdev[i] = regulator_register(&pmic->rdesc[i], &config);
> +		if (IS_ERR(pmic->rdev[i])) {

Use devm_regulator_register() and then you don't need to clean up the
regulators either.

> +static int __init axp20x_regulator_init(void)
> +{
> +	return platform_driver_register(&axp20x_regulator_driver);
> +}
> +
> +subsys_initcall(axp20x_regulator_init);

module_platform_driver().
Carlo Caione March 4, 2014, 8:56 p.m. UTC | #2
On Mon, Mar 3, 2014 at 2:56 AM, Mark Brown <broonie@kernel.org> wrote:
> On Sat, Mar 01, 2014 at 05:45:51PM +0100, Carlo Caione wrote:
>
>> index d59c826..0cef101 100644
>> --- a/arch/arm/configs/sunxi_defconfig
>> +++ b/arch/arm/configs/sunxi_defconfig
>> @@ -69,3 +69,4 @@ CONFIG_NFS_FS=y
>>  CONFIG_ROOT_NFS=y
>>  CONFIG_NLS=y
>>  CONFIG_PRINTK_TIME=y
>> +CONFIG_REGULATOR_AXP20X=y
>
> If you want to update the defconfig do it in a separate patch.

Ok, I'll split the patch.

>> +static int axp20x_set_suspend_voltage(struct regulator_dev *rdev, int uV)
>> +{
>> +     return regulator_set_voltage_sel_regmap(rdev, 0);
>> +}

Right. I'll fix it in v2.

> I'm not entirely clear what this is supposed to do but it's broken - it
> both ignores the voltage that is passed in and immediately writes to the
> normal voltage select register which will presumably distrupt the system.
>
>> +static int axp20x_io_enable_regmap(struct regulator_dev *rdev)
>> +{
>> +     return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
>> +                               rdev->desc->enable_mask, AXP20X_IO_ENABLED);
>> +}
>
> Please make some helpers for this (or extend the existing ones to cope
> with more than a single bit control) - there's several other devices
> which have similar multi-bit controls so we should factor this out
> rather than open coding.

No prob, I'll submit a patch for the helpers before submitting the v2
of this patchset

>> +     np = of_node_get(pdev->dev.parent->of_node);
>> +     if (!np)
>> +             return 0;
>> +
>> +     regulators = of_find_node_by_name(np, "regulators");
>> +     if (!regulators) {
>> +             dev_err(&pdev->dev, "regulators node not found\n");
>> +             return -EINVAL;
>> +     }
>
> The driver should be able to start up with no configuration provided at
> all except for the device being registered - the user won't be able to
> change anything but they will be able to read the current state of the
> hardware which can be useful for diagnostics.

seems reasonable

>> +static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 workmode)
>> +{
>> +     unsigned int mask = AXP20X_WORKMODE_DCDC2_MASK;
>> +
>> +     if ((id != AXP20X_DCDC2) && (id != AXP20X_DCDC3))
>> +             return -EINVAL;
>> +
>> +     if (id == AXP20X_DCDC3)
>> +             mask = AXP20X_WORKMODE_DCDC3_MASK;
>> +
>> +     return regmap_update_bits(rdev->regmap, AXP20X_DCDC_MODE, mask, workmode);
>> +}
>
> What is a workmode?

It defines if the output from DCDC2 or DCDC3 (that can be used as PWM chargers).
I'll document it better in the bindings documentation.

>> +             pmic->rdev[i] = regulator_register(&pmic->rdesc[i], &config);
>> +             if (IS_ERR(pmic->rdev[i])) {
>
> Use devm_regulator_register() and then you don't need to clean up the
> regulators either.
>
>> +static int __init axp20x_regulator_init(void)
>> +{
>> +     return platform_driver_register(&axp20x_regulator_driver);
>> +}
>> +
>> +subsys_initcall(axp20x_regulator_init);
>
> module_platform_driver().

I'll fix both in v2.

Thank you,
Maxime Ripard March 7, 2014, 6:22 p.m. UTC | #3
On Sat, Mar 01, 2014 at 05:45:51PM +0100, Carlo Caione wrote:
> +static struct platform_driver axp20x_regulator_driver = {
> +	.probe	= axp20x_regulator_probe,
> +	.remove	= axp20x_regulator_remove,
> +	.driver	= {
> +		.name		= "axp20x-regulator",
> +		.owner		= THIS_MODULE,
> +	},
> +};
> +
> +static int __init axp20x_regulator_init(void)
> +{
> +	return platform_driver_register(&axp20x_regulator_driver);
> +}
> +
> +subsys_initcall(axp20x_regulator_init);

Hmmm, really?

I thought the AXP was only connected through I2C? How is that a
platform device then?
Carlo Caione March 8, 2014, 11:43 a.m. UTC | #4
On Fri, Mar 7, 2014 at 7:22 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Sat, Mar 01, 2014 at 05:45:51PM +0100, Carlo Caione wrote:
>> +static struct platform_driver axp20x_regulator_driver = {
>> +     .probe  = axp20x_regulator_probe,
>> +     .remove = axp20x_regulator_remove,
>> +     .driver = {
>> +             .name           = "axp20x-regulator",
>> +             .owner          = THIS_MODULE,
>> +     },
>> +};
>> +
>> +static int __init axp20x_regulator_init(void)
>> +{
>> +     return platform_driver_register(&axp20x_regulator_driver);
>> +}
>> +
>> +subsys_initcall(axp20x_regulator_init);
>
> Hmmm, really?
>
> I thought the AXP was only connected through I2C? How is that a
> platform device then?

Not really. It is plain wrong.
I'll fix it in v2.

Thanks,
Mark Brown March 9, 2014, 7:51 a.m. UTC | #5
On Sat, Mar 08, 2014 at 12:43:04PM +0100, Carlo Caione wrote:
> On Fri, Mar 7, 2014 at 7:22 PM, Maxime Ripard
> > On Sat, Mar 01, 2014 at 05:45:51PM +0100, Carlo Caione wrote:

> >> +     return platform_driver_register(&axp20x_regulator_driver);

> > I thought the AXP was only connected through I2C? How is that a
> > platform device then?

> Not really. It is plain wrong.
> I'll fix it in v2.

Are you sure this is wrong?  For MFDs the core MFD is a driver of
whatever bus type is in use but the function drivers are platform
devices which talk to the hardware via the core device.
Carlo Caione March 9, 2014, 8:56 a.m. UTC | #6
On Sun, Mar 09, 2014 at 07:51:48AM +0000, Mark Brown wrote:
> On Sat, Mar 08, 2014 at 12:43:04PM +0100, Carlo Caione wrote:
> > On Fri, Mar 7, 2014 at 7:22 PM, Maxime Ripard
> > > On Sat, Mar 01, 2014 at 05:45:51PM +0100, Carlo Caione wrote:
> 
> > >> +     return platform_driver_register(&axp20x_regulator_driver);
> 
> > > I thought the AXP was only connected through I2C? How is that a
> > > platform device then?
> 
> > Not really. It is plain wrong.
> > I'll fix it in v2.
> 
> Are you sure this is wrong?  For MFDs the core MFD is a driver of
> whatever bus type is in use but the function drivers are platform
> devices which talk to the hardware via the core device.

ok, I spoke too soon. Sorry for the noise.
Carlo Caione March 11, 2014, 7:24 p.m. UTC | #7
On Mon, Mar 03, 2014 at 10:56:16AM +0900, Mark Brown wrote:

<snip>
 
> > +	np = of_node_get(pdev->dev.parent->of_node);
> > +	if (!np)
> > +		return 0;
> > +
> > +	regulators = of_find_node_by_name(np, "regulators");
> > +	if (!regulators) {
> > +		dev_err(&pdev->dev, "regulators node not found\n");
> > +		return -EINVAL;
> > +	}
> 
> The driver should be able to start up with no configuration provided at
> all except for the device being registered - the user won't be able to
> change anything but they will be able to read the current state of the
> hardware which can be useful for diagnostics.

If the device is DT enabled and no configuration is provided for
regulators, these are disabled according to:

http://lxr.linux.no/linux+v3.13.5/drivers/regulator/core.c#L3797

Am I missing something or do you have any pointer?

Thanks,
Mark Brown March 11, 2014, 7:29 p.m. UTC | #8
On Tue, Mar 11, 2014 at 08:24:11PM +0100, Carlo Caione wrote:
> On Mon, Mar 03, 2014 at 10:56:16AM +0900, Mark Brown wrote:

> > > +	regulators = of_find_node_by_name(np, "regulators");
> > > +	if (!regulators) {
> > > +		dev_err(&pdev->dev, "regulators node not found\n");
> > > +		return -EINVAL;
> > > +	}

> > The driver should be able to start up with no configuration provided at
> > all except for the device being registered - the user won't be able to
> > change anything but they will be able to read the current state of the
> > hardware which can be useful for diagnostics.

> If the device is DT enabled and no configuration is provided for
> regulators, these are disabled according to:

> http://lxr.linux.no/linux+v3.13.5/drivers/regulator/core.c#L3797

> Am I missing something or do you have any pointer?

With the above code the driver will return an error and never get as far
as registering the regulator.
Carlo Caione March 11, 2014, 9:06 p.m. UTC | #9
On Tue, Mar 11, 2014 at 07:29:40PM +0000, Mark Brown wrote:
> On Tue, Mar 11, 2014 at 08:24:11PM +0100, Carlo Caione wrote:
> > On Mon, Mar 03, 2014 at 10:56:16AM +0900, Mark Brown wrote:
> 
> > > > +	regulators = of_find_node_by_name(np, "regulators");
> > > > +	if (!regulators) {
> > > > +		dev_err(&pdev->dev, "regulators node not found\n");
> > > > +		return -EINVAL;
> > > > +	}
> 
> > > The driver should be able to start up with no configuration provided at
> > > all except for the device being registered - the user won't be able to
> > > change anything but they will be able to read the current state of the
> > > hardware which can be useful for diagnostics.
> 
> > If the device is DT enabled and no configuration is provided for
> > regulators, these are disabled according to:
> 
> > http://lxr.linux.no/linux+v3.13.5/drivers/regulator/core.c#L3797
> 
> > Am I missing something or do you have any pointer?
> 
> With the above code the driver will return an error and never get as far
> as registering the regulator.

I agree, but if I register the regulators without having the constrains
defined in the DT, when regulator_init_complete() is
called, the regulators are disabled powering off the SoC (at least for
DCDC2 and DCDC3).

Thanks,
Mark Brown March 12, 2014, 12:38 a.m. UTC | #10
On Tue, Mar 11, 2014 at 10:06:59PM +0100, Carlo Caione wrote:
> On Tue, Mar 11, 2014 at 07:29:40PM +0000, Mark Brown wrote:

> > With the above code the driver will return an error and never get as far
> > as registering the regulator.

> I agree, but if I register the regulators without having the constrains
> defined in the DT, when regulator_init_complete() is
> called, the regulators are disabled powering off the SoC (at least for
> DCDC2 and DCDC3).

Right, but the kernel will say what it's doing before it does so and
then the user can provide the appropriate setup in DT to force things
on.
diff mbox

Patch

diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
index d59c826..0cef101 100644
--- a/arch/arm/configs/sunxi_defconfig
+++ b/arch/arm/configs/sunxi_defconfig
@@ -69,3 +69,4 @@  CONFIG_NFS_FS=y
 CONFIG_ROOT_NFS=y
 CONFIG_NLS=y
 CONFIG_PRINTK_TIME=y
+CONFIG_REGULATOR_AXP20X=y
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index ce785f4..4cf9e32 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -131,6 +131,13 @@  config REGULATOR_AS3722
 	  AS3722 PMIC. This will enable support for all the software
 	  controllable DCDC/LDO regulators.
 
+config REGULATOR_AXP20X
+	tristate "X-POWERS AXP20X PMIC Regulators"
+	depends on MFD_AXP20X
+	help
+	  This driver provides support for the voltage regulators on the
+	  AXP20X PMIC.
+
 config REGULATOR_DA903X
 	tristate "Dialog Semiconductor DA9030/DA9034 regulators"
 	depends on PMIC_DA903X
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 01c597e..2cf4646 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -19,6 +19,7 @@  obj-$(CONFIG_REGULATOR_ANATOP) += anatop-regulator.o
 obj-$(CONFIG_REGULATOR_ARIZONA) += arizona-micsupp.o arizona-ldo1.o
 obj-$(CONFIG_REGULATOR_AS3711) += as3711-regulator.o
 obj-$(CONFIG_REGULATOR_AS3722) += as3722-regulator.o
+obj-$(CONFIG_REGULATOR_AXP20X) += axp20x-regulator.o
 obj-$(CONFIG_REGULATOR_DA903X)	+= da903x.o
 obj-$(CONFIG_REGULATOR_DA9052)	+= da9052-regulator.o
 obj-$(CONFIG_REGULATOR_DA9055)	+= da9055-regulator.o
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
new file mode 100644
index 0000000..9072f2f
--- /dev/null
+++ b/drivers/regulator/axp20x-regulator.c
@@ -0,0 +1,349 @@ 
+/*
+ * axp20x regulators driver.
+ *
+ * Copyright (C) 2013 Carlo Caione <carlo@caione.org>
+ *
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License. See the file "COPYING" in the main directory of this
+ * archive for more details.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/mfd/axp20x.h>
+#include <linux/regmap.h>
+
+#define AXP20X_IO_ENABLED		(0x03)
+
+#define AXP20X_WORKMODE_DCDC2_MASK	BIT(2)
+#define AXP20X_WORKMODE_DCDC3_MASK	BIT(1)
+
+#define AXP20X_FREQ_DCDC_MASK		(0x0f)
+
+struct axp20x_regulators {
+	struct regulator_desc	rdesc[AXP20X_REG_ID_MAX];
+	struct regulator_dev	*rdev[AXP20X_REG_ID_MAX];
+	struct axp20x_dev	*axp20x;
+};
+
+#define AXP20X_DESC(_id, _min, _max, _step, _vreg, _vmask, _ereg, _emask) 	\
+	[AXP20X_##_id] = {							\
+		.name		= #_id,						\
+		.type		= REGULATOR_VOLTAGE,				\
+		.id		= AXP20X_##_id,					\
+		.n_voltages	= (((_max) - (_min)) / (_step) + 1),		\
+		.owner		= THIS_MODULE,					\
+		.min_uV		= (_min) * 1000,				\
+		.uV_step	= (_step) * 1000,				\
+		.vsel_reg	= (_vreg),					\
+		.vsel_mask	= (_vmask),					\
+		.enable_reg	= (_ereg),					\
+		.enable_mask	= (_emask),					\
+		.ops		= &axp20x_ops,					\
+	}
+
+#define AXP20X_DESC_IO(_id, _min, _max, _step, _vreg, _vmask, _ereg, _emask)	\
+	[AXP20X_##_id] = {							\
+		.name		= #_id,						\
+		.type		= REGULATOR_VOLTAGE,				\
+		.id		= AXP20X_##_id,					\
+		.n_voltages	= (((_max) - (_min)) / (_step) + 1),		\
+		.owner		= THIS_MODULE,					\
+		.min_uV		= (_min) * 1000,				\
+		.uV_step	= (_step) * 1000,				\
+		.vsel_reg	= (_vreg),					\
+		.vsel_mask	= (_vmask),					\
+		.enable_reg	= (_ereg),					\
+		.enable_mask	= (_emask),					\
+		.ops		= &axp20x_ops_io,				\
+	}
+
+#define AXP20X_DESC_FIXED(_id, _volt)						\
+	[AXP20X_##_id] = {							\
+		.name		= #_id,						\
+		.type		= REGULATOR_VOLTAGE,				\
+		.id		= AXP20X_##_id,					\
+		.n_voltages	= 1,						\
+		.owner		= THIS_MODULE,					\
+		.min_uV		= (_volt) * 1000,				\
+		.ops		= &axp20x_ops,					\
+	}
+
+#define AXP20X_DESC_TABLE(_id, _table, _vreg, _vmask, _ereg, _emask)		\
+	[AXP20X_##_id] = {							\
+		.name		= #_id,						\
+		.type		= REGULATOR_VOLTAGE,				\
+		.id		= AXP20X_##_id,					\
+		.n_voltages	= ARRAY_SIZE(_table),				\
+		.owner		= THIS_MODULE,					\
+		.vsel_reg	= (_vreg),					\
+		.vsel_mask	= (_vmask),					\
+		.enable_reg	= (_ereg),					\
+		.enable_mask	= (_emask),					\
+		.volt_table	= (_table),					\
+		.ops		= &axp20x_ops_table,				\
+	}
+
+static int axp20x_ldo4_data[] = { 1250000, 1300000, 1400000, 1500000, 1600000, 1700000,
+				  1800000, 1900000, 2000000, 2500000, 2700000, 2800000,
+				  3000000, 3100000, 3200000, 3300000 };
+
+static int axp20x_set_suspend_voltage(struct regulator_dev *rdev, int uV)
+{
+	return regulator_set_voltage_sel_regmap(rdev, 0);
+}
+
+static int axp20x_io_enable_regmap(struct regulator_dev *rdev)
+{
+	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
+				  rdev->desc->enable_mask, AXP20X_IO_ENABLED);
+}
+
+static int axp109_io_is_enabled_regmap(struct regulator_dev *rdev)
+{
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(rdev->regmap, rdev->desc->enable_reg, &val);
+	if (ret != 0)
+		return ret;
+
+	val &= rdev->desc->enable_mask;
+	return (val == AXP20X_IO_ENABLED);
+}
+
+static struct regulator_ops axp20x_ops_table = {
+	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+	.list_voltage		= regulator_list_voltage_table,
+	.enable			= regulator_enable_regmap,
+	.disable		= regulator_disable_regmap,
+	.is_enabled		= regulator_is_enabled_regmap,
+	.set_suspend_enable	= regulator_enable_regmap,
+	.set_suspend_disable	= regulator_disable_regmap,
+	.set_suspend_voltage	= axp20x_set_suspend_voltage,
+};
+
+
+static struct regulator_ops axp20x_ops = {
+	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+	.list_voltage		= regulator_list_voltage_linear,
+	.enable			= regulator_enable_regmap,
+	.disable		= regulator_disable_regmap,
+	.is_enabled		= regulator_is_enabled_regmap,
+	.set_suspend_enable	= regulator_enable_regmap,
+	.set_suspend_disable	= regulator_disable_regmap,
+	.set_suspend_voltage	= axp20x_set_suspend_voltage,
+};
+
+static struct regulator_ops axp20x_ops_io = {
+	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+	.list_voltage		= regulator_list_voltage_linear,
+	.enable			= axp20x_io_enable_regmap,
+	.disable		= regulator_disable_regmap,
+	.is_enabled		= axp109_io_is_enabled_regmap,
+	.set_suspend_enable	= regulator_enable_regmap,
+	.set_suspend_disable	= regulator_disable_regmap,
+	.set_suspend_voltage	= axp20x_set_suspend_voltage,
+};
+
+static struct regulator_desc axp20x_regulators[] = {
+	AXP20X_DESC(DCDC2, 700, 2275, 25, AXP20X_DCDC2_V_OUT, 0x3f,
+		    AXP20X_PWR_OUT_CTRL, 0x10),
+	AXP20X_DESC(DCDC3, 700, 3500, 25, AXP20X_DCDC3_V_OUT, 0x7f,
+		    AXP20X_PWR_OUT_CTRL, 0x02),
+	AXP20X_DESC_FIXED(LDO1, 1300),
+	AXP20X_DESC(LDO2, 1800, 3300, 100, AXP20X_LDO24_V_OUT, 0xf0,
+		    AXP20X_PWR_OUT_CTRL, 0x04),
+	AXP20X_DESC(LDO3, 700, 3500, 25, AXP20X_LDO3_V_OUT, 0x7f,
+		    AXP20X_PWR_OUT_CTRL, 0x40),
+	AXP20X_DESC_TABLE(LDO4, axp20x_ldo4_data, AXP20X_LDO24_V_OUT, 0x0f,
+			  AXP20X_PWR_OUT_CTRL, 0x08),
+	AXP20X_DESC_IO(LDO5, 1800, 3300, 100, AXP20X_LDO5_V_OUT, 0xf0,
+		       AXP20X_GPIO0_CTRL, 0x07),
+};
+
+#define AXP_MATCH(_name, _id) \
+	[AXP20X_##_id] = { \
+		.name		= #_name, \
+		.driver_data	= (void *) &axp20x_regulators[AXP20X_##_id], \
+	}
+
+static struct of_regulator_match axp20x_matches[] = {
+	AXP_MATCH(dcdc2, DCDC2),
+	AXP_MATCH(dcdc3, DCDC3),
+	AXP_MATCH(ldo1, LDO1),
+	AXP_MATCH(ldo2, LDO2),
+	AXP_MATCH(ldo3, LDO3),
+	AXP_MATCH(ldo4, LDO4),
+	AXP_MATCH(ldo5, LDO5),
+};
+
+static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
+{
+	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
+
+	if (dcdcfreq < 750)
+		dcdcfreq = 750;
+
+	if (dcdcfreq > 1875)
+		dcdcfreq = 1875;
+
+	dcdcfreq = (dcdcfreq - 750) / 75;
+
+	return regmap_update_bits(axp20x->regmap, AXP20X_DCDC_FREQ,
+				  AXP20X_FREQ_DCDC_MASK, dcdcfreq);
+}
+
+static int axp20x_regulator_parse_dt(struct platform_device *pdev)
+{
+	struct device_node *np, *regulators;
+	int ret;
+	u32 dcdcfreq;
+
+	np = of_node_get(pdev->dev.parent->of_node);
+	if (!np)
+		return 0;
+
+	regulators = of_find_node_by_name(np, "regulators");
+	if (!regulators) {
+		dev_err(&pdev->dev, "regulators node not found\n");
+		return -EINVAL;
+	}
+
+	ret = of_regulator_match(&pdev->dev, regulators, axp20x_matches,
+				 ARRAY_SIZE(axp20x_matches));
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Error parsing regulator init data: %d\n",
+			ret);
+		return ret;
+	}
+
+	dcdcfreq = 0x08;
+	of_property_read_u32(regulators, "dcdc-freq", &dcdcfreq);
+	ret = axp20x_set_dcdc_freq(pdev, dcdcfreq);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Error setting dcdc frequency: %d\n", ret);
+		return ret;
+	}
+
+	of_node_put(regulators);
+
+	return 0;
+}
+
+static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 workmode)
+{
+	unsigned int mask = AXP20X_WORKMODE_DCDC2_MASK;
+
+	if ((id != AXP20X_DCDC2) && (id != AXP20X_DCDC3))
+		return -EINVAL;
+
+	if (id == AXP20X_DCDC3)
+		mask = AXP20X_WORKMODE_DCDC3_MASK;
+
+	return regmap_update_bits(rdev->regmap, AXP20X_DCDC_MODE, mask, workmode);
+}
+
+static int axp20x_regulator_probe(struct platform_device *pdev)
+{
+	struct axp20x_regulators *pmic;
+	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
+	struct regulator_config config = { };
+	struct regulator_init_data *init_data;
+	int ret, i;
+	u32 workmode;
+
+	ret = axp20x_regulator_parse_dt(pdev);
+	if (ret)
+		return ret;
+
+	pmic = devm_kzalloc(&pdev->dev, sizeof(*pmic), GFP_KERNEL);
+	if (!pmic) {
+		dev_err(&pdev->dev, "Failed to alloc pmic\n");
+		return -ENOMEM;
+	}
+
+	pmic->axp20x = axp20x;
+	memcpy(pmic->rdesc, axp20x_regulators, sizeof(pmic->rdesc));
+	platform_set_drvdata(pdev, pmic);
+
+	for (i = 0; i < AXP20X_REG_ID_MAX; i++) {
+		init_data = axp20x_matches[i].init_data;
+		if (!init_data)
+			continue;
+
+		config.dev = &pdev->dev;
+		config.init_data = init_data;
+		config.driver_data = pmic;
+		config.regmap = axp20x->regmap;
+		config.of_node = axp20x_matches[i].of_node;
+
+		pmic->rdev[i] = regulator_register(&pmic->rdesc[i], &config);
+		if (IS_ERR(pmic->rdev[i])) {
+			ret = PTR_ERR(pmic->rdev[i]);
+			dev_err(&pdev->dev, "Failed to register %s\n",
+				pmic->rdesc[i].name);
+
+			while (--i >= 0)
+				regulator_unregister(pmic->rdev[i]);
+
+			return ret;
+		}
+
+		ret = of_property_read_u32(axp20x_matches[i].of_node, "dcdc-workmode",
+					   &workmode);
+		if (!ret) {
+			ret = axp20x_set_dcdc_workmode(pmic->rdev[i], i, workmode);
+			if (ret)
+				dev_err(&pdev->dev, "Failed to set workmode on %s\n",
+					pmic->rdesc[i].name);
+		}
+	}
+
+	return 0;
+}
+
+static int axp20x_regulator_remove(struct platform_device *pdev)
+{
+	struct axp20x_regulators *pmic = platform_get_drvdata(pdev);
+	int i;
+
+	 for (i = 0; i < AXP20X_REG_ID_MAX; i++)
+		 regulator_unregister(pmic->rdev[i]);
+
+	 return 0;
+}
+
+static struct platform_driver axp20x_regulator_driver = {
+	.probe	= axp20x_regulator_probe,
+	.remove	= axp20x_regulator_remove,
+	.driver	= {
+		.name		= "axp20x-regulator",
+		.owner		= THIS_MODULE,
+	},
+};
+
+static int __init axp20x_regulator_init(void)
+{
+	return platform_driver_register(&axp20x_regulator_driver);
+}
+
+subsys_initcall(axp20x_regulator_init);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Carlo Caione <carlo@caione.org>");
+MODULE_DESCRIPTION("Regulator Driver for AXP20X PMIC");