syscon: update syscon_node_to_regmap to use the DM functions
PATCH v2 1/1 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).
---
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 patches1f6ca3f42f6edf143473159297f4c515b1cf36f6
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(-)
drivers/core/syscon-uclass.c +17 -38
@@ -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); }
Signature · 1 line
2.7.4
PATCH v2 1/1 syscon: update syscon_node_to_regmap to use the DM functions
Hi Tom,
Quoted 158 lines
From: Patrick DELAUNAY
Sent: mercredi 31 octobre 2018 16:49Subject: [PATCH v2] syscon: update
syscon_node_to_regmapto 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 listsyscon_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 <[email protected]>---
HiThis patch is a modified (after Simon Glass review) and rebased version on v2018.11-rc3 for [U-Boot] syscon: reset node list
syscon_listafter 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 printingWith the first patch the
syscon_node_to_regmap()is called each time that the classsysreset_sysconis probed.=> in v2018.09 probe is done only when reset is requested
NB: for stm32mp1, the node
rcc_rebootin 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 phasisAnd when I execute the reset command
->do_reset()
-->reset_cpu()
--->sysreset_walk_halt(SYSRESET_COLD);
----> loop on deviceUCLASS_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(-)drivers/core/syscon-uclass.c index renamed from drivers/core/syscon-uclass.c +0
-0303e166..3864f13 100644
drivers/core/syscon-uclass.c +17
-38@@ -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); } --
Quoted 1 line
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
PATCH v2 1/1 syscon: update syscon_node_to_regmap to use the DM functions
In reply to Patrick DelaunayQuoted 9 lines · Patrick Delaunay
+ 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 listsyscon_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 <[email protected]>
Applied to u-boot/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181217/4e6cabb5/attachment.sig>