Re: [PATCH RFT net-next 02/10] net: stmmac: Add support for Allwinner A523 GMAC200

[Yanteng Si <si.yanteng@linux.dev>]

在 7/2/25 12:57 AM, Chen-Yu Tsai 写道:
> From: Chen-Yu Tsai <wens@csie.org>
> 
> The Allwinner A523 SoC family has a second Ethernet controller, called
> the GMAC200 in the BSP and T527 datasheet, and referred to as GMAC1 for
> numbering. This controller, according to BSP sources, is fully
> compatible with a slightly newer version of the Synopsys DWMAC core.
> The glue layer around the controller is the same as found around older
> DWMAC cores on Allwinner SoCs. The only slight difference is that since
> this is the second controller on the SoC, the register for the clock
> delay controls is at a different offset. Last, the integration includes
> a dedicated clock gate for the memory bus and the whole thing is put in
> a separately controllable power domain.
> 
> Add a new driver for this hardware supporting the integration layer.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>   drivers/net/ethernet/stmicro/stmmac/Kconfig   |  12 ++
>   drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
>   .../ethernet/stmicro/stmmac/dwmac-sun55i.c    | 161 ++++++++++++++++++
>   3 files changed, 174 insertions(+)
>   create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-sun55i.c
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index 67fa879b1e52..38ce9a0cfb5b 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -263,6 +263,18 @@ config DWMAC_SUN8I
>   	  stmmac device driver. This driver is used for H3/A83T/A64
>   	  EMAC ethernet controller.
>   
> +config DWMAC_SUN55I
> +	tristate "Allwinner sun55i GMAC200 support"
> +	default ARCH_SUNXI
> +	depends on OF && (ARCH_SUNXI || COMPILE_TEST)
> +	select MDIO_BUS_MUX
> +	help
> +	  Support for Allwinner A523/T527 GMAC200 ethernet controllers.
> +
> +	  This selects Allwinner SoC glue layer support for the
> +	  stmmac device driver. This driver is used for A523/T527
> +	  GMAC200 ethernet controller.
> +
>   config DWMAC_THEAD
>   	tristate "T-HEAD dwmac support"
>   	depends on OF && (ARCH_THEAD || COMPILE_TEST)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> index b591d93f8503..51e068e26ce4 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_DWMAC_STI)		+= dwmac-sti.o
>   obj-$(CONFIG_DWMAC_STM32)	+= dwmac-stm32.o
>   obj-$(CONFIG_DWMAC_SUNXI)	+= dwmac-sunxi.o
>   obj-$(CONFIG_DWMAC_SUN8I)	+= dwmac-sun8i.o
> +obj-$(CONFIG_DWMAC_SUN55I)	+= dwmac-sun55i.o
>   obj-$(CONFIG_DWMAC_THEAD)	+= dwmac-thead.o
>   obj-$(CONFIG_DWMAC_DWC_QOS_ETH)	+= dwmac-dwc-qos-eth.o
>   obj-$(CONFIG_DWMAC_INTEL_PLAT)	+= dwmac-intel-plat.o
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun55i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun55i.c
> new file mode 100644
> index 000000000000..7fadb90e3098
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun55i.c
> @@ -0,0 +1,161 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * dwmac-sun55i.c - Allwinner sun55i GMAC200 specific glue layer
> + *
> + * Copyright (C) 2025 Chen-Yu Tsai <wens@csie.org>
> + *
> + * syscon parts taken from dwmac-sun8i.c, which is
> + *
> + * Copyright (C) 2017 Corentin Labbe <clabbe.montjoie@gmail.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/stmmac.h>
> +
> +#include "stmmac.h"
> +#include "stmmac_platform.h"
> +
> +#define SYSCON_REG		0x34
> +
> +/* RMII specific bits */
> +#define SYSCON_RMII_EN		BIT(13) /* 1: enable RMII (overrides EPIT) */
insert a blankline.
> +/* Generic system control EMAC_CLK bits */
> +#define SYSCON_ETXDC_MASK		GENMASK(12, 10)
> +#define SYSCON_ERXDC_MASK		GENMASK(9, 5)
ditto.
> +/* EMAC PHY Interface Type */
> +#define SYSCON_EPIT			BIT(2) /* 1: RGMII, 0: MII */
> +#define SYSCON_ETCS_MASK		GENMASK(1, 0)
> +#define SYSCON_ETCS_MII		0x0
> +#define SYSCON_ETCS_EXT_GMII	0x1
> +#define SYSCON_ETCS_INT_GMII	0x2
> +
> +#define MASK_TO_VAL(mask)   ((mask) >> (__builtin_ffsll(mask) - 1))
> +
> +static int sun55i_gmac200_set_syscon(struct device *dev,
> +				     struct plat_stmmacenet_data *plat)
> +{
> +	struct device_node *node = dev->of_node;
> +	struct regmap *regmap;
> +	u32 val, reg = 0;
> +
> +	regmap = syscon_regmap_lookup_by_phandle(node, "syscon");
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap), "Unable to map syscon\n");
> +
-----------
> +	if (!of_property_read_u32(node, "allwinner,tx-delay-ps", &val)) {
> +		if (val % 100) {
> +			dev_err(dev, "tx-delay must be a multiple of 100\n");
> +			return -EINVAL;
> +		}
> +		val /= 100;
> +		dev_dbg(dev, "set tx-delay to %x\n", val);
> +		if (val > MASK_TO_VAL(SYSCON_ETXDC_MASK))
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Invalid TX clock delay: %d\n",
> +					     val);
> +
> +		reg |= FIELD_PREP(SYSCON_ETXDC_MASK, val);
> +	}
> +
> +	if (!of_property_read_u32(node, "allwinner,rx-delay-ps", &val)) {
> +		if (val % 100) {
> +			dev_err(dev, "rx-delay must be a multiple of 100\n");
> +			return -EINVAL;
> +		}
> +		val /= 100;
> +		dev_dbg(dev, "set rx-delay to %x\n", val);
> +		if (val > MASK_TO_VAL(SYSCON_ERXDC_MASK))
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Invalid RX clock delay: %d\n",
> +					     val);
> +
> +		reg |= FIELD_PREP(SYSCON_ERXDC_MASK, val);
> +	}
------------
These two parts of the code are highly similar.
Can you construct a separate function?
> +
> +	switch (plat->mac_interface) {

> +	case PHY_INTERFACE_MODE_MII:
> +		/* default */
> +		break;
This line of comment seems a bit abrupt here.


Thanks,
Yanteng