From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1Sa4BK-0004R7-NZ for mharc-grub-devel@gnu.org; Thu, 31 May 2012 08:08:46 -0400 Received: from eggs.gnu.org ([208.118.235.92]:53484) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sa4BG-0004Qo-Ie for grub-devel@gnu.org; Thu, 31 May 2012 08:08:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Sa4B7-00028P-2N for grub-devel@gnu.org; Thu, 31 May 2012 08:08:42 -0400 Received: from mail-ey0-f169.google.com ([209.85.215.169]:44753) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sa4B6-00028H-Lk for grub-devel@gnu.org; Thu, 31 May 2012 08:08:32 -0400 Received: by eaan1 with SMTP id n1so311118eaa.0 for ; Thu, 31 May 2012 05:08:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:subject:references :in-reply-to:x-enigmail-version:content-type; bh=SgPTQnSOVyKVhui4/PnstPeWwfpcvfqZcLivFaR0L/4=; b=o74AEPFrZnSIcKJfQb+SXQJK8mlvpcOcNyEjDmlGGoTZZc35TzGRtlWuEERbb9udsz j5CzGkIR+4dTeX+gx2+Xm8s2JvHsBCehqHdD7BRphab4oDGz6OYmewnd6SKxTwnDjafb Kn3YfNDFeNZvZHsd1t+nBGIeeGzLkGumMDYQFac5UpqT7uQvE7Nt9Q1HkOUFVxn2abR3 KVvYYFZSY6oK2/3ih40IfLbeC+FZnPzrkoyZcHCIWbyq/2vummQUrrFH/ODxrNBFCMl/ FQdHhdBy9ZMKvMeK36eDJKMb2FBrhC3Fq0VJGFPyPh+noV01ZhQ396mUxoMgtXnkjMO1 ykYw== Received: by 10.14.100.194 with SMTP id z42mr9274585eef.146.1338466110504; Thu, 31 May 2012 05:08:30 -0700 (PDT) Received: from debian.x201.phnet (59-232.197-178.cust.bluewin.ch. [178.197.232.59]) by mx.google.com with ESMTPS id f16sm9846996eec.2.2012.05.31.05.08.27 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 31 May 2012 05:08:28 -0700 (PDT) Message-ID: <4FC75F3A.6040706@gmail.com> Date: Thu, 31 May 2012 14:08:26 +0200 From: =?UTF-8?B?VmxhZGltaXIgJ8+GLWNvZGVyL3BoY29kZXInIFNlcmJpbmVua28=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.4) Gecko/20120510 Icedove/10.0.4 MIME-Version: 1.0 To: grub-devel@gnu.org Subject: Re: EHCI driver References: <4FC491EC.8010808@weinigel.se> <1338331586.6016.156.camel@king.jenpiliny.cz> <4FC5E820.1060505@weinigel.se> <4FC60E90.2030703@weinigel.se> In-Reply-To: <4FC60E90.2030703@weinigel.se> X-Enigmail-Version: 1.4.1 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="------------enig0F6F606A9BF859DDD85F6645" X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.85.215.169 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: Thu, 31 May 2012 12:08:45 -0000 This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig0F6F606A9BF859DDD85F6645 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Thanks, I've committed the patch with just style changes. On 30.05.2012 14:12, Christer Weinigel wrote: > On 2012-05-30 11:28, Christer Weinigel wrote: >=20 >> [talk about EHCI problems with low speed split transactions] >=20 >> Now I just have to fix up the queue management so that it properly kee= ps >> track of QHs for two queues instead of one. And figure out if there ar= e >> any other problems. >=20 >=20 > So, how about something like this? >=20 > Fix low-speed USB split transactions on EHCI >=20 > 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. >=20 > 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. >=20 > 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. >=20 > 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. >=20 > 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. >=20 > 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. >=20 > BTW, there is one really strange issue when I have multiple keyboards=20 > For example if I have three keyboards, with devices addresses 2, 3, and= =20 > 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=20 > alternate code selected by #if in grub_ehci_cancel_transfer makes thing= s > work, but I have no idea why which is a bit frightening. >=20 > 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. >=20 > 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. >=20 > /Christer >=20 > 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 =3D (3 << 30), > GRUB_EHCI_MULT_RESERVED =3D (0 << 30), > - GRUB_EHCI_MULT_ONE =3D (0 << 30), > - GRUB_EHCI_MULT_TWO =3D (0 << 30), > - GRUB_EHCI_MULT_THREE =3D (0 << 30), > + GRUB_EHCI_MULT_ONE =3D (1 << 30), > + GRUB_EHCI_MULT_TWO =3D (2 << 30), > + GRUB_EHCI_MULT_THREE =3D (3 << 30), > GRUB_EHCI_DEVPORT_MASK =3D (0x7f << 23), > - GRUB_EHCI_HUBADDR_MASK =3D (0x7f << 16) > + GRUB_EHCI_HUBADDR_MASK =3D (0x7f << 16), > + GRUB_EHCI_CMASK_MASK =3D (0xff << 8), > + GRUB_EHCI_SMASK_MASK =3D (0xff << 0), > }; > =20 > enum > { > GRUB_EHCI_MULT_OFF =3D 30, > GRUB_EHCI_DEVPORT_OFF =3D 23, > - GRUB_EHCI_HUBADDR_OFF =3D 16 > + GRUB_EHCI_HUBADDR_OFF =3D 16, > + GRUB_EHCI_CMASK_OFF =3D 8, > + GRUB_EHCI_SMASK_OFF =3D 0, > }; > =20 > #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)); > =20 > /* 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_tr= ansfer_t transfer) > & GRUB_EHCI_DEVPORT_MASK; > ep_cap |=3D (transfer->dev->hubaddr << GRUB_EHCI_HUBADDR_OFF) > & GRUB_EHCI_HUBADDR_MASK; > + if (transfer->dev->speed =3D=3D GRUB_USB_SPEED_LOW && > + transfer->type !=3D GRUB_USB_TRANSACTION_TYPE_CONTROL) > + { > + ep_cap |=3D (1<<0) << GRUB_EHCI_SMASK_OFF; > + ep_cap |=3D (7<<2) << GRUB_EHCI_CMASK_OFF; > + } > qh->ep_cap =3D grub_cpu_to_le32 (ep_cap); > =20 > grub_dprintf ("ehci", "setup_qh: qh=3D%08x, not changed: qh_hptr=3D%= 08x\n", > @@ -949,6 +960,7 @@ grub_ehci_find_qh (struct grub_ehci *e, grub_usb_tr= ansfer_t transfer) > grub_uint32_t target, mask; > int i; > grub_ehci_qh_t qh =3D e->qh_virt; > + grub_ehci_qh_t head; > =20 > /* Prepare part of EP Characteristic to find existing QH */ > target =3D ((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 =3D 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 =3D 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 =3D=3D GRUB_USB_SPEED_LOW && > + transfer->type !=3D GRUB_USB_TRANSACTION_TYPE_CONTROL) > + head =3D &qh[0]; > + else > + head =3D &qh[1]; > + > + /* Linking - this new (last) QH will copy the QH from the head QH */= > + qh[i].qh_hptr =3D head->qh_hptr; > + /* Linking - the head QH will point to this new QH */ > + head->qh_hptr =3D grub_cpu_to_le32 (GRUB_EHCI_HPTR_TYPE_QH > + | grub_dma_virt2phys (&qh[i], > + e->qh_chunk)= ); > =20 > return &qh[i]; > } > @@ -1221,7 +1240,7 @@ grub_ehci_setup_transfer (grub_usb_controller_t d= ev, > /* 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) =3D=3D 0) > + & (GRUB_EHCI_ST_AS_STATUS | GRUB_EHCI_ST_PS_STATUS)) =3D=3D 0) > /* XXX: Fix it: Currently we don't do anything to restart EHCI */ > return GRUB_USB_ERR_INTERNAL; > =20 > @@ -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 d= ev, > & GRUB_EHCI_ST_HC_HALTED) !=3D 0) > return grub_ehci_parse_notrun (dev, transfer, actual); > if ((grub_ehci_oper_read32 (e, GRUB_EHCI_STATUS) > - & GRUB_EHCI_ST_AS_STATUS) =3D=3D 0) > + & (GRUB_EHCI_ST_AS_STATUS | GRUB_EHCI_ST_PS_STATUS)) =3D=3D 0) > return grub_ehci_parse_notrun (dev, transfer, actual); > =20 > token =3D 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; > =20 > /* QH can be active and should be de-activated and halted */ > =20 > @@ -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) !=3D 0) || > ((grub_ehci_oper_read32 (e, GRUB_EHCI_STATUS) > - & GRUB_EHCI_ST_AS_STATUS) =3D=3D 0)) > + & (GRUB_EHCI_ST_AS_STATUS | GRUB_EHCI_ST_PS_STATUS)) =3D=3D 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, > =20 > /* 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 =3D ((int) (e->qh_virt - cdata->qh_virt)) / sizeof (struct grub_eh= ci_qh); > + /* Find index of previous QH */ > + qh_phys =3D grub_dma_virt2phys(cdata->qh_virt, e->qh_chunk); > + for (i =3D 0; i < GRUB_EHCI_N_QH; i++) > + { > + if ((e->qh_virt[i].qh_hptr & GRUB_EHCI_QHTDPTR_MASK) =3D=3D qh_p= hys) > + break; > + } > + if (i =3D=3D 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 =3D 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 =3D grub_get_time_ms () + 2; > - while (((grub_ehci_oper_read32 (e, GRUB_EHCI_STATUS) > - & GRUB_EHCI_ST_AS_ADVANCE) =3D=3D 0) > - && (grub_get_time_ms () < maxtime)); > + e->qh_virt[i].qh_hptr =3D 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. */ > =20 > - /* 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_CO= MMAND)); > + /* Ensure command is written */ > + grub_ehci_oper_read32 (e, GRUB_EHCI_COMMAND); > + /* Wait answer with timeout */ > + maxtime =3D grub_get_time_ms () + 2; > + while (((grub_ehci_oper_read32 (e, GRUB_EHCI_STATUS) > + & GRUB_EHCI_ST_AS_ADVANCE) =3D=3D 0) > + && (grub_get_time_ms () < maxtime)); > =20 > - /* 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 mo= st > + * 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_CO= MMAND)); > + grub_ehci_oper_write32 (e, GRUB_EHCI_STATUS, > + GRUB_EHCI_ST_AS_ADVANCE > + | grub_ehci_oper_read32 (e, GRUB_EHCI_ST= ATUS)); > + /* Ensure command is written */ > + grub_ehci_oper_read32 (e, GRUB_EHCI_STATUS); > + } > =20 > /* 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); > =20 > + /* 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 =3D > + e->qh_virt[i].qh_hptr =3D > grub_cpu_to_le32 (grub_dma_virt2phys > (cdata->qh_virt, e->qh_chunk)); > +#else > + /* Free the QH */ > + cdata->qh_virt->ep_char =3D 0; > + cdata->qh_virt->qh_hptr =3D > + grub_cpu_to_le32 ((grub_dma_virt2phys (cdata->qh_virt, > + e->qh_chunk) & > + GRUB_EHCI_POINTER_MASK) | GRUB_EHCI_HPTR_TYPE_Q= H); > +#endif > + > grub_free (cdata); > =20 > 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)); > =20 > /* Now should be possible to power-up and enumerate ports etc. *= / >=20 > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel >=20 --=20 Regards Vladimir '=CF=86-coder/phcoder' Serbinenko --------------enig0F6F606A9BF859DDD85F6645 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iF4EAREKAAYFAk/HXzoACgkQNak7dOguQgljeAD7BGXef8gdX/M7tKsS6/UFojVU a4wfk/iPGNLsZoXrEG4A/3vKo5uv0VKXrlfSABG7wdwMQcDGAciKrcAyMzasAHjN =8fBY -----END PGP SIGNATURE----- --------------enig0F6F606A9BF859DDD85F6645--