Re: [PATCH 02/22] dt-bindings: Add bindings for Keem Bay IPC driver

[Rob Herring <robh@kernel.org>]

On Mon, Dec 07, 2020 at 06:42:07PM +0000, Daniele Alessandrelli wrote:
> Hi Rob,
> 
> Thanks for the feedback.
> 
> On Mon, 2020-12-07 at 10:01 -0600, Rob Herring wrote:
> > On Tue, Dec 01, 2020 at 02:34:51PM -0800, mgross@linux.intel.com wrote:
> > > From: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
> > > 
> > > Add DT binding documentation for the Intel Keem Bay IPC driver, which
> > > enables communication between the Computing Sub-System (CSS) and the
> > > Multimedia Sub-System (MSS) of the Intel Movidius SoC code named Keem
> > > Bay.
> > > 
> 
> [cut]
> 
> > > +
> > > +description:
> > > +  The Keem Bay IPC driver enables Inter-Processor Communication (IPC) with the
> > > +  Visual Processor Unit (VPU) embedded in the Intel Movidius SoC code named
> > > +  Keem Bay.
> > 
> > Sounds like a mailbox.
> 
> We did consider using the mailbox framework, but eventually decided
> against it; mainly because of the following two reasons:
> 
> 1. The channel concept in the Mailbox framework is different than the
>    channel concept in Keem Bay IPC:
> 
>    a. My understanding is that Mailbox channels are meant to be SW
>       representation of physical HW channels, while in Keem Bay IPC
>       channels are software abstractions to achieve communication
>       multiplexing over a single HW link
> 
>    b. Additionally, Keem Bay IPC has two different classes of channels 
>       (high-speed channels and general-purpose channels) that need to
>       access the same HW link with different priorities.
> 
> 2. The blocking / non-blocking TX behavior of mailbox channels is
>    defined at channel creation time (by the tx_block value of the
>    mailbox client passed to mbox_request_channel(); my understanding 
>    is that the tx_block value cannot be modified after the channel is
>    created), while in Keem Bay IPC the same channel can be used for
>    both blocking and non-blocking TX (behavior is controlled by the
>    timeout argument passed to keembay_ipc_send()).
> 
> Having said that, I guess that it could be possible to create a Mailbox
> driver implementing the core communication mechanism used by the Keem
> Bay IPC and then build our API around it (basically having two
> drivers). But I'm not sure that would make the code simpler or easier
> to maintain. Any thoughts on this?

The use of the mailbox binding and the kernel's mailbox framework are 
independent questions. I'm only concerned with the former (for purposes 
of this review).

Rob