On 11/01/18 11:09, Roger Quadros wrote: > +Heikki > > On 11/01/18 10:25, Felipe Balbi wrote: >> >> Hi, >> >> Roger Quadros <rogerq@xxxxxx> writes: >>>>>> - ret = dwc3_core_soft_reset(dwc); >>>>>> + ret = dwc3_core_get_phy(dwc); >>>>> >>>>> we can get_phy in dwc3_core_init() as it will get called on resume(). >>>>> This was the $subject of this patch. >>>> >>>> indeed. thanks :-) >>>> >>> >>> oops sorry. I meant we can't call dwc3_core_get_phy() in dwc3_core_init(). :P >> >> bit of a chicken-and-egg problem. We need to setup the PHY interface >> before getting the PHYs, but can't get PHY during resume. Maybe the best >> way here would be to check for the pointers being valid. Something like: >> >> if (!phy) >> get_phy(); >> > > OK that should take care of not calling get_phy() on suspend. > However there is one more issue with the approach > >> @@ -754,15 +754,15 @@ static int dwc3_core_init(struct dwc3 *dwc) >> dwc->maximum_speed = USB_SPEED_HIGH; >> } >> >> - ret = dwc3_core_get_phy(dwc); >> + ret = dwc3_phy_setup(dwc); >> if (ret) >> goto err0; > > here we configure PHY related bits and register the ulpi interface. > >> >> - ret = dwc3_core_soft_reset(dwc); >> + ret = dwc3_core_get_phy(dwc); >> if (ret) >> goto err0; >> > > we got the PHYs. all OK here. > >> - ret = dwc3_phy_setup(dwc); >> + ret = dwc3_core_soft_reset(dwc); >> if (ret) >> goto err0; > > Now we do a soft reset. This means we loose the PHY configuration bits that we did Actually I was wrong. We're only resetting the device side (DCTL.CSFTRST) which doesn't seem to affect GUSB2PHYCFGn and GUSB3PIPECTLn registers. So we're good. > in dwc3_phy_setup. So we need to call dwc3_phy_setup again but not re-register the ulpi interface. > I can use a flag there so that dwc3_ulpi_init() is done only once. > -- cheers, -roger Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki