[PATCH 00/11] remoteproc: stm32: Add support for synchronisation with MCU

[PATCH 00/11] remoteproc: stm32: Add support for synchronisation with MCU

[Mathieu Poirier]

This patchset needs to be applied on top of this one [1].

It refactors the STM32 platform code in order to introduce support for
synchronising with an MCU that would have been started by another entity.

Most of the code is taken from work that Arnaud has already published [2].

On top of making sure the classic case where the remoteproc core is in
charge of the MCU's lifecyle is still functional, this set also tested that
it can synchronise with the MCU on ST's mp157c board. 

Thanks,
Mathieu

[1]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=261069
[2]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=239877

Mathieu Poirier (11):
  remoteproc: stm32: Decouple rproc from memory translation
  remoteproc: stm32: Request IRQ with platform device
  remoteproc: stm32: Decouple rproc from DT parsing
  remoteproc: stm32: Remove memory translation from DT parsing
  remoteproc: stm32: Parse syscon that will manage M4 synchronisation
  remoteproc: stm32: Get coprocessor state
  remoteproc: stm32: Get loaded resource table for synchronisation
  remoteproc: stm32: Introduce new start and stop ops for
    synchronisation
  remoteproc: stm32: Introduce new parse fw ops for synchronisation
  remoteproc: stm32: Introduce new loaded rsc ops for synchronisation
  remoteproc: stm32: Allocate rproc for synchronisation with MCU

 drivers/remoteproc/stm32_rproc.c | 285 ++++++++++++++++++++++++++++---
 1 file changed, 265 insertions(+), 20 deletions(-)

-- 
2.20.1

[PATCH 01/11] remoteproc: stm32: Decouple rproc from memory translation

[Mathieu Poirier]

Remove the remote processor from the process of parsing the memory
ranges since there is no correlation between them.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/remoteproc/stm32_rproc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index a18f88044111..cde4b1a56381 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -127,10 +127,10 @@ static int stm32_rproc_mem_release(struct rproc *rproc,
 	return 0;
 }
 
-static int stm32_rproc_of_memory_translations(struct rproc *rproc)
+static int stm32_rproc_of_memory_translations(struct platform_device *pdev,
+					      struct stm32_rproc *ddata)
 {
-	struct device *parent, *dev = rproc->dev.parent;
-	struct stm32_rproc *ddata = rproc->priv;
+	struct device *parent, *dev = &pdev->dev;
 	struct device_node *np;
 	struct stm32_rproc_mem *p_mems;
 	struct stm32_rproc_mem_ranges *mem_range;
@@ -606,7 +606,7 @@ static int stm32_rproc_parse_dt(struct platform_device *pdev)
 
 	rproc->auto_boot = of_property_read_bool(np, "st,auto-boot");
 
-	return stm32_rproc_of_memory_translations(rproc);
+	return stm32_rproc_of_memory_translations(pdev, ddata);
 }
 
 static int stm32_rproc_probe(struct platform_device *pdev)
-- 
2.20.1

[PATCH 02/11] remoteproc: stm32: Request IRQ with platform device

[Mathieu Poirier]

Request IRQ with platform device rather than remote proc in order to
call stm32_rproc_parse_dt() before rproc_alloc().  That way we can
know whether we need to synchronise with the MCU or not.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/remoteproc/stm32_rproc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index cde4b1a56381..0c1f0b84e057 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -261,7 +261,8 @@ static int stm32_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
 
 static irqreturn_t stm32_rproc_wdg(int irq, void *data)
 {
-	struct rproc *rproc = data;
+	struct platform_device *pdev = data;
+	struct rproc *rproc = platform_get_drvdata(pdev);
 
 	rproc_report_crash(rproc, RPROC_WATCHDOG);
 
@@ -553,7 +554,7 @@ static int stm32_rproc_parse_dt(struct platform_device *pdev)
 
 	if (irq > 0) {
 		err = devm_request_irq(dev, irq, stm32_rproc_wdg, 0,
-				       dev_name(dev), rproc);
+				       dev_name(dev), pdev);
 		if (err) {
 			dev_err(dev, "failed to request wdg irq\n");
 			return err;
-- 
2.20.1

[PATCH 03/11] remoteproc: stm32: Decouple rproc from DT parsing

[Mathieu Poirier]

Remove the remote processor from the process of parsing the device tree
since (1) there is no correlation between them and (2) to use the
information that was gathered to make a decision on whether to
synchronise with the MCU or not.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/remoteproc/stm32_rproc.c | 38 +++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index 0c1f0b84e057..ca60c917e218 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -538,12 +538,11 @@ static int stm32_rproc_get_syscon(struct device_node *np, const char *prop,
 	return err;
 }
 
-static int stm32_rproc_parse_dt(struct platform_device *pdev)
+static int stm32_rproc_parse_dt(struct platform_device *pdev,
+				struct stm32_rproc *ddata, bool *auto_boot)
 {
 	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;
@@ -589,7 +588,7 @@ static int stm32_rproc_parse_dt(struct platform_device *pdev)
 
 	err = regmap_read(tz.map, tz.reg, &tzen);
 	if (err) {
-		dev_err(&rproc->dev, "failed to read tzen\n");
+		dev_err(dev, "failed to read tzen\n");
 		return err;
 	}
 	ddata->secured_soc = tzen & tz.mask;
@@ -605,7 +604,7 @@ static int stm32_rproc_parse_dt(struct platform_device *pdev)
 	if (err)
 		dev_warn(dev, "failed to get pdds\n");
 
-	rproc->auto_boot = of_property_read_bool(np, "st,auto-boot");
+	*auto_boot = of_property_read_bool(np, "st,auto-boot");
 
 	return stm32_rproc_of_memory_translations(pdev, ddata);
 }
@@ -616,18 +615,29 @@ static int stm32_rproc_probe(struct platform_device *pdev)
 	struct stm32_rproc *ddata;
 	struct device_node *np = dev->of_node;
 	struct rproc *rproc;
+	bool auto_boot = false;
 	int ret;
 
 	ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
 	if (ret)
 		return ret;
 
-	rproc = rproc_alloc(dev, np->name, &st_rproc_ops, NULL, sizeof(*ddata));
-	if (!rproc)
+	ddata = kzalloc(sizeof(*ddata), GFP_KERNEL);
+	if (!ddata)
 		return -ENOMEM;
 
+	ret = stm32_rproc_parse_dt(pdev, ddata, &auto_boot);
+	if (ret)
+		goto free_ddata;
+
+	rproc = rproc_alloc(dev, np->name, &st_rproc_ops, NULL, sizeof(*ddata));
+	if (!rproc) {
+		ret = -ENOMEM;
+		goto free_ddata;
+	}
+
+	rproc->auto_boot = auto_boot;
 	rproc->has_iommu = false;
-	ddata = rproc->priv;
 	ddata->workqueue = create_workqueue(dev_name(dev));
 	if (!ddata->workqueue) {
 		dev_err(dev, "cannot create workqueue\n");
@@ -635,20 +645,20 @@ static int stm32_rproc_probe(struct platform_device *pdev)
 		goto free_rproc;
 	}
 
-	platform_set_drvdata(pdev, rproc);
+	memcpy(rproc->priv, ddata, sizeof(*ddata));
 
-	ret = stm32_rproc_parse_dt(pdev);
-	if (ret)
-		goto free_wkq;
+	platform_set_drvdata(pdev, rproc);
 
 	ret = stm32_rproc_request_mbox(rproc);
 	if (ret)
-		goto free_rproc;
+		goto free_wkq;
 
 	ret = rproc_add(rproc);
 	if (ret)
 		goto free_mb;
 
+	kfree(ddata);
+
 	return 0;
 
 free_mb:
@@ -661,6 +671,8 @@ static int stm32_rproc_probe(struct platform_device *pdev)
 		device_init_wakeup(dev, false);
 	}
 	rproc_free(rproc);
+free_ddata:
+	kfree(ddata);
 	return ret;
 }
 
-- 
2.20.1

[PATCH 04/11] remoteproc: stm32: Remove memory translation from DT parsing

[Mathieu Poirier]

Other than one has to be done after the other, there is no correlation
between memory translation and DT parsing.  As move function
stm32_rproc_of_memory_translations() to stm32_rproc_probe() so that
stm32_rproc_parse_dt() can be extended to look for synchronisation
related binding in a clean way.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/remoteproc/stm32_rproc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index ca60c917e218..a3e278490bb4 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -606,7 +606,7 @@ static int stm32_rproc_parse_dt(struct platform_device *pdev,
 
 	*auto_boot = of_property_read_bool(np, "st,auto-boot");
 
-	return stm32_rproc_of_memory_translations(pdev, ddata);
+	return 0;
 }
 
 static int stm32_rproc_probe(struct platform_device *pdev)
@@ -630,6 +630,10 @@ static int stm32_rproc_probe(struct platform_device *pdev)
 	if (ret)
 		goto free_ddata;
 
+	ret = stm32_rproc_of_memory_translations(pdev, ddata);
+	if (ret)
+		goto free_ddata;
+
 	rproc = rproc_alloc(dev, np->name, &st_rproc_ops, NULL, sizeof(*ddata));
 	if (!rproc) {
 		ret = -ENOMEM;
-- 
2.20.1

[PATCH 05/11] remoteproc: stm32: Parse syscon that will manage M4 synchronisation

[Mathieu Poirier]

Get from the DT the syncon to probe the state of the remote processor
and the location of the resource table.

Mainly based on the work published by Arnaud Pouliquen [1].

[1]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=239877

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/remoteproc/stm32_rproc.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index a3e278490bb4..7d5d4a7dbb04 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -70,6 +70,8 @@ struct stm32_rproc {
 	struct reset_control *rst;
 	struct stm32_syscon hold_boot;
 	struct stm32_syscon pdds;
+	struct stm32_syscon m4_state;
+	struct stm32_syscon rsctbl;
 	int wdg_irq;
 	u32 nb_rmems;
 	struct stm32_rproc_mem *rmems;
@@ -606,6 +608,30 @@ static int stm32_rproc_parse_dt(struct platform_device *pdev,
 
 	*auto_boot = of_property_read_bool(np, "st,auto-boot");
 
+	/*
+	 * See if we can check the M4 status, i.e if it was started
+	 * from the boot loader or not.
+	 */
+        err = stm32_rproc_get_syscon(np, "st,syscfg-m4-state",
+                                     &ddata->m4_state);
+        if (err) {
+		/* remember this */
+		ddata->m4_state.map = NULL;
+                /* no coprocessor state syscon (optional) */
+                dev_warn(dev, "m4 state not supported\n");
+
+		/* no need to go further */
+		return 0;
+        }
+
+	/* See if we can get the resource table */
+        err = stm32_rproc_get_syscon(np, "st,syscfg-rsc-tbl",
+                                     &ddata->rsctbl);
+	if (err) {
+		/* no rsc table syscon (optional) */
+		dev_warn(dev, "rsc tbl syscon not supported\n");
+	}
+
 	return 0;
 }
 
-- 
2.20.1

[PATCH 06/11] remoteproc: stm32: Get coprocessor state

[Mathieu Poirier]

Introduce the required mechanic to get the state of the M4 when the
remoteproc core is initialising.

Mainly based on the work published by Arnaud Pouliquen [1].

[1]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=239877

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/remoteproc/stm32_rproc.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index 7d5d4a7dbb04..b8af15dd0510 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -38,6 +38,15 @@
 #define STM32_MBX_VQ1_ID	1
 #define STM32_MBX_SHUTDOWN	"shutdown"
 
+#define RSC_TBL_SIZE		(1024)
+
+#define M4_STATE_OFF		0
+#define M4_STATE_INI		1
+#define M4_STATE_CRUN		2
+#define M4_STATE_CSTOP		3
+#define M4_STATE_STANDBY	4
+#define M4_STATE_CRASH		5
+
 struct stm32_syscon {
 	struct regmap *map;
 	u32 reg;
@@ -635,12 +644,23 @@ static int stm32_rproc_parse_dt(struct platform_device *pdev,
 	return 0;
 }
 
+static int stm32_rproc_get_m4_status(struct stm32_rproc *ddata,
+				     unsigned int *state)
+{
+	/* See stm32_rproc_parse_dt() */
+	if (!ddata->m4_state.map)
+		return -EINVAL;
+
+	return regmap_read(ddata->m4_state.map, ddata->m4_state.reg, state);
+}
+
 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;
+	unsigned int state;
 	bool auto_boot = false;
 	int ret;
 
@@ -660,6 +680,15 @@ static int stm32_rproc_probe(struct platform_device *pdev)
 	if (ret)
 		goto free_ddata;
 
+	ret = stm32_rproc_get_m4_status(ddata, &state);
+	if (ret) {
+		/*
+		 * We couldn't get the coprocessor's state, assume
+		 * it is not running.
+		 */
+		state = M4_STATE_OFF;
+	}
+
 	rproc = rproc_alloc(dev, np->name, &st_rproc_ops, NULL, sizeof(*ddata));
 	if (!rproc) {
 		ret = -ENOMEM;
-- 
2.20.1

[PATCH 07/11] remoteproc: stm32: Get loaded resource table for synchronisation

[Mathieu Poirier]

Get the resource table location when synchronising with the M4 so
that the remoteproc and rpmsg subsystem can be initialised properly.

Mainly based on the work published by Arnaud Pouliquen [1].

[1]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=239877

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/remoteproc/stm32_rproc.c | 66 ++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index b8af15dd0510..5bac0baf8f4c 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -87,6 +87,7 @@ struct stm32_rproc {
 	struct stm32_mbox mb[MBOX_NB_MBX];
 	struct workqueue_struct *workqueue;
 	bool secured_soc;
+	void __iomem *rsc_va;
 };
 
 static int stm32_rproc_pa_to_da(struct rproc *rproc, phys_addr_t pa, u64 *da)
@@ -654,6 +655,65 @@ static int stm32_rproc_get_m4_status(struct stm32_rproc *ddata,
 	return regmap_read(ddata->m4_state.map, ddata->m4_state.reg, state);
 }
 
+static int stm32_rproc_da_to_pa(struct platform_device *pdev,
+				struct stm32_rproc *ddata,
+				u64 da, phys_addr_t *pa)
+{
+	struct device *dev = &pdev->dev;
+	struct stm32_rproc_mem *p_mem;
+	unsigned int i;
+
+	for (i = 0; i < ddata->nb_rmems; i++) {
+		p_mem = &ddata->rmems[i];
+
+		if (da < p_mem->dev_addr ||
+		    da >= p_mem->dev_addr + p_mem->size)
+			continue;
+
+		*pa = da - p_mem->dev_addr + p_mem->bus_addr;
+		dev_dbg(dev, "da %llx to pa %#x\n", da, *pa);
+
+		return 0;
+	}
+
+	dev_err(dev, "can't translate da %llx\n", da);
+
+	return -EINVAL;
+}
+
+static int stm32_rproc_get_loaded_rsc_table(struct platform_device *pdev,
+					    struct stm32_rproc *ddata)
+{
+	struct device *dev = &pdev->dev;
+	phys_addr_t rsc_pa;
+	u32 rsc_da;
+	int err;
+
+	err = regmap_read(ddata->rsctbl.map, ddata->rsctbl.reg, &rsc_da);
+	if (err) {
+		dev_err(dev, "failed to read rsc tbl addr\n");
+		return err;
+	}
+
+	if (!rsc_da)
+		/* no rsc table */
+		return 0;
+
+	err = stm32_rproc_da_to_pa(pdev, ddata, rsc_da, &rsc_pa);
+	if (err)
+		return err;
+
+	ddata->rsc_va = devm_ioremap_wc(dev, rsc_pa, RSC_TBL_SIZE);
+	if (IS_ERR_OR_NULL(ddata->rsc_va)) {
+		dev_err(dev, "Unable to map memory region: %pa+%zx\n",
+			&rsc_pa, RSC_TBL_SIZE);
+		ddata->rsc_va = NULL;
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
 static int stm32_rproc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -689,6 +749,12 @@ static int stm32_rproc_probe(struct platform_device *pdev)
 		state = M4_STATE_OFF;
 	}
 
+	if (state == M4_STATE_CRUN) {
+		ret = stm32_rproc_get_loaded_rsc_table(pdev, ddata);
+		if (ret)
+			goto free_ddata;
+	}
+
 	rproc = rproc_alloc(dev, np->name, &st_rproc_ops, NULL, sizeof(*ddata));
 	if (!rproc) {
 		ret = -ENOMEM;
-- 
2.20.1

[PATCH 08/11] remoteproc: stm32: Introduce new start and stop ops for synchronisation

[Mathieu Poirier]

Introduce new start and stop rproc_ops functions to be used when
synchonising with an MCU.

Mainly based on the work published by Arnaud Pouliquen [1].

[1]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=239877

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/remoteproc/stm32_rproc.c | 37 ++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index 5bac0baf8f4c..734605a9223e 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -449,6 +449,13 @@ static int stm32_rproc_start(struct rproc *rproc)
 	return stm32_rproc_set_hold_boot(rproc, true);
 }
 
+static int stm32_rproc_sync_start(struct rproc *rproc)
+{
+	stm32_rproc_add_coredump_trace(rproc);
+
+	return stm32_rproc_set_hold_boot(rproc, true);
+}
+
 static int stm32_rproc_stop(struct rproc *rproc)
 {
 	struct stm32_rproc *ddata = rproc->priv;
@@ -489,6 +496,30 @@ static int stm32_rproc_stop(struct rproc *rproc)
 	return 0;
 }
 
+static int stm32_rproc_sync_stop(struct rproc *rproc)
+{
+	struct stm32_rproc *ddata = rproc->priv;
+	int err;
+
+	err = stm32_rproc_stop(rproc);
+	if (err)
+		return err;
+
+	/* update copro state to OFF */
+	if (ddata->m4_state.map) {
+		err = regmap_update_bits(ddata->m4_state.map,
+					 ddata->m4_state.reg,
+					 ddata->m4_state.mask,
+					 M4_STATE_OFF);
+		if (err) {
+			dev_err(&rproc->dev, "failed to set copro state\n");
+			return err;
+		}
+	}
+
+	return 0;
+}
+
 static void stm32_rproc_kick(struct rproc *rproc, int vqid)
 {
 	struct stm32_rproc *ddata = rproc->priv;
@@ -522,6 +553,12 @@ static struct rproc_ops st_rproc_ops = {
 	.get_boot_addr	= rproc_elf_get_boot_addr,
 };
 
+static __maybe_unused struct rproc_ops st_rproc_sync_ops = {
+	.start		= stm32_rproc_sync_start,
+	.stop		= stm32_rproc_sync_stop,
+	.kick		= stm32_rproc_kick,
+};
+
 static const struct of_device_id stm32_rproc_match[] = {
 	{ .compatible = "st,stm32mp1-m4" },
 	{},
-- 
2.20.1

[PATCH 09/11] remoteproc: stm32: Introduce new parse fw ops for synchronisation

[Mathieu Poirier]

Introduce new parse firmware rproc_ops functions to be used when
synchonising with an MCU.

Mainly based on the work published by Arnaud Pouliquen [1].

[1]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=239877

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/remoteproc/stm32_rproc.c | 49 +++++++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index 734605a9223e..4268d71f191d 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -215,7 +215,34 @@ static int stm32_rproc_elf_load_rsc_table(struct rproc *rproc,
 	return 0;
 }
 
-static int stm32_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
+static int stm32_rproc_sync_elf_load_rsc_table(struct rproc *rproc,
+					       const struct firmware *fw)
+{
+	struct resource_table *table = NULL;
+	struct stm32_rproc *ddata = rproc->priv;
+
+	if (ddata->rsc_va) {
+		table = (struct resource_table *)ddata->rsc_va;
+		/* Assuming that the resource table fits in 1kB is fair */
+		rproc->cached_table = kmemdup(table, RSC_TBL_SIZE, GFP_KERNEL);
+		if (!rproc->cached_table)
+			return -ENOMEM;
+
+		rproc->table_ptr = rproc->cached_table;
+		rproc->table_sz = RSC_TBL_SIZE;
+		return 0;
+	}
+
+	rproc->cached_table = NULL;
+	rproc->table_ptr = NULL;
+	rproc->table_sz = 0;
+
+	dev_warn(&rproc->dev, "no resource table found for this firmware\n");
+	return 0;
+}
+
+static int stm32_rproc_parse_memory_regions(struct rproc *rproc,
+					    const struct firmware *fw)
 {
 	struct device *dev = rproc->dev.parent;
 	struct device_node *np = dev->of_node;
@@ -268,9 +295,28 @@ static int stm32_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
 		index++;
 	}
 
+	return 0;
+}
+
+static int stm32_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)
+{
+	int ret = stm32_rproc_parse_memory_regions(rproc, fw);
+	if (ret)
+		return ret;
+
 	return stm32_rproc_elf_load_rsc_table(rproc, fw);
 }
 
+static int stm32_rproc_sync_parse_fw(struct rproc *rproc,
+				     const struct firmware *fw)
+{
+	int ret = stm32_rproc_parse_memory_regions(rproc, fw);
+	if (ret)
+		return ret;
+
+	return stm32_rproc_sync_elf_load_rsc_table(rproc, fw);
+}
+
 static irqreturn_t stm32_rproc_wdg(int irq, void *data)
 {
 	struct platform_device *pdev = data;
@@ -557,6 +603,7 @@ static __maybe_unused struct rproc_ops st_rproc_sync_ops = {
 	.start		= stm32_rproc_sync_start,
 	.stop		= stm32_rproc_sync_stop,
 	.kick		= stm32_rproc_kick,
+	.parse_fw	= stm32_rproc_sync_parse_fw,
 };
 
 static const struct of_device_id stm32_rproc_match[] = {
-- 
2.20.1

[PATCH 10/11] remoteproc: stm32: Introduce new loaded rsc ops for synchronisation

[Mathieu Poirier]

Introduce new elf find loaded resource table rproc_ops functions to be
used when synchonising with an MCU.

Mainly based on the work published by Arnaud Pouliquen [1].

[1]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=239877

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/remoteproc/stm32_rproc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index 4268d71f191d..07be306c0fb1 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -317,6 +317,15 @@ static int stm32_rproc_sync_parse_fw(struct rproc *rproc,
 	return stm32_rproc_sync_elf_load_rsc_table(rproc, fw);
 }
 
+static struct resource_table *
+stm32_rproc_sync_elf_find_loaded_rsc_table(struct rproc *rproc,
+					   const struct firmware *fw)
+{
+	struct stm32_rproc *ddata = rproc->priv;
+
+	return (struct resource_table *)ddata->rsc_va;
+}
+
 static irqreturn_t stm32_rproc_wdg(int irq, void *data)
 {
 	struct platform_device *pdev = data;
@@ -604,6 +613,7 @@ static __maybe_unused struct rproc_ops st_rproc_sync_ops = {
 	.stop		= stm32_rproc_sync_stop,
 	.kick		= stm32_rproc_kick,
 	.parse_fw	= stm32_rproc_sync_parse_fw,
+	.find_loaded_rsc_table = stm32_rproc_sync_elf_find_loaded_rsc_table,
 };
 
 static const struct of_device_id stm32_rproc_match[] = {
-- 
2.20.1

[PATCH 11/11] remoteproc: stm32: Allocate rproc for synchronisation with MCU

[Mathieu Poirier]

Allocate remote processor structure with state machine if the MCU
has already been started by an external entity.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/remoteproc/stm32_rproc.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index 07be306c0fb1..f320ef9ee286 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -608,7 +608,7 @@ static struct rproc_ops st_rproc_ops = {
 	.get_boot_addr	= rproc_elf_get_boot_addr,
 };
 
-static __maybe_unused struct rproc_ops st_rproc_sync_ops = {
+static struct rproc_ops st_rproc_sync_ops = {
 	.start		= stm32_rproc_sync_start,
 	.stop		= stm32_rproc_sync_stop,
 	.kick		= stm32_rproc_kick,
@@ -616,6 +616,12 @@ static __maybe_unused struct rproc_ops st_rproc_sync_ops = {
 	.find_loaded_rsc_table = stm32_rproc_sync_elf_find_loaded_rsc_table,
 };
 
+static struct rproc_sync_states st_sync_states = {
+	.on_init = true, /* sync with MCU when the kernel boots */
+	.after_stop = false, /* don't resync with MCU if stopped from sysfs */
+	.after_crash = false, /* don't resync with MCU after a crash */
+};
+
 static const struct of_device_id stm32_rproc_match[] = {
 	{ .compatible = "st,stm32mp1-m4" },
 	{},
@@ -847,15 +853,22 @@ static int stm32_rproc_probe(struct platform_device *pdev)
 		ret = stm32_rproc_get_loaded_rsc_table(pdev, ddata);
 		if (ret)
 			goto free_ddata;
+
+		rproc = rproc_alloc_state_machine(dev, np->name, &st_rproc_ops,
+						  &st_rproc_sync_ops,
+						  &st_sync_states, NULL,
+						  sizeof(*ddata));
+	} else {
+		rproc = rproc_alloc(dev, np->name, &st_rproc_ops,
+				    NULL, sizeof(*ddata));
+		rproc->auto_boot = auto_boot;
 	}
 
-	rproc = rproc_alloc(dev, np->name, &st_rproc_ops, NULL, sizeof(*ddata));
 	if (!rproc) {
 		ret = -ENOMEM;
 		goto free_ddata;
 	}
 
-	rproc->auto_boot = auto_boot;
 	rproc->has_iommu = false;
 	ddata->workqueue = create_workqueue(dev_name(dev));
 	if (!ddata->workqueue) {
-- 
2.20.1

RE: [PATCH 01/11] remoteproc: stm32: Decouple rproc from memory translation

[Loic PALLARDY]

> -----Original Message-----
> From: linux-remoteproc-owner@vger.kernel.org <linux-remoteproc-
> owner@vger.kernel.org> On Behalf Of Mathieu Poirier
> Sent: mardi 24 mars 2020 23:03
> To: bjorn.andersson@linaro.org
> Cc: ohad@wizery.com; Loic PALLARDY <loic.pallardy@st.com>; s-
> anna@ti.com; peng.fan@nxp.com; Arnaud POULIQUEN
> <arnaud.pouliquen@st.com>; Fabien DESSENNE
> <fabien.dessenne@st.com>; linux-remoteproc@vger.kernel.org
> Subject: [PATCH 01/11] remoteproc: stm32: Decouple rproc from memory
> translation
> 
> Remove the remote processor from the process of parsing the memory
> ranges since there is no correlation between them.
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Thanks Mathieu

Reviewed-by: Loic Pallardy <loic.pallardy@st.com>
> ---
>  drivers/remoteproc/stm32_rproc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c
> b/drivers/remoteproc/stm32_rproc.c
> index a18f88044111..cde4b1a56381 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -127,10 +127,10 @@ static int stm32_rproc_mem_release(struct rproc
> *rproc,
>  	return 0;
>  }
> 
> -static int stm32_rproc_of_memory_translations(struct rproc *rproc)
> +static int stm32_rproc_of_memory_translations(struct platform_device
> *pdev,
> +					      struct stm32_rproc *ddata)
>  {
> -	struct device *parent, *dev = rproc->dev.parent;
> -	struct stm32_rproc *ddata = rproc->priv;
> +	struct device *parent, *dev = &pdev->dev;
>  	struct device_node *np;
>  	struct stm32_rproc_mem *p_mems;
>  	struct stm32_rproc_mem_ranges *mem_range;
> @@ -606,7 +606,7 @@ static int stm32_rproc_parse_dt(struct
> platform_device *pdev)
> 
>  	rproc->auto_boot = of_property_read_bool(np, "st,auto-boot");
> 
> -	return stm32_rproc_of_memory_translations(rproc);
> +	return stm32_rproc_of_memory_translations(pdev, ddata);
>  }
> 
>  static int stm32_rproc_probe(struct platform_device *pdev)
> --
> 2.20.1

RE: [PATCH 02/11] remoteproc: stm32: Request IRQ with platform device

[Loic PALLARDY]

> -----Original Message-----
> From: Mathieu Poirier <mathieu.poirier@linaro.org>
> Sent: mardi 24 mars 2020 23:03
> To: bjorn.andersson@linaro.org
> Cc: ohad@wizery.com; Loic PALLARDY <loic.pallardy@st.com>; s-
> anna@ti.com; peng.fan@nxp.com; Arnaud POULIQUEN
> <arnaud.pouliquen@st.com>; Fabien DESSENNE
> <fabien.dessenne@st.com>; linux-remoteproc@vger.kernel.org
> Subject: [PATCH 02/11] remoteproc: stm32: Request IRQ with platform
> device
> 
> Request IRQ with platform device rather than remote proc in order to
> call stm32_rproc_parse_dt() before rproc_alloc().  That way we can
> know whether we need to synchronise with the MCU or not.
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Thanks Mathieu
 
Reviewed-by: Loic Pallardy <loic.pallardy@st.com> 

> ---
>  drivers/remoteproc/stm32_rproc.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c
> b/drivers/remoteproc/stm32_rproc.c
> index cde4b1a56381..0c1f0b84e057 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -261,7 +261,8 @@ static int stm32_rproc_parse_fw(struct rproc *rproc,
> const struct firmware *fw)
> 
>  static irqreturn_t stm32_rproc_wdg(int irq, void *data)
>  {
> -	struct rproc *rproc = data;
> +	struct platform_device *pdev = data;
> +	struct rproc *rproc = platform_get_drvdata(pdev);
> 
>  	rproc_report_crash(rproc, RPROC_WATCHDOG);
> 
> @@ -553,7 +554,7 @@ static int stm32_rproc_parse_dt(struct
> platform_device *pdev)
> 
>  	if (irq > 0) {
>  		err = devm_request_irq(dev, irq, stm32_rproc_wdg, 0,
> -				       dev_name(dev), rproc);
> +				       dev_name(dev), pdev);
>  		if (err) {
>  			dev_err(dev, "failed to request wdg irq\n");
>  			return err;
> --
> 2.20.1

RE: [PATCH 03/11] remoteproc: stm32: Decouple rproc from DT parsing

[Loic PALLARDY]

> -----Original Message-----
> From: Mathieu Poirier <mathieu.poirier@linaro.org>
> Sent: mardi 24 mars 2020 23:03
> To: bjorn.andersson@linaro.org
> Cc: ohad@wizery.com; Loic PALLARDY <loic.pallardy@st.com>; s-
> anna@ti.com; peng.fan@nxp.com; Arnaud POULIQUEN
> <arnaud.pouliquen@st.com>; Fabien DESSENNE
> <fabien.dessenne@st.com>; linux-remoteproc@vger.kernel.org
> Subject: [PATCH 03/11] remoteproc: stm32: Decouple rproc from DT parsing
> 
> Remove the remote processor from the process of parsing the device tree
> since (1) there is no correlation between them and (2) to use the
> information that was gathered to make a decision on whether to
> synchronise with the MCU or not.
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Thanks Mathieu
 
Reviewed-by: Loic Pallardy <loic.pallardy@st.com>  

> ---
>  drivers/remoteproc/stm32_rproc.c | 38 +++++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c
> b/drivers/remoteproc/stm32_rproc.c
> index 0c1f0b84e057..ca60c917e218 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -538,12 +538,11 @@ static int stm32_rproc_get_syscon(struct
> device_node *np, const char *prop,
>  	return err;
>  }
> 
> -static int stm32_rproc_parse_dt(struct platform_device *pdev)
> +static int stm32_rproc_parse_dt(struct platform_device *pdev,
> +				struct stm32_rproc *ddata, bool *auto_boot)
>  {
>  	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;
> @@ -589,7 +588,7 @@ static int stm32_rproc_parse_dt(struct
> platform_device *pdev)
> 
>  	err = regmap_read(tz.map, tz.reg, &tzen);
>  	if (err) {
> -		dev_err(&rproc->dev, "failed to read tzen\n");
> +		dev_err(dev, "failed to read tzen\n");
>  		return err;
>  	}
>  	ddata->secured_soc = tzen & tz.mask;
> @@ -605,7 +604,7 @@ static int stm32_rproc_parse_dt(struct
> platform_device *pdev)
>  	if (err)
>  		dev_warn(dev, "failed to get pdds\n");
> 
> -	rproc->auto_boot = of_property_read_bool(np, "st,auto-boot");
> +	*auto_boot = of_property_read_bool(np, "st,auto-boot");
> 
>  	return stm32_rproc_of_memory_translations(pdev, ddata);
>  }
> @@ -616,18 +615,29 @@ static int stm32_rproc_probe(struct
> platform_device *pdev)
>  	struct stm32_rproc *ddata;
>  	struct device_node *np = dev->of_node;
>  	struct rproc *rproc;
> +	bool auto_boot = false;
>  	int ret;
> 
>  	ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
>  	if (ret)
>  		return ret;
> 
> -	rproc = rproc_alloc(dev, np->name, &st_rproc_ops, NULL,
> sizeof(*ddata));
> -	if (!rproc)
> +	ddata = kzalloc(sizeof(*ddata), GFP_KERNEL);
> +	if (!ddata)
>  		return -ENOMEM;
> 
> +	ret = stm32_rproc_parse_dt(pdev, ddata, &auto_boot);
> +	if (ret)
> +		goto free_ddata;
> +
> +	rproc = rproc_alloc(dev, np->name, &st_rproc_ops, NULL,
> sizeof(*ddata));
> +	if (!rproc) {
> +		ret = -ENOMEM;
> +		goto free_ddata;
> +	}
> +
> +	rproc->auto_boot = auto_boot;
>  	rproc->has_iommu = false;
> -	ddata = rproc->priv;
>  	ddata->workqueue = create_workqueue(dev_name(dev));
>  	if (!ddata->workqueue) {
>  		dev_err(dev, "cannot create workqueue\n");
> @@ -635,20 +645,20 @@ static int stm32_rproc_probe(struct
> platform_device *pdev)
>  		goto free_rproc;
>  	}
> 
> -	platform_set_drvdata(pdev, rproc);
> +	memcpy(rproc->priv, ddata, sizeof(*ddata));
> 
> -	ret = stm32_rproc_parse_dt(pdev);
> -	if (ret)
> -		goto free_wkq;
> +	platform_set_drvdata(pdev, rproc);
> 
>  	ret = stm32_rproc_request_mbox(rproc);
>  	if (ret)
> -		goto free_rproc;
> +		goto free_wkq;
> 
>  	ret = rproc_add(rproc);
>  	if (ret)
>  		goto free_mb;
> 
> +	kfree(ddata);
> +
>  	return 0;
> 
>  free_mb:
> @@ -661,6 +671,8 @@ static int stm32_rproc_probe(struct platform_device
> *pdev)
>  		device_init_wakeup(dev, false);
>  	}
>  	rproc_free(rproc);
> +free_ddata:
> +	kfree(ddata);
>  	return ret;
>  }
> 
> --
> 2.20.1

RE: [PATCH 04/11] remoteproc: stm32: Remove memory translation from DT parsing

[Loic PALLARDY]

> -----Original Message-----
> From: Mathieu Poirier <mathieu.poirier@linaro.org>
> Sent: mardi 24 mars 2020 23:03
> To: bjorn.andersson@linaro.org
> Cc: ohad@wizery.com; Loic PALLARDY <loic.pallardy@st.com>; s-
> anna@ti.com; peng.fan@nxp.com; Arnaud POULIQUEN
> <arnaud.pouliquen@st.com>; Fabien DESSENNE
> <fabien.dessenne@st.com>; linux-remoteproc@vger.kernel.org
> Subject: [PATCH 04/11] remoteproc: stm32: Remove memory translation
> from DT parsing
> 
> Other than one has to be done after the other, there is no correlation
> between memory translation and DT parsing.  As move function
> stm32_rproc_of_memory_translations() to stm32_rproc_probe() so that
> stm32_rproc_parse_dt() can be extended to look for synchronisation
> related binding in a clean way.
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Thanks Mathieu
 
Reviewed-by: Loic Pallardy <loic.pallardy@st.com>

> ---
>  drivers/remoteproc/stm32_rproc.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c
> b/drivers/remoteproc/stm32_rproc.c
> index ca60c917e218..a3e278490bb4 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -606,7 +606,7 @@ static int stm32_rproc_parse_dt(struct
> platform_device *pdev,
> 
>  	*auto_boot = of_property_read_bool(np, "st,auto-boot");
> 
> -	return stm32_rproc_of_memory_translations(pdev, ddata);
> +	return 0;
>  }
> 
>  static int stm32_rproc_probe(struct platform_device *pdev)
> @@ -630,6 +630,10 @@ static int stm32_rproc_probe(struct platform_device
> *pdev)
>  	if (ret)
>  		goto free_ddata;
> 
> +	ret = stm32_rproc_of_memory_translations(pdev, ddata);
> +	if (ret)
> +		goto free_ddata;
> +
>  	rproc = rproc_alloc(dev, np->name, &st_rproc_ops, NULL,
> sizeof(*ddata));
>  	if (!rproc) {
>  		ret = -ENOMEM;
> --
> 2.20.1

RE: [PATCH 05/11] remoteproc: stm32: Parse syscon that will manage M4 synchronisation

[Loic PALLARDY]

> -----Original Message-----
> From: Mathieu Poirier <mathieu.poirier@linaro.org>
> Sent: mardi 24 mars 2020 23:03
> To: bjorn.andersson@linaro.org
> Cc: ohad@wizery.com; Loic PALLARDY <loic.pallardy@st.com>; s-
> anna@ti.com; peng.fan@nxp.com; Arnaud POULIQUEN
> <arnaud.pouliquen@st.com>; Fabien DESSENNE
> <fabien.dessenne@st.com>; linux-remoteproc@vger.kernel.org
> Subject: [PATCH 05/11] remoteproc: stm32: Parse syscon that will manage
> M4 synchronisation
> 
> Get from the DT the syncon to probe the state of the remote processor
> and the location of the resource table.
> 
> Mainly based on the work published by Arnaud Pouliquen [1].
> 
> [1]. https://patchwork.kernel.org/project/linux-
> remoteproc/list/?series=239877
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Thanks Mathieu

Reviewed-by: Loic Pallardy <loic.pallardy@st.com>
> ---
>  drivers/remoteproc/stm32_rproc.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c
> b/drivers/remoteproc/stm32_rproc.c
> index a3e278490bb4..7d5d4a7dbb04 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -70,6 +70,8 @@ struct stm32_rproc {
>  	struct reset_control *rst;
>  	struct stm32_syscon hold_boot;
>  	struct stm32_syscon pdds;
> +	struct stm32_syscon m4_state;
> +	struct stm32_syscon rsctbl;
>  	int wdg_irq;
>  	u32 nb_rmems;
>  	struct stm32_rproc_mem *rmems;
> @@ -606,6 +608,30 @@ static int stm32_rproc_parse_dt(struct
> platform_device *pdev,
> 
>  	*auto_boot = of_property_read_bool(np, "st,auto-boot");
> 
> +	/*
> +	 * See if we can check the M4 status, i.e if it was started
> +	 * from the boot loader or not.
> +	 */
> +        err = stm32_rproc_get_syscon(np, "st,syscfg-m4-state",
> +                                     &ddata->m4_state);
> +        if (err) {
> +		/* remember this */
> +		ddata->m4_state.map = NULL;
> +                /* no coprocessor state syscon (optional) */
> +                dev_warn(dev, "m4 state not supported\n");
> +
> +		/* no need to go further */
> +		return 0;
> +        }
> +
> +	/* See if we can get the resource table */
> +        err = stm32_rproc_get_syscon(np, "st,syscfg-rsc-tbl",
> +                                     &ddata->rsctbl);
> +	if (err) {
> +		/* no rsc table syscon (optional) */
> +		dev_warn(dev, "rsc tbl syscon not supported\n");
> +	}
> +
>  	return 0;
>  }
> 
> --
> 2.20.1

RE: [PATCH 06/11] remoteproc: stm32: Get coprocessor state

[Loic PALLARDY]

Hi Mathieu,

> -----Original Message-----
> From: linux-remoteproc-owner@vger.kernel.org <linux-remoteproc-
> owner@vger.kernel.org> On Behalf Of Mathieu Poirier
> Sent: mardi 24 mars 2020 23:03
> To: bjorn.andersson@linaro.org
> Cc: ohad@wizery.com; Loic PALLARDY <loic.pallardy@st.com>; s-
> anna@ti.com; peng.fan@nxp.com; Arnaud POULIQUEN
> <arnaud.pouliquen@st.com>; Fabien DESSENNE
> <fabien.dessenne@st.com>; linux-remoteproc@vger.kernel.org
> Subject: [PATCH 06/11] remoteproc: stm32: Get coprocessor state
> 
> Introduce the required mechanic to get the state of the M4 when the
> remoteproc core is initialising.
> 
> Mainly based on the work published by Arnaud Pouliquen [1].
> 
> [1]. https://patchwork.kernel.org/project/linux-
> remoteproc/list/?series=239877
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>  drivers/remoteproc/stm32_rproc.c | 29
> +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c
> b/drivers/remoteproc/stm32_rproc.c
> index 7d5d4a7dbb04..b8af15dd0510 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -38,6 +38,15 @@
>  #define STM32_MBX_VQ1_ID	1
>  #define STM32_MBX_SHUTDOWN	"shutdown"
> 
> +#define RSC_TBL_SIZE		(1024)
> +
> +#define M4_STATE_OFF		0
> +#define M4_STATE_INI		1
Typo M4_STATE_INI -> M4_STATE_INIT

Else ok for me
Reviewed-by: Loic Pallardy <loic.pallardy@st.com>

Regards,
Loic

> +#define M4_STATE_CRUN		2
> +#define M4_STATE_CSTOP		3
> +#define M4_STATE_STANDBY	4
> +#define M4_STATE_CRASH		5
> +
>  struct stm32_syscon {
>  	struct regmap *map;
>  	u32 reg;
> @@ -635,12 +644,23 @@ static int stm32_rproc_parse_dt(struct
> platform_device *pdev,
>  	return 0;
>  }
> 
> +static int stm32_rproc_get_m4_status(struct stm32_rproc *ddata,
> +				     unsigned int *state)
> +{
> +	/* See stm32_rproc_parse_dt() */
> +	if (!ddata->m4_state.map)
> +		return -EINVAL;
> +
> +	return regmap_read(ddata->m4_state.map, ddata->m4_state.reg,
> state);
> +}
> +
>  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;
> +	unsigned int state;
>  	bool auto_boot = false;
>  	int ret;
> 
> @@ -660,6 +680,15 @@ static int stm32_rproc_probe(struct platform_device
> *pdev)
>  	if (ret)
>  		goto free_ddata;
> 
> +	ret = stm32_rproc_get_m4_status(ddata, &state);
> +	if (ret) {
> +		/*
> +		 * We couldn't get the coprocessor's state, assume
> +		 * it is not running.
> +		 */
> +		state = M4_STATE_OFF;
> +	}
> +
>  	rproc = rproc_alloc(dev, np->name, &st_rproc_ops, NULL,
> sizeof(*ddata));
>  	if (!rproc) {
>  		ret = -ENOMEM;
> --
> 2.20.1

RE: [PATCH 07/11] remoteproc: stm32: Get loaded resource table for synchronisation

[Loic PALLARDY]

> -----Original Message-----
> From: Mathieu Poirier <mathieu.poirier@linaro.org>
> Sent: mardi 24 mars 2020 23:03
> To: bjorn.andersson@linaro.org
> Cc: ohad@wizery.com; Loic PALLARDY <loic.pallardy@st.com>; s-
> anna@ti.com; peng.fan@nxp.com; Arnaud POULIQUEN
> <arnaud.pouliquen@st.com>; Fabien DESSENNE
> <fabien.dessenne@st.com>; linux-remoteproc@vger.kernel.org
> Subject: [PATCH 07/11] remoteproc: stm32: Get loaded resource table for
> synchronisation
> 
> Get the resource table location when synchronising with the M4 so
> that the remoteproc and rpmsg subsystem can be initialised properly.
> 
> Mainly based on the work published by Arnaud Pouliquen [1].
> 
> [1]. https://patchwork.kernel.org/project/linux-
> remoteproc/list/?series=239877
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Thanks Mathieu
 
Reviewed-by: Loic Pallardy <loic.pallardy@st.com>

> ---
>  drivers/remoteproc/stm32_rproc.c | 66
> ++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c
> b/drivers/remoteproc/stm32_rproc.c
> index b8af15dd0510..5bac0baf8f4c 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -87,6 +87,7 @@ struct stm32_rproc {
>  	struct stm32_mbox mb[MBOX_NB_MBX];
>  	struct workqueue_struct *workqueue;
>  	bool secured_soc;
> +	void __iomem *rsc_va;
>  };
> 
>  static int stm32_rproc_pa_to_da(struct rproc *rproc, phys_addr_t pa, u64
> *da)
> @@ -654,6 +655,65 @@ static int stm32_rproc_get_m4_status(struct
> stm32_rproc *ddata,
>  	return regmap_read(ddata->m4_state.map, ddata->m4_state.reg,
> state);
>  }
> 
> +static int stm32_rproc_da_to_pa(struct platform_device *pdev,
> +				struct stm32_rproc *ddata,
> +				u64 da, phys_addr_t *pa)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct stm32_rproc_mem *p_mem;
> +	unsigned int i;
> +
> +	for (i = 0; i < ddata->nb_rmems; i++) {
> +		p_mem = &ddata->rmems[i];
> +
> +		if (da < p_mem->dev_addr ||
> +		    da >= p_mem->dev_addr + p_mem->size)
> +			continue;
> +
> +		*pa = da - p_mem->dev_addr + p_mem->bus_addr;
> +		dev_dbg(dev, "da %llx to pa %#x\n", da, *pa);
> +
> +		return 0;
> +	}
> +
> +	dev_err(dev, "can't translate da %llx\n", da);
> +
> +	return -EINVAL;
> +}
> +
> +static int stm32_rproc_get_loaded_rsc_table(struct platform_device
> *pdev,
> +					    struct stm32_rproc *ddata)
> +{
> +	struct device *dev = &pdev->dev;
> +	phys_addr_t rsc_pa;
> +	u32 rsc_da;
> +	int err;
> +
> +	err = regmap_read(ddata->rsctbl.map, ddata->rsctbl.reg, &rsc_da);
> +	if (err) {
> +		dev_err(dev, "failed to read rsc tbl addr\n");
> +		return err;
> +	}
> +
> +	if (!rsc_da)
> +		/* no rsc table */
> +		return 0;
> +
> +	err = stm32_rproc_da_to_pa(pdev, ddata, rsc_da, &rsc_pa);
> +	if (err)
> +		return err;
> +
> +	ddata->rsc_va = devm_ioremap_wc(dev, rsc_pa, RSC_TBL_SIZE);
> +	if (IS_ERR_OR_NULL(ddata->rsc_va)) {
> +		dev_err(dev, "Unable to map memory region: %pa+%zx\n",
> +			&rsc_pa, RSC_TBL_SIZE);
> +		ddata->rsc_va = NULL;
> +		return -ENOMEM;
> +	}
> +
> +	return 0;
> +}
> +
>  static int stm32_rproc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -689,6 +749,12 @@ static int stm32_rproc_probe(struct platform_device
> *pdev)
>  		state = M4_STATE_OFF;
>  	}
> 
> +	if (state == M4_STATE_CRUN) {
> +		ret = stm32_rproc_get_loaded_rsc_table(pdev, ddata);
> +		if (ret)
> +			goto free_ddata;
> +	}
> +
>  	rproc = rproc_alloc(dev, np->name, &st_rproc_ops, NULL,
> sizeof(*ddata));
>  	if (!rproc) {
>  		ret = -ENOMEM;
> --
> 2.20.1

RE: [PATCH 08/11] remoteproc: stm32: Introduce new start and stop ops for synchronisation

[Loic PALLARDY]

Hi Mathieu,

> -----Original Message-----
> From: Mathieu Poirier <mathieu.poirier@linaro.org>
> Sent: mardi 24 mars 2020 23:03
> To: bjorn.andersson@linaro.org
> Cc: ohad@wizery.com; Loic PALLARDY <loic.pallardy@st.com>; s-
> anna@ti.com; peng.fan@nxp.com; Arnaud POULIQUEN
> <arnaud.pouliquen@st.com>; Fabien DESSENNE
> <fabien.dessenne@st.com>; linux-remoteproc@vger.kernel.org
> Subject: [PATCH 08/11] remoteproc: stm32: Introduce new start and stop ops
> for synchronisation
> 
> Introduce new start and stop rproc_ops functions to be used when
> synchonising with an MCU.
> 
> Mainly based on the work published by Arnaud Pouliquen [1].
> 
> [1]. https://patchwork.kernel.org/project/linux-
> remoteproc/list/?series=239877
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>  drivers/remoteproc/stm32_rproc.c | 37
> ++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c
> b/drivers/remoteproc/stm32_rproc.c
> index 5bac0baf8f4c..734605a9223e 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -449,6 +449,13 @@ static int stm32_rproc_start(struct rproc *rproc)
>  	return stm32_rproc_set_hold_boot(rproc, true);
>  }
> 
> +static int stm32_rproc_sync_start(struct rproc *rproc)
> +{
> +	stm32_rproc_add_coredump_trace(rproc);
> +
> +	return stm32_rproc_set_hold_boot(rproc, true);
> +}
> +
>  static int stm32_rproc_stop(struct rproc *rproc)
>  {
>  	struct stm32_rproc *ddata = rproc->priv;
> @@ -489,6 +496,30 @@ static int stm32_rproc_stop(struct rproc *rproc)
>  	return 0;
>  }
> 
> +static int stm32_rproc_sync_stop(struct rproc *rproc)
> +{
> +	struct stm32_rproc *ddata = rproc->priv;
> +	int err;
> +
> +	err = stm32_rproc_stop(rproc);
> +	if (err)
> +		return err;
> +
> +	/* update copro state to OFF */
> +	if (ddata->m4_state.map) {
> +		err = regmap_update_bits(ddata->m4_state.map,
> +					 ddata->m4_state.reg,
> +					 ddata->m4_state.mask,
> +					 M4_STATE_OFF);
> +		if (err) {
> +			dev_err(&rproc->dev, "failed to set copro state\n");
> +			return err;
> +		}
> +	}
In fact m4_state is updated in following way:
- it is set by Linux when M4 is guarantee, that means only when Linux is stopping the M4.
in that case M4 is under reset and m4_state could be updated to M4_STATE_OFF
- for all other states, it is M4 responsibility to update m4_state when running
That means the code above is common to both stm32_rproc_stop() and stm32_rproc_sync_stop().
Only one function is required.

Regards,
Loic
> +
> +	return 0;
> +}
> +
>  static void stm32_rproc_kick(struct rproc *rproc, int vqid)
>  {
>  	struct stm32_rproc *ddata = rproc->priv;
> @@ -522,6 +553,12 @@ static struct rproc_ops st_rproc_ops = {
>  	.get_boot_addr	= rproc_elf_get_boot_addr,
>  };
> 
> +static __maybe_unused struct rproc_ops st_rproc_sync_ops = {
> +	.start		= stm32_rproc_sync_start,
> +	.stop		= stm32_rproc_sync_stop,
> +	.kick		= stm32_rproc_kick,
> +};
> +
>  static const struct of_device_id stm32_rproc_match[] = {
>  	{ .compatible = "st,stm32mp1-m4" },
>  	{},
> --
> 2.20.1

RE: [PATCH 09/11] remoteproc: stm32: Introduce new parse fw ops for synchronisation

[Loic PALLARDY]

> -----Original Message-----
> From: Mathieu Poirier <mathieu.poirier@linaro.org>
> Sent: mardi 24 mars 2020 23:03
> To: bjorn.andersson@linaro.org
> Cc: ohad@wizery.com; Loic PALLARDY <loic.pallardy@st.com>; s-
> anna@ti.com; peng.fan@nxp.com; Arnaud POULIQUEN
> <arnaud.pouliquen@st.com>; Fabien DESSENNE
> <fabien.dessenne@st.com>; linux-remoteproc@vger.kernel.org
> Subject: [PATCH 09/11] remoteproc: stm32: Introduce new parse fw ops for
> synchronisation
> 
> Introduce new parse firmware rproc_ops functions to be used when
> synchonising with an MCU.
> 
> Mainly based on the work published by Arnaud Pouliquen [1].
> 
> [1]. https://patchwork.kernel.org/project/linux-
> remoteproc/list/?series=239877
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Thanks Mathieu
 
Reviewed-by: Loic Pallardy <loic.pallardy@st.com>
> ---
>  drivers/remoteproc/stm32_rproc.c | 49
> +++++++++++++++++++++++++++++++-
>  1 file changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c
> b/drivers/remoteproc/stm32_rproc.c
> index 734605a9223e..4268d71f191d 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -215,7 +215,34 @@ static int stm32_rproc_elf_load_rsc_table(struct
> rproc *rproc,
>  	return 0;
>  }
> 
> -static int stm32_rproc_parse_fw(struct rproc *rproc, const struct firmware
> *fw)
> +static int stm32_rproc_sync_elf_load_rsc_table(struct rproc *rproc,
> +					       const struct firmware *fw)
> +{
> +	struct resource_table *table = NULL;
> +	struct stm32_rproc *ddata = rproc->priv;
> +
> +	if (ddata->rsc_va) {
> +		table = (struct resource_table *)ddata->rsc_va;
> +		/* Assuming that the resource table fits in 1kB is fair */
> +		rproc->cached_table = kmemdup(table, RSC_TBL_SIZE,
> GFP_KERNEL);
> +		if (!rproc->cached_table)
> +			return -ENOMEM;
> +
> +		rproc->table_ptr = rproc->cached_table;
> +		rproc->table_sz = RSC_TBL_SIZE;
> +		return 0;
> +	}
> +
> +	rproc->cached_table = NULL;
> +	rproc->table_ptr = NULL;
> +	rproc->table_sz = 0;
> +
> +	dev_warn(&rproc->dev, "no resource table found for this
> firmware\n");
> +	return 0;
> +}
> +
> +static int stm32_rproc_parse_memory_regions(struct rproc *rproc,
> +					    const struct firmware *fw)
>  {
>  	struct device *dev = rproc->dev.parent;
>  	struct device_node *np = dev->of_node;
> @@ -268,9 +295,28 @@ static int stm32_rproc_parse_fw(struct rproc *rproc,
> const struct firmware *fw)
>  		index++;
>  	}
> 
> +	return 0;
> +}
> +
> +static int stm32_rproc_parse_fw(struct rproc *rproc, const struct firmware
> *fw)
> +{
> +	int ret = stm32_rproc_parse_memory_regions(rproc, fw);
> +	if (ret)
> +		return ret;
> +
>  	return stm32_rproc_elf_load_rsc_table(rproc, fw);
>  }
> 
> +static int stm32_rproc_sync_parse_fw(struct rproc *rproc,
> +				     const struct firmware *fw)
> +{
> +	int ret = stm32_rproc_parse_memory_regions(rproc, fw);
> +	if (ret)
> +		return ret;
> +
> +	return stm32_rproc_sync_elf_load_rsc_table(rproc, fw);
> +}
> +
>  static irqreturn_t stm32_rproc_wdg(int irq, void *data)
>  {
>  	struct platform_device *pdev = data;
> @@ -557,6 +603,7 @@ static __maybe_unused struct rproc_ops
> st_rproc_sync_ops = {
>  	.start		= stm32_rproc_sync_start,
>  	.stop		= stm32_rproc_sync_stop,
>  	.kick		= stm32_rproc_kick,
> +	.parse_fw	= stm32_rproc_sync_parse_fw,
>  };
> 
>  static const struct of_device_id stm32_rproc_match[] = {
> --
> 2.20.1

RE: [PATCH 10/11] remoteproc: stm32: Introduce new loaded rsc ops for synchronisation

[Loic PALLARDY]

> -----Original Message-----
> From: Mathieu Poirier <mathieu.poirier@linaro.org>
> Sent: mardi 24 mars 2020 23:03
> To: bjorn.andersson@linaro.org
> Cc: ohad@wizery.com; Loic PALLARDY <loic.pallardy@st.com>; s-
> anna@ti.com; peng.fan@nxp.com; Arnaud POULIQUEN
> <arnaud.pouliquen@st.com>; Fabien DESSENNE
> <fabien.dessenne@st.com>; linux-remoteproc@vger.kernel.org
> Subject: [PATCH 10/11] remoteproc: stm32: Introduce new loaded rsc ops for
> synchronisation
> 
> Introduce new elf find loaded resource table rproc_ops functions to be
> used when synchonising with an MCU.
> 
> Mainly based on the work published by Arnaud Pouliquen [1].
> 
> [1]. https://patchwork.kernel.org/project/linux-
> remoteproc/list/?series=239877
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Thanks Mathieu
 
Reviewed-by: Loic Pallardy <loic.pallardy@st.com>
> ---
>  drivers/remoteproc/stm32_rproc.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c
> b/drivers/remoteproc/stm32_rproc.c
> index 4268d71f191d..07be306c0fb1 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -317,6 +317,15 @@ static int stm32_rproc_sync_parse_fw(struct rproc
> *rproc,
>  	return stm32_rproc_sync_elf_load_rsc_table(rproc, fw);
>  }
> 
> +static struct resource_table *
> +stm32_rproc_sync_elf_find_loaded_rsc_table(struct rproc *rproc,
> +					   const struct firmware *fw)
> +{
> +	struct stm32_rproc *ddata = rproc->priv;
> +
> +	return (struct resource_table *)ddata->rsc_va;
> +}
> +
>  static irqreturn_t stm32_rproc_wdg(int irq, void *data)
>  {
>  	struct platform_device *pdev = data;
> @@ -604,6 +613,7 @@ static __maybe_unused struct rproc_ops
> st_rproc_sync_ops = {
>  	.stop		= stm32_rproc_sync_stop,
>  	.kick		= stm32_rproc_kick,
>  	.parse_fw	= stm32_rproc_sync_parse_fw,
> +	.find_loaded_rsc_table =
> stm32_rproc_sync_elf_find_loaded_rsc_table,
>  };
> 
>  static const struct of_device_id stm32_rproc_match[] = {
> --
> 2.20.1

RE: [PATCH 11/11] remoteproc: stm32: Allocate rproc for synchronisation with MCU

[Loic PALLARDY]

Hi Mathieu,

> -----Original Message-----
> From: linux-remoteproc-owner@vger.kernel.org <linux-remoteproc-
> owner@vger.kernel.org> On Behalf Of Mathieu Poirier
> Sent: mardi 24 mars 2020 23:03
> To: bjorn.andersson@linaro.org
> Cc: ohad@wizery.com; Loic PALLARDY <loic.pallardy@st.com>; s-
> anna@ti.com; peng.fan@nxp.com; Arnaud POULIQUEN
> <arnaud.pouliquen@st.com>; Fabien DESSENNE
> <fabien.dessenne@st.com>; linux-remoteproc@vger.kernel.org
> Subject: [PATCH 11/11] remoteproc: stm32: Allocate rproc for
> synchronisation with MCU
> 
> Allocate remote processor structure with state machine if the MCU
> has already been started by an external entity.
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>  drivers/remoteproc/stm32_rproc.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c
> b/drivers/remoteproc/stm32_rproc.c
> index 07be306c0fb1..f320ef9ee286 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -608,7 +608,7 @@ static struct rproc_ops st_rproc_ops = {
>  	.get_boot_addr	= rproc_elf_get_boot_addr,
>  };
> 
> -static __maybe_unused struct rproc_ops st_rproc_sync_ops = {
> +static struct rproc_ops st_rproc_sync_ops = {
>  	.start		= stm32_rproc_sync_start,
>  	.stop		= stm32_rproc_sync_stop,
>  	.kick		= stm32_rproc_kick,
> @@ -616,6 +616,12 @@ static __maybe_unused struct rproc_ops
> st_rproc_sync_ops = {
>  	.find_loaded_rsc_table =
> stm32_rproc_sync_elf_find_loaded_rsc_table,
>  };
> 
> +static struct rproc_sync_states st_sync_states = {
> +	.on_init = true, /* sync with MCU when the kernel boots */
> +	.after_stop = false, /* don't resync with MCU if stopped from sysfs
> */
> +	.after_crash = false, /* don't resync with MCU after a crash */
> +};
> +
>  static const struct of_device_id stm32_rproc_match[] = {
>  	{ .compatible = "st,stm32mp1-m4" },
>  	{},
> @@ -847,15 +853,22 @@ static int stm32_rproc_probe(struct
> platform_device *pdev)
>  		ret = stm32_rproc_get_loaded_rsc_table(pdev, ddata);
>  		if (ret)
>  			goto free_ddata;
> +
> +		rproc = rproc_alloc_state_machine(dev, np->name,
> &st_rproc_ops,
> +						  &st_rproc_sync_ops,
> +						  &st_sync_states, NULL,
> +						  sizeof(*ddata));
Could we have only one call to rproc_alloc_state_machine(), simply configuring
st_sync_states according to m4_state and other DT properties?

Regards,
Loic
> +	} else {
> +		rproc = rproc_alloc(dev, np->name, &st_rproc_ops,
> +				    NULL, sizeof(*ddata));
> +		rproc->auto_boot = auto_boot;
>  	}
> 
> -	rproc = rproc_alloc(dev, np->name, &st_rproc_ops, NULL,
> sizeof(*ddata));
>  	if (!rproc) {
>  		ret = -ENOMEM;
>  		goto free_ddata;
>  	}
> 
> -	rproc->auto_boot = auto_boot;
>  	rproc->has_iommu = false;
>  	ddata->workqueue = create_workqueue(dev_name(dev));
>  	if (!ddata->workqueue) {
> --
> 2.20.1

Re: [PATCH 11/11] remoteproc: stm32: Allocate rproc for synchronisation with MCU

[Mathieu Poirier]

On Fri, Mar 27, 2020 at 05:11:37PM +0000, Loic PALLARDY wrote:
> Hi Mathieu,
> 
> > -----Original Message-----
> > From: linux-remoteproc-owner@vger.kernel.org <linux-remoteproc-
> > owner@vger.kernel.org> On Behalf Of Mathieu Poirier
> > Sent: mardi 24 mars 2020 23:03
> > To: bjorn.andersson@linaro.org
> > Cc: ohad@wizery.com; Loic PALLARDY <loic.pallardy@st.com>; s-
> > anna@ti.com; peng.fan@nxp.com; Arnaud POULIQUEN
> > <arnaud.pouliquen@st.com>; Fabien DESSENNE
> > <fabien.dessenne@st.com>; linux-remoteproc@vger.kernel.org
> > Subject: [PATCH 11/11] remoteproc: stm32: Allocate rproc for
> > synchronisation with MCU
> > 
> > Allocate remote processor structure with state machine if the MCU
> > has already been started by an external entity.
> > 
> > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> > ---
> >  drivers/remoteproc/stm32_rproc.c | 19 ++++++++++++++++---
> >  1 file changed, 16 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/remoteproc/stm32_rproc.c
> > b/drivers/remoteproc/stm32_rproc.c
> > index 07be306c0fb1..f320ef9ee286 100644
> > --- a/drivers/remoteproc/stm32_rproc.c
> > +++ b/drivers/remoteproc/stm32_rproc.c
> > @@ -608,7 +608,7 @@ static struct rproc_ops st_rproc_ops = {
> >  	.get_boot_addr	= rproc_elf_get_boot_addr,
> >  };
> > 
> > -static __maybe_unused struct rproc_ops st_rproc_sync_ops = {
> > +static struct rproc_ops st_rproc_sync_ops = {
> >  	.start		= stm32_rproc_sync_start,
> >  	.stop		= stm32_rproc_sync_stop,
> >  	.kick		= stm32_rproc_kick,
> > @@ -616,6 +616,12 @@ static __maybe_unused struct rproc_ops
> > st_rproc_sync_ops = {
> >  	.find_loaded_rsc_table =
> > stm32_rproc_sync_elf_find_loaded_rsc_table,
> >  };
> > 
> > +static struct rproc_sync_states st_sync_states = {
> > +	.on_init = true, /* sync with MCU when the kernel boots */
> > +	.after_stop = false, /* don't resync with MCU if stopped from sysfs
> > */
> > +	.after_crash = false, /* don't resync with MCU after a crash */
> > +};
> > +
> >  static const struct of_device_id stm32_rproc_match[] = {
> >  	{ .compatible = "st,stm32mp1-m4" },
> >  	{},
> > @@ -847,15 +853,22 @@ static int stm32_rproc_probe(struct
> > platform_device *pdev)
> >  		ret = stm32_rproc_get_loaded_rsc_table(pdev, ddata);
> >  		if (ret)
> >  			goto free_ddata;
> > +
> > +		rproc = rproc_alloc_state_machine(dev, np->name,
> > &st_rproc_ops,
> > +						  &st_rproc_sync_ops,
> > +						  &st_sync_states, NULL,
> > +						  sizeof(*ddata));
> Could we have only one call to rproc_alloc_state_machine(), simply configuring
> st_sync_states according to m4_state and other DT properties?

Yes, very much so - I'll do that for V2.

Mathieu

> 
> Regards,
> Loic
> > +	} else {
> > +		rproc = rproc_alloc(dev, np->name, &st_rproc_ops,
> > +				    NULL, sizeof(*ddata));
> > +		rproc->auto_boot = auto_boot;
> >  	}
> > 
> > -	rproc = rproc_alloc(dev, np->name, &st_rproc_ops, NULL,
> > sizeof(*ddata));
> >  	if (!rproc) {
> >  		ret = -ENOMEM;
> >  		goto free_ddata;
> >  	}
> > 
> > -	rproc->auto_boot = auto_boot;
> >  	rproc->has_iommu = false;
> >  	ddata->workqueue = create_workqueue(dev_name(dev));
> >  	if (!ddata->workqueue) {
> > --
> > 2.20.1
> 

Re: [PATCH 06/11] remoteproc: stm32: Get coprocessor state

[Mathieu Poirier]

On Fri, Mar 27, 2020 at 04:58:00PM +0000, Loic PALLARDY wrote:
> Hi Mathieu,
> 
> > -----Original Message-----
> > From: linux-remoteproc-owner@vger.kernel.org <linux-remoteproc-
> > owner@vger.kernel.org> On Behalf Of Mathieu Poirier
> > Sent: mardi 24 mars 2020 23:03
> > To: bjorn.andersson@linaro.org
> > Cc: ohad@wizery.com; Loic PALLARDY <loic.pallardy@st.com>; s-
> > anna@ti.com; peng.fan@nxp.com; Arnaud POULIQUEN
> > <arnaud.pouliquen@st.com>; Fabien DESSENNE
> > <fabien.dessenne@st.com>; linux-remoteproc@vger.kernel.org
> > Subject: [PATCH 06/11] remoteproc: stm32: Get coprocessor state
> > 
> > Introduce the required mechanic to get the state of the M4 when the
> > remoteproc core is initialising.
> > 
> > Mainly based on the work published by Arnaud Pouliquen [1].
> > 
> > [1]. https://patchwork.kernel.org/project/linux-
> > remoteproc/list/?series=239877
> > 
> > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> > ---
> >  drivers/remoteproc/stm32_rproc.c | 29
> > +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> > 
> > diff --git a/drivers/remoteproc/stm32_rproc.c
> > b/drivers/remoteproc/stm32_rproc.c
> > index 7d5d4a7dbb04..b8af15dd0510 100644
> > --- a/drivers/remoteproc/stm32_rproc.c
> > +++ b/drivers/remoteproc/stm32_rproc.c
> > @@ -38,6 +38,15 @@
> >  #define STM32_MBX_VQ1_ID	1
> >  #define STM32_MBX_SHUTDOWN	"shutdown"
> > 
> > +#define RSC_TBL_SIZE		(1024)
> > +
> > +#define M4_STATE_OFF		0
> > +#define M4_STATE_INI		1
> Typo M4_STATE_INI -> M4_STATE_INIT

Good catch

> 
> Else ok for me
> Reviewed-by: Loic Pallardy <loic.pallardy@st.com>
> 
> Regards,
> Loic
> 
> > +#define M4_STATE_CRUN		2
> > +#define M4_STATE_CSTOP		3
> > +#define M4_STATE_STANDBY	4
> > +#define M4_STATE_CRASH		5
> > +
> >  struct stm32_syscon {
> >  	struct regmap *map;
> >  	u32 reg;
> > @@ -635,12 +644,23 @@ static int stm32_rproc_parse_dt(struct
> > platform_device *pdev,
> >  	return 0;
> >  }
> > 
> > +static int stm32_rproc_get_m4_status(struct stm32_rproc *ddata,
> > +				     unsigned int *state)
> > +{
> > +	/* See stm32_rproc_parse_dt() */
> > +	if (!ddata->m4_state.map)
> > +		return -EINVAL;
> > +
> > +	return regmap_read(ddata->m4_state.map, ddata->m4_state.reg,
> > state);
> > +}
> > +
> >  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;
> > +	unsigned int state;
> >  	bool auto_boot = false;
> >  	int ret;
> > 
> > @@ -660,6 +680,15 @@ static int stm32_rproc_probe(struct platform_device
> > *pdev)
> >  	if (ret)
> >  		goto free_ddata;
> > 
> > +	ret = stm32_rproc_get_m4_status(ddata, &state);
> > +	if (ret) {
> > +		/*
> > +		 * We couldn't get the coprocessor's state, assume
> > +		 * it is not running.
> > +		 */
> > +		state = M4_STATE_OFF;
> > +	}
> > +
> >  	rproc = rproc_alloc(dev, np->name, &st_rproc_ops, NULL,
> > sizeof(*ddata));
> >  	if (!rproc) {
> >  		ret = -ENOMEM;
> > --
> > 2.20.1
> 

Re: [PATCH 08/11] remoteproc: stm32: Introduce new start and stop ops for synchronisation

[Mathieu Poirier]

On Fri, Mar 27, 2020 at 05:04:34PM +0000, Loic PALLARDY wrote:
> Hi Mathieu,
> 
> > -----Original Message-----
> > From: Mathieu Poirier <mathieu.poirier@linaro.org>
> > Sent: mardi 24 mars 2020 23:03
> > To: bjorn.andersson@linaro.org
> > Cc: ohad@wizery.com; Loic PALLARDY <loic.pallardy@st.com>; s-
> > anna@ti.com; peng.fan@nxp.com; Arnaud POULIQUEN
> > <arnaud.pouliquen@st.com>; Fabien DESSENNE
> > <fabien.dessenne@st.com>; linux-remoteproc@vger.kernel.org
> > Subject: [PATCH 08/11] remoteproc: stm32: Introduce new start and stop ops
> > for synchronisation
> > 
> > Introduce new start and stop rproc_ops functions to be used when
> > synchonising with an MCU.
> > 
> > Mainly based on the work published by Arnaud Pouliquen [1].
> > 
> > [1]. https://patchwork.kernel.org/project/linux-
> > remoteproc/list/?series=239877
> > 
> > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> > ---
> >  drivers/remoteproc/stm32_rproc.c | 37
> > ++++++++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> > 
> > diff --git a/drivers/remoteproc/stm32_rproc.c
> > b/drivers/remoteproc/stm32_rproc.c
> > index 5bac0baf8f4c..734605a9223e 100644
> > --- a/drivers/remoteproc/stm32_rproc.c
> > +++ b/drivers/remoteproc/stm32_rproc.c
> > @@ -449,6 +449,13 @@ static int stm32_rproc_start(struct rproc *rproc)
> >  	return stm32_rproc_set_hold_boot(rproc, true);
> >  }
> > 
> > +static int stm32_rproc_sync_start(struct rproc *rproc)
> > +{
> > +	stm32_rproc_add_coredump_trace(rproc);
> > +
> > +	return stm32_rproc_set_hold_boot(rproc, true);
> > +}
> > +
> >  static int stm32_rproc_stop(struct rproc *rproc)
> >  {
> >  	struct stm32_rproc *ddata = rproc->priv;
> > @@ -489,6 +496,30 @@ static int stm32_rproc_stop(struct rproc *rproc)
> >  	return 0;
> >  }
> > 
> > +static int stm32_rproc_sync_stop(struct rproc *rproc)
> > +{
> > +	struct stm32_rproc *ddata = rproc->priv;
> > +	int err;
> > +
> > +	err = stm32_rproc_stop(rproc);
> > +	if (err)
> > +		return err;
> > +
> > +	/* update copro state to OFF */
> > +	if (ddata->m4_state.map) {
> > +		err = regmap_update_bits(ddata->m4_state.map,
> > +					 ddata->m4_state.reg,
> > +					 ddata->m4_state.mask,
> > +					 M4_STATE_OFF);
> > +		if (err) {
> > +			dev_err(&rproc->dev, "failed to set copro state\n");
> > +			return err;
> > +		}
> > +	}
> In fact m4_state is updated in following way:
> - it is set by Linux when M4 is guarantee, that means only when Linux is stopping the M4.
> in that case M4 is under reset and m4_state could be updated to M4_STATE_OFF
> - for all other states, it is M4 responsibility to update m4_state when running
> That means the code above is common to both stm32_rproc_stop() and stm32_rproc_sync_stop().
> Only one function is required.

Very well, I'll get it fixed.

> 
> Regards,
> Loic
> > +
> > +	return 0;
> > +}
> > +
> >  static void stm32_rproc_kick(struct rproc *rproc, int vqid)
> >  {
> >  	struct stm32_rproc *ddata = rproc->priv;
> > @@ -522,6 +553,12 @@ static struct rproc_ops st_rproc_ops = {
> >  	.get_boot_addr	= rproc_elf_get_boot_addr,
> >  };
> > 
> > +static __maybe_unused struct rproc_ops st_rproc_sync_ops = {
> > +	.start		= stm32_rproc_sync_start,
> > +	.stop		= stm32_rproc_sync_stop,
> > +	.kick		= stm32_rproc_kick,
> > +};
> > +
> >  static const struct of_device_id stm32_rproc_match[] = {
> >  	{ .compatible = "st,stm32mp1-m4" },
> >  	{},
> > --
> > 2.20.1
>