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 <thom...@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 <linvi...@tuxdriver.com>, with Cc: to b43-dev, and the linux-wireless mailing list <linux-wirel...@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 <sta...@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


_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

Reply via email to