| Message ID | 20221026190749.2280367-1-Frank.Li@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9d5333c931347005352d5b8beaa43528c94cfc9c |
| Headers | show |
| Series | [v2,1/1] usb: cdns3: host: fix endless superspeed hub port reset | expand |
On 22-10-26 15:07:49, Frank Li wrote: > From: Li Jun <jun.li@nxp.com> > > When usb 3.0 hub connect with one USB 2.0 device and NO USB 3.0 device, > some usb hub reports endless port reset message. > > [ 190.324169] usb 2-1: new SuperSpeed USB device number 88 using xhci-hcd > [ 190.352834] hub 2-1:1.0: USB hub found > [ 190.356995] hub 2-1:1.0: 4 ports detected > [ 190.700056] usb 2-1: USB disconnect, device number 88 > [ 192.472139] usb 2-1: new SuperSpeed USB device number 89 using xhci-hcd > [ 192.500820] hub 2-1:1.0: USB hub found > [ 192.504977] hub 2-1:1.0: 4 ports detected > [ 192.852066] usb 2-1: USB disconnect, device number 89 > > The reason is the runtime pm state of USB2.0 port is active and > USB 3.0 port is suspend, so parent device is active state. > > cat /sys/bus/platform/devices/5b110000.usb/5b130000.usb/xhci-hcd.1.auto/usb2/power/runtime_status > > suspended > > cat /sys/bus/platform/devices/5b110000.usb/5b130000.usb/xhci-hcd.1.auto/usb1/power/runtime_status > > active > > cat /sys/bus/platform/devices/5b110000.usb/5b130000.usb/xhci-hcd.1.auto/power/runtime_status > > active > > cat /sys/bus/platform/devices/5b110000.usb/5b130000.usb/power/runtime_status > > active > > So xhci_cdns3_suspend_quirk() have not called. U3 configure is not applied. > > move U3 configure into host start. Reinit again in resume function in case > controller power lost during suspend. > > Cc: stable@vger.kernel.org 5.10 > Signed-off-by: Li Jun <jun.li@nxp.com> > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > > Change from V1 to V2. > - call xhci_cdns3_plat_start() in xhci_cdns3_resume_quirk > in case power lost during suspend Reviewed-by: Peter Chen <peter.chen@kernel.org> > > drivers/usb/cdns3/host.c | 56 ++++++++++++++++++++-------------------- > 1 file changed, 28 insertions(+), 28 deletions(-) > > diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c > index 9643b905e2d8..6164fc4c96a4 100644 > --- a/drivers/usb/cdns3/host.c > +++ b/drivers/usb/cdns3/host.c > @@ -24,11 +24,37 @@ > #define CFG_RXDET_P3_EN BIT(15) > #define LPM_2_STB_SWITCH_EN BIT(25) > > -static int xhci_cdns3_suspend_quirk(struct usb_hcd *hcd); > +static void xhci_cdns3_plat_start(struct usb_hcd *hcd) > +{ > + struct xhci_hcd *xhci = hcd_to_xhci(hcd); > + u32 value; > + > + /* set usbcmd.EU3S */ > + value = readl(&xhci->op_regs->command); > + value |= CMD_PM_INDEX; > + writel(value, &xhci->op_regs->command); > + > + if (hcd->regs) { > + value = readl(hcd->regs + XECP_AUX_CTRL_REG1); > + value |= CFG_RXDET_P3_EN; > + writel(value, hcd->regs + XECP_AUX_CTRL_REG1); > + > + value = readl(hcd->regs + XECP_PORT_CAP_REG); > + value |= LPM_2_STB_SWITCH_EN; > + writel(value, hcd->regs + XECP_PORT_CAP_REG); > + } > +} > + > +static int xhci_cdns3_resume_quirk(struct usb_hcd *hcd) > +{ > + xhci_cdns3_plat_start(hcd); > + return 0; > +} > > static const struct xhci_plat_priv xhci_plat_cdns3_xhci = { > .quirks = XHCI_SKIP_PHY_INIT | XHCI_AVOID_BEI, > - .suspend_quirk = xhci_cdns3_suspend_quirk, > + .plat_start = xhci_cdns3_plat_start, > + .resume_quirk = xhci_cdns3_resume_quirk, > }; > > static int __cdns_host_init(struct cdns *cdns) > @@ -90,32 +116,6 @@ static int __cdns_host_init(struct cdns *cdns) > return ret; > } > > -static int xhci_cdns3_suspend_quirk(struct usb_hcd *hcd) > -{ > - struct xhci_hcd *xhci = hcd_to_xhci(hcd); > - u32 value; > - > - if (pm_runtime_status_suspended(hcd->self.controller)) > - return 0; > - > - /* set usbcmd.EU3S */ > - value = readl(&xhci->op_regs->command); > - value |= CMD_PM_INDEX; > - writel(value, &xhci->op_regs->command); > - > - if (hcd->regs) { > - value = readl(hcd->regs + XECP_AUX_CTRL_REG1); > - value |= CFG_RXDET_P3_EN; > - writel(value, hcd->regs + XECP_AUX_CTRL_REG1); > - > - value = readl(hcd->regs + XECP_PORT_CAP_REG); > - value |= LPM_2_STB_SWITCH_EN; > - writel(value, hcd->regs + XECP_PORT_CAP_REG); > - } > - > - return 0; > -} > - > static void cdns_host_exit(struct cdns *cdns) > { > kfree(cdns->xhci_plat_data); > -- > 2.34.1 >
Am Mittwoch, 26. Oktober 2022, 21:07:49 CEST schrieb Frank Li: > From: Li Jun <jun.li@nxp.com> > > When usb 3.0 hub connect with one USB 2.0 device and NO USB 3.0 device, > some usb hub reports endless port reset message. > > [ 190.324169] usb 2-1: new SuperSpeed USB device number 88 using xhci-hcd > [ 190.352834] hub 2-1:1.0: USB hub found > [ 190.356995] hub 2-1:1.0: 4 ports detected > [ 190.700056] usb 2-1: USB disconnect, device number 88 > [ 192.472139] usb 2-1: new SuperSpeed USB device number 89 using xhci-hcd > [ 192.500820] hub 2-1:1.0: USB hub found > [ 192.504977] hub 2-1:1.0: 4 ports detected > [ 192.852066] usb 2-1: USB disconnect, device number 89 > > The reason is the runtime pm state of USB2.0 port is active and > USB 3.0 port is suspend, so parent device is active state. > > cat > /sys/bus/platform/devices/5b110000.usb/5b130000.usb/xhci-hcd.1.auto/usb2/po > wer/runtime_status > > suspended > > cat > /sys/bus/platform/devices/5b110000.usb/5b130000.usb/xhci-hcd.1.auto/usb1/po > wer/runtime_status > > active > > cat > /sys/bus/platform/devices/5b110000.usb/5b130000.usb/xhci-hcd.1.auto/power/r > untime_status > > active > > cat > /sys/bus/platform/devices/5b110000.usb/5b130000.usb/power/runtime_status > > active > > So xhci_cdns3_suspend_quirk() have not called. U3 configure is not applied. > > move U3 configure into host start. Reinit again in resume function in case > controller power lost during suspend. > > Cc: stable@vger.kernel.org 5.10 > Signed-off-by: Li Jun <jun.li@nxp.com> > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > > Change from V1 to V2. > - call xhci_cdns3_plat_start() in xhci_cdns3_resume_quirk > in case power lost during suspend > > drivers/usb/cdns3/host.c | 56 ++++++++++++++++++++-------------------- > 1 file changed, 28 insertions(+), 28 deletions(-) > > diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c > index 9643b905e2d8..6164fc4c96a4 100644 > --- a/drivers/usb/cdns3/host.c > +++ b/drivers/usb/cdns3/host.c > @@ -24,11 +24,37 @@ > #define CFG_RXDET_P3_EN BIT(15) > #define LPM_2_STB_SWITCH_EN BIT(25) > > -static int xhci_cdns3_suspend_quirk(struct usb_hcd *hcd); > +static void xhci_cdns3_plat_start(struct usb_hcd *hcd) > +{ > + struct xhci_hcd *xhci = hcd_to_xhci(hcd); > + u32 value; > + > + /* set usbcmd.EU3S */ > + value = readl(&xhci->op_regs->command); > + value |= CMD_PM_INDEX; > + writel(value, &xhci->op_regs->command); > + > + if (hcd->regs) { > + value = readl(hcd->regs + XECP_AUX_CTRL_REG1); > + value |= CFG_RXDET_P3_EN; > + writel(value, hcd->regs + XECP_AUX_CTRL_REG1); > + > + value = readl(hcd->regs + XECP_PORT_CAP_REG); > + value |= LPM_2_STB_SWITCH_EN; > + writel(value, hcd->regs + XECP_PORT_CAP_REG); > + } > +} > + > +static int xhci_cdns3_resume_quirk(struct usb_hcd *hcd) > +{ > + xhci_cdns3_plat_start(hcd); > + return 0; > +} > > static const struct xhci_plat_priv xhci_plat_cdns3_xhci = { > .quirks = XHCI_SKIP_PHY_INIT | XHCI_AVOID_BEI, > - .suspend_quirk = xhci_cdns3_suspend_quirk, > + .plat_start = xhci_cdns3_plat_start, > + .resume_quirk = xhci_cdns3_resume_quirk, > }; > > static int __cdns_host_init(struct cdns *cdns) > @@ -90,32 +116,6 @@ static int __cdns_host_init(struct cdns *cdns) > return ret; > } > > -static int xhci_cdns3_suspend_quirk(struct usb_hcd *hcd) > -{ > - struct xhci_hcd *xhci = hcd_to_xhci(hcd); > - u32 value; > - > - if (pm_runtime_status_suspended(hcd->self.controller)) > - return 0; > - > - /* set usbcmd.EU3S */ > - value = readl(&xhci->op_regs->command); > - value |= CMD_PM_INDEX; > - writel(value, &xhci->op_regs->command); > - > - if (hcd->regs) { > - value = readl(hcd->regs + XECP_AUX_CTRL_REG1); > - value |= CFG_RXDET_P3_EN; > - writel(value, hcd->regs + XECP_AUX_CTRL_REG1); > - > - value = readl(hcd->regs + XECP_PORT_CAP_REG); > - value |= LPM_2_STB_SWITCH_EN; > - writel(value, hcd->regs + XECP_PORT_CAP_REG); > - } > - > - return 0; > -} > - > static void cdns_host_exit(struct cdns *cdns) > { > kfree(cdns->xhci_plat_data); Although I could only test it on v5.15 downstream kernel Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> -----Original Message----- > From: Alexander Stein <alexander.stein@ew.tq-group.com> > Sent: Thursday, October 27, 2022 2:06 AM > To: Frank Li <frank.li@nxp.com> > Cc: Jun Li <jun.li@nxp.com>; Frank Li <frank.li@nxp.com>; a- > govindraju@ti.com; gregkh@linuxfoundation.org; hzpeterchen@gmail.com; > imx@lists.linux.dev; linux-kernel@vger.kernel.org; linux- > usb@vger.kernel.org; pawell@cadence.com; peter.chen@kernel.org; > rogerq@kernel.org; stable@kernel.org > Subject: [EXT] Re: [PATCH v2 1/1] usb: cdns3: host: fix endless superspeed > hub port reset > > Caution: EXT Email > > Am Mittwoch, 26. Oktober 2022, 21:07:49 CEST schrieb Frank Li: > > From: Li Jun <jun.li@nxp.com> > > > > When usb 3.0 hub connect with one USB 2.0 device and NO USB 3.0 device, > > some usb hub reports endless port reset message. > > > > [ 190.324169] usb 2-1: new SuperSpeed USB device number 88 using xhci- > hcd > > [ 190.352834] hub 2-1:1.0: USB hub found > > [ 190.356995] hub 2-1:1.0: 4 ports detected > > [ 190.700056] usb 2-1: USB disconnect, device number 88 > > [ 192.472139] usb 2-1: new SuperSpeed USB device number 89 using xhci- > hcd > > [ 192.500820] hub 2-1:1.0: USB hub found > > [ 192.504977] hub 2-1:1.0: 4 ports detected > > [ 192.852066] usb 2-1: USB disconnect, device number 89 > > > > The reason is the runtime pm state of USB2.0 port is active and > > USB 3.0 port is suspend, so parent device is active state. > > > > cat > > /sys/bus/platform/devices/5b110000.usb/5b130000.usb/xhci- > hcd.1.auto/usb2/po > > wer/runtime_status > > > > suspended > > > > cat > > /sys/bus/platform/devices/5b110000.usb/5b130000.usb/xhci- > hcd.1.auto/usb1/po > > wer/runtime_status > > > > active > > > > cat > > /sys/bus/platform/devices/5b110000.usb/5b130000.usb/xhci- > hcd.1.auto/power/r > > untime_status > > > > active > > > > cat > > > /sys/bus/platform/devices/5b110000.usb/5b130000.usb/power/runtime_sta > tus > > > > active > > > > So xhci_cdns3_suspend_quirk() have not called. U3 configure is not applied. > > > > move U3 configure into host start. Reinit again in resume function in case > > controller power lost during suspend. > > > > Cc: stable@vger.kernel.org 5.10 > > Signed-off-by: Li Jun <jun.li@nxp.com> > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- [Frank Li] @greg, friendly ping. > > > > Change from V1 to V2. > > - call xhci_cdns3_plat_start() in xhci_cdns3_resume_quirk > > in case power lost during suspend > > > > drivers/usb/cdns3/host.c | 56 ++++++++++++++++++++-------------------- > > 1 file changed, 28 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c > > index 9643b905e2d8..6164fc4c96a4 100644 > > --- a/drivers/usb/cdns3/host.c > > +++ b/drivers/usb/cdns3/host.c > > @@ -24,11 +24,37 @@ > > #define CFG_RXDET_P3_EN BIT(15) > > #define LPM_2_STB_SWITCH_EN BIT(25) > > > > -static int xhci_cdns3_suspend_quirk(struct usb_hcd *hcd); > > +static void xhci_cdns3_plat_start(struct usb_hcd *hcd) > > +{ > > + struct xhci_hcd *xhci = hcd_to_xhci(hcd); > > + u32 value; > > + > > + /* set usbcmd.EU3S */ > > + value = readl(&xhci->op_regs->command); > > + value |= CMD_PM_INDEX; > > + writel(value, &xhci->op_regs->command); > > + > > + if (hcd->regs) { > > + value = readl(hcd->regs + XECP_AUX_CTRL_REG1); > > + value |= CFG_RXDET_P3_EN; > > + writel(value, hcd->regs + XECP_AUX_CTRL_REG1); > > + > > + value = readl(hcd->regs + XECP_PORT_CAP_REG); > > + value |= LPM_2_STB_SWITCH_EN; > > + writel(value, hcd->regs + XECP_PORT_CAP_REG); > > + } > > +} > > + > > +static int xhci_cdns3_resume_quirk(struct usb_hcd *hcd) > > +{ > > + xhci_cdns3_plat_start(hcd); > > + return 0; > > +} > > > > static const struct xhci_plat_priv xhci_plat_cdns3_xhci = { > > .quirks = XHCI_SKIP_PHY_INIT | XHCI_AVOID_BEI, > > - .suspend_quirk = xhci_cdns3_suspend_quirk, > > + .plat_start = xhci_cdns3_plat_start, > > + .resume_quirk = xhci_cdns3_resume_quirk, > > }; > > > > static int __cdns_host_init(struct cdns *cdns) > > @@ -90,32 +116,6 @@ static int __cdns_host_init(struct cdns *cdns) > > return ret; > > } > > > > -static int xhci_cdns3_suspend_quirk(struct usb_hcd *hcd) > > -{ > > - struct xhci_hcd *xhci = hcd_to_xhci(hcd); > > - u32 value; > > - > > - if (pm_runtime_status_suspended(hcd->self.controller)) > > - return 0; > > - > > - /* set usbcmd.EU3S */ > > - value = readl(&xhci->op_regs->command); > > - value |= CMD_PM_INDEX; > > - writel(value, &xhci->op_regs->command); > > - > > - if (hcd->regs) { > > - value = readl(hcd->regs + XECP_AUX_CTRL_REG1); > > - value |= CFG_RXDET_P3_EN; > > - writel(value, hcd->regs + XECP_AUX_CTRL_REG1); > > - > > - value = readl(hcd->regs + XECP_PORT_CAP_REG); > > - value |= LPM_2_STB_SWITCH_EN; > > - writel(value, hcd->regs + XECP_PORT_CAP_REG); > > - } > > - > > - return 0; > > -} > > - > > static void cdns_host_exit(struct cdns *cdns) > > { > > kfree(cdns->xhci_plat_data); > > Although I could only test it on v5.15 downstream kernel > Acked-by: Alexander Stein <alexander.stein@ew.tq-group.com> > >
diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c index 9643b905e2d8..6164fc4c96a4 100644 --- a/drivers/usb/cdns3/host.c +++ b/drivers/usb/cdns3/host.c @@ -24,11 +24,37 @@ #define CFG_RXDET_P3_EN BIT(15) #define LPM_2_STB_SWITCH_EN BIT(25) -static int xhci_cdns3_suspend_quirk(struct usb_hcd *hcd); +static void xhci_cdns3_plat_start(struct usb_hcd *hcd) +{ + struct xhci_hcd *xhci = hcd_to_xhci(hcd); + u32 value; + + /* set usbcmd.EU3S */ + value = readl(&xhci->op_regs->command); + value |= CMD_PM_INDEX; + writel(value, &xhci->op_regs->command); + + if (hcd->regs) { + value = readl(hcd->regs + XECP_AUX_CTRL_REG1); + value |= CFG_RXDET_P3_EN; + writel(value, hcd->regs + XECP_AUX_CTRL_REG1); + + value = readl(hcd->regs + XECP_PORT_CAP_REG); + value |= LPM_2_STB_SWITCH_EN; + writel(value, hcd->regs + XECP_PORT_CAP_REG); + } +} + +static int xhci_cdns3_resume_quirk(struct usb_hcd *hcd) +{ + xhci_cdns3_plat_start(hcd); + return 0; +} static const struct xhci_plat_priv xhci_plat_cdns3_xhci = { .quirks = XHCI_SKIP_PHY_INIT | XHCI_AVOID_BEI, - .suspend_quirk = xhci_cdns3_suspend_quirk, + .plat_start = xhci_cdns3_plat_start, + .resume_quirk = xhci_cdns3_resume_quirk, }; static int __cdns_host_init(struct cdns *cdns) @@ -90,32 +116,6 @@ static int __cdns_host_init(struct cdns *cdns) return ret; } -static int xhci_cdns3_suspend_quirk(struct usb_hcd *hcd) -{ - struct xhci_hcd *xhci = hcd_to_xhci(hcd); - u32 value; - - if (pm_runtime_status_suspended(hcd->self.controller)) - return 0; - - /* set usbcmd.EU3S */ - value = readl(&xhci->op_regs->command); - value |= CMD_PM_INDEX; - writel(value, &xhci->op_regs->command); - - if (hcd->regs) { - value = readl(hcd->regs + XECP_AUX_CTRL_REG1); - value |= CFG_RXDET_P3_EN; - writel(value, hcd->regs + XECP_AUX_CTRL_REG1); - - value = readl(hcd->regs + XECP_PORT_CAP_REG); - value |= LPM_2_STB_SWITCH_EN; - writel(value, hcd->regs + XECP_PORT_CAP_REG); - } - - return 0; -} - static void cdns_host_exit(struct cdns *cdns) { kfree(cdns->xhci_plat_data);