linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4
@ 2020-04-24 20:24 Mathieu Poirier
  2020-04-24 20:24 ` [PATCH v2 01/12] remoteproc: stm32: Decouple rproc from memory translation Mathieu Poirier
                   ` (12 more replies)
  0 siblings, 13 replies; 30+ messages in thread
From: Mathieu Poirier @ 2020-04-24 20:24 UTC (permalink / raw)
  To: bjorn.andersson, ohad, mcoquelin.stm32, alexandre.torgue
  Cc: loic.pallardy, arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

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 the M4 remote processor that would have been started by
the boot loader or another entity.

It carries the same functionatlity as the previeous revision but account
for changes in the remoteproc core to support synchronisation scenarios.
Some RB tags have been removed when the content of the patch has strayed 
too far from the original version. See patch 3, 8, 9 and 12 for more
details.

Tested on ST's mp157c board.

Thanks,
Mathieu

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

Mathieu Poirier (12):
  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 ops for synchronisation
  remoteproc: stm32: Update M4 state in stm32_rproc_stop()
  remoteproc: stm32: Introduce new parse fw ops for synchronisation
  remoteproc: stm32: Introduce new loaded rsc ops for synchronisation
  remoteproc: stm32: Set synchronisation state machine if needed

 drivers/remoteproc/stm32_rproc.c | 262 ++++++++++++++++++++++++++++---
 1 file changed, 244 insertions(+), 18 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v2 01/12] remoteproc: stm32: Decouple rproc from memory translation
  2020-04-24 20:24 [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4 Mathieu Poirier
@ 2020-04-24 20:24 ` Mathieu Poirier
  2020-05-14  4:57   ` Bjorn Andersson
  2020-04-24 20:24 ` [PATCH v2 02/12] remoteproc: stm32: Request IRQ with platform device Mathieu Poirier
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 30+ messages in thread
From: Mathieu Poirier @ 2020-04-24 20:24 UTC (permalink / raw)
  To: bjorn.andersson, ohad, mcoquelin.stm32, alexandre.torgue
  Cc: loic.pallardy, arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

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>
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 0f9d02ca4f5a..91fd59af0ffe 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


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 02/12] remoteproc: stm32: Request IRQ with platform device
  2020-04-24 20:24 [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4 Mathieu Poirier
  2020-04-24 20:24 ` [PATCH v2 01/12] remoteproc: stm32: Decouple rproc from memory translation Mathieu Poirier
@ 2020-04-24 20:24 ` Mathieu Poirier
  2020-05-14  4:57   ` Bjorn Andersson
  2020-04-24 20:24 ` [PATCH v2 03/12] remoteproc: stm32: Decouple rproc from DT parsing Mathieu Poirier
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 30+ messages in thread
From: Mathieu Poirier @ 2020-04-24 20:24 UTC (permalink / raw)
  To: bjorn.andersson, ohad, mcoquelin.stm32, alexandre.torgue
  Cc: loic.pallardy, arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

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>
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 91fd59af0ffe..1ac90adba9b1 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


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 03/12] remoteproc: stm32: Decouple rproc from DT parsing
  2020-04-24 20:24 [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4 Mathieu Poirier
  2020-04-24 20:24 ` [PATCH v2 01/12] remoteproc: stm32: Decouple rproc from memory translation Mathieu Poirier
  2020-04-24 20:24 ` [PATCH v2 02/12] remoteproc: stm32: Request IRQ with platform device Mathieu Poirier
@ 2020-04-24 20:24 ` Mathieu Poirier
  2020-04-29 13:37   ` Arnaud POULIQUEN
  2020-05-14  4:59   ` Bjorn Andersson
  2020-04-24 20:24 ` [PATCH v2 04/12] remoteproc: stm32: Remove memory translation " Mathieu Poirier
                   ` (9 subsequent siblings)
  12 siblings, 2 replies; 30+ messages in thread
From: Mathieu Poirier @ 2020-04-24 20:24 UTC (permalink / raw)
  To: bjorn.andersson, ohad, mcoquelin.stm32, alexandre.torgue
  Cc: loic.pallardy, arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

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 M4 or not.

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

diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index 1ac90adba9b1..57a426ea620b 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_info(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,6 +615,7 @@ 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));
@@ -626,9 +626,16 @@ static int stm32_rproc_probe(struct platform_device *pdev)
 	if (!rproc)
 		return -ENOMEM;
 
+	ddata = rproc->priv;
+
 	rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
+
+	ret = stm32_rproc_parse_dt(pdev, ddata, &auto_boot);
+	if (ret)
+		goto free_rproc;
+
+	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");
@@ -638,13 +645,9 @@ static int stm32_rproc_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, rproc);
 
-	ret = stm32_rproc_parse_dt(pdev);
-	if (ret)
-		goto free_wkq;
-
 	ret = stm32_rproc_request_mbox(rproc);
 	if (ret)
-		goto free_rproc;
+		goto free_wkq;
 
 	ret = rproc_add(rproc);
 	if (ret)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 04/12] remoteproc: stm32: Remove memory translation from DT parsing
  2020-04-24 20:24 [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4 Mathieu Poirier
                   ` (2 preceding siblings ...)
  2020-04-24 20:24 ` [PATCH v2 03/12] remoteproc: stm32: Decouple rproc from DT parsing Mathieu Poirier
@ 2020-04-24 20:24 ` Mathieu Poirier
  2020-05-14  5:03   ` Bjorn Andersson
  2020-04-24 20:24 ` [PATCH v2 05/12] remoteproc: stm32: Parse syscon that will manage M4 synchronisation Mathieu Poirier
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 30+ messages in thread
From: Mathieu Poirier @ 2020-04-24 20:24 UTC (permalink / raw)
  To: bjorn.andersson, ohad, mcoquelin.stm32, alexandre.torgue
  Cc: loic.pallardy, arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

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>
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 57a426ea620b..658439d4b00a 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)
@@ -634,6 +634,10 @@ static int stm32_rproc_probe(struct platform_device *pdev)
 	if (ret)
 		goto free_rproc;
 
+	ret = stm32_rproc_of_memory_translations(pdev, ddata);
+	if (ret)
+		goto free_rproc;
+
 	rproc->auto_boot = auto_boot;
 	rproc->has_iommu = false;
 	ddata->workqueue = create_workqueue(dev_name(dev));
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 05/12] remoteproc: stm32: Parse syscon that will manage M4 synchronisation
  2020-04-24 20:24 [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4 Mathieu Poirier
                   ` (3 preceding siblings ...)
  2020-04-24 20:24 ` [PATCH v2 04/12] remoteproc: stm32: Remove memory translation " Mathieu Poirier
@ 2020-04-24 20:24 ` Mathieu Poirier
  2020-05-14  5:03   ` Bjorn Andersson
  2020-04-24 20:24 ` [PATCH v2 06/12] remoteproc: stm32: Get coprocessor state Mathieu Poirier
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 30+ messages in thread
From: Mathieu Poirier @ 2020-04-24 20:24 UTC (permalink / raw)
  To: bjorn.andersson, ohad, mcoquelin.stm32, alexandre.torgue
  Cc: loic.pallardy, arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

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>
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 658439d4b00a..a285f338bed8 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


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 06/12] remoteproc: stm32: Get coprocessor state
  2020-04-24 20:24 [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4 Mathieu Poirier
                   ` (4 preceding siblings ...)
  2020-04-24 20:24 ` [PATCH v2 05/12] remoteproc: stm32: Parse syscon that will manage M4 synchronisation Mathieu Poirier
@ 2020-04-24 20:24 ` Mathieu Poirier
  2020-04-29 13:38   ` Arnaud POULIQUEN
  2020-04-24 20:25 ` [PATCH v2 07/12] remoteproc: stm32: Get loaded resource table for synchronisation Mathieu Poirier
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 30+ messages in thread
From: Mathieu Poirier @ 2020-04-24 20:24 UTC (permalink / raw)
  To: bjorn.andersson, ohad, mcoquelin.stm32, alexandre.torgue
  Cc: loic.pallardy, arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

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>
Reviewed-by: Loic Pallardy <loic.pallardy@st.com>
---
 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 a285f338bed8..89fbd2ffac93 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;
 
@@ -664,6 +684,15 @@ static int stm32_rproc_probe(struct platform_device *pdev)
 	if (ret)
 		goto free_rproc;
 
+	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->auto_boot = auto_boot;
 	rproc->has_iommu = false;
 	ddata->workqueue = create_workqueue(dev_name(dev));
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 07/12] remoteproc: stm32: Get loaded resource table for synchronisation
  2020-04-24 20:24 [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4 Mathieu Poirier
                   ` (5 preceding siblings ...)
  2020-04-24 20:24 ` [PATCH v2 06/12] remoteproc: stm32: Get coprocessor state Mathieu Poirier
@ 2020-04-24 20:25 ` Mathieu Poirier
  2020-04-24 20:25 ` [PATCH v2 08/12] remoteproc: stm32: Introduce new start ops " Mathieu Poirier
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 30+ messages in thread
From: Mathieu Poirier @ 2020-04-24 20:25 UTC (permalink / raw)
  To: bjorn.andersson, ohad, mcoquelin.stm32, alexandre.torgue
  Cc: loic.pallardy, arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

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>
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 89fbd2ffac93..8ba69e903851 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;
@@ -693,6 +753,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_rproc;
+	}
+
 	rproc->auto_boot = auto_boot;
 	rproc->has_iommu = false;
 	ddata->workqueue = create_workqueue(dev_name(dev));
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 08/12] remoteproc: stm32: Introduce new start ops for synchronisation
  2020-04-24 20:24 [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4 Mathieu Poirier
                   ` (6 preceding siblings ...)
  2020-04-24 20:25 ` [PATCH v2 07/12] remoteproc: stm32: Get loaded resource table for synchronisation Mathieu Poirier
@ 2020-04-24 20:25 ` Mathieu Poirier
  2020-04-29 14:50   ` Arnaud POULIQUEN
  2020-04-24 20:25 ` [PATCH v2 09/12] remoteproc: stm32: Update M4 state in stm32_rproc_stop() Mathieu Poirier
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 30+ messages in thread
From: Mathieu Poirier @ 2020-04-24 20:25 UTC (permalink / raw)
  To: bjorn.andersson, ohad, mcoquelin.stm32, alexandre.torgue
  Cc: loic.pallardy, arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

Introduce new start 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 | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index 8ba69e903851..404f17a97095 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;
@@ -522,6 +529,10 @@ 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,
+};
+
 static const struct of_device_id stm32_rproc_match[] = {
 	{ .compatible = "st,stm32mp1-m4" },
 	{},
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 09/12] remoteproc: stm32: Update M4 state in stm32_rproc_stop()
  2020-04-24 20:24 [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4 Mathieu Poirier
                   ` (7 preceding siblings ...)
  2020-04-24 20:25 ` [PATCH v2 08/12] remoteproc: stm32: Introduce new start ops " Mathieu Poirier
@ 2020-04-24 20:25 ` Mathieu Poirier
  2020-04-29 14:52   ` Arnaud POULIQUEN
  2020-04-24 20:25 ` [PATCH v2 10/12] remoteproc: stm32: Introduce new parse fw ops for synchronisation Mathieu Poirier
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 30+ messages in thread
From: Mathieu Poirier @ 2020-04-24 20:25 UTC (permalink / raw)
  To: bjorn.andersson, ohad, mcoquelin.stm32, alexandre.torgue
  Cc: loic.pallardy, arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

Update the M4 co-processor state in function stm32_rproc_stop() so
that it can be used in synchronisation scenarios.

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 | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index 404f17a97095..86d23c35d805 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -493,6 +493,18 @@ static int stm32_rproc_stop(struct rproc *rproc)
 		}
 	}
 
+	/* update coprocessor state to OFF if available */
+	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;
 }
 
@@ -531,6 +543,7 @@ static struct rproc_ops st_rproc_ops = {
 
 static __maybe_unused struct rproc_ops st_rproc_sync_ops = {
 	.start		= stm32_rproc_sync_start,
+	.stop		= stm32_rproc_stop,
 };
 
 static const struct of_device_id stm32_rproc_match[] = {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 10/12] remoteproc: stm32: Introduce new parse fw ops for synchronisation
  2020-04-24 20:24 [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4 Mathieu Poirier
                   ` (8 preceding siblings ...)
  2020-04-24 20:25 ` [PATCH v2 09/12] remoteproc: stm32: Update M4 state in stm32_rproc_stop() Mathieu Poirier
@ 2020-04-24 20:25 ` Mathieu Poirier
  2020-05-14  5:13   ` Bjorn Andersson
  2020-04-24 20:25 ` [PATCH v2 11/12] remoteproc: stm32: Introduce new loaded rsc " Mathieu Poirier
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 30+ messages in thread
From: Mathieu Poirier @ 2020-04-24 20:25 UTC (permalink / raw)
  To: bjorn.andersson, ohad, mcoquelin.stm32, alexandre.torgue
  Cc: loic.pallardy, arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

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>
Reviewed-by: Loic Pallardy <loic.pallardy@st.com>
---
 drivers/remoteproc/stm32_rproc.c | 51 +++++++++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index 86d23c35d805..b8ae8aed5585 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,30 @@ 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;
@@ -544,6 +592,7 @@ static struct rproc_ops st_rproc_ops = {
 static __maybe_unused struct rproc_ops st_rproc_sync_ops = {
 	.start		= stm32_rproc_sync_start,
 	.stop		= stm32_rproc_stop,
+	.parse_fw       = stm32_rproc_sync_parse_fw,
 };
 
 static const struct of_device_id stm32_rproc_match[] = {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 11/12] remoteproc: stm32: Introduce new loaded rsc ops for synchronisation
  2020-04-24 20:24 [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4 Mathieu Poirier
                   ` (9 preceding siblings ...)
  2020-04-24 20:25 ` [PATCH v2 10/12] remoteproc: stm32: Introduce new parse fw ops for synchronisation Mathieu Poirier
@ 2020-04-24 20:25 ` Mathieu Poirier
  2020-05-14  5:15   ` Bjorn Andersson
  2020-04-24 20:25 ` [PATCH v2 12/12] remoteproc: stm32: Set synchronisation state machine if needed Mathieu Poirier
  2020-04-29 15:08 ` [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4 Arnaud POULIQUEN
  12 siblings, 1 reply; 30+ messages in thread
From: Mathieu Poirier @ 2020-04-24 20:25 UTC (permalink / raw)
  To: bjorn.andersson, ohad, mcoquelin.stm32, alexandre.torgue
  Cc: loic.pallardy, arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

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>
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 b8ae8aed5585..dcae6103e3df 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -319,6 +319,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;
@@ -593,6 +602,7 @@ static __maybe_unused struct rproc_ops st_rproc_sync_ops = {
 	.start		= stm32_rproc_sync_start,
 	.stop		= stm32_rproc_stop,
 	.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


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v2 12/12] remoteproc: stm32: Set synchronisation state machine if needed
  2020-04-24 20:24 [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4 Mathieu Poirier
                   ` (10 preceding siblings ...)
  2020-04-24 20:25 ` [PATCH v2 11/12] remoteproc: stm32: Introduce new loaded rsc " Mathieu Poirier
@ 2020-04-24 20:25 ` Mathieu Poirier
  2020-04-29 14:47   ` Arnaud POULIQUEN
  2020-04-29 15:08 ` [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4 Arnaud POULIQUEN
  12 siblings, 1 reply; 30+ messages in thread
From: Mathieu Poirier @ 2020-04-24 20:25 UTC (permalink / raw)
  To: bjorn.andersson, ohad, mcoquelin.stm32, alexandre.torgue
  Cc: loic.pallardy, arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

Set the flags and operations to use if the M4 has been started
by another entity than the remoteproc core.

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

diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index dcae6103e3df..02dad3f51c7a 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -598,13 +598,20 @@ 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_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 struct rproc_sync_flags st_sync_flags = {
+	.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" },
 	{},
@@ -803,6 +810,7 @@ static int stm32_rproc_probe(struct platform_device *pdev)
 	struct stm32_rproc *ddata;
 	struct device_node *np = dev->of_node;
 	struct rproc *rproc;
+	struct rproc_sync_flags sync_flags = {0};
 	unsigned int state;
 	bool auto_boot = false;
 	int ret;
@@ -837,11 +845,17 @@ static int stm32_rproc_probe(struct platform_device *pdev)
 	}
 
 	if (state == M4_STATE_CRUN) {
+		auto_boot = true;
+		sync_flags = st_sync_flags;
 		ret = stm32_rproc_get_loaded_rsc_table(pdev, ddata);
 		if (ret)
 			goto free_rproc;
 	}
 
+	ret = rproc_set_state_machine(rproc, &st_rproc_sync_ops, sync_flags);
+	if (ret)
+		goto free_rproc;
+
 	rproc->auto_boot = auto_boot;
 	rproc->has_iommu = false;
 	ddata->workqueue = create_workqueue(dev_name(dev));
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 03/12] remoteproc: stm32: Decouple rproc from DT parsing
  2020-04-24 20:24 ` [PATCH v2 03/12] remoteproc: stm32: Decouple rproc from DT parsing Mathieu Poirier
@ 2020-04-29 13:37   ` Arnaud POULIQUEN
  2020-04-30 20:58     ` Mathieu Poirier
  2020-05-14  4:59   ` Bjorn Andersson
  1 sibling, 1 reply; 30+ messages in thread
From: Arnaud POULIQUEN @ 2020-04-29 13:37 UTC (permalink / raw)
  To: Mathieu Poirier, bjorn.andersson, ohad, mcoquelin.stm32,
	alexandre.torgue
  Cc: loic.pallardy, linux-remoteproc, linux-stm32, linux-arm-kernel,
	linux-kernel

Hi Mathieu,

On 4/24/20 10:24 PM, Mathieu Poirier wrote:
> 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 M4 or not.
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>  drivers/remoteproc/stm32_rproc.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
> index 1ac90adba9b1..57a426ea620b 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_info(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,6 +615,7 @@ 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;

Nitpicking: Seems that you don't need to initialize it. 
Perhaps you can simply suppress the local variable and directly use rproc->auto_boot.

else LGTM


Thanks,
Arnaud

>  	int ret;
>  
>  	ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
> @@ -626,9 +626,16 @@ static int stm32_rproc_probe(struct platform_device *pdev)
>  	if (!rproc)
>  		return -ENOMEM;
>  
> +	ddata = rproc->priv;
> +
>  	rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
> +
> +	ret = stm32_rproc_parse_dt(pdev, ddata, &auto_boot);
> +	if (ret)
> +		goto free_rproc;
> +
> +	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");
> @@ -638,13 +645,9 @@ static int stm32_rproc_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, rproc);
>  
> -	ret = stm32_rproc_parse_dt(pdev);
> -	if (ret)
> -		goto free_wkq;
> -
>  	ret = stm32_rproc_request_mbox(rproc);
>  	if (ret)
> -		goto free_rproc;
> +		goto free_wkq;
>  
>  	ret = rproc_add(rproc);
>  	if (ret)
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 06/12] remoteproc: stm32: Get coprocessor state
  2020-04-24 20:24 ` [PATCH v2 06/12] remoteproc: stm32: Get coprocessor state Mathieu Poirier
@ 2020-04-29 13:38   ` Arnaud POULIQUEN
  2020-05-01 17:40     ` Mathieu Poirier
  0 siblings, 1 reply; 30+ messages in thread
From: Arnaud POULIQUEN @ 2020-04-29 13:38 UTC (permalink / raw)
  To: Mathieu Poirier, bjorn.andersson, ohad, mcoquelin.stm32,
	alexandre.torgue
  Cc: loic.pallardy, linux-remoteproc, linux-stm32, linux-arm-kernel,
	linux-kernel



On 4/24/20 10:24 PM, Mathieu Poirier wrote:
> 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>
> Reviewed-by: Loic Pallardy <loic.pallardy@st.com>
> ---
>  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 a285f338bed8..89fbd2ffac93 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);
> +}
i would manage here the default state depending on the error types
if (!ddata->m4_state.map {
	/* 
	 * We couldn't get the coprocessor's state, assume
	 * it is not running.
	 */
	state = M4_STATE_OFF;

	return 0;
}

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;
>  
> @@ -664,6 +684,15 @@ static int stm32_rproc_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto free_rproc;
>  
> +	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;

So here just handle the error;

Regards
Arnaud
> +	}
> +
>  	rproc->auto_boot = auto_boot;
>  	rproc->has_iommu = false;
>  	ddata->workqueue = create_workqueue(dev_name(dev));
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 12/12] remoteproc: stm32: Set synchronisation state machine if needed
  2020-04-24 20:25 ` [PATCH v2 12/12] remoteproc: stm32: Set synchronisation state machine if needed Mathieu Poirier
@ 2020-04-29 14:47   ` Arnaud POULIQUEN
  2020-05-01 17:54     ` Mathieu Poirier
  0 siblings, 1 reply; 30+ messages in thread
From: Arnaud POULIQUEN @ 2020-04-29 14:47 UTC (permalink / raw)
  To: Mathieu Poirier, bjorn.andersson, ohad, mcoquelin.stm32,
	alexandre.torgue
  Cc: loic.pallardy, linux-remoteproc, linux-stm32, linux-arm-kernel,
	linux-kernel



On 4/24/20 10:25 PM, Mathieu Poirier wrote:
> Set the flags and operations to use if the M4 has been started
> by another entity than the remoteproc core.
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>  drivers/remoteproc/stm32_rproc.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
> index dcae6103e3df..02dad3f51c7a 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -598,13 +598,20 @@ 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_stop,
> +	.kick		= stm32_rproc_kick,

Seems independent of the path.

>  	.parse_fw       = stm32_rproc_sync_parse_fw,
>  	.find_loaded_rsc_table = stm32_rproc_sync_elf_find_loaded_rsc_table,
>  };
>  
> +static struct rproc_sync_flags st_sync_flags = {
> +	.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 */
> +};
> +
could be const

>  static const struct of_device_id stm32_rproc_match[] = {
>  	{ .compatible = "st,stm32mp1-m4" },
>  	{},
> @@ -803,6 +810,7 @@ static int stm32_rproc_probe(struct platform_device *pdev)
>  	struct stm32_rproc *ddata;
>  	struct device_node *np = dev->of_node;
>  	struct rproc *rproc;
> +	struct rproc_sync_flags sync_flags = {0};
>  	unsigned int state;
>  	bool auto_boot = false;
>  	int ret;
> @@ -837,11 +845,17 @@ static int stm32_rproc_probe(struct platform_device *pdev)
>  	}
>  
>  	if (state == M4_STATE_CRUN) {
> +		auto_boot = true;
> +		sync_flags = st_sync_flags;

seems an useless copy 

Regards,
Arnaud

>  		ret = stm32_rproc_get_loaded_rsc_table(pdev, ddata);
>  		if (ret)
>  			goto free_rproc;
>  	}
>  
> +	ret = rproc_set_state_machine(rproc, &st_rproc_sync_ops, sync_flags);
> +	if (ret)
> +		goto free_rproc;
> +
>  	rproc->auto_boot = auto_boot;
>  	rproc->has_iommu = false;
>  	ddata->workqueue = create_workqueue(dev_name(dev));
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 08/12] remoteproc: stm32: Introduce new start ops for synchronisation
  2020-04-24 20:25 ` [PATCH v2 08/12] remoteproc: stm32: Introduce new start ops " Mathieu Poirier
@ 2020-04-29 14:50   ` Arnaud POULIQUEN
  0 siblings, 0 replies; 30+ messages in thread
From: Arnaud POULIQUEN @ 2020-04-29 14:50 UTC (permalink / raw)
  To: Mathieu Poirier, bjorn.andersson, ohad, mcoquelin.stm32,
	alexandre.torgue
  Cc: loic.pallardy, linux-remoteproc, linux-stm32, linux-arm-kernel,
	linux-kernel



On 4/24/20 10:25 PM, Mathieu Poirier wrote:
> Introduce new start 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>

Reviewed-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>

Thanks,
Arnaud

> ---
>  drivers/remoteproc/stm32_rproc.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
> index 8ba69e903851..404f17a97095 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;
> @@ -522,6 +529,10 @@ 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,
> +};
> +
>  static const struct of_device_id stm32_rproc_match[] = {
>  	{ .compatible = "st,stm32mp1-m4" },
>  	{},
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 09/12] remoteproc: stm32: Update M4 state in stm32_rproc_stop()
  2020-04-24 20:25 ` [PATCH v2 09/12] remoteproc: stm32: Update M4 state in stm32_rproc_stop() Mathieu Poirier
@ 2020-04-29 14:52   ` Arnaud POULIQUEN
  0 siblings, 0 replies; 30+ messages in thread
From: Arnaud POULIQUEN @ 2020-04-29 14:52 UTC (permalink / raw)
  To: Mathieu Poirier, bjorn.andersson, ohad, mcoquelin.stm32,
	alexandre.torgue
  Cc: loic.pallardy, linux-remoteproc, linux-stm32, linux-arm-kernel,
	linux-kernel



On 4/24/20 10:25 PM, Mathieu Poirier wrote:
> Update the M4 co-processor state in function stm32_rproc_stop() so
> that it can be used in synchronisation scenarios.
> 
> 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>

Reviewed-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>

Thanks,
Arnaud

> ---
>  drivers/remoteproc/stm32_rproc.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
> index 404f17a97095..86d23c35d805 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -493,6 +493,18 @@ static int stm32_rproc_stop(struct rproc *rproc)
>  		}
>  	}
>  
> +	/* update coprocessor state to OFF if available */
> +	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;
>  }
>  
> @@ -531,6 +543,7 @@ static struct rproc_ops st_rproc_ops = {
>  
>  static __maybe_unused struct rproc_ops st_rproc_sync_ops = {
>  	.start		= stm32_rproc_sync_start,
> +	.stop		= stm32_rproc_stop,
>  };
>  
>  static const struct of_device_id stm32_rproc_match[] = {
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4
  2020-04-24 20:24 [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4 Mathieu Poirier
                   ` (11 preceding siblings ...)
  2020-04-24 20:25 ` [PATCH v2 12/12] remoteproc: stm32: Set synchronisation state machine if needed Mathieu Poirier
@ 2020-04-29 15:08 ` Arnaud POULIQUEN
  2020-05-01 17:59   ` Mathieu Poirier
  12 siblings, 1 reply; 30+ messages in thread
From: Arnaud POULIQUEN @ 2020-04-29 15:08 UTC (permalink / raw)
  To: Mathieu Poirier, bjorn.andersson, ohad, mcoquelin.stm32,
	alexandre.torgue
  Cc: loic.pallardy, linux-remoteproc, linux-stm32, linux-arm-kernel,
	linux-kernel

Hi Mathieu,

On 4/24/20 10:24 PM, Mathieu Poirier wrote:
> 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 the M4 remote processor that would have been started by
> the boot loader or another entity.
> 
> It carries the same functionatlity as the previeous revision but account
> for changes in the remoteproc core to support synchronisation scenarios.
> Some RB tags have been removed when the content of the patch has strayed 
> too far from the original version. See patch 3, 8, 9 and 12 for more
> details.

I reviewed the series, and made some tests on my side.
FYI, I do not answer to patches when tagged "Reviewed-by: Loic Pallardy" 
and with no extra remark. So consider them as Reviewed-by me but not
necessary to add the tag in commit, Reviewed-by: loic in commit is sufficient. 

Concerning tests, it works find except the crash recovery from a sync start.
But i suppose that you know the limitation, waiting Loic patches[1] update :)

[1]: https://lkml.org/lkml/2020/3/11/403

Thanks a lot for your work!
Arnaud
 

> 
> Tested on ST's mp157c board.
> 
> Thanks,
> Mathieu
> 
> [1]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=277049
> [2]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=239877
> 
> Mathieu Poirier (12):
>   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 ops for synchronisation
>   remoteproc: stm32: Update M4 state in stm32_rproc_stop()
>   remoteproc: stm32: Introduce new parse fw ops for synchronisation
>   remoteproc: stm32: Introduce new loaded rsc ops for synchronisation
>   remoteproc: stm32: Set synchronisation state machine if needed
> 
>  drivers/remoteproc/stm32_rproc.c | 262 ++++++++++++++++++++++++++++---
>  1 file changed, 244 insertions(+), 18 deletions(-)
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 03/12] remoteproc: stm32: Decouple rproc from DT parsing
  2020-04-29 13:37   ` Arnaud POULIQUEN
@ 2020-04-30 20:58     ` Mathieu Poirier
  0 siblings, 0 replies; 30+ messages in thread
From: Mathieu Poirier @ 2020-04-30 20:58 UTC (permalink / raw)
  To: Arnaud POULIQUEN
  Cc: bjorn.andersson, ohad, mcoquelin.stm32, alexandre.torgue,
	loic.pallardy, linux-remoteproc, linux-stm32, linux-arm-kernel,
	linux-kernel

On Wed, Apr 29, 2020 at 03:37:58PM +0200, Arnaud POULIQUEN wrote:
> Hi Mathieu,
> 
> On 4/24/20 10:24 PM, Mathieu Poirier wrote:
> > 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 M4 or not.
> > 
> > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> > ---
> >  drivers/remoteproc/stm32_rproc.c | 25 ++++++++++++++-----------
> >  1 file changed, 14 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
> > index 1ac90adba9b1..57a426ea620b 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_info(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,6 +615,7 @@ 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;
> 
> Nitpicking: Seems that you don't need to initialize it. 

I think you are correct.

> Perhaps you can simply suppress the local variable and directly use rproc->auto_boot.

... and change the value of rproc->auto_boot if state == M4_STATE_CRUN?  Sure,
that's possible.

Thanks for all the comments, it really helps to have a different perspective.  I
am out of time for today but will continue with the rest of your comments
tomorrow.

Mathieu

> 
> else LGTM
> 
> 
> Thanks,
> Arnaud
> 
> >  	int ret;
> >  
> >  	ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
> > @@ -626,9 +626,16 @@ static int stm32_rproc_probe(struct platform_device *pdev)
> >  	if (!rproc)
> >  		return -ENOMEM;
> >  
> > +	ddata = rproc->priv;
> > +
> >  	rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
> > +
> > +	ret = stm32_rproc_parse_dt(pdev, ddata, &auto_boot);
> > +	if (ret)
> > +		goto free_rproc;
> > +
> > +	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");
> > @@ -638,13 +645,9 @@ static int stm32_rproc_probe(struct platform_device *pdev)
> >  
> >  	platform_set_drvdata(pdev, rproc);
> >  
> > -	ret = stm32_rproc_parse_dt(pdev);
> > -	if (ret)
> > -		goto free_wkq;
> > -
> >  	ret = stm32_rproc_request_mbox(rproc);
> >  	if (ret)
> > -		goto free_rproc;
> > +		goto free_wkq;
> >  
> >  	ret = rproc_add(rproc);
> >  	if (ret)
> > 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 06/12] remoteproc: stm32: Get coprocessor state
  2020-04-29 13:38   ` Arnaud POULIQUEN
@ 2020-05-01 17:40     ` Mathieu Poirier
  0 siblings, 0 replies; 30+ messages in thread
From: Mathieu Poirier @ 2020-05-01 17:40 UTC (permalink / raw)
  To: Arnaud POULIQUEN
  Cc: bjorn.andersson, ohad, mcoquelin.stm32, alexandre.torgue,
	loic.pallardy, linux-remoteproc, linux-stm32, linux-arm-kernel,
	linux-kernel

On Wed, Apr 29, 2020 at 03:38:24PM +0200, Arnaud POULIQUEN wrote:
> 
> 
> On 4/24/20 10:24 PM, Mathieu Poirier wrote:
> > 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>
> > Reviewed-by: Loic Pallardy <loic.pallardy@st.com>
> > ---
> >  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 a285f338bed8..89fbd2ffac93 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);
> > +}
> i would manage here the default state depending on the error types
> if (!ddata->m4_state.map {
> 	/* 
> 	 * We couldn't get the coprocessor's state, assume
> 	 * it is not running.
> 	 */
> 	state = M4_STATE_OFF;
> 
> 	return 0;
> }
> 
> 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;
> >  
> > @@ -664,6 +684,15 @@ static int stm32_rproc_probe(struct platform_device *pdev)
> >  	if (ret)
> >  		goto free_rproc;
> >  
> > +	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;
> 
> So here just handle the error;

Ok

> 
> Regards
> Arnaud
> > +	}
> > +
> >  	rproc->auto_boot = auto_boot;
> >  	rproc->has_iommu = false;
> >  	ddata->workqueue = create_workqueue(dev_name(dev));
> > 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 12/12] remoteproc: stm32: Set synchronisation state machine if needed
  2020-04-29 14:47   ` Arnaud POULIQUEN
@ 2020-05-01 17:54     ` Mathieu Poirier
  0 siblings, 0 replies; 30+ messages in thread
From: Mathieu Poirier @ 2020-05-01 17:54 UTC (permalink / raw)
  To: Arnaud POULIQUEN
  Cc: bjorn.andersson, ohad, mcoquelin.stm32, alexandre.torgue,
	loic.pallardy, linux-remoteproc, linux-stm32, linux-arm-kernel,
	linux-kernel

On Wed, Apr 29, 2020 at 04:47:19PM +0200, Arnaud POULIQUEN wrote:
> 
> 
> On 4/24/20 10:25 PM, Mathieu Poirier wrote:
> > Set the flags and operations to use if the M4 has been started
> > by another entity than the remoteproc core.
> > 
> > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> > ---
> >  drivers/remoteproc/stm32_rproc.c | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
> > index dcae6103e3df..02dad3f51c7a 100644
> > --- a/drivers/remoteproc/stm32_rproc.c
> > +++ b/drivers/remoteproc/stm32_rproc.c
> > @@ -598,13 +598,20 @@ 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_stop,
> > +	.kick		= stm32_rproc_kick,
> 
> Seems independent of the path.

I agree - on the flip side I didn't find a better place to put it.  Had I did a
one line patch someone would have asked me to stuff it somewhere.  I'll have
another look to see if I can find something decent.

> 
> >  	.parse_fw       = stm32_rproc_sync_parse_fw,
> >  	.find_loaded_rsc_table = stm32_rproc_sync_elf_find_loaded_rsc_table,
> >  };
> >  
> > +static struct rproc_sync_flags st_sync_flags = {
> > +	.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 */
> > +};
> > +
> could be const

If I do make this a const I'll have to move the call to
rproc_set_state_machine() inside the "if (state == M4_STATE_CRUN)".  It also
means that people won't be able to make dynamic adjustment to the
synchronisation states based on specifics discovered at probe() time.  They will
need to declare different synchronisation ops for all the potential scenarios.

I don't have a strong opinion on any of this.  I'll wait a little to see what
other people think.  If nobody chimes in I'll make this a const in the next
revision.

> 
> >  static const struct of_device_id stm32_rproc_match[] = {
> >  	{ .compatible = "st,stm32mp1-m4" },
> >  	{},
> > @@ -803,6 +810,7 @@ static int stm32_rproc_probe(struct platform_device *pdev)
> >  	struct stm32_rproc *ddata;
> >  	struct device_node *np = dev->of_node;
> >  	struct rproc *rproc;
> > +	struct rproc_sync_flags sync_flags = {0};
> >  	unsigned int state;
> >  	bool auto_boot = false;
> >  	int ret;
> > @@ -837,11 +845,17 @@ static int stm32_rproc_probe(struct platform_device *pdev)
> >  	}
> >  
> >  	if (state == M4_STATE_CRUN) {
> > +		auto_boot = true;
> > +		sync_flags = st_sync_flags;
> 
> seems an useless copy 
> 
> Regards,
> Arnaud
> 
> >  		ret = stm32_rproc_get_loaded_rsc_table(pdev, ddata);
> >  		if (ret)
> >  			goto free_rproc;
> >  	}
> >  
> > +	ret = rproc_set_state_machine(rproc, &st_rproc_sync_ops, sync_flags);
> > +	if (ret)
> > +		goto free_rproc;
> > +
> >  	rproc->auto_boot = auto_boot;
> >  	rproc->has_iommu = false;
> >  	ddata->workqueue = create_workqueue(dev_name(dev));
> > 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4
  2020-04-29 15:08 ` [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4 Arnaud POULIQUEN
@ 2020-05-01 17:59   ` Mathieu Poirier
  0 siblings, 0 replies; 30+ messages in thread
From: Mathieu Poirier @ 2020-05-01 17:59 UTC (permalink / raw)
  To: Arnaud POULIQUEN
  Cc: bjorn.andersson, ohad, mcoquelin.stm32, alexandre.torgue,
	loic.pallardy, linux-remoteproc, linux-stm32, linux-arm-kernel,
	linux-kernel

On Wed, Apr 29, 2020 at 05:08:32PM +0200, Arnaud POULIQUEN wrote:
> Hi Mathieu,
> 
> On 4/24/20 10:24 PM, Mathieu Poirier wrote:
> > 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 the M4 remote processor that would have been started by
> > the boot loader or another entity.
> > 
> > It carries the same functionatlity as the previeous revision but account
> > for changes in the remoteproc core to support synchronisation scenarios.
> > Some RB tags have been removed when the content of the patch has strayed 
> > too far from the original version. See patch 3, 8, 9 and 12 for more
> > details.
> 
> I reviewed the series, and made some tests on my side.
> FYI, I do not answer to patches when tagged "Reviewed-by: Loic Pallardy" 
> and with no extra remark. So consider them as Reviewed-by me but not
> necessary to add the tag in commit, Reviewed-by: loic in commit is sufficient. 

Well, if you spent all this time reviewing the code might as well get credit for
it...  And it also helps maintainers get a feel for how many eyes have looked
at the code.

> 
> Concerning tests, it works find except the crash recovery from a sync start.
> But i suppose that you know the limitation, waiting Loic patches[1] update :)

As I commented in the patch itself, I'll fix this so that the condition leading
to the recovery limbo can't happen.

Thanks,
Mathieu

> 
> [1]: https://lkml.org/lkml/2020/3/11/403
> 
> Thanks a lot for your work!
> Arnaud
>  
> 
> > 
> > Tested on ST's mp157c board.
> > 
> > Thanks,
> > Mathieu
> > 
> > [1]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=277049
> > [2]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=239877
> > 
> > Mathieu Poirier (12):
> >   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 ops for synchronisation
> >   remoteproc: stm32: Update M4 state in stm32_rproc_stop()
> >   remoteproc: stm32: Introduce new parse fw ops for synchronisation
> >   remoteproc: stm32: Introduce new loaded rsc ops for synchronisation
> >   remoteproc: stm32: Set synchronisation state machine if needed
> > 
> >  drivers/remoteproc/stm32_rproc.c | 262 ++++++++++++++++++++++++++++---
> >  1 file changed, 244 insertions(+), 18 deletions(-)
> > 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 01/12] remoteproc: stm32: Decouple rproc from memory translation
  2020-04-24 20:24 ` [PATCH v2 01/12] remoteproc: stm32: Decouple rproc from memory translation Mathieu Poirier
@ 2020-05-14  4:57   ` Bjorn Andersson
  0 siblings, 0 replies; 30+ messages in thread
From: Bjorn Andersson @ 2020-05-14  4:57 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: ohad, mcoquelin.stm32, alexandre.torgue, loic.pallardy,
	arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

On Fri 24 Apr 13:24 PDT 2020, Mathieu Poirier wrote:

> 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>
> Reviewed-by: Loic Pallardy <loic.pallardy@st.com>

Reviewed-by: Bjorn Andersson <bjorn.andersson@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 0f9d02ca4f5a..91fd59af0ffe 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
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 02/12] remoteproc: stm32: Request IRQ with platform device
  2020-04-24 20:24 ` [PATCH v2 02/12] remoteproc: stm32: Request IRQ with platform device Mathieu Poirier
@ 2020-05-14  4:57   ` Bjorn Andersson
  0 siblings, 0 replies; 30+ messages in thread
From: Bjorn Andersson @ 2020-05-14  4:57 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: ohad, mcoquelin.stm32, alexandre.torgue, loic.pallardy,
	arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

On Fri 24 Apr 13:24 PDT 2020, Mathieu Poirier wrote:

> 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>
> Reviewed-by: Loic Pallardy <loic.pallardy@st.com>

Reviewed-by: Bjorn Andersson <bjorn.andersson@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 91fd59af0ffe..1ac90adba9b1 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
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 03/12] remoteproc: stm32: Decouple rproc from DT parsing
  2020-04-24 20:24 ` [PATCH v2 03/12] remoteproc: stm32: Decouple rproc from DT parsing Mathieu Poirier
  2020-04-29 13:37   ` Arnaud POULIQUEN
@ 2020-05-14  4:59   ` Bjorn Andersson
  1 sibling, 0 replies; 30+ messages in thread
From: Bjorn Andersson @ 2020-05-14  4:59 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: ohad, mcoquelin.stm32, alexandre.torgue, loic.pallardy,
	arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

On Fri 24 Apr 13:24 PDT 2020, Mathieu Poirier wrote:

> 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 M4 or not.
> 
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> ---
>  drivers/remoteproc/stm32_rproc.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
> index 1ac90adba9b1..57a426ea620b 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_info(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,6 +615,7 @@ 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));
> @@ -626,9 +626,16 @@ static int stm32_rproc_probe(struct platform_device *pdev)
>  	if (!rproc)
>  		return -ENOMEM;
>  
> +	ddata = rproc->priv;
> +
>  	rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
> +
> +	ret = stm32_rproc_parse_dt(pdev, ddata, &auto_boot);
> +	if (ret)
> +		goto free_rproc;
> +
> +	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");
> @@ -638,13 +645,9 @@ static int stm32_rproc_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, rproc);
>  
> -	ret = stm32_rproc_parse_dt(pdev);
> -	if (ret)
> -		goto free_wkq;
> -
>  	ret = stm32_rproc_request_mbox(rproc);
>  	if (ret)
> -		goto free_rproc;
> +		goto free_wkq;
>  
>  	ret = rproc_add(rproc);
>  	if (ret)
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 04/12] remoteproc: stm32: Remove memory translation from DT parsing
  2020-04-24 20:24 ` [PATCH v2 04/12] remoteproc: stm32: Remove memory translation " Mathieu Poirier
@ 2020-05-14  5:03   ` Bjorn Andersson
  0 siblings, 0 replies; 30+ messages in thread
From: Bjorn Andersson @ 2020-05-14  5:03 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: ohad, mcoquelin.stm32, alexandre.torgue, loic.pallardy,
	arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

On Fri 24 Apr 13:24 PDT 2020, Mathieu Poirier wrote:

> 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>
> Reviewed-by: Loic Pallardy <loic.pallardy@st.com>

Reviewed-by: Bjorn Andersson <bjorn.andersson@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 57a426ea620b..658439d4b00a 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)
> @@ -634,6 +634,10 @@ static int stm32_rproc_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto free_rproc;
>  
> +	ret = stm32_rproc_of_memory_translations(pdev, ddata);
> +	if (ret)
> +		goto free_rproc;
> +
>  	rproc->auto_boot = auto_boot;
>  	rproc->has_iommu = false;
>  	ddata->workqueue = create_workqueue(dev_name(dev));
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 05/12] remoteproc: stm32: Parse syscon that will manage M4 synchronisation
  2020-04-24 20:24 ` [PATCH v2 05/12] remoteproc: stm32: Parse syscon that will manage M4 synchronisation Mathieu Poirier
@ 2020-05-14  5:03   ` Bjorn Andersson
  0 siblings, 0 replies; 30+ messages in thread
From: Bjorn Andersson @ 2020-05-14  5:03 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: ohad, mcoquelin.stm32, alexandre.torgue, loic.pallardy,
	arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

On Fri 24 Apr 13:24 PDT 2020, Mathieu Poirier wrote:

> 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>
> Reviewed-by: Loic Pallardy <loic.pallardy@st.com>

Reviewed-by: Bjorn Andersson <bjorn.andersson@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 658439d4b00a..a285f338bed8 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
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 10/12] remoteproc: stm32: Introduce new parse fw ops for synchronisation
  2020-04-24 20:25 ` [PATCH v2 10/12] remoteproc: stm32: Introduce new parse fw ops for synchronisation Mathieu Poirier
@ 2020-05-14  5:13   ` Bjorn Andersson
  0 siblings, 0 replies; 30+ messages in thread
From: Bjorn Andersson @ 2020-05-14  5:13 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: ohad, mcoquelin.stm32, alexandre.torgue, loic.pallardy,
	arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

On Fri 24 Apr 13:25 PDT 2020, Mathieu Poirier wrote:

> 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>
> Reviewed-by: Loic Pallardy <loic.pallardy@st.com>
> ---
>  drivers/remoteproc/stm32_rproc.c | 51 +++++++++++++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
> index 86d23c35d805..b8ae8aed5585 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) {

Does it really make sense to try to sync with a remote that doesn't have
a resource table?

> +		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);

It's unfortunate that we need to create a clone of the resource table
that we found in ram, and then return the original memory when the core
ask for the loaded table...

I wonder if we somehow can avoid this in the core (i.e. skip overwriting
table_ptr with the cached_table during stop)

> +		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,30 @@ 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)

Rather than having a function parse_fw that is passed no fw and return
some state that was setup in probe, how about just do this during probe?

Regards,
Bjorn

> +{
> +	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;
> @@ -544,6 +592,7 @@ static struct rproc_ops st_rproc_ops = {
>  static __maybe_unused struct rproc_ops st_rproc_sync_ops = {
>  	.start		= stm32_rproc_sync_start,
>  	.stop		= stm32_rproc_stop,
> +	.parse_fw       = stm32_rproc_sync_parse_fw,
>  };
>  
>  static const struct of_device_id stm32_rproc_match[] = {
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v2 11/12] remoteproc: stm32: Introduce new loaded rsc ops for synchronisation
  2020-04-24 20:25 ` [PATCH v2 11/12] remoteproc: stm32: Introduce new loaded rsc " Mathieu Poirier
@ 2020-05-14  5:15   ` Bjorn Andersson
  0 siblings, 0 replies; 30+ messages in thread
From: Bjorn Andersson @ 2020-05-14  5:15 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: ohad, mcoquelin.stm32, alexandre.torgue, loic.pallardy,
	arnaud.pouliquen, linux-remoteproc, linux-stm32,
	linux-arm-kernel, linux-kernel

On Fri 24 Apr 13:25 PDT 2020, Mathieu Poirier wrote:

> 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>
> Reviewed-by: Loic Pallardy <loic.pallardy@st.com>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>


But I would have preferred if we during probe (when we discover rsc_va)
could just set it on the rproc.

Regards,
Bjorn

> ---
>  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 b8ae8aed5585..dcae6103e3df 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -319,6 +319,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;
> @@ -593,6 +602,7 @@ static __maybe_unused struct rproc_ops st_rproc_sync_ops = {
>  	.start		= stm32_rproc_sync_start,
>  	.stop		= stm32_rproc_stop,
>  	.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
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2020-05-14  5:17 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-24 20:24 [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4 Mathieu Poirier
2020-04-24 20:24 ` [PATCH v2 01/12] remoteproc: stm32: Decouple rproc from memory translation Mathieu Poirier
2020-05-14  4:57   ` Bjorn Andersson
2020-04-24 20:24 ` [PATCH v2 02/12] remoteproc: stm32: Request IRQ with platform device Mathieu Poirier
2020-05-14  4:57   ` Bjorn Andersson
2020-04-24 20:24 ` [PATCH v2 03/12] remoteproc: stm32: Decouple rproc from DT parsing Mathieu Poirier
2020-04-29 13:37   ` Arnaud POULIQUEN
2020-04-30 20:58     ` Mathieu Poirier
2020-05-14  4:59   ` Bjorn Andersson
2020-04-24 20:24 ` [PATCH v2 04/12] remoteproc: stm32: Remove memory translation " Mathieu Poirier
2020-05-14  5:03   ` Bjorn Andersson
2020-04-24 20:24 ` [PATCH v2 05/12] remoteproc: stm32: Parse syscon that will manage M4 synchronisation Mathieu Poirier
2020-05-14  5:03   ` Bjorn Andersson
2020-04-24 20:24 ` [PATCH v2 06/12] remoteproc: stm32: Get coprocessor state Mathieu Poirier
2020-04-29 13:38   ` Arnaud POULIQUEN
2020-05-01 17:40     ` Mathieu Poirier
2020-04-24 20:25 ` [PATCH v2 07/12] remoteproc: stm32: Get loaded resource table for synchronisation Mathieu Poirier
2020-04-24 20:25 ` [PATCH v2 08/12] remoteproc: stm32: Introduce new start ops " Mathieu Poirier
2020-04-29 14:50   ` Arnaud POULIQUEN
2020-04-24 20:25 ` [PATCH v2 09/12] remoteproc: stm32: Update M4 state in stm32_rproc_stop() Mathieu Poirier
2020-04-29 14:52   ` Arnaud POULIQUEN
2020-04-24 20:25 ` [PATCH v2 10/12] remoteproc: stm32: Introduce new parse fw ops for synchronisation Mathieu Poirier
2020-05-14  5:13   ` Bjorn Andersson
2020-04-24 20:25 ` [PATCH v2 11/12] remoteproc: stm32: Introduce new loaded rsc " Mathieu Poirier
2020-05-14  5:15   ` Bjorn Andersson
2020-04-24 20:25 ` [PATCH v2 12/12] remoteproc: stm32: Set synchronisation state machine if needed Mathieu Poirier
2020-04-29 14:47   ` Arnaud POULIQUEN
2020-05-01 17:54     ` Mathieu Poirier
2020-04-29 15:08 ` [PATCH v2 00/12] remoteproc: stm32: Add support for synchronising with M4 Arnaud POULIQUEN
2020-05-01 17:59   ` Mathieu Poirier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).