linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: akpm@linux-foundation.org (Andrew Morton)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] MMC: move regulator handling closer to core v3
Date: Wed, 8 Sep 2010 15:51:14 -0700	[thread overview]
Message-ID: <20100908155114.880463fc.akpm@linux-foundation.org> (raw)
In-Reply-To: <1283677538-31121-1-git-send-email-linus.walleij@stericsson.com>

On Sun,  5 Sep 2010 11:05:38 +0200
Linus Walleij <linus.walleij@stericsson.com> wrote:

> After discovering a problem in regulator reference counting I
> took Mark Brown's advice to move the reference count into the
> MMC core by making the regulator status a member of
> struct mmc_host.
> 
>
> ...
>
> -static inline void pxamci_set_power(struct pxamci_host *host, unsigned int vdd)
> +static inline void pxamci_set_power(struct pxamci_host *host,
> +				    unsigned char power_mode,
> +				    unsigned int vdd)
>  {
>  	int on;
>  
> -#ifdef CONFIG_REGULATOR
> -	if (host->vcc)
> -		mmc_regulator_set_ocr(host->vcc, vdd);
> -#endif
> +	if (host->vcc) {
> +		int ret;
> +
> +		if (power_mode == MMC_POWER_UP)
> +			ret = mmc_regulator_set_ocr(host->mmc, host->vcc, vdd);
> +		else if (power_mode == MMC_POWER_OFF)
> +			ret = mmc_regulator_set_ocr(host->mmc, host->vcc, 0);
> +	}

There's no point in copying the return value into a local then ignoring
it.  mmc_regulator_set_ocr() can return a negative errno so we should
test for that, clean up and propagate the error.

If we really do deliberately ignore the error then there should be a
code comment which excuses this behaviour and perhaps a warning printk.

The same comments apply to mmci_set_ios().

omap_hsmmc_1_set_power() gets it right.

Why doesn't omap_hsmmc_23_set_sleep() run .before_set_reg() and
.after_set_reg()?

>
> ...
>

  reply	other threads:[~2010-09-08 22:51 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-05  9:05 [PATCH] MMC: move regulator handling closer to core v3 Linus Walleij
2010-09-08 22:51 ` Andrew Morton [this message]
2010-09-09  6:01   ` Adrian Hunter
2010-09-09  9:38     ` Mark Brown
2010-09-09 12:56       ` Adrian Hunter
2010-09-09 13:12         ` Mark Brown
2010-09-10 10:05           ` Adrian Hunter
2010-09-10 10:37             ` Mark Brown
2010-09-09  7:03   ` Linus Walleij

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100908155114.880463fc.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).