On Wed, Apr 16, 2025 at 06:25:52PM +0200, Philipp Stanner wrote:

On Wed, 2025-04-16 at 18:39 +0300, Andy Shevchenko wrote:

...

On Wed, Apr 16, 2025 at 03:12:12PM +0200, Philipp Stanner wrote:

...

...

...

bus->remap_addr = pci_ioremap_bar(pci, 0);   if (!bus->remap_addr) {   dev_err(bus->dev, "ioremap error\n");

• ret = -ENXIO;
  

• goto err_remap_bar0;
  

• return -ENXIO;
  

Here and everywhere else these can now be converted to dev_err_probe(). Are you planning to do so?

I want to do what's necessary to get PCI in better shape, since that's what the GPUs and accelerators we / I care about use :)

IOW, I want pci_request_regions() removed from here.

Okay!

...

...

...

err_remap_bar4:   iounmap(bus->remap_addr);

This looks weird if the driver already is using pcim_enable_device(). Doesn't this look to you as an existing bug?

I looked briefly at it and it doesn't appear like an obvious bug to me because the drivers uses the (very old? deprecated?) pci_ioremap_bar().

In any case the driver doesn't set up any devres callback, so has to iounmap() manually.

Okay, so they are using managed and non-managed APIs, but release / error path ordering is fine. So, false alarm then.

@Bjorn: Any comments on pci_ioremap_bar()? Should we mark that as deprecated?

...

...

-err_remap_bar0:

• pci_release_regions(pci);

return ret;