Re: [PATCH v2 0/2] pci: host: new driver for Marvell Armada 7K/8K PCIe controller

[Thomas Petazzoni <thomas.petazzoni@free-electrons.com>]

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