[U-Boot] [PATCH] IDE: Improving speed on reading data Part 1/1

[PATCH] 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.
On my testplatform (Janz emPC-A400 with CompactFLASH card) this nearly doubled speed.
Also the ide_wait() code was rewritten to have lower latency by polling more frequently for status.
The patch is against "latest" u-boot git-repository
Please (still) be patient if style of submission or patches are offending.
Signed-off-by: Stefan Althoefer stefan.althoefer@web.de ----
diff -uprN u-boot-orig//common/cmd_ide.c u-boot/common/cmd_ide.c --- u-boot-orig//common/cmd_ide.c 2008-12-02 17:25:31.000000000 +0100 +++ u-boot/common/cmd_ide.c 2008-12-03 09:22:29.000000000 +0100 @@ -1302,6 +1361,7 @@ ulong ide_read (int device, lbaint_t blk ulong n = 0; unsigned char c; unsigned char pwrsave=0; /* power save */ + ulong scnt; #ifdef CONFIG_LBA48 unsigned char lba48 = 0;
@@ -1346,7 +1406,7 @@ ulong ide_read (int device, lbaint_t blk }
- while (blkcnt-- > 0) { + while (blkcnt > 0) {
c = ide_wait (device, IDE_TIME_OUT);
@@ -1368,7 +1428,8 @@ ulong ide_read (int device, lbaint_t blk #endif } #endif - ide_outb (device, ATA_SECT_CNT, 1); + scnt = (blkcnt > 128) ? 128 : blkcnt; + 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); @@ -1387,32 +1448,36 @@ ulong ide_read (int device, lbaint_t blk 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; + } 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) { #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; - } + break; + }
- input_data (device, buffer, ATA_SECTORWORDS); - (void) ide_inb (device, ATA_STATUS); /* clear IRQ */ - - ++n; - ++blknr; - buffer += ATA_BLOCKSIZE; + input_data (device, buffer, ATA_SECTORWORDS); + (void) ide_inb (device, ATA_STATUS); /* clear IRQ */ + + ++n; + ++blknr; + --blkcnt; + --scnt; + buffer += ATA_BLOCKSIZE; + } } IDE_READ_E: ide_led (DEVICE_LED(device), 0); /* LED off */ @@ -1548,11 +1613,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 */ uchar c;
while ((c = ide_inb(dev, ATA_STATUS)) & ATA_STAT_BUSY) { - udelay (100); + udelay (5); if (delay-- == 0) { break; }

Dear Stefan Althoefer,
In message 49384704.sPrSOzo/CYqyo4zk%stefan.althoefer@web.de you wrote:
[PATCH] 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.
On my testplatform (Janz emPC-A400 with CompactFLASH card) this nearly doubled speed.
Also the ide_wait() code was rewritten to have lower latency by polling more frequently for status.
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).
The patch is against "latest" u-boot git-repository
Please (still) be patient if style of submission or patches are offending.
Such comments must go below the "---" line
Signed-off-by: Stefan Althoefer stefan.althoefer@web.de
Thsi is the place to add comments that are not supposed to become part of the commit message.
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?
Best regards,
Wolfgang Denk

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 stefan.althoefer@web.de ---
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.
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 ){ + 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; + } 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) { #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 */ uchar c;
while ((c = ide_inb(dev, ATA_STATUS)) & ATA_STAT_BUSY) { - udelay (100); + udelay (5); if (delay-- == 0) { break; }

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 stefan.althoefer@web.de
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_LBA_LOW, (blknr >> 0) & 0xFF); ide_outb (device, ATA_LBA_MID, (blknr >> 8) & 0xFF); ide_outb (device, ATA_LBA_HIGH, (blknr >> 16) & 0xFF);ide_outb (device, ATA_SECT_CNT, scnt);
@@ -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);
if (delay-- == 0) { break; }udelay (5);
Best Regards, J.
participants (3)
-
Jean-Christophe PLAGNIOL-VILLARD
-
Stefan Althoefer
-
Wolfgang Denk