> 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; > + } Could you use enums rather than hardcoding constants? > + /* 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]; Wouldn't it be easier to have a range reserved for interrupt transfer? It's not like if we were short on QHs. > > /* 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; > + } This makes it iterate through uncached memory which may be very slow. > + /* 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); Isn't there a better way than arbitrary sleep? This sleep is pretty large. > + /* Wait answer with timeout */ > + maxtime = grub_get_time_ms () + 2; 2 ms seems to be short as a maximum timeout. There is quite some number of buggy and slow hardware around. > + 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. */ > + Which should be reported as an error if it occurs. > > + /* 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)); You may link it into wrong list AFAICS (interrupt vs bulk). -- Regards Vladimir 'φ-coder/phcoder' Serbinenko