DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=virtuousgeek.org;
	h=Received:Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References:X-Mailer:Mime-Version:Content-Type:Content-Transfer-Encoding:X-Identified-User;
	b=LkJwgBHeBF+VCBkPWjb7WtfAsx+nbxTY0io+2uoPGD9iNAKnm9Qi8Q4eFWyHJnHrdLzD141f7FMZ+2AQGdMMvvMATUm565WtEHrju8fw/eCHKHwW/fyL/kurk9Ql6mmH;
Date: Wed, 4 Nov 2009 11:03:21 -0800
From: Jesse Barnes <jbarnes@virtuousgeek.org>
To: rdh@east.sun.com
Cc: linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org
Subject: Re: [PATCH] pci/pcie: Avoid unnecessary PCIe link retrains
Message-ID: <20091104110321.5e58e112@jbarnes-piketon>
In-Reply-To: <19184.41676.262206.134000@gargle.gargle.HOWL>
References: <19184.41676.262206.134000@gargle.gargle.HOWL>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2484
Lines: 65

[Cc'ing linux-pci@vger.kernel.org too]

On Tue, 3 Nov 2009 16:38:20 -0500
RDH <rdh@east.sun.com> wrote:

> 
> This patch avoids unnecessary PCIe link retraining sequences within
> the PCIe ASPM common clock setup code by issuing a link retrain
> command only if we are actually changing the PCIe clock configuration.
> 
> Signed-off-by: Robert D. Houk <rdh@East.Sun.COM>
> ---
>  aspm.c |   20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> --- a/drivers/pci/pcie/aspm.c	2009-10-15 20:41:50.000000000
> -0400 +++ b/drivers/pci/pcie/aspm.c	2009-11-02
> 14:29:35.000000000 -0500 @@ -183,6 +183,7 @@ static void
> pcie_aspm_configure_common_c {
>  	int ppos, cpos, same_clock = 1;
>  	u16 reg16, parent_reg, child_reg[8];
> +	u16 lnkctl_ccc_or, lnkctl_ccc_and;
>  	unsigned long start_jiffies;
>  	struct pci_dev *child, *parent = link->pdev;
>  	struct pci_bus *linkbus = parent->subordinate;
> @@ -205,6 +206,25 @@ static void pcie_aspm_configure_common_c
>  	if (!(reg16 & PCI_EXP_LNKSTA_SLC))
>  		same_clock = 0;
>  
> +	/* Check upstream component for Common Clock Config */
> +	pci_read_config_word(parent, ppos + PCI_EXP_LNKCTL, &reg16);
> +	lnkctl_ccc_and = lnkctl_ccc_or = (reg16 &
> PCI_EXP_LNKCTL_CCC); +
> +	/* Scan downstream component for CCC, all functions */
> +	list_for_each_entry(child, &linkbus->devices, bus_list) {
> +		cpos = pci_find_capability(child, PCI_CAP_ID_EXP);
> +		pci_read_config_word(child, cpos + PCI_EXP_LNKCTL,
> &reg16);
> +		lnkctl_ccc_and &= (reg16 & PCI_EXP_LNKCTL_CCC);
> +		lnkctl_ccc_or |= (reg16 & PCI_EXP_LNKCTL_CCC);
> +	}
> +
> +	/* If we want Common Clock OFF and no device/function has it
> on */
> +	/* or we want Common Clock ON and every device/function has
> it on */
> +	/* then there is no need to retrain PCIe links */
> +	if (((same_clock == 0) && (lnkctl_ccc_or == 0))
> +            || ((same_clock == 1) && (lnkctl_ccc_and ==
> PCI_EXP_LNKCTL_CCC)))
> +		return;		/* Don't retrain link(s) */
> +
>  	/* Configure downstream component, all functions */
>  	list_for_each_entry(child, &linkbus->devices, bus_list) {
>  		cpos = pci_find_capability(child, PCI_CAP_ID_EXP);
> 

Seems ok to me, anyone have comments?

-- 
Jesse Barnes, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/