* [PATCH] clean up mach_reboot_fixups
@ 2007-03-14 22:24 Jeremy Fitzhardinge
2007-03-14 22:42 ` Andi Kleen
0 siblings, 1 reply; 6+ messages in thread
From: Jeremy Fitzhardinge @ 2007-03-14 22:24 UTC (permalink / raw)
To: Andi Kleen; +Cc: Linux Kernel Mailing List
The reboot_fixups stuff seems to be a bit of a mess, specifically the
header is in linux/ when its a purely i386-specific piece of code. I'm
not sure why it has its config option; its only currently needed for
"geode-gx1/cs5530a", so perhaps whatever config option controls that
hardware should enable this?
Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
---
arch/i386/kernel/reboot.c | 6 +++++-
arch/i386/kernel/reboot_fixups.c | 2 +-
include/asm-i386/reboot_fixups.h | 6 ++++++
include/linux/reboot_fixups.h | 10 ----------
4 files changed, 12 insertions(+), 12 deletions(-)
===================================================================
--- a/arch/i386/kernel/reboot.c
+++ b/arch/i386/kernel/reboot.c
@@ -17,7 +17,7 @@
#include <asm/apic.h>
#include <asm/desc.h>
#include "mach_reboot.h"
-#include <linux/reboot_fixups.h>
+#include <asm/reboot_fixups.h>
/*
* Power off function, if any
@@ -314,6 +314,10 @@ void machine_shutdown(void)
#ifdef CONFIG_X86_IO_APIC
disable_IO_APIC();
#endif
+}
+
+void __attribute__((weak)) mach_reboot_fixups(void)
+{
}
void machine_emergency_restart(void)
===================================================================
--- a/arch/i386/kernel/reboot_fixups.c
+++ b/arch/i386/kernel/reboot_fixups.c
@@ -10,7 +10,7 @@
#include <asm/delay.h>
#include <linux/pci.h>
-#include <linux/reboot_fixups.h>
+#include <asm/reboot_fixups.h>
static void cs5530a_warm_reset(struct pci_dev *dev)
{
===================================================================
--- /dev/null
+++ b/include/asm-i386/reboot_fixups.h
@@ -0,0 +1,6 @@
+#ifndef _LINUX_REBOOT_FIXUPS_H
+#define _LINUX_REBOOT_FIXUPS_H
+
+extern void mach_reboot_fixups(void);
+
+#endif /* _LINUX_REBOOT_FIXUPS_H */
===================================================================
--- a/include/linux/reboot_fixups.h
+++ /dev/null
@@ -1,10 +0,0 @@
-#ifndef _LINUX_REBOOT_FIXUPS_H
-#define _LINUX_REBOOT_FIXUPS_H
-
-#ifdef CONFIG_X86_REBOOTFIXUPS
-extern void mach_reboot_fixups(void);
-#else
-#define mach_reboot_fixups() ((void)(0))
-#endif
-
-#endif /* _LINUX_REBOOT_FIXUPS_H */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] clean up mach_reboot_fixups
2007-03-14 22:24 [PATCH] clean up mach_reboot_fixups Jeremy Fitzhardinge
@ 2007-03-14 22:42 ` Andi Kleen
2007-03-15 2:18 ` Geode cs5530a magic (Was: Re: [PATCH] clean up mach_reboot_fixups) Jeremy Fitzhardinge
0 siblings, 1 reply; 6+ messages in thread
From: Andi Kleen @ 2007-03-14 22:42 UTC (permalink / raw)
To: Jeremy Fitzhardinge; +Cc: Linux Kernel Mailing List
On Wednesday 14 March 2007 23:24, Jeremy Fitzhardinge wrote:
> The reboot_fixups stuff seems to be a bit of a mess, specifically the
> header is in linux/ when its a purely i386-specific piece of code. I'm
> not sure why it has its config option; its only currently needed for
> "geode-gx1/cs5530a", so perhaps whatever config option controls that
> hardware should enable this?
Thanks. Looks good.
-Andi
^ permalink raw reply [flat|nested] 6+ messages in thread
* Geode cs5530a magic (Was: Re: [PATCH] clean up mach_reboot_fixups)
2007-03-14 22:42 ` Andi Kleen
@ 2007-03-15 2:18 ` Jeremy Fitzhardinge
2007-03-19 14:48 ` Eric W. Biederman
0 siblings, 1 reply; 6+ messages in thread
From: Jeremy Fitzhardinge @ 2007-03-15 2:18 UTC (permalink / raw)
To: Andi Kleen; +Cc: Linux Kernel Mailing List, Alan Cox, Jaya Kumar
Andi Kleen wrote:
> On Wednesday 14 March 2007 23:24, Jeremy Fitzhardinge wrote:
>
>> The reboot_fixups stuff seems to be a bit of a mess, specifically the
>> header is in linux/ when its a purely i386-specific piece of code. I'm
>> not sure why it has its config option; its only currently needed for
>> "geode-gx1/cs5530a", so perhaps whatever config option controls that
>> hardware should enable this?
>>
>
> Thanks. Looks good.
It looks like a cs5530a is a PATA driver in drivers/ata/pata_cs5530.c.
Seems to me the cleanest fix is to register a reboot notifier in the
driver and have it do the magic rather than have the special
mach_reboot_fixups mechanism at all.
Assuming it needs to be done at all...
Alan? Jaya?
J
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Geode cs5530a magic (Was: Re: [PATCH] clean up mach_reboot_fixups)
2007-03-15 2:18 ` Geode cs5530a magic (Was: Re: [PATCH] clean up mach_reboot_fixups) Jeremy Fitzhardinge
@ 2007-03-19 14:48 ` Eric W. Biederman
0 siblings, 0 replies; 6+ messages in thread
From: Eric W. Biederman @ 2007-03-19 14:48 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: Andi Kleen, Linux Kernel Mailing List, Alan Cox, Jaya Kumar
Jeremy Fitzhardinge <jeremy@goop.org> writes:
> Andi Kleen wrote:
>> On Wednesday 14 March 2007 23:24, Jeremy Fitzhardinge wrote:
>>
>>> The reboot_fixups stuff seems to be a bit of a mess, specifically the
>>> header is in linux/ when its a purely i386-specific piece of code. I'm
>>> not sure why it has its config option; its only currently needed for
>>> "geode-gx1/cs5530a", so perhaps whatever config option controls that
>>> hardware should enable this?
>>>
>>
>> Thanks. Looks good.
>
> It looks like a cs5530a is a PATA driver in drivers/ata/pata_cs5530.c.
> Seems to me the cleanest fix is to register a reboot notifier in the
> driver and have it do the magic rather than have the special
> mach_reboot_fixups mechanism at all.
>
> Assuming it needs to be done at all...
>
> Alan? Jaya?
I'm guessing rather it is an embedded setup, then a weird pata thing.
If we want to remove crud from this path we should provide a hook like
pm_power_off to handle the reboot case.
This is not the kind of thing that a reboot notifier can be used for.
as:
a) reboot notifiers don't get called on all paths to machine_emergency_restart.
b) touching those bits looks like they immediately flip the reset line, so
trigger the reboot immediately.
Eric
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Clean up mach_reboot_fixups
2007-03-28 19:54 ` Andi Kleen
@ 2007-03-28 20:11 ` Jeremy Fitzhardinge
2007-03-28 20:14 ` Andi Kleen
0 siblings, 1 reply; 6+ messages in thread
From: Jeremy Fitzhardinge @ 2007-03-28 20:11 UTC (permalink / raw)
To: Andi Kleen
Cc: Jeremy Fitzhardinge, Virtualization Mailing List,
Stephane Eranian, Andrew Morton, Ingo Molnar, Jan Beulich,
Linux Kernel Mailing List
Two cleanups:
- reboot_fixups.h is entirely i386-dependent, so put it in asm-i386
- use a weak version rather than ifdeffery
[ Andi - the machine_ops probably depends on this, but only in a minor
context-clash way. ]
Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: Jaya Kumar <jayalk@intworks.biz>
---
arch/i386/kernel/reboot.c | 6 +++++-
arch/i386/kernel/reboot_fixups.c | 2 +-
include/asm-i386/reboot_fixups.h | 6 ++++++
include/linux/reboot_fixups.h | 10 ----------
4 files changed, 12 insertions(+), 12 deletions(-)
===================================================================
--- a/arch/i386/kernel/reboot.c
+++ b/arch/i386/kernel/reboot.c
@@ -17,7 +17,7 @@
#include <asm/apic.h>
#include <asm/desc.h>
#include "mach_reboot.h"
-#include <linux/reboot_fixups.h>
+#include <asm/reboot_fixups.h>
/*
* Power off function, if any
@@ -314,6 +314,10 @@ void machine_shutdown(void)
#ifdef CONFIG_X86_IO_APIC
disable_IO_APIC();
#endif
+}
+
+void __attribute__((weak)) mach_reboot_fixups(void)
+{
}
void machine_emergency_restart(void)
===================================================================
--- a/arch/i386/kernel/reboot_fixups.c
+++ b/arch/i386/kernel/reboot_fixups.c
@@ -10,7 +10,7 @@
#include <asm/delay.h>
#include <linux/pci.h>
-#include <linux/reboot_fixups.h>
+#include <asm/reboot_fixups.h>
static void cs5530a_warm_reset(struct pci_dev *dev)
{
===================================================================
--- /dev/null
+++ b/include/asm-i386/reboot_fixups.h
@@ -0,0 +1,6 @@
+#ifndef _LINUX_REBOOT_FIXUPS_H
+#define _LINUX_REBOOT_FIXUPS_H
+
+extern void mach_reboot_fixups(void);
+
+#endif /* _LINUX_REBOOT_FIXUPS_H */
===================================================================
--- a/include/linux/reboot_fixups.h
+++ /dev/null
@@ -1,10 +0,0 @@
-#ifndef _LINUX_REBOOT_FIXUPS_H
-#define _LINUX_REBOOT_FIXUPS_H
-
-#ifdef CONFIG_X86_REBOOTFIXUPS
-extern void mach_reboot_fixups(void);
-#else
-#define mach_reboot_fixups() ((void)(0))
-#endif
-
-#endif /* _LINUX_REBOOT_FIXUPS_H */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Clean up mach_reboot_fixups
2007-03-28 20:11 ` [PATCH] Clean up mach_reboot_fixups Jeremy Fitzhardinge
@ 2007-03-28 20:14 ` Andi Kleen
0 siblings, 0 replies; 6+ messages in thread
From: Andi Kleen @ 2007-03-28 20:14 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: Virtualization Mailing List, Stephane Eranian, Andrew Morton,
Ingo Molnar, Jan Beulich, Linux Kernel Mailing List
On Wednesday 28 March 2007 22:11, Jeremy Fitzhardinge wrote:
> Two cleanups:
> - reboot_fixups.h is entirely i386-dependent, so put it in asm-i386
> - use a weak version rather than ifdeffery
>
> [ Andi - the machine_ops probably depends on this, but only in a minor
> context-clash way. ]
Sorry I actually had it already, but forgot about it
-Andi
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-03-28 20:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-03-14 22:24 [PATCH] clean up mach_reboot_fixups Jeremy Fitzhardinge
2007-03-14 22:42 ` Andi Kleen
2007-03-15 2:18 ` Geode cs5530a magic (Was: Re: [PATCH] clean up mach_reboot_fixups) Jeremy Fitzhardinge
2007-03-19 14:48 ` Eric W. Biederman
-- strict thread matches above, loose matches on Subject: below --
2007-03-27 22:13 [PATCH] Simplify smp_call_function*() by using common implementation Jeremy Fitzhardinge
2007-03-28 19:43 ` [PATCH] Add machine_ops interface to abstract halting and rebooting Andi Kleen
2007-03-28 19:47 ` Jeremy Fitzhardinge
2007-03-28 19:54 ` Andi Kleen
2007-03-28 20:11 ` [PATCH] Clean up mach_reboot_fixups Jeremy Fitzhardinge
2007-03-28 20:14 ` Andi Kleen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox