net: ks8851: Dequeue RX packets explicitly
authorLukas Wunner <lukas@wunner.de>
Wed, 20 Mar 2019 14:02:00 +0000 (15:02 +0100)
committerDavid S. Miller <davem@davemloft.net>
Wed, 20 Mar 2019 19:43:52 +0000 (12:43 -0700)
The ks8851 driver lets the chip auto-dequeue received packets once they
have been read in full. It achieves that by setting the ADRFE flag in
the RXQCR register ("Auto-Dequeue RXQ Frame Enable").

However if allocation of a packet's socket buffer or retrieval of the
packet over the SPI bus fails, the packet will not have been read in
full and is not auto-dequeued. Such partial retrieval of a packet
confuses the chip's RX queue management:  On the next RX interrupt,
the first packet read from the queue will be the one left there
previously and this one can be retrieved without issues. But for any
newly received packets, the frame header status and byte count registers
(RXFHSR and RXFHBCR) contain bogus values, preventing their retrieval.

The chip allows explicitly dequeueing a packet from the RX queue by
setting the RRXEF flag in the RXQCR register ("Release RX Error Frame").
This could be used to dequeue the packet in case of an error, but if
that error is a failed SPI transfer, it is unknown if the packet was
transferred in full and was auto-dequeued or if it was only transferred
in part and requires an explicit dequeue. The safest approach is thus
to always dequeue packets explicitly and forgo auto-dequeueing.

Without this change, I've witnessed packet retrieval break completely
when an SPI DMA transfer fails, requiring a chip reset. Explicit
dequeueing magically fixes this and makes packet retrieval absolutely
robust for me.

The chip's documentation suggests auto-dequeuing and uses the RRXEF
flag only to dequeue error frames which the driver doesn't want to
retrieve. But that seems to be a fair-weather approach.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Ben Dooks <ben.dooks@codethink.co.uk>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/ethernet/micrel/ks8851.c

index bd6e9014bc74794b9a8a7e680f5b59ea7048382f..a93f8e842c07dd0849741af5190ee740f9f0cb05 100644 (file)
@@ -535,9 +535,8 @@ static void ks8851_rx_pkts(struct ks8851_net *ks)
                /* set dma read address */
                ks8851_wrreg16(ks, KS_RXFDPR, RXFDPR_RXFPAI | 0x00);
 
-               /* start the packet dma process, and set auto-dequeue rx */
-               ks8851_wrreg16(ks, KS_RXQCR,
-                              ks->rc_rxqcr | RXQCR_SDA | RXQCR_ADRFE);
+               /* start DMA access */
+               ks8851_wrreg16(ks, KS_RXQCR, ks->rc_rxqcr | RXQCR_SDA);
 
                if (rxlen > 4) {
                        unsigned int rxalign;
@@ -568,7 +567,8 @@ static void ks8851_rx_pkts(struct ks8851_net *ks)
                        }
                }
 
-               ks8851_wrreg16(ks, KS_RXQCR, ks->rc_rxqcr);
+               /* end DMA access and dequeue packet */
+               ks8851_wrreg16(ks, KS_RXQCR, ks->rc_rxqcr | RXQCR_RRXEF);
        }
 }