| Message ID | e8f04e642889b4c865aaf06762cde9386e0ff830.1713310411.git.Thinh.Nguyen@synopsys.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: dwc3: Disable susphy during initialization | expand |
Hello Thinh, On 17/04/2024 02:41, Thinh Nguyen wrote: > GUSB3PIPECTL.SUSPENDENABLE and GUSB2PHYCFG.SUSPHY should be cleared > during initialization. Suspend during initialization can result in > undefined behavior due to clock synchronization failure, which often > seen as core soft reset timeout. > > The programming guide recommended these bits to be cleared during > initialization for DWC_usb3.0 version 1.94 and above (along with > DWC_usb31 and DWC_usb32). The current check in the driver does not > account if it's set by default setting from coreConsultant. > > This is especially the case for DRD when switching mode to ensure the > phy clocks are available to change mode. Depending on the > platforms/design, some may be affected more than others. This is noted > in the DWC_usb3x programming guide under the above registers. > > Let's just disable them during driver load and mode switching. Restore > them when the controller initialization completes. > > Note that some platforms workaround this issue by disabling phy suspend > through "snps,dis_u3_susphy_quirk" and "snps,dis_u2_susphy_quirk" when > they should not need to. > > Cc: stable@vger.kernel.org > Fixes: 9ba3aca8fe82 ("usb: dwc3: Disable phy suspend after power-on reset") > Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> This patch is causing system suspend failures on TI AM62 platforms [1] I will try to explain why. Before this patch, both DWC3_GUSB3PIPECTL_SUSPHY and DWC3_GUSB2PHYCFG_SUSPHY bits (hence forth called 2 SUSPHY bits) were being set during initialization and even during re-initialization after a system suspend/resume. These bits are required to be set for system suspend/resume to work correctly on AM62 platforms. After this patch, the bits are only set when Host controller starts or when Gadget driver starts. On AM62 platform we have 2 USB controllers, one in Host and one in Dual role. Just after boot, for the Host controller we have the 2 SUSPHY bits set but for the Dual-Role controller, as no role has started the 2 SUSPHY bits are not set. Thus system suspend resume will fail. On the other hand, if we load a gadget driver just after boot then both controllers have the 2 SUSPHY bits set and system suspend resume works for the first time. However, after system resume, the core is re-initialized so the 2 SUSPHY bits are cleared for both controllers. For host controller it is never set again. For gadget controller as gadget start is called, the 2 SUSPHY bits are set again. The second system suspend resume will still fail as one controller (Host) doesn't have the 2 SUSPHY bits set. To summarize, the existing solution is not sufficient for us to have a reliable behavior. We need the 2 SUSPHY bits to be set regardless of what role we are in or whether the role has started or not. My suggestion is to move back the SUSPHY enable to end of dwc3_core_init(). Then if SUSPHY needs to be disabled for DRD role switching, it should be disabled and enabled exactly there. What do you suggest? [1] - https://lore.kernel.org/linux-arm-kernel/20240904194229.109886-1-msp@baylibre.com/
Hi Roger, On Wed, Sep 25, 2024, Roger Quadros wrote: > Hello Thinh, > > On 17/04/2024 02:41, Thinh Nguyen wrote: > > GUSB3PIPECTL.SUSPENDENABLE and GUSB2PHYCFG.SUSPHY should be cleared > > during initialization. Suspend during initialization can result in > > undefined behavior due to clock synchronization failure, which often > > seen as core soft reset timeout. > > > > The programming guide recommended these bits to be cleared during > > initialization for DWC_usb3.0 version 1.94 and above (along with > > DWC_usb31 and DWC_usb32). The current check in the driver does not > > account if it's set by default setting from coreConsultant. > > > > This is especially the case for DRD when switching mode to ensure the > > phy clocks are available to change mode. Depending on the > > platforms/design, some may be affected more than others. This is noted > > in the DWC_usb3x programming guide under the above registers. > > > > Let's just disable them during driver load and mode switching. Restore > > them when the controller initialization completes. > > > > Note that some platforms workaround this issue by disabling phy suspend > > through "snps,dis_u3_susphy_quirk" and "snps,dis_u2_susphy_quirk" when > > they should not need to. > > > > Cc: stable@vger.kernel.org > > Fixes: 9ba3aca8fe82 ("usb: dwc3: Disable phy suspend after power-on reset") > > Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > > This patch is causing system suspend failures on TI AM62 platforms [1] > > I will try to explain why. > Before this patch, both DWC3_GUSB3PIPECTL_SUSPHY and DWC3_GUSB2PHYCFG_SUSPHY > bits (hence forth called 2 SUSPHY bits) were being set during initialization > and even during re-initialization after a system suspend/resume. > > These bits are required to be set for system suspend/resume to work correctly > on AM62 platforms. Is it only for suspend or both suspend and resume? > > After this patch, the bits are only set when Host controller starts or > when Gadget driver starts. > > On AM62 platform we have 2 USB controllers, one in Host and one in Dual role. > Just after boot, for the Host controller we have the 2 SUSPHY bits set but > for the Dual-Role controller, as no role has started the 2 SUSPHY bits are > not set. Thus system suspend resume will fail. > > On the other hand, if we load a gadget driver just after boot then both > controllers have the 2 SUSPHY bits set and system suspend resume works for > the first time. > However, after system resume, the core is re-initialized so the 2 SUSPHY bits > are cleared for both controllers. For host controller it is never set again. > For gadget controller as gadget start is called, the 2 SUSPHY bits are set > again. The second system suspend resume will still fail as one controller > (Host) doesn't have the 2 SUSPHY bits set. > > To summarize, the existing solution is not sufficient for us to have a > reliable behavior. We need the 2 SUSPHY bits to be set regardless of what > role we are in or whether the role has started or not. > > My suggestion is to move back the SUSPHY enable to end of dwc3_core_init(). > Then if SUSPHY needs to be disabled for DRD role switching, it should be > disabled and enabled exactly there. > > What do you suggest? > > [1] - https://urldefense.com/v3/__https://lore.kernel.org/linux-arm-kernel/20240904194229.109886-1-msp@baylibre.com/__;!!A4F2R9G_pg!Y10q3gwCzryOoiXpk6DMGn74iFQIg6GloY10J16kWCbqwgS1Algo5HRg05vm38dMw8n47qmKpqJlyXt9Kqlm$ > Thanks for reporting the issue. This is quite an interesting behavior. As you said, we will need to isolate this change to only during DRD role switch. We may not necessarily just enable at the end of dwc3_core_init() since that would keep the SUSPHY bits on during the DRD role switch. If this issue only occurs before suspend, perhaps we can check and set these bits during suspend or dwc3_core_exit() instead? BR, Thinh
On 27/09/2024 00:51, Thinh Nguyen wrote: > Hi Roger, > > On Wed, Sep 25, 2024, Roger Quadros wrote: >> Hello Thinh, >> >> On 17/04/2024 02:41, Thinh Nguyen wrote: >>> GUSB3PIPECTL.SUSPENDENABLE and GUSB2PHYCFG.SUSPHY should be cleared >>> during initialization. Suspend during initialization can result in >>> undefined behavior due to clock synchronization failure, which often >>> seen as core soft reset timeout. >>> >>> The programming guide recommended these bits to be cleared during >>> initialization for DWC_usb3.0 version 1.94 and above (along with >>> DWC_usb31 and DWC_usb32). The current check in the driver does not >>> account if it's set by default setting from coreConsultant. >>> >>> This is especially the case for DRD when switching mode to ensure the >>> phy clocks are available to change mode. Depending on the >>> platforms/design, some may be affected more than others. This is noted >>> in the DWC_usb3x programming guide under the above registers. >>> >>> Let's just disable them during driver load and mode switching. Restore >>> them when the controller initialization completes. >>> >>> Note that some platforms workaround this issue by disabling phy suspend >>> through "snps,dis_u3_susphy_quirk" and "snps,dis_u2_susphy_quirk" when >>> they should not need to. >>> >>> Cc: stable@vger.kernel.org >>> Fixes: 9ba3aca8fe82 ("usb: dwc3: Disable phy suspend after power-on reset") >>> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> >> >> This patch is causing system suspend failures on TI AM62 platforms [1] >> >> I will try to explain why. >> Before this patch, both DWC3_GUSB3PIPECTL_SUSPHY and DWC3_GUSB2PHYCFG_SUSPHY >> bits (hence forth called 2 SUSPHY bits) were being set during initialization >> and even during re-initialization after a system suspend/resume. >> >> These bits are required to be set for system suspend/resume to work correctly >> on AM62 platforms. > > Is it only for suspend or both suspend and resume? I'm sure about suspend. It is not possible to toggle those bits while in system suspend so we can't really say if it is required exclusively for system resume or not. > >> >> After this patch, the bits are only set when Host controller starts or >> when Gadget driver starts. >> >> On AM62 platform we have 2 USB controllers, one in Host and one in Dual role. >> Just after boot, for the Host controller we have the 2 SUSPHY bits set but >> for the Dual-Role controller, as no role has started the 2 SUSPHY bits are >> not set. Thus system suspend resume will fail. >> >> On the other hand, if we load a gadget driver just after boot then both >> controllers have the 2 SUSPHY bits set and system suspend resume works for >> the first time. >> However, after system resume, the core is re-initialized so the 2 SUSPHY bits >> are cleared for both controllers. For host controller it is never set again. >> For gadget controller as gadget start is called, the 2 SUSPHY bits are set >> again. The second system suspend resume will still fail as one controller >> (Host) doesn't have the 2 SUSPHY bits set. >> >> To summarize, the existing solution is not sufficient for us to have a >> reliable behavior. We need the 2 SUSPHY bits to be set regardless of what >> role we are in or whether the role has started or not. >> >> My suggestion is to move back the SUSPHY enable to end of dwc3_core_init(). >> Then if SUSPHY needs to be disabled for DRD role switching, it should be >> disabled and enabled exactly there. >> >> What do you suggest? >> >> [1] - https://urldefense.com/v3/__https://lore.kernel.org/linux-arm-kernel/20240904194229.109886-1-msp@baylibre.com/__;!!A4F2R9G_pg!Y10q3gwCzryOoiXpk6DMGn74iFQIg6GloY10J16kWCbqwgS1Algo5HRg05vm38dMw8n47qmKpqJlyXt9Kqlm$ >> > > Thanks for reporting the issue. > > This is quite an interesting behavior. As you said, we will need to > isolate this change to only during DRD role switch. > > We may not necessarily just enable at the end of dwc3_core_init() since > that would keep the SUSPHY bits on during the DRD role switch. If this > issue only occurs before suspend, perhaps we can check and set these > bits during suspend or dwc3_core_exit() instead? dwc3_core_exit() is not always called in the system suspend path so it may not be sufficient. Any issues if we set this these bits at the end of dwc3_suspend_common() irrespective of runtime suspend or system suspend and operating role? And should we restore these bits in dwc3_resume_common() to the state they were before dwc3_suspend_common()?
On Fri, Sep 27, 2024, Roger Quadros wrote: > > > On 27/09/2024 00:51, Thinh Nguyen wrote: > > Hi Roger, > > > > On Wed, Sep 25, 2024, Roger Quadros wrote: > >> Hello Thinh, > >> > >> On 17/04/2024 02:41, Thinh Nguyen wrote: > >>> GUSB3PIPECTL.SUSPENDENABLE and GUSB2PHYCFG.SUSPHY should be cleared > >>> during initialization. Suspend during initialization can result in > >>> undefined behavior due to clock synchronization failure, which often > >>> seen as core soft reset timeout. > >>> > >>> The programming guide recommended these bits to be cleared during > >>> initialization for DWC_usb3.0 version 1.94 and above (along with > >>> DWC_usb31 and DWC_usb32). The current check in the driver does not > >>> account if it's set by default setting from coreConsultant. > >>> > >>> This is especially the case for DRD when switching mode to ensure the > >>> phy clocks are available to change mode. Depending on the > >>> platforms/design, some may be affected more than others. This is noted > >>> in the DWC_usb3x programming guide under the above registers. > >>> > >>> Let's just disable them during driver load and mode switching. Restore > >>> them when the controller initialization completes. > >>> > >>> Note that some platforms workaround this issue by disabling phy suspend > >>> through "snps,dis_u3_susphy_quirk" and "snps,dis_u2_susphy_quirk" when > >>> they should not need to. > >>> > >>> Cc: stable@vger.kernel.org > >>> Fixes: 9ba3aca8fe82 ("usb: dwc3: Disable phy suspend after power-on reset") > >>> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > >> > >> This patch is causing system suspend failures on TI AM62 platforms [1] > >> > >> I will try to explain why. > >> Before this patch, both DWC3_GUSB3PIPECTL_SUSPHY and DWC3_GUSB2PHYCFG_SUSPHY > >> bits (hence forth called 2 SUSPHY bits) were being set during initialization > >> and even during re-initialization after a system suspend/resume. > >> > >> These bits are required to be set for system suspend/resume to work correctly > >> on AM62 platforms. > > > > Is it only for suspend or both suspend and resume? > > I'm sure about suspend. It is not possible to toggle those bits while in system > suspend so we can't really say if it is required exclusively for system resume or not. > > > > >> > >> After this patch, the bits are only set when Host controller starts or > >> when Gadget driver starts. > >> > >> On AM62 platform we have 2 USB controllers, one in Host and one in Dual role. > >> Just after boot, for the Host controller we have the 2 SUSPHY bits set but > >> for the Dual-Role controller, as no role has started the 2 SUSPHY bits are > >> not set. Thus system suspend resume will fail. > >> > >> On the other hand, if we load a gadget driver just after boot then both > >> controllers have the 2 SUSPHY bits set and system suspend resume works for > >> the first time. > >> However, after system resume, the core is re-initialized so the 2 SUSPHY bits > >> are cleared for both controllers. For host controller it is never set again. > >> For gadget controller as gadget start is called, the 2 SUSPHY bits are set > >> again. The second system suspend resume will still fail as one controller > >> (Host) doesn't have the 2 SUSPHY bits set. > >> > >> To summarize, the existing solution is not sufficient for us to have a > >> reliable behavior. We need the 2 SUSPHY bits to be set regardless of what > >> role we are in or whether the role has started or not. > >> > >> My suggestion is to move back the SUSPHY enable to end of dwc3_core_init(). > >> Then if SUSPHY needs to be disabled for DRD role switching, it should be > >> disabled and enabled exactly there. > >> > >> What do you suggest? > >> > >> [1] - https://urldefense.com/v3/__https://lore.kernel.org/linux-arm-kernel/20240904194229.109886-1-msp@baylibre.com/__;!!A4F2R9G_pg!Y10q3gwCzryOoiXpk6DMGn74iFQIg6GloY10J16kWCbqwgS1Algo5HRg05vm38dMw8n47qmKpqJlyXt9Kqlm$ > >> > > > > Thanks for reporting the issue. > > > > This is quite an interesting behavior. As you said, we will need to > > isolate this change to only during DRD role switch. > > > > We may not necessarily just enable at the end of dwc3_core_init() since > > that would keep the SUSPHY bits on during the DRD role switch. If this > > issue only occurs before suspend, perhaps we can check and set these > > bits during suspend or dwc3_core_exit() instead? > > dwc3_core_exit() is not always called in the system suspend path so it > may not be sufficient. > > Any issues if we set this these bits at the end of dwc3_suspend_common() > irrespective of runtime suspend or system suspend and operating role? There should be no issue at this point. The problem occurs during initialization that involves initializing the usb role. > And should we restore these bits in dwc3_resume_common() to the state they > were before dwc3_suspend_common()? > Sounds good to me! Would you mind send a fix patch? Thanks, Thinh
On 01/10/2024 04:00, Thinh Nguyen wrote: > On Fri, Sep 27, 2024, Roger Quadros wrote: >> >> >> On 27/09/2024 00:51, Thinh Nguyen wrote: >>> Hi Roger, >>> >>> On Wed, Sep 25, 2024, Roger Quadros wrote: >>>> Hello Thinh, >>>> >>>> On 17/04/2024 02:41, Thinh Nguyen wrote: >>>>> GUSB3PIPECTL.SUSPENDENABLE and GUSB2PHYCFG.SUSPHY should be cleared >>>>> during initialization. Suspend during initialization can result in >>>>> undefined behavior due to clock synchronization failure, which often >>>>> seen as core soft reset timeout. >>>>> >>>>> The programming guide recommended these bits to be cleared during >>>>> initialization for DWC_usb3.0 version 1.94 and above (along with >>>>> DWC_usb31 and DWC_usb32). The current check in the driver does not >>>>> account if it's set by default setting from coreConsultant. >>>>> >>>>> This is especially the case for DRD when switching mode to ensure the >>>>> phy clocks are available to change mode. Depending on the >>>>> platforms/design, some may be affected more than others. This is noted >>>>> in the DWC_usb3x programming guide under the above registers. >>>>> >>>>> Let's just disable them during driver load and mode switching. Restore >>>>> them when the controller initialization completes. >>>>> >>>>> Note that some platforms workaround this issue by disabling phy suspend >>>>> through "snps,dis_u3_susphy_quirk" and "snps,dis_u2_susphy_quirk" when >>>>> they should not need to. >>>>> >>>>> Cc: stable@vger.kernel.org >>>>> Fixes: 9ba3aca8fe82 ("usb: dwc3: Disable phy suspend after power-on reset") >>>>> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> >>>> >>>> This patch is causing system suspend failures on TI AM62 platforms [1] >>>> >>>> I will try to explain why. >>>> Before this patch, both DWC3_GUSB3PIPECTL_SUSPHY and DWC3_GUSB2PHYCFG_SUSPHY >>>> bits (hence forth called 2 SUSPHY bits) were being set during initialization >>>> and even during re-initialization after a system suspend/resume. >>>> >>>> These bits are required to be set for system suspend/resume to work correctly >>>> on AM62 platforms. >>> >>> Is it only for suspend or both suspend and resume? >> >> I'm sure about suspend. It is not possible to toggle those bits while in system >> suspend so we can't really say if it is required exclusively for system resume or not. >> >>> >>>> >>>> After this patch, the bits are only set when Host controller starts or >>>> when Gadget driver starts. >>>> >>>> On AM62 platform we have 2 USB controllers, one in Host and one in Dual role. >>>> Just after boot, for the Host controller we have the 2 SUSPHY bits set but >>>> for the Dual-Role controller, as no role has started the 2 SUSPHY bits are >>>> not set. Thus system suspend resume will fail. >>>> >>>> On the other hand, if we load a gadget driver just after boot then both >>>> controllers have the 2 SUSPHY bits set and system suspend resume works for >>>> the first time. >>>> However, after system resume, the core is re-initialized so the 2 SUSPHY bits >>>> are cleared for both controllers. For host controller it is never set again. >>>> For gadget controller as gadget start is called, the 2 SUSPHY bits are set >>>> again. The second system suspend resume will still fail as one controller >>>> (Host) doesn't have the 2 SUSPHY bits set. >>>> >>>> To summarize, the existing solution is not sufficient for us to have a >>>> reliable behavior. We need the 2 SUSPHY bits to be set regardless of what >>>> role we are in or whether the role has started or not. >>>> >>>> My suggestion is to move back the SUSPHY enable to end of dwc3_core_init(). >>>> Then if SUSPHY needs to be disabled for DRD role switching, it should be >>>> disabled and enabled exactly there. >>>> >>>> What do you suggest? >>>> >>>> [1] - https://urldefense.com/v3/__https://lore.kernel.org/linux-arm-kernel/20240904194229.109886-1-msp@baylibre.com/__;!!A4F2R9G_pg!Y10q3gwCzryOoiXpk6DMGn74iFQIg6GloY10J16kWCbqwgS1Algo5HRg05vm38dMw8n47qmKpqJlyXt9Kqlm$ >>>> >>> >>> Thanks for reporting the issue. >>> >>> This is quite an interesting behavior. As you said, we will need to >>> isolate this change to only during DRD role switch. >>> >>> We may not necessarily just enable at the end of dwc3_core_init() since >>> that would keep the SUSPHY bits on during the DRD role switch. If this >>> issue only occurs before suspend, perhaps we can check and set these >>> bits during suspend or dwc3_core_exit() instead? >> >> dwc3_core_exit() is not always called in the system suspend path so it >> may not be sufficient. >> >> Any issues if we set this these bits at the end of dwc3_suspend_common() >> irrespective of runtime suspend or system suspend and operating role? > > There should be no issue at this point. The problem occurs during > initialization that involves initializing the usb role. > >> And should we restore these bits in dwc3_resume_common() to the state they >> were before dwc3_suspend_common()? >> > > Sounds good to me! Would you mind send a fix patch? Thanks for your suggestions. Yes, I will send a fix soon.
On Wed, Sep 25, 2024 at 10:50:05AM +0300, Roger Quadros wrote: > Hello Thinh, > > On 17/04/2024 02:41, Thinh Nguyen wrote: > > GUSB3PIPECTL.SUSPENDENABLE and GUSB2PHYCFG.SUSPHY should be cleared > > during initialization. Suspend during initialization can result in > > undefined behavior due to clock synchronization failure, which often > > seen as core soft reset timeout. > > > > The programming guide recommended these bits to be cleared during > > initialization for DWC_usb3.0 version 1.94 and above (along with > > DWC_usb31 and DWC_usb32). The current check in the driver does not > > account if it's set by default setting from coreConsultant. > > > > This is especially the case for DRD when switching mode to ensure the > > phy clocks are available to change mode. Depending on the > > platforms/design, some may be affected more than others. This is noted > > in the DWC_usb3x programming guide under the above registers. > > > > Let's just disable them during driver load and mode switching. Restore > > them when the controller initialization completes. > > > > Note that some platforms workaround this issue by disabling phy suspend > > through "snps,dis_u3_susphy_quirk" and "snps,dis_u2_susphy_quirk" when > > they should not need to. > > > > Cc: stable@vger.kernel.org > > Fixes: 9ba3aca8fe82 ("usb: dwc3: Disable phy suspend after power-on reset") > > Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > > This patch is causing system suspend failures on TI AM62 platforms [1] > > I will try to explain why. > Before this patch, both DWC3_GUSB3PIPECTL_SUSPHY and DWC3_GUSB2PHYCFG_SUSPHY > bits (hence forth called 2 SUSPHY bits) were being set during initialization > and even during re-initialization after a system suspend/resume. > > These bits are required to be set for system suspend/resume to work correctly > on AM62 platforms. > > After this patch, the bits are only set when Host controller starts or > when Gadget driver starts. > > On AM62 platform we have 2 USB controllers, one in Host and one in Dual role. > Just after boot, for the Host controller we have the 2 SUSPHY bits set but > for the Dual-Role controller, as no role has started the 2 SUSPHY bits are > not set. Thus system suspend resume will fail. > > On the other hand, if we load a gadget driver just after boot then both > controllers have the 2 SUSPHY bits set and system suspend resume works for > the first time. > However, after system resume, the core is re-initialized so the 2 SUSPHY bits > are cleared for both controllers. For host controller it is never set again. > For gadget controller as gadget start is called, the 2 SUSPHY bits are set > again. The second system suspend resume will still fail as one controller > (Host) doesn't have the 2 SUSPHY bits set. > > To summarize, the existing solution is not sufficient for us to have a > reliable behavior. We need the 2 SUSPHY bits to be set regardless of what > role we are in or whether the role has started or not. > > My suggestion is to move back the SUSPHY enable to end of dwc3_core_init(). > Then if SUSPHY needs to be disabled for DRD role switching, it should be > disabled and enabled exactly there. > > What do you suggest? > > [1] - https://lore.kernel.org/linux-arm-kernel/20240904194229.109886-1-msp@baylibre.com/ > > -- > cheers, > -roger > > > --- > > drivers/usb/dwc3/core.c | 90 +++++++++++++++++---------------------- > > drivers/usb/dwc3/core.h | 1 + > > drivers/usb/dwc3/gadget.c | 2 + > > drivers/usb/dwc3/host.c | 27 ++++++++++++ > > 4 files changed, 68 insertions(+), 52 deletions(-) > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > index 31684cdaaae3..100041320e8d 100644 > > --- a/drivers/usb/dwc3/core.c > > +++ b/drivers/usb/dwc3/core.c > > @@ -104,6 +104,27 @@ static int dwc3_get_dr_mode(struct dwc3 *dwc) > > return 0; > > } > > > > +void dwc3_enable_susphy(struct dwc3 *dwc, bool enable) > > +{ > > + u32 reg; > > + > > + reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0)); > > + if (enable && !dwc->dis_u3_susphy_quirk) > > + reg |= DWC3_GUSB3PIPECTL_SUSPHY; > > + else > > + reg &= ~DWC3_GUSB3PIPECTL_SUSPHY; > > + > > + dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg); > > + > > + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0)); > > + if (enable && !dwc->dis_u2_susphy_quirk) > > + reg |= DWC3_GUSB2PHYCFG_SUSPHY; > > + else > > + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY; > > + > > + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg); > > +} > > + > > void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode) > > { > > u32 reg; > > @@ -585,11 +606,8 @@ static int dwc3_core_ulpi_init(struct dwc3 *dwc) > > */ > > static int dwc3_phy_setup(struct dwc3 *dwc) > > { > > - unsigned int hw_mode; > > u32 reg; > > > > - hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0); > > - > > reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0)); > > > > /* > > @@ -599,21 +617,16 @@ static int dwc3_phy_setup(struct dwc3 *dwc) > > reg &= ~DWC3_GUSB3PIPECTL_UX_EXIT_PX; > > > > /* > > - * Above 1.94a, it is recommended to set DWC3_GUSB3PIPECTL_SUSPHY > > - * to '0' during coreConsultant configuration. So default value > > - * will be '0' when the core is reset. Application needs to set it > > - * to '1' after the core initialization is completed. > > - */ > > - if (!DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) > > - reg |= DWC3_GUSB3PIPECTL_SUSPHY; > > - > > - /* > > - * For DRD controllers, GUSB3PIPECTL.SUSPENDENABLE must be cleared after > > - * power-on reset, and it can be set after core initialization, which is > > - * after device soft-reset during initialization. > > + * Above DWC_usb3.0 1.94a, it is recommended to set > > + * DWC3_GUSB3PIPECTL_SUSPHY to '0' during coreConsultant configuration. > > + * So default value will be '0' when the core is reset. Application > > + * needs to set it to '1' after the core initialization is completed. > > + * > > + * Similarly for DRD controllers, GUSB3PIPECTL.SUSPENDENABLE must be > > + * cleared after power-on reset, and it can be set after core > > + * initialization. > > */ > > - if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD) > > - reg &= ~DWC3_GUSB3PIPECTL_SUSPHY; > > + reg &= ~DWC3_GUSB3PIPECTL_SUSPHY; > > > > if (dwc->u2ss_inp3_quirk) > > reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK; > > @@ -639,9 +652,6 @@ static int dwc3_phy_setup(struct dwc3 *dwc) > > if (dwc->tx_de_emphasis_quirk) > > reg |= DWC3_GUSB3PIPECTL_TX_DEEPH(dwc->tx_de_emphasis); > > > > - if (dwc->dis_u3_susphy_quirk) > > - reg &= ~DWC3_GUSB3PIPECTL_SUSPHY; > > - > > if (dwc->dis_del_phy_power_chg_quirk) > > reg &= ~DWC3_GUSB3PIPECTL_DEPOCHANGE; > > > > @@ -689,24 +699,15 @@ static int dwc3_phy_setup(struct dwc3 *dwc) > > } > > > > /* > > - * Above 1.94a, it is recommended to set DWC3_GUSB2PHYCFG_SUSPHY to > > - * '0' during coreConsultant configuration. So default value will > > - * be '0' when the core is reset. Application needs to set it to > > - * '1' after the core initialization is completed. > > - */ > > - if (!DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) > > - reg |= DWC3_GUSB2PHYCFG_SUSPHY; > > - > > - /* > > - * For DRD controllers, GUSB2PHYCFG.SUSPHY must be cleared after > > - * power-on reset, and it can be set after core initialization, which is > > - * after device soft-reset during initialization. > > + * Above DWC_usb3.0 1.94a, it is recommended to set > > + * DWC3_GUSB2PHYCFG_SUSPHY to '0' during coreConsultant configuration. > > + * So default value will be '0' when the core is reset. Application > > + * needs to set it to '1' after the core initialization is completed. > > + * > > + * Similarly for DRD controllers, GUSB2PHYCFG.SUSPHY must be cleared > > + * after power-on reset, and it can be set after core initialization. > > */ > > - if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD) > > - reg &= ~DWC3_GUSB2PHYCFG_SUSPHY; > > - > > - if (dwc->dis_u2_susphy_quirk) > > - reg &= ~DWC3_GUSB2PHYCFG_SUSPHY; > > + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY; > > > > if (dwc->dis_enblslpm_quirk) > > reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM; > > @@ -1227,21 +1228,6 @@ static int dwc3_core_init(struct dwc3 *dwc) > > if (ret) > > goto err_exit_phy; > > > > - if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD && > > - !DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) { > > - if (!dwc->dis_u3_susphy_quirk) { > > - reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0)); > > - reg |= DWC3_GUSB3PIPECTL_SUSPHY; > > - dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg); > > - } > > - > > - if (!dwc->dis_u2_susphy_quirk) { > > - reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0)); > > - reg |= DWC3_GUSB2PHYCFG_SUSPHY; > > - dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg); > > - } > > - } > > - > > dwc3_core_setup_global_control(dwc); > > dwc3_core_num_eps(dwc); > > > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > > index 7e80dd3d466b..180dd8d29287 100644 > > --- a/drivers/usb/dwc3/core.h > > +++ b/drivers/usb/dwc3/core.h > > @@ -1580,6 +1580,7 @@ int dwc3_event_buffers_setup(struct dwc3 *dwc); > > void dwc3_event_buffers_cleanup(struct dwc3 *dwc); > > > > int dwc3_core_soft_reset(struct dwc3 *dwc); > > +void dwc3_enable_susphy(struct dwc3 *dwc, bool enable); > > > > #if IS_ENABLED(CONFIG_USB_DWC3_HOST) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE) > > int dwc3_host_init(struct dwc3 *dwc); > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > > index 4df2661f6675..f94f68f1e7d2 100644 > > --- a/drivers/usb/dwc3/gadget.c > > +++ b/drivers/usb/dwc3/gadget.c > > @@ -2924,6 +2924,7 @@ static int __dwc3_gadget_start(struct dwc3 *dwc) > > dwc3_ep0_out_start(dwc); > > > > dwc3_gadget_enable_irq(dwc); > > + dwc3_enable_susphy(dwc, true); > > > > return 0; > > > > @@ -4690,6 +4691,7 @@ void dwc3_gadget_exit(struct dwc3 *dwc) > > if (!dwc->gadget) > > return; > > > > + dwc3_enable_susphy(dwc, false); > > usb_del_gadget(dwc->gadget); > > dwc3_gadget_free_endpoints(dwc); > > usb_put_gadget(dwc->gadget); > > diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c > > index 0204787df81d..a171b27a7845 100644 > > --- a/drivers/usb/dwc3/host.c > > +++ b/drivers/usb/dwc3/host.c > > @@ -10,10 +10,13 @@ > > #include <linux/irq.h> > > #include <linux/of.h> > > #include <linux/platform_device.h> > > +#include <linux/usb.h> > > +#include <linux/usb/hcd.h> > > > > #include "../host/xhci-port.h" > > #include "../host/xhci-ext-caps.h" > > #include "../host/xhci-caps.h" > > +#include "../host/xhci-plat.h" > > #include "core.h" > > > > #define XHCI_HCSPARAMS1 0x4 > > @@ -57,6 +60,24 @@ static void dwc3_power_off_all_roothub_ports(struct dwc3 *dwc) > > } > > } > > > > +static void dwc3_xhci_plat_start(struct usb_hcd *hcd) > > +{ > > + struct platform_device *pdev; > > + struct dwc3 *dwc; > > + > > + if (!usb_hcd_is_primary_hcd(hcd)) > > + return; > > + > > + pdev = to_platform_device(hcd->self.controller); > > + dwc = dev_get_drvdata(pdev->dev.parent); > > + > > + dwc3_enable_susphy(dwc, true); > > +} > > + > > +static const struct xhci_plat_priv dwc3_xhci_plat_quirk = { > > + .plat_start = dwc3_xhci_plat_start, > > +}; > > + > > static void dwc3_host_fill_xhci_irq_res(struct dwc3 *dwc, > > int irq, char *name) > > { > > @@ -167,6 +188,11 @@ int dwc3_host_init(struct dwc3 *dwc) > > } > > } > > > > + ret = platform_device_add_data(xhci, &dwc3_xhci_plat_quirk, > > + sizeof(struct xhci_plat_priv)); > > + if (ret) > > + goto err; > > + > > ret = platform_device_add(xhci); > > if (ret) { > > dev_err(dwc->dev, "failed to register xHCI device\n"); > > @@ -192,6 +218,7 @@ void dwc3_host_exit(struct dwc3 *dwc) > > if (dwc->sys_wakeup) > > device_init_wakeup(&dwc->xhci->dev, false); > > > > + dwc3_enable_susphy(dwc, false); > > platform_device_unregister(dwc->xhci); > > dwc->xhci = NULL; > > } > This patch seems to break system suspend on at least the Rockchip RK3566 platform. I noticed that I was no longer able to suspend and git bisect led me to this patch. My kernel message log shows the following, at which point it freezes and does not allow me to resume from suspend: [ 27.235049] PM: suspend entry (deep) [ 27.871641] Filesystems sync: 0.636 seconds [ 27.885320] Freezing user space processes [ 27.886932] Freezing user space processes completed (elapsed 0.001 seconds) [ 27.887642] OOM killer disabled. [ 27.887981] Freezing remaining freezable tasks [ 27.889655] Freezing remaining freezable tasks completed (elapsed 0.001 seconds) Thank you, Chris
Hi, On Fri, Oct 25, 2024, Chris Morgan wrote: > > This patch seems to break system suspend on at least the Rockchip > RK3566 platform. I noticed that I was no longer able to suspend > and git bisect led me to this patch. > > My kernel message log shows the following, at which point it freezes > and does not allow me to resume from suspend: > > [ 27.235049] PM: suspend entry (deep) > [ 27.871641] Filesystems sync: 0.636 seconds > [ 27.885320] Freezing user space processes > [ 27.886932] Freezing user space processes completed (elapsed 0.001 seconds) > [ 27.887642] OOM killer disabled. > [ 27.887981] Freezing remaining freezable tasks > [ 27.889655] Freezing remaining freezable tasks completed (elapsed 0.001 seconds) > > Thank you, > Chris Did you try out Roger's fix? 705e3ce37bcc ("usb: dwc3: core: Fix system suspend on TI AM62 platforms") https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=705e3ce37bccdf2ed6f848356ff355f480d51a91 BR, Thinh
Hi, On Tue, Oct 29, 2024, Chris Morgan wrote: > Sorry, to be specific it was the fix that causes the issues I'm now > observing. When I explicitly revert commit > 705e3ce37bccdf2ed6f848356ff355f480d51a91 things start working again > for me. With that commit in place, however, suspend fails for me. Ok, Roger's patch is causing issue on your platform and the $subject patch? Can you provide more details on your test sequence? * What does "no longer able to suspend" mean exactly (what error?) * What mode is your usb controller? * Is there any device connected while going into suspend? * Can you provide dwc3 regdump? Thanks, Thinh > > On Fri, Oct 25, 2024 at 5:40 PM Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: > > > > Hi, > > > > On Fri, Oct 25, 2024, Chris Morgan wrote: > > > > > > This patch seems to break system suspend on at least the Rockchip > > > RK3566 platform. I noticed that I was no longer able to suspend > > > and git bisect led me to this patch. > > > > > > My kernel message log shows the following, at which point it freezes > > > and does not allow me to resume from suspend: > > > > > > [ 27.235049] PM: suspend entry (deep) > > > [ 27.871641] Filesystems sync: 0.636 seconds > > > [ 27.885320] Freezing user space processes > > > [ 27.886932] Freezing user space processes completed (elapsed 0.001 seconds) > > > [ 27.887642] OOM killer disabled. > > > [ 27.887981] Freezing remaining freezable tasks > > > [ 27.889655] Freezing remaining freezable tasks completed (elapsed 0.001 seconds) > > > > > > Thank you, > > > Chris > > > > Did you try out Roger's fix? > > 705e3ce37bcc ("usb: dwc3: core: Fix system suspend on TI AM62 platforms") > > > > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=705e3ce37bccdf2ed6f848356ff355f480d51a91__;!!A4F2R9G_pg!ZXQdR2uLykSD67_3JSm0RZHuyJ7IVnw5EvmYvLnPsf3dDEilv5ZgHD9GX7gZr52t0H7oFKifzAEhbdK8EGYzmSji2UI$ > > > > BR, > > Thinh
Hi Chris, On 30/10/2024 00:49, Thinh Nguyen wrote: > Hi, > > On Tue, Oct 29, 2024, Chris Morgan wrote: >> Sorry, to be specific it was the fix that causes the issues I'm now >> observing. When I explicitly revert commit >> 705e3ce37bccdf2ed6f848356ff355f480d51a91 things start working again >> for me. With that commit in place, however, suspend fails for me. > > Ok, Roger's patch is causing issue on your platform and the $subject > patch? Can you provide more details on your test sequence? > > * What does "no longer able to suspend" mean exactly (what error?) > * What mode is your usb controller? > * Is there any device connected while going into suspend? > * Can you provide dwc3 regdump? Commit 705e3ce37bccdf2ed6f848356ff355f480d51a91 will enable DWC3_GUSB2PHYCFG_SUSPHY in DWC3_GUSB2PHYCFG(i) and DWC3_GUSB3PIPECTL_SUSPHY in DWC3_GUSB3PIPECTL(i) during system suspend path, unless snps,dis_u2_susphy_quirk or snps,dis_u3_susphy_quirk is set. I see rK356x.dtsi has snps,dis_u2_susphy_quirk; Does adding snps,dis_u3_susphy_quirk resolve the issue? cheers, -roger > > Thanks, > Thinh > >> >> On Fri, Oct 25, 2024 at 5:40 PM Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: >>> >>> Hi, >>> >>> On Fri, Oct 25, 2024, Chris Morgan wrote: >>>> >>>> This patch seems to break system suspend on at least the Rockchip >>>> RK3566 platform. I noticed that I was no longer able to suspend >>>> and git bisect led me to this patch. >>>> >>>> My kernel message log shows the following, at which point it freezes >>>> and does not allow me to resume from suspend: >>>> >>>> [ 27.235049] PM: suspend entry (deep) >>>> [ 27.871641] Filesystems sync: 0.636 seconds >>>> [ 27.885320] Freezing user space processes >>>> [ 27.886932] Freezing user space processes completed (elapsed 0.001 seconds) >>>> [ 27.887642] OOM killer disabled. >>>> [ 27.887981] Freezing remaining freezable tasks >>>> [ 27.889655] Freezing remaining freezable tasks completed (elapsed 0.001 seconds) >>>> >>>> Thank you, >>>> Chris >>> >>> Did you try out Roger's fix? >>> 705e3ce37bcc ("usb: dwc3: core: Fix system suspend on TI AM62 platforms") >>> >>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=705e3ce37bccdf2ed6f848356ff355f480d51a91__;!!A4F2R9G_pg!ZXQdR2uLykSD67_3JSm0RZHuyJ7IVnw5EvmYvLnPsf3dDEilv5ZgHD9GX7gZr52t0H7oFKifzAEhbdK8EGYzmSji2UI$ >>> >>> BR, >>> Thinh
On Wed, Oct 30, 2024 at 03:10:34PM +0200, Roger Quadros wrote: > Hi Chris, > > On 30/10/2024 00:49, Thinh Nguyen wrote: > > Hi, > > > > On Tue, Oct 29, 2024, Chris Morgan wrote: > >> Sorry, to be specific it was the fix that causes the issues I'm now > >> observing. When I explicitly revert commit > >> 705e3ce37bccdf2ed6f848356ff355f480d51a91 things start working again > >> for me. With that commit in place, however, suspend fails for me. > > > > Ok, Roger's patch is causing issue on your platform and the $subject > > patch? Can you provide more details on your test sequence? > > > > * What does "no longer able to suspend" mean exactly (what error?) > > * What mode is your usb controller? > > * Is there any device connected while going into suspend? > > * Can you provide dwc3 regdump? > > Commit 705e3ce37bccdf2ed6f848356ff355f480d51a91 will enable > DWC3_GUSB2PHYCFG_SUSPHY in DWC3_GUSB2PHYCFG(i) > and DWC3_GUSB3PIPECTL_SUSPHY in DWC3_GUSB3PIPECTL(i) during > system suspend path, unless snps,dis_u2_susphy_quirk or > snps,dis_u3_susphy_quirk is set. > > I see rK356x.dtsi has snps,dis_u2_susphy_quirk; > Does adding snps,dis_u3_susphy_quirk resolve the issue? I'm afraid it does not fix the issue. Specifically when I do "systemctl suspend" the device begins to suspend but freezes with the kernel log output via serial console listed previously. Note I have console enabled in suspend. Additionally button input no longer works at this point. Specifically, I'm testing this with the Anbernic RG353P device based on the Rockchip RK3566 SoC, in case you're curious. I'm not able to get you a register dump post suspend attempt as the system completely freezes, however I can get you a dump prior to suspend if that will help? Thank you, Chris > > cheers, > -roger > > > > Thanks, > > Thinh > > > >> > >> On Fri, Oct 25, 2024 at 5:40 PM Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: > >>> > >>> Hi, > >>> > >>> On Fri, Oct 25, 2024, Chris Morgan wrote: > >>>> > >>>> This patch seems to break system suspend on at least the Rockchip > >>>> RK3566 platform. I noticed that I was no longer able to suspend > >>>> and git bisect led me to this patch. > >>>> > >>>> My kernel message log shows the following, at which point it freezes > >>>> and does not allow me to resume from suspend: > >>>> > >>>> [ 27.235049] PM: suspend entry (deep) > >>>> [ 27.871641] Filesystems sync: 0.636 seconds > >>>> [ 27.885320] Freezing user space processes > >>>> [ 27.886932] Freezing user space processes completed (elapsed 0.001 seconds) > >>>> [ 27.887642] OOM killer disabled. > >>>> [ 27.887981] Freezing remaining freezable tasks > >>>> [ 27.889655] Freezing remaining freezable tasks completed (elapsed 0.001 seconds) > >>>> > >>>> Thank you, > >>>> Chris > >>> > >>> Did you try out Roger's fix? > >>> 705e3ce37bcc ("usb: dwc3: core: Fix system suspend on TI AM62 platforms") > >>> > >>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=705e3ce37bccdf2ed6f848356ff355f480d51a91__;!!A4F2R9G_pg!ZXQdR2uLykSD67_3JSm0RZHuyJ7IVnw5EvmYvLnPsf3dDEilv5ZgHD9GX7gZr52t0H7oFKifzAEhbdK8EGYzmSji2UI$ > >>> > >>> BR, > >>> Thinh >
On Wed, Oct 30, 2024, Chris Morgan wrote: > On Wed, Oct 30, 2024 at 03:10:34PM +0200, Roger Quadros wrote: > > Hi Chris, > > > > On 30/10/2024 00:49, Thinh Nguyen wrote: > > > Hi, > > > > > > On Tue, Oct 29, 2024, Chris Morgan wrote: > > >> Sorry, to be specific it was the fix that causes the issues I'm now > > >> observing. When I explicitly revert commit > > >> 705e3ce37bccdf2ed6f848356ff355f480d51a91 things start working again > > >> for me. With that commit in place, however, suspend fails for me. > > > > > > Ok, Roger's patch is causing issue on your platform and the $subject > > > patch? Can you provide more details on your test sequence? > > > > > > * What does "no longer able to suspend" mean exactly (what error?) > > > * What mode is your usb controller? > > > * Is there any device connected while going into suspend? > > > * Can you provide dwc3 regdump? > > > > Commit 705e3ce37bccdf2ed6f848356ff355f480d51a91 will enable > > DWC3_GUSB2PHYCFG_SUSPHY in DWC3_GUSB2PHYCFG(i) > > and DWC3_GUSB3PIPECTL_SUSPHY in DWC3_GUSB3PIPECTL(i) during > > system suspend path, unless snps,dis_u2_susphy_quirk or > > snps,dis_u3_susphy_quirk is set. > > > > I see rK356x.dtsi has snps,dis_u2_susphy_quirk; > > Does adding snps,dis_u3_susphy_quirk resolve the issue? > > I'm afraid it does not fix the issue. Specifically when I do > "systemctl suspend" the device begins to suspend but freezes with the > kernel log output via serial console listed previously. Note I have > console enabled in suspend. Additionally button input no longer > works at this point. > > Specifically, I'm testing this with the Anbernic RG353P device based on > the Rockchip RK3566 SoC, in case you're curious. > > I'm not able to get you a register dump post suspend attempt as the > system completely freezes, however I can get you a dump prior to > suspend if that will help? Yes, any data is useful. > > Thank you, > Chris Can you help answer the other bullet questions I have previously. Thanks, Thinh
On Thu, Oct 31, 2024 at 01:33:54AM +0000, Thinh Nguyen wrote: > On Wed, Oct 30, 2024, Chris Morgan wrote: > > On Wed, Oct 30, 2024 at 03:10:34PM +0200, Roger Quadros wrote: > > > Hi Chris, > > > > > > On 30/10/2024 00:49, Thinh Nguyen wrote: > > > > Hi, > > > > > > > > On Tue, Oct 29, 2024, Chris Morgan wrote: > > > >> Sorry, to be specific it was the fix that causes the issues I'm now > > > >> observing. When I explicitly revert commit > > > >> 705e3ce37bccdf2ed6f848356ff355f480d51a91 things start working again > > > >> for me. With that commit in place, however, suspend fails for me. > > > > > > > > Ok, Roger's patch is causing issue on your platform and the $subject > > > > patch? Can you provide more details on your test sequence? > > > > > > > > * What does "no longer able to suspend" mean exactly (what error?) > > > > * What mode is your usb controller? > > > > * Is there any device connected while going into suspend? > > > > * Can you provide dwc3 regdump? > > > > > > Commit 705e3ce37bccdf2ed6f848356ff355f480d51a91 will enable > > > DWC3_GUSB2PHYCFG_SUSPHY in DWC3_GUSB2PHYCFG(i) > > > and DWC3_GUSB3PIPECTL_SUSPHY in DWC3_GUSB3PIPECTL(i) during > > > system suspend path, unless snps,dis_u2_susphy_quirk or > > > snps,dis_u3_susphy_quirk is set. > > > > > > I see rK356x.dtsi has snps,dis_u2_susphy_quirk; > > > Does adding snps,dis_u3_susphy_quirk resolve the issue? > > > > I'm afraid it does not fix the issue. Specifically when I do > > "systemctl suspend" the device begins to suspend but freezes with the > > kernel log output via serial console listed previously. Note I have > > console enabled in suspend. Additionally button input no longer > > works at this point. > > > > Specifically, I'm testing this with the Anbernic RG353P device based on > > the Rockchip RK3566 SoC, in case you're curious. > > > > I'm not able to get you a register dump post suspend attempt as the > > system completely freezes, however I can get you a dump prior to > > suspend if that will help? > > Yes, any data is useful. > > > > > Thank you, > > Chris > > Can you help answer the other bullet questions I have previously. > > Thanks, > Thinh I have 2 ports, here is a dump of each: usb@fcc00000: GSBUSCFG0 = 0x00000001 GSBUSCFG1 = 0x00000300 GTXTHRCFG = 0x00000000 GRXTHRCFG = 0x00000000 GCTL = 0x30c12004 GEVTEN = 0x00000000 GSTS = 0x7e800000 GUCTL1 = 0x8504018a GSNPSID = 0x5533300a GGPIO = 0x00000000 GUID = 0x00060c00 GUCTL = 0x0a400010 GBUSERRADDR0 = 0x00000000 GBUSERRADDR1 = 0x00000000 GPRTBIMAP0 = 0x00000000 GPRTBIMAP1 = 0x00000000 GHWPARAMS0 = 0x2020400a GHWPARAMS1 = 0x0160c93b GHWPARAMS2 = 0x20180101 GHWPARAMS3 = 0x08290085 GHWPARAMS4 = 0x47822010 GHWPARAMS5 = 0x04204108 GHWPARAMS6 = 0x075e8020 GHWPARAMS7 = 0x03880800 GDBGFIFOSPACE = 0x00420000 GDBGLTSSM = 0x01090440 GDBGBMU = 0x00000000 GPRTBIMAP_HS0 = 0x00000000 GPRTBIMAP_HS1 = 0x00000000 GPRTBIMAP_FS0 = 0x00000000 GPRTBIMAP_FS1 = 0x00000000 GUCTL2 = 0x0000040d VER_NUMBER = 0x00000000 VER_TYPE = 0x00000000 GUSB2PHYCFG(0) = 0x40101508 GUSB2PHYCFG(1) = 0x00000000 GUSB2PHYCFG(2) = 0x00000000 GUSB2PHYCFG(3) = 0x00000000 GUSB2PHYCFG(4) = 0x00000000 GUSB2PHYCFG(5) = 0x00000000 GUSB2PHYCFG(6) = 0x00000000 GUSB2PHYCFG(7) = 0x00000000 GUSB2PHYCFG(8) = 0x00000000 GUSB2PHYCFG(9) = 0x00000000 GUSB2PHYCFG(10) = 0x00000000 GUSB2PHYCFG(11) = 0x00000000 GUSB2PHYCFG(12) = 0x00000000 GUSB2PHYCFG(13) = 0x00000000 GUSB2PHYCFG(14) = 0x00000000 GUSB2PHYCFG(15) = 0x00000000 GUSB2I2CCTL(0) = 0x00000000 GUSB2I2CCTL(1) = 0x00000000 GUSB2I2CCTL(2) = 0x00000000 GUSB2I2CCTL(3) = 0x00000000 GUSB2I2CCTL(4) = 0x00000000 GUSB2I2CCTL(5) = 0x00000000 GUSB2I2CCTL(6) = 0x00000000 GUSB2I2CCTL(7) = 0x00000000 GUSB2I2CCTL(8) = 0x00000000 GUSB2I2CCTL(9) = 0x00000000 GUSB2I2CCTL(10) = 0x00000000 GUSB2I2CCTL(11) = 0x00000000 GUSB2I2CCTL(12) = 0x00000000 GUSB2I2CCTL(13) = 0x00000000 GUSB2I2CCTL(14) = 0x00000000 GUSB2I2CCTL(15) = 0x00000000 GUSB2PHYACC(0) = 0x00000000 GUSB2PHYACC(1) = 0x00000000 GUSB2PHYACC(2) = 0x00000000 GUSB2PHYACC(3) = 0x00000000 GUSB2PHYACC(4) = 0x00000000 GUSB2PHYACC(5) = 0x00000000 GUSB2PHYACC(6) = 0x00000000 GUSB2PHYACC(7) = 0x00000000 GUSB2PHYACC(8) = 0x00000000 GUSB2PHYACC(9) = 0x00000000 GUSB2PHYACC(10) = 0x00000000 GUSB2PHYACC(11) = 0x00000000 GUSB2PHYACC(12) = 0x00000000 GUSB2PHYACC(13) = 0x00000000 GUSB2PHYACC(14) = 0x00000000 GUSB2PHYACC(15) = 0x00000000 GUSB3PIPECTL(0) = 0x010c0002 GUSB3PIPECTL(1) = 0x00000000 GUSB3PIPECTL(2) = 0x00000000 GUSB3PIPECTL(3) = 0x00000000 GUSB3PIPECTL(4) = 0x00000000 GUSB3PIPECTL(5) = 0x00000000 GUSB3PIPECTL(6) = 0x00000000 GUSB3PIPECTL(7) = 0x00000000 GUSB3PIPECTL(8) = 0x00000000 GUSB3PIPECTL(9) = 0x00000000 GUSB3PIPECTL(10) = 0x00000000 GUSB3PIPECTL(11) = 0x00000000 GUSB3PIPECTL(12) = 0x00000000 GUSB3PIPECTL(13) = 0x00000000 GUSB3PIPECTL(14) = 0x00000000 GUSB3PIPECTL(15) = 0x00000000 GTXFIFOSIZ(0) = 0x00000042 GTXFIFOSIZ(1) = 0x00420184 GTXFIFOSIZ(2) = 0x01c60184 GTXFIFOSIZ(3) = 0x034a0184 GTXFIFOSIZ(4) = 0x04ce0184 GTXFIFOSIZ(5) = 0x06520184 GTXFIFOSIZ(6) = 0x07d6002a GTXFIFOSIZ(7) = 0x08000000 GTXFIFOSIZ(8) = 0x08000000 GTXFIFOSIZ(9) = 0x08000000 GTXFIFOSIZ(10) = 0x00000000 GTXFIFOSIZ(11) = 0x00000000 GTXFIFOSIZ(12) = 0x00000000 GTXFIFOSIZ(13) = 0x00000000 GTXFIFOSIZ(14) = 0x00000000 GTXFIFOSIZ(15) = 0x00000000 GTXFIFOSIZ(16) = 0x00000000 GTXFIFOSIZ(17) = 0x00000000 GTXFIFOSIZ(18) = 0x00000000 GTXFIFOSIZ(19) = 0x00000000 GTXFIFOSIZ(20) = 0x00000000 GTXFIFOSIZ(21) = 0x00000000 GTXFIFOSIZ(22) = 0x00000000 GTXFIFOSIZ(23) = 0x00000000 GTXFIFOSIZ(24) = 0x00000000 GTXFIFOSIZ(25) = 0x00000000 GTXFIFOSIZ(26) = 0x00000000 GTXFIFOSIZ(27) = 0x00000000 GTXFIFOSIZ(28) = 0x00000000 GTXFIFOSIZ(29) = 0x00000000 GTXFIFOSIZ(30) = 0x00000000 GTXFIFOSIZ(31) = 0x00000000 GRXFIFOSIZ(0) = 0x00000388 GRXFIFOSIZ(1) = 0x03880000 GRXFIFOSIZ(2) = 0x03880000 GRXFIFOSIZ(3) = 0x00000000 GRXFIFOSIZ(4) = 0x00000000 GRXFIFOSIZ(5) = 0x00000000 GRXFIFOSIZ(6) = 0x00000000 GRXFIFOSIZ(7) = 0x00000000 GRXFIFOSIZ(8) = 0x00000000 GRXFIFOSIZ(9) = 0x00000000 GRXFIFOSIZ(10) = 0x00000000 GRXFIFOSIZ(11) = 0x00000000 GRXFIFOSIZ(12) = 0x00000000 GRXFIFOSIZ(13) = 0x00000000 GRXFIFOSIZ(14) = 0x00000000 GRXFIFOSIZ(15) = 0x00000000 GRXFIFOSIZ(16) = 0x00000000 GRXFIFOSIZ(17) = 0x00000000 GRXFIFOSIZ(18) = 0x00000000 GRXFIFOSIZ(19) = 0x00000000 GRXFIFOSIZ(20) = 0x00000000 GRXFIFOSIZ(21) = 0x00000000 GRXFIFOSIZ(22) = 0x00000000 GRXFIFOSIZ(23) = 0x00000000 GRXFIFOSIZ(24) = 0x00000000 GRXFIFOSIZ(25) = 0x00000000 GRXFIFOSIZ(26) = 0x00000000 GRXFIFOSIZ(27) = 0x00000000 GRXFIFOSIZ(28) = 0x00000000 GRXFIFOSIZ(29) = 0x00000000 GRXFIFOSIZ(30) = 0x00000000 GRXFIFOSIZ(31) = 0x00000000 GEVNTADRLO(0) = 0x01d8d000 GEVNTADRHI(0) = 0x00000000 GEVNTSIZ(0) = 0x00001000 GEVNTCOUNT(0) = 0x00000000 GHWPARAMS8 = 0x0000075e GUCTL3 = 0x00000000 GFLADJ = 0x0a07f000 DCFG = 0x00080804 DCTL = 0x00f00000 DEVTEN = 0x00000000 DSTS = 0x00d26c4c DGCMDPAR = 0x00000000 DGCMD = 0x00000000 DALEPENA = 0x00000000 DEPCMDPAR2(0) = 0x00000000 DEPCMDPAR1(0) = 0x00000000 DEPCMDPAR0(0) = 0x00000000 DEPCMD(0) = 0x00000000 DEPCMDPAR2(1) = 0x00000000 DEPCMDPAR1(1) = 0x00000000 DEPCMDPAR0(1) = 0x00000000 DEPCMD(1) = 0x00000000 DEPCMDPAR2(2) = 0x00000000 DEPCMDPAR1(2) = 0x00000000 DEPCMDPAR0(2) = 0x00000000 DEPCMD(2) = 0x00000000 DEPCMDPAR2(3) = 0x00000000 DEPCMDPAR1(3) = 0x00000000 DEPCMDPAR0(3) = 0x00000000 DEPCMD(3) = 0x00000000 DEPCMDPAR2(4) = 0x00000000 DEPCMDPAR1(4) = 0x00000000 DEPCMDPAR0(4) = 0x00000000 DEPCMD(4) = 0x00000000 DEPCMDPAR2(5) = 0x00000000 DEPCMDPAR1(5) = 0x00000000 DEPCMDPAR0(5) = 0x00000000 DEPCMD(5) = 0x00000000 DEPCMDPAR2(6) = 0x00000000 DEPCMDPAR1(6) = 0x00000000 DEPCMDPAR0(6) = 0x00000000 DEPCMD(6) = 0x00000000 DEPCMDPAR2(7) = 0x00000000 DEPCMDPAR1(7) = 0x00000000 DEPCMDPAR0(7) = 0x00000000 DEPCMD(7) = 0x00000000 DEPCMDPAR2(8) = 0x00000000 DEPCMDPAR1(8) = 0x00000000 DEPCMDPAR0(8) = 0x00000000 DEPCMD(8) = 0x00000000 DEPCMDPAR2(9) = 0x00000000 DEPCMDPAR1(9) = 0x00000000 DEPCMDPAR0(9) = 0x00000000 DEPCMD(9) = 0x00000000 DEPCMDPAR2(10) = 0x00000000 DEPCMDPAR1(10) = 0x00000000 DEPCMDPAR0(10) = 0x00000000 DEPCMD(10) = 0x00000000 DEPCMDPAR2(11) = 0x00000000 DEPCMDPAR1(11) = 0x00000000 DEPCMDPAR0(11) = 0x00000000 DEPCMD(11) = 0x00000000 DEPCMDPAR2(12) = 0x00000000 DEPCMDPAR1(12) = 0x00000000 DEPCMDPAR0(12) = 0x00000000 DEPCMD(12) = 0x00000000 DEPCMDPAR2(13) = 0x00000000 DEPCMDPAR1(13) = 0x00000000 DEPCMDPAR0(13) = 0x00000000 DEPCMD(13) = 0x00000000 DEPCMDPAR2(14) = 0x00000000 DEPCMDPAR1(14) = 0x00000000 DEPCMDPAR0(14) = 0x00000000 DEPCMD(14) = 0x00000000 DEPCMDPAR2(15) = 0x00000000 DEPCMDPAR1(15) = 0x00000000 DEPCMDPAR0(15) = 0x00000000 DEPCMD(15) = 0x00000000 DEPCMDPAR2(16) = 0x00000000 DEPCMDPAR1(16) = 0x00000000 DEPCMDPAR0(16) = 0x00000000 DEPCMD(16) = 0x00000000 DEPCMDPAR2(17) = 0x00000000 DEPCMDPAR1(17) = 0x00000000 DEPCMDPAR0(17) = 0x00000000 DEPCMD(17) = 0x00000000 DEPCMDPAR2(18) = 0x00000000 DEPCMDPAR1(18) = 0x00000000 DEPCMDPAR0(18) = 0x00000000 DEPCMD(18) = 0x00000000 DEPCMDPAR2(19) = 0x00000000 DEPCMDPAR1(19) = 0x00000000 DEPCMDPAR0(19) = 0x00000000 DEPCMD(19) = 0x00000000 DEPCMDPAR2(20) = 0x00000000 DEPCMDPAR1(20) = 0x00000000 DEPCMDPAR0(20) = 0x00000000 DEPCMD(20) = 0x00000000 DEPCMDPAR2(21) = 0x00000000 DEPCMDPAR1(21) = 0x00000000 DEPCMDPAR0(21) = 0x00000000 DEPCMD(21) = 0x00000000 DEPCMDPAR2(22) = 0x00000000 DEPCMDPAR1(22) = 0x00000000 DEPCMDPAR0(22) = 0x00000000 DEPCMD(22) = 0x00000000 DEPCMDPAR2(23) = 0x00000000 DEPCMDPAR1(23) = 0x00000000 DEPCMDPAR0(23) = 0x00000000 DEPCMD(23) = 0x00000000 DEPCMDPAR2(24) = 0x00000000 DEPCMDPAR1(24) = 0x00000000 DEPCMDPAR0(24) = 0x00000000 DEPCMD(24) = 0x00000000 DEPCMDPAR2(25) = 0x00000000 DEPCMDPAR1(25) = 0x00000000 DEPCMDPAR0(25) = 0x00000000 DEPCMD(25) = 0x00000000 DEPCMDPAR2(26) = 0x00000000 DEPCMDPAR1(26) = 0x00000000 DEPCMDPAR0(26) = 0x00000000 DEPCMD(26) = 0x00000000 DEPCMDPAR2(27) = 0x00000000 DEPCMDPAR1(27) = 0x00000000 DEPCMDPAR0(27) = 0x00000000 DEPCMD(27) = 0x00000000 DEPCMDPAR2(28) = 0x00000000 DEPCMDPAR1(28) = 0x00000000 DEPCMDPAR0(28) = 0x00000000 DEPCMD(28) = 0x00000000 DEPCMDPAR2(29) = 0x00000000 DEPCMDPAR1(29) = 0x00000000 DEPCMDPAR0(29) = 0x00000000 DEPCMD(29) = 0x00000000 DEPCMDPAR2(30) = 0x00000000 DEPCMDPAR1(30) = 0x00000000 DEPCMDPAR0(30) = 0x00000000 DEPCMD(30) = 0x00000000 DEPCMDPAR2(31) = 0x00000000 DEPCMDPAR1(31) = 0x00000000 DEPCMDPAR0(31) = 0x00000000 DEPCMD(31) = 0x00000000 OCFG = 0x00000000 OCTL = 0x00000000 OEVT = 0x00000000 OEVTEN = 0x00000000 OSTS = 0x00000000 fd000000: GSBUSCFG0 = 0x00000001 GSBUSCFG1 = 0x00000300 GTXTHRCFG = 0x00000000 GRXTHRCFG = 0x00000000 GCTL = 0x30c11004 GEVTEN = 0x00000000 GSTS = 0x7e800021 GUCTL1 = 0x8104018a GSNPSID = 0x5533300a GGPIO = 0x00000000 GUID = 0x00060c00 GUCTL = 0x0a400010 GBUSERRADDR0 = 0x00000000 GBUSERRADDR1 = 0x00000000 GPRTBIMAP0 = 0x00000000 GPRTBIMAP1 = 0x00000000 GHWPARAMS0 = 0x2020400a GHWPARAMS1 = 0x0160c93b GHWPARAMS2 = 0x20180101 GHWPARAMS3 = 0x08290085 GHWPARAMS4 = 0x47822010 GHWPARAMS5 = 0x04204108 GHWPARAMS6 = 0x075e8020 GHWPARAMS7 = 0x03880800 GDBGFIFOSPACE = 0x00820000 GDBGLTSSM = 0x40010440 GDBGBMU = 0x20000000 GPRTBIMAP_HS0 = 0x00000000 GPRTBIMAP_HS1 = 0x00000000 GPRTBIMAP_FS0 = 0x00000000 GPRTBIMAP_FS1 = 0x00000000 GUCTL2 = 0x0000040d VER_NUMBER = 0x00000000 VER_TYPE = 0x00000000 GUSB2PHYCFG(0) = 0x40101508 GUSB2PHYCFG(1) = 0x00000000 GUSB2PHYCFG(2) = 0x00000000 GUSB2PHYCFG(3) = 0x00000000 GUSB2PHYCFG(4) = 0x00000000 GUSB2PHYCFG(5) = 0x00000000 GUSB2PHYCFG(6) = 0x00000000 GUSB2PHYCFG(7) = 0x00000000 GUSB2PHYCFG(8) = 0x00000000 GUSB2PHYCFG(9) = 0x00000000 GUSB2PHYCFG(10) = 0x00000000 GUSB2PHYCFG(11) = 0x00000000 GUSB2PHYCFG(12) = 0x00000000 GUSB2PHYCFG(13) = 0x00000000 GUSB2PHYCFG(14) = 0x00000000 GUSB2PHYCFG(15) = 0x00000000 GUSB2I2CCTL(0) = 0x00000000 GUSB2I2CCTL(1) = 0x00000000 GUSB2I2CCTL(2) = 0x00000000 GUSB2I2CCTL(3) = 0x00000000 GUSB2I2CCTL(4) = 0x00000000 GUSB2I2CCTL(5) = 0x00000000 GUSB2I2CCTL(6) = 0x00000000 GUSB2I2CCTL(7) = 0x00000000 GUSB2I2CCTL(8) = 0x00000000 GUSB2I2CCTL(9) = 0x00000000 GUSB2I2CCTL(10) = 0x00000000 GUSB2I2CCTL(11) = 0x00000000 GUSB2I2CCTL(12) = 0x00000000 GUSB2I2CCTL(13) = 0x00000000 GUSB2I2CCTL(14) = 0x00000000 GUSB2I2CCTL(15) = 0x00000000 GUSB2PHYACC(0) = 0x00000000 GUSB2PHYACC(1) = 0x00000000 GUSB2PHYACC(2) = 0x00000000 GUSB2PHYACC(3) = 0x00000000 GUSB2PHYACC(4) = 0x00000000 GUSB2PHYACC(5) = 0x00000000 GUSB2PHYACC(6) = 0x00000000 GUSB2PHYACC(7) = 0x00000000 GUSB2PHYACC(8) = 0x00000000 GUSB2PHYACC(9) = 0x00000000 GUSB2PHYACC(10) = 0x00000000 GUSB2PHYACC(11) = 0x00000000 GUSB2PHYACC(12) = 0x00000000 GUSB2PHYACC(13) = 0x00000000 GUSB2PHYACC(14) = 0x00000000 GUSB2PHYACC(15) = 0x00000000 GUSB3PIPECTL(0) = 0x010e0002 GUSB3PIPECTL(1) = 0x00000000 GUSB3PIPECTL(2) = 0x00000000 GUSB3PIPECTL(3) = 0x00000000 GUSB3PIPECTL(4) = 0x00000000 GUSB3PIPECTL(5) = 0x00000000 GUSB3PIPECTL(6) = 0x00000000 GUSB3PIPECTL(7) = 0x00000000 GUSB3PIPECTL(8) = 0x00000000 GUSB3PIPECTL(9) = 0x00000000 GUSB3PIPECTL(10) = 0x00000000 GUSB3PIPECTL(11) = 0x00000000 GUSB3PIPECTL(12) = 0x00000000 GUSB3PIPECTL(13) = 0x00000000 GUSB3PIPECTL(14) = 0x00000000 GUSB3PIPECTL(15) = 0x00000000 GTXFIFOSIZ(0) = 0x00000082 GTXFIFOSIZ(1) = 0x00820103 GTXFIFOSIZ(2) = 0x01850286 GTXFIFOSIZ(3) = 0x040b0000 GTXFIFOSIZ(4) = 0x040b0000 GTXFIFOSIZ(5) = 0x040b0000 GTXFIFOSIZ(6) = 0x040b0000 GTXFIFOSIZ(7) = 0x040b0000 GTXFIFOSIZ(8) = 0x040b0000 GTXFIFOSIZ(9) = 0x040b0000 GTXFIFOSIZ(10) = 0x00000000 GTXFIFOSIZ(11) = 0x00000000 GTXFIFOSIZ(12) = 0x00000000 GTXFIFOSIZ(13) = 0x00000000 GTXFIFOSIZ(14) = 0x00000000 GTXFIFOSIZ(15) = 0x00000000 GTXFIFOSIZ(16) = 0x00000000 GTXFIFOSIZ(17) = 0x00000000 GTXFIFOSIZ(18) = 0x00000000 GTXFIFOSIZ(19) = 0x00000000 GTXFIFOSIZ(20) = 0x00000000 GTXFIFOSIZ(21) = 0x00000000 GTXFIFOSIZ(22) = 0x00000000 GTXFIFOSIZ(23) = 0x00000000 GTXFIFOSIZ(24) = 0x00000000 GTXFIFOSIZ(25) = 0x00000000 GTXFIFOSIZ(26) = 0x00000000 GTXFIFOSIZ(27) = 0x00000000 GTXFIFOSIZ(28) = 0x00000000 GTXFIFOSIZ(29) = 0x00000000 GTXFIFOSIZ(30) = 0x00000000 GTXFIFOSIZ(31) = 0x00000000 GRXFIFOSIZ(0) = 0x00000084 GRXFIFOSIZ(1) = 0x00840104 GRXFIFOSIZ(2) = 0x01880200 GRXFIFOSIZ(3) = 0x00000000 GRXFIFOSIZ(4) = 0x00000000 GRXFIFOSIZ(5) = 0x00000000 GRXFIFOSIZ(6) = 0x00000000 GRXFIFOSIZ(7) = 0x00000000 GRXFIFOSIZ(8) = 0x00000000 GRXFIFOSIZ(9) = 0x00000000 GRXFIFOSIZ(10) = 0x00000000 GRXFIFOSIZ(11) = 0x00000000 GRXFIFOSIZ(12) = 0x00000000 GRXFIFOSIZ(13) = 0x00000000 GRXFIFOSIZ(14) = 0x00000000 GRXFIFOSIZ(15) = 0x00000000 GRXFIFOSIZ(16) = 0x00000000 GRXFIFOSIZ(17) = 0x00000000 GRXFIFOSIZ(18) = 0x00000000 GRXFIFOSIZ(19) = 0x00000000 GRXFIFOSIZ(20) = 0x00000000 GRXFIFOSIZ(21) = 0x00000000 GRXFIFOSIZ(22) = 0x00000000 GRXFIFOSIZ(23) = 0x00000000 GRXFIFOSIZ(24) = 0x00000000 GRXFIFOSIZ(25) = 0x00000000 GRXFIFOSIZ(26) = 0x00000000 GRXFIFOSIZ(27) = 0x00000000 GRXFIFOSIZ(28) = 0x00000000 GRXFIFOSIZ(29) = 0x00000000 GRXFIFOSIZ(30) = 0x00000000 GRXFIFOSIZ(31) = 0x00000000 GEVNTADRLO(0) = 0x00000000 GEVNTADRHI(0) = 0x00000000 GEVNTSIZ(0) = 0x00001000 GEVNTCOUNT(0) = 0x00000000 GHWPARAMS8 = 0x0000075e GUCTL3 = 0x00000000 GFLADJ = 0x0a07f000 DCFG = 0x00080804 DCTL = 0x00f00000 DEVTEN = 0x00000000 DSTS = 0x00520004 DGCMDPAR = 0x00000000 DGCMD = 0x00000000 DALEPENA = 0x00000000 DEPCMDPAR2(0) = 0x00000000 DEPCMDPAR1(0) = 0x00000002 DEPCMDPAR0(0) = 0x01db2001 DEPCMD(0) = 0x00000000 DEPCMDPAR2(1) = 0x00000000 DEPCMDPAR1(1) = 0x00000000 DEPCMDPAR0(1) = 0x00000000 DEPCMD(1) = 0x00000000 DEPCMDPAR2(2) = 0x01db1000 DEPCMDPAR1(2) = 0x00000000 DEPCMDPAR0(2) = 0x00000040 DEPCMD(2) = 0x00000000 DEPCMDPAR2(3) = 0x00000000 DEPCMDPAR1(3) = 0x00000000 DEPCMDPAR0(3) = 0x00000000 DEPCMD(3) = 0x00000000 DEPCMDPAR2(4) = 0x01db5000 DEPCMDPAR1(4) = 0x00000000 DEPCMDPAR0(4) = 0x01db3000 DEPCMD(4) = 0x00000000 DEPCMDPAR2(5) = 0x00000000 DEPCMDPAR1(5) = 0x00000000 DEPCMDPAR0(5) = 0x00000000 DEPCMD(5) = 0x00000000 DEPCMDPAR2(6) = 0x00000000 DEPCMDPAR1(6) = 0x00000000 DEPCMDPAR0(6) = 0x00000000 DEPCMD(6) = 0x00000000 DEPCMDPAR2(7) = 0x00000000 DEPCMDPAR1(7) = 0x00000000 DEPCMDPAR0(7) = 0x00000000 DEPCMD(7) = 0x00000000 DEPCMDPAR2(8) = 0x00000000 DEPCMDPAR1(8) = 0x00000000 DEPCMDPAR0(8) = 0x00000000 DEPCMD(8) = 0x00000000 DEPCMDPAR2(9) = 0x00000000 DEPCMDPAR1(9) = 0x00000000 DEPCMDPAR0(9) = 0x00000000 DEPCMD(9) = 0x00000000 DEPCMDPAR2(10) = 0x00000000 DEPCMDPAR1(10) = 0x00000000 DEPCMDPAR0(10) = 0x00000000 DEPCMD(10) = 0x00000000 DEPCMDPAR2(11) = 0x00000000 DEPCMDPAR1(11) = 0x00000000 DEPCMDPAR0(11) = 0x00000000 DEPCMD(11) = 0x00000000 DEPCMDPAR2(12) = 0x00000000 DEPCMDPAR1(12) = 0x00000000 DEPCMDPAR0(12) = 0x00000000 DEPCMD(12) = 0x00000000 DEPCMDPAR2(13) = 0x00000000 DEPCMDPAR1(13) = 0x00000000 DEPCMDPAR0(13) = 0x00000000 DEPCMD(13) = 0x00000000 DEPCMDPAR2(14) = 0x00000000 DEPCMDPAR1(14) = 0x00000000 DEPCMDPAR0(14) = 0x00000000 DEPCMD(14) = 0x00000000 DEPCMDPAR2(15) = 0x00000000 DEPCMDPAR1(15) = 0x00000000 DEPCMDPAR0(15) = 0x00000000 DEPCMD(15) = 0x00000000 DEPCMDPAR2(16) = 0x00000000 DEPCMDPAR1(16) = 0x00000000 DEPCMDPAR0(16) = 0x00000000 DEPCMD(16) = 0x00000000 DEPCMDPAR2(17) = 0x00000000 DEPCMDPAR1(17) = 0x00000000 DEPCMDPAR0(17) = 0x00000000 DEPCMD(17) = 0x00000000 DEPCMDPAR2(18) = 0x00000000 DEPCMDPAR1(18) = 0x00000000 DEPCMDPAR0(18) = 0x00000000 DEPCMD(18) = 0x00000000 DEPCMDPAR2(19) = 0x00000000 DEPCMDPAR1(19) = 0x00000000 DEPCMDPAR0(19) = 0x00000000 DEPCMD(19) = 0x00000000 DEPCMDPAR2(20) = 0x00000000 DEPCMDPAR1(20) = 0x00000000 DEPCMDPAR0(20) = 0x00000000 DEPCMD(20) = 0x00000000 DEPCMDPAR2(21) = 0x00000000 DEPCMDPAR1(21) = 0x00000000 DEPCMDPAR0(21) = 0x00000000 DEPCMD(21) = 0x00000000 DEPCMDPAR2(22) = 0x00000000 DEPCMDPAR1(22) = 0x00000000 DEPCMDPAR0(22) = 0x00000000 DEPCMD(22) = 0x00000000 DEPCMDPAR2(23) = 0x00000000 DEPCMDPAR1(23) = 0x00000000 DEPCMDPAR0(23) = 0x00000000 DEPCMD(23) = 0x00000000 DEPCMDPAR2(24) = 0x00000000 DEPCMDPAR1(24) = 0x00000000 DEPCMDPAR0(24) = 0x00000000 DEPCMD(24) = 0x00000000 DEPCMDPAR2(25) = 0x00000000 DEPCMDPAR1(25) = 0x00000000 DEPCMDPAR0(25) = 0x00000000 DEPCMD(25) = 0x00000000 DEPCMDPAR2(26) = 0x00000000 DEPCMDPAR1(26) = 0x00000000 DEPCMDPAR0(26) = 0x00000000 DEPCMD(26) = 0x00000000 DEPCMDPAR2(27) = 0x00000000 DEPCMDPAR1(27) = 0x00000000 DEPCMDPAR0(27) = 0x00000000 DEPCMD(27) = 0x00000000 DEPCMDPAR2(28) = 0x00000000 DEPCMDPAR1(28) = 0x00000000 DEPCMDPAR0(28) = 0x00000000 DEPCMD(28) = 0x00000000 DEPCMDPAR2(29) = 0x00000000 DEPCMDPAR1(29) = 0x00000000 DEPCMDPAR0(29) = 0x00000000 DEPCMD(29) = 0x00000000 DEPCMDPAR2(30) = 0x00000000 DEPCMDPAR1(30) = 0x00000000 DEPCMDPAR0(30) = 0x00000000 DEPCMD(30) = 0x00000000 DEPCMDPAR2(31) = 0x00000000 DEPCMDPAR1(31) = 0x00000000 DEPCMDPAR0(31) = 0x00000000 DEPCMD(31) = 0x00000000 OCFG = 0x00000000 OCTL = 0x00000000 OEVT = 0x00000000 OEVTEN = 0x00000000 OSTS = 0x00000000 Thank you, Chris
Hi Chris, On 07/11/2024 20:50, Chris Morgan wrote: > On Thu, Oct 31, 2024 at 01:33:54AM +0000, Thinh Nguyen wrote: >> On Wed, Oct 30, 2024, Chris Morgan wrote: >>> On Wed, Oct 30, 2024 at 03:10:34PM +0200, Roger Quadros wrote: >>>> Hi Chris, >>>> >>>> On 30/10/2024 00:49, Thinh Nguyen wrote: >>>>> Hi, >>>>> >>>>> On Tue, Oct 29, 2024, Chris Morgan wrote: >>>>>> Sorry, to be specific it was the fix that causes the issues I'm now >>>>>> observing. When I explicitly revert commit >>>>>> 705e3ce37bccdf2ed6f848356ff355f480d51a91 things start working again >>>>>> for me. With that commit in place, however, suspend fails for me. >>>>> >>>>> Ok, Roger's patch is causing issue on your platform and the $subject >>>>> patch? Can you provide more details on your test sequence? >>>>> >>>>> * What does "no longer able to suspend" mean exactly (what error?) >>>>> * What mode is your usb controller? >>>>> * Is there any device connected while going into suspend? >>>>> * Can you provide dwc3 regdump? >>>> >>>> Commit 705e3ce37bccdf2ed6f848356ff355f480d51a91 will enable >>>> DWC3_GUSB2PHYCFG_SUSPHY in DWC3_GUSB2PHYCFG(i) >>>> and DWC3_GUSB3PIPECTL_SUSPHY in DWC3_GUSB3PIPECTL(i) during >>>> system suspend path, unless snps,dis_u2_susphy_quirk or >>>> snps,dis_u3_susphy_quirk is set. >>>> >>>> I see rK356x.dtsi has snps,dis_u2_susphy_quirk; >>>> Does adding snps,dis_u3_susphy_quirk resolve the issue? >>> >>> I'm afraid it does not fix the issue. Specifically when I do >>> "systemctl suspend" the device begins to suspend but freezes with the >>> kernel log output via serial console listed previously. Note I have >>> console enabled in suspend. Additionally button input no longer >>> works at this point. >>> >>> Specifically, I'm testing this with the Anbernic RG353P device based on >>> the Rockchip RK3566 SoC, in case you're curious. >>> >>> I'm not able to get you a register dump post suspend attempt as the >>> system completely freezes, however I can get you a dump prior to >>> suspend if that will help? >> >> Yes, any data is useful. >> >>> >>> Thank you, >>> Chris >> >> Can you help answer the other bullet questions I have previously. >> >> Thanks, >> Thinh > > I have 2 ports, here is a dump of each: Did you try this patch [1]. Does it fix the issue for you? [1] https://lore.kernel.org/all/20241104-am62-lpm-usb-fix-v1-1-e93df73a4f0d@kernel.org/
On Thu, Nov 07, 2024 at 09:02:51PM +0200, Roger Quadros wrote: > Hi Chris, > > On 07/11/2024 20:50, Chris Morgan wrote: > > On Thu, Oct 31, 2024 at 01:33:54AM +0000, Thinh Nguyen wrote: > >> On Wed, Oct 30, 2024, Chris Morgan wrote: > >>> On Wed, Oct 30, 2024 at 03:10:34PM +0200, Roger Quadros wrote: > >>>> Hi Chris, > >>>> > >>>> On 30/10/2024 00:49, Thinh Nguyen wrote: > >>>>> Hi, > >>>>> > >>>>> On Tue, Oct 29, 2024, Chris Morgan wrote: > >>>>>> Sorry, to be specific it was the fix that causes the issues I'm now > >>>>>> observing. When I explicitly revert commit > >>>>>> 705e3ce37bccdf2ed6f848356ff355f480d51a91 things start working again > >>>>>> for me. With that commit in place, however, suspend fails for me. > >>>>> > >>>>> Ok, Roger's patch is causing issue on your platform and the $subject > >>>>> patch? Can you provide more details on your test sequence? > >>>>> > >>>>> * What does "no longer able to suspend" mean exactly (what error?) > >>>>> * What mode is your usb controller? > >>>>> * Is there any device connected while going into suspend? > >>>>> * Can you provide dwc3 regdump? > >>>> > >>>> Commit 705e3ce37bccdf2ed6f848356ff355f480d51a91 will enable > >>>> DWC3_GUSB2PHYCFG_SUSPHY in DWC3_GUSB2PHYCFG(i) > >>>> and DWC3_GUSB3PIPECTL_SUSPHY in DWC3_GUSB3PIPECTL(i) during > >>>> system suspend path, unless snps,dis_u2_susphy_quirk or > >>>> snps,dis_u3_susphy_quirk is set. > >>>> > >>>> I see rK356x.dtsi has snps,dis_u2_susphy_quirk; > >>>> Does adding snps,dis_u3_susphy_quirk resolve the issue? > >>> > >>> I'm afraid it does not fix the issue. Specifically when I do > >>> "systemctl suspend" the device begins to suspend but freezes with the > >>> kernel log output via serial console listed previously. Note I have > >>> console enabled in suspend. Additionally button input no longer > >>> works at this point. > >>> > >>> Specifically, I'm testing this with the Anbernic RG353P device based on > >>> the Rockchip RK3566 SoC, in case you're curious. > >>> > >>> I'm not able to get you a register dump post suspend attempt as the > >>> system completely freezes, however I can get you a dump prior to > >>> suspend if that will help? > >> > >> Yes, any data is useful. > >> > >>> > >>> Thank you, > >>> Chris > >> > >> Can you help answer the other bullet questions I have previously. > >> > >> Thanks, > >> Thinh > > > > I have 2 ports, here is a dump of each: > > Did you try this patch [1]. Does it fix the issue for you? > > [1] https://lore.kernel.org/all/20241104-am62-lpm-usb-fix-v1-1-e93df73a4f0d@kernel.org/ > > -- > cheers, > -roger I'm afraid this doesn't fix it for me either. I still get the same issues. I don't know if we should wait for others who report this problem or if it's something specific just to the board I'm using. Thank you, Chris
On Thu, Nov 07, 2024, Chris Morgan wrote: > On Thu, Oct 31, 2024 at 01:33:54AM +0000, Thinh Nguyen wrote: > > On Wed, Oct 30, 2024, Chris Morgan wrote: > > > On Wed, Oct 30, 2024 at 03:10:34PM +0200, Roger Quadros wrote: > > > > Hi Chris, > > > > > > > > On 30/10/2024 00:49, Thinh Nguyen wrote: > > > > > Hi, > > > > > > > > > > On Tue, Oct 29, 2024, Chris Morgan wrote: > > > > >> Sorry, to be specific it was the fix that causes the issues I'm now > > > > >> observing. When I explicitly revert commit > > > > >> 705e3ce37bccdf2ed6f848356ff355f480d51a91 things start working again > > > > >> for me. With that commit in place, however, suspend fails for me. > > > > > > > > > > Ok, Roger's patch is causing issue on your platform and the $subject > > > > > patch? Can you provide more details on your test sequence? > > > > > > > > > > * What does "no longer able to suspend" mean exactly (what error?) > > > > > * What mode is your usb controller? > > > > > * Is there any device connected while going into suspend? > > > > > * Can you provide dwc3 regdump? > > > > > > > > Commit 705e3ce37bccdf2ed6f848356ff355f480d51a91 will enable > > > > DWC3_GUSB2PHYCFG_SUSPHY in DWC3_GUSB2PHYCFG(i) > > > > and DWC3_GUSB3PIPECTL_SUSPHY in DWC3_GUSB3PIPECTL(i) during > > > > system suspend path, unless snps,dis_u2_susphy_quirk or > > > > snps,dis_u3_susphy_quirk is set. > > > > > > > > I see rK356x.dtsi has snps,dis_u2_susphy_quirk; > > > > Does adding snps,dis_u3_susphy_quirk resolve the issue? > > > > > > I'm afraid it does not fix the issue. Specifically when I do > > > "systemctl suspend" the device begins to suspend but freezes with the > > > kernel log output via serial console listed previously. Note I have > > > console enabled in suspend. Additionally button input no longer > > > works at this point. > > > > > > Specifically, I'm testing this with the Anbernic RG353P device based on > > > the Rockchip RK3566 SoC, in case you're curious. > > > > > > I'm not able to get you a register dump post suspend attempt as the > > > system completely freezes, however I can get you a dump prior to > > > suspend if that will help? > > > > Yes, any data is useful. > > > > > > > > Thank you, > > > Chris > > > > Can you help answer the other bullet questions I have previously. > > > > Thanks, > > Thinh > > I have 2 ports, here is a dump of each: > > usb@fcc00000: > > fd000000: Thanks for the dumps! They're helpful. Looks like the fcc00000 is device mode and operating in usb2 speed only, and the fd000000 is host. Can you help narrow down the problems by checking these: When you set the snps,dis_u3_susphy_quirk, did you set it to both controllers? Which controller suspend causes the problem? The host or the device or both? you can check by unbind the driver for each 1 at a time to prevent suspend. e.g.: # echo usb@fcc00000 > /sys/bus/platform/drivers/dwc3/unbind # echo usb@fd000000 > /sys/bus/platform/drivers/dwc3/unbind Thanks, Thinh
I'm no longer able to replicate the issue, so I'm guessing some subsequent fixes solved it. I thought I tested them all but maybe I missed one. Thank you. On Wed, Nov 13, 2024 at 8:35 PM Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote: > > On Thu, Nov 07, 2024, Chris Morgan wrote: > > On Thu, Oct 31, 2024 at 01:33:54AM +0000, Thinh Nguyen wrote: > > > On Wed, Oct 30, 2024, Chris Morgan wrote: > > > > On Wed, Oct 30, 2024 at 03:10:34PM +0200, Roger Quadros wrote: > > > > > Hi Chris, > > > > > > > > > > On 30/10/2024 00:49, Thinh Nguyen wrote: > > > > > > Hi, > > > > > > > > > > > > On Tue, Oct 29, 2024, Chris Morgan wrote: > > > > > >> Sorry, to be specific it was the fix that causes the issues I'm now > > > > > >> observing. When I explicitly revert commit > > > > > >> 705e3ce37bccdf2ed6f848356ff355f480d51a91 things start working again > > > > > >> for me. With that commit in place, however, suspend fails for me. > > > > > > > > > > > > Ok, Roger's patch is causing issue on your platform and the $subject > > > > > > patch? Can you provide more details on your test sequence? > > > > > > > > > > > > * What does "no longer able to suspend" mean exactly (what error?) > > > > > > * What mode is your usb controller? > > > > > > * Is there any device connected while going into suspend? > > > > > > * Can you provide dwc3 regdump? > > > > > > > > > > Commit 705e3ce37bccdf2ed6f848356ff355f480d51a91 will enable > > > > > DWC3_GUSB2PHYCFG_SUSPHY in DWC3_GUSB2PHYCFG(i) > > > > > and DWC3_GUSB3PIPECTL_SUSPHY in DWC3_GUSB3PIPECTL(i) during > > > > > system suspend path, unless snps,dis_u2_susphy_quirk or > > > > > snps,dis_u3_susphy_quirk is set. > > > > > > > > > > I see rK356x.dtsi has snps,dis_u2_susphy_quirk; > > > > > Does adding snps,dis_u3_susphy_quirk resolve the issue? > > > > > > > > I'm afraid it does not fix the issue. Specifically when I do > > > > "systemctl suspend" the device begins to suspend but freezes with the > > > > kernel log output via serial console listed previously. Note I have > > > > console enabled in suspend. Additionally button input no longer > > > > works at this point. > > > > > > > > Specifically, I'm testing this with the Anbernic RG353P device based on > > > > the Rockchip RK3566 SoC, in case you're curious. > > > > > > > > I'm not able to get you a register dump post suspend attempt as the > > > > system completely freezes, however I can get you a dump prior to > > > > suspend if that will help? > > > > > > Yes, any data is useful. > > > > > > > > > > > Thank you, > > > > Chris > > > > > > Can you help answer the other bullet questions I have previously. > > > > > > Thanks, > > > Thinh > > > > I have 2 ports, here is a dump of each: > > > > usb@fcc00000: > > > > fd000000: > > Thanks for the dumps! They're helpful. Looks like the fcc00000 is device > mode and operating in usb2 speed only, and the fd000000 is host. > > Can you help narrow down the problems by checking these: > > When you set the snps,dis_u3_susphy_quirk, did you set it to both > controllers? > > Which controller suspend causes the problem? The host or the device or > both? you can check by unbind the driver for each 1 at a time to prevent > suspend. e.g.: > # echo usb@fcc00000 > /sys/bus/platform/drivers/dwc3/unbind > # echo usb@fd000000 > /sys/bus/platform/drivers/dwc3/unbind > > Thanks, > Thinh
On Tue, Nov 19, 2024, Chris Morgan wrote: > I'm no longer able to replicate the issue, so I'm guessing some > subsequent fixes solved it. I thought I tested them all but maybe I > missed one. > > Thank you. > Ok. Thanks for testing! BR, Thinh
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 31684cdaaae3..100041320e8d 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -104,6 +104,27 @@ static int dwc3_get_dr_mode(struct dwc3 *dwc) return 0; } +void dwc3_enable_susphy(struct dwc3 *dwc, bool enable) +{ + u32 reg; + + reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0)); + if (enable && !dwc->dis_u3_susphy_quirk) + reg |= DWC3_GUSB3PIPECTL_SUSPHY; + else + reg &= ~DWC3_GUSB3PIPECTL_SUSPHY; + + dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg); + + reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0)); + if (enable && !dwc->dis_u2_susphy_quirk) + reg |= DWC3_GUSB2PHYCFG_SUSPHY; + else + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY; + + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg); +} + void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode) { u32 reg; @@ -585,11 +606,8 @@ static int dwc3_core_ulpi_init(struct dwc3 *dwc) */ static int dwc3_phy_setup(struct dwc3 *dwc) { - unsigned int hw_mode; u32 reg; - hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0); - reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0)); /* @@ -599,21 +617,16 @@ static int dwc3_phy_setup(struct dwc3 *dwc) reg &= ~DWC3_GUSB3PIPECTL_UX_EXIT_PX; /* - * Above 1.94a, it is recommended to set DWC3_GUSB3PIPECTL_SUSPHY - * to '0' during coreConsultant configuration. So default value - * will be '0' when the core is reset. Application needs to set it - * to '1' after the core initialization is completed. - */ - if (!DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) - reg |= DWC3_GUSB3PIPECTL_SUSPHY; - - /* - * For DRD controllers, GUSB3PIPECTL.SUSPENDENABLE must be cleared after - * power-on reset, and it can be set after core initialization, which is - * after device soft-reset during initialization. + * Above DWC_usb3.0 1.94a, it is recommended to set + * DWC3_GUSB3PIPECTL_SUSPHY to '0' during coreConsultant configuration. + * So default value will be '0' when the core is reset. Application + * needs to set it to '1' after the core initialization is completed. + * + * Similarly for DRD controllers, GUSB3PIPECTL.SUSPENDENABLE must be + * cleared after power-on reset, and it can be set after core + * initialization. */ - if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD) - reg &= ~DWC3_GUSB3PIPECTL_SUSPHY; + reg &= ~DWC3_GUSB3PIPECTL_SUSPHY; if (dwc->u2ss_inp3_quirk) reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK; @@ -639,9 +652,6 @@ static int dwc3_phy_setup(struct dwc3 *dwc) if (dwc->tx_de_emphasis_quirk) reg |= DWC3_GUSB3PIPECTL_TX_DEEPH(dwc->tx_de_emphasis); - if (dwc->dis_u3_susphy_quirk) - reg &= ~DWC3_GUSB3PIPECTL_SUSPHY; - if (dwc->dis_del_phy_power_chg_quirk) reg &= ~DWC3_GUSB3PIPECTL_DEPOCHANGE; @@ -689,24 +699,15 @@ static int dwc3_phy_setup(struct dwc3 *dwc) } /* - * Above 1.94a, it is recommended to set DWC3_GUSB2PHYCFG_SUSPHY to - * '0' during coreConsultant configuration. So default value will - * be '0' when the core is reset. Application needs to set it to - * '1' after the core initialization is completed. - */ - if (!DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) - reg |= DWC3_GUSB2PHYCFG_SUSPHY; - - /* - * For DRD controllers, GUSB2PHYCFG.SUSPHY must be cleared after - * power-on reset, and it can be set after core initialization, which is - * after device soft-reset during initialization. + * Above DWC_usb3.0 1.94a, it is recommended to set + * DWC3_GUSB2PHYCFG_SUSPHY to '0' during coreConsultant configuration. + * So default value will be '0' when the core is reset. Application + * needs to set it to '1' after the core initialization is completed. + * + * Similarly for DRD controllers, GUSB2PHYCFG.SUSPHY must be cleared + * after power-on reset, and it can be set after core initialization. */ - if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD) - reg &= ~DWC3_GUSB2PHYCFG_SUSPHY; - - if (dwc->dis_u2_susphy_quirk) - reg &= ~DWC3_GUSB2PHYCFG_SUSPHY; + reg &= ~DWC3_GUSB2PHYCFG_SUSPHY; if (dwc->dis_enblslpm_quirk) reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM; @@ -1227,21 +1228,6 @@ static int dwc3_core_init(struct dwc3 *dwc) if (ret) goto err_exit_phy; - if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD && - !DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) { - if (!dwc->dis_u3_susphy_quirk) { - reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0)); - reg |= DWC3_GUSB3PIPECTL_SUSPHY; - dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg); - } - - if (!dwc->dis_u2_susphy_quirk) { - reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0)); - reg |= DWC3_GUSB2PHYCFG_SUSPHY; - dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg); - } - } - dwc3_core_setup_global_control(dwc); dwc3_core_num_eps(dwc); diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 7e80dd3d466b..180dd8d29287 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -1580,6 +1580,7 @@ int dwc3_event_buffers_setup(struct dwc3 *dwc); void dwc3_event_buffers_cleanup(struct dwc3 *dwc); int dwc3_core_soft_reset(struct dwc3 *dwc); +void dwc3_enable_susphy(struct dwc3 *dwc, bool enable); #if IS_ENABLED(CONFIG_USB_DWC3_HOST) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE) int dwc3_host_init(struct dwc3 *dwc); diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 4df2661f6675..f94f68f1e7d2 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2924,6 +2924,7 @@ static int __dwc3_gadget_start(struct dwc3 *dwc) dwc3_ep0_out_start(dwc); dwc3_gadget_enable_irq(dwc); + dwc3_enable_susphy(dwc, true); return 0; @@ -4690,6 +4691,7 @@ void dwc3_gadget_exit(struct dwc3 *dwc) if (!dwc->gadget) return; + dwc3_enable_susphy(dwc, false); usb_del_gadget(dwc->gadget); dwc3_gadget_free_endpoints(dwc); usb_put_gadget(dwc->gadget); diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c index 0204787df81d..a171b27a7845 100644 --- a/drivers/usb/dwc3/host.c +++ b/drivers/usb/dwc3/host.c @@ -10,10 +10,13 @@ #include <linux/irq.h> #include <linux/of.h> #include <linux/platform_device.h> +#include <linux/usb.h> +#include <linux/usb/hcd.h> #include "../host/xhci-port.h" #include "../host/xhci-ext-caps.h" #include "../host/xhci-caps.h" +#include "../host/xhci-plat.h" #include "core.h" #define XHCI_HCSPARAMS1 0x4 @@ -57,6 +60,24 @@ static void dwc3_power_off_all_roothub_ports(struct dwc3 *dwc) } } +static void dwc3_xhci_plat_start(struct usb_hcd *hcd) +{ + struct platform_device *pdev; + struct dwc3 *dwc; + + if (!usb_hcd_is_primary_hcd(hcd)) + return; + + pdev = to_platform_device(hcd->self.controller); + dwc = dev_get_drvdata(pdev->dev.parent); + + dwc3_enable_susphy(dwc, true); +} + +static const struct xhci_plat_priv dwc3_xhci_plat_quirk = { + .plat_start = dwc3_xhci_plat_start, +}; + static void dwc3_host_fill_xhci_irq_res(struct dwc3 *dwc, int irq, char *name) { @@ -167,6 +188,11 @@ int dwc3_host_init(struct dwc3 *dwc) } } + ret = platform_device_add_data(xhci, &dwc3_xhci_plat_quirk, + sizeof(struct xhci_plat_priv)); + if (ret) + goto err; + ret = platform_device_add(xhci); if (ret) { dev_err(dwc->dev, "failed to register xHCI device\n"); @@ -192,6 +218,7 @@ void dwc3_host_exit(struct dwc3 *dwc) if (dwc->sys_wakeup) device_init_wakeup(&dwc->xhci->dev, false); + dwc3_enable_susphy(dwc, false); platform_device_unregister(dwc->xhci); dwc->xhci = NULL; }
GUSB3PIPECTL.SUSPENDENABLE and GUSB2PHYCFG.SUSPHY should be cleared during initialization. Suspend during initialization can result in undefined behavior due to clock synchronization failure, which often seen as core soft reset timeout. The programming guide recommended these bits to be cleared during initialization for DWC_usb3.0 version 1.94 and above (along with DWC_usb31 and DWC_usb32). The current check in the driver does not account if it's set by default setting from coreConsultant. This is especially the case for DRD when switching mode to ensure the phy clocks are available to change mode. Depending on the platforms/design, some may be affected more than others. This is noted in the DWC_usb3x programming guide under the above registers. Let's just disable them during driver load and mode switching. Restore them when the controller initialization completes. Note that some platforms workaround this issue by disabling phy suspend through "snps,dis_u3_susphy_quirk" and "snps,dis_u2_susphy_quirk" when they should not need to. Cc: stable@vger.kernel.org Fixes: 9ba3aca8fe82 ("usb: dwc3: Disable phy suspend after power-on reset") Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> --- drivers/usb/dwc3/core.c | 90 +++++++++++++++++---------------------- drivers/usb/dwc3/core.h | 1 + drivers/usb/dwc3/gadget.c | 2 + drivers/usb/dwc3/host.c | 27 ++++++++++++ 4 files changed, 68 insertions(+), 52 deletions(-)