Message-ID: <CABb+yY136TROzy92mQ7G50ZG0BcfwwbEYpW-V2XVVCigVtNHug@mail.gmail.com>
Date: Sat, 20 Dec 2025 17:25:50 -0600
From: Jassi Brar <jassisinghbrar@...il.com>
To: Douglas Anderson <dianders@...omium.org>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mailbox: Document the behavior of mbox_send_message() w/
 NULL mssg

On Thu, Dec 18, 2025 at 3:40 PM Douglas Anderson <dianders@...omium.org> wrote:
>
> The way the mailbox core behaves when you pass a NULL `mssg` parameter
> to mbox_send_message() seems a little questionable. Specifically, the
> mailbox core stores the currently active message directly in its
> `active_req` field. In at least two places it decides that if this
> field is `NULL` then there is no active request. That means if `mssg`
> is NULL it will always think there is no active request. The two
> places where it does this are:
>
> 1. If a client calls mbox_send_message(), if `active_req` is NULL then
>    it will call the mailbox controller to send the new message even if
>    the mailbox controller hasn't yet called mbox_chan_txdone() on the
>    previous (NULL) message.
> 2. The mailbox core will never call the client's `tx_done()` callback
>    with a NULL message because `tx_tick()` returns early whenever the
>    message is NULL.
>
> The above could be seen as bugs and perhaps could be fixed. However,
> doing a `git grep mbox_send_message.*NULL` shows 14 hits in mainline
> today and people may be relying on the current behavior. It is,
> perhaps, better to accept the current behavior.
>
> The current behavior can actually serve the purpose of providing a
> simple way to assert an edge-triggered interrupt to the remote
> processor on the other side of the mailbox. Specifically:
>
> 1. Like a normal edge-triggered interrupt, if multiple edges arrive
>    before the interrupt is Acked they are coalesced.
> 2. Like a normal edge-triggered interrupt, as long as the receiver
>    (the remote processor in this case) "Ack"s the interrupt _before_
>    checking for work and the sender (the mailbox client in this case)
>    posts the interrupt _after_ adding new work then we can always be
>    certain that new work will be noticed. This assumes that the
>    mailbox clienut and remote processor have some out-of-band way to
>    communicate work and the mailbox is just being used as an
>    interrupt.
>
> Document the current behavior so that people can rely on it and know
> that it will keep working the same way.
>
> NOTE: if a given mailbox client mixes and matches some NULL and some
> non-NULL messages, things could get loopy without additional code
> changes and rules. Without code changes, if we transfer a non-NULL
> message then we'd stop coalescing future NULL messages until the queue
> clears. Also: if we were transferring a NULL message and a non-NULL
> came in, we'd send it right away but potentially report `tx_done()`
> too early. For now, document mixing and matching NULL and non-NULL
> messages as undefined.
>
So we are talking about  'doorbell' type controllers that don't transfer any
data but only raise an irq to the other end. Ideally the client would pass the
doorbell info via mssg in runtime, but there may be a driver that does not need
that info from the client. I believe some doorbell clients do call
 mbox_send_message(chan, 0) , the arbitrary mssg 0 is unused by the driver
and everything works fine.
 I agree that is not ideal - we should differentiate between nothing and
'null' data. Is that the issue you want addressed?

Thanks.

Powered by blists - more mailing lists