Re: iwpriv (Was: OLPC News 2007-12-30)

2008-01-01 Thread David Woodhouse

On Mon, 2007-12-31 at 18:10 +, David Woodhouse wrote:
  An interesting goal would be cleaning up CONFIG_OLPC so that
  it could be enabled in stock kernels of standard Linux distros.
 
 I actually see that as a prerequisite for getting the thing upstream.
 And the first step along that path is to stop making it worse.

Let's see if we can repeat history. If experience with the libertas
driver is anything to go by, I predict that by starting to look at the
problem, I will provoke others into a generating a storm of conflicting
patches by attempting to do the same thing themselves¹.

So here's an untested patch to make the reboot fixups slightly more
generic, so that we can easily add our own 'fixup' for the XO in a
fashion which will actually be mergeable upstream.

Untested-but-otherwise-Signed-Off-By: David Woodhouse [EMAIL PROTECTED]

diff --git a/arch/x86/kernel/reboot_32.c b/arch/x86/kernel/reboot_32.c
index bb1a0f8..dedb1d8 100644
--- a/arch/x86/kernel/reboot_32.c
+++ b/arch/x86/kernel/reboot_32.c
@@ -332,9 +332,7 @@ static void native_machine_shutdown(void)
 #endif
 }
 
-void __attribute__((weak)) mach_reboot_fixups(void)
-{
-}
+void (*mach_reboot_fixup)(void);
 
 static void native_machine_emergency_restart(void)
 {
@@ -347,7 +345,8 @@ static void native_machine_emergency_restart(void)
/* rebooting needs to touch the page at absolute addr 0 */
*((unsigned short *)__va(0x472)) = reboot_mode;
for (;;) {
-   mach_reboot_fixups(); /* for board specific fixups */
+   if (mach_reboot_fixup)
+   mach_reboot_fixup();
mach_reboot();
/* That didn't work - force a triple fault.. */
load_idt(no_idt);
diff --git a/arch/x86/kernel/reboot_fixups_32.c 
b/arch/x86/kernel/reboot_fixups_32.c
index f452726..d9607a7 100644
--- a/arch/x86/kernel/reboot_fixups_32.c
+++ b/arch/x86/kernel/reboot_fixups_32.c
@@ -14,16 +14,18 @@
 #include asm/msr.h
 #include asm/geode.h
 
-static void cs5530a_warm_reset(struct pci_dev *dev)
+static pci_dev *cs5530a_pci_dev;
+
+static void cs5530a_warm_reset(void)
 {
/* writing 1 to the reset control register, 0x44 causes the
cs5530a to perform a system warm reset */
-   pci_write_config_byte(dev, 0x44, 0x1);
+   pci_write_config_byte(cs5530_pci_dev, 0x44, 0x1);
udelay(50); /* shouldn't get here but be safe and spin-a-while */
return;
 }
 
-static void cs5536_warm_reset(struct pci_dev *dev)
+static void cs5536_warm_reset(void)
 {
/* writing 1 to the LSB of this MSR causes a hard reset */
wrmsrl(MSR_DIVIL_SOFT_RESET, 1ULL);
@@ -48,24 +50,23 @@ static struct device_fixup fixups_table[] = {
  * do return, we keep looking and then eventually fall back to the
  * standard mach_reboot on return.
  */
-void mach_reboot_fixups(void)
+int mach_reboot_fixup_setup(void)
 {
struct device_fixup *cur;
struct pci_dev *dev;
int i;
 
-   /* we can be called from sysrq-B code. In such a case it is
-* prohibited to dig PCI */
-   if (in_interrupt())
-   return;
-
for (i=0; i  ARRAY_SIZE(fixups_table); i++) {
cur = (fixups_table[i]);
dev = pci_get_device(cur-vendor, cur-device, NULL);
if (!dev)
continue;
 
-   cur-reboot_fixup(dev);
+   cs5530a_pci_dev = dev;
+   mach_reboot_fixup = cur-reboot_fixup;
}
+   return 0;
 }
 
+subsys_initcall(mach_reboot_fixup_setup);
+
diff --git a/include/asm-x86/reboot_fixups.h b/include/asm-x86/reboot_fixups.h
index 0cb7d87..4f79001 100644
--- a/include/asm-x86/reboot_fixups.h
+++ b/include/asm-x86/reboot_fixups.h
@@ -1,6 +1,6 @@
 #ifndef _LINUX_REBOOT_FIXUPS_H
 #define _LINUX_REBOOT_FIXUPS_H
 
-extern void mach_reboot_fixups(void);
+extern void (*mach_reboot_fixup)(void);
 
 #endif /* _LINUX_REBOOT_FIXUPS_H */

-- 
dwmw2

¹ Only this time I don't actually plan to follow through; I'm relying on
the interference 

___
Devel mailing list
Devel@lists.laptop.org
http://lists.laptop.org/listinfo/devel


Re: iwpriv (Was: OLPC News 2007-12-30)

2008-01-01 Thread Bernardo Innocenti
David Woodhouse wrote:


 So here's an untested patch to make the reboot fixups slightly more
 generic, so that we can easily add our own 'fixup' for the XO in a
 fashion which will actually be mergeable upstream.

It would be slightly nicer and generic if we had

 void (*mach_reboot_fixup)(void *arg);
 void *mach_reboot_fixup_arg;

rather than the cs5530a_pci_dev global.

But anyway,

 Untested-but-otherwise-Signed-Off-By: David Woodhouse [EMAIL PROTECTED]
Untested-but-otherwise-Acked-By: Bernardo Innocenti [EMAIL PROTECTED]

-- 
 \___/
 |___|   Bernardo Innocenti - http://www.codewiz.org/
  \___\  One Laptop Per Child - http://www.laptop.org/
___
Devel mailing list
Devel@lists.laptop.org
http://lists.laptop.org/listinfo/devel


iwpriv (Was: OLPC News 2007-12-30)

2007-12-31 Thread Bernardo Innocenti
David Woodhouse wrote:

 As a general rule, that is totally incorrect. Changes should be pushed
 towards upstream _before_ they're ever committed to our tree, and any
 change which has been made only in the OLPC tree and not pushed upstream
 should be considered volatile and likely to disappear... like the
 private wireless ioctls I removed last week because they weren't
 upstream for example¹.

 ¹ I have actually put them back now, temporarily. But they will be going
 away again. Nothing is stable until it's upstream.

btw, we still have code in /etc/init.d/olpc-configure that
tries to use one of those private ioctls to remap the leds,
and outputs errors if they're missing.  Is this still needed?


 However, you're right about this patch not going upstream -- I thought
 I'd already told you that the naïve patch to cs5536_warm_reset() as
 shown in ticket #4397 was not acceptable. It doesn't live in that
 function, and even if it did, it shouldn't be happening unconditionally
 based on CONFIG_OLPC.

An interesting goal would be cleaning up CONFIG_OLPC so that
it could be enabled in stock kernels of standard Linux distros.

-- 
 \___/
 |___|   Bernardo Innocenti - http://www.codewiz.org/
  \___\  One Laptop Per Child - http://www.laptop.org/

___
Devel mailing list
Devel@lists.laptop.org
http://lists.laptop.org/listinfo/devel


Re: iwpriv (Was: OLPC News 2007-12-30)

2007-12-31 Thread David Woodhouse

On Mon, 2007-12-31 at 12:56 -0500, Bernardo Innocenti wrote: 
 btw, we still have code in /etc/init.d/olpc-configure that
 tries to use one of those private ioctls to remap the leds,
 and outputs errors if they're missing.  Is this still needed?

Yes, I think so. And I think it probably even justifies a private ioctl.
So it'll get proper consideration and it'll get sent upstream. Not just
dumped into our kernel and forgotten. 

  However, you're right about this patch not going upstream -- I thought
  I'd already told you that the naïve patch to cs5536_warm_reset() as
  shown in ticket #4397 was not acceptable. It doesn't live in that
  function, and even if it did, it shouldn't be happening unconditionally
  based on CONFIG_OLPC.
 
 An interesting goal would be cleaning up CONFIG_OLPC so that
 it could be enabled in stock kernels of standard Linux distros.

I actually see that as a prerequisite for getting the thing upstream.
And the first step along that path is to stop making it worse.

-- 
dwmw2

___
Devel mailing list
Devel@lists.laptop.org
http://lists.laptop.org/listinfo/devel