Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
From:   Eugen Hristev <eugen.hristev@collabora.com>
To:     linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org, linux-pm@vger.kernel.org
Cc:     eballetbo@kernel.org, angelogioacchino.delregno@collabora.com,
        ulf.hansson@linaro.org, linux-kernel@vger.kernel.org,
        kernel@collabora.com, Eugen Hristev <eugen.hristev@collabora.com>
Subject: [PATCH] pmdomain: mediatek: fix race condition in power on/power off sequences
Date:   Wed, 29 Nov 2023 13:31:20 +0200
Message-Id: <20231129113120.4907-1-eugen.hristev@collabora.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

It can happen that during the power off sequence for a power domain
another power on sequence is started, and it can lead to powering on and
off in the same time for the similar power domain.
This can happen if parallel probing occurs: one device starts probing, and
one power domain is probe deferred, this leads to all power domains being
rolled back and powered off, while in the same time another device starts
probing and requests powering on the same power domains or similar.

This was encountered on MT8186, when the sequence is :
Power on SSUSB
Power on SSUSB_P1
Power on DIS
	-> probe deferred
Power off DIS
Power off SSUSB_P1
Power off SSUSB

During the sequence of powering off SSUSB, some new similar sequence starts,
and during the power on of SSUSB, clocks are enabled.
In this case, powering off SSUSB fails from the first sequence, because
power off ACK bit check times out (as clocks are powered back on by the second
sequence). In consequence, powering it on also times out, and it leads to
the whole power domain in a bad state.

To solve this issue, added a mutex that locks the whole power off/power on
sequence such that it would never happen that multiple sequences try to
enable or disable the same power domain in parallel.

Fixes: 59b644b01cf4 ("soc: mediatek: Add MediaTek SCPSYS power domains")
Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
---
 drivers/pmdomain/mediatek/mtk-pm-domains.c | 24 +++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/pmdomain/mediatek/mtk-pm-domains.c b/drivers/pmdomain/mediatek/mtk-pm-domains.c
index d5f0ee05c794..4f136b47e539 100644
--- a/drivers/pmdomain/mediatek/mtk-pm-domains.c
+++ b/drivers/pmdomain/mediatek/mtk-pm-domains.c
@@ -9,6 +9,7 @@
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/mfd/syscon.h>
+#include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/of_clk.h>
 #include <linux/platform_device.h>
@@ -56,6 +57,7 @@ struct scpsys {
 	struct device *dev;
 	struct regmap *base;
 	const struct scpsys_soc_data *soc_data;
+	struct mutex mutex;
 	struct genpd_onecell_data pd_data;
 	struct generic_pm_domain *domains[];
 };
@@ -238,9 +240,13 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
 	bool tmp;
 	int ret;
 
+	mutex_lock(&scpsys->mutex);
+
 	ret = scpsys_regulator_enable(pd->supply);
-	if (ret)
+	if (ret) {
+		mutex_unlock(&scpsys->mutex);
 		return ret;
+	}
 
 	ret = clk_bulk_prepare_enable(pd->num_clks, pd->clks);
 	if (ret)
@@ -291,6 +297,7 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
 			goto err_enable_bus_protect;
 	}
 
+	mutex_unlock(&scpsys->mutex);
 	return 0;
 
 err_enable_bus_protect:
@@ -305,6 +312,7 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
 	clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
 err_reg:
 	scpsys_regulator_disable(pd->supply);
+	mutex_unlock(&scpsys->mutex);
 	return ret;
 }
 
@@ -315,13 +323,15 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
 	bool tmp;
 	int ret;
 
+	mutex_lock(&scpsys->mutex);
+
 	ret = scpsys_bus_protect_enable(pd);
 	if (ret < 0)
-		return ret;
+		goto err_mutex_unlock;
 
 	ret = scpsys_sram_disable(pd);
 	if (ret < 0)
-		return ret;
+		goto err_mutex_unlock;
 
 	if (pd->data->ext_buck_iso_offs && MTK_SCPD_CAPS(pd, MTK_SCPD_EXT_BUCK_ISO))
 		regmap_set_bits(scpsys->base, pd->data->ext_buck_iso_offs,
@@ -340,13 +350,15 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
 	ret = readx_poll_timeout(scpsys_domain_is_on, pd, tmp, !tmp, MTK_POLL_DELAY_US,
 				 MTK_POLL_TIMEOUT);
 	if (ret < 0)
-		return ret;
+		goto err_mutex_unlock;
 
 	clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
 
 	scpsys_regulator_disable(pd->supply);
 
-	return 0;
+err_mutex_unlock:
+	mutex_unlock(&scpsys->mutex);
+	return ret;
 }
 
 static struct
@@ -700,6 +712,8 @@ static int scpsys_probe(struct platform_device *pdev)
 		return PTR_ERR(scpsys->base);
 	}
 
+	mutex_init(&scpsys->mutex);
+
 	ret = -ENODEV;
 	for_each_available_child_of_node(np, node) {
 		struct generic_pm_domain *domain;
-- 
2.34.1