Yo Andy!
On Mon, 2024-08-12 at 18:57 +0300, Andy Shevchenko wrote:
> (Reduced Cc list a lot)
>
> On Mon, Aug 05, 2024 at 10:01:31AM +0200, Philipp Stanner wrote:
> > pcim_iomap_table() and pcim_iomap_regions_request_all() have been
> > deprecated by the PCI subsystem in commit e354bb84a4c1 ("PCI:
> > Deprecate
> > pcim_iomap_table(), pcim_iomap_regions_request_all()").
> >
> > Replace these functions with their successors, pcim_iomap() and
> > pcim_request_all_regions()
>
> Missing period at the end.
ACK
>
> ...
>
> > - /* Map PF's configuration registers */
> > - err = pcim_iomap_regions_request_all(pdev, 1 <<
> > PCI_PF_REG_BAR_NUM,
> > - OTX2_CPT_DRV_NAME);
> > + err = pcim_request_all_regions(pdev, OTX2_CPT_DRV_NAME);
> > if (err) {
> > - dev_err(dev, "Couldn't get PCI resources 0x%x\n", err);
> > + dev_err(dev, "Couldn't request PCI resources 0x%x\n", err);
> > goto clear_drvdata;
> > }
>
> I haven't looked at the implementation differences of those two, but
> would it
> be really an equivalent change now?
Well, if I weren't convinced that it's 100% equivalent I weren't
posting it :)
pcim_iomap_regions_request_all() already uses
pcim_request_all_regions() internally.
The lines you quote here are not equivalent to the old version, but in
combination with the following lines the functionality is identical:
1. Request all regions
2. ioremap BAR OTX2_CPT_BAR_NUM
>
> Note, the resource may be requested, OR mapped, OR both.
Negative, that is not how pcim_iomap_regions_request_all() works. That
overengineered function requests *all* PCI BARs and ioremap()s those
specified in the bit mask.
If you don't set a bit, you'll request all regions and ioremap() none.
However you choose to use it, it will always request all regions and
map between 0 and PCI_STD_NUM_BARS.
> In accordance with the
> naming above I assume that this is not equivalent change with
> potential
> breakages.
The nasty thing of us in PCI is that you more or less already use the
code above anyways, because in v6.11 I reworked most of
drivers/pci/devres.c, so pcim_iomap_regions_request_all() uses both
pcim_request_all_regions() and pcim_iomap() in precisely that order
already.
The only hypothetical breakages which are not already in v6.11 anyways
I could imagine are:
* Someone complaining about changed error codes in case of failure
* Someone racing between the calls to pcim_request_all_regions() and
pcim_iomap(). But that's why the region request is actually there in
the first place, to block off drivers competing for the same
resource. And AFAIU probe() functions don't race anyways.
Anything I might have overlooked?
P.
>
>
> > - cptpf->reg_base = pcim_iomap_table(pdev)[PCI_PF_REG_BAR_NUM];
> > + /* Map PF's configuration registers */
> > + cptpf->reg_base = pcim_iomap(pdev, PCI_PF_REG_BAR_NUM, 0);
> > + if (!cptpf->reg_base) {
> > + err = -ENOMEM;
> > + dev_err(dev, "Couldn't ioremap PCI resource 0x%x\n", err);
> > + goto clear_drvdata;
> > + }
>
> (Yes, I see this).
>
> ...
>
> > --- a/drivers/crypto/marvell/octeontx2/otx2_cptvf_main.c
> > +++ b/drivers/crypto/marvell/octeontx2/otx2_cptvf_main.c
>
> Ditto. here.
>
