Micrel KS8695 intergrated ethernet driver
Subsystems (2)
PATCH 1/1 Micrel KS8695 intergrated ethernet driver
Hello,
Attached is a patch which implements the Micrel KS8695 SoC's Ethernet interfaces.
The driver is only of use on the KS8695.
Regards,
Daniel.
Signature · 2 lines
Daniel Silverstone http://www.simtec.co.uk/ PGP mail accepted and encouraged. Key Id: 2BC8 4016 2068 7895
KS8695: Add support for the KS8695 ethernet devices.
Implements the KS8695 ethernet device (ks8695net).
drivers/net/arm/Kconfig +8 -0
@@ -40,6 +40,14 @@ If you wish to compile a kernel for the AT91RM9200 and enable ethernet support, then you should always answer Y to this. +config ARM_KS8695_ETHER + tristate "KS8695 Ethernet support" + depends on ARM && ARCH_KS8695 + select MII + help + If you wish to compile a kernel for the KS8695 and want to + use the internal ethernet then you should answer Y to this. + config EP93XX_ETH tristate "EP93xx Ethernet support" depends on ARM && ARCH_EP93XX
drivers/net/arm/Makefile +1 -0
@@ -8,5 +8,6 @@ obj-$(CONFIG_ARM_ETHER3) += ether3.o obj-$(CONFIG_ARM_ETHER1) += ether1.o obj-$(CONFIG_ARM_AT91_ETHER) += at91_ether.o +obj-$(CONFIG_ARM_KS8695_ETHER) += ks8695net.o obj-$(CONFIG_EP93XX_ETH) += ep93xx_eth.o obj-$(CONFIG_IXP4XX_ETH) += ixp4xx_eth.o
drivers/net/arm/ks8695net.c new file +1661 -0
1,661 lines not on this page · raw message
drivers/net/arm/ks8695net.h new file +107 -0
107 lines not on this page · raw message
drivers/net/arm/Kconfig 2008-12-05 11:30:15.357075120 +0000
Quoted 18 lines · Daniel Silverstone
Hello,
Attached is a patch which implements the Micrel KS8695 SoC's Ethernet interfaces.
The driver is only of use on the KS8695.
KS8695: Add support for the KS8695 ethernet devices.
Implements the KS8695 ethernet device (ks8695net).
Signed-off-by Daniel Silverstone <[email protected]>Signed-off-by Vincent Sanders <[email protected]>Acked-by Ben Dooks <[email protected]>Index: linux-2.6.27/drivers/net/arm/Kconfig
===================================================================drivers/net/arm/Kconfig 2008-12-05 11:30:15.357075120 +0000 renamed from drivers/net/arm/Kconfig 2008-12-05 11:29:29.629076651 +0000 +0
-0
netdev patches should really be based on David Miller's net-next-2.6.
Quoted 4 lines
Index:
linux-2.6.27/drivers/net/arm/ks8695net.c
===================================================================drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +0
-0
[...]
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 11 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +11
-0+/** + * enum ks8695_dtype - Device type + * @KS8695_DTYPE_WAN: This device is a WAN interface + * @KS8695_DTYPE_LAN: This device is a LAN interface + * @KS8695_DTYPE_HPNA: This device is an HPNA interfacew + */ +enum ks8695_dtype { + KS8695_DTYPE_WAN, + KS8695_DTYPE_LAN, + KS8695_DTYPE_HPNA, +};
The enumerators should be indented with tabs.
Quoted 5 lines
+/**
+ * struct ks8695_priv - Private data for the KS8695 Ethernet + * @in_suspend: Flag to indicate if we're suspending/resuming + * @ndev: The net_device for this interface + * @dev: The device object for this interface
Which device object - the platform device?
[...]
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 11 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +11
-0+/** + * ks8695_get_priv - Retrieve the private data from a net_device. + * @ndev: The net_device to read the private data pointer from + * + * Returns the private structure for the KS8695 Ethernet device. + */ +static inline struct ks8695_priv * +ks8695_get_priv(struct net_device *ndev) +{ + return ndev->priv; +}
Don't use ndev->priv, use netdev_priv(ndev). Which makes this function redundant, except for the implicit pointer conversion.
[...]
Quoted 21 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +21
-0+/** + * ks8695_refill_rxbuffers - Re-fill the RX buffer ring + * @ksp: The device to refill + * + * Iterates the RX ring of the device looking for empty slots. + * For each empty slot, we allocate and map a new SKB and give it + * to the hardware. + * This can be called from interrupt context safely. + */ +static void +ks8695_refill_rxbuffers(struct ks8695_priv *ksp) +{ + /* Run around the RX ring, filling in any missing sk_buff's */ + int buff_n; + + for (buff_n = 0; buff_n < MAX_RX_DESC; ++buff_n) { + if (!ksp->rx_buffers[buff_n].skb) { + struct sk_buff *skb = dev_alloc_skb(MAX_RXBUF_SIZE); + dma_addr_t mapping; + + ksp->rx_buffers[buff_n].skb = skb;
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 10 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10
-0+ if (skb == NULL) { + /* Failed to allocate one, perhaps + * we'll try again later. + */ + break; + } + + mapping = dma_map_single(ksp->dev, skb->data, + MAX_RXBUF_SIZE, + DMA_FROM_DEVICE);
I take it that dma_map_single() can never fail on ARM?
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 8 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +8
-0+ ksp->rx_buffers[buff_n].dma_ptr = mapping; + skb->dev = ksp->ndev; + ksp->rx_buffers[buff_n].length = MAX_RXBUF_SIZE; + + /* Record this into the DMA ring */ + ksp->rx_ring[buff_n].data_ptr = cpu_to_le32(mapping); + ksp->rx_ring[buff_n].length = + cpu_to_le32(MAX_RXBUF_SIZE);
You need a wmb() in here, otherwise the descriptor might not be complete when the hardware sees RDES_OWN set.
Quoted 5 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +5
-0+ /* And give ownership over to the hardware */ + ksp->rx_ring[buff_n].status = cpu_to_le32(RDES_OWN); + } + } +}
[...]
Quoted 12 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +12
-0+ +/* Interrupt handling */ + +/** + * ks8695_tx_irq - Transmit IRQ handler + * @irq: The IRQ which went off (ignored) + * @dev_id: The net_device for the interrupt + * + * Process the TX ring, clearing out any transmitted slots. + * Allows the net_device to pass us new packets once slots are + * freed. + */
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 10 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10
-0+static irqreturn_t +ks8695_tx_irq(int irq, void *dev_id) +{ + struct net_device *ndev = (struct net_device *)dev_id; + struct ks8695_priv *ksp = ks8695_get_priv(ndev); + int buff_n; + + for (buff_n = 0; buff_n < MAX_TX_DESC; ++buff_n) { + if (ksp->tx_buffers[buff_n].skb && + !(ksp->tx_ring[buff_n].owner & cpu_to_le32(TDES_OWN))) {
rmb()
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 7 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +7
-0+ /* An SKB which is not owned by HW is present */ + /* Update the stats for the net_device */ + ndev->stats.tx_packets++; + ndev->stats.tx_bytes += ksp->tx_buffers[buff_n].length; + + /* Free the packet from the ring */ + ksp->tx_ring[buff_n].data_ptr = 0;
Clearing data_ptr seems redundant. The TDES_OWN flag indicates whether the descriptor is pending and the skb pointer indicates whether a buffer needs to be freed.
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 10 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10
-0+ /* Free the sk_buff */ + dma_unmap_single(ksp->dev, + ksp->tx_buffers[buff_n].dma_ptr, + ksp->tx_buffers[buff_n].length, + DMA_TO_DEVICE); + dev_kfree_skb_irq(ksp->tx_buffers[buff_n].skb); + ksp->tx_buffers[buff_n].skb = NULL; + ksp->tx_ring_used--; + } + }
Do you need to test all the descriptors or can you keep track of which to check first and then stop when you find one still pending? Maybe it doesn't matter when you only have 8 of them...
Quoted 19 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +19
-0+ netif_wake_queue(ndev); + + return IRQ_HANDLED; +} + +/** + * ks8695_rx_irq - Receive IRQ handler + * @irq: The IRQ which went off (ignored) + * @dev_id: The net_device for the interrupt + * + * Process the RX ring, passing any received packets up to the + * host. If we received anything other than errors, we then + * refill the ring. + */ +static irqreturn_t +ks8695_rx_irq(int irq, void *dev_id) +{ + struct net_device *ndev = (struct net_device *)dev_id; + struct ks8695_priv *ksp = ks8695_get_priv(ndev);
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 10 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10
-0+ struct sk_buff *skb; + int buff_n; + u32 flags; + int pktlen; + int last_rx_processed = -1; + + buff_n = ksp->next_rx_desc_read; + do { + if (ksp->rx_buffers[buff_n].skb && + !(ksp->rx_ring[buff_n].status & cpu_to_le32(RDES_OWN))) {
rmb()
Quoted 23 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +23
-0+ flags = le32_to_cpu(ksp->rx_ring[buff_n].status); + /* Found an SKB which we own, this means we + * received a packet + */ + if ((flags & (RDES_FS | RDES_LS)) != + (RDES_FS | RDES_LS)) { + /* This packet is not the first and + * the last segment. Therefore it is + * a "spanning" packet and we can't + * handle it + */ + goto rx_failure; + } + + if (flags & (RDES_ES | RDES_RE)) { + /* It's an error packet */ + ndev->stats.rx_errors++; + if (flags & RDES_TL) + ndev->stats.rx_length_errors++; + if (flags & RDES_RF) + ndev->stats.rx_length_errors++; + if (flags & RDES_CE) + ndev->stats.rx_crc_errors++;
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 10 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10
-0+ if (flags & RDES_RE) + ndev->stats.rx_missed_errors++; + + goto rx_failure; + } + + pktlen = flags & RDES_FLEN; + pktlen -= 4; /* Drop the CRC */ + + skb = ksp->rx_buffers[buff_n].skb;
I think this line belongs after the following comment.
Quoted 27 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +26
-0+ /* Extract the sk_buff from the ring */ + ksp->rx_buffers[buff_n].skb = NULL; + ksp->rx_ring[buff_n].data_ptr = 0; + /* Unmap the SKB */ + dma_unmap_single(ksp->dev, + ksp->rx_buffers[buff_n].dma_ptr, + ksp->rx_buffers[buff_n].length, + DMA_FROM_DEVICE); + + /* Relinquish the SKB to the network layer */ + skb_put(skb, pktlen); + skb->protocol = eth_type_trans(skb, ndev); + netif_rx(skb); + + /* Record stats */ + ndev->last_rx = jiffies; + ndev->stats.rx_packets++; + ndev->stats.rx_bytes += pktlen; + goto rx_finished; + +rx_failure: + /* This ring entry is an error, but we can + * re-use the skb + */ + /* Give the ring entry back to the hardware */ + ksp->rx_ring[buff_n].status = cpu_to_le32(RDES_OWN);
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 10 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10
-0+rx_finished: + /* And note this as processed so we can start + * from here next time + */ + last_rx_processed = buff_n; + } else { + /* Ran out of things to process, stop now */ + break; + } + buff_n = (buff_n + 1) % MAX_RX_DESC;
That's another expensive division.
Quoted 14 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +14
-0+ } while (buff_n != ksp->next_rx_desc_read); + + /* And note which RX descriptor we last did anything with */ + if (likely(last_rx_processed != -1)) + ksp->next_rx_desc_read = (last_rx_processed + 1) % MAX_RX_DESC; + + /* And refill the buffers */ + ks8695_refill_rxbuffers(ksp); + + /* Kick the RX DMA engine, in case it became suspended */ + ks8695_writereg(ksp, KS8695_DRSC, 0); + + return IRQ_HANDLED; +}
[...]
Quoted 11 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +11
-0+ +/* KS8695 Device functions */ + +/** + * ks8695_reset - Reset a KS8695 ethernet interface + * @ksp: The interface to reset + * + * Perform an engine reset of the interface and re-program it + * with sensible defaults. + */ +static void
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 10 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10
-0+ks8695_reset(struct ks8695_priv *ksp) +{ + int reset_timeout = watchdog; + /* Issue the reset via the TX DMA control register */ + ks8695_writereg(ksp, KS8695_DTXC, DTXC_TRST); + while (reset_timeout--) { + if (!(ks8695_readreg(ksp, KS8695_DTXC) & DTXC_TRST)) + break; + msleep(1); + }
Is there any reason this timeout should be tied to the TX watchdog timeout?
[...]
Quoted 25 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +25
-0+/** + * ks8695_init_net - Initialise a KS8695 ethernet interface + * @ksp: The interface to initialise + * + * This routine fills the RX ring, initialises the DMA engines, + * allocates the IRQs and then starts the packet TX and RX + * engines. + */ +static int +ks8695_init_net(struct ks8695_priv *ksp) +{ + int ret; + u32 ctrl; + + ks8695_refill_rxbuffers(ksp); + + /* Initialise the DMA engines */ + ks8695_writereg(ksp, KS8695_RDLB, (u32) ksp->rx_ring_dma); + ks8695_writereg(ksp, KS8695_TDLB, (u32) ksp->tx_ring_dma); + + /* Request the IRQs */ + ret = ks8695_setup_irq(ksp->rx_irq, ksp->rx_irq_name, + ks8695_rx_irq, ksp->ndev); + if (ret) + return ret;
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 10 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10
-0+ ret = ks8695_setup_irq(ksp->tx_irq, ksp->tx_irq_name, + ks8695_tx_irq, ksp->ndev); + if (ret) + return ret; + if (ksp->link_irq != -1) { + ret = ks8695_setup_irq(ksp->link_irq, ksp->link_irq_name, + ks8695_link_irq, ksp->ndev); + if (ret) + return ret; + }
The RX and TX IRQ handlers need to be released in case the TX or link IRQ setup fails.
Quoted 57 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +56
-0+ /* Set up the ring indices */ + ksp->next_rx_desc_read = 0; + ksp->tx_ring_next_slot = 0; + ksp->tx_ring_used = 0; + + /* Bring up transmission */ + ctrl = ks8695_readreg(ksp, KS8695_DTXC); + /* Enable packet transmission */ + ks8695_writereg(ksp, KS8695_DTXC, ctrl | DTXC_TE); + + /* Bring up the reception */ + ctrl = ks8695_readreg(ksp, KS8695_DRXC); + /* Enable packet reception */ + ks8695_writereg(ksp, KS8695_DRXC, ctrl | DRXC_RE); + /* And start the DMA engine */ + ks8695_writereg(ksp, KS8695_DRSC, 0); + + /* All done */ + return 0; +} + +/** + * ks8695_release_device - HW resource release for KS8695 e-net + * @ksp: The device to be freed + * + * This unallocates io memory regions, dma-coherent regions etc + * which were allocated in ks8695_probe. + */ +static void +ks8695_release_device(struct ks8695_priv *ksp) +{ + /* Unmap the registers */ + iounmap(ksp->io_regs); + if (ksp->phyiface_regs) + iounmap(ksp->phyiface_regs); + + /* And release the request */ + release_resource(ksp->regs_req); + kfree(ksp->regs_req); + if (ksp->phyiface_req) { + release_resource(ksp->phyiface_req); + kfree(ksp->phyiface_req); + } + + /* Free the ring buffers */ + dma_free_coherent(ksp->dev, RING_DMA_SIZE, + ksp->ring_base, ksp->ring_base_dma); +} +/** + * ks8695_get_settings - Get device-specific settings. + * @ndev: The network device to read settings from + * @cmd: The ethtool structure to read into + */ +static int +ks8695_get_settings(struct net_device *ndev, struct ethtool_cmd *cmd) +{
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 10 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10
-0+ struct ks8695_priv *ksp = ks8695_get_priv(ndev); + u32 ctrl; + + /* All ports on the KS8695 support these... */ + cmd->supported = (SUPPORTED_10baseT_Half | SUPPORTED_10baseT_Full | + SUPPORTED_100baseT_Half | SUPPORTED_100baseT_Full | + SUPPORTED_TP | SUPPORTED_MII); + cmd->advertising = ADVERTISED_TP | ADVERTISED_MII; + cmd->port = PORT_MII; + cmd->transceiver = XCVR_INTERNAL;
I think the TP and MII flags should be dependent on ksp->dtype.
Quoted 14 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +14
-0+ /* Port specific extras */ + switch (ksp->dtype) { + case KS8695_DTYPE_HPNA: + cmd->phy_address = 0; + /* not supported for HPNA */ + cmd->autoneg = AUTONEG_DISABLE; + + /* BUG: Erm, dtype hpna implies no phy regs */ + /* + ctrl = readl(KS8695_MISC_VA + KS8695_HMC); + cmd->speed = (ctrl & HMC_HSS) ? SPEED_100 : SPEED_10; + cmd->duplex = (ctrl & HMC_HDS) ? DUPLEX_FULL : DUPLEX_HALF; + */ + break;
[...]
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 4 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +4
-0+ case KS8695_DTYPE_LAN: + /* TODO: Implement this for the switch ports */ + break; + }
You'd better implement these or return -EOPNOTSUPP. Similarly in ks8695_set_settings() and ks8695_nwayreset().
Quoted 2 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +2
-0+ return 0; +}
[...]
Quoted 6 lines
+/**
+ * ks8695_set_pause - Configure pause/flow-control + * @ndev: The device to configure + * @param: The pause parameters to set+ *
+ * TODO: Implement this
Yes, or return -EOPNOTSUPP.
Quoted 6 lines
+ */
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +5
-0+static int +ks8695_set_pause(struct net_device *ndev, struct ethtool_pauseparam *param) +{ + return 0; +}
[...]
Quoted 15 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +15
-0+/** + * ks8695_timeout - Handle a network tx/rx timeout. + * @ndev: The net_device which timed out. + * + * A network transaction timed out, reset the device. + */ +static void +ks8695_timeout(struct net_device *ndev) +{ + struct ks8695_priv *ksp = ks8695_get_priv(ndev); + + netif_stop_queue(ndev); + ks8695_shutdown(ksp); + + ks8695_reset(ksp);
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 10 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10
-0+ + ks8695_update_mac(ksp); + + /* We ignore the return from this since it managed to init + * before it probably will be okay to init again. + */ + ks8695_init_net(ksp); + + netif_start_queue(ndev); +}
It looks like this might lose multicast settings.
Quoted 11 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +11
-0+/** + * ks8695_start_xmit - Start a packet transmission + * @skb: The packet to transmit + * @ndev: The network device to send the packet on + * + * This routine, called by the net layer, takes ownership of the + * sk_buff and adds it to the TX ring. It then kicks the TX DMA + * engine to ensure transmission begins. + */ +static int +ks8695_start_xmit(struct sk_buff *skb, struct net_device *ndev)
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 10 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10
-0+{ + struct ks8695_priv *ksp = ks8695_get_priv(ndev); + int buff_n; + + spin_lock_irq(&ksp->txq_lock); + + if (ksp->tx_ring_used == MAX_TX_DESC) { + /* Somehow we got entered when we have no room */ + spin_unlock_irq(&ksp->txq_lock); + return -ENOSPC;
You must return NETDEV_TX_OK or NETDEV_TX_BUSY, not 0 or a negative error code.
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 4 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +4
-0+ } + + buff_n = ksp->tx_ring_next_slot; + ksp->tx_ring_next_slot = (buff_n + 1) % MAX_TX_DESC;
That's an expensive division, which could be avoided by either (1) declaring buff_n unsigned or (2) explicitly masking.
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 4 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +4
-0+ if (ksp->tx_buffers[buff_n].skb) { + /* This slot is used, try later */ + spin_unlock_irq(&ksp->txq_lock); + return -EAGAIN;
Since you already tested tx_ring_used, wouldn't this indicate a bug?
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 13 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +13
-0+ } + + ksp->tx_buffers[buff_n].skb = skb; + ksp->tx_buffers[buff_n].length = skb->len; + ksp->tx_buffers[buff_n].dma_ptr = dma_map_single( + ksp->dev, skb->data, skb->len, DMA_TO_DEVICE); + + /* Fill out the TX descriptor */ + ksp->tx_ring[buff_n].data_ptr = + cpu_to_le32(ksp->tx_buffers[buff_n].dma_ptr); + ksp->tx_ring[buff_n].status = + cpu_to_le32(TDES_IC | TDES_FS | TDES_LS | + (skb->len & TDES_TBS));
wmb()
Quoted 2 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +2
-0+ /* Hand it over to the hardware */ + ksp->tx_ring[buff_n].owner = cpu_to_le32(TDES_OWN);
[...]
Quoted 16 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +16
-0+/** + * ks8695_probe - Probe and initialise a KS8695 ethernet interface + * @pdev: The platform device to probe + * + * Initialise a KS8695 ethernet device from platform data. + * + * This driver requires at least one IORESOURCE_MEM for the + * registers and two IORESOURCE_IRQ for the RX and TX IRQs + * respectively. It can optionally take an additional + * IORESOURCE_MEM for the switch or phy in the case of the lan or + * wan ports, and an IORESOURCE_IRQ for the link IRQ for the wan + * port. + */ +static int __devinit +ks8695_probe(struct platform_device *pdev) +{
[...]
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 3 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +3
-0+ /* Tell the net_device all about us */ + ndev->base_addr = (unsigned long)ksp->io_regs; + ndev->irq = ksp->rx_irq;
Don't bother setting base_addr and irq; they are for ancient ISA crap.
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 11 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +11
-0+ /* driver system setup */ + ether_setup(ndev); + + ndev->open = &ks8695_open; + ndev->stop = &ks8695_stop; + ndev->hard_start_xmit = &ks8695_start_xmit; + ndev->tx_timeout = &ks8695_timeout; + ndev->set_multicast_list = &ks8695_set_multicast; + ndev->watchdog_timeo = msecs_to_jiffies(watchdog); + ndev->ethtool_ops = &ks8695_ethtool_ops; + ndev->set_mac_address = &ks8695_set_mac;
You should use net_device_ops now.
Quoted 10 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10
-0+ /* Initialise the port (physically) */ + if (ksp->phyiface_regs && ksp->link_irq == -1) { + ks8695_init_switch(ksp); + ksp->dtype = KS8695_DTYPE_LAN; + } else if (ksp->phyiface_regs && ksp->link_irq != -1) { + ks8695_init_wan_phy(ksp); + ksp->dtype = KS8695_DTYPE_WAN; + } else { + /* No initialisation since HPNA does not have a PHY */ + ksp->dtype = KS8695_DTYPE_HPNA;
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 10 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10
-0+ } + + /* And bring up the net_device with the net core */ + platform_set_drvdata(pdev, ndev); + ret = register_netdev(ndev); + + if (ret == 0) { + DECLARE_MAC_BUF(mac); + dev_info(ksp->dev, "ks8695 ethernet (%s) MAC: %s\n", + ks8695_port_type(ksp), print_mac(mac, ndev->dev_addr));
Use %pM to format a MAC address instead of %s and print_mac().
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 3 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +3
-0+ } else { + /* Report the failure to register the net_device */ + goto failure;
I don't think this reports anything!
Quoted 12 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +12
-0+ } + + /* All is well */ + return 0; + + /* Error exit path */ +failure: + ks8695_release_device(ksp); + free_netdev(ndev); + + return ret; +}
[...]
Quoted 6 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +6
-0+/** + * ks8695_drv_resume - Resume a KS8695 ethernet platform device. + * @pdev: The device to resume + * + * This routine re-initialises and re-attaches a KS8695 ethernet + * device.
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 10 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +10
-0+ */ +static int +ks8695_drv_resume(struct platform_device *pdev) +{ + struct net_device *ndev = platform_get_drvdata(pdev); + struct ks8695_priv *ksp = ks8695_get_priv(ndev); + + if (netif_running(ndev)) { + ks8695_reset(ksp); + ks8695_init_net(ksp);
Does this reinit the MAC address or multicast?
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000
Quoted 7 lines
drivers/net/arm/ks8695net.c 2008-12-05 13:07:56.773076323 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +7
-0+ netif_device_attach(ndev); + } + + ksp->in_suspend = 0; + + return 0; +}
[...]
Ben.
Signature · 3 lines
Ben Hutchings, Senior Software Engineer, Solarflare Communications Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.
Quoted 2 lines · Ben Hutchings
Quoted 1 line
Index: linux-2.6.27/drivers/net/arm/Kconfig
netdev patches should really be based on David Miller's net-next-2.6.
Noted, I shall forward-port to that tree before issuing a new patch.
Quoted 7 lines
drivers/net/arm/ks8695net.c line 102
Quoted 6 lines
Index:
linux-2.6.27/drivers/net/arm/ks8695net.cdrivers/net/arm/ks8695net.c new file +5
-0+enum ks8695_dtype { +KS8695_DTYPE_WAN , +KS8695_DTYPE_LAN , +KS8695_DTYPE_HPNA , +};The enumerators should be indented with tabs.
Gah, I wonder how checkpatch missed that, fixed, thanks.
Quoted 6 lines
Quoted 5 lines
+/**
+ * struct ks8695_priv - Private data for the KS8695 Ethernet + * @in_suspend: Flag to indicate if we're suspending/resuming + * @ndev: The net_device for this interface + * @dev: The device object for this interfaceWhich device object - the platform device?
Assuming the struct device obtained from the struct platform_device is the "platform device object" then yes. I've updated the comment.
Quoted 4 lines
drivers/net/arm/ks8695net.c line 201
Quoted 2 lines
drivers/net/arm/ks8695net.c new file +2
-0+static inline struct ks8695_priv * +ks8695_get_priv (struct net_device *ndev)Don't use
ndev->priv, usenetdev_priv(ndev). Which makes this function redundant, except for the implicit pointer conversion.
Aah, I didn't know about that one, updated.
Quoted 7 lines
drivers/net/arm/ks8695net.c line 281
Quoted 6 lines
drivers/net/arm/ks8695net.c new file +5
-0+static void +ks8695_refill_rxbuffers (struct ks8695_priv *ksp) + mapping =dma_map_single (ksp->dev, skb->data, +MAX_RXBUF_SIZE , +DMA_FROM_DEVICE );I take it that
dma_map_single()can never fail on ARM?
No it can't. It essentially boils down to some pointer arithmetic on this platform.
Quoted 5 lines
drivers/net/arm/ks8695net.c line 289
Quoted 2 lines
drivers/net/arm/ks8695net.c new file +2
-0+ ksp->rx_ring[buff_n].length = +cpu_to_le32 (MAX_RXBUF_SIZE );You need a
wmb()in here, otherwise the descriptor might not be complete when the hardware seesRDES_OWNset.
Noted and added.
Quoted 4 lines
drivers/net/arm/ks8695net.c line 361
Quoted 3 lines
drivers/net/arm/ks8695net.c new file +3
-0+for (buff_n =0 ; buff_n <MAX_TX_DESC ; ++buff_n) { +if (ksp->tx_buffers[buff_n].skb && + !(ksp->tx_ring[buff_n].owner &cpu_to_le32 (TDES_OWN ))) {
rmb()
Added.
Quoted 5 lines
drivers/net/arm/ks8695net.c line 368
Quoted 2 lines
drivers/net/arm/ks8695net.c new file +2
-0+/* Free the packet from the ring */ + ksp->tx_ring[buff_n].data_ptr =0 ;Clearing
data_ptrseems redundant. TheTDES_OWNflag indicates whether the descriptor is pending and the skb pointer indicates whether a buffer needs to be freed.
This was simply me being neat. If you feel that it is an overly expensive thing to do then I shall elide it.
Quoted 3 lines
Do you need to test all the descriptors or can you keep track of which to check first and then stop when you find one still pending? Maybe it doesn't matter when you only have 8 of them...
Keeping track of which to check first seemed to make the overall code larger, and with a small number of TX descriptors, as you say, it really doesn't matter.
Also, as a point, I have tested shovelling as much data as I can through this over various protocols including NFS and basic TCP with multiple simultaneous streams and I have yet to exhaust the TX descriptor ring. I think the 266MHz ARM processor simply can't stoke it fast enough.
Quoted 5 lines
drivers/net/arm/ks8695net.c line 409
Quoted 4 lines
drivers/net/arm/ks8695net.c new file +4
-0+static irqreturn_t +ks8695_rx_irq(int irq, void *dev_id) + if (ksp->rx_buffers[buff_n].skb && + !(ksp->rx_ring[buff_n].status & cpu_to_le32(RDES_OWN))) {
rmb()
Added.
Quoted 7 lines
Quoted 1 line
+ skb = ksp->rx_buffers[buff_n].skb;
I think this line belongs after the following comment.
Quoted 3 lines
drivers/net/arm/ks8695net.c new file +3
-0+/* Extract the sk_buff from the ring */ + ksp->rx_buffers[buff_n].skb =NULL ; + ksp->rx_ring[buff_n].data_ptr =0 ;
I've shifted the comment and reworded it slightly so that the first like is marked 'retrieve the sk_buff' and the latter lines 'clear it from the ring' which I believe better describes the situation.
Quoted 2 lines
Quoted 1 line
+
buff_n= (buff_n+ 1) %MAX_RX_DESC;That's another expensive division.
I was expecting the compiler to go "aaha! mod <powerof2> => bitwise mask".
However, it does indeed seem to not be the case. I have reworked these into & MAX_RX_DESC_MASK etc.
Quoted 4 lines
drivers/net/arm/ks8695net.c line 541
Quoted 2 lines
drivers/net/arm/ks8695net.c new file +2
-0+static void +ks8695_reset (struct ks8695_priv *ksp)Is there any reason this timeout should be tied to the TX watchdog timeout?
It was a convenient timeout. Basically I wanted something which would allow me to time-out the reset of the chip, but I didn't feel it worthwhile to add another module parameter for it, and I didn't want to pluck a number out of the air per-se. If you feel it deserves its own separate timeout then I will add one.
Quoted 5 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+static int +ks8695_init_net(struct ks8695_priv *ksp)The RX and TX IRQ handlers need to be released in case the TX or link IRQ setup fails.
The only time that this might fail, I.E. initial device open, we check the return value of ks8695_init_net and call ks86985_shutdown if needed which will release the IRQs.
It seemed daft to replicate the cleanup code in two places. If however that is the accepted style then I shall add the cleanups to ks8695_init_net() -- Is my method acceptable or should I change?
Quoted 6 lines
drivers/net/arm/ks8695net.c line 787
Quoted 5 lines
drivers/net/arm/ks8695net.c new file +5
-0+static int +ks8695_get_settings (struct net_device *ndev,struct ethtool_cmd *cmd) +{ + cmd->advertising =ADVERTISED_TP |ADVERTISED_MII ; + cmd->port =PORT_MII ;I think the TP and MII flags should be dependent on
ksp->dtype.
Certainly until I can decode this switch datasheet I agree. Changed.
Quoted 14 lines
Quoted 4 lines
drivers/net/arm/ks8695net.c new file +4
-0+/* Port specific extras */ +switch (ksp->dtype) { +case KS8695_DTYPE_HPNA : + cmd->phy_address =0 ;drivers/net/arm/ks8695net.c line 803
Quoted 10 lines
drivers/net/arm/ks8695net.c new file +10
-0+/* not supported for HPNA */ + cmd->autoneg =AUTONEG_DISABLE ; + +/* BUG: Erm, dtype hpna implies no phy regs */ +/* +ctrl = readl(KS8695_MISC_VA + KS8695_HMC); +cmd->speed = (ctrl & HMC_HSS) ? SPEED_100 : SPEED_10; +cmd->duplex = (ctrl & HMC_HDS) ? DUPLEX_FULL : DUPLEX_HALF; +*/ +break ;
Quoted 7 lines
[...]
drivers/net/arm/ks8695net.c line 840
Quoted 4 lines
drivers/net/arm/ks8695net.c new file +4
-0+case KS8695_DTYPE_LAN : +/* TODO: Implement this for the switch ports */ +break ; + }You'd better implement these or return
-EOPNOTSUPP. Similarly inks8695_set_settings()andks8695_nwayreset().
Unfortunately I currently lack any hardware with HPNA on it, and the only reference for implementing the LAN switch stuff is an immensely confusing and likely-to-be-incorrect datasheet from Micrel which I have yet to successfully decode.
For now, all those functions will return -EOPNOTSUPP, thanks for the pointer.
Quoted 2 lines
Quoted 1 line
+ * TODO: Implement this
Yes, or return
-EOPNOTSUPP.
Until I can decode this datasheet, it's -EOPNOTSUPP :-(
Quoted 3 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+static void +ks8695_timeout(struct net_device *ndev)It looks like this might lose multicast settings.
Yep, when I wrote this routine, I hadn't gotten around to the multicast stuff, and I forgot to go back and correct it. I'll do so now.
Quoted 6 lines
Quoted 6 lines
drivers/net/arm/ks8695net.c new file +6
-0+static int +ks8695_start_xmit (struct sk_buff *skb,struct net_device *ndev) +{ +struct ks8695_priv *ksp =ks8695_get_priv (ndev); +int buff_n; +
Quoted 8 lines
drivers/net/arm/ks8695net.c line 1190
Quoted 6 lines
drivers/net/arm/ks8695net.c new file +6
-0+spin_lock_irq (&ksp->txq_lock); + +if (ksp->tx_ring_used ==MAX_TX_DESC ) { +/* Somehow we got entered when we have no room */ +spin_unlock_irq (&ksp->txq_lock); +return -ENOSPC ;You must return
NETDEV_TX_OKor NETDEV_TX_BUSY, not 0 or a negative error code.
Aha, thanks.
Quoted 5 lines
drivers/net/arm/ks8695net.c line 1194
Quoted 2 lines
drivers/net/arm/ks8695net.c new file +2
-0+ buff_n = ksp->tx_ring_next_slot; + ksp->tx_ring_next_slot = (buff_n +1 ) %MAX_TX_DESC ;That's an expensive division, which could be avoided by either (1) declaring
buff_nunsigned or (2) explicitly masking.
Using masking as elsewhere you noted this.
Quoted 5 lines
drivers/net/arm/ks8695net.c line 1199
Quoted 4 lines
drivers/net/arm/ks8695net.c new file +4
-0+if (ksp->tx_buffers[buff_n].skb) { +/* This slot is used, try later */ +spin_unlock_irq (&ksp->txq_lock); +return -EAGAIN ;Since you already tested
tx_ring_used, wouldn't this indicate a bug?
Yep, this was old code before I kept a track in tx_ring_used. Changed to:
BUG_ON(ksp->tx_buffers[buff_n].skb);
Quoted 5 lines
drivers/net/arm/ks8695net.c line 1212
Quoted 3 lines
drivers/net/arm/ks8695net.c new file +3
-0+ ksp->tx_ring[buff_n].status = +cpu_to_le32 (TDES_IC |TDES_FS |TDES_LS | + (skb->len &TDES_TBS ));
wmb()
Added
Quoted 6 lines
drivers/net/arm/ks8695net.c line 1448
Quoted 5 lines
drivers/net/arm/ks8695net.c new file +5
-0+static int __devinit +ks8695_probe (struct platform_device *pdev) +/* Tell the net_device all about us */ + ndev->base_addr = (unsigned long)ksp->io_regs; + ndev->irq = ksp->rx_irq;Don't bother setting
base_addrand irq; they are for ancient ISA crap.
Removed.
Quoted 9 lines
drivers/net/arm/ks8695net.c line 1460
Quoted 8 lines
drivers/net/arm/ks8695net.c new file +8
-0+ ndev->open = &ks8695_open; + ndev->stop = &ks8695_stop; + ndev->hard_start_xmit = &ks8695_start_xmit; + ndev->tx_timeout = &ks8695_timeout; + ndev->set_multicast_list = &ks8695_set_multicast; + ndev->watchdog_timeo =msecs_to_jiffies (watchdog); + ndev->ethtool_ops = &ks8695_ethtool_ops; + ndev->set_mac_address = &ks8695_set_mac;You should use
net_device_opsnow.
I shall do that as I bring it up to net-next-2.6 in a bit.
Quoted 3 lines
drivers/net/arm/ks8695net.c line 1538
Quoted 2 lines
drivers/net/arm/ks8695net.c new file +2
-0+dev_info (ksp->dev,"ks8695 ethernet (%s) MAC: %s\n" , +ks8695_port_type (ksp),print_mac (mac, ndev->dev_addr));Use %pM to format a MAC address instead of %s and
print_mac().
I believe this must be new since 2.6.27 so I'll get that as I move to net-next-2.6, thanks for the info.
Quoted 4 lines
drivers/net/arm/ks8695net.c line 1541
Quoted 3 lines
drivers/net/arm/ks8695net.c new file +3
-0+ }else { +/* Report the failure to register the net_device */ + goto failure;I don't think this reports anything!
It reports to the next level up via the return code, which IIRC prints a message, but I've added a dev_err() anyway before the goto failure.
Quoted 7 lines
drivers/net/arm/ks8695net.c line 1593
Quoted 6 lines
drivers/net/arm/ks8695net.c new file +5
-0+static int +ks8695_drv_resume (struct platform_device *pdev) + if (netif_running(ndev)) { +ks8695_reset (ksp); +ks8695_init_net (ksp);Does this reinit the MAC address or multicast?
Nope, as per the timeout, I'd missed adding it here. That is now corrected.
Thank you very much for your comments. Once this pull of net-next-2.6 is complete and I can do my forward-porting, I'll issue a new patch. Can I ask what the magic runes are which I need in subject / precis to actually request that my driver be merged rather than just reviewed?
Regards,
Daniel.
Signature · 2 lines
Daniel Silverstone http://www.simtec.co.uk/ PGP mail accepted and encouraged. Key Id: 2BC8 4016 2068 7895
On Mon, 2008-12-08 at 11:05 +0000, Daniel Silverstone wrote:
[...]
Quoted 6 lines
Quoted 4 lines
drivers/net/arm/ks8695net.c line 201
Quoted 2 lines
drivers/net/arm/ks8695net.c new file +2
-0+static inline struct ks8695_priv * +ks8695_get_priv (struct net_device *ndev)Don't use
ndev->priv, usenetdev_priv(ndev). Which makes this function redundant, except for the implicit pointer conversion.Aah, I didn't know about that one, updated.
Until recently they have been equivalent by default; direct access to priv is now deprecated and it will be removed.
[...]
Quoted 8 lines
Quoted 5 lines
drivers/net/arm/ks8695net.c line 368
Quoted 2 lines
drivers/net/arm/ks8695net.c new file +2
-0+/* Free the packet from the ring */ + ksp->tx_ring[buff_n].data_ptr =0 ;Clearing
data_ptrseems redundant. TheTDES_OWNflag indicates whether the descriptor is pending and the skb pointer indicates whether a buffer needs to be freed.This was simply me being neat. If you feel that it is an overly expensive thing to do then I shall elide it.
It's hardly expensive, but the comment is inaccurate because this field does not indicate whether the descriptor is free or not.
[...]
Quoted 5 lines
Quoted 2 lines
Quoted 1 line
+
buff_n= (buff_n+ 1) %MAX_RX_DESC;That's another expensive division.
I was expecting the compiler to go "aaha! mod <powerof2> => bitwise mask".
It can only do this if it knows the dividend is non-negative.
I wrote "another" in reference to my comment further down which I actually wrote earlier.
[...]
Quoted 6 lines
Quoted 5 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+static int +ks8695_init_net(struct ks8695_priv *ksp)The RX and TX IRQ handlers need to be released in case the TX or link IRQ setup fails.
Quoted 7 lines
The only time that this might fail, I.E. initial device open, we check the return value of
ks8695_init_netand callks86985_shutdownif needed which will release the IRQs.It seemed daft to replicate the cleanup code in two places. If however that is the accepted style then I shall add the cleanups to
ks8695_init_net()-- Is my method acceptable or should I change?
Local cleanup is normal but I think this is OK.
[...]
Quoted 6 lines
Quoted 3 lines
drivers/net/arm/ks8695net.c line 1538
Quoted 2 lines
drivers/net/arm/ks8695net.c new file +2
-0+dev_info (ksp->dev,"ks8695 ethernet (%s) MAC: %s\n" , +ks8695_port_type (ksp),print_mac (mac, ndev->dev_addr));Use %pM to format a MAC address instead of %s and
print_mac().I believe this must be new since 2.6.27 so I'll get that as I move to net-next-2.6, thanks for the info.
Yes, the %p<modifier> formats are a recent invention.
[...]
Quoted 4 lines
Thank you very much for your comments. Once this pull of net-next-2.6 is complete and I can do my forward-porting, I'll issue a new patch. Can I ask what the magic runes are which I need in subject / precis to actually request that my driver be merged rather than just reviewed?
Unless you add [RFC] to the subject it's implicit that you want the patch merged. You can check the patch status at
<http://patchwork.ozlabs.org/project/netdev/list/>.
Ben.
Signature · 3 lines
Ben Hutchings, Senior Software Engineer, Solarflare Communications Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.
David Miller
maintainerFrom: Ben Hutchings <[email protected]>
Date: Mon, 08 Dec 2008 15:17:53 +0000
Quoted 11 lines · Daniel Silverstone
[...]
Quoted 6 lines
Quoted 4 lines
drivers/net/arm/ks8695net.c line 201
Quoted 2 lines
drivers/net/arm/ks8695net.c new file +2
-0+static inline struct ks8695_priv * +ks8695_get_priv (struct net_device *ndev)Don't use
ndev->priv, usenetdev_priv(ndev). Which makes this function redundant, except for the implicit pointer conversion.Aah, I didn't know about that one, updated.
Until recently they have been equivalent by default; direct access to priv is now deprecated and it will be removed.
Not just "will be removed", but "is removed" in the current net-next-2.6 tree so this driver wouldn't even compile :-)
Quoted 7 lines · Daniel Silverstone
Quoted 4 lines
drivers/net/arm/ks8695net.c line 281
Quoted 3 lines
drivers/net/arm/ks8695net.c new file +3
-0+ mapping =dma_map_single (ksp->dev, skb->data, +MAX_RXBUF_SIZE , +DMA_FROM_DEVICE );I take it that
dma_map_single()can never fail on ARM?No it can't. It essentially boils down to some pointer arithmetic on this platform.
You should check for errors anyway. The driver might be ported to an architecture where it can fail (e.g. the SOC block gets reused), ARM variants may come up where it can fail, or people might use the driver as an example for their own new driver.
David Miller
maintainerFrom: Christoph Hellwig <[email protected]>
Date: Tue, 9 Dec 2008 03:57:39 -0500
Quoted 6 lines · Daniel Silverstone
Quoted 5 lines
Quoted 4 lines
drivers/net/arm/ks8695net.c line 281
Quoted 3 lines
drivers/net/arm/ks8695net.c new file +3
-0+ mapping =dma_map_single (ksp->dev, skb->data, +MAX_RXBUF_SIZE , +DMA_FROM_DEVICE );I take it that
dma_map_single()can never fail on ARM?
Quoted 7 lines
Quoted 2 lines
No it can't. It essentially boils down to some pointer arithmetic on this platform.
You should check for errors anyway. The driver might be ported to an architecture where it can fail (e.g. the SOC block gets reused), ARM variants may come up where it can fail, or people might use the driver as an example for their own new driver.
Agreed.
Quoted 11 lines · David Miller
Quoted 10 lines
Quoted 6 lines
Quoted 4 lines
drivers/net/arm/ks8695net.c line 281
Quoted 3 lines
drivers/net/arm/ks8695net.c new file +3
-0+ mapping =dma_map_single (ksp->dev, skb->data, +MAX_RXBUF_SIZE , +DMA_FROM_DEVICE );I take it that
dma_map_single()can never fail on ARM?No it can't. It essentially boils down to some pointer arithmetic on this platform.
You should check for errors anyway. The driver might be ported to an architecture where it can fail (e.g. the SOC block gets reused), ARM variants may come up where it can fail, or people might use the driver as an example for their own new driver.
Agreed.
Very well, I shall sort out an ammended patch. Thanks for your help in reviewing the code.
Regards,
Daniel.
Signature · 2 lines
Daniel Silverstone http://www.simtec.co.uk/ PGP mail accepted and encouraged. Key Id: 2BC8 4016 2068 7895
Quoted 10 lines · Daniel Silverstone
Quoted 5 lines
drivers/net/arm/ks8695net.c line 1199
Quoted 4 lines
drivers/net/arm/ks8695net.c new file +4
-0+if (ksp->tx_buffers[buff_n].skb) { +/* This slot is used, try later */ +spin_unlock_irq (&ksp->txq_lock); +return -EAGAIN ;Since you already tested
tx_ring_used, wouldn't this indicate a bug?Yep, this was old code before I kept a track in
tx_ring_used. Changed to:
BUG_ON(ksp->tx_buffers[buff_n].skb);
I would say that a network driver should never use BUG_ON(), it just makes both user's and developer's life difficult because the whole kernel will stop the execution. WARN_ON() is much nicer, it just prints a nice warning to the log which then can be reported either manually or automatically.
Signature · 1 line
Kalle Valo
Daniel Silverstone
Acked-by Ben DooksQuoted 1 line · Ben Hutchings
netdev patches should really be based on David Miller's net-next-2.6.
Attached is a patch against net-next-2.6 as cloned this morning, with the %pM instead of print_mac() and the net_device_ops instead of direct assignment of pointers.
Assuming this is okay, I'd very much like for it to be merged.
Regards,
Daniel.
Signature · 2 lines
Daniel Silverstone http://www.simtec.co.uk/ PGP mail accepted and encouraged. Key Id: 2BC8 4016 2068 7895
KS8695: Add support for the KS8695 ethernet devices.
Implements the KS8695 ethernet device (ks8695net).
This driver is only of use on the KS8695 which is an ARM9 based SoC. The documentation on this SoC is sparse and poor, with barely a register description and a rough outline of how the ethernet works, this driver was therefore written with strong reference to the Micrel supplied Linux 2.6.9 port, and to Andrew Victor's ks8695eth driver.
Kconfig | 8 Makefile | 1 ks8695net.c | 1675 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ ks8695net.h | 107 +++ 4 files changed, 1791 insertions(+)
drivers/net/arm/Kconfig +8 -0
@@ -40,6 +40,14 @@ If you wish to compile a kernel for the AT91RM9200 and enable ethernet support, then you should always answer Y to this. +config ARM_KS8695_ETHER + tristate "KS8695 Ethernet support" + depends on ARM && ARCH_KS8695 + select MII + help + If you wish to compile a kernel for the KS8695 and want to + use the internal ethernet then you should answer Y to this. + config EP93XX_ETH tristate "EP93xx Ethernet support" depends on ARM && ARCH_EP93XX
drivers/net/arm/Makefile +1 -0
@@ -8,5 +8,6 @@ obj-$(CONFIG_ARM_ETHER3) += ether3.o obj-$(CONFIG_ARM_ETHER1) += ether1.o obj-$(CONFIG_ARM_AT91_ETHER) += at91_ether.o +obj-$(CONFIG_ARM_KS8695_ETHER) += ks8695net.o obj-$(CONFIG_EP93XX_ETH) += ep93xx_eth.o obj-$(CONFIG_IXP4XX_ETH) += ixp4xx_eth.o
drivers/net/arm/ks8695net.c new file +1675 -0
1,675 lines not on this page · raw message
drivers/net/arm/ks8695net.h new file +107 -0
107 lines not on this page · raw message
Daniel Silverstone
Acked-by Ben DooksHello,
Attached is what I believe to be a reasonable final-cut of the driver.
I believe I have addressed all the concerns raised by Ben Hutchings, Christoph Hellwig and David Miller.
Assuming I have done so successfully, I'd like to request a merge of this version.
Regards,
Daniel.
Signature · 2 lines
Daniel Silverstone http://www.simtec.co.uk/ PGP mail accepted and encouraged. Key Id: 2BC8 4016 2068 7895
KS8695: Add support for the KS8695 ethernet devices.
Implements the KS8695 ethernet device (ks8695net).
This driver is only of use on the KS8695 which is an ARM9 based SoC. The documentation on this SoC is sparse and poor, with barely a register description and a rough outline of how the ethernet works, this driver was therefore written with strong reference to the Micrel supplied Linux 2.6.9 port, and to Andrew Victor's ks8695eth driver.
Kconfig | 8 Makefile | 1 ks8695net.c | 1691 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ ks8695net.h | 107 +++ 4 files changed, 1807 insertions(+)
drivers/net/arm/Kconfig +8 -0
@@ -40,6 +40,14 @@ If you wish to compile a kernel for the AT91RM9200 and enable ethernet support, then you should always answer Y to this. +config ARM_KS8695_ETHER + tristate "KS8695 Ethernet support" + depends on ARM && ARCH_KS8695 + select MII + help + If you wish to compile a kernel for the KS8695 and want to + use the internal ethernet then you should answer Y to this. + config EP93XX_ETH tristate "EP93xx Ethernet support" depends on ARM && ARCH_EP93XX
drivers/net/arm/Makefile +1 -0
@@ -8,5 +8,6 @@ obj-$(CONFIG_ARM_ETHER3) += ether3.o obj-$(CONFIG_ARM_ETHER1) += ether1.o obj-$(CONFIG_ARM_AT91_ETHER) += at91_ether.o +obj-$(CONFIG_ARM_KS8695_ETHER) += ks8695net.o obj-$(CONFIG_EP93XX_ETH) += ep93xx_eth.o obj-$(CONFIG_IXP4XX_ETH) += ixp4xx_eth.o
drivers/net/arm/ks8695net.c new file +1691 -0
1,691 lines not on this page · raw message
drivers/net/arm/ks8695net.h new file +107 -0
107 lines not on this page · raw message
A message that is not in the archive (sent off-list or not yet received): 1228764912.3177.39.camel@achroite
Daniel Silverstone
Acked-by Ben DooksRe: [Patch] Micrel KS8695 integrated ethernet driver
Ben pointed out that he had sent this to me but not CCed the list, and that I'd missed it. I've worked through it now.
Quoted 1 line · Ben Hutchings
There's a spelling error in your subject; it should say "integrated".
That's just a spello on the mailing list subject line, the subject line in the patch is okay.
Quoted 4 lines · Daniel Silverstone
[...]
Quoted 1 line
+#define MODULENAME "
ks8695_ether"Shouldn't this be "ks8695net"?
I was replacing Andrew Victor's ks8695_ether driver, so the platform data is for ks8695_ether, but for a while I had both drivers in-tree in order to check behaviour. I am happy for it to be merged either as ks8695_ether or ks8695net and if necessary I can change the platform data for the boards. However I was trying for path of least resistance on this particular front.
Quoted 5 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+/* + * Transmit timeout, default 5 seconds. + */ +static int watchdog = 5000;The comment should say this is also the reset timeout.
Yep, changed, thanks.
Quoted 5 lines
Quoted 4 lines
+ * enum ks8695_dtype - Device type + * @KS8695_DTYPE_WAN: This device is a WAN interface + * @KS8695_DTYPE_LAN: This device is a LAN interface + * @KS8695_DTYPE_HPNA: This device is an HPNA interfacewTypo: there's a rogue "w" on the end.
Argh, fixed.
Quoted 6 lines
Review
Quoted 5 lines
(quoted diff) +5
-0+static inline struct ks8695_priv * +ks8695_get_priv(struct net_device *ndev) +{ + return ndev->priv; +}This function needs to be removed.
Yes, I did the netdev_priv() change but forgot to remove it once I had compile-and-run-tested the kernel. I have removed it now, thanks.
Quoted 8 lines
Review
Quoted 5 lines
(quoted diff) +5
-0+static void +ks8695_init_partial_multicast(struct ks8695_priv *ksp, + /* Ran out of space in chip? */ + if (i == KS8695_NR_ADDRESSES) + break;Currently the caller already checks that you won't run out of space, so you can use
BUG_ON()here. Certainly I don't think this should quietly ignore the error.
Accepted. I'm still getting used to BUG_ON etc. Changed.
Quoted 20 lines
Quoted 18 lines
(quoted diff) +18
-0+ if (ndev->flags & IFF_ALLMULTI) { + /* enable all multicast mode */ + ctrl |= DRXC_RM; + } else if (ndev->mc_count > KS8695_NR_ADDRESSES) { + /* more specific multicast addresses than can be + * handled in hardware + */ + ctrl |= DRXC_RM; + } else if (ndev->mc_count > 0) { + /* enable specific multicasts */ + ctrl &= ~DRXC_RM; + ks8695_init_partial_multicast(ksp, ndev->mc_list, + ndev->mc_count); + } else if (ndev->flags & ~IFF_ALLMULTI) { + /* disable multicast mode */ + ctrl &= ~DRXC_RM; + ks8695_init_partial_multicast(ksp, NULL, 0); + }[...]
Quoted 6 lines
This last condition is bogus. I think it's supposed to mean !(ndev->flags &
IFF_ALLMULTI) but that will always be true here.Further, I think the code for
ndev->mc_count> 0 will also work forndev->mc_count== 0, so you could remove both conditions and the last block.
Yep, makes sense. That'll teach me to blindly go "and now this case, and now this case, and now this case" without trying to compress the cases.
Done.
New patch attached, which, regardless of my previous assertion, I *now* believe covers everything (providing you're happy about the driver name malarky). To celebrate this, I even changed the version a bit so we can be sure we're all reviewing the same code.
Thanks again, and hopefully this will be good enough for merging. Sorry for the repeated posting of patches, but I think I'm learning.
Regards,
Daniel.
Signature · 2 lines
Daniel Silverstone http://www.simtec.co.uk/ PGP mail accepted and encouraged. Key Id: 2BC8 4016 2068 7895
KS8695: Add support for the KS8695 ethernet devices.
Implements the KS8695 ethernet device (ks8695net).
This driver is only of use on the KS8695 which is an ARM9 based SoC. The documentation on this SoC is sparse and poor, with barely a register description and a rough outline of how the ethernet works, this driver was therefore written with strong reference to the Micrel supplied Linux 2.6.9 port, and to Andrew Victor's ks8695eth driver.
Kconfig | 8 Makefile | 1 ks8695net.c | 1674 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ ks8695net.h | 107 +++ 4 files changed, 1790 insertions(+)
drivers/net/arm/Kconfig +8 -0
@@ -40,6 +40,14 @@ If you wish to compile a kernel for the AT91RM9200 and enable ethernet support, then you should always answer Y to this. +config ARM_KS8695_ETHER + tristate "KS8695 Ethernet support" + depends on ARM && ARCH_KS8695 + select MII + help + If you wish to compile a kernel for the KS8695 and want to + use the internal ethernet then you should answer Y to this. + config EP93XX_ETH tristate "EP93xx Ethernet support" depends on ARM && ARCH_EP93XX
drivers/net/arm/Makefile +1 -0
@@ -8,5 +8,6 @@ obj-$(CONFIG_ARM_ETHER3) += ether3.o obj-$(CONFIG_ARM_ETHER1) += ether1.o obj-$(CONFIG_ARM_AT91_ETHER) += at91_ether.o +obj-$(CONFIG_ARM_KS8695_ETHER) += ks8695net.o obj-$(CONFIG_EP93XX_ETH) += ep93xx_eth.o obj-$(CONFIG_IXP4XX_ETH) += ixp4xx_eth.o
drivers/net/arm/ks8695net.c new file +1674 -0
1,674 lines not on this page · raw message
drivers/net/arm/ks8695net.h new file +107 -0
107 lines not on this page · raw message
Ben Hutchings
Reviewed-byRe: [Patch] Micrel KS8695 integrated ethernet driver
On Tue, 2008-12-09 at 17:30 +0000, Daniel Silverstone wrote:
[...]
Quoted 10 lines
KS8695: Add support for the KS8695 ethernet devices.
Implements the KS8695 ethernet device (ks8695net).
This driver is only of use on the KS8695 which is an ARM9 based SoC. The documentation on this SoC is sparse and poor, with barely a register description and a rough outline of how the ethernet works, this driver was therefore written with strong reference to the Micrel supplied Linux 2.6.9 port, and to Andrew Victor's ks8695eth driver.
Quoted 3 lines
Signed-off-by Daniel Silverstone <[email protected]>Signed-off-by Vincent Sanders <[email protected]>Acked-by Ben Dooks <[email protected]>
Quoted 84 lines
Kconfig | 8 Makefile | 1 ks8695net.c | 1674 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ ks8695net.h | 107 +++ 4 files changed, 1790 insertions(+)Index: net-next-2.6/drivers/net/arm/Kconfig
===================================================================drivers/net/arm/Kconfig 2008-12-08 12:56:10.213660241 +0000 renamed from drivers/net/arm/Kconfig 2008-12-08 12:55:46.817658739 +0000 +8
-0@@ -40,6 +40,14 @@ If you wish to compile a kernel for the AT91RM9200 and enable ethernet support, then you should always answer Y to this. +config ARM_KS8695_ETHER + tristate "KS8695 Ethernet support" + depends on ARM && ARCH_KS8695 + select MII + help + If you wish to compile a kernel for the KS8695 and want to + use the internal ethernet then you should answer Y to this. + config EP93XX_ETH tristate "EP93xx Ethernet support" depends on ARM && ARCH_EP93XXIndex: net-next-2.6/drivers/net/arm/Makefile
===================================================================drivers/net/arm/Makefile 2008-12-08 12:56:10.213660241 +0000 renamed from drivers/net/arm/Makefile 2008-12-08 12:55:46.841662533 +0000 +1
-0@@ -8,5 +8,6 @@ obj-$(CONFIG_ARM_ETHER3) += ether3.o obj-$(CONFIG_ARM_ETHER1) += ether1.o obj-$(CONFIG_ARM_AT91_ETHER) += at91_ether.o +obj-$(CONFIG_ARM_KS8695_ETHER) += ks8695net.o obj-$(CONFIG_EP93XX_ETH) += ep93xx_eth.o obj-$(CONFIG_IXP4XX_ETH) += ixp4xx_eth.oIndex:
net-next-2.6/drivers/net/arm/ks8695net.c
===================================================================drivers/net/arm/ks8695net.c 2008-12-09 17:26:28.469658828 +0000 renamed from dev/null 1970-01-01 00:00:00.000000000 +0000 +43
-0@@ -0,0 +1,1674 @@ +/* + * Micrel KS8695 (Centaur) Ethernet. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * Copyright 2008 Simtec Electronics + * Daniel Silverstone <[email protected]> + * Vincent Sanders <[email protected]> + */ + +#include <linux/module.h> +#include <linux/ioport.h> +#include <linux/netdevice.h> +#include <linux/etherdevice.h> +#include <linux/init.h> +#include <linux/skbuff.h> +#include <linux/spinlock.h> +#include <linux/crc32.h> +#include <linux/mii.h> +#include <linux/ethtool.h> +#include <linux/delay.h> +#include <linux/platform_device.h> +#include <linux/irq.h> +#include <linux/delay.h> +#include <linux/io.h> + +#include <asm/irq.h> + +#include <mach/regs-switch.h> +#include <mach/regs-misc.h> + +#include "ks8695net.h" + +#define MODULENAME "ks8695_ether" +#define MODULEVERSION "1.01"
[...]
Signature · 3 lines
Ben Hutchings, Senior Software Engineer, Solarflare Communications Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.
David Miller
maintainerRe: [Patch] Micrel KS8695 integrated ethernet driver
In reply to Daniel SilverstoneFrom: Daniel Silverstone <[email protected]>
Date: Tue, 09 Dec 2008 17:30:09 +0000
drivers/net/arm/ks8695net.c line 283
Quoted 4 lines
drivers/net/arm/ks8695net.c new file +4
-0+ mapping =dma_map_single (ksp->dev, skb->data, +MAX_RXBUF_SIZE , +DMA_FROM_DEVICE ); +if (!mapping) {
There is a proper way to check for DMA mapping failures. Comparing against zero is not it :-)
drivers/net/arm/ks8695net.c line 1205
Quoted 3 lines
drivers/net/arm/ks8695net.c new file +3
-0+ ksp->tx_buffers[buff_n].dma_ptr =dma_map_single ( + ksp->dev, skb->data, skb->len,DMA_TO_DEVICE ); +if (!ksp->tx_buffers[buff_n].dma_ptr) {
Likewise.
Daniel Silverstone
Acked-by Ben DooksRe: [Patch] Micrel KS8695 integrated ethernet driver
Quoted 10 lines · David Miller
Review
Quoted 4 lines
(quoted diff) +4
-0+ mapping = dma_map_single(ksp->dev, skb->data, + MAX_RXBUF_SIZE, + DMA_FROM_DEVICE); + if (!mapping) {There is a proper way to check for DMA mapping failures. Comparing against zero is not it :-)
Review
Quoted 3 lines
(quoted diff) +3
-0+ ksp->tx_buffers[buff_n].dma_ptr = dma_map_single( + ksp->dev, skb->data, skb->len, DMA_TO_DEVICE); + if (!ksp->tx_buffers[buff_n].dma_ptr) {Likewise.
When I had a quick frobble around for other drivers using dma_map_single I didn't see any error checking so I guessed. I guessed wrongly :-(
I've attached a driver which is the same as before but uses dma_mapping_error() to check the result.
I trust that this is the last issue :-)
Regards,
Daniel.
Signature · 2 lines
Daniel Silverstone http://www.simtec.co.uk/ PGP mail accepted and encouraged. Key Id: 2BC8 4016 2068 7895
KS8695: Add support for the KS8695 ethernet devices.
Implements the KS8695 ethernet device (ks8695net).
This driver is only of use on the KS8695 which is an ARM9 based SoC. The documentation on this SoC is sparse and poor, with barely a register description and a rough outline of how the ethernet works, this driver was therefore written with strong reference to the Micrel supplied Linux 2.6.9 port, and to Andrew Victor's ks8695eth driver.
Kconfig | 8 Makefile | 1 ks8695net.c | 1676 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ ks8695net.h | 107 +++ 4 files changed, 1792 insertions(+)
drivers/net/arm/Kconfig +8 -0
@@ -40,6 +40,14 @@ If you wish to compile a kernel for the AT91RM9200 and enable ethernet support, then you should always answer Y to this. +config ARM_KS8695_ETHER + tristate "KS8695 Ethernet support" + depends on ARM && ARCH_KS8695 + select MII + help + If you wish to compile a kernel for the KS8695 and want to + use the internal ethernet then you should answer Y to this. + config EP93XX_ETH tristate "EP93xx Ethernet support" depends on ARM && ARCH_EP93XX
drivers/net/arm/Makefile +1 -0
@@ -8,5 +8,6 @@ obj-$(CONFIG_ARM_ETHER3) += ether3.o obj-$(CONFIG_ARM_ETHER1) += ether1.o obj-$(CONFIG_ARM_AT91_ETHER) += at91_ether.o +obj-$(CONFIG_ARM_KS8695_ETHER) += ks8695net.o obj-$(CONFIG_EP93XX_ETH) += ep93xx_eth.o obj-$(CONFIG_IXP4XX_ETH) += ixp4xx_eth.o
drivers/net/arm/ks8695net.c new file +1676 -0
1,676 lines not on this page · raw message
drivers/net/arm/ks8695net.h new file +107 -0
107 lines not on this page · raw message
David Miller
maintainerRe: [Patch] Micrel KS8695 integrated ethernet driver
From: Daniel Silverstone <[email protected]>
Date: Wed, 10 Dec 2008 15:41:19 +0000
Quoted 4 lines
I've attached a driver which is the same as before but uses
dma_mapping_error()to check the result.I trust that this is the last issue :-)
Added to net-next-2.6, thanks Daniel.