linux-kernel - Re: [PATCH] usb: dwc3: Fix timeout issue during controller enter/exit from halt state

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: <d6aed1b7-b267-4b1b-817d-a42d92be04f6@samsung.com>
Date: Sat, 1 Feb 2025 22:13:55 +0530
From: Selvarasu Ganesan <selvarasu.g@...sung.com>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Cc: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"balbi@...com" <balbi@...com>, "linux-usb@...r.kernel.org"
	<linux-usb@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "jh0801.jung@...sung.com"
	<jh0801.jung@...sung.com>, "dh10.jung@...sung.com" <dh10.jung@...sung.com>,
	"naushad@...sung.com" <naushad@...sung.com>, "akash.m5@...sung.com"
	<akash.m5@...sung.com>, "h10.kim@...sung.com" <h10.kim@...sung.com>,
	"eomji.oh@...sung.com" <eomji.oh@...sung.com>, "alim.akhtar@...sung.com"
	<alim.akhtar@...sung.com>, "thiagu.r@...sung.com" <thiagu.r@...sung.com>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH] usb: dwc3: Fix timeout issue during controller
 enter/exit from halt state


On 2/1/2025 5:45 AM, Thinh Nguyen wrote:
> On Fri, Jan 31, 2025, Selvarasu Ganesan wrote:
>> There is a frequent timeout during controller enter/exit from halt state
>> after toggling the run_stop bit by SW. This timeout occurs when
>> performing frequent role switches between host and device, causing
>> device enumeration issues due to the timeout. This issue was not present
>> when USB2 suspend PHY was disabled by passing the SNPS quirks
>> (snps,dis_u2_susphy_quirk and snps,dis_enblslpm_quirk) from the DTS.
>> However, there is a requirement to enable USB2 suspend PHY by setting of
>> GUSB2PHYCFG.ENBLSLPM and GUSB2PHYCFG.SUSPHY bits when controller starts
>> in gadget or host mode results in the timeout issue.
>>
>> This commit addresses this timeout issue by ensuring that the bits
>> GUSB2PHYCFG.ENBLSLPM and GUSB2PHYCFG.SUSPHY are cleared before starting
>> the dwc3_gadget_run_stop sequence and restoring them after the
>> dwc3_gadget_run_stop sequence is completed.
>>
>> Fixes: 72246da40f37 ("usb: Introduce DesignWare USB3 DRD Driver")
>> Cc: stable@...r.kernel.org
>> Signed-off-by: Selvarasu Ganesan <selvarasu.g@...sung.com>
>> ---
>>   drivers/usb/dwc3/gadget.c | 21 +++++++++++++++++++++
>>   1 file changed, 21 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index d27af65eb08a..4a158f703d64 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -2629,10 +2629,25 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
>>   {
>>   	u32			reg;
>>   	u32			timeout = 2000;
>> +	u32			saved_config = 0;
>>   
>>   	if (pm_runtime_suspended(dwc->dev))
>>   		return 0;
>>   
> Can you add some comments here that this was added through experiment
> since it is not documented in the programming guide. It would be great
> to also note which platform you used to test this with.
>
>> +	reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>> +	if (unlikely(reg & DWC3_GUSB2PHYCFG_SUSPHY)) {
> I know we have "unlikely" in the other check, but we should not need it
> here or the other place. Please remove this here.


Hi Thinh,

Thanks for your review comments. I updated below newer version with 
addressed your comments. Please review it.

https://lore.kernel.org/linux-usb/20250201163903.459-1-selvarasu.g@samsung.com/

Thanks,
Selva
>> +		saved_config |= DWC3_GUSB2PHYCFG_SUSPHY;
>> +		reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>> +	}
>> +
>> +	if (reg & DWC3_GUSB2PHYCFG_ENBLSLPM) {
>> +		saved_config |= DWC3_GUSB2PHYCFG_ENBLSLPM;
>> +		reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
>> +	}
>> +
>> +	if (saved_config)
>> +		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>> +
>>   	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
>>   	if (is_on) {
>>   		if (DWC3_VER_IS_WITHIN(DWC3, ANY, 187A)) {
>> @@ -2660,6 +2675,12 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
>>   		reg &= DWC3_DSTS_DEVCTRLHLT;
>>   	} while (--timeout && !(!is_on ^ !reg));
>>   
>> +	if (saved_config) {
>> +		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>> +		reg |= saved_config;
>> +		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>> +	}
>> +
>>   	if (!timeout)
>>   		return -ETIMEDOUT;
>>   
>> -- 
>> 2.17.1
>>
> Thanks,
> Thinh