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 <[email protected]>
> ---
> > 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.
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot
