[PATCH 0/7] Add Support for Hi6220 PMIC Hi6553 MFD Core

[PATCH 0/7] Add Support for Hi6220 PMIC Hi6553 MFD Core

[Chen Feng]

  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)|             |
+----------------+             +-------------+

The patch sets are based on 4.3-rc7.

Chen Feng (7):
  doc:bindings:Add document for mfd hi665x PMIC
  doc:bindings:Document for mtcmos regulator on hi6220 SoC
  doc:bindings:Document for hi655x pmic driver
  mfd: hi655x: Add hi665x pmic driver
  regulator: add driver for mtcmos voltage regulator on hi6220 SoC
  regulator: hisilicon: Add hi655x pmic voltage regulator driver
  arm64: dts: Add mtcmos and pmic node for hi6220 HiKey board

 .../devicetree/bindings/mfd/hisilicon,hi655x.txt   |  18 ++
 .../bindings/regulator/hisilicon,hi6220-mtcmos.txt |  32 +++
 .../regulator/hisilicon,hi655x-regulator.txt       |  50 ++++
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi          | 210 ++++++++++++++
 drivers/mfd/Kconfig                                |   9 +
 drivers/mfd/Makefile                               |   1 +
 drivers/mfd/hi655x-pmic.c                          | 315 +++++++++++++++++++++
 drivers/regulator/Kconfig                          |  13 +
 drivers/regulator/Makefile                         |   2 +
 drivers/regulator/hi6220-mtcmos.c                  | 245 ++++++++++++++++
 drivers/regulator/hi655x-regulator.c               | 246 ++++++++++++++++
 include/linux/mfd/hi655x-pmic.h                    |  50 ++++
 include/linux/regulator/hi655x-regulator.h         |  63 +++++
 13 files changed, 1254 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/hisilicon,hi655x.txt
 create mode 100644 Documentation/devicetree/bindings/regulator/hisilicon,hi6220-mtcmos.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/hi6220-mtcmos.c
 create mode 100644 drivers/regulator/hi655x-regulator.c
 create mode 100644 include/linux/mfd/hi655x-pmic.h
 create mode 100644 include/linux/regulator/hi655x-regulator.h

-- 
1.9.1

[PATCH 1/7] doc:bindings:Add document for mfd hi665x PMIC

[Chen Feng]

Add document for mfd driver hi655x pmic driver

Signed-off-by: Chen Feng <puck.chen-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
Signed-off-by: Fei Wang <w.f-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 .../devicetree/bindings/mfd/hisilicon,hi655x.txt       | 18 ++++++++++++++++++
 1 file changed, 18 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..afa2fa5
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/hisilicon,hi655x.txt
@@ -0,0 +1,18 @@
+Hisilicon hi655x Power Management Integrated Circuit (PMIC)
+
+Required properties:
+- compatible: Should be "hisilicon,hi655x-pmic-driver"
+- reg: Base address of PMIC on hi6220 soc
+- #interrupt-cells: Should be 2, two cells are needed for irq.
+- interrupt-controller: hi655x has internal IRQs (has own IRQ domain).
+- pmu_irq_gpio: should be &gpio_pmu_irq_n, is the IRQ gpio of hi655x.
+
+Example:
+	pmic: pmic@f8000000 {
+		compatible = "hisilicon,hi655x-pmic-driver";
+		reg = <0x0 0xf8000000 0x0 0x1000>;
+		#interrupt-cells = <2>;
+		interrupt-controller;
+		pmic_gpios = <&gpio_pmu_irq_n>;
+		status = "okay";
+	}
-- 
1.9.1

Re: [PATCH 1/7] doc:bindings:Add document for mfd hi665x PMIC

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 856 bytes --]

On Thu, Nov 05, 2015 at 09:34:42PM +0800, Chen Feng wrote:

Please use subject lines matching the style for the subsystem.  This
makes it easier for people to identify relevant patches.

> +- #interrupt-cells: Should be 2, two cells are needed for irq.
> +- interrupt-controller: hi655x has internal IRQs (has own IRQ domain).
> +- pmu_irq_gpio: should be &gpio_pmu_irq_n, is the IRQ gpio of hi655x.

I'm not entirely sure what this is but it sounds worrying - why can you
not just use a normal interrupt specifier?  It also doesn't correspond
to the example:

> +Example:
> +	pmic: pmic@f8000000 {
> +		compatible = "hisilicon,hi655x-pmic-driver";
> +		reg = <0x0 0xf8000000 0x0 0x1000>;
> +		#interrupt-cells = <2>;
> +		interrupt-controller;
> +		pmic_gpios = <&gpio_pmu_irq_n>;
> +		status = "okay";
> +	}
> -- 
> 1.9.1
> 
> 

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 2/7] doc:bindings:Document for mtcmos regulator on hi6220 SoC

[Chen Feng]

Add Document for mtcmos driver on hi6220 SoC

Signed-off-by: Chen Feng <puck.chen-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
Signed-off-by: Fei Wang <w.f-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 .../bindings/regulator/hisilicon,hi6220-mtcmos.txt | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/hisilicon,hi6220-mtcmos.txt

diff --git a/Documentation/devicetree/bindings/regulator/hisilicon,hi6220-mtcmos.txt b/Documentation/devicetree/bindings/regulator/hisilicon,hi6220-mtcmos.txt
new file mode 100644
index 0000000..bb06e1b
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/hisilicon,hi6220-mtcmos.txt
@@ -0,0 +1,32 @@
+Hi6220 mtcmos Voltage regulators
+
+Required parent device properties:
+- compatible: Must be "hisilicon,hi6220-mtcmos-driver"
+- hisilicon,mtcmos-steady-us: The time to wait for power steady
+- hisilicon,mtcmos-sc-on-base: address of mtcmos on hi6220 SoC
+
+Required child device properties:
+- regulator-name: The name of mtcmos
+- hisilicon,ctrl-regs: Offset of ctrl-regs
+- hisilicon,ctrl-data: The bit to ctrl the regulator
+
+Example:
+        mtcmos {
+                compatible = "hisilicon,hi6220-mtcmos-driver";
+                hisilicon,mtcmos-steady-us = <10>;
+                hisilicon,mtcmos-sc-on-base  = <0xf7800000>;
+                hisilicon,mtcmos-acpu-on-base = <0xf65a0000>;
+
+                mtcmos1: regulator@a1{
+                        regulator-name = "G3D_PD_VDD";
+                        regulator-compatible = "mtcmos1";
+                        hisilicon,ctrl-regs = <0x830 0x834 0x83c>;
+                        hisilicon,ctrl-data = <1 0x1>;
+                };
+                mtcmos2: regulator@a2{
+                        regulator-name = "SOC_MED";
+                        regulator-compatible = "mtcmos2";
+                        hisilicon,ctrl-regs = <0x830 0x834 0x83c>;
+                        hisilicon,ctrl-data = <2 0x1>;
+                };
+        };
-- 
1.9.1

Re: [PATCH 2/7] doc:bindings:Document for mtcmos regulator on hi6220 SoC

[Mark Brown]

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

On Thu, Nov 05, 2015 at 09:34:43PM +0800, Chen Feng wrote:

> +- hisilicon,mtcmos-steady-us: The time to wait for power steady
> +- hisilicon,mtcmos-sc-on-base: address of mtcmos on hi6220 SoC
> +
> +Required child device properties:
> +- regulator-name: The name of mtcmos
> +- hisilicon,ctrl-regs: Offset of ctrl-regs
> +- hisilicon,ctrl-data: The bit to ctrl the regulator

This doesn't look like a regulator binding at all...  for one thing
there's no reference to the generic regulator bindings, and having a
mandatory regulator-name seems like there's a problem somewhere.

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

Re: [PATCH 2/7] doc:bindings:Document for mtcmos regulator on hi6220 SoC

[Rob Herring]

On Thu, Nov 05, 2015 at 09:34:43PM +0800, Chen Feng wrote:
> Add Document for mtcmos driver on hi6220 SoC
> 
> Signed-off-by: Chen Feng <puck.chen-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> Signed-off-by: Fei Wang <w.f-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  .../bindings/regulator/hisilicon,hi6220-mtcmos.txt | 32 ++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/hisilicon,hi6220-mtcmos.txt
> 
> diff --git a/Documentation/devicetree/bindings/regulator/hisilicon,hi6220-mtcmos.txt b/Documentation/devicetree/bindings/regulator/hisilicon,hi6220-mtcmos.txt
> new file mode 100644
> index 0000000..bb06e1b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/hisilicon,hi6220-mtcmos.txt
> @@ -0,0 +1,32 @@
> +Hi6220 mtcmos Voltage regulators
> +
> +Required parent device properties:
> +- compatible: Must be "hisilicon,hi6220-mtcmos-driver"

What does "driver" refer to?

> +- hisilicon,mtcmos-steady-us: The time to wait for power steady

Wouldn't regulator-enable-ramp-delay be equivalent?


> +- hisilicon,mtcmos-sc-on-base: address of mtcmos on hi6220 SoC
> +
> +Required child device properties:
> +- regulator-name: The name of mtcmos
> +- hisilicon,ctrl-regs: Offset of ctrl-regs
> +- hisilicon,ctrl-data: The bit to ctrl the regulator
> +
> +Example:
> +        mtcmos {
> +                compatible = "hisilicon,hi6220-mtcmos-driver";
> +                hisilicon,mtcmos-steady-us = <10>;
> +                hisilicon,mtcmos-sc-on-base  = <0xf7800000>;
> +                hisilicon,mtcmos-acpu-on-base = <0xf65a0000>;
> +
> +                mtcmos1: regulator@a1{
> +                        regulator-name = "G3D_PD_VDD";
> +                        regulator-compatible = "mtcmos1";

regulator-compatible is deprecated. Please drop.

> +                        hisilicon,ctrl-regs = <0x830 0x834 0x83c>;
> +                        hisilicon,ctrl-data = <1 0x1>;
> +                };
> +                mtcmos2: regulator@a2{
> +                        regulator-name = "SOC_MED";
> +                        regulator-compatible = "mtcmos2";
> +                        hisilicon,ctrl-regs = <0x830 0x834 0x83c>;
> +                        hisilicon,ctrl-data = <2 0x1>;
> +                };
> +        };
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/7] doc:bindings:Document for hi655x pmic driver

[Chen Feng]

Add Document for hi655x pmic driver

Signed-off-by: Chen Feng <puck.chen-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
Signed-off-by: Fei Wang <w.f-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 .../regulator/hisilicon,hi655x-regulator.txt       | 50 ++++++++++++++++++++++
 1 file changed, 50 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..66c6a73
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/hisilicon,hi655x-regulator.txt
@@ -0,0 +1,50 @@
+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.
+
+Required properties:
+- compatible: Must be "hisilicon,hi655x-regulator-pmic";
+- regulator-name: Regulator name in SoC.
+- regulator-min-microvolt: Smallest voltage support.
+- regulator-max-microvolt: Largest voltages support.
+- regulator-initial-mode: Init mode of this regulator.
+- regulator-valid-modes-mask: Valid support mode mask.
+- regulator-off-on-delay: The time wait for power steady
+- regulator-ctrl-regs: Registers offset of control register.
+  In turn with enable disable and status register offset.
+- regulator-ctrl-mask: The control mask of the register.
+- regulator-vset-regs: Voltage set register offset.
+- regulator-vset-mask: voltage set control mask.
+- regulator-n-vol: The num of support voltages.
+- regulator-vset-table: The table of support voltages.
+
+Example:
+        pmic: pmic@f8000000 {
+                compatible = "hisilicon,hi655x-pmic-driver";
+		...
+                ldo2: regulator@a21 {
+                        compatible = "hisilicon,hi655x-regulator-pmic";
+                        regulator-name = "ldo2";
+                        regulator-min-microvolt = <2500000>;
+                        regulator-max-microvolt = <3200000>;
+                        regulator-valid-modes-mask = <0x02>;
+                        regulator-initial-mode = <0x02>;
+                        regulator-off-on-delay = <120>;
+                        regulator-ctrl-regs = <0x029 0x02a 0x02b>;
+                        regulator-ctrl-mask = <0x1>;
+                        regulator-vset-regs = <0x072>;
+                        regulator-vset-mask = <0x3>;
+                        regulator-n-vol = <8>;
+                        regulator-vset-table  = <2500000>,<2600000>,
+                                                <2700000>,<2800000>,
+                                                <2900000>,<3000000>,
+                                                <3100000>,<3200000>;
+                };
+		...
+	}
-- 
1.9.1

Re: [PATCH 3/7] doc:bindings:Document for hi655x pmic driver

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 2795 bytes --]

On Thu, Nov 05, 2015 at 09:34:44PM +0800, Chen Feng wrote:

> +Required properties:
> +- compatible: Must be "hisilicon,hi655x-regulator-pmic";

If this is a subfunction of a MFD it shouldn't have a compatible string.
If it is instead a standalone device it should just have a name in the
form "vendor,chip" without any random suffixes.

> +- regulator-name: Regulator name in SoC.
> +- regulator-min-microvolt: Smallest voltage support.
> +- regulator-max-microvolt: Largest voltages support.

These should *never* be mandatory properties and the generic regulator
bindings should be reference rather than copied into the binding for a
specific device, that way the standard definitions for things are always
used and people know about any other properties that are available as
standard.

> +- regulator-off-on-delay: The time wait for power steady
> +- regulator-ctrl-regs: Registers offset of control register.
> +  In turn with enable disable and status register offset.
> +- regulator-ctrl-mask: The control mask of the register.
> +- regulator-vset-regs: Voltage set register offset.
> +- regulator-vset-mask: voltage set control mask.
> +- regulator-n-vol: The num of support voltages.
> +- regulator-vset-table: The table of support voltages.

Why is this in the binding?  This is a binding for a specific device,
there is no point in putting all these data tables in the DT - it just
bloats the DT and makes it harder for us to enhance our support for this
device in the future.  Just 

> +
> +Example:
> +        pmic: pmic@f8000000 {
> +                compatible = "hisilicon,hi655x-pmic-driver";
> +		...
> +                ldo2: regulator@a21 {
> +                        compatible = "hisilicon,hi655x-regulator-pmic";
> +                        regulator-name = "ldo2";
> +                        regulator-min-microvolt = <2500000>;
> +                        regulator-max-microvolt = <3200000>;
> +                        regulator-valid-modes-mask = <0x02>;
> +                        regulator-initial-mode = <0x02>;
> +                        regulator-off-on-delay = <120>;
> +                        regulator-ctrl-regs = <0x029 0x02a 0x02b>;
> +                        regulator-ctrl-mask = <0x1>;
> +                        regulator-vset-regs = <0x072>;
> +                        regulator-vset-mask = <0x3>;
> +                        regulator-n-vol = <8>;
> +                        regulator-vset-table  = <2500000>,<2600000>,
> +                                                <2700000>,<2800000>,
> +                                                <2900000>,<3000000>,
> +                                                <3100000>,<3200000>;
> +                };
> +		...
> +	}
> -- 
> 1.9.1
> 
> 

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 4/7] mfd: hi655x: Add hi665x pmic driver

[Chen Feng]

Add pmic driver to support hisilicon hi665x pmic.

Signed-off-by: Chen Feng <puck.chen-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
Signed-off-by: Fei Wang <w.f-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/mfd/Kconfig             |   9 ++
 drivers/mfd/Makefile            |   1 +
 drivers/mfd/hi655x-pmic.c       | 315 ++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/hi655x-pmic.h |  50 +++++++
 4 files changed, 375 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 99d6367..c805071 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -273,6 +273,15 @@ config MFD_HI6421_PMIC
 	  menus in order to enable them.
 	  We communicate with the Hi6421 via memory-mapped I/O.
 
+config MFD_HI655X_PMIC
+        bool "HiSilicon Hi655X series PMU/Codec IC"
+        depends on ARCH_HISI
+        depends on OF
+        select MFD_CORE
+        select REGMAP_MMIO
+        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 a59e3fc..11ec427 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -185,6 +185,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..942f96e
--- /dev/null
+++ b/drivers/mfd/hi655x-pmic.c
@@ -0,0 +1,315 @@
+/*
+ * Device driver for PMIC DRIVER in HI655X IC
+ *
+ * Copyright (c) 2015 Hisilicon Co. Ltd
+ *
+ * Fei Wang  <w.f-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
+ * Chen Feng <puck.chen-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/slab.h>
+#include <linux/ioport.h>
+#include <linux/interrupt.h>
+#include <linux/init.h>
+#include <linux/gpio.h>
+#include <linux/types.h>
+#include <linux/mutex.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/hardirq.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/of_platform.h>
+#include <linux/irqdomain.h>
+#include <linux/mfd/hi655x-pmic.h>
+#include <linux/regmap.h>
+
+static const struct of_device_id of_hi655x_pmic_child_match_tbl[] = {
+	{ .compatible = "hisilicon,hi655x-regulator-pmic", },
+	{},
+};
+
+static const struct of_device_id of_hi655x_pmic_match_tbl[] = {
+	{ .compatible = "hisilicon,hi655x-pmic-driver", },
+	{},
+};
+
+static unsigned int hi655x_pmic_get_version(struct hi655x_pmic *pmic)
+{
+	u32 val;
+
+	regmap_read(pmic->regmap,
+		    HI655X_REG_TO_BUS_ADDR(HI655X_VER_REG), &val);
+
+	return val;
+}
+
+static irqreturn_t hi655x_pmic_irq_handler(int irq, void *data)
+{
+	struct hi655x_pmic *pmic = (struct hi655x_pmic *)data;
+	u32 pending;
+	u32 ret = IRQ_NONE;
+	unsigned long offset;
+	int i;
+
+	for (i = 0; i < HI655X_IRQ_ARRAY; i++) {
+		regmap_read(pmic->regmap,
+			    HI655X_REG_TO_BUS_ADDR(i + HI655X_IRQ_STAT_BASE),
+			    &pending);
+		if (pending)
+			pr_debug("pending[%d]=0x%x\n\r", i, pending);
+
+		/* clear pmic-sub-interrupt */
+		regmap_write(pmic->regmap,
+			     HI655X_REG_TO_BUS_ADDR(i + HI655X_IRQ_STAT_BASE),
+			     pending);
+
+		if (pending) {
+			for_each_set_bit(offset, (unsigned long *)&pending,
+					 HI655X_BITS)
+				generic_handle_irq(pmic->irqs[offset +
+						   i * HI655X_BITS]);
+			ret = IRQ_HANDLED;
+		}
+	}
+	return ret;
+}
+
+static void hi655x_pmic_irq_mask(struct irq_data *d)
+{
+	u32 data, offset;
+	unsigned long pmic_spin_flag = 0;
+	struct hi655x_pmic *pmic = irq_data_get_irq_chip_data(d);
+
+	offset = ((irqd_to_hwirq(d) >> 3) + HI655X_IRQ_MASK_BASE);
+	spin_lock_irqsave(&pmic->ssi_hw_lock, pmic_spin_flag);
+	regmap_read(pmic->regmap, HI655X_REG_TO_BUS_ADDR(offset), &data);
+	data |= (1 << (irqd_to_hwirq(d) & 0x07));
+	regmap_write(pmic->regmap, HI655X_REG_TO_BUS_ADDR(offset), data);
+	spin_unlock_irqrestore(&pmic->ssi_hw_lock, pmic_spin_flag);
+}
+
+static void hi655x_pmic_irq_unmask(struct irq_data *d)
+{
+	u32 data, offset;
+	unsigned long pmic_spin_flag = 0;
+	struct hi655x_pmic *pmic = irq_data_get_irq_chip_data(d);
+
+	offset = ((irqd_to_hwirq(d) >> 3) + HI655X_IRQ_MASK_BASE);
+	spin_lock_irqsave(&pmic->ssi_hw_lock, pmic_spin_flag);
+	regmap_read(pmic->regmap, HI655X_REG_TO_BUS_ADDR(offset), &data);
+	data &= ~(1 << (irqd_to_hwirq(d) & 0x07));
+	regmap_write(pmic->regmap, HI655X_REG_TO_BUS_ADDR(offset), data);
+	spin_unlock_irqrestore(&pmic->ssi_hw_lock, pmic_spin_flag);
+}
+
+static struct irq_chip hi655x_pmic_irqchip = {
+	.name		= "hisi-hi655x-pmic-irqchip",
+	.irq_mask	= hi655x_pmic_irq_mask,
+	.irq_unmask	= hi655x_pmic_irq_unmask,
+};
+
+static int hi655x_pmic_irq_map(struct irq_domain *d, unsigned int virq,
+			       irq_hw_number_t hw)
+{
+	struct hi655x_pmic *pmic = d->host_data;
+
+	irq_set_chip_and_handler_name(virq, &hi655x_pmic_irqchip,
+				      handle_simple_irq,
+				      "hisi-hi655x-pmic-irqchip");
+	irq_set_chip_data(virq, pmic);
+	irq_set_irq_type(virq, IRQ_TYPE_NONE);
+
+	return 0;
+}
+
+static struct irq_domain_ops hi655x_domain_ops = {
+	.map	= hi655x_pmic_irq_map,
+	.xlate	= irq_domain_xlate_twocell,
+};
+
+static inline void hi655x_pmic_clear_int(struct hi655x_pmic *pmic)
+{
+	int addr;
+
+	for (addr = HI655X_IRQ_STAT_BASE;
+	     addr < (HI655X_IRQ_STAT_BASE + HI655X_IRQ_ARRAY);
+	     addr++) {
+		regmap_write(pmic->regmap,
+			     HI655X_REG_TO_BUS_ADDR(addr), HI655X_IRQ_CLR);
+	}
+}
+
+static inline void hi655x_pmic_mask_int(struct hi655x_pmic *pmic)
+{
+	int addr;
+
+	for (addr = HI655X_IRQ_MASK_BASE;
+	     addr < (HI655X_IRQ_MASK_BASE + HI655X_IRQ_ARRAY);
+	     addr++) {
+		regmap_write(pmic->regmap,
+			     HI655X_REG_TO_BUS_ADDR(addr), HI655X_IRQ_MASK);
+	}
+}
+
+static struct regmap_config hi655x_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 8,
+	.max_register = HI655X_REG_TO_BUS_ADDR(HI655X_REG_MAX),
+};
+
+static int hi655x_pmic_irq_init(struct platform_device *pdev,
+				struct hi655x_pmic *pmic)
+{
+	enum of_gpio_flags gpio_flags;
+	struct device_node *np = (&pdev->dev)->of_node;
+	struct device_node *gpio_np = NULL;
+	unsigned int virq = 0;
+	int i, ret = 0;
+
+	pmic->ver = hi655x_pmic_get_version(pmic);
+	if ((pmic->ver < PMU_VER_START) || (pmic->ver > PMU_VER_END)) {
+		dev_warn(&pdev->dev, "it is wrong pmu version\n");
+		return -EINVAL;
+	}
+
+	regmap_write(pmic->regmap, HI655X_REG_TO_BUS_ADDR(ANA_IRQM_REG0), 0xff);
+
+	gpio_np = of_parse_phandle(np, "pmic-gpios", 0);
+	if (!gpio_np) {
+		dev_err(&pdev->dev, "can't parse property\n");
+		return -ENOENT;
+	}
+	pmic->gpio = of_get_gpio_flags(gpio_np, 0, &gpio_flags);
+	if (pmic->gpio < 0) {
+		dev_err(&pdev->dev,
+			"failed to of_get_gpio_flags %d\n", pmic->gpio);
+		return  pmic->gpio;
+	}
+	if (!gpio_is_valid(pmic->gpio)) {
+		dev_err(&pdev->dev, "it is invalid gpio %d\n", pmic->gpio);
+		return -EINVAL;
+	}
+	ret = gpio_request_one(pmic->gpio, GPIOF_IN, "hi655x_pmic_irq");
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to request gpio %d  ret = %d\n",
+			pmic->gpio, ret);
+		return ret;
+	}
+	pmic->irq = gpio_to_irq(pmic->gpio);
+
+	hi655x_pmic_clear_int(pmic);
+	hi655x_pmic_mask_int(pmic);
+	pmic->domain = irq_domain_add_simple(np,
+			HI655X_NR_IRQ, 0, &hi655x_domain_ops, pmic);
+	if (!pmic->domain) {
+		dev_err(&pdev->dev, "failed irq domain add simple!\n");
+		ret = -ENODEV;
+		goto irq_domain_add_simple;
+	}
+
+	for (i = 0; i < HI655X_NR_IRQ; i++) {
+		virq = irq_create_mapping(pmic->domain, i);
+		if (!virq) {
+			dev_err(&pdev->dev, "Failed mapping hwirq\n");
+			ret = -ENOSPC;
+			goto irq_create_mapping;
+		}
+		pmic->irqs[i] = virq;
+	}
+
+	ret = request_threaded_irq(pmic->irq, hi655x_pmic_irq_handler,
+				   NULL, IRQF_TRIGGER_LOW |
+				   IRQF_SHARED | IRQF_NO_SUSPEND,
+				   "hi655x-pmic-irq", pmic);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "could not claim pmic %d\n", ret);
+		ret = -ENODEV;
+		goto request_threaded_irq;
+	}
+	return 0;
+
+irq_domain_add_simple:
+irq_create_mapping:
+request_threaded_irq:
+	free_irq(pmic->irq, pmic);
+	gpio_free(pmic->gpio);
+	return ret;
+}
+
+static int hi655x_pmic_probe(struct platform_device *pdev)
+{
+	struct device_node *np = (&pdev->dev)->of_node;
+	struct hi655x_pmic *pmic = NULL;
+	void __iomem *base;
+	int ret;
+
+	pmic = devm_kzalloc(&pdev->dev, sizeof(*pmic), GFP_KERNEL);
+	if (!pmic)
+		return -ENOMEM;
+
+	spin_lock_init(&pmic->ssi_hw_lock);
+	pmic->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, pmic->res);
+	if (!base)
+		return -ENOMEM;
+
+	pmic->regmap = devm_regmap_init_mmio_clk(&pdev->dev, NULL, base,
+						 &hi655x_regmap_config);
+	ret = hi655x_pmic_irq_init(pdev, pmic);
+	if (ret) {
+		dev_err(&pdev->dev, "pmic irq init failed: %d\n", ret);
+		return ret;
+	}
+
+	pmic->dev = &pdev->dev;
+	platform_set_drvdata(pdev, pmic);
+	of_platform_populate(np, of_hi655x_pmic_child_match_tbl,
+			     NULL, &pdev->dev);
+
+	return 0;
+}
+
+static int hi655x_pmic_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct hi655x_pmic *pmic = platform_get_drvdata(pdev);
+
+	free_irq(pmic->irq, pmic);
+	gpio_free(pmic->gpio);
+	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 hi655x_pmic_driver = {
+	.driver	= {
+		.name =	"hisi,hi655x-pmic",
+		.owner = THIS_MODULE,
+		.of_match_table = of_hi655x_pmic_match_tbl,
+	},
+	.probe  = hi655x_pmic_probe,
+	.remove	= hi655x_pmic_remove,
+};
+module_platform_driver(hi655x_pmic_driver);
+
+MODULE_AUTHOR("Fei Wang <w.f-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>");
+MODULE_DESCRIPTION("Hisi 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..b303fb6
--- /dev/null
+++ b/include/linux/mfd/hi655x-pmic.h
@@ -0,0 +1,50 @@
+/*
+ * Head file for hi655x pmic
+ *
+ * Copyright (c) 2015 Hisilicon.
+ *
+ * Fei Wang <w.f-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
+ * Chen Feng <puck.chen-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
+ *
+ * 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_REG_TO_BUS_ADDR(x)        ((x) << 2)
+
+#define HI655X_BITS                      (8)
+
+/*numb of sub-interrupt*/
+#define HI655X_NR_IRQ                    (32)
+
+#define HI655X_IRQ_STAT_BASE             (0x003)
+#define HI655X_IRQ_MASK_BASE             (0x007)
+#define HI655X_IRQ_ARRAY                 (4)
+#define HI655X_IRQ_MASK                  (0x0ff)
+#define HI655X_IRQ_CLR                   (0x0ff)
+#define HI655X_VER_REG                   (0x000)
+#define HI655X_VER_REG                   (0x000)
+#define HI655X_REG_MAX                   (0x000)
+
+#define PMU_VER_START                    (0x010)
+#define PMU_VER_END                      (0x038)
+#define ANA_IRQM_REG0                    (0x1b5)
+
+struct hi655x_pmic {
+	struct resource *res;
+	struct device *dev;
+	struct regmap *regmap;
+	spinlock_t ssi_hw_lock;
+	struct clk *clk;
+	struct irq_domain *domain;
+	int irq;
+	int gpio;
+	unsigned int irqs[HI655X_NR_IRQ];
+	unsigned int ver;
+};
+#endif
-- 
1.9.1

Re: [PATCH 4/7] mfd: hi655x: Add hi665x pmic driver

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1243 bytes --]

On Thu, Nov 05, 2015 at 09:34:45PM +0800, Chen Feng wrote:

> +config MFD_HI655X_PMIC
> +        bool "HiSilicon Hi655X series PMU/Codec IC"

Why is this bool and not tristate?

> +        depends on ARCH_HISI

Can we have an || COMPILE_TEST here?

> +static irqreturn_t hi655x_pmic_irq_handler(int irq, void *data)
> +{
> +	struct hi655x_pmic *pmic = (struct hi655x_pmic *)data;
> +	u32 pending;
> +	u32 ret = IRQ_NONE;
> +	unsigned long offset;
> +	int i;

This looks like you should be able to use regmap_irq?

> +static int hi655x_pmic_remove(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct hi655x_pmic *pmic = platform_get_drvdata(pdev);
> +
> +	free_irq(pmic->irq, pmic);
> +	gpio_free(pmic->gpio);
> +	devm_release_mem_region(dev, pmic->res->start,
> +				resource_size(pmic->res));
> +	devm_kfree(dev, pmic);
> +	platform_set_drvdata(pdev, NULL);

There is no point in using devm_ cleanup functions in the device removal
path unless there's some ordering issue with respect to other stuff
which doesn't seem to be the case here.

> +static struct platform_driver hi655x_pmic_driver = {
> +	.driver	= {
> +		.name =	"hisi,hi655x-pmic",

We don't normally use OF style names in the Linux driver names.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 4/7] mfd: hi655x: Add hi665x pmic driver

[Andy Shevchenko]

On Thu, Nov 5, 2015 at 3:34 PM, Chen Feng <puck.chen@hisilicon.com> wrote:
> Add pmic driver to support hisilicon hi665x pmic.

> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/err.h>
> +#include <linux/clk.h>
> +#include <linux/slab.h>
> +#include <linux/ioport.h>
> +#include <linux/interrupt.h>
> +#include <linux/init.h>
> +#include <linux/gpio.h>
> +#include <linux/types.h>
> +#include <linux/mutex.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/hardirq.h>
> +#include <linux/of_gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/irqdomain.h>
> +#include <linux/mfd/hi655x-pmic.h>
> +#include <linux/regmap.h>

Do you need all of those?

> +static irqreturn_t hi655x_pmic_irq_handler(int irq, void *data)
> +{
> +       struct hi655x_pmic *pmic = (struct hi655x_pmic *)data;

No need to explicitly cast from or to void *.

> +       u32 pending;

unsigned long?

> +       u32 ret = IRQ_NONE;

irqreturn_t ret …

> +       unsigned long offset;
> +       int i;
> +
> +       for (i = 0; i < HI655X_IRQ_ARRAY; i++) {
> +               regmap_read(pmic->regmap,
> +                           HI655X_REG_TO_BUS_ADDR(i + HI655X_IRQ_STAT_BASE),
> +                           &pending);
> +               if (pending)
> +                       pr_debug("pending[%d]=0x%x\n\r", i, pending);

Why not move below?

> +
> +               /* clear pmic-sub-interrupt */

"Clear …"

> +               regmap_write(pmic->regmap,
> +                            HI655X_REG_TO_BUS_ADDR(i + HI655X_IRQ_STAT_BASE),
> +                            pending);
> +
> +               if (pending) {
> +                       for_each_set_bit(offset, (unsigned long *)&pending,

If pending is unsigned long no need to cast.

> +                                        HI655X_BITS)
> +                               generic_handle_irq(pmic->irqs[offset +
> +                                                  i * HI655X_BITS]);
> +                       ret = IRQ_HANDLED;
> +               }
> +       }
> +       return ret;
> +}
> +
> +static void hi655x_pmic_irq_mask(struct irq_data *d)
> +{
> +       u32 data, offset;
> +       unsigned long pmic_spin_flag = 0;

Why not more shorter flags?
Redundant assignment btw.

> +       struct hi655x_pmic *pmic = irq_data_get_irq_chip_data(d);
> +
> +       offset = ((irqd_to_hwirq(d) >> 3) + HI655X_IRQ_MASK_BASE);

External parens are not needed.

> +       spin_lock_irqsave(&pmic->ssi_hw_lock, pmic_spin_flag);
> +       regmap_read(pmic->regmap, HI655X_REG_TO_BUS_ADDR(offset), &data);
> +       data |= (1 << (irqd_to_hwirq(d) & 0x07));

Ditto.

> +       regmap_write(pmic->regmap, HI655X_REG_TO_BUS_ADDR(offset), data);
> +       spin_unlock_irqrestore(&pmic->ssi_hw_lock, pmic_spin_flag);
> +}
> +
> +static void hi655x_pmic_irq_unmask(struct irq_data *d)
> +{
> +       u32 data, offset;
> +       unsigned long pmic_spin_flag = 0;

Same comments as above.

> +       struct hi655x_pmic *pmic = irq_data_get_irq_chip_data(d);
> +
> +       offset = ((irqd_to_hwirq(d) >> 3) + HI655X_IRQ_MASK_BASE);

Same comments as above.

> +       spin_lock_irqsave(&pmic->ssi_hw_lock, pmic_spin_flag);
> +       regmap_read(pmic->regmap, HI655X_REG_TO_BUS_ADDR(offset), &data);
> +       data &= ~(1 << (irqd_to_hwirq(d) & 0x07));

> +       regmap_write(pmic->regmap, HI655X_REG_TO_BUS_ADDR(offset), data);
> +       spin_unlock_irqrestore(&pmic->ssi_hw_lock, pmic_spin_flag);
> +}
> +
> +static struct irq_chip hi655x_pmic_irqchip = {
> +       .name           = "hisi-hi655x-pmic-irqchip",
> +       .irq_mask       = hi655x_pmic_irq_mask,
> +       .irq_unmask     = hi655x_pmic_irq_unmask,
> +};
> +
> +static int hi655x_pmic_irq_map(struct irq_domain *d, unsigned int virq,
> +                              irq_hw_number_t hw)
> +{
> +       struct hi655x_pmic *pmic = d->host_data;
> +
> +       irq_set_chip_and_handler_name(virq, &hi655x_pmic_irqchip,
> +                                     handle_simple_irq,
> +                                     "hisi-hi655x-pmic-irqchip");
> +       irq_set_chip_data(virq, pmic);
> +       irq_set_irq_type(virq, IRQ_TYPE_NONE);
> +
> +       return 0;
> +}
> +
> +static struct irq_domain_ops hi655x_domain_ops = {
> +       .map    = hi655x_pmic_irq_map,
> +       .xlate  = irq_domain_xlate_twocell,
> +};
> +
> +static inline void hi655x_pmic_clear_int(struct hi655x_pmic *pmic)
> +{
> +       int addr;
> +
> +       for (addr = HI655X_IRQ_STAT_BASE;
> +            addr < (HI655X_IRQ_STAT_BASE + HI655X_IRQ_ARRAY);
> +            addr++) {
> +               regmap_write(pmic->regmap,
> +                            HI655X_REG_TO_BUS_ADDR(addr), HI655X_IRQ_CLR);
> +       }

int addr = …;

do {
} while (++addr < …);

looks simpler.

> +}
> +
> +static inline void hi655x_pmic_mask_int(struct hi655x_pmic *pmic)
> +{
> +       int addr;
> +
> +       for (addr = HI655X_IRQ_MASK_BASE;
> +            addr < (HI655X_IRQ_MASK_BASE + HI655X_IRQ_ARRAY);
> +            addr++) {
> +               regmap_write(pmic->regmap,
> +                            HI655X_REG_TO_BUS_ADDR(addr), HI655X_IRQ_MASK);
> +       }

Ditto.

> +}
> +
> +static struct regmap_config hi655x_regmap_config = {
> +       .reg_bits = 32,
> +       .reg_stride = 4,
> +       .val_bits = 8,
> +       .max_register = HI655X_REG_TO_BUS_ADDR(HI655X_REG_MAX),
> +};
> +
> +static int hi655x_pmic_irq_init(struct platform_device *pdev,
> +                               struct hi655x_pmic *pmic)
> +{
> +       enum of_gpio_flags gpio_flags;
> +       struct device_node *np = (&pdev->dev)->of_node;

Why not dot?

Of course you may do
 struct device *dev = &pdev->dev;
and use it here and below.

> +       struct device_node *gpio_np = NULL;

Redundant assignment?

> +       unsigned int virq = 0;

> +       int i, ret = 0;

Ditto.

Btw unsigned int i;

> +
> +       pmic->ver = hi655x_pmic_get_version(pmic);
> +       if ((pmic->ver < PMU_VER_START) || (pmic->ver > PMU_VER_END)) {
> +               dev_warn(&pdev->dev, "it is wrong pmu version\n");
> +               return -EINVAL;
> +       }
> +
> +       regmap_write(pmic->regmap, HI655X_REG_TO_BUS_ADDR(ANA_IRQM_REG0), 0xff);
> +
> +       gpio_np = of_parse_phandle(np, "pmic-gpios", 0);
> +       if (!gpio_np) {
> +               dev_err(&pdev->dev, "can't parse property\n");
> +               return -ENOENT;
> +       }
> +       pmic->gpio = of_get_gpio_flags(gpio_np, 0, &gpio_flags);
> +       if (pmic->gpio < 0) {
> +               dev_err(&pdev->dev,
> +                       "failed to of_get_gpio_flags %d\n", pmic->gpio);
> +               return  pmic->gpio;
> +       }
> +       if (!gpio_is_valid(pmic->gpio)) {
> +               dev_err(&pdev->dev, "it is invalid gpio %d\n", pmic->gpio);
> +               return -EINVAL;
> +       }

I think you may join this and above one.

> +       ret = gpio_request_one(pmic->gpio, GPIOF_IN, "hi655x_pmic_irq");
> +       if (ret < 0) {
> +               dev_err(&pdev->dev, "failed to request gpio %d  ret = %d\n",
> +                       pmic->gpio, ret);
> +               return ret;
> +       }
> +       pmic->irq = gpio_to_irq(pmic->gpio);
> +
> +       hi655x_pmic_clear_int(pmic);
> +       hi655x_pmic_mask_int(pmic);
> +       pmic->domain = irq_domain_add_simple(np,
> +                       HI655X_NR_IRQ, 0, &hi655x_domain_ops, pmic);
> +       if (!pmic->domain) {
> +               dev_err(&pdev->dev, "failed irq domain add simple!\n");
> +               ret = -ENODEV;
> +               goto irq_domain_add_simple;
> +       }
> +
> +       for (i = 0; i < HI655X_NR_IRQ; i++) {
> +               virq = irq_create_mapping(pmic->domain, i);
> +               if (!virq) {
> +                       dev_err(&pdev->dev, "Failed mapping hwirq\n");
> +                       ret = -ENOSPC;
> +                       goto irq_create_mapping;
> +               }
> +               pmic->irqs[i] = virq;
> +       }
> +
> +       ret = request_threaded_irq(pmic->irq, hi655x_pmic_irq_handler,
> +                                  NULL, IRQF_TRIGGER_LOW |
> +                                  IRQF_SHARED | IRQF_NO_SUSPEND,
> +                                  "hi655x-pmic-irq", pmic);
> +       if (ret < 0) {
> +               dev_err(&pdev->dev, "could not claim pmic %d\n", ret);
> +               ret = -ENODEV;

Why replace actual error?

> +               goto request_threaded_irq;
> +       }
> +       return 0;
> +
> +irq_domain_add_simple:
> +irq_create_mapping:
> +request_threaded_irq:

Looks strange. Either one label, or put them in proper places.

> +       free_irq(pmic->irq, pmic);
> +       gpio_free(pmic->gpio);
> +       return ret;
> +}
> +
> +static int hi655x_pmic_probe(struct platform_device *pdev)
> +{
> +       struct device_node *np = (&pdev->dev)->of_node;

Why not dot?

> +       struct hi655x_pmic *pmic = NULL;

Redundant assignment.

> +       void __iomem *base;
> +       int ret;
> +
> +       pmic = devm_kzalloc(&pdev->dev, sizeof(*pmic), GFP_KERNEL);
> +       if (!pmic)
> +               return -ENOMEM;
> +
> +       spin_lock_init(&pmic->ssi_hw_lock);
> +       pmic->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       base = devm_ioremap_resource(&pdev->dev, pmic->res);
> +       if (!base)
> +               return -ENOMEM;
> +
> +       pmic->regmap = devm_regmap_init_mmio_clk(&pdev->dev, NULL, base,
> +                                                &hi655x_regmap_config);
> +       ret = hi655x_pmic_irq_init(pdev, pmic);
> +       if (ret) {
> +               dev_err(&pdev->dev, "pmic irq init failed: %d\n", ret);
> +               return ret;
> +       }
> +
> +       pmic->dev = &pdev->dev;
> +       platform_set_drvdata(pdev, pmic);
> +       of_platform_populate(np, of_hi655x_pmic_child_match_tbl,
> +                            NULL, &pdev->dev);
> +
> +       return 0;
> +}
> +
> +static int hi655x_pmic_remove(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct hi655x_pmic *pmic = platform_get_drvdata(pdev);
> +
> +       free_irq(pmic->irq, pmic);
> +       gpio_free(pmic->gpio);
> +       devm_release_mem_region(dev, pmic->res->start,
> +                               resource_size(pmic->res));

> +       devm_kfree(dev, pmic);

Why?


> +       platform_set_drvdata(pdev, NULL);
> +       return 0;
> +}
> +
> +static struct platform_driver hi655x_pmic_driver = {
> +       .driver = {
> +               .name = "hisi,hi655x-pmic",
> +               .owner = THIS_MODULE,
> +               .of_match_table = of_hi655x_pmic_match_tbl,
> +       },
> +       .probe  = hi655x_pmic_probe,
> +       .remove = hi655x_pmic_remove,
> +};
> +module_platform_driver(hi655x_pmic_driver);
> +
> +MODULE_AUTHOR("Fei Wang <w.f@huawei.com>");
> +MODULE_DESCRIPTION("Hisi 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..b303fb6
> --- /dev/null
> +++ b/include/linux/mfd/hi655x-pmic.h
> @@ -0,0 +1,50 @@
> +/*
> + * Head file for hi655x pmic
> + *
> + * Copyright (c) 2015 Hisilicon.
> + *
> + * Fei Wang <w.f@huawei.com>
> + * Chen Feng <puck.chen@hisilicon.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_REG_TO_BUS_ADDR(x)        ((x) << 2)
> +
> +#define HI655X_BITS                      (8)
> +
> +/*numb of sub-interrupt*/
> +#define HI655X_NR_IRQ                    (32)
> +
> +#define HI655X_IRQ_STAT_BASE             (0x003)
> +#define HI655X_IRQ_MASK_BASE             (0x007)
> +#define HI655X_IRQ_ARRAY                 (4)
> +#define HI655X_IRQ_MASK                  (0x0ff)
> +#define HI655X_IRQ_CLR                   (0x0ff)
> +#define HI655X_VER_REG                   (0x000)
> +#define HI655X_VER_REG                   (0x000)
> +#define HI655X_REG_MAX                   (0x000)
> +
> +#define PMU_VER_START                    (0x010)
> +#define PMU_VER_END                      (0x038)
> +#define ANA_IRQM_REG0                    (0x1b5)
> +
> +struct hi655x_pmic {
> +       struct resource *res;
> +       struct device *dev;
> +       struct regmap *regmap;
> +       spinlock_t ssi_hw_lock;

Just lock as a name.

> +       struct clk *clk;
> +       struct irq_domain *domain;
> +       int irq;
> +       int gpio;
> +       unsigned int irqs[HI655X_NR_IRQ];
> +       unsigned int ver;
> +};
> +#endif
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
With Best Regards,
Andy Shevchenko

[PATCH 5/7] regulator: add driver for mtcmos voltage regulator on hi6220 SoC

[Chen Feng]

Add driver to support mtcmos on hi6220

Signed-off-by: Chen Feng <puck.chen-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
Signed-off-by: Fei Wang <w.f-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/regulator/hi6220-mtcmos.c | 245 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 245 insertions(+)
 create mode 100644 drivers/regulator/hi6220-mtcmos.c

diff --git a/drivers/regulator/hi6220-mtcmos.c b/drivers/regulator/hi6220-mtcmos.c
new file mode 100644
index 0000000..c79ffc0
--- /dev/null
+++ b/drivers/regulator/hi6220-mtcmos.c
@@ -0,0 +1,245 @@
+/*
+ * Device driver for regulators in hi6220 mtcmos
+ *
+ * Copyright (c) 2015 Hisilicon.
+ *
+ * Fei Wang <w.f-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
+ * Chen Feng <puck.chen-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
+ *
+ * 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/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/sizes.h>
+
+enum {
+	HI6220_MTCMOS1,
+	HI6220_MTCMOS2,
+	HI6220_RG_MAX,
+};
+
+struct hi6220_mtcmos_ctrl_regs {
+	unsigned int enable_reg;
+	unsigned int disable_reg;
+	unsigned int status_reg;
+};
+
+struct hi6220_mtcmos_ctrl_data {
+	int shift;
+	unsigned int mask;
+};
+
+struct hi6220_mtcmos_info {
+	struct regulator_desc rdesc;
+	struct hi6220_mtcmos_ctrl_regs ctrl_regs;
+	struct hi6220_mtcmos_ctrl_data ctrl_data;
+};
+
+struct hi6220_mtcmos {
+	struct regulator_dev *rdev[HI6220_RG_MAX];
+	void __iomem *sc_on_regs;
+};
+
+static int hi6220_mtcmos_is_on(struct hi6220_mtcmos *mtcmos,
+			       unsigned int regs, unsigned int mask, int shift)
+{
+	unsigned int ret;
+
+	ret = readl(mtcmos->sc_on_regs + regs);
+	ret &= (mask << shift);
+
+	return ret;
+}
+
+static int hi6220_mtcmos_is_enabled(struct regulator_dev *rdev)
+{
+	int ret;
+	struct hi6220_mtcmos_info *sreg = rdev_get_drvdata(rdev);
+	struct platform_device *pdev =
+		container_of(rdev->dev.parent, struct platform_device, dev);
+	struct hi6220_mtcmos *mtcmos = platform_get_drvdata(pdev);
+	struct hi6220_mtcmos_ctrl_regs *ctrl_regs = &sreg->ctrl_regs;
+	struct hi6220_mtcmos_ctrl_data *ctrl_data = &sreg->ctrl_data;
+
+	ret = hi6220_mtcmos_is_on(mtcmos, ctrl_regs->status_reg,
+				  ctrl_data->mask, ctrl_data->shift);
+	return ret;
+}
+
+static int hi6220_mtcmos_op(struct hi6220_mtcmos *mtcmos,
+		      unsigned int regs, unsigned int mask, int shift)
+{
+	writel(mask << shift, mtcmos->sc_on_regs + regs);
+
+	return 0;
+}
+
+static int hi6220_mtcmos_enable(struct regulator_dev *rdev)
+{
+	int ret;
+	struct hi6220_mtcmos_info *sreg = rdev_get_drvdata(rdev);
+	struct platform_device *pdev =
+		container_of(rdev->dev.parent, struct platform_device, dev);
+	struct hi6220_mtcmos *mtcmos = platform_get_drvdata(pdev);
+	struct hi6220_mtcmos_ctrl_regs *ctrl_regs = &sreg->ctrl_regs;
+	struct hi6220_mtcmos_ctrl_data *ctrl_data = &sreg->ctrl_data;
+
+	hi6220_mtcmos_op(mtcmos, ctrl_regs->enable_reg,
+			 ctrl_data->mask, ctrl_data->shift);
+	ret =  hi6220_mtcmos_is_on(mtcmos, ctrl_regs->status_reg,
+				   ctrl_data->mask, ctrl_data->shift)
+	return ret;
+}
+
+static int hi6220_mtcmos_disable(struct regulator_dev *rdev)
+{
+	int ret;
+	struct hi6220_mtcmos_info *sreg = rdev_get_drvdata(rdev);
+	struct platform_device *pdev =
+		container_of(rdev->dev.parent, struct platform_device, dev);
+	struct hi6220_mtcmos *mtcmos = platform_get_drvdata(pdev);
+	struct hi6220_mtcmos_ctrl_regs  *ctrl_regs = &sreg->ctrl_regs;
+	struct hi6220_mtcmos_ctrl_data  *ctrl_data = &sreg->ctrl_data;
+
+	ret = hi6220_mtcmos_op(mtcmos, ctrl_regs->disable_reg,
+			       ctrl_data->mask, ctrl_data->shift);
+
+	return ret;
+}
+
+static struct regulator_ops hi6220_mtcmos_mtcmos_rops = {
+	.is_enabled = hi6220_mtcmos_is_enabled,
+	.enable = hi6220_mtcmos_enable,
+	.disable = hi6220_mtcmos_disable,
+};
+
+#define HI6220_MTCMOS(vreg) \
+{								\
+	.rdesc = {					\
+		.name = #vreg,			\
+		.ops	= &hi6220_mtcmos_mtcmos_rops, \
+		.type = REGULATOR_VOLTAGE,			\
+		.owner = THIS_MODULE,		\
+	},							\
+}
+
+static struct hi6220_mtcmos_info hi6220_mtcmos_info[] = {
+	HI6220_MTCMOS(MTCMOS1),
+	HI6220_MTCMOS(MTCMOS2),
+};
+
+static struct of_regulator_match hi6220_mtcmos_matches[] = {
+	{ .name = "mtcmos1",
+		.driver_data = &hi6220_mtcmos_info[HI6220_MTCMOS1], },
+	{ .name = "mtcmos2",
+		.driver_data = &hi6220_mtcmos_info[HI6220_MTCMOS2], },
+};
+
+static int hi6220_mtcmos_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct hi6220_mtcmos *mtcmos;
+	const __be32 *sc_on_regs = NULL;
+	void __iomem	*regs;
+	struct device *dev;
+	struct device_node *np, *child;
+	int i;
+	struct regulator_config config = { };
+	struct regulator_init_data *init_data;
+	struct hi6220_mtcmos_info *sreg;
+	u32 off_on_delay = 0;
+
+	dev = &pdev->dev;
+	np = dev->of_node;
+	mtcmos = devm_kzalloc(dev, sizeof(struct hi6220_mtcmos), GFP_KERNEL);
+	if (!mtcmos)
+		return -ENOMEM;
+
+	sc_on_regs = of_get_property(np, "hisilicon,mtcmos-sc-on-base", NULL);
+	if (sc_on_regs) {
+		regs = ioremap(be32_to_cpu(*sc_on_regs), SZ_4K);
+		mtcmos->sc_on_regs = regs;
+	} else
+		return -ENODEV;
+	of_property_read_u32(np, "hisilicon,mtcmos-steady-us", &off_on_delay);
+
+	for (i = 0; i < HI6220_RG_MAX; i++) {
+		init_data = hi6220_mtcmos_matches[i].init_data;
+		if (!init_data)
+			continue;
+		sreg = hi6220_mtcmos_matches[i].driver_data;
+		sreg->rdesc.off_on_delay = off_on_delay;
+		config.dev = &pdev->dev;
+		config.init_data = init_data;
+		config.driver_data = sreg;
+		config.of_node = hi6220_mtcmos_matches[i].of_node;
+		child = config.of_node;
+
+		ret = of_property_read_u32_array(child, "hisilicon,ctrl-regs",
+						 (u32 *)(&sreg->ctrl_regs),
+						 0x3);
+		ret = of_property_read_u32_array(child, "hisilicon,ctrl-data",
+						 (u32 *)(&sreg->ctrl_data),
+						 0x2);
+
+		mtcmos->rdev[i] = regulator_register(&sreg->rdesc, &config);
+		if (IS_ERR(mtcmos->rdev[i])) {
+			ret = PTR_ERR(mtcmos->rdev[i]);
+			dev_err(&pdev->dev, "failed to register mtcmos %s\n",
+				sreg->rdesc.name);
+			while (--i >= 0)
+				regulator_unregister(mtcmos->rdev[i]);
+
+			return ret;
+		}
+	}
+
+	platform_set_drvdata(pdev, mtcmos);
+
+	return 0;
+}
+
+static const struct of_device_id of_hi6220_mtcmos_match_tbl[] = {
+	{ .compatible = "hisilicon,hi6220-mtcmos-driver", },
+	{}
+};
+
+static struct platform_driver mtcmos_driver = {
+	.driver = {
+		.name = "hisi_hi6220_mtcmos",
+		.owner = THIS_MODULE,
+		.of_match_table = of_hi6220_mtcmos_match_tbl,
+	},
+	.probe = hi6220_mtcmos_probe,
+};
+
+static int __init hi6220_mtcmos_init(void)
+{
+	return platform_driver_register(&mtcmos_driver);
+}
+
+static void __exit hi6220_mtcmos_exit(void)
+{
+	platform_driver_unregister(&mtcmos_driver);
+}
+
+fs_initcall(hi6220_mtcmos_init);
+module_exit(hi6220_mtcmos_exit);
+
+MODULE_AUTHOR("Fei Wang <w.f-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>");
+MODULE_DESCRIPTION("Hi6220 mtcmos interface driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

Re: [PATCH 5/7] regulator: add driver for mtcmos voltage regulator on hi6220 SoC

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 2836 bytes --]

On Thu, Nov 05, 2015 at 09:34:46PM +0800, Chen Feng wrote:
> Add driver to support mtcmos on hi6220

I just noticed that these patches are all being posted to the IOMMU list
- that seems a bit odd?

> +static int hi6220_mtcmos_is_on(struct hi6220_mtcmos *mtcmos,
> +			       unsigned int regs, unsigned int mask, int shift)
> +{
> +	unsigned int ret;
> +
> +	ret = readl(mtcmos->sc_on_regs + regs);
> +	ret &= (mask << shift);
> +
> +	return ret;
> +}
> +
> +static int hi6220_mtcmos_is_enabled(struct regulator_dev *rdev)
> +{
> +	int ret;
> +	struct hi6220_mtcmos_info *sreg = rdev_get_drvdata(rdev);
> +	struct platform_device *pdev =
> +		container_of(rdev->dev.parent, struct platform_device, dev);
> +	struct hi6220_mtcmos *mtcmos = platform_get_drvdata(pdev);
> +	struct hi6220_mtcmos_ctrl_regs *ctrl_regs = &sreg->ctrl_regs;
> +	struct hi6220_mtcmos_ctrl_data *ctrl_data = &sreg->ctrl_data;
> +
> +	ret = hi6220_mtcmos_is_on(mtcmos, ctrl_regs->status_reg,
> +				  ctrl_data->mask, ctrl_data->shift);
> +	return ret;
> +}

That's a *lot* of code for checking if a single bit is set, the same
thinng applies to the rest of the driver.  Unless this is for some
reason very performance critical I'd recommend just using regmap-mmio
and the regmap helpers, that will enable you to remove almost all the
code here.  Even if you can't do that at least removing the extra level
of helper function would help.

> +	sc_on_regs = of_get_property(np, "hisilicon,mtcmos-sc-on-base", NULL);
> +	if (sc_on_regs) {
> +		regs = ioremap(be32_to_cpu(*sc_on_regs), SZ_4K);
> +		mtcmos->sc_on_regs = regs;
> +	} else
> +		return -ENODEV;

{ } on both sides of the if statement.  You should also use normal
reg resource specifiers for register blocks the you need rather than
open coding some custom properties with absolute addresses.

> +	for (i = 0; i < HI6220_RG_MAX; i++) {
> +		init_data = hi6220_mtcmos_matches[i].init_data;
> +		if (!init_data)
> +			continue;

No, you should register all regulators on the device not just those with
init_data - you should just let the core do the DT parsing for you using
the standard of_match feature in the regulator_desc.

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

devm_regulator_register().

> +static const struct of_device_id of_hi6220_mtcmos_match_tbl[] = {
> +	{ .compatible = "hisilicon,hi6220-mtcmos-driver", },

Why is -driver part of the compatible?

> +	.driver = {
> +		.name = "hisi_hi6220_mtcmos",

Linux generally uses - not _ in names.

> +static int __init hi6220_mtcmos_init(void)
> +{
> +	return platform_driver_register(&mtcmos_driver);
> +}
> +
> +static void __exit hi6220_mtcmos_exit(void)
> +{
> +	platform_driver_unregister(&mtcmos_driver);
> +}
> +
> +fs_initcall(hi6220_mtcmos_init);

Why is this at fs_initcall?!

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 6/7] regulator: hisilicon: Add hi655x pmic voltage regulator driver

[Chen Feng]

Add driver support for HiSilicon Hi655x voltage regulators.

Signed-off-by: Chen Feng <puck.chen-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
Signed-off-by: Fei Wang <w.f-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/regulator/Kconfig                  |  13 ++
 drivers/regulator/Makefile                 |   2 +
 drivers/regulator/hi655x-regulator.c       | 246 +++++++++++++++++++++++++++++
 include/linux/regulator/hi655x-regulator.h |  63 ++++++++
 4 files changed, 324 insertions(+)
 create mode 100644 drivers/regulator/hi655x-regulator.c
 create mode 100644 include/linux/regulator/hi655x-regulator.h

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 64bccff..9bbd43c 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -261,6 +261,19 @@ 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_HI6220_MTCMOS
+        bool "Hisilicon Hi6220 mtcmos support"
+        depends on ARCH_HISI
+        help
+          This driver provides support for the mtcmos regulators of Hi6220 Soc.
+
+config REGULATOR_HI655X
+        bool "HiSilicon Hi655x PMIC voltage regulator support"
+        depends on ARCH_HISI
+        help
+	  This driver provides support for the voltage regulators on the
+	  HiSilicon hi655x PMIC.
+
 config REGULATOR_ISL9305
 	tristate "Intersil ISL9305 regulator"
 	depends on I2C
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 0f81749..7345d43 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -34,6 +34,8 @@ 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_HI6220_MTCMOS) += hi6220-mtcmos.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..b06c810
--- /dev/null
+++ b/drivers/regulator/hi655x-regulator.c
@@ -0,0 +1,246 @@
+/*
+ * Device driver for regulators in hi655x IC
+ *
+ * Copyright (c) 2015 Hisilicon.
+ *
+ * Fei Wang <w.f-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
+ * Chen Feng <puck.chen-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
+ *
+ * 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/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/delay.h>
+#include <linux/time.h>
+#include <linux/regulator/hi655x-regulator.h>
+#include <linux/mfd/hi655x-pmic.h>
+#include <linux/regmap.h>
+#include <linux/bitops.h>
+
+static int hi655x_is_enabled(struct regulator_dev *rdev)
+{
+	unsigned int value = 0;
+
+	struct hi655x_regulator *regulator = rdev_get_drvdata(rdev);
+	struct hi655x_regulator_ctrl_regs *ctrl_regs = &regulator->ctrl_regs;
+
+	regmap_read(rdev->regmap, ctrl_regs->status_reg, &value);
+	return (value & BIT(regulator->ctrl_mask));
+}
+
+static int hi655x_enable(struct regulator_dev *rdev)
+{
+	int ret = 0;
+	struct hi655x_regulator *regulator = rdev_get_drvdata(rdev);
+	struct hi655x_regulator_ctrl_regs *ctrl_regs = &regulator->ctrl_regs;
+
+	ret = regmap_update_bits(rdev->regmap, ctrl_regs->enable_reg,
+				 regulator->ctrl_mask, regulator->ctrl_mask);
+	return ret;
+}
+
+static int hi655x_disable(struct regulator_dev *rdev)
+{
+	int ret = 0;
+	struct hi655x_regulator *regulator = rdev_get_drvdata(rdev);
+
+	if (!regulator) {
+		pr_err("get driver data error!\n");
+		return -ENODEV;
+	}
+	struct hi655x_regulator_ctrl_regs *ctrl_regs = &regulator->ctrl_regs;
+
+	ret = regmap_update_bits(rdev->regmap, ctrl_regs->disable_reg,
+				 regulator->ctrl_mask, regulator->ctrl_mask);
+	return ret;
+}
+
+static int hi655x_get_voltage(struct regulator_dev *rdev)
+{
+	unsigned int value = 0;
+	struct hi655x_regulator *regulator = rdev_get_drvdata(rdev);
+
+	if (!regulator) {
+		pr_err("get driver data error!\n");
+		return -ENODEV;
+	}
+	struct hi655x_regulator_vset_regs *vset_regs = &regulator->vset_regs;
+
+	regmap_read(rdev->regmap, vset_regs->vset_reg, &value);
+
+	return regulator->vset_table[value];
+}
+
+static int hi655x_set_voltage(struct regulator_dev *rdev,
+			      int min_uV, int max_uV, unsigned *selector)
+{
+	int i = 0;
+	int ret = 0;
+	int vol = 0;
+	struct hi655x_regulator *regulator = rdev_get_drvdata(rdev);
+
+	if (!regulator) {
+		pr_err("get driver data error!\n");
+		return -ENODEV;
+	}
+
+	struct hi655x_regulator_vset_regs *vset_regs = &regulator->vset_regs;
+
+	/**
+	 * search the matched vol and get its index
+	 */
+	for (i = 0; i < regulator->vol_numb; i++) {
+		vol = regulator->vset_table[i];
+		if ((vol >= min_uV) && (vol <= max_uV))
+			break;
+	}
+
+	if (i == regulator->vol_numb)
+		return -1;
+
+	regmap_update_bits(rdev->regmap, vset_regs->vset_reg,
+			   regulator->vset_mask, i);
+	*selector = i;
+
+	return ret;
+}
+
+static unsigned int hi655x_map_mode(unsigned int mode)
+{
+	/* hi655x pmic on hi6220 SoC only support normal mode */
+	if (mode == REGULATOR_MODE_NORMAL)
+		return REGULATOR_MODE_NORMAL;
+	else
+		return -EINVAL;
+}
+
+static int hi655x_set_mode(struct regulator_dev *rdev,
+			   unsigned int mode)
+
+{
+	if (mode == REGULATOR_MODE_NORMAL)
+		return 0;
+	else
+		return -EINVAL;
+}
+
+static struct regulator_ops hi655x_regulator_ops = {
+	.is_enabled = hi655x_is_enabled,
+	.enable = hi655x_enable,
+	.disable = hi655x_disable,
+	.list_voltage = regulator_list_voltage_table,
+	.get_voltage = hi655x_get_voltage,
+	.set_voltage = hi655x_set_voltage,
+	.set_mode = hi655x_set_mode,
+};
+
+static const struct of_device_id of_hi655x_regulator_match_tbl[] = {
+	{
+		.compatible = "hisilicon,hi655x-regulator-pmic",
+	},
+};
+MODULE_DEVICE_TABLE(of, of_hi655x_regulator_match_tbl);
+
+/**
+ * get the hi655x specific data from dt node.
+ */
+static void of_get_hi655x_ctr(struct hi655x_regulator *regulator,
+			      struct device *dev, struct device_node *np)
+{
+	unsigned int *vset_table = NULL;
+
+	of_property_read_u32_array(np, "regulator-ctrl-regs",
+				   (u32 *)&regulator->ctrl_regs, 0x3);
+	of_property_read_u32(np, "regulator-ctrl-mask", &regulator->ctrl_mask);
+	of_property_read_u32(np, "regulator-vset-regs",
+			     (u32 *)&regulator->vset_regs);
+	of_property_read_u32(np, "regulator-vset-mask", &regulator->vset_mask);
+	of_property_read_u32(np, "regulator-n-vol", &regulator->vol_numb);
+	of_property_read_u32(np, "regulator-off-on-delay",
+			     &regulator->rdesc.off_on_delay);
+
+	vset_table = devm_kzalloc(dev, regulator->vol_numb * sizeof(int),
+				  GFP_KERNEL);
+
+	of_property_read_u32_array(np, "regulator-vset-table",
+				   vset_table,
+				   regulator->vol_numb);
+	regulator->vset_table = vset_table;
+	regulator->rdesc.volt_table = vset_table;
+	regulator->rdesc.n_voltages = regulator->vol_numb;
+}
+
+static int hi655x_regulator_probe(struct platform_device *pdev)
+{
+	int ret = 0;
+	struct hi655x_regulator *regulator;
+	struct hi655x_pmic *pmic;
+	struct regulator_init_data *init_data;
+	struct regulator_config config = { };
+	struct device_node *np = pdev->dev.of_node;
+
+	pmic = dev_get_drvdata(pdev->dev.parent);
+	if (!pmic) {
+		pr_err("no pmic in the regulator parent node\n");
+		return -ENODEV;
+	}
+
+	regulator = devm_kzalloc(&pdev->dev, sizeof(*regulator), GFP_KERNEL);
+	if (!regulator)
+		return -ENOMEM;
+	of_get_hi655x_ctr(regulator, &pdev->dev, np);
+
+	regulator->rdesc.name = dev_name(&pdev->dev);
+	regulator->rdesc.type = REGULATOR_VOLTAGE;
+	regulator->rdesc.owner = THIS_MODULE;
+	regulator->rdesc.of_map_mode = hi655x_map_mode;
+	regulator->rdesc.ops = &hi655x_regulator_ops;
+	init_data = of_get_regulator_init_data(&pdev->dev, pdev->dev.of_node,
+					       &regulator->rdesc);
+	if (!init_data) {
+		pr_err("get init data from dts error!\n");
+		return -EINVAL;
+	}
+	config.dev = &pdev->dev;
+	config.init_data = init_data;
+	config.driver_data = regulator;
+	config.regmap = pmic->regmap;
+
+	regulator->regdev = devm_regulator_register(&pdev->dev,
+						    &regulator->rdesc,
+						    &config);
+	if (IS_ERR(regulator->regdev)) {
+		pr_err("register regulator to system error!\n");
+		return PTR_ERR(regulator->regdev);
+	}
+
+	platform_set_drvdata(pdev, regulator);
+	return 0;
+}
+
+static struct platform_driver hi655x_regulator_driver = {
+	.driver = {
+		.name	= "hi655x_regulator",
+		.of_match_table = of_hi655x_regulator_match_tbl,
+	},
+	.probe	= hi655x_regulator_probe,
+};
+module_platform_driver(hi655x_regulator_driver);
+
+MODULE_AUTHOR("Chen Feng <puck.chen-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>");
+MODULE_DESCRIPTION("Hisi hi655x regulator driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/regulator/hi655x-regulator.h b/include/linux/regulator/hi655x-regulator.h
new file mode 100644
index 0000000..4a32095
--- /dev/null
+++ b/include/linux/regulator/hi655x-regulator.h
@@ -0,0 +1,63 @@
+/*
+ * Device driver for regulators in HI6553 IC
+ *
+ * Copyright (c) 2015 Hisilicon.
+ *
+ * Fei Wang  <w.f-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
+ * Chen Feng <puck.chen-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
+ *
+ * this regulator's probe function will be called lots of times,,
+ * because of there are lots of regulator nodes in dtb.
+ * so,that's say, the driver must be inited before the regulator nodes
+ * registor to system.
+ *
+ * Makefile have proved my guess, please refor to the makefile.
+ * when the code is rebuild i hope we can build pmu sub_system.
+ * init order can not base on compile
+ */
+
+#ifndef __HISI_HI655X_REGULATOR_H__
+#define __HISI_HI655X_REGULATOR_H__
+
+enum hi655x_regulator_type {
+	PMIC_BUCK_TYPE = 0,
+	PMIC_LDO_TYPE = 1,
+	PMIC_LVS_TYPE = 2,
+	PMIC_BOOST_TYPE = 3,
+	MTCMOS_SC_ON_TYPE = 4,
+	MTCMOS_ACPU_ON_TYPE = 5,
+	SCHARGE_TYPE = 6,
+};
+
+struct hi655x_regulator_ctrl_regs {
+	unsigned int  enable_reg;
+	unsigned int  disable_reg;
+	unsigned int  status_reg;
+};
+
+struct hi655x_regulator_vset_regs {
+	unsigned int vset_reg;
+};
+
+struct hi655x_regulator_ctrl_data {
+	int          shift;
+	unsigned int val;
+};
+
+struct hi655x_regulator_vset_data {
+	int          shift;
+	unsigned int mask;
+};
+
+struct hi655x_regulator {
+	struct hi655x_regulator_ctrl_regs ctrl_regs;
+	struct hi655x_regulator_vset_regs vset_regs;
+	u32 ctrl_mask;
+	u32 vset_mask;
+	u32 vol_numb;
+	u32 *vset_table;
+	struct regulator_desc rdesc;
+	struct regulator_dev *regdev;
+};
+
+#endif
-- 
1.9.1

Re: [PATCH 6/7] regulator: hisilicon: Add hi655x pmic voltage regulator driver

[Mark Brown]

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

On Thu, Nov 05, 2015 at 09:34:47PM +0800, Chen Feng wrote:

> +config REGULATOR_HI6220_MTCMOS
> +        bool "Hisilicon Hi6220 mtcmos support"
> +        depends on ARCH_HISI
> +        help
> +          This driver provides support for the mtcmos regulators of Hi6220 Soc.
> +

The Kconfig and Makefile updates for MCTMOS should have been in the
patch adding the driver for that.

> +config REGULATOR_HI655X
> +        bool "HiSilicon Hi655x PMIC voltage regulator support"
> +        depends on ARCH_HISI

For both of these we should have an || COMPILE_TEST and there's no need
for either to be bool I can see, they should be tristate.

> +static int hi655x_is_enabled(struct regulator_dev *rdev)
> +{
> +	unsigned int value = 0;
> +
> +	struct hi655x_regulator *regulator = rdev_get_drvdata(rdev);
> +	struct hi655x_regulator_ctrl_regs *ctrl_regs = &regulator->ctrl_regs;
> +
> +	regmap_read(rdev->regmap, ctrl_regs->status_reg, &value);
> +	return (value & BIT(regulator->ctrl_mask));
> +}

Use the standard regmap helpers, don't open code them.

> +static int hi655x_set_voltage(struct regulator_dev *rdev,
> +			      int min_uV, int max_uV, unsigned *selector)

Use the standard helpers, including one of the map_voltage()s and
set_voltage_sel_regmap(), don't open code them.

> +static unsigned int hi655x_map_mode(unsigned int mode)
> +{
> +	/* hi655x pmic on hi6220 SoC only support normal mode */
> +	if (mode == REGULATOR_MODE_NORMAL)
> +		return REGULATOR_MODE_NORMAL;
> +	else
> +		return -EINVAL;
> +}

If the device only has one mode it should not have any mode operations,
they're only meaningful if there are multiple modes to set and they are
optional.

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

Re: [PATCH 6/7] regulator: hisilicon: Add hi655x pmic voltage regulator driver

[kbuild test robot]

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

Hi Chen,

[auto build test ERROR on regulator/for-next]
[also build test ERROR on v4.3 next-20151106]

url:    https://github.com/0day-ci/linux/commits/Chen-Feng/Add-Support-for-Hi6220-PMIC-Hi6553-MFD-Core/20151105-215603
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
config: arm-allmodconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All error/warnings (new ones prefixed by >>):

   drivers/regulator/hi6220-mtcmos.c: In function 'hi6220_mtcmos_enable':
>> drivers/regulator/hi6220-mtcmos.c:105:2: error: expected ';' before 'return'
     return ret;
     ^
>> drivers/regulator/hi6220-mtcmos.c:106:1: warning: no return statement in function returning non-void [-Wreturn-type]
    }
    ^
--
   drivers/regulator/hi655x-regulator.c: In function 'hi655x_disable':
>> drivers/regulator/hi655x-regulator.c:65:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
     struct hi655x_regulator_ctrl_regs *ctrl_regs = &regulator->ctrl_regs;
     ^
   drivers/regulator/hi655x-regulator.c: In function 'hi655x_get_voltage':
   drivers/regulator/hi655x-regulator.c:81:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
     struct hi655x_regulator_vset_regs *vset_regs = &regulator->vset_regs;
     ^
   drivers/regulator/hi655x-regulator.c: In function 'hi655x_set_voltage':
   drivers/regulator/hi655x-regulator.c:101:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
     struct hi655x_regulator_vset_regs *vset_regs = &regulator->vset_regs;
     ^
   drivers/regulator/hi655x-regulator.c: In function 'hi655x_regulator_probe':
   drivers/regulator/hi655x-regulator.c:189:6: warning: unused variable 'ret' [-Wunused-variable]
     int ret = 0;
         ^

vim +65 drivers/regulator/hi655x-regulator.c

    49		struct hi655x_regulator_ctrl_regs *ctrl_regs = &regulator->ctrl_regs;
    50	
    51		ret = regmap_update_bits(rdev->regmap, ctrl_regs->enable_reg,
    52					 regulator->ctrl_mask, regulator->ctrl_mask);
    53		return ret;
    54	}
    55	
    56	static int hi655x_disable(struct regulator_dev *rdev)
    57	{
    58		int ret = 0;
    59		struct hi655x_regulator *regulator = rdev_get_drvdata(rdev);
    60	
    61		if (!regulator) {
    62			pr_err("get driver data error!\n");
    63			return -ENODEV;
    64		}
  > 65		struct hi655x_regulator_ctrl_regs *ctrl_regs = &regulator->ctrl_regs;
    66	
    67		ret = regmap_update_bits(rdev->regmap, ctrl_regs->disable_reg,
    68					 regulator->ctrl_mask, regulator->ctrl_mask);
    69		return ret;
    70	}
    71	
    72	static int hi655x_get_voltage(struct regulator_dev *rdev)
    73	{

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 53757 bytes --]

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 6/7] regulator: hisilicon: Add hi655x pmic voltage regulator driver

[Andy Shevchenko]

On Thu, Nov 5, 2015 at 3:34 PM, Chen Feng <puck.chen-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org> wrote:
> Add driver support for HiSilicon Hi655x voltage regulators.

> +#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/delay.h>
> +#include <linux/time.h>
> +#include <linux/regulator/hi655x-regulator.h>
> +#include <linux/mfd/hi655x-pmic.h>
> +#include <linux/regmap.h>
> +#include <linux/bitops.h>
> +
> +static int hi655x_is_enabled(struct regulator_dev *rdev)
> +{
> +       unsigned int value = 0;
> +
> +       struct hi655x_regulator *regulator = rdev_get_drvdata(rdev);
> +       struct hi655x_regulator_ctrl_regs *ctrl_regs = &regulator->ctrl_regs;
> +
> +       regmap_read(rdev->regmap, ctrl_regs->status_reg, &value);
> +       return (value & BIT(regulator->ctrl_mask));
> +}
> +
> +static int hi655x_enable(struct regulator_dev *rdev)
> +{
> +       int ret = 0;
> +       struct hi655x_regulator *regulator = rdev_get_drvdata(rdev);
> +       struct hi655x_regulator_ctrl_regs *ctrl_regs = &regulator->ctrl_regs;
> +
> +       ret = regmap_update_bits(rdev->regmap, ctrl_regs->enable_reg,
> +                                regulator->ctrl_mask, regulator->ctrl_mask);
> +       return ret;
> +}
> +
> +static int hi655x_disable(struct regulator_dev *rdev)
> +{
> +       int ret = 0;
> +       struct hi655x_regulator *regulator = rdev_get_drvdata(rdev);
> +
> +       if (!regulator) {
> +               pr_err("get driver data error!\n");
> +               return -ENODEV;
> +       }
> +       struct hi655x_regulator_ctrl_regs *ctrl_regs = &regulator->ctrl_regs;
> +
> +       ret = regmap_update_bits(rdev->regmap, ctrl_regs->disable_reg,
> +                                regulator->ctrl_mask, regulator->ctrl_mask);
> +       return ret;
> +}
> +
> +static int hi655x_get_voltage(struct regulator_dev *rdev)
> +{
> +       unsigned int value = 0;
> +       struct hi655x_regulator *regulator = rdev_get_drvdata(rdev);
> +
> +       if (!regulator) {
> +               pr_err("get driver data error!\n");
> +               return -ENODEV;
> +       }
> +       struct hi655x_regulator_vset_regs *vset_regs = &regulator->vset_regs;
> +
> +       regmap_read(rdev->regmap, vset_regs->vset_reg, &value);
> +
> +       return regulator->vset_table[value];
> +}
> +
> +static int hi655x_set_voltage(struct regulator_dev *rdev,
> +                             int min_uV, int max_uV, unsigned *selector)
> +{
> +       int i = 0;
> +       int ret = 0;
> +       int vol = 0;
> +       struct hi655x_regulator *regulator = rdev_get_drvdata(rdev);
> +
> +       if (!regulator) {
> +               pr_err("get driver data error!\n");
> +               return -ENODEV;
> +       }
> +
> +       struct hi655x_regulator_vset_regs *vset_regs = &regulator->vset_regs;
> +
> +       /**
> +        * search the matched vol and get its index
> +        */
> +       for (i = 0; i < regulator->vol_numb; i++) {
> +               vol = regulator->vset_table[i];
> +               if ((vol >= min_uV) && (vol <= max_uV))
> +                       break;
> +       }
> +
> +       if (i == regulator->vol_numb)
> +               return -1;
> +
> +       regmap_update_bits(rdev->regmap, vset_regs->vset_reg,
> +                          regulator->vset_mask, i);
> +       *selector = i;
> +
> +       return ret;
> +}
> +
> +static unsigned int hi655x_map_mode(unsigned int mode)
> +{
> +       /* hi655x pmic on hi6220 SoC only support normal mode */
> +       if (mode == REGULATOR_MODE_NORMAL)
> +               return REGULATOR_MODE_NORMAL;
> +       else
> +               return -EINVAL;
> +}
> +
> +static int hi655x_set_mode(struct regulator_dev *rdev,
> +                          unsigned int mode)
> +
> +{
> +       if (mode == REGULATOR_MODE_NORMAL)
> +               return 0;
> +       else
> +               return -EINVAL;
> +}
> +
> +static struct regulator_ops hi655x_regulator_ops = {
> +       .is_enabled = hi655x_is_enabled,
> +       .enable = hi655x_enable,
> +       .disable = hi655x_disable,
> +       .list_voltage = regulator_list_voltage_table,
> +       .get_voltage = hi655x_get_voltage,
> +       .set_voltage = hi655x_set_voltage,
> +       .set_mode = hi655x_set_mode,
> +};
> +
> +static const struct of_device_id of_hi655x_regulator_match_tbl[] = {
> +       {
> +               .compatible = "hisilicon,hi655x-regulator-pmic",
> +       },
> +};
> +MODULE_DEVICE_TABLE(of, of_hi655x_regulator_match_tbl);
> +
> +/**
> + * get the hi655x specific data from dt node.
> + */
> +static void of_get_hi655x_ctr(struct hi655x_regulator *regulator,
> +                             struct device *dev, struct device_node *np)
> +{
> +       unsigned int *vset_table = NULL;
> +
> +       of_property_read_u32_array(np, "regulator-ctrl-regs",

device property API?

> +                                  (u32 *)&regulator->ctrl_regs, 0x3);
> +       of_property_read_u32(np, "regulator-ctrl-mask", &regulator->ctrl_mask);
> +       of_property_read_u32(np, "regulator-vset-regs",
> +                            (u32 *)&regulator->vset_regs);
> +       of_property_read_u32(np, "regulator-vset-mask", &regulator->vset_mask);
> +       of_property_read_u32(np, "regulator-n-vol", &regulator->vol_numb);
> +       of_property_read_u32(np, "regulator-off-on-delay",
> +                            &regulator->rdesc.off_on_delay);
> +
> +       vset_table = devm_kzalloc(dev, regulator->vol_numb * sizeof(int),
> +                                 GFP_KERNEL);
> +
> +       of_property_read_u32_array(np, "regulator-vset-table",
> +                                  vset_table,
> +                                  regulator->vol_numb);
> +       regulator->vset_table = vset_table;
> +       regulator->rdesc.volt_table = vset_table;
> +       regulator->rdesc.n_voltages = regulator->vol_numb;
> +}
> +
> +static int hi655x_regulator_probe(struct platform_device *pdev)
> +{
> +       int ret = 0;

Is it used?

make W=1

> +       struct hi655x_regulator *regulator;
> +       struct hi655x_pmic *pmic;
> +       struct regulator_init_data *init_data;
> +       struct regulator_config config = { };
> +       struct device_node *np = pdev->dev.of_node;
> +
> +       pmic = dev_get_drvdata(pdev->dev.parent);
> +       if (!pmic) {
> +               pr_err("no pmic in the regulator parent node\n");

pr_err -> dev_err. Here and in other places.

> +               return -ENODEV;
> +       }
> +
> +       regulator = devm_kzalloc(&pdev->dev, sizeof(*regulator), GFP_KERNEL);
> +       if (!regulator)
> +               return -ENOMEM;
> +       of_get_hi655x_ctr(regulator, &pdev->dev, np);
> +
> +       regulator->rdesc.name = dev_name(&pdev->dev);
> +       regulator->rdesc.type = REGULATOR_VOLTAGE;
> +       regulator->rdesc.owner = THIS_MODULE;
> +       regulator->rdesc.of_map_mode = hi655x_map_mode;
> +       regulator->rdesc.ops = &hi655x_regulator_ops;
> +       init_data = of_get_regulator_init_data(&pdev->dev, pdev->dev.of_node,
> +                                              &regulator->rdesc);
> +       if (!init_data) {
> +               pr_err("get init data from dts error!\n");
> +               return -EINVAL;
> +       }
> +       config.dev = &pdev->dev;
> +       config.init_data = init_data;
> +       config.driver_data = regulator;
> +       config.regmap = pmic->regmap;
> +
> +       regulator->regdev = devm_regulator_register(&pdev->dev,
> +                                                   &regulator->rdesc,
> +                                                   &config);
> +       if (IS_ERR(regulator->regdev)) {
> +               pr_err("register regulator to system error!\n");
> +               return PTR_ERR(regulator->regdev);
> +       }
> +
> +       platform_set_drvdata(pdev, regulator);
> +       return 0;
> +}
> +
> +static struct platform_driver hi655x_regulator_driver = {
> +       .driver = {
> +               .name   = "hi655x_regulator",
> +               .of_match_table = of_hi655x_regulator_match_tbl,
> +       },
> +       .probe  = hi655x_regulator_probe,
> +};
> +module_platform_driver(hi655x_regulator_driver);
> +
> +MODULE_AUTHOR("Chen Feng <puck.chen-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>");
> +MODULE_DESCRIPTION("Hisi hi655x regulator driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/regulator/hi655x-regulator.h b/include/linux/regulator/hi655x-regulator.h
> new file mode 100644
> index 0000000..4a32095
> --- /dev/null
> +++ b/include/linux/regulator/hi655x-regulator.h
> @@ -0,0 +1,63 @@
> +/*
> + * Device driver for regulators in HI6553 IC
> + *
> + * Copyright (c) 2015 Hisilicon.
> + *
> + * Fei Wang  <w.f-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> + * Chen Feng <puck.chen-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> + *
> + * this regulator's probe function will be called lots of times,,
> + * because of there are lots of regulator nodes in dtb.
> + * so,that's say, the driver must be inited before the regulator nodes
> + * registor to system.

Something with spelling is broken here.

> + *
> + * Makefile have proved my guess, please refor to the makefile.
> + * when the code is rebuild i hope we can build pmu sub_system.
> + * init order can not base on compile

And here.




-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/7] arm64: dts: Add mtcmos and pmic node for hi6220 HiKey board

[Chen Feng]

Add dts node for hi665x pmic and hi6220 mtcmos driver

Signed-off-by: Chen Feng <puck.chen-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
Signed-off-by: Fei Wang <w.f-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 210 ++++++++++++++++++++++++++++++
 1 file changed, 210 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
index 3f03380..5e0b8260 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -167,5 +167,215 @@
 			clocks = <&ao_ctrl 36>, <&ao_ctrl 36>;
 			clock-names = "uartclk", "apb_pclk";
 		};
+
+		mtcmos {
+			compatible = "hisilicon,hi6220-mtcmos-driver";
+			hisilicon,mtcmos-steady-us = <10>;
+			hisilicon,mtcmos-sc-on-base  = <0xf7800000>;
+			hisilicon,mtcmos-acpu-on-base = <0xf65a0000>;
+
+			g3d_vdd: regulator@a1{
+				regulator-name = "G3D_PD_VDD";
+				regulator-compatible = "mtcmos1";
+				hisilicon,ctrl-regs = <0x830 0x834 0x83c>;
+				hisilicon,ctrl-data = <1 0x1>;
+			};
+
+			soc_med: regulator@a2{
+				regulator-name = "SOC_MED";
+				regulator-compatible = "mtcmos2";
+				hisilicon,ctrl-regs = <0x830 0x834 0x83c>;
+				hisilicon,ctrl-data = <2 0x1>;
+			};
+		};
+	};
+
+	pmic: pmic@f8000000 {
+		compatible = "hisilicon,hi655x-pmic-driver";
+		reg = <0x0 0xf8000000 0x0 0x1000>;
+		#interrupt-cells = <2>;
+		interrupt-controller;
+		pmic-gpios = <&gpio_pmu_irq_n>;
+		status = "okay";
+		ldo2: regulator@a21 {
+			compatible = "hisilicon,hi655x-regulator-pmic";
+			regulator-name = "ldo2";
+			regulator-min-microvolt = <2500000>;
+			regulator-max-microvolt = <3200000>;
+			regulator-valid-modes-mask = <0x02>;
+			regulator-initial-mode = <0x02>;
+			regulator-off-on-delay = <120>;
+			regulator-ctrl-regs = <0x029 0x02a 0x02b>;
+			regulator-ctrl-mask = <0x1>;
+			regulator-vset-regs = <0x072>;
+			regulator-vset-mask = <0x3>;
+			regulator-n-vol = <8>;
+			regulator-vset-table  = <2500000>,<2600000>,
+						<2700000>,<2800000>,
+						<2900000>,<3000000>,
+						<3100000>,<3200000>;
+		};
+		ldo7: regulator@a26 {
+			compatible = "hisilicon,hi655x-regulator-pmic";
+			regulator-name = "ldo7";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-valid-modes-mask = <0x0a>;
+			regulator-initial-mode = <0x02>;
+			regulator-off-on-delay = <120>;
+			regulator-ctrl-regs = <0x029 0x02a 0x02b>;
+			regulator-ctrl-mask = <0x6>;
+			regulator-vset-regs = <0x078>;
+			regulator-vset-mask = <0x3>;
+			regulator-n-vol = <8>;
+			regulator-vset-table  = <1800000>,<1850000>,
+						<2850000>,<2900000>,
+						<3000000>,<3100000>,
+						<3200000>,<3300000>;
+		};
+		ldo10: regulator@a29 {
+			compatible = "hisilicon,hi655x-regulator-pmic";
+			regulator-name = "ldo10";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <3000000>;
+			regulator-valid-modes-mask = <0x0a>;
+			regulator-initial-mode = <0x02>;
+			regulator-off-on-delay = <360>;
+			regulator-ctrl-regs = <0x02c 0x02d 0x02e>;
+			regulator-ctrl-mask = <0x1>;
+			regulator-vset-regs = <0x07b>;
+			regulator-vset-mask = <0x3>;
+			regulator-n-vol = <8>;
+			regulator-vset-table  = <1800000>,<1850000>,
+						<1900000>,<2750000>,
+						<2800000>,<2850000>,
+						<2900000>,<3000000>;
+		};
+		ldo13: regulator@a32 {
+			compatible = "hisilicon,hi655x-regulator-pmic";
+			regulator-name = "ldo13";
+			regulator-min-microvolt = <1600000>;
+			regulator-max-microvolt = <1950000>;
+			regulator-initial-mode = <0x02>;
+			regulator-off-on-delay = <120>;
+			regulator-ctrl-regs = <0x02c 0x02d 0x02e>;
+			regulator-ctrl-mask = <0x4>;
+			regulator-vset-regs = <0x07e>;
+			regulator-vset-mask = <0x3>;
+			regulator-n-vol = <8>;
+			regulator-vset-table  = <1600000>,<1650000>,
+						<1700000>,<1750000>,
+						<1800000>,<1850000>,
+						<1900000>,<1950000>;
+		};
+		ldo14: regulator@a33 {
+			compatible = "hisilicon,hi655x-regulator-pmic";
+			regulator-name = "ldo14";
+			regulator-min-microvolt = <2500000>;
+			regulator-max-microvolt = <3200000>;
+			regulator-initial-mode = <0x02>;
+			regulator-off-on-delay = <120>;
+			regulator-ctrl-regs = <0x02c 0x02d 0x02e>;
+			regulator-ctrl-mask = <0x5>;
+			regulator-vset-regs = <0x07f>;
+			regulator-vset-mask = <0x3>;
+			regulator-n-vol = <8>;
+			regulator-vset-table  = <2500000>,<2600000>,
+						<2700000>,<2800000>,
+						<2900000>,<3000000>,
+						<3100000>,<3200000>;
+		};
+		ldo15: regulator@a34 {
+			compatible = "hisilicon,hi655x-regulator-pmic";
+			regulator-name = "ldo15";
+			regulator-min-microvolt = <1600000>;
+			regulator-max-microvolt = <1950000>;
+			regulator-boot-on;
+			regulator-always-on;
+			regulator-initial-mode = <0x02>;
+			regulator-off-on-delay = <120>;
+			regulator-ctrl-regs = <0x02c 0x02d 0x02e>;
+			regulator-ctrl-mask = <0x6>;
+			regulator-vset-regs = <0x080>;
+			regulator-vset-mask = <0x3>;
+			regulator-n-vol = <8>;
+			regulator-vset-table  = <1600000>,<1650000>,
+						<1700000>,<1750000>,
+						<1800000>,<1850000>,
+						<1900000>,<1950000>;
+		};
+		ldo17: regulator@a36 {
+			compatible = "hisilicon,hi655x-regulator-pmic";
+			regulator-name = "ldo17";
+			regulator-min-microvolt = <2500000>;
+			regulator-max-microvolt = <3200000>;
+			regulator-initial-mode = <0x02>;
+			regulator-off-on-delay = <120>;
+			regulator-ctrl-regs = <0x02f 0x030 0x031>;
+			regulator-ctrl-mask = <0x0>;
+			regulator-vset-regs = <0x082>;
+			regulator-vset-mask = <0x3>;
+			regulator-n-vol = <8>;
+			regulator-vset-table  = <2500000>,<2600000>,
+						<2700000>,<2800000>,
+						<2900000>,<3000000>,
+						<3100000>,<3200000>;
+		};
+		ldo19: regulator@a38 {
+			compatible = "hisilicon,hi655x-regulator-pmic";
+			regulator-name = "ldo19";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <3000000>;
+			regulator-initial-mode = <0x02>;
+			regulator-off-on-delay = <360>;
+			regulator-ctrl-regs = <0x02f 0x030 0x031>;
+			regulator-ctrl-mask = <0x2>;
+			regulator-vset-regs = <0x084>;
+			regulator-vset-mask = <0x3>;
+			regulator-n-vol = <8>;
+			regulator-vset-table  = <1800000>,<1850000>,
+						<1900000>,<2750000>,
+						<2800000>,<2850000>,
+						<2900000>,<3000000>;
+		};
+		ldo21: regulator@a40 {
+			compatible = "hisilicon,hi655x-regulator-pmic";
+			regulator-name = "ldo21";
+			regulator-min-microvolt = <1650000>;
+			regulator-max-microvolt = <2000000>;
+			regulator-always-on;
+			regulator-valid-modes-mask = <0x02>;
+			regulator-initial-mode = <0x02>;
+			regulator-off-on-delay = <120>;
+			regulator-ctrl-regs = <0x02f 0x030 0x031>;
+			regulator-ctrl-mask = <0x4>;
+			regulator-vset-regs = <0x086>;
+			regulator-vset-mask = <0x3>;
+			regulator-n-vol = <8>;
+			regulator-vset-table  = <1650000>,<1700000>,
+						<1750000>,<1800000>,
+						<1850000>,<1900000>,
+						<1950000>,<2000000>;
+		};
+		ldo22: regulator@a41 {
+			compatible = "hisilicon,hi655x-regulator-pmic";
+			regulator-name = "ldo22";
+			regulator-min-microvolt = <900000>;
+			regulator-max-microvolt = <1200000>;
+			regulator-boot-on;
+			regulator-always-on;
+			regulator-valid-modes-mask = <0x02>;
+			regulator-initial-mode = <0x02>;
+			regulator-off-on-delay = <120>;
+			regulator-ctrl-regs = <0x02f 0x030 0x031>;
+			regulator-ctrl-mask = <0x5>;
+			regulator-vset-regs = <0x087>;
+			regulator-vset-mask = <0x3>;
+			regulator-n-vol = <8>;
+			regulator-vset-table  = <900000>,<1000000>,
+						<1050000>,<1100000>,
+						<1150000>,<1175000>,
+						<1185000>,<1200000>;
+		};
 	};
 };
-- 
1.9.1