All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer
@ 2022-01-26  8:44 Alexey Sheplyakov
  2022-01-26  8:44 ` [PATCH 2/2] dt-bindings: dwmac: Add bindings for Baikal-T1/M SoCs Alexey Sheplyakov
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Alexey Sheplyakov @ 2022-01-26  8:44 UTC (permalink / raw)
  To: netdev
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	Alexey Sheplyakov, Dmitry Dunaev

The gigabit Ethernet controller available in Baikal-T1 and Baikal-M
SoCs is a Synopsys DesignWare MAC IP core, already supported by
the stmmac driver.

This patch implements some SoC specific operations (DMA reset and
speed fixup) necessary for Baikal-T1/M variants.

Signed-off-by: Alexey Sheplyakov <asheplyakov@basealt.ru>
Signed-off-by: Dmitry Dunaev <dmitry.dunaev@baikalelectronics.ru>
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 +
 drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
 .../ethernet/stmicro/stmmac/dwmac-baikal.c    | 199 ++++++++++++++++++
 .../ethernet/stmicro/stmmac/dwmac1000_core.c  |   1 +
 .../ethernet/stmicro/stmmac/dwmac1000_dma.c   |  46 ++--
 .../ethernet/stmicro/stmmac/dwmac1000_dma.h   |  26 +++
 .../net/ethernet/stmicro/stmmac/dwmac_lib.c   |   8 +
 7 files changed, 274 insertions(+), 18 deletions(-)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.h

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 929cfc22cd0c..d8e6dcb98e6c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -66,6 +66,17 @@ config DWMAC_ANARION
 
 	  This selects the Anarion SoC glue layer support for the stmmac driver.
 
+config DWMAC_BAIKAL
+	tristate "Baikal Electronics GMAC support"
+	default MIPS_BAIKAL_T1
+	depends on OF && (MIPS || ARM64 || COMPILE_TEST)
+	help
+	  Support for gigabit ethernet controller on Baikal Electronics SoCs.
+
+	  This selects the Baikal Electronics SoCs glue layer support for
+	  the stmmac driver. This driver is used for Baikal-T1 and Baikal-M
+	  SoCs gigabit ethernet controller.
+
 config DWMAC_INGENIC
 	tristate "Ingenic MAC support"
 	default MACH_INGENIC
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index d4e12e9ace4f..ad138062e199 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -14,6 +14,7 @@ stmmac-$(CONFIG_STMMAC_SELFTESTS) += stmmac_selftests.o
 # Ordering matters. Generic driver must be last.
 obj-$(CONFIG_STMMAC_PLATFORM)	+= stmmac-platform.o
 obj-$(CONFIG_DWMAC_ANARION)	+= dwmac-anarion.o
+obj-$(CONFIG_DWMAC_BAIKAL)	+= dwmac-baikal.o
 obj-$(CONFIG_DWMAC_INGENIC)	+= dwmac-ingenic.o
 obj-$(CONFIG_DWMAC_IPQ806X)	+= dwmac-ipq806x.o
 obj-$(CONFIG_DWMAC_LPC18XX)	+= dwmac-lpc18xx.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c
new file mode 100644
index 000000000000..9133188a5d1b
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c
@@ -0,0 +1,199 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Baikal-T1/M SoCs DWMAC glue layer
+ *
+ * Copyright (C) 2015,2016,2021 Baikal Electronics JSC
+ * Copyright (C) 2020-2022 BaseALT Ltd
+ * Authors: Dmitry Dunaev <dmitry.dunaev@baikalelectronics.ru>
+ *          Alexey Sheplyakov <asheplyakov@basealt.ru>
+ */
+
+#include <linux/clk.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include "stmmac.h"
+#include "stmmac_platform.h"
+#include "common.h"
+#include "dwmac_dma.h"
+#include "dwmac1000_dma.h"
+
+#define MAC_GPIO	0x00e0	/* GPIO register */
+#define MAC_GPIO_GPO	BIT(8)	/* Output port */
+
+struct baikal_dwmac {
+	struct device	*dev;
+	struct clk	*tx2_clk;
+};
+
+static int baikal_dwmac_dma_reset(void __iomem *ioaddr)
+{
+	int err;
+	u32 value;
+
+	/* DMA SW reset */
+	value = readl(ioaddr + DMA_BUS_MODE);
+	value |= DMA_BUS_MODE_SFT_RESET;
+	writel(value, ioaddr + DMA_BUS_MODE);
+
+	usleep_range(100, 120);
+
+	/* Clear PHY reset */
+	value = readl(ioaddr + MAC_GPIO);
+	value |= MAC_GPIO_GPO;
+	writel(value, ioaddr + MAC_GPIO);
+
+	return readl_poll_timeout(ioaddr + DMA_BUS_MODE, value,
+				  !(value & DMA_BUS_MODE_SFT_RESET),
+				  10000, 1000000);
+}
+
+static const struct stmmac_dma_ops baikal_dwmac_dma_ops = {
+	.reset = baikal_dwmac_dma_reset,
+	.init = dwmac1000_dma_init,
+	.init_rx_chan = dwmac1000_dma_init_rx,
+	.init_tx_chan = dwmac1000_dma_init_tx,
+	.axi = dwmac1000_dma_axi,
+	.dump_regs = dwmac1000_dump_dma_regs,
+	.dma_rx_mode = dwmac1000_dma_operation_mode_rx,
+	.dma_tx_mode = dwmac1000_dma_operation_mode_tx,
+	.enable_dma_transmission = dwmac_enable_dma_transmission,
+	.enable_dma_irq = dwmac_enable_dma_irq,
+	.disable_dma_irq = dwmac_disable_dma_irq,
+	.start_tx = dwmac_dma_start_tx,
+	.stop_tx = dwmac_dma_stop_tx,
+	.start_rx = dwmac_dma_start_rx,
+	.stop_rx = dwmac_dma_stop_rx,
+	.dma_interrupt = dwmac_dma_interrupt,
+	.get_hw_feature = dwmac1000_get_hw_feature,
+	.rx_watchdog = dwmac1000_rx_watchdog
+};
+
+static struct mac_device_info *baikal_dwmac_setup(void *ppriv)
+{
+	struct mac_device_info *mac;
+	struct stmmac_priv *priv = ppriv;
+	int ret;
+	u32 value;
+
+	mac = devm_kzalloc(priv->device, sizeof(*mac), GFP_KERNEL);
+	if (!mac)
+		return NULL;
+
+	/* Clear PHY reset */
+	value = readl(priv->ioaddr + MAC_GPIO);
+	value |= MAC_GPIO_GPO;
+	writel(value, priv->ioaddr + MAC_GPIO);
+
+	mac->dma = &baikal_dwmac_dma_ops;
+	priv->hw = mac;
+	ret = dwmac1000_setup(priv);
+	if (ret) {
+		dev_err(priv->device, "dwmac1000_setup: error %d", ret);
+		return NULL;
+	}
+
+	return mac;
+}
+
+static void baikal_dwmac_fix_mac_speed(void *priv, unsigned int speed)
+{
+	struct baikal_dwmac *dwmac = priv;
+	unsigned long tx2_clk_freq;
+
+	switch (speed) {
+	case SPEED_1000:
+		tx2_clk_freq = 250000000;
+		break;
+	case SPEED_100:
+		tx2_clk_freq = 50000000;
+		break;
+	case SPEED_10:
+		tx2_clk_freq = 5000000;
+		break;
+	default:
+		dev_warn(dwmac->dev, "invalid speed: %u\n", speed);
+		return;
+	}
+	dev_dbg(dwmac->dev, "speed %u, setting TX2 clock frequency to %lu\n",
+		speed, tx2_clk_freq);
+	clk_set_rate(dwmac->tx2_clk, tx2_clk_freq);
+}
+
+static int dwmac_baikal_probe(struct platform_device *pdev)
+{
+	struct plat_stmmacenet_data *plat_dat;
+	struct stmmac_resources stmmac_res;
+	struct baikal_dwmac *dwmac;
+	int ret;
+
+	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
+	if (!dwmac)
+		return -ENOMEM;
+
+	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
+	if (ret)
+		return ret;
+
+	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+	if (ret) {
+		dev_err(&pdev->dev, "no suitable DMA available\n");
+		return ret;
+	}
+
+	plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
+	if (IS_ERR(plat_dat)) {
+		dev_err(&pdev->dev, "dt configuration failed\n");
+		return PTR_ERR(plat_dat);
+	}
+
+	dwmac->dev = &pdev->dev;
+	dwmac->tx2_clk = devm_clk_get_optional(dwmac->dev, "tx2_clk");
+	if (IS_ERR(dwmac->tx2_clk)) {
+		ret = PTR_ERR(dwmac->tx2_clk);
+		dev_err(&pdev->dev, "couldn't get TX2 clock: %d\n", ret);
+		goto err_remove_config_dt;
+	}
+
+	if (dwmac->tx2_clk)
+		plat_dat->fix_mac_speed = baikal_dwmac_fix_mac_speed;
+	plat_dat->bsp_priv = dwmac;
+	plat_dat->has_gmac = 1;
+	plat_dat->enh_desc = 1;
+	plat_dat->tx_coe = 1;
+	plat_dat->rx_coe = 1;
+	plat_dat->clk_csr = 3;
+	plat_dat->setup = baikal_dwmac_setup;
+
+	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	if (ret)
+		goto err_remove_config_dt;
+
+	return 0;
+
+err_remove_config_dt:
+	stmmac_remove_config_dt(pdev, plat_dat);
+	return ret;
+}
+
+static const struct of_device_id dwmac_baikal_match[] = {
+	{ .compatible = "baikal,dwmac" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, dwmac_baikal_match);
+
+static struct platform_driver dwmac_baikal_driver = {
+	.probe	= dwmac_baikal_probe,
+	.remove	= stmmac_pltfr_remove,
+	.driver	= {
+		.name = "baikal-dwmac",
+		.pm = &stmmac_pltfr_pm_ops,
+		.of_match_table = of_match_ptr(dwmac_baikal_match)
+	}
+};
+module_platform_driver(dwmac_baikal_driver);
+
+MODULE_DESCRIPTION("Baikal-T1/M DWMAC driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
index 76edb9b72675..7b8a955d98a9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -563,3 +563,4 @@ int dwmac1000_setup(struct stmmac_priv *priv)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(dwmac1000_setup);
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
index f5581db0ba9b..1782a65cc9af 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
@@ -15,8 +15,9 @@
 #include <asm/io.h>
 #include "dwmac1000.h"
 #include "dwmac_dma.h"
+#include "dwmac1000_dma.h"
 
-static void dwmac1000_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
+void dwmac1000_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
 {
 	u32 value = readl(ioaddr + DMA_AXI_BUS_MODE);
 	int i;
@@ -69,9 +70,10 @@ static void dwmac1000_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
 
 	writel(value, ioaddr + DMA_AXI_BUS_MODE);
 }
+EXPORT_SYMBOL_GPL(dwmac1000_dma_axi);
 
-static void dwmac1000_dma_init(void __iomem *ioaddr,
-			       struct stmmac_dma_cfg *dma_cfg, int atds)
+void dwmac1000_dma_init(void __iomem *ioaddr,
+			struct stmmac_dma_cfg *dma_cfg, int atds)
 {
 	u32 value = readl(ioaddr + DMA_BUS_MODE);
 	int txpbl = dma_cfg->txpbl ?: dma_cfg->pbl;
@@ -109,22 +111,25 @@ static void dwmac1000_dma_init(void __iomem *ioaddr,
 	/* Mask interrupts by writing to CSR7 */
 	writel(DMA_INTR_DEFAULT_MASK, ioaddr + DMA_INTR_ENA);
 }
+EXPORT_SYMBOL_GPL(dwmac1000_dma_init);
 
-static void dwmac1000_dma_init_rx(void __iomem *ioaddr,
-				  struct stmmac_dma_cfg *dma_cfg,
-				  dma_addr_t dma_rx_phy, u32 chan)
+void dwmac1000_dma_init_rx(void __iomem *ioaddr,
+			   struct stmmac_dma_cfg *dma_cfg,
+			   dma_addr_t dma_rx_phy, u32 chan)
 {
 	/* RX descriptor base address list must be written into DMA CSR3 */
 	writel(lower_32_bits(dma_rx_phy), ioaddr + DMA_RCV_BASE_ADDR);
 }
+EXPORT_SYMBOL_GPL(dwmac1000_dma_init_rx);
 
-static void dwmac1000_dma_init_tx(void __iomem *ioaddr,
-				  struct stmmac_dma_cfg *dma_cfg,
-				  dma_addr_t dma_tx_phy, u32 chan)
+void dwmac1000_dma_init_tx(void __iomem *ioaddr,
+			   struct stmmac_dma_cfg *dma_cfg,
+			   dma_addr_t dma_tx_phy, u32 chan)
 {
 	/* TX descriptor base address list must be written into DMA CSR4 */
 	writel(lower_32_bits(dma_tx_phy), ioaddr + DMA_TX_BASE_ADDR);
 }
+EXPORT_SYMBOL_GPL(dwmac1000_dma_init_tx);
 
 static u32 dwmac1000_configure_fc(u32 csr6, int rxfifosz)
 {
@@ -147,8 +152,8 @@ static u32 dwmac1000_configure_fc(u32 csr6, int rxfifosz)
 	return csr6;
 }
 
-static void dwmac1000_dma_operation_mode_rx(void __iomem *ioaddr, int mode,
-					    u32 channel, int fifosz, u8 qmode)
+void dwmac1000_dma_operation_mode_rx(void __iomem *ioaddr, int mode,
+				     u32 channel, int fifosz, u8 qmode)
 {
 	u32 csr6 = readl(ioaddr + DMA_CONTROL);
 
@@ -174,9 +179,10 @@ static void dwmac1000_dma_operation_mode_rx(void __iomem *ioaddr, int mode,
 
 	writel(csr6, ioaddr + DMA_CONTROL);
 }
+EXPORT_SYMBOL_GPL(dwmac1000_dma_operation_mode_rx);
 
-static void dwmac1000_dma_operation_mode_tx(void __iomem *ioaddr, int mode,
-					    u32 channel, int fifosz, u8 qmode)
+void dwmac1000_dma_operation_mode_tx(void __iomem *ioaddr, int mode,
+				     u32 channel, int fifosz, u8 qmode)
 {
 	u32 csr6 = readl(ioaddr + DMA_CONTROL);
 
@@ -207,8 +213,9 @@ static void dwmac1000_dma_operation_mode_tx(void __iomem *ioaddr, int mode,
 
 	writel(csr6, ioaddr + DMA_CONTROL);
 }
+EXPORT_SYMBOL_GPL(dwmac1000_dma_operation_mode_tx);
 
-static void dwmac1000_dump_dma_regs(void __iomem *ioaddr, u32 *reg_space)
+void dwmac1000_dump_dma_regs(void __iomem *ioaddr, u32 *reg_space)
 {
 	int i;
 
@@ -217,9 +224,10 @@ static void dwmac1000_dump_dma_regs(void __iomem *ioaddr, u32 *reg_space)
 			reg_space[DMA_BUS_MODE / 4 + i] =
 				readl(ioaddr + DMA_BUS_MODE + i * 4);
 }
+EXPORT_SYMBOL_GPL(dwmac1000_dump_dma_regs);
 
-static int dwmac1000_get_hw_feature(void __iomem *ioaddr,
-				    struct dma_features *dma_cap)
+int dwmac1000_get_hw_feature(void __iomem *ioaddr,
+			     struct dma_features *dma_cap)
 {
 	u32 hw_cap = readl(ioaddr + DMA_HW_FEATURE);
 
@@ -262,12 +270,14 @@ static int dwmac1000_get_hw_feature(void __iomem *ioaddr,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(dwmac1000_get_hw_feature);
 
-static void dwmac1000_rx_watchdog(void __iomem *ioaddr, u32 riwt,
-				  u32 queue)
+void dwmac1000_rx_watchdog(void __iomem *ioaddr, u32 riwt,
+			   u32 queue)
 {
 	writel(riwt, ioaddr + DMA_RX_WATCHDOG);
 }
+EXPORT_SYMBOL_GPL(dwmac1000_rx_watchdog);
 
 const struct stmmac_dma_ops dwmac1000_dma_ops = {
 	.reset = dwmac_dma_reset,
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.h b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.h
new file mode 100644
index 000000000000..b254a0734447
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __DWMAC1000_DMA_H__
+#define __DWMAC1000_DMA_H__
+#include "dwmac1000.h"
+
+void dwmac1000_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi);
+void dwmac1000_dma_init(void __iomem *ioaddr,
+			struct stmmac_dma_cfg *dma_cfg, int atds);
+void dwmac1000_dma_init_rx(void __iomem *ioaddr,
+			   struct stmmac_dma_cfg *dma_cfg,
+			   dma_addr_t dma_rx_phy, u32 chan);
+void dwmac1000_dma_init_tx(void __iomem *ioaddr,
+			   struct stmmac_dma_cfg *dma_cfg,
+			   dma_addr_t dma_tx_phy, u32 chan);
+void dwmac1000_dma_operation_mode_rx(void __iomem *ioaddr, int mode,
+				     u32 channel, int fifosz, u8 qmode);
+void dwmac1000_dma_operation_mode_tx(void __iomem *ioaddr, int mode,
+				     u32 channel, int fifosz, u8 qmode);
+void dwmac1000_dump_dma_regs(void __iomem *ioaddr, u32 *reg_space);
+
+int  dwmac1000_get_hw_feature(void __iomem *ioaddr,
+			      struct dma_features *dma_cap);
+
+void dwmac1000_rx_watchdog(void __iomem *ioaddr, u32 riwt, u32 number_chan);
+#endif /* __DWMAC1000_DMA_H__ */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
index caa4bfc4c1d6..2d8d1b0e2b98 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
@@ -31,6 +31,7 @@ void dwmac_enable_dma_transmission(void __iomem *ioaddr)
 {
 	writel(1, ioaddr + DMA_XMT_POLL_DEMAND);
 }
+EXPORT_SYMBOL_GPL(dwmac_enable_dma_transmission);
 
 void dwmac_enable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx)
 {
@@ -43,6 +44,7 @@ void dwmac_enable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx)
 
 	writel(value, ioaddr + DMA_INTR_ENA);
 }
+EXPORT_SYMBOL_GPL(dwmac_enable_dma_irq);
 
 void dwmac_disable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx)
 {
@@ -55,6 +57,7 @@ void dwmac_disable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx)
 
 	writel(value, ioaddr + DMA_INTR_ENA);
 }
+EXPORT_SYMBOL_GPL(dwmac_disable_dma_irq);
 
 void dwmac_dma_start_tx(void __iomem *ioaddr, u32 chan)
 {
@@ -62,6 +65,7 @@ void dwmac_dma_start_tx(void __iomem *ioaddr, u32 chan)
 	value |= DMA_CONTROL_ST;
 	writel(value, ioaddr + DMA_CONTROL);
 }
+EXPORT_SYMBOL_GPL(dwmac_dma_start_tx);
 
 void dwmac_dma_stop_tx(void __iomem *ioaddr, u32 chan)
 {
@@ -69,6 +73,7 @@ void dwmac_dma_stop_tx(void __iomem *ioaddr, u32 chan)
 	value &= ~DMA_CONTROL_ST;
 	writel(value, ioaddr + DMA_CONTROL);
 }
+EXPORT_SYMBOL_GPL(dwmac_dma_stop_tx);
 
 void dwmac_dma_start_rx(void __iomem *ioaddr, u32 chan)
 {
@@ -76,6 +81,7 @@ void dwmac_dma_start_rx(void __iomem *ioaddr, u32 chan)
 	value |= DMA_CONTROL_SR;
 	writel(value, ioaddr + DMA_CONTROL);
 }
+EXPORT_SYMBOL_GPL(dwmac_dma_start_rx);
 
 void dwmac_dma_stop_rx(void __iomem *ioaddr, u32 chan)
 {
@@ -83,6 +89,7 @@ void dwmac_dma_stop_rx(void __iomem *ioaddr, u32 chan)
 	value &= ~DMA_CONTROL_SR;
 	writel(value, ioaddr + DMA_CONTROL);
 }
+EXPORT_SYMBOL_GPL(dwmac_dma_stop_rx);
 
 #ifdef DWMAC_DMA_DEBUG
 static void show_tx_process_state(unsigned int status)
@@ -230,6 +237,7 @@ int dwmac_dma_interrupt(void __iomem *ioaddr,
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(dwmac_dma_interrupt);
 
 void dwmac_dma_flush_tx_fifo(void __iomem *ioaddr)
 {
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] dt-bindings: dwmac: Add bindings for Baikal-T1/M SoCs
  2022-01-26  8:44 [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer Alexey Sheplyakov
@ 2022-01-26  8:44 ` Alexey Sheplyakov
  2022-01-27  1:00 ` [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer Jakub Kicinski
  2022-01-28 15:06 ` Serge Semin
  2 siblings, 0 replies; 14+ messages in thread
From: Alexey Sheplyakov @ 2022-01-26  8:44 UTC (permalink / raw)
  To: netdev
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Alexey Sheplyakov

Added dwmac bindings for Baikal-T1 and Baikal-M SoCs

Signed-off-by: Alexey Sheplyakov <asheplyakov@basealt.ru>
---
 Documentation/devicetree/bindings/net/snps,dwmac.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 7eb43707e601..a738059f03ef 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -53,6 +53,7 @@ properties:
         - allwinner,sun8i-r40-gmac
         - allwinner,sun8i-v3s-emac
         - allwinner,sun50i-a64-emac
+        - baikal,dwmac
         - loongson,ls2k-dwmac
         - loongson,ls7a-dwmac
         - amlogic,meson6-dwmac
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer
  2022-01-26  8:44 [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer Alexey Sheplyakov
  2022-01-26  8:44 ` [PATCH 2/2] dt-bindings: dwmac: Add bindings for Baikal-T1/M SoCs Alexey Sheplyakov
@ 2022-01-27  1:00 ` Jakub Kicinski
  2022-01-28 12:16   ` [PATCH v2 " Alexey Sheplyakov
  2022-01-28 17:00   ` [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer Alexey Sheplyakov
  2022-01-28 15:06 ` Serge Semin
  2 siblings, 2 replies; 14+ messages in thread
From: Jakub Kicinski @ 2022-01-27  1:00 UTC (permalink / raw)
  To: Alexey Sheplyakov
  Cc: netdev, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Dmitry Dunaev

On Wed, 26 Jan 2022 12:44:55 +0400 Alexey Sheplyakov wrote:
> The gigabit Ethernet controller available in Baikal-T1 and Baikal-M
> SoCs is a Synopsys DesignWare MAC IP core, already supported by
> the stmmac driver.
> 
> This patch implements some SoC specific operations (DMA reset and
> speed fixup) necessary for Baikal-T1/M variants.

drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c:33:13: warning: unused variable ‘err’ [-Wunused-variable]
   33 |         int err;
      |             ^~~

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer
  2022-01-27  1:00 ` [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer Jakub Kicinski
@ 2022-01-28 12:16   ` Alexey Sheplyakov
  2022-01-28 12:16     ` [PATCH v2 2/2] dt-bindings: dwmac: Add bindings for Baikal-T1/M SoCs Alexey Sheplyakov
  2022-01-28 17:00   ` [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer Alexey Sheplyakov
  1 sibling, 1 reply; 14+ messages in thread
From: Alexey Sheplyakov @ 2022-01-28 12:16 UTC (permalink / raw)
  To: netdev
  Cc: Alexey Sheplyakov, Giuseppe Cavallaro, Alexandre Torgue,
	Jose Abreu, Jakub Kicinski

The gigabit Ethernet controller available in Baikal-T1 and Baikal-M
SoCs is a Synopsys DesignWare MAC IP core, already supported by
the stmmac driver.

This patch implements some SoC specific operations (DMA reset and
speed fixup) necessary for Baikal-T1/M variants.

Signed-off-by: Alexey Sheplyakov <asheplyakov@basealt.ru>
Signed-off-by: Dmitry Dunaev <dmitry.dunaev@baikalelectronics.ru>
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 +
 drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
 .../ethernet/stmicro/stmmac/dwmac-baikal.c    | 198 ++++++++++++++++++
 .../ethernet/stmicro/stmmac/dwmac1000_core.c  |   1 +
 .../ethernet/stmicro/stmmac/dwmac1000_dma.c   |  46 ++--
 .../ethernet/stmicro/stmmac/dwmac1000_dma.h   |  26 +++
 .../net/ethernet/stmicro/stmmac/dwmac_lib.c   |   8 +
 7 files changed, 273 insertions(+), 18 deletions(-)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.h

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 929cfc22cd0c..d8e6dcb98e6c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -66,6 +66,17 @@ config DWMAC_ANARION
 
 	  This selects the Anarion SoC glue layer support for the stmmac driver.
 
+config DWMAC_BAIKAL
+	tristate "Baikal Electronics GMAC support"
+	default MIPS_BAIKAL_T1
+	depends on OF && (MIPS || ARM64 || COMPILE_TEST)
+	help
+	  Support for gigabit ethernet controller on Baikal Electronics SoCs.
+
+	  This selects the Baikal Electronics SoCs glue layer support for
+	  the stmmac driver. This driver is used for Baikal-T1 and Baikal-M
+	  SoCs gigabit ethernet controller.
+
 config DWMAC_INGENIC
 	tristate "Ingenic MAC support"
 	default MACH_INGENIC
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index d4e12e9ace4f..ad138062e199 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -14,6 +14,7 @@ stmmac-$(CONFIG_STMMAC_SELFTESTS) += stmmac_selftests.o
 # Ordering matters. Generic driver must be last.
 obj-$(CONFIG_STMMAC_PLATFORM)	+= stmmac-platform.o
 obj-$(CONFIG_DWMAC_ANARION)	+= dwmac-anarion.o
+obj-$(CONFIG_DWMAC_BAIKAL)	+= dwmac-baikal.o
 obj-$(CONFIG_DWMAC_INGENIC)	+= dwmac-ingenic.o
 obj-$(CONFIG_DWMAC_IPQ806X)	+= dwmac-ipq806x.o
 obj-$(CONFIG_DWMAC_LPC18XX)	+= dwmac-lpc18xx.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c
new file mode 100644
index 000000000000..c5e6169eed65
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c
@@ -0,0 +1,198 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Baikal-T1/M SoCs DWMAC glue layer
+ *
+ * Copyright (C) 2015,2016,2021 Baikal Electronics JSC
+ * Copyright (C) 2020-2022 BaseALT Ltd
+ * Authors: Dmitry Dunaev <dmitry.dunaev@baikalelectronics.ru>
+ *          Alexey Sheplyakov <asheplyakov@basealt.ru>
+ */
+
+#include <linux/clk.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include "stmmac.h"
+#include "stmmac_platform.h"
+#include "common.h"
+#include "dwmac_dma.h"
+#include "dwmac1000_dma.h"
+
+#define MAC_GPIO	0x00e0	/* GPIO register */
+#define MAC_GPIO_GPO	BIT(8)	/* Output port */
+
+struct baikal_dwmac {
+	struct device	*dev;
+	struct clk	*tx2_clk;
+};
+
+static int baikal_dwmac_dma_reset(void __iomem *ioaddr)
+{
+	u32 value;
+
+	/* DMA SW reset */
+	value = readl(ioaddr + DMA_BUS_MODE);
+	value |= DMA_BUS_MODE_SFT_RESET;
+	writel(value, ioaddr + DMA_BUS_MODE);
+
+	usleep_range(100, 120);
+
+	/* Clear PHY reset */
+	value = readl(ioaddr + MAC_GPIO);
+	value |= MAC_GPIO_GPO;
+	writel(value, ioaddr + MAC_GPIO);
+
+	return readl_poll_timeout(ioaddr + DMA_BUS_MODE, value,
+				  !(value & DMA_BUS_MODE_SFT_RESET),
+				  10000, 1000000);
+}
+
+static const struct stmmac_dma_ops baikal_dwmac_dma_ops = {
+	.reset = baikal_dwmac_dma_reset,
+	.init = dwmac1000_dma_init,
+	.init_rx_chan = dwmac1000_dma_init_rx,
+	.init_tx_chan = dwmac1000_dma_init_tx,
+	.axi = dwmac1000_dma_axi,
+	.dump_regs = dwmac1000_dump_dma_regs,
+	.dma_rx_mode = dwmac1000_dma_operation_mode_rx,
+	.dma_tx_mode = dwmac1000_dma_operation_mode_tx,
+	.enable_dma_transmission = dwmac_enable_dma_transmission,
+	.enable_dma_irq = dwmac_enable_dma_irq,
+	.disable_dma_irq = dwmac_disable_dma_irq,
+	.start_tx = dwmac_dma_start_tx,
+	.stop_tx = dwmac_dma_stop_tx,
+	.start_rx = dwmac_dma_start_rx,
+	.stop_rx = dwmac_dma_stop_rx,
+	.dma_interrupt = dwmac_dma_interrupt,
+	.get_hw_feature = dwmac1000_get_hw_feature,
+	.rx_watchdog = dwmac1000_rx_watchdog
+};
+
+static struct mac_device_info *baikal_dwmac_setup(void *ppriv)
+{
+	struct mac_device_info *mac;
+	struct stmmac_priv *priv = ppriv;
+	int ret;
+	u32 value;
+
+	mac = devm_kzalloc(priv->device, sizeof(*mac), GFP_KERNEL);
+	if (!mac)
+		return NULL;
+
+	/* Clear PHY reset */
+	value = readl(priv->ioaddr + MAC_GPIO);
+	value |= MAC_GPIO_GPO;
+	writel(value, priv->ioaddr + MAC_GPIO);
+
+	mac->dma = &baikal_dwmac_dma_ops;
+	priv->hw = mac;
+	ret = dwmac1000_setup(priv);
+	if (ret) {
+		dev_err(priv->device, "dwmac1000_setup: error %d", ret);
+		return NULL;
+	}
+
+	return mac;
+}
+
+static void baikal_dwmac_fix_mac_speed(void *priv, unsigned int speed)
+{
+	struct baikal_dwmac *dwmac = priv;
+	unsigned long tx2_clk_freq;
+
+	switch (speed) {
+	case SPEED_1000:
+		tx2_clk_freq = 250000000;
+		break;
+	case SPEED_100:
+		tx2_clk_freq = 50000000;
+		break;
+	case SPEED_10:
+		tx2_clk_freq = 5000000;
+		break;
+	default:
+		dev_warn(dwmac->dev, "invalid speed: %u\n", speed);
+		return;
+	}
+	dev_dbg(dwmac->dev, "speed %u, setting TX2 clock frequency to %lu\n",
+		speed, tx2_clk_freq);
+	clk_set_rate(dwmac->tx2_clk, tx2_clk_freq);
+}
+
+static int dwmac_baikal_probe(struct platform_device *pdev)
+{
+	struct plat_stmmacenet_data *plat_dat;
+	struct stmmac_resources stmmac_res;
+	struct baikal_dwmac *dwmac;
+	int ret;
+
+	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
+	if (!dwmac)
+		return -ENOMEM;
+
+	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
+	if (ret)
+		return ret;
+
+	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+	if (ret) {
+		dev_err(&pdev->dev, "no suitable DMA available\n");
+		return ret;
+	}
+
+	plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
+	if (IS_ERR(plat_dat)) {
+		dev_err(&pdev->dev, "dt configuration failed\n");
+		return PTR_ERR(plat_dat);
+	}
+
+	dwmac->dev = &pdev->dev;
+	dwmac->tx2_clk = devm_clk_get_optional(dwmac->dev, "tx2_clk");
+	if (IS_ERR(dwmac->tx2_clk)) {
+		ret = PTR_ERR(dwmac->tx2_clk);
+		dev_err(&pdev->dev, "couldn't get TX2 clock: %d\n", ret);
+		goto err_remove_config_dt;
+	}
+
+	if (dwmac->tx2_clk)
+		plat_dat->fix_mac_speed = baikal_dwmac_fix_mac_speed;
+	plat_dat->bsp_priv = dwmac;
+	plat_dat->has_gmac = 1;
+	plat_dat->enh_desc = 1;
+	plat_dat->tx_coe = 1;
+	plat_dat->rx_coe = 1;
+	plat_dat->clk_csr = 3;
+	plat_dat->setup = baikal_dwmac_setup;
+
+	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	if (ret)
+		goto err_remove_config_dt;
+
+	return 0;
+
+err_remove_config_dt:
+	stmmac_remove_config_dt(pdev, plat_dat);
+	return ret;
+}
+
+static const struct of_device_id dwmac_baikal_match[] = {
+	{ .compatible = "baikal,dwmac" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, dwmac_baikal_match);
+
+static struct platform_driver dwmac_baikal_driver = {
+	.probe	= dwmac_baikal_probe,
+	.remove	= stmmac_pltfr_remove,
+	.driver	= {
+		.name = "baikal-dwmac",
+		.pm = &stmmac_pltfr_pm_ops,
+		.of_match_table = of_match_ptr(dwmac_baikal_match)
+	}
+};
+module_platform_driver(dwmac_baikal_driver);
+
+MODULE_DESCRIPTION("Baikal-T1/M DWMAC driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
index 76edb9b72675..7b8a955d98a9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -563,3 +563,4 @@ int dwmac1000_setup(struct stmmac_priv *priv)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(dwmac1000_setup);
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
index f5581db0ba9b..1782a65cc9af 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
@@ -15,8 +15,9 @@
 #include <asm/io.h>
 #include "dwmac1000.h"
 #include "dwmac_dma.h"
+#include "dwmac1000_dma.h"
 
-static void dwmac1000_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
+void dwmac1000_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
 {
 	u32 value = readl(ioaddr + DMA_AXI_BUS_MODE);
 	int i;
@@ -69,9 +70,10 @@ static void dwmac1000_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
 
 	writel(value, ioaddr + DMA_AXI_BUS_MODE);
 }
+EXPORT_SYMBOL_GPL(dwmac1000_dma_axi);
 
-static void dwmac1000_dma_init(void __iomem *ioaddr,
-			       struct stmmac_dma_cfg *dma_cfg, int atds)
+void dwmac1000_dma_init(void __iomem *ioaddr,
+			struct stmmac_dma_cfg *dma_cfg, int atds)
 {
 	u32 value = readl(ioaddr + DMA_BUS_MODE);
 	int txpbl = dma_cfg->txpbl ?: dma_cfg->pbl;
@@ -109,22 +111,25 @@ static void dwmac1000_dma_init(void __iomem *ioaddr,
 	/* Mask interrupts by writing to CSR7 */
 	writel(DMA_INTR_DEFAULT_MASK, ioaddr + DMA_INTR_ENA);
 }
+EXPORT_SYMBOL_GPL(dwmac1000_dma_init);
 
-static void dwmac1000_dma_init_rx(void __iomem *ioaddr,
-				  struct stmmac_dma_cfg *dma_cfg,
-				  dma_addr_t dma_rx_phy, u32 chan)
+void dwmac1000_dma_init_rx(void __iomem *ioaddr,
+			   struct stmmac_dma_cfg *dma_cfg,
+			   dma_addr_t dma_rx_phy, u32 chan)
 {
 	/* RX descriptor base address list must be written into DMA CSR3 */
 	writel(lower_32_bits(dma_rx_phy), ioaddr + DMA_RCV_BASE_ADDR);
 }
+EXPORT_SYMBOL_GPL(dwmac1000_dma_init_rx);
 
-static void dwmac1000_dma_init_tx(void __iomem *ioaddr,
-				  struct stmmac_dma_cfg *dma_cfg,
-				  dma_addr_t dma_tx_phy, u32 chan)
+void dwmac1000_dma_init_tx(void __iomem *ioaddr,
+			   struct stmmac_dma_cfg *dma_cfg,
+			   dma_addr_t dma_tx_phy, u32 chan)
 {
 	/* TX descriptor base address list must be written into DMA CSR4 */
 	writel(lower_32_bits(dma_tx_phy), ioaddr + DMA_TX_BASE_ADDR);
 }
+EXPORT_SYMBOL_GPL(dwmac1000_dma_init_tx);
 
 static u32 dwmac1000_configure_fc(u32 csr6, int rxfifosz)
 {
@@ -147,8 +152,8 @@ static u32 dwmac1000_configure_fc(u32 csr6, int rxfifosz)
 	return csr6;
 }
 
-static void dwmac1000_dma_operation_mode_rx(void __iomem *ioaddr, int mode,
-					    u32 channel, int fifosz, u8 qmode)
+void dwmac1000_dma_operation_mode_rx(void __iomem *ioaddr, int mode,
+				     u32 channel, int fifosz, u8 qmode)
 {
 	u32 csr6 = readl(ioaddr + DMA_CONTROL);
 
@@ -174,9 +179,10 @@ static void dwmac1000_dma_operation_mode_rx(void __iomem *ioaddr, int mode,
 
 	writel(csr6, ioaddr + DMA_CONTROL);
 }
+EXPORT_SYMBOL_GPL(dwmac1000_dma_operation_mode_rx);
 
-static void dwmac1000_dma_operation_mode_tx(void __iomem *ioaddr, int mode,
-					    u32 channel, int fifosz, u8 qmode)
+void dwmac1000_dma_operation_mode_tx(void __iomem *ioaddr, int mode,
+				     u32 channel, int fifosz, u8 qmode)
 {
 	u32 csr6 = readl(ioaddr + DMA_CONTROL);
 
@@ -207,8 +213,9 @@ static void dwmac1000_dma_operation_mode_tx(void __iomem *ioaddr, int mode,
 
 	writel(csr6, ioaddr + DMA_CONTROL);
 }
+EXPORT_SYMBOL_GPL(dwmac1000_dma_operation_mode_tx);
 
-static void dwmac1000_dump_dma_regs(void __iomem *ioaddr, u32 *reg_space)
+void dwmac1000_dump_dma_regs(void __iomem *ioaddr, u32 *reg_space)
 {
 	int i;
 
@@ -217,9 +224,10 @@ static void dwmac1000_dump_dma_regs(void __iomem *ioaddr, u32 *reg_space)
 			reg_space[DMA_BUS_MODE / 4 + i] =
 				readl(ioaddr + DMA_BUS_MODE + i * 4);
 }
+EXPORT_SYMBOL_GPL(dwmac1000_dump_dma_regs);
 
-static int dwmac1000_get_hw_feature(void __iomem *ioaddr,
-				    struct dma_features *dma_cap)
+int dwmac1000_get_hw_feature(void __iomem *ioaddr,
+			     struct dma_features *dma_cap)
 {
 	u32 hw_cap = readl(ioaddr + DMA_HW_FEATURE);
 
@@ -262,12 +270,14 @@ static int dwmac1000_get_hw_feature(void __iomem *ioaddr,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(dwmac1000_get_hw_feature);
 
-static void dwmac1000_rx_watchdog(void __iomem *ioaddr, u32 riwt,
-				  u32 queue)
+void dwmac1000_rx_watchdog(void __iomem *ioaddr, u32 riwt,
+			   u32 queue)
 {
 	writel(riwt, ioaddr + DMA_RX_WATCHDOG);
 }
+EXPORT_SYMBOL_GPL(dwmac1000_rx_watchdog);
 
 const struct stmmac_dma_ops dwmac1000_dma_ops = {
 	.reset = dwmac_dma_reset,
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.h b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.h
new file mode 100644
index 000000000000..b254a0734447
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __DWMAC1000_DMA_H__
+#define __DWMAC1000_DMA_H__
+#include "dwmac1000.h"
+
+void dwmac1000_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi);
+void dwmac1000_dma_init(void __iomem *ioaddr,
+			struct stmmac_dma_cfg *dma_cfg, int atds);
+void dwmac1000_dma_init_rx(void __iomem *ioaddr,
+			   struct stmmac_dma_cfg *dma_cfg,
+			   dma_addr_t dma_rx_phy, u32 chan);
+void dwmac1000_dma_init_tx(void __iomem *ioaddr,
+			   struct stmmac_dma_cfg *dma_cfg,
+			   dma_addr_t dma_tx_phy, u32 chan);
+void dwmac1000_dma_operation_mode_rx(void __iomem *ioaddr, int mode,
+				     u32 channel, int fifosz, u8 qmode);
+void dwmac1000_dma_operation_mode_tx(void __iomem *ioaddr, int mode,
+				     u32 channel, int fifosz, u8 qmode);
+void dwmac1000_dump_dma_regs(void __iomem *ioaddr, u32 *reg_space);
+
+int  dwmac1000_get_hw_feature(void __iomem *ioaddr,
+			      struct dma_features *dma_cap);
+
+void dwmac1000_rx_watchdog(void __iomem *ioaddr, u32 riwt, u32 number_chan);
+#endif /* __DWMAC1000_DMA_H__ */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
index caa4bfc4c1d6..2d8d1b0e2b98 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
@@ -31,6 +31,7 @@ void dwmac_enable_dma_transmission(void __iomem *ioaddr)
 {
 	writel(1, ioaddr + DMA_XMT_POLL_DEMAND);
 }
+EXPORT_SYMBOL_GPL(dwmac_enable_dma_transmission);
 
 void dwmac_enable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx)
 {
@@ -43,6 +44,7 @@ void dwmac_enable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx)
 
 	writel(value, ioaddr + DMA_INTR_ENA);
 }
+EXPORT_SYMBOL_GPL(dwmac_enable_dma_irq);
 
 void dwmac_disable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx)
 {
@@ -55,6 +57,7 @@ void dwmac_disable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx)
 
 	writel(value, ioaddr + DMA_INTR_ENA);
 }
+EXPORT_SYMBOL_GPL(dwmac_disable_dma_irq);
 
 void dwmac_dma_start_tx(void __iomem *ioaddr, u32 chan)
 {
@@ -62,6 +65,7 @@ void dwmac_dma_start_tx(void __iomem *ioaddr, u32 chan)
 	value |= DMA_CONTROL_ST;
 	writel(value, ioaddr + DMA_CONTROL);
 }
+EXPORT_SYMBOL_GPL(dwmac_dma_start_tx);
 
 void dwmac_dma_stop_tx(void __iomem *ioaddr, u32 chan)
 {
@@ -69,6 +73,7 @@ void dwmac_dma_stop_tx(void __iomem *ioaddr, u32 chan)
 	value &= ~DMA_CONTROL_ST;
 	writel(value, ioaddr + DMA_CONTROL);
 }
+EXPORT_SYMBOL_GPL(dwmac_dma_stop_tx);
 
 void dwmac_dma_start_rx(void __iomem *ioaddr, u32 chan)
 {
@@ -76,6 +81,7 @@ void dwmac_dma_start_rx(void __iomem *ioaddr, u32 chan)
 	value |= DMA_CONTROL_SR;
 	writel(value, ioaddr + DMA_CONTROL);
 }
+EXPORT_SYMBOL_GPL(dwmac_dma_start_rx);
 
 void dwmac_dma_stop_rx(void __iomem *ioaddr, u32 chan)
 {
@@ -83,6 +89,7 @@ void dwmac_dma_stop_rx(void __iomem *ioaddr, u32 chan)
 	value &= ~DMA_CONTROL_SR;
 	writel(value, ioaddr + DMA_CONTROL);
 }
+EXPORT_SYMBOL_GPL(dwmac_dma_stop_rx);
 
 #ifdef DWMAC_DMA_DEBUG
 static void show_tx_process_state(unsigned int status)
@@ -230,6 +237,7 @@ int dwmac_dma_interrupt(void __iomem *ioaddr,
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(dwmac_dma_interrupt);
 
 void dwmac_dma_flush_tx_fifo(void __iomem *ioaddr)
 {
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/2] dt-bindings: dwmac: Add bindings for Baikal-T1/M SoCs
  2022-01-28 12:16   ` [PATCH v2 " Alexey Sheplyakov
@ 2022-01-28 12:16     ` Alexey Sheplyakov
  0 siblings, 0 replies; 14+ messages in thread
From: Alexey Sheplyakov @ 2022-01-28 12:16 UTC (permalink / raw)
  To: netdev
  Cc: Alexey Sheplyakov, Giuseppe Cavallaro, Alexandre Torgue,
	Jose Abreu, Jakub Kicinski

Added dwmac bindings for Baikal-T1 and Baikal-M SoCs

Signed-off-by: Alexey Sheplyakov <asheplyakov@basealt.ru>
---
 Documentation/devicetree/bindings/net/snps,dwmac.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 7eb43707e601..a738059f03ef 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -53,6 +53,7 @@ properties:
         - allwinner,sun8i-r40-gmac
         - allwinner,sun8i-v3s-emac
         - allwinner,sun50i-a64-emac
+        - baikal,dwmac
         - loongson,ls2k-dwmac
         - loongson,ls7a-dwmac
         - amlogic,meson6-dwmac
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer
  2022-01-26  8:44 [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer Alexey Sheplyakov
  2022-01-26  8:44 ` [PATCH 2/2] dt-bindings: dwmac: Add bindings for Baikal-T1/M SoCs Alexey Sheplyakov
  2022-01-27  1:00 ` [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer Jakub Kicinski
@ 2022-01-28 15:06 ` Serge Semin
  2022-01-28 18:55   ` Alexey Sheplyakov
  2022-02-03 10:49   ` Alexey Sheplyakov
  2 siblings, 2 replies; 14+ messages in thread
From: Serge Semin @ 2022-01-28 15:06 UTC (permalink / raw)
  To: Alexey Sheplyakov
  Cc: netdev, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	Dmitry Dunaev, Alexey Malahov, Pavel Parkhomenko, Serge Semin

Hello Alexey and network folks

First of all thanks for sharing this patchset with the community. The
changes indeed provide a limited support for the DW GMAC embedded into
the Baikal-T1/M1 SoCs. But the problem is that they don't cover all
the IP-blocks/Platform-setup peculiarities (believe me there are more
than just 2*Tx-clock and embedded GPIO features), moreover giving a
false impression of a full and stable Baikal-T1/M1 GMAC interface
support. There are good reasons why we haven't submitted the GMAC/xGBE
drivers so far. I've been working on the STMMAC code refactoring for
more than six months now so the driver would be better structured and
would support all of the required features including the DW XGMAC
interface embedded into the SoCs. So please don't rush with this
patchset including into the kernel. We are going to submit a more
comprehensive and thoroughly structured series of patchsets including
a bunch of STMMAC driver Fixes very soon. After that everyone will be
happy ;)

Also, Alexey, next time you submit something Baikal-related could you
please Cc someone from our team? (I am sure you know Alexey' email or
have seen my patches in the mailing lists.) Dmitry Dunaev hasn't been
working for Baikal Electronics for more than four years now so his
email address is disabled (you must have already noticed that by
getting a bounce back email). Moreover you can't add someone'
signed-off tag without getting a permission from one. In addition note
the original driver author was Dmitry, even though you have indeed
provided some useful modifications to the code.
          
My comments regarding the most problematic parts of this patch are
below.

On Wed, Jan 26, 2022 at 12:44:55PM +0400, Alexey Sheplyakov wrote:
> The gigabit Ethernet controller available in Baikal-T1 and Baikal-M
> SoCs is a Synopsys DesignWare MAC IP core, already supported by
> the stmmac driver.
> 
> This patch implements some SoC specific operations (DMA reset and
> speed fixup) necessary for Baikal-T1/M variants.
> 
> Signed-off-by: Alexey Sheplyakov <asheplyakov@basealt.ru>
> Signed-off-by: Dmitry Dunaev <dmitry.dunaev@baikalelectronics.ru>
> ---
>  drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 +
>  drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
>  .../ethernet/stmicro/stmmac/dwmac-baikal.c    | 199 ++++++++++++++++++
>  .../ethernet/stmicro/stmmac/dwmac1000_core.c  |   1 +
>  .../ethernet/stmicro/stmmac/dwmac1000_dma.c   |  46 ++--
>  .../ethernet/stmicro/stmmac/dwmac1000_dma.h   |  26 +++
>  .../net/ethernet/stmicro/stmmac/dwmac_lib.c   |   8 +
>  7 files changed, 274 insertions(+), 18 deletions(-)
>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c
>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.h
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index 929cfc22cd0c..d8e6dcb98e6c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -66,6 +66,17 @@ config DWMAC_ANARION
>  
>  	  This selects the Anarion SoC glue layer support for the stmmac driver.
>  
> +config DWMAC_BAIKAL
> +	tristate "Baikal Electronics GMAC support"
> +	default MIPS_BAIKAL_T1
> +	depends on OF && (MIPS || ARM64 || COMPILE_TEST)
> +	help
> +	  Support for gigabit ethernet controller on Baikal Electronics SoCs.
> +
> +	  This selects the Baikal Electronics SoCs glue layer support for
> +	  the stmmac driver. This driver is used for Baikal-T1 and Baikal-M
> +	  SoCs gigabit ethernet controller.
> +
>  config DWMAC_INGENIC
>  	tristate "Ingenic MAC support"
>  	default MACH_INGENIC
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> index d4e12e9ace4f..ad138062e199 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> @@ -14,6 +14,7 @@ stmmac-$(CONFIG_STMMAC_SELFTESTS) += stmmac_selftests.o
>  # Ordering matters. Generic driver must be last.
>  obj-$(CONFIG_STMMAC_PLATFORM)	+= stmmac-platform.o
>  obj-$(CONFIG_DWMAC_ANARION)	+= dwmac-anarion.o
> +obj-$(CONFIG_DWMAC_BAIKAL)	+= dwmac-baikal.o
>  obj-$(CONFIG_DWMAC_INGENIC)	+= dwmac-ingenic.o
>  obj-$(CONFIG_DWMAC_IPQ806X)	+= dwmac-ipq806x.o
>  obj-$(CONFIG_DWMAC_LPC18XX)	+= dwmac-lpc18xx.o
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c
> new file mode 100644
> index 000000000000..9133188a5d1b
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c
> @@ -0,0 +1,199 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Baikal-T1/M SoCs DWMAC glue layer
> + *
> + * Copyright (C) 2015,2016,2021 Baikal Electronics JSC
> + * Copyright (C) 2020-2022 BaseALT Ltd
> + * Authors: Dmitry Dunaev <dmitry.dunaev@baikalelectronics.ru>
> + *          Alexey Sheplyakov <asheplyakov@basealt.ru>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#include "stmmac.h"
> +#include "stmmac_platform.h"
> +#include "common.h"
> +#include "dwmac_dma.h"
> +#include "dwmac1000_dma.h"
> +
> +#define MAC_GPIO	0x00e0	/* GPIO register */
> +#define MAC_GPIO_GPO	BIT(8)	/* Output port */
> +
> +struct baikal_dwmac {
> +	struct device	*dev;
> +	struct clk	*tx2_clk;
> +};
> +

> +static int baikal_dwmac_dma_reset(void __iomem *ioaddr)
> +{
> +	int err;
> +	u32 value;
> +
> +	/* DMA SW reset */
> +	value = readl(ioaddr + DMA_BUS_MODE);
> +	value |= DMA_BUS_MODE_SFT_RESET;
> +	writel(value, ioaddr + DMA_BUS_MODE);
> +
> +	usleep_range(100, 120);
> +
> +	/* Clear PHY reset */
> +	value = readl(ioaddr + MAC_GPIO);
> +	value |= MAC_GPIO_GPO;
> +	writel(value, ioaddr + MAC_GPIO);
> +
> +	return readl_poll_timeout(ioaddr + DMA_BUS_MODE, value,
> +				  !(value & DMA_BUS_MODE_SFT_RESET),
> +				  10000, 1000000);
> +}
> +
> +static const struct stmmac_dma_ops baikal_dwmac_dma_ops = {
> +	.reset = baikal_dwmac_dma_reset,

First of all this modification is redundant for the platforms not
using the GMAC GPOs as resets, and is harmful if these signals are
used for something not related with the GMAC interface. Secondly this
callback won't properly work for all PHY types (though is acceptable
for some simple PHYs, which don't require much initialization or have
suitable default setups). The problem is in the way the MAC + PHY
initialization procedure is designed and in the way the embedded GPIOs
are used in the platform. Even if we assume that all DW GMAC/xGBE
GPIs/GPOs are used in conjunction with the corresponding GMAC
interface (it's wrong in general), the interface open procedure upon
return will still leave the PHY uninitialized or initialized with default
values. That happens due to the PHY initialization being performed
before the MAC initialization in the STMMAC open callback. Since the
later implies calling the DW GMAC soft-reset, the former turns to be
pointless due to the soft-reset causing the GPO toggle and consequent
PHY reset.

So to speak in order to cover all the GPI/GPO usage scenario and in
order to fix the problems described above the STMMAC core needs to be
also properly modified, which isn't that easy due to the way the
driver has evolved to.

> +	.init = dwmac1000_dma_init,
> +	.init_rx_chan = dwmac1000_dma_init_rx,
> +	.init_tx_chan = dwmac1000_dma_init_tx,
> +	.axi = dwmac1000_dma_axi,
> +	.dump_regs = dwmac1000_dump_dma_regs,
> +	.dma_rx_mode = dwmac1000_dma_operation_mode_rx,
> +	.dma_tx_mode = dwmac1000_dma_operation_mode_tx,
> +	.enable_dma_transmission = dwmac_enable_dma_transmission,
> +	.enable_dma_irq = dwmac_enable_dma_irq,
> +	.disable_dma_irq = dwmac_disable_dma_irq,
> +	.start_tx = dwmac_dma_start_tx,
> +	.stop_tx = dwmac_dma_stop_tx,
> +	.start_rx = dwmac_dma_start_rx,
> +	.stop_rx = dwmac_dma_stop_rx,
> +	.dma_interrupt = dwmac_dma_interrupt,
> +	.get_hw_feature = dwmac1000_get_hw_feature,
> +	.rx_watchdog = dwmac1000_rx_watchdog
> +};
> +
> +static struct mac_device_info *baikal_dwmac_setup(void *ppriv)
> +{
> +	struct mac_device_info *mac;
> +	struct stmmac_priv *priv = ppriv;
> +	int ret;
> +	u32 value;
> +
> +	mac = devm_kzalloc(priv->device, sizeof(*mac), GFP_KERNEL);
> +	if (!mac)
> +		return NULL;
> +
> +	/* Clear PHY reset */
> +	value = readl(priv->ioaddr + MAC_GPIO);
> +	value |= MAC_GPIO_GPO;
> +	writel(value, priv->ioaddr + MAC_GPIO);
> +
> +	mac->dma = &baikal_dwmac_dma_ops;
> +	priv->hw = mac;
> +	ret = dwmac1000_setup(priv);
> +	if (ret) {
> +		dev_err(priv->device, "dwmac1000_setup: error %d", ret);
> +		return NULL;
> +	}
> +
> +	return mac;
> +}
> +
> +static void baikal_dwmac_fix_mac_speed(void *priv, unsigned int speed)
> +{
> +	struct baikal_dwmac *dwmac = priv;
> +	unsigned long tx2_clk_freq;
> +
> +	switch (speed) {
> +	case SPEED_1000:
> +		tx2_clk_freq = 250000000;
> +		break;
> +	case SPEED_100:
> +		tx2_clk_freq = 50000000;
> +		break;
> +	case SPEED_10:
> +		tx2_clk_freq = 5000000;
> +		break;
> +	default:
> +		dev_warn(dwmac->dev, "invalid speed: %u\n", speed);
> +		return;
> +	}
> +	dev_dbg(dwmac->dev, "speed %u, setting TX2 clock frequency to %lu\n",
> +		speed, tx2_clk_freq);
> +	clk_set_rate(dwmac->tx2_clk, tx2_clk_freq);
> +}
> +
> +static int dwmac_baikal_probe(struct platform_device *pdev)
> +{
> +	struct plat_stmmacenet_data *plat_dat;
> +	struct stmmac_resources stmmac_res;
> +	struct baikal_dwmac *dwmac;
> +	int ret;
> +
> +	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
> +	if (!dwmac)
> +		return -ENOMEM;
> +
> +	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
> +	if (ret)
> +		return ret;
> +
> +	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> +	if (ret) {
> +		dev_err(&pdev->dev, "no suitable DMA available\n");
> +		return ret;
> +	}
> +
> +	plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
> +	if (IS_ERR(plat_dat)) {
> +		dev_err(&pdev->dev, "dt configuration failed\n");
> +		return PTR_ERR(plat_dat);
> +	}
> +
> +	dwmac->dev = &pdev->dev;

> +	dwmac->tx2_clk = devm_clk_get_optional(dwmac->dev, "tx2_clk");
> +	if (IS_ERR(dwmac->tx2_clk)) {
> +		ret = PTR_ERR(dwmac->tx2_clk);
> +		dev_err(&pdev->dev, "couldn't get TX2 clock: %d\n", ret);
> +		goto err_remove_config_dt;
> +	}

The bindings are much more comprehensive than just a single Tx-clock.
You are missing them here and in your DT-bindings patch. Please also
note you can't make the DT-resources name up without providing a
corresponding bindings schema update.

> +
> +	if (dwmac->tx2_clk)
> +		plat_dat->fix_mac_speed = baikal_dwmac_fix_mac_speed;
> +	plat_dat->bsp_priv = dwmac;
> +	plat_dat->has_gmac = 1;
> +	plat_dat->enh_desc = 1;
> +	plat_dat->tx_coe = 1;
> +	plat_dat->rx_coe = 1;

> +	plat_dat->clk_csr = 3;

Instead of fixing the stmmac_clk_csr_set() method you have provided
the clk_csr workaround. What if pclk rate is changed? Which BTW is
possible. =) In that case you'll get a wrong MDC rate.

> +	plat_dat->setup = baikal_dwmac_setup;
> +
> +	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> +	if (ret)
> +		goto err_remove_config_dt;
> +
> +	return 0;
> +
> +err_remove_config_dt:
> +	stmmac_remove_config_dt(pdev, plat_dat);
> +	return ret;
> +}
> +
> +static const struct of_device_id dwmac_baikal_match[] = {

> +	{ .compatible = "baikal,dwmac" },

Even though Baikal-T1 and Baikal-M1 have been synthesized with almost
identical IP-cores I wouldn't suggest to use the same compatible
string for both of them. At least those are different platforms with
different reference signals parameters. So it would be much better to
use the naming like "baikal,bt1-gmac" and "baikal,bm1-gmac" here.

> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, dwmac_baikal_match);
> +
> +static struct platform_driver dwmac_baikal_driver = {
> +	.probe	= dwmac_baikal_probe,
> +	.remove	= stmmac_pltfr_remove,
> +	.driver	= {
> +		.name = "baikal-dwmac",
> +		.pm = &stmmac_pltfr_pm_ops,
> +		.of_match_table = of_match_ptr(dwmac_baikal_match)
> +	}
> +};
> +module_platform_driver(dwmac_baikal_driver);
> +
> +MODULE_DESCRIPTION("Baikal-T1/M DWMAC driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> index 76edb9b72675..7b8a955d98a9 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> @@ -563,3 +563,4 @@ int dwmac1000_setup(struct stmmac_priv *priv)
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(dwmac1000_setup);

As I said providing a platform-specific reset method won't solve the
problem with the PHYs resetting on each interface up/down procedures.
So exporting this method and the methods below will be just useless
since the provided fix isn't complete.

One more time I'd strongly recommend to postpone the Baikal-T1/M1 GMAC
support adding to the mainline kernel until we are done with the
required STMMAC core driver preparations. There are much more problems
in there than the ones denoted above. Our team has been working on
this for the last six months and soon we'll be ready to share the
outcomes.

Regards
-Sergey

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
> index f5581db0ba9b..1782a65cc9af 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
> @@ -15,8 +15,9 @@
>  #include <asm/io.h>
>  #include "dwmac1000.h"
>  #include "dwmac_dma.h"
> +#include "dwmac1000_dma.h"
>  
> -static void dwmac1000_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
> +void dwmac1000_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
>  {
>  	u32 value = readl(ioaddr + DMA_AXI_BUS_MODE);
>  	int i;
> @@ -69,9 +70,10 @@ static void dwmac1000_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
>  
>  	writel(value, ioaddr + DMA_AXI_BUS_MODE);
>  }
> +EXPORT_SYMBOL_GPL(dwmac1000_dma_axi);
>  
> -static void dwmac1000_dma_init(void __iomem *ioaddr,
> -			       struct stmmac_dma_cfg *dma_cfg, int atds)
> +void dwmac1000_dma_init(void __iomem *ioaddr,
> +			struct stmmac_dma_cfg *dma_cfg, int atds)
>  {
>  	u32 value = readl(ioaddr + DMA_BUS_MODE);
>  	int txpbl = dma_cfg->txpbl ?: dma_cfg->pbl;
> @@ -109,22 +111,25 @@ static void dwmac1000_dma_init(void __iomem *ioaddr,
>  	/* Mask interrupts by writing to CSR7 */
>  	writel(DMA_INTR_DEFAULT_MASK, ioaddr + DMA_INTR_ENA);
>  }
> +EXPORT_SYMBOL_GPL(dwmac1000_dma_init);
>  
> -static void dwmac1000_dma_init_rx(void __iomem *ioaddr,
> -				  struct stmmac_dma_cfg *dma_cfg,
> -				  dma_addr_t dma_rx_phy, u32 chan)
> +void dwmac1000_dma_init_rx(void __iomem *ioaddr,
> +			   struct stmmac_dma_cfg *dma_cfg,
> +			   dma_addr_t dma_rx_phy, u32 chan)
>  {
>  	/* RX descriptor base address list must be written into DMA CSR3 */
>  	writel(lower_32_bits(dma_rx_phy), ioaddr + DMA_RCV_BASE_ADDR);
>  }
> +EXPORT_SYMBOL_GPL(dwmac1000_dma_init_rx);
>  
> -static void dwmac1000_dma_init_tx(void __iomem *ioaddr,
> -				  struct stmmac_dma_cfg *dma_cfg,
> -				  dma_addr_t dma_tx_phy, u32 chan)
> +void dwmac1000_dma_init_tx(void __iomem *ioaddr,
> +			   struct stmmac_dma_cfg *dma_cfg,
> +			   dma_addr_t dma_tx_phy, u32 chan)
>  {
>  	/* TX descriptor base address list must be written into DMA CSR4 */
>  	writel(lower_32_bits(dma_tx_phy), ioaddr + DMA_TX_BASE_ADDR);
>  }
> +EXPORT_SYMBOL_GPL(dwmac1000_dma_init_tx);
>  
>  static u32 dwmac1000_configure_fc(u32 csr6, int rxfifosz)
>  {
> @@ -147,8 +152,8 @@ static u32 dwmac1000_configure_fc(u32 csr6, int rxfifosz)
>  	return csr6;
>  }
>  
> -static void dwmac1000_dma_operation_mode_rx(void __iomem *ioaddr, int mode,
> -					    u32 channel, int fifosz, u8 qmode)
> +void dwmac1000_dma_operation_mode_rx(void __iomem *ioaddr, int mode,
> +				     u32 channel, int fifosz, u8 qmode)
>  {
>  	u32 csr6 = readl(ioaddr + DMA_CONTROL);
>  
> @@ -174,9 +179,10 @@ static void dwmac1000_dma_operation_mode_rx(void __iomem *ioaddr, int mode,
>  
>  	writel(csr6, ioaddr + DMA_CONTROL);
>  }
> +EXPORT_SYMBOL_GPL(dwmac1000_dma_operation_mode_rx);
>  
> -static void dwmac1000_dma_operation_mode_tx(void __iomem *ioaddr, int mode,
> -					    u32 channel, int fifosz, u8 qmode)
> +void dwmac1000_dma_operation_mode_tx(void __iomem *ioaddr, int mode,
> +				     u32 channel, int fifosz, u8 qmode)
>  {
>  	u32 csr6 = readl(ioaddr + DMA_CONTROL);
>  
> @@ -207,8 +213,9 @@ static void dwmac1000_dma_operation_mode_tx(void __iomem *ioaddr, int mode,
>  
>  	writel(csr6, ioaddr + DMA_CONTROL);
>  }
> +EXPORT_SYMBOL_GPL(dwmac1000_dma_operation_mode_tx);
>  
> -static void dwmac1000_dump_dma_regs(void __iomem *ioaddr, u32 *reg_space)
> +void dwmac1000_dump_dma_regs(void __iomem *ioaddr, u32 *reg_space)
>  {
>  	int i;
>  
> @@ -217,9 +224,10 @@ static void dwmac1000_dump_dma_regs(void __iomem *ioaddr, u32 *reg_space)
>  			reg_space[DMA_BUS_MODE / 4 + i] =
>  				readl(ioaddr + DMA_BUS_MODE + i * 4);
>  }
> +EXPORT_SYMBOL_GPL(dwmac1000_dump_dma_regs);
>  
> -static int dwmac1000_get_hw_feature(void __iomem *ioaddr,
> -				    struct dma_features *dma_cap)
> +int dwmac1000_get_hw_feature(void __iomem *ioaddr,
> +			     struct dma_features *dma_cap)
>  {
>  	u32 hw_cap = readl(ioaddr + DMA_HW_FEATURE);
>  
> @@ -262,12 +270,14 @@ static int dwmac1000_get_hw_feature(void __iomem *ioaddr,
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(dwmac1000_get_hw_feature);
>  
> -static void dwmac1000_rx_watchdog(void __iomem *ioaddr, u32 riwt,
> -				  u32 queue)
> +void dwmac1000_rx_watchdog(void __iomem *ioaddr, u32 riwt,
> +			   u32 queue)
>  {
>  	writel(riwt, ioaddr + DMA_RX_WATCHDOG);
>  }
> +EXPORT_SYMBOL_GPL(dwmac1000_rx_watchdog);
>  
>  const struct stmmac_dma_ops dwmac1000_dma_ops = {
>  	.reset = dwmac_dma_reset,
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.h b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.h
> new file mode 100644
> index 000000000000..b254a0734447
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __DWMAC1000_DMA_H__
> +#define __DWMAC1000_DMA_H__
> +#include "dwmac1000.h"
> +
> +void dwmac1000_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi);
> +void dwmac1000_dma_init(void __iomem *ioaddr,
> +			struct stmmac_dma_cfg *dma_cfg, int atds);
> +void dwmac1000_dma_init_rx(void __iomem *ioaddr,
> +			   struct stmmac_dma_cfg *dma_cfg,
> +			   dma_addr_t dma_rx_phy, u32 chan);
> +void dwmac1000_dma_init_tx(void __iomem *ioaddr,
> +			   struct stmmac_dma_cfg *dma_cfg,
> +			   dma_addr_t dma_tx_phy, u32 chan);
> +void dwmac1000_dma_operation_mode_rx(void __iomem *ioaddr, int mode,
> +				     u32 channel, int fifosz, u8 qmode);
> +void dwmac1000_dma_operation_mode_tx(void __iomem *ioaddr, int mode,
> +				     u32 channel, int fifosz, u8 qmode);
> +void dwmac1000_dump_dma_regs(void __iomem *ioaddr, u32 *reg_space);
> +
> +int  dwmac1000_get_hw_feature(void __iomem *ioaddr,
> +			      struct dma_features *dma_cap);
> +
> +void dwmac1000_rx_watchdog(void __iomem *ioaddr, u32 riwt, u32 number_chan);
> +#endif /* __DWMAC1000_DMA_H__ */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
> index caa4bfc4c1d6..2d8d1b0e2b98 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
> @@ -31,6 +31,7 @@ void dwmac_enable_dma_transmission(void __iomem *ioaddr)
>  {
>  	writel(1, ioaddr + DMA_XMT_POLL_DEMAND);
>  }
> +EXPORT_SYMBOL_GPL(dwmac_enable_dma_transmission);
>  
>  void dwmac_enable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx)
>  {
> @@ -43,6 +44,7 @@ void dwmac_enable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx)
>  
>  	writel(value, ioaddr + DMA_INTR_ENA);
>  }
> +EXPORT_SYMBOL_GPL(dwmac_enable_dma_irq);
>  
>  void dwmac_disable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx)
>  {
> @@ -55,6 +57,7 @@ void dwmac_disable_dma_irq(void __iomem *ioaddr, u32 chan, bool rx, bool tx)
>  
>  	writel(value, ioaddr + DMA_INTR_ENA);
>  }
> +EXPORT_SYMBOL_GPL(dwmac_disable_dma_irq);
>  
>  void dwmac_dma_start_tx(void __iomem *ioaddr, u32 chan)
>  {
> @@ -62,6 +65,7 @@ void dwmac_dma_start_tx(void __iomem *ioaddr, u32 chan)
>  	value |= DMA_CONTROL_ST;
>  	writel(value, ioaddr + DMA_CONTROL);
>  }
> +EXPORT_SYMBOL_GPL(dwmac_dma_start_tx);
>  
>  void dwmac_dma_stop_tx(void __iomem *ioaddr, u32 chan)
>  {
> @@ -69,6 +73,7 @@ void dwmac_dma_stop_tx(void __iomem *ioaddr, u32 chan)
>  	value &= ~DMA_CONTROL_ST;
>  	writel(value, ioaddr + DMA_CONTROL);
>  }
> +EXPORT_SYMBOL_GPL(dwmac_dma_stop_tx);
>  
>  void dwmac_dma_start_rx(void __iomem *ioaddr, u32 chan)
>  {
> @@ -76,6 +81,7 @@ void dwmac_dma_start_rx(void __iomem *ioaddr, u32 chan)
>  	value |= DMA_CONTROL_SR;
>  	writel(value, ioaddr + DMA_CONTROL);
>  }
> +EXPORT_SYMBOL_GPL(dwmac_dma_start_rx);
>  
>  void dwmac_dma_stop_rx(void __iomem *ioaddr, u32 chan)
>  {
> @@ -83,6 +89,7 @@ void dwmac_dma_stop_rx(void __iomem *ioaddr, u32 chan)
>  	value &= ~DMA_CONTROL_SR;
>  	writel(value, ioaddr + DMA_CONTROL);
>  }
> +EXPORT_SYMBOL_GPL(dwmac_dma_stop_rx);
>  
>  #ifdef DWMAC_DMA_DEBUG
>  static void show_tx_process_state(unsigned int status)
> @@ -230,6 +237,7 @@ int dwmac_dma_interrupt(void __iomem *ioaddr,
>  
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(dwmac_dma_interrupt);
>  
>  void dwmac_dma_flush_tx_fifo(void __iomem *ioaddr)
>  {
> -- 
> 2.32.0
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer
  2022-01-27  1:00 ` [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer Jakub Kicinski
  2022-01-28 12:16   ` [PATCH v2 " Alexey Sheplyakov
@ 2022-01-28 17:00   ` Alexey Sheplyakov
  1 sibling, 0 replies; 14+ messages in thread
From: Alexey Sheplyakov @ 2022-01-28 17:00 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Dmitry Dunaev

Hi, Jakub,

On Wed, Jan 26, 2022 at 05:00:42PM -0800, Jakub Kicinski wrote:
> On Wed, 26 Jan 2022 12:44:55 +0400 Alexey Sheplyakov wrote:
> > The gigabit Ethernet controller available in Baikal-T1 and Baikal-M
> > SoCs is a Synopsys DesignWare MAC IP core, already supported by
> > the stmmac driver.
> > 
> > This patch implements some SoC specific operations (DMA reset and
> > speed fixup) necessary for Baikal-T1/M variants.
> 
> drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c:33:13: warning: unused variable ‘err’ [-Wunused-variable]
>    33 |         int err;
>       |             ^~~

thanks for spotting this. I've sent v2 patchset which appears to compile
with -Werror.

Best regards,
	Alexey

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer
  2022-01-28 15:06 ` Serge Semin
@ 2022-01-28 18:55   ` Alexey Sheplyakov
  2022-01-28 20:27     ` Jakub Kicinski
  2022-02-03 10:49   ` Alexey Sheplyakov
  1 sibling, 1 reply; 14+ messages in thread
From: Alexey Sheplyakov @ 2022-01-28 18:55 UTC (permalink / raw)
  To: Serge Semin
  Cc: netdev, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	Alexey Malahov, Pavel Parkhomenko, Serge Semin,
	Evgeny Sinelnikov

Hi, Serge,

On Fri, Jan 28, 2022 at 06:06:42PM +0300, Serge Semin wrote:
> Hello Alexey and network folks
> 
> First of all thanks for sharing this patchset with the community. The
> changes indeed provide a limited support for the DW GMAC embedded into
> the Baikal-T1/M1 SoCs. But the problem is that they don't cover all
> the IP-blocks/Platform-setup peculiarities

In general quite a number of Linux drivers (GPUs, WiFi chips, foreign
filesystems, you name it) provide a limited support for the corresponding
hardware (filesystem, protocol, etc) and don't cover all peculiarities.
Yet having such a limited support in the mainline kernel is much more
useful than no support at all (or having to use out-of-tree drivers,
obosolete vendor kernels, binary blobs, etc).

Therefore "does not cover all peculiarities" does not sound like a valid
reason for rejecting this driver. That said it's definitely up to stmmac
maintainers to decide if the code meets the quality standards, does not
cause excessive maintanence burden, etc.

> (believe me there are more
> than just 2*Tx-clock and embedded GPIO features), moreover giving a
> false impression of a full and stable Baikal-T1/M1 GMAC interface
> support.

Such an impression is not intended. Perhaps the commit message should
be improved. What about this:

8<---

net: stmmac: initial support of Baikal-T1/M SoCs GMAC

The gigabit Ethernet controller available in Baikal-T1 and Baikal-M
SoCs is a Synopsys DesignWare MAC IP core, already supported by
the stmmac driver.

This patch implements some SoC specific operations (DMA reset and
speed fixup) necessary (but in general not enough) for Baikal-T1/M
variants.

Note that this driver does NOT cover all the IP-blocks/Platform-setup
peculiarities. It's known to work just fine on some Baikal-T1 boards
(including BFK3.1 reference board) and some Baikal-M based boards
(TF307 revision D, LGP-16), however it might or might not work with
other boards.

8<---

> There are good reasons why we haven't submitted the GMAC/xGBE
> drivers so far. I've been working on the STMMAC code refactoring for
> more than six months now so the driver would be better structured and
> would support all of the required features including the DW XGMAC
> interface embedded into the SoCs. So please don't rush with this
> patchset including into the kernel. We are going to submit a more
> comprehensive and thoroughly structured series of patchsets including
> a bunch of STMMAC driver Fixes very soon. After that everyone will be
> happy ;)

Don't get me wrong, but I've heard the very same thing back in 2020.
It's 2022 now. So I decided it's time to mainline this primitive driver
(which is definitely far from perfect) so people can use the mainline
kernel on (some of) their Baikal-M/T1 based boards.

And this simple driver can be easily removed/replaced if/when a more
advanced version is ready.

> Also, Alexey, next time you submit something Baikal-related could you
> please Cc someone from our team?

Sure. Hopefully I'll get some useful feedback (that is, other than
"don't bother, use the kernel from SDK", or "we are working on it,
please wait").

> (I am sure you know Alexey' email or
> have seen my patches in the mailing lists.) Dmitry Dunaev hasn't been
> working for Baikal Electronics for more than four years now so his
> email address is disabled (you must have already noticed that by
> getting a bounce back email). Moreover you can't add someone'
> signed-off tag without getting a permission from one.

Yep. Hence the question: what is the proper way to mention that the code
I post is based on work of other people (if those people ignore my emails,
or or their addresses are not valid any more, etc)?

Best regards,
	Alexey


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer
  2022-01-28 18:55   ` Alexey Sheplyakov
@ 2022-01-28 20:27     ` Jakub Kicinski
  2022-02-01 15:54       ` Serge Semin
  0 siblings, 1 reply; 14+ messages in thread
From: Jakub Kicinski @ 2022-01-28 20:27 UTC (permalink / raw)
  To: Serge Semin
  Cc: Alexey Sheplyakov, netdev, Giuseppe Cavallaro, Alexandre Torgue,
	Jose Abreu, Alexey Malahov, Pavel Parkhomenko, Serge Semin,
	Evgeny Sinelnikov

On Fri, 28 Jan 2022 22:55:09 +0400 Alexey Sheplyakov wrote:
> On Fri, Jan 28, 2022 at 06:06:42PM +0300, Serge Semin wrote:
> > Hello Alexey and network folks
> > 
> > First of all thanks for sharing this patchset with the community. The
> > changes indeed provide a limited support for the DW GMAC embedded into
> > the Baikal-T1/M1 SoCs. But the problem is that they don't cover all
> > the IP-blocks/Platform-setup peculiarities  
> 
> In general quite a number of Linux drivers (GPUs, WiFi chips, foreign
> filesystems, you name it) provide a limited support for the corresponding
> hardware (filesystem, protocol, etc) and don't cover all peculiarities.
> Yet having such a limited support in the mainline kernel is much more
> useful than no support at all (or having to use out-of-tree drivers,
> obosolete vendor kernels, binary blobs, etc).
> 
> Therefore "does not cover all peculiarities" does not sound like a valid
> reason for rejecting this driver. That said it's definitely up to stmmac
> maintainers to decide if the code meets the quality standards, does not
> cause excessive maintanence burden, etc.

Sounds sensible, Serge please take a look at the v2 and let us know if
there are any bugs in there. Or any differences in DT bindings or user
visible behaviors with what you're planning to do. If the driver is
functional and useful it can evolve and gain support for features and
platforms over time.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer
  2022-01-28 20:27     ` Jakub Kicinski
@ 2022-02-01 15:54       ` Serge Semin
  2022-02-01 19:08         ` Jakub Kicinski
  0 siblings, 1 reply; 14+ messages in thread
From: Serge Semin @ 2022-02-01 15:54 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Alexey Sheplyakov, netdev, Giuseppe Cavallaro, Alexandre Torgue,
	Jose Abreu, Alexey Malahov, Pavel Parkhomenko, Evgeny Sinelnikov,
	Serge Semin

Hello Jakub,

On Fri, Jan 28, 2022 at 12:27:18PM -0800, Jakub Kicinski wrote:
> On Fri, 28 Jan 2022 22:55:09 +0400 Alexey Sheplyakov wrote:
> > On Fri, Jan 28, 2022 at 06:06:42PM +0300, Serge Semin wrote:
> > > Hello Alexey and network folks
> > > 
> > > First of all thanks for sharing this patchset with the community. The
> > > changes indeed provide a limited support for the DW GMAC embedded into
> > > the Baikal-T1/M1 SoCs. But the problem is that they don't cover all
> > > the IP-blocks/Platform-setup peculiarities  
> > 
> > In general quite a number of Linux drivers (GPUs, WiFi chips, foreign
> > filesystems, you name it) provide a limited support for the corresponding
> > hardware (filesystem, protocol, etc) and don't cover all peculiarities.
> > Yet having such a limited support in the mainline kernel is much more
> > useful than no support at all (or having to use out-of-tree drivers,
> > obosolete vendor kernels, binary blobs, etc).
> > 
> > Therefore "does not cover all peculiarities" does not sound like a valid
> > reason for rejecting this driver. That said it's definitely up to stmmac
> > maintainers to decide if the code meets the quality standards, does not
> > cause excessive maintanence burden, etc.
> 
> Sounds sensible, Serge please take a look at the v2 and let us know if
> there are any bugs in there. Or any differences in DT bindings or user
> visible behaviors with what you're planning to do. If the driver is
> functional and useful it can evolve and gain support for features and
> platforms over time.

I've already posted my comments in this thread regarding the main
problematic issues of the driver, but Alexey for some reason ignored
them (dropped from his reply). Do you want me to copy my comments to
v2 and to proceed with review there?

Regarding the DT-bindings and the user-visible behavior. Right, I'll
add my comments in this matter. Thanks for suggesting. This was one of
the problems why I was against the driver integrating into the kernel.
One of our patchset brings a better organization to the current
DT-bindings of the Synopsys DW *MAC devices. In particular it splits
up the generic bindings for the vendor-specific MACs to use and the
bindings for the pure DW MAC compatible devices. In addition the
patchset will add the generic Tx/Rx clocks DT-bindings and the
DT-bindings for the AXI/MTL nodes. All of that and the rest of our
work will be posted a bit later as a set of the incremental patchsets
with small changes, one by one, for an easier review. We just need
some more time to finish the left of the work. The reason why the
already developed patches hasn't been delivered yet is that the rest
of the work may cause adding changes into the previous patches. In
order to decrease a number of the patches to review and present a
complete work for the community, we decided to post the patchsets
after the work is fully done.

-Sergey

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer
  2022-02-01 15:54       ` Serge Semin
@ 2022-02-01 19:08         ` Jakub Kicinski
  2022-02-07 15:22           ` Serge Semin
  0 siblings, 1 reply; 14+ messages in thread
From: Jakub Kicinski @ 2022-02-01 19:08 UTC (permalink / raw)
  To: Serge Semin
  Cc: Alexey Sheplyakov, netdev, Giuseppe Cavallaro, Alexandre Torgue,
	Jose Abreu, Alexey Malahov, Pavel Parkhomenko, Evgeny Sinelnikov,
	Serge Semin

On Tue, 1 Feb 2022 18:54:39 +0300 Serge Semin wrote:
> On Fri, Jan 28, 2022 at 12:27:18PM -0800, Jakub Kicinski wrote:
> > On Fri, 28 Jan 2022 22:55:09 +0400 Alexey Sheplyakov wrote:  
> > > In general quite a number of Linux drivers (GPUs, WiFi chips, foreign
> > > filesystems, you name it) provide a limited support for the corresponding
> > > hardware (filesystem, protocol, etc) and don't cover all peculiarities.
> > > Yet having such a limited support in the mainline kernel is much more
> > > useful than no support at all (or having to use out-of-tree drivers,
> > > obosolete vendor kernels, binary blobs, etc).
> > > 
> > > Therefore "does not cover all peculiarities" does not sound like a valid
> > > reason for rejecting this driver. That said it's definitely up to stmmac
> > > maintainers to decide if the code meets the quality standards, does not
> > > cause excessive maintanence burden, etc.  
> > 
> > Sounds sensible, Serge please take a look at the v2 and let us know if
> > there are any bugs in there. Or any differences in DT bindings or user
> > visible behaviors with what you're planning to do. If the driver is
> > functional and useful it can evolve and gain support for features and
> > platforms over time.  
> 
> I've already posted my comments in this thread regarding the main
> problematic issues of the driver, but Alexey for some reason ignored
> them (dropped from his reply). Do you want me to copy my comments to
> v2 and to proceed with review there?

Right, on a closer look there are indeed comments you raised that were
not addressed and not constrained to future compatibility. 

Alexey, please take another look at those and provide a changelog in
your next posting so we can easily check what was addressed.

> Regarding the DT-bindings and the user-visible behavior. Right, I'll
> add my comments in this matter. Thanks for suggesting. This was one of
> the problems why I was against the driver integrating into the kernel.
> One of our patchset brings a better organization to the current
> DT-bindings of the Synopsys DW *MAC devices. In particular it splits
> up the generic bindings for the vendor-specific MACs to use and the
> bindings for the pure DW MAC compatible devices. In addition the
> patchset will add the generic Tx/Rx clocks DT-bindings and the
> DT-bindings for the AXI/MTL nodes. All of that and the rest of our
> work will be posted a bit later as a set of the incremental patchsets
> with small changes, one by one, for an easier review. We just need
> some more time to finish the left of the work. The reason why the
> already developed patches hasn't been delivered yet is that the rest
> of the work may cause adding changes into the previous patches. In
> order to decrease a number of the patches to review and present a
> complete work for the community, we decided to post the patchsets
> after the work is fully done.

TBH starting to post stuff is probably best choice you can make,
for example the DT rework you mention sounds like a refactoring 
you can perform without posting any Baikal support.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer
  2022-01-28 15:06 ` Serge Semin
  2022-01-28 18:55   ` Alexey Sheplyakov
@ 2022-02-03 10:49   ` Alexey Sheplyakov
  2022-02-07 17:48     ` Serge Semin
  1 sibling, 1 reply; 14+ messages in thread
From: Alexey Sheplyakov @ 2022-02-03 10:49 UTC (permalink / raw)
  To: Serge Semin
  Cc: netdev, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	Dmitry Dunaev, Alexey Malahov, Pavel Parkhomenko, Serge Semin

Hello,

On Fri, Jan 28, 2022 at 06:06:42PM +0300, Serge Semin wrote:
 
> My comments regarding the most problematic parts of this patch are
> below.
> 
> On Wed, Jan 26, 2022 at 12:44:55PM +0400, Alexey Sheplyakov wrote:
> > The gigabit Ethernet controller available in Baikal-T1 and Baikal-M
> > SoCs is a Synopsys DesignWare MAC IP core, already supported by
> > the stmmac driver.
> > 
> > This patch implements some SoC specific operations (DMA reset and
> > speed fixup) necessary for Baikal-T1/M variants.
> > 
> > Signed-off-by: Alexey Sheplyakov <asheplyakov@basealt.ru>
> > Signed-off-by: Dmitry Dunaev <dmitry.dunaev@baikalelectronics.ru>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 +
> >  drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
> >  .../ethernet/stmicro/stmmac/dwmac-baikal.c    | 199 ++++++++++++++++++
> >  .../ethernet/stmicro/stmmac/dwmac1000_core.c  |   1 +
> >  .../ethernet/stmicro/stmmac/dwmac1000_dma.c   |  46 ++--
> >  .../ethernet/stmicro/stmmac/dwmac1000_dma.h   |  26 +++
> >  .../net/ethernet/stmicro/stmmac/dwmac_lib.c   |   8 +
> >  7 files changed, 274 insertions(+), 18 deletions(-)
> >  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c
> >  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.h
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > index 929cfc22cd0c..d8e6dcb98e6c 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > @@ -66,6 +66,17 @@ config DWMAC_ANARION
> >  
> >  	  This selects the Anarion SoC glue layer support for the stmmac driver.
> >  
> > +config DWMAC_BAIKAL
> > +	tristate "Baikal Electronics GMAC support"
> > +	default MIPS_BAIKAL_T1
> > +	depends on OF && (MIPS || ARM64 || COMPILE_TEST)
> > +	help
> > +	  Support for gigabit ethernet controller on Baikal Electronics SoCs.
> > +
> > +	  This selects the Baikal Electronics SoCs glue layer support for
> > +	  the stmmac driver. This driver is used for Baikal-T1 and Baikal-M
> > +	  SoCs gigabit ethernet controller.
> > +
> >  config DWMAC_INGENIC
> >  	tristate "Ingenic MAC support"
> >  	default MACH_INGENIC
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> > index d4e12e9ace4f..ad138062e199 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> > @@ -14,6 +14,7 @@ stmmac-$(CONFIG_STMMAC_SELFTESTS) += stmmac_selftests.o
> >  # Ordering matters. Generic driver must be last.
> >  obj-$(CONFIG_STMMAC_PLATFORM)	+= stmmac-platform.o
> >  obj-$(CONFIG_DWMAC_ANARION)	+= dwmac-anarion.o
> > +obj-$(CONFIG_DWMAC_BAIKAL)	+= dwmac-baikal.o
> >  obj-$(CONFIG_DWMAC_INGENIC)	+= dwmac-ingenic.o
> >  obj-$(CONFIG_DWMAC_IPQ806X)	+= dwmac-ipq806x.o
> >  obj-$(CONFIG_DWMAC_LPC18XX)	+= dwmac-lpc18xx.o
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c
> > new file mode 100644
> > index 000000000000..9133188a5d1b
> > --- /dev/null
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c
> > @@ -0,0 +1,199 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Baikal-T1/M SoCs DWMAC glue layer
> > + *
> > + * Copyright (C) 2015,2016,2021 Baikal Electronics JSC
> > + * Copyright (C) 2020-2022 BaseALT Ltd
> > + * Authors: Dmitry Dunaev <dmitry.dunaev@baikalelectronics.ru>
> > + *          Alexey Sheplyakov <asheplyakov@basealt.ru>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include "stmmac.h"
> > +#include "stmmac_platform.h"
> > +#include "common.h"
> > +#include "dwmac_dma.h"
> > +#include "dwmac1000_dma.h"
> > +
> > +#define MAC_GPIO	0x00e0	/* GPIO register */
> > +#define MAC_GPIO_GPO	BIT(8)	/* Output port */
> > +
> > +struct baikal_dwmac {
> > +	struct device	*dev;
> > +	struct clk	*tx2_clk;
> > +};
> > +
> 
> > +static int baikal_dwmac_dma_reset(void __iomem *ioaddr)
> > +{
> > +	int err;
> > +	u32 value;
> > +
> > +	/* DMA SW reset */
> > +	value = readl(ioaddr + DMA_BUS_MODE);
> > +	value |= DMA_BUS_MODE_SFT_RESET;
> > +	writel(value, ioaddr + DMA_BUS_MODE);
> > +
> > +	usleep_range(100, 120);
> > +
> > +	/* Clear PHY reset */
> > +	value = readl(ioaddr + MAC_GPIO);
> > +	value |= MAC_GPIO_GPO;
> > +	writel(value, ioaddr + MAC_GPIO);
> > +
> > +	return readl_poll_timeout(ioaddr + DMA_BUS_MODE, value,
> > +				  !(value & DMA_BUS_MODE_SFT_RESET),
> > +				  10000, 1000000);
> > +}
> > +
> > +static const struct stmmac_dma_ops baikal_dwmac_dma_ops = {
> > +	.reset = baikal_dwmac_dma_reset,
> 
> First of all this modification is redundant for the platforms not
> using the GMAC GPOs as resets, and is harmful if these signals are
> used for something not related with the GMAC interface. Secondly this
> callback won't properly work for all PHY types (though is acceptable
> for some simple PHYs, which don't require much initialization or have
> suitable default setups).

As a matter of fact with this DMA reset method Ethernet works just fine
on BFK3.1 board (based on Baikal-T1), TF307-MB-S-D board (Baikal-M),
LGP-16 system on the module (Baikal-M) [1], and a few other Baikal-M
based experimental boards. On all these boards Ethernet does NOT work
with original dwmac_dma_reset.

[1] https://www.lagrangeproject.com/lagrange-sarmah-som

> The problem is in the way the MAC + PHY
> initialization procedure is designed and in the way the embedded GPIOs
> are used in the platform. Even if we assume that all DW GMAC/xGBE
> GPIs/GPOs are used in conjunction with the corresponding GMAC
> interface (it's wrong in general), the interface open procedure upon
> return will still leave the PHY uninitialized or initialized with default
> values. That happens due to the PHY initialization being performed
> before the MAC initialization in the STMMAC open callback. Since the
> later implies calling the DW GMAC soft-reset, the former turns to be
> pointless due to the soft-reset causing the GPO toggle and consequent
> PHY reset.
> 
> So to speak in order to cover all the GPI/GPO usage scenario and in
> order to fix the problems described above the STMMAC core needs to be
> also properly modified, which isn't that easy due to the way the
> driver has evolved to.

I'm not trying to cover all usage scenarios. The current versions works
just fine with all Baikal-M and Baikal-T1 boards I've got so far, and
that is good enough for me.

> > +static int dwmac_baikal_probe(struct platform_device *pdev)
> > +{
> > +	struct plat_stmmacenet_data *plat_dat;
> > +	struct stmmac_resources stmmac_res;
> > +	struct baikal_dwmac *dwmac;
> > +	int ret;
> > +
> > +	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
> > +	if (!dwmac)
> > +		return -ENOMEM;
> > +
> > +	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "no suitable DMA available\n");
> > +		return ret;
> > +	}
> > +
> > +	plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
> > +	if (IS_ERR(plat_dat)) {
> > +		dev_err(&pdev->dev, "dt configuration failed\n");
> > +		return PTR_ERR(plat_dat);
> > +	}
> > +
> > +	dwmac->dev = &pdev->dev;
> 
> > +	dwmac->tx2_clk = devm_clk_get_optional(dwmac->dev, "tx2_clk");
> > +	if (IS_ERR(dwmac->tx2_clk)) {
> > +		ret = PTR_ERR(dwmac->tx2_clk);
> > +		dev_err(&pdev->dev, "couldn't get TX2 clock: %d\n", ret);
> > +		goto err_remove_config_dt;
> > +	}
> 
> The bindings are much more comprehensive than just a single Tx-clock.
> You are missing them here and in your DT-bindings patch. Please also
> note you can't make the DT-resources name up without providing a
> corresponding bindings schema update.

Can't parse this, sorry. Could you please elaborate what is exactly
wrong here?

> > +
> > +	if (dwmac->tx2_clk)
> > +		plat_dat->fix_mac_speed = baikal_dwmac_fix_mac_speed;
> > +	plat_dat->bsp_priv = dwmac;
> > +	plat_dat->has_gmac = 1;
> > +	plat_dat->enh_desc = 1;
> > +	plat_dat->tx_coe = 1;
> > +	plat_dat->rx_coe = 1;
> 
> > +	plat_dat->clk_csr = 3;
> 
> Instead of fixing the stmmac_clk_csr_set() method you have provided
> the clk_csr workaround. What if pclk rate is changed? Which BTW is
> possible. =) In that case you'll get a wrong MDC rate.

1) This works for me just fine with all Baikal-M and Baikal-T1 boards
   I've got so far. 
2) I avoid changes in the generic stmmac code on purpose to keep the risk
   of regressions minimal.
 
> > +	plat_dat->setup = baikal_dwmac_setup;
> > +
> > +	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> > +	if (ret)
> > +		goto err_remove_config_dt;
> > +
> > +	return 0;
> > +
> > +err_remove_config_dt:
> > +	stmmac_remove_config_dt(pdev, plat_dat);
> > +	return ret;
> > +}
> > +
> > +static const struct of_device_id dwmac_baikal_match[] = {
> 
> > +	{ .compatible = "baikal,dwmac" },
> 
> Even though Baikal-T1 and Baikal-M1 have been synthesized with almost
> identical IP-cores I wouldn't suggest to use the same compatible
> string for both of them. At least those are different platforms with
> different reference signals parameters. So it would be much better to
> use the naming like "baikal,bt1-gmac" and "baikal,bm1-gmac" here.

OK, makes sense.

> 
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(of, dwmac_baikal_match);
> > +
> > +static struct platform_driver dwmac_baikal_driver = {
> > +	.probe	= dwmac_baikal_probe,
> > +	.remove	= stmmac_pltfr_remove,
> > +	.driver	= {
> > +		.name = "baikal-dwmac",
> > +		.pm = &stmmac_pltfr_pm_ops,
> > +		.of_match_table = of_match_ptr(dwmac_baikal_match)
> > +	}
> > +};
> > +module_platform_driver(dwmac_baikal_driver);
> > +
> > +MODULE_DESCRIPTION("Baikal-T1/M DWMAC driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> > index 76edb9b72675..7b8a955d98a9 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> > @@ -563,3 +563,4 @@ int dwmac1000_setup(struct stmmac_priv *priv)
> >  
> >  	return 0;
> >  }
> > +EXPORT_SYMBOL_GPL(dwmac1000_setup);
> 
> As I said providing a platform-specific reset method won't solve the
> problem with the PHYs resetting on each interface up/down procedures.
> So exporting this method and the methods below will be just useless
> since the provided fix isn't complete.

When the experiment and a theory disagree the experiment always wins.
With this driver Ethernet works just fine with

* BFK3.1 board (Baikal-T1 reference boards)
* TF307-MB-S-D board (Baikal-M)
* LGP-16 system on the module (Baikal-M)

It might or might not work with other boards (although I haven't got
such boards myself yet).

Last but not least, if this driver is such a wrong and incomplete, why
Baikal Electronics ships it in its vendor kernel [2]?

[2] https://github.com/baikalelectronics/kernel/blob/v5.4_BE-aarch64_stable/drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c
    https://share.baikalelectronics.ru/index.php/s/Zi4tmLzpjCYccKb (warning: links are js-wrapped)


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer
  2022-02-01 19:08         ` Jakub Kicinski
@ 2022-02-07 15:22           ` Serge Semin
  0 siblings, 0 replies; 14+ messages in thread
From: Serge Semin @ 2022-02-07 15:22 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Serge Semin, Alexey Sheplyakov, netdev, Giuseppe Cavallaro,
	Alexandre Torgue, Jose Abreu, Alexey Malahov, Pavel Parkhomenko,
	Evgeny Sinelnikov

On Tue, Feb 01, 2022 at 11:08:38AM -0800, Jakub Kicinski wrote:
> On Tue, 1 Feb 2022 18:54:39 +0300 Serge Semin wrote:
> > On Fri, Jan 28, 2022 at 12:27:18PM -0800, Jakub Kicinski wrote:
> > > On Fri, 28 Jan 2022 22:55:09 +0400 Alexey Sheplyakov wrote:  
> > > > In general quite a number of Linux drivers (GPUs, WiFi chips, foreign
> > > > filesystems, you name it) provide a limited support for the corresponding
> > > > hardware (filesystem, protocol, etc) and don't cover all peculiarities.
> > > > Yet having such a limited support in the mainline kernel is much more
> > > > useful than no support at all (or having to use out-of-tree drivers,
> > > > obosolete vendor kernels, binary blobs, etc).
> > > > 
> > > > Therefore "does not cover all peculiarities" does not sound like a valid
> > > > reason for rejecting this driver. That said it's definitely up to stmmac
> > > > maintainers to decide if the code meets the quality standards, does not
> > > > cause excessive maintanence burden, etc.  
> > > 
> > > Sounds sensible, Serge please take a look at the v2 and let us know if
> > > there are any bugs in there. Or any differences in DT bindings or user
> > > visible behaviors with what you're planning to do. If the driver is
> > > functional and useful it can evolve and gain support for features and
> > > platforms over time.  
> > 
> > I've already posted my comments in this thread regarding the main
> > problematic issues of the driver, but Alexey for some reason ignored
> > them (dropped from his reply). Do you want me to copy my comments to
> > v2 and to proceed with review there?
> 
> Right, on a closer look there are indeed comments you raised that were
> not addressed and not constrained to future compatibility. 
> 
> Alexey, please take another look at those and provide a changelog in
> your next posting so we can easily check what was addressed.
> 
> > Regarding the DT-bindings and the user-visible behavior. Right, I'll
> > add my comments in this matter. Thanks for suggesting. This was one of
> > the problems why I was against the driver integrating into the kernel.
> > One of our patchset brings a better organization to the current
> > DT-bindings of the Synopsys DW *MAC devices. In particular it splits
> > up the generic bindings for the vendor-specific MACs to use and the
> > bindings for the pure DW MAC compatible devices. In addition the
> > patchset will add the generic Tx/Rx clocks DT-bindings and the
> > DT-bindings for the AXI/MTL nodes. All of that and the rest of our
> > work will be posted a bit later as a set of the incremental patchsets
> > with small changes, one by one, for an easier review. We just need
> > some more time to finish the left of the work. The reason why the
> > already developed patches hasn't been delivered yet is that the rest
> > of the work may cause adding changes into the previous patches. In
> > order to decrease a number of the patches to review and present a
> > complete work for the community, we decided to post the patchsets
> > after the work is fully done.
> 

> TBH starting to post stuff is probably best choice you can make,
> for example the DT rework you mention sounds like a refactoring 
> you can perform without posting any Baikal support.

I wouldn't say that the DT-part is a refactoring. Since it's
DT-bindings I can't change it' interface much (as I'd like to for
instance in the snps,axi-config or mtp-{tx,rx}-config sub-nodes
bindings). At the very least I can't make them incompatible with
already defined DT-interface. So that was more like an optimization
with updating the Yaml-schemas to be more generic for all the DW MACs:
MAC100, GMAC, EQOS and xGMAC.

Regarding submitting the stuff now and delivering the updates
afterwards. Thanks for suggesting. I thought about that at first, but
then changed my mind. The thing is that I am still in progress of the
STMMAC-related development. Even though a few more tasks left to be
done, they will concerns the bindings change. In addition working on
review comments will distract me from the rest of the STMMAC tasks. So
I'd rather finish all what we've planned first and then start sending
the series one after another with well structured cover letters. Thus
the community will have a coherent set of sets and less patches for
review, while I'll have less gray hairs due to deadlines and
distractions.)

-Sergey

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer
  2022-02-03 10:49   ` Alexey Sheplyakov
@ 2022-02-07 17:48     ` Serge Semin
  0 siblings, 0 replies; 14+ messages in thread
From: Serge Semin @ 2022-02-07 17:48 UTC (permalink / raw)
  To: Alexey Sheplyakov
  Cc: Serge Semin, netdev, Giuseppe Cavallaro, Alexandre Torgue,
	Jose Abreu, Alexey Malahov, Pavel Parkhomenko

On Thu, Feb 03, 2022 at 02:49:48PM +0400, Alexey Sheplyakov wrote:
> Hello,
> 
> On Fri, Jan 28, 2022 at 06:06:42PM +0300, Serge Semin wrote:
>  
> > My comments regarding the most problematic parts of this patch are
> > below.
> > 
> > On Wed, Jan 26, 2022 at 12:44:55PM +0400, Alexey Sheplyakov wrote:
> > > The gigabit Ethernet controller available in Baikal-T1 and Baikal-M
> > > SoCs is a Synopsys DesignWare MAC IP core, already supported by
> > > the stmmac driver.
> > > 
> > > This patch implements some SoC specific operations (DMA reset and
> > > speed fixup) necessary for Baikal-T1/M variants.
> > > 
> > > Signed-off-by: Alexey Sheplyakov <asheplyakov@basealt.ru>
> > > Signed-off-by: Dmitry Dunaev <dmitry.dunaev@baikalelectronics.ru>
> > > ---
> > >  drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 +
> > >  drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
> > >  .../ethernet/stmicro/stmmac/dwmac-baikal.c    | 199 ++++++++++++++++++
> > >  .../ethernet/stmicro/stmmac/dwmac1000_core.c  |   1 +
> > >  .../ethernet/stmicro/stmmac/dwmac1000_dma.c   |  46 ++--
> > >  .../ethernet/stmicro/stmmac/dwmac1000_dma.h   |  26 +++
> > >  .../net/ethernet/stmicro/stmmac/dwmac_lib.c   |   8 +
> > >  7 files changed, 274 insertions(+), 18 deletions(-)
> > >  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c
> > >  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.h
> > > 
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > > index 929cfc22cd0c..d8e6dcb98e6c 100644
> > > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > > @@ -66,6 +66,17 @@ config DWMAC_ANARION
> > >  
> > >  	  This selects the Anarion SoC glue layer support for the stmmac driver.
> > >  
> > > +config DWMAC_BAIKAL
> > > +	tristate "Baikal Electronics GMAC support"
> > > +	default MIPS_BAIKAL_T1
> > > +	depends on OF && (MIPS || ARM64 || COMPILE_TEST)
> > > +	help
> > > +	  Support for gigabit ethernet controller on Baikal Electronics SoCs.
> > > +
> > > +	  This selects the Baikal Electronics SoCs glue layer support for
> > > +	  the stmmac driver. This driver is used for Baikal-T1 and Baikal-M
> > > +	  SoCs gigabit ethernet controller.
> > > +
> > >  config DWMAC_INGENIC
> > >  	tristate "Ingenic MAC support"
> > >  	default MACH_INGENIC
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> > > index d4e12e9ace4f..ad138062e199 100644
> > > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> > > @@ -14,6 +14,7 @@ stmmac-$(CONFIG_STMMAC_SELFTESTS) += stmmac_selftests.o
> > >  # Ordering matters. Generic driver must be last.
> > >  obj-$(CONFIG_STMMAC_PLATFORM)	+= stmmac-platform.o
> > >  obj-$(CONFIG_DWMAC_ANARION)	+= dwmac-anarion.o
> > > +obj-$(CONFIG_DWMAC_BAIKAL)	+= dwmac-baikal.o
> > >  obj-$(CONFIG_DWMAC_INGENIC)	+= dwmac-ingenic.o
> > >  obj-$(CONFIG_DWMAC_IPQ806X)	+= dwmac-ipq806x.o
> > >  obj-$(CONFIG_DWMAC_LPC18XX)	+= dwmac-lpc18xx.o
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c
> > > new file mode 100644
> > > index 000000000000..9133188a5d1b
> > > --- /dev/null
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c
> > > @@ -0,0 +1,199 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +/*
> > > + * Baikal-T1/M SoCs DWMAC glue layer
> > > + *
> > > + * Copyright (C) 2015,2016,2021 Baikal Electronics JSC
> > > + * Copyright (C) 2020-2022 BaseALT Ltd
> > > + * Authors: Dmitry Dunaev <dmitry.dunaev@baikalelectronics.ru>
> > > + *          Alexey Sheplyakov <asheplyakov@basealt.ru>
> > > + */
> > > +
> > > +#include <linux/clk.h>
> > > +#include <linux/iopoll.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of.h>
> > > +#include <linux/platform_device.h>
> > > +
> > > +#include "stmmac.h"
> > > +#include "stmmac_platform.h"
> > > +#include "common.h"
> > > +#include "dwmac_dma.h"
> > > +#include "dwmac1000_dma.h"
> > > +
> > > +#define MAC_GPIO	0x00e0	/* GPIO register */
> > > +#define MAC_GPIO_GPO	BIT(8)	/* Output port */
> > > +
> > > +struct baikal_dwmac {
> > > +	struct device	*dev;
> > > +	struct clk	*tx2_clk;
> > > +};
> > > +
> > 
> > > +static int baikal_dwmac_dma_reset(void __iomem *ioaddr)
> > > +{
> > > +	int err;
> > > +	u32 value;
> > > +
> > > +	/* DMA SW reset */
> > > +	value = readl(ioaddr + DMA_BUS_MODE);
> > > +	value |= DMA_BUS_MODE_SFT_RESET;
> > > +	writel(value, ioaddr + DMA_BUS_MODE);
> > > +
> > > +	usleep_range(100, 120);
> > > +
> > > +	/* Clear PHY reset */
> > > +	value = readl(ioaddr + MAC_GPIO);
> > > +	value |= MAC_GPIO_GPO;
> > > +	writel(value, ioaddr + MAC_GPIO);
> > > +
> > > +	return readl_poll_timeout(ioaddr + DMA_BUS_MODE, value,
> > > +				  !(value & DMA_BUS_MODE_SFT_RESET),
> > > +				  10000, 1000000);
> > > +}
> > > +
> > > +static const struct stmmac_dma_ops baikal_dwmac_dma_ops = {
> > > +	.reset = baikal_dwmac_dma_reset,
> > 
> > First of all this modification is redundant for the platforms not
> > using the GMAC GPOs as resets, and is harmful if these signals are
> > used for something not related with the GMAC interface. Secondly this
> > callback won't properly work for all PHY types (though is acceptable
> > for some simple PHYs, which don't require much initialization or have
> > suitable default setups).
> 

> As a matter of fact with this DMA reset method Ethernet works just fine
> on BFK3.1 board (based on Baikal-T1), TF307-MB-S-D board (Baikal-M),
> LGP-16 system on the module (Baikal-M) [1], and a few other Baikal-M
> based experimental boards. On all these boards Ethernet does NOT work
> with original dwmac_dma_reset.
> 
> [1] https://www.lagrangeproject.com/lagrange-sarmah-som

This doesn't work for my TP SFBT1 and MRBT1 boards. Network traffic is
mainly lost.

> 
> > The problem is in the way the MAC + PHY
> > initialization procedure is designed and in the way the embedded GPIOs
> > are used in the platform. Even if we assume that all DW GMAC/xGBE
> > GPIs/GPOs are used in conjunction with the corresponding GMAC
> > interface (it's wrong in general), the interface open procedure upon
> > return will still leave the PHY uninitialized or initialized with default
> > values. That happens due to the PHY initialization being performed
> > before the MAC initialization in the STMMAC open callback. Since the
> > later implies calling the DW GMAC soft-reset, the former turns to be
> > pointless due to the soft-reset causing the GPO toggle and consequent
> > PHY reset.
> > 
> > So to speak in order to cover all the GPI/GPO usage scenario and in
> > order to fix the problems described above the STMMAC core needs to be
> > also properly modified, which isn't that easy due to the way the
> > driver has evolved to.
> 

> I'm not trying to cover all usage scenarios. The current versions works
> just fine with all Baikal-M and Baikal-T1 boards I've got so far, and
> that is good enough for me.

AFAICS you aren't submitting a driver for the "Boards you've got", but
you claim it's compatible with the Baikal-M/T GMAC (no matter what you
say in the commit message, the driver code says otherwise). As I said
above it appears to be isn't fully compatible. It doesn't work for the
boards I've got. Moreover as I said it might be even harmful for the
platforms, with GPOs used for something reset-unrelated.

> 
> > > +static int dwmac_baikal_probe(struct platform_device *pdev)
> > > +{
> > > +	struct plat_stmmacenet_data *plat_dat;
> > > +	struct stmmac_resources stmmac_res;
> > > +	struct baikal_dwmac *dwmac;
> > > +	int ret;
> > > +
> > > +	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
> > > +	if (!dwmac)
> > > +		return -ENOMEM;
> > > +
> > > +	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> > > +	if (ret) {
> > > +		dev_err(&pdev->dev, "no suitable DMA available\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
> > > +	if (IS_ERR(plat_dat)) {
> > > +		dev_err(&pdev->dev, "dt configuration failed\n");
> > > +		return PTR_ERR(plat_dat);
> > > +	}
> > > +
> > > +	dwmac->dev = &pdev->dev;
> > 
> > > +	dwmac->tx2_clk = devm_clk_get_optional(dwmac->dev, "tx2_clk");
> > > +	if (IS_ERR(dwmac->tx2_clk)) {
> > > +		ret = PTR_ERR(dwmac->tx2_clk);
> > > +		dev_err(&pdev->dev, "couldn't get TX2 clock: %d\n", ret);
> > > +		goto err_remove_config_dt;
> > > +	}
> > 
> > The bindings are much more comprehensive than just a single Tx-clock.
> > You are missing them here and in your DT-bindings patch. Please also
> > note you can't make the DT-resources name up without providing a
> > corresponding bindings schema update.
> 

> Can't parse this, sorry. Could you please elaborate what is exactly
> wrong here?

First of all, you don't need to create a special name for the
ordinary RGMII Tx clock source. DW GMAC doesn't care whether it's
doubled or not, just use "tx" here (suffix _clk is also redundant).
Secondly the MAC has got a certain set of the clock sources, embedded
GPIO controller, embedded RGMII Tx delay, Tx/Rx FIFO depths, AXI burst
length, etc. All of that is the device bindings.

> 
> > > +
> > > +	if (dwmac->tx2_clk)
> > > +		plat_dat->fix_mac_speed = baikal_dwmac_fix_mac_speed;
> > > +	plat_dat->bsp_priv = dwmac;
> > > +	plat_dat->has_gmac = 1;
> > > +	plat_dat->enh_desc = 1;
> > > +	plat_dat->tx_coe = 1;
> > > +	plat_dat->rx_coe = 1;
> > 
> > > +	plat_dat->clk_csr = 3;
> > 
> > Instead of fixing the stmmac_clk_csr_set() method you have provided
> > the clk_csr workaround. What if pclk rate is changed? Which BTW is
> > possible. =) In that case you'll get a wrong MDC rate.
> 
> 1) This works for me just fine with all Baikal-M and Baikal-T1 boards
>    I've got so far. 

I've changed APB clock to 200MHz on my platform. Now MDC lane is
clocked with 7.69MHz, which is out of what DW GMAC doc requires. Both
CSR and MDC clocks aren't fixed for a generic Baikal-T/M platform your
driver claims to be compatible with. So to speak this config is wrong.

> 2) I avoid changes in the generic stmmac code on purpose to keep the risk
>    of regressions minimal.

Good for you, but being afraid of introducing a regression in one place isn't
a good excuse of adding a broken code in another.

>  
> > > +	plat_dat->setup = baikal_dwmac_setup;
> > > +
> > > +	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> > > +	if (ret)
> > > +		goto err_remove_config_dt;
> > > +
> > > +	return 0;
> > > +
> > > +err_remove_config_dt:
> > > +	stmmac_remove_config_dt(pdev, plat_dat);
> > > +	return ret;
> > > +}
> > > +
> > > +static const struct of_device_id dwmac_baikal_match[] = {
> > 
> > > +	{ .compatible = "baikal,dwmac" },
> > 
> > Even though Baikal-T1 and Baikal-M1 have been synthesized with almost
> > identical IP-cores I wouldn't suggest to use the same compatible
> > string for both of them. At least those are different platforms with
> > different reference signals parameters. So it would be much better to
> > use the naming like "baikal,bt1-gmac" and "baikal,bm1-gmac" here.
> 
> OK, makes sense.
> 
> > 
> > > +	{ }
> > > +};
> > > +MODULE_DEVICE_TABLE(of, dwmac_baikal_match);
> > > +
> > > +static struct platform_driver dwmac_baikal_driver = {
> > > +	.probe	= dwmac_baikal_probe,
> > > +	.remove	= stmmac_pltfr_remove,
> > > +	.driver	= {
> > > +		.name = "baikal-dwmac",
> > > +		.pm = &stmmac_pltfr_pm_ops,
> > > +		.of_match_table = of_match_ptr(dwmac_baikal_match)
> > > +	}
> > > +};
> > > +module_platform_driver(dwmac_baikal_driver);
> > > +
> > > +MODULE_DESCRIPTION("Baikal-T1/M DWMAC driver");
> > > +MODULE_LICENSE("GPL");
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> > > index 76edb9b72675..7b8a955d98a9 100644
> > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> > > @@ -563,3 +563,4 @@ int dwmac1000_setup(struct stmmac_priv *priv)
> > >  
> > >  	return 0;
> > >  }
> > > +EXPORT_SYMBOL_GPL(dwmac1000_setup);
> > 
> > As I said providing a platform-specific reset method won't solve the
> > problem with the PHYs resetting on each interface up/down procedures.
> > So exporting this method and the methods below will be just useless
> > since the provided fix isn't complete.
> 

> When the experiment and a theory disagree the experiment always wins.
> With this driver Ethernet works just fine with
> 
> * BFK3.1 board (Baikal-T1 reference boards)
> * TF307-MB-S-D board (Baikal-M)
> * LGP-16 system on the module (Baikal-M)
> 
> It might or might not work with other boards (although I haven't got
> such boards myself yet).

Well, we've got the boards for which it doesn't work.

> 
> Last but not least, if this driver is such a wrong and incomplete, why
> Baikal Electronics ships it in its vendor kernel [2]?
> 
> [2] https://github.com/baikalelectronics/kernel/blob/v5.4_BE-aarch64_stable/drivers/net/ethernet/stmicro/stmmac/dwmac-baikal.c
>     https://share.baikalelectronics.ru/index.php/s/Zi4tmLzpjCYccKb (warning: links are js-wrapped)
> 

Really? Backward question then. Do you want a wrong and incomplete
driver being integrated into the kernel? As I see it, you do. But we
don't. We want the mainline kernel to be stable, portable, and not
having temporal solutions "working for my case, and the rest doesn't
bother me".

-Sergey

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-02-07 17:53 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-26  8:44 [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer Alexey Sheplyakov
2022-01-26  8:44 ` [PATCH 2/2] dt-bindings: dwmac: Add bindings for Baikal-T1/M SoCs Alexey Sheplyakov
2022-01-27  1:00 ` [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer Jakub Kicinski
2022-01-28 12:16   ` [PATCH v2 " Alexey Sheplyakov
2022-01-28 12:16     ` [PATCH v2 2/2] dt-bindings: dwmac: Add bindings for Baikal-T1/M SoCs Alexey Sheplyakov
2022-01-28 17:00   ` [PATCH 1/2] net: stmmac: added Baikal-T1/M SoCs glue layer Alexey Sheplyakov
2022-01-28 15:06 ` Serge Semin
2022-01-28 18:55   ` Alexey Sheplyakov
2022-01-28 20:27     ` Jakub Kicinski
2022-02-01 15:54       ` Serge Semin
2022-02-01 19:08         ` Jakub Kicinski
2022-02-07 15:22           ` Serge Semin
2022-02-03 10:49   ` Alexey Sheplyakov
2022-02-07 17:48     ` Serge Semin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.