Hi,
Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes:
> Typically FPGA devices are configured with CoreConsultant parameter
> DWC_USB3x_EN_LOG_PHYS_EP_SUPT=0 to reduce gate count and improve timing.
> This means that the number of INs equals to OUTs endpoints. But
> typically non-FPGA devices enable this CoreConsultant parameter to
> support flexible endpoint mapping and potentially may have unequal
> number of INs to OUTs physical endpoints.
>
> The driver must check how many physical endpoints are available for each
> direction and initialize them properly.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 47d3946ea220 ("usb: dwc3: refactor gadget endpoint count calculation")
> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx>
> ---
> drivers/usb/dwc3/core.c | 1 +
> drivers/usb/dwc3/core.h | 2 ++
> drivers/usb/dwc3/gadget.c | 19 ++++++++++++-------
> 3 files changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 841daec70b6e..1084aa8623c2 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -529,6 +529,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 1b241f937d8f..1295dac019f9 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -990,6 +990,7 @@ struct dwc3_scratchpad_array {
> * @u1sel: parameter from Set SEL request.
> * @u1pel: parameter from Set SEL request.
> * @num_eps: number of endpoints
> + * @num_in_eps: number of IN endpoints
> * @ep0_next_event: hold the next expected event
> * @ep0state: state of endpoint zero
> * @link_state: link state
> @@ -1193,6 +1194,7 @@ struct dwc3 {
> u8 speed;
>
> u8 num_eps;
> + u8 num_in_eps;
>
> struct dwc3_hwparams hwparams;
> struct dentry *root;
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 25f654b79e48..8a38ee10c00b 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2025,7 +2025,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];
> @@ -2628,16 +2628,21 @@ 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 epnum;
> + u8 i;
> + int ret;
>
> INIT_LIST_HEAD(&dwc->gadget->ep_list);
>
> - for (epnum = 0; epnum < total; epnum++) {
> - int ret;
> + for (i = 0; i < dwc->num_in_eps; i++) {
> + ret = dwc3_gadget_init_endpoint(dwc, i * 2 + 1);
> + if (ret)
> + return ret;
> + }
>
> - ret = dwc3_gadget_init_endpoint(dwc, epnum);
> + for (i = 0; i < dwc->num_eps - dwc->num_in_eps; i++) {
> + ret = dwc3_gadget_init_endpoint(dwc, i * 2);
> if (ret)
> return ret;
> }
> @@ -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.
How have you verified this patch? Did you read Bryan's commit log? This
is likely to reintroduce the problem raised by Bryan.
--
balbi
