Re: [PATCH] Adding machine-ops to SH arch

[Paul M. <le...@li...>]

On Thu, Aug 02, 2007 at 10:09:51PM +0100, Adrian McMenamin wrote:
> On 02/08/07, Adrian McMenamin <lkm...@gm...> wrote:
> > Paul/list
> >
> > This is my first attempt at this and it seems to work on the
> > Dreamcast. I need to add a set of fixups for the Dreamcast also (at
> > the moment it just replicates the behaviour in the current process.c)
> > - as that is the whole point.
> >
> Apologies, that patch was incomplete and in other ways messed up. This
> should be the proper patch
> 
> diff --git a/arch/sh/kernel/Makefile b/arch/sh/kernel/Makefile
> index 1f141a8..e019ad1 100644
> --- a/arch/sh/kernel/Makefile
> +++ b/arch/sh/kernel/Makefile
> @@ -6,9 +6,10 @@ extra-y	:= head.o init_task.o vmlinux.lds
> 
>  obj-y	:= debugtraps.o io.o io_generic.o irq.o machvec.o process.o ptrace.o \
>  	   semaphore.o setup.o signal.o sys_sh.o syscalls.o \
> -	   time.o topology.o traps.o
> +	   time.o topology.o traps.o reboot.o
> 
And here I was happy to have kept that alphabetized for so many years ;-)

> +void machine_emergency_restart(void)
> +{
> +        machine_ops.emergency_restart();
> +}
> +
Some whitespace damage here.

> +void machine_restart(char *cmd)
> +{
> +	printk(KERN_INFO "Got here\n");
> +	machine_ops.restart(cmd);

Debugging printk().

> --- /dev/null   2007-04-19 22:41:52.000000000 +0100
> +++ include/asm-sh/reboot.h     2007-08-02 00:06:49.000000000 +0100
> @@ -0,0 +1,20 @@
> +#ifndef _ASM_REBOOT_H
> +#define _ASM_REBOOT_H
> +
> +/*straight lift from i386 code*/
> +struct pt_regs;
> +
> +struct machine_ops
> +{
> +       void (*restart)(char *cmd);
> +       void (*halt)(void);
> +       void (*power_off)(void);
> +       void (*shutdown)(void);
> +       void (*crash_shutdown)(struct pt_regs *);
> +       void (*emergency_restart)(void);
> +};
> +
> +extern struct machine_ops machine_ops;
> +
I wonder if we care about shutdown/crash_shutdown() at all. Note that
machine_crash_shutdown() is currently defined by the kexec code, in
arch/sh/kernel/machine_kexec.c.

Can you consolidate the definitions from there in to your reboot.c and
test the build with CONFIG_KEXEC=y?

> +
> +#endif /* _ASM_REBOOT_H */
> diff -ruN /dev/null include/asm-sh/emergency-restart.h
> --- /dev/null   2007-04-19 22:41:52.000000000 +0100
> +++ include/asm-sh/emergency-restart.h  2007-08-02 21:09:09.000000000 +0100
> @@ -0,0 +1,6 @@
> +#ifndef _ASM_GENERIC_EMERGENCY_RESTART_H
> +#define _ASM_GENERIC_EMERGENCY_RESTART_H
> +
> +extern void machine_emergency_restart(void);
> +
> +#endif /* _ASM_GENERIC_EMERGENCY_RESTART_H */
> 
I also wonder if this is worth it. Perhaps we should just consolidate the
native_machine_emergency_restart() logic in native_machine_restart(), and
stick with the asm-generic/emergency_restart.h, we don't requite the cmd
parsing at all, so it's probably not something we care about. This also
allows emergency_restart to be killed off from the machine_ops struct.