| Message ID | 20200818161949.4083551-1-lorenzo@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4eea21dc67b0c6ba15ae41b1defa113a680a858e |
| Headers | show |
| Series | [v2] usb: gadget: u_ether: enable qmult on SuperSpeed Plus as well | expand |
On Tue, Aug 18, 2020 at 9:19 AM Lorenzo Colitti <lorenzo@google.com> wrote: > > The u_ether driver has a qmult setting that multiplies the > transmit queue length (which by default is 2). > > The intent is that it should be enabled at high/super speed, but > because the code does not explicitly check for USB_SUPER_PLUS, > it is disabled at that speed. > > Fix this by ensuring that the queue multiplier is enabled for any > wired link at high speed or above. Using >= for USB_SPEED_* > constants seems correct because it is what the gadget_is_xxxspeed > functions do. > > The queue multiplier substantially helps performance at higher > speeds. On a direct SuperSpeed Plus link to a Linux laptop, > iperf3 single TCP stream: > > Before (qmult=1): 1.3 Gbps > After (qmult=5): 3.2 Gbps > > Fixes: 04617db7aa68 ("usb: gadget: add SS descriptors to Ethernet gadget") > Signed-off-by: Lorenzo Colitti <lorenzo@google.com> > --- > drivers/usb/gadget/function/u_ether.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c > index c3cc6bd14e..31ea76adcc 100644 > --- a/drivers/usb/gadget/function/u_ether.c > +++ b/drivers/usb/gadget/function/u_ether.c > @@ -93,7 +93,7 @@ struct eth_dev { > static inline int qlen(struct usb_gadget *gadget, unsigned qmult) > { > if (gadget_is_dualspeed(gadget) && (gadget->speed == USB_SPEED_HIGH || > - gadget->speed == USB_SPEED_SUPER)) > + gadget->speed >= USB_SPEED_SUPER)) > return qmult * DEFAULT_QLEN; > else > return DEFAULT_QLEN; > -- > 2.28.0.220.ged08abb693-goog > Reviewed-by: Maciej Żenczykowski <maze@google.com>
On Wed, Aug 19, 2020 at 6:07 AM Maciej Żenczykowski <zenczykowski@gmail.com> wrote: > > > The u_ether driver has a qmult setting that multiplies the > > transmit queue length (which by default is 2). > > Reviewed-by: Maciej Żenczykowski <maze@google.com> Felipe, Greg: Is there anything that I should be doing about this patch, or should I just expect it to be merged at some point? If you were thinking of looking at it again when the comments on the NCM SuperSpeed patches are addressed, I just posted a new version of those (v3), with a cover letter to explain what changed. Thanks, Lorenzo
diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c index c3cc6bd14e..31ea76adcc 100644 --- a/drivers/usb/gadget/function/u_ether.c +++ b/drivers/usb/gadget/function/u_ether.c @@ -93,7 +93,7 @@ struct eth_dev { static inline int qlen(struct usb_gadget *gadget, unsigned qmult) { if (gadget_is_dualspeed(gadget) && (gadget->speed == USB_SPEED_HIGH || - gadget->speed == USB_SPEED_SUPER)) + gadget->speed >= USB_SPEED_SUPER)) return qmult * DEFAULT_QLEN; else return DEFAULT_QLEN;
The u_ether driver has a qmult setting that multiplies the transmit queue length (which by default is 2). The intent is that it should be enabled at high/super speed, but because the code does not explicitly check for USB_SUPER_PLUS, it is disabled at that speed. Fix this by ensuring that the queue multiplier is enabled for any wired link at high speed or above. Using >= for USB_SPEED_* constants seems correct because it is what the gadget_is_xxxspeed functions do. The queue multiplier substantially helps performance at higher speeds. On a direct SuperSpeed Plus link to a Linux laptop, iperf3 single TCP stream: Before (qmult=1): 1.3 Gbps After (qmult=5): 3.2 Gbps Fixes: 04617db7aa68 ("usb: gadget: add SS descriptors to Ethernet gadget") Signed-off-by: Lorenzo Colitti <lorenzo@google.com> --- drivers/usb/gadget/function/u_ether.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)