On Tuesday 11 November 2014 19:27:07 Jassi Brar wrote:
On 11 November 2014 18:32, Ashwin Chaugule ashwin.chaugule@linaro.org wrote:
On 10 November 2014 23:04, Jassi Brar jaswinder.singh@linaro.org 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.
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.
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); }
For any firmware that we have control over, the mbox_request_channel() case could even be made to work in ACPI, if we decide to add the properties we use in DT with the new _DSD method.
Arnd