[PATCH 0/3] stmmac patches

[PATCH 0/3] stmmac patches

[Alessandro Rubini]

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(-)

-- 
1.7.7.2

[PATCH 1/3] stmmac: added PCI identifiers

[Alessandro Rubini]

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 <rubini@gnudd.com>
Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 54a819a..c796de9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -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);
-- 
1.7.7.2

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

[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 <rubini@gnudd.com>
Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Acked-by: Rayagond Kokatanur <rayagond@vayavyalabs.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c |   47 ++++++++++++----------
 1 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index c796de9..b075a5d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -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
-- 
1.7.7.2

[PATCH 3/3] stmmac: remove kernel cmdline parsing

[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 <rubini@gnudd.com>
Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |   59 ---------------------
 1 files changed, 0 insertions(+), 59 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 96fa2da..625d0d1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -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 <peppe.cavallaro@st.com>");
 MODULE_LICENSE("GPL");
-- 
1.7.7.2

Re: [PATCH 0/3] stmmac patches

[David Miller]

From: Alessandro Rubini <rubini@gnudd.com>
Date: Tue, 24 Jan 2012 10:08:45 +0100

> 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.

> 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.

> 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.

Re: [PATCH 0/3] stmmac patches

[Alessandro Rubini]

>> 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.
 
> 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.

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

Fine with me.

thanks
/alessandro