Re: [PATCH 3/3] agp/amd64: Remove support for probing unlisted PCI devices

[Lukas Wunner <lukas@wunner.de>]

On Mon, Jul 07, 2025 at 07:37:10PM +0200, Hans de Goede wrote:
> AMD64 boards with AGP support are so old that the agp_amd64_pci_table
> should be complete and there is no need to probe unlisted PCI devices,
> so lets completely remove support for probing unlisted PCI devices.
[...]
> --- a/drivers/char/agp/amd64-agp.c
> +++ b/drivers/char/agp/amd64-agp.c
> @@ -734,47 +733,10 @@ static struct pci_driver agp_amd64_pci_driver = {
>  /* Not static due to IOMMU code calling it early. */
>  int __init agp_amd64_init(void)
>  {
> -	struct pci_dev *pdev = NULL;
> -	int err = 0;
> -
>  	if (agp_off)
>  		return -EINVAL;
>  
> -	err = pci_register_driver(&agp_amd64_pci_driver);
> -	if (err < 0)
> -		return err;
> -
> -	if (agp_bridges_found == 0) {
> -		if (!agp_try_unsupported && !agp_try_unsupported_boot) {
> -			printk(KERN_INFO PFX "No supported AGP bridge found.\n");
> -#ifdef MODULE
> -			printk(KERN_INFO PFX "You can try agp_try_unsupported=1\n");
> -#else
> -			printk(KERN_INFO PFX "You can boot with agp=try_unsupported\n");
> -#endif
> -			pci_unregister_driver(&agp_amd64_pci_driver);
> -			return -ENODEV;
> -		}
> -
> -		/* First check that we have at least one AMD64 NB */
> -		if (!amd_nb_num()) {
> -			pci_unregister_driver(&agp_amd64_pci_driver);
> -			return -ENODEV;
> -		}

Here the PCI driver used to be unregistered if no AGP bridges were found
and successfully probed...

> -
> -		/* Look for any AGP bridge */
> -		for_each_pci_dev(pdev)
> -			if (pci_find_capability(pdev, PCI_CAP_ID_AGP))
> -				pci_add_dynid(&agp_amd64_pci_driver,
> -					      pdev->vendor, pdev->device,
> -					      pdev->subsystem_vendor,
> -					      pdev->subsystem_device, 0, 0, 0);
> -		if (agp_bridges_found == 0) {
> -			pci_unregister_driver(&agp_amd64_pci_driver);
> -			err = -ENODEV;
> -		}
> -	}
> -	return err;
> +	return pci_register_driver(&agp_amd64_pci_driver);
>  }

... and now the PCI driver will be kept around.  Is that intentional?

Keeping the PCI driver around only makes sense if an AGP bridge is
later on hot-plugged.  I guess that never happens with AGP bridges,
at least it's not supported by this driver so far.

Thanks,

Lukas