Re: [PATCH 7/7] mailbox: goog-mba: Introduce the goog-mba mailbox driver

[Krzysztof Kozlowski <krzk@kernel.org>]

On 28/09/2026 23:34, Doug Anderson wrote:
> 
> ```C
> static struct mbox_chan *goog_mba_fw_xlate(struct mbox_controller *mbox,
>                                       const struct fwnode_reference_args *sp)
> {
>   int i;
> 
>   if (sp->nargs)
>     return ERR_PTR(-EINVAL);
> 
>   for (i = 0; i < mbox->num_chans; i++) {
>     if (!mbox->chans[i].cl)
>       return &mbox->chans[i];
>   }
> 
>   return ERR_PTR(-EBUSY);
> }
> ```
> 
> Then the client loops around and requests the same channel over and
> over again until it gets -EBUSY? Like:
> 
> ```C
> for (i = 0; i < FIFO_MAX; i++) {
>   client
>   chan[i] = mbox_request_channel(client[i], TX_CHAN);
>   if (IS_ERR(chan[i]) && PTR_ERR(chan[i]) == -EBUSY)
>     break;
> }
> fifo_depth = i;
> ```
> 
> I _guess_ that works, but it still feels like a bit of a hack to me.
> You said you were worried about people abusing the "has_queue" API.
> The above feels like it's abusing the "fw_xlate" API, turning it from
> something that is normally a "lookup" into an allocator function.
> 
> +Rob, Saravana, and devicetree@vger.kernel.org. DT folks: is the above
> something that looks right to you?

There is no allocation in your xlate code above, so this is not that
terrible as we talked on LPC.

> 
> 
> I researched whether other upstream drivers use of_xlate() /
> fw_xlate() as an "allocator" like this. I did find "exynos-mailbox,"
> which appears to be doing something similar. However, upon deeper
> digging it seems like "exynos-mailbox" isn't using this dynamic
> allocation for any compelling reason. It looks like, really,
> "exynos-mailbox" should just be returning one channel. The client
> (exynos-acpm) could just use the same channel for everything since:
> * It actually gets the _real_ channel ID out of the data.
> * It doesn't care about txdone.
> * All it does is ring a doorbell and there's no queueing.
> 
> 
> In any case, if using fw_xlate() as an allocator is truly the only way
> to proceed, I'll finish my prototype and send a v2, but I'm still

Why fw_xlate() would be an allocator? Can you extend your code to show that?

I think doing any allocation in xlate() is calls is fundamentally wrong.
These should not modify the state of the device, so no allocations, no
device_link_add() etc.

Why? There is simply no corresponding xlate_destroy() call. It's also
confusing, because the meaning is to translate from one domain resource
to another, not perform actual resource allocation.

> skeptical that this is better than just adding queuing into the core.
> Speaking of which, I actually want to go back to something you said
> earlier. I asked a bit about this but I don't think I saw a response
> (sorry if I missed it!):

Best regards,
Krzysztof