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

[Artur Petrosyan]

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@xxxxxxxxxxxx>
>> ---
>>    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