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....@intel.com>
>> ---
>> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
