[PATCH 0/4] Add Ethernet support on STM32F429

[PATCH 0/4] Add Ethernet support on STM32F429

[Alexandre TORGUE]

STM32F429 Chip embeds a Synopsys 3.50a MAC IP.
This series:
	-enhance current stmmac driver to control it (code already available) and 
	 adds basic glue for STM32F429 chip.
	-Enable basic Net config in kernel.

Note that DT patches are not present because STM32 pinctrl code is not
yet avalaible.

Regards 

Alexandre

Alexandre TORGUE (4):
  net: ethernet: dwmac: add Ethernet glue logic for stm32 chip
  Documentation: Bindings: Add STM32 DWMAC glue
  net: ethernet: stmmac: add support of Synopsys 3.50a MAC IP
  ARM: STM32: Enable Ethernet in stm32_defconfig

 .../devicetree/bindings/net/stm32-dwmac.txt        |  41 +++++
 arch/arm/configs/stm32_defconfig                   |   9 ++
 drivers/net/ethernet/stmicro/stmmac/Kconfig        |  12 ++
 drivers/net/ethernet/stmicro/stmmac/Makefile       |   1 +
 drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c  | 177 +++++++++++++++++++++
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |   1 +
 6 files changed, 241 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/stm32-dwmac.txt
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c

-- 
1.9.1

[PATCH 0/4] Add Ethernet support on STM32F429

[Alexandre TORGUE]

STM32F429 Chip embeds a Synopsys 3.50a MAC IP.
This series:
	-enhance current stmmac driver to control it (code already available) and 
	 adds basic glue for STM32F429 chip.
	-Enable basic Net config in kernel.

Note that DT patches are not present because STM32 pinctrl code is not
yet avalaible.

Regards 

Alexandre

Alexandre TORGUE (4):
  net: ethernet: dwmac: add Ethernet glue logic for stm32 chip
  Documentation: Bindings: Add STM32 DWMAC glue
  net: ethernet: stmmac: add support of Synopsys 3.50a MAC IP
  ARM: STM32: Enable Ethernet in stm32_defconfig

 .../devicetree/bindings/net/stm32-dwmac.txt        |  41 +++++
 arch/arm/configs/stm32_defconfig                   |   9 ++
 drivers/net/ethernet/stmicro/stmmac/Kconfig        |  12 ++
 drivers/net/ethernet/stmicro/stmmac/Makefile       |   1 +
 drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c  | 177 +++++++++++++++++++++
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |   1 +
 6 files changed, 241 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/stm32-dwmac.txt
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c

-- 
1.9.1

[PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Alexandre TORGUE]

stm324xx family chips support Synopsys MAC 3.510 IP.
This patch adds settings for logical glue logic:
-clocks
-mode selection MII or RMII.

Signed-off-by: Alexandre TORGUE <alexandre.torgue@gmail.com>

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index cec147d..a94dd15 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -114,6 +114,18 @@ config DWMAC_SUNXI
 	  This selects Allwinner SoC glue layer support for the
 	  stmmac device driver. This driver is used for A20/A31
 	  GMAC ethernet controller.
+
+config DWMAC_STM32
+	tristate "STM32 DWMAC support"
+	default ARCH_STM32
+	depends on OF
+	select MFD_SYSCON
+	---help---
+	  Support for ethernet controller on STM32 SOCs.
+
+	  This selects STM32 SoC glue layer support for the stmmac
+	  device driver. This driver is used on for the STM32 series
+	  SOCs GMAC ethernet controller.
 endif
 
 config STMMAC_PCI
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index b390161..9fb2061 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_DWMAC_SOCFPGA)	+= dwmac-socfpga.o
 obj-$(CONFIG_DWMAC_STI)		+= dwmac-sti.o
 obj-$(CONFIG_DWMAC_SUNXI)	+= dwmac-sunxi.o
 obj-$(CONFIG_DWMAC_GENERIC)	+= dwmac-generic.o
+obj-$(CONFIG_DWMAC_STM32)	+= dwmac-stm32.o
 stmmac-platform-objs:= stmmac_platform.o
 
 obj-$(CONFIG_STMMAC_PCI) += stmmac-pci.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
new file mode 100644
index 0000000..56ccc20
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
@@ -0,0 +1,177 @@
+/*
+ * dwmac-stm32.c - DWMAC Specific Glue layer for STM32 MCU
+ *
+ * Copyright (C) Alexandre Torgue 2015
+ * Author:  Alexandre Torgue <alexandre.torgue@gmail.com>
+ * License terms:  GNU General Public License (GPL), version 2
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/stmmac.h>
+#include <linux/phy.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_net.h>
+
+#include "stmmac_platform.h"
+
+#define MII_PHY_SEL_MASK	BIT(23)
+
+struct stm32_dwmac {
+	int interface;		/* MII interface */
+	struct clk *clk_tx;
+	struct clk *clk_rx;
+	u32 mode_reg;		/* MAC glue-logic mode register */
+	struct device *dev;
+	struct regmap *regmap;
+	u32 speed;
+};
+
+static int stm32_dwmac_init(struct platform_device *pdev, void *priv)
+{
+	struct stm32_dwmac *dwmac = priv;
+	struct regmap *regmap = dwmac->regmap;
+	int ret, iface = dwmac->interface;
+	u32 reg = dwmac->mode_reg;
+	u32 val;
+
+	if (dwmac->clk_tx)
+		ret = clk_prepare_enable(dwmac->clk_tx);
+		if (ret)
+			goto out;
+
+	if (dwmac->clk_rx)
+		ret = clk_prepare_enable(dwmac->clk_rx);
+		if (ret)
+			goto out_disable_clk_tx;
+
+	val = (iface == PHY_INTERFACE_MODE_MII) ? 0 : 1;
+	ret = regmap_update_bits(regmap, reg, MII_PHY_SEL_MASK, val);
+	if (ret)
+		goto out_disable_clk_tx_rx;
+
+	return 0;
+
+out_disable_clk_tx_rx:
+	clk_disable_unprepare(dwmac->clk_rx);
+out_disable_clk_tx:
+	clk_disable_unprepare(dwmac->clk_tx);
+out:
+	return ret;
+}
+
+static void stm32_dwmac_exit(struct platform_device *pdev, void *priv)
+{
+	struct stm32_dwmac *dwmac = priv;
+
+	if (dwmac->clk_tx)
+		clk_disable_unprepare(dwmac->clk_tx);
+	if (dwmac->clk_rx)
+		clk_disable_unprepare(dwmac->clk_rx);
+}
+
+static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
+				  struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct regmap *regmap;
+	int err;
+
+	if (!np)
+		return -EINVAL;
+
+	/*  Get TX/RX clocks */
+	dwmac->clk_tx = devm_clk_get(dev, "tx-clk");
+	if (IS_ERR(dwmac->clk_tx)) {
+		dev_warn(dev, "No tx clock provided...\n");
+		dwmac->clk_tx = NULL;
+	}
+	dwmac->clk_rx = devm_clk_get(dev, "rx-clk");
+	if (IS_ERR(dwmac->clk_rx)) {
+		dev_warn(dev, "No rx clock provided...\n");
+		dwmac->clk_rx = NULL;
+	}
+
+	/* Get mode register */
+	regmap = syscon_regmap_lookup_by_phandle(np, "st,syscon");
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	err = of_property_read_u32_index(np, "st,syscon", 1, &dwmac->mode_reg);
+	if (err) {
+		dev_err(dev, "Can't get sysconfig mode offset (%d)\n", err);
+		return err;
+	}
+
+	dwmac->dev = dev;
+	dwmac->interface = of_get_phy_mode(np);
+	dwmac->regmap = regmap;
+
+	return 0;
+}
+
+static int stm32_dwmac_probe(struct platform_device *pdev)
+{
+	struct plat_stmmacenet_data *plat_dat;
+	struct stmmac_resources stmmac_res;
+	struct stm32_dwmac *dwmac;
+	int ret;
+
+	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
+	if (ret)
+		return ret;
+
+	plat_dat = stmmac_probe_config_dt(pdev, &stmmac_res.mac);
+	if (IS_ERR(plat_dat))
+		return PTR_ERR(plat_dat);
+
+	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
+	if (!dwmac)
+		return -ENOMEM;
+
+	ret = stm32_dwmac_parse_data(dwmac, pdev);
+	if (ret) {
+		dev_err(&pdev->dev, "Unable to parse OF data\n");
+		return ret;
+	}
+
+	plat_dat->bsp_priv = dwmac;
+	plat_dat->init = stm32_dwmac_init;
+	plat_dat->exit = stm32_dwmac_exit;
+
+	ret = stm32_dwmac_init(pdev, plat_dat->bsp_priv);
+	if (ret)
+		return ret;
+
+	return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+}
+
+static const struct of_device_id stm32_dwmac_match[] = {
+	{ .compatible = "st,stm32-dwmac"},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, stm32_dwmac_match);
+
+static struct platform_driver stm32_dwmac_driver = {
+	.probe  = stm32_dwmac_probe,
+	.remove = stmmac_pltfr_remove,
+	.driver = {
+		.name           = "stm32-dwmac",
+		.pm		= &stmmac_pltfr_pm_ops,
+		.of_match_table = stm32_dwmac_match,
+	},
+};
+module_platform_driver(stm32_dwmac_driver);
+
+MODULE_AUTHOR("Alexandre Torgue <alexandre.torgue@gmail.com>");
+MODULE_DESCRIPTION("STMicroelectronics MCU DWMAC Specific Glue layer");
+MODULE_LICENSE("GPL");
+
-- 
1.9.1

[PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Alexandre TORGUE]

stm324xx family chips support Synopsys MAC 3.510 IP.
This patch adds settings for logical glue logic:
-clocks
-mode selection MII or RMII.

Signed-off-by: Alexandre TORGUE <alexandre.torgue@gmail.com>

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index cec147d..a94dd15 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -114,6 +114,18 @@ config DWMAC_SUNXI
 	  This selects Allwinner SoC glue layer support for the
 	  stmmac device driver. This driver is used for A20/A31
 	  GMAC ethernet controller.
+
+config DWMAC_STM32
+	tristate "STM32 DWMAC support"
+	default ARCH_STM32
+	depends on OF
+	select MFD_SYSCON
+	---help---
+	  Support for ethernet controller on STM32 SOCs.
+
+	  This selects STM32 SoC glue layer support for the stmmac
+	  device driver. This driver is used on for the STM32 series
+	  SOCs GMAC ethernet controller.
 endif
 
 config STMMAC_PCI
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index b390161..9fb2061 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_DWMAC_SOCFPGA)	+= dwmac-socfpga.o
 obj-$(CONFIG_DWMAC_STI)		+= dwmac-sti.o
 obj-$(CONFIG_DWMAC_SUNXI)	+= dwmac-sunxi.o
 obj-$(CONFIG_DWMAC_GENERIC)	+= dwmac-generic.o
+obj-$(CONFIG_DWMAC_STM32)	+= dwmac-stm32.o
 stmmac-platform-objs:= stmmac_platform.o
 
 obj-$(CONFIG_STMMAC_PCI) += stmmac-pci.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
new file mode 100644
index 0000000..56ccc20
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
@@ -0,0 +1,177 @@
+/*
+ * dwmac-stm32.c - DWMAC Specific Glue layer for STM32 MCU
+ *
+ * Copyright (C) Alexandre Torgue 2015
+ * Author:  Alexandre Torgue <alexandre.torgue@gmail.com>
+ * License terms:  GNU General Public License (GPL), version 2
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/stmmac.h>
+#include <linux/phy.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_net.h>
+
+#include "stmmac_platform.h"
+
+#define MII_PHY_SEL_MASK	BIT(23)
+
+struct stm32_dwmac {
+	int interface;		/* MII interface */
+	struct clk *clk_tx;
+	struct clk *clk_rx;
+	u32 mode_reg;		/* MAC glue-logic mode register */
+	struct device *dev;
+	struct regmap *regmap;
+	u32 speed;
+};
+
+static int stm32_dwmac_init(struct platform_device *pdev, void *priv)
+{
+	struct stm32_dwmac *dwmac = priv;
+	struct regmap *regmap = dwmac->regmap;
+	int ret, iface = dwmac->interface;
+	u32 reg = dwmac->mode_reg;
+	u32 val;
+
+	if (dwmac->clk_tx)
+		ret = clk_prepare_enable(dwmac->clk_tx);
+		if (ret)
+			goto out;
+
+	if (dwmac->clk_rx)
+		ret = clk_prepare_enable(dwmac->clk_rx);
+		if (ret)
+			goto out_disable_clk_tx;
+
+	val = (iface == PHY_INTERFACE_MODE_MII) ? 0 : 1;
+	ret = regmap_update_bits(regmap, reg, MII_PHY_SEL_MASK, val);
+	if (ret)
+		goto out_disable_clk_tx_rx;
+
+	return 0;
+
+out_disable_clk_tx_rx:
+	clk_disable_unprepare(dwmac->clk_rx);
+out_disable_clk_tx:
+	clk_disable_unprepare(dwmac->clk_tx);
+out:
+	return ret;
+}
+
+static void stm32_dwmac_exit(struct platform_device *pdev, void *priv)
+{
+	struct stm32_dwmac *dwmac = priv;
+
+	if (dwmac->clk_tx)
+		clk_disable_unprepare(dwmac->clk_tx);
+	if (dwmac->clk_rx)
+		clk_disable_unprepare(dwmac->clk_rx);
+}
+
+static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
+				  struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct regmap *regmap;
+	int err;
+
+	if (!np)
+		return -EINVAL;
+
+	/*  Get TX/RX clocks */
+	dwmac->clk_tx = devm_clk_get(dev, "tx-clk");
+	if (IS_ERR(dwmac->clk_tx)) {
+		dev_warn(dev, "No tx clock provided...\n");
+		dwmac->clk_tx = NULL;
+	}
+	dwmac->clk_rx = devm_clk_get(dev, "rx-clk");
+	if (IS_ERR(dwmac->clk_rx)) {
+		dev_warn(dev, "No rx clock provided...\n");
+		dwmac->clk_rx = NULL;
+	}
+
+	/* Get mode register */
+	regmap = syscon_regmap_lookup_by_phandle(np, "st,syscon");
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	err = of_property_read_u32_index(np, "st,syscon", 1, &dwmac->mode_reg);
+	if (err) {
+		dev_err(dev, "Can't get sysconfig mode offset (%d)\n", err);
+		return err;
+	}
+
+	dwmac->dev = dev;
+	dwmac->interface = of_get_phy_mode(np);
+	dwmac->regmap = regmap;
+
+	return 0;
+}
+
+static int stm32_dwmac_probe(struct platform_device *pdev)
+{
+	struct plat_stmmacenet_data *plat_dat;
+	struct stmmac_resources stmmac_res;
+	struct stm32_dwmac *dwmac;
+	int ret;
+
+	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
+	if (ret)
+		return ret;
+
+	plat_dat = stmmac_probe_config_dt(pdev, &stmmac_res.mac);
+	if (IS_ERR(plat_dat))
+		return PTR_ERR(plat_dat);
+
+	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
+	if (!dwmac)
+		return -ENOMEM;
+
+	ret = stm32_dwmac_parse_data(dwmac, pdev);
+	if (ret) {
+		dev_err(&pdev->dev, "Unable to parse OF data\n");
+		return ret;
+	}
+
+	plat_dat->bsp_priv = dwmac;
+	plat_dat->init = stm32_dwmac_init;
+	plat_dat->exit = stm32_dwmac_exit;
+
+	ret = stm32_dwmac_init(pdev, plat_dat->bsp_priv);
+	if (ret)
+		return ret;
+
+	return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+}
+
+static const struct of_device_id stm32_dwmac_match[] = {
+	{ .compatible = "st,stm32-dwmac"},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, stm32_dwmac_match);
+
+static struct platform_driver stm32_dwmac_driver = {
+	.probe  = stm32_dwmac_probe,
+	.remove = stmmac_pltfr_remove,
+	.driver = {
+		.name           = "stm32-dwmac",
+		.pm		= &stmmac_pltfr_pm_ops,
+		.of_match_table = stm32_dwmac_match,
+	},
+};
+module_platform_driver(stm32_dwmac_driver);
+
+MODULE_AUTHOR("Alexandre Torgue <alexandre.torgue@gmail.com>");
+MODULE_DESCRIPTION("STMicroelectronics MCU DWMAC Specific Glue layer");
+MODULE_LICENSE("GPL");
+
-- 
1.9.1

[PATCH 2/4] Documentation: Bindings: Add STM32 DWMAC glue

[Alexandre TORGUE]

Signed-off-by: Alexandre TORGUE <alexandre.torgue@gmail.com>

diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.txt b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
new file mode 100644
index 0000000..18734b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
@@ -0,0 +1,41 @@
+STMicroelectronics STM32 / MCU DWMAC glue layer controller
+
+This file documents platform glue layer for stmmac.
+Please see stmmac.txt for the other unchanged properties.
+
+The device node has following properties.
+
+Required properties:
+- compatible:  Should be "st,stm32-dwmac" to select glue, and
+	       "snps,dwmac-3.50a" to select IP vesrion.
+- clocks: Should contain the GMAC main clock, and tx clock
+- compatible:  Should be "st,stm32-dwmac" to select glue and
+	       "snps,dwmac-3.50a" to select IP version.
+- clocks: Should contain the MAC main clock
+- clock-names: Should contain the clock names "stmmaceth".
+- st,syscon : Should be phandle/offset pair. The phandle to the syscon node which
+	      encompases the glue register, and the offset of the control register.
+
+Optional properties:
+- clocks: Could contain:
+	- the tx clock,
+	- the rx clock
+- clock-names: Could contain the clock names "tx-clk", "rx-clk"
+
+Example:
+
+		ethernet0: dwmac@40028000 {
+			device_type = "network";
+			compatible = "st,stm32-dwmac", "snps,dwmac-3.50a";
+			status = "disabled";
+			reg = <0x40028000 0x8000>;
+			reg-names = "stmmaceth";
+			interrupts = <0 61 0>, <0 62 0>;
+			interrupt-names = "macirq", "eth_wake_irq";
+			clock-names = "stmmaceth", "tx-clk", "rx-clk";
+			clocks = <&rcc 0 25>, <&rcc 0 26>, <&rcc 0 27>;
+			st,syscon = <&syscfg 0x4>;
+			snps,pbl = <32>;
+			snps,mixed-burst;
+			dma-ranges;
+		};
-- 
1.9.1

[PATCH 2/4] Documentation: Bindings: Add STM32 DWMAC glue

[Alexandre TORGUE]

Signed-off-by: Alexandre TORGUE <alexandre.torgue@gmail.com>

diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.txt b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
new file mode 100644
index 0000000..18734b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
@@ -0,0 +1,41 @@
+STMicroelectronics STM32 / MCU DWMAC glue layer controller
+
+This file documents platform glue layer for stmmac.
+Please see stmmac.txt for the other unchanged properties.
+
+The device node has following properties.
+
+Required properties:
+- compatible:  Should be "st,stm32-dwmac" to select glue, and
+	       "snps,dwmac-3.50a" to select IP vesrion.
+- clocks: Should contain the GMAC main clock, and tx clock
+- compatible:  Should be "st,stm32-dwmac" to select glue and
+	       "snps,dwmac-3.50a" to select IP version.
+- clocks: Should contain the MAC main clock
+- clock-names: Should contain the clock names "stmmaceth".
+- st,syscon : Should be phandle/offset pair. The phandle to the syscon node which
+	      encompases the glue register, and the offset of the control register.
+
+Optional properties:
+- clocks: Could contain:
+	- the tx clock,
+	- the rx clock
+- clock-names: Could contain the clock names "tx-clk", "rx-clk"
+
+Example:
+
+		ethernet0: dwmac at 40028000 {
+			device_type = "network";
+			compatible = "st,stm32-dwmac", "snps,dwmac-3.50a";
+			status = "disabled";
+			reg = <0x40028000 0x8000>;
+			reg-names = "stmmaceth";
+			interrupts = <0 61 0>, <0 62 0>;
+			interrupt-names = "macirq", "eth_wake_irq";
+			clock-names = "stmmaceth", "tx-clk", "rx-clk";
+			clocks = <&rcc 0 25>, <&rcc 0 26>, <&rcc 0 27>;
+			st,syscon = <&syscfg 0x4>;
+			snps,pbl = <32>;
+			snps,mixed-burst;
+			dma-ranges;
+		};
-- 
1.9.1

[PATCH 3/4] net: ethernet: stmmac: add support of Synopsys 3.50a MAC IP

[Alexandre TORGUE]

Adds support of Synopsys 3.50a MAC IP in stmmac driver.

Signed-off-by: Alexandre TORGUE <alexandre.torgue@gmail.com>

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 6a52fa1..6cca626 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -178,6 +178,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 	 * once needed on other platforms.
 	 */
 	if (of_device_is_compatible(np, "st,spear600-gmac") ||
+		of_device_is_compatible(np, "snps,dwmac-3.50a") ||
 		of_device_is_compatible(np, "snps,dwmac-3.70a") ||
 		of_device_is_compatible(np, "snps,dwmac")) {
 		/* Note that the max-frame-size parameter as defined in the
-- 
1.9.1

[PATCH 3/4] net: ethernet: stmmac: add support of Synopsys 3.50a MAC IP

[Alexandre TORGUE]

Adds support of Synopsys 3.50a MAC IP in stmmac driver.

Signed-off-by: Alexandre TORGUE <alexandre.torgue@gmail.com>

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 6a52fa1..6cca626 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -178,6 +178,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 	 * once needed on other platforms.
 	 */
 	if (of_device_is_compatible(np, "st,spear600-gmac") ||
+		of_device_is_compatible(np, "snps,dwmac-3.50a") ||
 		of_device_is_compatible(np, "snps,dwmac-3.70a") ||
 		of_device_is_compatible(np, "snps,dwmac")) {
 		/* Note that the max-frame-size parameter as defined in the
-- 
1.9.1

[PATCH 4/4] ARM: STM32: Enable Ethernet in stm32_defconfig

[Alexandre TORGUE]

Enable basic Ethernet support (IPV4) for stm32 defconfig.

Signed-off-by: Alexandre TORGUE <alexandre.torgue@gmail.com>

diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig
index ec52505..8b8abe0 100644
--- a/arch/arm/configs/stm32_defconfig
+++ b/arch/arm/configs/stm32_defconfig
@@ -33,11 +33,20 @@ CONFIG_XIP_PHYS_ADDR=0x08008000
 CONFIG_BINFMT_FLAT=y
 CONFIG_BINFMT_SHARED_FLAT=y
 # CONFIG_COREDUMP is not set
+CONFIG_NET=y
+CONFIG_INET=y
+# CONFIG_INET_XFRM_MODE_TRANSPORT is not set
+# CONFIG_INET_XFRM_MODE_TUNNEL is not set
+# CONFIG_INET_XFRM_MODE_BEET is not set
+# CONFIG_IPV6 is not set
 CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
 # CONFIG_FW_LOADER is not set
 # CONFIG_BLK_DEV is not set
 CONFIG_EEPROM_93CX6=y
+CONFIG_NETDEVICES=y
+CONFIG_STMMAC_ETH=y
+# CONFIG_WLAN is not set
 # CONFIG_INPUT is not set
 # CONFIG_SERIO is not set
 # CONFIG_VT is not set
-- 
1.9.1

[PATCH 4/4] ARM: STM32: Enable Ethernet in stm32_defconfig

[Alexandre TORGUE]

Enable basic Ethernet support (IPV4) for stm32 defconfig.

Signed-off-by: Alexandre TORGUE <alexandre.torgue@gmail.com>

diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig
index ec52505..8b8abe0 100644
--- a/arch/arm/configs/stm32_defconfig
+++ b/arch/arm/configs/stm32_defconfig
@@ -33,11 +33,20 @@ CONFIG_XIP_PHYS_ADDR=0x08008000
 CONFIG_BINFMT_FLAT=y
 CONFIG_BINFMT_SHARED_FLAT=y
 # CONFIG_COREDUMP is not set
+CONFIG_NET=y
+CONFIG_INET=y
+# CONFIG_INET_XFRM_MODE_TRANSPORT is not set
+# CONFIG_INET_XFRM_MODE_TUNNEL is not set
+# CONFIG_INET_XFRM_MODE_BEET is not set
+# CONFIG_IPV6 is not set
 CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
 # CONFIG_FW_LOADER is not set
 # CONFIG_BLK_DEV is not set
 CONFIG_EEPROM_93CX6=y
+CONFIG_NETDEVICES=y
+CONFIG_STMMAC_ETH=y
+# CONFIG_WLAN is not set
 # CONFIG_INPUT is not set
 # CONFIG_SERIO is not set
 # CONFIG_VT is not set
-- 
1.9.1

Re: [PATCH 0/4] Add Ethernet support on STM32F429

[Giuseppe CAVALLARO]

Hello Alex

On 2/3/2016 3:54 PM, Alexandre TORGUE wrote:
> STM32F429 Chip embeds a Synopsys 3.50a MAC IP.
> This series:
> 	-enhance current stmmac driver to control it (code already available) and
> 	 adds basic glue for STM32F429 chip.
> 	-Enable basic Net config in kernel.
>
> Note that DT patches are not present because STM32 pinctrl code is not
> yet avalaible.

patches look ok for me

Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>

>
> Regards
>
> Alexandre
>
> Alexandre TORGUE (4):
>    net: ethernet: dwmac: add Ethernet glue logic for stm32 chip
>    Documentation: Bindings: Add STM32 DWMAC glue
>    net: ethernet: stmmac: add support of Synopsys 3.50a MAC IP
>    ARM: STM32: Enable Ethernet in stm32_defconfig
>
>   .../devicetree/bindings/net/stm32-dwmac.txt        |  41 +++++
>   arch/arm/configs/stm32_defconfig                   |   9 ++
>   drivers/net/ethernet/stmicro/stmmac/Kconfig        |  12 ++
>   drivers/net/ethernet/stmicro/stmmac/Makefile       |   1 +
>   drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c  | 177 +++++++++++++++++++++
>   .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |   1 +
>   6 files changed, 241 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/net/stm32-dwmac.txt
>   create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>

[PATCH 0/4] Add Ethernet support on STM32F429

[Giuseppe CAVALLARO]

Hello Alex

On 2/3/2016 3:54 PM, Alexandre TORGUE wrote:
> STM32F429 Chip embeds a Synopsys 3.50a MAC IP.
> This series:
> 	-enhance current stmmac driver to control it (code already available) and
> 	 adds basic glue for STM32F429 chip.
> 	-Enable basic Net config in kernel.
>
> Note that DT patches are not present because STM32 pinctrl code is not
> yet avalaible.

patches look ok for me

Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>

>
> Regards
>
> Alexandre
>
> Alexandre TORGUE (4):
>    net: ethernet: dwmac: add Ethernet glue logic for stm32 chip
>    Documentation: Bindings: Add STM32 DWMAC glue
>    net: ethernet: stmmac: add support of Synopsys 3.50a MAC IP
>    ARM: STM32: Enable Ethernet in stm32_defconfig
>
>   .../devicetree/bindings/net/stm32-dwmac.txt        |  41 +++++
>   arch/arm/configs/stm32_defconfig                   |   9 ++
>   drivers/net/ethernet/stmicro/stmmac/Kconfig        |  12 ++
>   drivers/net/ethernet/stmicro/stmmac/Makefile       |   1 +
>   drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c  | 177 +++++++++++++++++++++
>   .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |   1 +
>   6 files changed, 241 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/net/stm32-dwmac.txt
>   create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>

Re: [PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]

Hi Alexandre,

[auto build test WARNING on net/master]
[also build test WARNING on v4.5-rc2 next-20160203]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Alexandre-TORGUE/Add-Ethernet-support-on-STM32F429/20160203-225940
config: um-allyesconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=um 

All warnings (new ones prefixed by >>):

warning: (ST_IRQCHIP && HIP04_ETH && STMMAC_PLATFORM && DWMAC_IPQ806X && DWMAC_LPC18XX && DWMAC_ROCKCHIP && DWMAC_SOCFPGA && DWMAC_STI && DWMAC_STM32 && TI_CPSW && PINCTRL_ROCKCHIP && PINCTRL_DOVE && POWER_RESET_KEYSTONE && POWER_RESET_SYSCON && POWER_RESET_SYSCON_POWEROFF && S3C2410_WATCHDOG && VIDEO_OMAP3 && VIDEO_S5P_FIMC && USB_XHCI_MTK && RTC_DRV_AT91SAM9 && LPC18XX_DMAMUX && VIDEO_OMAP4 && HWSPINLOCK_QCOM && ATMEL_ST && QCOM_GSBI && PHY_HI6220_USB) selects MFD_SYSCON which has unmet direct dependencies (HAS_IOMEM)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 17523 bytes --]

[PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[kbuild test robot]

Hi Alexandre,

[auto build test WARNING on net/master]
[also build test WARNING on v4.5-rc2 next-20160203]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Alexandre-TORGUE/Add-Ethernet-support-on-STM32F429/20160203-225940
config: um-allyesconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=um 

All warnings (new ones prefixed by >>):

warning: (ST_IRQCHIP && HIP04_ETH && STMMAC_PLATFORM && DWMAC_IPQ806X && DWMAC_LPC18XX && DWMAC_ROCKCHIP && DWMAC_SOCFPGA && DWMAC_STI && DWMAC_STM32 && TI_CPSW && PINCTRL_ROCKCHIP && PINCTRL_DOVE && POWER_RESET_KEYSTONE && POWER_RESET_SYSCON && POWER_RESET_SYSCON_POWEROFF && S3C2410_WATCHDOG && VIDEO_OMAP3 && VIDEO_S5P_FIMC && USB_XHCI_MTK && RTC_DRV_AT91SAM9 && LPC18XX_DMAMUX && VIDEO_OMAP4 && HWSPINLOCK_QCOM && ATMEL_ST && QCOM_GSBI && PHY_HI6220_USB) selects MFD_SYSCON which has unmet direct dependencies (HAS_IOMEM)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 17523 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160204/cb3c4c9a/attachment-0001.obj>

Re: [PATCH 0/4] Add Ethernet support on STM32F429

[David Miller]

From: Alexandre TORGUE <alexandre.torgue@gmail.com>
Date: Wed,  3 Feb 2016 15:54:31 +0100

> STM32F429 Chip embeds a Synopsys 3.50a MAC IP.
> This series:
> 	-enhance current stmmac driver to control it (code already available) and 
> 	 adds basic glue for STM32F429 chip.
> 	-Enable basic Net config in kernel.
> 
> Note that DT patches are not present because STM32 pinctrl code is not
> yet avalaible.

Looks like this needs to be respun to deal with the warnings the kbuild
robot reported.

Thanks.

[PATCH 0/4] Add Ethernet support on STM32F429

[David Miller]

From: Alexandre TORGUE <alexandre.torgue@gmail.com>
Date: Wed,  3 Feb 2016 15:54:31 +0100

> STM32F429 Chip embeds a Synopsys 3.50a MAC IP.
> This series:
> 	-enhance current stmmac driver to control it (code already available) and 
> 	 adds basic glue for STM32F429 chip.
> 	-Enable basic Net config in kernel.
> 
> Note that DT patches are not present because STM32 pinctrl code is not
> yet avalaible.

Looks like this needs to be respun to deal with the warnings the kbuild
robot reported.

Thanks.

Re: [PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]

Hi Alexandre,

[auto build test WARNING on net/master]
[also build test WARNING on v4.5-rc3 next-20160212]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Alexandre-TORGUE/Add-Ethernet-support-on-STM32F429/20160203-225940
config: um-allyesconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=um 

All warnings (new ones prefixed by >>):

warning: (ST_IRQCHIP && HIP04_ETH && STMMAC_PLATFORM && DWMAC_IPQ806X && DWMAC_LPC18XX && DWMAC_ROCKCHIP && DWMAC_SOCFPGA && DWMAC_STI && DWMAC_STM32 && TI_CPSW && PINCTRL_ROCKCHIP && PINCTRL_DOVE && POWER_RESET_KEYSTONE && POWER_RESET_SYSCON && POWER_RESET_SYSCON_POWEROFF && S3C2410_WATCHDOG && VIDEO_OMAP3 && VIDEO_S5P_FIMC && USB_XHCI_MTK && RTC_DRV_AT91SAM9 && LPC18XX_DMAMUX && VIDEO_OMAP4 && HWSPINLOCK_QCOM && ATMEL_ST && QCOM_GSBI && PHY_HI6220_USB) selects MFD_SYSCON which has unmet direct dependencies (HAS_IOMEM)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 17523 bytes --]

[PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[kbuild test robot]

Hi Alexandre,

[auto build test WARNING on net/master]
[also build test WARNING on v4.5-rc3 next-20160212]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Alexandre-TORGUE/Add-Ethernet-support-on-STM32F429/20160203-225940
config: um-allyesconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=um 

All warnings (new ones prefixed by >>):

warning: (ST_IRQCHIP && HIP04_ETH && STMMAC_PLATFORM && DWMAC_IPQ806X && DWMAC_LPC18XX && DWMAC_ROCKCHIP && DWMAC_SOCFPGA && DWMAC_STI && DWMAC_STM32 && TI_CPSW && PINCTRL_ROCKCHIP && PINCTRL_DOVE && POWER_RESET_KEYSTONE && POWER_RESET_SYSCON && POWER_RESET_SYSCON_POWEROFF && S3C2410_WATCHDOG && VIDEO_OMAP3 && VIDEO_S5P_FIMC && USB_XHCI_MTK && RTC_DRV_AT91SAM9 && LPC18XX_DMAMUX && VIDEO_OMAP4 && HWSPINLOCK_QCOM && ATMEL_ST && QCOM_GSBI && PHY_HI6220_USB) selects MFD_SYSCON which has unmet direct dependencies (HAS_IOMEM)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 17523 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160212/d3544f93/attachment-0001.obj>

Re: [PATCH 0/4] Add Ethernet support on STM32F429

[Alexandre Torgue]

Hi David,

I will fix it in next patch version.
I just find a corruption issue in stmmac driver, I will also fix it in
next version.

Best regards

Alex

2016-02-09 10:52 GMT+01:00 David Miller <davem@davemloft.net>:
> From: Alexandre TORGUE <alexandre.torgue@gmail.com>
> Date: Wed,  3 Feb 2016 15:54:31 +0100
>
>> STM32F429 Chip embeds a Synopsys 3.50a MAC IP.
>> This series:
>>       -enhance current stmmac driver to control it (code already available) and
>>        adds basic glue for STM32F429 chip.
>>       -Enable basic Net config in kernel.
>>
>> Note that DT patches are not present because STM32 pinctrl code is not
>> yet avalaible.
>
> Looks like this needs to be respun to deal with the warnings the kbuild
> robot reported.
>
> Thanks.

[PATCH 0/4] Add Ethernet support on STM32F429

[Alexandre Torgue]

Hi David,

I will fix it in next patch version.
I just find a corruption issue in stmmac driver, I will also fix it in
next version.

Best regards

Alex

2016-02-09 10:52 GMT+01:00 David Miller <davem@davemloft.net>:
> From: Alexandre TORGUE <alexandre.torgue@gmail.com>
> Date: Wed,  3 Feb 2016 15:54:31 +0100
>
>> STM32F429 Chip embeds a Synopsys 3.50a MAC IP.
>> This series:
>>       -enhance current stmmac driver to control it (code already available) and
>>        adds basic glue for STM32F429 chip.
>>       -Enable basic Net config in kernel.
>>
>> Note that DT patches are not present because STM32 pinctrl code is not
>> yet avalaible.
>
> Looks like this needs to be respun to deal with the warnings the kbuild
> robot reported.
>
> Thanks.

Re: [PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Joachim Eastwood]

On 3 February 2016 at 15:54, Alexandre TORGUE
<alexandre.torgue@gmail.com> wrote:
> stm324xx family chips support Synopsys MAC 3.510 IP.
> This patch adds settings for logical glue logic:
> -clocks
> -mode selection MII or RMII.
>
> Signed-off-by: Alexandre TORGUE <alexandre.torgue@gmail.com>
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index cec147d..a94dd15 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -114,6 +114,18 @@ config DWMAC_SUNXI
>           This selects Allwinner SoC glue layer support for the
>           stmmac device driver. This driver is used for A20/A31
>           GMAC ethernet controller.
> +
> +config DWMAC_STM32
> +       tristate "STM32 DWMAC support"
> +       default ARCH_STM32
> +       depends on OF
> +       select MFD_SYSCON
> +       ---help---
> +         Support for ethernet controller on STM32 SOCs.
> +
> +         This selects STM32 SoC glue layer support for the stmmac
> +         device driver. This driver is used on for the STM32 series
> +         SOCs GMAC ethernet controller.
>  endif
>
>  config STMMAC_PCI
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> index b390161..9fb2061 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_DWMAC_SOCFPGA)   += dwmac-socfpga.o
>  obj-$(CONFIG_DWMAC_STI)                += dwmac-sti.o
>  obj-$(CONFIG_DWMAC_SUNXI)      += dwmac-sunxi.o
>  obj-$(CONFIG_DWMAC_GENERIC)    += dwmac-generic.o
> +obj-$(CONFIG_DWMAC_STM32)      += dwmac-stm32.o

Keep these sorted. There also a comment in the Makefile that states
that the generic should always be last.


>  stmmac-platform-objs:= stmmac_platform.o
>
>  obj-$(CONFIG_STMMAC_PCI) += stmmac-pci.o
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> new file mode 100644
> index 0000000..56ccc20
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> @@ -0,0 +1,177 @@
> +/*
> + * dwmac-stm32.c - DWMAC Specific Glue layer for STM32 MCU
> + *
> + * Copyright (C) Alexandre Torgue 2015
> + * Author:  Alexandre Torgue <alexandre.torgue@gmail.com>
> + * License terms:  GNU General Public License (GPL), version 2
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/stmmac.h>
> +#include <linux/phy.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/clk.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_net.h>

Consider sorting the includes.

> +
> +#include "stmmac_platform.h"
> +
> +#define MII_PHY_SEL_MASK       BIT(23)
> +
> +struct stm32_dwmac {
> +       int interface;          /* MII interface */
> +       struct clk *clk_tx;
> +       struct clk *clk_rx;
> +       u32 mode_reg;           /* MAC glue-logic mode register */
> +       struct device *dev;

dev doesn't seem to be used anywhere.

> +       struct regmap *regmap;
> +       u32 speed;
> +};
> +
> +static int stm32_dwmac_init(struct platform_device *pdev, void *priv)
> +{
> +       struct stm32_dwmac *dwmac = priv;
> +       struct regmap *regmap = dwmac->regmap;
> +       int ret, iface = dwmac->interface;
> +       u32 reg = dwmac->mode_reg;
> +       u32 val;
> +
> +       if (dwmac->clk_tx)
> +               ret = clk_prepare_enable(dwmac->clk_tx);

The clk API handles a NULL clk so you don't really need to check for it.


> +               if (ret)
> +                       goto out;
> +
> +       if (dwmac->clk_rx)
> +               ret = clk_prepare_enable(dwmac->clk_rx);

Same here.


> +               if (ret)
> +                       goto out_disable_clk_tx;
> +
> +       val = (iface == PHY_INTERFACE_MODE_MII) ? 0 : 1;
> +       ret = regmap_update_bits(regmap, reg, MII_PHY_SEL_MASK, val);
> +       if (ret)
> +               goto out_disable_clk_tx_rx;
> +
> +       return 0;
> +
> +out_disable_clk_tx_rx:
> +       clk_disable_unprepare(dwmac->clk_rx);
> +out_disable_clk_tx:
> +       clk_disable_unprepare(dwmac->clk_tx);
> +out:
> +       return ret;
> +}
> +
> +static void stm32_dwmac_exit(struct platform_device *pdev, void *priv)
> +{
> +       struct stm32_dwmac *dwmac = priv;
> +
> +       if (dwmac->clk_tx)
> +               clk_disable_unprepare(dwmac->clk_tx);
> +       if (dwmac->clk_rx)
> +               clk_disable_unprepare(dwmac->clk_rx);

Same here.


> +}
> +
> +static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
> +                                 struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct device_node *np = dev->of_node;
> +       struct regmap *regmap;
> +       int err;
> +
> +       if (!np)
> +               return -EINVAL;

Can this ever happen?
This is a DT only driver, right?

> +
> +       /*  Get TX/RX clocks */
> +       dwmac->clk_tx = devm_clk_get(dev, "tx-clk");
> +       if (IS_ERR(dwmac->clk_tx)) {
> +               dev_warn(dev, "No tx clock provided...\n");
> +               dwmac->clk_tx = NULL;
> +       }
> +       dwmac->clk_rx = devm_clk_get(dev, "rx-clk");
> +       if (IS_ERR(dwmac->clk_rx)) {
> +               dev_warn(dev, "No rx clock provided...\n");
> +               dwmac->clk_rx = NULL;
> +       }
> +
> +       /* Get mode register */
> +       regmap = syscon_regmap_lookup_by_phandle(np, "st,syscon");
> +       if (IS_ERR(regmap))
> +               return PTR_ERR(regmap);
> +
> +       err = of_property_read_u32_index(np, "st,syscon", 1, &dwmac->mode_reg);
> +       if (err) {
> +               dev_err(dev, "Can't get sysconfig mode offset (%d)\n", err);
> +               return err;
> +       }
> +
> +       dwmac->dev = dev;
> +       dwmac->interface = of_get_phy_mode(np);
> +       dwmac->regmap = regmap;
> +
> +       return 0;
> +}
> +
> +static int stm32_dwmac_probe(struct platform_device *pdev)
> +{
> +       struct plat_stmmacenet_data *plat_dat;
> +       struct stmmac_resources stmmac_res;
> +       struct stm32_dwmac *dwmac;
> +       int ret;
> +
> +       ret = stmmac_get_platform_resources(pdev, &stmmac_res);
> +       if (ret)
> +               return ret;
> +
> +       plat_dat = stmmac_probe_config_dt(pdev, &stmmac_res.mac);
> +       if (IS_ERR(plat_dat))
> +               return PTR_ERR(plat_dat);
> +
> +       dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
> +       if (!dwmac)
> +               return -ENOMEM;
> +
> +       ret = stm32_dwmac_parse_data(dwmac, pdev);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Unable to parse OF data\n");
> +               return ret;
> +       }
> +
> +       plat_dat->bsp_priv = dwmac;
> +       plat_dat->init = stm32_dwmac_init;
> +       plat_dat->exit = stm32_dwmac_exit;

Instead of using these callbacks could you rather implement the PM
callbacks directly in this driver?
I don't think it should add much code and it will make it look more
like standard driver. This will also give you some more control and
flexibility in your code.

> +
> +       ret = stm32_dwmac_init(pdev, plat_dat->bsp_priv);
> +       if (ret)
> +               return ret;
> +
> +       return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> +}
> +
> +static const struct of_device_id stm32_dwmac_match[] = {
> +       { .compatible = "st,stm32-dwmac"},
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, stm32_dwmac_match);
> +
> +static struct platform_driver stm32_dwmac_driver = {
> +       .probe  = stm32_dwmac_probe,
> +       .remove = stmmac_pltfr_remove,

Could you implement the .remove callback in your driver instead of
using stmmac_pltfr_remove?
Same reasons as above.


> +       .driver = {
> +               .name           = "stm32-dwmac",
> +               .pm             = &stmmac_pltfr_pm_ops,
> +               .of_match_table = stm32_dwmac_match,
> +       },
> +};
> +module_platform_driver(stm32_dwmac_driver);
> +
> +MODULE_AUTHOR("Alexandre Torgue <alexandre.torgue@gmail.com>");
> +MODULE_DESCRIPTION("STMicroelectronics MCU DWMAC Specific Glue layer");
> +MODULE_LICENSE("GPL");

Since you state:
> + * License terms:  GNU General Public License (GPL), version 2

You might want to switch use: MODULE_LICENSE("GPL v2");


regards,
Joachim Eastwood

Re: [PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Joachim Eastwood]

On 3 February 2016 at 15:54, Alexandre TORGUE
<alexandre.torgue@gmail.com> wrote:
> stm324xx family chips support Synopsys MAC 3.510 IP.
> This patch adds settings for logical glue logic:
> -clocks
> -mode selection MII or RMII.
>
> Signed-off-by: Alexandre TORGUE <alexandre.torgue@gmail.com>
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index cec147d..a94dd15 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -114,6 +114,18 @@ config DWMAC_SUNXI
>           This selects Allwinner SoC glue layer support for the
>           stmmac device driver. This driver is used for A20/A31
>           GMAC ethernet controller.
> +
> +config DWMAC_STM32
> +       tristate "STM32 DWMAC support"
> +       default ARCH_STM32
> +       depends on OF
> +       select MFD_SYSCON
> +       ---help---
> +         Support for ethernet controller on STM32 SOCs.
> +
> +         This selects STM32 SoC glue layer support for the stmmac
> +         device driver. This driver is used on for the STM32 series
> +         SOCs GMAC ethernet controller.
>  endif
>
>  config STMMAC_PCI
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> index b390161..9fb2061 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_DWMAC_SOCFPGA)   += dwmac-socfpga.o
>  obj-$(CONFIG_DWMAC_STI)                += dwmac-sti.o
>  obj-$(CONFIG_DWMAC_SUNXI)      += dwmac-sunxi.o
>  obj-$(CONFIG_DWMAC_GENERIC)    += dwmac-generic.o
> +obj-$(CONFIG_DWMAC_STM32)      += dwmac-stm32.o

Keep these sorted. There also a comment in the Makefile that states
that the generic should always be last.


>  stmmac-platform-objs:= stmmac_platform.o
>
>  obj-$(CONFIG_STMMAC_PCI) += stmmac-pci.o
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> new file mode 100644
> index 0000000..56ccc20
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> @@ -0,0 +1,177 @@
> +/*
> + * dwmac-stm32.c - DWMAC Specific Glue layer for STM32 MCU
> + *
> + * Copyright (C) Alexandre Torgue 2015
> + * Author:  Alexandre Torgue <alexandre.torgue@gmail.com>
> + * License terms:  GNU General Public License (GPL), version 2
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/stmmac.h>
> +#include <linux/phy.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/clk.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_net.h>

Consider sorting the includes.

> +
> +#include "stmmac_platform.h"
> +
> +#define MII_PHY_SEL_MASK       BIT(23)
> +
> +struct stm32_dwmac {
> +       int interface;          /* MII interface */
> +       struct clk *clk_tx;
> +       struct clk *clk_rx;
> +       u32 mode_reg;           /* MAC glue-logic mode register */
> +       struct device *dev;

dev doesn't seem to be used anywhere.

> +       struct regmap *regmap;
> +       u32 speed;
> +};
> +
> +static int stm32_dwmac_init(struct platform_device *pdev, void *priv)
> +{
> +       struct stm32_dwmac *dwmac = priv;
> +       struct regmap *regmap = dwmac->regmap;
> +       int ret, iface = dwmac->interface;
> +       u32 reg = dwmac->mode_reg;
> +       u32 val;
> +
> +       if (dwmac->clk_tx)
> +               ret = clk_prepare_enable(dwmac->clk_tx);

The clk API handles a NULL clk so you don't really need to check for it.


> +               if (ret)
> +                       goto out;
> +
> +       if (dwmac->clk_rx)
> +               ret = clk_prepare_enable(dwmac->clk_rx);

Same here.


> +               if (ret)
> +                       goto out_disable_clk_tx;
> +
> +       val = (iface == PHY_INTERFACE_MODE_MII) ? 0 : 1;
> +       ret = regmap_update_bits(regmap, reg, MII_PHY_SEL_MASK, val);
> +       if (ret)
> +               goto out_disable_clk_tx_rx;
> +
> +       return 0;
> +
> +out_disable_clk_tx_rx:
> +       clk_disable_unprepare(dwmac->clk_rx);
> +out_disable_clk_tx:
> +       clk_disable_unprepare(dwmac->clk_tx);
> +out:
> +       return ret;
> +}
> +
> +static void stm32_dwmac_exit(struct platform_device *pdev, void *priv)
> +{
> +       struct stm32_dwmac *dwmac = priv;
> +
> +       if (dwmac->clk_tx)
> +               clk_disable_unprepare(dwmac->clk_tx);
> +       if (dwmac->clk_rx)
> +               clk_disable_unprepare(dwmac->clk_rx);

Same here.


> +}
> +
> +static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
> +                                 struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct device_node *np = dev->of_node;
> +       struct regmap *regmap;
> +       int err;
> +
> +       if (!np)
> +               return -EINVAL;

Can this ever happen?
This is a DT only driver, right?

> +
> +       /*  Get TX/RX clocks */
> +       dwmac->clk_tx = devm_clk_get(dev, "tx-clk");
> +       if (IS_ERR(dwmac->clk_tx)) {
> +               dev_warn(dev, "No tx clock provided...\n");
> +               dwmac->clk_tx = NULL;
> +       }
> +       dwmac->clk_rx = devm_clk_get(dev, "rx-clk");
> +       if (IS_ERR(dwmac->clk_rx)) {
> +               dev_warn(dev, "No rx clock provided...\n");
> +               dwmac->clk_rx = NULL;
> +       }
> +
> +       /* Get mode register */
> +       regmap = syscon_regmap_lookup_by_phandle(np, "st,syscon");
> +       if (IS_ERR(regmap))
> +               return PTR_ERR(regmap);
> +
> +       err = of_property_read_u32_index(np, "st,syscon", 1, &dwmac->mode_reg);
> +       if (err) {
> +               dev_err(dev, "Can't get sysconfig mode offset (%d)\n", err);
> +               return err;
> +       }
> +
> +       dwmac->dev = dev;
> +       dwmac->interface = of_get_phy_mode(np);
> +       dwmac->regmap = regmap;
> +
> +       return 0;
> +}
> +
> +static int stm32_dwmac_probe(struct platform_device *pdev)
> +{
> +       struct plat_stmmacenet_data *plat_dat;
> +       struct stmmac_resources stmmac_res;
> +       struct stm32_dwmac *dwmac;
> +       int ret;
> +
> +       ret = stmmac_get_platform_resources(pdev, &stmmac_res);
> +       if (ret)
> +               return ret;
> +
> +       plat_dat = stmmac_probe_config_dt(pdev, &stmmac_res.mac);
> +       if (IS_ERR(plat_dat))
> +               return PTR_ERR(plat_dat);
> +
> +       dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
> +       if (!dwmac)
> +               return -ENOMEM;
> +
> +       ret = stm32_dwmac_parse_data(dwmac, pdev);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Unable to parse OF data\n");
> +               return ret;
> +       }
> +
> +       plat_dat->bsp_priv = dwmac;
> +       plat_dat->init = stm32_dwmac_init;
> +       plat_dat->exit = stm32_dwmac_exit;

Instead of using these callbacks could you rather implement the PM
callbacks directly in this driver?
I don't think it should add much code and it will make it look more
like standard driver. This will also give you some more control and
flexibility in your code.

> +
> +       ret = stm32_dwmac_init(pdev, plat_dat->bsp_priv);
> +       if (ret)
> +               return ret;
> +
> +       return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> +}
> +
> +static const struct of_device_id stm32_dwmac_match[] = {
> +       { .compatible = "st,stm32-dwmac"},
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, stm32_dwmac_match);
> +
> +static struct platform_driver stm32_dwmac_driver = {
> +       .probe  = stm32_dwmac_probe,
> +       .remove = stmmac_pltfr_remove,

Could you implement the .remove callback in your driver instead of
using stmmac_pltfr_remove?
Same reasons as above.


> +       .driver = {
> +               .name           = "stm32-dwmac",
> +               .pm             = &stmmac_pltfr_pm_ops,
> +               .of_match_table = stm32_dwmac_match,
> +       },
> +};
> +module_platform_driver(stm32_dwmac_driver);
> +
> +MODULE_AUTHOR("Alexandre Torgue <alexandre.torgue@gmail.com>");
> +MODULE_DESCRIPTION("STMicroelectronics MCU DWMAC Specific Glue layer");
> +MODULE_LICENSE("GPL");

Since you state:
> + * License terms:  GNU General Public License (GPL), version 2

You might want to switch use: MODULE_LICENSE("GPL v2");


regards,
Joachim Eastwood

[PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Joachim Eastwood]

On 3 February 2016 at 15:54, Alexandre TORGUE
<alexandre.torgue@gmail.com> wrote:
> stm324xx family chips support Synopsys MAC 3.510 IP.
> This patch adds settings for logical glue logic:
> -clocks
> -mode selection MII or RMII.
>
> Signed-off-by: Alexandre TORGUE <alexandre.torgue@gmail.com>
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index cec147d..a94dd15 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -114,6 +114,18 @@ config DWMAC_SUNXI
>           This selects Allwinner SoC glue layer support for the
>           stmmac device driver. This driver is used for A20/A31
>           GMAC ethernet controller.
> +
> +config DWMAC_STM32
> +       tristate "STM32 DWMAC support"
> +       default ARCH_STM32
> +       depends on OF
> +       select MFD_SYSCON
> +       ---help---
> +         Support for ethernet controller on STM32 SOCs.
> +
> +         This selects STM32 SoC glue layer support for the stmmac
> +         device driver. This driver is used on for the STM32 series
> +         SOCs GMAC ethernet controller.
>  endif
>
>  config STMMAC_PCI
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> index b390161..9fb2061 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_DWMAC_SOCFPGA)   += dwmac-socfpga.o
>  obj-$(CONFIG_DWMAC_STI)                += dwmac-sti.o
>  obj-$(CONFIG_DWMAC_SUNXI)      += dwmac-sunxi.o
>  obj-$(CONFIG_DWMAC_GENERIC)    += dwmac-generic.o
> +obj-$(CONFIG_DWMAC_STM32)      += dwmac-stm32.o

Keep these sorted. There also a comment in the Makefile that states
that the generic should always be last.


>  stmmac-platform-objs:= stmmac_platform.o
>
>  obj-$(CONFIG_STMMAC_PCI) += stmmac-pci.o
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> new file mode 100644
> index 0000000..56ccc20
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> @@ -0,0 +1,177 @@
> +/*
> + * dwmac-stm32.c - DWMAC Specific Glue layer for STM32 MCU
> + *
> + * Copyright (C) Alexandre Torgue 2015
> + * Author:  Alexandre Torgue <alexandre.torgue@gmail.com>
> + * License terms:  GNU General Public License (GPL), version 2
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/stmmac.h>
> +#include <linux/phy.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/clk.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_net.h>

Consider sorting the includes.

> +
> +#include "stmmac_platform.h"
> +
> +#define MII_PHY_SEL_MASK       BIT(23)
> +
> +struct stm32_dwmac {
> +       int interface;          /* MII interface */
> +       struct clk *clk_tx;
> +       struct clk *clk_rx;
> +       u32 mode_reg;           /* MAC glue-logic mode register */
> +       struct device *dev;

dev doesn't seem to be used anywhere.

> +       struct regmap *regmap;
> +       u32 speed;
> +};
> +
> +static int stm32_dwmac_init(struct platform_device *pdev, void *priv)
> +{
> +       struct stm32_dwmac *dwmac = priv;
> +       struct regmap *regmap = dwmac->regmap;
> +       int ret, iface = dwmac->interface;
> +       u32 reg = dwmac->mode_reg;
> +       u32 val;
> +
> +       if (dwmac->clk_tx)
> +               ret = clk_prepare_enable(dwmac->clk_tx);

The clk API handles a NULL clk so you don't really need to check for it.


> +               if (ret)
> +                       goto out;
> +
> +       if (dwmac->clk_rx)
> +               ret = clk_prepare_enable(dwmac->clk_rx);

Same here.


> +               if (ret)
> +                       goto out_disable_clk_tx;
> +
> +       val = (iface == PHY_INTERFACE_MODE_MII) ? 0 : 1;
> +       ret = regmap_update_bits(regmap, reg, MII_PHY_SEL_MASK, val);
> +       if (ret)
> +               goto out_disable_clk_tx_rx;
> +
> +       return 0;
> +
> +out_disable_clk_tx_rx:
> +       clk_disable_unprepare(dwmac->clk_rx);
> +out_disable_clk_tx:
> +       clk_disable_unprepare(dwmac->clk_tx);
> +out:
> +       return ret;
> +}
> +
> +static void stm32_dwmac_exit(struct platform_device *pdev, void *priv)
> +{
> +       struct stm32_dwmac *dwmac = priv;
> +
> +       if (dwmac->clk_tx)
> +               clk_disable_unprepare(dwmac->clk_tx);
> +       if (dwmac->clk_rx)
> +               clk_disable_unprepare(dwmac->clk_rx);

Same here.


> +}
> +
> +static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
> +                                 struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct device_node *np = dev->of_node;
> +       struct regmap *regmap;
> +       int err;
> +
> +       if (!np)
> +               return -EINVAL;

Can this ever happen?
This is a DT only driver, right?

> +
> +       /*  Get TX/RX clocks */
> +       dwmac->clk_tx = devm_clk_get(dev, "tx-clk");
> +       if (IS_ERR(dwmac->clk_tx)) {
> +               dev_warn(dev, "No tx clock provided...\n");
> +               dwmac->clk_tx = NULL;
> +       }
> +       dwmac->clk_rx = devm_clk_get(dev, "rx-clk");
> +       if (IS_ERR(dwmac->clk_rx)) {
> +               dev_warn(dev, "No rx clock provided...\n");
> +               dwmac->clk_rx = NULL;
> +       }
> +
> +       /* Get mode register */
> +       regmap = syscon_regmap_lookup_by_phandle(np, "st,syscon");
> +       if (IS_ERR(regmap))
> +               return PTR_ERR(regmap);
> +
> +       err = of_property_read_u32_index(np, "st,syscon", 1, &dwmac->mode_reg);
> +       if (err) {
> +               dev_err(dev, "Can't get sysconfig mode offset (%d)\n", err);
> +               return err;
> +       }
> +
> +       dwmac->dev = dev;
> +       dwmac->interface = of_get_phy_mode(np);
> +       dwmac->regmap = regmap;
> +
> +       return 0;
> +}
> +
> +static int stm32_dwmac_probe(struct platform_device *pdev)
> +{
> +       struct plat_stmmacenet_data *plat_dat;
> +       struct stmmac_resources stmmac_res;
> +       struct stm32_dwmac *dwmac;
> +       int ret;
> +
> +       ret = stmmac_get_platform_resources(pdev, &stmmac_res);
> +       if (ret)
> +               return ret;
> +
> +       plat_dat = stmmac_probe_config_dt(pdev, &stmmac_res.mac);
> +       if (IS_ERR(plat_dat))
> +               return PTR_ERR(plat_dat);
> +
> +       dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
> +       if (!dwmac)
> +               return -ENOMEM;
> +
> +       ret = stm32_dwmac_parse_data(dwmac, pdev);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Unable to parse OF data\n");
> +               return ret;
> +       }
> +
> +       plat_dat->bsp_priv = dwmac;
> +       plat_dat->init = stm32_dwmac_init;
> +       plat_dat->exit = stm32_dwmac_exit;

Instead of using these callbacks could you rather implement the PM
callbacks directly in this driver?
I don't think it should add much code and it will make it look more
like standard driver. This will also give you some more control and
flexibility in your code.

> +
> +       ret = stm32_dwmac_init(pdev, plat_dat->bsp_priv);
> +       if (ret)
> +               return ret;
> +
> +       return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> +}
> +
> +static const struct of_device_id stm32_dwmac_match[] = {
> +       { .compatible = "st,stm32-dwmac"},
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, stm32_dwmac_match);
> +
> +static struct platform_driver stm32_dwmac_driver = {
> +       .probe  = stm32_dwmac_probe,
> +       .remove = stmmac_pltfr_remove,

Could you implement the .remove callback in your driver instead of
using stmmac_pltfr_remove?
Same reasons as above.


> +       .driver = {
> +               .name           = "stm32-dwmac",
> +               .pm             = &stmmac_pltfr_pm_ops,
> +               .of_match_table = stm32_dwmac_match,
> +       },
> +};
> +module_platform_driver(stm32_dwmac_driver);
> +
> +MODULE_AUTHOR("Alexandre Torgue <alexandre.torgue@gmail.com>");
> +MODULE_DESCRIPTION("STMicroelectronics MCU DWMAC Specific Glue layer");
> +MODULE_LICENSE("GPL");

Since you state:
> + * License terms:  GNU General Public License (GPL), version 2

You might want to switch use: MODULE_LICENSE("GPL v2");


regards,
Joachim Eastwood

Re: [PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Alexandre Torgue]

2016-02-13 14:48 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
> On 3 February 2016 at 15:54, Alexandre TORGUE
> <alexandre.torgue@gmail.com> wrote:
>> stm324xx family chips support Synopsys MAC 3.510 IP.
>> This patch adds settings for logical glue logic:
>> -clocks
>> -mode selection MII or RMII.
>>
>> Signed-off-by: Alexandre TORGUE <alexandre.torgue@gmail.com>
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> index cec147d..a94dd15 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> @@ -114,6 +114,18 @@ config DWMAC_SUNXI
>>           This selects Allwinner SoC glue layer support for the
>>           stmmac device driver. This driver is used for A20/A31
>>           GMAC ethernet controller.
>> +
>> +config DWMAC_STM32
>> +       tristate "STM32 DWMAC support"
>> +       default ARCH_STM32
>> +       depends on OF
>> +       select MFD_SYSCON
>> +       ---help---
>> +         Support for ethernet controller on STM32 SOCs.
>> +
>> +         This selects STM32 SoC glue layer support for the stmmac
>> +         device driver. This driver is used on for the STM32 series
>> +         SOCs GMAC ethernet controller.
>>  endif
>>
>>  config STMMAC_PCI
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> index b390161..9fb2061 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_DWMAC_SOCFPGA)   += dwmac-socfpga.o
>>  obj-$(CONFIG_DWMAC_STI)                += dwmac-sti.o
>>  obj-$(CONFIG_DWMAC_SUNXI)      += dwmac-sunxi.o
>>  obj-$(CONFIG_DWMAC_GENERIC)    += dwmac-generic.o
>> +obj-$(CONFIG_DWMAC_STM32)      += dwmac-stm32.o
>
> Keep these sorted. There also a comment in the Makefile that states
> that the generic should always be last.

ok
>
>
>>  stmmac-platform-objs:= stmmac_platform.o
>>
>>  obj-$(CONFIG_STMMAC_PCI) += stmmac-pci.o
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>> new file mode 100644
>> index 0000000..56ccc20
>> --- /dev/null
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>> @@ -0,0 +1,177 @@
>> +/*
>> + * dwmac-stm32.c - DWMAC Specific Glue layer for STM32 MCU
>> + *
>> + * Copyright (C) Alexandre Torgue 2015
>> + * Author:  Alexandre Torgue <alexandre.torgue@gmail.com>
>> + * License terms:  GNU General Public License (GPL), version 2
>> + *
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/slab.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/stmmac.h>
>> +#include <linux/phy.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/regmap.h>
>> +#include <linux/clk.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_net.h>
>
> Consider sorting the includes.

ok
>
>> +
>> +#include "stmmac_platform.h"
>> +
>> +#define MII_PHY_SEL_MASK       BIT(23)
>> +
>> +struct stm32_dwmac {
>> +       int interface;          /* MII interface */
>> +       struct clk *clk_tx;
>> +       struct clk *clk_rx;
>> +       u32 mode_reg;           /* MAC glue-logic mode register */
>> +       struct device *dev;
>
> dev doesn't seem to be used anywhere.

ok
>
>> +       struct regmap *regmap;
>> +       u32 speed;
>> +};
>> +
>> +static int stm32_dwmac_init(struct platform_device *pdev, void *priv)
>> +{
>> +       struct stm32_dwmac *dwmac = priv;
>> +       struct regmap *regmap = dwmac->regmap;
>> +       int ret, iface = dwmac->interface;
>> +       u32 reg = dwmac->mode_reg;
>> +       u32 val;
>> +
>> +       if (dwmac->clk_tx)
>> +               ret = clk_prepare_enable(dwmac->clk_tx);
>
> The clk API handles a NULL clk so you don't really need to check for it.

I agree
>
>
>> +               if (ret)
>> +                       goto out;
>> +
>> +       if (dwmac->clk_rx)
>> +               ret = clk_prepare_enable(dwmac->clk_rx);
>
> Same here.
I agree
>
>
>> +               if (ret)
>> +                       goto out_disable_clk_tx;
>> +
>> +       val = (iface == PHY_INTERFACE_MODE_MII) ? 0 : 1;
>> +       ret = regmap_update_bits(regmap, reg, MII_PHY_SEL_MASK, val);
>> +       if (ret)
>> +               goto out_disable_clk_tx_rx;
>> +
>> +       return 0;
>> +
>> +out_disable_clk_tx_rx:
>> +       clk_disable_unprepare(dwmac->clk_rx);
>> +out_disable_clk_tx:
>> +       clk_disable_unprepare(dwmac->clk_tx);
>> +out:
>> +       return ret;
>> +}
>> +
>> +static void stm32_dwmac_exit(struct platform_device *pdev, void *priv)
>> +{
>> +       struct stm32_dwmac *dwmac = priv;
>> +
>> +       if (dwmac->clk_tx)
>> +               clk_disable_unprepare(dwmac->clk_tx);
>> +       if (dwmac->clk_rx)
>> +               clk_disable_unprepare(dwmac->clk_rx);
>
> Same here.
I agree
>
>
>> +}
>> +
>> +static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
>> +                                 struct platform_device *pdev)
>> +{
>> +       struct device *dev = &pdev->dev;
>> +       struct device_node *np = dev->of_node;
>> +       struct regmap *regmap;
>> +       int err;
>> +
>> +       if (!np)
>> +               return -EINVAL;
>
> Can this ever happen?
> This is a DT only driver, right?
You're right. It is only DT.
>
>> +
>> +       /*  Get TX/RX clocks */
>> +       dwmac->clk_tx = devm_clk_get(dev, "tx-clk");
>> +       if (IS_ERR(dwmac->clk_tx)) {
>> +               dev_warn(dev, "No tx clock provided...\n");
>> +               dwmac->clk_tx = NULL;
>> +       }
>> +       dwmac->clk_rx = devm_clk_get(dev, "rx-clk");
>> +       if (IS_ERR(dwmac->clk_rx)) {
>> +               dev_warn(dev, "No rx clock provided...\n");
>> +               dwmac->clk_rx = NULL;
>> +       }
>> +
>> +       /* Get mode register */
>> +       regmap = syscon_regmap_lookup_by_phandle(np, "st,syscon");
>> +       if (IS_ERR(regmap))
>> +               return PTR_ERR(regmap);
>> +
>> +       err = of_property_read_u32_index(np, "st,syscon", 1, &dwmac->mode_reg);
>> +       if (err) {
>> +               dev_err(dev, "Can't get sysconfig mode offset (%d)\n", err);
>> +               return err;
>> +       }
>> +
>> +       dwmac->dev = dev;
>> +       dwmac->interface = of_get_phy_mode(np);
>> +       dwmac->regmap = regmap;
>> +
>> +       return 0;
>> +}
>> +
>> +static int stm32_dwmac_probe(struct platform_device *pdev)
>> +{
>> +       struct plat_stmmacenet_data *plat_dat;
>> +       struct stmmac_resources stmmac_res;
>> +       struct stm32_dwmac *dwmac;
>> +       int ret;
>> +
>> +       ret = stmmac_get_platform_resources(pdev, &stmmac_res);
>> +       if (ret)
>> +               return ret;
>> +
>> +       plat_dat = stmmac_probe_config_dt(pdev, &stmmac_res.mac);
>> +       if (IS_ERR(plat_dat))
>> +               return PTR_ERR(plat_dat);
>> +
>> +       dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
>> +       if (!dwmac)
>> +               return -ENOMEM;
>> +
>> +       ret = stm32_dwmac_parse_data(dwmac, pdev);
>> +       if (ret) {
>> +               dev_err(&pdev->dev, "Unable to parse OF data\n");
>> +               return ret;
>> +       }
>> +
>> +       plat_dat->bsp_priv = dwmac;
>> +       plat_dat->init = stm32_dwmac_init;
>> +       plat_dat->exit = stm32_dwmac_exit;
>
> Instead of using these callbacks could you rather implement the PM
> callbacks directly in this driver?
> I don't think it should add much code and it will make it look more
> like standard driver. This will also give you some more control and
> flexibility in your code.

I prefer to keep the code as it is. Glue layer is directly linked to
stmmac driver and I don't want to brake the link between the glue and
the stmmac driver.

>
>> +
>> +       ret = stm32_dwmac_init(pdev, plat_dat->bsp_priv);
>> +       if (ret)
>> +               return ret;
>> +
>> +       return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>> +}
>> +
>> +static const struct of_device_id stm32_dwmac_match[] = {
>> +       { .compatible = "st,stm32-dwmac"},
>> +       { }
>> +};
>> +MODULE_DEVICE_TABLE(of, stm32_dwmac_match);
>> +
>> +static struct platform_driver stm32_dwmac_driver = {
>> +       .probe  = stm32_dwmac_probe,
>> +       .remove = stmmac_pltfr_remove,
>
> Could you implement the .remove callback in your driver instead of
> using stmmac_pltfr_remove?
> Same reasons as above.

As explain above, I prefere to keep the code as it is in order to not
remove glue layer before the stmmac driver.

>
>
>> +       .driver = {
>> +               .name           = "stm32-dwmac",
>> +               .pm             = &stmmac_pltfr_pm_ops,
>> +               .of_match_table = stm32_dwmac_match,
>> +       },
>> +};
>> +module_platform_driver(stm32_dwmac_driver);
>> +
>> +MODULE_AUTHOR("Alexandre Torgue <alexandre.torgue@gmail.com>");
>> +MODULE_DESCRIPTION("STMicroelectronics MCU DWMAC Specific Glue layer");
>> +MODULE_LICENSE("GPL");
>
> Since you state:
>> + * License terms:  GNU General Public License (GPL), version 2
>
> You might want to switch use: MODULE_LICENSE("GPL v2");
>
I agree.
>
> regards,
> Joachim Eastwood

Thanks Joachim for the review and remarks. Main will be corrected in
next patch version.

regards
Alexandre Torgue

Re: [PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Alexandre Torgue]

2016-02-13 14:48 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
> On 3 February 2016 at 15:54, Alexandre TORGUE
> <alexandre.torgue@gmail.com> wrote:
>> stm324xx family chips support Synopsys MAC 3.510 IP.
>> This patch adds settings for logical glue logic:
>> -clocks
>> -mode selection MII or RMII.
>>
>> Signed-off-by: Alexandre TORGUE <alexandre.torgue@gmail.com>
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> index cec147d..a94dd15 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> @@ -114,6 +114,18 @@ config DWMAC_SUNXI
>>           This selects Allwinner SoC glue layer support for the
>>           stmmac device driver. This driver is used for A20/A31
>>           GMAC ethernet controller.
>> +
>> +config DWMAC_STM32
>> +       tristate "STM32 DWMAC support"
>> +       default ARCH_STM32
>> +       depends on OF
>> +       select MFD_SYSCON
>> +       ---help---
>> +         Support for ethernet controller on STM32 SOCs.
>> +
>> +         This selects STM32 SoC glue layer support for the stmmac
>> +         device driver. This driver is used on for the STM32 series
>> +         SOCs GMAC ethernet controller.
>>  endif
>>
>>  config STMMAC_PCI
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> index b390161..9fb2061 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_DWMAC_SOCFPGA)   += dwmac-socfpga.o
>>  obj-$(CONFIG_DWMAC_STI)                += dwmac-sti.o
>>  obj-$(CONFIG_DWMAC_SUNXI)      += dwmac-sunxi.o
>>  obj-$(CONFIG_DWMAC_GENERIC)    += dwmac-generic.o
>> +obj-$(CONFIG_DWMAC_STM32)      += dwmac-stm32.o
>
> Keep these sorted. There also a comment in the Makefile that states
> that the generic should always be last.

ok
>
>
>>  stmmac-platform-objs:= stmmac_platform.o
>>
>>  obj-$(CONFIG_STMMAC_PCI) += stmmac-pci.o
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>> new file mode 100644
>> index 0000000..56ccc20
>> --- /dev/null
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>> @@ -0,0 +1,177 @@
>> +/*
>> + * dwmac-stm32.c - DWMAC Specific Glue layer for STM32 MCU
>> + *
>> + * Copyright (C) Alexandre Torgue 2015
>> + * Author:  Alexandre Torgue <alexandre.torgue@gmail.com>
>> + * License terms:  GNU General Public License (GPL), version 2
>> + *
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/slab.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/stmmac.h>
>> +#include <linux/phy.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/regmap.h>
>> +#include <linux/clk.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_net.h>
>
> Consider sorting the includes.

ok
>
>> +
>> +#include "stmmac_platform.h"
>> +
>> +#define MII_PHY_SEL_MASK       BIT(23)
>> +
>> +struct stm32_dwmac {
>> +       int interface;          /* MII interface */
>> +       struct clk *clk_tx;
>> +       struct clk *clk_rx;
>> +       u32 mode_reg;           /* MAC glue-logic mode register */
>> +       struct device *dev;
>
> dev doesn't seem to be used anywhere.

ok
>
>> +       struct regmap *regmap;
>> +       u32 speed;
>> +};
>> +
>> +static int stm32_dwmac_init(struct platform_device *pdev, void *priv)
>> +{
>> +       struct stm32_dwmac *dwmac = priv;
>> +       struct regmap *regmap = dwmac->regmap;
>> +       int ret, iface = dwmac->interface;
>> +       u32 reg = dwmac->mode_reg;
>> +       u32 val;
>> +
>> +       if (dwmac->clk_tx)
>> +               ret = clk_prepare_enable(dwmac->clk_tx);
>
> The clk API handles a NULL clk so you don't really need to check for it.

I agree
>
>
>> +               if (ret)
>> +                       goto out;
>> +
>> +       if (dwmac->clk_rx)
>> +               ret = clk_prepare_enable(dwmac->clk_rx);
>
> Same here.
I agree
>
>
>> +               if (ret)
>> +                       goto out_disable_clk_tx;
>> +
>> +       val = (iface == PHY_INTERFACE_MODE_MII) ? 0 : 1;
>> +       ret = regmap_update_bits(regmap, reg, MII_PHY_SEL_MASK, val);
>> +       if (ret)
>> +               goto out_disable_clk_tx_rx;
>> +
>> +       return 0;
>> +
>> +out_disable_clk_tx_rx:
>> +       clk_disable_unprepare(dwmac->clk_rx);
>> +out_disable_clk_tx:
>> +       clk_disable_unprepare(dwmac->clk_tx);
>> +out:
>> +       return ret;
>> +}
>> +
>> +static void stm32_dwmac_exit(struct platform_device *pdev, void *priv)
>> +{
>> +       struct stm32_dwmac *dwmac = priv;
>> +
>> +       if (dwmac->clk_tx)
>> +               clk_disable_unprepare(dwmac->clk_tx);
>> +       if (dwmac->clk_rx)
>> +               clk_disable_unprepare(dwmac->clk_rx);
>
> Same here.
I agree
>
>
>> +}
>> +
>> +static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
>> +                                 struct platform_device *pdev)
>> +{
>> +       struct device *dev = &pdev->dev;
>> +       struct device_node *np = dev->of_node;
>> +       struct regmap *regmap;
>> +       int err;
>> +
>> +       if (!np)
>> +               return -EINVAL;
>
> Can this ever happen?
> This is a DT only driver, right?
You're right. It is only DT.
>
>> +
>> +       /*  Get TX/RX clocks */
>> +       dwmac->clk_tx = devm_clk_get(dev, "tx-clk");
>> +       if (IS_ERR(dwmac->clk_tx)) {
>> +               dev_warn(dev, "No tx clock provided...\n");
>> +               dwmac->clk_tx = NULL;
>> +       }
>> +       dwmac->clk_rx = devm_clk_get(dev, "rx-clk");
>> +       if (IS_ERR(dwmac->clk_rx)) {
>> +               dev_warn(dev, "No rx clock provided...\n");
>> +               dwmac->clk_rx = NULL;
>> +       }
>> +
>> +       /* Get mode register */
>> +       regmap = syscon_regmap_lookup_by_phandle(np, "st,syscon");
>> +       if (IS_ERR(regmap))
>> +               return PTR_ERR(regmap);
>> +
>> +       err = of_property_read_u32_index(np, "st,syscon", 1, &dwmac->mode_reg);
>> +       if (err) {
>> +               dev_err(dev, "Can't get sysconfig mode offset (%d)\n", err);
>> +               return err;
>> +       }
>> +
>> +       dwmac->dev = dev;
>> +       dwmac->interface = of_get_phy_mode(np);
>> +       dwmac->regmap = regmap;
>> +
>> +       return 0;
>> +}
>> +
>> +static int stm32_dwmac_probe(struct platform_device *pdev)
>> +{
>> +       struct plat_stmmacenet_data *plat_dat;
>> +       struct stmmac_resources stmmac_res;
>> +       struct stm32_dwmac *dwmac;
>> +       int ret;
>> +
>> +       ret = stmmac_get_platform_resources(pdev, &stmmac_res);
>> +       if (ret)
>> +               return ret;
>> +
>> +       plat_dat = stmmac_probe_config_dt(pdev, &stmmac_res.mac);
>> +       if (IS_ERR(plat_dat))
>> +               return PTR_ERR(plat_dat);
>> +
>> +       dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
>> +       if (!dwmac)
>> +               return -ENOMEM;
>> +
>> +       ret = stm32_dwmac_parse_data(dwmac, pdev);
>> +       if (ret) {
>> +               dev_err(&pdev->dev, "Unable to parse OF data\n");
>> +               return ret;
>> +       }
>> +
>> +       plat_dat->bsp_priv = dwmac;
>> +       plat_dat->init = stm32_dwmac_init;
>> +       plat_dat->exit = stm32_dwmac_exit;
>
> Instead of using these callbacks could you rather implement the PM
> callbacks directly in this driver?
> I don't think it should add much code and it will make it look more
> like standard driver. This will also give you some more control and
> flexibility in your code.

I prefer to keep the code as it is. Glue layer is directly linked to
stmmac driver and I don't want to brake the link between the glue and
the stmmac driver.

>
>> +
>> +       ret = stm32_dwmac_init(pdev, plat_dat->bsp_priv);
>> +       if (ret)
>> +               return ret;
>> +
>> +       return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>> +}
>> +
>> +static const struct of_device_id stm32_dwmac_match[] = {
>> +       { .compatible = "st,stm32-dwmac"},
>> +       { }
>> +};
>> +MODULE_DEVICE_TABLE(of, stm32_dwmac_match);
>> +
>> +static struct platform_driver stm32_dwmac_driver = {
>> +       .probe  = stm32_dwmac_probe,
>> +       .remove = stmmac_pltfr_remove,
>
> Could you implement the .remove callback in your driver instead of
> using stmmac_pltfr_remove?
> Same reasons as above.

As explain above, I prefere to keep the code as it is in order to not
remove glue layer before the stmmac driver.

>
>
>> +       .driver = {
>> +               .name           = "stm32-dwmac",
>> +               .pm             = &stmmac_pltfr_pm_ops,
>> +               .of_match_table = stm32_dwmac_match,
>> +       },
>> +};
>> +module_platform_driver(stm32_dwmac_driver);
>> +
>> +MODULE_AUTHOR("Alexandre Torgue <alexandre.torgue@gmail.com>");
>> +MODULE_DESCRIPTION("STMicroelectronics MCU DWMAC Specific Glue layer");
>> +MODULE_LICENSE("GPL");
>
> Since you state:
>> + * License terms:  GNU General Public License (GPL), version 2
>
> You might want to switch use: MODULE_LICENSE("GPL v2");
>
I agree.
>
> regards,
> Joachim Eastwood

Thanks Joachim for the review and remarks. Main will be corrected in
next patch version.

regards
Alexandre Torgue

[PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Alexandre Torgue]

2016-02-13 14:48 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
> On 3 February 2016 at 15:54, Alexandre TORGUE
> <alexandre.torgue@gmail.com> wrote:
>> stm324xx family chips support Synopsys MAC 3.510 IP.
>> This patch adds settings for logical glue logic:
>> -clocks
>> -mode selection MII or RMII.
>>
>> Signed-off-by: Alexandre TORGUE <alexandre.torgue@gmail.com>
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> index cec147d..a94dd15 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> @@ -114,6 +114,18 @@ config DWMAC_SUNXI
>>           This selects Allwinner SoC glue layer support for the
>>           stmmac device driver. This driver is used for A20/A31
>>           GMAC ethernet controller.
>> +
>> +config DWMAC_STM32
>> +       tristate "STM32 DWMAC support"
>> +       default ARCH_STM32
>> +       depends on OF
>> +       select MFD_SYSCON
>> +       ---help---
>> +         Support for ethernet controller on STM32 SOCs.
>> +
>> +         This selects STM32 SoC glue layer support for the stmmac
>> +         device driver. This driver is used on for the STM32 series
>> +         SOCs GMAC ethernet controller.
>>  endif
>>
>>  config STMMAC_PCI
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> index b390161..9fb2061 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_DWMAC_SOCFPGA)   += dwmac-socfpga.o
>>  obj-$(CONFIG_DWMAC_STI)                += dwmac-sti.o
>>  obj-$(CONFIG_DWMAC_SUNXI)      += dwmac-sunxi.o
>>  obj-$(CONFIG_DWMAC_GENERIC)    += dwmac-generic.o
>> +obj-$(CONFIG_DWMAC_STM32)      += dwmac-stm32.o
>
> Keep these sorted. There also a comment in the Makefile that states
> that the generic should always be last.

ok
>
>
>>  stmmac-platform-objs:= stmmac_platform.o
>>
>>  obj-$(CONFIG_STMMAC_PCI) += stmmac-pci.o
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>> new file mode 100644
>> index 0000000..56ccc20
>> --- /dev/null
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>> @@ -0,0 +1,177 @@
>> +/*
>> + * dwmac-stm32.c - DWMAC Specific Glue layer for STM32 MCU
>> + *
>> + * Copyright (C) Alexandre Torgue 2015
>> + * Author:  Alexandre Torgue <alexandre.torgue@gmail.com>
>> + * License terms:  GNU General Public License (GPL), version 2
>> + *
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/slab.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/stmmac.h>
>> +#include <linux/phy.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/regmap.h>
>> +#include <linux/clk.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_net.h>
>
> Consider sorting the includes.

ok
>
>> +
>> +#include "stmmac_platform.h"
>> +
>> +#define MII_PHY_SEL_MASK       BIT(23)
>> +
>> +struct stm32_dwmac {
>> +       int interface;          /* MII interface */
>> +       struct clk *clk_tx;
>> +       struct clk *clk_rx;
>> +       u32 mode_reg;           /* MAC glue-logic mode register */
>> +       struct device *dev;
>
> dev doesn't seem to be used anywhere.

ok
>
>> +       struct regmap *regmap;
>> +       u32 speed;
>> +};
>> +
>> +static int stm32_dwmac_init(struct platform_device *pdev, void *priv)
>> +{
>> +       struct stm32_dwmac *dwmac = priv;
>> +       struct regmap *regmap = dwmac->regmap;
>> +       int ret, iface = dwmac->interface;
>> +       u32 reg = dwmac->mode_reg;
>> +       u32 val;
>> +
>> +       if (dwmac->clk_tx)
>> +               ret = clk_prepare_enable(dwmac->clk_tx);
>
> The clk API handles a NULL clk so you don't really need to check for it.

I agree
>
>
>> +               if (ret)
>> +                       goto out;
>> +
>> +       if (dwmac->clk_rx)
>> +               ret = clk_prepare_enable(dwmac->clk_rx);
>
> Same here.
I agree
>
>
>> +               if (ret)
>> +                       goto out_disable_clk_tx;
>> +
>> +       val = (iface == PHY_INTERFACE_MODE_MII) ? 0 : 1;
>> +       ret = regmap_update_bits(regmap, reg, MII_PHY_SEL_MASK, val);
>> +       if (ret)
>> +               goto out_disable_clk_tx_rx;
>> +
>> +       return 0;
>> +
>> +out_disable_clk_tx_rx:
>> +       clk_disable_unprepare(dwmac->clk_rx);
>> +out_disable_clk_tx:
>> +       clk_disable_unprepare(dwmac->clk_tx);
>> +out:
>> +       return ret;
>> +}
>> +
>> +static void stm32_dwmac_exit(struct platform_device *pdev, void *priv)
>> +{
>> +       struct stm32_dwmac *dwmac = priv;
>> +
>> +       if (dwmac->clk_tx)
>> +               clk_disable_unprepare(dwmac->clk_tx);
>> +       if (dwmac->clk_rx)
>> +               clk_disable_unprepare(dwmac->clk_rx);
>
> Same here.
I agree
>
>
>> +}
>> +
>> +static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
>> +                                 struct platform_device *pdev)
>> +{
>> +       struct device *dev = &pdev->dev;
>> +       struct device_node *np = dev->of_node;
>> +       struct regmap *regmap;
>> +       int err;
>> +
>> +       if (!np)
>> +               return -EINVAL;
>
> Can this ever happen?
> This is a DT only driver, right?
You're right. It is only DT.
>
>> +
>> +       /*  Get TX/RX clocks */
>> +       dwmac->clk_tx = devm_clk_get(dev, "tx-clk");
>> +       if (IS_ERR(dwmac->clk_tx)) {
>> +               dev_warn(dev, "No tx clock provided...\n");
>> +               dwmac->clk_tx = NULL;
>> +       }
>> +       dwmac->clk_rx = devm_clk_get(dev, "rx-clk");
>> +       if (IS_ERR(dwmac->clk_rx)) {
>> +               dev_warn(dev, "No rx clock provided...\n");
>> +               dwmac->clk_rx = NULL;
>> +       }
>> +
>> +       /* Get mode register */
>> +       regmap = syscon_regmap_lookup_by_phandle(np, "st,syscon");
>> +       if (IS_ERR(regmap))
>> +               return PTR_ERR(regmap);
>> +
>> +       err = of_property_read_u32_index(np, "st,syscon", 1, &dwmac->mode_reg);
>> +       if (err) {
>> +               dev_err(dev, "Can't get sysconfig mode offset (%d)\n", err);
>> +               return err;
>> +       }
>> +
>> +       dwmac->dev = dev;
>> +       dwmac->interface = of_get_phy_mode(np);
>> +       dwmac->regmap = regmap;
>> +
>> +       return 0;
>> +}
>> +
>> +static int stm32_dwmac_probe(struct platform_device *pdev)
>> +{
>> +       struct plat_stmmacenet_data *plat_dat;
>> +       struct stmmac_resources stmmac_res;
>> +       struct stm32_dwmac *dwmac;
>> +       int ret;
>> +
>> +       ret = stmmac_get_platform_resources(pdev, &stmmac_res);
>> +       if (ret)
>> +               return ret;
>> +
>> +       plat_dat = stmmac_probe_config_dt(pdev, &stmmac_res.mac);
>> +       if (IS_ERR(plat_dat))
>> +               return PTR_ERR(plat_dat);
>> +
>> +       dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
>> +       if (!dwmac)
>> +               return -ENOMEM;
>> +
>> +       ret = stm32_dwmac_parse_data(dwmac, pdev);
>> +       if (ret) {
>> +               dev_err(&pdev->dev, "Unable to parse OF data\n");
>> +               return ret;
>> +       }
>> +
>> +       plat_dat->bsp_priv = dwmac;
>> +       plat_dat->init = stm32_dwmac_init;
>> +       plat_dat->exit = stm32_dwmac_exit;
>
> Instead of using these callbacks could you rather implement the PM
> callbacks directly in this driver?
> I don't think it should add much code and it will make it look more
> like standard driver. This will also give you some more control and
> flexibility in your code.

I prefer to keep the code as it is. Glue layer is directly linked to
stmmac driver and I don't want to brake the link between the glue and
the stmmac driver.

>
>> +
>> +       ret = stm32_dwmac_init(pdev, plat_dat->bsp_priv);
>> +       if (ret)
>> +               return ret;
>> +
>> +       return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>> +}
>> +
>> +static const struct of_device_id stm32_dwmac_match[] = {
>> +       { .compatible = "st,stm32-dwmac"},
>> +       { }
>> +};
>> +MODULE_DEVICE_TABLE(of, stm32_dwmac_match);
>> +
>> +static struct platform_driver stm32_dwmac_driver = {
>> +       .probe  = stm32_dwmac_probe,
>> +       .remove = stmmac_pltfr_remove,
>
> Could you implement the .remove callback in your driver instead of
> using stmmac_pltfr_remove?
> Same reasons as above.

As explain above, I prefere to keep the code as it is in order to not
remove glue layer before the stmmac driver.

>
>
>> +       .driver = {
>> +               .name           = "stm32-dwmac",
>> +               .pm             = &stmmac_pltfr_pm_ops,
>> +               .of_match_table = stm32_dwmac_match,
>> +       },
>> +};
>> +module_platform_driver(stm32_dwmac_driver);
>> +
>> +MODULE_AUTHOR("Alexandre Torgue <alexandre.torgue@gmail.com>");
>> +MODULE_DESCRIPTION("STMicroelectronics MCU DWMAC Specific Glue layer");
>> +MODULE_LICENSE("GPL");
>
> Since you state:
>> + * License terms:  GNU General Public License (GPL), version 2
>
> You might want to switch use: MODULE_LICENSE("GPL v2");
>
I agree.
>
> regards,
> Joachim Eastwood

Thanks Joachim for the review and remarks. Main will be corrected in
next patch version.

regards
Alexandre Torgue

Re: [PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Joachim Eastwood]

On 22 February 2016 at 15:50, Alexandre Torgue
<alexandre.torgue@gmail.com> wrote:
> 2016-02-13 14:48 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
>> On 3 February 2016 at 15:54, Alexandre TORGUE
>> <alexandre.torgue@gmail.com> wrote:
>>> +       plat_dat->bsp_priv = dwmac;
>>> +       plat_dat->init = stm32_dwmac_init;
>>> +       plat_dat->exit = stm32_dwmac_exit;
>>
>> Instead of using these callbacks could you rather implement the PM
>> callbacks directly in this driver?
>> I don't think it should add much code and it will make it look more
>> like standard driver. This will also give you some more control and
>> flexibility in your code.
>
> I prefer to keep the code as it is. Glue layer is directly linked to
> stmmac driver and I don't want to brake the link between the glue and
> the stmmac driver.

What do you mean by break the link?

There has been numerous of patch sets to make the stmmac "glue"
drivers into more standard platform drivers.
http://marc.info/?l=linux-netdev&m=143159850631093&w=2
http://marc.info/?l=linux-netdev&m=143708560009851&w=2
http://marc.info/?l=linux-netdev&m=143812136600541&w=2

Do you see any advantage by using the init and exit hooks in your
driver instead of using the standard driver PM callbacks and remove
function?
The only "cost" I see is slightly more boilerplate code. But since you
already have init/exit functions you could easily make them into PM
resume/suspend so I doubt there would be much increase in code size.

One other thing;
Do you need to have the PHY mode setup code in the init function which
is called each time on resume?
If you could move it to probe you could drop the interface priv data
member and use plat_dat->interface as stmmac_probe_config_dt() has
already done of_get_phy_mode().


regards,
Joachim Eastwood

Re: [PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Joachim Eastwood]

On 22 February 2016 at 15:50, Alexandre Torgue
<alexandre.torgue@gmail.com> wrote:
> 2016-02-13 14:48 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
>> On 3 February 2016 at 15:54, Alexandre TORGUE
>> <alexandre.torgue@gmail.com> wrote:
>>> +       plat_dat->bsp_priv = dwmac;
>>> +       plat_dat->init = stm32_dwmac_init;
>>> +       plat_dat->exit = stm32_dwmac_exit;
>>
>> Instead of using these callbacks could you rather implement the PM
>> callbacks directly in this driver?
>> I don't think it should add much code and it will make it look more
>> like standard driver. This will also give you some more control and
>> flexibility in your code.
>
> I prefer to keep the code as it is. Glue layer is directly linked to
> stmmac driver and I don't want to brake the link between the glue and
> the stmmac driver.

What do you mean by break the link?

There has been numerous of patch sets to make the stmmac "glue"
drivers into more standard platform drivers.
http://marc.info/?l=linux-netdev&m=143159850631093&w=2
http://marc.info/?l=linux-netdev&m=143708560009851&w=2
http://marc.info/?l=linux-netdev&m=143812136600541&w=2

Do you see any advantage by using the init and exit hooks in your
driver instead of using the standard driver PM callbacks and remove
function?
The only "cost" I see is slightly more boilerplate code. But since you
already have init/exit functions you could easily make them into PM
resume/suspend so I doubt there would be much increase in code size.

One other thing;
Do you need to have the PHY mode setup code in the init function which
is called each time on resume?
If you could move it to probe you could drop the interface priv data
member and use plat_dat->interface as stmmac_probe_config_dt() has
already done of_get_phy_mode().


regards,
Joachim Eastwood

[PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Joachim Eastwood]

On 22 February 2016 at 15:50, Alexandre Torgue
<alexandre.torgue@gmail.com> wrote:
> 2016-02-13 14:48 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
>> On 3 February 2016 at 15:54, Alexandre TORGUE
>> <alexandre.torgue@gmail.com> wrote:
>>> +       plat_dat->bsp_priv = dwmac;
>>> +       plat_dat->init = stm32_dwmac_init;
>>> +       plat_dat->exit = stm32_dwmac_exit;
>>
>> Instead of using these callbacks could you rather implement the PM
>> callbacks directly in this driver?
>> I don't think it should add much code and it will make it look more
>> like standard driver. This will also give you some more control and
>> flexibility in your code.
>
> I prefer to keep the code as it is. Glue layer is directly linked to
> stmmac driver and I don't want to brake the link between the glue and
> the stmmac driver.

What do you mean by break the link?

There has been numerous of patch sets to make the stmmac "glue"
drivers into more standard platform drivers.
http://marc.info/?l=linux-netdev&m=143159850631093&w=2
http://marc.info/?l=linux-netdev&m=143708560009851&w=2
http://marc.info/?l=linux-netdev&m=143812136600541&w=2

Do you see any advantage by using the init and exit hooks in your
driver instead of using the standard driver PM callbacks and remove
function?
The only "cost" I see is slightly more boilerplate code. But since you
already have init/exit functions you could easily make them into PM
resume/suspend so I doubt there would be much increase in code size.

One other thing;
Do you need to have the PHY mode setup code in the init function which
is called each time on resume?
If you could move it to probe you could drop the interface priv data
member and use plat_dat->interface as stmmac_probe_config_dt() has
already done of_get_phy_mode().


regards,
Joachim Eastwood

Re: [PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Alexandre Torgue]

2016-02-22 22:52 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
> On 22 February 2016 at 15:50, Alexandre Torgue
> <alexandre.torgue@gmail.com> wrote:
>> 2016-02-13 14:48 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
>>> On 3 February 2016 at 15:54, Alexandre TORGUE
>>> <alexandre.torgue@gmail.com> wrote:
>>>> +       plat_dat->bsp_priv = dwmac;
>>>> +       plat_dat->init = stm32_dwmac_init;
>>>> +       plat_dat->exit = stm32_dwmac_exit;
>>>
>>> Instead of using these callbacks could you rather implement the PM
>>> callbacks directly in this driver?
>>> I don't think it should add much code and it will make it look more
>>> like standard driver. This will also give you some more control and
>>> flexibility in your code.
>>
>> I prefer to keep the code as it is. Glue layer is directly linked to
>> stmmac driver and I don't want to brake the link between the glue and
>> the stmmac driver.
>
> What do you mean by break the link?
>

I thought that you wanted to split stmmac_pltfr_supend (glue part and
stmamc part), but I well understood it is not the case (sorry for
mistake).

> There has been numerous of patch sets to make the stmmac "glue"
> drivers into more standard platform drivers.
> http://marc.info/?l=linux-netdev&m=143159850631093&w=2
> http://marc.info/?l=linux-netdev&m=143708560009851&w=2
> http://marc.info/?l=linux-netdev&m=143812136600541&w=2
>
> Do you see any advantage by using the init and exit hooks in your
> driver instead of using the standard driver PM callbacks and remove
> function?
> The only "cost" I see is slightly more boilerplate code. But since you
> already have init/exit functions you could easily make them into PM
> resume/suspend so I doubt there would be much increase in code size.
>

If I well understood you want to continue the stmmac glue driver
rework by moving stmmac_pltfr_suspend/resume/remove in each glue
driver (stm32, sun, sti ....).
Each glue driver will call directly stmmac_suspend/resume/remove and
their own init/exit function.
If it is what you meant, I can do it.

> One other thing;
> Do you need to have the PHY mode setup code in the init function which
> is called each time on resume?

I can't guarantee that after a suspend the sysconfig register will
contain same data than before suspend.

Best regards.

Alex

> If you could move it to probe you could drop the interface priv data
> member and use plat_dat->interface as stmmac_probe_config_dt() has
> already done of_get_phy_mode().
>
>
> regards,
> Joachim Eastwood

Re: [PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Alexandre Torgue]

2016-02-22 22:52 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
> On 22 February 2016 at 15:50, Alexandre Torgue
> <alexandre.torgue@gmail.com> wrote:
>> 2016-02-13 14:48 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
>>> On 3 February 2016 at 15:54, Alexandre TORGUE
>>> <alexandre.torgue@gmail.com> wrote:
>>>> +       plat_dat->bsp_priv = dwmac;
>>>> +       plat_dat->init = stm32_dwmac_init;
>>>> +       plat_dat->exit = stm32_dwmac_exit;
>>>
>>> Instead of using these callbacks could you rather implement the PM
>>> callbacks directly in this driver?
>>> I don't think it should add much code and it will make it look more
>>> like standard driver. This will also give you some more control and
>>> flexibility in your code.
>>
>> I prefer to keep the code as it is. Glue layer is directly linked to
>> stmmac driver and I don't want to brake the link between the glue and
>> the stmmac driver.
>
> What do you mean by break the link?
>

I thought that you wanted to split stmmac_pltfr_supend (glue part and
stmamc part), but I well understood it is not the case (sorry for
mistake).

> There has been numerous of patch sets to make the stmmac "glue"
> drivers into more standard platform drivers.
> http://marc.info/?l=linux-netdev&m=143159850631093&w=2
> http://marc.info/?l=linux-netdev&m=143708560009851&w=2
> http://marc.info/?l=linux-netdev&m=143812136600541&w=2
>
> Do you see any advantage by using the init and exit hooks in your
> driver instead of using the standard driver PM callbacks and remove
> function?
> The only "cost" I see is slightly more boilerplate code. But since you
> already have init/exit functions you could easily make them into PM
> resume/suspend so I doubt there would be much increase in code size.
>

If I well understood you want to continue the stmmac glue driver
rework by moving stmmac_pltfr_suspend/resume/remove in each glue
driver (stm32, sun, sti ....).
Each glue driver will call directly stmmac_suspend/resume/remove and
their own init/exit function.
If it is what you meant, I can do it.

> One other thing;
> Do you need to have the PHY mode setup code in the init function which
> is called each time on resume?

I can't guarantee that after a suspend the sysconfig register will
contain same data than before suspend.

Best regards.

Alex

> If you could move it to probe you could drop the interface priv data
> member and use plat_dat->interface as stmmac_probe_config_dt() has
> already done of_get_phy_mode().
>
>
> regards,
> Joachim Eastwood

[PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Alexandre Torgue]

2016-02-22 22:52 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
> On 22 February 2016 at 15:50, Alexandre Torgue
> <alexandre.torgue@gmail.com> wrote:
>> 2016-02-13 14:48 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
>>> On 3 February 2016 at 15:54, Alexandre TORGUE
>>> <alexandre.torgue@gmail.com> wrote:
>>>> +       plat_dat->bsp_priv = dwmac;
>>>> +       plat_dat->init = stm32_dwmac_init;
>>>> +       plat_dat->exit = stm32_dwmac_exit;
>>>
>>> Instead of using these callbacks could you rather implement the PM
>>> callbacks directly in this driver?
>>> I don't think it should add much code and it will make it look more
>>> like standard driver. This will also give you some more control and
>>> flexibility in your code.
>>
>> I prefer to keep the code as it is. Glue layer is directly linked to
>> stmmac driver and I don't want to brake the link between the glue and
>> the stmmac driver.
>
> What do you mean by break the link?
>

I thought that you wanted to split stmmac_pltfr_supend (glue part and
stmamc part), but I well understood it is not the case (sorry for
mistake).

> There has been numerous of patch sets to make the stmmac "glue"
> drivers into more standard platform drivers.
> http://marc.info/?l=linux-netdev&m=143159850631093&w=2
> http://marc.info/?l=linux-netdev&m=143708560009851&w=2
> http://marc.info/?l=linux-netdev&m=143812136600541&w=2
>
> Do you see any advantage by using the init and exit hooks in your
> driver instead of using the standard driver PM callbacks and remove
> function?
> The only "cost" I see is slightly more boilerplate code. But since you
> already have init/exit functions you could easily make them into PM
> resume/suspend so I doubt there would be much increase in code size.
>

If I well understood you want to continue the stmmac glue driver
rework by moving stmmac_pltfr_suspend/resume/remove in each glue
driver (stm32, sun, sti ....).
Each glue driver will call directly stmmac_suspend/resume/remove and
their own init/exit function.
If it is what you meant, I can do it.

> One other thing;
> Do you need to have the PHY mode setup code in the init function which
> is called each time on resume?

I can't guarantee that after a suspend the sysconfig register will
contain same data than before suspend.

Best regards.

Alex

> If you could move it to probe you could drop the interface priv data
> member and use plat_dat->interface as stmmac_probe_config_dt() has
> already done of_get_phy_mode().
>
>
> regards,
> Joachim Eastwood

Re: [PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Joachim Eastwood]

On 23 February 2016 at 10:59, Alexandre Torgue
<alexandre.torgue@gmail.com> wrote:
> 2016-02-22 22:52 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
>> On 22 February 2016 at 15:50, Alexandre Torgue
>> <alexandre.torgue@gmail.com> wrote:
>>> 2016-02-13 14:48 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
>>>> On 3 February 2016 at 15:54, Alexandre TORGUE
>>>> <alexandre.torgue@gmail.com> wrote:
>>>>> +       plat_dat->bsp_priv = dwmac;
>>>>> +       plat_dat->init = stm32_dwmac_init;
>>>>> +       plat_dat->exit = stm32_dwmac_exit;
>>>>
>>>> Instead of using these callbacks could you rather implement the PM
>>>> callbacks directly in this driver?
>>>> I don't think it should add much code and it will make it look more
>>>> like standard driver. This will also give you some more control and
>>>> flexibility in your code.
>>>
>>> I prefer to keep the code as it is. Glue layer is directly linked to
>>> stmmac driver and I don't want to brake the link between the glue and
>>> the stmmac driver.
>>
>> What do you mean by break the link?
>>
>
> I thought that you wanted to split stmmac_pltfr_supend (glue part and
> stmamc part), but I well understood it is not the case (sorry for
> mistake).
>
>> There has been numerous of patch sets to make the stmmac "glue"
>> drivers into more standard platform drivers.
>> http://marc.info/?l=linux-netdev&m=143159850631093&w=2
>> http://marc.info/?l=linux-netdev&m=143708560009851&w=2
>> http://marc.info/?l=linux-netdev&m=143812136600541&w=2
>>
>> Do you see any advantage by using the init and exit hooks in your
>> driver instead of using the standard driver PM callbacks and remove
>> function?
>> The only "cost" I see is slightly more boilerplate code. But since you
>> already have init/exit functions you could easily make them into PM
>> resume/suspend so I doubt there would be much increase in code size.
>>
>
> If I well understood you want to continue the stmmac glue driver
> rework by moving stmmac_pltfr_suspend/resume/remove in each glue
> driver (stm32, sun, sti ....).

At least I want to avoid the init/exit callbacks for new drivers like
stm32-dwmac.


> Each glue driver will call directly stmmac_suspend/resume/remove and
> their own init/exit function.
> If it is what you meant, I can do it.

Yes, in your stm32 driver's suspend/resume/remove functions call
stmmac_suspend/stmmac_resume/stmmac_dvr_remove directly. Then you
shouldn't need to use the init/exit callbacks. Just put the need code
in the driver's suspend/resume/remove functions instead of init/exit
functions.

For example:
static int stm32_dwmac_resume(struct device *dev)
{
    struct net_device *ndev = dev_get_drvdata(dev);
    struct plat_stmmacenet_data *plat_dat = get_stmmac_plat_data(ndev)
    struct stm32_dwmac *dwmac =plat_dat->bsp_priv;

    /* enable clocks */
    /* set phy mode */

    return stmmac_resume(ndev);
}

If it makes sense to have the enable clk/phy mode stuff in it's own
function that is fine too.


>> One other thing;
>> Do you need to have the PHY mode setup code in the init function which
>> is called each time on resume?
>
> I can't guarantee that after a suspend the sysconfig register will
> contain same data than before suspend.

I see.


regards,
Joachim Eastwood

Re: [PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Joachim Eastwood]

On 23 February 2016 at 10:59, Alexandre Torgue
<alexandre.torgue@gmail.com> wrote:
> 2016-02-22 22:52 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
>> On 22 February 2016 at 15:50, Alexandre Torgue
>> <alexandre.torgue@gmail.com> wrote:
>>> 2016-02-13 14:48 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
>>>> On 3 February 2016 at 15:54, Alexandre TORGUE
>>>> <alexandre.torgue@gmail.com> wrote:
>>>>> +       plat_dat->bsp_priv = dwmac;
>>>>> +       plat_dat->init = stm32_dwmac_init;
>>>>> +       plat_dat->exit = stm32_dwmac_exit;
>>>>
>>>> Instead of using these callbacks could you rather implement the PM
>>>> callbacks directly in this driver?
>>>> I don't think it should add much code and it will make it look more
>>>> like standard driver. This will also give you some more control and
>>>> flexibility in your code.
>>>
>>> I prefer to keep the code as it is. Glue layer is directly linked to
>>> stmmac driver and I don't want to brake the link between the glue and
>>> the stmmac driver.
>>
>> What do you mean by break the link?
>>
>
> I thought that you wanted to split stmmac_pltfr_supend (glue part and
> stmamc part), but I well understood it is not the case (sorry for
> mistake).
>
>> There has been numerous of patch sets to make the stmmac "glue"
>> drivers into more standard platform drivers.
>> http://marc.info/?l=linux-netdev&m=143159850631093&w=2
>> http://marc.info/?l=linux-netdev&m=143708560009851&w=2
>> http://marc.info/?l=linux-netdev&m=143812136600541&w=2
>>
>> Do you see any advantage by using the init and exit hooks in your
>> driver instead of using the standard driver PM callbacks and remove
>> function?
>> The only "cost" I see is slightly more boilerplate code. But since you
>> already have init/exit functions you could easily make them into PM
>> resume/suspend so I doubt there would be much increase in code size.
>>
>
> If I well understood you want to continue the stmmac glue driver
> rework by moving stmmac_pltfr_suspend/resume/remove in each glue
> driver (stm32, sun, sti ....).

At least I want to avoid the init/exit callbacks for new drivers like
stm32-dwmac.


> Each glue driver will call directly stmmac_suspend/resume/remove and
> their own init/exit function.
> If it is what you meant, I can do it.

Yes, in your stm32 driver's suspend/resume/remove functions call
stmmac_suspend/stmmac_resume/stmmac_dvr_remove directly. Then you
shouldn't need to use the init/exit callbacks. Just put the need code
in the driver's suspend/resume/remove functions instead of init/exit
functions.

For example:
static int stm32_dwmac_resume(struct device *dev)
{
    struct net_device *ndev = dev_get_drvdata(dev);
    struct plat_stmmacenet_data *plat_dat = get_stmmac_plat_data(ndev)
    struct stm32_dwmac *dwmac =plat_dat->bsp_priv;

    /* enable clocks */
    /* set phy mode */

    return stmmac_resume(ndev);
}

If it makes sense to have the enable clk/phy mode stuff in it's own
function that is fine too.


>> One other thing;
>> Do you need to have the PHY mode setup code in the init function which
>> is called each time on resume?
>
> I can't guarantee that after a suspend the sysconfig register will
> contain same data than before suspend.

I see.


regards,
Joachim Eastwood

[PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Joachim Eastwood]

On 23 February 2016 at 10:59, Alexandre Torgue
<alexandre.torgue@gmail.com> wrote:
> 2016-02-22 22:52 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
>> On 22 February 2016 at 15:50, Alexandre Torgue
>> <alexandre.torgue@gmail.com> wrote:
>>> 2016-02-13 14:48 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
>>>> On 3 February 2016 at 15:54, Alexandre TORGUE
>>>> <alexandre.torgue@gmail.com> wrote:
>>>>> +       plat_dat->bsp_priv = dwmac;
>>>>> +       plat_dat->init = stm32_dwmac_init;
>>>>> +       plat_dat->exit = stm32_dwmac_exit;
>>>>
>>>> Instead of using these callbacks could you rather implement the PM
>>>> callbacks directly in this driver?
>>>> I don't think it should add much code and it will make it look more
>>>> like standard driver. This will also give you some more control and
>>>> flexibility in your code.
>>>
>>> I prefer to keep the code as it is. Glue layer is directly linked to
>>> stmmac driver and I don't want to brake the link between the glue and
>>> the stmmac driver.
>>
>> What do you mean by break the link?
>>
>
> I thought that you wanted to split stmmac_pltfr_supend (glue part and
> stmamc part), but I well understood it is not the case (sorry for
> mistake).
>
>> There has been numerous of patch sets to make the stmmac "glue"
>> drivers into more standard platform drivers.
>> http://marc.info/?l=linux-netdev&m=143159850631093&w=2
>> http://marc.info/?l=linux-netdev&m=143708560009851&w=2
>> http://marc.info/?l=linux-netdev&m=143812136600541&w=2
>>
>> Do you see any advantage by using the init and exit hooks in your
>> driver instead of using the standard driver PM callbacks and remove
>> function?
>> The only "cost" I see is slightly more boilerplate code. But since you
>> already have init/exit functions you could easily make them into PM
>> resume/suspend so I doubt there would be much increase in code size.
>>
>
> If I well understood you want to continue the stmmac glue driver
> rework by moving stmmac_pltfr_suspend/resume/remove in each glue
> driver (stm32, sun, sti ....).

At least I want to avoid the init/exit callbacks for new drivers like
stm32-dwmac.


> Each glue driver will call directly stmmac_suspend/resume/remove and
> their own init/exit function.
> If it is what you meant, I can do it.

Yes, in your stm32 driver's suspend/resume/remove functions call
stmmac_suspend/stmmac_resume/stmmac_dvr_remove directly. Then you
shouldn't need to use the init/exit callbacks. Just put the need code
in the driver's suspend/resume/remove functions instead of init/exit
functions.

For example:
static int stm32_dwmac_resume(struct device *dev)
{
    struct net_device *ndev = dev_get_drvdata(dev);
    struct plat_stmmacenet_data *plat_dat = get_stmmac_plat_data(ndev)
    struct stm32_dwmac *dwmac =plat_dat->bsp_priv;

    /* enable clocks */
    /* set phy mode */

    return stmmac_resume(ndev);
}

If it makes sense to have the enable clk/phy mode stuff in it's own
function that is fine too.


>> One other thing;
>> Do you need to have the PHY mode setup code in the init function which
>> is called each time on resume?
>
> I can't guarantee that after a suspend the sysconfig register will
> contain same data than before suspend.

I see.


regards,
Joachim Eastwood

Re: [PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Alexandre Torgue]

2016-02-23 12:21 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
> On 23 February 2016 at 10:59, Alexandre Torgue
> <alexandre.torgue@gmail.com> wrote:
>> 2016-02-22 22:52 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
>>> On 22 February 2016 at 15:50, Alexandre Torgue
>>> <alexandre.torgue@gmail.com> wrote:
>>>> 2016-02-13 14:48 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
>>>>> On 3 February 2016 at 15:54, Alexandre TORGUE
>>>>> <alexandre.torgue@gmail.com> wrote:
>>>>>> +       plat_dat->bsp_priv = dwmac;
>>>>>> +       plat_dat->init = stm32_dwmac_init;
>>>>>> +       plat_dat->exit = stm32_dwmac_exit;
>>>>>
>>>>> Instead of using these callbacks could you rather implement the PM
>>>>> callbacks directly in this driver?
>>>>> I don't think it should add much code and it will make it look more
>>>>> like standard driver. This will also give you some more control and
>>>>> flexibility in your code.
>>>>
>>>> I prefer to keep the code as it is. Glue layer is directly linked to
>>>> stmmac driver and I don't want to brake the link between the glue and
>>>> the stmmac driver.
>>>
>>> What do you mean by break the link?
>>>
>>
>> I thought that you wanted to split stmmac_pltfr_supend (glue part and
>> stmamc part), but I well understood it is not the case (sorry for
>> mistake).
>>
>>> There has been numerous of patch sets to make the stmmac "glue"
>>> drivers into more standard platform drivers.
>>> http://marc.info/?l=linux-netdev&m=143159850631093&w=2
>>> http://marc.info/?l=linux-netdev&m=143708560009851&w=2
>>> http://marc.info/?l=linux-netdev&m=143812136600541&w=2
>>>
>>> Do you see any advantage by using the init and exit hooks in your
>>> driver instead of using the standard driver PM callbacks and remove
>>> function?
>>> The only "cost" I see is slightly more boilerplate code. But since you
>>> already have init/exit functions you could easily make them into PM
>>> resume/suspend so I doubt there would be much increase in code size.
>>>
>>
>> If I well understood you want to continue the stmmac glue driver
>> rework by moving stmmac_pltfr_suspend/resume/remove in each glue
>> driver (stm32, sun, sti ....).
>
> At least I want to avoid the init/exit callbacks for new drivers like
> stm32-dwmac.
>
>
>> Each glue driver will call directly stmmac_suspend/resume/remove and
>> their own init/exit function.
>> If it is what you meant, I can do it.
>
> Yes, in your stm32 driver's suspend/resume/remove functions call
> stmmac_suspend/stmmac_resume/stmmac_dvr_remove directly. Then you
> shouldn't need to use the init/exit callbacks. Just put the need code
> in the driver's suspend/resume/remove functions instead of init/exit
> functions.
>
> For example:
> static int stm32_dwmac_resume(struct device *dev)
> {
>     struct net_device *ndev = dev_get_drvdata(dev);
>     struct plat_stmmacenet_data *plat_dat = get_stmmac_plat_data(ndev)
>     struct stm32_dwmac *dwmac =plat_dat->bsp_priv;
>
>     /* enable clocks */
>     /* set phy mode */
>
>     return stmmac_resume(ndev);
> }
>
> If it makes sense to have the enable clk/phy mode stuff in it's own
> function that is fine too.

Ok. I will send v2 with this approach. After this series I could
provide a new series to change all glues with this approach ? (but I
will not able to test on each platform).

Best regards

Alexandre
>
>
>>> One other thing;
>>> Do you need to have the PHY mode setup code in the init function which
>>> is called each time on resume?
>>
>> I can't guarantee that after a suspend the sysconfig register will
>> contain same data than before suspend.
>
> I see.
>
>
> regards,
> Joachim Eastwood

Re: [PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Alexandre Torgue]

2016-02-23 12:21 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
> On 23 February 2016 at 10:59, Alexandre Torgue
> <alexandre.torgue@gmail.com> wrote:
>> 2016-02-22 22:52 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
>>> On 22 February 2016 at 15:50, Alexandre Torgue
>>> <alexandre.torgue@gmail.com> wrote:
>>>> 2016-02-13 14:48 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
>>>>> On 3 February 2016 at 15:54, Alexandre TORGUE
>>>>> <alexandre.torgue@gmail.com> wrote:
>>>>>> +       plat_dat->bsp_priv = dwmac;
>>>>>> +       plat_dat->init = stm32_dwmac_init;
>>>>>> +       plat_dat->exit = stm32_dwmac_exit;
>>>>>
>>>>> Instead of using these callbacks could you rather implement the PM
>>>>> callbacks directly in this driver?
>>>>> I don't think it should add much code and it will make it look more
>>>>> like standard driver. This will also give you some more control and
>>>>> flexibility in your code.
>>>>
>>>> I prefer to keep the code as it is. Glue layer is directly linked to
>>>> stmmac driver and I don't want to brake the link between the glue and
>>>> the stmmac driver.
>>>
>>> What do you mean by break the link?
>>>
>>
>> I thought that you wanted to split stmmac_pltfr_supend (glue part and
>> stmamc part), but I well understood it is not the case (sorry for
>> mistake).
>>
>>> There has been numerous of patch sets to make the stmmac "glue"
>>> drivers into more standard platform drivers.
>>> http://marc.info/?l=linux-netdev&m=143159850631093&w=2
>>> http://marc.info/?l=linux-netdev&m=143708560009851&w=2
>>> http://marc.info/?l=linux-netdev&m=143812136600541&w=2
>>>
>>> Do you see any advantage by using the init and exit hooks in your
>>> driver instead of using the standard driver PM callbacks and remove
>>> function?
>>> The only "cost" I see is slightly more boilerplate code. But since you
>>> already have init/exit functions you could easily make them into PM
>>> resume/suspend so I doubt there would be much increase in code size.
>>>
>>
>> If I well understood you want to continue the stmmac glue driver
>> rework by moving stmmac_pltfr_suspend/resume/remove in each glue
>> driver (stm32, sun, sti ....).
>
> At least I want to avoid the init/exit callbacks for new drivers like
> stm32-dwmac.
>
>
>> Each glue driver will call directly stmmac_suspend/resume/remove and
>> their own init/exit function.
>> If it is what you meant, I can do it.
>
> Yes, in your stm32 driver's suspend/resume/remove functions call
> stmmac_suspend/stmmac_resume/stmmac_dvr_remove directly. Then you
> shouldn't need to use the init/exit callbacks. Just put the need code
> in the driver's suspend/resume/remove functions instead of init/exit
> functions.
>
> For example:
> static int stm32_dwmac_resume(struct device *dev)
> {
>     struct net_device *ndev = dev_get_drvdata(dev);
>     struct plat_stmmacenet_data *plat_dat = get_stmmac_plat_data(ndev)
>     struct stm32_dwmac *dwmac =plat_dat->bsp_priv;
>
>     /* enable clocks */
>     /* set phy mode */
>
>     return stmmac_resume(ndev);
> }
>
> If it makes sense to have the enable clk/phy mode stuff in it's own
> function that is fine too.

Ok. I will send v2 with this approach. After this series I could
provide a new series to change all glues with this approach ? (but I
will not able to test on each platform).

Best regards

Alexandre
>
>
>>> One other thing;
>>> Do you need to have the PHY mode setup code in the init function which
>>> is called each time on resume?
>>
>> I can't guarantee that after a suspend the sysconfig register will
>> contain same data than before suspend.
>
> I see.
>
>
> regards,
> Joachim Eastwood

[PATCH 1/4] net: ethernet: dwmac: add Ethernet glue logic for stm32 chip

[Alexandre Torgue]

2016-02-23 12:21 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
> On 23 February 2016 at 10:59, Alexandre Torgue
> <alexandre.torgue@gmail.com> wrote:
>> 2016-02-22 22:52 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
>>> On 22 February 2016 at 15:50, Alexandre Torgue
>>> <alexandre.torgue@gmail.com> wrote:
>>>> 2016-02-13 14:48 GMT+01:00 Joachim  Eastwood <manabian@gmail.com>:
>>>>> On 3 February 2016 at 15:54, Alexandre TORGUE
>>>>> <alexandre.torgue@gmail.com> wrote:
>>>>>> +       plat_dat->bsp_priv = dwmac;
>>>>>> +       plat_dat->init = stm32_dwmac_init;
>>>>>> +       plat_dat->exit = stm32_dwmac_exit;
>>>>>
>>>>> Instead of using these callbacks could you rather implement the PM
>>>>> callbacks directly in this driver?
>>>>> I don't think it should add much code and it will make it look more
>>>>> like standard driver. This will also give you some more control and
>>>>> flexibility in your code.
>>>>
>>>> I prefer to keep the code as it is. Glue layer is directly linked to
>>>> stmmac driver and I don't want to brake the link between the glue and
>>>> the stmmac driver.
>>>
>>> What do you mean by break the link?
>>>
>>
>> I thought that you wanted to split stmmac_pltfr_supend (glue part and
>> stmamc part), but I well understood it is not the case (sorry for
>> mistake).
>>
>>> There has been numerous of patch sets to make the stmmac "glue"
>>> drivers into more standard platform drivers.
>>> http://marc.info/?l=linux-netdev&m=143159850631093&w=2
>>> http://marc.info/?l=linux-netdev&m=143708560009851&w=2
>>> http://marc.info/?l=linux-netdev&m=143812136600541&w=2
>>>
>>> Do you see any advantage by using the init and exit hooks in your
>>> driver instead of using the standard driver PM callbacks and remove
>>> function?
>>> The only "cost" I see is slightly more boilerplate code. But since you
>>> already have init/exit functions you could easily make them into PM
>>> resume/suspend so I doubt there would be much increase in code size.
>>>
>>
>> If I well understood you want to continue the stmmac glue driver
>> rework by moving stmmac_pltfr_suspend/resume/remove in each glue
>> driver (stm32, sun, sti ....).
>
> At least I want to avoid the init/exit callbacks for new drivers like
> stm32-dwmac.
>
>
>> Each glue driver will call directly stmmac_suspend/resume/remove and
>> their own init/exit function.
>> If it is what you meant, I can do it.
>
> Yes, in your stm32 driver's suspend/resume/remove functions call
> stmmac_suspend/stmmac_resume/stmmac_dvr_remove directly. Then you
> shouldn't need to use the init/exit callbacks. Just put the need code
> in the driver's suspend/resume/remove functions instead of init/exit
> functions.
>
> For example:
> static int stm32_dwmac_resume(struct device *dev)
> {
>     struct net_device *ndev = dev_get_drvdata(dev);
>     struct plat_stmmacenet_data *plat_dat = get_stmmac_plat_data(ndev)
>     struct stm32_dwmac *dwmac =plat_dat->bsp_priv;
>
>     /* enable clocks */
>     /* set phy mode */
>
>     return stmmac_resume(ndev);
> }
>
> If it makes sense to have the enable clk/phy mode stuff in it's own
> function that is fine too.

Ok. I will send v2 with this approach. After this series I could
provide a new series to change all glues with this approach ? (but I
will not able to test on each platform).

Best regards

Alexandre
>
>
>>> One other thing;
>>> Do you need to have the PHY mode setup code in the init function which
>>> is called each time on resume?
>>
>> I can't guarantee that after a suspend the sysconfig register will
>> contain same data than before suspend.
>
> I see.
>
>
> regards,
> Joachim Eastwood