[PATCH 0/8] stm32 m4 remoteproc on STM32MP157c

[PATCH 0/8] stm32 m4 remoteproc on STM32MP157c

[Fabien Dessenne]

STMicrolectronics STM32MP157 MPU are based on a Dual Arm Cortex-A7 core and a
Cortex-M4.
This patchset adds the support of the stm32_rproc driver allowing to control
the M4 remote processor.

Fabien Dessenne (8):
  dt-bindings: stm32: add bindings for ML-AHB interconnect
  dt-bindings: remoteproc: add bindings for stm32 remote processor
    driver
  remoteproc: stm32: add an ST stm32_rproc driver
  ARM: dts: stm32: add m4 remoteproc support on STM32MP157c
  ARM: dts: stm32: declare copro reserved memories on STM32MP157c-ed1
  ARM: dts: stm32: enable m4 coprocessor support on STM32MP157c-ed1
  ARM: dts: stm32: declare copro reserved memories on STM32MP157a-dk1
  ARM: dts: stm32: enable m4 coprocessor support on STM32MP157a-dk1

 .../devicetree/bindings/arm/stm32/mlahb.txt        |  30 +
 .../devicetree/bindings/remoteproc/stm32-rproc.txt |  67 +++
 arch/arm/boot/dts/stm32mp157a-dk1.dts              |  54 ++
 arch/arm/boot/dts/stm32mp157c-ed1.dts              |  54 ++
 arch/arm/boot/dts/stm32mp157c.dtsi                 |  21 +
 drivers/remoteproc/Kconfig                         |  15 +
 drivers/remoteproc/Makefile                        |   1 +
 drivers/remoteproc/stm32_rproc.c                   | 624 +++++++++++++++++++++
 8 files changed, 866 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/stm32/mlahb.txt
 create mode 100644 Documentation/devicetree/bindings/remoteproc/stm32-rproc.txt
 create mode 100644 drivers/remoteproc/stm32_rproc.c

-- 
2.7.4

[PATCH 1/8] dt-bindings: stm32: add bindings for ML-AHB interconnect

[Fabien Dessenne]

Document the ML-AHB interconnect for stm32 SoCs.

Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 .../devicetree/bindings/arm/stm32/mlahb.txt        | 30 ++++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/stm32/mlahb.txt

diff --git a/Documentation/devicetree/bindings/arm/stm32/mlahb.txt b/Documentation/devicetree/bindings/arm/stm32/mlahb.txt
new file mode 100644
index 0000000..880cb38
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/stm32/mlahb.txt
@@ -0,0 +1,30 @@
+ML-AHB interconnect bindings
+
+These bindings describe the STM32 SoCs ML-AHB interconnect bus which connects
+a Cortex-M subsystem with dedicated memories.
+
+Required properties:
+- compatible: should be "simple-bus"
+- ranges: describes memory addresses translation between the local CPU and the
+	   remote Cortex-M processor. Each memory region, is declared with 3
+	   parameters:
+		 - param 1: device base address (Cortex-M processor address)
+		 - param 2: physical base address (local CPU address)
+		 - param 3: size of the memory region.
+
+The Cortex-M remote processor accessed via the mlahb interconnect is described
+by a child node.
+
+Example:
+mlahb: mlahb@0 {
+	compatible = "simple-bus";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	ranges = <0x00000000 0x38000000 0x10000>,
+		 <0x10000000 0x10000000 0x60000>,
+		 <0x30000000 0x30000000 0x60000>;
+
+	m4_rproc: m4@0 {
+		...
+	};
+};
-- 
2.7.4

[PATCH 2/8] dt-bindings: remoteproc: add bindings for stm32 remote processor driver

[Fabien Dessenne]

Add the device tree bindings document for the stm32 remoteproc devices.

Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 .../devicetree/bindings/remoteproc/stm32-rproc.txt | 67 ++++++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/stm32-rproc.txt

diff --git a/Documentation/devicetree/bindings/remoteproc/stm32-rproc.txt b/Documentation/devicetree/bindings/remoteproc/stm32-rproc.txt
new file mode 100644
index 0000000..cc90c88
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/stm32-rproc.txt
@@ -0,0 +1,67 @@
+STMicroelectronics STM32 Remoteproc
+-----------------------------------
+This document defines the binding for the remoteproc component that loads and
+boots firmwares on the ST32MP family chipset.
+
+Required properties:
+- compatible:	Must be "st,stm32mp1-rproc"
+- reg:		Address ranges of the remote processor dedicated memories.
+		The parent node should provide an appropriate ranges property
+		for properly translating these into bus addresses.
+- resets:	Reference to a reset controller asserting the remote processor.
+- reset-names:	Must be "mcu_rst"
+- st,syscfg-holdboot: Reference to the system configuration which holds the
+		remote processor reset hold boot
+	1st cell: phandle of syscon block
+	2nd cell: register offset containing the hold boot setting
+	3rd cell: register bitmask for the hold boot field
+- st,syscfg-tz: Reference to the system configuration which holds the RCC trust
+		zone mode
+	1st cell: phandle to syscon block
+	2nd cell: register offset containing the RCC trust zone mode setting
+	3rd cell: register bitmask for the RCC trust zone mode bit
+
+Optional properties:
+- interrupt-parent: phandle to the interrupt controller node.
+- interrupts:	Should contain the watchdog interrupt
+- interrupt-names: Must be "wdg"
+- mboxes:	List of phandle and mailbox channel specifiers:
+		- a channel (a) used to communicate through virtqueues with the
+		  remote proc.
+		  Bi-directional channel:
+		      - from local to remote = send message
+		      - from remote to local = send message ack
+		- a channel (b) working the opposite direction of channel (a)
+		- a channel (c) used by the local proc to notify the remote proc
+		  that it is about to be shut down.
+		  Mono-directional channel:
+		      - from local to remote, where ACK from the remote means
+		        that it is ready for shutdown
+- mbox-names:	This property is required if the mboxes property is used.
+		- must be "vq0" for channel (a)
+		- must be "vq1" for channel (b)
+		- must be "shutdown" for channel (c)
+- memory-region: list of phandles to the reserved memory regions associated with
+		the remoteproc device
+		(see ../reserved-memory/reserved-memory.txt)
+- st,syscfg-pdds: Reference to the system configuration which holds the remote
+		processor deep sleep setting
+	1st cell: phandle to syscon block
+	2nd cell: register offset containing the deep sleep setting
+	3rd cell: register bitmask for the deep sleep bit
+- auto_boot:	If defined, when remoteproc is probed, it looks for a default
+		firmware and if it finds some, it loads the firmware and starts
+		the remote processor.
+- recovery:	If defined, remoteproc enables the crash recovery process.
+
+Example:
+	m4_rproc: m4@0 {
+		compatible = "st,stm32mp1-rproc";
+		reg = <0x00000000 0x10000>,
+		      <0x10000000 0x40000>,
+		      <0x30000000 0x40000>;
+		resets = <&rcc MCU_R>;
+		reset-names = "mcu_rst";
+		st,syscfg-holdboot = <&rcc 0x10C 0x1>;
+		st,syscfg-tz = <&rcc 0x000 0x1>;
+	};
-- 
2.7.4

[PATCH 3/8] remoteproc: stm32: add an ST stm32_rproc driver

[Fabien Dessenne]

This patch introduces a new remoteproc driver to control Cortex-M4
co-processor of the STM32 family.
It provides with the following features:
- start and stop
- dedicated co-processor memory regions registration
- coredump and recovery

Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
Signed-off-by: Loic Pallardy <loic.pallardy@st.com>
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
---
 drivers/remoteproc/Kconfig       |  15 +
 drivers/remoteproc/Makefile      |   1 +
 drivers/remoteproc/stm32_rproc.c | 624 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 640 insertions(+)
 create mode 100644 drivers/remoteproc/stm32_rproc.c

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index f0abd26..0fba05a 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -197,6 +197,21 @@ config ST_REMOTEPROC
 config ST_SLIM_REMOTEPROC
 	tristate
 
+config STM32_RPROC
+	tristate "STM32 remoteproc support"
+	depends on ARCH_STM32
+	depends on REMOTEPROC
+	select MAILBOX
+	help
+	  Say y here to support STM32 MCU processors via the
+	  remote processor framework.
+
+	  You want to say y here in order to enable AMP
+	  use-cases to run on your platform (dedicated firmware could be
+	  offloaded to remote MCU processors using this framework).
+
+	  This can be either built-in or a loadable module.
+
 endif # REMOTEPROC
 
 endmenu
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index ce5d061..00f09e6 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -26,3 +26,4 @@ qcom_wcnss_pil-y			+= qcom_wcnss.o
 qcom_wcnss_pil-y			+= qcom_wcnss_iris.o
 obj-$(CONFIG_ST_REMOTEPROC)		+= st_remoteproc.o
 obj-$(CONFIG_ST_SLIM_REMOTEPROC)	+= st_slim_rproc.o
+obj-$(CONFIG_STM32_RPROC)		+= stm32_rproc.o
diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
new file mode 100644
index 0000000..4fd53c4
--- /dev/null
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -0,0 +1,624 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) STMicroelectronics 2018 - All Rights Reserved
+ * Authors: Ludovic Barre <ludovic.barre@st.com> for STMicroelectronics.
+ *          Fabien Dessenne <fabien.dessenne@st.com> for STMicroelectronics.
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/mailbox_client.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/regmap.h>
+#include <linux/remoteproc.h>
+#include <linux/reset.h>
+
+#include "remoteproc_internal.h"
+
+#define HOLD_BOOT		0
+#define RELEASE_BOOT		1
+
+#define MBOX_NB_VQ		2
+#define MBOX_NB_MBX		3
+
+#define STM32_SMC_RCC		0x82001000
+#define STM32_SMC_REG_WRITE	0x1
+
+#define STM32_MBX_VQ0		"vq0"
+#define STM32_MBX_VQ1		"vq1"
+#define STM32_MBX_SHUTDOWN	"shutdown"
+
+struct stm32_syscon {
+	struct regmap *map;
+	u32 reg;
+	u32 mask;
+};
+
+struct stm32_rproc_mem {
+	char name[20];
+	void __iomem *cpu_addr;
+	phys_addr_t bus_addr;
+	u32 dev_addr;
+	size_t size;
+};
+
+struct stm32_rproc_mem_ranges {
+	u32 dev_addr;
+	u32 bus_addr;
+	u32 size;
+};
+
+struct stm32_mbox {
+	const unsigned char name[10];
+	struct mbox_chan *chan;
+	struct mbox_client client;
+	int vq_id;
+};
+
+struct stm32_rproc {
+	struct reset_control *rst;
+	struct stm32_syscon hold_boot;
+	struct stm32_syscon pdds;
+	u32 nb_rmems;
+	struct stm32_rproc_mem *rmems;
+	struct stm32_mbox mb[MBOX_NB_MBX];
+	bool secured_soc;
+};
+
+static int stm32_rproc_pa_to_da(struct rproc *rproc, phys_addr_t pa, u64 *da)
+{
+	unsigned int i;
+	struct stm32_rproc *ddata = rproc->priv;
+	struct stm32_rproc_mem *p_mem;
+
+	for (i = 0; i < ddata->nb_rmems; i++) {
+		p_mem = &ddata->rmems[i];
+
+		if (pa < p_mem->bus_addr ||
+		    pa >= p_mem->bus_addr + p_mem->size)
+			continue;
+		*da = pa - p_mem->bus_addr + p_mem->dev_addr;
+		dev_dbg(rproc->dev.parent, "pa %#x to da %llx\n", pa, *da);
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int stm32_rproc_mem_alloc(struct rproc *rproc,
+				 struct rproc_mem_entry *mem)
+{
+	struct device *dev = rproc->dev.parent;
+	void *va;
+
+	dev_dbg(dev, "map memory: %pa+%zx\n", &mem->dma, mem->len);
+	va = ioremap_wc(mem->dma, mem->len);
+	if (IS_ERR_OR_NULL(va)) {
+		dev_err(dev, "Unable to map memory region: %pa+%zx\n",
+			&mem->dma, mem->len);
+		return -ENOMEM;
+	}
+
+	/* Update memory entry va */
+	mem->va = va;
+
+	return 0;
+}
+
+static int stm32_rproc_mem_release(struct rproc *rproc,
+				   struct rproc_mem_entry *mem)
+{
+	dev_dbg(rproc->dev.parent, "unmap memory: %pa\n", &mem->dma);
+	iounmap(mem->va);
+
+	return 0;
+}
+
+static int stm32_rproc_of_memory_translations(struct rproc *rproc)
+{
+	struct device *parent, *dev = rproc->dev.parent;
+	struct stm32_rproc *ddata = rproc->priv;
+	struct device_node *np;
+	struct stm32_rproc_mem *p_mems;
+	struct stm32_rproc_mem_ranges *mem_range;
+	int cnt, array_size, i, ret = 0;
+
+	parent = dev->parent;
+	np = parent->of_node;
+
+	cnt = of_property_count_elems_of_size(np, "ranges",
+					      sizeof(*mem_range));
+	if (cnt <= 0) {
+		dev_err(dev, "%s: ranges property not defined\n", __func__);
+		return -EINVAL;
+	}
+
+	p_mems = devm_kcalloc(dev, cnt, sizeof(*p_mems), GFP_KERNEL);
+	if (!p_mems)
+		return -ENOMEM;
+	mem_range = kcalloc(cnt, sizeof(*mem_range), GFP_KERNEL);
+	if (!mem_range)
+		return -ENOMEM;
+
+	array_size = cnt * sizeof(struct stm32_rproc_mem_ranges) / sizeof(u32);
+
+	ret = of_property_read_u32_array(np, "ranges",
+					 (u32 *)mem_range, array_size);
+	if (ret) {
+		dev_err(dev, "error while get ranges property: %x\n", ret);
+		goto free_mem;
+	}
+
+	for (i = 0; i < cnt; i++) {
+		p_mems[i].bus_addr = mem_range[i].bus_addr;
+		p_mems[i].dev_addr = mem_range[i].dev_addr;
+		p_mems[i].size     = mem_range[i].size;
+
+		dev_dbg(dev, "memory range[%i]: da %#x, pa %#x, size %#x:\n",
+			i, p_mems[i].dev_addr, p_mems[i].bus_addr,
+			p_mems[i].size);
+	}
+
+	ddata->rmems = p_mems;
+	ddata->nb_rmems = cnt;
+
+free_mem:
+	kfree(mem_range);
+	return ret;
+}
+
+static int stm32_rproc_mbox_idx(struct rproc *rproc, const unsigned char *name)
+{
+	struct stm32_rproc *ddata = rproc->priv;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ddata->mb); i++) {
+		if (!strncmp(ddata->mb[i].name, name, strlen(name)))
+			return i;
+	}
+	dev_err(&rproc->dev, "mailbox %s not found\n", name);
+
+	return -EINVAL;
+}
+
+static int stm32_rproc_elf_load_rsc_table(struct rproc *rproc,
+					  const struct firmware *fw)
+{
+	if (rproc_elf_load_rsc_table(rproc, fw))
+		dev_warn(&rproc->dev, "no resource table found for this firmware\n");
+
+	return 0;
+}
+
+static int stm32_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
+{
+	struct device *dev = rproc->dev.parent;
+	struct device_node *np = dev->of_node;
+	struct of_phandle_iterator it;
+	struct rproc_mem_entry *mem;
+	struct reserved_mem *rmem;
+	u64 da;
+	int index = 0;
+
+	/* Register associated reserved memory regions */
+	of_phandle_iterator_init(&it, np, "memory-region", NULL, 0);
+	while (of_phandle_iterator_next(&it) == 0) {
+		rmem = of_reserved_mem_lookup(it.node);
+		if (!rmem) {
+			dev_err(dev, "unable to acquire memory-region\n");
+			return -EINVAL;
+		}
+
+		if (stm32_rproc_pa_to_da(rproc, rmem->base, &da) < 0) {
+			dev_err(dev, "memory region not valid %pa\n",
+				&rmem->base);
+			return -EINVAL;
+		}
+
+		/*  No need to map vdev buffer */
+		if (strcmp(it.node->name, "vdev0buffer")) {
+			/* Register memory region */
+			mem = rproc_mem_entry_init(dev, NULL,
+						   (dma_addr_t)rmem->base,
+						   rmem->size, da,
+						   stm32_rproc_mem_alloc,
+						   stm32_rproc_mem_release,
+						   it.node->name);
+
+			if (mem)
+				rproc_coredump_add_segment(rproc, da,
+							   rmem->size);
+		} else {
+			/* Register reserved memory for vdev buffer alloc */
+			mem = rproc_of_resm_mem_entry_init(dev, index,
+							   rmem->size,
+							   rmem->base,
+							   it.node->name);
+		}
+
+		if (!mem)
+			return -ENOMEM;
+
+		rproc_add_carveout(rproc, mem);
+		index++;
+	}
+
+	return stm32_rproc_elf_load_rsc_table(rproc, fw);
+}
+
+static irqreturn_t stm32_rproc_wdg(int irq, void *data)
+{
+	struct rproc *rproc = data;
+
+	rproc_report_crash(rproc, RPROC_WATCHDOG);
+
+	return IRQ_HANDLED;
+}
+
+static void stm32_rproc_mb_callback(struct mbox_client *cl, void *data)
+{
+	struct rproc *rproc = dev_get_drvdata(cl->dev);
+	struct stm32_mbox *mb = container_of(cl, struct stm32_mbox, client);
+
+	if (rproc_vq_interrupt(rproc, mb->vq_id) == IRQ_NONE)
+		dev_dbg(&rproc->dev, "no message found in vq%d\n", mb->vq_id);
+}
+
+static void stm32_rproc_free_mbox(struct rproc *rproc)
+{
+	struct stm32_rproc *ddata = rproc->priv;
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(ddata->mb); i++) {
+		if (ddata->mb[i].chan)
+			mbox_free_channel(ddata->mb[i].chan);
+		ddata->mb[i].chan = NULL;
+	}
+}
+
+static const struct stm32_mbox stm32_rproc_mbox[MBOX_NB_MBX] = {
+	{
+		.name = STM32_MBX_VQ0,
+		.vq_id = 0,
+		.client = {
+			.rx_callback = stm32_rproc_mb_callback,
+			.tx_block = false,
+		},
+	},
+	{
+		.name = STM32_MBX_VQ1,
+		.vq_id = 1,
+		.client = {
+			.rx_callback = stm32_rproc_mb_callback,
+			.tx_block = false,
+		},
+	},
+	{
+		.name = STM32_MBX_SHUTDOWN,
+		.vq_id = -1,
+		.client = {
+			.tx_block = true,
+			.tx_done = NULL,
+			.tx_tout = 500, /* 500 ms time out */
+		},
+	}
+};
+
+static void stm32_rproc_request_mbox(struct rproc *rproc)
+{
+	struct stm32_rproc *ddata = rproc->priv;
+	struct device *dev = &rproc->dev;
+	unsigned int i;
+	const unsigned char *name;
+	struct mbox_client *cl;
+
+	/* Initialise mailbox structure table */
+	memcpy(ddata->mb, stm32_rproc_mbox, sizeof(stm32_rproc_mbox));
+
+	for (i = 0; i < MBOX_NB_MBX; i++) {
+		name = ddata->mb[i].name;
+
+		cl = &ddata->mb[i].client;
+		cl->dev = dev->parent;
+
+		ddata->mb[i].chan = mbox_request_channel_byname(cl, name);
+		if (IS_ERR(ddata->mb[i].chan)) {
+			dev_warn(dev, "cannot get %s mbox\n", name);
+			ddata->mb[i].chan = NULL;
+		}
+	}
+}
+
+static int stm32_rproc_set_hold_boot(struct rproc *rproc, bool hold)
+{
+	struct stm32_rproc *ddata = rproc->priv;
+	struct stm32_syscon hold_boot = ddata->hold_boot;
+	struct arm_smccc_res smc_res;
+	int val, err;
+
+	val = hold ? HOLD_BOOT : RELEASE_BOOT;
+
+	if (ddata->secured_soc) {
+		arm_smccc_smc(STM32_SMC_RCC, STM32_SMC_REG_WRITE,
+			      hold_boot.reg, val, 0, 0, 0, 0, &smc_res);
+		err = smc_res.a0;
+	} else {
+		err = regmap_update_bits(hold_boot.map, hold_boot.reg,
+					 hold_boot.mask, val);
+	}
+
+	if (err)
+		dev_err(&rproc->dev, "failed to set hold boot\n");
+
+	return err;
+}
+
+static void stm32_rproc_add_coredump_trace(struct rproc *rproc)
+{
+	struct rproc_debug_trace *trace;
+	struct rproc_dump_segment *segment;
+	bool already_added;
+
+	list_for_each_entry(trace, &rproc->traces, node) {
+		already_added = false;
+
+		list_for_each_entry(segment, &rproc->dump_segments, node) {
+			if (segment->da == trace->trace_mem.da) {
+				already_added = true;
+				break;
+			}
+		}
+
+		if (!already_added)
+			rproc_coredump_add_segment(rproc, trace->trace_mem.da,
+						   trace->trace_mem.len);
+	}
+}
+
+static int stm32_rproc_start(struct rproc *rproc)
+{
+	int err;
+
+	stm32_rproc_add_coredump_trace(rproc);
+
+	err = stm32_rproc_set_hold_boot(rproc, false);
+	if (err)
+		return err;
+
+	return stm32_rproc_set_hold_boot(rproc, true);
+}
+
+static int stm32_rproc_stop(struct rproc *rproc)
+{
+	struct stm32_rproc *ddata = rproc->priv;
+	int err, dummy_data, idx;
+
+	/* request shutdown of the remote processor */
+	if (rproc->state != RPROC_OFFLINE) {
+		idx = stm32_rproc_mbox_idx(rproc, STM32_MBX_SHUTDOWN);
+		if (idx >= 0 && ddata->mb[idx].chan) {
+			/* a dummy data is sent to allow to block on transmit */
+			err = mbox_send_message(ddata->mb[idx].chan,
+						&dummy_data);
+			if (err < 0)
+				dev_warn(&rproc->dev, "warning: remote FW shutdown without ack\n");
+		}
+	}
+
+	err = stm32_rproc_set_hold_boot(rproc, true);
+	if (err)
+		return err;
+
+	err = reset_control_assert(ddata->rst);
+	if (err) {
+		dev_err(&rproc->dev, "failed to assert the reset\n");
+		return err;
+	}
+
+	/* to allow platform Standby power mode, set remote proc Deep Sleep */
+	if (ddata->pdds.map) {
+		err = regmap_update_bits(ddata->pdds.map, ddata->pdds.reg,
+					 ddata->pdds.mask, 1);
+		if (err) {
+			dev_err(&rproc->dev, "failed to set pdds\n");
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static void stm32_rproc_kick(struct rproc *rproc, int vqid)
+{
+	struct stm32_rproc *ddata = rproc->priv;
+	unsigned int i;
+	int err;
+
+	if (WARN_ON(vqid >= MBOX_NB_VQ))
+		return;
+
+	for (i = 0; i < MBOX_NB_MBX; i++) {
+		if (vqid != ddata->mb[i].vq_id)
+			continue;
+		if (!ddata->mb[i].chan)
+			return;
+		err = mbox_send_message(ddata->mb[i].chan, (void *)vqid);
+		if (err < 0)
+			dev_err(&rproc->dev, "%s: failed (%s, err:%d)\n",
+				__func__, ddata->mb[i].name, err);
+		return;
+	}
+}
+
+static struct rproc_ops st_rproc_ops = {
+	.start		= stm32_rproc_start,
+	.stop		= stm32_rproc_stop,
+	.kick		= stm32_rproc_kick,
+	.load		= rproc_elf_load_segments,
+	.parse_fw	= stm32_rproc_parse_fw,
+	.find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
+	.sanity_check	= rproc_elf_sanity_check,
+	.get_boot_addr	= rproc_elf_get_boot_addr,
+};
+
+static const struct of_device_id stm32_rproc_match[] = {
+	{ .compatible = "st,stm32mp1-rproc" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, stm32_rproc_match);
+
+static int stm32_rproc_get_syscon(struct device_node *np, const char *prop,
+				  struct stm32_syscon *syscon)
+{
+	int err = 0;
+
+	syscon->map = syscon_regmap_lookup_by_phandle(np, prop);
+	if (IS_ERR(syscon->map)) {
+		err = PTR_ERR(syscon->map);
+		syscon->map = NULL;
+		goto out;
+	}
+
+	err = of_property_read_u32_index(np, prop, 1, &syscon->reg);
+	if (err)
+		goto out;
+
+	err = of_property_read_u32_index(np, prop, 2, &syscon->mask);
+
+out:
+	return err;
+}
+
+static int stm32_rproc_parse_dt(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct rproc *rproc = platform_get_drvdata(pdev);
+	struct stm32_rproc *ddata = rproc->priv;
+	struct stm32_syscon tz;
+	unsigned int tzen;
+	int err, irq;
+
+	irq = platform_get_irq_byname(pdev, "wdg");
+	if (irq > 0) {
+		err = devm_request_irq(dev, irq, stm32_rproc_wdg, 0,
+				       dev_name(dev), rproc);
+		if (err) {
+			dev_err(dev, "failed to request wdg irq\n");
+			return err;
+		}
+
+		dev_info(dev, "wdg irq registered\n");
+	}
+
+	ddata->rst = devm_reset_control_get(dev, "mcu_rst");
+	if (IS_ERR(ddata->rst)) {
+		dev_err(dev, "failed to get mcu reset\n");
+		return PTR_ERR(ddata->rst);
+	}
+
+	/*
+	 * if platform is secured the hold boot bit must be written by
+	 * smc call and read normally.
+	 * if not secure the hold boot bit could be read/write normally
+	 */
+	err = stm32_rproc_get_syscon(np, "st,syscfg-tz", &tz);
+	if (err) {
+		dev_err(dev, "failed to get tz syscfg\n");
+		return err;
+	}
+
+	err = regmap_read(tz.map, tz.reg, &tzen);
+	if (err) {
+		dev_err(&rproc->dev, "failed to read tzen\n");
+		return err;
+	}
+	ddata->secured_soc = tzen & tz.mask;
+
+	err = stm32_rproc_get_syscon(np, "st,syscfg-holdboot",
+				     &ddata->hold_boot);
+	if (err) {
+		dev_err(dev, "failed to get hold boot\n");
+		return err;
+	}
+
+	err = stm32_rproc_get_syscon(np, "st,syscfg-pdds", &ddata->pdds);
+	if (err)
+		dev_warn(dev, "failed to get pdds\n");
+
+	rproc->auto_boot = of_property_read_bool(np, "auto_boot");
+	rproc->recovery_disabled = !of_property_read_bool(np, "recovery");
+
+	return stm32_rproc_of_memory_translations(rproc);
+}
+
+static int stm32_rproc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct stm32_rproc *ddata;
+	struct device_node *np = dev->of_node;
+	struct rproc *rproc;
+	int ret;
+
+	rproc = rproc_alloc(dev, np->name, &st_rproc_ops, NULL, sizeof(*ddata));
+	if (!rproc)
+		return -ENOMEM;
+
+	rproc->has_iommu = false;
+	ddata = rproc->priv;
+
+	platform_set_drvdata(pdev, rproc);
+
+	ret = stm32_rproc_parse_dt(pdev);
+	if (ret)
+		goto free_rproc;
+
+	stm32_rproc_request_mbox(rproc);
+
+	ret = rproc_add(rproc);
+	if (ret)
+		goto free_mb;
+
+	return 0;
+
+free_mb:
+	stm32_rproc_free_mbox(rproc);
+free_rproc:
+	rproc_free(rproc);
+	return ret;
+}
+
+static int stm32_rproc_remove(struct platform_device *pdev)
+{
+	struct rproc *rproc = platform_get_drvdata(pdev);
+
+	if (atomic_read(&rproc->power) > 0)
+		rproc_shutdown(rproc);
+
+	rproc_del(rproc);
+	stm32_rproc_free_mbox(rproc);
+	rproc_free(rproc);
+
+	return 0;
+}
+
+static struct platform_driver stm32_rproc_driver = {
+	.probe = stm32_rproc_probe,
+	.remove = stm32_rproc_remove,
+	.driver = {
+		.name = "stm32-rproc",
+		.of_match_table = of_match_ptr(stm32_rproc_match),
+	},
+};
+module_platform_driver(stm32_rproc_driver);
+
+MODULE_DESCRIPTION("STM32 Remote Processor Control Driver");
+MODULE_AUTHOR("Ludovic Barre <ludovic.barre@st.com>");
+MODULE_AUTHOR("Fabien Dessenne <fabien.dessenne@st.com>");
+MODULE_LICENSE("GPL v2");
+
-- 
2.7.4

[PATCH 4/8] ARM: dts: stm32: add m4 remoteproc support on STM32MP157c

[Fabien Dessenne]

Declare the M4 remote processor in a sub-node of the ahb simple bus.

Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 arch/arm/boot/dts/stm32mp157c.dtsi | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
index c664b55..105e21f 100644
--- a/arch/arm/boot/dts/stm32mp157c.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c.dtsi
@@ -1242,4 +1242,25 @@
 			status = "disabled";
 		};
 	};
+
+	mlahb: mlahb@0 {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0x00000000 0x38000000 0x10000>,
+			 <0x10000000 0x10000000 0x60000>,
+			 <0x30000000 0x30000000 0x60000>;
+
+		m4_rproc: m4@0 {
+			compatible = "st,stm32mp1-rproc";
+			reg = <0x00000000 0x10000>,
+			      <0x10000000 0x40000>,
+			      <0x30000000 0x40000>;
+			resets = <&rcc MCU_R>;
+			reset-names = "mcu_rst";
+			st,syscfg-holdboot = <&rcc 0x10C 0x1>;
+			st,syscfg-tz = <&rcc 0x000 0x1>;
+			status = "disabled";
+		};
+	};
 };
-- 
2.7.4

[PATCH 5/8] ARM: dts: stm32: declare copro reserved memories on STM32MP157c-ed1

[Fabien Dessenne]

Declare reserved memories shared by the processors for STM32MP157c-ed1
board.

Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 arch/arm/boot/dts/stm32mp157c-ed1.dts | 42 +++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/arch/arm/boot/dts/stm32mp157c-ed1.dts b/arch/arm/boot/dts/stm32mp157c-ed1.dts
index 626ceb3..acfc5cd 100644
--- a/arch/arm/boot/dts/stm32mp157c-ed1.dts
+++ b/arch/arm/boot/dts/stm32mp157c-ed1.dts
@@ -21,6 +21,48 @@
 		reg = <0xC0000000 0x40000000>;
 	};
 
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		mcuram2: mcuram2@10000000 {
+			compatible = "shared-dma-pool";
+			reg = <0x10000000 0x40000>;
+			no-map;
+		};
+
+		vdev0vring0: vdev0vring0@10040000 {
+			compatible = "shared-dma-pool";
+			reg = <0x10040000 0x1000>;
+			no-map;
+		};
+
+		vdev0vring1: vdev0vring1@10041000 {
+			compatible = "shared-dma-pool";
+			reg = <0x10041000 0x1000>;
+			no-map;
+		};
+
+		vdev0buffer: vdev0buffer@10042000 {
+			compatible = "shared-dma-pool";
+			reg = <0x10042000 0x4000>;
+			no-map;
+		};
+
+		mcuram: mcuram@30000000 {
+			compatible = "shared-dma-pool";
+			reg = <0x30000000 0x40000>;
+			no-map;
+		};
+
+		retram: retram@38000000 {
+			compatible = "shared-dma-pool";
+			reg = <0x38000000 0x10000>;
+			no-map;
+		};
+	};
+
 	aliases {
 		serial0 = &uart4;
 	};
-- 
2.7.4

[PATCH 6/8] ARM: dts: stm32: enable m4 coprocessor support on STM32MP157c-ed1

[Fabien Dessenne]

Enable m4 coprocessor for STM32MP157c-ed1 board.

Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 arch/arm/boot/dts/stm32mp157c-ed1.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/stm32mp157c-ed1.dts b/arch/arm/boot/dts/stm32mp157c-ed1.dts
index acfc5cd..c0b4cca 100644
--- a/arch/arm/boot/dts/stm32mp157c-ed1.dts
+++ b/arch/arm/boot/dts/stm32mp157c-ed1.dts
@@ -134,6 +134,18 @@
 	status = "okay";
 };
 
+&m4_rproc {
+	memory-region = <&retram>, <&mcuram>, <&mcuram2>, <&vdev0vring0>,
+			<&vdev0vring1>, <&vdev0buffer>;
+	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>;
+	mbox-names = "vq0", "vq1", "shutdown";
+	interrupt-parent = <&exti>;
+	interrupts = <68 1>;
+	interrupt-names = "wdg";
+	recovery;
+	status = "okay";
+};
+
 &rng1 {
 	status = "okay";
 };
-- 
2.7.4

[PATCH 7/8] ARM: dts: stm32: declare copro reserved memories on STM32MP157a-dk1

[Fabien Dessenne]

Declare reserved memories shared by the processors for STM32MP157a-dk1

Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 arch/arm/boot/dts/stm32mp157a-dk1.dts | 42 +++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/arch/arm/boot/dts/stm32mp157a-dk1.dts b/arch/arm/boot/dts/stm32mp157a-dk1.dts
index 85a761a..26ce8de 100644
--- a/arch/arm/boot/dts/stm32mp157a-dk1.dts
+++ b/arch/arm/boot/dts/stm32mp157a-dk1.dts
@@ -27,6 +27,48 @@
 		reg = <0xc0000000 0x20000000>;
 	};
 
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		mcuram2: mcuram2@10000000 {
+			compatible = "shared-dma-pool";
+			reg = <0x10000000 0x40000>;
+			no-map;
+		};
+
+		vdev0vring0: vdev0vring0@10040000 {
+			compatible = "shared-dma-pool";
+			reg = <0x10040000 0x1000>;
+			no-map;
+		};
+
+		vdev0vring1: vdev0vring1@10041000 {
+			compatible = "shared-dma-pool";
+			reg = <0x10041000 0x1000>;
+			no-map;
+		};
+
+		vdev0buffer: vdev0buffer@10042000 {
+			compatible = "shared-dma-pool";
+			reg = <0x10042000 0x4000>;
+			no-map;
+		};
+
+		mcuram: mcuram@30000000 {
+			compatible = "shared-dma-pool";
+			reg = <0x30000000 0x40000>;
+			no-map;
+		};
+
+		retram: retram@38000000 {
+			compatible = "shared-dma-pool";
+			reg = <0x38000000 0x10000>;
+			no-map;
+		};
+	};
+
 	led {
 		compatible = "gpio-leds";
 		blue {
-- 
2.7.4

[PATCH 8/8] ARM: dts: stm32: enable m4 coprocessor support on STM32MP157a-dk1

[Fabien Dessenne]

Enable m4 coprocessor for STM32MP157a-dk1 board.

Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
---
 arch/arm/boot/dts/stm32mp157a-dk1.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/stm32mp157a-dk1.dts b/arch/arm/boot/dts/stm32mp157a-dk1.dts
index 26ce8de..818a979 100644
--- a/arch/arm/boot/dts/stm32mp157a-dk1.dts
+++ b/arch/arm/boot/dts/stm32mp157a-dk1.dts
@@ -116,6 +116,18 @@
 	status = "okay";
 };
 
+&m4_rproc {
+	memory-region = <&retram>, <&mcuram>, <&mcuram2>, <&vdev0vring0>,
+			<&vdev0vring1>, <&vdev0buffer>;
+	mboxes = <&ipcc 0>, <&ipcc 1>, <&ipcc 2>;
+	mbox-names = "vq0", "vq1", "shutdown";
+	interrupt-parent = <&exti>;
+	interrupts = <68 1>;
+	interrupt-names = "wdg";
+	recovery;
+	status = "okay";
+};
+
 &rng1 {
 	status = "okay";
 };
-- 
2.7.4

Re: [PATCH 0/8] stm32 m4 remoteproc on STM32MP157c

[Alexandre Torgue]

Hi fabien

On 3/5/19 3:24 PM, Fabien Dessenne wrote:
> STMicrolectronics STM32MP157 MPU are based on a Dual Arm Cortex-A7 core and a
> Cortex-M4.
> This patchset adds the support of the stm32_rproc driver allowing to control
> the M4 remote processor.
> 
> Fabien Dessenne (8):
>    dt-bindings: stm32: add bindings for ML-AHB interconnect
>    dt-bindings: remoteproc: add bindings for stm32 remote processor
>      driver
>    remoteproc: stm32: add an ST stm32_rproc driver
>    ARM: dts: stm32: add m4 remoteproc support on STM32MP157c
>    ARM: dts: stm32: declare copro reserved memories on STM32MP157c-ed1
>    ARM: dts: stm32: enable m4 coprocessor support on STM32MP157c-ed1
>    ARM: dts: stm32: declare copro reserved memories on STM32MP157a-dk1
>    ARM: dts: stm32: enable m4 coprocessor support on STM32MP157a-dk1
> 
>   .../devicetree/bindings/arm/stm32/mlahb.txt        |  30 +
>   .../devicetree/bindings/remoteproc/stm32-rproc.txt |  67 +++
>   arch/arm/boot/dts/stm32mp157a-dk1.dts              |  54 ++
>   arch/arm/boot/dts/stm32mp157c-ed1.dts              |  54 ++
>   arch/arm/boot/dts/stm32mp157c.dtsi                 |  21 +
>   drivers/remoteproc/Kconfig                         |  15 +
>   drivers/remoteproc/Makefile                        |   1 +
>   drivers/remoteproc/stm32_rproc.c                   | 624 +++++++++++++++++++++
>   8 files changed, 866 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/arm/stm32/mlahb.txt
>   create mode 100644 Documentation/devicetree/bindings/remoteproc/stm32-rproc.txt
>   create mode 100644 drivers/remoteproc/stm32_rproc.c
> 

Concerning DT patches, at first glance I don't see obvious issues. I'll 
take Dt patches as soon as driver and bindings patches are acked.

regards
Alex

Re: [PATCH 1/8] dt-bindings: stm32: add bindings for ML-AHB interconnect

[Rob Herring]

On Tue, Mar 05, 2019 at 03:24:02PM +0100, Fabien Dessenne wrote:
> Document the ML-AHB interconnect for stm32 SoCs.
> 
> Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
> ---
>  .../devicetree/bindings/arm/stm32/mlahb.txt        | 30 ++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/stm32/mlahb.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/stm32/mlahb.txt b/Documentation/devicetree/bindings/arm/stm32/mlahb.txt
> new file mode 100644
> index 0000000..880cb38
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/stm32/mlahb.txt
> @@ -0,0 +1,30 @@
> +ML-AHB interconnect bindings
> +
> +These bindings describe the STM32 SoCs ML-AHB interconnect bus which connects
> +a Cortex-M subsystem with dedicated memories.
> +
> +Required properties:
> +- compatible: should be "simple-bus"

A binding for simple-bus was the first thing that looked odd.

> +- ranges: describes memory addresses translation between the local CPU and the
> +	   remote Cortex-M processor. Each memory region, is declared with 3
> +	   parameters:
> +		 - param 1: device base address (Cortex-M processor address)
> +		 - param 2: physical base address (local CPU address)
> +		 - param 3: size of the memory region.

Given that the driver is parsing ranges itself, this looks like abuse of 
ranges.

What exactly is address 0 supposed to be here? If it is the M4's view of 
memory, then dma-ranges is what you want to use here.


> +
> +The Cortex-M remote processor accessed via the mlahb interconnect is described
> +by a child node.
> +
> +Example:
> +mlahb: mlahb@0 {

Note that the unit-address is wrong here as it should be 38000000.

> +	compatible = "simple-bus";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	ranges = <0x00000000 0x38000000 0x10000>,
> +		 <0x10000000 0x10000000 0x60000>,
> +		 <0x30000000 0x30000000 0x60000>;
> +
> +	m4_rproc: m4@0 {
> +		...
> +	};
> +};
> -- 
> 2.7.4
> 

Re: [PATCH 2/8] dt-bindings: remoteproc: add bindings for stm32 remote processor driver

[Rob Herring]

On Tue, Mar 05, 2019 at 03:24:03PM +0100, Fabien Dessenne wrote:
> Add the device tree bindings document for the stm32 remoteproc devices.
> 
> Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
> ---
>  .../devicetree/bindings/remoteproc/stm32-rproc.txt | 67 ++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/stm32-rproc.txt
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/stm32-rproc.txt b/Documentation/devicetree/bindings/remoteproc/stm32-rproc.txt
> new file mode 100644
> index 0000000..cc90c88
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/stm32-rproc.txt
> @@ -0,0 +1,67 @@
> +STMicroelectronics STM32 Remoteproc
> +-----------------------------------
> +This document defines the binding for the remoteproc component that loads and
> +boots firmwares on the ST32MP family chipset.
> +
> +Required properties:
> +- compatible:	Must be "st,stm32mp1-rproc"

'rproc' is what this is called in the h/w manual?

> +- reg:		Address ranges of the remote processor dedicated memories.
> +		The parent node should provide an appropriate ranges property
> +		for properly translating these into bus addresses.
> +- resets:	Reference to a reset controller asserting the remote processor.
> +- reset-names:	Must be "mcu_rst"

-names is kind of pointless when there is only 1 entry.

> +- st,syscfg-holdboot: Reference to the system configuration which holds the
> +		remote processor reset hold boot
> +	1st cell: phandle of syscon block
> +	2nd cell: register offset containing the hold boot setting
> +	3rd cell: register bitmask for the hold boot field
> +- st,syscfg-tz: Reference to the system configuration which holds the RCC trust
> +		zone mode
> +	1st cell: phandle to syscon block
> +	2nd cell: register offset containing the RCC trust zone mode setting
> +	3rd cell: register bitmask for the RCC trust zone mode bit
> +
> +Optional properties:
> +- interrupt-parent: phandle to the interrupt controller node.

This is implied.

> +- interrupts:	Should contain the watchdog interrupt
> +- interrupt-names: Must be "wdg"

-names is kind of pointless when there is only 1 entry.

> +- mboxes:	List of phandle and mailbox channel specifiers:

How would one talk to the processor if this is optional?

> +		- a channel (a) used to communicate through virtqueues with the
> +		  remote proc.
> +		  Bi-directional channel:
> +		      - from local to remote = send message
> +		      - from remote to local = send message ack
> +		- a channel (b) working the opposite direction of channel (a)
> +		- a channel (c) used by the local proc to notify the remote proc
> +		  that it is about to be shut down.
> +		  Mono-directional channel:

Unidirectional

> +		      - from local to remote, where ACK from the remote means
> +		        that it is ready for shutdown
> +- mbox-names:	This property is required if the mboxes property is used.
> +		- must be "vq0" for channel (a)
> +		- must be "vq1" for channel (b)
> +		- must be "shutdown" for channel (c)
> +- memory-region: list of phandles to the reserved memory regions associated with
> +		the remoteproc device
> +		(see ../reserved-memory/reserved-memory.txt)

How many?

> +- st,syscfg-pdds: Reference to the system configuration which holds the remote
> +		processor deep sleep setting
> +	1st cell: phandle to syscon block
> +	2nd cell: register offset containing the deep sleep setting
> +	3rd cell: register bitmask for the deep sleep bit
> +- auto_boot:	If defined, when remoteproc is probed, it looks for a default
> +		firmware and if it finds some, it loads the firmware and starts
> +		the remote processor.

How do you know what is and isn't default firmware? Seems like this 
could be accomplished using the standard 'firmware-name' to specify what 
firmware to load and boot.

> +- recovery:	If defined, remoteproc enables the crash recovery process.

Doesn't seem like a static DT property. This is a feature to enable or 
something you set on boot to recover?

> +
> +Example:
> +	m4_rproc: m4@0 {
> +		compatible = "st,stm32mp1-rproc";
> +		reg = <0x00000000 0x10000>,
> +		      <0x10000000 0x40000>,
> +		      <0x30000000 0x40000>;
> +		resets = <&rcc MCU_R>;
> +		reset-names = "mcu_rst";
> +		st,syscfg-holdboot = <&rcc 0x10C 0x1>;
> +		st,syscfg-tz = <&rcc 0x000 0x1>;
> +	};
> -- 
> 2.7.4
> 

Re: [PATCH 1/8] dt-bindings: stm32: add bindings for ML-AHB interconnect

[Fabien DESSENNE]

Hi Rob,

Let me clarify the context and the reason of the proposed approach.

The remoteproc framework deals with 'carveout' memory regions.
 From the remoteproc_core.c:

  * 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). 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).

For this, the remoteproc drivers have to register these memory regions
providing their memory mapping remote processor view / local processor
view. See rproc_mem_entry_init() and rproc_add_carveout().

An implementation solution consists in declaring the memory mapping inside
the remoteproc driver. (Ex: imx_rproc_att_imx7d[] from imx_rproc.c)

For the stm32 rproc driver that we are introducing, we would like to have
something more flexible than hardcoded values.

One reason for this, is that some memory parts can be accessed through
different 2 bus port, with different addresses and access speed, and we
would like to let the user customize this.

Using DeviceTree "ranges" seems to fit with these requirements.

If you think that this is not the right approach, please let me know if you
think about something better.

See also below my answer to your specific remarks

BR

On 28/03/2019 12:07 AM, Rob Herring wrote:

> On Tue, Mar 05, 2019 at 03:24:02PM +0100, Fabien Dessenne wrote:
>> Document the ML-AHB interconnect for stm32 SoCs.
>>
>> Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
>> ---
>>   .../devicetree/bindings/arm/stm32/mlahb.txt        | 30 ++++++++++++++++++++++
>>   1 file changed, 30 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/arm/stm32/mlahb.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/stm32/mlahb.txt b/Documentation/devicetree/bindings/arm/stm32/mlahb.txt
>> new file mode 100644
>> index 0000000..880cb38
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/stm32/mlahb.txt
>> @@ -0,0 +1,30 @@
>> +ML-AHB interconnect bindings
>> +
>> +These bindings describe the STM32 SoCs ML-AHB interconnect bus which connects
>> +a Cortex-M subsystem with dedicated memories.
>> +
>> +Required properties:
>> +- compatible: should be "simple-bus"
> A binding for simple-bus was the first thing that looked odd.

Since we want to use "ranges" (or "dma-ranges"), we need to define a parent-bus.
This bus has nothing specific, so it is a "simple-bus"

>
>> +- ranges: describes memory addresses translation between the local CPU and the
>> +	   remote Cortex-M processor. Each memory region, is declared with 3
>> +	   parameters:
>> +		 - param 1: device base address (Cortex-M processor address)
>> +		 - param 2: physical base address (local CPU address)
>> +		 - param 3: size of the memory region.
> Given that the driver is parsing ranges itself, this looks like abuse of
> ranges.

That's correct. As explained above, we need to provide the remoteproc framework
with carveout mappings.

>
> What exactly is address 0 supposed to be here? If it is the M4's view of
> memory, then dma-ranges is what you want to use here.

"dma-ranges" is probably more appropriated here. But the driver still needs to
parse this property.

>
>
>> +
>> +The Cortex-M remote processor accessed via the mlahb interconnect is described
>> +by a child node.
>> +
>> +Example:
>> +mlahb: mlahb@0 {
> Note that the unit-address is wrong here as it should be 38000000.

OK

>
>> +	compatible = "simple-bus";
>> +	#address-cells = <1>;
>> +	#size-cells = <1>;
>> +	ranges = <0x00000000 0x38000000 0x10000>,
>> +		 <0x10000000 0x10000000 0x60000>,
>> +		 <0x30000000 0x30000000 0x60000>;
>> +
>> +	m4_rproc: m4@0 {
>> +		...
>> +	};
>> +};
>> -- 
>> 2.7.4
>>
>>

Re: [PATCH 1/8] dt-bindings: stm32: add bindings for ML-AHB interconnect

[Rob Herring]

 On Fri, Mar 29, 2019 at 10:59 AM Fabien DESSENNE
<fabien.dessenne@st.com> wrote:
>
> Hi Rob,
>
> Let me clarify the context and the reason of the proposed approach.
>
> The remoteproc framework deals with 'carveout' memory regions.
>  From the remoteproc_core.c:
>
>   * 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). 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).
>
> For this, the remoteproc drivers have to register these memory regions
> providing their memory mapping remote processor view / local processor
> view. See rproc_mem_entry_init() and rproc_add_carveout().
>
> An implementation solution consists in declaring the memory mapping inside
> the remoteproc driver. (Ex: imx_rproc_att_imx7d[] from imx_rproc.c)
>
> For the stm32 rproc driver that we are introducing, we would like to have
> something more flexible than hardcoded values.

I need an explanation that is not in terms of remoteproc. That's a Linux thing.

> One reason for this, is that some memory parts can be accessed through
> different 2 bus port, with different addresses and access speed, and we
> would like to let the user customize this.
>
> Using DeviceTree "ranges" seems to fit with these requirements.

'ranges' is strictly about the cpu's (running Linux) view of the
system. 'dma-ranges' is for the device's (the remoteproc) view of
memory. You simply cannot redefine them for your own custom use.

If the cpu has 2 views of the same memory, then you can use ranges to
describe that.

> If you think that this is not the right approach, please let me know if you
> think about something better.
>
> See also below my answer to your specific remarks
>
> BR
>
> On 28/03/2019 12:07 AM, Rob Herring wrote:
>
> > On Tue, Mar 05, 2019 at 03:24:02PM +0100, Fabien Dessenne wrote:
> >> Document the ML-AHB interconnect for stm32 SoCs.
> >>
> >> Signed-off-by: Fabien Dessenne <fabien.dessenne@st.com>
> >> ---
> >>   .../devicetree/bindings/arm/stm32/mlahb.txt        | 30 ++++++++++++++++++++++
> >>   1 file changed, 30 insertions(+)
> >>   create mode 100644 Documentation/devicetree/bindings/arm/stm32/mlahb.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/stm32/mlahb.txt b/Documentation/devicetree/bindings/arm/stm32/mlahb.txt
> >> new file mode 100644
> >> index 0000000..880cb38
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/arm/stm32/mlahb.txt
> >> @@ -0,0 +1,30 @@
> >> +ML-AHB interconnect bindings
> >> +
> >> +These bindings describe the STM32 SoCs ML-AHB interconnect bus which connects
> >> +a Cortex-M subsystem with dedicated memories.
> >> +
> >> +Required properties:
> >> +- compatible: should be "simple-bus"
> > A binding for simple-bus was the first thing that looked odd.
>
> Since we want to use "ranges" (or "dma-ranges"), we need to define a parent-bus.
> This bus has nothing specific, so it is a "simple-bus"

Okay, fair enough.

> >> +- ranges: describes memory addresses translation between the local CPU and the
> >> +       remote Cortex-M processor. Each memory region, is declared with 3
> >> +       parameters:
> >> +             - param 1: device base address (Cortex-M processor address)
> >> +             - param 2: physical base address (local CPU address)
> >> +             - param 3: size of the memory region.
> > Given that the driver is parsing ranges itself, this looks like abuse of
> > ranges.
>
> That's correct. As explained above, we need to provide the remoteproc framework
> with carveout mappings.
>
> >
> > What exactly is address 0 supposed to be here? If it is the M4's view of
> > memory, then dma-ranges is what you want to use here.
>
> "dma-ranges" is probably more appropriated here. But the driver still needs to
> parse this property.

That is more acceptable assuming what's in dma-ranges matches the
standard definition.

Rob