2018-11-16 09:20:26

by Biao Huang (黄彪)

[permalink] [raw]
Subject: [v3, PATCH 1/2] net:stmmac: dwmac-mediatek: add support for mt2712

Add Ethernet support for MediaTek SoCs from the mt2712 family

Change-Id: Id3c535627088227793bd36405994edf2dc765e6a
Signed-off-by: Biao Huang <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/Kconfig | 8 +
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
.../net/ethernet/stmicro/stmmac/dwmac-mediatek.c | 383 ++++++++++++++++++++
3 files changed, 392 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..28a7a28 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 && (ARCH_MEDIATEK || COMPILE_TEST)
+ 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..a80b3e0
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
@@ -0,0 +1,383 @@
+// 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);
+ u32 rx_delay_max;
+ u32 tx_delay_max;
+};
+
+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:
+ dev_err(plat->dev, "phy interface not supported\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->rx_delay ?
+ PHY_DLY_TXC_ENABLE : 0;
+ delay_val |= (mac_delay->rx_delay <<
+ PHY_DLY_TXC_SHIFT) & PHY_DLY_TXC_STAGES;
+ delay_val |= mac_delay->rx_inv ? PHY_DLY_TXC_INV : 0;
+ fine_val |= mac_delay->tx_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:
+ dev_err(plat->dev, "phy interface not supported\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 tx_delay, rx_delay;
+
+ 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(plat->np, "tx-delay", &tx_delay)) {
+ if (tx_delay < plat->variant->tx_delay_max) {
+ plat->mac_delay.tx_delay = tx_delay;
+ } else {
+ dev_err(plat->dev, "Invalid TX clock delay: %d\n", tx_delay);
+ return -EINVAL;
+ }
+ }
+
+ if (!of_property_read_u32(plat->np, "rx-delay", &rx_delay)) {
+ if (rx_delay < plat->variant->rx_delay_max) {
+ plat->mac_delay.rx_delay = rx_delay;
+ } else {
+ dev_err(plat->dev, "Invalid RX clock delay: %d\n", rx_delay);
+ return -EINVAL;
+ }
+ }
+
+ plat->mac_delay.tx_inv = of_property_read_bool(plat->np, "txc-inverse");
+
+ plat->mac_delay.rx_inv = of_property_read_bool(plat->np, "rxc-inverse");
+
+ 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,
+ .rx_delay_max = 32,
+ .tx_delay_max = 32,
+};
+
+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 = ETH_DATA_LEN;
+ 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



2018-11-16 09:20:01

by Biao Huang (黄彪)

[permalink] [raw]
Subject: [v3, PATCH 2/2] dt-binding: mediatek-dwmac: add binding document for MediaTek MT2712 DWMAC

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

Change-Id: I3728666bf65927164bd82fa8dddb90df8270bd44
Signed-off-by: Biao Huang <[email protected]>
---
.../devicetree/bindings/net/mediatek-dwmac.txt | 77 ++++++++++++++++++++
1 file changed, 77 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..7fd56e0
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/mediatek-dwmac.txt
@@ -0,0 +1,77 @@
+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
+
+Optional properties:
+- tx-delay: TX clock delay macro value. Range is 0~31. Default is 0.
+ It should be defined for rgmii/rgmii-rxid/mii interface.
+- rx-delay: RX clock delay macro value. Range is 0~31. Default is 0.
+ It should be defined for rgmii/rgmii-txid/mii/rmii interface.
+- fine-tune: This property will select coarse-tune delay or fine delay
+ for rgmii interface.
+ If fine-tune delay is enabled, tx-delay/rx-delay is 170+/-50ps
+ per stage.
+ Else coarse-tune delay is enabled, tx-delay/rx-delay is 0.55+/-0.2ns
+ per stage.
+ This property do not apply to non-rgmii PHYs.
+ Only coarse-tune delay is supported for mii/rmii PHYs.
+- rmii-rxc: Reference clock of rmii is from external PHYs,
+ and it can be connected to TXC or RXC pin on MT2712 SoC.
+ If ref_clk <--> TXC, disable it.
+ Else ref_clk <--> RXC, enable it.
+- txc-inverse: Inverse tx clock for mii/rgmii.
+ Inverse tx clock inside MAC relative to reference clock for rmii,
+ and it rarely happen.
+- rxc-inverse: Inverse rx clock for mii/rgmii interfaces.
+ Inverse reference clock for rmii.
+
+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;
+ tx-delay = <9>;
+ rx-delay = <9>;
+ fine-tune;
+ rmii-rxc;
+ txc-inverse;
+ rxc-inverse;
+ };
--
1.7.9.5


2018-11-17 00:23:58

by Sean Wang

[permalink] [raw]
Subject: Re: [v3, PATCH 2/2] dt-binding: mediatek-dwmac: add binding document for MediaTek MT2712 DWMAC

On Fri, Nov 16, 2018 at 1:19 AM Biao Huang <[email protected]> wrote:
>
> The commit adds the device tree binding documentation for the MediaTek DWMAC
> found on MediaTek MT2712.
>
> Change-Id: I3728666bf65927164bd82fa8dddb90df8270bd44

Drop change-id

> Signed-off-by: Biao Huang <[email protected]>
> ---
> .../devicetree/bindings/net/mediatek-dwmac.txt | 77 ++++++++++++++++++++
> 1 file changed, 77 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..7fd56e0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/mediatek-dwmac.txt
> @@ -0,0 +1,77 @@
> +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

About not including these parent clocks to the controller, you can
refer to assigned-clocks, assigned-clock-parents, assigned-clock-rates
noted in Documentation/devicetree/bindings/clock/clock-bindings.txt to
determine what speed these MUXs should be run at and see [1] as the
example how applied in dts.

[1]
https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/mt7623.dtsi#L660

> +- mac-address: See ethernet.txt in the same directory
> +- phy-mode: See ethernet.txt in the same directory
> +
> +Optional properties:
> +- tx-delay: TX clock delay macro value. Range is 0~31. Default is 0.
> + It should be defined for rgmii/rgmii-rxid/mii interface.
> +- rx-delay: RX clock delay macro value. Range is 0~31. Default is 0.
> + It should be defined for rgmii/rgmii-txid/mii/rmii interface.
> +- fine-tune: This property will select coarse-tune delay or fine delay
> + for rgmii interface.
what is the property's type?

> + If fine-tune delay is enabled, tx-delay/rx-delay is 170+/-50ps
> + per stage.
> + Else coarse-tune delay is enabled, tx-delay/rx-delay is 0.55+/-0.2ns
> + per stage.
> + This property do not apply to non-rgmii PHYs.
> + Only coarse-tune delay is supported for mii/rmii PHYs.
> +- rmii-rxc: Reference clock of rmii is from external PHYs,
what is the property's type?

> + and it can be connected to TXC or RXC pin on MT2712 SoC.
> + If ref_clk <--> TXC, disable it.
> + Else ref_clk <--> RXC, enable it.
> +- txc-inverse: Inverse tx clock for mii/rgmii.
what is the property's type?

> + Inverse tx clock inside MAC relative to reference clock for rmii,
> + and it rarely happen.
> +- rxc-inverse: Inverse rx clock for mii/rgmii interfaces.
what is the property's type?

> + Inverse reference clock for rmii.

If these optional properties look like generic enough, it would be
good that place them to stmmac.txt. Otherwise, they should be added
"mediatek," as the prefix string for these vendor-specific things.

> +
> +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;
> + tx-delay = <9>;
> + rx-delay = <9>;
> + fine-tune;
> + rmii-rxc;
> + txc-inverse;
> + rxc-inverse;
> + };
> --
> 1.7.9.5
>
>
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

2018-11-17 01:04:53

by Sean Wang

[permalink] [raw]
Subject: Re: [v3, PATCH 1/2] net:stmmac: dwmac-mediatek: add support for mt2712

wi
On Fri, Nov 16, 2018 at 1:19 AM Biao Huang <[email protected]> wrote:
>
> Add Ethernet support for MediaTek SoCs from the mt2712 family
>
> Change-Id: Id3c535627088227793bd36405994edf2dc765e6a

Drop change-Id

> Signed-off-by: Biao Huang <[email protected]>
> ---
> drivers/net/ethernet/stmicro/stmmac/Kconfig | 8 +
> drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
> .../net/ethernet/stmicro/stmmac/dwmac-mediatek.c | 383 ++++++++++++++++++++
> 3 files changed, 392 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..28a7a28 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 && (ARCH_MEDIATEK || COMPILE_TEST)
> + 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..a80b3e0
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> @@ -0,0 +1,383 @@
> +// 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);

Each variant usually can reuse the same config_dt, enable/disable_clks function.

> + u32 rx_delay_max;
> + u32 tx_delay_max;
> +};
> +
> +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)

Kconfig indicates it's a mt27xx driver. so it's good if there is a
more generic prefix.

> +{
> + 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:
> + dev_err(plat->dev, "phy interface not supported\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->rx_delay ?
> + PHY_DLY_TXC_ENABLE : 0;
> + delay_val |= (mac_delay->rx_delay <<
> + PHY_DLY_TXC_SHIFT) & PHY_DLY_TXC_STAGES;
> + delay_val |= mac_delay->rx_inv ? PHY_DLY_TXC_INV : 0;
> + fine_val |= mac_delay->tx_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:
> + dev_err(plat->dev, "phy interface not supported\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]);
> + }

can we use devm_clk_bulk_get?

> +
> + 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;
> + }

can we use clk_bulk_enable?

> +
> + 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;
> +

I guess clk_set_parent can be trimmed if we involve assigned-clocks,
assigned-clock-parents, assigned-clock-rates in the dts

> + return 0;
> +
> +err_disable_clks:
> + while (--clk >= 0)
> + clk_disable_unprepare(plat->clks[clk]);
> +

clk_bulk_enable would take care about the failed case

> + 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]);
> +}

can we use clk_bulk_disable ?

> +
> +static int mt2712_config_dt(struct mediatek_dwmac_plat_data *plat)
> +{
> + u32 tx_delay, rx_delay;
> +
> + plat->peri_regmap = syscon_regmap_lookup_by_compatible("mediatek,mt2712-pericfg");

the driver would be not generic enough if we assigned a specific
string into here.
how about apply syscon_regmap_lookup_by_phandle in here?

> + 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(plat->np, "tx-delay", &tx_delay)) {
> + if (tx_delay < plat->variant->tx_delay_max) {
> + plat->mac_delay.tx_delay = tx_delay;
> + } else {
> + dev_err(plat->dev, "Invalid TX clock delay: %d\n", tx_delay);
> + return -EINVAL;
> + }
> + }
> +
> + if (!of_property_read_u32(plat->np, "rx-delay", &rx_delay)) {
> + if (rx_delay < plat->variant->rx_delay_max) {
> + plat->mac_delay.rx_delay = rx_delay;
> + } else {
> + dev_err(plat->dev, "Invalid RX clock delay: %d\n", rx_delay);
> + return -EINVAL;
> + }
> + }
> +
> + plat->mac_delay.tx_inv = of_property_read_bool(plat->np, "txc-inverse");
> +
can we drop the empty line?

> + plat->mac_delay.rx_inv = of_property_read_bool(plat->np, "rxc-inverse");
> +
can we drop the empty line?

> + plat->fine_tune = of_property_read_bool(plat->np, "fine-tune");
> +
can we drop the empty line?

> + 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,
> + .rx_delay_max = 32,
> + .tx_delay_max = 32,
> +};

I thought we add the variant structure when the SoC variant is really in.

> +
> +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;

sorting these declarations into reverse Xmas tree seems to be good

> +
> + 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 = ETH_DATA_LEN;
> + 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
>
>
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

2018-11-17 14:57:08

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [v3, PATCH 2/2] dt-binding: mediatek-dwmac: add binding document for MediaTek MT2712 DWMAC

On Fri, Nov 16, 2018 at 05:18:46PM +0800, Biao Huang wrote:
> The commit adds the device tree binding documentation for the MediaTek DWMAC
> found on MediaTek MT2712.
>
> Change-Id: I3728666bf65927164bd82fa8dddb90df8270bd44
> Signed-off-by: Biao Huang <[email protected]>
> ---
> .../devicetree/bindings/net/mediatek-dwmac.txt | 77 ++++++++++++++++++++
> 1 file changed, 77 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..7fd56e0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/mediatek-dwmac.txt
> @@ -0,0 +1,77 @@
> +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

How many?

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

> +- mac-address: See ethernet.txt in the same directory
> +- phy-mode: See ethernet.txt in the same directory
> +
> +Optional properties:
> +- tx-delay: TX clock delay macro value. Range is 0~31. Default is 0.
> + It should be defined for rgmii/rgmii-rxid/mii interface.
> +- rx-delay: RX clock delay macro value. Range is 0~31. Default is 0.
> + It should be defined for rgmii/rgmii-txid/mii/rmii interface.
> +- fine-tune: This property will select coarse-tune delay or fine delay
> + for rgmii interface.
> + If fine-tune delay is enabled, tx-delay/rx-delay is 170+/-50ps
> + per stage.
> + Else coarse-tune delay is enabled, tx-delay/rx-delay is 0.55+/-0.2ns
> + per stage.
> + This property do not apply to non-rgmii PHYs.
> + Only coarse-tune delay is supported for mii/rmii PHYs.

Perhaps the delays should be in ps and the driver can figure out
fine-tune or not based on the value.

> +- rmii-rxc: Reference clock of rmii is from external PHYs,
> + and it can be connected to TXC or RXC pin on MT2712 SoC.
> + If ref_clk <--> TXC, disable it.
> + Else ref_clk <--> RXC, enable it.
> +- txc-inverse: Inverse tx clock for mii/rgmii.
> + Inverse tx clock inside MAC relative to reference clock for rmii,
> + and it rarely happen.
> +- rxc-inverse: Inverse rx clock for mii/rgmii interfaces.
> + Inverse reference clock for rmii.

These should all have vendor prefixes. 'snps' if these are all standard
GMAC controls or 'mediatek' if Mediatek specific.

> +
> +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;
> + tx-delay = <9>;
> + rx-delay = <9>;
> + fine-tune;
> + rmii-rxc;
> + txc-inverse;
> + rxc-inverse;
> + };
> --
> 1.7.9.5
>

2018-11-19 02:15:34

by Biao Huang (黄彪)

[permalink] [raw]
Subject: Re: [v3, PATCH 2/2] dt-binding: mediatek-dwmac: add binding document for MediaTek MT2712 DWMAC

Dear Sean,

Thanks for your detailed comments~

On Sat, 2018-11-17 at 08:21 +0800, Sean Wang wrote:
> On Fri, Nov 16, 2018 at 1:19 AM Biao Huang <[email protected]> wrote:
> >
> > The commit adds the device tree binding documentation for the MediaTek DWMAC
> > found on MediaTek MT2712.
> >
> > Change-Id: I3728666bf65927164bd82fa8dddb90df8270bd44
>
> Drop change-id
sorry, I forgot it. will remove in next version.
>
> > Signed-off-by: Biao Huang <[email protected]>
> > ---
> > .../devicetree/bindings/net/mediatek-dwmac.txt | 77 ++++++++++++++++++++
> > 1 file changed, 77 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..7fd56e0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/mediatek-dwmac.txt
> > @@ -0,0 +1,77 @@
> > +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
>
> About not including these parent clocks to the controller, you can
> refer to assigned-clocks, assigned-clock-parents, assigned-clock-rates
> noted in Documentation/devicetree/bindings/clock/clock-bindings.txt to
> determine what speed these MUXs should be run at and see [1] as the
> example how applied in dts.
>
> [1]
> https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/mt7623.dtsi#L660
>
Got it, I'll remove parent info to dts.
> > +- mac-address: See ethernet.txt in the same directory
> > +- phy-mode: See ethernet.txt in the same directory
> > +
> > +Optional properties:
> > +- tx-delay: TX clock delay macro value. Range is 0~31. Default is 0.
> > + It should be defined for rgmii/rgmii-rxid/mii interface.
> > +- rx-delay: RX clock delay macro value. Range is 0~31. Default is 0.
> > + It should be defined for rgmii/rgmii-txid/mii/rmii interface.
> > +- fine-tune: This property will select coarse-tune delay or fine delay
> > + for rgmii interface.
> what is the property's type?
>
ok, property type info will be added in next version.
> > + If fine-tune delay is enabled, tx-delay/rx-delay is 170+/-50ps
> > + per stage.
> > + Else coarse-tune delay is enabled, tx-delay/rx-delay is 0.55+/-0.2ns
> > + per stage.
> > + This property do not apply to non-rgmii PHYs.
> > + Only coarse-tune delay is supported for mii/rmii PHYs.
> > +- rmii-rxc: Reference clock of rmii is from external PHYs,
> what is the property's type?
>
got it.
> > + and it can be connected to TXC or RXC pin on MT2712 SoC.
> > + If ref_clk <--> TXC, disable it.
> > + Else ref_clk <--> RXC, enable it.
> > +- txc-inverse: Inverse tx clock for mii/rgmii.
> what is the property's type?
>
> > + Inverse tx clock inside MAC relative to reference clock for rmii,
> > + and it rarely happen.
> > +- rxc-inverse: Inverse rx clock for mii/rgmii interfaces.
> what is the property's type?
>
got it.
> > + Inverse reference clock for rmii.
>
> If these optional properties look like generic enough, it would be
> good that place them to stmmac.txt. Otherwise, they should be added
> "mediatek," as the prefix string for these vendor-specific things.
>
it's mediatek-specific, and the prefix string will be added in next
version.
> > +
> > +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;
> > + tx-delay = <9>;
> > + rx-delay = <9>;
> > + fine-tune;
> > + rmii-rxc;
> > + txc-inverse;
> > + rxc-inverse;
> > + };
> > --
> > 1.7.9.5
> >
> >
> > _______________________________________________
> > Linux-mediatek mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-mediatek



2018-11-19 02:44:39

by Biao Huang (黄彪)

[permalink] [raw]
Subject: Re: [v3, PATCH 1/2] net:stmmac: dwmac-mediatek: add support for mt2712

Hi Sean,
Thanks for your comments.
I'll send a more general version soon.

On Sat, 2018-11-17 at 09:03 +0800, Sean Wang wrote:
> wi
> On Fri, Nov 16, 2018 at 1:19 AM Biao Huang <[email protected]> wrote:
> >
> > Add Ethernet support for MediaTek SoCs from the mt2712 family
> >
> > Change-Id: Id3c535627088227793bd36405994edf2dc765e6a
>
> Drop change-Id
>
OK, I forgot it.
> > Signed-off-by: Biao Huang <[email protected]>
> > ---
> > drivers/net/ethernet/stmicro/stmmac/Kconfig | 8 +
> > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
> > .../net/ethernet/stmicro/stmmac/dwmac-mediatek.c | 383 ++++++++++++++++++++
> > 3 files changed, 392 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..28a7a28 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 && (ARCH_MEDIATEK || COMPILE_TEST)
> > + 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..a80b3e0
> > --- /dev/null
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> > @@ -0,0 +1,383 @@
> > +// 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);
>
> Each variant usually can reuse the same config_dt, enable/disable_clks function.
>
ok, I'll try to make it more general.
> > + u32 rx_delay_max;
> > + u32 tx_delay_max;
> > +};
> > +
> > +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)
>
> Kconfig indicates it's a mt27xx driver. so it's good if there is a
> more generic prefix.
>
in this version, mt2712_set_interface is only for mt2712.
I'll try a more general solution.
> > +{
> > + 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:
> > + dev_err(plat->dev, "phy interface not supported\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->rx_delay ?
> > + PHY_DLY_TXC_ENABLE : 0;
> > + delay_val |= (mac_delay->rx_delay <<
> > + PHY_DLY_TXC_SHIFT) & PHY_DLY_TXC_STAGES;
> > + delay_val |= mac_delay->rx_inv ? PHY_DLY_TXC_INV : 0;
> > + fine_val |= mac_delay->tx_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:
> > + dev_err(plat->dev, "phy interface not supported\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]);
> > + }
>
> can we use devm_clk_bulk_get?
>
yes, will change to devm_clk_bulk_xx API in next version.
> > +
> > + 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;
> > + }
>
> can we use clk_bulk_enable?
>
yes
> > +
> > + 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;
> > +
>
> I guess clk_set_parent can be trimmed if we involve assigned-clocks,
> assigned-clock-parents, assigned-clock-rates in the dts
>
yes.
> > + return 0;
> > +
> > +err_disable_clks:
> > + while (--clk >= 0)
> > + clk_disable_unprepare(plat->clks[clk]);
> > +
>
> clk_bulk_enable would take care about the failed case
>
Got it.
> > + 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]);
> > +}
>
> can we use clk_bulk_disable ?
>
yes
> > +
> > +static int mt2712_config_dt(struct mediatek_dwmac_plat_data *plat)
> > +{
> > + u32 tx_delay, rx_delay;
> > +
> > + plat->peri_regmap = syscon_regmap_lookup_by_compatible("mediatek,mt2712-pericfg");
>
> the driver would be not generic enough if we assigned a specific
> string into here.
> how about apply syscon_regmap_lookup_by_phandle in here?
>
ok, I'll try it.
> > + 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(plat->np, "tx-delay", &tx_delay)) {
> > + if (tx_delay < plat->variant->tx_delay_max) {
> > + plat->mac_delay.tx_delay = tx_delay;
> > + } else {
> > + dev_err(plat->dev, "Invalid TX clock delay: %d\n", tx_delay);
> > + return -EINVAL;
> > + }
> > + }
> > +
> > + if (!of_property_read_u32(plat->np, "rx-delay", &rx_delay)) {
> > + if (rx_delay < plat->variant->rx_delay_max) {
> > + plat->mac_delay.rx_delay = rx_delay;
> > + } else {
> > + dev_err(plat->dev, "Invalid RX clock delay: %d\n", rx_delay);
> > + return -EINVAL;
> > + }
> > + }
> > +
> > + plat->mac_delay.tx_inv = of_property_read_bool(plat->np, "txc-inverse");
> > +
> can we drop the empty line?
>
yes
> > + plat->mac_delay.rx_inv = of_property_read_bool(plat->np, "rxc-inverse");
> > +
> can we drop the empty line?
>
yes
> > + plat->fine_tune = of_property_read_bool(plat->np, "fine-tune");
> > +
> can we drop the empty line?
>
yes
> > + 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,
> > + .rx_delay_max = 32,
> > + .tx_delay_max = 32,
> > +};
>
> I thought we add the variant structure when the SoC variant is really in.
>
ok.
> > +
> > +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;
>
> sorting these declarations into reverse Xmas tree seems to be good
>
good tips.
> > +
> > + 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 = ETH_DATA_LEN;
> > + 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
> >
> >
> > _______________________________________________
> > Linux-mediatek mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-mediatek



2018-11-19 03:00:59

by Biao Huang (黄彪)

[permalink] [raw]
Subject: Re: [v3, PATCH 2/2] dt-binding: mediatek-dwmac: add binding document for MediaTek MT2712 DWMAC

Hi Rob,
Thanks for your comments.
On Sat, 2018-11-17 at 22:56 +0800, Rob Herring wrote:
> On Fri, Nov 16, 2018 at 05:18:46PM +0800, Biao Huang wrote:
> > The commit adds the device tree binding documentation for the MediaTek DWMAC
> > found on MediaTek MT2712.
> >
> > Change-Id: I3728666bf65927164bd82fa8dddb90df8270bd44
> > Signed-off-by: Biao Huang <[email protected]>
> > ---
> > .../devicetree/bindings/net/mediatek-dwmac.txt | 77 ++++++++++++++++++++
> > 1 file changed, 77 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..7fd56e0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/mediatek-dwmac.txt
> > @@ -0,0 +1,77 @@
> > +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
>
> How many?
>
the common stmmac driver will parse interrupt-name "macirq",
so even only one interrupt is used in mediatek dwmac design,
the interrupt-names is still remained in device tree.
> > +- 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.
>
Got it. assigned-clocks/assigned-clocks-parents properties can handle
it.
> > +- mac-address: See ethernet.txt in the same directory
> > +- phy-mode: See ethernet.txt in the same directory
> > +
> > +Optional properties:
> > +- tx-delay: TX clock delay macro value. Range is 0~31. Default is 0.
> > + It should be defined for rgmii/rgmii-rxid/mii interface.
> > +- rx-delay: RX clock delay macro value. Range is 0~31. Default is 0.
> > + It should be defined for rgmii/rgmii-txid/mii/rmii interface.
> > +- fine-tune: This property will select coarse-tune delay or fine delay
> > + for rgmii interface.
> > + If fine-tune delay is enabled, tx-delay/rx-delay is 170+/-50ps
> > + per stage.
> > + Else coarse-tune delay is enabled, tx-delay/rx-delay is 0.55+/-0.2ns
> > + per stage.
> > + This property do not apply to non-rgmii PHYs.
> > + Only coarse-tune delay is supported for mii/rmii PHYs.
>
> Perhaps the delays should be in ps and the driver can figure out
> fine-tune or not based on the value.
>
the delay time in mediatek dwmac design is not so accurate,
the current mt2712 and the following ICs will not use the same delay
design, but will use stages to indicate different delay time.
so, maybe "mediatek,tx-delay" represent the delay stage is a good
choice.
> > +- rmii-rxc: Reference clock of rmii is from external PHYs,
> > + and it can be connected to TXC or RXC pin on MT2712 SoC.
> > + If ref_clk <--> TXC, disable it.
> > + Else ref_clk <--> RXC, enable it.
> > +- txc-inverse: Inverse tx clock for mii/rgmii.
> > + Inverse tx clock inside MAC relative to reference clock for rmii,
> > + and it rarely happen.
> > +- rxc-inverse: Inverse rx clock for mii/rgmii interfaces.
> > + Inverse reference clock for rmii.
>
> These should all have vendor prefixes. 'snps' if these are all standard
> GMAC controls or 'mediatek' if Mediatek specific.
>
Got it, will be modified in next version.
> > +
> > +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;
> > + tx-delay = <9>;
> > + rx-delay = <9>;
> > + fine-tune;
> > + rmii-rxc;
> > + txc-inverse;
> > + rxc-inverse;
> > + };
> > --
> > 1.7.9.5
> >