[PATCH v3 0/5] ARM: Add GXP UMAC Support

[PATCH v3 0/5] ARM: Add GXP UMAC Support

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

The GXP contains two Ethernet MACs that can be
connected externally to several physical devices. From an external
interface perspective the BMC provides two SERDES interface connections
capable of either SGMII or 1000Base-X operation. The BMC also provides
a RMII interface for sideband connections to external Ethernet controllers.

The primary MAC (umac0) can be mapped to either SGMII/1000-BaseX
SERDES interface.  The secondary MAC (umac1) can be mapped to only
the second SGMII/1000-Base X Serdes interface or it can be mapped for
RMII sideband.

The MDIO(mdio0) interface from the primary MAC (umac0) is used for
external PHY status. The MDIO(mdio1) interface from
the secondary MAC (umac1) is routed to the SGMII/100Base-X IP blocks
on the two SERDES interface connections. In most cases the internal
phy connects directly to the external phy.

---

Changes since v2:
 *Removed PHY Configuration from MAC driver to Bootloader
 *Fixed several issues with the Kconfig and Makefile for both
  the mdio and umac driver.
 *Fixed code alignment where applicable
 *Removed MDIO from MAC yaml.
 *Added description to explain the use-ncsi use case.
 *Removed multiple unecessary functions and function calls
  from MAC driver

Changes since v1:
 *Corrected improper descriptions and use of | in yaml files
 *Used reverse christmas tree format for network drivers
 *Moved gxp-umac-mdio.c to /mdio/
 *Fixed dependencies on both Kconfigs
 *Added COMPILE_TEST to both Kconfigs
 *Used devm_ functions where possible in both drivers
 *Moved mac-address to inside of port in yaml files
 *Exchanged listing individual yaml files for hpe,gxp*
 *Restricted use of le32

Nick Hawkins (5):
  dt-bindings: net: Add HPE GXP UMAC MDIO
  net: hpe: Add GXP UMAC MDIO
  dt-bindings: net: Add HPE GXP UMAC
  net: hpe: Add GXP UMAC Driver
  MAINTAINERS: HPE: Add GXP UMAC Networking Files

 .../bindings/net/hpe,gxp-umac-mdio.yaml       |  50 ++
 .../devicetree/bindings/net/hpe,gxp-umac.yaml |  97 +++
 MAINTAINERS                                   |   2 +
 drivers/net/ethernet/Kconfig                  |   1 +
 drivers/net/ethernet/Makefile                 |   1 +
 drivers/net/ethernet/hpe/Kconfig              |  32 +
 drivers/net/ethernet/hpe/Makefile             |   1 +
 drivers/net/ethernet/hpe/gxp-umac.c           | 759 ++++++++++++++++++
 drivers/net/ethernet/hpe/gxp-umac.h           |  89 ++
 drivers/net/mdio/Kconfig                      |  13 +
 drivers/net/mdio/Makefile                     |   1 +
 drivers/net/mdio/mdio-gxp-umac.c              | 142 ++++
 12 files changed, 1188 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/hpe,gxp-umac-mdio.yaml
 create mode 100644 Documentation/devicetree/bindings/net/hpe,gxp-umac.yaml
 create mode 100644 drivers/net/ethernet/hpe/Kconfig
 create mode 100644 drivers/net/ethernet/hpe/Makefile
 create mode 100644 drivers/net/ethernet/hpe/gxp-umac.c
 create mode 100644 drivers/net/ethernet/hpe/gxp-umac.h
 create mode 100644 drivers/net/mdio/mdio-gxp-umac.c

-- 
2.17.1

[PATCH v3 1/5] dt-bindings: net: Add HPE GXP UMAC MDIO

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

Provide access to the register regions and interrupt for Universal
MAC(UMAC). The driver under the hpe,gxp-umac-mdio will provide an
interface for managing both the internal and external PHYs.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

---
v3:
 * no change
v2:
 * remove |
 * remove unecessary description
---
 .../bindings/net/hpe,gxp-umac-mdio.yaml       | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/hpe,gxp-umac-mdio.yaml

diff --git a/Documentation/devicetree/bindings/net/hpe,gxp-umac-mdio.yaml b/Documentation/devicetree/bindings/net/hpe,gxp-umac-mdio.yaml
new file mode 100644
index 000000000000..a8ab93c681bf
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/hpe,gxp-umac-mdio.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/hpe,gxp-umac-mdio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HPE GXP UMAC MDIO Controller
+
+maintainers:
+  - Nicholas Hawkins <nick.hawkins@hpe.com>
+
+description:
+  The HPE GXP Unversal MAC (UMAC) MDIO controller provides a configuration
+  path for both external PHY's and SERDES connected PHY's.
+
+allOf:
+  - $ref: mdio.yaml#
+
+properties:
+  compatible:
+    const: hpe,gxp-umac-mdio
+
+  reg:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    mdio0: mdio@4080 {
+      compatible = "hpe,gxp-umac-mdio";
+      reg = <0x4080 0x10>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      ethphy0: ethernet-phy@0 {
+        compatible = "ethernet-phy-ieee802.3-c22";
+        phy-mode = "sgmii";
+        reg = <0>;
+      };
+    };
-- 
2.17.1

[PATCH v3 2/5] net: hpe: Add GXP UMAC MDIO

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

The GXP contains two Universal Ethernet MACs that can be
connected externally to several physical devices. From an external
interface perspective the BMC provides two SERDES interface connections
capable of either SGMII or 1000Base-X operation. The BMC also provides
a RMII interface for sideband connections to external Ethernet controllers.

The primary MAC (umac0) can be mapped to either SGMII/1000-BaseX
SERDES interface.  The secondary MAC (umac1) can be mapped to only
the second SGMII/1000-Base X Serdes interface or it can be mapped for
RMII sideband.

The MDIO(mdio0) interface from the primary MAC (umac0) is used for
external PHY status and configuration. The MDIO(mdio1) interface from
the secondary MAC (umac1) is routed to the SGMII/100Base-X IP blocks
on the two SERDES interface connections.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v3:
 *Remove Kconfig and Makefile changes for ethernet
 *Fixed Kconfig alignment and COMPILE_TEST issue
 *removed raw write/reads
 *Put consistent spacing in Kconfing help description
v2:
 *Move from /ethernet to /mdio
 *Add COMPILE_TEST to Kconfig
 *Fix christmas tree variable declaration layout
 *return the error code instead of using defined where possible
 *Modify Kconfig to add depends on OF_MDIO && HAS_IOMEM &&
  MDIO_DEVRES
 *replace , with ;
 *use devm_of_mdiobus_register
 *remove umac_mdio_remove function
 *remove of_ptr_match
 *fix size_of on alloc
---
 drivers/net/mdio/Kconfig         |  13 +++
 drivers/net/mdio/Makefile        |   1 +
 drivers/net/mdio/mdio-gxp-umac.c | 142 +++++++++++++++++++++++++++++++
 3 files changed, 156 insertions(+)
 create mode 100644 drivers/net/mdio/mdio-gxp-umac.c

diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
index 9ff2e6f22f3f..c3bf91c86fde 100644
--- a/drivers/net/mdio/Kconfig
+++ b/drivers/net/mdio/Kconfig
@@ -115,6 +115,19 @@ config MDIO_GPIO
 	  To compile this driver as a module, choose M here: the module
 	  will be called mdio-gpio.
 
+config GXP_UMAC_MDIO
+	tristate "GXP UMAC mdio support"
+	depends on ARCH_HPE || COMPILE_TEST
+	depends on OF_MDIO && HAS_IOMEM
+	depends on MDIO_DEVRES
+	help
+	  Say y here to support the GXP UMAC MDIO bus. The
+	  MDIO (mdio0) interface from the primary MAC (umac0)
+	  is used for external PHY status and configuration.
+	  The MDIO (mdio1) interface from the secondary MAC
+	  (umac1) is routed to the SGMII/100Base-X IP blocks
+	  on the two SERDES interface connections.
+
 config MDIO_HISI_FEMAC
 	tristate "Hisilicon FEMAC MDIO bus controller"
 	depends on HAS_IOMEM && OF_MDIO
diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile
index 7d4cb4c11e4e..4d00299e327f 100644
--- a/drivers/net/mdio/Makefile
+++ b/drivers/net/mdio/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_MDIO_BCM_UNIMAC)		+= mdio-bcm-unimac.o
 obj-$(CONFIG_MDIO_BITBANG)		+= mdio-bitbang.o
 obj-$(CONFIG_MDIO_CAVIUM)		+= mdio-cavium.o
 obj-$(CONFIG_MDIO_GPIO)			+= mdio-gpio.o
+obj-$(CONFIG_GXP_UMAC_MDIO)		+= mdio-gxp-umac.o
 obj-$(CONFIG_MDIO_HISI_FEMAC)		+= mdio-hisi-femac.o
 obj-$(CONFIG_MDIO_I2C)			+= mdio-i2c.o
 obj-$(CONFIG_MDIO_IPQ4019)		+= mdio-ipq4019.o
diff --git a/drivers/net/mdio/mdio-gxp-umac.c b/drivers/net/mdio/mdio-gxp-umac.c
new file mode 100644
index 000000000000..bd1996e937c1
--- /dev/null
+++ b/drivers/net/mdio/mdio-gxp-umac.c
@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (C) 2023 Hewlett-Packard Development Company, L.P. */
+
+#include <linux/err.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_mdio.h>
+#include <linux/platform_device.h>
+
+#define UMAC_MII                0x00  /* R/W MII Register */
+#define UMAC_MII_PHY_ADDR_MASK  0x001F0000
+#define UMAC_MII_PHY_ADDR_SHIFT 16
+#define UMAC_MII_MOWNER         BIT(9)
+#define UMAC_MII_MRNW           BIT(8)
+#define UMAC_MII_REG_ADDR_MASK  0x0000001F
+#define UMAC_MII_DATA           0x04  /* R/W MII Data Register */
+
+struct umac_mdio_priv {
+	void __iomem *base;
+};
+
+static int umac_mdio_read(struct mii_bus *bus, int phy_id, int reg)
+{
+	struct umac_mdio_priv *umac_mdio = bus->priv;
+	unsigned int status;
+	unsigned int value;
+	int ret;
+
+	status = readl(umac_mdio->base + UMAC_MII);
+
+	status &= ~(UMAC_MII_PHY_ADDR_MASK | UMAC_MII_REG_ADDR_MASK);
+	status |= ((phy_id << UMAC_MII_PHY_ADDR_SHIFT) &
+			UMAC_MII_PHY_ADDR_MASK);
+	status |= (reg & UMAC_MII_REG_ADDR_MASK);
+	status |= UMAC_MII_MRNW; /* set bit for read mode */
+	writel(status, umac_mdio->base + UMAC_MII);
+
+	status |= UMAC_MII_MOWNER; /* set bit to activate mii transfer */
+	writel(status, umac_mdio->base + UMAC_MII);
+
+	ret = readl_poll_timeout(umac_mdio->base + UMAC_MII, status,
+				 !(status & UMAC_MII_MOWNER), 1000, 100000);
+	if (ret) {
+		dev_err(bus->parent, "mdio read time out\n");
+		return ret;
+	}
+
+	value = readl(umac_mdio->base + UMAC_MII_DATA);
+	return value;
+}
+
+static int umac_mdio_write(struct mii_bus *bus, int phy_id, int reg, u16 value)
+{
+	struct umac_mdio_priv *umac_mdio = bus->priv;
+	unsigned int status;
+	int ret;
+
+	writel(value, umac_mdio->base + UMAC_MII_DATA);
+
+	status = readl(umac_mdio->base + UMAC_MII);
+
+	status &= ~(UMAC_MII_PHY_ADDR_MASK | UMAC_MII_REG_ADDR_MASK);
+	status |= ((phy_id << UMAC_MII_PHY_ADDR_SHIFT) &
+			UMAC_MII_PHY_ADDR_MASK);
+	status |= (reg & UMAC_MII_REG_ADDR_MASK);
+	status &= ~UMAC_MII_MRNW; /* clear bit for write mode */
+	writel(status, umac_mdio->base + UMAC_MII);
+
+	status |= UMAC_MII_MOWNER; /* set bit to activate mii transfer */
+	writel(status, umac_mdio->base + UMAC_MII);
+
+	ret = readl_poll_timeout(umac_mdio->base + UMAC_MII, status,
+				 !(status & UMAC_MII_MOWNER), 1000, 100000);
+	if (ret)
+		dev_err(bus->parent, "mdio read time out\n");
+
+	return ret;
+}
+
+static int umac_mdio_probe(struct platform_device *pdev)
+{
+	struct umac_mdio_priv *umac_mdio;
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	struct mii_bus *bus;
+	int ret;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "fail to get resource\n");
+		return -ENODEV;
+	}
+
+	bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*umac_mdio));
+	if (!bus) {
+		dev_err(&pdev->dev, "failed to alloc mii bus\n");
+		return -ENOMEM;
+	}
+
+	snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(&pdev->dev));
+
+	bus->name	= dev_name(&pdev->dev);
+	bus->read	= umac_mdio_read;
+	bus->write	= umac_mdio_write;
+	bus->parent	= &pdev->dev;
+	umac_mdio = bus->priv;
+	umac_mdio->base = devm_ioremap_resource(&pdev->dev, res);
+	if (!umac_mdio->base) {
+		dev_err(&pdev->dev, "failed to do ioremap\n");
+		return -ENODEV;
+	}
+
+	ret = devm_of_mdiobus_register(dev, bus, pdev->dev.of_node);
+
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id umac_mdio_of_matches[] = {
+	{ .compatible = "hpe,gxp-umac-mdio", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, umac_mdio_of_matches);
+
+static struct platform_driver umac_driver = {
+	.driver	= {
+		.name    = "gxp-umac-mdio",
+		.of_match_table = umac_mdio_of_matches,
+	},
+	.probe   = umac_mdio_probe,
+};
+
+module_platform_driver(umac_driver);
+
+MODULE_AUTHOR("Nick Hawkins <nick.hawkins@hpe.com>");
+MODULE_DESCRIPTION("HPE GXP UMAC MDIO driver");
+MODULE_LICENSE("GPL");
-- 
2.17.1

[PATCH v3 3/5] dt-bindings: net: Add HPE GXP UMAC

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

Provide access to the register regions and interrupt for Universal
MAC(UMAC). The driver under the hpe,gxp-umac binding will provide an
interface for sending and receiving networking data from both of the
UMACs on the system.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---

v3:
 *Remove MDIO references
 *Modify description for use-ncsi
v2:
 *Move mac-addresses into ports
 *Remove | where not needed
---
 .../devicetree/bindings/net/hpe,gxp-umac.yaml | 97 +++++++++++++++++++
 1 file changed, 97 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/hpe,gxp-umac.yaml

diff --git a/Documentation/devicetree/bindings/net/hpe,gxp-umac.yaml b/Documentation/devicetree/bindings/net/hpe,gxp-umac.yaml
new file mode 100644
index 000000000000..d3f72694c814
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/hpe,gxp-umac.yaml
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/hpe,gxp-umac.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HPE GXP Unified MAC Controller
+
+maintainers:
+  - Nick Hawkins <nick.hawkins@hpe.com>
+
+description:
+  HPE GXP 802.3 10/100/1000T Ethernet Unifed MAC controller.
+  Device node of the controller has following properties.
+
+properties:
+  compatible:
+    const: hpe,gxp-umac
+
+  use-ncsi:
+    type: boolean
+    description:
+      Indicates if the device should use NCSI (Network Controlled
+      Sideband Interface). Only one of the two MACs can support
+      NCSI and it requires there to be a physical connection on
+      the board to be present. This property indicates that
+      physical connection is present and should be used.
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  ethernet-ports:
+    type: object
+    additionalProperties: false
+    description: Ethernet ports to PHY
+
+    properties:
+      "#address-cells":
+        const: 1
+
+      "#size-cells":
+        const: 0
+
+    patternProperties:
+      "^port@[0-1]$":
+        type: object
+        additionalProperties: false
+        description: Port to PHY
+
+        properties:
+          reg:
+            minimum: 0
+            maximum: 1
+
+          phy-handle:
+            maxItems: 1
+
+          mac-address: true
+
+        required:
+          - reg
+          - phy-handle
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - ethernet-ports
+
+examples:
+  - |
+    ethernet@4000 {
+      compatible = "hpe,gxp-umac";
+      reg = <0x4000 0x80>;
+      interrupts = <22>;
+      ethernet-ports {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        port@0 {
+          reg = <0>;
+          phy-handle = <&int_phy0>;
+          mac-address = [00 00 00 00 00 00];
+        };
+
+        port@1 {
+          reg = <1>;
+          phy-handle = <&ext_phy1>;
+          mac-address = [00 00 00 00 00 00];
+        };
+      };
+    };
-- 
2.17.1

[PATCH v3 4/5] net: hpe: Add GXP UMAC Driver

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

The GXP contains two Ethernet MACs that can be connected externally
to several physical devices. From an external interface perspective
the BMC provides two SERDES interface connections capable of either
SGMII or 1000Base-X operation. The BMC also provides a RMII interface
for sideband connections to external Ethernet controllers.

The primary MAC (umac0) can be mapped to either SGMII/1000-BaseX
SERDES interface.  The secondary MAC (umac1) can be mapped to only
the second SGMII/1000-Base X Serdes interface or it can be mapped for
RMII sideband.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v3:
 *Modify Kconfig and Makefile for ethernet directory for hpe
 *Remove version driver info
 *Removed direct writes to PHYs in MAC driver
 *Removed fixed-link code
 *Fixed code alignment
 *Updated commit description
 *Removed Direct PHY Configuration
v2:
 *Change to reverse christmas tree format
 *Fix use of cpu_to_le32
 *Add use of __le32 variable
 *use devm_alloc_etherdev
 *Removed remove function
 *Used devm_register_netdev
 *Removed unnecessary variable err
 *Removed free_netdev
 *Fixed compatible string
 *Removed of_match_ptr
 *Fixed return on failure to register
---
 drivers/net/ethernet/Kconfig        |   1 +
 drivers/net/ethernet/Makefile       |   1 +
 drivers/net/ethernet/hpe/Kconfig    |  32 ++
 drivers/net/ethernet/hpe/Makefile   |   1 +
 drivers/net/ethernet/hpe/gxp-umac.c | 759 ++++++++++++++++++++++++++++
 drivers/net/ethernet/hpe/gxp-umac.h |  89 ++++
 6 files changed, 883 insertions(+)
 create mode 100644 drivers/net/ethernet/hpe/Kconfig
 create mode 100644 drivers/net/ethernet/hpe/Makefile
 create mode 100644 drivers/net/ethernet/hpe/gxp-umac.c
 create mode 100644 drivers/net/ethernet/hpe/gxp-umac.h

diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index 5a274b99f299..b4921b84be51 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -80,6 +80,7 @@ source "drivers/net/ethernet/fujitsu/Kconfig"
 source "drivers/net/ethernet/fungible/Kconfig"
 source "drivers/net/ethernet/google/Kconfig"
 source "drivers/net/ethernet/hisilicon/Kconfig"
+source "drivers/net/ethernet/hpe/Kconfig"
 source "drivers/net/ethernet/huawei/Kconfig"
 source "drivers/net/ethernet/i825xx/Kconfig"
 source "drivers/net/ethernet/ibm/Kconfig"
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
index 0d872d4efcd1..ce97bbae2cdc 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_NET_VENDOR_FUJITSU) += fujitsu/
 obj-$(CONFIG_NET_VENDOR_FUNGIBLE) += fungible/
 obj-$(CONFIG_NET_VENDOR_GOOGLE) += google/
 obj-$(CONFIG_NET_VENDOR_HISILICON) += hisilicon/
+obj-$(CONFIG_NET_VENDOR_HPE) += hpe/
 obj-$(CONFIG_NET_VENDOR_HUAWEI) += huawei/
 obj-$(CONFIG_NET_VENDOR_IBM) += ibm/
 obj-$(CONFIG_NET_VENDOR_INTEL) += intel/
diff --git a/drivers/net/ethernet/hpe/Kconfig b/drivers/net/ethernet/hpe/Kconfig
new file mode 100644
index 000000000000..6d24ef413067
--- /dev/null
+++ b/drivers/net/ethernet/hpe/Kconfig
@@ -0,0 +1,32 @@
+config NET_VENDOR_HPE
+	bool "HPE device"
+	default y
+	depends on ARCH_HPE
+	help
+	  Say y here to support the HPE network devices.
+	  The GXP contains two Ethernet MACs that can be
+	  connected externally to several physical devices.
+	  From an external interface perspective the BMC
+	  provides two SERDES interface connections capable
+	  of either SGMII or 1000Base-X operation. The BMC
+	  also provides a RMII interface for sideband
+	  connections to external Ethernet controllers.
+
+if NET_VENDOR_HPE
+
+config GXP_UMAC
+	tristate "GXP UMAC support"
+	depends on ARCH_HPE || COMPILE_TEST
+	select CRC32
+	select MII
+	select PHYLIB
+	select GXP_UMAC_MDIO
+	help
+	  Say y here to support the GXP UMACs interface. The
+	  primary MAC (umac0) can be mapped to either
+	  SGMII/1000-BaseX SERDES interface. The secondary MAC
+	  (umac1) can be mapped to only the second
+	  SGMII/1000-Base X Serdes interface or it can be
+	  mapped for RMII sideband.
+
+endif
diff --git a/drivers/net/ethernet/hpe/Makefile b/drivers/net/ethernet/hpe/Makefile
new file mode 100644
index 000000000000..e84bb86f82bc
--- /dev/null
+++ b/drivers/net/ethernet/hpe/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_GXP_UMAC) += gxp-umac.o
diff --git a/drivers/net/ethernet/hpe/gxp-umac.c b/drivers/net/ethernet/hpe/gxp-umac.c
new file mode 100644
index 000000000000..148ad36a73c6
--- /dev/null
+++ b/drivers/net/ethernet/hpe/gxp-umac.c
@@ -0,0 +1,759 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (C) 2023 Hewlett-Packard Enterprise Development Company, L.P. */
+
+#include <linux/dma-mapping.h>
+#include <linux/etherdevice.h>
+#include <linux/ethtool.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <net/ncsi.h>
+#include <linux/of_device.h>
+#include <linux/of_mdio.h>
+#include <linux/of_net.h>
+#include <linux/phy.h>
+#include "gxp-umac.h"
+
+#define DRV_MODULE_NAME		"gxp-umac"
+
+#define NUMBER_OF_PORTS 2
+#define EXTERNAL_PORT 1
+#define INTERNAL_PORT 0
+
+struct umac_priv {
+	void __iomem *base;
+	int irq;
+	struct platform_device *pdev;
+	struct umac_tx_descs *tx_descs;
+	struct umac_rx_descs *rx_descs;
+	dma_addr_t tx_descs_dma_addr;
+	dma_addr_t rx_descs_dma_addr;
+	unsigned int tx_cur;
+	unsigned int tx_done;
+	unsigned int rx_cur;
+	struct napi_struct napi;
+	struct net_device *ndev;
+	struct phy_device *phy_dev;
+	struct phy_device *int_phy_dev;
+	struct ncsi_dev *ncsidev;
+	bool use_ncsi;
+};
+
+static void umac_get_drvinfo(struct net_device *ndev,
+			     struct ethtool_drvinfo *info)
+{
+	strscpy(info->driver, DRV_MODULE_NAME, sizeof(info->driver));
+}
+
+static struct net_device_stats *umac_get_stats(struct net_device *ndev)
+{
+	return &ndev->stats;
+}
+
+static int umac_ioctl(struct net_device *ndev, struct ifreq *ifr, int cmd)
+{
+	if (!netif_running(ndev))
+		return -EINVAL;
+
+	if (!ndev->phydev)
+		return -ENODEV;
+
+	return phy_mii_ioctl(ndev->phydev, ifr, cmd);
+}
+
+static void umac_set_mac_address(struct net_device *ndev, void *p_addr)
+{
+	struct umac_priv *umac = netdev_priv(ndev);
+	char *addr = (char *)p_addr;
+	unsigned int value;
+
+	/* update address to register */
+	value = addr[0] << 8 | addr[1];
+	writel(value, umac->base + UMAC_MAC_ADDR_HI);
+	value = addr[2] << 8 | addr[3];
+	writel(value, umac->base + UMAC_MAC_ADDR_MID);
+	value = addr[4] << 8 | addr[5];
+	writel(value, umac->base + UMAC_MAC_ADDR_LO);
+}
+
+static int umac_eth_mac_addr(struct net_device *ndev, void *p)
+{
+	struct sockaddr *addr = p;
+	int ret;
+
+	ret = eth_prepare_mac_addr_change(ndev, p);
+	if (ret < 0)
+		return ret;
+
+	eth_commit_mac_addr_change(ndev, p);
+	umac_set_mac_address(ndev, addr->sa_data);
+
+	return 0;
+}
+
+static void umac_channel_enable(struct umac_priv *umac)
+{
+	unsigned int value;
+
+	value = readl(umac->base + UMAC_CONFIG_STATUS);
+	value |= UMAC_CFG_TXEN | UMAC_CFG_RXEN;
+	writel(value, umac->base + UMAC_CONFIG_STATUS);
+
+	/* start processing by writing the ring prompt register */
+	writel(0, umac->base + UMAC_RING_PROMPT);
+}
+
+static void umac_channel_disable(struct umac_priv *umac)
+{
+	writel(0, umac->base + UMAC_CONFIG_STATUS);
+}
+
+static int umac_init_ring_discriptor(struct net_device *ndev)
+{
+	struct umac_priv *umac = netdev_priv(ndev);
+	struct platform_device *pdev = umac->pdev;
+
+	struct umac_tx_desc_entry *ptxdesc;
+	struct umac_rx_desc_entry *prxdesc;
+
+	unsigned int i;
+
+	if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32))) {
+		netdev_err(ndev, "No suitable DMA available\n");
+		return -ENOMEM;
+	}
+
+	umac->tx_descs = dma_alloc_coherent(&pdev->dev,
+					    sizeof(struct umac_tx_descs),
+					    &umac->tx_descs_dma_addr, GFP_KERNEL);
+	if (!umac->tx_descs)
+		return -ENOMEM;
+
+	umac->rx_descs = dma_alloc_coherent(&pdev->dev,
+					    sizeof(struct umac_rx_descs),
+					    &umac->rx_descs_dma_addr, GFP_KERNEL);
+	if (!umac->rx_descs) {
+		dma_free_coherent(&pdev->dev, sizeof(struct umac_tx_descs),
+				  umac->tx_descs,
+				  umac->tx_descs_dma_addr);
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < UMAC_MAX_TX_DESC_ENTRIES; i++) {
+		ptxdesc = &umac->tx_descs->entrylist[i];
+		ptxdesc->dmaaddress = cpu_to_le32(umac->tx_descs_dma_addr +
+						  offsetof(struct umac_tx_descs,
+							   framelist[i][0]));
+	}
+
+	for (i = 0; i < UMAC_MAX_RX_DESC_ENTRIES; i++) {
+		prxdesc = &umac->rx_descs->entrylist[i];
+		prxdesc->dmaaddress = cpu_to_le32(umac->rx_descs_dma_addr +
+						  offsetof(struct umac_rx_descs,
+							   framelist[i][0]));
+		prxdesc->status = UMAC_RING_ENTRY_HW_OWN;
+		prxdesc->count = UMAC_MAX_RX_FRAME_SIZE;
+	}
+
+	umac->tx_cur = 0;
+	umac->tx_done = 0;
+	umac->rx_cur = 0;
+
+	return 0;
+}
+
+static int umac_init_hw(struct net_device *ndev)
+{
+	struct umac_priv *umac = netdev_priv(ndev);
+	unsigned int value;
+
+	/* initialize tx and rx rings to first entry */
+	writel(0, umac->base + UMAC_RING_PTR);
+
+	/* clear the missed bit */
+	writel(0, umac->base + UMAC_CLEAR_STATUS);
+
+	/* disable checksum generation */
+	writel(0, umac->base + UMAC_CKSUM_CONFIG);
+
+	/* write the ring size register */
+	value = ((UMAC_RING_SIZE_256 << UMAC_TX_RING_SIZE_SHIFT) &
+			UMAC_TX_RING_SIZE_MASK) |
+		((UMAC_RING_SIZE_256 << UMAC_RX_RING_SIZE_SHIFT) &
+			UMAC_RX_RING_SIZE_MASK);
+	writel(value, umac->base + UMAC_RING_SIZE);
+
+	/* write rx ring base address */
+	writel(umac->rx_descs_dma_addr,
+	       umac->base + UMAC_RX_RING_ADDR);
+
+	/* write tx ring base address */
+	writel(umac->tx_descs_dma_addr,
+	       umac->base + UMAC_TX_RING_ADDR);
+
+	/* write burst size */
+	writel(0x22, umac->base + UMAC_DMA_CONFIG);
+
+	umac_channel_disable(umac);
+
+	/* disable clocks and gigabit mode (leave channels disabled) */
+	value = readl(umac->base + UMAC_CONFIG_STATUS);
+	value &= 0xfffff9ff;
+	writel(value, umac->base + UMAC_CONFIG_STATUS);
+	udelay(2);
+
+	if (umac->use_ncsi) {
+		/* set correct tx clock */
+		value &= UMAC_CFG_TX_CLK_EN;
+		value &= ~UMAC_CFG_GTX_CLK_EN;
+		value &= ~UMAC_CFG_GIGABIT_MODE; /* RMII mode */
+		value |= UMAC_CFG_FULL_DUPLEX; /* full duplex */
+	} else {
+		value |= UMAC_CFG_FULL_DUPLEX;
+
+		if (ndev->phydev->speed == SPEED_1000) {
+			value &= ~UMAC_CFG_TX_CLK_EN;
+			value |= UMAC_CFG_GTX_CLK_EN;
+			value |= UMAC_CFG_GIGABIT_MODE;
+		} else {
+			value |= UMAC_CFG_TX_CLK_EN;
+			value &= ~UMAC_CFG_GTX_CLK_EN;
+			value &= ~UMAC_CFG_GIGABIT_MODE;
+		}
+	}
+	writel(value, umac->base + UMAC_CONFIG_STATUS);
+	udelay(2);
+
+	umac_channel_enable(umac);
+
+	return 0;
+}
+
+static int umac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
+{
+	struct umac_priv *umac = netdev_priv(ndev);
+	struct umac_tx_desc_entry *ptxdesc;
+	unsigned int length;
+	u8 *pframe;
+
+	ptxdesc = &umac->tx_descs->entrylist[umac->tx_cur];
+	pframe = umac->tx_descs->framelist[umac->tx_cur];
+
+	length = skb->len;
+	if (length > 1514) {
+		netdev_err(ndev, "send data %d bytes > 1514, clamp it to 1514\n",
+			   skb->len);
+		length = 1514;
+	}
+
+	memset(pframe, 0, UMAC_MAX_FRAME_SIZE);
+	memcpy(pframe, skb->data, length);
+
+	if (length < ETH_ZLEN)
+		length = ETH_ZLEN; /* minimum tx byte */
+
+	ptxdesc->count = length;
+	ptxdesc->status = UMAC_RING_ENTRY_HW_OWN;
+	ptxdesc->cksumoffset = 0; /* disable checksum generation */
+
+	umac->tx_cur++;
+	if (umac->tx_cur >= UMAC_MAX_TX_DESC_ENTRIES)
+		umac->tx_cur = 0;
+
+	/* if current tx ring buffer is full, stop the queue */
+	ptxdesc = &umac->tx_descs->entrylist[umac->tx_cur];
+	if (ptxdesc->status & UMAC_RING_ENTRY_HW_OWN)
+		netif_stop_queue(ndev);
+
+	/* start processing by writing the ring prompt register */
+	writel(0, umac->base + UMAC_RING_PROMPT);
+	dev_kfree_skb(skb);
+
+	return NETDEV_TX_OK;
+}
+
+static int umac_rx(struct net_device *ndev, int budget)
+{
+	struct umac_priv *umac = netdev_priv(ndev);
+
+	struct umac_rx_desc_entry *prxdesc;
+	struct sk_buff *skb;
+
+	unsigned int rxlength;
+	int rxpktcount = 0;
+	u8 *pframe;
+	u8 *skb_buf;
+
+	prxdesc = &umac->rx_descs->entrylist[umac->rx_cur];
+	pframe = umac->rx_descs->framelist[umac->rx_cur];
+
+	while (!(prxdesc->status & UMAC_RING_ENTRY_HW_OWN)) {
+		rxlength = prxdesc->count;
+		skb = netdev_alloc_skb(ndev, rxlength);
+		if (!skb) {
+			/* run out of memory */
+			ndev->stats.rx_dropped++;
+			return rxpktcount;
+		}
+
+		/* make 16 bytes aligned for 14 bytes ethernet header */
+		skb_buf = skb_put(skb, rxlength);
+		memcpy(skb_buf, pframe, rxlength);
+
+		skb->protocol = eth_type_trans(skb, ndev);
+		netif_receive_skb(skb);
+		rxpktcount++;
+
+		prxdesc->status = UMAC_RING_ENTRY_HW_OWN;
+		prxdesc->count = UMAC_MAX_FRAME_SIZE;
+
+		ndev->stats.rx_packets++;
+		ndev->stats.rx_bytes += rxlength;
+
+		/* move to next buffer */
+		umac->rx_cur++;
+		if (umac->rx_cur >= UMAC_MAX_RX_DESC_ENTRIES)
+			umac->rx_cur = 0;
+
+		if (rxpktcount >= budget)
+			break;
+
+		prxdesc = &umac->rx_descs->entrylist[umac->rx_cur];
+		pframe = umac->rx_descs->framelist[umac->rx_cur];
+	}
+	/* start processing by writing the ring prompt register */
+	writel(0, umac->base + UMAC_RING_PROMPT);
+
+	return rxpktcount;
+}
+
+static void umac_tx_done(struct net_device *ndev)
+{
+	struct umac_priv *umac = netdev_priv(ndev);
+
+	unsigned int txptr;
+	unsigned int value;
+	struct umac_tx_desc_entry *ptxdesc;
+
+	value = readl(umac->base + UMAC_RING_PTR);
+	txptr = (value & UMAC_TX_RING_PTR_MASK) >> UMAC_TX_RING_PTR_SHIFT;
+
+	ptxdesc = &umac->tx_descs->entrylist[umac->tx_done];
+
+	while (!(ptxdesc->status & UMAC_RING_ENTRY_HW_OWN)) {
+		if (umac->tx_done == txptr)
+			break;
+
+		ndev->stats.tx_packets++;
+		ndev->stats.tx_bytes += ptxdesc->count;
+
+		umac->tx_done++;
+		if (umac->tx_done >= UMAC_MAX_TX_DESC_ENTRIES)
+			umac->tx_done = 0;
+		ptxdesc = &umac->tx_descs->entrylist[umac->tx_done];
+	}
+
+	/* clear tx interrupt */
+	value = readl(umac->base + UMAC_INTERRUPT);
+	value &= ~UMAC_TX_INT;
+	writel(value, umac->base + UMAC_INTERRUPT);
+
+	if (netif_queue_stopped(ndev))
+		netif_wake_queue(ndev);
+}
+
+static void umac_irq_enable(struct umac_priv *umac)
+{
+	unsigned int value;
+
+	/* enable interrupt */
+	value = readl(umac->base + UMAC_INTERRUPT);
+	value |= (UMAC_RX_INTEN | UMAC_TX_INTEN);
+	writel(value, umac->base + UMAC_INTERRUPT);
+}
+
+static void umac_irq_disable(struct umac_priv *umac)
+{
+	unsigned int value;
+
+	/* clear and disable interrupt */
+	value = readl(umac->base + UMAC_INTERRUPT);
+	value |= (UMAC_RX_INT | UMAC_TX_INT);
+	value &= ~(UMAC_RX_INTEN | UMAC_TX_INTEN);
+	writel(value, umac->base + UMAC_INTERRUPT);
+}
+
+static irqreturn_t umac_interrupt(int irq, void *p_ndev)
+{
+	struct net_device *ndev = (struct net_device *)p_ndev;
+	struct umac_priv *umac = netdev_priv(ndev);
+
+	if (umac->use_ncsi || netif_running(ndev)) {
+		umac_irq_disable(umac);
+		napi_schedule(&umac->napi);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int umac_poll(struct napi_struct *napi, int budget)
+{
+	struct umac_priv *umac = container_of(napi, struct umac_priv, napi);
+	struct net_device *ndev = umac->ndev;
+	unsigned int value;
+	int rx_done;
+
+	umac_tx_done(ndev);
+
+	rx_done = umac_rx(ndev, budget);
+
+	if (rx_done < budget) {
+		napi_complete_done(napi, rx_done);
+		/* clear rx interrupt */
+		value = readl(umac->base + UMAC_INTERRUPT);
+		value &= ~UMAC_RX_INT;
+		writel(value, umac->base + UMAC_INTERRUPT);
+
+		/* enable interrupt */
+		umac_irq_enable(umac);
+	}
+
+	return rx_done;
+}
+
+static int umac_open(struct net_device *ndev)
+{
+	struct umac_priv *umac = netdev_priv(ndev);
+	int err;
+
+	if (request_irq(ndev->irq, umac_interrupt, 0x0, ndev->name, ndev)) {
+		netdev_err(ndev, "failed to register irq\n");
+		return -EAGAIN;
+	}
+
+	umac_init_ring_discriptor(ndev);
+	umac_init_hw(ndev);
+
+	if (umac->use_ncsi)
+		netif_carrier_on(ndev);
+	else
+		phy_start(ndev->phydev);
+
+	napi_enable(&umac->napi);
+	netif_start_queue(ndev);
+	umac_irq_enable(umac);
+
+	if (umac->use_ncsi) {
+		err = ncsi_start_dev(umac->ncsidev);
+		if (err) {
+			netdev_err(ndev, "failed to start ncsi\n");
+			free_irq(ndev->irq, ndev);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static int umac_stop(struct net_device *ndev)
+{
+	struct umac_priv *umac = netdev_priv(ndev);
+	struct platform_device *pdev = umac->pdev;
+
+	dma_free_coherent(&pdev->dev, sizeof(struct umac_tx_descs),
+			  umac->tx_descs, umac->tx_descs_dma_addr);
+	dma_free_coherent(&pdev->dev, sizeof(struct umac_rx_descs),
+			  umac->rx_descs, umac->rx_descs_dma_addr);
+	netif_stop_queue(ndev);
+
+	if (umac->use_ncsi)
+		ncsi_stop_dev(umac->ncsidev);
+	else
+		phy_stop(ndev->phydev);
+	umac_irq_disable(umac);
+	umac_channel_disable(umac);
+	napi_disable(&umac->napi);
+
+	free_irq(ndev->irq, ndev);
+
+	return 0;
+}
+
+static const struct ethtool_ops umac_ethtool_ops = {
+	.get_ts_info	= ethtool_op_get_ts_info,
+	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
+	.set_link_ksettings	= phy_ethtool_set_link_ksettings,
+	.get_drvinfo	= umac_get_drvinfo,
+	.nway_reset	= phy_ethtool_nway_reset,
+	.get_link	= ethtool_op_get_link,
+};
+
+static const struct net_device_ops umac_netdev_ops = {
+	.ndo_open		= umac_open,
+	.ndo_stop		= umac_stop,
+	.ndo_start_xmit		= umac_start_xmit,
+	.ndo_get_stats		= umac_get_stats,
+	.ndo_do_ioctl		= umac_ioctl,
+	.ndo_validate_addr	= eth_validate_addr,
+	.ndo_set_mac_address	= umac_eth_mac_addr,
+};
+
+static int umac_init_mac_address(struct net_device *ndev)
+{
+	struct umac_priv *umac = netdev_priv(ndev);
+	struct platform_device *pdev = umac->pdev;
+	char addr[ETH_ALEN];
+	int err;
+
+	err = of_get_mac_address(pdev->dev.of_node, addr);
+	if (err)
+		netdev_err(ndev, "Failed to get address from device-tree: %d\n",
+			   err);
+		return -EINVAL;
+
+	if (is_valid_ether_addr(addr)) {
+		dev_addr_set(ndev, addr);
+		netdev_dbg(ndev,
+			   "Read MAC address %pM from DTB\n", ndev->dev_addr);
+	} else {
+		netdev_err(ndev, "Mac Address is Invalid");
+		return -EINVAL;
+	}
+
+	dev_addr_set(ndev, addr);
+	umac_set_mac_address(ndev, addr);
+
+	return 0;
+}
+
+static void umac_ncsi_handler(struct ncsi_dev *ncsidev)
+{
+	if (unlikely(ncsidev->state != ncsi_dev_state_functional))
+		return;
+}
+
+static void umac_adjust_link(struct net_device *ndev)
+{
+	struct umac_priv *umac = netdev_priv(ndev);
+	int value;
+
+	if (ndev->phydev->link) {
+		/* disable both clock */
+		value = readl(umac->base + UMAC_CONFIG_STATUS);
+		value &= 0xfffff9ff;
+		writel(value, umac->base + UMAC_CONFIG_STATUS);
+		udelay(2);
+
+		if (ndev->phydev->duplex)
+			value |= UMAC_CFG_FULL_DUPLEX;
+		else
+			value &= ~UMAC_CFG_FULL_DUPLEX;
+
+		switch (ndev->phydev->speed) {
+		case SPEED_1000:
+			value &= ~UMAC_CFG_TX_CLK_EN;
+			value |= UMAC_CFG_GTX_CLK_EN;
+			value |= UMAC_CFG_GIGABIT_MODE;
+			break;
+		case SPEED_100:
+		case SPEED_10:
+			value |= UMAC_CFG_TX_CLK_EN;
+			value &= ~UMAC_CFG_GTX_CLK_EN;
+			value &= ~UMAC_CFG_GIGABIT_MODE;
+			break;
+		}
+		/* update duplex and gigabit_mode to umac */
+		writel(value, umac->base + UMAC_CONFIG_STATUS);
+		udelay(2);
+	} else {
+		/* disable both clock */
+		value = readl(umac->base + UMAC_CONFIG_STATUS);
+		value &= 0xfffff9ff;
+		writel(value, umac->base + UMAC_CONFIG_STATUS);
+		udelay(2);
+
+		value &= ~UMAC_CFG_FULL_DUPLEX;
+		value &= ~UMAC_CFG_GTX_CLK_EN;
+		value &= ~UMAC_CFG_GIGABIT_MODE;
+		value |= UMAC_CFG_TX_CLK_EN;
+		writel(value, umac->base + UMAC_CONFIG_STATUS);
+		udelay(2);
+	}
+}
+
+static struct device_node *gxp_umac_get_eth_child_node(struct device_node *ether_np, int id)
+{
+	struct device_node *port_np;
+	int port_id;
+
+	for_each_child_of_node(ether_np, port_np) {
+		/* It is not a 'port' node, continue. */
+		if (strcmp(port_np->name, "port"))
+			continue;
+		if (of_property_read_u32(port_np, "reg", &port_id) < 0)
+			continue;
+
+		if (port_id == id)
+			return port_np;
+	}
+
+	/* Not found! */
+	return NULL;
+}
+
+static int umac_setup_phy(struct net_device *ndev)
+{
+	struct umac_priv *umac = netdev_priv(ndev);
+	struct platform_device *pdev = umac->pdev;
+	struct device_node *phy_handle;
+	phy_interface_t interface;
+	struct device_node *eth_ports_np;
+	struct device_node *port_np;
+	int ret;
+	int i;
+
+	/* Get child node ethernet-ports. */
+	eth_ports_np = of_get_child_by_name(pdev->dev.of_node, "ethernet-ports");
+	if (!eth_ports_np) {
+		dev_err(&pdev->dev, "No ethernet-ports child node found!\n");
+		return -ENODEV;
+	}
+
+	for (i = 0; i < NUMBER_OF_PORTS; i++) {
+		/* Get port@i of node ethernet-ports */
+		port_np = gxp_umac_get_eth_child_node(eth_ports_np, i);
+		if (!port_np)
+			break;
+
+		if (i == INTERNAL_PORT) {
+			phy_handle = of_parse_phandle(port_np, "phy-handle", 0);
+			if (phy_handle) {
+				umac->int_phy_dev = of_phy_find_device(phy_handle);
+				if (!umac->int_phy_dev)
+					return -ENODEV;
+			} else {
+				return dev_err_probe(&pdev->dev, PTR_ERR(phy_handle),
+						     "Failed to map phy-handle for port %d", i);
+			}
+		}
+
+		if (i == EXTERNAL_PORT) {
+			phy_handle = of_parse_phandle(port_np, "phy-handle", 0);
+			if (phy_handle) {
+				if (ret)
+					dev_err(&pdev->dev, "cannot register phy board fixup\n");
+
+				ret = of_get_phy_mode(phy_handle, &interface);
+				if (ret)
+					interface = PHY_INTERFACE_MODE_NA;
+
+				umac->phy_dev = of_phy_connect(ndev, phy_handle,
+							       &umac_adjust_link,
+							       0, interface);
+
+				if (!umac->phy_dev)
+					return -ENODEV;
+
+				phy_remove_link_mode(umac->phy_dev,
+						     ETHTOOL_LINK_MODE_10baseT_Half_BIT);
+				phy_remove_link_mode(umac->phy_dev,
+						     ETHTOOL_LINK_MODE_10baseT_Full_BIT);
+			} else {
+				return dev_err_probe(&pdev->dev, PTR_ERR(phy_handle),
+						     "Failed to map phy-handle for port %d", i);
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int umac_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct net_device *ndev;
+	struct umac_priv *umac;
+	struct resource *res;
+	int ret;
+
+	ndev = devm_alloc_etherdev(dev, sizeof(*umac));
+	if (!ndev)
+		return -ENOMEM;
+
+	SET_NETDEV_DEV(ndev, &pdev->dev);
+
+	umac = netdev_priv(ndev);
+	umac->pdev = pdev;
+	umac->ndev = ndev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		netdev_err(ndev, "failed to get I/O memory\n");
+		return -ENXIO;
+	}
+
+	umac->base = devm_ioremap_resource(&pdev->dev, res);
+	if (!umac->base) {
+		netdev_err(ndev, "failed to remap I/O memory\n");
+		return -EBUSY;
+	}
+
+	ndev->irq = platform_get_irq(pdev, 0);
+	if (ndev->irq < 0) {
+		netdev_err(ndev, "failed to get irq\n");
+		return -ENXIO;
+	}
+
+	platform_set_drvdata(pdev, ndev);
+
+	ndev->netdev_ops = &umac_netdev_ops;
+	ndev->ethtool_ops = &umac_ethtool_ops;
+
+	umac_init_mac_address(ndev);
+	umac_channel_disable(umac);
+	ret = umac_setup_phy(ndev);
+	if (ret != 0) {
+		netdev_err(ndev, "failed to setup phy ret=%d\n", ret);
+		return -ENODEV;
+	}
+
+	umac->use_ncsi = false;
+	if (of_get_property(pdev->dev.of_node, "use-ncsi", NULL)) {
+		if (!IS_ENABLED(CONFIG_NET_NCSI)) {
+			netdev_err(ndev, "NCSI stack not enabled\n");
+			return 0;
+		}
+
+		dev_info(&pdev->dev, "Using NCSI interface\n");
+		umac->use_ncsi = true;
+		umac->ncsidev = ncsi_register_dev(ndev, umac_ncsi_handler);
+		if (!umac->ncsidev)
+			return -ENODEV;
+	}
+
+	netif_napi_add(ndev, &umac->napi, umac_poll);
+	ret = devm_register_netdev(dev, ndev);
+	if (ret != 0)
+		netdev_err(ndev, "failed to register UMAC ret=%d\n", ret);
+
+	return ret;
+}
+
+static const struct of_device_id umac_of_matches[] = {
+	{ .compatible = "hpe,gxp-umac", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, umac_of_matches);
+
+static struct platform_driver umac_driver = {
+	.driver	= {
+		.name    = "gxp-umac",
+		.of_match_table = umac_of_matches,
+	},
+	.probe   = umac_probe,
+};
+
+module_platform_driver(umac_driver);
+
+MODULE_AUTHOR("Nick Hawkins <nick.hawkins@hpe.com");
+MODULE_DESCRIPTION("HPE GXP UMAC driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/ethernet/hpe/gxp-umac.h b/drivers/net/ethernet/hpe/gxp-umac.h
new file mode 100644
index 000000000000..b8e313cfdcc6
--- /dev/null
+++ b/drivers/net/ethernet/hpe/gxp-umac.h
@@ -0,0 +1,89 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright (C) 2023 Hewlett-Packard Development Company, L.P. */
+
+#ifndef _UMAC_H_
+#define _UMAC_H_
+
+#define UMAC_CONFIG_STATUS	0x00        /* R/W Configuration and Status
+					     * Register I
+					     */
+#define UMAC_CFG_TXEN		0x00001000  // transmit enable
+#define UMAC_CFG_RXEN		0x00000800  // receive enable
+#define UMAC_CFG_GTX_CLK_EN	0x00000400  // gigabit clock enable
+#define UMAC_CFG_TX_CLK_EN	0x00000200  // 10/100 clock enable
+#define UMAC_CFG_GIGABIT_MODE	0x00000004  // MAC gigabit mode enable
+#define UMAC_CFG_FULL_DUPLEX	0x00000001  // enable ignoring of collisions
+#define UMAC_RING_PTR		0x04        //  R/W Ring Pointer Register
+#define UMAC_TX_RING_PTR_MASK	0x7FFF0000  // transmit ring entry pointer
+#define UMAC_TX_RING_PTR_SHIFT	16
+#define UMAC_RX_RING_PTR_MASK	0x00007FFF  // receive ring entry pointer
+#define UMAC_RX_RING_PTR_SHIFT	0
+
+#define UMAC_CLEAR_STATUS	0x0C	//  W   Clear Status Register
+#define UMAC_RING_PROMPT	0x08	//  W	Ring Prompt Register
+#define UMAC_CKSUM_CONFIG	0x10	//  R/W	Checksum Config Register
+#define UMAC_RING_SIZE		0x14	//  R/W	Ring Size Register
+#define UMAC_TX_RING_SIZE_MASK	0xFF000000
+#define UMAC_TX_RING_SIZE_SHIFT	24
+#define UMAC_RX_RING_SIZE_MASK	0x00FF0000
+#define UMAC_RX_RING_SIZE_SHIFT	16
+#define UMAC_LAST_RX_PKT_SIZE	0x0000FFFF
+#define UMAC_RING_SIZE_256	0x3F
+
+#define UMAC_MAC_ADDR_HI	0x18	//  R/W	MAC Address[47:32] Register
+#define UMAC_MAC_ADDR_MID	0x1C	//  R/W	MAC Address[31:16] Register
+#define UMAC_MAC_ADDR_LO	0x20	//  R/W	MAC Address[15:0] Register
+#define UMAC_INTERRUPT		0x30	//  R/W	MAC Interrupt Configuration
+					// and Status Register
+#define UMAC_RX_INTEN		0x00000008
+#define UMAC_RX_INT		0x00000004
+#define UMAC_TX_INTEN		0x00000002
+#define UMAC_TX_INT		0x00000001
+
+#define UMAC_RX_RING_ADDR	0x4C	//  R/W	Rx Ring Base Address Register
+#define UMAC_TX_RING_ADDR	0x50	//  R/W	Tx Ring Base Address Register
+#define UMAC_DMA_CONFIG		0x54	//  R/W	DMA Config Register
+
+#define UMAC_MAX_TX_DESC_ENTRIES	0x100	/* 256,number of ring buffer
+						 *  entries supported
+						 */
+#define UMAC_MAX_RX_DESC_ENTRIES	0x100
+#define UMAC_MAX_TX_FRAME_SIZE		0x600
+#define UMAC_MAX_RX_FRAME_SIZE		0x600
+
+// ring status masks
+#define UMAC_RING_ENTRY_HW_OWN		0x8000
+
+// maximum ethernet frame size
+#define UMAC_MIN_FRAME_SIZE		60    // excludes preable, sfd, and fcs
+#define UMAC_MAX_PAYLOAD_SIZE		1500
+#define UMAC_MAX_FRAME_SIZE		1514  // excludes preable, sfd, and fcs
+
+struct umac_rx_desc_entry {
+	__le32  dmaaddress;   // Start address for DMA operationg
+	u16  status;       // Packet tx status and ownership flag
+	u16  count;        // Number of bytes received
+	u16  checksum;     // On-the-fly packet checksum
+	u16  control;      // Checksum-in-time flag
+	u32  reserved;
+} __aligned(16);
+
+struct umac_rx_descs {
+	struct umac_rx_desc_entry entrylist[UMAC_MAX_RX_DESC_ENTRIES];
+	u8 framelist[UMAC_MAX_RX_DESC_ENTRIES][UMAC_MAX_RX_FRAME_SIZE];
+} __packed;
+
+struct umac_tx_desc_entry {
+	__le32  dmaaddress;   // Start address for DMA operationg
+	u16  status;       // Packet rx status, type, and ownership flag
+	u16  count;        // Number of bytes received
+	u32  cksumoffset;  // Specifies where to place packet checksum
+	u32  reserved;
+} __aligned(16);
+
+struct umac_tx_descs {
+	struct umac_tx_desc_entry entrylist[UMAC_MAX_TX_DESC_ENTRIES];
+	u8 framelist[UMAC_MAX_TX_DESC_ENTRIES][UMAC_MAX_TX_FRAME_SIZE];
+} __packed;
+
+#endif
-- 
2.17.1

[PATCH v3 5/5] MAINTAINERS: HPE: Add GXP UMAC Networking Files

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

List the files added for supporting the UMAC networking on GXP.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v3:
 *No change
v2:
 *Changed dt-binding net directory files to "hpe,gxp*"
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 27ef11624748..c0bb534bec97 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2243,6 +2243,7 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/arm/hpe,gxp.yaml
 F:	Documentation/devicetree/bindings/hwmon/hpe,gxp-fan-ctrl.yaml
 F:	Documentation/devicetree/bindings/i2c/hpe,gxp-i2c.yaml
+F:	Documentation/devicetree/bindings/net/hpe,gxp*
 F:	Documentation/devicetree/bindings/spi/hpe,gxp-spifi.yaml
 F:	Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml
 F:	Documentation/hwmon/gxp-fan-ctrl.rst
@@ -2252,6 +2253,7 @@ F:	arch/arm/mach-hpe/
 F:	drivers/clocksource/timer-gxp.c
 F:	drivers/hwmon/gxp-fan-ctrl.c
 F:	drivers/i2c/busses/i2c-gxp.c
+F:	drivers/net/ethernet/hpe/
 F:	drivers/spi/spi-gxp.c
 F:	drivers/watchdog/gxp-wdt.c
 
-- 
2.17.1

Re: [PATCH v3 0/5] ARM: Add GXP UMAC Support

[Andrew Lunn]

> Changes since v2:
>  *Removed PHY Configuration from MAC driver to Bootloader

By PHY, you mean PCS?

Can you still dynamically swap between 1000BaseX and SGMII?
Is there an interface to determine if the PCS has link?

Is the PHY code upstream in uboot and barebox? Can you detect when it
is missing because the vendor bootloader has been replaced, and report
useful error messages?

       Andrew

Re: [PATCH v3 1/5] dt-bindings: net: Add HPE GXP UMAC MDIO

[Andrew Lunn]

On Wed, Aug 16, 2023 at 04:52:16PM -0500, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Provide access to the register regions and interrupt for Universal
> MAC(UMAC). The driver under the hpe,gxp-umac-mdio will provide an
> interface for managing both the internal and external PHYs.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Your Signed-off-by: should come last.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH v3 2/5] net: hpe: Add GXP UMAC MDIO

[Andrew Lunn]

On Wed, Aug 16, 2023 at 04:52:17PM -0500, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> The GXP contains two Universal Ethernet MACs that can be
> connected externally to several physical devices. From an external
> interface perspective the BMC provides two SERDES interface connections
> capable of either SGMII or 1000Base-X operation. The BMC also provides
> a RMII interface for sideband connections to external Ethernet controllers.
> 
> The primary MAC (umac0) can be mapped to either SGMII/1000-BaseX
> SERDES interface.  The secondary MAC (umac1) can be mapped to only
> the second SGMII/1000-Base X Serdes interface or it can be mapped for
> RMII sideband.
> 
> The MDIO(mdio0) interface from the primary MAC (umac0) is used for
> external PHY status and configuration. The MDIO(mdio1) interface from
> the secondary MAC (umac1) is routed to the SGMII/100Base-X IP blocks

I think that is a typo. 100BaseX does not exist, the nearest is 100BaseFX.

> +config GXP_UMAC_MDIO
> +	tristate "GXP UMAC mdio support"
> +	depends on ARCH_HPE || COMPILE_TEST
> +	depends on OF_MDIO && HAS_IOMEM
> +	depends on MDIO_DEVRES
> +	help
> +	  Say y here to support the GXP UMAC MDIO bus. The
> +	  MDIO (mdio0) interface from the primary MAC (umac0)
> +	  is used for external PHY status and configuration.
> +	  The MDIO (mdio1) interface from the secondary MAC
> +	  (umac1) is routed to the SGMII/100Base-X IP blocks

Same here.

> --- a/drivers/net/mdio/Makefile
> +++ b/drivers/net/mdio/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_MDIO_BCM_UNIMAC)		+= mdio-bcm-unimac.o
>  obj-$(CONFIG_MDIO_BITBANG)		+= mdio-bitbang.o
>  obj-$(CONFIG_MDIO_CAVIUM)		+= mdio-cavium.o
>  obj-$(CONFIG_MDIO_GPIO)			+= mdio-gpio.o
> +obj-$(CONFIG_GXP_UMAC_MDIO)		+= mdio-gxp-umac.o
>  obj-$(CONFIG_MDIO_HISI_FEMAC)		+= mdio-hisi-femac.o

Don't you think this looks out of place. The only one not CONFIG_MDIO ?

> +static int umac_mdio_write(struct mii_bus *bus, int phy_id, int reg, u16 value)
> +{
> +	struct umac_mdio_priv *umac_mdio = bus->priv;
> +	unsigned int status;
> +	int ret;
> +
> +	writel(value, umac_mdio->base + UMAC_MII_DATA);

...

> +	if (ret)
> +		dev_err(bus->parent, "mdio read time out\n");

cut/paste error.


    Andrew

---
pw-bot: cr

Re: [PATCH v3 4/5] net: hpe: Add GXP UMAC Driver

[Andrew Lunn]

> +static int umac_ioctl(struct net_device *ndev, struct ifreq *ifr, int cmd)
> +{
> +	if (!netif_running(ndev))
> +		return -EINVAL;
> +
> +	if (!ndev->phydev)
> +		return -ENODEV;
> +
> +	return phy_mii_ioctl(ndev->phydev, ifr, cmd);

Sometimes power management does not allow it, but can you use phy_do_ioctl()?

> +static int umac_init_hw(struct net_device *ndev)


> +{
> +	} else {
> +		value |= UMAC_CFG_FULL_DUPLEX;
> +
> +		if (ndev->phydev->speed == SPEED_1000) {

I'm pretty sure i pointed this out once before. It is not safe to
access phydev members outside of the adjust link callback.

> +static int umac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> +{
> +	struct umac_priv *umac = netdev_priv(ndev);
> +	struct umac_tx_desc_entry *ptxdesc;
> +	unsigned int length;
> +	u8 *pframe;
> +
> +	ptxdesc = &umac->tx_descs->entrylist[umac->tx_cur];
> +	pframe = umac->tx_descs->framelist[umac->tx_cur];
> +
> +	length = skb->len;
> +	if (length > 1514) {
> +		netdev_err(ndev, "send data %d bytes > 1514, clamp it to 1514\n",
> +			   skb->len);

Than should be rate limited.

Also, if you chop the end of the packet, it is going to be useless. It
is better to drop it, to improve your goodput.

> +		length = 1514;
> +	}
> +
> +	memset(pframe, 0, UMAC_MAX_FRAME_SIZE);
> +	memcpy(pframe, skb->data, length);

Is this cached or uncached memory? uncached is expansive so you want
to avoid touching it twice. Depending on how busy your cache is,
touching it twice might cause it to expelled from L1 on the first
write, so you could be writing to L2 twice for no reason. Do the math
and calculate the tail space you need to zero.

I would also suggest you look at the page pool code and use that for
all you buffer handling. It is likely to be more efficient than what
you have here.

> +static int umac_setup_phy(struct net_device *ndev)
> +{
> +	struct umac_priv *umac = netdev_priv(ndev);
> +	struct platform_device *pdev = umac->pdev;
> +	struct device_node *phy_handle;
> +	phy_interface_t interface;
> +	struct device_node *eth_ports_np;
> +	struct device_node *port_np;
> +	int ret;
> +	int i;
> +
> +	/* Get child node ethernet-ports. */
> +	eth_ports_np = of_get_child_by_name(pdev->dev.of_node, "ethernet-ports");
> +	if (!eth_ports_np) {
> +		dev_err(&pdev->dev, "No ethernet-ports child node found!\n");
> +		return -ENODEV;
> +	}
> +
> +	for (i = 0; i < NUMBER_OF_PORTS; i++) {
> +		/* Get port@i of node ethernet-ports */
> +		port_np = gxp_umac_get_eth_child_node(eth_ports_np, i);
> +		if (!port_np)
> +			break;
> +
> +		if (i == INTERNAL_PORT) {
> +			phy_handle = of_parse_phandle(port_np, "phy-handle", 0);
> +			if (phy_handle) {
> +				umac->int_phy_dev = of_phy_find_device(phy_handle);
> +				if (!umac->int_phy_dev)
> +					return -ENODEV;

You appear to find the PHY, and then do nothing with it?

    Andrew

Re: [PATCH v3 5/5] MAINTAINERS: HPE: Add GXP UMAC Networking Files

[Andrew Lunn]

On Wed, Aug 16, 2023 at 04:52:20PM -0500, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> List the files added for supporting the UMAC networking on GXP.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH v3 4/5] net: hpe: Add GXP UMAC Driver

[Simon Horman]

On Wed, Aug 16, 2023 at 04:52:19PM -0500, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> The GXP contains two Ethernet MACs that can be connected externally
> to several physical devices. From an external interface perspective
> the BMC provides two SERDES interface connections capable of either
> SGMII or 1000Base-X operation. The BMC also provides a RMII interface
> for sideband connections to external Ethernet controllers.
> 
> The primary MAC (umac0) can be mapped to either SGMII/1000-BaseX
> SERDES interface.  The secondary MAC (umac1) can be mapped to only
> the second SGMII/1000-Base X Serdes interface or it can be mapped for
> RMII sideband.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

...

> diff --git a/drivers/net/ethernet/hpe/gxp-umac.c b/drivers/net/ethernet/hpe/gxp-umac.c

...

> +static int umac_init_mac_address(struct net_device *ndev)
> +{
> +	struct umac_priv *umac = netdev_priv(ndev);
> +	struct platform_device *pdev = umac->pdev;
> +	char addr[ETH_ALEN];
> +	int err;
> +
> +	err = of_get_mac_address(pdev->dev.of_node, addr);
> +	if (err)
> +		netdev_err(ndev, "Failed to get address from device-tree: %d\n",
> +			   err);
> +		return -EINVAL;

Hi Nick,

it looks like there should be some {} involved in the condition above,
else the function will return -EINVAL unconditionally.

Flagged by W=1 builds with clang-16 and gcc-13.

> +
> +	if (is_valid_ether_addr(addr)) {
> +		dev_addr_set(ndev, addr);
> +		netdev_dbg(ndev,
> +			   "Read MAC address %pM from DTB\n", ndev->dev_addr);
> +	} else {
> +		netdev_err(ndev, "Mac Address is Invalid");
> +		return -EINVAL;
> +	}
> +
> +	dev_addr_set(ndev, addr);
> +	umac_set_mac_address(ndev, addr);
> +
> +	return 0;
> +}

...

Re: [PATCH v3 3/5] dt-bindings: net: Add HPE GXP UMAC

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 630 bytes --]

On Wed, Aug 16, 2023 at 04:52:18PM -0500, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Provide access to the register regions and interrupt for Universal
> MAC(UMAC). The driver under the hpe,gxp-umac binding will provide an
> interface for sending and receiving networking data from both of the
> UMACs on the system.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> 
> ---
> 
> v3:
>  *Remove MDIO references
>  *Modify description for use-ncsi

Thanks for the description update. This seems good to me, thanks.
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

RE: [PATCH v3 4/5] net: hpe: Add GXP UMAC Driver

[Hawkins, Nick]

Hi Andrew,

> > +static int umac_init_hw(struct net_device *ndev)


> > +{
> > +	} else {
> > +		value |= UMAC_CFG_FULL_DUPLEX;
> > +
> > +		if (ndev->phydev->speed == SPEED_1000) {

> I'm pretty sure i pointed this out once before. It is not safe to
> access phydev members outside of the adjust link callback.

My mistake I missed this phydev member access. I will rely on
the adjust link callback to configure it.

> > +static int umac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> > +{
> > +	struct umac_priv *umac = netdev_priv(ndev);
> > +	struct umac_tx_desc_entry *ptxdesc;
> > +	unsigned int length;
> > +	u8 *pframe;
> > +
> > +	ptxdesc = &umac->tx_descs->entrylist[umac->tx_cur];
> > +	pframe = umac->tx_descs->framelist[umac->tx_cur];
> > +
> > +	length = skb->len;
> > +	if (length > 1514) {
> > +		netdev_err(ndev, "send data %d bytes > 1514, clamp it to 1514\n",
> > +			   skb->len);

> Than should be rate limited.

How is this done? Is there a particular function to call that
will handle it in the backend?
I thought the rate limiting is happening in this
function already below at:

/* if current tx ring buffer is full, stop the queue */
        ptxdesc = &umac->tx_descs->entrylist[umac->tx_cur];
        if (ptxdesc->status & UMAC_RING_ENTRY_HW_OWN)
                netif_stop_queue(ndev);

> Also, if you chop the end of the packet, it is going to be useless. It
> is better to drop it, to improve your goodput.

I will drop it.

Thanks,

-Nick Hawkins

Re: [PATCH v3 4/5] net: hpe: Add GXP UMAC Driver

[Andrew Lunn]

> > > +	if (length > 1514) {
> > > +		netdev_err(ndev, "send data %d bytes > 1514, clamp it to 1514\n",
> > > +			   skb->len);
> 
> > Than should be rate limited.
> 
> How is this done? Is there a particular function to call that
> will handle it in the backend?

Sorry, i was ambiguous. I meant the netdev_err() should be rate
limited, otherwise some user space application could DOS your system
by sending big packets at line rate flooding your logs.

   Andrew

Re: [PATCH v3 4/5] net: hpe: Add GXP UMAC Driver

[Hawkins, Nick]

Hi Andrew,

> > + length = 1514;
> > + }
> > +
> > + memset(pframe, 0, UMAC_MAX_FRAME_SIZE);
> > + memcpy(pframe, skb->data, length);


> Is this cached or uncached memory? uncached is expansive so you want
> to avoid touching it twice. Depending on how busy your cache is,
> touching it twice might cause it to expelled from L1 on the first
> write, so you could be writing to L2 twice for no reason. Do the math
> and calculate the tail space you need to zero.

> I would also suggest you look at the page pool code and use that for
> all you buffer handling. It is likely to be more efficient than what
> you have here.

Would this be the #include <linux/dmapool.h> library?

Thank you for the assistance,

-Nick Hawkins

Re: [PATCH v3 4/5] net: hpe: Add GXP UMAC Driver

[Andrew Lunn]

> Would this be the #include <linux/dmapool.h> library?

<include/net/page_pool/helpers.h>

Take a look at driver/net/ethernet/freescale/fec_main.c That
driver/device is of similar complexity to yours. It had a recent
change from its own buffer management to page pool. It
started with

commit 95698ff6177b5f1f13f251da60e7348413046ae4
Author: Shenwei Wang <shenwei.wang@nxp.com>
Date:   Fri Sep 30 15:44:27 2022 -0500

    net: fec: using page pool to manage RX buffers

but there are additional patches later.

    Andrew

Re: [PATCH v3 4/5] net: hpe: Add GXP UMAC Driver

[Hawkins, Nick]

> <include/net/page_pool/helpers.h>

Hi Andrew,

I can't seem to find this file in linux master. Where is it?

> Take a look at driver/net/ethernet/freescale/fec_main.c That
> driver/device is of similar complexity to yours. It had a recent
> change from its own buffer management to page pool. It
> started with

I have looked over this driver and have a couple questions
about the pages in general.

How do I determine what the correct pool size should be for the
RX and TX?

I must admit I am not familiar with XDP.
Is it required for the page pool library? Does it require any kind
of hardware enablement? Or is it just purely code?

Thanks for the feedback and assistance,

-Nick Hawkins

Re: [PATCH v3 4/5] net: hpe: Add GXP UMAC Driver

[Andrew Lunn]

On Tue, Aug 22, 2023 at 07:00:49PM +0000, Hawkins, Nick wrote:
> 
> > <include/net/page_pool/helpers.h>
> 
> Hi Andrew,
> 
> I can't seem to find this file in linux master. Where is it?

~/linux$ ls include/net/page_pool/helpers.h
include/net/page_pool/helpers.h

When you say master, do you mean net-next/main? This is a network
driver, so you should be based on top of that tree.

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#netdev-faq

> > Take a look at driver/net/ethernet/freescale/fec_main.c That
> > driver/device is of similar complexity to yours. It had a recent
> > change from its own buffer management to page pool. It
> > started with
> 
> I have looked over this driver and have a couple questions
> about the pages in general.
> 
> How do I determine what the correct pool size should be for the
> RX and TX?

There has been some recent discussion about that. Search the netdev
list over the last couple of week. 

> I must admit I am not familiar with XDP.
> Is it required for the page pool library?

Nope, not required at all. The FEC driver was first converted to page
pool, and then XDP support added. The conversion to page pool made the
driver faster, it could handle more packets per second. That is why i
suggested using it, plus it means less driver code, which means less
bugs.

	Andrew

RE: [PATCH v3 4/5] net: hpe: Add GXP UMAC Driver

[Hawkins, Nick]

Hi Andrew,

Thank you for pointing me at the correct repo to use. I was using the
incorrect one.

> Nope, not required at all. The FEC driver was first converted to page
> pool, and then XDP support added. The conversion to page pool made the
> driver faster, it could handle more packets per second. That is why i
> suggested using it, plus it means less driver code, which means less
> bugs.

I have been trying to figure out how exactly I can translate the current code
over to using the page pool api over the past week. It seems like it is quiet
a complex change. As the driver seems to be keeping up with our
performance requirements would it be acceptable to mark this as a
TODO: for a future enhancement?

Thank you for the assistance,

-Nick Hawkins

Re: [PATCH v3 4/5] net: hpe: Add GXP UMAC Driver

[Andrew Lunn]

> I have been trying to figure out how exactly I can translate the current code
> over to using the page pool api over the past week. It seems like it is quiet
> a complex change. As the driver seems to be keeping up with our
> performance requirements would it be acceptable to mark this as a

Its not just performance. Using well debugged and shared core code
means less bugs in your driver. It makes maintenance simpler since
there are more people who understand the page pool code than what you
have in your driver and it makes your driver more like other drivers.

So overall you will end up with a better quality driver by adapting
the page pool code.

    Andrew

Re: [PATCH v3 4/5] net: hpe: Add GXP UMAC Driver

[Hawkins, Nick]

> > I have been trying to figure out how exactly I can translate the current code
> > over to using the page pool api over the past week. It seems like it is quiet
> > a complex change. As the driver seems to be keeping up with our
> > performance requirements would it be acceptable to mark this as a


> Its not just performance. Using well debugged and shared core code
> means less bugs in your driver. It makes maintenance simpler since
> there are more people who understand the page pool code than what you
> have in your driver and it makes your driver more like other drivers.


> So overall you will end up with a better quality driver by adapting
> the page pool code.

Greetings Andrew,

In that case I will continue to attempt to try and adopt the page pool
API. In all the examples with page pool HW rings it appears they are
using alloc_etherdev_mqs. Are there any HW requirements to use this
library? If there are none what is the typical number for rx and tx
queues?

Thank you for the assistance,

-Nick Hawkins

Re: [PATCH v3 4/5] net: hpe: Add GXP UMAC Driver

[Andrew Lunn]

> Greetings Andrew,
> 
> In that case I will continue to attempt to try and adopt the page pool
> API. In all the examples with page pool HW rings it appears they are
> using alloc_etherdev_mqs. Are there any HW requirements to use this
> library? If there are none what is the typical number for rx and tx
> queues?

There are no hardware requirements as far as i understand it. If your
hardware only has one RX queue and one TX queue, define it as
1. Having more allows you to spread the load over multiple CPUs, with
each queue typically having its own interrupt, and interrupts are then
mapped to a single CPU. But if you don't have any of that, it should
not be a hindrance.

    Andrew