Re: [PATCH v5 1/3] dt-bindings: mailbox: add google,gs101-mbox

Hi, Jassi,

On 1/3/25 3:39 AM, Jassi Brar wrote:

quoted

quoted

Looking at v6, I prefer this version... maybe modify it a bit.

Just to summarize for the readers, in the end I chose for the
controllers to allow #mbox-cells = <0>; and for the clients to still use
the mboxes property, but just to reference the phandle to the controller:
        mboxes = <&ap2apm_mailbox>;

This was already supported, see drivers/mailbox/bcm2835-mailbox.c for example.

Thanks for the pointer. I was referring to the bindings patch:
https://lore.kernel.org/linux-arm-kernel/20241220-acpm-v4-upstream-mbox-v6-1-a6942806e52a@linaro.org/ (local)

quoted

Then I updated the mailbox core to allow clients to request channels by
passing some args containing channel identifiers to the controllers,
that the controllers xlate() using their own method.

This is unnecessary.
If you don't pass the doorbell number from DT, each channel populated
by the driver is just a s/w construct or a 'virtual' channel. Make use
of 'void *data'  in send_data() to specify the doorbell.

I think this introduces concurrency problems if the channel identifiers
passed by 'void *data' don't match the virtual channel used for sending
the messages. Do we want to allow this?

Also, if we use 'void *data' to pass channel identifiers, the channel
checks will have to be made at send_data() time. Thus if passing wrong
channel type for example, the mailbox client will eventually get a
-ENOBUFS and a "Try increasing MBOX_TX_QUEUE_LEN" message, which I find
misleading.

Thanks,
ta