On Wed, Aug 23, 2017 at 01:53:17PM -0700, Paul Burton wrote:
> Many platforms implement variations upon the same theme of hanging the
> CPU using an infinite loop in their _machine_halt, _machine_restart or
> pm_power_off callbacks. Our generic machine_halt(), machine_restart() &
> machine_power_off() functions will do this for us if the platform
> doesn't specify the appropriate callback or the callback function
> returns, so there's no need for each platform to implement the same
> thing.
>
> This patch removes many platforms implementations of this functionality,
> leaving it to the generic code to handle.
>
> Signed-off-by: Paul Burton <paul.burton@xxxxxxxxxx>
> Cc: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
> Cc: linux-mips@xxxxxxxxxxxxxx
This doesn't apply cleanly due to removal of lantiq/xway/reset.c.
Any reason not to remove ip27_machine_power_off() also? I guess not the
noreturn in ip27_machine_halt() due to the SMP management it does.
Should stop_this_cpu() do something more efficient too?
We do have to be careful about these callbacks disabling IRQs and
returning, on SMP platforms at least. smp_call_function() says not to
call it with IRQs disabled. Perhaps generic code should warn in the SMP
#ifdef if interrupts have been disabled by the platform callbacks before
doing the SMP call, though of course for half of them it might never
reach that point.
> diff --git a/arch/mips/alchemy/board-gpr.c b/arch/mips/alchemy/board-gpr.c
> index 6fb6b3faa158..218d1255a4cb 100644
> --- a/arch/mips/alchemy/board-gpr.c
> +++ b/arch/mips/alchemy/board-gpr.c
> @@ -80,18 +80,10 @@ static void gpr_reset(char *c)
> cpu_wait();
> }
>
> -static void gpr_power_off(void)
> -{
> - while (1)
> - cpu_wait();
> -}
> -
Presumably the loop in gpr_reset() could go too, so it falls back to
generic code? (I can't see any sign of SMP support for alchemy).
> diff --git a/arch/mips/ath79/setup.c b/arch/mips/ath79/setup.c
> index f206dafbb0a3..ce28428cf256 100644
> --- a/arch/mips/ath79/setup.c
> +++ b/arch/mips/ath79/setup.c
> @@ -46,12 +46,6 @@ static void ath79_restart(char *command)
> cpu_wait();
> }
>
> -static void ath79_halt(void)
> -{
> - while (1)
> - cpu_wait();
> -}
The ath79_restart could presumably fall back to generic too?
> diff --git a/arch/mips/bcm47xx/setup.c b/arch/mips/bcm47xx/setup.c
> index 6054d49e608e..f7ab6b4085ad 100644
> --- a/arch/mips/bcm47xx/setup.c
> +++ b/arch/mips/bcm47xx/setup.c
> @@ -77,8 +77,6 @@ static void bcm47xx_machine_restart(char *command)
> break;
> #endif
> }
> - while (1)
> - cpu_relax();
> }
>
> static void bcm47xx_machine_halt(void)
> @@ -97,8 +95,6 @@ static void bcm47xx_machine_halt(void)
> break;
> #endif
> }
> - while (1)
> - cpu_relax();
> }
These do disable interrupts, but no SMP that I can see so I suppose its
safe.
>
> #ifdef CONFIG_BCM47XX_SSB
> diff --git a/arch/mips/bcm63xx/setup.c b/arch/mips/bcm63xx/setup.c
> index 2be9caaa2085..43a9617a19af 100644
> --- a/arch/mips/bcm63xx/setup.c
> +++ b/arch/mips/bcm63xx/setup.c
> @@ -22,13 +22,6 @@
> #include <bcm63xx_io.h>
> #include <bcm63xx_gpio.h>
>
> -void bcm63xx_machine_halt(void)
There's a declaration of this in
arch/mips/include/asm/mach-bcm63xx/bcm63xx_cpu.h that can be removed
now too.
> -{
> - pr_info("System halted\n");
> - while (1)
> - ;
> -}
> -
> static void bcm6348_a1_reboot(void)
> {
> u32 reg;
> @@ -148,9 +141,7 @@ void __init plat_mem_setup(void)
> {
> add_memory_region(0, bcm63xx_get_memory_size(), BOOT_MEM_RAM);
>
> - _machine_halt = bcm63xx_machine_halt;
> _machine_restart = __bcm63xx_machine_reboot;
Worth bcm63xx_machine_reboot()'s fallback loop falling back to generic?
It seems to support SMP, but it doesn't disable IRQs.
> diff --git a/arch/mips/cobalt/reset.c b/arch/mips/cobalt/reset.c
> index 4eedd481dd00..727f139ed460 100644
> --- a/arch/mips/cobalt/reset.c
> +++ b/arch/mips/cobalt/reset.c
> @@ -37,10 +37,6 @@ void cobalt_machine_halt(void)
> led_trigger_event(power_off_led_trigger, LED_FULL);
>
> local_irq_disable();
> - while (1) {
> - if (cpu_wait)
> - cpu_wait();
> - }
The local_irq_disable() could be dropped to.
> diff --git a/arch/mips/lasat/reset.c b/arch/mips/lasat/reset.c
> index e21f0b9a586e..d4a5d5b787a9 100644
> --- a/arch/mips/lasat/reset.c
> +++ b/arch/mips/lasat/reset.c
> @@ -49,7 +49,6 @@ static void lasat_machine_halt(void)
> local_irq_disable();
>
> prom_monitor();
> - for (;;) ;
same for lasat_machine_restart?
(no SMP here either AFAICT)
> diff --git a/arch/mips/loongson64/common/reset.c b/arch/mips/loongson64/common/reset.c
> index a60715e11306..ec290392c175 100644
> --- a/arch/mips/loongson64/common/reset.c
> +++ b/arch/mips/loongson64/common/reset.c
> @@ -48,10 +48,6 @@ static void loongson_restart(char *command)
> void (*fw_restart)(void) = (void *)loongson_sysconf.restart_addr;
>
> fw_restart();
> - while (1) {
> - if (cpu_wait)
> - cpu_wait();
> - }
Loongson64 does support SMP. If fw_restart() disabled IRQs before
returning that could be a problem?
But maybe it'd be a problem for it to return at all...
> @@ -64,26 +60,12 @@ static void loongson_poweroff(void)
> void (*fw_poweroff)(void) = (void *)loongson_sysconf.poweroff_addr;
>
> fw_poweroff();
> - while (1) {
> - if (cpu_wait)
> - cpu_wait();
> - }
same?
> diff --git a/arch/mips/sgi-ip22/ip22-reset.c b/arch/mips/sgi-ip22/ip22-reset.c
> index 03a39ac5ead9..ce1f435f31de 100644
> --- a/arch/mips/sgi-ip22/ip22-reset.c
> +++ b/arch/mips/sgi-ip22/ip22-reset.c
> @@ -71,7 +71,6 @@ static void __noreturn sgi_machine_restart(char *command)
> if (machine_state & MACHINE_SHUTTING_DOWN)
> sgi_machine_power_off();
> sgimc->cpuctrl0 |= SGIMC_CCTRL0_SYSINIT;
> - while (1);
Don't forget to drop the __noreturn (I haven't checked this on other
paths).
> diff --git a/arch/mips/txx9/generic/setup.c b/arch/mips/txx9/generic/setup.c
> index 1791a44ee570..c58ab1bdd3e5 100644
> --- a/arch/mips/txx9/generic/setup.c
> +++ b/arch/mips/txx9/generic/setup.c
> @@ -370,25 +370,6 @@ const char *__init prom_getenv(const char *name)
> return NULL;
> }
>
> -static void __noreturn txx9_machine_halt(void)
> -{
> - local_irq_disable();
> - clear_c0_status(ST0_IM);
> - while (1) {
> - if (cpu_wait) {
> - (*cpu_wait)();
> - if (cpu_has_counter) {
> - /*
> - * Clear counter interrupt while it
> - * breaks WAIT instruction even if
> - * masked.
> - */
> - write_c0_compare(0);
> - }
> - }
> - }
> -}
I think this was used indirectly (via _machine_halt) by
tx4927_machine_restart(), tx4938_machine_restart(),
tx4939_machine_restart(), jmr3927_machine_restart(),
toshiba_rbtx4927_restart(), and rbtx4938_machine_restart() as a fallback
if restart doesn't work.
I suppose those fallbacks should be dropped to avoid calling NULL and it
should just fall through to the generic halt code?
There's also a while loop in rbtx4939_machine_restart(). No SMP here
either apparently.
> diff --git a/arch/mips/vr41xx/common/pmu.c b/arch/mips/vr41xx/common/pmu.c
> index 39a0db3e2b34..65157b686b5c 100644
> --- a/arch/mips/vr41xx/common/pmu.c
> +++ b/arch/mips/vr41xx/common/pmu.c
> @@ -84,7 +84,6 @@ static void vr41xx_restart(char *command)
> {
> local_irq_disable();
> software_reset();
> - while (1) ;
No SMP, so I suppose its fine.
Cheers
James
Attachment:
signature.asc
Description: Digital signature
