Re: [PATCH 4/7] PCI / PCIe: Disable PCIe port services during port initialization

[Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>]

(2010/07/31 7:34), Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> Subject: PCI / PCIe: Disable PCIe port services during port initialization
> 
> In principle PCIe port services may be enabled by the BIOS, so it's
> better to disable them during port initialization to avoid spurious
> events from being generated.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/pci/pcie/portdrv_core.c |   29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6/drivers/pci/pcie/portdrv_core.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/pcie/portdrv_core.c
> +++ linux-2.6/drivers/pci/pcie/portdrv_core.c
> @@ -256,20 +256,43 @@ static int get_port_device_capability(st
>  	/* Hot-Plug Capable */
>  	if ((cap_mask & PCIE_PORT_SERVICE_HP) && (reg16 & PCI_EXP_FLAGS_SLOT)) {
>  		pci_read_config_dword(dev, pos + PCI_EXP_SLTCAP, &reg32);
> -		if (reg32 & PCI_EXP_SLTCAP_HPC)
> +		if (reg32 & PCI_EXP_SLTCAP_HPC) {
>  			services |= PCIE_PORT_SERVICE_HP;
> +			/*
> +			 * Disable hot-plug interrupts in case they have been
> +			 * enabled by the BIOS and the hot-plug service driver
> +			 * is not loaded.
> +			 */
> +			pos += PCI_EXP_SLTCTL;
> +			pci_read_config_word(dev, pos, &reg16);
> +			reg16 &= ~(PCI_EXP_SLTCTL_CCIE | PCI_EXP_SLTCTL_HPIE);
> +			pci_write_config_word(dev, pos, reg16);
> +		}
>  	}
>  	/* AER capable */
>  	if ((cap_mask & PCIE_PORT_SERVICE_AER)
> -	    && pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR))
> +	    && pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR)) {
>  		services |= PCIE_PORT_SERVICE_AER;
> +		/*
> +		 * Disable AER on this port in case it's been enabled by the
> +		 * BIOS (the AER service driver will enable it when necessary).
> +		 */
> +		pci_disable_pcie_error_reporting(dev);
> +	}
>  	/* VC support */
>  	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_VC))
>  		services |= PCIE_PORT_SERVICE_VC;
>  	/* Root ports are capable of generating PME too */
>  	if ((cap_mask & PCIE_PORT_SERVICE_PME)
> -	    && dev->pcie_type == PCI_EXP_TYPE_ROOT_PORT)
> +	    && dev->pcie_type == PCI_EXP_TYPE_ROOT_PORT) {
>  		services |= PCIE_PORT_SERVICE_PME;
> +		/*
> +		 * Disable PME interrupt on this port in case it's been enabled
> +		 * by the BIOS (the PME service driver will enable it when
> +		 * necessary).
> +		 */
> +		pcie_pme_interrupt_enable(dev, false);
> +	}
>  
>  	return services;
>  }
> 

I believe that comments described well are always welcomed.

Reviewed-by: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>