linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Jon Hunter <jonathanh@nvidia.com>
Cc: Liam Girdwood <lgirdwood@gmail.com>,
	linux-kernel@vger.kernel.org, linux-tegra@vger.kernel.org,
	Javier Martinez Canillas <javier@dowhile0.org>
Subject: Re: [PATCH] regulator: core: Unset supplies when regulators are unregistered
Date: Mon, 9 Jan 2017 16:40:39 +0000	[thread overview]
Message-ID: <20170109164039.4hsiqpvx2khriktw@sirena.org.uk> (raw)
In-Reply-To: <084533f5-d611-59ac-7329-657b0523bdb3@nvidia.com>

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

On Mon, Jan 09, 2017 at 01:37:18PM +0000, Jon Hunter wrote:
> On 06/01/17 18:29, Mark Brown wrote:

> > Why is a parent device doing this?  This doesn't seem like safe or
> > helpful behaviour and with probe deferral we'd generally expect the
> > device to acquire resources before it starts making use of them.

> It is not really the parent that is doing this. The child regulator in
> this case is physically a separate regulator from the main PMIC device
> that is registering the supply regulators.

The parent of the regulator being unregistered.

> > We can't completely stop people doing this but we do make fairly strong
> > efforts to stop people pulling in use devices.

> Right, but before removing/unregistering a regulator today, we never
> check to see if it is the supply for any other regulator.

We're trying to not specifically look for that by making supplies look
like regular consumers which we were handling by holding a reference on
the module (far from bulletproof but hey).  Special casing them was way
too much of a source of bugs to be sustainable.

> > This seems like storing up trouble for the future, we'll end up with
> > live child devices with parents that weren't around or being refcounted
> > through some of the lifetime of the device which will doubtless come
> > back and bite us later.

> Yes I am not completely happy. Maybe we should wait for the parent
> device to be bound before allowing any of its's regulators to be added
> as supplies for other regulators? I gave the following a quick test and
> this seems to work too ...

Yeah, that thought occurred to me too.  

> The above prevents anyone from using a regulator as a supply if the
> parent has not been bound yet. Therefore, if one of the parent's
> regulators fails to register, after some have already been successfully
> registered, there is no chance any of the successfully added regulators
> will have been already added as a supply to some other regulator in the
> system. Hope that's clear!

Can you cook it up into a proper patch please?  I *think* that should be
OK from a correctness point of view and it should be way more robust (as
well as simpler to implement).

> > Please don't mix different changes into one commit, as covered in
> > SubmittingPatches please send one patch per change.  This makes things
> > easier to 

> Sorry, may be I worded this badly, however, it is a consequence of this
> particular change and otherwise it would not be needed.

Ah, OK.  That makes more sense - at a thousand foot view it wasn't
obvious.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

      reply	other threads:[~2017-01-09 16:41 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-06 11:54 [PATCH] regulator: core: Unset supplies when regulators are unregistered Jon Hunter
2017-01-06 18:29 ` Mark Brown
2017-01-09 13:37   ` Jon Hunter
2017-01-09 16:40     ` Mark Brown [this message]

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=20170109164039.4hsiqpvx2khriktw@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=javier@dowhile0.org \
    --cc=jonathanh@nvidia.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.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).