From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lelnx194.ext.ti.com ([198.47.27.80]:11157 "EHLO lelnx194.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752437AbeAKJXW (ORCPT ); Thu, 11 Jan 2018 04:23:22 -0500 Subject: Re: [PATCH] usb: dwc3: core: Don't try to get PHYs during suspend/resume From: Roger Quadros To: Felipe Balbi CC: , , , , "linux-stable # = v4 . 13" , Heikki Krogerus References: <1515589914-23460-1-git-send-email-rogerq@ti.com> <0c2c7e45-9324-316a-d44b-dd17a3a2c68b@ti.com> <87373dzvmi.fsf@linux.intel.com> <327eac7d-80e2-bbd6-4fb4-98d947335698@ti.com> <87wp0pyfmo.fsf@linux.intel.com> <610c35d0-31b9-0c51-81e4-9f0d1daf5c51@ti.com> <87inc8yf7k.fsf@linux.intel.com> Message-ID: Date: Thu, 11 Jan 2018 11:23:17 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org List-ID: On 11/01/18 11:09, Roger Quadros wrote: > +Heikki > > On 11/01/18 10:25, Felipe Balbi wrote: >> >> Hi, >> >> Roger Quadros 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