Bug in ks8851.c

From: Nekludov, Max
Date: Tue Apr 01 2014 - 11:58:08 EST


Reset on RX failure and TX timeout.

Electromagnetic noice can make device to not send TX interrupts.
As result outgoing queue will be suspended forever.
Also EMI can raise RX interrupt with zero value in KS_RXFC register.

Patch attached.ks8851: reset on RX failure and TX timeout

Electromagnetic noice can make device to not send TX interrupts.
As result outgoing queue will be suspended forever.
Also EMI can raise RX interrupt with zero value in KS_RXFC register.

Signed-off-by: Max Nekludov <Max.Nekludov@xxxxxxxxxx>
---
drivers/net/ethernet/micrel/ks8851.c | 66 +++++++++++++++++++++++++++++++++++-
1 file changed, 65 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/micrel/ks8851.c b/drivers/net/ethernet/micrel/ks8851.c
index e0c92e0..25b7229 100644
--- a/drivers/net/ethernet/micrel/ks8851.c
+++ b/drivers/net/ethernet/micrel/ks8851.c
@@ -123,6 +123,7 @@ struct ks8851_net {

struct work_struct tx_work;
struct work_struct rxctrl_work;
+ struct delayed_work wdt_work;

struct sk_buff_head txq;

@@ -140,6 +141,9 @@ static int msg_enable;
/* shift for byte-enable data */
#define BYTE_EN(_x) ((_x) << 2)

+#define KS8851_TX_QUEUE_SIZE 6144
+#define KS8851_TX_TIMEOUT_MS 100
+
/* turn register number and byte-enable mask into data for start of packet */
#define MK_OP(_byteen, _reg) (BYTE_EN(_byteen) | (_reg) << (8+2) | (_reg) >> 6)

@@ -495,6 +499,7 @@ static void ks8851_dbg_dumpkkt(struct ks8851_net *ks, u8 *rxpkt)
rxpkt[8], rxpkt[9], rxpkt[10], rxpkt[11],
rxpkt[12], rxpkt[13], rxpkt[14], rxpkt[15]);
}
+static void ks8851_reset(struct ks8851_net *ks);

/**
* ks8851_rx_pkts - receive packets from the host
@@ -517,6 +522,12 @@ static void ks8851_rx_pkts(struct ks8851_net *ks)

netif_dbg(ks, rx_status, ks->netdev,
"%s: %d packets\n", __func__, rxfc);
+
+ if (rxfc == 0) {
+ mdelay(1);
+ /* RX interrupt but no data in buffer. Which means hardware failure condition. Reset. */
+ ks8851_reset(ks);
+ }

/* Currently we're issuing a read per packet, but we could possibly
* improve the code by issuing a single read, getting the receive
@@ -619,6 +630,10 @@ static irqreturn_t ks8851_irq(int irq, void *_ks)

if (status & IRQ_TXI) {
handled |= IRQ_TXI;
+
+ mutex_unlock(&ks->lock);
+ cancel_delayed_work_sync(&ks->wdt_work);
+ mutex_lock(&ks->lock);

/* no lock here, tx queue should have been stopped */

@@ -749,6 +764,17 @@ static void ks8851_done_tx(struct ks8851_net *ks, struct sk_buff *txb)
dev_kfree_skb(txb);
}

+static void ks8851_wdt_work(struct delayed_work *work)
+{
+ struct ks8851_net *ks = container_of(work, struct ks8851_net, wdt_work);
+ mutex_lock(&ks->lock);
+
+ mdelay(1);
+ ks8851_reset(ks);
+
+ mutex_unlock(&ks->lock);
+}
+
/**
* ks8851_tx_work - process tx packet(s)
* @work: The work strucutre what was scheduled.
@@ -778,6 +804,8 @@ static void ks8851_tx_work(struct work_struct *work)
}
}

+ schedule_delayed_work(&ks->wdt_work, msecs_to_jiffies(KS8851_TX_TIMEOUT_MS));
+
mutex_unlock(&ks->lock);
}

@@ -858,6 +886,40 @@ static int ks8851_net_open(struct net_device *dev)
return 0;
}

+static void ks8851_reset(struct ks8851_net *ks)
+{
+ netif_stop_queue(ks->netdev);
+
+ /* turn off the IRQs and ack any outstanding */
+ ks8851_wrreg16(ks, KS_IER, 0x0000);
+ ks8851_wrreg16(ks, KS_ISR, 0xffff);
+
+ /* shutdown RX process */
+ ks8851_wrreg16(ks, KS_RXCR1, 0x0000);
+
+ /* shutdown TX process */
+ ks8851_wrreg16(ks, KS_TXCR, 0x0000);
+
+ ks->tx_space = KS8851_TX_QUEUE_SIZE;
+
+ mutex_unlock(&ks->lock);
+
+ /* ensure any queued tx buffers are dumped */
+ while (!skb_queue_empty(&ks->txq)) {
+ struct sk_buff *txb = skb_dequeue(&ks->txq);
+
+ netif_dbg(ks, ifdown, ks->netdev,
+ "%s: freeing txb %p\n", __func__, txb);
+
+ dev_kfree_skb(txb);
+ }
+
+ /* reopen device */
+ ks8851_net_open(ks->netdev);
+
+ mutex_lock(&ks->lock);
+}
+
/**
* ks8851_net_stop - close network device
* @dev: The device being closed.
@@ -883,6 +945,7 @@ static int ks8851_net_stop(struct net_device *dev)
/* stop any outstanding work */
flush_work(&ks->tx_work);
flush_work(&ks->rxctrl_work);
+ cancel_delayed_work_sync(&ks->wdt_work);

mutex_lock(&ks->lock);
/* shutdown RX process */
@@ -1415,7 +1478,7 @@ static int ks8851_probe(struct spi_device *spi)

ks->netdev = ndev;
ks->spidev = spi;
- ks->tx_space = 6144;
+ ks->tx_space = KS8851_TX_QUEUE_SIZE;

ks->vdd_reg = regulator_get_optional(&spi->dev, "vdd");
if (IS_ERR(ks->vdd_reg)) {
@@ -1437,6 +1500,7 @@ static int ks8851_probe(struct spi_device *spi)

INIT_WORK(&ks->tx_work, ks8851_tx_work);
INIT_WORK(&ks->rxctrl_work, ks8851_rxctrl_work);
+ INIT_DELAYED_WORK(&ks->wdt_work, ks8851_wdt_work);

/* initialise pre-made spi transfer messages */

--
1.8.3.2