Re: [U-Boot] [PATCH 4/5] stm32mp1: Fixup the Linux DeviceTree with coprocessor information
Hi Suman,
Thank you for your comments.
On 11/10/2019 9:57 PM, Suman Anna wrote:
> Hi Fabien,
>
> On 10/9/19 10:36 AM, Fabien Dessenne wrote:
>> When the coprocessor has been started, provide the context to Linux
>> kernel so it can handle it:
>> - update the coprocessor node of kernel DeviceTree with the
>>"early-booted" property.
> Has this property been acked by DT maintainers at the kernel-level?
> We have used something similar but moving away from it and instead just
> relying on reading the hardware reset status in the kernel remoteproc
> driver, and configuring the driver accordingly.
Good point, that "early-booted" property has not been reviewed by the
kernel DT maintainers.
This property shall be read by some kernel remoteproc drivers, together
with the kernel "remoteproc: add support for preloaded firmware"
patchset [1] which is still under review/rework (I expect Loic to send
an update version (very) soon).
I will nevertheless rework this u-boot patchset in order to get rid of
that property. Instead of, I plan to use some additional HW register to
track the processor state.
[1] https://lkml.org/lkml/2018/11/30/157
>
>> - write the resource table address in a dedicated backup register.
> Is this an actual register on the SoC or some block of memory dedicated
> for sharing information from U-Boot to kernel?
>
> We have the same problem and I am currently going with a
> design-by-contract approach for early-booted usecases where I am
> expecting the resource table to be placed at a specific location in
> memory regions given to remoteproc.
I am interested in having the resource table placed in a specific
remoteproc memory region.
For the time being I write the RscTable in a backup register which is an
actual SoC register. When the "specific memory region" dev is available,
I may consider moving to that alternate implementation.
BR
Fabien
>
> regards
> Suman
>
>> Signed-off-by: Fabien Dessenne
>> ---
>> board/st/stm32mp1/stm32mp1.c | 16 +---
>> 1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
>> index 18f9b84..8c669d0 100644
>> --- a/board/st/stm32mp1/stm32mp1.c
>> +++ b/board/st/stm32mp1/stm32mp1.c
>> @@ -891,6 +891,7 @@ void board_mtdparts_default(const char **mtdids, const
>> char **mtdparts)
>> #if defined(CONFIG_OF_BOARD_SETUP)
>> int ft_board_setup(void *blob, bd_t *bd)
>> {
>> +int off, id = 0; /* Copro id fixed to 0 as only one coproc on mp1 */
>> #ifdef CONFIG_FDT_FIXUP_PARTITIONS
>> struct node_info nodes[] = {
>> { "st,stm32f469-qspi", MTD_DEV_TYPE_NOR, },
>> @@ -899,6 +900,17 @@ int ft_board_setup(void *blob, bd_t *bd)
>> fdt_fixup_mtdparts(blob, nodes, ARRAY_SIZE(nodes));
>> #endif
>>
>> +/* Update DT if coprocessor started */
>> +off = fdt_path_offset(blob, "/mlahb/m4@1000");
>> +if (off > 0) {
>> +if (!rproc_is_running(id)) {
>> +fdt_setprop_empty(blob, off, "early-booted");
>> +} else {
>> +fdt_delprop(blob, off, "early-booted");
>> +writel(0, TAMP_COPRO_RSC_TBL_ADDRESS);
>> +}
>> +}
>> +
>> return 0;
>> }
>> #endif
>> @@ -918,10 +930,8 @@ static void board_copro_image_process(ulong fw_image,
>> size_t fw_size)
>> printf("Load Remote Processor %d with data@addr=0x%08lx %u bytes:%s\n",
>> id, fw_image, fw_size, ret ? " Failed!" : " Success!");
>>
>> -if (!ret) {
>> +if (!ret)
>> rproc_start(id);
>> -env_set("copro_state", "booted");
>> -}
>> }
>>
>> U_BOOT_FIT_LOADABLE_HANDLER(IH_TYPE_COPRO, board_copro_image_process);
>>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 4/5] stm32mp1: Fixup the Linux DeviceTree with coprocessor information
Hi Fabien,
On 10/9/19 10:36 AM, Fabien Dessenne wrote:
> When the coprocessor has been started, provide the context to Linux
> kernel so it can handle it:
> - update the coprocessor node of kernel DeviceTree with the
> "early-booted" property.
Has this property been acked by DT maintainers at the kernel-level?
We have used something similar but moving away from it and instead just
relying on reading the hardware reset status in the kernel remoteproc
driver, and configuring the driver accordingly.
> - write the resource table address in a dedicated backup register.
Is this an actual register on the SoC or some block of memory dedicated
for sharing information from U-Boot to kernel?
We have the same problem and I am currently going with a
design-by-contract approach for early-booted usecases where I am
expecting the resource table to be placed at a specific location in
memory regions given to remoteproc.
regards
Suman
>
> Signed-off-by: Fabien Dessenne
> ---
> board/st/stm32mp1/stm32mp1.c | 16 +---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index 18f9b84..8c669d0 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -891,6 +891,7 @@ void board_mtdparts_default(const char **mtdids, const
> char **mtdparts)
> #if defined(CONFIG_OF_BOARD_SETUP)
> int ft_board_setup(void *blob, bd_t *bd)
> {
> + int off, id = 0; /* Copro id fixed to 0 as only one coproc on mp1 */
> #ifdef CONFIG_FDT_FIXUP_PARTITIONS
> struct node_info nodes[] = {
> { "st,stm32f469-qspi", MTD_DEV_TYPE_NOR, },
> @@ -899,6 +900,17 @@ int ft_board_setup(void *blob, bd_t *bd)
> fdt_fixup_mtdparts(blob, nodes, ARRAY_SIZE(nodes));
> #endif
>
> + /* Update DT if coprocessor started */
> + off = fdt_path_offset(blob, "/mlahb/m4@1000");
> + if (off > 0) {
> + if (!rproc_is_running(id)) {
> + fdt_setprop_empty(blob, off, "early-booted");
> + } else {
> + fdt_delprop(blob, off, "early-booted");
> + writel(0, TAMP_COPRO_RSC_TBL_ADDRESS);
> + }
> + }
> +
> return 0;
> }
> #endif
> @@ -918,10 +930,8 @@ static void board_copro_image_process(ulong fw_image,
> size_t fw_size)
> printf("Load Remote Processor %d with data@addr=0x%08lx %u bytes:%s\n",
> id, fw_image, fw_size, ret ? " Failed!" : " Success!");
>
> - if (!ret) {
> + if (!ret)
> rproc_start(id);
> - env_set("copro_state", "booted");
> - }
> }
>
> U_BOOT_FIT_LOADABLE_HANDLER(IH_TYPE_COPRO, board_copro_image_process);
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 4/5] stm32mp1: Fixup the Linux DeviceTree with coprocessor information
When the coprocessor has been started, provide the context to Linux
kernel so it can handle it:
- update the coprocessor node of kernel DeviceTree with the
"early-booted" property.
- write the resource table address in a dedicated backup register.
Signed-off-by: Fabien Dessenne
---
board/st/stm32mp1/stm32mp1.c | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
index 18f9b84..8c669d0 100644
--- a/board/st/stm32mp1/stm32mp1.c
+++ b/board/st/stm32mp1/stm32mp1.c
@@ -891,6 +891,7 @@ void board_mtdparts_default(const char **mtdids, const char
**mtdparts)
#if defined(CONFIG_OF_BOARD_SETUP)
int ft_board_setup(void *blob, bd_t *bd)
{
+ int off, id = 0; /* Copro id fixed to 0 as only one coproc on mp1 */
#ifdef CONFIG_FDT_FIXUP_PARTITIONS
struct node_info nodes[] = {
{ "st,stm32f469-qspi", MTD_DEV_TYPE_NOR, },
@@ -899,6 +900,17 @@ int ft_board_setup(void *blob, bd_t *bd)
fdt_fixup_mtdparts(blob, nodes, ARRAY_SIZE(nodes));
#endif
+ /* Update DT if coprocessor started */
+ off = fdt_path_offset(blob, "/mlahb/m4@1000");
+ if (off > 0) {
+ if (!rproc_is_running(id)) {
+ fdt_setprop_empty(blob, off, "early-booted");
+ } else {
+ fdt_delprop(blob, off, "early-booted");
+ writel(0, TAMP_COPRO_RSC_TBL_ADDRESS);
+ }
+ }
+
return 0;
}
#endif
@@ -918,10 +930,8 @@ static void board_copro_image_process(ulong fw_image,
size_t fw_size)
printf("Load Remote Processor %d with data@addr=0x%08lx %u bytes:%s\n",
id, fw_image, fw_size, ret ? " Failed!" : " Success!");
- if (!ret) {
+ if (!ret)
rproc_start(id);
- env_set("copro_state", "booted");
- }
}
U_BOOT_FIT_LOADABLE_HANDLER(IH_TYPE_COPRO, board_copro_image_process);
--
2.7.4
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
