Re: [PATCH] irqchip/qcom-mpm: Fix missing mailbox TX done acknowledgment

[Jassi Brar]

On Tue, Mar 10, 2026 at 10:22 AM Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
>
> Hi,
>
> On Mon, Mar 9, 2026 at 7:23 PM <jassisinghbrar@xxxxxxxxx> wrote:
> >
> > From: Jassi Brar <jassisinghbrar@xxxxxxxxx>
> >
> > The mbox_client for qcom-mpm sends NULL doorbell messages via
> > mbox_send_message() but never signals TX completion.
> > Set knows_txdone=true and call mbox_client_txdone() after a
> > successful send, matching the pattern used by other Qualcomm
> > mailbox clients (smp2p, smsm, qcom_aoss etc) of similar controller.
> >
> > Fixes: a6199bb514d8a6 "irqchip: Add Qualcomm MPM controller driver"
> > Signed-off-by: Jassi Brar <jassisinghbrar@xxxxxxxxx>
> > ---
> >  drivers/irqchip/irq-qcom-mpm.c | 3 +++
> >  1 file changed, 3 insertions(+)
>
> It's up to you, but according to all the research I did w/ NULL
> messages, the mbox_client_txdone() didn't really do anything useful in
> this case so we don't _really_ need to add it. The fact that it
> historically did nothing is one reason why the new
> mbox_ring_doorbell() series explicitly documents that you need not
> (and, ideally, should not) call txdone() for doorbells.
>
> Specifically, mbox_client_txdone() will just call tx_tick(). That will
> set `chan->active_req` to NULL (it already was). It will call
> msg_submit() which likely doesn't do anything (since we don't queue
> NULL messages in normal situations). It will notice that `mssg` is
> NULL so it will return before calling tx_done() or signalling the
> completion.
>
> If we make this change, then I'll need to spin my mbox_ring_doorbell()
> series to delete the code. That's OK with me if that's what you want
> to do, but I don't see a lot of benefit.
>
This is the only driver that doesn't "do the right thing" by missing
mbox_client_txdone() while being one.
I came across it while looking into if/how we can make
mbox_send_message(NULL) work.
Our root problem is active_req uses NULL as an 'idle' marker (early
days when doorbell
clients weren't known). If active_req used some other marker, NULL
messages would work like any
other message without a new api, even in blocking mode with optional
tx-callbacks respected.

For example, zynqmp-ipi-mailbox.c has 'txdone_poll = true' so
zynqmp_power.c can not use this doorbell api -
it needs to block on hrtimer based polling (which never happens when
message is NULL). Such clients still need a solution.

One option is to use the sentinel value ((void*) -1) internally.  The
only catch is then it will no longer be
a legitimate message, though I don't see any client using it (for
example as a bitmask). I personally think that is a
good 'tradeoff' for fixing the existing api without causing churn. I
would appreciate your take on the
 RFC  https://lkml.org/lkml/2026/3/10/2378

Regards
Jassi