Hi, Peter Chen <[email protected]> writes: >> >> diff --git a/drivers/usb/gadget/function/u_ether.c >> >> b/drivers/usb/gadget/function/u_ether.c >> >> index f4a640216913..119a2e5848e8 100644 >> >> --- a/drivers/usb/gadget/function/u_ether.c >> >> +++ b/drivers/usb/gadget/function/u_ether.c >> >> @@ -589,14 +589,6 @@ static netdev_tx_t eth_start_xmit(struct sk_buff >> >> *skb, >> >> >> >> req->length = length; >> >> >> >> - /* throttle high/super speed IRQ rate back slightly */ >> >> - if (gadget_is_dualspeed(dev->gadget)) >> >> - req->no_interrupt = (((dev->gadget->speed == USB_SPEED_HIGH || >> >> - dev->gadget->speed == USB_SPEED_SUPER)) >> >> && >> >> - !list_empty(&dev->tx_reqs)) >> >> - ? ((atomic_read(&dev->tx_qlen) % dev->qmult) != 0) >> >> - : 0; >> >> - >> >> retval = usb_ep_queue(in, req, GFP_ATOMIC); >> >> switch (retval) { >> >> default: >> >> -- >> > >> > Felipe, it may increase cpu utilization since more interrupts will be >> > there, >> > it may affect the SoC which has lower cpu frequency. This code existed >> > many years, why this problem has only reported at dwc3 recently? >> >> No idea, but at least for networking gadgets we shouldn't throttle. This >> has been a bug since the beginning. Read Dave Miller's explanation at >> [1] >> >> moreover, dwc3 seems to be the only one actually throttling IRQ. Here's >> a rundown of a few of the UDCs: >> >> - chipidea: uses TD_IOC conditionally, but always sets TD_TERMINATE >> >> lastnode->ptr->next = cpu_to_le32(TD_TERMINATE); >> if (!hwreq->req.no_interrupt) >> lastnode->ptr->token |= cpu_to_le32(TD_IOC); >> >> I'm guessing TD_TERMINATE works similar to dwc3's LST bit. If >> it's set, it will force an interrupt. > > No, TD_TERMINATE just stands for it is the last TD, and this pointer will > be updated when the new request is added. The interrupt is only triggered > by IOC (Interrupt On Complete) bit at TD. > > I am not sure if dwc3 supports ITC (Interrupt Threshold Control) > software control, it is an EHCI compliant register entry, and > the device mode is supported for chipidea too. It is a timeout > mechanism from controller side for pending requests. > > The interrupt will be triggered either the request has completed for TD > which IOC bit is set or the ITC is fired (125us currently) and the > request has completed, so the problem David described should not exist, > at least for chipidea.
In other words, you don't *really* throttle interrupt as they'll fire after the micro-frame expires :-p > If DWC3 has similar ITC bits, would you try to tune it? The default ITC > value for chipidea is not enough, and we tuned it before. there's no such thing in dwc3 >> - musb: no_interrupt only used for tracing >> >> - atmel_usba_udc: no_interrupt only used for tracing >> >> - mv_u3d_core: probably throttles interrupt, but probably exhibits same >> behavior. It's just that it hasn't been reported. >> >> - fsl_udc_core: probably throttles interrupt, but probably exhibits same >> behavior. It's just that it hasn't been reported. > > The above two uses chipidea IP core too. so why do we still have these drivers in tree? Might as well remove them already. -- balbi
signature.asc
Description: PGP signature
