Re: [PATCH] USB: xhci: add port test modes support for USB2

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

 



Hi,

Guoqing Zhang <guoqing.zhang@xxxxxxxxx> writes:
> For USB2 ports, the port test modes TEST_J_State, Test_K_State,
> Test_Packet and Test_SE0_NAK can be enabled as described in USB2
> spec.
>
> Signed-off-by: Guoqing Zhang <guoqing.zhang@xxxxxxxxx>
> ---
>  drivers/usb/host/xhci-hub.c | 55 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index d61fcc4..ccc558c 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -873,16 +873,18 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
>  		u16 wIndex, char *buf, u16 wLength)
>  {
>  	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
> +	struct xhci_command *command;
>  	int max_ports;
>  	unsigned long flags;
>  	u32 temp, status;
> -	int retval = 0;
> +	int i, retval = 0;
>  	__le32 __iomem **port_array;
>  	int slot_id;
>  	struct xhci_bus_state *bus_state;
>  	u16 link_state = 0;
>  	u16 wake_mask = 0;
>  	u16 timeout = 0;
> +	u16 selector = 0;
>  
>  	max_ports = xhci_get_ports(hcd, &port_array);
>  	bus_state = &xhci->bus_state[hcd_index(hcd)];
> @@ -956,6 +958,8 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
>  			link_state = (wIndex & 0xff00) >> 3;
>  		if (wValue == USB_PORT_FEAT_REMOTE_WAKE_MASK)
>  			wake_mask = wIndex & 0xff00;
> +		if (wValue == USB_PORT_FEAT_TEST)
> +			selector = (wIndex & 0xff00) >> 8;
>  		/* The MSB of wIndex is the U1/U2 timeout */
>  		timeout = (wIndex & 0xff00) >> 8;
>  		wIndex &= 0xff;
> @@ -1014,7 +1018,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
>  			break;
>  		case USB_PORT_FEAT_LINK_STATE:
>  			temp = readl(port_array[wIndex]);
> -

trailing change

>  			/* Disable port */
>  			if (link_state == USB_SS_PORT_LS_SS_DISABLED) {
>  				xhci_dbg(xhci, "Disable port %d\n", wIndex);
> @@ -1131,6 +1134,54 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
>  			temp |= PORT_U2_TIMEOUT(timeout);
>  			writel(temp, port_array[wIndex] + PORTPMSC);
>  			break;
> +		case USB_PORT_FEAT_TEST:
> +			/* 4.19.6 Port Test Modes (USB2 Test Mode) */
> +			if (hcd->speed != HCD_USB2)
> +				goto error;
> +			/* FIXME: Test_Force_Enable case to be implemented */
> +			if (!selector || selector > 4)

Why don't you use our macros?

if (selector < TEST_J || selector > TEST_PACKET)
	goto error;

> +				goto error;
> +			/* Disable all Device Slots */
> +			for (i = 0; i < MAX_HC_SLOTS; i++) {
> +				if (!xhci->dcbaa->dev_context_ptrs[i])
> +					continue;
> +				command = xhci_alloc_command(xhci, false,
> +						false, GFP_KERNEL);
> +				if (!command)
> +					return -ENOMEM;
> +				if (xhci_queue_slot_control(xhci, command,
> +						TRB_DISABLE_SLOT, i)) {
> +					xhci_err(xhci,
> +						"Disable slot[%d] fail!\n", i);
> +						goto error;
> +					}
> +				xhci_dbg(xhci, "Disable Slot[%d].\n", i);
> +			}
> +			/* Put all ports to the Disable state by clear PP */
> +			xhci_dbg(xhci, "Disable all port (PP = 0)\n");
> +			for (i = 0; i < max_ports; i++) {
> +				temp = readl(port_array[i]);
> +				temp &= ~PORT_POWER;
> +				writel(temp, port_array[i]);
> +			}
> +			/* Stop the controller */
> +			xhci_dbg(xhci, "Stop controller\n");
> +			temp = readl(&xhci->op_regs->command);
> +			temp &= ~CMD_RUN;
> +			writel(temp, &xhci->op_regs->command);
> +			if (xhci_handshake(&xhci->op_regs->status,
> +				STS_HALT, STS_HALT, XHCI_MAX_HALT_USEC)) {
> +				xhci_warn(xhci, "Stop controller timeout\n");
> +				return -ETIMEDOUT;
> +			}
> +			/* Disable runtime PM for test mode */
> +			pm_runtime_forbid(hcd->self.controller);
> +			/* Set PORTPMSC.PTC field for selected test mode */
> +			xhci_dbg(xhci, "Enter Test Mode: %d\n", selector);
> +			temp = readl(port_array[wIndex] + PORTPMSC);
> +			temp |= selector << 28;
> +			writel(temp, port_array[wIndex] + PORTPMSC);
> +			break;

this really looks to be duplicated code from elsewhere. Are you sure you
don't have functions already written with parts of this code? Perhaps
just refactor them a bit so they are callable from this context?

> Intel Finland Oy
> Registered Address: PL 281, 00181 Helsinki 
> Business Identity Code: 0357606 - 4 
> Domiciled in Helsinki 
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.

oh no :-( We don't send emails with this type of notice to public
mailing lists. You really, really need to get this note out of here.

-- 
balbi

Attachment: signature.asc
Description: PGP signature


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

  Powered by Linux