* [PATCH 0/2] remoteproc: stm32: add support of detaching a remote processor @ 2021-03-18 14:59 Arnaud Pouliquen 2021-03-18 14:59 ` [PATCH 1/2] dt-bindings: remoteproc: stm32-rproc: add new mailbox channel for detach Arnaud Pouliquen 2021-03-18 14:59 ` [PATCH 2/2] remoteproc: stm32: add capability to detach Arnaud Pouliquen 0 siblings, 2 replies; 5+ messages in thread From: Arnaud Pouliquen @ 2021-03-18 14:59 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 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] 5+ messages in thread
* [PATCH 1/2] dt-bindings: remoteproc: stm32-rproc: add new mailbox channel for detach 2021-03-18 14:59 [PATCH 0/2] remoteproc: stm32: add support of detaching a remote processor Arnaud Pouliquen @ 2021-03-18 14:59 ` Arnaud Pouliquen 2021-03-19 21:49 ` Rob Herring 2021-03-18 14:59 ` [PATCH 2/2] remoteproc: stm32: add capability to detach Arnaud Pouliquen 1 sibling, 1 reply; 5+ messages in thread From: Arnaud Pouliquen @ 2021-03-18 14:59 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> --- .../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..f5976cb431c0 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] 5+ messages in thread
* Re: [PATCH 1/2] dt-bindings: remoteproc: stm32-rproc: add new mailbox channel for detach 2021-03-18 14:59 ` [PATCH 1/2] dt-bindings: remoteproc: stm32-rproc: add new mailbox channel for detach Arnaud Pouliquen @ 2021-03-19 21:49 ` Rob Herring 0 siblings, 0 replies; 5+ messages in thread From: Rob Herring @ 2021-03-19 21:49 UTC (permalink / raw) To: Arnaud Pouliquen Cc: linux-remoteproc, Alexandre Torgue, Mathieu Poirier, Bjorn Andersson, linux-arm-kernel, linux-kernel, linux-stm32, devicetree, Ohad Ben-Cohen On Thu, 18 Mar 2021 15:59:22 +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> > --- > .../bindings/remoteproc/st,stm32-rproc.yaml | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: ./Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml:68:8: [error] syntax error: expected <block end>, but found '<block sequence start>' (syntax) dtschema/dtc warnings/errors: make[1]: *** Deleting file 'Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.example.dts' Traceback (most recent call last): File "/usr/local/bin/dt-extract-example", line 45, in <module> binding = yaml.load(open(args.yamlfile, encoding='utf-8').read()) File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 343, in load return constructor.get_single_data() File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 111, in get_single_data node = self.composer.get_single_node() File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node File "_ruamel_yaml.pyx", line 773, in _ruamel_yaml.CParser._compose_node File "_ruamel_yaml.pyx", line 852, in _ruamel_yaml.CParser._compose_sequence_node File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event ruamel.yaml.parser.ParserError: while parsing a block collection in "<unicode string>", line 54, column 7 did not find expected '-' indicator in "<unicode string>", line 68, column 8 make[1]: *** [Documentation/devicetree/bindings/Makefile:20: Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.example.dts] Error 1 make[1]: *** Waiting for unfinished jobs.... ./Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml: while parsing a block collection in "<unicode string>", line 54, column 7 did not find expected '-' indicator in "<unicode string>", line 68, column 8 /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml: ignoring, error parsing file warning: no schema found in file: ./Documentation/devicetree/bindings/remoteproc/st,stm32-rproc.yaml make: *** [Makefile:1380: dt_binding_check] Error 2 See https://patchwork.ozlabs.org/patch/1455311 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] remoteproc: stm32: add capability to detach 2021-03-18 14:59 [PATCH 0/2] remoteproc: stm32: add support of detaching a remote processor Arnaud Pouliquen 2021-03-18 14:59 ` [PATCH 1/2] dt-bindings: remoteproc: stm32-rproc: add new mailbox channel for detach Arnaud Pouliquen @ 2021-03-18 14:59 ` Arnaud Pouliquen 2021-04-13 20:51 ` Bjorn Andersson 1 sibling, 1 reply; 5+ messages in thread From: Arnaud Pouliquen @ 2021-03-18 14:59 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] 5+ messages in thread
* Re: [PATCH 2/2] remoteproc: stm32: add capability to detach 2021-03-18 14:59 ` [PATCH 2/2] remoteproc: stm32: add capability to detach Arnaud Pouliquen @ 2021-04-13 20:51 ` Bjorn Andersson 0 siblings, 0 replies; 5+ messages in thread From: Bjorn Andersson @ 2021-04-13 20:51 UTC (permalink / raw) To: Arnaud Pouliquen Cc: Ohad Ben-Cohen, Mathieu Poirier, Rob Herring, Alexandre Torgue, devicetree, linux-stm32, linux-arm-kernel, linux-remoteproc, linux-kernel On Thu 18 Mar 09:59 CDT 2021, 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. > > 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); Isn't it the stm32_ipcc driver on the other side of this call? In which case I believe "data" is ignored, and you would be able to just pass NULL here. As long as "data" isn't dereferenced it's probably better to send some bugus value, than an address to this variable on the stack. If on the other hand you pair this with one of the mailbox drivers that dereferences "data", you should initialize it... Apart from this, I think the patch looks good! Regards, Bjorn > + 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 [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-04-13 20:51 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-18 14:59 [PATCH 0/2] remoteproc: stm32: add support of detaching a remote processor Arnaud Pouliquen 2021-03-18 14:59 ` [PATCH 1/2] dt-bindings: remoteproc: stm32-rproc: add new mailbox channel for detach Arnaud Pouliquen 2021-03-19 21:49 ` Rob Herring 2021-03-18 14:59 ` [PATCH 2/2] remoteproc: stm32: add capability to detach Arnaud Pouliquen 2021-04-13 20:51 ` Bjorn Andersson
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).