[PATCH 2/4] PCI/ERR: Add support for resetting the slot in a platforms specific way

[Lukas Wunner]

On Tue, Apr 15, 2025 at 07:03:17PM +0530, Manivannan Sadhasivam wrote:
> On Fri, Apr 04, 2025 at 10:46:27AM +0200, Lukas Wunner wrote:
> > On Fri, Apr 04, 2025 at 01:52:22PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > > When the PCI error handling requires resetting the slot, reset it
> > > using the host bridge specific 'reset_slot' callback if available
> > > before calling the 'slot_reset' callback of the PCI drivers.
> > > 
> > > The 'reset_slot' callback is responsible for resetting the given slot
> > > referenced by the 'pci_dev' pointer in a platform specific way and
> > > bring it back to the working state if possible. If any error occurs
> > > during the slot reset operation, relevant errno should be returned.
> > 
> > This feels like something that should be plumbed into
> > pcibios_reset_secondary_bus(), rather than pcie_do_recovery().
> 
> I did consider that, but didn't go for it since there was no platform
> reset code present in that function. But I will try to use it as I
> don't have a strong preference to do reset slot in pcie_do_recovery().

The only platform overriding pcibios_reset_secondary_bus() is powerpc,
and it only does that on PowerNV.

I think you could continue to stick with the approach of adding a
->reset_slot() callback to struct pci_host_bridge, but it would
be good if at the same time you could convert PowerNV to use the
newly introduced callback as well.  And then remove the way to
override the reset procedure via pcibios_reset_secondary_bus().

All pci_host_bridge's which do not define a ->reset_slot() could be
assigned a default callback which just calls pci_reset_secondary_bus().

Alternatively, pcibios_reset_secondary_bus() could be made to invoke the
pci_host_bridge's ->reset_slot() callback if it's not NULL, else
pci_reset_secondary_bus().  And in that case, the __weak attribute
could be removed as well as the powerpc-specific version of
pcibios_reset_secondary_bus().

I guess what I'm trying to say is, you don't *have* to plumb this
into pcibios_reset_secondary_bus().  In fact, having a host bridge
specific callback could be useful if the SoC contains several
host bridges which require different callbacks to perform a reset.

I just want to make sure that the code remains maintainable,
i.e. we don't have two separate ways to override how a bus reset
is performed.

Thanks,

Lukas