Shantur Rathore wrote: > In RK3399 as per documentation ( > https://urldefense.com/v3/__https://usermanual.wiki/Document/RockchipDeveloperGuidelinux44USB.31610806__;!!A4F2R9G_pg!JqYr6U87SL3rYylirF6W2vwNzC0Ft8YiZwTlMTwWl7bpaHGZuh-JMfOvUaG781GKZIBd$ > ), there are 7 Input Endpoints and 6 Output endpoints, in total 13 > endpoints. > > Currently dwc3/gadget.c driver uses the number of endpoints > available and starts setting them up with even endpoints as output > endpoints and odd numbered as even endpoints. This leads to 7 Output > endpoints and 6 input endpoints for RK3399. > > If you try to create a composite gadget which uses all the input > endpoints, one can see the issue. You just need to create functions to > use up the last input ep and it would fail to create. No need to > connect it to the host. > This was confirmed when running a rockchip-linux bsp image. > > [root@rockpro rock]# ls /sys/kernel/debug/usb/fe800000.usb/ > ep0in ep0out ep1in ep1out ep2in ep2out ep3in ep3out ep4in > ep4out ep5in ep5out ep6in link_state lsp_dump mode regdump > testmode > > Currently in linux mainline it is > > [root@rockpro rock]# ls /sys/kernel/debug/usb/fe800000.usb/ > ep0in ep0out ep1in ep1out ep2in ep2out ep3in ep3out ep4in > ep4out ep5in ep5out ep6out link_state lsp_dump mode regdump > testmode > > ep6 being out instead of in as per the hardware spec. > > Upon investigation of rockchip bsp kernel, > https://urldefense.com/v3/__https://github.com/rockchip-linux/kernel/__;!!A4F2R9G_pg!JqYr6U87SL3rYylirF6W2vwNzC0Ft8YiZwTlMTwWl7bpaHGZuh-JMfOvUaG788DHJSUE$ > > The issue was clear, currently, dwc3/gadget driver doesn't take > DWC3_NUM_IN_EPS into consideration while enumerating them. > > The patch below fixes the issue and ep6 is correctly enumerated as input. No Signed-of-by? > --- > drivers/usb/dwc3/core.c | 1 + > drivers/usb/dwc3/core.h | 1 + > drivers/usb/dwc3/gadget.c | 40 ++++++++++++++++++++++++--------------- > 3 files changed, 27 insertions(+), 15 deletions(-) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 01866dcb953b..279c9a97cb8c 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -555,6 +555,7 @@ static void dwc3_core_num_eps(struct dwc3 *dwc) > struct dwc3_hwparams *parms = &dwc->hwparams; > > dwc->num_eps = DWC3_NUM_EPS(parms); > + dwc->num_in_eps = DWC3_NUM_IN_EPS(parms); > } > > static void dwc3_cache_hwparams(struct dwc3 *dwc) > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 5612bfdf37da..89a0998c618c 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -1227,6 +1227,7 @@ struct dwc3 { > u8 speed; > > u8 num_eps; > + u8 num_in_eps; > > struct dwc3_hwparams hwparams; > struct debugfs_regset32 *regset; > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 804b50548163..d9d19dc0a29f 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -693,9 +693,11 @@ void dwc3_gadget_clear_tx_fifos(struct dwc3 *dwc) > > dwc->last_fifo_depth = fifo_depth; > /* Clear existing TXFIFO for all IN eps except ep0 */ > - for (num = 3; num < min_t(int, dwc->num_eps, DWC3_ENDPOINTS_NUM); > - num += 2) { > + for (num = 3; num < DWC3_ENDPOINTS_NUM; num += 2) { > dep = dwc->eps[num]; > + > + if(!dep) > + continue; > /* Don't change TXFRAMNUM on usb31 version */ > size = DWC3_IP_IS(DWC3) ? 0 : > dwc3_readl(dwc->regs, DWC3_GTXFIFOSIZ(num >> 1)) & > @@ -2257,7 +2259,7 @@ static void dwc3_stop_active_transfers(struct dwc3 *dwc) > { > u32 epnum; > > - for (epnum = 2; epnum < dwc->num_eps; epnum++) { > + for (epnum = 2; epnum < DWC3_ENDPOINTS_NUM; epnum++) { > struct dwc3_ep *dep; > > dep = dwc->eps[epnum]; > @@ -2960,10 +2962,9 @@ static int dwc3_gadget_init_out_endpoint(struct dwc3_ep *dep) > return dwc3_alloc_trb_pool(dep); > } > > -static int dwc3_gadget_init_endpoint(struct dwc3 *dwc, u8 epnum) > +static int dwc3_gadget_init_endpoint(struct dwc3 *dwc, u8 epnum, bool direction) > { > struct dwc3_ep *dep; > - bool direction = epnum & 1; > int ret; > u8 num = epnum >> 1; > > @@ -3011,21 +3012,30 @@ static int dwc3_gadget_init_endpoint(struct dwc3 *dwc, u8 epnum) > return 0; > } > > -static int dwc3_gadget_init_endpoints(struct dwc3 *dwc, u8 total) > +static int dwc3_gadget_init_endpoints(struct dwc3 *dwc, u8 total, u8 num_in_eps) > { > - u8 epnum; > + u8 num; > + int ret; > > INIT_LIST_HEAD(&dwc->gadget->ep_list); > > - for (epnum = 0; epnum < total; epnum++) { > - int ret; > + /* init input endpoints as reported by hw */ > + for (num = 0; num < num_in_eps; num++) { > > - ret = dwc3_gadget_init_endpoint(dwc, epnum); > - if (ret) > - return ret; > - } > + ret = dwc3_gadget_init_endpoint(dwc, (num << 1) + 1, 1); > + if (ret) > + return ret; > + } > > - return 0; > + /* init rest endpoints as output endpoints */ > + for (num = 0; num < total - num_in_eps; num++) { > + > + ret = dwc3_gadget_init_endpoint(dwc, num << 1, 0); > + if (ret) > + return ret; > + } > + > + return ret; > } > * DWC3_NUM_EPS(parms) is the total number of endpoints configured in HW * DWC3_NUM_IN_EPS(parms) is the max number of IN endpoints that SW may configure The number of OUT endpoints does not mean DWC3_NUM_EPS(parms) - DWC3_NUM_IN_EPS(parms). As long as physical endpoint 0 and 1 are dedicated for control endpoint, other endpoints can be assigned as IN or OUT direction. So, you can have as many as DWC3_NUM_EPS(parms) - 1 number of OUT endpoints. Currently, dwc3 driver assumes that DWC3_NUM_IN_EPS(params) is at least half of DWC3_NUM_EPS(parms). If that's not the case, we may see problems. To cover most application setup, the driver tries to setup number of OUT = IN. For your case, is there an application that needs all DWC3_NUM_IN_EPS(params)? If you're going to make a change, please keep in mind of the info above to prevent regression. Thanks, Thinh
