Subject: Bug fixes for Thinkpad 701c suspend/resume disk problem
To: None <port-i386@NetBSD.ORG>
From: Robert V. Baron <rvb@gluck.coda.cs.cmu.edu>
List: port-i386
Date: 04/02/1998 22:16:38
A while back some people reported problems with the the thinkpad 701c
disk after coming out of suspend/resume.  I will explain what is wrong
and give a patch for it against 1.3.  If you don't have the problem
you can stop reading now.

There are two problems with the 701c.  After a suspend resume the disk
will report and error and the WDCE_MCR status will be set.  This does
not get reported quite right, wdcwait returns WDCS_ERR where as it
normally reports -1 for an error.  This confuses some of the code and
is fixed under the ifdef tp701c conditional.  Another thing that
happens is that we quickly do an unwedge to get the disk back to
normal.  If you do not have a 701c, you don't need this.  I'm not sure
if it will ever cause problems.

The second problem is a general driver problem that will happen when
any kind of error condition occurs.  It is under the 
ifdef reincrement_blkno_on_error.  What is wrong with the logic is
that the wd keeps the both b_blkno and p_offset (offset to the start
of the partition) and essentially does a b_blkno += p_offset every
time it starts to process the io request.  When it restarts the io
after an error it does the += again and this is not good.  (Manuel:
please look at this.)

Finally there are some typos fixed in wdc.c when WDDEBUG is enabled.

Index: wd.c
===================================================================
RCS file: /afs/cs/project/netbsd/cvs/src/sys/dev/isa/wd.c,v
retrieving revision 1.8
diff -c -r1.8 wd.c
*** wd.c	1998/03/24 17:52:21	1.8
--- wd.c	1998/04/02 14:35:30
***************
*** 311,321 ****
--- 311,330 ----
  
  		xfer->d_link = d_link;
  		xfer->c_bp = bp;
+ #ifdef	reincrement_blkno_on_errro
  		xfer->c_p_offset = p_offset;
+ #endif
  		xfer->databuf = bp->b_data;
  		xfer->c_bcount = bp->b_bcount;
  		xfer->c_flags |= bp->b_flags & (B_READ|B_WRITE);
+ #ifdef	reincrement_blkno_on_error
  		xfer->c_blkno = bp->b_blkno;
+ #else
+ 		{ daddr_t blkno = bp->b_blkno + p_offset;
+ 		  blkno /= (wd->d_link->sc_lp->d_secsize / DEV_BSIZE);
+ 		  xfer->c_blkno = blkno;
+ 		}
+ #endif
  
  		/* Instrumentation. */
  		disk_busy(&wd->sc_dk);
Index: wdc.c
===================================================================
RCS file: /afs/cs/project/netbsd/cvs/src/sys/dev/isa/wdc.c,v
retrieving revision 1.1.1.1
diff -c -r1.1.1.1 wdc.c
*** wdc.c	1997/12/24 12:28:11	1.1.1.1
--- wdc.c	1998/04/02 15:06:35
***************
*** 64,69 ****
--- 64,71 ----
  #include "atapibus.h"
  #include "wd.h"
  
+ #define	tp701
+ 
  #if NATAPIBUS > 0
  #include <dev/scsipi/scsipi_all.h>
  #include <dev/scsipi/atapiconf.h>
***************
*** 129,135 ****
  #endif
  
  #ifdef WDDEBUG
! #define WDDEBUG_PRINT(args)	printf args
  #else
  #define WDDEBUG_PRINT(args)
  #endif
--- 131,138 ----
  #endif
  
  #ifdef WDDEBUG
! int wddebug = 0;
! #define WDDEBUG_PRINT(args)	if (wddebug < 0) printf args
  #else
  #define WDDEBUG_PRINT(args)
  #endif
***************
*** 438,452 ****
--- 441,459 ----
  
  	/* When starting a transfer... */
  	if (xfer->c_skip == 0) {
+ #ifdef	reincrement_blkno_on_error
  		daddr_t blkno;
+ #endif
  
  		WDDEBUG_PRINT(("\n%s: wdc_ata_start %s %d@%d; map ",
  		    wdc->sc_dev.dv_xname,
  		    (xfer->c_flags & B_READ) ? "read" : "write",
  		    xfer->c_bcount, xfer->c_blkno));
  
+ #ifdef	reincrement_blkno_on_error
  		blkno = xfer->c_blkno+xfer->c_p_offset;
  		xfer->c_blkno = blkno / (d_link->sc_lp->d_secsize / DEV_BSIZE);
+ #endif
  	} else {
  		WDDEBUG_PRINT((" %d)%x", xfer->c_skip,
  		    inb(wdc->sc_iobase + wd_altsts)));
***************
*** 563,575 ****
  		/* Initiate command! */
  		if (wdccommand(wdc, d_link, command, d_link->drive,
  		    cylin, head, sector, nblks) != 0) {
  			wderror(d_link, NULL,
  			    "wdc_ata_start: timeout waiting for unbusy");
  			wdcunwedge(wdc);
  			return;
  		}
  
! 		WDDEBUG_PRINT(("sector %d cylin %d head %d addr %x sts %x\n",
  		    sector, cylin, head, xfer->databuf,
  		    inb(wdc->sc_iobase + wd_altsts)));
  
--- 570,585 ----
  		/* Initiate command! */
  		if (wdccommand(wdc, d_link, command, d_link->drive,
  		    cylin, head, sector, nblks) != 0) {
+ #ifdef	tp701
+ 			if (!wdc->sc_status)
+ #endif
  			wderror(d_link, NULL,
  			    "wdc_ata_start: timeout waiting for unbusy");
  			wdcunwedge(wdc);
  			return;
  		}
  
! 		WDDEBUG_PRINT(("sector %ld cylin %ld head %ld addr %p sts %x\n",
  		    sector, cylin, head, xfer->databuf,
  		    inb(wdc->sc_iobase + wd_altsts)));
  
***************
*** 619,626 ****
--- 629,641 ----
  	d_link = xfer->d_link;
  
  	if (wait_for_unbusy(wdc) < 0) {
+ #ifdef	tp701
+ 		if (!wdc->sc_error)
+ 		wdcerror(wdc, "wdcintr: timeout waiting for unbusy");
+ #else
  		wdcerror(wdc, "wdcintr: timeout waiting for unbusy");
  		return 0;
+ #endif
  	}
  
  	wdc->sc_flags &= ~WDCF_IRQ_WAIT;
***************
*** 633,639 ****
  			return 1;
  		}
  		WDDEBUG_PRINT(("wdc_ata_start from wdc_ata_intr(open) flags 0x%x\n",
! 		    dc->sc_flags));
  		wdc_ata_start(wdc,xfer);
  		return 1;
  	}
--- 648,654 ----
  			return 1;
  		}
  		WDDEBUG_PRINT(("wdc_ata_start from wdc_ata_intr(open) flags 0x%x\n",
! 		    wdc->sc_flags));
  		wdc_ata_start(wdc,xfer);
  		return 1;
  	}
***************
*** 647,652 ****
--- 662,675 ----
  #ifdef WDDEBUG
  		wderror(d_link, NULL, "wdc_ata_start");
  #endif
+ #ifdef	tp701
+ 		if (wdc->sc_error & WDCE_MCR) {
+ 			wdcerror(wdc, "wd: oops");
+ 			wdcunwedge(wdc);
+ 			wdc->sc_flags &= ~(WDCF_ERROR);
+ 			return 1;
+ 		}
+ #endif
  		if ((wdc->sc_flags & WDCF_SINGLE) == 0) {
  			wdc->sc_flags |= WDCF_ERROR;
  			goto restart;
***************
*** 1073,1078 ****
--- 1096,1104 ----
  #ifdef WDCNDELAY_DEBUG
  	extern int cold;
  #endif
+ #ifdef	tp701
+ 	wdc->sc_error = 0;
+ #endif
  
  	WDDEBUG_PRINT(("wdcwait iobase %x\n", iobase));
  
***************
*** 1099,1105 ****
--- 1125,1138 ----
  	}
  	if (status & WDCS_ERR) {
  		wdc->sc_error = inb(iobase+wd_error);
+ #ifdef	tp701
+ 		if (wdc->sc_error & WDCE_MCR) {
+ 			wdcerror(wdc, "wd: media change");
+ 		}
+ 		return -1;
+ #else
  		return WDCS_ERR;
+ #endif
  	}
  #ifdef WDCNDELAY_DEBUG
  	/* After autoconfig, there should be no long delays. */
***************
*** 1240,1246 ****
  	s = splbio();
  
  	/* insert at the end of command list */
! 	TAILQ_INSERT_TAIL(&wdc->sc_xfer,xfer , c_xferchain)
  	WDDEBUG_PRINT(("wdcstart from wdc_exec_xfer, flags 0x%x\n",
  	    wdc->sc_flags));
  	wdcstart(wdc);
--- 1273,1279 ----
  	s = splbio();
  
  	/* insert at the end of command list */
! 	TAILQ_INSERT_TAIL(&wdc->sc_xfer,xfer , c_xferchain);
  	WDDEBUG_PRINT(("wdcstart from wdc_exec_xfer, flags 0x%x\n",
  	    wdc->sc_flags));
  	wdcstart(wdc);
Index: wdlink.h
===================================================================
RCS file: /afs/cs/project/netbsd/cvs/src/sys/dev/isa/wdlink.h,v
retrieving revision 1.3
diff -c -r1.3 wdlink.h
*** wdlink.h	1998/03/25 20:26:56	1.3
--- wdlink.h	1998/04/02 14:35:47
***************
*** 122,128 ****
--- 122,130 ----
  	int c_skip;		/* bytes already transferred */
  	int c_nblks;		/* number of blocks currently transferring */
  	int c_nbytes;		/* number of bytes currently transferring */
+ #ifdef	reincrement_blkno_on_error
  	u_long c_p_offset;	/* offset of the partition */
+ #endif
  	TAILQ_ENTRY(wdc_xfer) c_xferchain;
  	LIST_ENTRY(wdc_xfer) free_list;
  };