Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1377688 > unrolled thread

[PATCH v7 1/2] drivers: base: add support for registering notifier about deferred probe

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v7 1/2] drivers: base: add support for registering notifier  about deferred probe Marek Szyprowski <m.szyprowski@samsung.com> - 2016-04-13 11:40 +0200
    Re: [PATCH v7 1/2] drivers: base: add support for registering  notifier about deferred probe Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-04-13 16:20 +0200
      Re: [PATCH v7 1/2] drivers: base: add support for registering notifier  about deferred probe Marek Szyprowski <m.szyprowski@samsung.com> - 2016-04-14 09:40 +0200
        Re: [PATCH v7 1/2] drivers: base: add support for registering  notifier about deferred probe Russell King - ARM Linux <linux@arm.linux.org.uk> - 2016-04-14 10:50 +0200
        Re: [PATCH v7 1/2] drivers: base: add support for registering  notifier about deferred probe Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-04-14 23:20 +0200
          Re: [PATCH v7 1/2] drivers: base: add support for registering  notifier about deferred probe Russell King - ARM Linux <linux@arm.linux.org.uk> - 2016-04-14 23:40 +0200
            Re: [PATCH v7 1/2] drivers: base: add support for registering  notifier about deferred probe Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-04-15 00:00 +0200

#1377688 — [PATCH v7 1/2] drivers: base: add support for registering notifier about deferred probe

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(+)

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
  */
-- 
1.9.2

[toc] | [next] | [standalone]

#1377982 — Re: [PATCH v7 1/2] drivers: base: add support for registering notifier about deferred probe

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

[toc] | [prev] | [next] | [standalone]

#1378548

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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

[toc] | [prev] | [next] | [standalone]

#1378608 — Re: [PATCH v7 1/2] drivers: base: add support for registering notifier about deferred probe

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.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1379309 — Re: [PATCH v7 1/2] drivers: base: add support for registering notifier about deferred probe

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

[toc] | [prev] | [next] | [standalone]

#1379313 — Re: [PATCH v7 1/2] drivers: base: add support for registering notifier about deferred probe

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.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1379320 — Re: [PATCH v7 1/2] drivers: base: add support for registering notifier about deferred probe

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web