Re: [PATCH 3/3] amba: Properly handle device probe without IRQ domain

From: Kefeng Wang
Date: Wed Aug 25 2021 - 10:42:53 EST



On 2021/8/25 20:33, Rob Herring wrote:
On Tue, Aug 24, 2021 at 11:05 PM Kefeng Wang <wangkefeng.wang@xxxxxxxxxx> wrote:

...

Similar to other resources the AMBA bus "gets" for the device, I think
this should be moved into amba_probe() and not here. There's no reason
to delay the addition of the device (and loading its module) because
the IRQ isn't ready yet.
The following code in the amba_device_try_add() will be called, it uses irq[0]
and irq[1], so I put of_amba_device_decode_irq() into amba_device_try_add().

470 if (dev->irq[0])
471 ret = device_create_file(&dev->dev, &dev_attr_irq0);
472 if (ret == 0 && dev->irq[1])
473 ret = device_create_file(&dev->dev, &dev_attr_irq1);
474 if (ret == 0)
475 return ret;
I wonder if we could just remove these. Why does userspace need them
in the first place? It's only an ABI if someone notices. Looking at
the history, AMBA bus was added in 2003 with just 'irq' and then
changed (ABI break) in 2004 to 'irq0' and 'irq1'.

Rob

Ok, I will kill all irq parts,

diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 962041148482..c08e8b30e02c 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -20,8 +20,6 @@
 #include <linux/platform_device.h>
 #include <linux/reset.h>

-#include <asm/irq.h>
-
 #define to_amba_driver(d)      container_of(d, struct amba_driver, drv)

 /* called on periphid match and class 0x9 coresight device. */
@@ -135,8 +133,6 @@ static ssize_t name##_show(struct device *_dev,                             \
 static DEVICE_ATTR_RO(name)

 amba_attr_func(id, "%08x\n", dev->periphid);
-amba_attr_func(irq0, "%u\n", dev->irq[0]);
-amba_attr_func(irq1, "%u\n", dev->irq[1]);
 amba_attr_func(resource, "\t%016llx\t%016llx\t%016lx\n",
         (unsigned long long)dev->res.start, (unsigned long long)dev->res.end,
         dev->res.flags);
@@ -467,10 +463,6 @@ static int amba_device_try_add(struct amba_device *dev, struct resource *parent)
        if (ret)
                goto err_release;

-       if (dev->irq[0])
-               ret = device_create_file(&dev->dev, &dev_attr_irq0);
-       if (ret == 0 && dev->irq[1])
-               ret = device_create_file(&dev->dev, &dev_attr_irq1);

and do some cleanup about error handling in the next version.


[1] https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/log/arch/arm/common/amba.c
.