On Tue, Jul 15, 2014 at 12:48 AM, Tobias Klauser [off-list ref] wrote:

On 2014-07-15 at 00:18:05 +0200, Iyappan Subramanian [off-list ref] wrote:

quoted

This patch adds network driver for APM X-Gene SoC ethernet.

Signed-off-by: Iyappan Subramanian <redacted>
Signed-off-by: Ravi Patel <redacted>
Signed-off-by: Keyur Chudgar <redacted>
---
 drivers/net/ethernet/Kconfig                       |   1 +
 drivers/net/ethernet/Makefile                      |   1 +
 drivers/net/ethernet/apm/Kconfig                   |   1 +
 drivers/net/ethernet/apm/Makefile                  |   5 +
 drivers/net/ethernet/apm/xgene/Kconfig             |   9 +
 drivers/net/ethernet/apm/xgene/Makefile            |   6 +
 .../net/ethernet/apm/xgene/xgene_enet_ethtool.c    | 125 +++
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.c     | 747 ++++++++++++++++
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.h     | 375 ++++++++
 drivers/net/ethernet/apm/xgene/xgene_enet_main.c   | 962 +++++++++++++++++++++
 drivers/net/ethernet/apm/xgene/xgene_enet_main.h   | 107 +++
 11 files changed, 2339 insertions(+)
 create mode 100644 drivers/net/ethernet/apm/Kconfig
 create mode 100644 drivers/net/ethernet/apm/Makefile
 create mode 100644 drivers/net/ethernet/apm/xgene/Kconfig
 create mode 100644 drivers/net/ethernet/apm/xgene/Makefile
 create mode 100644 drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
 create mode 100644 drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
 create mode 100644 drivers/net/ethernet/apm/xgene/xgene_enet_hw.h
 create mode 100644 drivers/net/ethernet/apm/xgene/xgene_enet_main.c
 create mode 100644 drivers/net/ethernet/apm/xgene/xgene_enet_main.h

[...]

quoted

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
new file mode 100644
index 0000000..e52af60
--- /dev/null
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c

@@ -0,0 +1,747 @@

[...]

quoted

+int xgene_enet_mdio_config(struct xgene_enet_pdata *pdata)
+{
+     struct net_device *ndev = pdata->ndev;
+     struct device *dev = &pdata->pdev->dev;
+     struct device_node *child_np;
+     struct device_node *mdio_np = NULL;
+     struct mii_bus *mdio_bus;
+     int ret;
+
+     for_each_child_of_node(dev->of_node, child_np) {
+             if (of_device_is_compatible(child_np, "apm,xgene-mdio")) {
+                     mdio_np = child_np;
+                     break;
+             }
+     }
+
+     if (!mdio_np) {
+             netdev_dbg(ndev, "No mdio node in the dts\n");
+             return -1;
+     }
+
+     mdio_bus = mdiobus_alloc();
+     if (!mdio_bus)
+             return -ENOMEM;
+
+     mdio_bus->name = "APM X-Gene MDIO bus";
+     mdio_bus->read = xgene_enet_mdio_read;
+     mdio_bus->write = xgene_enet_mdio_write;
+     snprintf(mdio_bus->id, MII_BUS_ID_SIZE, "%s-%s", "xgene-mii",
+              ndev->name);
+
+     mdio_bus->priv = pdata;
+     mdio_bus->parent = &ndev->dev;
+
+     ret = of_mdiobus_register(mdio_bus, mdio_np);
+     if (ret) {
+             netdev_err(ndev, "Failed to register MDIO bus\n");
+             goto err;
+     }
+     pdata->mdio_bus = mdio_bus;
+
+     ret = xgene_enet_phy_connect(ndev);
+     if (ret)

mdiobus_unregister() should be called on this error path.

Thanks.  I will fix this.

quoted

+             goto err;
+
+     return ret;
+
+err:
+     mdiobus_free(mdio_bus);
+
+     return ret;
+}

[...]

quoted

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
new file mode 100644
index 0000000..756523a
--- /dev/null
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c

[...]

quoted

+static struct rtnl_link_stats64 *xgene_enet_get_stats64(
+                     struct net_device *ndev,
+                     struct rtnl_link_stats64 *storage)
+{
+     struct xgene_enet_pdata *pdata = netdev_priv(ndev);
+     struct rtnl_link_stats64 *stats = &pdata->stats;
+
+     spin_lock(&pdata->stats_lock);

What is the purpose of this spinlock? It doesn't seem to protect any
hardware-specific stats readout. Is it really necessary?

I agree and will remove the spin lock.

quoted

+     stats->rx_errors += stats->rx_length_errors +
+                         stats->rx_crc_errors +
+                         stats->rx_frame_errors +
+                         stats->rx_fifo_errors;
+     memcpy(storage, &pdata->stats, sizeof(struct rtnl_link_stats64));
+     spin_unlock(&pdata->stats_lock);
+
+     return storage;
+}

[...]

quoted

+static int xgene_enet_init_hw(struct xgene_enet_pdata *pdata)
+{
+     struct net_device *ndev = pdata->ndev;
+     struct xgene_enet_desc_ring *buf_pool;
+     u16 dst_ring_num;
+     int ret;
+
+     xgene_gmac_tx_disable(pdata);
+     xgene_gmac_rx_disable(pdata);
+
+     ret = xgene_enet_create_desc_rings(ndev);
+     if (ret) {
+             netdev_err(ndev, "Error in ring configuration\n");
+             return ret;
+     }
+
+     /* setup buffer pool */
+     buf_pool = pdata->rx_ring->buf_pool;
+     xgene_enet_init_bufpool(buf_pool);
+     ret = xgene_enet_refill_bufpool(buf_pool, pdata->rx_buff_cnt);
+     if (ret)

Shouldn't xgene_enet_delete_desc_rings() be called here?

I will fix this.

quoted

+             return ret;
+
+     dst_ring_num = xgene_enet_dst_ring_num(pdata->rx_ring);
+     xgene_enet_cle_bypass(pdata, dst_ring_num, buf_pool->id);
+
+     return ret;
+}
+
+static int xgene_enet_probe(struct platform_device *pdev)
+{
+     struct net_device *ndev;
+     struct xgene_enet_pdata *pdata;
+     struct device *dev = &pdev->dev;
+     struct napi_struct *napi;
+     int ret;
+
+     ndev = alloc_etherdev(sizeof(struct xgene_enet_pdata));
+     if (!ndev)
+             return -ENOMEM;
+
+     pdata = netdev_priv(ndev);
+
+     pdata->pdev = pdev;
+     pdata->ndev = ndev;
+     SET_NETDEV_DEV(ndev, dev);
+     platform_set_drvdata(pdev, pdata);
+     ndev->netdev_ops = &xgene_ndev_ops;
+     xgene_enet_set_ethtool_ops(ndev);
+     ndev->features |= NETIF_F_IP_CSUM |
+                       NETIF_F_GSO |
+                       NETIF_F_GRO;
+
+     ret = xgene_enet_get_resources(pdata);
+     if (ret)
+             goto err;
+
+     xgene_enet_reset(pdata);
+     xgene_gmac_init(pdata, SPEED_1000);
+
+     spin_lock_init(&pdata->stats_lock);
+     ret = register_netdev(ndev);
+     if (ret) {
+             netdev_err(ndev, "Failed to register netdev\n");
+             goto err;
+     }
+
+     ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+     if (ret) {
+             netdev_err(ndev, "No usable DMA configuration\n");
+             goto err;
+     }
+
+     ret = xgene_enet_init_hw(pdata);
+     if (ret)
+             goto err;
+
+     napi = &pdata->rx_ring->napi;
+     netif_napi_add(ndev, napi, xgene_enet_napi, NAPI_POLL_WEIGHT);
+     ret = xgene_enet_mdio_config(pdata);
+
+     return ret;
+err:
+     free_netdev(ndev);
+     return ret;
+}
+
+static int xgene_enet_remove(struct platform_device *pdev)
+{
+     struct xgene_enet_pdata *pdata;
+     struct net_device *ndev;
+
+     pdata = platform_get_drvdata(pdev);
+     ndev = pdata->ndev;
+
+     xgene_gmac_rx_disable(pdata);
+     xgene_gmac_tx_disable(pdata);
+
+     netif_napi_del(&pdata->rx_ring->napi);
+     xgene_enet_mdio_remove(pdata);
+     xgene_enet_delete_desc_rings(pdata);
+     unregister_netdev(ndev);
+     xgene_gport_shutdown(pdata);
+     free_netdev(ndev);
+
+     return 0;
+}
+
+static struct of_device_id xgene_enet_match[] = {
+     {.compatible = "apm,xgene-enet",},
+     {},
+};
+
+MODULE_DEVICE_TABLE(of, xgene_enet_match);
+
+static struct platform_driver xgene_enet_driver = {
+     .driver = {
+                .name = "xgene-enet",
+                .owner = THIS_MODULE,
+                .of_match_table = xgene_enet_match,
+                },
+     .probe = xgene_enet_probe,
+     .remove = xgene_enet_remove,
+};
+
+module_platform_driver(xgene_enet_driver);
+
+MODULE_DESCRIPTION("APM X-Gene SoC Ethernet driver");
+MODULE_VERSION(XGENE_DRV_VERSION);
+MODULE_AUTHOR("Keyur Chudgar [off-list ref]");
+MODULE_LICENSE("GPL");