From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com ([134.134.136.24]:38177 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750801AbcKBLDX (ORCPT ); Wed, 2 Nov 2016 07:03:23 -0400 From: Felipe Balbi To: Peter Chen Cc: Linux USB , David Miller , Ville =?utf-8?B?U3lyasOkbMOk?= , stable@vger.kernel.org Subject: Re: [PATCH] usb: gadget: u_ether: remove interrupt throttling In-Reply-To: <20161102083632.GA22181@b29397-desktop> References: <20161101112959.19640-1-felipe.balbi@linux.intel.com> <20161102060202.GD28525@b29397-desktop> <87vaw6nwn3.fsf@linux.intel.com> <20161102083632.GA22181@b29397-desktop> Date: Wed, 02 Nov 2016 13:02:59 +0200 Message-ID: <87lgx2nnz0.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: stable-owner@vger.kernel.org List-ID: --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Peter Chen writes: >> >> diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadg= et/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, >> >>=20=20 >> >> req->length =3D length; >> >>=20=20 >> >> - /* throttle high/super speed IRQ rate back slightly */ >> >> - if (gadget_is_dualspeed(dev->gadget)) >> >> - req->no_interrupt =3D (((dev->gadget->speed =3D=3D USB_SPEED_HIGH = || >> >> - dev->gadget->speed =3D=3D USB_SPEED_SUPER)) && >> >> - !list_empty(&dev->tx_reqs)) >> >> - ? ((atomic_read(&dev->tx_qlen) % dev->qmult) !=3D 0) >> >> - : 0; >> >> - >> >> retval =3D usb_ep_queue(in, req, GFP_ATOMIC); >> >> switch (retval) { >> >> default: >> >> --=20 >> > >> > 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? >>=20 >> 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] >>=20 >> moreover, dwc3 seems to be the only one actually throttling IRQ. Here's >> a rundown of a few of the UDCs: >>=20 >> - chipidea: uses TD_IOC conditionally, but always sets TD_TERMINATE >>=20 >> lastnode->ptr->next =3D cpu_to_le32(TD_TERMINATE); >> if (!hwreq->req.no_interrupt) >> lastnode->ptr->token |=3D cpu_to_le32(TD_IOC); >>=20 >> 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 >>=20 >> - atmel_usba_udc: no_interrupt only used for tracing >>=20 >> - mv_u3d_core: probably throttles interrupt, but probably exhibits same >> behavior. It's just that it hasn't been reported. >>=20 >> - 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. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBCAAGBQJYGcfjAAoJEMy+uJnhGpkG1K0QAIp4YtqWLCfD9X7stK6qC69U mv4bXOqR0txkNOpMVTDpHbqFG9UHab8VElg6/tYxHqsXqc62NhUWL6DtDY/jqmQL ToSiympo5+Jx85jUBQPaV91pWRTzu4usqEcKhvoTBu+rcLO4Sr9K6z0HTTXeV3St vKQrKCk9NHK82YeI5/whevNIApFtw/zMv1n6ZUIhiwceEaP0gvO91v2IHeVv79eb YrhNVmt7cNR/qZ0cI9arzAPS1SemvHxjUmPqKQNEmlJBOCW+KukVnalrEDQ8TUlL 5RiEvoLmNxw2LK+Ck9VJKLtc0XmRN/loBI7jn7tndqUOo0vAi5vffW2rVD1DEfoW OOnnBFWDjnO/1l+Nh2MM50pWAwXvvQSfj2XgU/LC/+5sXTYtOlxLMxrFmvbFNtLP efnZq3Fr+R+e+ZrSKfDw+7F0AtO1yOkCh2lwY9yJYxYd8a/tEMrBMyYFG+obNvhG sKZR8/30dTEMUpT/YabAQnz5Butkrxw8Xg/6OLUgQarCuCh60uaheqVABgUJnhLj AXQWKiKVfmrZztNX7c9LBnXtn3XzEQCrIznh+sEhfHisG1nXKKZvUZzh/iAQfotj VXpkkrWrnp+U6yBWETmH+b8e2ntSZ3j8b7ivJSv5DYgIrKgwYAP9TQl6H4ffufI/ cqBZoSFXdTHt1LoNeBJA =xg1c -----END PGP SIGNATURE----- --=-=-=--