Re: [PATCH 3/3 v2] usbcore: use kernel assigned address for devices under xHCI

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

 



Hi Andiry,

I'm trying to test these 3 patches, along with the other four power
management patches.  I couldn't get the fourth power management patch to
apply without editing it a bit.  You said it was based on the MSI work,
but your patch adds an include for linux/pci.h, which the MSI patch
already added.

I'm not sure what these last three patches are based on, as they don't
apply after the first four PM patches are applied, and they don't apply
to my master branch.  I hate to ask, but can you resend the two series,
based on linus' tree?

Sarah Sharp

On Mon, Aug 23, 2010 at 05:34:29PM +0800, Andiry Xu wrote:
> >From e4f5b3086d397348ce29dd706106268a0cdf2b32 Mon Sep 17 00:00:00 2001
> From: Andiry Xu <andiry.xu@xxxxxxx>
> Date: Tue, 17 Aug 2010 15:10:56 +0800
> Subject: [PATCH 3/3] usbcore: use kernel assigned address for devices under xHCI
> 
> xHCI driver uses hardware assigned device address. This may cause device
> address conflict in certain cases.
> 
> Use kernel assigned address for devices under xHCI. Store the xHC assigned
> address locally in xHCI driver.
> 
> Signed-off-by: Andiry Xu <andiry.xu@xxxxxxx>
> ---
>  drivers/usb/core/hub.c  |   27 +++++++++++++--------------
>  drivers/usb/host/xhci.c |    8 ++++----
>  drivers/usb/host/xhci.h |    2 ++
>  3 files changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index d552c50..4ab5982 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2595,16 +2595,14 @@ static int hub_set_address(struct usb_device *udev, int devnum)
>  		return 0;
>  	if (udev->state != USB_STATE_DEFAULT)
>  		return -EINVAL;
> -	if (hcd->driver->address_device) {
> +	if (hcd->driver->address_device)
>  		retval = hcd->driver->address_device(hcd, udev);
> -	} else {
> +	else
>  		retval = usb_control_msg(udev, usb_sndaddr0pipe(),
>  				USB_REQ_SET_ADDRESS, 0, devnum, 0,
>  				NULL, 0, USB_CTRL_SET_TIMEOUT);
> -		if (retval == 0)
> -			update_address(udev, devnum);
> -	}
>  	if (retval == 0) {
> +		update_address(udev, devnum);
>  		/* Device now using proper address. */
>  		usb_set_device_state(udev, USB_STATE_ADDRESS);
>  		usb_ep0_reinit(udev);
> @@ -3096,16 +3094,17 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1,
>  			udev->speed = USB_SPEED_UNKNOWN;
>  
>  		/*
> -		 * xHCI needs to issue an address device command later
> -		 * in the hub_port_init sequence for SS/HS/FS/LS devices.
> +		 * Set the address.
> +		 * Note xHCI needs to issue an address device command later
> +		 * in the hub_port_init sequence for SS/HS/FS/LS devices,
> +		 * and xHC will assign an address to the device. But use
> +		 * kernel assigned address here, to avoid any address conflict
> +		 * issue.
>  		 */
> -		if (!(hcd->driver->flags & HCD_USB3)) {
> -			/* set the address */
> -			choose_address(udev);
> -			if (udev->devnum <= 0) {
> -				status = -ENOTCONN;	/* Don't retry */
> -				goto loop;
> -			}
> +		choose_address(udev);
> +		if (udev->devnum <= 0) {
> +			status = -ENOTCONN;	/* Don't retry */
> +			goto loop;
>  		}
>  
>  		/* reset (non-USB 3.0 devices) and get descriptor */
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index d817505..554b024 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -2348,15 +2348,15 @@ int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev)
>  	 * address given back to us by the HC.
>  	 */
>  	slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->out_ctx);
> -	udev->devnum = (slot_ctx->dev_state & DEV_ADDR_MASK) + 1;
> +	/* Use kernel assigned address for devices; store xHC assigned
> +	 * address locally. */
> +	virt_dev->address = (slot_ctx->dev_state & DEV_ADDR_MASK) + 1;
>  	/* Zero the input context control for later use */
>  	ctrl_ctx = xhci_get_input_control_ctx(xhci, virt_dev->in_ctx);
>  	ctrl_ctx->add_flags = 0;
>  	ctrl_ctx->drop_flags = 0;
>  
> -	xhci_dbg(xhci, "Device address = %d\n", udev->devnum);
> -	/* XXX Meh, not sure if anyone else but choose_address uses this. */
> -	set_bit(udev->devnum, udev->bus->devmap.devicemap);
> +	xhci_dbg(xhci, "Internal device address = %d\n", virt_dev->address);
>  
>  	return 0;
>  }
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 3777233..41d2378 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -748,6 +748,8 @@ struct xhci_virt_device {
>  	int				num_rings_cached;
>  	/* Indicate whether the device is configured */
>  	bool				configured;
> +	/* Store xHC assigned device address */
> +	int				address;
>  #define	XHCI_MAX_RINGS_CACHED	31
>  	struct xhci_virt_ep		eps[31];
>  	struct completion		cmd_completion;
> -- 
> 1.7.0.4
> 
> 
> 
--
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