2007-08-03 19:26:26

by Adrian McMenamin

[permalink] [raw]
Subject: [PATCH] SH: add machine-ops and Dreamcast specific fix-up

(This code is closely modelled on the i386 machine-ops, though
currently the only board specific fixup is for the Dreamcast and so
there is not a separate set of mach fixups.)

This needs to be tested against a wide range of SH boards and would be
good to go in -mm if Paul acks it.

Add machine-ops code to the SH code base, allowing board specific
reboot and halt code. Currently only Dreamcast specific warm reboot
fixup in code.

Signed-off by: Adrian McMenamin <[email protected]>

diff --git a/arch/sh/boards/dreamcast/Makefile
b/arch/sh/boards/dreamcast/Makefile
index e6fcd3d..7b97546 100644
--- a/arch/sh/boards/dreamcast/Makefile
+++ b/arch/sh/boards/dreamcast/Makefile
@@ -2,5 +2,5 @@
# Makefile for the Sega Dreamcast specific parts of the kernel
#

-obj-y := setup.o irq.o rtc.o reboot.o
+obj-y := setup.o irq.o rtc.o

diff --git a/arch/sh/kernel/Makefile b/arch/sh/kernel/Makefile
index bdb30ba..7ab2359 100644
--- a/arch/sh/kernel/Makefile
+++ b/arch/sh/kernel/Makefile
@@ -5,15 +5,11 @@
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 \
+ reboot.o semaphore.o setup.o signal.o sys_sh.o syscalls.o \
time.o topology.o traps.o

obj-y += cpu/ timers/

-ifneq ($(CONFIG_SH_DREAMCAST),y)
- obj-y += reboot.o
-endif
-
obj-$(CONFIG_VSYSCALL) += vsyscall/

obj-$(CONFIG_SMP) += smp.o
diff --git a/arch/sh/kernel/machine_kexec.c b/arch/sh/kernel/machine_kexec.c
index 790ed69..201b370 100644
--- a/arch/sh/kernel/machine_kexec.c
+++ b/arch/sh/kernel/machine_kexec.c
@@ -29,9 +29,6 @@ extern const unsigned char relocate_new_kernel[];
extern const unsigned int relocate_new_kernel_size;
extern void *gdb_vbr_vector;

-void machine_shutdown(void)
-{
-}

void machine_crash_shutdown(struct pt_regs *regs)
{
diff --git a/arch/sh/kernel/reboot.c b/arch/sh/kernel/reboot.c
new file mode 100644
index 0000000..c6247ae
--- /dev/null
+++ b/arch/sh/kernel/reboot.c
@@ -0,0 +1,107 @@
+/*
+ * linux/arch/sh/kernel/reboot.c
+ *
+ * Essentially copied from i386 code
+ * and process.c
+ *
+ * Copyright (C) 1995 Linus Torvalds
+ *
+ * SuperH version: Copyright (C) 1999, 2000 Niibe Yutaka & Kaz Kojima
+ * Copyright (C) 2006 Lineo Solutions Inc. support SH4A UBC
+ * Copyright (C) 2002 - 2007 Paul Mundt
+ * Copyright 2007 Adrian McMenamin
+ *
+ * Licensed under the terms of version 2 of GNU GPL
+ */
+
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <asm/io.h>
+#include <asm/reboot.h>
+
+/*
+ * Power off function, if any
+ */
+void (*pm_power_off)(void);
+EXPORT_SYMBOL(pm_power_off);
+
+/* fixups for individual machines -
+ * in i386 this is in a file of its own
+ * but we only have the Dreamcast for now.
+ */
+
+static void mach_reboot_fixups(void)
+{
+ if (mach_is_dreamcast()) {
+ writel(0x00007611, 0xA05F6890);
+ }
+}
+
+static void native_machine_shutdown(void)
+{
+}
+
+static void native_machine_emergency_restart(void)
+{
+ mach_reboot_fixups();
+ /* SR.BL=1 and invoke address error to let CPU reset (manual reset) */
+ asm volatile("ldc %0, sr\n\t"
+ "mov.l @%1, %0" : : "r" (0x10000000), "r" (0x80000001));
+}
+
+static void native_machine_restart(char * __unused)
+{
+ native_machine_shutdown();
+ native_machine_emergency_restart();
+}
+
+static void native_machine_halt(void)
+{
+ local_irq_disable();
+
+ while (1)
+ cpu_sleep();
+}
+
+static void native_machine_power_off(void)
+{
+ if (pm_power_off) {
+ native_machine_shutdown();
+ pm_power_off();
+ }
+}
+
+struct machine_ops machine_ops = {
+ .power_off = native_machine_power_off,
+ .shutdown = native_machine_shutdown,
+ .emergency_restart = native_machine_emergency_restart,
+ .restart = native_machine_restart,
+ .halt = native_machine_halt,
+};
+
+void machine_power_off(void)
+{
+ machine_ops.power_off();
+}
+
+void machine_shutdown(void)
+{
+ machine_ops.shutdown();
+}
+
+void machine_emergency_restart(void)
+{
+ machine_ops.emergency_restart();
+}
+
+void machine_restart(char *cmd)
+{
+ machine_ops.restart(cmd);
+}
+
+void machine_halt(void)
+{
+ machine_ops.halt();
+}
diff --git a/include/asm-sh/emergency-restart.h
b/include/asm-sh/emergency-restart.h
index 108d8c4..d6bec92 100644
--- a/include/asm-sh/emergency-restart.h
+++ b/include/asm-sh/emergency-restart.h
@@ -1,6 +1,6 @@
-#ifndef _ASM_EMERGENCY_RESTART_H
-#define _ASM_EMERGENCY_RESTART_H
+#ifndef _ASM_SH_EMERGENCY_RESTART_H
+#define _ASM_SH_EMERGENCY_RESTART_H

-#include <asm-generic/emergency-restart.h>
+extern void machine_emergency_restart(void);

-#endif /* _ASM_EMERGENCY_RESTART_H */
+#endif /* _ASM_SH_EMERGENCY_RESTART_H */


2007-08-04 03:07:34

by Paul Mundt

[permalink] [raw]
Subject: Re: [PATCH] SH: add machine-ops and Dreamcast specific fix-up

On Fri, Aug 03, 2007 at 08:26:17PM +0100, Adrian McMenamin wrote:
> diff --git a/arch/sh/boards/dreamcast/Makefile
> b/arch/sh/boards/dreamcast/Makefile
> index e6fcd3d..7b97546 100644
> --- a/arch/sh/boards/dreamcast/Makefile
> +++ b/arch/sh/boards/dreamcast/Makefile
> @@ -2,5 +2,5 @@
> # Makefile for the Sega Dreamcast specific parts of the kernel
> #
>
> -obj-y := setup.o irq.o rtc.o reboot.o
> +obj-y := setup.o irq.o rtc.o
>

You've created this diff against a bogus kernel.

> diff --git a/arch/sh/kernel/Makefile b/arch/sh/kernel/Makefile
> index bdb30ba..7ab2359 100644
> --- a/arch/sh/kernel/Makefile
> +++ b/arch/sh/kernel/Makefile
> @@ -5,15 +5,11 @@
> 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 \
> + reboot.o semaphore.o setup.o signal.o sys_sh.o syscalls.o \
> time.o topology.o traps.o
>
> obj-y += cpu/ timers/
>
> -ifneq ($(CONFIG_SH_DREAMCAST),y)
> - obj-y += reboot.o
> -endif
> -
> obj-$(CONFIG_VSYSCALL) += vsyscall/
>
> obj-$(CONFIG_SMP) += smp.o
> diff --git a/arch/sh/kernel/machine_kexec.c b/arch/sh/kernel/machine_kexec.c
> index 790ed69..201b370 100644
> --- a/arch/sh/kernel/machine_kexec.c
> +++ b/arch/sh/kernel/machine_kexec.c
> @@ -29,9 +29,6 @@ extern const unsigned char relocate_new_kernel[];
> extern const unsigned int relocate_new_kernel_size;
> extern void *gdb_vbr_vector;
>
> -void machine_shutdown(void)
> -{
> -}
>
> void machine_crash_shutdown(struct pt_regs *regs)
> {

You've missed machine_crash_shutdown().

> +static void mach_reboot_fixups(void)
> +{
> + if (mach_is_dreamcast()) {
> + writel(0x00007611, 0xA05F6890);
> + }
> +}
> +
Whether it's only the dreamcast or not is irrelevant, why bother adding
abstraction if you intend to add pointless hacks that completely
side-steps it?

> diff --git a/include/asm-sh/emergency-restart.h
> b/include/asm-sh/emergency-restart.h
> index 108d8c4..d6bec92 100644
> --- a/include/asm-sh/emergency-restart.h
> +++ b/include/asm-sh/emergency-restart.h
> @@ -1,6 +1,6 @@
> -#ifndef _ASM_EMERGENCY_RESTART_H
> -#define _ASM_EMERGENCY_RESTART_H
> +#ifndef _ASM_SH_EMERGENCY_RESTART_H
> +#define _ASM_SH_EMERGENCY_RESTART_H
>
> -#include <asm-generic/emergency-restart.h>
> +extern void machine_emergency_restart(void);
>
> -#endif /* _ASM_EMERGENCY_RESTART_H */
> +#endif /* _ASM_SH_EMERGENCY_RESTART_H */
>
Pointless. Separating out machine_emergency_restart() buys us nothing,
leave this alone and just kill it off from the machine_ops entirely.
You've also ignored my earlier mail where I suggested this and killing
off some of the other ops we had no use for (as well as consolidating
machine_crash_shutdown()). I do wish you would read these things and wait
until there's been a resolution one way or another.

2007-08-04 07:55:13

by Adrian McMenamin

[permalink] [raw]
Subject: Re: [PATCH] SH: add machine-ops and Dreamcast specific fix-up

On Sat, 2007-08-04 at 12:06 +0900, Paul Mundt wrote:
> On Fri, Aug 03, 2007 at 08:26:17PM +0100, Adrian McMenamin wrote:
> > diff --git a/arch/sh/boards/dreamcast/Makefile
> > b/arch/sh/boards/dreamcast/Makefile
> > index e6fcd3d..7b97546 100644
> > --- a/arch/sh/boards/dreamcast/Makefile
> > +++ b/arch/sh/boards/dreamcast/Makefile
> > @@ -2,5 +2,5 @@
> > # Makefile for the Sega Dreamcast specific parts of the kernel
> > #
> >
> > -obj-y := setup.o irq.o rtc.o reboot.o
> > +obj-y := setup.o irq.o rtc.o
> >
>
> You've created this diff against a bogus kernel.

True. My apologies - I missed that.

>
> > diff --git a/arch/sh/kernel/Makefile b/arch/sh/kernel/Makefile
> > index bdb30ba..7ab2359 100644
> > --- a/arch/sh/kernel/Makefile
> > +++ b/arch/sh/kernel/Makefile

....

> >
> > void machine_crash_shutdown(struct pt_regs *regs)
> > {
>
> You've missed machine_crash_shutdown().
>

Apologies - though I am a bit puzzled as why the code compiled and
linked with the two definitions, but that's not much of an excuse.


> > +static void mach_reboot_fixups(void)
> > +{
> > + if (mach_is_dreamcast()) {
> > + writel(0x00007611, 0xA05F6890);
> > + }
> > +}
> > +
> Whether it's only the dreamcast or not is irrelevant, why bother adding
> abstraction if you intend to add pointless hacks that completely
> side-steps it?
>

I don't understand the point you are trying to make. Please explain with
more clarity. What have I completely side stepped? I have followed,
broadly, the same pattern used in i386. Just that there, afaics, they
pick up on various PCI cards as the basis on which to modify the reboot.


> > diff --git a/include/asm-sh/emergency-restart.h
> > b/include/asm-sh/emergency-restart.h
> > index 108d8c4..d6bec92 100644
> > --- a/include/asm-sh/emergency-restart.h
> > +++ b/include/asm-sh/emergency-restart.h
> > @@ -1,6 +1,6 @@
> > -#ifndef _ASM_EMERGENCY_RESTART_H
> > -#define _ASM_EMERGENCY_RESTART_H
> > +#ifndef _ASM_SH_EMERGENCY_RESTART_H
> > +#define _ASM_SH_EMERGENCY_RESTART_H
> >
> > -#include <asm-generic/emergency-restart.h>
> > +extern void machine_emergency_restart(void);
> >
> > -#endif /* _ASM_EMERGENCY_RESTART_H */
> > +#endif /* _ASM_SH_EMERGENCY_RESTART_H */
> >
> Pointless. Separating out machine_emergency_restart() buys us nothing,
> leave this alone and just kill it off from the machine_ops entirely.
> You've also ignored my earlier mail where I suggested this and killing
> off some of the other ops we had no use for (as well as consolidating
> machine_crash_shutdown()). I do wish you would read these things and wait
> until there's been a resolution one way or another.

I haven't ignored it. It was just explained with your customary
clarity :)


2007-08-04 12:51:39

by Paul Mundt

[permalink] [raw]
Subject: Re: [PATCH] SH: add machine-ops and Dreamcast specific fix-up

On Sat, Aug 04, 2007 at 08:54:36AM +0100, Adrian McMenamin wrote:
> On Sat, 2007-08-04 at 12:06 +0900, Paul Mundt wrote:
> > On Fri, Aug 03, 2007 at 08:26:17PM +0100, Adrian McMenamin wrote:
> > > +static void mach_reboot_fixups(void)
> > > +{
> > > + if (mach_is_dreamcast()) {
> > > + writel(0x00007611, 0xA05F6890);
> > > + }
> > > +}
> > > +
> > Whether it's only the dreamcast or not is irrelevant, why bother adding
> > abstraction if you intend to add pointless hacks that completely
> > side-steps it?
> >
>
> I don't understand the point you are trying to make. Please explain with
> more clarity. What have I completely side stepped? I have followed,
> broadly, the same pattern used in i386. Just that there, afaics, they
> pick up on various PCI cards as the basis on which to modify the reboot.
>
You've introduced infrastructure to permit different machine types to
provide their own reboot hooks, and instead of actually providing a
machine-specific implementation, you've just hacked the native
implementation ith machine-type checks. This is conceptually no different
from your previous hack of sprinkling Dreamcast hooks in process.c.

The entire point of this abstraction is so that you can push what logic
you need down in to your board directory and _not_ have to shove this
sort of pointless crap in to the common code.

> > > diff --git a/include/asm-sh/emergency-restart.h
> > > b/include/asm-sh/emergency-restart.h
> > > index 108d8c4..d6bec92 100644
> > > --- a/include/asm-sh/emergency-restart.h
> > > +++ b/include/asm-sh/emergency-restart.h
> > > @@ -1,6 +1,6 @@
> > > -#ifndef _ASM_EMERGENCY_RESTART_H
> > > -#define _ASM_EMERGENCY_RESTART_H
> > > +#ifndef _ASM_SH_EMERGENCY_RESTART_H
> > > +#define _ASM_SH_EMERGENCY_RESTART_H
> > >
> > > -#include <asm-generic/emergency-restart.h>
> > > +extern void machine_emergency_restart(void);
> > >
> > > -#endif /* _ASM_EMERGENCY_RESTART_H */
> > > +#endif /* _ASM_SH_EMERGENCY_RESTART_H */
> > >
> > Pointless. Separating out machine_emergency_restart() buys us nothing,
> > leave this alone and just kill it off from the machine_ops entirely.
> > You've also ignored my earlier mail where I suggested this and killing
> > off some of the other ops we had no use for (as well as consolidating
> > machine_crash_shutdown()). I do wish you would read these things and wait
> > until there's been a resolution one way or another.
>
> I haven't ignored it. It was just explained with your customary
> clarity :)

If there was something you were unclear about, perhaps you should have
asked for clarification instead of ignoring it? I didn't think any of
these points were terribly difficult to parse.