Re: [PATCH v7 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 d3720492f32e5a284ef73b3dc9f69d02b13df2c4
Author: Mario Limonciello <mario.limonciello@amd.com>

PCI: Cache bridge firmware secondary bus number

This commit caches the firmware-assigned secondary bus number on a PCI
bridge's struct pci_dev in pci_read_bridge_windows() before bus renumbering
can occur.

> 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_mmio_pref(struct pci_dev *dev, struct resource *res,
>  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?

pci_read_bridge_windows() is only called from pci_setup_device() when
pci_scan_device() first allocates and initializes a new struct pci_dev.  On
later bus rescans, pci_scan_single_device() finds the existing device via
pci_get_slot() and returns early without calling pci_setup_device() or
pci_read_bridge_windows().  Because pci_read_bridge_windows() runs at most
once per bridge instance on a zero-initialized struct pci_dev,
fw_sec_bus_sampled is always false on entry and is never read elsewhere,
making the bitfield and the commit message's rescan explanation unnecessary.
In addition, since bus 0 is the root bus and fw_sec_busnr is only set when
secondary > 0 && secondary < 0xff, fw_sec_bus_valid is redundant with
checking whether fw_sec_busnr is non-zero.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-vfct-regression-v7-0-69b1e0cf45c2@amd.com?part=1