Hi,
Peter Chen <hzpeterc...@gmail.com> writes:
>> Peter Chen <hzpeterc...@gmail.com> 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
>
> No, even in one uFrame, there are at most ~10 packets for bulk at USB2.13 > At least, you can throttle interrupt within SoF, it is useful for > high throughout use case. And that's still a bug for Networking drivers, that's what Dave Miller is saying. >> > 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 >> > > So, how about add another parameter to support throttling interrupt > separately. Current parameter 'mult' combined user request number > and throttle interrupt together. sorry, but no. -- balbi
signature.asc
Description: PGP signature
