On Sun, Jan 19, 2014 at 07:56:53PM -0800, Olof Johansson wrote:
> + if (host->card_regulator) {
NULL is a potentially valid regulator; use IS_ERR(). Also see below...
> + dev_dbg(host->parent, "Enabling external regulator");
> + if (regulator_enable(host->card_regulator))
> + dev_err(host->parent, "Failed to enable external regulator");
You should really log the error code here.
> + host->card_regulator = regulator_get(host->parent, "card-external-vcc");
> +
...this won't handle probe deferral or other errors. Given what this is
for it should probably be using regulator_get_optional(), the driver is
happy to carry on if a supply isn't available. On the other hand it
just enables and disables so it's probably easier to just ignore that
and let the stub regulator get used anyway.
I have to say I do worry what happens if the device has multiple
supplies that need to be managed (which is common enough, for example
analogue and digital supplies tend to be decoupled) or if the device can
do useful things with the supplies. In the case of SDIO it's *probably*
less relevant though I have seen applications where it might be.
Attachment:
signature.asc
Description: Digital signature
