2009-06-15 11:24:32

by Liam Girdwood

[permalink] [raw]
Subject: [GIT PULL] voltage regulator updates for 2.6.31-rc1

Hi Linus,

Please pull the following voltage regulator updates for 2.6.31-rc1.

This update contains new drivers for the LP3971 and Maxim 1586 PMICs and
a userspace consumer driver allowing fine grained regulator control by
applications. Also noteworthy are list_voltage() support for fixed
regulators and subsys_initcall() for regulator drivers

Thanks

Liam

---

The following changes since commit 45e3e1935e2857c54783291107d33323b3ef33c8:
Linus Torvalds (1):
Merge branch 'master' of git://git.kernel.org/.../sam/kbuild-next

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/lrg/voltage-2.6.git for-linus

Greg Kroah-Hartman (1):
regulator: remove driver_data direct access of struct device

Liam Girdwood (2):
regulator: build fix for powerpc - renamed show_state
regulator: lp3971 - fix driver link error when built-in.

Marek Szyprowski (1):
LP3971 PMIC regulator driver (updated and combined version)

Mark Brown (3):
regulator: Move regulator drivers to subsys_initcall()
regulator: Support list_voltage for fixed voltage regulator
regulator: Set MODULE_ALIAS for regulator drivers

Mike Rapoport (1):
regulator: add userspace-consumer driver

Philipp Zabel (2):
regulator/max1586: support increased V3 voltage range
regulator/max1586: fix V3 gain calculation integer overflow

Robert Jarzmik (1):
Maxim 1586 regulator driver

drivers/regulator/Kconfig | 26 ++
drivers/regulator/Makefile | 3 +
drivers/regulator/da903x.c | 2 +-
drivers/regulator/fixed.c | 18 +-
drivers/regulator/lp3971.c | 562 ++++++++++++++++++++++++++
drivers/regulator/max1586.c | 282 +++++++++++++
drivers/regulator/pcf50633-regulator.c | 2 +-
drivers/regulator/userspace-consumer.c | 200 +++++++++
drivers/regulator/virtual.c | 1 +
drivers/regulator/wm8350-regulator.c | 1 +
drivers/regulator/wm8400-regulator.c | 8 +-
include/linux/regulator/lp3971.h | 51 +++
include/linux/regulator/max1586.h | 63 +++
include/linux/regulator/userspace-consumer.h | 25 ++
14 files changed, 1236 insertions(+), 8 deletions(-)
create mode 100644 drivers/regulator/lp3971.c
create mode 100644 drivers/regulator/max1586.c
create mode 100644 drivers/regulator/userspace-consumer.c
create mode 100644 include/linux/regulator/lp3971.h
create mode 100644 include/linux/regulator/max1586.h
create mode 100644 include/linux/regulator/userspace-consumer.h


2009-06-15 16:30:44

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] voltage regulator updates for 2.6.31-rc1



On Mon, 15 Jun 2009, Liam Girdwood wrote:
>
> drivers/regulator/lp3971.c | 562 ++++++++++++++++++++++++++
> include/linux/regulator/lp3971.h | 51 +++

So I really don't like this.

Why the heck would "lp3971.h" be under "include/linux"? That makes no
sense what-so-ever.

Why isn't it just

drivers/regulator/lp3971.c
drivers/regulator/lp3971.h

instead? There is never any reason for anything but the lp3971 driver to
include its own header file, they should _not_ be split up to be in two
different places.

In fact, why do we have a "include/linux/regulator/" at all? Does anybody
else ever care? I doubt it.

Anyway, I pulled it, but I think this needs to be fixed.

Linus

2009-06-15 17:17:29

by Mark Brown

[permalink] [raw]
Subject: Re: [GIT PULL] voltage regulator updates for 2.6.31-rc1

On Mon, Jun 15, 2009 at 09:29:54AM -0700, Linus Torvalds wrote:
> On Mon, 15 Jun 2009, Liam Girdwood wrote:

> > include/linux/regulator/lp3971.h | 51 +++

> Why the heck would "lp3971.h" be under "include/linux"? That makes no
> sense what-so-ever.

> Why isn't it just

> drivers/regulator/lp3971.c
> drivers/regulator/lp3971.h

It defines a platform data structure for the architecture code which
instantiates the driver to use to pass configuration to the driver. If
the header were in drivers/regulator then any system using one of these
regulators would need to peer into drivers/regulator in order to set up
the driver.

> instead? There is never any reason for anything but the lp3971 driver to
> include its own header file, they should _not_ be split up to be in two
> different places.

All the driver-internal definitions are included in the driver itself,
the regulator drivers have been good about not exposing this.

> In fact, why do we have a "include/linux/regulator/" at all? Does anybody
> else ever care? I doubt it.

At the minute the regulator API makes at least some platform data
mandatory for regulators (in order to tell the API what configuration
can safely be done on the regulator on a given board). This could be
changed but it seems to be working well at the minute and provides a
fairly simple and direct way of matching the configuration to the
correct regulator.

Even if the regulator API were reorganised to not require this a
noticable proportion of regulators require some tuning for things like
the passive components surrounding the regulator which would need
platform data.

2009-06-15 17:29:17

by Linus Torvalds

[permalink] [raw]
Subject: Re: [GIT PULL] voltage regulator updates for 2.6.31-rc1



On Mon, 15 Jun 2009, Mark Brown wrote:

> On Mon, Jun 15, 2009 at 09:29:54AM -0700, Linus Torvalds wrote:
> > On Mon, 15 Jun 2009, Liam Girdwood wrote:
>
> > > include/linux/regulator/lp3971.h | 51 +++
>
> > Why the heck would "lp3971.h" be under "include/linux"? That makes no
> > sense what-so-ever.
>
> > Why isn't it just
>
> > drivers/regulator/lp3971.c
> > drivers/regulator/lp3971.h
>
> It defines a platform data structure for the architecture code which
> instantiates the driver to use to pass configuration to the driver. If
> the header were in drivers/regulator then any system using one of these
> regulators would need to peer into drivers/regulator in order to set up
> the driver.

A quick grep shows:

[torvalds@nehalem linux]$ git grep lp3971.h
drivers/regulator/lp3971.c:#include <linux/regulator/lp3971.h>

ie _nobody_ includes lp3971.h except for lp3971.c.

So tell me again, why is that lp3971.h file separate from the only driver
that uses it?

Linus

2009-06-15 18:04:55

by Mark Brown

[permalink] [raw]
Subject: Re: [GIT PULL] voltage regulator updates for 2.6.31-rc1

On Mon, Jun 15, 2009 at 10:28:31AM -0700, Linus Torvalds wrote:
> On Mon, 15 Jun 2009, Mark Brown wrote:

> > It defines a platform data structure for the architecture code which
> > instantiates the driver to use to pass configuration to the driver. If

> A quick grep shows:

> [torvalds@nehalem linux]$ git grep lp3971.h
> drivers/regulator/lp3971.c:#include <linux/regulator/lp3971.h>

> ie _nobody_ includes lp3971.h except for lp3971.c.

> So tell me again, why is that lp3971.h file separate from the only driver
> that uses it?

Currently there are no in-tree users of the driver at all - being an I2C
device it requires explict instantiation. When users do get merged they
will need to provide this platform data so it seems to make sense to add
the ability to specify the platform data along with the driver. If we
omit the platform data for now then any users of the driver would need
to be submitted along with a patch adding the platform data (and would
need to carry that patch while out of tree).

We could also say we won't merge any drivers that don't have an in-tree
user but that goes against the general approach to driver merging and
increases the barrier to upstream submission for people working on
systems using regulators.

This situation where you've got drivers merged that are never actually
used in mainline isn't very unusual - sometimes people produce drivers
while working on systems that are never going to be mainlined, sometimes
it's simply a case of the driver support being ready for mainline before
the board code itself is in a mainlineable state. It's also not unique
to regulator drivers - one other example I just turned up is the tsc2007
touchscreen driver.