Re: [PATCH] PCI: cpcihp: Check pci_hp_add_bridge() return value

[Bjorn Helgaas <helgaas@kernel.org>]

On Mon, Feb 23, 2026 at 02:36:46AM +0000, Majed Alkhaleefah wrote:
> Check the return value of pci_hp_add_bridge() in cpci_configure_slot(). If
> bridge initialization fails, log an error message using err().
> 
> Current functionality doesn't consider uninitialized bridges a failure.
> Changing this may introduce regressions. As such, intentionally avoid
> propagating the failure (e.g., returning -ENODEV).
> 
> This patch addresses an item in the cpcihp TODO list.

This sentence could just be dropped and the item removed from the list
in the same patch.

> Compile-tested only.
> 
> Signed-off-by: Majed Alkhaleefah <majedalkhaleefah@gmail.com>
> ---
>  drivers/pci/hotplug/cpci_hotplug_pci.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/cpci_hotplug_pci.c b/drivers/pci/hotplug/cpci_hotplug_pci.c
> index 6c48066acb44..331c21833c44 100644
> --- a/drivers/pci/hotplug/cpci_hotplug_pci.c
> +++ b/drivers/pci/hotplug/cpci_hotplug_pci.c
> @@ -269,8 +269,12 @@ int cpci_configure_slot(struct slot *slot)
>  	parent = slot->dev->bus;
>  
>  	for_each_pci_bridge(dev, parent) {
> -		if (PCI_SLOT(dev->devfn) == PCI_SLOT(slot->devfn))
> -			pci_hp_add_bridge(dev);
> +		if (PCI_SLOT(dev->devfn) == PCI_SLOT(slot->devfn)) {
> +			if (pci_hp_add_bridge(dev) != 0) {
> +				err("Failed to initialize bridge %s at slot %02x",
> +				    pci_name(dev), slot->number);

There are five callers of pci_hp_add_bridge(), and none of them checks
the return value, so I'm not sure it's worth just changing this one.

I suspect something in the pci_hp_add_bridge() path would already log
a message in the event of a failure.  If that's the case, I doubt it's
worth adding another message here (and maybe pci_hp_add_bridge()
should just be made a void function).  If we *do* need a message here,
I would want to update all the callers similarly.

> +			}
> +		}
>  	}
>  
>  	pci_assign_unassigned_bridge_resources(parent->self);
> -- 
> 2.43.0
>