Merge tag 'rproc-v4.13' of git://github.com/andersson/remoteproc

Pull remoteproc updates from Bjorn Andersson:
 "This introduces the Keystone 2 DSP driver and refactors the start/stop
  code in recovery. The Davinci DSP driver gets a few fixes and the
  Kconfig gets cleaned up"

* tag 'rproc-v4.13' of git://github.com/andersson/remoteproc:
  remoteproc/keystone: Fix circular dependencies for ARM configs
  remoteproc: Drop redundant REMOTEPROC dependency from driver Kconfigs
  remoteproc: Drop VIRTUALIZATION dependency from REMOTEPROC
  remoteproc/keystone: Ensure the DSPs are in reset in probe
  remoteproc/keystone: Add a remoteproc driver for Keystone 2 DSPs
  dt-bindings: remoteproc: Add Keystone DSP remoteproc binding
  remoteproc/davinci: fix unbalanced reset between start and stop ops
  remoteproc/davinci: simplify the reset function
  remoteproc/davinci: Update Kconfig to depend on DMA_CMA
  remoteproc: fix spelling mistake: "Resouce" -> "Resource"
  remoteproc: Modify recovery path to use rproc_{start,stop}()
  remoteproc: Introduce rproc_{start,stop}() functions

Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt

+TI Keystone DSP devices
+=======================
+
+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, and should also
+have an alias with the stem 'rproc' defined. 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"
+
+- 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-gpios: 		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 aliases */
+	aliases {
+		rproc0 = &dsp0;
+		rproc1 = &dsp1;
+		rproc2 = &dsp2;
+		rproc3 = &dsp3;
+		rproc4 = &dsp4;
+		rproc5 = &dsp5;
+		rproc6 = &dsp6;
+		rproc7 = &dsp7;
+	};
+
+	/* 66AK2H/K DSP memory node */
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		dsp_common_memory: dsp-common-memory@81f800000 {
+			compatible = "shared-dma-pool";
+			reg = <0x00000008 0x1f800000 0x00000000 0x800000>;
+			reusable;
+		};
+	};
+
+	/* 66AK2H/K DSP node */
+	soc {
+		dsp0: dsp@10800000 {
+			compatible = "ti,k2hk-dsp";
+			reg = <0x10800000 0x00100000>,
+			      <0x10e00000 0x00008000>,
+			      <0x10f00000 0x00008000>;
+			reg-names = "l2sram", "l1pram", "l1dram";
+			clocks = <&clkgem0>;
+			ti,syscon-dev = <&devctrl 0x40>;
+			resets = <&pscrst 0>;
+			interrupt-parent = <&kirq0>;
+			interrupts = <0 8>;
+			interrupt-names = "vring", "exception";
+			kick-gpios = <&dspgpio0 27 0>;
+			memory-region = <&dsp_common_memory>;
+		};
+
+	};

drivers/remoteproc/Kconfig

@@ -6,7 +6,6 @@ config REMOTEPROC
 	select CRC32
 	select FW_LOADER
 	select VIRTIO
-	select VIRTUALIZATION
 	help
 	  Support for remote processors (such as DSP coprocessors). These
 	  are mainly used on embedded systems.
@@ -18,7 +17,6 @@ config OMAP_REMOTEPROC
 	depends on HAS_DMA
 	depends on ARCH_OMAP4 || SOC_OMAP5
 	depends on OMAP_IOMMU
-	depends on REMOTEPROC
 	select MAILBOX
 	select OMAP2PLUS_MBOX
 	select RPMSG_VIRTIO
@@ -38,7 +36,6 @@ config OMAP_REMOTEPROC
 config WKUP_M3_RPROC
 	tristate "AMx3xx Wakeup M3 remoteproc support"
 	depends on SOC_AM33XX || SOC_AM43XX
-	depends on REMOTEPROC
 	help
 	  Say y here to support Wakeup M3 remote processor on TI AM33xx
 	  and AM43xx family of SoCs.
@@ -51,8 +48,7 @@ config WKUP_M3_RPROC
 config DA8XX_REMOTEPROC
 	tristate "DA8xx/OMAP-L13x remoteproc support"
 	depends on ARCH_DAVINCI_DA8XX
-	depends on REMOTEPROC
-	select CMA if MMU
+	depends on DMA_CMA
 	select RPMSG_VIRTIO
 	help
 	  Say y here to support DA8xx/OMAP-L13x remote processors via the
@@ -71,10 +67,20 @@ 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
+	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
-	depends on REMOTEPROC
 	depends on QCOM_SMEM
 	depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
 	select MFD_SYSCON
@@ -92,7 +98,6 @@ config QCOM_Q6V5_PIL
 	tristate "Qualcomm Hexagon V5 Peripherial Image Loader"
 	depends on OF && ARCH_QCOM
 	depends on QCOM_SMEM
-	depends on REMOTEPROC
 	depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
 	select MFD_SYSCON
 	select QCOM_RPROC_COMMON
@@ -106,7 +111,6 @@ config QCOM_WCNSS_PIL
 	depends on OF && ARCH_QCOM
 	depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
 	depends on QCOM_SMEM
-	depends on REMOTEPROC
 	select QCOM_MDT_LOADER
 	select QCOM_RPROC_COMMON
 	select QCOM_SCM
@@ -117,7 +121,6 @@ config QCOM_WCNSS_PIL
 config ST_REMOTEPROC
 	tristate "ST remoteproc support"
 	depends on ARCH_STI
-	depends on REMOTEPROC
 	select MAILBOX
 	select STI_MBOX
 	select RPMSG_VIRTIO
@@ -128,7 +131,6 @@ config ST_REMOTEPROC
 
 config ST_SLIM_REMOTEPROC
 	tristate
-	depends on REMOTEPROC
 
 endif # REMOTEPROC
 

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

drivers/remoteproc/da8xx_remoteproc.c

@@ -137,6 +137,7 @@ static int da8xx_rproc_stop(struct rproc *rproc)
 {
 	struct da8xx_rproc *drproc = rproc->priv;
 
+	davinci_clk_reset_assert(drproc->dsp_clk);
 	clk_disable(drproc->dsp_clk);
 
 	return 0;
@@ -157,22 +158,6 @@ static const struct rproc_ops da8xx_rproc_ops = {
 	.kick = da8xx_rproc_kick,
 };
 
-static int reset_assert(struct device *dev)
-{
-	struct clk *dsp_clk;
-
-	dsp_clk = clk_get(dev, NULL);
-	if (IS_ERR(dsp_clk)) {
-		dev_err(dev, "clk_get error: %ld\n", PTR_ERR(dsp_clk));
-		return PTR_ERR(dsp_clk);
-	}
-
-	davinci_clk_reset_assert(dsp_clk);
-	clk_put(dsp_clk);
-
-	return 0;
-}
-
 static int da8xx_rproc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -223,6 +208,7 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
 
 	drproc = rproc->priv;
 	drproc->rproc = rproc;
+	drproc->dsp_clk = dsp_clk;
 	rproc->has_iommu = false;
 
 	platform_set_drvdata(pdev, rproc);
@@ -241,7 +227,7 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
 	 * *not* in reset, but da8xx_rproc_start() needs the DSP to be
 	 * held in reset at the time it is called.
 	 */
-	ret = reset_assert(dev);
+	ret = davinci_clk_reset_assert(drproc->dsp_clk);
 	if (ret)
 		goto free_rproc;
 
@@ -250,7 +236,6 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
 	drproc->ack_fxn = irq_data->chip->irq_ack;
 	drproc->irq_data = irq_data;
 	drproc->irq = irq;
-	drproc->dsp_clk = dsp_clk;
 
 	ret = rproc_add(rproc);
 	if (ret) {
@@ -268,20 +253,9 @@ static int da8xx_rproc_probe(struct platform_device *pdev)
 
 static int da8xx_rproc_remove(struct platform_device *pdev)
 {
-	struct device *dev = &pdev->dev;
 	struct rproc *rproc = platform_get_drvdata(pdev);
 	struct da8xx_rproc *drproc = (struct da8xx_rproc *)rproc->priv;
 
-	/*
-	 * It's important to place the DSP in reset before going away,
-	 * since a subsequent insmod of this module may enable the DSP's
-	 * clock before its program/boot-address has been loaded and
-	 * before this module's probe has had a chance to reset the DSP.
-	 * Without the reset, the DSP can lockup permanently when it
-	 * begins executing garbage.
-	 */
-	reset_assert(dev);
-
 	/*
 	 * The devm subsystem might end up releasing things before
 	 * freeing the irq, thus allowing an interrupt to sneak in while

drivers/remoteproc/remoteproc_core.c

@@ -847,6 +847,63 @@ static void rproc_resource_cleanup(struct rproc *rproc)
 		kref_put(&rvdev->refcount, rproc_vdev_release);
 }
 
+static int rproc_start(struct rproc *rproc, const struct firmware *fw)
+{
+	struct resource_table *table, *loaded_table;
+	struct device *dev = &rproc->dev;
+	int ret, tablesz;
+
+	/* look for the resource table */
+	table = rproc_find_rsc_table(rproc, fw, &tablesz);
+	if (!table) {
+		dev_err(dev, "Resource table look up failed\n");
+		return -EINVAL;
+	}
+
+	/* load the ELF segments to memory */
+	ret = rproc_load_segments(rproc, fw);
+	if (ret) {
+		dev_err(dev, "Failed to load program segments: %d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * The starting device has been given the rproc->cached_table as the
+	 * resource table. The address of the vring along with the other
+	 * allocated resources (carveouts etc) is stored in cached_table.
+	 * In order to pass this information to the remote device we must copy
+	 * this information to device memory. We also update the table_ptr so
+	 * that any subsequent changes will be applied to the loaded version.
+	 */
+	loaded_table = rproc_find_loaded_rsc_table(rproc, fw);
+	if (loaded_table) {
+		memcpy(loaded_table, rproc->cached_table, tablesz);
+		rproc->table_ptr = loaded_table;
+	}
+
+	/* power up the remote processor */
+	ret = rproc->ops->start(rproc);
+	if (ret) {
+		dev_err(dev, "can't start rproc %s: %d\n", rproc->name, ret);
+		return ret;
+	}
+
+	/* probe any subdevices for the remote processor */
+	ret = rproc_probe_subdevices(rproc);
+	if (ret) {
+		dev_err(dev, "failed to probe subdevices for %s: %d\n",
+			rproc->name, ret);
+		rproc->ops->stop(rproc);
+		return ret;
+	}
+
+	rproc->state = RPROC_RUNNING;
+
+	dev_info(dev, "remote processor %s is now up\n", rproc->name);
+
+	return 0;
+}
+
 /*
  * take a firmware and boot a remote processor with it.
  */
@@ -854,7 +911,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
 {
 	struct device *dev = &rproc->dev;
 	const char *name = rproc->firmware;
-	struct resource_table *table, *loaded_table;
+	struct resource_table *table;
 	int ret, tablesz;
 
 	ret = rproc_fw_sanity_check(rproc, fw);
@@ -905,50 +962,12 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw)
 		goto clean_up_resources;
 	}
 
-	/* load the ELF segments to memory */
-	ret = rproc_load_segments(rproc, fw);
-	if (ret) {
-		dev_err(dev, "Failed to load program segments: %d\n", ret);
-		goto clean_up_resources;
-	}
-
-	/*
-	 * The starting device has been given the rproc->cached_table as the
-	 * resource table. The address of the vring along with the other
-	 * allocated resources (carveouts etc) is stored in cached_table.
-	 * In order to pass this information to the remote device we must copy
-	 * this information to device memory. We also update the table_ptr so
-	 * that any subsequent changes will be applied to the loaded version.
-	 */
-	loaded_table = rproc_find_loaded_rsc_table(rproc, fw);
-	if (loaded_table) {
-		memcpy(loaded_table, rproc->cached_table, tablesz);
-		rproc->table_ptr = loaded_table;
-	}
-
-	/* power up the remote processor */
-	ret = rproc->ops->start(rproc);
-	if (ret) {
-		dev_err(dev, "can't start rproc %s: %d\n", rproc->name, ret);
+	ret = rproc_start(rproc, fw);
+	if (ret)
 		goto clean_up_resources;
-	}
-
-	/* probe any subdevices for the remote processor */
-	ret = rproc_probe_subdevices(rproc);
-	if (ret) {
-		dev_err(dev, "failed to probe subdevices for %s: %d\n",
-			rproc->name, ret);
-		goto stop_rproc;
-	}
-
-	rproc->state = RPROC_RUNNING;
-
-	dev_info(dev, "remote processor %s is now up\n", rproc->name);
 
 	return 0;
 
-stop_rproc:
-	rproc->ops->stop(rproc);
 clean_up_resources:
 	rproc_resource_cleanup(rproc);
 clean_up:
@@ -994,6 +1013,32 @@ static int rproc_trigger_auto_boot(struct rproc *rproc)
 	return ret;
 }
 
+static int rproc_stop(struct rproc *rproc)
+{
+	struct device *dev = &rproc->dev;
+	int ret;
+
+	/* remove any subdevices for the remote processor */
+	rproc_remove_subdevices(rproc);
+
+	/* power off the remote processor */
+	ret = rproc->ops->stop(rproc);
+	if (ret) {
+		dev_err(dev, "can't stop rproc: %d\n", ret);
+		return ret;
+	}
+
+	/* if in crash state, unlock crash handler */
+	if (rproc->state == RPROC_CRASHED)
+		complete_all(&rproc->crash_comp);
+
+	rproc->state = RPROC_OFFLINE;
+
+	dev_info(dev, "stopped remote processor %s\n", rproc->name);
+
+	return 0;
+}
+
 /**
  * rproc_trigger_recovery() - recover a remoteproc
  * @rproc: the remote processor
@@ -1006,23 +1051,40 @@ static int rproc_trigger_auto_boot(struct rproc *rproc)
  */
 int rproc_trigger_recovery(struct rproc *rproc)
 {
-	dev_err(&rproc->dev, "recovering %s\n", rproc->name);
+	const struct firmware *firmware_p;
+	struct device *dev = &rproc->dev;
+	int ret;
+
+	dev_err(dev, "recovering %s\n", rproc->name);
 
 	init_completion(&rproc->crash_comp);
 
-	/* shut down the remote */
-	/* TODO: make sure this works with rproc->power > 1 */
-	rproc_shutdown(rproc);
+	ret = mutex_lock_interruptible(&rproc->lock);
+	if (ret)
+		return ret;
+
+	ret = rproc_stop(rproc);
+	if (ret)
+		goto unlock_mutex;
 
 	/* wait until there is no more rproc users */
 	wait_for_completion(&rproc->crash_comp);
 
-	/*
-	 * boot the remote processor up again
-	 */
-	rproc_boot(rproc);
+	/* load firmware */
+	ret = request_firmware(&firmware_p, rproc->firmware, dev);
+	if (ret < 0) {
+		dev_err(dev, "request_firmware failed: %d\n", ret);
+		goto unlock_mutex;
+	}
 
-	return 0;
+	/* boot the remote processor up again */
+	ret = rproc_start(rproc, firmware_p);
+
+	release_firmware(firmware_p);
+
+unlock_mutex:
+	mutex_unlock(&rproc->lock);
+	return ret;
 }
 
 /**
@@ -1163,14 +1225,9 @@ void rproc_shutdown(struct rproc *rproc)
 	if (!atomic_dec_and_test(&rproc->power))
 		goto out;
 
-	/* remove any subdevices for the remote processor */
-	rproc_remove_subdevices(rproc);
-
-	/* power off the remote processor */
-	ret = rproc->ops->stop(rproc);
+	ret = rproc_stop(rproc);
 	if (ret) {
 		atomic_inc(&rproc->power);
-		dev_err(dev, "can't stop rproc: %d\n", ret);
 		goto out;
 	}
 
@@ -1183,15 +1240,6 @@ void rproc_shutdown(struct rproc *rproc)
 	kfree(rproc->cached_table);
 	rproc->cached_table = NULL;
 	rproc->table_ptr = NULL;
-
-	/* if in crash state, unlock crash handler */
-	if (rproc->state == RPROC_CRASHED)
-		complete_all(&rproc->crash_comp);
-
-	rproc->state = RPROC_OFFLINE;
-
-	dev_info(dev, "stopped remote processor %s\n", rproc->name);
-
 out:
 	mutex_unlock(&rproc->lock);
 }