[PATCH] rpmsg: glink: Split protocol start out of native_probe

[PATCH] 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 after qcom_glink_native_probe()
returns. Since native_probe() sends the initial version command, the
remote may ACK before the local IRQ is enabled, stalling the version
handshake on a fast remote.

Split the protocol start (qcom_glink_send_version() and
qcom_glink_create_chrdev()) into a new qcom_glink_native_start().
Transports now enable their IRQ first, then call native_start(), so the
version ACK is guaranteed to be serviced.

Signed-off-by: Chunkai Deng <chunkai.deng@oss.qualcomm.com>
---
 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);
 

---
base-commit: a225caacc36546a09586e3ece36c0313146e7da9
change-id: 20260604-rpmsg-glink-split-protocol-start-3df74dbd5c94

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

Re: [PATCH] rpmsg: glink: Split protocol start out of native_probe

[Konrad Dybcio]

On 6/19/26 7:01 AM, Chunkai Deng wrote:
> The SMEM and RPM transports request their receive interrupt with
> IRQF_NO_AUTOEN and enable it only after qcom_glink_native_probe()
> returns. Since native_probe() sends the initial version command, the
> remote may ACK before the local IRQ is enabled, stalling the version
> handshake on a fast remote.
> 
> Split the protocol start (qcom_glink_send_version() and
> qcom_glink_create_chrdev()) into a new qcom_glink_native_start().
> Transports now enable their IRQ first, then call native_start(), so the
> version ACK is guaranteed to be serviced.
> 
> Signed-off-by: Chunkai Deng <chunkai.deng@oss.qualcomm.com>
> ---

Would it not be simpler to just move the devm_request_irq a bit down
and remove the NO_AUTOEN flag then?

Konrad

Re: [PATCH] rpmsg: glink: Split protocol start out of native_probe

[Chunkai Deng]

On 6/29/2026 8:47 PM, Konrad Dybcio wrote:

> On 6/19/26 7:01 AM, Chunkai Deng wrote:
>> The SMEM and RPM transports request their receive interrupt with
>> IRQF_NO_AUTOEN and enable it only after qcom_glink_native_probe()
>> returns. Since native_probe() sends the initial version command, the
>> remote may ACK before the local IRQ is enabled, stalling the version
>> handshake on a fast remote.
>>
>> Split the protocol start (qcom_glink_send_version() and
>> qcom_glink_create_chrdev()) into a new qcom_glink_native_start().
>> Transports now enable their IRQ first, then call native_start(), so the
>> version ACK is guaranteed to be serviced.
>>
>> Signed-off-by: Chunkai Deng <chunkai.deng@oss.qualcomm.com>
>> ---
> Would it not be simpler to just move the devm_request_irq a bit down
> and remove the NO_AUTOEN flag then?

Thanks for the suggestion.
I analyzed and verified it locally,but I'd personally still lean towards
keeping the current change as-is. My reasoning:

(1) The IRQ must be live before the version handshake — that's the core
point that can't change. The version command is sent from native_start(),
so we need to request/enable the IRQ first and only then call
native_start(). That ordering is only possible with the version send
kept out of native_probe(), so the split needs to stay.

(2) Dropping IRQF_NO_AUTOEN does make the structure a little simpler,
and functionally it's equivalent, but I'd lean towards keeping the
request-disabled + explicit enable_irq() form. It makes the ordering
constraint visible at the call site: the IRQ is requested up front,
and enable_irq() marks the exact point where the handler becomes safe
to run(glink assigned) and where we want RX servicing to begin. Folding
that into an implicit auto-enable hides that intent, and I think the
explicit form is easier to maintain.

That said, if you'd prefer the simpler form, I'm happy to do the IRQ
cleanup as a separate follow-up patch on top of this fix rather than
mixing the refactor into the race fix.

Best regards,
Chunkai Deng

Re: [PATCH] rpmsg: glink: Split protocol start out of native_probe

[Konrad Dybcio]

On 7/6/26 10:59 AM, Chunkai Deng wrote:
> On 6/29/2026 8:47 PM, Konrad Dybcio wrote:
> 
>> On 6/19/26 7:01 AM, Chunkai Deng wrote:
>>> The SMEM and RPM transports request their receive interrupt with
>>> IRQF_NO_AUTOEN and enable it only after qcom_glink_native_probe()
>>> returns. Since native_probe() sends the initial version command, the
>>> remote may ACK before the local IRQ is enabled, stalling the version
>>> handshake on a fast remote.
>>>
>>> Split the protocol start (qcom_glink_send_version() and
>>> qcom_glink_create_chrdev()) into a new qcom_glink_native_start().
>>> Transports now enable their IRQ first, then call native_start(), so the
>>> version ACK is guaranteed to be serviced.
>>>
>>> Signed-off-by: Chunkai Deng <chunkai.deng@oss.qualcomm.com>
>>> ---
>> Would it not be simpler to just move the devm_request_irq a bit down
>> and remove the NO_AUTOEN flag then?
> 
> Thanks for the suggestion.
> I analyzed and verified it locally,but I'd personally still lean towards
> keeping the current change as-is. My reasoning:
> 
> (1) The IRQ must be live before the version handshake — that's the core
> point that can't change. The version command is sent from native_start(),
> so we need to request/enable the IRQ first and only then call
> native_start(). That ordering is only possible with the version send
> kept out of native_probe(), so the split needs to stay.
> 
> (2) Dropping IRQF_NO_AUTOEN does make the structure a little simpler,
> and functionally it's equivalent, but I'd lean towards keeping the
> request-disabled + explicit enable_irq() form. It makes the ordering
> constraint visible at the call site: the IRQ is requested up front,
> and enable_irq() marks the exact point where the handler becomes safe
> to run(glink assigned) and where we want RX servicing to begin. Folding
> that into an implicit auto-enable hides that intent, and I think the
> explicit form is easier to maintain.
> 
> That said, if you'd prefer the simpler form, I'm happy to do the IRQ
> cleanup as a separate follow-up patch on top of this fix rather than
> mixing the refactor into the race fix.

Please don't paste AI responses into your email client, there's a human
on the other side..

If this patch was also created with the use of a coding agent, it must
be disclosed and follow the rules established in:

Documentation/process/coding-assistants.rst

Konrad

Re: [PATCH] rpmsg: glink: Split protocol start out of native_probe

[Chunkai Deng]

On 7/17/2026 6:54 PM, Konrad Dybcio wrote:

> On 7/6/26 10:59 AM, Chunkai Deng wrote:
>> On 6/29/2026 8:47 PM, Konrad Dybcio wrote:
>>
>>> On 6/19/26 7:01 AM, Chunkai Deng wrote:
>>>> The SMEM and RPM transports request their receive interrupt with
>>>> IRQF_NO_AUTOEN and enable it only after qcom_glink_native_probe()
>>>> returns. Since native_probe() sends the initial version command, the
>>>> remote may ACK before the local IRQ is enabled, stalling the version
>>>> handshake on a fast remote.
>>>>
>>>> Split the protocol start (qcom_glink_send_version() and
>>>> qcom_glink_create_chrdev()) into a new qcom_glink_native_start().
>>>> Transports now enable their IRQ first, then call native_start(), so the
>>>> version ACK is guaranteed to be serviced.
>>>>
>>>> Signed-off-by: Chunkai Deng <chunkai.deng@oss.qualcomm.com>
>>>> ---
>>> Would it not be simpler to just move the devm_request_irq a bit down
>>> and remove the NO_AUTOEN flag then?
>> Thanks for the suggestion.
>> I analyzed and verified it locally,but I'd personally still lean towards
>> keeping the current change as-is. My reasoning:
>>
>> (1) The IRQ must be live before the version handshake — that's the core
>> point that can't change. The version command is sent from native_start(),
>> so we need to request/enable the IRQ first and only then call
>> native_start(). That ordering is only possible with the version send
>> kept out of native_probe(), so the split needs to stay.
>>
>> (2) Dropping IRQF_NO_AUTOEN does make the structure a little simpler,
>> and functionally it's equivalent, but I'd lean towards keeping the
>> request-disabled + explicit enable_irq() form. It makes the ordering
>> constraint visible at the call site: the IRQ is requested up front,
>> and enable_irq() marks the exact point where the handler becomes safe
>> to run(glink assigned) and where we want RX servicing to begin. Folding
>> that into an implicit auto-enable hides that intent, and I think the
>> explicit form is easier to maintain.
>>
>> That said, if you'd prefer the simpler form, I'm happy to do the IRQ
>> cleanup as a separate follow-up patch on top of this fix rather than
>> mixing the refactor into the race fix.
> Please don't paste AI responses into your email client, there's a human
> on the other side..
>
> If this patch was also created with the use of a coding agent, it must
> be disclosed and follow the rules established in:
>
> Documentation/process/coding-assistants.rst

Understood, and sorry about that. I'll pay more attention to this going forward.
I did use AI tools to help with some of the analysis, but thatwas my initial view from the beginning. The potential issue here is that send_version() run before the IRQ is enabled. If the send_version ACK arrives during that window, it may not be handled. While removing IRQF_NO_AUTOEN and rearranging the devm_request_irq may simplify things a bit, it doesn't really address that race.
Thanks, Chunkai Deng