On Fri, Jul 28, 2023 at 02:14:01PM +0100, daire.mcnamara@xxxxxxxxxxxxx wrote:
> From: Daire McNamara <daire.mcnamara@xxxxxxxxxxxxx>
>
> Continuing to use pci_host_common_probe() for the PCIe Root Complex on
> PolarFire SoC was leading to an extremely large _init() function and
> some unnatural code flow. Re-partition so some tasks are done in
> a _probe() routine, which calls pci_host_common_probe() and then use a
> much smaller _init() function, mainly to enable interrupts after address
> translation tables are set up.
> ...
> +++ b/drivers/pci/controller/pcie-microchip-host.c
> @@ -384,6 +384,8 @@ static struct {
>
> static char poss_clks[][5] = { "fic0", "fic1", "fic2", "fic3" };
>
> +static struct mc_pcie *port;
> ...
> +static int mc_host_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> void __iomem *bridge_base_addr;
> int ret;
> u32 val;
> @@ -1112,13 +1141,8 @@ static int mc_platform_init(struct pci_config_window *cfg)
> port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
Saving this per-device pointer in a static singleton is kind of
problematic. We may know there can only be a single instance right
now, but even if that remains true forever for this driver, we don't
want this pattern to be be copied elsewhere.
I didn't look hard enough to figure out exactly what problem this
singleton solves, but is there any other way to address it?
I suspect it's related to using pci_host_common_probe(), and it's
great to share that code, but it seems like that basically forces some
non-ECAM initialization into the struct pci_ecam_ops.init() method
where it doesn't really fit very well.
Bjorn
