<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p>Dear Przemek,</p>
    <p>Thank you for your reply.<br>
    </p>
    <div class="moz-cite-prefix">Przemek Kitszel 於 12/18/2024 9:26 PM
      寫道:<br>
    </div>
    <blockquote type="cite"
      cite="mid:7a4f5769-0010-40fd-8bb7-a20f2725114f@intel.com">On
      12/18/24 12:44, Joey Lu wrote:
      <br>
      <blockquote type="cite">Add support for Gigabit Ethernet on
        Nuvoton MA35 series using dwmac driver.
        <br>
        <br>
        Signed-off-by: Joey Lu <a class="moz-txt-link-rfc2396E" href="mailto:a0987203069@gmail.com"><a0987203069@gmail.com></a>
        <br>
        ---
        <br>
          drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 ++
        <br>
          drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
        <br>
          .../ethernet/stmicro/stmmac/dwmac-nuvoton.c   | 182
        ++++++++++++++++++
        <br>
          3 files changed, 194 insertions(+)
        <br>
          create mode 100644
        drivers/net/ethernet/stmicro/stmmac/dwmac-nuvoton.c
        <br>
        <br>
        diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig
        b/drivers/net/ethernet/stmicro/stmmac/Kconfig
        <br>
        index 6658536a4e17..c8cbc0ec1311 100644
        <br>
        --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
        <br>
        +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
        <br>
        @@ -121,6 +121,17 @@ config DWMAC_MESON
        <br>
                the stmmac device driver. This driver is used for
        Meson6,
        <br>
                Meson8, Meson8b and GXBB SoCs.
        <br>
          +config DWMAC_NUVOTON
        <br>
        +    tristate "Nuvoton MA35 dwmac support"
        <br>
        +    default ARCH_MA35
        <br>
        +    depends on OF && (ARCH_MA35 || COMPILE_TEST)
        <br>
        +    select MFD_SYSCON
        <br>
        +    help
        <br>
        +      Support for Ethernet controller on Nuvoton MA35 series
        SoC.
        <br>
        +
        <br>
        +      This selects the Nuvoton MA35 series SoC glue layer
        support
        <br>
        +      for the stmmac device driver.
        <br>
        +
        <br>
          config DWMAC_QCOM_ETHQOS
        <br>
              tristate "Qualcomm ETHQOS support"
        <br>
              default ARCH_QCOM
        <br>
        diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile
        b/drivers/net/ethernet/stmicro/stmmac/Makefile
        <br>
        index 2389fd261344..9812b824459f 100644
        <br>
        --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
        <br>
        +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
        <br>
        @@ -19,6 +19,7 @@ obj-$(CONFIG_DWMAC_IPQ806X)    +=
        dwmac-ipq806x.o
        <br>
          obj-$(CONFIG_DWMAC_LPC18XX)    += dwmac-lpc18xx.o
        <br>
          obj-$(CONFIG_DWMAC_MEDIATEK)    += dwmac-mediatek.o
        <br>
          obj-$(CONFIG_DWMAC_MESON)    += dwmac-meson.o dwmac-meson8b.o
        <br>
        +obj-$(CONFIG_DWMAC_NUVOTON)    += dwmac-nuvoton.o
        <br>
          obj-$(CONFIG_DWMAC_QCOM_ETHQOS)    += dwmac-qcom-ethqos.o
        <br>
          obj-$(CONFIG_DWMAC_ROCKCHIP)    += dwmac-rk.o
        <br>
          obj-$(CONFIG_DWMAC_RZN1)    += dwmac-rzn1.o
        <br>
        diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-nuvoton.c
        b/drivers/net/ethernet/stmicro/stmmac/dwmac-nuvoton.c
        <br>
        new file mode 100644
        <br>
        index 000000000000..c5b8933c1f44
        <br>
        --- /dev/null
        <br>
        +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-nuvoton.c
        <br>
        @@ -0,0 +1,182 @@
        <br>
        +// SPDX-License-Identifier: GPL-2.0-only
        <br>
        +/*
        <br>
        + * Nuvoton DWMAC specific glue layer
        <br>
        + *
        <br>
        + * Copyright (C) 2024 Nuvoton Technology Corp.
        <br>
        + *
        <br>
        + * Author: Joey Lu <a class="moz-txt-link-rfc2396E" href="mailto:yclu4@nuvoton.com"><yclu4@nuvoton.com></a>
        <br>
        + */
        <br>
        +
        <br>
        +#include <linux/mfd/syscon.h>
        <br>
        +#include <linux/of_device.h>
        <br>
        +#include <linux/of_net.h>
        <br>
        +#include <linux/platform_device.h>
        <br>
        +#include <linux/regmap.h>
        <br>
        +#include <linux/stmmac.h>
        <br>
        +
        <br>
        +#include "stmmac.h"
        <br>
        +#include "stmmac_platform.h"
        <br>
        +
        <br>
        +#define REG_SYS_GMAC0MISCR  0x108
        <br>
        +#define REG_SYS_GMAC1MISCR  0x10C
        <br>
        +
        <br>
        +#define MISCR_RMII          BIT(0)
        <br>
        +
        <br>
        +/* 2000ps is mapped to 0 ~ 0xF */
        <br>
        +#define PATH_DELAY_DEC      134
        <br>
      </blockquote>
      <br>
      would be great to previx your macros by NVT_
      <br>
    </blockquote>
    Got it.<br>
    <blockquote type="cite"
      cite="mid:7a4f5769-0010-40fd-8bb7-a20f2725114f@intel.com">
      <br>
      why 134 and not 125?
      <br>
    </blockquote>
    <p>The interval is confirmed to be 134. The mapping is as follows:</p>
    <code>0000</code> = 0.00 ns<br>
    <code>0001</code> = 0.13 ns<br>
    <code>0010</code> = 0.27 ns<br>
    ...<br>
    <code>1111</code> = 2.00 ns
    <blockquote type="cite"
      cite="mid:7a4f5769-0010-40fd-8bb7-a20f2725114f@intel.com">
      <br>
      <blockquote type="cite">+#define TX_DELAY_OFFSET     16
        <br>
      </blockquote>
      <br>
      please remove and replace the usage point by FIELD_PREP()
      <br>
    </blockquote>
    Got it.<br>
    <blockquote type="cite"
      cite="mid:7a4f5769-0010-40fd-8bb7-a20f2725114f@intel.com">
      <br>
      <blockquote type="cite">+#define TX_DELAY_MASK       GENMASK(19,
        16)
        <br>
        +#define RX_DELAY_OFFSET     20
        <br>
      </blockquote>
      <br>
      ditto
      <br>
    </blockquote>
    Got it.<br>
    <blockquote type="cite"
      cite="mid:7a4f5769-0010-40fd-8bb7-a20f2725114f@intel.com">
      <br>
      <blockquote type="cite">+#define RX_DELAY_MASK       GENMASK(23,
        20)
        <br>
        +
        <br>
        +struct nvt_priv_data {
        <br>
        +    struct platform_device *pdev;
        <br>
        +    struct regmap *regmap;
        <br>
        +};
        <br>
        +
        <br>
        +static struct nvt_priv_data *
        <br>
        +nuvoton_gmac_setup(struct platform_device *pdev, struct
        plat_stmmacenet_data *plat)
        <br>
      </blockquote>
      <br>
      please stick to one previx for all functions, structs, and
      defines,
      <br>
      NVT/nvt looks good
      <br>
      s/nuvoton/nvt/
      <br>
    </blockquote>
    Okay. I will use nvt as the prefix
    <blockquote type="cite"
      cite="mid:7a4f5769-0010-40fd-8bb7-a20f2725114f@intel.com">
      <br>
      <blockquote type="cite">+{
        <br>
        +    struct device *dev = &pdev->dev;
        <br>
        +    struct nvt_priv_data *bsp_priv;
        <br>
        +    phy_interface_t phy_mode;
        <br>
        +    u32 tx_delay, rx_delay;
        <br>
        +    u32 macid, arg, reg;
        <br>
        +
        <br>
        +    bsp_priv = devm_kzalloc(dev, sizeof(*bsp_priv),
        GFP_KERNEL);
        <br>
        +    if (!bsp_priv)
        <br>
        +        return ERR_PTR(-ENOMEM);
        <br>
        +
        <br>
        +    bsp_priv->regmap =
        <br>
        +        syscon_regmap_lookup_by_phandle_args(dev->of_node,
        "nuvoton,sys", 1, &macid);
        <br>
        +    if (IS_ERR(bsp_priv->regmap)) {
        <br>
        +        dev_err_probe(dev, PTR_ERR(bsp_priv->regmap),
        "Failed to get sys register\n");
        <br>
        +        return ERR_PTR(-ENODEV);
        <br>
        +    }
        <br>
        +    if (macid > 1) {
        <br>
        +        dev_err_probe(dev, -EINVAL, "Invalid sys arguments\n");
        <br>
        +        return ERR_PTR(-EINVAL);
        <br>
        +    }
        <br>
        +
        <br>
        +    if (of_property_read_u32(dev->of_node,
        "tx-internal-delay-ps", &arg)) {
        <br>
        +        tx_delay = 0; /* Default value is 0 */
        <br>
      </blockquote>
      <br>
      please remove obvious comments
      <br>
    </blockquote>
    Got it.<br>
    <blockquote type="cite"
      cite="mid:7a4f5769-0010-40fd-8bb7-a20f2725114f@intel.com">
      <br>
      <blockquote type="cite">+    } else {
        <br>
        +        if (arg <= 2000) {
        <br>
        +            tx_delay = (arg == 2000) ? 0xF : (arg /
        PATH_DELAY_DEC);
        <br>
        +            dev_dbg(dev, "Set Tx path delay to 0x%x\n",
        tx_delay);
        <br>
        +        } else {
        <br>
        +            dev_err(dev, "Invalid Tx path delay argument.\n");
        <br>
        +            return ERR_PTR(-EINVAL);
        <br>
        +        }
        <br>
        +    }
        <br>
        +    if (of_property_read_u32(dev->of_node,
        "rx-internal-delay-ps", &arg)) {
        <br>
        +        rx_delay = 0; /* Default value is 0 */
        <br>
        +    } else {
        <br>
        +        if (arg <= 2000) {
        <br>
        +            rx_delay = (arg == 2000) ? 0xF : (arg /
        PATH_DELAY_DEC);
        <br>
        +            dev_dbg(dev, "Set Rx path delay to 0x%x\n",
        rx_delay);
        <br>
        +        } else {
        <br>
        +            dev_err(dev, "Invalid Rx path delay argument.\n");
        <br>
        +            return ERR_PTR(-EINVAL);
        <br>
        +        }
        <br>
        +    }
        <br>
        +
        <br>
        +    regmap_read(bsp_priv->regmap,
        <br>
        +            macid == 0 ? REG_SYS_GMAC0MISCR :
        REG_SYS_GMAC1MISCR, &reg);
        <br>
        +    reg &= ~(TX_DELAY_MASK | RX_DELAY_MASK);
        <br>
        +
        <br>
        +    if (of_get_phy_mode(pdev->dev.of_node, &phy_mode)) {
        <br>
        +        dev_err(dev, "missing phy mode property\n");
        <br>
        +        return ERR_PTR(-EINVAL);
        <br>
        +    }
        <br>
        +
        <br>
        +    switch (phy_mode) {
        <br>
        +    case PHY_INTERFACE_MODE_RGMII:
        <br>
        +    case PHY_INTERFACE_MODE_RGMII_ID:
        <br>
        +    case PHY_INTERFACE_MODE_RGMII_RXID:
        <br>
        +    case PHY_INTERFACE_MODE_RGMII_TXID:
        <br>
        +        reg &= ~MISCR_RMII;
        <br>
        +        break;
        <br>
        +    case PHY_INTERFACE_MODE_RMII:
        <br>
        +        reg |= MISCR_RMII;
        <br>
        +        break;
        <br>
        +    default:
        <br>
        +        dev_err(dev, "Unsupported phy-mode (%d)\n", phy_mode);
        <br>
        +        return ERR_PTR(-EINVAL);
        <br>
        +    }
        <br>
        +
        <br>
        +    if (!(reg & MISCR_RMII)) {
        <br>
        +        reg |= tx_delay << TX_DELAY_OFFSET;
        <br>
        +        reg |= rx_delay << RX_DELAY_OFFSET;
        <br>
        +    }
        <br>
        +
        <br>
        +    regmap_write(bsp_priv->regmap,
        <br>
        +             macid == 0 ? REG_SYS_GMAC0MISCR :
        REG_SYS_GMAC1MISCR, reg);
        <br>
        +
        <br>
        +    bsp_priv->pdev = pdev;
        <br>
        +
        <br>
        +    return bsp_priv;
        <br>
        +}
        <br>
        +
        <br>
        +static int nuvoton_gmac_probe(struct platform_device *pdev)
        <br>
        +{
        <br>
        +    struct plat_stmmacenet_data *plat_dat;
        <br>
        +    struct stmmac_resources stmmac_res;
        <br>
        +    int ret;
        <br>
        +
        <br>
        +    ret = stmmac_get_platform_resources(pdev, &stmmac_res);
        <br>
        +    if (ret)
        <br>
        +        return ret;
        <br>
        +
        <br>
        +    plat_dat = devm_stmmac_probe_config_dt(pdev,
        stmmac_res.mac);
        <br>
        +    if (IS_ERR(plat_dat))
        <br>
        +        return PTR_ERR(plat_dat);
        <br>
        +
        <br>
        +    /* Nuvoton DWMAC configs */
        <br>
        +    plat_dat->has_gmac = 1;
        <br>
        +    plat_dat->tx_fifo_size = 2048;
        <br>
        +    plat_dat->rx_fifo_size = 4096;
        <br>
        +    plat_dat->multicast_filter_bins = 0;
        <br>
        +    plat_dat->unicast_filter_entries = 8;
        <br>
        +    plat_dat->flags &= ~STMMAC_FLAG_USE_PHY_WOL;
        <br>
        +
        <br>
        +    plat_dat->bsp_priv = nuvoton_gmac_setup(pdev, plat_dat);
        <br>
      </blockquote>
      <br>
      would be great to extend plat_stmmacenet_data allocation to
      allocate
      <br>
      also the space for the priv data - but this is outside of the
      scope
      <br>
      of this patchset
      <br>
    </blockquote>
    <p>You're right, this is a misuse and will be corrected.</p>
    <blockquote type="cite"
      cite="mid:7a4f5769-0010-40fd-8bb7-a20f2725114f@intel.com">
      <br>
      <blockquote type="cite">+    if (IS_ERR(plat_dat->bsp_priv)) {
        <br>
        +        ret = PTR_ERR(plat_dat->bsp_priv);
        <br>
        +        return ret;
        <br>
      </blockquote>
      <br>
      just return PTR_ERR(...)
      <br>
    </blockquote>
    Got it.<br>
    <blockquote type="cite"
      cite="mid:7a4f5769-0010-40fd-8bb7-a20f2725114f@intel.com">
      <br>
      <blockquote type="cite">+    }
        <br>
        +
        <br>
        +    ret = stmmac_dvr_probe(&pdev->dev, plat_dat,
        &stmmac_res);
        <br>
        +    if (ret)
        <br>
        +        return ret;
        <br>
        +
        <br>
        +    /* The PMT flag is determined by the RWK property.
        <br>
        +     * However, our hardware is configured to support only MGK.
        <br>
        +     * This is an override on PMT to enable WoL capability.
        <br>
        +     */
        <br>
        +    plat_dat->pmt = 1;
        <br>
        +    device_set_wakeup_capable(&pdev->dev, 1);
        <br>
        +
        <br>
        +    return 0;
        <br>
        +}
        <br>
        +
        <br>
        +static const struct of_device_id nuvoton_dwmac_match[] = {
        <br>
        +    { .compatible = "nuvoton,ma35d1-dwmac"},
        <br>
        +    { }
        <br>
        +};
        <br>
        +MODULE_DEVICE_TABLE(of, nuvoton_dwmac_match);
        <br>
        +
        <br>
        +static struct platform_driver nuvoton_dwmac_driver = {
        <br>
        +    .probe  = nuvoton_gmac_probe,
        <br>
        +    .remove = stmmac_pltfr_remove,
        <br>
        +    .driver = {
        <br>
        +        .name           = "nuvoton-dwmac",
        <br>
        +        .pm        = &stmmac_pltfr_pm_ops,
        <br>
        +        .of_match_table = nuvoton_dwmac_match,
        <br>
        +    },
        <br>
        +};
        <br>
        +module_platform_driver(nuvoton_dwmac_driver);
        <br>
        +
        <br>
        +MODULE_AUTHOR("Joey Lu <a class="moz-txt-link-rfc2396E" href="mailto:yclu4@nuvoton.com"><yclu4@nuvoton.com></a>");
        <br>
        +MODULE_DESCRIPTION("Nuvoton DWMAC specific glue layer");
        <br>
        +MODULE_LICENSE("GPL v2");
        <br>
      </blockquote>
      <br>
    </blockquote>
    <p>Thanks!</p>
    <p>BR,</p>
    <p>Joey<br>
    </p>
  </body>
</html>