Re: [PATCH 07/29] mmc: add a function to get a regulator, supplying card's Vdd

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]

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.

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.

It'd at least be nice to write things more directly, or add some
comments.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]