public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
To: Mark Brown <broonie@kernel.org>
Cc: <lgirdwood@gmail.com>, <patches@opensource.wolfsonmicro.com>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] regulator: arizona-ldo1: Only enable status change if we have LDOENA
Date: Fri, 22 Apr 2016 17:38:02 +0100	[thread overview]
Message-ID: <571A536A.9000509@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <20160422150446.GF3217@sirena.org.uk>

On 22/04/16 16:04, Mark Brown wrote:
> On Fri, Apr 22, 2016 at 02:43:28PM +0100, Richard Fitzgerald wrote:
>> The driver was hardcoding REGULATOR_CHANGE_STATUS on the regulator
>> which made the regulator core assume that it can be powered off.
>>
>> The power state of the regulator is controlled by the LDOENA pin so
>> this patch changes to only setting the REGULATOR_CHANGE_STATUS flag
>> if we have a valid gpio for LDOENA.
> What's the difference between this and the previous version of the patch
> and what problem is this aiming to solve?  If we want to disable the
> regulator why would we not be happy to do that by removing the supply?
The background to all this is that runtime suspend and resume needs to 
know whether the DCVDD turned off. If it definitely turned off a regmap 
cache sync is safe - if not or I can't be sure then I need the overhead 
of a forced reset to restore register defaults before the sync.

What I'm trying to achieve here is to stop the regulator core sending 
false notifications that LDO1 has been turned off. The way that the 
regulator core code handles the disable notifier has no dependency on 
what happens to the parent supply. The REGULATOR_CHANGE_STATUS flag is 
used to indicate whether the status of _this_ regulator can be changed 
(it doesn't affect whether the parent is disabled).

So if LDO1 is disabled without an LDOENA and without this patch, it 
looks like the current core behaviour of the functions 
regulator_disable(), _regulator_disable() and _regulator_do_disable() is:

1) Are we the last user? - Yes
2) _regulator_can_change_status()? - Yes because REGULATOR_CHANGE_STATUS 
is set
3) Send PRE_DISABLE notification
4) call _regulator_do_disable(), no GPIO pin and no disable callback so 
return 0 (claims success even though there was no way to disable it)
5) Send REGULATOR_EVENT_DISABLE
6) Return to regulator_disable() and then disable the parent supply

The result will be that we got a disable notification though LDO1 wasn't 
disabled.

I think it's a bug that LDO1 claimed to be able to turn off when it 
couldn't, and fixing that prevents bogus disable notifications.

  reply	other threads:[~2016-04-22 16:38 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-22 13:43 [PATCH] regulator: arizona-ldo1: Only enable status change if we have LDOENA Richard Fitzgerald
2016-04-22 15:04 ` Mark Brown
2016-04-22 16:38   ` Richard Fitzgerald [this message]
2016-04-24 23:23     ` Mark Brown
2016-04-25  9:26       ` Richard Fitzgerald

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=571A536A.9000509@opensource.wolfsonmicro.com \
    --to=rf@opensource.wolfsonmicro.com \
    --cc=broonie@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@opensource.wolfsonmicro.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox