Re: [PATCH net-next v7 3/3] net: stmmac: dwmac-nuvoton: Add dwmac glue for Nuvoton MA35 family

[Joey Lu <a0987203069@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 13685 bytes --]

Dear Paul,

Thank you for the reply!

Paul Menzel 於 1/14/2025 9:49 AM 寫道:
> Dear Joey,
>
>
> Thank you for your patch.
>
> Am 13.01.25 um 00:54 schrieb Joey Lu:
>> Add support for Gigabit Ethernet on Nuvoton MA35 series using dwmac 
>> driver.
>
> It’d be great if you added the datasheet name and revision to the 
> commit message.

I will update the commit message in the next patch.

For reference, the datasheet is *MA35D1 Series Datasheet*.

>
> Also, please document how tested the driver. Maybe even paste new log 
> messages.

These are the kernel configurations for testing the MA35D1 GMAC driver: 
ARCH_MA35, STMMAC_PLATFORM, DWMAC_NUVOTON.

I'm not sure if this information is sufficient, so please provide some 
guidance on what else I should include to meet your requirements.

I will include the log messages at the end of the email.

>
>> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>> Signed-off-by: Joey Lu <a0987203069@gmail.com>
>
> As you use your company email address in the AUTHOR line below, please 
> also add that email address to the commit message (and maybe even as 
> the author).
>
I will update the AUTHOR to use my personal email address instead of the 
company email.
>> ---
>>   drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 ++
>>   drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
>>   .../ethernet/stmicro/stmmac/dwmac-nuvoton.c   | 179 ++++++++++++++++++
>>   3 files changed, 191 insertions(+)
>>   create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-nuvoton.c
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig 
>> b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> index 4cc85a36a1ab..2b424544cf6f 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> @@ -121,6 +121,17 @@ config DWMAC_MESON
>>         the stmmac device driver. This driver is used for Meson6,
>>         Meson8, Meson8b and GXBB SoCs.
>>   +config DWMAC_NUVOTON
>> +    tristate "Nuvoton MA35 dwmac support"
>> +    default ARCH_MA35
>> +    depends on OF && (ARCH_MA35 || COMPILE_TEST)
>> +    select MFD_SYSCON
>> +    help
>> +      Support for Ethernet controller on Nuvoton MA35 series SoC.
>> +
>> +      This selects the Nuvoton MA35 series SoC glue layer support
>> +      for the stmmac device driver.
>
> Also mention the module name `dwmac-nuvoton`?
>
Got it!
>> +
>>   config DWMAC_QCOM_ETHQOS
>>       tristate "Qualcomm ETHQOS support"
>>       default ARCH_QCOM
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile 
>> b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> index b26f0e79c2b3..48e25b85ea06 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> @@ -19,6 +19,7 @@ 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_NUVOTON)    += dwmac-nuvoton.o
>>   obj-$(CONFIG_DWMAC_QCOM_ETHQOS)    += dwmac-qcom-ethqos.o
>>   obj-$(CONFIG_DWMAC_ROCKCHIP)    += dwmac-rk.o
>>   obj-$(CONFIG_DWMAC_RZN1)    += dwmac-rzn1.o
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-nuvoton.c 
>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-nuvoton.c
>> new file mode 100644
>> index 000000000000..edf1b88ce1cd
>> --- /dev/null
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-nuvoton.c
>> @@ -0,0 +1,179 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Nuvoton DWMAC specific glue layer
>> + *
>> + * Copyright (C) 2024 Nuvoton Technology Corp.
>> + *
>> + * Author: Joey Lu <yclu4@nuvoton.com>
>> + */
>> +
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_net.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/stmmac.h>
>> +
>> +#include "stmmac.h"
>> +#include "stmmac_platform.h"
>> +
>> +#define NVT_REG_SYS_GMAC0MISCR  0x108
>> +#define NVT_REG_SYS_GMAC1MISCR  0x10C
>> +
>> +#define NVT_MISCR_RMII          BIT(0)
>> +
>> +/* 2000ps is mapped to 0x0 ~ 0xF */
>
> Excuse my ignorance: What is ps?
>
Sorry for the confusion. I will include a complete and clear description 
in the next patch.
>> +#define NVT_PATH_DELAY_DEC      134
>> +#define NVT_TX_DELAY_MASK       GENMASK(19, 16)
>> +#define NVT_RX_DELAY_MASK       GENMASK(23, 20)
>> +
>> +struct nvt_priv_data {
>> +    struct platform_device *pdev;
>> +    struct regmap *regmap;
>> +};
>> +
>> +static struct nvt_priv_data *
>> +nvt_gmac_setup(struct platform_device *pdev, struct 
>> plat_stmmacenet_data *plat)
>> +{
>> +    struct device *dev = &pdev->dev;
>> +    struct nvt_priv_data *bsp_priv;
>> +    phy_interface_t phy_mode;
>> +    u32 tx_delay, rx_delay;
>
> Please append the unit to the variable name.
>
Got it!
>> +    u32 macid, arg, reg;
>> +
>> +    bsp_priv = devm_kzalloc(dev, sizeof(*bsp_priv), GFP_KERNEL);
>> +    if (!bsp_priv)
>> +        return ERR_PTR(-ENOMEM);
>> +
>> +    bsp_priv->regmap =
>> +        syscon_regmap_lookup_by_phandle_args(dev->of_node, 
>> "nuvoton,sys", 1, &macid);
>> +    if (IS_ERR(bsp_priv->regmap)) {
>> +        dev_err_probe(dev, PTR_ERR(bsp_priv->regmap), "Failed to get 
>> sys register\n");
>> +        return ERR_PTR(-ENODEV);
>> +    }
>> +    if (macid > 1) {
>> +        dev_err_probe(dev, -EINVAL, "Invalid sys arguments\n");
>> +        return ERR_PTR(-EINVAL);
>> +    }
>> +
>> +    if (of_property_read_u32(dev->of_node, "tx-internal-delay-ps", 
>> &arg)) {
>> +        tx_delay = 0;
>> +    } else {
>> +        if (arg <= 2000) {
>> +            tx_delay = (arg == 2000) ? 0xF : (arg / 
>> NVT_PATH_DELAY_DEC);
>
> Write hexcharacters lowercase?
>
Got it!
>> +            dev_dbg(dev, "Set Tx path delay to 0x%x\n", tx_delay);
>> +        } else {
>> +            dev_err(dev, "Invalid Tx path delay argument.\n");
>> +            return ERR_PTR(-EINVAL);
>> +        }
>> +    }
>> +    if (of_property_read_u32(dev->of_node, "rx-internal-delay-ps", 
>> &arg)) {
>> +        rx_delay = 0;
>> +    } else {
>> +        if (arg <= 2000) {
>> +            rx_delay = (arg == 2000) ? 0xF : (arg / 
>> NVT_PATH_DELAY_DEC);
>> +            dev_dbg(dev, "Set Rx path delay to 0x%x\n", rx_delay);
>> +        } else {
>> +            dev_err(dev, "Invalid Rx path delay argument.\n");
>> +            return ERR_PTR(-EINVAL);
>> +        }
>> +    }
>> +
>> +    regmap_read(bsp_priv->regmap,
>> +            macid == 0 ? NVT_REG_SYS_GMAC0MISCR : 
>> NVT_REG_SYS_GMAC1MISCR, &reg);
>> +    reg &= ~(NVT_TX_DELAY_MASK | NVT_RX_DELAY_MASK);
>> +
>> +    if (of_get_phy_mode(pdev->dev.of_node, &phy_mode)) {
>> +        dev_err(dev, "missing phy mode property\n");
>> +        return ERR_PTR(-EINVAL);
>> +    }
>> +
>> +    switch (phy_mode) {
>> +    case PHY_INTERFACE_MODE_RGMII:
>> +    case PHY_INTERFACE_MODE_RGMII_ID:
>> +    case PHY_INTERFACE_MODE_RGMII_RXID:
>> +    case PHY_INTERFACE_MODE_RGMII_TXID:
>> +        reg &= ~NVT_MISCR_RMII;
>> +        break;
>> +    case PHY_INTERFACE_MODE_RMII:
>> +        reg |= NVT_MISCR_RMII;
>> +        break;
>> +    default:
>> +        dev_err(dev, "Unsupported phy-mode (%d)\n", phy_mode);
>> +        return ERR_PTR(-EINVAL);
>> +    }
>> +
>> +    if (!(reg & NVT_MISCR_RMII)) {
>> +        reg |= FIELD_PREP(NVT_TX_DELAY_MASK, tx_delay);
>> +        reg |= FIELD_PREP(NVT_RX_DELAY_MASK, rx_delay);
>> +    }
>> +
>> +    regmap_write(bsp_priv->regmap,
>> +             macid == 0 ? NVT_REG_SYS_GMAC0MISCR : 
>> NVT_REG_SYS_GMAC1MISCR, reg);
>> +
>> +    bsp_priv->pdev = pdev;
>> +
>> +    return bsp_priv;
>> +}
>> +
>> +static int nvt_gmac_probe(struct platform_device *pdev)
>> +{
>> +    struct plat_stmmacenet_data *plat_dat;
>> +    struct stmmac_resources stmmac_res;
>> +    struct nvt_priv_data *priv_data;
>> +    int ret;
>> +
>> +    ret = stmmac_get_platform_resources(pdev, &stmmac_res);
>> +    if (ret)
>> +        return ret;
>> +
>> +    plat_dat = devm_stmmac_probe_config_dt(pdev, stmmac_res.mac);
>> +    if (IS_ERR(plat_dat))
>> +        return PTR_ERR(plat_dat);
>> +
>> +    /* Nuvoton DWMAC configs */
>> +    plat_dat->has_gmac = 1;
>> +    plat_dat->tx_fifo_size = 2048;
>> +    plat_dat->rx_fifo_size = 4096;
>> +    plat_dat->multicast_filter_bins = 0;
>> +    plat_dat->unicast_filter_entries = 8;
>> +    plat_dat->flags &= ~STMMAC_FLAG_USE_PHY_WOL;
>> +
>> +    priv_data = nvt_gmac_setup(pdev, plat_dat);
>> +    if (IS_ERR(priv_data))
>> +        return PTR_ERR(priv_data);
>> +
>> +    ret = stmmac_pltfr_probe(pdev, plat_dat, &stmmac_res);
>> +    if (ret)
>> +        return ret;
>> +
>> +    /* The PMT flag is determined by the RWK property.
>> +     * However, our hardware is configured to support only MGK.
>> +     * This is an override on PMT to enable WoL capability.
>> +     */
>> +    plat_dat->pmt = 1;
>> +    device_set_wakeup_capable(&pdev->dev, 1);
>> +
>> +    return 0;
>> +}
>> +
>> +static const struct of_device_id nvt_dwmac_match[] = {
>> +    { .compatible = "nuvoton,ma35d1-dwmac"},
>> +    { }
>> +};
>> +MODULE_DEVICE_TABLE(of, nvt_dwmac_match);
>> +
>> +static struct platform_driver nvt_dwmac_driver = {
>> +    .probe  = nvt_gmac_probe,
>> +    .remove = stmmac_pltfr_remove,
>> +    .driver = {
>> +        .name           = "nuvoton-dwmac",
>> +        .pm        = &stmmac_pltfr_pm_ops,
>> +        .of_match_table = nvt_dwmac_match,
>> +    },
>> +};
>> +module_platform_driver(nvt_dwmac_driver);
>> +
>> +MODULE_AUTHOR("Joey Lu <yclu4@nuvoton.com>");
>
> Maybe Nuvoton can set up a generic address?
>
I will switch to using my personal email instead.🙂
>> +MODULE_DESCRIPTION("Nuvoton DWMAC specific glue layer");
>> +MODULE_LICENSE("GPL v2");
>
>
> Kind regards,
>
> Paul

BR,

Joey


log:

[    T0] Booting Linux on physical CPU 0x0000000000 [0x411fd040]
[    T0] Linux version 6.13.0-rc7 (joeylu@ubuntu) 
(aarch64-nuvoton-linux-gnu-gcc.br_real (Buildroot 2021.02.6) 9.4.0, GNU 
ld (GNU Binutils) 2.35.2)
[    T0] Machine model: Nuvoton MA35D1-SOM
[    T0] Memory limited to 248MB
...

[    T1] nuvoton-dwmac 40120000.ethernet: IRQ eth_wake_irq not found
[    T1] nuvoton-dwmac 40120000.ethernet: IRQ eth_lpi not found
[    T1] nuvoton-dwmac 40120000.ethernet: IRQ sfty not found
[    T1] nuvoton-dwmac 40120000.ethernet: User ID: 0x10, Synopsys ID: 0x37
[    T1] nuvoton-dwmac 40120000.ethernet:       DWMAC1000
[    T1] nuvoton-dwmac 40120000.ethernet: DMA HW capability register 
supported
[    T1] nuvoton-dwmac 40120000.ethernet: RX Checksum Offload Engine 
supported
[    T1] nuvoton-dwmac 40120000.ethernet: COE Type 2
[    T1] nuvoton-dwmac 40120000.ethernet: TX Checksum insertion supported
[    T1] nuvoton-dwmac 40120000.ethernet: Enhanced/Alternate descriptors
[    T1] nuvoton-dwmac 40120000.ethernet: Enabled extended descriptors
[    T1] nuvoton-dwmac 40120000.ethernet: Ring mode enabled
[    T1] nuvoton-dwmac 40120000.ethernet: Enable RX Mitigation via HW 
Watchdog Timer
[    T1] nuvoton-dwmac 40130000.ethernet: IRQ eth_wake_irq not found
[    T1] nuvoton-dwmac 40130000.ethernet: IRQ eth_lpi not found
[    T1] nuvoton-dwmac 40130000.ethernet: IRQ sfty not found
[    T1] nuvoton-dwmac 40130000.ethernet: User ID: 0x10, Synopsys ID: 0x37
[    T1] nuvoton-dwmac 40130000.ethernet:       DWMAC1000
[    T1] nuvoton-dwmac 40130000.ethernet: DMA HW capability register 
supported
[    T1] nuvoton-dwmac 40130000.ethernet: RX Checksum Offload Engine 
supported
[    T1] nuvoton-dwmac 40130000.ethernet: COE Type 2
[    T1] nuvoton-dwmac 40130000.ethernet: TX Checksum insertion supported
[    T1] nuvoton-dwmac 40130000.ethernet: Enhanced/Alternate descriptors
[    T1] nuvoton-dwmac 40130000.ethernet: Enabled extended descriptors
[    T1] nuvoton-dwmac 40130000.ethernet: Ring mode enabled
[    T1] nuvoton-dwmac 40130000.ethernet: Enable RX Mitigation via HW 
Watchdog Timer
...

# udhcpc -i eth0
udhcpc: started, v1.33.1
[   T93] nuvoton-dwmac 40120000.ethernet eth0: Register 
MEM_TYPE_PAGE_POOL RxQ-0
[   T93] nuvoton-dwmac 40120000.ethernet eth0: PHY [stmmac-0:00] driver 
[Generic PHY] (irq=POLL)
[   T93] nuvoton-dwmac 40120000.ethernet eth0: No Safety Features 
support found
[   T93] nuvoton-dwmac 40120000.ethernet eth0: No MAC Management 
Counters available
[   T93] nuvoton-dwmac 40120000.ethernet eth0: IEEE 1588-2008 Advanced 
Timestamp supported
[   T93] nuvoton-dwmac 40120000.ethernet eth0: registered PTP clock
[   T93] nuvoton-dwmac 40120000.ethernet eth0: configuring for 
phy/rgmii-id link mode
udhcpc: sending discover
udhcpc: sending discover
[   T33] nuvoton-dwmac 40120000.ethernet eth0: Link is Up - 1Gbps/Full - 
flow control rx/tx
udhcpc: sending discover
udhcpc: sending select for 192.168.0.103
udhcpc: lease of 192.168.0.103 obtained, lease time 86400
deleting routers
adding dns 192.168.0.1

[-- Attachment #2: Type: text/html, Size: 21239 bytes --]