From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1SZhlE-00017R-Uf for mharc-grub-devel@gnu.org; Wed, 30 May 2012 08:12:20 -0400 Received: from eggs.gnu.org ([208.118.235.92]:45343) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SZhl5-0000dt-Cy for grub-devel@gnu.org; Wed, 30 May 2012 08:12:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SZhkx-0003Zb-UG for grub-devel@gnu.org; Wed, 30 May 2012 08:12:10 -0400 Received: from 37-46-169-123.customers.ownit.se ([37.46.169.123]:57398 helo=zoo.weinigel.se) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SZhkx-0003YZ-Fe for grub-devel@gnu.org; Wed, 30 May 2012 08:12:03 -0400 Received: from [127.0.0.1] (zoo.weinigel.se [127.0.0.1]) by zoo.weinigel.se (Postfix) with ESMTP id 16AAB60D07; Wed, 30 May 2012 14:12:01 +0200 (CEST) Message-ID: <4FC60E90.2030703@weinigel.se> Date: Wed, 30 May 2012 14:12:00 +0200 From: Christer Weinigel User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120430 Thunderbird/12.0.1 MIME-Version: 1.0 To: The development of GNU GRUB Subject: Re: EHCI driver References: <4FC491EC.8010808@weinigel.se> <1338331586.6016.156.camel@king.jenpiliny.cz> <4FC5E820.1060505@weinigel.se> In-Reply-To: <4FC5E820.1060505@weinigel.se> X-Enigmail-Version: 1.4.1 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 37.46.169.123 Cc: =?UTF-8?B?QWxlxaEgTmVzcnN0YQ==?= X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 30 May 2012 12:12:18 -0000 On 2012-05-30 11:28, Christer Weinigel wrote: > [talk about EHCI problems with low speed split transactions] > Now I just have to fix up the queue management so that it properly keeps > track of QHs for two queues instead of one. And figure out if there are > any other problems. So, how about something like this? Fix low-speed USB split transactions on EHCI Low-speed split transactions did not work on EHCI. The reason is that only control and interrupt transfers are allowed with low-speed, there is no such thing as bulk transfers for low-speed. GRUB doesn't know about interrupt transfers, it always uses bulk transfers, and the EHCI driver did not support interrupt transfers either. It seems that when doing split transactions from a QH (queue header) on the asynchronous schedule in the EHCI controller, the SPLIT packet on the bus will be for a full-speed transfer, even if the QH says that it is a low-speed transfer. To actually get a low-speed transfer the QH has to be on the periodic schedule. This patch adds a hack that checks if a bulk transfer is for a low-speed device and in that case puts the QH on the periodic schedule instead of on the asynchronous schedule. Checking for bulk transfers to a low speed device is a rather ugly hack, it would be better to implement proper support for interrupt transfers in GRUB. But this made my keyboard work with minimal changes. The same queue of transfers is used for all 1024 frames in the periodic schedule, so the interrupt transfer will be retried every frame, there is no support for longer poll times. I don't know if that is a problem in practice, but I could imagine that some slow device might choke if the host polls it too often. To be able to use the same pool of QHs for both the periodic and the asynchronous schedules the way QHs are allocated had to change a bit. Instead of qh_virt[i-1] always being followed by qh_virt[i], they are now proper linked lists. When cancelling a transfer we have to look through the list for the previous QH instead of just using qh_virt[i-1]. On the other hand, cancelling happens very seldom so this doesn't really affect performance. The declarations of GRUB_EHCI_MULT_ONE, TWO and THREE were all zero which is explicitly disallowed by the EHCI specification: "A zero in this field yields undefined results." I changed the defines to have the correct values. BTW, there is one really strange issue when I have multiple keyboards For example if I have three keyboards, with devices addresses 2, 3, and 4. If I unplug and replug keyboard 3, this for some reason sets the halted flags or clears the active flags for keyboards 2 and 4. The alternate code selected by #if in grub_ehci_cancel_transfer makes things work, but I have no idea why which is a bit frightening. Note, this change is against some old copy of ehci.c from a few months ago, so it probably won't apply to the latest code in bazaar. But if you think this looks good, I can figure out how to check out the bazaar repo again and make a new patch. I'm trying to use thunderbird to send this patch, so it might be a bit mangled, but hopefully I'm managed to get it right. /Christer diff --git a/grub-core/bus/usb/ehci.c b/grub-core/bus/usb/ehci.c index 0f41361..d8ecf26 100644 --- a/grub-core/bus/usb/ehci.c +++ b/grub-core/bus/usb/ehci.c @@ -209,18 +209,22 @@ enum { GRUB_EHCI_MULT_MASK = (3 << 30), GRUB_EHCI_MULT_RESERVED = (0 << 30), - GRUB_EHCI_MULT_ONE = (0 << 30), - GRUB_EHCI_MULT_TWO = (0 << 30), - GRUB_EHCI_MULT_THREE = (0 << 30), + GRUB_EHCI_MULT_ONE = (1 << 30), + GRUB_EHCI_MULT_TWO = (2 << 30), + GRUB_EHCI_MULT_THREE = (3 << 30), GRUB_EHCI_DEVPORT_MASK = (0x7f << 23), - GRUB_EHCI_HUBADDR_MASK = (0x7f << 16) + GRUB_EHCI_HUBADDR_MASK = (0x7f << 16), + GRUB_EHCI_CMASK_MASK = (0xff << 8), + GRUB_EHCI_SMASK_MASK = (0xff << 0), }; enum { GRUB_EHCI_MULT_OFF = 30, GRUB_EHCI_DEVPORT_OFF = 23, - GRUB_EHCI_HUBADDR_OFF = 16 + GRUB_EHCI_HUBADDR_OFF = 16, + GRUB_EHCI_CMASK_OFF = 8, + GRUB_EHCI_SMASK_OFF = 0, }; #define GRUB_EHCI_TERMINATE (1<<0) @@ -782,6 +786,7 @@ grub_ehci_pci_iter (grub_pci_device_t dev, /* Enable asynchronous list */ grub_ehci_oper_write32 (e, GRUB_EHCI_COMMAND, GRUB_EHCI_CMD_AS_ENABL + | GRUB_EHCI_CMD_PS_ENABL | grub_ehci_oper_read32 (e, GRUB_EHCI_COMMAND)); /* Now should be possible to power-up and enumerate ports etc. */ @@ -926,6 +931,12 @@ grub_ehci_setup_qh (grub_ehci_qh_t qh, grub_usb_transfer_t transfer) & GRUB_EHCI_DEVPORT_MASK; ep_cap |= (transfer->dev->hubaddr << GRUB_EHCI_HUBADDR_OFF) & GRUB_EHCI_HUBADDR_MASK; + if (transfer->dev->speed == GRUB_USB_SPEED_LOW && + transfer->type != GRUB_USB_TRANSACTION_TYPE_CONTROL) + { + ep_cap |= (1<<0) << GRUB_EHCI_SMASK_OFF; + ep_cap |= (7<<2) << GRUB_EHCI_CMASK_OFF; + } qh->ep_cap = grub_cpu_to_le32 (ep_cap); grub_dprintf ("ehci", "setup_qh: qh=%08x, not changed: qh_hptr=%08x\n", @@ -949,6 +960,7 @@ grub_ehci_find_qh (struct grub_ehci *e, grub_usb_transfer_t transfer) grub_uint32_t target, mask; int i; grub_ehci_qh_t qh = e->qh_virt; + grub_ehci_qh_t head; /* Prepare part of EP Characteristic to find existing QH */ target = ((transfer->endpoint << GRUB_EHCI_EP_NUM_OFF) | @@ -983,14 +995,21 @@ grub_ehci_find_qh (struct grub_ehci *e, grub_usb_transfer_t transfer) * de-allocate QHs of unplugged devices. */ /* We should preset new QH and link it into AL */ grub_ehci_setup_qh (&qh[i], transfer); - /* Linking - this new (last) QH will point to first QH */ - qh[i].qh_hptr = grub_cpu_to_le32 (GRUB_EHCI_HPTR_TYPE_QH - | grub_dma_virt2phys (&qh[1], - e->qh_chunk)); - /* Linking - previous last QH will point to this new QH */ - qh[i - 1].qh_hptr = grub_cpu_to_le32 (GRUB_EHCI_HPTR_TYPE_QH - | grub_dma_virt2phys (&qh[i], - e->qh_chunk)); + + /* low speed interrupt transfers are linked to the periodic + * scheudle, everything else to the asynchronous schedule */ + if (transfer->dev->speed == GRUB_USB_SPEED_LOW && + transfer->type != GRUB_USB_TRANSACTION_TYPE_CONTROL) + head = &qh[0]; + else + head = &qh[1]; + + /* Linking - this new (last) QH will copy the QH from the head QH */ + qh[i].qh_hptr = head->qh_hptr; + /* Linking - the head QH will point to this new QH */ + head->qh_hptr = grub_cpu_to_le32 (GRUB_EHCI_HPTR_TYPE_QH + | grub_dma_virt2phys (&qh[i], + e->qh_chunk)); return &qh[i]; } @@ -1221,7 +1240,7 @@ grub_ehci_setup_transfer (grub_usb_controller_t dev, /* XXX: Fix it: Currently we don't do anything to restart EHCI */ return GRUB_USB_ERR_INTERNAL; if ((grub_ehci_oper_read32 (e, GRUB_EHCI_STATUS) - & GRUB_EHCI_ST_AS_STATUS) == 0) + & (GRUB_EHCI_ST_AS_STATUS | GRUB_EHCI_ST_PS_STATUS)) == 0) /* XXX: Fix it: Currently we don't do anything to restart EHCI */ return GRUB_USB_ERR_INTERNAL; @@ -1372,6 +1391,7 @@ grub_ehci_parse_notrun (grub_usb_controller_t dev, /* Try enable EHCI and AL */ grub_ehci_oper_write32 (e, GRUB_EHCI_COMMAND, GRUB_EHCI_CMD_RUNSTOP | GRUB_EHCI_CMD_AS_ENABL + | GRUB_EHCI_CMD_PS_ENABL | grub_ehci_oper_read32 (e, GRUB_EHCI_COMMAND)); /* Ensure command is written */ grub_ehci_oper_read32 (e, GRUB_EHCI_COMMAND); @@ -1470,7 +1490,7 @@ grub_ehci_check_transfer (grub_usb_controller_t dev, & GRUB_EHCI_ST_HC_HALTED) != 0) return grub_ehci_parse_notrun (dev, transfer, actual); if ((grub_ehci_oper_read32 (e, GRUB_EHCI_STATUS) - & GRUB_EHCI_ST_AS_STATUS) == 0) + & (GRUB_EHCI_ST_AS_STATUS | GRUB_EHCI_ST_PS_STATUS)) == 0) return grub_ehci_parse_notrun (dev, transfer, actual); token = grub_le_to_cpu32 (cdata->qh_virt->td_overlay.token); @@ -1508,6 +1528,7 @@ grub_ehci_cancel_transfer (grub_usb_controller_t dev, grub_size_t actual; int i; grub_uint64_t maxtime; + grub_uint32_t qh_phys; /* QH can be active and should be de-activated and halted */ @@ -1518,7 +1539,7 @@ grub_ehci_cancel_transfer (grub_usb_controller_t dev, if (((grub_ehci_oper_read32 (e, GRUB_EHCI_STATUS) & GRUB_EHCI_ST_HC_HALTED) != 0) || ((grub_ehci_oper_read32 (e, GRUB_EHCI_STATUS) - & GRUB_EHCI_ST_AS_STATUS) == 0)) + & (GRUB_EHCI_ST_AS_STATUS | GRUB_EHCI_ST_PS_STATUS)) == 0)) { grub_ehci_pre_finish_transfer (transfer); grub_ehci_free_tds (e, cdata->td_first_virt, transfer, &actual); @@ -1530,44 +1551,83 @@ grub_ehci_cancel_transfer (grub_usb_controller_t dev, /* EHCI and AL are running. What to do? * Try to Halt QH via de-scheduling QH. */ - /* Find index of current QH - we need previous QH, i.e. i-1 */ - i = ((int) (e->qh_virt - cdata->qh_virt)) / sizeof (struct grub_ehci_qh); + /* Find index of previous QH */ + qh_phys = grub_dma_virt2phys(cdata->qh_virt, e->qh_chunk); + for (i = 0; i < GRUB_EHCI_N_QH; i++) + { + if ((e->qh_virt[i].qh_hptr & GRUB_EHCI_QHTDPTR_MASK) == qh_phys) + break; + } + if (i == GRUB_EHCI_N_QH) + { + grub_printf ("%s: prev not found, queues are corrupt\n", __func__); + return GRUB_USB_ERR_UNRECOVERABLE; + } /* Unlink QH from AL */ - e->qh_virt[i - 1].qh_hptr = cdata->qh_virt->qh_hptr; - /* Ring the doorbell */ - grub_ehci_oper_write32 (e, GRUB_EHCI_COMMAND, - GRUB_EHCI_CMD_AS_ADV_D - | grub_ehci_oper_read32 (e, GRUB_EHCI_COMMAND)); - /* Ensure command is written */ - grub_ehci_oper_read32 (e, GRUB_EHCI_COMMAND); - /* Wait answer with timeout */ - maxtime = grub_get_time_ms () + 2; - while (((grub_ehci_oper_read32 (e, GRUB_EHCI_STATUS) - & GRUB_EHCI_ST_AS_ADVANCE) == 0) - && (grub_get_time_ms () < maxtime)); + e->qh_virt[i].qh_hptr = cdata->qh_virt->qh_hptr; + + /* If this is an interrupt transfer, we just wait for the periodic + * schedule to advance a few times and then assume that the EHCI + * controller has read the updated QH. */ + if (cdata->qh_virt->ep_cap & GRUB_EHCI_SMASK_MASK) + { + grub_millisleep(20); + } + else + { + /* For the asynchronous schedule we use the advance doorbell to find + * out when the EHCI controller has read the updated QH. */ - /* We do not detect the timeout because if timeout occurs, it most - * probably means something wrong with EHCI - maybe stopped etc. */ + /* Ring the doorbell */ + grub_ehci_oper_write32 (e, GRUB_EHCI_COMMAND, + GRUB_EHCI_CMD_AS_ADV_D + | grub_ehci_oper_read32 (e, GRUB_EHCI_COMMAND)); + /* Ensure command is written */ + grub_ehci_oper_read32 (e, GRUB_EHCI_COMMAND); + /* Wait answer with timeout */ + maxtime = grub_get_time_ms () + 2; + while (((grub_ehci_oper_read32 (e, GRUB_EHCI_STATUS) + & GRUB_EHCI_ST_AS_ADVANCE) == 0) + && (grub_get_time_ms () < maxtime)); - /* Shut up the doorbell */ - grub_ehci_oper_write32 (e, GRUB_EHCI_COMMAND, - ~GRUB_EHCI_CMD_AS_ADV_D - & grub_ehci_oper_read32 (e, GRUB_EHCI_COMMAND)); - grub_ehci_oper_write32 (e, GRUB_EHCI_STATUS, - GRUB_EHCI_ST_AS_ADVANCE - | grub_ehci_oper_read32 (e, GRUB_EHCI_STATUS)); - /* Ensure command is written */ - grub_ehci_oper_read32 (e, GRUB_EHCI_STATUS); + /* We do not detect the timeout because if timeout occurs, it most + * probably means something wrong with EHCI - maybe stopped etc. */ + + /* Shut up the doorbell */ + grub_ehci_oper_write32 (e, GRUB_EHCI_COMMAND, + ~GRUB_EHCI_CMD_AS_ADV_D + & grub_ehci_oper_read32 (e, GRUB_EHCI_COMMAND)); + grub_ehci_oper_write32 (e, GRUB_EHCI_STATUS, + GRUB_EHCI_ST_AS_ADVANCE + | grub_ehci_oper_read32 (e, GRUB_EHCI_STATUS)); + /* Ensure command is written */ + grub_ehci_oper_read32 (e, GRUB_EHCI_STATUS); + } /* Now is QH out of AL and we can do anything with it... */ grub_ehci_pre_finish_transfer (transfer); grub_ehci_free_tds (e, cdata->td_first_virt, transfer, &actual); grub_ehci_free_td (e, cdata->td_alt_virt); + /* FIXME Putting the QH back on the list should work, but for some + * strange reason doing that will affect other QHs on the periodic + * list. So free the QH instead of putting it back on the list + * which does seem to work, but I would like to know why. */ + +#if 0 /* Finaly we should return QH back to the AL... */ - e->qh_virt[i - 1].qh_hptr = + e->qh_virt[i].qh_hptr = grub_cpu_to_le32 (grub_dma_virt2phys (cdata->qh_virt, e->qh_chunk)); +#else + /* Free the QH */ + cdata->qh_virt->ep_char = 0; + cdata->qh_virt->qh_hptr = + grub_cpu_to_le32 ((grub_dma_virt2phys (cdata->qh_virt, + e->qh_chunk) & + GRUB_EHCI_POINTER_MASK) | GRUB_EHCI_HPTR_TYPE_QH); +#endif + grub_free (cdata); grub_dprintf ("ehci", "cancel_transfer: end\n"); @@ -1777,6 +1837,7 @@ grub_ehci_restore_hw (void) /* Enable asynchronous list */ grub_ehci_oper_write32 (e, GRUB_EHCI_COMMAND, GRUB_EHCI_CMD_AS_ENABL + | GRUB_EHCI_CMD_PS_ENABL | grub_ehci_oper_read32 (e, GRUB_EHCI_COMMAND)); /* Now should be possible to power-up and enumerate ports etc. */