Re: BUG: pc_keyb.c

Re: BUG: pc_keyb.c

[Andries.Brouwer]

    From alan@lxorguk.ukuu.org.uk Tue Aug 21 00:48:32 2001

    > But the present code does not guarantee such a delay at all.
    > For example, kbd_write_cmd() does
    >     kb_wait();
    >     outb(...);
    >     kb_wait();
    > where the second kb_wait reads the status very quickly after the first.

    Thats wrong by the spec. I dug out my docs - there is a required 1mS (not
    2 tho) delay for keyboard port accesses.

Since there are various keyboard and mouse paths that can lead to
register access, it seems that we must either prefix each access
by a wait, or otherwise we must remember at what time we last did
a read.

Something else is that on some ancient (MCA) systems a delay is required
between finding the ready bit and actually reading the data
(Frank van Gilluwe, p. 273: wait 7 us).

On the other hand, 1 ms is a very long time these days; it is a bit
surprising that modern hardware should need delays in that order of
magnitude; maybe the problem is elsewhere and the mdelay(2) happens
to change the timing and avoid the problem.

Re: BUG: pc_keyb.c

[Alan Cox]

>   Due to writing to the status register before it's ready as far as I 
> can se.
> 
>   fix: change all mdelay(1) in pc_keyb.c to mdelay(2)'s .. ( mdelay(1) 
> will be on the timing limit.

Thats interesting. I thought the rule was 1mS. (There is indeed a timing
requirement). Any keyboard controller experts around ?

>   Might also be present during high load on machines running GL on AGP 
> video cards, not 100 % sure same symptoms, above seem's to fix ???? ( 
> strange ).

XFree86 touches they keyboard controller directly unless you rebuild it 
against a recent -ac kernel. This causes amusing problems with legacy free
PC's as well as the timing issue.

That might explain the other one

Re: BUG: pc_keyb.c

[Alan Cox]

> But the present code does not guarantee such a delay at all.
> For example, kbd_write_cmd() does
> 	kb_wait();
> 	outb(...);
> 	kb_wait();
> where the second kb_wait reads the status very quickly after the first.

Thats wrong by the spec. I dug out my docs - there is a required 1mS (not
2 tho) delay for keyboard port accesses.

Re: BUG: pc_keyb.c

[Alan Cox]

> On the other hand, 1 ms is a very long time these days; it is a bit
> surprising that modern hardware should need delays in that order of

I have two boxes that need the 1mS delay, and I actually had to fix a 
missing one to deal with hangs. So its still there, lurking in some boxes
I've had ex Digital engineers confirm the hinote needs the 1mS delay a while
back

Alan

Re: BUG: pc_keyb.c

[Andries.Brouwer]

>> Due to writing to the status register before it's ready as far as
>> I can see.

Which writes are you thinking of?

>> Fix: change all mdelay(1) in pc_keyb.c to mdelay(2)'s

There are four of them. Any one in particular?

(1) The first is

static void kb_wait(void) {
	do {
		status = handle_kbd_event();
		if (! (status & KBD_STAT_IBF))
			return;
		mdelay(1);
	} ...
}

Here handle_kbd_event() does kbd_read_status() which does
inb(KBD_STATUS_REG).
So, the mdelay(1) separates reads of the status register.

(2) The second is (in send_data, waiting for an ack):
	for (;;) {
		if (acknowledge)
			return 1;
		mdelay(1);
	}
Here the delay seems completely arbitrary.

(3) The third is in kbd_wait_for_input():
	do {
		int retval = kbd_read_data();
		if (retval >= 0)
			return retval;
		mdelay(1);
	}
Here kbd_read_data() does kbd_read_status().
Again the mdelay(1) separates reads of the status register.

(4) The last is in detect_auxiliary_port() which is __init
and hence probably irrelevant.


If it is really necessary to have mdelay(1) or even mdelay(2),
then it seems to me that this implies that there is a minimum
delay between two reads of the status register.
But the present code does not guarantee such a delay at all.
For example, kbd_write_cmd() does
	kb_wait();
	outb(...);
	kb_wait();
where the second kb_wait reads the status very quickly after the first.


So, I am inclined to think that the present mdelay(1) is just random
nonsense. It does not guarantee anything at all. If some delay is required
somewhere then we must introduce a mechanism that enforces the delay.

Andries

BUG: pc_keyb.c

[Lars Segerlund]

  Digital Hinote II will hang on kernals 2.4.X if ps2 mouse ( built in 
trackball is used ), not present in 2.0.38-39 kernels .... this will 
hapen after a bit of use, but is repeatable.

  Due to writing to the status register before it's ready as far as I 
can se.

  fix: change all mdelay(1) in pc_keyb.c to mdelay(2)'s .. ( mdelay(1) 
will be on the timing limit.
( /usr/src/linux/drivers/char/pc_keyb.c )

  Might also be present during high load on machines running GL on AGP 
video cards, not 100 % sure same symptoms, above seem's to fix ???? ( 
strange ).

  / best regards, Lars Segerlund.