Re: [PATCH v4 1/2] drivers: bus: simple-pm-bus: Add support for probing simple bus only devices

From: Kevin Hilman <khilman@baylibre.com>
To: Saravana Kannan <saravanak@google.com>,
	Russell King <linux@armlinux.org.uk>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Magnus Damm <magnus.damm@gmail.com>,
	Tony Lindgren <tony@atomide.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Damien Le Moal <damien.lemoal@wdc.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Saravana Kannan <saravanak@google.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	kernel-team@android.com, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-oxnas@groups.io,
	linux-renesas-soc@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-riscv@lists.infradead.org
Subject: Re: [PATCH v4 1/2] drivers: bus: simple-pm-bus: Add support for probing simple bus only devices
Date: Mon, 31 Jan 2022 19:18:35 -0800	[thread overview]
Message-ID: <7hk0efmfzo.fsf@baylibre.com> (raw)
In-Reply-To: <20210929000735.585237-2-saravanak@google.com>

Hi Saravana,

Saravana Kannan <saravanak@google.com> writes:

> fw_devlink could end up creating device links for bus only devices.
> However, bus only devices don't get probed and can block probe() or
> sync_state() [1] call backs of other devices. To avoid this, probe these
> devices using the simple-pm-bus driver.
>
> However, there are instances of devices that are not simple buses (they get
> probed by their specific drivers) that also list the "simple-bus" (or other
> bus only compatible strings) in their compatible property to automatically
> populate their child devices. We still want these devices to get probed by
> their specific drivers. So, we make sure this driver only probes devices
> that are only buses.
>
> [1] - https://lore.kernel.org/lkml/CAPDyKFo9Bxremkb1dDrr4OcXSpE0keVze94Cm=zrkOVxHHxBmQ@mail.gmail.com/
> Fixes: c442a0d18744 ("driver core: Set fw_devlink to "permissive" behavior by default")
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> Tested-by: Saravana Kannan <saravanak@google.com>
> Tested-by: Ulf Hansson <ulf.hansson@linaro.org>

This patch landed in stable/linux-5.10.y as commit d5f13bbb5104 and it
broke suspend/resume on at least one TI AM335x board I'm testing on:
upstream dts: arch/arm/boot/dts/am335x-icev2.dts, upstream defconfig:
arch/arm/configs/omap2plus_defconfig.

Bisecting between vanilla v5.10 (good) and stable/linux-5.10.y (bad)
pointed me to this patch, and I confirmed that reverting just this patch
on top of stable/linux-5.10.y makes it work again.

Also interesting, this same platform works fine on vanilla v5.15, which
also includes this patch.  That suggests that either 1) this patch
should not have been backported to v5.10 stable or 2) there are some
other dependencies that are missing in v5.10.

Since vanilla v5.10 works fine, I'm leaning towards (1), but if you have
any ideas for deps that need backporting, I'm happy to try.

I haven't debugged exactly where it's hanging yet, but, enabling
CONFIG_DEBUG_DRIVER=y, and suspending with "no_console_suspend" on the
command line, the last line before it hangs is:

   [   28.129966] simple-pm-bus ocp: noirq power domain suspend

Any ideas?

Kevin