Hi Tom,

From: Patrick DELAUNAY Sent: mercredi 31 octobre 2018 16:49

Subject: [PATCH v2] syscon: update syscon_node_to_regmap to use the DM functions

• Update the function syscon_node_to_regmap() to force bound on syscon uclass and directly use the list of device from DM.
• Remove the static list syscon_list.

This patch avoid issue (crash) when syscon_node_to_regmap() is called before and after reallocation (list content is invalid).

Signed-off-by: Patrick Delaunay patrick.delaunay@st.com

Hi

This patch is a modified (after Simon Glass review) and rebased version on v2018.11-rc3 for [U-Boot] syscon: reset node list syscon_list after relocation http://patchwork.ozlabs.org/patch/983139/

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

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.

This patch solves the issue by using DM list for syscon uclass.

Tested on my board, the initial issue is solved and I check the behavior with dm_dump_all(): the RCC driver is not binded/probed as syscon uclass by default (checked with dm tree) and correctly bounded and probed when it is necessary (before relocation: print_resetinfo() or after relocation for command reset).

Regards

Patrick

Changes in v2:

• minor update after Simon review (remove variable ofnode and add one comment)

drivers/core/syscon-uclass.c | 55 ++++++++++++++------------------------------ 1 file changed, 17 insertions(+), 38 deletions(-)

diff --git a/drivers/core/syscon-uclass.c b/drivers/core/syscon-uclass.c index 303e166..3864f13 100644 --- a/drivers/core/syscon-uclass.c +++ b/drivers/core/syscon-uclass.c @@ -123,52 +123,31 @@ U_BOOT_DRIVER(generic_syscon) = {

• The syscon node can be bound to another driver, but still works
• as a syscon provider.

*/ -static LIST_HEAD(syscon_list);

-struct syscon {

• ofnode node;
• struct regmap *regmap;
• struct list_head list;

-};

-static struct syscon *of_syscon_register(ofnode node) +struct regmap *syscon_node_to_regmap(ofnode node) {

• struct syscon *syscon;

• struct udevice *dev, *parent; int ret;

• if (!uclass_get_device_by_ofnode(UCLASS_SYSCON, node, &dev))

• return syscon_get_regmap(dev);
  

• if (!ofnode_device_is_compatible(node, "syscon")) return ERR_PTR(-EINVAL);

• syscon = malloc(sizeof(*syscon));
• if (!syscon)

• return ERR_PTR(-ENOMEM);
  

• /* bound to driver with same ofnode or to root if not found */
• if (device_find_global_by_ofnode(node, &parent))

• parent = dm_root();
  
  

• ret = regmap_init_mem(node, &syscon->regmap);
• if (ret) {

• free(syscon);
  

• /* force bound to syscon class */
• ret = device_bind_driver_to_node(parent, "syscon",

• 			 ofnode_get_name(node),
  

• 			 node, &dev);
  

• if (ret) return ERR_PTR(ret);

• }
• list_add_tail(&syscon->list, &syscon_list);
• return syscon;

-}

-struct regmap *syscon_node_to_regmap(ofnode node) -{

• struct syscon *entry, *syscon = NULL;
• list_for_each_entry(entry, &syscon_list, list)

• if (ofnode_equal(entry->node, node)) {
  

• 	syscon = entry;
  

• 	break;
  

• }
  

• if (!syscon)

• syscon = of_syscon_register(node);
  

• if (IS_ERR(syscon))

• return ERR_CAST(syscon);
  

• ret = device_probe(dev);
• if (ret)

• return ERR_PTR(ret);
  
  

• return syscon->regmap;

• return syscon_get_regmap(dev);

}

2.7.4

Juste a little reminder because I don't see this patch in v2019.01-rc1. http://patchwork.ozlabs.org/patch/991519/

That fix a regression on stm32mp1 board detected lately since v2018.11-rc1. It could also impact the other boards using CONFIG_SYSRESET_SYSCON and CONFIG_SYSRESET.

Tell me if I can do anything on my side, you need v3 with rebase.

Patrick