On 11/6/2015 9:55 AM, Felipe Balbi wrote: > > Hi, > > "McCauley, Ben" <Ben.McCauley@xxxxxxxxxx> writes: >> Felipe, >> >> -----Original Message----- >> From: Felipe Balbi [mailto:balbi@xxxxxx] >> Sent: Friday, November 06, 2015 10:06 AM >> To: McCauley, Ben <Ben.McCauley@xxxxxxxxxx> >> Cc: linux-usb@xxxxxxxxxxxxxxx; Schroeder, Jay <Jay.Schroeder@xxxxxxxxxx> >> Subject: Re: [PATCH] USB: DCW3: GADGET: Set Correct Max Speed >>> >>> >>> Hi, >>> >>> "McCauley, Ben" <Ben.McCauley@xxxxxxxxxx> writes: >>>> The max speed was always being set to USB_SUPER_SPEED even when the >>>> phy was only capable of HIGH_SPEED. This >>> >>> this statement is untrue >>> >>>> uses the value set for the phy to set the max for the gadget. It >>>> resolves an issue with Window PCs where they report that using a >>>> USB3.0 port would result in better performance even when connecting >>>> through a 3.0 port. >>> >>> man, who gave you this kernel ? Which kernel version are you using ? >>> Have you even tested mainline ? >> >> This Kernel is from http://omapzoom.org/?p=kernel/omap.git;a=shortlog;h=refs/heads/p-ti-linux-3.14.y-common >> at branch "development/omapzoom/glsdk-7.-2.00.02". >> I have not tested it on main line. > > seems like you should be talking to your FAE or using TI's e2e forum for > this. > >>>> Signed-off-by: McCauley, Ben <ben.mccauley@xxxxxxxxxx> >>>> --- >>>> >>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>>> index 1eaf31c..0a388e3 100644 >>>> --- a/drivers/usb/dwc3/gadget.c >>>> +++ b/drivers/usb/dwc3/gadget.c >>>> @@ -2899,7 +2899,7 @@ >>> >>> which kernel are you using ? This file is 2868 lines, so you're patching outside >>> the file. >>> >>>> } >>>> >>>> dwc->gadget.ops = &dwc3_gadget_ops; >>>> - dwc->gadget.max_speed = USB_SPEED_SUPER; >>>> + dwc->gadget.max_speed = dwc->maximum_speed; >>> >>> this is corrected at dwc3_gadget_start(): > > and I see that in that kernel, this is corrected at > dwc3_gadget_restart() instead. > >>> /** >>> * WORKAROUND: DWC3 revision < 2.20a have an issue >>> * which would cause metastability state on Run/Stop >>> * bit if we try to force the IP to USB2-only mode. >>> * >>> * Because of that, we cannot configure the IP to any >>> * speed other than the SuperSpeed >>> * >>> * Refers to: >>> * >>> * STAR#9000525659: Clock Domain Crossing on DCTL in >>> * USB 2.0 Mode >>> */ >>> if (dwc->revision < DWC3_REVISION_220A) { >>> reg |= DWC3_DCFG_SUPERSPEED; >>> } else { >>> switch (dwc->maximum_speed) { >>> case USB_SPEED_LOW: >>> reg |= DWC3_DSTS_LOWSPEED; >>> break; >>> case USB_SPEED_FULL: >>> reg |= DWC3_DSTS_FULLSPEED1; >>> break; >>> case USB_SPEED_HIGH: >>> reg |= DWC3_DSTS_HIGHSPEED; >>> break; >>> case USB_SPEED_SUPER: /* FALLTHROUGH */ >>> case USB_SPEED_UNKNOWN: /* FALTHROUGH */ >>> default: >>> reg |= DWC3_DSTS_SUPERSPEED; >>> } >>> } >>> >> >> I am on 3.14 LTS though even on 4.3 I do not see dwc->gadget.max_speed >> being updated to reflect the actual max speed as set in >> drivers/usb/dwc3/core.c [dwc3_probe()]. dwc->gadget.max_speed is used >> in a number of locations to determine what response is sent to the >> host. >> >> dwc3_gadget_start() only appears to set the max speed to the register >> and not update the gadget. > > but the IP _is_ super-speed capable. Max speed should only be used to > determine if $this gadget can run with $this controller; speed, instead, > is used for all sorts of different things and _that_ is updated on > Connection Done IRQ. > > Can you point one place where you think it's wrong ? And, please, refer > to mainline code. We can't support a v3.14 kernel which might contain a > ton of additions which are not in mainline (for example, mainline > doesn't have a dwc3_gadget_restart() function). > Hi Felipe, There are 3 related speed settings which may be confusing the matter. dwc->maximum_speed A user-supplied parameter that determines how to program the DCFG.speed. This is the code you're referring to above. gadget->speed The current connected speed of the gadget. Used in various ways by the upper layer. gadget->max_speed Reports the maximum speed supported by the gadget. This is used by upper layer to determine which BOS descriptors to send and the settings within those descriptors. It's also used to set the bcdUSB appropriately. Ben is talking about setting the gadget->max_speed, which is hard-coded in dwc3 to USB_SPEED_SUPER. For this core, the maximum supported speed may not actually be super speed. Many customers choose to use the 3.0 core in 2.0-only mode with a HS phy. And with the 3.1 core, it my be integrated in a system that is HS-only, SS (gen1), or SSP (gen2) capable. So the gadget->max_speed needs to be set correctly based on some user-defined or detected value. We have a similar change in our local tree for 3.1, except we set it based on GHWPARAMS3.SSPHY_INTERFACE. But I think it makes sense to set it from dwc->maximum_speed as well, for testing or to override it. Regards, John -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
