[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <15ba0933-b0c1-40eb-9d3c-d8837d6ee12a@linux.dev>
Date: Wed, 2 Jul 2025 09:59:52 +0800
From: Yanteng Si <si.yanteng@...ux.dev>
To: Chen-Yu Tsai <wens@...nel.org>, Andrew Lunn <andrew+netdev@...n.ch>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Chen-Yu Tsai <wens@...e.org>,
Jernej Skrabec <jernej@...nel.org>, Samuel Holland <samuel@...lland.org>
Cc: netdev@...r.kernel.org, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-sunxi@...ts.linux.dev,
linux-kernel@...r.kernel.org, Andre Przywara <andre.przywara@....com>
Subject: Re: [PATCH RFT net-next 02/10] net: stmmac: Add support for Allwinner
A523 GMAC200
在 7/2/25 12:57 AM, Chen-Yu Tsai 写道:
> From: Chen-Yu Tsai <wens@...e.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@...e.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@...e.org>
> + *
> + * syscon parts taken from dwmac-sun8i.c, which is
> + *
> + * Copyright (C) 2017 Corentin Labbe <clabbe.montjoie@...il.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
Powered by blists - more mailing lists