Re: [PATCH v5 2/2] Ethernet driver for the WIZnet W5100 chip
From: Mark Brown <hidden>
Date: 2012-03-31 21:23:09
Also in:
lkml
On Fri, Mar 30, 2012 at 01:00:06PM +0600, Mike Sinkovsky wrote:
quoted hunk ↗ jump to hunk
+config WIZNET_W5100 + tristate "WIZnet W5100 Ethernet support" + depends on ARM || BLACKFIN
What are the architecture dependencies here?
quoted hunk ↗ jump to hunk
+static irqreturn_t w5100_interrupt(int irq, void *ndev_instance) +{ + struct net_device *ndev = ndev_instance; + struct w5100_priv *priv = netdev_priv(ndev); + + int ir = w5100_read(priv, W5100_S0_IR); + w5100_write(priv, W5100_S0_IR, ir); + + if (ir & S0_IR_RECV) { + if (napi_schedule_prep(&priv->napi)) { + w5100_write(priv, W5100_IMR, 0); + mmiowb(); + __napi_schedule(&priv->napi); + } + } + + if (ir & S0_IR_SENDOK) { + if (unlikely(netif_queue_stopped(ndev))) + netif_wake_queue(ndev); + } + + return IRQ_HANDLED;
This unconditionally acknowledges the interrupt even if one wasn't reported by the device.
quoted hunk ↗ jump to hunk
+ irq = platform_get_irq(pdev, 0); + if (irq < 0) + return irq; + ret = devm_request_irq(&pdev->dev, irq, w5100_interrupt, + IRQ_TYPE_LEVEL_LOW, name, ndev); + if (ret < 0) + return ret; + priv->irq = irq; + + link = platform_get_resource(pdev, IORESOURCE_IO, 0); + if (!link) { + priv->link_gpio = -1;
This is rather an abuse of the resource API and will run into trouble on device tree based systems. You should use platform data for non-DT systems.
quoted hunk ↗ jump to hunk
+ if (request_irq(priv->link_irq, w5100_detect_link, + IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING, + link_name, priv->ndev) < 0)
Suggest using request_any_context_irq() to increase the range of supported interrupt controllers.
quoted hunk ↗ jump to hunk
+err_register: + free_netdev(ndev); + platform_set_drvdata(pdev, NULL); + dev_info(&pdev->dev, "probe failed (%d)\n", err);
This will be done for you by the driver core.