Hello Bjorn, On Mon, 25 Apr 2016 12:28:46 -0500, Bjorn Helgaas wrote: > How about if you just shuffle those bits around into > an armada8k_add_pcie_port() and an armada8k_pcie_establish_link(), and > we'll call that good for now? I just sent a v3 which implements this. However, to be honest, after looking at the other drivers, I found the XYZ_add_pcie_port() function to not be very useful: it simply continues the work done by the probe function, and there is really no reason to split the remaining work of the probe() in a separate function. The XYZ_add_pcie_port() is called only one time for each probe() call, so having a separate function is somewhat useless. In addition, the separation of work between probe() and XYZ_add_pcie_port() is not very consistent accross drivers. In pci-imx6, XYZ_add_pcie_port() only sets up the interrupt handler and calls dw_pcie_host_init(). In pci-dra7xx, XYZ_add_pcie_port() sets up the interrupt, maps some registers and calls dw_pcie_host_init(). So my v3 implements what you suggested, and creates a armada8k_add_pcie_port() to make it consistent with the other drivers, but long term, I'm not sure this particular function is really useful. Best regards, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- 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
