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

Harald Seiler hws at denx.de
Tue Mar 2 12:35:40 CET 2021


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.

-- 
Harald

> Regards,
>     Harald
> 
> Harald Seiler (4):
>   nds32: Remove dead reset_cpu() implementation
>   board: ns3: Remove superfluous reset logic
>   Revert "lpc32xx: cpu: add support for soft reset"
>   reset: Remove addr parameter from reset_cpu()
> 
>  arch/arc/lib/reset.c                          |  4 ++--
>  arch/arm/cpu/arm920t/ep93xx/cpu.c             |  2 +-
>  arch/arm/cpu/arm920t/imx/timer.c              |  2 +-
>  arch/arm/cpu/arm926ejs/armada100/timer.c      |  2 +-
>  arch/arm/cpu/arm926ejs/mx25/reset.c           |  2 +-
>  arch/arm/cpu/arm926ejs/mx27/reset.c           |  2 +-
>  arch/arm/cpu/arm926ejs/mxs/mxs.c              |  4 ++--
>  arch/arm/cpu/arm926ejs/spear/reset.c          |  2 +-
>  arch/arm/cpu/arm946es/cpu.c                   |  2 +-
>  arch/arm/cpu/armv7/bcm281xx/reset.c           |  2 +-
>  arch/arm/cpu/armv7/bcmcygnus/reset.c          |  2 +-
>  arch/arm/cpu/armv7/bcmnsp/reset.c             |  2 +-
>  arch/arm/cpu/armv7/ls102xa/cpu.c              |  2 +-
>  arch/arm/cpu/armv7/s5p4418/cpu.c              |  2 +-
>  arch/arm/cpu/armv7/stv0991/reset.c            |  2 +-
>  arch/arm/cpu/armv7m/cpu.c                     |  2 +-
>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c       |  4 ++--
>  arch/arm/cpu/armv8/s32v234/generic.c          |  2 +-
>  arch/arm/cpu/pxa/pxa2xx.c                     |  4 ++--
>  arch/arm/cpu/sa1100/cpu.c                     |  2 +-
>  arch/arm/lib/interrupts.c                     |  2 +-
>  arch/arm/lib/interrupts_m.c                   |  2 +-
>  arch/arm/lib/reset.c                          |  2 +-
>  arch/arm/mach-at91/arm920t/reset.c            |  2 +-
>  arch/arm/mach-at91/arm926ejs/reset.c          |  2 +-
>  arch/arm/mach-at91/armv7/reset.c              |  2 +-
>  arch/arm/mach-bcm283x/reset.c                 |  2 +-
>  arch/arm/mach-davinci/reset.c                 |  2 +-
>  arch/arm/mach-exynos/soc.c                    |  2 +-
>  arch/arm/mach-imx/imx8m/soc.c                 |  2 +-
>  arch/arm/mach-imx/mx7ulp/soc.c                |  2 +-
>  arch/arm/mach-k3/common.c                     |  2 +-
>  arch/arm/mach-keystone/ddr3.c                 |  4 ++--
>  arch/arm/mach-keystone/init.c                 |  2 +-
>  arch/arm/mach-kirkwood/cpu.c                  |  2 +-
>  arch/arm/mach-lpc32xx/cpu.c                   | 23 +++++-------------
>  arch/arm/mach-mediatek/mt7622/init.c          |  2 +-
>  arch/arm/mach-mediatek/mt8512/init.c          |  2 +-
>  arch/arm/mach-mediatek/mt8516/init.c          |  2 +-
>  arch/arm/mach-mediatek/mt8518/init.c          |  2 +-
>  arch/arm/mach-meson/board-common.c            |  4 ++--
>  arch/arm/mach-mvebu/armada3700/cpu.c          |  2 +-
>  arch/arm/mach-mvebu/armada8k/cpu.c            |  2 +-
>  arch/arm/mach-mvebu/cpu.c                     |  2 +-
>  arch/arm/mach-octeontx/cpu.c                  |  2 +-
>  arch/arm/mach-octeontx2/cpu.c                 |  2 +-
>  arch/arm/mach-omap2/omap5/hwinit.c            |  2 +-
>  arch/arm/mach-omap2/reset.c                   |  2 +-
>  arch/arm/mach-orion5x/cpu.c                   |  2 +-
>  arch/arm/mach-owl/soc.c                       |  2 +-
>  .../mach-socfpga/include/mach/reset_manager.h |  2 +-
>  arch/arm/mach-sunxi/board.c                   |  2 +-
>  arch/arm/mach-tegra/cmd_enterrcm.c            |  2 +-
>  arch/arm/mach-tegra/pmc.c                     |  2 +-
>  arch/arm/mach-uniphier/arm32/psci.c           |  2 +-
>  arch/arm/mach-uniphier/reset.c                |  2 +-
>  arch/arm/mach-zynq/cpu.c                      |  2 +-
>  arch/arm/mach-zynqmp-r5/cpu.c                 |  2 +-
>  arch/nds32/cpu/n1213/ag101/cpu.c              |  2 +-
>  arch/nds32/cpu/n1213/start.S                  | 22 -----------------
>  arch/nds32/lib/interrupts.c                   |  2 +-
>  arch/sandbox/cpu/sdl.c                        |  4 ++--
>  arch/sh/cpu/sh4/cpu.c                         |  2 +-
>  arch/sh/cpu/sh4/watchdog.c                    |  2 +-
>  arch/x86/cpu/ivybridge/cpu.c                  |  2 +-
>  board/BuR/brppt2/board.c                      |  2 +-
>  board/abilis/tb100/tb100.c                    |  2 +-
>  .../imx8qm_rom7720_a1/imx8qm_rom7720_a1.c     |  2 +-
>  board/armltd/total_compute/total_compute.c    |  2 +-
>  board/armltd/vexpress/vexpress_common.c       |  2 +-
>  board/armltd/vexpress64/vexpress64.c          |  2 +-
>  .../armadillo-800eva/armadillo-800eva.c       |  2 +-
>  board/beacon/beacon-rzg2m/beacon-rzg2m.c      |  2 +-
>  board/bosch/shc/board.c                       |  2 +-
>  board/broadcom/bcmns2/northstar2.c            |  2 +-
>  board/broadcom/bcmns3/ns3.c                   | 24 +++----------------
>  board/broadcom/bcmstb/bcmstb.c                |  2 +-
>  board/cavium/thunderx/thunderx.c              |  2 +-
>  board/compulab/cm_t335/spl.c                  |  2 +-
>  board/cortina/presidio-asic/presidio.c        |  2 +-
>  board/freescale/imx8qm_mek/imx8qm_mek.c       |  2 +-
>  board/freescale/imx8qxp_mek/imx8qxp_mek.c     |  2 +-
>  board/freescale/mx6memcal/spl.c               |  2 +-
>  board/ge/b1x5v2/spl.c                         |  2 +-
>  board/highbank/highbank.c                     |  2 +-
>  board/hisilicon/hikey/hikey.c                 |  2 +-
>  board/hisilicon/hikey960/hikey960.c           |  2 +-
>  board/hisilicon/poplar/poplar.c               |  2 +-
>  board/kmc/kzm9g/kzm9g.c                       |  2 +-
>  board/liebherr/display5/spl.c                 |  2 +-
>  board/phytium/durian/durian.c                 |  2 +-
>  .../dragonboard410c/dragonboard410c.c         |  2 +-
>  .../dragonboard820c/dragonboard820c.c         |  2 +-
>  board/renesas/alt/alt.c                       |  2 +-
>  board/renesas/alt/alt_spl.c                   |  2 +-
>  board/renesas/blanche/blanche.c               |  2 +-
>  board/renesas/condor/condor.c                 |  2 +-
>  board/renesas/draak/draak.c                   |  2 +-
>  board/renesas/eagle/eagle.c                   |  2 +-
>  board/renesas/ebisu/ebisu.c                   |  2 +-
>  board/renesas/gose/gose.c                     |  2 +-
>  board/renesas/gose/gose_spl.c                 |  2 +-
>  board/renesas/grpeach/grpeach.c               |  2 +-
>  board/renesas/koelsch/koelsch.c               |  2 +-
>  board/renesas/koelsch/koelsch_spl.c           |  2 +-
>  board/renesas/lager/lager.c                   |  2 +-
>  board/renesas/lager/lager_spl.c               |  2 +-
>  board/renesas/porter/porter.c                 |  2 +-
>  board/renesas/porter/porter_spl.c             |  2 +-
>  board/renesas/rcar-common/gen3-spl.c          |  2 +-
>  board/renesas/salvator-x/salvator-x.c         |  2 +-
>  board/renesas/silk/silk.c                     |  2 +-
>  board/renesas/silk/silk_spl.c                 |  2 +-
>  board/renesas/stout/cpld.c                    |  2 +-
>  board/renesas/stout/stout_spl.c               |  2 +-
>  board/siemens/capricorn/board.c               |  2 +-
>  board/synopsys/emsdp/emsdp.c                  |  2 +-
>  board/synopsys/iot_devkit/iot_devkit.c        |  2 +-
>  board/technexion/pico-imx6ul/spl.c            |  2 +-
>  board/technexion/pico-imx7d/spl.c             |  2 +-
>  board/toradex/apalis-imx8/apalis-imx8.c       |  2 +-
>  board/toradex/apalis-imx8x/apalis-imx8x.c     |  2 +-
>  board/toradex/apalis_imx6/apalis_imx6.c       |  2 +-
>  board/toradex/colibri-imx8x/colibri-imx8x.c   |  2 +-
>  board/toradex/colibri_imx6/colibri_imx6.c     |  2 +-
>  board/toradex/colibri_imx7/colibri_imx7.c     |  2 +-
>  board/xen/xenguest_arm64/xenguest_arm64.c     |  2 +-
>  board/xilinx/versal/board.c                   |  2 +-
>  board/xilinx/zynqmp/zynqmp.c                  |  2 +-
>  cmd/tpm_test.c                                |  2 +-
>  drivers/sysreset/sysreset-uclass.c            |  2 +-
>  drivers/watchdog/imx_watchdog.c               |  2 +-
>  drivers/watchdog/ulp_wdog.c                   |  2 +-
>  include/cpu_func.h                            |  4 ++--
>  include/sysreset.h                            |  2 +-
>  135 files changed, 149 insertions(+), 200 deletions(-)
> 



More information about the U-Boot mailing list