| Message ID | 1460540160-18762-2-git-send-email-m.szyprowski@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 13, 2016 at 11:35:59AM +0200, Marek Szyprowski wrote: > This patch adds code which allow other subsystems get a notification > when deferred probe has been triggered. This way one can retry some > actions, which earlier failed with -EPROBE_DEFER error code. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Why would some other subsystem want/care about this? You aren't telling them what device was deferred, and you don't need to as the bus itself already knows this information as it did the deferring! confused, greg k-h
Hello, On 2016-04-13 16:12, Greg Kroah-Hartman wrote: > On Wed, Apr 13, 2016 at 11:35:59AM +0200, Marek Szyprowski wrote: >> This patch adds code which allow other subsystems get a notification >> when deferred probe has been triggered. This way one can retry some >> actions, which earlier failed with -EPROBE_DEFER error code. >> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > Why would some other subsystem want/care about this? You aren't telling > them what device was deferred, and you don't need to as the bus itself > already knows this information as it did the deferring! > > confused, This notifier is just to let others that the deferred probe has happened and it is a good time to retry operation, which earlier failed due to missing resources (i.e. power domains, clocks). Such case is with registering AMBA device (not the driver!). During AMBA device registration, bus code has to read some device's registers to get its device CID/PID. To do this, device's clocks and power domain has to be turned on. Those however might not be available that time. With this notifier, AMBA bus code is able to retry device registration, which earlier failed due to missing clocks or power domain. This CID/PID reading has to be done during device registration time because of the already deployed userspace ABI. CID/PID values are reported to userspace, which might rely on them to load proper driver modules. Best regards
On Thu, Apr 14, 2016 at 09:36:38AM +0200, Marek Szyprowski wrote: > This CID/PID reading has to be done during device registration time because > of the already deployed userspace ABI. CID/PID values are reported to > userspace, which might rely on them to load proper driver modules. There is no "might" about it - the CID/PID are the way drivers get matched, and it's what is in the module device tables exported for modprobe/udev to use.
On Thu, Apr 14, 2016 at 09:36:38AM +0200, Marek Szyprowski wrote: > Hello, > > On 2016-04-13 16:12, Greg Kroah-Hartman wrote: > > On Wed, Apr 13, 2016 at 11:35:59AM +0200, Marek Szyprowski wrote: > > > This patch adds code which allow other subsystems get a notification > > > when deferred probe has been triggered. This way one can retry some > > > actions, which earlier failed with -EPROBE_DEFER error code. > > > > > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > > Why would some other subsystem want/care about this? You aren't telling > > them what device was deferred, and you don't need to as the bus itself > > already knows this information as it did the deferring! > > > > confused, > > This notifier is just to let others that the deferred probe has happened and > it is a good time to retry operation, which earlier failed due to missing > resources (i.e. power domains, clocks). Such case is with registering AMBA > device (not the driver!). During AMBA device registration, bus code has to > read > some device's registers to get its device CID/PID. To do this, device's > clocks > and power domain has to be turned on. Those however might not be available > that time. With this notifier, AMBA bus code is able to retry device > registration, which earlier failed due to missing clocks or power domain. Ick, no, notifiers are horrid, all you are getting is that "someone" deferred, which makes no sense. You know, in your bus, when you deferr a driver probe. So do the work then. Don't rely on a random "signal" from a random device at a random point in time to potentially do some sort of work. That's looney. > This CID/PID reading has to be done during device registration time because > of the already deployed userspace ABI. CID/PID values are reported to > userspace, which might rely on them to load proper driver modules. As Russell said, it is "must", not "might", and has been that way for over a decade, this isn't anything new. thanks, greg k-h
On Thu, Apr 14, 2016 at 02:17:45PM -0700, Greg Kroah-Hartman wrote: > On Thu, Apr 14, 2016 at 09:36:38AM +0200, Marek Szyprowski wrote: > > Hello, > > > > On 2016-04-13 16:12, Greg Kroah-Hartman wrote: > > > On Wed, Apr 13, 2016 at 11:35:59AM +0200, Marek Szyprowski wrote: > > > > This patch adds code which allow other subsystems get a notification > > > > when deferred probe has been triggered. This way one can retry some > > > > actions, which earlier failed with -EPROBE_DEFER error code. > > > > > > > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > > > Why would some other subsystem want/care about this? You aren't telling > > > them what device was deferred, and you don't need to as the bus itself > > > already knows this information as it did the deferring! > > > > > > confused, > > > > This notifier is just to let others that the deferred probe has happened and > > it is a good time to retry operation, which earlier failed due to missing > > resources (i.e. power domains, clocks). Such case is with registering AMBA > > device (not the driver!). During AMBA device registration, bus code has to > > read > > some device's registers to get its device CID/PID. To do this, device's > > clocks > > and power domain has to be turned on. Those however might not be available > > that time. With this notifier, AMBA bus code is able to retry device > > registration, which earlier failed due to missing clocks or power domain. > > Ick, no, notifiers are horrid, all you are getting is that "someone" > deferred, which makes no sense. > > You know, in your bus, when you deferr a driver probe. So do the work > then. You're not understanding the problem. The problem is not at driver probe time, the problem is at device instantiation time, which is way earlier. We need to power up the device and enable clocks in order to read the device's vendor and part number, which is what identifies it to the driver. This is also used by userspace to work out which driver module to load. So, we need this information to be present when the device is registered. It's just like the PCI vendor and device IDs - in PCI, these must be readable when the device is instantiated. The problem that's being addressed here is that there's no way at the moment to know when the drivers on a different bus (namely the platform bus) have probed and are providing the clock and power domain resources necessary to be able to read these identifying values. I guess if you don't like a notifier, the other alternative is to setup a delayed workqueue and have the workqueue repeatedly attempt to register the devices until they all succeed. That's not particularly nice, because we'd be wasting CPU cycles running that workqueue for no reason until all the devices get registered.
On Thu, Apr 14, 2016 at 10:33:22PM +0100, Russell King - ARM Linux wrote: > The problem that's being addressed here is that there's no way at the > moment to know when the drivers on a different bus (namely the platform > bus) have probed and are providing the clock and power domain resources > necessary to be able to read these identifying values. > > I guess if you don't like a notifier, the other alternative is to > setup a delayed workqueue and have the workqueue repeatedly attempt > to register the devices until they all succeed. That's not > particularly nice, because we'd be wasting CPU cycles running > that workqueue for no reason until all the devices get registered. I like that idea, it handles the issue of this crazy hardware where it belongs, in that bus subsystem :) thanks, greg k-h
diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 16688f50729c..208466eb83fb 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -19,6 +19,7 @@ #include <linux/device.h> #include <linux/delay.h> +#include <linux/notifier.h> #include <linux/module.h> #include <linux/kthread.h> #include <linux/wait.h> @@ -53,6 +54,7 @@ static LIST_HEAD(deferred_probe_pending_list); static LIST_HEAD(deferred_probe_active_list); static struct workqueue_struct *deferred_wq; static atomic_t deferred_trigger_count = ATOMIC_INIT(0); +static BLOCKING_NOTIFIER_HEAD(deferred_notifier_head); /* * In some cases, like suspend to RAM or hibernation, It might be reasonable @@ -113,6 +115,9 @@ static void deferred_probe_work_func(struct work_struct *work) put_device(dev); } mutex_unlock(&deferred_probe_mutex); + + /* Notify any listeners about triggering deferred probe */ + blocking_notifier_call_chain(&deferred_notifier_head, 0, NULL); } static DECLARE_WORK(deferred_probe_work, deferred_probe_work_func); @@ -224,6 +229,32 @@ static int deferred_probe_initcall(void) late_initcall(deferred_probe_initcall); /** + * deferred_probe_register_notifier - Register a notifier for deferred probe + * @nb: notifier block to signal + * + * This function allows caller to get a notification when deferred probe has + * been triggered. This lets it to retry some actions, which earlier failed + * with -EPROBE_DEFER error code. + */ +int deferred_probe_register_notifier(struct notifier_block *nb) +{ + return blocking_notifier_chain_register(&deferred_notifier_head, nb); +} +EXPORT_SYMBOL_GPL(deferred_probe_register_notifier); + +/** + * deferred_probe_unregister_notifier - Unregister a notifier + * @nb: notifier block to signal + * + * Unregister a previously registered deferred probe notifier. + */ +int deferred_probe_unregister_notifier(struct notifier_block *nb) +{ + return blocking_notifier_chain_unregister(&deferred_notifier_head, nb); +} +EXPORT_SYMBOL_GPL(deferred_probe_unregister_notifier); + +/** * device_is_bound() - Check if device is bound to a driver * @dev: device to check * diff --git a/include/linux/device.h b/include/linux/device.h index 002c59728dbe..c6496d1db5de 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -1066,6 +1066,9 @@ extern int __must_check device_reprobe(struct device *dev); extern bool device_is_bound(struct device *dev); +extern int deferred_probe_register_notifier(struct notifier_block *nb); +extern int deferred_probe_unregister_notifier(struct notifier_block *nb); + /* * Easy functions for dynamically creating devices on the fly */
This patch adds code which allow other subsystems get a notification when deferred probe has been triggered. This way one can retry some actions, which earlier failed with -EPROBE_DEFER error code. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- drivers/base/dd.c | 31 +++++++++++++++++++++++++++++++ include/linux/device.h | 3 +++ 2 files changed, 34 insertions(+)