linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Brown <broonie@opensource.wolfsonmicro.com>
To: Grant Likely <grant.likely@secretlab.ca>
Cc: Linus Walleij <linus.walleij@stericsson.com>,
	linux-kernel@vger.kernel.org, "Rafael J. Wysocki" <rjw@sisk.pl>,
	Greg Kroah-Hartman <gregkh@suse.de>,
	Kevin Hilman <khilman@ti.com>,
	Alan Stern <stern@rowland.harvard.edu>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jesse Barnes <jbarnes@virtuousgeek.org>
Subject: Re: [PATCH] gpiolib: Defer failed gpio requests by default
Date: Fri, 18 May 2012 09:45:25 +0100	[thread overview]
Message-ID: <20120518084525.GG4073@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <20120518043219.740113E062C@localhost>

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

On Thu, May 17, 2012 at 10:32:19PM -0600, Grant Likely wrote:

> be done is to move the device to the end of the list.  The agreement
> when this was last discussed on the ML was to make the drivers do an
> explicit move immediately after it has validated all its dependencies.
> That is why it doesn't work to return -EPROBE_DEFER by default from
> helpers; because it becomes really hard to audit if the drivers are
> doing the right thing.

Ah, oh dear - I missed that bit of the discussion I'm afraid and had
thought we'd gone with your option 2 below.  This means that the ASoC
usage is currently not helping anything (though it's no worse than what
we had before), and the regulator framework usage won't do the right
thing (though in practice I don't expect it to actually go off and
there's fun and games associated with that if we actually need to
suspend regulators from software anyway).

> I see two solutions to this though;
> 1) add a new .preprobe hook to device drivers that will respect
>    -EPROBE_DEFER and ignore -EPROBE_DEFER on the regular .probe.
>    Probing a driver will call both, but if the .preprobe hook is
>    populated, then it will also reorder dpm_list between the two
>    calls.
>    - This still requires modifying drivers, but at least it is
>      auditable by mere-mortal reviewers.
>    - It also means all bus_type code has to add a new hook.  Blech.

Yeah, this seems very painful, especially the bit where we have to
change a good selection of buses and drivers to implement and use it.

> 2) simply force devices to the end of dpm_list before each probe
>    attempt (provided that it doesn't have any children).
>    - I've avoided this because it adds a claim of the dpm_list_mtx
>      mutex on every single probe call and adds a lot more manipulation
>      of the list....
>    - However, it should make your patch here actually safe.  If a
>      device gets deferred, it will be guaranteed to be at the end of
>      the list because it gets moved there on every probe attempt.
>    - also after reading through the code (see my notes below; I'm a
>      bit frustrated with the whole thing) I think it is probably the
>      right thing to do.

This does feel much more robust to me than manually reordering in
drivers, we'll have to see if it causes a performance problem though and
if it does perhaps we can handle it.

I guess the other thing is that if we get a lot of drivers implementing
deferral and we stop playing around with initcall stuff then we'll loose
a reasonable proportion of the advantage of not taking the mutex and
reordering.

Either of these options would avoid the surprise that currently exists
with the API.

> Will you be at Connect?  I could use some time sitting in a room with
> smart people to dig through this code and talk out what it should be
> doing.

I won't be unfortunately.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

  reply	other threads:[~2012-05-18  8:45 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-02 11:49 [PATCH] gpiolib: Defer failed gpio requests by default Mark Brown
2012-05-18  4:32 ` Grant Likely
2012-05-18  8:45   ` Mark Brown [this message]
2012-05-18 16:35   ` Alan Stern
  -- strict thread matches above, loose matches on Subject: below --
2012-07-09 11:22 Mark Brown
2012-07-09 20:31 ` Linus Walleij
2012-07-09 21:54   ` Grant Likely
2012-07-10 11:07     ` Mark Brown
2012-07-17 18:20 ` 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=20120518084525.GG4073@opensource.wolfsonmicro.com \
    --to=broonie@opensource.wolfsonmicro.com \
    --cc=grant.likely@secretlab.ca \
    --cc=gregkh@suse.de \
    --cc=jbarnes@virtuousgeek.org \
    --cc=khilman@ti.com \
    --cc=linus.walleij@stericsson.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rjw@sisk.pl \
    --cc=stern@rowland.harvard.edu \
    --cc=tglx@linutronix.de \
    /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).