From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jean-Christophe PLAGNIOL-VILLARD Date: Sun, 4 Jan 2009 08:47:02 +0100 Subject: [U-Boot] [PATCH] IDE: Improving speed on reading data Part 1/1 In-Reply-To: References: <49384704.sPrSOzo/CYqyo4zk%stefan.althoefer@web.de> <20081215230720.DBEA5832E8A1@gemini.denx.de> Message-ID: <20090104074702.GE6960@game.jcrosoft.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 22:22 Tue 16 Dec , Stefan Althoefer wrote: > IDE: Improving speed on reading data > > This patch improves the speed when reading blocks from > IDE devices by reading more than one block at a time. Up to > 128 blocks are requested in one read command. > > The ide_wait() code was rewritten to have lower latency > by polling more frequently for status. > > On my testplatform (Janz emPC-A400 with CompactFLASH card) > this nearly doubled speed. > > Also, error handling has been improved, so that ide_read does not > attempt to read beyond the last sector of the device. > > Signed-off-by: Stefan Althoefer > --- > > Can you please use git-format-patch to format the patch? I don't know > > how you created the diff, but it looks strange to me (and harldy > > readable). > > Sorry I wasn't fully aware of the wonderful world of git-* when > I prepared the first patches ... ok I'm still not (fully). > > >> - ide_outb (device, ATA_SECT_CNT, 1); > >> + scnt = (blkcnt > 128) ? 128 : blkcnt; > >> + ide_outb (device, ATA_SECT_CNT, scnt); > > > > What happens if you try to read at or beyond the end of the device? > > Good point. The old code wasn't very smart with this either (it did not > check for bounds but let the device decide). My code added a deadlock > to this behaviour (break did not work in a inner loop). > > I added some checks against block_dev_desc_t->lba to fix this. please send patch as a separate e-mail or put the comment which will not appear in the commit message after the --- > > common/cmd_ide.c | 63 ++++++++++++++++++++++++++++++++++------------------- > 1 files changed, 40 insertions(+), 23 deletions(-) > > diff --git a/common/cmd_ide.c b/common/cmd_ide.c > index dd62c87..ec64767 100644 > --- a/common/cmd_ide.c > +++ b/common/cmd_ide.c > @@ -1361,6 +1361,8 @@ ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer) > ulong n = 0; > unsigned char c; > unsigned char pwrsave=0; /* power save */ > + ulong scnt; > + lbaint_t blkrem; > #ifdef CONFIG_LBA48 > unsigned char lba48 = 0; > > @@ -1372,6 +1374,12 @@ ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer) > debug ("ide_read dev %d start %qX, blocks %lX buffer at %lX\n", > device, blknr, blkcnt, (ulong)buffer); > > + if( blknr >= ide_get_dev(device)->lba ){ please replace with if (blknr >= ide_get_dev(device)->lba) { > + printf ("IDE read: device %d read beyond last block\n", device); > + goto IDE_READ_E; > + } > + blkrem = ide_get_dev(device)->lba - blknr; > + > ide_led (DEVICE_LED(device), 1); /* LED on */ > > /* Select device > @@ -1405,7 +1413,7 @@ ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer) > } > > > - while (blkcnt-- > 0) { > + while (blkcnt > 0) { > > c = ide_wait (device, IDE_TIME_OUT); > > @@ -1427,7 +1435,9 @@ ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer) > #endif > } > #endif > - ide_outb (device, ATA_SECT_CNT, 1); > + scnt = (blkcnt > 128) ? 128 : blkcnt; > + scnt = (scnt > blkrem) ? blkrem : scnt; > + ide_outb (device, ATA_SECT_CNT, scnt); > ide_outb (device, ATA_LBA_LOW, (blknr >> 0) & 0xFF); > ide_outb (device, ATA_LBA_MID, (blknr >> 8) & 0xFF); > ide_outb (device, ATA_LBA_HIGH, (blknr >> 16) & 0xFF); > @@ -1446,32 +1456,39 @@ ulong ide_read (int device, lbaint_t blknr, ulong blkcnt, void *buffer) > ide_outb (device, ATA_COMMAND, ATA_CMD_READ); > } > > - udelay (50); > + while (scnt > 0) { > + udelay (50); > > - if(pwrsave) { > - c = ide_wait (device, IDE_SPIN_UP_TIME_OUT); /* may take up to 4 sec */ > - pwrsave=0; > - } else { > - c = ide_wait (device, IDE_TIME_OUT); /* can't take over 500 ms */ > - } > + if(pwrsave) { > + c = ide_wait (device, IDE_SPIN_UP_TIME_OUT); /* may take up to 4 sec */ > + pwrsave=0; please add a space before and after the '=' add be carefull of the 80 chars limit > + } else { > + c = ide_wait (device, IDE_TIME_OUT); /* can't take over 500 ms */ > + } > > - if ((c&(ATA_STAT_DRQ|ATA_STAT_BUSY|ATA_STAT_ERR)) != ATA_STAT_DRQ) { > + if ((c&(ATA_STAT_DRQ|ATA_STAT_BUSY|ATA_STAT_ERR)) != ATA_STAT_DRQ) { please add a space before and after the '&' and '|' > #if defined(CONFIG_SYS_64BIT_LBA) && defined(CONFIG_SYS_64BIT_VSPRINTF) > - printf ("Error (no IRQ) dev %d blk %qd: status 0x%02x\n", > - device, blknr, c); > + printf ("Error (no IRQ) dev %d blk %qd: status 0x%02x\n", > + device, blknr, c); > #else > - printf ("Error (no IRQ) dev %d blk %ld: status 0x%02x\n", > - device, (ulong)blknr, c); > + printf ("Error (no IRQ) dev %d blk %ld: status 0x%02x\n", > + device, (ulong)blknr, c); > #endif > - break; > - } > + goto IDE_READ_E; > + } > > - input_data (device, buffer, ATA_SECTORWORDS); > - (void) ide_inb (device, ATA_STATUS); /* clear IRQ */ > + input_data (device, buffer, ATA_SECTORWORDS); > + (void) ide_inb (device, ATA_STATUS); /* clear IRQ */ > > - ++n; > - ++blknr; > - buffer += ATA_BLOCKSIZE; > + ++n; > + ++blknr; > + --blkcnt; > + --blkrem; > + --scnt; > + buffer += ATA_BLOCKSIZE; > + } > + if (blkrem == 0) > + break; > } > IDE_READ_E: > ide_led (DEVICE_LED(device), 0); /* LED off */ > @@ -1607,11 +1624,11 @@ OUT: > */ > static uchar ide_wait (int dev, ulong t) > { > - ulong delay = 10 * t; /* poll every 100 us */ > + ulong delay = (1000/5) * t; /* poll every 5 us */ why not 200 instead? > uchar c; > > while ((c = ide_inb(dev, ATA_STATUS)) & ATA_STAT_BUSY) { > - udelay (100); > + udelay (5); > if (delay-- == 0) { > break; > } Best Regards, J.