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

[Antonio Ospite]

On Wed, 11 May 2011 15:08:53 +0200
Mark Brown <broo...@opensource.wolfsonmicro.com> wrote:

[...]
> > +   switch (power_mode) {
> > +   case MMC_POWER_OFF:
> > +           if (host->vcc) {
> > +                   int ret = mmc_regulator_set_ocr(host->mmc,
> > +                                                   host->vcc, 0);
> > +                   if (ret)
> > +                           return ret;
> > +           } else {
> > +                   host->pdata->setpower(&host->spi->dev, vdd);
> > +           }
> > +           break;
> > +
> > +   case MMC_POWER_UP:
> > +           if (host->vcc) {
> > +                   int 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);
> >             }
> > +           msleep(host->powerup_msecs);
> > +           break;
> 
> This stuff all looks like it should be factored out.
> 

OK, avoiding some duplication will be good, I agree.

I am resending a v4 with the equivalent code:

        if (host->vcc) {
                int ret;

                if (power_mode == MMC_POWER_OFF)
                        vdd = 0;

                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?

pgpwfG7EtLNd9.pgp
Description: PGP signature