[PATCH 1/2] cleanup regulator supply definitions in mach-omap2 to use REGULATOR_SUPPLY

[PATCH 1/2] cleanup regulator supply definitions in mach-omap2 to use REGULATOR_SUPPLY

[Santosh Shilimkar]

On 6/6/2011 12:16 PM, green at linuxhacker.ru wrote:
> From: Oleg Drokin<green@linuxhacker.ru>
>
> CC: Mark Brown<broonie@opensource.wolfsonmicro.com>
> CC: Mike Rapoport<mike@compulab.co.il>
> CC: Nishant Kamat<nskamat@ti.com>
> CC: Steve Sakoman<steve@sakoman.com>
> CC: Felipe Balbi<balbi@ti.com>
> CC: Santosh Shilimkar<santosh.shilimkar@ti.com>
> Signed-off-by: Oleg Drokin<green@linuxhacker.ru>
> ---
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>	

[PATCH 1/2] cleanup regulator supply definitions in mach-omap2 to use REGULATOR_SUPPLY

[Felipe Balbi]

Hi,

On Mon, Jun 06, 2011 at 02:46:01AM -0400, green at linuxhacker.ru wrote:
> From: Oleg Drokin <green@linuxhacker.ru>
> 
> CC: Mark Brown <broonie@opensource.wolfsonmicro.com>
> CC: Mike Rapoport <mike@compulab.co.il>
> CC: Nishant Kamat <nskamat@ti.com>
> CC: Steve Sakoman <steve@sakoman.com>
> CC: Felipe Balbi <balbi@ti.com>
> CC: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Oleg Drokin <green@linuxhacker.ru>
> ---
>  arch/arm/mach-omap2/board-4430sdp.c          |   16 +++++-----------
>  arch/arm/mach-omap2/board-cm-t35.c           |   10 ++++------
>  arch/arm/mach-omap2/board-ldp.c              |    5 ++---
>  arch/arm/mach-omap2/board-omap3beagle.c      |   10 ++++------
>  arch/arm/mach-omap2/board-omap3evm.c         |   10 ++++------
>  arch/arm/mach-omap2/board-omap3logic.c       |    5 ++---
>  arch/arm/mach-omap2/board-omap3stalker.c     |   10 ++++------
>  arch/arm/mach-omap2/board-omap3touchbook.c   |   10 ++++------
>  arch/arm/mach-omap2/board-omap4panda.c       |   16 +++++-----------
>  arch/arm/mach-omap2/board-overo.c            |    5 ++---
>  arch/arm/mach-omap2/board-zoom-peripherals.c |   21 ++++++++-------------
>  11 files changed, 44 insertions(+), 74 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
> index 63de2d3..914885a 100644
> --- a/arch/arm/mach-omap2/board-4430sdp.c
> +++ b/arch/arm/mach-omap2/board-4430sdp.c
> @@ -332,17 +332,11 @@ static struct omap2_hsmmc_info mmc[] = {
>  	{}	/* Terminator */
>  };
>  
> -static struct regulator_consumer_supply sdp4430_vaux_supply[] = {
> -	{
> -		.supply = "vmmc",
> -		.dev_name = "omap_hsmmc.1",
> -	},
> -};
> +static struct regulator_consumer_supply sdp4430_vaux_supply =
> +	REGULATOR_SUPPLY("vmmc", "omap_hsmmc.1");

this should be an array, as it was before.

> +
>  static struct regulator_consumer_supply sdp4430_vmmc_supply[] = {
> -	{
> -		.supply = "vmmc",
> -		.dev_name = "omap_hsmmc.0",
> -	},
> +	REGULATOR_SUPPLY("vmmc", "omap_hsmmc.0"),
>  };
>  
>  static int omap4_twl6030_hsmmc_late_init(struct device *dev)
> @@ -400,7 +394,7 @@ static struct regulator_init_data sdp4430_vaux1 = {
>  					| REGULATOR_CHANGE_STATUS,
>  	},
>  	.num_consumer_supplies  = 1,

and this should use ARRAY_SIZE(),

> -	.consumer_supplies      = sdp4430_vaux_supply,
> +	.consumer_supplies      = &sdp4430_vaux_supply,
>  };
>  
>  static struct regulator_init_data sdp4430_vaux2 = {
> diff --git a/arch/arm/mach-omap2/board-cm-t35.c b/arch/arm/mach-omap2/board-cm-t35.c
> index 77456de..33855e4 100644
> --- a/arch/arm/mach-omap2/board-cm-t35.c
> +++ b/arch/arm/mach-omap2/board-cm-t35.c
> @@ -337,13 +337,11 @@ static void __init cm_t35_init_display(void)
>  	}
>  }
>  
> -static struct regulator_consumer_supply cm_t35_vmmc1_supply = {
> -	.supply			= "vmmc",
> -};
> +static struct regulator_consumer_supply cm_t35_vmmc1_supply =
> +	REGULATOR_SUPPLY("vmmc", "omap_hsmmc.0");

this should also be array. It should read as:

static struct regulator_consumer_supply cm_t35_vmmc1_supply[] =
	REGULATOR_SUPPLY("vmmc", "omap_hsmmc.0",
};

ditto to all others.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110606/89b3bba3/attachment.sig>

[PATCH 1/2] cleanup regulator supply definitions in mach-omap2 to use REGULATOR_SUPPLY

[Oleg Drokin]

Hello!
On Jun 6, 2011, at 5:21 AM, Felipe Balbi wrote:

>> -static struct regulator_consumer_supply sdp4430_vaux_supply[] = {
>> -	{
>> -		.supply = "vmmc",
>> -		.dev_name = "omap_hsmmc.1",
>> -	},
>> -};
>> +static struct regulator_consumer_supply sdp4430_vaux_supply =
>> +	REGULATOR_SUPPLY("vmmc", "omap_hsmmc.1");
> this should be an array, as it was before.

Only one is defined right now.
Whoever needs a second element can convert it to array, I think?
What;s the importance of having it as an array right now?

>> --- a/arch/arm/mach-omap2/board-cm-t35.c
>> +++ b/arch/arm/mach-omap2/board-cm-t35.c
>> @@ -337,13 +337,11 @@ static void __init cm_t35_init_display(void)
>> 	}
>> }
>> 
>> -static struct regulator_consumer_supply cm_t35_vmmc1_supply = {
>> -	.supply			= "vmmc",
>> -};
>> +static struct regulator_consumer_supply cm_t35_vmmc1_supply =
>> +	REGULATOR_SUPPLY("vmmc", "omap_hsmmc.0");
> 
> this should also be array. It should read as:
> 
> static struct regulator_consumer_supply cm_t35_vmmc1_supply[] =
> 	REGULATOR_SUPPLY("vmmc", "omap_hsmmc.0",
> };

Why this one should be an array? There is only one vmmc supply in
functionality-reduced implementation TWL used on this board.
(in fact this particular board files defines more supplies then there are in the TWL
and I planned to do another patch that would kill all the defined but not really
existing supplies next like vpll2 and such).

Bye,
    Oleg

[PATCH 1/2] cleanup regulator supply definitions in mach-omap2 to use REGULATOR_SUPPLY

[Felipe Balbi]

Hi,

On Mon, Jun 06, 2011 at 11:45:29AM -0400, Oleg Drokin wrote:
> >> -static struct regulator_consumer_supply sdp4430_vaux_supply[] = {
> >> -	{
> >> -		.supply = "vmmc",
> >> -		.dev_name = "omap_hsmmc.1",
> >> -	},
> >> -};
> >> +static struct regulator_consumer_supply sdp4430_vaux_supply =
> >> +	REGULATOR_SUPPLY("vmmc", "omap_hsmmc.1");
> > this should be an array, as it was before.
> 
> Only one is defined right now.
> Whoever needs a second element can convert it to array, I think?
> What;s the importance of having it as an array right now?

because later patches will be easier to review. Look below:

When I have this:

static struct regulator_consumer_supply sdp4430_vmmc_supply[] = {
	REGULATOR_SUPPLY("vmmc", "omap_hsmmc.0"),
};

static struct regulator_init_data sdp4430_vmmc = {
	.constraints = {
		.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(sdp4430_vmmc_supply),
	.consumer_supplies      = sdp4430_vmmc_supply,
};

and I want to add a new REGULATOR_SUPPLY() the diff is simple:

@@ -1,5 +1,6 @@
 static struct regulator_consumer_supply sdp4430_vmmc_supply[] = {
 	REGULATOR_SUPPLY("vmmc", "omap_hsmmc.0"),
+	REGULATOR_SUPPLY("vmmc2", "omap_hsmmc.0"),
 };
 
 static struct regulator_init_data sdp4430_vmmc = {

now when I have this:

static struct regulator_consumer_supply sdp4430_vmmc_supply =
	REGULATOR_SUPPLY("vmmc", "omap_hsmmc.0");

static struct regulator_init_data sdp4430_vmmc = {
	.constraints = {
		.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  = 1,
	.consumer_supplies      = &sdp4430_vmmc_supply,
};

and I want to add the same supply, the diff is less obvious:

@@ -1,5 +1,7 @@
-static struct regulator_consumer_supply sdp4430_vmmc_supply =
-	REGULATOR_SUPPLY("vmmc", "omap_hsmmc.0");
+static struct regulator_consumer_supply sdp4430_vmmc_supply[] = {
+	REGULATOR_SUPPLY("vmmc", "omap_hsmmc.0"),
+	REGULATOR_SUPPLY("vmmc2", "omap_hsmmc.0"),
+};
 
 static struct regulator_init_data sdp4430_vmmc = {
 	.constraints = {
@@ -12,6 +14,6 @@ static struct regulator_init_data sdp443
 					| REGULATOR_CHANGE_MODE
 					| REGULATOR_CHANGE_STATUS,
 	},
-	.num_consumer_supplies  = 1,
-	.consumer_supplies      = &sdp4430_vmmc_supply,
+	.num_consumer_supplies  = ARRAY_SIZE(sdp4430_vmmc_supply),
+	.consumer_supplies      = sdp4430_vmmc_supply,
 };

can you see now ?

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110606/0ff1e589/attachment.sig>

[PATCH 1/2] cleanup regulator supply definitions in mach-omap2 to use REGULATOR_SUPPLY

[Oleg Drokin]

Hello!

On Jun 6, 2011, at 1:13 PM, Felipe Balbi wrote:
> On Mon, Jun 06, 2011 at 11:45:29AM -0400, Oleg Drokin wrote:
>>>> -static struct regulator_consumer_supply sdp4430_vaux_supply[] = {
>>>> -	{
>>>> -		.supply = "vmmc",
>>>> -		.dev_name = "omap_hsmmc.1",
>>>> -	},
>>>> -};
>>>> +static struct regulator_consumer_supply sdp4430_vaux_supply =
>>>> +	REGULATOR_SUPPLY("vmmc", "omap_hsmmc.1");
>>> this should be an array, as it was before.
>> 
>> Only one is defined right now.
>> Whoever needs a second element can convert it to array, I think?
>> What;s the importance of having it as an array right now?
> because later patches will be easier to review. Look below:
...
> can you see now ?

Ok, I get the idea. Do you think it would be best to convert every supply
definition to an array then just in case?
I wonder what are the chances of additional regulators to appear at all
in many of these.
What about supplies that are not going to have additional elements
(like that cm-t35 board, because there simply are no more regulators in the twl)?

Bye,
    Oleg

[PATCH 1/2] cleanup regulator supply definitions in mach-omap2 to use REGULATOR_SUPPLY

[Felipe Balbi]

On Mon, Jun 06, 2011 at 01:18:41PM -0400, Oleg Drokin wrote:
> Hello!
> 
> On Jun 6, 2011, at 1:13 PM, Felipe Balbi wrote:
> > On Mon, Jun 06, 2011 at 11:45:29AM -0400, Oleg Drokin wrote:
> >>>> -static struct regulator_consumer_supply sdp4430_vaux_supply[] = {
> >>>> -	{
> >>>> -		.supply = "vmmc",
> >>>> -		.dev_name = "omap_hsmmc.1",
> >>>> -	},
> >>>> -};
> >>>> +static struct regulator_consumer_supply sdp4430_vaux_supply =
> >>>> +	REGULATOR_SUPPLY("vmmc", "omap_hsmmc.1");
> >>> this should be an array, as it was before.
> >> 
> >> Only one is defined right now.
> >> Whoever needs a second element can convert it to array, I think?
> >> What;s the importance of having it as an array right now?
> > because later patches will be easier to review. Look below:
> ...
> > can you see now ?
> 
> Ok, I get the idea. Do you think it would be best to convert every supply
> definition to an array then just in case?

I think that while you're doing this big conversion, why not ?

> I wonder what are the chances of additional regulators to appear at all
> in many of these.
> What about supplies that are not going to have additional elements
> (like that cm-t35 board, because there simply are no more regulators in the twl)?

Still, It's the same thing I was discussing with Alan Stern on another
thread, it comes down to uniformity and you never know when someone
might decide to hook up and always on supply to any node :-p

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110606/a30dde24/attachment.sig>

[PATCH 1/2] cleanup regulator supply definitions in mach-omap2 to use REGULATOR_SUPPLY

[Oleg Drokin]

Hello!

On Jun 6, 2011, at 1:28 PM, Felipe Balbi wrote:

>> Ok, I get the idea. Do you think it would be best to convert every supply
>> definition to an array then just in case?
> I think that while you're doing this big conversion, why not ?

Well, ok then. I'll redo this patch.

Bye,
    Oleg

[PATCH 2/2] Remove old-style supply.dev assignments common in hsmmc init

[Felipe Balbi]

On Mon, Jun 06, 2011 at 02:46:02AM -0400, green at linuxhacker.ru wrote:
> From: Oleg Drokin <green@linuxhacker.ru>
> 
> CC: Mark Brown <broonie@opensource.wolfsonmicro.com>
> CC: Mike Rapoport <mike@compulab.co.il>
> CC: Nishant Kamat <nskamat@ti.com>
> CC: Steve Sakoman <steve@sakoman.com>
> CC: Felipe Balbi <balbi@ti.com>
> Signed-off-by: Oleg Drokin <green@linuxhacker.ru>

Reviewed-by: Felipe Balbi <balbi@ti.com>

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110606/b4a6fd3b/attachment.sig>

[PATCH 2/2] Remove old-style supply.dev assignments common in hsmmc init

[Grazvydas Ignotas]

On Mon, Jun 6, 2011 at 9:46 AM,  <green@linuxhacker.ru> wrote:
> From: Oleg Drokin <green@linuxhacker.ru>
>
> CC: Mark Brown <broonie@opensource.wolfsonmicro.com>
> CC: Mike Rapoport <mike@compulab.co.il>
> CC: Nishant Kamat <nskamat@ti.com>
> CC: Steve Sakoman <steve@sakoman.com>
> CC: Felipe Balbi <balbi@ti.com>
> Signed-off-by: Oleg Drokin <green@linuxhacker.ru>
> ---
> ?arch/arm/mach-omap2/board-cm-t35.c ? ? ? ? ? | ? ?2 --
> ?arch/arm/mach-omap2/board-ldp.c ? ? ? ? ? ? ?| ? ?2 --
> ?arch/arm/mach-omap2/board-omap3beagle.c ? ? ?| ? ?4 ----
> ?arch/arm/mach-omap2/board-omap3evm.c ? ? ? ? | ? ?4 ----
> ?arch/arm/mach-omap2/board-omap3logic.c ? ? ? | ? ?2 --
> ?arch/arm/mach-omap2/board-omap3stalker.c ? ? | ? ?4 ----
> ?arch/arm/mach-omap2/board-omap3touchbook.c ? | ? ?4 ----
> ?arch/arm/mach-omap2/board-overo.c ? ? ? ? ? ?| ? ?2 --
> ?arch/arm/mach-omap2/board-zoom-peripherals.c | ? ?7 -------
> ?9 files changed, 0 insertions(+), 31 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-cm-t35.c b/arch/arm/mach-omap2/board-cm-t35.c
> index 33855e4..a38cd70 100644
> --- a/arch/arm/mach-omap2/board-cm-t35.c
> +++ b/arch/arm/mach-omap2/board-cm-t35.c
> @@ -480,8 +480,6 @@ static int cm_t35_twl_gpio_setup(struct device *dev, unsigned gpio,
> ? ? ? ?omap2_hsmmc_init(mmc);
>
> ? ? ? ?/* link regulators to MMC adapters */

Would be useful to remove this comment too.

> - ? ? ? cm_t35_vmmc1_supply.dev = mmc[0].dev;
> - ? ? ? cm_t35_vsim_supply.dev = mmc[0].dev;
>
> ? ? ? ?return 0;
> ?}