Re: [PATCH 4/4] USB: TWL6025 allow different regulator name

2011-05-09 Thread Mark Brown
On Mon, May 09, 2011 at 04:51:26PM +0300, Felipe Balbi wrote:
> On Mon, May 09, 2011 at 03:35:43PM +0200, Mark Brown wrote:

> > -   twl->usb3v3 = regulator_get(twl->dev, "vusb");
> > +   twl->usb3v3 = regulator_get(twl->dev, regulator_name);
> > if (IS_ERR(twl->usb3v3))
> > return -ENODEV;

> then, imagine 5 years from now how that branch will look. How long do
> you think it'll take until someone decides to pass that name via
> platform_data to avoid growing that conditional ?

Probably way more than five years; frankly someone needs to yell at
whoever wrote the documentation and point out to them that VBUS is a
well known name that came from the spec.

In this particular case we actually shouldn't be worry about this at all
as it turns out that the consumer and supply are both internal to the
chip and hard wired together with no external users (the
regulator_init_data is part of the twl core not the board) so we
shouldn't be making this visible outside the drivers in the first place,
the name is totally irrelevant to anything except the twl drivers.  The
name is much more important if someone mapping out the regulators on a
board has to worry about it.

> > > We pass in the data of how regulators are wired at the board level and
> > > drivers would regulator_get(my_dev->dev, "whatever fancy name I want as
> > > it doesn't matter at all");

> > No, not at all.  Consumer drivers should be hard coding the strings they
> > request and the strings used should correspond to the pin names used on
> > the device.

> but that's the thing. Why do they need to depend on that string ?

They have to depend on *something* textual.  Even if you change it into
program code it's still going to have to be written down somewhere.

> > As Liam said this will just make the situation worse.  Users will have
> > to figure out what the names the driver authors assigned to the various
> > device supplies map onto in the physical system via an additional layer
> > of indirection which will at best be written down in comments in the
> > driver.

> My point is that the "name" shouldn't matter. Instead of matching a
> "name" match a "reference". Have something earlier in the boot assing
> regulators to devices, or something like that, so that drivers don't
> need to care about "names".

Unfortunately we write computer programs using text and that does rather
mean that things need names eventually, and that's going to have to
include the driver since whatever else does the mapping is going to need
to figure out what the driver is using to refer to a given regulator.

At some point these magic references are going to have to come from a
human in some form the human has a hope of comprehending.  The names
aren't really for the benefit of the driver, they're there so that a
human reading a schematic can work out which regulator to attach to
which supply and produce a mapping which other people can read and
understand.

> > Here you're apparently talking about something different - you're
> > talking about how we pass in the regulator init data for the regulators
> > supplied by the chip.  The patch being discussed changes the consumer
> > side for USB, not the regulator driver side.  The naming on the driver

> see above, how long do you think it'll take for someone to try and pass
> that name via pdata ? On the next name change, this is already likely to
> happen to prevent that conditional from growing.

People try to pass things as platform data all the time, usually as the
first thing they think of because they are having a hard time
distinguishing between the label the rail was given on the board and the
label the chip uses for the supply.  This isn't a problem, we just tell
them not to do that so that people can use the driver on other boards.

> > side is a particular problem for TWL devices since unlike most PMICs the
> > regulators aren't simply numbered but are instead assigned individual
> > names so you can't just use a big array indexed by number.

> why not ? The name shouldn't matter. In anycase, if you look into
> /sys/class/regulator/ all directories are regulator.:

>   dev_set_name(&rdev->dev, "regulator.%d",
>atomic_inc_return(®ulator_no) - 1);

That's not terribly useful for describing the relationships between
devices as the names come on a first come first served basis and are
therefore not at all stable.  We could try assiging base numbers to the
various regulator drivers but then you end up with a massive legibility
problem as nobody numbers the supplies on devices.

> > > What I was expecting to see, was a mechanism where we define how those
> shouldn't depend on strings at all in order to fetch the correct
> regulator. How many:

> if (xxx_features() & XXX_CLASS_IS_)
>   regulator_name = "my_first_name";
> else
>   regulator_name = "my_second_name";

> will have to pop in drivers until we figure that it won't scale ?

Note that the combination of _features a

Re: [PATCH 4/4] USB: TWL6025 allow different regulator name

2011-05-09 Thread Felipe Balbi
On Mon, May 09, 2011 at 03:35:43PM +0200, Mark Brown wrote:
> On Mon, May 09, 2011 at 04:07:07PM +0300, Felipe Balbi wrote:
> 
> > The thing is. We already had problems in the past with the Clock FW
> > because drivers were passing clock names on platform_data and I really
> > want to avoid the same on regulators. We need something clever.
> 
> Right, which is why you should *never* do that.  Any time I see anyone
> requesting a regulator based on anything other than a fixed string in
> the driver and the struct device for the consumer I push back very hard,
> and so should everyone else.

ok, so let's go back to the original patch:

> @@ -190,6 +190,12 @@ static int twl6030_phy_suspend(struct otg_transceiver 
> *x, int suspend)
>  
>  static int twl6030_usb_ldo_init(struct twl6030_usb *twl)
>  {
> + char *regulator_name;
> +
> + if (twl_features() & TWL6025_SUBCLASS)
> + regulator_name = "ldousb";
> + else
> + regulator_name = "vusb";
>  
>   /* Set to OTG_REV 1.3 and turn on the ID_WAKEUP_COMP */
>   twl6030_writeb(twl, TWL6030_MODULE_ID0 , 0x1, TWL6030_BACKUP_REG);
> @@ -200,7 +206,7 @@ static int twl6030_usb_ldo_init(struct twl6030_usb *twl)
>   /* Program MISC2 register and set bit VUSB_IN_VBAT */
>   twl6030_writeb(twl, TWL6030_MODULE_ID0 , 0x10, TWL6030_MISC2);
>  
> - twl->usb3v3 = regulator_get(twl->dev, "vusb");
> + twl->usb3v3 = regulator_get(twl->dev, regulator_name);
>   if (IS_ERR(twl->usb3v3))
>   return -ENODEV;

then, imagine 5 years from now how that branch will look. How long do
you think it'll take until someone decides to pass that name via
platform_data to avoid growing that conditional ?


> > We pass in the data of how regulators are wired at the board level and
> > drivers would regulator_get(my_dev->dev, "whatever fancy name I want as
> > it doesn't matter at all");
> 
> No, not at all.  Consumer drivers should be hard coding the strings they
> request and the strings used should correspond to the pin names used on
> the device.

but that's the thing. Why do they need to depend on that string ?

> > on the TRM. In other words, we should match on the functionality they
> > provide, not on the name.
> 
> As Liam said this will just make the situation worse.  Users will have
> to figure out what the names the driver authors assigned to the various
> device supplies map onto in the physical system via an additional layer
> of indirection which will at best be written down in comments in the
> driver.

My point is that the "name" shouldn't matter. Instead of matching a
"name" match a "reference". Have something earlier in the boot assing
regulators to devices, or something like that, so that drivers don't
need to care about "names".

> > Let's try to thing some 5 - 6 years ahead. With the current trend, we
> > will be working on support for the PMIC on OMAP10, imagine if each one
> > of those revisions decide to change the name of the regulator, because
> > this new HW engineer working on the PMIC design doesn't like the old
> > name. We will have X regulator pointers and X regulator name pointers in
> > our platform_data. It will be really cumbersome and prone to errors if
> > people continue on copy&pasting old code to "generate" a new driver.
> 
> Here you're apparently talking about something different - you're
> talking about how we pass in the regulator init data for the regulators
> supplied by the chip.  The patch being discussed changes the consumer
> side for USB, not the regulator driver side.  The naming on the driver

see above, how long do you think it'll take for someone to try and pass
that name via pdata ? On the next name change, this is already likely to
happen to prevent that conditional from growing.

> side is a particular problem for TWL devices since unlike most PMICs the
> regulators aren't simply numbered but are instead assigned individual
> names so you can't just use a big array indexed by number.

why not ? The name shouldn't matter. In anycase, if you look into
/sys/class/regulator/ all directories are regulator.:

dev_set_name(&rdev->dev, "regulator.%d",
 atomic_inc_return(®ulator_no) - 1);

> > What I was expecting to see, was a mechanism where we define how those
> > things are wired (it doesn't matter if the data uses DeviceTree or what)
> > at the HW level and we ask the framework to give us the regulator
> > connected to device X which provides a certain functionality. Just like
> > clkdev has managed to get close to. Currently drivers will:
> 
> This is preceisely what is happening.  A well written consumer driver
> asks the regulator core for the regulator supplying a given supply, with
> the supplies named in the same way as they are named in the datasheet.
> 
> You appear to be arging strongly for us to adopt the model which is
> already in use.

I guess I hasn't been clear enough then, my whole point is that drivers
shouldn't depend on string

Re: [PATCH 4/4] USB: TWL6025 allow different regulator name

2011-05-09 Thread Mark Brown
On Mon, May 09, 2011 at 04:07:07PM +0300, Felipe Balbi wrote:

> The thing is. We already had problems in the past with the Clock FW
> because drivers were passing clock names on platform_data and I really
> want to avoid the same on regulators. We need something clever.

Right, which is why you should *never* do that.  Any time I see anyone
requesting a regulator based on anything other than a fixed string in
the driver and the struct device for the consumer I push back very hard,
and so should everyone else.

> We pass in the data of how regulators are wired at the board level and
> drivers would regulator_get(my_dev->dev, "whatever fancy name I want as
> it doesn't matter at all");

No, not at all.  Consumer drivers should be hard coding the strings they
request and the strings used should correspond to the pin names used on
the device.

> If a driver has >1 regulator, you can distinguish them by which
> functionality they provide, no matter the name, they will be connected
> to a particular ball on the IC package which has a certain definition

Having more than one supply is the common case for devices; relatively
few devices have a single supply.

> on the TRM. In other words, we should match on the functionality they
> provide, not on the name.

As Liam said this will just make the situation worse.  Users will have
to figure out what the names the driver authors assigned to the various
device supplies map onto in the physical system via an additional layer
of indirection which will at best be written down in comments in the
driver.

> Let's try to thing some 5 - 6 years ahead. With the current trend, we
> will be working on support for the PMIC on OMAP10, imagine if each one
> of those revisions decide to change the name of the regulator, because
> this new HW engineer working on the PMIC design doesn't like the old
> name. We will have X regulator pointers and X regulator name pointers in
> our platform_data. It will be really cumbersome and prone to errors if
> people continue on copy&pasting old code to "generate" a new driver.

Here you're apparently talking about something different - you're
talking about how we pass in the regulator init data for the regulators
supplied by the chip.  The patch being discussed changes the consumer
side for USB, not the regulator driver side.  The naming on the driver
side is a particular problem for TWL devices since unlike most PMICs the
regulators aren't simply numbered but are instead assigned individual
names so you can't just use a big array indexed by number.

> What I was expecting to see, was a mechanism where we define how those
> things are wired (it doesn't matter if the data uses DeviceTree or what)
> at the HW level and we ask the framework to give us the regulator
> connected to device X which provides a certain functionality. Just like
> clkdev has managed to get close to. Currently drivers will:

This is preceisely what is happening.  A well written consumer driver
asks the regulator core for the regulator supplying a given supply, with
the supplies named in the same way as they are named in the datasheet.

You appear to be arging strongly for us to adopt the model which is
already in use.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] USB: TWL6025 allow different regulator name

2011-05-09 Thread Liam Girdwood
On Mon, 2011-05-09 at 15:16 +0300, Felipe Balbi wrote:
> Hi,
> 
> On Mon, May 09, 2011 at 12:43:49PM +0100, Liam Girdwood wrote:
> > On Mon, 2011-05-09 at 12:03 +0300, Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Sun, May 08, 2011 at 04:08:37PM +0100, Liam Girdwood wrote:
> > > > On Wed, 2011-04-27 at 13:45 +0300, Felipe Balbi wrote:
> > > > > Hi,
> > > > > 
> > > > > On Wed, Apr 27, 2011 at 10:39:51AM +0100, Graeme Gregory wrote:
> > > > > > The twl6025 uses a different regulator for USB than the 6030 so 
> > > > > > select
> > > > > > the correct regulator name depending on the subclass of device.
> > > > > > 
> > > > > > Signed-off-by: Graeme Gregory 
> > > > > 
> > > > > I don't see the point of this patch. It's just a string. Use the same
> > > > > name and add a comment saying that on datasheet/TRM/documentation the
> > > > > name LDO is actually referred to as LDOUSB. It's the same 
> > > > > functionality
> > > > > anyway.
> > > > > 
> > > > 
> > > > I think for the avoidance of any doubt, it's probably best to use the
> > > > TWL6025 string name here as it will importantly match the TWL6025 TRM
> > > > and any schematics using the TWL6025. Getting this wrong during TWL6025
> > > > board integration has the potential for hardware damage.
> > > 
> > > I would rather have something that doesn't depend on a correct string
> > > and matches based on the device pointer instead. I agree that having the
> > > correct string makes it easier to reference schematics/trm and the like,
> > > but making the SW depend on the correct spelling of a simple string, is
> > > too much for me :-(
> > > 
> > > Specially when getting it wrong "has the potential for hardware damage"
> > > :-)
> > > 
> > 
> > I think it's the lesser evil though, especially for device integrators.
> > They will just match the regulator name from the schematics together
> > with the TRM name when creating their regulator constraints and having
> > different names here will definitely cause confusion.
> 
> Any chance Device Tree could be used to pass that data to kernel
> instead, together with regulator names and all needed data for each one
> of them ?
> 

Yes, I think this should be the long term aim, but atm we will have to
stick with current implementation until regulator has device tree
support.

Liam

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] USB: TWL6025 allow different regulator name

2011-05-09 Thread Felipe Balbi
Hi,

On Mon, May 09, 2011 at 02:29:41PM +0200, Mark Brown wrote:
> On Mon, May 09, 2011 at 03:16:03PM +0300, Felipe Balbi wrote:
> > On Mon, May 09, 2011 at 12:43:49PM +0100, Liam Girdwood wrote:
> 
> > > I think it's the lesser evil though, especially for device integrators.
> > > They will just match the regulator name from the schematics together
> > > with the TRM name when creating their regulator constraints and having
> > > different names here will definitely cause confusion.
> 
> > Any chance Device Tree could be used to pass that data to kernel
> > instead, together with regulator names and all needed data for each one
> > of them ?
> 
> I'm pretty sure that as soon as we have viable device tree support for
> relevant platforms in mainline we'll have regulator support, though I'm
> not sure that this will help too much with the naming as you'll still
> have to figure out what the consumer requests.  We shouldn't be passing
> in the consumer supply names from device tree (at least not a board
> specific one) as this breaks the model that the supply names correspond
> to the chip pins.

The thing is. We already had problems in the past with the Clock FW
because drivers were passing clock names on platform_data and I really
want to avoid the same on regulators. We need something clever.

We pass in the data of how regulators are wired at the board level and
drivers would regulator_get(my_dev->dev, "whatever fancy name I want as
it doesn't matter at all");

If a driver has >1 regulator, you can distinguish them by which
functionality they provide, no matter the name, they will be connected
to a particular ball on the IC package which has a certain definition
on the TRM. In other words, we should match on the functionality they
provide, not on the name.

Let's try to thing some 5 - 6 years ahead. With the current trend, we
will be working on support for the PMIC on OMAP10, imagine if each one
of those revisions decide to change the name of the regulator, because
this new HW engineer working on the PMIC design doesn't like the old
name. We will have X regulator pointers and X regulator name pointers in
our platform_data. It will be really cumbersome and prone to errors if
people continue on copy&pasting old code to "generate" a new driver.

What I was expecting to see, was a mechanism where we define how those
things are wired (it doesn't matter if the data uses DeviceTree or what)
at the HW level and we ask the framework to give us the regulator
connected to device X which provides a certain functionality. Just like
clkdev has managed to get close to. Currently drivers will:

clk_get(dev, "ick");
clk_get(dev, "fck");

etc...

and the name really doesn't matter. Clkdev still isn't perfect as it
uses device names, then when we want/need to change the device/driver
name (due to some re-factoring for example) we end up breaking things.
IMHO, struct device should point to its dependencies, be it a clock, be
it a regulator.

-- 
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] USB: TWL6025 allow different regulator name

2011-05-09 Thread Mark Brown
On Mon, May 09, 2011 at 03:16:03PM +0300, Felipe Balbi wrote:
> On Mon, May 09, 2011 at 12:43:49PM +0100, Liam Girdwood wrote:

> > I think it's the lesser evil though, especially for device integrators.
> > They will just match the regulator name from the schematics together
> > with the TRM name when creating their regulator constraints and having
> > different names here will definitely cause confusion.

> Any chance Device Tree could be used to pass that data to kernel
> instead, together with regulator names and all needed data for each one
> of them ?

I'm pretty sure that as soon as we have viable device tree support for
relevant platforms in mainline we'll have regulator support, though I'm
not sure that this will help too much with the naming as you'll still
have to figure out what the consumer requests.  We shouldn't be passing
in the consumer supply names from device tree (at least not a board
specific one) as this breaks the model that the supply names correspond
to the chip pins.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] USB: TWL6025 allow different regulator name

2011-05-09 Thread Felipe Balbi
Hi,

On Mon, May 09, 2011 at 12:43:49PM +0100, Liam Girdwood wrote:
> On Mon, 2011-05-09 at 12:03 +0300, Felipe Balbi wrote:
> > Hi,
> > 
> > On Sun, May 08, 2011 at 04:08:37PM +0100, Liam Girdwood wrote:
> > > On Wed, 2011-04-27 at 13:45 +0300, Felipe Balbi wrote:
> > > > Hi,
> > > > 
> > > > On Wed, Apr 27, 2011 at 10:39:51AM +0100, Graeme Gregory wrote:
> > > > > The twl6025 uses a different regulator for USB than the 6030 so select
> > > > > the correct regulator name depending on the subclass of device.
> > > > > 
> > > > > Signed-off-by: Graeme Gregory 
> > > > 
> > > > I don't see the point of this patch. It's just a string. Use the same
> > > > name and add a comment saying that on datasheet/TRM/documentation the
> > > > name LDO is actually referred to as LDOUSB. It's the same functionality
> > > > anyway.
> > > > 
> > > 
> > > I think for the avoidance of any doubt, it's probably best to use the
> > > TWL6025 string name here as it will importantly match the TWL6025 TRM
> > > and any schematics using the TWL6025. Getting this wrong during TWL6025
> > > board integration has the potential for hardware damage.
> > 
> > I would rather have something that doesn't depend on a correct string
> > and matches based on the device pointer instead. I agree that having the
> > correct string makes it easier to reference schematics/trm and the like,
> > but making the SW depend on the correct spelling of a simple string, is
> > too much for me :-(
> > 
> > Specially when getting it wrong "has the potential for hardware damage"
> > :-)
> > 
> 
> I think it's the lesser evil though, especially for device integrators.
> They will just match the regulator name from the schematics together
> with the TRM name when creating their regulator constraints and having
> different names here will definitely cause confusion.

Any chance Device Tree could be used to pass that data to kernel
instead, together with regulator names and all needed data for each one
of them ?

-- 
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] USB: TWL6025 allow different regulator name

2011-05-09 Thread Liam Girdwood
On Mon, 2011-05-09 at 12:03 +0300, Felipe Balbi wrote:
> Hi,
> 
> On Sun, May 08, 2011 at 04:08:37PM +0100, Liam Girdwood wrote:
> > On Wed, 2011-04-27 at 13:45 +0300, Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Wed, Apr 27, 2011 at 10:39:51AM +0100, Graeme Gregory wrote:
> > > > The twl6025 uses a different regulator for USB than the 6030 so select
> > > > the correct regulator name depending on the subclass of device.
> > > > 
> > > > Signed-off-by: Graeme Gregory 
> > > 
> > > I don't see the point of this patch. It's just a string. Use the same
> > > name and add a comment saying that on datasheet/TRM/documentation the
> > > name LDO is actually referred to as LDOUSB. It's the same functionality
> > > anyway.
> > > 
> > 
> > I think for the avoidance of any doubt, it's probably best to use the
> > TWL6025 string name here as it will importantly match the TWL6025 TRM
> > and any schematics using the TWL6025. Getting this wrong during TWL6025
> > board integration has the potential for hardware damage.
> 
> I would rather have something that doesn't depend on a correct string
> and matches based on the device pointer instead. I agree that having the
> correct string makes it easier to reference schematics/trm and the like,
> but making the SW depend on the correct spelling of a simple string, is
> too much for me :-(
> 
> Specially when getting it wrong "has the potential for hardware damage"
> :-)
> 

I think it's the lesser evil though, especially for device integrators.
They will just match the regulator name from the schematics together
with the TRM name when creating their regulator constraints and having
different names here will definitely cause confusion.

Liam

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] USB: TWL6025 allow different regulator name

2011-05-09 Thread Felipe Balbi
Hi,

On Sun, May 08, 2011 at 04:08:37PM +0100, Liam Girdwood wrote:
> On Wed, 2011-04-27 at 13:45 +0300, Felipe Balbi wrote:
> > Hi,
> > 
> > On Wed, Apr 27, 2011 at 10:39:51AM +0100, Graeme Gregory wrote:
> > > The twl6025 uses a different regulator for USB than the 6030 so select
> > > the correct regulator name depending on the subclass of device.
> > > 
> > > Signed-off-by: Graeme Gregory 
> > 
> > I don't see the point of this patch. It's just a string. Use the same
> > name and add a comment saying that on datasheet/TRM/documentation the
> > name LDO is actually referred to as LDOUSB. It's the same functionality
> > anyway.
> > 
> 
> I think for the avoidance of any doubt, it's probably best to use the
> TWL6025 string name here as it will importantly match the TWL6025 TRM
> and any schematics using the TWL6025. Getting this wrong during TWL6025
> board integration has the potential for hardware damage.

I would rather have something that doesn't depend on a correct string
and matches based on the device pointer instead. I agree that having the
correct string makes it easier to reference schematics/trm and the like,
but making the SW depend on the correct spelling of a simple string, is
too much for me :-(

Specially when getting it wrong "has the potential for hardware damage"
:-)

-- 
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] USB: TWL6025 allow different regulator name

2011-05-08 Thread Liam Girdwood
On Wed, 2011-04-27 at 13:45 +0300, Felipe Balbi wrote:
> Hi,
> 
> On Wed, Apr 27, 2011 at 10:39:51AM +0100, Graeme Gregory wrote:
> > The twl6025 uses a different regulator for USB than the 6030 so select
> > the correct regulator name depending on the subclass of device.
> > 
> > Signed-off-by: Graeme Gregory 
> 
> I don't see the point of this patch. It's just a string. Use the same
> name and add a comment saying that on datasheet/TRM/documentation the
> name LDO is actually referred to as LDOUSB. It's the same functionality
> anyway.
> 

I think for the avoidance of any doubt, it's probably best to use the
TWL6025 string name here as it will importantly match the TWL6025 TRM
and any schematics using the TWL6025. Getting this wrong during TWL6025
board integration has the potential for hardware damage.

Liam

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] USB: TWL6025 allow different regulator name

2011-04-27 Thread Felipe Balbi
Hi,

On Wed, Apr 27, 2011 at 10:39:51AM +0100, Graeme Gregory wrote:
> The twl6025 uses a different regulator for USB than the 6030 so select
> the correct regulator name depending on the subclass of device.
> 
> Signed-off-by: Graeme Gregory 

I don't see the point of this patch. It's just a string. Use the same
name and add a comment saying that on datasheet/TRM/documentation the
name LDO is actually referred to as LDOUSB. It's the same functionality
anyway.

-- 
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/4] USB: TWL6025 allow different regulator name

2011-04-27 Thread Graeme Gregory
The twl6025 uses a different regulator for USB than the 6030 so select
the correct regulator name depending on the subclass of device.

Signed-off-by: Graeme Gregory 
---
 drivers/usb/otg/twl6030-usb.c |8 +++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/otg/twl6030-usb.c b/drivers/usb/otg/twl6030-usb.c
index 6e920de..3d82419 100644
--- a/drivers/usb/otg/twl6030-usb.c
+++ b/drivers/usb/otg/twl6030-usb.c
@@ -190,6 +190,12 @@ static int twl6030_phy_suspend(struct otg_transceiver *x, 
int suspend)
 
 static int twl6030_usb_ldo_init(struct twl6030_usb *twl)
 {
+   char *regulator_name;
+
+   if (twl_features() & TWL6025_SUBCLASS)
+   regulator_name = "ldousb";
+   else
+   regulator_name = "vusb";
 
/* Set to OTG_REV 1.3 and turn on the ID_WAKEUP_COMP */
twl6030_writeb(twl, TWL6030_MODULE_ID0 , 0x1, TWL6030_BACKUP_REG);
@@ -200,7 +206,7 @@ static int twl6030_usb_ldo_init(struct twl6030_usb *twl)
/* Program MISC2 register and set bit VUSB_IN_VBAT */
twl6030_writeb(twl, TWL6030_MODULE_ID0 , 0x10, TWL6030_MISC2);
 
-   twl->usb3v3 = regulator_get(twl->dev, "vusb");
+   twl->usb3v3 = regulator_get(twl->dev, regulator_name);
if (IS_ERR(twl->usb3v3))
return -ENODEV;
 
-- 
1.7.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html