Re: [GIT PATCH] PCI patches for 2.6.24

[Greg KH]

On Fri, Feb 01, 2008 at 04:42:06PM -0800, Andrew Morton wrote:
> On Fri, 1 Feb 2008 15:11:47 -0800
> Greg KH <gregkh@xxxxxxx> wrote:
> 
> >       PCI: PCIE ASPM support
> 
> drivers/built-in.o: In function `pci_scan_slot':
> drivers/pci/probe.c:1016: undefined reference to `pcie_aspm_init_link_state'
> drivers/built-in.o: In function `pci_stop_dev':
> drivers/pci/remove.c:36: undefined reference to `pcie_aspm_exit_link_state'
> drivers/built-in.o: In function `pci_set_power_state':
> drivers/pci/pci.c:524: undefined reference to `pcie_aspm_pm_state_change'
> make: *** [.tmp_vmlinux1] Error 1
> 
> http://userweb.kernel.org/~akpm/config-vmm.txt
> 
> 
> Needs this, I guess:
> 
> 
> --- a/drivers/pci/pcie/Kconfig~fix-gregkh-pci-pci-pcie-aspm-support
> +++ a/drivers/pci/pcie/Kconfig
> @@ -32,7 +32,7 @@ source "drivers/pci/pcie/aer/Kconfig"
>  #
>  config PCIEASPM
>  	bool "PCI Express ASPM support(Experimental)"
> -	depends on PCI && EXPERIMENTAL
> +	depends on PCI && EXPERIMENTAL && PCIEPORTBUS
>  	default y
>  	help
>  	  This enables PCI Express ASPM (Active State Power Management) and
> _

Oops, sorry, I'll add that to my queue for the next round.

> 
> Also, that ASPM patch unnecessarily adds a pile of macros:
> 
> +#ifdef CONFIG_PCIEASPM
> +extern void pcie_aspm_init_link_state(struct pci_dev *pdev);
> +extern void pcie_aspm_exit_link_state(struct pci_dev *pdev);
> +extern void pcie_aspm_pm_state_change(struct pci_dev *pdev);
> +extern void pci_disable_link_state(struct pci_dev *pdev, int state);
> +#else
> +#define pcie_aspm_init_link_state(pdev)                do {} while (0)
> +#define pcie_aspm_exit_link_state(pdev)                do {} while (0)
> +#define pcie_aspm_pm_state_change(pdev)                do {} while (0)
> +#define pci_disable_link_state(pdev, state)    do {} while (0)
> +#endif
> 
> Please don't do this.  
> 
> A static inline function is cleaner and provides typechecking.  It also
> provides an access to the caller's argument and can avoid unused-varaiable
> warnings.
> 
> The only reason to use a macro in this situation is if the caller's
> argument is for some reason not defined if !CONFIG_PCIEASPM.
> 
> Greg, please check for this in your reviewing - reject macros *by default*.
>  They are inferior.

Ick, missed that, again, my appologies.  In cleaning up pci.h I saw a
few places like this, I'll fix that next go-around also.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/