[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: EHCI driver
From: |
Vladimir 'φ-coder/phcoder' Serbinenko |
Subject: |
Re: EHCI driver |
Date: |
Thu, 31 May 2012 14:08:26 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:10.0.4) Gecko/20120510 Icedove/10.0.4 |
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:
>
>> [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. */
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
signature.asc
Description: OpenPGP digital signature