Skip to contentSkip to conversation

stmmac patches

PATCH In review Alessandro Rubini 2 participants 6 messages Jan 24, 2012

Subsystems (4)
Networking Network drivers Arm STM32 stmmac Ethernet driver
lore
Share & tools
lore

Alessandro Rubini

author
#0 lore raw

cover stmmac patches

The following three patches add support for the STA2X11 device in the stmmac driver. The device is a PCI-e bridge, so I need just to add my identifiers. The identifiers being added are already in pci_ids of linux-next.

The second patch is a cleanup, already acked by both authors.

As for the third, I noted that there is "#ifndef MODULE" adding __setup directives, but the module parameters already work, and I successfully use stmmac.phy_id=1 to boot my system, so the __setup adre just duplicates. This has not been acked by Giuseppe Cavallaro but I offer it here nonetheless.

Alessandro Rubini (3):
stmmac: added PCI identifiers
stmmac: err out on second probe; use precompiled static structures
stmmac: remove kernel cmdline parsing

 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |   59 ---------------------
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c  |   53 ++++++++++--------
 2 files changed, 29 insertions(+), 83 deletions(-)
Signature · 1 line
1.7.7.2
author
#1 lore raw

PATCH 1/3 stmmac: added PCI identifiers

STM has a device ID within its own VENDOR space, and it is being used in the STA2X11 I/O Hub.

Signed-off-by Alessandro Rubini <[email protected]>
Acked-by Giancarlo Asnaghi <[email protected]>
Acked-by Giuseppe Cavallaro <[email protected]>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c +3 -3
index 54a819a..c796de9 100644
@@ -170,9 +170,9 @@ static int stmmac_pci_resume(struct pci_dev *pdev)
 #define STMMAC_DEVICE_ID 0x1108
 
 static DEFINE_PCI_DEVICE_TABLE(stmmac_id_table) = {
-	{
-	PCI_DEVICE(STMMAC_VENDOR_ID, STMMAC_DEVICE_ID)}, {
-	}
+	{PCI_DEVICE(STMMAC_VENDOR_ID, STMMAC_DEVICE_ID)},
+	{PCI_DEVICE(PCI_VENDOR_ID_STMICRO, PCI_DEVICE_ID_STMICRO_MAC)},
+	{}
 };
 
 MODULE_DEVICE_TABLE(pci, stmmac_id_table);
Signature · 1 line
1.7.7.2
author
#2 lore raw

PATCH 2/3 stmmac: err out on second probe; use precompiled static structures

In reply to Alessandro Rubini

The driver can support one device only, and this patch makes it explicit, spitting an error in case someone makes an ASIC or FPGA with two stmmac cells in it.

Also, the data structures are static and compile-time initialized, instead of being in the global name space and filled at run time. This also eases turning some fields to parameters, as different chips will have different setups.

Signed-off-by Alessandro Rubini <[email protected]>
Acked-by Giancarlo Asnaghi <[email protected]>
Acked-by Giuseppe Cavallaro <[email protected]>
Acked-by Rayagond Kokatanur <[email protected]>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c |   47 ++++++++++++----------
 1 files changed, 26 insertions(+), 21 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c +26 -21
index c796de9..b075a5d 100644
@@ -24,27 +24,26 @@
 *******************************************************************************/
 
 #include <linux/pci.h>
+#include <linux/atomic.h>
 #include "stmmac.h"
 
-struct plat_stmmacenet_data plat_dat;
-struct stmmac_mdio_bus_data mdio_data;
+/* This driver supports one PCI device only, with static platform data */
+static atomic_t stmmac_pci_probed;
 
-static void stmmac_default_data(void)
-{
-	memset(&plat_dat, 0, sizeof(struct plat_stmmacenet_data));
-	plat_dat.bus_id = 1;
-	plat_dat.phy_addr = 0;
-	plat_dat.interface = PHY_INTERFACE_MODE_GMII;
-	plat_dat.pbl = 32;
-	plat_dat.clk_csr = 2;	/* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */
-	plat_dat.has_gmac = 1;
-	plat_dat.force_sf_dma_mode = 1;
-
-	mdio_data.bus_id = 1;
-	mdio_data.phy_reset = NULL;
-	mdio_data.phy_mask = 0;
-	plat_dat.mdio_bus_data = &mdio_data;
-}
+static struct stmmac_mdio_bus_data stmmac_default_mdio_data = {
+	.bus_id = 1,
+};
+
+static struct plat_stmmacenet_data stmmac_default_data = {
+	.bus_id = 1,
+	.phy_addr = 0,
+	.interface = PHY_INTERFACE_MODE_GMII,
+	.pbl = 32,
+	.clk_csr = 2,	/* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */
+	.has_gmac = 1,
+	.force_sf_dma_mode = 1,
+	.mdio_bus_data = &stmmac_default_mdio_data,
+};
 
 /**
  * stmmac_pci_probe

@@ -66,6 +65,13 @@ static int __devinit stmmac_pci_probe(struct pci_dev *pdev,
 	struct stmmac_priv *priv = NULL;
 	int i;
 
+	/* One device only in this version */
+	if (atomic_inc_and_test(&stmmac_pci_probed)) {
+		atomic_dec(&stmmac_pci_probed);
+		dev_err(&pdev->dev, "this driver supports one device only\n");
+		return -ENODEV;
+	}
+
 	/* Enable pci device */
 	ret = pci_enable_device(pdev);
 	if (ret) {

@@ -94,9 +100,7 @@ static int __devinit stmmac_pci_probe(struct pci_dev *pdev,
 	}
 	pci_set_master(pdev);
 
-	stmmac_default_data();
-
-	priv = stmmac_dvr_probe(&(pdev->dev), &plat_dat);
+	priv = stmmac_dvr_probe(&(pdev->dev), &stmmac_default_data);
 	if (!priv) {
 		pr_err("%s: main drivr probe failed", __func__);
 		goto err_out;

@@ -140,6 +144,7 @@ static void __devexit stmmac_pci_remove(struct pci_dev *pdev)
 	pci_iounmap(pdev, priv->ioaddr);
 	pci_release_regions(pdev);
 	pci_disable_device(pdev);
+	atomic_dec(&stmmac_pci_probed);
 }
 
 #ifdef CONFIG_PM
Signature · 1 line
1.7.7.2
author
#3 lore raw

PATCH 3/3 stmmac: remove kernel cmdline parsing

In reply to Alessandro Rubini

All configurable arguments are already there as module_param, and they work in the main command line as well, by saying
e.g. "stmmac.phyaddr=1" (I do it on my board).

Signed-off-by Alessandro Rubini <[email protected]>
Acked-by Giancarlo Asnaghi <[email protected]>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |   59 ---------------------
 1 files changed, 0 insertions(+), 59 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +0 -59
index 96fa2da..625d0d1 100644
@@ -1981,65 +1981,6 @@ int stmmac_restore(struct net_device *ndev)
 }
 #endif /* CONFIG_PM */
 
-#ifndef MODULE
-static int __init stmmac_cmdline_opt(char *str)
-{
-	char *opt;
-
-	if (!str || !*str)
-		return -EINVAL;
-	while ((opt = strsep(&str, ",")) != NULL) {
-		if (!strncmp(opt, "debug:", 6)) {
-			if (strict_strtoul(opt + 6, 0, (unsigned long *)&debug))
-				goto err;
-		} else if (!strncmp(opt, "phyaddr:", 8)) {
-			if (strict_strtoul(opt + 8, 0,
-					   (unsigned long *)&phyaddr))
-				goto err;
-		} else if (!strncmp(opt, "dma_txsize:", 11)) {
-			if (strict_strtoul(opt + 11, 0,
-					   (unsigned long *)&dma_txsize))
-				goto err;
-		} else if (!strncmp(opt, "dma_rxsize:", 11)) {
-			if (strict_strtoul(opt + 11, 0,
-					   (unsigned long *)&dma_rxsize))
-				goto err;
-		} else if (!strncmp(opt, "buf_sz:", 7)) {
-			if (strict_strtoul(opt + 7, 0,
-					   (unsigned long *)&buf_sz))
-				goto err;
-		} else if (!strncmp(opt, "tc:", 3)) {
-			if (strict_strtoul(opt + 3, 0, (unsigned long *)&tc))
-				goto err;
-		} else if (!strncmp(opt, "watchdog:", 9)) {
-			if (strict_strtoul(opt + 9, 0,
-					   (unsigned long *)&watchdog))
-				goto err;
-		} else if (!strncmp(opt, "flow_ctrl:", 10)) {
-			if (strict_strtoul(opt + 10, 0,
-					   (unsigned long *)&flow_ctrl))
-				goto err;
-		} else if (!strncmp(opt, "pause:", 6)) {
-			if (strict_strtoul(opt + 6, 0, (unsigned long *)&pause))
-				goto err;
-#ifdef CONFIG_STMMAC_TIMER
-		} else if (!strncmp(opt, "tmrate:", 7)) {
-			if (strict_strtoul(opt + 7, 0,
-					   (unsigned long *)&tmrate))
-				goto err;
-#endif
-		}
-	}
-	return 0;
-
-err:
-	pr_err("%s: ERROR broken module parameter conversion", __func__);
-	return -EINVAL;
-}
-
-__setup("stmmaceth=", stmmac_cmdline_opt);
-#endif
-
 MODULE_DESCRIPTION("STMMAC 10/100/1000 Ethernet device driver");
 MODULE_AUTHOR("Giuseppe Cavallaro <[email protected]>");
 MODULE_LICENSE("GPL");
Signature · 1 line
1.7.7.2
maintainer
#4 lore raw
In reply to Alessandro Rubini

From: Alessandro Rubini <[email protected]>
Date: Tue, 24 Jan 2012 10:08:45 +0100

Quoted 4 lines

The following three patches add support for the STA2X11 device in the stmmac driver. The device is a PCI-e bridge, so I need just to add my identifiers. The identifiers being added are already in pci_ids of linux-next.

This patch is fine.

Quoted 1 line

The second patch is a cleanup, already acked by both authors.

I'm not applying this, instead if you want to make changes in this area then fix this driver so that it can properly support multiple device instances.

I can't even imagine ever writing a driver where from the start it can handle only one device instance, this is pure insanity especially for a PCI device driver.

Quoted 5 lines

As for the third, I noted that there is "#ifndef MODULE" adding __setup directives, but the module parameters already work, and I successfully use stmmac.phy_id=1 to boot my system, so the __setup adre just duplicates. This has not been acked by Giuseppe Cavallaro but I offer it here nonetheless.

You're breaking things for anyone using 'stmmaceth='. So I'm not applying this patch either.

author
#5 lore raw
Quoted 5 lines
Quoted 1 line

The second patch is a cleanup, already acked by both authors.

I'm not applying this, instead if you want to make changes in this area then fix this driver so that it can properly support multiple device instances.

Ok, I'll do, but not soon.

Quoted 3 lines

I can't even imagine ever writing a driver where from the start it can handle only one device instance, this is pure insanity especially for a PCI device driver.

I agree. But it _already_ is like this. I just want the fact to be clear, so when somebody burns two macs in the fpga it will complain
instead of breaking. That said, I have no problem if you still
refuse it.

Quoted 2 lines

You're breaking things for anyone using 'stmmaceth='. So I'm not applying this patch either.

Fine with me.

thanks
/alessandro