| Message ID | 20220831183242.27826-2-quic_wcheng@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix controller halt and endxfer timeout issues | expand |
Hi Wesley,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on usb/usb-testing]
[also build test ERROR on next-20220831]
[cannot apply to linus/master v6.0-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Wesley-Cheng/Fix-controller-halt-and-endxfer-timeout-issues/20220901-023750
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: x86_64-randconfig-a012 (https://download.01.org/0day-ci/archive/20220901/202209011138.FP2kL0sl-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/18bece9b81c07fbd2dbcec20ef8cc7e56d1ebf35
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Wesley-Cheng/Fix-controller-halt-and-endxfer-timeout-issues/20220901-023750
git checkout 18bece9b81c07fbd2dbcec20ef8cc7e56d1ebf35
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/usb/dwc3/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/usb/dwc3/gadget.c:968:6: error: conflicting types for 'dwc3_remove_requests'
void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep, int status)
^
drivers/usb/dwc3/core.h:1563:6: note: previous declaration is here
void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep);
^
>> drivers/usb/dwc3/gadget.c:1029:33: error: too many arguments to function call, expected 2, have 3
dwc3_remove_requests(dwc, dep, -ECONNRESET);
~~~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~
drivers/usb/dwc3/core.h:1563:6: note: 'dwc3_remove_requests' declared here
void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep);
^
drivers/usb/dwc3/gadget.c:2347:34: error: too many arguments to function call, expected 2, have 3
dwc3_remove_requests(dwc, dep, -ESHUTDOWN);
~~~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~
drivers/usb/dwc3/core.h:1563:6: note: 'dwc3_remove_requests' declared here
void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep);
^
3 errors generated.
vim +/dwc3_remove_requests +968 drivers/usb/dwc3/gadget.c
967
> 968 void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep, int status)
969 {
970 struct dwc3_request *req;
971
972 dwc3_stop_active_transfer(dep, true, false);
973
974 /* If endxfer is delayed, avoid unmapping requests */
975 if (dep->flags & DWC3_EP_DELAY_STOP)
976 return;
977
978 /* - giveback all requests to gadget driver */
979 while (!list_empty(&dep->started_list)) {
980 req = next_request(&dep->started_list);
981
982 dwc3_gadget_giveback(dep, req, status);
983 }
984
985 while (!list_empty(&dep->pending_list)) {
986 req = next_request(&dep->pending_list);
987
988 dwc3_gadget_giveback(dep, req, status);
989 }
990
991 while (!list_empty(&dep->cancelled_list)) {
992 req = next_request(&dep->cancelled_list);
993
994 dwc3_gadget_giveback(dep, req, status);
995 }
996 }
997
998 /**
999 * __dwc3_gadget_ep_disable - disables a hw endpoint
1000 * @dep: the endpoint to disable
1001 *
1002 * This function undoes what __dwc3_gadget_ep_enable did and also removes
1003 * requests which are currently being processed by the hardware and those which
1004 * are not yet scheduled.
1005 *
1006 * Caller should take care of locking.
1007 */
1008 static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
1009 {
1010 struct dwc3 *dwc = dep->dwc;
1011 u32 reg;
1012
1013 trace_dwc3_gadget_ep_disable(dep);
1014
1015 /* make sure HW endpoint isn't stalled */
1016 if (dep->flags & DWC3_EP_STALL)
1017 __dwc3_gadget_ep_set_halt(dep, 0, false);
1018
1019 reg = dwc3_readl(dwc->regs, DWC3_DALEPENA);
1020 reg &= ~DWC3_DALEPENA_EP(dep->number);
1021 dwc3_writel(dwc->regs, DWC3_DALEPENA, reg);
1022
1023 /* Clear out the ep descriptors for non-ep0 */
1024 if (dep->number > 1) {
1025 dep->endpoint.comp_desc = NULL;
1026 dep->endpoint.desc = NULL;
1027 }
1028
> 1029 dwc3_remove_requests(dwc, dep, -ECONNRESET);
1030
1031 dep->stream_capable = false;
1032 dep->type = 0;
1033 dep->flags &= DWC3_EP_TXFIFO_RESIZED;
1034
1035 return 0;
1036 }
1037
On Wed, Aug 31, 2022 at 11:32:38AM -0700, Wesley Cheng wrote: > If DWC3_EP_DELAYED_STOP is set during stop active transfers, then do not > continue attempting to unmap request buffers during dwc3_remove_requests(). > This can lead to SMMU faults, as the controller has not stopped the > processing of the TRB. Defer this sequence to the EP0 out start, which > ensures that there are no pending SETUP transactions before issuing the > endxfer. > > Reviewed-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> > --- > drivers/usb/dwc3/core.h | 3 +++ > drivers/usb/dwc3/ep0.c | 5 ++++- > drivers/usb/dwc3/gadget.c | 6 +++++- > 3 files changed, 12 insertions(+), 2 deletions(-) As the kernel-test robot reported, this patch doesn't even build. How did you test it? greg k-h
On 9/1/2022 7:10 AM, Greg KH wrote: > On Wed, Aug 31, 2022 at 11:32:38AM -0700, Wesley Cheng wrote: >> If DWC3_EP_DELAYED_STOP is set during stop active transfers, then do not >> continue attempting to unmap request buffers during dwc3_remove_requests(). >> This can lead to SMMU faults, as the controller has not stopped the >> processing of the TRB. Defer this sequence to the EP0 out start, which >> ensures that there are no pending SETUP transactions before issuing the >> endxfer. >> >> Reviewed-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> >> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> >> --- >> drivers/usb/dwc3/core.h | 3 +++ >> drivers/usb/dwc3/ep0.c | 5 ++++- >> drivers/usb/dwc3/gadget.c | 6 +++++- >> 3 files changed, 12 insertions(+), 2 deletions(-) > > As the kernel-test robot reported, this patch doesn't even build. How > did you test it? > Sorry Greg, didn't get a chance to test it before submitting, but fixed the build errors in v6. I had initially missed the argument changes to the APIs. V6 works as expected. Thanks Wesley Cheng
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 4fe4287dc934..7327e5767df9 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -1560,6 +1560,7 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd, int dwc3_send_gadget_generic_command(struct dwc3 *dwc, unsigned int cmd, u32 param); void dwc3_gadget_clear_tx_fifos(struct dwc3 *dwc); +void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep); #else static inline int dwc3_gadget_init(struct dwc3 *dwc) { return 0; } @@ -1581,6 +1582,8 @@ static inline int dwc3_send_gadget_generic_command(struct dwc3 *dwc, { return 0; } static inline void dwc3_gadget_clear_tx_fifos(struct dwc3 *dwc) { } +static inline void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep) +{ } #endif #if IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE) diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c index 33cee0089609..fbb154a5ee1f 100644 --- a/drivers/usb/dwc3/ep0.c +++ b/drivers/usb/dwc3/ep0.c @@ -293,7 +293,10 @@ void dwc3_ep0_out_start(struct dwc3 *dwc) continue; dwc3_ep->flags &= ~DWC3_EP_DELAY_STOP; - dwc3_stop_active_transfer(dwc3_ep, true, true); + if (dwc->connected) + dwc3_stop_active_transfer(dwc3_ep, true, true); + else + dwc3_remove_requests(dwc, dwc3_ep); } } diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index d6c0cb79ace3..6f2491fc109e 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -965,12 +965,16 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep, unsigned int action) return 0; } -static void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep, int status) +void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep, int status) { struct dwc3_request *req; dwc3_stop_active_transfer(dep, true, false); + /* If endxfer is delayed, avoid unmapping requests */ + if (dep->flags & DWC3_EP_DELAY_STOP) + return; + /* - giveback all requests to gadget driver */ while (!list_empty(&dep->started_list)) { req = next_request(&dep->started_list);