Re: [PATCH v3] mmc_spi.c: add support for the regulator framework

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 23 May 2011 17:10:23 +0200
Antonio Ospite <ospite@xxxxxxxxxxxxxxxxx> wrote:

> On Wed, 18 May 2011 12:42:12 -0700
> Mark Brown <broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> 
> > On Wed, May 18, 2011 at 07:23:20PM +0200, Antonio Ospite wrote:
> > 
> [...]
> > >   2. Add a proper function with all the needed parameters to abstract
> > >      the actual var names, this would be something like:
> > >      mmc_regulator_set_power(mmc, power_mode, vdd, vcc, pdata)
> > >      and yet checking for pdata->setpower can be tricky as 'setpower' 
> > >      is an arbitrary name.
> > 
> > This would be good.
> 
> A WIP patch, I don't have a lot of time to spend on this, I tried to
> handle the driver specific 'setpower' callback with a macro, let me
> know if you have a better idea:
> 
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index bcb793e..dbbe535 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -298,6 +298,32 @@ static inline int mmc_regulator_set_ocr(struct mmc_host *mmc,
>  }
>  #endif
>  
> +#define STRUCT_FIELD(s, f) ((s) && (s)->f ? (s)->f : NULL )
> +

Any opinion on this macro? See its use below. It is meant to deal with
driver specific struct fields, which can have arbitrary names, I though
that using some syntactic sugar to deal with those as arguments when
calling the function was not that horrible.

If that looks acceptable to you too I will submit the
mmc_regulator_set_power () patch, otherwise I would ask to consider the
simple patch to mmc_spi.c for now.

[...]
> diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
> index 2e8db20..c9a229f 100644
> --- a/drivers/mmc/host/mmc_spi.c
> +++ b/drivers/mmc/host/mmc_spi.c
> @@ -1231,21 +1231,18 @@ static inline int mmc_spi_setpower(struct mmc_spi_host *host,
>  					unsigned char power_mode,
>  					unsigned int vdd)
>  {
> +	int ret;
> +
>  	if (!mmc_spi_canpower(host))
>  		return -ENOSYS;
>  
> -	if (host->vcc) {
> -		int ret;
>  
> -		if (power_mode == MMC_POWER_OFF)
> -			vdd = 0;
> +	ret = mmc_regulator_set_power(host->mmc, power_mode, host->vcc, vdd,
> +				      &host->spi->dev,
> +				      STRUCT_FIELD(host->pdata, setpower));
                                      ^^^^^^^^^^^^^
> +	if (ret)
> +		return ret;
>  
> -		ret = mmc_regulator_set_ocr(host->mmc, host->vcc, vdd);
> -		if (ret)
> -			return ret;
> -	} else {
> -		host->pdata->setpower(&host->spi->dev, vdd);
> -	}
>  
>  	if (power_mode == MMC_POWER_UP)
>  		msleep(host->powerup_msecs);

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Attachment: pgpDCiCqEn8yJ.pgp
Description: PGP signature


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux