Re: [PATCH] xhci: Fix the lack of support for the Handle Port Configure Error

2015-02-11 Thread Mathias Nyman
On 06.02.2015 17:20, Gregory CLEMENT wrote:
 From: Shimmer Huang shimm...@marvell.com
 
 Linux xHCI driver does not check the CEC bit in register PORTSC when
 handling port status events. 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 and clear the CEC bit if it's set to 1
 in function handle_port_status().
 
 [gregory.clem...@free-electrons.com: ported from 3.10 and added more
 explanations(from Shimmer) in the commit log]
 
 Signed-off-by: Guang Shen gs...@marvell.com
 Signed-off-by: Shimmer Huang shimm...@marvell.com
 Signed-off-by: Nadav Haklai nad...@marvell.com
 Reviewed-by: Yehuda Yitschak yehu...@marvell.com
 Signed-off-by: Gregory CLEMENT gregory.clem...@free-electrons.com
 Cc: sta...@vger.kernel.org
 ---
 Hi,
 
 usually I tried to add a kernel version for the stable team, but for
 this patch I don't know since when it makes sens to apply it.
 
 Gregory
 
 
  drivers/usb/host/xhci-ring.c | 7 +++
  1 file changed, 7 insertions(+)
 
 diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
 index e692e769c50c..45d8dd7e07f3 100644
 --- a/drivers/usb/host/xhci-ring.c
 +++ b/drivers/usb/host/xhci-ring.c
 @@ -1541,6 +1541,13 @@ static void handle_port_status(struct xhci_hcd *xhci,
   port_id);
  
   temp = readl(port_array[faked_port_index]);
 +
 + if (temp  PORT_CEC) {
 + xhci_dbg(xhci, port failed to configure its link partner.\n);
 + xhci_test_and_clear_bit(xhci, port_array,
 + faked_port_index, PORT_CEC);
 + }
 +
   if (hcd-state == HC_STATE_SUSPENDED) {
   xhci_dbg(xhci, resume root hub\n);
   usb_hcd_resume_root_hub(hcd);
 

Nice catch,

Looks like we're not handling the Config Error Change (CEC) at all in xhci.

I think we instead need to set a Config Error in the return status of the 
GetPortStatus
requests by setting USB_PORT_FEAT_C_PORT_CONFIG_ERROR in xhci_get_port_status().

This should cause a ClearPortFeature request, where we then can clear the bit in
xhci_clear_port_change_bit()

So going the long way round informing the usb core about the error instead of
immediately clearing the bit.

-Mathias



 
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] xhci: Fix the lack of support for the Handle Port Configure Error

2015-02-06 Thread Gregory CLEMENT
From: Shimmer Huang shimm...@marvell.com

Linux xHCI driver does not check the CEC bit in register PORTSC when
handling port status events. 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 and clear the CEC bit if it's set to 1
in function handle_port_status().

[gregory.clem...@free-electrons.com: ported from 3.10 and added more
explanations(from Shimmer) in the commit log]

Signed-off-by: Guang Shen gs...@marvell.com
Signed-off-by: Shimmer Huang shimm...@marvell.com
Signed-off-by: Nadav Haklai nad...@marvell.com
Reviewed-by: Yehuda Yitschak yehu...@marvell.com
Signed-off-by: Gregory CLEMENT gregory.clem...@free-electrons.com
Cc: sta...@vger.kernel.org
---
Hi,

usually I tried to add a kernel version for the stable team, but for
this patch I don't know since when it makes sens to apply it.

Gregory


 drivers/usb/host/xhci-ring.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index e692e769c50c..45d8dd7e07f3 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1541,6 +1541,13 @@ static void handle_port_status(struct xhci_hcd *xhci,
port_id);
 
temp = readl(port_array[faked_port_index]);
+
+   if (temp  PORT_CEC) {
+   xhci_dbg(xhci, port failed to configure its link partner.\n);
+   xhci_test_and_clear_bit(xhci, port_array,
+   faked_port_index, PORT_CEC);
+   }
+
if (hcd-state == HC_STATE_SUSPENDED) {
xhci_dbg(xhci, resume root hub\n);
usb_hcd_resume_root_hub(hcd);
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html