Re: [PATCH] PCI: remove type return

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Sat, 3 Aug 2024, Guilherme Giacomo Simoes wrote:

> I can see that the function pci_hp_add_brigde have a int return propagation.

typo in function name. Add parenthesis after function names like this:
pci_hp_add_bridge()

> But in the drivers that pci_hp_add_bridge is called, your return never is
> cheked.

checked.

> This make me a think that the add bridge for pci hotplug drivers is not crucial
> for functionaly and your failed only should show a message in logs.

functionality

> 
> Then, I maked this patch for remove your return propagation for this moment.

Please write the commit message using imperative tone. Don't use "I", 
"me", "you", "your", or "we" at all.

Also, you need to signoff your patches (please read 
Documentation/process/submitting-patches.rst).

The lack of return value checking seems to be on the list in
pci_hp_add_bridge(). So perhaps the right course of action would be to 
handle return values correctly.

-- 
 i.

> ---
>  drivers/pci/pci.h   | 2 +-
>  drivers/pci/probe.c | 7 +++----
>  2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 79c8398f3938..a35dbfd89961 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -189,7 +189,7 @@ static inline int pci_proc_detach_bus(struct pci_bus *bus) { return 0; }
>  #endif
>  
>  /* Functions for PCI Hotplug drivers to use */
> -int pci_hp_add_bridge(struct pci_dev *dev);
> +void pci_hp_add_bridge(struct pci_dev *dev);
>  
>  #if defined(CONFIG_SYSFS) && defined(HAVE_PCI_LEGACY)
>  void pci_create_legacy_files(struct pci_bus *bus);
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index b14b9876c030..b13c4c912eb1 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -3352,7 +3352,7 @@ void __init pci_sort_breadthfirst(void)
>  	bus_sort_breadthfirst(&pci_bus_type, &pci_sort_bf_cmp);
>  }
>  
> -int pci_hp_add_bridge(struct pci_dev *dev)
> +void pci_hp_add_bridge(struct pci_dev *dev)
>  {
>  	struct pci_bus *parent = dev->bus;
>  	int busnr, start = parent->busn_res.start;
> @@ -3365,7 +3365,7 @@ int pci_hp_add_bridge(struct pci_dev *dev)
>  	}
>  	if (busnr-- > end) {
>  		pci_err(dev, "No bus number available for hot-added bridge\n");
> -		return -1;
> +		return;
>  	}
>  
>  	/* Scan bridges that are already configured */
> @@ -3381,8 +3381,7 @@ int pci_hp_add_bridge(struct pci_dev *dev)
>  	pci_scan_bridge_extend(parent, dev, busnr, available_buses, 1);
>  
>  	if (!dev->subordinate)
> -		return -1;
> +		pci_err(dev, "No bus subordinate");
>  
> -	return 0;
>  }
>  EXPORT_SYMBOL_GPL(pci_hp_add_bridge);
>