[v2, PATCH 0/2] add Ethernet driver support for mt2712

[v2, PATCH 0/2] add Ethernet driver support for mt2712

[Biao Huang]

The mt2712 Ethernet IP is from Synopsys, so extend stmmac to support mt2712.

[PATCH 1/2] net:stmmac: dwmac-mediatek: add support for mt2712

[Biao Huang]

Add Ethernet support for MediaTek SoCs from the mt2712 family

Signed-off-by: Biao Huang <biao.huang@mediatek.com>
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig        |    8 +
 drivers/net/ethernet/stmicro/stmmac/Makefile       |    1 +
 .../net/ethernet/stmicro/stmmac/dwmac-mediatek.c   |  364 ++++++++++++++++++++
 3 files changed, 373 insertions(+)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index edf2036..76d779e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -75,6 +75,14 @@ config DWMAC_LPC18XX
 	---help---
 	  Support for NXP LPC18xx/43xx DWMAC Ethernet.
 
+config DWMAC_MEDIATEK
+	tristate "MediaTek MT27xx GMAC support"
+	depends on OF
+	help
+	  Support for MediaTek GMAC Ethernet controller.
+
+	  This selects the MT2712 SoC support for the stmmac driver.
+
 config DWMAC_MESON
 	tristate "Amlogic Meson dwmac support"
 	default ARCH_MESON
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index 99967a8..bf09701 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_STMMAC_PLATFORM)	+= stmmac-platform.o
 obj-$(CONFIG_DWMAC_ANARION)	+= dwmac-anarion.o
 obj-$(CONFIG_DWMAC_IPQ806X)	+= dwmac-ipq806x.o
 obj-$(CONFIG_DWMAC_LPC18XX)	+= dwmac-lpc18xx.o
+obj-$(CONFIG_DWMAC_MEDIATEK)	+= dwmac-mediatek.o
 obj-$(CONFIG_DWMAC_MESON)	+= dwmac-meson.o dwmac-meson8b.o
 obj-$(CONFIG_DWMAC_OXNAS)	+= dwmac-oxnas.o
 obj-$(CONFIG_DWMAC_ROCKCHIP)	+= dwmac-rk.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
new file mode 100644
index 0000000..9ccf3a5
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
@@ -0,0 +1,364 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (c) 2018 MediaTek Inc.
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_net.h>
+#include <linux/regmap.h>
+#include <linux/stmmac.h>
+
+#include "stmmac.h"
+#include "stmmac_platform.h"
+
+/* Peri Configuration register */
+#define PERI_ETH_PHY_INTF_SEL	0x418
+#define PHY_INTF_MII_GMII	0
+#define PHY_INTF_RGMII		1
+#define PHY_INTF_RMII		4
+#define RMII_CLK_SRC_RXC	BIT(4)
+#define RMII_CLK_SRC_INTERNAL	BIT(5)
+
+#define PERI_ETH_PHY_DLY	0x428
+#define PHY_DLY_GTXC_INV	BIT(6)
+#define PHY_DLY_GTXC_ENABLE	BIT(5)
+#define PHY_DLY_GTXC_STAGES	GENMASK(4, 0)
+#define PHY_DLY_TXC_INV		BIT(20)
+#define PHY_DLY_TXC_ENABLE	BIT(19)
+#define PHY_DLY_TXC_STAGES	GENMASK(18, 14)
+#define PHY_DLY_TXC_SHIFT	14
+#define PHY_DLY_RXC_INV		BIT(13)
+#define PHY_DLY_RXC_ENABLE	BIT(12)
+#define PHY_DLY_RXC_STAGES	GENMASK(11, 7)
+#define PHY_DLY_RXC_SHIFT	7
+
+#define PERI_ETH_DLY_FINE	0x800
+#define ETH_RMII_DLY_TX_INV	BIT(2)
+#define ETH_FINE_DLY_GTXC	BIT(1)
+#define ETH_FINE_DLY_RXC	BIT(0)
+
+enum dwmac_clks_map {
+	DWMAC_CLK_AXI_DRAM,
+	DWMAC_CLK_APB_REG,
+	DWMAC_CLK_MAC_EXT,
+	DWMAC_CLK_MAC_PARENT,
+	DWMAC_CLK_PTP_REF,
+	DWMAC_CLK_PTP_PARENT,
+	DWMAC_CLK_PTP_TOP,
+	DWMAC_CLK_MAX
+};
+
+struct mac_delay_struct {
+	u32 tx_delay;
+	u32 rx_delay;
+	u32 tx_inv;
+	u32 rx_inv;
+};
+
+struct mediatek_dwmac_plat_data {
+	struct device *dev;
+	struct regmap *peri_regmap;
+	struct clk *clks[DWMAC_CLK_MAX];
+	struct device_node *np;
+	int phy_mode;
+	struct mac_delay_struct mac_delay;
+	const struct mediatek_dwmac_variant *variant;
+	int fine_tune;
+	int rmii_rxc;
+};
+
+struct mediatek_dwmac_variant {
+	int (*dwmac_config_dt)(struct mediatek_dwmac_plat_data *plat);
+	int (*dwmac_enable_clks)(struct mediatek_dwmac_plat_data *plat);
+	void (*dwmac_disable_clks)(struct mediatek_dwmac_plat_data *plat);
+};
+
+static const char * const mediatek_dwmac_clks_name[] = {
+	"axi", "apb", "mac_ext", "mac_parent", "ptp_ref", "ptp_parent", "ptp_top"
+};
+
+static int mt2712_set_interface(struct mediatek_dwmac_plat_data *plat)
+{
+	int rmii_rxc = plat->rmii_rxc ? RMII_CLK_SRC_RXC : 0;
+	u32 intf_val = 0;
+
+	/* select phy interface in top control domain */
+	switch (plat->phy_mode) {
+	case PHY_INTERFACE_MODE_MII:
+		intf_val |= PHY_INTF_MII_GMII;
+		break;
+	case PHY_INTERFACE_MODE_RMII:
+		intf_val |= PHY_INTF_RMII;
+		intf_val |= rmii_rxc;
+		break;
+	case PHY_INTERFACE_MODE_RGMII:
+	case PHY_INTERFACE_MODE_RGMII_TXID:
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+	case PHY_INTERFACE_MODE_RGMII_ID:
+		intf_val |= PHY_INTF_RGMII;
+		break;
+	default:
+		pr_err("phy interface not support\n");
+		return -EINVAL;
+	}
+
+	regmap_write(plat->peri_regmap, PERI_ETH_PHY_INTF_SEL, intf_val);
+
+	return 0;
+}
+
+static int mt2712_set_delay(struct mediatek_dwmac_plat_data *plat)
+{
+	struct mac_delay_struct *mac_delay = &plat->mac_delay;
+	u32 delay_val = 0;
+	u32 fine_val = 0;
+
+	switch (plat->phy_mode) {
+	case PHY_INTERFACE_MODE_MII:
+		delay_val |= mac_delay->tx_delay ? PHY_DLY_TXC_ENABLE : 0;
+		delay_val |= (mac_delay->tx_delay << PHY_DLY_TXC_SHIFT) &
+			     PHY_DLY_TXC_STAGES;
+		delay_val |= mac_delay->tx_inv ? PHY_DLY_TXC_INV : 0;
+		delay_val |= mac_delay->rx_delay ? PHY_DLY_RXC_ENABLE : 0;
+		delay_val |= (mac_delay->rx_delay << PHY_DLY_RXC_SHIFT) &
+			     PHY_DLY_RXC_STAGES;
+		delay_val |= mac_delay->rx_inv ? PHY_DLY_RXC_INV : 0;
+		break;
+	case PHY_INTERFACE_MODE_RMII:
+		if (plat->rmii_rxc) {
+			delay_val |= mac_delay->rx_delay ?
+				     PHY_DLY_RXC_ENABLE : 0;
+			delay_val |= (mac_delay->rx_delay <<
+				      PHY_DLY_RXC_SHIFT) & PHY_DLY_RXC_STAGES;
+			delay_val |= mac_delay->rx_inv ? PHY_DLY_RXC_INV : 0;
+			fine_val |= mac_delay->tx_inv ?
+				     ETH_RMII_DLY_TX_INV : 0;
+		} else {
+			delay_val |= mac_delay->tx_delay ?
+				     PHY_DLY_TXC_ENABLE : 0;
+			delay_val |= (mac_delay->tx_delay <<
+				     PHY_DLY_TXC_SHIFT) & PHY_DLY_TXC_STAGES;
+			delay_val |= mac_delay->tx_inv ? PHY_DLY_TXC_INV : 0;
+			fine_val |= mac_delay->rx_inv ?
+				     ETH_RMII_DLY_TX_INV : 0;
+		}
+		break;
+	case PHY_INTERFACE_MODE_RGMII:
+		fine_val = plat->fine_tune ?
+			    (ETH_FINE_DLY_GTXC | ETH_FINE_DLY_RXC) : 0;
+		delay_val |= mac_delay->tx_delay ? PHY_DLY_GTXC_ENABLE : 0;
+		delay_val |= mac_delay->tx_delay & PHY_DLY_GTXC_STAGES;
+		delay_val |= mac_delay->tx_inv ? PHY_DLY_GTXC_INV : 0;
+		delay_val |= mac_delay->rx_delay ? PHY_DLY_RXC_ENABLE : 0;
+		delay_val |= (mac_delay->rx_delay << PHY_DLY_RXC_SHIFT) &
+			     PHY_DLY_RXC_STAGES;
+		delay_val |= mac_delay->rx_inv ? PHY_DLY_RXC_INV : 0;
+		break;
+	case PHY_INTERFACE_MODE_RGMII_TXID:
+		fine_val = plat->fine_tune ? ETH_FINE_DLY_RXC : 0;
+		delay_val |= mac_delay->rx_delay ? PHY_DLY_RXC_ENABLE : 0;
+		delay_val |= (mac_delay->rx_delay << PHY_DLY_RXC_SHIFT) &
+			     PHY_DLY_RXC_STAGES;
+		delay_val |= mac_delay->rx_inv ? PHY_DLY_RXC_INV : 0;
+		break;
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+		fine_val = plat->fine_tune ? ETH_FINE_DLY_GTXC : 0;
+		delay_val |= mac_delay->tx_delay ? PHY_DLY_GTXC_ENABLE : 0;
+		delay_val |= mac_delay->tx_delay & PHY_DLY_GTXC_STAGES;
+		delay_val |= mac_delay->tx_inv ? PHY_DLY_GTXC_INV : 0;
+		break;
+	case PHY_INTERFACE_MODE_RGMII_ID:
+		break;
+	default:
+		pr_err("phy interface not support\n");
+		return -EINVAL;
+	}
+	regmap_write(plat->peri_regmap, PERI_ETH_PHY_DLY, delay_val);
+	regmap_write(plat->peri_regmap, PERI_ETH_DLY_FINE, fine_val);
+
+	return 0;
+}
+
+static int mt2712_get_clks(struct mediatek_dwmac_plat_data *plat)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(plat->clks); i++) {
+		plat->clks[i] = devm_clk_get(plat->dev,
+					     mediatek_dwmac_clks_name[i]);
+		if (IS_ERR(plat->clks[i]))
+			return PTR_ERR(plat->clks[i]);
+	}
+
+	return 0;
+}
+
+static int mt2712_enable_clks(struct mediatek_dwmac_plat_data *plat)
+{
+	int clk, ret;
+
+	for (clk = 0; clk < DWMAC_CLK_MAX ; clk++) {
+		ret = clk_prepare_enable(plat->clks[clk]);
+		if (ret)
+			goto err_disable_clks;
+	}
+
+	ret = clk_set_parent(plat->clks[DWMAC_CLK_MAC_EXT], plat->clks[DWMAC_CLK_MAC_PARENT]);
+	if (ret)
+		goto err_disable_clks;
+
+	ret = clk_set_parent(plat->clks[DWMAC_CLK_PTP_REF], plat->clks[DWMAC_CLK_PTP_PARENT]);
+	if (ret)
+		goto err_disable_clks;
+
+	return 0;
+
+err_disable_clks:
+	while (--clk >= 0)
+		clk_disable_unprepare(plat->clks[clk]);
+
+	return ret;
+}
+
+static void mt2712_disable_clks(struct mediatek_dwmac_plat_data *plat)
+{
+	int clk;
+
+	for (clk = DWMAC_CLK_MAX - 1; clk >= 0; clk--)
+		clk_disable_unprepare(plat->clks[clk]);
+}
+
+static int mt2712_config_dt(struct mediatek_dwmac_plat_data *plat)
+{
+	u32 mac_timings[4];
+
+	plat->peri_regmap = syscon_regmap_lookup_by_compatible("mediatek,mt2712-pericfg");
+	if (IS_ERR(plat->peri_regmap)) {
+		dev_err(plat->dev, "Failed to get pericfg syscon\n");
+		return PTR_ERR(plat->peri_regmap);
+	}
+
+	if (!of_property_read_u32_array(plat->np, "mac-delay", mac_timings,
+					ARRAY_SIZE(mac_timings))) {
+		plat->mac_delay.tx_delay = mac_timings[0];
+		plat->mac_delay.rx_delay = mac_timings[1];
+		plat->mac_delay.tx_inv = mac_timings[2];
+		plat->mac_delay.rx_inv = mac_timings[3];
+	}
+
+	plat->fine_tune = of_property_read_bool(plat->np, "fine-tune");
+
+	plat->rmii_rxc = of_property_read_bool(plat->np, "rmii-rxc");
+
+	mt2712_set_interface(plat);
+
+	mt2712_set_delay(plat);
+
+	return mt2712_get_clks(plat);
+}
+
+static const struct mediatek_dwmac_variant mt2712_gmac_variant = {
+		.dwmac_config_dt = mt2712_config_dt,
+		.dwmac_enable_clks = mt2712_enable_clks,
+		.dwmac_disable_clks = mt2712_disable_clks,
+};
+
+static int mediatek_dwmac_config_dt(struct mediatek_dwmac_plat_data *plat)
+{
+	const struct mediatek_dwmac_variant *variant = plat->variant;
+
+	/* Set the DMA mask, 4GB mode enabled */
+	dma_set_mask_and_coherent(plat->dev, DMA_BIT_MASK(33));
+
+	return variant->dwmac_config_dt(plat);
+}
+
+static int mediatek_dwmac_init(struct platform_device *pdev, void *priv)
+{
+	struct mediatek_dwmac_plat_data *plat = priv;
+	const struct mediatek_dwmac_variant *variant = plat->variant;
+
+	return variant->dwmac_enable_clks(plat);
+}
+
+static void mediatek_dwmac_exit(struct platform_device *pdev, void *priv)
+{
+	struct mediatek_dwmac_plat_data *plat = priv;
+	const struct mediatek_dwmac_variant *variant = plat->variant;
+
+	variant->dwmac_disable_clks(plat);
+}
+
+static int mediatek_dwmac_probe(struct platform_device *pdev)
+{
+	int ret = 0;
+	struct plat_stmmacenet_data *plat_dat;
+	struct stmmac_resources stmmac_res;
+	struct mediatek_dwmac_plat_data *priv_plat;
+
+	priv_plat = devm_kzalloc(&pdev->dev, sizeof(*priv_plat), GFP_KERNEL);
+	if (!priv_plat)
+		return -ENOMEM;
+
+	priv_plat->variant = of_device_get_match_data(&pdev->dev);
+	if (!priv_plat->variant) {
+		dev_err(&pdev->dev, "Missing dwmac-mediatek variant\n");
+		return -EINVAL;
+	}
+
+	priv_plat->dev = &pdev->dev;
+	priv_plat->np = pdev->dev.of_node;
+	priv_plat->phy_mode = of_get_phy_mode(priv_plat->np);
+
+	ret = mediatek_dwmac_config_dt(priv_plat);
+	if (ret)
+		return ret;
+
+	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
+	if (ret)
+		return ret;
+
+	plat_dat = stmmac_probe_config_dt(pdev, &stmmac_res.mac);
+	if (IS_ERR(plat_dat))
+		return PTR_ERR(plat_dat);
+
+	plat_dat->interface = priv_plat->phy_mode;
+	/* clk_csr_i = 250-300MHz & MDC = clk_csr_i/124 */
+	plat_dat->clk_csr = 5;
+	plat_dat->has_gmac4 = 1;
+	plat_dat->has_gmac = 0;
+	plat_dat->pmt = 0;
+	plat_dat->maxmtu = 1500;
+	plat_dat->bsp_priv = priv_plat;
+	plat_dat->init = mediatek_dwmac_init;
+	plat_dat->exit = mediatek_dwmac_exit;
+	mediatek_dwmac_init(pdev, priv_plat);
+
+	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	if (ret) {
+		stmmac_remove_config_dt(pdev, plat_dat);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id mediatek_dwmac_match[] = {
+	{ .compatible = "mediatek,mt2712-gmac",
+	  .data = &mt2712_gmac_variant },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, mediatek_dwmac_match);
+
+static struct platform_driver mediatek_dwmac_driver = {
+	.probe  = mediatek_dwmac_probe,
+	.remove = stmmac_pltfr_remove,
+	.driver = {
+		.name           = "dwmac-mediatek",
+		.pm		= &stmmac_pltfr_pm_ops,
+		.of_match_table = mediatek_dwmac_match,
+	},
+};
+module_platform_driver(mediatek_dwmac_driver);
-- 
1.7.9.5

[PATCH 2/2] dt-binding: mediatek-dwmac: add binding document for MediaTek MT2712 DWMAC

[Biao Huang]

The commit adds the device tree binding documentation for the MediaTek DWMAC
found on MediaTek MT2712.

Signed-off-by: Biao Huang <biao.huang@mediatek.com>
---
 .../devicetree/bindings/net/mediatek-dwmac.txt     |   49 ++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/mediatek-dwmac.txt

diff --git a/Documentation/devicetree/bindings/net/mediatek-dwmac.txt b/Documentation/devicetree/bindings/net/mediatek-dwmac.txt
new file mode 100644
index 0000000..8baf0e4
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/mediatek-dwmac.txt
@@ -0,0 +1,49 @@
+MediaTek DWMAC glue layer controller
+
+This file documents platform glue layer for stmmac.
+Please see stmmac.txt for the other unchanged properties.
+
+The device node has following properties.
+
+Required properties:
+- compatible:  Should be "mediatek,mt2712-gmac" for MT2712 SoC
+- reg:  Address and length of the register set for the device
+- interrupts:  Should contain the MAC interrupts
+- interrupt-names: Should contain a list of interrupt names corresponding to
+	the interrupts in the interrupts property, if available.
+	Should be "macirq" for the main MAC IRQ
+- clocks: Must contain a phandle for each entry in clock-names.
+- clock-names: the name of the clock listed in the clocks property. These are
+	"axi", "apb", "mac_ext", "mac_parent", "ptp_ref", "ptp_parent", "ptp_top"
+	for MT2712 SoC
+- mac-address: See ethernet.txt in the same directory
+- phy-mode: See ethernet.txt in the same directory
+
+Example:
+
+	eth: ethernet@1101c000 {
+		compatible = "mediatek,mt2712-gmac";
+		reg = <0 0x1101c000 0 0x1300>;
+		interrupts = <GIC_SPI 237 IRQ_TYPE_LEVEL_LOW>;
+		interrupt-names = "macirq";
+		phy-mode ="rgmii-id";
+		mac-address = [00 55 7b b5 7d f7];
+		clock-names = "axi",
+			      "apb",
+			      "mac_ext",
+			      "mac_parent",
+			      "ptp_ref",
+			      "ptp_parent",
+			      "ptp_top";
+		clocks = <&pericfg CLK_PERI_GMAC>,
+			 <&pericfg CLK_PERI_GMAC_PCLK>,
+			 <&topckgen CLK_TOP_ETHER_125M_SEL>,
+			 <&topckgen CLK_TOP_ETHERPLL_125M>,
+			 <&topckgen CLK_TOP_ETHER_50M_SEL>,
+			 <&topckgen CLK_TOP_APLL1_D3>,
+			 <&topckgen CLK_TOP_APLL1>;
+		snps,txpbl = <32>;
+		snps,rxpbl = <32>;
+		snps,reset-gpio = <&pio 87 GPIO_ACTIVE_LOW>;
+		snps,reset-active-low;
+	};
-- 
1.7.9.5

Re: [v2, PATCH 0/2] add Ethernet driver support for mt2712

[biao huang]

On Mon, 2018-10-29 at 11:04 +0800, Biao Huang wrote:
> The mt2712 Ethernet IP is from Synopsys, so extend stmmac to support mt2712.
> 
sorry, I forgot to add detail descriptions for these patches.

changes in v2:
--since STMMAC driver applied to most Synopsys ip Ethernet, I add
dwmac-mediatek.c to support MediaTek mt2712 in v2.
--abandon Ethernet drivers proposed in v1.
--rewrite dt-binding document for v2 driver.

Best Regards!

Re: [PATCH 1/2] net:stmmac: dwmac-mediatek: add support for mt2712

[Corentin Labbe]

Hello
I have some minor comments below

On Mon, Oct 29, 2018 at 11:04:53AM +0800, Biao Huang wrote:
> Add Ethernet support for MediaTek SoCs from the mt2712 family
> 
> Signed-off-by: Biao Huang <biao.huang@mediatek.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/Kconfig        |    8 +
>  drivers/net/ethernet/stmicro/stmmac/Makefile       |    1 +
>  .../net/ethernet/stmicro/stmmac/dwmac-mediatek.c   |  364 ++++++++++++++++++++
>  3 files changed, 373 insertions(+)
>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index edf2036..76d779e 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -75,6 +75,14 @@ config DWMAC_LPC18XX
>  	---help---
>  	  Support for NXP LPC18xx/43xx DWMAC Ethernet.
>  
> +config DWMAC_MEDIATEK
> +	tristate "MediaTek MT27xx GMAC support"
> +	depends on OF

You should add something like && (COMPILE_TEST || ARCH_MEDIATEK)

[...]
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> new file mode 100644
> index 0000000..9ccf3a5
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> @@ -0,0 +1,364 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (c) 2018 MediaTek Inc.

Only SPDX can use the // comment style, the rest should use /**/


[...]
> +static int mt2712_set_interface(struct mediatek_dwmac_plat_data *plat)
> +{
> +	int rmii_rxc = plat->rmii_rxc ? RMII_CLK_SRC_RXC : 0;
> +	u32 intf_val = 0;
> +
> +	/* select phy interface in top control domain */
> +	switch (plat->phy_mode) {
> +	case PHY_INTERFACE_MODE_MII:
> +		intf_val |= PHY_INTF_MII_GMII;
> +		break;
> +	case PHY_INTERFACE_MODE_RMII:
> +		intf_val |= PHY_INTF_RMII;
> +		intf_val |= rmii_rxc;
> +		break;
> +	case PHY_INTERFACE_MODE_RGMII:
> +	case PHY_INTERFACE_MODE_RGMII_TXID:
> +	case PHY_INTERFACE_MODE_RGMII_RXID:
> +	case PHY_INTERFACE_MODE_RGMII_ID:
> +		intf_val |= PHY_INTF_RGMII;
> +		break;
> +	default:
> +		pr_err("phy interface not support\n");

I think you could use dev_err() instead.
And I think it is better spelled "not supported".


[...]
> +static int mediatek_dwmac_probe(struct platform_device *pdev)
> +{
> +	int ret = 0;
> +	struct plat_stmmacenet_data *plat_dat;
> +	struct stmmac_resources stmmac_res;
> +	struct mediatek_dwmac_plat_data *priv_plat;
> +
> +	priv_plat = devm_kzalloc(&pdev->dev, sizeof(*priv_plat), GFP_KERNEL);
> +	if (!priv_plat)
> +		return -ENOMEM;
> +
> +	priv_plat->variant = of_device_get_match_data(&pdev->dev);
> +	if (!priv_plat->variant) {
> +		dev_err(&pdev->dev, "Missing dwmac-mediatek variant\n");
> +		return -EINVAL;
> +	}
> +
> +	priv_plat->dev = &pdev->dev;
> +	priv_plat->np = pdev->dev.of_node;
> +	priv_plat->phy_mode = of_get_phy_mode(priv_plat->np);
> +
> +	ret = mediatek_dwmac_config_dt(priv_plat);
> +	if (ret)
> +		return ret;
> +
> +	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
> +	if (ret)
> +		return ret;
> +
> +	plat_dat = stmmac_probe_config_dt(pdev, &stmmac_res.mac);
> +	if (IS_ERR(plat_dat))
> +		return PTR_ERR(plat_dat);
> +
> +	plat_dat->interface = priv_plat->phy_mode;
> +	/* clk_csr_i = 250-300MHz & MDC = clk_csr_i/124 */
> +	plat_dat->clk_csr = 5;
> +	plat_dat->has_gmac4 = 1;
> +	plat_dat->has_gmac = 0;
> +	plat_dat->pmt = 0;
> +	plat_dat->maxmtu = 1500;

ETH_DATA_LEN ?

Regards

Re: [PATCH 1/2] net:stmmac: dwmac-mediatek: add support for mt2712

[Andrew Lunn]

> +static int mt2712_config_dt(struct mediatek_dwmac_plat_data *plat)
> +{
> +	u32 mac_timings[4];
> +
> +	plat->peri_regmap = syscon_regmap_lookup_by_compatible("mediatek,mt2712-pericfg");
> +	if (IS_ERR(plat->peri_regmap)) {
> +		dev_err(plat->dev, "Failed to get pericfg syscon\n");
> +		return PTR_ERR(plat->peri_regmap);
> +	}
> +
> +	if (!of_property_read_u32_array(plat->np, "mac-delay", mac_timings,
> +					ARRAY_SIZE(mac_timings))) {
> +		plat->mac_delay.tx_delay = mac_timings[0];
> +		plat->mac_delay.rx_delay = mac_timings[1];
> +		plat->mac_delay.tx_inv = mac_timings[2];
> +		plat->mac_delay.rx_inv = mac_timings[3];
> +	}
> +
> +	plat->fine_tune = of_property_read_bool(plat->np, "fine-tune");
> +
> +	plat->rmii_rxc = of_property_read_bool(plat->np, "rmii-rxc");

Please add document for these properties in the binding.

Ideally, you should reuse the binding that some of the other stmmac
glue layer uses. e.g. there is already allwinner,tx-delay-ps,
allwinner,rx-delay-ps.

       Thanks
              Andrew

Re: [PATCH 1/2] net:stmmac: dwmac-mediatek: add support for mt2712

[biao huang]

Thanks for your kindly comments.
On Mon, 2018-10-29 at 13:27 +0100, Andrew Lunn wrote:
> > +static int mt2712_config_dt(struct mediatek_dwmac_plat_data *plat)
> > +{
> > +	u32 mac_timings[4];
> > +
> > +	plat->peri_regmap = syscon_regmap_lookup_by_compatible("mediatek,mt2712-pericfg");
> > +	if (IS_ERR(plat->peri_regmap)) {
> > +		dev_err(plat->dev, "Failed to get pericfg syscon\n");
> > +		return PTR_ERR(plat->peri_regmap);
> > +	}
> > +
> > +	if (!of_property_read_u32_array(plat->np, "mac-delay", mac_timings,
> > +					ARRAY_SIZE(mac_timings))) {
> > +		plat->mac_delay.tx_delay = mac_timings[0];
> > +		plat->mac_delay.rx_delay = mac_timings[1];
> > +		plat->mac_delay.tx_inv = mac_timings[2];
> > +		plat->mac_delay.rx_inv = mac_timings[3];
> > +	}
> > +
> > +	plat->fine_tune = of_property_read_bool(plat->np, "fine-tune");
> > +
> > +	plat->rmii_rxc = of_property_read_bool(plat->np, "rmii-rxc");
> 
> Please add document for these properties in the binding.
ok, I forgot these properties.
> 
> Ideally, you should reuse the binding that some of the other stmmac
> glue layer uses. e.g. there is already allwinner,tx-delay-ps,
> allwinner,rx-delay-ps.
take it, seems that tx-delay/rx-delay will be more readable than
mac-delay in dts. will be changed in next patch.
> 
>        Thanks
>               Andrew

Re: [PATCH 1/2] net:stmmac: dwmac-mediatek: add support for mt2712

[biao huang]

Thanks for your nice comments.
On Mon, 2018-10-29 at 11:08 +0100, Corentin Labbe wrote:
> Hello
> I have some minor comments below
> 
> On Mon, Oct 29, 2018 at 11:04:53AM +0800, Biao Huang wrote:
> > Add Ethernet support for MediaTek SoCs from the mt2712 family
> > 
> > Signed-off-by: Biao Huang <biao.huang@mediatek.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/Kconfig        |    8 +
> >  drivers/net/ethernet/stmicro/stmmac/Makefile       |    1 +
> >  .../net/ethernet/stmicro/stmmac/dwmac-mediatek.c   |  364 ++++++++++++++++++++
> >  3 files changed, 373 insertions(+)
> >  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > index edf2036..76d779e 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > @@ -75,6 +75,14 @@ config DWMAC_LPC18XX
> >  	---help---
> >  	  Support for NXP LPC18xx/43xx DWMAC Ethernet.
> >  
> > +config DWMAC_MEDIATEK
> > +	tristate "MediaTek MT27xx GMAC support"
> > +	depends on OF
> 
> You should add something like && (COMPILE_TEST || ARCH_MEDIATEK)
ok, I'll add it in next patch.
> 
> [...]
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> > new file mode 100644
> > index 0000000..9ccf3a5
> > --- /dev/null
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> > @@ -0,0 +1,364 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +//
> > +// Copyright (c) 2018 MediaTek Inc.
> 
> Only SPDX can use the // comment style, the rest should use /**/
will fix it.
> 
> 
> [...]
> > +static int mt2712_set_interface(struct mediatek_dwmac_plat_data *plat)
> > +{
> > +	int rmii_rxc = plat->rmii_rxc ? RMII_CLK_SRC_RXC : 0;
> > +	u32 intf_val = 0;
> > +
> > +	/* select phy interface in top control domain */
> > +	switch (plat->phy_mode) {
> > +	case PHY_INTERFACE_MODE_MII:
> > +		intf_val |= PHY_INTF_MII_GMII;
> > +		break;
> > +	case PHY_INTERFACE_MODE_RMII:
> > +		intf_val |= PHY_INTF_RMII;
> > +		intf_val |= rmii_rxc;
> > +		break;
> > +	case PHY_INTERFACE_MODE_RGMII:
> > +	case PHY_INTERFACE_MODE_RGMII_TXID:
> > +	case PHY_INTERFACE_MODE_RGMII_RXID:
> > +	case PHY_INTERFACE_MODE_RGMII_ID:
> > +		intf_val |= PHY_INTF_RGMII;
> > +		break;
> > +	default:
> > +		pr_err("phy interface not support\n");
> 
> I think you could use dev_err() instead.
> And I think it is better spelled "not supported".
yes, take it.
> 
> 
> [...]
> > +static int mediatek_dwmac_probe(struct platform_device *pdev)
> > +{
> > +	int ret = 0;
> > +	struct plat_stmmacenet_data *plat_dat;
> > +	struct stmmac_resources stmmac_res;
> > +	struct mediatek_dwmac_plat_data *priv_plat;
> > +
> > +	priv_plat = devm_kzalloc(&pdev->dev, sizeof(*priv_plat), GFP_KERNEL);
> > +	if (!priv_plat)
> > +		return -ENOMEM;
> > +
> > +	priv_plat->variant = of_device_get_match_data(&pdev->dev);
> > +	if (!priv_plat->variant) {
> > +		dev_err(&pdev->dev, "Missing dwmac-mediatek variant\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	priv_plat->dev = &pdev->dev;
> > +	priv_plat->np = pdev->dev.of_node;
> > +	priv_plat->phy_mode = of_get_phy_mode(priv_plat->np);
> > +
> > +	ret = mediatek_dwmac_config_dt(priv_plat);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
> > +	if (ret)
> > +		return ret;
> > +
> > +	plat_dat = stmmac_probe_config_dt(pdev, &stmmac_res.mac);
> > +	if (IS_ERR(plat_dat))
> > +		return PTR_ERR(plat_dat);
> > +
> > +	plat_dat->interface = priv_plat->phy_mode;
> > +	/* clk_csr_i = 250-300MHz & MDC = clk_csr_i/124 */
> > +	plat_dat->clk_csr = 5;
> > +	plat_dat->has_gmac4 = 1;
> > +	plat_dat->has_gmac = 0;
> > +	plat_dat->pmt = 0;
> > +	plat_dat->maxmtu = 1500;
> 
> ETH_DATA_LEN ?
how about getting maxmtu from device tree rather than assignment here?
> 
> Regards

Re: [PATCH 1/2] net:stmmac: dwmac-mediatek: add support for mt2712

[Andrew Lunn]

> > > +	plat_dat->interface = priv_plat->phy_mode;
> > > +	/* clk_csr_i = 250-300MHz & MDC = clk_csr_i/124 */
> > > +	plat_dat->clk_csr = 5;
> > > +	plat_dat->has_gmac4 = 1;
> > > +	plat_dat->has_gmac = 0;
> > > +	plat_dat->pmt = 0;
> > > +	plat_dat->maxmtu = 1500;
> > 
> > ETH_DATA_LEN ?
> how about getting maxmtu from device tree rather than assignment here?

Does it vary between devices? I expect it is a constant, determined by
the DMA hardware design. So no need to make it configurabale.

    Andrew

Re: [PATCH 2/2] dt-binding: mediatek-dwmac: add binding document for MediaTek MT2712 DWMAC

[biao huang]

Dear Rob,

Thanks for your comments.

On Mon, 2018-11-12 at 10:22 -0600, Rob Herring wrote:
> On Mon, Oct 29, 2018 at 11:04:54AM +0800, Biao Huang wrote:
> > The commit adds the device tree binding documentation for the MediaTek DWMAC
> > found on MediaTek MT2712.
> > 
> > Signed-off-by: Biao Huang <biao.huang@mediatek.com>
> > ---
> >  .../devicetree/bindings/net/mediatek-dwmac.txt     |   49 ++++++++++++++++++++
> >  1 file changed, 49 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/mediatek-dwmac.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/net/mediatek-dwmac.txt b/Documentation/devicetree/bindings/net/mediatek-dwmac.txt
> > new file mode 100644
> > index 0000000..8baf0e4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/mediatek-dwmac.txt
> > @@ -0,0 +1,49 @@
> > +MediaTek DWMAC glue layer controller
> > +
> > +This file documents platform glue layer for stmmac.
> > +Please see stmmac.txt for the other unchanged properties.
> > +
> > +The device node has following properties.
> > +
> > +Required properties:
> > +- compatible:  Should be "mediatek,mt2712-gmac" for MT2712 SoC
> > +- reg:  Address and length of the register set for the device
> > +- interrupts:  Should contain the MAC interrupts
> > +- interrupt-names: Should contain a list of interrupt names corresponding to
> > +	the interrupts in the interrupts property, if available.
> > +	Should be "macirq" for the main MAC IRQ
> > +- clocks: Must contain a phandle for each entry in clock-names.
> > +- clock-names: the name of the clock listed in the clocks property. These are
> > +	"axi", "apb", "mac_ext", "mac_parent", "ptp_ref", "ptp_parent", "ptp_top"
> > +	for MT2712 SoC
> 
> Clocks should represent the physical clocks connected to a block. Parent 
> clocks are not in that category.
There are two categories of physical clocks related to MAC.
1. axi/apb/ptp_top, these clks are from clk gates, so only on/off is
needed.
2. mac_ext/ptp_ref, these clks are from mux. If different source is
selected, the clk frequecy will be different. So it's necessary to set a
proper parent for them, i.e. mac_parent/ptp_parent.

How should I handle parent clocks if not defined here? Any examples I
can refer to?
> 
> > +- mac-address: See ethernet.txt in the same directory
> > +- phy-mode: See ethernet.txt in the same directory
> > +
> > +Example:
> > +
> > +	eth: ethernet@1101c000 {
> > +		compatible = "mediatek,mt2712-gmac";
> > +		reg = <0 0x1101c000 0 0x1300>;
> > +		interrupts = <GIC_SPI 237 IRQ_TYPE_LEVEL_LOW>;
> > +		interrupt-names = "macirq";
> > +		phy-mode ="rgmii-id";
> > +		mac-address = [00 55 7b b5 7d f7];
> > +		clock-names = "axi",
> > +			      "apb",
> > +			      "mac_ext",
> > +			      "mac_parent",
> > +			      "ptp_ref",
> > +			      "ptp_parent",
> > +			      "ptp_top";
> > +		clocks = <&pericfg CLK_PERI_GMAC>,
> > +			 <&pericfg CLK_PERI_GMAC_PCLK>,
> > +			 <&topckgen CLK_TOP_ETHER_125M_SEL>,
> > +			 <&topckgen CLK_TOP_ETHERPLL_125M>,
> > +			 <&topckgen CLK_TOP_ETHER_50M_SEL>,
> > +			 <&topckgen CLK_TOP_APLL1_D3>,
> > +			 <&topckgen CLK_TOP_APLL1>;
> > +		snps,txpbl = <32>;
> > +		snps,rxpbl = <32>;
> > +		snps,reset-gpio = <&pio 87 GPIO_ACTIVE_LOW>;
> > +		snps,reset-active-low;
> > +	};
> > -- 
> > 1.7.9.5
> > 
> 
Best Regards!
Biao