diff mbox series

[2/2] net: stmmac: Add Ingenic SoCs MAC support.

Message ID 1623086867-119039-3-git-send-email-zhouyanjie@wanyeetech.com (mailing list archive)
State Superseded
Headers show
Series Add Ingenic SoCs MAC support. | expand

Commit Message

Zhou Yanjie June 7, 2021, 5:27 p.m. UTC
Add support for Ingenic SoC MAC glue layer support for the stmmac
device driver. This driver is used on for the MAC ethernet controller
found in the JZ4775 SoC, the X1000 SoC, the X1600 SoC, the X1830 SoC,
and the X2000 SoC.

Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig        |  16 +-
 drivers/net/ethernet/stmicro/stmmac/Makefile       |   1 +
 .../net/ethernet/stmicro/stmmac/dwmac-ingenic.c    | 367 +++++++++++++++++++++
 3 files changed, 382 insertions(+), 2 deletions(-)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-ingenic.c

Comments

Andrew Lunn June 8, 2021, 12:01 a.m. UTC | #1
>  config DWMAC_ROCKCHIP
>  	tristate "Rockchip dwmac support"
> -	default ARCH_ROCKCHIP
> +	default MACH_ROCKCHIP
>  	depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST)
>  	select MFD_SYSCON
>  	help
> @@ -164,7 +176,7 @@ config DWMAC_STI
>  
>  config DWMAC_STM32
>  	tristate "STM32 DWMAC support"
> -	default ARCH_STM32
> +	default MACH_STM32

It would be good to explain in the commit message why you are changing
these two. It is not obvious.

> +static int jz4775_mac_set_mode(struct plat_stmmacenet_data *plat_dat)
> +{
> +	struct ingenic_mac *mac = plat_dat->bsp_priv;
> +	int val;
> +
> +	switch (plat_dat->interface) {
> +	case PHY_INTERFACE_MODE_MII:
> +		val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, MACPHYC_TXCLK_SEL_INPUT) |
> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_MII);
> +		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_MII\n");
> +		break;
> +
> +	case PHY_INTERFACE_MODE_GMII:
> +		val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, MACPHYC_TXCLK_SEL_INPUT) |
> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_GMII);
> +		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_GMII\n");
> +		break;
> +
> +	case PHY_INTERFACE_MODE_RMII:
> +		val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, MACPHYC_TXCLK_SEL_INPUT) |
> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RMII);
> +		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_RMII\n");
> +		break;
> +
> +	case PHY_INTERFACE_MODE_RGMII:

What about the other three RGMII modes?

> +	case PHY_INTERFACE_MODE_RGMII:
> +		val = FIELD_PREP(MACPHYC_TX_SEL_MASK, MACPHYC_TX_SEL_DELAY) |
> +			  FIELD_PREP(MACPHYC_TX_DELAY_MASK, MACPHYC_TX_DELAY_63_UNIT) |
> +			  FIELD_PREP(MACPHYC_RX_SEL_MASK, MACPHYC_RX_SEL_ORIGIN) |
> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RGMII);

What exactly does MACPHYC_TX_DELAY_63_UNIT mean here? Ideally, the MAC
should not be adding any RGMII delays. It should however pass mode
through to the PHY, so it can add the delays, if the mode indicates it
should, e.g. PHY_INTERFACE_MODE_RGMII_ID. This is also why you should
be handling all 4 RGMII modes here, not just one.

	 Andrew
Paul Cercueil June 8, 2021, 8:46 a.m. UTC | #2
Hi Zhou,

Le mar., juin 8 2021 at 01:27:47 +0800, 周琰杰 (Zhou Yanjie) 
<zhouyanjie@wanyeetech.com> a écrit :
> Add support for Ingenic SoC MAC glue layer support for the stmmac
> device driver. This driver is used on for the MAC ethernet controller
> found in the JZ4775 SoC, the X1000 SoC, the X1600 SoC, the X1830 SoC,
> and the X2000 SoC.
> 
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/Kconfig        |  16 +-
>  drivers/net/ethernet/stmicro/stmmac/Makefile       |   1 +
>  .../net/ethernet/stmicro/stmmac/dwmac-ingenic.c    | 367 
> +++++++++++++++++++++
>  3 files changed, 382 insertions(+), 2 deletions(-)
>  create mode 100644 
> drivers/net/ethernet/stmicro/stmmac/dwmac-ingenic.c
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig 
> b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index 7737e4d0..fb58537 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -66,6 +66,18 @@ config DWMAC_ANARION
> 
>  	  This selects the Anarion SoC glue layer support for the stmmac 
> driver.
> 
> +config DWMAC_INGENIC
> +	tristate "Ingenic MAC support"
> +	default MACH_INGENIC
> +	depends on OF && HAS_IOMEM && (MACH_INGENIC || COMPILE_TEST)
> +	select MFD_SYSCON
> +	help
> +	  Support for ethernet controller on Ingenic SoCs.
> +
> +	  This selects Ingenic SoCs glue layer support for the stmmac
> +	  device driver. This driver is used on for the Ingenic SoCs
> +	  MAC ethernet controller.
> +
>  config DWMAC_IPQ806X
>  	tristate "QCA IPQ806x DWMAC support"
>  	default ARCH_QCOM
> @@ -129,7 +141,7 @@ config DWMAC_QCOM_ETHQOS
> 
>  config DWMAC_ROCKCHIP
>  	tristate "Rockchip dwmac support"
> -	default ARCH_ROCKCHIP
> +	default MACH_ROCKCHIP
>  	depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST)
>  	select MFD_SYSCON
>  	help
> @@ -164,7 +176,7 @@ config DWMAC_STI
> 
>  config DWMAC_STM32
>  	tristate "STM32 DWMAC support"
> -	default ARCH_STM32
> +	default MACH_STM32
>  	depends on OF && HAS_IOMEM && (ARCH_STM32 || COMPILE_TEST)
>  	select MFD_SYSCON
>  	help
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile 
> b/drivers/net/ethernet/stmicro/stmmac/Makefile
> index f2e478b..6471f93 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> @@ -14,6 +14,7 @@ stmmac-$(CONFIG_STMMAC_SELFTESTS) += 
> stmmac_selftests.o
>  # Ordering matters. Generic driver must be last.
>  obj-$(CONFIG_STMMAC_PLATFORM)	+= stmmac-platform.o
>  obj-$(CONFIG_DWMAC_ANARION)	+= dwmac-anarion.o
> +obj-$(CONFIG_DWMAC_INGENIC)	+= dwmac-ingenic.o
>  obj-$(CONFIG_DWMAC_IPQ806X)	+= dwmac-ipq806x.o
>  obj-$(CONFIG_DWMAC_LPC18XX)	+= dwmac-lpc18xx.o
>  obj-$(CONFIG_DWMAC_MEDIATEK)	+= dwmac-mediatek.o
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-ingenic.c 
> b/drivers/net/ethernet/stmicro/stmmac/dwmac-ingenic.c
> new file mode 100644
> index 00000000..8be8caa
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-ingenic.c
> @@ -0,0 +1,367 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * dwmac-ingenic.c - Ingenic SoCs DWMAC specific glue layer
> + *
> + * Copyright (c) 2020 周琰杰 (Zhou Yanjie) 
> <zhouyanjie@wanyeetech.com>

2021?

> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_net.h>
> +#include <linux/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/stmmac.h>
> +
> +#include "stmmac_platform.h"
> +
> +#define MACPHYC_TXCLK_SEL_MASK		GENMASK(31, 31)
> +#define MACPHYC_TXCLK_SEL_OUTPUT	0x1
> +#define MACPHYC_TXCLK_SEL_INPUT		0x0
> +#define MACPHYC_MODE_SEL_MASK		GENMASK(31, 31)
> +#define MACPHYC_MODE_SEL_RMII		0x0
> +#define MACPHYC_TX_SEL_MASK			GENMASK(19, 19)
> +#define MACPHYC_TX_SEL_ORIGIN		0x0
> +#define MACPHYC_TX_SEL_DELAY		0x1
> +#define MACPHYC_TX_DELAY_MASK		GENMASK(18, 12)
> +#define MACPHYC_TX_DELAY_63_UNIT	0x3e
> +#define MACPHYC_RX_SEL_MASK			GENMASK(11, 11)
> +#define MACPHYC_RX_SEL_ORIGIN		0x0
> +#define MACPHYC_RX_SEL_DELAY		0x1
> +#define MACPHYC_RX_DELAY_MASK		GENMASK(10, 4)
> +#define MACPHYC_SOFT_RST_MASK		GENMASK(3, 3)
> +#define MACPHYC_PHY_INFT_MASK		GENMASK(2, 0)
> +#define MACPHYC_PHY_INFT_RMII		0x4
> +#define MACPHYC_PHY_INFT_RGMII		0x1
> +#define MACPHYC_PHY_INFT_GMII		0x0
> +#define MACPHYC_PHY_INFT_MII		0x0
> +
> +enum ingenic_mac_version {
> +	ID_JZ4775,
> +	ID_X1000,
> +	ID_X1600,
> +	ID_X1830,
> +	ID_X2000,

You could test it on all these? I never heard about the X1600 before.

> +};
> +
> +struct ingenic_mac {
> +	const struct ingenic_soc_info *soc_info;
> +	struct device *dev;
> +	struct regmap *regmap;
> +};
> +
> +struct ingenic_soc_info {
> +	enum ingenic_mac_version version;
> +	u32 mask;
> +
> +	int (*set_mode)(struct plat_stmmacenet_data *plat_dat);
> +	int (*suspend)(struct ingenic_mac *mac);
> +	void (*resume)(struct ingenic_mac *mac);

These suspend/resume callbacks are not used anywhere - just drop them.

> +};
> +
> +static int ingenic_mac_init(struct plat_stmmacenet_data *plat_dat)
> +{
> +	struct ingenic_mac *mac = plat_dat->bsp_priv;
> +	int ret;
> +
> +	if (mac->soc_info->set_mode) {
> +		ret = mac->soc_info->set_mode(plat_dat);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return ret;

You are returning an uninitialized variable.

> +}
> +
> +static int jz4775_mac_set_mode(struct plat_stmmacenet_data *plat_dat)
> +{
> +	struct ingenic_mac *mac = plat_dat->bsp_priv;
> +	int val;

unsigned int val;

> +
> +	switch (plat_dat->interface) {
> +	case PHY_INTERFACE_MODE_MII:
> +		val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, MACPHYC_TXCLK_SEL_INPUT) |
> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_MII);
> +		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_MII\n");

Use dev_dbg() with mac->dev, instead of pr_debug().

(Same for all pr_debug() calls below)

> +		break;
> +
> +	case PHY_INTERFACE_MODE_GMII:
> +		val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, MACPHYC_TXCLK_SEL_INPUT) |
> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_GMII);
> +		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_GMII\n");
> +		break;
> +
> +	case PHY_INTERFACE_MODE_RMII:
> +		val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, MACPHYC_TXCLK_SEL_INPUT) |
> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RMII);
> +		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_RMII\n");
> +		break;
> +
> +	case PHY_INTERFACE_MODE_RGMII:
> +		val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, MACPHYC_TXCLK_SEL_INPUT) |
> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RGMII);
> +		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_RGMII\n");
> +		break;
> +
> +	default:
> +		dev_err(mac->dev, "unsupported interface %d", plat_dat->interface);
> +		return -EINVAL;
> +	}
> +
> +	/* Update MAC PHY control register */
> +	return regmap_update_bits(mac->regmap, 0, mac->soc_info->mask, val);
> +}
> +
> +static int x1000_mac_set_mode(struct plat_stmmacenet_data *plat_dat)
> +{
> +	struct ingenic_mac *mac = plat_dat->bsp_priv;
> +	int val;
> +
> +	switch (plat_dat->interface) {
> +	case PHY_INTERFACE_MODE_RMII:
> +		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_RMII\n");
> +		break;
> +
> +	default:
> +		dev_err(mac->dev, "unsupported interface %d", plat_dat->interface);
> +		return -EINVAL;
> +	}
> +
> +	/* Update MAC PHY control register */
> +	return regmap_update_bits(mac->regmap, 0, mac->soc_info->mask, val);

You're passing 'val', which is an uninitialized variable.

> +}
> +
> +static int x1600_mac_set_mode(struct plat_stmmacenet_data *plat_dat)
> +{
> +	struct ingenic_mac *mac = plat_dat->bsp_priv;
> +	int val;

unsigned int val;

> +
> +	switch (plat_dat->interface) {
> +	case PHY_INTERFACE_MODE_RMII:
> +		val = FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RMII);
> +		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_RMII\n");
> +		break;
> +
> +	default:
> +		dev_err(mac->dev, "unsupported interface %d", plat_dat->interface);
> +		return -EINVAL;
> +	}
> +
> +	/* Update MAC PHY control register */
> +	return regmap_update_bits(mac->regmap, 0, mac->soc_info->mask, val);
> +}
> +
> +static int x1830_mac_set_mode(struct plat_stmmacenet_data *plat_dat)
> +{
> +	struct ingenic_mac *mac = plat_dat->bsp_priv;
> +	int val;

Same here,

> +
> +	switch (plat_dat->interface) {
> +	case PHY_INTERFACE_MODE_RMII:
> +		val = FIELD_PREP(MACPHYC_MODE_SEL_MASK, MACPHYC_MODE_SEL_RMII) |
> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RMII);
> +		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_RMII\n");
> +		break;
> +
> +	default:
> +		dev_err(mac->dev, "unsupported interface %d", plat_dat->interface);
> +		return -EINVAL;
> +	}
> +
> +	/* Update MAC PHY control register */
> +	return regmap_update_bits(mac->regmap, 0, mac->soc_info->mask, val);
> +}
> +
> +static int x2000_mac_set_mode(struct plat_stmmacenet_data *plat_dat)
> +{
> +	struct ingenic_mac *mac = plat_dat->bsp_priv;
> +	int val;

Same here.

> +
> +	switch (plat_dat->interface) {
> +	case PHY_INTERFACE_MODE_RMII:
> +		val = FIELD_PREP(MACPHYC_TX_SEL_MASK, MACPHYC_TX_SEL_ORIGIN) |
> +			  FIELD_PREP(MACPHYC_RX_SEL_MASK, MACPHYC_RX_SEL_ORIGIN) |
> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RMII);
> +		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_RMII\n");
> +		break;
> +
> +	case PHY_INTERFACE_MODE_RGMII:
> +		val = FIELD_PREP(MACPHYC_TX_SEL_MASK, MACPHYC_TX_SEL_DELAY) |
> +			  FIELD_PREP(MACPHYC_TX_DELAY_MASK, MACPHYC_TX_DELAY_63_UNIT) |
> +			  FIELD_PREP(MACPHYC_RX_SEL_MASK, MACPHYC_RX_SEL_ORIGIN) |
> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RGMII);
> +		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_RGMII\n");
> +		break;
> +
> +	default:
> +		dev_err(mac->dev, "unsupported interface %d", plat_dat->interface);
> +		return -EINVAL;
> +	}
> +
> +	/* Update MAC PHY control register */
> +	return regmap_update_bits(mac->regmap, 0, mac->soc_info->mask, val);
> +}
> +
> +static int ingenic_mac_probe(struct platform_device *pdev)
> +{
> +	struct plat_stmmacenet_data *plat_dat;
> +	struct stmmac_resources stmmac_res;
> +	struct ingenic_mac *mac;
> +	const struct ingenic_soc_info *data;
> +	int ret;
> +
> +	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
> +	if (ret)
> +		return ret;
> +
> +	plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
> +	if (IS_ERR(plat_dat))
> +		return PTR_ERR(plat_dat);
> +
> +	mac = devm_kzalloc(&pdev->dev, sizeof(*mac), GFP_KERNEL);
> +	if (!mac) {
> +		ret = -ENOMEM;
> +		goto err_remove_config_dt;
> +	}
> +
> +	data = of_device_get_match_data(&pdev->dev);
> +	if (!data) {
> +		dev_err(&pdev->dev, "no of match data provided\n");
> +		ret = -EINVAL;
> +		goto err_remove_config_dt;
> +	}
> +
> +	/* Get MAC PHY control register */
> +	mac->regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, 
> "mode-reg");
> +	if (IS_ERR(mac->regmap)) {
> +		pr_err("%s: failed to get syscon regmap\n", __func__);

dev_err?

> +		goto err_remove_config_dt;
> +	}
> +
> +	mac->soc_info = data;
> +	mac->dev = &pdev->dev;
> +
> +	plat_dat->bsp_priv = mac;
> +
> +	ret = ingenic_mac_init(plat_dat);
> +	if (ret)
> +		goto err_remove_config_dt;
> +
> +	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> +	if (ret)
> +		goto err_remove_config_dt;
> +
> +	return 0;
> +
> +err_remove_config_dt:
> +	stmmac_remove_config_dt(pdev, plat_dat);
> +
> +	return ret;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP

Remove this #ifdef.

> +static int ingenic_mac_suspend(struct device *dev)
> +{
> +	struct net_device *ndev = dev_get_drvdata(dev);
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	struct ingenic_mac *mac = priv->plat->bsp_priv;
> +
> +	int ret;
> +
> +	ret = stmmac_suspend(dev);
> +
> +	if (mac->soc_info->suspend)
> +		ret = mac->soc_info->suspend(mac);
> +
> +	return ret;
> +}
> +
> +static int ingenic_mac_resume(struct device *dev)
> +{
> +	struct net_device *ndev = dev_get_drvdata(dev);
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	struct ingenic_mac *mac = priv->plat->bsp_priv;
> +	int ret;
> +
> +	if (mac->soc_info->resume)
> +		mac->soc_info->resume(mac);
> +
> +	ret = ingenic_mac_init(priv->plat);
> +	if (ret)
> +		return ret;
> +
> +	ret = stmmac_resume(dev);
> +
> +	return ret;
> +}
> +#endif /* CONFIG_PM_SLEEP */
> +
> +static SIMPLE_DEV_PM_OPS(ingenic_mac_pm_ops,
> +	ingenic_mac_suspend, ingenic_mac_resume);
> +
> +static struct ingenic_soc_info jz4775_soc_info = {
> +	.version = ID_JZ4775,
> +	.mask = MACPHYC_TXCLK_SEL_MASK | MACPHYC_SOFT_RST_MASK | 
> MACPHYC_PHY_INFT_MASK,
> +
> +	.set_mode = jz4775_mac_set_mode,
> +};
> +
> +static struct ingenic_soc_info x1000_soc_info = {
> +	.version = ID_X1000,
> +	.mask = MACPHYC_SOFT_RST_MASK,
> +
> +	.set_mode = x1000_mac_set_mode,
> +};
> +
> +static struct ingenic_soc_info x1600_soc_info = {
> +	.version = ID_X1600,
> +	.mask = MACPHYC_SOFT_RST_MASK | MACPHYC_PHY_INFT_MASK,
> +
> +	.set_mode = x1600_mac_set_mode,
> +};
> +
> +static struct ingenic_soc_info x1830_soc_info = {
> +	.version = ID_X1830,
> +	.mask = MACPHYC_MODE_SEL_MASK | MACPHYC_SOFT_RST_MASK | 
> MACPHYC_PHY_INFT_MASK,
> +
> +	.set_mode = x1830_mac_set_mode,
> +};
> +
> +static struct ingenic_soc_info x2000_soc_info = {
> +	.version = ID_X2000,
> +	.mask = MACPHYC_TX_SEL_MASK | MACPHYC_TX_DELAY_MASK | 
> MACPHYC_RX_SEL_MASK |
> +			MACPHYC_RX_DELAY_MASK | MACPHYC_SOFT_RST_MASK | 
> MACPHYC_PHY_INFT_MASK,
> +
> +	.set_mode = x2000_mac_set_mode,
> +};
> +
> +static const struct of_device_id ingenic_mac_of_matches[] = {
> +	{ .compatible = "ingenic,jz4775-mac", .data = &jz4775_soc_info },
> +	{ .compatible = "ingenic,x1000-mac", .data = &x1000_soc_info },
> +	{ .compatible = "ingenic,x1600-mac", .data = &x1600_soc_info },
> +	{ .compatible = "ingenic,x1830-mac", .data = &x1830_soc_info },
> +	{ .compatible = "ingenic,x2000-mac", .data = &x2000_soc_info },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ingenic_mac_of_matches);
> +
> +static struct platform_driver ingenic_mac_driver = {
> +	.probe		= ingenic_mac_probe,
> +	.remove		= stmmac_pltfr_remove,
> +	.driver		= {
> +		.name	= "ingenic-mac",
> +		.pm		= &ingenic_mac_pm_ops,

.pm = pm_ptr(&ingenic_mac_pm_ops),

> +		.of_match_table = ingenic_mac_of_matches,
> +	},
> +};
> +module_platform_driver(ingenic_mac_driver);
> +
> +MODULE_AUTHOR("周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>");
> +MODULE_DESCRIPTION("Ingenic SoCs DWMAC specific glue layer");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
> 

Cheers,
-Paul
Zhou Yanjie June 8, 2021, 11:33 a.m. UTC | #3
Hi Andrew,

On 2021/6/8 上午8:01, Andrew Lunn wrote:
>>   config DWMAC_ROCKCHIP
>>   	tristate "Rockchip dwmac support"
>> -	default ARCH_ROCKCHIP
>> +	default MACH_ROCKCHIP
>>   	depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST)
>>   	select MFD_SYSCON
>>   	help
>> @@ -164,7 +176,7 @@ config DWMAC_STI
>>   
>>   config DWMAC_STM32
>>   	tristate "STM32 DWMAC support"
>> -	default ARCH_STM32
>> +	default MACH_STM32
> It would be good to explain in the commit message why you are changing
> these two. It is not obvious.


Apologize for my carelessness, this is left over accidentally when 
cleaning up the code, I will remove them in the next version.


>
>> +static int jz4775_mac_set_mode(struct plat_stmmacenet_data *plat_dat)
>> +{
>> +	struct ingenic_mac *mac = plat_dat->bsp_priv;
>> +	int val;
>> +
>> +	switch (plat_dat->interface) {
>> +	case PHY_INTERFACE_MODE_MII:
>> +		val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, MACPHYC_TXCLK_SEL_INPUT) |
>> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_MII);
>> +		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_MII\n");
>> +		break;
>> +
>> +	case PHY_INTERFACE_MODE_GMII:
>> +		val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, MACPHYC_TXCLK_SEL_INPUT) |
>> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_GMII);
>> +		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_GMII\n");
>> +		break;
>> +
>> +	case PHY_INTERFACE_MODE_RMII:
>> +		val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, MACPHYC_TXCLK_SEL_INPUT) |
>> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RMII);
>> +		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_RMII\n");
>> +		break;
>> +
>> +	case PHY_INTERFACE_MODE_RGMII:
> What about the other three RGMII modes?
>
>> +	case PHY_INTERFACE_MODE_RGMII:
>> +		val = FIELD_PREP(MACPHYC_TX_SEL_MASK, MACPHYC_TX_SEL_DELAY) |
>> +			  FIELD_PREP(MACPHYC_TX_DELAY_MASK, MACPHYC_TX_DELAY_63_UNIT) |
>> +			  FIELD_PREP(MACPHYC_RX_SEL_MASK, MACPHYC_RX_SEL_ORIGIN) |
>> +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RGMII);
> What exactly does MACPHYC_TX_DELAY_63_UNIT mean here? Ideally, the MAC
> should not be adding any RGMII delays. It should however pass mode
> through to the PHY, so it can add the delays, if the mode indicates it
> should, e.g. PHY_INTERFACE_MODE_RGMII_ID. This is also why you should
> be handling all 4 RGMII modes here, not just one.


MACPHYC_TX_DELAY_63_UNIT means set MAC TX clk delay to 63 units (similar to the "tx-delay" in dwmac-rk.c). However, the manual does not clearly describe the time span of one unit, after consulting engineer of Ingenic, I learned that the value is recommended to be set to 63.
I will change it to be similar to the way done in dwmac-rk.c.

Thanks and best regards!


>
> 	 Andrew
Andrew Lunn June 8, 2021, 12:21 p.m. UTC | #4
> > > +	case PHY_INTERFACE_MODE_RGMII:
> > > +		val = FIELD_PREP(MACPHYC_TX_SEL_MASK, MACPHYC_TX_SEL_DELAY) |
> > > +			  FIELD_PREP(MACPHYC_TX_DELAY_MASK, MACPHYC_TX_DELAY_63_UNIT) |
> > > +			  FIELD_PREP(MACPHYC_RX_SEL_MASK, MACPHYC_RX_SEL_ORIGIN) |
> > > +			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RGMII);
> > What exactly does MACPHYC_TX_DELAY_63_UNIT mean here? Ideally, the MAC
> > should not be adding any RGMII delays. It should however pass mode
> > through to the PHY, so it can add the delays, if the mode indicates it
> > should, e.g. PHY_INTERFACE_MODE_RGMII_ID. This is also why you should
> > be handling all 4 RGMII modes here, not just one.
> 
> 
> MACPHYC_TX_DELAY_63_UNIT means set MAC TX clk delay to 63 units (similar to the "tx-delay" in dwmac-rk.c). However, the manual does not clearly describe the time span of one unit, after consulting engineer of Ingenic, I learned that the value is recommended to be set to 63.
> I will change it to be similar to the way done in dwmac-rk.c.

Please wrap your text to around 75 characters per line.

I suspect you don't understand RGMII delays. As i said, normally, the
MAC does not add delays, the PHY does. Please take a closer look at
this.

	Andrew
Zhou Yanjie June 8, 2021, 1:24 p.m. UTC | #5
Hi Paul,

On 2021/6/8 下午4:46, Paul Cercueil wrote:
> Hi Zhou,
>
> Le mar., juin 8 2021 at 01:27:47 +0800, 周琰杰 (Zhou Yanjie) 
> <zhouyanjie@wanyeetech.com> a écrit :
>> Add support for Ingenic SoC MAC glue layer support for the stmmac
>> device driver. This driver is used on for the MAC ethernet controller
>> found in the JZ4775 SoC, the X1000 SoC, the X1600 SoC, the X1830 SoC,
>> and the X2000 SoC.
>>
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>> ---
>>  drivers/net/ethernet/stmicro/stmmac/Kconfig        |  16 +-
>>  drivers/net/ethernet/stmicro/stmmac/Makefile       |   1 +
>>  .../net/ethernet/stmicro/stmmac/dwmac-ingenic.c    | 367 
>> +++++++++++++++++++++
>>  3 files changed, 382 insertions(+), 2 deletions(-)
>>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-ingenic.c
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig 
>> b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> index 7737e4d0..fb58537 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> @@ -66,6 +66,18 @@ config DWMAC_ANARION
>>
>>        This selects the Anarion SoC glue layer support for the stmmac 
>> driver.
>>
>> +config DWMAC_INGENIC
>> +    tristate "Ingenic MAC support"
>> +    default MACH_INGENIC
>> +    depends on OF && HAS_IOMEM && (MACH_INGENIC || COMPILE_TEST)
>> +    select MFD_SYSCON
>> +    help
>> +      Support for ethernet controller on Ingenic SoCs.
>> +
>> +      This selects Ingenic SoCs glue layer support for the stmmac
>> +      device driver. This driver is used on for the Ingenic SoCs
>> +      MAC ethernet controller.
>> +
>>  config DWMAC_IPQ806X
>>      tristate "QCA IPQ806x DWMAC support"
>>      default ARCH_QCOM
>> @@ -129,7 +141,7 @@ config DWMAC_QCOM_ETHQOS
>>
>>  config DWMAC_ROCKCHIP
>>      tristate "Rockchip dwmac support"
>> -    default ARCH_ROCKCHIP
>> +    default MACH_ROCKCHIP
>>      depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST)
>>      select MFD_SYSCON
>>      help
>> @@ -164,7 +176,7 @@ config DWMAC_STI
>>
>>  config DWMAC_STM32
>>      tristate "STM32 DWMAC support"
>> -    default ARCH_STM32
>> +    default MACH_STM32
>>      depends on OF && HAS_IOMEM && (ARCH_STM32 || COMPILE_TEST)
>>      select MFD_SYSCON
>>      help
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile 
>> b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> index f2e478b..6471f93 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
>> @@ -14,6 +14,7 @@ stmmac-$(CONFIG_STMMAC_SELFTESTS) += 
>> stmmac_selftests.o
>>  # Ordering matters. Generic driver must be last.
>>  obj-$(CONFIG_STMMAC_PLATFORM)    += stmmac-platform.o
>>  obj-$(CONFIG_DWMAC_ANARION)    += dwmac-anarion.o
>> +obj-$(CONFIG_DWMAC_INGENIC)    += dwmac-ingenic.o
>>  obj-$(CONFIG_DWMAC_IPQ806X)    += dwmac-ipq806x.o
>>  obj-$(CONFIG_DWMAC_LPC18XX)    += dwmac-lpc18xx.o
>>  obj-$(CONFIG_DWMAC_MEDIATEK)    += dwmac-mediatek.o
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-ingenic.c 
>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-ingenic.c
>> new file mode 100644
>> index 00000000..8be8caa
>> --- /dev/null
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-ingenic.c
>> @@ -0,0 +1,367 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * dwmac-ingenic.c - Ingenic SoCs DWMAC specific glue layer
>> + *
>> + * Copyright (c) 2020 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>
> 2021?


Sure, I will change it.


>
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/clk.h>
>> +#include <linux/kernel.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_net.h>
>> +#include <linux/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +#include <linux/stmmac.h>
>> +
>> +#include "stmmac_platform.h"
>> +
>> +#define MACPHYC_TXCLK_SEL_MASK        GENMASK(31, 31)
>> +#define MACPHYC_TXCLK_SEL_OUTPUT    0x1
>> +#define MACPHYC_TXCLK_SEL_INPUT        0x0
>> +#define MACPHYC_MODE_SEL_MASK        GENMASK(31, 31)
>> +#define MACPHYC_MODE_SEL_RMII        0x0
>> +#define MACPHYC_TX_SEL_MASK            GENMASK(19, 19)
>> +#define MACPHYC_TX_SEL_ORIGIN        0x0
>> +#define MACPHYC_TX_SEL_DELAY        0x1
>> +#define MACPHYC_TX_DELAY_MASK        GENMASK(18, 12)
>> +#define MACPHYC_TX_DELAY_63_UNIT    0x3e
>> +#define MACPHYC_RX_SEL_MASK            GENMASK(11, 11)
>> +#define MACPHYC_RX_SEL_ORIGIN        0x0
>> +#define MACPHYC_RX_SEL_DELAY        0x1
>> +#define MACPHYC_RX_DELAY_MASK        GENMASK(10, 4)
>> +#define MACPHYC_SOFT_RST_MASK        GENMASK(3, 3)
>> +#define MACPHYC_PHY_INFT_MASK        GENMASK(2, 0)
>> +#define MACPHYC_PHY_INFT_RMII        0x4
>> +#define MACPHYC_PHY_INFT_RGMII        0x1
>> +#define MACPHYC_PHY_INFT_GMII        0x0
>> +#define MACPHYC_PHY_INFT_MII        0x0
>> +
>> +enum ingenic_mac_version {
>> +    ID_JZ4775,
>> +    ID_X1000,
>> +    ID_X1600,
>> +    ID_X1830,
>> +    ID_X2000,
>
> You could test it on all these? I never heard about the X1600 before.
>

Yes, X1600 is a new model for industrial control applications that has

just been launched. It has two CAN interfaces and one CDBUS interface.


>> +};
>> +
>> +struct ingenic_mac {
>> +    const struct ingenic_soc_info *soc_info;
>> +    struct device *dev;
>> +    struct regmap *regmap;
>> +};
>> +
>> +struct ingenic_soc_info {
>> +    enum ingenic_mac_version version;
>> +    u32 mask;
>> +
>> +    int (*set_mode)(struct plat_stmmacenet_data *plat_dat);
>> +    int (*suspend)(struct ingenic_mac *mac);
>> +    void (*resume)(struct ingenic_mac *mac);
>
> These suspend/resume callbacks are not used anywhere - just drop them.
>

Sure.


>> +};
>> +
>> +static int ingenic_mac_init(struct plat_stmmacenet_data *plat_dat)
>> +{
>> +    struct ingenic_mac *mac = plat_dat->bsp_priv;
>> +    int ret;
>> +
>> +    if (mac->soc_info->set_mode) {
>> +        ret = mac->soc_info->set_mode(plat_dat);
>> +        if (ret)
>> +            return ret;
>> +    }
>> +
>> +    return ret;
>
> You are returning an uninitialized variable.
>

Sure, I'll change it in the next version.


>> +}
>> +
>> +static int jz4775_mac_set_mode(struct plat_stmmacenet_data *plat_dat)
>> +{
>> +    struct ingenic_mac *mac = plat_dat->bsp_priv;
>> +    int val;
>
> unsigned int val;
>

Sure.


>> +
>> +    switch (plat_dat->interface) {
>> +    case PHY_INTERFACE_MODE_MII:
>> +        val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, 
>> MACPHYC_TXCLK_SEL_INPUT) |
>> +              FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_MII);
>> +        pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_MII\n");
>
> Use dev_dbg() with mac->dev, instead of pr_debug().
>
> (Same for all pr_debug() calls below)
>

Sure.


>> +        break;
>> +
>> +    case PHY_INTERFACE_MODE_GMII:
>> +        val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, 
>> MACPHYC_TXCLK_SEL_INPUT) |
>> +              FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_GMII);
>> +        pr_debug("MAC PHY Control Register: 
>> PHY_INTERFACE_MODE_GMII\n");
>> +        break;
>> +
>> +    case PHY_INTERFACE_MODE_RMII:
>> +        val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, 
>> MACPHYC_TXCLK_SEL_INPUT) |
>> +              FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RMII);
>> +        pr_debug("MAC PHY Control Register: 
>> PHY_INTERFACE_MODE_RMII\n");
>> +        break;
>> +
>> +    case PHY_INTERFACE_MODE_RGMII:
>> +        val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, 
>> MACPHYC_TXCLK_SEL_INPUT) |
>> +              FIELD_PREP(MACPHYC_PHY_INFT_MASK, 
>> MACPHYC_PHY_INFT_RGMII);
>> +        pr_debug("MAC PHY Control Register: 
>> PHY_INTERFACE_MODE_RGMII\n");
>> +        break;
>> +
>> +    default:
>> +        dev_err(mac->dev, "unsupported interface %d", 
>> plat_dat->interface);
>> +        return -EINVAL;
>> +    }
>> +
>> +    /* Update MAC PHY control register */
>> +    return regmap_update_bits(mac->regmap, 0, mac->soc_info->mask, 
>> val);
>> +}
>> +
>> +static int x1000_mac_set_mode(struct plat_stmmacenet_data *plat_dat)
>> +{
>> +    struct ingenic_mac *mac = plat_dat->bsp_priv;
>> +    int val;
>> +
>> +    switch (plat_dat->interface) {
>> +    case PHY_INTERFACE_MODE_RMII:
>> +        pr_debug("MAC PHY Control Register: 
>> PHY_INTERFACE_MODE_RMII\n");
>> +        break;
>> +
>> +    default:
>> +        dev_err(mac->dev, "unsupported interface %d", 
>> plat_dat->interface);
>> +        return -EINVAL;
>> +    }
>> +
>> +    /* Update MAC PHY control register */
>> +    return regmap_update_bits(mac->regmap, 0, mac->soc_info->mask, 
>> val);
>
> You're passing 'val', which is an uninitialized variable.
>

I will fix this int the next version.


>> +}
>> +
>> +static int x1600_mac_set_mode(struct plat_stmmacenet_data *plat_dat)
>> +{
>> +    struct ingenic_mac *mac = plat_dat->bsp_priv;
>> +    int val;
>
> unsigned int val;
>

Sure.


>> +
>> +    switch (plat_dat->interface) {
>> +    case PHY_INTERFACE_MODE_RMII:
>> +        val = FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RMII);
>> +        pr_debug("MAC PHY Control Register: 
>> PHY_INTERFACE_MODE_RMII\n");
>> +        break;
>> +
>> +    default:
>> +        dev_err(mac->dev, "unsupported interface %d", 
>> plat_dat->interface);
>> +        return -EINVAL;
>> +    }
>> +
>> +    /* Update MAC PHY control register */
>> +    return regmap_update_bits(mac->regmap, 0, mac->soc_info->mask, 
>> val);
>> +}
>> +
>> +static int x1830_mac_set_mode(struct plat_stmmacenet_data *plat_dat)
>> +{
>> +    struct ingenic_mac *mac = plat_dat->bsp_priv;
>> +    int val;
>
> Same here,
>

Sure.


>> +
>> +    switch (plat_dat->interface) {
>> +    case PHY_INTERFACE_MODE_RMII:
>> +        val = FIELD_PREP(MACPHYC_MODE_SEL_MASK, 
>> MACPHYC_MODE_SEL_RMII) |
>> +              FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RMII);
>> +        pr_debug("MAC PHY Control Register: 
>> PHY_INTERFACE_MODE_RMII\n");
>> +        break;
>> +
>> +    default:
>> +        dev_err(mac->dev, "unsupported interface %d", 
>> plat_dat->interface);
>> +        return -EINVAL;
>> +    }
>> +
>> +    /* Update MAC PHY control register */
>> +    return regmap_update_bits(mac->regmap, 0, mac->soc_info->mask, 
>> val);
>> +}
>> +
>> +static int x2000_mac_set_mode(struct plat_stmmacenet_data *plat_dat)
>> +{
>> +    struct ingenic_mac *mac = plat_dat->bsp_priv;
>> +    int val;
>
> Same here.
>

Sure.


>> +
>> +    switch (plat_dat->interface) {
>> +    case PHY_INTERFACE_MODE_RMII:
>> +        val = FIELD_PREP(MACPHYC_TX_SEL_MASK, MACPHYC_TX_SEL_ORIGIN) |
>> +              FIELD_PREP(MACPHYC_RX_SEL_MASK, MACPHYC_RX_SEL_ORIGIN) |
>> +              FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RMII);
>> +        pr_debug("MAC PHY Control Register: 
>> PHY_INTERFACE_MODE_RMII\n");
>> +        break;
>> +
>> +    case PHY_INTERFACE_MODE_RGMII:
>> +        val = FIELD_PREP(MACPHYC_TX_SEL_MASK, MACPHYC_TX_SEL_DELAY) |
>> +              FIELD_PREP(MACPHYC_TX_DELAY_MASK, 
>> MACPHYC_TX_DELAY_63_UNIT) |
>> +              FIELD_PREP(MACPHYC_RX_SEL_MASK, MACPHYC_RX_SEL_ORIGIN) |
>> +              FIELD_PREP(MACPHYC_PHY_INFT_MASK, 
>> MACPHYC_PHY_INFT_RGMII);
>> +        pr_debug("MAC PHY Control Register: 
>> PHY_INTERFACE_MODE_RGMII\n");
>> +        break;
>> +
>> +    default:
>> +        dev_err(mac->dev, "unsupported interface %d", 
>> plat_dat->interface);
>> +        return -EINVAL;
>> +    }
>> +
>> +    /* Update MAC PHY control register */
>> +    return regmap_update_bits(mac->regmap, 0, mac->soc_info->mask, 
>> val);
>> +}
>> +
>> +static int ingenic_mac_probe(struct platform_device *pdev)
>> +{
>> +    struct plat_stmmacenet_data *plat_dat;
>> +    struct stmmac_resources stmmac_res;
>> +    struct ingenic_mac *mac;
>> +    const struct ingenic_soc_info *data;
>> +    int ret;
>> +
>> +    ret = stmmac_get_platform_resources(pdev, &stmmac_res);
>> +    if (ret)
>> +        return ret;
>> +
>> +    plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
>> +    if (IS_ERR(plat_dat))
>> +        return PTR_ERR(plat_dat);
>> +
>> +    mac = devm_kzalloc(&pdev->dev, sizeof(*mac), GFP_KERNEL);
>> +    if (!mac) {
>> +        ret = -ENOMEM;
>> +        goto err_remove_config_dt;
>> +    }
>> +
>> +    data = of_device_get_match_data(&pdev->dev);
>> +    if (!data) {
>> +        dev_err(&pdev->dev, "no of match data provided\n");
>> +        ret = -EINVAL;
>> +        goto err_remove_config_dt;
>> +    }
>> +
>> +    /* Get MAC PHY control register */
>> +    mac->regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, 
>> "mode-reg");
>> +    if (IS_ERR(mac->regmap)) {
>> +        pr_err("%s: failed to get syscon regmap\n", __func__);
>
> dev_err?
>

Sure, I will change this in v2.


>> +        goto err_remove_config_dt;
>> +    }
>> +
>> +    mac->soc_info = data;
>> +    mac->dev = &pdev->dev;
>> +
>> +    plat_dat->bsp_priv = mac;
>> +
>> +    ret = ingenic_mac_init(plat_dat);
>> +    if (ret)
>> +        goto err_remove_config_dt;
>> +
>> +    ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>> +    if (ret)
>> +        goto err_remove_config_dt;
>> +
>> +    return 0;
>> +
>> +err_remove_config_dt:
>> +    stmmac_remove_config_dt(pdev, plat_dat);
>> +
>> +    return ret;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>
> Remove this #ifdef.
>

Sure.


>> +static int ingenic_mac_suspend(struct device *dev)
>> +{
>> +    struct net_device *ndev = dev_get_drvdata(dev);
>> +    struct stmmac_priv *priv = netdev_priv(ndev);
>> +    struct ingenic_mac *mac = priv->plat->bsp_priv;
>> +
>> +    int ret;
>> +
>> +    ret = stmmac_suspend(dev);
>> +
>> +    if (mac->soc_info->suspend)
>> +        ret = mac->soc_info->suspend(mac);
>> +
>> +    return ret;
>> +}
>> +
>> +static int ingenic_mac_resume(struct device *dev)
>> +{
>> +    struct net_device *ndev = dev_get_drvdata(dev);
>> +    struct stmmac_priv *priv = netdev_priv(ndev);
>> +    struct ingenic_mac *mac = priv->plat->bsp_priv;
>> +    int ret;
>> +
>> +    if (mac->soc_info->resume)
>> +        mac->soc_info->resume(mac);
>> +
>> +    ret = ingenic_mac_init(priv->plat);
>> +    if (ret)
>> +        return ret;
>> +
>> +    ret = stmmac_resume(dev);
>> +
>> +    return ret;
>> +}
>> +#endif /* CONFIG_PM_SLEEP */
>> +
>> +static SIMPLE_DEV_PM_OPS(ingenic_mac_pm_ops,
>> +    ingenic_mac_suspend, ingenic_mac_resume);
>> +
>> +static struct ingenic_soc_info jz4775_soc_info = {
>> +    .version = ID_JZ4775,
>> +    .mask = MACPHYC_TXCLK_SEL_MASK | MACPHYC_SOFT_RST_MASK | 
>> MACPHYC_PHY_INFT_MASK,
>> +
>> +    .set_mode = jz4775_mac_set_mode,
>> +};
>> +
>> +static struct ingenic_soc_info x1000_soc_info = {
>> +    .version = ID_X1000,
>> +    .mask = MACPHYC_SOFT_RST_MASK,
>> +
>> +    .set_mode = x1000_mac_set_mode,
>> +};
>> +
>> +static struct ingenic_soc_info x1600_soc_info = {
>> +    .version = ID_X1600,
>> +    .mask = MACPHYC_SOFT_RST_MASK | MACPHYC_PHY_INFT_MASK,
>> +
>> +    .set_mode = x1600_mac_set_mode,
>> +};
>> +
>> +static struct ingenic_soc_info x1830_soc_info = {
>> +    .version = ID_X1830,
>> +    .mask = MACPHYC_MODE_SEL_MASK | MACPHYC_SOFT_RST_MASK | 
>> MACPHYC_PHY_INFT_MASK,
>> +
>> +    .set_mode = x1830_mac_set_mode,
>> +};
>> +
>> +static struct ingenic_soc_info x2000_soc_info = {
>> +    .version = ID_X2000,
>> +    .mask = MACPHYC_TX_SEL_MASK | MACPHYC_TX_DELAY_MASK | 
>> MACPHYC_RX_SEL_MASK |
>> +            MACPHYC_RX_DELAY_MASK | MACPHYC_SOFT_RST_MASK | 
>> MACPHYC_PHY_INFT_MASK,
>> +
>> +    .set_mode = x2000_mac_set_mode,
>> +};
>> +
>> +static const struct of_device_id ingenic_mac_of_matches[] = {
>> +    { .compatible = "ingenic,jz4775-mac", .data = &jz4775_soc_info },
>> +    { .compatible = "ingenic,x1000-mac", .data = &x1000_soc_info },
>> +    { .compatible = "ingenic,x1600-mac", .data = &x1600_soc_info },
>> +    { .compatible = "ingenic,x1830-mac", .data = &x1830_soc_info },
>> +    { .compatible = "ingenic,x2000-mac", .data = &x2000_soc_info },
>> +    { }
>> +};
>> +MODULE_DEVICE_TABLE(of, ingenic_mac_of_matches);
>> +
>> +static struct platform_driver ingenic_mac_driver = {
>> +    .probe        = ingenic_mac_probe,
>> +    .remove        = stmmac_pltfr_remove,
>> +    .driver        = {
>> +        .name    = "ingenic-mac",
>> +        .pm        = &ingenic_mac_pm_ops,
>
> .pm = pm_ptr(&ingenic_mac_pm_ops),
>

Sure.


Thanks and best regards!


>> +        .of_match_table = ingenic_mac_of_matches,
>> +    },
>> +};
>> +module_platform_driver(ingenic_mac_driver);
>> +
>> +MODULE_AUTHOR("周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>");
>> +MODULE_DESCRIPTION("Ingenic SoCs DWMAC specific glue layer");
>> +MODULE_LICENSE("GPL v2");
>> -- 
>> 2.7.4
>>
>
> Cheers,
> -Paul
>
Zhou Yanjie June 8, 2021, 1:48 p.m. UTC | #6
Hello Andrew,

On 2021/6/8 下午8:21, Andrew Lunn wrote:
> Please wrap your text to around 75 characters per line.


Sure.


>
> I suspect you don't understand RGMII delays. As i said, normally, the
> MAC does not add delays, the PHY does. Please take a closer look at
> this.


According to the description of ethernet-controller.yaml, "rgmii" seems

to allow MAC to add TX delay (the description in ethernet-controller.yaml

is "RX and TX delays are added by the MAC when required"), while rgmii-id

and rgmii-txid do not allow MAC to add delay (the description in

ethernet-controller.yaml is"RGMII with internal RX and TX delays provided

by the PHY, the MAC should not add the RX or TX delays in this case" and

"RGMII with internal TX delay provided by the PHY, the MAC should not add

an TX delay in this case"), I will add support for the other three RGMII 
modes

in the next version (I forgot to reply to this in the previous email).


Thanks and best regards!
Andrew Lunn June 8, 2021, 2:04 p.m. UTC | #7
On Tue, Jun 08, 2021 at 09:48:38PM +0800, Zhou Yanjie wrote:
> Hello Andrew,
> 
> On 2021/6/8 下午8:21, Andrew Lunn wrote:
> > Please wrap your text to around 75 characters per line.
> 
> 
> Sure.
> 
> 
> > 
> > I suspect you don't understand RGMII delays. As i said, normally, the
> > MAC does not add delays, the PHY does. Please take a closer look at
> > this.
> 
> 
> According to the description of ethernet-controller.yaml, "rgmii" seems
> 
> to allow MAC to add TX delay (the description in ethernet-controller.yaml
> 
> is "RX and TX delays are added by the MAC when required"), while rgmii-id
> 
> and rgmii-txid do not allow MAC to add delay (the description in
> 
> ethernet-controller.yaml is"RGMII with internal RX and TX delays provided
> 
> by the PHY, the MAC should not add the RX or TX delays in this case" and
> 
> "RGMII with internal TX delay provided by the PHY, the MAC should not add
> 
> an TX delay in this case"), I will add support for the other three RGMII
> modes
> 
> in the next version (I forgot to reply to this in the previous email).

Please follow the code all the way through. What gets passed to
phylink_create() is very important. If you have both the MAC and the
PHY adding delays, bad things will happen.

    Andrew
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 7737e4d0..fb58537 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -66,6 +66,18 @@  config DWMAC_ANARION
 
 	  This selects the Anarion SoC glue layer support for the stmmac driver.
 
+config DWMAC_INGENIC
+	tristate "Ingenic MAC support"
+	default MACH_INGENIC
+	depends on OF && HAS_IOMEM && (MACH_INGENIC || COMPILE_TEST)
+	select MFD_SYSCON
+	help
+	  Support for ethernet controller on Ingenic SoCs.
+
+	  This selects Ingenic SoCs glue layer support for the stmmac
+	  device driver. This driver is used on for the Ingenic SoCs
+	  MAC ethernet controller.
+
 config DWMAC_IPQ806X
 	tristate "QCA IPQ806x DWMAC support"
 	default ARCH_QCOM
@@ -129,7 +141,7 @@  config DWMAC_QCOM_ETHQOS
 
 config DWMAC_ROCKCHIP
 	tristate "Rockchip dwmac support"
-	default ARCH_ROCKCHIP
+	default MACH_ROCKCHIP
 	depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST)
 	select MFD_SYSCON
 	help
@@ -164,7 +176,7 @@  config DWMAC_STI
 
 config DWMAC_STM32
 	tristate "STM32 DWMAC support"
-	default ARCH_STM32
+	default MACH_STM32
 	depends on OF && HAS_IOMEM && (ARCH_STM32 || COMPILE_TEST)
 	select MFD_SYSCON
 	help
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index f2e478b..6471f93 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -14,6 +14,7 @@  stmmac-$(CONFIG_STMMAC_SELFTESTS) += stmmac_selftests.o
 # Ordering matters. Generic driver must be last.
 obj-$(CONFIG_STMMAC_PLATFORM)	+= stmmac-platform.o
 obj-$(CONFIG_DWMAC_ANARION)	+= dwmac-anarion.o
+obj-$(CONFIG_DWMAC_INGENIC)	+= dwmac-ingenic.o
 obj-$(CONFIG_DWMAC_IPQ806X)	+= dwmac-ipq806x.o
 obj-$(CONFIG_DWMAC_LPC18XX)	+= dwmac-lpc18xx.o
 obj-$(CONFIG_DWMAC_MEDIATEK)	+= dwmac-mediatek.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-ingenic.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-ingenic.c
new file mode 100644
index 00000000..8be8caa
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-ingenic.c
@@ -0,0 +1,367 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * dwmac-ingenic.c - Ingenic SoCs DWMAC specific glue layer
+ *
+ * Copyright (c) 2020 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_net.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/stmmac.h>
+
+#include "stmmac_platform.h"
+
+#define MACPHYC_TXCLK_SEL_MASK		GENMASK(31, 31)
+#define MACPHYC_TXCLK_SEL_OUTPUT	0x1
+#define MACPHYC_TXCLK_SEL_INPUT		0x0
+#define MACPHYC_MODE_SEL_MASK		GENMASK(31, 31)
+#define MACPHYC_MODE_SEL_RMII		0x0
+#define MACPHYC_TX_SEL_MASK			GENMASK(19, 19)
+#define MACPHYC_TX_SEL_ORIGIN		0x0
+#define MACPHYC_TX_SEL_DELAY		0x1
+#define MACPHYC_TX_DELAY_MASK		GENMASK(18, 12)
+#define MACPHYC_TX_DELAY_63_UNIT	0x3e
+#define MACPHYC_RX_SEL_MASK			GENMASK(11, 11)
+#define MACPHYC_RX_SEL_ORIGIN		0x0
+#define MACPHYC_RX_SEL_DELAY		0x1
+#define MACPHYC_RX_DELAY_MASK		GENMASK(10, 4)
+#define MACPHYC_SOFT_RST_MASK		GENMASK(3, 3)
+#define MACPHYC_PHY_INFT_MASK		GENMASK(2, 0)
+#define MACPHYC_PHY_INFT_RMII		0x4
+#define MACPHYC_PHY_INFT_RGMII		0x1
+#define MACPHYC_PHY_INFT_GMII		0x0
+#define MACPHYC_PHY_INFT_MII		0x0
+
+enum ingenic_mac_version {
+	ID_JZ4775,
+	ID_X1000,
+	ID_X1600,
+	ID_X1830,
+	ID_X2000,
+};
+
+struct ingenic_mac {
+	const struct ingenic_soc_info *soc_info;
+	struct device *dev;
+	struct regmap *regmap;
+};
+
+struct ingenic_soc_info {
+	enum ingenic_mac_version version;
+	u32 mask;
+
+	int (*set_mode)(struct plat_stmmacenet_data *plat_dat);
+	int (*suspend)(struct ingenic_mac *mac);
+	void (*resume)(struct ingenic_mac *mac);
+};
+
+static int ingenic_mac_init(struct plat_stmmacenet_data *plat_dat)
+{
+	struct ingenic_mac *mac = plat_dat->bsp_priv;
+	int ret;
+
+	if (mac->soc_info->set_mode) {
+		ret = mac->soc_info->set_mode(plat_dat);
+		if (ret)
+			return ret;
+	}
+
+	return ret;
+}
+
+static int jz4775_mac_set_mode(struct plat_stmmacenet_data *plat_dat)
+{
+	struct ingenic_mac *mac = plat_dat->bsp_priv;
+	int val;
+
+	switch (plat_dat->interface) {
+	case PHY_INTERFACE_MODE_MII:
+		val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, MACPHYC_TXCLK_SEL_INPUT) |
+			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_MII);
+		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_MII\n");
+		break;
+
+	case PHY_INTERFACE_MODE_GMII:
+		val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, MACPHYC_TXCLK_SEL_INPUT) |
+			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_GMII);
+		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_GMII\n");
+		break;
+
+	case PHY_INTERFACE_MODE_RMII:
+		val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, MACPHYC_TXCLK_SEL_INPUT) |
+			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RMII);
+		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_RMII\n");
+		break;
+
+	case PHY_INTERFACE_MODE_RGMII:
+		val = FIELD_PREP(MACPHYC_TXCLK_SEL_MASK, MACPHYC_TXCLK_SEL_INPUT) |
+			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RGMII);
+		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_RGMII\n");
+		break;
+
+	default:
+		dev_err(mac->dev, "unsupported interface %d", plat_dat->interface);
+		return -EINVAL;
+	}
+
+	/* Update MAC PHY control register */
+	return regmap_update_bits(mac->regmap, 0, mac->soc_info->mask, val);
+}
+
+static int x1000_mac_set_mode(struct plat_stmmacenet_data *plat_dat)
+{
+	struct ingenic_mac *mac = plat_dat->bsp_priv;
+	int val;
+
+	switch (plat_dat->interface) {
+	case PHY_INTERFACE_MODE_RMII:
+		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_RMII\n");
+		break;
+
+	default:
+		dev_err(mac->dev, "unsupported interface %d", plat_dat->interface);
+		return -EINVAL;
+	}
+
+	/* Update MAC PHY control register */
+	return regmap_update_bits(mac->regmap, 0, mac->soc_info->mask, val);
+}
+
+static int x1600_mac_set_mode(struct plat_stmmacenet_data *plat_dat)
+{
+	struct ingenic_mac *mac = plat_dat->bsp_priv;
+	int val;
+
+	switch (plat_dat->interface) {
+	case PHY_INTERFACE_MODE_RMII:
+		val = FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RMII);
+		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_RMII\n");
+		break;
+
+	default:
+		dev_err(mac->dev, "unsupported interface %d", plat_dat->interface);
+		return -EINVAL;
+	}
+
+	/* Update MAC PHY control register */
+	return regmap_update_bits(mac->regmap, 0, mac->soc_info->mask, val);
+}
+
+static int x1830_mac_set_mode(struct plat_stmmacenet_data *plat_dat)
+{
+	struct ingenic_mac *mac = plat_dat->bsp_priv;
+	int val;
+
+	switch (plat_dat->interface) {
+	case PHY_INTERFACE_MODE_RMII:
+		val = FIELD_PREP(MACPHYC_MODE_SEL_MASK, MACPHYC_MODE_SEL_RMII) |
+			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RMII);
+		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_RMII\n");
+		break;
+
+	default:
+		dev_err(mac->dev, "unsupported interface %d", plat_dat->interface);
+		return -EINVAL;
+	}
+
+	/* Update MAC PHY control register */
+	return regmap_update_bits(mac->regmap, 0, mac->soc_info->mask, val);
+}
+
+static int x2000_mac_set_mode(struct plat_stmmacenet_data *plat_dat)
+{
+	struct ingenic_mac *mac = plat_dat->bsp_priv;
+	int val;
+
+	switch (plat_dat->interface) {
+	case PHY_INTERFACE_MODE_RMII:
+		val = FIELD_PREP(MACPHYC_TX_SEL_MASK, MACPHYC_TX_SEL_ORIGIN) |
+			  FIELD_PREP(MACPHYC_RX_SEL_MASK, MACPHYC_RX_SEL_ORIGIN) |
+			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RMII);
+		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_RMII\n");
+		break;
+
+	case PHY_INTERFACE_MODE_RGMII:
+		val = FIELD_PREP(MACPHYC_TX_SEL_MASK, MACPHYC_TX_SEL_DELAY) |
+			  FIELD_PREP(MACPHYC_TX_DELAY_MASK, MACPHYC_TX_DELAY_63_UNIT) |
+			  FIELD_PREP(MACPHYC_RX_SEL_MASK, MACPHYC_RX_SEL_ORIGIN) |
+			  FIELD_PREP(MACPHYC_PHY_INFT_MASK, MACPHYC_PHY_INFT_RGMII);
+		pr_debug("MAC PHY Control Register: PHY_INTERFACE_MODE_RGMII\n");
+		break;
+
+	default:
+		dev_err(mac->dev, "unsupported interface %d", plat_dat->interface);
+		return -EINVAL;
+	}
+
+	/* Update MAC PHY control register */
+	return regmap_update_bits(mac->regmap, 0, mac->soc_info->mask, val);
+}
+
+static int ingenic_mac_probe(struct platform_device *pdev)
+{
+	struct plat_stmmacenet_data *plat_dat;
+	struct stmmac_resources stmmac_res;
+	struct ingenic_mac *mac;
+	const struct ingenic_soc_info *data;
+	int ret;
+
+	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
+	if (ret)
+		return ret;
+
+	plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
+	if (IS_ERR(plat_dat))
+		return PTR_ERR(plat_dat);
+
+	mac = devm_kzalloc(&pdev->dev, sizeof(*mac), GFP_KERNEL);
+	if (!mac) {
+		ret = -ENOMEM;
+		goto err_remove_config_dt;
+	}
+
+	data = of_device_get_match_data(&pdev->dev);
+	if (!data) {
+		dev_err(&pdev->dev, "no of match data provided\n");
+		ret = -EINVAL;
+		goto err_remove_config_dt;
+	}
+
+	/* Get MAC PHY control register */
+	mac->regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "mode-reg");
+	if (IS_ERR(mac->regmap)) {
+		pr_err("%s: failed to get syscon regmap\n", __func__);
+		goto err_remove_config_dt;
+	}
+
+	mac->soc_info = data;
+	mac->dev = &pdev->dev;
+
+	plat_dat->bsp_priv = mac;
+
+	ret = ingenic_mac_init(plat_dat);
+	if (ret)
+		goto err_remove_config_dt;
+
+	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	if (ret)
+		goto err_remove_config_dt;
+
+	return 0;
+
+err_remove_config_dt:
+	stmmac_remove_config_dt(pdev, plat_dat);
+
+	return ret;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int ingenic_mac_suspend(struct device *dev)
+{
+	struct net_device *ndev = dev_get_drvdata(dev);
+	struct stmmac_priv *priv = netdev_priv(ndev);
+	struct ingenic_mac *mac = priv->plat->bsp_priv;
+
+	int ret;
+
+	ret = stmmac_suspend(dev);
+
+	if (mac->soc_info->suspend)
+		ret = mac->soc_info->suspend(mac);
+
+	return ret;
+}
+
+static int ingenic_mac_resume(struct device *dev)
+{
+	struct net_device *ndev = dev_get_drvdata(dev);
+	struct stmmac_priv *priv = netdev_priv(ndev);
+	struct ingenic_mac *mac = priv->plat->bsp_priv;
+	int ret;
+
+	if (mac->soc_info->resume)
+		mac->soc_info->resume(mac);
+
+	ret = ingenic_mac_init(priv->plat);
+	if (ret)
+		return ret;
+
+	ret = stmmac_resume(dev);
+
+	return ret;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static SIMPLE_DEV_PM_OPS(ingenic_mac_pm_ops,
+	ingenic_mac_suspend, ingenic_mac_resume);
+
+static struct ingenic_soc_info jz4775_soc_info = {
+	.version = ID_JZ4775,
+	.mask = MACPHYC_TXCLK_SEL_MASK | MACPHYC_SOFT_RST_MASK | MACPHYC_PHY_INFT_MASK,
+
+	.set_mode = jz4775_mac_set_mode,
+};
+
+static struct ingenic_soc_info x1000_soc_info = {
+	.version = ID_X1000,
+	.mask = MACPHYC_SOFT_RST_MASK,
+
+	.set_mode = x1000_mac_set_mode,
+};
+
+static struct ingenic_soc_info x1600_soc_info = {
+	.version = ID_X1600,
+	.mask = MACPHYC_SOFT_RST_MASK | MACPHYC_PHY_INFT_MASK,
+
+	.set_mode = x1600_mac_set_mode,
+};
+
+static struct ingenic_soc_info x1830_soc_info = {
+	.version = ID_X1830,
+	.mask = MACPHYC_MODE_SEL_MASK | MACPHYC_SOFT_RST_MASK | MACPHYC_PHY_INFT_MASK,
+
+	.set_mode = x1830_mac_set_mode,
+};
+
+static struct ingenic_soc_info x2000_soc_info = {
+	.version = ID_X2000,
+	.mask = MACPHYC_TX_SEL_MASK | MACPHYC_TX_DELAY_MASK | MACPHYC_RX_SEL_MASK |
+			MACPHYC_RX_DELAY_MASK | MACPHYC_SOFT_RST_MASK | MACPHYC_PHY_INFT_MASK,
+
+	.set_mode = x2000_mac_set_mode,
+};
+
+static const struct of_device_id ingenic_mac_of_matches[] = {
+	{ .compatible = "ingenic,jz4775-mac", .data = &jz4775_soc_info },
+	{ .compatible = "ingenic,x1000-mac", .data = &x1000_soc_info },
+	{ .compatible = "ingenic,x1600-mac", .data = &x1600_soc_info },
+	{ .compatible = "ingenic,x1830-mac", .data = &x1830_soc_info },
+	{ .compatible = "ingenic,x2000-mac", .data = &x2000_soc_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ingenic_mac_of_matches);
+
+static struct platform_driver ingenic_mac_driver = {
+	.probe		= ingenic_mac_probe,
+	.remove		= stmmac_pltfr_remove,
+	.driver		= {
+		.name	= "ingenic-mac",
+		.pm		= &ingenic_mac_pm_ops,
+		.of_match_table = ingenic_mac_of_matches,
+	},
+};
+module_platform_driver(ingenic_mac_driver);
+
+MODULE_AUTHOR("周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>");
+MODULE_DESCRIPTION("Ingenic SoCs DWMAC specific glue layer");
+MODULE_LICENSE("GPL v2");