Re: [PATCH 1/1] usb: xhci: handle Config Error Change (CEC) in xhci driver

["Lu, Baolu" <baolu.lu@linux.intel.com>]

Hi Alan,

Do you have any comments for this patch?

Thanks,
  Baolu

On 03/06/2015 04:12 PM, Lu Baolu wrote:
> Linux xHCI driver doesn't report and handle port cofig error change.
> If Port Configure Error for root hub port occurs, CEC bit in PORTSC
> would be set by xHC and remains 1. This happends when the root port
> fails to configure its link partner, e.g. the port fails to exchange
> port capabilities information using Port Capability LMPs.
>
> Then the Port Status Change Events will be blocked until all status
> change bits(CEC is one of the change bits) are cleared('0') (refer to
> xHCI spec 4.19.2). Otherwise, the port status change event for this
> root port will not be generated anymore, then root port would look
> like dead for user and can't be recovered until a Host Controller
> Reset(HCRST).
>
> This patch is to check CEC bit in PORTSC in xhci_get_port_status()
> and set a Config Error in the return status if CEC is set. This will
> cause a ClearPortFeature request, where CEC bit is cleared in
> xhci_clear_port_change_bit().
>
> [Mathias Nyman contributed the idea. The commit log is based on patch
> posted at http://marc.info/?l=linux-kernel&m=142323612321434&w=2]
>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Cc: stable <stable@vger.kernel.org> # v3.2+
> ---
>   drivers/usb/host/xhci-hub.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index a7865c4..0827d7c 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -387,6 +387,10 @@ static void xhci_clear_port_change_bit(struct xhci_hcd *xhci, u16 wValue,
>   		status = PORT_PLC;
>   		port_change_bit = "link state";
>   		break;
> +	case USB_PORT_FEAT_C_PORT_CONFIG_ERROR:
> +		status = PORT_CEC;
> +		port_change_bit = "config error";
> +		break;
>   	default:
>   		/* Should never happen */
>   		return;
> @@ -588,6 +592,8 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd,
>   			status |= USB_PORT_STAT_C_LINK_STATE << 16;
>   		if ((raw_port_status & PORT_WRC))
>   			status |= USB_PORT_STAT_C_BH_RESET << 16;
> +		if ((raw_port_status & PORT_CEC))
> +			status |= USB_PORT_STAT_C_CONFIG_ERROR << 16;
>   	}
>   
>   	if (hcd->speed != HCD_USB3) {
> @@ -1005,6 +1011,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
>   		case USB_PORT_FEAT_C_OVER_CURRENT:
>   		case USB_PORT_FEAT_C_ENABLE:
>   		case USB_PORT_FEAT_C_PORT_LINK_STATE:
> +		case USB_PORT_FEAT_C_PORT_CONFIG_ERROR:
>   			xhci_clear_port_change_bit(xhci, wValue, wIndex,
>   					port_array[wIndex], temp);
>   			break;
> @@ -1069,7 +1076,7 @@ int xhci_hub_status_data(struct usb_hcd *hcd, char *buf)
>   	 */
>   	status = bus_state->resuming_ports;
>   
> -	mask = PORT_CSC | PORT_PEC | PORT_OCC | PORT_PLC | PORT_WRC;
> +	mask = PORT_CSC | PORT_PEC | PORT_OCC | PORT_PLC | PORT_WRC | PORT_CEC;
>   
>   	spin_lock_irqsave(&xhci->lock, flags);
>   	/* For each port, did anything change?  If so, set that bit in buf. */