Re: [PATCH] soc: qcom: pd-mapper: defer probing on sdm845

On Wed, 5 Feb 2025 at 23:57, Frank Oltmanns frank@oltmanns.dev wrote:

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.

The cause for such issues also must be identified. I mean, in-kernel pd-mapper uncovered a lot of issues that were not noticed because usually the starting order was a bit different. But we need to understand how to actually fix those issues instead of working them around.

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@vger.kernel.org Signed-off-by: Frank Oltmanns frank@oltmanns.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@oltmanns.dev