[PATCH] Toshiba Laptop Support and IRQ Locks

[PATCH] Toshiba Laptop Support and IRQ Locks

[John Weber]

Hi,

Toshiba laptop support is broken.  Here's my rookie attempt at fixing it.

--- linux-2.5.30/drivers/char/toshiba.c	2002-08-01 17:16:39.000000000 -0400
+++ linux-bleed/drivers/char/toshiba.c	2002-08-02 11:37:14.000000000 -0400
@@ -82,6 +82,8 @@

  static int tosh_fn = 0;

+extern rwlock_t tosh_lock;
+
  MODULE_PARM(tosh_fn, "i");

  MODULE_LICENSE("GPL");
@@ -114,11 +116,10 @@
  	if (tosh_fn!=0) {
  		scan = inb(tosh_fn);
  	} else {
- 
	save_flags(flags);
- 
	cli();
+ 
         write_lock_irq(&tosh_lock);
  		outb(0x8e, 0xe4);
  		scan = inb(0xe5);
- 
	restore_flags(flags);
+ 
	write_unlock_irq(&tosh_lock);
  	}

          return (int) scan;
@@ -141,35 +142,32 @@
  	if (tosh_id==0xfccb) {
  		if (eax==0xfe00) {
  	 
	/* fan status */
- 
		save_flags(flags);
- 
		cli();
+ 
	        write_lock_irq(&tosh_lock);
  	 
	outb(0xbe, 0xe4);
  	 
	al = inb(0xe5);
- 
		restore_flags(flags);
+ 
		write_unlock_irq(&tosh_lock);
  	 
	regs->eax = 0x00;
  	 
	regs->ecx = (unsigned int) (al & 0x01);
  		}
  		if ((eax==0xff00) && (ecx==0x0000)) {
  	 
	/* fan off */
- 
		save_flags(flags);
- 
		cli();
+ 
	        write_lock_irq(&tosh_lock);
  	 
	outb(0xbe, 0xe4);
  	 
	al = inb(0xe5);
  	 
	outb(0xbe, 0xe4);
  	 
	outb (al | 0x01, 0xe5);
- 
		restore_flags(flags);
+ 
		write_unlock_irq(&tosh_lock);
  	 
	regs->eax = 0x00;
  	 
	regs->ecx = 0x00;
  		}
  		if ((eax==0xff00) && (ecx==0x0001)) {
  	 
	/* fan on */
- 
		save_flags(flags);
- 
		cli();
+ 
	        write_lock_irq(&tosh_lock);
  	 
	outb(0xbe, 0xe4);
  	 
	al = inb(0xe5);
  	 
	outb(0xbe, 0xe4);
  	 
	outb(al & 0xfe, 0xe5);
- 
		restore_flags(flags);
+ 
		write_unlock_irq(&tosh_lock);
  	 
	regs->eax = 0x00;
  	 
	regs->ecx = 0x01;
  		}
@@ -180,33 +178,30 @@
  	if (tosh_id==0xfccc) {
  		if (eax==0xfe00) {
  	 
	/* fan status */
- 
		save_flags(flags);
- 
		cli();
+ 
	        write_lock_irq(&tosh_lock);
  	 
	outb(0xe0, 0xe4);
  	 
	al = inb(0xe5);
- 
		restore_flags(flags);
+ 
		write_unlock_irq(&tosh_lock);
  	 
	regs->eax = 0x00;
  	 
	regs->ecx = al & 0x01;
  		}
  		if ((eax==0xff00) && (ecx==0x0000)) {
  	 
	/* fan off */
- 
		save_flags(flags);
- 
		cli();
+ 
	        write_lock_irq(&tosh_lock);
  	 
	outb(0xe0, 0xe4);
  	 
	al = inb(0xe5);
  	 
	outw(0xe0 | ((al & 0xfe) << 8), 0xe4);
- 
		restore_flags(flags);
+ 
		write_unlock_irq(&tosh_lock);
  	 
	regs->eax = 0x00;
  	 
	regs->ecx = 0x00;
  		}
  		if ((eax==0xff00) && (ecx==0x0001)) {
  	 
	/* fan on */
- 
		save_flags(flags);
- 
		cli();
+ 
	        write_lock_irq(&tosh_lock);
  	 
	outb(0xe0, 0xe4);
  	 
	al = inb(0xe5);
  	 
	outw(0xe0 | ((al | 0x01) << 8), 0xe4);
- 
		restore_flags(flags);
+ 
		write_unlock_irq(&tosh_lock);
  	 
	regs->eax = 0x00;
  	 
	regs->ecx = 0x01;
  		}

Re: [PATCH] Toshiba Laptop Support and IRQ Locks

[Alan Cox]

On Fri, 2002-08-02 at 17:03, John Weber wrote:
> Hi,
> 
> Toshiba laptop support is broken.  Here's my rookie attempt at fixing it.

Looks basically sound. You probably want to use spinlock_irqsave - the
spin locks are less overhead than the reader/writer locks and you don't
really seem to be using it for anything else. I'm assuming we want the
irqsave to block interrupts because the I/O cycles might have to happen
one after another - if not they could be relaxed - perhaps Jonathan
knows ?

Re: [PATCH] Toshiba Laptop Support and IRQ Locks

[Jonathan Buzzard]

alan@lxorguk.ukuu.org.uk said:
> Hi,
> 
> Toshiba laptop support is broken.  Here's my rookie attempt at fixing
> it.

Whats broken? I have not seen the patch, though I don't track the latest
2.5 kernels either.

> Looks basically sound. You probably want to use spinlock_irqsave - the
> spin locks are less overhead than the reader/writer locks and you
> don't really seem to be using it for anything else. I'm assuming we
> want the irqsave to block interrupts because the I/O cycles might have
> to happen one after another - if not they could be relaxed - perhaps
> Jonathan knows ?

Someone show me the patch and I can say for sure.

Two things to bare in mind, Toshiba have yet to do any sort of
multi processor laptop, are extremely unlikely to ever manufacture
one, and to the best of my knowledge the module only loads on Toshiba
laptops. If it loads on anything else it is broken and needs fixing
so it does not.

On this ground I always felt that use of cli() was more than justified.
Though I understand that you might want to banish this stuff from the
kernel, and use something multiprocessor safe.

Secondly bare in mind that the Toshiba laptop module for the most part
puts the processor into System Management Mode. This does its own
locking, once you are in SMM thats *everything* else on hold till it
finishes.

While I am at it, I have been recently pestered with patches to
toshiba.c to add random proc interfaces for various HCI calls. I
have personally rejected the patches as I don't believe that exposing
HCI calls in the kernel is the right thing to do. Firstly there are
over 30 calls to expose, and it makes not sense to just expose one or
two that happen to be the favourite of some random person. Secondly
everything the patches do can be achieved with a userspace program
that use the ioctl on /dev/toshiba that the module provides. Thus
avoiding any bloat to the kernel, and avoiding the need to constantly
add new proc based interfaces each time I discover a new HCI call. 

If someone does propose such a patch I recommend rejecting it.

JAB.

-- 
Jonathan A. Buzzard                 Email: jonathan@buzzard.org.uk
Northumberland, United Kingdom.       Tel: +44(0)1661-832195

Re: [PATCH] Toshiba Laptop Support and IRQ Locks

[Brian Gerst]

Jonathan Buzzard wrote:
> alan@lxorguk.ukuu.org.uk said:
> 
>>Hi,
>>
>>Toshiba laptop support is broken.  Here's my rookie attempt at fixing
>>it.
> 
> 
> Whats broken? I have not seen the patch, though I don't track the latest
> 2.5 kernels either.
> 
> 
>>Looks basically sound. You probably want to use spinlock_irqsave - the
>>spin locks are less overhead than the reader/writer locks and you
>>don't really seem to be using it for anything else. I'm assuming we
>>want the irqsave to block interrupts because the I/O cycles might have
>>to happen one after another - if not they could be relaxed - perhaps
>>Jonathan knows ?
> 
> 
> Someone show me the patch and I can say for sure.
> 
> Two things to bare in mind, Toshiba have yet to do any sort of
> multi processor laptop, are extremely unlikely to ever manufacture
> one, and to the best of my knowledge the module only loads on Toshiba
> laptops. If it loads on anything else it is broken and needs fixing
> so it does not.

What about P4 Hyperthreading?

--
				Brian Gerst

Re: [PATCH] Toshiba Laptop Support and IRQ Locks

[John Weber]

Alan Cox wrote:
> On Fri, 2002-08-02 at 17:03, John Weber wrote:
> 
>>Hi,
>>
>>Toshiba laptop support is broken.  Here's my rookie attempt at fixing it.
> 
> 
> Looks basically sound. You probably want to use spinlock_irqsave - the
> spin locks are less overhead than the reader/writer locks and you don't
> really seem to be using it for anything else. I'm assuming we want the
> irqsave to block interrupts because the I/O cycles might have to happen
> one after another - if not they could be relaxed - perhaps Jonathan
> knows ?

Alrighty then, the patch below uses spinlocks instead of cli() and 
friends -- to conform to the new irq locking mechanism -- and some minor 
module changes while we're at it.

--- linux-2.5.30/drivers/char/toshiba.c	2002-08-01 17:16:39.000000000 -0400
+++ linux-bleed/drivers/char/toshiba.c	2002-08-02 18:43:53.000000000 -0400
@@ -82,7 +82,13 @@

  static int tosh_fn = 0;

+extern spinlock_t tosh_lock;
+
  MODULE_PARM(tosh_fn, "i");
+MODULE_PARM_DESC(tosh_fn, "User specified Fn key detection port");
+MODULE_AUTHOR("Jonathan Buzzard <jonathan@buzzard.org.uk>");
+MODULE_DESCRIPTION("Toshiba laptop SMM driver");
+MODULE_SUPPORTED_DEVICE("toshiba");

  MODULE_LICENSE("GPL");

@@ -114,11 +120,10 @@
  	if (tosh_fn!=0) {
  		scan = inb(tosh_fn);
  	} else {
- 
	save_flags(flags);
- 
	cli();
+ 
         spin_lock_irqsave(&tosh_lock,flags);
  		outb(0x8e, 0xe4);
  		scan = inb(0xe5);
- 
	restore_flags(flags);
+ 
	spin_unlock_irqrestore(&tosh_lock,flags);
  	}

          return (int) scan;
@@ -141,35 +146,32 @@
  	if (tosh_id==0xfccb) {
  		if (eax==0xfe00) {
  	 
	/* fan status */
- 
		save_flags(flags);
- 
		cli();
+ 
	        spin_lock_irqsave(&tosh_lock,flags);
  	 
	outb(0xbe, 0xe4);
  	 
	al = inb(0xe5);
- 
		restore_flags(flags);
+ 
		spin_unlock_irqrestore(&tosh_lock,flags);
  	 
	regs->eax = 0x00;
  	 
	regs->ecx = (unsigned int) (al & 0x01);
  		}
  		if ((eax==0xff00) && (ecx==0x0000)) {
  	 
	/* fan off */
- 
		save_flags(flags);
- 
		cli();
+ 
	        spin_lock_irqsave(&tosh_lock,flags);
  	 
	outb(0xbe, 0xe4);
  	 
	al = inb(0xe5);
  	 
	outb(0xbe, 0xe4);
  	 
	outb (al | 0x01, 0xe5);
- 
		restore_flags(flags);
+ 
		spin_unlock_irqrestore(&tosh_lock,flags);
  	 
	regs->eax = 0x00;
  	 
	regs->ecx = 0x00;
  		}
  		if ((eax==0xff00) && (ecx==0x0001)) {
  	 
	/* fan on */
- 
		save_flags(flags);
- 
		cli();
+ 
	        spin_lock_irqsave(&tosh_lock,flags);
  	 
	outb(0xbe, 0xe4);
  	 
	al = inb(0xe5);
  	 
	outb(0xbe, 0xe4);
  	 
	outb(al & 0xfe, 0xe5);
- 
		restore_flags(flags);
+ 
		spin_unlock_irqrestore(&tosh_lock,flags);
  	 
	regs->eax = 0x00;
  	 
	regs->ecx = 0x01;
  		}
@@ -180,33 +182,30 @@
  	if (tosh_id==0xfccc) {
  		if (eax==0xfe00) {
  	 
	/* fan status */
- 
		save_flags(flags);
- 
		cli();
+ 
	        spin_lock_irqsave(&tosh_lock,flags);
  	 
	outb(0xe0, 0xe4);
  	 
	al = inb(0xe5);
- 
		restore_flags(flags);
+ 
		spin_unlock_irqrestore(&tosh_lock,flags);
  	 
	regs->eax = 0x00;
  	 
	regs->ecx = al & 0x01;
  		}
  		if ((eax==0xff00) && (ecx==0x0000)) {
  	 
	/* fan off */
- 
		save_flags(flags);
- 
		cli();
+ 
	        spin_lock_irqsave(&tosh_lock,flags);
  	 
	outb(0xe0, 0xe4);
  	 
	al = inb(0xe5);
  	 
	outw(0xe0 | ((al & 0xfe) << 8), 0xe4);
- 
		restore_flags(flags);
+ 
		spin_unlock_irqrestore(&tosh_lock,flags);
  	 
	regs->eax = 0x00;
  	 
	regs->ecx = 0x00;
  		}
  		if ((eax==0xff00) && (ecx==0x0001)) {
  	 
	/* fan on */
- 
		save_flags(flags);
- 
		cli();
+ 
	        spin_lock_irqsave(&tosh_lock,flags);
  	 
	outb(0xe0, 0xe4);
  	 
	al = inb(0xe5);
  	 
	outw(0xe0 | ((al | 0x01) << 8), 0xe4);
- 
		restore_flags(flags);
+ 
		spin_unlock_irqrestore(&tosh_lock,flags);
  	 
	regs->eax = 0x00;
  	 
	regs->ecx = 0x01;
  		}

Re: [PATCH] Toshiba Laptop Support and IRQ Locks

[Jonathan Buzzard]

bgerst@didntduck.org said:
> > Two things to bare in mind, Toshiba have yet to do any sort of
> > multi processor laptop, are extremely unlikely to ever manufacture
> > one, and to the best of my knowledge the module only loads on Toshiba
> > laptops. If it loads on anything else it is broken and needs fixing
> > so it does not.
>
> What about P4 Hyperthreading? 

The cli() and associated code in the toshiba SMM driver is for laptops
manufactured prior to 1999. It is not relevant and never will be on
any laptop with a P4 processor in it. For that matter it is not
even relevant on a PIII or PII processor. It is strictly Pentiums and
486's.

JAB.

-- 
Jonathan A. Buzzard                 Email: jonathan@buzzard.org.uk
Northumberland, United Kingdom.       Tel: +44(0)1661-832195

Re: [PATCH] Toshiba Laptop Support and IRQ Locks

[rwhron]

> Alrighty then, the patch below uses spinlocks instead of cli() and
> friends -- to conform to the new irq locking mechanism -- and some minor
> module changes while we're at it.

The patch seems mangled in:
http://marc.theaimsgroup.com/?l=linux-kernel&m=102832986723995&w=2
I'd like to try it if someone has an unmangled version.

2.5.34 compile on my toshiba tecra 8000 stopped with:

drivers/built-in.o: In function `tosh_fn_status':
drivers/built-in.o(.text+0x17569): undefined reference to `save_flags'
drivers/built-in.o(.text+0x1756e): undefined reference to `cli'
drivers/built-in.o(.text+0x1757f): undefined reference to `restore_flags'
drivers/built-in.o: In function `tosh_emulate_fan':

The patch below gets it to compile and boot.

diff -ruN linux-2.5.34/drivers/char/toshiba.c linux/drivers/char/toshiba.c
--- linux-2.5.34/drivers/char/toshiba.c 2002-05-21 01:07:42.000000000 -0400
+++ linux/drivers/char/toshiba.c        2002-09-10 17:30:27.000000000 -0400
@@ -57,6 +57,7 @@
 #define TOSH_DEBUG 0

 #include <linux/module.h>
+#include <linux/interrupt.h>
 #include <linux/version.h>
 #include <linux/kernel.h>
 #include <linux/sched.h>

The keyboard isn't working though.  That may be a config issue.

egrep ^C.*INPUT .config
CONFIG_INPUT=y
CONFIG_INPUT_MOUSEDEV=y
CONFIG_INPUT_MOUSEDEV_PSAUX=y
CONFIG_INPUT_MOUSEDEV_SCREEN_X=1024
CONFIG_INPUT_MOUSEDEV_SCREEN_Y=768
CONFIG_INPUT_KEYBOARD=y

I haven't tried 2.5 on the laptop for a long time.  
Anyone running 2.5 on a toshiba laptop?  A tecra 8000?

-- 
Randy Hron
http://home.earthlink.net/~rwhron/kernel/bigbox.html