Re: [PATCH] RFC: mailbox: Fix NULL message support in mbox_send_message()

[Joonwon Kang]

> The active_req field serves double duty as both the "is a TX in
> flight" flag (NULL means idle) and the storage for the in-flight
> message pointer. When a client sends NULL via mbox_send_message(),
> active_req is set to NULL, which the framework misinterprets as
> "no active request." This breaks the TX state machine by:
> 
>  - tx_tick() short-circuits on (!mssg), skipping the tx_done
>    callback and the tx_complete completion
>  - txdone_hrtimer() skips the channel entirely since active_req
>    is NULL, so poll-based TX-done detection never fires.
> 
> Fix this by introducing a MBOX_NO_MSG sentinel value that means
> "no active request," freeing NULL to be valid message data. The
> sentinel is internal to the mailbox core and is never exposed to
> controller drivers or clients.

The following drivers are currently using ->active_req which now could be
assigned MBOX_NO_MSG.
- drivers/mailbox/tegra-hsp.c
- drivers/mailbox/mtk-vcp-mailbox.c

One of them is using ->active_req to wait until the channel is empty. In
this case, strictly speaking, that controller driver should be aware of
the sentinel value MBOX_NO_MSG, which means the sentinel value should be
exposed to the controller. Or, if a future controller driver to come is to
use ->active_req for the same purpose for doorbell or non-doorbell, it
should also be aware of the sentinel value anyway.

However, I believe that it is not intuitive to the controller developers
that a pointer value could be other value than a real memory address,
NULL or error encoded value, which is -1(== MBOX_NO_MSG). For this reason,
I think it will be better to change the type of ->active_req to give a
better indication to the controller developers, e.g. to integer as in the
original patch
https://lore.kernel.org/all/20251126045926.2413532-1-joonwonkang@xxxxxxxxxx/.

Or, we could change those drivers not to use ->active_req, hide
->active_req entirely in the mailbox core and keep this patch.

Thanks.

> 
> The only tradeoff is that 'MBOX_NO_MSG' can not be used as a message
> by clients.