Hi Kent,
On 14/01/13(Mon) 10:05, Kent Fritz wrote:
> On Fri, Jan 11, 2013 at 5:29 PM, Stefan Sperling <s...@openbsd.org> wrote:
> > I see. So this is happening during pms_probe() which runs before the
> > protocol is selected. Maybe fix it like this? I think the code should
> > cope with hardware that returns unrecognizable garbage. But I don't
> > know very much about PS/2.
> >
> > Thanks for pinning down the problem!
> >
> > Index: pckbc.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/ic/pckbc.c,v
> > retrieving revision 1.31
> > diff -u -p -r1.31 pckbc.c
> > --- pckbc.c 17 Oct 2012 19:16:10 -0000 1.31
> > +++ pckbc.c 12 Jan 2013 01:25:41 -0000
> > @@ -620,6 +620,11 @@ pckbc_poll_cmd1(struct pckbc_internal *t
> > #ifdef PCKBCDEBUG
> > printf("pckbc_cmd: lost 0x%x\n", c);
> > #endif
> > + /* Don't retry cmd forever. */
> > + if (cmd->retries++ >= 5) {
> > + cmd->status = EIO;
> > + return;
> > + }
> > }
> >
> > while (cmd->responseidx < cmd->responselen) {
>
> That patch works fine. Tested on i386 on nT-i1250. Thanks for
> pointing me in the right direction!
Could you try the diff below and tell me if it also fix your problem? I
believe it's better to handle the bat failure core (0xfc) like we
already do with the bat completion code rather than exiting for any
value..
M.
Index: pckbc.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/ic/pckbc.c,v
retrieving revision 1.31
diff -u -p -r1.31 pckbc.c
--- pckbc.c 17 Oct 2012 19:16:10 -0000 1.31
+++ pckbc.c 11 Feb 2013 10:24:49 -0000
@@ -48,6 +48,12 @@
#include <dev/pckbc/pckbdvar.h>
#endif
+#ifdef PCKBCDEBUG
+#define DPRINTF(x...) do { printf(x); } while (0);
+#else
+#define DPRINTF(x...)
+#endif
+
/* descriptor for one device command */
struct pckbc_devcmd {
TAILQ_ENTRY(pckbc_devcmd) next;
@@ -102,9 +108,10 @@ int pckbcintr_internal(struct pckbc_inte
const char *pckbc_slot_names[] = { "kbd", "aux" };
-#define KBC_DEVCMD_ACK 0xfa
-#define KBC_DEVCMD_RESEND 0xfe
-#define KBC_DEVCMD_BAT 0xaa
+#define KBC_DEVCMD_ACK 0xfa
+#define KBC_DEVCMD_RESEND 0xfe
+#define KBC_DEVCMD_BAT_DONE 0xaa
+#define KBC_DEVCMD_BAT_FAIL 0xfc
#define KBD_DELAY DELAY(8)
@@ -587,39 +594,32 @@ pckbc_poll_cmd1(struct pckbc_internal *t
break;
}
- if (c == KBC_DEVCMD_ACK) {
+ switch (c) {
+ case KBC_DEVCMD_ACK:
cmd->cmdidx++;
continue;
- }
/*
* Some legacy free PCs keep returning Basic Assurance Test
* (BAT) instead of something usable, so fail gracefully.
*/
- if (c == KBC_DEVCMD_RESEND || c == KBC_DEVCMD_BAT) {
-#ifdef PCKBCDEBUG
- printf("pckbc_cmd: %s\n",
+ case KBC_DEVCMD_RESEND:
+ case KBC_DEVCMD_BAT_DONE:
+ case KBC_DEVCMD_BAT_FAIL:
+ DPRINTF("pckbc_cmd: %s\n",
c == KBC_DEVCMD_RESEND ? "RESEND": "BAT");
-#endif
if (cmd->retries++ < 5)
continue;
- else {
-#ifdef PCKBCDEBUG
- printf("pckbc: cmd failed\n");
-#endif
- cmd->status = ENXIO;
- return;
- }
- }
- if (c == -1) {
-#ifdef PCKBCDEBUG
- printf("pckbc_cmd: timeout\n");
-#endif
+
+ DPRINTF("pckbc_cmd: cmd failed\n");
+ cmd->status = ENXIO;
+ return;
+ case -1:
+ DPRINTF("pckbc_cmd: timeout\n");
cmd->status = EIO;
return;
+ default:
+ DPRINTF("pckbc_cmd: lost 0x%x\n", c);
}
-#ifdef PCKBCDEBUG
- printf("pckbc_cmd: lost 0x%x\n", c);
-#endif
}
while (cmd->responseidx < cmd->responselen) {
