All of lore.kernel.org
 help / color / mirror / Atom feed
From: Herman van Hazendonk <github.com@herrie.org>
To: sboyd@kernel.org
Cc: Herman van Hazendonk <github.com@herrie.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH 2/3] clk: qcom: lcc-msm8960: serialise probe and block sysfs rebind
Date: Tue,  2 Jun 2026 06:50:00 +0200	[thread overview]
Message-ID: <20260602045002.290918-3-github.com@herrie.org> (raw)
In-Reply-To: <20260602045002.290918-1-github.com@herrie.org>

The probe path mutates file-static clk_rcg structures:

  - The .freq_tbl pointer on slimbus_src, mi2s_osr_src, the four
    codec_i2s_*/spare_i2s_* osr_src entries, and pcm_src is selected
    from the PLL4 L-register read.

  - pxo_parent_data.fw_name/.name and lcc_pxo_pll4[0].fw_name/.name
    are switched to cxo for the MDM9615 compatible.

These are all file-static, and the registered clk_rcg objects are
referenced by qcom_cc_really_probe() through the same pointers. Two
hazards follow from that:

  1. Concurrent probe (a hypothetical second LCC instance, or sysfs
     bind racing the first probe) would race both the freq_tbl
     assignment and qcom_cc_really_probe()'s registration, leaving
     the clock tree in an inconsistent state with no diagnostic.

  2. Sysfs unbind + rebind would either race the previous instance's
     devm cleanup of those clocks (devm teardown runs after .remove
     returns, so concurrent probe-side mutation is observable), or,
     for a MDM9615 -> MSM8960 sequence, leave pxo_parent_data stuck
     on cxo for the new bind because remove cannot easily undo the
     compatible-specific patch.

Address both with a static bool guarded by a mutex (check-and-claim
atomically inside the locked scope so two concurrent probes cannot
both see the flag clear) and .suppress_bind_attrs = true on the
platform_driver. Sysfs bind/unbind is blocked entirely; module
unload is the only tear-down path and is serialised by the module
mutex, so no .remove callback is needed.

Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
 drivers/clk/qcom/lcc-msm8960.c | 93 +++++++++++++++++++++++++++++++---
 1 file changed, 85 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/qcom/lcc-msm8960.c b/drivers/clk/qcom/lcc-msm8960.c
index 44321c01f957..1e0873ce1cc2 100644
--- a/drivers/clk/qcom/lcc-msm8960.c
+++ b/drivers/clk/qcom/lcc-msm8960.c
@@ -6,6 +6,7 @@
 #include <linux/kernel.h>
 #include <linux/bitops.h>
 #include <linux/err.h>
+#include <linux/mutex.h>
 #include <linux/platform_device.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -450,12 +451,38 @@ static const struct of_device_id lcc_msm8960_match_table[] = {
 };
 MODULE_DEVICE_TABLE(of, lcc_msm8960_match_table);
 
+/*
+ * The probe path mutates file-static clk_rcg structures (.freq_tbl
+ * pointers selected from the PLL4 L-register, and pxo_parent_data /
+ * lcc_pxo_pll4 for the MDM9615 cxo override). All clk_rcg objects in
+ * this file are registered by pointer through qcom_cc_really_probe(),
+ * so a second concurrent probe of the same driver would race both that
+ * registration and the freq_tbl assignment. Enforce single-instance
+ * binding with a static flag protected by an internal mutex.
+ */
+static bool lcc_msm8960_bound;
+static DEFINE_MUTEX(lcc_msm8960_bound_lock);
+
 static int lcc_msm8960_probe(struct platform_device *pdev)
 {
 	u32 val;
 	int ret;
 	struct regmap *regmap;
 
+	mutex_lock(&lcc_msm8960_bound_lock);
+	if (lcc_msm8960_bound) {
+		mutex_unlock(&lcc_msm8960_bound_lock);
+		return dev_err_probe(&pdev->dev, -EBUSY,
+			"only a single LCC instance is supported\n");
+	}
+	/*
+	 * Claim ownership inside the same locked region as the check
+	 * so a second concurrent probe cannot pass the check before
+	 * we set the flag.
+	 */
+	lcc_msm8960_bound = true;
+	mutex_unlock(&lcc_msm8960_bound_lock);
+
 	/* patch for the cxo <-> pxo difference */
 	if (of_device_is_compatible(pdev->dev.of_node, "qcom,lcc-mdm9615")) {
 		pxo_parent_data.fw_name = "cxo";
@@ -465,14 +492,18 @@ static int lcc_msm8960_probe(struct platform_device *pdev)
 	}
 
 	regmap = qcom_cc_map(pdev, &lcc_msm8960_desc);
-	if (IS_ERR(regmap))
-		return PTR_ERR(regmap);
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
+		goto err_unclaim;
+	}
 
 	/* Use the correct frequency plan depending on speed of PLL4 */
 	ret = regmap_read(regmap, 0x4, &val);
-	if (ret)
-		return dev_err_probe(&pdev->dev, ret,
-				     "failed to read PLL4 L register\n");
+	if (ret) {
+		dev_err_probe(&pdev->dev, ret,
+			      "failed to read PLL4 L register\n");
+		goto err_unclaim;
+	}
 	if (val == 0x12) {
 		slimbus_src.freq_tbl = clk_tbl_aif_osr_492;
 		mi2s_osr_src.freq_tbl = clk_tbl_aif_osr_492;
@@ -484,18 +515,64 @@ static int lcc_msm8960_probe(struct platform_device *pdev)
 	}
 	/* Enable PLL4 source on the LPASS Primary PLL Mux */
 	ret = regmap_write(regmap, 0xc4, 0x1);
+	if (ret) {
+		dev_err_probe(&pdev->dev, ret,
+			      "failed to select PLL4 on LPASS Primary PLL Mux\n");
+		goto err_unclaim;
+	}
+
+	ret = qcom_cc_really_probe(&pdev->dev, &lcc_msm8960_desc, regmap);
 	if (ret)
-		return dev_err_probe(&pdev->dev, ret,
-				     "failed to select PLL4 on LPASS Primary PLL Mux\n");
+		goto err_unclaim;
+
+	return 0;
+
+err_unclaim:
+	mutex_lock(&lcc_msm8960_bound_lock);
+	lcc_msm8960_bound = false;
+	mutex_unlock(&lcc_msm8960_bound_lock);
+	return ret;
+}
 
-	return qcom_cc_really_probe(&pdev->dev, &lcc_msm8960_desc, regmap);
+/*
+ * Clear the singleton bound flag so a future probe can succeed.
+ *
+ * .suppress_bind_attrs = true below blocks the sysfs bind/unbind path,
+ * which is the rebind race window we care about. However, the driver
+ * core can still call .remove via device removal (DT overlay removal,
+ * hot-unplug). Without clearing the flag here, the next probe of a
+ * fresh instance would fail with -EBUSY. The probe and remove on the
+ * same device are serialised by device_lock so this clear does not
+ * race the devm cleanup of the previous instance.
+ */
+static void lcc_msm8960_remove(struct platform_device *pdev)
+{
+	mutex_lock(&lcc_msm8960_bound_lock);
+	lcc_msm8960_bound = false;
+	mutex_unlock(&lcc_msm8960_bound_lock);
 }
 
 static struct platform_driver lcc_msm8960_driver = {
 	.probe		= lcc_msm8960_probe,
+	.remove		= lcc_msm8960_remove,
 	.driver		= {
 		.name	= "lcc-msm8960",
 		.of_match_table = lcc_msm8960_match_table,
+		/*
+		 * The probe path mutates file-static clk_rcg structures
+		 * (.freq_tbl pointers selected from the PLL4 L-register,
+		 * pxo_parent_data switched to cxo for MDM9615) that are
+		 * then registered by pointer through qcom_cc_really_probe().
+		 * A sysfs unbind + rebind would either race the previous
+		 * instance's devm teardown of those clocks or silently
+		 * leave pxo_parent_data stuck on its previous compatible's
+		 * setting (e.g. MDM9615 -> MSM8960 stuck on cxo). Block
+		 * sysfs bind / unbind to remove both hazards; non-sysfs
+		 * removal (DT overlay, hot-unplug) is serialised against
+		 * probe by device_lock so .remove just clears the singleton
+		 * flag and lets devm undo the clock registrations.
+		 */
+		.suppress_bind_attrs = true,
 	},
 };
 module_platform_driver(lcc_msm8960_driver);
-- 
2.43.0


  parent reply	other threads:[~2026-06-02  4:50 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260602045002.290918-1-github.com@herrie.org>
2026-06-02  4:49 ` [PATCH 1/3] clk: qcom: lcc-msm8960: check regmap_read/regmap_write return values in probe Herman van Hazendonk
2026-06-08  9:27   ` Konrad Dybcio
2026-06-02  4:50 ` Herman van Hazendonk [this message]
2026-06-02  4:50 ` [PATCH 3/3] clk: qcom: lcc-msm8960: re-apply PLL4 mux on resume Herman van Hazendonk
2026-06-08  9:30   ` Konrad Dybcio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260602045002.290918-3-github.com@herrie.org \
    --to=github.com@herrie.org \
    --cc=andersson@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=sboyd@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.