From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Message-ID: <54FC141B.3030206@linux.intel.com> Date: Sun, 08 Mar 2015 17:19:23 +0800 From: "Lu, Baolu" MIME-Version: 1.0 To: Mathias Nyman , Greg Kroah-Hartman , Alan Stern CC: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, stable Subject: Re: [PATCH 1/1] usb: xhci: handle Config Error Change (CEC) in xhci driver References: <1425629552-17808-1-git-send-email-baolu.lu@linux.intel.com> In-Reply-To: <1425629552-17808-1-git-send-email-baolu.lu@linux.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: 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 > Cc: stable # 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. */