Message-ID: <20120904214937.GD16847@xanatos>
Date:	Tue, 4 Sep 2012 14:49:37 -0700
From:	Sarah Sharp <sarah.a.sharp@...ux.intel.com>
To:	"Alexis R. Cortes" <alexis.cortes@...com>
Cc:	gregkh@...uxfoundation.org, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org, brian.quach@...com,
	jorge.llamas@...com
Subject: Re: usb: host: xhci: Fix Compliance Mode on SN65LVPE502CP Hardware

Hi Alexis,

You caught me just before I left for vacation, which is why nothing has
been done with this patch.  I think it's fine, and I'll try to apply it
to my tree and send it to Greg tomorrow.

It is a larger patch, but it fixes a bug that's pretty user visible
(dead USB ports), so my inclination is to queue it for stable as well.
Greg, any objections?

Sarah Sharp

On Tue, Sep 04, 2012 at 01:40:42PM -0500, Alexis R. Cortes wrote:
> Hi Sarah/Greg,
> 
> Sorry to bother you, but I was wondering if you have some news concerning my patch for the Compliance Mode issue of our re-driver. Should I submit the patch again?
> 
> Thanks and Best Regards,
> Alexis Cortes.
> 
> -----Original Message-----
> From: linux-usb-owner@...r.kernel.org [mailto:linux-usb-owner@...r.kernel.org] On Behalf Of Alexis R. Cortes
> Sent: Tuesday, August 14, 2012 11:14 AM
> To: sarah.a.sharp@...ux.intel.com
> Cc: gregkh@...uxfoundation.org; linux-usb@...r.kernel.org; linux-kernel@...r.kernel.org; brian.quach@...com; jorge.llamas@...com
> Subject: Re: usb: host: xhci: Fix Compliance Mode on SN65LVPE502CP Hardware
> 
> Hi Sarah,
> 
> I was wondering if you have any news regarding this patch, or if there's something else I need to change on code. I also noticed that I forgot to write '[PATCH]' at the beginning of the patch's subject, Should I resend it?
> 
> Thanks and Best Regards,
> Alexis Cortes.     
> 
> On 8/3/2012 2:00 PM, Alexis R. Cortes wrote:
> > This patch is intended to work around a known issue on the 
> > SN65LVPE502CP USB3.0 re-driver that can delay the negotiation between 
> > a device and the host past the usual handshake timeout.
> > 
> > If that happens on the first insertion, the host controller port will 
> > enter in Compliance Mode and NO port status event will be generated 
> > (as per xHCI Spec) making impossible to detect this event by software. 
> > The port will remain in compliance mode until a warm reset is applied 
> > to it.
> > 
> > As a result of this, the port will seem "dead" to the user and no 
> > device connections or disconnections will be detected.
> > 
> > For solving this, the patch creates a timer which polls every 2 
> > seconds the link state of each host controller's port (this by reading 
> > the PORTSC register) and recovers the port by issuing a Warm reset 
> > every time Compliance mode is detected.
> > 
> > If a xHC USB3.0 port has previously entered to U0, the compliance mode 
> > issue will NOT occur only until system resumes from sleep/hibernate, 
> > therefore, the compliance mode timer is stopped when all xHC USB 3.0 
> > ports have entered U0. The timer is initialized again after each 
> > system resume.
> > 
> > Since the issue is being caused by a pice of hardware, the timer will 
> > be enabled ONLY on those systems that have the SN65LVPE502CP installed 
> > (this patch uses DMI strings for detecting those systems) therefore 
> > making this patch to act as a quirk (XHCI_COMP_MODE_QUIRK has been 
> > added to the xhci stack).
> > 
> > This patch applies for these systems:
> > Vendor: Hewlett-Packard. System Models: Z420, Z620 and Z820.
> > 
> > Signed-off-by: Alexis R. Cortes <alexis.cortes@...com>
> > ---
> >  drivers/usb/host/xhci-hub.c |   42 +++++++++++++++
> >  drivers/usb/host/xhci.c     |  121 +++++++++++++++++++++++++++++++++++++++++++
> >  drivers/usb/host/xhci.h     |    6 ++
> >  3 files changed, 169 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c 
> > index 7b01094..32ca289 100644
> > --- a/drivers/usb/host/xhci-hub.c
> > +++ b/drivers/usb/host/xhci-hub.c
> > @@ -493,11 +493,48 @@ static void xhci_hub_report_link_state(u32 *status, u32 status_reg)
> >  		 * when this bit is set.
> >  		 */
> >  		pls |= USB_PORT_STAT_CONNECTION;
> > +	} else {
> > +		/*
> > +		 * If CAS bit isn't set but the Port is already at
> > +		 * Compliance Mode, fake a connection so the USB core
> > +		 * notices the Compliance state and resets the port.
> > +		 * This resolves an issue generated by the SN65LVPE502CP
> > +		 * in which sometimes the port enters compliance mode
> > +		 * caused by a delay on the host-device negotiation.
> > +		 */
> > +		if (pls == USB_SS_PORT_LS_COMP_MOD)
> > +			pls |= USB_PORT_STAT_CONNECTION;
> >  	}
> > +
> >  	/* update status field */
> >  	*status |= pls;
> >  }
> >  
> > +/*
> > + * Function for Compliance Mode Quirk.
> > + *
> > + * This Function verifies if all xhc USB3 ports have entered U0, if 
> > +so,
> > + * the compliance mode timer is deleted. A port won't enter
> > + * compliance mode if it has previously entered U0.
> > + */
> > +void xhci_del_comp_mod_timer(struct xhci_hcd *xhci, u32 status, u16 
> > +wIndex) {
> > +	u32 all_ports_seen_u0 = ((1 << xhci->num_usb3_ports)-1);
> > +	bool port_in_u0 = ((status & PORT_PLS_MASK) == XDEV_U0);
> > +
> > +	if (!(xhci->quirks & XHCI_COMP_MODE_QUIRK))
> > +		return;
> > +
> > +	if ((xhci->port_status_u0 != all_ports_seen_u0) && port_in_u0) {
> > +		xhci->port_status_u0 |= 1 << wIndex;
> > +		if (xhci->port_status_u0 == all_ports_seen_u0) {
> > +			del_timer_sync(&xhci->comp_mode_recovery_timer);
> > +			xhci_dbg(xhci, "All USB3 ports have entered U0 already!\n");
> > +			xhci_dbg(xhci, "Compliance Mode Recovery Timer Deleted.\n");
> > +		}
> > +	}
> > +}
> > +
> >  int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
> >  		u16 wIndex, char *buf, u16 wLength)  { @@ -645,6 +682,11 @@ int 
> > xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
> >  		/* Update Port Link State for super speed ports*/
> >  		if (hcd->speed == HCD_USB3) {
> >  			xhci_hub_report_link_state(&status, temp);
> > +			/*
> > +			 * Verify if all USB3 Ports Have entered U0 already.
> > +			 * Delete Compliance Mode Timer if so.
> > +			 */
> > +			xhci_del_comp_mod_timer(xhci, temp, wIndex);
> >  		}
> >  		if (bus_state->port_c_suspend & (1 << wIndex))
> >  			status |= 1 << USB_PORT_FEAT_C_SUSPEND; diff --git 
> > a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 
> > a979cd0..347a29f 100644
> > --- a/drivers/usb/host/xhci.c
> > +++ b/drivers/usb/host/xhci.c
> > @@ -26,6 +26,7 @@
> >  #include <linux/module.h>
> >  #include <linux/moduleparam.h>
> >  #include <linux/slab.h>
> > +#include <linux/dmi.h>
> >  
> >  #include "xhci.h"
> >  
> > @@ -397,6 +398,95 @@ static void xhci_msix_sync_irqs(struct xhci_hcd 
> > *xhci)
> >  
> >  #endif
> >  
> > +static void compliance_mode_recovery(unsigned long arg) {
> > +	struct xhci_hcd *xhci;
> > +	struct usb_hcd *hcd;
> > +	u32 temp;
> > +	int i;
> > +
> > +	xhci = (struct xhci_hcd *)arg;
> > +
> > +	for (i = 0; i < xhci->num_usb3_ports; i++) {
> > +		temp = xhci_readl(xhci, xhci->usb3_ports[i]);
> > +		if ((temp & PORT_PLS_MASK) == USB_SS_PORT_LS_COMP_MOD) {
> > +			/*
> > +			 * Compliance Mode Detected. Letting USB Core
> > +			 * handle the Warm Reset
> > +			 */
> > +			xhci_dbg(xhci, "Compliance Mode Detected->Port %d!\n",
> > +					i + 1);
> > +			xhci_dbg(xhci, "Attempting Recovery routine!\n");
> > +			hcd = xhci->shared_hcd;
> > +
> > +			if (hcd->state == HC_STATE_SUSPENDED)
> > +				usb_hcd_resume_root_hub(hcd);
> > +
> > +			usb_hcd_poll_rh_status(hcd);
> > +		}
> > +	}
> > +
> > +	if (xhci->port_status_u0 != ((1 << xhci->num_usb3_ports)-1))
> > +		mod_timer(&xhci->comp_mode_recovery_timer,
> > +			jiffies + msecs_to_jiffies(COMP_MODE_RCVRY_MSECS));
> > +}
> > +
> > +/*
> > + * Quirk to work around issue generated by the SN65LVPE502CP USB3.0 
> > +re-driver
> > + * that causes ports behind that hardware to enter compliance mode sometimes.
> > + * The quirk creates a timer that polls every 2 seconds the link 
> > +state of
> > + * each host controller's port and recovers it by issuing a Warm 
> > +reset
> > + * if Compliance mode is detected, otherwise the port will become 
> > +"dead" (no
> > + * device connections or disconnections will be detected anymore). 
> > +Becasue no
> > + * status event is generated when entering compliance mode (per xhci 
> > +spec),
> > + * this quirk is needed on systems that have the failing hardware installed.
> > + */
> > +static void compliance_mode_recovery_timer_init(struct xhci_hcd 
> > +*xhci) {
> > +	xhci->port_status_u0 = 0;
> > +	init_timer(&xhci->comp_mode_recovery_timer);
> > +
> > +	xhci->comp_mode_recovery_timer.data = (unsigned long) xhci;
> > +	xhci->comp_mode_recovery_timer.function = compliance_mode_recovery;
> > +	xhci->comp_mode_recovery_timer.expires = jiffies +
> > +			msecs_to_jiffies(COMP_MODE_RCVRY_MSECS);
> > +
> > +	set_timer_slack(&xhci->comp_mode_recovery_timer,
> > +			msecs_to_jiffies(COMP_MODE_RCVRY_MSECS));
> > +	add_timer(&xhci->comp_mode_recovery_timer);
> > +	xhci_dbg(xhci, "Compliance Mode Recovery Timer Initialized.\n"); }
> > +
> > +/*
> > + * This function identifies the systems that have installed the 
> > +SN65LVPE502CP
> > + * USB3.0 re-driver and that need the Compliance Mode Quirk.
> > + * Systems:
> > + * Vendor: Hewlett-Packard -> System Models: Z420, Z620 and Z820  */ 
> > +static bool compliance_mode_recovery_timer_quirk_check(void)
> > +{
> > +	const char *dmi_product_name, *dmi_sys_vendor;
> > +
> > +	dmi_product_name = dmi_get_system_info(DMI_PRODUCT_NAME);
> > +	dmi_sys_vendor = dmi_get_system_info(DMI_SYS_VENDOR);
> > +
> > +	if (!(strstr(dmi_sys_vendor, "Hewlett-Packard")))
> > +		return false;
> > +
> > +	if (strstr(dmi_product_name, "Z420") ||
> > +			strstr(dmi_product_name, "Z620") ||
> > +			strstr(dmi_product_name, "Z820"))
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> > +static int xhci_all_ports_seen_u0(struct xhci_hcd *xhci) {
> > +	return (xhci->port_status_u0 == ((1 << xhci->num_usb3_ports)-1)); }
> > +
> > +
> >  /*
> >   * Initialize memory for HCD and xHC (one-time init).
> >   *
> > @@ -420,6 +510,12 @@ int xhci_init(struct usb_hcd *hcd)
> >  	retval = xhci_mem_init(xhci, GFP_KERNEL);
> >  	xhci_dbg(xhci, "Finished xhci_init\n");
> >  
> > +	/* Initializing Compliance Mode Recovery Data If Needed */
> > +	if (compliance_mode_recovery_timer_quirk_check()) {
> > +		xhci->quirks |= XHCI_COMP_MODE_QUIRK;
> > +		compliance_mode_recovery_timer_init(xhci);
> > +	}
> > +
> >  	return retval;
> >  }
> >  
> > @@ -628,6 +724,11 @@ void xhci_stop(struct usb_hcd *hcd)
> >  	del_timer_sync(&xhci->event_ring_timer);
> >  #endif
> >  
> > +	/* Deleting Compliance Mode Recovery Timer */
> > +	if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) &&
> > +			(!(xhci_all_ports_seen_u0(xhci))))
> > +		del_timer_sync(&xhci->comp_mode_recovery_timer);
> > +
> >  	if (xhci->quirks & XHCI_AMD_PLL_FIX)
> >  		usb_amd_dev_put();
> >  
> > @@ -802,6 +903,16 @@ int xhci_suspend(struct xhci_hcd *xhci)
> >  	}
> >  	spin_unlock_irq(&xhci->lock);
> >  
> > +	/*
> > +	 * Deleting Compliance Mode Recovery Timer because the xHCI Host
> > +	 * is about to be suspended.
> > +	 */
> > +	if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) &&
> > +			(!(xhci_all_ports_seen_u0(xhci)))) {
> > +		del_timer_sync(&xhci->comp_mode_recovery_timer);
> > +		xhci_dbg(xhci, "Compliance Mode Recovery Timer Deleted!\n");
> > +	}
> > +
> >  	/* step 5: remove core well power */
> >  	/* synchronize irq when using MSI-X */
> >  	xhci_msix_sync_irqs(xhci);
> > @@ -934,6 +1045,16 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
> >  		usb_hcd_resume_root_hub(hcd);
> >  		usb_hcd_resume_root_hub(xhci->shared_hcd);
> >  	}
> > +
> > +	/*
> > +	 * If system is subject to the Quirk, Compliance Mode Timer needs to
> > +	 * be re-initialized Always after a system resume. Ports are subject
> > +	 * to suffer the Compliance Mode issue again. It doesn't matter if
> > +	 * ports have entered previously to U0 before system's suspension.
> > +	 */
> > +	if (xhci->quirks & XHCI_COMP_MODE_QUIRK)
> > +		compliance_mode_recovery_timer_init(xhci);
> > +
> >  	return retval;
> >  }
> >  #endif	/* CONFIG_PM */
> > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 
> > 55c0785..f1e7874 100644
> > --- a/drivers/usb/host/xhci.h
> > +++ b/drivers/usb/host/xhci.h
> > @@ -1494,6 +1494,7 @@ struct xhci_hcd {
> >  #define XHCI_TRUST_TX_LENGTH	(1 << 10)
> >  #define XHCI_LPM_SUPPORT	(1 << 11)
> >  #define XHCI_INTEL_HOST		(1 << 12)
> > +#define XHCI_COMP_MODE_QUIRK	(1 << 13)
> >  	unsigned int		num_active_eps;
> >  	unsigned int		limit_active_eps;
> >  	/* There are two roothubs to keep track of bus suspend info for */ 
> > @@ -1510,6 +1511,11 @@ struct xhci_hcd {
> >  	unsigned		sw_lpm_support:1;
> >  	/* support xHCI 1.0 spec USB2 hardware LPM */
> >  	unsigned		hw_lpm_support:1;
> > +	/* Compliance Mode Recovery Data */
> > +	struct timer_list	comp_mode_recovery_timer;
> > +	u32			port_status_u0;
> > +/* Compliance Mode Timer Triggered every 2 seconds */ #define 
> > +COMP_MODE_RCVRY_MSECS 2000
> >  };
> >  
> >  /* convert between an HCD pointer and the corresponding EHCI_HCD */
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@...r.kernel.org More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists