Re: [PATCH v3 01/15] mailbox: Deprecate NULL mbox messages; Introduce mbox_ring_doorbell()

[Doug Anderson]

Hi,

On Mon, Mar 9, 2026 at 8:24 PM Jassi Brar <jassisinghbrar@xxxxxxxxx> wrote:
>
> Hi Doug,
>
> On Mon, Feb 16, 2026 at 12:11 PM Douglas Anderson <dianders@xxxxxxxxxxxx> wrote:
> > @@ -249,6 +255,28 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg)
> >         if (!chan || !chan->cl)
> >                 return -EINVAL;
> >
> > +       /*
> > +        * The mailbox core gets confused when mbox_send_message() is called
> > +        * with NULL messages since the code directly stores messages in
> > +        * `active_req` and assumes that a NULL `active_req` means no request
> > +        * is active. This causes the core to call the mailbox controller a
> > +        * second time even if the previous message hasn't finished and also
> > +        * means the client's tx_done() callback will never be called. However,
> > +        * clients historically passed NULL anyway. Deprecate passing NULL
> > +        * here by adding a warning.
> > +        *
> > +        * Clients who don't have a message should switch to using
> > +        * mbox_ring_doorbell(), which explicitly documents the immediate
> > +        * sending of doorbells, the lack of txdone, and what happens if you
> > +        * mix doorbells and normal messages.
> > +        *
> > +        * TODO: when it's certain that all clients have transitioned, consider
> > +        * changing this to return -EINVAL.
> > +        */
> > +       if (!mssg)
> > +               dev_warn_once(chan->mbox->dev,
> > +                             "NULL mbox messages are deprecated; use mbox_ring_doorbell\n");
> > +
> Does this patchset leave some such clients out?  If not, should we
> drop this block and return -EINVAL already?

This patchset series got all the clients that were in mainline at the
time I last checked. However, it was unclear to me if all the patches
would all go through your tree or not, since they don't touch mailbox
drivers but the somewhat widespread places that were clients of
mailbox.

If all the patches aren't going through your tree, we'll need to keep
it like this until we can confirm all of the clients have been
updated. If all the patcheds are going through your tree, then we
could make it return -EINVAL right away, but it we'd have to do that
as a last patch in the series. I think it would still make sense for
the first patch (which adds the mbox_ring_doorbell() call) to have a
warning like this and then a final patch to switch to -EINVAL. That
keeps things bisectable.

What do you think?

I'm happy to post a patch atop my series that switches it to -EINVAL
and you can land it whenever you see fit. Would that work?

-Doug