Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells

[Jassi Brar]

On Fri, Jul 7, 2017 at 5:02 PM, Sudeep Holla <sudeep.holla@xxxxxxx> wrote:
>
>
> On 06/07/17 19:37, Jassi Brar wrote:
>> On Thu, Jul 6, 2017 at 10:14 PM, Sudeep Holla <sudeep.holla@xxxxxxx> wrote:
>>>
>>> On 06/07/17 15:37, Jassi Brar wrote:
>>>> On Thu, Jul 6, 2017 at 3:03 PM, Sudeep Holla <sudeep.holla@xxxxxxx> wrote:
>>>
>>
>>>> I see no reason why you must have SCPI and SCMI both running.
>>>>
>>>
>>> We can still have 2 different protocols using same MHU channel with
>>> different doorbells, what's wrong with that ?
>>>
>> Only for SCMI and SCPI - both written by same person to achieve the
>> same thing - I think it should not be necessary.
>>
>
> Really ? you decide based on that and repeatedly ignore what ARM MHU
> specification offers. Wow, I am surprised.
>
>> But yes, SCMI running alongside some complementary protocol (that
>> implements what SCMI doesn't provide) is a very solid usecase. And
>> that is the reason you need a platform specific 'transport layer'.
>>
>
> The whole SCMI exercise is to reduce such platform specific code.
> It's made to work with ACPI using doorbells via PCC channels. Now, you
> say for DT we need per platform shim. Mailbox is my transport, why do I
> need anything more as I don't care what is sent in mailbox controller as
> long as the signal is sent to the other side.
>
>>>> And even then there is a solution - a shim arbitrator. Other
>>>> platforms, those share a channel, do that. No big deal.
>>>>
>>>
>>> Example please? Please remember these protocols are generic and we
>>> can't add any platform specific code into them.
>>>
>> You do need to have platform specific glue as I said.
>>
>
> Yes, please propose along bindings to do that as you are objecting the
> one in $subject. I have asked you many times the same thing.
>
"platform specific transport layer"  =>  no generic bindings for the controller.

The bindings, that already exist for mailbox controllers plus what
properties the SCMI need, are to be used by a thin platform specific
driver to provide an interface to the generic SCMI implementation to
send the "doorbell".

For example, look at how the common DWC3 driver is used by platforms
that have different h/w integrations.
   $ grep synopsys,dwc3 -rw -B15 arch/arm/boot/dts/

That is, SCMI node as a child of platform specific SCMI node. Or maybe
even SCMI as a library kinda thing.

I can't explain it any better.


>>>>  BTW, I hope you realise that we need a 'transport layer' which will
>>>> be the platform specific glue between mailbox controller specifics and
>>>> the generic SCMI code.
>>>
>>> Why ?
>>>
>> Because you should not restrict the usage of SCMI to only platforms
>> with mailbox controller that does not require any information to
>> trigger a signal on the other side -- what you call "doorbell".
>>
>
> Indeed, it's restricted, read the specification....
>
Its a shame to hear that.


>> Why shouldn't Rockchip be able to implement SCMI? Just because it uses
>> different mechanism to signal the other end?
>>
>
> They can, all we need is just a mailbox channel, we don't care wants
> sent in the controller as along as signal is sent. The shared memory has
> to be as per the specification. I still don't know what I am missing to
> convey you.
>

In scmi_do_xfer() you pass pointer to 'struct scmi_xfer' as 'data'. That is

    mbox_send_message(struct mbox_chan *, struct scmi_xfer*);

A controller driver defines is own format for the data it needs to
send a message.

For example, Rockchip driver expects
rockchip_mbox_send_data(struct mbox_chan *, struct rockchip_mbox_msg *)

Fow the huck will it work ?!

SCMI is purely a s/w protocol which can work over any physical link.
But it wouldn't because you have funny notions of a "doorbell".