diff mbox series

[RFC,5/6] ethernet: dfl-eth-group: add DFL eth group private feature driver

Message ID 1603442745-13085-6-git-send-email-yilun.xu@intel.com
State New
Headers show
Series Add the netdev support for Intel PAC N3000 FPGA | expand

Commit Message

Xu Yilun Oct. 23, 2020, 8:45 a.m. UTC
This driver supports the DFL Ether Group private feature for the Intel(R)
PAC N3000 FPGA Smart NIC.

The DFL Ether Group private feature contains an Ether Wrapper and several
ports (phy-mac pair). There are two types of Ether Groups, line side &
host side. These 2 types of Ether Groups, together with FPGA internal
logic, act as several independent pipes between the host Ethernet
Controller and the front-panel cages. The FPGA logic between the 2 Ether
Groups implements user defined mac layer offloading.

The line side ether interfaces connect to the Parkvale serdes transceiver
chips, which are working in 4 ports 10G/25G retimer mode.

There are several configurations (8x10G, 2x1x25G, 2x2x25G ...) for
different connections between line side and host side. Not all links are
active in some configurations.

For the line side link, the driver connects to the Parkvale phy device
and then register net device for each active link.

For the host side link, its link state is always on. The host side link
always has same features as host side ether controller, so there is no
need to register a netdev for it. The driver just enables the link on
probe.

This patch supports the 25G configurations. Support for 10G will be in
other patches.

Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
 .../ABI/testing/sysfs-class-net-dfl-eth-group      |  19 +
 drivers/net/ethernet/intel/Kconfig                 |  18 +
 drivers/net/ethernet/intel/Makefile                |   2 +
 drivers/net/ethernet/intel/dfl-eth-group-25g.c     | 525 +++++++++++++++++
 drivers/net/ethernet/intel/dfl-eth-group-main.c    | 632 +++++++++++++++++++++
 drivers/net/ethernet/intel/dfl-eth-group.h         |  83 +++
 drivers/net/ethernet/intel/intel-m10-bmc-retimer.c |   4 +-
 7 files changed, 1282 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-net-dfl-eth-group
 create mode 100644 drivers/net/ethernet/intel/dfl-eth-group-25g.c
 create mode 100644 drivers/net/ethernet/intel/dfl-eth-group-main.c
 create mode 100644 drivers/net/ethernet/intel/dfl-eth-group.h

Comments

Andrew Lunn Oct. 24, 2020, 2:37 p.m. UTC | #1
> +++ b/Documentation/ABI/testing/sysfs-class-net-dfl-eth-group
> @@ -0,0 +1,19 @@
> +What:		/sys/class/net/<iface>/tx_pause_frame_quanta
> +Date:		Oct 2020
> +KernelVersion:	5.11
> +Contact:	Xu Yilun <yilun.xu@intel.com>
> +Description:
> +		Read-Write. Value representing the tx pause quanta of Pause
> +		flow control frames to be sent to remote partner. Read the file
> +		for the actual tx pause quanta value. Write the file to set
> +		value of the tx pause quanta.
> +

Is this really configuring the quanta? My very limited understanding
is that the quanta is defined as 512 bit times. For this to work, you
are going to have to modify the quanta on both ends of the link,
otherwise increasing the quanta is actually going to shorten the pause
time.

> +What:		/sys/class/net/<iface>/tx_pause_frame_holdoff
> +Date:		Oct 2020
> +KernelVersion:	5.11
> +Contact:	Xu Yilun <yilun.xu@intel.com>
> +Description:
> +		Read-Write. Value representing the separation between 2
> +		consecutive XOFF flow control frames. Read the file for the
> +		actual separation value. Write the file to set value of the
> +		separation.

This is the wrong API for this. Please extend the ethtool -A|--pause
API. Now that it is netlink, adding extra attributes should be simple.

	Andrew
Tom Rix Oct. 24, 2020, 5:25 p.m. UTC | #2
On 10/23/20 1:45 AM, Xu Yilun wrote:
> This driver supports the DFL Ether Group private feature for the Intel(R)
> PAC N3000 FPGA Smart NIC.
>
> The DFL Ether Group private feature contains an Ether Wrapper and several
> ports (phy-mac pair). There are two types of Ether Groups, line side &

(phy-mac pairs)

line side and

> host side. These 2 types of Ether Groups, together with FPGA internal
> logic, act as several independent pipes between the host Ethernet
> Controller and the front-panel cages. The FPGA logic between the 2 Ether
> Groups implements user defined mac layer offloading.
>
> The line side ether interfaces connect to the Parkvale serdes transceiver
> chips, which are working in 4 ports 10G/25G retimer mode.
>
> There are several configurations (8x10G, 2x1x25G, 2x2x25G ...) for
> different connections between line side and host side. Not all links are
> active in some configurations.
>
> For the line side link, the driver connects to the Parkvale phy device
> and then register net device for each active link.
and then registers a
>
> For the host side link, its link state is always on. The host side link
, the link
> always has same features as host side ether controller, so there is no
has the same
> need to register a netdev for it. The driver just enables the link on
> probe.
>
> This patch supports the 25G configurations. Support for 10G will be in
> other patches.
>
> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> ---
>  .../ABI/testing/sysfs-class-net-dfl-eth-group      |  19 +
>  drivers/net/ethernet/intel/Kconfig                 |  18 +
>  drivers/net/ethernet/intel/Makefile                |   2 +
>  drivers/net/ethernet/intel/dfl-eth-group-25g.c     | 525 +++++++++++++++++
>  drivers/net/ethernet/intel/dfl-eth-group-main.c    | 632 +++++++++++++++++++++

Is it necessary to have a separate dfl-eth-group-25g.c file ?

From below, it looks like adding a 25G prefix or suffix to the register #defines

would make them unique wrt the upcoming 10G.

since this patch is doing two big things, I wonder if the n3000_netdev part should be split out and added after 10G patch.

>  drivers/net/ethernet/intel/dfl-eth-group.h         |  83 +++
>  drivers/net/ethernet/intel/intel-m10-bmc-retimer.c |   4 +-
>  7 files changed, 1282 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-net-dfl-eth-group
>  create mode 100644 drivers/net/ethernet/intel/dfl-eth-group-25g.c
>  create mode 100644 drivers/net/ethernet/intel/dfl-eth-group-main.c
>  create mode 100644 drivers/net/ethernet/intel/dfl-eth-group.h
>
> diff --git a/Documentation/ABI/testing/sysfs-class-net-dfl-eth-group b/Documentation/ABI/testing/sysfs-class-net-dfl-eth-group
> new file mode 100644
> index 0000000..ad528f2
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-net-dfl-eth-group
> @@ -0,0 +1,19 @@
> +What:		/sys/class/net/<iface>/tx_pause_frame_quanta
> +Date:		Oct 2020
> +KernelVersion:	5.11
> +Contact:	Xu Yilun <yilun.xu@intel.com>
> +Description:
> +		Read-Write. Value representing the tx pause quanta of Pause
> +		flow control frames to be sent to remote partner. Read the file
> +		for the actual tx pause quanta value. Write the file to set
> +		value of the tx pause quanta.

units ?

What are the valid range of values ?

Similar below.

> +
> +What:		/sys/class/net/<iface>/tx_pause_frame_holdoff
> +Date:		Oct 2020
> +KernelVersion:	5.11
> +Contact:	Xu Yilun <yilun.xu@intel.com>
> +Description:
> +		Read-Write. Value representing the separation between 2
> +		consecutive XOFF flow control frames. Read the file for the
> +		actual separation value. Write the file to set value of the
> +		separation.
> diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig
> index 81c43d4..61b5d91 100644
> --- a/drivers/net/ethernet/intel/Kconfig
> +++ b/drivers/net/ethernet/intel/Kconfig
> @@ -343,6 +343,24 @@ config IGC
>  	  To compile this driver as a module, choose M here. The module
>  	  will be called igc.
>  
> +config FPGA_DFL_ETH_GROUP
> +        tristate "DFL Ether Group private feature support"
> +        depends on FPGA_DFL && HAS_IOMEM
> +        help
> +          This driver supports DFL Ether Group private feature for
The last 4 lines have white space issues, spaces should be replaced by tabs
> +	  Intel(R) PAC N3000 FPGA Smart NIC.
> +
> +	  The DFL Ether Group private feature contains several ether
> +	  interfaces, each of them contains an Ether Wrapper and several
> +	  ports (phy-mac pairs). There are two types of interfaces, Line
> +	  side & CPU side. These 2 types of interfaces, together with FPGA
Side and Host Side (be consistent with Documentation in patch 1)
> +	  internal logic, act as several independent pipes between the
> +	  host Ethernet Controller and the front-panel cages. The FPGA
> +	  logic between 2 interfaces implements user defined mac layer
between these interfaces implements the user
> +	  offloading.
> +
> +	  This driver implements each active port as a netdev.
> +
>  config INTEL_M10_BMC_RETIMER
>  	tristate "Intel(R) MAX 10 BMC ethernet retimer support"
>  	depends on MFD_INTEL_M10_BMC
> diff --git a/drivers/net/ethernet/intel/Makefile b/drivers/net/ethernet/intel/Makefile
> index 5965447..1624c26 100644
> --- a/drivers/net/ethernet/intel/Makefile
> +++ b/drivers/net/ethernet/intel/Makefile
> @@ -17,4 +17,6 @@ obj-$(CONFIG_IAVF) += iavf/
>  obj-$(CONFIG_FM10K) += fm10k/
>  obj-$(CONFIG_ICE) += ice/
>  
> +dfl-eth-group-objs := dfl-eth-group-main.o dfl-eth-group-25g.o
> +obj-$(CONFIG_FPGA_DFL_ETH_GROUP) += dfl-eth-group.o
>  obj-$(CONFIG_INTEL_M10_BMC_RETIMER) += intel-m10-bmc-retimer.o
> diff --git a/drivers/net/ethernet/intel/dfl-eth-group-25g.c b/drivers/net/ethernet/intel/dfl-eth-group-25g.c
> new file mode 100644
> index 0000000..a690364
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/dfl-eth-group-25g.c
> @@ -0,0 +1,525 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Driver for 25G Ether Group private feature on Intel PAC (Programmable
> + * Acceleration Card) N3000
> + *
> + * Copyright (C) 2019-2020 Intel Corporation, Inc.
> + *
> + * Authors:
> + *   Wu Hao <hao.wu@intel.com>
> + *   Xu Yilun <yilun.xu@intel.com>
> + */
> +#include <linux/netdevice.h>
> +
> +#include "dfl-eth-group.h"
> +
> +/* 25G PHY/MAC Register */
> +#define PHY_CONFIG	0x310
> +#define PHY_MAC_RESET_MASK	GENMASK(2, 0)
> +#define PHY_PMA_SLOOP		0x313
> +#define MAX_TX_SIZE_CONFIG	0x407
> +#define MAX_RX_SIZE_CONFIG	0x506
> +#define TX_FLOW_CTRL_EN		0x605
> +#define TX_FLOW_CTRL_EN_PAUSE	BIT(0)
> +#define TX_FLOW_CTRL_QUANTA	0x620
> +#define TX_FLOW_CTRL_HOLDOFF	0x628
> +#define TX_FLOW_CTRL_SEL	0x640
> +#define TX_FLOW_CTRL_SEL_PAUSE	0x0
> +#define TX_FLOW_CTRL_SEL_PFC	0x1

Generally #defines should have a unique-ish prefix to avoid collisions.

Maybe

DFL_ETH_GROUP_25G_

> +
> +static int edev25g40g_reset(struct eth_dev *edev, bool en)

The name of this function should have prefix similar to its filename, maybe

dfl_eth_group_25g_

similar other places

> +{
> +	struct eth_com *mac = edev->mac;
> +	struct device *dev = edev->dev;
> +	u32 val;
> +	int ret;
> +
> +	ret = eth_com_read_reg(mac, PHY_CONFIG, &val);
> +	if (ret) {
> +		dev_err(dev, "fail to read PHY_CONFIG: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* skip if config is in expected state already */
> +	if ((((val & PHY_MAC_RESET_MASK) == PHY_MAC_RESET_MASK) && en) ||
> +	    (((val & PHY_MAC_RESET_MASK) == 0) && !en))
> +		return 0;
> +
> +	if (en)
> +		val |= PHY_MAC_RESET_MASK;
> +	else
> +		val &= ~PHY_MAC_RESET_MASK;
> +
> +	ret = eth_com_write_reg(mac, PHY_CONFIG, val);
> +	if (ret)
> +		dev_err(dev, "fail to write PHY_CONFIG: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static ssize_t tx_pause_frame_quanta_show(struct device *d,
> +					  struct device_attribute *attr,
> +					  char *buf)
> +{
> +	struct eth_dev *edev = net_device_to_eth_dev(to_net_dev(d));
> +	u32 data;
> +	int ret;
> +
> +	ret = eth_com_read_reg(edev->mac, TX_FLOW_CTRL_QUANTA, &data);

if (!ret)

  ret = sysfs_emit();

return ret;

similar other places

> +
> +	return ret ? : sprintf(buf, "0x%x\n", data);
> +}
> +
> +static ssize_t tx_pause_frame_quanta_store(struct device *d,
> +					   struct device_attribute *attr,
> +					   const char *buf, size_t len)
> +{
> +	struct net_device *netdev = to_net_dev(d);
> +	struct eth_dev *edev;
> +	u32 data;
> +	int ret;
> +
> +	if (kstrtou32(buf, 0, &data))
> +		return -EINVAL;
> +
> +	edev = net_device_to_eth_dev(netdev);
> +
> +	rtnl_lock();
> +
> +	if (netif_running(netdev)) {
> +		netdev_err(netdev, "must be stopped to change pause param\n");
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	ret = eth_com_write_reg(edev->mac, TX_FLOW_CTRL_QUANTA, data);
> +
> +out:
> +	rtnl_unlock();
> +
> +	return ret ? : len;
> +}
> +static DEVICE_ATTR_RW(tx_pause_frame_quanta);
> +
> +static ssize_t tx_pause_frame_holdoff_show(struct device *d,
> +					   struct device_attribute *attr,
> +					   char *buf)
> +{
> +	struct eth_dev *edev = net_device_to_eth_dev(to_net_dev(d));
> +	u32 data;
> +	int ret;
> +
> +	ret = eth_com_read_reg(edev->mac, TX_FLOW_CTRL_HOLDOFF, &data);
> +
> +	return ret ? : sprintf(buf, "0x%x\n", data);
> +}
> +
> +static ssize_t tx_pause_frame_holdoff_store(struct device *d,
> +					    struct device_attribute *attr,
> +					    const char *buf, size_t len)
> +{
> +	struct net_device *netdev = to_net_dev(d);
> +	struct eth_dev *edev;
> +	u32 data;
> +	int ret;
> +
> +	if (kstrtou32(buf, 0, &data))
> +		return -EINVAL;
> +
> +	edev = net_device_to_eth_dev(netdev);
> +
> +	rtnl_lock();
> +
> +	if (netif_running(netdev)) {
> +		netdev_err(netdev, "must be stopped to change pause param\n");
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	ret = eth_com_write_reg(edev->mac, TX_FLOW_CTRL_HOLDOFF, data);
> +
> +out:
> +	rtnl_unlock();
> +
> +	return ret ? : len;
> +}
> +static DEVICE_ATTR_RW(tx_pause_frame_holdoff);
> +
> +static struct attribute *edev25g_dev_attrs[] = {
> +	&dev_attr_tx_pause_frame_quanta.attr,
> +	&dev_attr_tx_pause_frame_holdoff.attr,
> +	NULL
> +};
> +
> +/* device attributes */
> +static const struct attribute_group edev25g_attr_group = {
> +	.attrs = edev25g_dev_attrs,
> +};
> +
> +/* ethtool ops */
> +static struct stat_info stats_25g[] = {
> +	/* TX Statistics */
> +	{STAT_INFO(0x800, "tx_fragments")},

magic numbers should be #defines

why is STAT_INFO needed ? seem like unneeded layer of abstraction

> +	{STAT_INFO(0x802, "tx_jabbers")},
> +	{STAT_INFO(0x804, "tx_fcs")},
> +	{STAT_INFO(0x806, "tx_crc_err")},
> +	{STAT_INFO(0x808, "tx_mcast_data_err")},
> +	{STAT_INFO(0x80a, "tx_bcast_data_err")},
> +	{STAT_INFO(0x80c, "tx_ucast_data_err")},
> +	{STAT_INFO(0x80e, "tx_mcast_ctrl_err")},
> +	{STAT_INFO(0x810, "tx_bcast_ctrl_err")},
> +	{STAT_INFO(0x812, "tx_ucast_ctrl_err")},
> +	{STAT_INFO(0x814, "tx_pause_err")},
> +	{STAT_INFO(0x816, "tx_64_byte")},
> +	{STAT_INFO(0x818, "tx_65_127_byte")},
> +	{STAT_INFO(0x81a, "tx_128_255_byte")},
> +	{STAT_INFO(0x81c, "tx_256_511_byte")},
> +	{STAT_INFO(0x81e, "tx_512_1023_byte")},
> +	{STAT_INFO(0x820, "tx_1024_1518_byte")},
> +	{STAT_INFO(0x822, "tx_1519_max_byte")},
> +	{STAT_INFO(0x824, "tx_oversize")},
> +	{STAT_INFO(0x826, "tx_mcast_data_ok")},
> +	{STAT_INFO(0x828, "tx_bcast_data_ok")},
> +	{STAT_INFO(0x82a, "tx_ucast_data_ok")},
> +	{STAT_INFO(0x82c, "tx_mcast_ctrl_ok")},
> +	{STAT_INFO(0x82e, "tx_bcast_ctrl_ok")},
> +	{STAT_INFO(0x830, "tx_ucast_ctrl_ok")},
> +	{STAT_INFO(0x832, "tx_pause")},
> +	{STAT_INFO(0x834, "tx_runt")},
gap
> +	{STAT_INFO(0x860, "tx_payload_octets_ok")},
> +	{STAT_INFO(0x862, "tx_frame_octets_ok")},
> +
> +	/* RX Statistics */
> +	{STAT_INFO(0x900, "rx_fragments")},
> +	{STAT_INFO(0x902, "rx_jabbers")},
> +	{STAT_INFO(0x904, "rx_fcs")},
> +	{STAT_INFO(0x906, "rx_crc_err")},
> +	{STAT_INFO(0x908, "rx_mcast_data_err")},
> +	{STAT_INFO(0x90a, "rx_bcast_data_err")},
> +	{STAT_INFO(0x90c, "rx_ucast_data_err")},
> +	{STAT_INFO(0x90e, "rx_mcast_ctrl_err")},
> +	{STAT_INFO(0x910, "rx_bcast_ctrl_err")},
> +	{STAT_INFO(0x912, "rx_ucast_ctrl_err")},
> +	{STAT_INFO(0x914, "rx_pause_err")},
> +	{STAT_INFO(0x916, "rx_64_byte")},
> +	{STAT_INFO(0x918, "rx_65_127_byte")},
> +	{STAT_INFO(0x91a, "rx_128_255_byte")},
> +	{STAT_INFO(0x91c, "rx_256_511_byte")},
> +	{STAT_INFO(0x91e, "rx_512_1023_byte")},
> +	{STAT_INFO(0x920, "rx_1024_1518_byte")},
> +	{STAT_INFO(0x922, "rx_1519_max_byte")},
> +	{STAT_INFO(0x924, "rx_oversize")},
> +	{STAT_INFO(0x926, "rx_mcast_data_ok")},
> +	{STAT_INFO(0x928, "rx_bcast_data_ok")},
> +	{STAT_INFO(0x92a, "rx_ucast_data_ok")},
> +	{STAT_INFO(0x92c, "rx_mcast_ctrl_ok")},
> +	{STAT_INFO(0x92e, "rx_bcast_ctrl_ok")},
> +	{STAT_INFO(0x930, "rx_ucast_ctrl_ok")},
> +	{STAT_INFO(0x932, "rx_pause")},
> +	{STAT_INFO(0x934, "rx_runt")},
gap
> +	{STAT_INFO(0x960, "rx_payload_octets_ok")},
> +	{STAT_INFO(0x962, "rx_frame_octets_ok")},
> +};
> +
> +static void edev25g_get_strings(struct net_device *netdev, u32 stringset, u8 *s)
> +{
> +	struct eth_dev *edev = net_device_to_eth_dev(netdev);
> +	unsigned int i;
> +
> +	if (stringset != ETH_SS_STATS || edev->lw_mac)
> +		return;
> +
> +	for (i = 0; i < ARRAY_SIZE(stats_25g); i++, s += ETH_GSTRING_LEN)
> +		memcpy(s, stats_25g[i].string, ETH_GSTRING_LEN);
> +}
> +
> +static int edev25g_get_sset_count(struct net_device *netdev, int stringset)
> +{
> +	struct eth_dev *edev = net_device_to_eth_dev(netdev);
> +
> +	if (stringset != ETH_SS_STATS || edev->lw_mac)
> +		return -EOPNOTSUPP;
> +
> +	return (int)ARRAY_SIZE(stats_25g);
> +}
> +
> +static void edev25g_get_stats(struct net_device *netdev,
> +			      struct ethtool_stats *stats, u64 *data)
> +{
> +	struct eth_dev *edev = net_device_to_eth_dev(netdev);
> +	unsigned int i;
> +
> +	if (edev->lw_mac || !netif_running(netdev))
> +		return;
> +
> +	for (i = 0; i < ARRAY_SIZE(stats_25g); i++)
> +		data[i] = read_mac_stats(edev->mac, stats_25g[i].addr);
> +}
> +
> +static int edev25g_get_link_ksettings(struct net_device *netdev,
> +				      struct ethtool_link_ksettings *cmd)
> +{
> +	if (!netdev->phydev)
> +		return -ENODEV;
> +
> +	phy_ethtool_ksettings_get(netdev->phydev, cmd);
> +
> +	return 0;
> +}
> +
> +static int edev25g_pause_init(struct net_device *netdev)
> +{
> +	struct eth_dev *edev = net_device_to_eth_dev(netdev);
> +
> +	return eth_com_write_reg(edev->mac, TX_FLOW_CTRL_SEL,
> +				 TX_FLOW_CTRL_SEL_PAUSE);
> +}
> +
> +static void edev25g_get_pauseparam(struct net_device *netdev,

> +				   struct ethtool_pauseparam *pause)
> +{
> +	struct eth_dev *edev = net_device_to_eth_dev(netdev);
> +	u32 data;
> +	int ret;
> +
> +	pause->autoneg = 0;
> +	pause->rx_pause = 0;
> +
> +	ret = eth_com_read_reg(edev->mac, TX_FLOW_CTRL_EN, &data);
> +	if (ret) {
> +		pause->tx_pause = 0;
> +		return;
> +	}
> +
> +	pause->tx_pause = (data & TX_FLOW_CTRL_EN_PAUSE) ? 0x1 : 0;
> +}
> +
> +static int edev25g_set_pauseparam(struct net_device *netdev,
> +				  struct ethtool_pauseparam *pause)
> +{
> +	struct eth_dev *edev = net_device_to_eth_dev(netdev);
> +	bool enable = pause->tx_pause;
> +
> +	if (pause->autoneg || pause->rx_pause)
> +		return -EOPNOTSUPP;
> +
> +	return eth_com_write_reg(edev->mac, TX_FLOW_CTRL_EN,
> +				 enable ? TX_FLOW_CTRL_EN_PAUSE : 0);
> +}
> +
> +static const struct ethtool_ops edev25g_ethtool_ops = {
> +	.get_link = ethtool_op_get_link,
> +	.get_strings = edev25g_get_strings,
> +	.get_sset_count = edev25g_get_sset_count,
> +	.get_ethtool_stats = edev25g_get_stats,
> +	.get_link_ksettings = edev25g_get_link_ksettings,
> +	.get_pauseparam = edev25g_get_pauseparam,
> +	.set_pauseparam = edev25g_set_pauseparam,
> +};
> +
> +/* netdev ops */
> +static int edev25g_netdev_open(struct net_device *netdev)
> +{
> +	struct n3000_net_priv *priv = netdev_priv(netdev);
> +	struct eth_dev *edev = priv->edev;
> +	int ret;
> +
> +	ret = edev25g40g_reset(edev, false);
> +	if (ret)
> +		return ret;
> +
> +	if (netdev->phydev)
> +		phy_start(netdev->phydev);
> +
> +	return 0;
> +}
> +
> +static int edev25g_netdev_stop(struct net_device *netdev)
> +{
> +	struct n3000_net_priv *priv = netdev_priv(netdev);
> +	struct eth_dev *edev = priv->edev;
> +	int ret;
> +
> +	ret = edev25g40g_reset(edev, true);
> +	if (ret)
> +		return ret;
> +
> +	if (netdev->phydev)
> +		phy_stop(netdev->phydev);
> +
> +	return 0;
> +}
> +
> +static int edev25g_mtu_init(struct net_device *netdev)
> +{
> +	struct eth_dev *edev = net_device_to_eth_dev(netdev);
> +	struct eth_com *mac = edev->mac;
> +	u32 tx = 0, rx = 0, mtu;
> +	int ret;
> +
> +	ret = eth_com_read_reg(mac, MAX_TX_SIZE_CONFIG, &tx);
> +	if (ret)
> +		return ret;
> +
> +	ret = eth_com_read_reg(mac, MAX_RX_SIZE_CONFIG, &rx);
> +	if (ret)
> +		return ret;
> +
> +	mtu = min(min(tx, rx), netdev->max_mtu);
> +
> +	ret = eth_com_write_reg(mac, MAX_TX_SIZE_CONFIG, rx);
This looks wrong. Did you mean to use 'mtu' ?
> +	if (ret)
> +		return ret;
> +
> +	ret = eth_com_write_reg(mac, MAX_RX_SIZE_CONFIG, tx);
> +	if (ret)
> +		return ret;
> +
> +	netdev->mtu = mtu;
> +
> +	return 0;
> +}
> +
> +static int edev25g_change_mtu(struct net_device *netdev, int new_mtu)
> +{
> +	struct eth_dev *edev = net_device_to_eth_dev(netdev);
> +	struct eth_com *mac = edev->mac;
> +	int ret;
> +
> +	ret = eth_com_write_reg(mac, MAX_TX_SIZE_CONFIG, new_mtu);
> +	if (ret)
> +		return ret;
> +
> +	ret = eth_com_write_reg(mac, MAX_RX_SIZE_CONFIG, new_mtu);
> +	if (ret)
Should the old mtu be restored to tx here ?
> +		return ret;
> +
> +	netdev->mtu = new_mtu;
> +
> +	return 0;
> +}
> +
> +static int edev25g_set_loopback(struct net_device *netdev, bool en)
> +{
> +	struct eth_dev *edev = net_device_to_eth_dev(netdev);
> +
> +	return eth_com_write_reg(edev->mac, PHY_PMA_SLOOP, en);
> +}
> +
> +static int edev25g_set_features(struct net_device *netdev,
> +				netdev_features_t features)
> +{
> +	netdev_features_t changed = netdev->features ^ features;
> +
> +	if (changed & NETIF_F_LOOPBACK)
> +		return edev25g_set_loopback(netdev,
> +					    !!(features & NETIF_F_LOOPBACK));
> +
> +	return 0;
> +}
> +
> +static const struct net_device_ops edev25g_netdev_ops = {
> +	.ndo_open = edev25g_netdev_open,
> +	.ndo_stop = edev25g_netdev_stop,
> +	.ndo_change_mtu = edev25g_change_mtu,
> +	.ndo_set_features = edev25g_set_features,
> +	.ndo_start_xmit = n3000_dummy_netdev_xmit,
> +};
> +
> +static void edev25g_adjust_link(struct net_device *netdev)
> +{}
> +
> +static int edev25g_netdev_init(struct net_device *netdev)
> +{
> +	int ret;
> +
> +	ret = edev25g_pause_init(netdev);
> +	if (ret)
> +		return ret;
> +
> +	ret = edev25g_mtu_init(netdev);
> +	if (ret)
> +		return ret;
> +
> +	return edev25g_set_loopback(netdev,
> +				    !!(netdev->features & NETIF_F_LOOPBACK));

The enable param calculated the same a couple of places.

Maybe drop the 'bool en' and move the calculation inside edev25g_set_loopback

> +}
> +
> +static int dfl_eth_dev_25g_init(struct eth_dev *edev)
> +{
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> +	struct device *dev = edev->dev;
> +	struct phy_device *phydev;
> +	struct net_device *netdev;
> +	int ret;
> +
> +	netdev = n3000_netdev_create(edev);
> +	if (!netdev)
> +		return -ENOMEM;
> +
> +	netdev->hw_features |= NETIF_F_LOOPBACK;
> +	netdev->netdev_ops = &edev25g_netdev_ops;
> +	netdev->ethtool_ops = &edev25g_ethtool_ops;
> +
> +	phydev = phy_connect(netdev, edev->phy_id, edev25g_adjust_link,
> +			     PHY_INTERFACE_MODE_NA);
> +	if (IS_ERR(phydev)) {
> +		dev_err(dev, "PHY connection failed\n");
> +		ret = PTR_ERR(phydev);
> +		goto err_free_netdev;
> +	}
> +
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_25000baseCR_Full_BIT, mask);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_25000baseSR_Full_BIT, mask);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, mask);
> +	linkmode_and(phydev->supported, phydev->supported, mask);
> +	linkmode_copy(phydev->advertising, phydev->supported);
> +
> +	phy_attached_info(phydev);
> +
> +	ret = edev25g_netdev_init(netdev);
> +	if (ret) {
> +		dev_err(dev, "fail to init netdev %s\n", netdev->name);
> +		goto err_phy_disconnect;
> +	}
> +
> +	netdev->sysfs_groups[0] = &edev25g_attr_group;
> +
> +	netif_carrier_off(netdev);
> +	ret = register_netdev(netdev);
> +	if (ret) {
> +		dev_err(dev, "fail to register netdev %s\n", netdev->name);
> +		goto err_phy_disconnect;
> +	}
> +
> +	edev->netdev = netdev;
> +
> +	return 0;
> +
> +err_phy_disconnect:
> +	if (netdev->phydev)
> +		phy_disconnect(phydev);
> +err_free_netdev:
> +	free_netdev(netdev);
> +
> +	return ret;
> +}
> +
> +static void dfl_eth_dev_25g_remove(struct eth_dev *edev)
> +{
> +	struct net_device *netdev = edev->netdev;
> +
> +	if (netdev->phydev)
> +		phy_disconnect(netdev->phydev);
> +
> +	unregister_netdev(netdev);
> +}
> +
> +struct eth_dev_ops dfl_eth_dev_25g_ops = {
> +	.lineside_init = dfl_eth_dev_25g_init,
> +	.lineside_remove = dfl_eth_dev_25g_remove,
> +	.reset = edev25g40g_reset,
> +};
> +
> +struct eth_dev_ops dfl_eth_dev_40g_ops = {
Change only references 25g. The 40g parts should be split.
> +	.reset = edev25g40g_reset,
> +};
> diff --git a/drivers/net/ethernet/intel/dfl-eth-group-main.c b/drivers/net/ethernet/intel/dfl-eth-group-main.c
> new file mode 100644
> index 0000000..a29b8b1
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/dfl-eth-group-main.c
> @@ -0,0 +1,632 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* DFL device driver for Ether Group private feature on Intel PAC (Programmable
> + * Acceleration Card) N3000
> + *
> + * Copyright (C) 2019-2020 Intel Corporation, Inc.
> + *
> + * Authors:
> + *   Wu Hao <hao.wu@intel.com>
> + *   Xu Yilun <yilun.xu@intel.com>
> + */
> +#include <linux/bitfield.h>
> +#include <linux/dfl.h>
> +#include <linux/errno.h>
> +#include <linux/ethtool.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/netdevice.h>
> +#include <linux/rtnetlink.h>
> +#include <linux/stddef.h>
> +#include <linux/types.h>
> +
> +#include "dfl-eth-group.h"
> +
> +struct dfl_eth_group {
> +	char name[32];
> +	struct device *dev;
> +	void __iomem *base;
> +	/* lock to protect register access of the ether group */
> +	struct mutex reg_lock;
> +	struct dfl_device *dfl_dev;
> +	unsigned int config;
> +	unsigned int direction;
> +	unsigned int group_id;
> +	unsigned int speed;
> +	unsigned int lw_mac;
> +	unsigned int num_edevs;
> +	struct eth_dev *edevs;
> +	struct eth_dev_ops *ops;
> +};
> +
> +u64 read_mac_stats(struct eth_com *ecom, unsigned int addr)
> +{
> +	u32 data_l, data_h;
> +
> +	if (eth_com_read_reg(ecom, addr, &data_l) ||
> +	    eth_com_read_reg(ecom, addr + 1, &data_h))
> +		return 0xffffffffffffffffULL;
return -1; ?
> +
> +	return data_l + ((u64)data_h << 32);
> +}
> +
> +netdev_tx_t n3000_dummy_netdev_xmit(struct sk_buff *skb,
> +				    struct net_device *dev)
> +{
> +	kfree_skb(skb);
> +	net_warn_ratelimited("%s(): Dropping skb.\n", __func__);
> +	return NETDEV_TX_OK;
> +}
> +
> +static void n3000_netdev_setup(struct net_device *netdev)
> +{
> +	netdev->features = 0;
> +	netdev->hard_header_len = 0;
> +	netdev->priv_flags |= IFF_NO_QUEUE;
> +	netdev->needs_free_netdev = true;
> +	netdev->min_mtu = 0;
> +	netdev->max_mtu = ETH_MAX_MTU;
> +}
> +
> +struct net_device *n3000_netdev_create(struct eth_dev *edev)
> +{
> +	struct dfl_eth_group *egroup = edev->egroup;
> +	struct n3000_net_priv *priv;
> +	struct net_device *netdev;
> +	char name[IFNAMSIZ];
> +
> +	/* The name of n3000 network device is using this format "npacAgBlC"
> +	 *
> +	 * A is the unique ethdev index
> +	 * B is the group id of this ETH Group.
> +	 * C is the PHY/MAC link index for Line side ethernet group.
> +	 */
> +	snprintf(name, IFNAMSIZ, "npac%%dg%ul%u",
> +		 egroup->group_id, edev->index);
> +
> +	netdev = alloc_netdev(sizeof(*priv), name, NET_NAME_UNKNOWN,
> +			      n3000_netdev_setup);
> +	if (!netdev)
> +		return NULL;
> +
> +	priv = netdev_priv(netdev);
> +	priv->edev = edev;
> +	SET_NETDEV_DEV(netdev, egroup->dev);
> +
> +	return netdev;
> +}
> +
> +enum n3000_eth_cfg {
> +	ETH_CONFIG_8x10G,
Since these are important, should explicitly set.
> +	ETH_CONFIG_4x25G,
> +	ETH_CONFIG_2x1x25G,
> +	ETH_CONFIG_4x25G_2x25G,
> +	ETH_CONFIG_2x2x25G,
> +	ETH_CONFIG_MAX
> +};
> +
> +#define N3000_EDEV_MAX 8
> +
> +static int phy_addr_table[ETH_CONFIG_MAX][N3000_EDEV_MAX] = {
> +	/* 8x10G configuration
> +	 *
> +	 *    [retimer_dev]   <------->   [eth_dev]
> +	 *	  0			   0
> +	 *	  1			   1
> +	 *	  2			   2
> +	 *	  3			   3
> +	 *	  4			   4
> +	 *	  5			   5
> +	 *	  6			   6
> +	 *	  7			   7
> +	 */
> +	[ETH_CONFIG_8x10G] = {0, 1, 2, 3, 4, 5, 6, 7},
> +
> +	/* 4x25G and 4x25G_2x25G configuration
> +	 *
> +	 *    [retimer_dev]   <------->   [eth_dev]
> +	 *	  0			   0
> +	 *	  1			   1
> +	 *	  2			   2
> +	 *	  3			   3
> +	 *	  4
> +	 *	  5
> +	 *	  6
> +	 *	  7
> +	 */
> +	[ETH_CONFIG_4x25G] = {0, 1, 2, 3, -1, -1, -1, -1},
> +	[ETH_CONFIG_4x25G_2x25G] = {0, 1, 2, 3, -1, -1, -1, -1},
> +
> +	/* 2x1x25G configuration
> +	 *
> +	 *    [retimer_dev]   <------->   [eth_dev]
> +	 *        0                      0
> +	 *        1
> +	 *        2
> +	 *        3
> +	 *        4                      1
> +	 *        5
> +	 *        6
> +	 *        7
> +	 */
> +	[ETH_CONFIG_2x1x25G] = {0, 4, -1, -1, -1, -1, -1, -1},
> +
> +	/* 2x2x25G configuration
> +	 *
> +	 *    [retimer_dev]   <------->   [eth_dev]
> +	 *	  0			   0
> +	 *	  1			   1
> +	 *	  2
> +	 *	  3
> +	 *	  4			   2
> +	 *	  5			   3
> +	 *	  6
> +	 *	  7
> +	 */
> +	[ETH_CONFIG_2x2x25G] = {0, 1, 4, 5, -1, -1, -1, -1},
> +};
> +
> +#define eth_group_for_each_dev(edev, egp) \
> +	for ((edev) = (egp)->edevs; (edev) < (egp)->edevs + (egp)->num_edevs; \
> +	     (edev)++)
> +
> +#define eth_group_reverse_each_dev(edev, egp) \
> +	for ((edev)--; (edev) >= (egp)->edevs; (edev)--)

Continuing to ask about another layer of abstraction,

Are these really needed ?

> +
> +static struct mii_bus *eth_group_get_phy_bus(struct dfl_eth_group *egroup)
> +{
> +	char mii_name[MII_BUS_ID_SIZE];
> +	struct device *base_dev;
> +	struct mii_bus *bus;
> +
> +	base_dev = dfl_dev_get_base_dev(egroup->dfl_dev);
> +	if (!base_dev)
> +		return ERR_PTR(-ENODEV);
> +
> +	snprintf(mii_name, MII_BUS_ID_SIZE, DFL_ETH_MII_ID_FMT,
> +		 dev_name(base_dev));
> +
> +	bus = mdio_find_bus(mii_name);
> +	if (!bus)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	return bus;
> +}
> +
> +static int eth_dev_get_phy_id(struct eth_dev *edev, struct mii_bus *bus)
> +{
> +	struct dfl_eth_group *egroup = edev->egroup;
> +	struct phy_device *phydev;
> +	int phyaddr;
> +
> +	phyaddr = phy_addr_table[egroup->config][edev->index];
> +	if (phyaddr < 0)
> +		return -ENODEV;
> +
> +	phydev = mdiobus_get_phy(bus, phyaddr);
> +	if (!phydev) {
> +		dev_err(egroup->dev, "fail to get phydev\n");
> +		return -EPROBE_DEFER;
> +	}
> +
> +	strncpy(edev->phy_id, phydev_name(phydev), MII_BUS_ID_SIZE + 3);

+ 2

Since with + 3, the next statement will overwrite.

> +	edev->phy_id[MII_BUS_ID_SIZE + 2] = '\0';
> +
> +	return 0;
> +}
> +
> +static int init_lineside_eth_devs(struct dfl_eth_group *egroup,
> +				  struct mii_bus *phy_bus)
> +{
> +	struct eth_dev *edev;
> +	int ret = 0;
> +
> +	if (!egroup->ops->lineside_init)
> +		return -ENODEV;
> +
> +	eth_group_for_each_dev(edev, egroup) {
> +		ret = eth_dev_get_phy_id(edev, phy_bus);
> +		if (ret)
> +			break;
> +
> +		ret = egroup->ops->lineside_init(edev);
> +		if (ret)
> +			break;

maybe change to

goto err;

and skip the if-check.

> +	}
> +
> +	if (!ret)
> +		return 0;
> +

return 0;


err:
> +	dev_err(egroup->dev, "failed to init lineside edev %d", edev->index);
> +
> +	if (egroup->ops->lineside_remove)
> +		eth_group_reverse_each_dev(edev, egroup)
> +			egroup->ops->lineside_remove(edev);
> +
> +	return ret;
> +}
> +
> +static void remove_lineside_eth_devs(struct dfl_eth_group *egroup)
> +{
> +	struct eth_dev *edev;
> +
> +	if (!egroup->ops->lineside_remove)
> +		return;
> +
> +	eth_group_for_each_dev(edev, egroup)
> +		egroup->ops->lineside_remove(edev);
> +}
> +
> +#define ETH_GROUP_INFO		0x8
> +#define LIGHT_WEIGHT_MAC	BIT_ULL(25)
> +#define INFO_DIRECTION		BIT_ULL(24)
> +#define INFO_SPEED		GENMASK_ULL(23, 16)
> +#define INFO_PHY_NUM		GENMASK_ULL(15, 8)
> +#define INFO_GROUP_ID		GENMASK_ULL(7, 0)
> +
> +#define ETH_GROUP_CTRL		0x10
> +#define CTRL_CMD		GENMASK_ULL(63, 62)
> +#define CMD_NOP			0
> +#define CMD_RD			1
> +#define CMD_WR			2
> +#define CTRL_DEV_SELECT		GENMASK_ULL(53, 49)
> +#define CTRL_FEAT_SELECT	BIT_ULL(48)
> +#define SELECT_IP		0
> +#define SELECT_FEAT		1
> +#define CTRL_ADDR		GENMASK_ULL(47, 32)
> +#define CTRL_WR_DATA		GENMASK_ULL(31, 0)
> +
> +#define ETH_GROUP_STAT		0x18
> +#define STAT_RW_VAL		BIT_ULL(32)
> +#define STAT_RD_DATA		GENMASK_ULL(31, 0)
> +
> +enum ecom_type {
> +	ETH_GROUP_PHY	= 1,
> +	ETH_GROUP_MAC,
> +	ETH_GROUP_ETHER
> +};
> +
> +struct eth_com {
> +	struct dfl_eth_group *egroup;
> +	unsigned int type;
> +	u8 select;
> +};

Should have better prefixes and be closer to the top of the files.

similar problem below.

> +
> +static const char *eth_com_type_string(enum ecom_type type)
> +{
> +	switch (type) {
> +	case ETH_GROUP_PHY:
> +		return "phy";
> +	case ETH_GROUP_MAC:
> +		return "mac";
> +	case ETH_GROUP_ETHER:
> +		return "ethernet wrapper";
> +	default:
> +		return "unknown";
> +	}
> +}
> +
> +#define eth_com_base(com)	((com)->egroup->base)
> +#define eth_com_dev(com)	((com)->egroup->dev)
> +
> +#define RW_VAL_INVL		1 /* us */
> +#define RW_VAL_POLL_TIMEOUT	10 /* us */
> +
> +static int __do_eth_com_write_reg(struct eth_com *ecom, bool add_feature,
> +				  u16 addr, u32 data)
> +{
This is only called do_eth_com_write_reg, so should be just be part of the caller.
> +	void __iomem *base = eth_com_base(ecom);
> +	struct device *dev = eth_com_dev(ecom);
> +	u64 v = 0;
> +
> +	dev_dbg(dev, "%s [%s] select 0x%x add_feat %d addr 0x%x data 0x%x\n",
> +		__func__, eth_com_type_string(ecom->type),
> +		ecom->select, add_feature, addr, data);
> +
> +	/* only PHY has additional feature registers */
> +	if (add_feature && ecom->type != ETH_GROUP_PHY)
> +		return -EINVAL;
> +
> +	v |= FIELD_PREP(CTRL_CMD, CMD_WR);
> +	v |= FIELD_PREP(CTRL_DEV_SELECT, ecom->select);
> +	v |= FIELD_PREP(CTRL_ADDR, addr);
> +	v |= FIELD_PREP(CTRL_WR_DATA, data);
> +	v |= FIELD_PREP(CTRL_FEAT_SELECT, !!add_feature);
> +
> +	writeq(v, base + ETH_GROUP_CTRL);
> +
> +	if (readq_poll_timeout(base + ETH_GROUP_STAT, v, v & STAT_RW_VAL,
> +			       RW_VAL_INVL, RW_VAL_POLL_TIMEOUT))
> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}
> +
> +static int __do_eth_com_read_reg(struct eth_com *ecom, bool add_feature,
> +				 u16 addr, u32 *data)
> +{
> +	void __iomem *base = eth_com_base(ecom);
> +	struct device *dev = eth_com_dev(ecom);
> +	u64 v = 0;
> +
> +	dev_dbg(dev, "%s [%s] select %x add_feat %d addr %x\n",
> +		__func__, eth_com_type_string(ecom->type),
> +		ecom->select, add_feature, addr);
> +
> +	/* only PHY has additional feature registers */
> +	if (add_feature && ecom->type != ETH_GROUP_PHY)
> +		return -EINVAL;
> +
> +	v |= FIELD_PREP(CTRL_CMD, CMD_RD);
> +	v |= FIELD_PREP(CTRL_DEV_SELECT, ecom->select);
> +	v |= FIELD_PREP(CTRL_ADDR, addr);
> +	v |= FIELD_PREP(CTRL_FEAT_SELECT, !!add_feature);
> +
> +	writeq(v, base + ETH_GROUP_CTRL);
> +
> +	if (readq_poll_timeout(base + ETH_GROUP_STAT, v, v & STAT_RW_VAL,
> +			       RW_VAL_INVL, RW_VAL_POLL_TIMEOUT))
> +		return -ETIMEDOUT;
> +
> +	*data = FIELD_GET(STAT_RD_DATA, v);
> +
> +	return 0;
> +}
> +
> +int do_eth_com_write_reg(struct eth_com *ecom, bool add_feature,
> +			 u16 addr, u32 data)
> +{
> +	int ret;
> +
> +	mutex_lock(&ecom->egroup->reg_lock);
> +	ret = __do_eth_com_write_reg(ecom, add_feature, addr, data);
> +	mutex_unlock(&ecom->egroup->reg_lock);
> +	return ret;
> +}
> +
> +int do_eth_com_read_reg(struct eth_com *ecom, bool add_feature,
> +			u16 addr, u32 *data)
> +{
> +	int ret;
> +
> +	mutex_lock(&ecom->egroup->reg_lock);
> +	ret = __do_eth_com_read_reg(ecom, add_feature, addr, data);
> +	mutex_unlock(&ecom->egroup->reg_lock);
> +	return ret;
> +}
> +
> +static struct eth_com *
> +eth_com_create(struct dfl_eth_group *egroup, enum ecom_type type,
> +	       unsigned int link_idx)
> +{
> +	struct eth_com *ecom;
> +
> +	ecom = devm_kzalloc(egroup->dev, sizeof(*ecom), GFP_KERNEL);
> +	if (!ecom)
> +		return ERR_PTR(-ENOMEM);
> +
> +	ecom->egroup = egroup;
> +	ecom->type = type;
> +
> +	if (type == ETH_GROUP_PHY)
> +		ecom->select = link_idx * 2 + 2;
> +	else if (type == ETH_GROUP_MAC)
> +		ecom->select = link_idx * 2 + 3;
> +	else if (type == ETH_GROUP_ETHER)
should change the else-if to else or catch the invalid input earlier.
> +		ecom->select = 0;
> +
> +	return ecom;
> +}
> +
> +static int init_eth_dev(struct eth_dev *edev, struct dfl_eth_group *egroup,
> +			unsigned int link_idx)
> +{
> +	edev->egroup = egroup;
> +	edev->dev = egroup->dev;
> +	edev->index = link_idx;
> +	edev->lw_mac = !!egroup->lw_mac;
> +	edev->phy = eth_com_create(egroup, ETH_GROUP_PHY, link_idx);
> +	if (IS_ERR(edev->phy))
> +		return PTR_ERR(edev->phy);
> +
> +	edev->mac = eth_com_create(egroup, ETH_GROUP_MAC, link_idx);
> +	if (IS_ERR(edev->mac))
kfree(edev->phy)
> +		return PTR_ERR(edev->mac);
> +
> +	return 0;
> +}
> +
> +static int eth_devs_init(struct dfl_eth_group *egroup)
> +{
> +	int ret, i;
> +
> +	egroup->edevs = devm_kcalloc(egroup->dev, egroup->num_edevs,
> +				     sizeof(*egroup->edevs), GFP_KERNEL);
> +	if (!egroup->edevs)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < egroup->num_edevs; i++) {
> +		ret = init_eth_dev(&egroup->edevs[i], egroup, i);
> +		if (ret)
free the early successes
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int eth_group_setup(struct dfl_eth_group *egroup)
> +{
> +	int net_cfg, ret;
> +	u64 v;
> +
> +	/* read parameters of this ethernet components group */
> +	v = readq(egroup->base + ETH_GROUP_INFO);
> +
> +	egroup->direction = FIELD_GET(INFO_DIRECTION, v);
> +	egroup->speed = FIELD_GET(INFO_SPEED, v);
> +	egroup->num_edevs = FIELD_GET(INFO_PHY_NUM, v);
> +	egroup->group_id = FIELD_GET(INFO_GROUP_ID, v);
> +	egroup->lw_mac = FIELD_GET(LIGHT_WEIGHT_MAC, v);
> +
> +	net_cfg = dfl_dev_get_vendor_net_cfg(egroup->dfl_dev);
> +	if (net_cfg < 0)
> +		return -EINVAL;
> +
> +	egroup->config = (unsigned int)net_cfg;
should check bounds or will overflow phy_addr_table
> +
> +	ret = eth_devs_init(egroup);
> +	if (ret)
> +		return ret;
> +
> +	switch (egroup->speed) {
> +	case 25:
> +		egroup->ops = &dfl_eth_dev_25g_ops;
> +		break;
> +	case 40:
> +		egroup->ops = &dfl_eth_dev_40g_ops;
> +		break;

default:

  fail();

> +	}
> +
> +	mutex_init(&egroup->reg_lock);
> +
> +	return 0;
> +}
> +
> +static void eth_group_destroy(struct dfl_eth_group *egroup)
> +{
> +	mutex_destroy(&egroup->reg_lock);
> +}
> +
> +static void eth_group_devs_disable(struct dfl_eth_group *egroup)
> +{
> +	struct eth_dev *edev;
> +
> +	eth_group_for_each_dev(edev, egroup)
> +		egroup->ops->reset(edev, true);
> +}
> +
> +static int eth_group_devs_enable(struct dfl_eth_group *egroup)
> +{
> +	struct eth_dev *edev;
> +	int ret;
> +
> +	eth_group_for_each_dev(edev, egroup) {
> +		ret = egroup->ops->reset(edev, false);
> +		if (ret) {
> +			dev_err(egroup->dev, "fail to enable edev%d\n",
> +				edev->index);
> +			eth_group_devs_disable(egroup);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int dfl_eth_group_line_side_init(struct dfl_eth_group *egroup)
earlier function has 'lineside'  use should be consistent.
> +{
> +	struct mii_bus *phy_bus;
> +	int ret;
> +
> +	if (!egroup->ops || !egroup->ops->reset ||
> +	    !egroup->ops->lineside_init || !egroup->ops->lineside_remove)
> +		return -EINVAL;
> +
> +	eth_group_devs_disable(egroup);
> +
> +	phy_bus = eth_group_get_phy_bus(egroup);
> +	if (IS_ERR(phy_bus))
> +		return PTR_ERR(phy_bus);
> +
> +	ret = init_lineside_eth_devs(egroup, phy_bus);
> +	put_device(&phy_bus->dev);
> +
> +	return ret;
> +}
> +
> +static void dfl_eth_group_line_side_uinit(struct dfl_eth_group *egroup)
> +{
> +	remove_lineside_eth_devs(egroup);
remove_lineside_eth_devs only called here, so move function contents to here.
> +}
> +
> +static int dfl_eth_group_host_side_init(struct dfl_eth_group *egroup)
> +{
> +	if (!egroup->ops || !egroup->ops->reset)
> +		return -EINVAL;
> +
> +	return eth_group_devs_enable(egroup);
> +}
> +
> +static int dfl_eth_group_probe(struct dfl_device *dfl_dev)
> +{
> +	struct device *dev = &dfl_dev->dev;
> +	struct dfl_eth_group *egroup;
> +	int ret;
> +
> +	egroup = devm_kzalloc(dev, sizeof(*egroup), GFP_KERNEL);
> +	if (!egroup)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(&dfl_dev->dev, egroup);
> +
> +	egroup->dev = dev;
> +	egroup->dfl_dev = dfl_dev;
> +
> +	egroup->base = devm_ioremap_resource(dev, &dfl_dev->mmio_res);
> +	if (IS_ERR(egroup->base)) {
> +		dev_err(dev, "get mem resource fail!\n");
> +		return PTR_ERR(egroup->base);
> +	}
> +
> +	ret = eth_group_setup(egroup);
> +	if (ret)
> +		return ret;
> +
> +	if (egroup->direction == 1)
1 is magic, should have #define DFL_ETH_GROUP_DIRECTION_LINE/HOST 1/0 or similar
> +		ret = dfl_eth_group_line_side_init(egroup);
> +	else
> +		ret = dfl_eth_group_host_side_init(egroup);
> +
> +	if (!ret)
> +		return 0;
> +
> +	eth_group_destroy(egroup);
> +
> +	return ret;
> +}
> +
> +static void dfl_eth_group_remove(struct dfl_device *dfl_dev)
> +{
> +	struct dfl_eth_group *egroup = dev_get_drvdata(&dfl_dev->dev);
> +
> +	if (egroup->direction == 1)
> +		dfl_eth_group_line_side_uinit(egroup);
> +
> +	eth_group_devs_disable(egroup);
> +	eth_group_destroy(egroup);
> +}
> +
> +#define FME_FEATURE_ID_ETH_GROUP	0x10
> +
> +static const struct dfl_device_id dfl_eth_group_ids[] = {
> +	{ FME_ID, FME_FEATURE_ID_ETH_GROUP },
> +	{ }
> +};
> +
> +static struct dfl_driver dfl_eth_group_driver = {
> +	.drv	= {
> +		.name       = "dfl-eth-group",
> +	},
> +	.id_table = dfl_eth_group_ids,
> +	.probe   = dfl_eth_group_probe,
> +	.remove  = dfl_eth_group_remove,
> +};
> +
> +module_dfl_driver(dfl_eth_group_driver);
> +
> +MODULE_DEVICE_TABLE(dfl, dfl_eth_group_ids);
> +MODULE_DESCRIPTION("DFL ether group driver");
> +MODULE_AUTHOR("Intel Corporation");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/net/ethernet/intel/dfl-eth-group.h b/drivers/net/ethernet/intel/dfl-eth-group.h
> new file mode 100644
> index 0000000..2e90f86
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/dfl-eth-group.h
> @@ -0,0 +1,83 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/* Internal header file for FPGA DFL Ether Group Driver
> + *
> + * Copyright (C) 2020 Intel Corporation. All rights reserved.
> + */
> +
> +#ifndef __DFL_ETH_GROUP_H__
> +#define __DFL_ETH_GROUP_H__
This file would not be needed if there was single dfl-eth-group.c ?
> +
> +#include <linux/netdevice.h>
> +#include <linux/phy.h>
> +#include <linux/rtnetlink.h>
> +
> +/* Used when trying to find a virtual mii bus on a specific dfl device.
> + * dev_name(dfl base device)-mii
> + */
> +#define DFL_ETH_MII_ID_FMT "%s-mii"
> +
> +struct eth_dev {
> +	struct dfl_eth_group *egroup;
> +	struct device *dev;
> +	int index;
> +	bool lw_mac;
> +	struct eth_com *phy;
> +	struct eth_com *mac;
> +	struct net_device *netdev;
> +
> +	char phy_id[MII_BUS_ID_SIZE + 3];
This +3 may become problematic, maybe use ARRAY_SIZE() when accessing phy_id
> +};
> +
> +struct eth_dev_ops {
> +	int (*lineside_init)(struct eth_dev *edev);
> +	void (*lineside_remove)(struct eth_dev *edev);
> +	int (*reset)(struct eth_dev *edev, bool en);
> +};
> +
> +struct n3000_net_priv {
> +	struct eth_dev *edev;
> +};
> +
> +static inline struct eth_dev *net_device_to_eth_dev(struct net_device *netdev)
> +{
> +	struct n3000_net_priv *priv = netdev_priv(netdev);
> +
> +	return priv->edev;
> +}
> +
> +struct stat_info {
> +	unsigned int addr;
> +	char string[ETH_GSTRING_LEN];
> +};
> +
> +#define STAT_INFO(_addr, _string) \
> +	.addr = _addr, .string = _string,
> +
> +int do_eth_com_write_reg(struct eth_com *ecom, bool add_feature,
> +			 u16 addr, u32 data);
> +int do_eth_com_read_reg(struct eth_com *ecom, bool add_feature,
> +			u16 addr, u32 *data);
> +
> +#define eth_com_write_reg(ecom, addr, data)	\
> +	do_eth_com_write_reg(ecom, false, addr, data)
> +
> +#define eth_com_read_reg(ecom, addr, data)	\
> +	do_eth_com_read_reg(ecom, false, addr, data)
> +
> +#define eth_com_add_feat_write_reg(ecom, addr, data)	\
> +	do_eth_com_write_reg(ecom, true, addr, data)
> +
> +#define eth_com_add_feat_read_reg(ecom, addr, data)	\
> +	do_eth_com_read_reg(ecom, true, addr, data)
> +
> +

This seems like an unneeded layer of abstraction.

And functions' name maybe too verbose.

Tom

> u64 read_mac_stats(struct eth_com *ecom, unsigned int addr);
> +
> +struct net_device *n3000_netdev_create(struct eth_dev *edev);
> +netdev_tx_t n3000_dummy_netdev_xmit(struct sk_buff *skb,
> +				    struct net_device *dev);
> +
> +extern struct eth_dev_ops dfl_eth_dev_25g_ops;
> +extern struct eth_dev_ops dfl_eth_dev_40g_ops;
> +
> +#endif /* __DFL_ETH_GROUP_H__ */
> diff --git a/drivers/net/ethernet/intel/intel-m10-bmc-retimer.c b/drivers/net/ethernet/intel/intel-m10-bmc-retimer.c
> index c7b0558..3f686d2 100644
> --- a/drivers/net/ethernet/intel/intel-m10-bmc-retimer.c
> +++ b/drivers/net/ethernet/intel/intel-m10-bmc-retimer.c
> @@ -10,6 +10,8 @@
>  #include <linux/phy.h>
>  #include <linux/platform_device.h>
>  
> +#include "dfl-eth-group.h"
> +
>  #define NUM_CHIP	2
>  #define MAX_LINK	4
>  
> @@ -148,7 +150,7 @@ static int m10bmc_retimer_mii_bus_init(struct m10bmc_retimer *retimer)
>  	bus->name = M10BMC_RETIMER_MII_NAME;
>  	bus->read = m10bmc_retimer_read;
>  	bus->write = m10bmc_retimer_write;
> -	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii",
> +	snprintf(bus->id, MII_BUS_ID_SIZE, DFL_ETH_MII_ID_FMT,
>  		 dev_name(retimer->base_dev));
>  	bus->parent = retimer->dev;
>  	bus->phy_mask = ~(BITS_MASK(retimer->num_devs));
Andrew Lunn Oct. 25, 2020, 2:47 p.m. UTC | #3
> > +u64 read_mac_stats(struct eth_com *ecom, unsigned int addr)
> > +{
> > +	u32 data_l, data_h;
> > +
> > +	if (eth_com_read_reg(ecom, addr, &data_l) ||
> > +	    eth_com_read_reg(ecom, addr + 1, &data_h))
> > +		return 0xffffffffffffffffULL;

> return -1; ?

Since this is a u64 function, i expect you get a compiler
warning. Maybe only with W=1. It is better to use U64_MAX.

	 Andrew
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-class-net-dfl-eth-group b/Documentation/ABI/testing/sysfs-class-net-dfl-eth-group
new file mode 100644
index 0000000..ad528f2
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-net-dfl-eth-group
@@ -0,0 +1,19 @@ 
+What:		/sys/class/net/<iface>/tx_pause_frame_quanta
+Date:		Oct 2020
+KernelVersion:	5.11
+Contact:	Xu Yilun <yilun.xu@intel.com>
+Description:
+		Read-Write. Value representing the tx pause quanta of Pause
+		flow control frames to be sent to remote partner. Read the file
+		for the actual tx pause quanta value. Write the file to set
+		value of the tx pause quanta.
+
+What:		/sys/class/net/<iface>/tx_pause_frame_holdoff
+Date:		Oct 2020
+KernelVersion:	5.11
+Contact:	Xu Yilun <yilun.xu@intel.com>
+Description:
+		Read-Write. Value representing the separation between 2
+		consecutive XOFF flow control frames. Read the file for the
+		actual separation value. Write the file to set value of the
+		separation.
diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig
index 81c43d4..61b5d91 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -343,6 +343,24 @@  config IGC
 	  To compile this driver as a module, choose M here. The module
 	  will be called igc.
 
+config FPGA_DFL_ETH_GROUP
+        tristate "DFL Ether Group private feature support"
+        depends on FPGA_DFL && HAS_IOMEM
+        help
+          This driver supports DFL Ether Group private feature for
+	  Intel(R) PAC N3000 FPGA Smart NIC.
+
+	  The DFL Ether Group private feature contains several ether
+	  interfaces, each of them contains an Ether Wrapper and several
+	  ports (phy-mac pairs). There are two types of interfaces, Line
+	  side & CPU side. These 2 types of interfaces, together with FPGA
+	  internal logic, act as several independent pipes between the
+	  host Ethernet Controller and the front-panel cages. The FPGA
+	  logic between 2 interfaces implements user defined mac layer
+	  offloading.
+
+	  This driver implements each active port as a netdev.
+
 config INTEL_M10_BMC_RETIMER
 	tristate "Intel(R) MAX 10 BMC ethernet retimer support"
 	depends on MFD_INTEL_M10_BMC
diff --git a/drivers/net/ethernet/intel/Makefile b/drivers/net/ethernet/intel/Makefile
index 5965447..1624c26 100644
--- a/drivers/net/ethernet/intel/Makefile
+++ b/drivers/net/ethernet/intel/Makefile
@@ -17,4 +17,6 @@  obj-$(CONFIG_IAVF) += iavf/
 obj-$(CONFIG_FM10K) += fm10k/
 obj-$(CONFIG_ICE) += ice/
 
+dfl-eth-group-objs := dfl-eth-group-main.o dfl-eth-group-25g.o
+obj-$(CONFIG_FPGA_DFL_ETH_GROUP) += dfl-eth-group.o
 obj-$(CONFIG_INTEL_M10_BMC_RETIMER) += intel-m10-bmc-retimer.o
diff --git a/drivers/net/ethernet/intel/dfl-eth-group-25g.c b/drivers/net/ethernet/intel/dfl-eth-group-25g.c
new file mode 100644
index 0000000..a690364
--- /dev/null
+++ b/drivers/net/ethernet/intel/dfl-eth-group-25g.c
@@ -0,0 +1,525 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* Driver for 25G Ether Group private feature on Intel PAC (Programmable
+ * Acceleration Card) N3000
+ *
+ * Copyright (C) 2019-2020 Intel Corporation, Inc.
+ *
+ * Authors:
+ *   Wu Hao <hao.wu@intel.com>
+ *   Xu Yilun <yilun.xu@intel.com>
+ */
+#include <linux/netdevice.h>
+
+#include "dfl-eth-group.h"
+
+/* 25G PHY/MAC Register */
+#define PHY_CONFIG	0x310
+#define PHY_MAC_RESET_MASK	GENMASK(2, 0)
+#define PHY_PMA_SLOOP		0x313
+#define MAX_TX_SIZE_CONFIG	0x407
+#define MAX_RX_SIZE_CONFIG	0x506
+#define TX_FLOW_CTRL_EN		0x605
+#define TX_FLOW_CTRL_EN_PAUSE	BIT(0)
+#define TX_FLOW_CTRL_QUANTA	0x620
+#define TX_FLOW_CTRL_HOLDOFF	0x628
+#define TX_FLOW_CTRL_SEL	0x640
+#define TX_FLOW_CTRL_SEL_PAUSE	0x0
+#define TX_FLOW_CTRL_SEL_PFC	0x1
+
+static int edev25g40g_reset(struct eth_dev *edev, bool en)
+{
+	struct eth_com *mac = edev->mac;
+	struct device *dev = edev->dev;
+	u32 val;
+	int ret;
+
+	ret = eth_com_read_reg(mac, PHY_CONFIG, &val);
+	if (ret) {
+		dev_err(dev, "fail to read PHY_CONFIG: %d\n", ret);
+		return ret;
+	}
+
+	/* skip if config is in expected state already */
+	if ((((val & PHY_MAC_RESET_MASK) == PHY_MAC_RESET_MASK) && en) ||
+	    (((val & PHY_MAC_RESET_MASK) == 0) && !en))
+		return 0;
+
+	if (en)
+		val |= PHY_MAC_RESET_MASK;
+	else
+		val &= ~PHY_MAC_RESET_MASK;
+
+	ret = eth_com_write_reg(mac, PHY_CONFIG, val);
+	if (ret)
+		dev_err(dev, "fail to write PHY_CONFIG: %d\n", ret);
+
+	return ret;
+}
+
+static ssize_t tx_pause_frame_quanta_show(struct device *d,
+					  struct device_attribute *attr,
+					  char *buf)
+{
+	struct eth_dev *edev = net_device_to_eth_dev(to_net_dev(d));
+	u32 data;
+	int ret;
+
+	ret = eth_com_read_reg(edev->mac, TX_FLOW_CTRL_QUANTA, &data);
+
+	return ret ? : sprintf(buf, "0x%x\n", data);
+}
+
+static ssize_t tx_pause_frame_quanta_store(struct device *d,
+					   struct device_attribute *attr,
+					   const char *buf, size_t len)
+{
+	struct net_device *netdev = to_net_dev(d);
+	struct eth_dev *edev;
+	u32 data;
+	int ret;
+
+	if (kstrtou32(buf, 0, &data))
+		return -EINVAL;
+
+	edev = net_device_to_eth_dev(netdev);
+
+	rtnl_lock();
+
+	if (netif_running(netdev)) {
+		netdev_err(netdev, "must be stopped to change pause param\n");
+		ret = -EBUSY;
+		goto out;
+	}
+
+	ret = eth_com_write_reg(edev->mac, TX_FLOW_CTRL_QUANTA, data);
+
+out:
+	rtnl_unlock();
+
+	return ret ? : len;
+}
+static DEVICE_ATTR_RW(tx_pause_frame_quanta);
+
+static ssize_t tx_pause_frame_holdoff_show(struct device *d,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct eth_dev *edev = net_device_to_eth_dev(to_net_dev(d));
+	u32 data;
+	int ret;
+
+	ret = eth_com_read_reg(edev->mac, TX_FLOW_CTRL_HOLDOFF, &data);
+
+	return ret ? : sprintf(buf, "0x%x\n", data);
+}
+
+static ssize_t tx_pause_frame_holdoff_store(struct device *d,
+					    struct device_attribute *attr,
+					    const char *buf, size_t len)
+{
+	struct net_device *netdev = to_net_dev(d);
+	struct eth_dev *edev;
+	u32 data;
+	int ret;
+
+	if (kstrtou32(buf, 0, &data))
+		return -EINVAL;
+
+	edev = net_device_to_eth_dev(netdev);
+
+	rtnl_lock();
+
+	if (netif_running(netdev)) {
+		netdev_err(netdev, "must be stopped to change pause param\n");
+		ret = -EBUSY;
+		goto out;
+	}
+
+	ret = eth_com_write_reg(edev->mac, TX_FLOW_CTRL_HOLDOFF, data);
+
+out:
+	rtnl_unlock();
+
+	return ret ? : len;
+}
+static DEVICE_ATTR_RW(tx_pause_frame_holdoff);
+
+static struct attribute *edev25g_dev_attrs[] = {
+	&dev_attr_tx_pause_frame_quanta.attr,
+	&dev_attr_tx_pause_frame_holdoff.attr,
+	NULL
+};
+
+/* device attributes */
+static const struct attribute_group edev25g_attr_group = {
+	.attrs = edev25g_dev_attrs,
+};
+
+/* ethtool ops */
+static struct stat_info stats_25g[] = {
+	/* TX Statistics */
+	{STAT_INFO(0x800, "tx_fragments")},
+	{STAT_INFO(0x802, "tx_jabbers")},
+	{STAT_INFO(0x804, "tx_fcs")},
+	{STAT_INFO(0x806, "tx_crc_err")},
+	{STAT_INFO(0x808, "tx_mcast_data_err")},
+	{STAT_INFO(0x80a, "tx_bcast_data_err")},
+	{STAT_INFO(0x80c, "tx_ucast_data_err")},
+	{STAT_INFO(0x80e, "tx_mcast_ctrl_err")},
+	{STAT_INFO(0x810, "tx_bcast_ctrl_err")},
+	{STAT_INFO(0x812, "tx_ucast_ctrl_err")},
+	{STAT_INFO(0x814, "tx_pause_err")},
+	{STAT_INFO(0x816, "tx_64_byte")},
+	{STAT_INFO(0x818, "tx_65_127_byte")},
+	{STAT_INFO(0x81a, "tx_128_255_byte")},
+	{STAT_INFO(0x81c, "tx_256_511_byte")},
+	{STAT_INFO(0x81e, "tx_512_1023_byte")},
+	{STAT_INFO(0x820, "tx_1024_1518_byte")},
+	{STAT_INFO(0x822, "tx_1519_max_byte")},
+	{STAT_INFO(0x824, "tx_oversize")},
+	{STAT_INFO(0x826, "tx_mcast_data_ok")},
+	{STAT_INFO(0x828, "tx_bcast_data_ok")},
+	{STAT_INFO(0x82a, "tx_ucast_data_ok")},
+	{STAT_INFO(0x82c, "tx_mcast_ctrl_ok")},
+	{STAT_INFO(0x82e, "tx_bcast_ctrl_ok")},
+	{STAT_INFO(0x830, "tx_ucast_ctrl_ok")},
+	{STAT_INFO(0x832, "tx_pause")},
+	{STAT_INFO(0x834, "tx_runt")},
+	{STAT_INFO(0x860, "tx_payload_octets_ok")},
+	{STAT_INFO(0x862, "tx_frame_octets_ok")},
+
+	/* RX Statistics */
+	{STAT_INFO(0x900, "rx_fragments")},
+	{STAT_INFO(0x902, "rx_jabbers")},
+	{STAT_INFO(0x904, "rx_fcs")},
+	{STAT_INFO(0x906, "rx_crc_err")},
+	{STAT_INFO(0x908, "rx_mcast_data_err")},
+	{STAT_INFO(0x90a, "rx_bcast_data_err")},
+	{STAT_INFO(0x90c, "rx_ucast_data_err")},
+	{STAT_INFO(0x90e, "rx_mcast_ctrl_err")},
+	{STAT_INFO(0x910, "rx_bcast_ctrl_err")},
+	{STAT_INFO(0x912, "rx_ucast_ctrl_err")},
+	{STAT_INFO(0x914, "rx_pause_err")},
+	{STAT_INFO(0x916, "rx_64_byte")},
+	{STAT_INFO(0x918, "rx_65_127_byte")},
+	{STAT_INFO(0x91a, "rx_128_255_byte")},
+	{STAT_INFO(0x91c, "rx_256_511_byte")},
+	{STAT_INFO(0x91e, "rx_512_1023_byte")},
+	{STAT_INFO(0x920, "rx_1024_1518_byte")},
+	{STAT_INFO(0x922, "rx_1519_max_byte")},
+	{STAT_INFO(0x924, "rx_oversize")},
+	{STAT_INFO(0x926, "rx_mcast_data_ok")},
+	{STAT_INFO(0x928, "rx_bcast_data_ok")},
+	{STAT_INFO(0x92a, "rx_ucast_data_ok")},
+	{STAT_INFO(0x92c, "rx_mcast_ctrl_ok")},
+	{STAT_INFO(0x92e, "rx_bcast_ctrl_ok")},
+	{STAT_INFO(0x930, "rx_ucast_ctrl_ok")},
+	{STAT_INFO(0x932, "rx_pause")},
+	{STAT_INFO(0x934, "rx_runt")},
+	{STAT_INFO(0x960, "rx_payload_octets_ok")},
+	{STAT_INFO(0x962, "rx_frame_octets_ok")},
+};
+
+static void edev25g_get_strings(struct net_device *netdev, u32 stringset, u8 *s)
+{
+	struct eth_dev *edev = net_device_to_eth_dev(netdev);
+	unsigned int i;
+
+	if (stringset != ETH_SS_STATS || edev->lw_mac)
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(stats_25g); i++, s += ETH_GSTRING_LEN)
+		memcpy(s, stats_25g[i].string, ETH_GSTRING_LEN);
+}
+
+static int edev25g_get_sset_count(struct net_device *netdev, int stringset)
+{
+	struct eth_dev *edev = net_device_to_eth_dev(netdev);
+
+	if (stringset != ETH_SS_STATS || edev->lw_mac)
+		return -EOPNOTSUPP;
+
+	return (int)ARRAY_SIZE(stats_25g);
+}
+
+static void edev25g_get_stats(struct net_device *netdev,
+			      struct ethtool_stats *stats, u64 *data)
+{
+	struct eth_dev *edev = net_device_to_eth_dev(netdev);
+	unsigned int i;
+
+	if (edev->lw_mac || !netif_running(netdev))
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(stats_25g); i++)
+		data[i] = read_mac_stats(edev->mac, stats_25g[i].addr);
+}
+
+static int edev25g_get_link_ksettings(struct net_device *netdev,
+				      struct ethtool_link_ksettings *cmd)
+{
+	if (!netdev->phydev)
+		return -ENODEV;
+
+	phy_ethtool_ksettings_get(netdev->phydev, cmd);
+
+	return 0;
+}
+
+static int edev25g_pause_init(struct net_device *netdev)
+{
+	struct eth_dev *edev = net_device_to_eth_dev(netdev);
+
+	return eth_com_write_reg(edev->mac, TX_FLOW_CTRL_SEL,
+				 TX_FLOW_CTRL_SEL_PAUSE);
+}
+
+static void edev25g_get_pauseparam(struct net_device *netdev,
+				   struct ethtool_pauseparam *pause)
+{
+	struct eth_dev *edev = net_device_to_eth_dev(netdev);
+	u32 data;
+	int ret;
+
+	pause->autoneg = 0;
+	pause->rx_pause = 0;
+
+	ret = eth_com_read_reg(edev->mac, TX_FLOW_CTRL_EN, &data);
+	if (ret) {
+		pause->tx_pause = 0;
+		return;
+	}
+
+	pause->tx_pause = (data & TX_FLOW_CTRL_EN_PAUSE) ? 0x1 : 0;
+}
+
+static int edev25g_set_pauseparam(struct net_device *netdev,
+				  struct ethtool_pauseparam *pause)
+{
+	struct eth_dev *edev = net_device_to_eth_dev(netdev);
+	bool enable = pause->tx_pause;
+
+	if (pause->autoneg || pause->rx_pause)
+		return -EOPNOTSUPP;
+
+	return eth_com_write_reg(edev->mac, TX_FLOW_CTRL_EN,
+				 enable ? TX_FLOW_CTRL_EN_PAUSE : 0);
+}
+
+static const struct ethtool_ops edev25g_ethtool_ops = {
+	.get_link = ethtool_op_get_link,
+	.get_strings = edev25g_get_strings,
+	.get_sset_count = edev25g_get_sset_count,
+	.get_ethtool_stats = edev25g_get_stats,
+	.get_link_ksettings = edev25g_get_link_ksettings,
+	.get_pauseparam = edev25g_get_pauseparam,
+	.set_pauseparam = edev25g_set_pauseparam,
+};
+
+/* netdev ops */
+static int edev25g_netdev_open(struct net_device *netdev)
+{
+	struct n3000_net_priv *priv = netdev_priv(netdev);
+	struct eth_dev *edev = priv->edev;
+	int ret;
+
+	ret = edev25g40g_reset(edev, false);
+	if (ret)
+		return ret;
+
+	if (netdev->phydev)
+		phy_start(netdev->phydev);
+
+	return 0;
+}
+
+static int edev25g_netdev_stop(struct net_device *netdev)
+{
+	struct n3000_net_priv *priv = netdev_priv(netdev);
+	struct eth_dev *edev = priv->edev;
+	int ret;
+
+	ret = edev25g40g_reset(edev, true);
+	if (ret)
+		return ret;
+
+	if (netdev->phydev)
+		phy_stop(netdev->phydev);
+
+	return 0;
+}
+
+static int edev25g_mtu_init(struct net_device *netdev)
+{
+	struct eth_dev *edev = net_device_to_eth_dev(netdev);
+	struct eth_com *mac = edev->mac;
+	u32 tx = 0, rx = 0, mtu;
+	int ret;
+
+	ret = eth_com_read_reg(mac, MAX_TX_SIZE_CONFIG, &tx);
+	if (ret)
+		return ret;
+
+	ret = eth_com_read_reg(mac, MAX_RX_SIZE_CONFIG, &rx);
+	if (ret)
+		return ret;
+
+	mtu = min(min(tx, rx), netdev->max_mtu);
+
+	ret = eth_com_write_reg(mac, MAX_TX_SIZE_CONFIG, rx);
+	if (ret)
+		return ret;
+
+	ret = eth_com_write_reg(mac, MAX_RX_SIZE_CONFIG, tx);
+	if (ret)
+		return ret;
+
+	netdev->mtu = mtu;
+
+	return 0;
+}
+
+static int edev25g_change_mtu(struct net_device *netdev, int new_mtu)
+{
+	struct eth_dev *edev = net_device_to_eth_dev(netdev);
+	struct eth_com *mac = edev->mac;
+	int ret;
+
+	ret = eth_com_write_reg(mac, MAX_TX_SIZE_CONFIG, new_mtu);
+	if (ret)
+		return ret;
+
+	ret = eth_com_write_reg(mac, MAX_RX_SIZE_CONFIG, new_mtu);
+	if (ret)
+		return ret;
+
+	netdev->mtu = new_mtu;
+
+	return 0;
+}
+
+static int edev25g_set_loopback(struct net_device *netdev, bool en)
+{
+	struct eth_dev *edev = net_device_to_eth_dev(netdev);
+
+	return eth_com_write_reg(edev->mac, PHY_PMA_SLOOP, en);
+}
+
+static int edev25g_set_features(struct net_device *netdev,
+				netdev_features_t features)
+{
+	netdev_features_t changed = netdev->features ^ features;
+
+	if (changed & NETIF_F_LOOPBACK)
+		return edev25g_set_loopback(netdev,
+					    !!(features & NETIF_F_LOOPBACK));
+
+	return 0;
+}
+
+static const struct net_device_ops edev25g_netdev_ops = {
+	.ndo_open = edev25g_netdev_open,
+	.ndo_stop = edev25g_netdev_stop,
+	.ndo_change_mtu = edev25g_change_mtu,
+	.ndo_set_features = edev25g_set_features,
+	.ndo_start_xmit = n3000_dummy_netdev_xmit,
+};
+
+static void edev25g_adjust_link(struct net_device *netdev)
+{}
+
+static int edev25g_netdev_init(struct net_device *netdev)
+{
+	int ret;
+
+	ret = edev25g_pause_init(netdev);
+	if (ret)
+		return ret;
+
+	ret = edev25g_mtu_init(netdev);
+	if (ret)
+		return ret;
+
+	return edev25g_set_loopback(netdev,
+				    !!(netdev->features & NETIF_F_LOOPBACK));
+}
+
+static int dfl_eth_dev_25g_init(struct eth_dev *edev)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
+	struct device *dev = edev->dev;
+	struct phy_device *phydev;
+	struct net_device *netdev;
+	int ret;
+
+	netdev = n3000_netdev_create(edev);
+	if (!netdev)
+		return -ENOMEM;
+
+	netdev->hw_features |= NETIF_F_LOOPBACK;
+	netdev->netdev_ops = &edev25g_netdev_ops;
+	netdev->ethtool_ops = &edev25g_ethtool_ops;
+
+	phydev = phy_connect(netdev, edev->phy_id, edev25g_adjust_link,
+			     PHY_INTERFACE_MODE_NA);
+	if (IS_ERR(phydev)) {
+		dev_err(dev, "PHY connection failed\n");
+		ret = PTR_ERR(phydev);
+		goto err_free_netdev;
+	}
+
+	linkmode_set_bit(ETHTOOL_LINK_MODE_25000baseCR_Full_BIT, mask);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_25000baseSR_Full_BIT, mask);
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, mask);
+	linkmode_and(phydev->supported, phydev->supported, mask);
+	linkmode_copy(phydev->advertising, phydev->supported);
+
+	phy_attached_info(phydev);
+
+	ret = edev25g_netdev_init(netdev);
+	if (ret) {
+		dev_err(dev, "fail to init netdev %s\n", netdev->name);
+		goto err_phy_disconnect;
+	}
+
+	netdev->sysfs_groups[0] = &edev25g_attr_group;
+
+	netif_carrier_off(netdev);
+	ret = register_netdev(netdev);
+	if (ret) {
+		dev_err(dev, "fail to register netdev %s\n", netdev->name);
+		goto err_phy_disconnect;
+	}
+
+	edev->netdev = netdev;
+
+	return 0;
+
+err_phy_disconnect:
+	if (netdev->phydev)
+		phy_disconnect(phydev);
+err_free_netdev:
+	free_netdev(netdev);
+
+	return ret;
+}
+
+static void dfl_eth_dev_25g_remove(struct eth_dev *edev)
+{
+	struct net_device *netdev = edev->netdev;
+
+	if (netdev->phydev)
+		phy_disconnect(netdev->phydev);
+
+	unregister_netdev(netdev);
+}
+
+struct eth_dev_ops dfl_eth_dev_25g_ops = {
+	.lineside_init = dfl_eth_dev_25g_init,
+	.lineside_remove = dfl_eth_dev_25g_remove,
+	.reset = edev25g40g_reset,
+};
+
+struct eth_dev_ops dfl_eth_dev_40g_ops = {
+	.reset = edev25g40g_reset,
+};
diff --git a/drivers/net/ethernet/intel/dfl-eth-group-main.c b/drivers/net/ethernet/intel/dfl-eth-group-main.c
new file mode 100644
index 0000000..a29b8b1
--- /dev/null
+++ b/drivers/net/ethernet/intel/dfl-eth-group-main.c
@@ -0,0 +1,632 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* DFL device driver for Ether Group private feature on Intel PAC (Programmable
+ * Acceleration Card) N3000
+ *
+ * Copyright (C) 2019-2020 Intel Corporation, Inc.
+ *
+ * Authors:
+ *   Wu Hao <hao.wu@intel.com>
+ *   Xu Yilun <yilun.xu@intel.com>
+ */
+#include <linux/bitfield.h>
+#include <linux/dfl.h>
+#include <linux/errno.h>
+#include <linux/ethtool.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/io-64-nonatomic-lo-hi.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/rtnetlink.h>
+#include <linux/stddef.h>
+#include <linux/types.h>
+
+#include "dfl-eth-group.h"
+
+struct dfl_eth_group {
+	char name[32];
+	struct device *dev;
+	void __iomem *base;
+	/* lock to protect register access of the ether group */
+	struct mutex reg_lock;
+	struct dfl_device *dfl_dev;
+	unsigned int config;
+	unsigned int direction;
+	unsigned int group_id;
+	unsigned int speed;
+	unsigned int lw_mac;
+	unsigned int num_edevs;
+	struct eth_dev *edevs;
+	struct eth_dev_ops *ops;
+};
+
+u64 read_mac_stats(struct eth_com *ecom, unsigned int addr)
+{
+	u32 data_l, data_h;
+
+	if (eth_com_read_reg(ecom, addr, &data_l) ||
+	    eth_com_read_reg(ecom, addr + 1, &data_h))
+		return 0xffffffffffffffffULL;
+
+	return data_l + ((u64)data_h << 32);
+}
+
+netdev_tx_t n3000_dummy_netdev_xmit(struct sk_buff *skb,
+				    struct net_device *dev)
+{
+	kfree_skb(skb);
+	net_warn_ratelimited("%s(): Dropping skb.\n", __func__);
+	return NETDEV_TX_OK;
+}
+
+static void n3000_netdev_setup(struct net_device *netdev)
+{
+	netdev->features = 0;
+	netdev->hard_header_len = 0;
+	netdev->priv_flags |= IFF_NO_QUEUE;
+	netdev->needs_free_netdev = true;
+	netdev->min_mtu = 0;
+	netdev->max_mtu = ETH_MAX_MTU;
+}
+
+struct net_device *n3000_netdev_create(struct eth_dev *edev)
+{
+	struct dfl_eth_group *egroup = edev->egroup;
+	struct n3000_net_priv *priv;
+	struct net_device *netdev;
+	char name[IFNAMSIZ];
+
+	/* The name of n3000 network device is using this format "npacAgBlC"
+	 *
+	 * A is the unique ethdev index
+	 * B is the group id of this ETH Group.
+	 * C is the PHY/MAC link index for Line side ethernet group.
+	 */
+	snprintf(name, IFNAMSIZ, "npac%%dg%ul%u",
+		 egroup->group_id, edev->index);
+
+	netdev = alloc_netdev(sizeof(*priv), name, NET_NAME_UNKNOWN,
+			      n3000_netdev_setup);
+	if (!netdev)
+		return NULL;
+
+	priv = netdev_priv(netdev);
+	priv->edev = edev;
+	SET_NETDEV_DEV(netdev, egroup->dev);
+
+	return netdev;
+}
+
+enum n3000_eth_cfg {
+	ETH_CONFIG_8x10G,
+	ETH_CONFIG_4x25G,
+	ETH_CONFIG_2x1x25G,
+	ETH_CONFIG_4x25G_2x25G,
+	ETH_CONFIG_2x2x25G,
+	ETH_CONFIG_MAX
+};
+
+#define N3000_EDEV_MAX 8
+
+static int phy_addr_table[ETH_CONFIG_MAX][N3000_EDEV_MAX] = {
+	/* 8x10G configuration
+	 *
+	 *    [retimer_dev]   <------->   [eth_dev]
+	 *	  0			   0
+	 *	  1			   1
+	 *	  2			   2
+	 *	  3			   3
+	 *	  4			   4
+	 *	  5			   5
+	 *	  6			   6
+	 *	  7			   7
+	 */
+	[ETH_CONFIG_8x10G] = {0, 1, 2, 3, 4, 5, 6, 7},
+
+	/* 4x25G and 4x25G_2x25G configuration
+	 *
+	 *    [retimer_dev]   <------->   [eth_dev]
+	 *	  0			   0
+	 *	  1			   1
+	 *	  2			   2
+	 *	  3			   3
+	 *	  4
+	 *	  5
+	 *	  6
+	 *	  7
+	 */
+	[ETH_CONFIG_4x25G] = {0, 1, 2, 3, -1, -1, -1, -1},
+	[ETH_CONFIG_4x25G_2x25G] = {0, 1, 2, 3, -1, -1, -1, -1},
+
+	/* 2x1x25G configuration
+	 *
+	 *    [retimer_dev]   <------->   [eth_dev]
+	 *        0                      0
+	 *        1
+	 *        2
+	 *        3
+	 *        4                      1
+	 *        5
+	 *        6
+	 *        7
+	 */
+	[ETH_CONFIG_2x1x25G] = {0, 4, -1, -1, -1, -1, -1, -1},
+
+	/* 2x2x25G configuration
+	 *
+	 *    [retimer_dev]   <------->   [eth_dev]
+	 *	  0			   0
+	 *	  1			   1
+	 *	  2
+	 *	  3
+	 *	  4			   2
+	 *	  5			   3
+	 *	  6
+	 *	  7
+	 */
+	[ETH_CONFIG_2x2x25G] = {0, 1, 4, 5, -1, -1, -1, -1},
+};
+
+#define eth_group_for_each_dev(edev, egp) \
+	for ((edev) = (egp)->edevs; (edev) < (egp)->edevs + (egp)->num_edevs; \
+	     (edev)++)
+
+#define eth_group_reverse_each_dev(edev, egp) \
+	for ((edev)--; (edev) >= (egp)->edevs; (edev)--)
+
+static struct mii_bus *eth_group_get_phy_bus(struct dfl_eth_group *egroup)
+{
+	char mii_name[MII_BUS_ID_SIZE];
+	struct device *base_dev;
+	struct mii_bus *bus;
+
+	base_dev = dfl_dev_get_base_dev(egroup->dfl_dev);
+	if (!base_dev)
+		return ERR_PTR(-ENODEV);
+
+	snprintf(mii_name, MII_BUS_ID_SIZE, DFL_ETH_MII_ID_FMT,
+		 dev_name(base_dev));
+
+	bus = mdio_find_bus(mii_name);
+	if (!bus)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	return bus;
+}
+
+static int eth_dev_get_phy_id(struct eth_dev *edev, struct mii_bus *bus)
+{
+	struct dfl_eth_group *egroup = edev->egroup;
+	struct phy_device *phydev;
+	int phyaddr;
+
+	phyaddr = phy_addr_table[egroup->config][edev->index];
+	if (phyaddr < 0)
+		return -ENODEV;
+
+	phydev = mdiobus_get_phy(bus, phyaddr);
+	if (!phydev) {
+		dev_err(egroup->dev, "fail to get phydev\n");
+		return -EPROBE_DEFER;
+	}
+
+	strncpy(edev->phy_id, phydev_name(phydev), MII_BUS_ID_SIZE + 3);
+	edev->phy_id[MII_BUS_ID_SIZE + 2] = '\0';
+
+	return 0;
+}
+
+static int init_lineside_eth_devs(struct dfl_eth_group *egroup,
+				  struct mii_bus *phy_bus)
+{
+	struct eth_dev *edev;
+	int ret = 0;
+
+	if (!egroup->ops->lineside_init)
+		return -ENODEV;
+
+	eth_group_for_each_dev(edev, egroup) {
+		ret = eth_dev_get_phy_id(edev, phy_bus);
+		if (ret)
+			break;
+
+		ret = egroup->ops->lineside_init(edev);
+		if (ret)
+			break;
+	}
+
+	if (!ret)
+		return 0;
+
+	dev_err(egroup->dev, "failed to init lineside edev %d", edev->index);
+
+	if (egroup->ops->lineside_remove)
+		eth_group_reverse_each_dev(edev, egroup)
+			egroup->ops->lineside_remove(edev);
+
+	return ret;
+}
+
+static void remove_lineside_eth_devs(struct dfl_eth_group *egroup)
+{
+	struct eth_dev *edev;
+
+	if (!egroup->ops->lineside_remove)
+		return;
+
+	eth_group_for_each_dev(edev, egroup)
+		egroup->ops->lineside_remove(edev);
+}
+
+#define ETH_GROUP_INFO		0x8
+#define LIGHT_WEIGHT_MAC	BIT_ULL(25)
+#define INFO_DIRECTION		BIT_ULL(24)
+#define INFO_SPEED		GENMASK_ULL(23, 16)
+#define INFO_PHY_NUM		GENMASK_ULL(15, 8)
+#define INFO_GROUP_ID		GENMASK_ULL(7, 0)
+
+#define ETH_GROUP_CTRL		0x10
+#define CTRL_CMD		GENMASK_ULL(63, 62)
+#define CMD_NOP			0
+#define CMD_RD			1
+#define CMD_WR			2
+#define CTRL_DEV_SELECT		GENMASK_ULL(53, 49)
+#define CTRL_FEAT_SELECT	BIT_ULL(48)
+#define SELECT_IP		0
+#define SELECT_FEAT		1
+#define CTRL_ADDR		GENMASK_ULL(47, 32)
+#define CTRL_WR_DATA		GENMASK_ULL(31, 0)
+
+#define ETH_GROUP_STAT		0x18
+#define STAT_RW_VAL		BIT_ULL(32)
+#define STAT_RD_DATA		GENMASK_ULL(31, 0)
+
+enum ecom_type {
+	ETH_GROUP_PHY	= 1,
+	ETH_GROUP_MAC,
+	ETH_GROUP_ETHER
+};
+
+struct eth_com {
+	struct dfl_eth_group *egroup;
+	unsigned int type;
+	u8 select;
+};
+
+static const char *eth_com_type_string(enum ecom_type type)
+{
+	switch (type) {
+	case ETH_GROUP_PHY:
+		return "phy";
+	case ETH_GROUP_MAC:
+		return "mac";
+	case ETH_GROUP_ETHER:
+		return "ethernet wrapper";
+	default:
+		return "unknown";
+	}
+}
+
+#define eth_com_base(com)	((com)->egroup->base)
+#define eth_com_dev(com)	((com)->egroup->dev)
+
+#define RW_VAL_INVL		1 /* us */
+#define RW_VAL_POLL_TIMEOUT	10 /* us */
+
+static int __do_eth_com_write_reg(struct eth_com *ecom, bool add_feature,
+				  u16 addr, u32 data)
+{
+	void __iomem *base = eth_com_base(ecom);
+	struct device *dev = eth_com_dev(ecom);
+	u64 v = 0;
+
+	dev_dbg(dev, "%s [%s] select 0x%x add_feat %d addr 0x%x data 0x%x\n",
+		__func__, eth_com_type_string(ecom->type),
+		ecom->select, add_feature, addr, data);
+
+	/* only PHY has additional feature registers */
+	if (add_feature && ecom->type != ETH_GROUP_PHY)
+		return -EINVAL;
+
+	v |= FIELD_PREP(CTRL_CMD, CMD_WR);
+	v |= FIELD_PREP(CTRL_DEV_SELECT, ecom->select);
+	v |= FIELD_PREP(CTRL_ADDR, addr);
+	v |= FIELD_PREP(CTRL_WR_DATA, data);
+	v |= FIELD_PREP(CTRL_FEAT_SELECT, !!add_feature);
+
+	writeq(v, base + ETH_GROUP_CTRL);
+
+	if (readq_poll_timeout(base + ETH_GROUP_STAT, v, v & STAT_RW_VAL,
+			       RW_VAL_INVL, RW_VAL_POLL_TIMEOUT))
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int __do_eth_com_read_reg(struct eth_com *ecom, bool add_feature,
+				 u16 addr, u32 *data)
+{
+	void __iomem *base = eth_com_base(ecom);
+	struct device *dev = eth_com_dev(ecom);
+	u64 v = 0;
+
+	dev_dbg(dev, "%s [%s] select %x add_feat %d addr %x\n",
+		__func__, eth_com_type_string(ecom->type),
+		ecom->select, add_feature, addr);
+
+	/* only PHY has additional feature registers */
+	if (add_feature && ecom->type != ETH_GROUP_PHY)
+		return -EINVAL;
+
+	v |= FIELD_PREP(CTRL_CMD, CMD_RD);
+	v |= FIELD_PREP(CTRL_DEV_SELECT, ecom->select);
+	v |= FIELD_PREP(CTRL_ADDR, addr);
+	v |= FIELD_PREP(CTRL_FEAT_SELECT, !!add_feature);
+
+	writeq(v, base + ETH_GROUP_CTRL);
+
+	if (readq_poll_timeout(base + ETH_GROUP_STAT, v, v & STAT_RW_VAL,
+			       RW_VAL_INVL, RW_VAL_POLL_TIMEOUT))
+		return -ETIMEDOUT;
+
+	*data = FIELD_GET(STAT_RD_DATA, v);
+
+	return 0;
+}
+
+int do_eth_com_write_reg(struct eth_com *ecom, bool add_feature,
+			 u16 addr, u32 data)
+{
+	int ret;
+
+	mutex_lock(&ecom->egroup->reg_lock);
+	ret = __do_eth_com_write_reg(ecom, add_feature, addr, data);
+	mutex_unlock(&ecom->egroup->reg_lock);
+	return ret;
+}
+
+int do_eth_com_read_reg(struct eth_com *ecom, bool add_feature,
+			u16 addr, u32 *data)
+{
+	int ret;
+
+	mutex_lock(&ecom->egroup->reg_lock);
+	ret = __do_eth_com_read_reg(ecom, add_feature, addr, data);
+	mutex_unlock(&ecom->egroup->reg_lock);
+	return ret;
+}
+
+static struct eth_com *
+eth_com_create(struct dfl_eth_group *egroup, enum ecom_type type,
+	       unsigned int link_idx)
+{
+	struct eth_com *ecom;
+
+	ecom = devm_kzalloc(egroup->dev, sizeof(*ecom), GFP_KERNEL);
+	if (!ecom)
+		return ERR_PTR(-ENOMEM);
+
+	ecom->egroup = egroup;
+	ecom->type = type;
+
+	if (type == ETH_GROUP_PHY)
+		ecom->select = link_idx * 2 + 2;
+	else if (type == ETH_GROUP_MAC)
+		ecom->select = link_idx * 2 + 3;
+	else if (type == ETH_GROUP_ETHER)
+		ecom->select = 0;
+
+	return ecom;
+}
+
+static int init_eth_dev(struct eth_dev *edev, struct dfl_eth_group *egroup,
+			unsigned int link_idx)
+{
+	edev->egroup = egroup;
+	edev->dev = egroup->dev;
+	edev->index = link_idx;
+	edev->lw_mac = !!egroup->lw_mac;
+	edev->phy = eth_com_create(egroup, ETH_GROUP_PHY, link_idx);
+	if (IS_ERR(edev->phy))
+		return PTR_ERR(edev->phy);
+
+	edev->mac = eth_com_create(egroup, ETH_GROUP_MAC, link_idx);
+	if (IS_ERR(edev->mac))
+		return PTR_ERR(edev->mac);
+
+	return 0;
+}
+
+static int eth_devs_init(struct dfl_eth_group *egroup)
+{
+	int ret, i;
+
+	egroup->edevs = devm_kcalloc(egroup->dev, egroup->num_edevs,
+				     sizeof(*egroup->edevs), GFP_KERNEL);
+	if (!egroup->edevs)
+		return -ENOMEM;
+
+	for (i = 0; i < egroup->num_edevs; i++) {
+		ret = init_eth_dev(&egroup->edevs[i], egroup, i);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int eth_group_setup(struct dfl_eth_group *egroup)
+{
+	int net_cfg, ret;
+	u64 v;
+
+	/* read parameters of this ethernet components group */
+	v = readq(egroup->base + ETH_GROUP_INFO);
+
+	egroup->direction = FIELD_GET(INFO_DIRECTION, v);
+	egroup->speed = FIELD_GET(INFO_SPEED, v);
+	egroup->num_edevs = FIELD_GET(INFO_PHY_NUM, v);
+	egroup->group_id = FIELD_GET(INFO_GROUP_ID, v);
+	egroup->lw_mac = FIELD_GET(LIGHT_WEIGHT_MAC, v);
+
+	net_cfg = dfl_dev_get_vendor_net_cfg(egroup->dfl_dev);
+	if (net_cfg < 0)
+		return -EINVAL;
+
+	egroup->config = (unsigned int)net_cfg;
+
+	ret = eth_devs_init(egroup);
+	if (ret)
+		return ret;
+
+	switch (egroup->speed) {
+	case 25:
+		egroup->ops = &dfl_eth_dev_25g_ops;
+		break;
+	case 40:
+		egroup->ops = &dfl_eth_dev_40g_ops;
+		break;
+	}
+
+	mutex_init(&egroup->reg_lock);
+
+	return 0;
+}
+
+static void eth_group_destroy(struct dfl_eth_group *egroup)
+{
+	mutex_destroy(&egroup->reg_lock);
+}
+
+static void eth_group_devs_disable(struct dfl_eth_group *egroup)
+{
+	struct eth_dev *edev;
+
+	eth_group_for_each_dev(edev, egroup)
+		egroup->ops->reset(edev, true);
+}
+
+static int eth_group_devs_enable(struct dfl_eth_group *egroup)
+{
+	struct eth_dev *edev;
+	int ret;
+
+	eth_group_for_each_dev(edev, egroup) {
+		ret = egroup->ops->reset(edev, false);
+		if (ret) {
+			dev_err(egroup->dev, "fail to enable edev%d\n",
+				edev->index);
+			eth_group_devs_disable(egroup);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int dfl_eth_group_line_side_init(struct dfl_eth_group *egroup)
+{
+	struct mii_bus *phy_bus;
+	int ret;
+
+	if (!egroup->ops || !egroup->ops->reset ||
+	    !egroup->ops->lineside_init || !egroup->ops->lineside_remove)
+		return -EINVAL;
+
+	eth_group_devs_disable(egroup);
+
+	phy_bus = eth_group_get_phy_bus(egroup);
+	if (IS_ERR(phy_bus))
+		return PTR_ERR(phy_bus);
+
+	ret = init_lineside_eth_devs(egroup, phy_bus);
+	put_device(&phy_bus->dev);
+
+	return ret;
+}
+
+static void dfl_eth_group_line_side_uinit(struct dfl_eth_group *egroup)
+{
+	remove_lineside_eth_devs(egroup);
+}
+
+static int dfl_eth_group_host_side_init(struct dfl_eth_group *egroup)
+{
+	if (!egroup->ops || !egroup->ops->reset)
+		return -EINVAL;
+
+	return eth_group_devs_enable(egroup);
+}
+
+static int dfl_eth_group_probe(struct dfl_device *dfl_dev)
+{
+	struct device *dev = &dfl_dev->dev;
+	struct dfl_eth_group *egroup;
+	int ret;
+
+	egroup = devm_kzalloc(dev, sizeof(*egroup), GFP_KERNEL);
+	if (!egroup)
+		return -ENOMEM;
+
+	dev_set_drvdata(&dfl_dev->dev, egroup);
+
+	egroup->dev = dev;
+	egroup->dfl_dev = dfl_dev;
+
+	egroup->base = devm_ioremap_resource(dev, &dfl_dev->mmio_res);
+	if (IS_ERR(egroup->base)) {
+		dev_err(dev, "get mem resource fail!\n");
+		return PTR_ERR(egroup->base);
+	}
+
+	ret = eth_group_setup(egroup);
+	if (ret)
+		return ret;
+
+	if (egroup->direction == 1)
+		ret = dfl_eth_group_line_side_init(egroup);
+	else
+		ret = dfl_eth_group_host_side_init(egroup);
+
+	if (!ret)
+		return 0;
+
+	eth_group_destroy(egroup);
+
+	return ret;
+}
+
+static void dfl_eth_group_remove(struct dfl_device *dfl_dev)
+{
+	struct dfl_eth_group *egroup = dev_get_drvdata(&dfl_dev->dev);
+
+	if (egroup->direction == 1)
+		dfl_eth_group_line_side_uinit(egroup);
+
+	eth_group_devs_disable(egroup);
+	eth_group_destroy(egroup);
+}
+
+#define FME_FEATURE_ID_ETH_GROUP	0x10
+
+static const struct dfl_device_id dfl_eth_group_ids[] = {
+	{ FME_ID, FME_FEATURE_ID_ETH_GROUP },
+	{ }
+};
+
+static struct dfl_driver dfl_eth_group_driver = {
+	.drv	= {
+		.name       = "dfl-eth-group",
+	},
+	.id_table = dfl_eth_group_ids,
+	.probe   = dfl_eth_group_probe,
+	.remove  = dfl_eth_group_remove,
+};
+
+module_dfl_driver(dfl_eth_group_driver);
+
+MODULE_DEVICE_TABLE(dfl, dfl_eth_group_ids);
+MODULE_DESCRIPTION("DFL ether group driver");
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/net/ethernet/intel/dfl-eth-group.h b/drivers/net/ethernet/intel/dfl-eth-group.h
new file mode 100644
index 0000000..2e90f86
--- /dev/null
+++ b/drivers/net/ethernet/intel/dfl-eth-group.h
@@ -0,0 +1,83 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/* Internal header file for FPGA DFL Ether Group Driver
+ *
+ * Copyright (C) 2020 Intel Corporation. All rights reserved.
+ */
+
+#ifndef __DFL_ETH_GROUP_H__
+#define __DFL_ETH_GROUP_H__
+
+#include <linux/netdevice.h>
+#include <linux/phy.h>
+#include <linux/rtnetlink.h>
+
+/* Used when trying to find a virtual mii bus on a specific dfl device.
+ * dev_name(dfl base device)-mii
+ */
+#define DFL_ETH_MII_ID_FMT "%s-mii"
+
+struct eth_dev {
+	struct dfl_eth_group *egroup;
+	struct device *dev;
+	int index;
+	bool lw_mac;
+	struct eth_com *phy;
+	struct eth_com *mac;
+	struct net_device *netdev;
+
+	char phy_id[MII_BUS_ID_SIZE + 3];
+};
+
+struct eth_dev_ops {
+	int (*lineside_init)(struct eth_dev *edev);
+	void (*lineside_remove)(struct eth_dev *edev);
+	int (*reset)(struct eth_dev *edev, bool en);
+};
+
+struct n3000_net_priv {
+	struct eth_dev *edev;
+};
+
+static inline struct eth_dev *net_device_to_eth_dev(struct net_device *netdev)
+{
+	struct n3000_net_priv *priv = netdev_priv(netdev);
+
+	return priv->edev;
+}
+
+struct stat_info {
+	unsigned int addr;
+	char string[ETH_GSTRING_LEN];
+};
+
+#define STAT_INFO(_addr, _string) \
+	.addr = _addr, .string = _string,
+
+int do_eth_com_write_reg(struct eth_com *ecom, bool add_feature,
+			 u16 addr, u32 data);
+int do_eth_com_read_reg(struct eth_com *ecom, bool add_feature,
+			u16 addr, u32 *data);
+
+#define eth_com_write_reg(ecom, addr, data)	\
+	do_eth_com_write_reg(ecom, false, addr, data)
+
+#define eth_com_read_reg(ecom, addr, data)	\
+	do_eth_com_read_reg(ecom, false, addr, data)
+
+#define eth_com_add_feat_write_reg(ecom, addr, data)	\
+	do_eth_com_write_reg(ecom, true, addr, data)
+
+#define eth_com_add_feat_read_reg(ecom, addr, data)	\
+	do_eth_com_read_reg(ecom, true, addr, data)
+
+u64 read_mac_stats(struct eth_com *ecom, unsigned int addr);
+
+struct net_device *n3000_netdev_create(struct eth_dev *edev);
+netdev_tx_t n3000_dummy_netdev_xmit(struct sk_buff *skb,
+				    struct net_device *dev);
+
+extern struct eth_dev_ops dfl_eth_dev_25g_ops;
+extern struct eth_dev_ops dfl_eth_dev_40g_ops;
+
+#endif /* __DFL_ETH_GROUP_H__ */
diff --git a/drivers/net/ethernet/intel/intel-m10-bmc-retimer.c b/drivers/net/ethernet/intel/intel-m10-bmc-retimer.c
index c7b0558..3f686d2 100644
--- a/drivers/net/ethernet/intel/intel-m10-bmc-retimer.c
+++ b/drivers/net/ethernet/intel/intel-m10-bmc-retimer.c
@@ -10,6 +10,8 @@ 
 #include <linux/phy.h>
 #include <linux/platform_device.h>
 
+#include "dfl-eth-group.h"
+
 #define NUM_CHIP	2
 #define MAX_LINK	4
 
@@ -148,7 +150,7 @@  static int m10bmc_retimer_mii_bus_init(struct m10bmc_retimer *retimer)
 	bus->name = M10BMC_RETIMER_MII_NAME;
 	bus->read = m10bmc_retimer_read;
 	bus->write = m10bmc_retimer_write;
-	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii",
+	snprintf(bus->id, MII_BUS_ID_SIZE, DFL_ETH_MII_ID_FMT,
 		 dev_name(retimer->base_dev));
 	bus->parent = retimer->dev;
 	bus->phy_mask = ~(BITS_MASK(retimer->num_devs));