Re: [PATCH v4] PCI/portdrv: Allow probing even without child services

[Bjorn Helgaas]

On Thu, Jul 23, 2026 at 08:27:41AM +0200, Lukas Wunner wrote:
> On Wed, Jul 22, 2026 at 04:09:42PM -0700, Brian Norris wrote:
> > +++ b/drivers/pci/pcie/portdrv.c
> > @@ -330,7 +330,7 @@ static int pcie_device_init(struct pci_dev *pdev, int service, int irq)
> >   */
> >  static int pcie_port_device_register(struct pci_dev *dev)
> >  {
> > -	int status, capabilities, i, nr_service;
> > +	int status, capabilities, i;
> >  	int irqs[PCIE_PORT_DEVICE_MAXSERVICES];
> 
> Nit:  Move the amended line below the irqs[] declaration to sustain
> inverse Christmas tree ordering.

I moved this locally.

> Otherwise this is
> Reviewed-by: Lukas Wunner <lukas@xxxxxxxxx>
> 
> One caveat, I note there is a pre-existing issue on Loongson LS7A,
> which (according to 62b6dee1b44a) suffers from a defect wherein
> disabling bus mastering prevents forwarding of MMIO requests downstream.
> This happens right now (and continues to happen after the present patch)
> if the error path in pcie_port_device_register() is taken.
> 
> I also note that pci_enable_device() is only called because per
> 1ce5e83063bf, it was necessary in 2009 for legacy interrupt initialization
> "on some architectures", which sounds fairly vague to me.  In any case,
> it seems to suggest that pci_disable_device() should be called if
> pcie_init_service_irqs() fails.  Right now that is done, after the patch
> it isn't.  I guess portdrv would have to remember on probe whether
> pci_enable_device() was called and only then call pci_disable_device()
> on remove.  I guess it would be possible to abuse dev_set_drvdata()
> to store this status bit (as a fake pointer).  Then again, I'm not
> sure pci_enable_device() is still necessary for legacy interrupt
> initalization today.  I guess "legacy interrupt" refers to INTx.
> A lot has changed in generic and PCI irq code since 2009.
> I assume we otherwise don't have a need to access BARs on PCIe ports
> currently, but perhaps new port services may come along which do?

I think we also need pci_enable_device() so MSI-X configuration can
access the BAR.