diff mbox

OMAP4: PANDA, SDP: Fix EHCI regulator supply

Message ID 1308926240-13888-1-git-send-email-jaswinder.singh@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Jassi Brar June 24, 2011, 2:37 p.m. UTC
VUSB is a fixed level line and hence have no set_voltage
callback in regulator ops, but has apply_uV set to true.
As a result it fails to register with the regulator core.
Remove setting apply_uV.

Also, assign name to VUSB supply, without which regulator core
fails to find it and assigns the default 'dummy' regulator to
the ehci-omap device.

Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
 arch/arm/mach-omap2/board-4430sdp.c    |    6 +++++-
 arch/arm/mach-omap2/board-omap4panda.c |    6 +++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

Comments

Jassi Brar June 24, 2011, 5:07 p.m. UTC | #1
[CC'ing Liam and Mark as well]

On 24 June 2011 20:07, Jassi Brar <jaswinder.singh@linaro.org> wrote:
> VUSB is a fixed level line and hence have no set_voltage
> callback in regulator ops, but has apply_uV set to true.
> As a result it fails to register with the regulator core.
> Remove setting apply_uV.
>
> Also, assign name to VUSB supply, without which regulator core
> fails to find it and assigns the default 'dummy' regulator to
> the ehci-omap device.
>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> ---
>  arch/arm/mach-omap2/board-4430sdp.c    |    6 +++++-
>  arch/arm/mach-omap2/board-omap4panda.c |    6 +++++-
>  2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
> index 63de2d3..1ec60be 100644
> --- a/arch/arm/mach-omap2/board-4430sdp.c
> +++ b/arch/arm/mach-omap2/board-4430sdp.c
> @@ -504,16 +504,20 @@ static struct regulator_init_data sdp4430_vdac = {
>        },
>  };
>
> +static struct regulator_consumer_supply sdp4430_vusb_supply =
> +       REGULATOR_SUPPLY("hsusb0", "ehci-omap.0");
> +
>  static struct regulator_init_data sdp4430_vusb = {
>        .constraints = {
>                .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,
>        },
> +       .num_consumer_supplies  = 1,
> +       .consumer_supplies      = &sdp4430_vusb_supply,
>  };
>
>  static struct regulator_init_data sdp4430_clk32kg = {
> diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c
> index d4f9879..7429f7e 100644
> --- a/arch/arm/mach-omap2/board-omap4panda.c
> +++ b/arch/arm/mach-omap2/board-omap4panda.c
> @@ -362,16 +362,20 @@ static struct regulator_init_data omap4_panda_vdac = {
>        },
>  };
>
> +static struct regulator_consumer_supply omap4_panda_vusb_supply =
> +       REGULATOR_SUPPLY("hsusb0", "ehci-omap.0");
> +
>  static struct regulator_init_data omap4_panda_vusb = {
>        .constraints = {
>                .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,
>        },
> +       .num_consumer_supplies  = 1,
> +       .consumer_supplies      = &omap4_panda_vusb_supply,
>  };
>
>  static struct regulator_init_data omap4_panda_clk32kg = {
> --
> 1.7.4.1
>
>
Felipe Balbi June 27, 2011, 8 a.m. UTC | #2
Hi,

On Fri, Jun 24, 2011 at 10:37:56PM +0530, Jaswinder Singh wrote:
> [CC'ing Liam and Mark as well]
> 
> On 24 June 2011 20:07, Jassi Brar <jaswinder.singh@linaro.org> wrote:
> > VUSB is a fixed level line and hence have no set_voltage
> > callback in regulator ops, but has apply_uV set to true.
> > As a result it fails to register with the regulator core.
> > Remove setting apply_uV.
> >
> > Also, assign name to VUSB supply, without which regulator core
> > fails to find it and assigns the default 'dummy' regulator to
> > the ehci-omap device.
> >
> > Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> > ---
> >  arch/arm/mach-omap2/board-4430sdp.c    |    6 +++++-
> >  arch/arm/mach-omap2/board-omap4panda.c |    6 +++++-
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
> > index 63de2d3..1ec60be 100644
> > --- a/arch/arm/mach-omap2/board-4430sdp.c
> > +++ b/arch/arm/mach-omap2/board-4430sdp.c
> > @@ -504,16 +504,20 @@ static struct regulator_init_data sdp4430_vdac = {
> >        },
> >  };
> >
> > +static struct regulator_consumer_supply sdp4430_vusb_supply =
> > +       REGULATOR_SUPPLY("hsusb0", "ehci-omap.0");

this should be an array.

> >  static struct regulator_init_data sdp4430_vusb = {
> >        .constraints = {
> >                .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,
> >        },
> > +       .num_consumer_supplies  = 1,

ARRAY_SIZE()

> > +       .consumer_supplies      = &sdp4430_vusb_supply,

drop the &.

ditto to other.
Jassi Brar June 27, 2011, 8:08 a.m. UTC | #3
Hi Felipe,

On 27 June 2011 13:30, Felipe Balbi <balbi@ti.com> wrote:

>> >
>> > +static struct regulator_consumer_supply sdp4430_vusb_supply =
>> > +       REGULATOR_SUPPLY("hsusb0", "ehci-omap.0");
>
> this should be an array.
Ok, I can make it an array of _one_ element.
Though I am not sure why is that a good thing, or are we to use another
possible VUSB supply on Panda/SDP boards ?  Please suggest so that
I can add that too.

>> >  static struct regulator_init_data sdp4430_vusb = {
>> >        .constraints = {
>> >                .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,
>> >        },
>> > +       .num_consumer_supplies  = 1,
>
> ARRAY_SIZE()
Yes, of course if we are to switch to single-element array.

Thanks,
Jassi
Felipe Balbi June 27, 2011, 8:35 a.m. UTC | #4
Hi,

On Mon, Jun 27, 2011 at 01:38:54PM +0530, Jaswinder Singh wrote:
> Hi Felipe,
> 
> On 27 June 2011 13:30, Felipe Balbi <balbi@ti.com> wrote:
> 
> >> >
> >> > +static struct regulator_consumer_supply sdp4430_vusb_supply =
> >> > +       REGULATOR_SUPPLY("hsusb0", "ehci-omap.0");
> >
> > this should be an array.
> Ok, I can make it an array of _one_ element.
> Though I am not sure why is that a good thing, or are we to use another
> possible VUSB supply on Panda/SDP boards ?  Please suggest so that
> I can add that too.

same comment I gave before to another patch:

it makes the diff a lot easier to understand should anyone modify this
later. It's also a matter of consistency.
Jassi Brar June 27, 2011, 10:05 a.m. UTC | #5
Hi Felipe,

On 27 June 2011 14:05, Felipe Balbi <balbi@ti.com> wrote:
>> >> > +static struct regulator_consumer_supply sdp4430_vusb_supply =
>> >> > +       REGULATOR_SUPPLY("hsusb0", "ehci-omap.0");
>> >
>> > this should be an array.
>> Ok, I can make it an array of _one_ element.
>> Though I am not sure why is that a good thing, or are we to use another
>> possible VUSB supply on Panda/SDP boards ?  Please suggest so that
>> I can add that too.
>
> same comment I gave before to another patch:
>
> it makes the diff a lot easier to understand should anyone modify this
> later. It's also a matter of consistency.
>
A quick grep showed otherwise though ...

In arch/arm/mach-omap2/
Total regulators defined                          =  71
Regulators with exactly 1 supply           =   58
Single element non-array definitions     =  46/58
Single element array definitions             =  12/58

Even if we consider 20% to be norm for consistency, I am not sure it's
a good one.

Still many samsung guys piously enclose magic value defines in parenthesis,
just to maintain 'consistency' !

And, I don't understand how does diff become any easier beyond 2
elements in the array.

Sorry for being bitchy, but I am unable to buy any reason other than
having more than
one element to use array.

-Jassi
Felipe Balbi June 27, 2011, 10:11 a.m. UTC | #6
Hi,

On Mon, Jun 27, 2011 at 03:35:41PM +0530, Jaswinder Singh wrote:
> On 27 June 2011 14:05, Felipe Balbi <balbi@ti.com> wrote:
> >> >> > +static struct regulator_consumer_supply sdp4430_vusb_supply =
> >> >> > +       REGULATOR_SUPPLY("hsusb0", "ehci-omap.0");
> >> >
> >> > this should be an array.
> >> Ok, I can make it an array of _one_ element.
> >> Though I am not sure why is that a good thing, or are we to use another
> >> possible VUSB supply on Panda/SDP boards ?  Please suggest so that
> >> I can add that too.
> >
> > same comment I gave before to another patch:
> >
> > it makes the diff a lot easier to understand should anyone modify this
> > later. It's also a matter of consistency.
> >
> A quick grep showed otherwise though ...
> 
> In arch/arm/mach-omap2/
> Total regulators defined                          =  71
> Regulators with exactly 1 supply           =   58
> Single element non-array definitions     =  46/58
> Single element array definitions             =  12/58
> 
> Even if we consider 20% to be norm for consistency, I am not sure it's
> a good one.

the patch which converted all non-array, to array seems to have been
taken yet, then.

> Still many samsung guys piously enclose magic value defines in parenthesis,
> just to maintain 'consistency' !

that's just plain stupidity.

> And, I don't understand how does diff become any easier beyond 2
> elements in the array.

http://marc.info/?l=linux-omap&m=130738044715490&w=2

> Sorry for being bitchy, but I am unable to buy any reason other than
> having more than
> one element to use array.

I also seem to recall someone (either Russell or Linus) once explained
why we should never mistake one-element arrays with pointers.
Unfortunately, I fail to find the thread, it's quite old.
Jassi Brar June 27, 2011, 11:10 a.m. UTC | #7
On 27 June 2011 15:41, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Mon, Jun 27, 2011 at 03:35:41PM +0530, Jaswinder Singh wrote:
>> On 27 June 2011 14:05, Felipe Balbi <balbi@ti.com> wrote:
>> >> >> > +static struct regulator_consumer_supply sdp4430_vusb_supply =
>> >> >> > +       REGULATOR_SUPPLY("hsusb0", "ehci-omap.0");
>> >> >
>> >> > this should be an array.
>> >> Ok, I can make it an array of _one_ element.
>> >> Though I am not sure why is that a good thing, or are we to use another
>> >> possible VUSB supply on Panda/SDP boards ?  Please suggest so that
>> >> I can add that too.
>> >
>> > same comment I gave before to another patch:
>> >
>> > it makes the diff a lot easier to understand should anyone modify this
>> > later. It's also a matter of consistency.
>> >
>> A quick grep showed otherwise though ...
>>
>> In arch/arm/mach-omap2/
>> Total regulators defined                          =  71
>> Regulators with exactly 1 supply           =   58
>> Single element non-array definitions     =  46/58
>> Single element array definitions             =  12/58
>>
>> Even if we consider 20% to be norm for consistency, I am not sure it's
>> a good one.
>
> the patch which converted all non-array, to array seems to have been
> taken yet, then.
>
Ok, I don't have that patch. If everything else has been converted
then there is no point in sticking out. Please let me know which repo has that.
I'll adapt to that.

>> And, I don't understand how does diff become any easier beyond 2
>> elements in the array.
>
> http://marc.info/?l=linux-omap&m=130738044715490&w=2
>
Yes, that's why I said "beyond 2 elements"
Only, if any, "difficulty" would be _first_ time when someone patches
to add second supply.
After that it'll just be same as you expect.

>> Sorry for being bitchy, but I am unable to buy any reason other than
>> having more than
>> one element to use array.
>
> I also seem to recall someone (either Russell or Linus) once explained
> why we should never mistake one-element arrays with pointers.
> Unfortunately, I fail to find the thread, it's quite old.
Yes, I vaguely remember the thread, but not sure if it's issue here.

-j
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
index 63de2d3..1ec60be 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -504,16 +504,20 @@  static struct regulator_init_data sdp4430_vdac = {
 	},
 };
 
+static struct regulator_consumer_supply sdp4430_vusb_supply =
+	REGULATOR_SUPPLY("hsusb0", "ehci-omap.0");
+
 static struct regulator_init_data sdp4430_vusb = {
 	.constraints = {
 		.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,
 	},
+	.num_consumer_supplies	= 1,
+	.consumer_supplies	= &sdp4430_vusb_supply,
 };
 
 static struct regulator_init_data sdp4430_clk32kg = {
diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c
index d4f9879..7429f7e 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -362,16 +362,20 @@  static struct regulator_init_data omap4_panda_vdac = {
 	},
 };
 
+static struct regulator_consumer_supply omap4_panda_vusb_supply =
+	REGULATOR_SUPPLY("hsusb0", "ehci-omap.0");
+
 static struct regulator_init_data omap4_panda_vusb = {
 	.constraints = {
 		.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,
 	},
+	.num_consumer_supplies	= 1,
+	.consumer_supplies	= &omap4_panda_vusb_supply,
 };
 
 static struct regulator_init_data omap4_panda_clk32kg = {