[PATCH 1/2] usb: Prefer firmware values when determining whether a port is removable

[PATCH 1/2] usb: Prefer firmware values when determining whether a port is removable

[Matthew Garrett]

Windows appears to pay more attention to the ACPI values than any hub
configuration, so prefer the firmware's opinion on whether a port is
fixed or removable before falling back to the hub values.

Signed-off-by: Matthew Garrett <mjg59@coreos.com>
---
 drivers/usb/core/hub.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index d7c3d5a..ac33fdd 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2350,6 +2350,23 @@ static void set_usb_port_removable(struct usb_device *udev)
 
 	hub = usb_hub_to_struct_hub(udev->parent);
 
+	/*
+	 * If the platform firmware has provided information about a port,
+	 * use that to determine whether it's removable.
+	 */
+	switch (hub->ports[udev->portnum - 1]->connect_type) {
+	case USB_PORT_CONNECT_TYPE_HOT_PLUG:
+		udev->removable = USB_DEVICE_REMOVABLE;
+		return;
+	case USB_PORT_CONNECT_TYPE_HARD_WIRED:
+		udev->removable = USB_DEVICE_FIXED;
+		return;
+	}
+
+	/*
+	 * Otherwise, check whether the hub knows whether a port is removable
+	 * or not
+	 */
 	wHubCharacteristics = le16_to_cpu(hub->descriptor->wHubCharacteristics);
 
 	if (!(wHubCharacteristics & HUB_CHAR_COMPOUND))
@@ -2369,21 +2386,6 @@ static void set_usb_port_removable(struct usb_device *udev)
 	else
 		udev->removable = USB_DEVICE_FIXED;
 
-	/*
-	 * Platform firmware may have populated an alternative value for
-	 * removable.  If the parent port has a known connect_type use
-	 * that instead.
-	 */
-	switch (hub->ports[udev->portnum - 1]->connect_type) {
-	case USB_PORT_CONNECT_TYPE_HOT_PLUG:
-		udev->removable = USB_DEVICE_REMOVABLE;
-		break;
-	case USB_PORT_CONNECT_TYPE_HARD_WIRED:
-		udev->removable = USB_DEVICE_FIXED;
-		break;
-	default: /* use what was set above */
-		break;
-	}
 }
 
 /**
-- 
2.3.4

[PATCH 2/2] usb: Set unused ports to "fixed" rather than "unknown"

[Matthew Garrett]

The Microsoft document "Using ACPI to Configure USB Ports on a Computer"
makes it clear that the removable flag will be cleared on ports that are
marked as unused by the firmware. Handle this case to match.

Signed-off-by: Matthew Garrett <mjg59@coreos.com>
---
 drivers/usb/core/hub.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index ac33fdd..8166bcc 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2359,8 +2359,11 @@ static void set_usb_port_removable(struct usb_device *udev)
 		udev->removable = USB_DEVICE_REMOVABLE;
 		return;
 	case USB_PORT_CONNECT_TYPE_HARD_WIRED:
+	case USB_PORT_NOT_USED:
 		udev->removable = USB_DEVICE_FIXED;
 		return;
+	default:
+		break;
 	}
 
 	/*
-- 
2.3.4

Re: [PATCH 1/2] usb: Prefer firmware values when determining whether a port is removable

[Greg KH]

On Wed, Apr 08, 2015 at 04:36:00PM -0700, Matthew Garrett wrote:
> Windows appears to pay more attention to the ACPI values than any hub
> configuration, so prefer the firmware's opinion on whether a port is
> fixed or removable before falling back to the hub values.
> 
> Signed-off-by: Matthew Garrett <mjg59@coreos.com>
> ---
>  drivers/usb/core/hub.c | 32 +++++++++++++++++---------------
>  1 file changed, 17 insertions(+), 15 deletions(-)

I like your trust in firmware :)

Anyway, is this all a Windows 8 requirement?  If so, I'll feel
comfortable making these changes, otherwise we are at the mercy of the
bios people to randomly get things right, and we all know how often that
works...

thanks,

greg k-h

Re: [PATCH 1/2] usb: Prefer firmware values when determining whether a port is removable

[Matthew Garrett]

On Thu, Apr 9, 2015 at 2:31 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> Anyway, is this all a Windows 8 requirement?  If so, I'll feel
> comfortable making these changes, otherwise we are at the mercy of the
> bios people to randomly get things right, and we all know how often that
> works...

It's covered by
System.Fundamentals.SystemUSB.XHCIControllersMustHaveEmbeddedInfo - I
guess there's an argument for having the order depend on the
controller, but I suspect that anything with _PLD and _UPC objects
will be fine. The alternative means we're just relying on a different
set of firmware information (ie, did the hardware vendor bother
setting the hub flags), so...


-- 
Matthew Garrett | matthew.garrett@coreos.com