Message-Id: <20241008231615.430073-1-djakov@kernel.org>
Date: Wed,  9 Oct 2024 02:16:15 +0300
From: djakov@...nel.org
To: broonie@...nel.org
Cc: ruanjinjie@...wei.com,
	dmitry.baryshkov@...aro.org,
	dan.carpenter@...aro.org,
	naresh.kamboju@...aro.org,
	linux-arm-msm@...r.kernel.org,
	linux-spi@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Georgi Djakov <djakov@...nel.org>,
	Linux Kernel Functional Testing <lkft@...aro.org>
Subject: [PATCH] spi: geni-qcom: Fix boot warning related to pm_runtime and devres

From: Georgi Djakov <djakov@...nel.org>

During boot, users sometimes observe the following warning:

[7.841431] WARNING: CPU: 4 PID: 492 at
drivers/interconnect/core.c:685 __icc_enable
(drivers/interconnect/core.c:685 (discriminator 7))
[..]
[7.841494] CPU: 4 PID: 492 Comm: (udev-worker) Not tainted 6.1.111-rc1 #1
[7.841497] Hardware name: Thundercomm Dragonboard 845c (DT)
[7.841499] pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[7.841502] pc : __icc_enable (drivers/interconnect/core.c:685
(discriminator 7))
[7.841505] lr : icc_disable (drivers/interconnect/core.c:708)
[..]
[7.841541] Call trace:
[7.841542] __icc_enable (drivers/interconnect/core.c:685 (discriminator 7))
[7.841545] icc_disable (drivers/interconnect/core.c:708)
[7.841547] geni_icc_disable (drivers/soc/qcom/qcom-geni-se.c:862)
[7.841553] spi_geni_runtime_suspend+0x3c/0x4c spi_geni_qcom
[7.841561] pm_generic_runtime_suspend (drivers/base/power/generic_ops.c:28)
[7.841565] __rpm_callback (drivers/base/power/runtime.c:395)
[7.841568] rpm_callback (drivers/base/power/runtime.c:532)
[7.841570] rpm_suspend (drivers/base/power/runtime.c:672)
[7.841572] rpm_idle (drivers/base/power/runtime.c:504 (discriminator 1))
[7.841574] update_autosuspend (drivers/base/power/runtime.c:1662)
[7.841576] pm_runtime_disable_action (include/linux/spinlock.h:401
drivers/base/power/runtime.c:1703 include/linux/pm_runtime.h:599
drivers/base/power/runtime.c:1517)
[7.841579] devm_action_release (drivers/base/devres.c:720)
[7.841581] release_nodes (drivers/base/devres.c:503)
[7.841583] devres_release_all (drivers/base/devres.c:532)
[7.841585] device_unbind_cleanup (drivers/base/dd.c:531)
[7.841589] really_probe (drivers/base/dd.c:710)
[7.841592] __driver_probe_device (drivers/base/dd.c:785)
[7.841594] driver_probe_device (drivers/base/dd.c:815)
[7.841596] __driver_attach (drivers/base/dd.c:1202)
[7.841598] bus_for_each_dev (drivers/base/bus.c:301)
[7.841600] driver_attach (drivers/base/dd.c:1219)
[7.841602] bus_add_driver (drivers/base/bus.c:618)
[7.841604] driver_register (drivers/base/driver.c:246)
[7.841607] __platform_driver_register (drivers/base/platform.c:868)
[7.841609] spi_geni_driver_init+0x28/0x1000 spi_geni_qcom
[7.841615] do_one_initcall (init/main.c:1298)
[7.841619] do_init_module (kernel/module/main.c:2469)
[7.841623] load_module (kernel/module/main.c:2878)
[..]

This occurs when the spi-geni driver receives an -EPROBE_DEFER error
from spi_geni_grab_gpi_chan(), causing devres to start releasing all
resources as shown below:

[7.138679] geni_spi 880000.spi: DEVRES REL ffff800081443800 devm_icc_release (8 bytes)
[7.138751] geni_spi 880000.spi: DEVRES REL ffff800081443800 devm_icc_release (8 bytes)
[7.138827] geni_spi 880000.spi: DEVRES REL ffff800081443800 pm_runtime_disable_action (16 bytes)
[7.139494] geni_spi 880000.spi: DEVRES REL ffff800081443800 devm_pm_opp_config_release (16 bytes)
[7.139512] geni_spi 880000.spi: DEVRES REL ffff800081443800 devm_spi_release_controller (8 bytes)
[7.139516] geni_spi 880000.spi: DEVRES REL ffff800081443800 devm_clk_release (16 bytes)
[7.139519] geni_spi 880000.spi: DEVRES REL ffff800081443800 devm_ioremap_release (8 bytes)
[7.139524] geni_spi 880000.spi: DEVRES REL ffff800081443800 devm_region_release (24 bytes)
[7.139527] geni_spi 880000.spi: DEVRES REL ffff800081443800 devm_kzalloc_release (22 bytes)
[7.139530] geni_spi 880000.spi: DEVRES REL ffff800081443800 devm_pinctrl_release (8 bytes)
[7.139539] geni_spi 880000.spi: DEVRES REL ffff800081443800 devm_kzalloc_release (40 bytes)

The issue here is that pm_runtime_disable_action() results in a call to
spi_geni_runtime_suspend(), which attempts to suspend the device and
disable an interconnect path that devm_icc_release() has just released.

Resolve this by calling geni_icc_get() before enabling runtime PM. This
approach ensures that when devres releases resources in reverse order,
it will start with pm_runtime_disable_action(), suspending the device,
and then proceed to free the remaining resources.

Reported-by: Naresh Kamboju <naresh.kamboju@...aro.org>
Reported-by: Linux Kernel Functional Testing <lkft@...aro.org>
Closes: https://lore.kernel.org/r/CA+G9fYtsjFtddG8i+k-SpV8U6okL0p4zpsTiwGfNH5GUA8dWAA@mail.gmail.com
Fixes: 89e362c883c6 ("spi: geni-qcom: Undo runtime PM changes at driver exit time")
Signed-off-by: Georgi Djakov <djakov@...nel.org>
---
 drivers/spi/spi-geni-qcom.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index f6e40f90418f..768d7482102a 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -1116,6 +1116,11 @@ static int spi_geni_probe(struct platform_device *pdev)
 	init_completion(&mas->tx_reset_done);
 	init_completion(&mas->rx_reset_done);
 	spin_lock_init(&mas->lock);
+
+	ret = geni_icc_get(&mas->se, NULL);
+	if (ret)
+		return ret;
+
 	pm_runtime_use_autosuspend(&pdev->dev);
 	pm_runtime_set_autosuspend_delay(&pdev->dev, 250);
 	ret = devm_pm_runtime_enable(dev);
@@ -1125,9 +1130,6 @@ static int spi_geni_probe(struct platform_device *pdev)
 	if (device_property_read_bool(&pdev->dev, "spi-slave"))
 		spi->target = true;
 
-	ret = geni_icc_get(&mas->se, NULL);
-	if (ret)
-		return ret;
 	/* Set the bus quota to a reasonable value for register access */
 	mas->se.icc_paths[GENI_TO_CORE].avg_bw = Bps_to_icc(CORE_2X_50_MHZ);
 	mas->se.icc_paths[CPU_TO_GENI].avg_bw = GENI_DEFAULT_BW;

Powered by blists - more mailing lists