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

* [Patch] [bug #26237] multiple problems with usb devices
@ 2010-03-14 22:23 Aleš Nesrsta
  2010-04-02 20:46 ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 1 reply; 41+ messages in thread
From: Aleš Nesrsta @ 2010-03-14 22:23 UTC (permalink / raw)
  To: grub-devel

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

Hello,

according to e-mail from Vladimir Serbinenko I am sending patch related
to (mostly) OHCI problem (bug #26237).
Workaround related to grub_memalign problem is removed from patch
because Vladimir Serbinenko wrotes that it will be corrected.

Patch was done by:

diff -urB  grub2-1.98~experimental.20100120 grub2-1.98~my_patched.20100312 > grub2-1.98~my_patched.20100312.patch

As I shortly read the patch file, there sometimes remains some few
things which can be optimized - sorry, it is mostly the rest from
debugging experiments (even I make cleaning of code today)...

Ales

> Od: Vladimir Serbinenko <INVALID.NOREPLY@gnu.org>
> Komu: Oliver Henshaw <yoho_ahoy@hotmail.com>, Vladimir Serbinenko
> <phcoder@gmail.com>, Ales Nesrsta <starous@volny.cz>, bug-grub@gnu.org
> Předmět: [bug #26237] multiple problems with usb devices
> Datum: Sat, 13 Mar 2010 22:08:44 +0000
> 
> Update of bug #26237 (project grub):
> 
>                  Release:                    None => Bazaar - trunk         
> 
>     _______________________________________________________
> 
> Follow-up Comment #6:
> 
> Hello. I found out the problem with grub_memalign. I'll apply the patch
> tomorrow. Some of the fixes can go directly in while other require
> investigation and perhaps even copyright assignment. Can you remove all
> gratuituous changes (like commenting out dprintfs, adding commented out code
> and change register names) and send patch in unified format (-u option in
> diff) to grub-devel@gnu.org? Porting code from linux is appropriate only if
> license is compatible and porting was approved by maintainer. Are perhaps
> interested in coding EHCI?
> 
>     _______________________________________________________
> 
> Reply to this item at:
> 
>   <http://savannah.gnu.org/bugs/?26237>
> 
> _______________________________________________
>   Message sent via/by Savannah
>   http://savannah.gnu.org/
> 
>

[-- Attachment #2: grub2-1.98~my_patched.20100312.patch --]
[-- Type: text/x-patch, Size: 32273 bytes --]

diff -urB grub2-1.98~experimental.20100120/bus/usb/ohci.c grub2-1.98~my_patched.20100312/bus/usb/ohci.c

--- grub2-1.98~experimental.20100120/bus/usb/ohci.c	2010-01-20 23:42:30.000000000 +0100
+++ grub2-1.98~my_patched.20100312/bus/usb/ohci.c	2010-03-14 20:34:55.000000000 +0100
@@ -84,15 +84,35 @@
   GRUB_OHCI_REG_INTENA,
   GRUB_OHCI_REG_INTDIS,
   GRUB_OHCI_REG_HCCA,
-  GRUB_OHCI_REG_PERIODIC,
+  GRUB_OHCI_REG_PERIODICCURR,
   GRUB_OHCI_REG_CONTROLHEAD,
   GRUB_OHCI_REG_CONTROLCURR,
   GRUB_OHCI_REG_BULKHEAD,
   GRUB_OHCI_REG_BULKCURR,
   GRUB_OHCI_REG_DONEHEAD,
   GRUB_OHCI_REG_FRAME_INTERVAL,
-  GRUB_OHCI_REG_RHUBA = 18,
-  GRUB_OHCI_REG_RHUBPORT = 21
+  GRUB_OHCI_REG_FRAME_REMAINING,
+  GRUB_OHCI_REG_FRAME_NUMBER,
+  GRUB_OHCI_REG_PERIODIC_START,
+  GRUB_OHCI_REG_LS_TRESHOLD,
+  GRUB_OHCI_REG_RHUB_DESC_A,
+  GRUB_OHCI_REG_RHUB_DESC_B,
+  GRUB_OHCI_REG_RHUB_STATUS,
+  GRUB_OHCI_REG_RHUB_PORT_STAT_1,
+  GRUB_OHCI_REG_RHUB_PORT_STAT_2,
+  GRUB_OHCI_REG_RHUB_PORT_STAT_3,
+  GRUB_OHCI_REG_RHUB_PORT_STAT_4,
+  GRUB_OHCI_REG_RHUB_PORT_STAT_5,
+  GRUB_OHCI_REG_RHUB_PORT_STAT_6,
+  GRUB_OHCI_REG_RHUB_PORT_STAT_7,
+  GRUB_OHCI_REG_RHUB_PORT_STAT_8,
+  GRUB_OHCI_REG_RHUB_PORT_STAT_9,
+  GRUB_OHCI_REG_RHUB_PORT_STAT_10,
+  GRUB_OHCI_REG_RHUB_PORT_STAT_11,
+  GRUB_OHCI_REG_RHUB_PORT_STAT_12,
+  GRUB_OHCI_REG_RHUB_PORT_STAT_13,
+  GRUB_OHCI_REG_RHUB_PORT_STAT_14,
+  GRUB_OHCI_REG_RHUB_PORT_STAT_15
 } grub_ohci_reg_t;
 
 static grub_uint32_t
@@ -254,20 +277,34 @@
       break;
     }
 
-  /* Generate no interrupts.  */
+  /* Generate no interrupts. Except last TD in ED but it is handled in
+   *  grub_ohci_transfer */
   token |= 7 << 21;
 
   /* Set the token.  */
   token |= toggle << 24;
   token |= 1 << 25;
 
-  buffer = (grub_uint32_t) data;
-  buffer_end = buffer + size - 1;
+  /* Set "Not accessed" error code */
+  token |= 15 << 28;
+  
+  /* Set correct buffer values in TD if zero transfer occurs */
+  if (size) 
+     {
+	 buffer = (grub_uint32_t) 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
@@ -276,13 +313,15 @@
 {
   struct grub_ohci *o = (struct grub_ohci *) dev->data;
   grub_ohci_ed_t ed;
-  grub_ohci_td_t td_list;
+  grub_ohci_td_t td_list ;
   grub_uint32_t target;
   grub_uint32_t td_tail;
   grub_uint32_t td_head;
   grub_uint32_t status;
   grub_uint32_t control;
   grub_usb_err_t err;
+  grub_uint8_t errcode = 0;
+  grub_ohci_td_t tderr = NULL;
   int i;
 
   /* Allocate an Endpoint Descriptor.  */
@@ -297,7 +336,8 @@
       return GRUB_USB_ERR_INTERNAL;
     }
 
-  grub_dprintf ("ohci", "alloc=%p\n", td_list);
+  grub_dprintf ("ohci", "alloc=%p, transcnt=0x%02x\n",
+                td_list, transfer->transcnt);
 
   /* Setup all Transfer Descriptors.  */
   for (i = 0; i < transfer->transcnt; i++)
@@ -310,6 +350,18 @@
       td_list[i].next_td = grub_cpu_to_le32 (&td_list[i + 1]);
     }
 
+  /* 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);
+  /* For safety's sake we partialy initialize last TD... */
+  td_list[transfer->transcnt].token = 0;
+  td_list[transfer->transcnt].buffer = 0;
+  td_list[transfer->transcnt].buffer_end = 0;
+  td_list[transfer->transcnt].next_td =
+    (grub_uint32_t) &td_list[transfer->transcnt];
+  
   /* Setup the Endpoint Descriptor.  */
 
   /* Set the device address.  */
@@ -336,30 +388,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, (grub_uint32_t) ed);
+	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);
 
@@ -369,21 +439,12 @@
     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,
 			      (grub_uint32_t) ed);
-	grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROLHEAD+1,
-			      (grub_uint32_t) ed);
+	grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROLCURR,
+			      0);
 
 	/* Enable the Control list.  */
 	control |= 1 << 4;
@@ -397,38 +459,59 @@
     }
 
   grub_dprintf ("ohci", "wait for completion\n");
-  grub_dprintf ("ohci", "control=0x%02x status=0x%02x\n",
-		grub_ohci_readreg32 (o, GRUB_OHCI_REG_CONTROL),
-		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;
+      /* Detected a HALT.  */
+      if (grub_le_to_cpu32 (&ed->td_head) & 1)
+        break;
+  
+  /* 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!
+   */
+      if ((grub_ohci_readreg32 (o, GRUB_OHCI_REG_INTSTATUS) & 0x2) != 0)
+        {
+          if ((grub_le_to_cpu32 (o->hcca->donehead) & ~0xf)
+               == (grub_uint32_t) &td_list[transfer->transcnt - 1])
+            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",
+                        o->hcca->donehead);
+          o->hcca->donehead = 0;
+          grub_ohci_writereg32 (o, GRUB_OHCI_REG_INTSTATUS, (1 << 1));
+          continue;
+        }
     }
+  while (1);
 
-  grub_dprintf ("ohci", "complete\n");
-
-/*   if (ed->td_head & 1) */
-/*     err = GRUB_USB_ERR_STALL; */
-/*   else if (ed->td */
-
-
-  if (ed->td_head & 1)
+  grub_dprintf ("ohci", "completed\n");
+  
+  if (grub_le_to_cpu32 (ed->td_head) & 1)
     {
-      grub_uint8_t errcode;
-      grub_ohci_td_t tderr;
-
-      tderr = (grub_ohci_td_t) grub_ohci_readreg32 (o,
-						    GRUB_OHCI_REG_DONEHEAD);
-      errcode = tderr->token >> 28;
+      tderr = (grub_ohci_td_t)
+                (grub_ohci_readreg32 (o, GRUB_OHCI_REG_DONEHEAD) & ~0xf);
+      if (tderr == 0)
+        /* If DONEHEAD==0 it means that correct address is in HCCA.
+         * It should be always, but... */
+        tderr = (grub_ohci_td_t) (grub_le_to_cpu32 (o->hcca->donehead) & ~0xf);
 
+      errcode = grub_le_to_cpu32 (tderr->token) >> 28;
+      
       switch (errcode)
 	{
 	case 0:
@@ -515,9 +598,31 @@
 
   /* 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. */
+  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;
+  grub_ohci_writereg32 (o, GRUB_OHCI_REG_INTSTATUS, (1 << 1)); /* Clears WDH */
+  /* Auxiliary cleaning. */
+  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);
@@ -533,24 +638,24 @@
    grub_uint32_t status;
 
    /* Reset the port.  */
-   status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUBPORT + port);
+   status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUB_PORT_STAT_1 + port);
    status |= (1 << 4); /* XXX: Magic.  */
-   grub_ohci_writereg32 (o, GRUB_OHCI_REG_RHUBPORT + port, status);
+   grub_ohci_writereg32 (o, GRUB_OHCI_REG_RHUB_PORT_STAT_1 + port, status);
    grub_millisleep (100);
 
    /* End the reset signaling.  */
-   status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUBPORT + port);
+   status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUB_PORT_STAT_1 + port);
    status |= (1 << 20); /* XXX: Magic.  */
-   grub_ohci_writereg32 (o, GRUB_OHCI_REG_RHUBPORT + port, status);
+   grub_ohci_writereg32 (o, GRUB_OHCI_REG_RHUB_PORT_STAT_1 + port, status);
    grub_millisleep (10);
 
    /* Enable the port.  */
-   status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUBPORT + port);
+   status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUB_PORT_STAT_1 + port);
    status |= (enable << 1); /* XXX: Magic.  */
-   grub_ohci_writereg32 (o, GRUB_OHCI_REG_RHUBPORT + port, status);
+   grub_ohci_writereg32 (o, GRUB_OHCI_REG_RHUB_PORT_STAT_1 + port, status);
 
-   status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUBPORT + port);
-   grub_dprintf ("ohci", "portstatus=0x%02x\n", status);
+   status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUB_PORT_STAT_1 + port);
+   grub_dprintf ("ohci", "port=0x%02x, portstatus=0x%02x\n", port, status);
 
    return GRUB_ERR_NONE;
 }
@@ -561,9 +666,9 @@
    struct grub_ohci *o = (struct grub_ohci *) dev->data;
    grub_uint32_t status;
 
-   status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUBPORT + port);
+   status = grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUB_PORT_STAT_1 + port);
 
-   grub_dprintf ("ohci", "detect_dev status=0x%02x\n", status);
+   grub_dprintf ("ohci", "detect_dev port=0x%02x, status=0x%02x\n", port, status);
 
    if (! (status & 1))
      return GRUB_USB_SPEED_NONE;
@@ -579,7 +684,7 @@
   struct grub_ohci *o = (struct grub_ohci *) dev->data;
   grub_uint32_t portinfo;
 
-  portinfo = grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUBA);
+  portinfo = grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUB_DESC_A);
 
   grub_dprintf ("ohci", "root hub ports=%d\n", portinfo & 0xFF);
 
diff -urB grub2-1.98~experimental.20100120/bus/usb/usbtrans.c grub2-1.98~my_patched.20100312/bus/usb/usbtrans.c
--- grub2-1.98~experimental.20100120/bus/usb/usbtrans.c	2010-01-20 23:42:30.000000000 +0100
+++ grub2-1.98~my_patched.20100312/bus/usb/usbtrans.c	2010-03-14 20:41:33.000000000 +0100
@@ -54,6 +54,12 @@
     max = 64;
 
   datablocks = (size + max - 1) / max;
+  /* Note: This, together with "simple and effective" toggling of toggle bit,
+   * is not safe, it will work only in the case if
+   * OHCI will realy send one TD as one packet - I think it is not guaranted!
+   * Fortunately, control messages are very short, so prabably we do never
+   * need to split it. But another case are bulk transfers - see note about
+   * toggle bit and data blocks in bulk transfer function.	*/
 
   /* XXX: Discriminate between different types of control
      messages.  */
@@ -103,7 +109,7 @@
       size -= max;
     }
 
-  /* End with an empty OUT transaction.  */
+  /* End with an empty OUT/IN transaction (reverse direction than data).  */
   transfer->transactions[datablocks + 1].size = 0;
   transfer->transactions[datablocks + 1].data = NULL;
   if (reqtype & 128)
@@ -133,6 +138,10 @@
   grub_usb_err_t err;
   int toggle = dev->toggle[endpoint];
 
+  grub_dprintf ("usb",
+		"bulk: endpoint=0x%02x type=0x%02x size=%d\n",
+		endpoint, type, size);
+
   /* Use the maximum packet size given in the endpoint descriptor.  */
   if (dev->initialized)
     {
@@ -178,6 +187,22 @@
       tr->size = (size > max) ? max : size;
       /* XXX: Use the right most bit as the data toggle.  Simple and
 	 effective.  */
+      /* Addendum to XXX: This is simple and effective but it is really not
+       * good idea! Why:
+       * 1. It is a wasting of memory - for each 64 (or less!) bytes we need
+       * one TD = 16 bytes! TD can transfer up to 8k if buffer is 4k page
+       * aligned, or 4k if buffer is not aligned (because there can be only one
+       * 4k page crossing.)			 	     
+       * 2. I think there is not guaranted that OHCI will send each TD as one
+       * packet even if it has buffer size up to max packet size. So we can
+       * possibly lost toggle bit synchronization!
+       * Proposal enhancement - to do...:
+       * A. Change toggle bit control in the way that it is controled by OHCI
+       *    itself during TD transfers and final toggle value is read from
+       *    OHCI. This is MUST if we want to do next point B. 			 	 			 			 			 
+       * B. Change max. buffer size in one TD to 4k, i.e. split data to 4k
+       *    blocks instead of "max" (<=64).
+       */
       tr->toggle = toggle;
       toggle = toggle ? 0 : 1;
       tr->pid = type;
diff -urB grub2-1.98~experimental.20100120/disk/scsi.c grub2-1.98~my_patched.20100312/disk/scsi.c
--- grub2-1.98~experimental.20100120/disk/scsi.c	2010-01-20 23:42:31.000000000 +0100
+++ grub2-1.98~my_patched.20100312/disk/scsi.c	2010-03-14 20:50:57.000000000 +0100
@@ -50,6 +50,57 @@
 }
 
 \f
+/* 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 probably should be followed by Request Sense.
+     If not so, some 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 probably should be followed by Request Sense.
+     If not so, some 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 probably should be followed by Request Sense.
+     If not so, some 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 probably should be followed by Request Sense.
+     If not so, some 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 probably should be followed by Request Sense.
+     If not so, some 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 probably should be followed by Request Sense.
+     If not so, some 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 probably should be followed by Request Sense.
+     If not so, some 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
 
@@ -290,6 +396,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)
 	{
@@ -300,12 +416,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);
-
-      grub_dprintf ("scsi", "capacity=%llu, blksize=%d\n",
-		    (unsigned long long) disk->total_sectors,
-		    scsi->blocksize);
+      disk->total_sectors = ((grub_uint64_t)scsi->size
+                             * (grub_uint64_t)scsi->blocksize)
+			    >> GRUB_DISK_SECTOR_BITS;
+
+      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;
     }
diff -urB grub2-1.98~experimental.20100120/disk/usbms.c grub2-1.98~my_patched.20100312/disk/usbms.c
--- grub2-1.98~experimental.20100120/disk/usbms.c	2010-01-20 23:42:31.000000000 +0100
+++ grub2-1.98~my_patched.20100312/disk/usbms.c	2010-03-14 21:10:57.000000000 +0100
@@ -125,14 +125,12 @@
 		{
 		  /* Bulk IN endpoint.  */
 		  usbms->in = endp;
-		  grub_usb_clear_halt (usbdev, endp->endp_addr & 128);
 		  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);
 		  usbms->out_maxsz = endp->maxpacket;
 		}
 	    }
@@ -143,6 +141,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 +152,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.  */
@@ -162,11 +163,6 @@
 	  else
 	    usbms->luns = luns;
 
-	  /* XXX: Check the magic values, does this really make
-	     sense?  */
-	  grub_usb_control_msg (usbdev, (1 << 6) | 1, 255,
-				0, i, 0, 0);
-
 	  /* XXX: To make Qemu work?  */
 	  if (usbms->luns == 0)
 	    usbms->luns = 1;
@@ -174,12 +170,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;
 	}
@@ -240,70 +236,56 @@
   cbw.lun = scsi->lun << GRUB_SCSI_LUN_SHIFT;
   cbw.length = cmdsize;
   grub_memcpy (cbw.cbwcb, cmd, cmdsize);
-
-  /* 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;
     }
-  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");
-	}
+      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.  */
+  /* 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);
diff -urB grub2-1.98~experimental.20100120/include/grub/scsicmd.h grub2-1.98~my_patched.20100312/include/grub/scsicmd.h
--- grub2-1.98~experimental.20100120/include/grub/scsicmd.h	2010-01-20 23:42:31.000000000 +0100
+++ grub2-1.98~my_patched.20100312/include/grub/scsicmd.h	2010-03-09 18:19:07.000000000 +0100
@@ -25,14 +25,24 @@
 #define GRUB_SCSI_REMOVABLE_BIT	7
 #define GRUB_SCSI_LUN_SHIFT	5
 
-struct grub_scsi_inquiry
+struct grub_scsi_test_unit_ready
 {
   grub_uint8_t opcode;
-  grub_uint8_t lun;
-  grub_uint16_t reserved;
-  grub_uint16_t alloc_length;
+  grub_uint8_t lun; /* 7-5 LUN, 4-0 reserved */
+  grub_uint8_t reserved1;
   grub_uint8_t reserved2;
-  grub_uint8_t pad[5];
+  grub_uint8_t reserved3;
+  grub_uint8_t control;
+} __attribute__((packed));
+
+struct grub_scsi_inquiry
+{
+  grub_uint8_t opcode;
+  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
diff -urB grub2-1.98~experimental.20100120/include/grub/usbtrans.h grub2-1.98~my_patched.20100312/include/grub/usbtrans.h
--- grub2-1.98~experimental.20100120/include/grub/usbtrans.h	2010-01-20 23:42:31.000000000 +0100
+++ grub2-1.98~my_patched.20100312/include/grub/usbtrans.h	2010-03-06 13:04:12.000000000 +0100
@@ -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)


^ permalink raw reply	[flat|nested] 41+ messages in thread