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

[Sudeep Holla]

On 09/10/17 23:57, Rob Herring wrote:
> 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.
> 

Just to align on the same page, I would like to define the terms used
above in context of SCMI:
1. commands : this is platform specific "command" to trigger/signal the
	      firmware that SCMI packet is sent. This is always platform
	      or controller specific and is not part of the
	      specification. It's just like doorbell but since MHU can
	      send 32-bit data, we need a way to specify the exact bit
	      as 32-bit data with just one bit set.

2. sub-commands : It is SCMI packet that includes the actual command
		defined in the SCMI specification. There are sent in
		SHMEM as Jassi mentioned above.
-- 
Regards,
Sudeep