[RFC PATCH 00/11] Device tree support for regulators

[RFC PATCH 00/11] Device tree support for regulators

[Rajendra Nayak]

Hi Grant, Mark,

This RFC is an attempt to move the regulator mappings from
board files into device tree for OMAP. In the process I have
defined some helper routines for regulators and defined
the bindings for these.
The patches are based on top of Benoit's series which adds
DT support for i2c and twl for OMAP from here
git://gitorious.org/omap-pm/linux.git for_3.2/5_omap_dt_i2c_twl

I know Benoit is planning a respin of the series soon, in which
he intends to register all twl child nodes as platform devices,
which would mean I just drop the last patch in this series.
The first couple of patches in the series are just fixes and
cleanups leading to the regulator DT migration.

The series is tested on OMAP4SDP and OMAP4PANDA boards.

regards,
Rajendra

Rajendra Nayak (11):
  OMAP: TWL: Clean up mode and ops mask passed from board files
  regulator: Fix error check in set_consumer_device_supply
  DT: regulator: Helper routine to extract regulator_init_data
  omap4: SDP: Pass regulator_init_data from DT
  TWL: regulator: Make twl-regulator driver extract data from DT
  DT: regulator: Helper routine to extract fixed_voltage_config
  regulator: Make fixed regulator driver extract data from DT
  omap4: panda: Pass fixed regulator data from DT
  DT: regulator: Helper to extract regulator node based on supply name
  regulator: Implement consumer regulator mapping from device tree
  DT: regulator: register regulators as platform devices

 .../devicetree/bindings/regulator/regulator.txt    |   56 ++++++
 .../bindings/regulator/twl-regulator.txt           |   18 ++
 arch/arm/boot/dts/omap4-panda.dts                  |   10 +
 arch/arm/boot/dts/omap4-sdp.dts                    |   16 ++
 arch/arm/mach-omap2/board-2430sdp.c                |    5 -
 arch/arm/mach-omap2/board-3430sdp.c                |   30 ----
 arch/arm/mach-omap2/board-4430sdp.c                |   10 -
 arch/arm/mach-omap2/board-cm-t35.c                 |   10 -
 arch/arm/mach-omap2/board-devkit8000.c             |   13 --
 arch/arm/mach-omap2/board-igep0020.c               |   11 --
 arch/arm/mach-omap2/board-ldp.c                    |    9 -
 arch/arm/mach-omap2/board-omap3beagle.c            |   10 -
 arch/arm/mach-omap2/board-omap3evm.c               |   14 --
 arch/arm/mach-omap2/board-omap3logic.c             |    5 -
 arch/arm/mach-omap2/board-omap3pandora.c           |   26 ---
 arch/arm/mach-omap2/board-omap3stalker.c           |    8 -
 arch/arm/mach-omap2/board-omap3touchbook.c         |   10 -
 arch/arm/mach-omap2/board-overo.c                  |    5 -
 arch/arm/mach-omap2/board-rm680.c                  |    4 -
 arch/arm/mach-omap2/board-rx51-peripherals.c       |   55 ------
 arch/arm/mach-omap2/board-zoom-peripherals.c       |   14 --
 arch/arm/mach-omap2/twl-common.c                   |   44 -----
 drivers/mfd/twl-core.c                             |    3 +
 drivers/of/Kconfig                                 |    6 +
 drivers/of/Makefile                                |    1 +
 drivers/of/of_regulator.c                          |  183 ++++++++++++++++++++
 drivers/regulator/core.c                           |   25 +++-
 drivers/regulator/fixed.c                          |   20 ++-
 drivers/regulator/twl-regulator.c                  |   28 +++-
 include/linux/of_regulator.h                       |   42 +++++
 include/linux/regulator/driver.h                   |    3 +
 31 files changed, 406 insertions(+), 288 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/regulator.txt
 create mode 100644 Documentation/devicetree/bindings/regulator/twl-regulator.txt
 create mode 100644 drivers/of/of_regulator.c
 create mode 100644 include/linux/of_regulator.h

[RFC PATCH 01/11] OMAP: TWL: Clean up mode and ops mask passed from board files

[Rajendra Nayak]

The TWL driver seems to set the default .valid_modes_mask to
(REGULATOR_MODE_NORMAL | REGULATOR_MODE_STANDBY) and .valid_ops_mask
to (REGULATOR_CHANGE_VOLTAGE | REGULATOR_CHANGE_MODE | REGULATOR_CHANGE_STATUS)
for all the registered regulators.

There is no need for all the board files to pass this information
additionally, when the driver already sets them by default.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/board-2430sdp.c          |    5 --
 arch/arm/mach-omap2/board-3430sdp.c          |   30 --------------
 arch/arm/mach-omap2/board-4430sdp.c          |   10 -----
 arch/arm/mach-omap2/board-cm-t35.c           |   10 -----
 arch/arm/mach-omap2/board-devkit8000.c       |   13 ------
 arch/arm/mach-omap2/board-igep0020.c         |   11 -----
 arch/arm/mach-omap2/board-ldp.c              |    9 ----
 arch/arm/mach-omap2/board-omap3beagle.c      |   10 -----
 arch/arm/mach-omap2/board-omap3evm.c         |   14 -------
 arch/arm/mach-omap2/board-omap3logic.c       |    5 --
 arch/arm/mach-omap2/board-omap3pandora.c     |   26 ------------
 arch/arm/mach-omap2/board-omap3stalker.c     |    8 ----
 arch/arm/mach-omap2/board-omap3touchbook.c   |   10 -----
 arch/arm/mach-omap2/board-overo.c            |    5 --
 arch/arm/mach-omap2/board-rm680.c            |    4 --
 arch/arm/mach-omap2/board-rx51-peripherals.c |   55 --------------------------
 arch/arm/mach-omap2/board-zoom-peripherals.c |   14 -------
 arch/arm/mach-omap2/twl-common.c             |   44 --------------------
 18 files changed, 0 insertions(+), 283 deletions(-)

diff --git a/arch/arm/mach-omap2/board-2430sdp.c b/arch/arm/mach-omap2/board-2430sdp.c
index 2028464..32154f4 100644
--- a/arch/arm/mach-omap2/board-2430sdp.c
+++ b/arch/arm/mach-omap2/board-2430sdp.c
@@ -156,11 +156,6 @@ static struct regulator_init_data sdp2430_vmmc1 = {
 	.constraints = {
 		.min_uV			= 1850000,
 		.max_uV			= 3150000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(sdp2430_vmmc1_supplies),
 	.consumer_supplies	= &sdp2430_vmmc1_supplies[0],
diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c
index bd600cf..01ce6df 100644
--- a/arch/arm/mach-omap2/board-3430sdp.c
+++ b/arch/arm/mach-omap2/board-3430sdp.c
@@ -306,10 +306,6 @@ static struct regulator_init_data sdp3430_vaux1 = {
 		.min_uV			= 2800000,
 		.max_uV			= 2800000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 };
 
@@ -319,10 +315,6 @@ static struct regulator_init_data sdp3430_vaux2 = {
 		.min_uV			= 2800000,
 		.max_uV			= 2800000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 };
 
@@ -332,10 +324,6 @@ static struct regulator_init_data sdp3430_vaux3 = {
 		.min_uV			= 2800000,
 		.max_uV			= 2800000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies		= ARRAY_SIZE(sdp3430_vaux3_supplies),
 	.consumer_supplies		= sdp3430_vaux3_supplies,
@@ -347,10 +335,6 @@ static struct regulator_init_data sdp3430_vaux4 = {
 		.min_uV			= 1800000,
 		.max_uV			= 1800000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 };
 
@@ -359,11 +343,6 @@ static struct regulator_init_data sdp3430_vmmc1 = {
 	.constraints = {
 		.min_uV			= 1850000,
 		.max_uV			= 3150000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(sdp3430_vmmc1_supplies),
 	.consumer_supplies	= sdp3430_vmmc1_supplies,
@@ -375,10 +354,6 @@ static struct regulator_init_data sdp3430_vmmc2 = {
 		.min_uV			= 1850000,
 		.max_uV			= 1850000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(sdp3430_vmmc2_supplies),
 	.consumer_supplies	= sdp3430_vmmc2_supplies,
@@ -389,11 +364,6 @@ static struct regulator_init_data sdp3430_vsim = {
 	.constraints = {
 		.min_uV			= 1800000,
 		.max_uV			= 3000000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(sdp3430_vsim_supplies),
 	.consumer_supplies	= sdp3430_vsim_supplies,
diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
index c7cef44..acfbed0 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -511,11 +511,6 @@ static struct regulator_init_data sdp4430_vaux1 = {
 		.min_uV			= 1000000,
 		.max_uV			= 3000000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask	 = REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies  = ARRAY_SIZE(sdp4430_vaux_supply),
 	.consumer_supplies      = sdp4430_vaux_supply,
@@ -526,11 +521,6 @@ static struct regulator_init_data sdp4430_vusim = {
 		.min_uV			= 1200000,
 		.max_uV			= 2900000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask	 = REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 };
 
diff --git a/arch/arm/mach-omap2/board-cm-t35.c b/arch/arm/mach-omap2/board-cm-t35.c
index 3af8aab..e6e909d 100644
--- a/arch/arm/mach-omap2/board-cm-t35.c
+++ b/arch/arm/mach-omap2/board-cm-t35.c
@@ -348,11 +348,6 @@ static struct regulator_init_data cm_t35_vmmc1 = {
 	.constraints = {
 		.min_uV			= 1850000,
 		.max_uV			= 3150000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(cm_t35_vmmc1_supply),
 	.consumer_supplies	= cm_t35_vmmc1_supply,
@@ -363,11 +358,6 @@ static struct regulator_init_data cm_t35_vsim = {
 	.constraints = {
 		.min_uV			= 1800000,
 		.max_uV			= 3000000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(cm_t35_vsim_supply),
 	.consumer_supplies	= cm_t35_vsim_supply,
diff --git a/arch/arm/mach-omap2/board-devkit8000.c b/arch/arm/mach-omap2/board-devkit8000.c
index b6002ec..7bfbe5f 100644
--- a/arch/arm/mach-omap2/board-devkit8000.c
+++ b/arch/arm/mach-omap2/board-devkit8000.c
@@ -275,11 +275,6 @@ static struct regulator_init_data devkit8000_vmmc1 = {
 	.constraints = {
 		.min_uV			= 1850000,
 		.max_uV			= 3150000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(devkit8000_vmmc1_supply),
 	.consumer_supplies	= devkit8000_vmmc1_supply,
@@ -290,10 +285,6 @@ static struct regulator_init_data devkit8000_vpll1 = {
 	.constraints = {
 		.min_uV			= 1800000,
 		.max_uV			= 1800000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(devkit8000_vpll1_supplies),
 	.consumer_supplies	= devkit8000_vpll1_supplies,
@@ -305,10 +296,6 @@ static struct regulator_init_data devkit8000_vio = {
 		.min_uV                 = 1800000,
 		.max_uV                 = 1800000,
 		.apply_uV               = true,
-		.valid_modes_mask       = REGULATOR_MODE_NORMAL
-			| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask         = REGULATOR_CHANGE_MODE
-			| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies  = ARRAY_SIZE(devkit8000_vio_supply),
 	.consumer_supplies      = devkit8000_vio_supply,
diff --git a/arch/arm/mach-omap2/board-igep0020.c b/arch/arm/mach-omap2/board-igep0020.c
index 2521823..3e0c54c 100644
--- a/arch/arm/mach-omap2/board-igep0020.c
+++ b/arch/arm/mach-omap2/board-igep0020.c
@@ -231,11 +231,6 @@ static struct regulator_init_data igep_vmmc1 = {
 	.constraints = {
 		.min_uV			= 1850000,
 		.max_uV			= 3150000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies  = ARRAY_SIZE(igep_vmmc1_supply),
 	.consumer_supplies      = igep_vmmc1_supply,
@@ -250,11 +245,6 @@ static struct regulator_init_data igep_vio = {
 		.min_uV			= 1800000,
 		.max_uV			= 1800000,
 		.apply_uV		= 1,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies  = ARRAY_SIZE(igep_vio_supply),
 	.consumer_supplies      = igep_vio_supply,
@@ -266,7 +256,6 @@ static struct regulator_consumer_supply igep_vmmc2_supply[] = {
 
 static struct regulator_init_data igep_vmmc2 = {
 	.constraints		= {
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL,
 		.always_on		= 1,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(igep_vmmc2_supply),
diff --git a/arch/arm/mach-omap2/board-ldp.c b/arch/arm/mach-omap2/board-ldp.c
index 218764c..8a81b92 100644
--- a/arch/arm/mach-omap2/board-ldp.c
+++ b/arch/arm/mach-omap2/board-ldp.c
@@ -214,11 +214,6 @@ static struct regulator_init_data ldp_vmmc1 = {
 	.constraints = {
 		.min_uV			= 1850000,
 		.max_uV			= 3150000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(ldp_vmmc1_supply),
 	.consumer_supplies	= ldp_vmmc1_supply,
@@ -235,10 +230,6 @@ static struct regulator_init_data ldp_vaux1 = {
 		.min_uV			= 3000000,
 		.max_uV			= 3000000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies		= ARRAY_SIZE(ldp_vaux1_supplies),
 	.consumer_supplies		= ldp_vaux1_supplies,
diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
index a7923ca..0da3c9c 100644
--- a/arch/arm/mach-omap2/board-omap3beagle.c
+++ b/arch/arm/mach-omap2/board-omap3beagle.c
@@ -338,11 +338,6 @@ static struct regulator_init_data beagle_vmmc1 = {
 	.constraints = {
 		.min_uV			= 1850000,
 		.max_uV			= 3150000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(beagle_vmmc1_supply),
 	.consumer_supplies	= beagle_vmmc1_supply,
@@ -353,11 +348,6 @@ static struct regulator_init_data beagle_vsim = {
 	.constraints = {
 		.min_uV			= 1800000,
 		.max_uV			= 3000000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(beagle_vsim_supply),
 	.consumer_supplies	= beagle_vsim_supply,
diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
index c452b3f..f2508dc 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -286,11 +286,6 @@ static struct regulator_init_data omap3evm_vmmc1 = {
 	.constraints = {
 		.min_uV			= 1850000,
 		.max_uV			= 3150000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(omap3evm_vmmc1_supply),
 	.consumer_supplies	= omap3evm_vmmc1_supply,
@@ -301,11 +296,6 @@ static struct regulator_init_data omap3evm_vsim = {
 	.constraints = {
 		.min_uV			= 1800000,
 		.max_uV			= 3000000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(omap3evm_vsim_supply),
 	.consumer_supplies	= omap3evm_vsim_supply,
@@ -441,10 +431,6 @@ static struct regulator_init_data omap3evm_vio = {
 		.min_uV			= 1800000,
 		.max_uV			= 1800000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(omap3evm_vio_supply),
 	.consumer_supplies	= omap3evm_vio_supply,
diff --git a/arch/arm/mach-omap2/board-omap3logic.c b/arch/arm/mach-omap2/board-omap3logic.c
index 703aeb5..90584e2 100644
--- a/arch/arm/mach-omap2/board-omap3logic.c
+++ b/arch/arm/mach-omap2/board-omap3logic.c
@@ -64,11 +64,6 @@ static struct regulator_init_data omap3logic_vmmc1 = {
 		.name			= "VMMC1",
 		.min_uV			= 1850000,
 		.max_uV			= 3150000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies  = ARRAY_SIZE(omap3logic_vmmc1_supply),
 	.consumer_supplies      = omap3logic_vmmc1_supply,
diff --git a/arch/arm/mach-omap2/board-omap3pandora.c b/arch/arm/mach-omap2/board-omap3pandora.c
index 080d7bd..37638d3 100644
--- a/arch/arm/mach-omap2/board-omap3pandora.c
+++ b/arch/arm/mach-omap2/board-omap3pandora.c
@@ -362,11 +362,6 @@ static struct regulator_init_data pandora_vmmc1 = {
 	.constraints = {
 		.min_uV			= 1850000,
 		.max_uV			= 3150000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(pandora_vmmc1_supply),
 	.consumer_supplies	= pandora_vmmc1_supply,
@@ -377,11 +372,6 @@ static struct regulator_init_data pandora_vmmc2 = {
 	.constraints = {
 		.min_uV			= 1850000,
 		.max_uV			= 3150000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(pandora_vmmc2_supply),
 	.consumer_supplies	= pandora_vmmc2_supply,
@@ -393,10 +383,6 @@ static struct regulator_init_data pandora_vaux1 = {
 		.min_uV			= 3000000,
 		.max_uV			= 3000000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(pandora_vcc_lcd_supply),
 	.consumer_supplies	= pandora_vcc_lcd_supply,
@@ -408,10 +394,6 @@ static struct regulator_init_data pandora_vaux2 = {
 		.min_uV			= 1800000,
 		.max_uV			= 1800000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(pandora_usb_phy_supply),
 	.consumer_supplies	= pandora_usb_phy_supply,
@@ -423,10 +405,6 @@ static struct regulator_init_data pandora_vaux4 = {
 		.min_uV			= 2800000,
 		.max_uV			= 2800000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(pandora_vaux4_supplies),
 	.consumer_supplies	= pandora_vaux4_supplies,
@@ -438,10 +416,6 @@ static struct regulator_init_data pandora_vsim = {
 		.min_uV			= 2800000,
 		.max_uV			= 2800000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(pandora_adac_supply),
 	.consumer_supplies	= pandora_adac_supply,
diff --git a/arch/arm/mach-omap2/board-omap3stalker.c b/arch/arm/mach-omap2/board-omap3stalker.c
index 8e10498..92116f2 100644
--- a/arch/arm/mach-omap2/board-omap3stalker.c
+++ b/arch/arm/mach-omap2/board-omap3stalker.c
@@ -218,10 +218,6 @@ static struct regulator_init_data omap3stalker_vmmc1 = {
 	.constraints		= {
 		.min_uV			= 1850000,
 		.max_uV			= 3150000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-		| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-		| REGULATOR_CHANGE_MODE | REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(omap3stalker_vmmc1_supply),
 	.consumer_supplies	= omap3stalker_vmmc1_supply,
@@ -232,10 +228,6 @@ static struct regulator_init_data omap3stalker_vsim = {
 	.constraints		= {
 		.min_uV			= 1800000,
 		.max_uV			= 3000000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-		| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-		| REGULATOR_CHANGE_MODE | REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(omap3stalker_vsim_supply),
 	.consumer_supplies	= omap3stalker_vsim_supply,
diff --git a/arch/arm/mach-omap2/board-omap3touchbook.c b/arch/arm/mach-omap2/board-omap3touchbook.c
index 852ea04..d97dc67 100644
--- a/arch/arm/mach-omap2/board-omap3touchbook.c
+++ b/arch/arm/mach-omap2/board-omap3touchbook.c
@@ -181,11 +181,6 @@ static struct regulator_init_data touchbook_vmmc1 = {
 	.constraints = {
 		.min_uV			= 1850000,
 		.max_uV			= 3150000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(touchbook_vmmc1_supply),
 	.consumer_supplies	= touchbook_vmmc1_supply,
@@ -196,11 +191,6 @@ static struct regulator_init_data touchbook_vsim = {
 	.constraints = {
 		.min_uV			= 1800000,
 		.max_uV			= 3000000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(touchbook_vsim_supply),
 	.consumer_supplies	= touchbook_vsim_supply,
diff --git a/arch/arm/mach-omap2/board-overo.c b/arch/arm/mach-omap2/board-overo.c
index f0b8489..baee16b 100644
--- a/arch/arm/mach-omap2/board-overo.c
+++ b/arch/arm/mach-omap2/board-overo.c
@@ -428,11 +428,6 @@ static struct regulator_init_data overo_vmmc1 = {
 	.constraints = {
 		.min_uV			= 1850000,
 		.max_uV			= 3150000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(overo_vmmc1_supply),
 	.consumer_supplies	= overo_vmmc1_supply,
diff --git a/arch/arm/mach-omap2/board-rm680.c b/arch/arm/mach-omap2/board-rm680.c
index 7dfed24..5015db7 100644
--- a/arch/arm/mach-omap2/board-rm680.c
+++ b/arch/arm/mach-omap2/board-rm680.c
@@ -41,10 +41,6 @@ static struct regulator_consumer_supply rm680_vemmc_consumers[] = {
 static struct regulator_init_data rm680_vemmc = {
 	.constraints =	{
 		.name			= "rm680_vemmc",
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_STATUS
-					| REGULATOR_CHANGE_MODE,
 	},
 	.num_consumer_supplies		= ARRAY_SIZE(rm680_vemmc_consumers),
 	.consumer_supplies		= rm680_vemmc_consumers,
diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c b/arch/arm/mach-omap2/board-rx51-peripherals.c
index 5a886cd..498c8dd 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -464,10 +464,6 @@ static struct regulator_init_data rx51_vaux1 = {
 		.min_uV			= 2800000,
 		.max_uV			= 2800000,
 		.always_on		= true, /* due battery cover sensor */
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(rx51_vaux1_consumers),
 	.consumer_supplies	= rx51_vaux1_consumers,
@@ -478,10 +474,6 @@ static struct regulator_init_data rx51_vaux2 = {
 		.name			= "VCSI",
 		.min_uV			= 1800000,
 		.max_uV			= 1800000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(rx51_vaux2_supply),
 	.consumer_supplies	= rx51_vaux2_supply,
@@ -494,10 +486,6 @@ static struct regulator_init_data rx51_vaux3_cam = {
 		.min_uV			= 1800000,
 		.max_uV			= 1800000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 };
 
@@ -507,11 +495,6 @@ static struct regulator_init_data rx51_vaux3_mmc = {
 		.min_uV			= 2800000,
 		.max_uV			= 3000000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(rx51_vaux3_supply),
 	.consumer_supplies	= rx51_vaux3_supply,
@@ -523,10 +506,6 @@ static struct regulator_init_data rx51_vaux4 = {
 		.min_uV			= 2800000,
 		.max_uV			= 2800000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 };
 
@@ -534,11 +513,6 @@ static struct regulator_init_data rx51_vmmc1 = {
 	.constraints = {
 		.min_uV			= 1850000,
 		.max_uV			= 3150000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(rx51_vmmc1_supply),
 	.consumer_supplies	= rx51_vmmc1_supply,
@@ -551,11 +525,6 @@ static struct regulator_init_data rx51_vmmc2 = {
 		.max_uV			= 3000000,
 		.always_on		= true, /* due VIO leak to AIC34 VDDs */
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(rx51_vmmc2_supplies),
 	.consumer_supplies	= rx51_vmmc2_supplies,
@@ -568,9 +537,6 @@ static struct regulator_init_data rx51_vpll1 = {
 		.max_uV			= 1800000,
 		.apply_uV		= true,
 		.always_on		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE,
 	},
 };
 
@@ -581,9 +547,6 @@ static struct regulator_init_data rx51_vpll2 = {
 		.max_uV			= 1800000,
 		.apply_uV		= true,
 		.always_on		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE,
 	},
 };
 
@@ -593,10 +556,6 @@ static struct regulator_init_data rx51_vsim = {
 		.min_uV			= 1800000,
 		.max_uV			= 1800000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(rx51_vsim_supply),
 	.consumer_supplies	= rx51_vsim_supply,
@@ -606,11 +565,6 @@ static struct regulator_init_data rx51_vio = {
 	.constraints = {
 		.min_uV			= 1800000,
 		.max_uV			= 1800000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(rx51_vio_supplies),
 	.consumer_supplies	= rx51_vio_supplies,
@@ -622,9 +576,6 @@ static struct regulator_init_data rx51_vintana1 = {
 		.min_uV			= 1500000,
 		.max_uV			= 1500000,
 		.always_on		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE,
 	},
 };
 
@@ -635,9 +586,6 @@ static struct regulator_init_data rx51_vintana2 = {
 		.max_uV			= 2750000,
 		.apply_uV		= true,
 		.always_on		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE,
 	},
 };
 
@@ -647,9 +595,6 @@ static struct regulator_init_data rx51_vintdig = {
 		.min_uV			= 1500000,
 		.max_uV			= 1500000,
 		.always_on		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE,
 	},
 };
 
diff --git a/arch/arm/mach-omap2/board-zoom-peripherals.c b/arch/arm/mach-omap2/board-zoom-peripherals.c
index 6d0aa4f..58b5ad0 100644
--- a/arch/arm/mach-omap2/board-zoom-peripherals.c
+++ b/arch/arm/mach-omap2/board-zoom-peripherals.c
@@ -126,11 +126,6 @@ static struct regulator_init_data zoom_vmmc1 = {
 	.constraints = {
 		.min_uV			= 1850000,
 		.max_uV			= 3150000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies  = ARRAY_SIZE(zoom_vmmc1_supply),
 	.consumer_supplies      = zoom_vmmc1_supply,
@@ -142,10 +137,6 @@ static struct regulator_init_data zoom_vmmc2 = {
 		.min_uV			= 1850000,
 		.max_uV			= 1850000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies  = ARRAY_SIZE(zoom_vmmc2_supply),
 	.consumer_supplies      = zoom_vmmc2_supply,
@@ -156,11 +147,6 @@ static struct regulator_init_data zoom_vsim = {
 	.constraints = {
 		.min_uV			= 1800000,
 		.max_uV			= 3000000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies  = ARRAY_SIZE(zoom_vsim_supply),
 	.consumer_supplies      = zoom_vsim_supply,
diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
index daa056e..a1d4046 100644
--- a/arch/arm/mach-omap2/twl-common.c
+++ b/arch/arm/mach-omap2/twl-common.c
@@ -88,10 +88,6 @@ static struct regulator_init_data omap3_vdac_idata = {
 	.constraints = {
 		.min_uV			= 1800000,
 		.max_uV			= 1800000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies	= ARRAY_SIZE(omap3_vdda_dac_supplies),
 	.consumer_supplies	= omap3_vdda_dac_supplies,
@@ -106,10 +102,6 @@ static struct regulator_init_data omap3_vpll2_idata = {
 	.constraints = {
 		.min_uV                 = 1800000,
 		.max_uV                 = 1800000,
-		.valid_modes_mask       = REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask         = REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies		= ARRAY_SIZE(omap3_vpll2_supplies),
 	.consumer_supplies		= omap3_vpll2_supplies,
@@ -158,10 +150,6 @@ static struct regulator_init_data omap4_vdac_idata = {
 	.constraints = {
 		.min_uV			= 1800000,
 		.max_uV			= 1800000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 };
 
@@ -170,11 +158,6 @@ static struct regulator_init_data omap4_vaux2_idata = {
 		.min_uV			= 1200000,
 		.max_uV			= 2800000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 };
 
@@ -183,11 +166,6 @@ static struct regulator_init_data omap4_vaux3_idata = {
 		.min_uV			= 1000000,
 		.max_uV			= 3000000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 };
 
@@ -201,11 +179,6 @@ static struct regulator_init_data omap4_vmmc_idata = {
 		.min_uV			= 1200000,
 		.max_uV			= 3000000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 	.num_consumer_supplies  = ARRAY_SIZE(omap4_vmmc_supply),
 	.consumer_supplies      = omap4_vmmc_supply,
@@ -216,11 +189,6 @@ static struct regulator_init_data omap4_vpp_idata = {
 		.min_uV			= 1800000,
 		.max_uV			= 2500000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_VOLTAGE
-					| REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 };
 
@@ -228,10 +196,6 @@ static struct regulator_init_data omap4_vana_idata = {
 	.constraints = {
 		.min_uV			= 2100000,
 		.max_uV			= 2100000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 };
 
@@ -239,10 +203,6 @@ static struct regulator_init_data omap4_vcxio_idata = {
 	.constraints = {
 		.min_uV			= 1800000,
 		.max_uV			= 1800000,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 };
 
@@ -251,10 +211,6 @@ static struct regulator_init_data omap4_vusb_idata = {
 		.min_uV			= 3300000,
 		.max_uV			= 3300000,
 		.apply_uV		= true,
-		.valid_modes_mask	= REGULATOR_MODE_NORMAL
-					| REGULATOR_MODE_STANDBY,
-		.valid_ops_mask		= REGULATOR_CHANGE_MODE
-					| REGULATOR_CHANGE_STATUS,
 	},
 };
 
-- 
1.7.1

[RFC PATCH 02/11] regulator: Fix error check in set_consumer_device_supply

[Rajendra Nayak]

The parameters to set_consumer_device_supply() can be considered
invalid (and hence it could return -EINVAL) if nether consumer_dev nor
consumer_dev_name are passed, not when *both* are passed.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 drivers/regulator/core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index d8e6a42..9365359 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -981,7 +981,7 @@ static int set_consumer_device_supply(struct regulator_dev *rdev,
 	struct regulator_map *node;
 	int has_dev;
 
-	if (consumer_dev && consumer_dev_name)
+	if (!consumer_dev && !consumer_dev_name)
 		return -EINVAL;
 
 	if (!consumer_dev_name && consumer_dev)
-- 
1.7.1

[RFC PATCH 03/11] DT: regulator: Helper routine to extract regulator_init_data

[Rajendra Nayak]

The helper routine is meant to be used by regulator drivers
to extract the regulator_init_data structure passed from device tree.
'consumer_supplies' which is part of regulator_init_data is not extracted
as the regulator consumer mappings are passed through DT differently,
implemented in subsequent patches.

Also add documentation for regulator bindings to be used to pass
regulator_init_data struct information from device tree.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 .../devicetree/bindings/regulator/regulator.txt    |   37 +++++++++
 drivers/of/Kconfig                                 |    6 ++
 drivers/of/Makefile                                |    1 +
 drivers/of/of_regulator.c                          |   85 ++++++++++++++++++++
 include/linux/of_regulator.h                       |   23 +++++
 5 files changed, 152 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/regulator.txt
 create mode 100644 drivers/of/of_regulator.c
 create mode 100644 include/linux/of_regulator.h

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
new file mode 100644
index 0000000..001e5ce
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -0,0 +1,37 @@
+Voltage/Current Regulators
+
+Required properties:
+- compatible: Must be "regulator";
+
+Optional properties:
+- supply-regulator: Name of the parent regulator
+- min-uV: smallest voltage consumers may set
+- max-uV: largest voltage consumers may set
+- uV-offset: Offset applied to voltages from consumer to compensate for voltage drops
+- min-uA: smallest current consumers may set
+- max-uA: largest current consumers may set
+- mode-fast: boolean, Can handle fast changes in its load
+- mode-normal: boolean, Normal regulator power supply mode
+- mode-idle: boolean, Can be more efficient during light loads
+- mode-standby: boolean, Can be most efficient during light loads
+- change-voltage: boolean, Output voltage can be changed by software
+- change-current: boolean, Output current can be chnaged by software
+- change-mode: boolean, Operating mode can be changed by software
+- change-status: boolean, Enable/Disable control for regulator exists
+- change-drms: boolean, Dynamic regulator mode switching is enabled
+- input-uV: Input voltage for regulator when supplied by another regulator
+- initial-mode: Mode to set at startup
+- always-on: boolean, regulator should never be disabled
+- boot-on: bootloader/firmware enabled regulator
+- apply-uV: apply uV constraint if min == max
+
+Example:
+
+	xyz-regulator: regulator@0 {
+		compatible = "regulator";
+		min-uV = <1000000>;
+		max-uV = <2500000>;
+		mode-fast;
+		change-voltage;
+		always-on;
+	};
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index b3bfea5..296b96d 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -87,4 +87,10 @@ config OF_PCI_IRQ
 	help
 	  OpenFirmware PCI IRQ routing helpers
 
+config OF_REGULATOR
+	def_bool y
+	depends on REGULATOR
+	help
+	  OpenFirmware regulator framework helpers
+
 endmenu # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index 74b959c..bea2852 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_OF_SPI)	+= of_spi.o
 obj-$(CONFIG_OF_MDIO)	+= of_mdio.o
 obj-$(CONFIG_OF_PCI)	+= of_pci.o
 obj-$(CONFIG_OF_PCI_IRQ)  += of_pci_irq.o
+obj-$(CONFIG_OF_REGULATOR) += of_regulator.o
diff --git a/drivers/of/of_regulator.c b/drivers/of/of_regulator.c
new file mode 100644
index 0000000..f01d275
--- /dev/null
+++ b/drivers/of/of_regulator.c
@@ -0,0 +1,85 @@
+/*
+ * OF helpers for regulator framework
+ *
+ * Copyright (C) 2011 Texas Instruments, Inc.
+ * Rajendra Nayak <rnayak@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/regulator/machine.h>
+
+static void of_get_regulation_constraints(struct device_node *np,
+					struct regulator_init_data **init_data)
+{
+	unsigned int valid_modes_mask = 0, valid_ops_mask = 0;
+
+	of_property_read_u32(np, "min-uV", &(*init_data)->constraints.min_uV);
+	of_property_read_u32(np, "max-uV", &(*init_data)->constraints.max_uV);
+	of_property_read_u32(np, "uV-offset", &(*init_data)->constraints.uV_offset);
+	of_property_read_u32(np, "min-uA", &(*init_data)->constraints.min_uA);
+	of_property_read_u32(np, "max-uA", &(*init_data)->constraints.max_uA);
+
+	/* valid modes mask */
+	if (of_find_property(np, "mode-fast", NULL))
+		valid_modes_mask |= REGULATOR_MODE_FAST;
+	if (of_find_property(np, "mode-normal", NULL))
+		valid_modes_mask |= REGULATOR_MODE_NORMAL;
+	if (of_find_property(np, "mode-idle", NULL))
+		valid_modes_mask |= REGULATOR_MODE_IDLE;
+	if (of_find_property(np, "mode-standby", NULL))
+		valid_modes_mask |= REGULATOR_MODE_STANDBY;
+
+	/* valid ops mask */
+	if (of_find_property(np, "change-voltage", NULL))
+		valid_ops_mask |= REGULATOR_CHANGE_VOLTAGE;
+	if (of_find_property(np, "change-current", NULL))
+		valid_ops_mask |= REGULATOR_CHANGE_CURRENT;
+	if (of_find_property(np, "change-mode", NULL))
+		valid_ops_mask |= REGULATOR_CHANGE_MODE;
+	if (of_find_property(np, "change-status", NULL))
+		valid_ops_mask |= REGULATOR_CHANGE_STATUS;
+
+	(*init_data)->constraints.valid_modes_mask = valid_modes_mask;
+	(*init_data)->constraints.valid_ops_mask = valid_ops_mask;
+
+	of_property_read_u32(np, "input-uV",
+			&(*init_data)->constraints.input_uV);
+	of_property_read_u32(np, "initial-mode",
+			&(*init_data)->constraints.initial_mode);
+
+	if (of_find_property(np, "always-on", NULL))
+			(*init_data)->constraints.always_on = true;
+	if (of_find_property(np, "boot-on", NULL))
+			(*init_data)->constraints.boot_on = true;
+	if (of_find_property(np, "apply-uV", NULL))
+			(*init_data)->constraints.apply_uV = true;
+}
+
+/**
+ * of_get_regulator_init_data - extarct regulator_init_data structure info
+ * @np: Pointer to regulator device tree node
+ *
+ * Populates regulator_init_data structure by extracting data from device
+ * tree node, returns a pointer to the populated struture or NULL if memory
+ * alloc fails.
+ */
+struct regulator_init_data *of_get_regulator_init_data(struct device_node *np)
+{
+	struct regulator_init_data *init_data;
+
+	init_data = kzalloc(sizeof(struct regulator_init_data), GFP_KERNEL);
+	if (!init_data)
+		return NULL; /* Out of memory? */
+
+	init_data->supply_regulator = (char *)of_get_property(np,
+						"supply-regulator", NULL);
+	of_get_regulation_constraints(np, &init_data);
+	return init_data;
+}
+EXPORT_SYMBOL(of_get_regulator_init_data);
diff --git a/include/linux/of_regulator.h b/include/linux/of_regulator.h
new file mode 100644
index 0000000..5eb048c
--- /dev/null
+++ b/include/linux/of_regulator.h
@@ -0,0 +1,23 @@
+/*
+ * OpenFirmware regulator support routines
+ *
+ */
+
+#ifndef __LINUX_OF_REG_H
+#define __LINUX_OF_REG_H
+
+#include <linux/regulator/machine.h>
+
+#if defined(CONFIG_OF_REGULATOR)
+extern struct regulator_init_data
+	*of_get_regulator_init_data(struct device_node *np);
+#else
+static inline struct regulator_init_data
+	*of_get_regulator_init_data(struct device_node *np)
+{
+	return NULL;
+}
+#endif /* CONFIG_OF_REGULATOR */
+
+#endif /* __LINUX_OF_REG_H */
+
-- 
1.7.1

[RFC PATCH 04/11] omap4: SDP: Pass regulator_init_data from DT

[Rajendra Nayak]

Pass regulator_init_data information for omap4sdp from device tree so the
regulator driver can then use the regulator helper
routine to extract and use them during the driver probe().

Add documentation for TWL regulator specific bindings.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 .../bindings/regulator/twl-regulator.txt           |   18 ++++++++++++++++++
 arch/arm/boot/dts/omap4-sdp.dts                    |   16 ++++++++++++++++
 2 files changed, 34 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/twl-regulator.txt

diff --git a/Documentation/devicetree/bindings/regulator/twl-regulator.txt b/Documentation/devicetree/bindings/regulator/twl-regulator.txt
new file mode 100644
index 0000000..59df44b
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/twl-regulator.txt
@@ -0,0 +1,18 @@
+TWL family of regulators
+
+Required properties:
+- compatible: Must be "regulator","ti,twl-reg";
+
+Optional properties:
+- Any optional property defined in bindings/regulator/regulator.txt
+- ti,reg-id: Identifier for a TWL regulator, defined in linux/i2c/twl.h file.
+
+Example:
+
+	xyz-regulator: regulator@0 {
+		compatible = "regulator","ti,twl-reg";
+		ti,reg-id = <37>; /* TWL6030_REG_VAUX1_6030 */
+		min-uV  = <1000000>;
+		max-uV  = <3000000>;
+		apply-uV;
+	};
diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
index 14faf92..2a613e2 100644
--- a/arch/arm/boot/dts/omap4-sdp.dts
+++ b/arch/arm/boot/dts/omap4-sdp.dts
@@ -52,6 +52,22 @@
 			interrupts = <11>;
 			reg = <0>;
 		};
+
+		vaux1-regulator: regulator@0 {
+			compatible = "regulator","ti,twl-reg";
+			ti,reg-id = <37>;  /* TWL6030_REG_VAUX1_6030 */
+			min-uV = <1000000>;
+			max-uV = <3000000>;
+			apply-uV;
+		};
+
+		vusim-regulator: regulator@1 {
+			compatible = "regulator","ti,twl-reg";
+			ti,reg-id = <42>; /* TWL6030_REG_VUSIM */
+			min-uV = <1200000>;
+			max-uV = <2900000>;
+			apply_uV;
+		};
 	};
 };
 
-- 
1.7.1

[RFC PATCH 05/11] TWL: regulator: Make twl-regulator driver extract data from DT

[Rajendra Nayak]

Modify the twl regulator driver to extract the regulator_init_data from
device tree when passed, instead of getting it through platform_data
structures (on non-DT builds)

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 drivers/regulator/twl-regulator.c |   28 +++++++++++++++++++++++++---
 1 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/twl-regulator.c b/drivers/regulator/twl-regulator.c
index ee8747f..df1b95a 100644
--- a/drivers/regulator/twl-regulator.c
+++ b/drivers/regulator/twl-regulator.c
@@ -17,6 +17,8 @@
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
 #include <linux/i2c/twl.h>
+#include <linux/of.h>
+#include <linux/of_regulator.h>
 
 
 /*
@@ -1011,6 +1013,9 @@ static int __devinit twlreg_probe(struct platform_device *pdev)
 	struct regulation_constraints	*c;
 	struct regulator_dev		*rdev;
 
+	if (pdev->dev.of_node)
+		of_property_read_u32(pdev->dev.of_node, "ti,reg-id", &pdev->id);
+
 	for (i = 0, info = NULL; i < ARRAY_SIZE(twl_regs); i++) {
 		if (twl_regs[i].desc.id != pdev->id)
 			continue;
@@ -1020,7 +1025,11 @@ static int __devinit twlreg_probe(struct platform_device *pdev)
 	if (!info)
 		return -ENODEV;
 
-	initdata = pdev->dev.platform_data;
+	if (pdev->dev.of_node)
+		initdata = of_get_regulator_init_data(pdev->dev.of_node);
+	else
+		initdata = pdev->dev.platform_data;
+
 	if (!initdata)
 		return -EINVAL;
 
@@ -1101,14 +1110,27 @@ static int __devexit twlreg_remove(struct platform_device *pdev)
 
 MODULE_ALIAS("platform:twl_reg");
 
+#if defined(CONFIG_OF)
+static const struct of_device_id twl_of_match[] __devinitconst = {
+	{ .compatible = "ti,twl-reg", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, twl_of_match);
+#else
+#define twl_of_match NULL
+#endif
+
 static struct platform_driver twlreg_driver = {
 	.probe		= twlreg_probe,
 	.remove		= __devexit_p(twlreg_remove),
 	/* NOTE: short name, to work around driver model truncation of
 	 * "twl_regulator.12" (and friends) to "twl_regulator.1".
 	 */
-	.driver.name	= "twl_reg",
-	.driver.owner	= THIS_MODULE,
+	.driver  = {
+		.name  = "twl_reg",
+		.owner = THIS_MODULE,
+		.of_match_table = twl_of_match,
+	},
 };
 
 static int __init twlreg_init(void)
-- 
1.7.1

[RFC PATCH 06/11] DT: regulator: Helper routine to extract fixed_voltage_config

[Rajendra Nayak]

The helper routine defined here (of_get_fixed_voltage_config) can
be used to extract information about fixed regulators (which are not
software controlable) from device tree.

Add documenation about additional bindings for fixed
regulators that can be passed through DT.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 .../devicetree/bindings/regulator/regulator.txt    |   19 ++++++++++++
 drivers/of/of_regulator.c                          |   31 ++++++++++++++++++++
 include/linux/of_regulator.h                       |    7 ++++
 3 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index 001e5ce..f8c51d8 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -2,6 +2,8 @@ Voltage/Current Regulators
 
 Required properties:
 - compatible: Must be "regulator";
+or
+- compatible: Must be "regulator-fixed"; /* For Fixed volatge regulator */
 
 Optional properties:
 - supply-regulator: Name of the parent regulator
@@ -24,6 +26,13 @@ Optional properties:
 - always-on: boolean, regulator should never be disabled
 - boot-on: bootloader/firmware enabled regulator
 - apply-uV: apply uV constraint if min == max
+# For fixed voltage regulators
+- supply-name: Name of the regulator supply
+- microvolts: Output voltage of regulator
+- gpio: gpio to use for enable control
+- startup-delay: startup time in microseconds
+- enable-high: Polarity of enable GPIO, 1 = Active High, 0 = Active low
+- enabled-at-boot: 1 = yes, 0 = no
 
 Example:
 
@@ -35,3 +44,13 @@ Example:
 		change-voltage;
 		always-on;
 	};
+
+	abc-fixedregulator {
+		compatible = "regulator-fixed";
+		supply-name = "fixed-supply";
+		microvolts = <1800000>;
+		gpio = <43>;
+		startup-delay = <70000>;
+		enable-high;
+		enabled-at-boot;
+	};
diff --git a/drivers/of/of_regulator.c b/drivers/of/of_regulator.c
index f01d275..4d7a49d 100644
--- a/drivers/of/of_regulator.c
+++ b/drivers/of/of_regulator.c
@@ -13,6 +13,7 @@
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/regulator/machine.h>
+#include <linux/regulator/fixed.h>
 
 static void of_get_regulation_constraints(struct device_node *np,
 					struct regulator_init_data **init_data)
@@ -83,3 +84,33 @@ struct regulator_init_data *of_get_regulator_init_data(struct device_node *np)
 	return init_data;
 }
 EXPORT_SYMBOL(of_get_regulator_init_data);
+
+/**
+ * of_get_fixed_voltage_config - extract fixed_voltage_config structure info
+ * @np: Pointer to fixed-regulator device tree node
+ *
+ * Populates fixed_voltage_config structure by extracting data from device
+ * tree node, returns a pointer to the populated structure of NULL if memory
+ * alloc fails.
+ */
+struct fixed_voltage_config *of_get_fixed_voltage_config(struct device_node *np)
+{
+	struct fixed_voltage_config *config;
+
+	config = kzalloc(sizeof(struct fixed_voltage_config), GFP_KERNEL);
+	if (!config)
+		return NULL;
+
+	config->supply_name = (char *)of_get_property(np, "supply-name", NULL);
+	of_property_read_u32(np, "microvolts", &config->microvolts);
+	of_property_read_u32(np, "gpio", &config->gpio);
+	of_property_read_u32(np, "startup-delay", &config->startup_delay);
+	if (of_find_property(np, "enable-high", NULL))
+		config->enable_high = true;
+	if (of_find_property(np, "enabled-at-boot", NULL))
+		config->enabled_at_boot = true;
+	config->init_data = of_get_regulator_init_data(np);
+
+	return config;
+}
+EXPORT_SYMBOL(of_get_fixed_voltage_config);
diff --git a/include/linux/of_regulator.h b/include/linux/of_regulator.h
index 5eb048c..39860c5 100644
--- a/include/linux/of_regulator.h
+++ b/include/linux/of_regulator.h
@@ -11,12 +11,19 @@
 #if defined(CONFIG_OF_REGULATOR)
 extern struct regulator_init_data
 	*of_get_regulator_init_data(struct device_node *np);
+extern struct fixed_voltage_config
+	*of_get_fixed_voltage_config(struct device_node *np);
 #else
 static inline struct regulator_init_data
 	*of_get_regulator_init_data(struct device_node *np)
 {
 	return NULL;
 }
+static inline struct fixed_voltage_config
+	*of_get_fixed_voltage_config(struct device_node *np)
+{
+	return NULL;
+}
 #endif /* CONFIG_OF_REGULATOR */
 
 #endif /* __LINUX_OF_REG_H */
-- 
1.7.1

[RFC PATCH 07/11] regulator: Make fixed regulator driver extract data from DT

[Rajendra Nayak]

Modify the fixed regulator driver to extract fixed_voltage_config from
device tree when passed, instead of getting it through platform_data
structures (on non-DT builds)

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 drivers/regulator/fixed.c |   20 +++++++++++++++++++-
 1 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 2fe9d99..a214b30 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -26,6 +26,8 @@
 #include <linux/gpio.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_regulator.h>
 
 struct fixed_voltage_data {
 	struct regulator_desc desc;
@@ -104,10 +106,15 @@ static struct regulator_ops fixed_voltage_ops = {
 
 static int __devinit reg_fixed_voltage_probe(struct platform_device *pdev)
 {
-	struct fixed_voltage_config *config = pdev->dev.platform_data;
+	struct fixed_voltage_config *config;
 	struct fixed_voltage_data *drvdata;
 	int ret;
 
+	if (pdev->dev.of_node)
+		config = of_get_fixed_voltage_config(pdev->dev.of_node);
+	else
+		config = pdev->dev.platform_data;
+
 	drvdata = kzalloc(sizeof(struct fixed_voltage_data), GFP_KERNEL);
 	if (drvdata == NULL) {
 		dev_err(&pdev->dev, "Failed to allocate device data\n");
@@ -216,12 +223,23 @@ static int __devexit reg_fixed_voltage_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#if defined(CONFIG_OF)
+static const struct of_device_id fixed_of_match[] __devinitconst = {
+	{ .compatible = "regulator-fixed", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, fixed_of_match);
+#else
+#define fixed_of_match NULL
+#endif
+
 static struct platform_driver regulator_fixed_voltage_driver = {
 	.probe		= reg_fixed_voltage_probe,
 	.remove		= __devexit_p(reg_fixed_voltage_remove),
 	.driver		= {
 		.name		= "reg-fixed-voltage",
 		.owner		= THIS_MODULE,
+		.of_match_table = fixed_of_match,
 	},
 };
 
-- 
1.7.1

[RFC PATCH 08/11] omap4: panda: Pass fixed regulator data from DT

[Rajendra Nayak]

Pass the fixed voltage regulator information for
omap4panda board from device tree.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/boot/dts/omap4-panda.dts |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap4-panda.dts b/arch/arm/boot/dts/omap4-panda.dts
index 0a83e3f..9c34c7f 100644
--- a/arch/arm/boot/dts/omap4-panda.dts
+++ b/arch/arm/boot/dts/omap4-panda.dts
@@ -52,6 +52,16 @@
 			interrupts = <11>;
 			reg = <0>;
 		};
+
+		vwl1271-fixedregulator {
+			compatible = "regulator-fixed","ti,twl-reg";
+			supply-name = "vwl1271";
+			microvolts = <1800000>;
+			gpio = <43>;
+			startup-delay = <70000>;
+			enable-high;
+			enabled-at-boot;
+		};
 	};
 };
 
-- 
1.7.1

[RFC PATCH 09/11] DT: regulator: Helper to extract regulator node based on supply name

[Rajendra Nayak]

Device nodes in DT can associate themselves with one or more
regulators by providing a list of phandles (to regulator nodes)
and corresponding supply names.

For Example:
	devicenode: node@0x0 {
		...
		...
		regulator = <&regulator1>,<&regulator2>;
		regulator-names = "supply1","supply2";
	};

of_get_regulator() extracts the regulator node for a given
device, based on the supply name.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 drivers/of/of_regulator.c    |   37 +++++++++++++++++++++++++++++++++++++
 include/linux/of_regulator.h |    7 +++++++
 2 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/drivers/of/of_regulator.c b/drivers/of/of_regulator.c
index 4d7a49d..6f8fd4e 100644
--- a/drivers/of/of_regulator.c
+++ b/drivers/of/of_regulator.c
@@ -114,3 +114,40 @@ struct fixed_voltage_config *of_get_fixed_voltage_config(struct device_node *np)
 	return config;
 }
 EXPORT_SYMBOL(of_get_fixed_voltage_config);
+
+/**
+ * of_get_regulator - get a regulator device node based on supply name
+ * @dev: Device pointer for the consumer (of regulator) device
+ * @supply: regulator supply name
+ *
+ * Extract the regulator device node corresponding to the supply name.
+ * retruns the device node corresponding to the regulator if found, else
+ * returns NULL.
+ */
+struct device_node *of_get_regulator(struct device *dev, const char *supply)
+{
+	struct device_node *regnode = NULL;
+	u32 reghandle;
+	int regsz, namesz;
+	const void *regprop;
+	const char *namesprop;
+
+	regprop = of_get_property(dev->of_node, "regulator", &regsz);
+	namesprop = of_get_property(dev->of_node, "regulator-names", &namesz);
+	if (!regprop || !namesprop)
+		return NULL;
+
+	while (regsz && namesz) {
+		if (!strcmp(namesprop, supply)) {
+			reghandle = be32_to_cpup(regprop);
+			regnode = of_find_node_by_phandle(reghandle);
+			break;
+		}
+		regsz -= 4;
+		regprop += 4;
+		namesz -= strlen(namesprop) + 1;
+		namesprop += strlen(namesprop) + 1;
+	}
+	return regnode;
+}
+EXPORT_SYMBOL(of_get_regulator);
diff --git a/include/linux/of_regulator.h b/include/linux/of_regulator.h
index 39860c5..5fc7329 100644
--- a/include/linux/of_regulator.h
+++ b/include/linux/of_regulator.h
@@ -13,6 +13,8 @@ extern struct regulator_init_data
 	*of_get_regulator_init_data(struct device_node *np);
 extern struct fixed_voltage_config
 	*of_get_fixed_voltage_config(struct device_node *np);
+extern struct device_node *of_get_regulator(struct device *dev,
+	const char *id);
 #else
 static inline struct regulator_init_data
 	*of_get_regulator_init_data(struct device_node *np)
@@ -24,6 +26,11 @@ static inline struct fixed_voltage_config
 {
 	return NULL;
 }
+static inline struct device_node *of_get_regulator(struct device *dev,
+	const char *id)
+{
+	return NULL;
+}
 #endif /* CONFIG_OF_REGULATOR */
 
 #endif /* __LINUX_OF_REG_H */
-- 
1.7.1

[RFC PATCH 10/11] regulator: Implement consumer regulator mapping from device tree

[Rajendra Nayak]

With device tree, the consumer regulator mapping is deferred till
a regulator_get is called from the corresponding device driver,
instead of being done during regulator_register.
This avoids a complete scan of all DT nodes to identify consumers
for all regulators.

Devices can assocaite with one or more regulators by providing a
list of phandles and supply names.

For Example:
        devicenode: node@0x0 {
                ...
                ...
                regulator = <&regulator1>,<&regulator2>;
                regulator-names = "supply1","supply2";
        };

When a device driver calls a regulator_get, specifying the
supply name, the phandle and eventually the regulator node
is extracted from the device and a mapping created by calling
set_consumer_device_supply().

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 drivers/regulator/core.c         |   23 +++++++++++++++++++++++
 include/linux/regulator/driver.h |    3 +++
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 9365359..61da2e7 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -25,6 +25,8 @@
 #include <linux/mutex.h>
 #include <linux/suspend.h>
 #include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/of_regulator.h>
 #include <linux/regulator/consumer.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
@@ -1167,6 +1169,15 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
 
 	mutex_lock(&regulator_list_mutex);
 
+#ifdef CONFIG_OF
+	struct device_node *node;
+	node = of_get_regulator(dev, id);
+	if (!node)
+		goto out;
+	list_for_each_entry(rdev, &regulator_list, list)
+		if (node == rdev->node)
+			goto found;
+#else
 	list_for_each_entry(map, &regulator_map_list, list) {
 		/* If the mapping has a device set up it must match */
 		if (map->dev_name &&
@@ -1178,6 +1189,7 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
 			goto found;
 		}
 	}
+#endif
 
 	if (board_wants_dummy_regulator) {
 		rdev = dummy_regulator_rdev;
@@ -1216,6 +1228,15 @@ found:
 	if (!try_module_get(rdev->owner))
 		goto out;
 
+#ifdef CONFIG_OF
+	ret = set_consumer_device_supply(rdev, dev, devname, id);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set supply %d\n", ret);
+		unset_regulator_supplies(rdev);
+		goto out;
+	}
+#endif
+
 	regulator = create_regulator(rdev, dev, id);
 	if (regulator == NULL) {
 		regulator = ERR_PTR(-ENOMEM);
@@ -2619,6 +2640,8 @@ struct regulator_dev *regulator_register(struct regulator_desc *regulator_desc,
 	rdev->reg_data = driver_data;
 	rdev->owner = regulator_desc->owner;
 	rdev->desc = regulator_desc;
+	if (dev && dev->of_node)
+		rdev->node = dev->of_node;
 	INIT_LIST_HEAD(&rdev->consumer_list);
 	INIT_LIST_HEAD(&rdev->list);
 	BLOCKING_INIT_NOTIFIER_HEAD(&rdev->notifier);
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 1a80bc7..4aebbf5 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -196,6 +196,9 @@ struct regulator_dev {
 	struct mutex mutex; /* consumer lock */
 	struct module *owner;
 	struct device dev;
+#ifdef CONFIG_OF
+	struct device_node *node;
+#endif
 	struct regulation_constraints *constraints;
 	struct regulator *supply;	/* for tree */
 
-- 
1.7.1

[RFC PATCH 11/11] DT: regulator: register regulators as platform devices

[Rajendra Nayak]

of_regulator_register_devices() registers all regulators
as platform devices. Use this to register all twl regulators
from the twl driver probe.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 drivers/mfd/twl-core.c       |    3 +++
 drivers/of/of_regulator.c    |   30 ++++++++++++++++++++++++++++++
 include/linux/of_regulator.h |    5 +++++
 3 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index a12af12..f210e28 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -36,6 +36,7 @@
 #include <linux/slab.h>
 #include <linux/of_irq.h>
 #include <linux/irqdomain.h>
+#include <linux/of_regulator.h>
 
 #include <linux/regulator/machine.h>
 
@@ -1357,6 +1358,8 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	else
 		status = add_children(pdata, id->driver_data);
 
+	of_regulator_register_devices(node);
+
 fail:
 	if (status < 0)
 		twl_remove(client);
diff --git a/drivers/of/of_regulator.c b/drivers/of/of_regulator.c
index 6f8fd4e..0312a6a 100644
--- a/drivers/of/of_regulator.c
+++ b/drivers/of/of_regulator.c
@@ -12,6 +12,7 @@
 
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/regulator/machine.h>
 #include <linux/regulator/fixed.h>
 
@@ -151,3 +152,32 @@ struct device_node *of_get_regulator(struct device *dev, const char *supply)
 	return regnode;
 }
 EXPORT_SYMBOL(of_get_regulator);
+
+/**
+ * of_regulator_register_devices - Register regulator devices to platform bus
+ * @np:	Parent device node with regulator child nodes
+ *
+ * Registers all the regulator and regulator-fixed nodes as platform devices
+ *
+ */
+void of_regulator_register_devices(struct device_node *np)
+{
+	struct device_node *child;
+	struct platform_device *dev;
+
+	for_each_child_of_node(np, child) {
+		if (of_device_is_compatible(child, "regulator")
+			|| of_device_is_compatible(child, "regulator-fixed")) {
+			dev = of_device_alloc(child, NULL, NULL);
+			if (!dev)
+				return;
+			dev->dev.bus = &platform_bus_type;
+			if (of_device_add(dev) != 0) {
+				platform_device_put(dev);
+				return;
+			}
+		}
+	}
+	return;
+}
+
diff --git a/include/linux/of_regulator.h b/include/linux/of_regulator.h
index 5fc7329..38cf7e3 100644
--- a/include/linux/of_regulator.h
+++ b/include/linux/of_regulator.h
@@ -15,6 +15,7 @@ extern struct fixed_voltage_config
 	*of_get_fixed_voltage_config(struct device_node *np);
 extern struct device_node *of_get_regulator(struct device *dev,
 	const char *id);
+extern void of_regulator_register_devices(struct device_node *np);
 #else
 static inline struct regulator_init_data
 	*of_get_regulator_init_data(struct device_node *np)
@@ -31,6 +32,10 @@ static inline struct device_node *of_get_regulator(struct device *dev,
 {
 	return NULL;
 }
+static inline void of_regulator_register_devices(struct device_node *np)
+{
+	return NULL;
+}
 #endif /* CONFIG_OF_REGULATOR */
 
 #endif /* __LINUX_OF_REG_H */
-- 
1.7.1

Re: [RFC PATCH 11/11] DT: regulator: register regulators as platform devices

[Mark Brown]

On Thu, Sep 15, 2011 at 04:52:07PM +0530, Rajendra Nayak wrote:
> of_regulator_register_devices() registers all regulators
> as platform devices. Use this to register all twl regulators
> from the twl driver probe.

Regulators can be devices of any type, not just platform devices.

>  drivers/mfd/twl-core.c       |    3 +++
>  drivers/of/of_regulator.c    |   30 ++++++++++++++++++++++++++++++
>  include/linux/of_regulator.h |    5 +++++

Again, in the regulator code not hidden away please.

> +/**
> + * of_regulator_register_devices - Register regulator devices to platform bus
> + * @np:	Parent device node with regulator child nodes
> + *
> + * Registers all the regulator and regulator-fixed nodes as platform devices
> + *
> + */
> +void of_regulator_register_devices(struct device_node *np)
> +{
> +	struct device_node *child;
> +	struct platform_device *dev;
> +
> +	for_each_child_of_node(np, child) {
> +		if (of_device_is_compatible(child, "regulator")
> +			|| of_device_is_compatible(child, "regulator-fixed")) {
> +			dev = of_device_alloc(child, NULL, NULL);
> +			if (!dev)
> +				return;
> +			dev->dev.bus = &platform_bus_type;
> +			if (of_device_add(dev) != 0) {
> +				platform_device_put(dev);
> +				return;
> +			}

I'm not entirely sure what this is for?  Surely we should be
instantiating the subdevices of the MFD in the same way we always have
done?

> +		}
> +	}
> +	return;
> +}
> +
> diff --git a/include/linux/of_regulator.h b/include/linux/of_regulator.h
> index 5fc7329..38cf7e3 100644
> --- a/include/linux/of_regulator.h
> +++ b/include/linux/of_regulator.h
> @@ -15,6 +15,7 @@ extern struct fixed_voltage_config
>  	*of_get_fixed_voltage_config(struct device_node *np);
>  extern struct device_node *of_get_regulator(struct device *dev,
>  	const char *id);
> +extern void of_regulator_register_devices(struct device_node *np);
>  #else
>  static inline struct regulator_init_data
>  	*of_get_regulator_init_data(struct device_node *np)
> @@ -31,6 +32,10 @@ static inline struct device_node *of_get_regulator(struct device *dev,
>  {
>  	return NULL;
>  }
> +static inline void of_regulator_register_devices(struct device_node *np)
> +{
> +	return NULL;
> +}
>  #endif /* CONFIG_OF_REGULATOR */
>  
>  #endif /* __LINUX_OF_REG_H */
> -- 
> 1.7.1
> 

Re: [RFC PATCH 11/11] DT: regulator: register regulators as platform devices

[Rajendra Nayak]

On Thursday 15 September 2011 07:51 PM, Mark Brown wrote:
> On Thu, Sep 15, 2011 at 04:52:07PM +0530, Rajendra Nayak wrote:
>> of_regulator_register_devices() registers all regulators
>> as platform devices. Use this to register all twl regulators
>> from the twl driver probe.
>
> Regulators can be devices of any type, not just platform devices.

Right, I am just going to drop this patch, since we would have all
twl child nodes being added as platform devices.

>
>>   drivers/mfd/twl-core.c       |    3 +++
>>   drivers/of/of_regulator.c    |   30 ++++++++++++++++++++++++++++++
>>   include/linux/of_regulator.h |    5 +++++
>
> Again, in the regulator code not hidden away please.
>
>> +/**
>> + * of_regulator_register_devices - Register regulator devices to platform bus
>> + * @np:	Parent device node with regulator child nodes
>> + *
>> + * Registers all the regulator and regulator-fixed nodes as platform devices
>> + *
>> + */
>> +void of_regulator_register_devices(struct device_node *np)
>> +{
>> +	struct device_node *child;
>> +	struct platform_device *dev;
>> +
>> +	for_each_child_of_node(np, child) {
>> +		if (of_device_is_compatible(child, "regulator")
>> +			|| of_device_is_compatible(child, "regulator-fixed")) {
>> +			dev = of_device_alloc(child, NULL, NULL);
>> +			if (!dev)
>> +				return;
>> +			dev->dev.bus =&platform_bus_type;
>> +			if (of_device_add(dev) != 0) {
>> +				platform_device_put(dev);
>> +				return;
>> +			}
>
> I'm not entirely sure what this is for?  Surely we should be
> instantiating the subdevices of the MFD in the same way we always have
> done?
>
>> +		}
>> +	}
>> +	return;
>> +}
>> +
>> diff --git a/include/linux/of_regulator.h b/include/linux/of_regulator.h
>> index 5fc7329..38cf7e3 100644
>> --- a/include/linux/of_regulator.h
>> +++ b/include/linux/of_regulator.h
>> @@ -15,6 +15,7 @@ extern struct fixed_voltage_config
>>   	*of_get_fixed_voltage_config(struct device_node *np);
>>   extern struct device_node *of_get_regulator(struct device *dev,
>>   	const char *id);
>> +extern void of_regulator_register_devices(struct device_node *np);
>>   #else
>>   static inline struct regulator_init_data
>>   	*of_get_regulator_init_data(struct device_node *np)
>> @@ -31,6 +32,10 @@ static inline struct device_node *of_get_regulator(struct device *dev,
>>   {
>>   	return NULL;
>>   }
>> +static inline void of_regulator_register_devices(struct device_node *np)
>> +{
>> +	return NULL;
>> +}
>>   #endif /* CONFIG_OF_REGULATOR */
>>
>>   #endif /* __LINUX_OF_REG_H */
>> --
>> 1.7.1
>>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH 10/11] regulator: Implement consumer regulator mapping from device tree

[Mark Brown]

On Thu, Sep 15, 2011 at 04:52:06PM +0530, Rajendra Nayak wrote:

> +#ifdef CONFIG_OF
> +	struct device_node *node;
> +	node = of_get_regulator(dev, id);
> +	if (!node)
> +		goto out;
> +	list_for_each_entry(rdev, &regulator_list, list)
> +		if (node == rdev->node)
> +			goto found;
> +#else
>  	list_for_each_entry(map, &regulator_map_list, list) {
>  		/* If the mapping has a device set up it must match */
>  		if (map->dev_name &&
> @@ -1178,6 +1189,7 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
>  			goto found;
>  		}
>  	}
> +#endif

This forces all machines to use device tree when CONFIG_OF is enabled.
I'd expect to see both mechanisms supported simultaneously, for example
by falling back to the current code if the device tree lookup fails.

> @@ -1216,6 +1228,15 @@ found:
>  	if (!try_module_get(rdev->owner))
>  		goto out;
>  
> +#ifdef CONFIG_OF
> +	ret = set_consumer_device_supply(rdev, dev, devname, id);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to set supply %d\n", ret);
> +		unset_regulator_supplies(rdev);
> +		goto out;
> +	}
> +#endif
> +

This seems wrong, why are we adding things to the regulator_map which is
really only there for lookups when we already did the lookup using the
device tree?

> @@ -2619,6 +2640,8 @@ struct regulator_dev *regulator_register(struct regulator_desc *regulator_desc,
>  	rdev->reg_data = driver_data;
>  	rdev->owner = regulator_desc->owner;
>  	rdev->desc = regulator_desc;
> +	if (dev && dev->of_node)
> +		rdev->node = dev->of_node;

dev is mandatory.

Re: [RFC PATCH 10/11] regulator: Implement consumer regulator mapping from device tree

[Rajendra Nayak]

On Thursday 15 September 2011 07:29 PM, Mark Brown wrote:
> On Thu, Sep 15, 2011 at 04:52:06PM +0530, Rajendra Nayak wrote:
>
>> +#ifdef CONFIG_OF
>> +	struct device_node *node;
>> +	node = of_get_regulator(dev, id);
>> +	if (!node)
>> +		goto out;
>> +	list_for_each_entry(rdev, &regulator_list, list)
>> +		if (node == rdev->node)
>> +			goto found;
>> +#else
>>   	list_for_each_entry(map,&regulator_map_list, list) {
>>   		/* If the mapping has a device set up it must match */
>>   		if (map->dev_name &&
>> @@ -1178,6 +1189,7 @@ static struct regulator *_regulator_get(struct device *dev, const char *id,
>>   			goto found;
>>   		}
>>   	}
>> +#endif
>
> This forces all machines to use device tree when CONFIG_OF is enabled.
> I'd expect to see both mechanisms supported simultaneously, for example
> by falling back to the current code if the device tree lookup fails.

agree, will change this to fall back if the dt lookup fails.

>
>> @@ -1216,6 +1228,15 @@ found:
>>   	if (!try_module_get(rdev->owner))
>>   		goto out;
>>
>> +#ifdef CONFIG_OF
>> +	ret = set_consumer_device_supply(rdev, dev, devname, id);
>> +	if (ret <  0) {
>> +		dev_err(dev, "Failed to set supply %d\n", ret);
>> +		unset_regulator_supplies(rdev);
>> +		goto out;
>> +	}
>> +#endif
>> +
>
> This seems wrong, why are we adding things to the regulator_map which is
> really only there for lookups when we already did the lookup using the
> device tree?

I did this only so the mappings show up in sysfs.

>
>> @@ -2619,6 +2640,8 @@ struct regulator_dev *regulator_register(struct regulator_desc *regulator_desc,
>>   	rdev->reg_data = driver_data;
>>   	rdev->owner = regulator_desc->owner;
>>   	rdev->desc = regulator_desc;
>> +	if (dev &&  dev->of_node)
>> +		rdev->node = dev->of_node;
>
> dev is mandatory.

ok

Re: [RFC PATCH 10/11] regulator: Implement consumer regulator mapping from device tree

[Mark Brown]

On Fri, Sep 16, 2011 at 12:51:06PM +0530, Rajendra Nayak wrote:
> On Thursday 15 September 2011 07:29 PM, Mark Brown wrote:

> >This seems wrong, why are we adding things to the regulator_map which is
> >really only there for lookups when we already did the lookup using the
> >device tree?

> I did this only so the mappings show up in sysfs.

Reworking to bind sysfs at regulator_get() time seems a better approach
to this.

Re: [RFC PATCH 09/11] DT: regulator: Helper to extract regulator node based on supply name

[Mark Brown]

On Thu, Sep 15, 2011 at 04:52:05PM +0530, Rajendra Nayak wrote:

> 		regulator = <&regulator1>,<&regulator2>;
> 		regulator-names = "supply1","supply2";
> 	};

This syntax is really painful - we're relying on keeping two arrays in
sync which isn't good for legibility or robustness.  I'd expect
something like having a property with the supply name referencing the
regulator node concerned like:

	dbvdd = <&regulator1>;
	dcvdd = <&regulator2>;

or something.  Keeping the two arrays separate doesn't seem great.

Re: [RFC PATCH 09/11] DT: regulator: Helper to extract regulator node based on supply name

[Grant Likely]

On Thu, Sep 15, 2011 at 02:54:16PM +0100, Mark Brown wrote:
> On Thu, Sep 15, 2011 at 04:52:05PM +0530, Rajendra Nayak wrote:
> 
> > 		regulator = <&regulator1>,<&regulator2>;
> > 		regulator-names = "supply1","supply2";
> > 	};
> 
> This syntax is really painful - we're relying on keeping two arrays in
> sync which isn't good for legibility or robustness.  I'd expect
> something like having a property with the supply name referencing the
> regulator node concerned like:
> 
> 	dbvdd = <&regulator1>;
> 	dcvdd = <&regulator2>;
> 
> or something.  Keeping the two arrays separate doesn't seem great.

We've got two competing approaches here.  For reg and interrupts, the
proposal on the table that we talked about at LPC is to do reg-names
and interrupts-names so as to preserve the existing semantics of the
reg and interrupts properties.  For gpios we're using the binding
"<name>-gpios" for named gpio references.  There isn't the same
pressure to preserve existing bindings in that case.  I'm okay with
either approach, providing that "-regulator" is encoded into the name.

g.

Re: [RFC PATCH 09/11] DT: regulator: Helper to extract regulator node based on supply name

[Mark Brown]

On Thu, Sep 15, 2011 at 04:50:35PM -0600, Grant Likely wrote:

> We've got two competing approaches here.  For reg and interrupts, the
> proposal on the table that we talked about at LPC is to do reg-names
> and interrupts-names so as to preserve the existing semantics of the
> reg and interrupts properties.  For gpios we're using the binding
> "<name>-gpios" for named gpio references.  There isn't the same
> pressure to preserve existing bindings in that case.  I'm okay with
> either approach, providing that "-regulator" is encoded into the name.

I think I prefer the latter but I'm probably not too fussed.

I'd rather use something like -supply for these just on the basis that
you don't always connect things to regulators (for example, speaker
supplies are generally attached direct to the battery).

Re: [RFC PATCH 09/11] DT: regulator: Helper to extract regulator node based on supply name

[Rajendra Nayak]

On Friday 16 September 2011 04:33 AM, Mark Brown wrote:
> On Thu, Sep 15, 2011 at 04:50:35PM -0600, Grant Likely wrote:
>
>> We've got two competing approaches here.  For reg and interrupts, the
>> proposal on the table that we talked about at LPC is to do reg-names
>> and interrupts-names so as to preserve the existing semantics of the
>> reg and interrupts properties.  For gpios we're using the binding
>> "<name>-gpios" for named gpio references.  There isn't the same
>> pressure to preserve existing bindings in that case.  I'm okay with
>> either approach, providing that "-regulator" is encoded into the name.
>
> I think I prefer the latter but I'm probably not too fussed.
>
> I'd rather use something like -supply for these just on the basis that
> you don't always connect things to regulators (for example, speaker
> supplies are generally attached direct to the battery).

Ok, I'll go with <name>-supply then.

Re: [RFC PATCH 07/11] regulator: Make fixed regulator driver extract data from DT

[Mark Brown]

On Thu, Sep 15, 2011 at 04:52:03PM +0530, Rajendra Nayak wrote:
> Modify the fixed regulator driver to extract fixed_voltage_config from
> device tree when passed, instead of getting it through platform_data
> structures (on non-DT builds)

The code you added in the previous patch should be part of this driver.

Re: [RFC PATCH 06/11] DT: regulator: Helper routine to extract fixed_voltage_config

[Mark Brown]

On Thu, Sep 15, 2011 at 04:52:02PM +0530, Rajendra Nayak wrote:

>  .../devicetree/bindings/regulator/regulator.txt    |   19 ++++++++++++
>  drivers/of/of_regulator.c                          |   31 ++++++++++++++++++++
>  include/linux/of_regulator.h                       |    7 ++++
>  3 files changed, 57 insertions(+), 0 deletions(-)

Again, this should be part of the regulator API code not hidden away
where it will only get reviewed by device tree people.

>  Required properties:
>  - compatible: Must be "regulator";
> +or
> +- compatible: Must be "regulator-fixed"; /* For Fixed volatge regulator */

This seems at best confusing - the fixed voltage regulator is a
particular regulator driver, and the general concept of a fixed voltage
regulator is still a subset of regulators.

> +# For fixed voltage regulators
> +- supply-name: Name of the regulator supply
> +- microvolts: Output voltage of regulator
> +- gpio: gpio to use for enable control
> +- startup-delay: startup time in microseconds
> +- enable-high: Polarity of enable GPIO, 1 = Active High, 0 = Active low
> +- enabled-at-boot: 1 = yes, 0 = no

Much of this is specific to the Linux fixed voltage regulator driver
rather than a generic regulator with a fixed voltage.

Re: [RFC PATCH 06/11] DT: regulator: Helper routine to extract fixed_voltage_config

[Rajendra Nayak]

On Thursday 15 September 2011 07:20 PM, Mark Brown wrote:
> On Thu, Sep 15, 2011 at 04:52:02PM +0530, Rajendra Nayak wrote:
>
>>   .../devicetree/bindings/regulator/regulator.txt    |   19 ++++++++++++
>>   drivers/of/of_regulator.c                          |   31 ++++++++++++++++++++
>>   include/linux/of_regulator.h                       |    7 ++++
>>   3 files changed, 57 insertions(+), 0 deletions(-)
>
> Again, this should be part of the regulator API code not hidden away
> where it will only get reviewed by device tree people.

Ok, will do.

>
>>   Required properties:
>>   - compatible: Must be "regulator";
>> +or
>> +- compatible: Must be "regulator-fixed"; /* For Fixed volatge regulator */
>
> This seems at best confusing - the fixed voltage regulator is a
> particular regulator driver, and the general concept of a fixed voltage
> regulator is still a subset of regulators.

Yeah, I will be defining the compatible based on the different regulator
types, fixed being one of them.
so they would be something like
- compatible: Must be "regulator-twl" for twl regulators
- compatible: Must be "regulator-fixed" for fixed regulators

>
>> +# For fixed voltage regulators
>> +- supply-name: Name of the regulator supply
>> +- microvolts: Output voltage of regulator
>> +- gpio: gpio to use for enable control
>> +- startup-delay: startup time in microseconds
>> +- enable-high: Polarity of enable GPIO, 1 = Active High, 0 = Active low
>> +- enabled-at-boot: 1 = yes, 0 = no
>
> Much of this is specific to the Linux fixed voltage regulator driver
> rather than a generic regulator with a fixed voltage.

So how should these non-generic things be handled through device
tree? Should they never be part of dt or should the bindings just be
defined such that its clear they are linux specific?

Re: [RFC PATCH 06/11] DT: regulator: Helper routine to extract fixed_voltage_config

[Mark Brown]

On Fri, Sep 16, 2011 at 12:49:17PM +0530, Rajendra Nayak wrote:
> On Thursday 15 September 2011 07:20 PM, Mark Brown wrote:
> >On Thu, Sep 15, 2011 at 04:52:02PM +0530, Rajendra Nayak wrote:

> >>+# For fixed voltage regulators
> >>+- supply-name: Name of the regulator supply
> >>+- microvolts: Output voltage of regulator
> >>+- gpio: gpio to use for enable control
> >>+- startup-delay: startup time in microseconds
> >>+- enable-high: Polarity of enable GPIO, 1 = Active High, 0 = Active low
> >>+- enabled-at-boot: 1 = yes, 0 = no

> >Much of this is specific to the Linux fixed voltage regulator driver
> >rather than a generic regulator with a fixed voltage.

> So how should these non-generic things be handled through device
> tree? Should they never be part of dt or should the bindings just be
> defined such that its clear they are linux specific?

Having them be part of the fixed voltage regulator bindings would be
fine, the problem is that you're adding this in framework code not in a
driver.

Re: [RFC PATCH 06/11] DT: regulator: Helper routine to extract fixed_voltage_config

[Rajendra Nayak]

On Friday 16 September 2011 02:31 PM, Mark Brown wrote:
> On Fri, Sep 16, 2011 at 12:49:17PM +0530, Rajendra Nayak wrote:
>> On Thursday 15 September 2011 07:20 PM, Mark Brown wrote:
>>> On Thu, Sep 15, 2011 at 04:52:02PM +0530, Rajendra Nayak wrote:
>
>>>> +# For fixed voltage regulators
>>>> +- supply-name: Name of the regulator supply
>>>> +- microvolts: Output voltage of regulator
>>>> +- gpio: gpio to use for enable control
>>>> +- startup-delay: startup time in microseconds
>>>> +- enable-high: Polarity of enable GPIO, 1 = Active High, 0 = Active low
>>>> +- enabled-at-boot: 1 = yes, 0 = no
>
>>> Much of this is specific to the Linux fixed voltage regulator driver
>>> rather than a generic regulator with a fixed voltage.
>
>> So how should these non-generic things be handled through device
>> tree? Should they never be part of dt or should the bindings just be
>> defined such that its clear they are linux specific?
>
> Having them be part of the fixed voltage regulator bindings would be
> fine, the problem is that you're adding this in framework code not in a
> driver.

agree, will make the bindings fixed voltage regulator specific.

Re: [RFC PATCH 06/11] DT: regulator: Helper routine to extract fixed_voltage_config

[Grant Likely]

On Thu, Sep 15, 2011 at 04:52:02PM +0530, Rajendra Nayak wrote:
> The helper routine defined here (of_get_fixed_voltage_config) can
> be used to extract information about fixed regulators (which are not
> software controlable) from device tree.
> 
> Add documenation about additional bindings for fixed
> regulators that can be passed through DT.
> 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
>  .../devicetree/bindings/regulator/regulator.txt    |   19 ++++++++++++
>  drivers/of/of_regulator.c                          |   31 ++++++++++++++++++++
>  include/linux/of_regulator.h                       |    7 ++++
>  3 files changed, 57 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
> index 001e5ce..f8c51d8 100644
> --- a/Documentation/devicetree/bindings/regulator/regulator.txt
> +++ b/Documentation/devicetree/bindings/regulator/regulator.txt
> @@ -2,6 +2,8 @@ Voltage/Current Regulators
>  
>  Required properties:
>  - compatible: Must be "regulator";
> +or
> +- compatible: Must be "regulator-fixed"; /* For Fixed volatge regulator */

regulator-fixed should actually just be another driver binding that
uses the regulator binding.

>  
>  Optional properties:
>  - supply-regulator: Name of the parent regulator
> @@ -24,6 +26,13 @@ Optional properties:
>  - always-on: boolean, regulator should never be disabled
>  - boot-on: bootloader/firmware enabled regulator
>  - apply-uV: apply uV constraint if min == max
> +# For fixed voltage regulators
> +- supply-name: Name of the regulator supply
> +- microvolts: Output voltage of regulator
> +- gpio: gpio to use for enable control
> +- startup-delay: startup time in microseconds
> +- enable-high: Polarity of enable GPIO, 1 = Active High, 0 = Active low
> +- enabled-at-boot: 1 = yes, 0 = no
>  
>  Example:
>  
> @@ -35,3 +44,13 @@ Example:
>  		change-voltage;
>  		always-on;
>  	};
> +
> +	abc-fixedregulator {
> +		compatible = "regulator-fixed";
> +		supply-name = "fixed-supply";
> +		microvolts = <1800000>;
> +		gpio = <43>;
> +		startup-delay = <70000>;
> +		enable-high;
> +		enabled-at-boot;
> +	};
> diff --git a/drivers/of/of_regulator.c b/drivers/of/of_regulator.c
> index f01d275..4d7a49d 100644
> --- a/drivers/of/of_regulator.c
> +++ b/drivers/of/of_regulator.c
> @@ -13,6 +13,7 @@
>  #include <linux/slab.h>
>  #include <linux/of.h>
>  #include <linux/regulator/machine.h>
> +#include <linux/regulator/fixed.h>
>  
>  static void of_get_regulation_constraints(struct device_node *np,
>  					struct regulator_init_data **init_data)
> @@ -83,3 +84,33 @@ struct regulator_init_data *of_get_regulator_init_data(struct device_node *np)
>  	return init_data;
>  }
>  EXPORT_SYMBOL(of_get_regulator_init_data);
> +
> +/**
> + * of_get_fixed_voltage_config - extract fixed_voltage_config structure info
> + * @np: Pointer to fixed-regulator device tree node
> + *
> + * Populates fixed_voltage_config structure by extracting data from device
> + * tree node, returns a pointer to the populated structure of NULL if memory
> + * alloc fails.
> + */
> +struct fixed_voltage_config *of_get_fixed_voltage_config(struct device_node *np)
> +{
> +	struct fixed_voltage_config *config;
> +
> +	config = kzalloc(sizeof(struct fixed_voltage_config), GFP_KERNEL);
> +	if (!config)
> +		return NULL;
> +
> +	config->supply_name = (char *)of_get_property(np, "supply-name", NULL);
> +	of_property_read_u32(np, "microvolts", &config->microvolts);
> +	of_property_read_u32(np, "gpio", &config->gpio);
> +	of_property_read_u32(np, "startup-delay", &config->startup_delay);
> +	if (of_find_property(np, "enable-high", NULL))
> +		config->enable_high = true;
> +	if (of_find_property(np, "enabled-at-boot", NULL))
> +		config->enabled_at_boot = true;
> +	config->init_data = of_get_regulator_init_data(np);
> +
> +	return config;
> +}
> +EXPORT_SYMBOL(of_get_fixed_voltage_config);
> diff --git a/include/linux/of_regulator.h b/include/linux/of_regulator.h
> index 5eb048c..39860c5 100644
> --- a/include/linux/of_regulator.h
> +++ b/include/linux/of_regulator.h
> @@ -11,12 +11,19 @@
>  #if defined(CONFIG_OF_REGULATOR)
>  extern struct regulator_init_data
>  	*of_get_regulator_init_data(struct device_node *np);
> +extern struct fixed_voltage_config
> +	*of_get_fixed_voltage_config(struct device_node *np);
>  #else
>  static inline struct regulator_init_data
>  	*of_get_regulator_init_data(struct device_node *np)
>  {
>  	return NULL;
>  }
> +static inline struct fixed_voltage_config
> +	*of_get_fixed_voltage_config(struct device_node *np)
> +{
> +	return NULL;
> +}
>  #endif /* CONFIG_OF_REGULATOR */
>  
>  #endif /* __LINUX_OF_REG_H */
> -- 
> 1.7.1
> 

Re: [RFC PATCH 05/11] TWL: regulator: Make twl-regulator driver extract data from DT

[Grant Likely]

On Thu, Sep 15, 2011 at 04:52:01PM +0530, Rajendra Nayak wrote:
> Modify the twl regulator driver to extract the regulator_init_data from
> device tree when passed, instead of getting it through platform_data
> structures (on non-DT builds)
> 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
>  drivers/regulator/twl-regulator.c |   28 +++++++++++++++++++++++++---
>  1 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/regulator/twl-regulator.c b/drivers/regulator/twl-regulator.c
> index ee8747f..df1b95a 100644
> --- a/drivers/regulator/twl-regulator.c
> +++ b/drivers/regulator/twl-regulator.c
> @@ -17,6 +17,8 @@
>  #include <linux/regulator/driver.h>
>  #include <linux/regulator/machine.h>
>  #include <linux/i2c/twl.h>
> +#include <linux/of.h>
> +#include <linux/of_regulator.h>
>  
>  
>  /*
> @@ -1011,6 +1013,9 @@ static int __devinit twlreg_probe(struct platform_device *pdev)
>  	struct regulation_constraints	*c;
>  	struct regulator_dev		*rdev;
>  
> +	if (pdev->dev.of_node)
> +		of_property_read_u32(pdev->dev.of_node, "ti,reg-id", &pdev->id);
> +

Don't do this.  As much as possible, don't reply on plaform_device->id
when using DT.  Plus it is illegal to modify pdev->id after the device
is registered.

>  	for (i = 0, info = NULL; i < ARRAY_SIZE(twl_regs); i++) {
>  		if (twl_regs[i].desc.id != pdev->id)
>  			continue;
> @@ -1020,7 +1025,11 @@ static int __devinit twlreg_probe(struct platform_device *pdev)
>  	if (!info)
>  		return -ENODEV;
>  
> -	initdata = pdev->dev.platform_data;
> +	if (pdev->dev.of_node)
> +		initdata = of_get_regulator_init_data(pdev->dev.of_node);
> +	else
> +		initdata = pdev->dev.platform_data;
> +
>  	if (!initdata)
>  		return -EINVAL;
>  
> @@ -1101,14 +1110,27 @@ static int __devexit twlreg_remove(struct platform_device *pdev)
>  
>  MODULE_ALIAS("platform:twl_reg");
>  
> +#if defined(CONFIG_OF)
> +static const struct of_device_id twl_of_match[] __devinitconst = {
> +	{ .compatible = "ti,twl-reg", },

This looks rather generic.  Is this a specific chip?  It should be.

g.

> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, twl_of_match);
> +#else
> +#define twl_of_match NULL
> +#endif
> +
>  static struct platform_driver twlreg_driver = {
>  	.probe		= twlreg_probe,
>  	.remove		= __devexit_p(twlreg_remove),
>  	/* NOTE: short name, to work around driver model truncation of
>  	 * "twl_regulator.12" (and friends) to "twl_regulator.1".
>  	 */
> -	.driver.name	= "twl_reg",
> -	.driver.owner	= THIS_MODULE,
> +	.driver  = {
> +		.name  = "twl_reg",
> +		.owner = THIS_MODULE,
> +		.of_match_table = twl_of_match,
> +	},
>  };
>  
>  static int __init twlreg_init(void)
> -- 
> 1.7.1
> 

Re: [RFC PATCH 05/11] TWL: regulator: Make twl-regulator driver extract data from DT

[Rajendra Nayak]

On Friday 16 September 2011 03:48 AM, Grant Likely wrote:
> On Thu, Sep 15, 2011 at 04:52:01PM +0530, Rajendra Nayak wrote:
>> Modify the twl regulator driver to extract the regulator_init_data from
>> device tree when passed, instead of getting it through platform_data
>> structures (on non-DT builds)
>>
>> Signed-off-by: Rajendra Nayak<rnayak@ti.com>
>> ---
>>   drivers/regulator/twl-regulator.c |   28 +++++++++++++++++++++++++---
>>   1 files changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/regulator/twl-regulator.c b/drivers/regulator/twl-regulator.c
>> index ee8747f..df1b95a 100644
>> --- a/drivers/regulator/twl-regulator.c
>> +++ b/drivers/regulator/twl-regulator.c
>> @@ -17,6 +17,8 @@
>>   #include<linux/regulator/driver.h>
>>   #include<linux/regulator/machine.h>
>>   #include<linux/i2c/twl.h>
>> +#include<linux/of.h>
>> +#include<linux/of_regulator.h>
>>
>>
>>   /*
>> @@ -1011,6 +1013,9 @@ static int __devinit twlreg_probe(struct platform_device *pdev)
>>   	struct regulation_constraints	*c;
>>   	struct regulator_dev		*rdev;
>>
>> +	if (pdev->dev.of_node)
>> +		of_property_read_u32(pdev->dev.of_node, "ti,reg-id",&pdev->id);
>> +
>
> Don't do this.  As much as possible, don't reply on plaform_device->id
> when using DT.  Plus it is illegal to modify pdev->id after the device
> is registered.

yeah, I did this hackery to just get around the drivers per-regulator
lookup table for which it uses the pdev->id as the index.
I will need to do this lookup based on compatible instead I guess.

>
>>   	for (i = 0, info = NULL; i<  ARRAY_SIZE(twl_regs); i++) {
>>   		if (twl_regs[i].desc.id != pdev->id)
>>   			continue;
>> @@ -1020,7 +1025,11 @@ static int __devinit twlreg_probe(struct platform_device *pdev)
>>   	if (!info)
>>   		return -ENODEV;
>>
>> -	initdata = pdev->dev.platform_data;
>> +	if (pdev->dev.of_node)
>> +		initdata = of_get_regulator_init_data(pdev->dev.of_node);
>> +	else
>> +		initdata = pdev->dev.platform_data;
>> +
>>   	if (!initdata)
>>   		return -EINVAL;
>>
>> @@ -1101,14 +1110,27 @@ static int __devexit twlreg_remove(struct platform_device *pdev)
>>
>>   MODULE_ALIAS("platform:twl_reg");
>>
>> +#if defined(CONFIG_OF)
>> +static const struct of_device_id twl_of_match[] __devinitconst = {
>> +	{ .compatible = "ti,twl-reg", },
>
> This looks rather generic.  Is this a specific chip?  It should be.

We have multiple chips in the twl family like twl6030/twl6040/twl6025
but just one driver which handles all variants.

>
> g.
>
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, twl_of_match);
>> +#else
>> +#define twl_of_match NULL
>> +#endif
>> +
>>   static struct platform_driver twlreg_driver = {
>>   	.probe		= twlreg_probe,
>>   	.remove		= __devexit_p(twlreg_remove),
>>   	/* NOTE: short name, to work around driver model truncation of
>>   	 * "twl_regulator.12" (and friends) to "twl_regulator.1".
>>   	 */
>> -	.driver.name	= "twl_reg",
>> -	.driver.owner	= THIS_MODULE,
>> +	.driver  = {
>> +		.name  = "twl_reg",
>> +		.owner = THIS_MODULE,
>> +		.of_match_table = twl_of_match,
>> +	},
>>   };
>>
>>   static int __init twlreg_init(void)
>> --
>> 1.7.1
>>

Re: [RFC PATCH 04/11] omap4: SDP: Pass regulator_init_data from DT

[Mark Brown]

On Thu, Sep 15, 2011 at 04:52:00PM +0530, Rajendra Nayak wrote:

> +Required properties:
> +- compatible: Must be "regulator","ti,twl-reg";

I'd expect listings for the specific chips too.

> +	xyz-regulator: regulator@0 {
> +		compatible = "regulator","ti,twl-reg";
> +		ti,reg-id = <37>; /* TWL6030_REG_VAUX1_6030 */

These magic numbers are *very* Linux specific, we should have a better
way of specifying regulators - I'd off the top of my head expect that
the compatible property would identify the regulator.

Re: [RFC PATCH 04/11] omap4: SDP: Pass regulator_init_data from DT

[Grant Likely]

On Thu, Sep 15, 2011 at 02:46:18PM +0100, Mark Brown wrote:
> On Thu, Sep 15, 2011 at 04:52:00PM +0530, Rajendra Nayak wrote:
> 
> > +Required properties:
> > +- compatible: Must be "regulator","ti,twl-reg";
> 
> I'd expect listings for the specific chips too.
> 
> > +	xyz-regulator: regulator@0 {
> > +		compatible = "regulator","ti,twl-reg";
> > +		ti,reg-id = <37>; /* TWL6030_REG_VAUX1_6030 */
> 
> These magic numbers are *very* Linux specific, we should have a better
> way of specifying regulators - I'd off the top of my head expect that
> the compatible property would identify the regulator.

Yes, that is exactly what it should do.

g.

Re: [RFC PATCH 04/11] omap4: SDP: Pass regulator_init_data from DT

[Rajendra Nayak]

On Thursday 15 September 2011 07:16 PM, Mark Brown wrote:
> On Thu, Sep 15, 2011 at 04:52:00PM +0530, Rajendra Nayak wrote:
>
>> +Required properties:
>> +- compatible: Must be "regulator","ti,twl-reg";
>
> I'd expect listings for the specific chips too.

I just did'nt do that because we have just one driver for
all twl chips (twl4030/twl6030/twl6025) and there seems to be
no real need to identify specific chips while we could
do knowing just the chip family.

>
>> +	xyz-regulator: regulator@0 {
>> +		compatible = "regulator","ti,twl-reg";
>> +		ti,reg-id =<37>; /* TWL6030_REG_VAUX1_6030 */
>
> These magic numbers are *very* Linux specific, we should have a better
> way of specifying regulators - I'd off the top of my head expect that
> the compatible property would identify the regulator.

The driver seems to use a per-regulator table, and it uses the above
id to indexed into it. I could probably do it with the compatible
property, but that would mean I have a compatible for *each* regulator
instance, like "ti,twl-reg-vaux1", "ti,twl-reg-vmmc" etc.
Does that sound reasonable?

Re: [RFC PATCH 04/11] omap4: SDP: Pass regulator_init_data from DT

[Mark Brown]

On Fri, Sep 16, 2011 at 12:47:05PM +0530, Rajendra Nayak wrote:
> On Thursday 15 September 2011 07:16 PM, Mark Brown wrote:
> >On Thu, Sep 15, 2011 at 04:52:00PM +0530, Rajendra Nayak wrote:

> >>+Required properties:
> >>+- compatible: Must be "regulator","ti,twl-reg";

> >I'd expect listings for the specific chips too.

> I just did'nt do that because we have just one driver for
> all twl chips (twl4030/twl6030/twl6025) and there seems to be
> no real need to identify specific chips while we could
> do knowing just the chip family.

The driver can bind to as many names as it likes.

> >>+	xyz-regulator: regulator@0 {
> >>+		compatible = "regulator","ti,twl-reg";
> >>+		ti,reg-id =<37>; /* TWL6030_REG_VAUX1_6030 */

> >These magic numbers are *very* Linux specific, we should have a better
> >way of specifying regulators - I'd off the top of my head expect that
> >the compatible property would identify the regulator.

> The driver seems to use a per-regulator table, and it uses the above
> id to indexed into it. I could probably do it with the compatible

I know what the driver is doing, the problem is that it's very much
specific to Linux (and Linux may change the numbers at some point).

> property, but that would mean I have a compatible for *each* regulator
> instance, like "ti,twl-reg-vaux1", "ti,twl-reg-vmmc" etc.
> Does that sound reasonable?

Yes.

Re: [RFC PATCH 04/11] omap4: SDP: Pass regulator_init_data from DT

[Rajendra Nayak]

On Friday 16 September 2011 02:30 PM, Mark Brown wrote:
> On Fri, Sep 16, 2011 at 12:47:05PM +0530, Rajendra Nayak wrote:
>> On Thursday 15 September 2011 07:16 PM, Mark Brown wrote:
>>> On Thu, Sep 15, 2011 at 04:52:00PM +0530, Rajendra Nayak wrote:
>
>>>> +Required properties:
>>>> +- compatible: Must be "regulator","ti,twl-reg";
>
>>> I'd expect listings for the specific chips too.
>
>> I just did'nt do that because we have just one driver for
>> all twl chips (twl4030/twl6030/twl6025) and there seems to be
>> no real need to identify specific chips while we could
>> do knowing just the chip family.
>
> The driver can bind to as many names as it likes.

makes sense, I'll add one per chip variant.

>
>>>> +	xyz-regulator: regulator@0 {
>>>> +		compatible = "regulator","ti,twl-reg";
>>>> +		ti,reg-id =<37>; /* TWL6030_REG_VAUX1_6030 */
>
>>> These magic numbers are *very* Linux specific, we should have a better
>>> way of specifying regulators - I'd off the top of my head expect that
>>> the compatible property would identify the regulator.
>
>> The driver seems to use a per-regulator table, and it uses the above
>> id to indexed into it. I could probably do it with the compatible
>
> I know what the driver is doing, the problem is that it's very much
> specific to Linux (and Linux may change the numbers at some point).
>
>> property, but that would mean I have a compatible for *each* regulator
>> instance, like "ti,twl-reg-vaux1", "ti,twl-reg-vmmc" etc.
>> Does that sound reasonable?
>
> Yes.

Re: [RFC PATCH 04/11] omap4: SDP: Pass regulator_init_data from DT

[Grant Likely]

On Thu, Sep 15, 2011 at 04:52:00PM +0530, Rajendra Nayak wrote:
> Pass regulator_init_data information for omap4sdp from device tree so the
> regulator driver can then use the regulator helper
> routine to extract and use them during the driver probe().
> 
> Add documentation for TWL regulator specific bindings.
> 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
>  .../bindings/regulator/twl-regulator.txt           |   18 ++++++++++++++++++
>  arch/arm/boot/dts/omap4-sdp.dts                    |   16 ++++++++++++++++
>  2 files changed, 34 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/regulator/twl-regulator.txt
> 
> diff --git a/Documentation/devicetree/bindings/regulator/twl-regulator.txt b/Documentation/devicetree/bindings/regulator/twl-regulator.txt
> new file mode 100644
> index 0000000..59df44b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/twl-regulator.txt
> @@ -0,0 +1,18 @@
> +TWL family of regulators
> +
> +Required properties:
> +- compatible: Must be "regulator","ti,twl-reg";

Most specific values always come first.  Of course, this point is moot
since "regulator" shouldn't be used at all.

> +
> +Optional properties:
> +- Any optional property defined in bindings/regulator/regulator.txt
> +- ti,reg-id: Identifier for a TWL regulator, defined in linux/i2c/twl.h file.

This needs more explanation.  I don't understand what it means.

> +
> +Example:
> +
> +	xyz-regulator: regulator@0 {
> +		compatible = "regulator","ti,twl-reg";
> +		ti,reg-id = <37>; /* TWL6030_REG_VAUX1_6030 */
> +		min-uV  = <1000000>;
> +		max-uV  = <3000000>;
> +		apply-uV;
> +	};
> diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
> index 14faf92..2a613e2 100644
> --- a/arch/arm/boot/dts/omap4-sdp.dts
> +++ b/arch/arm/boot/dts/omap4-sdp.dts
> @@ -52,6 +52,22 @@
>  			interrupts = <11>;
>  			reg = <0>;
>  		};
> +
> +		vaux1-regulator: regulator@0 {
> +			compatible = "regulator","ti,twl-reg";
> +			ti,reg-id = <37>;  /* TWL6030_REG_VAUX1_6030 */
> +			min-uV = <1000000>;
> +			max-uV = <3000000>;
> +			apply-uV;
> +		};
> +
> +		vusim-regulator: regulator@1 {
> +			compatible = "regulator","ti,twl-reg";
> +			ti,reg-id = <42>; /* TWL6030_REG_VUSIM */
> +			min-uV = <1200000>;
> +			max-uV = <2900000>;
> +			apply_uV;
> +		};
>  	};
>  };
>  
> -- 
> 1.7.1
> 

Re: [RFC PATCH 03/11] DT: regulator: Helper routine to extract regulator_init_data

[Mark Brown]

On Thu, Sep 15, 2011 at 04:51:59PM +0530, Rajendra Nayak wrote:

>  .../devicetree/bindings/regulator/regulator.txt    |   37 +++++++++
>  drivers/of/Kconfig                                 |    6 ++
>  drivers/of/Makefile                                |    1 +
>  drivers/of/of_regulator.c                          |   85 ++++++++++++++++++++
>  include/linux/of_regulator.h                       |   23 +++++

Don't go hiding the bindings for things away from the code they're
binding.  Bindings for the regualtor API are a regulator API thing and
should be part of the regulator API code.

> +Required properties:
> +- compatible: Must be "regulator";

Is this idiomatic or should we just have a helper that parses a big list
of properties from the device node?

> +- mode-fast: boolean, Can handle fast changes in its load
> +- mode-normal: boolean, Normal regulator power supply mode
> +- mode-idle: boolean, Can be more efficient during light loads
> +- mode-standby: boolean, Can be most efficient during light loads

I guess these are actually permissions to set the given modes?  The
documentation should be clearer.

> +- apply-uV: apply uV constraint if min == max

This seems a bit Linux/runtime policy specific (especially the last
bit).

Re: [RFC PATCH 03/11] DT: regulator: Helper routine to extract regulator_init_data

[Rob Herring]

On 09/15/2011 08:44 AM, Mark Brown wrote:
> On Thu, Sep 15, 2011 at 04:51:59PM +0530, Rajendra Nayak wrote:
> 
>>  .../devicetree/bindings/regulator/regulator.txt    |   37 +++++++++
>>  drivers/of/Kconfig                                 |    6 ++
>>  drivers/of/Makefile                                |    1 +
>>  drivers/of/of_regulator.c                          |   85 ++++++++++++++++++++
>>  include/linux/of_regulator.h                       |   23 +++++
> 
> Don't go hiding the bindings for things away from the code they're
> binding.  Bindings for the regualtor API are a regulator API thing and
> should be part of the regulator API code.
> 

Agreed, but FYI not all subsystem maintainers agree. Moving of_i2c.c
into i2c was rejected.

Rob

>> +Required properties:
>> +- compatible: Must be "regulator";
> 
> Is this idiomatic or should we just have a helper that parses a big list
> of properties from the device node?
> 
>> +- mode-fast: boolean, Can handle fast changes in its load
>> +- mode-normal: boolean, Normal regulator power supply mode
>> +- mode-idle: boolean, Can be more efficient during light loads
>> +- mode-standby: boolean, Can be most efficient during light loads
> 
> I guess these are actually permissions to set the given modes?  The
> documentation should be clearer.
> 
>> +- apply-uV: apply uV constraint if min == max
> 
> This seems a bit Linux/runtime policy specific (especially the last
> bit).
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [RFC PATCH 03/11] DT: regulator: Helper routine to extract regulator_init_data

[Rajendra Nayak]

On Thursday 15 September 2011 07:14 PM, Mark Brown wrote:
> On Thu, Sep 15, 2011 at 04:51:59PM +0530, Rajendra Nayak wrote:
>
>>   .../devicetree/bindings/regulator/regulator.txt    |   37 +++++++++
>>   drivers/of/Kconfig                                 |    6 ++
>>   drivers/of/Makefile                                |    1 +
>>   drivers/of/of_regulator.c                          |   85 ++++++++++++++++++++
>>   include/linux/of_regulator.h                       |   23 +++++
>
> Don't go hiding the bindings for things away from the code they're
> binding.  Bindings for the regualtor API are a regulator API thing and
> should be part of the regulator API code.

Sure, Grant seems to think so too, so I'll just move these into
drivers/regulator/

>
>> +Required properties:
>> +- compatible: Must be "regulator";
>
> Is this idiomatic or should we just have a helper that parses a big list
> of properties from the device node?

I will just be dropping this and use compatible
to specify the specific device type like "regulator-twl" or 
"regulator-fixed".

>
>> +- mode-fast: boolean, Can handle fast changes in its load
>> +- mode-normal: boolean, Normal regulator power supply mode
>> +- mode-idle: boolean, Can be more efficient during light loads
>> +- mode-standby: boolean, Can be most efficient during light loads
>
> I guess these are actually permissions to set the given modes?  The
> documentation should be clearer.

Ok

>
>> +- apply-uV: apply uV constraint if min == max
>
> This seems a bit Linux/runtime policy specific (especially the last
> bit).

So these bindings should be defined differently?

Re: [RFC PATCH 03/11] DT: regulator: Helper routine to extract regulator_init_data

[Mark Brown]

On Fri, Sep 16, 2011 at 12:45:55PM +0530, Rajendra Nayak wrote:
> On Thursday 15 September 2011 07:14 PM, Mark Brown wrote:
> >On Thu, Sep 15, 2011 at 04:51:59PM +0530, Rajendra Nayak wrote:

> >>+- apply-uV: apply uV constraint if min == max

> >This seems a bit Linux/runtime policy specific (especially the last
> >bit).

> So these bindings should be defined differently?

If at all.

Re: [RFC PATCH 03/11] DT: regulator: Helper routine to extract regulator_init_data

[Rob Herring]

On 09/15/2011 06:21 AM, Rajendra Nayak wrote:
> The helper routine is meant to be used by regulator drivers
> to extract the regulator_init_data structure passed from device tree.
> 'consumer_supplies' which is part of regulator_init_data is not extracted
> as the regulator consumer mappings are passed through DT differently,
> implemented in subsequent patches.
> 
> Also add documentation for regulator bindings to be used to pass
> regulator_init_data struct information from device tree.
> 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
>  .../devicetree/bindings/regulator/regulator.txt    |   37 +++++++++
>  drivers/of/Kconfig                                 |    6 ++
>  drivers/of/Makefile                                |    1 +
>  drivers/of/of_regulator.c                          |   85 ++++++++++++++++++++
>  include/linux/of_regulator.h                       |   23 +++++
>  5 files changed, 152 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/regulator/regulator.txt
>  create mode 100644 drivers/of/of_regulator.c
>  create mode 100644 include/linux/of_regulator.h
> 
> diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
> new file mode 100644
> index 0000000..001e5ce
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/regulator.txt
> @@ -0,0 +1,37 @@
> +Voltage/Current Regulators
> +
> +Required properties:
> +- compatible: Must be "regulator";

This is way too generic. compatible is not for matching a class of
devices. It is for matching a specific piece of h/w. So I would expect
names like "wolfson,wm8350-dcdc".


> +
> +Optional properties:
> +- supply-regulator: Name of the parent regulator
> +- min-uV: smallest voltage consumers may set
> +- max-uV: largest voltage consumers may set
> +- uV-offset: Offset applied to voltages from consumer to compensate for voltage drops
> +- min-uA: smallest current consumers may set
> +- max-uA: largest current consumers may set
> +- mode-fast: boolean, Can handle fast changes in its load
> +- mode-normal: boolean, Normal regulator power supply mode
> +- mode-idle: boolean, Can be more efficient during light loads
> +- mode-standby: boolean, Can be most efficient during light loads
> +- change-voltage: boolean, Output voltage can be changed by software
> +- change-current: boolean, Output current can be chnaged by software
> +- change-mode: boolean, Operating mode can be changed by software
> +- change-status: boolean, Enable/Disable control for regulator exists
> +- change-drms: boolean, Dynamic regulator mode switching is enabled
> +- input-uV: Input voltage for regulator when supplied by another regulator
> +- initial-mode: Mode to set at startup
> +- always-on: boolean, regulator should never be disabled
> +- boot-on: bootloader/firmware enabled regulator
> +- apply-uV: apply uV constraint if min == max
> +
> +Example:
> +
> +	xyz-regulator: regulator@0 {
> +		compatible = "regulator";
> +		min-uV = <1000000>;
> +		max-uV = <2500000>;
> +		mode-fast;
> +		change-voltage;
> +		always-on;
> +	};

You need to show how a node references the regulator.

Rob

> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index b3bfea5..296b96d 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -87,4 +87,10 @@ config OF_PCI_IRQ
>  	help
>  	  OpenFirmware PCI IRQ routing helpers
>  
> +config OF_REGULATOR
> +	def_bool y
> +	depends on REGULATOR
> +	help
> +	  OpenFirmware regulator framework helpers
> +
>  endmenu # OF
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index 74b959c..bea2852 100644
> --- a/drivers/of/Makefile
> +++ b/drivers/of/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_OF_SPI)	+= of_spi.o
>  obj-$(CONFIG_OF_MDIO)	+= of_mdio.o
>  obj-$(CONFIG_OF_PCI)	+= of_pci.o
>  obj-$(CONFIG_OF_PCI_IRQ)  += of_pci_irq.o
> +obj-$(CONFIG_OF_REGULATOR) += of_regulator.o
> diff --git a/drivers/of/of_regulator.c b/drivers/of/of_regulator.c
> new file mode 100644
> index 0000000..f01d275
> --- /dev/null
> +++ b/drivers/of/of_regulator.c
> @@ -0,0 +1,85 @@
> +/*
> + * OF helpers for regulator framework
> + *
> + * Copyright (C) 2011 Texas Instruments, Inc.
> + * Rajendra Nayak <rnayak@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/regulator/machine.h>
> +
> +static void of_get_regulation_constraints(struct device_node *np,
> +					struct regulator_init_data **init_data)
> +{
> +	unsigned int valid_modes_mask = 0, valid_ops_mask = 0;
> +
> +	of_property_read_u32(np, "min-uV", &(*init_data)->constraints.min_uV);
> +	of_property_read_u32(np, "max-uV", &(*init_data)->constraints.max_uV);
> +	of_property_read_u32(np, "uV-offset", &(*init_data)->constraints.uV_offset);
> +	of_property_read_u32(np, "min-uA", &(*init_data)->constraints.min_uA);
> +	of_property_read_u32(np, "max-uA", &(*init_data)->constraints.max_uA);
> +
> +	/* valid modes mask */
> +	if (of_find_property(np, "mode-fast", NULL))
> +		valid_modes_mask |= REGULATOR_MODE_FAST;
> +	if (of_find_property(np, "mode-normal", NULL))
> +		valid_modes_mask |= REGULATOR_MODE_NORMAL;
> +	if (of_find_property(np, "mode-idle", NULL))
> +		valid_modes_mask |= REGULATOR_MODE_IDLE;
> +	if (of_find_property(np, "mode-standby", NULL))
> +		valid_modes_mask |= REGULATOR_MODE_STANDBY;
> +
> +	/* valid ops mask */
> +	if (of_find_property(np, "change-voltage", NULL))
> +		valid_ops_mask |= REGULATOR_CHANGE_VOLTAGE;
> +	if (of_find_property(np, "change-current", NULL))
> +		valid_ops_mask |= REGULATOR_CHANGE_CURRENT;
> +	if (of_find_property(np, "change-mode", NULL))
> +		valid_ops_mask |= REGULATOR_CHANGE_MODE;
> +	if (of_find_property(np, "change-status", NULL))
> +		valid_ops_mask |= REGULATOR_CHANGE_STATUS;
> +
> +	(*init_data)->constraints.valid_modes_mask = valid_modes_mask;
> +	(*init_data)->constraints.valid_ops_mask = valid_ops_mask;
> +
> +	of_property_read_u32(np, "input-uV",
> +			&(*init_data)->constraints.input_uV);
> +	of_property_read_u32(np, "initial-mode",
> +			&(*init_data)->constraints.initial_mode);
> +
> +	if (of_find_property(np, "always-on", NULL))
> +			(*init_data)->constraints.always_on = true;
> +	if (of_find_property(np, "boot-on", NULL))
> +			(*init_data)->constraints.boot_on = true;
> +	if (of_find_property(np, "apply-uV", NULL))
> +			(*init_data)->constraints.apply_uV = true;
> +}
> +
> +/**
> + * of_get_regulator_init_data - extarct regulator_init_data structure info
> + * @np: Pointer to regulator device tree node
> + *
> + * Populates regulator_init_data structure by extracting data from device
> + * tree node, returns a pointer to the populated struture or NULL if memory
> + * alloc fails.
> + */
> +struct regulator_init_data *of_get_regulator_init_data(struct device_node *np)
> +{
> +	struct regulator_init_data *init_data;
> +
> +	init_data = kzalloc(sizeof(struct regulator_init_data), GFP_KERNEL);
> +	if (!init_data)
> +		return NULL; /* Out of memory? */
> +
> +	init_data->supply_regulator = (char *)of_get_property(np,
> +						"supply-regulator", NULL);
> +	of_get_regulation_constraints(np, &init_data);
> +	return init_data;
> +}
> +EXPORT_SYMBOL(of_get_regulator_init_data);
> diff --git a/include/linux/of_regulator.h b/include/linux/of_regulator.h
> new file mode 100644
> index 0000000..5eb048c
> --- /dev/null
> +++ b/include/linux/of_regulator.h
> @@ -0,0 +1,23 @@
> +/*
> + * OpenFirmware regulator support routines
> + *
> + */
> +
> +#ifndef __LINUX_OF_REG_H
> +#define __LINUX_OF_REG_H
> +
> +#include <linux/regulator/machine.h>
> +
> +#if defined(CONFIG_OF_REGULATOR)
> +extern struct regulator_init_data
> +	*of_get_regulator_init_data(struct device_node *np);
> +#else
> +static inline struct regulator_init_data
> +	*of_get_regulator_init_data(struct device_node *np)
> +{
> +	return NULL;
> +}
> +#endif /* CONFIG_OF_REGULATOR */
> +
> +#endif /* __LINUX_OF_REG_H */
> +

Re: [RFC PATCH 03/11] DT: regulator: Helper routine to extract regulator_init_data

[Grant Likely]

On Thu, Sep 15, 2011 at 04:51:59PM +0530, Rajendra Nayak wrote:
> The helper routine is meant to be used by regulator drivers
> to extract the regulator_init_data structure passed from device tree.
> 'consumer_supplies' which is part of regulator_init_data is not extracted
> as the regulator consumer mappings are passed through DT differently,
> implemented in subsequent patches.
> 
> Also add documentation for regulator bindings to be used to pass
> regulator_init_data struct information from device tree.
> 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
>  .../devicetree/bindings/regulator/regulator.txt    |   37 +++++++++
>  drivers/of/Kconfig                                 |    6 ++
>  drivers/of/Makefile                                |    1 +
>  drivers/of/of_regulator.c                          |   85 ++++++++++++++++++++

I originally put the DT stuff under drivers/of for i2c and spi because
there was resistance from driver subsystem maintainers to having code
for something that was powerpc only.  Now that it is no longer the
case, I recommend putting this code under drivers/regulator.

>  include/linux/of_regulator.h                       |   23 +++++
>  5 files changed, 152 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/regulator/regulator.txt
>  create mode 100644 drivers/of/of_regulator.c
>  create mode 100644 include/linux/of_regulator.h
> 
> diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
> new file mode 100644
> index 0000000..001e5ce
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/regulator.txt
> @@ -0,0 +1,37 @@
> +Voltage/Current Regulators
> +
> +Required properties:
> +- compatible: Must be "regulator";

A "regulator" compatible doesn't actually help much.  Compatible is
for specifying the specific device, not for trying to describe what
kind of device it is.  Instead, specific regulator bindings can adopt
& extend this binding.

> +
> +Optional properties:
> +- supply-regulator: Name of the parent regulator

If this is a reference to another regulator node then don't use names.
Use phandles instead.  In which case it should probably be named
something like "regulator-parent" (similar to interrupt parent).

However, I can imagine it possible for a regulator to have multiple
parents.  It may just be better to go with something like the gpio
scheme of <phandle gpio-specifier> list of entries.  Or maybe just a
list of phandles would be sufficient.

> +- min-uV: smallest voltage consumers may set
> +- max-uV: largest voltage consumers may set
> +- uV-offset: Offset applied to voltages from consumer to compensate for voltage drops
> +- min-uA: smallest current consumers may set
> +- max-uA: largest current consumers may set
> +- mode-fast: boolean, Can handle fast changes in its load
> +- mode-normal: boolean, Normal regulator power supply mode
> +- mode-idle: boolean, Can be more efficient during light loads
> +- mode-standby: boolean, Can be most efficient during light loads
> +- change-voltage: boolean, Output voltage can be changed by software
> +- change-current: boolean, Output current can be chnaged by software
> +- change-mode: boolean, Operating mode can be changed by software
> +- change-status: boolean, Enable/Disable control for regulator exists
> +- change-drms: boolean, Dynamic regulator mode switching is enabled
> +- input-uV: Input voltage for regulator when supplied by another regulator
> +- initial-mode: Mode to set at startup
> +- always-on: boolean, regulator should never be disabled
> +- boot-on: bootloader/firmware enabled regulator
> +- apply-uV: apply uV constraint if min == max

To avoid collisions, I'd prefix all of these with a common prefix.
Something like regulator-*.

These map 1:1 to how Linux currently implements regulators; which
isn't exactly bad, but it means that even if Linux changes, we're
still have to support this binding.  Does this represent the best
layout for high level description of regulators?

> +
> +Example:
> +
> +	xyz-regulator: regulator@0 {
> +		compatible = "regulator";
> +		min-uV = <1000000>;
> +		max-uV = <2500000>;
> +		mode-fast;
> +		change-voltage;
> +		always-on;
> +	};
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index b3bfea5..296b96d 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -87,4 +87,10 @@ config OF_PCI_IRQ
>  	help
>  	  OpenFirmware PCI IRQ routing helpers
>  
> +config OF_REGULATOR
> +	def_bool y
> +	depends on REGULATOR
> +	help
> +	  OpenFirmware regulator framework helpers
> +
>  endmenu # OF
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index 74b959c..bea2852 100644
> --- a/drivers/of/Makefile
> +++ b/drivers/of/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_OF_SPI)	+= of_spi.o
>  obj-$(CONFIG_OF_MDIO)	+= of_mdio.o
>  obj-$(CONFIG_OF_PCI)	+= of_pci.o
>  obj-$(CONFIG_OF_PCI_IRQ)  += of_pci_irq.o
> +obj-$(CONFIG_OF_REGULATOR) += of_regulator.o
> diff --git a/drivers/of/of_regulator.c b/drivers/of/of_regulator.c
> new file mode 100644
> index 0000000..f01d275
> --- /dev/null
> +++ b/drivers/of/of_regulator.c
> @@ -0,0 +1,85 @@
> +/*
> + * OF helpers for regulator framework
> + *
> + * Copyright (C) 2011 Texas Instruments, Inc.
> + * Rajendra Nayak <rnayak@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/regulator/machine.h>
> +
> +static void of_get_regulation_constraints(struct device_node *np,
> +					struct regulator_init_data **init_data)
> +{
> +	unsigned int valid_modes_mask = 0, valid_ops_mask = 0;
> +
> +	of_property_read_u32(np, "min-uV", &(*init_data)->constraints.min_uV);
> +	of_property_read_u32(np, "max-uV", &(*init_data)->constraints.max_uV);
> +	of_property_read_u32(np, "uV-offset", &(*init_data)->constraints.uV_offset);
> +	of_property_read_u32(np, "min-uA", &(*init_data)->constraints.min_uA);
> +	of_property_read_u32(np, "max-uA", &(*init_data)->constraints.max_uA);

Are all these structure members are int and unsigned int.  They need
to be u32 to be used with of_property_read_u32()  (which also tells
me that the of_property_read_*() api still needs work).

> +
> +	/* valid modes mask */
> +	if (of_find_property(np, "mode-fast", NULL))
> +		valid_modes_mask |= REGULATOR_MODE_FAST;
> +	if (of_find_property(np, "mode-normal", NULL))
> +		valid_modes_mask |= REGULATOR_MODE_NORMAL;
> +	if (of_find_property(np, "mode-idle", NULL))
> +		valid_modes_mask |= REGULATOR_MODE_IDLE;
> +	if (of_find_property(np, "mode-standby", NULL))
> +		valid_modes_mask |= REGULATOR_MODE_STANDBY;
> +
> +	/* valid ops mask */
> +	if (of_find_property(np, "change-voltage", NULL))
> +		valid_ops_mask |= REGULATOR_CHANGE_VOLTAGE;
> +	if (of_find_property(np, "change-current", NULL))
> +		valid_ops_mask |= REGULATOR_CHANGE_CURRENT;
> +	if (of_find_property(np, "change-mode", NULL))
> +		valid_ops_mask |= REGULATOR_CHANGE_MODE;
> +	if (of_find_property(np, "change-status", NULL))
> +		valid_ops_mask |= REGULATOR_CHANGE_STATUS;
> +
> +	(*init_data)->constraints.valid_modes_mask = valid_modes_mask;
> +	(*init_data)->constraints.valid_ops_mask = valid_ops_mask;
> +
> +	of_property_read_u32(np, "input-uV",
> +			&(*init_data)->constraints.input_uV);
> +	of_property_read_u32(np, "initial-mode",
> +			&(*init_data)->constraints.initial_mode);
> +
> +	if (of_find_property(np, "always-on", NULL))
> +			(*init_data)->constraints.always_on = true;
> +	if (of_find_property(np, "boot-on", NULL))
> +			(*init_data)->constraints.boot_on = true;
> +	if (of_find_property(np, "apply-uV", NULL))
> +			(*init_data)->constraints.apply_uV = true;
> +}
> +
> +/**
> + * of_get_regulator_init_data - extarct regulator_init_data structure info
> + * @np: Pointer to regulator device tree node
> + *
> + * Populates regulator_init_data structure by extracting data from device
> + * tree node, returns a pointer to the populated struture or NULL if memory
> + * alloc fails.
> + */
> +struct regulator_init_data *of_get_regulator_init_data(struct device_node *np)
> +{
> +	struct regulator_init_data *init_data;
> +
> +	init_data = kzalloc(sizeof(struct regulator_init_data), GFP_KERNEL);
> +	if (!init_data)
> +		return NULL; /* Out of memory? */
> +
> +	init_data->supply_regulator = (char *)of_get_property(np,
> +						"supply-regulator", NULL);
> +	of_get_regulation_constraints(np, &init_data);
> +	return init_data;
> +}
> +EXPORT_SYMBOL(of_get_regulator_init_data);

Who will call this?  If it is done by proper device drivers, it would
be better have it do the alloc so that it can do devm_kzalloc().  Or
maybe have a devm_kzalloc variant.

> diff --git a/include/linux/of_regulator.h b/include/linux/of_regulator.h
> new file mode 100644
> index 0000000..5eb048c
> --- /dev/null
> +++ b/include/linux/of_regulator.h
> @@ -0,0 +1,23 @@
> +/*
> + * OpenFirmware regulator support routines
> + *
> + */
> +
> +#ifndef __LINUX_OF_REG_H
> +#define __LINUX_OF_REG_H
> +
> +#include <linux/regulator/machine.h>
> +
> +#if defined(CONFIG_OF_REGULATOR)
> +extern struct regulator_init_data
> +	*of_get_regulator_init_data(struct device_node *np);
> +#else
> +static inline struct regulator_init_data
> +	*of_get_regulator_init_data(struct device_node *np)
> +{
> +	return NULL;
> +}
> +#endif /* CONFIG_OF_REGULATOR */
> +
> +#endif /* __LINUX_OF_REG_H */
> +
> -- 
> 1.7.1
> 

Re: [RFC PATCH 03/11] DT: regulator: Helper routine to extract regulator_init_data

[Rajendra Nayak]

On Friday 16 September 2011 03:42 AM, Grant Likely wrote:
> On Thu, Sep 15, 2011 at 04:51:59PM +0530, Rajendra Nayak wrote:
>> The helper routine is meant to be used by regulator drivers
>> to extract the regulator_init_data structure passed from device tree.
>> 'consumer_supplies' which is part of regulator_init_data is not extracted
>> as the regulator consumer mappings are passed through DT differently,
>> implemented in subsequent patches.
>>
>> Also add documentation for regulator bindings to be used to pass
>> regulator_init_data struct information from device tree.
>>
>> Signed-off-by: Rajendra Nayak<rnayak@ti.com>
>> ---
>>   .../devicetree/bindings/regulator/regulator.txt    |   37 +++++++++
>>   drivers/of/Kconfig                                 |    6 ++
>>   drivers/of/Makefile                                |    1 +
>>   drivers/of/of_regulator.c                          |   85 ++++++++++++++++++++
>
> I originally put the DT stuff under drivers/of for i2c and spi because
> there was resistance from driver subsystem maintainers to having code
> for something that was powerpc only.  Now that it is no longer the
> case, I recommend putting this code under drivers/regulator.

sure, will move these in drivers/regulator.

>
>>   include/linux/of_regulator.h                       |   23 +++++
>>   5 files changed, 152 insertions(+), 0 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/regulator/regulator.txt
>>   create mode 100644 drivers/of/of_regulator.c
>>   create mode 100644 include/linux/of_regulator.h
>>
>> diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
>> new file mode 100644
>> index 0000000..001e5ce
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/regulator/regulator.txt
>> @@ -0,0 +1,37 @@
>> +Voltage/Current Regulators
>> +
>> +Required properties:
>> +- compatible: Must be "regulator";
>
> A "regulator" compatible doesn't actually help much.  Compatible is
> for specifying the specific device, not for trying to describe what
> kind of device it is.  Instead, specific regulator bindings can adopt
> &  extend this binding.

yeah, will have a compatible for each specific device.

>
>> +
>> +Optional properties:
>> +- supply-regulator: Name of the parent regulator
>
> If this is a reference to another regulator node then don't use names.
> Use phandles instead.  In which case it should probably be named
> something like "regulator-parent" (similar to interrupt parent).
>
> However, I can imagine it possible for a regulator to have multiple
> parents.  It may just be better to go with something like the gpio
> scheme of<phandle gpio-specifier>  list of entries.  Or maybe just a
> list of phandles would be sufficient.

Ok, I can use phandles instead of the name, and have a list to specify
multiple parents.
The linux regulator framework though limits to just one parent I guess.

>
>> +- min-uV: smallest voltage consumers may set
>> +- max-uV: largest voltage consumers may set
>> +- uV-offset: Offset applied to voltages from consumer to compensate for voltage drops
>> +- min-uA: smallest current consumers may set
>> +- max-uA: largest current consumers may set
>> +- mode-fast: boolean, Can handle fast changes in its load
>> +- mode-normal: boolean, Normal regulator power supply mode
>> +- mode-idle: boolean, Can be more efficient during light loads
>> +- mode-standby: boolean, Can be most efficient during light loads
>> +- change-voltage: boolean, Output voltage can be changed by software
>> +- change-current: boolean, Output current can be chnaged by software
>> +- change-mode: boolean, Operating mode can be changed by software
>> +- change-status: boolean, Enable/Disable control for regulator exists
>> +- change-drms: boolean, Dynamic regulator mode switching is enabled
>> +- input-uV: Input voltage for regulator when supplied by another regulator
>> +- initial-mode: Mode to set at startup
>> +- always-on: boolean, regulator should never be disabled
>> +- boot-on: bootloader/firmware enabled regulator
>> +- apply-uV: apply uV constraint if min == max
>
> To avoid collisions, I'd prefix all of these with a common prefix.
> Something like regulator-*.

Ok.

>
> These map 1:1 to how Linux currently implements regulators; which
> isn't exactly bad, but it means that even if Linux changes, we're
> still have to support this binding.  Does this represent the best
> layout for high level description of regulators?

I guess, except for some like apply-uV, which like Mark pointed
out are very linux/runtime policy specific.
Mark, what do you think?

>
>> +
>> +Example:
>> +
>> +	xyz-regulator: regulator@0 {
>> +		compatible = "regulator";
>> +		min-uV =<1000000>;
>> +		max-uV =<2500000>;
>> +		mode-fast;
>> +		change-voltage;
>> +		always-on;
>> +	};
>> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
>> index b3bfea5..296b96d 100644
>> --- a/drivers/of/Kconfig
>> +++ b/drivers/of/Kconfig
>> @@ -87,4 +87,10 @@ config OF_PCI_IRQ
>>   	help
>>   	  OpenFirmware PCI IRQ routing helpers
>>
>> +config OF_REGULATOR
>> +	def_bool y
>> +	depends on REGULATOR
>> +	help
>> +	  OpenFirmware regulator framework helpers
>> +
>>   endmenu # OF
>> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
>> index 74b959c..bea2852 100644
>> --- a/drivers/of/Makefile
>> +++ b/drivers/of/Makefile
>> @@ -12,3 +12,4 @@ obj-$(CONFIG_OF_SPI)	+= of_spi.o
>>   obj-$(CONFIG_OF_MDIO)	+= of_mdio.o
>>   obj-$(CONFIG_OF_PCI)	+= of_pci.o
>>   obj-$(CONFIG_OF_PCI_IRQ)  += of_pci_irq.o
>> +obj-$(CONFIG_OF_REGULATOR) += of_regulator.o
>> diff --git a/drivers/of/of_regulator.c b/drivers/of/of_regulator.c
>> new file mode 100644
>> index 0000000..f01d275
>> --- /dev/null
>> +++ b/drivers/of/of_regulator.c
>> @@ -0,0 +1,85 @@
>> +/*
>> + * OF helpers for regulator framework
>> + *
>> + * Copyright (C) 2011 Texas Instruments, Inc.
>> + * Rajendra Nayak<rnayak@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#include<linux/slab.h>
>> +#include<linux/of.h>
>> +#include<linux/regulator/machine.h>
>> +
>> +static void of_get_regulation_constraints(struct device_node *np,
>> +					struct regulator_init_data **init_data)
>> +{
>> +	unsigned int valid_modes_mask = 0, valid_ops_mask = 0;
>> +
>> +	of_property_read_u32(np, "min-uV",&(*init_data)->constraints.min_uV);
>> +	of_property_read_u32(np, "max-uV",&(*init_data)->constraints.max_uV);
>> +	of_property_read_u32(np, "uV-offset",&(*init_data)->constraints.uV_offset);
>> +	of_property_read_u32(np, "min-uA",&(*init_data)->constraints.min_uA);
>> +	of_property_read_u32(np, "max-uA",&(*init_data)->constraints.max_uA);
>
> Are all these structure members are int and unsigned int.  They need
> to be u32 to be used with of_property_read_u32()  (which also tells
> me that the of_property_read_*() api still needs work).

right, will fix that.

>
>> +
>> +	/* valid modes mask */
>> +	if (of_find_property(np, "mode-fast", NULL))
>> +		valid_modes_mask |= REGULATOR_MODE_FAST;
>> +	if (of_find_property(np, "mode-normal", NULL))
>> +		valid_modes_mask |= REGULATOR_MODE_NORMAL;
>> +	if (of_find_property(np, "mode-idle", NULL))
>> +		valid_modes_mask |= REGULATOR_MODE_IDLE;
>> +	if (of_find_property(np, "mode-standby", NULL))
>> +		valid_modes_mask |= REGULATOR_MODE_STANDBY;
>> +
>> +	/* valid ops mask */
>> +	if (of_find_property(np, "change-voltage", NULL))
>> +		valid_ops_mask |= REGULATOR_CHANGE_VOLTAGE;
>> +	if (of_find_property(np, "change-current", NULL))
>> +		valid_ops_mask |= REGULATOR_CHANGE_CURRENT;
>> +	if (of_find_property(np, "change-mode", NULL))
>> +		valid_ops_mask |= REGULATOR_CHANGE_MODE;
>> +	if (of_find_property(np, "change-status", NULL))
>> +		valid_ops_mask |= REGULATOR_CHANGE_STATUS;
>> +
>> +	(*init_data)->constraints.valid_modes_mask = valid_modes_mask;
>> +	(*init_data)->constraints.valid_ops_mask = valid_ops_mask;
>> +
>> +	of_property_read_u32(np, "input-uV",
>> +			&(*init_data)->constraints.input_uV);
>> +	of_property_read_u32(np, "initial-mode",
>> +			&(*init_data)->constraints.initial_mode);
>> +
>> +	if (of_find_property(np, "always-on", NULL))
>> +			(*init_data)->constraints.always_on = true;
>> +	if (of_find_property(np, "boot-on", NULL))
>> +			(*init_data)->constraints.boot_on = true;
>> +	if (of_find_property(np, "apply-uV", NULL))
>> +			(*init_data)->constraints.apply_uV = true;
>> +}
>> +
>> +/**
>> + * of_get_regulator_init_data - extarct regulator_init_data structure info
>> + * @np: Pointer to regulator device tree node
>> + *
>> + * Populates regulator_init_data structure by extracting data from device
>> + * tree node, returns a pointer to the populated struture or NULL if memory
>> + * alloc fails.
>> + */
>> +struct regulator_init_data *of_get_regulator_init_data(struct device_node *np)
>> +{
>> +	struct regulator_init_data *init_data;
>> +
>> +	init_data = kzalloc(sizeof(struct regulator_init_data), GFP_KERNEL);
>> +	if (!init_data)
>> +		return NULL; /* Out of memory? */
>> +
>> +	init_data->supply_regulator = (char *)of_get_property(np,
>> +						"supply-regulator", NULL);
>> +	of_get_regulation_constraints(np,&init_data);
>> +	return init_data;
>> +}
>> +EXPORT_SYMBOL(of_get_regulator_init_data);
>
> Who will call this?  If it is done by proper device drivers, it would
> be better have it do the alloc so that it can do devm_kzalloc().  Or
> maybe have a devm_kzalloc variant.

Yes, its expected to be called always from proper device drivers. So I
could use devm_kzalloc() instead.

>
>> diff --git a/include/linux/of_regulator.h b/include/linux/of_regulator.h
>> new file mode 100644
>> index 0000000..5eb048c
>> --- /dev/null
>> +++ b/include/linux/of_regulator.h
>> @@ -0,0 +1,23 @@
>> +/*
>> + * OpenFirmware regulator support routines
>> + *
>> + */
>> +
>> +#ifndef __LINUX_OF_REG_H
>> +#define __LINUX_OF_REG_H
>> +
>> +#include<linux/regulator/machine.h>
>> +
>> +#if defined(CONFIG_OF_REGULATOR)
>> +extern struct regulator_init_data
>> +	*of_get_regulator_init_data(struct device_node *np);
>> +#else
>> +static inline struct regulator_init_data
>> +	*of_get_regulator_init_data(struct device_node *np)
>> +{
>> +	return NULL;
>> +}
>> +#endif /* CONFIG_OF_REGULATOR */
>> +
>> +#endif /* __LINUX_OF_REG_H */
>> +
>> --
>> 1.7.1
>>

Re: [RFC PATCH 03/11] DT: regulator: Helper routine to extract regulator_init_data

[Mark Brown]

On Fri, Sep 16, 2011 at 12:54:18PM +0530, Rajendra Nayak wrote:
> On Friday 16 September 2011 03:42 AM, Grant Likely wrote:

> >However, I can imagine it possible for a regulator to have multiple
> >parents.  It may just be better to go with something like the gpio
> >scheme of<phandle gpio-specifier>  list of entries.  Or maybe just a
> >list of phandles would be sufficient.

> Ok, I can use phandles instead of the name, and have a list to specify
> multiple parents.
> The linux regulator framework though limits to just one parent I guess.

I think if we've got multiple parents they should be listed as named
supplies rather than as parents since it's probably important which is
which.  In fact we probably want to list all parents as supplies since
that's what they are, they just have special handling in the code due to
the recursion.

> >These map 1:1 to how Linux currently implements regulators; which
> >isn't exactly bad, but it means that even if Linux changes, we're
> >still have to support this binding.  Does this represent the best
> >layout for high level description of regulators?

> I guess, except for some like apply-uV, which like Mark pointed
> out are very linux/runtime policy specific.
> Mark, what do you think?

Yes, overall this feels far too direct.

Re: [RFC PATCH 02/11] regulator: Fix error check in set_consumer_device_supply

[Mark Brown]

On Thu, Sep 15, 2011 at 04:51:58PM +0530, Rajendra Nayak wrote:
> The parameters to set_consumer_device_supply() can be considered
> invalid (and hence it could return -EINVAL) if nether consumer_dev nor
> consumer_dev_name are passed, not when *both* are passed.

> Signed-off-by: Rajendra Nayak <rnayak@ti.com>

No, passing both is a clear bug on the part of the caller - the two ways
of specifying the device are not supposed to be used together.

Re: [RFC PATCH 02/11] regulator: Fix error check in set_consumer_device_supply

[Rajendra Nayak]

On Thursday 15 September 2011 07:03 PM, Mark Brown wrote:
> On Thu, Sep 15, 2011 at 04:51:58PM +0530, Rajendra Nayak wrote:
>> The parameters to set_consumer_device_supply() can be considered
>> invalid (and hence it could return -EINVAL) if nether consumer_dev nor
>> consumer_dev_name are passed, not when *both* are passed.
>
>> Signed-off-by: Rajendra Nayak<rnayak@ti.com>
>
> No, passing both is a clear bug on the part of the caller - the two ways
> of specifying the device are not supposed to be used together.

Got it, I'll just drop this one.

Re: [RFC PATCH 01/11] OMAP: TWL: Clean up mode and ops mask passed from board files

[Mark Brown]

On Thu, Sep 15, 2011 at 04:51:57PM +0530, Rajendra Nayak wrote:
> The TWL driver seems to set the default .valid_modes_mask to
> (REGULATOR_MODE_NORMAL | REGULATOR_MODE_STANDBY) and .valid_ops_mask
> to (REGULATOR_CHANGE_VOLTAGE | REGULATOR_CHANGE_MODE | REGULATOR_CHANGE_STATUS)
> for all the registered regulators.
> 
> There is no need for all the board files to pass this information
> additionally, when the driver already sets them by default.

"the driver"?  This sounds very buggy...

Re: [RFC PATCH 01/11] OMAP: TWL: Clean up mode and ops mask passed from board files

[Rajendra Nayak]

On Thursday 15 September 2011 07:02 PM, Mark Brown wrote:
> On Thu, Sep 15, 2011 at 04:51:57PM +0530, Rajendra Nayak wrote:
>> The TWL driver seems to set the default .valid_modes_mask to
>> (REGULATOR_MODE_NORMAL | REGULATOR_MODE_STANDBY) and .valid_ops_mask
>> to (REGULATOR_CHANGE_VOLTAGE | REGULATOR_CHANGE_MODE | REGULATOR_CHANGE_STATUS)
>> for all the registered regulators.
>>
>> There is no need for all the board files to pass this information
>> additionally, when the driver already sets them by default.
>
> "the driver"?  This sounds very buggy...

Yeah, the driver currently sets some default based on what
the chip itself supports and completely ignores the board
inputs.
How about the driver using the defaults and letting the boards
override it (if it ever wants to)?
Currently none of the boards (using any of the twl
regulators) seem to override the defaults and that way a lot of
this duplication in board files could be avoided.

Re: [RFC PATCH 01/11] OMAP: TWL: Clean up mode and ops mask passed from board files

[Mark Brown]

On Fri, Sep 16, 2011 at 12:41:53PM +0530, Rajendra Nayak wrote:
> On Thursday 15 September 2011 07:02 PM, Mark Brown wrote:
> >On Thu, Sep 15, 2011 at 04:51:57PM +0530, Rajendra Nayak wrote:

> >>There is no need for all the board files to pass this information
> >>additionally, when the driver already sets them by default.

> >"the driver"?  This sounds very buggy...

> Yeah, the driver currently sets some default based on what
> the chip itself supports and completely ignores the board
> inputs.
> How about the driver using the defaults and letting the boards
> override it (if it ever wants to)?
> Currently none of the boards (using any of the twl
> regulators) seem to override the defaults and that way a lot of
> this duplication in board files could be avoided.

No, this is just plain buggy.  It's fine for OMAP to have some common
stuff that the board reference but if the drivers are doing this that's
bad.  The drivers and framework shouldn't touch the hardware unless the
board says it's OK.

Re: [RFC PATCH 01/11] OMAP: TWL: Clean up mode and ops mask passed from board files

[Rajendra Nayak]

On Friday 16 September 2011 02:27 PM, Mark Brown wrote:
> On Fri, Sep 16, 2011 at 12:41:53PM +0530, Rajendra Nayak wrote:
>> On Thursday 15 September 2011 07:02 PM, Mark Brown wrote:
>>> On Thu, Sep 15, 2011 at 04:51:57PM +0530, Rajendra Nayak wrote:
>
>>>> There is no need for all the board files to pass this information
>>>> additionally, when the driver already sets them by default.
>
>>> "the driver"?  This sounds very buggy...
>
>> Yeah, the driver currently sets some default based on what
>> the chip itself supports and completely ignores the board
>> inputs.
>> How about the driver using the defaults and letting the boards
>> override it (if it ever wants to)?
>> Currently none of the boards (using any of the twl
>> regulators) seem to override the defaults and that way a lot of
>> this duplication in board files could be avoided.
>
> No, this is just plain buggy.  It's fine for OMAP to have some common
> stuff that the board reference but if the drivers are doing this that's
> bad.  The drivers and framework shouldn't touch the hardware unless the
> board says it's OK.

fair enough, I'll fix up the driver then.