Thread (29 messages) 29 messages, 4 authors, 2014-01-30

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 = <&eth8clk 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help