Possible error in IDE code.....

Possible error in IDE code.....

[Donald Walton]

In adding IDE support to my application, the software was not able to 
read the MBR properly.  It would read one byte and the rest of the 
buffer would be zeroes.

I tracked the problem to file - ide_disk.c, version 1.3, function - 
ide_read_sector.  One of the parameters passed in is len.  It unclear 
whether this is the buffer length or number of sectors to be read.  
However, a value of 1 is passed in for the length and only one byte is read.

Correcting the problem is not a big deal but I would like to know 
whether you meant for the length to be sector count or byte count.

Thanks for the great work you have done.

Sincerely,
Don Walton

Re: Possible error in IDE code.....

[Jonathan Larmour]

Donald Walton wrote:
> In adding IDE support to my application, the software was not able to 
> read the MBR properly.  It would read one byte and the rest of the 
> buffer would be zeroes.
> 
> I tracked the problem to file - ide_disk.c, version 1.3, function - 
> ide_read_sector.  One of the parameters passed in is len.  It unclear 
> whether this is the buffer length or number of sectors to be read.  
> However, a value of 1 is passed in for the length and only one byte is 
> read.
> 
> Correcting the problem is not a big deal but I would like to know 
> whether you meant for the length to be sector count or byte count.

It's meant to be the sector count (with 512-byte sectors). It's my bad 
because I had updated this package after the CYGPKG_IO_DISK package changed 
in this regard, but I evidently missed this bit.

I've checked in the below change. Update your CVS and try it out.

Jifl


Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/disk/ide/current/ChangeLog,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -5 -p -r1.3 -r1.4
--- ChangeLog   21 Sep 2006 16:36:24 -0000      1.3
+++ ChangeLog   17 Nov 2006 18:04:43 -0000      1.4
@@ -1,5 +1,11 @@
+2006-11-17  Jonathan Larmour  <jifl@eCosCentric.com>
+
+       * src/ide_disk.c (ide_read_sector, ide_write_sector): Length
+       is counted in sectors now, not bytes (due to change in io/disk
+       API).
+
  2006-09-21  Jonathan Larmour  <jifl@eCosCentric.com>

         * src/ide_disk.h: DISK_FUNS is now implicitly static.
         (IDE_DISK_INSTANCE): Reflect updated io/disk API by using
         ide_disk_controller.
Index: src/ide_disk.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/disk/ide/current/src/ide_disk.c,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -5 -p -r1.3 -r1.4
--- src/ide_disk.c      21 Sep 2006 16:36:24 -0000      1.3
+++ src/ide_disk.c      17 Nov 2006 18:04:43 -0000      1.4
@@ -262,12 +262,12 @@ ide_read_sector(int ctlr, int dev, cyg_u
      // but who knows
      //
      for (j = 0, c=0 ; j < (CYGDAT_DEVS_DISK_IDE_SECTOR_SIZE / 
sizeof(cyg_uint16));
           j++) {
          HAL_IDE_READ_UINT16(ctlr, IDE_REG_DATA, p);
-        if (c++<len) *b++=p&0xff;
-        if (c++<len) *b++=(p>>8)&0xff;
+        if (c++<(len*512)) *b++=p&0xff;
+        if (c++<(len*512)) *b++=(p>>8)&0xff;
      }
      return 1;
  }

  static int
@@ -296,12 +296,12 @@ ide_write_sector(int ctlr, int dev, cyg_
      // It would be fine if all buffers were word aligned,
      // but who knows
      //
      for (j = 0, c=0 ; j < (CYGDAT_DEVS_DISK_IDE_SECTOR_SIZE / 
sizeof(cyg_uint16));
           j++) {
-        p = (c++<len) ? *b++ : 0;
-        p |= (c++<len) ? (*b++<<8) : 0;
+        p = (c++<(len*512)) ? *b++ : 0;
+        p |= (c++<(len*512)) ? (*b++<<8) : 0;
          HAL_IDE_WRITE_UINT16(ctlr, IDE_REG_DATA, p);
      }
      return 1;
  }


-- 
eCosCentric    http://www.eCosCentric.com/    The eCos and RedBoot experts
------["The best things in life aren't things."]------      Opinions==mine