[PATCH] (ata.mod) Fix ATAPI protocol

[PATCH] (ata.mod) Fix ATAPI protocol

[Christian Franke]

[-- Attachment #1: Type: text/plain, Size: 884 bytes --]

This patch fixes the data I/O protocol of the ATA PACKET command.

The current implementation does not read the byte count registers. ATAPI 
read may not work if the drive sends the data in more than one block.

In conjunction with my recent SCSI blocksize patch, ATAPI CD/DVD drives 
should now work.

Christian

2008-01-18  Christian Franke  <franke@computer.org>

	* disk/ata.c (GRUB_ATAPI_REG_*): New defines.
	(GRUB_ATAPI_IREASON_*): Likewise.
	(grub_ata_pio_write): Fix timeout error return.
	(grub_atapi_wait_drq): New function.
	(grub_atapi_packet): New parameter `size'.
	Use grub_atapi_wait_drq () and direct write instead of
	grub_ata_pio_write ().
	(grub_atapi_read): Replace grub_ata_pio_read () by a loop which
	reads the number of bytes requested by the device for each DRQ
	assertion.
	(grub_atapi_write): Remove old implementation, return not
	implemented instead.



[-- Attachment #2: grub2-atapi-fix.patch --]
[-- Type: text/x-diff, Size: 6250 bytes --]

diff --git a/disk/ata.c b/disk/ata.c
index af7158c..bb891d4 100644
--- a/disk/ata.c
+++ b/disk/ata.c
@@ -44,12 +44,15 @@ static const int grub_ata_ioaddress2[] = { 0x3f6, 0x376 };
 #define GRUB_ATA_REG_ERROR	1
 #define GRUB_ATA_REG_FEATURES	1
 #define GRUB_ATA_REG_SECTORS	2
+#define GRUB_ATAPI_REG_IREASON	2
 #define GRUB_ATA_REG_SECTNUM	3
 #define GRUB_ATA_REG_CYLLSB	4
 #define GRUB_ATA_REG_CYLMSB	5
 #define GRUB_ATA_REG_LBALOW	3
 #define GRUB_ATA_REG_LBAMID	4
+#define GRUB_ATAPI_REG_CNTLOW	4
 #define GRUB_ATA_REG_LBAHIGH	5
+#define GRUB_ATAPI_REG_CNTHIGH	5
 #define GRUB_ATA_REG_DISK	6
 #define GRUB_ATA_REG_CMD	7
 #define GRUB_ATA_REG_STATUS	7
@@ -65,6 +68,13 @@ static const int grub_ata_ioaddress2[] = { 0x3f6, 0x376 };
 #define GRUB_ATA_STATUS_READY	0x40
 #define GRUB_ATA_STATUS_BUSY	0x80
 
+/* ATAPI interrupt reason values (I/O, D/C bits).  */
+#define GRUB_ATAPI_IREASON_MASK     0x3
+#define GRUB_ATAPI_IREASON_DATA_OUT 0x0
+#define GRUB_ATAPI_IREASON_CMD_OUT  0x1
+#define GRUB_ATAPI_IREASON_DATA_IN  0x2
+#define GRUB_ATAPI_IREASON_ERROR    0x3
+
 enum grub_ata_commands
   {
     GRUB_ATA_CMD_READ_SECTORS = 0x20,
@@ -229,7 +239,7 @@ grub_ata_pio_write (struct grub_ata_device *dev, char *buf,
 
   /* Wait until drive is ready to read data.  */
   if (grub_ata_wait_status (dev, GRUB_ATA_STATUS_DRQ, 0, milliseconds))
-    return 0;
+    return grub_errno;
 
   /* Write the data, word by word.  */
   for (i = 0; i < size / 2; i++)
@@ -300,21 +310,65 @@ grub_atapi_identify (struct grub_ata_device *dev)
 }
 
 static grub_err_t
-grub_atapi_packet (struct grub_ata_device *dev, char *packet)
+grub_atapi_wait_drq (struct grub_ata_device *dev,
+		     grub_uint8_t ireason,
+		     int milliseconds)
 {
+  grub_millisleep (1); /* ATA allows 400ns to assert BSY.  */
+
+  if (grub_ata_wait_status (dev, 0, GRUB_ATA_STATUS_BUSY, milliseconds))
+    return grub_errno;
+
+  grub_uint8_t sts = grub_ata_regget (dev, GRUB_ATA_REG_STATUS);
+  grub_uint8_t irs = grub_ata_regget (dev, GRUB_ATAPI_REG_IREASON);
+
+  /* OK if DRQ is asserted and interrupt reason is as expected.  */
+  if ((sts & GRUB_ATA_STATUS_DRQ)
+      && (irs & GRUB_ATAPI_IREASON_MASK) == ireason)
+    return GRUB_ERR_NONE;
+
+  /* No DRQ implies error condition.  */
+  grub_dprintf("ata", "atapi error: status=0x%x, ireason=0x%x, error=0x%x\n",
+	       sts, irs, grub_ata_regget (dev, GRUB_ATA_REG_ERROR));
+
+  if (! (sts & GRUB_ATA_STATUS_DRQ)
+      && (irs & GRUB_ATAPI_IREASON_MASK) == GRUB_ATAPI_IREASON_ERROR)
+    {
+      if (ireason == GRUB_ATAPI_IREASON_CMD_OUT)
+	return grub_error (GRUB_ERR_READ_ERROR, "ATA PACKET command error");
+      else
+	return grub_error (GRUB_ERR_READ_ERROR, "ATAPI read error");
+    }
+
+  return grub_error (GRUB_ERR_READ_ERROR, "ATAPI protocol error");
+}
+
+static grub_err_t
+grub_atapi_packet (struct grub_ata_device *dev, char *packet,
+		   grub_size_t size)
+{
+  if (grub_ata_wait_status(dev, 0, GRUB_ATA_STATUS_BUSY, GRUB_ATA_TOUT_STD))
+    return grub_errno;
+
+  /* Send ATA PACKET command.  */
   grub_ata_regset (dev, GRUB_ATA_REG_DISK, dev->device << 4);
   grub_ata_regset (dev, GRUB_ATA_REG_FEATURES, 0);
-  grub_ata_regset (dev, GRUB_ATA_REG_SECTORS, 0);
-  grub_ata_regset (dev, GRUB_ATA_REG_LBAHIGH, 0xFF);
-  grub_ata_regset (dev, GRUB_ATA_REG_LBAMID, 0xFF);
+  grub_ata_regset (dev, GRUB_ATAPI_REG_IREASON, 0);
+  grub_ata_regset (dev, GRUB_ATAPI_REG_CNTHIGH, size >> 8);
+  grub_ata_regset (dev, GRUB_ATAPI_REG_CNTLOW, size & 0xFF);
 
   grub_ata_regset (dev, GRUB_ATA_REG_CMD, GRUB_ATA_CMD_PACKET);
 
-  grub_ata_wait ();
-
-  if (grub_ata_pio_write (dev, packet, 12, GRUB_ATA_TOUT_STD))
+  /* Wait for !BSY, DRQ, !I/O, C/D.  */
+  if (grub_atapi_wait_drq (dev, GRUB_ATAPI_IREASON_CMD_OUT, GRUB_ATA_TOUT_STD))
     return grub_errno;
 
+  /* Write the packet.  */
+  grub_uint16_t *buf16 = (grub_uint16_t *) packet;
+  unsigned i;
+  for (i = 0; i < 12 / 2; i++)
+    grub_outw (grub_cpu_to_le16 (buf16[i]), dev->ioaddress + GRUB_ATA_REG_DATA);
+
   return GRUB_ERR_NONE;
 }
 
@@ -847,27 +901,51 @@ grub_atapi_read (struct grub_scsi *scsi,
 {
   struct grub_ata_device *dev = (struct grub_ata_device *) scsi->data;
 
-  if (grub_atapi_packet (dev, cmd))
+  grub_dprintf("ata", "grub_atapi_read (size=%u)\n", size);
+
+  if (grub_atapi_packet (dev, cmd, size))
     return grub_errno;
 
-  grub_ata_wait (); /* XXX */
+  grub_size_t nread = 0;
+  while (nread < size)
+    {
+      /* Wait for !BSY, DRQ, I/O, !C/D.  */
+      if (grub_atapi_wait_drq (dev, GRUB_ATAPI_IREASON_DATA_IN, GRUB_ATA_TOUT_DATA))
+	return grub_errno;
+
+      /* Get byte count for this DRQ assertion.  */
+      unsigned cnt = grub_ata_regget (dev, GRUB_ATAPI_REG_CNTHIGH) << 8
+		   | grub_ata_regget (dev, GRUB_ATAPI_REG_CNTLOW);
+      grub_dprintf("ata", "DRQ count=%u\n", cnt);
+
+      /* Count of last transfer may be uneven.  */
+      if (! (0 < cnt && cnt <= size - nread && (! (cnt & 1) || cnt == size - nread)))
+	return grub_error (GRUB_ERR_READ_ERROR, "Invalid ATAPI transfer count");
+
+      /* Read the data.  */
+      grub_uint16_t *buf16 = (grub_uint16_t *) (buf + nread);
+      unsigned i;
+      for (i = 0; i < cnt / 2; i++)
+	buf16[i] = grub_le_to_cpu16 (grub_inw (dev->ioaddress + GRUB_ATA_REG_DATA));
 
-  return grub_ata_pio_read (dev, buf, size, GRUB_ATA_TOUT_DATA);
+      if (cnt & 1)
+	buf[nread + cnt - 1] = (char) grub_le_to_cpu16 (grub_inw (dev->ioaddress + GRUB_ATA_REG_DATA));
+
+      nread += cnt;
+    }
+
+  return GRUB_ERR_NONE;
 }
 
 static grub_err_t
-grub_atapi_write (struct grub_scsi *scsi,
+grub_atapi_write (struct grub_scsi *scsi __attribute__((unused)),
 		  grub_size_t cmdsize __attribute__((unused)),
-		  char *cmd, grub_size_t size, char *buf)
+		  char *cmd __attribute__((unused)),
+		  grub_size_t size __attribute__((unused)),
+		  char *buf __attribute__((unused)))
 {
-  struct grub_ata_device *dev = (struct grub_ata_device *) scsi->data;
-
-  if (grub_atapi_packet (dev, cmd))
-    return grub_errno;
-
-  grub_ata_wait (); /* XXX */
-
-  return grub_ata_pio_write (dev, buf, size, GRUB_ATA_TOUT_DATA);
+  // XXX: scsi.mod does not use write yet.
+  return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, "ATAPI write not implemented");
 }
 
 static grub_err_t

Re: [PATCH] (ata.mod) Fix ATAPI protocol

[Marco Gerards]

Christian Franke <Christian.Franke@t-online.de> writes:

> This patch fixes the data I/O protocol of the ATA PACKET command.
>
> The current implementation does not read the byte count
> registers. ATAPI read may not work if the drive sends the data in more
> than one block.
>
> In conjunction with my recent SCSI blocksize patch, ATAPI CD/DVD
> drives should now work.

Sorry for not commenting on many things on this list.  I am too busy
to check all this.  Perhaps my USB code might help in some occasions
for scsi.c, I am not completely sure if they are in sync.  I sent in
the USB code earlier.  This code can be committed, although endianess
is not handled correctly at all places.

--
Marco

Re: [PATCH] (ata.mod) Fix ATAPI protocol

[Christian Franke]

Marco Gerards wrote:
> Christian Franke <...> writes:
> 
> > This patch fixes the data I/O protocol of the ATA PACKET command.
> > 
> > The current implementation does not read the byte count
> > registers. ATAPI read may not work if the drive sends the data in
> > more than one block.
> > 
> > In conjunction with my recent SCSI blocksize patch, ATAPI CD/DVD
> > drives should now work.
> > 
> 
> Sorry for not commenting on many things on this list.  I am too busy
> to check all this.  

N.P.

Meantime, I have tested the SCSI and ATAPI fixes on several PC. If there
are no further comments, I will commit these soon.

Christian

Re: [PATCH] (ata.mod) Fix ATAPI protocol

[Christian Franke]

Christian Franke wrote:
> Marco Gerards wrote:
>   
>> Christian Franke <...> writes:
>>
>>     
>>> This patch fixes the data I/O protocol of the ATA PACKET command.
>>>
>>> The current implementation does not read the byte count
>>> registers. ATAPI read may not work if the drive sends the data in
>>> more than one block.
>>>
>>> In conjunction with my recent SCSI blocksize patch, ATAPI CD/DVD
>>> drives should now work.
>>>
>>>       
>> Sorry for not commenting on many things on this list.  I am too busy
>> to check all this.  
>>     
>
> N.P.
>
> Meantime, I have tested the SCSI and ATAPI fixes on several PC. If there
> are no further comments, I will commit these soon.
>
>   

Committed, plus an additional wait in grub_atapi_identify.

Open issues:
- grub_pio_read/write() check the ERR bit without ensuring !BSY.
- ata_read fails if (batch % size) == 0.
- ata_write does not work at all, it uses the read cmd.

Christian

Re: [PATCH] (ata.mod) Fix ATAPI protocol

[Marco Gerards]

Christian Franke <Christian.Franke@t-online.de> writes:

> Christian Franke wrote:
>> Marco Gerards wrote:
>>
>>> Christian Franke <...> writes:
>>>
>>>
>>>> This patch fixes the data I/O protocol of the ATA PACKET command.
>>>>
>>>> The current implementation does not read the byte count
>>>> registers. ATAPI read may not work if the drive sends the data in
>>>> more than one block.
>>>>
>>>> In conjunction with my recent SCSI blocksize patch, ATAPI CD/DVD
>>>> drives should now work.
>>>>
>>>>
>>> Sorry for not commenting on many things on this list.  I am too busy
>>> to check all this.
>>
>> N.P.
>>
>> Meantime, I have tested the SCSI and ATAPI fixes on several PC. If there
>> are no further comments, I will commit these soon.
>>
>>
>
> Committed, plus an additional wait in grub_atapi_identify.

Great! :-)

> Open issues:
> - grub_pio_read/write() check the ERR bit without ensuring !BSY.
> - ata_read fails if (batch % size) == 0.
> - ata_write does not work at all, it uses the read cmd.

grub_ata_write does not use the read cmd, or do you mean indirectly?
In that case, where does it use the read command?

--
Marco

Re: [PATCH] (ata.mod) Fix ATAPI protocol

[Christian Franke]

Marco Gerards wrote:
> > Committed, plus an additional wait in grub_atapi_identify.
> 
> Great! :-)
> 
> > Open issues:
> > - grub_pio_read/write() check the ERR bit without ensuring !BSY.
> > - ata_read fails if (batch % size) == 0.
> > - ata_write does not work at all, it uses the read cmd.
> 
> grub_ata_write does not use the read cmd, or do you mean indirectly?
> In that case, where does it use the read command?
> 

cmd_write is set, but not used:

grub_ata_readwrite (...)
{
  ...
      cmd = GRUB_ATA_CMD_READ_SECTORS;
      cmd_write = GRUB_ATA_CMD_WRITE_SECTORS;


  if (rw == 0)
    {
      ...
    } else {
      /* Write sectors.  */
      if (grub_ata_cmd (dev, cmd))
-----------------------------^^^ GRUB_ATA_CMD_READ_SECTORS
      ...
    }
  ...
}

I will post a patch for all 3 issues soon.

Christian

USB support merge (Re: [PATCH] (ata.mod) Fix ATAPI protocol)

[Robert Millan]

On Mon, Jan 19, 2009 at 11:15:41AM +0100, Marco Gerards wrote:
> Perhaps my USB code might help in some occasions
> for scsi.c, I am not completely sure if they are in sync.  I sent in
> the USB code earlier.  This code can be committed, although endianess
> is not handled correctly at all places.

Hi,

Alright then.  I just checked in your last USB patch, after reviewing it:

  http://lists.gnu.org/archive/html/grub-devel/2008-08/msg00604.html

I agree it's best to commit it.  It has a few minor problems, but this is
much better than leaving it to bitrot.  Then we can work on fixing them more
easily.

Let's enumerate them again:

  - OHCI works on qemu but hasn't been tested on real hardware.

  - --enable-grub-emu-usb isn't yet known to work.

  - There might be endianess issues affecting powerpc (note: we aren't
    building it there yet, we'd have to move PCI support to i386.rmk first)

  - A few parts of it are marked "XXX".

I'll add one more to the list: keyboard support wasn't included in this
patch.  I'll look into your other pieces of code and retrieve a working
keyboard driver.

Other than this, massive storage on UHCI should work fine.  See
http://grub.enbug.org/USBSupport for details on how to use it.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] (ata.mod) Fix ATAPI protocol

[Robert Millan]

[-- Attachment #1: Type: text/plain, Size: 906 bytes --]

On Sun, Feb 08, 2009 at 07:09:53PM +0100, Robert Millan wrote:
> On Mon, Jan 19, 2009 at 11:15:41AM +0100, Marco Gerards wrote:
> > Perhaps my USB code might help in some occasions
> > for scsi.c, I am not completely sure if they are in sync.  I sent in
> > the USB code earlier.  This code can be committed, although endianess
> > is not handled correctly at all places.

Regarding scsi.c (and ata.c), there's more stuff in the other patch from Marco
at:  http://mgerards.net/grub/grub_usb_2008-08-26.diff

It includes modifications that apply to revision 1830.  I re-diffed them
against that version and am attaching them, just in case someone will find
them useful.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

[-- Attachment #2: ata-scsi.diff --]
[-- Type: text/x-diff, Size: 15919 bytes --]

Index: disk/scsi.c
===================================================================
--- disk/scsi.c	(revision 1830)
+++ disk/scsi.c	(working copy)
@@ -111,6 +111,8 @@
 
   scsi = disk->data;
 
+  grub_dprintf ("scsi", "READ10: sector=%lld, size=%d\n", sector, size);
+
   rd.opcode = grub_scsi_cmd_read10;
   rd.lun = scsi->lun << GRUB_SCSI_LUN_SHIFT;
   rd.lba = grub_cpu_to_be32 (sector);
@@ -133,6 +135,8 @@
 
   scsi = disk->data;
 
+  grub_dprintf ("scsi", "READ12: sector=%lld, size=%d\n", sector, size);
+
   rd.opcode = grub_scsi_cmd_read12;
   rd.lun = scsi->lun << GRUB_SCSI_LUN_SHIFT;
   rd.lba = grub_cpu_to_be32 (sector);
@@ -143,7 +147,6 @@
   return scsi->dev->read (scsi, sizeof (rd), (char *) &rd, size * 512, buf);
 }
 
-#if 0
 /* Send a SCSI request for DISK: write the data stored in BUF to SIZE
    sectors starting with SECTOR.  */
 static grub_err_t
@@ -186,7 +189,6 @@
 
   return scsi->dev->write (scsi, sizeof (wr), (char *) &wr, size * 512, buf);
 }
-#endif
 
 \f
 static int
@@ -194,8 +196,6 @@
 {
   grub_scsi_dev_t p;
 
-  auto int scsi_iterate (const char *name, int luns);
-
   int scsi_iterate (const char *name, int luns)
     {
       char sname[40];
@@ -249,6 +249,7 @@
       if (! p->open (name, scsi))
 	{
 	  disk->id = (unsigned long) "scsi"; /* XXX */
+	  disk->has_partitions = 1;
 	  disk->data = scsi;
 	  scsi->dev = p;
 	  scsi->lun = lun;
@@ -274,19 +275,13 @@
 	  
 	  /* Try to be conservative about the device types
 	     supported.  */
-	  if (scsi->devtype != grub_scsi_devtype_direct
-	      && scsi->devtype != grub_scsi_devtype_cdrom)
+	  if (scsi->devtype != 0x00 && scsi->devtype != 0x05)
 	    {
 	      p->close (scsi);
 	      return grub_error (GRUB_ERR_UNKNOWN_DEVICE,
 				 "unknown SCSI device");
 	    }
 
-	  if (scsi->devtype == grub_scsi_devtype_cdrom)
-	    disk->has_partitions = 0;
-	  else
-	    disk->has_partitions = 1;
-
 	  err = grub_scsi_read_capacity (scsi);
 	  if (err)
 	    {
@@ -297,11 +292,12 @@
 
 	  /* SCSI blocks can be something else than 512, although GRUB
 	     wants 512 byte blocks.  */
-	  disk->total_sectors = ((scsi->size * scsi->blocksize)
-				 << GRUB_DISK_SECTOR_BITS);
+/* 	  disk->total_sectors = ((scsi->size) */
+/* 				 << GRUB_DISK_SECTOR_BITS); */
+	  disk->total_sectors = scsi->size;
 
 	  grub_dprintf ("scsi", "capacity=%d, blksize=%d\n",
-			(int) disk->total_sectors, scsi->blocksize);
+			scsi->size, scsi->blocksize);
 
 	  return GRUB_ERR_NONE;
 	}
@@ -327,18 +323,15 @@
 
   scsi = disk->data;
 
-  /* SCSI sectors are variable in size.  GRUB uses 512 byte
-     sectors.  */
   sector = grub_divmod64 (sector, scsi->blocksize >> GRUB_DISK_SECTOR_BITS,
 			  NULL);
 
-  /* Depending on the type, select a read function.  */
   switch (scsi->devtype)
     {
-    case grub_scsi_devtype_direct:
+    case 0:
       return grub_scsi_read10 (disk, sector, size, buf);
 
-    case grub_scsi_devtype_cdrom:
+    case 0x05:
       return grub_scsi_read12 (disk, sector, size, buf);
     }
 
@@ -347,15 +340,14 @@
 }
 
 static grub_err_t
-grub_scsi_write (grub_disk_t disk __attribute((unused)),
-		 grub_disk_addr_t sector __attribute((unused)),
-		 grub_size_t size __attribute((unused)),
-		 const char *buf __attribute((unused)))
+grub_scsi_write (grub_disk_t disk __attribute((unused)), grub_disk_addr_t sector,
+		 grub_size_t size, const char *buf)
 {
 #if 0
   /* XXX: Not tested yet!  */
 
   /* XXX: This should depend on the device type?  */
+  /* XXX: What is the sector size != 512?  */
   return grub_scsi_write10 (disk, sector, size, buf);
 #endif
   return GRUB_ERR_NOT_IMPLEMENTED_YET;
Index: disk/ata.c
===================================================================
--- disk/ata.c	(revision 1830)
+++ disk/ata.c	(working copy)
@@ -119,7 +119,7 @@
   grub_outb (val, dev->ioaddress + reg);
 }
 
-static inline grub_uint8_t
+static inline int
 grub_ata_regget (struct grub_ata_device *dev, int reg)
 {
   return grub_inb (dev->ioaddress + reg);
@@ -131,51 +131,29 @@
   grub_outb (val, dev->ioaddress2 + reg);
 }
 
-static inline grub_uint8_t
+static inline int
 grub_ata_regget2 (struct grub_ata_device *dev, int reg)
 {
   return grub_inb (dev->ioaddress2 + reg);
 }
 
-static inline grub_err_t
-grub_ata_wait_status (struct grub_ata_device *dev,
-		      grub_uint8_t maskset, grub_uint8_t maskclear)
+/* Wait until the device DEV has the status set to ready.  */
+static inline void
+grub_ata_wait_busy (struct grub_ata_device *dev)
 {
-  int i;
-
-  for (i = 0; i < 1000; i++)
-    {
-      grub_uint8_t reg;
-
-      reg = grub_ata_regget (dev, GRUB_ATA_REG_STATUS);
-      if ((reg & maskset) == maskset && (reg & maskclear) == 0)
-	return GRUB_ERR_NONE;
-
-      grub_millisleep (1);
-    }
-
-  return grub_error (GRUB_ERR_TIMEOUT, "ata timeout");
+  while ((grub_ata_regget (dev, GRUB_ATA_REG_STATUS) & GRUB_ATA_STATUS_BUSY));
 }
 
 static inline void
-grub_ata_wait (void)
+grub_ata_wait_drq (struct grub_ata_device *dev)
 {
-  grub_millisleep (50);
+  while (! (grub_ata_regget (dev, GRUB_ATA_REG_STATUS) & GRUB_ATA_STATUS_DRQ));
 }
 
-static grub_err_t
-grub_ata_cmd (struct grub_ata_device *dev, int cmd)
+static inline void
+grub_ata_wait (void)
 {
-  grub_err_t err;
-
-  err = grub_ata_wait_status (dev, 0, 
-			      GRUB_ATA_STATUS_DRQ | GRUB_ATA_STATUS_BUSY);
-  if (err)
-    return err;
-
-  grub_ata_regset (dev, GRUB_ATA_REG_CMD, cmd);
-
-  return GRUB_ERR_NONE;
+  grub_millisleep (1);
 }
 
 /* Byteorder has to be changed before strings can be read.  */
@@ -191,7 +169,7 @@
   dst[len] = '\0';
 }
 
-static grub_err_t
+static int
 grub_ata_pio_read (struct grub_ata_device *dev, char *buf,
 		   grub_size_t size)
 {
@@ -202,17 +180,16 @@
     return grub_ata_regget (dev, GRUB_ATA_REG_ERROR);
 
   /* Wait until the data is available.  */
-  if (grub_ata_wait_status (dev, GRUB_ATA_STATUS_DRQ, 0))
-    return grub_errno;;
+  grub_ata_wait_drq (dev);
 
   /* Read in the data, word by word.  */
   for (i = 0; i < size / 2; i++)
     buf16[i] = grub_le_to_cpu16 (grub_inw(dev->ioaddress + GRUB_ATA_REG_DATA));
 
   if (grub_ata_regget (dev, GRUB_ATA_REG_STATUS) & GRUB_ATA_STATUS_ERR)
-    return grub_error (GRUB_ERR_READ_ERROR, "ATA read error");
+    return grub_ata_regget (dev, GRUB_ATA_REG_ERROR);
 
-  return GRUB_ERR_NONE;
+  return 0;
 }
 
 static grub_err_t
@@ -225,18 +202,17 @@
   if (grub_ata_regget (dev, GRUB_ATA_REG_STATUS) & GRUB_ATA_STATUS_ERR)
     return grub_ata_regget (dev, GRUB_ATA_REG_ERROR);
 
-  /* Wait until the data is available.  */
-  if (grub_ata_wait_status (dev, GRUB_ATA_STATUS_DRQ, 0))
-    return 0;
+  /* Wait until the device is ready to write.  */
+  grub_ata_wait_drq (dev);
 
   /* Write the data, word by word.  */
   for (i = 0; i < size / 2; i++)
     grub_outw(grub_cpu_to_le16 (buf16[i]), dev->ioaddress + GRUB_ATA_REG_DATA);
 
   if (grub_ata_regget (dev, GRUB_ATA_REG_STATUS) & GRUB_ATA_STATUS_ERR)
-    return grub_error (GRUB_ERR_WRITE_ERROR, "ATA write error");
+    return grub_ata_regget (dev, GRUB_ATA_REG_ERROR);
 
-  return GRUB_ERR_NONE;
+  return 0;
 }
 
 static void
@@ -268,33 +244,22 @@
   if (! info)
     return grub_errno;
 
-  if (grub_ata_wait_status (dev, 0, GRUB_ATA_STATUS_BUSY))
-    {
-      grub_free (info);
-      return grub_errno;
-    }
+  grub_ata_wait_busy (dev);
 
   grub_ata_regset (dev, GRUB_ATA_REG_DISK, 0xE0 | dev->device << 4);
+  grub_ata_regset (dev, GRUB_ATA_REG_CMD,
+		   GRUB_ATA_CMD_IDENTIFY_PACKET_DEVICE);
+  grub_ata_wait ();
 
-  if (grub_ata_cmd (dev, GRUB_ATA_CMD_IDENTIFY_PACKET_DEVICE))
-    {
-      grub_free (info);
-      return grub_errno;
-    }
+  grub_ata_pio_read (dev, info, 256);
 
-  if (grub_ata_pio_read (dev, info, 256))
-    {
-      grub_free (info);
-      return grub_errno;
-    }
-
   dev->atapi = 1;
 
   grub_ata_dumpinfo (dev, info);
 
   grub_free (info);
 
-  return GRUB_ERR_NONE;
+  return 0;
 }
 
 static grub_err_t
@@ -321,7 +286,7 @@
 {
   char *info;
   grub_uint16_t *info16;
-  int ataerr = 0;
+  int ataerr;
 
   info = grub_malloc (GRUB_DISK_SECTOR_SIZE);
   if (! info)
@@ -329,22 +294,13 @@
 
   info16 = (grub_uint16_t *) info;
 
-  if (grub_ata_wait_status (dev, 0, GRUB_ATA_STATUS_BUSY))
-    {
-      grub_free (info);
-      return grub_errno;
-    }
+  grub_ata_wait_busy (dev);
 
   grub_ata_regset (dev, GRUB_ATA_REG_DISK, 0xE0 | dev->device << 4);
-  if (grub_ata_cmd (dev, GRUB_ATA_CMD_IDENTIFY_DEVICE))
-    {
-      grub_free (info);
-      return grub_errno;
-    }
+  grub_ata_regset (dev, GRUB_ATA_REG_CMD, GRUB_ATA_CMD_IDENTIFY_DEVICE);
   grub_ata_wait ();
 
-  if (grub_ata_pio_read (dev, info, GRUB_DISK_SECTOR_SIZE))
-    ataerr = grub_ata_regget (dev, GRUB_ATA_REG_ERROR);
+  ataerr = grub_ata_pio_read (dev, info, GRUB_DISK_SECTOR_SIZE);
   if (ataerr & 4)
     {
       /* ATAPI device detected.  */
@@ -409,8 +365,8 @@
   /* Setup the device information.  */
   dev->port = port;
   dev->device = device;
-  dev->ioaddress = addr;
-  dev->ioaddress2 = addr2;
+  dev->ioaddress = grub_ata_ioaddress[dev->port];
+  dev->ioaddress2 = grub_ata_ioaddress2[dev->port];
   dev->next = NULL;
 
   /* Try to detect if the port is in use by writing to it,
@@ -429,11 +385,8 @@
   /* Detect if the device is present by issuing a EXECUTE
      DEVICE DIAGNOSTICS command.  */
   grub_ata_regset (dev, GRUB_ATA_REG_DISK, dev->device << 4);
-  if (grub_ata_cmd (dev, GRUB_ATA_CMD_EXEC_DEV_DIAGNOSTICS))
-    {
-      grub_free (dev);
-      return grub_errno;
-    }
+  grub_ata_regset (dev, GRUB_ATA_REG_CMD,
+		   GRUB_ATA_CMD_EXEC_DEV_DIAGNOSTICS);
   grub_ata_wait ();
 
   grub_dprintf ("ata", "Registers: %x %x %x %x\n",
@@ -450,19 +403,19 @@
     {
       grub_dprintf ("ata", "ATAPI signature detected\n");
     }
-  else if (grub_ata_regget (dev, GRUB_ATA_REG_SECTORS) == 0x01
-	   && grub_ata_regget (dev, GRUB_ATA_REG_LBALOW) == 0x01
-	   && grub_ata_regget (dev, GRUB_ATA_REG_LBAMID) == 0x00
-	   && grub_ata_regget (dev, GRUB_ATA_REG_LBAHIGH) == 0x00)
+  else if (! (grub_ata_regget (dev, GRUB_ATA_REG_SECTORS) == 0x01
+	      && grub_ata_regget (dev, GRUB_ATA_REG_LBALOW) == 0x01
+	      && grub_ata_regget (dev, GRUB_ATA_REG_LBAMID) == 0x00
+	      && grub_ata_regget (dev, GRUB_ATA_REG_LBAHIGH) == 0x00))
     {
-      grub_dprintf ("ata", "ATA detected\n");
-    }
-  else
-    {
       grub_dprintf ("ata", "incorrect signature\n");
       grub_free (dev);
       return 0;
     }
+  else
+    {
+      grub_dprintf ("ata", "ATA detected\n");
+    }
 
 
   /* Use the IDENTIFY DEVICE command to query the device.  */
@@ -480,8 +433,7 @@
 }
 
 static int
-grub_ata_pciinit (int bus, int device, int func,
-		  grub_pci_id_t pciid __attribute__((unused)))
+grub_ata_pciinit (int bus, int device, int func, grub_pci_id_t pciid)
 {
   static int compat_use[2] = { 0 };
   grub_pci_address_t addr;
@@ -491,7 +443,6 @@
   int rega;
   int regb;
   int i;
-  static int controller = 0;
 
   /* Read class.  */
   addr = grub_pci_make_address (bus, device, func, 2);
@@ -540,13 +491,11 @@
 
       if (rega && regb)
 	{
-	  grub_ata_device_initialize (controller * 2 + i, 0, rega, regb);
-	  grub_ata_device_initialize (controller * 2 + i, 1, rega, regb);
+	  grub_ata_device_initialize (i, 0, rega, regb);
+	  grub_ata_device_initialize (i, 1, rega, regb);
 	}
     }
 
-  controller++;
-
   return 0;
 }
 
@@ -574,8 +523,7 @@
 		     grub_disk_addr_t sector,
 		     grub_size_t size)
 {
-  if (grub_ata_wait_status (dev, 0, GRUB_ATA_STATUS_BUSY))
-    return grub_errno;
+  grub_ata_wait_busy (dev);
 
   switch (addressing)
     {
@@ -671,31 +619,21 @@
       if (rw == 0)
 	{
 	  /* Read 256/65536 sectors.  */
-	  if (grub_ata_cmd (dev, cmd))
-	    return grub_errno;
-
-	  /* Wait for the command to complete.  */
-	  if (grub_ata_wait_status (dev, 0, GRUB_ATA_STATUS_BUSY))
-	    return grub_errno;
-
+	  grub_ata_regset (dev, GRUB_ATA_REG_CMD, cmd);
+	  grub_ata_wait ();
 	  for (sect = 0; sect < batch; sect++)
 	    {
 	      if (grub_ata_pio_read (dev, buf,
 				     GRUB_DISK_SECTOR_SIZE))
-		return grub_errno;
+		return grub_error (GRUB_ERR_READ_ERROR, "ATA read error");
 	      buf += GRUB_DISK_SECTOR_SIZE;
 	    }
 	}
       else
 	{
 	  /* Write 256/65536 sectors.  */
-	  if (grub_ata_cmd (dev, cmd))
-	    return grub_errno;
-
-	  /* Wait for the command to complete.  */
-	  if (grub_ata_wait_status (dev, 0, GRUB_ATA_STATUS_BUSY))
-	    return grub_errno;
-
+	  grub_ata_regset (dev, GRUB_ATA_REG_CMD, cmd_write);
+	  grub_ata_wait ();
 	  for (sect = 0; sect < batch; sect++)
 	    {
 	      if (grub_ata_pio_write (dev, buf,
@@ -714,28 +652,18 @@
   if (rw == 0)
     {
       /* Read sectors.  */
-      if (grub_ata_cmd (dev, cmd))
-	return grub_errno;
-
-      /* Wait for the command to complete.  */
-      if (grub_ata_wait_status (dev, 0, GRUB_ATA_STATUS_BUSY))
-	return grub_errno;
-
+      grub_ata_regset (dev, GRUB_ATA_REG_CMD, cmd);
+      grub_ata_wait ();
       for (sect = 0; sect < (size % batch); sect++)
 	{
 	  if (grub_ata_pio_read (dev, buf, GRUB_DISK_SECTOR_SIZE))
-	    return grub_errno;
+	    return grub_error (GRUB_ERR_READ_ERROR, "ATA read error");
 	  buf += GRUB_DISK_SECTOR_SIZE;
 	}
     } else {
       /* Write sectors.  */
-      if (grub_ata_cmd (dev, cmd))
-	return grub_errno;
-
-      /* Wait for the command to complete.  */
-      if (grub_ata_wait_status (dev, 0, GRUB_ATA_STATUS_BUSY))
-	return grub_errno;
-
+      grub_ata_regset (dev, GRUB_ATA_REG_CMD, cmd_write);
+      grub_ata_wait ();
       for (sect = 0; sect < (size % batch); sect++)
 	{
 	  if (grub_ata_pio_write (dev, buf, GRUB_DISK_SECTOR_SIZE))
@@ -762,6 +690,9 @@
       if (dev->atapi)
 	continue;
 
+      if (dev->atapi)
+	continue;
+
       if (hook (devname))
 	return 1;
     }
@@ -933,8 +864,105 @@
     .write = grub_atapi_write
   }; 
 
+
 \f
+/* ATAPI code.  */
 
+static int
+grub_atapi_iterate (int (*hook) (const char *name, int luns))
+{
+  struct grub_ata_device *dev;
+
+  for (dev = grub_ata_devices; dev; dev = dev->next)
+    {
+      char devname[5];
+      grub_sprintf (devname, "ata%d", dev->port * 2 + dev->device);
+
+      if (! dev->atapi)
+	continue;
+
+      if (hook (devname, 1))
+	return 1;
+    }
+
+  return 0;
+
+}
+
+static grub_err_t
+grub_atapi_read (struct grub_scsi *scsi, grub_size_t cmdsize, char *cmd,
+		 grub_size_t size, char *buf)
+{
+  struct grub_ata_device *dev = (struct grub_ata_device *) scsi->data;
+
+  if (grub_atapi_packet (dev, cmd))
+    return grub_errno;
+
+  grub_ata_wait (); /* XXX */
+
+  return grub_ata_pio_read (dev, buf, size);
+}
+
+static grub_err_t
+grub_atapi_write (struct grub_scsi *scsi, grub_size_t cmdsize, char *cmd,
+		  grub_size_t size, char *buf)
+{
+  struct grub_ata_device *dev = (struct grub_ata_device *) scsi->data;
+
+  if (grub_atapi_packet (dev, cmd))
+    return grub_errno;
+
+  grub_ata_wait (); /* XXX */
+
+  return grub_ata_pio_write (dev, buf, size);
+}
+
+static grub_err_t
+grub_atapi_open (const char *name, struct grub_scsi *scsi)
+{
+  struct grub_ata_device *dev;
+  struct grub_ata_device *devfnd;
+
+  for (dev = grub_ata_devices; dev; dev = dev->next)
+    {
+      char devname[5];
+      grub_sprintf (devname, "ata%d", dev->port * 2 + dev->device);
+
+      if (!grub_strcmp (devname, name))
+	{
+	  devfnd = dev;
+	  break;
+	}
+    }
+
+  if (! devfnd)
+    return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "No such ATAPI device");
+
+  scsi->data = devfnd;
+  scsi->name = grub_strdup (name);
+  scsi->luns = 1;
+
+  return GRUB_ERR_NONE;
+}
+
+static void
+grub_atapi_close (struct grub_scsi *scsi)
+{
+  grub_free (scsi->name);
+}
+
+static struct grub_scsi_dev grub_atapi_dev =
+  {
+    .name = "ATAPI",
+    .iterate = grub_atapi_iterate,
+    .open = grub_atapi_open,
+    .close = grub_atapi_close,
+    .read = grub_atapi_read,
+    .write = grub_atapi_write
+  }; 
+
+\f
+
 GRUB_MOD_INIT(ata)
 {
   (void) mod;			/* To stop warning. */

Re: USB support merge (Re: [PATCH] (ata.mod) Fix ATAPI protocol)

[Marco Gerards]

Robert Millan <rmh@aybabtu.com> writes:

> On Mon, Jan 19, 2009 at 11:15:41AM +0100, Marco Gerards wrote:
>> Perhaps my USB code might help in some occasions
>> for scsi.c, I am not completely sure if they are in sync.  I sent in
>> the USB code earlier.  This code can be committed, although endianess
>> is not handled correctly at all places.
>
> Hi,
>
> Alright then.  I just checked in your last USB patch, after reviewing it:
>
>   http://lists.gnu.org/archive/html/grub-devel/2008-08/msg00604.html
>
> I agree it's best to commit it.  It has a few minor problems, but this is
> much better than leaving it to bitrot.  Then we can work on fixing them more
> easily.

Thanks, I am happy it is merged now.  It would help if people reported
success/failure with USB support.

> Let's enumerate them again:
>
>   - OHCI works on qemu but hasn't been tested on real hardware.
>
>   - --enable-grub-emu-usb isn't yet known to work.
>
>   - There might be endianess issues affecting powerpc (note: we aren't
>     building it there yet, we'd have to move PCI support to i386.rmk first)

It is even certain :-)

>   - A few parts of it are marked "XXX".
>
> I'll add one more to the list: keyboard support wasn't included in this
> patch.  I'll look into your other pieces of code and retrieve a working
> keyboard driver.

Right, this shouldn't be too hard as the code was there.  But I have
seen you submitted this already.

--
Marco