| Message ID | 1308926240-13888-1-git-send-email-jaswinder.singh@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
[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 > >
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.
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
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.
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
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.
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 --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 = {
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(-)