stmmac: Several PCI-related improvements

PATCH Superseded Philipp Stanner 4 participants 9 messages Feb 26, 2025 – Feb 28, 2025

Subsystems (5)

Networking Network drivers Stable branch Arm STM32 +1 more subsystems

lore

Share & tools

lore

Philipp Stanner

author

2025-02-26 08:52 UTC #0 lore raw

v4 cover stmmac: Several PCI-related improvements

Changes in v4:
- Add missing full stop. (Yanteng)
- Move forgotten unused-variable-removes to the appropriate places.
- Add applicable RB / TB tags

Changes in v3:
- Several formatting nits (Paolo)
- Split up patch into a patch series (Yanteng)

Philipp Stanner (4):
stmmac: loongson: Pass correct arg to PCI function
stmmac: loongson: Remove surplus loop
stmmac: Remove pcim_* functions for driver detach
stmmac: Replace deprecated PCI functions

 .../ethernet/stmicro/stmmac/dwmac-loongson.c  | 31 ++++++-------------
 .../net/ethernet/stmicro/stmmac/stmmac_pci.c  | 26 +++++-----------
 2 files changed, 16 insertions(+), 41 deletions(-)

Signature · 1 line

2.48.1

Philipp Stanner

author

2025-02-26 08:52 UTC #1 lore raw

PATCH v4 1/4 stmmac: loongson: Pass correct arg to PCI function

pcim_iomap_regions() should receive the driver's name as its third parameter, not the PCI device's name.

Define the driver name with a macro and use it at the appropriate places, including pcim_iomap_regions().

Cc [email protected] # v5.14+

Fixes 30bba69d7db4 ("stmmac: pci: Add dwmac support for Loongson")

Signed-off-by Philipp Stanner <[email protected]>

Reviewed-by Andrew Lunn <[email protected]>

Reviewed-by Yanteng Si <[email protected]>

Tested-by Henry Chen <[email protected]>

---
 drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +4 -2

index bfe6e2d631bd..73a6715a93e6 100644
@@ -11,6 +11,8 @@
 #include "dwmac_dma.h"
 #include "dwmac1000.h"
 
+#define DRIVER_NAME "dwmac-loongson-pci"
+
 /* Normal Loongson Tx Summary */
 #define DMA_INTR_ENA_NIE_TX_LOONGSON	0x00040000
 /* Normal Loongson Rx Summary */

@@ -555,7 +557,7 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id
 	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));
+		ret = pcim_iomap_regions(pdev, BIT(0), DRIVER_NAME);
 		if (ret)
 			goto err_disable_device;
 		break;

@@ -673,7 +675,7 @@ static const struct pci_device_id loongson_dwmac_id_table[] = {
 MODULE_DEVICE_TABLE(pci, loongson_dwmac_id_table);
 
 static struct pci_driver loongson_dwmac_driver = {
-	.name = "dwmac-loongson-pci",
+	.name = DRIVER_NAME,
 	.id_table = loongson_dwmac_id_table,
 	.probe = loongson_dwmac_probe,
 	.remove = loongson_dwmac_remove,

Signature · 1 line

2.48.1

Jakub Kicinski

maintainer

2025-02-28 02:35 UTC #2 lore raw

Re: [PATCH net-next v4 1/4] stmmac: loongson: Pass correct arg to PCI function

Quoted 8 lines · On Wed, 26 Feb 2025 09:52:05 +0100 Philipp Stanner wrote:

pcim_iomap_regions() should receive the driver's name as its third parameter, not the PCI device's name.
Define the driver name with a macro and use it at the appropriate places, including pcim_iomap_regions().
Cc [email protected] # v5.14+
Fixes 30bba69d7db4 ("stmmac: pci: Add dwmac support for Loongson")

Since you sent this as a fix (which.. yea.. I guess.. why not..) I'll apply it to the fixes tree. But then the other patches have to wait and be reposted next Thu. The fixes are merged with net-next every Thu, but since this series was tagged as net-next I missed it in today's cross merge :(

Philipp Stanner

2025-02-28 09:26 UTC #3 lore raw

Re: [PATCH net-next v4 1/4] stmmac: loongson: Pass correct arg to PCI function

Quoted 7 lines · Jakub Kicinski

Quoted 6 lines · On Wed, 26 Feb 2025 09:52:05 +0100 Philipp Stanner wrote:
pcim_iomap_regions() should receive the driver's name as its third parameter, not the PCI device's name.
Define the driver name with a macro and use it at the appropriate places, including pcim_iomap_regions().

Quoted 9 lines

Quoted 2 lines
Cc [email protected] # v5.14+
Fixes 30bba69d7db4 ("stmmac: pci: Add dwmac support for Loongson")
Since you sent this as a fix (which.. yea.. I guess.. why not..) I'll apply it to the fixes tree. But then the other patches have to wait and be reposted next Thu. The fixes are merged with net-next every Thu, but since this series was tagged as net-next I missed it in today's cross merge :(

Oh OK, I see – I'm not very familiar with the net subsystem process. So far I always had it like this: fire everything into Linus's master and Greg & Sasha then pick those with Fixes tags into the stable trees automatically :)

Anyways, I interpret your message so that this series is done and I don't have to do anything about it anymore. Correct me if I'm wrong.

Thanks
P.

Jakub Kicinski

maintainer

2025-02-28 22:03 UTC #4 lore raw

Re: [PATCH net-next v4 1/4] stmmac: loongson: Pass correct arg to PCI function

Quoted 10 lines · On Fri, 28 Feb 2025 10:26:24 +0100 Philipp Stanner wrote:

Quoted 5 lines
Since you sent this as a fix (which.. yea.. I guess.. why not..) I'll apply it to the fixes tree. But then the other patches have to wait and be reposted next Thu. The fixes are merged with net-next every Thu, but since this series was tagged as net-next I missed it in today's cross merge :(
Oh OK, I see – I'm not very familiar with the net subsystem process. So far I always had it like this: fire everything into Linus's master and Greg & Sasha then pick those with Fixes tags into the stable trees automatically :)

Admittedly the fixes vs linux-next material routing is a little tricky.

Quoted 2 lines

Anyways, I interpret your message so that this series is done and I don't have to do anything about it anymore. Correct me if I'm wrong.

You need to repost patches 2,3,4 once the first patch appears in:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/

Philipp Stanner

author

2025-02-26 08:52 UTC #5 lore raw

PATCH v4 2/4 stmmac: loongson: Remove surplus loop

In reply to Philipp Stanner

loongson_dwmac_probe() contains a loop which doesn't have an effect, because it tries to call pcim_iomap_regions() with the same parameters several times. The break statement at the loop's end furthermore ensures that the loop only runs once anyways.

Remove the surplus loop.

Signed-off-by Philipp Stanner <[email protected]>

Reviewed-by Andrew Lunn <[email protected]>

Reviewed-by Yanteng Si <[email protected]>

Reviewed-by Huacai Chen <[email protected]>

Tested-by Henry Chen <[email protected]>

---
 .../net/ethernet/stmicro/stmmac/dwmac-loongson.c    | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +4 -9

index 73a6715a93e6..e2959ac5c1ca 100644
@@ -524,7 +524,7 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id
 	struct stmmac_pci_info *info;
 	struct stmmac_resources res;
 	struct loongson_data *ld;
-	int ret, i;
+	int ret;
 
 	plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL);
 	if (!plat)

@@ -554,14 +554,9 @@ 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), DRIVER_NAME);
-		if (ret)
-			goto err_disable_device;
-		break;
-	}
+	ret = pcim_iomap_regions(pdev, BIT(0), DRIVER_NAME);
+	if (ret)
+		goto err_disable_device;
 
 	memset(&res, 0, sizeof(res));
 	res.addr = pcim_iomap_table(pdev)[0];

Signature · 1 line

2.48.1

Philipp Stanner

author

2025-02-26 08:52 UTC #6 lore raw

PATCH v4 3/4 stmmac: Remove pcim_* functions for driver detach

In reply to Philipp Stanner

Functions prefixed with "pcim_" are managed devres functions which perform automatic cleanup once the driver unloads. It is, thus, not necessary to call any cleanup functions in remove() callbacks.

Remove the pcim_ cleanup function calls in the remove() callbacks.

Signed-off-by Philipp Stanner <[email protected]>

Reviewed-by Andrew Lunn <[email protected]>

Reviewed-by Yanteng Si <[email protected]>

Tested-by Henry Chen <[email protected]>

---
 drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c |  8 --------
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c     | 12 +-----------
 2 files changed, 1 insertion(+), 19 deletions(-)

drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +0 -8

index e2959ac5c1ca..5d7746d787ac 100644
@@ -603,7 +603,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);

@@ -614,13 +613,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);
 }

drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c +1 ~~-11~~

index 352b01678c22..1637c8139b9d 100644
@@ -226,21 +226,11 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
  * stmmac_pci_remove
  *
  * @pdev: platform device pointer
- * Description: this function calls the main to free the net resources
- * and releases the PCI resources.
+ * Description: this function calls the main to free the net resources.
  */
 static void stmmac_pci_remove(struct pci_dev *pdev)
 {
-	int i;
-
 	stmmac_dvr_remove(&pdev->dev);
-
-	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
-		if (pci_resource_len(pdev, i) == 0)
-			continue;
-		pcim_iounmap_regions(pdev, BIT(i));
-		break;
-	}
 }
 
 static int __maybe_unused stmmac_pci_suspend(struct device *dev)

Signature · 1 line

2.48.1

Philipp Stanner

author

2025-02-26 08:52 UTC #7 lore raw

PATCH v4 4/4 stmmac: Replace deprecated PCI functions

In reply to Philipp Stanner

From: Philipp Stanner <[email protected]>

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

Replace them with their successor function, pcim_iomap_region().

Make variable declaration order at closeby places comply with reverse christmas tree order.

Signed-off-by Philipp Stanner <[email protected]>

Reviewed-by Andrew Lunn <[email protected]>

Reviewed-by Huacai Chen <[email protected]>

Tested-by Henry Chen <[email protected]>

---
 .../net/ethernet/stmicro/stmmac/dwmac-loongson.c   |  8 +++-----
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c   | 14 ++++++--------
 2 files changed, 9 insertions(+), 13 deletions(-)

drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c +3 -5

index 5d7746d787ac..25ef7b9c5dce 100644
@@ -521,8 +521,8 @@ static int loongson_dwmac_acpi_config(struct pci_dev *pdev,
 static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	struct plat_stmmacenet_data *plat;
+	struct stmmac_resources res = {};
 	struct stmmac_pci_info *info;
-	struct stmmac_resources res;
 	struct loongson_data *ld;
 	int ret;

@@ -554,13 +554,11 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id
 	pci_set_master(pdev);
 
 	/* Get the base address of device */
-	ret = pcim_iomap_regions(pdev, BIT(0), DRIVER_NAME);
+	res.addr = pcim_iomap_region(pdev, 0, DRIVER_NAME);
+	ret = PTR_ERR_OR_ZERO(res.addr);
 	if (ret)
 		goto err_disable_device;
 
-	memset(&res, 0, sizeof(res));
-	res.addr = pcim_iomap_table(pdev)[0];
-
 	plat->bsp_priv = ld;
 	plat->setup = loongson_dwmac_setup;
 	ld->dev = &pdev->dev;

drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c +6 -8

index 1637c8139b9d..b7adda35b7b7 100644
@@ -155,9 +155,9 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
 {
 	struct stmmac_pci_info *info = (struct stmmac_pci_info *)id->driver_data;
 	struct plat_stmmacenet_data *plat;
-	struct stmmac_resources res;
-	int i;
+	struct stmmac_resources res = {};
 	int ret;
+	int i;
 
 	plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL);
 	if (!plat)

@@ -188,13 +188,13 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
 		return ret;
 	}
 
-	/* Get the base address of device */
+	/* The first BAR > 0 is the base IO addr of our device. */
 	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		if (pci_resource_len(pdev, i) == 0)
 			continue;
-		ret = pcim_iomap_regions(pdev, BIT(i), pci_name(pdev));
-		if (ret)
-			return ret;
+		res.addr = pcim_iomap_region(pdev, i, STMMAC_RESOURCE_NAME);
+		if (IS_ERR(res.addr))
+			return PTR_ERR(res.addr);
 		break;
 	}

@@ -204,8 +204,6 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		return ret;
 
-	memset(&res, 0, sizeof(res));
-	res.addr = pcim_iomap_table(pdev)[i];
 	res.wol_irq = pdev->irq;
 	res.irq = pdev->irq;

Signature · 1 line

2.48.1

[email protected]

2025-02-28 02:39 UTC #8 lore raw

In reply to Philipp Stanner

This series was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

Quoted 10 lines · On Wed, 26 Feb 2025 09:52:04 +0100 you wrote:

Changes in v4:
- Add missing full stop. (Yanteng)
- Move forgotten unused-variable-removes to the appropriate places.
- Add applicable RB / TB tags
Changes in v3:
- Several formatting nits (Paolo)
- Split up patch into a patch series (Yanteng)
[...]

Here is the summary with links:
- [net-next,v4,1/4] stmmac: loongson: Pass correct arg to PCI function

    https://git.kernel.org/netdev/net/c/00371a3f4877

- [net-next,v4,2/4] stmmac: loongson: Remove surplus loop (no matching commit)
- [net-next,v4,3/4] stmmac: Remove pcim_* functions for driver detach (no matching commit)
- [net-next,v4,4/4] stmmac: Replace deprecated PCI functions (no matching commit)

You are awesome, thank you!

Signature · 2 lines

Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html