Hi,
Felipe Balbi wrote:
Hi,
Thinh Nguyen Thinh.Nguyen@synopsys.com writes:
@@ -3863,7 +3868,7 @@ int dwc3_gadget_init(struct dwc3 *dwc) * sure we're starting from a well known location. */
- ret = dwc3_gadget_init_endpoints(dwc, dwc->num_eps);
- ret = dwc3_gadget_init_endpoints(dwc); if (ret) goto err4;
heh, looking at original commit, we used to have num_in_eps and num_out_eps. In fact, this commit will reintroduce another problem that Bryan tried to solve. num_eps - num_in_eps is not necessarily num_out_eps.
From my understanding, that's not what Bryan's saying. Here's the snippet of the commit message:
" It's possible to configure RTL such that DWC_USB3_NUM_EPS is equal to DWC_USB3_NUM_IN_EPS.
dwc3-core calculates the number of OUT endpoints as DWC_USB3_NUM minus DWC_USB3_NUM_IN_EPS. If RTL has been configured with DWC_USB3_NUM_IN_EPS equal to DWC_USB3_NUM then dwc3-core will calculate the number of OUT endpoints as zero.
For example a from dwc3_core_num_eps() shows: [ 1.565000] /usb0@f01d0000: found 8 IN and 0 OUT endpoints "
He just stated that you can configure to have num_eps equals to num_in_eps. Basically it means there's no OUT physical endpoint. Not
no, that's not what it means. I don't have access to DWC3 documentation anymore, but from what I remember every physical endpoint _can_ be configured as bidirectional. In other words, DWC3_USB3_NUM_EPS == DWC3_USB3_NUM_IN_EPS could mean that every endpoint in the system is bidirectional.
sure why you would ever want to do that because that will prevent any device from working. The reason we have DWC_USB3x_NUM_IN_EPS and DWC_USB3x_NUM_EPS exposed in the global HW param so that the driver know how many endpoints are available for each direction. If for some reason this mechanism fails, there's something fundamentally wrong in the HW configuration. We have not seen this problem, and I don't think Bryan did from his commit statement either.
Please confirm this internally. That was my original assumption too, until Bryan pointed me to a particular section of the specification. Unfortunately it's far too long ago and I can't even verify documentation :-)
How have you verified this patch? Did you read Bryan's commit log? This is likely to reintroduce the problem raised by Bryan.
We verified with our FPGA HAPS with various number of endpoints. No issue is seen.
That's cool. Could you please make sure our understanding of this is sound and won't interfere with any designs? If we modify this part of the code again, I'd like to see a clear reference to a specific section of the databook detailing the expected behavior :-)
cheers
Hm... I didn't consider bidirection endpoint other than control endpoint.
DWC3_USB3x_NUM_EPS specifies the number of device mode for single directional endpoints. A bidirectional endpoint needs 2 single directional endpoints, 1 IN and 1 OUT. So, if your setup uses 3 bidirection endpoints and only those, DWC3_USB3x_NUM_EPS should be 6. DWC3_USB3x_NUM_IN_EPS specifies the maximum number of IN endpoint active at any time.
However, I will have to double check and confirm internally regarding how to determine many endpoint would be available if bidirection endpoints come into play.
Thanks for pointing this out. Will get back on this.
Thinh