Linux clock framework development
 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: 3+ 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-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

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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox