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;