Message-ID: <20251222105640.1097766-1-joonwonkang@google.com>
Date: Mon, 22 Dec 2025 10:56:37 +0000
From: Joonwon Kang <joonwonkang@...gle.com>
To: jassisinghbrar@...il.com
Cc: jonathanh@...dia.com, joonwonkang@...gle.com, linux-kernel@...r.kernel.org, 
	sudeep.holla@....com, thierry.reding@...il.com, lee@...nel.org
Subject: Re: [PATCH] mailbox: Allow NULL message sending

> On Mon, Dec 8, 2025 at 12:51 AM Joonwon Kang <joonwonkang@...gle.com> wrote:
> >
> > On Wed, Dec 3, 2025 at 11:57 PM Joonwon Kang <joonwonkang@...gle.com> wrote:
> > > >
> > > > On Tue, Nov 25, 2025 at 11:00 PM Joonwon Kang <joonwonkang@...gle.com> wrote:
> > > > > >
> > > > > > Clients may want to send interrupt only without any useful message
> > > > > > involved. Since the current mailbox framework does not allow NULL
> > > > > > message sending(although it allows receiving it), the clients should
> > > > > > allocate a dummy message buffer and pretend sending it. Besides, if
> > > > > > the mailbox controller calls `mbox_chan_txdone()` when the client
> > > > > > drivers happen to send NULL message anyway, it will result in unexpected
> > > > > > results by making the tx status messed up. This commit lifts the
> > > > > > limitation and allows the clients to send interrupt only without any
> > > > > > message buffer allocated.
> > > > > >
> > > > > Interrupts without data messages are called 'doorbells' and we already
> > > > > support them.
> > > > > thanks
> > > >
> > > > I am not sure if it is already supported. Let me draw two cases which imply
> > > > that it is not supported. If the cases make sense, could you reconsider the
> > > > patch? If it is supported in another branch, could you refer me to that
> > > > branch? I am currently referring to the `for-next` branch of your mailbox
> > > > repo.
> > > >
> > > I believe you are talking about some hypothetical situation?
> > > Otherwise, which controller is that?
> > > A controller driver is supposed to either expect data or not, but not both.
> >
> > I did not notice this controller's expectation since I could not find this info
> > in the API doc. So, now I believe that Case 2 could be seen as a hypothetical
> > situation. However, what about Case 1? If the message to send is NULL,
> > `chan->cl->tx_done(chan->cl, mssg, r)` and `complete(&chan->tx_complete)` will
> > **never** be called from `tx_tick()`. It also means that there is no way for a
> > client to know that the sending is really done or not. Even though a controller
> > driver(I mean any typical controller, not a specific controller) calls
> > `mbox_chan_txdone()` after receiving the corresponding ACK interrupt from the
> > remote to the previously sent NULL message, the client will be blocking not
> > knowing the sending is done.
> >
> For non-data channels (doorbells), the driver may expect the doorbell
> info at runtime
> via 'mssg'. If it doesn't, the clients send '0' or any arbitrary
> non-null value which remains
> unused by anyone.

By "the driver", I believe that you meant the "controller" driver. If so, yes
the controller driver may or, more importantly, may **not** expect data via
`mssg` as you clarified earlier: "A controller driver is supposed to either
expect data or not, but not both". Also, I think it is the controller driver's
responsibility to check if the `mssg` is NULL or not when it needs it, and the
mailbox "framework" should treat the data/message just as blackbox. In other
words, the framework should not enforce that any `mssg` to be sent should be
non-NULL. Currently, clients should send the unused data even when no data is
expected or should unconditionally believe that the tx is done and not use
blocking mode. This limitation could be lifted by this patch.

> Maybe the api can define some MAILBOX_NULL_DATA integer that
> can be used instead because the controller driver anyway doesn't need it.

I guess you suggested an alternative here. If we add such integer to the client
API, however, it will be a big change to the clients since now they should be
aware of the additional contract on the new integer when sending a doorbell:
they should go with `mbox_send_message(.*, MAILBOX_NULL_DATA)`. Instead, it
will be better if clients could just call `mbox_send_message(.*, NULL)` without
having that additional API requirement.

Or if you meant instead to add the new integer but hide it from the clients,
I think I can upload another patch for that, but it may be equivalent to the
solution in this patch. Could you help me understand the benefit your
alternative has over this patch, if you meant this direction?

> If/when the doorbell is rung is upto the controller - it may be just
> setting a bit or observing
> a status bit for the remote or whatever. The driver must say tx_done()
> appropriately.

If you meant here that the doorbell sending status is assumed to be checked
only by polling, it does not sound generic enough to me. If a device supports
ACK "interrupt" for the doorbell, I think that the mailbox framework should
support it since the mailbox API already claims to support both IRQ and polling
for tx done: TXDONE_BY_IRQ and TXDONE_BY_POLL.

> Again, it will help if we talk about the controller you have in mind exactly.

Sorry that I am working on company private mailbox controller drivers which
have not been upstreamed. What I can say is that the mailbox devices support
TXDONE_BY_IRQ, not TXDONE_BY_POLL, and is to send doorbell. Since NULL message
sending is currently not supported on the framework, we may have to enforce
clients to send unused non-NULL data, which is not practically possible since
the clients are out of our control, or ignore the ACK interrupt entirely and
enforce clients not to use blocking mode, which is also not practically
possible and not good for system stability. I believe the case of TXDONE_BY_IRQ
+ doorbell is generic enough, not exceptional, and should be supported on the
current mailbox framework. If `mbox_send_message(.*, NULL)` just works out, the
problems will be resolved.

Thanks!

Powered by blists - more mailing lists