On Thu, 2018-03-01 at 20:54 +0100, Heiner Kallweit wrote:
> Am 01.03.2018 um 12:27 schrieb Andy Shevchenko:
> > - rc = pci_request_regions(pdev, MODULENAME);
> > + rc = pcim_iomap_regions(pdev, BIT(region), MODULENAME);
> > if (rc < 0) {
> > - netif_err(tp, probe, dev, "could not request
> > regions\n");
> > + netif_err(tp, probe, dev, "cannot remap MMIO,
> > aborting\n");
> > return rc;
> > }
> >
> >
> > + tp->mmio_addr = pcim_iomap_table(pdev)[region];
> >
>
> pcim_iomap_table() can return NULL in case of an error.
No.
> Shouldn't we catch this?
No.
Yeah, I'm a bit tired to explain everyone that pcim_iomap_table() will
never fail if previous pcim_iomap_regions() not failed.
> Typical benefit of switching to device-managed functions is that we
> don't
> have to clean up in the probe() error path and in remove(). With the
> change here we don't have any such benefit and we just exchange two
> calls
> against two other calls w/o functional change (AFAICS).
> Which benefit do you see justifying this patch?
-67% statistics is a good sign, no?
> However I don't know the PCI API's good enough to be able to judge
> whether
> one set of calls is preferable.
More than above, it's about consistency. While you switch to devm_, for
PCI driver it's naturally to go for pcim_.
Esp. taking into account that pcim_enable_device() _is_ already there.
--
Andy Shevchenko <[email protected]>
Intel Finland Oy
