On 11 November 2014 22:03, Arnd Bergmann <arnd@xxxxxxxx> wrote:
> On Tuesday 11 November 2014 19:27:07 Jassi Brar wrote:
>> On 11 November 2014 18:32, Ashwin Chaugule <ashwin.chaugule@xxxxxxxxxx> wrote:
>> > On 10 November 2014 23:04, Jassi Brar <jaswinder.singh@xxxxxxxxxx> wrote:
>>
>> In even simpler terms.... I prefer controller specific
>> encoding(0x50434300) instead of controller specific api
>> (pcc_mbox_request_channel). For a different class of controller, it
>> is much cleaner to define a new encoding as compared to another
>> xyz_mbox_request_channel() api.
>
> The problem with this approach is that it still leaves the interface
> as controller specific, because the client now has to know that it
> must pass the PCC identifier instead of an index.
>
Yup. I hope you are aware that the "index" argument of
pcc_mbox_request_channel() is just the same thing. The "index" there
is actually the 'Type' value defined in ACPI for the client.
> A client driver using DT would just specify to use the first channel
> that is defined locally in the "mboxes" properties, so it can ask for
>
> chan = mbox_request_channel(client, 0);
>
> to get the first channel defined for this client while a driver using
> ACPI has to look up the identifier in its own local tables:
>
> u32 channel_id = this_driver_parse_acpi_and_find_pccid(dev);
> chan = mbox_request_channel(client, channel_id);
>
> which is not the same interface as the first, even if you assume the
> channel_id to be globally unique in the system.
>
Yes. As I said, same for pcc_mbox_request_channel(). But since PCC
clients are limited and specified by ACPI (Type value won't change
across platforms, I understand), the client drivers could have that
value hardcoded.
> Since we will need clients to have a consistent interface across
> DT and ACPI, I think the difference in firmware interfaces is better
> abstracted in a PCC specific interface.
>
> The client driver would still need to parse two different firmware
> structures, but it would at least have a consistent interface:
>
> u32 channel_id = of_property_read_u32(dev->of_node, "pccid");
> if (!channel_id)
> channel_id = this_driver_parse_acpi_and_find_pccid(dev);
> chan = mbox_request_channel(client, channel_id);
>
> Alternatively, the driver could use the regular API on DT:
>
> chan = mbox_request_channel(cl, 0);
> if (!chan) {
> u32 chan_id = this_driver_parse_acpi_and_find_pccid(dev);
> chan = mbox_request_channel(client, channel_id);
> }
>
OK, but if the 'Type' value, that a client passes to
(pcc_)mbox_req_chan(), does not change across platforms, would we need
that?
Thanks,
-Jassi
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
