[PATCH 0/4] imx8mm-evk: Enable CPU freq and voltage switching

[PATCH 0/4] imx8mm-evk: Enable CPU freq and voltage switching

[Leonard Crestez]

This was sent as a series but patches can be safely applied in any order.
Without the pll_1416x bugfix cpufreq will refuse to switch.

The patch adding PMIC to DT is mostly based on our internal tree and I'm not
sure all properties are actually required. I tested that cpufreq switches
voltages safely and also SVNS mode across reset/shutdown seems to work
as expected.

There is also a naming disagreement between datasheet/schematics and
regulator driver: between BD71837 and BD71847 the BUCK3/4 regulators
were removed but datasheet and board schematics kept the names for
BUCK5/6/7/8. The driver however renumbered 5/6/7/8 to 3/4/5/6.

More concretely:

    bd71847_regulators[2].name = 'buck3'
    bd71847_regulators[2].vsel_reg = BD718XX_REG_1ST_NODVS_BUCK_VOLT
    BD718XX_REG_1ST_NODVS_BUCK_VOLT = 0x14

    bd71837_regulators[4].name = 'buck5'
    bd71837_regulators[4].vsel_reg = BD718XX_REG_1ST_NODVS_BUCK_VOLT
    BD718XX_REG_1ST_NODVS_BUCK_VOLT = 0x14

    bd71837_regulators[2].name = 'buck3'
    bd71837_regulators[2].vsel_reg = BD71837_REG_BUCK3_VOLT_RUN
    BD71837_REG_BUCK3_VOLT_RUN = 0x12

For both versions the datasheet refers to 0x14 as BUCK5_VOLT5 rather
than "1ST_NODVS_BUCK".

Using the names from DT bindings works fine.

Leonard Crestez (4):
  clk: imx: Fix PLL_1416X not rounding rates
  arm64: dts: imx8mm: Add cpufreq properties
  arm64: defconfig: Enable ROHM_BD718XX PMIC for imx8mm-evk
  arm64: dts: imx8mm-evk: Add BD71847 PMIC

 arch/arm64/boot/dts/freescale/imx8mm-evk.dts | 145 +++++++++++++++++++
 arch/arm64/boot/dts/freescale/imx8mm.dtsi    |  30 ++++
 arch/arm64/configs/defconfig                 |   2 +
 drivers/clk/imx/clk-pll14xx.c                |   2 +-
 4 files changed, 178 insertions(+), 1 deletion(-)

-- 
2.17.1

[PATCH 2/4] arm64: dts: imx8mm: Add cpufreq properties

[Leonard Crestez]

This is very similar to imx8mq cpufreq-dt support.

Operating points are from datasheet:
https://www.nxp.com/docs/en/data-sheet/IMX8MMCEC.pdf

Higher opps were omitted (just like imx8mq) because it requires checking
speed grade from OCOTP fuses.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 30 +++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index de3498c2dd44..6b407a94c06e 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -46,43 +46,73 @@
 
 		A53_0: cpu@0 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a53";
 			reg = <0x0>;
+			clock-latency = <61036>; /* two CLK32 periods */
+			clocks = <&clk IMX8MM_CLK_ARM>;
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			operating-points-v2 = <&a53_opp_table>;
 		};
 
 		A53_1: cpu@1 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a53";
 			reg = <0x1>;
+			clock-latency = <61036>; /* two CLK32 periods */
+			clocks = <&clk IMX8MM_CLK_ARM>;
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			operating-points-v2 = <&a53_opp_table>;
 		};
 
 		A53_2: cpu@2 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a53";
 			reg = <0x2>;
+			clock-latency = <61036>; /* two CLK32 periods */
+			clocks = <&clk IMX8MM_CLK_ARM>;
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			operating-points-v2 = <&a53_opp_table>;
 		};
 
 		A53_3: cpu@3 {
 			device_type = "cpu";
 			compatible = "arm,cortex-a53";
 			reg = <0x3>;
+			clock-latency = <61036>; /* two CLK32 periods */
+			clocks = <&clk IMX8MM_CLK_ARM>;
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			operating-points-v2 = <&a53_opp_table>;
 		};
 
 		A53_L2: l2-cache0 {
 			compatible = "cache";
 		};
 	};
 
+	a53_opp_table: opp-table {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp-1200000000 {
+			opp-hz = /bits/ 64 <1200000000>;
+			opp-microvolt = <850000>;
+			clock-latency-ns = <150000>;
+		};
+
+		opp-1600000000 {
+			opp-hz = /bits/ 64 <1600000000>;
+			opp-microvolt = <900000>;
+			clock-latency-ns = <150000>;
+			opp-suspend;
+		};
+	};
+
 	memory@40000000 {
 		device_type = "memory";
 		reg = <0x0 0x40000000 0 0x80000000>;
 	};
 
-- 
2.17.1

Re: [PATCH 2/4] arm64: dts: imx8mm: Add cpufreq properties

[Shawn Guo]

On Fri, Apr 12, 2019 at 02:10:03PM +0000, Leonard Crestez wrote:
> This is very similar to imx8mq cpufreq-dt support.
> 
> Operating points are from datasheet:
> https://www.nxp.com/docs/en/data-sheet/IMX8MMCEC.pdf
> 
> Higher opps were omitted (just like imx8mq) because it requires checking
> speed grade from OCOTP fuses.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>

Applied, thanks.

[PATCH 1/4] clk: imx: Fix PLL_1416X not rounding rates

[Leonard Crestez]

Code which initializes the "clk_init_data.ops" checks pll->rate_table
before that field is ever assigned to so it always picks
"clk_pll1416x_min_ops".

This breaks dynamic rate rounding for features such as cpufreq.

Fix by checking pll_clk->rate_table instead, here pll_clk refers to
the constant initialization data coming from per-soc clk driver.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/clk/imx/clk-pll14xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index 1acfa3e3cfb4..113d71042199 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -360,11 +360,11 @@ struct clk *imx_clk_pll14xx(const char *name, const char *parent_name,
 	init.parent_names = &parent_name;
 	init.num_parents = 1;
 
 	switch (pll_clk->type) {
 	case PLL_1416X:
-		if (!pll->rate_table)
+		if (!pll_clk->rate_table)
 			init.ops = &clk_pll1416x_min_ops;
 		else
 			init.ops = &clk_pll1416x_ops;
 		break;
 	case PLL_1443X:
-- 
2.17.1

Re: [PATCH 1/4] clk: imx: Fix PLL_1416X not rounding rates

[Stephen Boyd]

Quoting Leonard Crestez (2019-04-12 07:10:03)
> Code which initializes the "clk_init_data.ops" checks pll->rate_table
> before that field is ever assigned to so it always picks
> "clk_pll1416x_min_ops".
> 
> This breaks dynamic rate rounding for features such as cpufreq.
> 
> Fix by checking pll_clk->rate_table instead, here pll_clk refers to
> the constant initialization data coming from per-soc clk driver.
> 

Any Fixes tag here?

> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  drivers/clk/imx/clk-pll14xx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Re: [PATCH 1/4] clk: imx: Fix PLL_1416X not rounding rates

[Leonard Crestez]

On 4/12/19 7:43 PM, Stephen Boyd wrote:
> Quoting Leonard Crestez (2019-04-12 07:10:03)
>> Code which initializes the "clk_init_data.ops" checks pll->rate_table
>> before that field is ever assigned to so it always picks
>> "clk_pll1416x_min_ops".
>>
>> This breaks dynamic rate rounding for features such as cpufreq.
>>
>> Fix by checking pll_clk->rate_table instead, here pll_clk refers to
>> the constant initialization data coming from per-soc clk driver.
> 
> Any Fixes tag here?

Fixes: 8646d4dcc7fb ("clk: imx: Add PLLs driver for imx8mm soc")

That's the only commit on this file, it's only for a very new SOC.

--
Regards,
Leonard

Re: [PATCH 1/4] clk: imx: Fix PLL_1416X not rounding rates

[Stephen Boyd]

Quoting Leonard Crestez (2019-04-12 09:51:37)
> On 4/12/19 7:43 PM, Stephen Boyd wrote:
> > Quoting Leonard Crestez (2019-04-12 07:10:03)
> >> Code which initializes the "clk_init_data.ops" checks pll->rate_table
> >> before that field is ever assigned to so it always picks
> >> "clk_pll1416x_min_ops".
> >>
> >> This breaks dynamic rate rounding for features such as cpufreq.
> >>
> >> Fix by checking pll_clk->rate_table instead, here pll_clk refers to
> >> the constant initialization data coming from per-soc clk driver.
> > 
> > Any Fixes tag here?
> 
> Fixes: 8646d4dcc7fb ("clk: imx: Add PLLs driver for imx8mm soc")
> 
> That's the only commit on this file, it's only for a very new SOC.
> 

Ok. I take it I should push this into clk-fixes so it can fix badness in
the v5.1 series?

Re: [PATCH 1/4] clk: imx: Fix PLL_1416X not rounding rates

[Leonard Crestez]

On 4/12/2019 9:34 PM, Stephen Boyd wrote:
> Quoting Leonard Crestez (2019-04-12 09:51:37)
>> On 4/12/19 7:43 PM, Stephen Boyd wrote:
>>> Quoting Leonard Crestez (2019-04-12 07:10:03)
>>>> Code which initializes the "clk_init_data.ops" checks pll->rate_table
>>>> before that field is ever assigned to so it always picks
>>>> "clk_pll1416x_min_ops".
>>>>
>>>> This breaks dynamic rate rounding for features such as cpufreq.
>>>>
>>>> Fix by checking pll_clk->rate_table instead, here pll_clk refers to
>>>> the constant initialization data coming from per-soc clk driver.
>>>
>>> Any Fixes tag here?
>>
>> Fixes: 8646d4dcc7fb ("clk: imx: Add PLLs driver for imx8mm soc")
>>
>> That's the only commit on this file, it's only for a very new SOC.
> 
> Ok. I take it I should push this into clk-fixes so it can fix badness in
> the v5.1 series?

Since this a bugfix and the original patch is in v5.1 the answer would 
be yes.

It would make a lot of sense to accept this separately from the rest of 
the series.

The original patch is not in any release so there's no need for stuff 
like CC: stable.

Re: [PATCH 1/4] clk: imx: Fix PLL_1416X not rounding rates

[Stephen Boyd]

Quoting Leonard Crestez (2019-04-12 07:10:03)
> Code which initializes the "clk_init_data.ops" checks pll->rate_table
> before that field is ever assigned to so it always picks
> "clk_pll1416x_min_ops".
> 
> This breaks dynamic rate rounding for features such as cpufreq.
> 
> Fix by checking pll_clk->rate_table instead, here pll_clk refers to
> the constant initialization data coming from per-soc clk driver.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---

Applied to clk-fixes

[PATCH 3/4] arm64: defconfig: Enable ROHM_BD718XX PMIC for imx8mm-evk

[Leonard Crestez]

Enable mfd and regulator driver for PMIC found on imx8mm-evk boards

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index b1850a7b2426..483d0e251f37 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -438,12 +438,14 @@ CONFIG_MFD_HI6421_PMIC=y
 CONFIG_MFD_HI655X_PMIC=y
 CONFIG_MFD_MAX77620=y
 CONFIG_MFD_SPMI_PMIC=y
 CONFIG_MFD_RK808=y
 CONFIG_MFD_SEC_CORE=y
+CONFIG_MFD_ROHM_BD718XX=y
 CONFIG_REGULATOR_FIXED_VOLTAGE=y
 CONFIG_REGULATOR_AXP20X=y
+CONFIG_REGULATOR_BD718XX=y
 CONFIG_REGULATOR_BD9571MWV=y
 CONFIG_REGULATOR_FAN53555=y
 CONFIG_REGULATOR_GPIO=y
 CONFIG_REGULATOR_HI6421V530=y
 CONFIG_REGULATOR_HI655X=y
-- 
2.17.1

Re: [PATCH 3/4] arm64: defconfig: Enable ROHM_BD718XX PMIC for imx8mm-evk

[Shawn Guo]

On Fri, Apr 12, 2019 at 02:10:04PM +0000, Leonard Crestez wrote:
> Enable mfd and regulator driver for PMIC found on imx8mm-evk boards
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>

Applied, thanks.

[PATCH 4/4] arm64: dts: imx8mm-evk: Add BD71847 PMIC

[Leonard Crestez]

The BUCK2 regulator is used for cpufreq voltage control, otherwise
configuration is mostly static.

This uses the newly-implemented rohm,reset-snvs-powered property to
properly handle the SNVS state of imx8mm.

Between BD71837 and BD71847 the BUCK3/4 regulators were removed but
datasheet and board schematics kept the names for BUCK5/6/7/8. The
driver however renumbered 5/6/7/8 to 3/4/5/6. Use the names from DT
bindings and add comments to signal this.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mm-evk.dts | 145 +++++++++++++++++++
 1 file changed, 145 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm-evk.dts b/arch/arm64/boot/dts/freescale/imx8mm-evk.dts
index 2d5d89475b76..098ccaefd169 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mm-evk.dts
@@ -37,10 +37,14 @@
 		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
 		enable-active-high;
 	};
 };
 
+&A53_0 {
+	cpu-supply = <&buck2_reg>;
+};
+
 &fec1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_fec1>;
 	phy-mode = "rgmii-id";
 	phy-handle = <&ethphy0>;
@@ -93,10 +97,138 @@
 	pinctrl-0 = <&pinctrl_wdog>;
 	fsl,ext-reset-output;
 	status = "okay";
 };
 
+&i2c1 {
+	clock-frequency = <400000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c1>;
+	status = "okay";
+
+	pmic@4b {
+		reg = <0x4b>;
+		compatible = "rohm,bd71847";
+		pinctrl-0 = <&pinctrl_pmic>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <3 GPIO_ACTIVE_LOW>;
+		rohm,reset-snvs-powered;
+
+		regulators {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			buck1_reg: regulator@0 {
+				reg = <0>;
+				regulator-compatible = "BUCK1";
+				regulator-min-microvolt = <700000>;
+				regulator-max-microvolt = <1300000>;
+				regulator-boot-on;
+				regulator-always-on;
+				regulator-ramp-delay = <1250>;
+			};
+
+			buck2_reg: regulator@1 {
+				reg = <1>;
+				regulator-compatible = "BUCK2";
+				regulator-min-microvolt = <700000>;
+				regulator-max-microvolt = <1300000>;
+				regulator-boot-on;
+				regulator-always-on;
+				regulator-ramp-delay = <1250>;
+				rohm,dvs-run-voltage = <1000000>;
+				rohm,dvs-idle-voltage = <900000>;
+			};
+
+			buck3_reg: regulator@2 {
+				// BUCK5 in datasheet
+				reg = <2>;
+				regulator-compatible = "BUCK3";
+				regulator-min-microvolt = <700000>;
+				regulator-max-microvolt = <1350000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			buck4_reg: regulator@3 {
+				// BUCK6 in datasheet
+				reg = <3>;
+				regulator-compatible = "BUCK4";
+				regulator-min-microvolt = <3000000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			buck5_reg: regulator@4 {
+				// BUCK7 in datasheet
+				reg = <4>;
+				regulator-compatible = "BUCK5";
+				regulator-min-microvolt = <1605000>;
+				regulator-max-microvolt = <1995000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			buck6_reg: regulator@5 {
+				// BUCK8 in datasheet
+				reg = <5>;
+				regulator-compatible = "BUCK6";
+				regulator-min-microvolt = <800000>;
+				regulator-max-microvolt = <1400000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo1_reg: regulator@6 {
+				reg = <6>;
+				regulator-compatible = "LDO1";
+				regulator-min-microvolt = <3000000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo2_reg: regulator@7 {
+				reg = <7>;
+				regulator-compatible = "LDO2";
+				regulator-min-microvolt = <900000>;
+				regulator-max-microvolt = <900000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo3_reg: regulator@8 {
+				reg = <8>;
+				regulator-compatible = "LDO3";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo4_reg: regulator@9 {
+				reg = <9>;
+				regulator-compatible = "LDO4";
+				regulator-min-microvolt = <900000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo6_reg: regulator@11 {
+				reg = <11>;
+				regulator-compatible = "LDO6";
+				regulator-min-microvolt = <900000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+		};
+	};
+};
+
 &iomuxc {
 	pinctrl-names = "default";
 
 	pinctrl_fec1: fec1grp {
 		fsl,pins = <
@@ -122,10 +254,23 @@
 		fsl,pins = <
 			MX8MM_IOMUXC_NAND_READY_B_GPIO3_IO16	0x19
 		>;
 	};
 
+	pinctrl_i2c1: i2c1grp {
+		fsl,pins = <
+			MX8MM_IOMUXC_I2C1_SCL_I2C1_SCL			0x400001c3
+			MX8MM_IOMUXC_I2C1_SDA_I2C1_SDA			0x400001c3
+		>;
+	};
+
+	pinctrl_pmic: pmicirq {
+		fsl,pins = <
+			MX8MM_IOMUXC_GPIO1_IO03_GPIO1_IO3		0x41
+		>;
+	};
+
 	pinctrl_reg_usdhc2_vmmc: regusdhc2vmmc {
 		fsl,pins = <
 			MX8MM_IOMUXC_SD2_RESET_B_GPIO2_IO19	0x41
 		>;
 	};
-- 
2.17.1

Re: [PATCH 4/4] arm64: dts: imx8mm-evk: Add BD71847 PMIC

[Vaittinen, Matti]

On Fri, 2019-04-12 at 14:10 +0000, Leonard Crestez wrote:
> The BUCK2 regulator is used for cpufreq voltage control, otherwise
> configuration is mostly static.
> 
> This uses the newly-implemented rohm,reset-snvs-powered property to
> properly handle the SNVS state of imx8mm.
> 
> Between BD71837 and BD71847 the BUCK3/4 regulators were removed but
> datasheet and board schematics kept the names for BUCK5/6/7/8. The
> driver however renumbered 5/6/7/8 to 3/4/5/6. Use the names from DT
> bindings and add comments to signal this.

Yep. I had hard time when deciding the naming for bucks. But I found
leaving the gap in buck names confusing, it makes only sense if BD71847
user also knows the BD71837. Also, I had only seen the draft version of
BD71847 data-sheet (draft derived from BD71837 data-sheet) back then. I
thought the naming of bucks would be changed in final version...

> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Acked-By: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>

Br,
    Matti Vaittinen

Re: [PATCH 4/4] arm64: dts: imx8mm-evk: Add BD71847 PMIC

[Shawn Guo]

On Fri, Apr 12, 2019 at 02:10:05PM +0000, Leonard Crestez wrote:
> The BUCK2 regulator is used for cpufreq voltage control, otherwise
> configuration is mostly static.
> 
> This uses the newly-implemented rohm,reset-snvs-powered property to
> properly handle the SNVS state of imx8mm.
> 
> Between BD71837 and BD71847 the BUCK3/4 regulators were removed but
> datasheet and board schematics kept the names for BUCK5/6/7/8. The
> driver however renumbered 5/6/7/8 to 3/4/5/6. Use the names from DT
> bindings and add comments to signal this.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mm-evk.dts | 145 +++++++++++++++++++
>  1 file changed, 145 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-evk.dts b/arch/arm64/boot/dts/freescale/imx8mm-evk.dts
> index 2d5d89475b76..098ccaefd169 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-evk.dts
> @@ -37,10 +37,14 @@
>  		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
>  		enable-active-high;
>  	};
>  };
>  
> +&A53_0 {
> +	cpu-supply = <&buck2_reg>;
> +};
> +
>  &fec1 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_fec1>;
>  	phy-mode = "rgmii-id";
>  	phy-handle = <&ethphy0>;
> @@ -93,10 +97,138 @@
>  	pinctrl-0 = <&pinctrl_wdog>;
>  	fsl,ext-reset-output;
>  	status = "okay";
>  };
>  
> +&i2c1 {
> +	clock-frequency = <400000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c1>;
> +	status = "okay";
> +
> +	pmic@4b {
> +		reg = <0x4b>;
> +		compatible = "rohm,bd71847";

We usually start properties with 'compatible' than anything else.  So
please move 'reg' afterwards.

> +		pinctrl-0 = <&pinctrl_pmic>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <3 GPIO_ACTIVE_LOW>;
> +		rohm,reset-snvs-powered;
> +
> +		regulators {
> +			#address-cells = <1>;
> +			#size-cells = <0>;

Looking at bindings doc, I'm not sure if we want the artificial address
cell (unit address and 'reg' property).  I think node names like
regulator-xxx should just work?

Shawn

> +
> +			buck1_reg: regulator@0 {
> +				reg = <0>;
> +				regulator-compatible = "BUCK1";
> +				regulator-min-microvolt = <700000>;
> +				regulator-max-microvolt = <1300000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +				regulator-ramp-delay = <1250>;
> +			};
> +
> +			buck2_reg: regulator@1 {
> +				reg = <1>;
> +				regulator-compatible = "BUCK2";
> +				regulator-min-microvolt = <700000>;
> +				regulator-max-microvolt = <1300000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +				regulator-ramp-delay = <1250>;
> +				rohm,dvs-run-voltage = <1000000>;
> +				rohm,dvs-idle-voltage = <900000>;
> +			};
> +
> +			buck3_reg: regulator@2 {
> +				// BUCK5 in datasheet
> +				reg = <2>;
> +				regulator-compatible = "BUCK3";
> +				regulator-min-microvolt = <700000>;
> +				regulator-max-microvolt = <1350000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			buck4_reg: regulator@3 {
> +				// BUCK6 in datasheet
> +				reg = <3>;
> +				regulator-compatible = "BUCK4";
> +				regulator-min-microvolt = <3000000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			buck5_reg: regulator@4 {
> +				// BUCK7 in datasheet
> +				reg = <4>;
> +				regulator-compatible = "BUCK5";
> +				regulator-min-microvolt = <1605000>;
> +				regulator-max-microvolt = <1995000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			buck6_reg: regulator@5 {
> +				// BUCK8 in datasheet
> +				reg = <5>;
> +				regulator-compatible = "BUCK6";
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <1400000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo1_reg: regulator@6 {
> +				reg = <6>;
> +				regulator-compatible = "LDO1";
> +				regulator-min-microvolt = <3000000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo2_reg: regulator@7 {
> +				reg = <7>;
> +				regulator-compatible = "LDO2";
> +				regulator-min-microvolt = <900000>;
> +				regulator-max-microvolt = <900000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo3_reg: regulator@8 {
> +				reg = <8>;
> +				regulator-compatible = "LDO3";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo4_reg: regulator@9 {
> +				reg = <9>;
> +				regulator-compatible = "LDO4";
> +				regulator-min-microvolt = <900000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +
> +			ldo6_reg: regulator@11 {
> +				reg = <11>;
> +				regulator-compatible = "LDO6";
> +				regulator-min-microvolt = <900000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-boot-on;
> +				regulator-always-on;
> +			};
> +		};
> +	};
> +};
> +
>  &iomuxc {
>  	pinctrl-names = "default";
>  
>  	pinctrl_fec1: fec1grp {
>  		fsl,pins = <
> @@ -122,10 +254,23 @@
>  		fsl,pins = <
>  			MX8MM_IOMUXC_NAND_READY_B_GPIO3_IO16	0x19
>  		>;
>  	};
>  
> +	pinctrl_i2c1: i2c1grp {
> +		fsl,pins = <
> +			MX8MM_IOMUXC_I2C1_SCL_I2C1_SCL			0x400001c3
> +			MX8MM_IOMUXC_I2C1_SDA_I2C1_SDA			0x400001c3
> +		>;
> +	};
> +
> +	pinctrl_pmic: pmicirq {
> +		fsl,pins = <
> +			MX8MM_IOMUXC_GPIO1_IO03_GPIO1_IO3		0x41
> +		>;
> +	};
> +
>  	pinctrl_reg_usdhc2_vmmc: regusdhc2vmmc {
>  		fsl,pins = <
>  			MX8MM_IOMUXC_SD2_RESET_B_GPIO2_IO19	0x41
>  		>;
>  	};
> -- 
> 2.17.1
>