linux-remoteproc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] remoteproc: stm32: add support of detaching a remote processor
@ 2021-03-22  9:26 Arnaud Pouliquen
  2021-03-22  9:26 ` [PATCH v2 1/2] dt-bindings: remoteproc: stm32-rproc: add new mailbox channel for detach Arnaud Pouliquen
  2021-03-22  9:26 ` [PATCH v2 2/2] remoteproc: stm32: add capability to detach Arnaud Pouliquen
  0 siblings, 2 replies; 8+ messages in thread
From: Arnaud Pouliquen @ 2021-03-22  9:26 UTC (permalink / raw)
  To: Bjorn Andersson, Ohad Ben-Cohen, Mathieu Poirier, Rob Herring,
	Alexandre Torgue
  Cc: devicetree, linux-stm32, linux-arm-kernel, linux-remoteproc,
	linux-kernel, Arnaud Pouliquen

Update from V1:
Fix bindings issue, reported by Rob Herring.

This patchset is the stm32mp1 platform implementation of the detach operation
added in series [1].

On detach, the stm32 rproc driver sends a mailbox signal to the remote 
processor to inform it that it will be detached. 

Applied and tested on Bjorn's "for_next" branch (2b81aa17008e)

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

Arnaud Pouliquen (2):
  dt-bindings: remoteproc: stm32-rproc: add new mailbox channel for
    detach
  remoteproc: stm32: add capability to detach

 .../bindings/remoteproc/st,stm32-rproc.yaml   | 11 +++++-
 drivers/remoteproc/stm32_rproc.c              | 38 ++++++++++++++++++-
 2 files changed, 45 insertions(+), 4 deletions(-)

-- 
2.17.1


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

* [PATCH v2 1/2] dt-bindings: remoteproc: stm32-rproc: add new mailbox channel for detach
  2021-03-22  9:26 [PATCH v2 0/2] remoteproc: stm32: add support of detaching a remote processor Arnaud Pouliquen
@ 2021-03-22  9:26 ` Arnaud Pouliquen
  2021-03-30 14:41   ` Rob Herring
  2021-03-22  9:26 ` [PATCH v2 2/2] remoteproc: stm32: add capability to detach Arnaud Pouliquen
  1 sibling, 1 reply; 8+ messages in thread
From: Arnaud Pouliquen @ 2021-03-22  9:26 UTC (permalink / raw)
  To: Bjorn Andersson, Ohad Ben-Cohen, Mathieu Poirier, Rob Herring,
	Alexandre Torgue
  Cc: devicetree, linux-stm32, linux-arm-kernel, linux-remoteproc,
	linux-kernel, Arnaud Pouliquen

Add the "detach" mailbox item, that allows to define a mailbox to
send a IPCC signal to the remote processor on remoteproc detach action.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
---
update from V1:

Fix indentation error reported by 'make dt_binding_check'.

---
 .../bindings/remoteproc/st,stm32-rproc.yaml           | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml
index a1171dfba024..64afdcfb613d 100644
--- a/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml
@@ -65,16 +65,23 @@ properties:
           Unidirectional channel:
             - from local to remote, where ACK from the remote means that it is
               ready for shutdown
+      - description: |
+          A channel (d) used by the local proc to notify the remote proc that it
+          has to stop interprocessor communnication.
+          Unidirectional channel:
+            - from local to remote, where ACK from the remote means that communnication
+              as been stopped on the remote side.
     minItems: 1
-    maxItems: 3
+    maxItems: 4
 
   mbox-names:
     items:
       - const: vq0
       - const: vq1
       - const: shutdown
+      - const: detach
     minItems: 1
-    maxItems: 3
+    maxItems: 4
 
   memory-region:
     description:
-- 
2.17.1


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

* [PATCH v2 2/2] remoteproc: stm32: add capability to detach
  2021-03-22  9:26 [PATCH v2 0/2] remoteproc: stm32: add support of detaching a remote processor Arnaud Pouliquen
  2021-03-22  9:26 ` [PATCH v2 1/2] dt-bindings: remoteproc: stm32-rproc: add new mailbox channel for detach Arnaud Pouliquen
@ 2021-03-22  9:26 ` Arnaud Pouliquen
  2021-03-23 21:19   ` Mathieu Poirier
  1 sibling, 1 reply; 8+ messages in thread
From: Arnaud Pouliquen @ 2021-03-22  9:26 UTC (permalink / raw)
  To: Bjorn Andersson, Ohad Ben-Cohen, Mathieu Poirier, Rob Herring,
	Alexandre Torgue
  Cc: devicetree, linux-stm32, linux-arm-kernel, linux-remoteproc,
	linux-kernel, Arnaud Pouliquen

From: Arnaud Pouliquen <arnaud.pouliquen@foss-st.com>

A mechanism similar to the shutdown mailbox signal is implemented to
detach a remote processor.

Upon detachment, a signal is sent to the remote firmware, allowing it
to perform specific actions such as stopping RPMsg communication.

The Cortex-M hold boot is also disabled to allow the remote processor
to restart in case of crash.

Notice that for this feature to be supported, the remote firmware
resource table must be stored at the beginning of a 1kB section
(default size provided to the remoteproc core).

This restriction should be lifted in the future by using a backup
register to store the actual size of the resource table.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss-st.com>
---
 drivers/remoteproc/stm32_rproc.c | 38 ++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
index 3d45f51de4d0..298ef5b19e27 100644
--- a/drivers/remoteproc/stm32_rproc.c
+++ b/drivers/remoteproc/stm32_rproc.c
@@ -28,7 +28,7 @@
 #define RELEASE_BOOT		1
 
 #define MBOX_NB_VQ		2
-#define MBOX_NB_MBX		3
+#define MBOX_NB_MBX		4
 
 #define STM32_SMC_RCC		0x82001000
 #define STM32_SMC_REG_WRITE	0x1
@@ -38,6 +38,7 @@
 #define STM32_MBX_VQ1		"vq1"
 #define STM32_MBX_VQ1_ID	1
 #define STM32_MBX_SHUTDOWN	"shutdown"
+#define STM32_MBX_DETACH	"detach"
 
 #define RSC_TBL_SIZE		1024
 
@@ -336,6 +337,15 @@ static const struct stm32_mbox stm32_rproc_mbox[MBOX_NB_MBX] = {
 			.tx_done = NULL,
 			.tx_tout = 500, /* 500 ms time out */
 		},
+	},
+	{
+		.name = STM32_MBX_DETACH,
+		.vq_id = -1,
+		.client = {
+			.tx_block = true,
+			.tx_done = NULL,
+			.tx_tout = 200, /* 200 ms time out to detach should be fair enough */
+		},
 	}
 };
 
@@ -461,6 +471,25 @@ static int stm32_rproc_attach(struct rproc *rproc)
 	return stm32_rproc_set_hold_boot(rproc, true);
 }
 
+static int stm32_rproc_detach(struct rproc *rproc)
+{
+	struct stm32_rproc *ddata = rproc->priv;
+	int err, dummy_data, idx;
+
+	/* Inform the remote processor of the detach */
+	idx = stm32_rproc_mbox_idx(rproc, STM32_MBX_DETACH);
+	if (idx >= 0 && ddata->mb[idx].chan) {
+		/* A dummy data is sent to allow to block on transmit */
+		err = mbox_send_message(ddata->mb[idx].chan,
+					&dummy_data);
+		if (err < 0)
+			dev_warn(&rproc->dev, "warning: remote FW detach without ack\n");
+	}
+
+	/* Allow remote processor to auto-reboot */
+	return stm32_rproc_set_hold_boot(rproc, false);
+}
+
 static int stm32_rproc_stop(struct rproc *rproc)
 {
 	struct stm32_rproc *ddata = rproc->priv;
@@ -597,7 +626,11 @@ stm32_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz)
 	}
 
 done:
-	/* Assuming the resource table fits in 1kB is fair */
+	/*
+	 * Assuming the resource table fits in 1kB is fair.
+	 * Notice for the detach, that this 1 kB memory area has to be reserved in the coprocessor
+	 * firmware for the resource table. A clean of this whole area is done on detach.
+	 */
 	*table_sz = RSC_TBL_SIZE;
 	return (struct resource_table *)ddata->rsc_va;
 }
@@ -607,6 +640,7 @@ static const struct rproc_ops st_rproc_ops = {
 	.start		= stm32_rproc_start,
 	.stop		= stm32_rproc_stop,
 	.attach		= stm32_rproc_attach,
+	.detach		= stm32_rproc_detach,
 	.kick		= stm32_rproc_kick,
 	.load		= rproc_elf_load_segments,
 	.parse_fw	= stm32_rproc_parse_fw,
-- 
2.17.1


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

* Re: [PATCH v2 2/2] remoteproc: stm32: add capability to detach
  2021-03-22  9:26 ` [PATCH v2 2/2] remoteproc: stm32: add capability to detach Arnaud Pouliquen
@ 2021-03-23 21:19   ` Mathieu Poirier
  2021-03-24 14:07     ` Arnaud POULIQUEN
  2021-03-25 22:38     ` Mathieu Poirier
  0 siblings, 2 replies; 8+ messages in thread
From: Mathieu Poirier @ 2021-03-23 21:19 UTC (permalink / raw)
  To: Arnaud Pouliquen
  Cc: Bjorn Andersson, Ohad Ben-Cohen, Rob Herring, Alexandre Torgue,
	devicetree, linux-stm32, linux-arm-kernel, linux-remoteproc,
	linux-kernel

Good day Arnaud,

On Mon, Mar 22, 2021 at 10:26:51AM +0100, Arnaud Pouliquen wrote:
> From: Arnaud Pouliquen <arnaud.pouliquen@foss-st.com>
> 
> A mechanism similar to the shutdown mailbox signal is implemented to
> detach a remote processor.
> 
> Upon detachment, a signal is sent to the remote firmware, allowing it
> to perform specific actions such as stopping RPMsg communication.
> 
> The Cortex-M hold boot is also disabled to allow the remote processor
> to restart in case of crash.
> 
> Notice that for this feature to be supported, the remote firmware
> resource table must be stored at the beginning of a 1kB section
> (default size provided to the remoteproc core).
> 
> This restriction should be lifted in the future by using a backup
> register to store the actual size of the resource table.

I'm not sure the above two paragraphs add anything valuable to the changelog.
At this time the size of 1kB is fixed and future enhancement are, well, in the
future.  So for now this patch is working with the rest of the current
environment and that is the important part.

> 
> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss-st.com>
> ---
>  drivers/remoteproc/stm32_rproc.c | 38 ++++++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
> index 3d45f51de4d0..298ef5b19e27 100644
> --- a/drivers/remoteproc/stm32_rproc.c
> +++ b/drivers/remoteproc/stm32_rproc.c
> @@ -28,7 +28,7 @@
>  #define RELEASE_BOOT		1
>  
>  #define MBOX_NB_VQ		2
> -#define MBOX_NB_MBX		3
> +#define MBOX_NB_MBX		4
>  
>  #define STM32_SMC_RCC		0x82001000
>  #define STM32_SMC_REG_WRITE	0x1
> @@ -38,6 +38,7 @@
>  #define STM32_MBX_VQ1		"vq1"
>  #define STM32_MBX_VQ1_ID	1
>  #define STM32_MBX_SHUTDOWN	"shutdown"
> +#define STM32_MBX_DETACH	"detach"
>  
>  #define RSC_TBL_SIZE		1024
>  
> @@ -336,6 +337,15 @@ static const struct stm32_mbox stm32_rproc_mbox[MBOX_NB_MBX] = {
>  			.tx_done = NULL,
>  			.tx_tout = 500, /* 500 ms time out */
>  		},
> +	},
> +	{
> +		.name = STM32_MBX_DETACH,
> +		.vq_id = -1,
> +		.client = {
> +			.tx_block = true,
> +			.tx_done = NULL,
> +			.tx_tout = 200, /* 200 ms time out to detach should be fair enough */
> +		},
>  	}
>  };
>  
> @@ -461,6 +471,25 @@ static int stm32_rproc_attach(struct rproc *rproc)
>  	return stm32_rproc_set_hold_boot(rproc, true);
>  }
>  
> +static int stm32_rproc_detach(struct rproc *rproc)
> +{
> +	struct stm32_rproc *ddata = rproc->priv;
> +	int err, dummy_data, idx;
> +
> +	/* Inform the remote processor of the detach */
> +	idx = stm32_rproc_mbox_idx(rproc, STM32_MBX_DETACH);
> +	if (idx >= 0 && ddata->mb[idx].chan) {
> +		/* A dummy data is sent to allow to block on transmit */
> +		err = mbox_send_message(ddata->mb[idx].chan,
> +					&dummy_data);
> +		if (err < 0)
> +			dev_warn(&rproc->dev, "warning: remote FW detach without ack\n");
> +	}
> +
> +	/* Allow remote processor to auto-reboot */
> +	return stm32_rproc_set_hold_boot(rproc, false);
> +}
> +
>  static int stm32_rproc_stop(struct rproc *rproc)
>  {
>  	struct stm32_rproc *ddata = rproc->priv;
> @@ -597,7 +626,11 @@ stm32_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz)
>  	}
>  
>  done:
> -	/* Assuming the resource table fits in 1kB is fair */
> +	/*
> +	 * Assuming the resource table fits in 1kB is fair.
> +	 * Notice for the detach, that this 1 kB memory area has to be reserved in the coprocessor
> +	 * firmware for the resource table. A clean of this whole area is done on detach.
> +	 */

Can you rework the last sentence?  I'm not sure if it means the M4 will clean
the resource table or if that should be the application processor... I'm also
not clear on what you mean by "clean".  Usually it means zero'ing out but in
this case it means a re-initialisation of the original values.


>  	*table_sz = RSC_TBL_SIZE;
>  	return (struct resource_table *)ddata->rsc_va;
>  }
> @@ -607,6 +640,7 @@ static const struct rproc_ops st_rproc_ops = {
>  	.start		= stm32_rproc_start,
>  	.stop		= stm32_rproc_stop,
>  	.attach		= stm32_rproc_attach,
> +	.detach		= stm32_rproc_detach,

With the above:

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

>  	.kick		= stm32_rproc_kick,
>  	.load		= rproc_elf_load_segments,
>  	.parse_fw	= stm32_rproc_parse_fw,
> -- 
> 2.17.1
> 

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

* Re: [PATCH v2 2/2] remoteproc: stm32: add capability to detach
  2021-03-23 21:19   ` Mathieu Poirier
@ 2021-03-24 14:07     ` Arnaud POULIQUEN
  2021-03-25 22:38     ` Mathieu Poirier
  1 sibling, 0 replies; 8+ messages in thread
From: Arnaud POULIQUEN @ 2021-03-24 14:07 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Bjorn Andersson, Ohad Ben-Cohen, Rob Herring, Alexandre Torgue,
	devicetree, linux-stm32, linux-arm-kernel, linux-remoteproc,
	linux-kernel

Hi Mathieu,

On 3/23/21 10:19 PM, Mathieu Poirier wrote:
> Good day Arnaud,
> 
> On Mon, Mar 22, 2021 at 10:26:51AM +0100, Arnaud Pouliquen wrote:
>> From: Arnaud Pouliquen <arnaud.pouliquen@foss-st.com>
>>
>> A mechanism similar to the shutdown mailbox signal is implemented to
>> detach a remote processor.
>>
>> Upon detachment, a signal is sent to the remote firmware, allowing it
>> to perform specific actions such as stopping RPMsg communication.
>>
>> The Cortex-M hold boot is also disabled to allow the remote processor
>> to restart in case of crash.
>>
>> Notice that for this feature to be supported, the remote firmware
>> resource table must be stored at the beginning of a 1kB section
>> (default size provided to the remoteproc core).
>>
>> This restriction should be lifted in the future by using a backup
>> register to store the actual size of the resource table.
> 
> I'm not sure the above two paragraphs add anything valuable to the changelog.
> At this time the size of 1kB is fixed and future enhancement are, well, in the
> future.  So for now this patch is working with the rest of the current
> environment and that is the important part.
> 
>>
>> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss-st.com>
>> ---
>>  drivers/remoteproc/stm32_rproc.c | 38 ++++++++++++++++++++++++++++++--
>>  1 file changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
>> index 3d45f51de4d0..298ef5b19e27 100644
>> --- a/drivers/remoteproc/stm32_rproc.c
>> +++ b/drivers/remoteproc/stm32_rproc.c
>> @@ -28,7 +28,7 @@
>>  #define RELEASE_BOOT		1
>>  
>>  #define MBOX_NB_VQ		2
>> -#define MBOX_NB_MBX		3
>> +#define MBOX_NB_MBX		4
>>  
>>  #define STM32_SMC_RCC		0x82001000
>>  #define STM32_SMC_REG_WRITE	0x1
>> @@ -38,6 +38,7 @@
>>  #define STM32_MBX_VQ1		"vq1"
>>  #define STM32_MBX_VQ1_ID	1
>>  #define STM32_MBX_SHUTDOWN	"shutdown"
>> +#define STM32_MBX_DETACH	"detach"
>>  
>>  #define RSC_TBL_SIZE		1024
>>  
>> @@ -336,6 +337,15 @@ static const struct stm32_mbox stm32_rproc_mbox[MBOX_NB_MBX] = {
>>  			.tx_done = NULL,
>>  			.tx_tout = 500, /* 500 ms time out */
>>  		},
>> +	},
>> +	{
>> +		.name = STM32_MBX_DETACH,
>> +		.vq_id = -1,
>> +		.client = {
>> +			.tx_block = true,
>> +			.tx_done = NULL,
>> +			.tx_tout = 200, /* 200 ms time out to detach should be fair enough */
>> +		},
>>  	}
>>  };
>>  
>> @@ -461,6 +471,25 @@ static int stm32_rproc_attach(struct rproc *rproc)
>>  	return stm32_rproc_set_hold_boot(rproc, true);
>>  }
>>  
>> +static int stm32_rproc_detach(struct rproc *rproc)
>> +{
>> +	struct stm32_rproc *ddata = rproc->priv;
>> +	int err, dummy_data, idx;
>> +
>> +	/* Inform the remote processor of the detach */
>> +	idx = stm32_rproc_mbox_idx(rproc, STM32_MBX_DETACH);
>> +	if (idx >= 0 && ddata->mb[idx].chan) {
>> +		/* A dummy data is sent to allow to block on transmit */
>> +		err = mbox_send_message(ddata->mb[idx].chan,
>> +					&dummy_data);
>> +		if (err < 0)
>> +			dev_warn(&rproc->dev, "warning: remote FW detach without ack\n");
>> +	}
>> +
>> +	/* Allow remote processor to auto-reboot */
>> +	return stm32_rproc_set_hold_boot(rproc, false);
>> +}
>> +
>>  static int stm32_rproc_stop(struct rproc *rproc)
>>  {
>>  	struct stm32_rproc *ddata = rproc->priv;
>> @@ -597,7 +626,11 @@ stm32_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz)
>>  	}
>>  
>>  done:
>> -	/* Assuming the resource table fits in 1kB is fair */
>> +	/*
>> +	 * Assuming the resource table fits in 1kB is fair.
>> +	 * Notice for the detach, that this 1 kB memory area has to be reserved in the coprocessor
>> +	 * firmware for the resource table. A clean of this whole area is done on detach.
>> +	 */
> 
> Can you rework the last sentence?  I'm not sure if it means the M4 will clean
> the resource table or if that should be the application processor... I'm also
> not clear on what you mean by "clean".  Usually it means zero'ing out but in
> this case it means a re-initialisation of the original values.
This is done by the remoteproc core itself by overwriting the resource table
area with the rproc->clean_table backup.
i will rework the comment in this way.

Thanks,
Arnaud

> 
> 
>>  	*table_sz = RSC_TBL_SIZE;
>>  	return (struct resource_table *)ddata->rsc_va;
>>  }
>> @@ -607,6 +640,7 @@ static const struct rproc_ops st_rproc_ops = {
>>  	.start		= stm32_rproc_start,
>>  	.stop		= stm32_rproc_stop,
>>  	.attach		= stm32_rproc_attach,
>> +	.detach		= stm32_rproc_detach,
> 
> With the above:
> 
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> 
>>  	.kick		= stm32_rproc_kick,
>>  	.load		= rproc_elf_load_segments,
>>  	.parse_fw	= stm32_rproc_parse_fw,
>> -- 
>> 2.17.1
>>

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

* Re: [PATCH v2 2/2] remoteproc: stm32: add capability to detach
  2021-03-23 21:19   ` Mathieu Poirier
  2021-03-24 14:07     ` Arnaud POULIQUEN
@ 2021-03-25 22:38     ` Mathieu Poirier
  1 sibling, 0 replies; 8+ messages in thread
From: Mathieu Poirier @ 2021-03-25 22:38 UTC (permalink / raw)
  To: Arnaud Pouliquen
  Cc: Bjorn Andersson, Ohad Ben-Cohen, Rob Herring, Alexandre Torgue,
	devicetree, linux-stm32, linux-arm-kernel, linux-remoteproc,
	linux-kernel

On Tue, Mar 23, 2021 at 03:19:11PM -0600, Mathieu Poirier wrote:
> Good day Arnaud,
> 
> On Mon, Mar 22, 2021 at 10:26:51AM +0100, Arnaud Pouliquen wrote:
> > From: Arnaud Pouliquen <arnaud.pouliquen@foss-st.com>
> > 
> > A mechanism similar to the shutdown mailbox signal is implemented to
> > detach a remote processor.
> > 
> > Upon detachment, a signal is sent to the remote firmware, allowing it
> > to perform specific actions such as stopping RPMsg communication.
> > 
> > The Cortex-M hold boot is also disabled to allow the remote processor
> > to restart in case of crash.
> > 
> > Notice that for this feature to be supported, the remote firmware
> > resource table must be stored at the beginning of a 1kB section
> > (default size provided to the remoteproc core).
> > 
> > This restriction should be lifted in the future by using a backup
> > register to store the actual size of the resource table.
> 
> I'm not sure the above two paragraphs add anything valuable to the changelog.
> At this time the size of 1kB is fixed and future enhancement are, well, in the
> future.  So for now this patch is working with the rest of the current
> environment and that is the important part.
> 
> > 
> > Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss-st.com>
> > ---
> >  drivers/remoteproc/stm32_rproc.c | 38 ++++++++++++++++++++++++++++++--
> >  1 file changed, 36 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c
> > index 3d45f51de4d0..298ef5b19e27 100644
> > --- a/drivers/remoteproc/stm32_rproc.c
> > +++ b/drivers/remoteproc/stm32_rproc.c
> > @@ -28,7 +28,7 @@
> >  #define RELEASE_BOOT		1
> >  
> >  #define MBOX_NB_VQ		2
> > -#define MBOX_NB_MBX		3
> > +#define MBOX_NB_MBX		4
> >  
> >  #define STM32_SMC_RCC		0x82001000
> >  #define STM32_SMC_REG_WRITE	0x1
> > @@ -38,6 +38,7 @@
> >  #define STM32_MBX_VQ1		"vq1"
> >  #define STM32_MBX_VQ1_ID	1
> >  #define STM32_MBX_SHUTDOWN	"shutdown"
> > +#define STM32_MBX_DETACH	"detach"
> >  
> >  #define RSC_TBL_SIZE		1024
> >  
> > @@ -336,6 +337,15 @@ static const struct stm32_mbox stm32_rproc_mbox[MBOX_NB_MBX] = {
> >  			.tx_done = NULL,
> >  			.tx_tout = 500, /* 500 ms time out */
> >  		},
> > +	},
> > +	{
> > +		.name = STM32_MBX_DETACH,
> > +		.vq_id = -1,
> > +		.client = {
> > +			.tx_block = true,
> > +			.tx_done = NULL,
> > +			.tx_tout = 200, /* 200 ms time out to detach should be fair enough */
> > +		},
> >  	}
> >  };
> >  
> > @@ -461,6 +471,25 @@ static int stm32_rproc_attach(struct rproc *rproc)
> >  	return stm32_rproc_set_hold_boot(rproc, true);
> >  }
> >  
> > +static int stm32_rproc_detach(struct rproc *rproc)
> > +{
> > +	struct stm32_rproc *ddata = rproc->priv;
> > +	int err, dummy_data, idx;
> > +
> > +	/* Inform the remote processor of the detach */
> > +	idx = stm32_rproc_mbox_idx(rproc, STM32_MBX_DETACH);
> > +	if (idx >= 0 && ddata->mb[idx].chan) {
> > +		/* A dummy data is sent to allow to block on transmit */
> > +		err = mbox_send_message(ddata->mb[idx].chan,
> > +					&dummy_data);
> > +		if (err < 0)
> > +			dev_warn(&rproc->dev, "warning: remote FW detach without ack\n");
> > +	}
> > +
> > +	/* Allow remote processor to auto-reboot */
> > +	return stm32_rproc_set_hold_boot(rproc, false);
> > +}
> > +
> >  static int stm32_rproc_stop(struct rproc *rproc)
> >  {
> >  	struct stm32_rproc *ddata = rproc->priv;
> > @@ -597,7 +626,11 @@ stm32_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz)
> >  	}
> >  
> >  done:
> > -	/* Assuming the resource table fits in 1kB is fair */
> > +	/*
> > +	 * Assuming the resource table fits in 1kB is fair.
> > +	 * Notice for the detach, that this 1 kB memory area has to be reserved in the coprocessor
> > +	 * firmware for the resource table. A clean of this whole area is done on detach.
> > +	 */
> 
> Can you rework the last sentence?  I'm not sure if it means the M4 will clean
> the resource table or if that should be the application processor... I'm also
> not clear on what you mean by "clean".  Usually it means zero'ing out but in
> this case it means a re-initialisation of the original values.
> 
> 
> >  	*table_sz = RSC_TBL_SIZE;
> >  	return (struct resource_table *)ddata->rsc_va;
> >  }
> > @@ -607,6 +640,7 @@ static const struct rproc_ops st_rproc_ops = {
> >  	.start		= stm32_rproc_start,
> >  	.stop		= stm32_rproc_stop,
> >  	.attach		= stm32_rproc_attach,
> > +	.detach		= stm32_rproc_detach,
> 
> With the above:
> 
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Thanks for the firmware test image:

Tested-by: Mathieu Poirier <mathieu.poirier@linaro.org>

> 
> >  	.kick		= stm32_rproc_kick,
> >  	.load		= rproc_elf_load_segments,
> >  	.parse_fw	= stm32_rproc_parse_fw,
> > -- 
> > 2.17.1
> > 

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

* Re: [PATCH v2 1/2] dt-bindings: remoteproc: stm32-rproc: add new mailbox channel for detach
  2021-03-22  9:26 ` [PATCH v2 1/2] dt-bindings: remoteproc: stm32-rproc: add new mailbox channel for detach Arnaud Pouliquen
@ 2021-03-30 14:41   ` Rob Herring
  2021-03-31  7:31     ` Arnaud POULIQUEN
  0 siblings, 1 reply; 8+ messages in thread
From: Rob Herring @ 2021-03-30 14:41 UTC (permalink / raw)
  To: Arnaud Pouliquen
  Cc: Ohad Ben-Cohen, linux-arm-kernel, Mathieu Poirier,
	Bjorn Andersson, linux-kernel, Alexandre Torgue,
	linux-remoteproc, linux-stm32, devicetree

On Mon, 22 Mar 2021 10:26:50 +0100, Arnaud Pouliquen wrote:
> Add the "detach" mailbox item, that allows to define a mailbox to
> send a IPCC signal to the remote processor on remoteproc detach action.
> 
> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
> ---
> update from V1:
> 
> Fix indentation error reported by 'make dt_binding_check'.
> 
> ---
>  .../bindings/remoteproc/st,stm32-rproc.yaml           | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH v2 1/2] dt-bindings: remoteproc: stm32-rproc: add new mailbox channel for detach
  2021-03-30 14:41   ` Rob Herring
@ 2021-03-31  7:31     ` Arnaud POULIQUEN
  0 siblings, 0 replies; 8+ messages in thread
From: Arnaud POULIQUEN @ 2021-03-31  7:31 UTC (permalink / raw)
  To: Rob Herring
  Cc: Ohad Ben-Cohen, linux-arm-kernel, Mathieu Poirier,
	Bjorn Andersson, linux-kernel, Alexandre Torgue,
	linux-remoteproc, linux-stm32, devicetree



On 3/30/21 4:41 PM, Rob Herring wrote:
> On Mon, 22 Mar 2021 10:26:50 +0100, Arnaud Pouliquen wrote:
>> Add the "detach" mailbox item, that allows to define a mailbox to
>> send a IPCC signal to the remote processor on remoteproc detach action.
>>
>> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
>> ---
>> update from V1:
>>
>> Fix indentation error reported by 'make dt_binding_check'.
>>
>> ---
>>  .../bindings/remoteproc/st,stm32-rproc.yaml           | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> 

Thank Rob for the review
Since there is already a v3 that fixes the patch 2/2 , i will send a v4 to
include your reviewed-by

Regards,
Arnaud

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

end of thread, other threads:[~2021-03-31  7:32 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22  9:26 [PATCH v2 0/2] remoteproc: stm32: add support of detaching a remote processor Arnaud Pouliquen
2021-03-22  9:26 ` [PATCH v2 1/2] dt-bindings: remoteproc: stm32-rproc: add new mailbox channel for detach Arnaud Pouliquen
2021-03-30 14:41   ` Rob Herring
2021-03-31  7:31     ` Arnaud POULIQUEN
2021-03-22  9:26 ` [PATCH v2 2/2] remoteproc: stm32: add capability to detach Arnaud Pouliquen
2021-03-23 21:19   ` Mathieu Poirier
2021-03-24 14:07     ` Arnaud POULIQUEN
2021-03-25 22:38     ` 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).