[PATCH v3 00/14] regulator: max77686: Add GPIO control

[PATCH v3 00/14] regulator: max77686: Add GPIO control

[Krzysztof Kozlowski]

Hi,


Patches touching max77802 were not tested on hardware.
I am kindly asking for testing it because I don't have the board
with Maxim 77802.

Changes since v2
================
Re-work the board file support removal after Javier's comments: use
new DT style parsing. This imposes a lot of changes.
1. Add "of_compatible" for regulator drivers.
2. Provide backward compatibility if such "of_compatible" is not present.
   The driver will search for regulators node and use it as dev->of_node.
   Everything should be bisect-friendly.
3. New patches: 1, 2, 3, 5, 13 and 14.
4. Because of new style DT parsing it is much easier to put "gpio"
   properties in regulators top node (not in each regulator).
   This will be also new-gpio-lib friendly.

Changes since v1
================
1. Add patch: 1/8 "regulator: max77686: Consistently index opmode
   array by rdev id".
2. Remove patch "regulator: max77686: Make regulator_desc array
   const" (applied).
3. Re-work patches removing from regulators board file support (2/8
   and 3/8). Parse regulators with of_regulator_match() at once, remove
   num_regulators.
4. Patch 4/8: Add depends on OF to mfd/Kconfig. Add Javier's
   reviewed-by.
5. Patch 5/8: Add Javier's reviewed-by.
6. Patch 6/8: Add depends on GPIOLIB to regulator/Kconfig. Rename
   "external control" to "GPIO control" and "ext_control_gpios" to
   simpler "gpios".
   I tried to use new GPIO API but it ended with more problems
   https://lkml.org/lkml/2014/10/29/239


Description
===========
This patch helps in proper description of max77686 regulators in DTS by
allowing to control them over GPIO. This allows removal of
fixed regulators from DTS which duplicate the description of hardware.

The first five patches are cleanups, including board support removal.

The whole patchset should be taken at once.

Patchset is rebased on next-20141023 AND:
1. ARM: EXYNOS: Call regulator suspend prepare/finish
   https://lkml.org/lkml/2014/10/20/545
2. regulator: max77686/trats2: Disable some regulators in suspend
   https://lkml.org/lkml/2014/10/27/280


Best regards,
Krzysztof

*** BLURB HERE ***

Krzysztof Kozlowski (14):
  mfd: max77686/802: Map regulator driver to its own of_node
  mfd/regulator: dt-bindings: max77686: Document of_compatible for
    regulator
  regulator: dt-bindings: max77802: Document of_compatible for regulator
  regulator: max77686: Consistently index opmode array by rdev id
  regulator: max77802: Don't ignore return value of current opmode
  regulator: max77802: Remove support for board files
  regulator: max77686: Remove support for board files
  mfd: max77686/802: Remove support for board files
  regulator: max77686: Initialize opmode explicitly to normal mode
  regulator: max77686: Add GPIO control
  mfd/regulator: dt-bindings: max77686: Document gpio properties
  ARM: dts: exynos4412-trats: Switch max77686 regulators to GPIO control
  ARM: dts: exynos5420-peach: Update to new max77802 regulator
    compatible
  ARM: dts: exynos5800-peach: Update to new max77802 regulator
    compatible

 Documentation/devicetree/bindings/mfd/max77686.txt |  16 +-
 .../devicetree/bindings/regulator/max77802.txt     |   8 +-
 arch/arm/boot/dts/exynos4412-trats2.dts            |  28 +---
 arch/arm/boot/dts/exynos5420-peach-pit.dts         |   2 +
 arch/arm/boot/dts/exynos5800-peach-pi.dts          |   2 +
 drivers/mfd/Kconfig                                |   1 +
 drivers/mfd/max77686.c                             |  33 +---
 drivers/regulator/max77686.c                       | 186 +++++++++++++--------
 drivers/regulator/max77802.c                       | 116 +++++--------
 include/linux/mfd/max77686-private.h               |   1 -
 include/linux/mfd/max77686.h                       |  28 ----
 11 files changed, 200 insertions(+), 221 deletions(-)

-- 
1.9.1

[PATCH v3 01/14] mfd: max77686/802: Map regulator driver to its own of_node

[Krzysztof Kozlowski]

Add of_compatible fields for max77686 and max77802 regulator drivers.
The driver's node should be the same as voltage-regulators node. This
simplifies parsing of regulators init data from DTS.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/mfd/max77686.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
index 929795eae9fc..9e1046bdef90 100644
--- a/drivers/mfd/max77686.c
+++ b/drivers/mfd/max77686.c
@@ -38,13 +38,19 @@
 #define I2C_ADDR_RTC	(0x0C >> 1)
 
 static const struct mfd_cell max77686_devs[] = {
-	{ .name = "max77686-pmic", },
+	{
+		.name = "max77686-pmic",
+		.of_compatible = "maxim,max77686-pmic",
+	},
 	{ .name = "max77686-rtc", },
 	{ .name = "max77686-clk", },
 };
 
 static const struct mfd_cell max77802_devs[] = {
-	{ .name = "max77802-pmic", },
+	{
+		.name = "max77802-pmic",
+		.of_compatible = "maxim,max77802-pmic",
+	},
 	{ .name = "max77802-clk", },
 	{ .name = "max77802-rtc", },
 };
-- 
1.9.1

Re: [PATCH v3 01/14] mfd: max77686/802: Map regulator driver to its own of_node

[Mark Brown]

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

On Thu, Oct 30, 2014 at 12:20:40PM +0100, Krzysztof Kozlowski wrote:
> Add of_compatible fields for max77686 and max77802 regulator drivers.
> The driver's node should be the same as voltage-regulators node. This
> simplifies parsing of regulators init data from DTS.

No, this is broken.  You're introducing an ABI break that conveys no
additional information, I can't see any reason why this should make it
simpler to parse init data (you've certainly not articulated one in the
changelog here) but even if it did you are changing the ABI incompatibly
and convenience isn't a good reason to do that.

I'm getting very frustrated with what's going on with these drivers,
there seem to be a lot of rather large sets of patches spawning lots of
discussion but also frequent review problems and very little actually
getting merged (look at the set of changes in the past few merge windows
for example).  There's something going wrong here.

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

Re: [PATCH v3 01/14] mfd: max77686/802: Map regulator driver to its own of_node

[Krzysztof Kozlowski]

On pią, 2014-10-31 at 12:23 +0000, Mark Brown wrote:
> On Thu, Oct 30, 2014 at 12:20:40PM +0100, Krzysztof Kozlowski wrote:
> > Add of_compatible fields for max77686 and max77802 regulator drivers.
> > The driver's node should be the same as voltage-regulators node. This
> > simplifies parsing of regulators init data from DTS.
> 
> No, this is broken.  You're introducing an ABI break that conveys no
> additional information, I can't see any reason why this should make it
> simpler to parse init data (you've certainly not articulated one in the
> changelog here) but even if it did you are changing the ABI incompatibly
> and convenience isn't a good reason to do that.

The ABI won't be broken - both drivers would work fine with old and new
DTB. However I agree that I should justify this more...

Javier and you explained me using parent's device for rdev->dev so I
think this change won't be needed and I'll just drop it.

Thank you for feedback.

> I'm getting very frustrated with what's going on with these drivers,
> there seem to be a lot of rather large sets of patches spawning lots of
> discussion but also frequent review problems and very little actually
> getting merged (look at the set of changes in the past few merge windows
> for example).  There's something going wrong here.

If I over-spammed you, then I am deeply sorry.

Best regards,
Krzysztof

Re: [PATCH v3 01/14] mfd: max77686/802: Map regulator driver to its own of_node

[Mark Brown]

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

On Fri, Oct 31, 2014 at 02:07:54PM +0100, Krzysztof Kozlowski wrote:
> On pią, 2014-10-31 at 12:23 +0000, Mark Brown wrote:

> > I'm getting very frustrated with what's going on with these drivers,
> > there seem to be a lot of rather large sets of patches spawning lots of
> > discussion but also frequent review problems and very little actually
> > getting merged (look at the set of changes in the past few merge windows
> > for example).  There's something going wrong here.

> If I over-spammed you, then I am deeply sorry.

It's not just about volume, it's also about the fact that so little of
the mail around these drivers (not just from you) is translating into
code that gets merged.

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

[PATCH v3 02/14] mfd/regulator: dt-bindings: max77686: Document of_compatible for regulator

[Krzysztof Kozlowski]

Document new required property for regulator driver: of_compatible. The
property allows MFD core to bind the driver to node with regulators and
this simplifies parsing regulators init data from DTS

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 Documentation/devicetree/bindings/mfd/max77686.txt | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mfd/max77686.txt b/Documentation/devicetree/bindings/mfd/max77686.txt
index 75fdfaf41831..f4010a9f66eb 100644
--- a/Documentation/devicetree/bindings/mfd/max77686.txt
+++ b/Documentation/devicetree/bindings/mfd/max77686.txt
@@ -18,8 +18,11 @@ Required properties:
 
 Optional node:
 - voltage-regulators : The regulators of max77686 have to be instantiated
-  under subnode named "voltage-regulators" using the following format.
+  under subnode named "voltage-regulators".
+  Required property:
+  - compatible : Must be "maxim,max77686-pmic"
 
+  Optional properties: For each regulator use following format:
 	regulator_name {
 		regulator-compatible = LDOn/BUCKn
 		standard regulator constraints....
@@ -49,6 +52,8 @@ Example:
 		reg = <0x09>;
 
 		voltage-regulators {
+			compatible = "maxim,max77686-pmic";
+
 			ldo11_reg {
 				regulator-compatible = "LDO11";
 				regulator-name = "vdd_ldo11";
-- 
1.9.1

Re: [PATCH v3 02/14] mfd/regulator: dt-bindings: max77686: Document of_compatible for regulator

[Lee Jones]

On Thu, 30 Oct 2014, Krzysztof Kozlowski wrote:

> Document new required property for regulator driver: of_compatible. The
> property allows MFD core to bind the driver to node with regulators and
> this simplifies parsing regulators init data from DTS
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
>  Documentation/devicetree/bindings/mfd/max77686.txt | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

I want a thumbs-up from Mark on this before applying.

> diff --git a/Documentation/devicetree/bindings/mfd/max77686.txt b/Documentation/devicetree/bindings/mfd/max77686.txt
> index 75fdfaf41831..f4010a9f66eb 100644
> --- a/Documentation/devicetree/bindings/mfd/max77686.txt
> +++ b/Documentation/devicetree/bindings/mfd/max77686.txt
> @@ -18,8 +18,11 @@ Required properties:
>  
>  Optional node:
>  - voltage-regulators : The regulators of max77686 have to be instantiated
> -  under subnode named "voltage-regulators" using the following format.
> +  under subnode named "voltage-regulators".
> +  Required property:
> +  - compatible : Must be "maxim,max77686-pmic"
>  
> +  Optional properties: For each regulator use following format:
>  	regulator_name {
>  		regulator-compatible = LDOn/BUCKn
>  		standard regulator constraints....
> @@ -49,6 +52,8 @@ Example:
>  		reg = <0x09>;
>  
>  		voltage-regulators {
> +			compatible = "maxim,max77686-pmic";
> +
>  			ldo11_reg {
>  				regulator-compatible = "LDO11";
>  				regulator-name = "vdd_ldo11";

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

Re: [PATCH v3 02/14] mfd/regulator: dt-bindings: max77686: Document of_compatible for regulator

[Krzysztof Kozlowski]

On pon, 2014-11-03 at 17:00 +0000, Lee Jones wrote:
> On Thu, 30 Oct 2014, Krzysztof Kozlowski wrote:
> 
> > Document new required property for regulator driver: of_compatible. The
> > property allows MFD core to bind the driver to node with regulators and
> > this simplifies parsing regulators init data from DTS
> > 
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > ---
> >  Documentation/devicetree/bindings/mfd/max77686.txt | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> I want a thumbs-up from Mark on this before applying.

I'll be resending new version, without the compatible property.

Best regards,
Krzysztof

> 
> > diff --git a/Documentation/devicetree/bindings/mfd/max77686.txt b/Documentation/devicetree/bindings/mfd/max77686.txt
> > index 75fdfaf41831..f4010a9f66eb 100644
> > --- a/Documentation/devicetree/bindings/mfd/max77686.txt
> > +++ b/Documentation/devicetree/bindings/mfd/max77686.txt
> > @@ -18,8 +18,11 @@ Required properties:
> >  
> >  Optional node:
> >  - voltage-regulators : The regulators of max77686 have to be instantiated
> > -  under subnode named "voltage-regulators" using the following format.
> > +  under subnode named "voltage-regulators".
> > +  Required property:
> > +  - compatible : Must be "maxim,max77686-pmic"
> >  
> > +  Optional properties: For each regulator use following format:
> >  	regulator_name {
> >  		regulator-compatible = LDOn/BUCKn
> >  		standard regulator constraints....
> > @@ -49,6 +52,8 @@ Example:
> >  		reg = <0x09>;
> >  
> >  		voltage-regulators {
> > +			compatible = "maxim,max77686-pmic";
> > +
> >  			ldo11_reg {
> >  				regulator-compatible = "LDO11";
> >  				regulator-name = "vdd_ldo11";
> 

[PATCH v3 03/14] regulator: dt-bindings: max77802: Document of_compatible for regulator

[Krzysztof Kozlowski]

Document new required property for regulator driver: of_compatible.
The property allows MFD core to bind the driver to node with regulators
and this simplifies parsing regulators init data from DTS

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 Documentation/devicetree/bindings/regulator/max77802.txt | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/regulator/max77802.txt b/Documentation/devicetree/bindings/regulator/max77802.txt
index 5aeaffc0f1f0..fa545e7d3b0a 100644
--- a/Documentation/devicetree/bindings/regulator/max77802.txt
+++ b/Documentation/devicetree/bindings/regulator/max77802.txt
@@ -10,7 +10,11 @@ Following properties should be present in main device node of the MFD chip.
 
 Optional node:
 - regulators : The regulators of max77802 have to be instantiated
-  under subnode named "regulators" using the following format.
+  under subnode named "regulators".
+  Required property:
+  - compatible : Must be "maxim,max77802-pmic"
+
+  Optional properties: For each regulator use following format:
 
 	regulator-name {
 		standard regulator constraints....
@@ -36,6 +40,8 @@ Example:
 		#size-cells = <0>;
 
 		regulators {
+			compatible = "maxim,max77802-pmic";
+
 			ldo11_reg: LDO11 {
 				regulator-name = "vdd_ldo11";
 				regulator-min-microvolt = <1900000>;
-- 
1.9.1

[PATCH v3 04/14] regulator: max77686: Consistently index opmode array by rdev id

[Krzysztof Kozlowski]

Mixed indexes were used for array of opmodes in max77686_data structure:
id of regulator and index of regulator_desc array.

These indexes are exactly the same but the mixture may confuse. Use
consistently the id of regulator.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/regulator/max77686.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
index 09b0d8c20a9d..27c5f4556044 100644
--- a/drivers/regulator/max77686.c
+++ b/drivers/regulator/max77686.c
@@ -82,6 +82,7 @@ enum max77686_ramp_rate {
 };
 
 struct max77686_data {
+	/* Array indexed by regulator id */
 	unsigned int opmode[MAX77686_REGULATORS];
 };
 
@@ -513,12 +514,13 @@ static int max77686_pmic_probe(struct platform_device *pdev)
 
 	for (i = 0; i < MAX77686_REGULATORS; i++) {
 		struct regulator_dev *rdev;
+		int id = regulators[i].id;
 
 		config.init_data = pdata->regulators[i].initdata;
 		config.of_node = pdata->regulators[i].of_node;
 
-		max77686->opmode[i] = regulators[i].enable_mask >>
-						max77686_get_opmode_shift(i);
+		max77686->opmode[id] = regulators[i].enable_mask >>
+						max77686_get_opmode_shift(id);
 		rdev = devm_regulator_register(&pdev->dev,
 						&regulators[i], &config);
 		if (IS_ERR(rdev)) {
-- 
1.9.1

[RFT v3 05/14] regulator: max77802: Don't ignore return value of current opmode

[Krzysztof Kozlowski]

The return value of regmap_read() of current opmode for regulator was
silently ignored and whatever happened to be in 'val' variable was used
as new opmode. This could lead to using bogus opmode.

Don't ignore what regmap_read() returns. If it fails just fall back to
normal opmode.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/regulator/max77802.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
index b9958d927297..60daca2028e9 100644
--- a/drivers/regulator/max77802.c
+++ b/drivers/regulator/max77802.c
@@ -606,7 +606,13 @@ static int max77802_pmic_probe(struct platform_device *pdev)
 		config.of_node = pdata->regulators[i].of_node;
 
 		ret = regmap_read(iodev->regmap, regulators[i].enable_reg, &val);
-		val = val >> shift & MAX77802_OPMODE_MASK;
+		if (ret < 0) {
+			dev_warn(&pdev->dev,
+				"cannot read current mode for %d\n", i);
+			val = MAX77802_OPMODE_NORMAL;
+		} else {
+			val = val >> shift & MAX77802_OPMODE_MASK;
+		}
 
 		/*
 		 * If the regulator is disabled and the system warm rebooted,
-- 
1.9.1

Re: [RFT v3 05/14] regulator: max77802: Don't ignore return value of current opmode

[Javier Martinez Canillas]

Hello Krzysztof,

On 10/30/2014 12:20 PM, Krzysztof Kozlowski wrote:
> The return value of regmap_read() of current opmode for regulator was
> silently ignored and whatever happened to be in 'val' variable was used
> as new opmode. This could lead to using bogus opmode.
> 
> Don't ignore what regmap_read() returns. If it fails just fall back to
> normal opmode.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
>  drivers/regulator/max77802.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
> index b9958d927297..60daca2028e9 100644
> --- a/drivers/regulator/max77802.c
> +++ b/drivers/regulator/max77802.c
> @@ -606,7 +606,13 @@ static int max77802_pmic_probe(struct platform_device *pdev)
>  		config.of_node = pdata->regulators[i].of_node;
>  
>  		ret = regmap_read(iodev->regmap, regulators[i].enable_reg, &val);
> -		val = val >> shift & MAX77802_OPMODE_MASK;
> +		if (ret < 0) {
> +			dev_warn(&pdev->dev,
> +				"cannot read current mode for %d\n", i);
> +			val = MAX77802_OPMODE_NORMAL;
> +		} else {
> +			val = val >> shift & MAX77802_OPMODE_MASK;
> +		}
>  
>  		/*
>  		 * If the regulator is disabled and the system warm rebooted,
> 

Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Best regards,
Javier

[RFT v3 06/14] regulator: max77802: Remove support for board files

[Krzysztof Kozlowski]

The driver is used only on Exynos based boards with DTS support.
Convert the driver to DTS-only version. Parse all regulators at once,
not one-by-one. Remove dependency on data provided by max77686 MFD
driver.

Use new DT style parsing method for regulators init data. The regulator
driver now should have its own of_compatible property. If it is not
present then it will use "regulators" sub-node of parent's node.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/regulator/max77802.c | 108 +++++++++++++++----------------------------
 1 file changed, 37 insertions(+), 71 deletions(-)

diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
index 60daca2028e9..2cb938610230 100644
--- a/drivers/regulator/max77802.c
+++ b/drivers/regulator/max77802.c
@@ -70,7 +70,9 @@ static unsigned int ramp_table_77802_4bit[] = {
 };
 
 struct max77802_regulator_prv {
+	/* Array indexed by regulator id */
 	unsigned int opmode[MAX77802_REG_MAX];
+	bool missing_of_node;
 };
 
 static inline int max77802_map_mode(int mode)
@@ -362,6 +364,7 @@ static struct regulator_ops max77802_buck_dvs_ops = {
 /* LDOs 3-7, 9-14, 18-26, 28, 29, 32-34 */
 #define regulator_77802_desc_p_ldo(num, supply, log)	{		\
 	.name		= "LDO"#num,					\
+	.of_match	= of_match_ptr("LDO"#num),			\
 	.id		= MAX77802_LDO##num,				\
 	.supply_name	= "inl"#supply,					\
 	.ops		= &max77802_ldo_ops_logic##log,			\
@@ -380,6 +383,7 @@ static struct regulator_ops max77802_buck_dvs_ops = {
 /* LDOs 1, 2, 8, 15, 17, 27, 30, 35 */
 #define regulator_77802_desc_n_ldo(num, supply, log)   {		\
 	.name		= "LDO"#num,					\
+	.of_match	= of_match_ptr("LDO"#num),			\
 	.id		= MAX77802_LDO##num,				\
 	.supply_name	= "inl"#supply,					\
 	.ops		= &max77802_ldo_ops_logic##log,			\
@@ -398,6 +402,7 @@ static struct regulator_ops max77802_buck_dvs_ops = {
 /* BUCKs 1, 6 */
 #define regulator_77802_desc_16_buck(num)	{		\
 	.name		= "BUCK"#num,					\
+	.of_match	= of_match_ptr("BUCK"#num),			\
 	.id		= MAX77802_BUCK##num,				\
 	.supply_name	= "inb"#num,					\
 	.ops		= &max77802_buck_16_dvs_ops,			\
@@ -416,6 +421,7 @@ static struct regulator_ops max77802_buck_dvs_ops = {
 /* BUCKS 2-4 */
 #define regulator_77802_desc_234_buck(num)	{		\
 	.name		= "BUCK"#num,					\
+	.of_match	= of_match_ptr("BUCK"#num),			\
 	.id		= MAX77802_BUCK##num,				\
 	.supply_name	= "inb"#num,					\
 	.ops		= &max77802_buck_234_ops,			\
@@ -435,6 +441,7 @@ static struct regulator_ops max77802_buck_dvs_ops = {
 /* BUCK 5 */
 #define regulator_77802_desc_buck5(num)		{		\
 	.name		= "BUCK"#num,					\
+	.of_match	= of_match_ptr("BUCK"#num),			\
 	.id		= MAX77802_BUCK##num,				\
 	.supply_name	= "inb"#num,					\
 	.ops		= &max77802_buck_dvs_ops,			\
@@ -453,6 +460,7 @@ static struct regulator_ops max77802_buck_dvs_ops = {
 /* BUCKs 7-10 */
 #define regulator_77802_desc_buck7_10(num)	{		\
 	.name		= "BUCK"#num,					\
+	.of_match	= of_match_ptr("BUCK"#num),			\
 	.id		= MAX77802_BUCK##num,				\
 	.supply_name	= "inb"#num,					\
 	.ops		= &max77802_buck_dvs_ops,			\
@@ -513,83 +521,31 @@ static struct regulator_desc regulators[] = {
 	regulator_77802_desc_n_ldo(35, 2, 1),
 };
 
-#ifdef CONFIG_OF
-static int max77802_pmic_dt_parse_pdata(struct platform_device *pdev,
-					struct max77686_platform_data *pdata)
-{
-	struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
-	struct device_node *pmic_np, *regulators_np;
-	struct max77686_regulator_data *rdata;
-	struct of_regulator_match rmatch;
-	unsigned int i;
-
-	pmic_np = iodev->dev->of_node;
-	regulators_np = of_get_child_by_name(pmic_np, "regulators");
-	if (!regulators_np) {
-		dev_err(&pdev->dev, "could not find regulators sub-node\n");
-		return -EINVAL;
-	}
-
-	pdata->num_regulators = ARRAY_SIZE(regulators);
-	rdata = devm_kzalloc(&pdev->dev, sizeof(*rdata) *
-			     pdata->num_regulators, GFP_KERNEL);
-	if (!rdata) {
-		of_node_put(regulators_np);
-		return -ENOMEM;
-	}
-
-	for (i = 0; i < pdata->num_regulators; i++) {
-		rmatch.name = regulators[i].name;
-		rmatch.init_data = NULL;
-		rmatch.of_node = NULL;
-		if (of_regulator_match(&pdev->dev, regulators_np, &rmatch,
-				       1) != 1) {
-			dev_warn(&pdev->dev, "No matching regulator for '%s'\n",
-				 rmatch.name);
-			continue;
-		}
-		rdata[i].initdata = rmatch.init_data;
-		rdata[i].of_node = rmatch.of_node;
-		rdata[i].id = regulators[i].id;
-	}
-
-	pdata->regulators = rdata;
-	of_node_put(regulators_np);
-
-	return 0;
-}
-#else
-static int max77802_pmic_dt_parse_pdata(struct platform_device *pdev,
-					struct max77686_platform_data *pdata)
-{
-	return 0;
-}
-#endif /* CONFIG_OF */
-
 static int max77802_pmic_probe(struct platform_device *pdev)
 {
 	struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
-	struct max77686_platform_data *pdata = dev_get_platdata(iodev->dev);
 	struct max77802_regulator_prv *max77802;
-	int i, ret = 0, val;
+	int i, val;
 	struct regulator_config config = { };
 
-	/* This is allocated by the MFD driver */
-	if (!pdata) {
-		dev_err(&pdev->dev, "no platform data found for regulator\n");
-		return -ENODEV;
-	}
-
 	max77802 = devm_kzalloc(&pdev->dev,
 				sizeof(struct max77802_regulator_prv),
 				GFP_KERNEL);
 	if (!max77802)
 		return -ENOMEM;
 
-	if (iodev->dev->of_node) {
-		ret = max77802_pmic_dt_parse_pdata(pdev, pdata);
-		if (ret)
-			return ret;
+	if (!pdev->dev.of_node) {
+		/*
+		 * Backward DTS compatiblity where regulator driver had not
+		 * a compatible property for itself.
+		 */
+		if (!iodev->dev->of_node) {
+			dev_err(&pdev->dev, "No OF node for driver and its parent\n");
+			return -EINVAL;
+		}
+		pdev->dev.of_node = of_get_child_by_name(iodev->dev->of_node,
+								"regulators");
+		max77802->missing_of_node = true;
 	}
 
 	config.dev = iodev->dev;
@@ -599,11 +555,9 @@ static int max77802_pmic_probe(struct platform_device *pdev)
 
 	for (i = 0; i < MAX77802_REG_MAX; i++) {
 		struct regulator_dev *rdev;
-		int id = pdata->regulators[i].id;
+		int id = regulators[i].id;
 		int shift = max77802_get_opmode_shift(id);
-
-		config.init_data = pdata->regulators[i].initdata;
-		config.of_node = pdata->regulators[i].of_node;
+		int ret;
 
 		ret = regmap_read(iodev->regmap, regulators[i].enable_reg, &val);
 		if (ret < 0) {
@@ -627,15 +581,26 @@ static int max77802_pmic_probe(struct platform_device *pdev)
 		rdev = devm_regulator_register(&pdev->dev,
 					       &regulators[i], &config);
 		if (IS_ERR(rdev)) {
+			ret = PTR_ERR(rdev);
 			dev_err(&pdev->dev,
-				"regulator init failed for %d\n", i);
-			return PTR_ERR(rdev);
+				"regulator init failed for %d: %d\n", i, ret);
+			return ret;
 		}
 	}
 
 	return 0;
 }
 
+static int max77802_pmic_remove(struct platform_device *pdev)
+{
+	struct max77802_regulator_prv *max77802 = platform_get_drvdata(pdev);
+
+	if (max77802->missing_of_node)
+		of_node_put(pdev->dev.of_node);
+
+	return 0;
+}
+
 static const struct platform_device_id max77802_pmic_id[] = {
 	{"max77802-pmic", 0},
 	{ },
@@ -648,6 +613,7 @@ static struct platform_driver max77802_pmic_driver = {
 		.owner = THIS_MODULE,
 	},
 	.probe = max77802_pmic_probe,
+	.remove = max77802_pmic_remove,
 	.id_table = max77802_pmic_id,
 };
 
-- 
1.9.1

Re: [RFT v3 06/14] regulator: max77802: Remove support for board files

[Javier Martinez Canillas]

Hello Krzysztof,

On 10/30/2014 12:20 PM, Krzysztof Kozlowski wrote:
>  static int max77802_pmic_probe(struct platform_device *pdev)
>  {
>  	struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
> -	struct max77686_platform_data *pdata = dev_get_platdata(iodev->dev);
>  	struct max77802_regulator_prv *max77802;
> -	int i, ret = 0, val;
> +	int i, val;
>  	struct regulator_config config = { };
>  
> -	/* This is allocated by the MFD driver */
> -	if (!pdata) {
> -		dev_err(&pdev->dev, "no platform data found for regulator\n");
> -		return -ENODEV;
> -	}
> -
>  	max77802 = devm_kzalloc(&pdev->dev,
>  				sizeof(struct max77802_regulator_prv),
>  				GFP_KERNEL);
>  	if (!max77802)
>  		return -ENOMEM;
>  
> -	if (iodev->dev->of_node) {
> -		ret = max77802_pmic_dt_parse_pdata(pdev, pdata);
> -		if (ret)
> -			return ret;
> +	if (!pdev->dev.of_node) {
> +		/*
> +		 * Backward DTS compatiblity where regulator driver had not
> +		 * a compatible property for itself.
> +		 */
> +		if (!iodev->dev->of_node) {
> +			dev_err(&pdev->dev, "No OF node for driver and its parent\n");
> +			return -EINVAL;
> +		}
> +		pdev->dev.of_node = of_get_child_by_name(iodev->dev->of_node,
> +								"regulators");
> +		max77802->missing_of_node = true;
>  	}

I may be missing something but I don't understand why a compatible string
for the regulators sub-node is needed. Isn't enough to just fill the
.regulators_node field in the struct regulator_desc? e.g:

    .regulators_node = of_match_ptr("regulators") for max77802
    .regulators_node = of_match_ptr("voltage-regulators") for max77686

AFAIU this should be enough for the core to extract the init_data and will
make your change much more simpler and you can drop patches 1-3 and 13-14.

Or maybe I misread the regulator_of_get_init_data() function?

Best regards,
Javier

Re: [RFT v3 06/14] regulator: max77802: Remove support for board files

[Krzysztof Kozlowski]

On czw, 2014-10-30 at 12:50 +0100, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 10/30/2014 12:20 PM, Krzysztof Kozlowski wrote:
> >  static int max77802_pmic_probe(struct platform_device *pdev)
> >  {
> >  	struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
> > -	struct max77686_platform_data *pdata = dev_get_platdata(iodev->dev);
> >  	struct max77802_regulator_prv *max77802;
> > -	int i, ret = 0, val;
> > +	int i, val;
> >  	struct regulator_config config = { };
> >  
> > -	/* This is allocated by the MFD driver */
> > -	if (!pdata) {
> > -		dev_err(&pdev->dev, "no platform data found for regulator\n");
> > -		return -ENODEV;
> > -	}
> > -
> >  	max77802 = devm_kzalloc(&pdev->dev,
> >  				sizeof(struct max77802_regulator_prv),
> >  				GFP_KERNEL);
> >  	if (!max77802)
> >  		return -ENOMEM;
> >  
> > -	if (iodev->dev->of_node) {
> > -		ret = max77802_pmic_dt_parse_pdata(pdev, pdata);
> > -		if (ret)
> > -			return ret;
> > +	if (!pdev->dev.of_node) {
> > +		/*
> > +		 * Backward DTS compatiblity where regulator driver had not
> > +		 * a compatible property for itself.
> > +		 */
> > +		if (!iodev->dev->of_node) {
> > +			dev_err(&pdev->dev, "No OF node for driver and its parent\n");
> > +			return -EINVAL;
> > +		}
> > +		pdev->dev.of_node = of_get_child_by_name(iodev->dev->of_node,
> > +								"regulators");
> > +		max77802->missing_of_node = true;
> >  	}
> 
> I may be missing something but I don't understand why a compatible string
> for the regulators sub-node is needed. Isn't enough to just fill the
> .regulators_node field in the struct regulator_desc? e.g:
> 
>     .regulators_node = of_match_ptr("regulators") for max77802
>     .regulators_node = of_match_ptr("voltage-regulators") for max77686
> 
> AFAIU this should be enough for the core to extract the init_data and will
> make your change much more simpler and you can drop patches 1-3 and 13-14.
> 
> Or maybe I misread the regulator_of_get_init_data() function?

The regulator_of_get_init_data() searches from dev->of_node or its child
node.

But dev->of_node is NULL.

That's why of_compatible is needed.

Best regards,
Krzysztof

Re: [RFT v3 06/14] regulator: max77802: Remove support for board files

[Javier Martinez Canillas]

Hello Krzysztof,

On 10/30/2014 01:10 PM, Krzysztof Kozlowski wrote:
>> 
>> I may be missing something but I don't understand why a compatible string
>> for the regulators sub-node is needed. Isn't enough to just fill the
>> .regulators_node field in the struct regulator_desc? e.g:
>> 
>>     .regulators_node = of_match_ptr("regulators") for max77802
>>     .regulators_node = of_match_ptr("voltage-regulators") for max77686
>> 
>> AFAIU this should be enough for the core to extract the init_data and will
>> make your change much more simpler and you can drop patches 1-3 and 13-14.
>> 
>> Or maybe I misread the regulator_of_get_init_data() function?
> 
> The regulator_of_get_init_data() searches from dev->of_node or its child
> node.
> 
> But dev->of_node is NULL.
> 
> That's why of_compatible is needed.

Yes but regulator_register() does dev = config->dev and config->dev is set
to config.dev = iodev->dev in the driver probe function which is the
pdev->dev.parent (the PMIC struct device) that has an associated of_node.

So, regulator_of_get_init_data() will call of_get_child_by_name() passing
the PMIC of_node and the sub-node name that contains the regulators. That
is, whatever was set in desc->regulators_node and that should be enough.

Best regards,
Javier

Re: [RFT v3 06/14] regulator: max77802: Remove support for board files

[Krzysztof Kozlowski]

On czw, 2014-10-30 at 13:21 +0100, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 10/30/2014 01:10 PM, Krzysztof Kozlowski wrote:
> >> 
> >> I may be missing something but I don't understand why a compatible string
> >> for the regulators sub-node is needed. Isn't enough to just fill the
> >> .regulators_node field in the struct regulator_desc? e.g:
> >> 
> >>     .regulators_node = of_match_ptr("regulators") for max77802
> >>     .regulators_node = of_match_ptr("voltage-regulators") for max77686
> >> 
> >> AFAIU this should be enough for the core to extract the init_data and will
> >> make your change much more simpler and you can drop patches 1-3 and 13-14.
> >> 
> >> Or maybe I misread the regulator_of_get_init_data() function?
> > 
> > The regulator_of_get_init_data() searches from dev->of_node or its child
> > node.
> > 
> > But dev->of_node is NULL.
> > 
> > That's why of_compatible is needed.
> 
> Yes but regulator_register() does dev = config->dev and config->dev is set
> to config.dev = iodev->dev in the driver probe function which is the
> pdev->dev.parent (the PMIC struct device) that has an associated of_node.
> 
> So, regulator_of_get_init_data() will call of_get_child_by_name() passing
> the PMIC of_node and the sub-node name that contains the regulators. That
> is, whatever was set in desc->regulators_node and that should be enough.

I missed that one in max77802 (in max77686: config.dev = &pdev->dev).
Now I wonder if it is proper to attach regulators to driver's parent
device. Consider regulator_register, around line 3640:
	rdev->dev.parent = dev;
The parent of regulators will be equal to parent of regulator driver -
main MFD driver.

Best regards,
Krzysztof

Re: [RFT v3 06/14] regulator: max77802: Remove support for board files

[Javier Martinez Canillas]

Hello Krzysztof,

On 10/30/2014 01:30 PM, Krzysztof Kozlowski wrote:
tor_of_get_init_data() function?
>> > 
>> > The regulator_of_get_init_data() searches from dev->of_node or its child
>> > node.
>> > 
>> > But dev->of_node is NULL.
>> > 
>> > That's why of_compatible is needed.
>> 
>> Yes but regulator_register() does dev = config->dev and config->dev is set
>> to config.dev = iodev->dev in the driver probe function which is the
>> pdev->dev.parent (the PMIC struct device) that has an associated of_node.
>> 
>> So, regulator_of_get_init_data() will call of_get_child_by_name() passing
>> the PMIC of_node and the sub-node name that contains the regulators. That
>> is, whatever was set in desc->regulators_node and that should be enough.
> 
> I missed that one in max77802 (in max77686: config.dev = &pdev->dev).
> Now I wonder if it is proper to attach regulators to driver's parent
> device. Consider regulator_register, around line 3640:
> 	rdev->dev.parent = dev;
> The parent of regulators will be equal to parent of regulator driver -
> main MFD driver.
>

An early version of the max77802 regulator driver also set config.dev to
&pdev->dev but then Mark asked me if I was sure that it should not be the
MFD device instead.

I then read in regulator_register() that the rdev->dev.parent was set
to the device passed in config.dev as you said so I was convinced that it
should be the parent device instead.

Please take a look at [0] for that discussion.

Best regards,
Javier

[0]: https://lkml.org/lkml/2014/6/17/174

Re: [RFT v3 06/14] regulator: max77802: Remove support for board files

[Krzysztof Kozlowski]

On czw, 2014-10-30 at 13:42 +0100, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 10/30/2014 01:30 PM, Krzysztof Kozlowski wrote:
> tor_of_get_init_data() function?
> >> > 
> >> > The regulator_of_get_init_data() searches from dev->of_node or its child
> >> > node.
> >> > 
> >> > But dev->of_node is NULL.
> >> > 
> >> > That's why of_compatible is needed.
> >> 
> >> Yes but regulator_register() does dev = config->dev and config->dev is set
> >> to config.dev = iodev->dev in the driver probe function which is the
> >> pdev->dev.parent (the PMIC struct device) that has an associated of_node.
> >> 
> >> So, regulator_of_get_init_data() will call of_get_child_by_name() passing
> >> the PMIC of_node and the sub-node name that contains the regulators. That
> >> is, whatever was set in desc->regulators_node and that should be enough.
> > 
> > I missed that one in max77802 (in max77686: config.dev = &pdev->dev).
> > Now I wonder if it is proper to attach regulators to driver's parent
> > device. Consider regulator_register, around line 3640:
> > 	rdev->dev.parent = dev;
> > The parent of regulators will be equal to parent of regulator driver -
> > main MFD driver.
> >
> 
> An early version of the max77802 regulator driver also set config.dev to
> &pdev->dev but then Mark asked me if I was sure that it should not be the
> MFD device instead.
> 
> I then read in regulator_register() that the rdev->dev.parent was set
> to the device passed in config.dev as you said so I was convinced that it
> should be the parent device instead.
> 
> Please take a look at [0] for that discussion.

To me a intuitive structure would be:
MFD device
  |
  - clock device
     |
     - clock1
     - clock2
  - regulator device
     |
     - LDO1
     - LDO2
etc.

This also maps to structure in DTS. dev_err* messages and any
allocations should be done on behalf of regulator device, not parent.

Various drivers do this differently... The wm8* drivers set it mostly to
parent (MFD)...

I do not insists, especially because using parent's device would make
this driver simpler.

Mark, maybe you could shed light on it?


Best regards,
Krzysztof

Re: [RFT v3 06/14] regulator: max77802: Remove support for board files

[Javier Martinez Canillas]

Hello Krzysztof,

On 10/30/2014 01:53 PM, Krzysztof Kozlowski wrote:
> 
> To me a intuitive structure would be:
> MFD device
>   |
>   - clock device
>      |
>      - clock1
>      - clock2
>   - regulator device
>      |
>      - LDO1
>      - LDO2
> etc.
> 
> This also maps to structure in DTS. dev_err* messages and any
> allocations should be done on behalf of regulator device, not parent.
>
> Various drivers do this differently... The wm8* drivers set it mostly to
> parent (MFD)...
>
> I do not insists, especially because using parent's device would make
> this driver simpler.
>

Another reason to set it to the parent is to lookup the regulator input supply
node. The regulator_register() function does:

	if (supply) {
		struct regulator_dev *r;

		r = regulator_dev_lookup(dev, supply, &ret);

where dev = config->dev so that will determine on which device node your
input supplies have to be defined. For example on the Peach Pit DTS has this:

	max77802: max77802-pmic@9 {
		...
		inb1-supply = <&tps65090_dcdc2>;
		...
		inb10-supply = <&tps65090_dcdc1>;

		inl1-supply = <&buck5_reg>;
		...
		inl10-supply = <&buck7_reg>;
		...

		regulators {
		...
		};
	};

which is how most (all?) DTS define the input supplies. If you instead
do config.dev = &pdev->dev, then the input supplies have to be in the
"regulators" node which is not the standard AFAICT.

This is not a problem for max77686 because I see that DTS don't define
the input supplies but I guess that is because the power scheme is not
completely modeled.
 
> Mark, maybe you could shed light on it?
> 
> 
> Best regards,
> Krzysztof
> 
> 

Best regards,
Javier

[PATCH v3 07/14] regulator: max77686: Remove support for board files

[Krzysztof Kozlowski]

The driver is used only on Exynos based boards with DTS support.
Convert the driver to DTS-only version. Parse all regulators at once,
not one-by-one. Remove dependency on data provided by max77686 MFD
driver.

Use new DT style parsing method for regulators init data. The regulator
driver now should have its own of_compatible property. If it is not
present then it will use "voltage-regulators" sub-node of parent's
node.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/regulator/max77686.c | 110 +++++++++++++++----------------------------
 1 file changed, 39 insertions(+), 71 deletions(-)

diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
index 27c5f4556044..ac05b318e405 100644
--- a/drivers/regulator/max77686.c
+++ b/drivers/regulator/max77686.c
@@ -84,6 +84,7 @@ enum max77686_ramp_rate {
 struct max77686_data {
 	/* Array indexed by regulator id */
 	unsigned int opmode[MAX77686_REGULATORS];
+	bool missing_of_node;
 };
 
 static unsigned int max77686_get_opmode_shift(int id)
@@ -281,6 +282,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
 
 #define regulator_desc_ldo(num)		{				\
 	.name		= "LDO"#num,					\
+	.of_match	= of_match_ptr("LDO"#num),			\
 	.id		= MAX77686_LDO##num,				\
 	.ops		= &max77686_ops,				\
 	.type		= REGULATOR_VOLTAGE,				\
@@ -297,6 +299,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
 }
 #define regulator_desc_lpm_ldo(num)	{				\
 	.name		= "LDO"#num,					\
+	.of_match	= of_match_ptr("LDO"#num),			\
 	.id		= MAX77686_LDO##num,				\
 	.ops		= &max77686_ldo_ops,				\
 	.type		= REGULATOR_VOLTAGE,				\
@@ -313,6 +316,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
 }
 #define regulator_desc_ldo_low(num)		{			\
 	.name		= "LDO"#num,					\
+	.of_match	= of_match_ptr("LDO"#num),			\
 	.id		= MAX77686_LDO##num,				\
 	.ops		= &max77686_ldo_ops,				\
 	.type		= REGULATOR_VOLTAGE,				\
@@ -329,6 +333,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
 }
 #define regulator_desc_ldo1_low(num)		{			\
 	.name		= "LDO"#num,					\
+	.of_match	= of_match_ptr("LDO"#num),			\
 	.id		= MAX77686_LDO##num,				\
 	.ops		= &max77686_ops,				\
 	.type		= REGULATOR_VOLTAGE,				\
@@ -345,6 +350,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
 }
 #define regulator_desc_buck(num)		{			\
 	.name		= "BUCK"#num,					\
+	.of_match	= of_match_ptr("BUCK"#num),			\
 	.id		= MAX77686_BUCK##num,				\
 	.ops		= &max77686_ops,				\
 	.type		= REGULATOR_VOLTAGE,				\
@@ -360,6 +366,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
 }
 #define regulator_desc_buck1(num)		{			\
 	.name		= "BUCK"#num,					\
+	.of_match	= of_match_ptr("BUCK"#num),			\
 	.id		= MAX77686_BUCK##num,				\
 	.ops		= &max77686_buck1_ops,				\
 	.type		= REGULATOR_VOLTAGE,				\
@@ -375,6 +382,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
 }
 #define regulator_desc_buck_dvs(num)		{			\
 	.name		= "BUCK"#num,					\
+	.of_match	= of_match_ptr("BUCK"#num),			\
 	.id		= MAX77686_BUCK##num,				\
 	.ops		= &max77686_buck_dvs_ops,			\
 	.type		= REGULATOR_VOLTAGE,				\
@@ -428,85 +436,34 @@ static const struct regulator_desc regulators[] = {
 	regulator_desc_buck(9),
 };
 
-#ifdef CONFIG_OF
-static int max77686_pmic_dt_parse_pdata(struct platform_device *pdev,
-					struct max77686_platform_data *pdata)
-{
-	struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
-	struct device_node *pmic_np, *regulators_np;
-	struct max77686_regulator_data *rdata;
-	struct of_regulator_match rmatch;
-	unsigned int i;
-
-	pmic_np = iodev->dev->of_node;
-	regulators_np = of_get_child_by_name(pmic_np, "voltage-regulators");
-	if (!regulators_np) {
-		dev_err(&pdev->dev, "could not find regulators sub-node\n");
-		return -EINVAL;
-	}
-
-	pdata->num_regulators = ARRAY_SIZE(regulators);
-	rdata = devm_kzalloc(&pdev->dev, sizeof(*rdata) *
-			     pdata->num_regulators, GFP_KERNEL);
-	if (!rdata) {
-		of_node_put(regulators_np);
-		return -ENOMEM;
-	}
-
-	for (i = 0; i < pdata->num_regulators; i++) {
-		rmatch.name = regulators[i].name;
-		rmatch.init_data = NULL;
-		rmatch.of_node = NULL;
-		of_regulator_match(&pdev->dev, regulators_np, &rmatch, 1);
-		rdata[i].initdata = rmatch.init_data;
-		rdata[i].of_node = rmatch.of_node;
-	}
-
-	pdata->regulators = rdata;
-	of_node_put(regulators_np);
-
-	return 0;
-}
-#else
-static int max77686_pmic_dt_parse_pdata(struct platform_device *pdev,
-					struct max77686_platform_data *pdata)
-{
-	return 0;
-}
-#endif /* CONFIG_OF */
-
 static int max77686_pmic_probe(struct platform_device *pdev)
 {
 	struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
-	struct max77686_platform_data *pdata = dev_get_platdata(iodev->dev);
 	struct max77686_data *max77686;
-	int i, ret = 0;
+	int i;
 	struct regulator_config config = { };
 
 	dev_dbg(&pdev->dev, "%s\n", __func__);
 
-	if (!pdata) {
-		dev_err(&pdev->dev, "no platform data found for regulator\n");
-		return -ENODEV;
-	}
-
-	if (iodev->dev->of_node) {
-		ret = max77686_pmic_dt_parse_pdata(pdev, pdata);
-		if (ret)
-			return ret;
-	}
-
-	if (pdata->num_regulators != MAX77686_REGULATORS) {
-		dev_err(&pdev->dev,
-			"Invalid initial data for regulator's initialiation\n");
-		return -EINVAL;
-	}
-
 	max77686 = devm_kzalloc(&pdev->dev, sizeof(struct max77686_data),
 				GFP_KERNEL);
 	if (!max77686)
 		return -ENOMEM;
 
+	if (!pdev->dev.of_node) {
+		/*
+		 * Backward DTS compatiblity where regulator driver had not
+		 * a compatible property for itself.
+		 */
+		if (!iodev->dev->of_node) {
+			dev_err(&pdev->dev, "No OF node for driver and its parent\n");
+			return -EINVAL;
+		}
+		pdev->dev.of_node = of_get_child_by_name(iodev->dev->of_node,
+				"voltage-regulators");
+		max77686->missing_of_node = true;
+	}
+
 	config.dev = &pdev->dev;
 	config.regmap = iodev->regmap;
 	config.driver_data = max77686;
@@ -516,23 +473,33 @@ static int max77686_pmic_probe(struct platform_device *pdev)
 		struct regulator_dev *rdev;
 		int id = regulators[i].id;
 
-		config.init_data = pdata->regulators[i].initdata;
-		config.of_node = pdata->regulators[i].of_node;
-
 		max77686->opmode[id] = regulators[i].enable_mask >>
 						max77686_get_opmode_shift(id);
 		rdev = devm_regulator_register(&pdev->dev,
 						&regulators[i], &config);
 		if (IS_ERR(rdev)) {
+			int ret = PTR_ERR(rdev);
 			dev_err(&pdev->dev,
-				"regulator init failed for %d\n", i);
-			return PTR_ERR(rdev);
+				"regulator init failed for %d: %d\n", i, ret);
+			if (max77686->missing_of_node)
+				of_node_put(pdev->dev.of_node);
+			return ret;
 		}
 	}
 
 	return 0;
 }
 
+static int max77686_pmic_remove(struct platform_device *pdev)
+{
+	struct max77686_data *max77686 = platform_get_drvdata(pdev);
+
+	if (max77686->missing_of_node)
+		of_node_put(pdev->dev.of_node);
+
+	return 0;
+}
+
 static const struct platform_device_id max77686_pmic_id[] = {
 	{"max77686-pmic", 0},
 	{ },
@@ -545,6 +512,7 @@ static struct platform_driver max77686_pmic_driver = {
 		.owner = THIS_MODULE,
 	},
 	.probe = max77686_pmic_probe,
+	.remove = max77686_pmic_remove,
 	.id_table = max77686_pmic_id,
 };
 
-- 
1.9.1

[PATCH v3 08/14] mfd: max77686/802: Remove support for board files

[Krzysztof Kozlowski]

The driver is used only on Exynos based boards with DTS support.
Convert the driver to DTS-only version. This simplifies a little the
code:
1. No dead (unused) entries in platform_data structure.
2. More code removed.
3. Regulator driver does not depend on allocated memory
   from MFD driver.
4. It makes also easier extending the regulator driver.

Add to the max77686 MFD driver dependency on CONFIG_OF because without
DTS the regulator drivers won't bind.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/mfd/Kconfig                  |  1 +
 drivers/mfd/max77686.c               | 23 -----------------------
 include/linux/mfd/max77686-private.h |  1 -
 include/linux/mfd/max77686.h         | 28 ----------------------------
 4 files changed, 1 insertion(+), 52 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index cbdb10918af1..fd9d19ccf8c0 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -401,6 +401,7 @@ config MFD_MAX14577
 config MFD_MAX77686
 	bool "Maxim Semiconductor MAX77686/802 PMIC Support"
 	depends on I2C=y
+	depends on OF
 	select MFD_CORE
 	select REGMAP_I2C
 	select REGMAP_IRQ
diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
index 9e1046bdef90..418b08b6c066 100644
--- a/drivers/mfd/max77686.c
+++ b/drivers/mfd/max77686.c
@@ -211,24 +211,10 @@ static const struct of_device_id max77686_pmic_dt_match[] = {
 	{ },
 };
 
-static struct max77686_platform_data *max77686_i2c_parse_dt_pdata(struct device
-								  *dev)
-{
-	struct max77686_platform_data *pd;
-
-	pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
-	if (!pd)
-		return NULL;
-
-	dev->platform_data = pd;
-	return pd;
-}
-
 static int max77686_i2c_probe(struct i2c_client *i2c,
 			      const struct i2c_device_id *id)
 {
 	struct max77686_dev *max77686 = NULL;
-	struct max77686_platform_data *pdata = dev_get_platdata(&i2c->dev);
 	const struct of_device_id *match;
 	unsigned int data;
 	int ret = 0;
@@ -239,14 +225,6 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
 	const struct mfd_cell *cells;
 	int n_devs;
 
-	if (IS_ENABLED(CONFIG_OF) && i2c->dev.of_node && !pdata)
-		pdata = max77686_i2c_parse_dt_pdata(&i2c->dev);
-
-	if (!pdata) {
-		dev_err(&i2c->dev, "No platform data found.\n");
-		return -EINVAL;
-	}
-
 	max77686 = devm_kzalloc(&i2c->dev,
 				sizeof(struct max77686_dev), GFP_KERNEL);
 	if (!max77686)
@@ -265,7 +243,6 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
 	max77686->dev = &i2c->dev;
 	max77686->i2c = i2c;
 
-	max77686->wakeup = pdata->wakeup;
 	max77686->irq = i2c->irq;
 
 	if (max77686->type == TYPE_MAX77686) {
diff --git a/include/linux/mfd/max77686-private.h b/include/linux/mfd/max77686-private.h
index 960b92ad450d..f5043490d67c 100644
--- a/include/linux/mfd/max77686-private.h
+++ b/include/linux/mfd/max77686-private.h
@@ -447,7 +447,6 @@ struct max77686_dev {
 	struct regmap_irq_chip_data *rtc_irq_data;
 
 	int irq;
-	bool wakeup;
 	struct mutex irqlock;
 	int irq_masks_cur[MAX77686_IRQ_GROUP_NR];
 	int irq_masks_cache[MAX77686_IRQ_GROUP_NR];
diff --git a/include/linux/mfd/max77686.h b/include/linux/mfd/max77686.h
index 553f7d09258a..bb995ab9a575 100644
--- a/include/linux/mfd/max77686.h
+++ b/include/linux/mfd/max77686.h
@@ -119,12 +119,6 @@ enum max77802_regulators {
 	MAX77802_REG_MAX,
 };
 
-struct max77686_regulator_data {
-	int id;
-	struct regulator_init_data *initdata;
-	struct device_node *of_node;
-};
-
 enum max77686_opmode {
 	MAX77686_OPMODE_NORMAL,
 	MAX77686_OPMODE_LP,
@@ -136,26 +130,4 @@ struct max77686_opmode_data {
 	int mode;
 };
 
-struct max77686_platform_data {
-	int ono;
-	int wakeup;
-
-	/* ---- PMIC ---- */
-	struct max77686_regulator_data *regulators;
-	int num_regulators;
-
-	struct max77686_opmode_data *opmode_data;
-
-	/*
-	 * GPIO-DVS feature is not enabled with the current version of
-	 * MAX77686 driver. Buck2/3/4_voltages[0] is used as the default
-	 * voltage at probe. DVS/SELB gpios are set as OUTPUT-LOW.
-	 */
-	int buck234_gpio_dvs[3]; /* GPIO of [0]DVS1, [1]DVS2, [2]DVS3 */
-	int buck234_gpio_selb[3]; /* [0]SELB2, [1]SELB3, [2]SELB4 */
-	unsigned int buck2_voltage[8]; /* buckx_voltage in uV */
-	unsigned int buck3_voltage[8];
-	unsigned int buck4_voltage[8];
-};
-
 #endif /* __LINUX_MFD_MAX77686_H */
-- 
1.9.1

Re: [PATCH v3 08/14] mfd: max77686/802: Remove support for board files

[Lee Jones]

On Thu, 30 Oct 2014, Krzysztof Kozlowski wrote:

> The driver is used only on Exynos based boards with DTS support.
> Convert the driver to DTS-only version. This simplifies a little the
> code:
> 1. No dead (unused) entries in platform_data structure.
> 2. More code removed.
> 3. Regulator driver does not depend on allocated memory
>    from MFD driver.
> 4. It makes also easier extending the regulator driver.
> 
> Add to the max77686 MFD driver dependency on CONFIG_OF because without
> DTS the regulator drivers won't bind.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  drivers/mfd/Kconfig                  |  1 +
>  drivers/mfd/max77686.c               | 23 -----------------------
>  include/linux/mfd/max77686-private.h |  1 -
>  include/linux/mfd/max77686.h         | 28 ----------------------------
>  4 files changed, 1 insertion(+), 52 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index cbdb10918af1..fd9d19ccf8c0 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -401,6 +401,7 @@ config MFD_MAX14577
>  config MFD_MAX77686
>  	bool "Maxim Semiconductor MAX77686/802 PMIC Support"
>  	depends on I2C=y
> +	depends on OF
>  	select MFD_CORE
>  	select REGMAP_I2C
>  	select REGMAP_IRQ
> diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
> index 9e1046bdef90..418b08b6c066 100644
> --- a/drivers/mfd/max77686.c
> +++ b/drivers/mfd/max77686.c
> @@ -211,24 +211,10 @@ static const struct of_device_id max77686_pmic_dt_match[] = {
>  	{ },
>  };
>  
> -static struct max77686_platform_data *max77686_i2c_parse_dt_pdata(struct device
> -								  *dev)
> -{
> -	struct max77686_platform_data *pd;
> -
> -	pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
> -	if (!pd)
> -		return NULL;
> -
> -	dev->platform_data = pd;
> -	return pd;
> -}
> -
>  static int max77686_i2c_probe(struct i2c_client *i2c,
>  			      const struct i2c_device_id *id)
>  {
>  	struct max77686_dev *max77686 = NULL;
> -	struct max77686_platform_data *pdata = dev_get_platdata(&i2c->dev);
>  	const struct of_device_id *match;
>  	unsigned int data;
>  	int ret = 0;
> @@ -239,14 +225,6 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
>  	const struct mfd_cell *cells;
>  	int n_devs;
>  
> -	if (IS_ENABLED(CONFIG_OF) && i2c->dev.of_node && !pdata)
> -		pdata = max77686_i2c_parse_dt_pdata(&i2c->dev);
> -
> -	if (!pdata) {
> -		dev_err(&i2c->dev, "No platform data found.\n");
> -		return -EINVAL;
> -	}
> -
>  	max77686 = devm_kzalloc(&i2c->dev,
>  				sizeof(struct max77686_dev), GFP_KERNEL);
>  	if (!max77686)
> @@ -265,7 +243,6 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
>  	max77686->dev = &i2c->dev;
>  	max77686->i2c = i2c;
>  
> -	max77686->wakeup = pdata->wakeup;
>  	max77686->irq = i2c->irq;
>  
>  	if (max77686->type == TYPE_MAX77686) {
> diff --git a/include/linux/mfd/max77686-private.h b/include/linux/mfd/max77686-private.h
> index 960b92ad450d..f5043490d67c 100644
> --- a/include/linux/mfd/max77686-private.h
> +++ b/include/linux/mfd/max77686-private.h
> @@ -447,7 +447,6 @@ struct max77686_dev {
>  	struct regmap_irq_chip_data *rtc_irq_data;
>  
>  	int irq;
> -	bool wakeup;
>  	struct mutex irqlock;
>  	int irq_masks_cur[MAX77686_IRQ_GROUP_NR];
>  	int irq_masks_cache[MAX77686_IRQ_GROUP_NR];
> diff --git a/include/linux/mfd/max77686.h b/include/linux/mfd/max77686.h
> index 553f7d09258a..bb995ab9a575 100644
> --- a/include/linux/mfd/max77686.h
> +++ b/include/linux/mfd/max77686.h
> @@ -119,12 +119,6 @@ enum max77802_regulators {
>  	MAX77802_REG_MAX,
>  };
>  
> -struct max77686_regulator_data {
> -	int id;
> -	struct regulator_init_data *initdata;
> -	struct device_node *of_node;
> -};
> -
>  enum max77686_opmode {
>  	MAX77686_OPMODE_NORMAL,
>  	MAX77686_OPMODE_LP,
> @@ -136,26 +130,4 @@ struct max77686_opmode_data {
>  	int mode;
>  };
>  
> -struct max77686_platform_data {
> -	int ono;
> -	int wakeup;
> -
> -	/* ---- PMIC ---- */
> -	struct max77686_regulator_data *regulators;
> -	int num_regulators;
> -
> -	struct max77686_opmode_data *opmode_data;
> -
> -	/*
> -	 * GPIO-DVS feature is not enabled with the current version of
> -	 * MAX77686 driver. Buck2/3/4_voltages[0] is used as the default
> -	 * voltage at probe. DVS/SELB gpios are set as OUTPUT-LOW.
> -	 */
> -	int buck234_gpio_dvs[3]; /* GPIO of [0]DVS1, [1]DVS2, [2]DVS3 */
> -	int buck234_gpio_selb[3]; /* [0]SELB2, [1]SELB3, [2]SELB4 */
> -	unsigned int buck2_voltage[8]; /* buckx_voltage in uV */
> -	unsigned int buck3_voltage[8];
> -	unsigned int buck4_voltage[8];
> -};
> -
>  #endif /* __LINUX_MFD_MAX77686_H */

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

Re: [PATCH v3 08/14] mfd: max77686/802: Remove support for board files

[Krzysztof Kozlowski]

On pon, 2014-11-03 at 17:01 +0000, Lee Jones wrote:
> On Thu, 30 Oct 2014, Krzysztof Kozlowski wrote:
> 
> > The driver is used only on Exynos based boards with DTS support.
> > Convert the driver to DTS-only version. This simplifies a little the
> > code:
> > 1. No dead (unused) entries in platform_data structure.
> > 2. More code removed.
> > 3. Regulator driver does not depend on allocated memory
> >    from MFD driver.
> > 4. It makes also easier extending the regulator driver.
> > 
> > Add to the max77686 MFD driver dependency on CONFIG_OF because without
> > DTS the regulator drivers won't bind.
> > 
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> > ---
> >  drivers/mfd/Kconfig                  |  1 +
> >  drivers/mfd/max77686.c               | 23 -----------------------
> >  include/linux/mfd/max77686-private.h |  1 -
> >  include/linux/mfd/max77686.h         | 28 ----------------------------
> >  4 files changed, 1 insertion(+), 52 deletions(-)
> 
> Applied, thanks.


Noooo, don't apply it. It depends on previous patches removing the board
file support from regulators.

If the patch looks OK, please only ack it. I would like to push
everything through regulator tree.

Best regards,
Krzysztof

Re: [PATCH v3 08/14] mfd: max77686/802: Remove support for board files

[Lee Jones]

On Tue, 04 Nov 2014, Krzysztof Kozlowski wrote:

> On pon, 2014-11-03 at 17:01 +0000, Lee Jones wrote:
> > On Thu, 30 Oct 2014, Krzysztof Kozlowski wrote:
> > 
> > > The driver is used only on Exynos based boards with DTS support.
> > > Convert the driver to DTS-only version. This simplifies a little the
> > > code:
> > > 1. No dead (unused) entries in platform_data structure.
> > > 2. More code removed.
> > > 3. Regulator driver does not depend on allocated memory
> > >    from MFD driver.
> > > 4. It makes also easier extending the regulator driver.
> > > 
> > > Add to the max77686 MFD driver dependency on CONFIG_OF because without
> > > DTS the regulator drivers won't bind.
> > > 
> > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > > Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> > > ---
> > >  drivers/mfd/Kconfig                  |  1 +
> > >  drivers/mfd/max77686.c               | 23 -----------------------
> > >  include/linux/mfd/max77686-private.h |  1 -
> > >  include/linux/mfd/max77686.h         | 28 ----------------------------
> > >  4 files changed, 1 insertion(+), 52 deletions(-)
> > 
> > Applied, thanks.
> 
> 
> Noooo, don't apply it. It depends on previous patches removing the board
> file support from regulators.
> 
> If the patch looks OK, please only ack it. I would like to push
> everything through regulator tree.

Very well, patch dropped.

As long as Mark puts it on a succinct tagged branch it can go in
through his tree:

Acked-by: Lee Jones <lee.jones@linaro.org>

Mark,
  Please send me the tag to pull from once applied, thanks.

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

[PATCH v3 09/14] regulator: max77686: Initialize opmode explicitly to normal mode

[Krzysztof Kozlowski]

Minor nit: Initialize the opmode for each regulator to normal mode in a
readable explicit way.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Suggested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/regulator/max77686.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
index ac05b318e405..2c276d147c90 100644
--- a/drivers/regulator/max77686.c
+++ b/drivers/regulator/max77686.c
@@ -473,8 +473,8 @@ static int max77686_pmic_probe(struct platform_device *pdev)
 		struct regulator_dev *rdev;
 		int id = regulators[i].id;
 
-		max77686->opmode[id] = regulators[i].enable_mask >>
-						max77686_get_opmode_shift(id);
+		max77686->opmode[id] = MAX77686_NORMAL;
+
 		rdev = devm_regulator_register(&pdev->dev,
 						&regulators[i], &config);
 		if (IS_ERR(rdev)) {
-- 
1.9.1

[PATCH v3 10/14] regulator: max77686: Add GPIO control

[Krzysztof Kozlowski]

Add enable control over GPIO for regulators supporting this: LDO20,
LDO21, LDO22, buck8 and buck9.

This is needed for proper (and full) configuration of the Maxim 77686
PMIC without creating redundant 'regulator-fixed' entries.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/regulator/max77686.c | 83 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 78 insertions(+), 5 deletions(-)

diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
index 2c276d147c90..9108cde069fe 100644
--- a/drivers/regulator/max77686.c
+++ b/drivers/regulator/max77686.c
@@ -26,6 +26,7 @@
 #include <linux/bug.h>
 #include <linux/err.h>
 #include <linux/gpio.h>
+#include <linux/of_gpio.h>
 #include <linux/slab.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/driver.h>
@@ -46,6 +47,11 @@
 #define MAX77686_DVS_UVSTEP	12500
 
 /*
+ * Value for configuring buck[89] and LDO{20,21,22} as GPIO control.
+ * It is the same as 'off' for other regulators.
+ */
+#define MAX77686_GPIO_CONTROL		0x0
+/*
  * Values used for configuring LDOs and bucks.
  * Forcing low power mode: LDO1, 3-5, 9, 13, 17-26
  */
@@ -82,6 +88,9 @@ enum max77686_ramp_rate {
 };
 
 struct max77686_data {
+	/* GPIO control over regulators */
+	int gpio[MAX77686_REGULATORS];
+
 	/* Array indexed by regulator id */
 	unsigned int opmode[MAX77686_REGULATORS];
 	bool missing_of_node;
@@ -101,6 +110,25 @@ static unsigned int max77686_get_opmode_shift(int id)
 	}
 }
 
+/*
+ * When regulator is configured for GPIO control then it
+ * replaces "normal" mode. Any change from low power mode to normal
+ * should actually change to GPIO control.
+ * Map normal mode to proper value for such regulators.
+ */
+static int max77686_map_normal_mode(struct max77686_data *max77686, int id)
+{
+	switch (id) {
+	case MAX77686_BUCK8:
+	case MAX77686_BUCK9:
+	case MAX77686_LDO20 ... MAX77686_LDO22:
+		if (gpio_is_valid(max77686->gpio[id]))
+			return MAX77686_GPIO_CONTROL;
+	}
+
+	return MAX77686_NORMAL;
+}
+
 /* Some BUCKs and LDOs supports Normal[ON/OFF] mode during suspend */
 static int max77686_set_suspend_disable(struct regulator_dev *rdev)
 {
@@ -137,7 +165,7 @@ static int max77686_set_suspend_mode(struct regulator_dev *rdev,
 		val = MAX77686_LDO_LOWPOWER_PWRREQ;
 		break;
 	case REGULATOR_MODE_NORMAL:			/* ON in Normal Mode */
-		val = MAX77686_NORMAL;
+		val = max77686_map_normal_mode(max77686, id);
 		break;
 	default:
 		pr_warn("%s: regulator_suspend_mode : 0x%x not supported\n",
@@ -161,7 +189,7 @@ static int max77686_ldo_set_suspend_mode(struct regulator_dev *rdev,
 {
 	unsigned int val;
 	struct max77686_data *max77686 = rdev_get_drvdata(rdev);
-	int ret;
+	int ret, id = rdev_get_id(rdev);
 
 	switch (mode) {
 	case REGULATOR_MODE_STANDBY:			/* switch off */
@@ -171,7 +199,7 @@ static int max77686_ldo_set_suspend_mode(struct regulator_dev *rdev,
 		val = MAX77686_LDO_LOWPOWER_PWRREQ;
 		break;
 	case REGULATOR_MODE_NORMAL:			/* ON in Normal Mode */
-		val = MAX77686_NORMAL;
+		val = max77686_map_normal_mode(max77686, id);
 		break;
 	default:
 		pr_warn("%s: regulator_suspend_mode : 0x%x not supported\n",
@@ -185,7 +213,7 @@ static int max77686_ldo_set_suspend_mode(struct regulator_dev *rdev,
 	if (ret)
 		return ret;
 
-	max77686->opmode[rdev_get_id(rdev)] = val;
+	max77686->opmode[id] = val;
 	return 0;
 }
 
@@ -198,7 +226,7 @@ static int max77686_enable(struct regulator_dev *rdev)
 	shift = max77686_get_opmode_shift(id);
 
 	if (max77686->opmode[id] == MAX77686_OFF_PWRREQ)
-		max77686->opmode[id] = MAX77686_NORMAL;
+		max77686->opmode[id] = max77686_map_normal_mode(max77686, id);
 
 	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
 				  rdev->desc->enable_mask,
@@ -436,6 +464,33 @@ static const struct regulator_desc regulators[] = {
 	regulator_desc_buck(9),
 };
 
+static int max77686_enable_gpio_control(struct regulator_dev *rdev)
+{
+	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
+					rdev->desc->enable_mask,
+					MAX77686_GPIO_CONTROL);
+}
+
+static void max77686_pmic_dt_parse_gpio_control(struct platform_device *pdev,
+					int *gpio)
+{
+	struct device_node *np = pdev->dev.of_node;
+	int i;
+
+	/*
+	 * 0 is a valid GPIO so initialize all GPIOs to negative value
+	 * to indicate that GPIO control won't be used for this regulator.
+	 */
+	for (i = 0; i < MAX77686_REGULATORS; i++)
+		gpio[i] = -EINVAL;
+
+	gpio[MAX77686_LDO20] = of_get_named_gpio(np, "ldo20-gpio", 0);
+	gpio[MAX77686_LDO21] = of_get_named_gpio(np, "ldo21-gpio", 0);
+	gpio[MAX77686_LDO22] = of_get_named_gpio(np, "ldo22-gpio", 0);
+	gpio[MAX77686_BUCK8] = of_get_named_gpio(np, "buck8-gpio", 0);
+	gpio[MAX77686_BUCK9] = of_get_named_gpio(np, "buck9-gpio", 0);
+}
+
 static int max77686_pmic_probe(struct platform_device *pdev)
 {
 	struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
@@ -467,12 +522,17 @@ static int max77686_pmic_probe(struct platform_device *pdev)
 	config.dev = &pdev->dev;
 	config.regmap = iodev->regmap;
 	config.driver_data = max77686;
+	config.ena_gpio_flags = GPIOF_OUT_INIT_HIGH;
+	config.ena_gpio_initialized = true;
 	platform_set_drvdata(pdev, max77686);
 
+	max77686_pmic_dt_parse_gpio_control(pdev, max77686->gpio);
+
 	for (i = 0; i < MAX77686_REGULATORS; i++) {
 		struct regulator_dev *rdev;
 		int id = regulators[i].id;
 
+		config.ena_gpio = max77686->gpio[id];
 		max77686->opmode[id] = MAX77686_NORMAL;
 
 		rdev = devm_regulator_register(&pdev->dev,
@@ -485,6 +545,19 @@ static int max77686_pmic_probe(struct platform_device *pdev)
 				of_node_put(pdev->dev.of_node);
 			return ret;
 		}
+
+		if (gpio_is_valid(config.ena_gpio)) {
+			int ret = max77686_enable_gpio_control(rdev);
+
+			if (ret < 0) {
+				dev_err(&pdev->dev,
+					"regulator enable ext control failed for %d\n",
+					i);
+				if (max77686->missing_of_node)
+					of_node_put(pdev->dev.of_node);
+				return ret;
+			}
+		}
 	}
 
 	return 0;
-- 
1.9.1

[PATCH v3 11/14] mfd/regulator: dt-bindings: max77686: Document gpio properties

[Krzysztof Kozlowski]

Document usage of gpio properties which turn on external/GPIO control
over regulator.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 Documentation/devicetree/bindings/mfd/max77686.txt | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/max77686.txt b/Documentation/devicetree/bindings/mfd/max77686.txt
index f4010a9f66eb..b8013e69da65 100644
--- a/Documentation/devicetree/bindings/mfd/max77686.txt
+++ b/Documentation/devicetree/bindings/mfd/max77686.txt
@@ -22,7 +22,14 @@ Optional node:
   Required property:
   - compatible : Must be "maxim,max77686-pmic"
 
-  Optional properties: For each regulator use following format:
+  Optional properties:
+  - ldo20-gpio : GPIO to use for LDO20 enable control.
+  - ldo21-gpio : GPIO to use for LDO21 enable control.
+  - ldo22-gpio : GPIO to use for LDO22 enable control.
+  - buck8-gpio : GPIO to use for BUCK8 enable control.
+  - buck9-gpio : GPIO to use for BUCK9 enable control.
+
+  For each regulator use following format:
 	regulator_name {
 		regulator-compatible = LDOn/BUCKn
 		standard regulator constraints....
@@ -42,7 +49,6 @@ to get matched with their hardware counterparts as follow:
 	-BUCKn	:	1-4.
   Use standard regulator bindings for it ('regulator-off-in-suspend').
 
-
 Example:
 
 	max77686@09 {
@@ -53,6 +59,7 @@ Example:
 
 		voltage-regulators {
 			compatible = "maxim,max77686-pmic";
+			ldo21-gpio = <&gpy2 0 GPIO_ACTIVE_HIGH>;
 
 			ldo11_reg {
 				regulator-compatible = "LDO11";
-- 
1.9.1

[PATCH v3 12/14] ARM: dts: exynos4412-trats: Switch max77686 regulators to GPIO control

[Krzysztof Kozlowski]

Remove fixed regulators (duplicating what max77686 provides) and
add GPIO control to max77686 regulators. Add of_compatible to
voltage-regulators node.

This gives the system full control over those regulators. Previously
the state of such regulators was a mixture of what max77686 driver set
over I2C and what regulator-fixed set through GPIO.

Removal of 'regulator-always-on' from CAM_ISP_CORE_1.2V (buck9) allows
disabling it when it is not used. Previously this regulator was always
enabled because its enable state is a OR of:
 - ENB9 GPIO (turned by regulator-fixed),
 - BUCK9EN field in BUCK9CTRL register (max77686 through I2C).

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 arch/arm/boot/dts/exynos4412-trats2.dts | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts
index 7a68e0832cd6..ebbd12d1fe9a 100644
--- a/arch/arm/boot/dts/exynos4412-trats2.dts
+++ b/arch/arm/boot/dts/exynos4412-trats2.dts
@@ -58,15 +58,6 @@
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		vemmc_reg: regulator-0 {
-			compatible = "regulator-fixed";
-			regulator-name = "VMEM_VDD_2.8V";
-			regulator-min-microvolt = <2800000>;
-			regulator-max-microvolt = <2800000>;
-			gpio = <&gpk0 2 0>;
-			enable-active-high;
-		};
-
 		cam_io_reg: voltage-regulator-1 {
 			compatible = "regulator-fixed";
 			regulator-name = "CAM_SENSOR_A";
@@ -94,16 +85,6 @@
 			enable-active-high;
 		};
 
-		cam_isp_core_reg: voltage-regulator-4 {
-			compatible = "regulator-fixed";
-			regulator-name = "CAM_ISP_CORE_1.2V_EN";
-			regulator-min-microvolt = <1200000>;
-			regulator-max-microvolt = <1200000>;
-			gpio = <&gpm0 3 0>;
-			enable-active-high;
-			regulator-always-on;
-		};
-
 		ps_als_reg: voltage-regulator-5 {
 			compatible = "regulator-fixed";
 			regulator-name = "LED_A_3.0V";
@@ -221,6 +202,13 @@
 			#clock-cells = <1>;
 
 			voltage-regulators {
+				compatible = "maxim,max77686-pmic";
+
+				ldo21-gpio = <&gpy2 0 GPIO_ACTIVE_HIGH>;
+				ldo22-gpio = <&gpk0 2 GPIO_ACTIVE_HIGH>;
+				buck8-gpio = <&gpk0 2 GPIO_ACTIVE_HIGH>;
+				buck9-gpio = <&gpm0 3 GPIO_ACTIVE_HIGH>;
+
 				ldo1_reg: ldo1 {
 					regulator-compatible = "LDO1";
 					regulator-name = "VALIVE_1.0V_AP";
@@ -591,7 +579,7 @@
 		broken-cd;
 		non-removable;
 		card-detect-delay = <200>;
-		vmmc-supply = <&vemmc_reg>;
+		vmmc-supply = <&ldo22_reg>;
 		clock-frequency = <400000000>;
 		samsung,dw-mshc-ciu-div = <0>;
 		samsung,dw-mshc-sdr-timing = <2 3>;
-- 
1.9.1

[PATCH v3 13/14] ARM: dts: exynos5420-peach: Update to new max77802 regulator compatible

[Krzysztof Kozlowski]

Update to new bindings for max77802 regulator driver by adding proper
compatible field for regulators node.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 arch/arm/boot/dts/exynos5420-peach-pit.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index 9a050e19a4dc..1bc25cd9b437 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -185,6 +185,8 @@
 		inl10-supply = <&buck7_reg>;
 
 		regulators {
+			compatible = "maxim,max77802-pmic";
+
 			buck1_reg: BUCK1 {
 				regulator-name = "vdd_mif";
 				regulator-min-microvolt = <800000>;
-- 
1.9.1

[PATCH v3 14/14] ARM: dts: exynos5800-peach: Update to new max77802 regulator compatible

[Krzysztof Kozlowski]

Update to new bindings for max77802 regulator driver by adding proper
compatible field for regulators node.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 arch/arm/boot/dts/exynos5800-peach-pi.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index e8fdda827fc9..689c6bc84fd3 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -184,6 +184,8 @@
 		inl10-supply = <&buck7_reg>;
 
 		regulators {
+			compatible = "maxim,max77802-pmic";
+
 			buck1_reg: BUCK1 {
 				regulator-name = "vdd_mif";
 				regulator-min-microvolt = <800000>;
-- 
1.9.1