Hi, John Youn <John.Youn@xxxxxxxxxxxx> writes: >>>>>>>> - DWC_USB3_NUM indicates the number of Device mode single directional >>>>>>>> endpoints, including OUT and IN endpoint 0. >>>>>>>> >>>>>>>> - DWC_USB3_NUM_IN_EPS indicates the maximum number of Device mode IN >>>>>>>> endpoints active at any time, including control endpoint 0. >>>>>>>> >>>>>>>> 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 >>>>>>>> >>>>>>>> This patch works around this case by detecting when DWC_USB3_NUM_EPS is >>>>>>>> equal to DWC3_USB3_NUM_IN_EPS and over-rides the calculation of the number >>>>>>> >>>>>>> What if NUM_IN_EPS=7 and NUM_EPS=8? You will still have a problem. >>>>>>> >>>>>>> It's possible to fix this for the general case rather than for this >>>>>>> specific case. >>>>>> >>>>>> What is the reason for computing NUM_OUT_EPS in the first place? >>>>>> Isn't it true that any endpoint can be used as an OUT endpoint? >>>>>> >>>>>> So the real restrictions on a configuration are: >>>>>> >>>>>> number of IN endpoints <= NUM_IN_EPS, and >>>>>> >>>>>> number of IN endpoints + number of OUT endpoints <= NUM_EPS, >>>>>> >>>>>> where ep0 must be counted twice, as both an IN and an OUT endpoint. >>>>>> The value of NUM_OUT_EPS isn't used and shouldn't matter. >>>>>> >>>>> >>>>> Yes that's correct. The general fix should take all that into account >>>>> so that any combination of NUM_EPS and NUM_IN_EPS will work fine. >>>>> >>>>> However it must also account for FPGA configs where each HW endpoint >>>>> has a fixed number/direction, which the current code is compatible >>>>> with. >>>> >>>> I'm not scanning something here.. >>>> >>>> If FPGA configurations can have fixed endpoint directions then you could >>>> conceivably have a control IN/OUT pair plus a number a fixed >>>> configuration of all IN or all OUT endpoints, even checking >>>> GHWPARAMS6.EN_FPGA wouldn't let you know which direction those endpoints >>>> had been configured to. >>>> >>>> If I understand you correctly an FPGA could have pretty much any bit-map >>>> of fixed endpoint directions - so just calculating the number of IN/OUT >>>> endpoints based on some combination of DWC_USB3_NUM_IN_EPS and >>>> DWC_USB3_NUM_EPS wouldn't inform you of the fixed direction of an >>>> individual endpoint ... You'd need a descriptor specifying the fixed >>>> direction of each endpoint right ? >>>> >>> >>> Hi Bryan, >>> >>> The EP num and direction are fixed in a predefined way. >> >> Fair enough. >> >> See >>> DWC_USB3_EN_LOG_PHYS_EP_SUPT in the databook and my earlier reply to >>> Felipe. >> >> Thanks, I'll take a look tomorrow when I have the databook in front of me. >> >>> Unfortunately GHWPARAMS6.EN_FPGA doesn't directly tell you that >>> DWC_USB3_EN_LOG_PHYS_EP_SUPT is disabled. We just have to make that >>> assumption. Or add a quirk. >> >> So, if I'm understanding the conclusion. >> >> 1. We'll go ahead and make a change to just grab the # of endpoints >> I'll send out that patch once its tested. >> 2. For 4.12 Filipe will do something for the # of IN endpoints > > Up to here, it should be fine, and it will fix your case. It will work > with all cores but in a more limited fashion by not allowing endpoints > to be truly bidirectional. > >> 3. A quirk may eventually be required for DWC_USB3_EN_LOG_PHYS_EP_SUPT >> but lets no do anything on that unless someone actually complains. > > I think it should be that if someone complains about the above > limitations: > > 3. Rework endpoint code to remove the limitations and work correctly > for any direction. This should be the default behavior. > 4. Then based on EN_FPGA and/or a quirk, revert to previous behavior. Right, I have an idea about how to implement that, but it doesn't make sense to do so until we actually have users for this. It'll be a rather extensive change to the driver which I want to avoid unless people really, really need this change ;-) -- balbi
Attachment:
signature.asc
Description: PGP signature
