Re: [PATCH v7 03/16] usb: cleanup setting udev->removable from port_dev->connect_type

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

 



On Wed, 19 Mar 2014, Dan Williams wrote:

> Once usb-acpi has set the port's connect type the usb_device's
> ->removable attribute can be set in the standard location
> set_usb_port_removable().
> 
> This also changes behavior in the case where the firmware says that the
> port connect type is unknown.  In that case just use the default setting
> determined from the hub descriptor.
> 
> Note, we no longer pass udev->portnum to acpi_find_child_device() in the
> root hub case since:
> 1/ the usb-core sets this to zero
> 2/ acpi always expects zero
> ...just pass zero.
> 
> Suggested-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>

This is the one patch I haven't ACKed yet from among the first ten.

> @@ -155,40 +155,18 @@ static struct acpi_device *usb_acpi_find_companion(struct device *dev)
>  	 */
>  	if (is_usb_device(dev)) {
>  		udev = to_usb_device(dev);
> -		port1 = udev->portnum;
> -		if (udev->parent) {
> -			struct usb_hub *hub;
> -
> -			hub = usb_hub_to_struct_hub(udev->parent);
> -			/*
> -			 * According usb port's connect type to set usb device's
> -			 * removability.
> -			 */
> -			switch (hub->ports[port1 - 1]->connect_type) {
> -			case USB_PORT_CONNECT_TYPE_HOT_PLUG:
> -				udev->removable = USB_DEVICE_REMOVABLE;
> -				break;
> -			case USB_PORT_CONNECT_TYPE_HARD_WIRED:
> -				udev->removable = USB_DEVICE_FIXED;
> -				break;
> -			default:
> -				udev->removable = USB_DEVICE_REMOVABLE_UNKNOWN;
> -				break;
> -			}
> -
> +		if (udev->parent)
>  			return NULL;
> -		}

We should try to find the device's ACPI companion here.  That can be
added in a later patch, though.

>  
> -		/* root hub's parent is the usb hcd. */
> -		return acpi_find_child_device(ACPI_COMPANION(dev->parent),
> -				port1, false);
> +		/* root hub is only child (_ADR=0) under it's parent, the hcd */

s/it's/its/

Remember, "it's" is a contraction of "it is".

s/the hcd/the HC/

"hcd" refers either to a data structure (struct usb_hcd) or a Host
Controller Driver.  But here you are referring to the host controller 
itself, or the ACPI device corresponding to it.

> +		adev = ACPI_COMPANION(dev->parent);
> +		return acpi_find_child_device(adev, 0, false);
>  	} else if (is_usb_port(dev)) {
>  		struct usb_port *port_dev = to_usb_port(dev);
> -		struct acpi_device *adev = NULL;
> +		int port1 = port_dev->portnum;
>  
>  		/* Get the struct usb_device point of port's hub */
>  		udev = to_usb_device(dev->parent->parent);
> -		port1 = port_dev->portnum;
>  
>  		/*
>  		 * The root hub ports' parent is the root hub. The non-root-hub

The rest of this comment says "The non-root-hub ports' parent is the
parent hub port which the hub is connected to."  I'm not sure that's 
really true.

In the ACPI code example shown in section D.1.1 of the xHCI spec, the
device path for port IP1 (which is a port on the integrated hub) is
USB0.RHUB.HCP2.IHUB.IP1.  Thus, IP1's parent is the integrated hub,
IHUB, not the hub's upstream port, HCP2.

This also should be fixed later.  For now, after you fix the spelling 
errors above, you can add

Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>

Alan Stern

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