Hi Sergei,
On 4/16/2021 09:43, Artur Petrosyan wrote:
> Hi Sergei,
>
> On 4/15/2021 13:12, Sergei Shtylyov wrote:
>> On 15.04.2021 8:40, Artur Petrosyan wrote:
>>
>>> When core is in hibernation state and an external
>>> hub is connected, upper layer sends URB enqueue request,
>>> which results in port reset issue.
>>>
>>> - Added exit from hibernation state to avoid port
>>> reset issue and process upper layer request properly.
>>>
>>> Signed-off-by: Artur Petrosyan <Arthur.Petrosyan@xxxxxxxxxxxx>
>>> ---
>>> drivers/usb/dwc2/hcd.c | 17 +++++++++++++++++
>>> 1 file changed, 17 insertions(+)
>>>
>>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>>> index cc9ad6cf02d9..3b03b2d73aaa 100644
>>> --- a/drivers/usb/dwc2/hcd.c
>>> +++ b/drivers/usb/dwc2/hcd.c
>>> @@ -4631,12 +4631,29 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
>>> struct dwc2_qh *qh;
>>> bool qh_allocated = false;
>>> struct dwc2_qtd *qtd;
>>> + struct dwc2_gregs_backup *gr;
>>> +
>>> + gr = &hsotg->gr_backup;
>>>
>>> if (dbg_urb(urb)) {
>>> dev_vdbg(hsotg->dev, "DWC OTG HCD URB Enqueue\n");
>>> dwc2_dump_urb_info(hcd, urb, "urb_enqueue");
>>> }
>>>
>>> + if (hsotg->hibernated) {
>>> + if (gr->gotgctl & GOTGCTL_CURMODE_HOST) {
>>> + retval = dwc2_exit_hibernation(hsotg, 0, 0, 1);
>>> + if (retval)
>>> + dev_err(hsotg->dev,
>>> + "exit hibernation failed.\n");
>>> + } else {
>>> + retval = dwc2_exit_hibernation(hsotg, 0, 0, 0);
>>> + if (retval)
>>> + dev_err(hsotg->dev,
>>> + "exit hibernation failed.\n");
>>
>> Why not put these identical *if*s outside the the outer *if*?
>>
> Well the reason that the conditions are implemented like they are, is
> that the inner if checks whether core operates in host mode or device
> mode and the outside if checks if the core is hibernated or not.
>
> So now imagine that the ifs are combined and core is not hibernated but
> the operational mode of the driver is let's say gadget. If the case is
> such then the driver will try to exit from gadget hibernation because of
> the else condition as the if condition will be false again because core
> is not hibernated. As a result if we combine the outside and inner ifs
> then it will try to exit from gadget hibernation but core is not
> hibernated and that would be an issue.
>
Sorry I got your point wrong there. You meant the ifs for dev_err().
Thank you for the review I will update them.
>
>>
>>> + }
>>> + }
>>> +
>>> if (hsotg->in_ppd) {
>>> retval = dwc2_exit_partial_power_down(hsotg, 0, true);
>>> if (retval)
>>
>> MBR, Sergei
>>
>
> Regards,
> Artur
>
