[PATCH] b43: use rx desc underrun interrupt
Larry Finger
Larry.Finger at lwfinger.net
Sat Apr 20 14:35:45 EDT 2013
On 04/20/2013 09:14 AM, Thommy wrote:
> Hi,
>
> new try sending in the patch (with pine instead of gmail webclient this
> time). Patch created on latest wireless-testing
>
> BR,
> Thommy Jakobsson
>
> Signed-off-by: Thommy Jakobsson <thommyj at gmail.com>
> ---
> drivers/net/wireless/b43/b43.h | 1 +
> drivers/net/wireless/b43/dma.c | 16 +++++++++++++++
> drivers/net/wireless/b43/dma.h | 4 +++-
> drivers/net/wireless/b43/main.c | 43 ++++++++++++++++-----------------------
> 4 files changed, 38 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/wireless/b43/b43.h b/drivers/net/wireless/b43/b43.h
> index 7f3d461..fdcd00f 100644
> --- a/drivers/net/wireless/b43/b43.h
> +++ b/drivers/net/wireless/b43/b43.h
> @@ -680,6 +680,7 @@ struct b43_noise_calculation {
>
> struct b43_stats {
> u8 link_noise;
> + u32 rxdesc_underruns;
> };
>
> struct b43_key {
> diff --git a/drivers/net/wireless/b43/dma.c b/drivers/net/wireless/b43/dma.c
> index 1221469..2ae00dd 100644
> --- a/drivers/net/wireless/b43/dma.c
> +++ b/drivers/net/wireless/b43/dma.c
> @@ -1733,6 +1733,22 @@ drop_recycle_buffer:
> sync_descbuffer_for_device(ring, dmaaddr, ring->rx_buffersize);
> }
>
> +void b43_dma_rx_discard(struct b43_dmaring *ring)
> +{
> + B43_WARN_ON(ring->tx);
> +
> + /* Device has filled all buffers, drop all packets in buffers
> + * and let TCP decrease speed.
> + * Set index to one desc after the last one
> + * so the device will see all slots as free again
> + */
> + /*
> + *TODO: How to increase rx_drop in mac80211?
> + */
> + b43_dma_write(ring, B43_DMA32_RXINDEX, ring->nr_slots *
> + sizeof(struct b43_dmadesc32));
> +}
> +
> void b43_dma_rx(struct b43_dmaring *ring)
> {
> const struct b43_dma_ops *ops = ring->ops;
> diff --git a/drivers/net/wireless/b43/dma.h b/drivers/net/wireless/b43/dma.h
> index 9fdd198..fed8163 100644
> --- a/drivers/net/wireless/b43/dma.h
> +++ b/drivers/net/wireless/b43/dma.h
> @@ -9,7 +9,7 @@
> /* DMA-Interrupt reasons. */
> #define B43_DMAIRQ_FATALMASK ((1 << 10) | (1 << 11) | (1 << 12) \
> | (1 << 14) | (1 << 15))
> -#define B43_DMAIRQ_NONFATALMASK (1 << 13)
> +#define B43_DMAIRQ_RDESC_UFLOW (1 << 13)
> #define B43_DMAIRQ_RX_DONE (1 << 16)
>
> /*** 32-bit DMA Engine. ***/
> @@ -295,6 +295,8 @@ int b43_dma_tx(struct b43_wldev *dev,
> void b43_dma_handle_txstatus(struct b43_wldev *dev,
> const struct b43_txstatus *status);
>
> +void b43_dma_rx_discard(struct b43_dmaring *ring);
> +
> void b43_dma_rx(struct b43_dmaring *ring);
>
> void b43_dma_direct_fifo_rx(struct b43_wldev *dev,
> diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
> index d377f77..277fe75 100644
> --- a/drivers/net/wireless/b43/main.c
> +++ b/drivers/net/wireless/b43/main.c
> @@ -1902,30 +1902,19 @@ static void b43_do_interrupt_thread(struct b43_wldev *dev)
> }
> }
>
> - if (unlikely(merged_dma_reason & (B43_DMAIRQ_FATALMASK |
> - B43_DMAIRQ_NONFATALMASK))) {
> - if (merged_dma_reason & B43_DMAIRQ_FATALMASK) {
> - b43err(dev->wl, "Fatal DMA error: "
> - "0x%08X, 0x%08X, 0x%08X, "
> - "0x%08X, 0x%08X, 0x%08X\n",
> - dma_reason[0], dma_reason[1],
> - dma_reason[2], dma_reason[3],
> - dma_reason[4], dma_reason[5]);
> - b43err(dev->wl, "This device does not support DMA "
> + if (unlikely(merged_dma_reason & (B43_DMAIRQ_FATALMASK))) {
> + b43err(dev->wl, "Fatal DMA error: "
> + "0x%08X, 0x%08X, 0x%08X, "
> + "0x%08X, 0x%08X, 0x%08X\n",
> + dma_reason[0], dma_reason[1],
> + dma_reason[2], dma_reason[3],
> + dma_reason[4], dma_reason[5]);
> + b43err(dev->wl, "This device does not support DMA "
> "on your system. It will now be switched to PIO.\n");
> - /* Fall back to PIO transfers if we get fatal DMA errors! */
> - dev->use_pio = true;
> - b43_controller_restart(dev, "DMA error");
> - return;
> - }
> - if (merged_dma_reason & B43_DMAIRQ_NONFATALMASK) {
> - b43err(dev->wl, "DMA error: "
> - "0x%08X, 0x%08X, 0x%08X, "
> - "0x%08X, 0x%08X, 0x%08X\n",
> - dma_reason[0], dma_reason[1],
> - dma_reason[2], dma_reason[3],
> - dma_reason[4], dma_reason[5]);
> - }
> + /* Fall back to PIO transfers if we get fatal DMA errors! */
> + dev->use_pio = true;
> + b43_controller_restart(dev, "DMA error");
> + return;
> }
>
> if (unlikely(reason & B43_IRQ_UCODE_DEBUG))
> @@ -1944,6 +1933,10 @@ static void b43_do_interrupt_thread(struct b43_wldev *dev)
> handle_irq_noise(dev);
>
> /* Check the DMA reason registers for received data. */
> + if (dma_reason[0] & B43_DMAIRQ_RDESC_UFLOW) {
> + b43warn(dev->wl, "RX descriptor underrun, dropping packets\n");
> + b43_dma_rx_discard(dev->dma.rx_ring);
> + }
> if (dma_reason[0] & B43_DMAIRQ_RX_DONE) {
> if (b43_using_pio_transfers(dev))
> b43_pio_rx(dev->pio.rx_queue);
> @@ -2001,7 +1994,7 @@ static irqreturn_t b43_do_interrupt(struct b43_wldev *dev)
> return IRQ_NONE;
>
> dev->dma_reason[0] = b43_read32(dev, B43_MMIO_DMA0_REASON)
> - & 0x0001DC00;
> + & 0x0001FC00;
> dev->dma_reason[1] = b43_read32(dev, B43_MMIO_DMA1_REASON)
> & 0x0000DC00;
> dev->dma_reason[2] = b43_read32(dev, B43_MMIO_DMA2_REASON)
> @@ -3130,7 +3123,7 @@ static int b43_chip_init(struct b43_wldev *dev)
> b43_write32(dev, 0x018C, 0x02000000);
> }
> b43_write32(dev, B43_MMIO_GEN_IRQ_REASON, 0x00004000);
> - b43_write32(dev, B43_MMIO_DMA0_IRQ_MASK, 0x0001DC00);
> + b43_write32(dev, B43_MMIO_DMA0_IRQ_MASK, 0x0001FC00);
> b43_write32(dev, B43_MMIO_DMA1_IRQ_MASK, 0x0000DC00);
> b43_write32(dev, B43_MMIO_DMA2_IRQ_MASK, 0x0000DC00);
> b43_write32(dev, B43_MMIO_DMA3_IRQ_MASK, 0x0001DC00);
Thommy,
This version did apply cleanly; however, there are still a couple of things that
need fixing.
With scripts/checkpatch.pl, there is a warning that a quoted string is split
over two lines. Although testing for that condition is a recent addition to the
script, it is probably best to honor that requirement with new code. To do so,
the lines starting at 1907 in main.c should be
b43err(dev->wl,
"Fatal DMA error: 0x%08X, 0x%08X, 0x%08X, 0x%08X,
0x%08X, 0x%08X\n",
My mailer will likely wrap the string, but ignore that. Yes, that line is longer
than 80 characters, but that is OK for quoted strings. The idea is that if they
are not split, it is easier to grep for them. In this case, that is not an
issue, but I prefer to make all new code be free of errors, warnings, and checks.
Your subject and commit message are not appropriate. Have you read
Documentation/SubmittingPatches? The info you have at the start would not be
appropriate for the permanent kernel record. If you want to add such informal
remarks, include a line with --- after the formal commit message, and place
informal remarks below that line. Everything is available at patch submission
time, but the scripts strip it off before it is included in the git
repositories. The b43-dev list is OK for discussions of the patch, but the final
patch needs to be sent to John Linville <linville at tuxdriver.com>, with Cc: to
b43-dev, and the linux-wireless mailing list <linux-wireless at vger.kernel.org>.
When the patch is appropriate for backporting to earlier, stable kernels, and
this one certainly fits that criterion, follow the "Signed-off-by" line with one
that says "Cc: Stable <stable at vger.kernel.org>". That way, the patch will
automatically be included in earlier versions as soon as it is incorporated in
the mainline version (Linus's tree).
Lastly, I have a question about the patch. Is it necessary to log every
underrun? Perhaps you might want to place the b43_warn statement inside an "if
(B43_DEBUG)" conditional. That way the condition will be logged for people
really interested in debugging the driver, but the general user of a distro
kernel will never see the message.
Thanks for finding this issue.
Larry
More information about the b43-dev
mailing list