Thanks, that was quick!

On 6/25/26 18:18, Philipp Zabel wrote:

On Do, 2026-06-25 at 18:10 +0400, George Moussalem via B4 Relay wrote:

quoted

From: George Moussalem <redacted>

Add support to bring up the M0 core of the bluetooth subsystem found in
the IPQ5018 SoC.

The signed firmware loaded is authenticated by TrustZone. If successful,
the M0 core boots the firmware and the peripheral is taken out of reset
using a Secure Channel Manager call to TrustZone.

Signed-off-by: George Moussalem <redacted>
---
 drivers/remoteproc/Kconfig            |  12 ++
 drivers/remoteproc/Makefile           |   1 +
 drivers/remoteproc/qcom_m0_btss_pil.c | 261 ++++++++++++++++++++++++++++++++++
 3 files changed, 274 insertions(+)

[...]

quoted

diff --git a/drivers/remoteproc/qcom_m0_btss_pil.c b/drivers/remoteproc/qcom_m0_btss_pil.c
new file mode 100644
index 000000000000..7168e270e4d4
--- /dev/null
+++ b/drivers/remoteproc/qcom_m0_btss_pil.c

@@ -0,0 +1,261 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2026 The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/elf.h>
+#include <linux/firmware/qcom/qcom_scm.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/soc/qcom/mdt_loader.h>
+
+#include "qcom_common.h"
+
+#define BTSS_PAS_ID	0xc
+
+struct m0_btss {
+	struct device *dev;
+	phys_addr_t mem_phys;
+	phys_addr_t mem_reloc;
+	void __iomem *mem_region;
+	size_t mem_size;
+	struct reset_control *btss_reset;

Why is this stored here? It doesn't seem to be used.

will remove it and use devm_reset_control_get_exclusive_deasserted as
suggested below.

[...]

quoted

+static int m0_btss_pil_probe(struct platform_device *pdev)
+{
+	// struct reset_control *btss_reset;

It looks like this shouldn't be commented out.

quoted

+	struct device *dev = &pdev->dev;
+	const char *fw_name = NULL;
+	struct m0_btss *desc;
+	struct clk *lpo_clk;
+	struct rproc *rproc;
+	int ret;
+
+	ret = of_property_read_string(dev->of_node, "firmware-name",
+				      &fw_name);
+	if (ret < 0)
+		return ret;
+
+	rproc = devm_rproc_alloc(dev, "m0btss", &m0_btss_ops,
+				 fw_name, sizeof(*desc));
+	if (!rproc) {
+		dev_err(dev, "failed to allocate rproc\n");
+		return -ENOMEM;
+	}
+
+	desc = rproc->priv;
+	desc->dev = dev;
+
+	ret = m0_btss_alloc_memory_region(desc);
+	if (ret)
+		return ret;
+
+	lpo_clk = devm_clk_get_enabled(dev, "btss_lpo_clk");
+	if (IS_ERR(lpo_clk))
+		return dev_err_probe(dev, PTR_ERR(lpo_clk),
+				     "Failed to get lpo clock\n");
+
+	desc->btss_reset = devm_reset_control_get(dev, "btss_reset");

Please use devm_reset_control_get_exclusive() directly.

quoted

+	if (IS_ERR_OR_NULL(desc->btss_reset))
+		return dev_err_probe(dev, PTR_ERR(desc->btss_reset),
+				     "unable to acquire btss_reset\n");
+
+	ret = reset_control_deassert(desc->btss_reset);
+	if (ret)
+		return dev_err_probe(rproc->dev.parent, ret,
+				     "Failed to deassert reset\n");

Shouldn't this be asserted on remove? Otherwise after an unbind/bind
cycle probe will enable the clock with reset already deasserted.
That may or may not be problematic.

Consider using devm_reset_control_get_exclusive_deasserted().


regards
Philipp

Regards,
George