On Mon, Nov 09, 2015 at 11:32:07AM +1100, Benjamin Herrenschmidt wrote:
>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.
>
Yeah, It's something that I hilighed in the cover letter. I was
thinking we might need a better way to enable Tx/Rx before the
interrupt is up, but couldn't figure out one way. So I need some
advice here.
Thanks,
Gavin
>> + 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
