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


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

  Powered by Linux