Re: [PATCH V2 0/4] misc: xgene: Add support for APM X-Gene SoC Queue Manager/Traffic Manager
From: Arnd Bergmann <arnd@arndb.de>
Date: 2014-01-30 14:36:00
Also in:
linux-arm-kernel, linux-devicetree, lkml
On Tuesday 28 January 2014, Ravi Patel wrote:
On Tue, Jan 14, 2014 at 7:15 AM, Arnd Bergmann [off-list ref] wrote:
-
quoted
For the DT binding, I would suggest using something along the lines of what we have for clocks, pinctrl and dmaengine. OMAP doesn't use this (yet), but now would be a good time to standardize it. The QMTM node should define a "#mailbox-cells" property that indicates how many 32-bit cells a qmtm needs to describe the connection between the controller and the slave. My best guess is that this would be hardcoded to <3>, using two cells for a 64-bit FIFO bus address, and a 32-bit cell for the slave-id signal number. All other parameters that you have in the big table in the qmtm driver at the moment can then get moved into the slave drivers, as they are constant per type of slave. This will simplify the QMTM driver. In the slave, you should have a "mailboxes" property with a phandle to the qmtm followed by the three cells to identify the actual queue. If it's possible that a device uses more than one rx and one tx queue, we also need a "mailbox-names" property to identify the individual queues.We explored on DT bindings suggestion given by you. We have come up with a sample DT binding for how it will look like. Herewith we have provided the same. Would you please review and give us your comments before we change our driver and DTS file to accomodate it? Sample DTS node for QM: qmlite: qmtm@17030000 { compatible = "apm,xgene-qmtm-lite";
I would use 'mailbox@17030000' as the node name, as the name part is supposed to be descriptive of the function rather than the implemention.
reg = <0x0 0x17030000 0x0 0x10000>,
<0x0 0x10000000 0x0 0x400000>;
interrupts = <0x0 0x40 0x4>,
<0x0 0x3c 0x4>;
status = "ok";
#clock-cells = <1>;
clocks = <&qmlclk 0>;
#mailbox-cells = <3>;
};The #clock-cells seems misplaced here, unless this is also a clock provider, which I don't think it is.
Sample DTS node for Ethernet:
menet: ethernet@17020000 {
compatible = "apm,xgene-enet";
status = "disabled";
reg = <0x0 0x17020000 0x0 0x30>,
<0x0 0x17020000 0x0 0x10000>,
<0x0 0x17020000 0x0 0x20>;Unrelated, but it seems strange to have three register sets of different sizes at the same offset.
mailboxes = <&qmlite 0x0 0x1000002c 0x0000>,
<&qmlite 0x0 0x10000052 0x0020>,
<&qmlite 0x0 0x10000060 0x0f00>
mailbox-names = "mb-tx", "mb-fp", "mb-rx";I would leave out the "mb-" part of the strings and just document them as "tx", "rx" and "fp".
interrupts = <0x0 0x38 0x4>,
<0x0 0x39 0x4>,
<0x0 0x3a 0x4>;
#clock-cells = <1>;Same comment about #clock-cells here.
clocks = <ð8clk 0>;
local-mac-address = <0x0 0x11 0x3a 0x8a 0x5a 0x78>;
max-frame-size = <0x233a>;
phyid = <3>;
phy-mode = "rgmii";
};
The mailbox node in DTS has following format:
mailbox = <&parent 'higher 32 bit bus address' 'lower 32 bit bus
address' 'signal id'>sounds good.
Ethernet driver will call following function in platform_probe: mailbox_get(dev, "mb-tx"); mailbox_get API will return the the context of allocated and configured mailbox. For now, mailbox_get API will be implemented in xgene QMTM driver. Eventually when mailbox framework in Linux will be standardized, we will use it instead.
Ok.
quoted
For the in-kernel interfaces, we should probably start a conversation with the owners of the mailbox drivers to get a common API, for now I'd suggest you just leave it as it is, and only adapt for the new binding.Sure. For now we will put our driver mostly as is in the drivers/mailbox. Can you please help us identify the owners of the mailbox drivers? As you suggested, we can start conversation with them to define common in kernel APIs.
Please talk to "Suman Anna" [off-list ref] for the TI part and Rob Herring [off-list ref] for pl320. The pl320 driver was written by Mark Langsdorf for Calxeda, but I don't have an updated email address for him and assume that the calxeda address is no longer functional. Arnd