lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250205-qcom_pdm_defer-v1-1-a2e9a39ea9b9@oltmanns.dev>
Date: Wed, 05 Feb 2025 22:57:11 +0100
From: Frank Oltmanns <frank@...manns.dev>
To: Bjorn Andersson <andersson@...nel.org>, 
 Konrad Dybcio <konradybcio@...nel.org>, 
 Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, 
 Chris Lew <quic_clew@...cinc.com>
Cc: linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org, 
 Stephan Gerhold <stephan.gerhold@...aro.org>, 
 Johan Hovold <johan+linaro@...nel.org>, 
 Caleb Connolly <caleb.connolly@...aro.org>, 
 Joel Selvaraj <joelselvaraj.oss@...il.com>, 
 Alexey Minnekhanov <alexeymin@...tmarketos.org>, stable@...r.kernel.org, 
 Frank Oltmanns <frank@...manns.dev>
Subject: [PATCH] soc: qcom: pd-mapper: defer probing on sdm845

On xiaomi-beryllium and oneplus-enchilada audio does not work reliably
with the in-kernel pd-mapper. Deferring the probe solves these issues.
Specifically, audio only works reliably with the in-kernel pd-mapper, if
the probe succeeds when remoteproc3 triggers the first successful probe.
I.e., probes from remoteproc0, 1, and 2 need to be deferred until
remoteproc3 has been probed.

Introduce a device specific quirk that lists the first auxdev for which
the probe must be executed. Until then, defer probes from other auxdevs.

Fixes: 1ebcde047c54 ("soc: qcom: add pd-mapper implementation")
Cc: stable@...r.kernel.org
Signed-off-by: Frank Oltmanns <frank@...manns.dev>
---
The in-kernel pd-mapper has been causing audio issues on sdm845
devices (specifically, xiaomi-beryllium and oneplus-enchilada). I
observed that Stephan’s approach [1] - which defers module probing by
blocklisting the module and triggering a later probe - works reliably.

Inspired by this, I experimented with delaying the probe within the
module itself by returning -EPROBE_DEFER in qcom_pdm_probe() until a
certain time (13.9 seconds after boot, based on ktime_get()) had
elapsed. This method also restored audio functionality.

Further logging of auxdev->id in qcom_pdm_probe() led to an interesting
discovery: audio only works reliably with the in-kernel pd-mapper when
the first successful probe is triggered by remoteproc3. In other words,
probes from remoteproc0, 1, and 2 must be deferred until remoteproc3 has
been probed.

To address this, I propose introducing a quirk table (which currently
only contains sdm845) to defer probing until the correct auxiliary
device (remoteproc3) initiates the probe.

I look forward to your feedback.

Thanks,
  Frank

[1]: https://lore.kernel.org/linux-arm-msm/Zwj3jDhc9fRoCCn6@linaro.org/
---
 drivers/soc/qcom/qcom_pd_mapper.c | 43 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/soc/qcom/qcom_pd_mapper.c b/drivers/soc/qcom/qcom_pd_mapper.c
index 154ca5beb47160cc404a46a27840818fe3187420..34b26df665a888ac4872f56e948e73b561ae3b6b 100644
--- a/drivers/soc/qcom/qcom_pd_mapper.c
+++ b/drivers/soc/qcom/qcom_pd_mapper.c
@@ -46,6 +46,11 @@ struct qcom_pdm_data {
 	struct list_head services;
 };
 
+struct qcom_pdm_probe_first_dev_quirk {
+	const char *name;
+	u32 id;
+};
+
 static DEFINE_MUTEX(qcom_pdm_mutex); /* protects __qcom_pdm_data */
 static struct qcom_pdm_data *__qcom_pdm_data;
 
@@ -526,6 +531,11 @@ static const struct qcom_pdm_domain_data *x1e80100_domains[] = {
 	NULL,
 };
 
+static const struct qcom_pdm_probe_first_dev_quirk first_dev_remoteproc3 = {
+	.id = 3,
+	.name = "pd-mapper"
+};
+
 static const struct of_device_id qcom_pdm_domains[] __maybe_unused = {
 	{ .compatible = "qcom,apq8016", .data = NULL, },
 	{ .compatible = "qcom,apq8064", .data = NULL, },
@@ -566,6 +576,10 @@ static const struct of_device_id qcom_pdm_domains[] __maybe_unused = {
 	{},
 };
 
+static const struct of_device_id qcom_pdm_defer[] __maybe_unused = {
+	{ .compatible = "qcom,sdm845", .data = &first_dev_remoteproc3, },
+	{},
+};
 static void qcom_pdm_stop(struct qcom_pdm_data *data)
 {
 	qcom_pdm_free_domains(data);
@@ -637,6 +651,25 @@ static struct qcom_pdm_data *qcom_pdm_start(void)
 	return ERR_PTR(ret);
 }
 
+static bool qcom_pdm_ready(struct auxiliary_device *auxdev)
+{
+	const struct of_device_id *match;
+	struct device_node *root;
+	struct qcom_pdm_probe_first_dev_quirk *first_dev;
+
+	root = of_find_node_by_path("/");
+	if (!root)
+		return true;
+
+	match = of_match_node(qcom_pdm_defer, root);
+	of_node_put(root);
+	if (!match)
+		return true;
+
+	first_dev = (struct qcom_pdm_probe_first_dev_quirk *) match->data;
+	return (auxdev->id == first_dev->id) && !strcmp(auxdev->name, first_dev->name);
+}
+
 static int qcom_pdm_probe(struct auxiliary_device *auxdev,
 			  const struct auxiliary_device_id *id)
 
@@ -647,6 +680,15 @@ static int qcom_pdm_probe(struct auxiliary_device *auxdev,
 	mutex_lock(&qcom_pdm_mutex);
 
 	if (!__qcom_pdm_data) {
+		if (!qcom_pdm_ready(auxdev)) {
+			pr_debug("%s: Deferring probe for device %s (id: %u)\n",
+				__func__, auxdev->name, auxdev->id);
+			ret = -EPROBE_DEFER;
+			goto probe_stop;
+		}
+		pr_debug("%s: Probing for device %s (id: %u), starting pdm\n",
+			__func__, auxdev->name, auxdev->id);
+
 		data = qcom_pdm_start();
 
 		if (IS_ERR(data))
@@ -659,6 +701,7 @@ static int qcom_pdm_probe(struct auxiliary_device *auxdev,
 
 	auxiliary_set_drvdata(auxdev, __qcom_pdm_data);
 
+probe_stop:
 	mutex_unlock(&qcom_pdm_mutex);
 
 	return ret;

---
base-commit: 7f048b202333b967782a98aa21bb3354dc379bbf
change-id: 20250205-qcom_pdm_defer-3dc1271d74d9

Best regards,
-- 
Frank Oltmanns <frank@...manns.dev>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ