[PATCH v1] RFC: amba: Remove amba specific deferred probe handling
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
Cc: Ulf Hansson
Cc: John Stultz
Cc: Saravana Kannan
Cc: Sudeep Holla
Cc: Nicolas Saenz Julienne
Cc: Geert Uytterhoeven
Cc: Russell King
Cc: Rob Herring
Signed-off-by: Saravana Kannan
---
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-8y...@mail.gmail.com/
-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(_bustype);
+ int ret = bus_register(_bustype);
+
+ if (ret)
+ return ret;
+ return amba_driver_register(_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(_devices_lock);
-
- list_for_each_entry_safe(ddev, tmp, _devices, node) {
- int ret = amba_device_try_add(ddev->dev, ddev->parent);
-
- if (ret == -EPROBE_DEFER)
- continue;
-
- list_del_init(>node);
- kfree(ddev);
- }
-
-
Re: [PATCH v1] RFC: amba: Remove amba specific deferred probe handling
On Wed, Mar 3, 2021 at 12:32 AM Saravana Kannan wrote:
>
> 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
> Cc: Ulf Hansson
> Cc: John Stultz
> Cc: Saravana Kannan
> Cc: Sudeep Holla
> Cc: Nicolas Saenz Julienne
> Cc: Geert Uytterhoeven
> Cc: Russell King
> Cc: Rob Herring
> Signed-off-by: Saravana Kannan
> ---
>
> 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.
Btw, what really is the requirement wrt the uevents? Will this whole
thing work if I figure out a way to do this:
1. Add an amba device without the AMBA_ID and MODALIAS uevent vars and
without periphid set.
2. Once the resources (clocks, etc) are available, set periphid and
add those uevents.
3. Trigger a normal deferred probe attempt.
Will userspace properly load the right driver and will things work if
there is a couple of seconds of (theoretical) delay between (1) and
(2)? If so, that might be pretty easy to do without a stub device too.
-Saravana
>
> Thoughts?
>
> [1] -
> https://lore.kernel.org/linux-arm-kernel/cagetcx8cn-b6l2y10lkb91s3n06b6+be2z_a0402eyny-8y...@mail.gmail.com/
>
> -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(_bustype);
> + int ret = bus_register(_bustype);
> +
> + if (ret)
> + return ret;
> + return amba_driver_register(_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;
> -
