[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1447029127.8727.29.camel@kernel.crashing.org>
Date: Mon, 09 Nov 2015 11:32:07 +1100
From: Benjamin Herrenschmidt <benh@...nel.crashing.org>
To: Gavin Shan <gwshan@...ux.vnet.ibm.com>, netdev@...r.kernel.org
Cc: davem@...emloft.net, sergei.shtylyov@...entembedded.com
Subject: Re: [PATCH RFC 5/6] net/faraday: Enable NCSI interface
On Mon, 2015-11-09 at 11:10 +1100, Gavin Shan wrote:
>
> - if (likely(netif_running(netdev))) {
> + /* When running in NCSI mode, the interface should be
> + * ready to receive or transmit NCSI packet before it's
> + * opened.
> + */
No, that's not right. open/close is when a driver allocates it's data
structures, DMA ring descriptors, packet buffers etc... and request
it's interrupts.
You cannot expect packets to flow on a closed interface and you
shouldn't be getting any interrupts on a closed interface either.
> + if (likely(priv->use_ncsi || netif_running(netdev))) {
> /* Disable interrupts for polling */
> iowrite32(0, priv->base + FTGMAC100_OFFSET_IER);
> napi_schedule(&priv->napi);
> @@ -1162,8 +1168,18 @@ static int ftgmac100_open(struct net_device
> *netdev)
>
> /* enable all interrupts */
> iowrite32(INT_MASK_ALL_ENABLED, priv->base +
> FTGMAC100_OFFSET_IER);
> + /* Start the NCSI device */
> + if (priv->use_ncsi){
> + err = ncsi_start_dev(priv->ndev);
> + if (err)
> + goto err_ncsi;
> + }
> return 0;
>
> +err_ncsi:
> + napi_disable(&priv->napi);
> + netif_stop_queue(netdev);
> + iowrite32(0, priv->base + FTGMAC100_OFFSET_IER);
> err_hw:
> free_irq(priv->irq, netdev);
> err_irq:
> @@ -1172,7 +1188,7 @@ err_alloc:
> return err;
> }
>
> -static int ftgmac100_stop(struct net_device *netdev)
> +static int ftgmac100_stop_dev(struct net_device *netdev)
> {
> struct ftgmac100 *priv = netdev_priv(netdev);
>
> @@ -1191,6 +1207,18 @@ static int ftgmac100_stop(struct net_device
> *netdev)
> return 0;
> }
>
> +static int ftgmac100_stop(struct net_device *netdev)
> +{
> + struct ftgmac100 *priv = netdev_priv(netdev);
> +
> + /* Stop NCSI device */
> + if (priv->use_ncsi) {
> + ncsi_stop_dev(priv->ndev);
> + return 0;
> + }
> +
> + return ftgmac100_stop_dev(netdev);
> +}
> static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
> struct net_device *netdev)
> {
> @@ -1291,6 +1319,21 @@ static const struct net_device_ops
> ftgmac100_netdev_ops = {
> .ndo_do_ioctl = ftgmac100_do_ioctl,
> };
>
> +static void ftgmac100_ncsi_handler(struct ncsi_dev *nd)
> +{
> + struct net_device *netdev = nd->nd_dev;
> +
> + if (nd->nd_state != ncsi_dev_state_functional)
> + return;
> +
> + if (nd->nd_link_up) {
> + pr_info("NCSI dev is up\n");
> + netif_start_queue(netdev);
> + } else {
> + pr_info("NCSI dev is down\n");
> + ftgmac100_stop_dev(netdev);
> + }
> +}
> /*******************************************************************
> ***********
> * struct platform_driver functions
>
> *********************************************************************
> ********/
> @@ -1300,7 +1343,7 @@ static int ftgmac100_probe(struct
> platform_device *pdev)
> int irq;
> struct net_device *netdev;
> struct ftgmac100 *priv;
> - int err;
> + int err = 0;
>
> if (!pdev)
> return -ENODEV;
> @@ -1320,7 +1363,17 @@ static int ftgmac100_probe(struct
> platform_device *pdev)
> goto err_alloc_etherdev;
> }
>
> + /* Check for NCSI mode */
> + priv = netdev_priv(netdev);
> SET_NETDEV_DEV(netdev, &pdev->dev);
> + if (pdev->dev.of_node &&
> + of_get_property(pdev->dev.of_node, "use-nc-si", NULL)) {
> + dev_info(&pdev->dev, "Using NCSI interface\n");
> + priv->phydev = NULL;
> + priv->use_ncsi = true;
> + } else {
> + priv->use_ncsi = false;
> + }
>
> netdev->ethtool_ops = &ftgmac100_ethtool_ops;
> netdev->netdev_ops = &ftgmac100_netdev_ops;
> @@ -1329,7 +1382,6 @@ static int ftgmac100_probe(struct
> platform_device *pdev)
> platform_set_drvdata(pdev, netdev);
>
> /* setup private data */
> - priv = netdev_priv(netdev);
> priv->netdev = netdev;
> priv->dev = &pdev->dev;
>
> @@ -1356,24 +1408,14 @@ static int ftgmac100_probe(struct
> platform_device *pdev)
>
> priv->irq = irq;
>
> - /* Check for NC-SI mode */
> - if (pdev->dev.of_node &&
> - of_get_property(pdev->dev.of_node, "use-nc-si", NULL))
> - priv->use_ncsi = true;
> - else
> - priv->use_ncsi = false;
> + /* Read MAC address or setup a new one */
> + ftgmac100_setup_mac(priv);
>
> - /* If we use NC-SI, we need to set that up, which isn't
> implemented yet
> - * so we pray things were setup by the bootloader and just
> mark our link
> - * as up (otherwise we can't get packets through).
> - *
> - * Eventually, we'll have a proper NC-SI stack as a helper
> we can
> - * instanciate
> - */
> + /* Register NCSI device */
> if (priv->use_ncsi) {
> - /* XXX */
> - priv->phydev = NULL;
> - dev_info(&pdev->dev, "Using NC-SI interface\n");
> + priv->ndev = ncsi_register_dev(netdev,
> ftgmac100_ncsi_handler);
> + if (!priv->ndev)
> + goto err_ncsi_dev;
> } else {
> err = ftgmac100_setup_mdio(priv);
>
> @@ -1384,9 +1426,6 @@ static int ftgmac100_probe(struct
> platform_device *pdev)
> dev_warn(&pdev->dev, "Error %d setting up
> MDIO\n", err);
> }
>
> - /* Read MAC address or setup a new one */
> - ftgmac100_setup_mac(priv);
> -
> /* Register network device */
> err = register_netdev(netdev);
> if (err) {
> @@ -1399,7 +1438,11 @@ static int ftgmac100_probe(struct
> platform_device *pdev)
> return 0;
>
> err_register_netdev:
> - ftgmac100_destroy_mdio(priv);
> + if (!priv->use_ncsi)
> + ftgmac100_destroy_mdio(priv);
> + else
> + ncsi_unregister_dev(priv->ndev);
> +err_ncsi_dev:
> iounmap(priv->base);
> err_ioremap:
> release_resource(priv->res);
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists