Re: [PATCH net-next v11 3/3] net: stmmac: dwmac-nuvoton: Add dwmac glue for Nuvoton MA35 family
flat view
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
Date: 2026-02-05 09:39:18
Also in:
linux-arm-kernel, lkml, netdev, openbmc
Hi, On Thu, Feb 05, 2026 at 09:40:05AM +0800, Joey Lu wrote:
quoted hunk ↗ jump to hunk
+ +struct nvt_priv_data { + struct platform_device *pdev;
This looks to me like it's write-only, does it serve a useful purpose?
+ struct regmap *regmap;
This doesn't seem to be used outside of nvt_gmac_setup().
+};
Given the above two comments, do you actually need struct nvt_priv_data ?
quoted hunk ↗ jump to hunk
+ +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 macid, arg, reg; + u32 tx_delay_step; + u32 rx_delay_step; + u32 miscr; + + 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)) + return ERR_PTR(dev_err_probe(dev, PTR_ERR(bsp_priv->regmap), + "Failed to get sys register\n")); + if (macid > 1) { + dev_err(dev, "Invalid sys arguments\n"); + return ERR_PTR(-EINVAL); + } + + if (of_property_read_u32(dev->of_node, "tx-internal-delay-ps", &arg)) { + tx_delay_step = 0; + } else { + if (arg <= 2000) { + tx_delay_step = (arg == 2000) ? 0xf : (arg / NVT_PATH_DELAY_STEP); + dev_dbg(dev, "Set Tx path delay to 0x%x\n", tx_delay_step); + } 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_step = 0; + } else { + if (arg <= 2000) { + rx_delay_step = (arg == 2000) ? 0xf : (arg / NVT_PATH_DELAY_STEP); + dev_dbg(dev, "Set Rx path delay to 0x%x\n", rx_delay_step); + } else { + dev_err(dev, "Invalid Rx path delay argument.\n"); + return ERR_PTR(-EINVAL); + } + }
Each of these could be moved into a separate function:
static int nvt_gmac_get_delay(struct device *dev, const char *property)
{
u32 arg;
if (of_property_read_u32(dev->of_node, property, &arg))
return 0;
if (arg > 2000) {
dev_err(dev, "Invalid %s argument.\n", property);
return -EINVAL;
}
if (arg == 2000)
return 15;
return arg / NVT_PATH_DELAY_STEP;
}
then:
int ret;
ret = nvt_gmac_get_delay(dev, "tx-internal-delay-ps");
if (ret < 0)
return ERR_PTR(ret);
tx_delay = ret;
ret = nvt_gmac_get_delay(dev, "rx-internal-delay-ps");
if (ret < 0)
return ERR_PTR(ret);
rx_delay = ret;
quoted hunk ↗ jump to hunk
+ + miscr = (macid == 0) ? NVT_REG_SYS_GMAC0MISCR : NVT_REG_SYS_GMAC1MISCR; + regmap_read(bsp_priv->regmap, miscr, ®); + 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_step); + reg |= FIELD_PREP(NVT_RX_DELAY_MASK, rx_delay_step);
You can move this inside the switch above under the RGMII case. Theses delays are, after all, only for RGMII.
quoted hunk ↗ jump to hunk
+ } + + regmap_write(bsp_priv->regmap, miscr, reg);
Consider: regmap_update_bits(bsp_priv->regmap, miscr, NVT_TX_DELAY_MASK | NVT_RX_DELAY_MASK | NVT_MISCR_RMII, reg);
quoted hunk ↗ jump to hunk
+ 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->core_type = DWMAC_CORE_GMAC;
Is the hardware not compatible with any of the compatible types that devm_stmmac_probe_config_dt() will automatically set this for you? Which version of the core do you have?
+ plat_dat->tx_fifo_size = 2048; + plat_dat->rx_fifo_size = 4096;
There are tx-fifo-depth / rx-fifo-depth properties that can be used to describe these in DT.
+ plat_dat->multicast_filter_bins = 0; + plat_dat->unicast_filter_entries = 8;
If this core is v3.50, v3.70 or v3.72, then there are snps,multicast-filter-bins and snps,perfect-filter-entries which can be used to describe both of these. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!