diff mbox

[1/3] Documentation: DT: add Keystone DSP remoteproc binding

Message ID 20170526165317.23164-2-s-anna@ti.com (mailing list archive)
State Superseded
Headers show

Commit Message

Suman Anna May 26, 2017, 4:53 p.m. UTC
Add the device tree bindings document for the Texas Instrument's
Keystone 2 DSP remoteproc devices.

Signed-off-by: Suman Anna <s-anna@ti.com>
Signed-off-by: Sam Nelson <sam.nelson@ti.com>
---
 .../bindings/remoteproc/ti,keystone-rproc.txt      | 132 +++++++++++++++++++++
 1 file changed, 132 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt

Comments

Rob Herring (Arm) May 31, 2017, 7:12 p.m. UTC | #1
On Fri, May 26, 2017 at 11:53:15AM -0500, Suman Anna wrote:
> Add the device tree bindings document for the Texas Instrument's
> Keystone 2 DSP remoteproc devices.
> 
> Signed-off-by: Suman Anna <s-anna@ti.com>
> Signed-off-by: Sam Nelson <sam.nelson@ti.com>
> ---
>  .../bindings/remoteproc/ti,keystone-rproc.txt      | 132 +++++++++++++++++++++
>  1 file changed, 132 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt b/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
> new file mode 100644
> index 000000000000..f1ba88edd00d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
> @@ -0,0 +1,132 @@
> +TI Keystone DSP devices
> +=======================
> +
> +Binding status: Unstable - Subject to changes for using multiple memory regions

I don't really see what would be unstable here. memory-region is easily 
extended to multiple entries.

> +
> +The TI Keystone 2 family of SoCs usually have one or more (upto 8) TI DSP Core
> +sub-systems that are used to offload some of the processor-intensive tasks or
> +algorithms, for achieving various system level goals.
> +
> +These processor sub-systems usually contain additional sub-modules like L1
> +and/or L2 caches/SRAMs, an Interrupt Controller, an external memory controller,
> +a dedicated local power/sleep controller etc. The DSP processor core in
> +Keystone 2 SoCs is usually a TMS320C66x CorePac processor.
> +
> +DSP Device Node:
> +================
> +Each DSP Core sub-system is represented as a single DT node. Each node has a
> +number of required or optional properties that enable the OS running on the
> +host processor (ARM CorePac) to perform the device management of the remote
> +processor and to communicate with the remote processor.
> +
> +Required properties:
> +--------------------
> +The following are the mandatory properties:
> +
> +- compatible:		Should be one of the following,
> +			    "ti,k2hk-dsp" for DSPs on Keystone 2 66AK2H/K SoCs
> +			    "ti,k2l-dsp" for DSPs on Keystone 2 66AK2L SoCs
> +			    "ti,k2e-dsp" for DSPs on Keystone 2 66AK2E SoCs
> +
> +- reg:			Should contain an entry for each value in 'reg-names'.
> +			Each entry should have the memory region's start address
> +			and the size of the region, the representation matching
> +			the parent node's '#address-cells' and '#size-cells' values.
> +
> +- reg-names:		Should contain strings with the following names, each
> +			representing a specific internal memory region, and
> +			should be defined in this order,
> +			     "l2sram", "l1pram", "l1dram"
> +
> +- label:		Should contain a string identifying the DSP instance
> +			within the SoC. Should be the string "dsp" followed by
> +			the instance number. Eg: "dsp0", "dsp1",..."dsp7" etc

Why does a user need to know or care?

> +
> +- clocks: 		Should contain the device's input clock, and should be
> +			defined as per the bindings in,
> +			Documentation/devicetree/bindings/clock/keystone-gate.txt
> +
> +- ti,syscon-dev:	Should be a pair of the phandle to the Keystone Device
> +			State Control node, and the register offset of the DSP
> +			boot address register within that node's address space.
> +
> +- resets:		Should contain the phandle to the reset controller node
> +			managing the resets for this device, and a reset
> +			specifier. Please refer to the following reset bindings
> +			for the reset argument specifier as per SoC,
> +			Documentation/devicetree/bindings/reset/ti-syscon-reset.txt
> +			    for 66AK2HK/66AK2L/66AK2E SoCs
> +
> +- interrupt-parent:	Should contain a phandle to the Keystone 2 IRQ controller
> +			IP node that is used by the ARM CorePac processor to
> +			receive interrupts from the DSP remote processors. See
> +			Documentation/devicetree/bindings/interrupt-controller/ti,keystone-irq.txt
> +			for details.
> +
> +- interrupts: 		Should contain an entry for each value in 'interrupt-names'.
> +			Each entry should have the interrupt source number used by
> +			the remote processor to the host processor. The values should
> +			follow the interrupt-specifier format as dictated by the
> +			'interrupt-parent' node. The purpose of each is as per the
> +			description in the 'interrupt-names' property.
> +
> +- interrupt-names:	Should contain strings with the following names, each
> +			representing a specific interrupt,
> +			    "vring" - interrupt for virtio based IPC
> +			    "exception" - interrupt for exception notification
> +
> +- kick-gpio: 		Should specify the gpio device needed for the virtio IPC

-gpios

> +			stack. This will be used to interrupt the remote processor.
> +			The gpio device to be used is as per the bindings in,
> +			Documentation/devicetree/bindings/gpio/gpio-dsp-keystone.txt
> +
> +Optional properties:
> +--------------------
> +
> +- memory-region:	phandle to the reserved memory node to be associated
> +			with the remoteproc device. The reserved memory node
> +			can be a CMA memory node, and should be defined as
> +			per the bindings in
> +			Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> +
> +
> +Example:
> +--------
> +
> +	/* 66AK2H/K DSP node in SoC DTS file */
> +	soc {
> +		dsp0: dsp@10800000 {
> +			compatible = "ti,k2hk-dsp";
> +			reg = <0x10800000 0x00100000>,
> +			      <0x10e00000 0x00008000>,
> +			      <0x10f00000 0x00008000>;
> +			reg-names = "l2sram", "l1pram", "l1dram";
> +			label = "dsp0";
> +			clocks = <&clkgem0>;
> +			ti,syscon-dev = <&devctrl 0x40>;
> +			resets = <&pscrst 0>;
> +			interrupt-parent = <&kirq0>;
> +			interrupts = <0 8>;
> +			interrupt-names = "vring", "exception";
> +			kick-gpio = <&dspgpio0 27 0>;
> +		};
> +
> +	};
> +
> +	/* K2HK EVM Board file */
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		dsp_common_cma_pool: dsp_common_cma_pool@81f800000 {
> +			compatible = "shared-dma-pool";
> +			reg = <0x00000008 0x1f800000 0x00000000 0x800000>;
> +			reusable;
> +			status = "okay";
> +		};
> +	};
> +
> +	&dsp0 {
> +		memory-region = <&dsp_common_cma_pool>;
> +	};
> -- 
> 2.12.0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Suman Anna May 31, 2017, 8:05 p.m. UTC | #2
Hi Rob,

On 05/31/2017 02:12 PM, Rob Herring wrote:
> On Fri, May 26, 2017 at 11:53:15AM -0500, Suman Anna wrote:
>> Add the device tree bindings document for the Texas Instrument's
>> Keystone 2 DSP remoteproc devices.
>>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> Signed-off-by: Sam Nelson <sam.nelson@ti.com>
>> ---
>>  .../bindings/remoteproc/ti,keystone-rproc.txt      | 132 +++++++++++++++++++++
>>  1 file changed, 132 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt b/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
>> new file mode 100644
>> index 000000000000..f1ba88edd00d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
>> @@ -0,0 +1,132 @@
>> +TI Keystone DSP devices
>> +=======================
>> +
>> +Binding status: Unstable - Subject to changes for using multiple memory regions
> 
> I don't really see what would be unstable here. memory-region is easily 
> extended to multiple entries.

OK will drop this line.

> 
>> +
>> +The TI Keystone 2 family of SoCs usually have one or more (upto 8) TI DSP Core
>> +sub-systems that are used to offload some of the processor-intensive tasks or
>> +algorithms, for achieving various system level goals.
>> +
>> +These processor sub-systems usually contain additional sub-modules like L1
>> +and/or L2 caches/SRAMs, an Interrupt Controller, an external memory controller,
>> +a dedicated local power/sleep controller etc. The DSP processor core in
>> +Keystone 2 SoCs is usually a TMS320C66x CorePac processor.
>> +
>> +DSP Device Node:
>> +================
>> +Each DSP Core sub-system is represented as a single DT node. Each node has a
>> +number of required or optional properties that enable the OS running on the
>> +host processor (ARM CorePac) to perform the device management of the remote
>> +processor and to communicate with the remote processor.
>> +
>> +Required properties:
>> +--------------------
>> +The following are the mandatory properties:
>> +
>> +- compatible:		Should be one of the following,
>> +			    "ti,k2hk-dsp" for DSPs on Keystone 2 66AK2H/K SoCs
>> +			    "ti,k2l-dsp" for DSPs on Keystone 2 66AK2L SoCs
>> +			    "ti,k2e-dsp" for DSPs on Keystone 2 66AK2E SoCs
>> +
>> +- reg:			Should contain an entry for each value in 'reg-names'.
>> +			Each entry should have the memory region's start address
>> +			and the size of the region, the representation matching
>> +			the parent node's '#address-cells' and '#size-cells' values.
>> +
>> +- reg-names:		Should contain strings with the following names, each
>> +			representing a specific internal memory region, and
>> +			should be defined in this order,
>> +			     "l2sram", "l1pram", "l1dram"
>> +
>> +- label:		Should contain a string identifying the DSP instance
>> +			within the SoC. Should be the string "dsp" followed by
>> +			the instance number. Eg: "dsp0", "dsp1",..."dsp7" etc
> 
> Why does a user need to know or care?

This is used to distinguish the exact DSP instance from others since the
DT node name is a generic "dsp". One of the uses is to be able to
construct a firmware name within the driver using this label.

> 
>> +
>> +- clocks: 		Should contain the device's input clock, and should be
>> +			defined as per the bindings in,
>> +			Documentation/devicetree/bindings/clock/keystone-gate.txt
>> +
>> +- ti,syscon-dev:	Should be a pair of the phandle to the Keystone Device
>> +			State Control node, and the register offset of the DSP
>> +			boot address register within that node's address space.
>> +
>> +- resets:		Should contain the phandle to the reset controller node
>> +			managing the resets for this device, and a reset
>> +			specifier. Please refer to the following reset bindings
>> +			for the reset argument specifier as per SoC,
>> +			Documentation/devicetree/bindings/reset/ti-syscon-reset.txt
>> +			    for 66AK2HK/66AK2L/66AK2E SoCs
>> +
>> +- interrupt-parent:	Should contain a phandle to the Keystone 2 IRQ controller
>> +			IP node that is used by the ARM CorePac processor to
>> +			receive interrupts from the DSP remote processors. See
>> +			Documentation/devicetree/bindings/interrupt-controller/ti,keystone-irq.txt
>> +			for details.
>> +
>> +- interrupts: 		Should contain an entry for each value in 'interrupt-names'.
>> +			Each entry should have the interrupt source number used by
>> +			the remote processor to the host processor. The values should
>> +			follow the interrupt-specifier format as dictated by the
>> +			'interrupt-parent' node. The purpose of each is as per the
>> +			description in the 'interrupt-names' property.
>> +
>> +- interrupt-names:	Should contain strings with the following names, each
>> +			representing a specific interrupt,
>> +			    "vring" - interrupt for virtio based IPC
>> +			    "exception" - interrupt for exception notification
>> +
>> +- kick-gpio: 		Should specify the gpio device needed for the virtio IPC
> 
> -gpios

Will fix.

> 
>> +			stack. This will be used to interrupt the remote processor.
>> +			The gpio device to be used is as per the bindings in,
>> +			Documentation/devicetree/bindings/gpio/gpio-dsp-keystone.txt
>> +
>> +Optional properties:
>> +--------------------
>> +
>> +- memory-region:	phandle to the reserved memory node to be associated
>> +			with the remoteproc device. The reserved memory node
>> +			can be a CMA memory node, and should be defined as
>> +			per the bindings in
>> +			Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
>> +
>> +
>> +Example:
>> +--------
>> +
>> +	/* 66AK2H/K DSP node in SoC DTS file */
>> +	soc {
>> +		dsp0: dsp@10800000 {
>> +			compatible = "ti,k2hk-dsp";
>> +			reg = <0x10800000 0x00100000>,
>> +			      <0x10e00000 0x00008000>,
>> +			      <0x10f00000 0x00008000>;
>> +			reg-names = "l2sram", "l1pram", "l1dram";
>> +			label = "dsp0";
>> +			clocks = <&clkgem0>;
>> +			ti,syscon-dev = <&devctrl 0x40>;
>> +			resets = <&pscrst 0>;
>> +			interrupt-parent = <&kirq0>;
>> +			interrupts = <0 8>;
>> +			interrupt-names = "vring", "exception";
>> +			kick-gpio = <&dspgpio0 27 0>;
>> +		};
>> +
>> +	};
>> +
>> +	/* K2HK EVM Board file */
>> +	reserved-memory {
>> +		#address-cells = <2>;
>> +		#size-cells = <2>;
>> +		ranges;
>> +
>> +		dsp_common_cma_pool: dsp_common_cma_pool@81f800000 {
>> +			compatible = "shared-dma-pool";
>> +			reg = <0x00000008 0x1f800000 0x00000000 0x800000>;
>> +			reusable;
>> +			status = "okay";
>> +		};
>> +	};
>> +
>> +	&dsp0 {
>> +		memory-region = <&dsp_common_cma_pool>;
>> +	};

Will also fix this up based on your comments on the Davinci remoteproc
driver binding.

regards
Suman
--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring (Arm) June 5, 2017, 5:26 p.m. UTC | #3
On Wed, May 31, 2017 at 03:05:59PM -0500, Suman Anna wrote:
> Hi Rob,
> 
> On 05/31/2017 02:12 PM, Rob Herring wrote:
> > On Fri, May 26, 2017 at 11:53:15AM -0500, Suman Anna wrote:
> >> Add the device tree bindings document for the Texas Instrument's
> >> Keystone 2 DSP remoteproc devices.
> >>
> >> Signed-off-by: Suman Anna <s-anna@ti.com>
> >> Signed-off-by: Sam Nelson <sam.nelson@ti.com>
> >> ---
> >>  .../bindings/remoteproc/ti,keystone-rproc.txt      | 132 +++++++++++++++++++++
> >>  1 file changed, 132 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt b/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
> >> new file mode 100644
> >> index 000000000000..f1ba88edd00d
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
> >> @@ -0,0 +1,132 @@
> >> +TI Keystone DSP devices
> >> +=======================
> >> +
> >> +Binding status: Unstable - Subject to changes for using multiple memory regions
> > 
> > I don't really see what would be unstable here. memory-region is easily 
> > extended to multiple entries.
> 
> OK will drop this line.
> 
> > 
> >> +
> >> +The TI Keystone 2 family of SoCs usually have one or more (upto 8) TI DSP Core
> >> +sub-systems that are used to offload some of the processor-intensive tasks or
> >> +algorithms, for achieving various system level goals.
> >> +
> >> +These processor sub-systems usually contain additional sub-modules like L1
> >> +and/or L2 caches/SRAMs, an Interrupt Controller, an external memory controller,
> >> +a dedicated local power/sleep controller etc. The DSP processor core in
> >> +Keystone 2 SoCs is usually a TMS320C66x CorePac processor.
> >> +
> >> +DSP Device Node:
> >> +================
> >> +Each DSP Core sub-system is represented as a single DT node. Each node has a
> >> +number of required or optional properties that enable the OS running on the
> >> +host processor (ARM CorePac) to perform the device management of the remote
> >> +processor and to communicate with the remote processor.
> >> +
> >> +Required properties:
> >> +--------------------
> >> +The following are the mandatory properties:
> >> +
> >> +- compatible:		Should be one of the following,
> >> +			    "ti,k2hk-dsp" for DSPs on Keystone 2 66AK2H/K SoCs
> >> +			    "ti,k2l-dsp" for DSPs on Keystone 2 66AK2L SoCs
> >> +			    "ti,k2e-dsp" for DSPs on Keystone 2 66AK2E SoCs
> >> +
> >> +- reg:			Should contain an entry for each value in 'reg-names'.
> >> +			Each entry should have the memory region's start address
> >> +			and the size of the region, the representation matching
> >> +			the parent node's '#address-cells' and '#size-cells' values.
> >> +
> >> +- reg-names:		Should contain strings with the following names, each
> >> +			representing a specific internal memory region, and
> >> +			should be defined in this order,
> >> +			     "l2sram", "l1pram", "l1dram"
> >> +
> >> +- label:		Should contain a string identifying the DSP instance
> >> +			within the SoC. Should be the string "dsp" followed by
> >> +			the instance number. Eg: "dsp0", "dsp1",..."dsp7" etc
> > 
> > Why does a user need to know or care?
> 
> This is used to distinguish the exact DSP instance from others since the
> DT node name is a generic "dsp". One of the uses is to be able to
> construct a firmware name within the driver using this label.

firmware-name doesn't work for you?

It was obvious that you are using this to number instances. Why do you 
care which instance is which? For example, I want dsp0 because it has X 
or runs at XXMHz, etc. If each instance is different (i.e. not a pool of 
DSPs), then you should describe the difference some way.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Suman Anna June 5, 2017, 6:21 p.m. UTC | #4
Hi Rob,

>>
>> On 05/31/2017 02:12 PM, Rob Herring wrote:
>>> On Fri, May 26, 2017 at 11:53:15AM -0500, Suman Anna wrote:
>>>> Add the device tree bindings document for the Texas Instrument's
>>>> Keystone 2 DSP remoteproc devices.
>>>>
>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>>> Signed-off-by: Sam Nelson <sam.nelson@ti.com>
>>>> ---
>>>>  .../bindings/remoteproc/ti,keystone-rproc.txt      | 132 +++++++++++++++++++++
>>>>  1 file changed, 132 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt b/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
>>>> new file mode 100644
>>>> index 000000000000..f1ba88edd00d
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
>>>> @@ -0,0 +1,132 @@
>>>> +TI Keystone DSP devices
>>>> +=======================
>>>> +
>>>> +Binding status: Unstable - Subject to changes for using multiple memory regions
>>>
>>> I don't really see what would be unstable here. memory-region is easily 
>>> extended to multiple entries.
>>
>> OK will drop this line.
>>
>>>
>>>> +
>>>> +The TI Keystone 2 family of SoCs usually have one or more (upto 8) TI DSP Core
>>>> +sub-systems that are used to offload some of the processor-intensive tasks or
>>>> +algorithms, for achieving various system level goals.
>>>> +
>>>> +These processor sub-systems usually contain additional sub-modules like L1
>>>> +and/or L2 caches/SRAMs, an Interrupt Controller, an external memory controller,
>>>> +a dedicated local power/sleep controller etc. The DSP processor core in
>>>> +Keystone 2 SoCs is usually a TMS320C66x CorePac processor.
>>>> +
>>>> +DSP Device Node:
>>>> +================
>>>> +Each DSP Core sub-system is represented as a single DT node. Each node has a
>>>> +number of required or optional properties that enable the OS running on the
>>>> +host processor (ARM CorePac) to perform the device management of the remote
>>>> +processor and to communicate with the remote processor.
>>>> +
>>>> +Required properties:
>>>> +--------------------
>>>> +The following are the mandatory properties:
>>>> +
>>>> +- compatible:		Should be one of the following,
>>>> +			    "ti,k2hk-dsp" for DSPs on Keystone 2 66AK2H/K SoCs
>>>> +			    "ti,k2l-dsp" for DSPs on Keystone 2 66AK2L SoCs
>>>> +			    "ti,k2e-dsp" for DSPs on Keystone 2 66AK2E SoCs
>>>> +
>>>> +- reg:			Should contain an entry for each value in 'reg-names'.
>>>> +			Each entry should have the memory region's start address
>>>> +			and the size of the region, the representation matching
>>>> +			the parent node's '#address-cells' and '#size-cells' values.
>>>> +
>>>> +- reg-names:		Should contain strings with the following names, each
>>>> +			representing a specific internal memory region, and
>>>> +			should be defined in this order,
>>>> +			     "l2sram", "l1pram", "l1dram"
>>>> +
>>>> +- label:		Should contain a string identifying the DSP instance
>>>> +			within the SoC. Should be the string "dsp" followed by
>>>> +			the instance number. Eg: "dsp0", "dsp1",..."dsp7" etc
>>>
>>> Why does a user need to know or care?
>>
>> This is used to distinguish the exact DSP instance from others since the
>> DT node name is a generic "dsp". One of the uses is to be able to
>> construct a firmware name within the driver using this label.
> 
> firmware-name doesn't work for you?

Has the stance changed w.r.t coding up the firmware-name in DT now? My
understanding was that this has to be coded up in the driver from a
previous related discussion on this [1]. I am more than happy to move
the firmware name into DT for all remoteprocs if that is an accepted
solution now.

> It was obvious that you are using this to number instances. Why do you 
> care which instance is which? For example, I want dsp0 because it has X 
> or runs at XXMHz, etc. If each instance is different (i.e. not a pool of 
> DSPs), then you should describe the difference some way.

Well, I am not exactly using the label to number the instance, it is a
possibility outside of using it to encode the firmware name. The
partitioning is really upto the application/SoC s/w system integration
aspects.  Most of the times, you are not running identical software on
these DSPs or other remote processors. For example, you might have one
DSP running an OpenCL stack, another DSP dedicated for audio
post-processing etc.

That said, I do have a need for numbering the instances. This is needed
usually for constructing a destination address to encode the processor
when using a common Inter-Processor Communication API across all
processors (Linux or otherwise). Pre-DT, I have used the platform device
id for this on Linux behaving as the master processor. For DT, my
preferred solution for that is an alias. If an alias is not acceptable,
then I still have to provide an equivalent functionality through a
driver-specific scheme.

regards
Suman

[1] http://www.spinics.net/lists/devicetree-spec/msg00140.html

--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring (Arm) June 5, 2017, 7:10 p.m. UTC | #5
On Mon, Jun 5, 2017 at 1:21 PM, Suman Anna <s-anna@ti.com> wrote:
> Hi Rob,
>
>>>
>>> On 05/31/2017 02:12 PM, Rob Herring wrote:
>>>> On Fri, May 26, 2017 at 11:53:15AM -0500, Suman Anna wrote:
>>>>> Add the device tree bindings document for the Texas Instrument's
>>>>> Keystone 2 DSP remoteproc devices.
>>>>>
>>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>>>> Signed-off-by: Sam Nelson <sam.nelson@ti.com>
>>>>> ---
>>>>>  .../bindings/remoteproc/ti,keystone-rproc.txt      | 132 +++++++++++++++++++++
>>>>>  1 file changed, 132 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt b/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
>>>>> new file mode 100644
>>>>> index 000000000000..f1ba88edd00d
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
>>>>> @@ -0,0 +1,132 @@
>>>>> +TI Keystone DSP devices
>>>>> +=======================
>>>>> +
>>>>> +Binding status: Unstable - Subject to changes for using multiple memory regions
>>>>
>>>> I don't really see what would be unstable here. memory-region is easily
>>>> extended to multiple entries.
>>>
>>> OK will drop this line.
>>>
>>>>
>>>>> +
>>>>> +The TI Keystone 2 family of SoCs usually have one or more (upto 8) TI DSP Core
>>>>> +sub-systems that are used to offload some of the processor-intensive tasks or
>>>>> +algorithms, for achieving various system level goals.
>>>>> +
>>>>> +These processor sub-systems usually contain additional sub-modules like L1
>>>>> +and/or L2 caches/SRAMs, an Interrupt Controller, an external memory controller,
>>>>> +a dedicated local power/sleep controller etc. The DSP processor core in
>>>>> +Keystone 2 SoCs is usually a TMS320C66x CorePac processor.
>>>>> +
>>>>> +DSP Device Node:
>>>>> +================
>>>>> +Each DSP Core sub-system is represented as a single DT node. Each node has a
>>>>> +number of required or optional properties that enable the OS running on the
>>>>> +host processor (ARM CorePac) to perform the device management of the remote
>>>>> +processor and to communicate with the remote processor.
>>>>> +
>>>>> +Required properties:
>>>>> +--------------------
>>>>> +The following are the mandatory properties:
>>>>> +
>>>>> +- compatible:             Should be one of the following,
>>>>> +                      "ti,k2hk-dsp" for DSPs on Keystone 2 66AK2H/K SoCs
>>>>> +                      "ti,k2l-dsp" for DSPs on Keystone 2 66AK2L SoCs
>>>>> +                      "ti,k2e-dsp" for DSPs on Keystone 2 66AK2E SoCs
>>>>> +
>>>>> +- reg:                    Should contain an entry for each value in 'reg-names'.
>>>>> +                  Each entry should have the memory region's start address
>>>>> +                  and the size of the region, the representation matching
>>>>> +                  the parent node's '#address-cells' and '#size-cells' values.
>>>>> +
>>>>> +- reg-names:              Should contain strings with the following names, each
>>>>> +                  representing a specific internal memory region, and
>>>>> +                  should be defined in this order,
>>>>> +                       "l2sram", "l1pram", "l1dram"
>>>>> +
>>>>> +- label:          Should contain a string identifying the DSP instance
>>>>> +                  within the SoC. Should be the string "dsp" followed by
>>>>> +                  the instance number. Eg: "dsp0", "dsp1",..."dsp7" etc
>>>>
>>>> Why does a user need to know or care?
>>>
>>> This is used to distinguish the exact DSP instance from others since the
>>> DT node name is a generic "dsp". One of the uses is to be able to
>>> construct a firmware name within the driver using this label.
>>
>> firmware-name doesn't work for you?
>
> Has the stance changed w.r.t coding up the firmware-name in DT now? My
> understanding was that this has to be coded up in the driver from a
> previous related discussion on this [1]. I am more than happy to move
> the firmware name into DT for all remoteprocs if that is an accepted
> solution now.

There's not a hard and fast rule. For FPGAs at least it made sense to
use firmware-name.

>> It was obvious that you are using this to number instances. Why do you
>> care which instance is which? For example, I want dsp0 because it has X
>> or runs at XXMHz, etc. If each instance is different (i.e. not a pool of
>> DSPs), then you should describe the difference some way.
>
> Well, I am not exactly using the label to number the instance, it is a
> possibility outside of using it to encode the firmware name. The
> partitioning is really upto the application/SoC s/w system integration
> aspects.  Most of the times, you are not running identical software on
> these DSPs or other remote processors. For example, you might have one
> DSP running an OpenCL stack, another DSP dedicated for audio
> post-processing etc.
>
> That said, I do have a need for numbering the instances. This is needed
> usually for constructing a destination address to encode the processor
> when using a common Inter-Processor Communication API across all
> processors (Linux or otherwise). Pre-DT, I have used the platform device
> id for this on Linux behaving as the master processor. For DT, my
> preferred solution for that is an alias. If an alias is not acceptable,
> then I still have to provide an equivalent functionality through a
> driver-specific scheme.

I would prefer aliases over labels here. It may make sense to just
have a property for the desitination address (or some offset). Or
perhaps a list of IPC targets and the index to that list is the
instance.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Suman Anna June 5, 2017, 8:05 p.m. UTC | #6
Hi Rob,

On 06/05/2017 02:10 PM, Rob Herring wrote:
> On Mon, Jun 5, 2017 at 1:21 PM, Suman Anna <s-anna@ti.com> wrote:
>> Hi Rob,
>>
>>>>
>>>> On 05/31/2017 02:12 PM, Rob Herring wrote:
>>>>> On Fri, May 26, 2017 at 11:53:15AM -0500, Suman Anna wrote:
>>>>>> Add the device tree bindings document for the Texas Instrument's
>>>>>> Keystone 2 DSP remoteproc devices.
>>>>>>
>>>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>>>>> Signed-off-by: Sam Nelson <sam.nelson@ti.com>
>>>>>> ---
>>>>>>  .../bindings/remoteproc/ti,keystone-rproc.txt      | 132 +++++++++++++++++++++
>>>>>>  1 file changed, 132 insertions(+)
>>>>>>  create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt b/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
>>>>>> new file mode 100644
>>>>>> index 000000000000..f1ba88edd00d
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
>>>>>> @@ -0,0 +1,132 @@
>>>>>> +TI Keystone DSP devices
>>>>>> +=======================
>>>>>> +
>>>>>> +Binding status: Unstable - Subject to changes for using multiple memory regions
>>>>>
>>>>> I don't really see what would be unstable here. memory-region is easily
>>>>> extended to multiple entries.
>>>>
>>>> OK will drop this line.
>>>>
>>>>>
>>>>>> +
>>>>>> +The TI Keystone 2 family of SoCs usually have one or more (upto 8) TI DSP Core
>>>>>> +sub-systems that are used to offload some of the processor-intensive tasks or
>>>>>> +algorithms, for achieving various system level goals.
>>>>>> +
>>>>>> +These processor sub-systems usually contain additional sub-modules like L1
>>>>>> +and/or L2 caches/SRAMs, an Interrupt Controller, an external memory controller,
>>>>>> +a dedicated local power/sleep controller etc. The DSP processor core in
>>>>>> +Keystone 2 SoCs is usually a TMS320C66x CorePac processor.
>>>>>> +
>>>>>> +DSP Device Node:
>>>>>> +================
>>>>>> +Each DSP Core sub-system is represented as a single DT node. Each node has a
>>>>>> +number of required or optional properties that enable the OS running on the
>>>>>> +host processor (ARM CorePac) to perform the device management of the remote
>>>>>> +processor and to communicate with the remote processor.
>>>>>> +
>>>>>> +Required properties:
>>>>>> +--------------------
>>>>>> +The following are the mandatory properties:
>>>>>> +
>>>>>> +- compatible:             Should be one of the following,
>>>>>> +                      "ti,k2hk-dsp" for DSPs on Keystone 2 66AK2H/K SoCs
>>>>>> +                      "ti,k2l-dsp" for DSPs on Keystone 2 66AK2L SoCs
>>>>>> +                      "ti,k2e-dsp" for DSPs on Keystone 2 66AK2E SoCs
>>>>>> +
>>>>>> +- reg:                    Should contain an entry for each value in 'reg-names'.
>>>>>> +                  Each entry should have the memory region's start address
>>>>>> +                  and the size of the region, the representation matching
>>>>>> +                  the parent node's '#address-cells' and '#size-cells' values.
>>>>>> +
>>>>>> +- reg-names:              Should contain strings with the following names, each
>>>>>> +                  representing a specific internal memory region, and
>>>>>> +                  should be defined in this order,
>>>>>> +                       "l2sram", "l1pram", "l1dram"
>>>>>> +
>>>>>> +- label:          Should contain a string identifying the DSP instance
>>>>>> +                  within the SoC. Should be the string "dsp" followed by
>>>>>> +                  the instance number. Eg: "dsp0", "dsp1",..."dsp7" etc
>>>>>
>>>>> Why does a user need to know or care?
>>>>
>>>> This is used to distinguish the exact DSP instance from others since the
>>>> DT node name is a generic "dsp". One of the uses is to be able to
>>>> construct a firmware name within the driver using this label.
>>>
>>> firmware-name doesn't work for you?
>>
>> Has the stance changed w.r.t coding up the firmware-name in DT now? My
>> understanding was that this has to be coded up in the driver from a
>> previous related discussion on this [1]. I am more than happy to move
>> the firmware name into DT for all remoteprocs if that is an accepted
>> solution now.
> 
> There's not a hard and fast rule. For FPGAs at least it made sense to
> use firmware-name.

OK, thanks for clarifying. I do have FPGA-like usage for some other
remote processors, for which I will use the firmware-name property. For
now, I will construct the f/w name in driver using the alias id.

> 
>>> It was obvious that you are using this to number instances. Why do you
>>> care which instance is which? For example, I want dsp0 because it has X
>>> or runs at XXMHz, etc. If each instance is different (i.e. not a pool of
>>> DSPs), then you should describe the difference some way.
>>
>> Well, I am not exactly using the label to number the instance, it is a
>> possibility outside of using it to encode the firmware name. The
>> partitioning is really upto the application/SoC s/w system integration
>> aspects.  Most of the times, you are not running identical software on
>> these DSPs or other remote processors. For example, you might have one
>> DSP running an OpenCL stack, another DSP dedicated for audio
>> post-processing etc.
>>
>> That said, I do have a need for numbering the instances. This is needed
>> usually for constructing a destination address to encode the processor
>> when using a common Inter-Processor Communication API across all
>> processors (Linux or otherwise). Pre-DT, I have used the platform device
>> id for this on Linux behaving as the master processor. For DT, my
>> preferred solution for that is an alias. If an alias is not acceptable,
>> then I still have to provide an equivalent functionality through a
>> driver-specific scheme.
> 
> I would prefer aliases over labels here. It may make sense to just
> have a property for the desitination address (or some offset). Or
> perhaps a list of IPC targets and the index to that list is the
> instance.

OK, I will go ahead and switch to using the aliases. The processor
number is just one part of the destination address encoding, and my
usage drivers are rpmsg bus drivers which operate on non-DT devices, so
can't use the IPC targets and index into that list.

Thanks for the review and the comments.

regards
Suman

--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt b/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
new file mode 100644
index 000000000000..f1ba88edd00d
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
@@ -0,0 +1,132 @@ 
+TI Keystone DSP devices
+=======================
+
+Binding status: Unstable - Subject to changes for using multiple memory regions
+
+The TI Keystone 2 family of SoCs usually have one or more (upto 8) TI DSP Core
+sub-systems that are used to offload some of the processor-intensive tasks or
+algorithms, for achieving various system level goals.
+
+These processor sub-systems usually contain additional sub-modules like L1
+and/or L2 caches/SRAMs, an Interrupt Controller, an external memory controller,
+a dedicated local power/sleep controller etc. The DSP processor core in
+Keystone 2 SoCs is usually a TMS320C66x CorePac processor.
+
+DSP Device Node:
+================
+Each DSP Core sub-system is represented as a single DT node. Each node has a
+number of required or optional properties that enable the OS running on the
+host processor (ARM CorePac) to perform the device management of the remote
+processor and to communicate with the remote processor.
+
+Required properties:
+--------------------
+The following are the mandatory properties:
+
+- compatible:		Should be one of the following,
+			    "ti,k2hk-dsp" for DSPs on Keystone 2 66AK2H/K SoCs
+			    "ti,k2l-dsp" for DSPs on Keystone 2 66AK2L SoCs
+			    "ti,k2e-dsp" for DSPs on Keystone 2 66AK2E SoCs
+
+- reg:			Should contain an entry for each value in 'reg-names'.
+			Each entry should have the memory region's start address
+			and the size of the region, the representation matching
+			the parent node's '#address-cells' and '#size-cells' values.
+
+- reg-names:		Should contain strings with the following names, each
+			representing a specific internal memory region, and
+			should be defined in this order,
+			     "l2sram", "l1pram", "l1dram"
+
+- label:		Should contain a string identifying the DSP instance
+			within the SoC. Should be the string "dsp" followed by
+			the instance number. Eg: "dsp0", "dsp1",..."dsp7" etc
+
+- clocks: 		Should contain the device's input clock, and should be
+			defined as per the bindings in,
+			Documentation/devicetree/bindings/clock/keystone-gate.txt
+
+- ti,syscon-dev:	Should be a pair of the phandle to the Keystone Device
+			State Control node, and the register offset of the DSP
+			boot address register within that node's address space.
+
+- resets:		Should contain the phandle to the reset controller node
+			managing the resets for this device, and a reset
+			specifier. Please refer to the following reset bindings
+			for the reset argument specifier as per SoC,
+			Documentation/devicetree/bindings/reset/ti-syscon-reset.txt
+			    for 66AK2HK/66AK2L/66AK2E SoCs
+
+- interrupt-parent:	Should contain a phandle to the Keystone 2 IRQ controller
+			IP node that is used by the ARM CorePac processor to
+			receive interrupts from the DSP remote processors. See
+			Documentation/devicetree/bindings/interrupt-controller/ti,keystone-irq.txt
+			for details.
+
+- interrupts: 		Should contain an entry for each value in 'interrupt-names'.
+			Each entry should have the interrupt source number used by
+			the remote processor to the host processor. The values should
+			follow the interrupt-specifier format as dictated by the
+			'interrupt-parent' node. The purpose of each is as per the
+			description in the 'interrupt-names' property.
+
+- interrupt-names:	Should contain strings with the following names, each
+			representing a specific interrupt,
+			    "vring" - interrupt for virtio based IPC
+			    "exception" - interrupt for exception notification
+
+- kick-gpio: 		Should specify the gpio device needed for the virtio IPC
+			stack. This will be used to interrupt the remote processor.
+			The gpio device to be used is as per the bindings in,
+			Documentation/devicetree/bindings/gpio/gpio-dsp-keystone.txt
+
+Optional properties:
+--------------------
+
+- memory-region:	phandle to the reserved memory node to be associated
+			with the remoteproc device. The reserved memory node
+			can be a CMA memory node, and should be defined as
+			per the bindings in
+			Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+
+
+Example:
+--------
+
+	/* 66AK2H/K DSP node in SoC DTS file */
+	soc {
+		dsp0: dsp@10800000 {
+			compatible = "ti,k2hk-dsp";
+			reg = <0x10800000 0x00100000>,
+			      <0x10e00000 0x00008000>,
+			      <0x10f00000 0x00008000>;
+			reg-names = "l2sram", "l1pram", "l1dram";
+			label = "dsp0";
+			clocks = <&clkgem0>;
+			ti,syscon-dev = <&devctrl 0x40>;
+			resets = <&pscrst 0>;
+			interrupt-parent = <&kirq0>;
+			interrupts = <0 8>;
+			interrupt-names = "vring", "exception";
+			kick-gpio = <&dspgpio0 27 0>;
+		};
+
+	};
+
+	/* K2HK EVM Board file */
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		dsp_common_cma_pool: dsp_common_cma_pool@81f800000 {
+			compatible = "shared-dma-pool";
+			reg = <0x00000008 0x1f800000 0x00000000 0x800000>;
+			reusable;
+			status = "okay";
+		};
+	};
+
+	&dsp0 {
+		memory-region = <&dsp_common_cma_pool>;
+	};