Re: [PATCH v2 6/9] ethernet: stmicro: Simplify PCI devres usage

[Philipp Stanner <pstanner@redhat.com>]

On Wed, 2024-08-21 at 11:14 +0300, Andy Shevchenko wrote:
> On Wed, Aug 21, 2024 at 10:19 AM Philipp Stanner
> <pstanner@redhat.com> wrote:
> > 
> > stmicro uses PCI devres in the wrong way. Resources requested
> > through pcim_* functions don't need to be cleaned up manually in
> > the
> > remove() callback or in the error unwind path of a probe()
> > function.
> 
> > Moreover, there is an unnecessary loop which only requests and
> > ioremaps
> > BAR 0, but iterates over all BARs nevertheless.
> 
> Seems like loongson was cargo-culted a lot without a clear
> understanding of this code in the main driver...
> 
> > Furthermore, 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 pcim_iomap_region().
> > 
> > Remove the unnecessary manual pcim_* cleanup calls.
> > 
> > Remove the unnecessary loop over all BARs.
> 
> ...
> 
> > -       for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> > -               if (pci_resource_len(pdev, i) == 0)
> > -                       continue;
> > -               pcim_iounmap_regions(pdev, BIT(i));
> 
> Here is the BARx, which contradicts the probe :-)

I'm not sure what should be done about it. The only interesting
question is whether the other code with pcim_iomap_regions(... BIT(i)
does also only grap BAR 0.
In that case the driver wouldn't even be knowing what its own hardware
is / does, though.


P.

> 
> > -               break;
> > -       }
>