[PATCH] syscon: reset node list syscon_list after relocation

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] syscon: reset node list syscon_list after relocation

Patrick DELAUNAY-2
Reset the list head after the reallocation because the list syscon_list
use allocated pointer and they are no more valid.
This patch avoid issue (crash) when syscon_node_to_regmap() is called
before and after reallocation.

Signed-off-by: Patrick Delaunay <[hidden email]>
---
Hi

This patch correct a crash see on v2018.11-rc1 with my board stm32mp1
for the command "reset".

The crash is a side effect of 2 patches
1f6ca3f42f6edf143473159297f4c515b1cf36f6
 sysreset: syscon: update regmap access to syscon

23471aed5c33e104d6fa64575932577618543bec
 board_f: Add reset status printing

With the first patch the syscon_node_to_regmap() is called
each time that the class sysreset_syscon is probed.

=> in v2018.09 probe is done only when reset is requested

NB: for stm32mp1, the node rcc_reboot in tagged pre-relocated to
    support reset in pre-reloc phases (allow panic).

With the second patch, U-Boot probes all the sysreset uclass in preloc
phase to allow to print the reset status, and the list is initialized
in board_f / pre-reloc:

-> print_resetinfo
--> uclass_first_device_err(UCLASS_SYSRESET, &dev);
---> syscon_reboot_probe()
----> syscon_node_to_regmap(node)
-----> of_syscon_register()
-------> list_add_tail(&syscon->list, &syscon_list);

During relocation, the content of syscon_list (static) is updated
but the list use pointers allocated by malloc in pre-reloc phasis

And when I execute the reset command
-> do_reset()
--> reset_cpu()
---> sysreset_walk_halt(SYSRESET_COLD);
----> loop on device UCLASS_SYSRESET
-----> syscon_reboot_probe()
------> syscon_node_to_regmap(node)
-------> list_for_each_entry(entry, &syscon_list, list)

I have a crash here because the pointer syscon_list.next is invalid.

I solve the issue by resetting the list after reloc and it is working
but I don't know if a more elegant solution exist
(keep the list in .bss section to be set to 0).

Regards

Patrick


 drivers/core/syscon-uclass.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/core/syscon-uclass.c b/drivers/core/syscon-uclass.c
index 303e166..aacb43a 100644
--- a/drivers/core/syscon-uclass.c
+++ b/drivers/core/syscon-uclass.c
@@ -156,8 +156,15 @@ static struct syscon *of_syscon_register(ofnode node)
 
 struct regmap *syscon_node_to_regmap(ofnode node)
 {
+ static int reloc_done;
  struct syscon *entry, *syscon = NULL;
 
+ /* content of list is not relocated (malloc): reinit when needed */
+ if (!reloc_done && (gd->flags & GD_FLG_RELOC)) {
+ INIT_LIST_HEAD(&syscon_list);
+ reloc_done++;
+ }
+
  list_for_each_entry(entry, &syscon_list, list)
  if (ofnode_equal(entry->node, node)) {
  syscon = entry;
--
2.7.4

_______________________________________________
U-Boot mailing list
[hidden email]
https://lists.denx.de/listinfo/u-boot