[parisc-linux] The final patch for the keyboard problem

[parisc-linux] The final patch for the keyboard problem

[Thomas Marteau]

[-- Attachment #1: Type: text/plain, Size: 420 bytes --]

Hi all,

	It is my last patch solving the keyboard problem on BXXX and CXXX. It
should correct most of them but I would like to have feedback before I
check the bug report. I must also add that this patch is not yet tested
with Matthew Wilcox's version, according to his mail (
http://lists.parisc-linux.org/pipermail/parisc-linux/2001-August/013302.html
). I will do it as soon as possible.

Regards, Thomas.
ESIEE Team

[-- Attachment #2: keyboard.patch --]
[-- Type: text/plain, Size: 7009 bytes --]

diff -Nru linux/drivers/char/hp_psaux.c linux.new/drivers/char/hp_psaux.c
--- linux/drivers/char/hp_psaux.c	Wed Aug  1 00:11:04 2001
+++ linux.new/drivers/char/hp_psaux.c	Tue Aug  7 09:10:39 2001
@@ -42,6 +42,7 @@
 
 /* HP specific LASI PS/2 keyboard and psaux constants */
 #define	AUX_REPLY_ACK	0xFA	/* Command byte ACK. */
+#define	AUX_RESEND	0xFE	/* Sent by the keyb. Asking for resending the last command. */
 #define	AUX_RECONNECT	0xAA	/* scancode when ps2 device is plugged (back) in */
 
 #define	LASI_PSAUX_OFFSET 0x0100 /* offset from keyboard to psaux port */
@@ -70,8 +71,7 @@
 #define LASI_STAT_CLKSHD 0x80
 
 static void *lasikbd_hpa;
-static void *lasips2_hpa;
-
+static int cmd_status;
 
 static inline u8 read_input(void *hpa)
 {
@@ -105,9 +105,10 @@
 		}
 	}
 
+#if 0
 	if (wait)
 		printk(KERN_DEBUG "Lasi PS/2 wait %d\n", wait);
-	
+#endif
 	gsc_writeb(val, hpa+LASI_XMTDATA);
 
 	return 1;
@@ -122,9 +123,23 @@
 
 static void lasikbd_leds(unsigned char leds)
 {
-	write_output(KBD_CMD_SET_LEDS, lasikbd_hpa);
-	write_output(leds, lasikbd_hpa);
-	write_output(KBD_CMD_ENABLE, lasikbd_hpa);
+	cmd_status=2;
+	while (cmd_status!=0) {
+		write_output(KBD_CMD_SET_LEDS, lasikbd_hpa);
+		mdelay(1); 
+	}
+   
+	cmd_status=2;
+	while (cmd_status!=0) {
+		write_output(leds, lasikbd_hpa);
+		mdelay(1);
+	}
+
+	cmd_status=2;
+	while (cmd_status!=0) {
+	   write_output(KBD_CMD_ENABLE, lasikbd_hpa);   
+	   mdelay(1);
+	}
 }
 
 #if 0
@@ -154,10 +169,56 @@
 }
 #endif 
 
-static int __init lasi_ps2_reset(void *hpa, int id)
+static int init_keyb(void* hpa)
+{
+	int fail;
+	u8 data;
+	
+	
+	do {
+		while ((read_status(hpa) & LASI_STAT_TBNE)==0x01);
+		gsc_writeb(KBD_CMD_SET_LEDS, hpa+LASI_XMTDATA);
+		
+		while ((read_status(hpa) & LASI_STAT_RBNE)==0x00);
+		
+		while ((read_status(hpa) & LASI_STAT_RBNE)==0x01) data=read_input(hpa);      
+	}while(data!=AUX_REPLY_ACK); 
+	
+	fail=0;
+	
+	do {
+		while ((read_status(hpa) & LASI_STAT_TBNE)==0x01);
+		gsc_writeb(0x00, hpa+LASI_XMTDATA);
+		
+		while ((read_status(hpa) & LASI_STAT_RBNE)==0x00);
+   
+		while ((read_status(hpa) & LASI_STAT_RBNE)==0x01) {
+			data=read_input(hpa);
+			fail++; 
+			if(fail==10) break; 
+		}
+   	}while(data!=AUX_REPLY_ACK); 
+
+	fail=0;
+	do {   
+		while ((read_status(hpa) & LASI_STAT_TBNE)==0x01);
+		gsc_writeb(KBD_CMD_ENABLE, hpa+LASI_XMTDATA);
+      
+		while ((read_status(hpa) & LASI_STAT_RBNE)==0x00);
+		while ((read_status(hpa) & LASI_STAT_RBNE)==0x01) {
+			data=read_input(hpa);
+			fail++; 
+			if(fail==10) break; 
+		}
+
+	}while(data!=AUX_REPLY_ACK); 
+   
+	return 1;
+}
+
+static void __init lasi_ps2_reset(void *hpa)
 {
 	u8 control;
-	int ret = 1;
 
 	/* reset the interface */
 	gsc_writeb(0xff, hpa+LASI_RESET);
@@ -166,25 +227,8 @@
 	/* enable it */
 	control = read_control(hpa);
 	write_control(control | LASI_CTRL_ENBL, hpa);
-
-        /* initializes the leds at the default state */
-        if (id==0) {
-           write_output(KBD_CMD_SET_LEDS, hpa);
-	   write_output(0, hpa);
-	   ret = write_output(KBD_CMD_ENABLE, hpa);
-	}
-
-	return ret;
 }
 
-static int inited;
-
-static void lasi_ps2_init_hw(void)
-{
-	++inited;
-}
-
-
 /* Greatly inspired by pc_keyb.c */
 
 /*
@@ -362,7 +406,7 @@
 	lock_kernel();
 	fasync_aux(-1, file, 0);
 	if (--aux_count) {
-	   unlock_kernel();
+		unlock_kernel();
 		return 0;
 	}
 	unlock_kernel();
@@ -398,62 +442,60 @@
         id = gsc_readb(hpa+LASI_ID) & 0x0f;
         
         if (id==1) 
-           hpa -= LASI_PSAUX_OFFSET; 
-        lasikbd_hpa = hpa;
-        
-
+		hpa -= LASI_PSAUX_OFFSET; 
+	
         status_keyb = read_status(hpa);
         status_mouse = read_status(hpa+LASI_PSAUX_OFFSET);
 
         while ((status_keyb|status_mouse) & LASI_STAT_RBNE){
            
-           while (status_keyb & LASI_STAT_RBNE) {
+		while (status_keyb & LASI_STAT_RBNE) {
 	      
-              scancode = read_input(hpa);
+		scancode = read_input(hpa);
 
-	      /* XXX don't know if this is a valid fix, but filtering
-	       * 0xfa avoids 'unknown scancode' errors on, eg, capslock
-	       * on some keyboards.
-	       */
-	      if (inited && scancode != 0xfa)
-		 handle_at_scancode(scancode); 
+	        /* XXX don't know if this is a valid fix, but filtering
+	         * 0xfa avoids 'unknown scancode' errors on, eg, capslock
+	         * on some keyboards.
+	         */
+	      	      
+		if (scancode == AUX_REPLY_ACK) 
+			cmd_status=0;
+			
+		else if (scancode == AUX_RESEND)
+			cmd_status=1;
+		else 
+			handle_at_scancode(scancode); 
 	      
-	      status_keyb =read_status(hpa);
-           }
+		status_keyb =read_status(hpa);
+		}
 	   
 #ifdef CONFIG_PSMOUSE
-           while (status_mouse & LASI_STAT_RBNE) {
-	      scancode = read_input(hpa+LASI_PSAUX_OFFSET);
-	      handle_mouse_scancode(scancode);
-              status_mouse = read_status(hpa+LASI_PSAUX_OFFSET);
-	   }
-           status_mouse = read_status(hpa+LASI_PSAUX_OFFSET);
+		while (status_mouse & LASI_STAT_RBNE) {
+			scancode = read_input(hpa+LASI_PSAUX_OFFSET);
+			handle_mouse_scancode(scancode);
+			status_mouse = read_status(hpa+LASI_PSAUX_OFFSET);
+		}
+		status_mouse = read_status(hpa+LASI_PSAUX_OFFSET);
 #endif /* CONFIG_PSMOUSE */
-           status_keyb = read_status(hpa);
+		status_keyb = read_status(hpa);
         }
 
         tasklet_schedule(&keyboard_tasklet);
         return (status_keyb|status_mouse);
 }
-
-
-
 	
 extern struct pt_regs *kbd_pt_regs;
 
 static void lasikbd_interrupt(int irq, void *dev, struct pt_regs *regs)
 {
-	lasips2_hpa = dev; /* save "hpa" for lasikbd_leds() */
 	kbd_pt_regs = regs;
-	handle_lasikbd_event(lasips2_hpa);
+	handle_lasikbd_event(dev);
 }
 
-
 extern int pckbd_translate(unsigned char, unsigned char *, char);
 
 static struct kbd_ops gsc_ps2_kbd_ops = {
 	translate:	pckbd_translate,
-	init_hw:	lasi_ps2_init_hw,
 	leds:		lasikbd_leds,
 #ifdef CONFIG_MAGIC_SYSRQ
 	sysrq_key:	0x54,
@@ -487,20 +529,11 @@
 	}
 	
 	/* reset the PS/2 port */
-	device_found = lasi_ps2_reset(hpa,id);
-
-	/* allocate the irq and memory region for that device */
-	if (!(irq = busdevice_alloc_irq(d)))
-		return -ENODEV;
-		    
-	if (request_irq(irq, lasikbd_interrupt, 0, name, hpa))
-		return -ENODEV;
-	
-	if (!request_mem_region((unsigned long)hpa, LASI_STATUS + 4, name))
-		return -ENODEV;
+	lasi_ps2_reset(hpa);
 
 	switch (id) {
 	case 0:	
+	        device_found = init_keyb(hpa);
 		register_kbd_ops(&gsc_ps2_kbd_ops);
 		break;
 	case 1:
@@ -526,6 +559,18 @@
 #endif
 	} /* of case */
 	
+        /* allocate the irq and memory region for that device */
+	if (!(irq = busdevice_alloc_irq(d)))
+		return -ENODEV;
+		    
+	if (request_irq(irq, lasikbd_interrupt, 0, name, hpa))
+		return -ENODEV;
+	
+	if (!request_mem_region((unsigned long)hpa, LASI_STATUS + 4, name))
+		return -ENODEV;
+
+
+
 	printk(KERN_INFO "PS/2 %s controller at 0x%08lx (irq %d) found, "
 			 "%sdevice attached.\n",
 			name, (unsigned long)hpa, irq,
@@ -548,4 +593,3 @@
 }
 
 module_init(gsc_ps2_init);
-

Re: [parisc-linux] The final patch for the keyboard problem

[Richard Hirst]

Hi Thimas,

On Tue, Aug 07, 2001 at 09:27:41AM +0200, Thomas Marteau wrote:

> +static int cmd_status;

Calling this awaiting_ack, and setting it true/false might make the code
more readable.  Also I suspect it should be volatile really, as it is
modified via an interrupt handler.

> +	cmd_status=2;
> +	while (cmd_status!=0) {
> +		write_output(KBD_CMD_SET_LEDS, lasikbd_hpa);
> +		mdelay(1); 
> +	}

Nasty, as it will hang the kernel if the keyboard never ACKs.  Should at
least limit the muber of times you loop here.

> +		while ((read_status(hpa) & LASI_STAT_TBNE)==0x01);

Why not simply

		while ((read_status(hpa) & LASI_STAT_TBNE))
			;

> +	do {
> +		while ((read_status(hpa) & LASI_STAT_TBNE)==0x01);
> +		gsc_writeb(0x00, hpa+LASI_XMTDATA);
> +		
> +		while ((read_status(hpa) & LASI_STAT_RBNE)==0x00);
> +   
> +		while ((read_status(hpa) & LASI_STAT_RBNE)==0x01) {
> +			data=read_input(hpa);
> +			fail++; 
> +			if(fail==10) break; 
> +		}
> +   	}while(data!=AUX_REPLY_ACK); 

Do you really want the third 'while' loop in that block of code?

	loop = 10;
	do {
		while ((read_status(hpa) & LASI_STAT_TBNE))
			;
		gsc_writeb(0x00, hpa+LASI_XMTDATA);
		while (!(read_status(hpa) & LASI_STAT_RBNE))
			;
		data = read_input(hpa);
		loop--;
	} while (data != AUX_REPLY_ACK && loop);

One less while loop, but it can still hang in either of the first two while
loops if the h/w misbehaves.

> +        /* allocate the irq and memory region for that device */
> +	if (!(irq = busdevice_alloc_irq(d)))
> +		return -ENODEV;
> +		    
> +	if (request_irq(irq, lasikbd_interrupt, 0, name, hpa))
> +		return -ENODEV;
> +	
> +	if (!request_mem_region((unsigned long)hpa, LASI_STATUS + 4, name))
> +		return -ENODEV;

Typically a driver would release resources before exiting.  e.g. if
request_mem_region() fails you should release_irq().

There are a couple of compiler warnings also.

Anyway, I'm about to try it on a couple of machines here; I'll let you
know how it goes.

Cheers,
  Richard

Re: [parisc-linux] The final patch for the keyboard problem

[Richard Hirst]

On Tue, Aug 07, 2001 at 11:58:17AM +0100, Richard Hirst wrote:
> Anyway, I'm about to try it on a couple of machines here; I'll let you
> know how it goes.

HP US keyboard on B180.  Worked until I unplugged and replugged the
keyboard.  Keyboard was dead after that, but machine still responded
to pings.

Compaq UK keyboard on B180.  Worked initially, except pressing '\' (to
the left of Z) gave "unrecognised scancode (7f) ignored".  I then pressed
CapsLock, and the machine hung, not even responding to pings.

Went back to the HP keyboard, and CapsLock hangs the machine there also.

Richard

Re: [parisc-linux] The final patch for the keyboard problem

[Matthew Wilcox]

On Tue, Aug 07, 2001 at 09:27:41AM +0200, Thomas Marteau wrote:
> I must also add that this patch is not yet tested
> with Matthew Wilcox's version, according to his mail (
> http://lists.parisc-linux.org/pipermail/parisc-linux/2001-August/013302.html
> ). I will do it as soon as possible.

Don't worry about that; if you manage to fix the keyboard problems,
I'll adapt my patch.

-- 
Revolutions do not require corporate support.

Re: [parisc-linux] The final patch for the keyboard problem

[Richard Hirst]

On Tue, Aug 07, 2001 at 01:14:19PM +0100, Richard Hirst wrote:
> Went back to the HP keyboard, and CapsLock hangs the machine there also.


I've fixed the lockup on CapsLock, using an analyser to monitor what happened
on the PS/2 cable:

(fw=firmware, kb=keyboard, li=linux)

B180, HP keyboard, this is what the firmware does on reset:

fw F5	disable scanning
kb FA	ack

fw F2	???
kb FA	ack
kb AB
kb 83

fw FF	reset
kb FA	ack
kb AA	power-on reset

fw F0	???
kb FA	ack

fw 02	???
kb FA	ack

fw F0	???
kb FA	ack

fw 00	???
kb FA	ack
kb 02

fw ED	set leds
kb FA	ack

fw 02	NumLock
kb FA	ack

fw F4	enable scanning
kb FA	ack

Now CapsLock pressed and released:

kb 58	keypress

fw ED	set leds
kb FA	ack

fw 06	CapsLock + NumLock
kb FA	ack

kb F0	keyrelease
kb 58

Now we boot Linux:

li ED	set leds
kb FA	ack

li 00	No LEDs
kb FA	ack

li F4	enable scanning
kb FA	ack

Now we press and release '1':

kb 16	keypress

kb F0	keyrelease
kb 16

Now we press CapsLock:

kb 58	keypress

li ED	set leds
li ED	set leds
kb FA	ack

System is hung.

The problem was that the driver retried the 'set leds' every 1ms, but the data rate
on the serial PS/2 cable is such that it takes at least 1ms for the command to reach
the keyboard, and a further 1ms for the ack to return.

I changed the driver to allow 5ms rather than 1ms, and it works now.  In fact, for
my keyboard, the time from command being written to ack being received is about
2.5ms.

Can't say I like the idea of mdelay(5) being called 3 times every time I press
CapsLock though.

Richard

Re: [parisc-linux] The final patch for the keyboard problem

[Richard Hirst]

On Tue, Aug 07, 2001 at 01:14:19PM +0100, Richard Hirst wrote:
> On Tue, Aug 07, 2001 at 11:58:17AM +0100, Richard Hirst wrote:
> > Anyway, I'm about to try it on a couple of machines here; I'll let you
> > know how it goes.
> 
> HP US keyboard on B180.  Worked until I unplugged and replugged the
> keyboard.  Keyboard was dead after that, but machine still responded
> to pings.

Tried unplugging/replugging again; survived this time.  Keyboard
generates a scancode AA (reconnected), but the console says unrecognised
scancode 7f.

btw, I guess we should reinitialise the LED status on a replug.

Back to the compaq UK keyboard, CapsLock works also with the longer
mdelay() calls.  The CapsLock LED doesn't come on, but the others
do, so I'll assume faulty k/b for now.  Unplug/replug still results
in a dead keyboard (init=/bin/sh, so I can't tell if the machine is
still alive).

On the mdelay() calls, the PC driver would check for an ack every 1ms,
but would only resend the command every 1000ms.  Your code was resending
every 1ms.  Checking every 1ms and resending less frequently is better
than just checking and resending every 5ms.
  
Richard

Re: [parisc-linux] The final patch for the keyboard problem

[Grant Grundler]

Richard Hirst wrote:
> I've fixed the lockup on CapsLock, using an analyser to monitor what happened
> on the PS/2 cable:

Cool!
reverse engineering is more fun when one has the right tools...

> I changed the driver to allow 5ms rather than 1ms, and it works now.
> In fact, for my keyboard, the time from command being written to ack
> being received is about 2.5ms.
>
> Can't say I like the idea of mdelay(5) being called 3 times every time I
> press CapsLock though.

Would sleep/wakeup be better than polling?
It seems the keyboard does generate interrupts.

grant

Grant Grundler
parisc-linux {PCI|IOMMU|SMP} hacker
+1.408.447.7253

Re: [parisc-linux] The final patch for the keyboard problem

[Richard Hirst]

On Tue, Aug 07, 2001 at 09:27:41AM +0200, Thomas Marteau wrote:
> Hi all,
> 
> 	It is my last patch solving the keyboard problem on BXXX and CXXX. It

With this patch applied my B180 hangs in init_keyb(), if there is no
keyboard attached.  I was trying to boot a 2.4.7 kernel based installer.

Richard.

Re: [parisc-linux] The final patch for the keyboard problem

[marteau]

Hi Richard,                                                           
                                                                      
            Thanks for the feedback. I think that I will work on it   
tomorrow in order to give a version that do not hang with the leds and
take care of the presence of the device. I think that the fact that   
the system is hanged is due to my "while"s I planned to remove them   
too...                                                                
                                                                      
Thanks again, Thomas                                                  
ESIEE Team