[v2,2/5] net: hpe: Add GXP UMAC MDIO

Message ID 20230802201824.3683-3-nick.hawkins@hpe.com
State New
Headers
Series ARM: Add GXP UMAC Support |

Commit Message

Hawkins, Nick Aug. 2, 2023, 8:18 p.m. UTC
  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>

---

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/ethernet/Kconfig     |   1 +
 drivers/net/ethernet/Makefile    |   1 +
 drivers/net/mdio/Kconfig         |  13 +++
 drivers/net/mdio/Makefile        |   1 +
 drivers/net/mdio/mdio-gxp-umac.c | 142 +++++++++++++++++++++++++++++++
 5 files changed, 158 insertions(+)
 create mode 100644 drivers/net/mdio/mdio-gxp-umac.c
  

Comments

Andrew Lunn Aug. 2, 2023, 10:39 p.m. UTC | #1
> +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

nitpick: help should be indented same as depends.

> +	  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.

Is it an external MDIO bus? So anything could be connected to it,
e.g. an Ethernet switch. 

> +	  The MDIO(mdio1) interface from the secondary MAC
> +	  (umac1) is routed to the SGMII/100Base-X IP blocks
> +	  on the two SERDES interface connections.

Is this one purely internal? If so, then the text is valid. If it also
goes external, there is no reason you could not put a PHY or an
Ethernet switch on it. You can then skip using the first MDIO bus all
together.

> +#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         0x00000200
> +#define UMAC_MII_MRNW           0x00000100

Are these two bits? If so, please use BIT().

> +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 = __raw_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 */
> +	__raw_writel(status, umac_mdio->base + UMAC_MII);
> +
> +	status |= UMAC_MII_MOWNER; /* set bit to activate mii transfer */
> +	__raw_writel(status, umac_mdio->base + UMAC_MII);

I assume UMAC_MII_MOWNER must be set in a separate operation? But
using __raw_writel() i'm not sure there is any barrier between the two
writes.

	Andrew
  
Randy Dunlap Aug. 2, 2023, 10:42 p.m. UTC | #2
Hi Nick,

On 8/2/23 13:18, nick.hawkins@hpe.com wrote:
> diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
> index 9ff2e6f22f3f..58e054bff786 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

Indent line above with one tab only.

> +	  Say y here to support the GXP UMAC MDIO bus. The
> +	  MDIO(mdio0) interface from the primary MAC (umac0)

Consistent spacing:
	  MDIO (mdio0)

> +	  is used for external PHY status and configuration.
> +	  The MDIO(mdio1) interface from the secondary MAC

	      MDIO (mdio1)

> +	  (umac1) is routed to the SGMII/100Base-X IP blocks
> +	  on the two SERDES interface connections.

thanks.
  
Simon Horman Aug. 3, 2023, 11:39 a.m. UTC | #3
On Wed, Aug 02, 2023 at 03:18:21PM -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
> on the two SERDES interface connections.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

...

> 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"

Hi Nick,

I think this hunk belongs in [PATCH v2 4/5] net: hpe: Add GXP UMAC Driver.
As it is that patch where drivers/net/ethernet/hpe/Kconfig is added.
And as things stands, the above caused a build failure.
  

Patch

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..2e3cae9dbe97 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -44,6 +44,7 @@  obj-$(CONFIG_NET_VENDOR_FREESCALE) += freescale/
 obj-$(CONFIG_NET_VENDOR_FUJITSU) += fujitsu/
 obj-$(CONFIG_NET_VENDOR_FUNGIBLE) += fungible/
 obj-$(CONFIG_NET_VENDOR_GOOGLE) += google/
+obj-$(CONFIG_NET_VENDOR_HPE) += hpe/
 obj-$(CONFIG_NET_VENDOR_HISILICON) += hisilicon/
 obj-$(CONFIG_NET_VENDOR_HUAWEI) += huawei/
 obj-$(CONFIG_NET_VENDOR_IBM) += ibm/
diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
index 9ff2e6f22f3f..58e054bff786 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..ddce19a7bb1f
--- /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         0x00000200
+#define UMAC_MII_MRNW           0x00000100
+#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 = __raw_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 */
+	__raw_writel(status, umac_mdio->base + UMAC_MII);
+
+	status |= UMAC_MII_MOWNER; /* set bit to activate mii transfer */
+	__raw_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 = __raw_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;
+
+	__raw_writel(value, umac_mdio->base + UMAC_MII_DATA);
+
+	status = __raw_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 */
+	__raw_writel(status, umac_mdio->base + UMAC_MII);
+
+	status |= UMAC_MII_MOWNER; /* set bit to activate mii transfer */
+	__raw_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");