Re: [PATCH v3 03/22] dt-bindings: arm: scmi: add ARM MHU specific mailbox client bindings

[Rob Herring]

On Mon, Oct 9, 2017 at 9:46 AM, Jassi Brar <jassisinghbrar@xxxxxxxxx> wrote:
> On Mon, Oct 9, 2017 at 7:22 PM, Rob Herring <robh@xxxxxxxxxx> wrote:
>> On Fri, Oct 6, 2017 at 9:26 PM, Jassi Brar <jassisinghbrar@xxxxxxxxx> wrote:
>>> On Fri, Oct 6, 2017 at 9:24 PM, Rob Herring <robh@xxxxxxxxxx> wrote:
>>>> On Fri, Oct 6, 2017 at 6:01 AM, Jassi Brar <jassisinghbrar@xxxxxxxxx> wrote:
>>>>> On Fri, Oct 6, 2017 at 4:50 AM, Rob Herring <robh@xxxxxxxxxx> wrote:
>>>>>> On Thu, Sep 28, 2017 at 02:11:27PM +0100, Sudeep Holla wrote:
>>>
>>>>>>
>>>>>>> +- mbox-data : For each phandle listed in mboxes property, an unsigned 32-bit
>>>>>>> +           data as expected by the mailbox controller
>>>>>>
>>>>>> Shouldn't that be cells as part of mboxes property?
>>>>>>
>>>>> A MHU client can send any number of commands (such u32 values) over a channel.
>>>>> This client (SCMI) sends just one command over a channel, but other
>>>>> clients may/do send two or more.
>>
>> The above definition doesn't support 2 or more as it is 1-1 with channels.
>>
> I thought you suggested to make controller driver accept the command
> as another cell in client's mboxes property.
> Which we can't do.

Yes, agreed. But I'm wondering since a client may need more than one,
how would that be expressed?

>>>> Okay, then I guess I don't understand why this is in DT.
>>>>
>>> Yeah the client has to provide code (u32 value) for the commands it
>>> sends, and that value is going to be platform specific. For example,
>>> on Juno the ITS_AN_SCMI_COMMAND may be defined as BIT(7) while on my
>>> platform it may be 0x4567
>>>
>>> For MHU based platforms, it becomes easy if the u32 is passed from DT.
>>> And that should be ok since that is like a h/w parameter - a value
>>> chosen/expected by the remote firmware.
>>
>> Could it ever be more than 1 cell?
>>
> SCMI sends sub-commands via SHMEM, so it is always going to be 1cell for _scmi_.
> However many firmwares are unlikely to use just one command over a
> channel - say, the protocol is trivial or the linux and remote have no
> SHMEM.

I'd hope the normal case is not enumerating commands and sub-commands
in DT and this is special case of a "generic" protocol with platform
specific aspects. It could be solved with a specific compatible for
each platform/implementation. We'll probably regret not doing that,
but I'm going to pretend that this time SoC vendors won't mess it up.

>> I guess being in DT is fine, but I'm still not sure about the naming.
>> The current name suggests it is part of the mbox binding. Do we want
>> that or should it be SCMI specific? Then "data" is vague. Perhaps
>> "scmi-commands"?
>>
> Sure. I have no problem with whatever we wanna call it.

Okay. That should have an "arm" prefix too BTW.

Rob