From: Andries.Brouwer@cwi.nl
To: alan@lxorguk.ukuu.org.uk, lars.segerlund@comsys.se
Cc: Andries.Brouwer@cwi.nl, linux-kernel@vger.kernel.org
Subject: Re: BUG: pc_keyb.c
Date: Mon, 20 Aug 2001 19:46:06 GMT [thread overview]
Message-ID: <200108201946.TAA179233@vlet.cwi.nl> (raw)
>> 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
next reply other threads:[~2001-08-20 19:46 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2001-08-20 19:46 Andries.Brouwer [this message]
-- strict thread matches above, loose matches on Subject: below --
2001-08-21 15:45 BUG: pc_keyb.c Andries.Brouwer
[not found] <no.id>
2001-08-20 10:26 ` Alan Cox
2001-08-20 22:51 ` Alan Cox
2001-08-21 15:51 ` Alan Cox
2001-08-20 5:57 Lars Segerlund
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200108201946.TAA179233@vlet.cwi.nl \
--to=andries.brouwer@cwi.nl \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=lars.segerlund@comsys.se \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).