On Wed, 2013-09-18 at 19:02 +0800, Minghuan Lian wrote: > @@ -592,6 +719,7 @@ int fsl_pci_mcheck_exception(struct pt_regs *regs) > #if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx) > > struct device_node *fsl_pci_primary; > +extern const struct of_device_id fsl_pci_ids[]; Externs go in headers. > -static int fsl_pci_probe(struct platform_device *pdev) > +static int __init fsl_pci_probe(struct platform_device *pdev) > { > int ret; > - struct device_node *node; > + struct fsl_pci *pci; > + > + if (!of_device_is_available(pdev->dev.of_node)) { > + dev_warn(&pdev->dev, "disabled\n"); > + return -ENODEV; > + } This should be dev_dbg(). > -#ifdef CONFIG_PM > -static int fsl_pci_resume(struct device *dev) > +static int __exit fsl_pci_remove(struct platform_device *pdev) Why __exit? What happens if someone unbinds the PCI controller via sysfs? > +/* > + * Structure of a PCI controller (host bridge) > + */ > +struct fsl_pci { > + struct list_head node; > + int is_pcie; bool is_pcie; > +/* Return link status 0-> link, 1-> no link */ > +int fsl_pci_check_link(struct fsl_pci *pci); bool > + > +/* > + * The fsl_arch_* functions are arch hooks. Those functions are > + * implemented as weak symbols so that they can be overridden by > + * architecture specific code if needed. > + */ > + > +/* Return PCI64 DMA offset */ > +u64 fsl_arch_pci64_dma_offset(void); Is this always guaranteed to exist? > +/* Register PCI/PCIe controller to architecture system */ > +int __weak fsl_arch_pci_sys_register(struct fsl_pci *pci); > + > +/* Remove PCI/PCIe controller from architecture system */ > +void __weak fsl_arch_pci_sys_remove(struct fsl_pci *pci); Why do these need to be weak? Won't there be exactly one implementation per supported arch? -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html