Re: [PATCH v10 1/1] Mailbox: Add support for Platform Communication Channel

[Ashwin Chaugule <ashwin.chaugule@linaro.org>]

On 10 November 2014 08:39, Jassi Brar <jaswinder.singh@linaro.org> wrote:
> On 10 November 2014 18:27, Ashwin Chaugule <ashwin.chaugule@linaro.org> wrote:
>> On 9 November 2014 23:13, Jassi Brar <jaswinder.singh@linaro.org> wrote:
>>>
>>>>>> based on the discussions that followed since, we decided that its best
>>>>>> to add a separate PCC lookup and registration API. The main reason
>>>>>> being, we dont have a way to list all mbox providers in ACPI in a way
>>>>>> that DT does. e.g. in DT, the client->dev is used to look up mbox
>>>>>> controllers. In ACPI, a client cant specify which mbox controllers to
>>>>>> associate with, if it can attach to multiple. With the PCC specific
>>>>>> API, if the client calls it, the controller knows where to look,
>>>>>> because that lookup is PCC specific.
>>>>>>
>>>>>> In your patch, the assumption that PCC is the only ACPI mbox provider,
>>>>>> maybe true today, but that can change anytime.
>>>>>>
>>>>> Please read my patch again, we can have ACPI as well as DT populated
>>>>> clients. All that you intend to do with this patch can be done there
>>>>> and _without_ adding new apis.
>>>>
>>>> Read it again. Not arguing that your patch wont work for DT and ACPI,
>>>> but your assumption that ACPI supports PCC as the only mbox
>>>> controller, may not hold true. The global_xlate function will work
>>>> fine for PCC, but may not work for other ACPI (non-DT) mbox
>>>> controllers. Using the signature field/index byte works only for PCC.
>>>> We've already been through this discussion with Mark and Arnd and we
>>>> came up with the PCC API.
>>>>
>>> Please read it yet again. There is no assumption that PCC is the only
>>> mbox in ACPI (though I think that is very likely). The function and
>>> its argument are both named _global_. 'Signature' is mentioned only as
>>> an example in case of PCC. The PCC controller driver could expect the
>>> global_id to be 'signature' of the subspace, similarly another non-DT
>>> mailbox controller driver will expect its own different 'signature'
>>> (say 0xdead0000 | id_16bits). In the patch I submitted we try
>>> .global_xlate() of all such mboxes and only one, which finds its
>>> id-space specified, will return a channel.
>>>
>>> Ideally, global-id space isn't very clean, but for mailbox we anyway
>>> have to have a direct understanding between controller and client
>>> drivers. So having global IDs is a great tradeoff if we avoid messing
>>> up the api.
>>
>> How is this different than expecting the client to pass a string name
>> of the mbox controller it wants?
>>
> Global-ID is ugly, string matching is uglier. String matching requires
> changes to client and provider structures as opposed to simple
> numerical comparison to find a suitable channel.

And both have the problem that we cant guarantee uniqueness [1][2].
Having a separate API solves this problem.

Ashwin

[1] - http://www.spinics.net/lists/linux-acpi/msg52271.html
[2] - http://www.spinics.net/lists/linux-acpi/msg52377.html