On Fri, 2024-12-13 at 16:10 +0100, Stefano Garzarella wrote:
On Wed, Dec 11, 2024 at 11:47:05AM +0100, Philipp Stanner wrote:
> The PCI functions
>
> pcim_iomap_regions()
> pcim_iounmap_regions()
> pcim_iomap_table()
>
> have been deprecated by the PCI subsystem.
>
> Replace these functions with their successors pcim_iomap_region()
> and
> pcim_iounmap_region().
>
> Signed-off-by: Philipp Stanner <pstanner@xxxxxxxxxx>
> ---
> Changes in v3:
> - Move __iomem *io declaration into the loop. (Stefano)
>
> Changes in v2:
> - Fix build warning because of dead variable.
> - Make "bars_found" a boolean, since only true or false are
> relevant.
> ---
> drivers/vdpa/solidrun/snet_main.c | 56 ++++++++++++++--------------
> ---
> 1 file changed, 26 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/vdpa/solidrun/snet_main.c
> b/drivers/vdpa/solidrun/snet_main.c
> index c8b74980dbd1..643e335f00f1 100644
> --- a/drivers/vdpa/solidrun/snet_main.c
> +++ b/drivers/vdpa/solidrun/snet_main.c
> @@ -556,59 +556,58 @@ static const struct vdpa_config_ops
> snet_config_ops = {
> static int psnet_open_pf_bar(struct pci_dev *pdev, struct psnet
> *psnet)
> {
> char *name;
> - int ret, i, mask = 0;
> + unsigned short i;
> + bool bars_found = false;
> +
> + name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "psnet[%s]-
> bars", pci_name(pdev));
> + if (!name)
> + return -ENOMEM;
> +
> /* We don't know which BAR will be used to communicate..
> * We will map every bar with len > 0.
> *
> * Later, we will discover the BAR and unmap all other
> BARs.
> */
> for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> - if (pci_resource_len(pdev, i))
> - mask |= (1 << i);
> + void __iomem *io;
> +
> + if (pci_resource_len(pdev, i) == 0)
> + continue;
> +
> + io = pcim_iomap_region(pdev, i, name);
> + if (IS_ERR(io)) {
> + SNET_ERR(pdev, "Failed to request and map
> PCI BARs\n");
> + return PTR_ERR(io);
> + }
> +
> + psnet->bars[i] = io;
> + bars_found = true;
> }
>
> /* No BAR can be used.. */
> - if (!mask) {
> + if (!bars_found) {
> SNET_ERR(pdev, "Failed to find a PCI BAR\n");
> return -ENODEV;
> }
>
> - name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "psnet[%s]-
> bars", pci_name(pdev));
> - if (!name)
> - return -ENOMEM;
> -
> - ret = pcim_iomap_regions(pdev, mask, name);
> - if (ret) {
> - SNET_ERR(pdev, "Failed to request and map PCI
> BARs\n");
> - return ret;
> - }
> -
> - for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> - if (mask & (1 << i))
> - psnet->bars[i] =
> pcim_iomap_table(pdev)[i];
> - }
> -
> return 0;
> }
>
> static int snet_open_vf_bar(struct pci_dev *pdev, struct snet
> *snet)
> {
> char *name;
> - int ret;
>
> name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "snet[%s]-
> bars", pci_name(pdev));
> if (!name)
> return -ENOMEM;
>
> /* Request and map BAR */
> - ret = pcim_iomap_regions(pdev, BIT(snet->psnet-
> >cfg.vf_bar), name);
> - if (ret) {
> + snet->bar = pcim_iomap_region(pdev, snet->psnet-
> >cfg.vf_bar, name);
Could we also use a temporary variable here as we did in
psnet_open_pf_bar()?
We can. Some maintainers want it so, others so
It seems to me that no one uses `!snet->bar` for now to check whether
they are configured or not, but maybe better to prevent, WDYT?
So it seems that this driver is a bit special as its probe() function
can take different paths running into one of the respective request
functions.
Doesn't look to me as if pcim_iomap_region() could fail and the driver
would still load?
Anyways, better safe than sorry. If you think it's safer I can modify
it so those pointers are valid or NULL.
P.
Thanks,
Stefano
> + if (IS_ERR(snet->bar)) {
> SNET_ERR(pdev, "Failed to request and map PCI BAR
> for a VF\n");
> - return ret;
> + return PTR_ERR(snet->bar);
> }
>
> - snet->bar = pcim_iomap_table(pdev)[snet->psnet-
> >cfg.vf_bar];
> -
> return 0;
> }
>
> @@ -656,15 +655,12 @@ static int psnet_detect_bar(struct psnet
> *psnet, u32 off)
>
> static void psnet_unmap_unused_bars(struct pci_dev *pdev, struct
> psnet *psnet)
> {
> - int i, mask = 0;
> + unsigned short i;
>
> for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> if (psnet->bars[i] && i != psnet->barno)
> - mask |= (1 << i);
> + pcim_iounmap_region(pdev, i);
> }
> -
> - if (mask)
> - pcim_iounmap_regions(pdev, mask);
> }
>
> /* Read SNET config from PCI BAR */
> --
> 2.47.1
>