Re: [patch 2.6.29-rc6+misc] MMC: regulator utilities

[Pierre Ossman <drzeus-mmc@drzeus.cx>]

On Thu, 26 Feb 2009 13:48:30 -0800
David Brownell <david-b@pacbell.net> wrote:

> From: David Brownell <dbrownell@users.sourceforge.net>
> 
> 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@users.sourceforge.net>
> ---
> 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. :)

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.

> +/**
> + * 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.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  rdesktop, core developer          http://www.rdesktop.org

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.