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. -Doug
