| Message ID | 20221019152117.1234968-1-Frank.Li@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] usb: cdns3: host: fix endless superspeed hub port reset | expand |
On Wed, Oct 19, 2022 at 11:21 PM Frank Li <Frank.Li@nxp.com> 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 > > Signed-off-by: Li Jun <jun.li@nxp.com> > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > drivers/usb/cdns3/host.c | 58 +++++++++++++++++++++------------------- > 1 file changed, 31 insertions(+), 27 deletions(-) > > diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c > index 9643b905e2d8..9092b6f4d7c4 100644 > --- a/drivers/usb/cdns3/host.c > +++ b/drivers/usb/cdns3/host.c > @@ -24,11 +24,41 @@ > #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_suspend_quirk(struct usb_hcd *hcd) > +{ > + if (pm_runtime_status_suspended(hcd->self.controller)) > + return 0; > + > + xhci_cdns3_plat_start(hcd); > + Why does it need to call again at suspend? Peter > + 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, > }; > > static int __cdns_host_init(struct cdns *cdns) > @@ -90,32 +120,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 >
> -----Original Message----- > From: Peter Chen <hzpeterchen@gmail.com> > Sent: Thursday, October 20, 2022 8:52 PM > To: Frank Li <frank.li@nxp.com> > Cc: imx@lists.linux.dev; Pawel Laszczak <pawell@cadence.com>; Peter Chen > <peter.chen@kernel.org>; Roger Quadros <rogerq@kernel.org>; Aswath > Govindraju <a-govindraju@ti.com>; Greg Kroah-Hartman > <gregkh@linuxfoundation.org>; open list:CADENCE USBSSP DRD IP DRIVER > <linux-usb@vger.kernel.org>; open list <linux-kernel@vger.kernel.org>; > stable@kernel.org > Subject: [EXT] Re: [PATCH 1/1] usb: cdns3: host: fix endless superspeed hub > port reset > > Caution: EXT Email > > On Wed, Oct 19, 2022 at 11:21 PM Frank Li <Frank.Li@nxp.com> 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_st > atus > > > > active > > > > So xhci_cdns3_suspend_quirk() have not called. U3 configure is not applied. > > > > move U3 configure into host start > > > > Signed-off-by: Li Jun <jun.li@nxp.com> > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > drivers/usb/cdns3/host.c | 58 +++++++++++++++++++++------------------- > > 1 file changed, 31 insertions(+), 27 deletions(-) > > > > diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c > > index 9643b905e2d8..9092b6f4d7c4 100644 > > --- a/drivers/usb/cdns3/host.c > > +++ b/drivers/usb/cdns3/host.c > > @@ -24,11 +24,41 @@ > > #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_suspend_quirk(struct usb_hcd *hcd) > > +{ > > + if (pm_runtime_status_suspended(hcd->self.controller)) > > + return 0; > > + > > + xhci_cdns3_plat_start(hcd); > > + > > Why does it need to call again at suspend? [Frank Li] Power may lost at suspend. > > Peter > > > + 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, > > }; > > > > static int __cdns_host_init(struct cdns *cdns) > > @@ -90,32 +120,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 > >
diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c index 9643b905e2d8..9092b6f4d7c4 100644 --- a/drivers/usb/cdns3/host.c +++ b/drivers/usb/cdns3/host.c @@ -24,11 +24,41 @@ #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_suspend_quirk(struct usb_hcd *hcd) +{ + if (pm_runtime_status_suspended(hcd->self.controller)) + return 0; + + 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, }; static int __cdns_host_init(struct cdns *cdns) @@ -90,32 +120,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);