Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

usb: dwc3: gadget: Rewrite endpoint allocation flow

The driver dwc3 deviates from the programming guide in regard to
endpoint configuration. It does this command sequence:

DEPSTARTCFG -> DEPXFERCFG -> DEPCFG

Instead of the suggested flow:

DEPSTARTCFG -> DEPCFG -> DEPXFERCFG

The reasons for this deviation were as follow, quoted:

1) The databook says to do %DWC3_DEPCMD_DEPSTARTCFG for every
%USB_REQ_SET_CONFIGURATION and %USB_REQ_SET_INTERFACE
(8.1.5). This is incorrect in the scenario of multiple
interfaces.

2) The databook does not mention doing more
%DWC3_DEPCMD_DEPXFERCFG for new endpoint on alt setting
(8.1.6).

Regarding 1), DEPSTARTCFG resets the endpoints' resource and can be a
problem if used with SET_INTERFACE request of a multiple interface
configuration. But we can still satisfy the programming guide
requirement by assigning the endpoint resource as part of
usb_ep_enable(). We will only reset endpoint resources on controller
initialization and SET_CONFIGURATION request.

Regarding 2), the later versions of the programming guide were updated
to clarify this flow (see "Alternate Initialization on SetInterface
Request" of the programming guide). As long as the platform has enough
physical endpoints, we can assign resource to a new endpoint.

The order of the command sequence will not be a problem to most
platforms for the current implementation of the dwc3 driver. However,
this order is required in different scenarios (such as initialization
during controller's hibernation restore). Let's keep the flow consistent
and follow the programming guide.

Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Link: https://lore.kernel.org/r/c143583a5afb087deb8c3aa5eb227ee23515f272.1706754219.git.Thinh.Nguyen@synopsys.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Thinh Nguyen and committed by
Greg Kroah-Hartman
b311048c d4718eff

+41 -51
+1
drivers/usb/dwc3/core.h
··· 756 756 #define DWC3_EP_PENDING_CLEAR_STALL BIT(11) 757 757 #define DWC3_EP_TXFIFO_RESIZED BIT(12) 758 758 #define DWC3_EP_DELAY_STOP BIT(13) 759 + #define DWC3_EP_RESOURCE_ALLOCATED BIT(14) 759 760 760 761 /* This last one is specific to EP0 */ 761 762 #define DWC3_EP0_DIR_IN BIT(31)
+1
drivers/usb/dwc3/ep0.c
··· 646 646 return -EINVAL; 647 647 648 648 case USB_STATE_ADDRESS: 649 + dwc3_gadget_start_config(dwc, 2); 649 650 dwc3_gadget_clear_tx_fifos(dwc); 650 651 651 652 ret = dwc3_ep0_delegate_req(dwc, ctrl);
+38 -51
drivers/usb/dwc3/gadget.c
··· 519 519 static int dwc3_gadget_set_xfer_resource(struct dwc3_ep *dep) 520 520 { 521 521 struct dwc3_gadget_ep_cmd_params params; 522 + int ret; 523 + 524 + if (dep->flags & DWC3_EP_RESOURCE_ALLOCATED) 525 + return 0; 522 526 523 527 memset(&params, 0x00, sizeof(params)); 524 528 525 529 params.param0 = DWC3_DEPXFERCFG_NUM_XFER_RES(1); 526 530 527 - return dwc3_send_gadget_ep_cmd(dep, DWC3_DEPCMD_SETTRANSFRESOURCE, 531 + ret = dwc3_send_gadget_ep_cmd(dep, DWC3_DEPCMD_SETTRANSFRESOURCE, 528 532 &params); 533 + if (ret) 534 + return ret; 535 + 536 + dep->flags |= DWC3_EP_RESOURCE_ALLOCATED; 537 + return 0; 529 538 } 530 539 531 540 /** 532 - * dwc3_gadget_start_config - configure ep resources 533 - * @dep: endpoint that is being enabled 541 + * dwc3_gadget_start_config - reset endpoint resources 542 + * @dwc: pointer to the DWC3 context 543 + * @resource_index: DEPSTARTCFG.XferRscIdx value (must be 0 or 2) 534 544 * 535 - * Issue a %DWC3_DEPCMD_DEPSTARTCFG command to @dep. After the command's 536 - * completion, it will set Transfer Resource for all available endpoints. 545 + * Set resource_index=0 to reset all endpoints' resources allocation. Do this as 546 + * part of the power-on/soft-reset initialization. 537 547 * 538 - * The assignment of transfer resources cannot perfectly follow the data book 539 - * due to the fact that the controller driver does not have all knowledge of the 540 - * configuration in advance. It is given this information piecemeal by the 541 - * composite gadget framework after every SET_CONFIGURATION and 542 - * SET_INTERFACE. Trying to follow the databook programming model in this 543 - * scenario can cause errors. For two reasons: 544 - * 545 - * 1) The databook says to do %DWC3_DEPCMD_DEPSTARTCFG for every 546 - * %USB_REQ_SET_CONFIGURATION and %USB_REQ_SET_INTERFACE (8.1.5). This is 547 - * incorrect in the scenario of multiple interfaces. 548 - * 549 - * 2) The databook does not mention doing more %DWC3_DEPCMD_DEPXFERCFG for new 550 - * endpoint on alt setting (8.1.6). 551 - * 552 - * The following simplified method is used instead: 553 - * 554 - * All hardware endpoints can be assigned a transfer resource and this setting 555 - * will stay persistent until either a core reset or hibernation. So whenever we 556 - * do a %DWC3_DEPCMD_DEPSTARTCFG(0) we can go ahead and do 557 - * %DWC3_DEPCMD_DEPXFERCFG for every hardware endpoint as well. We are 558 - * guaranteed that there are as many transfer resources as endpoints. 559 - * 560 - * This function is called for each endpoint when it is being enabled but is 561 - * triggered only when called for EP0-out, which always happens first, and which 562 - * should only happen in one of the above conditions. 548 + * Set resource_index=2 to reset only non-control endpoints' resources. Do this 549 + * on receiving the SET_CONFIGURATION request or hibernation resume. 563 550 */ 564 - static int dwc3_gadget_start_config(struct dwc3_ep *dep) 551 + int dwc3_gadget_start_config(struct dwc3 *dwc, unsigned int resource_index) 565 552 { 566 553 struct dwc3_gadget_ep_cmd_params params; 567 - struct dwc3 *dwc; 568 554 u32 cmd; 569 555 int i; 570 556 int ret; 571 557 572 - if (dep->number) 573 - return 0; 558 + if (resource_index != 0 && resource_index != 2) 559 + return -EINVAL; 574 560 575 561 memset(&params, 0x00, sizeof(params)); 576 562 cmd = DWC3_DEPCMD_DEPSTARTCFG; 577 - dwc = dep->dwc; 563 + cmd |= DWC3_DEPCMD_PARAM(resource_index); 578 564 579 - ret = dwc3_send_gadget_ep_cmd(dep, cmd, &params); 565 + ret = dwc3_send_gadget_ep_cmd(dwc->eps[0], cmd, &params); 580 566 if (ret) 581 567 return ret; 582 568 583 - for (i = 0; i < DWC3_ENDPOINTS_NUM; i++) { 584 - struct dwc3_ep *dep = dwc->eps[i]; 585 - 586 - if (!dep) 587 - continue; 588 - 589 - ret = dwc3_gadget_set_xfer_resource(dep); 590 - if (ret) 591 - return ret; 592 - } 569 + /* Reset resource allocation flags */ 570 + for (i = resource_index; i < dwc->num_eps && dwc->eps[i]; i++) 571 + dwc->eps[i]->flags &= ~DWC3_EP_RESOURCE_ALLOCATED; 593 572 594 573 return 0; 595 574 } ··· 869 890 ret = dwc3_gadget_resize_tx_fifos(dep); 870 891 if (ret) 871 892 return ret; 872 - 873 - ret = dwc3_gadget_start_config(dep); 874 - if (ret) 875 - return ret; 876 893 } 877 894 878 895 ret = dwc3_gadget_set_ep_config(dep, action); 879 896 if (ret) 880 897 return ret; 898 + 899 + if (!(dep->flags & DWC3_EP_RESOURCE_ALLOCATED)) { 900 + ret = dwc3_gadget_set_xfer_resource(dep); 901 + if (ret) 902 + return ret; 903 + } 881 904 882 905 if (!(dep->flags & DWC3_EP_ENABLED)) { 883 906 struct dwc3_trb *trb_st_hw; ··· 1034 1053 1035 1054 dep->stream_capable = false; 1036 1055 dep->type = 0; 1037 - mask = DWC3_EP_TXFIFO_RESIZED; 1056 + mask = DWC3_EP_TXFIFO_RESIZED | DWC3_EP_RESOURCE_ALLOCATED; 1038 1057 /* 1039 1058 * dwc3_remove_requests() can exit early if DWC3 EP delayed stop is 1040 1059 * set. Do not clear DEP flags, so that the end transfer command will ··· 2894 2913 2895 2914 /* Start with SuperSpeed Default */ 2896 2915 dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512); 2916 + 2917 + ret = dwc3_gadget_start_config(dwc, 0); 2918 + if (ret) { 2919 + dev_err(dwc->dev, "failed to config endpoints\n"); 2920 + return ret; 2921 + } 2897 2922 2898 2923 dep = dwc->eps[0]; 2899 2924 dep->flags = 0;
+1
drivers/usb/dwc3/gadget.h
··· 121 121 int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value, int protocol); 122 122 void dwc3_ep0_send_delayed_status(struct dwc3 *dwc); 123 123 void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force, bool interrupt); 124 + int dwc3_gadget_start_config(struct dwc3 *dwc, unsigned int resource_index); 124 125 125 126 /** 126 127 * dwc3_gadget_ep_get_transfer_index - Gets transfer index from HW