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

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

[Lu Baolu]

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. */
-- 
2.1.0

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

[Lu, Baolu]

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. */

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

[Alan Stern]

On Sun, 8 Mar 2015, Lu, Baolu wrote:

> Hi Alan,
> 
> Do you have any comments for this patch?

No comments.

Alan Stern

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

[Mathias Nyman]

On 06.03.2015 10:12, 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+

Looks good, I'll add it to the queue,

-Mathias