All of lore.kernel.org
 help / color / mirror / Atom feed
From: Adrian Hunter <adrian.hunter@nokia.com>
To: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"mm-commits@vger.kernel.org" <mm-commits@vger.kernel.org>,
	"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
	"lrg@slimlogic.co.uk" <lrg@slimlogic.co.uk>,
	"sameo@linux.intel.com" <sameo@linux.intel.com>
Subject: Re: + mmc-use-regulator-framework-correctly.patch added to -mm tree
Date: Sun, 08 Aug 2010 14:41:40 +0300	[thread overview]
Message-ID: <4C5E97F4.8050501@nokia.com> (raw)
In-Reply-To: <D7F4B8A4-C756-44EC-AD90-0460DCEC80FC@opensource.wolfsonmicro.com>

Mark Brown wrote:
> On 5 Aug 2010, at 22:32, akpm@linux-foundation.org wrote:
> 
>> Subject: mmc: use regulator framework correctly
>> From: Adrian Hunter <adrian.hunter@nokia.com>
> 
>> Issues with the regulator framework no longer exist, so regulator_enable()
>> / regulator_disable() should be used correctly.
> 
> There have been no changes in the regulator framework here. David had some strong ideas about what he wanted the regulator API to do which caused him to make a number of forceful statements but that's not the same thing.
> 
>> -	int			enabled;
>> -
>> -	enabled = regulator_is_enabled(supply);
>> -	if (enabled < 0)
>> -		return enabled;
>>
>> 	if (vdd_bit) {
>> 		int		tmp;
>> @@ -819,9 +814,9 @@ int mmc_regulator_set_ocr(struct regulat
>> 		else
>> 			result = 0;
>>
>> -		if (result == 0 && !enabled)
>> +		if (result == 0)
>> 			result = regulator_enable(supply);
>> -	} else if (enabled) {
>> +	} else {
>> 		result = regulator_disable(supply);
>> 	}
> 
> This is only going to do the right thing if we can rely on the MMC framework matching the number of enables it does with the number of disables, including handling of startup with an already enabled regulator causing the refcount to start at one if regulator_get_exclusive() is used (which the current code does rely on). I'd rather see the analysis in the changelog explaining why the new code is safe.

Yes, the patch is no good.  If I had tried to make a decent commit message,
as you pointed out, I would not have sent the patch.  Sorry all.

Andrew please remove this patch.

  reply	other threads:[~2010-08-08 11:42 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-05 21:32 + mmc-use-regulator-framework-correctly.patch added to -mm tree akpm
2010-08-05 21:43 ` Mark Brown
2010-08-08 11:41   ` Adrian Hunter [this message]
  -- strict thread matches above, loose matches on Subject: below --
2010-08-05 21:32 akpm

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=4C5E97F4.8050501@nokia.com \
    --to=adrian.hunter@nokia.com \
    --cc=akpm@linux-foundation.org \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=linux-mmc@vger.kernel.org \
    --cc=lrg@slimlogic.co.uk \
    --cc=mm-commits@vger.kernel.org \
    --cc=sameo@linux.intel.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.