Re: [PATCH 2/4] Add error check for pci_map_single return value in rtl8180 RX path

[Larry Finger <Larry.Finger@lwfinger.net>]

On 02/05/2014 03:38 PM, andrea merello wrote:
> From: "andrea.merello" <andrea.merello@gmail.com>
>
> In original code the old RX DMA buffer is unmapped and processed and at the end
> of the isr a new buffer is mapped with pci_map_single and attached to the RX
> descriptor.
>
> If pci_map_single fails then the RX descriptor remains with no valid DMA buffer
> attached.
> In this condition the DMA will target where it shouldn't with obvious evil
> consequences.
>
> Simply avoiding re-arming the descriptor will prevent buggy DMA but it will
> result soon in RX stuck.
>
> This patch move the DMA mapping of the new buffer at the beginning of the ISR
> (and it adds error check for pci_map_single success/fail).
>
> If the DMA mapping fails then we do not unmap the old buffer and we re-arm the
> descriptor without processing it, with the old DMA buffer still attached.
>
> In this way we lose the currently RX-ed packet, but whenever next calls to
> pci_map_single will succeed again,then the RX process will go on without stuck.
>
> Signed-off-by: andrea.merello <andrea.merello@gmail.com>
> Signed-off-by: andrea merello <andrea.merello@gmail.com>

I have no way to test this patch, but it looks OK.

Larry

> ---
>   drivers/net/wireless/rtl818x/rtl8180/dev.c | 16 +++++++++++++---
>   1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/rtl818x/rtl8180/dev.c b/drivers/net/wireless/rtl818x/rtl8180/dev.c
> index 79b9398..1c6ac25 100644
> --- a/drivers/net/wireless/rtl818x/rtl8180/dev.c
> +++ b/drivers/net/wireless/rtl818x/rtl8180/dev.c
> @@ -107,6 +107,7 @@ static void rtl8180_handle_rx(struct ieee80211_hw *dev)
>   	struct rtl8180_priv *priv = dev->priv;
>   	unsigned int count = 32;
>   	u8 signal, agc, sq;
> +	dma_addr_t mapping;
>
>   	while (count--) {
>   		struct rtl8180_rx_desc *entry = &priv->rx_ring[priv->rx_idx];
> @@ -128,6 +129,17 @@ static void rtl8180_handle_rx(struct ieee80211_hw *dev)
>   			if (unlikely(!new_skb))
>   				goto done;
>
> +			mapping = pci_map_single(priv->pdev,
> +					       skb_tail_pointer(new_skb),
> +					       MAX_RX_SIZE, PCI_DMA_FROMDEVICE);
> +
> +			if (pci_dma_mapping_error(priv->pdev, mapping)) {
> +				kfree_skb(new_skb);
> +				dev_err(&priv->pdev->dev, "RX DMA map error\n");
> +
> +				goto done;
> +			}
> +
>   			pci_unmap_single(priv->pdev,
>   					 *((dma_addr_t *)skb->cb),
>   					 MAX_RX_SIZE, PCI_DMA_FROMDEVICE);
> @@ -158,9 +170,7 @@ static void rtl8180_handle_rx(struct ieee80211_hw *dev)
>
>   			skb = new_skb;
>   			priv->rx_buf[priv->rx_idx] = skb;
> -			*((dma_addr_t *) skb->cb) =
> -				pci_map_single(priv->pdev, skb_tail_pointer(skb),
> -					       MAX_RX_SIZE, PCI_DMA_FROMDEVICE);
> +			*((dma_addr_t *) skb->cb) = mapping;
>   		}
>
>   	done:
>