Message-Id: <201403191333.01095.arnd@arndb.de>
Date:	Wed, 19 Mar 2014 13:33:00 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	Dinh Nguyen <dinguyen@...era.com>
Cc:	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
	arm@...nel.org, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org,
	Giuseppe Cavallaro <peppe.cavallaro@...com>,
	Srinivas Kandagatla <srinivas.kandagatla@...com>,
	Hans de Goede <hdegoede@...hat.com>
Subject: stmmac-socfpga breakage in arm-soc

Hi Dinh,

I just merged your stmmac socfpga glue driver with Ack from
David Miller, but then found multiple issues with it. I've
attempted to fix them up in an untested patch below, which
I could apply on top, if everyone thinks this is a good idea.

If we can't get agreement on this, it's probably better if
we revery your stmmac series for now and try to get it
right for 3.16.

On a more general note about the stmmac driver, I think it
should be restructured into a base 'library' driver and
multiple front-ends per SoC or interface (PCI, platform-generic,
sun7i, socfpga, sti, spear, ...) that can be built as
modules, like we do for other drivers that get reused a lot
with smaller variations.

Any comments?

---
>From f9e2d095026531ffaecda84daedf275735622cb1 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@...db.de>
Date: Wed, 19 Mar 2014 12:57:05 +0100
Subject: [PATCH] net: stmmac: improve binding and fix build

The new stmmac front-end for socfpga caused build failures
for modular drivers, e.g. 'make allmodconfig', which prompted
me to look closer into it. I found that both the DT binding
and the implementation of the driver are rather nonstandard
and do things very different from the other SoC specific glue
drivers for stmmac.

This puts things back in order, hopefully fixing all the
important issues:

* Replaced the parent/child DT nodes with a combined node
* Renamed the device node from 'ethernet0' to 'ethernet'
  as the standard name.
* Removed interrupt-names and clock-names properties that
  are not documented in the binding and not used.
* Added a new DWMAC_SOCFPGA Kconfig symbol to control
  compilation of this driver
* Removed code related to scanning the DT nodes that
  are no longer there.
* Replaced platform_driver and module stuff with call from
  main stmmac driver.
* Removed bogus of_machine_is_compatible("altr,socfpga-vt"))
  check that should be handled through separate compatible
  properties of the dwmac node itself.

Signed-off-by: Arnd Bergmann <arnd@...db.de>

diff --git a/Documentation/devicetree/bindings/net/socfpga-dwmac.txt b/Documentation/devicetree/bindings/net/socfpga-dwmac.txt
index d53d376..ec216e0 100644
--- a/Documentation/devicetree/bindings/net/socfpga-dwmac.txt
+++ b/Documentation/devicetree/bindings/net/socfpga-dwmac.txt
@@ -1,35 +1,25 @@
 Altera SOCFPGA SoC DWMAC controller
 
-The device node has following properties.
+This is a variant of the dwmac/stmmac driver an inherits all descriptions
+present in Documentation/devicetree/bindings/net/stmmac.txt.
+
+The device node has additional properties:
 
 Required properties:
- - compatible	: Should contain "altr,socfpga-stmmac"
+ - compatible	: Should contain "altr,socfpga-stmmac" along with
+		  "snps,dwmac" and any applicable more detailed
+		  designware version numbers documented in stmmac.txt
  - altr,sysmgr-syscon : Should be the phandle to the system manager node that
    encompasses the glue register, and the register offset.
 
-Sub-nodes:
-The dwmac core should be added as subnode to SOCFPGA dwmac glue.
-- dwmac :	The binding details of dwmac can be found in
-  Documentation/devicetree/bindings/net/stmmac.txt
-
 Example:
 
-ethernet0: ethernet0 {
-	#address-cells = <1>;
-	#size-cells = <1>;
-
-	compatible = "altr,socfpga-stmmac";
+gmac0: ethernet@...00000 {
+	compatible = "altr,socfpga-stmmac", "snps,dwmac-3.70a", "snps,dwmac";
 	altr,sysmgr-syscon = <&sysmgr 0x60>;
 	status = "disabled";
-	ranges;
-
-	gmac0: gmac0@...00000 {
-		compatible = "snps,dwmac-3.70a", "snps,dwmac";
-		reg = <0xff700000 0x2000>;
-		interrupts = <0 115 4>;
-		interrupt-names = "macirq";
-		mac-address = [00 00 00 00 00 00];/* Filled in by U-Boot */
-		clocks = <&emac0_clk>;
-		clock-names = "stmmaceth";
-	};
+	reg = <0xff700000 0x2000>;
+	interrupts = <0 115 4>;
+	mac-address = [00 00 00 00 00 00];/* Filled in by U-Boot */
+	clocks = <&emac0_clk>;
 };
diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index 6d7eaa4..1595712 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -451,43 +451,24 @@
 				};
 			};
 
-		ethernet0: ethernet0 {
-			#address-cells = <1>;
-			#size-cells = <1>;
-			compatible = "altr,socfpga-stmmac";
-			altr,sysmgr-syscon = <&sysmgr 0x60>;
+		gmac0: ethernet@...00000 {
+			compatible = "altr,socfpga-stmmac", "snps,dwmac-3.70a", "snps,dwmac";
 			status = "disabled";
-			ranges;
-
-			gmac0: gmac0@...00000 {
-				compatible = "snps,dwmac-3.70a", "snps,dwmac";
-				reg = <0xff700000 0x2000>;
-				interrupts = <0 115 4>;
-				interrupt-names = "macirq";
-				mac-address = [00 00 00 00 00 00];/* Filled in by U-Boot */
-				clocks = <&emac0_clk>;
-				clock-names = "stmmaceth";
-			};
+			altr,sysmgr-syscon = <&sysmgr 0x60>;
+			reg = <0xff700000 0x2000>;
+			interrupts = <0 115 4>;
+			mac-address = [00 00 00 00 00 00];/* Filled in by U-Boot */
+			clocks = <&emac0_clk>;
 		};
 
-		ethernet1: ethernet1 {
-			#address-cells = <1>;
-			#size-cells = <1>;
-			compatible = "altr,socfpga-stmmac";
-			altr,sysmgr-syscon = <&sysmgr 0x60>;
+		gmac1: ethernet@...02000 {
+			compatible = "altr,socfpga-stmmac", "snps,dwmac-3.70a", "snps,dwmac";
 			status = "disabled";
-			ranges;
-
-			gmac1: gmac1@...02000 {
-				device_type = "network";
-				compatible = "snps,dwmac-3.70a", "snps,dwmac";
-				reg = <0xff702000 0x2000>;
-				interrupts = <0 120 4>;
-				interrupt-names = "macirq";
-				mac-address = [00 00 00 00 00 00];/* Filled in by U-Boot */
-				clocks = <&emac1_clk>;
-				clock-names = "stmmaceth";
-			};
+			altr,sysmgr-syscon = <&sysmgr 0x60>;
+			reg = <0xff702000 0x2000>;
+			interrupts = <0 120 4>;
+			mac-address = [00 00 00 00 00 00];/* Filled in by U-Boot */
+			clocks = <&emac1_clk>;
 		};
 
 		L2: l2-cache@...ef000 {
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index f2d7c70..f8d5112 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -26,6 +26,16 @@ config STMMAC_PLATFORM
 
 	  If unsure, say N.
 
+config DWMAC_SOCFPGA
+	bool "SOCFPGA dwmac support"
+	depends on STMMAC_PLATFORM && MFD_SYSCON && (ARCH_SOCFPGA || COMPILE_TEST)
+	help
+	  Support for ethernet controller on Altera SOCFPGA
+
+	  This selects the Altera SOCFGA SoC glue layer support
+	  for the stmmac device driver. This driver is used for
+	  arria5 and cyclone5 FPGA SoCs.
+
 config DWMAC_SUNXI
 	bool "Allwinner GMAC support"
 	depends on STMMAC_PLATFORM && ARCH_SUNXI
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index fd5e48d..18695eb 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -1,9 +1,9 @@
 obj-$(CONFIG_STMMAC_ETH) += stmmac.o
-stmmac-$(CONFIG_ARCH_SOCFPGA) += dwmac-socfpga.o
 stmmac-$(CONFIG_STMMAC_PLATFORM) += stmmac_platform.o
 stmmac-$(CONFIG_STMMAC_PCI) += stmmac_pci.o
 stmmac-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o
 stmmac-$(CONFIG_DWMAC_STI) += dwmac-sti.o
+stmmac-$(CONFIG_DWMAC_SOCFPGA) += dwmac-socfpga.o
 stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o ring_mode.o	\
 	      chain_mode.o dwmac_lib.o dwmac1000_core.o  dwmac1000_dma.o \
 	      dwmac100_core.o dwmac100_dma.o enh_desc.o  norm_desc.o \
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index c7f034b..dd3de82 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -40,30 +40,16 @@ struct socfpga_dwmac {
 	u32	reg_offset;
 	struct	device *dev;
 	struct regmap *sys_mgr_base_addr;
-	struct	device_node *dwmac_np;
 };
 
 static int socfpga_dwmac_parse_data(struct socfpga_dwmac *dwmac, struct device *dev)
 {
 	struct device_node *np	= dev->of_node;
-	struct device_node *stmmac_np;
 	struct regmap *sys_mgr_base_addr;
 	u32 reg_offset;
 	int ret;
 
-	stmmac_np = of_get_next_available_child(np, NULL);
-	if (!stmmac_np) {
-		dev_info(dev, "No dwmac node found\n");
-		return -EINVAL;
-	}
-
-	if (!of_device_is_compatible(stmmac_np, "snps,dwmac")) {
-		dev_info(dev, "dwmac node isn't compatible with snps,dwmac\n");
-		return -EINVAL;
-	}
-
-	dwmac->interface = of_get_phy_mode(stmmac_np);
-	of_node_put(stmmac_np);
+	dwmac->interface = of_get_phy_mode(np);
 
 	sys_mgr_base_addr = syscon_regmap_lookup_by_phandle(np, "altr,sysmgr-syscon");
 	if (IS_ERR(sys_mgr_base_addr)) {
@@ -79,7 +65,6 @@ static int socfpga_dwmac_parse_data(struct socfpga_dwmac *dwmac, struct device *
 
 	dwmac->reg_offset = reg_offset;
 	dwmac->sys_mgr_base_addr = sys_mgr_base_addr;
-	dwmac->dwmac_np = stmmac_np;
 	dwmac->dev = dev;
 
 	return 0;
@@ -92,9 +77,6 @@ static int socfpga_dwmac_setup(struct socfpga_dwmac *dwmac)
 	u32 reg_offset = dwmac->reg_offset;
 	u32 ctrl, val, shift = 0;
 
-	if (of_machine_is_compatible("altr,socfpga-vt"))
-		return 0;
-
 	switch (phymode) {
 	case PHY_INTERFACE_MODE_RGMII:
 		val = SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_RGMII;
@@ -116,68 +98,31 @@ static int socfpga_dwmac_setup(struct socfpga_dwmac *dwmac)
 	return 0;
 }
 
-static int socfpga_dwmac_probe(struct platform_device *pdev)
+static void *socfpga_dwmac_probe(struct platform_device *pdev)
 {
 	struct device		*dev = &pdev->dev;
-	struct device_node	*node = dev->of_node;
-	int			ret = -ENOMEM;
+	int			ret;
 	struct socfpga_dwmac	*dwmac;
 
 	dwmac = devm_kzalloc(dev, sizeof(*dwmac), GFP_KERNEL);
 	if (!dwmac)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	ret = socfpga_dwmac_parse_data(dwmac, dev);
 	if (ret) {
 		dev_err(dev, "Unable to parse OF data\n");
-		return ret;
+		return ERR_PTR(ret);
 	}
 
 	ret = socfpga_dwmac_setup(dwmac);
 	if (ret) {
 		dev_err(dev, "couldn't setup SoC glue (%d)\n", ret);
-		return ret;
-	}
-
-	if (node) {
-		ret = of_platform_populate(node, NULL, NULL, dev);
-		if (ret) {
-			dev_err(dev, "failed to add dwmac core\n");
-			return ret;
-		}
-	} else {
-		dev_err(dev, "no device node, failed to add dwmac core\n");
-		return -ENODEV;
+		return ERR_PTR(ret);
 	}
 
-	platform_set_drvdata(pdev, dwmac);
-
-	return 0;
+	return dwmac;
 }
 
-static int socfpga_dwmac_remove(struct platform_device *pdev)
-{
-	return 0;
-}
-
-static const struct of_device_id socfpga_dwmac_match[] = {
-	{ .compatible = "altr,socfpga-stmmac" },
-	{},
-};
-MODULE_DEVICE_TABLE(of, socfpga_dwmac_match);
-
-static struct platform_driver socfpga_dwmac_driver = {
-	.probe		= socfpga_dwmac_probe,
-	.remove		= socfpga_dwmac_remove,
-	.driver		= {
-		.name	= "socfpga-dwmac",
-		.of_match_table = of_match_ptr(socfpga_dwmac_match),
-	},
+const struct stmmac_of_data socfpga_gmac_data = {
+	.setup = socfpga_dwmac_probe,	
 };
-
-module_platform_driver(socfpga_dwmac_driver);
-
-MODULE_ALIAS("platform:socfpga-dwmac");
-MODULE_AUTHOR("Dinh Nguyen <dinguyen@...era.com>");
-MODULE_LICENSE("GPL v2");
-MODULE_DESCRIPTION("Altera SOCFPGA DWMAC Glue Layer");
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index f9e60d7..cd2f6a1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -136,6 +136,7 @@ extern const struct stmmac_of_data sun7i_gmac_data;
 #ifdef CONFIG_DWMAC_STI
 extern const struct stmmac_of_data sti_gmac_data;
 #endif
+extern const struct stmmac_of_data socfpga_gmac_data;
 extern struct platform_driver stmmac_pltfr_driver;
 static inline int stmmac_register_platform(void)
 {
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 8fb32a8..46aef510 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -38,6 +38,9 @@ static const struct of_device_id stmmac_dt_ids[] = {
 	{ .compatible = "st,stih416-dwmac", .data = &sti_gmac_data},
 	{ .compatible = "st,stid127-dwmac", .data = &sti_gmac_data},
 #endif
+#ifdef CONFIG_DWMAC_SOCFPGA
+	{ .compatible = "altr,socfpga-stmmac", .data = &socfpga_gmac_data },
+#endif
 	/* SoC specific glue layers should come before generic bindings */
 	{ .compatible = "st,spear600-gmac"},
 	{ .compatible = "snps,dwmac-3.610"},
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists