[U-Boot] [PATCH 0/7] Add STM32 Cortex-M4 remoteproc driver

[U-Boot] [PATCH 0/7] Add STM32 Cortex-M4 remoteproc driver

[Fabien Dessenne]

This patchset adds an STM32 remoteproc driver.

The two first patches add the xxx_translate_dma_address() API which is
the equivalent of the xxx_translate_address() relying on the "dma-ranges"
property instead of the "ranges" property.

The patches 3 & 4 add the support of the ELF image loading (the current
implementation supports only binary image loading).

The 5th patch is about the driver, and the two last patches are about
MAINTAINERS and configs update.

Fabien Dessenne (7):
  fdt: Introduce fdt_translate_dma_address()
  dm: core: Introduce xxx_translate_dma_address()
  remoteproc: add da_to_pa ops
  remoteproc: add elf file load support
  remoteproc: Introduce STM32 Cortex-M4 remoteproc driver
  MAINTAINERS: Add stm32 remoteproc driver
  configs: stm32mp15: enable stm32 remoteproc

 MAINTAINERS                         |   1 +
 common/fdt_support.c                |   6 +
 configs/stm32mp15_basic_defconfig   |   2 +
 configs/stm32mp15_trusted_defconfig |   2 +
 drivers/core/of_addr.c              |   4 +
 drivers/core/ofnode.c               |   8 ++
 drivers/core/read.c                 |   5 +
 drivers/remoteproc/Kconfig          |  10 ++
 drivers/remoteproc/Makefile         |   1 +
 drivers/remoteproc/rproc-uclass.c   | 128 ++++++++++++++++++
 drivers/remoteproc/stm32_copro.c    | 257 ++++++++++++++++++++++++++++++++++++
 include/dm/of_addr.h                |  18 +++
 include/dm/ofnode.h                 |  16 ++-
 include/dm/read.h                   |  20 ++-
 include/fdt_support.h               |   2 +
 include/remoteproc.h                |  32 ++++-
 16 files changed, 509 insertions(+), 3 deletions(-)
 create mode 100644 drivers/remoteproc/stm32_copro.c

-- 
2.7.4

[U-Boot] [PATCH 1/7] fdt: Introduce fdt_translate_dma_address()

[Fabien Dessenne]

Add the fdt_translate_dma_address() function to translate DMA address to
CPU address.
This function works the same way as fdt_translate_address(), with the
difference that the translation relies on the "dma-ranges" property
instead of the "ranges" property.

Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 common/fdt_support.c  | 6 ++++++
 include/fdt_support.h | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index 4e7cf6e..6ec0742 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -1279,6 +1279,12 @@ u64 fdt_translate_address(const void *blob, int node_offset,
 	return __of_translate_address(blob, node_offset, in_addr, "ranges");
 }
 
+u64 fdt_translate_dma_address(const void *blob, int node_offset,
+			      const fdt32_t *in_addr)
+{
+	return __of_translate_address(blob, node_offset, in_addr, "dma-ranges");
+}
+
 /**
  * fdt_node_offset_by_compat_reg: Find a node that matches compatiable and
  * who's reg property matches a physical cpu address
diff --git a/include/fdt_support.h b/include/fdt_support.h
index 27fe564..b92d8c0 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -220,6 +220,8 @@ static inline void fdt_fixup_mtdparts(void *fdt,
 void fdt_del_node_and_alias(void *blob, const char *alias);
 u64 fdt_translate_address(const void *blob, int node_offset,
 			  const __be32 *in_addr);
+u64 fdt_translate_dma_address(const void *blob, int node_offset,
+			      const __be32 *in_addr);
 int fdt_node_offset_by_compat_reg(void *blob, const char *compat,
 					phys_addr_t compat_off);
 int fdt_alloc_phandle(void *blob);
-- 
2.7.4

[U-Boot] [PATCH 2/7] dm: core: Introduce xxx_translate_dma_address()

[Fabien Dessenne]

Add the following functions to translate DMA address to CPU address:
- dev_translate_dma_address()
- ofnode_translate_dma_address()
- of_translate_dma_address()
These functions work the same way as xxx_translate_address(), with the
difference that the translation relies on the "dma-ranges" property
instead of the "ranges" property.

Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 drivers/core/of_addr.c |  4 ++++
 drivers/core/ofnode.c  |  8 ++++++++
 drivers/core/read.c    |  5 +++++
 include/dm/of_addr.h   | 18 ++++++++++++++++++
 include/dm/ofnode.h    | 16 +++++++++++++++-
 include/dm/read.h      | 20 +++++++++++++++++++-
 6 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/drivers/core/of_addr.c b/drivers/core/of_addr.c
index 1bfaaee..4e256d9 100644
--- a/drivers/core/of_addr.c
+++ b/drivers/core/of_addr.c
@@ -318,6 +318,10 @@ u64 of_translate_address(const struct device_node *dev, const __be32 *in_addr)
 	return __of_translate_address(dev, in_addr, "ranges");
 }
 
+u64 of_translate_dma_address(const struct device_node *dev, const __be32 *in_addr)
+{
+	return __of_translate_address(dev, in_addr, "dma-ranges");
+}
 
 static int __of_address_to_resource(const struct device_node *dev,
 		const __be32 *addrp, u64 size, unsigned int flags,
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index cc0c031..e68a735 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -763,6 +763,14 @@ u64 ofnode_translate_address(ofnode node, const fdt32_t *in_addr)
 		return fdt_translate_address(gd->fdt_blob, ofnode_to_offset(node), in_addr);
 }
 
+u64 ofnode_translate_dma_address(ofnode node, const fdt32_t *in_addr)
+{
+	if (ofnode_is_np(node))
+		return of_translate_dma_address(ofnode_to_np(node), in_addr);
+	else
+		return fdt_translate_dma_address(gd->fdt_blob, ofnode_to_offset(node), in_addr);
+}
+
 int ofnode_device_is_compatible(ofnode node, const char *compat)
 {
 	if (ofnode_is_np(node))
diff --git a/drivers/core/read.c b/drivers/core/read.c
index 6bda077..1a044b0 100644
--- a/drivers/core/read.c
+++ b/drivers/core/read.c
@@ -265,6 +265,11 @@ u64 dev_translate_address(struct udevice *dev, const fdt32_t *in_addr)
 	return ofnode_translate_address(dev_ofnode(dev), in_addr);
 }
 
+u64 dev_translate_dma_address(struct udevice *dev, const fdt32_t *in_addr)
+{
+	return ofnode_translate_dma_address(dev_ofnode(dev), in_addr);
+}
+
 int dev_read_alias_highest_id(const char *stem)
 {
 	if (of_live_active())
diff --git a/include/dm/of_addr.h b/include/dm/of_addr.h
index 12b1a99..3fa1ffc 100644
--- a/include/dm/of_addr.h
+++ b/include/dm/of_addr.h
@@ -27,6 +27,24 @@
 u64 of_translate_address(const struct device_node *no, const __be32 *in_addr);
 
 /**
+ * of_translate_dma_address() - translate a device-tree DMA address to a CPU
+ *				address
+ *
+ * Translate a DMA address from the device-tree into a CPU physical address,
+ * this walks up the tree and applies the various bus mappings on the way.
+ *
+ * Note: We consider that crossing any level with #size-cells == 0 to mean
+ * that translation is impossible (that is we are not dealing with a value
+ * that can be mapped to a cpu physical address). This is not really specified
+ * that way, but this is traditionally the way IBM at least do things
+ *
+ * @np: node to check
+ * @in_addr: pointer to input DMA address
+ * @return translated DMA address or OF_BAD_ADDR on error
+ */
+u64 of_translate_dma_address(const struct device_node *no, const __be32 *in_addr);
+
+/**
  * of_get_address() - obtain an address from a node
  *
  * Extract an address from a node, returns the region size and the address
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index d206ee2..07a3f93 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -751,7 +751,7 @@ ofnode ofnode_by_prop_value(ofnode from, const char *propname,
 	     node = ofnode_next_subnode(node))
 
 /**
- * ofnode_translate_address() - Tranlate a device-tree address
+ * ofnode_translate_address() - Translate a device-tree address
  *
  * Translate an address from the device-tree into a CPU physical address. This
  * function walks up the tree and applies the various bus mappings along the
@@ -765,6 +765,20 @@ ofnode ofnode_by_prop_value(ofnode from, const char *propname,
 u64 ofnode_translate_address(ofnode node, const fdt32_t *in_addr);
 
 /**
+ * ofnode_translate_dma_address() - Translate a device-tree DMA address
+ *
+ * Translate a DMA address from the device-tree into a CPU physical address.
+ * This function walks up the tree and applies the various bus mappings along
+ * the way.
+ *
+ * @ofnode: Device tree node giving the context in which to translate the
+ *          DMA address
+ * @in_addr: pointer to the DMA address to translate
+ * @return the translated DMA address; OF_BAD_ADDR on error
+ */
+u64 ofnode_translate_dma_address(ofnode node, const fdt32_t *in_addr);
+
+/**
  * ofnode_device_is_compatible() - check if the node is compatible with compat
  *
  * This allows to check whether the node is comaptible with the compat.
diff --git a/include/dm/read.h b/include/dm/read.h
index 60b727c..62d4be6 100644
--- a/include/dm/read.h
+++ b/include/dm/read.h
@@ -499,7 +499,7 @@ int dev_read_resource_byname(struct udevice *dev, const char *name,
 			     struct resource *res);
 
 /**
- * dev_translate_address() - Tranlate a device-tree address
+ * dev_translate_address() - Translate a device-tree address
  *
  * Translate an address from the device-tree into a CPU physical address.  This
  * function walks up the tree and applies the various bus mappings along the
@@ -512,6 +512,19 @@ int dev_read_resource_byname(struct udevice *dev, const char *name,
 u64 dev_translate_address(struct udevice *dev, const fdt32_t *in_addr);
 
 /**
+ * dev_translate_dma_address() - Translate a device-tree DMA address
+ *
+ * Translate a DMA address from the device-tree into a CPU physical address.
+ * This function walks up the tree and applies the various bus mappings along
+ * the way.
+ *
+ * @dev: device giving the context in which to translate the DMA address
+ * @in_addr: pointer to the DMA address to translate
+ * @return the translated DMA address; OF_BAD_ADDR on error
+ */
+u64 dev_translate_dma_address(struct udevice *dev, const fdt32_t *in_addr);
+
+/**
  * dev_read_alias_highest_id - Get highest alias id for the given stem
  * @stem:	Alias stem to be examined
  *
@@ -751,6 +764,11 @@ static inline u64 dev_translate_address(struct udevice *dev, const fdt32_t *in_a
 	return ofnode_translate_address(dev_ofnode(dev), in_addr);
 }
 
+static inline u64 dev_translate_dma_address(struct udevice *dev, const fdt32_t *in_addr)
+{
+	return ofnode_translate_dma_address(dev_ofnode(dev), in_addr);
+}
+
 static inline int dev_read_alias_highest_id(const char *stem)
 {
 	return fdtdec_get_alias_highest_id(gd->fdt_blob, stem);
-- 
2.7.4

[U-Boot] [PATCH 3/7] remoteproc: add da_to_pa ops

[Fabien Dessenne]

This patch introduces da_to_pa function to allow translation
between device address (remote processor view) and physical
address (main processor view).

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 include/remoteproc.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/remoteproc.h b/include/remoteproc.h
index a59dba8..58df11a 100644
--- a/include/remoteproc.h
+++ b/include/remoteproc.h
@@ -63,6 +63,8 @@ struct dm_rproc_uclass_pdata {
  *		Return 0 on success, 1 if not running, -ve on others errors
  * @ping:	Ping the remote device for basic communication check(optional)
  *		Return 0 on success, 1 if not responding, -ve on other errors
+ * @da_to_pa:   Return translated physical address (device address different
+ *              from physical address)
  */
 struct dm_rproc_ops {
 	int (*init)(struct udevice *dev);
@@ -72,6 +74,7 @@ struct dm_rproc_ops {
 	int (*reset)(struct udevice *dev);
 	int (*is_running)(struct udevice *dev);
 	int (*ping)(struct udevice *dev);
+	ulong (*da_to_pa)(struct udevice *dev, ulong da);
 };
 
 /* Accessor */
-- 
2.7.4

[U-Boot] [PATCH 4/7] remoteproc: add elf file load support

[Fabien Dessenne]

The current implementation supports only binary file load.
Add helpers to support ELF format (check validity, sanity check, and
load).
Note that since an ELF image is built for the remote processor, the load
function uses the da_to_pa ops to translate the addresses.

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 drivers/remoteproc/rproc-uclass.c | 128 ++++++++++++++++++++++++++++++++++++++
 include/remoteproc.h              |  29 ++++++++-
 2 files changed, 156 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/rproc-uclass.c b/drivers/remoteproc/rproc-uclass.c
index c8a41a6..ac5f9e2 100644
--- a/drivers/remoteproc/rproc-uclass.c
+++ b/drivers/remoteproc/rproc-uclass.c
@@ -5,6 +5,7 @@
  */
 #define pr_fmt(fmt) "%s: " fmt, __func__
 #include <common.h>
+#include <elf.h>
 #include <errno.h>
 #include <fdtdec.h>
 #include <malloc.h>
@@ -291,6 +292,133 @@ int rproc_dev_init(int id)
 	return ret;
 }
 
+/*
+ * Determine if a valid ELF image exists at the given memory location.
+ * First look at the ELF header magic field, then make sure that it is
+ * executable.
+ */
+bool rproc_elf_is_valid(unsigned long addr, int size)
+{
+	Elf32_Ehdr *ehdr; /* Elf header structure pointer */
+
+	ehdr = (Elf32_Ehdr *)addr;
+
+	if (!IS_ELF(*ehdr) || size <= sizeof(Elf32_Ehdr)) {
+		pr_err("No elf image@address 0x%08lx\n", addr);
+		return false;
+	}
+
+	if (ehdr->e_type != ET_EXEC) {
+		pr_err("Not a 32-bit elf image at address 0x%08lx\n", addr);
+		return false;
+	}
+
+	return true;
+}
+
+/* Basic function to verify ELF image format */
+int rproc_elf_sanity_check(ulong addr, ulong size)
+{
+	Elf32_Ehdr *ehdr;
+	char class;
+
+	if (!addr) {
+		pr_err("Invalid fw address?\n");
+		return -EINVAL;
+	}
+
+	if (size < sizeof(Elf32_Ehdr)) {
+		pr_err("Image is too small\n");
+		return -EINVAL;
+	}
+
+	ehdr = (Elf32_Ehdr *)addr;
+
+	/* We only support ELF32@this point */
+	class = ehdr->e_ident[EI_CLASS];
+	if (class != ELFCLASS32) {
+		pr_err("Unsupported class: %d\n", class);
+		return -EINVAL;
+	}
+
+	/* We assume the firmware has the same endianness as the host */
+# ifdef __LITTLE_ENDIAN
+	if (ehdr->e_ident[EI_DATA] != ELFDATA2LSB) {
+# else /* BIG ENDIAN */
+	if (ehdr->e_ident[EI_DATA] != ELFDATA2MSB) {
+# endif
+		pr_err("Unsupported firmware endianness\n");
+		return -EINVAL;
+	}
+
+	if (size < ehdr->e_shoff + sizeof(Elf32_Shdr)) {
+		pr_err("Image is too small\n");
+		return -EINVAL;
+	}
+
+	if (memcmp(ehdr->e_ident, ELFMAG, SELFMAG)) {
+		pr_err("Image is corrupted (bad magic)\n");
+		return -EINVAL;
+	}
+
+	if (ehdr->e_phnum == 0) {
+		pr_err("No loadable segments\n");
+		return -EINVAL;
+	}
+
+	if (ehdr->e_phoff > size) {
+		pr_err("Firmware size is too small\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/* A very simple elf loader, assumes the image is valid */
+int rproc_elf_load_image(struct udevice *dev, unsigned long addr)
+{
+	Elf32_Ehdr *ehdr; /* Elf header structure pointer */
+	Elf32_Phdr *phdr; /* Program header structure pointer */
+	const struct dm_rproc_ops *ops;
+	unsigned int i;
+
+	ehdr = (Elf32_Ehdr *)addr;
+	phdr = (Elf32_Phdr *)(addr + ehdr->e_phoff);
+
+	ops = rproc_get_ops(dev);
+	if (!ops) {
+		dev_dbg(dev, "Driver has no ops?\n");
+		return -EINVAL;
+	}
+
+	/* Load each program header */
+	for (i = 0; i < ehdr->e_phnum; ++i) {
+		void *dst = (void *)(uintptr_t)phdr->p_paddr;
+		void *src = (void *)addr + phdr->p_offset;
+
+		if (phdr->p_type != PT_LOAD)
+			continue;
+
+		if (ops->da_to_pa)
+			dst = (void *)ops->da_to_pa(dev, (ulong)dst);
+
+		dev_dbg(dev, "Loading phdr %i to 0x%p (%i bytes)\n",
+			i, dst, phdr->p_filesz);
+		if (phdr->p_filesz)
+			memcpy(dst, src, phdr->p_filesz);
+		if (phdr->p_filesz != phdr->p_memsz)
+			memset(dst + phdr->p_filesz, 0x00,
+			       phdr->p_memsz - phdr->p_filesz);
+		flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
+			    roundup((unsigned long)dst + phdr->p_filesz,
+				    ARCH_DMA_MINALIGN) -
+			    rounddown((unsigned long)dst, ARCH_DMA_MINALIGN));
+		++phdr;
+	}
+
+	return 0;
+}
+
 int rproc_load(int id, ulong addr, ulong size)
 {
 	struct udevice *dev = NULL;
diff --git a/include/remoteproc.h b/include/remoteproc.h
index 58df11a..5cc04e6 100644
--- a/include/remoteproc.h
+++ b/include/remoteproc.h
@@ -104,7 +104,7 @@ int rproc_dev_init(int id);
 bool rproc_is_initialized(void);
 
 /**
- * rproc_load() - load binary to a remote processor
+ * rproc_load() - load binary or elf to a remote processor
  * @id:		id of the remote processor
  * @addr:	address in memory where the binary image is located
  * @size:	size of the binary image
@@ -159,6 +159,27 @@ int rproc_ping(int id);
  * Return: 0 if all ok, else appropriate error value.
  */
 int rproc_is_running(int id);
+
+/**
+ * rproc_elf_is_valid() - check if an image is a valid ELF one
+ * @addr:	image address
+ * @size:	image size
+ */
+bool rproc_elf_is_valid(unsigned long addr, int size);
+
+/**
+ * rproc_elf_sanity_check() - verify an ELF image format
+ * @addr:	ELF image address
+ * @size:	ELF image size
+ */
+int rproc_elf_sanity_check(ulong addr, ulong size);
+
+/**
+ * rproc_elf_load_image() - load an ELF image
+ * @dev:	device loading the ELF image
+ * @addr:	valid ELF image address
+ */
+int rproc_elf_load_image(struct udevice *dev, unsigned long addr);
 #else
 static inline int rproc_init(void) { return -ENOSYS; }
 static inline int rproc_dev_init(int id) { return -ENOSYS; }
@@ -169,6 +190,12 @@ static inline int rproc_stop(int id) { return -ENOSYS; }
 static inline int rproc_reset(int id) { return -ENOSYS; }
 static inline int rproc_ping(int id) { return -ENOSYS; }
 static inline int rproc_is_running(int id) { return -ENOSYS; }
+static inline bool rproc_elf_is_valid(unsigned long addr,
+				      int size) { return -ENOSYS; }
+static inline int rproc_elf_sanity_check(ulong addr,
+					 ulong size) { return -ENOSYS; }
+static inline int rproc_elf_load_image(struct udevice *dev,
+				       unsigned long addr)  { return -ENOSYS; }
 #endif
 
 #endif	/* _RPROC_H_ */
-- 
2.7.4

[U-Boot] [PATCH 5/7] remoteproc: Introduce STM32 Cortex-M4 remoteproc driver

[Fabien Dessenne]

This patch introduces support of Cortex-M4 remote processor for STM32
MCU and MPU families.

Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 drivers/remoteproc/Kconfig       |  10 ++
 drivers/remoteproc/Makefile      |   1 +
 drivers/remoteproc/stm32_copro.c | 257 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 268 insertions(+)
 create mode 100644 drivers/remoteproc/stm32_copro.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 9eb532b..fa6f111 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -40,6 +40,16 @@ config REMOTEPROC_SANDBOX
 	  Say 'y' here to add support for test processor which does dummy
 	  operations for sandbox platform.
 
+config REMOTEPROC_STM32_COPRO
+	bool "Support for STM32 coprocessor"
+	select REMOTEPROC
+	depends on DM
+	depends on ARCH_STM32MP
+	depends on OF_CONTROL
+	help
+	  Say 'y' here to add support for STM32 Cortex-M4 coprocessors via the
+	  remoteproc framework.
+
 config REMOTEPROC_TI_POWER
 	bool "Support for TI Power processor"
 	select REMOTEPROC
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 77eb708..5b120c1 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -10,4 +10,5 @@ obj-$(CONFIG_$(SPL_)REMOTEPROC) += rproc-uclass.o
 obj-$(CONFIG_K3_SYSTEM_CONTROLLER) += k3_system_controller.o
 obj-$(CONFIG_REMOTEPROC_K3) += k3_rproc.o
 obj-$(CONFIG_REMOTEPROC_SANDBOX) += sandbox_testproc.o
+obj-$(CONFIG_REMOTEPROC_STM32_COPRO) += stm32_copro.o
 obj-$(CONFIG_REMOTEPROC_TI_POWER) += ti_power_proc.o
diff --git a/drivers/remoteproc/stm32_copro.c b/drivers/remoteproc/stm32_copro.c
new file mode 100644
index 0000000..5abdf30
--- /dev/null
+++ b/drivers/remoteproc/stm32_copro.c
@@ -0,0 +1,257 @@
+// SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause
+/*
+ * Copyright (C) 2019, STMicroelectronics - All Rights Reserved
+ */
+#define pr_fmt(fmt) "%s: " fmt, __func__
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <fdtdec.h>
+#include <regmap.h>
+#include <remoteproc.h>
+#include <reset.h>
+#include <syscon.h>
+
+#define RCC_GCR_HOLD_BOOT	0
+#define RCC_GCR_RELEASE_BOOT	1
+
+/**
+ * struct stm32_copro_privdata - power processor private data
+ * @reset_ctl:		reset controller handle
+ * @hold_boot_regmap:	regmap for remote processor reset hold boot
+ * @hold_boot_offset:	offset of the register controlling the hold boot setting
+ * @hold_boot_mask:	bitmask of the register for the hold boot field
+ * @is_running:		is the remote processor running
+ */
+struct stm32_copro_privdata {
+	struct reset_ctl reset_ctl;
+	struct regmap *hold_boot_regmap;
+	uint hold_boot_offset;
+	uint hold_boot_mask;
+	bool is_running;
+};
+
+/**
+ * stm32_copro_probe() - Basic probe
+ * @dev:	corresponding STM32 remote processor device
+ *
+ * Return: 0 if all went ok, else corresponding -ve error
+ */
+static int stm32_copro_probe(struct udevice *dev)
+{
+	struct stm32_copro_privdata *priv;
+	struct regmap *regmap;
+	const fdt32_t *cell;
+	int len, ret;
+
+	priv = dev_get_priv(dev);
+
+	regmap = syscon_regmap_lookup_by_phandle(dev, "st,syscfg-holdboot");
+	if (IS_ERR(regmap)) {
+		dev_err(dev, "unable to find holdboot regmap (%ld)\n",
+			PTR_ERR(regmap));
+		return PTR_ERR(regmap);
+	}
+
+	cell = dev_read_prop(dev, "st,syscfg-holdboot", &len);
+	if (len < 3 * sizeof(fdt32_t)) {
+		dev_err(dev, "holdboot offset and mask not available\n");
+		return -EINVAL;
+	}
+
+	priv->hold_boot_regmap = regmap;
+	priv->hold_boot_offset = fdtdec_get_number(cell + 1, 1);
+	priv->hold_boot_mask = fdtdec_get_number(cell + 2, 1);
+
+	ret = reset_get_by_index(dev, 0, &priv->reset_ctl);
+	if (ret) {
+		dev_err(dev, "failed to get reset (%d)\n", ret);
+		return ret;
+	}
+
+	dev_dbg(dev, "probed\n");
+
+	return 0;
+}
+
+/**
+ * stm32_copro_set_hold_boot() - Hold boot bit management
+ * @dev:	corresponding STM32 remote processor device
+ * @hold:	hold boot value
+ *
+ * Return: 0 if all went ok, else corresponding -ve error
+ */
+static int stm32_copro_set_hold_boot(struct udevice *dev, bool hold)
+{
+	struct stm32_copro_privdata *priv;
+	uint val;
+	int ret;
+
+	priv = dev_get_priv(dev);
+
+	val = hold ? RCC_GCR_HOLD_BOOT : RCC_GCR_RELEASE_BOOT;
+
+	/*
+	 * Note: shall run an SMC call (STM32_SMC_RCC) if platform is secured.
+	 * To be updated when the code for this SMC service is available which
+	 * is not the case for the time being.
+	 */
+	ret = regmap_update_bits(priv->hold_boot_regmap, priv->hold_boot_offset,
+				 priv->hold_boot_mask, val);
+	if (ret)
+		dev_err(dev, "failed to set hold boot\n");
+
+	return ret;
+}
+
+/**
+ * stm32_copro_da_to_pa() - Convert DeviceAddress to PhysicalAddress
+ * @dev:	corresponding STM32 remote processor device
+ * @da:		device address
+ *
+ * Return: 0 if all went ok, else corresponding -ve error
+ */
+static ulong stm32_copro_da_to_pa(struct udevice *dev, ulong da)
+{
+	fdt32_t in_addr = cpu_to_be32(da);
+
+	return dev_translate_dma_address(dev, &in_addr);
+}
+
+/**
+ * stm32_copro_load() - Loadup the STM32 remote processor
+ * @dev:	corresponding STM32 remote processor device
+ * @addr:	Address in memory where image is stored
+ * @size:	Size in bytes of the image
+ *
+ * Return: 0 if all went ok, else corresponding -ve error
+ */
+static int stm32_copro_load(struct udevice *dev, ulong addr, ulong size)
+{
+	struct stm32_copro_privdata *priv;
+	int ret;
+
+	priv = dev_get_priv(dev);
+
+	ret = stm32_copro_set_hold_boot(dev, true);
+	if (ret)
+		return ret;
+
+	ret = reset_assert(&priv->reset_ctl);
+	if (ret) {
+		dev_err(dev, "Unable to assert reset line (ret=%d)\n", ret);
+		return ret;
+	}
+
+	/* Support only ELF image */
+	if (!rproc_elf_is_valid(addr, size))
+		return -EINVAL;
+
+	if (!rproc_elf_sanity_check(addr, size))
+		return -EINVAL;
+
+	return rproc_elf_load_image(dev, addr);
+}
+
+/**
+ * stm32_copro_start() - Start the STM32 remote processor
+ * @dev:	corresponding STM32 remote processor device
+ *
+ * Return: 0 if all went ok, else corresponding -ve error
+ */
+static int stm32_copro_start(struct udevice *dev)
+{
+	struct stm32_copro_privdata *priv;
+	int ret;
+
+	priv = dev_get_priv(dev);
+
+	/* move hold boot from true to false start the copro */
+	ret = stm32_copro_set_hold_boot(dev, false);
+	if (ret)
+		return ret;
+
+	/*
+	 * Once copro running, reset hold boot flag to avoid copro
+	 * rebooting autonomously
+	 */
+	ret = stm32_copro_set_hold_boot(dev, true);
+	priv->is_running = !ret;
+	return ret;
+}
+
+/**
+ * stm32_copro_reset() - Reset the STM32 remote processor
+ * @dev:	corresponding STM32 remote processor device
+ *
+ * Return: 0 if all went ok, else corresponding -ve error
+ */
+static int stm32_copro_reset(struct udevice *dev)
+{
+	struct stm32_copro_privdata *priv;
+	int ret;
+
+	priv = dev_get_priv(dev);
+
+	ret = stm32_copro_set_hold_boot(dev, true);
+	if (ret)
+		return ret;
+
+	ret = reset_assert(&priv->reset_ctl);
+	if (ret) {
+		dev_err(dev, "Unable to assert reset line (ret=%d)\n", ret);
+		return ret;
+	}
+
+	priv->is_running = false;
+
+	return 0;
+}
+
+/**
+ * stm32_copro_stop() - Stop the STM32 remote processor
+ * @dev:	corresponding STM32 remote processor device
+ *
+ * Return: 0 if all went ok, else corresponding -ve error
+ */
+static int stm32_copro_stop(struct udevice *dev)
+{
+	return stm32_copro_reset(dev);
+}
+
+/**
+ * stm32_copro_is_running() - Is the STM32 remote processor running
+ * @dev:	corresponding STM32 remote processor device
+ *
+ * Return: 1 if the remote processor is running, 0 otherwise
+ */
+static int stm32_copro_is_running(struct udevice *dev)
+{
+	struct stm32_copro_privdata *priv;
+
+	priv = dev_get_priv(dev);
+	return priv->is_running;
+}
+
+static const struct dm_rproc_ops stm32_copro_ops = {
+	.load = stm32_copro_load,
+	.start = stm32_copro_start,
+	.stop =  stm32_copro_stop,
+	.reset = stm32_copro_reset,
+	.is_running = stm32_copro_is_running,
+	.da_to_pa = stm32_copro_da_to_pa,
+};
+
+static const struct udevice_id stm32_copro_ids[] = {
+	{.compatible = "st,stm32mp1-rproc"},
+	{}
+};
+
+U_BOOT_DRIVER(stm32_copro) = {
+	.name = "stm32_m4_proc",
+	.of_match = stm32_copro_ids,
+	.id = UCLASS_REMOTEPROC,
+	.ops = &stm32_copro_ops,
+	.probe = stm32_copro_probe,
+	.priv_auto_alloc_size = sizeof(struct stm32_copro_privdata),
+};
-- 
2.7.4

[U-Boot] [PATCH 6/7] MAINTAINERS: Add stm32 remoteproc driver

[Fabien Dessenne]

Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 33fd465..5c505d9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -309,6 +309,7 @@ F:	drivers/power/pmic/stpmic1.c
 F:	drivers/power/regulator/stm32-vrefbuf.c
 F:	drivers/power/regulator/stpmic1.c
 F:	drivers/ram/stm32mp1/
+F:	drivers/remoteproc/stm32_copro.c
 F:	drivers/misc/stm32_rcc.c
 F:	drivers/reset/stm32-reset.c
 F:	drivers/spi/stm32_qspi.c
-- 
2.7.4

[U-Boot] [PATCH 7/7] configs: stm32mp15: enable stm32 remoteproc

[Fabien Dessenne]

Activate the remote processor support for stm32mp15 configs.

Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 configs/stm32mp15_basic_defconfig   | 2 ++
 configs/stm32mp15_trusted_defconfig | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/configs/stm32mp15_basic_defconfig b/configs/stm32mp15_basic_defconfig
index 0ea9dff..5185072 100644
--- a/configs/stm32mp15_basic_defconfig
+++ b/configs/stm32mp15_basic_defconfig
@@ -67,6 +67,8 @@ CONFIG_DM_REGULATOR_FIXED=y
 CONFIG_DM_REGULATOR_GPIO=y
 CONFIG_DM_REGULATOR_STM32_VREFBUF=y
 CONFIG_DM_REGULATOR_STPMIC1=y
+CONFIG_REMOTEPROC=y
+CONFIG_REMOTEPROC_STM32_COPRO=y
 CONFIG_SERIAL_RX_BUFFER=y
 CONFIG_STM32_SERIAL=y
 CONFIG_USB=y
diff --git a/configs/stm32mp15_trusted_defconfig b/configs/stm32mp15_trusted_defconfig
index 3c2bb75..037d7e8 100644
--- a/configs/stm32mp15_trusted_defconfig
+++ b/configs/stm32mp15_trusted_defconfig
@@ -57,6 +57,8 @@ CONFIG_DM_REGULATOR_FIXED=y
 CONFIG_DM_REGULATOR_GPIO=y
 CONFIG_DM_REGULATOR_STM32_VREFBUF=y
 CONFIG_DM_REGULATOR_STPMIC1=y
+CONFIG_REMOTEPROC=y
+CONFIG_REMOTEPROC_STM32_COPRO=y
 CONFIG_SERIAL_RX_BUFFER=y
 CONFIG_STM32_SERIAL=y
 CONFIG_USB=y
-- 
2.7.4

[U-Boot] [PATCH 1/7] fdt: Introduce fdt_translate_dma_address()

[Simon Glass]

Hi Fabien,

On Wed, 22 May 2019 at 02:07, Fabien Dessenne <fabien.dessenne@st.com> wrote:
>
> Add the fdt_translate_dma_address() function to translate DMA address to
> CPU address.
> This function works the same way as fdt_translate_address(), with the
> difference that the translation relies on the "dma-ranges" property
> instead of the "ranges" property.
>
> Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
> ---
>  common/fdt_support.c  | 6 ++++++
>  include/fdt_support.h | 2 ++
>  2 files changed, 8 insertions(+)

Please can you add a simple test for this, and also a function comment
in the header file?

Regards,
Simon

[U-Boot] [PATCH 2/7] dm: core: Introduce xxx_translate_dma_address()

[Simon Glass]

Hi Fabien,

On Wed, 22 May 2019 at 02:07, Fabien Dessenne <fabien.dessenne@st.com> wrote:
>
> Add the following functions to translate DMA address to CPU address:
> - dev_translate_dma_address()
> - ofnode_translate_dma_address()
> - of_translate_dma_address()
> These functions work the same way as xxx_translate_address(), with the
> difference that the translation relies on the "dma-ranges" property
> instead of the "ranges" property.
>

Looks good, but again, needs a test.

> Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
> ---
>  drivers/core/of_addr.c |  4 ++++
>  drivers/core/ofnode.c  |  8 ++++++++
>  drivers/core/read.c    |  5 +++++
>  include/dm/of_addr.h   | 18 ++++++++++++++++++
>  include/dm/ofnode.h    | 16 +++++++++++++++-
>  include/dm/read.h      | 20 +++++++++++++++++++-
>  6 files changed, 69 insertions(+), 2 deletions(-)
>

Regards,
Simon

[U-Boot] [PATCH 3/7] remoteproc: add da_to_pa ops

[Simon Glass]

Hi Fabien,

On Wed, 22 May 2019 at 02:07, Fabien Dessenne <fabien.dessenne@st.com> wrote:
>
> This patch introduces da_to_pa function to allow translation
> between device address (remote processor view) and physical
> address (main processor view).
>
> Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
> Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
> ---
>  include/remoteproc.h | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/include/remoteproc.h b/include/remoteproc.h
> index a59dba8..58df11a 100644
> --- a/include/remoteproc.h
> +++ b/include/remoteproc.h
> @@ -63,6 +63,8 @@ struct dm_rproc_uclass_pdata {
>   *             Return 0 on success, 1 if not running, -ve on others errors
>   * @ping:      Ping the remote device for basic communication check(optional)
>   *             Return 0 on success, 1 if not responding, -ve on other errors
> + * @da_to_pa:   Return translated physical address (device address different
> + *              from physical address)

How about device_addr_to_phys()?

Also please add a full function comment here and below. This uclass
header seems to be delinquent.

>   */
>  struct dm_rproc_ops {
>         int (*init)(struct udevice *dev);
> @@ -72,6 +74,7 @@ struct dm_rproc_ops {
>         int (*reset)(struct udevice *dev);
>         int (*is_running)(struct udevice *dev);
>         int (*ping)(struct udevice *dev);
> +       ulong (*da_to_pa)(struct udevice *dev, ulong da);
>  };
>
>  /* Accessor */
> --
> 2.7.4
>

Regards,
Simon

[U-Boot] [PATCH 4/7] remoteproc: add elf file load support

[Simon Glass]

Hi Fabien,

On Wed, 22 May 2019 at 02:07, Fabien Dessenne <fabien.dessenne@st.com> wrote:
>
> The current implementation supports only binary file load.
> Add helpers to support ELF format (check validity, sanity check, and
> load).
> Note that since an ELF image is built for the remote processor, the load
> function uses the da_to_pa ops to translate the addresses.
>
> Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
> Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
> ---
>  drivers/remoteproc/rproc-uclass.c | 128 ++++++++++++++++++++++++++++++++++++++
>  include/remoteproc.h              |  29 ++++++++-
>  2 files changed, 156 insertions(+), 1 deletion(-)

It doesn't look like we can easily make use of the existing ELF loader.

But please add a test for this in test/dm/remoteproc.c and see below.



>
> diff --git a/drivers/remoteproc/rproc-uclass.c b/drivers/remoteproc/rproc-uclass.c
> index c8a41a6..ac5f9e2 100644
> --- a/drivers/remoteproc/rproc-uclass.c
> +++ b/drivers/remoteproc/rproc-uclass.c
> @@ -5,6 +5,7 @@
>   */
>  #define pr_fmt(fmt) "%s: " fmt, __func__
>  #include <common.h>
> +#include <elf.h>
>  #include <errno.h>
>  #include <fdtdec.h>
>  #include <malloc.h>
> @@ -291,6 +292,133 @@ int rproc_dev_init(int id)
>         return ret;
>  }
>
> +/*
> + * Determine if a valid ELF image exists at the given memory location.
> + * First look at the ELF header magic field, then make sure that it is
> + * executable.
> + */
> +bool rproc_elf_is_valid(unsigned long addr, int size)
> +{
> +       Elf32_Ehdr *ehdr; /* Elf header structure pointer */
> +
> +       ehdr = (Elf32_Ehdr *)addr;
> +
> +       if (!IS_ELF(*ehdr) || size <= sizeof(Elf32_Ehdr)) {
> +               pr_err("No elf image at address 0x%08lx\n", addr);
> +               return false;
> +       }
> +
> +       if (ehdr->e_type != ET_EXEC) {
> +               pr_err("Not a 32-bit elf image at address 0x%08lx\n", addr);
> +               return false;
> +       }
> +
> +       return true;
> +}
> +
> +/* Basic function to verify ELF image format */
> +int rproc_elf_sanity_check(ulong addr, ulong size)
> +{
> +       Elf32_Ehdr *ehdr;
> +       char class;
> +
> +       if (!addr) {
> +               pr_err("Invalid fw address?\n");
> +               return -EINVAL;

EFAULT ?

> +       }
> +
> +       if (size < sizeof(Elf32_Ehdr)) {
> +               pr_err("Image is too small\n");
> +               return -EINVAL;

ENOSPC?

> +       }
> +
> +       ehdr = (Elf32_Ehdr *)addr;
> +
> +       /* We only support ELF32 at this point */
> +       class = ehdr->e_ident[EI_CLASS];
> +       if (class != ELFCLASS32) {
> +               pr_err("Unsupported class: %d\n", class);
> +               return -EINVAL;

EPROTONOSUPP?

> +       }
> +
> +       /* We assume the firmware has the same endianness as the host */
> +# ifdef __LITTLE_ENDIAN
> +       if (ehdr->e_ident[EI_DATA] != ELFDATA2LSB) {
> +# else /* BIG ENDIAN */
> +       if (ehdr->e_ident[EI_DATA] != ELFDATA2MSB) {
> +# endif
> +               pr_err("Unsupported firmware endianness\n");
> +               return -EINVAL;
> +       }
> +
> +       if (size < ehdr->e_shoff + sizeof(Elf32_Shdr)) {
> +               pr_err("Image is too small\n");
> +               return -EINVAL;

ESPC

> +       }
> +
> +       if (memcmp(ehdr->e_ident, ELFMAG, SELFMAG)) {
> +               pr_err("Image is corrupted (bad magic)\n");
> +               return -EINVAL;

EBADF

> +       }
> +
> +       if (ehdr->e_phnum == 0) {
> +               pr_err("No loadable segments\n");
> +               return -EINVAL;
> +       }
> +
> +       if (ehdr->e_phoff > size) {
> +               pr_err("Firmware size is too small\n");
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +

They are just suggestions, but please try to return useful numbers. In
general it should be possible to see what went wrong without needing
to output text.

> +/* A very simple elf loader, assumes the image is valid */
> +int rproc_elf_load_image(struct udevice *dev, unsigned long addr)
> +{
> +       Elf32_Ehdr *ehdr; /* Elf header structure pointer */
> +       Elf32_Phdr *phdr; /* Program header structure pointer */
> +       const struct dm_rproc_ops *ops;
> +       unsigned int i;
> +
> +       ehdr = (Elf32_Ehdr *)addr;
> +       phdr = (Elf32_Phdr *)(addr + ehdr->e_phoff);
> +
> +       ops = rproc_get_ops(dev);
> +       if (!ops) {
> +               dev_dbg(dev, "Driver has no ops?\n");
> +               return -EINVAL;
> +       }

This is not allowed, so you don't need to check for it.

> +
> +       /* Load each program header */
> +       for (i = 0; i < ehdr->e_phnum; ++i) {
> +               void *dst = (void *)(uintptr_t)phdr->p_paddr;
> +               void *src = (void *)addr + phdr->p_offset;
> +
> +               if (phdr->p_type != PT_LOAD)
> +                       continue;
> +
> +               if (ops->da_to_pa)
> +                       dst = (void *)ops->da_to_pa(dev, (ulong)dst);
> +
> +               dev_dbg(dev, "Loading phdr %i to 0x%p (%i bytes)\n",
> +                       i, dst, phdr->p_filesz);
> +               if (phdr->p_filesz)
> +                       memcpy(dst, src, phdr->p_filesz);
> +               if (phdr->p_filesz != phdr->p_memsz)
> +                       memset(dst + phdr->p_filesz, 0x00,
> +                              phdr->p_memsz - phdr->p_filesz);
> +               flush_cache(rounddown((unsigned long)dst, ARCH_DMA_MINALIGN),
> +                           roundup((unsigned long)dst + phdr->p_filesz,
> +                                   ARCH_DMA_MINALIGN) -
> +                           rounddown((unsigned long)dst, ARCH_DMA_MINALIGN));
> +               ++phdr;
> +       }
> +
> +       return 0;
> +}
> +
>  int rproc_load(int id, ulong addr, ulong size)
>  {
>         struct udevice *dev = NULL;
> diff --git a/include/remoteproc.h b/include/remoteproc.h
> index 58df11a..5cc04e6 100644
> --- a/include/remoteproc.h
> +++ b/include/remoteproc.h
> @@ -104,7 +104,7 @@ int rproc_dev_init(int id);
>  bool rproc_is_initialized(void);
>
>  /**
> - * rproc_load() - load binary to a remote processor
> + * rproc_load() - load binary or elf to a remote processor
>   * @id:                id of the remote processor
>   * @addr:      address in memory where the binary image is located
>   * @size:      size of the binary image
> @@ -159,6 +159,27 @@ int rproc_ping(int id);
>   * Return: 0 if all ok, else appropriate error value.

What does ok mean? Running?

>   */
>  int rproc_is_running(int id);
> +
> +/**
> + * rproc_elf_is_valid() - check if an image is a valid ELF one

In what sense?

> + * @addr:      image address

Is this address in the remote space?

In general your comments are a bit brief

> + * @size:      image size

@return - also please use an int return (e.g. 0 is valid and -ve error
means not). You could rename to rproc_elf_check_valid()

> + */
> +bool rproc_elf_is_valid(unsigned long addr, int size);
> +
> +/**
> + * rproc_elf_sanity_check() - verify an ELF image format

What is different about this from the above functions?

> + * @addr:      ELF image address
> + * @size:      ELF image size
> + */
> +int rproc_elf_sanity_check(ulong addr, ulong size);
> +
> +/**
> + * rproc_elf_load_image() - load an ELF image
> + * @dev:       device loading the ELF image
> + * @addr:      valid ELF image address

@return
> + */
> +int rproc_elf_load_image(struct udevice *dev, unsigned long addr);
>  #else
>  static inline int rproc_init(void) { return -ENOSYS; }
>  static inline int rproc_dev_init(int id) { return -ENOSYS; }
> @@ -169,6 +190,12 @@ static inline int rproc_stop(int id) { return -ENOSYS; }
>  static inline int rproc_reset(int id) { return -ENOSYS; }
>  static inline int rproc_ping(int id) { return -ENOSYS; }
>  static inline int rproc_is_running(int id) { return -ENOSYS; }
> +static inline bool rproc_elf_is_valid(unsigned long addr,
> +                                     int size) { return -ENOSYS; }
> +static inline int rproc_elf_sanity_check(ulong addr,
> +                                        ulong size) { return -ENOSYS; }
> +static inline int rproc_elf_load_image(struct udevice *dev,
> +                                      unsigned long addr)  { return -ENOSYS; }
>  #endif
>
>  #endif /* _RPROC_H_ */
> --
> 2.7.4
>

Regards,
Simon