Thread (11 messages) 11 messages, 4 authors, 2018-11-27

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

flat view

From: Sean Wang <sean.wang@kernel.org>
Date: 2018-11-23 00:54:43
Also in: linux-arm-kernel, linux-mediatek, lkml, netdev

On Thu, Nov 22, 2018 at 2:29 AM Biao Huang [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Add Ethernet support for MediaTek SoCs from the mt2712 family

Signed-off-by: Biao Huang <redacted>
---
 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 324049e..6209cc1 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..dd8d4cc
--- /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 for mt2712 */
+#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)
+
+struct mac_delay_struct {
+       u32 tx_delay;
+       u32 rx_delay;
+       u32 tx_inv;
bool should be enough
+       u32 rx_inv;
bool should be enough
quoted hunk ↗ jump to hunk
+};
+
+struct mediatek_dwmac_plat_data {
+       const struct mediatek_dwmac_variant *variant;
+       struct mac_delay_struct mac_delay;
+       struct clk_bulk_data *clks;
+       struct device_node *np;
+       struct regmap *peri_regmap;
+       struct device *dev;
+       int fine_tune;
bool
+       int phy_mode;
+       int rmii_rxc;
bool
quoted hunk ↗ jump to hunk
+};
+
+struct mediatek_dwmac_variant {
+       int (*dwmac_set_phy_interface)(struct mediatek_dwmac_plat_data *plat);
+       int (*dwmac_set_delay)(struct mediatek_dwmac_plat_data *plat);
+
+       /* clock ids to be requested */
+       const char * const *clk_list;
+       int num_clks;
+
+       u32 dma_bit_mask;
+       u32 rx_delay_max;
+       u32 tx_delay_max;
+};
+
+/* list of clocks required for mac */
+static const char * const mt2712_dwmac_clk_l[] = {
+       "axi", "apb", "mac_main", "ptp_ref"
+};
+
+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;
how about putting into one line such as intf_val |= (PHY_INTF_RMII | rmii_rxc) ?
quoted hunk ↗ jump to hunk
+               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;
The same type declaration can be put into the one line
+
+       switch (plat->phy_mode) {
There exists some room for code optimization in the switch statement
such as PHY_INTERFACE_MODE_MII and PHY_INTERFACE_MODE_RGMII both are
almost the same and even the configuration for the other PHY modes can
reuse their partial setup. It appears to be better using a common way
to set up various PHY modes.
quoted hunk ↗ jump to hunk
+       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;
why is fine_val got from tx_inv?
quoted hunk ↗ jump to hunk
+               } 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;
ditto, why is fine_val got from tx_inv?
quoted hunk ↗ jump to hunk
+               }
+               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;
why is PHY_INTERFACE_MODE_RGMII_TXID applied with *_RXC_* register
bits, not with *_TXC_* bits? I'm a little confused about what path the
register PHY_DLY_RXC_* cause the effects to? MAC to PHY or PHY to MAC?
quoted hunk ↗ jump to hunk
+               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;
ditto, as the above quetion
quoted hunk ↗ jump to hunk
+               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 const struct mediatek_dwmac_variant mt2712_gmac_variant = {
+               .dwmac_set_phy_interface = mt2712_set_interface,
+               .dwmac_set_delay = mt2712_set_delay,
+               .clk_list = mt2712_dwmac_clk_l,
+               .num_clks = ARRAY_SIZE(mt2712_dwmac_clk_l),
+               .dma_bit_mask = 33,
+               .rx_delay_max = 32,
+               .tx_delay_max = 32,
+};
+
+static int mediatek_dwmac_config_dt(struct mediatek_dwmac_plat_data *plat)
+{
+       u32 tx_delay, rx_delay;
+
+       plat->peri_regmap = syscon_regmap_lookup_by_phandle(plat->np, "mediatek,pericfg");
+       if (IS_ERR(plat->peri_regmap)) {
+               dev_err(plat->dev, "Failed to get pericfg syscon\n");
+               return PTR_ERR(plat->peri_regmap);
+       }
+
+       plat->phy_mode = of_get_phy_mode(plat->np);
+       if (plat->phy_mode < 0) {
+               dev_err(plat->dev, "not find phy-mode\n");
+               return -EINVAL;
+       }
+
+       if (!of_property_read_u32(plat->np, "mediatek,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, "mediatek,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, "mediatek,txc-inverse");
+       plat->mac_delay.rx_inv = of_property_read_bool(plat->np, "mediatek,rxc-inverse");
+       plat->fine_tune = of_property_read_bool(plat->np, "mediatek,fine-tune");
+       plat->rmii_rxc = of_property_read_bool(plat->np, "mediatek,rmii-rxc");
+
+       return 0;
+}
+
+static int mediatek_dwmac_clk_init(struct mediatek_dwmac_plat_data *plat)
+{
+       const struct mediatek_dwmac_variant *variant = plat->variant;
+       int num = variant->num_clks;
+       int i;
put into the same line seems good
quoted hunk ↗ jump to hunk
+
+       plat->clks = devm_kcalloc(plat->dev, num, sizeof(*plat->clks), GFP_KERNEL);
+       if (!plat->clks)
+               return -ENOMEM;
+
+       for (i = 0; i < num; i++)
+               plat->clks[i].id = variant->clk_list[i];
+
+       return devm_clk_bulk_get(plat->dev, num, plat->clks);
+}
+
+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;
+       int ret = 0;
zero initialized seems unnecessary
quoted hunk ↗ jump to hunk
+
+       ret = dma_set_mask_and_coherent(plat->dev, DMA_BIT_MASK(variant->dma_bit_mask));
+       if (ret) {
+               dev_err(plat->dev, "No suitable DMA available, err = %d\n", ret);
+               return ret;
+       }
+
+       ret = variant->dwmac_set_phy_interface(plat);
+       if (ret) {
+               dev_err(plat->dev, "failed to set phy interface, err = %d\n", ret);
+               return ret;
+       }
+
+       ret = variant->dwmac_set_delay(plat);
+       if (ret) {
+               dev_err(plat->dev, "failed to set delay value, err = %d\n", ret);
+               return ret;
+       }
+
+       ret = clk_bulk_prepare_enable(variant->num_clks, plat->clks);
+       if (ret) {
+               dev_err(plat->dev, "failed to enable clks, err = %d\n", ret);
+               return ret;
+       }
+
+       return 0;
+}
+
+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;
+
+       clk_bulk_disable_unprepare(variant->num_clks, plat->clks);
+}
+
+static int mediatek_dwmac_probe(struct platform_device *pdev)
+{
+       struct mediatek_dwmac_plat_data *priv_plat;
+       struct plat_stmmacenet_data *plat_dat;
+       struct stmmac_resources stmmac_res;
+       int ret = 0;
zero initialized seems unnecessary
+
+       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;
+
+       ret = mediatek_dwmac_config_dt(priv_plat);
+       if (ret)
+               return ret;
+
+       ret = mediatek_dwmac_clk_init(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
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help