Re: [PATCH v4 2/2] dt-bindings: mailbox: Add Xilinx IPI Mailbox

[Wendy Liang]

On Wed, Oct 10, 2018 at 2:59 AM Sudeep Holla <sudeep.holla@xxxxxxx> wrote:
>
> On Wed, Oct 10, 2018 at 12:18:32AM -0700, Wendy Liang wrote:
> > Xilinx ZynqMP IPI(Inter Processor Interrupt) is a hardware block
> > in ZynqMP SoC used for the communication between various processor
> > systems.
> >
> > Signed-off-by: Wendy Liang <wendy.liang@xxxxxxxxxx>
>
> [...]
>
> > +Optional properties:
> > +--------------------
> > +- method:              The method of accessing the IPI agent registers.
> > +                       Permitted values are: "smc" and "hvc". Default is
> > +                       "smc".
>
> You are mixing the hardware messaging based mailbox and the software
> "smc/hvc" based mailbox together here. Please keep them separated.
> IIUC smc/hvc based mailcox is used for "tx" or too keep it simple in
> one direction and hardware based is used for "rx" or the other direction
> for communication.
>
Hi Sudeep,

Thanks for your comments.

The IPI hardware block has both buffers and registers. The hardware
block has dedicated
buffers for each mailboxes, and thus, in the implementation, we directly access
the buffers from IPI driver. However, the controller registers are
shared between
mailboxes in the hardware, as the ATF will also access the registers,
we need to use
SMC/HVC to access the registers (control or ISR). And the SMC/HVC here is for
the register access.

I am not clear on smc/hvc based mailbox is used for tx, and hardware
based is used
for  "rx". As for both TX and RX, we need to write/read the registers
(through SMC) and
write/read the buffers provided by the IPI hardware block directly.

Thanks,
Wendy

> You *should not* mix them as single unit. Also lots of other vendor need
> SMC/HVC based mailbox. So make it generic and keep it separate.
>
> --
> Regards,
> Sudeep