[PATCH] firmware: arm_scmi: Queue in scmi layer for mailbox implementation

[Cristian Marussi]

On Fri, Oct 04, 2024 at 03:12:57PM -0700, Justin Chen wrote:
> The mailbox layer has its own queue. However this confuses the per
> message timeouts since the clock starts ticking the moment the messages
> get queued up. So all messages in the queue have there timeout clocks
> ticking instead of only the message inflight. To fix this, lets move the
> queue back into the SCMI layer.

Hi Justin,

thanks for reporting this, I'll try to understand better in the
following since we always avoided a lock in the mailbox driver (till
now) since it seemed not to be needed due to the interaction and inner
workings of the mailbox subsytem and its queues....so I just want to
understand better...not saying that this patch is wrong by itself for
now...

when you say:

"this confuses the per-message timeouts since the clock starts ticking
the moment the message get queued up."

...you are referring to the core mailbox subsystem queues and the code flow
that derives from mbox_send_message() right ?

So roughly the mailbox subsystem callchain involved would be this:

mbox_send_message()
	add_to_rbuf()
	msg_submit()

Am I right ? Is this the code flow you are referring to m?

In this case I am not sure what you mean with "the timer starts ticking
as soon as the message is queued"...which timer ? ... because:

 - the SCMI mailbox client is registered as "tx_block = false" so the
   sent messages are never waited for with wait_for_completion

and on the other side

 - once we have 1 message in-flight already, the next attempt to send a
   message will result in a call to add_to_rbuf() BUT the following
   msg_submit() will bailout immediately at line 63

	if (!chan->msg_count || chan->active_req)
		goto exit;

   after the first message is queued since there is an active_req
   inflight, and so no more messages will be queued till the next
   tx_tick(), which is invoked only by us from the SCMI stack when the
   reply to the inflight message has been read back. (this also means
   tx_prepare is NOT called until we kick from SCMI MBOX driver with
   mbox_client_txdone()

What I am missing, here ?

Thanks,
Cristian