Re: [PATCH V2 1/4] USB: Set usb port's DeviceRemovable according acpi information in EHCI

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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?

> +	}
> +
>  	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
> 
--
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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux