Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 1/2] USB: core: Make port power cycle a seperate helper function
@ 2019-11-29 17:41 Kai-Heng Feng
  2019-11-29 17:41 ` [PATCH v2 2/2] USB: core: Attempt power cycle port when it's in eSS.Disabled state Kai-Heng Feng
  0 siblings, 1 reply; 4+ messages in thread
From: Kai-Heng Feng @ 2019-11-29 17:41 UTC (permalink / raw)
  To: gregkh; +Cc: stern, linux-usb, linux-kernel, Kai-Heng Feng

Add a new function, hub_port_power_cycle() to power cycle port's power.
It'll be used by a following patch.

In addition to that, check the return value of usb_hub_set_port_power(),
so we don't need to wait if the set power operation fails.

Furthermore, remove parameter *hdev from usb_hub_set_port_power(), since
we can get *hdev from *hub directly.

Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v2:
- No change.

 drivers/usb/core/hub.c  | 30 ++++++++++++++++++++++++------
 drivers/usb/core/hub.h  |  3 +--
 drivers/usb/core/port.c |  4 ++--
 3 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 1709895387b9..6b6cd76ac5e6 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -817,9 +817,9 @@ static void hub_tt_work(struct work_struct *work)
  *
  * Return: 0 if successful. A negative error code otherwise.
  */
-int usb_hub_set_port_power(struct usb_device *hdev, struct usb_hub *hub,
-			   int port1, bool set)
+int usb_hub_set_port_power(struct usb_hub *hub, int port1, bool set)
 {
+	struct usb_device *hdev = hub->hdev;
 	int ret;
 
 	if (set)
@@ -2739,6 +2739,27 @@ static bool hub_port_warm_reset_required(struct usb_hub *hub, int port1,
 		|| link_state == USB_SS_PORT_LS_COMP_MOD;
 }
 
+static void hub_port_power_cycle(struct usb_hub *hub, int port1)
+{
+	struct usb_port *port_dev = hub->ports[port1  - 1];
+	int ret;
+
+	ret = usb_hub_set_port_power(hub, port1, false);
+	if (ret) {
+		dev_info(&port_dev->dev, "failed to disable port power\n");
+		return;
+	}
+
+	msleep(2 * hub_power_on_good_delay(hub));
+	ret = usb_hub_set_port_power(hub, port1, true);
+	if (ret) {
+		dev_info(&port_dev->dev, "failed to enable port power\n");
+		return;
+	}
+
+	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)
 {
@@ -5216,10 +5237,7 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
 		/* When halfway through our retry count, power-cycle the port */
 		if (i == (SET_CONFIG_TRIES / 2) - 1) {
 			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));
+			hub_port_power_cycle(hub, port1);
 		}
 	}
 	if (hub->hdev->parent ||
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index a9e24e4b8df1..325a55637a6f 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -113,8 +113,7 @@ extern int usb_hub_create_port_device(struct usb_hub *hub,
 		int port1);
 extern void usb_hub_remove_port_device(struct usb_hub *hub,
 		int port1);
-extern int usb_hub_set_port_power(struct usb_device *hdev, struct usb_hub *hub,
-		int port1, bool set);
+extern int usb_hub_set_port_power(struct usb_hub *hub, int port1, bool set);
 extern struct usb_hub *usb_hub_to_struct_hub(struct usb_device *hdev);
 extern int hub_port_debounce(struct usb_hub *hub, int port1,
 		bool must_be_connected);
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index bbbb35fa639f..0fc6f24c6da1 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -214,7 +214,7 @@ static int usb_port_runtime_resume(struct device *dev)
 		pm_runtime_get_sync(&peer->dev);
 
 	usb_autopm_get_interface(intf);
-	retval = usb_hub_set_port_power(hdev, hub, port1, true);
+	retval = usb_hub_set_port_power(hub, port1, true);
 	msleep(hub_power_on_good_delay(hub));
 	if (udev && !retval) {
 		/*
@@ -267,7 +267,7 @@ static int usb_port_runtime_suspend(struct device *dev)
 		return -EBUSY;
 
 	usb_autopm_get_interface(intf);
-	retval = usb_hub_set_port_power(hdev, hub, port1, false);
+	retval = usb_hub_set_port_power(hub, port1, false);
 	usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_CONNECTION);
 	if (!port_dev->is_superspeed)
 		usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 2/2] USB: core: Attempt power cycle port when it's in eSS.Disabled state
  2019-11-29 17:41 [PATCH v2 1/2] USB: core: Make port power cycle a seperate helper function Kai-Heng Feng
@ 2019-11-29 17:41 ` Kai-Heng Feng
  2019-12-11 13:45   ` Kai-Heng Feng
  0 siblings, 1 reply; 4+ messages in thread
From: Kai-Heng Feng @ 2019-11-29 17:41 UTC (permalink / raw)
  To: gregkh; +Cc: stern, linux-usb, linux-kernel, Kai-Heng Feng

On Dell TB16, Realtek USB ethernet (r8152) connects to an SMSC hub which
then connects to ASMedia xHCI's root hub:

/:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 5000M
    |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/7p, 5000M
            |__ Port 2: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M

Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 004 Device 002: ID 0424:5537 Standard Microsystems Corp. USB5537B
Bus 004 Device 003: ID 0bda:8153 Realtek Semiconductor Corp. RTL8153 Gigabit Ethernet Adapter

The SMSC hub may disconnect after system resume from suspend. When this
happens, the reset resume attempt fails, and the last resort to disable
the port and see something comes up later, also fails.

When the issue occurs, the link state stays in eSS.Disabled state
despite the warm reset attempts. Accoding to spec this can be caused by
invalid VBus, after some expiremets, the SMSC hub can be brought back
after a powercycle.

So let's power cycle the port at the end of reset resume attempt, if
it's in eSS.Disabled state.

Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v2:
- Lower dev_info() to dev_dbg().

 drivers/usb/core/hub.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 6b6cd76ac5e6..a2e6001046f5 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2739,6 +2739,18 @@ static bool hub_port_warm_reset_required(struct usb_hub *hub, int port1,
 		|| link_state == USB_SS_PORT_LS_COMP_MOD;
 }
 
+static bool hub_port_power_cycle_required(struct usb_hub *hub, int port1,
+		u16 portstatus)
+{
+	u16 link_state;
+
+	if (!hub_is_superspeed(hub->hdev))
+		return false;
+
+	link_state = portstatus & USB_PORT_STAT_LINK_STATE;
+	return link_state == USB_SS_PORT_LS_SS_DISABLED;
+}
+
 static void hub_port_power_cycle(struct usb_hub *hub, int port1)
 {
 	struct usb_port *port_dev = hub->ports[port1  - 1];
@@ -3601,6 +3613,10 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
 	if (status < 0) {
 		dev_dbg(&udev->dev, "can't resume, status %d\n", status);
 		hub_port_logical_disconnect(hub, port1);
+		if (hub_port_power_cycle_required(hub, port1, portstatus)) {
+			dev_dbg(&udev->dev, "device in disabled state, attempt power cycle\n");
+			hub_port_power_cycle(hub, port1);
+		}
 	} else  {
 		/* Try to enable USB2 hardware LPM */
 		usb_enable_usb2_hardware_lpm(udev);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 2/2] USB: core: Attempt power cycle port when it's in eSS.Disabled state
  2019-11-29 17:41 ` [PATCH v2 2/2] USB: core: Attempt power cycle port when it's in eSS.Disabled state Kai-Heng Feng
@ 2019-12-11 13:45   ` Kai-Heng Feng
  2019-12-11 15:08     ` Alan Stern
  0 siblings, 1 reply; 4+ messages in thread
From: Kai-Heng Feng @ 2019-12-11 13:45 UTC (permalink / raw)
  To: Greg KH; +Cc: stern, linux-usb, linux-kernel



> On Nov 30, 2019, at 01:41, Kai-Heng Feng <kai.heng.feng@canonical.com> wrote:
> 
> On Dell TB16, Realtek USB ethernet (r8152) connects to an SMSC hub which
> then connects to ASMedia xHCI's root hub:
> 
> /:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 5000M
>    |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/7p, 5000M
>            |__ Port 2: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M
> 
> Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> Bus 004 Device 002: ID 0424:5537 Standard Microsystems Corp. USB5537B
> Bus 004 Device 003: ID 0bda:8153 Realtek Semiconductor Corp. RTL8153 Gigabit Ethernet Adapter
> 
> The SMSC hub may disconnect after system resume from suspend. When this
> happens, the reset resume attempt fails, and the last resort to disable
> the port and see something comes up later, also fails.
> 
> When the issue occurs, the link state stays in eSS.Disabled state
> despite the warm reset attempts. Accoding to spec this can be caused by
> invalid VBus, after some expiremets, the SMSC hub can be brought back
> after a powercycle.
> 
> So let's power cycle the port at the end of reset resume attempt, if
> it's in eSS.Disabled state.

If there's no objection, can this fix be reviewed and merged?

Kai-Heng

> 
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> v2:
> - Lower dev_info() to dev_dbg().
> 
> drivers/usb/core/hub.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 6b6cd76ac5e6..a2e6001046f5 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2739,6 +2739,18 @@ static bool hub_port_warm_reset_required(struct usb_hub *hub, int port1,
> 		|| link_state == USB_SS_PORT_LS_COMP_MOD;
> }
> 
> +static bool hub_port_power_cycle_required(struct usb_hub *hub, int port1,
> +		u16 portstatus)
> +{
> +	u16 link_state;
> +
> +	if (!hub_is_superspeed(hub->hdev))
> +		return false;
> +
> +	link_state = portstatus & USB_PORT_STAT_LINK_STATE;
> +	return link_state == USB_SS_PORT_LS_SS_DISABLED;
> +}
> +
> static void hub_port_power_cycle(struct usb_hub *hub, int port1)
> {
> 	struct usb_port *port_dev = hub->ports[port1  - 1];
> @@ -3601,6 +3613,10 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
> 	if (status < 0) {
> 		dev_dbg(&udev->dev, "can't resume, status %d\n", status);
> 		hub_port_logical_disconnect(hub, port1);
> +		if (hub_port_power_cycle_required(hub, port1, portstatus)) {
> +			dev_dbg(&udev->dev, "device in disabled state, attempt power cycle\n");
> +			hub_port_power_cycle(hub, port1);
> +		}
> 	} else  {
> 		/* Try to enable USB2 hardware LPM */
> 		usb_enable_usb2_hardware_lpm(udev);
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 2/2] USB: core: Attempt power cycle port when it's in eSS.Disabled state
  2019-12-11 13:45   ` Kai-Heng Feng
@ 2019-12-11 15:08     ` Alan Stern
  0 siblings, 0 replies; 4+ messages in thread
From: Alan Stern @ 2019-12-11 15:08 UTC (permalink / raw)
  To: Kai-Heng Feng, Mathias Nyman; +Cc: Greg KH, USB list, Kernel development list

On Wed, 11 Dec 2019, Kai-Heng Feng wrote:

> 
> 
> > On Nov 30, 2019, at 01:41, Kai-Heng Feng <kai.heng.feng@canonical.com> wrote:
> > 
> > On Dell TB16, Realtek USB ethernet (r8152) connects to an SMSC hub which
> > then connects to ASMedia xHCI's root hub:
> > 
> > /:  Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/2p, 5000M
> >    |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/7p, 5000M
> >            |__ Port 2: Dev 3, If 0, Class=Vendor Specific Class, Driver=r8152, 5000M
> > 
> > Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
> > Bus 004 Device 002: ID 0424:5537 Standard Microsystems Corp. USB5537B
> > Bus 004 Device 003: ID 0bda:8153 Realtek Semiconductor Corp. RTL8153 Gigabit Ethernet Adapter
> > 
> > The SMSC hub may disconnect after system resume from suspend. When this
> > happens, the reset resume attempt fails, and the last resort to disable
> > the port and see something comes up later, also fails.
> > 
> > When the issue occurs, the link state stays in eSS.Disabled state
> > despite the warm reset attempts. Accoding to spec this can be caused by
> > invalid VBus, after some expiremets, the SMSC hub can be brought back
> > after a powercycle.
> > 
> > So let's power cycle the port at the end of reset resume attempt, if
> > it's in eSS.Disabled state.
> 
> If there's no objection, can this fix be reviewed and merged?
> 
> Kai-Heng

The patch is okay as far as I'm concerned, since it doesn't do anything 
for non-SuperSpeed connections.  It should be reviewed by Mathias.

Alan Stern

> > 
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > ---
> > v2:
> > - Lower dev_info() to dev_dbg().
> > 
> > drivers/usb/core/hub.c | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> > 
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 6b6cd76ac5e6..a2e6001046f5 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -2739,6 +2739,18 @@ static bool hub_port_warm_reset_required(struct usb_hub *hub, int port1,
> > 		|| link_state == USB_SS_PORT_LS_COMP_MOD;
> > }
> > 
> > +static bool hub_port_power_cycle_required(struct usb_hub *hub, int port1,
> > +		u16 portstatus)
> > +{
> > +	u16 link_state;
> > +
> > +	if (!hub_is_superspeed(hub->hdev))
> > +		return false;
> > +
> > +	link_state = portstatus & USB_PORT_STAT_LINK_STATE;
> > +	return link_state == USB_SS_PORT_LS_SS_DISABLED;
> > +}
> > +
> > static void hub_port_power_cycle(struct usb_hub *hub, int port1)
> > {
> > 	struct usb_port *port_dev = hub->ports[port1  - 1];
> > @@ -3601,6 +3613,10 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg)
> > 	if (status < 0) {
> > 		dev_dbg(&udev->dev, "can't resume, status %d\n", status);
> > 		hub_port_logical_disconnect(hub, port1);
> > +		if (hub_port_power_cycle_required(hub, port1, portstatus)) {
> > +			dev_dbg(&udev->dev, "device in disabled state, attempt power cycle\n");
> > +			hub_port_power_cycle(hub, port1);
> > +		}
> > 	} else  {
> > 		/* Try to enable USB2 hardware LPM */
> > 		usb_enable_usb2_hardware_lpm(udev);
> > -- 
> > 2.17.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-29 17:41 [PATCH v2 1/2] USB: core: Make port power cycle a seperate helper function Kai-Heng Feng
2019-11-29 17:41 ` [PATCH v2 2/2] USB: core: Attempt power cycle port when it's in eSS.Disabled state Kai-Heng Feng
2019-12-11 13:45   ` Kai-Heng Feng
2019-12-11 15:08     ` Alan Stern

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git