grub-devel.gnu.org archive mirror
 help / color / mirror / Atom feed
From: "Aleš Nesrsta" <starous@volny.cz>
To: The development of GNU GRUB <grub-devel@gnu.org>
Subject: [PATCH] Trunk: usbtrans.c - wrong max packet size for bulk transfer
Date: Thu, 23 Sep 2010 22:27:27 +0200	[thread overview]
Message-ID: <1285273648.6015.30.camel@pracovna> (raw)
In-Reply-To: <20100923130819.GV21862@riva.ucam.org>

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

Hi,

I discovered problem which happens if bulk EP has shorter wMaxPacketSize
value than control (zero) EP.
There was bug in usbtrans.c, see patch.

Another problem happens when bulk EP has wMaxPacketSize <= 16 - in this
case transfer error happens because it is not possible to allocate
sufficient number of TDs.
It can happen - according USB1.1 specification, any bulk EP can have
wMaxPacketSize value equal to 8, 16, 32 or 64 (USB 2.0 capable bulk EP
can have 512).
So I increased number of TDs in UHCI and OHCI driver to 640 - it should
be enough for the worse case wMaxPacketSize=8, with some reserve for
"background" communication.

I successfully tested included patch on UHCI with GARMIN OREGON 300
which has:
EP1 (OUT) wMaxPacketSize = 64
EP3 (IN)  wMaxPacketSize = 16
(i.e. this device is related to both problems)

Regards
Ales


[-- Attachment #2: usb_maxpacketbulk_100923_0 --]
[-- Type: text/x-patch, Size: 2189 bytes --]

diff -urB ./grub/grub-core/bus/usb/ohci.c ./grub_patched/grub-core/bus/usb/ohci.c
--- ./grub/grub-core/bus/usb/ohci.c	2010-09-23 21:45:45.000000000 +0200
+++ ./grub_patched/grub-core/bus/usb/ohci.c	2010-09-23 21:13:00.000000000 +0200
@@ -150,7 +150,7 @@
 #define GRUB_OHCI_RESET_CONNECT_CHANGE (1 << 16)
 #define GRUB_OHCI_CTRL_EDS 256
 #define GRUB_OHCI_BULK_EDS 510
-#define GRUB_OHCI_TDS 256
+#define GRUB_OHCI_TDS 640
 
 #define GRUB_OHCI_ED_ADDR_MASK 0x7ff
 
diff -urB ./grub/grub-core/bus/usb/uhci.c ./grub_patched/grub-core/bus/usb/uhci.c
--- ./grub/grub-core/bus/usb/uhci.c	2010-09-23 21:45:45.000000000 +0200
+++ ./grub_patched/grub-core/bus/usb/uhci.c	2010-09-23 21:11:58.000000000 +0200
@@ -29,6 +29,7 @@
 #define GRUB_UHCI_IOMASK	(0x7FF << 5)
 
 #define N_QH  256
+#define N_TD  640
 
 typedef enum
   {
@@ -105,7 +106,7 @@
   /* N_QH Queue Heads.  */
   grub_uhci_qh_t qh;
 
-  /* 256 Transfer Descriptors.  */
+  /* N_TD Transfer Descriptors.  */
   grub_uhci_td_t td;
 
   /* Free Transfer Descriptors.  */
@@ -227,7 +228,7 @@
 
   /* The TD pointer of UHCI is only 32 bits, make sure this
      code works on on 64 bits architectures.  */
-  u->td = (grub_uhci_td_t) grub_memalign (4096, 4096*2);
+  u->td = (grub_uhci_td_t) grub_memalign (4096, 32*N_TD);
   if (! u->td)
     goto fail;
 
@@ -244,9 +245,9 @@
 
   /* Link all Transfer Descriptors in a list of available Transfer
      Descriptors.  */
-  for (i = 0; i < 256; i++)
+  for (i = 0; i < N_TD; i++)
     u->td[i].linkptr = (grub_uint32_t) (grub_addr_t) &u->td[i + 1];
-  u->td[255 - 1].linkptr = 0;
+  u->td[N_TD - 2].linkptr = 0;
   u->tdfree = u->td;
 
   /* Make sure UHCI is disabled!  */
diff -urB ./grub/grub-core/bus/usb/usbtrans.c ./grub_patched/grub-core/bus/usb/usbtrans.c
--- ./grub/grub-core/bus/usb/usbtrans.c	2010-09-23 21:45:45.000000000 +0200
+++ ./grub_patched/grub-core/bus/usb/usbtrans.c	2010-09-23 21:13:56.000000000 +0200
@@ -228,7 +228,7 @@
   if (dev->initialized)
     {
       struct grub_usb_desc_endp *endpdesc;
-      endpdesc = grub_usb_get_endpdescriptor (dev, 0);
+      endpdesc = grub_usb_get_endpdescriptor (dev, endpoint);
 
       if (endpdesc)
 	max = endpdesc->maxpacket;

  parent reply	other threads:[~2010-09-23 20:27 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-21 18:35 [PATCH] Re-enable grub-extras Colin Watson
2010-09-23 13:08 ` Colin Watson
2010-09-23 13:12   ` Colin Watson
2010-09-23 14:23     ` Colin Watson
2010-09-23 14:36   ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-09-24  9:09     ` Colin Watson
2010-09-23 20:27   ` Aleš Nesrsta [this message]
2010-10-01  7:35     ` [PATCH] Trunk: usbtrans.c - wrong max packet size for bulk transfer Vladimir 'φ-coder/phcoder' Serbinenko
2010-10-01 15:07       ` Aleš Nesrsta
2010-10-01 18:33         ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-10-02 18:58           ` Aleš Nesrsta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1285273648.6015.30.camel@pracovna \
    --to=starous@volny.cz \
    --cc=grub-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).