Hi Harald,
On Tue, 2 Mar 2021 at 06:35, Harald Seiler hws@denx.de wrote:
Hi,
On Tue, 2020-12-15 at 16:47 +0100, Harald Seiler wrote:
Hi,
this is something I had on my mind for a longer time but never got around to actually do until now ... A while back, while working on the patchset that led to commit c5635a032a4b ("ARM: imx8m: Don't use the addr parameter of reset_cpu()"), I noticed that the `addr` parameter of reset_cpu() seems to not actually hold any meaningful value. All call-sites in the current tree just pass 0 and the vast majority of reset_cpu() implementations actually ignore the parameter.
I dug a bit deeper to find out why this `addr` parameter exists in the first place and found out that it's mostly a legacy artifact:
Historically, the reset_cpu() function had this `addr` parameter to pass an address of a reset vector location, where the CPU should reset to.The times where this was used are long gone and the only trace it left is some (dead) code for the NDS32 arch. The `addr` parameter lived on and it looks like it was sometimes used as a way to indicate different types of resets (e.g. COLD vs WARM).
Today, however, reset_cpu() is only ever called with `addr` 0 in the mainline tree and as such, any code that gives a meaning to the `addr` value will only ever follow the `addr == 0` branch. This is probably not what the authors intended and as it seems quite unobvious to me, I think the best way forward is to remove the `addr` parameter entirely.
This removes any ambiguity in the "contract" of reset_cpu() and thus hopefully prevents more code being added which wrongly assumes that the parameter can be used for any meaningful purpose. Instead, code which wants to properly support multiple reset types needs to be implemented as a sysreset driver.
I did this API change via a coccinelle patch, see "reset: Remove addr parameter from reset_cpu()" for details. I also ran buildman for all boards I could, to verify that everything still compiles. One notable exception is NDS32 because I couldn't get the compiler to work there ...
I wanted to ask what the current state regarding this patchset is. Is there anything I should still take care of?
I am a bit worried about it going stale if it stays lying around for too long and new call-sites get introduced. So far everything is still fine though, I just applied the patchset to v2021.04-rc3 and ran a worldbuild
- I do not see any builds regressing.
This series seems like a good idea to me.
Regards, Simon