linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Brian Norris <briannorris@chromium.org>
Cc: Chen-Yu Tsai <wenst@chromium.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Linux Kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] regulator: core: resolve supply voltage deferral silently
Date: Thu, 2 Sep 2021 18:06:13 +0100	[thread overview]
Message-ID: <20210902170613.GG11164@sirena.org.uk> (raw)
In-Reply-To: <CA+ASDXMLBpF6bQLCoxkN-+CqjxOX-ujzYBTV1f=zU1J7fFNuDA@mail.gmail.com>

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

On Wed, Sep 01, 2021 at 01:06:28PM -0700, Brian Norris wrote:
> On Wed, Sep 1, 2021 at 8:09 AM Mark Brown <broonie@kernel.org> wrote:

> > This doesn't make sense to me.  Why are we getting as far as trying to
> > read the voltage if we've been told to defer probe?  This suggests that
> > we ought to be doing this earlier on.  I see that the logic is already
> > there to handle a deferral being generated here but it looks off.

> Take a look at the commit this "Fixes":

> 21e39809fd7c ("regulator: vctrl: Avoid lockdep warning in enable/disable ops")

That driver change is at most tangentially related to the code that's
being updated, this is a prime example of just randomly shoving Fixes
lines onto things :( .  It's perfectly fine to send patches without a
Fixes line, adding them when they're not really related at best creates
noise and at worst causes problems with backporting (especially given
the whole pulling random commits into stable thing we have these days).

> Frankly, I'm not sure if we're abusing regulator framework features
> (particularly, around use of ->supply) in commit 21e39809fd7c, or if
> this is just a lacking area in the framework. I'm interested in
> whether you have thoughts on doing this Better(TM).

That's definitely an abuse of the API, the hardware design is pretty
much a gross hack anywhere as far as I remember.  As Chen-Yu says I'd
only expect this to be possible in the case where the supply is in
bypass mode and hasn't got its own parent.  In any case I can see why
it's happening now...

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

  parent reply	other threads:[~2021-09-02 17:06 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-26 19:40 [PATCH] regulator: core: resolve supply voltage deferral silently Brian Norris
2021-09-01 15:08 ` Mark Brown
2021-09-01 20:06   ` Brian Norris
2021-09-02  4:09     ` Chen-Yu Tsai
2021-09-02 17:06     ` Mark Brown [this message]
2021-09-02 22:41       ` Brian Norris
2021-09-03 11:10         ` Mark Brown
2021-09-03 17:09           ` Brian Norris
2021-09-03 17:54             ` Mark Brown
2021-09-03 20:10               ` Brian Norris
2021-09-13 10:53 ` Mark Brown

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=20210902170613.GG11164@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=briannorris@chromium.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=wenst@chromium.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).