All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/5] Add Support for Hi6220 PMIC Hi6553 MFD Core
@ 2016-01-19  6:31 Chen Feng
  2016-01-19  6:32 ` [PATCH v6 1/5] doc: bindings: Add document for mfd hi665x PMIC Chen Feng
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Chen Feng @ 2016-01-19  6:31 UTC (permalink / raw)
  To: lee.jones, lgirdwood, broonie, linux-kernel, puck.chen,
	yudongbin, saberlily.xia, suzhuangluan, kong.kongxinwei,
	xuyiping, z.liuxinliang, weidong2, w.f, puck.chen, shimingxing,
	oliver.fu, albert.lubing, chenxiang9, liuzixing, haojian.zhuang
  Cc: qijiwen, peter.panshilin, dan.zhao, linuxarm, dev

The patch sets add support for Hi6220 PMIC Hi655x MFD core and its
regulator driver.
Current testing and support board is Hikey which is one of 96boards.
It is an arm64 open source board. For more information about this board,
please access https://www.96boards.org.

This is hardware layout for access PMIC Hi655x from AP SoC Hi6220.
Between PMIC Hi655x and Hi6220, the physical signal channel is SSI.
We can use memory-mapped I/O to communicate.

+----------------+             +-------------+
|                |             |             |
|    Hi6220      |   SSI bus   |   Hi655x    |
|                |-------------|             |
|                |(REGMAP_MMIO)|             |
+----------------+             +-------------+

V2: Code refactoring of regulator.

V3: Drop mtcmos from this patch and use regmap-irq.

V4: Move the vset-table to driver code and donot open code for it.

V6: Use regulators_node and of_match, donot open code fot it.

Chen Feng (5):
  doc: bindings: Add document for mfd hi665x PMIC
  doc: bindings: Document for hi655x regulator driver
  mfd: hi655x: Add hi665x pmic driver
  regulator: add regulator driver of hi655x pmic
  hisilicon/dts: Add hi655x pmic dts node

 .../devicetree/bindings/mfd/hisilicon,hi655x.txt   |  28 +++
 .../regulator/hisilicon,hi655x-regulator.txt       |  28 +++
 arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts     |   5 +
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi          |  87 ++++++++
 drivers/mfd/Kconfig                                |  10 +
 drivers/mfd/Makefile                               |   1 +
 drivers/mfd/hi655x-pmic.c                          | 170 +++++++++++++++
 drivers/regulator/Kconfig                          |   8 +
 drivers/regulator/Makefile                         |   1 +
 drivers/regulator/hi655x-regulator.c               | 227 +++++++++++++++++++++
 include/linux/mfd/hi655x-pmic.h                    |  56 +++++
 11 files changed, 621 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/hisilicon,hi655x.txt
 create mode 100644 Documentation/devicetree/bindings/regulator/hisilicon,hi655x-regulator.txt
 create mode 100644 drivers/mfd/hi655x-pmic.c
 create mode 100644 drivers/regulator/hi655x-regulator.c
 create mode 100644 include/linux/mfd/hi655x-pmic.h

-- 
1.9.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v6 1/5] doc: bindings: Add document for mfd hi665x PMIC
  2016-01-19  6:31 [PATCH v6 0/5] Add Support for Hi6220 PMIC Hi6553 MFD Core Chen Feng
@ 2016-01-19  6:32 ` Chen Feng
  2016-01-19  6:32 ` [PATCH v6 2/5] doc: bindings: Document for hi655x regulator driver Chen Feng
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Chen Feng @ 2016-01-19  6:32 UTC (permalink / raw)
  To: lee.jones, lgirdwood, broonie, linux-kernel, puck.chen,
	yudongbin, saberlily.xia, suzhuangluan, kong.kongxinwei,
	xuyiping, z.liuxinliang, weidong2, w.f, puck.chen, shimingxing,
	oliver.fu, albert.lubing, chenxiang9, liuzixing, haojian.zhuang
  Cc: qijiwen, peter.panshilin, dan.zhao, linuxarm, dev

Add document for mfd driver hi655x pmic driver

Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
Signed-off-by: Fei Wang <w.f@huawei.com>
Signed-off-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
---
 .../devicetree/bindings/mfd/hisilicon,hi655x.txt   | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/hisilicon,hi655x.txt

diff --git a/Documentation/devicetree/bindings/mfd/hisilicon,hi655x.txt b/Documentation/devicetree/bindings/mfd/hisilicon,hi655x.txt
new file mode 100644
index 0000000..3180c40
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/hisilicon,hi655x.txt
@@ -0,0 +1,28 @@
+Hisilicon hi655x Power Management Integrated Circuit (PMIC)
+
+The hardware layout for access PMIC Hi655x from AP SoC Hi6220.
+Between PMIC Hi655x and Hi6220, the physical signal channel is SSI.
+We can use memory-mapped I/O to communicate.
+
++----------------+             +-------------+
+|                |             |             |
+|    Hi6220      |   SSI bus   |   Hi655x    |
+|                |-------------|             |
+|                |(REGMAP_MMIO)|             |
++----------------+             +-------------+
+
+Required properties:
+- compatible: Should be "hisilicon,hi655x-pmic"
+- reg: Base address of PMIC on hi6220 soc
+- interrupt-controller: Hi655x has internal IRQs (has own IRQ domain).
+- pmic-gpios: The gpio used by pmic irq.
+
+Example:
+	pmic: pmic@f8000000 {
+		compatible = "hisilicon,hi655x-pmic";
+		reg = <0x0 0xf8000000 0x0 0x1000>;
+		#interrupt-cells = <2>;
+		interrupt-controller;
+		pmic-gpios = <&gpio1 2 0>;
+		status = "disabled";
+	}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 2/5] doc: bindings: Document for hi655x regulator driver
  2016-01-19  6:31 [PATCH v6 0/5] Add Support for Hi6220 PMIC Hi6553 MFD Core Chen Feng
  2016-01-19  6:32 ` [PATCH v6 1/5] doc: bindings: Add document for mfd hi665x PMIC Chen Feng
@ 2016-01-19  6:32 ` Chen Feng
  2016-01-19  6:32 ` [PATCH v6 3/5] mfd: hi655x: Add hi665x pmic driver Chen Feng
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Chen Feng @ 2016-01-19  6:32 UTC (permalink / raw)
  To: lee.jones, lgirdwood, broonie, linux-kernel, puck.chen,
	yudongbin, saberlily.xia, suzhuangluan, kong.kongxinwei,
	xuyiping, z.liuxinliang, weidong2, w.f, puck.chen, shimingxing,
	oliver.fu, albert.lubing, chenxiang9, liuzixing, haojian.zhuang
  Cc: qijiwen, peter.panshilin, dan.zhao, linuxarm, dev

Add Document for hi655x pmic regulator driver

Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
Signed-off-by: Fei Wang <w.f@huawei.com>
Signed-off-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
---
 .../regulator/hisilicon,hi655x-regulator.txt       | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/hisilicon,hi655x-regulator.txt

diff --git a/Documentation/devicetree/bindings/regulator/hisilicon,hi655x-regulator.txt b/Documentation/devicetree/bindings/regulator/hisilicon,hi655x-regulator.txt
new file mode 100644
index 0000000..c94c1a3
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/hisilicon,hi655x-regulator.txt
@@ -0,0 +1,28 @@
+Hisilicon Hi655x Voltage regulators
+
+Note:
+The hi655x regulator control is managed by hi655x Power IC.
+So the node of this regulator must be child node of hi655x
+pmic node.
+
+The driver uses the regulator core framework, so please also
+take the bindings of regulator.txt for reference.
+
+The valid names for regulators are:
+
+ldo2 ldo7 ldo10 ldo13 ldo14 ldo15 ldo17 ldo19 ldo21 ldo22
+
+Example:
+        pmic: pmic@f8000000 {
+                compatible = "hisilicon,hi655x-pmic";
+		...
+		regulators {
+			ldo2: ldo2@a21 {
+				regulator-compatible = "LDO2";
+				regulator-min-microvolt = <2500000>;
+				regulator-max-microvolt = <3200000>;
+				regulator-enable-ramp-delay = <120>;
+			};
+			...
+		}
+	}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 3/5] mfd: hi655x: Add hi665x pmic driver
  2016-01-19  6:31 [PATCH v6 0/5] Add Support for Hi6220 PMIC Hi6553 MFD Core Chen Feng
  2016-01-19  6:32 ` [PATCH v6 1/5] doc: bindings: Add document for mfd hi665x PMIC Chen Feng
  2016-01-19  6:32 ` [PATCH v6 2/5] doc: bindings: Document for hi655x regulator driver Chen Feng
@ 2016-01-19  6:32 ` Chen Feng
  2016-01-19  6:32 ` [PATCH v6 4/5] regulator: add regulator driver of hi655x pmic Chen Feng
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Chen Feng @ 2016-01-19  6:32 UTC (permalink / raw)
  To: lee.jones, lgirdwood, broonie, linux-kernel, puck.chen,
	yudongbin, saberlily.xia, suzhuangluan, kong.kongxinwei,
	xuyiping, z.liuxinliang, weidong2, w.f, puck.chen, shimingxing,
	oliver.fu, albert.lubing, chenxiang9, liuzixing, haojian.zhuang
  Cc: qijiwen, peter.panshilin, dan.zhao, linuxarm, dev

Add pmic mfd driver to support hisilicon hi665x.

Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
Signed-off-by: Fei Wang <w.f@huawei.com>
Signed-off-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
---
 drivers/mfd/Kconfig             |  10 +++
 drivers/mfd/Makefile            |   1 +
 drivers/mfd/hi655x-pmic.c       | 170 ++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/hi655x-pmic.h |  56 +++++++++++++
 4 files changed, 237 insertions(+)
 create mode 100644 drivers/mfd/hi655x-pmic.c
 create mode 100644 include/linux/mfd/hi655x-pmic.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 4d92df6..0c84186 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -284,6 +284,16 @@ config MFD_HI6421_PMIC
 	  menus in order to enable them.
 	  We communicate with the Hi6421 via memory-mapped I/O.
 
+config MFD_HI655X_PMIC
+	tristate "HiSilicon Hi655X series PMU/Codec IC"
+	depends on ARCH_HISI || COMPILE_TEST
+	depends on OF
+	select MFD_CORE
+	select REGMAP_MMIO
+	select REGMAP_IRQ
+	help
+	  Select this option to enable Hisilicon hi655x series pmic driver.
+
 config HTC_EGPIO
 	bool "HTC EGPIO support"
 	depends on GPIOLIB && ARM
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index a8b76b8..6a7b0e1 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -186,6 +186,7 @@ obj-$(CONFIG_MFD_STW481X)	+= stw481x.o
 obj-$(CONFIG_MFD_IPAQ_MICRO)	+= ipaq-micro.o
 obj-$(CONFIG_MFD_MENF21BMC)	+= menf21bmc.o
 obj-$(CONFIG_MFD_HI6421_PMIC)	+= hi6421-pmic-core.o
+obj-$(CONFIG_MFD_HI655X_PMIC)   += hi655x-pmic.o
 obj-$(CONFIG_MFD_DLN2)		+= dln2.o
 obj-$(CONFIG_MFD_RT5033)	+= rt5033.o
 obj-$(CONFIG_MFD_SKY81452)	+= sky81452.o
diff --git a/drivers/mfd/hi655x-pmic.c b/drivers/mfd/hi655x-pmic.c
new file mode 100644
index 0000000..68c33a8
--- /dev/null
+++ b/drivers/mfd/hi655x-pmic.c
@@ -0,0 +1,170 @@
+/*
+ * Device driver for regulators in hi655x IC
+ *
+ * Copyright (c) 2016 Hisilicon.
+ *
+ * Chen Feng <puck.chen@hisilicon.com>
+ * Fei  Wang <w.f@huawei.com>
+ *
+ * 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/io.h>
+#include <linux/interrupt.h>
+#include <linux/init.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of_platform.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/hi655x-pmic.h>
+#include <linux/regmap.h>
+
+static const struct mfd_cell hi655x_pmic_devs[] = {
+	{ .name = "hi655x-regulator", },
+};
+
+static const struct regmap_irq hi655x_irqs[] = {
+	{ .reg_offset = 0, .mask = OTMP_D1R_INT },
+	{ .reg_offset = 0, .mask = VSYS_2P5_R_INT },
+	{ .reg_offset = 0, .mask = VSYS_UV_D3R_INT },
+	{ .reg_offset = 0, .mask = VSYS_6P0_D200UR_INT },
+	{ .reg_offset = 0, .mask = PWRON_D4SR_INT },
+	{ .reg_offset = 0, .mask = PWRON_D20F_INT },
+	{ .reg_offset = 0, .mask = PWRON_D20R_INT },
+	{ .reg_offset = 0, .mask = RESERVE_INT },
+};
+
+static const struct regmap_irq_chip hi655x_irq_chip = {
+	.name = "hi655x-pmic",
+	.irqs = hi655x_irqs,
+	.num_regs = 1,
+	.num_irqs = ARRAY_SIZE(hi655x_irqs),
+	.status_base = HI655X_IRQ_STAT_BASE,
+	.mask_base = HI655X_IRQ_MASK_BASE,
+};
+
+static unsigned int hi655x_pmic_get_version(struct hi655x_pmic *pmic)
+{
+	u32 val;
+
+	regmap_read(pmic->regmap,
+		    HI655X_BUS_ADDR(HI655X_VER_REG), &val);
+
+	return val;
+}
+
+static struct regmap_config hi655x_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = HI655X_STRIDE,
+	.val_bits = 8,
+	.max_register = HI655X_BUS_ADDR(0xFFF),
+};
+
+static void hi655x_local_irq_clear(struct regmap *map)
+{
+	int i;
+
+	regmap_write(map, HI655X_ANA_IRQM_BASE, HI655X_IRQ_CLR);
+	for (i = 0; i < HI655X_IRQ_ARRAY; i++) {
+		regmap_write(map, HI655X_IRQ_STAT_BASE + i * HI655X_STRIDE,
+			     HI655X_IRQ_CLR);
+	}
+}
+
+static int hi655x_pmic_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct hi655x_pmic *pmic;
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	void __iomem *base;
+
+	pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL);
+	pmic->dev = dev;
+
+	pmic->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!pmic->res) {
+		dev_err(dev, "platform_get_resource err\n");
+		return -ENOENT;
+	}
+	base = devm_ioremap_resource(dev, pmic->res);
+	if (!base) {
+		dev_err(dev, "cannot map register memory\n");
+		return -ENOMEM;
+	}
+	pmic->regmap = devm_regmap_init_mmio_clk(dev, NULL, base,
+						 &hi655x_regmap_config);
+
+	pmic->ver = hi655x_pmic_get_version(pmic);
+	if ((pmic->ver < PMU_VER_START) || (pmic->ver > PMU_VER_END)) {
+		dev_warn(dev, "it is wrong pmu version\n");
+		return -EINVAL;
+	}
+
+	hi655x_local_irq_clear(pmic->regmap);
+
+	pmic->gpio = of_get_named_gpio(np, "pmic-gpios", 0);
+	if (!gpio_is_valid(pmic->gpio)) {
+		dev_err(dev, "cannot get the pmic-gpios\n");
+		return -ENODEV;
+	}
+
+	ret = devm_gpio_request_one(dev, pmic->gpio, GPIOF_IN,
+				    "hi655x_pmic_irq");
+	if (ret < 0) {
+		dev_err(dev, "failed to request gpio %d  ret = %d\n",
+			pmic->gpio, ret);
+		return ret;
+	}
+
+	ret = regmap_add_irq_chip(pmic->regmap, gpio_to_irq(pmic->gpio),
+				  IRQF_TRIGGER_LOW | IRQF_NO_SUSPEND, 0,
+				  &hi655x_irq_chip, &pmic->irq_data);
+	if (ret) {
+		dev_err(dev, "add pmic irq chip error! ret %d\n", ret);
+		return ret;
+	}
+
+	/* bind pmic to device */
+	platform_set_drvdata(pdev, pmic);
+
+	ret = mfd_add_devices(dev, 0, hi655x_pmic_devs,
+			      ARRAY_SIZE(hi655x_pmic_devs), NULL, 0, NULL);
+	if (ret) {
+		dev_err(dev, "add mfd devices failed: %d\n", ret);
+		regmap_del_irq_chip(pmic->irq, pmic->irq_data);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int hi655x_pmic_remove(struct platform_device *pdev)
+{
+	mfd_remove_devices(&pdev->dev);
+
+	return 0;
+}
+
+static const struct of_device_id of_hi655x_pmic_match_tbl[] = {
+	{ .compatible = "hisilicon,hi655x-pmic", },
+	{},
+};
+
+static struct platform_driver hi655x_pmic_driver = {
+	.driver	= {
+		.name =	"hi655x-pmic",
+		.of_match_table = of_hi655x_pmic_match_tbl,
+	},
+	.probe  = hi655x_pmic_probe,
+	.remove = hi655x_pmic_remove,
+};
+module_platform_driver(hi655x_pmic_driver);
+
+MODULE_AUTHOR("Chen Feng <puck.chen@hisilicon.com>");
+MODULE_DESCRIPTION("Hisilicon hi655x pmic driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/hi655x-pmic.h b/include/linux/mfd/hi655x-pmic.h
new file mode 100644
index 0000000..e1af0a1
--- /dev/null
+++ b/include/linux/mfd/hi655x-pmic.h
@@ -0,0 +1,56 @@
+/*
+ * Device driver for regulators in hi655x IC
+ *
+ * Copyright (c) 2016 Hisilicon.
+ *
+ * Chen Feng <puck.chen@hisilicon.com>
+ * Fei  Wang <w.f@huawei.com>
+ *
+ * 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.
+ */
+
+#ifndef __HI655X_PMIC_H
+#define __HI655X_PMIC_H
+
+/* Hi655x registers are mapped to memory bus in 4 bytes stride */
+#define HI655X_STRIDE                   (4)
+#define HI655X_BUS_ADDR(x)              ((x) << 2)
+
+#define HI655X_BITS                     (8)
+
+#define HI655X_NR_IRQ                   (32)
+
+#define HI655X_IRQ_STAT_BASE            (0x003 << 2)
+#define HI655X_IRQ_MASK_BASE            (0x007 << 2)
+#define HI655X_ANA_IRQM_BASE            (0x1b5 << 2)
+#define HI655X_IRQ_ARRAY                (4)
+#define HI655X_IRQ_MASK                 (0xFF)
+#define HI655X_IRQ_CLR                  (0xFF)
+#define HI655X_VER_REG                  (0x00)
+
+#define PMU_VER_START                   (0x10)
+#define PMU_VER_END                     (0x38)
+
+#define RESERVE_INT                     (BIT(7))
+#define PWRON_D20R_INT                  (BIT(6))
+#define PWRON_D20F_INT                  (BIT(5))
+#define PWRON_D4SR_INT                  (BIT(4))
+#define VSYS_6P0_D200UR_INT             (BIT(3))
+#define VSYS_UV_D3R_INT                 (BIT(2))
+#define VSYS_2P5_R_INT                  (BIT(1))
+#define OTMP_D1R_INT                    (BIT(0))
+
+struct hi655x_pmic {
+	struct resource *res;
+	struct device *dev;
+	struct regmap *regmap;
+	struct clk *clk;
+	int irq;
+	int gpio;
+	unsigned int ver;
+	struct regmap_irq_chip_data *irq_data;
+};
+
+#endif
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 4/5] regulator: add regulator driver of hi655x pmic
  2016-01-19  6:31 [PATCH v6 0/5] Add Support for Hi6220 PMIC Hi6553 MFD Core Chen Feng
                   ` (2 preceding siblings ...)
  2016-01-19  6:32 ` [PATCH v6 3/5] mfd: hi655x: Add hi665x pmic driver Chen Feng
@ 2016-01-19  6:32 ` Chen Feng
  2016-01-19  6:32 ` [PATCH v6 5/5] hisilicon/dts: Add hi655x pmic dts node Chen Feng
  2016-01-19 17:30 ` [PATCH v6 0/5] Add Support for Hi6220 PMIC Hi6553 MFD Core Mark Brown
  5 siblings, 0 replies; 13+ messages in thread
From: Chen Feng @ 2016-01-19  6:32 UTC (permalink / raw)
  To: lee.jones, lgirdwood, broonie, linux-kernel, puck.chen,
	yudongbin, saberlily.xia, suzhuangluan, kong.kongxinwei,
	xuyiping, z.liuxinliang, weidong2, w.f, puck.chen, shimingxing,
	oliver.fu, albert.lubing, chenxiang9, liuzixing, haojian.zhuang
  Cc: qijiwen, peter.panshilin, dan.zhao, linuxarm, dev

Add regulator support for hi655x pmic

Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
Signed-off-by: Fei Wang <w.f@huawei.com>
Signed-off-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
---
 drivers/regulator/Kconfig            |   8 ++
 drivers/regulator/Makefile           |   1 +
 drivers/regulator/hi655x-regulator.c | 227 +++++++++++++++++++++++++++++++++++
 3 files changed, 236 insertions(+)
 create mode 100644 drivers/regulator/hi655x-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 8df0b0e..2e9eaa7 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -261,6 +261,14 @@ 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_HI655X
+	tristate "Hisilicon HI655X PMIC regulators support"
+	depends on ARCH_HISI || COMPILE_TEST
+	depends on MFD_HI655X_PMIC && OF
+	help
+	  This driver provides support for the voltage regulators of the
+	  Hisilicon Hi655x PMIC device.
+
 config REGULATOR_ISL9305
 	tristate "Intersil ISL9305 regulator"
 	depends on I2C
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 0f81749..8e4db96 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -34,6 +34,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_HI655X) += hi655x-regulator.o
 obj-$(CONFIG_REGULATOR_ISL6271A) += isl6271a-regulator.o
 obj-$(CONFIG_REGULATOR_ISL9305) += isl9305.o
 obj-$(CONFIG_REGULATOR_LP3971) += lp3971.o
diff --git a/drivers/regulator/hi655x-regulator.c b/drivers/regulator/hi655x-regulator.c
new file mode 100644
index 0000000..95f2048
--- /dev/null
+++ b/drivers/regulator/hi655x-regulator.c
@@ -0,0 +1,227 @@
+/*
+ * Device driver for regulators in hi655x IC
+ *
+ * Copyright (c) 2016 Hisilicon.
+ *
+ * Chen Feng <puck.chen@hisilicon.com>
+ * Fei  Wang <w.f@huawei.com>
+ *
+ * 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/bitops.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/mfd/hi655x-pmic.h>
+
+struct hi655x_regulator {
+	unsigned int disable_reg;
+	unsigned int status_reg;
+	unsigned int ctrl_regs;
+	unsigned int ctrl_mask;
+	struct regulator_desc rdesc;
+};
+
+/* LDO7 & LDO10 */
+static const unsigned int ldo7_voltages[] = {
+	1800000, 1850000, 2850000, 2900000,
+	3000000, 3100000, 3200000, 3300000,
+};
+
+static const unsigned int ldo19_voltages[] = {
+	1800000, 1850000, 1900000, 1750000,
+	2800000, 2850000, 2900000, 3000000,
+};
+
+static const unsigned int ldo22_voltages[] = {
+	 900000, 1000000, 1050000, 1100000,
+	1150000, 1175000, 1185000, 1200000,
+};
+
+enum hi655x_regulator_id {
+	HI655X_LDO0,
+	HI655X_LDO1,
+	HI655X_LDO2,
+	HI655X_LDO3,
+	HI655X_LDO4,
+	HI655X_LDO5,
+	HI655X_LDO6,
+	HI655X_LDO7,
+	HI655X_LDO8,
+	HI655X_LDO9,
+	HI655X_LDO10,
+	HI655X_LDO11,
+	HI655X_LDO12,
+	HI655X_LDO13,
+	HI655X_LDO14,
+	HI655X_LDO15,
+	HI655X_LDO16,
+	HI655X_LDO17,
+	HI655X_LDO18,
+	HI655X_LDO19,
+	HI655X_LDO20,
+	HI655X_LDO21,
+	HI655X_LDO22,
+};
+
+static int hi655x_is_enabled(struct regulator_dev *rdev)
+{
+	unsigned int value = 0;
+
+	struct hi655x_regulator *regulator = rdev_get_drvdata(rdev);
+
+	regmap_read(rdev->regmap, regulator->status_reg, &value);
+	return (value & BIT(regulator->ctrl_mask));
+}
+
+static int hi655x_disable(struct regulator_dev *rdev)
+{
+	int ret = 0;
+
+	struct hi655x_regulator *regulator = rdev_get_drvdata(rdev);
+
+	ret = regmap_write(rdev->regmap, regulator->disable_reg,
+			   BIT(regulator->ctrl_mask));
+	return ret;
+}
+
+static struct regulator_ops hi655x_regulator_ops = {
+	.enable = regulator_enable_regmap,
+	.disable = hi655x_disable,
+	.is_enabled = hi655x_is_enabled,
+	.list_voltage = regulator_list_voltage_table,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+};
+
+static struct regulator_ops hi655x_ldo_linear_ops = {
+	.enable = regulator_enable_regmap,
+	.disable = hi655x_disable,
+	.is_enabled = hi655x_is_enabled,
+	.list_voltage = regulator_list_voltage_linear,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+};
+
+#define HI655X_LDO(_ID, vreg, vmask, ereg, dreg,                 \
+		   sreg, cmask, vtable) {                        \
+	.rdesc = {                                               \
+		.name            = #_ID,                         \
+		.of_match        = of_match_ptr(#_ID),           \
+		.ops             = &hi655x_regulator_ops,        \
+		.regulators_node = of_match_ptr("regulators"),   \
+		.type            = REGULATOR_VOLTAGE,            \
+		.id              = HI655X_##_ID,                 \
+		.owner           = THIS_MODULE,                  \
+		.n_voltages      = ARRAY_SIZE(vtable),           \
+		.volt_table      = vtable,                       \
+		.vsel_reg        = HI655X_BUS_ADDR(vreg),        \
+		.vsel_mask       = vmask,                        \
+		.enable_reg      = HI655X_BUS_ADDR(ereg),        \
+		.enable_mask     = cmask,                        \
+	},                                                       \
+	.disable_reg = HI655X_BUS_ADDR(dreg),                    \
+	.status_reg = HI655X_BUS_ADDR(sreg),                     \
+	.ctrl_mask = cmask,                                      \
+}
+
+#define HI655X_LDO_LINEAR(_ID, vreg, vmask, ereg, dreg,          \
+			  sreg, cmask, minv, nvolt, vstep) {     \
+	.rdesc = {                                               \
+		.name            = #_ID,                         \
+		.of_match        = of_match_ptr(#_ID),           \
+		.ops             = &hi655x_ldo_linear_ops,       \
+		.regulators_node = of_match_ptr("regulators"),   \
+		.type            = REGULATOR_VOLTAGE,            \
+		.id              = HI655X_##_ID,                 \
+		.owner           = THIS_MODULE,                  \
+		.min_uV          = minv,                         \
+		.n_voltages      = nvolt,                        \
+		.uV_step         = vstep,                        \
+		.uV_step         = vstep,                        \
+		.vsel_reg        = HI655X_BUS_ADDR(vreg),        \
+		.vsel_mask       = vmask,                        \
+		.enable_reg      = HI655X_BUS_ADDR(ereg),        \
+		.enable_mask     = cmask,                        \
+	},                                                       \
+	.disable_reg = HI655X_BUS_ADDR(dreg),                    \
+	.status_reg = HI655X_BUS_ADDR(sreg),                     \
+	.ctrl_mask = cmask,                                      \
+}
+
+static struct hi655x_regulator regulators[] = {
+	HI655X_LDO_LINEAR(LDO2, 0x72, 0x07, 0x29, 0x2a, 0x2b, 0x01,
+			  2500000, 8, 100000),
+	HI655X_LDO(LDO7, 0x78, 0x07, 0x29, 0x2a, 0x2b, 0x06, ldo7_voltages),
+	HI655X_LDO(LDO10, 0x78, 0x07, 0x29, 0x2a, 0x2b, 0x01, ldo7_voltages),
+	HI655X_LDO_LINEAR(LDO13, 0x7e, 0x07, 0x2c, 0x2d, 0x2e, 0x04,
+			  1600000, 8, 50000),
+	HI655X_LDO_LINEAR(LDO14, 0x7f, 0x07, 0x2c, 0x2d, 0x2e, 0x05,
+			  2500000, 8, 100000),
+	HI655X_LDO_LINEAR(LDO15, 0x80, 0x07, 0x2c, 0x2d, 0x2e, 0x06,
+			  1600000, 8, 50000),
+	HI655X_LDO_LINEAR(LDO17, 0x82, 0x07, 0x2f, 0x30, 0x31, 0x00,
+			  2500000, 8, 100000),
+	HI655X_LDO(LDO19, 0x84, 0x07, 0x2f, 0x30, 0x31, 0x02, ldo19_voltages),
+	HI655X_LDO_LINEAR(LDO21, 0x86, 0x07, 0x2f, 0x30, 0x31, 0x04,
+			  1650000, 8, 50000),
+	HI655X_LDO(LDO22, 0x87, 0x07, 0x2f, 0x30, 0x31, 0x05, ldo22_voltages),
+};
+
+static int hi655x_regulator_probe(struct platform_device *pdev)
+{
+	unsigned int i;
+	struct hi655x_regulator *regulator;
+	struct hi655x_pmic *pmic;
+	struct regulator_config config = { };
+	struct regulator_dev *rdev;
+
+	pmic = dev_get_drvdata(pdev->dev.parent);
+	if (!pmic) {
+		dev_err(&pdev->dev, "no pmic in the regulator parent node\n");
+		return -ENODEV;
+	}
+
+	regulator = devm_kzalloc(&pdev->dev, sizeof(*regulator), GFP_KERNEL);
+	if (!regulator)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, regulator);
+
+	config.dev = pdev->dev.parent;
+	config.regmap = pmic->regmap;
+	config.driver_data = regulator;
+	for (i = 0; i < ARRAY_SIZE(regulators); i++) {
+		rdev = devm_regulator_register(&pdev->dev,
+					       &regulators[i].rdesc,
+					       &config);
+		if (IS_ERR(rdev)) {
+			dev_err(&pdev->dev, "failed to register regulator %s\n",
+				regulator->rdesc.name);
+			return PTR_ERR(rdev);
+		}
+	}
+	return 0;
+}
+
+static struct platform_driver hi655x_regulator_driver = {
+	.driver = {
+		.name	= "hi655x-regulator",
+	},
+	.probe	= hi655x_regulator_probe,
+};
+module_platform_driver(hi655x_regulator_driver);
+
+MODULE_AUTHOR("Chen Feng <puck.chen@hisilicon.com>");
+MODULE_DESCRIPTION("Hisilicon hi655x regulator driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 5/5] hisilicon/dts: Add hi655x pmic dts node
  2016-01-19  6:31 [PATCH v6 0/5] Add Support for Hi6220 PMIC Hi6553 MFD Core Chen Feng
                   ` (3 preceding siblings ...)
  2016-01-19  6:32 ` [PATCH v6 4/5] regulator: add regulator driver of hi655x pmic Chen Feng
@ 2016-01-19  6:32 ` Chen Feng
  2016-01-20 13:08   ` Mark Brown
  2016-01-19 17:30 ` [PATCH v6 0/5] Add Support for Hi6220 PMIC Hi6553 MFD Core Mark Brown
  5 siblings, 1 reply; 13+ messages in thread
From: Chen Feng @ 2016-01-19  6:32 UTC (permalink / raw)
  To: lee.jones, lgirdwood, broonie, linux-kernel, puck.chen,
	yudongbin, saberlily.xia, suzhuangluan, kong.kongxinwei,
	xuyiping, z.liuxinliang, weidong2, w.f, puck.chen, shimingxing,
	oliver.fu, albert.lubing, chenxiang9, liuzixing, haojian.zhuang
  Cc: qijiwen, peter.panshilin, dan.zhao, linuxarm, dev

Add the mfd hi655x dts node and regulator support

Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
Signed-off-by: Fei Wang <w.f@huawei.com>
Signed-off-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
---
 arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts |  5 ++
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi      | 87 ++++++++++++++++++++++++++
 2 files changed, 92 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
index 8d43a0f..f714ac7 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
@@ -31,4 +31,9 @@
 		device_type = "memory";
 		reg = <0x0 0x0 0x0 0x40000000>;
 	};
+
+};
+
+&pmic {
+	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 82d2488..6de9881 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -208,5 +208,92 @@
 			clock-names = "uartclk", "apb_pclk";
 			status = "disabled";
 		};
+
+	};
+
+	pmic: pmic@f8000000 {
+		compatible = "hisilicon,hi655x-pmic";
+		reg = <0x0 0xf8000000 0x0 0x1000>;
+		#interrupt-cells = <2>;
+		interrupt-controller;
+		pmic-gpios = <&gpio1 2 0>;
+		status = "disabled";
+
+		regulators {
+			ldo2: ldo2@a21 {
+				regulator-compatible = "LDO2";
+				regulator-min-microvolt = <2500000>;
+				regulator-max-microvolt = <3200000>;
+				regulator-enable-ramp-delay = <120>;
+			};
+
+			ldo7: ldo7@a26 {
+				regulator-compatible = "LDO7";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-enable-ramp-delay = <120>;
+			};
+
+			ldo10: ldo10@a29 {
+				regulator-compatible = "LDO10";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3000000>;
+				regulator-enable-ramp-delay = <360>;
+			};
+
+			ldo13: ldo13@a32 {
+				regulator-compatible = "LDO13";
+				regulator-min-microvolt = <1600000>;
+				regulator-max-microvolt = <1950000>;
+				regulator-enable-ramp-delay = <120>;
+			};
+
+			ldo14: ldo14@a33 {
+				regulator-compatible = "LDO14";
+				regulator-min-microvolt = <2500000>;
+				regulator-max-microvolt = <3200000>;
+				regulator-enable-ramp-delay = <120>;
+			};
+
+			ldo15: ldo15@a34 {
+				regulator-compatible = "LDO15";
+				regulator-min-microvolt = <1600000>;
+				regulator-max-microvolt = <1950000>;
+				regulator-boot-on;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <120>;
+			};
+
+			ldo17: ldo17@a36 {
+				regulator-compatible = "LDO17";
+				regulator-min-microvolt = <2500000>;
+				regulator-max-microvolt = <3200000>;
+				regulator-enable-ramp-delay = <120>;
+			};
+
+			ldo19: ldo19@a38 {
+				regulator-compatible = "LDO19";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3000000>;
+				regulator-enable-ramp-delay = <360>;
+			};
+
+			ldo21: ldo21@a40 {
+				regulator-compatible = "LDO21";
+				regulator-min-microvolt = <1650000>;
+				regulator-max-microvolt = <2000000>;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <120>;
+			};
+
+			ldo22: ldo22@a41 {
+				regulator-compatible = "LDO22";
+				regulator-min-microvolt = <900000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-boot-on;
+				regulator-always-on;
+				regulator-enable-ramp-delay = <120>;
+			};
+		};
 	};
 };
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 0/5] Add Support for Hi6220 PMIC Hi6553 MFD Core
  2016-01-19  6:31 [PATCH v6 0/5] Add Support for Hi6220 PMIC Hi6553 MFD Core Chen Feng
                   ` (4 preceding siblings ...)
  2016-01-19  6:32 ` [PATCH v6 5/5] hisilicon/dts: Add hi655x pmic dts node Chen Feng
@ 2016-01-19 17:30 ` Mark Brown
  2016-01-20  3:51   ` chenfeng
  5 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2016-01-19 17:30 UTC (permalink / raw)
  To: Chen Feng
  Cc: lee.jones, lgirdwood, linux-kernel, yudongbin, saberlily.xia,
	suzhuangluan, kong.kongxinwei, xuyiping, z.liuxinliang, weidong2,
	w.f, puck.chen, shimingxing, oliver.fu, albert.lubing,
	chenxiang9, liuzixing, haojian.zhuang, qijiwen, peter.panshilin,
	dan.zhao, linuxarm, dev

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

On Tue, Jan 19, 2016 at 02:31:59PM +0800, Chen Feng wrote:

> Chen Feng (5):
>   doc: bindings: Add document for mfd hi665x PMIC
>   doc: bindings: Document for hi655x regulator driver
>   mfd: hi655x: Add hi665x pmic driver
>   regulator: add regulator driver of hi655x pmic
>   hisilicon/dts: Add hi655x pmic dts node

I've asked you several times to use subject lines reflecting the style
for the subsystem for the DT bindings (and have just noticed that the
same thing applies to the DTS update).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 0/5] Add Support for Hi6220 PMIC Hi6553 MFD Core
  2016-01-19 17:30 ` [PATCH v6 0/5] Add Support for Hi6220 PMIC Hi6553 MFD Core Mark Brown
@ 2016-01-20  3:51   ` chenfeng
  2016-01-20 11:46     ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: chenfeng @ 2016-01-20  3:51 UTC (permalink / raw)
  To: Mark Brown
  Cc: lee.jones, lgirdwood, linux-kernel, yudongbin, saberlily.xia,
	suzhuangluan, kong.kongxinwei, xuyiping, z.liuxinliang, weidong2,
	w.f, puck.chen, shimingxing, oliver.fu, albert.lubing,
	chenxiang9, liuzixing, haojian.zhuang, qijiwen, peter.panshilin,
	dan.zhao, linuxarm, dev



On 2016/1/20 1:30, Mark Brown wrote:
> On Tue, Jan 19, 2016 at 02:31:59PM +0800, Chen Feng wrote:
> 
>> Chen Feng (5):
>>   doc: bindings: Add document for mfd hi665x PMIC
>>   doc: bindings: Document for hi655x regulator driver
>>   mfd: hi655x: Add hi665x pmic driver
>>   regulator: add regulator driver of hi655x pmic
>>   hisilicon/dts: Add hi655x pmic dts node
> 
> I've asked you several times to use subject lines reflecting the style
> for the subsystem for the DT bindings (and have just noticed that the
> same thing applies to the DTS update).
> 
Mark,

Really sorry for this.
Since I have not caught what you mean before this time.

I changed the style problem from doc:bindings to doc: bindings.
A blank space added. But this was not accurately what you mean.

I will change it like this:
bindings: mfd: hi655x: Add document for mfd hi665x PMIC
bindings: regulator: hi655x: Document for hi655x regulator driver
regulator: hi655x: add regulator driver of hi655x pmic
arm64: dts: hisilicon: Add hi655x pmic dts node

Will this be okay?

Have you also reviewed the regulator driver code? I changed it to
of_match and regulators_node without opening code.

Thanks again for your advice.

Feng

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 0/5] Add Support for Hi6220 PMIC Hi6553 MFD Core
  2016-01-20  3:51   ` chenfeng
@ 2016-01-20 11:46     ` Mark Brown
  2016-01-21 10:14       ` chenfeng
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2016-01-20 11:46 UTC (permalink / raw)
  To: chenfeng
  Cc: lee.jones, lgirdwood, linux-kernel, yudongbin, saberlily.xia,
	suzhuangluan, kong.kongxinwei, xuyiping, z.liuxinliang, weidong2,
	w.f, puck.chen, shimingxing, oliver.fu, albert.lubing,
	chenxiang9, liuzixing, haojian.zhuang, qijiwen, peter.panshilin,
	dan.zhao, linuxarm, dev

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

On Wed, Jan 20, 2016 at 11:51:27AM +0800, chenfeng wrote:

> I will change it like this:
> bindings: regulator: hi655x: Document for hi655x regulator driver

As we discussed previously I'd expected something more like:

     regulator: hi655x: Document for hi655x regulator driver

> arm64: dts: hisilicon: Add hi655x pmic dts node

That's what I'd expect.  

> Have you also reviewed the regulator driver code? I changed it to
> of_match and regulators_node without opening code.

Not yet.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 5/5] hisilicon/dts: Add hi655x pmic dts node
  2016-01-19  6:32 ` [PATCH v6 5/5] hisilicon/dts: Add hi655x pmic dts node Chen Feng
@ 2016-01-20 13:08   ` Mark Brown
  2016-01-21 10:32     ` chenfeng
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2016-01-20 13:08 UTC (permalink / raw)
  To: Chen Feng
  Cc: lee.jones, lgirdwood, linux-kernel, yudongbin, saberlily.xia,
	suzhuangluan, kong.kongxinwei, xuyiping, z.liuxinliang, weidong2,
	w.f, puck.chen, shimingxing, oliver.fu, albert.lubing,
	chenxiang9, liuzixing, haojian.zhuang, qijiwen, peter.panshilin,
	dan.zhao, linuxarm, dev

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

On Tue, Jan 19, 2016 at 02:32:04PM +0800, Chen Feng wrote:

> index 82d2488..6de9881 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi

> +		regulators {
> +			ldo2: ldo2@a21 {
> +				regulator-compatible = "LDO2";

Why are you using the legacy regulator-compatible property?  No new
bindings should use this.

> +				regulator-min-microvolt = <2500000>;
> +				regulator-max-microvolt = <3200000>;

This is broken as it misunderstands the purpose of specifying
constraints.  The constraints are there to say what the safe and
supported configuration is on a given board, it is not possible to
provide this information safely in a general include that is used by all
systems using the PMIC.  Specifying the maximum voltage range for the
regulators is almost guaranteed to result in at least some
configurations being enabled which will not work, in the worst case this
may include configurations which could physically damage the system.

In general it is very unusual to include the regulators in a .dtsi since
essentially all the configuration for them should be board specific.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 0/5] Add Support for Hi6220 PMIC Hi6553 MFD Core
  2016-01-20 11:46     ` Mark Brown
@ 2016-01-21 10:14       ` chenfeng
  0 siblings, 0 replies; 13+ messages in thread
From: chenfeng @ 2016-01-21 10:14 UTC (permalink / raw)
  To: Mark Brown
  Cc: lee.jones, lgirdwood, linux-kernel, yudongbin, saberlily.xia,
	suzhuangluan, kong.kongxinwei, xuyiping, z.liuxinliang, weidong2,
	w.f, puck.chen, shimingxing, oliver.fu, albert.lubing,
	chenxiang9, liuzixing, haojian.zhuang, qijiwen, peter.panshilin,
	dan.zhao, linuxarm, dev



On 2016/1/20 19:46, Mark Brown wrote:
> On Wed, Jan 20, 2016 at 11:51:27AM +0800, chenfeng wrote:
> 
>> I will change it like this:
>> bindings: regulator: hi655x: Document for hi655x regulator driver
> 
> As we discussed previously I'd expected something more like:
> 
>      regulator: hi655x: Document for hi655x regulator driver
> 
ok
>> arm64: dts: hisilicon: Add hi655x pmic dts node
> 
> That's what I'd expect.  
> 
>> Have you also reviewed the regulator driver code? I changed it to
>> of_match and regulators_node without opening code.
> 
> Not yet.
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 5/5] hisilicon/dts: Add hi655x pmic dts node
  2016-01-20 13:08   ` Mark Brown
@ 2016-01-21 10:32     ` chenfeng
  2016-01-21 22:11       ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: chenfeng @ 2016-01-21 10:32 UTC (permalink / raw)
  To: Mark Brown
  Cc: lee.jones, lgirdwood, linux-kernel, yudongbin, saberlily.xia,
	suzhuangluan, kong.kongxinwei, xuyiping, z.liuxinliang, weidong2,
	w.f, puck.chen, shimingxing, oliver.fu, albert.lubing,
	chenxiang9, liuzixing, haojian.zhuang, qijiwen, peter.panshilin,
	dan.zhao, linuxarm, dev


Mark,
On 2016/1/20 21:08, Mark Brown wrote:
> On Tue, Jan 19, 2016 at 02:32:04PM +0800, Chen Feng wrote:
> 
>> index 82d2488..6de9881 100644
>> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
>> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> 
>> +		regulators {
>> +			ldo2: ldo2@a21 {
>> +				regulator-compatible = "LDO2";
> 

I will change it like this:
                regulators {
                        ldo2: LDO2@a21 {
                                regulator-name = "ldo2";
                                regulator-min-microvolt = <2500000>;
                                regulator-max-microvolt = <3200000>;
                                regulator-valid-modes-mask = <0x02>;
                                regulator-enable-ramp-delay = <120>;
                        };

> Why are you using the legacy regulator-compatible property?  No new
> bindings should use this.
> 
>> +				regulator-min-microvolt = <2500000>;
>> +				regulator-max-microvolt = <3200000>;
> 
> This is broken as it misunderstands the purpose of specifying
> constraints.  The constraints are there to say what the safe and
> supported configuration is on a given board, it is not possible to
> provide this information safely in a general include that is used by all
> systems using the PMIC.  Specifying the maximum voltage range for the
> regulators is almost guaranteed to result in at least some
> configurations being enabled which will not work, in the worst case this
> may include configurations which could physically damage the system.
> 
> In general it is very unusual to include the regulators in a .dtsi since
> essentially all the configuration for them should be board specific.
> 
Do you mean that I should move this into dts and enable it by default?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 5/5] hisilicon/dts: Add hi655x pmic dts node
  2016-01-21 10:32     ` chenfeng
@ 2016-01-21 22:11       ` Mark Brown
  0 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2016-01-21 22:11 UTC (permalink / raw)
  To: chenfeng
  Cc: lee.jones, lgirdwood, linux-kernel, yudongbin, saberlily.xia,
	suzhuangluan, kong.kongxinwei, xuyiping, z.liuxinliang, weidong2,
	w.f, puck.chen, shimingxing, oliver.fu, albert.lubing,
	chenxiang9, liuzixing, haojian.zhuang, qijiwen, peter.panshilin,
	dan.zhao, linuxarm, dev

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

On Thu, Jan 21, 2016 at 06:32:32PM +0800, chenfeng wrote:
> On 2016/1/20 21:08, Mark Brown wrote:
> > On Tue, Jan 19, 2016 at 02:32:04PM +0800, Chen Feng wrote:

> I will change it like this:
>                 regulators {
>                         ldo2: LDO2@a21 {
>                                 regulator-name = "ldo2";
>                                 regulator-min-microvolt = <2500000>;
>                                 regulator-max-microvolt = <3200000>;
>                                 regulator-valid-modes-mask = <0x02>;
>                                 regulator-enable-ramp-delay = <120>;
>                         };

No, do not include the voltage constraints, see the second half of my
reply.

> >> +				regulator-min-microvolt = <2500000>;
> >> +				regulator-max-microvolt = <3200000>;

> > This is broken as it misunderstands the purpose of specifying
> > constraints.  The constraints are there to say what the safe and
> > supported configuration is on a given board, it is not possible to
> > provide this information safely in a general include that is used by all
> > systems using the PMIC.  Specifying the maximum voltage range for the
> > regulators is almost guaranteed to result in at least some
> > configurations being enabled which will not work, in the worst case this
> > may include configurations which could physically damage the system.

> > In general it is very unusual to include the regulators in a .dtsi since
> > essentially all the configuration for them should be board specific.

> Do you mean that I should move this into dts and enable it by default?

It would be more normal to put everything to do with the regulators into
the board DTS.  It is unlikely that a straight move would be the right
thing, you would need to understand what all the voltage ranges on the
board are and set them appropriately and should normally also be naming
the regulators as per the schematic so users can tie the DT and the
schematic together.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-01-21 22:11 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-19  6:31 [PATCH v6 0/5] Add Support for Hi6220 PMIC Hi6553 MFD Core Chen Feng
2016-01-19  6:32 ` [PATCH v6 1/5] doc: bindings: Add document for mfd hi665x PMIC Chen Feng
2016-01-19  6:32 ` [PATCH v6 2/5] doc: bindings: Document for hi655x regulator driver Chen Feng
2016-01-19  6:32 ` [PATCH v6 3/5] mfd: hi655x: Add hi665x pmic driver Chen Feng
2016-01-19  6:32 ` [PATCH v6 4/5] regulator: add regulator driver of hi655x pmic Chen Feng
2016-01-19  6:32 ` [PATCH v6 5/5] hisilicon/dts: Add hi655x pmic dts node Chen Feng
2016-01-20 13:08   ` Mark Brown
2016-01-21 10:32     ` chenfeng
2016-01-21 22:11       ` Mark Brown
2016-01-19 17:30 ` [PATCH v6 0/5] Add Support for Hi6220 PMIC Hi6553 MFD Core Mark Brown
2016-01-20  3:51   ` chenfeng
2016-01-20 11:46     ` Mark Brown
2016-01-21 10:14       ` chenfeng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.