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) > > > -Doug > -- Regards, Artur
