On Mon, 2024-08-19 at 21:23 +0300, Andy Shevchenko wrote: > On Mon, Aug 19, 2024 at 06:51:46PM +0200, Philipp Stanner wrote: > > pcim_iomap_regions() and pcim_iomap_table() have been deprecated by > > the PCI subsystem in commit e354bb84a4c1 ("PCI: Deprecate > > pcim_iomap_table(), pcim_iomap_regions_request_all()"). > > > > Replace these functions with the function pcim_iomap_region(). > > ... > > cavium_ptp_probe() > > > - pcim_iounmap_regions(pdev, 1 << PCI_PTP_BAR_NO); > > + pcim_iounmap_region(pdev, PCI_PTP_BAR_NO); > > > > error_free: > > devm_kfree(dev, clock); > > Both are questionable. Why do we need either of them? You seem to criticize my pcim_iounmap_region() etc. in other unwind paths, too. I think your criticism is often justified. This driver here, however, was the one which made me suspicious and hesitate and removing those calls; because of the code below: pcim_iounmap_region(pdev, PCI_PTP_BAR_NO); error_free: devm_kfree(dev, clock); error: /* For `cavium_ptp_get()` we need to differentiate between the case * when the core has not tried to probe this device and the case when * the probe failed. In the later case we pretend that the * initialization was successful and keep the error in * `dev->driver_data`. */ pci_set_drvdata(pdev, ERR_PTR(err)); return 0; } So in case of an error they return 0 and do... stuff. I don't want to touch that without someone who maintains (and, ideally, understands) the code details what's going on here. P.