[PATCH v1] RFC: amba: Remove amba specific deferred probe handling

From: Saravana Kannan
Date: Wed Mar 03 2021 - 09:33:16 EST


The addition/probe of amba devices has its own weird deferred probe
mechanism that needs to be maintained separately. It doesn't
automatically get any bugs fixes or improvements to the common deferred
probe mechanism.

It also has an arbitrary 5 second periodic attempt. So, even if the
resources are available, there can be an arbitrary delay before amba
devices are probed.

This patch used a proxy/stub device so that amba devices can hook into
the common deferred probe mechanism. This also means amba devices get
probed as soon as their resources are available.

Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
Cc: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
Cc: John Stultz <john.stultz@xxxxxxxxxx>
Cc: Saravana Kannan <saravanak@xxxxxxxxxx>
Cc: Sudeep Holla <sudeep.holla@xxxxxxx>
Cc: Nicolas Saenz Julienne <nsaenzjulienne@xxxxxxx>
Cc: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
Cc: Russell King <linux@xxxxxxxxxxxxxxx>
Cc: Rob Herring <robh@xxxxxxxxxx>
Signed-off-by: Saravana Kannan <saravanak@xxxxxxxxxx>
---

We talked about this almost a year ago[1] and it has been nagging me all
this time. So, finally got around to giving it a shot. This actually
seems to work -- I tested it on a device that was lying around.

Thoughts?

[1] - https://lore.kernel.org/linux-arm-kernel/CAGETcx8Cn-b6L2y10LKb91S3n06b6+Be2z_A0402EyNy-8yECg@xxxxxxxxxxxxxx/
-Saravana

drivers/amba/bus.c | 116 ++++++++++++++++++---------------------
include/linux/amba/bus.h | 1 +
2 files changed, 53 insertions(+), 64 deletions(-)

diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 939ca220bf78..393d189b6bca 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -24,6 +24,9 @@

#define to_amba_driver(d) container_of(d, struct amba_driver, drv)

+static int amba_proxy_probe(struct amba_device *adev,
+ const struct amba_id *id);
+
/* called on periphid match and class 0x9 coresight device. */
static int
amba_cs_uci_id_match(const struct amba_id *table, struct amba_device *dev)
@@ -46,6 +49,8 @@ amba_cs_uci_id_match(const struct amba_id *table, struct amba_device *dev)
static const struct amba_id *
amba_lookup(const struct amba_id *table, struct amba_device *dev)
{
+ if (!table)
+ return NULL;
while (table->mask) {
if (((dev->periphid & table->mask) == table->id) &&
((dev->cid != CORESIGHT_CID) ||
@@ -185,6 +190,9 @@ static int amba_probe(struct device *dev)
const struct amba_id *id = amba_lookup(pcdrv->id_table, pcdev);
int ret;

+ if (!pcdev->periphid)
+ return pcdrv->probe(pcdev, 0);
+
do {
ret = of_clk_set_defaults(dev->of_node, false);
if (ret < 0)
@@ -224,6 +232,9 @@ static int amba_remove(struct device *dev)
struct amba_device *pcdev = to_amba_device(dev);
struct amba_driver *drv = to_amba_driver(dev->driver);

+ if (!pcdev->periphid)
+ return 0;
+
pm_runtime_get_sync(dev);
if (drv->remove)
drv->remove(pcdev);
@@ -325,9 +336,20 @@ struct bus_type amba_bustype = {
};
EXPORT_SYMBOL_GPL(amba_bustype);

+static struct amba_driver amba_proxy_drv = {
+ .drv = {
+ .name = "amba-proxy",
+ },
+ .probe = amba_proxy_probe,
+};
+
static int __init amba_init(void)
{
- return bus_register(&amba_bustype);
+ int ret = bus_register(&amba_bustype);
+
+ if (ret)
+ return ret;
+ return amba_driver_register(&amba_proxy_drv);
}

postcore_initcall(amba_init);
@@ -490,58 +512,19 @@ static int amba_device_try_add(struct amba_device *dev, struct resource *parent)
goto err_release;
}

-/*
- * 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
- * periodic worker, until all resources are available and registration succeeds.
- */
-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 void amba_deferred_retry_func(struct work_struct *dummy);
-static DECLARE_DELAYED_WORK(deferred_retry_work, amba_deferred_retry_func);
-
-#define DEFERRED_DEVICE_TIMEOUT (msecs_to_jiffies(5 * 1000))
-
-static int amba_deferred_retry(void)
+static int amba_proxy_probe(struct amba_device *adev,
+ const struct amba_id *id)
{
- 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);
- }
-
- mutex_unlock(&deferred_devices_lock);
+ int ret;

- return 0;
-}
-late_initcall(amba_deferred_retry);
+ if (!adev->other_dev)
+ return -ENODEV;

-static void amba_deferred_retry_func(struct work_struct *dummy)
-{
- amba_deferred_retry();
+ ret = amba_device_try_add(adev->other_dev, adev->dev.platform_data);
+ if (ret != -EPROBE_DEFER)
+ adev->other_dev = NULL;

- if (!list_empty(&deferred_devices))
- schedule_delayed_work(&deferred_retry_work,
- DEFERRED_DEVICE_TIMEOUT);
+ return ret;
}

/**
@@ -557,25 +540,30 @@ int amba_device_add(struct amba_device *dev, struct resource *parent)
{
int ret = amba_device_try_add(dev, parent);

+ /*
+ * 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, create a dummy proxy device that'll
+ * keep deferring probe until all the resources are available and then
+ * add the real device.
+ */
if (ret == -EPROBE_DEFER) {
- struct deferred_device *ddev;
+ struct amba_device *proxy_dev;

- ddev = kmalloc(sizeof(*ddev), GFP_KERNEL);
- if (!ddev)
+ proxy_dev = amba_device_alloc(NULL, 0, 0);
+ if (!proxy_dev)
return -ENOMEM;

- ddev->dev = dev;
- ddev->parent = parent;
- ret = 0;
-
- mutex_lock(&deferred_devices_lock);
-
- if (list_empty(&deferred_devices))
- schedule_delayed_work(&deferred_retry_work,
- DEFERRED_DEVICE_TIMEOUT);
- list_add_tail(&ddev->node, &deferred_devices);
-
- mutex_unlock(&deferred_devices_lock);
+ proxy_dev->other_dev = dev;
+ proxy_dev->dev.platform_data = parent;
+ proxy_dev->driver_override = "amba-proxy";
+ dev_set_name(&proxy_dev->dev, "proxy:%s", dev_name(&dev->dev));
+ ret = device_add(&proxy_dev->dev);
+ if (ret)
+ put_device(&proxy_dev->dev);
}
return ret;
}
diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
index 6cc93ab5b809..432b32cf8fcc 100644
--- a/include/linux/amba/bus.h
+++ b/include/linux/amba/bus.h
@@ -71,6 +71,7 @@ struct amba_device {
struct amba_cs_uci_id uci;
unsigned int irq[AMBA_NR_IRQS];
char *driver_override;
+ struct amba_device *other_dev;
};

struct amba_driver {
--
2.30.1.766.gb4fecdf3b7-goog