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

Artur Petrosyan
2021-04-16T05:46:26Z

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@synopsys.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