| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 9 Mar 2022 11:01:14 -0800
From: Wesley Cheng <wcheng@...eaurora.org>
To: Greg KH <gregkh@...uxfoundation.org>,
Wesley Cheng <quic_wcheng@...cinc.com>
Cc: balbi@...nel.org, linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org, quic_jackp@...cinc.com,
Thinh.Nguyen@...opsys.com
Subject: Re: [PATCH] usb: dwc3: gadget: Wait for ep0 xfers to complete during
dequeue
Hi Greg,
On 3/9/2022 2:21 AM, Greg KH wrote:
> On Tue, Mar 08, 2022 at 04:41:48PM -0800, Wesley Cheng wrote:
>> From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
>>
>> If the request being dequeued is currently active, then the current
>> logic is to issue a stop transfer command, and allow the command
>> completion to cleanup the cancelled list. The DWC3 controller will
>> run into an end transfer command timeout if there is an ongoing EP0
>> transaction. If this is the case, wait for the EP0 completion event
>> before proceeding to retry the endxfer command again.
>>
>> Co-developed-by: Wesley Cheng <quic_wcheng@...cinc.com>
>> Signed-off-by: Wesley Cheng <quic_wcheng@...cinc.com>
>> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
>
>
> You sent this twice? What is the differences between the patches?
>
Sorry for sending it twice. It was a mistake on my end, where I thought
the patch wasn't getting sent out by our email server, so I sent it out
several times to check. Ended up just showing up after 30 mins or so :/.
> And as you sent it, your signed-off-by needs to be at the end, as per
> the kernel documentation.
>
I'll fix this.
>> ---
>> Patch discussion below:
>> https://lore.kernel.org/linux-usb/1644836933-141376-1-git-send-email-dh10.jung@samsung.com/T/#t
>
> So this is a v2?
>
>
This is an entirely different change, but the change was discussed in
the above thread.
>>
>> drivers/usb/dwc3/core.h | 2 +-
>> drivers/usb/dwc3/ep0.c | 14 ++++++++++++++
>> drivers/usb/dwc3/gadget.c | 13 ++++++++-----
>> drivers/usb/dwc3/gadget.h | 1 +
>> 4 files changed, 24 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index eb9c1efced05..f557f5f36a7f 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -736,7 +736,7 @@ struct dwc3_ep {
>> #define DWC3_EP_FIRST_STREAM_PRIMED BIT(10)
>> #define DWC3_EP_PENDING_CLEAR_STALL BIT(11)
>> #define DWC3_EP_TXFIFO_RESIZED BIT(12)
>> -
>> +#define DWC3_EP_DELAY_STOP BIT(13)
>
> Why did you loose the blank line?
>
I can add that back in.
Thanks
Wesley Cheng
>> /* This last one is specific to EP0 */
>> #define DWC3_EP0_DIR_IN BIT(31)
>>
>> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
>> index 658739410992..1064be5518f6 100644
>> --- a/drivers/usb/dwc3/ep0.c
>> +++ b/drivers/usb/dwc3/ep0.c
>> @@ -271,6 +271,7 @@ void dwc3_ep0_out_start(struct dwc3 *dwc)
>> {
>> struct dwc3_ep *dep;
>> int ret;
>> + int i;
>>
>> complete(&dwc->ep0_in_setup);
>>
>> @@ -279,6 +280,19 @@ void dwc3_ep0_out_start(struct dwc3 *dwc)
>> DWC3_TRBCTL_CONTROL_SETUP, false);
>> ret = dwc3_ep0_start_trans(dep);
>> WARN_ON(ret < 0);
>> + for (i = 2; i < DWC3_ENDPOINTS_NUM; i++) {
>> + struct dwc3_ep *dwc3_ep;
>> +
>> + dwc3_ep = dwc->eps[i];
>> + if (!dwc3_ep)
>> + continue;
>> +
>> + if (!(dwc3_ep->flags & DWC3_EP_DELAY_STOP))
>> + continue;
>> +
>> + dwc3_ep->flags &= ~DWC3_EP_DELAY_STOP;
>> + dwc3_stop_active_transfer(dwc3_ep, true, true);
>> + }
>> }
>>
>> static struct dwc3_ep *dwc3_wIndex_to_dep(struct dwc3 *dwc, __le16 wIndex_le)
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index a0c883f19a41..ccef508b1296 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -654,9 +654,6 @@ static int dwc3_gadget_set_ep_config(struct dwc3_ep *dep, unsigned int action)
>> return dwc3_send_gadget_ep_cmd(dep, DWC3_DEPCMD_SETEPCONFIG, ¶ms);
>> }
>>
>> -static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force,
>> - bool interrupt);
>> -
>> /**
>> * dwc3_gadget_calc_tx_fifo_size - calculates the txfifo size value
>> * @dwc: pointer to the DWC3 context
>> @@ -1899,6 +1896,7 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct dwc3_request *req)
>> */
>> if ((dep->flags & DWC3_EP_END_TRANSFER_PENDING) ||
>> (dep->flags & DWC3_EP_WEDGE) ||
>> + (dep->flags & DWC3_EP_DELAY_STOP) ||
>> (dep->flags & DWC3_EP_STALL)) {
>> dep->flags |= DWC3_EP_DELAY_START;
>> return 0;
>> @@ -2033,6 +2031,9 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
>> if (r == req) {
>> struct dwc3_request *t;
>>
>> + if (dwc->ep0state != EP0_SETUP_PHASE && !dwc->delayed_status)
>> + dep->flags |= DWC3_EP_DELAY_STOP;
>> +
>> /* wait until it is processed */
>> dwc3_stop_active_transfer(dep, true, true);
>>
>> @@ -2116,7 +2117,8 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value, int protocol)
>> list_for_each_entry_safe(req, tmp, &dep->started_list, list)
>> dwc3_gadget_move_cancelled_request(req, DWC3_REQUEST_STATUS_STALLED);
>>
>> - if (dep->flags & DWC3_EP_END_TRANSFER_PENDING) {
>> + if (dep->flags & DWC3_EP_END_TRANSFER_PENDING ||
>> + (dep->flags & DWC3_EP_DELAY_STOP)) {
>> dep->flags |= DWC3_EP_PENDING_CLEAR_STALL;
>> return 0;
>> }
>> @@ -3596,7 +3598,7 @@ static void dwc3_reset_gadget(struct dwc3 *dwc)
>> }
>> }
>>
>> -static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force,
>> +void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force,
>> bool interrupt)
>
> This is a horrid api (2 booleans?) But you aren't adding it so I guess
> we can live with it :(
>
> thanks,
>
> greg k-h
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Powered by blists - more mailing lists