| Message ID | 1642607498-8458-1-git-send-email-quic_ppratap@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] usb: hub: Power cycle root hub if CSC is set during hub_port_reset | expand |
On Wed, Jan 19, 2022 at 09:21:38PM +0530, Pratham Pratap wrote: > When a FS device is following a suspend-reset-enumeration-data > transfer sequence, Such a sequence never happens. The kernel always does a resume before a reset, if the port is suspended. I seem to recall reading something in the USB-2 spec saying that this was required (i.e., a suspended port should never be reset without being resumed first), but now I can't find it. > sometimes it goes back in suspend just after reset > without the link entering L0. This is seen in only when the following > scenarios are met: > - SOF and EOR happens at the same clock cycle > - UTMI line state should transition from SE0 to K at the same clock > cycle(if the UTMI line state transition from SE0 to J at the same > clock cycle then problem is not seen) This is not true in general. You're talking about a specific host controller with a specific PHY, aren't you? If you are, you should say so. > Attemting a power cycle of the root hub recovers the problem described. > To identify the issue, PLS goes to disabled state followed by CSC bit > being set(because of CCS status change). > > Signed-off-by: Pratham Pratap <quic_ppratap@quicinc.com> > --- > v1: > Problem is seen on core emulation setup with eUSB2 PHY test chip. > This failure is seen only in full speed host mode usecase with all > available eUSB2 repeater randomly in 1 out of 5000 to 6000 iterations. This information should be part of the patch description. And it should be mentioned in a comment in the code. > As of now, we don't have any SOC with eUSB2 PHY on which this fix can > be tested. If you can't test the patch, why are you submitting it? > > drivers/usb/core/hub.c | 34 ++++++++++++++++++++++++++-------- > drivers/usb/host/xhci-plat.c | 3 +++ > include/linux/usb/hcd.h | 1 + > include/uapi/linux/usb/ch11.h | 1 + > 4 files changed, 31 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index 47a1c8b..6a65092 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -2834,10 +2834,20 @@ static bool hub_port_warm_reset_required(struct usb_hub *hub, int port1, > || link_state == USB_SS_PORT_LS_COMP_MOD; > } > > +static void usb_hub_port_power_cycle(struct usb_device *hdev, struct usb_hub *hub, int port1) > +{ > + dev_info(&hub->ports[port1 - 1]->dev, "attempt power cycle\n"); > + usb_hub_set_port_power(hdev, hub, port1, false); > + msleep(2 * hub_power_on_good_delay(hub)); > + usb_hub_set_port_power(hdev, hub, port1, true); > + msleep(hub_power_on_good_delay(hub)); > +} > + > static int hub_port_wait_reset(struct usb_hub *hub, int port1, > struct usb_device *udev, unsigned int delay, bool warm) > { > int delay_time, ret; > + struct usb_hcd *hcd = bus_to_hcd(udev->bus); udev may be a NULL pointer. You can use hub->hdev instead. > u16 portstatus; > u16 portchange; > u32 ext_portstatus = 0; > @@ -2887,8 +2897,21 @@ static int hub_port_wait_reset(struct usb_hub *hub, int port1, > return -ENOTCONN; > > /* Device went away? */ > - if (!(portstatus & USB_PORT_STAT_CONNECTION)) > + if (!(portstatus & USB_PORT_STAT_CONNECTION)) { > + /* > + * When a FS device is following a suspend-reset-enumeration-data_transfer > + * sequence, sometimes it goes back in suspend just after reset without the > + * link entering L0. To fix this when CSC bit is set(because of CCS status > + * change) power cycle the root hub. > + */ > + if (udev->reset_resume && (!udev->parent && hcd->fs_suspend_reset) && Unnecessary extra parentheses. Also, at this point udev can be a NULL pointer; you must test it before dereferencing it. Furthermore, udev->parent must always be set; you probably meant to write !hub->hdev->parent. > + (portstatus & USB_PORT_STAT_CSC)) { You probably mean portchange here, not portstatus. There is no CSC bit in portstatus. > + usb_hub_port_power_cycle(hdev, hub, port1); > + return -EAGAIN; > + } > + > return -ENOTCONN; > + } > > /* Retry if connect change is set but status is still connected. > * A USB 3.0 connection may bounce if multiple warm resets were issued, > @@ -5393,13 +5416,8 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus, > break; > > /* When halfway through our retry count, power-cycle the port */ > - if (i == (PORT_INIT_TRIES - 1) / 2) { > - dev_info(&port_dev->dev, "attempt power cycle\n"); > - usb_hub_set_port_power(hdev, hub, port1, false); > - msleep(2 * hub_power_on_good_delay(hub)); > - usb_hub_set_port_power(hdev, hub, port1, true); > - msleep(hub_power_on_good_delay(hub)); > - } > + if (i == (PORT_INIT_TRIES - 1) / 2) > + usb_hub_port_power_cycle(hdev, hub, port1); > } > if (hub->hdev->parent || > !hcd->driver->port_handed_over || > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c > index c1edcc9..607c4f0 100644 > --- a/drivers/usb/host/xhci-plat.c > +++ b/drivers/usb/host/xhci-plat.c > @@ -342,6 +342,9 @@ static int xhci_plat_probe(struct platform_device *pdev) > hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node); > xhci->shared_hcd->tpl_support = hcd->tpl_support; > > + hcd->fs_suspend_reset = of_property_read_bool(sysdev->of_node, "fs-suspend-reset"); > + xhci->shared_hcd->fs_suspend_reset = hcd->fs_suspend_reset; > + > if (priv) { > ret = xhci_priv_plat_setup(hcd); > if (ret) > diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h > index 548a028..05ccbc8 100644 > --- a/include/linux/usb/hcd.h > +++ b/include/linux/usb/hcd.h > @@ -172,6 +172,7 @@ struct usb_hcd { > unsigned tpl_support:1; /* OTG & EH TPL support */ > unsigned cant_recv_wakeups:1; > /* wakeup requests from downstream aren't received */ > + unsigned fs_suspend_reset:1; /* fs suspend reset bug */ > > unsigned int irq; /* irq allocated */ > void __iomem *regs; /* device memory/io */ > diff --git a/include/uapi/linux/usb/ch11.h b/include/uapi/linux/usb/ch11.h > index fb0cd24..576bbf9 100644 > --- a/include/uapi/linux/usb/ch11.h > +++ b/include/uapi/linux/usb/ch11.h > @@ -135,6 +135,7 @@ struct usb_port_status { > #define USB_PORT_STAT_TEST 0x0800 > #define USB_PORT_STAT_INDICATOR 0x1000 > /* bits 13 to 15 are reserved */ > +#define USB_PORT_STAT_CSC 0x20000 This doesn't make any sense; you are defining a name for bit 17 in wPortStatus, which is a 16-bit field. Are you sure you don't want to use USB_PORT_STAT_C_CONNECTION, which is already defined a little bit lower down in this file? > > /* > * Additions to wPortStatus bit field from USB 3.0 > -- > 2.7.4 Alan Stern
Hi Pratham,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on usb/usb-testing]
[also build test ERROR on linus/master peter-chen-usb/for-usb-next v5.16 next-20220118]
[cannot apply to balbi-usb/testing/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Pratham-Pratap/usb-hub-Power-cycle-root-hub-if-CSC-is-set-during-hub_port_reset/20220119-235321
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: alpha-allmodconfig (https://download.01.org/0day-ci/archive/20220120/202201200108.JJYDWfTS-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/ec4d5f04b268fc19d3b5d2843d1889531dafd22f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Pratham-Pratap/usb-hub-Power-cycle-root-hub-if-CSC-is-set-during-hub_port_reset/20220119-235321
git checkout ec4d5f04b268fc19d3b5d2843d1889531dafd22f
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash drivers/usb/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/usb/core/hub.c: In function 'hub_port_wait_reset':
>> drivers/usb/core/hub.c:2907:25: error: 'struct usb_device' has no member named 'reset_resume'
2907 | if (udev->reset_resume && (!udev->parent && hcd->fs_suspend_reset) &&
| ^~
>> drivers/usb/core/hub.c:2909:50: error: 'hdev' undeclared (first use in this function); did you mean 'udev'?
2909 | usb_hub_port_power_cycle(hdev, hub, port1);
| ^~~~
| udev
drivers/usb/core/hub.c:2909:50: note: each undeclared identifier is reported only once for each function it appears in
vim +2907 drivers/usb/core/hub.c
2845
2846 static int hub_port_wait_reset(struct usb_hub *hub, int port1,
2847 struct usb_device *udev, unsigned int delay, bool warm)
2848 {
2849 int delay_time, ret;
2850 struct usb_hcd *hcd = bus_to_hcd(udev->bus);
2851 u16 portstatus;
2852 u16 portchange;
2853 u32 ext_portstatus = 0;
2854
2855 for (delay_time = 0;
2856 delay_time < HUB_RESET_TIMEOUT;
2857 delay_time += delay) {
2858 /* wait to give the device a chance to reset */
2859 msleep(delay);
2860
2861 /* read and decode port status */
2862 if (hub_is_superspeedplus(hub->hdev))
2863 ret = hub_ext_port_status(hub, port1,
2864 HUB_EXT_PORT_STATUS,
2865 &portstatus, &portchange,
2866 &ext_portstatus);
2867 else
2868 ret = hub_port_status(hub, port1, &portstatus,
2869 &portchange);
2870 if (ret < 0)
2871 return ret;
2872
2873 /*
2874 * The port state is unknown until the reset completes.
2875 *
2876 * On top of that, some chips may require additional time
2877 * to re-establish a connection after the reset is complete,
2878 * so also wait for the connection to be re-established.
2879 */
2880 if (!(portstatus & USB_PORT_STAT_RESET) &&
2881 (portstatus & USB_PORT_STAT_CONNECTION))
2882 break;
2883
2884 /* switch to the long delay after two short delay failures */
2885 if (delay_time >= 2 * HUB_SHORT_RESET_TIME)
2886 delay = HUB_LONG_RESET_TIME;
2887
2888 dev_dbg(&hub->ports[port1 - 1]->dev,
2889 "not %sreset yet, waiting %dms\n",
2890 warm ? "warm " : "", delay);
2891 }
2892
2893 if ((portstatus & USB_PORT_STAT_RESET))
2894 return -EBUSY;
2895
2896 if (hub_port_warm_reset_required(hub, port1, portstatus))
2897 return -ENOTCONN;
2898
2899 /* Device went away? */
2900 if (!(portstatus & USB_PORT_STAT_CONNECTION)) {
2901 /*
2902 * When a FS device is following a suspend-reset-enumeration-data_transfer
2903 * sequence, sometimes it goes back in suspend just after reset without the
2904 * link entering L0. To fix this when CSC bit is set(because of CCS status
2905 * change) power cycle the root hub.
2906 */
> 2907 if (udev->reset_resume && (!udev->parent && hcd->fs_suspend_reset) &&
2908 (portstatus & USB_PORT_STAT_CSC)) {
> 2909 usb_hub_port_power_cycle(hdev, hub, port1);
2910 return -EAGAIN;
2911 }
2912
2913 return -ENOTCONN;
2914 }
2915
2916 /* Retry if connect change is set but status is still connected.
2917 * A USB 3.0 connection may bounce if multiple warm resets were issued,
2918 * but the device may have successfully re-connected. Ignore it.
2919 */
2920 if (!hub_is_superspeed(hub->hdev) &&
2921 (portchange & USB_PORT_STAT_C_CONNECTION)) {
2922 usb_clear_port_feature(hub->hdev, port1,
2923 USB_PORT_FEAT_C_CONNECTION);
2924 return -EAGAIN;
2925 }
2926
2927 if (!(portstatus & USB_PORT_STAT_ENABLE))
2928 return -EBUSY;
2929
2930 if (!udev)
2931 return 0;
2932
2933 if (hub_is_superspeedplus(hub->hdev)) {
2934 /* extended portstatus Rx and Tx lane count are zero based */
2935 udev->rx_lanes = USB_EXT_PORT_RX_LANES(ext_portstatus) + 1;
2936 udev->tx_lanes = USB_EXT_PORT_TX_LANES(ext_portstatus) + 1;
2937 udev->ssp_rate = get_port_ssp_rate(hub->hdev, ext_portstatus);
2938 } else {
2939 udev->rx_lanes = 1;
2940 udev->tx_lanes = 1;
2941 udev->ssp_rate = USB_SSP_GEN_UNKNOWN;
2942 }
2943 if (hub_is_wusb(hub))
2944 udev->speed = USB_SPEED_WIRELESS;
2945 else if (udev->ssp_rate != USB_SSP_GEN_UNKNOWN)
2946 udev->speed = USB_SPEED_SUPER_PLUS;
2947 else if (hub_is_superspeed(hub->hdev))
2948 udev->speed = USB_SPEED_SUPER;
2949 else if (portstatus & USB_PORT_STAT_HIGH_SPEED)
2950 udev->speed = USB_SPEED_HIGH;
2951 else if (portstatus & USB_PORT_STAT_LOW_SPEED)
2952 udev->speed = USB_SPEED_LOW;
2953 else
2954 udev->speed = USB_SPEED_FULL;
2955 return 0;
2956 }
2957
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Alan, Thanks for reviewing. On 1/19/2022 10:59 PM, Alan Stern wrote: > On Wed, Jan 19, 2022 at 09:21:38PM +0530, Pratham Pratap wrote: >> When a FS device is following a suspend-reset-enumeration-data >> transfer sequence, > Such a sequence never happens. The kernel always does a resume before a > reset, if the port is suspended. > > I seem to recall reading something in the USB-2 spec saying that this was > required (i.e., a suspended port should never be reset without being > resumed first), but now I can't find it. What if resume failed and the driver ends up in reset resume path? Also, there can be a possibility of some class driver preforming reset-resume(based on quirks). > >> sometimes it goes back in suspend just after reset >> without the link entering L0. This is seen in only when the following >> scenarios are met: >> - SOF and EOR happens at the same clock cycle >> - UTMI line state should transition from SE0 to K at the same clock >> cycle(if the UTMI line state transition from SE0 to J at the same >> clock cycle then problem is not seen) > This is not true in general. You're talking about a specific host > controller with a specific PHY, aren't you? If you are, you should say > so. Yes, this is seen with dwc controller and SNPS eUSB2 PHY >> Attemting a power cycle of the root hub recovers the problem described. >> To identify the issue, PLS goes to disabled state followed by CSC bit >> being set(because of CCS status change). >> >> Signed-off-by: Pratham Pratap <quic_ppratap@quicinc.com> >> --- >> v1: >> Problem is seen on core emulation setup with eUSB2 PHY test chip. >> This failure is seen only in full speed host mode usecase with all >> available eUSB2 repeater randomly in 1 out of 5000 to 6000 iterations. > This information should be part of the patch description. And it should > be mentioned in a comment in the code. Will make it as part of next patch version. >> As of now, we don't have any SOC with eUSB2 PHY on which this fix can >> be tested. > If you can't test the patch, why are you submitting it? This patch is tested in emulation environment not in SW world since we don't have any SOC yet to test it. >> drivers/usb/core/hub.c | 34 ++++++++++++++++++++++++++-------- >> drivers/usb/host/xhci-plat.c | 3 +++ >> include/linux/usb/hcd.h | 1 + >> include/uapi/linux/usb/ch11.h | 1 + >> 4 files changed, 31 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c >> index 47a1c8b..6a65092 100644 >> --- a/drivers/usb/core/hub.c >> +++ b/drivers/usb/core/hub.c >> @@ -2834,10 +2834,20 @@ static bool hub_port_warm_reset_required(struct usb_hub *hub, int port1, >> || link_state == USB_SS_PORT_LS_COMP_MOD; >> } >> >> +static void usb_hub_port_power_cycle(struct usb_device *hdev, struct usb_hub *hub, int port1) >> +{ >> + dev_info(&hub->ports[port1 - 1]->dev, "attempt power cycle\n"); >> + usb_hub_set_port_power(hdev, hub, port1, false); >> + msleep(2 * hub_power_on_good_delay(hub)); >> + usb_hub_set_port_power(hdev, hub, port1, true); >> + msleep(hub_power_on_good_delay(hub)); >> +} >> + >> static int hub_port_wait_reset(struct usb_hub *hub, int port1, >> struct usb_device *udev, unsigned int delay, bool warm) >> { >> int delay_time, ret; >> + struct usb_hcd *hcd = bus_to_hcd(udev->bus); > udev may be a NULL pointer. You can use hub->hdev instead. sure, will take care of it in v2 >> u16 portstatus; >> u16 portchange; >> u32 ext_portstatus = 0; >> @@ -2887,8 +2897,21 @@ static int hub_port_wait_reset(struct usb_hub *hub, int port1, >> return -ENOTCONN; >> >> /* Device went away? */ >> - if (!(portstatus & USB_PORT_STAT_CONNECTION)) >> + if (!(portstatus & USB_PORT_STAT_CONNECTION)) { >> + /* >> + * When a FS device is following a suspend-reset-enumeration-data_transfer >> + * sequence, sometimes it goes back in suspend just after reset without the >> + * link entering L0. To fix this when CSC bit is set(because of CCS status >> + * change) power cycle the root hub. >> + */ >> + if (udev->reset_resume && (!udev->parent && hcd->fs_suspend_reset) && > Unnecessary extra parentheses. Also, at this point udev can be a NULL > pointer; you must test it before dereferencing it. > > Furthermore, udev->parent must always be set; you probably meant to write > !hub->hdev->parent. > >> + (portstatus & USB_PORT_STAT_CSC)) { > You probably mean portchange here, not portstatus. There is no CSC bit > in portstatus. yes >> + usb_hub_port_power_cycle(hdev, hub, port1); >> + return -EAGAIN; >> + } >> + >> return -ENOTCONN; >> + } >> >> /* Retry if connect change is set but status is still connected. >> * A USB 3.0 connection may bounce if multiple warm resets were issued, >> @@ -5393,13 +5416,8 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus, >> break; >> >> /* When halfway through our retry count, power-cycle the port */ >> - if (i == (PORT_INIT_TRIES - 1) / 2) { >> - dev_info(&port_dev->dev, "attempt power cycle\n"); >> - usb_hub_set_port_power(hdev, hub, port1, false); >> - msleep(2 * hub_power_on_good_delay(hub)); >> - usb_hub_set_port_power(hdev, hub, port1, true); >> - msleep(hub_power_on_good_delay(hub)); >> - } >> + if (i == (PORT_INIT_TRIES - 1) / 2) >> + usb_hub_port_power_cycle(hdev, hub, port1); >> } >> if (hub->hdev->parent || >> !hcd->driver->port_handed_over || >> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c >> index c1edcc9..607c4f0 100644 >> --- a/drivers/usb/host/xhci-plat.c >> +++ b/drivers/usb/host/xhci-plat.c >> @@ -342,6 +342,9 @@ static int xhci_plat_probe(struct platform_device *pdev) >> hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node); >> xhci->shared_hcd->tpl_support = hcd->tpl_support; >> >> + hcd->fs_suspend_reset = of_property_read_bool(sysdev->of_node, "fs-suspend-reset"); >> + xhci->shared_hcd->fs_suspend_reset = hcd->fs_suspend_reset; >> + >> if (priv) { >> ret = xhci_priv_plat_setup(hcd); >> if (ret) >> diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h >> index 548a028..05ccbc8 100644 >> --- a/include/linux/usb/hcd.h >> +++ b/include/linux/usb/hcd.h >> @@ -172,6 +172,7 @@ struct usb_hcd { >> unsigned tpl_support:1; /* OTG & EH TPL support */ >> unsigned cant_recv_wakeups:1; >> /* wakeup requests from downstream aren't received */ >> + unsigned fs_suspend_reset:1; /* fs suspend reset bug */ >> >> unsigned int irq; /* irq allocated */ >> void __iomem *regs; /* device memory/io */ >> diff --git a/include/uapi/linux/usb/ch11.h b/include/uapi/linux/usb/ch11.h >> index fb0cd24..576bbf9 100644 >> --- a/include/uapi/linux/usb/ch11.h >> +++ b/include/uapi/linux/usb/ch11.h >> @@ -135,6 +135,7 @@ struct usb_port_status { >> #define USB_PORT_STAT_TEST 0x0800 >> #define USB_PORT_STAT_INDICATOR 0x1000 >> /* bits 13 to 15 are reserved */ >> +#define USB_PORT_STAT_CSC 0x20000 > This doesn't make any sense; you are defining a name for bit 17 in > wPortStatus, which is a 16-bit field. Are you sure you don't want to use > USB_PORT_STAT_C_CONNECTION, which is already defined a little bit lower > down in this file? Ah! my bad. I must have overlooked it. Definitely we can use USB_PORT_STAT_C_CONNECTION. Will take care of it in v2. >> >> /* >> * Additions to wPortStatus bit field from USB 3.0 >> -- >> 2.7.4 > Alan Stern Thanks, Pratham
On Fri, Jan 21, 2022 at 06:39:59PM +0530, Pratham Pratap wrote: > Hi Alan, > > Thanks for reviewing. > > On 1/19/2022 10:59 PM, Alan Stern wrote: > > On Wed, Jan 19, 2022 at 09:21:38PM +0530, Pratham Pratap wrote: > > > When a FS device is following a suspend-reset-enumeration-data > > > transfer sequence, > > Such a sequence never happens. The kernel always does a resume before a > > reset, if the port is suspended. > > > > I seem to recall reading something in the USB-2 spec saying that this was > > required (i.e., a suspended port should never be reset without being > > resumed first), but now I can't find it. > What if resume failed and the driver ends up in reset resume path? Use The Source, Luke. If the resume fails then usb_reset_and_verify_device() will return -EINVAL immediately, without performing a reset: if (udev->state == USB_STATE_NOTATTACHED || udev->state == USB_STATE_SUSPENDED) { dev_dbg(&udev->dev, "device reset not allowed in state %d\n", udev->state); return -EINVAL; } > Also, there can be a possibility of some class driver preforming > reset-resume(based on quirks). Despite its name, reset-resume involves performing a resume first and a reset second. You can tell from the fact that the reset isn't performed until finish_port_resume() tests the udev->reset_resume flag, which occurs after the port has been resumed. (Furthermore, the reset part of a reset-resume is carried out by calling usb_reset_and_verify_device(), which as pointed out above, won't do anything if the device is still suspended.) Alan Stern
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 47a1c8b..6a65092 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2834,10 +2834,20 @@ static bool hub_port_warm_reset_required(struct usb_hub *hub, int port1, || link_state == USB_SS_PORT_LS_COMP_MOD; } +static void usb_hub_port_power_cycle(struct usb_device *hdev, struct usb_hub *hub, int port1) +{ + dev_info(&hub->ports[port1 - 1]->dev, "attempt power cycle\n"); + usb_hub_set_port_power(hdev, hub, port1, false); + msleep(2 * hub_power_on_good_delay(hub)); + usb_hub_set_port_power(hdev, hub, port1, true); + msleep(hub_power_on_good_delay(hub)); +} + static int hub_port_wait_reset(struct usb_hub *hub, int port1, struct usb_device *udev, unsigned int delay, bool warm) { int delay_time, ret; + struct usb_hcd *hcd = bus_to_hcd(udev->bus); u16 portstatus; u16 portchange; u32 ext_portstatus = 0; @@ -2887,8 +2897,21 @@ static int hub_port_wait_reset(struct usb_hub *hub, int port1, return -ENOTCONN; /* Device went away? */ - if (!(portstatus & USB_PORT_STAT_CONNECTION)) + if (!(portstatus & USB_PORT_STAT_CONNECTION)) { + /* + * When a FS device is following a suspend-reset-enumeration-data_transfer + * sequence, sometimes it goes back in suspend just after reset without the + * link entering L0. To fix this when CSC bit is set(because of CCS status + * change) power cycle the root hub. + */ + if (udev->reset_resume && (!udev->parent && hcd->fs_suspend_reset) && + (portstatus & USB_PORT_STAT_CSC)) { + usb_hub_port_power_cycle(hdev, hub, port1); + return -EAGAIN; + } + return -ENOTCONN; + } /* Retry if connect change is set but status is still connected. * A USB 3.0 connection may bounce if multiple warm resets were issued, @@ -5393,13 +5416,8 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus, break; /* When halfway through our retry count, power-cycle the port */ - if (i == (PORT_INIT_TRIES - 1) / 2) { - dev_info(&port_dev->dev, "attempt power cycle\n"); - usb_hub_set_port_power(hdev, hub, port1, false); - msleep(2 * hub_power_on_good_delay(hub)); - usb_hub_set_port_power(hdev, hub, port1, true); - msleep(hub_power_on_good_delay(hub)); - } + if (i == (PORT_INIT_TRIES - 1) / 2) + usb_hub_port_power_cycle(hdev, hub, port1); } if (hub->hdev->parent || !hcd->driver->port_handed_over || diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index c1edcc9..607c4f0 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -342,6 +342,9 @@ static int xhci_plat_probe(struct platform_device *pdev) hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node); xhci->shared_hcd->tpl_support = hcd->tpl_support; + hcd->fs_suspend_reset = of_property_read_bool(sysdev->of_node, "fs-suspend-reset"); + xhci->shared_hcd->fs_suspend_reset = hcd->fs_suspend_reset; + if (priv) { ret = xhci_priv_plat_setup(hcd); if (ret) diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index 548a028..05ccbc8 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -172,6 +172,7 @@ struct usb_hcd { unsigned tpl_support:1; /* OTG & EH TPL support */ unsigned cant_recv_wakeups:1; /* wakeup requests from downstream aren't received */ + unsigned fs_suspend_reset:1; /* fs suspend reset bug */ unsigned int irq; /* irq allocated */ void __iomem *regs; /* device memory/io */ diff --git a/include/uapi/linux/usb/ch11.h b/include/uapi/linux/usb/ch11.h index fb0cd24..576bbf9 100644 --- a/include/uapi/linux/usb/ch11.h +++ b/include/uapi/linux/usb/ch11.h @@ -135,6 +135,7 @@ struct usb_port_status { #define USB_PORT_STAT_TEST 0x0800 #define USB_PORT_STAT_INDICATOR 0x1000 /* bits 13 to 15 are reserved */ +#define USB_PORT_STAT_CSC 0x20000 /* * Additions to wPortStatus bit field from USB 3.0
When a FS device is following a suspend-reset-enumeration-data transfer sequence, sometimes it goes back in suspend just after reset without the link entering L0. This is seen in only when the following scenarios are met: - SOF and EOR happens at the same clock cycle - UTMI line state should transition from SE0 to K at the same clock cycle(if the UTMI line state transition from SE0 to J at the same clock cycle then problem is not seen) Attemting a power cycle of the root hub recovers the problem described. To identify the issue, PLS goes to disabled state followed by CSC bit being set(because of CCS status change). Signed-off-by: Pratham Pratap <quic_ppratap@quicinc.com> --- v1: Problem is seen on core emulation setup with eUSB2 PHY test chip. This failure is seen only in full speed host mode usecase with all available eUSB2 repeater randomly in 1 out of 5000 to 6000 iterations. As of now, we don't have any SOC with eUSB2 PHY on which this fix can be tested. drivers/usb/core/hub.c | 34 ++++++++++++++++++++++++++-------- drivers/usb/host/xhci-plat.c | 3 +++ include/linux/usb/hcd.h | 1 + include/uapi/linux/usb/ch11.h | 1 + 4 files changed, 31 insertions(+), 8 deletions(-)