2017-06-06 08:52:08

by Guodong Xu

[permalink] [raw]
Subject: [PATCH v4 0/8] MFD: add driver for HiSilicon Hi6421v530 PMIC

This patchset adds driver for HiSilicon Hi6421v530 PMIC.

Mainline kernel already has driver support to a similar chip, Hi6421.
Hi6421 and Hi6421v530 are both from the same vendor, HiSilicon, but
they are at different revisions. They both use the same Memory-mapped
I/O method to communicate with Main SoC. However, they differ quite a
lot in their regulator designs. Eg. they have completely different LDO
voltage points.

Patch 1 and 2 are hi6421-pmic cleaning up.
Patch 3 and 4 extends hi6421-pmic-core.c to support Hi6421v530 revision.
Patch 5 add hi6421v530-regulator.c driver for LDO regulators.
Patch 6 fixes an issue for hi6421 regulator, which is not related to v530
but it's found in this review.
Patch 7 is dts change, it depends on and can be applied to hi3660/hikey960
patchset [1].
Patch 8 enables the relevant config items.

[1], http://www.spinics.net/lists/devicetree/msg178303.html

Major changes in v4:
- put hi6421-pmic cleanup in separate patches.
- solve review comments from Lee Johes.
- regulator-name should not have '/' character. Otherwise it "Failed to
create debugfs directory"

Major changes in v3:
- in hi6421-pmic-core.c
* use shorter license script.
* arrange #include in alphabetical order.
* using recommended error log messages from Lee Jones.
- in hi6421v530-regulator.c
* remove unused #include files
* arrange remaining ones in alphabetical order.

Major changes in v2:
- instead of writing a new driver, extend hi6421-pmic-core.c
to support its v530 revision
- update hi6421v530-regulator.c to use modern regulator driver
design logics.

Guodong Xu (6):
mfd: hi6421-pmic: cleanup: change license text to shorter form
mfd: hi6421-pmic: cleanup: update dev_err messages
dt-bindings: mfd: hi6421: Add hi6421v530 compatible string
mfd: hi6421-pmic: add support for HiSilicon Hi6421v530
regulator: hi6421: Describe consumed platform device
arm64: defconfig: enable support hi6421v530 PMIC

Wang Xiaoyin (2):
regulator: hi6421v530: add driver for hi6421v530 voltage regulator
arm64: dts: hikey960: add device node for pmic and regulators

Documentation/devicetree/bindings/mfd/hi6421.txt | 4 +-
arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts | 46 +++++
arch/arm64/configs/defconfig | 2 +
drivers/mfd/hi6421-pmic-core.c | 87 +++++----
drivers/regulator/Kconfig | 10 ++
drivers/regulator/Makefile | 1 +
drivers/regulator/hi6421-regulator.c | 7 +
drivers/regulator/hi6421v530-regulator.c | 207 ++++++++++++++++++++++
include/linux/mfd/hi6421-pmic.h | 5 +
9 files changed, 336 insertions(+), 33 deletions(-)
create mode 100644 drivers/regulator/hi6421v530-regulator.c

--
2.10.2


2017-06-06 08:52:29

by Guodong Xu

[permalink] [raw]
Subject: [PATCH v4 1/8] mfd: hi6421-pmic: cleanup: change license text to shorter form

Change license text to a shorter form of GPLv2.

Signed-off-by: Guodong Xu <[email protected]>
---
drivers/mfd/hi6421-pmic-core.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/mfd/hi6421-pmic-core.c b/drivers/mfd/hi6421-pmic-core.c
index 3fd703f..ad9e3d8 100644
--- a/drivers/mfd/hi6421-pmic-core.c
+++ b/drivers/mfd/hi6421-pmic-core.c
@@ -9,17 +9,8 @@
* Author: Guodong Xu <[email protected]>
*
* 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 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, see <http://www.gnu.org/licenses/>.
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
*/

#include <linux/device.h>
--
2.10.2

2017-06-06 08:52:42

by Guodong Xu

[permalink] [raw]
Subject: [PATCH v4 3/8] dt-bindings: mfd: hi6421: Add hi6421v530 compatible string

Add compatible string for HiSilicon Hi6421v530 PMIC.

Signed-off-by: Guodong Xu <[email protected]>
Acked-for-mfd-by: Lee Jones <[email protected]>
---
Documentation/devicetree/bindings/mfd/hi6421.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mfd/hi6421.txt b/Documentation/devicetree/bindings/mfd/hi6421.txt
index 0d5a446..22da96d 100644
--- a/Documentation/devicetree/bindings/mfd/hi6421.txt
+++ b/Documentation/devicetree/bindings/mfd/hi6421.txt
@@ -1,7 +1,9 @@
* HI6421 Multi-Functional Device (MFD), by HiSilicon Ltd.

Required parent device properties:
-- compatible : contains "hisilicon,hi6421-pmic";
+- compatible : One of the following chip-specific strings:
+ "hisilicon,hi6421-pmic";
+ "hisilicon,hi6421v530-pmic";
- reg : register range space of hi6421;

Supported Hi6421 sub-devices include:
--
2.10.2

2017-06-06 08:52:57

by Guodong Xu

[permalink] [raw]
Subject: [PATCH v4 4/8] mfd: hi6421-pmic: add support for HiSilicon Hi6421v530

Add support for HiSilicon Hi6421v530 PMIC. Hi6421v530 communicates with
main SoC via memory-mapped I/O.

Hi6421v530 and Hi6421 are PMIC chips from the same vendor, HiSilicon,
but at different revisions. They share the same memory-mapped I/O
design. They differ in integrated devices, such as regulator details,
LDO voltage points.

Signed-off-by: Wang Xiaoyin <[email protected]>
Signed-off-by: Guodong Xu <[email protected]>
---
drivers/mfd/hi6421-pmic-core.c | 68 ++++++++++++++++++++++++++++++-----------
include/linux/mfd/hi6421-pmic.h | 5 +++
2 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/drivers/mfd/hi6421-pmic-core.c b/drivers/mfd/hi6421-pmic-core.c
index b1139d4..4568a43 100644
--- a/drivers/mfd/hi6421-pmic-core.c
+++ b/drivers/mfd/hi6421-pmic-core.c
@@ -1,9 +1,9 @@
/*
- * Device driver for Hi6421 IC
+ * Device driver for Hi6421 PMIC
*
* Copyright (c) <2011-2014> HiSilicon Technologies Co., Ltd.
* http://www.hisilicon.com
- * Copyright (c) <2013-2014> Linaro Ltd.
+ * Copyright (c) <2013-2017> Linaro Ltd.
* http://www.linaro.org
*
* Author: Guodong Xu <[email protected]>
@@ -16,16 +16,20 @@
#include <linux/device.h>
#include <linux/err.h>
#include <linux/mfd/core.h>
+#include <linux/mfd/hi6421-pmic.h>
#include <linux/module.h>
-#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
-#include <linux/mfd/hi6421-pmic.h>

static const struct mfd_cell hi6421_devs[] = {
{ .name = "hi6421-regulator", },
};

+static const struct mfd_cell hi6421v530_devs[] = {
+ { .name = "hi6421v530-regulator", },
+};
+
static const struct regmap_config hi6421_regmap_config = {
.reg_bits = 32,
.reg_stride = 4,
@@ -33,12 +37,31 @@ static const struct regmap_config hi6421_regmap_config = {
.max_register = HI6421_REG_TO_BUS_ADDR(HI6421_REG_MAX),
};

+static const struct of_device_id of_hi6421_pmic_match[] = {
+ {
+ .compatible = "hisilicon,hi6421-pmic",
+ .data = (void *)HI6421
+ },
+ {
+ .compatible = "hisilicon,hi6421v530-pmic",
+ .data = (void *)HI6421_V530
+ },
+ { },
+};
+MODULE_DEVICE_TABLE(of, of_hi6421_pmic_match);
+
static int hi6421_pmic_probe(struct platform_device *pdev)
{
struct hi6421_pmic *pmic;
struct resource *res;
+ const struct of_device_id *id;
+ const struct mfd_cell *subdevs;
void __iomem *base;
- int ret;
+ int n_subdevs, ret;
+
+ id = of_match_device(of_hi6421_pmic_match, &pdev->dev);
+ if (!id)
+ return -EINVAL;

pmic = devm_kzalloc(&pdev->dev, sizeof(*pmic), GFP_KERNEL);
if (!pmic)
@@ -57,18 +80,33 @@ static int hi6421_pmic_probe(struct platform_device *pdev)
return PTR_ERR(pmic->regmap);
}

- /* set over-current protection debounce 8ms */
- regmap_update_bits(pmic->regmap, HI6421_OCP_DEB_CTRL_REG,
+ platform_set_drvdata(pdev, pmic);
+
+ switch ((unsigned long)id->data) {
+ case HI6421:
+ /* set over-current protection debounce 8ms */
+ regmap_update_bits(pmic->regmap, HI6421_OCP_DEB_CTRL_REG,
(HI6421_OCP_DEB_SEL_MASK
| HI6421_OCP_EN_DEBOUNCE_MASK
| HI6421_OCP_AUTO_STOP_MASK),
(HI6421_OCP_DEB_SEL_8MS
| HI6421_OCP_EN_DEBOUNCE_ENABLE));

- platform_set_drvdata(pdev, pmic);
+ subdevs = hi6421_devs;
+ n_subdevs = ARRAY_SIZE(hi6421_devs);
+ break;
+ case HI6421_V530:
+ subdevs = hi6421v530_devs;
+ n_subdevs = ARRAY_SIZE(hi6421v530_devs);
+ break;
+ default:
+ dev_err(&pdev->dev, "Unknown device type %ld\n",
+ (unsigned long)id->data);
+ return -EINVAL;
+ }

- ret = devm_mfd_add_devices(&pdev->dev, 0, hi6421_devs,
- ARRAY_SIZE(hi6421_devs), NULL, 0, NULL);
+ ret = devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE,
+ subdevs, n_subdevs, NULL, 0, NULL);
if (ret) {
dev_err(&pdev->dev, "Failed to add child devices: %d\n", ret);
return ret;
@@ -77,16 +115,10 @@ static int hi6421_pmic_probe(struct platform_device *pdev)
return 0;
}

-static const struct of_device_id of_hi6421_pmic_match_tbl[] = {
- { .compatible = "hisilicon,hi6421-pmic", },
- { },
-};
-MODULE_DEVICE_TABLE(of, of_hi6421_pmic_match_tbl);
-
static struct platform_driver hi6421_pmic_driver = {
.driver = {
- .name = "hi6421_pmic",
- .of_match_table = of_hi6421_pmic_match_tbl,
+ .name = "hi6421_pmic",
+ .of_match_table = of_hi6421_pmic_match,
},
.probe = hi6421_pmic_probe,
};
diff --git a/include/linux/mfd/hi6421-pmic.h b/include/linux/mfd/hi6421-pmic.h
index 587273e..2457438 100644
--- a/include/linux/mfd/hi6421-pmic.h
+++ b/include/linux/mfd/hi6421-pmic.h
@@ -38,4 +38,9 @@ struct hi6421_pmic {
struct regmap *regmap;
};

+enum hi6421_type {
+ HI6421 = 1,
+ HI6421_V530 = 2,
+};
+
#endif /* __HI6421_PMIC_H */
--
2.10.2

2017-06-06 08:53:08

by Guodong Xu

[permalink] [raw]
Subject: [PATCH v4 5/8] regulator: hi6421v530: add driver for hi6421v530 voltage regulator

From: Wang Xiaoyin <[email protected]>

add the driver for hi6421v530 voltage regulator

Signed-off-by: Wang Xiaoyin <[email protected]>
Signed-off-by: Guodong Xu <[email protected]>
---
drivers/regulator/Kconfig | 10 ++
drivers/regulator/Makefile | 1 +
drivers/regulator/hi6421v530-regulator.c | 207 +++++++++++++++++++++++++++++++
3 files changed, 218 insertions(+)
create mode 100644 drivers/regulator/hi6421v530-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 48db87d..78cd8d8 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -296,6 +296,16 @@ config REGULATOR_HI6421
21 general purpose LDOs, 3 dedicated LDOs, and 5 BUCKs. All
of them come with support to either ECO (idle) or sleep mode.

+config REGULATOR_HI6421V530
+ tristate "HiSilicon Hi6421v530 PMIC voltage regulator support"
+ depends on MFD_HI6421_PMIC && OF
+ help
+ This driver provides support for the voltage regulators on
+ HiSilicon Hi6421v530 PMU / Codec IC.
+ Hi6421v530 is a multi-function device which, on regulator part,
+ provides 5 general purpose LDOs, and all of them come with support
+ to either ECO (idle) or sleep mode.
+
config REGULATOR_HI655X
tristate "Hisilicon HI655X PMIC regulators support"
depends on ARCH_HISI || COMPILE_TEST
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index dc3503f..36e2b75 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_REGULATOR_DB8500_PRCMU) += db8500-prcmu.o
obj-$(CONFIG_REGULATOR_FAN53555) += fan53555.o
obj-$(CONFIG_REGULATOR_GPIO) += gpio-regulator.o
obj-$(CONFIG_REGULATOR_HI6421) += hi6421-regulator.o
+obj-$(CONFIG_REGULATOR_HI6421V530) += hi6421v530-regulator.o
obj-$(CONFIG_REGULATOR_HI655X) += hi655x-regulator.o
obj-$(CONFIG_REGULATOR_ISL6271A) += isl6271a-regulator.o
obj-$(CONFIG_REGULATOR_ISL9305) += isl9305.o
diff --git a/drivers/regulator/hi6421v530-regulator.c b/drivers/regulator/hi6421v530-regulator.c
new file mode 100644
index 0000000..46bbba9
--- /dev/null
+++ b/drivers/regulator/hi6421v530-regulator.c
@@ -0,0 +1,207 @@
+/*
+ * Device driver for regulators in Hi6421V530 IC
+ *
+ * Copyright (c) <2017> HiSilicon Technologies Co., Ltd.
+ * http://www.hisilicon.com
+ * Copyright (c) <2017> Linaro Ltd.
+ * http://www.linaro.org
+ *
+ * Author: Wang Xiaoyin <[email protected]>
+ * Guodong Xu <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/mfd/hi6421-pmic.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+
+/*
+ * struct hi6421v530_regulator_info - hi6421v530 regulator information
+ * @desc: regulator description
+ * @mode_mask: ECO mode bitmask of LDOs; for BUCKs, this masks sleep
+ * @eco_microamp: eco mode load upper limit (in uA), valid for LDOs only
+ */
+struct hi6421v530_regulator_info {
+ struct regulator_desc rdesc;
+ u8 mode_mask;
+ u32 eco_microamp;
+};
+
+/* HI6421v530 regulators */
+enum hi6421v530_regulator_id {
+ HI6421V530_LDO3,
+ HI6421V530_LDO9,
+ HI6421V530_LDO11,
+ HI6421V530_LDO15,
+ HI6421V530_LDO16,
+};
+
+static const unsigned int ldo_3_voltages[] = {
+ 1800000, 1825000, 1850000, 1875000,
+ 1900000, 1925000, 1950000, 1975000,
+ 2000000, 2025000, 2050000, 2075000,
+ 2100000, 2125000, 2150000, 2200000,
+};
+
+static const unsigned int ldo_9_11_voltages[] = {
+ 1750000, 1800000, 1825000, 2800000,
+ 2850000, 2950000, 3000000, 3300000,
+};
+
+static const unsigned int ldo_15_16_voltages[] = {
+ 1750000, 1800000, 2400000, 2600000,
+ 2700000, 2850000, 2950000, 3000000,
+};
+
+static const struct regulator_ops hi6421v530_ldo_ops;
+
+#define HI6421V530_LDO_ENABLE_TIME (350)
+
+/*
+ * _id - LDO id name string
+ * v_table - voltage table
+ * vreg - voltage select register
+ * vmask - voltage select mask
+ * ereg - enable register
+ * emask - enable mask
+ * odelay - off/on delay time in uS
+ * ecomask - eco mode mask
+ * ecoamp - eco mode load uppler limit in uA
+ */
+#define HI6421V530_LDO(_ID, v_table, vreg, vmask, ereg, emask, \
+ odelay, ecomask, ecoamp) { \
+ .rdesc = { \
+ .name = #_ID, \
+ .of_match = of_match_ptr(#_ID), \
+ .regulators_node = of_match_ptr("regulators"), \
+ .ops = &hi6421v530_ldo_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .id = HI6421V530_##_ID, \
+ .owner = THIS_MODULE, \
+ .n_voltages = ARRAY_SIZE(v_table), \
+ .volt_table = v_table, \
+ .vsel_reg = HI6421_REG_TO_BUS_ADDR(vreg), \
+ .vsel_mask = vmask, \
+ .enable_reg = HI6421_REG_TO_BUS_ADDR(ereg), \
+ .enable_mask = emask, \
+ .enable_time = HI6421V530_LDO_ENABLE_TIME, \
+ .off_on_delay = odelay, \
+ }, \
+ .mode_mask = ecomask, \
+ .eco_microamp = ecoamp, \
+}
+
+/* HI6421V530 regulator information */
+
+static struct hi6421v530_regulator_info hi6421v530_regulator_info[] = {
+ HI6421V530_LDO(LDO3, ldo_3_voltages, 0x061, 0xf, 0x060, 0x2,
+ 20000, 0x6, 8000),
+ HI6421V530_LDO(LDO9, ldo_9_11_voltages, 0x06b, 0x7, 0x06a, 0x2,
+ 40000, 0x6, 8000),
+ HI6421V530_LDO(LDO11, ldo_9_11_voltages, 0x06f, 0x7, 0x06e, 0x2,
+ 40000, 0x6, 8000),
+ HI6421V530_LDO(LDO15, ldo_15_16_voltages, 0x077, 0x7, 0x076, 0x2,
+ 40000, 0x6, 8000),
+ HI6421V530_LDO(LDO16, ldo_15_16_voltages, 0x079, 0x7, 0x078, 0x2,
+ 40000, 0x6, 8000),
+};
+
+static unsigned int hi6421v530_regulator_ldo_get_mode(
+ struct regulator_dev *rdev)
+{
+ struct hi6421v530_regulator_info *info;
+ unsigned int reg_val;
+
+ info = rdev_get_drvdata(rdev);
+ regmap_read(rdev->regmap, rdev->desc->enable_reg, &reg_val);
+
+ if (reg_val & (info->mode_mask))
+ return REGULATOR_MODE_IDLE;
+
+ return REGULATOR_MODE_NORMAL;
+}
+
+static int hi6421v530_regulator_ldo_set_mode(struct regulator_dev *rdev,
+ unsigned int mode)
+{
+ struct hi6421v530_regulator_info *info;
+ unsigned int new_mode;
+
+ info = rdev_get_drvdata(rdev);
+ switch (mode) {
+ case REGULATOR_MODE_NORMAL:
+ new_mode = 0;
+ break;
+ case REGULATOR_MODE_IDLE:
+ new_mode = info->mode_mask;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
+ info->mode_mask, new_mode);
+
+ return 0;
+}
+
+
+static const struct regulator_ops hi6421v530_ldo_ops = {
+ .is_enabled = regulator_is_enabled_regmap,
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .list_voltage = regulator_list_voltage_table,
+ .map_voltage = regulator_map_voltage_ascend,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_mode = hi6421v530_regulator_ldo_get_mode,
+ .set_mode = hi6421v530_regulator_ldo_set_mode,
+};
+
+static int hi6421v530_regulator_probe(struct platform_device *pdev)
+{
+ struct hi6421_pmic *pmic;
+ struct regulator_dev *rdev;
+ struct regulator_config config = { };
+ unsigned int i;
+
+ pmic = dev_get_drvdata(pdev->dev.parent);
+ if (!pmic) {
+ dev_err(&pdev->dev, "no pmic in the regulator parent node\n");
+ return -ENODEV;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(hi6421v530_regulator_info); i++) {
+ config.dev = pdev->dev.parent;
+ config.regmap = pmic->regmap;
+ config.driver_data = &hi6421v530_regulator_info[i];
+
+ rdev = devm_regulator_register(&pdev->dev,
+ &hi6421v530_regulator_info[i].rdesc,
+ &config);
+ if (IS_ERR(rdev)) {
+ dev_err(&pdev->dev, "failed to register regulator %s\n",
+ hi6421v530_regulator_info[i].rdesc.name);
+ return PTR_ERR(rdev);
+ }
+ }
+ return 0;
+}
+
+static struct platform_driver hi6421v530_regulator_driver = {
+ .driver = {
+ .name = "hi6421v530-regulator",
+ },
+ .probe = hi6421v530_regulator_probe,
+};
+module_platform_driver(hi6421v530_regulator_driver);
+
+MODULE_AUTHOR("Wang Xiaoyin <[email protected]>");
+MODULE_DESCRIPTION("Hi6421v530 regulator driver");
+MODULE_LICENSE("GPL v2");
--
2.10.2

2017-06-06 08:53:20

by Guodong Xu

[permalink] [raw]
Subject: [PATCH v4 6/8] regulator: hi6421: Describe consumed platform device

The hi6421-regulator driver consumes a similarly named platform device.
Adding that to the module device table, allows modprobe to locate this
driver once the device is created.

Cc: Jeremy Linton <[email protected]>
Cc: Mark Brown <[email protected]>
Signed-off-by: Guodong Xu <[email protected]>
---
drivers/regulator/hi6421-regulator.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/regulator/hi6421-regulator.c b/drivers/regulator/hi6421-regulator.c
index 62c5f54..259c3a8 100644
--- a/drivers/regulator/hi6421-regulator.c
+++ b/drivers/regulator/hi6421-regulator.c
@@ -621,7 +621,14 @@ static int hi6421_regulator_probe(struct platform_device *pdev)
return 0;
}

+static const struct platform_device_id hi6421_regulator_table[] = {
+ { .name = "hi6421-regulator" },
+ {},
+};
+MODULE_DEVICE_TABLE(platform, hi6421_regulator_table);
+
static struct platform_driver hi6421_regulator_driver = {
+ .id_table = hi6421_regulator_table,
.driver = {
.name = "hi6421-regulator",
},
--
2.10.2

2017-06-06 08:53:38

by Guodong Xu

[permalink] [raw]
Subject: [PATCH v4 8/8] arm64: defconfig: enable support hi6421v530 PMIC

Enable configs for hi6421v530 mfd and regulator driver
+ CONFIG_MFD_HI6421_PMIC=y
+ CONFIG_REGULATOR_HI6421V530=y

Signed-off-by: Guodong Xu <[email protected]>
---
arch/arm64/configs/defconfig | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index ce07285..867d68c 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -305,6 +305,7 @@ CONFIG_S3C2410_WATCHDOG=y
CONFIG_MESON_GXBB_WATCHDOG=m
CONFIG_MESON_WATCHDOG=m
CONFIG_MFD_EXYNOS_LPASS=m
+CONFIG_MFD_HI6421_PMIC=y
CONFIG_MFD_MAX77620=y
CONFIG_MFD_RK808=y
CONFIG_MFD_SPMI_PMIC=y
@@ -315,6 +316,7 @@ CONFIG_MFD_CROS_EC=y
CONFIG_MFD_CROS_EC_I2C=y
CONFIG_REGULATOR_FIXED_VOLTAGE=y
CONFIG_REGULATOR_GPIO=y
+CONFIG_REGULATOR_HI6421V530=y
CONFIG_REGULATOR_HI655X=y
CONFIG_REGULATOR_MAX77620=y
CONFIG_REGULATOR_PWM=y
--
2.10.2

2017-06-06 08:53:29

by Guodong Xu

[permalink] [raw]
Subject: [PATCH v4 7/8] arm64: dts: hikey960: add device node for pmic and regulators

From: Wang Xiaoyin <[email protected]>

add device node for hi6421 pmic core and hi6421v530
voltage regulator,include LDO(1,3,9,11,15,16)

Signed-off-by: Wang Xiaoyin <[email protected]>
Signed-off-by: Guodong Xu <[email protected]>
---
arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts | 46 +++++++++++++++++++++++
1 file changed, 46 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
index ca448f0..e579333 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
@@ -97,6 +97,52 @@
default-state = "off";
};
};
+
+ pmic: pmic@fff34000 {
+ compatible = "hisilicon,hi6421v530-pmic";
+ reg = <0x0 0xfff34000 0x0 0x1000>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+
+ regulators {
+ ldo3: LDO3 { /* HDMI */
+ regulator-name = "VOUT3_1V85";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <2200000>;
+ regulator-enable-ramp-delay = <120>;
+ };
+
+ ldo9: LDO9 { /* SDCARD I/O */
+ regulator-name = "VOUT9_1V8_2V95";
+ regulator-min-microvolt = <1750000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-enable-ramp-delay = <240>;
+ };
+
+ ldo11: LDO11 { /* Low Speed Connector */
+ regulator-name = "VOUT11_1V8_2V95";
+ regulator-min-microvolt = <1750000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-enable-ramp-delay = <240>;
+ };
+
+ ldo15: LDO15 { /* UFS VCC */
+ regulator-name = "VOUT15_3V0";
+ regulator-min-microvolt = <1750000>;
+ regulator-max-microvolt = <3000000>;
+ regulator-boot-on;
+ regulator-always-on;
+ regulator-enable-ramp-delay = <120>;
+ };
+
+ ldo16: LDO16 { /* SD VDD */
+ regulator-name = "VOUT16_2V95";
+ regulator-min-microvolt = <1750000>;
+ regulator-max-microvolt = <3000000>;
+ regulator-enable-ramp-delay = <360>;
+ };
+ };
+ };
};

&i2c0 {
--
2.10.2

2017-06-06 08:54:41

by Guodong Xu

[permalink] [raw]
Subject: [PATCH v4 2/8] mfd: hi6421-pmic: cleanup: update dev_err messages

Update dev_err messages to make them more readable.

Signed-off-by: Guodong Xu <[email protected]>
---
drivers/mfd/hi6421-pmic-core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/hi6421-pmic-core.c b/drivers/mfd/hi6421-pmic-core.c
index ad9e3d8..b1139d4 100644
--- a/drivers/mfd/hi6421-pmic-core.c
+++ b/drivers/mfd/hi6421-pmic-core.c
@@ -52,8 +52,8 @@ static int hi6421_pmic_probe(struct platform_device *pdev)
pmic->regmap = devm_regmap_init_mmio_clk(&pdev->dev, NULL, base,
&hi6421_regmap_config);
if (IS_ERR(pmic->regmap)) {
- dev_err(&pdev->dev,
- "regmap init failed: %ld\n", PTR_ERR(pmic->regmap));
+ dev_err(&pdev->dev, "Failed to initialise Regmap: %ld\n",
+ PTR_ERR(pmic->regmap));
return PTR_ERR(pmic->regmap);
}

@@ -70,7 +70,7 @@ static int hi6421_pmic_probe(struct platform_device *pdev)
ret = devm_mfd_add_devices(&pdev->dev, 0, hi6421_devs,
ARRAY_SIZE(hi6421_devs), NULL, 0, NULL);
if (ret) {
- dev_err(&pdev->dev, "add mfd devices failed: %d\n", ret);
+ dev_err(&pdev->dev, "Failed to add child devices: %d\n", ret);
return ret;
}

--
2.10.2

2017-06-06 12:54:23

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v4 0/8] MFD: add driver for HiSilicon Hi6421v530 PMIC

On Tue, Jun 6, 2017 at 10:51 AM, Guodong Xu <[email protected]> wrote:
> This patchset adds driver for HiSilicon Hi6421v530 PMIC.
>
> Mainline kernel already has driver support to a similar chip, Hi6421.
> Hi6421 and Hi6421v530 are both from the same vendor, HiSilicon, but
> they are at different revisions. They both use the same Memory-mapped
> I/O method to communicate with Main SoC. However, they differ quite a
> lot in their regulator designs. Eg. they have completely different LDO
> voltage points.
>
> Patch 1 and 2 are hi6421-pmic cleaning up.
> Patch 3 and 4 extends hi6421-pmic-core.c to support Hi6421v530 revision.
> Patch 5 add hi6421v530-regulator.c driver for LDO regulators.
> Patch 6 fixes an issue for hi6421 regulator, which is not related to v530
> but it's found in this review.
> Patch 7 is dts change, it depends on and can be applied to hi3660/hikey960
> patchset [1].
> Patch 8 enables the relevant config items.

Looks good to me,

Arnd

2017-06-06 13:50:01

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v4 1/8] mfd: hi6421-pmic: cleanup: change license text to shorter form

On Tue, 06 Jun 2017, Guodong Xu wrote:

> Change license text to a shorter form of GPLv2.
>
> Signed-off-by: Guodong Xu <[email protected]>
> ---
> drivers/mfd/hi6421-pmic-core.c | 13 ++-----------
> 1 file changed, 2 insertions(+), 11 deletions(-)

For my own reference:
Acked-for-MFD-by: Lee Jones <[email protected]>

> diff --git a/drivers/mfd/hi6421-pmic-core.c b/drivers/mfd/hi6421-pmic-core.c
> index 3fd703f..ad9e3d8 100644
> --- a/drivers/mfd/hi6421-pmic-core.c
> +++ b/drivers/mfd/hi6421-pmic-core.c
> @@ -9,17 +9,8 @@
> * Author: Guodong Xu <[email protected]>
> *
> * 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 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, see <http://www.gnu.org/licenses/>.
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> */
>
> #include <linux/device.h>

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

2017-06-06 13:50:28

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v4 2/8] mfd: hi6421-pmic: cleanup: update dev_err messages

On Tue, 06 Jun 2017, Guodong Xu wrote:

> Update dev_err messages to make them more readable.
>
> Signed-off-by: Guodong Xu <[email protected]>
> ---
> drivers/mfd/hi6421-pmic-core.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)

For my own reference:
Acked-for-MFD-by: Lee Jones <[email protected]>

> diff --git a/drivers/mfd/hi6421-pmic-core.c b/drivers/mfd/hi6421-pmic-core.c
> index ad9e3d8..b1139d4 100644
> --- a/drivers/mfd/hi6421-pmic-core.c
> +++ b/drivers/mfd/hi6421-pmic-core.c
> @@ -52,8 +52,8 @@ static int hi6421_pmic_probe(struct platform_device *pdev)
> pmic->regmap = devm_regmap_init_mmio_clk(&pdev->dev, NULL, base,
> &hi6421_regmap_config);
> if (IS_ERR(pmic->regmap)) {
> - dev_err(&pdev->dev,
> - "regmap init failed: %ld\n", PTR_ERR(pmic->regmap));
> + dev_err(&pdev->dev, "Failed to initialise Regmap: %ld\n",
> + PTR_ERR(pmic->regmap));
> return PTR_ERR(pmic->regmap);
> }
>
> @@ -70,7 +70,7 @@ static int hi6421_pmic_probe(struct platform_device *pdev)
> ret = devm_mfd_add_devices(&pdev->dev, 0, hi6421_devs,
> ARRAY_SIZE(hi6421_devs), NULL, 0, NULL);
> if (ret) {
> - dev_err(&pdev->dev, "add mfd devices failed: %d\n", ret);
> + dev_err(&pdev->dev, "Failed to add child devices: %d\n", ret);
> return ret;
> }
>

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

2017-06-06 13:54:54

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v4 4/8] mfd: hi6421-pmic: add support for HiSilicon Hi6421v530

On Tue, 06 Jun 2017, Guodong Xu wrote:

> Add support for HiSilicon Hi6421v530 PMIC. Hi6421v530 communicates with
> main SoC via memory-mapped I/O.
>
> Hi6421v530 and Hi6421 are PMIC chips from the same vendor, HiSilicon,
> but at different revisions. They share the same memory-mapped I/O
> design. They differ in integrated devices, such as regulator details,
> LDO voltage points.
>
> Signed-off-by: Wang Xiaoyin <[email protected]>
> Signed-off-by: Guodong Xu <[email protected]>
> ---
> drivers/mfd/hi6421-pmic-core.c | 68 ++++++++++++++++++++++++++++++-----------
> include/linux/mfd/hi6421-pmic.h | 5 +++
> 2 files changed, 55 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mfd/hi6421-pmic-core.c b/drivers/mfd/hi6421-pmic-core.c
> index b1139d4..4568a43 100644
> --- a/drivers/mfd/hi6421-pmic-core.c
> +++ b/drivers/mfd/hi6421-pmic-core.c

[...]

> static int hi6421_pmic_probe(struct platform_device *pdev)
> {
> struct hi6421_pmic *pmic;
> struct resource *res;
> + const struct of_device_id *id;
> + const struct mfd_cell *subdevs;
> void __iomem *base;
> - int ret;
> + int n_subdevs, ret;
> +
> + id = of_match_device(of_hi6421_pmic_match, &pdev->dev);
> + if (!id)
> + return -EINVAL;
>
> pmic = devm_kzalloc(&pdev->dev, sizeof(*pmic), GFP_KERNEL);
> if (!pmic)
> @@ -57,18 +80,33 @@ static int hi6421_pmic_probe(struct platform_device *pdev)
> return PTR_ERR(pmic->regmap);
> }
>
> - /* set over-current protection debounce 8ms */
> - regmap_update_bits(pmic->regmap, HI6421_OCP_DEB_CTRL_REG,
> + platform_set_drvdata(pdev, pmic);
> +
> + switch ((unsigned long)id->data) {

Whilst this is not a blocker, it would be nicer to place this into a
'type' variable, in order to clarify what data you are using.

> + case HI6421:
> + /* set over-current protection debounce 8ms */
> + regmap_update_bits(pmic->regmap, HI6421_OCP_DEB_CTRL_REG,
> (HI6421_OCP_DEB_SEL_MASK
> | HI6421_OCP_EN_DEBOUNCE_MASK
> | HI6421_OCP_AUTO_STOP_MASK),
> (HI6421_OCP_DEB_SEL_8MS
> | HI6421_OCP_EN_DEBOUNCE_ENABLE));
>
> - platform_set_drvdata(pdev, pmic);
> + subdevs = hi6421_devs;
> + n_subdevs = ARRAY_SIZE(hi6421_devs);
> + break;
> + case HI6421_V530:
> + subdevs = hi6421v530_devs;
> + n_subdevs = ARRAY_SIZE(hi6421v530_devs);
> + break;
> + default:
> + dev_err(&pdev->dev, "Unknown device type %ld\n",
> + (unsigned long)id->data);
> + return -EINVAL;
> + }
>
> - ret = devm_mfd_add_devices(&pdev->dev, 0, hi6421_devs,
> - ARRAY_SIZE(hi6421_devs), NULL, 0, NULL);
> + ret = devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE,
> + subdevs, n_subdevs, NULL, 0, NULL);
> if (ret) {
> dev_err(&pdev->dev, "Failed to add child devices: %d\n", ret);
> return ret;
> @@ -77,16 +115,10 @@ static int hi6421_pmic_probe(struct platform_device *pdev)
> return 0;
> }
>
> -static const struct of_device_id of_hi6421_pmic_match_tbl[] = {
> - { .compatible = "hisilicon,hi6421-pmic", },
> - { },
> -};
> -MODULE_DEVICE_TABLE(of, of_hi6421_pmic_match_tbl);
> -
> static struct platform_driver hi6421_pmic_driver = {
> .driver = {
> - .name = "hi6421_pmic",
> - .of_match_table = of_hi6421_pmic_match_tbl,
> + .name = "hi6421_pmic",
> + .of_match_table = of_hi6421_pmic_match,
> },
> .probe = hi6421_pmic_probe,
> };
> diff --git a/include/linux/mfd/hi6421-pmic.h b/include/linux/mfd/hi6421-pmic.h
> index 587273e..2457438 100644
> --- a/include/linux/mfd/hi6421-pmic.h
> +++ b/include/linux/mfd/hi6421-pmic.h
> @@ -38,4 +38,9 @@ struct hi6421_pmic {
> struct regmap *regmap;
> };
>
> +enum hi6421_type {
> + HI6421 = 1,
> + HI6421_V530 = 2,
> +};
> +

As programmers we usually start counting at 0. Then you can rely on
the C standard to populate the remaining values.

Something like:

enum hi6421_type {
HI6421 = 0,
HI6421_V530,
};

... is more normal.

> #endif /* __HI6421_PMIC_H */

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

2017-06-06 13:58:36

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v4 0/8] MFD: add driver for HiSilicon Hi6421v530 PMIC

On Tue, 06 Jun 2017, Arnd Bergmann wrote:

> On Tue, Jun 6, 2017 at 10:51 AM, Guodong Xu <[email protected]> wrote:
> > This patchset adds driver for HiSilicon Hi6421v530 PMIC.
> >
> > Mainline kernel already has driver support to a similar chip, Hi6421.
> > Hi6421 and Hi6421v530 are both from the same vendor, HiSilicon, but
> > they are at different revisions. They both use the same Memory-mapped
> > I/O method to communicate with Main SoC. However, they differ quite a
> > lot in their regulator designs. Eg. they have completely different LDO
> > voltage points.
> >
> > Patch 1 and 2 are hi6421-pmic cleaning up.
> > Patch 3 and 4 extends hi6421-pmic-core.c to support Hi6421v530 revision.
> > Patch 5 add hi6421v530-regulator.c driver for LDO regulators.
> > Patch 6 fixes an issue for hi6421 regulator, which is not related to v530
> > but it's found in this review.
> > Patch 7 is dts change, it depends on and can be applied to hi3660/hikey960
> > patchset [1].
> > Patch 8 enables the relevant config items.
>
> Looks good to me,

Is that an Ack?

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

2017-06-07 07:04:55

by Guodong Xu

[permalink] [raw]
Subject: Re: [PATCH v4 4/8] mfd: hi6421-pmic: add support for HiSilicon Hi6421v530

On Tue, Jun 6, 2017 at 9:54 PM, Lee Jones <[email protected]> wrote:
>
> On Tue, 06 Jun 2017, Guodong Xu wrote:
>
> > Add support for HiSilicon Hi6421v530 PMIC. Hi6421v530 communicates with
> > main SoC via memory-mapped I/O.
> >
> > Hi6421v530 and Hi6421 are PMIC chips from the same vendor, HiSilicon,
> > but at different revisions. They share the same memory-mapped I/O
> > design. They differ in integrated devices, such as regulator details,
> > LDO voltage points.
> >
> > Signed-off-by: Wang Xiaoyin <[email protected]>
> > Signed-off-by: Guodong Xu <[email protected]>
> > ---
> > drivers/mfd/hi6421-pmic-core.c | 68 ++++++++++++++++++++++++++++++-----------
> > include/linux/mfd/hi6421-pmic.h | 5 +++
> > 2 files changed, 55 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/mfd/hi6421-pmic-core.c b/drivers/mfd/hi6421-pmic-core.c
> > index b1139d4..4568a43 100644
> > --- a/drivers/mfd/hi6421-pmic-core.c
> > +++ b/drivers/mfd/hi6421-pmic-core.c
>
> [...]
>
> > static int hi6421_pmic_probe(struct platform_device *pdev)
> > {
> > struct hi6421_pmic *pmic;
> > struct resource *res;
> > + const struct of_device_id *id;
> > + const struct mfd_cell *subdevs;
> > void __iomem *base;
> > - int ret;
> > + int n_subdevs, ret;
> > +
> > + id = of_match_device(of_hi6421_pmic_match, &pdev->dev);
> > + if (!id)
> > + return -EINVAL;
> >
> > pmic = devm_kzalloc(&pdev->dev, sizeof(*pmic), GFP_KERNEL);
> > if (!pmic)
> > @@ -57,18 +80,33 @@ static int hi6421_pmic_probe(struct platform_device *pdev)
> > return PTR_ERR(pmic->regmap);
> > }
> >
> > - /* set over-current protection debounce 8ms */
> > - regmap_update_bits(pmic->regmap, HI6421_OCP_DEB_CTRL_REG,
> > + platform_set_drvdata(pdev, pmic);
> > +
> > + switch ((unsigned long)id->data) {
>
> Whilst this is not a blocker, it would be nicer to place this into a
> 'type' variable, in order to clarify what data you are using.
>

I will add that.
enum hi6421_type type;


>
> > + case HI6421:
> > + /* set over-current protection debounce 8ms */
> > + regmap_update_bits(pmic->regmap, HI6421_OCP_DEB_CTRL_REG,
> > (HI6421_OCP_DEB_SEL_MASK
> > | HI6421_OCP_EN_DEBOUNCE_MASK
> > | HI6421_OCP_AUTO_STOP_MASK),
> > (HI6421_OCP_DEB_SEL_8MS
> > | HI6421_OCP_EN_DEBOUNCE_ENABLE));
> >
> > - platform_set_drvdata(pdev, pmic);
> > + subdevs = hi6421_devs;
> > + n_subdevs = ARRAY_SIZE(hi6421_devs);
> > + break;
> > + case HI6421_V530:
> > + subdevs = hi6421v530_devs;
> > + n_subdevs = ARRAY_SIZE(hi6421v530_devs);
> > + break;
> > + default:
> > + dev_err(&pdev->dev, "Unknown device type %ld\n",
> > + (unsigned long)id->data);
> > + return -EINVAL;
> > + }
> >
> > - ret = devm_mfd_add_devices(&pdev->dev, 0, hi6421_devs,
> > - ARRAY_SIZE(hi6421_devs), NULL, 0, NULL);
> > + ret = devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE,
> > + subdevs, n_subdevs, NULL, 0, NULL);
> > if (ret) {
> > dev_err(&pdev->dev, "Failed to add child devices: %d\n", ret);
> > return ret;
> > @@ -77,16 +115,10 @@ static int hi6421_pmic_probe(struct platform_device *pdev)
> > return 0;
> > }
> >
> > -static const struct of_device_id of_hi6421_pmic_match_tbl[] = {
> > - { .compatible = "hisilicon,hi6421-pmic", },
> > - { },
> > -};
> > -MODULE_DEVICE_TABLE(of, of_hi6421_pmic_match_tbl);
> > -
> > static struct platform_driver hi6421_pmic_driver = {
> > .driver = {
> > - .name = "hi6421_pmic",
> > - .of_match_table = of_hi6421_pmic_match_tbl,
> > + .name = "hi6421_pmic",
> > + .of_match_table = of_hi6421_pmic_match,
> > },
> > .probe = hi6421_pmic_probe,
> > };
> > diff --git a/include/linux/mfd/hi6421-pmic.h b/include/linux/mfd/hi6421-pmic.h
> > index 587273e..2457438 100644
> > --- a/include/linux/mfd/hi6421-pmic.h
> > +++ b/include/linux/mfd/hi6421-pmic.h
> > @@ -38,4 +38,9 @@ struct hi6421_pmic {
> > struct regmap *regmap;
> > };
> >
> > +enum hi6421_type {
> > + HI6421 = 1,
> > + HI6421_V530 = 2,
> > +};
> > +
>
> As programmers we usually start counting at 0. Then you can rely on
> the C standard to populate the remaining values.
>
> Something like:
>
> enum hi6421_type {
> HI6421 = 0,
> HI6421_V530,
> };
>
> ... is more normal.


:) sure. thanks. I will do.

-Guodong

>
>
> > #endif /* __HI6421_PMIC_H */
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

2017-06-07 07:12:37

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v4 0/8] MFD: add driver for HiSilicon Hi6421v530 PMIC

On Tue, Jun 6, 2017 at 3:58 PM, Lee Jones <[email protected]> wrote:
> On Tue, 06 Jun 2017, Arnd Bergmann wrote:
>
>> On Tue, Jun 6, 2017 at 10:51 AM, Guodong Xu <[email protected]> wrote:
>> > This patchset adds driver for HiSilicon Hi6421v530 PMIC.
>> >
>> > Mainline kernel already has driver support to a similar chip, Hi6421.
>> > Hi6421 and Hi6421v530 are both from the same vendor, HiSilicon, but
>> > they are at different revisions. They both use the same Memory-mapped
>> > I/O method to communicate with Main SoC. However, they differ quite a
>> > lot in their regulator designs. Eg. they have completely different LDO
>> > voltage points.
>> >
>> > Patch 1 and 2 are hi6421-pmic cleaning up.
>> > Patch 3 and 4 extends hi6421-pmic-core.c to support Hi6421v530 revision.
>> > Patch 5 add hi6421v530-regulator.c driver for LDO regulators.
>> > Patch 6 fixes an issue for hi6421 regulator, which is not related to v530
>> > but it's found in this review.
>> > Patch 7 is dts change, it depends on and can be applied to hi3660/hikey960
>> > patchset [1].
>> > Patch 8 enables the relevant config items.
>>
>> Looks good to me,
>
> Is that an Ack?

Yes, sorry for not being explicit about this.

Acked-by: Arnd Bergmann <[email protected]>