linux-kernel - Re: [PATCH 13/15] usb: dwc2: Add exit hibernation mode before removing drive

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] [day] [month] [year] [list]

Message-ID: <30efe9e7-d132-75a4-f76a-48aaeb6400f4@synopsys.com>
Date:   Fri, 16 Apr 2021 05:46:17 +0000
From:   Artur Petrosyan <Arthur.Petrosyan@...opsys.com>
To:     Sergei Shtylyov <sergei.shtylyov@...il.com>,
        Felipe Balbi <balbi@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Minas Harutyunyan <Minas.Harutyunyan@...opsys.com>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     John Youn <John.Youn@...opsys.com>
Subject: Re: [PATCH 13/15] usb: dwc2: Add exit hibernation mode before
 removing drive

Hi Sergei,

On 4/15/2021 13:24, Sergei Shtylyov wrote:
> On 15.04.2021 8:41, Artur Petrosyan wrote:
> 
>> When dwc2 core is in hibernation mode loading
>> driver again causes driver fail. Because in
>> that mode registers are not accessible.
>>
>> In order to exit from hibernation checking
>> dwc2 core power saving state in "dwc2_driver_remove()"
>> function. If core is in hibernation, then checking the
>> operational mode of the driver. To check whether dwc2 core
>> is operating in host mode or device mode there is one way
>> which is retrieving the backup value of "gotgctl" and compare
>> the "CurMod" value. If previously core entered hibernation
>> in host mode then the exit is performed for host if not then
>> exit is performed for device mode. The introduced checking
>> is because in hibernation state all registers are not
>> accessible.
>>
>> Signed-off-by: Artur Petrosyan <Arthur.Petrosyan@...opsys.com>
>> ---
>>    drivers/usb/dwc2/platform.c | 16 ++++++++++++++++
>>    1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
>> index f8b819cfa80e..2ae4748ed5ec 100644
>> --- a/drivers/usb/dwc2/platform.c
>> +++ b/drivers/usb/dwc2/platform.c
>> @@ -316,8 +316,24 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg)
>>    static int dwc2_driver_remove(struct platform_device *dev)
>>    {
>>    	struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
>> +	struct dwc2_gregs_backup *gr;
>>    	int ret = 0;
>>    
>> +	/* Exit Hibernation when driver is removed. */
>> +	if (hsotg->hibernated) {
>> +		if (gr->gotgctl & GOTGCTL_CURMODE_HOST) {
>> +			ret = dwc2_exit_hibernation(hsotg, 0, 0, 1);
>> +			if (ret)
>> +				dev_err(hsotg->dev,
>> +					"exit hibernation failed.\n");
>> +		} else {
>> +			ret = dwc2_exit_hibernation(hsotg, 0, 0, 0);
>> +			if (ret)
>> +				dev_err(hsotg->dev,
>> +					"exit hibernation failed.\n");
> 
>      Again, why duplicate the innermost *if*?
Again the reason is that combination of inner and outside ifs would give 
as a situation when core would not be hibernated but driver would try to 
exit from host or device hibernation.

> 
>>   +		}
>> +	}
>> +
>>    	/* Exit Partial Power Down when driver is removed. */
>>    	if (hsotg->in_ppd) {
>>    		ret = dwc2_exit_partial_power_down(hsotg, 0, true);
> 
> MBR, Sergei
> 
Regards,
Artur