[PATCH v3 0/4] remoteproc: Introduce wkup_m3_rproc driver

[PATCH v3 0/4] remoteproc: Introduce wkup_m3_rproc driver

[Dave Gerlach]

Hi,
This patch series is version three of the series to add a
wkup_m3_rproc driver for TI AM335x SoCs. This family of SoCs
contains an ARM Cortex M3 coprocessor that is responsible for
low-level power tasks that cannot be handled by the main ARM
Cortex A8 so firmware running from the CM3 can be used instead.
This driver handles loading of the firmware and reset of the CM3
once it is booted.

The previous version of this series can be found here [1].
I have pushed a branch based on v4.0-rc5 containing the entire
am335x suspend series here for a higher level view of the entire
series of patch sets here [2]. This series depends on "remoteproc:
add IOMMU hardware capability flag" which is currently queued
here [3].

Based on comments on the DT node included in the "ARM: OMAP2+:
wkup_m3_rproc support patches" series (v3 of that will immediately
follow this series) the DT node moved under a different parent
node so some changes to the driver were necessary to calculate proper
device addresses for firmware loading.

This series also now includes a patch to introduce an
rproc_get_by_phandle API to the remoteproc core so that users of
this wkup_m3_rproc driver are able to get a handle to the rproc
and boot it as the rproc must be booted directly by the user.
An example user, wkup_m3_ipc, can be seen in previously mentioned
branch at [2].

v2 -> v3:
-Modify wkup_m3_rproc driver to properly handle device address
 based on new DT location in l4_wkup node.
-In binding doc, change ti,am3352-wkup-m3 from am3353-wkup_m3 to match
 other am3352 compats
-General cleanup of address representation in wkup_m3_rproc driver
-Includes rproc_get_by_phandle patch now

The driver expects to load firmware am335x-pm-firmware.elf from
/lib/firmware which is found here [4].

Regards,
Dave

[1] http://www.spinics.net/lists/linux-omap/msg116364.html
[2] https://github.com/dgerlach/linux-pm/tree/pm-v4.0-rc5-am335x-suspend
[3] https://git.kernel.org/cgit/linux/kernel/git/ohad/remoteproc.git/commit/?h=for-next&id=315491e5d6ee66838a18a8ca0c14e6ffb376e48c
[4] https://git.ti.com/ti-cm3-pm-firmware/amx3-cm3/commits/next-upstream

Dave Gerlach (3):
  remoteproc: introduce rproc_get_by_phandle API
  Documentation: dt: add bindings for TI Wakeup M3 processor
  remoteproc/wkup_m3: add a remoteproc driver for TI Wakeup M3

Suman Anna (1):
  remoteproc: add a rproc ops for performing address translation

 .../bindings/remoteproc/wkup_m3_rproc.txt          |  52 +++++
 Documentation/remoteproc.txt                       |   6 +
 drivers/remoteproc/Kconfig                         |  13 ++
 drivers/remoteproc/Makefile                        |   1 +
 drivers/remoteproc/remoteproc_core.c               | 114 +++++++++-
 drivers/remoteproc/wkup_m3_rproc.c                 | 249 +++++++++++++++++++++
 include/linux/platform_data/wkup_m3.h              |  30 +++
 include/linux/remoteproc.h                         |   4 +
 8 files changed, 463 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/wkup_m3_rproc.txt
 create mode 100644 drivers/remoteproc/wkup_m3_rproc.c
 create mode 100644 include/linux/platform_data/wkup_m3.h

-- 
2.3.0

[PATCH v3 1/4] remoteproc: introduce rproc_get_by_phandle API

[Dave Gerlach]

Allow users of remoteproc the ability to get a handle to an rproc by
passing a phandle supplied in the user's device tree node. This is
useful in situations that require manual booting of the rproc.

This patch uses the code removed by commit 40e575b1d0b3 ("remoteproc:
remove the get_by_name/put API") for the ref counting a rproc klist
code but has rproc_get_by_name replaced with an rproc_get_by_phandle API.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
 Documentation/remoteproc.txt         |  6 +++
 drivers/remoteproc/remoteproc_core.c | 83 ++++++++++++++++++++++++++++++++++++
 include/linux/remoteproc.h           |  2 +
 3 files changed, 91 insertions(+)

diff --git a/Documentation/remoteproc.txt b/Documentation/remoteproc.txt
index e6469fd..ef0219f 100644
--- a/Documentation/remoteproc.txt
+++ b/Documentation/remoteproc.txt
@@ -51,6 +51,12 @@ cost.
         rproc_shutdown() returns, and users can still use it with a subsequent
         rproc_boot(), if needed.
 
+  struct rproc *rproc_get_by_phandle(phandle phandle)
+    - Find an rproc handle using a device tree phandle. Returns the rproc
+      handle on success, and NULL on failure. This function increments
+      the remote processor's refcount, so always use rproc_put() to
+      decrement it back once rproc isn't needed anymore.
+
 3. Typical usage
 
 #include <linux/remoteproc.h>
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 3cd85a63..5a6c192 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -36,6 +36,7 @@
 #include <linux/remoteproc.h>
 #include <linux/iommu.h>
 #include <linux/idr.h>
+#include <linux/klist.h>
 #include <linux/elf.h>
 #include <linux/crc32.h>
 #include <linux/virtio_ids.h>
@@ -44,6 +45,17 @@
 
 #include "remoteproc_internal.h"
 
+static void klist_rproc_get(struct klist_node *n);
+static void klist_rproc_put(struct klist_node *n);
+
+/*
+ * klist of the available remote processors.
+ *
+ * We need this in order to support rproc lookups (needed by the
+ * rproc_get_by_phandle()).
+ */
+static DEFINE_KLIST(rprocs, klist_rproc_get, klist_rproc_put);
+
 typedef int (*rproc_handle_resources_t)(struct rproc *rproc,
 				struct resource_table *table, int len);
 typedef int (*rproc_handle_resource_t)(struct rproc *rproc,
@@ -1162,6 +1174,71 @@ out:
 }
 EXPORT_SYMBOL(rproc_shutdown);
 
+/* will be called when an rproc is added to the rprocs klist */
+static void klist_rproc_get(struct klist_node *n)
+{
+	struct rproc *rproc = container_of(n, struct rproc, node);
+
+	get_device(&rproc->dev);
+}
+
+/* will be called when an rproc is removed from the rprocs klist */
+static void klist_rproc_put(struct klist_node *n)
+{
+	struct rproc *rproc = container_of(n, struct rproc, node);
+
+	put_device(&rproc->dev);
+}
+
+static struct rproc *next_rproc(struct klist_iter *i)
+{
+	struct klist_node *n;
+
+	n = klist_next(i);
+	if (!n)
+		return NULL;
+
+	return container_of(n, struct rproc, node);
+}
+
+/**
+ * rproc_get_by_phandle() - find a remote processor by phandle
+ * @phandle: phandle to the rproc
+ *
+ * Finds an rproc handle using the remote processor's phandle, and then
+ * return a handle to the rproc.
+ *
+ * Returns the rproc handle on success, and NULL on failure.
+ *
+ * This function increments the remote processor's refcount, so always
+ * use rproc_put() to decrement it back once rproc isn't needed anymore.
+ *
+ */
+struct rproc *rproc_get_by_phandle(phandle phandle)
+{
+	struct rproc *rproc;
+	struct klist_iter i;
+	struct device_node *np;
+
+	np = of_find_node_by_phandle(phandle);
+	if (!np)
+		return NULL;
+
+	/* find the remote processor, and upref its refcount */
+	klist_iter_init(&rprocs, &i);
+	while ((rproc = next_rproc(&i)) != NULL)
+		if (rproc->dev.parent && rproc->dev.parent->of_node == np) {
+			get_device(&rproc->dev);
+			break;
+		}
+	klist_iter_exit(&i);
+
+	of_node_put(np);
+
+	return rproc;
+}
+EXPORT_SYMBOL(rproc_get_by_phandle);
+
 /**
  * rproc_add() - register a remote processor
  * @rproc: the remote processor handle to register
@@ -1191,6 +1268,9 @@ int rproc_add(struct rproc *rproc)
 	if (ret < 0)
 		return ret;
 
+	/* expose to rproc_get_by_phandle users */
+	klist_add_tail(&rproc->node, &rprocs);
+
 	dev_info(dev, "%s is available\n", rproc->name);
 
 	dev_info(dev, "Note: remoteproc is still under development and considered experimental.\n");
@@ -1380,6 +1460,9 @@ int rproc_del(struct rproc *rproc)
 	/* Free the copy of the resource table */
 	kfree(rproc->cached_table);
 
+	/* the rproc is downref'ed as soon as it's removed from the klist */
+	klist_del(&rproc->node);
+
 	device_del(&rproc->dev);
 
 	return 0;
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 9e7e745..0c7d403 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -41,6 +41,7 @@
 #include <linux/virtio.h>
 #include <linux/completion.h>
 #include <linux/idr.h>
+#include <linux/of.h>
 
 /**
  * struct resource_table - firmware resource table header
@@ -479,6 +480,7 @@ struct rproc_vdev {
 	u32 rsc_offset;
 };
 
+struct rproc *rproc_get_by_phandle(phandle phandle);
 struct rproc *rproc_alloc(struct device *dev, const char *name,
 				const struct rproc_ops *ops,
 				const char *firmware, int len);
-- 
2.3.0

[PATCH v3 2/4] remoteproc: add a rproc ops for performing address translation

[Dave Gerlach]

From: Suman Anna <s-anna@ti.com>

The rproc_da_to_va API is currently used to perform any device to
kernel address translations to meet the different needs of the remoteproc
core/drivers (eg: loading). The functionality is achieved within the
remoteproc core, and is limited only for carveouts allocated within the
core.

A new rproc ops, da_to_va, is added to provide flexibility to platform
implementations to perform the address translation themselves when the
above conditions cannot be met by the implementations. The rproc_da_to_va()
API is extended to invoke this ops if present, and fallback to regular
processing if the platform implementation cannot provide the translation.
This will allow any remoteproc implementations to translate addresses for
dedicated memories like internal memories.

While at this, also update the rproc_da_to_va() documentation since it
is an exported function.

Signed-off-by: Suman Anna <s-anna@ti.com>
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 drivers/remoteproc/remoteproc_core.c | 31 +++++++++++++++++++++++++------
 include/linux/remoteproc.h           |  2 ++
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 5a6c192..f1efe62 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -159,28 +159,46 @@ static void rproc_disable_iommu(struct rproc *rproc)
 	return;
 }
 
-/*
+/**
+ * rproc_da_to_va() - lookup the kernel virtual address for a remoteproc address
+ * @rproc: handle of a remote processor
+ * @da: remoteproc device address to translate
+ * @len: length of the memory region @da is pointing to
+ *
  * Some remote processors will ask us to allocate them physically contiguous
  * memory regions (which we call "carveouts"), and map them to specific
- * device addresses (which are hardcoded in the firmware).
+ * device addresses (which are hardcoded in the firmware). They may also have
+ * dedicated memory regions internal to the processors, and use them either
+ * exclusively or alongside carveouts.
  *
  * They may then ask us to copy objects into specific device addresses (e.g.
  * code/data sections) or expose us certain symbols in other device address
  * (e.g. their trace buffer).
  *
- * This function is an internal helper with which we can go over the allocated
- * carveouts and translate specific device address to kernel virtual addresses
- * so we can access the referenced memory.
+ * This function is a helper function with which we can go over the allocated
+ * carveouts and translate specific device addresses to kernel virtual addresses
+ * so we can access the referenced memory. This function also allows to perform
+ * translations on the internal remoteproc memory regions through a platform
+ * implementation specific da_to_va ops, if present.
+ *
+ * The function returns a valid kernel address on success or NULL on failure.
  *
  * Note: phys_to_virt(iommu_iova_to_phys(rproc->domain, da)) will work too,
  * but only on kernel direct mapped RAM memory. Instead, we're just using
- * here the output of the DMA API, which should be more correct.
+ * here the output of the DMA API for the carveouts, which should be more
+ * correct.
  */
 void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
 {
 	struct rproc_mem_entry *carveout;
 	void *ptr = NULL;
 
+	if (rproc->ops->da_to_va) {
+		ptr = rproc->ops->da_to_va(rproc, da, len);
+		if (ptr)
+			goto out;
+	}
+
 	list_for_each_entry(carveout, &rproc->carveouts, node) {
 		int offset = da - carveout->da;
 
@@ -197,6 +215,7 @@ void *rproc_da_to_va(struct rproc *rproc, u64 da, int len)
 		break;
 	}
 
+out:
 	return ptr;
 }
 EXPORT_SYMBOL(rproc_da_to_va);
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 0c7d403..81b224d 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -331,11 +331,13 @@ struct rproc;
  * @start:	power on the device and boot it
  * @stop:	power off the device
  * @kick:	kick a virtqueue (virtqueue id given as a parameter)
+ * @da_to_va:	optional platform hook to perform address translations
  */
 struct rproc_ops {
 	int (*start)(struct rproc *rproc);
 	int (*stop)(struct rproc *rproc);
 	void (*kick)(struct rproc *rproc, int vqid);
+	void * (*da_to_va)(struct rproc *rproc, u64 da, int len);
 };
 
 /**
-- 
2.3.0

[PATCH v3 3/4] Documentation: dt: add bindings for TI Wakeup M3 processor

[Dave Gerlach]

Add the device tree bindings document for the TI Wakeup M3 remote
processor devices on AM33xx and AM43xx SoCs. These devices are used
to offload low-level power management functionality, and are handled
by the wkup_m3 remoteproc driver.

Signed-off-by: Suman Anna <s-anna@ti.com>
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 .../bindings/remoteproc/wkup_m3_rproc.txt          | 52 ++++++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/wkup_m3_rproc.txt

diff --git a/Documentation/devicetree/bindings/remoteproc/wkup_m3_rproc.txt b/Documentation/devicetree/bindings/remoteproc/wkup_m3_rproc.txt
new file mode 100644
index 0000000..3a70073
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/wkup_m3_rproc.txt
@@ -0,0 +1,52 @@
+TI Wakeup M3 Remoteproc Driver
+==============================
+
+The TI AM33xx and AM43xx family of devices use a small Cortex M3 co-processor
+(commonly referred to as Wakeup M3 or CM3) to help with various low power tasks
+that cannot be controlled from the MPU. This CM3 processor requires a firmware
+binary to accomplish this. The wkup_m3 remoteproc driver handles the loading of
+the firmware and booting of the CM3.
+
+Wkup M3 Device Node:
+====================
+A wkup_m3 device node is used to represent the Wakeup M3 processor instance
+within the SoC. It is added as a child node of the parent interconnect bus
+(l4_wkup) through which it is accessible to the MPU.
+
+Required properties:
+--------------------
+- compatible:		Should be one of,
+				"ti,am3352-wkup-m3" for AM33xx SoCs
+				"ti,am4372-wkup-m3" for AM43xx SoCs
+- reg:			Should contain the address ranges for the two internal
+			memory regions, UMEM and DMEM. The parent node should
+			provide an appropriate ranges property for properly
+			translating these into bus addresses.
+- reg-names:		Contains the corresponding names for the two memory
+			regions. These should be named "umem" & "dmem".
+- ti,hwmods:		Name of the hwmod associated with the wkupm3 device.
+- ti,pm-firmware:	Name of firmware file to be used for loading and
+			booting the wkup_m3 remote processor.
+
+Example:
+--------
+/* AM33xx */
+ocp {
+	 l4_wkup: l4_wkup@44c00000 {
+		compatible = "am335-l4-wkup", "simple-bus";
+		ranges = <0 0x44c00000 0x400000>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		wkup_m3: wkup_m3@100000 {
+			compatible = "ti,am3352-wkup-m3";
+			reg = <0x100000 0x4000>,
+			      <0x180000 0x2000>;
+			reg-names = "umem", "dmem";
+			ti,hwmods = "wkup_m3";
+			ti,pm-firmware = "am335x-pm-firmware.elf";
+		};
+	};
+
+	...
+};
-- 
2.3.0

[PATCH v3 4/4] remoteproc/wkup_m3: add a remoteproc driver for TI Wakeup M3

[Dave Gerlach]

Add a remoteproc driver to load the firmware and boot a small
Wakeup M3 processor present on TI AM33xx and AM43xx SoCs. This
Wakeup M3 remote processor is an integrated Cortex M3 that allows
the SoC to enter the lowest possible power state by taking control
from the MPU after it has gone into its own low power state and
shutting off any additional peripherals.

Signed-off-by: Suman Anna <s-anna@ti.com>
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 drivers/remoteproc/Kconfig            |  13 ++
 drivers/remoteproc/Makefile           |   1 +
 drivers/remoteproc/wkup_m3_rproc.c    | 249 ++++++++++++++++++++++++++++++++++
 include/linux/platform_data/wkup_m3.h |  30 ++++
 4 files changed, 293 insertions(+)
 create mode 100644 drivers/remoteproc/wkup_m3_rproc.c
 create mode 100644 include/linux/platform_data/wkup_m3.h

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 5e343ba..28c711f 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -41,6 +41,19 @@ config STE_MODEM_RPROC
 	  This can be either built-in or a loadable module.
 	  If unsure say N.
 
+config WKUP_M3_RPROC
+	tristate "AMx3xx Wakeup M3 remoteproc support"
+	depends on SOC_AM33XX || SOC_AM43XX
+	select REMOTEPROC
+	help
+	  Say y here to support Wakeup M3 remote processor on TI AM33xx
+	  and AM43xx family of SoCs.
+
+	  Required for Suspend-to-RAM on AM33xx and AM43xx SoCs. Also needed
+	  for deep CPUIdle states on AM33xx SoCs. Allows for loading of the
+	  firmware onto these remote processors.
+	  If unsure say N.
+
 config DA8XX_REMOTEPROC
 	tristate "DA8xx/OMAP-L13x remoteproc support"
 	depends on ARCH_DAVINCI_DA8XX
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index ac2ff75..81b04d1 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -9,4 +9,5 @@ remoteproc-y				+= remoteproc_virtio.o
 remoteproc-y				+= remoteproc_elf_loader.o
 obj-$(CONFIG_OMAP_REMOTEPROC)		+= omap_remoteproc.o
 obj-$(CONFIG_STE_MODEM_RPROC)	 	+= ste_modem_rproc.o
+obj-$(CONFIG_WKUP_M3_RPROC)		+= wkup_m3_rproc.o
 obj-$(CONFIG_DA8XX_REMOTEPROC)		+= da8xx_remoteproc.o
diff --git a/drivers/remoteproc/wkup_m3_rproc.c b/drivers/remoteproc/wkup_m3_rproc.c
new file mode 100644
index 0000000..134ffa7
--- /dev/null
+++ b/drivers/remoteproc/wkup_m3_rproc.c
@@ -0,0 +1,249 @@
+/*
+ * TI AMx3 Wakeup M3 Remote Processor driver
+ *
+ * Copyright (C) 2014-2015 Texas Instruments, Inc.
+ *
+ * Dave Gerlach <d-gerlach@ti.com>
+ * Suman Anna <s-anna@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/err.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/remoteproc.h>
+
+#include <linux/platform_data/wkup_m3.h>
+
+#include "remoteproc_internal.h"
+
+#define WKUPM3_MEM_MAX	2
+
+/**
+ * struct wkup_m3_mem - WkupM3 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 from Wakeup M3 view
+ * @size: Size of the memory region
+ */
+struct wkup_m3_mem {
+	void __iomem *cpu_addr;
+	phys_addr_t bus_addr;
+	u32 dev_addr;
+	size_t size;
+};
+
+/**
+ * struct wkup_m3_rproc - WkupM3 remote processor state
+ * @rproc: rproc handle
+ * @pdev: pointer to platform device
+ * @mem: WkupM3 memory information
+ */
+struct wkup_m3_rproc {
+	struct rproc *rproc;
+	struct platform_device *pdev;
+	struct wkup_m3_mem mem[WKUPM3_MEM_MAX];
+};
+
+static int wkup_m3_rproc_start(struct rproc *rproc)
+{
+	struct wkup_m3_rproc *wkupm3 = rproc->priv;
+	struct platform_device *pdev = wkupm3->pdev;
+	struct device *dev = &pdev->dev;
+	struct wkup_m3_platform_data *pdata = dev_get_platdata(dev);
+
+	if (pdata->deassert_reset(pdev, pdata->reset_name)) {
+		dev_err(dev, "Unable to reset wkup_m3!\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static int wkup_m3_rproc_stop(struct rproc *rproc)
+{
+	struct wkup_m3_rproc *wkupm3 = rproc->priv;
+	struct platform_device *pdev = wkupm3->pdev;
+	struct device *dev = &pdev->dev;
+	struct wkup_m3_platform_data *pdata = dev_get_platdata(dev);
+
+	if (pdata->assert_reset(pdev, pdata->reset_name)) {
+		dev_err(dev, "Unable to assert reset of wkup_m3!\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static void *wkup_m3_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
+{
+	struct wkup_m3_rproc *wkupm3 = rproc->priv;
+	void *va = NULL;
+	int i;
+	u32 offset;
+
+	if (len <= 0)
+		return NULL;
+
+	for (i = 0; i < WKUPM3_MEM_MAX; i++) {
+		if (da >= wkupm3->mem[i].dev_addr && da + len <=
+		    wkupm3->mem[i].dev_addr +  wkupm3->mem[i].size) {
+			offset = da -  wkupm3->mem[i].dev_addr;
+			va = (__force void *)(wkupm3->mem[i].cpu_addr + offset);
+			break;
+		}
+	}
+
+	return va;
+}
+
+static struct rproc_ops wkup_m3_rproc_ops = {
+	.start		= wkup_m3_rproc_start,
+	.stop		= wkup_m3_rproc_stop,
+	.da_to_va	= wkup_m3_rproc_da_to_va,
+};
+
+static const struct of_device_id wkup_m3_rproc_of_match[] = {
+	{ .compatible = "ti,am3352-wkup-m3", },
+	{ .compatible = "ti,am4372-wkup-m3", },
+	{},
+};
+
+static int wkup_m3_rproc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct wkup_m3_platform_data *pdata = dev->platform_data;
+	const char *mem_names[WKUPM3_MEM_MAX] = { "umem", "dmem" };
+	struct wkup_m3_rproc *wkupm3;
+	const char *fw_name;
+	struct rproc *rproc;
+	struct resource *res;
+	const __be32 *addrp;
+	u32 l4_offset = 0;
+	u64 size;
+	int ret;
+	int i;
+
+	if (!(pdata && pdata->deassert_reset && pdata->assert_reset &&
+	      pdata->reset_name)) {
+		dev_err(dev, "Platform data missing!\n");
+		return -ENODEV;
+	}
+
+	ret = of_property_read_string(dev->of_node, "ti,pm-firmware",
+				      &fw_name);
+	if (ret) {
+		dev_err(dev, "No firmware filename given\n");
+		return -ENODEV;
+	}
+
+	pm_runtime_enable(&pdev->dev);
+	ret = pm_runtime_get_sync(&pdev->dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "pm_runtime_get_sync() failed\n");
+		goto err;
+	}
+
+	rproc = rproc_alloc(dev, "wkup_m3", &wkup_m3_rproc_ops,
+			    fw_name, sizeof(*wkupm3));
+	if (!rproc) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	wkupm3 = rproc->priv;
+	wkupm3->rproc = rproc;
+	wkupm3->pdev = pdev;
+
+	for (i = 0; i < ARRAY_SIZE(mem_names); i++) {
+		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+						   mem_names[i]);
+		wkupm3->mem[i].cpu_addr = devm_ioremap_resource(dev, res);
+		if (IS_ERR(wkupm3->mem[i].cpu_addr)) {
+			dev_err(&pdev->dev, "devm_ioremap_resource failed for resource %d\n",
+				i);
+			ret = PTR_ERR(wkupm3->mem[i].cpu_addr);
+			goto err;
+		}
+		wkupm3->mem[i].bus_addr = res->start;
+		wkupm3->mem[i].size = resource_size(res);
+		addrp = of_get_address(dev->of_node, i, &size, NULL);
+		if (!strcmp(mem_names[i], "umem"))
+			l4_offset = be32_to_cpu(*addrp);
+		wkupm3->mem[i].dev_addr = be32_to_cpu(*addrp) - l4_offset;
+	}
+
+	dev_set_drvdata(dev, rproc);
+
+	ret = rproc_add(rproc);
+	if (ret) {
+		dev_err(dev, "rproc_add failed\n");
+		goto err_put_rproc;
+	}
+
+	return 0;
+
+err_put_rproc:
+	rproc_put(rproc);
+err:
+	pm_runtime_put_noidle(dev);
+	pm_runtime_disable(dev);
+	return ret;
+}
+
+static int wkup_m3_rproc_remove(struct platform_device *pdev)
+{
+	struct rproc *rproc = platform_get_drvdata(pdev);
+
+	rproc_del(rproc);
+	rproc_put(rproc);
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int wkup_m3_rpm_suspend(struct device *dev)
+{
+	return -EBUSY;
+}
+
+static int wkup_m3_rpm_resume(struct device *dev)
+{
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops wkup_m3_rproc_pm_ops = {
+	SET_RUNTIME_PM_OPS(wkup_m3_rpm_suspend, wkup_m3_rpm_resume, NULL)
+};
+
+static struct platform_driver wkup_m3_rproc_driver = {
+	.probe = wkup_m3_rproc_probe,
+	.remove = wkup_m3_rproc_remove,
+	.driver = {
+		.name = "wkup_m3_rproc",
+		.of_match_table = wkup_m3_rproc_of_match,
+		.pm = &wkup_m3_rproc_pm_ops,
+	},
+};
+
+module_platform_driver(wkup_m3_rproc_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("TI Wakeup M3 remote processor control driver");
+MODULE_AUTHOR("Dave Gerlach <d-gerlach@ti.com>");
diff --git a/include/linux/platform_data/wkup_m3.h b/include/linux/platform_data/wkup_m3.h
new file mode 100644
index 0000000..3f1d77e
--- /dev/null
+++ b/include/linux/platform_data/wkup_m3.h
@@ -0,0 +1,30 @@
+/*
+ * TI Wakeup M3 remote processor platform data
+ *
+ * Copyright (C) 2014-2015 Texas Instruments, Inc.
+ *
+ * Dave Gerlach <d-gerlach@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.
+ */
+
+#ifndef _LINUX_PLATFORM_DATA_WKUP_M3_H
+#define _LINUX_PLATFORM_DATA_WKUP_M3_H
+
+struct platform_device;
+
+struct wkup_m3_platform_data {
+	const char *reset_name;
+
+	int (*assert_reset)(struct platform_device *pdev, const char *name);
+	int (*deassert_reset)(struct platform_device *pdev, const char *name);
+};
+
+#endif /* _LINUX_PLATFORM_DATA_WKUP_M3_H */
-- 
2.3.0

Re: [PATCH v3 0/4] remoteproc: Introduce wkup_m3_rproc driver

[Suman Anna]

Hi Ohad,

On 04/01/2015 02:37 PM, Dave Gerlach wrote:
> Hi,
> This patch series is version three of the series to add a
> wkup_m3_rproc driver for TI AM335x SoCs. This family of SoCs
> contains an ARM Cortex M3 coprocessor that is responsible for
> low-level power tasks that cannot be handled by the main ARM
> Cortex A8 so firmware running from the CM3 can be used instead.
> This driver handles loading of the firmware and reset of the CM3
> once it is booted.
> 
> The previous version of this series can be found here [1].
> I have pushed a branch based on v4.0-rc5 containing the entire
> am335x suspend series here for a higher level view of the entire
> series of patch sets here [2]. This series depends on "remoteproc:
> add IOMMU hardware capability flag" which is currently queued
> here [3].
> 
> Based on comments on the DT node included in the "ARM: OMAP2+:
> wkup_m3_rproc support patches" series (v3 of that will immediately
> follow this series) the DT node moved under a different parent
> node so some changes to the driver were necessary to calculate proper
> device addresses for firmware loading.
> 
> This series also now includes a patch to introduce an
> rproc_get_by_phandle API to the remoteproc core so that users of
> this wkup_m3_rproc driver are able to get a handle to the rproc
> and boot it as the rproc must be booted directly by the user.
> An example user, wkup_m3_ipc, can be seen in previously mentioned
> branch at [2].
> 
> v2 -> v3:
> -Modify wkup_m3_rproc driver to properly handle device address
>  based on new DT location in l4_wkup node.
> -In binding doc, change ti,am3352-wkup-m3 from am3353-wkup_m3 to match
>  other am3352 compats
> -General cleanup of address representation in wkup_m3_rproc driver
> -Includes rproc_get_by_phandle patch now
> 
> The driver expects to load firmware am335x-pm-firmware.elf from
> /lib/firmware which is found here [4].
> 
> Regards,
> Dave
> 
> [1] http://www.spinics.net/lists/linux-omap/msg116364.html
> [2] https://github.com/dgerlach/linux-pm/tree/pm-v4.0-rc5-am335x-suspend
> [3] https://git.kernel.org/cgit/linux/kernel/git/ohad/remoteproc.git/commit/?h=for-next&id=315491e5d6ee66838a18a8ca0c14e6ffb376e48c
> [4] https://git.ti.com/ti-cm3-pm-firmware/amx3-cm3/commits/next-upstream
> 
> Dave Gerlach (3):
>   remoteproc: introduce rproc_get_by_phandle API
>   Documentation: dt: add bindings for TI Wakeup M3 processor
>   remoteproc/wkup_m3: add a remoteproc driver for TI Wakeup M3
> 
> Suman Anna (1):
>   remoteproc: add a rproc ops for performing address translation

Ping, do you have any comments on this series? This is the core
dependency for achieving suspend/resume and cpuidle on AM335x and AM437x
SoCs. I am hoping to have this series make 4.2.

regards
Suman

> 
>  .../bindings/remoteproc/wkup_m3_rproc.txt          |  52 +++++
>  Documentation/remoteproc.txt                       |   6 +
>  drivers/remoteproc/Kconfig                         |  13 ++
>  drivers/remoteproc/Makefile                        |   1 +
>  drivers/remoteproc/remoteproc_core.c               | 114 +++++++++-
>  drivers/remoteproc/wkup_m3_rproc.c                 | 249 +++++++++++++++++++++
>  include/linux/platform_data/wkup_m3.h              |  30 +++
>  include/linux/remoteproc.h                         |   4 +
>  8 files changed, 463 insertions(+), 6 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/wkup_m3_rproc.txt
>  create mode 100644 drivers/remoteproc/wkup_m3_rproc.c
>  create mode 100644 include/linux/platform_data/wkup_m3.h
> 

Re: [PATCH v3 0/4] remoteproc: Introduce wkup_m3_rproc driver

[Ohad Ben-Cohen]

Hi Suman,

On Wed, Apr 29, 2015 at 7:05 PM, Suman Anna <s-anna@ti.com> wrote:
> Ping, do you have any comments on this series?

I'll get to review this next week.

Thanks,
Ohad.

Re: [PATCH v3 1/4] remoteproc: introduce rproc_get_by_phandle API

[Ohad Ben-Cohen]

Hi Dave,

On Wed, Apr 1, 2015 at 10:37 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
> Allow users of remoteproc the ability to get a handle to an rproc by
> passing a phandle supplied in the user's device tree node. This is
> useful in situations that require manual booting of the rproc.
>
> This patch uses the code removed by commit 40e575b1d0b3 ("remoteproc:
> remove the get_by_name/put API") for the ref counting a rproc klist
> code but has rproc_get_by_name replaced with an rproc_get_by_phandle API.

The general idea makes sense to me, but I'm not sure we really do need
a klist here, since the usage profile of this list is expected to be
super simple: very small number of accessors, looking for small number
of list members a small number of times, and probably never do need to
modify the list while accessing it.

I suspect that the code would be simpler to maintain, debug and
understand if we just use a simple list with a simple locking
methodology here.

Thanks,
Ohad.

Re: [PATCH v3 2/4] remoteproc: add a rproc ops for performing address translation

[Ohad Ben-Cohen]

Hi Dave,

On Wed, Apr 1, 2015 at 10:37 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
> From: Suman Anna <s-anna@ti.com>
>
> The rproc_da_to_va API is currently used to perform any device to
> kernel address translations to meet the different needs of the remoteproc
> core/drivers (eg: loading). The functionality is achieved within the
> remoteproc core, and is limited only for carveouts allocated within the
> core.
>
> A new rproc ops, da_to_va, is added to provide flexibility to platform
> implementations to perform the address translation themselves when the
> above conditions cannot be met by the implementations. The rproc_da_to_va()
> API is extended to invoke this ops if present, and fallback to regular
> processing if the platform implementation cannot provide the translation.
> This will allow any remoteproc implementations to translate addresses for
> dedicated memories like internal memories.

Can you please provide specific examples where this is needed and how
it is going to be used?

Thanks,
Ohad.

Re: [PATCH v3 4/4] remoteproc/wkup_m3: add a remoteproc driver for TI Wakeup M3

[Ohad Ben-Cohen]

Hi Dave,

On Wed, Apr 1, 2015 at 10:37 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
> Add a remoteproc driver to load the firmware and boot a small
> Wakeup M3 processor present on TI AM33xx and AM43xx SoCs. This
> Wakeup M3 remote processor is an integrated Cortex M3 that allows
> the SoC to enter the lowest possible power state by taking control
> from the MPU after it has gone into its own low power state and
> shutting off any additional peripherals.

>From a remoteproc point of view this looks generally ok.

The only non-standard remoteproc aspect here is the handling of the
internal memories. Can you please generally describe how are these
memories being used in the context of remoteproc? how does the
resource table of your firmware look like - do you also have carveouts
or other resources?

Thanks,
Ohad.

Re: [PATCH v3 2/4] remoteproc: add a rproc ops for performing address translation

[Suman Anna]

Hi Ohad,

On 05/09/2015 02:54 AM, Ohad Ben-Cohen wrote:
> Hi Dave,
> 
> On Wed, Apr 1, 2015 at 10:37 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>> From: Suman Anna <s-anna@ti.com>
>>
>> The rproc_da_to_va API is currently used to perform any device to
>> kernel address translations to meet the different needs of the remoteproc
>> core/drivers (eg: loading). The functionality is achieved within the
>> remoteproc core, and is limited only for carveouts allocated within the
>> core.
>>
>> A new rproc ops, da_to_va, is added to provide flexibility to platform
>> implementations to perform the address translation themselves when the
>> above conditions cannot be met by the implementations. The rproc_da_to_va()
>> API is extended to invoke this ops if present, and fallback to regular
>> processing if the platform implementation cannot provide the translation.
>> This will allow any remoteproc implementations to translate addresses for
>> dedicated memories like internal memories.
> 
> Can you please provide specific examples where this is needed and how
> it is going to be used?

We will be using this for the WkupM3 remoteproc driver, and also using
it for a PRUSS remoteproc driver (on downstream kernel for now) on TI
AM335x/AM437x SoCs. The driver uses a firmware where all segments are
placed only in internal RAMs (nothing in DDR), and we use the remoteproc
core's ELF loader code to perform the loading. We need a way for the
remoteproc elf loader core to be able to translate these device
addresses into a kernel mapped addresses so that the loader code can
copy the firmware segments.

The previous attempt on this was to use a new resource type through the
resource table, whereby we are publishing and storing the internal
memory translations were stored in the remoteproc core [1].

regards
Suman

[1] https://patchwork.kernel.org/patch/5602981/

Re: [PATCH v3 4/4] remoteproc/wkup_m3: add a remoteproc driver for TI Wakeup M3

[Suman Anna]

Hi Ohad,

On 05/09/2015 03:42 AM, Ohad Ben-Cohen wrote:
> Hi Dave,
> 
> On Wed, Apr 1, 2015 at 10:37 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>> Add a remoteproc driver to load the firmware and boot a small
>> Wakeup M3 processor present on TI AM33xx and AM43xx SoCs. This
>> Wakeup M3 remote processor is an integrated Cortex M3 that allows
>> the SoC to enter the lowest possible power state by taking control
>> from the MPU after it has gone into its own low power state and
>> shutting off any additional peripherals.
> 
> From a remoteproc point of view this looks generally ok.
> 
> The only non-standard remoteproc aspect here is the handling of the
> internal memories. Can you please generally describe how are these
> memories being used in the context of remoteproc? 

The WkupM3 executes its code entirely from this internal RAM.

how does the
> resource table of your firmware look like - do you also have carveouts
> or other resources?

We don't have any carveouts or usage of any external DDR, as this
processor is used during Power Management, like cpuidle or suspend path,
and is used to control the MPU and DDR states. The resource table is
very simple and straight-forward [1]. We used to have RSC_INTMEM as part
of the resource table in the previous version, but we removed them since
that approach was NACKed.

regards
Suman

[1]
https://git.ti.com/ti-cm3-pm-firmware/amx3-cm3/blobs/next-upstream/src/sys_exec/rsc_table.h

Re: [PATCH v3 1/4] remoteproc: introduce rproc_get_by_phandle API

[Suman Anna]

Hi Ohad,

On 05/09/2015 02:39 AM, Ohad Ben-Cohen wrote:
> Hi Dave,
> 
> On Wed, Apr 1, 2015 at 10:37 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>> Allow users of remoteproc the ability to get a handle to an rproc by
>> passing a phandle supplied in the user's device tree node. This is
>> useful in situations that require manual booting of the rproc.
>>
>> This patch uses the code removed by commit 40e575b1d0b3 ("remoteproc:
>> remove the get_by_name/put API") for the ref counting a rproc klist
>> code but has rproc_get_by_name replaced with an rproc_get_by_phandle API.
> 
> The general idea makes sense to me, but I'm not sure we really do need
> a klist here, since the usage profile of this list is expected to be
> super simple: very small number of accessors, looking for small number
> of list members a small number of times, and probably never do need to
> modify the list while accessing it.
> 
> I suspect that the code would be simpler to maintain, debug and
> understand if we just use a simple list with a simple locking
> methodology here.

The klist usage is something that we restored from previous remoteproc
core code as used by the rproc_get_by_name() API. This was removed in
commit 40e575b1d0b3 ("remoteproc: remove the get_by_name/put API"). We
chose to use the code that had been present before rather than inventing
something new all over again. If you feel that a regular list is the way
to go forward, we can make the switch.

regards
Suman

Re: [PATCH v3 3/4] Documentation: dt: add bindings for TI Wakeup M3 processor

[Tony Lindgren]

Hi,

* Dave Gerlach <d-gerlach@ti.com> [150401 12:38]:
> Add the device tree bindings document for the TI Wakeup M3 remote
> processor devices on AM33xx and AM43xx SoCs. These devices are used
> to offload low-level power management functionality, and are handled
> by the wkup_m3 remoteproc driver.
> 
> Signed-off-by: Suman Anna <s-anna@ti.com>
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>

This binding looks OK to me now, thanks for updating it:

Acked-by: Tony Lindgren <tony@atomide.com>

> ---
>  .../bindings/remoteproc/wkup_m3_rproc.txt          | 52 ++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/wkup_m3_rproc.txt
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/wkup_m3_rproc.txt b/Documentation/devicetree/bindings/remoteproc/wkup_m3_rproc.txt
> new file mode 100644
> index 0000000..3a70073
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/wkup_m3_rproc.txt
> @@ -0,0 +1,52 @@
> +TI Wakeup M3 Remoteproc Driver
> +==============================
> +
> +The TI AM33xx and AM43xx family of devices use a small Cortex M3 co-processor
> +(commonly referred to as Wakeup M3 or CM3) to help with various low power tasks
> +that cannot be controlled from the MPU. This CM3 processor requires a firmware
> +binary to accomplish this. The wkup_m3 remoteproc driver handles the loading of
> +the firmware and booting of the CM3.
> +
> +Wkup M3 Device Node:
> +====================
> +A wkup_m3 device node is used to represent the Wakeup M3 processor instance
> +within the SoC. It is added as a child node of the parent interconnect bus
> +(l4_wkup) through which it is accessible to the MPU.
> +
> +Required properties:
> +--------------------
> +- compatible:		Should be one of,
> +				"ti,am3352-wkup-m3" for AM33xx SoCs
> +				"ti,am4372-wkup-m3" for AM43xx SoCs
> +- reg:			Should contain the address ranges for the two internal
> +			memory regions, UMEM and DMEM. The parent node should
> +			provide an appropriate ranges property for properly
> +			translating these into bus addresses.
> +- reg-names:		Contains the corresponding names for the two memory
> +			regions. These should be named "umem" & "dmem".
> +- ti,hwmods:		Name of the hwmod associated with the wkupm3 device.
> +- ti,pm-firmware:	Name of firmware file to be used for loading and
> +			booting the wkup_m3 remote processor.
> +
> +Example:
> +--------
> +/* AM33xx */
> +ocp {
> +	 l4_wkup: l4_wkup@44c00000 {
> +		compatible = "am335-l4-wkup", "simple-bus";
> +		ranges = <0 0x44c00000 0x400000>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +
> +		wkup_m3: wkup_m3@100000 {
> +			compatible = "ti,am3352-wkup-m3";
> +			reg = <0x100000 0x4000>,
> +			      <0x180000 0x2000>;
> +			reg-names = "umem", "dmem";
> +			ti,hwmods = "wkup_m3";
> +			ti,pm-firmware = "am335x-pm-firmware.elf";
> +		};
> +	};
> +
> +	...
> +};
> -- 
> 2.3.0
> 

Re: [PATCH v3 1/4] remoteproc: introduce rproc_get_by_phandle API

[Ohad Ben-Cohen]

Hi Suman,

On Mon, May 11, 2015 at 6:09 PM, Suman Anna <s-anna@ti.com> wrote:
> On 05/09/2015 02:39 AM, Ohad Ben-Cohen wrote:
> > On Wed, Apr 1, 2015 at 10:37 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
> >> This patch uses the code removed by commit 40e575b1d0b3 ("remoteproc:
> >> remove the get_by_name/put API") for the ref counting a rproc klist
> >> code but has rproc_get_by_name replaced with an rproc_get_by_phandle API.
> >
> > The general idea makes sense to me, but I'm not sure we really do need
> > a klist here, since the usage profile of this list is expected to be
> > super simple: very small number of accessors, looking for small number
> > of list members a small number of times, and probably never do need to
> > modify the list while accessing it.
> >
> > I suspect that the code would be simpler to maintain, debug and
> > understand if we just use a simple list with a simple locking
> > methodology here.
>
> The klist usage is something that we restored from previous remoteproc
> core code as used by the rproc_get_by_name() API. This was removed in
> commit 40e575b1d0b3 ("remoteproc: remove the get_by_name/put API"). We
> chose to use the code that had been present before rather than inventing
> something new all over again. If you feel that a regular list is the way
> to go forward, we can make the switch.

Yes, please. Using a regular list with a simple locking methodology
should make the code easier to understand and debug.

Thanks,
Ohad.

Re: [PATCH v3 4/4] remoteproc/wkup_m3: add a remoteproc driver for TI Wakeup M3

[Ohad Ben-Cohen]

Hi Suman,

On Mon, May 11, 2015 at 6:01 PM, Suman Anna <s-anna@ti.com> wrote:
> We don't have any carveouts or usage of any external DDR, as this
> processor is used during Power Management, like cpuidle or suspend path,
> and is used to control the MPU and DDR states. The resource table is
> very simple and straight-forward [1].

Ok thanks.

Could you please document in the patch how the WkupM3 memory is
managed? Perhaps add to this latter explanation of yours also what
stands behind the umem/dmem names, justify usage of '__force' and
document the code around the l4_offset math (which I suspect may not
always be correct, as it seems the value of l4_offset depends on the
order of mem resources returned by platform_get_resource_byname).

Thanks,
Ohad.

Re: [PATCH v3 1/4] remoteproc: introduce rproc_get_by_phandle API

[Dave Gerlach]

Ohad,
On 05/16/2015 02:18 AM, Ohad Ben-Cohen wrote:
> Hi Suman,
> 
> On Mon, May 11, 2015 at 6:09 PM, Suman Anna <s-anna@ti.com> wrote:
>> On 05/09/2015 02:39 AM, Ohad Ben-Cohen wrote:
>>> On Wed, Apr 1, 2015 at 10:37 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>> This patch uses the code removed by commit 40e575b1d0b3 ("remoteproc:
>>>> remove the get_by_name/put API") for the ref counting a rproc klist
>>>> code but has rproc_get_by_name replaced with an rproc_get_by_phandle API.
>>>
>>> The general idea makes sense to me, but I'm not sure we really do need
>>> a klist here, since the usage profile of this list is expected to be
>>> super simple: very small number of accessors, looking for small number
>>> of list members a small number of times, and probably never do need to
>>> modify the list while accessing it.
>>>
>>> I suspect that the code would be simpler to maintain, debug and
>>> understand if we just use a simple list with a simple locking
>>> methodology here.
>>
>> The klist usage is something that we restored from previous remoteproc
>> core code as used by the rproc_get_by_name() API. This was removed in
>> commit 40e575b1d0b3 ("remoteproc: remove the get_by_name/put API"). We
>> chose to use the code that had been present before rather than inventing
>> something new all over again. If you feel that a regular list is the way
>> to go forward, we can make the switch.
> 
> Yes, please. Using a regular list with a simple locking methodology
> should make the code easier to understand and debug.

Ok, that makes sense, we can change this. Thanks for your input.

Regards,
Dave

> 
> Thanks,
> Ohad.
>