[PATCH 0/3] regulator: add driver for Hi6421 PMIC SoC

[PATCH 0/3] regulator: add driver for Hi6421 PMIC SoC

[Guodong Xu]

This patchset adds a core driver and regulators driver for Hi6421 PMIC SoC.

Hi6421 is a PMIC SoC designed and manufactured by HiSilicon Ltd. It includes
multi-functions, such as regulators, codec, ADCs, Coulomb counter, etc.

Hi6421 can be reused in various Hi3620 based boards. Memory-mapped I/O is
the way used by application processor to communicate with Hi6421.

Guodong Xu (3):
  mfd: Add hi6421 PMIC core driver
  regulator: add driver for hi6421 voltage regulator
  ARM: dts: add dedicated dtsi for hi6421

 .../bindings/regulator/hi6421-regulator.txt        |   82 +++
 arch/arm/boot/dts/hi6421.dtsi                      |  489 +++++++++++++++++
 drivers/mfd/Kconfig                                |   11 +
 drivers/mfd/Makefile                               |    1 +
 drivers/mfd/hi6421-pmic-core.c                     |  171 ++++++
 drivers/regulator/Kconfig                          |    8 +-
 drivers/regulator/Makefile                         |    2 +-
 drivers/regulator/hi6421-regulator.c               |  559 ++++++++++++++++++++
 include/linux/mfd/hi6421-pmic.h                    |   55 ++
 9 files changed, 1376 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/hi6421-regulator.txt
 create mode 100644 arch/arm/boot/dts/hi6421.dtsi
 create mode 100644 drivers/mfd/hi6421-pmic-core.c
 create mode 100644 drivers/regulator/hi6421-regulator.c
 create mode 100644 include/linux/mfd/hi6421-pmic.h

-- 
1.7.4.1

[PATCH 1/3] mfd: Add hi6421 PMIC core driver

[Guodong Xu]

This adds driver to support HiSilicon Hi6421 PMIC. Hi6421 includes multi-
functions, such as regulators, codec, ADCs, Coulomb counter, etc.
This driver includes core APIs _only_.

Drivers for individul components, like voltage regulators, are
implemented in corresponding driver directories and files.

Memory-mapped I/O is the way to communicate with Hi6421.

Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 drivers/mfd/Kconfig             |   11 +++
 drivers/mfd/Makefile            |    1 +
 drivers/mfd/hi6421-pmic-core.c  |  171 +++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/hi6421-pmic.h |   55 +++++++++++++
 4 files changed, 238 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mfd/hi6421-pmic-core.c
 create mode 100644 include/linux/mfd/hi6421-pmic.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index d54e985..111c109 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1116,6 +1116,17 @@ config MFD_WM8994
 	  core support for the WM8994, in order to use the actual
 	  functionaltiy of the device other drivers must be enabled.
 
+config MFD_HI6421_PMIC
+	tristate "HiSilicon Hi6421 PMU/Codec IC"
+	depends on OF
+	help
+	  This driver supports HiSilicon Hi6421 PMIC. Hi6421 includes multi-
+	  functions, such as regulators, codec, ADCs, Coulomb counter, etc.
+	  This driver includes core APIs _only_. You have to select
+	  individul components like voltage regulators under corresponding
+	  menus in order to enable them.
+	  Memory-mapped I/O is the way to communicate with Hi6421.
+
 endmenu
 endif
 
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 718e94a..58cda25 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -155,3 +155,4 @@ obj-$(CONFIG_MFD_LM3533)	+= lm3533-core.o lm3533-ctrlbank.o
 obj-$(CONFIG_VEXPRESS_CONFIG)	+= vexpress-config.o vexpress-sysreg.o
 obj-$(CONFIG_MFD_RETU)		+= retu-mfd.o
 obj-$(CONFIG_MFD_AS3711)	+= as3711.o
+obj-$(CONFIG_MFD_HI6421_PMIC)	+= hi6421-pmic-core.o
diff --git a/drivers/mfd/hi6421-pmic-core.c b/drivers/mfd/hi6421-pmic-core.c
new file mode 100644
index 0000000..3bf8c43
--- /dev/null
+++ b/drivers/mfd/hi6421-pmic-core.c
@@ -0,0 +1,171 @@
+/*
+ * Device driver for Hi6421 IC
+ *
+ * Copyright (c) <2011-2013> HiSilicon Technologies Co., Ltd.
+ *              http://www.hisilicon.com
+ * Copyright (c) <2011-2013> Linaro Ltd.
+ *              http://www.linaro.org
+ *
+ * Author: Guodong Xu <guodong.xu@linaro.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.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/mfd/hi6421-pmic.h>
+
+static struct of_device_id of_hi6421_pmic_child_match_tbl[] = {
+	/* regulators */
+	{
+		.compatible = "hisilicon,hi6421-ldo",
+	},
+	{
+		.compatible = "hisilicon,hi6421-buck012",
+	},
+	{
+		.compatible = "hisilicon,hi6421-buck345",
+	},
+	{ /* end */ }
+};
+
+static struct of_device_id of_hi6421_pmic_match_tbl[] = {
+	{
+		.compatible = "hisilicon,hi6421-pmic",
+	},
+	{ /* end */ }
+};
+
+/*
+ * The PMIC register is only 8-bit.
+ * Hisilicon SoC use hardware to map PMIC register into SoC mapping.
+ * At here, we are accessing SoC register with 32-bit.
+ */
+u32 hi6421_pmic_read(struct hi6421_pmic *pmic, int reg)
+{
+	unsigned long flags;
+	u32 ret;
+	spin_lock_irqsave(&pmic->lock, flags);
+	ret = readl_relaxed(pmic->regs + (reg << 2));
+	spin_unlock_irqrestore(&pmic->lock, flags);
+	return ret;
+}
+EXPORT_SYMBOL(hi6421_pmic_read);
+
+void hi6421_pmic_write(struct hi6421_pmic *pmic, int reg, u32 val)
+{
+	unsigned long flags;
+	spin_lock_irqsave(&pmic->lock, flags);
+	writel_relaxed(val, pmic->regs + (reg << 2));
+	spin_unlock_irqrestore(&pmic->lock, flags);
+}
+EXPORT_SYMBOL(hi6421_pmic_write);
+
+void hi6421_pmic_rmw(struct hi6421_pmic *pmic, int reg,
+		     u32 mask, u32 bits)
+{
+	u32 data;
+
+	spin_lock(&pmic->lock);
+	data = readl_relaxed(pmic->regs + (reg << 2)) & ~mask;
+	data |= mask & bits;
+	writel_relaxed(data, pmic->regs + (reg << 2));
+	spin_unlock(&pmic->lock);
+}
+EXPORT_SYMBOL(hi6421_pmic_rmw);
+
+static int hi6421_pmic_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct hi6421_pmic *pmic = NULL;
+
+	pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL);
+	if (!pmic) {
+		dev_err(dev, "cannot allocate hi6421_pmic device info\n");
+		return -ENOMEM;
+	}
+
+	/* get resources */
+	pmic->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!pmic->res) {
+		dev_err(dev, "platform_get_resource err\n");
+		return -ENOENT;
+	}
+
+	if (!devm_request_mem_region(dev, pmic->res->start,
+				     resource_size(pmic->res),
+				     pdev->name)) {
+		dev_err(dev, "cannot claim register memory\n");
+		return -ENOMEM;
+	}
+
+	pmic->regs = devm_ioremap(dev, pmic->res->start,
+				  resource_size(pmic->res));
+	if (!pmic->regs) {
+		dev_err(dev, "cannot map register memory\n");
+		return -ENOMEM;
+	}
+
+	platform_set_drvdata(pdev, pmic);
+
+	spin_lock_init(&pmic->lock);
+
+	/* set over-current protection debounce 8ms*/
+	hi6421_pmic_rmw(pmic, OCP_DEB_CTRL_REG,
+		OCP_DEB_SEL_MASK | OCP_EN_DEBOUNCE_MASK | OCP_AUTO_STOP_MASK,
+		OCP_DEB_SEL_8MS | OCP_EN_DEBOUNCE_ENABLE);
+
+	/* populate sub nodes */
+	of_platform_populate(np, of_hi6421_pmic_child_match_tbl, NULL, dev);
+
+	return 0;
+}
+
+static int hi6421_pmic_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct hi6421_pmic *pmic = platform_get_drvdata(pdev);
+
+	devm_iounmap(dev, pmic->regs);
+	devm_release_mem_region(dev, pmic->res->start,
+				resource_size(pmic->res));
+	devm_kfree(dev, pmic);
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static struct platform_driver hi6421_pmic_driver = {
+	.driver = {
+		.name	= "hi6421_pmic",
+		.owner  = THIS_MODULE,
+		.of_match_table = of_hi6421_pmic_match_tbl,
+	},
+	.probe	= hi6421_pmic_probe,
+	.remove	= hi6421_pmic_remove,
+};
+module_platform_driver(hi6421_pmic_driver);
+
+MODULE_AUTHOR("Guodong Xu <guodong.xu@linaro.org>");
+MODULE_DESCRIPTION("Hi6421 PMIC driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/hi6421-pmic.h b/include/linux/mfd/hi6421-pmic.h
new file mode 100644
index 0000000..2df16ba
--- /dev/null
+++ b/include/linux/mfd/hi6421-pmic.h
@@ -0,0 +1,55 @@
+/*
+ * Header file for device driver Hi6421 PMIC
+ *
+ * Copyright (c) <2011-2013> HiSilicon Technologies Co., Ltd.
+ *              http://www.hisilicon.com
+ * Copyright (c) <2011-2013> Linaro Ltd.
+ *              http://www.linaro.org
+ *
+ * Author: Guodong Xu <guodong.xu@linaro.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.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef	__HI6421_PMIC_H
+#define	__HI6421_PMIC_H
+
+#define	OCP_DEB_CTRL_REG		(0x51)
+#define	OCP_DEB_SEL_MASK		(0x0C)
+#define OCP_DEB_SEL_8MS			(0x00)
+#define OCP_DEB_SEL_16MS		(0x04)
+#define OCP_DEB_SEL_32MS		(0x08)
+#define OCP_DEB_SEL_64MS		(0x0C)
+#define OCP_EN_DEBOUNCE_MASK		(0x02)
+#define OCP_EN_DEBOUNCE_ENABLE		(0x02)
+#define OCP_AUTO_STOP_MASK		(0x01)
+#define OCP_AUTO_STOP_ENABLE		(0x01)
+#define HI6421_REGS_ENA_PROTECT_TIME	(100)	/* in microseconds */
+#define HI6421_ECO_MODE_ENABLE		(1)
+#define HI6421_ECO_MODE_DISABLE		(0)
+
+struct hi6421_pmic {
+	struct resource		*res;
+	struct device		*dev;
+	void __iomem		*regs;
+	spinlock_t		lock;
+};
+
+/* Register Access Helpers */
+u32 hi6421_pmic_read(struct hi6421_pmic *pmic, int reg);
+void hi6421_pmic_write(struct hi6421_pmic *pmic, int reg, u32 val);
+void hi6421_pmic_rmw(struct hi6421_pmic *pmic, int reg, u32 mask, u32 bits);
+
+#endif		/* __HI6421_PMIC_H */
-- 
1.7.4.1

[PATCH 2/3] regulator: add driver for hi6421 voltage regulator

[Guodong Xu]

Add driver support for HiSilicon Hi6421 voltage regulators.

Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
---
 .../bindings/regulator/hi6421-regulator.txt        |   82 +++
 drivers/regulator/Kconfig                          |    8 +-
 drivers/regulator/Makefile                         |    2 +-
 drivers/regulator/hi6421-regulator.c               |  559 ++++++++++++++++++++
 4 files changed, 649 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/hi6421-regulator.txt
 create mode 100644 drivers/regulator/hi6421-regulator.c

diff --git a/Documentation/devicetree/bindings/regulator/hi6421-regulator.txt b/Documentation/devicetree/bindings/regulator/hi6421-regulator.txt
new file mode 100644
index 0000000..165dc26
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/hi6421-regulator.txt
@@ -0,0 +1,82 @@
+Hi6421 regulator
+
+Hi6421 is a power management IC designed by HiSilicon Technologies Co., Ltd. It
+has functionalities of the following:
+- Power Management Unit, including regulators
+- Audio processing, codecs
+- Misc functions, such as LEDs, RTC, etc.
+
+In structure, Hi6421 device tree node is devided into two levels, each has its
+own compatible field.
+
+In its first level, hi6421 chip level properties are defined, such as reg,
+interrupt, gpios.
+
+In its second level, sub-component level of properties are defined. For example,
+in case of LDO regulators, there are regulator-name, regulator-min-microvolt
+properties; in case of rtc, there are interrupts property.
+
+This document describes devicetree binding (second level) information about
+Hi6421 regulators.
+
+Required properties:
+- compatible: three types of regulator are defined,
+	- "hisilicon,hi6421-ldo"
+	- "hisilicon,hi6421-buck012"
+	- "hisilicon,hi6421-buck345"
+- hisilicon,hi6421-ctrl: <ctrl_reg enable_mask eco_mode_mask>
+	- ctrl_reg: control register offset address
+	- enable_mask: regulator on/off bitmask
+	- eco_mode_mask: ECO mode on/off bitmask
+- hisilicon,hi6421-vset: <vset_reg vset_mask>
+	- vset_reg: voltage set register offset address
+	- vset_mask: voltage setting bitmask
+- hisilicon,hi6421-n-voltages: <n>
+	- n: number of voltage levels supported
+- hisilicon,hi6421-vset-table: array of voltages selectable in this regulator
+			       in unit of microvolt
+	Note: 1) size of this array equals <n> in "hisilicon,hi6421-n-voltages"
+	      2) this and next property "hisilicon,hi6421-uv-step" cannot be
+	      set at the same time for the same regulator node.
+- hisilicon,hi6421-uv-step: <uV_step>
+	- uV_step: step (in uV) for a linear mapping between selector and
+		   result voltage
+- hisilicon,hi6421-off-on-delay-us: <off_on_delay>
+	- off_on_delay: guard time (in microseconds), before re-enabling
+		        a previously disabled regulator
+- hisilicon,hi6421-enable-time-us: <enable_time>
+	- enable_time: time taken for initial enable of regulator (in uS)
+
+Properties defined by the standard binding for regulators: (See regulator.txt)
+- regulator-name:
+- regulator-min-microvolt:
+- regulator-max-microvolt:
+- regulator-boot-on:
+- regulator-always-on:
+
+Optional properties:
+- hisilicon,hi6421-eco-microamp: maximum current allowed in ECO mode (in uA)
+
+Example:
+
+		pmic: pmic@fcc00000 {
+			compatible = "hisilicon,hi6421-pmic";
+			reg = <0xfcc00000 0x0180>; /* 0x60 << 2 */
+
+			ldo0: ldo@20 {
+				compatible = "hisilicon,hi6421-ldo";
+				regulator-name = "LDO0";
+				regulator-min-microvolt = <2850000>;
+				regulator-max-microvolt = <2850000>;
+				hisilicon,hi6421-ctrl = <0x20 0x10 0x20>;
+				hisilicon,hi6421-vset = <0x20 0x07>;
+				hisilicon,hi6421-n-voltages = <8>;
+				hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+							      <2400000>, <2500000>,
+							      <2600000>, <2700000>,
+							      <2850000>, <3000000>;
+				hisilicon,hi6421-off-on-delay-us = <10000>;
+				hisilicon,hi6421-enable-time-us = <250>;
+				hisilicon,hi6421-eco-microamp = <8000>;
+			};
+		};
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 8bb2644..616254c 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -514,5 +514,11 @@ config REGULATOR_AS3711
 	  This driver provides support for the voltage regulators on the
 	  AS3711 PMIC
 
-endif
+config REGULATOR_HI6421
+	tristate "HiSilicon Hi6421 PMIC voltage regulator support"
+	depends on MFD_HI6421_PMIC && OF
+	help
+	  This driver provides support for the voltage regulators on the
+	  HiSilicon Hi6421 PMU / Codec IC.
 
+endif
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 47a34ff..5a67225 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -70,6 +70,6 @@ obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o
 obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
 obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
 obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
-
+obj-$(CONFIG_REGULATOR_HI6421) += hi6421-regulator.o
 
 ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
diff --git a/drivers/regulator/hi6421-regulator.c b/drivers/regulator/hi6421-regulator.c
new file mode 100644
index 0000000..62fd2df
--- /dev/null
+++ b/drivers/regulator/hi6421-regulator.c
@@ -0,0 +1,559 @@
+/*
+ * Device driver for regulators in Hi6421 IC
+ *
+ * Copyright (c) <2011-2013> HiSilicon Technologies Co., Ltd.
+ *              http://www.hisilicon.com
+ * Copyright (c) <2011-2013> Linaro Ltd.
+ *              http://www.linaro.org
+ *
+ * Author: Guodong Xu <guodong.xu@linaro.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.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/mfd/hi6421-pmic.h>
+#include <linux/delay.h>
+#include <linux/time.h>
+
+struct hi6421_regulator_register_info {
+	u32 ctrl_reg;
+	u32 enable_mask;
+	u32 eco_mode_mask;
+	u32 vset_reg;
+	u32 vset_mask;
+};
+
+struct hi6421_regulator {
+	const char *name;
+	struct hi6421_regulator_register_info register_info;
+	struct timeval last_off_time;
+	u32 off_on_delay;
+	u32 eco_uA;
+	struct regulator_desc rdesc;
+	int (*dt_parse)(struct hi6421_regulator *, struct platform_device *);
+};
+
+static DEFINE_MUTEX(enable_mutex);
+struct timeval last_enabled;
+
+
+static inline struct hi6421_pmic *rdev_to_pmic(struct regulator_dev *dev)
+{
+	/* regulator_dev parent to->
+	 * hi6421 regulator platform device_dev parent to->
+	 * hi6421 pmic platform device_dev
+	 */
+	return dev_get_drvdata(rdev_get_dev(dev)->parent->parent);
+}
+
+/* helper function to ensure when it returns it is at least 'delay_us'
+ * microseconds after 'since'.
+ */
+static void ensured_time_after(struct timeval since, u32 delay_us)
+{
+	struct timeval now;
+	u64 elapsed_ns64, delay_ns64;
+	u32 actual_us32;
+
+	delay_ns64 = delay_us * NSEC_PER_USEC;
+	do_gettimeofday(&now);
+	elapsed_ns64 = timeval_to_ns(&now) - timeval_to_ns(&since);
+	if (delay_ns64 > elapsed_ns64) {
+		actual_us32 = ((u32)(delay_ns64 - elapsed_ns64) /
+							NSEC_PER_USEC);
+		if (actual_us32 >= 1000) {
+			mdelay(actual_us32 / 1000);
+			udelay(actual_us32 % 1000);
+		} else if (actual_us32 > 0) {
+			udelay(actual_us32);
+		}
+	}
+	return;
+}
+
+static int hi6421_regulator_is_enabled(struct regulator_dev *dev)
+{
+	u32 reg_val;
+	struct hi6421_regulator *sreg = rdev_get_drvdata(dev);
+	struct hi6421_pmic *pmic = rdev_to_pmic(dev);
+
+	reg_val = hi6421_pmic_read(pmic, sreg->register_info.ctrl_reg);
+
+	return ((reg_val & sreg->register_info.enable_mask) != 0);
+}
+
+static int hi6421_regulator_enable(struct regulator_dev *dev)
+{
+	struct hi6421_regulator *sreg = rdev_get_drvdata(dev);
+	struct hi6421_pmic *pmic = rdev_to_pmic(dev);
+
+	/* keep a distance of off_on_delay from last time disabled */
+	ensured_time_after(sreg->last_off_time, sreg->off_on_delay);
+
+	/* cannot enable more than one regulator at one time */
+	mutex_lock(&enable_mutex);
+	ensured_time_after(last_enabled, HI6421_REGS_ENA_PROTECT_TIME);
+
+	/* set enable register */
+	hi6421_pmic_rmw(pmic, sreg->register_info.ctrl_reg,
+				sreg->register_info.enable_mask,
+				sreg->register_info.enable_mask);
+
+	do_gettimeofday(&last_enabled);
+	mutex_unlock(&enable_mutex);
+
+	return 0;
+}
+
+static int hi6421_regulator_disable(struct regulator_dev *dev)
+{
+	struct hi6421_regulator *sreg = rdev_get_drvdata(dev);
+	struct hi6421_pmic *pmic = rdev_to_pmic(dev);
+
+	/* set enable register to 0 */
+	hi6421_pmic_rmw(pmic, sreg->register_info.ctrl_reg,
+				sreg->register_info.enable_mask, 0);
+
+	do_gettimeofday(&sreg->last_off_time);
+
+	return 0;
+}
+
+static int hi6421_regulator_get_voltage(struct regulator_dev *dev)
+{
+	struct hi6421_regulator *sreg = rdev_get_drvdata(dev);
+	struct hi6421_pmic *pmic = rdev_to_pmic(dev);
+	u32 reg_val, selector;
+
+	/* get voltage selector */
+	reg_val = hi6421_pmic_read(pmic, sreg->register_info.vset_reg);
+	selector = (reg_val & sreg->register_info.vset_mask) >>
+				(ffs(sreg->register_info.vset_mask) - 1);
+
+	return sreg->rdesc.ops->list_voltage(dev, selector);
+}
+
+static int hi6421_regulator_ldo_set_voltage(struct regulator_dev *dev,
+				int min_uV, int max_uV, unsigned *selector)
+{
+	struct hi6421_regulator *sreg = rdev_get_drvdata(dev);
+	struct hi6421_pmic *pmic = rdev_to_pmic(dev);
+	u32 vsel;
+	int ret = 0;
+
+	for (vsel = 0; vsel < sreg->rdesc.n_voltages; vsel++) {
+		int uV = sreg->rdesc.volt_table[vsel];
+		/* Break at the first in-range value */
+		if (min_uV <= uV && uV <= max_uV)
+			break;
+	}
+
+	/* unlikely to happen. sanity test done by regulator core */
+	if (unlikely(vsel == sreg->rdesc.n_voltages))
+		return -EINVAL;
+
+	*selector = vsel;
+	/* set voltage selector */
+	hi6421_pmic_rmw(pmic, sreg->register_info.vset_reg,
+		sreg->register_info.vset_mask,
+		vsel << (ffs(sreg->register_info.vset_mask) - 1));
+
+	return ret;
+}
+
+static int hi6421_regulator_buck012_set_voltage(struct regulator_dev *dev,
+				int min_uV, int max_uV, unsigned *selector)
+{
+	struct hi6421_regulator *sreg = rdev_get_drvdata(dev);
+	struct hi6421_pmic *pmic = rdev_to_pmic(dev);
+	u32 vsel;
+	int ret = 0;
+
+	vsel = DIV_ROUND_UP((max_uV - sreg->rdesc.min_uV),
+				sreg->rdesc.uV_step);
+
+	*selector = vsel;
+	/* set voltage selector */
+	hi6421_pmic_rmw(pmic, sreg->register_info.vset_reg,
+		sreg->register_info.vset_mask,
+		vsel << (ffs(sreg->register_info.vset_mask) - 1));
+
+	return ret;
+}
+
+static unsigned int hi6421_regulator_get_mode(struct regulator_dev *dev)
+{
+	struct hi6421_regulator *sreg = rdev_get_drvdata(dev);
+	struct hi6421_pmic *pmic = rdev_to_pmic(dev);
+	u32 reg_val;
+
+	reg_val = hi6421_pmic_read(pmic, sreg->register_info.ctrl_reg);
+	if (reg_val & sreg->register_info.eco_mode_mask)
+		return REGULATOR_MODE_IDLE;
+	else
+		return REGULATOR_MODE_NORMAL;
+}
+
+static int hi6421_regulator_set_mode(struct regulator_dev *dev,
+						unsigned int mode)
+{
+	struct hi6421_regulator *sreg = rdev_get_drvdata(dev);
+	struct hi6421_pmic *pmic = rdev_to_pmic(dev);
+	u32 eco_mode;
+
+	switch (mode) {
+	case REGULATOR_MODE_NORMAL:
+		eco_mode = HI6421_ECO_MODE_DISABLE;
+		break;
+	case REGULATOR_MODE_IDLE:
+		eco_mode = HI6421_ECO_MODE_ENABLE;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* set mode */
+	hi6421_pmic_rmw(pmic, sreg->register_info.ctrl_reg,
+		sreg->register_info.eco_mode_mask,
+		eco_mode << (ffs(sreg->register_info.eco_mode_mask) - 1));
+
+	return 0;
+}
+
+
+unsigned int hi6421_regulator_get_optimum_mode(struct regulator_dev *dev,
+			int input_uV, int output_uV, int load_uA)
+{
+	struct hi6421_regulator *sreg = rdev_get_drvdata(dev);
+
+	if ((load_uA == 0) || (load_uA > sreg->eco_uA))
+		return REGULATOR_MODE_NORMAL;
+	else
+		return REGULATOR_MODE_IDLE;
+}
+
+static int hi6421_dt_parse_common(struct hi6421_regulator *sreg,
+					struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct regulator_desc *rdesc = &sreg->rdesc;
+	unsigned int register_info[3];
+	int ret = 0;
+
+	/* parse .register_info.ctrl_reg */
+	ret = of_property_read_u32_array(np, "hisilicon,hi6421-ctrl",
+						register_info, 3);
+	if (ret) {
+		dev_err(dev, "no hisilicon,hi6421-ctrl property set\n");
+		goto dt_parse_common_end;
+	}
+	sreg->register_info.ctrl_reg = register_info[0];
+	sreg->register_info.enable_mask = register_info[1];
+	sreg->register_info.eco_mode_mask = register_info[2];
+
+	/* parse .register_info.vset_reg */
+	ret = of_property_read_u32_array(np, "hisilicon,hi6421-vset",
+						register_info, 2);
+	if (ret) {
+		dev_err(dev, "no hisilicon,hi6421-vset property set\n");
+		goto dt_parse_common_end;
+	}
+	sreg->register_info.vset_reg = register_info[0];
+	sreg->register_info.vset_mask = register_info[1];
+
+	/* parse .off-on-delay */
+	ret = of_property_read_u32(np, "hisilicon,hi6421-off-on-delay-us",
+						&sreg->off_on_delay);
+	if (ret) {
+		dev_err(dev, "no hisilicon,hi6421-off-on-delay-us property set\n");
+		goto dt_parse_common_end;
+	}
+
+	/* parse .enable_time */
+	ret = of_property_read_u32(np, "hisilicon,hi6421-enable-time-us",
+				   &rdesc->enable_time);
+	if (ret) {
+		dev_err(dev, "no hisilicon,hi6421-enable-time-us property set\n");
+		goto dt_parse_common_end;
+	}
+
+	/* parse .eco_uA */
+	ret = of_property_read_u32(np, "hisilicon,hi6421-eco-microamp",
+				   &sreg->eco_uA);
+	if (ret) {
+		sreg->eco_uA = 0;
+		ret = 0;
+	}
+
+dt_parse_common_end:
+	return ret;
+}
+
+static int hi6421_dt_parse_ldo(struct hi6421_regulator *sreg,
+				struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct regulator_desc *rdesc = &sreg->rdesc;
+	unsigned int *v_table;
+	int ret = 0;
+
+	/* parse .n_voltages, and .volt_table */
+	ret = of_property_read_u32(np, "hisilicon,hi6421-n-voltages",
+				   &rdesc->n_voltages);
+	if (ret) {
+		dev_err(dev, "no hisilicon,hi6421-n-voltages property set\n");
+		goto dt_parse_ldo_end;
+	}
+
+	/* alloc space for .volt_table */
+	v_table = devm_kzalloc(dev, sizeof(unsigned int) * rdesc->n_voltages,
+								GFP_KERNEL);
+	if (unlikely(!v_table)) {
+		ret = -ENOMEM;
+		dev_err(dev, "no memory for .volt_table\n");
+		goto dt_parse_ldo_end;
+	}
+
+	ret = of_property_read_u32_array(np, "hisilicon,hi6421-vset-table",
+						v_table, rdesc->n_voltages);
+	if (ret) {
+		dev_err(dev, "no hisilicon,hi6421-vset-table property set\n");
+		goto dt_parse_ldo_end1;
+	}
+	rdesc->volt_table = v_table;
+
+	/* parse hi6421 regulator's dt common part */
+	ret = hi6421_dt_parse_common(sreg, pdev);
+	if (ret) {
+		dev_err(dev, "failure in hi6421_dt_parse_common\n");
+		goto dt_parse_ldo_end1;
+	}
+
+dt_parse_ldo_end1:
+	devm_kfree(dev, v_table);
+dt_parse_ldo_end:
+	return ret;
+}
+
+static int hi6421_dt_parse_buck012(struct hi6421_regulator *sreg,
+					struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct regulator_desc *rdesc = &sreg->rdesc;
+	int ret = 0;
+
+	/* parse .n_voltages, and .uV_step */
+	ret = of_property_read_u32(np, "hisilicon,hi6421-n-voltages",
+				   &rdesc->n_voltages);
+	if (ret) {
+		dev_err(dev, "no hisilicon,hi6421-n-voltages property set\n");
+		goto dt_parse_buck012_end;
+	}
+	ret = of_property_read_u32(np, "hisilicon,hi6421-uv-step",
+				   &rdesc->uV_step);
+	if (ret) {
+		dev_err(dev, "no hisilicon,hi6421-uv-step property set\n");
+		goto dt_parse_buck012_end;
+	}
+
+	/* parse hi6421 regulator's dt common part */
+	ret = hi6421_dt_parse_common(sreg, pdev);
+	if (ret) {
+		dev_err(dev, "failure in hi6421_dt_parse_common\n");
+		goto dt_parse_buck012_end;
+	}
+
+dt_parse_buck012_end:
+	return ret;
+}
+
+static struct regulator_ops hi6421_ldo_rops = {
+	.is_enabled = hi6421_regulator_is_enabled,
+	.enable = hi6421_regulator_enable,
+	.disable = hi6421_regulator_disable,
+	.list_voltage = regulator_list_voltage_table,
+	.get_voltage = hi6421_regulator_get_voltage,
+	.set_voltage = hi6421_regulator_ldo_set_voltage,
+	.get_mode = hi6421_regulator_get_mode,
+	.set_mode = hi6421_regulator_set_mode,
+	.get_optimum_mode = hi6421_regulator_get_optimum_mode,
+};
+
+static struct regulator_ops hi6421_buck012_rops = {
+	.is_enabled = hi6421_regulator_is_enabled,
+	.enable = hi6421_regulator_enable,
+	.disable = hi6421_regulator_disable,
+	.list_voltage = regulator_list_voltage_linear,
+	.get_voltage = hi6421_regulator_get_voltage,
+	.set_voltage = hi6421_regulator_buck012_set_voltage,
+	.get_mode = hi6421_regulator_get_mode,
+	.set_mode = hi6421_regulator_set_mode,
+	.get_optimum_mode = hi6421_regulator_get_optimum_mode,
+};
+
+static struct regulator_ops hi6421_buck345_rops = {
+	.is_enabled = hi6421_regulator_is_enabled,
+	.enable = hi6421_regulator_enable,
+	.disable = hi6421_regulator_disable,
+	.list_voltage = regulator_list_voltage_table,
+	.get_voltage = hi6421_regulator_get_voltage,
+	.set_voltage = hi6421_regulator_ldo_set_voltage,
+	.get_mode = hi6421_regulator_get_mode,
+	.set_mode = hi6421_regulator_set_mode,
+	.get_optimum_mode = hi6421_regulator_get_optimum_mode,
+};
+
+static const struct hi6421_regulator hi6421_regulator_ldo = {
+	.rdesc = {
+	.ops = &hi6421_ldo_rops,
+		.type = REGULATOR_VOLTAGE,
+		.owner = THIS_MODULE,
+		},
+	.dt_parse = hi6421_dt_parse_ldo,
+};
+
+static const struct hi6421_regulator hi6421_regulator_buck012 = {
+	.rdesc = {
+		.ops = &hi6421_buck012_rops,
+		.type = REGULATOR_VOLTAGE,
+		.owner = THIS_MODULE,
+		},
+	.dt_parse = hi6421_dt_parse_buck012,
+};
+
+static const struct hi6421_regulator hi6421_regulator_buck345 = {
+	.rdesc = {
+		.ops = &hi6421_buck345_rops,
+		.type = REGULATOR_VOLTAGE,
+		.owner = THIS_MODULE,
+		},
+	.dt_parse = hi6421_dt_parse_ldo,
+};
+
+static struct of_device_id of_hi6421_regulator_match_tbl[] = {
+	{
+		.compatible = "hisilicon,hi6421-ldo",
+		.data = &hi6421_regulator_ldo,
+	},
+	{
+		.compatible = "hisilicon,hi6421-buck012",
+		.data = &hi6421_regulator_buck012,
+	},
+	{
+		.compatible = "hisilicon,hi6421-buck345",
+		.data = &hi6421_regulator_buck345,
+	},
+	{ /* end */ }
+};
+
+static int hi6421_regulator_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct regulator_desc *rdesc;
+	struct regulator_dev *rdev;
+	struct hi6421_regulator *sreg = NULL;
+	struct regulator_init_data *initdata;
+	struct regulator_config config = { };
+	const struct of_device_id *match;
+	const struct hi6421_regulator *template = NULL;
+	int ret = 0;
+
+	/* to check which type of regulator this is */
+	match = of_match_device(of_hi6421_regulator_match_tbl, &pdev->dev);
+	if (match)
+		template = match->data;
+	else
+		return -EINVAL;
+
+	initdata = of_get_regulator_init_data(dev, np);
+	sreg = kmemdup(template, sizeof(*sreg), GFP_KERNEL);
+	if (!sreg)
+		return -ENOMEM;
+
+	sreg->name = initdata->constraints.name;
+	rdesc = &sreg->rdesc;
+	rdesc->name = sreg->name;
+	rdesc->min_uV = initdata->constraints.min_uV;
+
+	/* to parse device tree data for regulator specific */
+	ret = sreg->dt_parse(sreg, pdev);
+	if (ret) {
+		dev_err(dev, "device tree parameter parse error!\n");
+		goto hi6421_probe_end;
+	}
+
+	config.dev = &pdev->dev;
+	config.init_data = initdata;
+	config.driver_data = sreg;
+	config.of_node = pdev->dev.of_node;
+
+	/* register regulator */
+	rdev = regulator_register(rdesc, &config);
+	if (IS_ERR(rdev)) {
+		dev_err(dev, "failed to register %s\n",
+			rdesc->name);
+		ret = PTR_ERR(rdev);
+		goto hi6421_probe_end;
+	}
+
+	platform_set_drvdata(pdev, rdev);
+
+hi6421_probe_end:
+	if (ret)
+		kfree(sreg);
+	return ret;
+}
+
+static int hi6421_regulator_remove(struct platform_device *pdev)
+{
+	struct regulator_dev *rdev = platform_get_drvdata(pdev);
+	struct hi6421_regulator *sreg = rdev_get_drvdata(rdev);
+
+	regulator_unregister(rdev);
+	kfree(sreg);
+	return 0;
+}
+
+static struct platform_driver hi6421_regulator_driver = {
+	.driver = {
+		.name	= "hi6421_regulator",
+		.owner  = THIS_MODULE,
+		.of_match_table = of_hi6421_regulator_match_tbl,
+	},
+	.probe	= hi6421_regulator_probe,
+	.remove	= hi6421_regulator_remove,
+};
+module_platform_driver(hi6421_regulator_driver);
+
+MODULE_AUTHOR("Guodong Xu <guodong.xu@linaro.org>");
+MODULE_DESCRIPTION("Hi6421 regulator driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.4.1

[PATCH 3/3] ARM: dts: add dedicated dtsi for hi6421

[Guodong Xu]

Add dedicated dtsi file for Hi6421 PMIC SoC.

Hi6421 is a PMIC SoC manufactured by HiSilicon Ltd., which can be
reused in various Hi3620 based boards.

Note: This file is supposed to be included in a board DTS that will
create the pmic node in order to allow the &pmic reference to work.

Exmaple:
...
        pmic: pmic@fcc00000 {
                reg = <0xfcc00000 0x0180>; /* 0x60 << 2 */
        };
...

/include/ "hi6421.dtsi"
...

Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
---
 arch/arm/boot/dts/hi6421.dtsi |  489 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 489 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/boot/dts/hi6421.dtsi

diff --git a/arch/arm/boot/dts/hi6421.dtsi b/arch/arm/boot/dts/hi6421.dtsi
new file mode 100644
index 0000000..93647e2
--- /dev/null
+++ b/arch/arm/boot/dts/hi6421.dtsi
@@ -0,0 +1,489 @@
+/*
+ * HiSilicon HI6421 PMIC device tree source
+ *
+ * Copyright (c) <2011-2013> HiSilicon Technologies Co., Ltd.
+ *              http://www.hisilicon.com
+ * Copyright (c) <2011-2013> Linaro Ltd.
+ *              http://www.linaro.org
+ *
+ * Author: Guodong Xu <guodong.xu@linaro.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.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+&pmic {
+	compatible = "hisilicon,hi6421-pmic";
+
+	ldo0: ldo@20 {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO0";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <3000000>;
+		hisilicon,hi6421-ctrl = <0x20 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x20 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+					      <2400000>, <2500000>,
+					      <2600000>, <2700000>,
+					      <2850000>, <3000000>;
+		hisilicon,hi6421-off-on-delay-us = <10000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldo1: ldo@21 {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO1";
+		regulator-min-microvolt = <1700000>;
+		regulator-max-microvolt = <2000000>;
+		hisilicon,hi6421-ctrl = <0x21 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x21 0x03>;
+		hisilicon,hi6421-n-voltages = <4>;
+		hisilicon,hi6421-vset-table = <1700000>, <1800000>,
+					      <1900000>, <2000000>;
+		hisilicon,hi6421-off-on-delay-us = <10000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <5000>;
+	};
+
+	ldo2: ldo@22 {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO2";
+		regulator-min-microvolt = <1050000>;
+		regulator-max-microvolt = <1400000>;
+		hisilicon,hi6421-ctrl = <0x22 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x22 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1050000>, <1100000>,
+					      <1150000>, <1200000>,
+					      <1250000>, <1300000>,
+					      <1350000>, <1400000>;
+		hisilicon,hi6421-off-on-delay-us = <20000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldo3: ldo@23 {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO3";
+		regulator-min-microvolt = <1050000>;
+		regulator-max-microvolt = <1400000>;
+		hisilicon,hi6421-ctrl = <0x23 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x23 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1050000>, <1100000>,
+					      <1150000>, <1200000>,
+					      <1250000>, <1300000>,
+					      <1350000>, <1400000>;
+		hisilicon,hi6421-off-on-delay-us = <20000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldo4: ldo@24 {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO4";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <3000000>;
+		hisilicon,hi6421-ctrl = <0x24 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x24 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+					      <2400000>, <2500000>,
+					      <2600000>, <2700000>,
+					      <2850000>, <3000000>;
+		hisilicon,hi6421-off-on-delay-us = <20000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldo5: ldo@25 {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO5";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <3000000>;
+		hisilicon,hi6421-ctrl = <0x25 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x25 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+					      <2400000>, <2500000>,
+					      <2600000>, <2700000>,
+					      <2850000>, <3000000>;
+		hisilicon,hi6421-off-on-delay-us = <20000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldo6: ldo@26 {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO6";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <3000000>;
+		hisilicon,hi6421-ctrl = <0x26 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x26 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+					      <2400000>, <2500000>,
+					      <2600000>, <2700000>,
+					      <2850000>, <3000000>;
+		hisilicon,hi6421-off-on-delay-us = <20000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldo7: ldo@27 {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO7";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <3000000>;
+		hisilicon,hi6421-ctrl = <0x27 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x27 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+					      <2400000>, <2500000>,
+					      <2600000>, <2700000>,
+					      <2850000>, <3000000>;
+		hisilicon,hi6421-off-on-delay-us = <20000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <5000>;
+	};
+
+	ldo8: ldo@28 {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO8";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <3300000>;
+		hisilicon,hi6421-ctrl = <0x28 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x28 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+					      <2400000>, <2600000>,
+					      <2700000>, <2850000>,
+					      <3000000>, <3300000>;
+		hisilicon,hi6421-off-on-delay-us = <20000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldo9: ldo@29 {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO9";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <3000000>;
+		hisilicon,hi6421-ctrl = <0x29 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x29 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+					      <2400000>, <2500000>,
+					      <2600000>, <2700000>,
+					      <2850000>, <3000000>;
+		hisilicon,hi6421-off-on-delay-us = <40000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldo10: ldo@2a {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO10";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <3000000>;
+		hisilicon,hi6421-ctrl = <0x2a 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x2a 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+					      <2400000>, <2500000>,
+					      <2600000>, <2700000>,
+					      <2850000>, <3000000>;
+		hisilicon,hi6421-off-on-delay-us = <40000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldo11: ldo@2b {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO11";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <3000000>;
+		hisilicon,hi6421-ctrl = <0x2b 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x2b 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+					      <2400000>, <2500000>,
+					      <2600000>, <2700000>,
+					      <2850000>, <3000000>;
+		hisilicon,hi6421-off-on-delay-us = <40000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldo12: ldo@2c {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO12";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <3000000>;
+		hisilicon,hi6421-ctrl = <0x2c 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x2c 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+					      <2400000>, <2500000>,
+					      <2600000>, <2700000>,
+					      <2850000>, <3000000>;
+		hisilicon,hi6421-off-on-delay-us = <40000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldo13: ldo@2d {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO13";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <3000000>;
+		hisilicon,hi6421-ctrl = <0x2d 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x2d 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+					      <2400000>, <2500000>,
+					      <2600000>, <2700000>,
+					      <2850000>, <3000000>;
+		hisilicon,hi6421-off-on-delay-us = <40000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldo14: ldo@2e {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO14";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <3000000>;
+		hisilicon,hi6421-ctrl = <0x2e 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x2e 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+					      <2400000>, <2500000>,
+					      <2600000>, <2700000>,
+					      <2850000>, <3000000>;
+		hisilicon,hi6421-off-on-delay-us = <40000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldo15: ldo@2f {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO15";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <3300000>;
+		hisilicon,hi6421-ctrl = <0x2f 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x2f 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+					      <2400000>, <2600000>,
+					      <2700000>, <2850000>,
+					      <3000000>, <3300000>;
+		hisilicon,hi6421-off-on-delay-us = <40000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldo16: ldo@30 {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO16";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <3000000>;
+		hisilicon,hi6421-ctrl = <0x30 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x30 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+					      <2400000>, <2500000>,
+					      <2600000>, <2700000>,
+					      <2850000>, <3000000>;
+		hisilicon,hi6421-off-on-delay-us = <40000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldo17: ldo@31 {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO17";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <3000000>;
+		hisilicon,hi6421-ctrl = <0x31 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x31 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+					      <2400000>, <2500000>,
+					      <2600000>, <2700000>,
+					      <2850000>, <3000000>;
+		hisilicon,hi6421-off-on-delay-us = <40000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldo18: ldo@32 {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO18";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <3000000>;
+		hisilicon,hi6421-ctrl = <0x32 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x32 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+					      <2400000>, <2500000>,
+					      <2600000>, <2700000>,
+					      <2850000>, <3000000>;
+		hisilicon,hi6421-off-on-delay-us = <40000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldo19: ldo@33 {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO19";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <3000000>;
+		hisilicon,hi6421-ctrl = <0x2a 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x2a 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+					      <2400000>, <2500000>,
+					      <2600000>, <2700000>,
+					      <2850000>, <3000000>;
+		hisilicon,hi6421-off-on-delay-us = <40000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldo20: ldo@34 {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDO20";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <3000000>;
+		hisilicon,hi6421-ctrl = <0x34 0x10 0x20>;
+		hisilicon,hi6421-vset = <0x34 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1500000>, <1800000>,
+					      <2400000>, <2500000>,
+					      <2600000>, <2700000>,
+					      <2850000>, <3000000>;
+		hisilicon,hi6421-off-on-delay-us = <40000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <8000>;
+	};
+
+	ldoaudio: ldo@36 {
+		compatible = "hisilicon,hi6421-ldo";
+		regulator-name = "LDOAUDIO";
+		regulator-min-microvolt = <2800000>;
+		regulator-max-microvolt = <3300000>;
+		hisilicon,hi6421-ctrl = <0x36 0x01 0x02>;
+		hisilicon,hi6421-vset = <0x36 0x70>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <2800000>, <2850000>,
+					      <2900000>, <2950000>,
+					      <3000000>, <3100000>,
+					      <3200000>, <3300000>;
+		hisilicon,hi6421-off-on-delay-us = <40000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+		hisilicon,hi6421-eco-microamp = <5000>;
+	};
+
+	buck0: buck@0c {
+		compatible = "hisilicon,hi6421-buck012";
+		regulator-name = "BUCK0";
+		regulator-min-microvolt = <700000>;
+		regulator-max-microvolt = <1600000>;
+		hisilicon,hi6421-ctrl = <0x0c 0x01 0x10>;
+		hisilicon,hi6421-vset = <0x0d 0x7f>;
+		hisilicon,hi6421-n-voltages = <128>;
+		hisilicon,hi6421-uv-step = <7086>;
+		hisilicon,hi6421-off-on-delay-us = <20000>;
+		hisilicon,hi6421-enable-time-us = <300>;
+	};
+
+	buck1: buck@0e {
+		compatible = "hisilicon,hi6421-buck012";
+		regulator-name = "BUCK1";
+		regulator-min-microvolt = <700000>;
+		regulator-max-microvolt = <1600000>;
+		regulator-boot-on;
+		regulator-always-on;
+		hisilicon,hi6421-ctrl = <0x0e 0x01 0x10>;
+		hisilicon,hi6421-vset = <0x0f 0x7f>;
+		hisilicon,hi6421-n-voltages = <128>;
+		hisilicon,hi6421-uv-step = <7086>;
+		hisilicon,hi6421-off-on-delay-us = <20000>;
+		hisilicon,hi6421-enable-time-us = <300>;
+	};
+
+	buck2: buck@10 {
+		compatible = "hisilicon,hi6421-buck012";
+		regulator-name = "BUCK2";
+		regulator-min-microvolt = <700000>;
+		regulator-max-microvolt = <1600000>;
+		hisilicon,hi6421-ctrl = <0x10 0x01 0x10>;
+		hisilicon,hi6421-vset = <0x11 0x7f>;
+		hisilicon,hi6421-n-voltages = <128>;
+		hisilicon,hi6421-uv-step = <7086>;
+		hisilicon,hi6421-off-on-delay-us = <100>;
+		hisilicon,hi6421-enable-time-us = <250>;
+	};
+
+	buck3: buck@12 {
+		compatible = "hisilicon,hi6421-buck345";
+		regulator-name = "BUCK3";
+		regulator-min-microvolt = <950000>;
+		regulator-max-microvolt = <1200000>;
+		hisilicon,hi6421-ctrl = <0x12 0x01 0x10>;
+		hisilicon,hi6421-vset = <0x13 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <950000>, <1050000>,
+					      <1100000>, <1170000>,
+					      <1134000>, <1150000>,
+					      <1167000>, <1200000>;
+		hisilicon,hi6421-off-on-delay-us = <20000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+	};
+
+	buck4: buck@14 {
+		compatible = "hisilicon,hi6421-buck345";
+		regulator-name = "BUCK4";
+		regulator-min-microvolt = <1150000>;
+		regulator-max-microvolt = <2000000>;
+		hisilicon,hi6421-ctrl = <0x14 0x01 0x10>;
+		hisilicon,hi6421-vset = <0x15 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1150000>, <1200000>,
+					      <1250000>, <1350000>,
+					      <1700000>, <1800000>,
+					      <1900000>, <2000000>;
+		hisilicon,hi6421-off-on-delay-us = <20000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+	};
+
+	buck5: buck@16 {
+		compatible = "hisilicon,hi6421-buck345";
+		regulator-name = "BUCK5";
+		regulator-min-microvolt = <1150000>;
+		regulator-max-microvolt = <1900000>;
+		hisilicon,hi6421-ctrl = <0x16 0x01 0x10>;
+		hisilicon,hi6421-vset = <0x17 0x07>;
+		hisilicon,hi6421-n-voltages = <8>;
+		hisilicon,hi6421-vset-table = <1150000>, <1200000>,
+					      <1250000>, <1350000>,
+					      <1600000>, <1700000>,
+					      <1800000>, <1900000>;
+		hisilicon,hi6421-off-on-delay-us = <20000>;
+		hisilicon,hi6421-enable-time-us = <250>;
+	};
+}; /* end of pmic */
-- 
1.7.4.1

Re: [PATCH 1/3] mfd: Add hi6421 PMIC core driver

[Mark Brown]

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

On Tue, Jun 04, 2013 at 10:28:41PM +0800, Guodong Xu wrote:

> +static struct of_device_id of_hi6421_pmic_child_match_tbl[] = {
> +	/* regulators */
> +	{
> +		.compatible = "hisilicon,hi6421-ldo",
> +	},
> +	{
> +		.compatible = "hisilicon,hi6421-buck012",
> +	},
> +	{
> +		.compatible = "hisilicon,hi6421-buck345",
> +	},
> +	{ /* end */ }
> +};

I would expect this to be in the regulator driver.

> +/*
> + * The PMIC register is only 8-bit.
> + * Hisilicon SoC use hardware to map PMIC register into SoC mapping.
> + * At here, we are accessing SoC register with 32-bit.
> + */
> +u32 hi6421_pmic_read(struct hi6421_pmic *pmic, int reg)
> +{
> +	unsigned long flags;
> +	u32 ret;
> +	spin_lock_irqsave(&pmic->lock, flags);
> +	ret = readl_relaxed(pmic->regs + (reg << 2));
> +	spin_unlock_irqrestore(&pmic->lock, flags);
> +	return ret;
> +}
> +EXPORT_SYMBOL(hi6421_pmic_read);

This all looks like you want to be using regmap MMIO.  This would save a
bit of code and get you access to infrastructure like tracepoints and
debugfs regiseter dumps as well as the regulator API helpers.

> +	if (!devm_request_mem_region(dev, pmic->res->start,
> +				     resource_size(pmic->res),
> +				     pdev->name)) {
> +		dev_err(dev, "cannot claim register memory\n");
> +		return -ENOMEM;
> +	}
> +
> +	pmic->regs = devm_ioremap(dev, pmic->res->start,
> +				  resource_size(pmic->res));

devm_request_and_ioremap().

> +	/* populate sub nodes */
> +	of_platform_populate(np, of_hi6421_pmic_child_match_tbl, NULL, dev);

You should be using the MFD API for this.

> +static int hi6421_pmic_remove(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct hi6421_pmic *pmic = platform_get_drvdata(pdev);
> +
> +	devm_iounmap(dev, pmic->regs);
> +	devm_release_mem_region(dev, pmic->res->start,
> +				resource_size(pmic->res));
> +	devm_kfree(dev, pmic);

No point in cleaning up devm_ stuff, a major goal of the API is to save
writing that code.  The only thing that's needed is to remove the MFD
children which you're not doing.

> +	platform_set_drvdata(pdev, NULL);

This isn't required, it's never OK to access the driver data unless from
code that set it.

> +#define	OCP_DEB_CTRL_REG		(0x51)
> +#define	OCP_DEB_SEL_MASK		(0x0C)
> +#define OCP_DEB_SEL_8MS			(0x00)
> +#define OCP_DEB_SEL_16MS		(0x04)
> +#define OCP_DEB_SEL_32MS		(0x08)
> +#define OCP_DEB_SEL_64MS		(0x0C)
> +#define OCP_EN_DEBOUNCE_MASK		(0x02)
> +#define OCP_EN_DEBOUNCE_ENABLE		(0x02)
> +#define OCP_AUTO_STOP_MASK		(0x01)
> +#define OCP_AUTO_STOP_ENABLE		(0x01)

These should be namespaced.

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

Re: [PATCH 3/3] ARM: dts: add dedicated dtsi for hi6421

[Mark Brown]

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

On Tue, Jun 04, 2013 at 10:28:43PM +0800, Guodong Xu wrote:

> +	ldo1: ldo@21 {
> +		compatible = "hisilicon,hi6421-ldo";
> +		regulator-name = "LDO1";
> +		regulator-min-microvolt = <1700000>;
> +		regulator-max-microvolt = <2000000>;

This all looks very board specific.  There's no point in names like
"LDO1" really - the goal for names is to provide something which matches
the schematics to aid diagnostics.

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

Re: [PATCH 2/3] regulator: add driver for hi6421 voltage regulator

[Mark Brown]

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

On Tue, Jun 04, 2013 at 10:28:42PM +0800, Guodong Xu wrote:

> +Required properties:
> +- compatible: three types of regulator are defined,
> +	- "hisilicon,hi6421-ldo"
> +	- "hisilicon,hi6421-buck012"
> +	- "hisilicon,hi6421-buck345"

What do these mean?

> +- hisilicon,hi6421-ctrl: <ctrl_reg enable_mask eco_mode_mask>
> +- hisilicon,hi6421-vset: <vset_reg vset_mask>
> +- hisilicon,hi6421-n-voltages: <n>
> +- hisilicon,hi6421-vset-table: array of voltages selectable in this regulator
> +- hisilicon,hi6421-uv-step: <uV_step>
> +- hisilicon,hi6421-off-on-delay-us: <off_on_delay>
> +- hisilicon,hi6421-enable-time-us: <enable_time>

This lot are really not at all driver specific, they would apply to any
regulator which is able to use helpers like the regmap ones.  I have to
say I'm not a fan of dumping this much stuff into device tree (since it
means that the driver isn't able to figure out much of the hardware by
itself) but if we are going to do this then we should just define this
as a generic binding which any regulator can reuse rather than as one
that is specific to a particular driver.

> +Properties defined by the standard binding for regulators: (See regulator.txt)
> +- regulator-name:
> +- regulator-min-microvolt:
> +- regulator-max-microvolt:
> +- regulator-boot-on:
> +- regulator-always-on:

No need to list the properties, any new ones added in the core ought to
be supported.

> +Optional properties:
> +- hisilicon,hi6421-eco-microamp: maximum current allowed in ECO mode (in uA)

It's *possible* that whatever "ECO mode" is might be device specific but
I rather suspect it's just a low power mode in which case it's fairly
generic (the code certainly looks that way).

> +++ b/drivers/regulator/Kconfig
> @@ -514,5 +514,11 @@ config REGULATOR_AS3711
>  	  This driver provides support for the voltage regulators on the
>  	  AS3711 PMIC
>  
> -endif
> +config REGULATOR_HI6421
> +	tristate "HiSilicon Hi6421 PMIC voltage regulator support"
> +	depends on MFD_HI6421_PMIC && OF
> +	help
> +	  This driver provides support for the voltage regulators on the
> +	  HiSilicon Hi6421 PMU / Codec IC.

Keep these files sorted as much as possible.

> +static DEFINE_MUTEX(enable_mutex);
> +struct timeval last_enabled;

No globals.  Coordinate through the MFD if you have to.

> +/* helper function to ensure when it returns it is at least 'delay_us'
> + * microseconds after 'since'.
> + */
> +static void ensured_time_after(struct timeval since, u32 delay_us)
> +{

This should obviously be in generic code if it's needed.  Should also be
"ensure" not "ensured".

> +	}
> +	return;
> +}

The return is pointless.

> +static int hi6421_regulator_is_enabled(struct regulator_dev *dev)
> +{

If you convert the core to regmap almost all of the code in this driver
can be replaced with regmap helpers.

> +static int hi6421_regulator_enable(struct regulator_dev *dev)
> +{
> +	struct hi6421_regulator *sreg = rdev_get_drvdata(dev);
> +	struct hi6421_pmic *pmic = rdev_to_pmic(dev);
> +
> +	/* keep a distance of off_on_delay from last time disabled */
> +	ensured_time_after(sreg->last_off_time, sreg->off_on_delay);
> +
> +	/* cannot enable more than one regulator at one time */
> +	mutex_lock(&enable_mutex);
> +	ensured_time_after(last_enabled, HI6421_REGS_ENA_PROTECT_TIME);
> +
> +	/* set enable register */
> +	hi6421_pmic_rmw(pmic, sreg->register_info.ctrl_reg,
> +				sreg->register_info.enable_mask,
> +				sreg->register_info.enable_mask);
> +
> +	do_gettimeofday(&last_enabled);
> +	mutex_unlock(&enable_mutex);

What exactly is the constraint here?  It's very unusual for a regulator
to have a constraint requiring a delay between power off and power on,
or for there to be restrictions on powering up multiple regulators
simulataneously.  If there are such constraints why don't they also
affect voltage changes?

> +static int hi6421_regulator_ldo_set_voltage(struct regulator_dev *dev,
> +				int min_uV, int max_uV, unsigned *selector)
> +{

Implement map_voltage() and set_voltage_sel().

> +unsigned int hi6421_regulator_get_optimum_mode(struct regulator_dev *dev,
> +			int input_uV, int output_uV, int load_uA)
> +{
> +	struct hi6421_regulator *sreg = rdev_get_drvdata(dev);
> +
> +	if ((load_uA == 0) || (load_uA > sreg->eco_uA))
> +		return REGULATOR_MODE_NORMAL;
> +	else
> +		return REGULATOR_MODE_IDLE;

Why normal mode if the load is zero?

> +static const struct hi6421_regulator hi6421_regulator_ldo = {
> +	.rdesc = {
> +	.ops = &hi6421_ldo_rops,
> +		.type = REGULATOR_VOLTAGE,
> +		.owner = THIS_MODULE,
> +		},

Coding style.

> +	sreg->name = initdata->constraints.name;
> +	rdesc = &sreg->rdesc;
> +	rdesc->name = sreg->name;

You're abusing the name property here.  The descriptor name should
reflect the name the device has in silicon but you're using the
constraints name which should reflect the name of the supply on the
board.

> +	rdesc->min_uV = initdata->constraints.min_uV;

Similarly this should reflect the silicon not the board.

> +hi6421_probe_end:
> +	if (ret)
> +		kfree(sreg);

Use devm.

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