All of lore.kernel.org
 help / color / mirror / Atom feed
From: swarren@wwwdotorg.org (Stephen Warren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v8] reset: Add driver for gpio-controlled reset pins
Date: Tue, 16 Jul 2013 09:45:43 -0600	[thread overview]
Message-ID: <51E56AA7.7020103@wwwdotorg.org> (raw)
In-Reply-To: <20130716041056.GA30067@S2101-09.ap.freescale.net>

On 07/15/2013 10:10 PM, Shawn Guo wrote:
> On Mon, Jul 15, 2013 at 09:35:52PM -0600, Stephen Warren wrote:
>>> It's a little bit late to register gpio-reset driver at module_init
>>> time, because gpio-reset provides reset control via gpio for other
>>> devices which are mostly probed at module_init time too.  And it
>>> becomes even worse, when the gpio comes from IO expander on I2C bus,
>>> e.g. pca953x.  In that case, gpio-reset needs to be ready before I2C
>>> bus driver which is generally ready at subsys_initcall time.  Let's
>>> register gpio-reset driver in arch_initcall() to have it ready early
>>> enough.
>>
>> There's no need for the reset driver to be registered before its users;
>> the users of the reset GPIO will simply have their probe deferred until
>> the reset controller is available, and then everything will work out
>> just fine.
>>
>>> The defer probe mechanism is not used here, because a reset controller
>>> driver should be reasonably registered early than other devices.  More
>>> importantly, defer probe doe not help in some nasty cases, e.g. the
>>> gpio-pca953x device itself needs a reset from gpio-reset driver to start
>>> working.
>>
>> That should work fine with deferred probe.
> 
> I should probably rework the commit log.  But I do not see a problem
> to register gpio-reset driver a little bit earlier. 

Registering the driver earlier won't cause any bugs. However, it's not
the correct approach.

Deferred probe /is/ the approach for assuring correct dependencies
between drivers. It works and should be used. There are not enough
initcall levels to play games using initcalls and solve all the issues,
and the ordering requirements vary board-to-board. Deferred probe at
runtime handles this without having to manually place all the drivers
into specific initcall levels, and dynamically adjusts to board
differences, since it all happens automatically at run-time.

Consider a SoC that has:

* An external PMIC, which the CPU has to communicate with to enable
power to all SoC components outside the CPU and a single I2C instance
dedicated to communicating with the PMIC.
* An on-SoC reset controller (for on-SoC) modules that resets other
on-SoC I2C controllers
* An on-SoC I2C controller which communicates with a GPIO expander
* One of the GPIOs on that expander is the reset GPIO for some other
device connected by I2C

What initcall levels are you going to use for all the drivers
(PM-related I2C, PMIC, on-SoC reset controller, secondary I2C
controller, GPIO expander IC, GPIO-reset "device", the final device
affected by the GPIO reset controller).

Now wonder whether that same order is going to be suitable for every
single other board. That's quite unlikely...

 On the other hand,
> if we reply on deferred probe, many drivers probe could be deferred.
> For example, on my system, the gpio-pca953x on I2C bus works as a GPIO
> controller and provides resets to many board level components.
> Deferring probe of gpio-pca953x on I2C bus means every single probe of
> all these components gets deferred.  IMO, this situation should be
> reasonably avoided.

Yes, deferred probe will happen. If we want to solve that, I believe we
need a more generic solution specifically targeted at that, rather that
playing games with initcalls.

(It'd be nice if each DT node declared all its resources in a way the DT
core could parse them and determine the dependency graph itself, without
having to write code in all drivers to extract that information from DT.
This would need a standardized resource representation in DT, and as
such would mean throwing out all the bindings and starting again...
Perhaps some parallel set of properties could be invented to hint at the
order, but still fall back to deferred probe where that information was
missing or inaccurate.)

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
To: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>,
	Fabio Estevam
	<fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>,
	Mike Turquette
	<mturquette-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>,
	Len Brown <len.brown-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
	"Rafael J. Wysocki" <rjw-KKrjLPT3xs0@public.gmane.org>,
	Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH v8] reset: Add driver for gpio-controlled reset pins
Date: Tue, 16 Jul 2013 09:45:43 -0600	[thread overview]
Message-ID: <51E56AA7.7020103@wwwdotorg.org> (raw)
In-Reply-To: <20130716041056.GA30067-rvtDTF3kK1ictlrPMvKcciBecyulp+rMXqFh9Ls21Oc@public.gmane.org>

On 07/15/2013 10:10 PM, Shawn Guo wrote:
> On Mon, Jul 15, 2013 at 09:35:52PM -0600, Stephen Warren wrote:
>>> It's a little bit late to register gpio-reset driver at module_init
>>> time, because gpio-reset provides reset control via gpio for other
>>> devices which are mostly probed at module_init time too.  And it
>>> becomes even worse, when the gpio comes from IO expander on I2C bus,
>>> e.g. pca953x.  In that case, gpio-reset needs to be ready before I2C
>>> bus driver which is generally ready at subsys_initcall time.  Let's
>>> register gpio-reset driver in arch_initcall() to have it ready early
>>> enough.
>>
>> There's no need for the reset driver to be registered before its users;
>> the users of the reset GPIO will simply have their probe deferred until
>> the reset controller is available, and then everything will work out
>> just fine.
>>
>>> The defer probe mechanism is not used here, because a reset controller
>>> driver should be reasonably registered early than other devices.  More
>>> importantly, defer probe doe not help in some nasty cases, e.g. the
>>> gpio-pca953x device itself needs a reset from gpio-reset driver to start
>>> working.
>>
>> That should work fine with deferred probe.
> 
> I should probably rework the commit log.  But I do not see a problem
> to register gpio-reset driver a little bit earlier. 

Registering the driver earlier won't cause any bugs. However, it's not
the correct approach.

Deferred probe /is/ the approach for assuring correct dependencies
between drivers. It works and should be used. There are not enough
initcall levels to play games using initcalls and solve all the issues,
and the ordering requirements vary board-to-board. Deferred probe at
runtime handles this without having to manually place all the drivers
into specific initcall levels, and dynamically adjusts to board
differences, since it all happens automatically at run-time.

Consider a SoC that has:

* An external PMIC, which the CPU has to communicate with to enable
power to all SoC components outside the CPU and a single I2C instance
dedicated to communicating with the PMIC.
* An on-SoC reset controller (for on-SoC) modules that resets other
on-SoC I2C controllers
* An on-SoC I2C controller which communicates with a GPIO expander
* One of the GPIOs on that expander is the reset GPIO for some other
device connected by I2C

What initcall levels are you going to use for all the drivers
(PM-related I2C, PMIC, on-SoC reset controller, secondary I2C
controller, GPIO expander IC, GPIO-reset "device", the final device
affected by the GPIO reset controller).

Now wonder whether that same order is going to be suitable for every
single other board. That's quite unlikely...

 On the other hand,
> if we reply on deferred probe, many drivers probe could be deferred.
> For example, on my system, the gpio-pca953x on I2C bus works as a GPIO
> controller and provides resets to many board level components.
> Deferring probe of gpio-pca953x on I2C bus means every single probe of
> all these components gets deferred.  IMO, this situation should be
> reasonably avoided.

Yes, deferred probe will happen. If we want to solve that, I believe we
need a more generic solution specifically targeted at that, rather that
playing games with initcalls.

(It'd be nice if each DT node declared all its resources in a way the DT
core could parse them and determine the dependency graph itself, without
having to write code in all drivers to extract that information from DT.
This would need a standardized resource representation in DT, and as
such would mean throwing out all the bindings and starting again...
Perhaps some parallel set of properties could be invented to hint at the
order, but still fall back to deferred probe where that information was
missing or inaccurate.)

  parent reply	other threads:[~2013-07-16 15:45 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-30  9:09 [PATCH v8] reset: Add driver for gpio-controlled reset pins Philipp Zabel
2013-05-30  9:09 ` Philipp Zabel
2013-05-30 10:37 ` Pavel Machek
2013-05-30 10:37   ` Pavel Machek
2013-07-16  1:50 ` Shawn Guo
2013-07-16  1:50   ` Shawn Guo
2013-07-16  3:35   ` Stephen Warren
2013-07-16  3:35     ` Stephen Warren
2013-07-16  4:10     ` Shawn Guo
2013-07-16  4:10       ` Shawn Guo
2013-07-16  9:49       ` Philipp Zabel
2013-07-16  9:49         ` Philipp Zabel
2013-07-16 12:56         ` Shawn Guo
2013-07-16 12:56           ` Shawn Guo
2013-07-16 15:45       ` Stephen Warren [this message]
2013-07-16 15:45         ` Stephen Warren
2013-07-17  3:02         ` Shawn Guo
2013-07-17  3:02           ` Shawn Guo
2013-07-17 16:57           ` Stephen Warren
2013-07-17 16:57             ` Stephen Warren
2013-07-17 21:38             ` Pavel Machek
2013-07-17 21:38               ` Pavel Machek
2013-07-17 22:24               ` Stephen Warren
2013-07-17 22:24                 ` Stephen Warren
2013-07-18 11:25                 ` Pavel Machek
2013-07-18 11:25                   ` Pavel Machek
2013-07-18 18:45                   ` Olof Johansson
2013-07-18 18:45                     ` Olof Johansson
2013-07-19  3:23                     ` Shawn Guo
2013-07-19  3:23                       ` Shawn Guo
2013-07-19 15:45                       ` Stephen Warren
2013-07-19 15:45                         ` Stephen Warren
2013-07-22  7:47                         ` Shawn Guo
2013-07-22  7:47                           ` Shawn Guo
2013-07-26 10:26                     ` Philipp Zabel
2013-07-26 10:26                       ` Philipp Zabel
2013-07-18 22:55                   ` Grant Likely
2013-07-18 22:55                     ` Grant Likely
2013-07-18 22:50             ` Grant Likely
2013-07-18 22:50               ` Grant Likely
2013-07-16  9:59     ` Philipp Zabel
2013-07-16  9:59       ` Philipp Zabel
2013-07-16 15:48       ` Stephen Warren
2013-07-16 15:48         ` Stephen Warren
2013-07-16  6:51   ` Shawn Guo
2013-07-16  6:51     ` Shawn Guo
2013-07-16  9:51     ` Philipp Zabel
2013-07-16  9:51       ` Philipp Zabel
2013-07-16 12:15       ` Shawn Guo
2013-07-16 12:15         ` Shawn Guo
2013-07-16 15:47     ` Stephen Warren
2013-07-16 15:47       ` Stephen Warren
2013-07-17  3:43       ` Shawn Guo
2013-07-17  3:43         ` Shawn Guo
2013-07-17  7:17       ` Philipp Zabel
2013-07-17  7:17         ` Philipp Zabel

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=51E56AA7.7020103@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=linux-arm-kernel@lists.infradead.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 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.