lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 7 Mar 2017 17:22:26 -0800 From: Florian Fainelli <f.fainelli@...il.com> To: Iyappan Subramanian <isubramanian@....com>, davem@...emloft.net, netdev@...r.kernel.org, andrew@...n.ch, David.Laight@...lab.com Cc: linux-arm-kernel@...ts.infradead.org, patches@....com, kchudgar@....com Subject: Re: [PATCH v4 net-next 4/6] drivers: net: xgene-v2: Add base driver On 03/07/2017 05:08 PM, Iyappan Subramanian wrote: > This patch adds, > > - probe, remove, shutdown > - open, close and stats > - create and delete ring > - request and delete irq > > Signed-off-by: Iyappan Subramanian <isubramanian@....com> > Signed-off-by: Keyur Chudgar <kchudgar@....com> > --- > + pdata->resources.phy_mode = phy_mode; > + > + if (pdata->resources.phy_mode != PHY_INTERFACE_MODE_RGMII) { > + dev_err(dev, "Incorrect phy-connection-type specified\n"); > + return -ENODEV; > + } This does not take into account all other PHY_INTERFACE_MODE_RGMII variants, is that really intentional here? > + > + ret = platform_get_irq(pdev, 0); > + if (ret <= 0) { 0 can be a valid interrupt on some platforms AFAIR, so you may want to just check < 0. > + dev_err(dev, "Unable to get ENET IRQ\n"); > + ret = ret ? : -ENXIO; > + return ret; > + } > + pdata->resources.irq = ret; > + > +static int xge_request_irq(struct net_device *ndev) > +{ > + struct xge_pdata *pdata = netdev_priv(ndev); > + struct device *dev = &pdata->pdev->dev; > + int ret; > + > + snprintf(pdata->irq_name, IRQ_ID_SIZE, "%s", ndev->name); > + > + ret = devm_request_irq(dev, pdata->resources.irq, xge_irq, > + 0, pdata->irq_name, pdata); > + if (ret) > + netdev_err(ndev, "Failed to request irq %s\n", pdata->irq_name); The preference for network driver is to manage the request_irq() in the ndo_open() callback and free_irq() in the ndo_close() which kind of defeats the purpose of using devm_* functions for that purpose. > +static int xge_open(struct net_device *ndev) > +{ > + struct xge_pdata *pdata = netdev_priv(ndev); > + int ret; > + > + ret = xge_create_desc_rings(ndev); > + if (ret) > + return ret; > + > + napi_enable(&pdata->napi); > + ret = xge_request_irq(ndev); > + if (ret) > + return ret; > + > + xge_intr_enable(pdata); > + xge_wr_csr(pdata, DMARXCTRL, 1); > + xge_mac_enable(pdata); > + netif_start_queue(ndev); > + netif_carrier_on(ndev); Can't you use PHYLIB to get the link indication and not manage the link state manually here? Setting netif_carrier_on() without checking the actualy physical medium is just plain wrong/ > +static void xge_timeout(struct net_device *ndev) > +{ > + struct xge_pdata *pdata = netdev_priv(ndev); > + > + rtnl_lock(); > + > + if (netif_running(ndev)) { Reduce indention here. > + netif_carrier_off(ndev); > + netif_stop_queue(ndev); > + xge_intr_disable(pdata); > + napi_disable(&pdata->napi); > + > +static int xge_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct net_device *ndev; > + struct xge_pdata *pdata; > + int ret; > + > + ndev = alloc_etherdev(sizeof(struct xge_pdata)); sizeof(*pdata). -- Florian
Powered by blists - more mailing lists