[v2,01/14] dt-bindings: remoteproc: Add TI PRUSS bindings

Message ID 1549290167-876-2-git-send-email-rogerq@ti.com
State Changes Requested
Headers show
Series
  • Add support for TI PRU ICSS
Related show

Checks

Context Check Description
robh/checkpatch success

Commit Message

Roger Quadros Feb. 4, 2019, 2:22 p.m.
From: Suman Anna <s-anna@ti.com>

This patch adds the bindings for the Programmable Real-Time Unit
and Industrial Communication Subsystem (PRU-ICSS) present on various
SoCs such as AM33xx, AM437x, AM57xx, Keystone 66AK2G SoC, etc. It is
present on the Davinci based OMAPL138 SoCs and K3 architecture
based AM65x SoCs as well (not covered for now).

Signed-off-by: Suman Anna <s-anna@ti.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 .../devicetree/bindings/soc/ti/ti,pruss.txt        | 212 +++++++++++++++++++++
 1 file changed, 212 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/ti/ti,pruss.txt

Comments

Tony Lindgren Feb. 4, 2019, 4:33 p.m. | #1
Hi,

* Roger Quadros <rogerq@ti.com> [190204 14:23]:
> From: Suman Anna <s-anna@ti.com>
...
> +Example:
> +========
> +1.	/* AM33xx PRU-ICSS */
> +
> +	pruss: pruss@0 {
> +		compatible = "ti,am3356-pruss";
> +		reg = <0x0 0x2000>,
> +		      <0x2000 0x2000>,
> +		      <0x10000 0x3000>;
> +		reg-names = "dram0", "dram1",
> +			    "shrdram2";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;

Thanks for fixing up the reg ranges for the top level node.

Ideally there would not even be a top level node here as
AFAIK the whole PRUSS is a collection of devices on a PRU
internal interconnect. So following that path a bit further..
How about just get rid of the top level node and just do:

pruss: pruss@0 {
	dram0: memory@0 {
	       device_type = "memory";
	       reg = <0x0 0x2000>;
	};

	dram1: memory@2000 {
	       device_type = "memory";
	       reg = <0x2000 0x2000>;
	};

	shrdram2: memory@10000 {
		device_type = "memory";
		reg = <0x10000 0x3000>;
	};

	pruss_cfg: cfg@26000 {
		...
	};
	...
};

If the device_type = "memory" cannot be used here for
being specific to the top level properties, then
there's probably some other generic property usable
here :)

> +		pruss_mii_rt: mii_rt@32000 {
> +			reg = <0x32000 0x58>;
> +		};

The node name should not have underscores so
pruss_mii_rt: mii-rt@32000. Please check the others
too, like app_node.

> +	app_node: app_node {
> +		prus = <&pru0>, <&pru1>;
> +		firmware-name = "pruss-app-fw", "pruss-app-fw-2";
> +		ti,pruss-gp-mux-sel = <2>, <1>;
> +		/* setup interrupts for prus:
> +		   prus[0] => pru1_0: ev=16, chnl=2, host-irq=7,
> +		   prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */
> +		ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
> +	}

If the ti,pruss-gp-mux-sel and ti,pru-interrupt-map are
firmware configuration options, maybe leave them out of
the dts completely and make the app-node optional.

And have a proper compatible for this node such as
"ti,pruss-app-xyz". And this should be only set if the the
hardware is wired up in such way that things need to be
configured in the dts rather than by the firmware.

And then you can just hide mux-sel and interrupt-map
behind the compatible property for that hardware. And
leave them out from the dts and have the handling driver
would set mux-sel and interrupt-map based on the
match->data during probe.

Regards,

Tony
Roger Quadros Feb. 5, 2019, 9:39 a.m. | #2
Hi Tony & Suman,

On 04/02/19 18:33, Tony Lindgren wrote:
> Hi,
> 
> * Roger Quadros <rogerq@ti.com> [190204 14:23]:
>> From: Suman Anna <s-anna@ti.com>
> ...
>> +Example:
>> +========
>> +1.	/* AM33xx PRU-ICSS */
>> +
>> +	pruss: pruss@0 {
>> +		compatible = "ti,am3356-pruss";
>> +		reg = <0x0 0x2000>,
>> +		      <0x2000 0x2000>,
>> +		      <0x10000 0x3000>;
>> +		reg-names = "dram0", "dram1",
>> +			    "shrdram2";
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		ranges;
> 
> Thanks for fixing up the reg ranges for the top level node.
> 
> Ideally there would not even be a top level node here as
> AFAIK the whole PRUSS is a collection of devices on a PRU
> internal interconnect. So following that path a bit further..
> How about just get rid of the top level node and just do:
> 
> pruss: pruss@0 {
> 	dram0: memory@0 {
> 	       device_type = "memory";
> 	       reg = <0x0 0x2000>;
> 	};
> 
> 	dram1: memory@2000 {
> 	       device_type = "memory";
> 	       reg = <0x2000 0x2000>;
> 	};

Actually dram0 and dram1 are data memories for PRU0 and PRU1 respectively.
Isn't it better if they are moved to the pru node?
e.g.

	pru0: pru@34000 {
		compatible = "ti,am3356-pru";
		reg = <0x34000 0x2000>,
		      <0x22000 0x400>,
		      <0x22400 0x100>,
		      <0x0     0x2000>;
		reg-names = "iram", "control", "debug", "dram";
		...
	};

	pru1: pru@38000 {
		compatible = "ti,am3356-pru";
		reg = <0x38000 0x2000>,
		      <0x24000 0x400>,
		      <0x24400 0x100>,
		      <0x2000  0x2000>;
		reg-names = "iram", "control", "debug", "dram";
		...
	};

I think it is better to place a restriction that firmware on PRU0 cannot use data
memory of PRU1 and vice versa.

Application drivers do sometimes need to read/write to data memory. The pru_rproc
driver could provide a API for the application drivers to get virtual address of
the respective PRU's data memory.

> 
> 	shrdram2: memory@10000 {
> 		device_type = "memory";
> 		reg = <0x10000 0x3000>;
> 	};

Shared RAM is not so straight forward. Both PRU firmwares and both application drivers
might need to read/write here. The area split is decided by firmware design and there
is no hardware protection to prevent from stomping on each others toes.

We need a carveout based memory allocator at least I think that can do a
allocate(base_offset, size); into shared RAM.

This could be used by pru_rproc driver at firmware load time and by application drivers
at initialization time.

Thoughts?

> 
> 	pruss_cfg: cfg@26000 {
> 		...
> 	};
> 	...
> };
> 
> If the device_type = "memory" cannot be used here for
> being specific to the top level properties, then
> there's probably some other generic property usable
> here :)
> 
>> +		pruss_mii_rt: mii_rt@32000 {
>> +			reg = <0x32000 0x58>;
>> +		};
> 
> The node name should not have underscores so
> pruss_mii_rt: mii-rt@32000. Please check the others
> too, like app_node.
> 

OK.

>> +	app_node: app_node {
>> +		prus = <&pru0>, <&pru1>;
>> +		firmware-name = "pruss-app-fw", "pruss-app-fw-2";
>> +		ti,pruss-gp-mux-sel = <2>, <1>;
>> +		/* setup interrupts for prus:
>> +		   prus[0] => pru1_0: ev=16, chnl=2, host-irq=7,
>> +		   prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */
>> +		ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
>> +	}
> 
> If the ti,pruss-gp-mux-sel and ti,pru-interrupt-map are
> firmware configuration options, maybe leave them out of
> the dts completely and make the app-node optional.

Yes the app-node is optional. I will mention it.

No, ti,pruss-gp-mux-sel and ti,pru-interrupt-map are not firmware options.
But these settings are application/firmware specific.

ti,pru-interrupt-map specifies the configuration to be used for the INTC interrupt
controller.

ti,pruss-gp-mux-sel is used to configure this register.
"Table 30-20. PRUSS_GPCFG0" in http://www.tij.co.jp/jp/lit/ug/spruhz7h/spruhz7h.pdf
"29:26 PR1_PRU0_GP_MUX_SEL"

It configures how the pins from the PRUSS module are routed internally
to the various modules.

see "30.2.1 PRU-ICSS I/O Interface"
and "Table 30-1. PRU-ICSS1 I/O Signals"

> 
> And have a proper compatible for this node such as
> "ti,pruss-app-xyz". And this should be only set if the the
> hardware is wired up in such way that things need to be
> configured in the dts rather than by the firmware.

Yes, compatible is a required property as we need to load
the appropriate application (kernel space) driver for it.
I will fix the example.

> 
> And then you can just hide mux-sel and interrupt-map
> behind the compatible property for that hardware. And
> leave them out from the dts and have the handling driver
> would set mux-sel and interrupt-map based on the
> match->data during probe.

To summarize:

I'll mark the app node as optional. Only required if a kernel
driver is required for the application.
Compatible is mandatory for app node.
ti,pruss-gp-mux-sel and ti,pru-interrupt-map are optional
for app node.

cheers,
-roger
Murali Karicheri Feb. 5, 2019, 3:08 p.m. | #3
Hi Roger,

On 02/05/2019 04:39 AM, Roger Quadros wrote:
> Hi Tony & Suman,
> 
> On 04/02/19 18:33, Tony Lindgren wrote:
>> Hi,
>>
>> * Roger Quadros <rogerq@ti.com> [190204 14:23]:
>>> From: Suman Anna <s-anna@ti.com>
>> ...
>>> +Example:
>>> +========
>>> +1.	/* AM33xx PRU-ICSS */
>>> +
>>> +	pruss: pruss@0 {
>>> +		compatible = "ti,am3356-pruss";
>>> +		reg = <0x0 0x2000>,
>>> +		      <0x2000 0x2000>,
>>> +		      <0x10000 0x3000>;
>>> +		reg-names = "dram0", "dram1",
>>> +			    "shrdram2";
>>> +		#address-cells = <1>;
>>> +		#size-cells = <1>;
>>> +		ranges;
>>
>> Thanks for fixing up the reg ranges for the top level node.
>>
>> Ideally there would not even be a top level node here as
>> AFAIK the whole PRUSS is a collection of devices on a PRU
>> internal interconnect. So following that path a bit further..
>> How about just get rid of the top level node and just do:
>>
>> pruss: pruss@0 {
>> 	dram0: memory@0 {
>> 	       device_type = "memory";
>> 	       reg = <0x0 0x2000>;
>> 	};
>>
>> 	dram1: memory@2000 {
>> 	       device_type = "memory";
>> 	       reg = <0x2000 0x2000>;
>> 	};
> 
> Actually dram0 and dram1 are data memories for PRU0 and PRU1 respectively.
> Isn't it better if they are moved to the pru node?
> e.g.
> 
> 	pru0: pru@34000 {
> 		compatible = "ti,am3356-pru";
> 		reg = <0x34000 0x2000>,
> 		      <0x22000 0x400>,
> 		      <0x22400 0x100>,
> 		      <0x0     0x2000>;
> 		reg-names = "iram", "control", "debug", "dram";
> 		...
> 	};
> 
> 	pru1: pru@38000 {
> 		compatible = "ti,am3356-pru";
> 		reg = <0x38000 0x2000>,
> 		      <0x24000 0x400>,
> 		      <0x24400 0x100>,
> 		      <0x2000  0x2000>;
> 		reg-names = "iram", "control", "debug", "dram";
> 		...
> 	};
> 
> I think it is better to place a restriction that firmware on PRU0 cannot use data
> memory of PRU1 and vice versa.
> 
That will not work as there are switch firmware cases where PRU access
DRAM of other PRU and is a valid case to support in the future. So let
us not do that.

Murali
> Application drivers do sometimes need to read/write to data memory. The pru_rproc
> driver could provide a API for the application drivers to get virtual address of
> the respective PRU's data memory.
> 
>>
>> 	shrdram2: memory@10000 {
>> 		device_type = "memory";
>> 		reg = <0x10000 0x3000>;
>> 	};
> 
> Shared RAM is not so straight forward. Both PRU firmwares and both application drivers
> might need to read/write here. The area split is decided by firmware design and there
> is no hardware protection to prevent from stomping on each others toes.
> 
> We need a carveout based memory allocator at least I think that can do a
> allocate(base_offset, size); into shared RAM.
> 
> This could be used by pru_rproc driver at firmware load time and by application drivers
> at initialization time.
> 
> Thoughts?
> 
>>
>> 	pruss_cfg: cfg@26000 {
>> 		...
>> 	};
>> 	...
>> };
>>
>> If the device_type = "memory" cannot be used here for
>> being specific to the top level properties, then
>> there's probably some other generic property usable
>> here :)
>>
>>> +		pruss_mii_rt: mii_rt@32000 {
>>> +			reg = <0x32000 0x58>;
>>> +		};
>>
>> The node name should not have underscores so
>> pruss_mii_rt: mii-rt@32000. Please check the others
>> too, like app_node.
>>
> 
> OK.
> 
>>> +	app_node: app_node {
>>> +		prus = <&pru0>, <&pru1>;
>>> +		firmware-name = "pruss-app-fw", "pruss-app-fw-2";
>>> +		ti,pruss-gp-mux-sel = <2>, <1>;
>>> +		/* setup interrupts for prus:
>>> +		   prus[0] => pru1_0: ev=16, chnl=2, host-irq=7,
>>> +		   prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */
>>> +		ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
>>> +	}
>>
>> If the ti,pruss-gp-mux-sel and ti,pru-interrupt-map are
>> firmware configuration options, maybe leave them out of
>> the dts completely and make the app-node optional.
> 
> Yes the app-node is optional. I will mention it.
> 
> No, ti,pruss-gp-mux-sel and ti,pru-interrupt-map are not firmware options.
> But these settings are application/firmware specific.
> 
> ti,pru-interrupt-map specifies the configuration to be used for the INTC interrupt
> controller.
> 
> ti,pruss-gp-mux-sel is used to configure this register.
> "Table 30-20. PRUSS_GPCFG0" in http://www.tij.co.jp/jp/lit/ug/spruhz7h/spruhz7h.pdf
> "29:26 PR1_PRU0_GP_MUX_SEL"
> 
> It configures how the pins from the PRUSS module are routed internally
> to the various modules.
> 
> see "30.2.1 PRU-ICSS I/O Interface"
> and "Table 30-1. PRU-ICSS1 I/O Signals"
> 
>>
>> And have a proper compatible for this node such as
>> "ti,pruss-app-xyz". And this should be only set if the the
>> hardware is wired up in such way that things need to be
>> configured in the dts rather than by the firmware.
> 
> Yes, compatible is a required property as we need to load
> the appropriate application (kernel space) driver for it.
> I will fix the example.
> 
>>
>> And then you can just hide mux-sel and interrupt-map
>> behind the compatible property for that hardware. And
>> leave them out from the dts and have the handling driver
>> would set mux-sel and interrupt-map based on the
>> match->data during probe.
> 
> To summarize:
> 
> I'll mark the app node as optional. Only required if a kernel
> driver is required for the application.
> Compatible is mandatory for app node.
> ti,pruss-gp-mux-sel and ti,pru-interrupt-map are optional
> for app node.
> 
> cheers,
> -roger
>
Roger Quadros Feb. 5, 2019, 3:41 p.m. | #4
Murali,

On 05/02/19 17:08, Murali Karicheri wrote:
> Hi Roger,
> 
> On 02/05/2019 04:39 AM, Roger Quadros wrote:
>> Hi Tony & Suman,
>>
>> On 04/02/19 18:33, Tony Lindgren wrote:
>>> Hi,
>>>
>>> * Roger Quadros <rogerq@ti.com> [190204 14:23]:
>>>> From: Suman Anna <s-anna@ti.com>
>>> ...
>>>> +Example:
>>>> +========
>>>> +1.    /* AM33xx PRU-ICSS */
>>>> +
>>>> +    pruss: pruss@0 {
>>>> +        compatible = "ti,am3356-pruss";
>>>> +        reg = <0x0 0x2000>,
>>>> +              <0x2000 0x2000>,
>>>> +              <0x10000 0x3000>;
>>>> +        reg-names = "dram0", "dram1",
>>>> +                "shrdram2";
>>>> +        #address-cells = <1>;
>>>> +        #size-cells = <1>;
>>>> +        ranges;
>>>
>>> Thanks for fixing up the reg ranges for the top level node.
>>>
>>> Ideally there would not even be a top level node here as
>>> AFAIK the whole PRUSS is a collection of devices on a PRU
>>> internal interconnect. So following that path a bit further..
>>> How about just get rid of the top level node and just do:
>>>
>>> pruss: pruss@0 {
>>>     dram0: memory@0 {
>>>            device_type = "memory";
>>>            reg = <0x0 0x2000>;
>>>     };
>>>
>>>     dram1: memory@2000 {
>>>            device_type = "memory";
>>>            reg = <0x2000 0x2000>;
>>>     };
>>
>> Actually dram0 and dram1 are data memories for PRU0 and PRU1 respectively.
>> Isn't it better if they are moved to the pru node?
>> e.g.
>>
>>     pru0: pru@34000 {
>>         compatible = "ti,am3356-pru";
>>         reg = <0x34000 0x2000>,
>>               <0x22000 0x400>,
>>               <0x22400 0x100>,
>>               <0x0     0x2000>;
>>         reg-names = "iram", "control", "debug", "dram";
>>         ...
>>     };
>>
>>     pru1: pru@38000 {
>>         compatible = "ti,am3356-pru";
>>         reg = <0x38000 0x2000>,
>>               <0x24000 0x400>,
>>               <0x24400 0x100>,
>>               <0x2000  0x2000>;
>>         reg-names = "iram", "control", "debug", "dram";
>>         ...
>>     };
>>
>> I think it is better to place a restriction that firmware on PRU0 cannot use data
>> memory of PRU1 and vice versa.
>>
> That will not work as there are switch firmware cases where PRU access
> DRAM of other PRU and is a valid case to support in the future. So let
> us not do that.

PRU firmware accessing DRAM of other PRU is a design contract and that use case
requires both PRUs to be loaded with matching firmware. That should continue to work.

What I'm suggesting here is that kernel remoteproc driver should have nothing to do
with the other PRU's data RAM.

The application driver if needs both PRUs then it can obviously access both DRAMs
as it has a phandle to both PRUs.

cheers,
-roger

> 
> Murali
>> Application drivers do sometimes need to read/write to data memory. The pru_rproc
>> driver could provide a API for the application drivers to get virtual address of
>> the respective PRU's data memory.
>>
>>>
>>>     shrdram2: memory@10000 {
>>>         device_type = "memory";
>>>         reg = <0x10000 0x3000>;
>>>     };
>>
>> Shared RAM is not so straight forward. Both PRU firmwares and both application drivers
>> might need to read/write here. The area split is decided by firmware design and there
>> is no hardware protection to prevent from stomping on each others toes.
>>
>> We need a carveout based memory allocator at least I think that can do a
>> allocate(base_offset, size); into shared RAM.
>>
>> This could be used by pru_rproc driver at firmware load time and by application drivers
>> at initialization time.
>>
>> Thoughts?
>>
>>>
>>>     pruss_cfg: cfg@26000 {
>>>         ...
>>>     };
>>>     ...
>>> };
>>>
>>> If the device_type = "memory" cannot be used here for
>>> being specific to the top level properties, then
>>> there's probably some other generic property usable
>>> here :)
>>>
>>>> +        pruss_mii_rt: mii_rt@32000 {
>>>> +            reg = <0x32000 0x58>;
>>>> +        };
>>>
>>> The node name should not have underscores so
>>> pruss_mii_rt: mii-rt@32000. Please check the others
>>> too, like app_node.
>>>
>>
>> OK.
>>
>>>> +    app_node: app_node {
>>>> +        prus = <&pru0>, <&pru1>;
>>>> +        firmware-name = "pruss-app-fw", "pruss-app-fw-2";
>>>> +        ti,pruss-gp-mux-sel = <2>, <1>;
>>>> +        /* setup interrupts for prus:
>>>> +           prus[0] => pru1_0: ev=16, chnl=2, host-irq=7,
>>>> +           prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */
>>>> +        ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
>>>> +    }
>>>
>>> If the ti,pruss-gp-mux-sel and ti,pru-interrupt-map are
>>> firmware configuration options, maybe leave them out of
>>> the dts completely and make the app-node optional.
>>
>> Yes the app-node is optional. I will mention it.
>>
>> No, ti,pruss-gp-mux-sel and ti,pru-interrupt-map are not firmware options.
>> But these settings are application/firmware specific.
>>
>> ti,pru-interrupt-map specifies the configuration to be used for the INTC interrupt
>> controller.
>>
>> ti,pruss-gp-mux-sel is used to configure this register.
>> "Table 30-20. PRUSS_GPCFG0" in http://www.tij.co.jp/jp/lit/ug/spruhz7h/spruhz7h.pdf
>> "29:26 PR1_PRU0_GP_MUX_SEL"
>>
>> It configures how the pins from the PRUSS module are routed internally
>> to the various modules.
>>
>> see "30.2.1 PRU-ICSS I/O Interface"
>> and "Table 30-1. PRU-ICSS1 I/O Signals"
>>
>>>
>>> And have a proper compatible for this node such as
>>> "ti,pruss-app-xyz". And this should be only set if the the
>>> hardware is wired up in such way that things need to be
>>> configured in the dts rather than by the firmware.
>>
>> Yes, compatible is a required property as we need to load
>> the appropriate application (kernel space) driver for it.
>> I will fix the example.
>>
>>>
>>> And then you can just hide mux-sel and interrupt-map
>>> behind the compatible property for that hardware. And
>>> leave them out from the dts and have the handling driver
>>> would set mux-sel and interrupt-map based on the
>>> match->data during probe.
>>
>> To summarize:
>>
>> I'll mark the app node as optional. Only required if a kernel
>> driver is required for the application.
>> Compatible is mandatory for app node.
>> ti,pruss-gp-mux-sel and ti,pru-interrupt-map are optional
>> for app node.
>>
>> cheers,
>> -roger
>>
Murali Karicheri Feb. 5, 2019, 4:15 p.m. | #5
Roger,

On 02/05/2019 10:41 AM, Roger Quadros wrote:
> Murali,
> 
> On 05/02/19 17:08, Murali Karicheri wrote:
>> Hi Roger,
>>
>> On 02/05/2019 04:39 AM, Roger Quadros wrote:
>>> Hi Tony & Suman,
>>>
>>> On 04/02/19 18:33, Tony Lindgren wrote:
>>>> Hi,
>>>>
>>>> * Roger Quadros <rogerq@ti.com> [190204 14:23]:
>>>>> From: Suman Anna <s-anna@ti.com>
>>>> ...
>>>>> +Example:
>>>>> +========
>>>>> +1.    /* AM33xx PRU-ICSS */
>>>>> +
>>>>> +    pruss: pruss@0 {
>>>>> +        compatible = "ti,am3356-pruss";
>>>>> +        reg = <0x0 0x2000>,
>>>>> +              <0x2000 0x2000>,
>>>>> +              <0x10000 0x3000>;
>>>>> +        reg-names = "dram0", "dram1",
>>>>> +                "shrdram2";
>>>>> +        #address-cells = <1>;
>>>>> +        #size-cells = <1>;
>>>>> +        ranges;
>>>>
>>>> Thanks for fixing up the reg ranges for the top level node.
>>>>
>>>> Ideally there would not even be a top level node here as
>>>> AFAIK the whole PRUSS is a collection of devices on a PRU
>>>> internal interconnect. So following that path a bit further..
>>>> How about just get rid of the top level node and just do:
>>>>
>>>> pruss: pruss@0 {
>>>>      dram0: memory@0 {
>>>>             device_type = "memory";
>>>>             reg = <0x0 0x2000>;
>>>>      };
>>>>
>>>>      dram1: memory@2000 {
>>>>             device_type = "memory";
>>>>             reg = <0x2000 0x2000>;
>>>>      };
>>>
>>> Actually dram0 and dram1 are data memories for PRU0 and PRU1 respectively.
>>> Isn't it better if they are moved to the pru node?
>>> e.g.
>>>
>>>      pru0: pru@34000 {
>>>          compatible = "ti,am3356-pru";
>>>          reg = <0x34000 0x2000>,
>>>                <0x22000 0x400>,
>>>                <0x22400 0x100>,
>>>                <0x0     0x2000>;
>>>          reg-names = "iram", "control", "debug", "dram";
>>>          ...
>>>      };
>>>
>>>      pru1: pru@38000 {
>>>          compatible = "ti,am3356-pru";
>>>          reg = <0x38000 0x2000>,
>>>                <0x24000 0x400>,
>>>                <0x24400 0x100>,
>>>                <0x2000  0x2000>;
>>>          reg-names = "iram", "control", "debug", "dram";
>>>          ...
>>>      };
>>>
>>> I think it is better to place a restriction that firmware on PRU0 cannot use data
>>> memory of PRU1 and vice versa.
>>>
>> That will not work as there are switch firmware cases where PRU access
>> DRAM of other PRU and is a valid case to support in the future. So let
>> us not do that.
> 
> PRU firmware accessing DRAM of other PRU is a design contract and that use case
> requires both PRUs to be loaded with matching firmware. That should continue to work.
> 
> What I'm suggesting here is that kernel remoteproc driver should have nothing to do
> with the other PRU's data RAM.
> 
> The application driver if needs both PRUs then it can obviously access both DRAMs
> as it has a phandle to both PRUs.
> 
That should be fine.

Regards,
Murali

> cheers,
> -roger
> 
>>
>> Murali
>>> Application drivers do sometimes need to read/write to data memory. The pru_rproc
>>> driver could provide a API for the application drivers to get virtual address of
>>> the respective PRU's data memory.
>>>
>>>>
>>>>      shrdram2: memory@10000 {
>>>>          device_type = "memory";
>>>>          reg = <0x10000 0x3000>;
>>>>      };
>>>
>>> Shared RAM is not so straight forward. Both PRU firmwares and both application drivers
>>> might need to read/write here. The area split is decided by firmware design and there
>>> is no hardware protection to prevent from stomping on each others toes.
>>>
>>> We need a carveout based memory allocator at least I think that can do a
>>> allocate(base_offset, size); into shared RAM.
>>>
>>> This could be used by pru_rproc driver at firmware load time and by application drivers
>>> at initialization time.
>>>
>>> Thoughts?
>>>
>>>>
>>>>      pruss_cfg: cfg@26000 {
>>>>          ...
>>>>      };
>>>>      ...
>>>> };
>>>>
>>>> If the device_type = "memory" cannot be used here for
>>>> being specific to the top level properties, then
>>>> there's probably some other generic property usable
>>>> here :)
>>>>
>>>>> +        pruss_mii_rt: mii_rt@32000 {
>>>>> +            reg = <0x32000 0x58>;
>>>>> +        };
>>>>
>>>> The node name should not have underscores so
>>>> pruss_mii_rt: mii-rt@32000. Please check the others
>>>> too, like app_node.
>>>>
>>>
>>> OK.
>>>
>>>>> +    app_node: app_node {
>>>>> +        prus = <&pru0>, <&pru1>;
>>>>> +        firmware-name = "pruss-app-fw", "pruss-app-fw-2";
>>>>> +        ti,pruss-gp-mux-sel = <2>, <1>;
>>>>> +        /* setup interrupts for prus:
>>>>> +           prus[0] => pru1_0: ev=16, chnl=2, host-irq=7,
>>>>> +           prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */
>>>>> +        ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
>>>>> +    }
>>>>
>>>> If the ti,pruss-gp-mux-sel and ti,pru-interrupt-map are
>>>> firmware configuration options, maybe leave them out of
>>>> the dts completely and make the app-node optional.
>>>
>>> Yes the app-node is optional. I will mention it.
>>>
>>> No, ti,pruss-gp-mux-sel and ti,pru-interrupt-map are not firmware options.
>>> But these settings are application/firmware specific.
>>>
>>> ti,pru-interrupt-map specifies the configuration to be used for the INTC interrupt
>>> controller.
>>>
>>> ti,pruss-gp-mux-sel is used to configure this register.
>>> "Table 30-20. PRUSS_GPCFG0" in http://www.tij.co.jp/jp/lit/ug/spruhz7h/spruhz7h.pdf
>>> "29:26 PR1_PRU0_GP_MUX_SEL"
>>>
>>> It configures how the pins from the PRUSS module are routed internally
>>> to the various modules.
>>>
>>> see "30.2.1 PRU-ICSS I/O Interface"
>>> and "Table 30-1. PRU-ICSS1 I/O Signals"
>>>
>>>>
>>>> And have a proper compatible for this node such as
>>>> "ti,pruss-app-xyz". And this should be only set if the the
>>>> hardware is wired up in such way that things need to be
>>>> configured in the dts rather than by the firmware.
>>>
>>> Yes, compatible is a required property as we need to load
>>> the appropriate application (kernel space) driver for it.
>>> I will fix the example.
>>>
>>>>
>>>> And then you can just hide mux-sel and interrupt-map
>>>> behind the compatible property for that hardware. And
>>>> leave them out from the dts and have the handling driver
>>>> would set mux-sel and interrupt-map based on the
>>>> match->data during probe.
>>>
>>> To summarize:
>>>
>>> I'll mark the app node as optional. Only required if a kernel
>>> driver is required for the application.
>>> Compatible is mandatory for app node.
>>> ti,pruss-gp-mux-sel and ti,pru-interrupt-map are optional
>>> for app node.
>>>
>>> cheers,
>>> -roger
>>>
>
Tony Lindgren Feb. 5, 2019, 4:19 p.m. | #6
* Murali Karicheri <m-karicheri2@ti.com> [190205 16:13]:
> On 02/05/2019 10:41 AM, Roger Quadros wrote:
> > What I'm suggesting here is that kernel remoteproc driver should have nothing to do
> > with the other PRU's data RAM.
> > 
> > The application driver if needs both PRUs then it can obviously access both DRAMs
> > as it has a phandle to both PRUs.
> > 
> That should be fine.

That sounds good to me too.

For dts, yeah please allocate the resources for the modules
where the resources belong to on the PRUSS internal interconnect :)
Devices can move around on the interconnect between SoCs and the
modules can get swapped or added.

Regards,

Tony
Tony Lindgren Feb. 5, 2019, 4:41 p.m. | #7
* Roger Quadros <rogerq@ti.com> [190205 09:40]:
> On 04/02/19 18:33, Tony Lindgren wrote:
> > 
> > 	shrdram2: memory@10000 {
> > 		device_type = "memory";
> > 		reg = <0x10000 0x3000>;
> > 	};
> 
> Shared RAM is not so straight forward. Both PRU firmwares and both application drivers
> might need to read/write here. The area split is decided by firmware design and there
> is no hardware protection to prevent from stomping on each others toes.
> 
> We need a carveout based memory allocator at least I think that can do a
> allocate(base_offset, size); into shared RAM.
> 
> This could be used by pru_rproc driver at firmware load time and by application drivers
> at initialization time.
> 
> Thoughts?

That sounds sane to me :)

> > If the ti,pruss-gp-mux-sel and ti,pru-interrupt-map are
> > firmware configuration options, maybe leave them out of
> > the dts completely and make the app-node optional.
> 
> Yes the app-node is optional. I will mention it.
> 
> No, ti,pruss-gp-mux-sel and ti,pru-interrupt-map are not firmware options.
> But these settings are application/firmware specific.
> 
> ti,pru-interrupt-map specifies the configuration to be used for the INTC interrupt
> controller.

OK. So just to see if we have a standard solution available already..
It sounds a bit similar to what we're doing with omap-wakeupgen.c
and stacked interrupts? I wonder if something similar might help
here?

> ti,pruss-gp-mux-sel is used to configure this register.
> "Table 30-20. PRUSS_GPCFG0" in http://www.tij.co.jp/jp/lit/ug/spruhz7h/spruhz7h.pdf
> "29:26 PR1_PRU0_GP_MUX_SEL"
> 
> It configures how the pins from the PRUSS module are routed internally
> to the various modules.
> 
> see "30.2.1 PRU-ICSS I/O Interface"
> and "Table 30-1. PRU-ICSS1 I/O Signals"

Well these are external signals for PRUSS processor (although not
necessarily external signals for the SoC). So why not handle them
with a standard pinctlr binding with #pinctrl-cells?

Sure it may not even be the Linux pinctrl framework running on the
main SoC handling these pins, but after all you're describing
hardware for a processor. Maybe Linus W has some comments on this?

Regards,

Tony
Roger Quadros Feb. 6, 2019, 3:04 p.m. | #8
On 05/02/19 18:19, Tony Lindgren wrote:
> * Murali Karicheri <m-karicheri2@ti.com> [190205 16:13]:
>> On 02/05/2019 10:41 AM, Roger Quadros wrote:
>>> What I'm suggesting here is that kernel remoteproc driver should have nothing to do
>>> with the other PRU's data RAM.
>>>
>>> The application driver if needs both PRUs then it can obviously access both DRAMs
>>> as it has a phandle to both PRUs.
>>>
>> That should be fine.
> 
> That sounds good to me too.
> 
> For dts, yeah please allocate the resources for the modules
> where the resources belong to on the PRUSS internal interconnect :)
> Devices can move around on the interconnect between SoCs and the
> modules can get swapped or added.

If you take a look at "Figure 30-1. PRU-ICSS Overview" in 
http://www.tij.co.jp/jp/lit/ug/spruhz7h/spruhz7h.pdf

You can see that DRAM0 and DRAM1 are not part of PRU. That means
they shouldn't be in the PRU node then.

cheers,
-roger
Linus Walleij Feb. 8, 2019, 1:51 p.m. | #9
On Mon, Feb 4, 2019 at 3:24 PM Roger Quadros <rogerq@ti.com> wrote:

> From: Suman Anna <s-anna@ti.com>
>
> This patch adds the bindings for the Programmable Real-Time Unit
> and Industrial Communication Subsystem (PRU-ICSS) present on various
> SoCs such as AM33xx, AM437x, AM57xx, Keystone 66AK2G SoC, etc. It is
> present on the Davinci based OMAPL138 SoCs and K3 architecture
> based AM65x SoCs as well (not covered for now).
>
> Signed-off-by: Suman Anna <s-anna@ti.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>

(...)
> +               pruss_intc: intc@20000 {
> +                       compatible = "ti,am3356-pruss-intc";
> +                       reg = <0x20000 0x2000>;
> +                       reg-names = "intc";
> +                       interrupt-controller;
> +                       #interrupt-cells = <1>;
> +                       interrupts = <20 21 22 23 24 25 26 27>;
> +                       interrupt-names = "host2", "host3", "host4",
> +                                         "host5", "host6", "host7",
> +                                         "host8", "host9";

If thsese interrupts are mapped 1-to-1 to a parent interrupt controller
then this is a hierarchical interrupt domain and then these should
be handled locally in the driver as offset from child to parent
statically encoded in the driver.

Several old drivers and old device tree bindings make this kind
of maps, but it is not how we do it anymore, if we can avoid it.

To be able to use hierarchical interrupt domain in the kernel, the top
interrupt controller must use the hierarchical (v2) irqdomain, so
if this is anything else than the ARM GIC it will be an interesting
undertaking to handle this.

The more I understand of hierarchical irqdomains, the more of
workarounds where we should be using it I see, we really need
to spread this knowledge. Using it requires a lot of upfront work
sometimes, sorry about that but the end result is so much better.

Yours,
Linus Walleij
Suman Anna Feb. 14, 2019, 2:47 a.m. | #10
On 2/6/19 9:04 AM, Roger Quadros wrote:
> 
> 
> On 05/02/19 18:19, Tony Lindgren wrote:
>> * Murali Karicheri <m-karicheri2@ti.com> [190205 16:13]:
>>> On 02/05/2019 10:41 AM, Roger Quadros wrote:
>>>> What I'm suggesting here is that kernel remoteproc driver should have nothing to do
>>>> with the other PRU's data RAM.
>>>>
>>>> The application driver if needs both PRUs then it can obviously access both DRAMs
>>>> as it has a phandle to both PRUs.
>>>>
>>> That should be fine.
>>
>> That sounds good to me too.
>>
>> For dts, yeah please allocate the resources for the modules
>> where the resources belong to on the PRUSS internal interconnect :)
>> Devices can move around on the interconnect between SoCs and the
>> modules can get swapped or added.
> 
> If you take a look at "Figure 30-1. PRU-ICSS Overview" in 
> http://www.tij.co.jp/jp/lit/ug/spruhz7h/spruhz7h.pdf
> 
> You can see that DRAM0 and DRAM1 are not part of PRU. That means
> they shouldn't be in the PRU node then.

Yes, they do not belong to a PRU, and should not be defined underneath
one. Both are accessible from both PRU cores, so it is upto the
application on how they can partition the usage.

regards
Suman
Suman Anna Feb. 14, 2019, 2:52 a.m. | #11
Hi Roger,

On 2/4/19 8:22 AM, Roger Quadros wrote:
> From: Suman Anna <s-anna@ti.com>
> 
> This patch adds the bindings for the Programmable Real-Time Unit
> and Industrial Communication Subsystem (PRU-ICSS) present on various
> SoCs such as AM33xx, AM437x, AM57xx, Keystone 66AK2G SoC, etc. It is
> present on the Davinci based OMAPL138 SoCs and K3 architecture
> based AM65x SoCs as well (not covered for now).
> 
> Signed-off-by: Suman Anna <s-anna@ti.com>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  .../devicetree/bindings/soc/ti/ti,pruss.txt        | 212 +++++++++++++++++++++
>  1 file changed, 212 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
> 
> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
> new file mode 100644
> index 0000000..5ac76fd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
> @@ -0,0 +1,212 @@
> +PRU-ICSS on TI SoCs
> +===================
> +
> +The Programmable Real-Time Unit and Industrial Communication Subsystem
> +(PRU-ICSS) is present on various TI SoCs such as AM335x, AM437x, Keystone
> +66AK2G, etc. A PRUSS consists of dual 32-bit RISC cores (Programmable
> +Real-Time Units, or PRUs) with program memory and data memory.
> +
> +The programmable nature of the PRUs provide flexibility to implement
> +custom peripheral interfaces, fast real-time responses, or specialized
> +data handling. The common peripheral modules include the following,
> +
> +  - Enhanced GPIO with async capture and serial support
> +  - an Ethernet MII_RT module with two MII ports
> +  - an MDIO port to control external Ethernet PHYs
> +  - an Industrial Ethernet Peripheral (IEP) to manage/generate Industrial
> +    Ethernet functions
> +  - an Enhanced Capture Module (eCAP)
> +  - a 16550-compatible UART to support PROFIBUS
> +  - Interrupt controller with 64 input events and 10 Host interrupts.
> +
> +A shared Data RAM, if present, can be accessed by both the PRU cores. The
> +Interrupt Controller (INTC) and a CFG module are common to both the PRU
> +cores.
> +
> +Various sub-modules within a PRU-ICSS subsystem are represented as individual
> +nodes.
> +
> +PRUSS Node
> +=============
> +
> +This node represents the entire ICSS instance and the various modules are
> +contained as children. The PRUSS driver is responsible for managing the
> +common resources i.e. DRAM0, DRAM1, SHARED_RAM and CFG space.
> +
> +Required Properties:
> +--------------------
> +- compatible     : should be one of,
> +                       "ti,am3356-pruss" for AM335x family of SoCs
> +                       "ti,am4376-pruss" for AM437x family of SoCs
> +                       "ti,am5728-pruss" for AM57xx family of SoCs
> +                       "ti,k2g-pruss" for 66AK2G family of SoCs
> +- reg            : base address and size for each of the Data RAMs as
> +                   mentioned in reg-names, and in the same order as the
> +                   reg-names

Hmm, not sure what prompted you to deviate from the design we had on TI
SDK kernels. Your revised bindings does not allow us to use this device
for the scalability with either UIO/VFIO.

Let's sort this out before you post the next series.

regards
Suman

> +- reg-names      : should contain a string(s) from among the following names,
> +                   each representing a specific Data RAM region. Some PRU-ICSS
> +		   instances on certain SoCs might not have Shared DRAM.
> +                       "dram0" for Data RAM0,
> +                       "dram1" for Data RAM1,
> +                       "shrdram2" for Shared Data RAM,
> +- #address-cells : should be 1
> +- #size-cells    : should be 1
> +- ranges         : no specific range translations required, child nodes have the
> +                   same address view as the parent, so should be mentioned without
> +                   any value for the property
> +
> +Optional Properties:
> +--------------------
> +- no-shared-ram	: Should be present if the instance doesn't have Shared RAM.
> +		  e.g. AM4376 ICSS0 instance doesn't have Shared RAM.
> +
> +The PRUSS node will have one or more of the folowing child nodes.
> +
> +PRU CORES
> +=========
> +ICSS typically has 2 PRU cores. These should be represented as remoteproc devices.
> +
> +INTC node
> +=========
> +ICSS has one INTC interrupt controller module. This should be represented as
> +a standard interrupt-controller node.
> +
> +CFG, IEP, MII_RT
> +================
> +The individual sub-modules CFG, IEP and MII_RT are represented as a syscon
> +node each with specific node names as below:
> +                  "cfg" for CFG sub-module,
> +                  "iep" for IEP sub-module,
> +                  "mii_rt" for MII-RT sub-module,
> +
> +See Documentation/devicetree/bindings/mfd/syscon.txt for details.
> +
> +MDIO
> +====
> +Each PRUSS has an MDIO module that can be used to control external PHYs. The
> +MDIO module used within the PRU-ICSS is an instance of the MDIO Controller
> +used in TI Davinci SoCs. Please refer to the corresponding binding document,
> +Documentation/devicetree/bindings/net/davinci-mdio.txt for details.
> +
> +Application/User Nodes
> +=======================
> +A PRU application/user node typically uses one or more PRU device nodes to
> +implement a PRU application/functionality. Each application/client node would
> +need a reference to at least a PRU node, and optionally pass some configuration
> +parameters.
> +
> +Required Properties:
> +--------------------
> +- prus                 : phandles to the PRU nodes used
> +
> +Optional Properties:
> +--------------------
> +- firmware-name        : firmwares for the PRU cores, the default firmware
> +                         for the core from the PRU node will be used if not
> +                         provided. The firmware names should correspond to
> +                         the PRU cores listed in the 'prus' property
> +- ti,pruss-gp-mux-sel  : array of values for the GP_MUX_SEL under PRUSS_GPCFG
> +                         register for a PRU. This selects the internal muxing
> +                         scheme for the PRU instance. If not provided, the
> +                         default out-of-reset value (0) for the PRU core is
> +                         used. Values should correspond to the PRU cores listed
> +                         in the 'prus' property
> +- ti,pru-interrupt-map : PRU interrupt mappings, containing an array of entries
> +                         with each entry consisting of 4 cell-values. First one
> +                         is an index towards the "prus" property to identify the
> +                         PRU core for the interrupt map, second is the PRU
> +                         System Event id, third is the PRU interrupt channel id
> +                         and fourth is the PRU host interrupt id. If provided,
> +                         this map will supercede any other configuration
> +                         provided through firmware
> +
> +Example:
> +========
> +1.	/* AM33xx PRU-ICSS */
> +
> +	pruss: pruss@0 {
> +		compatible = "ti,am3356-pruss";
> +		reg = <0x0 0x2000>,
> +		      <0x2000 0x2000>,
> +		      <0x10000 0x3000>;
> +		reg-names = "dram0", "dram1",
> +			    "shrdram2";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		pruss_cfg: cfg@26000 {
> +			compatible = "syscon";
> +			reg = <0x26000 0x2000>;
> +		};
> +
> +		pruss_iep: iep@2e000 {
> +			compatible = "syscon";
> +			reg = <0x2e000 0x31c>;
> +		};
> +
> +		pruss_mii_rt: mii_rt@32000 {
> +			compatible = "syscon";
> +			reg = <0x32000 0x58>;
> +		};
> +
> +		pruss_intc: intc@20000 {
> +			compatible = "ti,am3356-pruss-intc";
> +			reg = <0x20000 0x2000>;
> +			reg-names = "intc";
> +			interrupt-controller;
> +			#interrupt-cells = <1>;
> +			interrupts = <20 21 22 23 24 25 26 27>;
> +			interrupt-names = "host2", "host3", "host4",
> +					  "host5", "host6", "host7",
> +					  "host8", "host9";
> +		};
> +
> +		pru0: pru@34000 {
> +			compatible = "ti,am3356-pru";
> +			reg = <0x34000 0x2000>,
> +			      <0x22000 0x400>,
> +			      <0x22400 0x100>;
> +			reg-names = "iram", "control", "debug";
> +			gpcfg = <&pruss_cfg 0x8>;
> +			firmware-name = "am335x-pru0-fw";
> +			interrupt-parent = <&pruss_intc>;
> +			interrupts = <16>, <17>;
> +			interrupt-names = "vring", "kick";
> +		};
> +
> +		pru1: pru@38000 {
> +			compatible = "ti,am3356-pru";
> +			reg = <0x38000 0x2000>,
> +			      <0x24000 0x400>,
> +			      <0x24400 0x100>;
> +			reg-names = "iram", "control", "debug";
> +			gpcfg = <&pruss_cfg 0xc>;
> +			firmware-name = "am335x-pru1-fw";
> +			interrupt-parent = <&pruss_intc>;
> +			interrupts = <18>, <19>;
> +			interrupt-names = "vring", "kick";
> +		};
> +
> +		pruss_mdio: mdio@32400 {
> +			compatible = "ti,davinci_mdio";
> +			reg = <0x32400 0x90>;
> +			clocks = <&dpll_core_m4_ck>;
> +			clock-names = "fck";
> +			bus_freq = <1000000>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			status = "disabled";
> +		};
> +	};
> +
> +2:	/* PRU application node example */
> +	app_node: app_node {
> +		prus = <&pru0>, <&pru1>;
> +		firmware-name = "pruss-app-fw", "pruss-app-fw-2";
> +		ti,pruss-gp-mux-sel = <2>, <1>;
> +		/* setup interrupts for prus:
> +		   prus[0] => pru1_0: ev=16, chnl=2, host-irq=7,
> +		   prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */
> +		ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
> +	}
>
Suman Anna Feb. 14, 2019, 3:01 a.m. | #12
On 2/5/19 10:41 AM, Tony Lindgren wrote:
> * Roger Quadros <rogerq@ti.com> [190205 09:40]:
>> On 04/02/19 18:33, Tony Lindgren wrote:
>>>
>>> 	shrdram2: memory@10000 {
>>> 		device_type = "memory";
>>> 		reg = <0x10000 0x3000>;
>>> 	};
>>
>> Shared RAM is not so straight forward. Both PRU firmwares and both application drivers
>> might need to read/write here. The area split is decided by firmware design and there
>> is no hardware protection to prevent from stomping on each others toes.
>>
>> We need a carveout based memory allocator at least I think that can do a
>> allocate(base_offset, size); into shared RAM.
>>
>> This could be used by pru_rproc driver at firmware load time and by application drivers
>> at initialization time.
>>
>> Thoughts?
> 
> That sounds sane to me :)
> 
>>> If the ti,pruss-gp-mux-sel and ti,pru-interrupt-map are
>>> firmware configuration options, maybe leave them out of
>>> the dts completely and make the app-node optional.
>>
>> Yes the app-node is optional. I will mention it.
>>
>> No, ti,pruss-gp-mux-sel and ti,pru-interrupt-map are not firmware options.
>> But these settings are application/firmware specific.
>>
>> ti,pru-interrupt-map specifies the configuration to be used for the INTC interrupt
>> controller.
> 
> OK. So just to see if we have a standard solution available already..
> It sounds a bit similar to what we're doing with omap-wakeupgen.c
> and stacked interrupts? I wonder if something similar might help
> here?
> 
>> ti,pruss-gp-mux-sel is used to configure this register.
>> "Table 30-20. PRUSS_GPCFG0" in http://www.tij.co.jp/jp/lit/ug/spruhz7h/spruhz7h.pdf
>> "29:26 PR1_PRU0_GP_MUX_SEL"
>>
>> It configures how the pins from the PRUSS module are routed internally
>> to the various modules.

Actually, that's not entirely accurate. This is an internal pinmux (not
controllable per pin, but rather dictates a different sets of groups of
pins at the PRUSS boundary, which are then again multiplexed at the SoC
level using the standard padconf/pinmux. It is a single register per
core into which you can set some values between 0 through 4 IIRC
(unfortunately the values are also not uniform across the various SoCs).

regards
Suman

>>
>> see "30.2.1 PRU-ICSS I/O Interface"
>> and "Table 30-1. PRU-ICSS1 I/O Signals"
> 
> Well these are external signals for PRUSS processor (although not
> necessarily external signals for the SoC). So why not handle them
> with a standard pinctlr binding with #pinctrl-cells?
> 
> Sure it may not even be the Linux pinctrl framework running on the
> main SoC handling these pins, but after all you're describing
> hardware for a processor. Maybe Linus W has some comments on this?
> 
> Regards,
> 
> Tony
>
Suman Anna Feb. 14, 2019, 3:12 a.m. | #13
On 2/8/19 7:51 AM, Linus Walleij wrote:
> On Mon, Feb 4, 2019 at 3:24 PM Roger Quadros <rogerq@ti.com> wrote:
> 
>> From: Suman Anna <s-anna@ti.com>
>>
>> This patch adds the bindings for the Programmable Real-Time Unit
>> and Industrial Communication Subsystem (PRU-ICSS) present on various
>> SoCs such as AM33xx, AM437x, AM57xx, Keystone 66AK2G SoC, etc. It is
>> present on the Davinci based OMAPL138 SoCs and K3 architecture
>> based AM65x SoCs as well (not covered for now).
>>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
> 
> (...)
>> +               pruss_intc: intc@20000 {
>> +                       compatible = "ti,am3356-pruss-intc";
>> +                       reg = <0x20000 0x2000>;
>> +                       reg-names = "intc";
>> +                       interrupt-controller;
>> +                       #interrupt-cells = <1>;
>> +                       interrupts = <20 21 22 23 24 25 26 27>;
>> +                       interrupt-names = "host2", "host3", "host4",
>> +                                         "host5", "host6", "host7",
>> +                                         "host8", "host9";
> 
> If thsese interrupts are mapped 1-to-1 to a parent interrupt controller
> then this is a hierarchical interrupt domain and then these should
> be handled locally in the driver as offset from child to parent
> statically encoded in the driver.
> 
> Several old drivers and old device tree bindings make this kind
> of maps, but it is not how we do it anymore, if we can avoid it.
> 
> To be able to use hierarchical interrupt domain in the kernel, the top
> interrupt controller must use the hierarchical (v2) irqdomain, so
> if this is anything else than the ARM GIC it will be an interesting
> undertaking to handle this.

These are interrupt lines coming towards the host processor running
Linux and are directly connected to the ARM GIC. This INTC module is
actually an PRUSS internal interrupt controller that can take in 64 (on
most SoCs) external events/interrupt sources and multiplexing them
through two layers of many-to-one events-to-intr channels &
intr-channels-to-host interrupts. Couple of the host interrupts go to
the PRU cores themselves while the remaining ones come out of the IP to
connect to other GICs in the SoC.

We have implemented this as an irqchip using chained interrupt handlers
with the consumers using the event numbers on the Linux-side. The PRUs
also access some of the associated registers for clearing an event source.

regards
Suman

> 
> The more I understand of hierarchical irqdomains, the more of
> workarounds where we should be using it I see, we really need
> to spread this knowledge. Using it requires a lot of upfront work
> sometimes, sorry about that but the end result is so much better.
> 
> Yours,
> Linus Walleij
>
Linus Walleij Feb. 14, 2019, 8:37 a.m. | #14
On Thu, Feb 14, 2019 at 4:13 AM Suman Anna <s-anna@ti.com> wrote:
> [Me]

> > To be able to use hierarchical interrupt domain in the kernel, the top
> > interrupt controller must use the hierarchical (v2) irqdomain, so
> > if this is anything else than the ARM GIC it will be an interesting
> > undertaking to handle this.
>
> These are interrupt lines coming towards the host processor running
> Linux and are directly connected to the ARM GIC. This INTC module is
> actually an PRUSS internal interrupt controller that can take in 64 (on
> most SoCs) external events/interrupt sources and multiplexing them
> through two layers of many-to-one events-to-intr channels &
> intr-channels-to-host interrupts. Couple of the host interrupts go to
> the PRU cores themselves while the remaining ones come out of the IP to
> connect to other GICs in the SoC.

If the muxing is static (like set up once at probe) so that while the system is
running, there is one and one only event mapped to the GIC from
the component below it, then it is hierarchical.

> We have implemented this as an irqchip using chained interrupt handlers
> with the consumers using the event numbers on the Linux-side. The PRUs
> also access some of the associated registers for clearing an event source.

Chaining with cascading is when two or more interrupts fire the
same upper level (say GIC) IRQ. If there is a 1:1 mapping,
it is not chained/cascaded but hierarchical.

I understand you used old irqdomain/chip frameworks in the past,
because everyone was working around the fact that they didn't have
an abstraction for hierarchical IRQs. Using chained interrupts
and custom 1:1 maps and assigning a long list of IRQs like this
patch does was the most common workaround. But we should
step out of that habit now.

Different levels of the IRQ handling having to do different stuff is
what hierarchical irqdomains do best, so it sounds like a good fit.

We handle some stuff at our level of the hierarchy and then fall
up to the next higher level using calls such as
irq_chip_ack_parent(), irq_chip_mask_parent() and friends.

Yours,
Linus Walleij
Roger Quadros Feb. 14, 2019, 10:55 a.m. | #15
On 14/02/19 10:37, Linus Walleij wrote:
> On Thu, Feb 14, 2019 at 4:13 AM Suman Anna <s-anna@ti.com> wrote:
>> [Me]
> 
>>> To be able to use hierarchical interrupt domain in the kernel, the top
>>> interrupt controller must use the hierarchical (v2) irqdomain, so
>>> if this is anything else than the ARM GIC it will be an interesting
>>> undertaking to handle this.
>>
>> These are interrupt lines coming towards the host processor running
>> Linux and are directly connected to the ARM GIC. This INTC module is
>> actually an PRUSS internal interrupt controller that can take in 64 (on
>> most SoCs) external events/interrupt sources and multiplexing them
>> through two layers of many-to-one events-to-intr channels &
>> intr-channels-to-host interrupts. Couple of the host interrupts go to
>> the PRU cores themselves while the remaining ones come out of the IP to
>> connect to other GICs in the SoC.
> 
> If the muxing is static (like set up once at probe) so that while the system is
> running, there is one and one only event mapped to the GIC from
> the component below it, then it is hierarchical.

This is how it looks.

[GIC]<---8---[INTC]<---64---[events from peripherals]

The 8 interrupt lines from INTC to the GIC are 1:1 mapped and fixed per SoC.
The muxing between 64 inputs to INTC and its 8 outputs are programmable
and might not necessarily be static per boot/probe as it depends on what firmware
is loaded on the PRU.

A typical PRUSS use case will usually use just one firmware per boot but if required it
can switch at runtime and the muxing might change.

> 
>> We have implemented this as an irqchip using chained interrupt handlers
>> with the consumers using the event numbers on the Linux-side. The PRUs
>> also access some of the associated registers for clearing an event source.
> 
> Chaining with cascading is when two or more interrupts fire the
> same upper level (say GIC) IRQ. If there is a 1:1 mapping,
> it is not chained/cascaded but hierarchical.
> 
> I understand you used old irqdomain/chip frameworks in the past,
> because everyone was working around the fact that they didn't have
> an abstraction for hierarchical IRQs. Using chained interrupts
> and custom 1:1 maps and assigning a long list of IRQs like this
> patch does was the most common workaround. But we should
> step out of that habit now.
> 
> Different levels of the IRQ handling having to do different stuff is
> what hierarchical irqdomains do best, so it sounds like a good fit.
> 
> We handle some stuff at our level of the hierarchy and then fall
> up to the next higher level using calls such as
> irq_chip_ack_parent(), irq_chip_mask_parent() and friends.
> 
> Yours,
> Linus Walleij
> 

--
cheers,
-roger
 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Roger Quadros Feb. 14, 2019, 11:08 a.m. | #16
+Robert, Dan, Matthijs

Hi Suman,

On 14/02/19 04:52, Suman Anna wrote:
> Hi Roger,
> 
> On 2/4/19 8:22 AM, Roger Quadros wrote:
>> From: Suman Anna <s-anna@ti.com>
>>
>> This patch adds the bindings for the Programmable Real-Time Unit
>> and Industrial Communication Subsystem (PRU-ICSS) present on various
>> SoCs such as AM33xx, AM437x, AM57xx, Keystone 66AK2G SoC, etc. It is
>> present on the Davinci based OMAPL138 SoCs and K3 architecture
>> based AM65x SoCs as well (not covered for now).
>>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  .../devicetree/bindings/soc/ti/ti,pruss.txt        | 212 +++++++++++++++++++++
>>  1 file changed, 212 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
>>
>> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
>> new file mode 100644
>> index 0000000..5ac76fd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
>> @@ -0,0 +1,212 @@
>> +PRU-ICSS on TI SoCs
>> +===================
>> +
>> +The Programmable Real-Time Unit and Industrial Communication Subsystem
>> +(PRU-ICSS) is present on various TI SoCs such as AM335x, AM437x, Keystone
>> +66AK2G, etc. A PRUSS consists of dual 32-bit RISC cores (Programmable
>> +Real-Time Units, or PRUs) with program memory and data memory.
>> +
>> +The programmable nature of the PRUs provide flexibility to implement
>> +custom peripheral interfaces, fast real-time responses, or specialized
>> +data handling. The common peripheral modules include the following,
>> +
>> +  - Enhanced GPIO with async capture and serial support
>> +  - an Ethernet MII_RT module with two MII ports
>> +  - an MDIO port to control external Ethernet PHYs
>> +  - an Industrial Ethernet Peripheral (IEP) to manage/generate Industrial
>> +    Ethernet functions
>> +  - an Enhanced Capture Module (eCAP)
>> +  - a 16550-compatible UART to support PROFIBUS
>> +  - Interrupt controller with 64 input events and 10 Host interrupts.
>> +
>> +A shared Data RAM, if present, can be accessed by both the PRU cores. The
>> +Interrupt Controller (INTC) and a CFG module are common to both the PRU
>> +cores.
>> +
>> +Various sub-modules within a PRU-ICSS subsystem are represented as individual
>> +nodes.
>> +
>> +PRUSS Node
>> +=============
>> +
>> +This node represents the entire ICSS instance and the various modules are
>> +contained as children. The PRUSS driver is responsible for managing the
>> +common resources i.e. DRAM0, DRAM1, SHARED_RAM and CFG space.
>> +
>> +Required Properties:
>> +--------------------
>> +- compatible     : should be one of,
>> +                       "ti,am3356-pruss" for AM335x family of SoCs
>> +                       "ti,am4376-pruss" for AM437x family of SoCs
>> +                       "ti,am5728-pruss" for AM57xx family of SoCs
>> +                       "ti,k2g-pruss" for 66AK2G family of SoCs
>> +- reg            : base address and size for each of the Data RAMs as
>> +                   mentioned in reg-names, and in the same order as the
>> +                   reg-names
> 
> Hmm, not sure what prompted you to deviate from the design we had on TI
> SDK kernels. Your revised bindings does not allow us to use this device
> for the scalability with either UIO/VFIO.

My understanding was that the device tree node will have to be modified
in any case to support the current uio_pruss.c driver so we don't
need to add things that we don't need right away.

> 
> Let's sort this out before you post the next series.

Let's discuss this here so I don't have to explain the whole thing again
in the next series.

Tony,

Suman is mainly concerned about the following changes in v2
1) pruss node does not contain reg property representing entire ICSS.
2) pruss node does not contain interrupts.

Both of these are required if drivers/uio/uio_pruss.c or in future if
VFIO is to be used.

The beagleboard community is a primary user of this driver and we need to
find a solution so that PRUSS is usable either via remoteproc or via UIO.

Ideal case should allow user to use either of the drivers by just doing
a unbind and bind.

I don't have a better idea than having a encapsulating node that has
the appropriate reg and interrupt properties.

cheers,
-roger

> 
> regards
> Suman
> 
>> +- reg-names      : should contain a string(s) from among the following names,
>> +                   each representing a specific Data RAM region. Some PRU-ICSS
>> +		   instances on certain SoCs might not have Shared DRAM.
>> +                       "dram0" for Data RAM0,
>> +                       "dram1" for Data RAM1,
>> +                       "shrdram2" for Shared Data RAM,
>> +- #address-cells : should be 1
>> +- #size-cells    : should be 1
>> +- ranges         : no specific range translations required, child nodes have the
>> +                   same address view as the parent, so should be mentioned without
>> +                   any value for the property
>> +
>> +Optional Properties:
>> +--------------------
>> +- no-shared-ram	: Should be present if the instance doesn't have Shared RAM.
>> +		  e.g. AM4376 ICSS0 instance doesn't have Shared RAM.
>> +
>> +The PRUSS node will have one or more of the folowing child nodes.
>> +
>> +PRU CORES
>> +=========
>> +ICSS typically has 2 PRU cores. These should be represented as remoteproc devices.
>> +
>> +INTC node
>> +=========
>> +ICSS has one INTC interrupt controller module. This should be represented as
>> +a standard interrupt-controller node.
>> +
>> +CFG, IEP, MII_RT
>> +================
>> +The individual sub-modules CFG, IEP and MII_RT are represented as a syscon
>> +node each with specific node names as below:
>> +                  "cfg" for CFG sub-module,
>> +                  "iep" for IEP sub-module,
>> +                  "mii_rt" for MII-RT sub-module,
>> +
>> +See Documentation/devicetree/bindings/mfd/syscon.txt for details.
>> +
>> +MDIO
>> +====
>> +Each PRUSS has an MDIO module that can be used to control external PHYs. The
>> +MDIO module used within the PRU-ICSS is an instance of the MDIO Controller
>> +used in TI Davinci SoCs. Please refer to the corresponding binding document,
>> +Documentation/devicetree/bindings/net/davinci-mdio.txt for details.
>> +
>> +Application/User Nodes
>> +=======================
>> +A PRU application/user node typically uses one or more PRU device nodes to
>> +implement a PRU application/functionality. Each application/client node would
>> +need a reference to at least a PRU node, and optionally pass some configuration
>> +parameters.
>> +
>> +Required Properties:
>> +--------------------
>> +- prus                 : phandles to the PRU nodes used
>> +
>> +Optional Properties:
>> +--------------------
>> +- firmware-name        : firmwares for the PRU cores, the default firmware
>> +                         for the core from the PRU node will be used if not
>> +                         provided. The firmware names should correspond to
>> +                         the PRU cores listed in the 'prus' property
>> +- ti,pruss-gp-mux-sel  : array of values for the GP_MUX_SEL under PRUSS_GPCFG
>> +                         register for a PRU. This selects the internal muxing
>> +                         scheme for the PRU instance. If not provided, the
>> +                         default out-of-reset value (0) for the PRU core is
>> +                         used. Values should correspond to the PRU cores listed
>> +                         in the 'prus' property
>> +- ti,pru-interrupt-map : PRU interrupt mappings, containing an array of entries
>> +                         with each entry consisting of 4 cell-values. First one
>> +                         is an index towards the "prus" property to identify the
>> +                         PRU core for the interrupt map, second is the PRU
>> +                         System Event id, third is the PRU interrupt channel id
>> +                         and fourth is the PRU host interrupt id. If provided,
>> +                         this map will supercede any other configuration
>> +                         provided through firmware
>> +
>> +Example:
>> +========
>> +1.	/* AM33xx PRU-ICSS */
>> +
>> +	pruss: pruss@0 {
>> +		compatible = "ti,am3356-pruss";
>> +		reg = <0x0 0x2000>,
>> +		      <0x2000 0x2000>,
>> +		      <0x10000 0x3000>;
>> +		reg-names = "dram0", "dram1",
>> +			    "shrdram2";
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		ranges;
>> +
>> +		pruss_cfg: cfg@26000 {
>> +			compatible = "syscon";
>> +			reg = <0x26000 0x2000>;
>> +		};
>> +
>> +		pruss_iep: iep@2e000 {
>> +			compatible = "syscon";
>> +			reg = <0x2e000 0x31c>;
>> +		};
>> +
>> +		pruss_mii_rt: mii_rt@32000 {
>> +			compatible = "syscon";
>> +			reg = <0x32000 0x58>;
>> +		};
>> +
>> +		pruss_intc: intc@20000 {
>> +			compatible = "ti,am3356-pruss-intc";
>> +			reg = <0x20000 0x2000>;
>> +			reg-names = "intc";
>> +			interrupt-controller;
>> +			#interrupt-cells = <1>;
>> +			interrupts = <20 21 22 23 24 25 26 27>;
>> +			interrupt-names = "host2", "host3", "host4",
>> +					  "host5", "host6", "host7",
>> +					  "host8", "host9";
>> +		};
>> +
>> +		pru0: pru@34000 {
>> +			compatible = "ti,am3356-pru";
>> +			reg = <0x34000 0x2000>,
>> +			      <0x22000 0x400>,
>> +			      <0x22400 0x100>;
>> +			reg-names = "iram", "control", "debug";
>> +			gpcfg = <&pruss_cfg 0x8>;
>> +			firmware-name = "am335x-pru0-fw";
>> +			interrupt-parent = <&pruss_intc>;
>> +			interrupts = <16>, <17>;
>> +			interrupt-names = "vring", "kick";
>> +		};
>> +
>> +		pru1: pru@38000 {
>> +			compatible = "ti,am3356-pru";
>> +			reg = <0x38000 0x2000>,
>> +			      <0x24000 0x400>,
>> +			      <0x24400 0x100>;
>> +			reg-names = "iram", "control", "debug";
>> +			gpcfg = <&pruss_cfg 0xc>;
>> +			firmware-name = "am335x-pru1-fw";
>> +			interrupt-parent = <&pruss_intc>;
>> +			interrupts = <18>, <19>;
>> +			interrupt-names = "vring", "kick";
>> +		};
>> +
>> +		pruss_mdio: mdio@32400 {
>> +			compatible = "ti,davinci_mdio";
>> +			reg = <0x32400 0x90>;
>> +			clocks = <&dpll_core_m4_ck>;
>> +			clock-names = "fck";
>> +			bus_freq = <1000000>;
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +			status = "disabled";
>> +		};
>> +	};
>> +
>> +2:	/* PRU application node example */
>> +	app_node: app_node {
>> +		prus = <&pru0>, <&pru1>;
>> +		firmware-name = "pruss-app-fw", "pruss-app-fw-2";
>> +		ti,pruss-gp-mux-sel = <2>, <1>;
>> +		/* setup interrupts for prus:
>> +		   prus[0] => pru1_0: ev=16, chnl=2, host-irq=7,
>> +		   prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */
>> +		ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
>> +	}
>>
>
Roger Quadros Feb. 14, 2019, 3:48 p.m. | #17
fixed DTML id.

On 14/02/19 17:44, Roger Quadros wrote:
> On 14/02/19 14:52, Marc Zyngier wrote:
>> On Thu, 14 Feb 2019 10:55:10 +0000,
>> Roger Quadros <rogerq@ti.com> wrote:
>>>
>>>
>>> On 14/02/19 10:37, Linus Walleij wrote:
>>>> On Thu, Feb 14, 2019 at 4:13 AM Suman Anna <s-anna@ti.com> wrote:
>>>>> [Me]
>>>>
>>>>>> To be able to use hierarchical interrupt domain in the kernel, the top
>>>>>> interrupt controller must use the hierarchical (v2) irqdomain, so
>>>>>> if this is anything else than the ARM GIC it will be an interesting
>>>>>> undertaking to handle this.
>>>>>
>>>>> These are interrupt lines coming towards the host processor running
>>>>> Linux and are directly connected to the ARM GIC. This INTC module is
>>>>> actually an PRUSS internal interrupt controller that can take in 64 (on
>>>>> most SoCs) external events/interrupt sources and multiplexing them
>>>>> through two layers of many-to-one events-to-intr channels &
>>>>> intr-channels-to-host interrupts. Couple of the host interrupts go to
>>>>> the PRU cores themselves while the remaining ones come out of the IP to
>>>>> connect to other GICs in the SoC.
>>>>
>>>> If the muxing is static (like set up once at probe) so that while
>>>> the system is running, there is one and one only event mapped to
>>>> the GIC from the component below it, then it is hierarchical.
>>>
>>> This is how it looks.
>>>
>>> [GIC]<---8---[INTC]<---64---[events from peripherals]
>>>
>>> The 8 interrupt lines from INTC to the GIC are 1:1 mapped and fixed
>>> per SoC.  The muxing between 64 inputs to INTC and its 8 outputs are
>>> programmable and might not necessarily be static per boot/probe as
>>> it depends on what firmware is loaded on the PRU.
>>
>> But the point is that at any given time, there are at most 8 out of 64
>> inputs that are used, right? You *never* end-up with two (or more) of
>> these "events" being multiplexed on a single output line.
>>
> 
> Since the INTC's internal logic allows assigning more than one event each outputs,
> at most all 64 events can be assigned to one output or distributed among the 8 outputs.
> 
>> If these assertions do hold, then your design is typical of a
>> hierarchy, for which we have countless examples in the tree (including
>> for some TI HW).
> 
> OK.
> Suman, Andrew, Lokesh, thoughts?
>
Tony Lindgren Feb. 14, 2019, 3:56 p.m. | #18
* Roger Quadros <rogerq@ti.com> [190214 11:09]:
> Suman is mainly concerned about the following changes in v2
> 1) pruss node does not contain reg property representing entire ICSS.
> 2) pruss node does not contain interrupts.
> 
> Both of these are required if drivers/uio/uio_pruss.c or in future if
> VFIO is to be used.
> 
> The beagleboard community is a primary user of this driver and we need to
> find a solution so that PRUSS is usable either via remoteproc or via UIO.
> 
> Ideal case should allow user to use either of the drivers by just doing
> a unbind and bind.
> 
> I don't have a better idea than having a encapsulating node that has
> the appropriate reg and interrupt properties.

If there are existing use cases that need to be supported
you should list them as non-standard usage in the binding
and not recommended for future use. Rob may have some
comments on how to deal with this.

Then you can have device driver that needs to pass them
parse them from the PRUSS parent node. That does not mean
there needs to be a top level device driver for PRUSS,
the child control module can just parse the non-standard
bindings for compability from the parent node.

And naturally in addition to handling the non-standard
binding we need to have a proper standardized binding
too :)

Regards,

Tony
Roger Quadros Feb. 14, 2019, 4:50 p.m. | #19
On 14/02/19 17:51, Marc Zyngier wrote:
> On 14/02/2019 15:44, Roger Quadros wrote:
>> On 14/02/19 14:52, Marc Zyngier wrote:
>>> On Thu, 14 Feb 2019 10:55:10 +0000,
>>> Roger Quadros <rogerq@ti.com> wrote:
>>>>
>>>>
>>>> On 14/02/19 10:37, Linus Walleij wrote:
>>>>> On Thu, Feb 14, 2019 at 4:13 AM Suman Anna <s-anna@ti.com> wrote:
>>>>>> [Me]
>>>>>
>>>>>>> To be able to use hierarchical interrupt domain in the kernel, the top
>>>>>>> interrupt controller must use the hierarchical (v2) irqdomain, so
>>>>>>> if this is anything else than the ARM GIC it will be an interesting
>>>>>>> undertaking to handle this.
>>>>>>
>>>>>> These are interrupt lines coming towards the host processor running
>>>>>> Linux and are directly connected to the ARM GIC. This INTC module is
>>>>>> actually an PRUSS internal interrupt controller that can take in 64 (on
>>>>>> most SoCs) external events/interrupt sources and multiplexing them
>>>>>> through two layers of many-to-one events-to-intr channels &
>>>>>> intr-channels-to-host interrupts. Couple of the host interrupts go to
>>>>>> the PRU cores themselves while the remaining ones come out of the IP to
>>>>>> connect to other GICs in the SoC.
>>>>>
>>>>> If the muxing is static (like set up once at probe) so that while
>>>>> the system is running, there is one and one only event mapped to
>>>>> the GIC from the component below it, then it is hierarchical.
>>>>
>>>> This is how it looks.
>>>>
>>>> [GIC]<---8---[INTC]<---64---[events from peripherals]
>>>>
>>>> The 8 interrupt lines from INTC to the GIC are 1:1 mapped and fixed
>>>> per SoC.  The muxing between 64 inputs to INTC and its 8 outputs are
>>>> programmable and might not necessarily be static per boot/probe as
>>>> it depends on what firmware is loaded on the PRU.
>>>
>>> But the point is that at any given time, there are at most 8 out of 64
>>> inputs that are used, right? You *never* end-up with two (or more) of
>>> these "events" being multiplexed on a single output line.
>>>
>>
>> Since the INTC's internal logic allows assigning more than one event each outputs,
>> at most all 64 events can be assigned to one output or distributed among the 8 outputs.
> 
> OK. Do you get individual masking and status bits for each input?

Yes, we have individual enable/disable and status bits for each of the 64 events.

In addition to that it is possible to determine priority if multiple events come to the same
output by reading a register specific to that output.

--
cheers,
-roger
 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Suman Anna Feb. 15, 2019, 12:59 a.m. | #20
On 2/14/19 9:48 AM, Roger Quadros wrote:
> fixed DTML id.
> 
> On 14/02/19 17:44, Roger Quadros wrote:
>> On 14/02/19 14:52, Marc Zyngier wrote:
>>> On Thu, 14 Feb 2019 10:55:10 +0000,
>>> Roger Quadros <rogerq@ti.com> wrote:
>>>>
>>>>
>>>> On 14/02/19 10:37, Linus Walleij wrote:
>>>>> On Thu, Feb 14, 2019 at 4:13 AM Suman Anna <s-anna@ti.com> wrote:
>>>>>> [Me]
>>>>>
>>>>>>> To be able to use hierarchical interrupt domain in the kernel, the top
>>>>>>> interrupt controller must use the hierarchical (v2) irqdomain, so
>>>>>>> if this is anything else than the ARM GIC it will be an interesting
>>>>>>> undertaking to handle this.
>>>>>>
>>>>>> These are interrupt lines coming towards the host processor running
>>>>>> Linux and are directly connected to the ARM GIC. This INTC module is
>>>>>> actually an PRUSS internal interrupt controller that can take in 64 (on
>>>>>> most SoCs) external events/interrupt sources and multiplexing them
>>>>>> through two layers of many-to-one events-to-intr channels &
>>>>>> intr-channels-to-host interrupts. Couple of the host interrupts go to
>>>>>> the PRU cores themselves while the remaining ones come out of the IP to
>>>>>> connect to other GICs in the SoC.
>>>>>
>>>>> If the muxing is static (like set up once at probe) so that while
>>>>> the system is running, there is one and one only event mapped to
>>>>> the GIC from the component below it, then it is hierarchical.
>>>>
>>>> This is how it looks.
>>>>
>>>> [GIC]<---8---[INTC]<---64---[events from peripherals]
>>>>
>>>> The 8 interrupt lines from INTC to the GIC are 1:1 mapped and fixed
>>>> per SoC.  The muxing between 64 inputs to INTC and its 8 outputs are
>>>> programmable and might not necessarily be static per boot/probe as
>>>> it depends on what firmware is loaded on the PRU.
>>>
>>> But the point is that at any given time, there are at most 8 out of 64
>>> inputs that are used, right? You *never* end-up with two (or more) of
>>> these "events" being multiplexed on a single output line.
>>>
>>
>> Since the INTC's internal logic allows assigning more than one event each outputs,
>> at most all 64 events can be assigned to one output or distributed among the 8 outputs.
>>
>>> If these assertions do hold, then your design is typical of a
>>> hierarchy, for which we have countless examples in the tree (including
>>> for some TI HW).
>>
>> OK.
>> Suman, Andrew, Lokesh, thoughts?
>>

Mark, Linus,

So, I hope it is clear from Roger's responses that above assertions do
not hold true to this INTC, and so want to confirm that we are good with
the current non-hierarchical design.

regards
Suman
Suman Anna Feb. 15, 2019, 1:08 a.m. | #21
Hi Roger,

On 2/14/19 5:08 AM, Roger Quadros wrote:
> +Robert, Dan, Matthijs
> 
> Hi Suman,
> 
> On 14/02/19 04:52, Suman Anna wrote:
>> Hi Roger,
>>
>> On 2/4/19 8:22 AM, Roger Quadros wrote:
>>> From: Suman Anna <s-anna@ti.com>
>>>
>>> This patch adds the bindings for the Programmable Real-Time Unit
>>> and Industrial Communication Subsystem (PRU-ICSS) present on various
>>> SoCs such as AM33xx, AM437x, AM57xx, Keystone 66AK2G SoC, etc. It is
>>> present on the Davinci based OMAPL138 SoCs and K3 architecture
>>> based AM65x SoCs as well (not covered for now).
>>>
>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>>  .../devicetree/bindings/soc/ti/ti,pruss.txt        | 212 +++++++++++++++++++++
>>>  1 file changed, 212 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
>>> new file mode 100644
>>> index 0000000..5ac76fd
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
>>> @@ -0,0 +1,212 @@
>>> +PRU-ICSS on TI SoCs
>>> +===================
>>> +
>>> +The Programmable Real-Time Unit and Industrial Communication Subsystem
>>> +(PRU-ICSS) is present on various TI SoCs such as AM335x, AM437x, Keystone
>>> +66AK2G, etc. A PRUSS consists of dual 32-bit RISC cores (Programmable
>>> +Real-Time Units, or PRUs) with program memory and data memory.
>>> +
>>> +The programmable nature of the PRUs provide flexibility to implement
>>> +custom peripheral interfaces, fast real-time responses, or specialized
>>> +data handling. The common peripheral modules include the following,
>>> +
>>> +  - Enhanced GPIO with async capture and serial support
>>> +  - an Ethernet MII_RT module with two MII ports
>>> +  - an MDIO port to control external Ethernet PHYs
>>> +  - an Industrial Ethernet Peripheral (IEP) to manage/generate Industrial
>>> +    Ethernet functions
>>> +  - an Enhanced Capture Module (eCAP)
>>> +  - a 16550-compatible UART to support PROFIBUS
>>> +  - Interrupt controller with 64 input events and 10 Host interrupts.
>>> +
>>> +A shared Data RAM, if present, can be accessed by both the PRU cores. The
>>> +Interrupt Controller (INTC) and a CFG module are common to both the PRU
>>> +cores.
>>> +
>>> +Various sub-modules within a PRU-ICSS subsystem are represented as individual
>>> +nodes.
>>> +
>>> +PRUSS Node
>>> +=============
>>> +
>>> +This node represents the entire ICSS instance and the various modules are
>>> +contained as children. The PRUSS driver is responsible for managing the
>>> +common resources i.e. DRAM0, DRAM1, SHARED_RAM and CFG space.
>>> +
>>> +Required Properties:
>>> +--------------------
>>> +- compatible     : should be one of,
>>> +                       "ti,am3356-pruss" for AM335x family of SoCs
>>> +                       "ti,am4376-pruss" for AM437x family of SoCs
>>> +                       "ti,am5728-pruss" for AM57xx family of SoCs
>>> +                       "ti,k2g-pruss" for 66AK2G family of SoCs
>>> +- reg            : base address and size for each of the Data RAMs as
>>> +                   mentioned in reg-names, and in the same order as the
>>> +                   reg-names
>>
>> Hmm, not sure what prompted you to deviate from the design we had on TI
>> SDK kernels. Your revised bindings does not allow us to use this device
>> for the scalability with either UIO/VFIO.
> 
> My understanding was that the device tree node will have to be modified
> in any case to support the current uio_pruss.c driver so we don't
> need to add things that we don't need right away.

Well, the previous design allows it to scale (as in add new properties
if needed to satisfy the UIO case) maintaining compatibility, but your
current binding does not even allow that. The modification you are
saying that can be done in the future cannot be done without breaking
the binding ABI with your current design.

> 
>>
>> Let's sort this out before you post the next series.
> 
> Let's discuss this here so I don't have to explain the whole thing again
> in the next series.

Yeah, thanks for the below summary.

regards
Suman

> 
> Tony,
> 
> Suman is mainly concerned about the following changes in v2
> 1) pruss node does not contain reg property representing entire ICSS.
> 2) pruss node does not contain interrupts.
> 
> Both of these are required if drivers/uio/uio_pruss.c or in future if
> VFIO is to be used.
> 
> The beagleboard community is a primary user of this driver and we need to
> find a solution so that PRUSS is usable either via remoteproc or via UIO.
> 
> Ideal case should allow user to use either of the drivers by just doing
> a unbind and bind.
> 
> I don't have a better idea than having a encapsulating node that has
> the appropriate reg and interrupt properties.
> 
> cheers,
> -roger
> 
>>
>> regards
>> Suman
>>
>>> +- reg-names      : should contain a string(s) from among the following names,
>>> +                   each representing a specific Data RAM region. Some PRU-ICSS
>>> +		   instances on certain SoCs might not have Shared DRAM.
>>> +                       "dram0" for Data RAM0,
>>> +                       "dram1" for Data RAM1,
>>> +                       "shrdram2" for Shared Data RAM,
>>> +- #address-cells : should be 1
>>> +- #size-cells    : should be 1
>>> +- ranges         : no specific range translations required, child nodes have the
>>> +                   same address view as the parent, so should be mentioned without
>>> +                   any value for the property
>>> +
>>> +Optional Properties:
>>> +--------------------
>>> +- no-shared-ram	: Should be present if the instance doesn't have Shared RAM.
>>> +		  e.g. AM4376 ICSS0 instance doesn't have Shared RAM.
>>> +
>>> +The PRUSS node will have one or more of the folowing child nodes.
>>> +
>>> +PRU CORES
>>> +=========
>>> +ICSS typically has 2 PRU cores. These should be represented as remoteproc devices.
>>> +
>>> +INTC node
>>> +=========
>>> +ICSS has one INTC interrupt controller module. This should be represented as
>>> +a standard interrupt-controller node.
>>> +
>>> +CFG, IEP, MII_RT
>>> +================
>>> +The individual sub-modules CFG, IEP and MII_RT are represented as a syscon
>>> +node each with specific node names as below:
>>> +                  "cfg" for CFG sub-module,
>>> +                  "iep" for IEP sub-module,
>>> +                  "mii_rt" for MII-RT sub-module,
>>> +
>>> +See Documentation/devicetree/bindings/mfd/syscon.txt for details.
>>> +
>>> +MDIO
>>> +====
>>> +Each PRUSS has an MDIO module that can be used to control external PHYs. The
>>> +MDIO module used within the PRU-ICSS is an instance of the MDIO Controller
>>> +used in TI Davinci SoCs. Please refer to the corresponding binding document,
>>> +Documentation/devicetree/bindings/net/davinci-mdio.txt for details.
>>> +
>>> +Application/User Nodes
>>> +=======================
>>> +A PRU application/user node typically uses one or more PRU device nodes to
>>> +implement a PRU application/functionality. Each application/client node would
>>> +need a reference to at least a PRU node, and optionally pass some configuration
>>> +parameters.
>>> +
>>> +Required Properties:
>>> +--------------------
>>> +- prus                 : phandles to the PRU nodes used
>>> +
>>> +Optional Properties:
>>> +--------------------
>>> +- firmware-name        : firmwares for the PRU cores, the default firmware
>>> +                         for the core from the PRU node will be used if not
>>> +                         provided. The firmware names should correspond to
>>> +                         the PRU cores listed in the 'prus' property
>>> +- ti,pruss-gp-mux-sel  : array of values for the GP_MUX_SEL under PRUSS_GPCFG
>>> +                         register for a PRU. This selects the internal muxing
>>> +                         scheme for the PRU instance. If not provided, the
>>> +                         default out-of-reset value (0) for the PRU core is
>>> +                         used. Values should correspond to the PRU cores listed
>>> +                         in the 'prus' property
>>> +- ti,pru-interrupt-map : PRU interrupt mappings, containing an array of entries
>>> +                         with each entry consisting of 4 cell-values. First one
>>> +                         is an index towards the "prus" property to identify the
>>> +                         PRU core for the interrupt map, second is the PRU
>>> +                         System Event id, third is the PRU interrupt channel id
>>> +                         and fourth is the PRU host interrupt id. If provided,
>>> +                         this map will supercede any other configuration
>>> +                         provided through firmware
>>> +
>>> +Example:
>>> +========
>>> +1.	/* AM33xx PRU-ICSS */
>>> +
>>> +	pruss: pruss@0 {
>>> +		compatible = "ti,am3356-pruss";
>>> +		reg = <0x0 0x2000>,
>>> +		      <0x2000 0x2000>,
>>> +		      <0x10000 0x3000>;
>>> +		reg-names = "dram0", "dram1",
>>> +			    "shrdram2";
>>> +		#address-cells = <1>;
>>> +		#size-cells = <1>;
>>> +		ranges;
>>> +
>>> +		pruss_cfg: cfg@26000 {
>>> +			compatible = "syscon";
>>> +			reg = <0x26000 0x2000>;
>>> +		};
>>> +
>>> +		pruss_iep: iep@2e000 {
>>> +			compatible = "syscon";
>>> +			reg = <0x2e000 0x31c>;
>>> +		};
>>> +
>>> +		pruss_mii_rt: mii_rt@32000 {
>>> +			compatible = "syscon";
>>> +			reg = <0x32000 0x58>;
>>> +		};
>>> +
>>> +		pruss_intc: intc@20000 {
>>> +			compatible = "ti,am3356-pruss-intc";
>>> +			reg = <0x20000 0x2000>;
>>> +			reg-names = "intc";
>>> +			interrupt-controller;
>>> +			#interrupt-cells = <1>;
>>> +			interrupts = <20 21 22 23 24 25 26 27>;
>>> +			interrupt-names = "host2", "host3", "host4",
>>> +					  "host5", "host6", "host7",
>>> +					  "host8", "host9";
>>> +		};
>>> +
>>> +		pru0: pru@34000 {
>>> +			compatible = "ti,am3356-pru";
>>> +			reg = <0x34000 0x2000>,
>>> +			      <0x22000 0x400>,
>>> +			      <0x22400 0x100>;
>>> +			reg-names = "iram", "control", "debug";
>>> +			gpcfg = <&pruss_cfg 0x8>;
>>> +			firmware-name = "am335x-pru0-fw";
>>> +			interrupt-parent = <&pruss_intc>;
>>> +			interrupts = <16>, <17>;
>>> +			interrupt-names = "vring", "kick";
>>> +		};
>>> +
>>> +		pru1: pru@38000 {
>>> +			compatible = "ti,am3356-pru";
>>> +			reg = <0x38000 0x2000>,
>>> +			      <0x24000 0x400>,
>>> +			      <0x24400 0x100>;
>>> +			reg-names = "iram", "control", "debug";
>>> +			gpcfg = <&pruss_cfg 0xc>;
>>> +			firmware-name = "am335x-pru1-fw";
>>> +			interrupt-parent = <&pruss_intc>;
>>> +			interrupts = <18>, <19>;
>>> +			interrupt-names = "vring", "kick";
>>> +		};
>>> +
>>> +		pruss_mdio: mdio@32400 {
>>> +			compatible = "ti,davinci_mdio";
>>> +			reg = <0x32400 0x90>;
>>> +			clocks = <&dpll_core_m4_ck>;
>>> +			clock-names = "fck";
>>> +			bus_freq = <1000000>;
>>> +			#address-cells = <1>;
>>> +			#size-cells = <0>;
>>> +			status = "disabled";
>>> +		};
>>> +	};
>>> +
>>> +2:	/* PRU application node example */
>>> +	app_node: app_node {
>>> +		prus = <&pru0>, <&pru1>;
>>> +		firmware-name = "pruss-app-fw", "pruss-app-fw-2";
>>> +		ti,pruss-gp-mux-sel = <2>, <1>;
>>> +		/* setup interrupts for prus:
>>> +		   prus[0] => pru1_0: ev=16, chnl=2, host-irq=7,
>>> +		   prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */
>>> +		ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
>>> +	}
>>>
>>
>
Suman Anna Feb. 15, 2019, 1:22 a.m. | #22
Hi Tony,

On 2/14/19 9:56 AM, Tony Lindgren wrote:
> * Roger Quadros <rogerq@ti.com> [190214 11:09]:
>> Suman is mainly concerned about the following changes in v2
>> 1) pruss node does not contain reg property representing entire ICSS.
>> 2) pruss node does not contain interrupts.
>>
>> Both of these are required if drivers/uio/uio_pruss.c or in future if
>> VFIO is to be used.
>>
>> The beagleboard community is a primary user of this driver and we need to
>> find a solution so that PRUSS is usable either via remoteproc or via UIO.
>>
>> Ideal case should allow user to use either of the drivers by just doing
>> a unbind and bind.
>>
>> I don't have a better idea than having a encapsulating node that has
>> the appropriate reg and interrupt properties.
> 
> If there are existing use cases that need to be supported
> you should list them as non-standard usage in the binding
> and not recommended for future use. Rob may have some
> comments on how to deal with this.
> 
> Then you can have device driver that needs to pass them
> parse them from the PRUSS parent node. That does not mean
> there needs to be a top level device driver for PRUSS,
> the child control module can just parse the non-standard
> bindings for compability from the parent node.

The PRUSS SoC bus driver was handling all possible architectures (OMAP,
K2 and K3) which have different clocking and reset integration, and also
catering to the UIO vs remoteproc usecases, by taking care of clocks and
resets. I am ok to replace this layer with the ti-sysc layer on OMAP
SoCs since most of the functionality added to the driver is associated
with OCP, but we would still need a PRUSS driver.

Not all sub-modules are peripherals and managed by respective peripheral
drivers, and we still need a central entity managing the sub-system wide
resources. Layering wise - it is similar if we would have done a device
for the PRUSS local interconnect, but that driver wouldn't have much to
do with interconnect functionality. K2 and K3 families uses TI-SCI and
so you don't have a similar target-module concept that allows you to
query the PRUSS parent node for PRUSS specific ranges or properties.
In anycase, I don't think these drivers should depend on a parent
interconnect driver.

regards
Suman


> 
> And naturally in addition to handling the non-standard
> binding we need to have a proper standardized binding
> too :)
> 
> Regards,
> 
> Tony
>
Matthijs van Duin Feb. 15, 2019, 1:43 p.m. | #23
On Thu, 14 Feb 2019 at 12:08, Roger Quadros <rogerq@ti.com> wrote:
> The beagleboard community is a primary user of this driver and we need to
> find a solution so that PRUSS is usable either via remoteproc or via UIO.

While being able to switch drivers without changing the DT by forcibly
binding a different driver would definitely be a nice feature, and
would have been possible with the older remoteproc-pru bindings, I
think it's a stretch to say this "needs" a solution. Right now, in
practice, selection between uio-pruss and remoteproc-pru is done
simply by modifying the device tree appropriately (typically by having
u-boot apply an overlay to the DT), and I don't think anyone views
this as unduly burdensome?

Matthijs
Linus Walleij Feb. 20, 2019, 9:51 a.m. | #24
On Fri, Feb 15, 2019 at 2:00 AM Suman Anna <s-anna@ti.com> wrote:

> Mark, Linus,
>
> So, I hope it is clear from Roger's responses that above assertions do
> not hold true to this INTC, and so want to confirm that we are good with
> the current non-hierarchical design.

IIUC the 64 lines are latched onto 8 lines, but all 64 lines have
individual masking and ACKing bits and can all be used at the same
time, so yes that is cascading and then you should indeed use
the chained (or nested) IRQ handler.

Yours,
Linus Walleij

Patch

diff --git a/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
new file mode 100644
index 0000000..5ac76fd
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/ti/ti,pruss.txt
@@ -0,0 +1,212 @@ 
+PRU-ICSS on TI SoCs
+===================
+
+The Programmable Real-Time Unit and Industrial Communication Subsystem
+(PRU-ICSS) is present on various TI SoCs such as AM335x, AM437x, Keystone
+66AK2G, etc. A PRUSS consists of dual 32-bit RISC cores (Programmable
+Real-Time Units, or PRUs) with program memory and data memory.
+
+The programmable nature of the PRUs provide flexibility to implement
+custom peripheral interfaces, fast real-time responses, or specialized
+data handling. The common peripheral modules include the following,
+
+  - Enhanced GPIO with async capture and serial support
+  - an Ethernet MII_RT module with two MII ports
+  - an MDIO port to control external Ethernet PHYs
+  - an Industrial Ethernet Peripheral (IEP) to manage/generate Industrial
+    Ethernet functions
+  - an Enhanced Capture Module (eCAP)
+  - a 16550-compatible UART to support PROFIBUS
+  - Interrupt controller with 64 input events and 10 Host interrupts.
+
+A shared Data RAM, if present, can be accessed by both the PRU cores. The
+Interrupt Controller (INTC) and a CFG module are common to both the PRU
+cores.
+
+Various sub-modules within a PRU-ICSS subsystem are represented as individual
+nodes.
+
+PRUSS Node
+=============
+
+This node represents the entire ICSS instance and the various modules are
+contained as children. The PRUSS driver is responsible for managing the
+common resources i.e. DRAM0, DRAM1, SHARED_RAM and CFG space.
+
+Required Properties:
+--------------------
+- compatible     : should be one of,
+                       "ti,am3356-pruss" for AM335x family of SoCs
+                       "ti,am4376-pruss" for AM437x family of SoCs
+                       "ti,am5728-pruss" for AM57xx family of SoCs
+                       "ti,k2g-pruss" for 66AK2G family of SoCs
+- reg            : base address and size for each of the Data RAMs as
+                   mentioned in reg-names, and in the same order as the
+                   reg-names
+- reg-names      : should contain a string(s) from among the following names,
+                   each representing a specific Data RAM region. Some PRU-ICSS
+		   instances on certain SoCs might not have Shared DRAM.
+                       "dram0" for Data RAM0,
+                       "dram1" for Data RAM1,
+                       "shrdram2" for Shared Data RAM,
+- #address-cells : should be 1
+- #size-cells    : should be 1
+- ranges         : no specific range translations required, child nodes have the
+                   same address view as the parent, so should be mentioned without
+                   any value for the property
+
+Optional Properties:
+--------------------
+- no-shared-ram	: Should be present if the instance doesn't have Shared RAM.
+		  e.g. AM4376 ICSS0 instance doesn't have Shared RAM.
+
+The PRUSS node will have one or more of the folowing child nodes.
+
+PRU CORES
+=========
+ICSS typically has 2 PRU cores. These should be represented as remoteproc devices.
+
+INTC node
+=========
+ICSS has one INTC interrupt controller module. This should be represented as
+a standard interrupt-controller node.
+
+CFG, IEP, MII_RT
+================
+The individual sub-modules CFG, IEP and MII_RT are represented as a syscon
+node each with specific node names as below:
+                  "cfg" for CFG sub-module,
+                  "iep" for IEP sub-module,
+                  "mii_rt" for MII-RT sub-module,
+
+See Documentation/devicetree/bindings/mfd/syscon.txt for details.
+
+MDIO
+====
+Each PRUSS has an MDIO module that can be used to control external PHYs. The
+MDIO module used within the PRU-ICSS is an instance of the MDIO Controller
+used in TI Davinci SoCs. Please refer to the corresponding binding document,
+Documentation/devicetree/bindings/net/davinci-mdio.txt for details.
+
+Application/User Nodes
+=======================
+A PRU application/user node typically uses one or more PRU device nodes to
+implement a PRU application/functionality. Each application/client node would
+need a reference to at least a PRU node, and optionally pass some configuration
+parameters.
+
+Required Properties:
+--------------------
+- prus                 : phandles to the PRU nodes used
+
+Optional Properties:
+--------------------
+- firmware-name        : firmwares for the PRU cores, the default firmware
+                         for the core from the PRU node will be used if not
+                         provided. The firmware names should correspond to
+                         the PRU cores listed in the 'prus' property
+- ti,pruss-gp-mux-sel  : array of values for the GP_MUX_SEL under PRUSS_GPCFG
+                         register for a PRU. This selects the internal muxing
+                         scheme for the PRU instance. If not provided, the
+                         default out-of-reset value (0) for the PRU core is
+                         used. Values should correspond to the PRU cores listed
+                         in the 'prus' property
+- ti,pru-interrupt-map : PRU interrupt mappings, containing an array of entries
+                         with each entry consisting of 4 cell-values. First one
+                         is an index towards the "prus" property to identify the
+                         PRU core for the interrupt map, second is the PRU
+                         System Event id, third is the PRU interrupt channel id
+                         and fourth is the PRU host interrupt id. If provided,
+                         this map will supercede any other configuration
+                         provided through firmware
+
+Example:
+========
+1.	/* AM33xx PRU-ICSS */
+
+	pruss: pruss@0 {
+		compatible = "ti,am3356-pruss";
+		reg = <0x0 0x2000>,
+		      <0x2000 0x2000>,
+		      <0x10000 0x3000>;
+		reg-names = "dram0", "dram1",
+			    "shrdram2";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		pruss_cfg: cfg@26000 {
+			compatible = "syscon";
+			reg = <0x26000 0x2000>;
+		};
+
+		pruss_iep: iep@2e000 {
+			compatible = "syscon";
+			reg = <0x2e000 0x31c>;
+		};
+
+		pruss_mii_rt: mii_rt@32000 {
+			compatible = "syscon";
+			reg = <0x32000 0x58>;
+		};
+
+		pruss_intc: intc@20000 {
+			compatible = "ti,am3356-pruss-intc";
+			reg = <0x20000 0x2000>;
+			reg-names = "intc";
+			interrupt-controller;
+			#interrupt-cells = <1>;
+			interrupts = <20 21 22 23 24 25 26 27>;
+			interrupt-names = "host2", "host3", "host4",
+					  "host5", "host6", "host7",
+					  "host8", "host9";
+		};
+
+		pru0: pru@34000 {
+			compatible = "ti,am3356-pru";
+			reg = <0x34000 0x2000>,
+			      <0x22000 0x400>,
+			      <0x22400 0x100>;
+			reg-names = "iram", "control", "debug";
+			gpcfg = <&pruss_cfg 0x8>;
+			firmware-name = "am335x-pru0-fw";
+			interrupt-parent = <&pruss_intc>;
+			interrupts = <16>, <17>;
+			interrupt-names = "vring", "kick";
+		};
+
+		pru1: pru@38000 {
+			compatible = "ti,am3356-pru";
+			reg = <0x38000 0x2000>,
+			      <0x24000 0x400>,
+			      <0x24400 0x100>;
+			reg-names = "iram", "control", "debug";
+			gpcfg = <&pruss_cfg 0xc>;
+			firmware-name = "am335x-pru1-fw";
+			interrupt-parent = <&pruss_intc>;
+			interrupts = <18>, <19>;
+			interrupt-names = "vring", "kick";
+		};
+
+		pruss_mdio: mdio@32400 {
+			compatible = "ti,davinci_mdio";
+			reg = <0x32400 0x90>;
+			clocks = <&dpll_core_m4_ck>;
+			clock-names = "fck";
+			bus_freq = <1000000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+	};
+
+2:	/* PRU application node example */
+	app_node: app_node {
+		prus = <&pru0>, <&pru1>;
+		firmware-name = "pruss-app-fw", "pruss-app-fw-2";
+		ti,pruss-gp-mux-sel = <2>, <1>;
+		/* setup interrupts for prus:
+		   prus[0] => pru1_0: ev=16, chnl=2, host-irq=7,
+		   prus[1] => pru1_1: ev=19, chnl=1, host-irq=3 */
+		ti,pru-interrupt-map = <0 16 2 7 >, <1 19 1 3>;
+	}