[PATCH v2] ASPM: Use msleep instead of cpu_relax during link retraining

[PATCH v2] ASPM: Use msleep instead of cpu_relax during link retraining

[Andrew Patterson]

ASPM: Use msleep instead of cpu_relax during link retraining

The cpu_relax() function can be a noop on certain architectures
like IA-64 when CPU threads are disabled, so use msleep instead
during link retraining busy/wait loop.

Introduce define LINK_RETRAIN_TIMEOUT instead of hard-coding
timeout in pcie_aspm_configure_common_clock.

Use time_after() to avoid jiffy wraparound when checking for expired
timeout.

After timeout expires, recheck link status register link training bit
instead of checking for expired timeout to avoid possible false positive.

Note that Matthew Wilcox came up with the first rough version of
this patch.

Signed-off-by: Andrew Patterson <andrew.patterson@hp.com>
---

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 4d8e2c7..e67eaa2 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -17,6 +17,7 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/jiffies.h>
+#include <linux/delay.h>
 #include <linux/pci-aspm.h>
 #include "../pci.h"
 
@@ -75,6 +76,8 @@ static const char *policy_str[] = {
 	[POLICY_POWERSAVE] = "powersave"
 };
 
+#define LINK_RETRAIN_TIMEOUT HZ
+
 static int policy_to_aspm_state(struct pci_dev *pdev)
 {
 	struct pcie_link_state *link_state = pdev->link_state;
@@ -238,16 +241,19 @@ static void pcie_aspm_configure_common_clock(struct pci_dev *pdev)
 	pci_write_config_word(pdev, pos + PCI_EXP_LNKCTL, reg16);
 
 	/* Wait for link training end */
-	/* break out after waiting for 1 second */
+	/* break out after waiting for timeout */
 	start_jiffies = jiffies;
-	while ((jiffies - start_jiffies) < HZ) {
+	for (;;) {
 		pci_read_config_word(pdev, pos + PCI_EXP_LNKSTA, &reg16);
 		if (!(reg16 & PCI_EXP_LNKSTA_LT))
 			break;
-		cpu_relax();
+		if (time_after(start_jiffies,
+			       start_jiffies + LINK_RETRAIN_TIMEOUT))
+			break;
+		msleep(1);
 	}
 	/* training failed -> recover */
-	if ((jiffies - start_jiffies) >= HZ) {
+	if (reg16 & PCI_EXP_LNKSTA_LT) {
 		dev_printk (KERN_ERR, &pdev->dev, "ASPM: Could not configure"
 			    " common clock\n");
 		i = 0;

Re: [PATCH v2] ASPM: Use msleep instead of cpu_relax during link retraining

[Joe Perches]

On Mon, 2009-01-05 at 16:07 -0700, Andrew Patterson wrote:
> ASPM: Use msleep instead of cpu_relax during link retraining
> Signed-off-by: Andrew Patterson <andrew.patterson@hp.com>
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 4d8e2c7..e67eaa2 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
[]
> @@ -238,16 +241,19 @@ static void pcie_aspm_configure_common_clock(struct pci_dev *pdev)
>  	pci_write_config_word(pdev, pos + PCI_EXP_LNKCTL, reg16);
>  
>  	/* Wait for link training end */
> -	/* break out after waiting for 1 second */
> +	/* break out after waiting for timeout */
>  	start_jiffies = jiffies;
> -	while ((jiffies - start_jiffies) < HZ) {
> +	for (;;) {
>  		pci_read_config_word(pdev, pos + PCI_EXP_LNKSTA, &reg16);
>  		if (!(reg16 & PCI_EXP_LNKSTA_LT))
>  			break;
> -		cpu_relax();
> +		if (time_after(start_jiffies,
> +			       start_jiffies + LINK_RETRAIN_TIMEOUT))
> +			break;
> +		msleep(1);
>  	}

if (time_after(jiffies, start_jiffies + LINK_RETRAIN_TIMEOUT))

Re: [PATCH v2] ASPM: Use msleep instead of cpu_relax during link retraining

[Joe Perches]

I didn't notice you already sent v3.