Skip to contentSkip to conversation

dt-bindings: net: Add bindings for National Instruments XGE netdev

PATCH Superseded Moritz Fischer 4 participants 5 messages Feb 16, 2018 – Feb 20, 2018

Subsystems (3)

v3 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

PATCH v3 1/2 dt-bindings: net: Add bindings for National Instruments XGE netdev

This adds bindings for the NI XGE 1G/10G network device.

---

Changes from v2:
- Addressed Rob's comments w.r.t to IRQ names and typo

Changes from v1:
- Corrected from nixge -> nixge.txt

---
 Documentation/devicetree/bindings/net/nixge.txt | 32 +++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/nixge.txt
Documentation/devicetree/bindings/net/nixge.txt new file +32 -0
new file mode 100644
index 000000000000..e55af7f0881a
@@ -0,0 +1,32 @@
+* NI XGE Ethernet controller
+
+Required properties:
+- compatible: Should be "ni,xge-enet-2.00"
+- reg: Address and length of the register set for the device
+- interrupts: Should contain tx and rx interrupt
+- interrupt-names: Should be "rx" and "tx"
+- phy-mode: See ethernet.txt file in the same directory.
+- phy-handle: See ethernet.txt file in the same directory.
+- nvmem-cells: Phandle of nvmem cell containing the MAC address
+- nvmem-cell-names: Should be "address"
+
+Examples (10G generic PHY):
+	nixge0: ethernet@40000000 {
+		compatible = "ni,xge-enet-2.00";
+		reg = <0x40000000 0x6000>;
+
+		nvmem-cells = <&eth1_addr>;
+		nvmem-cell-names = "address";
+
+		interrupts = <0 29 IRQ_TYPE_LEVEL_HIGH>, <0 30 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "rx", "tx";
+		interrupt-parent = <&intc>;
+
+		phy-mode = "xgmii";
+		phy-handle = <&ethernet_phy1>;
+
+		ethernet_phy1: ethernet-phy@4 {
+			compatible = "ethernet-phy-ieee802.3-c45";
+			reg = <4>;
+		};
+	};
Signature · 1 line
2.16.1
#1 lore raw

PATCH v3 2/2 net: ethernet: nixge: Add support for National Instruments XGE netdev

Add support for the National Instruments XGE 1/10G network device.

It uses the EEPROM on the board via NVMEM.

---

Changes from v2:
- Implement recv side NAPI
- Improved error handling
- Implemented C45 writes
- Added ethtool callbacks & blink functionality
- Improved nixge_ctrl_poll_timeout() macro
- Removed dev_dbg() for mdio accesses
- Added businfo to ethtool drvinfo

Changes from v1:
- Added dependency on ARCH_ZYNQ (Kbuild)
- Removed unused variables
- Use of_phy_connect as suggested
- Removed masking of (un)supported modes
- Added #define for some constants
- Removed empty pm functions
- Reworked mac_address handling
- Made nixge_mdio_*() static (sparse)
- Removed driver version
- Addressed timeout loop
- Adressed return values on timeout

---
 drivers/net/ethernet/Kconfig     |    1 +
 drivers/net/ethernet/Makefile    |    1 +
 drivers/net/ethernet/ni/Kconfig  |   27 +
 drivers/net/ethernet/ni/Makefile |    1 +
 drivers/net/ethernet/ni/nixge.c  | 1352 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 1382 insertions(+)
 create mode 100644 drivers/net/ethernet/ni/Kconfig
 create mode 100644 drivers/net/ethernet/ni/Makefile
 create mode 100644 drivers/net/ethernet/ni/nixge.c
drivers/net/ethernet/Kconfig +1 -0
index b6cf4b6962f5..908218561fdd 100644
@@ -129,6 +129,7 @@ config FEALNX
 
 source "drivers/net/ethernet/natsemi/Kconfig"
 source "drivers/net/ethernet/netronome/Kconfig"
+source "drivers/net/ethernet/ni/Kconfig"
 source "drivers/net/ethernet/8390/Kconfig"
 
 config NET_NETX
drivers/net/ethernet/Makefile +1 -0
index 3cdf01e96e0b..d732e9522b76 100644
@@ -61,6 +61,7 @@ obj-$(CONFIG_NET_VENDOR_MYRI) += myricom/
 obj-$(CONFIG_FEALNX) += fealnx.o
 obj-$(CONFIG_NET_VENDOR_NATSEMI) += natsemi/
 obj-$(CONFIG_NET_VENDOR_NETRONOME) += netronome/
+obj-$(CONFIG_NET_VENDOR_NI) += ni/
 obj-$(CONFIG_NET_NETX) += netx-eth.o
 obj-$(CONFIG_NET_VENDOR_NUVOTON) += nuvoton/
 obj-$(CONFIG_NET_VENDOR_NVIDIA) += nvidia/
drivers/net/ethernet/ni/Kconfig new file +27 -0
new file mode 100644
index 000000000000..cd30f7de16de
@@ -0,0 +1,27 @@
+#
+# National Instuments network device configuration
+#
+
+config NET_VENDOR_NI
+	bool "National Instruments Devices"
+	default y
+	---help---
+	  If you have a network (Ethernet) device belonging to this class, say Y.
+
+	  Note that the answer to this question doesn't directly affect the
+	  kernel: saying N will just cause the configurator to skip all
+	  the questions about National Instrument devices.
+	  If you say Y, you will be asked for your specific device in the
+	  following questions.
+
+if NET_VENDOR_NI
+
+config NI_XGE_MANAGEMENT_ENET
+	tristate "National Instruments XGE management enet support"
+	depends on ARCH_ZYNQ
+	select PHYLIB
+	---help---
+	  Simple LAN device for debug or management purposes. Can
+	  support either 10G or 1G PHYs via SFP+ ports.
+
+endif
drivers/net/ethernet/ni/Makefile new file +1 -0
new file mode 100644
index 000000000000..99c664651c51
@@ -0,0 +1 @@
+obj-$(CONFIG_NI_XGE_MANAGEMENT_ENET) += nixge.o
drivers/net/ethernet/ni/nixge.c new file +1352 -0

1,352 lines not on this page · raw message

Signature · 1 line
2.16.1
#2 lore raw

Re: [PATCH v3 2/2] net: ethernet: nixge: Add support for National Instruments XGE netdev

drivers/net/ethernet/ni/nixge.c line 119

Quoted 9 lines · Moritz Fischer
drivers/net/ethernet/ni/nixge.c new file +9 -0
+#define NIXGE_MDIO_CLAUSE45	BIT(12)
+#define NIXGE_MDIO_CLAUSE22	0
+#define NIXGE_MDIO_OP(n)     (((n) & 0x3) << 10)
+#define NIXGE_MDIO_OP_ADDRESS	0
+#define NIXGE_MDIO_OP_WRITE	BIT(0)
+#define NIXGE_MDIO_OP_READ	(BIT(1) | BIT(0))
+#define MDIO_C22_WRITE		BIT(0)
+#define MDIO_C22_READ		BIT(1)
+#define MDIO_READ_POST		2

Hi Moritz

NIXGE prefix?

drivers/net/ethernet/ni/nixge.c line 123

Quoted 4 lines
drivers/net/ethernet/ni/nixge.c new file +4 -0
+#define NIXGE_MDIO_ADDR(n)   (((n) & 0x1f) << 5)
+#define NIXGE_MDIO_MMD(n)    (((n) & 0x1f) << 0)
+
+#define NIXGE_MAX_PHY_ADDR	32

This is nothing specific to this device. Please use the standard PHY_MAX_ADDR.

Quoted 30 lines
drivers/net/ethernet/ni/nixge.c new file +30 -0
+static int nixge_open(struct net_device *ndev)
+{
+	struct nixge_priv *priv = netdev_priv(ndev);
+	struct phy_device *phy;
+	int ret;
+
+	nixge_device_reset(ndev);
+
+	phy = of_phy_connect(ndev, priv->phy_node,
+			     &nixge_handle_link_change, 0, priv->phy_mode);
+	if (!phy)
+		return -ENODEV;
+
+	phy_start(phy);
+
+	/* Enable tasklets for Axi DMA error handling */
+	tasklet_init(&priv->dma_err_tasklet, nixge_dma_err_handler,
+		     (unsigned long)priv);
+
+	napi_enable(&priv->napi);
+
+	/* Enable interrupts for Axi DMA Tx */
+	ret = request_irq(priv->tx_irq, nixge_tx_irq, 0, ndev->name, ndev);
+	if (ret)
+		goto err_tx_irq;
+	/* Enable interrupts for Axi DMA Rx */
+	ret = request_irq(priv->rx_irq, nixge_rx_irq, 0, ndev->name, ndev);
+	if (ret)
+		goto err_rx_irq;
+

drivers/net/ethernet/ni/nixge.c line 888

Quoted 10 lines
drivers/net/ethernet/ni/nixge.c new file +10 -0
+	netif_start_queue(ndev);
+
+	return 0;
+
+err_rx_irq:
+	free_irq(priv->tx_irq, ndev);
+err_tx_irq:
+	tasklet_kill(&priv->dma_err_tasklet);
+	netdev_err(ndev, "request_irq() failed\n");
+	return ret;

Maybe you also want to stop the phy and disconnect it?

drivers/net/ethernet/ni/nixge.c line 900

Quoted 10 lines
drivers/net/ethernet/ni/nixge.c new file +10 -0
+static int nixge_stop(struct net_device *ndev)
+{
+	struct nixge_priv *priv = netdev_priv(ndev);
+	u32 cr;
+
+	netif_stop_queue(ndev);
+	napi_disable(&priv->napi);
+
+	if (ndev->phydev)
+		phy_stop(ndev->phydev);

phy_disconnect() ?

Quoted 39 lines
drivers/net/ethernet/ni/nixge.c new file +38 -0
+	cr = nixge_dma_read_reg(priv, XAXIDMA_RX_CR_OFFSET);
+	nixge_dma_write_reg(priv, XAXIDMA_RX_CR_OFFSET,
+			    cr & (~XAXIDMA_CR_RUNSTOP_MASK));
+	cr = nixge_dma_read_reg(priv, XAXIDMA_TX_CR_OFFSET);
+	nixge_dma_write_reg(priv, XAXIDMA_TX_CR_OFFSET,
+			    cr & (~XAXIDMA_CR_RUNSTOP_MASK));
+
+	tasklet_kill(&priv->dma_err_tasklet);
+
+	free_irq(priv->tx_irq, ndev);
+	free_irq(priv->rx_irq, ndev);
+
+	nixge_hw_dma_bd_release(ndev);
+
+	return 0;
+}
 
+static int nixge_mdio_read(struct mii_bus *bus, int phy_id, int reg)
+{
+	struct nixge_priv *priv = bus->priv;
+	u32 status, tmp;
+	int err;
+	u16 device;
+
+	if (reg & MII_ADDR_C45) {
+		device = (reg >> 16) & 0x1f;
+
+		nixge_ctrl_write_reg(priv, NIXGE_REG_MDIO_ADDR, reg & 0xffff);
+
+		tmp = NIXGE_MDIO_CLAUSE45 | NIXGE_MDIO_OP(NIXGE_MDIO_OP_ADDRESS)
+			| NIXGE_MDIO_ADDR(phy_id) | NIXGE_MDIO_MMD(device);
+
+		nixge_ctrl_write_reg(priv, NIXGE_REG_MDIO_OP, tmp);
+		nixge_ctrl_write_reg(priv, NIXGE_REG_MDIO_CTRL, 1);
+
+		err = nixge_ctrl_poll_timeout(priv, NIXGE_REG_MDIO_CTRL, status,
+					      !status, 10, 1000);
+		if (err) {
+			dev_err(priv->dev, "timeout setting address");

drivers/net/ethernet/ni/nixge.c line 1106

Quoted 10 lines
drivers/net/ethernet/ni/nixge.c new file +10 -0
+			return err;
+		}
+
+		tmp = NIXGE_MDIO_CLAUSE45 | NIXGE_MDIO_OP(NIXGE_MDIO_OP_READ) |
+			NIXGE_MDIO_ADDR(phy_id) | NIXGE_MDIO_MMD(device);
+	} else {
+		device = reg & 0x1f;
+
+		tmp = NIXGE_MDIO_CLAUSE22 | NIXGE_MDIO_OP(MDIO_C22_READ) |
+			NIXGE_MDIO_ADDR(phy_id) | NIXGE_MDIO_MMD(device);

It would be nice to have some naming consistency here. NIXGE_MDIO_C45_READ and NIXGE_MDIO_C22_READ?

Quoted 13 lines
drivers/net/ethernet/ni/nixge.c new file +13 -0
+	}
+
+	nixge_ctrl_write_reg(priv, NIXGE_REG_MDIO_OP, tmp);
+	nixge_ctrl_write_reg(priv, NIXGE_REG_MDIO_CTRL, 1);
+
+	err = nixge_ctrl_poll_timeout(priv, NIXGE_REG_MDIO_CTRL, status,
+				      !status, 10, 1000);
+	if (err) {
+		dev_err(priv->dev, "timeout setting read command");
+		return err;
+	}
+
+	status = nixge_ctrl_read_reg(priv, NIXGE_REG_MDIO_DATA);

drivers/net/ethernet/ni/nixge.c line 1182

Quoted 10 lines
drivers/net/ethernet/ni/nixge.c new file +10 -0
+
+	return status;
+}
+
+static int nixge_mdio_setup(struct nixge_priv *priv, struct device_node *np)
+{
+	struct mii_bus *bus;
+	int err;
+
+	bus = mdiobus_alloc();

devm_mdiobus_alloc() ?

Quoted 21 lines
drivers/net/ethernet/ni/nixge.c new file +21 -0
+	if (!bus)
+		return -ENOMEM;
+
+	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(priv->dev));
+	bus->priv = priv;
+	bus->name = "nixge_mii_bus";
+	bus->read = nixge_mdio_read;
+	bus->write = nixge_mdio_write;
+	bus->parent = priv->dev;
+
+	priv->mii_bus = bus;
+	err = of_mdiobus_register(bus, np);
+	if (err)
+		goto err_register;
+
+	return 0;
+
+err_register:
+	mdiobus_free(bus);
+	return err;
+}

drivers/net/ethernet/ni/nixge.c

Quoted 10 lines
drivers/net/ethernet/ni/nixge.c new file +9 -0
 
+static int nixge_remove(struct platform_device *pdev)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct nixge_priv *priv = netdev_priv(ndev);
+
+	mdiobus_unregister(priv->mii_bus);
+	mdiobus_free(priv->mii_bus);
+
+	unregister_netdev(ndev);

I'm not sure about the ordering here. It might be better to unregister the netdev, then destroy the MDIO bus.

drivers/net/ethernet/ni/nixge.c line 1331

Quoted 5 lines
drivers/net/ethernet/ni/nixge.c new file +5 -0
+
+	free_netdev(ndev);
+
+	return 0;
+}

Andrew

#3 lore raw

Re: [PATCH v3 2/2] net: ethernet: nixge: Add support for National Instruments XGE netdev

In reply to Moritz Fischer

Hi Moritz,

Quoted 5 lines · Moritz Fischer

Add support for the National Instruments XGE 1/10G network device.

It uses the EEPROM on the board via NVMEM.

<snip>

Quoted 4 lines
drivers/net/ethernet/ni/nixge.c new file +1 -0
@@ -0,0 +1,1352 @@
+// SPDX-License-Identifier: GPL-2.0

<snip>

Quoted 1 line

+MODULE_LICENSE("GPL");

This does not match your license above. Per module.h "GPL" would mean "GPL-2.0+" Can you use one or the other an\d ensure both of these are in sync?

Signature · 2 lines
Cordially
Philippe Ombredanne
maintainer Reviewed-by on v3 1/2
#4 lore raw
In reply to Moritz Fischer
Quoted 8 lines · Moritz Fischer

This adds bindings for the NI XGE 1G/10G network device.

---

Changes from v2:
- Addressed Rob's comments w.r.t to IRQ names and typo

Quoted 7 lines

Changes from v1:
- Corrected from nixge -> nixge.txt

---
 Documentation/devicetree/bindings/net/nixge.txt | 32 +++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/nixge.txt