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

Philipp Stanner
2024-08-21T09:36:46Z

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;
> > -       }
>