[PATCH v2 0/2] rpmsg: glink: Fix version handshake race against a late RX interrupt

[PATCH v2 0/2] rpmsg: glink: Fix version handshake race against a late RX interrupt

[Chunkai Deng]

The glink transports enable their receive interrupt only after
qcom_glink_native_probe() has returned, while the version command is sent
from within native_probe(). A remote that answers quickly can have its
version ACK dropped, and the handshake never completes.

Patch 1 moves the version command out into a new
qcom_glink_native_start(), which the transports call once their interrupt
is live. Patch 2 drops IRQF_NO_AUTOEN, which is no longer needed once the
ordering is explicit.

---
Changes in v2:
- Drop IRQF_NO_AUTOEN in a second patch, as Konrad suggested on v1.
- Keep a failing chrdev registration non-fatal in native_start() and note
  it in the kernel-doc; v1 turned it into a probe failure by mistake.
- Add Assisted-by tags per Documentation/process/coding-assistants.rst.
- Link to v1: https://patch.msgid.link/20260618-rpmsg-glink-split-protocol-start-v1-1-c4f93986cdb4@oss.qualcomm.com

---
Chunkai Deng (2):
      rpmsg: glink: Split protocol start out of native_probe
      rpmsg: glink: Request the RX interrupt already enabled

 drivers/rpmsg/qcom_glink_native.c | 32 ++++++++++++++++++++++++++++----
 drivers/rpmsg/qcom_glink_native.h |  1 +
 drivers/rpmsg/qcom_glink_rpm.c    | 30 ++++++++++++++++++++----------
 drivers/rpmsg/qcom_glink_smem.c   | 27 +++++++++++++++++----------
 4 files changed, 66 insertions(+), 24 deletions(-)
---
base-commit: a225caacc36546a09586e3ece36c0313146e7da9
change-id: 20260604-rpmsg-glink-split-protocol-start-3df74dbd5c94

Best regards,
--  
Chunkai Deng <chunkai.deng@oss.qualcomm.com>

[PATCH v2 1/2] rpmsg: glink: Split protocol start out of native_probe

[Chunkai Deng]

The SMEM and RPM transports request their receive interrupt with
IRQF_NO_AUTOEN and enable it only once qcom_glink_native_probe() has
returned. But native_probe() sends the version command, so on a fast
remote the version ACK can land while the interrupt is still masked. The
ACK is dropped and the handshake never completes.

Move the version command and the chrdev registration into a new
qcom_glink_native_start(), leaving native_probe() to set up the glink
instance. Both transports enable their interrupt before calling
native_start().

Signed-off-by: Chunkai Deng <chunkai.deng@oss.qualcomm.com>
Assisted-by: Claude:claude-opus-5
---
 drivers/rpmsg/qcom_glink_native.c | 32 ++++++++++++++++++++++++++++----
 drivers/rpmsg/qcom_glink_native.h |  1 +
 drivers/rpmsg/qcom_glink_rpm.c    |  8 ++++++++
 drivers/rpmsg/qcom_glink_smem.c   |  8 ++++++++
 4 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index d9d4468e4cbd..2a284b22a037 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -1928,17 +1928,41 @@ struct qcom_glink *qcom_glink_native_probe(struct device *dev,
 	if (ret)
 		dev_err(dev, "failed to add groups\n");
 
+	return glink;
+}
+EXPORT_SYMBOL_GPL(qcom_glink_native_probe);
+
+/**
+ * qcom_glink_native_start() - start the GLINK protocol handshake
+ * @glink:	glink handle returned by qcom_glink_native_probe()
+ *
+ * Send the initial version command and register the chrdev. This is split
+ * out from qcom_glink_native_probe() so that a transport can enable its
+ * receive interrupt before the version handshake is initiated, ensuring the
+ * version ACK from the remote is not missed.
+ *
+ * Failure to register the chrdev is not fatal and only logged, matching the
+ * previous behaviour of qcom_glink_native_probe().
+ *
+ * Return: 0 on success, negative errno if sending the version command failed.
+ */
+int qcom_glink_native_start(struct qcom_glink *glink)
+{
+	int ret;
+
 	ret = qcom_glink_send_version(glink);
-	if (ret)
-		return ERR_PTR(ret);
+	if (ret) {
+		dev_err(glink->dev, "failed to send version: %d\n", ret);
+		return ret;
+	}
 
 	ret = qcom_glink_create_chrdev(glink);
 	if (ret)
 		dev_err(glink->dev, "failed to register chrdev\n");
 
-	return glink;
+	return 0;
 }
-EXPORT_SYMBOL_GPL(qcom_glink_native_probe);
+EXPORT_SYMBOL_GPL(qcom_glink_native_start);
 
 static int qcom_glink_remove_device(struct device *dev, void *data)
 {
diff --git a/drivers/rpmsg/qcom_glink_native.h b/drivers/rpmsg/qcom_glink_native.h
index 8dbec24de23e..783209980c3a 100644
--- a/drivers/rpmsg/qcom_glink_native.h
+++ b/drivers/rpmsg/qcom_glink_native.h
@@ -35,6 +35,7 @@ struct qcom_glink *qcom_glink_native_probe(struct device *dev,
 					   struct qcom_glink_pipe *rx,
 					   struct qcom_glink_pipe *tx,
 					   bool intentless);
+int qcom_glink_native_start(struct qcom_glink *glink);
 void qcom_glink_native_remove(struct qcom_glink *glink);
 void qcom_glink_native_rx(struct qcom_glink *glink);
 
diff --git a/drivers/rpmsg/qcom_glink_rpm.c b/drivers/rpmsg/qcom_glink_rpm.c
index e3ba2c63a5fc..34f18c3e58c8 100644
--- a/drivers/rpmsg/qcom_glink_rpm.c
+++ b/drivers/rpmsg/qcom_glink_rpm.c
@@ -358,6 +358,14 @@ static int glink_rpm_probe(struct platform_device *pdev)
 
 	enable_irq(rpm->irq);
 
+	ret = qcom_glink_native_start(glink);
+	if (ret) {
+		disable_irq(rpm->irq);
+		qcom_glink_native_remove(glink);
+		mbox_free_channel(rpm->mbox_chan);
+		return ret;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/rpmsg/qcom_glink_smem.c b/drivers/rpmsg/qcom_glink_smem.c
index 62adc4db2317..28f6cfda6352 100644
--- a/drivers/rpmsg/qcom_glink_smem.c
+++ b/drivers/rpmsg/qcom_glink_smem.c
@@ -348,8 +348,16 @@ struct qcom_glink_smem *qcom_glink_smem_register(struct device *parent,
 
 	enable_irq(smem->irq);
 
+	ret = qcom_glink_native_start(glink);
+	if (ret)
+		goto err_disable_irq;
+
 	return smem;
 
+err_disable_irq:
+	disable_irq(smem->irq);
+	qcom_glink_native_remove(glink);
+
 err_free_mbox:
 	mbox_free_channel(smem->mbox_chan);
 

-- 
2.43.0

[PATCH v2 2/2] rpmsg: glink: Request the RX interrupt already enabled

[Chunkai Deng]

The receive interrupt is requested masked early in probe and enabled
further down, once the glink instance has been stored in the transport.
Now that the version command is no longer sent from
qcom_glink_native_probe(), nothing needs to happen between those two
points.

Request the interrupt where it used to be enabled, and drop
IRQF_NO_AUTOEN along with the enable_irq() call. The RPM error path
becomes a set of labels, the way SMEM already unwinds.

Signed-off-by: Chunkai Deng <chunkai.deng@oss.qualcomm.com>
Assisted-by: Claude:claude-opus-5
---
 drivers/rpmsg/qcom_glink_rpm.c  | 34 ++++++++++++++++++----------------
 drivers/rpmsg/qcom_glink_smem.c | 19 +++++++++----------
 2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/drivers/rpmsg/qcom_glink_rpm.c b/drivers/rpmsg/qcom_glink_rpm.c
index 34f18c3e58c8..a85d78f8283e 100644
--- a/drivers/rpmsg/qcom_glink_rpm.c
+++ b/drivers/rpmsg/qcom_glink_rpm.c
@@ -316,15 +316,6 @@ static int glink_rpm_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	rpm->irq = of_irq_get(dev->of_node, 0);
-	ret = devm_request_irq(dev, rpm->irq, qcom_glink_rpm_intr,
-			       IRQF_NO_SUSPEND | IRQF_NO_AUTOEN,
-			       "glink-rpm", rpm);
-	if (ret) {
-		dev_err(dev, "failed to request IRQ\n");
-		return ret;
-	}
-
 	rpm->mbox_client.dev = dev;
 	rpm->mbox_client.knows_txdone = true;
 	rpm->mbox_chan = mbox_request_channel(&rpm->mbox_client, 0);
@@ -356,17 +347,28 @@ static int glink_rpm_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, rpm);
 
-	enable_irq(rpm->irq);
-
-	ret = qcom_glink_native_start(glink);
+	rpm->irq = of_irq_get(dev->of_node, 0);
+	ret = devm_request_irq(dev, rpm->irq, qcom_glink_rpm_intr,
+			       IRQF_NO_SUSPEND, "glink-rpm", rpm);
 	if (ret) {
-		disable_irq(rpm->irq);
-		qcom_glink_native_remove(glink);
-		mbox_free_channel(rpm->mbox_chan);
-		return ret;
+		dev_err(dev, "failed to request IRQ\n");
+		goto err_glink_remove;
 	}
 
+	ret = qcom_glink_native_start(glink);
+	if (ret)
+		goto err_disable_irq;
+
 	return 0;
+
+err_disable_irq:
+	disable_irq(rpm->irq);
+
+err_glink_remove:
+	qcom_glink_native_remove(glink);
+	mbox_free_channel(rpm->mbox_chan);
+
+	return ret;
 }
 
 static void glink_rpm_remove(struct platform_device *pdev)
diff --git a/drivers/rpmsg/qcom_glink_smem.c b/drivers/rpmsg/qcom_glink_smem.c
index 28f6cfda6352..2d6fa2d3a99b 100644
--- a/drivers/rpmsg/qcom_glink_smem.c
+++ b/drivers/rpmsg/qcom_glink_smem.c
@@ -304,15 +304,6 @@ struct qcom_glink_smem *qcom_glink_smem_register(struct device *parent,
 		goto err_put_dev;
 	}
 
-	smem->irq = of_irq_get(smem->dev.of_node, 0);
-	ret = devm_request_irq(&smem->dev, smem->irq, qcom_glink_smem_intr,
-			       IRQF_NO_SUSPEND | IRQF_NO_AUTOEN,
-			       "glink-smem", smem);
-	if (ret) {
-		dev_err(&smem->dev, "failed to request IRQ\n");
-		goto err_put_dev;
-	}
-
 	smem->mbox_client.dev = &smem->dev;
 	smem->mbox_client.knows_txdone = true;
 	smem->mbox_chan = mbox_request_channel(&smem->mbox_client, 0);
@@ -346,7 +337,13 @@ struct qcom_glink_smem *qcom_glink_smem_register(struct device *parent,
 
 	smem->glink = glink;
 
-	enable_irq(smem->irq);
+	smem->irq = of_irq_get(smem->dev.of_node, 0);
+	ret = devm_request_irq(&smem->dev, smem->irq, qcom_glink_smem_intr,
+			       IRQF_NO_SUSPEND, "glink-smem", smem);
+	if (ret) {
+		dev_err(&smem->dev, "failed to request IRQ\n");
+		goto err_glink_remove;
+	}
 
 	ret = qcom_glink_native_start(glink);
 	if (ret)
@@ -356,6 +353,8 @@ struct qcom_glink_smem *qcom_glink_smem_register(struct device *parent,
 
 err_disable_irq:
 	disable_irq(smem->irq);
+
+err_glink_remove:
 	qcom_glink_native_remove(glink);
 
 err_free_mbox:

-- 
2.43.0