[U-Boot] [PATCH] syscon: update syscon_node_to_regmap to use the DM functions

[Patrick DELAUNAY]

Hi Tom

> From: Tom Rini <trini at konsulko.com>
> Sent: mardi 30 octobre 2018 14:52
> Subject: Re: [PATCH] syscon: update syscon_node_to_regmap to use the DM
> functions
> 
> On Tue, Oct 30, 2018 at 02:44:29PM +0100, Patrick Delaunay wrote:
> > + 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 at 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).
> 
> So this is a regression we should fix, to be clear, yes?  Thanks!

Yes it is a regression.
It could also impact the boards using CONFIG_SYSRESET_SYSCON and CONFIG_SYSRESET.

Ragards

> --
> Tom

Patrick