Re: [PATCH v6 1/3] PCI: Cache bridge firmware secondary bus number

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] PCI: dead fw_sec_bus_sampled in pci_read_bridge_windows() and
  log mismatch
--

commit 263ccf4863cf26244ff7441d7c145a467a3a1fcd
Author: Mario Limonciello <mario.limonciello@amd.com>

PCI: Cache bridge firmware secondary bus number

This commit caches the firmware-assigned secondary bus number on bridge
devices in pci_read_bridge_windows() so downstream devices can query the
original bus number after bus renumbering.

> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 27008e2ea5afc..f20a4b8f7a23b 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -505,14 +505,26 @@ static void pci_read_bridge_windows(struct pci_dev *bridge)
>  {
>  	u32 buses;
> +	u8 secondary;
> +	u8 subordinate;
>  	u16 io;
>  	u32 pmem, tmp;
>  	struct resource res;
>  
>  	pci_read_config_dword(bridge, PCI_PRIMARY_BUS, &buses);
>  	res.flags = IORESOURCE_BUS;
> -	res.start = FIELD_GET(PCI_SECONDARY_BUS_MASK, buses);
> -	res.end = FIELD_GET(PCI_SUBORDINATE_BUS_MASK, buses);
> +	secondary = FIELD_GET(PCI_SECONDARY_BUS_MASK, buses);
> +	subordinate = FIELD_GET(PCI_SUBORDINATE_BUS_MASK, buses);
> +	res.start = secondary;
> +	res.end = subordinate;
> +
> +	if (!bridge->fw_sec_bus_sampled) {
> +		bridge->fw_sec_bus_sampled = true;
> +		if (secondary > 0 && secondary < 0xff) {
> +			bridge->fw_sec_busnr = secondary;
> +			bridge->fw_sec_bus_valid = true;
> +		}
> +	}

[Severity: Low]
As reported by Lazar, Lijo on v5 [1], is bridge->fw_sec_bus_sampled needed
here, and does the commit message match where the bus number is now saved?

pci_read_bridge_windows() is only called from pci_setup_device() ->
pci_scan_device() when a struct pci_dev is first allocated and zeroed.
Later bus rescans find the existing device in pci_scan_single_device() via
pci_get_slot() and return early without calling pci_setup_device() or
pci_read_bridge_windows() again, so fw_sec_bus_sampled is always false on
entry and is never read elsewhere.

Also, the commit message still describes saving the bus number during pass
0 of bridge scanning and guarding against rescans, which no longer matches
caching it in pci_read_bridge_windows() (and since a valid secondary bus
number is always non-zero, fw_sec_bus_valid is redundant with checking
fw_sec_busnr != 0).

[1] https://lore.kernel.org/r/eba45235-a6b1-4a48-9c56-9e069c299478@amd.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-vfct-regression-v6-0-393797bbb6e3@amd.com?part=1