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

[Jassi Brar]

On Tue, Feb 9, 2016 at 6:01 PM, Nishanth Menon <nm@xxxxxx> wrote:
> On 02/08/2016 10:14 PM, Jassi Brar wrote:
>
> Thanks for the review.
>
>> On Fri, Feb 5, 2016 at 10:04 PM, Nishanth Menon <nm@xxxxxx> wrote:
>>> +
>>> +       msgmgr: msgmgr@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@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>;
};


>>> +...
>>> +       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!