(Old thread name: Exynos4210: fix power domain for MDMA1 device)
This patchset solves the problem of registering AMBA device, which might
be a part of power domain. In some cases not all resources needed for
device registration might be available, because their provides are not
yet probed. The caller for amba_device_add() don't support -EPROBE_DEFER
return code, so in such case, success is returned and device is added to
special deferred list. Registration of devices from that list is
performed everytime when deferred probe action has been performed until
it finally succeeds.
This patchset originates from the patches for fixing mysterious boot
hang on Exynos 4210 SoCs when IOMMU was enabled. There is no direct
dependency between IOMMU devices and AMBA devices, however enabling
IOMMU changes the device probe order, what results in turning off some
power domains during boot. Then the registration of MDMA1 (PL330 AMBA)
device happens, what results in system hangs, because the common bus
code tries to read PID/CID registers from turned-off device.
Best regards
Marek Szyprowski
Samsung R&D Institute Poland
Changelog:
v7:
- replaced late_initcall approach with a notifier registered to device core
v6: https://lkml.org/lkml/2016/4/12/414
- got back to v1-style approach on Russell King request to avoid ABI break
- use list for storing deferred devices and retry their registration from
late_initcall
v5: https://lkml.org/lkml/2016/2/10/179
- added 2 more patches to avoid regression with existing drivers (nvdimm and
sa1111), for more information, see https://lkml.org/lkml/2015/12/17/390
- changed thread name to "AMBA: add complete support for power domains"
v4: https://lkml.org/lkml/2015/12/2/52
- fixed more issues pointed by Ulf Hansson and Russell King
v3: https://lkml.org/lkml/2015/12/1/334
- fixed issues pointed by Ulf Hansson
- dropped patch for exynos4210 dts, because it already got queued for merging
v2: https://lkml.org/lkml/2015/11/26/229
- added 2 patches from 'On-demand device probing' thread
(https://lkml.org/lkml/2015/9/29/189), which move PID/CIR reading
from amba_device_add() to amba_match()
- moved dev_pm_domain_attach() to amba_match(), which is allowed to
return -EPROBE_DEFER
v1: http://www.spinics.net/lists/arm-kernel/msg463185.html
- initial version
Patch summary:
Marek Szyprowski (2):
drivers: base: add support for registering notifier about deferred
probe
drivers: amba: properly handle devices with power domains
drivers/amba/bus.c | 106 ++++++++++++++++++++++++++++++++++++++++++++-----
drivers/base/dd.c | 31 +++++++++++++++
include/linux/device.h | 3 ++
3 files changed, 130 insertions(+), 10 deletions(-)
--
1.9.2
To read pid/cid registers, the probed device need to be properly turned on.
When it is inside a power domain, the bus code should ensure that the
given power domain is enabled before trying to access device's registers.
However in some cases power domain (or clocks) might not be yet available.
Returning EPROBE_DEFER is not a solution in such case, because callers
don't handle this special error code. Instead such devices are added to the
special list and their registration is retried in late_initcall, when
hopefully all resources are available.
Signed-off-by: Marek Szyprowski <[email protected]>
---
drivers/amba/bus.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 96 insertions(+), 10 deletions(-)
diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index f0099360039e..3329262007f9 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -336,16 +336,7 @@ static void amba_device_release(struct device *dev)
kfree(d);
}
-/**
- * amba_device_add - add a previously allocated AMBA device structure
- * @dev: AMBA device allocated by amba_device_alloc
- * @parent: resource parent for this devices resources
- *
- * Claim the resource, and read the device cell ID if not already
- * initialized. Register the AMBA device with the Linux device
- * manager.
- */
-int amba_device_add(struct amba_device *dev, struct resource *parent)
+static int amba_device_try_add(struct amba_device *dev, struct resource *parent)
{
u32 size;
void __iomem *tmp;
@@ -373,6 +364,12 @@ int amba_device_add(struct amba_device *dev, struct resource *parent)
goto err_release;
}
+ ret = dev_pm_domain_attach(&dev->dev, true);
+ if (ret == -EPROBE_DEFER) {
+ iounmap(tmp);
+ goto err_release;
+ }
+
ret = amba_get_enable_pclk(dev);
if (ret == 0) {
u32 pid, cid;
@@ -398,6 +395,7 @@ int amba_device_add(struct amba_device *dev, struct resource *parent)
}
iounmap(tmp);
+ dev_pm_domain_detach(&dev->dev, true);
if (ret)
goto err_release;
@@ -421,6 +419,94 @@ int amba_device_add(struct amba_device *dev, struct resource *parent)
err_out:
return ret;
}
+
+/*
+ * Registration of AMBA device require reading its pid and cid registers.
+ * To do this, the device must be turned on (if it is a part of power domain)
+ * and have clocks enabled. However in some cases those resources might not be
+ * yet available. Returning EPROBE_DEFER is not a solution in such case,
+ * because callers don't handle this special error code. Instead such devices
+ * are added to the special list and their registration is retried from
+ * deferred probe notifier, when hopefully all resources are available.
+ */
+struct deferred_device {
+ struct amba_device *dev;
+ struct resource *parent;
+ struct list_head node;
+};
+
+static LIST_HEAD(deferred_devices);
+static DEFINE_MUTEX(deferred_devices_lock);
+
+static int amba_deferred_probe_notifier(struct notifier_block *nb,
+ unsigned long action, void *data);
+static struct notifier_block amba_deferred_probe_nb = {
+ .notifier_call = amba_deferred_probe_notifier,
+};
+
+static int amba_deferred_probe_notifier(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct deferred_device *ddev, *tmp;
+
+ mutex_lock(&deferred_devices_lock);
+
+ list_for_each_entry_safe(ddev, tmp, &deferred_devices, node) {
+ int ret = amba_device_try_add(ddev->dev, ddev->parent);
+
+ if (ret == -EPROBE_DEFER)
+ continue;
+
+ list_del_init(&ddev->node);
+ kfree(ddev);
+ }
+
+ if (list_empty(&deferred_devices))
+ deferred_probe_unregister_notifier(&amba_deferred_probe_nb);
+
+ mutex_unlock(&deferred_devices_lock);
+
+ return NOTIFY_DONE;
+}
+
+/**
+ * amba_device_add - add a previously allocated AMBA device structure
+ * @dev: AMBA device allocated by amba_device_alloc
+ * @parent: resource parent for this devices resources
+ *
+ * Claim the resource, and read the device cell ID if not already
+ * initialized. Register the AMBA device with the Linux device
+ * manager.
+ */
+int amba_device_add(struct amba_device *dev, struct resource *parent)
+{
+ int ret = amba_device_try_add(dev, parent);
+
+ if (ret == -EPROBE_DEFER) {
+ struct deferred_device *ddev;
+
+ ddev = kmalloc(sizeof(*ddev), GFP_KERNEL);
+ if (!ddev)
+ return -ENOMEM;
+
+ ddev->dev = dev;
+ ddev->parent = parent;
+ ret = 0;
+
+ mutex_lock(&deferred_devices_lock);
+
+ if (list_empty(&deferred_devices))
+ ret = deferred_probe_register_notifier(
+ &amba_deferred_probe_nb);
+ if (ret == 0)
+ list_add_tail(&ddev->node, &deferred_devices);
+ else
+ kfree(ddev);
+
+ mutex_unlock(&deferred_devices_lock);
+ }
+ return ret;
+}
EXPORT_SYMBOL_GPL(amba_device_add);
static struct amba_device *
--
1.9.2
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 <[email protected]>
---
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
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 <[email protected]>
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 <[email protected]>
> 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
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.
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 <[email protected]>
> > 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 <[email protected]>
> > > 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.
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