On Mon, 2014-03-03 at 16:36 -0500, Alan Stern wrote:
> On Fri, 28 Feb 2014, Dan Williams wrote:
>
> > Once usb-acpi has set the port's connect type the usb_device's
> > ->removable attribute can be set in the standard location
> > set_usb_port_removable().
> >
> > This also changes behavior in the case where the firmware says that the
> > port connect type is unknown. In that case just use the default setting
> > determined from the hub descriptor.
> >
> > Suggested-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
>
> Good clean-up.
>
> > @@ -155,36 +156,15 @@ static struct acpi_device *usb_acpi_find_companion(struct device *dev)
> > */
> > if (is_usb_device(dev)) {
> > udev = to_usb_device(dev);
> > - port1 = udev->portnum;
> > - if (udev->parent) {
> > - struct usb_hub *hub;
> > -
> > - hub = usb_hub_to_struct_hub(udev->parent);
> > - /*
> > - * According usb port's connect type to set usb device's
> > - * removability.
> > - */
> > - switch (hub->ports[port1 - 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:
> > - udev->removable = USB_DEVICE_REMOVABLE_UNKNOWN;
> > - break;
> > - }
> > -
> > + if (udev->parent)
> > return NULL;
> > - }
> >
> > /* root hub's parent is the usb hcd. */
> > - return acpi_find_child_device(ACPI_COMPANION(dev->parent),
> > - port1, false);
> > + port1 = udev->portnum;
> > + adev = ACPI_COMPANION(dev->parent);
> > + return acpi_find_child_device(adev, port1, false);
>
> Does this entirely make sense? As far as I can see, port1 is never
> going to be anything other than 0. After all, root hubs don't have
> upstream ports. I don't know what ACPI expects to see here.
>
ACPI wants to know which child to find and in this case there is only
ever one child of the hcd, the root hub. It will always be at
ACPI-address zero relative to the hcd device. Indeed it does not make
sense to derive the port number given we are always looking for '0'. So
while we are making the other cleanup might as well remove this
irrelevant lookup of the port number.
8<---------
Subject: usb: cleanup setting udev->removable from port_dev->connect_type
From: Dan Williams <dan.j.williams@xxxxxxxxx>
Once usb-acpi has set the port's connect type the usb_device's
->removable attribute can be set in the standard location
set_usb_port_removable().
This also changes behavior in the case where the firmware says that the
port connect type is unknown. In that case just use the default setting
determined from the hub descriptor.
Note, we no longer pass udev->portnum to acpi_find_child_device() in the
root hub case since:
1/ the usb-core sets this to zero
2/ acpi always expects zero
...just pass zero.
Suggested-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
---
drivers/usb/core/hub.c | 22 +++++++++++++++++-----
drivers/usb/core/usb-acpi.c | 34 ++++++----------------------------
2 files changed, 23 insertions(+), 33 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 71e1a0a9f222..67de9b63a98d 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2283,6 +2283,22 @@ static void set_usb_port_removable(struct usb_device *udev)
udev->removable = USB_DEVICE_REMOVABLE;
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;
+ }
}
/**
@@ -2352,11 +2368,7 @@ int usb_new_device(struct usb_device *udev)
device_enable_async_suspend(&udev->dev);
- /*
- * check whether the hub marks this port as non-removable. Do it
- * now so that platform-specific data can override it in
- * device_add()
- */
+ /* check whether the hub or firmware marks this port as non-removable */
if (udev->parent)
set_usb_port_removable(udev);
diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index f91ef0220066..ce0f4e8d81cb 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -136,8 +136,8 @@ out:
static struct acpi_device *usb_acpi_find_companion(struct device *dev)
{
- int port1;
struct usb_device *udev;
+ struct acpi_device *adev;
acpi_handle *parent_handle;
/*
@@ -155,40 +155,18 @@ static struct acpi_device *usb_acpi_find_companion(struct device *dev)
*/
if (is_usb_device(dev)) {
udev = to_usb_device(dev);
- port1 = udev->portnum;
- if (udev->parent) {
- struct usb_hub *hub;
-
- hub = usb_hub_to_struct_hub(udev->parent);
- /*
- * According usb port's connect type to set usb device's
- * removability.
- */
- switch (hub->ports[port1 - 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:
- udev->removable = USB_DEVICE_REMOVABLE_UNKNOWN;
- break;
- }
-
+ if (udev->parent)
return NULL;
- }
- /* root hub's parent is the usb hcd. */
- return acpi_find_child_device(ACPI_COMPANION(dev->parent),
- port1, false);
+ /* root hub is only child (_ADR=0) under it's parent, the hcd */
+ adev = ACPI_COMPANION(dev->parent);
+ return acpi_find_child_device(adev, 0, false);
} else if (is_usb_port(dev)) {
struct usb_port *port_dev = to_usb_port(dev);
- struct acpi_device *adev = NULL;
+ int port1 = port_dev->portnum;
/* Get the struct usb_device point of port's hub */
udev = to_usb_device(dev->parent->parent);
- port1 = port_dev->portnum;
/*
* The root hub ports' parent is the root hub. The non-root-hub
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
