| Message ID | 20130716015038.GD28375@S2101-09.ap.freescale.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/15/2013 07:50 PM, Shawn Guo wrote: > Hi Philipp, > > On Thu, May 30, 2013 at 11:09:00AM +0200, Philipp Zabel wrote: >> This driver implements a reset controller device that toggle a gpio >> connected to a reset pin of a peripheral IC. The delay between assertion >> and de-assertion of the reset signal can be configured via device tree. >> >> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> >> Reviewed-by: Stephen Warren <swarren@nvidia.com> > > I see this patch is very useful, as GPIOs are widely used to reset > components/devices on board. But I do not find the patch in v3.11-rc1. > What's your plan about it? > > Also, I'm wondering if we should register the driver a little bit > early. Please see the following patch. If it makes sense to you, > I can send the patch to you, or you can just quash it into yours. > > Shawn > > ---8<-------- > > From 2f8ce9e5d6525b98f3828b707458e83fabb39d50 Mon Sep 17 00:00:00 2001 > From: Shawn Guo <shawn.guo@linaro.org> > Date: Sun, 14 Jul 2013 20:41:00 +0800 > Subject: [PATCH] ENGR00269945: reset: register gpio-reset driver in > arch_initcall > > 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.
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. 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. Shawn
Hi Shawn, Am Dienstag, den 16.07.2013, 12:10 +0800 schrieb Shawn Guo: > 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. 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. so you want to have gpio-reset probed at arch_initcall time and you have gpio-pca953x probed at subsys_initcall time. Won't then all gpio-reset devices that use gpios on pca953x to reset other peripherals need to be deferred? regards Philipp
Hi Stephen, Am Montag, den 15.07.2013, 21:35 -0600 schrieb Stephen Warren: > On 07/15/2013 07:50 PM, Shawn Guo wrote: > > Hi Philipp, > > > > On Thu, May 30, 2013 at 11:09:00AM +0200, Philipp Zabel wrote: > >> This driver implements a reset controller device that toggle a gpio > >> connected to a reset pin of a peripheral IC. The delay between assertion > >> and de-assertion of the reset signal can be configured via device tree. > >> > >> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > >> Reviewed-by: Stephen Warren <swarren@nvidia.com> > > > > I see this patch is very useful, as GPIOs are widely used to reset > > components/devices on board. But I do not find the patch in v3.11-rc1. > > What's your plan about it? > > > > Also, I'm wondering if we should register the driver a little bit > > early. Please see the following patch. If it makes sense to you, > > I can send the patch to you, or you can just quash it into yours. > > > > Shawn > > > > ---8<-------- > > > > From 2f8ce9e5d6525b98f3828b707458e83fabb39d50 Mon Sep 17 00:00:00 2001 > > From: Shawn Guo <shawn.guo@linaro.org> > > Date: Sun, 14 Jul 2013 20:41:00 +0800 > > Subject: [PATCH] ENGR00269945: reset: register gpio-reset driver in > > arch_initcall > > > > 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. Deferred probing is fine, but it'd be nice to keep the probe deferral loops to a minimum where possible and/or reasonable. regards Philipp
On Tue, Jul 16, 2013 at 11:49:26AM +0200, Philipp Zabel wrote: > so you want to have gpio-reset probed at arch_initcall time and you have > gpio-pca953x probed at subsys_initcall time. Won't then all gpio-reset > devices that use gpios on pca953x to reset other peripherals need to be > deferred? Yes, they will be deferred, but they will be probed and ready at subsys_initcall time when gpio-pca953x probes. This is still good since most of component device drivers probes at module_init time. Shawn
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.)
On 07/16/2013 03:59 AM, Philipp Zabel wrote: ... > Deferred probing is fine, but it'd be nice to keep the probe deferral > loops to a minimum where possible and/or reasonable. I agree, but manually selecting initcalls for drivers is not the solution. See my other email.
On Tue, Jul 16, 2013 at 09:45:43AM -0600, Stephen Warren wrote: > 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. I do not quite follow the argument here. I agree with you that deferred probe is the approach to solve dependencies. But it does not necessarily mean that initcall can not be used to help it save some nasty or nested deferring. Deferred probe and initcalls are not two mutually exclusive mechanisms but two which can help each other. Shawn
On 07/16/2013 09:02 PM, Shawn Guo wrote: > On Tue, Jul 16, 2013 at 09:45:43AM -0600, Stephen Warren wrote: >> 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. > > I do not quite follow the argument here. I agree with you that > deferred probe is the approach to solve dependencies. But it does not > necessarily mean that initcall can not be used to help it save some > nasty or nested deferring. Deferred probe and initcalls are not two > mutually exclusive mechanisms but two which can help each other. My understanding is that deferred probe was implemented specifically to avoid having to, or allowing, the use of initcall levels to determine probe order. However, if someone closely associated with the implementation of deferred probe (e.g. Grant, or a device core maintainer) is willing to step up and say I'm wrong, I'll drop my objection.
On Wed 2013-07-17 10:57:08, Stephen Warren wrote: > On 07/16/2013 09:02 PM, Shawn Guo wrote: > > On Tue, Jul 16, 2013 at 09:45:43AM -0600, Stephen Warren wrote: > >> 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. > > > > I do not quite follow the argument here. I agree with you that > > deferred probe is the approach to solve dependencies. But it does not > > necessarily mean that initcall can not be used to help it save some > > nasty or nested deferring. Deferred probe and initcalls are not two > > mutually exclusive mechanisms but two which can help each other. > > My understanding is that deferred probe was implemented specifically to > avoid having to, or allowing, the use of initcall levels to determine > probe order. > > However, if someone closely associated with the implementation of > deferred probe (e.g. Grant, or a device core maintainer) is willing to > step up and say I'm wrong, I'll drop my objection. AFAIR, defered probing is known to be a "hack" by its authors. We should still try to get initcall levels right... Pavel
On 07/17/2013 03:38 PM, Pavel Machek wrote: > On Wed 2013-07-17 10:57:08, Stephen Warren wrote: >> On 07/16/2013 09:02 PM, Shawn Guo wrote: >>> On Tue, Jul 16, 2013 at 09:45:43AM -0600, Stephen Warren wrote: >>>> 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. >>> >>> I do not quite follow the argument here. I agree with you that >>> deferred probe is the approach to solve dependencies. But it does not >>> necessarily mean that initcall can not be used to help it save some >>> nasty or nested deferring. Deferred probe and initcalls are not two >>> mutually exclusive mechanisms but two which can help each other. >> >> My understanding is that deferred probe was implemented specifically to >> avoid having to, or allowing, the use of initcall levels to determine >> probe order. >> >> However, if someone closely associated with the implementation of >> deferred probe (e.g. Grant, or a device core maintainer) is willing to >> step up and say I'm wrong, I'll drop my objection. > > AFAIR, defered probing is known to be a "hack" by its authors. We > should still try to get initcall levels right... I don't /think/ it was the concept of deferred probe that was considered hacky, but perhaps just the first proof-of-concept implementation, and any hackiness was presumably solved before the feature was merged.
Hi! > >>> I do not quite follow the argument here. I agree with you that > >>> deferred probe is the approach to solve dependencies. But it does not > >>> necessarily mean that initcall can not be used to help it save some > >>> nasty or nested deferring. Deferred probe and initcalls are not two > >>> mutually exclusive mechanisms but two which can help each other. > >> > >> My understanding is that deferred probe was implemented specifically to > >> avoid having to, or allowing, the use of initcall levels to determine > >> probe order. > >> > >> However, if someone closely associated with the implementation of > >> deferred probe (e.g. Grant, or a device core maintainer) is willing to > >> step up and say I'm wrong, I'll drop my objection. > > > > AFAIR, defered probing is known to be a "hack" by its authors. We > > should still try to get initcall levels right... > > I don't /think/ it was the concept of deferred probe that was considered > hacky, but perhaps just the first proof-of-concept implementation, and > any hackiness was presumably solved before the feature was merged. Well... http://lwn.net/Articles/485194/ From: Grant Likely <grant.likely@secretlab.ca> To: linux-kernel@vger.kernel.org Subject: [PATCH] drivercore: Add driver probe deferral mechanism Date: Mon, 5 Mar 2012 08:47:41 -0700 Message-ID: <1330962461-9061-1-git-send-email-grant.likely@secretlab.ca> ... I know that not everybody is happy with this approach, but I've yet to see a better alternative. However, it is *really easy* to find all the users of deferred probe since any user must return -EPROBE_DEFER explicitly. If/when a better approach is found, all the users will be easy to find and modify. ... It sound to me like keeping ammount of -EPROBE_DEFER to minimum is still preferred. Pavel
Hi, On Thu, Jul 18, 2013 at 4:25 AM, Pavel Machek <pavel@denx.de> wrote: > It sound to me like keeping ammount of -EPROBE_DEFER to minimum is > still preferred. Hand-crafting initcall level ordering of various drivers and subsystem is probably an even greater evil though. We've done it in the past, but now that we have deferred probe we have the option of not having to do it every time, such as this. You're spending an awful lot of energy arguing over this, but nobody has actually shown data of how much deferral is happening -- and that it's a real problem. Until someone does so there's no reason to change it from the default module init level at all, I'd say. -Olof
On Wed, Jul 17, 2013 at 9:57 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 07/16/2013 09:02 PM, Shawn Guo wrote: >> On Tue, Jul 16, 2013 at 09:45:43AM -0600, Stephen Warren wrote: >>> 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. >> >> I do not quite follow the argument here. I agree with you that >> deferred probe is the approach to solve dependencies. But it does not >> necessarily mean that initcall can not be used to help it save some >> nasty or nested deferring. Deferred probe and initcalls are not two >> mutually exclusive mechanisms but two which can help each other. > > My understanding is that deferred probe was implemented specifically to > avoid having to, or allowing, the use of initcall levels to determine > probe order. Correct. Futzing around with initcalls may optimize particular use cases, but it isn't complete. I've been pushing for all drivers to use module_initcall() unless there is a very strong reason to do otherwise. Premature optimization (without measuring time consumed) is not a strong justification. g.
On Thu, Jul 18, 2013 at 4:25 AM, Pavel Machek <pavel@denx.de> wrote: > Hi! > >> >>> I do not quite follow the argument here. I agree with you that >> >>> deferred probe is the approach to solve dependencies. But it does not >> >>> necessarily mean that initcall can not be used to help it save some >> >>> nasty or nested deferring. Deferred probe and initcalls are not two >> >>> mutually exclusive mechanisms but two which can help each other. >> >> >> >> My understanding is that deferred probe was implemented specifically to >> >> avoid having to, or allowing, the use of initcall levels to determine >> >> probe order. >> >> >> >> However, if someone closely associated with the implementation of >> >> deferred probe (e.g. Grant, or a device core maintainer) is willing to >> >> step up and say I'm wrong, I'll drop my objection. >> > >> > AFAIR, defered probing is known to be a "hack" by its authors. We >> > should still try to get initcall levels right... >> >> I don't /think/ it was the concept of deferred probe that was considered >> hacky, but perhaps just the first proof-of-concept implementation, and >> any hackiness was presumably solved before the feature was merged. > > Well... > > http://lwn.net/Articles/485194/ > > From: Grant Likely <grant.likely@secretlab.ca> > To: linux-kernel@vger.kernel.org > Subject: [PATCH] drivercore: Add driver probe deferral > mechanism > Date: Mon, 5 Mar 2012 08:47:41 -0700 > Message-ID: > <1330962461-9061-1-git-send-email-grant.likely@secretlab.ca> > ... > > I know that not everybody is happy with this approach, but I've yet to > see a better alternative. However, it is *really easy* to find all > the > users of deferred probe since any user must return -EPROBE_DEFER > explicitly. > If/when a better approach is found, all the users will be easy to find > and modify. > > ... > > It sound to me like keeping ammount of -EPROBE_DEFER to minimum is > still preferred. That wasn't the point I was trying to make in the above thread. My point was that -EPROBE_DEFER is the best option that we currently have to get rid of the initcall ordering madness. I'm all ears if someone comes up with a simple and inexpensive alternative that can take into account dependencies between devices. g.
On Thu, Jul 18, 2013 at 11:45:29AM -0700, Olof Johansson wrote: > Hi, > > On Thu, Jul 18, 2013 at 4:25 AM, Pavel Machek <pavel@denx.de> wrote: > > > It sound to me like keeping ammount of -EPROBE_DEFER to minimum is > > still preferred. > > Hand-crafting initcall level ordering of various drivers and subsystem > is probably an even greater evil though. We've done it in the past, > but now that we have deferred probe we have the option of not having > to do it every time, such as this. > > You're spending an awful lot of energy arguing over this, but nobody > has actually shown data of how much deferral is happening -- and that > it's a real problem. Until someone does so there's no reason to change > it from the default module init level at all, I'd say. On imx6qdl-sabreauto board, there are 3 MAX7310 units as IO expanders to output 3 x 8 = 24 GPIOs. 18 of them are used for resets, enables and pin steering for various peripherals on the board. BACKLITE_ON SAT_SHUTDN_B CPU_PER_RST_B MAIN_PER_RST_B IPOD_RST_B MLB_RST_B SSI_STEERING GPS_RST_B GPS_PWREN VIDEO_ADC_PWRDN_B ENET_CAN1_STEER EIMD30_BTUART3_STEER CAN_STBY CAN_EN USB_H1_PWR USB_OTG_PWR_ON SAT_RST_B NAND_BT_WIFI_STEER Most of these GPIOs need to set up properly at client device driver's probe stage - module_init() time. So if we have all these service providers resets/enables/steering API registered at module_init() too, the probes of all these client device drivers stand a good chance to be deferred. Shawn
On 07/18/2013 09:23 PM, Shawn Guo wrote: > On Thu, Jul 18, 2013 at 11:45:29AM -0700, Olof Johansson wrote: ... >> You're spending an awful lot of energy arguing over this, but nobody >> has actually shown data of how much deferral is happening -- and that >> it's a real problem. Until someone does so there's no reason to change >> it from the default module init level at all, I'd say. > > On imx6qdl-sabreauto board, there are 3 MAX7310 units as IO expanders to > output 3 x 8 = 24 GPIOs. 18 of them are used for resets, enables and > pin steering for various peripherals on the board. > > BACKLITE_ON > SAT_SHUTDN_B > CPU_PER_RST_B > MAIN_PER_RST_B > IPOD_RST_B > MLB_RST_B > SSI_STEERING > GPS_RST_B > > GPS_PWREN > VIDEO_ADC_PWRDN_B > ENET_CAN1_STEER > EIMD30_BTUART3_STEER > CAN_STBY > CAN_EN > USB_H1_PWR > > USB_OTG_PWR_ON > SAT_RST_B > NAND_BT_WIFI_STEER > > Most of these GPIOs need to set up properly at client device driver's > probe stage - module_init() time. So if we have all these service > providers resets/enables/steering API registered at module_init() too, > the probes of all these client device drivers stand a good chance to be > deferred. That sounds like it /could/ well be an issue. Do you have all that actually hooked up through device tree so you can demonstrate the amount of deferred probing that /actually/ happens though? In order to push the probe order in the right direction, I would personally prefer to see some kind of explicit hinting or outright representation of probe order in the DT. If we fiddle with initcall levels to do this, (a) it won't always work; what may be optimal for one board/SoC may not be optimal for another, and with multi-platform kernels, we have to take a broader view, (b) the only beneficiary is Linux; if we add information to DT for this, every OS could benefit in the same way. My view is that when one HW object depends on another (e.g. it's reset by some other module, sends an interrupt to another module, is affected by a pin controller module, etc.), that really is a HW issue, and hence is appropriate to represent in device tree. In fact, we already do represent the dependencies in device tree; e.g. a foo-gpios/foo-reset/... property already indicates this. The issue is that there's no standard naming/structure across all types of DT binding (GPIO, reset, ...), and hence the only way to parse these dependencies is to do exactly what deferred probe is doing; try probing each driver, which then encompasses all device-specific and subsystem-specific naming and DT structure in its actions, and if it fails, then defer probe. If we had some explicit information re: dependencies in the DT, in an entirely standard format, perhaps the device core or similar could parse this and not even attempt probe until the relevant devices had completed probe. If the information turned out to be bogus (e.g. loops, missing drivers), we could always fall back to pure deferred probe later in the boot sequence. Or perhaps we should revisit whether DT node order should be defined as having a guaranteed influence on probe order. We could enforce that the source order has no influence, but then run a post-processing tool that finds all the dependencies from the phandles in the DT, and then re-sorts the DTB to get the probing order right? That would probably require every single driver to be module_initcall, or to ignore driver registration order and only probe devices in the order they appear in DT.
On Fri, Jul 19, 2013 at 09:45:20AM -0600, Stephen Warren wrote: > That sounds like it /could/ well be an issue. Do you have all that > actually hooked up through device tree so you can demonstrate the amount > of deferred probing that /actually/ happens though? > > In order to push the probe order in the right direction, I would > personally prefer to see some kind of explicit hinting or outright > representation of probe order in the DT. I do not have a kernel supporting all those devices yet. Shawn
Am Donnerstag, den 18.07.2013, 11:45 -0700 schrieb Olof Johansson: > Hi, > > On Thu, Jul 18, 2013 at 4:25 AM, Pavel Machek <pavel@denx.de> wrote: > > > It sound to me like keeping ammount of -EPROBE_DEFER to minimum is > > still preferred. > > Hand-crafting initcall level ordering of various drivers and subsystem > is probably an even greater evil though. We've done it in the past, > but now that we have deferred probe we have the option of not having > to do it every time, such as this. > > You're spending an awful lot of energy arguing over this, but nobody > has actually shown data of how much deferral is happening -- and that > it's a real problem. Until someone does so there's no reason to change > it from the default module init level at all, I'd say. Alright, I've backed out the arch_initcall change and resent the patch. If you are fine with that, I'll send you a pull request for this and the "reset: allow drivers to request probe deferral" patch. regards Philipp
diff --git a/drivers/reset/gpio-reset.c b/drivers/reset/gpio-reset.c index 4f372f9..a7f214f 100644 --- a/drivers/reset/gpio-reset.c +++ b/drivers/reset/gpio-reset.c @@ -168,7 +168,17 @@ static struct platform_driver gpio_reset_driver = { }, }; -module_platform_driver(gpio_reset_driver); +static int __init gpio_reset_init(void) +{ + return platform_driver_register(&gpio_reset_driver); +} +arch_initcall(gpio_reset_init); + +static void __exit gpio_reset_exit(void) +{ + platform_driver_unregister(&gpio_reset_driver); +} +module_exit(gpio_reset_exit); MODULE_AUTHOR("Philipp Zabel <p.zabel@pengutronix.de>"); MODULE_DESCRIPTION("gpio reset controller");