Re: Regression in probing some AMBA devices possibly devlink related

[Saravana Kannan]

On Sat, Feb 25, 2023 at 6:29 AM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>
> Hi Saravana,
>
> I have a boot regression for Ux500 on mainline, but bisecting mainline
> isn't quite working for misc reasons :/
>
> I'm not sure about this regression, but it smells like devlink-related.
>
> Ux500 have a bunch of normal and some AMBA devices. After
> boot this happens and we hang waiting for rootfs (which is on eMMC):

Hmmm... my recent changes were definitely tested on systems with AMBA
devices and it worked.

> [   31.849586] amba 80126000.mmc: deferred probe pending
> [   31.854801] amba 80118000.mmc: deferred probe pending
> [   31.859895] amba 80005000.mmc: deferred probe pending
> [   31.870297] amba 80120000.uart: deferred probe pending
> [   31.875472] amba 80121000.uart: deferred probe pending
> [   31.880689] amba 80004000.i2c: deferred probe pending
> [   31.885799] amba 80128000.i2c: deferred probe pending
> [   31.890932] amba 80110000.i2c: deferred probe pending
> [   51.688361] vmem_3v3: disabling

What does /debug/devices_deferred say about these? That should tell us
exactly what's blocking it.

Also if you convert all the pr_debug and dev_dbg in
drivers/base/core.c that should give us enough of an idea of what's
happening. Can you do that too and send it as an attachment (I logs
are hard to read when they get word wrapped)?

> The last regulator (vmem_3v3) is something the eMMC that didn't
> probe was supposed to use.
>
> All the failing drivers are AMBA PrimeCell devices:
> drivers/mmc/host/mmci.c
> drivers/tty/serial/amba-pl011.c
> drivers/i2c/busses/i2c-nomadik.c
>
> This makes me suspect something was done for ordinary (platform)
> devices that didn't happen for AMBA devices?
>
> This is the main portion of the device tree containing these
> devices and their resources:
> arch/arm/boot/dts/ste-dbx5x0.dtsi

I'll take a closer look on Monday. Btw, I always prefer the dts file
because there's always some property that adds a dependency and that
might be the clue to whatever is broken. But I'll take a look at this.

It's unlikely the patch attached might fix it, but can you give it a shot?

-Saravana
From 5cc74db68155c8eb3e14b6d54895f4fc11390dce Mon Sep 17 00:00:00 2001
From: Saravana Kannan <saravanak@xxxxxxxxxx>
Date: Mon, 13 Feb 2023 13:40:43 -0800
Subject: [PATCH] add class support to device links

Signed-off-by: Saravana Kannan <saravanak@xxxxxxxxxx>
---
 drivers/base/core.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index abc6fd2868a5..a3cc516dc26b 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1393,7 +1393,15 @@ void device_links_driver_bound(struct device *dev)
 			continue;
 
 		WARN_ON(link->status != DL_STATE_DORMANT);
-		WRITE_ONCE(link->status, DL_STATE_AVAILABLE);
+
+		/*
+		 * A "class" device doesn't really probe. Consider the link as
+		 * active as soon as the supplier probes.
+		 */
+		if (link->consumer->class)
+			WRITE_ONCE(link->status, DL_STATE_ACTIVE);
+		else
+			WRITE_ONCE(link->status, DL_STATE_AVAILABLE);
 
 		if (link->flags & DL_FLAG_AUTOPROBE_CONSUMER)
 			driver_deferred_probe_add(link->consumer);
@@ -3599,6 +3607,14 @@ int device_add(struct device *dev)
 		devtmpfs_create_node(dev);
 	}
 
+	/*
+	 * Once a device is added to a class, consider it as bound to a driver
+	 * from a device links perspective. This way, we wouldn't block any
+	 * consumers of this device from probing.
+	 */
+	if (dev->class)
+		dev->links.status = DL_DEV_DRIVER_BOUND;
+
 	/* Notify clients of device addition.  This call must come
 	 * after dpm_sysfs_add() and before kobject_uevent().
 	 */
-- 
2.39.2.637.g21b0678d19-goog