grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Patch] [bug #26237] multiple problems with usb devices


From: Vladimir 'φ-coder/phcoder' Serbinenko
Subject: Re: [Patch] [bug #26237] multiple problems with usb devices
Date: Sun, 23 May 2010 01:13:39 +0200
User-agent: Mozilla-Thunderbird 2.0.0.22 (X11/20091109)

Aleš Nesrsta wrote:
> Hi Vladimir,
>
> I have some news - I made extensive testing and researching and I
> discovered some things related to OHCI and generally to USB MS or USB
> itself.
>
> I tried to fix them and it looks like I was probably successful in some
> cases:
>
> - multiple LUNs related bug (meaning of value from device & coding into
> CBW)
> - bulk transport toggling related bug - on devices with endpoints with
> the same "low" nuber (e.g. 01 & 81)
> - bug related to devices with max. packet size for control pipe 0 lower
> than 64
> - problem with one kind of OHCI BIOS legacy support maybe solved
> - another bug - some devices hangs because of Bulk-only Mass Storage
> Reset used during opening when it is not expected by device
> - my OHCI done-head loop should not hang anymore - timeout was added
> - "ls" should not hang anymore on USB MS device with not ready unit -
> timeout was added in SCSI open unit ready loop
>   
I hope in future we'll be able to do useful operations during wait.
> - experimentally added support for another subclasses of USB MS devices
> (as all are SCSI based - it possibly should work, at least it is working
> with one my device with subclass SFF-8070i - but I don't have another
> devices to test)
>
> In fact, all my USB MS devices are working now with grub2 with exception
> of one camera which have not Bulk-Only but CBI protocol.
>
> What I was not solved till now:
> - Some devices are not working on UHCI
> - If I connect some device and load uhci module, I cannot connect
> another device without rebooting. On OHCI it is possible without
> rebooting in this sequence:
>       rmmod usbms
>       rmmod ohci
>       (unplug old device and plug new one)
>       insmod ohci
>       insmod usbms
>   
In my yeeloongfw branch I have hotplug support for ATA. I'm planning on
adding usbms hotplugging support but first I want to make Geode USB work.
> - It looks like my OHCI is not properly reporting STALL state of pipe -
> it is probably reason why done-head loop hanged if some problem happened
> in communication.
>   
In general there should be timeout in any loop which may hang if device
doesn't behave the way it's supposed to.

The fixes I've come up with are available in yeeloongfw branch. I've
adjusted your previous patch to work on top of yeeloongfw branch.
> I want to prepare patch and send it here but I cannot get actual bzr
> code. I would want have it at least because grub_memalign problem is not
> solved in last grub version wich I have (1.98) and I have to solve it by
> some workarounds (maybe it is also reason why UHCI does not work
> properly on my computer, I did not any workaround in its code yet).
>
> I cannot get actual bzr code because of this error:
> $ bzr get http://bzr.savannah.gnu.org/r/grub
> bzr: ERROR: Not a branch:
> "http://bzr.savannah.gnu.org/r/grub/.bzr/branch/";.
>
> I tried to found some advice in man bzr and forums but I was not
> successful. It looks like problem is that there is really not existing
> folder ".../r/.bzr/branch" (I found "branch-lock" and "repository" only)
> - should I use some another address or some option ?
> Do You have some experiences with such problem ?
> Could You put me in right direction...?
>
> (I tried to download CVS source code - it works - but it looks outdated.
> I also tried register ssh public code and made access via ssh to
> sftp://address@hidden/srv/bzr/grub - but I cannot login,
> there is ssh debug message that it is not possible to find
> authentication method...)
>
> Best regards
> Ales
>
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>   


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

=== modified file 'bus/usb/ohci.c'
--- bus/usb/ohci.c      2010-05-22 22:17:51 +0000
+++ bus/usb/ohci.c      2010-05-22 22:17:58 +0000
@@ -317,13 +317,29 @@
   token |= toggle << 24;
   token |= 1 << 25;
 
+  /* Set "Not accessed" error code */
+  token |= 15 << 28;
+
   buffer = data;
   buffer_end = buffer + size - 1;
 
+  /* Set correct buffer values in TD if zero transfer occurs */
+  if (size) 
+    {
+      buffer = data;
+      buffer_end = buffer + size - 1;
+      td->buffer = grub_cpu_to_le32 (buffer);
+      td->buffer_end = grub_cpu_to_le32 (buffer_end);
+    }
+  else 
+    {
+      td->buffer = 0;
+      td->buffer_end = 0;
+    }
+ 
+  /* Set the rest of TD */
   td->token = grub_cpu_to_le32 (token);
-  td->buffer = grub_cpu_to_le32 (buffer);
   td->next_td = 0;
-  td->buffer_end = grub_cpu_to_le32 (buffer_end);
 }
 
 static grub_usb_err_t
@@ -375,6 +391,24 @@
                                             + (i + 1) * sizeof (td_list[0]));
     }
 
+  /* The last-1 TD token we should change to enable interrupt when TD finishes.
+   * As OHCI interrupts are disabled, it does only setting of WDH bit in
+   * HcInterruptStatus register - and that is what we want to safely detect
+   * end of all transactions. */
+  td_list[transfer->transcnt - 1].token &= ~(7 << 21);
+  
+  td_list[transfer->transcnt].token = 15 << 28;
+  td_list[transfer->transcnt].buffer = 0;
+  td_list[transfer->transcnt].buffer_end = 0;
+  td_list[transfer->transcnt].next_td = grub_cpu_to_le32 (td_list_addr
+                                                         + transfer->transcnt
+                                                         * sizeof (*td_list));
+
+  for (i = 0; i <= transfer->transcnt; i++)
+    grub_dprintf ("ohci", "transaction %d: %x, %x, %x, %x\n", i,
+                 td_list[i].token, td_list[i].buffer,
+                 td_list[i].buffer_end, td_list[i].next_td);
+
   /* Setup the Endpoint Descriptor.  */
 
   /* Set the device address.  */
@@ -401,30 +435,48 @@
   grub_dprintf ("ohci", "program OHCI\n");
 
   /* Program the OHCI to actually transfer.  */
+
+  /* Disable the Control and Bulk lists.  */
+  control = grub_ohci_readreg32 (o, GRUB_OHCI_REG_CONTROL);
+  control &= ~(3 << 4);
+  grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROL, control);
+
+  /* Clear BulkListFilled and ControlListFilled.  */
+  status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_CMDSTATUS);
+  status &= ~(3 << 1);
+  grub_ohci_writereg32 (o, GRUB_OHCI_REG_CMDSTATUS, status);
+
+  /* Now we should wait for start of next frame. Because we are not using
+   * interrupt, we reset SF bit and wait when it goes to 1. */
+  /* SF bit reset. (SF bit indicates Start Of Frame (SOF) packet) */
+  grub_ohci_writereg32 (o, GRUB_OHCI_REG_INTSTATUS, (1<<2));
+  /* Wait for new SOF */
+  while ((grub_ohci_readreg32 (o, GRUB_OHCI_REG_INTSTATUS) & 0x4) == 0);
+  /* Now it should be safe to change CONTROL and BULK lists. */
+
+  /* This we do for safety's sake - it should be done in previous call
+   * of grub_ohci_transfer and nobody should change it in meantime...
+   * It should be done before start of control or bulk OHCI list. */   
+  o->hcca->donehead = 0;
+  grub_ohci_writereg32 (o, GRUB_OHCI_REG_INTSTATUS, (1 << 1)); /* Clears WDH */
+
   switch (transfer->type)
     {
     case GRUB_USB_TRANSACTION_TYPE_BULK:
       {
        grub_dprintf ("ohci", "add to bulk list\n");
 
-       status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_CMDSTATUS);
-       control = grub_ohci_readreg32 (o, GRUB_OHCI_REG_CONTROL);
-
-       /* Disable the Control and Bulk lists.  */
-       control &= ~(3 << 4);
-       grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROL, control);
-
-       /* Clear BulkListFilled.  */
-       status &= ~(1 << 2);
-       grub_ohci_writereg32 (o, GRUB_OHCI_REG_CMDSTATUS, status);
-
+       /* Set BulkList Head and Current */
        grub_ohci_writereg32 (o, GRUB_OHCI_REG_BULKHEAD, ed_addr);
+       grub_ohci_writereg32 (o, GRUB_OHCI_REG_BULKCURR, 0);
 
        /* Enable the Bulk list.  */
+       control = grub_ohci_readreg32 (o, GRUB_OHCI_REG_CONTROL);
        control |= 1 << 5;
        grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROL, control);
 
        /* Set BulkListFilled.  */
+       status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_CMDSTATUS);
        status |= 1 << 2;
        grub_ohci_writereg32 (o, GRUB_OHCI_REG_CMDSTATUS, status);
 
@@ -433,21 +485,9 @@
 
     case GRUB_USB_TRANSACTION_TYPE_CONTROL:
       {
-       grub_dprintf ("ohci", "add to control list\n");
-       status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_CMDSTATUS);
-       control = grub_ohci_readreg32 (o, GRUB_OHCI_REG_CONTROL);
-
-       /* Disable the Control and Bulk lists.  */
-       control &= ~(3 << 4);
-       grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROL, control);
-
-       /* Clear ControlListFilled.  */
-       status &= ~(1 << 1);
-       grub_ohci_writereg32 (o, GRUB_OHCI_REG_CMDSTATUS, status);
-
+       /* Set ControlList Head and Current */
        grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROLHEAD, ed_addr);
-       grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROLHEAD+1,
-                             ed_addr);
+       grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROLCURR, 0);
 
        /* Enable the Control list.  */
        control |= 1 << 4;
@@ -466,16 +506,61 @@
                grub_ohci_readreg32 (o, GRUB_OHCI_REG_CMDSTATUS));
 
   /* Wait until the transfer is completed or STALLs.  */
-  while ((ed->td_head & ~0xf) != (ed->td_tail & ~0xf))
+  do
     {
       grub_cpu_idle ();
 
       grub_dprintf ("ohci", "head=0x%02x tail=0x%02x\n", ed->td_head, 
ed->td_tail);
-
-      /* Detected a STALL.  */
-      if (ed->td_head & 1)
-       break;
+      grub_dprintf ("ohci", "framenumber=0x%02x\n", o->hcca->framenumber);
+
+      /* Detected a HALT.  */
+      if (grub_le_to_cpu32 (ed->td_head) & 1)
+        break;
+
+      //      if ((ed->td_head & ~0xf) == (ed->td_tail & ~0xf))
+      //break;
+  
+      /* The original style of detection of finished transaction looks 
hazardeous.
+        while ((grub_le_to_cpu32 (grub_ohci_read_mem32 (&ed->td_head)) & ~0xf)
+         != (grub_le_to_cpu32 (grub_ohci_read_mem32 (&ed->td_tail)) & ~0xf));
+      */
+      /* This should be according to OHCI specification:
+       * TD is finished and ED is updated when TD is retired and HcDoneHead
+       * register updated and, if allowed by WDH bit, written into 
HccaDoneHead.
+       * So we should:
+       * - allow WritebackDoneHead interrupt (WDH) by proper setting of last TD
+       *   token - it is done above in transaction settings
+       * - detect setting of WDH bit in HcInterruptStatus register
+       * - compare HccaDoneHead value with address of last-1 TD. If it is not
+       *   equal, check ED for halt and if not so, reset WDH bit and wait again
+       *   - but it should not happen - debug it!
+       */
+      grub_uint32_t intstatus = grub_ohci_readreg32 (o,
+                                                    GRUB_OHCI_REG_INTSTATUS);
+      grub_dprintf ("ohci", "intstatus = %x\n", intstatus);
+      if ((intstatus & 0x2) != 0)
+        {
+         grub_dprintf ("ohci", "donehead = %x\n",
+                       grub_le_to_cpu32 (o->hcca->donehead));
+          if ((grub_le_to_cpu32 (o->hcca->donehead) & ~0xf)
+             == td_list_addr + (transfer->transcnt - 1) * sizeof (*td_list))
+            break;
+
+          /* Done Head can be updated on some another place if ED is halted. 
*/          
+          if (grub_le_to_cpu32 (ed->td_head) & 1)
+            break;
+
+          /* If there is not HALT in ED, it is not correct, so debug it, reset
+           * donehead and WDH and continue waiting. */
+          grub_dprintf ("ohci", "Incorrect HccaDoneHead=0x%08x\n",
+                        o->hcca->donehead);
+          o->hcca->donehead = 0;
+          grub_ohci_writereg32 (o, GRUB_OHCI_REG_INTSTATUS, (1 << 1));
+         //          continue;
+         break;
+        }
     }
+  while (1);
 
   grub_dprintf ("ohci", "complete\n");
 
@@ -484,23 +569,34 @@
 /*   else if (ed->td */
 
 
-  if (ed->td_head & 1)
+  if (grub_le_to_cpu32 (ed->td_head) & 1)
     {
-      grub_uint8_t errcode;
+      grub_uint8_t errcode = 0;
       grub_ohci_td_t tderr;
       grub_uint32_t td_err_addr;
 
-      td_err_addr = grub_ohci_readreg32 (o, GRUB_OHCI_REG_DONEHEAD);
-
-      tderr = (grub_ohci_td_t) ((char *) td_list
-                               + (td_err_addr - td_list_addr));
-      errcode = tderr->token >> 28;
+      td_err_addr = grub_ohci_readreg32 (o, GRUB_OHCI_REG_DONEHEAD) & ~0xf;
+      grub_dprintf ("ohci", "td_err_addr =0x%x\n", td_err_addr);
+      if (td_err_addr == 0)
+        /* If DONEHEAD==0 it means that correct address is in HCCA.
+         * It should be always now! */
+        td_err_addr = grub_le_to_cpu32 (o->hcca->donehead) & ~0xf;
+
+      grub_dprintf ("ohci", "td_err_addr =0x%x\n", td_err_addr);
+
+      if (td_err_addr != 0)
+       {
+         tderr = (grub_ohci_td_t) ((char *) td_list
+                                   + (td_err_addr - td_list_addr));
+         errcode = grub_le_to_cpu32 (tderr->token) >> 28;
+       }
+      grub_dprintf ("ohci", "errcode = %d\n", errcode);
 
       switch (errcode)
        {
        case 0:
          /* XXX: Should not happen!  */
-         grub_error (GRUB_ERR_IO, "OHCI without reporting the reason");
+         grub_error (GRUB_ERR_IO, "OHCI failed without reporting the reason");
          err = GRUB_USB_ERR_INTERNAL;
          break;
 
@@ -582,12 +678,41 @@
 
   /* Clear BulkListFilled and ControlListFilled.  */
   status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_CMDSTATUS);
-  status &= ~((1 << 2) | (1 << 3));
+  status &= ~(3 << 1);
   grub_ohci_writereg32 (o, GRUB_OHCI_REG_CMDSTATUS, status);
-
+  
+  /* Set ED to be skipped - for safety */
+  ed->target |= grub_cpu_to_le32 (1 << 14);
+
+  /* Now we should wait for start of next frame. Because we are not using
+   * interrupt, we reset SF bit and wait when it goes to 1. */
+  /* SF bit reset. (SF bit indicates Start Of Frame (SOF) packet) */
+  grub_ohci_writereg32 (o, GRUB_OHCI_REG_INTSTATUS, (1<<2));
+  /* Wait for new SOF */
+  while ((grub_ohci_readreg32 (o, GRUB_OHCI_REG_INTSTATUS) & 0x4) == 0);
+  /* Now it should be safe to change CONTROL and BULK lists. */
+  
+  /* Important cleaning. */
+  o->hcca->donehead = 0;
+  /* Additional debug print. to be removed.
+  grub_dprintf ("ohci", "Before reset of WDH: INTSTATUS=0x%08x\n",
+    grub_ohci_readreg32 (o, GRUB_OHCI_REG_INTSTATUS));
+    */
+  grub_ohci_writereg32 (o, GRUB_OHCI_REG_INTSTATUS, (1 << 1)); /* Clears WDH */
+  /* Additional debug print. to be removed.
+  grub_dprintf ("ohci", "After reset of WDH: INTSTATUS=0x%08x\n",
+    grub_ohci_readreg32 (o, GRUB_OHCI_REG_INTSTATUS));
+    */
+  grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROLHEAD, 0);
+  grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROLCURR, 0);
+  grub_ohci_writereg32 (o, GRUB_OHCI_REG_BULKHEAD, 0);
+  grub_ohci_writereg32 (o, GRUB_OHCI_REG_BULKCURR, 0);
+
+  grub_dprintf ("ohci", "OHCI finished, freeing, err=0x%02x\n", err);
+  
   /* XXX */
-  grub_free (td_list);
-  grub_free (ed);
+  grub_dma_free (td_list_chunk);
+  grub_dma_free (ed_chunk);
 
   return err;
 }
@@ -668,8 +793,11 @@
 
 GRUB_MOD_INIT(ohci)
 {
+  grub_dprintf ("ohci", "init1\n");
   grub_ohci_inithw ();
+  grub_dprintf ("ohci", "init2\n");
   grub_usb_controller_dev_register (&usb_controller);
+  grub_dprintf ("ohci", "init3\n");
 }
 
 GRUB_MOD_FINI(ohci)

=== modified file 'bus/usb/usbhub.c'
--- bus/usb/usbhub.c    2009-12-24 22:53:05 +0000
+++ bus/usb/usbhub.c    2010-05-22 22:17:58 +0000
@@ -65,6 +65,9 @@
   dev->initialized = 1;
   grub_usb_devs[i] = dev;
 
+  grub_dprintf ("usb", "dev=%p, speed = %d\n", grub_usb_devs[i],
+               grub_usb_devs[i]->speed);
+
   return dev;
 }
 
@@ -155,11 +158,15 @@
        {
          grub_usb_device_t dev;
 
+         grub_dprintf ("usb", "calling portstatus\n");
+
          /* Enable the port.  */
          err = controller->dev->portstatus (controller, i, 1);
          if (err)
            continue;
 
+         grub_dprintf ("usb", "calling add_dev\n");
+
          /* Enable the port and create a device.  */
          dev = grub_usb_hub_add_dev (controller, speed);
          if (! dev)
@@ -183,6 +190,8 @@
     {
       if (grub_usb_devs[i])
        {
+         grub_dprintf ("usb", "dev=%p, speed = %d\n", grub_usb_devs[i],
+                       grub_usb_devs[i]->speed);
          if (hook (grub_usb_devs[i]))
              return 1;
        }

=== modified file 'commands/usbtest.c'
--- commands/usbtest.c  2010-01-03 18:24:22 +0000
+++ commands/usbtest.c  2010-05-22 22:17:58 +0000
@@ -83,15 +83,14 @@
                              0x06, (3 << 8) | index,
                              langid, descstr.length, (char *) descstrp);
 
-  *string = grub_malloc (descstr.length / 2);
+  *string = grub_malloc (descstr.length * 2);
   if (! *string)
     {
       grub_free (descstrp);
       return GRUB_USB_ERR_INTERNAL;
     }
 
-  grub_utf16_to_utf8 ((grub_uint8_t *) *string, descstrp->str, 
descstrp->length / 2 - 1);
-  (*string)[descstr.length / 2 - 1] = '\0';
+  *grub_utf16_to_utf8 ((grub_uint8_t *) *string, descstrp->str, 
descstrp->length / 2 - 1) = 0;
   grub_free (descstrp);
 
   return GRUB_USB_ERR_NONE;
@@ -124,6 +123,11 @@
   int i;
 
   descdev = &dev->descdev;
+  grub_dprintf ("usb", "dev=%p, descdev=%p\n", dev, descdev);
+
+  grub_dprintf ("usb", "strprod=%d\n", descdev->strprod);
+  grub_dprintf ("usb", "strvendor=%d\n", descdev->strvendor);
+  grub_dprintf ("usb", "strserial=%d\n", descdev->strserial);
 
   usb_print_str ("Product", dev, descdev->strprod);
   usb_print_str ("Vendor", dev, descdev->strvendor);
@@ -139,6 +143,11 @@
 
   grub_printf ("%s speed device\n", usb_devspeed[dev->speed]);
 
+  grub_dprintf ("usb", "configcnt=%d\n", descdev->configcnt);
+  grub_dprintf ("usb", "config=%p\n", dev->config);
+
+  return 0;
+
   for (i = 0; i < descdev->configcnt; i++)
     {
       struct grub_usb_desc_config *config;

=== modified file 'disk/scsi.c'
--- disk/scsi.c 2010-03-05 14:29:28 +0000
+++ disk/scsi.c 2010-05-22 22:17:58 +0000
@@ -50,6 +50,57 @@
 }
 
 
+/* Check result of previous operation.  */
+static grub_err_t
+grub_scsi_request_sense (grub_scsi_t scsi)
+{
+  struct grub_scsi_request_sense rs;
+  struct grub_scsi_request_sense_data rsd;
+  grub_err_t err;
+
+  rs.opcode = grub_scsi_cmd_request_sense;
+  rs.lun = scsi->lun << GRUB_SCSI_LUN_SHIFT;
+  rs.reserved1 = 0;
+  rs.reserved2 = 0;
+  rs.alloc_length = 0x12; /* XXX: Hardcoded for now */
+  rs.control = 0;
+
+  err = scsi->dev->read (scsi, sizeof (rs), (char *) &rs,
+                        sizeof (rsd), (char *) &rsd);
+  if (err)
+    return err;
+
+  return GRUB_ERR_NONE;
+}
+/* Self commenting... */
+static grub_err_t
+grub_scsi_test_unit_ready (grub_scsi_t scsi)
+{
+  struct grub_scsi_test_unit_ready tur;
+  grub_err_t err;
+  grub_err_t err_sense;
+  
+  tur.opcode = grub_scsi_cmd_test_unit_ready;
+  tur.lun = scsi->lun << GRUB_SCSI_LUN_SHIFT;
+  tur.reserved1 = 0;
+  tur.reserved2 = 0;
+  tur.reserved3 = 0;
+  tur.control = 0;
+
+  err = scsi->dev->read (scsi, sizeof (tur), (char *) &tur,
+                        0, NULL);
+
+  /* Each SCSI command should be followed by Request Sense.
+     If not so, many devices STALLs or definitely freezes. */
+  err_sense = grub_scsi_request_sense (scsi);
+  /* err_sense is ignored for now and Request Sense Data also... */
+  
+  if (err)
+    return err;
+
+  return GRUB_ERR_NONE;
+}
+
 /* Determine the the device is removable and the type of the device
    SCSI.  */
 static grub_err_t
@@ -58,15 +109,23 @@
   struct grub_scsi_inquiry iq;
   struct grub_scsi_inquiry_data iqd;
   grub_err_t err;
+  grub_err_t err_sense;
 
   iq.opcode = grub_scsi_cmd_inquiry;
   iq.lun = scsi->lun << GRUB_SCSI_LUN_SHIFT;
+  iq.page = 0;
   iq.reserved = 0;
   iq.alloc_length = 0x24; /* XXX: Hardcoded for now */
-  iq.reserved2 = 0;
+  iq.control = 0;
 
   err = scsi->dev->read (scsi, sizeof (iq), (char *) &iq,
                         sizeof (iqd), (char *) &iqd);
+
+  /* Each SCSI command should be followed by Request Sense.
+     If not so, many devices STALLs or definitely freezes. */
+  err_sense = grub_scsi_request_sense (scsi);
+  /* err_sense is ignored for now and Request Sense Data also... */
+
   if (err)
     return err;
 
@@ -83,13 +142,24 @@
   struct grub_scsi_read_capacity rc;
   struct grub_scsi_read_capacity_data rcd;
   grub_err_t err;
+  grub_err_t err_sense;
 
   rc.opcode = grub_scsi_cmd_read_capacity;
   rc.lun = scsi->lun << GRUB_SCSI_LUN_SHIFT;
-  grub_memset (rc.reserved, 0, sizeof (rc.reserved));
+  rc.logical_block_addr = 0;
+  rc.reserved1 = 0;
+  rc.reserved2 = 0;
+  rc.PMI = 0;
+  rc.control = 0;
 
   err = scsi->dev->read (scsi, sizeof (rc), (char *) &rc,
                         sizeof (rcd), (char *) &rcd);
+
+  /* Each SCSI command should be followed by Request Sense.
+     If not so, many devices STALLs or definitely freezes. */
+  err_sense = grub_scsi_request_sense (scsi);
+  /* err_sense is ignored for now and Request Sense Data also... */
+
   if (err)
     return err;
 
@@ -107,6 +177,8 @@
 {
   grub_scsi_t scsi;
   struct grub_scsi_read10 rd;
+  grub_err_t err;
+  grub_err_t err_sense;
 
   scsi = disk->data;
 
@@ -118,7 +190,14 @@
   rd.reserved2 = 0;
   rd.pad = 0;
 
-  return scsi->dev->read (scsi, sizeof (rd), (char *) &rd, size * 
scsi->blocksize, buf);
+  err = scsi->dev->read (scsi, sizeof (rd), (char *) &rd, size * 
scsi->blocksize, buf);
+
+  /* Each SCSI command should be followed by Request Sense.
+     If not so, many devices STALLs or definitely freezes. */
+  err_sense = grub_scsi_request_sense (scsi);
+  /* err_sense is ignored for now and Request Sense Data also... */
+
+  return err;
 }
 
 /* Send a SCSI request for DISK: read SIZE sectors starting with
@@ -129,6 +208,8 @@
 {
   grub_scsi_t scsi;
   struct grub_scsi_read12 rd;
+  grub_err_t err;
+  grub_err_t err_sense;
 
   scsi = disk->data;
 
@@ -139,7 +220,14 @@
   rd.reserved = 0;
   rd.control = 0;
 
-  return scsi->dev->read (scsi, sizeof (rd), (char *) &rd, size * 
scsi->blocksize, buf);
+  err = scsi->dev->read (scsi, sizeof (rd), (char *) &rd, size * 
scsi->blocksize, buf);
+
+  /* Each SCSI command should be followed by Request Sense.
+     If not so, many devices STALLs or definitely freezes. */
+  err_sense = grub_scsi_request_sense (scsi);
+  /* err_sense is ignored for now and Request Sense Data also... */
+
+  return err;
 }
 
 #if 0
@@ -151,6 +239,8 @@
 {
   grub_scsi_t scsi;
   struct grub_scsi_write10 wr;
+  grub_err_t err;
+  grub_err_t err_sense;
 
   scsi = disk->data;
 
@@ -162,7 +252,14 @@
   wr.reserved2 = 0;
   wr.pad = 0;
 
-  return scsi->dev->write (scsi, sizeof (wr), (char *) &wr, size * 
scsi->blocksize, buf);
+  err = scsi->dev->write (scsi, sizeof (wr), (char *) &wr, size * 
scsi->blocksize, buf);
+
+  /* Each SCSI command should be followed by Request Sense.
+     If not so, many devices STALLs or definitely freezes. */
+  err_sense = grub_scsi_request_sense (scsi);
+  /* err_sense is ignored for now and Request Sense Data also... */
+
+  return err;
 }
 
 /* Send a SCSI request for DISK: write the data stored in BUF to SIZE
@@ -173,6 +270,8 @@
 {
   grub_scsi_t scsi;
   struct grub_scsi_write10 wr;
+  grub_err_t err;
+  grub_err_t err_sense;
 
   scsi = disk->data;
 
@@ -183,7 +282,14 @@
   wr.reserved = 0;
   wr.pad = 0;
 
-  return scsi->dev->write (scsi, sizeof (wr), (char *) &wr, size * 
scsi->blocksize, buf);
+  err = scsi->dev->write (scsi, sizeof (wr), (char *) &wr, size * 
scsi->blocksize, buf);
+
+  /* Each SCSI command should be followed by Request Sense.
+     If not so, many devices STALLs or definitely freezes. */
+  err_sense = grub_scsi_request_sense (scsi);
+  /* err_sense is ignored for now and Request Sense Data also... */
+
+  return err;
 }
 #endif
 
@@ -292,6 +398,16 @@
       else
        disk->has_partitions = 1;
 
+
+      /* According to USB MS tests, issue Test Unit Ready until OK */
+      /* XXX: there should be some timeout... */
+      do
+        {
+          err = grub_scsi_test_unit_ready (scsi);
+        }
+      while (err == GRUB_ERR_READ_ERROR);
+
+      /* Read capacity of media */
       err = grub_scsi_read_capacity (scsi);
       if (err)
        {
@@ -302,12 +418,14 @@
 
       /* 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 = ((grub_uint64_t)scsi->size
+                             * (grub_uint64_t)scsi->blocksize)
+                           >> GRUB_DISK_SECTOR_BITS;
 
-      grub_dprintf ("scsi", "capacity=%llu, blksize=%d\n",
-                   (unsigned long long) disk->total_sectors,
-                   scsi->blocksize);
+      grub_dprintf ("scsi", "blocks=%u, blocksize=%u\n",
+                   scsi->size, scsi->blocksize);
+      grub_dprintf ("scsi", "Disk total 512 sectors = %llu\n",
+                   disk->total_sectors);
 
       return GRUB_ERR_NONE;
     }

=== modified file 'disk/usbms.c'
--- disk/usbms.c        2010-01-20 08:12:47 +0000
+++ disk/usbms.c        2010-05-22 22:17:58 +0000
@@ -125,14 +125,16 @@
                {
                  /* Bulk IN endpoint.  */
                  usbms->in = endp;
-                 grub_usb_clear_halt (usbdev, endp->endp_addr & 128);
+                 /* Clear Halt is not possible yet! */
+                 /* grub_usb_clear_halt (usbdev, endp->endp_addr); */
                  usbms->in_maxsz = endp->maxpacket;
                }
              else if (!(endp->endp_addr & 128) && (endp->attrib & 3) == 2)
                {
                  /* Bulk OUT endpoint.  */
                  usbms->out = endp;
-                 grub_usb_clear_halt (usbdev, endp->endp_addr & 128);
+                 /* Clear Halt is not possible yet! */
+                 /* grub_usb_clear_halt (usbdev, endp->endp_addr); */
                  usbms->out_maxsz = endp->maxpacket;
                }
            }
@@ -143,6 +145,9 @@
              return 0;
            }
 
+         /* XXX: Activate the first configuration.  */
+         grub_usb_set_configuration (usbdev, 1);
+
          /* Query the amount of LUNs.  */
          err = grub_usb_control_msg (usbdev, 0xA1, 254,
                                      0, i, 1, (char *) &luns);
@@ -151,8 +156,8 @@
              /* In case of a stall, clear the stall.  */
              if (err == GRUB_USB_ERR_STALL)
                {
-                 grub_usb_clear_halt (usbdev, usbms->in->endp_addr & 3);
-                 grub_usb_clear_halt (usbdev, usbms->out->endp_addr & 3);
+                 grub_usb_clear_halt (usbdev, usbms->in->endp_addr);
+                 grub_usb_clear_halt (usbdev, usbms->out->endp_addr);
                }
 
              /* Just set the amount of LUNs to one.  */
@@ -164,8 +169,9 @@
 
          /* XXX: Check the magic values, does this really make
             sense?  */
-         grub_usb_control_msg (usbdev, (1 << 6) | 1, 255,
-                               0, i, 0, 0);
+         /* What is it? Removed. */
+         /* grub_usb_control_msg (usbdev, (1 << 6) | 1, 255,
+                               0, i, 0, 0); */
 
          /* XXX: To make Qemu work?  */
          if (usbms->luns == 0)
@@ -174,12 +180,12 @@
          usbms->next = grub_usbms_dev_list;
          grub_usbms_dev_list = usbms;
 
-         /* XXX: Activate the first configuration.  */
-         grub_usb_set_configuration (usbdev, 1);
-
+         /* Reset recovery procedure */
          /* Bulk-Only Mass Storage Reset, after the reset commands
             will be accepted.  */
          grub_usbms_reset (usbdev, i);
+         grub_usb_clear_halt (usbdev, usbms->in->endp_addr);
+         grub_usb_clear_halt (usbdev, usbms->out->endp_addr);
 
          return 0;
        }
@@ -225,6 +231,7 @@
   static grub_uint32_t tag = 0;
   grub_usb_err_t err = GRUB_USB_ERR_NONE;
   int retrycnt = 3 + 1;
+  grub_size_t i;
 
  retry:
   retrycnt--;
@@ -240,70 +247,86 @@
   cbw.lun = scsi->lun << GRUB_SCSI_LUN_SHIFT;
   cbw.length = cmdsize;
   grub_memcpy (cbw.cbwcb, cmd, cmdsize);
+  
+  /* Debug print of CBW content. */
+  grub_dprintf ("usb", "CBW: sign=0x%08x tag=0x%08x len=0x%08x\n",
+       cbw.signature, cbw.tag, cbw.transfer_length);
+  grub_dprintf ("usb", "CBW: flags=0x%02x lun=0x%02x CB_len=0x%02x\n",
+       cbw.flags, cbw.lun, cbw.length);
+  grub_dprintf ("usb", "CBW: cmd:\n %02x %02x %02x %02x %02x %02x %02x %02x 
%02x %02x %02x %02x %02x %02x %02x %02x\n",
+       cbw.cbwcb[ 0], cbw.cbwcb[ 1], cbw.cbwcb[ 2], cbw.cbwcb[ 3],
+       cbw.cbwcb[ 4], cbw.cbwcb[ 5], cbw.cbwcb[ 6], cbw.cbwcb[ 7],
+       cbw.cbwcb[ 8], cbw.cbwcb[ 9], cbw.cbwcb[10], cbw.cbwcb[11],
+       cbw.cbwcb[12], cbw.cbwcb[13], cbw.cbwcb[14], cbw.cbwcb[15]);
 
-  /* Write the request.  */
+  /* Write the request.  XXX: Error recovery should be corrected! */
   err = grub_usb_bulk_write (dev->dev, dev->out->endp_addr & 15,
                             sizeof (cbw), (char *) &cbw);
   if (err)
     {
       if (err == GRUB_USB_ERR_STALL)
        {
+         grub_usb_clear_halt (dev->dev, dev->in->endp_addr);
          grub_usb_clear_halt (dev->dev, dev->out->endp_addr);
          goto retry;
        }
       return grub_error (GRUB_ERR_IO, "USB Mass Storage request failed");
     }
 
-  /* Read/write the data.  */
-  if (read_write == 0)
+  /* Read/write the data, (maybe) according to specification.  */
+  if (size && (read_write == 0))
     {
       err = grub_usb_bulk_read (dev->dev, dev->in->endp_addr & 15, size, buf);
-      grub_dprintf ("usb", "read: %d %d\n", err, GRUB_USB_ERR_STALL);
-      if (err)
-       {
-         if (err == GRUB_USB_ERR_STALL)
-           {
-             grub_usb_clear_halt (dev->dev, dev->in->endp_addr);
-             goto retry;
-           }
-         return grub_error (GRUB_ERR_READ_ERROR,
-                            "can't read from USB Mass Storage device");
-       }
+      grub_dprintf ("usb", "read: %d %d\n", err, GRUB_USB_ERR_STALL); 
+      if (err) goto CheckCSW;
+      /* Debug print of received data. */
+      grub_dprintf ("usb", "buf:\n");
+      if (size <= 256)
+        for (i=0; i<size; i++)
+          grub_dprintf ("usb", "0x%02x: 0x%02x\n", i, buf[i]);
+      else
+          grub_dprintf ("usb", "Too much data for debug print...\n");
     }
-  else
+  else if (size)
     {
-      err = grub_usb_bulk_write (dev->dev, dev->in->endp_addr & 15, size, buf);
+      err = grub_usb_bulk_write (dev->dev, dev->out->endp_addr & 15, size, 
buf);
       grub_dprintf ("usb", "write: %d %d\n", err, GRUB_USB_ERR_STALL);
-      if (err)
-       {
-         if (err == GRUB_USB_ERR_STALL)
-           {
-             grub_usb_clear_halt (dev->dev, dev->out->endp_addr);
-             goto retry;
-           }
-         return grub_error (GRUB_ERR_WRITE_ERROR,
-                            "can't write to USB Mass Storage device");
-       }
+      grub_dprintf ("usb", "buf:\n");
+      /* Debug print of sent data. */
+      if (size <= 256)
+        for (i=0; i<size; i++)
+          grub_dprintf ("usb", "0x%02x: 0x%02x\n", i, buf[i]);
+      else
+          grub_dprintf ("usb", "Too much data for debug print...\n");
+      if (err) goto CheckCSW;
     }
 
-  /* Read the status.  */
+  /* Read the status - (maybe) according to specification.  */
+CheckCSW:
   err = grub_usb_bulk_read (dev->dev, dev->in->endp_addr & 15,
                            sizeof (status), (char *) &status);
   if (err)
     {
-      if (err == GRUB_USB_ERR_STALL)
-       {
-         grub_usb_clear_halt (dev->dev, dev->in->endp_addr);
+      grub_usb_clear_halt (dev->dev, dev->in->endp_addr);
+      err = grub_usb_bulk_read (dev->dev, dev->in->endp_addr & 15,
+                               sizeof (status), (char *) &status);
+      if (err)
+        { /* Bulk-only reset device. */
+          grub_usbms_reset (dev->dev, dev->interface);
+          grub_usb_clear_halt (dev->dev, dev->in->endp_addr);
+          grub_usb_clear_halt (dev->dev, dev->out->endp_addr);
          goto retry;
-       }
-      return grub_error (GRUB_ERR_READ_ERROR,
-                        "can't read status from USB Mass Storage device");
+        }
     }
 
-  /* XXX: Magic and check this code.  */
+  /* Debug print of CSW content. */
+  grub_dprintf ("usb", "CSW: sign=0x%08x tag=0x%08x resid=0x%08x\n",
+       status.signature, status.tag, status.residue);
+  grub_dprintf ("usb", "CSW: status=0x%02x\n", status.status);
+  
+  /* If phase error, do bulk-only reset device. */
   if (status.status == 2)
     {
-      /* XXX: Phase error, reset device.  */
       grub_usbms_reset (dev->dev, dev->interface);
       grub_usb_clear_halt (dev->dev, dev->in->endp_addr);
       grub_usb_clear_halt (dev->dev, dev->out->endp_addr);

=== modified file 'include/grub/scsicmd.h'
--- include/grub/scsicmd.h      2008-08-27 15:05:00 +0000
+++ include/grub/scsicmd.h      2010-05-22 22:17:58 +0000
@@ -25,14 +25,24 @@
 #define GRUB_SCSI_REMOVABLE_BIT        7
 #define GRUB_SCSI_LUN_SHIFT    5
 
+struct grub_scsi_test_unit_ready
+{
+  grub_uint8_t opcode;
+  grub_uint8_t lun; /* 7-5 LUN, 4-0 reserved */
+  grub_uint8_t reserved1;
+  grub_uint8_t reserved2;
+  grub_uint8_t reserved3;
+  grub_uint8_t control;
+} __attribute__((packed));
+
 struct grub_scsi_inquiry
 {
   grub_uint8_t opcode;
-  grub_uint8_t lun;
-  grub_uint16_t reserved;
-  grub_uint16_t alloc_length;
-  grub_uint8_t reserved2;
-  grub_uint8_t pad[5];
+  grub_uint8_t lun; /* 7-5 LUN, 4-1 reserved, 0 EVPD */
+  grub_uint8_t page; /* page code if EVPD=1 */
+  grub_uint8_t reserved;
+  grub_uint8_t alloc_length;
+  grub_uint8_t control;
 } __attribute__((packed));
 
 struct grub_scsi_inquiry_data
@@ -47,12 +57,40 @@
   char prodrev[4];
 } __attribute__((packed));
 
+struct grub_scsi_request_sense
+{
+  grub_uint8_t opcode;
+  grub_uint8_t lun; /* 7-5 LUN, 4-0 reserved */
+  grub_uint8_t reserved1;
+  grub_uint8_t reserved2;
+  grub_uint8_t alloc_length;
+  grub_uint8_t control;
+} __attribute__((packed));
+
+struct grub_scsi_request_sense_data
+{
+  grub_uint8_t error_code; /* 7 Valid, 6-0 Err. code */
+  grub_uint8_t segment_number;
+  grub_uint8_t sense_key; /*7 FileMark, 6 EndOfMedia, 5 ILI, 4-0 sense key */
+  grub_uint32_t information;
+  grub_uint8_t additional_sense_length;
+  grub_uint32_t cmd_specific_info;
+  grub_uint8_t additional_sense_code;
+  grub_uint8_t additional_sense_code_qualifier;
+  grub_uint8_t field_replaceable_unit_code;
+  grub_uint8_t sense_key_specific[3];
+  /* there can be additional sense field */
+} __attribute__((packed));
+
 struct grub_scsi_read_capacity
 {
   grub_uint8_t opcode;
-  grub_uint8_t lun;
-  grub_uint8_t reserved[8];
-  grub_uint8_t pad[2];
+  grub_uint8_t lun; /* 7-5 LUN, 4-1 reserved, 0 reserved */
+  grub_uint32_t logical_block_addr; /* only if PMI=1 */
+  grub_uint8_t reserved1;
+  grub_uint8_t reserved2;
+  grub_uint8_t PMI;
+  grub_uint8_t control;
 } __attribute__((packed));
 
 struct grub_scsi_read_capacity_data
@@ -106,11 +144,13 @@
 typedef enum
   {
     grub_scsi_cmd_inquiry = 0x12,
+    grub_scsi_cmd_test_unit_ready = 0x00,
     grub_scsi_cmd_read_capacity = 0x25,
     grub_scsi_cmd_read10 = 0x28,
     grub_scsi_cmd_write10 = 0x2a,
     grub_scsi_cmd_read12 = 0xa8,
-    grub_scsi_cmd_write12 = 0xaa
+    grub_scsi_cmd_write12 = 0xaa,
+    grub_scsi_cmd_request_sense = 0x03
   } grub_scsi_cmd_t;
 
 typedef enum

=== modified file 'include/grub/usbtrans.h'
--- include/grub/usbtrans.h     2010-05-05 08:40:48 +0000
+++ include/grub/usbtrans.h     2010-05-22 22:17:58 +0000
@@ -86,9 +86,9 @@
 
 #define GRUB_USB_REQ_HUB_GET_PORT_STATUS 0x00
 
-#define GRUB_USB_FEATURE_ENDP_HALT     0x01
-#define GRUB_USB_FEATURE_DEV_REMOTE_WU 0x02
-#define GRUB_USB_FEATURE_TEST_MODE     0x04
+#define GRUB_USB_FEATURE_ENDP_HALT     0x00
+#define GRUB_USB_FEATURE_DEV_REMOTE_WU 0x01
+#define GRUB_USB_FEATURE_TEST_MODE     0x02
 
 #define GRUB_USB_HUB_STATUS_CONNECTED  (1 << 0)
 #define GRUB_USB_HUB_STATUS_LOWSPEED   (1 << 9)

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]