Em Qui, 2011-04-21 Ãs 14:49 +0200, Antonio Ospite escreveu:
> 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.
Hi,
The #ifdefs on pxamci were needed because at the time it was written
IS_ERR(regulator_get()) would return _false_ if CONFIG_REGULATOR is not
defined, breaking compatibility with the older ocr_mask/setpower.
I don't know if the regulator api changed it's old behaviour, see
http://www.mail-archive.com/openezx-devel@xxxxxxxxxxxxxxxxx/msg01954.html
--
Daniel Ribeiro
Attachment:
signature.asc
Description: Esta =?ISO-8859-1?Q?=E9?= uma parte de mensagem assinada digitalmente
