On Monday 02 March 2009, Pierre Ossman wrote:
> On Thu, 26 Feb 2009 13:48:30 -0800
> David Brownell <david-b@xxxxxxxxxxx> wrote:
>
> > From: David Brownell <dbrownell@xxxxxxxxxxxxxxxxxxxxx>
> >
> > Add optional glue between MMC and regulator stacks, using a new
> > regulator interface to learn what voltages are available.
> >
> > This is intended to be selected and driven by MMC host adapters.
> > It only handles reusable parts of the regulator-to-MMC glue; the
> > adapter drivers will have access to details that affect how this
> > is used. Examples include when to use multiple voltage rails or
> > configure (internal or external) level shifters.
> >
> > Signed-off-by: David Brownell <dbrownell@xxxxxxxxxxxxxxxxxxxxx>
> > ---
> > Changes from previous version: adapter must select this, and
> > callers now pass in the regulator. mmc_regulator_set_ocr()
> > is still not tested, mmc_regulator_get_ocrmask() passed sanity
> > testing.
> >
> > Pierre: Mark may have a need for this soonish. The omap_hsmmc
> > code will want it at some point.
> >
>
> I have no insight into the regulator stuff, so I'm going to have to
> trust you on this. :)
Works for me. ;)
> Some nitpicking though:
>
> > --- a/drivers/mmc/core/Kconfig
> > +++ b/drivers/mmc/core/Kconfig
> > @@ -14,3 +14,10 @@ config MMC_UNSAFE_RESUME
> > This option is usually just for embedded systems which use
> > a MMC/SD card for rootfs. Most people should say N here.
> >
> > +config MMC_REGULATOR
> > + bool
> > + depends on REGULATOR
> > + help
> > + Select this if your MMC host adapter driver wants helper
> > + utilities for accessing power rails.
> > +
>
> Is there a need for a special Kconfig for this? Can't we just build
> these two whenever REGULATOR is defined? Or always, provided the
> regulator API is present even when the code isn't.
The first patch had a "default y" there, nobody commented.
I'll simplify that, and use #ifdef CONFIG_REGULATOR instead.
> > +/**
> > + * mmc_regulator_set_ocr - set regulator to match host->ios voltage
> > + * @host: mmc host whose supply voltage will be changed
> > + * @supply: regulator to use
> > + *
> > + * MMC host drivers may use this to enable or disable a regulator using
> > + * a particular supply voltage. This would normally be called from the
> > + * set_ios() method.
> > + */
> > +int mmc_regulator_set_ocr(struct mmc_host *host, struct regulator *supply)
> > +{
>
> Why not pass the vdd directly? Saves a few dereferences if nothing else.
This call syntax is simpler, which is usually a win.
Passing a third parameter would create fault paths
of the "pass *wrong* parameter" flavor.
In terms of object code, when I've looked at such things
the dereferences generally cost the same as a ref to a
parameter, but passing an extra parameter isn't free.
- Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
