[PATCH 0/3] Add Keystone2 Remoteproc driver

[PATCH 0/3] Add Keystone2 Remoteproc driver

[Suman Anna]

Hi,

This series adds the DT binding and the driver for loading and booting
the DSP devices present on various Keystone2 SoC families. The current
series supports the 66AK2H/K, 66AK2L and 66AK2E SoC families. Support
for the remaining 66AK2G SoC family will be added later as it is
awaiting the dependent clock and reset drivers to be merged into
mainline.

Supported features include basic load/boot using DSP internal memory
regions and error recovery. IPC based on the virtio-rpmsg transport
with vrings in external DDR memory and interrupts supported through
MMRs.

The DTS nodes will be posted separately once the binding is accepted.
The virtio-rpmsg based IPC stack would require some support in the
virtio_rpmsg driver (first 2 patches from Loic's "virtio_rpmsg: make
rpmsg channel configurable" series [1] to begin with) if using CMA
pools from HighMem area.

regards
Suman

[1] https://lkml.org/lkml/2017/3/28/349

Andrew F. Davis (1):
  remoteproc/keystone: ensure the DSPs are in reset in probe

Suman Anna (2):
  Documentation: DT: add Keystone DSP remoteproc binding
  remoteproc/keystone: add a remoteproc driver for Keystone 2 DSPs

 .../bindings/remoteproc/ti,keystone-rproc.txt      | 132 ++++++
 drivers/remoteproc/Kconfig                         |  13 +
 drivers/remoteproc/Makefile                        |   1 +
 drivers/remoteproc/keystone_remoteproc.c           | 525 +++++++++++++++++++++
 4 files changed, 671 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
 create mode 100644 drivers/remoteproc/keystone_remoteproc.c

-- 
2.12.0

[PATCH 0/3] Add Keystone2 Remoteproc driver

[Suman Anna]

Hi,

This series adds the DT binding and the driver for loading and booting
the DSP devices present on various Keystone2 SoC families. The current
series supports the 66AK2H/K, 66AK2L and 66AK2E SoC families. Support
for the remaining 66AK2G SoC family will be added later as it is
awaiting the dependent clock and reset drivers to be merged into
mainline.

Supported features include basic load/boot using DSP internal memory
regions and error recovery. IPC based on the virtio-rpmsg transport
with vrings in external DDR memory and interrupts supported through
MMRs.

The DTS nodes will be posted separately once the binding is accepted.
The virtio-rpmsg based IPC stack would require some support in the
virtio_rpmsg driver (first 2 patches from Loic's "virtio_rpmsg: make
rpmsg channel configurable" series [1] to begin with) if using CMA
pools from HighMem area.

regards
Suman

[1] https://lkml.org/lkml/2017/3/28/349

Andrew F. Davis (1):
  remoteproc/keystone: ensure the DSPs are in reset in probe

Suman Anna (2):
  Documentation: DT: add Keystone DSP remoteproc binding
  remoteproc/keystone: add a remoteproc driver for Keystone 2 DSPs

 .../bindings/remoteproc/ti,keystone-rproc.txt      | 132 ++++++
 drivers/remoteproc/Kconfig                         |  13 +
 drivers/remoteproc/Makefile                        |   1 +
 drivers/remoteproc/keystone_remoteproc.c           | 525 +++++++++++++++++++++
 4 files changed, 671 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
 create mode 100644 drivers/remoteproc/keystone_remoteproc.c

-- 
2.12.0

[PATCH 0/3] Add Keystone2 Remoteproc driver

[Suman Anna]

Hi,

This series adds the DT binding and the driver for loading and booting
the DSP devices present on various Keystone2 SoC families. The current
series supports the 66AK2H/K, 66AK2L and 66AK2E SoC families. Support
for the remaining 66AK2G SoC family will be added later as it is
awaiting the dependent clock and reset drivers to be merged into
mainline.

Supported features include basic load/boot using DSP internal memory
regions and error recovery. IPC based on the virtio-rpmsg transport
with vrings in external DDR memory and interrupts supported through
MMRs.

The DTS nodes will be posted separately once the binding is accepted.
The virtio-rpmsg based IPC stack would require some support in the
virtio_rpmsg driver (first 2 patches from Loic's "virtio_rpmsg: make
rpmsg channel configurable" series [1] to begin with) if using CMA
pools from HighMem area.

regards
Suman

[1] https://lkml.org/lkml/2017/3/28/349

Andrew F. Davis (1):
  remoteproc/keystone: ensure the DSPs are in reset in probe

Suman Anna (2):
  Documentation: DT: add Keystone DSP remoteproc binding
  remoteproc/keystone: add a remoteproc driver for Keystone 2 DSPs

 .../bindings/remoteproc/ti,keystone-rproc.txt      | 132 ++++++
 drivers/remoteproc/Kconfig                         |  13 +
 drivers/remoteproc/Makefile                        |   1 +
 drivers/remoteproc/keystone_remoteproc.c           | 525 +++++++++++++++++++++
 4 files changed, 671 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt
 create mode 100644 drivers/remoteproc/keystone_remoteproc.c

-- 
2.12.0

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

[Suman Anna]

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
+
+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>;
+	};
-- 
2.12.0

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

[Suman Anna]

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
+
+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>;
+	};
-- 
2.12.0

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

[Suman Anna]

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
+
+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>;
+	};
-- 
2.12.0

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

[Suman Anna]

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
+
+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 at 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 at 81f800000 {
+			compatible = "shared-dma-pool";
+			reg = <0x00000008 0x1f800000 0x00000000 0x800000>;
+			reusable;
+			status = "okay";
+		};
+	};
+
+	&dsp0 {
+		memory-region = <&dsp_common_cma_pool>;
+	};
-- 
2.12.0

[PATCH 2/3] remoteproc/keystone: add a remoteproc driver for Keystone 2 DSPs

[Suman Anna]

The Texas Instrument's Keystone 2 family of SoCs has 1 or more
TMS320C66x DSP Core Subsystems (C66x CorePacs). Each subsystem has
a C66x Fixed/Floating-Point DSP Core, with 32KB of L1P and L1D SRAMs,
that can be configured and partitioned as either RAM and/or Cache,
and 1 MB of L2 SRAM. The CorePac also includes an Internal DMA (IDMA),
External Memory Controller (EMC), Extended Memory Controller (XMC)
with a Memory Protection and Address Extension (MPAX) unit, a Bandwidth
Management (BWM) unit, an Interrupt Controller (INTC) and a Powerdown
Controller (PDC).

A new remoteproc module is added to perform the device management of
these DSP devices. The driver expects the firmware names to be of the
form "keystone-dsp<X>-fw", where X is the corresponding DSP number, and
uses the standard remoteproc core ELF loader. The support is limited
to images only using the DSP internal memories at the moment. This
remoteproc driver is also designed to work with virtio, and uses the
IPC Generation registers for performing the virtio signalling and
getting notified of exceptions.

The driver currently supports the 66AK2H/66AK2K, 66AK2L and 66AK2E
SoCs.

Signed-off-by: Suman Anna <s-anna@ti.com>
Signed-off-by: Sam Nelson <sam.nelson@ti.com>
Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 drivers/remoteproc/Kconfig               |  13 +
 drivers/remoteproc/Makefile              |   1 +
 drivers/remoteproc/keystone_remoteproc.c | 515 +++++++++++++++++++++++++++++++
 3 files changed, 529 insertions(+)
 create mode 100644 drivers/remoteproc/keystone_remoteproc.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index faad69a1a597..9839b35b6ab3 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -71,6 +71,19 @@ config DA8XX_REMOTEPROC
 	  It's safe to say n here if you're not interested in multimedia
 	  offloading.
 
+config KEYSTONE_REMOTEPROC
+	tristate "Keystone Remoteproc support"
+	depends on ARCH_KEYSTONE
+	depends on RESET_CONTROLLER
+	depends on REMOTEPROC
+	select RPMSG_VIRTIO
+	help
+	  Say Y here here to support Keystone remote processors (DSP)
+	  via the remote processor framework.
+
+	  It's safe to say N here if you're not interested in the Keystone
+	  DSPs or just want to use a bare minimum kernel.
+
 config QCOM_ADSP_PIL
 	tristate "Qualcomm ADSP Peripheral Image Loader"
 	depends on OF && ARCH_QCOM
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index ffc5e430df27..f1ce5fc8a2f3 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -11,6 +11,7 @@ remoteproc-y				+= remoteproc_elf_loader.o
 obj-$(CONFIG_OMAP_REMOTEPROC)		+= omap_remoteproc.o
 obj-$(CONFIG_WKUP_M3_RPROC)		+= wkup_m3_rproc.o
 obj-$(CONFIG_DA8XX_REMOTEPROC)		+= da8xx_remoteproc.o
+obj-$(CONFIG_KEYSTONE_REMOTEPROC)	+= keystone_remoteproc.o
 obj-$(CONFIG_QCOM_ADSP_PIL)		+= qcom_adsp_pil.o
 obj-$(CONFIG_QCOM_RPROC_COMMON)		+= qcom_common.o
 obj-$(CONFIG_QCOM_Q6V5_PIL)		+= qcom_q6v5_pil.o
diff --git a/drivers/remoteproc/keystone_remoteproc.c b/drivers/remoteproc/keystone_remoteproc.c
new file mode 100644
index 000000000000..d01d5e3fce28
--- /dev/null
+++ b/drivers/remoteproc/keystone_remoteproc.c
@@ -0,0 +1,515 @@
+/*
+ * TI Keystone DSP remoteproc driver
+ *
+ * Copyright (C) 2015-2017 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/workqueue.h>
+#include <linux/of_address.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/of_gpio.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+#include <linux/remoteproc.h>
+#include <linux/reset.h>
+
+#include "remoteproc_internal.h"
+
+#define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK	(SZ_16M - 1)
+
+/**
+ * struct keystone_rproc_mem - internal memory structure
+ * @cpu_addr: MPU virtual address of the memory region
+ * @bus_addr: Bus address used to access the memory region
+ * @dev_addr: Device address of the memory region from DSP view
+ * @size: Size of the memory region
+ */
+struct keystone_rproc_mem {
+	void __iomem *cpu_addr;
+	phys_addr_t bus_addr;
+	u32 dev_addr;
+	size_t size;
+};
+
+/**
+ * struct keystone_rproc - keystone remote processor driver structure
+ * @dev: cached device pointer
+ * @rproc: remoteproc device handle
+ * @mem: internal memory regions data
+ * @num_mems: number of internal memory regions
+ * @dev_ctrl: device control regmap handle
+ * @reset: reset control handle
+ * @boot_offset: boot register offset in @dev_ctrl regmap
+ * @irq_ring: irq entry for vring
+ * @irq_fault: irq entry for exception
+ * @kick_gpio: gpio used for virtio kicks
+ * @workqueue: workqueue for processing virtio interrupts
+ */
+struct keystone_rproc {
+	struct device *dev;
+	struct rproc *rproc;
+	struct keystone_rproc_mem *mem;
+	int num_mems;
+	struct regmap *dev_ctrl;
+	struct reset_control *reset;
+	u32 boot_offset;
+	int irq_ring;
+	int irq_fault;
+	int kick_gpio;
+	struct work_struct workqueue;
+};
+
+/* Put the DSP processor into reset */
+static void keystone_rproc_dsp_reset(struct keystone_rproc *ksproc)
+{
+	reset_control_assert(ksproc->reset);
+}
+
+/* Configure the boot address and boot the DSP processor */
+static int keystone_rproc_dsp_boot(struct keystone_rproc *ksproc, u32 boot_addr)
+{
+	int ret;
+
+	if (boot_addr & (SZ_1K - 1)) {
+		dev_err(ksproc->dev, "invalid boot address 0x%x, must be aligned on a 1KB boundary\n",
+			boot_addr);
+		return -EINVAL;
+	}
+
+	ret = regmap_write(ksproc->dev_ctrl, ksproc->boot_offset, boot_addr);
+	if (ret) {
+		dev_err(ksproc->dev, "regmap_write of boot address failed, status = %d\n",
+			ret);
+		return ret;
+	}
+
+	reset_control_deassert(ksproc->reset);
+
+	return 0;
+}
+
+/*
+ * Process the remoteproc exceptions
+ *
+ * The exception reporting on Keystone DSP remote processors is very simple
+ * compared to the equivalent processors on the OMAP family, it is notified
+ * through a software-designed specific interrupt source in the IPC interrupt
+ * generation register.
+ *
+ * This function just invokes the rproc_report_crash to report the exception
+ * to the remoteproc driver core, to trigger a recovery.
+ */
+static irqreturn_t keystone_rproc_exception_interrupt(int irq, void *dev_id)
+{
+	struct keystone_rproc *ksproc = dev_id;
+
+	rproc_report_crash(ksproc->rproc, RPROC_FATAL_ERROR);
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * Main virtqueue message workqueue function
+ *
+ * This function is executed upon scheduling of the keystone remoteproc
+ * driver's workqueue. The workqueue is scheduled by the vring ISR handler.
+ *
+ * There is no payload message indicating the virtqueue index as is the
+ * case with mailbox-based implementations on OMAP family. As such, this
+ * handler processes both the Tx and Rx virtqueue indices on every invocation.
+ * The rproc_vq_interrupt function can detect if there are new unprocessed
+ * messages or not (returns IRQ_NONE vs IRQ_HANDLED), but there is no need
+ * to check for these return values. The index 0 triggering will process all
+ * pending Rx buffers, and the index 1 triggering will process all newly
+ * available Tx buffers and will wakeup any potentially blocked senders.
+ *
+ * NOTE:
+ * 1. A payload could be added by using some of the source bits in the
+ *    IPC interrupt generation registers, but this would need additional
+ *    changes to the overall IPC stack, and currently there are no benefits
+ *    of adapting that approach.
+ * 2. The current logic is based on an inherent design assumption of supporting
+ *    only 2 vrings, but this can be changed if needed.
+ */
+static void handle_event(struct work_struct *work)
+{
+	struct keystone_rproc *ksproc =
+		container_of(work, struct keystone_rproc, workqueue);
+
+	rproc_vq_interrupt(ksproc->rproc, 0);
+	rproc_vq_interrupt(ksproc->rproc, 1);
+}
+
+/*
+ * Interrupt handler for processing vring kicks from remote processor
+ */
+static irqreturn_t keystone_rproc_vring_interrupt(int irq, void *dev_id)
+{
+	struct keystone_rproc *ksproc = dev_id;
+
+	schedule_work(&ksproc->workqueue);
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * Power up the DSP remote processor.
+ *
+ * This function will be invoked only after the firmware for this rproc
+ * was loaded, parsed successfully, and all of its resource requirements
+ * were met.
+ */
+static int keystone_rproc_start(struct rproc *rproc)
+{
+	struct keystone_rproc *ksproc = rproc->priv;
+	int ret;
+
+	INIT_WORK(&ksproc->workqueue, handle_event);
+
+	ret = request_irq(ksproc->irq_ring, keystone_rproc_vring_interrupt, 0,
+			  dev_name(ksproc->dev), ksproc);
+	if (ret) {
+		dev_err(ksproc->dev, "failed to enable vring interrupt, ret = %d\n",
+			ret);
+		goto out;
+	}
+
+	ret = request_irq(ksproc->irq_fault, keystone_rproc_exception_interrupt,
+			  0, dev_name(ksproc->dev), ksproc);
+	if (ret) {
+		dev_err(ksproc->dev, "failed to enable exception interrupt, ret = %d\n",
+			ret);
+		goto free_vring_irq;
+	}
+
+	ret = keystone_rproc_dsp_boot(ksproc, rproc->bootaddr);
+	if (ret)
+		goto free_exc_irq;
+
+	return 0;
+
+free_exc_irq:
+	free_irq(ksproc->irq_fault, ksproc);
+free_vring_irq:
+	free_irq(ksproc->irq_ring, ksproc);
+	flush_work(&ksproc->workqueue);
+out:
+	return ret;
+}
+
+/*
+ * Stop the DSP remote processor.
+ *
+ * This function puts the DSP processor into reset, and finishes processing
+ * of any pending messages.
+ */
+static int keystone_rproc_stop(struct rproc *rproc)
+{
+	struct keystone_rproc *ksproc = rproc->priv;
+
+	keystone_rproc_dsp_reset(ksproc);
+	free_irq(ksproc->irq_fault, ksproc);
+	free_irq(ksproc->irq_ring, ksproc);
+	flush_work(&ksproc->workqueue);
+
+	return 0;
+}
+
+/*
+ * Kick the remote processor to notify about pending unprocessed messages.
+ * The vqid usage is not used and is inconsequential, as the kick is performed
+ * through a simulated GPIO (a bit in an IPC interrupt-triggering register),
+ * the remote processor is expected to process both its Tx and Rx virtqueues.
+ */
+static void keystone_rproc_kick(struct rproc *rproc, int vqid)
+{
+	struct keystone_rproc *ksproc = rproc->priv;
+
+	if (WARN_ON(ksproc->kick_gpio < 0))
+		return;
+
+	gpio_set_value(ksproc->kick_gpio, 1);
+}
+
+/*
+ * Custom function to translate a DSP device address (internal RAMs only) to a
+ * kernel virtual address.  The DSPs can access their RAMs at either an internal
+ * address visible only from a DSP, or at the SoC-level bus address. Both these
+ * addresses need to be looked through for translation. The translated addresses
+ * can be used either by the remoteproc core for loading (when using kernel
+ * remoteproc loader), or by any rpmsg bus drivers.
+ */
+static void *keystone_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
+{
+	struct keystone_rproc *ksproc = rproc->priv;
+	void __iomem *va = NULL;
+	phys_addr_t bus_addr;
+	u32 dev_addr, offset;
+	size_t size;
+	int i;
+
+	if (len <= 0)
+		return NULL;
+
+	for (i = 0; i < ksproc->num_mems; i++) {
+		bus_addr = ksproc->mem[i].bus_addr;
+		dev_addr = ksproc->mem[i].dev_addr;
+		size = ksproc->mem[i].size;
+
+		if (da < KEYSTONE_RPROC_LOCAL_ADDRESS_MASK) {
+			/* handle DSP-view addresses */
+			if ((da >= dev_addr) &&
+			    ((da + len) <= (dev_addr + size))) {
+				offset = da - dev_addr;
+				va = ksproc->mem[i].cpu_addr + offset;
+				break;
+			}
+		} else {
+			/* handle SoC-view addresses */
+			if ((da >= bus_addr) &&
+			    (da + len) <= (bus_addr + size)) {
+				offset = da - bus_addr;
+				va = ksproc->mem[i].cpu_addr + offset;
+				break;
+			}
+		}
+	}
+
+	return (__force void *)va;
+}
+
+static const struct rproc_ops keystone_rproc_ops = {
+	.start		= keystone_rproc_start,
+	.stop		= keystone_rproc_stop,
+	.kick		= keystone_rproc_kick,
+	.da_to_va	= keystone_rproc_da_to_va,
+};
+
+static int keystone_rproc_of_get_memories(struct platform_device *pdev,
+					  struct keystone_rproc *ksproc)
+{
+	static const char * const mem_names[] = {"l2sram", "l1pram", "l1dram"};
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	int num_mems = 0;
+	int i;
+
+	num_mems = ARRAY_SIZE(mem_names);
+	ksproc->mem = devm_kcalloc(ksproc->dev, num_mems,
+				   sizeof(*ksproc->mem), GFP_KERNEL);
+	if (!ksproc->mem)
+		return -ENOMEM;
+
+	for (i = 0; i < num_mems; i++) {
+		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+						   mem_names[i]);
+		ksproc->mem[i].cpu_addr = devm_ioremap_resource(dev, res);
+		if (IS_ERR(ksproc->mem[i].cpu_addr)) {
+			dev_err(dev, "failed to parse and map %s memory\n",
+				mem_names[i]);
+			return PTR_ERR(ksproc->mem[i].cpu_addr);
+		}
+		ksproc->mem[i].bus_addr = res->start;
+		ksproc->mem[i].dev_addr =
+				res->start & KEYSTONE_RPROC_LOCAL_ADDRESS_MASK;
+		ksproc->mem[i].size = resource_size(res);
+
+		/* zero out memories to start in a pristine state */
+		memset((__force void *)ksproc->mem[i].cpu_addr, 0,
+		       ksproc->mem[i].size);
+	}
+	ksproc->num_mems = num_mems;
+
+	return 0;
+}
+
+static int keystone_rproc_of_get_dev_syscon(struct platform_device *pdev,
+					    struct keystone_rproc *ksproc)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	if (!of_property_read_bool(np, "ti,syscon-dev")) {
+		dev_err(dev, "ti,syscon-dev property is absent\n");
+		return -EINVAL;
+	}
+
+	ksproc->dev_ctrl =
+		syscon_regmap_lookup_by_phandle(np, "ti,syscon-dev");
+	if (IS_ERR(ksproc->dev_ctrl)) {
+		ret = PTR_ERR(ksproc->dev_ctrl);
+		return ret;
+	}
+
+	if (of_property_read_u32_index(np, "ti,syscon-dev", 1,
+				       &ksproc->boot_offset)) {
+		dev_err(dev, "couldn't read the boot register offset\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int keystone_rproc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct keystone_rproc *ksproc;
+	struct rproc *rproc;
+	const char *name;
+	char *fw_name = NULL;
+	char *template = "keystone-%s-fw";
+	int name_len = 0;
+	int ret = 0;
+
+	if (!np) {
+		dev_err(dev, "only DT-based devices are supported\n");
+		return -ENODEV;
+	}
+
+	ret = of_property_read_string(np, "label", &name);
+	if (ret < 0) {
+		dev_err(dev, "failed to get label, status = %d\n", ret);
+		return ret;
+	}
+
+	/* construct a custom default fw name - subject to change in future */
+	name_len = strlen(name) + strlen(template) - 2 + 1;
+	fw_name = devm_kzalloc(dev, name_len, GFP_KERNEL);
+	if (!fw_name)
+		return -ENOMEM;
+	snprintf(fw_name, name_len, template, name);
+
+	rproc = rproc_alloc(dev, dev_name(dev), &keystone_rproc_ops, fw_name,
+			    sizeof(*ksproc));
+	if (!rproc)
+		return -ENOMEM;
+
+	rproc->has_iommu = false;
+	ksproc = rproc->priv;
+	ksproc->rproc = rproc;
+	ksproc->dev = dev;
+
+	ret = keystone_rproc_of_get_dev_syscon(pdev, ksproc);
+	if (ret)
+		goto free_rproc;
+
+	ksproc->reset = devm_reset_control_get(dev, NULL);
+	if (IS_ERR(ksproc->reset)) {
+		ret = PTR_ERR(ksproc->reset);
+		goto free_rproc;
+	}
+
+	/* enable clock for accessing DSP internal memories */
+	pm_runtime_enable(dev);
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		dev_err(dev, "failed to enable clock, status = %d\n", ret);
+		pm_runtime_put_noidle(dev);
+		goto disable_rpm;
+	}
+
+	ret = keystone_rproc_of_get_memories(pdev, ksproc);
+	if (ret)
+		goto disable_clk;
+
+	ksproc->irq_ring = platform_get_irq_byname(pdev, "vring");
+	if (ksproc->irq_ring < 0) {
+		ret = ksproc->irq_ring;
+		dev_err(dev, "failed to get vring interrupt, status = %d\n",
+			ret);
+		goto disable_clk;
+	}
+
+	ksproc->irq_fault = platform_get_irq_byname(pdev, "exception");
+	if (ksproc->irq_fault < 0) {
+		ret = ksproc->irq_fault;
+		dev_err(dev, "failed to get exception interrupt, status = %d\n",
+			ret);
+		goto disable_clk;
+	}
+
+	ksproc->kick_gpio = of_get_named_gpio_flags(np, "kick-gpio", 0, NULL);
+	if (ksproc->kick_gpio < 0) {
+		ret = ksproc->kick_gpio;
+		dev_err(dev, "failed to get gpio for virtio kicks, status = %d\n",
+			ret);
+		goto disable_clk;
+	}
+
+	if (of_reserved_mem_device_init(dev))
+		dev_warn(dev, "device does not have specific CMA pool\n");
+
+	ret = rproc_add(rproc);
+	if (ret) {
+		dev_err(dev, "failed to add register device with remoteproc core, status = %d\n",
+			ret);
+		goto release_mem;
+	}
+
+	platform_set_drvdata(pdev, ksproc);
+
+	return 0;
+
+release_mem:
+	of_reserved_mem_device_release(dev);
+disable_clk:
+	pm_runtime_put_sync(dev);
+disable_rpm:
+	pm_runtime_disable(dev);
+free_rproc:
+	rproc_free(rproc);
+	return ret;
+}
+
+static int keystone_rproc_remove(struct platform_device *pdev)
+{
+	struct keystone_rproc *ksproc = platform_get_drvdata(pdev);
+
+	rproc_del(ksproc->rproc);
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+	rproc_free(ksproc->rproc);
+	of_reserved_mem_device_release(&pdev->dev);
+
+	return 0;
+}
+
+static const struct of_device_id keystone_rproc_of_match[] = {
+	{ .compatible = "ti,k2hk-dsp", },
+	{ .compatible = "ti,k2l-dsp", },
+	{ .compatible = "ti,k2e-dsp", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, keystone_rproc_of_match);
+
+static struct platform_driver keystone_rproc_driver = {
+	.probe	= keystone_rproc_probe,
+	.remove	= keystone_rproc_remove,
+	.driver	= {
+		.name = "keystone-rproc",
+		.of_match_table = keystone_rproc_of_match,
+	},
+};
+
+module_platform_driver(keystone_rproc_driver);
+
+MODULE_AUTHOR("Suman Anna <s-anna@ti.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("TI Keystone DSP Remoteproc driver");
-- 
2.12.0

[PATCH 2/3] remoteproc/keystone: add a remoteproc driver for Keystone 2 DSPs

[Suman Anna]

The Texas Instrument's Keystone 2 family of SoCs has 1 or more
TMS320C66x DSP Core Subsystems (C66x CorePacs). Each subsystem has
a C66x Fixed/Floating-Point DSP Core, with 32KB of L1P and L1D SRAMs,
that can be configured and partitioned as either RAM and/or Cache,
and 1 MB of L2 SRAM. The CorePac also includes an Internal DMA (IDMA),
External Memory Controller (EMC), Extended Memory Controller (XMC)
with a Memory Protection and Address Extension (MPAX) unit, a Bandwidth
Management (BWM) unit, an Interrupt Controller (INTC) and a Powerdown
Controller (PDC).

A new remoteproc module is added to perform the device management of
these DSP devices. The driver expects the firmware names to be of the
form "keystone-dsp<X>-fw", where X is the corresponding DSP number, and
uses the standard remoteproc core ELF loader. The support is limited
to images only using the DSP internal memories at the moment. This
remoteproc driver is also designed to work with virtio, and uses the
IPC Generation registers for performing the virtio signalling and
getting notified of exceptions.

The driver currently supports the 66AK2H/66AK2K, 66AK2L and 66AK2E
SoCs.

Signed-off-by: Suman Anna <s-anna@ti.com>
Signed-off-by: Sam Nelson <sam.nelson@ti.com>
Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 drivers/remoteproc/Kconfig               |  13 +
 drivers/remoteproc/Makefile              |   1 +
 drivers/remoteproc/keystone_remoteproc.c | 515 +++++++++++++++++++++++++++++++
 3 files changed, 529 insertions(+)
 create mode 100644 drivers/remoteproc/keystone_remoteproc.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index faad69a1a597..9839b35b6ab3 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -71,6 +71,19 @@ config DA8XX_REMOTEPROC
 	  It's safe to say n here if you're not interested in multimedia
 	  offloading.
 
+config KEYSTONE_REMOTEPROC
+	tristate "Keystone Remoteproc support"
+	depends on ARCH_KEYSTONE
+	depends on RESET_CONTROLLER
+	depends on REMOTEPROC
+	select RPMSG_VIRTIO
+	help
+	  Say Y here here to support Keystone remote processors (DSP)
+	  via the remote processor framework.
+
+	  It's safe to say N here if you're not interested in the Keystone
+	  DSPs or just want to use a bare minimum kernel.
+
 config QCOM_ADSP_PIL
 	tristate "Qualcomm ADSP Peripheral Image Loader"
 	depends on OF && ARCH_QCOM
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index ffc5e430df27..f1ce5fc8a2f3 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -11,6 +11,7 @@ remoteproc-y				+= remoteproc_elf_loader.o
 obj-$(CONFIG_OMAP_REMOTEPROC)		+= omap_remoteproc.o
 obj-$(CONFIG_WKUP_M3_RPROC)		+= wkup_m3_rproc.o
 obj-$(CONFIG_DA8XX_REMOTEPROC)		+= da8xx_remoteproc.o
+obj-$(CONFIG_KEYSTONE_REMOTEPROC)	+= keystone_remoteproc.o
 obj-$(CONFIG_QCOM_ADSP_PIL)		+= qcom_adsp_pil.o
 obj-$(CONFIG_QCOM_RPROC_COMMON)		+= qcom_common.o
 obj-$(CONFIG_QCOM_Q6V5_PIL)		+= qcom_q6v5_pil.o
diff --git a/drivers/remoteproc/keystone_remoteproc.c b/drivers/remoteproc/keystone_remoteproc.c
new file mode 100644
index 000000000000..d01d5e3fce28
--- /dev/null
+++ b/drivers/remoteproc/keystone_remoteproc.c
@@ -0,0 +1,515 @@
+/*
+ * TI Keystone DSP remoteproc driver
+ *
+ * Copyright (C) 2015-2017 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/workqueue.h>
+#include <linux/of_address.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/of_gpio.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+#include <linux/remoteproc.h>
+#include <linux/reset.h>
+
+#include "remoteproc_internal.h"
+
+#define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK	(SZ_16M - 1)
+
+/**
+ * struct keystone_rproc_mem - internal memory structure
+ * @cpu_addr: MPU virtual address of the memory region
+ * @bus_addr: Bus address used to access the memory region
+ * @dev_addr: Device address of the memory region from DSP view
+ * @size: Size of the memory region
+ */
+struct keystone_rproc_mem {
+	void __iomem *cpu_addr;
+	phys_addr_t bus_addr;
+	u32 dev_addr;
+	size_t size;
+};
+
+/**
+ * struct keystone_rproc - keystone remote processor driver structure
+ * @dev: cached device pointer
+ * @rproc: remoteproc device handle
+ * @mem: internal memory regions data
+ * @num_mems: number of internal memory regions
+ * @dev_ctrl: device control regmap handle
+ * @reset: reset control handle
+ * @boot_offset: boot register offset in @dev_ctrl regmap
+ * @irq_ring: irq entry for vring
+ * @irq_fault: irq entry for exception
+ * @kick_gpio: gpio used for virtio kicks
+ * @workqueue: workqueue for processing virtio interrupts
+ */
+struct keystone_rproc {
+	struct device *dev;
+	struct rproc *rproc;
+	struct keystone_rproc_mem *mem;
+	int num_mems;
+	struct regmap *dev_ctrl;
+	struct reset_control *reset;
+	u32 boot_offset;
+	int irq_ring;
+	int irq_fault;
+	int kick_gpio;
+	struct work_struct workqueue;
+};
+
+/* Put the DSP processor into reset */
+static void keystone_rproc_dsp_reset(struct keystone_rproc *ksproc)
+{
+	reset_control_assert(ksproc->reset);
+}
+
+/* Configure the boot address and boot the DSP processor */
+static int keystone_rproc_dsp_boot(struct keystone_rproc *ksproc, u32 boot_addr)
+{
+	int ret;
+
+	if (boot_addr & (SZ_1K - 1)) {
+		dev_err(ksproc->dev, "invalid boot address 0x%x, must be aligned on a 1KB boundary\n",
+			boot_addr);
+		return -EINVAL;
+	}
+
+	ret = regmap_write(ksproc->dev_ctrl, ksproc->boot_offset, boot_addr);
+	if (ret) {
+		dev_err(ksproc->dev, "regmap_write of boot address failed, status = %d\n",
+			ret);
+		return ret;
+	}
+
+	reset_control_deassert(ksproc->reset);
+
+	return 0;
+}
+
+/*
+ * Process the remoteproc exceptions
+ *
+ * The exception reporting on Keystone DSP remote processors is very simple
+ * compared to the equivalent processors on the OMAP family, it is notified
+ * through a software-designed specific interrupt source in the IPC interrupt
+ * generation register.
+ *
+ * This function just invokes the rproc_report_crash to report the exception
+ * to the remoteproc driver core, to trigger a recovery.
+ */
+static irqreturn_t keystone_rproc_exception_interrupt(int irq, void *dev_id)
+{
+	struct keystone_rproc *ksproc = dev_id;
+
+	rproc_report_crash(ksproc->rproc, RPROC_FATAL_ERROR);
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * Main virtqueue message workqueue function
+ *
+ * This function is executed upon scheduling of the keystone remoteproc
+ * driver's workqueue. The workqueue is scheduled by the vring ISR handler.
+ *
+ * There is no payload message indicating the virtqueue index as is the
+ * case with mailbox-based implementations on OMAP family. As such, this
+ * handler processes both the Tx and Rx virtqueue indices on every invocation.
+ * The rproc_vq_interrupt function can detect if there are new unprocessed
+ * messages or not (returns IRQ_NONE vs IRQ_HANDLED), but there is no need
+ * to check for these return values. The index 0 triggering will process all
+ * pending Rx buffers, and the index 1 triggering will process all newly
+ * available Tx buffers and will wakeup any potentially blocked senders.
+ *
+ * NOTE:
+ * 1. A payload could be added by using some of the source bits in the
+ *    IPC interrupt generation registers, but this would need additional
+ *    changes to the overall IPC stack, and currently there are no benefits
+ *    of adapting that approach.
+ * 2. The current logic is based on an inherent design assumption of supporting
+ *    only 2 vrings, but this can be changed if needed.
+ */
+static void handle_event(struct work_struct *work)
+{
+	struct keystone_rproc *ksproc =
+		container_of(work, struct keystone_rproc, workqueue);
+
+	rproc_vq_interrupt(ksproc->rproc, 0);
+	rproc_vq_interrupt(ksproc->rproc, 1);
+}
+
+/*
+ * Interrupt handler for processing vring kicks from remote processor
+ */
+static irqreturn_t keystone_rproc_vring_interrupt(int irq, void *dev_id)
+{
+	struct keystone_rproc *ksproc = dev_id;
+
+	schedule_work(&ksproc->workqueue);
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * Power up the DSP remote processor.
+ *
+ * This function will be invoked only after the firmware for this rproc
+ * was loaded, parsed successfully, and all of its resource requirements
+ * were met.
+ */
+static int keystone_rproc_start(struct rproc *rproc)
+{
+	struct keystone_rproc *ksproc = rproc->priv;
+	int ret;
+
+	INIT_WORK(&ksproc->workqueue, handle_event);
+
+	ret = request_irq(ksproc->irq_ring, keystone_rproc_vring_interrupt, 0,
+			  dev_name(ksproc->dev), ksproc);
+	if (ret) {
+		dev_err(ksproc->dev, "failed to enable vring interrupt, ret = %d\n",
+			ret);
+		goto out;
+	}
+
+	ret = request_irq(ksproc->irq_fault, keystone_rproc_exception_interrupt,
+			  0, dev_name(ksproc->dev), ksproc);
+	if (ret) {
+		dev_err(ksproc->dev, "failed to enable exception interrupt, ret = %d\n",
+			ret);
+		goto free_vring_irq;
+	}
+
+	ret = keystone_rproc_dsp_boot(ksproc, rproc->bootaddr);
+	if (ret)
+		goto free_exc_irq;
+
+	return 0;
+
+free_exc_irq:
+	free_irq(ksproc->irq_fault, ksproc);
+free_vring_irq:
+	free_irq(ksproc->irq_ring, ksproc);
+	flush_work(&ksproc->workqueue);
+out:
+	return ret;
+}
+
+/*
+ * Stop the DSP remote processor.
+ *
+ * This function puts the DSP processor into reset, and finishes processing
+ * of any pending messages.
+ */
+static int keystone_rproc_stop(struct rproc *rproc)
+{
+	struct keystone_rproc *ksproc = rproc->priv;
+
+	keystone_rproc_dsp_reset(ksproc);
+	free_irq(ksproc->irq_fault, ksproc);
+	free_irq(ksproc->irq_ring, ksproc);
+	flush_work(&ksproc->workqueue);
+
+	return 0;
+}
+
+/*
+ * Kick the remote processor to notify about pending unprocessed messages.
+ * The vqid usage is not used and is inconsequential, as the kick is performed
+ * through a simulated GPIO (a bit in an IPC interrupt-triggering register),
+ * the remote processor is expected to process both its Tx and Rx virtqueues.
+ */
+static void keystone_rproc_kick(struct rproc *rproc, int vqid)
+{
+	struct keystone_rproc *ksproc = rproc->priv;
+
+	if (WARN_ON(ksproc->kick_gpio < 0))
+		return;
+
+	gpio_set_value(ksproc->kick_gpio, 1);
+}
+
+/*
+ * Custom function to translate a DSP device address (internal RAMs only) to a
+ * kernel virtual address.  The DSPs can access their RAMs at either an internal
+ * address visible only from a DSP, or at the SoC-level bus address. Both these
+ * addresses need to be looked through for translation. The translated addresses
+ * can be used either by the remoteproc core for loading (when using kernel
+ * remoteproc loader), or by any rpmsg bus drivers.
+ */
+static void *keystone_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
+{
+	struct keystone_rproc *ksproc = rproc->priv;
+	void __iomem *va = NULL;
+	phys_addr_t bus_addr;
+	u32 dev_addr, offset;
+	size_t size;
+	int i;
+
+	if (len <= 0)
+		return NULL;
+
+	for (i = 0; i < ksproc->num_mems; i++) {
+		bus_addr = ksproc->mem[i].bus_addr;
+		dev_addr = ksproc->mem[i].dev_addr;
+		size = ksproc->mem[i].size;
+
+		if (da < KEYSTONE_RPROC_LOCAL_ADDRESS_MASK) {
+			/* handle DSP-view addresses */
+			if ((da >= dev_addr) &&
+			    ((da + len) <= (dev_addr + size))) {
+				offset = da - dev_addr;
+				va = ksproc->mem[i].cpu_addr + offset;
+				break;
+			}
+		} else {
+			/* handle SoC-view addresses */
+			if ((da >= bus_addr) &&
+			    (da + len) <= (bus_addr + size)) {
+				offset = da - bus_addr;
+				va = ksproc->mem[i].cpu_addr + offset;
+				break;
+			}
+		}
+	}
+
+	return (__force void *)va;
+}
+
+static const struct rproc_ops keystone_rproc_ops = {
+	.start		= keystone_rproc_start,
+	.stop		= keystone_rproc_stop,
+	.kick		= keystone_rproc_kick,
+	.da_to_va	= keystone_rproc_da_to_va,
+};
+
+static int keystone_rproc_of_get_memories(struct platform_device *pdev,
+					  struct keystone_rproc *ksproc)
+{
+	static const char * const mem_names[] = {"l2sram", "l1pram", "l1dram"};
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	int num_mems = 0;
+	int i;
+
+	num_mems = ARRAY_SIZE(mem_names);
+	ksproc->mem = devm_kcalloc(ksproc->dev, num_mems,
+				   sizeof(*ksproc->mem), GFP_KERNEL);
+	if (!ksproc->mem)
+		return -ENOMEM;
+
+	for (i = 0; i < num_mems; i++) {
+		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+						   mem_names[i]);
+		ksproc->mem[i].cpu_addr = devm_ioremap_resource(dev, res);
+		if (IS_ERR(ksproc->mem[i].cpu_addr)) {
+			dev_err(dev, "failed to parse and map %s memory\n",
+				mem_names[i]);
+			return PTR_ERR(ksproc->mem[i].cpu_addr);
+		}
+		ksproc->mem[i].bus_addr = res->start;
+		ksproc->mem[i].dev_addr =
+				res->start & KEYSTONE_RPROC_LOCAL_ADDRESS_MASK;
+		ksproc->mem[i].size = resource_size(res);
+
+		/* zero out memories to start in a pristine state */
+		memset((__force void *)ksproc->mem[i].cpu_addr, 0,
+		       ksproc->mem[i].size);
+	}
+	ksproc->num_mems = num_mems;
+
+	return 0;
+}
+
+static int keystone_rproc_of_get_dev_syscon(struct platform_device *pdev,
+					    struct keystone_rproc *ksproc)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	if (!of_property_read_bool(np, "ti,syscon-dev")) {
+		dev_err(dev, "ti,syscon-dev property is absent\n");
+		return -EINVAL;
+	}
+
+	ksproc->dev_ctrl =
+		syscon_regmap_lookup_by_phandle(np, "ti,syscon-dev");
+	if (IS_ERR(ksproc->dev_ctrl)) {
+		ret = PTR_ERR(ksproc->dev_ctrl);
+		return ret;
+	}
+
+	if (of_property_read_u32_index(np, "ti,syscon-dev", 1,
+				       &ksproc->boot_offset)) {
+		dev_err(dev, "couldn't read the boot register offset\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int keystone_rproc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct keystone_rproc *ksproc;
+	struct rproc *rproc;
+	const char *name;
+	char *fw_name = NULL;
+	char *template = "keystone-%s-fw";
+	int name_len = 0;
+	int ret = 0;
+
+	if (!np) {
+		dev_err(dev, "only DT-based devices are supported\n");
+		return -ENODEV;
+	}
+
+	ret = of_property_read_string(np, "label", &name);
+	if (ret < 0) {
+		dev_err(dev, "failed to get label, status = %d\n", ret);
+		return ret;
+	}
+
+	/* construct a custom default fw name - subject to change in future */
+	name_len = strlen(name) + strlen(template) - 2 + 1;
+	fw_name = devm_kzalloc(dev, name_len, GFP_KERNEL);
+	if (!fw_name)
+		return -ENOMEM;
+	snprintf(fw_name, name_len, template, name);
+
+	rproc = rproc_alloc(dev, dev_name(dev), &keystone_rproc_ops, fw_name,
+			    sizeof(*ksproc));
+	if (!rproc)
+		return -ENOMEM;
+
+	rproc->has_iommu = false;
+	ksproc = rproc->priv;
+	ksproc->rproc = rproc;
+	ksproc->dev = dev;
+
+	ret = keystone_rproc_of_get_dev_syscon(pdev, ksproc);
+	if (ret)
+		goto free_rproc;
+
+	ksproc->reset = devm_reset_control_get(dev, NULL);
+	if (IS_ERR(ksproc->reset)) {
+		ret = PTR_ERR(ksproc->reset);
+		goto free_rproc;
+	}
+
+	/* enable clock for accessing DSP internal memories */
+	pm_runtime_enable(dev);
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		dev_err(dev, "failed to enable clock, status = %d\n", ret);
+		pm_runtime_put_noidle(dev);
+		goto disable_rpm;
+	}
+
+	ret = keystone_rproc_of_get_memories(pdev, ksproc);
+	if (ret)
+		goto disable_clk;
+
+	ksproc->irq_ring = platform_get_irq_byname(pdev, "vring");
+	if (ksproc->irq_ring < 0) {
+		ret = ksproc->irq_ring;
+		dev_err(dev, "failed to get vring interrupt, status = %d\n",
+			ret);
+		goto disable_clk;
+	}
+
+	ksproc->irq_fault = platform_get_irq_byname(pdev, "exception");
+	if (ksproc->irq_fault < 0) {
+		ret = ksproc->irq_fault;
+		dev_err(dev, "failed to get exception interrupt, status = %d\n",
+			ret);
+		goto disable_clk;
+	}
+
+	ksproc->kick_gpio = of_get_named_gpio_flags(np, "kick-gpio", 0, NULL);
+	if (ksproc->kick_gpio < 0) {
+		ret = ksproc->kick_gpio;
+		dev_err(dev, "failed to get gpio for virtio kicks, status = %d\n",
+			ret);
+		goto disable_clk;
+	}
+
+	if (of_reserved_mem_device_init(dev))
+		dev_warn(dev, "device does not have specific CMA pool\n");
+
+	ret = rproc_add(rproc);
+	if (ret) {
+		dev_err(dev, "failed to add register device with remoteproc core, status = %d\n",
+			ret);
+		goto release_mem;
+	}
+
+	platform_set_drvdata(pdev, ksproc);
+
+	return 0;
+
+release_mem:
+	of_reserved_mem_device_release(dev);
+disable_clk:
+	pm_runtime_put_sync(dev);
+disable_rpm:
+	pm_runtime_disable(dev);
+free_rproc:
+	rproc_free(rproc);
+	return ret;
+}
+
+static int keystone_rproc_remove(struct platform_device *pdev)
+{
+	struct keystone_rproc *ksproc = platform_get_drvdata(pdev);
+
+	rproc_del(ksproc->rproc);
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+	rproc_free(ksproc->rproc);
+	of_reserved_mem_device_release(&pdev->dev);
+
+	return 0;
+}
+
+static const struct of_device_id keystone_rproc_of_match[] = {
+	{ .compatible = "ti,k2hk-dsp", },
+	{ .compatible = "ti,k2l-dsp", },
+	{ .compatible = "ti,k2e-dsp", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, keystone_rproc_of_match);
+
+static struct platform_driver keystone_rproc_driver = {
+	.probe	= keystone_rproc_probe,
+	.remove	= keystone_rproc_remove,
+	.driver	= {
+		.name = "keystone-rproc",
+		.of_match_table = keystone_rproc_of_match,
+	},
+};
+
+module_platform_driver(keystone_rproc_driver);
+
+MODULE_AUTHOR("Suman Anna <s-anna@ti.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("TI Keystone DSP Remoteproc driver");
-- 
2.12.0

[PATCH 2/3] remoteproc/keystone: add a remoteproc driver for Keystone 2 DSPs

[Suman Anna]

The Texas Instrument's Keystone 2 family of SoCs has 1 or more
TMS320C66x DSP Core Subsystems (C66x CorePacs). Each subsystem has
a C66x Fixed/Floating-Point DSP Core, with 32KB of L1P and L1D SRAMs,
that can be configured and partitioned as either RAM and/or Cache,
and 1 MB of L2 SRAM. The CorePac also includes an Internal DMA (IDMA),
External Memory Controller (EMC), Extended Memory Controller (XMC)
with a Memory Protection and Address Extension (MPAX) unit, a Bandwidth
Management (BWM) unit, an Interrupt Controller (INTC) and a Powerdown
Controller (PDC).

A new remoteproc module is added to perform the device management of
these DSP devices. The driver expects the firmware names to be of the
form "keystone-dsp<X>-fw", where X is the corresponding DSP number, and
uses the standard remoteproc core ELF loader. The support is limited
to images only using the DSP internal memories at the moment. This
remoteproc driver is also designed to work with virtio, and uses the
IPC Generation registers for performing the virtio signalling and
getting notified of exceptions.

The driver currently supports the 66AK2H/66AK2K, 66AK2L and 66AK2E
SoCs.

Signed-off-by: Suman Anna <s-anna@ti.com>
Signed-off-by: Sam Nelson <sam.nelson@ti.com>
Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 drivers/remoteproc/Kconfig               |  13 +
 drivers/remoteproc/Makefile              |   1 +
 drivers/remoteproc/keystone_remoteproc.c | 515 +++++++++++++++++++++++++++++++
 3 files changed, 529 insertions(+)
 create mode 100644 drivers/remoteproc/keystone_remoteproc.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index faad69a1a597..9839b35b6ab3 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -71,6 +71,19 @@ config DA8XX_REMOTEPROC
 	  It's safe to say n here if you're not interested in multimedia
 	  offloading.
 
+config KEYSTONE_REMOTEPROC
+	tristate "Keystone Remoteproc support"
+	depends on ARCH_KEYSTONE
+	depends on RESET_CONTROLLER
+	depends on REMOTEPROC
+	select RPMSG_VIRTIO
+	help
+	  Say Y here here to support Keystone remote processors (DSP)
+	  via the remote processor framework.
+
+	  It's safe to say N here if you're not interested in the Keystone
+	  DSPs or just want to use a bare minimum kernel.
+
 config QCOM_ADSP_PIL
 	tristate "Qualcomm ADSP Peripheral Image Loader"
 	depends on OF && ARCH_QCOM
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index ffc5e430df27..f1ce5fc8a2f3 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -11,6 +11,7 @@ remoteproc-y				+= remoteproc_elf_loader.o
 obj-$(CONFIG_OMAP_REMOTEPROC)		+= omap_remoteproc.o
 obj-$(CONFIG_WKUP_M3_RPROC)		+= wkup_m3_rproc.o
 obj-$(CONFIG_DA8XX_REMOTEPROC)		+= da8xx_remoteproc.o
+obj-$(CONFIG_KEYSTONE_REMOTEPROC)	+= keystone_remoteproc.o
 obj-$(CONFIG_QCOM_ADSP_PIL)		+= qcom_adsp_pil.o
 obj-$(CONFIG_QCOM_RPROC_COMMON)		+= qcom_common.o
 obj-$(CONFIG_QCOM_Q6V5_PIL)		+= qcom_q6v5_pil.o
diff --git a/drivers/remoteproc/keystone_remoteproc.c b/drivers/remoteproc/keystone_remoteproc.c
new file mode 100644
index 000000000000..d01d5e3fce28
--- /dev/null
+++ b/drivers/remoteproc/keystone_remoteproc.c
@@ -0,0 +1,515 @@
+/*
+ * TI Keystone DSP remoteproc driver
+ *
+ * Copyright (C) 2015-2017 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/workqueue.h>
+#include <linux/of_address.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/of_gpio.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+#include <linux/remoteproc.h>
+#include <linux/reset.h>
+
+#include "remoteproc_internal.h"
+
+#define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK	(SZ_16M - 1)
+
+/**
+ * struct keystone_rproc_mem - internal memory structure
+ * @cpu_addr: MPU virtual address of the memory region
+ * @bus_addr: Bus address used to access the memory region
+ * @dev_addr: Device address of the memory region from DSP view
+ * @size: Size of the memory region
+ */
+struct keystone_rproc_mem {
+	void __iomem *cpu_addr;
+	phys_addr_t bus_addr;
+	u32 dev_addr;
+	size_t size;
+};
+
+/**
+ * struct keystone_rproc - keystone remote processor driver structure
+ * @dev: cached device pointer
+ * @rproc: remoteproc device handle
+ * @mem: internal memory regions data
+ * @num_mems: number of internal memory regions
+ * @dev_ctrl: device control regmap handle
+ * @reset: reset control handle
+ * @boot_offset: boot register offset in @dev_ctrl regmap
+ * @irq_ring: irq entry for vring
+ * @irq_fault: irq entry for exception
+ * @kick_gpio: gpio used for virtio kicks
+ * @workqueue: workqueue for processing virtio interrupts
+ */
+struct keystone_rproc {
+	struct device *dev;
+	struct rproc *rproc;
+	struct keystone_rproc_mem *mem;
+	int num_mems;
+	struct regmap *dev_ctrl;
+	struct reset_control *reset;
+	u32 boot_offset;
+	int irq_ring;
+	int irq_fault;
+	int kick_gpio;
+	struct work_struct workqueue;
+};
+
+/* Put the DSP processor into reset */
+static void keystone_rproc_dsp_reset(struct keystone_rproc *ksproc)
+{
+	reset_control_assert(ksproc->reset);
+}
+
+/* Configure the boot address and boot the DSP processor */
+static int keystone_rproc_dsp_boot(struct keystone_rproc *ksproc, u32 boot_addr)
+{
+	int ret;
+
+	if (boot_addr & (SZ_1K - 1)) {
+		dev_err(ksproc->dev, "invalid boot address 0x%x, must be aligned on a 1KB boundary\n",
+			boot_addr);
+		return -EINVAL;
+	}
+
+	ret = regmap_write(ksproc->dev_ctrl, ksproc->boot_offset, boot_addr);
+	if (ret) {
+		dev_err(ksproc->dev, "regmap_write of boot address failed, status = %d\n",
+			ret);
+		return ret;
+	}
+
+	reset_control_deassert(ksproc->reset);
+
+	return 0;
+}
+
+/*
+ * Process the remoteproc exceptions
+ *
+ * The exception reporting on Keystone DSP remote processors is very simple
+ * compared to the equivalent processors on the OMAP family, it is notified
+ * through a software-designed specific interrupt source in the IPC interrupt
+ * generation register.
+ *
+ * This function just invokes the rproc_report_crash to report the exception
+ * to the remoteproc driver core, to trigger a recovery.
+ */
+static irqreturn_t keystone_rproc_exception_interrupt(int irq, void *dev_id)
+{
+	struct keystone_rproc *ksproc = dev_id;
+
+	rproc_report_crash(ksproc->rproc, RPROC_FATAL_ERROR);
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * Main virtqueue message workqueue function
+ *
+ * This function is executed upon scheduling of the keystone remoteproc
+ * driver's workqueue. The workqueue is scheduled by the vring ISR handler.
+ *
+ * There is no payload message indicating the virtqueue index as is the
+ * case with mailbox-based implementations on OMAP family. As such, this
+ * handler processes both the Tx and Rx virtqueue indices on every invocation.
+ * The rproc_vq_interrupt function can detect if there are new unprocessed
+ * messages or not (returns IRQ_NONE vs IRQ_HANDLED), but there is no need
+ * to check for these return values. The index 0 triggering will process all
+ * pending Rx buffers, and the index 1 triggering will process all newly
+ * available Tx buffers and will wakeup any potentially blocked senders.
+ *
+ * NOTE:
+ * 1. A payload could be added by using some of the source bits in the
+ *    IPC interrupt generation registers, but this would need additional
+ *    changes to the overall IPC stack, and currently there are no benefits
+ *    of adapting that approach.
+ * 2. The current logic is based on an inherent design assumption of supporting
+ *    only 2 vrings, but this can be changed if needed.
+ */
+static void handle_event(struct work_struct *work)
+{
+	struct keystone_rproc *ksproc =
+		container_of(work, struct keystone_rproc, workqueue);
+
+	rproc_vq_interrupt(ksproc->rproc, 0);
+	rproc_vq_interrupt(ksproc->rproc, 1);
+}
+
+/*
+ * Interrupt handler for processing vring kicks from remote processor
+ */
+static irqreturn_t keystone_rproc_vring_interrupt(int irq, void *dev_id)
+{
+	struct keystone_rproc *ksproc = dev_id;
+
+	schedule_work(&ksproc->workqueue);
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * Power up the DSP remote processor.
+ *
+ * This function will be invoked only after the firmware for this rproc
+ * was loaded, parsed successfully, and all of its resource requirements
+ * were met.
+ */
+static int keystone_rproc_start(struct rproc *rproc)
+{
+	struct keystone_rproc *ksproc = rproc->priv;
+	int ret;
+
+	INIT_WORK(&ksproc->workqueue, handle_event);
+
+	ret = request_irq(ksproc->irq_ring, keystone_rproc_vring_interrupt, 0,
+			  dev_name(ksproc->dev), ksproc);
+	if (ret) {
+		dev_err(ksproc->dev, "failed to enable vring interrupt, ret = %d\n",
+			ret);
+		goto out;
+	}
+
+	ret = request_irq(ksproc->irq_fault, keystone_rproc_exception_interrupt,
+			  0, dev_name(ksproc->dev), ksproc);
+	if (ret) {
+		dev_err(ksproc->dev, "failed to enable exception interrupt, ret = %d\n",
+			ret);
+		goto free_vring_irq;
+	}
+
+	ret = keystone_rproc_dsp_boot(ksproc, rproc->bootaddr);
+	if (ret)
+		goto free_exc_irq;
+
+	return 0;
+
+free_exc_irq:
+	free_irq(ksproc->irq_fault, ksproc);
+free_vring_irq:
+	free_irq(ksproc->irq_ring, ksproc);
+	flush_work(&ksproc->workqueue);
+out:
+	return ret;
+}
+
+/*
+ * Stop the DSP remote processor.
+ *
+ * This function puts the DSP processor into reset, and finishes processing
+ * of any pending messages.
+ */
+static int keystone_rproc_stop(struct rproc *rproc)
+{
+	struct keystone_rproc *ksproc = rproc->priv;
+
+	keystone_rproc_dsp_reset(ksproc);
+	free_irq(ksproc->irq_fault, ksproc);
+	free_irq(ksproc->irq_ring, ksproc);
+	flush_work(&ksproc->workqueue);
+
+	return 0;
+}
+
+/*
+ * Kick the remote processor to notify about pending unprocessed messages.
+ * The vqid usage is not used and is inconsequential, as the kick is performed
+ * through a simulated GPIO (a bit in an IPC interrupt-triggering register),
+ * the remote processor is expected to process both its Tx and Rx virtqueues.
+ */
+static void keystone_rproc_kick(struct rproc *rproc, int vqid)
+{
+	struct keystone_rproc *ksproc = rproc->priv;
+
+	if (WARN_ON(ksproc->kick_gpio < 0))
+		return;
+
+	gpio_set_value(ksproc->kick_gpio, 1);
+}
+
+/*
+ * Custom function to translate a DSP device address (internal RAMs only) to a
+ * kernel virtual address.  The DSPs can access their RAMs at either an internal
+ * address visible only from a DSP, or at the SoC-level bus address. Both these
+ * addresses need to be looked through for translation. The translated addresses
+ * can be used either by the remoteproc core for loading (when using kernel
+ * remoteproc loader), or by any rpmsg bus drivers.
+ */
+static void *keystone_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
+{
+	struct keystone_rproc *ksproc = rproc->priv;
+	void __iomem *va = NULL;
+	phys_addr_t bus_addr;
+	u32 dev_addr, offset;
+	size_t size;
+	int i;
+
+	if (len <= 0)
+		return NULL;
+
+	for (i = 0; i < ksproc->num_mems; i++) {
+		bus_addr = ksproc->mem[i].bus_addr;
+		dev_addr = ksproc->mem[i].dev_addr;
+		size = ksproc->mem[i].size;
+
+		if (da < KEYSTONE_RPROC_LOCAL_ADDRESS_MASK) {
+			/* handle DSP-view addresses */
+			if ((da >= dev_addr) &&
+			    ((da + len) <= (dev_addr + size))) {
+				offset = da - dev_addr;
+				va = ksproc->mem[i].cpu_addr + offset;
+				break;
+			}
+		} else {
+			/* handle SoC-view addresses */
+			if ((da >= bus_addr) &&
+			    (da + len) <= (bus_addr + size)) {
+				offset = da - bus_addr;
+				va = ksproc->mem[i].cpu_addr + offset;
+				break;
+			}
+		}
+	}
+
+	return (__force void *)va;
+}
+
+static const struct rproc_ops keystone_rproc_ops = {
+	.start		= keystone_rproc_start,
+	.stop		= keystone_rproc_stop,
+	.kick		= keystone_rproc_kick,
+	.da_to_va	= keystone_rproc_da_to_va,
+};
+
+static int keystone_rproc_of_get_memories(struct platform_device *pdev,
+					  struct keystone_rproc *ksproc)
+{
+	static const char * const mem_names[] = {"l2sram", "l1pram", "l1dram"};
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	int num_mems = 0;
+	int i;
+
+	num_mems = ARRAY_SIZE(mem_names);
+	ksproc->mem = devm_kcalloc(ksproc->dev, num_mems,
+				   sizeof(*ksproc->mem), GFP_KERNEL);
+	if (!ksproc->mem)
+		return -ENOMEM;
+
+	for (i = 0; i < num_mems; i++) {
+		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+						   mem_names[i]);
+		ksproc->mem[i].cpu_addr = devm_ioremap_resource(dev, res);
+		if (IS_ERR(ksproc->mem[i].cpu_addr)) {
+			dev_err(dev, "failed to parse and map %s memory\n",
+				mem_names[i]);
+			return PTR_ERR(ksproc->mem[i].cpu_addr);
+		}
+		ksproc->mem[i].bus_addr = res->start;
+		ksproc->mem[i].dev_addr =
+				res->start & KEYSTONE_RPROC_LOCAL_ADDRESS_MASK;
+		ksproc->mem[i].size = resource_size(res);
+
+		/* zero out memories to start in a pristine state */
+		memset((__force void *)ksproc->mem[i].cpu_addr, 0,
+		       ksproc->mem[i].size);
+	}
+	ksproc->num_mems = num_mems;
+
+	return 0;
+}
+
+static int keystone_rproc_of_get_dev_syscon(struct platform_device *pdev,
+					    struct keystone_rproc *ksproc)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	if (!of_property_read_bool(np, "ti,syscon-dev")) {
+		dev_err(dev, "ti,syscon-dev property is absent\n");
+		return -EINVAL;
+	}
+
+	ksproc->dev_ctrl =
+		syscon_regmap_lookup_by_phandle(np, "ti,syscon-dev");
+	if (IS_ERR(ksproc->dev_ctrl)) {
+		ret = PTR_ERR(ksproc->dev_ctrl);
+		return ret;
+	}
+
+	if (of_property_read_u32_index(np, "ti,syscon-dev", 1,
+				       &ksproc->boot_offset)) {
+		dev_err(dev, "couldn't read the boot register offset\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int keystone_rproc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct keystone_rproc *ksproc;
+	struct rproc *rproc;
+	const char *name;
+	char *fw_name = NULL;
+	char *template = "keystone-%s-fw";
+	int name_len = 0;
+	int ret = 0;
+
+	if (!np) {
+		dev_err(dev, "only DT-based devices are supported\n");
+		return -ENODEV;
+	}
+
+	ret = of_property_read_string(np, "label", &name);
+	if (ret < 0) {
+		dev_err(dev, "failed to get label, status = %d\n", ret);
+		return ret;
+	}
+
+	/* construct a custom default fw name - subject to change in future */
+	name_len = strlen(name) + strlen(template) - 2 + 1;
+	fw_name = devm_kzalloc(dev, name_len, GFP_KERNEL);
+	if (!fw_name)
+		return -ENOMEM;
+	snprintf(fw_name, name_len, template, name);
+
+	rproc = rproc_alloc(dev, dev_name(dev), &keystone_rproc_ops, fw_name,
+			    sizeof(*ksproc));
+	if (!rproc)
+		return -ENOMEM;
+
+	rproc->has_iommu = false;
+	ksproc = rproc->priv;
+	ksproc->rproc = rproc;
+	ksproc->dev = dev;
+
+	ret = keystone_rproc_of_get_dev_syscon(pdev, ksproc);
+	if (ret)
+		goto free_rproc;
+
+	ksproc->reset = devm_reset_control_get(dev, NULL);
+	if (IS_ERR(ksproc->reset)) {
+		ret = PTR_ERR(ksproc->reset);
+		goto free_rproc;
+	}
+
+	/* enable clock for accessing DSP internal memories */
+	pm_runtime_enable(dev);
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		dev_err(dev, "failed to enable clock, status = %d\n", ret);
+		pm_runtime_put_noidle(dev);
+		goto disable_rpm;
+	}
+
+	ret = keystone_rproc_of_get_memories(pdev, ksproc);
+	if (ret)
+		goto disable_clk;
+
+	ksproc->irq_ring = platform_get_irq_byname(pdev, "vring");
+	if (ksproc->irq_ring < 0) {
+		ret = ksproc->irq_ring;
+		dev_err(dev, "failed to get vring interrupt, status = %d\n",
+			ret);
+		goto disable_clk;
+	}
+
+	ksproc->irq_fault = platform_get_irq_byname(pdev, "exception");
+	if (ksproc->irq_fault < 0) {
+		ret = ksproc->irq_fault;
+		dev_err(dev, "failed to get exception interrupt, status = %d\n",
+			ret);
+		goto disable_clk;
+	}
+
+	ksproc->kick_gpio = of_get_named_gpio_flags(np, "kick-gpio", 0, NULL);
+	if (ksproc->kick_gpio < 0) {
+		ret = ksproc->kick_gpio;
+		dev_err(dev, "failed to get gpio for virtio kicks, status = %d\n",
+			ret);
+		goto disable_clk;
+	}
+
+	if (of_reserved_mem_device_init(dev))
+		dev_warn(dev, "device does not have specific CMA pool\n");
+
+	ret = rproc_add(rproc);
+	if (ret) {
+		dev_err(dev, "failed to add register device with remoteproc core, status = %d\n",
+			ret);
+		goto release_mem;
+	}
+
+	platform_set_drvdata(pdev, ksproc);
+
+	return 0;
+
+release_mem:
+	of_reserved_mem_device_release(dev);
+disable_clk:
+	pm_runtime_put_sync(dev);
+disable_rpm:
+	pm_runtime_disable(dev);
+free_rproc:
+	rproc_free(rproc);
+	return ret;
+}
+
+static int keystone_rproc_remove(struct platform_device *pdev)
+{
+	struct keystone_rproc *ksproc = platform_get_drvdata(pdev);
+
+	rproc_del(ksproc->rproc);
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+	rproc_free(ksproc->rproc);
+	of_reserved_mem_device_release(&pdev->dev);
+
+	return 0;
+}
+
+static const struct of_device_id keystone_rproc_of_match[] = {
+	{ .compatible = "ti,k2hk-dsp", },
+	{ .compatible = "ti,k2l-dsp", },
+	{ .compatible = "ti,k2e-dsp", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, keystone_rproc_of_match);
+
+static struct platform_driver keystone_rproc_driver = {
+	.probe	= keystone_rproc_probe,
+	.remove	= keystone_rproc_remove,
+	.driver	= {
+		.name = "keystone-rproc",
+		.of_match_table = keystone_rproc_of_match,
+	},
+};
+
+module_platform_driver(keystone_rproc_driver);
+
+MODULE_AUTHOR("Suman Anna <s-anna@ti.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("TI Keystone DSP Remoteproc driver");
-- 
2.12.0

[PATCH 2/3] remoteproc/keystone: add a remoteproc driver for Keystone 2 DSPs

[Suman Anna]

The Texas Instrument's Keystone 2 family of SoCs has 1 or more
TMS320C66x DSP Core Subsystems (C66x CorePacs). Each subsystem has
a C66x Fixed/Floating-Point DSP Core, with 32KB of L1P and L1D SRAMs,
that can be configured and partitioned as either RAM and/or Cache,
and 1 MB of L2 SRAM. The CorePac also includes an Internal DMA (IDMA),
External Memory Controller (EMC), Extended Memory Controller (XMC)
with a Memory Protection and Address Extension (MPAX) unit, a Bandwidth
Management (BWM) unit, an Interrupt Controller (INTC) and a Powerdown
Controller (PDC).

A new remoteproc module is added to perform the device management of
these DSP devices. The driver expects the firmware names to be of the
form "keystone-dsp<X>-fw", where X is the corresponding DSP number, and
uses the standard remoteproc core ELF loader. The support is limited
to images only using the DSP internal memories at the moment. This
remoteproc driver is also designed to work with virtio, and uses the
IPC Generation registers for performing the virtio signalling and
getting notified of exceptions.

The driver currently supports the 66AK2H/66AK2K, 66AK2L and 66AK2E
SoCs.

Signed-off-by: Suman Anna <s-anna@ti.com>
Signed-off-by: Sam Nelson <sam.nelson@ti.com>
Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 drivers/remoteproc/Kconfig               |  13 +
 drivers/remoteproc/Makefile              |   1 +
 drivers/remoteproc/keystone_remoteproc.c | 515 +++++++++++++++++++++++++++++++
 3 files changed, 529 insertions(+)
 create mode 100644 drivers/remoteproc/keystone_remoteproc.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index faad69a1a597..9839b35b6ab3 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -71,6 +71,19 @@ config DA8XX_REMOTEPROC
 	  It's safe to say n here if you're not interested in multimedia
 	  offloading.
 
+config KEYSTONE_REMOTEPROC
+	tristate "Keystone Remoteproc support"
+	depends on ARCH_KEYSTONE
+	depends on RESET_CONTROLLER
+	depends on REMOTEPROC
+	select RPMSG_VIRTIO
+	help
+	  Say Y here here to support Keystone remote processors (DSP)
+	  via the remote processor framework.
+
+	  It's safe to say N here if you're not interested in the Keystone
+	  DSPs or just want to use a bare minimum kernel.
+
 config QCOM_ADSP_PIL
 	tristate "Qualcomm ADSP Peripheral Image Loader"
 	depends on OF && ARCH_QCOM
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index ffc5e430df27..f1ce5fc8a2f3 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -11,6 +11,7 @@ remoteproc-y				+= remoteproc_elf_loader.o
 obj-$(CONFIG_OMAP_REMOTEPROC)		+= omap_remoteproc.o
 obj-$(CONFIG_WKUP_M3_RPROC)		+= wkup_m3_rproc.o
 obj-$(CONFIG_DA8XX_REMOTEPROC)		+= da8xx_remoteproc.o
+obj-$(CONFIG_KEYSTONE_REMOTEPROC)	+= keystone_remoteproc.o
 obj-$(CONFIG_QCOM_ADSP_PIL)		+= qcom_adsp_pil.o
 obj-$(CONFIG_QCOM_RPROC_COMMON)		+= qcom_common.o
 obj-$(CONFIG_QCOM_Q6V5_PIL)		+= qcom_q6v5_pil.o
diff --git a/drivers/remoteproc/keystone_remoteproc.c b/drivers/remoteproc/keystone_remoteproc.c
new file mode 100644
index 000000000000..d01d5e3fce28
--- /dev/null
+++ b/drivers/remoteproc/keystone_remoteproc.c
@@ -0,0 +1,515 @@
+/*
+ * TI Keystone DSP remoteproc driver
+ *
+ * Copyright (C) 2015-2017 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/workqueue.h>
+#include <linux/of_address.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/of_gpio.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+#include <linux/remoteproc.h>
+#include <linux/reset.h>
+
+#include "remoteproc_internal.h"
+
+#define KEYSTONE_RPROC_LOCAL_ADDRESS_MASK	(SZ_16M - 1)
+
+/**
+ * struct keystone_rproc_mem - internal memory structure
+ * @cpu_addr: MPU virtual address of the memory region
+ * @bus_addr: Bus address used to access the memory region
+ * @dev_addr: Device address of the memory region from DSP view
+ * @size: Size of the memory region
+ */
+struct keystone_rproc_mem {
+	void __iomem *cpu_addr;
+	phys_addr_t bus_addr;
+	u32 dev_addr;
+	size_t size;
+};
+
+/**
+ * struct keystone_rproc - keystone remote processor driver structure
+ * @dev: cached device pointer
+ * @rproc: remoteproc device handle
+ * @mem: internal memory regions data
+ * @num_mems: number of internal memory regions
+ * @dev_ctrl: device control regmap handle
+ * @reset: reset control handle
+ * @boot_offset: boot register offset in @dev_ctrl regmap
+ * @irq_ring: irq entry for vring
+ * @irq_fault: irq entry for exception
+ * @kick_gpio: gpio used for virtio kicks
+ * @workqueue: workqueue for processing virtio interrupts
+ */
+struct keystone_rproc {
+	struct device *dev;
+	struct rproc *rproc;
+	struct keystone_rproc_mem *mem;
+	int num_mems;
+	struct regmap *dev_ctrl;
+	struct reset_control *reset;
+	u32 boot_offset;
+	int irq_ring;
+	int irq_fault;
+	int kick_gpio;
+	struct work_struct workqueue;
+};
+
+/* Put the DSP processor into reset */
+static void keystone_rproc_dsp_reset(struct keystone_rproc *ksproc)
+{
+	reset_control_assert(ksproc->reset);
+}
+
+/* Configure the boot address and boot the DSP processor */
+static int keystone_rproc_dsp_boot(struct keystone_rproc *ksproc, u32 boot_addr)
+{
+	int ret;
+
+	if (boot_addr & (SZ_1K - 1)) {
+		dev_err(ksproc->dev, "invalid boot address 0x%x, must be aligned on a 1KB boundary\n",
+			boot_addr);
+		return -EINVAL;
+	}
+
+	ret = regmap_write(ksproc->dev_ctrl, ksproc->boot_offset, boot_addr);
+	if (ret) {
+		dev_err(ksproc->dev, "regmap_write of boot address failed, status = %d\n",
+			ret);
+		return ret;
+	}
+
+	reset_control_deassert(ksproc->reset);
+
+	return 0;
+}
+
+/*
+ * Process the remoteproc exceptions
+ *
+ * The exception reporting on Keystone DSP remote processors is very simple
+ * compared to the equivalent processors on the OMAP family, it is notified
+ * through a software-designed specific interrupt source in the IPC interrupt
+ * generation register.
+ *
+ * This function just invokes the rproc_report_crash to report the exception
+ * to the remoteproc driver core, to trigger a recovery.
+ */
+static irqreturn_t keystone_rproc_exception_interrupt(int irq, void *dev_id)
+{
+	struct keystone_rproc *ksproc = dev_id;
+
+	rproc_report_crash(ksproc->rproc, RPROC_FATAL_ERROR);
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * Main virtqueue message workqueue function
+ *
+ * This function is executed upon scheduling of the keystone remoteproc
+ * driver's workqueue. The workqueue is scheduled by the vring ISR handler.
+ *
+ * There is no payload message indicating the virtqueue index as is the
+ * case with mailbox-based implementations on OMAP family. As such, this
+ * handler processes both the Tx and Rx virtqueue indices on every invocation.
+ * The rproc_vq_interrupt function can detect if there are new unprocessed
+ * messages or not (returns IRQ_NONE vs IRQ_HANDLED), but there is no need
+ * to check for these return values. The index 0 triggering will process all
+ * pending Rx buffers, and the index 1 triggering will process all newly
+ * available Tx buffers and will wakeup any potentially blocked senders.
+ *
+ * NOTE:
+ * 1. A payload could be added by using some of the source bits in the
+ *    IPC interrupt generation registers, but this would need additional
+ *    changes to the overall IPC stack, and currently there are no benefits
+ *    of adapting that approach.
+ * 2. The current logic is based on an inherent design assumption of supporting
+ *    only 2 vrings, but this can be changed if needed.
+ */
+static void handle_event(struct work_struct *work)
+{
+	struct keystone_rproc *ksproc =
+		container_of(work, struct keystone_rproc, workqueue);
+
+	rproc_vq_interrupt(ksproc->rproc, 0);
+	rproc_vq_interrupt(ksproc->rproc, 1);
+}
+
+/*
+ * Interrupt handler for processing vring kicks from remote processor
+ */
+static irqreturn_t keystone_rproc_vring_interrupt(int irq, void *dev_id)
+{
+	struct keystone_rproc *ksproc = dev_id;
+
+	schedule_work(&ksproc->workqueue);
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * Power up the DSP remote processor.
+ *
+ * This function will be invoked only after the firmware for this rproc
+ * was loaded, parsed successfully, and all of its resource requirements
+ * were met.
+ */
+static int keystone_rproc_start(struct rproc *rproc)
+{
+	struct keystone_rproc *ksproc = rproc->priv;
+	int ret;
+
+	INIT_WORK(&ksproc->workqueue, handle_event);
+
+	ret = request_irq(ksproc->irq_ring, keystone_rproc_vring_interrupt, 0,
+			  dev_name(ksproc->dev), ksproc);
+	if (ret) {
+		dev_err(ksproc->dev, "failed to enable vring interrupt, ret = %d\n",
+			ret);
+		goto out;
+	}
+
+	ret = request_irq(ksproc->irq_fault, keystone_rproc_exception_interrupt,
+			  0, dev_name(ksproc->dev), ksproc);
+	if (ret) {
+		dev_err(ksproc->dev, "failed to enable exception interrupt, ret = %d\n",
+			ret);
+		goto free_vring_irq;
+	}
+
+	ret = keystone_rproc_dsp_boot(ksproc, rproc->bootaddr);
+	if (ret)
+		goto free_exc_irq;
+
+	return 0;
+
+free_exc_irq:
+	free_irq(ksproc->irq_fault, ksproc);
+free_vring_irq:
+	free_irq(ksproc->irq_ring, ksproc);
+	flush_work(&ksproc->workqueue);
+out:
+	return ret;
+}
+
+/*
+ * Stop the DSP remote processor.
+ *
+ * This function puts the DSP processor into reset, and finishes processing
+ * of any pending messages.
+ */
+static int keystone_rproc_stop(struct rproc *rproc)
+{
+	struct keystone_rproc *ksproc = rproc->priv;
+
+	keystone_rproc_dsp_reset(ksproc);
+	free_irq(ksproc->irq_fault, ksproc);
+	free_irq(ksproc->irq_ring, ksproc);
+	flush_work(&ksproc->workqueue);
+
+	return 0;
+}
+
+/*
+ * Kick the remote processor to notify about pending unprocessed messages.
+ * The vqid usage is not used and is inconsequential, as the kick is performed
+ * through a simulated GPIO (a bit in an IPC interrupt-triggering register),
+ * the remote processor is expected to process both its Tx and Rx virtqueues.
+ */
+static void keystone_rproc_kick(struct rproc *rproc, int vqid)
+{
+	struct keystone_rproc *ksproc = rproc->priv;
+
+	if (WARN_ON(ksproc->kick_gpio < 0))
+		return;
+
+	gpio_set_value(ksproc->kick_gpio, 1);
+}
+
+/*
+ * Custom function to translate a DSP device address (internal RAMs only) to a
+ * kernel virtual address.  The DSPs can access their RAMs at either an internal
+ * address visible only from a DSP, or at the SoC-level bus address. Both these
+ * addresses need to be looked through for translation. The translated addresses
+ * can be used either by the remoteproc core for loading (when using kernel
+ * remoteproc loader), or by any rpmsg bus drivers.
+ */
+static void *keystone_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
+{
+	struct keystone_rproc *ksproc = rproc->priv;
+	void __iomem *va = NULL;
+	phys_addr_t bus_addr;
+	u32 dev_addr, offset;
+	size_t size;
+	int i;
+
+	if (len <= 0)
+		return NULL;
+
+	for (i = 0; i < ksproc->num_mems; i++) {
+		bus_addr = ksproc->mem[i].bus_addr;
+		dev_addr = ksproc->mem[i].dev_addr;
+		size = ksproc->mem[i].size;
+
+		if (da < KEYSTONE_RPROC_LOCAL_ADDRESS_MASK) {
+			/* handle DSP-view addresses */
+			if ((da >= dev_addr) &&
+			    ((da + len) <= (dev_addr + size))) {
+				offset = da - dev_addr;
+				va = ksproc->mem[i].cpu_addr + offset;
+				break;
+			}
+		} else {
+			/* handle SoC-view addresses */
+			if ((da >= bus_addr) &&
+			    (da + len) <= (bus_addr + size)) {
+				offset = da - bus_addr;
+				va = ksproc->mem[i].cpu_addr + offset;
+				break;
+			}
+		}
+	}
+
+	return (__force void *)va;
+}
+
+static const struct rproc_ops keystone_rproc_ops = {
+	.start		= keystone_rproc_start,
+	.stop		= keystone_rproc_stop,
+	.kick		= keystone_rproc_kick,
+	.da_to_va	= keystone_rproc_da_to_va,
+};
+
+static int keystone_rproc_of_get_memories(struct platform_device *pdev,
+					  struct keystone_rproc *ksproc)
+{
+	static const char * const mem_names[] = {"l2sram", "l1pram", "l1dram"};
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	int num_mems = 0;
+	int i;
+
+	num_mems = ARRAY_SIZE(mem_names);
+	ksproc->mem = devm_kcalloc(ksproc->dev, num_mems,
+				   sizeof(*ksproc->mem), GFP_KERNEL);
+	if (!ksproc->mem)
+		return -ENOMEM;
+
+	for (i = 0; i < num_mems; i++) {
+		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+						   mem_names[i]);
+		ksproc->mem[i].cpu_addr = devm_ioremap_resource(dev, res);
+		if (IS_ERR(ksproc->mem[i].cpu_addr)) {
+			dev_err(dev, "failed to parse and map %s memory\n",
+				mem_names[i]);
+			return PTR_ERR(ksproc->mem[i].cpu_addr);
+		}
+		ksproc->mem[i].bus_addr = res->start;
+		ksproc->mem[i].dev_addr =
+				res->start & KEYSTONE_RPROC_LOCAL_ADDRESS_MASK;
+		ksproc->mem[i].size = resource_size(res);
+
+		/* zero out memories to start in a pristine state */
+		memset((__force void *)ksproc->mem[i].cpu_addr, 0,
+		       ksproc->mem[i].size);
+	}
+	ksproc->num_mems = num_mems;
+
+	return 0;
+}
+
+static int keystone_rproc_of_get_dev_syscon(struct platform_device *pdev,
+					    struct keystone_rproc *ksproc)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	if (!of_property_read_bool(np, "ti,syscon-dev")) {
+		dev_err(dev, "ti,syscon-dev property is absent\n");
+		return -EINVAL;
+	}
+
+	ksproc->dev_ctrl =
+		syscon_regmap_lookup_by_phandle(np, "ti,syscon-dev");
+	if (IS_ERR(ksproc->dev_ctrl)) {
+		ret = PTR_ERR(ksproc->dev_ctrl);
+		return ret;
+	}
+
+	if (of_property_read_u32_index(np, "ti,syscon-dev", 1,
+				       &ksproc->boot_offset)) {
+		dev_err(dev, "couldn't read the boot register offset\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int keystone_rproc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct keystone_rproc *ksproc;
+	struct rproc *rproc;
+	const char *name;
+	char *fw_name = NULL;
+	char *template = "keystone-%s-fw";
+	int name_len = 0;
+	int ret = 0;
+
+	if (!np) {
+		dev_err(dev, "only DT-based devices are supported\n");
+		return -ENODEV;
+	}
+
+	ret = of_property_read_string(np, "label", &name);
+	if (ret < 0) {
+		dev_err(dev, "failed to get label, status = %d\n", ret);
+		return ret;
+	}
+
+	/* construct a custom default fw name - subject to change in future */
+	name_len = strlen(name) + strlen(template) - 2 + 1;
+	fw_name = devm_kzalloc(dev, name_len, GFP_KERNEL);
+	if (!fw_name)
+		return -ENOMEM;
+	snprintf(fw_name, name_len, template, name);
+
+	rproc = rproc_alloc(dev, dev_name(dev), &keystone_rproc_ops, fw_name,
+			    sizeof(*ksproc));
+	if (!rproc)
+		return -ENOMEM;
+
+	rproc->has_iommu = false;
+	ksproc = rproc->priv;
+	ksproc->rproc = rproc;
+	ksproc->dev = dev;
+
+	ret = keystone_rproc_of_get_dev_syscon(pdev, ksproc);
+	if (ret)
+		goto free_rproc;
+
+	ksproc->reset = devm_reset_control_get(dev, NULL);
+	if (IS_ERR(ksproc->reset)) {
+		ret = PTR_ERR(ksproc->reset);
+		goto free_rproc;
+	}
+
+	/* enable clock for accessing DSP internal memories */
+	pm_runtime_enable(dev);
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		dev_err(dev, "failed to enable clock, status = %d\n", ret);
+		pm_runtime_put_noidle(dev);
+		goto disable_rpm;
+	}
+
+	ret = keystone_rproc_of_get_memories(pdev, ksproc);
+	if (ret)
+		goto disable_clk;
+
+	ksproc->irq_ring = platform_get_irq_byname(pdev, "vring");
+	if (ksproc->irq_ring < 0) {
+		ret = ksproc->irq_ring;
+		dev_err(dev, "failed to get vring interrupt, status = %d\n",
+			ret);
+		goto disable_clk;
+	}
+
+	ksproc->irq_fault = platform_get_irq_byname(pdev, "exception");
+	if (ksproc->irq_fault < 0) {
+		ret = ksproc->irq_fault;
+		dev_err(dev, "failed to get exception interrupt, status = %d\n",
+			ret);
+		goto disable_clk;
+	}
+
+	ksproc->kick_gpio = of_get_named_gpio_flags(np, "kick-gpio", 0, NULL);
+	if (ksproc->kick_gpio < 0) {
+		ret = ksproc->kick_gpio;
+		dev_err(dev, "failed to get gpio for virtio kicks, status = %d\n",
+			ret);
+		goto disable_clk;
+	}
+
+	if (of_reserved_mem_device_init(dev))
+		dev_warn(dev, "device does not have specific CMA pool\n");
+
+	ret = rproc_add(rproc);
+	if (ret) {
+		dev_err(dev, "failed to add register device with remoteproc core, status = %d\n",
+			ret);
+		goto release_mem;
+	}
+
+	platform_set_drvdata(pdev, ksproc);
+
+	return 0;
+
+release_mem:
+	of_reserved_mem_device_release(dev);
+disable_clk:
+	pm_runtime_put_sync(dev);
+disable_rpm:
+	pm_runtime_disable(dev);
+free_rproc:
+	rproc_free(rproc);
+	return ret;
+}
+
+static int keystone_rproc_remove(struct platform_device *pdev)
+{
+	struct keystone_rproc *ksproc = platform_get_drvdata(pdev);
+
+	rproc_del(ksproc->rproc);
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+	rproc_free(ksproc->rproc);
+	of_reserved_mem_device_release(&pdev->dev);
+
+	return 0;
+}
+
+static const struct of_device_id keystone_rproc_of_match[] = {
+	{ .compatible = "ti,k2hk-dsp", },
+	{ .compatible = "ti,k2l-dsp", },
+	{ .compatible = "ti,k2e-dsp", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, keystone_rproc_of_match);
+
+static struct platform_driver keystone_rproc_driver = {
+	.probe	= keystone_rproc_probe,
+	.remove	= keystone_rproc_remove,
+	.driver	= {
+		.name = "keystone-rproc",
+		.of_match_table = keystone_rproc_of_match,
+	},
+};
+
+module_platform_driver(keystone_rproc_driver);
+
+MODULE_AUTHOR("Suman Anna <s-anna@ti.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("TI Keystone DSP Remoteproc driver");
-- 
2.12.0

[PATCH 3/3] remoteproc/keystone: ensure the DSPs are in reset in probe

[Suman Anna]

From: "Andrew F. Davis" <afd@ti.com>

The DSPs are expected to be in reset when the driver probes a device.
If the DSPs are out of reset in probe, the system may crash when the
firmware is being loaded. So, add a check to make sure the DSP resets
are asserted, and if not, throw a eye-catchy warning and assert the
resets specifically.

Signed-off-by: Andrew F. Davis <afd@ti.com>
[s-anna@ti.com: replace warning with a WARN]
Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/remoteproc/keystone_remoteproc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/remoteproc/keystone_remoteproc.c b/drivers/remoteproc/keystone_remoteproc.c
index d01d5e3fce28..8233664e85fa 100644
--- a/drivers/remoteproc/keystone_remoteproc.c
+++ b/drivers/remoteproc/keystone_remoteproc.c
@@ -456,6 +456,16 @@ static int keystone_rproc_probe(struct platform_device *pdev)
 	if (of_reserved_mem_device_init(dev))
 		dev_warn(dev, "device does not have specific CMA pool\n");
 
+	/* ensure the DSP is in reset before loading firmware */
+	ret = reset_control_status(ksproc->reset);
+	if (ret < 0) {
+		dev_err(dev, "failed to get reset status, status = %d\n", ret);
+		goto release_mem;
+	} else if (ret == 0) {
+		WARN(1, "device is not in reset\n");
+		keystone_rproc_dsp_reset(ksproc);
+	}
+
 	ret = rproc_add(rproc);
 	if (ret) {
 		dev_err(dev, "failed to add register device with remoteproc core, status = %d\n",
-- 
2.12.0

[PATCH 3/3] remoteproc/keystone: ensure the DSPs are in reset in probe

[Suman Anna]

From: "Andrew F. Davis" <afd@ti.com>

The DSPs are expected to be in reset when the driver probes a device.
If the DSPs are out of reset in probe, the system may crash when the
firmware is being loaded. So, add a check to make sure the DSP resets
are asserted, and if not, throw a eye-catchy warning and assert the
resets specifically.

Signed-off-by: Andrew F. Davis <afd@ti.com>
[s-anna@ti.com: replace warning with a WARN]
Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/remoteproc/keystone_remoteproc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/remoteproc/keystone_remoteproc.c b/drivers/remoteproc/keystone_remoteproc.c
index d01d5e3fce28..8233664e85fa 100644
--- a/drivers/remoteproc/keystone_remoteproc.c
+++ b/drivers/remoteproc/keystone_remoteproc.c
@@ -456,6 +456,16 @@ static int keystone_rproc_probe(struct platform_device *pdev)
 	if (of_reserved_mem_device_init(dev))
 		dev_warn(dev, "device does not have specific CMA pool\n");
 
+	/* ensure the DSP is in reset before loading firmware */
+	ret = reset_control_status(ksproc->reset);
+	if (ret < 0) {
+		dev_err(dev, "failed to get reset status, status = %d\n", ret);
+		goto release_mem;
+	} else if (ret == 0) {
+		WARN(1, "device is not in reset\n");
+		keystone_rproc_dsp_reset(ksproc);
+	}
+
 	ret = rproc_add(rproc);
 	if (ret) {
 		dev_err(dev, "failed to add register device with remoteproc core, status = %d\n",
-- 
2.12.0

[PATCH 3/3] remoteproc/keystone: ensure the DSPs are in reset in probe

[Suman Anna]

From: "Andrew F. Davis" <afd@ti.com>

The DSPs are expected to be in reset when the driver probes a device.
If the DSPs are out of reset in probe, the system may crash when the
firmware is being loaded. So, add a check to make sure the DSP resets
are asserted, and if not, throw a eye-catchy warning and assert the
resets specifically.

Signed-off-by: Andrew F. Davis <afd@ti.com>
[s-anna@ti.com: replace warning with a WARN]
Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/remoteproc/keystone_remoteproc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/remoteproc/keystone_remoteproc.c b/drivers/remoteproc/keystone_remoteproc.c
index d01d5e3fce28..8233664e85fa 100644
--- a/drivers/remoteproc/keystone_remoteproc.c
+++ b/drivers/remoteproc/keystone_remoteproc.c
@@ -456,6 +456,16 @@ static int keystone_rproc_probe(struct platform_device *pdev)
 	if (of_reserved_mem_device_init(dev))
 		dev_warn(dev, "device does not have specific CMA pool\n");
 
+	/* ensure the DSP is in reset before loading firmware */
+	ret = reset_control_status(ksproc->reset);
+	if (ret < 0) {
+		dev_err(dev, "failed to get reset status, status = %d\n", ret);
+		goto release_mem;
+	} else if (ret == 0) {
+		WARN(1, "device is not in reset\n");
+		keystone_rproc_dsp_reset(ksproc);
+	}
+
 	ret = rproc_add(rproc);
 	if (ret) {
 		dev_err(dev, "failed to add register device with remoteproc core, status = %d\n",
-- 
2.12.0

[PATCH 3/3] remoteproc/keystone: ensure the DSPs are in reset in probe

[Suman Anna]

From: "Andrew F. Davis" <afd@ti.com>

The DSPs are expected to be in reset when the driver probes a device.
If the DSPs are out of reset in probe, the system may crash when the
firmware is being loaded. So, add a check to make sure the DSP resets
are asserted, and if not, throw a eye-catchy warning and assert the
resets specifically.

Signed-off-by: Andrew F. Davis <afd@ti.com>
[s-anna at ti.com: replace warning with a WARN]
Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/remoteproc/keystone_remoteproc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/remoteproc/keystone_remoteproc.c b/drivers/remoteproc/keystone_remoteproc.c
index d01d5e3fce28..8233664e85fa 100644
--- a/drivers/remoteproc/keystone_remoteproc.c
+++ b/drivers/remoteproc/keystone_remoteproc.c
@@ -456,6 +456,16 @@ static int keystone_rproc_probe(struct platform_device *pdev)
 	if (of_reserved_mem_device_init(dev))
 		dev_warn(dev, "device does not have specific CMA pool\n");
 
+	/* ensure the DSP is in reset before loading firmware */
+	ret = reset_control_status(ksproc->reset);
+	if (ret < 0) {
+		dev_err(dev, "failed to get reset status, status = %d\n", ret);
+		goto release_mem;
+	} else if (ret == 0) {
+		WARN(1, "device is not in reset\n");
+		keystone_rproc_dsp_reset(ksproc);
+	}
+
 	ret = rproc_add(rproc);
 	if (ret) {
 		dev_err(dev, "failed to add register device with remoteproc core, status = %d\n",
-- 
2.12.0

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

[Rob Herring]

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
> 

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

[Rob Herring]

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 at 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 at 81f800000 {
> +			compatible = "shared-dma-pool";
> +			reg = <0x00000008 0x1f800000 0x00000000 0x800000>;
> +			reusable;
> +			status = "okay";
> +		};
> +	};
> +
> +	&dsp0 {
> +		memory-region = <&dsp_common_cma_pool>;
> +	};
> -- 
> 2.12.0
> 

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

[Suman Anna]

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

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

[Suman Anna]

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

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

[Suman Anna]

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

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

[Suman Anna]

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 at 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 at 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

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

[Rob Herring]

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

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

[Rob Herring]

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

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

[Suman Anna]

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

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

[Suman Anna]

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

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

[Suman Anna]

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

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

[Suman Anna]

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

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

[Rob Herring]

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

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

[Rob Herring]

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

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

[Rob Herring]

On Mon, Jun 5, 2017 at 1:21 PM, Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> 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-l0cyMroinI0@public.gmane.org>
>>>>> Signed-off-by: Sam Nelson <sam.nelson-l0cyMroinI0@public.gmane.org>
>>>>> ---
>>>>>  .../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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Rob Herring]

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

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

[Suman Anna]

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

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

[Suman Anna]

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

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

[Suman Anna]

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-l0cyMroinI0@public.gmane.org> 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-l0cyMroinI0@public.gmane.org>
>>>>>> Signed-off-by: Sam Nelson <sam.nelson-l0cyMroinI0@public.gmane.org>
>>>>>> ---
>>>>>>  .../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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Suman Anna]

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