Re: [PATCHv5 2/4] mailbox: Introduce framework for mailbox

[Matt Porter <mporter@linaro.org>]

On Fri, May 16, 2014 at 07:03:25PM +0530, Jassi Brar wrote:
> On 15 May 2014 19:57, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thursday 15 May 2014 11:41:00 Jassi Brar wrote:

...

> >> +struct mbox_controller {
> >> +     struct device *dev;
> >> +     struct mbox_chan_ops *ops;
> >> +     struct mbox_chan *chans;
> >> +     int num_chans;
> >> +     bool txdone_irq;
> >> +     bool txdone_poll;
> >> +     unsigned txpoll_period;
> >> +     struct mbox_chan *(*of_xlate)(struct mbox_controller *mbox,
> >> +                                     const struct of_phandle_args *sp);
> >> +     /*
> >> +      * If the controller supports only TXDONE_BY_POLL,
> >> +      * this timer polls all the links for txdone.
> >> +      */
> >> +     struct timer_list poll;
> >> +     unsigned period;
> >> +     /* Hook to add to the global controller list */
> >> +     struct list_head node;
> >> +} __aligned(32);
> >
> > What is the __aligned(32) for?
> >
> Attempt to align access to mailbox?
> 
> I am still open to opinion about whether the mailbox ownership should
> be exclusive or shared among clients. Need to handle async messages
> from remote is one reason one might want more than one client to own a
> channel. Allowing for RX via notifiers might be one option but that
> breaks semantics of 'ownership' of a mailbox channel.

This is the case we have on a new family of Broadcom SoCs. One mailbox
channel is the "system" channel and is shared amongst many clients for
reception of unsolicited events from the coprocessor. The same channel
is also used commonly in drivers for debug/inspection of the M0 state.
Functionality for clock, pmu, pinmux, and cpu idle/suspend is implemented
via IPC with the M0 so all of those drivers need to share one mailbox.

There's a lot of common code necessary to construct/parse IPCs for
each of the drivers. I suppose this IPC driver/api could be the
sole owner of that system channel. However, we'd need to develop some
binding to represent IPC resources that devices need to operate. Coming
into this late...I wonder if I missed something about where these vendor
specific IPC layers should live and how, if it makes sense, they should
be represented in DT. In our case, the IPC is an integral part of the
hardware as it's loaded from ROM.

-Matt