On 11 November 2014 22:03, Arnd Bergmann arnd@arndb.de wrote:
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.
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