[PATCH 0/4] Remove addr parameter from reset_cpu()

Simon Glass sjg at chromium.org
Fri Mar 5 05:08:36 CET 2021


Hi Harald,

On Tue, 2 Mar 2021 at 06:35, Harald Seiler <hws at 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


More information about the U-Boot mailing list