drivercore: Add driver probe deferral mechanism
PATCH 1/1 drivercore: Add driver probe deferral mechanism
Allow drivers to report at probe time that they cannot get all the resources required by the device, and should be retried at a later time.
This should completely solve the problem of getting devices initialized in the right order. Right now this is mostly handled by mucking about with initcall ordering which is a complete hack, and doesn't even remotely handle the case where device drivers are in modules. This approach completely sidesteps the issues by allowing driver registration to occur in any order, and any driver can request to be retried after a few more other drivers get probed.
This still is not tested, but I'd like to get early feedback on if this is the correct approach.
v2: - added locking so it should no longer be utterly broken in that regard
- remove device from deferred list at device_del time.
- Still completely untested with any real use case, but has been boot tested.
---
Mark, I'm particularly interested in your thoughts on this approach. It is decidedly "low-tech" in its approach to handling device dependencies, but it has the advantage of being simple and should handle a wide range of use-cases reliably. Would this work for ALSA SoC probing?
g.
drivers/base/base.h | 1 + drivers/base/core.c | 2 ++ drivers/base/dd.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++- include/linux/device.h | 5 ++++ 4 files changed, 71 insertions(+), 1 deletions(-)
drivers/base/base.h +1 -0
@@ -105,6 +105,7 @@ extern void bus_remove_driver(struct device_driver *drv);extern void driver_detach (struct device_driver *drv);extern int driver_probe_device (struct device_driver *drv,struct device *dev); +extern void driver_deferred_probe_del (struct device *dev);static inline int driver_match_device (struct device_driver *drv,struct device *dev) {
drivers/base/core.c +2 -0
@@ -588,6 +588,7 @@ void device_initialize(struct device *dev) { dev->kobj.kset = devices_kset;kobject_init (&dev->kobj, &device_ktype); +INIT_LIST_HEAD (&dev->deferred_probe);INIT_LIST_HEAD (&dev->dma_pools);mutex_init (&dev->mutex);lockdep_set_novalidate_class (&dev->mutex);
@@ -1119,6 +1120,7 @@ void device_del(struct device *dev)device_remove_file (dev, &uevent_attr);device_remove_attrs (dev);bus_remove_device (dev); +driver_deferred_probe_del (dev);/* * Some platform devices are driven without driver attached
drivers/base/dd.c +63 -1
82 lines not on this page · raw message
include/linux/device.h +5 -0
@@ -506,6 +506,10 @@ struct device_dma_parameters {* @mutex: Mutex to synchronize calls to its driver. * @bus: Type of bus device is on. * @driver: Which driver has allocated this +* @deferred_probe: entry in deferred_probe_list which is used to retry the +* binding of drivers which were unable to get all the resources +* needed by the device; typically because it depends on another +* driver getting probed first. * @platform_data: Platform data specific to the device. * Example: For devices on custom boards, as typical of embedded * and SOC based hardware, Linux often uses platform_data to point
@@ -564,6 +568,7 @@ struct device {struct bus_type *bus;/* type of bus device is on */ struct device_driver *driver;/* which driver has allocated this device */ +struct list_head deferred_probe;void *platform_data;/* Platform specific data, device core doesn't touch it */ struct dev_pm_info power;
Quoted 2 lines · Grant Likely
Allow drivers to report at probe time that they cannot get all the resources required by the device, and should be retried at a later time.
When is "later"?
And why would a driver not be able to get all of the proper resources?
Why can't a bus, at a later time, just try to reprobe everything when it determines that it is a "later" time now, without having to do this added change to the core?
Quoted 7 lines
This should completely solve the problem of getting devices initialized in the right order. Right now this is mostly handled by mucking about with initcall ordering which is a complete hack, and doesn't even remotely handle the case where device drivers are in modules. This approach completely sidesteps the issues by allowing driver registration to occur in any order, and any driver can request to be retried after a few more other drivers get probed.
Why would drivers in modules be an issue? If a driver depends on another driver, making it a module dependancy would solve the problem, right?
thanks,
greg k-h
Quoted 1 line · Greg KH
And why would a driver not be able to get all of the proper resources?
If the resources are provided by another driver (for example, a clock, GPIO or regulator) then the driver doing the providing needs to have initialized before the resource can be used. Currently we have no way of ensuring that this happens.
Quoted 3 lines
Why can't a bus, at a later time, just try to reprobe everything when it determines that it is a "later" time now, without having to do this added change to the core?
The buses can't do anything to help as they have no visibility of this and the chances are that devices on several different buses are involved.
Quoted 3 lines
Why would drivers in modules be an issue? If a driver depends on another driver, making it a module dependancy would solve the problem, right?
The general problem here is that the drivers depend on each other in system-specific fashions that have nothing to do with the control heirachy the device model uses - for example, a device may depend on having a clock provided but the specific clock used could come from essentially any device in a given system with the driver accessing that clock via the clock API which matches devices up with their providers. The API indirection means we can share code between systems but means that the kernel infrastructure has no visibility of the relationships.
Quoted 5 lines · Greg KH
Quoted 2 lines · Grant Likely
Allow drivers to report at probe time that they cannot get all the resources required by the device, and should be retried at a later time.
When is "later"?
In this case, after at least one other device has successfully probed. The 'later' is handled in a workqueue that walks the list asynchronously from normal initialization.
Quoted 1 line
And why would a driver not be able to get all of the proper resources?
Discussed below...
Quoted 3 lines
Why can't a bus, at a later time, just try to reprobe everything when it determines that it is a "later" time now, without having to do this added change to the core?
It can't be done by a specific bus type because it has zero relationship with the bus type. For example, it is typical for an SDHCI driver to require a GPIO line for the card detect switch, and the device cannot be initialized until it has it. However, the bus_type that the SDHCI driver is attached to could be anything; platform_bus_type, pci, amba, etc. It isn't a bus_type deficiency, but rather that the driver core has no way to gracefully handle devices that get probed in an undetermined order.
It has to be done at the core level because any device in the system, regardless of bus_type, may require another device to be probed first. Originally I tried modifying the drivers to successfully probe anyway and then 'go to sleep' to try again later, but it turned out to push a lot of complexity into the device drivers when it can be solved far more simply if the driver core has the ability to retry drivers that request it.
Another example is sound on embedded systems. A "sound card" typically consists of multiple devices; one or more codecs (often i2c or spi attached), a sound bus (often i2s), a dma controller, and a lump of machine/platform specific code that ties them all together. Right now the ASoC code is going through all kinds of gymnastics make each component register with the ASoC layer and the 'tie together' driver has to wait for each of them to show up. As Mark will attest, it is a lot of domain specific code that cannot be easily generalized to be used by other subsystems.
I also could have implemented this without the deferred_probe list, but doing so would have required every unbound device in the system to be retried against every matching driver each time an -EAGAIN event occurred. I figured that would be rather excessive when the kernel already has specific knowledge of devices that need to be retried.
Quoted 11 lines
Quoted 7 lines
This should completely solve the problem of getting devices initialized in the right order. Right now this is mostly handled by mucking about with initcall ordering which is a complete hack, and doesn't even remotely handle the case where device drivers are in modules. This approach completely sidesteps the issues by allowing driver registration to occur in any order, and any driver can request to be retried after a few more other drivers get probed.
Why would drivers in modules be an issue? If a driver depends on another driver, making it a module dependancy would solve the problem, right?
Not nearly that simple. Consider the case where driver A depends on a gpio resource provided by driver B. Well, driver A could depend on driver B, but driver A has zero knowledge of what B actually is because B is different for each system.
So, since A cannot depend on B because B is dynamic, the problem cannot be solved with enforcing a module load order.
g.
Quoted 28 lines · Grant Likely
Quoted 5 lines · Greg KH
Quoted 2 lines · Grant Likely
Allow drivers to report at probe time that they cannot get all the resources required by the device, and should be retried at a later time.
When is "later"?
In this case, after at least one other device has successfully probed. The 'later' is handled in a workqueue that walks the list asynchronously from normal initialization.
Quoted 1 line
And why would a driver not be able to get all of the proper resources?
Discussed below...
Quoted 3 lines
Why can't a bus, at a later time, just try to reprobe everything when it determines that it is a "later" time now, without having to do this added change to the core?
It can't be done by a specific bus type because it has zero relationship with the bus type. For example, it is typical for an SDHCI driver to require a GPIO line for the card detect switch, and the device cannot be initialized until it has it. However, the
bus_typethat the SDHCI driver is attached to could be anything;platform_bus_type, pci, amba, etc. It isn't abus_typedeficiency, but rather that the driver core has no way to gracefully handle devices that get probed in an undetermined order.
Quoted 7 lines
It has to be done at the core level because any device in the system, regardless of
bus_type, may require another device to be probed first. Originally I tried modifying the drivers to successfully probe anyway and then 'go to sleep' to try again later, but it turned out to push a lot of complexity into the device drivers when it can be solved far more simply if the driver core has the ability to retry drivers that request it.
So the driver core is just going to sit and spin and continue to try to probe drivers for as long as it gets that error value returned? What is going to ever cause that loop to terminate? It seems a bit hacky to just keep looping over and over and hoping that sometime everything will all settle down so that we can go to sleep again.
It just doesn't feel right, there has to be some other way to handle stuff like this in a way that is known to terminate properly other than just guessing.
greg k-h
Quoted 5 lines · Greg KH
So the driver core is just going to sit and spin and continue to try to probe drivers for as long as it gets that error value returned? What is going to ever cause that loop to terminate? It seems a bit hacky to just keep looping over and over and hoping that sometime everything will all settle down so that we can go to sleep again.
Well, it only needs to try as long as there are still new devices succeeding to get probed. The order that I think this should happen in is:
* go through all initcalls, record any devices that are not yet ready
* retry all devices on the list as long as at least one of them has succeeded.
* when a new device gets matched from a module load, do that loop again
If I read the patch correctly, the workqueue would be scheduled every time a new device gets added, which retries the devices more often than necessary and can have significant boot time impact, and it also introduces more asynchronicity that may expose new bugs.
Maybe we can have a late_initcall that enables the automatic retry and probes everything once:
static bool deferred_probe;
static int __init deferred_probe_start(void)
{
deferred_probe = true;
mutex_lock(&deferred_probe_mutex);
if (!list_empty(&deferred_probe_list))
schedule_work(&deferred_probe_work);
mutex_unlock(&deferred_probe_mutex);
flush_work_sync(&deferred_probe_work);
}late_initcall(retry_devices);
Arnd
Quoted 7 lines · Arnd Bergmann
Quoted 5 lines · Greg KH
So the driver core is just going to sit and spin and continue to try to probe drivers for as long as it gets that error value returned? What is going to ever cause that loop to terminate? It seems a bit hacky to just keep looping over and over and hoping that sometime everything will all settle down so that we can go to sleep again.
Quoted 8 lines
Well, it only needs to try as long as there are still new devices succeeding to get probed. The order that I think this should happen in is:
* go through all initcalls, record any devices that are not yet ready
* retry all devices on the list as long as at least one of them has succeeded.
* when a new device gets matched from a module load, do that loop again
You don't know when init calls are finished, or if a module is loaded, the driver core isn't that smart at all.
Quoted 19 lines
If I read the patch correctly, the workqueue would be scheduled every time a new device gets added, which retries the devices more often than necessary and can have significant boot time impact, and it also introduces more asynchronicity that may expose new bugs.
Maybe we can have a
late_initcallthat enables the automatic retry and probes everything once:static bool
deferred_probe;
static int__initdeferred_probe_start(void){ deferred_probe = true; mutex_lock(&deferred_probe_mutex); if (!list_empty(&deferred_probe_list)) schedule_work(&deferred_probe_work); mutex_unlock(&deferred_probe_mutex); flush_work_sync(&deferred_probe_work); }
Quoted 1 line
late_initcall(retry_devices);
I wonder if doing this all from a workqueue in the first place is going to cause problems as probe isn't normally done this way at all.
greg k-h
Quoted 7 lines · Greg KH
Quoted 4 lines
* go through all initcalls, record any devices that are not yet ready
* retry all devices on the list as long as at least one of them has succeeded.
* when a new device gets matched from a module load, do that loop againYou don't know when init calls are finished, or if a module is loaded, the driver core isn't that smart at all.
We know when most initcalls are finished (at late_initcall time), and after that we don't need to know when a module gets loaded, only when a device gets matched, and that's something we do know and that Grant's patch uses already.
Quoted 4 lines
Quoted 1 line
late_initcall(retry_devices);I wonder if doing this all from a workqueue in the first place is going to cause problems as probe isn't normally done this way at all.
True. It will definitely cause problems for any driver that calls flush_work() in its probe function. We would need a private singlethread_workqueue to avoid that.
Arnd
Quoted 6 lines · Arnd Bergmann
Quoted 5 lines · Greg KH
Quoted 4 lines
* go through all initcalls, record any devices that are not yet ready
* retry all devices on the list as long as at least one of them has succeeded.
* when a new device gets matched from a module load, do that loop again
Quoted 7 lines
Quoted 2 lines
You don't know when init calls are finished, or if a module is loaded, the driver core isn't that smart at all.
We know when most initcalls are finished (at
late_initcalltime), and after that we don't need to know when a module gets loaded, only when a device gets matched, and that's something we do know and that Grant's patch uses already.
Yes, I'm thinking about doing exactly that.
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
late_initcall(retry_devices);I wonder if doing this all from a workqueue in the first place is going to cause problems as probe isn't normally done this way at all.
Problems with drivers or problems with the driver core? It shouldn't be a problem for drivers since only drivers that have explicitly requested deferral will end up with the device added to the deferral list.
Quoted 3 lines
True. It will definitely cause problems for any driver that calls
flush_work()in its probe function. We would need a privatesinglethread_workqueueto avoid that.
Ah, good point. That's easy enough to do.
g.
Signature · 2 lines
Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd.
Quoted 2 lines · Greg KH
I wonder if doing this all from a workqueue in the first place is going to cause problems as probe isn't normally done this way at all.
Yeah, I would expect unforeseen problems with the async thread too. It's probably all solvable, but it sounds troublesome to find out if things go wrong.
We have sync hooks (BUS_NOTIFY_*) where any kind of code can subscribe to when devices get added or get bound to a driver. Can't the code that relies on later hookups to already existing devices/bindings not just plug into that?
Thanks,
Kay
Quoted 5 lines · Kay Sievers
Quoted 2 lines · Greg KH
I wonder if doing this all from a workqueue in the first place is going to cause problems as probe isn't normally done this way at all.
Quoted 8 lines
Yeah, I would expect unforeseen problems with the async thread too. It's probably all solvable, but it sounds troublesome to find out if things go wrong.
We have sync hooks (BUS_NOTIFY_*) where any kind of code can subscribe to when devices get added or get bound to a driver. Can't the code that relies on later hookups to already existing devices/bindings not just plug into that?
I tried that. It resulted in a lot of complexity that each driver needs to implement correctly which is why I started looking for a different way to go about it.
g.
Quoted 10 lines · Grant Likely
Quoted 9 lines · Kay Sievers
Quoted 2 lines · Greg KH
I wonder if doing this all from a workqueue in the first place is going to cause problems as probe isn't normally done this way at all.
Yeah, I would expect unforeseen problems with the async thread too. It's probably all solvable, but it sounds troublesome to find out if things go wrong.
Quoted 8 lines
Quoted 4 lines
We have sync hooks (BUS_NOTIFY_*) where any kind of code can subscribe to when devices get added or get bound to a driver. Can't the code that relies on later hookups to already existing devices/bindings not just plug into that?
I tried that. It resulted in a lot of complexity that each driver needs to implement correctly which is why I started looking for a different way to go about it.
No, the bus that wants this just has to do it, not the drivers themselves, right?
Quoted 16 lines · Greg KH
Quoted 15 lines · Grant Likely
Quoted 13 lines · Kay Sievers
Quoted 2 lines · Greg KH
I wonder if doing this all from a workqueue in the first place is going to cause problems as probe isn't normally done this way at all.
Yeah, I would expect unforeseen problems with the async thread too. It's probably all solvable, but it sounds troublesome to find out if things go wrong.
We have sync hooks (BUS_NOTIFY_*) where any kind of code can subscribe to when devices get added or get bound to a driver. Can't the code that relies on later hookups to already existing devices/bindings not just plug into that?
Quoted 6 lines
Quoted 3 lines
I tried that. It resulted in a lot of complexity that each driver needs to implement correctly which is why I started looking for a different way to go about it.
No, the bus that wants this just has to do it, not the drivers themselves, right?
It's not about the bus_type, and there is nothing that the bus can do to solve this problem because it the dependencies are completely orthogonal to the bus. ie. what does an i2c bus know about the audio path to a codec? The problem must be solved at the driver scope.
g.
Quoted 21 lines · Grant Likely
Quoted 20 lines · Greg KH
Quoted 18 lines · Grant Likely
Quoted 5 lines · Kay Sievers
Quoted 2 lines · Greg KH
I wonder if doing this all from a workqueue in the first place is going to cause problems as probe isn't normally done this way at all.
Quoted 8 lines
Yeah, I would expect unforeseen problems with the async thread too. It's probably all solvable, but it sounds troublesome to find out if things go wrong.
We have sync hooks (BUS_NOTIFY_*) where any kind of code can subscribe to when devices get added or get bound to a driver. Can't the code that relies on later hookups to already existing devices/bindings not just plug into that?
I tried that. It resulted in a lot of complexity that each driver needs to implement correctly which is why I started looking for a different way to go about it.
Quoted 7 lines
Quoted 2 lines
No, the bus that wants this just has to do it, not the drivers themselves, right?
It's not about the
bus_type, and there is nothing that the bus can do to solve this problem because it the dependencies are completely orthogonal to the bus. ie. what does an i2c bus know about the audio path to a codec? The problem must be solved at the driver scope.
Ok, let's look at your next implementation and see how it goes.
Quoted 25 lines · Greg KH
Quoted 24 lines · Grant Likely
Quoted 22 lines · Greg KH
Quoted 10 lines · Grant Likely
Quoted 9 lines · Kay Sievers
Quoted 2 lines · Greg KH
I wonder if doing this all from a workqueue in the first place is going to cause problems as probe isn't normally done this way at all.
Yeah, I would expect unforeseen problems with the async thread too. It's probably all solvable, but it sounds troublesome to find out if things go wrong.
Quoted 8 lines
Quoted 4 lines
We have sync hooks (BUS_NOTIFY_*) where any kind of code can subscribe to when devices get added or get bound to a driver. Can't the code that relies on later hookups to already existing devices/bindings not just plug into that?
I tried that. It resulted in a lot of complexity that each driver needs to implement correctly which is why I started looking for a different way to go about it.
No, the bus that wants this just has to do it, not the drivers themselves, right?
Quoted 6 lines
Quoted 4 lines
It's not about the
bus_type, and there is nothing that the bus can do to solve this problem because it the dependencies are completely orthogonal to the bus. ie. what does an i2c bus know about the audio path to a codec? The problem must be solved at the driver scope.Ok, let's look at your next implementation and see how it goes.
Okay, thanks. I'll get it posted soon.
g.
Quoted 10 lines · Grant Likely
Quoted 9 lines · Kay Sievers
Quoted 2 lines · Greg KH
I wonder if doing this all from a workqueue in the first place is going to cause problems as probe isn't normally done this way at all.
Yeah, I would expect unforeseen problems with the async thread too. It's probably all solvable, but it sounds troublesome to find out if things go wrong.
Quoted 8 lines
Quoted 4 lines
We have sync hooks (BUS_NOTIFY_*) where any kind of code can subscribe to when devices get added or get bound to a driver. Can't the code that relies on later hookups to already existing devices/bindings not just plug into that?
I tried that. It resulted in a lot of complexity that each driver needs to implement correctly which is why I started looking for a different way to go about it.
What I mean is that we might not necessarily need to have that in 'struct device' and the driver-binding code. Can't we just allocate the reprobe-in-the-back logic in its own object, let this object hook into the existing notifier chain (if needed add more notifiers) and have your reprobe object itself check for the device, carry its own list of devices and trigger the needed work?
(As a general note, not specifically related to this patch: The current 'struct device' is a total mess already, and really needs to be cleaned up. The list.h way of doing lists might perform nice in some always-be-a-list-member used cases, but it is really not the appropriate way of doing non-common functionality on common objects:
performance does not matter at all here, and the object-to-add itself has to carry the list-data needed for every non-common list.)
Kay
Quoted 8 lines · Kay Sievers
Quoted 2 lines · Greg KH
I wonder if doing this all from a workqueue in the first place is going to cause problems as probe isn't normally done this way at all.
Yeah, I would expect unforeseen problems with the async thread too. It's probably all solvable, but it sounds troublesome to find out if things go wrong.
I think it is worth solving though since the feature is important. I can certainly block out the deferral code with a Kconfig symbol so that it only gets built when a driver selects CONFIG_PROBE_DEFERRAL. That would keep the code out of x86 kernels while us embedded folks iron out the bugs.
g.
Quoted 37 lines · Greg KH
Quoted 28 lines · Grant Likely
Quoted 5 lines · Greg KH
Quoted 2 lines · Grant Likely
Allow drivers to report at probe time that they cannot get all the resources required by the device, and should be retried at a later time.
When is "later"?
In this case, after at least one other device has successfully probed. The 'later' is handled in a workqueue that walks the list asynchronously from normal initialization.
Quoted 1 line
And why would a driver not be able to get all of the proper resources?
Discussed below...
Quoted 3 lines
Why can't a bus, at a later time, just try to reprobe everything when it determines that it is a "later" time now, without having to do this added change to the core?
It can't be done by a specific bus type because it has zero relationship with the bus type. For example, it is typical for an SDHCI driver to require a GPIO line for the card detect switch, and the device cannot be initialized until it has it. However, the
bus_typethat the SDHCI driver is attached to could be anything;platform_bus_type, pci, amba, etc. It isn't abus_typedeficiency, but rather that the driver core has no way to gracefully handle devices that get probed in an undetermined order.Quoted 7 lines
It has to be done at the core level because any device in the system, regardless of
bus_type, may require another device to be probed first. Originally I tried modifying the drivers to successfully probe anyway and then 'go to sleep' to try again later, but it turned out to push a lot of complexity into the device drivers when it can be solved far more simply if the driver core has the ability to retry drivers that request it.
Quoted 5 lines
So the driver core is just going to sit and spin and continue to try to probe drivers for as long as it gets that error value returned? What is going to ever cause that loop to terminate? It seems a bit hacky to just keep looping over and over and hoping that sometime everything will all settle down so that we can go to sleep again.
No, that would be insane. The list of deferred devices is only walked when triggered, and it stops at the end of the list unless retriggered again. Also, for the drivers using this, reattempting probe is cheap since the driver is expected to test resources first before initializing hardware. If it can't get what it needs, it returns -EAGAIN immediately. We /could/ implement dependency checking in the core code instead of calling each driver's probe, but that would mean teaching the driver core about every possible resource a driver might want which would be absolutely horrible to code, would probably be more expensive, and definitely more complex.
in this version of the patch there is no way for a driver to be dropped from the deferred list and the list walk is quite inefficient.
The next version will solve both problems by removing each device
from the list when the list is walked. If a driver requests deferral again, then the device gets re-added to the end.
Another problem with this version is that every successful probe will trigger walking the list because that was the obvious way to implement it. So, it is theoretically possible that the list will get walked once every single time a driver is bound (but probably not in practice because the list walk is handled in a workqueue that is naturally serialized). I'm looking at changing how the workqueue is scheduled so that it only gets kicked at the end of a block of driver probing (like at the end of syscalls), but that is more of an optimization than anything.
g.
Quoted 7 lines
It just doesn't feel right, there has to be some other way to handle stuff like this in a way that is known to terminate properly other than just guessing.
greg k-h
Signature · 2 lines
Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd.
Quoted 5 lines · On Mon, 04 Jul 2011 11:11:59 -0600 Grant Likely wrote:
drivers/base/dd.c +0
-0
drivers/base/dd.c line 36
Quoted 9 lines
drivers/base/dd.c +6
-0@@ -28,6 +28,62 @@#include "base.h" #include "power/power.h" +/** +* deferred_probe_work_func() - Retry probing devices in the deferred list. +*/ +static DEFINE_MUTEX (deferred_probe_mutex); +static LIST_HEAD (deferred_probe_list); +static void deferred_probe_work_func (struct work_struct *work)
The kernel-doc notation needs to be immediately before the function, without the intervening data...
Quoted 7 lines
drivers/base/dd.c +7
-0+{ +struct device *dev; +/* +* This bit is tricky. We want to process every device in the +* deferred list, but devices can be removed from the list at any +* time while inside this for-each loop. There are two things that +* need to be protected against:
drivers/base/dd.c line 53
Quoted 10 lines
drivers/base/dd.c +10
-0+* - if the device is removed from the deferred_probe_list, then we +* loose our place in the loop. Since any device can be removed +* asynchronously, list_for_each_entry_safe() wouldn't make things +* much better. Simplest solution is to restart walking the list +* whenever the current device gets removed. Not the most efficient, +* but is simple to implement and easy to audit for correctness. +* - if the device is unregistered, and freed, then there is a risk +* of a null pointer dereference. This code uses get/put_device() +* to ensure the device cannot disappear from under our feet. +*/
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
Quoted 5 lines · Grant Likely
Mark, I'm particularly interested in your thoughts on this approach. It is decidedly "low-tech" in its approach to handling device dependencies, but it has the advantage of being simple and should handle a wide range of use-cases reliably. Would this work for ALSA SoC probing?
It's essentially what we're doing currently for the part of the system where we decide that everything is registered and we should run the actual probe functions so it'll help with that. Having a clock API we can actually use off-SoC will help with a lot of the remaining stuff.
I *think* we'll still going to need to have the infrastructure to deal with running all the probes together, at least for a while, as the current code really assumes that it's got some of the card wide stuff around when all the devices get instantiated but I think if we were starting from fresh this would be fairly good. The only thing I can think might be an issue is n way dependencies, but those mostly shake out as being a dependency of the overall card on subdevices. I'd need to separate out the implementation issues from the assumptions to be 100% clear if that was the case, though.
Quoted 8 lines · Mark Brown
Quoted 5 lines · Grant Likely
Mark, I'm particularly interested in your thoughts on this approach. It is decidedly "low-tech" in its approach to handling device dependencies, but it has the advantage of being simple and should handle a wide range of use-cases reliably. Would this work for ALSA SoC probing?
Quoted 4 lines
It's essentially what we're doing currently for the part of the system where we decide that everything is registered and we should run the actual probe functions so it'll help with that. Having a clock API we can actually use off-SoC will help with a lot of the remaining stuff.
Is that the bit that snd_soc_instantiate_cards() is doing? If I made snd_soc_register_card() attempt to instantiate immediately and return -EAGAIN if resources were not available, it looks like I should be able to remove the card_list entirely. Would that be the right way to go about it?
Quoted 5 lines
I *think* we'll still going to need to have the infrastructure to deal with running all the probes together, at least for a while, as the current code really assumes that it's got some of the card wide stuff around when all the devices get instantiated but I think if we were starting from fresh this would be fairly good.
I'm not sure what you're getting at here. Are you talking about the infrastructure to keep track of codecs, DAIs and the like? Yes, that infrastructure is still needed because the drivers need an api for each of the kinds of resources it needs.
Quoted 5 lines
The only thing I can
think might be an issue is n way dependencies, but those mostly shake out as being a dependency of the overall card on subdevices. I'd need to separate out the implementation issues from the assumptions to be 100% clear if that was the case, though.
Do you have an example? I don't see any problems with complex dependencies providing that none of them are circular. If, say, device A depends on B and C, and B also depends on C, then it may take a couple of cycles before everything gets probed, but it all still works correctly.
g.
Quoted 11 lines · Grant Likely
Quoted 3 lines · Mark Brown
where we decide that everything is registered and we should run the actual probe functions so it'll help with that. Having a clock API we can actually use off-SoC will help with a lot of the remaining stuff.
Is that the bit that
snd_soc_instantiate_cards()is doing? If I madesnd_soc_register_card()attempt to instantiate immediately and return-EAGAINif resources were not available, it looks like I should be able to remove thecard_listentirely. Would that be the right way to go about it?
That's exactly what I'd expect to do, yes.
Quoted 10 lines
Quoted 5 lines
I *think* we'll still going to need to have the infrastructure to deal with running all the probes together, at least for a while, as the current code really assumes that it's got some of the card wide stuff around when all the devices get instantiated but I think if we were starting from fresh this would be fairly good.
I'm not sure what you're getting at here. Are you talking about the infrastructure to keep track of codecs, DAIs and the like? Yes, that infrastructure is still needed because the drivers need an api for each of the kinds of resources it needs.
Sort of, but I'm more thinking of the fact that we currently defer all the "real" probes until the full card is ready - ideally these could all be done as part of the device model probe, but this is something we can improve over time rather than a critical thing. The regmap API will help here.
Quoted 11 lines
Quoted 5 lines
The only thing I can
think might be an issue is n way dependencies, but those mostly shake out as being a dependency of the overall card on subdevices. I'd need to separate out the implementation issues from the assumptions to be 100% clear if that was the case, though.Do you have an example? I don't see any problems with complex dependencies providing that none of them are circular. If, say, device A depends on B and C, and B also depends on C, then it may take a couple of cycles before everything gets probed, but it all still works correctly.
It's circular dependencies that worry me. I don't have any concrete examples in mainline now but I'm aware of systems that are more complicated than mainline. Things like clock trees that go out of one device, into another, and back into to the first. Like I say I think these all shake out as simple dependencies of component devices on the card so it's probably not a problem.