On 2012年11月01日 23:54, Sarah Sharp wrote: > On Mon, Oct 29, 2012 at 04:28:38PM +0800, Lan Tianyu wrote: >> ACPI provide "_PLD" and "_UPC" aml methods to describe usb port >> visibility and connectability. This patch is to use those information >> to set usb port's DeviceRemovable. > > You should probably mention you're changing the EHCI roothub descriptors > based on those methods. Hi Sarah: Thanks for your review. I will add in the next version. > > Comments below. > >> Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx> >> --- >> drivers/usb/core/hub.c | 23 +++++++++++++++++++++++ >> drivers/usb/core/usb.h | 4 ---- >> drivers/usb/host/ehci-hub.c | 9 +++++++++ >> include/linux/usb.h | 4 ++++ >> 4 files changed, 36 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c >> index 43ce146..2297fc9 100644 >> --- a/drivers/usb/core/hub.c >> +++ b/drivers/usb/core/hub.c >> @@ -1547,6 +1547,25 @@ static int hub_configure(struct usb_hub *hub, >> dev_err(hub->intfdev, >> "couldn't create port%d device.\n", i + 1); >> >> + if (hub_is_superspeed(hdev)) { >> + for (i = 1; i <= hdev->maxchild; i++) >> + if (hub->ports[i - 1]->connect_type >> + == USB_PORT_CONNECT_TYPE_HARD_WIRED) >> + hub->descriptor->u.hs.DeviceRemovable[i/8] >> + |= 1 << (i%8); >> + } else { >> + u16 port_removable = >> + le16_to_cpu(hub->descriptor->u.ss.DeviceRemovable); >> + >> + for (i = 1; i <= hdev->maxchild; i++) >> + if (hub->ports[i - 1]->connect_type >> + == USB_PORT_CONNECT_TYPE_HARD_WIRED) >> + port_removable |= 1 << i; >> + >> + hub->descriptor->u.ss.DeviceRemovable = >> + cpu_to_le16(port_removable); > > Er, do you have this logic backwards? You're saying "if this a > SuperSpeed hub" then set the *High Speed* hub descriptor Device > Removable bits (u.hs.DeviceRemovable[]), else set the SuperSpeed Device > Removable bits. > > You probably didn't catch this because the ACPI tables didn't describe > any external hubs, and the roothub Device Removable bits are written by > the xHCI driver code, right? Oh. Yeah. Good catching. I usually use lsusb to get DeviceRemovable value which directly get result from xhci driver. > >> + } >> + >> hub_activate(hub, HUB_INIT); >> return 0; >> >> @@ -5192,8 +5211,12 @@ usb_get_hub_port_connect_type(struct usb_device *hdev, int port1) >> { >> struct usb_hub *hub = hdev_to_hub(hdev); >> >> + if (!hub) >> + return USB_PORT_CONNECT_TYPE_UNKNOWN; >> + >> return hub->ports[port1 - 1]->connect_type; >> } >> +EXPORT_SYMBOL_GPL(usb_get_hub_port_connect_type); >> >> #ifdef CONFIG_ACPI >> /** >> diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h >> index 1c528c1..1633f6e 100644 >> --- a/drivers/usb/core/usb.h >> +++ b/drivers/usb/core/usb.h >> @@ -169,10 +169,6 @@ extern void usb_notify_add_device(struct usb_device *udev); >> extern void usb_notify_remove_device(struct usb_device *udev); >> extern void usb_notify_add_bus(struct usb_bus *ubus); >> extern void usb_notify_remove_bus(struct usb_bus *ubus); >> -extern enum usb_port_connect_type >> - usb_get_hub_port_connect_type(struct usb_device *hdev, int port1); >> -extern void usb_set_hub_port_connect_type(struct usb_device *hdev, int port1, >> - enum usb_port_connect_type type); >> >> #ifdef CONFIG_ACPI >> extern int usb_acpi_register(void); >> diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c >> index a7ec827..265525d 100644 >> --- a/drivers/usb/host/ehci-hub.c >> +++ b/drivers/usb/host/ehci-hub.c >> @@ -649,6 +649,7 @@ ehci_hub_descriptor ( >> struct usb_hub_descriptor *desc >> ) { >> int ports = HCS_N_PORTS (ehci->hcs_params); >> + int i; >> u16 temp; >> >> desc->bDescriptorType = 0x29; >> @@ -663,6 +664,14 @@ ehci_hub_descriptor ( >> memset(&desc->u.hs.DeviceRemovable[0], 0, temp); >> memset(&desc->u.hs.DeviceRemovable[temp], 0xff, temp); >> >> + for (i = 1; i <= ports; i++) { >> + if (usb_get_hub_port_connect_type( >> + ehci_to_hcd(ehci)->self.root_hub, i) >> + == USB_PORT_CONNECT_TYPE_HARD_WIRED) >> + desc->u.hs.DeviceRemovable[i/8] |= 1 << (i%8); >> + } >> + >> + >> temp = 0x0008; /* per-port overcurrent reporting */ >> if (HCS_PPC (ehci->hcs_params)) >> temp |= 0x0001; /* per-port power control */ >> diff --git a/include/linux/usb.h b/include/linux/usb.h >> index f51f998..6651648 100644 >> --- a/include/linux/usb.h >> +++ b/include/linux/usb.h >> @@ -577,6 +577,10 @@ static inline struct usb_device *interface_to_usbdev(struct usb_interface *intf) >> return to_usb_device(intf->dev.parent); >> } >> >> +extern enum usb_port_connect_type >> + usb_get_hub_port_connect_type(struct usb_device *hdev, int port1); >> +extern void usb_set_hub_port_connect_type(struct usb_device *hdev, int port1, >> + enum usb_port_connect_type type); >> extern struct usb_device *usb_get_dev(struct usb_device *dev); >> extern void usb_put_dev(struct usb_device *dev); >> extern struct usb_device *usb_hub_find_child(struct usb_device *hdev, >> -- >> 1.7.9.5 >> -- Best regards Tianyu Lan -- 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