Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1327902 > unrolled thread

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

Started byNishanth Menon <nm@ti.com>
First post2016-02-05 17:40 +0100
Last post2016-02-09 05:20 +0100
Articles 5 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/2] Documentation: dt: mailbox: Add TI Message Manager Nishanth Menon <nm@ti.com> - 2016-02-05 17:40 +0100
    Re: [PATCH 1/2] Documentation: dt: mailbox: Add TI Message Manager Rob Herring <robh@kernel.org> - 2016-02-08 20:40 +0100
      Re: [PATCH 1/2] Documentation: dt: mailbox: Add TI Message Manager Suman Anna <s-anna@ti.com> - 2016-02-08 21:30 +0100
      Re: [PATCH 1/2] Documentation: dt: mailbox: Add TI Message Manager Nishanth Menon <nm@ti.com> - 2016-02-08 21:40 +0100
    Re: [PATCH 1/2] Documentation: dt: mailbox: Add TI Message Manager Jassi Brar <jassisinghbrar@gmail.com> - 2016-02-09 05:20 +0100

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

FromNishanth Menon <nm@ti.com>
Date2016-02-05 17:40 +0100
Subject[PATCH 1/2] Documentation: dt: mailbox: Add TI Message Manager
Message-ID<qYRAl-2Gm-1@gated-at.bofh.it>
Message Manager is a hardware block used to communicate with various
processor systems within certain Texas Instrument's Keystone
generation SoCs.

This hardware engine is used to transfer messages from various compute
entities(or processors) within the SoC. It is designed to be self
contained without needing software initialization for operation.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 .../bindings/mailbox/ti,message-manager.txt        | 72 ++++++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/ti,message-manager.txt

diff --git a/Documentation/devicetree/bindings/mailbox/ti,message-manager.txt b/Documentation/devicetree/bindings/mailbox/ti,message-manager.txt
new file mode 100644
index 000000000000..f3d73b0b3c66
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/ti,message-manager.txt
@@ -0,0 +1,72 @@
+Texas Instruments' Message Manager Driver
+========================================
+
+The Texas Instruments' Message Manager is a mailbox controller that has
+configurable queues selectable at SoC(System on Chip) integration. The Message
+manager is broken up into queues in different address regions that are called
+"proxies" - each instance is unidirectional and is instantiated at SoC
+integration level to indicate receive or transmit path.
+
+Message Manager Device Node:
+===========================
+
+Required properties:
+--------------------
+- compatible:		Shall be:
+				"ti,k2g-message-manager"
+				"ti,message-manager"
+- reg-names 		queue_proxy_region - Map the queue Proxy region
+			queue_state_debug_region - Map the queue state debug
+			region.
+- reg:			Contains the register map per reg-names
+- #mbox-cells		Shall be 1
+
+Child Nodes:
+============
+A child node is used for representing the actual queue device that is
+used for the communication between the host processor and a remote processor.
+Each child node should have a unique node name across all the different
+message manager device nodes.
+
+Required Properties:
+--------------------
+- ti,queue-id:		Indicates the queue number this node represents
+- ti,proxy-id:		Proxy ID representing the processor in the SoC.
+
+Optional Properties:
+--------------------
+- interrupt-names:	'rx' - indicates a receive interrupt (mandatory ONLY if
+			this is a receive queue)
+- interrupts:		Contains the interrupt information corresponding to
+			interrupt-names property.
+
+Example:
+--------
+
+	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 324 IRQ_TYPE_EDGE_RISING>;
+		};
+	};
+
+...
+	pmmc {
+		...
+		mbox-names = "tx", "rx";
+		mboxes = <&msgmgr &msgmgr_proxy_pmmc_tx>
+			 <&msgmgr &msgmgr_proxy_pmmc_rx>;
+		...
+	};
-- 
2.7.0

[toc] | [next] | [standalone]

#1329502

FromRob Herring <robh@kernel.org>
Date2016-02-08 20:40 +0100
Message-ID<qZZPc-1iv-3@gated-at.bofh.it>
In reply to#1327902
On Fri, Feb 05, 2016 at 10:34:03AM -0600, Nishanth Menon wrote:
> Message Manager is a hardware block used to communicate with various
> processor systems within certain Texas Instrument's Keystone
> generation SoCs.
> 
> This hardware engine is used to transfer messages from various compute
> entities(or processors) within the SoC. It is designed to be self
> contained without needing software initialization for operation.
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>  .../bindings/mailbox/ti,message-manager.txt        | 72 ++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/ti,message-manager.txt
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/ti,message-manager.txt b/Documentation/devicetree/bindings/mailbox/ti,message-manager.txt
> new file mode 100644
> index 000000000000..f3d73b0b3c66
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/ti,message-manager.txt
> @@ -0,0 +1,72 @@
> +Texas Instruments' Message Manager Driver
> +========================================
> +
> +The Texas Instruments' Message Manager is a mailbox controller that has
> +configurable queues selectable at SoC(System on Chip) integration. The Message
> +manager is broken up into queues in different address regions that are called
> +"proxies" - each instance is unidirectional and is instantiated at SoC
> +integration level to indicate receive or transmit path.
> +
> +Message Manager Device Node:
> +===========================
> +
> +Required properties:
> +--------------------
> +- compatible:		Shall be:
> +				"ti,k2g-message-manager"
> +				"ti,message-manager"

Given that queues are configurable at integration time, does a generic 
property really make sense here?

> +- reg-names 		queue_proxy_region - Map the queue Proxy region
> +			queue_state_debug_region - Map the queue state debug
> +			region.
> +- reg:			Contains the register map per reg-names
> +- #mbox-cells		Shall be 1

And the value contained is what?

> +
> +Child Nodes:
> +============
> +A child node is used for representing the actual queue device that is
> +used for the communication between the host processor and a remote processor.
> +Each child node should have a unique node name across all the different
> +message manager device nodes.
> +
> +Required Properties:
> +--------------------
> +- ti,queue-id:		Indicates the queue number this node represents
> +- ti,proxy-id:		Proxy ID representing the processor in the SoC.

What determines these values?

> +
> +Optional Properties:
> +--------------------
> +- interrupt-names:	'rx' - indicates a receive interrupt (mandatory ONLY if
> +			this is a receive queue)

Kind of pointless if there is only 1.

> +- interrupts:		Contains the interrupt information corresponding to
> +			interrupt-names property.
> +
> +Example:
> +--------
> +
> +	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 324 IRQ_TYPE_EDGE_RISING>;
> +		};
> +	};
> +
> +...
> +	pmmc {
> +		...
> +		mbox-names = "tx", "rx";
> +		mboxes = <&msgmgr &msgmgr_proxy_pmmc_tx>
> +			 <&msgmgr &msgmgr_proxy_pmmc_rx>;

While I guess this is valid DT, it is a bit strange having the cell 
values be phandles. Why not just make the queue and proxy ids be the 
cell values? The interrupts could be moved to the parent and the child 
nodes eliminated altogether.

The alternative would be just drop msgmgr phandle and point to the child 
nodes. I prefer getting rid of the child nodes though.

Rob

[toc] | [prev] | [next] | [standalone]

#1329531

FromSuman Anna <s-anna@ti.com>
Date2016-02-08 21:30 +0100
Message-ID<r00BA-1PS-9@gated-at.bofh.it>
In reply to#1329502
Hi Rob,

On 02/08/2016 01:37 PM, Rob Herring wrote:
> On Fri, Feb 05, 2016 at 10:34:03AM -0600, Nishanth Menon wrote:
>> Message Manager is a hardware block used to communicate with various
>> processor systems within certain Texas Instrument's Keystone
>> generation SoCs.
>>
>> This hardware engine is used to transfer messages from various compute
>> entities(or processors) within the SoC. It is designed to be self
>> contained without needing software initialization for operation.
>>
>> Signed-off-by: Nishanth Menon <nm@ti.com>
>> ---
>>  .../bindings/mailbox/ti,message-manager.txt        | 72 ++++++++++++++++++++++
>>  1 file changed, 72 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mailbox/ti,message-manager.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mailbox/ti,message-manager.txt b/Documentation/devicetree/bindings/mailbox/ti,message-manager.txt
>> new file mode 100644
>> index 000000000000..f3d73b0b3c66
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mailbox/ti,message-manager.txt
>> @@ -0,0 +1,72 @@
>> +Texas Instruments' Message Manager Driver
>> +========================================
>> +
>> +The Texas Instruments' Message Manager is a mailbox controller that has
>> +configurable queues selectable at SoC(System on Chip) integration. The Message
>> +manager is broken up into queues in different address regions that are called
>> +"proxies" - each instance is unidirectional and is instantiated at SoC
>> +integration level to indicate receive or transmit path.
>> +
>> +Message Manager Device Node:
>> +===========================
>> +
>> +Required properties:
>> +--------------------
>> +- compatible:		Shall be:
>> +				"ti,k2g-message-manager"
>> +				"ti,message-manager"
> 
> Given that queues are configurable at integration time, does a generic 
> property really make sense here?
> 
>> +- reg-names 		queue_proxy_region - Map the queue Proxy region
>> +			queue_state_debug_region - Map the queue state debug
>> +			region.
>> +- reg:			Contains the register map per reg-names
>> +- #mbox-cells		Shall be 1
> 
> And the value contained is what?
> 
>> +
>> +Child Nodes:
>> +============
>> +A child node is used for representing the actual queue device that is
>> +used for the communication between the host processor and a remote processor.
>> +Each child node should have a unique node name across all the different
>> +message manager device nodes.
>> +
>> +Required Properties:
>> +--------------------
>> +- ti,queue-id:		Indicates the queue number this node represents
>> +- ti,proxy-id:		Proxy ID representing the processor in the SoC.
> 
> What determines these values?
> 
>> +
>> +Optional Properties:
>> +--------------------
>> +- interrupt-names:	'rx' - indicates a receive interrupt (mandatory ONLY if
>> +			this is a receive queue)
> 
> Kind of pointless if there is only 1.
> 
>> +- interrupts:		Contains the interrupt information corresponding to
>> +			interrupt-names property.
>> +
>> +Example:
>> +--------
>> +
>> +	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 324 IRQ_TYPE_EDGE_RISING>;
>> +		};
>> +	};
>> +
>> +...
>> +	pmmc {
>> +		...
>> +		mbox-names = "tx", "rx";
>> +		mboxes = <&msgmgr &msgmgr_proxy_pmmc_tx>
>> +			 <&msgmgr &msgmgr_proxy_pmmc_rx>;
> 
> While I guess this is valid DT, it is a bit strange having the cell 
> values be phandles. Why not just make the queue and proxy ids be the 
> cell values? The interrupts could be moved to the parent and the child 
> nodes eliminated altogether.

The mailbox controller node (msgmgr) and its child nodes all are
registered with the mailbox core together and the client users request a
mailbox using the framework API that requires the controller node.
Usually the channels are given numbers and the request is done based on
indices, but that meant that we had to introduce a logical device id
just for the sake of the API. Instead, we just chose to go with the
phandles.

Doing a queue and proxy id as cell values requires the mailbox
controller node registration phase to parse through all the client nodes
with the matching controller node. The current approach was in-line with
how all the mailbox controller nodes are implemented today, except for
the usage of phandle instead of an index for the cell-value.

> 
> The alternative would be just drop msgmgr phandle and point to the child 
> nodes. I prefer getting rid of the child nodes though.

The 'mboxes' property is a standard property supported by the mailbox
framework core, and requires the phandle of the controller node along
with a cell-value to lookup a channel. So, we cannot use the child nodes
directly.

regards
Suman

[toc] | [prev] | [next] | [standalone]

#1329544

FromNishanth Menon <nm@ti.com>
Date2016-02-08 21:40 +0100
Message-ID<r00Li-1TO-45@gated-at.bofh.it>
In reply to#1329502
On Mon, Feb 8, 2016 at 1:37 PM, Rob Herring <robh@kernel.org> wrote:

Thank you for reviewing the binding.

> On Fri, Feb 05, 2016 at 10:34:03AM -0600, Nishanth Menon wrote:
>> Message Manager is a hardware block used to communicate with various
>> processor systems within certain Texas Instrument's Keystone
>> generation SoCs.
>>
>> This hardware engine is used to transfer messages from various compute
>> entities(or processors) within the SoC. It is designed to be self
>> contained without needing software initialization for operation.
>>
>> Signed-off-by: Nishanth Menon <nm@ti.com>
>> ---
>>  .../bindings/mailbox/ti,message-manager.txt        | 72 ++++++++++++++++++++++
>>  1 file changed, 72 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mailbox/ti,message-manager.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mailbox/ti,message-manager.txt b/Documentation/devicetree/bindings/mailbox/ti,message-manager.txt
>> new file mode 100644
>> index 000000000000..f3d73b0b3c66
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mailbox/ti,message-manager.txt
>> @@ -0,0 +1,72 @@
>> +Texas Instruments' Message Manager Driver
>> +========================================
>> +
>> +The Texas Instruments' Message Manager is a mailbox controller that has
>> +configurable queues selectable at SoC(System on Chip) integration. The Message
>> +manager is broken up into queues in different address regions that are called
>> +"proxies" - each instance is unidirectional and is instantiated at SoC
>> +integration level to indicate receive or transmit path.
>> +
>> +Message Manager Device Node:
>> +===========================
>> +
>> +Required properties:
>> +--------------------
>> +- compatible:                Shall be:
>> +                             "ti,k2g-message-manager"
>> +                             "ti,message-manager"
>
> Given that queues are configurable at integration time, does a generic
> property really make sense here?

True. I can drop the generic "ti,message-manager" binding.

>> +- reg-names          queue_proxy_region - Map the queue Proxy region
>> +                     queue_state_debug_region - Map the queue state debug
>> +                     region.
>> +- reg:                       Contains the register map per reg-names
>> +- #mbox-cells                Shall be 1
>
> And the value contained is what?

phandle -> I think Suman has already explained in his response.

>> +
>> +Child Nodes:
>> +============
>> +A child node is used for representing the actual queue device that is
>> +used for the communication between the host processor and a remote processor.
>> +Each child node should have a unique node name across all the different
>> +message manager device nodes.
>> +
>> +Required Properties:
>> +--------------------
>> +- ti,queue-id:               Indicates the queue number this node represents
>> +- ti,proxy-id:               Proxy ID representing the processor in the SoC.
>
> What determines these values?

This is SoC integration dependent -> Every queue, proxy combination
has it's own memory region allocated for it.

>> +
>> +Optional Properties:
>> +--------------------
>> +- interrupt-names:   'rx' - indicates a receive interrupt (mandatory ONLY if
>> +                     this is a receive queue)
>
> Kind of pointless if there is only 1.
>

This is primarily to ensure a future compatibility where we are trying
to convince the hardware team to provide an error interrupt per queue
as well for slave usage.
it would probably be pointless at that time to deal with "legacy"
binding defintions when the next hardware ip definition takes place.

>> +- interrupts:                Contains the interrupt information corresponding to
>> +                     interrupt-names property.
>> +
>> +Example:
>> +--------
>> +
>> +     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 324 IRQ_TYPE_EDGE_RISING>;
>> +             };
>> +     };
>> +
>> +...
>> +     pmmc {
>> +             ...
>> +             mbox-names = "tx", "rx";
>> +             mboxes = <&msgmgr &msgmgr_proxy_pmmc_tx>
>> +                      <&msgmgr &msgmgr_proxy_pmmc_rx>;
>
> While I guess this is valid DT, it is a bit strange having the cell
> values be phandles. Why not just make the queue and proxy ids be the
> cell values? The interrupts could be moved to the parent and the child
> nodes eliminated altogether.
>
> The alternative would be just drop msgmgr phandle and point to the child
> nodes. I prefer getting rid of the child nodes though.

I see that Suman has already responded to this.

--
Regards,
Nishanth Menon

[toc] | [prev] | [next] | [standalone]

#1329897

FromJassi Brar <jassisinghbrar@gmail.com>
Date2016-02-09 05:20 +0100
Message-ID<r07Wq-76h-5@gated-at.bofh.it>
In reply to#1327902
On Fri, Feb 5, 2016 at 10:04 PM, Nishanth Menon <nm@ti.com> 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 324 IRQ_TYPE_EDGE_RISING>;
> +               };
> +       };
> +
I think we should get rid of consumer specifics from the provider node...

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

cheers.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web