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

[Bjorn Helgaas <helgaas@kernel.org>]

On Tue, Sep 08, 2026 at 07:30:28PM -0600, Fahmy Hassan wrote:
> cpqhp_configure_device() calls pci_hp_add_bridge() without checking
> its return value. If pci_hp_add_bridge() fails (e.g. no bus number
> is available for the hot-added bridge), dev->subordinate is left
> unset, so the subsequent pci_bus_add_devices(child) call is
> correctly skipped -- but the failure itself goes completely
> unreported, making such problems very hard to diagnose in the field.
> 
> Log an error via the driver's existing err() macro when
> pci_hp_add_bridge() fails, and remove the now-resolved TODO entry.

The patch doesn't seem to remove the TODO.

There are four other drivers that call pci_hp_add_bridge(), and none
of them check the return value either.  We should leave it alone or
fix them all, so the review effort can be applied to all of them at
once.

> Signed-off-by: Fahmy Hassan <fahmymohammed@gmail.com>
> ---
>  drivers/pci/hotplug/cpqphp_pci.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/hotplug/cpqphp_pci.c b/drivers/pci/hotplug/cpqphp_pci.c
> index 81c58b1ec200..34803f36ba65 100644
> --- a/drivers/pci/hotplug/cpqphp_pci.c
> +++ b/drivers/pci/hotplug/cpqphp_pci.c
> @@ -100,7 +100,9 @@ int cpqhp_configure_device(struct controller *ctrl, struct pci_func *func)
>  	}
>  
>  	if (func->pci_dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
> -		pci_hp_add_bridge(func->pci_dev);
> +		if (pci_hp_add_bridge(func->pci_dev))
> +			err("bus/device/function %x/%x/%x: pci_hp_add_bridge() failed\n",
> +			    func->bus, func->device, func->function);
>  		child = func->pci_dev->subordinate;
>  		if (child)
>  			pci_bus_add_devices(child);
> -- 
> 2.53.0
>