[PATCH 0/5] p54spi: fixes to achieve link stability

[PATCH 0/5] p54spi: fixes to achieve link stability

[Max Filippov]

This patch series fixes STLC DMA write register access bug and cleans
up code that used to mask it.
Also it introduces fix for yet another firmware/DMA bug.

It has been tested in heterogeneous mesh network with Nokia N810
on one side and PC with b43 on the other. There's been hours
of ping -f with different packet size and hours of iperf, where
it showed a peak ~6Mbit throughput.

[PATCH 1/5] p54spi: fix incorrect access sequence to DMA_WRITE_CTRL in p54spi_spi_write_dma

[Max Filippov]

Host is not allowed to modify DMA_WRITE_CTRL register
if bit HOST_ALLOWED in it is not set. Wait for HOST_ALLOWED first.

Also get rid of timeout in p54spi_wait_bit as it's been playing
a role of workaround for such an incorrect register access.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 drivers/net/wireless/p54/p54spi.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/p54/p54spi.c b/drivers/net/wireless/p54/p54spi.c
index e4e708e..ff92bc3 100644
--- a/drivers/net/wireless/p54/p54spi.c
+++ b/drivers/net/wireless/p54/p54spi.c
@@ -158,8 +158,6 @@ static int p54spi_wait_bit(struct p54s_priv *priv, u16 reg, __le32 bits)
 		__le32 buffer = p54spi_read32(priv, reg);
 		if ((buffer & bits) == bits)
 			return 1;
-
-		msleep(0);
 	}
 	return 0;
 }
@@ -167,9 +165,6 @@ static int p54spi_wait_bit(struct p54s_priv *priv, u16 reg, __le32 bits)
 static int p54spi_spi_write_dma(struct p54s_priv *priv, __le32 base,
 				const void *buf, size_t len)
 {
-	p54spi_write16(priv, SPI_ADRS_DMA_WRITE_CTRL,
-		       cpu_to_le16(SPI_DMA_WRITE_CTRL_ENABLE));
-
 	if (!p54spi_wait_bit(priv, SPI_ADRS_DMA_WRITE_CTRL,
 			     cpu_to_le32(HOST_ALLOWED))) {
 		dev_err(&priv->spi->dev, "spi_write_dma not allowed "
@@ -177,6 +172,9 @@ static int p54spi_spi_write_dma(struct p54s_priv *priv, __le32 base,
 		return -EAGAIN;
 	}
 
+	p54spi_write16(priv, SPI_ADRS_DMA_WRITE_CTRL,
+		       cpu_to_le16(SPI_DMA_WRITE_CTRL_ENABLE));
+
 	p54spi_write16(priv, SPI_ADRS_DMA_WRITE_LEN, cpu_to_le16(len));
 	p54spi_write32(priv, SPI_ADRS_DMA_WRITE_BASE, base);
 	p54spi_spi_write(priv, SPI_ADRS_DMA_DATA, buf, len);
-- 
1.6.0.6

[PATCH 2/5] p54spi: cosmetic fixes: use even byte count in SPI write; drop unused interrupt status read

[Max Filippov]

When SPI write of odd length is requested, p54spi_write splits it
into two parts: one for all data, except the last byte, and one
for last byte and padding byte. Unfortunately, the length of
first part is not amended. It works because all meaningful bytes
have proper value and the last byte of odd length SPI write
transaction is ignored.

p54spi_work has dummy HOST_INTERRUPTS register read at the end.
Drop it, as its result is not used and it has no side effects.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 drivers/net/wireless/p54/p54spi.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/p54/p54spi.c b/drivers/net/wireless/p54/p54spi.c
index ff92bc3..3991977 100644
--- a/drivers/net/wireless/p54/p54spi.c
+++ b/drivers/net/wireless/p54/p54spi.c
@@ -82,7 +82,7 @@ static void p54spi_spi_write(struct p54s_priv *priv, u8 address,
 	spi_message_add_tail(&t[0], &m);
 
 	t[1].tx_buf = buf;
-	t[1].len = len;
+	t[1].len = len & ~1;
 	spi_message_add_tail(&t[1], &m);
 
 	if (len % 2) {
@@ -527,11 +527,6 @@ static void p54spi_work(struct work_struct *work)
 	}
 
 	ret = p54spi_wq_tx(priv);
-	if (ret < 0)
-		goto out;
-
-	ints = p54spi_read32(priv, SPI_ADRS_HOST_INTERRUPTS);
-
 out:
 	mutex_unlock(&priv->mutex);
 }
-- 
1.6.0.6

[PATCH 3/5] p54spi: return status of p54spi_wakeup

[Max Filippov]

Return whether wakeup operation succeeded.
Make use of this return value.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 drivers/net/wireless/p54/p54spi.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/p54/p54spi.c b/drivers/net/wireless/p54/p54spi.c
index 3991977..13566b1 100644
--- a/drivers/net/wireless/p54/p54spi.c
+++ b/drivers/net/wireless/p54/p54spi.c
@@ -311,7 +311,7 @@ static inline void p54spi_int_ack(struct p54s_priv *priv, u32 val)
 	p54spi_write32(priv, SPI_ADRS_HOST_INT_ACK, cpu_to_le32(val));
 }
 
-static void p54spi_wakeup(struct p54s_priv *priv)
+static int p54spi_wakeup(struct p54s_priv *priv)
 {
 	/* wake the chip */
 	p54spi_write32(priv, SPI_ADRS_ARM_INTERRUPTS,
@@ -321,13 +321,11 @@ static void p54spi_wakeup(struct p54s_priv *priv)
 	if (!p54spi_wait_bit(priv, SPI_ADRS_HOST_INTERRUPTS,
 			     cpu_to_le32(SPI_HOST_INT_READY))) {
 		dev_err(&priv->spi->dev, "INT_READY timeout\n");
-		goto out;
+		return -EBUSY;
 	}
 
 	p54spi_int_ack(priv, SPI_HOST_INT_READY);
-
-out:
-	return;
+	return 0;
 }
 
 static inline void p54spi_sleep(struct p54s_priv *priv)
@@ -360,7 +358,8 @@ static int p54spi_rx(struct p54s_priv *priv)
 	struct sk_buff *skb;
 	u16 len;
 
-	p54spi_wakeup(priv);
+	if (p54spi_wakeup(priv) < 0)
+		return -EBUSY;
 
 	/* dummy read to flush SPI DMA controller bug */
 	p54spi_read16(priv, SPI_ADRS_GEN_PURP_1);
@@ -411,7 +410,8 @@ static int p54spi_tx_frame(struct p54s_priv *priv, struct sk_buff *skb)
 	struct p54_hdr *hdr = (struct p54_hdr *) skb->data;
 	int ret = 0;
 
-	p54spi_wakeup(priv);
+	if (p54spi_wakeup(priv) < 0)
+		return -EBUSY;
 
 	ret = p54spi_spi_write_dma(priv, hdr->req_id, skb->data, skb->len);
 	if (ret < 0)
-- 
1.6.0.6

[PATCH 4/5] p54spi: always call p54spi_sleep in p54spi_tx_frame if p54spi_wakeup succeeded

[Max Filippov]

Put chip into sleep state, once it's been awaken.
Also, propagate error code to the caller.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 drivers/net/wireless/p54/p54spi.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/p54/p54spi.c b/drivers/net/wireless/p54/p54spi.c
index 13566b1..9e0c1a2 100644
--- a/drivers/net/wireless/p54/p54spi.c
+++ b/drivers/net/wireless/p54/p54spi.c
@@ -420,16 +420,16 @@ static int p54spi_tx_frame(struct p54s_priv *priv, struct sk_buff *skb)
 	if (!p54spi_wait_bit(priv, SPI_ADRS_HOST_INTERRUPTS,
 			     cpu_to_le32(SPI_HOST_INT_WR_READY))) {
 		dev_err(&priv->spi->dev, "WR_READY timeout\n");
-		ret = -1;
+		ret = -EAGAIN;
 		goto out;
 	}
 
 	p54spi_int_ack(priv, SPI_HOST_INT_WR_READY);
-	p54spi_sleep(priv);
 
 	if (FREE_AFTER_TX(skb))
 		p54_free_skb(priv->hw, skb);
 out:
+	p54spi_sleep(priv);
 	return ret;
 }
 
-- 
1.6.0.6

[PATCH 5/5] p54spi: use firmware/DMA bug workaround that work under hight load in p54spi_rx

[Max Filippov]

Under high load first data word, read after available data size
is sometimes lost in p54spi_rx. It seems to depend on frequency
of interrupts and latency of data read request relatively to
'data available' interrupt. The worst consequence of this bug
is loss of packet transmission acknowledgement, which in turn
causes overflow of tx queues and permanent link loss.

Read data size and first data word in one SPI transaction.
No packets from LMAC should have length less than 1 word,
so this shouldn't interfere with the next read transaction.

Also call p54spi_sleep if p54spi_wake succeeded.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 drivers/net/wireless/p54/p54spi.c |   28 ++++++++++++++++++++--------
 1 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/p54/p54spi.c b/drivers/net/wireless/p54/p54spi.c
index 9e0c1a2..fe90f88 100644
--- a/drivers/net/wireless/p54/p54spi.c
+++ b/drivers/net/wireless/p54/p54spi.c
@@ -357,32 +357,44 @@ static int p54spi_rx(struct p54s_priv *priv)
 {
 	struct sk_buff *skb;
 	u16 len;
+	u16 rx_head[2];
+#define READAHEAD_SZ (sizeof(rx_head)-sizeof(u16))
 
 	if (p54spi_wakeup(priv) < 0)
 		return -EBUSY;
 
-	/* dummy read to flush SPI DMA controller bug */
-	p54spi_read16(priv, SPI_ADRS_GEN_PURP_1);
-
-	len = p54spi_read16(priv, SPI_ADRS_DMA_DATA);
+	/* Read data size and first data word in one SPI transaction
+	 * This is workaround for firmware/DMA bug,
+	 * when first data word gets lost under high load.
+	 */
+	p54spi_spi_read(priv, SPI_ADRS_DMA_DATA, rx_head, sizeof(rx_head));
+	len = rx_head[0];
 
 	if (len == 0) {
-		dev_err(&priv->spi->dev, "rx request of zero bytes");
+		p54spi_sleep(priv);
+		dev_err(&priv->spi->dev, "rx request of zero bytes\n");
 		return 0;
 	}
 
-
 	/* Firmware may insert up to 4 padding bytes after the lmac header,
 	 * but it does not amend the size of SPI data transfer.
 	 * Such packets has correct data size in header, thus referencing
 	 * past the end of allocated skb. Reserve extra 4 bytes for this case */
 	skb = dev_alloc_skb(len + 4);
 	if (!skb) {
+		p54spi_sleep(priv);
 		dev_err(&priv->spi->dev, "could not alloc skb");
-		return 0;
+		return -ENOMEM;
 	}
 
-	p54spi_spi_read(priv, SPI_ADRS_DMA_DATA, skb_put(skb, len), len);
+	if (len <= READAHEAD_SZ) {
+		memcpy(skb_put(skb, len), rx_head + 1, len);
+	} else {
+		memcpy(skb_put(skb, READAHEAD_SZ), rx_head + 1, READAHEAD_SZ);
+		p54spi_spi_read(priv, SPI_ADRS_DMA_DATA,
+				skb_put(skb, len - READAHEAD_SZ),
+				len - READAHEAD_SZ);
+	}
 	p54spi_sleep(priv);
 	/* Put additional bytes to compensate for the possible
 	 * alignment-caused truncation */
-- 
1.6.0.6

Re: [PATCH 5/5] p54spi: use firmware/DMA bug workaround that work under hight load in p54spi_rx

[Kalle Valo]

Max Filippov <jcmvbkbc@gmail.com> writes:

> Under high load first data word, read after available data size
> is sometimes lost in p54spi_rx. It seems to depend on frequency
> of interrupts and latency of data read request relatively to
> 'data available' interrupt. The worst consequence of this bug
> is loss of packet transmission acknowledgement, which in turn
> causes overflow of tx queues and permanent link loss.
>
> Read data size and first data word in one SPI transaction.
> No packets from LMAC should have length less than 1 word,
> so this shouldn't interfere with the next read transaction.

There is a bug in omap2_mcspi which corrupts some DMA transfers, I don't
know if the bug you see is this one or something else. I'll send the
patch to linux-omap sometime later this week.

-- 
Kalle Valo

Re: [PATCH 5/5] p54spi: use firmware/DMA bug workaround that work under hight load in p54spi_rx

[Max Filippov]

> Max Filippov <jcmvbkbc@gmail.com> writes:
> > Under high load first data word, read after available data size
> > is sometimes lost in p54spi_rx. It seems to depend on frequency
> > of interrupts and latency of data read request relatively to
> > 'data available' interrupt. The worst consequence of this bug
> > is loss of packet transmission acknowledgement, which in turn
> > causes overflow of tx queues and permanent link loss.
> >
> > Read data size and first data word in one SPI transaction.
> > No packets from LMAC should have length less than 1 word,
> > so this shouldn't interfere with the next read transaction.
>
> There is a bug in omap2_mcspi which corrupts some DMA transfers, I don't
> know if the bug you see is this one or something else. I'll send the
> patch to linux-omap sometime later this week.

I guess it will be easy to check whether your patch changes something in 
original p54spi behavior. I'll test this.

Thanks.
-- Max