From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754898AbeAKJqp (ORCPT + 1 other); Thu, 11 Jan 2018 04:46:45 -0500 Received: from fllnx210.ext.ti.com ([198.47.19.17]:35722 "EHLO fllnx210.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754831AbeAKJql (ORCPT ); Thu, 11 Jan 2018 04:46:41 -0500 Subject: Re: [PATCH] usb: dwc3: core: Don't try to get PHYs during suspend/resume 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> <877esoyc5a.fsf@linux.intel.com> From: Roger Quadros Message-ID: <3b5190ec-0730-52a8-37d5-af0b2e178e69@ti.com> Date: Thu, 11 Jan 2018 11:46:35 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <877esoyc5a.fsf@linux.intel.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-GB Content-Transfer-Encoding: 8bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On 11/01/18 11:31, Felipe Balbi wrote: > > Hi, > > Roger Quadros writes: >>> 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 >> 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. > > sounds like it's better to extract out a smaller function that just > checks if we need ULPI bus and registers it, something akin to: > > @@ -482,6 +482,21 @@ static void dwc3_cache_hwparams(struct dwc3 *dwc) > parms->hwparams8 = dwc3_readl(dwc->regs, DWC3_GHWPARAMS8); > } > > +static int dwc3_ulpi_init(struct dwc3 *dwc) > +{ > + int intf; > + > + intf = DWC3_GHWPARAMS3_HSPHY_IFC(dwc->hwparams.hwparams3); > + > + if (intf == DWC3_GHWPARAMS3_HSPHY_IFC_ULPI || > + (intf == DWC3_GHWPARAMS3_HSPHY_IFC_UTMI_ULPI && > + dwc->hsphy_interface && > + !strncmp(dwc->hsphy_interface, "ulpi", 4))) > + return dwc3_ulpi_init(dwc); > + > + return 0; > +} > + > /** > * dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core > * @dwc: Pointer to our controller context structure > @@ -563,11 +578,6 @@ static int dwc3_phy_setup(struct dwc3 *dwc) > break; > } > /* FALLTHROUGH */ > - case DWC3_GHWPARAMS3_HSPHY_IFC_ULPI: > - ret = dwc3_ulpi_init(dwc); > - if (ret) > - return ret; > - /* FALLTHROUGH */ > default: > break; > } > > Then we just call that outside of any functions that get called during PM. > Right. Seems like we've covered everything. I'll send a patch in a while. -- cheers, -roger Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki