Re: [PATCH] rpmsg: glink: Split protocol start out of native_probe
From: Chunkai Deng
Date: Mon Jul 20 2026 - 06:37:46 EST
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@xxxxxxxxxxxxxxxx>
>>>> ---
>>> 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