[PATCH v2 0/6] LG Optimus Black (P970) codename sniper support and lp872x improvements

[PATCH v2 0/6] LG Optimus Black (P970) codename sniper support and lp872x improvements

[Paul Kocialkowski]

Changes since v1:
* Remove lp872x patches that were accepted already from the series
* Explain why removing the invalid DVS GPIO warning is enough
* Update copyright notice on omap3-sniper.dts
* Rename the devicetree property for the enable GPIO to enable-gpios
* Rename function to enable GPIO to lp872x_hw_enable
* Add enable delays after requesting the enable GPIO
* Use IOPAD macros for pinmux description
* Mux the dp3t uart selection pins to gpio
* Restrict not-reset gpio controllers to only avoid reset when needed

This series introduces support for the LG Optimus Black, as described in the
patch adding devicetree support for the device.

In order to power the external mmc (mmc1), the lp872x regulator is used.
Its code had to be improved a bit to work on the device.

Note that the patch adding devicetree support for the device requires the
changes applied to the lp872x regulator, so all this is sent in one series
despite the fact they're different parts of the code.

[PATCH v2 1/4] regulator: lp872x: Remove warning about invalid DVS GPIO

[Paul Kocialkowski]

Some devices don't hook the DVS pin to a GPIO but to ground or VCC.
In those cases, it is not a problem to have no DVS GPIO provided, as the current
code will already switch to software-only DVS selection:

When the DVS GPIO is invalid, lp872x_init_dvs jumps to the set_default_dvs_mode
label, which instructs the chip not to use the DVS pin at all and do it all in
software instead (by clearing the LP8720_EXT_DVS_M bit in the
LP872X_GENERAL_CFG register).

That is reflected later in the code, when setting the bucks (the DVS pin only
applies to the bucks) by checking for the LP8720_EXT_DVS_M bit on the
LP872X_GENERAL_CFG register (in lp872x_select_buck_vout_addr) to decide whether
to use software or hardware DVS selection.

Thus, there is no need to print a warning when the DVS GPIO is invalid.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 drivers/regulator/lp872x.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/regulator/lp872x.c b/drivers/regulator/lp872x.c
index 19d7584..21c49d8 100644
--- a/drivers/regulator/lp872x.c
+++ b/drivers/regulator/lp872x.c
@@ -738,10 +738,8 @@ static int lp872x_init_dvs(struct lp872x *lp)
 		goto set_default_dvs_mode;
 
 	gpio = dvs->gpio;
-	if (!gpio_is_valid(gpio)) {
-		dev_warn(lp->dev, "invalid gpio: %d\n", gpio);
+	if (!gpio_is_valid(gpio))
 		goto set_default_dvs_mode;
-	}
 
 	pinstate = dvs->init_state;
 	ret = devm_gpio_request_one(lp->dev, gpio, pinstate, "LP872X DVS");
-- 
1.9.1

[PATCH v2 2/4] regulator: lp872x: Add enable GPIO pin support

[Paul Kocialkowski]

LP872x regulators are made active via the EN pin, which might be hooked to a
GPIO. This adds support for driving the GPIO high when the driver is in use.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 .../devicetree/bindings/regulator/lp872x.txt       |  1 +
 drivers/regulator/lp872x.c                         | 34 ++++++++++++++++++++++
 include/linux/regulator/lp872x.h                   |  5 ++++
 3 files changed, 40 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/lp872x.txt b/Documentation/devicetree/bindings/regulator/lp872x.txt
index 7818318..ca58a68 100644
--- a/Documentation/devicetree/bindings/regulator/lp872x.txt
+++ b/Documentation/devicetree/bindings/regulator/lp872x.txt
@@ -28,6 +28,7 @@ Optional properties:
   - ti,dvs-gpio: GPIO specifier for external DVS pin control of LP872x devices.
   - ti,dvs-vsel: DVS selector. 0 = SEL_V1, 1 = SEL_V2.
   - ti,dvs-state: initial DVS pin state. 0 = DVS_LOW, 1 = DVS_HIGH.
+  - enable-gpios: GPIO specifier for EN pin control of LP872x devices.
 
   Sub nodes for regulator_init_data
     LP8720 has maximum 6 nodes. (child name: ldo1 ~ 5 and buck)
diff --git a/drivers/regulator/lp872x.c b/drivers/regulator/lp872x.c
index 21c49d8..3899211 100644
--- a/drivers/regulator/lp872x.c
+++ b/drivers/regulator/lp872x.c
@@ -15,6 +15,7 @@
 #include <linux/regmap.h>
 #include <linux/err.h>
 #include <linux/gpio.h>
+#include <linux/delay.h>
 #include <linux/regulator/lp872x.h>
 #include <linux/regulator/driver.h>
 #include <linux/platform_device.h>
@@ -757,6 +758,33 @@ set_default_dvs_mode:
 				default_dvs_mode[lp->chipid]);
 }
 
+static int lp872x_hw_enable(struct lp872x *lp)
+{
+	int ret, gpio;
+
+	if (!lp->pdata)
+		return -EINVAL;
+
+	gpio = lp->pdata->enable_gpio;
+	if (!gpio_is_valid(gpio))
+		return 0;
+
+	/* Always set enable GPIO high. */
+	ret = devm_gpio_request_one(lp->dev, gpio, GPIOF_OUT_INIT_HIGH, "LP872X EN");
+	if (ret) {
+		dev_err(lp->dev, "gpio request err: %d\n", ret);
+		return ret;
+	}
+
+	/* Each chip has a different enable delay. */
+	if (lp->chipid == LP8720)
+		usleep_range(LP8720_ENABLE_DELAY, 1.5 * LP8720_ENABLE_DELAY);
+	else
+		usleep_range(LP8725_ENABLE_DELAY, 1.5 * LP8725_ENABLE_DELAY);
+
+	return 0;
+}
+
 static int lp872x_config(struct lp872x *lp)
 {
 	struct lp872x_platform_data *pdata = lp->pdata;
@@ -875,6 +903,8 @@ static struct lp872x_platform_data
 	of_property_read_u8(np, "ti,dvs-state", &dvs_state);
 	pdata->dvs->init_state = dvs_state ? DVS_HIGH : DVS_LOW;
 
+	pdata->enable_gpio = of_get_named_gpio(np, "enable-gpios", 0);
+
 	if (of_get_child_count(np) == 0)
 		goto out;
 
@@ -948,6 +978,10 @@ static int lp872x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 	lp->chipid = id->driver_data;
 	i2c_set_clientdata(cl, lp);
 
+	ret = lp872x_hw_enable(lp);
+	if (ret)
+		return ret;
+
 	ret = lp872x_config(lp);
 	if (ret)
 		return ret;
diff --git a/include/linux/regulator/lp872x.h b/include/linux/regulator/lp872x.h
index 132e05c..6029279 100644
--- a/include/linux/regulator/lp872x.h
+++ b/include/linux/regulator/lp872x.h
@@ -18,6 +18,9 @@
 
 #define LP872X_MAX_REGULATORS		9
 
+#define LP8720_ENABLE_DELAY		200
+#define LP8725_ENABLE_DELAY		30000
+
 enum lp872x_regulator_id {
 	LP8720_ID_BASE,
 	LP8720_ID_LDO1 = LP8720_ID_BASE,
@@ -79,12 +82,14 @@ struct lp872x_regulator_data {
  * @update_config     : if LP872X_GENERAL_CFG register is updated, set true
  * @regulator_data    : platform regulator id and init data
  * @dvs               : dvs data for buck voltage control
+ * @enable_gpio       : gpio pin number for enable control
  */
 struct lp872x_platform_data {
 	u8 general_config;
 	bool update_config;
 	struct lp872x_regulator_data regulator_data[LP872X_MAX_REGULATORS];
 	struct lp872x_dvs *dvs;
+	int enable_gpio;
 };
 
 #endif
-- 
1.9.1

[PATCH v2 3/4] ARM: LG Optimus Black (P970) codename sniper support, with basic features

[Paul Kocialkowski]

The LG Optimus Black (P970) codename sniper is a smartphone that was designed
and manufactured by LG Electronics (LGE) and released back in 2011.
It is using an OMAP3630 SoC, GP version.

This adds devicetree support for the device, with only a few basic features
supported, such as debug uart, i2c, internal emmc and external mmc.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/boot/dts/Makefile         |   1 +
 arch/arm/boot/dts/omap3-sniper.dts | 215 +++++++++++++++++++++++++++++++++++++
 2 files changed, 216 insertions(+)
 create mode 100644 arch/arm/boot/dts/omap3-sniper.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index a4a6d70..7314cf8 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -460,6 +460,7 @@ dtb-$(CONFIG_ARCH_OMAP3) += \
 	omap3-sbc-t3517.dtb \
 	omap3-sbc-t3530.dtb \
 	omap3-sbc-t3730.dtb \
+	omap3-sniper.dtb \
 	omap3-thunder.dtb \
 	omap3-zoom3.dtb
 dtb-$(CONFIG_SOC_TI81XX) += \
diff --git a/arch/arm/boot/dts/omap3-sniper.dts b/arch/arm/boot/dts/omap3-sniper.dts
new file mode 100644
index 0000000..24bcd82
--- /dev/null
+++ b/arch/arm/boot/dts/omap3-sniper.dts
@@ -0,0 +1,215 @@
+/*
+ * Copyright (C) 2015-2016 Paul Kocialkowski <contact@paulk.fr>
+ *
+ * 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.
+ */
+/dts-v1/;
+
+#include "omap36xx.dtsi"
+
+/ {
+	model = "LG Optimus Black (P970)";
+	compatible = "lge,omap3-sniper", "ti,omap36xx", "ti,omap3";
+
+	cpus {
+		cpu@0 {
+			cpu0-supply = <&vcc>;
+		};
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <0x80000000 0x20000000>; /* 512 MB */
+	};
+};
+
+&omap3_pmx_core {
+	pinctrl-names = "default";
+
+	uart3_pins: pinmux_uart3_pins {
+		pinctrl-single,pins = <
+			OMAP3_CORE1_IOPAD(0x219e, PIN_INPUT | MUX_MODE0)	/* uart3_rx_irrx */
+			OMAP3_CORE1_IOPAD(0x21a0, PIN_OUTPUT | MUX_MODE0)	/* uart3_tx_irtx */
+		>;
+	};
+
+	dp3t_sel_pins: pinmux_dp3t_sel_pins {
+		pinctrl-single,pins = <
+			OMAP3_CORE1_IOPAD(0x2196, PIN_OUTPUT | MUX_MODE4)	/* gpio_161 */
+			OMAP3_CORE1_IOPAD(0x2198, PIN_OUTPUT | MUX_MODE4)	/* gpio_162 */
+		>;
+	};
+
+	i2c1_pins: pinmux_i2c1_pins {
+		pinctrl-single,pins = <
+			OMAP3_CORE1_IOPAD(0x21ba, PIN_INPUT | MUX_MODE0)	/* i2c1_scl */
+			OMAP3_CORE1_IOPAD(0x21bc, PIN_INPUT | MUX_MODE0)	/* i2c1_sda */
+		>;
+	};
+
+	i2c2_pins: pinmux_i2c2_pins {
+		pinctrl-single,pins = <
+			OMAP3_CORE1_IOPAD(0x21be, PIN_INPUT | MUX_MODE0)	/* i2c2_scl */
+			OMAP3_CORE1_IOPAD(0x21c0, PIN_INPUT | MUX_MODE0)	/* i2c2_sda */
+		>;
+	};
+
+	i2c3_pins: pinmux_i2c3_pins {
+		pinctrl-single,pins = <
+			OMAP3_CORE1_IOPAD(0x21c2, PIN_INPUT | MUX_MODE0)	/* i2c3_scl */
+			OMAP3_CORE1_IOPAD(0x21c4, PIN_INPUT | MUX_MODE0)	/* i2c3_sda */
+		>;
+	};
+
+	lp8720_en_pin: pinmux_lp8720_en_pin {
+		pinctrl-single,pins = <
+			OMAP3_CORE1_IOPAD(0x2080, PIN_OUTPUT | MUX_MODE4)	/* gpio_37 */
+		>;
+	};
+
+	mmc1_pins: pinmux_mmc1_pins {
+		pinctrl-single,pins = <
+			OMAP3_CORE1_IOPAD(0x2144, PIN_INPUT | MUX_MODE0)	/* sdmmc1_clk */
+			OMAP3_CORE1_IOPAD(0x2146, PIN_INPUT | MUX_MODE0)	/* sdmmc1_cmd */
+			OMAP3_CORE1_IOPAD(0x2148, PIN_INPUT | MUX_MODE0)	/* sdmmc1_dat0 */
+			OMAP3_CORE1_IOPAD(0x214a, PIN_INPUT | MUX_MODE0)	/* sdmmc1_dat1 */
+			OMAP3_CORE1_IOPAD(0x214c, PIN_INPUT | MUX_MODE0)	/* sdmmc1_dat2 */
+			OMAP3_CORE1_IOPAD(0x214e, PIN_INPUT | MUX_MODE0)	/* sdmmc1_dat3 */
+		>;
+	};
+
+	mmc2_pins: pinmux_mmc2_pins {
+		pinctrl-single,pins = <
+			OMAP3_CORE1_IOPAD(0x2158, PIN_INPUT | MUX_MODE0)	/* sdmmc2_clk */
+			OMAP3_CORE1_IOPAD(0x215a, PIN_INPUT | MUX_MODE0)	/* sdmmc2_cmd */
+			OMAP3_CORE1_IOPAD(0x215c, PIN_INPUT | MUX_MODE0)	/* sdmmc2_dat0 */
+			OMAP3_CORE1_IOPAD(0x215e, PIN_INPUT | MUX_MODE0)	/* sdmmc2_dat1 */
+			OMAP3_CORE1_IOPAD(0x2160, PIN_INPUT | MUX_MODE0)	/* sdmmc2_dat2 */
+			OMAP3_CORE1_IOPAD(0x2162, PIN_INPUT | MUX_MODE0)	/* sdmmc2_dat3 */
+			OMAP3_CORE1_IOPAD(0x2164, PIN_INPUT | MUX_MODE0)	/* sdmmc2_dat4 */
+			OMAP3_CORE1_IOPAD(0x2166, PIN_INPUT | MUX_MODE0)	/* sdmmc2_dat5 */
+			OMAP3_CORE1_IOPAD(0x2168, PIN_INPUT | MUX_MODE0)	/* sdmmc2_dat6 */
+			OMAP3_CORE1_IOPAD(0x216a, PIN_INPUT | MUX_MODE0)	/* sdmmc2_dat7 */
+		>;
+	};
+};
+
+&omap3_pmx_wkup {
+	pinctrl-names = "default";
+
+	mmc1_cd_pin: pinmux_mmc1_cd_pin {
+		pinctrl-single,pins = <
+			OMAP3_WKUP_IOPAD(0x2a1a, PIN_INPUT | MUX_MODE4)		/* gpio_10 */
+		>;
+	};
+};
+
+&gpio2 {
+	ti,no-reset-on-init;
+};
+
+&gpio5 {
+	ti,no-reset-on-init;
+};
+
+&gpio6 {
+	ti,no-reset-on-init;
+};
+
+&uart3 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart3_pins &dp3t_sel_pins>;
+
+	interrupts-extended = <&intc 74 &omap3_pmx_core OMAP3_UART3_RX>;
+};
+
+&i2c1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c1_pins>;
+
+	clock-frequency = <2600000>;
+
+	twl: twl@48 {
+		reg = <0x48>;
+		interrupts = <7>; /* SYS_NIRQ cascaded to intc */
+		interrupt-parent = <&intc>;
+
+		twl_power: power {
+			compatible = "ti,twl4030-power";
+			ti,use_poweroff;
+		};
+	};
+};
+
+#include "twl4030.dtsi"
+#include "twl4030_omap3.dtsi"
+
+&i2c2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c2_pins>;
+
+	clock-frequency = <400000>;
+};
+
+&i2c3 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c3_pins>;
+
+	clock-frequency = <400000>;
+
+	lp8720@7d {
+		pinctrl-names = "default";
+		pinctrl-0 = <&lp8720_en_pin>;
+
+		compatible = "ti,lp8720";
+		reg = <0x7d>;
+
+		enable-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>; /* gpio_37 */
+
+		lp8720_ldo1: ldo1 {
+			regulator-min-microvolt = <3000000>;
+			regulator-max-microvolt = <3000000>;
+		};
+	};
+};
+
+&mmc1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc1_pins &mmc1_cd_pin>;
+	vmmc-supply = <&lp8720_ldo1>;
+	cd-gpios = <&gpio1 10 GPIO_ACTIVE_LOW>; /* gpio 10 */
+	bus-width = <4>;
+};
+
+&mmc2 {
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc2_pins>;
+	vmmc-supply = <&vmmc2>;
+	ti,non-removable;
+	bus-width = <8>;
+};
+
+&mmc3 {
+	status = "disabled";
+};
+
+/*
+ * The TWL4030 VAUX2 and VDAC regulators power sensors that are slaves on I2C3.
+ * When not powered, these sensors cause the I2C3 clock to stay low at all times,
+ * making it impossible to reach other devices on I2C3.
+ */
+
+&vaux2 {
+	regulator-min-microvolt = <2800000>;
+	regulator-max-microvolt = <2800000>;
+	regulator-always-on;
+};
+
+&vdac {
+	regulator-min-microvolt = <1800000>;
+	regulator-max-microvolt = <1800000>;
+	regulator-always-on;
+};
-- 
1.9.1

[PATCH v2 4/4] ARM: multi_v7_defconfig: Enable LP872x regulator support

[Paul Kocialkowski]

The LP872x regulator is used in the LG Optimus Black (P970) codename sniper
to supply the external mmc card.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 314f6be..bc44876 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -467,6 +467,7 @@ CONFIG_REGULATOR_RK808=y
 CONFIG_REGULATOR_GPIO=y
 CONFIG_MFD_SYSCON=y
 CONFIG_POWER_RESET_SYSCON=y
+CONFIG_REGULATOR_LP872X=y
 CONFIG_REGULATOR_MAX14577=m
 CONFIG_REGULATOR_MAX8907=y
 CONFIG_REGULATOR_MAX8973=y
-- 
1.9.1

Re: [PATCH v2 3/4] ARM: LG Optimus Black (P970) codename sniper support, with basic features

[Paul Kocialkowski]

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

Hi,

Le vendredi 05 février 2016 à 19:42 +0100, Paul Kocialkowski a écrit :
> The LG Optimus Black (P970) codename sniper is a smartphone that was designed
> and manufactured by LG Electronics (LGE) and released back in 2011.
> It is using an OMAP3630 SoC, GP version.
> 
> This adds devicetree support for the device, with only a few basic features
> supported, such as debug uart, i2c, internal emmc and external mmc.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>  arch/arm/boot/dts/Makefile         |   1 +
>  arch/arm/boot/dts/omap3-sniper.dts | 215 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 216 insertions(+)
>  create mode 100644 arch/arm/boot/dts/omap3-sniper.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index a4a6d70..7314cf8 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -460,6 +460,7 @@ dtb-$(CONFIG_ARCH_OMAP3) += \
>  	omap3-sbc-t3517.dtb \
>  	omap3-sbc-t3530.dtb \
>  	omap3-sbc-t3730.dtb \
> +	omap3-sniper.dtb \
>  	omap3-thunder.dtb \
>  	omap3-zoom3.dtb
>  dtb-$(CONFIG_SOC_TI81XX) += \
> diff --git a/arch/arm/boot/dts/omap3-sniper.dts b/arch/arm/boot/dts/omap3-sniper.dts
> new file mode 100644
> index 0000000..24bcd82
> --- /dev/null
> +++ b/arch/arm/boot/dts/omap3-sniper.dts
> @@ -0,0 +1,215 @@
> +/*
> + * Copyright (C) 2015-2016 Paul Kocialkowski <contact@paulk.fr>
> + *
> + * 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.
> + */
> +/dts-v1/;
> +
> +#include "omap36xx.dtsi"
> +
> +/ {
> +	model = "LG Optimus Black (P970)";
> +	compatible = "lge,omap3-sniper", "ti,omap36xx", "ti,omap3";
> +
> +	cpus {
> +		cpu@0 {
> +			cpu0-supply = <&vcc>;
> +		};
> +	};
> +
> +	memory {
> +		device_type = "memory";
> +		reg = <0x80000000 0x20000000>; /* 512 MB */
> +	};
> +};
> +
> +&omap3_pmx_core {
> +	pinctrl-names = "default";
> +
> +	uart3_pins: pinmux_uart3_pins {
> +		pinctrl-single,pins = <
> +			OMAP3_CORE1_IOPAD(0x219e, PIN_INPUT | MUX_MODE0)	/* uart3_rx_irrx */
> +			OMAP3_CORE1_IOPAD(0x21a0, PIN_OUTPUT | MUX_MODE0)	/* uart3_tx_irtx */
> +		>;
> +	};
> +
> +	dp3t_sel_pins: pinmux_dp3t_sel_pins {
> +		pinctrl-single,pins = <
> +			OMAP3_CORE1_IOPAD(0x2196, PIN_OUTPUT | MUX_MODE4)	/* gpio_161 */
> +			OMAP3_CORE1_IOPAD(0x2198, PIN_OUTPUT | MUX_MODE4)	/* gpio_162 */
> +		>;
> +	};
> +
> +	i2c1_pins: pinmux_i2c1_pins {
> +		pinctrl-single,pins = <
> +			OMAP3_CORE1_IOPAD(0x21ba, PIN_INPUT | MUX_MODE0)	/* i2c1_scl */
> +			OMAP3_CORE1_IOPAD(0x21bc, PIN_INPUT | MUX_MODE0)	/* i2c1_sda */
> +		>;
> +	};
> +
> +	i2c2_pins: pinmux_i2c2_pins {
> +		pinctrl-single,pins = <
> +			OMAP3_CORE1_IOPAD(0x21be, PIN_INPUT | MUX_MODE0)	/* i2c2_scl */
> +			OMAP3_CORE1_IOPAD(0x21c0, PIN_INPUT | MUX_MODE0)	/* i2c2_sda */
> +		>;
> +	};
> +
> +	i2c3_pins: pinmux_i2c3_pins {
> +		pinctrl-single,pins = <
> +			OMAP3_CORE1_IOPAD(0x21c2, PIN_INPUT | MUX_MODE0)	/* i2c3_scl */
> +			OMAP3_CORE1_IOPAD(0x21c4, PIN_INPUT | MUX_MODE0)	/* i2c3_sda */
> +		>;
> +	};
> +
> +	lp8720_en_pin: pinmux_lp8720_en_pin {
> +		pinctrl-single,pins = <
> +			OMAP3_CORE1_IOPAD(0x2080, PIN_OUTPUT | MUX_MODE4)	/* gpio_37 */
> +		>;
> +	};
> +
> +	mmc1_pins: pinmux_mmc1_pins {
> +		pinctrl-single,pins = <
> +			OMAP3_CORE1_IOPAD(0x2144, PIN_INPUT | MUX_MODE0)	/* sdmmc1_clk */
> +			OMAP3_CORE1_IOPAD(0x2146, PIN_INPUT | MUX_MODE0)	/* sdmmc1_cmd */
> +			OMAP3_CORE1_IOPAD(0x2148, PIN_INPUT | MUX_MODE0)	/* sdmmc1_dat0 */
> +			OMAP3_CORE1_IOPAD(0x214a, PIN_INPUT | MUX_MODE0)	/* sdmmc1_dat1 */
> +			OMAP3_CORE1_IOPAD(0x214c, PIN_INPUT | MUX_MODE0)	/* sdmmc1_dat2 */
> +			OMAP3_CORE1_IOPAD(0x214e, PIN_INPUT | MUX_MODE0)	/* sdmmc1_dat3 */
> +		>;
> +	};
> +
> +	mmc2_pins: pinmux_mmc2_pins {
> +		pinctrl-single,pins = <
> +			OMAP3_CORE1_IOPAD(0x2158, PIN_INPUT | MUX_MODE0)	/* sdmmc2_clk */
> +			OMAP3_CORE1_IOPAD(0x215a, PIN_INPUT | MUX_MODE0)	/* sdmmc2_cmd */
> +			OMAP3_CORE1_IOPAD(0x215c, PIN_INPUT | MUX_MODE0)	/* sdmmc2_dat0 */
> +			OMAP3_CORE1_IOPAD(0x215e, PIN_INPUT | MUX_MODE0)	/* sdmmc2_dat1 */
> +			OMAP3_CORE1_IOPAD(0x2160, PIN_INPUT | MUX_MODE0)	/* sdmmc2_dat2 */
> +			OMAP3_CORE1_IOPAD(0x2162, PIN_INPUT | MUX_MODE0)	/* sdmmc2_dat3 */
> +			OMAP3_CORE1_IOPAD(0x2164, PIN_INPUT | MUX_MODE0)	/* sdmmc2_dat4 */
> +			OMAP3_CORE1_IOPAD(0x2166, PIN_INPUT | MUX_MODE0)	/* sdmmc2_dat5 */
> +			OMAP3_CORE1_IOPAD(0x2168, PIN_INPUT | MUX_MODE0)	/* sdmmc2_dat6 */
> +			OMAP3_CORE1_IOPAD(0x216a, PIN_INPUT | MUX_MODE0)	/* sdmmc2_dat7 */
> +		>;
> +	};
> +};
> +
> +&omap3_pmx_wkup {
> +	pinctrl-names = "default";
> +
> +	mmc1_cd_pin: pinmux_mmc1_cd_pin {
> +		pinctrl-single,pins = <
> +			OMAP3_WKUP_IOPAD(0x2a1a, PIN_INPUT | MUX_MODE4)		/* gpio_10 */
> +		>;
> +	};
> +};
> +
> +&gpio2 {
> +	ti,no-reset-on-init;
> +};
> +
> +&gpio5 {
> +	ti,no-reset-on-init;
> +};
> +
> +&gpio6 {
> +	ti,no-reset-on-init;
> +};
> +
> +&uart3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&uart3_pins &dp3t_sel_pins>;
> +
> +	interrupts-extended = <&intc 74 &omap3_pmx_core OMAP3_UART3_RX>;
> +};
> +
> +&i2c1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c1_pins>;
> +
> +	clock-frequency = <2600000>;
> +
> +	twl: twl@48 {
> +		reg = <0x48>;
> +		interrupts = <7>; /* SYS_NIRQ cascaded to intc */
> +		interrupt-parent = <&intc>;
> +
> +		twl_power: power {
> +			compatible = "ti,twl4030-power";
> +			ti,use_poweroff;
> +		};
> +	};
> +};
> +
> +#include "twl4030.dtsi"
> +#include "twl4030_omap3.dtsi"
> +
> +&i2c2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c2_pins>;
> +
> +	clock-frequency = <400000>;
> +};
> +
> +&i2c3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c3_pins>;
> +
> +	clock-frequency = <400000>;
> +
> +	lp8720@7d {
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&lp8720_en_pin>;
> +
> +		compatible = "ti,lp8720";
> +		reg = <0x7d>;
> +
> +		enable-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>; /* gpio_37 */
> +
> +		lp8720_ldo1: ldo1 {
> +			regulator-min-microvolt = <3000000>;
> +			regulator-max-microvolt = <3000000>;
> +		};
> +	};
> +};
> +
> +&mmc1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&mmc1_pins &mmc1_cd_pin>;
> +	vmmc-supply = <&lp8720_ldo1>;
> +	cd-gpios = <&gpio1 10 GPIO_ACTIVE_LOW>; /* gpio 10 */
> +	bus-width = <4>;
> +};
> +
> +&mmc2 {
> +

Please discard this newline in case v2 gets merged.

> +	pinctrl-names = "default";
> +	pinctrl-0 = <&mmc2_pins>;
> +	vmmc-supply = <&vmmc2>;
> +	ti,non-removable;
> +	bus-width = <8>;
> +};
> +
> +&mmc3 {
> +	status = "disabled";
> +};
> +
> +/*
> + * The TWL4030 VAUX2 and VDAC regulators power sensors that are slaves on I2C3.
> + * When not powered, these sensors cause the I2C3 clock to stay low at all times,
> + * making it impossible to reach other devices on I2C3.
> + */
> +
> +&vaux2 {
> +	regulator-min-microvolt = <2800000>;
> +	regulator-max-microvolt = <2800000>;
> +	regulator-always-on;
> +};
> +
> +&vdac {
> +	regulator-min-microvolt = <1800000>;
> +	regulator-max-microvolt = <1800000>;
> +	regulator-always-on;
> +};


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 3/4] ARM: LG Optimus Black (P970) codename sniper support, with basic features

[Paul Kocialkowski]

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

Hi,

Le vendredi 05 février 2016 à 20:39 +0100, Paul Kocialkowski a écrit :
> Le vendredi 05 février 2016 à 19:42 +0100, Paul Kocialkowski a écrit :
> > The LG Optimus Black (P970) codename sniper is a smartphone that was designed
> > and manufactured by LG Electronics (LGE) and released back in 2011.
> > It is using an OMAP3630 SoC, GP version.
> > 
> > This adds devicetree support for the device, with only a few basic features
> > supported, such as debug uart, i2c, internal emmc and external mmc.

[snip}

> > +&mmc2 {
> > +
> 
> Please discard this newline in case v2 gets merged.

All things considered, I'd rather not have this patch merged as-is, but
wait for the next version.

I'll wait for feedback before sending v3.

-- 
Paul Kocialkowski, Replicant developer

Replicant is a fully free Android distribution running on several
devices, a free software mobile operating system putting the emphasis on
freedom and privacy/security.

Website: https://www.replicant.us/
Blog: https://blog.replicant.us/
Wiki/tracker/forums: https://redmine.replicant.us/


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/4] regulator: lp872x: Add enable GPIO pin support

[Rob Herring]

On Fri, Feb 05, 2016 at 07:42:19PM +0100, Paul Kocialkowski wrote:
> LP872x regulators are made active via the EN pin, which might be hooked to a
> GPIO. This adds support for driving the GPIO high when the driver is in use.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>  .../devicetree/bindings/regulator/lp872x.txt       |  1 +

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/regulator/lp872x.c                         | 34 ++++++++++++++++++++++
>  include/linux/regulator/lp872x.h                   |  5 ++++
>  3 files changed, 40 insertions(+)

Re: [PATCH v2 1/4] regulator: lp872x: Remove warning about invalid DVS GPIO

[Kim, Milo]

On 2/6/2016 3:42 AM, Paul Kocialkowski wrote:
> Some devices don't hook the DVS pin to a GPIO but to ground or VCC.
> In those cases, it is not a problem to have no DVS GPIO provided, as the current
> code will already switch to software-only DVS selection:
>
> When the DVS GPIO is invalid, lp872x_init_dvs jumps to the set_default_dvs_mode
> label, which instructs the chip not to use the DVS pin at all and do it all in
> software instead (by clearing the LP8720_EXT_DVS_M bit in the
> LP872X_GENERAL_CFG register).
>
> That is reflected later in the code, when setting the bucks (the DVS pin only
> applies to the bucks) by checking for the LP8720_EXT_DVS_M bit on the
> LP872X_GENERAL_CFG register (in lp872x_select_buck_vout_addr) to decide whether
> to use software or hardware DVS selection.
>
> Thus, there is no need to print a warning when the DVS GPIO is invalid.
>
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>

Acked-by: Milo Kim <milo.kim@ti.com>

> ---
>   drivers/regulator/lp872x.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/regulator/lp872x.c b/drivers/regulator/lp872x.c
> index 19d7584..21c49d8 100644
> --- a/drivers/regulator/lp872x.c
> +++ b/drivers/regulator/lp872x.c
> @@ -738,10 +738,8 @@ static int lp872x_init_dvs(struct lp872x *lp)
>   		goto set_default_dvs_mode;
>
>   	gpio = dvs->gpio;
> -	if (!gpio_is_valid(gpio)) {
> -		dev_warn(lp->dev, "invalid gpio: %d\n", gpio);
> +	if (!gpio_is_valid(gpio))
>   		goto set_default_dvs_mode;
> -	}
>
>   	pinstate = dvs->init_state;
>   	ret = devm_gpio_request_one(lp->dev, gpio, pinstate, "LP872X DVS");
>

Re: [PATCH v2 2/4] regulator: lp872x: Add enable GPIO pin support

[Kim, Milo]

Hi Paul,

Thanks for the patch. Please see my comments below.

On 2/6/2016 3:42 AM, Paul Kocialkowski wrote:
> LP872x regulators are made active via the EN pin, which might be hooked to a
> GPIO. This adds support for driving the GPIO high when the driver is in use.
>
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>   .../devicetree/bindings/regulator/lp872x.txt       |  1 +
>   drivers/regulator/lp872x.c                         | 34 ++++++++++++++++++++++
>   include/linux/regulator/lp872x.h                   |  5 ++++
>   3 files changed, 40 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/regulator/lp872x.txt b/Documentation/devicetree/bindings/regulator/lp872x.txt
> index 7818318..ca58a68 100644
> --- a/Documentation/devicetree/bindings/regulator/lp872x.txt
> +++ b/Documentation/devicetree/bindings/regulator/lp872x.txt
> @@ -28,6 +28,7 @@ Optional properties:
>     - ti,dvs-gpio: GPIO specifier for external DVS pin control of LP872x devices.
>     - ti,dvs-vsel: DVS selector. 0 = SEL_V1, 1 = SEL_V2.
>     - ti,dvs-state: initial DVS pin state. 0 = DVS_LOW, 1 = DVS_HIGH.
> +  - enable-gpios: GPIO specifier for EN pin control of LP872x devices.
>
>     Sub nodes for regulator_init_data
>       LP8720 has maximum 6 nodes. (child name: ldo1 ~ 5 and buck)
> diff --git a/drivers/regulator/lp872x.c b/drivers/regulator/lp872x.c
> index 21c49d8..3899211 100644
> --- a/drivers/regulator/lp872x.c
> +++ b/drivers/regulator/lp872x.c
> @@ -15,6 +15,7 @@
>   #include <linux/regmap.h>
>   #include <linux/err.h>
>   #include <linux/gpio.h>
> +#include <linux/delay.h>
>   #include <linux/regulator/lp872x.h>
>   #include <linux/regulator/driver.h>
>   #include <linux/platform_device.h>
> @@ -757,6 +758,33 @@ set_default_dvs_mode:
>   				default_dvs_mode[lp->chipid]);
>   }
>
> +static int lp872x_hw_enable(struct lp872x *lp)
> +{
> +	int ret, gpio;
> +
> +	if (!lp->pdata)
> +		return -EINVAL;
> +
> +	gpio = lp->pdata->enable_gpio;
> +	if (!gpio_is_valid(gpio))
> +		return 0;

It seems gpio is always 0 when it starts from DT. 'enable_gpio' is 
updated in lp872x_config() but lp872x_hw_enable() is called first.

> +
> +	/* Always set enable GPIO high. */
> +	ret = devm_gpio_request_one(lp->dev, gpio, GPIOF_OUT_INIT_HIGH, "LP872X EN");
> +	if (ret) {
> +		dev_err(lp->dev, "gpio request err: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Each chip has a different enable delay. */
> +	if (lp->chipid == LP8720)
> +		usleep_range(LP8720_ENABLE_DELAY, 1.5 * LP8720_ENABLE_DELAY);
> +	else
> +		usleep_range(LP8725_ENABLE_DELAY, 1.5 * LP8725_ENABLE_DELAY);
> +
> +	return 0;
> +}
> +
>   static int lp872x_config(struct lp872x *lp)
>   {
>   	struct lp872x_platform_data *pdata = lp->pdata;
> @@ -875,6 +903,8 @@ static struct lp872x_platform_data
>   	of_property_read_u8(np, "ti,dvs-state", &dvs_state);
>   	pdata->dvs->init_state = dvs_state ? DVS_HIGH : DVS_LOW;
>
> +	pdata->enable_gpio = of_get_named_gpio(np, "enable-gpios", 0);

Please move this code to lp872x_populate_pdata_from_dt().
lp872x_hw_enable() is called prior to lp872x_config(), so enable_gpio 
should be set on parsing the DT properties.

> +
>   	if (of_get_child_count(np) == 0)
>   		goto out;
>
> @@ -948,6 +978,10 @@ static int lp872x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
>   	lp->chipid = id->driver_data;
>   	i2c_set_clientdata(cl, lp);
>
> +	ret = lp872x_hw_enable(lp);
> +	if (ret)
> +		return ret;
> +
>   	ret = lp872x_config(lp);
>   	if (ret)
>   		return ret;
> diff --git a/include/linux/regulator/lp872x.h b/include/linux/regulator/lp872x.h
> index 132e05c..6029279 100644
> --- a/include/linux/regulator/lp872x.h
> +++ b/include/linux/regulator/lp872x.h
> @@ -18,6 +18,9 @@
>
>   #define LP872X_MAX_REGULATORS		9
>
> +#define LP8720_ENABLE_DELAY		200
> +#define LP8725_ENABLE_DELAY		30000
> +
>   enum lp872x_regulator_id {
>   	LP8720_ID_BASE,
>   	LP8720_ID_LDO1 = LP8720_ID_BASE,
> @@ -79,12 +82,14 @@ struct lp872x_regulator_data {
>    * @update_config     : if LP872X_GENERAL_CFG register is updated, set true
>    * @regulator_data    : platform regulator id and init data
>    * @dvs               : dvs data for buck voltage control
> + * @enable_gpio       : gpio pin number for enable control
>    */
>   struct lp872x_platform_data {
>   	u8 general_config;
>   	bool update_config;
>   	struct lp872x_regulator_data regulator_data[LP872X_MAX_REGULATORS];
>   	struct lp872x_dvs *dvs;
> +	int enable_gpio;
>   };
>
>   #endif
>

Best regards,
Milo

Re: [PATCH v2 2/4] regulator: lp872x: Add enable GPIO pin support

[Paul Kocialkowski]

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

Hi,
Le vendredi 12 février 2016 à 09:14 +0900, Kim, Milo a écrit :
> Thanks for the patch. Please see my comments below.

Thanks for the review!

> On 2/6/2016 3:42 AM, Paul Kocialkowski wrote:
> > LP872x regulators are made active via the EN pin, which might be hooked to a
> > GPIO. This adds support for driving the GPIO high when the driver is in use.
> >
> > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > ---
> >   .../devicetree/bindings/regulator/lp872x.txt       |  1 +
> >   drivers/regulator/lp872x.c                         | 34 ++++++++++++++++++++++
> >   include/linux/regulator/lp872x.h                   |  5 ++++
> >   3 files changed, 40 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/regulator/lp872x.txt b/Documentation/devicetree/bindings/regulator/lp872x.txt
> > index 7818318..ca58a68 100644
> > --- a/Documentation/devicetree/bindings/regulator/lp872x.txt
> > +++ b/Documentation/devicetree/bindings/regulator/lp872x.txt
> > @@ -28,6 +28,7 @@ Optional properties:
> >     - ti,dvs-gpio: GPIO specifier for external DVS pin control of LP872x devices.
> >     - ti,dvs-vsel: DVS selector. 0 = SEL_V1, 1 = SEL_V2.
> >     - ti,dvs-state: initial DVS pin state. 0 = DVS_LOW, 1 = DVS_HIGH.
> > +  - enable-gpios: GPIO specifier for EN pin control of LP872x devices.
> >
> >     Sub nodes for regulator_init_data
> >       LP8720 has maximum 6 nodes. (child name: ldo1 ~ 5 and buck)
> > diff --git a/drivers/regulator/lp872x.c b/drivers/regulator/lp872x.c
> > index 21c49d8..3899211 100644
> > --- a/drivers/regulator/lp872x.c
> > +++ b/drivers/regulator/lp872x.c
> > @@ -15,6 +15,7 @@
> >   #include <linux/regmap.h>
> >   #include <linux/err.h>
> >   #include <linux/gpio.h>
> > +#include <linux/delay.h>
> >   #include <linux/regulator/lp872x.h>
> >   #include <linux/regulator/driver.h>
> >   #include <linux/platform_device.h>
> > @@ -757,6 +758,33 @@ set_default_dvs_mode:
> >   				default_dvs_mode[lp->chipid]);
> >   }
> >
> > +static int lp872x_hw_enable(struct lp872x *lp)
> > +{
> > +	int ret, gpio;
> > +
> > +	if (!lp->pdata)
> > +		return -EINVAL;
> > +
> > +	gpio = lp->pdata->enable_gpio;
> > +	if (!gpio_is_valid(gpio))
> > +		return 0;
> 
> It seems gpio is always 0 when it starts from DT. 'enable_gpio' is 
> updated in lp872x_config() but lp872x_hw_enable() is called first.

Actually, enable_gpio is updated in lp872x_populate_pdata_from_dt.

> > +
> > +	/* Always set enable GPIO high. */
> > +	ret = devm_gpio_request_one(lp->dev, gpio, GPIOF_OUT_INIT_HIGH, "LP872X EN");
> > +	if (ret) {
> > +		dev_err(lp->dev, "gpio request err: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	/* Each chip has a different enable delay. */
> > +	if (lp->chipid == LP8720)
> > +		usleep_range(LP8720_ENABLE_DELAY, 1.5 * LP8720_ENABLE_DELAY);
> > +	else
> > +		usleep_range(LP8725_ENABLE_DELAY, 1.5 * LP8725_ENABLE_DELAY);
> > +
> > +	return 0;
> > +}
> > +
> >   static int lp872x_config(struct lp872x *lp)
> >   {
> >   	struct lp872x_platform_data *pdata = lp->pdata;
> > @@ -875,6 +903,8 @@ static struct lp872x_platform_data
> >   	of_property_read_u8(np, "ti,dvs-state", &dvs_state);
> >   	pdata->dvs->init_state = dvs_state ? DVS_HIGH : DVS_LOW;
> >
> > +	pdata->enable_gpio = of_get_named_gpio(np, "enable-gpios", 0);
> 
> Please move this code to lp872x_populate_pdata_from_dt().

This already adds it in lp872x_populate_pdata_from_dt (see the context
around the insertion).

I don't know why the diff makes it seem like it's added in
lp872x_config. It's really not.

> lp872x_hw_enable() is called prior to lp872x_config(), so enable_gpio 
> should be set on parsing the DT properties.

Of course.

> > +
> >   	if (of_get_child_count(np) == 0)
> >   		goto out;
> >
> > @@ -948,6 +978,10 @@ static int lp872x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
> >   	lp->chipid = id->driver_data;
> >   	i2c_set_clientdata(cl, lp);
> >
> > +	ret = lp872x_hw_enable(lp);
> > +	if (ret)
> > +		return ret;
> > +
> >   	ret = lp872x_config(lp);
> >   	if (ret)
> >   		return ret;
> > diff --git a/include/linux/regulator/lp872x.h b/include/linux/regulator/lp872x.h
> > index 132e05c..6029279 100644
> > --- a/include/linux/regulator/lp872x.h
> > +++ b/include/linux/regulator/lp872x.h
> > @@ -18,6 +18,9 @@
> >
> >   #define LP872X_MAX_REGULATORS		9
> >
> > +#define LP8720_ENABLE_DELAY		200
> > +#define LP8725_ENABLE_DELAY		30000
> > +
> >   enum lp872x_regulator_id {
> >   	LP8720_ID_BASE,
> >   	LP8720_ID_LDO1 = LP8720_ID_BASE,
> > @@ -79,12 +82,14 @@ struct lp872x_regulator_data {
> >    * @update_config     : if LP872X_GENERAL_CFG register is updated, set true
> >    * @regulator_data    : platform regulator id and init data
> >    * @dvs               : dvs data for buck voltage control
> > + * @enable_gpio       : gpio pin number for enable control
> >    */
> >   struct lp872x_platform_data {
> >   	u8 general_config;
> >   	bool update_config;
> >   	struct lp872x_regulator_data regulator_data[LP872X_MAX_REGULATORS];
> >   	struct lp872x_dvs *dvs;
> > +	int enable_gpio;
> >   };
> >
> >   #endif
> >
> 
> Best regards,
> Milo


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/4] regulator: lp872x: Add enable GPIO pin support

[Kim, Milo]

On 2/13/2016 3:25 AM, Paul Kocialkowski wrote:
>>> > >+
>>> > >+	/* Always set enable GPIO high. */
>>> > >+	ret = devm_gpio_request_one(lp->dev, gpio, GPIOF_OUT_INIT_HIGH, "LP872X EN");
>>> > >+	if (ret) {
>>> > >+		dev_err(lp->dev, "gpio request err: %d\n", ret);
>>> > >+		return ret;
>>> > >+	}
>>> > >+
>>> > >+	/* Each chip has a different enable delay. */
>>> > >+	if (lp->chipid == LP8720)
>>> > >+		usleep_range(LP8720_ENABLE_DELAY, 1.5 * LP8720_ENABLE_DELAY);
>>> > >+	else
>>> > >+		usleep_range(LP8725_ENABLE_DELAY, 1.5 * LP8725_ENABLE_DELAY);
>>> > >+
>>> > >+	return 0;
>>> > >+}
>>> > >+
>>> > >   static int lp872x_config(struct lp872x *lp)
>>> > >   {
>>> > >   	struct lp872x_platform_data *pdata = lp->pdata;
>>> > >@@ -875,6 +903,8 @@ static struct lp872x_platform_data
>>> > >   	of_property_read_u8(np, "ti,dvs-state", &dvs_state);
>>> > >   	pdata->dvs->init_state = dvs_state ? DVS_HIGH : DVS_LOW;
>>> > >
>>> > >+	pdata->enable_gpio = of_get_named_gpio(np, "enable-gpios", 0);
>> >
>> >Please move this code to lp872x_populate_pdata_from_dt().
> This already adds it in lp872x_populate_pdata_from_dt (see the context
> around the insertion).

Ah, sorry. Your patch is correct.

> I don't know why the diff makes it seem like it's added in
> lp872x_config. It's really not.

I think it was from a line break of lp872x_populate_pdata_from_dt().
Let me add my ACK to this patch. Thank you.

Best regards,
Milo

Re: [PATCH v2 2/4] regulator: lp872x: Add enable GPIO pin support

[Kim, Milo]

On 2/6/2016 3:42 AM, Paul Kocialkowski wrote:
> LP872x regulators are made active via the EN pin, which might be hooked to a
> GPIO. This adds support for driving the GPIO high when the driver is in use.
>
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>   .../devicetree/bindings/regulator/lp872x.txt       |  1 +
>   drivers/regulator/lp872x.c                         | 34 ++++++++++++++++++++++
>   include/linux/regulator/lp872x.h                   |  5 ++++
>   3 files changed, 40 insertions(+)

Acked-by: Milo Kim <milo.kim@ti.com>

Best regards,
Milo