Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966544AbcCPINu (ORCPT ); Wed, 16 Mar 2016 04:13:50 -0400 Received: from mail.kernel.org ([198.145.29.136]:60909 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966237AbcCPINg (ORCPT ); Wed, 16 Mar 2016 04:13:36 -0400 From: lizf@kernel.org To: stable@vger.kernel.org Cc: linux-kernel@vger.kernel.org, John Youn , John Youn , Felipe Balbi , Zefan Li Subject: [PATCH 3.4 105/107] usb: dwc3: Fix assignment of EP transfer resources Date: Wed, 16 Mar 2016 16:06:39 +0800 Message-Id: <1458115601-5762-105-git-send-email-lizf@kernel.org> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1458115541-5712-1-git-send-email-lizf@kernel.org> References: <1458115541-5712-1-git-send-email-lizf@kernel.org> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7469 Lines: 209 From: John Youn 3.4.111-rc1 review patch. If anyone has any objections, please let me know. ------------------ commit c450960187f45d4260db87c7dd4fc0bceb5565d8 upstream. The assignement of EP transfer resources was not handled properly in the dwc3 driver. Commit aebda6187181 ("usb: dwc3: Reset the transfer resource index on SET_INTERFACE") previously fixed one aspect of this where resources may be exhausted with multiple calls to SET_INTERFACE. However, it introduced an issue where composite devices with multiple interfaces can be assigned the same transfer resources for different endpoints. This patch solves both issues. The assignment of transfer resources cannot perfectly follow the data book due to the fact that the controller driver does not have all knowledge of the configuration in advance. It is given this information piecemeal by the composite gadget framework after every SET_CONFIGURATION and SET_INTERFACE. Trying to follow the databook programming model in this scenario can cause errors. For two reasons: 1) The databook says to do DEPSTARTCFG for every SET_CONFIGURATION and SET_INTERFACE (8.1.5). This is incorrect in the scenario of multiple interfaces. 2) The databook does not mention doing more DEPXFERCFG for new endpoint on alt setting (8.1.6). The following simplified method is used instead: All hardware endpoints can be assigned a transfer resource and this setting will stay persistent until either a core reset or hibernation. So whenever we do a DEPSTARTCFG(0) we can go ahead and do DEPXFERCFG for every hardware endpoint as well. We are guaranteed that there are as many transfer resources as endpoints. This patch triggers off of the calling dwc3_gadget_start_config() for EP0-out, which always happens first, and which should only happen in one of the above conditions. Fixes: aebda6187181 ("usb: dwc3: Reset the transfer resource index on SET_INTERFACE") Reported-by: Ravi Babu Signed-off-by: John Youn Signed-off-by: Felipe Balbi [lizf: Backported to 3.4: adjust context] Signed-off-by: Zefan Li --- drivers/usb/dwc3/core.h | 1 - drivers/usb/dwc3/ep0.c | 5 ---- drivers/usb/dwc3/gadget.c | 70 +++++++++++++++++++++++++++++++++++------------ 3 files changed, 52 insertions(+), 24 deletions(-) diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 194cafd..9707601 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -614,7 +614,6 @@ struct dwc3 { unsigned three_stage_setup:1; unsigned ep0_bounced:1; unsigned ep0_expect_in:1; - unsigned start_config_issued:1; unsigned setup_packet_pending:1; unsigned delayed_status:1; unsigned needs_fifo_resize:1; diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c index 7c0eaeb..b6051f3 100644 --- a/drivers/usb/dwc3/ep0.c +++ b/drivers/usb/dwc3/ep0.c @@ -442,7 +442,6 @@ static int dwc3_ep0_set_config(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl) u32 cfg; int ret; - dwc->start_config_issued = false; cfg = le16_to_cpu(ctrl->wValue); switch (dwc->dev_state) { @@ -496,10 +495,6 @@ static int dwc3_ep0_std_request(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl) dev_vdbg(dwc->dev, "USB_REQ_SET_CONFIGURATION\n"); ret = dwc3_ep0_set_config(dwc, ctrl); break; - case USB_REQ_SET_INTERFACE: - dev_vdbg(dwc->dev ,"USB_REQ_SET_INTERFACE"); - dwc->start_config_issued = false; - /* Fall through */ default: dev_vdbg(dwc->dev, "Forwarding to gadget driver\n"); ret = dwc3_ep0_delegate_req(dwc, ctrl); diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index b43c6f9..fba74f8 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -359,24 +359,66 @@ static void dwc3_free_trb_pool(struct dwc3_ep *dep) dep->trb_pool_dma = 0; } +static int dwc3_gadget_set_xfer_resource(struct dwc3 *dwc, struct dwc3_ep *dep); + +/** + * dwc3_gadget_start_config - Configure EP resources + * @dwc: pointer to our controller context structure + * @dep: endpoint that is being enabled + * + * The assignment of transfer resources cannot perfectly follow the + * data book due to the fact that the controller driver does not have + * all knowledge of the configuration in advance. It is given this + * information piecemeal by the composite gadget framework after every + * SET_CONFIGURATION and SET_INTERFACE. Trying to follow the databook + * programming model in this scenario can cause errors. For two + * reasons: + * + * 1) The databook says to do DEPSTARTCFG for every SET_CONFIGURATION + * and SET_INTERFACE (8.1.5). This is incorrect in the scenario of + * multiple interfaces. + * + * 2) The databook does not mention doing more DEPXFERCFG for new + * endpoint on alt setting (8.1.6). + * + * The following simplified method is used instead: + * + * All hardware endpoints can be assigned a transfer resource and this + * setting will stay persistent until either a core reset or + * hibernation. So whenever we do a DEPSTARTCFG(0) we can go ahead and + * do DEPXFERCFG for every hardware endpoint as well. We are + * guaranteed that there are as many transfer resources as endpoints. + * + * This function is called for each endpoint when it is being enabled + * but is triggered only when called for EP0-out, which always happens + * first, and which should only happen in one of the above conditions. + */ static int dwc3_gadget_start_config(struct dwc3 *dwc, struct dwc3_ep *dep) { struct dwc3_gadget_ep_cmd_params params; u32 cmd; + int i; + int ret; + + if (dep->number) + return 0; memset(¶ms, 0x00, sizeof(params)); + cmd = DWC3_DEPCMD_DEPSTARTCFG; - if (dep->number != 1) { - cmd = DWC3_DEPCMD_DEPSTARTCFG; - /* XferRscIdx == 0 for ep0 and 2 for the remaining */ - if (dep->number > 1) { - if (dwc->start_config_issued) - return 0; - dwc->start_config_issued = true; - cmd |= DWC3_DEPCMD_PARAM(2); - } + ret = dwc3_send_gadget_ep_cmd(dwc, 0, cmd, ¶ms); + if (ret) + return ret; - return dwc3_send_gadget_ep_cmd(dwc, 0, cmd, ¶ms); + for (i = 0; i < DWC3_ENDPOINTS_NUM; i++) { + struct dwc3_ep *dep = dwc->eps[i]; + + if (!dep) + continue; + + ret = dwc3_gadget_set_xfer_resource(dwc, dep); + if (ret) + return ret; } return 0; @@ -471,10 +513,6 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep, struct dwc3_trb *trb_st_hw; struct dwc3_trb *trb_link; - ret = dwc3_gadget_set_xfer_resource(dwc, dep); - if (ret) - return ret; - dep->desc = desc; dep->comp_desc = comp_desc; dep->type = usb_endpoint_type(desc); @@ -1375,8 +1413,6 @@ static int dwc3_gadget_start(struct usb_gadget *g, reg |= dwc->maximum_speed; dwc3_writel(dwc->regs, DWC3_DCFG, reg); - dwc->start_config_issued = false; - /* Start with SuperSpeed Default */ dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512); @@ -1861,7 +1897,6 @@ static void dwc3_gadget_disconnect_interrupt(struct dwc3 *dwc) dwc3_stop_active_transfers(dwc); dwc3_disconnect_gadget(dwc); - dwc->start_config_issued = false; dwc->gadget.speed = USB_SPEED_UNKNOWN; dwc->setup_packet_pending = false; @@ -1949,7 +1984,6 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc) dwc3_stop_active_transfers(dwc); dwc3_clear_stall_all_ep(dwc); - dwc->start_config_issued = false; /* Reset device address to zero */ reg = dwc3_readl(dwc->regs, DWC3_DCFG); -- 1.9.1