Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> > The issue was that we were using getStatus every time we polled for new
> > devices which suposedly (I fixed few other bugs in the code I used at
> > the time so, I'm not sure) drove hub in my USB keyboard crazy. The
> > correct way is to poll interrupt pipe.
> >
> Just to be clear: polling interrupt pipe on hub is what I have
> implemented in keylayouts, just so you don't implement it again
> > Another thing I added is the ability to do background transfers. It is
> > important for USB keyboard support because otherwise we lose messages on
> > keyboard interrupt pipe. It triggered a bug in uhci module. Now there
> > are 2 issues:
> > 1) code is new or modified. Needs testing.
> > 2) On yeeloong the data read from mass storage is sometimes corrupted.
> > Happens in mainline, not sure about other branches. It seems that it
> > wasn't the case before. It's either a regression (perhaps from my code
> > for partial transfers) or cache issues (some cache isn't correctly flushed)
> >
> >> Could I help You with it - at least with testing ?
> >>
> >>
> > Yes, a test run of keylayouts branch on your hardware would be helpful.
> > Especially I'm interested if USB data corruption happens in your case too.
Hi Vladimir,
I made some tests on machine with UHCI with kbdlayouts branch (rev.
2424).
I did not notice any evident data corruption. But there were some
another odd results:
1.
My USB keyboard is low speed device (Genius KB-06XE, model no. K639).
Low speed device transfer was not properly handled - I made some simple
patch - see attachment, I did not commit it into repository.
Control type transfer with keyboard is working with this patch, but bulk
(interrupt) transfer returns always err=7 and in UHCI TD status&control
are set bits "CRC/Time Out Error" (bit 18) and "Stalled" (bit 22). It is
the same behavior as some normal full speed mass-storage devices do also
(I reported it in some previous e-mails). I have some idea right now -
probably there is bad handling of UHCI transfer status in uhci.c when
more than one bit is set - GRUB_USB_ERR_TIMEOUT is returned instead of
GRUB_USB_ERR_STALL. I will try to play with this part during weekend and
I will report to You if some success happens...
2.
Next bad thing is some problem with device attachment detection or
handling of newly attached device on non-root hubs. Behavior of this
problem looks little bit randomly and probably depends also on used port
of hub - some ports are often working, some not (but all hub ports are
working normally in Linux/Windows). I did not find reason yet.
3.
Sometimes partitions of USB mass storage devices were not properly
detected - maybe disk cache problem again?
Maybe it is data corruption which is reported by you - but I never
detected data corruption when I read data from files.
---
I am not sure if interrupt transfers can be handled via bulk queue on
OHCI - according specification, it should be handled via interrupt
pointers table which is currently not implemented in ohci.c. Did you
test background polling of interrupt pipe on some PC with OHCI?
---
At the end some maybe bad idea - if I am not wrong, two simultaneous
transfers can happen (be active in UHCI/OHCI) with current background
bulk/interrupt transfer. There is question if are all related functions
fully reentrant ? I.e., is correct handling of some shared data ?
For the first look I don't see such problem - with one exception:
donehead interrupt can be probably incorrectly handled in ohci.c - it
can be detected and misinterpreted by wrong call of
grub_ohci_check_transfer. The first aid in this case can be forcing of
OHCI driver into "bad OHCI" mode, i.e. donehead interrupt ignoring - in
this mode it should work properly.
Regards
Ales
--- kbdlayouts/grub-core/bus/usb/uhci.c 2010-09-03 22:13:28.000000000 +0200
+++ kbdlayouts_changed/grub-core/bus/usb/uhci.c 2010-09-01 21:10:24.000000000 +0200
@@ -414,7 +414,7 @@
grub_uhci_transaction (struct grub_uhci *u, unsigned int endp,
grub_transfer_type_t type, unsigned int addr,
unsigned int toggle, grub_size_t size,
- grub_uint32_t data)
+ grub_uint32_t data, grub_usb_speed_t speed)
{
grub_uhci_td_t td;
static const unsigned int tf[] = { 0x69, 0xE1, 0x2D };
@@ -439,7 +439,8 @@
td->linkptr = 1;
/* Active! Only retry a transfer 3 times. */
- td->ctrl_status = (1 << 23) | (3 << 27);
+ td->ctrl_status = (1 << 23) | (3 << 27) |
+ ((speed == GRUB_USB_SPEED_LOW) ? (1 << 26) : 0);
/* If zero bytes are transmitted, size is 0x7FF. Otherwise size is
size-1. */
@@ -495,7 +496,8 @@
td = grub_uhci_transaction (u, transfer->endpoint & 15, tr->pid,
transfer->devaddr, tr->toggle,
- tr->size, tr->data);
+ tr->size, tr->data,
+ transfer->dev->speed);
if (! td)
{
grub_size_t actual = 0;
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
