On Thu, 3 May 2012, Mark Brown wrote: > On Thu, May 03, 2012 at 05:05:36PM +0200, Guennadi Liakhovetski wrote: > > > + ret = mmc_regulator_get_ocrmask(supply); > > + if (ret > 0) > > + mmc->ocr_avail = ret; > > + > > + if (regulator_can_change_status(supply)) { > > + mmc->caps |= MMC_CAP_POWER_OFF_CARD; > > + return supply; > > + } > > + > > + devm_regulator_put(supply); > > + > > + if (ret <= 0) > > + dev_warn(dev, "Ignoring useless (dummy?) regulator\n"); > > This code seems very tricky and a bit confusing - you're checking that > either you can change the voltage or change the status but you're doing > it in a fairly abstruse way and it'll come out as saying that a fixed > voltage regulator is a useful regulator to have which probably isn't what > you want if it's worth ignoring a dummy regulator. This function handles 3 cases: 1. A regulator that can (in principle) change state, i.e., switch on and off. Such a regulator is good to keep for the runtime to power up and down the card. 2. A regulator, that cannot switch, but at least can tell its supplied voltage is used ones to read out supported voltages and released again. 3. A regulator that can do none of the above is defined as "useless" So, a fixed voltage regulator is indeed a useful one - it can tell us the voltage and maybe even switch power. A dummy regulator can do none of the above. > You may also run into trouble on boards that use the ability to disable > unused regulators at the end of boot - they'll power things off even > without the ability to change status at runtime. Aha, you mean, I shouldn't put() the regulator, even if it cannot change status itself? Can this also happen with a dummy regulator? > It'd at least be nice to write things more directly, or add some > comments. Hm, sure, I can add some comments:-) Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html