Re: [PATCH 2/2] dt-bindings: Add DT bindings info for FlexRM mailbox driver

[Anup Patel]

On Thu, Dec 1, 2016 at 3:08 AM, Rob Herring <robh@xxxxxxxxxx> wrote:
> On Fri, Nov 25, 2016 at 10:05:51AM +0530, Anup Patel wrote:
>> This patch adds device tree bindings document for the FlexRM
>> mailbox driver.
>
> Bindings document h/w, not drivers.

OK, I will rephrase this.

>
>>
>> Reviewed-by: Ray Jui <ray.jui@xxxxxxxxxxxx>
>> Reviewed-by: Scott Branden <scott.branden@xxxxxxxxxxxx>
>> Signed-off-by: Anup Patel <anup.patel@xxxxxxxxxxxx>
>> ---
>>  .../bindings/mailbox/brcm,flexrm-mbox.txt          | 60 ++++++++++++++++++++++
>>  1 file changed, 60 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mailbox/brcm,flexrm-mbox.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mailbox/brcm,flexrm-mbox.txt b/Documentation/devicetree/bindings/mailbox/brcm,flexrm-mbox.txt
>> new file mode 100644
>> index 0000000..7969b1c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mailbox/brcm,flexrm-mbox.txt
>> @@ -0,0 +1,60 @@
>> +Broadcom FlexRM Mailbox Driver
>
> h/w

OK, I will rephrase this.

>
>> +===============================
>> +The Broadcom FlexRM ring manager provides a set of rings which can be
>> +used to submit work to offload engines. An SoC may have multiple FlexRM
>> +hardware blocks. There is one device tree entry per block. The FlexRM
>> +mailbox drivers creates a mailbox instance using FlexRM rings where
>> +each mailbox channel is a separate FlexRM ring.
>> +
>> +Required properties:
>> +--------------------
>> +- compatible:        Should be "brcm,flexrm-mbox"
>
> Sounds generic. Add SoC specific compatible strings please.

OK, will do.

>
>> +- reg:               Specifies base physical address and size of the FlexRM
>> +             ring registers
>> +- msi-parent:        Phandles (and potential Device IDs) to MSI controllers
>> +             The FlexRM engine will send MSIs (instead of wired
>> +             interrupts) to CPU. There is one MSI for each FlexRM ring.
>
> One ring is one h/w block, right? How many instances is not really
> relevant.

No, FlexRM is the HW block. One instance of FlexRM provides a set of
rings (typically 32 or more).

There are lot other registers in FlexRM apart from ring registers. Out of
these, only ring registers are accessible to non-secured world (i.e. Linux)
whereas all other registers are only accessible to secure-world firmware
(typically ATF).

>
>> +             Refer devicetree/bindings/interrupt-controller/msi.txt
>> +- #mbox-cells:       Specifies the number of cells needed to encode a mailbox
>> +             channel. This should be 3.
>> +
>> +             The 1st cell is the mailbox channel number.
>> +
>> +             The 2nd cell contains MSI completion threshold. This is the
>> +             number of completion messages for which FlexRM will inject
>> +             one MSI interrupt to CPU.
>> +
>> +             The 3nd cell contains MSI timer value representing time for
>> +             which FlexRM will wait to accumulate N completion messages
>> +             where N is the value specified by 2nd cell above. If FlexRM
>> +             does not get required number of completion messages in time
>> +             specified by this cell then it will inject one MSI interrupt
>> +             to CPU provided atleast one completion message is available.
>> +
>> +Optional properties:
>> +--------------------
>> +- dma-coherent:      Present if DMA operations made by the FlexRM engine (such
>> +             as DMA descriptor access, access to buffers pointed by DMA
>> +             descriptors and read/write pointer updates to DDR) are
>> +             cache coherent with the CPU.
>> +
>> +Example:
>> +--------
>> +crypto_mbox: mbox@67000000 {
>> +     compatible = "brcm,flexrm-mbox";
>> +     reg = <0x67000000 0x200000>;
>> +     msi-parent = <&gic_its 0x7f00>;
>> +     #mbox-cells = <3>;
>> +};
>> +
>> +crypto_client {
>
> Is this a h/w block or purely a driver on top of the mailbox? The latter
> doesn't belong in DT.

The FlexRM is one HW block. It provides ring-based programming
interface to various offload engines which are separate HW blocks.

The driver for FlexRM is implemented as mailbox controller driver
while the offload engine drivers will be mailbox clients.

>
>> +     ...
>> +     mboxes = <&crypto_mbox 0 0x1 0xffff>,
>> +              <&crypto_mbox 1 0x1 0xffff>,
>> +              <&crypto_mbox 16 0x1 0xffff>,
>> +              <&crypto_mbox 17 0x1 0xffff>,
>> +              <&crypto_mbox 30 0x1 0xffff>,
>> +              <&crypto_mbox 31 0x1 0xffff>;
>> +     };
>> +     ...
>> +};

Regards,
Anup