Re: [PATCH v2] stmmac: Replace deprecated PCI functions

From: Yanteng Si
Date: Thu Feb 20 2025 - 21:02:30 EST



在 2/18/25 9:21 PM, Philipp Stanner 写道:
From: Philipp Stanner <pstanner@xxxxxxxxxx>

The PCI functions
- pcim_iomap_regions()
- pcim_iomap_table() and
- pcim_iounmap_regions()
have been deprecated.

The usage of pcim_* cleanup functions in the driver detach path (remove
callback) is actually not necessary, since they perform that cleanup
automatically.

Furthermore, loongson_dwmac_probe() contains a surplus loop. That loop
does not use index i in pcim_iomap_regions(), but costantly attempts to
request and ioremap BAR 0. This would actually fail (since you cannot
request the same BAR more than once), but presumably never fails because
the preceding length check detects that all BARs except for 0 do not
exist.

Replace them with pcim_iomap_region(). Remove the surplus loop.

So, two things are done in one patch. How about splitting it into two patches?



Signed-off-by: Philipp Stanner <pstanner@xxxxxxxxxx>
---
Changes in v2:
- Fix build errors because of missing ';'
- Address in the commit message why the patch removes a loop. (Andrew)
---
.../ethernet/stmicro/stmmac/dwmac-loongson.c | 31 ++++++-------------
.../net/ethernet/stmicro/stmmac/stmmac_pci.c | 24 ++++----------
2 files changed, 15 insertions(+), 40 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
index bfe6e2d631bd..6f7c479c1a51 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
@@ -11,6 +11,8 @@
#include "dwmac_dma.h"
#include "dwmac1000.h"
+#define DRIVER_NAME "dwmac-loongson-pci"

This appears to have nothing to do with the commit message.

I believe it would be better if it were split off and made into

an independent patch.

+
/* Normal Loongson Tx Summary */
#define DMA_INTR_ENA_NIE_TX_LOONGSON 0x00040000
/* Normal Loongson Rx Summary */
@@ -520,9 +522,9 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id
{
struct plat_stmmacenet_data *plat;
struct stmmac_pci_info *info;
- struct stmmac_resources res;
+ struct stmmac_resources res = {};
struct loongson_data *ld;
- int ret, i;
+ int ret;
plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL);
if (!plat)
@@ -552,17 +554,10 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id
pci_set_master(pdev);
/* Get the base address of device */
- for (i = 0; i < PCI_STD_NUM_BARS; i++) {
- if (pci_resource_len(pdev, i) == 0)
- continue;
- ret = pcim_iomap_regions(pdev, BIT(0), pci_name(pdev));
- if (ret)
- goto err_disable_device;
- break;
- }
-
- memset(&res, 0, sizeof(res));
- res.addr = pcim_iomap_table(pdev)[0];
+ res.addr = pcim_iomap_region(pdev, 0, DRIVER_NAME);
+ ret = PTR_ERR_OR_ZERO(res.addr);
+ if (ret)
+ goto err_disable_device;
plat->bsp_priv = ld;
plat->setup = loongson_dwmac_setup;

According to the description in the commit message, the reason for

remove the surplus loop here is to fix the problem of requesting

the same BAR multiple times, that's good.

@@ -606,7 +601,6 @@ static void loongson_dwmac_remove(struct pci_dev *pdev)
struct net_device *ndev = dev_get_drvdata(&pdev->dev);
struct stmmac_priv *priv = netdev_priv(ndev);
struct loongson_data *ld;
- int i;
ld = priv->plat->bsp_priv;
stmmac_dvr_remove(&pdev->dev);
@@ -617,13 +611,6 @@ static void loongson_dwmac_remove(struct pci_dev *pdev)
if (ld->loongson_id == DWMAC_CORE_LS_MULTICHAN)
loongson_dwmac_msi_clear(pdev);
- for (i = 0; i < PCI_STD_NUM_BARS; i++) {
- if (pci_resource_len(pdev, i) == 0)
- continue;
- pcim_iounmap_regions(pdev, BIT(i));
- break;
- }
-
pci_disable_device(pdev);
}

According to the commit message, the reason for removing the surplus

loop here is that there's no need to use the pcim_* cleanup functions in

the `remove()` function. This is different from the modifications in the

`probe()` function. I think it can be split out and made into a separate

patch.


How about splitting it like this?

Patch 1/3: Use the `DRIVER_NAME` macro.

Patch 2/3: remove the surplus loop to fix the problem of requesting the same BAR multiple times

Patch 3/3: remove the surplus pcim_* cleanup functions in the `remove()` function


Huacai, Qunqin, Would you mind helping to test it?


Thanks,

Yanteng