[LinuxBIOS] filo ide speedup patch!

[Peter Stuge]

Hi,

On Fri, Apr 06, 2007 at 10:51:00AM -0400, Ward Vandewege wrote:
> > Fresh LB buildtarget and make after rebuilding FILO still uses
> > the old payload file.. How can that be?
> 
> I don't think that was the case - perhaps the patch just didn't fix
> the problem?

Of course you are right. Sorry about doubting that.


After having processed the read command, it seems the hard disk
reports "not busy" before "there is data" but that's the wrong order
according to the ATA-3 working draft that I'm using for reference.
(From 1997 but it's the latest I've found available at no cost. T13
d2008r7b)

The print_status function read the status register again, and by then
the DRQ ("data request") bit was set, causing my confusion.

I've added a wait for DRQ and the timeout should not be too long
because the same code is used to detect that no disk is attached.
I've used 50ms.

The attached patch should apply over the last full patch.


The FIXME I've added is also from reviewing the PIO protocol in the
same draft. It seems the real thing to wait for is not 50ms but
rather BSY=0&RDY=1 after having written the device register. This
could make a big difference also for ATAPI and filesystems doing
single sector reads, but I don't want to change it until multi sector
reads are working.


> > Ok, I've added a little more debugging output and this patch also
> > contains full error checking.
> 
> Output attached; I assume you wanted me to patch this onto a fresh
> filo checkout.

Yes, thanks.


> Seems like we regressed to the original problem.

It's still the same problem, but now the error is reported rather
than an attempt made to start an incomplete kernel.

Another thing that needs to be investigated is why FILO reads
4096+1380 bytes from the disk _one byte at a time_ before the menu
timeou, but that's also for later.


//Peter
-------------- next part --------------
--- drivers/ide.c.org	2007-04-06 00:08:39.000000000 +0200
+++ drivers/ide.c	2007-04-06 20:17:01.000000000 +0200
@@ -254,6 +254,7 @@
  * So if any IDE commands takes this long we know we have problems.
  */
 #define IDE_TIMEOUT (32*TICKS_PER_SEC)
+#define IDE_TIMEOUT_50ms (TICKS_PER_SEC/20)
 
 static int not_bsy(struct controller *ctrl)
 {
@@ -261,6 +262,12 @@
 	return !(ctrl->stat & IDE_STATUS_BSY);
 }
 
+static int not_bsy_and_rdy(struct controller *ctrl)
+{
+	ctrl->stat=inb(IDE_REG_STATUS(ctrl));
+	return (!(ctrl->stat & IDE_STATUS_BSY)) && (ctrl->stat & IDE_STATUS_RDY);
+}
+
 /* IDE drives assert BSY bit within 400 nsec when SRST is set.
  * Use 2 msec since our tick is 1 msec */
 #define IDE_RESET_PULSE (2*TICKS_PER_SEC / 1000)
@@ -271,6 +278,12 @@
 	return ctrl->stat & IDE_STATUS_BSY;
 }
 
+static int drq(struct controller *ctrl)
+{
+	ctrl->stat=inb(IDE_REG_STATUS(ctrl));
+	return ctrl->stat & IDE_STATUS_DRQ;
+}
+
 #if  !BSY_SET_DURING_SPINUP
 static int timeout(struct controller *ctrl)
 {
@@ -333,6 +346,10 @@
 		 * amount of time to use here.
 		 */
 		mdelay(50); 
+		/*
+		 * FIXME: change mdelay(50) to:
+		 * await_ide(not_bsy_and_rdy, ctrl, currticks() + IDE_TIMEOUT_50ms);
+		 */
 	}
 	outb(cmd->feature,         IDE_REG_FEATURE(ctrl));
 	if (cmd->command == IDE_CMD_READ_SECTORS_EXT) {
@@ -395,11 +412,17 @@
 			count = harddisk_info[drive].hw_sector_size;
 		if (await_ide(not_bsy, ctrl, currticks() + IDE_TIMEOUT) < 0)
 			return -1;
-		if (!(ctrl->stat & IDE_STATUS_DRQ)) {
-			debug("no drq before insw\n");
+		if (ctrl->stat & (IDE_STATUS_WFT | IDE_STATUS_ERR)) {
+			debug("IDE error before insw()\n");
 			print_status(ctrl);
 			return -1;
 		}
+		if (!(ctrl->stat & IDE_STATUS_DRQ))
+			if (await_ide(drq, ctrl, currticks() + IDE_TIMEOUT_50ms) < 0) {
+				debug("No error after BSY=0 and no DRQ after 50ms\n");
+				print_status(ctrl);
+				return -1;
+			}
 		insw(IDE_REG_DATA(ctrl), buffer, count/2);
 		buffer += count;
 		bytes -= count;