[PATCH v3 1/4] soc: qcom: ice: Fix race between qcom_ice_probe() and of_qcom_ice_get()

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>

The current platform driver design causes probe ordering races with
consumers (UFS, eMMC) due to ICE's dependency on SCM firmware calls. If ICE
probe fails (missing ICE SCM or DT registers), devm_of_qcom_ice_get() loops
with -EPROBE_DEFER, leaving consumers non-functional even when ICE should
be gracefully disabled. devm_of_qcom_ice_get() doesn't know if the ICE
driver probe has failed due to above reasons or it is waiting for the SCM
driver.

Moreover, there is no devlink dependency between ICE and consumer drivers
as 'qcom,ice' is not considered as a DT 'supplier'. So the consumer drivers
have no idea of when the ICE driver is going to probe.

To address these issues, introduce a global ice_handle to store the valid
ICE handle pointer, and set during successful ICE driver probe. On probe
failure, set it to an error pointer and propagate the error from
of_qcom_ice_get().

Additionally, add a global ice_mutex to synchronize qcom_ice_probe() and
of_qcom_ice_get().

Note that this change only fixes the standalone ICE DT node bindings and
not the ones with 'ice' range embedded in the consumer nodes, where there
is no issue.

Cc: <stable@xxxxxxxxxxxxxxx> # 6.4
Fixes: 2afbf43a4aec ("soc: qcom: Make the Qualcomm UFS/SDCC ICE a dedicated driver")
Reported-by: Sumit Garg <sumit.garg@xxxxxxxxxxxxxxxx>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
---
 drivers/soc/qcom/ice.c | 44 +++++++++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c
index b203bc685cad..3c3c189e24f9 100644
--- a/drivers/soc/qcom/ice.c
+++ b/drivers/soc/qcom/ice.c
@@ -113,6 +113,9 @@ struct qcom_ice {
 	u8 hwkm_version;
 };
 
+static DEFINE_MUTEX(ice_mutex);
+static struct qcom_ice *ice_handle;
+
 static bool qcom_ice_check_supported(struct qcom_ice *ice)
 {
 	u32 regval = qcom_ice_readl(ice, QCOM_ICE_REG_VERSION);
@@ -608,7 +611,6 @@ static struct qcom_ice *qcom_ice_create(struct device *dev,
 static struct qcom_ice *of_qcom_ice_get(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
-	struct qcom_ice *ice;
 	struct resource *res;
 	void __iomem *base;
 	struct device_link *link;
@@ -631,6 +633,22 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
 		return qcom_ice_create(&pdev->dev, base);
 	}
 
+	guard(mutex)(&ice_mutex);
+
+	/*
+	 * If ice_handle is NULL, then it means the ICE driver is not probed
+	 * yet. So return -EPROBE_DEFER to let the client try later.
+	 */
+	if (!ice_handle)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	/*
+	 * If ice_handle has error code, then it means the ICE driver has probe
+	 * failed. So return the handle for the client to digest it.
+	 */
+	if (IS_ERR(ice_handle))
+		return ice_handle;
+
 	/*
 	 * If the consumer node does not provider an 'ice' reg range
 	 * (legacy DT binding), then it must at least provide a phandle
@@ -647,24 +665,16 @@ static struct qcom_ice *of_qcom_ice_get(struct device *dev)
 		return ERR_PTR(-EPROBE_DEFER);
 	}
 
-	ice = platform_get_drvdata(pdev);
-	if (!ice) {
-		dev_err(dev, "Cannot get ice instance from %s\n",
-			dev_name(&pdev->dev));
-		platform_device_put(pdev);
-		return ERR_PTR(-EPROBE_DEFER);
-	}
-
 	link = device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_SUPPLIER);
 	if (!link) {
 		dev_err(&pdev->dev,
 			"Failed to create device link to consumer %s\n",
 			dev_name(dev));
 		platform_device_put(pdev);
-		ice = ERR_PTR(-EINVAL);
+		return ERR_PTR(-EINVAL);
 	}
 
-	return ice;
+	return ice_handle;
 }
 
 static void qcom_ice_put(const struct qcom_ice *ice)
@@ -716,20 +726,20 @@ EXPORT_SYMBOL_GPL(devm_of_qcom_ice_get);
 
 static int qcom_ice_probe(struct platform_device *pdev)
 {
-	struct qcom_ice *engine;
 	void __iomem *base;
 
+	guard(mutex)(&ice_mutex);
+
 	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base)) {
 		dev_warn(&pdev->dev, "ICE registers not found\n");
+		ice_handle = base;
 		return PTR_ERR(base);
 	}
 
-	engine = qcom_ice_create(&pdev->dev, base);
-	if (IS_ERR(engine))
-		return PTR_ERR(engine);
-
-	platform_set_drvdata(pdev, engine);
+	ice_handle = qcom_ice_create(&pdev->dev, base);
+	if (IS_ERR(ice_handle))
+		return PTR_ERR(ice_handle);
 
 	return 0;
 }

-- 
2.51.0