On Sat, 17 Nov 2012, 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 change ehci root hub descriptors and set usb hub port's DeviceRemovable
> in the hub_configure(). When hub descriptor request is issued at first time,
> usb port device isn't created and usb port is not bound with acpi. So first
> hub descriptor request is not changed based on ACPI information. After usb
> port device being created, set hub port's DeviceRemovable according ACPI
> information and this also works for non-root hub.
I think this patch, and patch 2/10, can both be improved.
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1546,6 +1546,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);
> + }
> +
You've got all this code here, and you added copies of the same thing
to ehci-hcd and xhci-hcd. That's wasteful, and it also ignores the
other HCDs.
Instead, how about sticking the new code into a separate function:
void hub_adjust_DeviceRemovable(struct usb_device *hdev,
struct usb_hub_descriptor *desc);
Call this new function here and at the appropriate place in
hcd.c:rh_call_control(). Then no modifications would be needed in
ehci-hcd or xhci-hcd.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
