> From: Huang Rui [mailto:ray.huang@xxxxxxx] > Sent: Monday, October 20, 2014 2:01 AM > > On Mon, Oct 20, 2014 at 04:41:54PM +0800, Huang Rui wrote: > > On Fri, Oct 17, 2014 at 01:48:19PM -0500, Felipe Balbi wrote: > > > Hi, > > > > > > On Fri, Oct 17, 2014 at 06:41:04PM +0000, Paul Zimmerman wrote: > > > > > From: Felipe Balbi [mailto:balbi@xxxxxx] > > > > > Sent: Friday, October 17, 2014 8:00 AM > > > > > > > > > > On Fri, Oct 17, 2014 at 04:53:41PM +0800, Huang Rui wrote: > > > > > > AMD NL needs to suspend usb3 ss phy, but this doesn't enable on simulation > > > > > > board. > > > > > > > > > > > > Signed-off-by: Huang Rui <ray.huang@xxxxxxx> > > > > > > --- > > > > > > drivers/usb/dwc3/core.c | 7 ++++++- > > > > > > drivers/usb/dwc3/dwc3-pci.c | 3 ++- > > > > > > drivers/usb/dwc3/platform_data.h | 1 + > > > > > > 3 files changed, 9 insertions(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > > > > > index 3ccfe41..4a98696 100644 > > > > > > --- a/drivers/usb/dwc3/core.c > > > > > > +++ b/drivers/usb/dwc3/core.c > > > > > > @@ -395,6 +395,9 @@ static void dwc3_phy_setup(struct dwc3 *dwc) > > > > > > if (dwc->quirks & DWC3_QUIRK_TX_DEEPH) > > > > > > reg |= DWC3_GUSB3PIPECTL_TX_DEEPH(1); > > > > > > > > > > > > + if (dwc->quirks & DWC3_QUIRK_SUSPHY) > > > > > > > > > > should be: > > > > > > > > > > if (!dwc->suspend_usb3_phy_quirk) > > > > > > > > > > > + reg |= DWC3_GUSB3PIPECTL_SUSPHY; > > > > > > > > > > IIRC, databook asks us to set that bit anyway, so the quirk is disabling > > > > > that bit. Am I missing something ? Paul ? > > > > > > > > It looks to me that Huang's patch is enabling that bit, not disabling > > > > it. > > > > > > right, but that's what's actually quirky right ? IIRC, Databook asks us > > > to set usb2 and usb3 suspend phy bits and we're just not doing it. > > > > > > > Currently the driver does not set either DWC3_GUSB3PIPECTL_SUSPHY or > > > > DWC3_GUSB2PHYCFG_SUSPHY (unless it has been added by that big patch > > > > series you just posted). According to the databook, both of those > > > > bits should be set to 1 after the core initialization has completed. > > > > > > there you go. So unless that quirk bit flag is set, we're safe of > > > setting SUSPHY bit for everybody. > > > > > > > I read the databook again, below words (DWC3_GUSB3PIPECTL_SUSPHY) is > copied from databook: > > For DRD/OTG configurations, it is recommended that this bit is set to‘ > 0’ during coreConsultant configuration. If it is set to ’1’, then the > application should clear this bit after power-on reset. Application > needs to set it to ’1’ after the core initialization is completed. > For all other configurations, this bit can be set to ’1’ during core > configuration. > > I see it's recommended to set '0' if on DRD/OTG configuration. No, it's recommended to set it to '0' during coreConsultant configuration. This is a part of the synthesis process, i.e. when building the RTL for the SoC. This determines what the default value of the bit will be when the core is reset. At runtime it is still recommended to set it to '1' when in device mode, after the core initialization is completed. -- Paul ��.n��������+%������w��{.n�����{���)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
