On Thu, 21 Apr 2011 13:40:41 +0100
Mark Brown <broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> On Thu, Apr 21, 2011 at 02:27:53PM +0200, Antonio Ospite wrote:
>
> > +#ifdef CONFIG_REGULATOR
> > + host->vcc = regulator_get(mmc_dev(host->mmc), "vmmc");
> > +
> > + if (IS_ERR(host->vcc)) {
> > + host->vcc = NULL;
> > + } else {
> > + host->mmc->ocr_avail = mmc_regulator_get_ocrmask(host->vcc);
> > + if (host->pdata && host->pdata->ocr_mask)
> > + dev_warn(mmc_dev(host->mmc),
> > + "ocr_mask/setpower will not be used\n");
> > }
> > +#endif
>
> Why is this code conditional? The regulator API will stub itself out
> (by returning a null pointer, which plays well with your use of null) if
> it's disabled. I'm also not seeing any corresponding code to release
> the regulator.
>
Actually I saw this done conditionally in pxamci.c and thought it was
really needed. About the call to regulator_put() if was there but got
lost when squashing the commits into the final patch and I was still
reading that part in my mind even if it was not there anymore.
The importance of reviewers with fresh eyes.
I'll resubmit this forth patch with the needed changes.
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:
pgpxLUMJnYVuz.pgp
Description: PGP signature
