linux-kernel - Re: [PATCH v2 5/5] usb: dwc3: gadget: Increase DWC3 controller halt timeout

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220713025643.GC8200@jackp-linux.qualcomm.com>
Date:   Tue, 12 Jul 2022 19:56:43 -0700
From:   Jack Pham <quic_jackp@...cinc.com>
To:     Wesley Cheng <quic_wcheng@...cinc.com>
CC:     <balbi@...nel.org>, <gregkh@...uxfoundation.org>,
        <linux-kernel@...r.kernel.org>, <linux-usb@...r.kernel.org>,
        <Thinh.Nguyen@...opsys.com>
Subject: Re: [PATCH v2 5/5] usb: dwc3: gadget: Increase DWC3 controller halt
 timeout

Hi Wesley,

On Tue, Jul 12, 2022 at 05:35:23PM -0700, Wesley Cheng wrote:
> Since EP0 transactions need to be completed before the controller halt
> sequence is finished, this may take some time depending on the host and the
> enabled functions.  Increase the controller halt timeout, so that we give
> the controller sufficient time to handle EP0 transfers.
> 
> Fixes: 861c010a2ee1 ("usb: dwc3: gadget: Refactor pullup()")
> Suggested-by: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
> Signed-off-by: Wesley Cheng <quic_wcheng@...cinc.com>
> ---
> Link:
>   https://lore.kernel.org/linux-usb/4988ed34-04a4-060a-ccef-f57790f76a2b@synopsys.com/
> 
>  drivers/usb/dwc3/gadget.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 41b7007358de..e32d7293c447 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2476,6 +2476,7 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
>  	dwc3_gadget_dctl_write_safe(dwc, reg);
>  
>  	do {
> +		msleep(1);

Be aware that this probably won't sleep for *just* 1ms.  From
Documentation/timers/timers-howto.rst:

	msleep(1~20) may not do what the caller intends, and
	will often sleep longer (~20 ms actual sleep for any
	value given in the 1~20ms range). In many cases this
	is not the desired behavior.

So with timeout==500 this loop could very well end up iterating for up
to 10 seconds.  Granted this shouldn't be called from any atomic context
but just wanted to make sure that the effective increase in timeout as
$SUBJECT intends is made clear here and that it's not overly generous.

>  		reg = dwc3_readl(dwc->regs, DWC3_DSTS);
>  		reg &= DWC3_DSTS_DEVCTRLHLT;
>  	} while (--timeout && !(!is_on ^ !reg));

Jack