On 5/1/2019 05:55, Doug Anderson wrote: > Hi, > > On Tue, Apr 30, 2019 at 12:11 AM Artur Petrosyan > <Arthur.Petrosyan@xxxxxxxxxxxx> wrote: >> >> Hi, >> >> On 4/29/2019 21:35, Doug Anderson wrote: >>> Hi, >>> >>> On Mon, Apr 29, 2019 at 4:03 AM Artur Petrosyan >>> <Arthur.Petrosyan@xxxxxxxxxxxx> wrote: >>>> >>>> Hi, >>>> >>>> On 4/27/2019 00:45, Doug Anderson wrote: >>>>> Hi, >>>>> >>>>> On Fri, Apr 19, 2019 at 1:05 PM Artur Petrosyan >>>>> <Arthur.Petrosyan@xxxxxxxxxxxx> wrote: >>>>>> >>>>>> - In dwc2_port_suspend() function added waiting for the >>>>>> HPRT0.PrtSusp register field to be set. >>>>>> >>>>>> - In _dwc2_hcd_suspend() function added checking of >>>>>> "hsotg->flags.b.port_connect_status" port connection >>>>>> status if port connection status is 0 then skipping >>>>>> power saving (entering partial power down mode). >>>>>> Because if there is no device connected there would >>>>>> be no need to enter partial power down mode. >>>>>> >>>>>> - Added "hsotg->bus_suspended = true" beceuse after >>>>> >>>>> s/beceuse/because >>>>> >>>>>> entering partial power down in host mode the >>>>>> bus_suspended flag must be set. >>>>> >>>>> The above statement sounds to me like trying to win an argument by >>>>> saying "I'm right because I'm right." Can you give more details about >>>>> why "bus_suspended" must be set? See below also. >>>>> >>>> There is no intention of winning any argument. >>> >>> I was trying to say that your statement does not convey any >>> information about the "why". It just says: "I now set this variable >>> because it needs to be set". This tells me nothing useful because >>> presumably if it did't need to be set then you wouldn't have set it. >>> I want to know more information about how the code was broken before >>> you did this. What specifically requires this variable to be set? >> Specifically that variable is set when core enters to hibernation or >> partial power down. >>> >>> >>>> Are you familiar with "bus_suspended" flag ? have you looked at what is >>>> it used for ? >>>> >>>> * @bus_suspended: True if bus is suspended >>>> >>>> So when entering to hibernation is performed bus is suspended. To >>>> indicate that "hsotg->bus_suspended" is assigned to true. >>> >>> Perhaps you can help me understand what the difference is between >>> "port suspend" and "bus suspend" on dwc2. I think this is where my >>> confusion lies since there are functions for both and they do subtly >>> different things. ...but both functions set bus_suspended. >> dwc2_port_suspend() is a function which is called when set port feature >> "USB_PORT_FEAT_SUSPEND" is asserted. Yet, bus_suspended is a variable. >> That variable should be set any time that host enters to hibernation or >> partial power down. >> >>> >>> >>>>>> @@ -4514,6 +4519,8 @@ static int _dwc2_hcd_suspend(struct usb_hcd *hcd) >>>>>> goto skip_power_saving; >>>>>> } >>>>>> >>>>>> + hsotg->bus_suspended = true; >>>>>> + >>>>> >>>>> I'm a bit unsure about this. Specifically I note that >>>>> _dwc2_hcd_resume() has a special case "if (hsotg->bus_suspended)". >>>>> Does that now become dead code? >>>> No it doesn't became a dead code. >>> >>> Can you explain when it gets triggered, then? >> _dwc2_hcd_resume() is triggered by the system. >> As an example lets assume you have hibernated the PC and then you turn >> the PC on. When you turn the PC back on in that case _dwc2_hcd_resume() >> function is called to resume from suspended state (Hibernation/partial >> power down) > > I think you are still not understanding my question here. Please > re-read it again. Your question is "Can you explain when it gets triggered, then?" so I have explained one case when it is triggered. > > > -Doug > -- Regards, Artur