Thread (18 messages) flat view 18 messages, 4 authors, 2016-02-26

[PATCH 1/2] Documentation: dt: mailbox: Add TI Message Manager

From: jassisinghbrar@gmail.com (Jassi Brar)
Date: 2016-02-09 14:54:11
Also in: linux-devicetree, lkml

On Tue, Feb 9, 2016 at 6:01 PM, Nishanth Menon [off-list ref] wrote:
On 02/08/2016 10:14 PM, Jassi Brar wrote:

Thanks for the review.
quoted
On Fri, Feb 5, 2016 at 10:04 PM, Nishanth Menon [off-list ref] wrote:
quoted
+
+       msgmgr: msgmgr at 02a00000 {
+               compatible = "ti,k2g-message-manager", "ti,message-manager";
+               #mbox-cells = <1>;
+               reg-names = "queue_proxy_region", "queue_state_debug_region";
+               reg = <0x02a00000 0x400000>, <0x028c3400 0x400>;
+
+               msgmgr_proxy_pmmc_tx_prio0: pmmc_tx_prio0 {
+                       ti,queue-id = <0>;
+                       ti,proxy-id = <0>;
+               };
+
+               msgmgr_proxy_pmmc_rx: pmmc_rx {
+                       ti,queue-id = <5>;
+                       ti,proxy-id = <2>;
+                       interrupt-names = "rx";
+                       interrupts = <GIC_SPI 32I didn't respond because I think Suman got Rob's point wrong.I didn't respond because I think Suman got Rob's point wrong.4 IRQ_TYPE_EDGE_RISING>;
+               };
+       };
+
I think we should get rid of consumer specifics from the provider node...

If I get rid of the consumer nodes, how do you propose I describe the rx
queue interrupt(s) in the msmgr dt node (Every Rx queue will have it's
own interrupt - and it cannot be reverse computed from queue ID, proxy ID)?
One option is to have controller driver construct interrupt name from
queue and proxy ids like

msgmgr: msgmgr at 02a00000 {
   ....
     interrupt-names = "irq_5_2", "irq_0_0";     /* irq_<queue-id>_<proxy-id> */
     interrupts = <GIC_SPI 324 IRQ_TYPE_EDGE_RISING>,
                        <GIC_SPI 325 IRQ_TYPE_EDGE_RISING>;
}

and have the consumer specify queue and proxy ids in mboxes property like
 pmmc {
       ....
       mbox-names = "tx", "rx";
       mboxes = <&msgmgr 0 0>
                          <&msgmgr 5 2>;
};

quoted
quoted
+...
+       pmmc {
+               ...
+               mbox-names = "tx", "rx";
+               mboxes = <&msgmgr &msgmgr_proxy_pmmc_tx>
+                        <&msgmgr &msgmgr_proxy_pmmc_rx>;
+               ...
+       };
... and have consumers like
       pmmc {
               ...
               mbox-names = "tx", "rx";
               mboxes = <&msgmgr 0 0>
                        <&msgmgr 5 2>;
       };

I leave the IRQ for you to decide how to specify - a 'dummy' or
'valid' always provided as last cell in mboxes or some other way.
(I'll review other patches in detail later)
What do we do with the issues that Suman pointed out in the mailbox
framework itself? Could you respond to that thread[1] as well?
Phandle of provider in consumer node is quite normal and acceptable.
I think Rob (at least I am) is talking about the second cell where you
specify phandle (&msgmgr_proxy_xxx) instead of values from those child
nodes directly.
Which is what I suggest   mboxes = <&msgmgr 0 0>,  <&msgmgr 5 2>;

Cheers!
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help