[PATCH RFC 3/3] amba: bus: Move reading periphid operation from amba_match() to amba_probe()

From: Zijun Hu
Date: Sun Sep 08 2024 - 19:39:08 EST


From: Zijun Hu <quic_zijuhu@xxxxxxxxxxx>

amba_match(), as bus_type @amba_bustype's match(), reads periphid from
hardware and may return -EPROBE_DEFER consequently, and it is the only
one that breaks below ideal rule in current kernel tree:

bus_type's match() should only return bool type compatible integer 0 or
1 ideally since its main operations are lookup and comparison normally.

fixed by moving reading periphid operation to amba_probe().

Signed-off-by: Zijun Hu <quic_zijuhu@xxxxxxxxxxx>
---
drivers/amba/bus.c | 55 +++++++++++++++++++++++++++++-------------------
include/linux/amba/bus.h | 1 -
2 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 033d626aff46..8fe2e054b5ce 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -188,7 +188,7 @@ static int amba_read_periphid(struct amba_device *dev)
}

if (cid == AMBA_CID || cid == CORESIGHT_CID) {
- dev->periphid = pid;
+ WRITE_ONCE(dev->periphid, pid);
dev->cid = cid;
}

@@ -246,24 +246,14 @@ static int amba_match(struct device *dev, const struct device_driver *drv)
struct amba_device *pcdev = to_amba_device(dev);
const struct amba_driver *pcdrv = to_amba_driver(drv);

- mutex_lock(&pcdev->periphid_lock);
- if (!pcdev->periphid) {
- int ret = amba_read_periphid(pcdev);
-
- /*
- * Returning any error other than -EPROBE_DEFER from bus match
- * can cause driver registration failure. So, if there's a
- * permanent failure in reading pid and cid, simply map it to
- * -EPROBE_DEFER.
- */
- if (ret) {
- mutex_unlock(&pcdev->periphid_lock);
- return -EPROBE_DEFER;
- }
- dev_set_uevent_suppress(dev, false);
- kobject_uevent(&dev->kobj, KOBJ_ADD);
- }
- mutex_unlock(&pcdev->periphid_lock);
+ /*
+ * For an AMBA device without valid periphid, only read periphid
+ * in amba_probe() for it when try to bind @amba_proxy_drv.
+ * For @pcdev->periphid, Reading here has a little race with
+ * writing in amba_probe().
+ */
+ if (!READ_ONCE(pcdev->periphid))
+ return pcdrv == &amba_proxy_drv ? 1 : 0;

/* When driver_override is set, only bind to the matching driver */
if (pcdev->driver_override)
@@ -315,10 +305,24 @@ static int amba_probe(struct device *dev)
{
struct amba_device *pcdev = to_amba_device(dev);
struct amba_driver *pcdrv = to_amba_driver(dev->driver);
- const struct amba_id *id = amba_lookup(pcdrv->id_table, pcdev);
+ const struct amba_id *id;
int ret;

do {
+ if (!pcdev->periphid) {
+ ret = amba_read_periphid(pcdev);
+ if (ret) {
+ dev_err_probe(dev, ret, "failed to read periphid\n");
+ } else {
+ dev_set_uevent_suppress(dev, false);
+ kobject_uevent(&dev->kobj, KOBJ_ADD);
+ }
+
+ ret = -EPROBE_DEFER;
+ break;
+ }
+ id = amba_lookup(pcdrv->id_table, pcdev);
+
ret = of_amba_device_decode_irq(pcdev);
if (ret)
break;
@@ -389,10 +393,15 @@ static void amba_shutdown(struct device *dev)

static int amba_dma_configure(struct device *dev)
{
+ struct amba_device *pcdev = to_amba_device(dev);
struct amba_driver *drv = to_amba_driver(dev->driver);
enum dev_dma_attr attr;
int ret = 0;

+ /* To successfully go to amba_probe() to read periphid */
+ if (!pcdev->periphid)
+ return 0;
+
if (dev->of_node) {
ret = of_dma_configure(dev, dev->of_node, true);
} else if (has_acpi_companion(dev)) {
@@ -411,8 +420,12 @@ static int amba_dma_configure(struct device *dev)

static void amba_dma_cleanup(struct device *dev)
{
+ struct amba_device *pcdev = to_amba_device(dev);
struct amba_driver *drv = to_amba_driver(dev->driver);

+ if (!pcdev->periphid)
+ return;
+
if (!drv->driver_managed_dma)
iommu_device_unuse_default_domain(dev);
}
@@ -535,7 +548,6 @@ static void amba_device_release(struct device *dev)
fwnode_handle_put(dev_fwnode(&d->dev));
if (d->res.parent)
release_resource(&d->res);
- mutex_destroy(&d->periphid_lock);
kfree(d);
}

@@ -593,7 +605,6 @@ static void amba_device_initialize(struct amba_device *dev, const char *name)
dev->dev.dma_mask = &dev->dev.coherent_dma_mask;
dev->dev.dma_parms = &dev->dma_parms;
dev->res.name = dev_name(&dev->dev);
- mutex_init(&dev->periphid_lock);
}

/**
diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
index 958a55bcc708..4bb3467d9c3d 100644
--- a/include/linux/amba/bus.h
+++ b/include/linux/amba/bus.h
@@ -67,7 +67,6 @@ struct amba_device {
struct clk *pclk;
struct device_dma_parameters dma_parms;
unsigned int periphid;
- struct mutex periphid_lock;
unsigned int cid;
struct amba_cs_uci_id uci;
unsigned int irq[AMBA_NR_IRQS];

--
2.34.1