All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] rtl818x: port fixes from rtl8187se merge process
@ 2014-02-05 21:38 andrea merello
  2014-02-05 21:38 ` [PATCH 1/4] In rtl8180/rtl8187 drivers, few register bit definitions have misleading names. Fix them andrea merello
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: andrea merello @ 2014-02-05 21:38 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Larry.Finger, andrea merello

While working on rtl818x driver for adding rtl8187se support I have
made some fixes and improvements unrelated to rtl8187se support itself.

In this first patch set I include the following four changes:

andrea.merello (4):
  In rtl8180/rtl8187 drivers, few register bit definitions have
    misleading names. Fix them
  Add error check for pci_map_single return value in rtl8180 RX path
  Add error check for pci_map_single return value in rtl8180 TX path
  Write rtl8180 beacon interval register when beacon interval changes

 drivers/net/wireless/rtl818x/rtl8180/dev.c | 37 +++++++++++++++++++++++-------
 drivers/net/wireless/rtl818x/rtl8187/dev.c | 16 ++++++-------
 drivers/net/wireless/rtl818x/rtl818x.h     | 12 +++++-----
 3 files changed, 43 insertions(+), 22 deletions(-)

-- 
1.8.3.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/4] In rtl8180/rtl8187 drivers, few register bit definitions have misleading names. Fix them
  2014-02-05 21:38 [PATCH 0/4] rtl818x: port fixes from rtl8187se merge process andrea merello
@ 2014-02-05 21:38 ` andrea merello
  2014-02-06 20:30   ` Larry Finger
  2014-02-05 21:38 ` [PATCH 2/4] Add error check for pci_map_single return value in rtl8180 RX path andrea merello
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: andrea merello @ 2014-02-05 21:38 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Larry.Finger, andrea.merello

From: "andrea.merello" <andrea.merello@gmail.com>

- names of form FOOBAR_SHIFT, suggesting they should be used as
  shift offset, for example
  reg |= (1 << ENABLE_FOO_SHIFT).

  However they are actually defined as (1 << x) and thus they are
  used (correctly) like
  reg |= ENABLE_FOO_SHIFT;

- RTL818X_TX_CONF_HW_SEQNUM  that actually means _disable_ hardware
  sequence number

This patch kills the misleading _SHIFT suffix and changes
RTL818X_TX_CONF_HW_SEQNUM with RTL818X_TX_CONF_SW_SEQNUM

Signed-off-by: andrea.merello <andrea.merello@gmail.com>
Signed-off-by: andrea merello <andrea.merello@gmail.com>
---
 drivers/net/wireless/rtl818x/rtl8180/dev.c | 10 +++++-----
 drivers/net/wireless/rtl818x/rtl8187/dev.c | 16 ++++++++--------
 drivers/net/wireless/rtl818x/rtl818x.h     | 12 ++++++------
 3 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/rtl818x/rtl8180/dev.c b/drivers/net/wireless/rtl818x/rtl8180/dev.c
index 8ec17aa..79b9398 100644
--- a/drivers/net/wireless/rtl818x/rtl8180/dev.c
+++ b/drivers/net/wireless/rtl818x/rtl8180/dev.c
@@ -602,13 +602,13 @@ static int rtl8180_start(struct ieee80211_hw *dev)
 
 	if (priv->r8185) {
 		reg = rtl818x_ioread8(priv, &priv->map->CW_CONF);
-		reg &= ~RTL818X_CW_CONF_PERPACKET_CW_SHIFT;
-		reg |= RTL818X_CW_CONF_PERPACKET_RETRY_SHIFT;
+		reg &= ~RTL818X_CW_CONF_PERPACKET_CW;
+		reg |= RTL818X_CW_CONF_PERPACKET_RETRY;
 		rtl818x_iowrite8(priv, &priv->map->CW_CONF, reg);
 
 		reg = rtl818x_ioread8(priv, &priv->map->TX_AGC_CTL);
-		reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT;
-		reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL_SHIFT;
+		reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_GAIN;
+		reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL;
 		reg |=  RTL818X_TX_AGC_CTL_FEEDBACK_ANT;
 		rtl818x_iowrite8(priv, &priv->map->TX_AGC_CTL, reg);
 
@@ -623,7 +623,7 @@ static int rtl8180_start(struct ieee80211_hw *dev)
 	if (priv->r8185)
 		reg &= ~RTL818X_TX_CONF_PROBE_DTS;
 	else
-		reg &= ~RTL818X_TX_CONF_HW_SEQNUM;
+		reg &= ~RTL818X_TX_CONF_SW_SEQNUM;
 
 	/* different meaning, same value on both rtl8185 and rtl8180 */
 	reg &= ~RTL818X_TX_CONF_SAT_HWPLCP;
diff --git a/drivers/net/wireless/rtl818x/rtl8187/dev.c b/drivers/net/wireless/rtl818x/rtl8187/dev.c
index fd78df8..b0b1730 100644
--- a/drivers/net/wireless/rtl818x/rtl8187/dev.c
+++ b/drivers/net/wireless/rtl818x/rtl8187/dev.c
@@ -785,7 +785,7 @@ static int rtl8187b_init_hw(struct ieee80211_hw *dev)
 	rtl818x_iowrite16(priv, (__le16 *)0xFF34, 0x0FFF);
 
 	reg = rtl818x_ioread8(priv, &priv->map->CW_CONF);
-	reg |= RTL818X_CW_CONF_PERPACKET_RETRY_SHIFT;
+	reg |= RTL818X_CW_CONF_PERPACKET_RETRY;
 	rtl818x_iowrite8(priv, &priv->map->CW_CONF, reg);
 
 	/* Auto Rate Fallback Register (ARFR): 1M-54M setting */
@@ -943,13 +943,13 @@ static int rtl8187_start(struct ieee80211_hw *dev)
 		rtl818x_iowrite32(priv, &priv->map->RX_CONF, reg);
 
 		reg = rtl818x_ioread8(priv, &priv->map->TX_AGC_CTL);
-		reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT;
-		reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL_SHIFT;
+		reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_GAIN;
+		reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL;
 		reg &= ~RTL818X_TX_AGC_CTL_FEEDBACK_ANT;
 		rtl818x_iowrite8(priv, &priv->map->TX_AGC_CTL, reg);
 
 		rtl818x_iowrite32(priv, &priv->map->TX_CONF,
-				  RTL818X_TX_CONF_HW_SEQNUM |
+				  RTL818X_TX_CONF_SW_SEQNUM |
 				  RTL818X_TX_CONF_DISREQQSIZE |
 				  (RETRY_COUNT << 8  /* short retry limit */) |
 				  (RETRY_COUNT << 0  /* long retry limit */) |
@@ -986,13 +986,13 @@ static int rtl8187_start(struct ieee80211_hw *dev)
 	rtl818x_iowrite32(priv, &priv->map->RX_CONF, reg);
 
 	reg = rtl818x_ioread8(priv, &priv->map->CW_CONF);
-	reg &= ~RTL818X_CW_CONF_PERPACKET_CW_SHIFT;
-	reg |= RTL818X_CW_CONF_PERPACKET_RETRY_SHIFT;
+	reg &= ~RTL818X_CW_CONF_PERPACKET_CW;
+	reg |= RTL818X_CW_CONF_PERPACKET_RETRY;
 	rtl818x_iowrite8(priv, &priv->map->CW_CONF, reg);
 
 	reg = rtl818x_ioread8(priv, &priv->map->TX_AGC_CTL);
-	reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT;
-	reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL_SHIFT;
+	reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_GAIN;
+	reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL;
 	reg &= ~RTL818X_TX_AGC_CTL_FEEDBACK_ANT;
 	rtl818x_iowrite8(priv, &priv->map->TX_AGC_CTL, reg);
 
diff --git a/drivers/net/wireless/rtl818x/rtl818x.h b/drivers/net/wireless/rtl818x/rtl818x.h
index ce23dfd..89fe1d3 100644
--- a/drivers/net/wireless/rtl818x/rtl818x.h
+++ b/drivers/net/wireless/rtl818x/rtl818x.h
@@ -71,7 +71,7 @@ struct rtl818x_csr {
 #define RTL818X_TX_CONF_HWVER_MASK	(7 << 25)
 #define RTL818X_TX_CONF_DISREQQSIZE	(1 << 28)
 #define RTL818X_TX_CONF_PROBE_DTS	(1 << 29)
-#define RTL818X_TX_CONF_HW_SEQNUM	(1 << 30)
+#define RTL818X_TX_CONF_SW_SEQNUM	(1 << 30)
 #define RTL818X_TX_CONF_CW_MIN		(1 << 31)
 	__le32	RX_CONF;
 #define RTL818X_RX_CONF_MONITOR		(1 <<  0)
@@ -144,9 +144,9 @@ struct rtl818x_csr {
 	__le32	HSSI_PARA;
 	u8	reserved_13[4];
 	u8	TX_AGC_CTL;
-#define RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT		(1 << 0)
-#define RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL_SHIFT	(1 << 1)
-#define RTL818X_TX_AGC_CTL_FEEDBACK_ANT			(1 << 2)
+#define RTL818X_TX_AGC_CTL_PERPACKET_GAIN	(1 << 0)
+#define RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL	(1 << 1)
+#define RTL818X_TX_AGC_CTL_FEEDBACK_ANT		(1 << 2)
 	u8	TX_GAIN_CCK;
 	u8	TX_GAIN_OFDM;
 	u8	TX_ANTENNA;
@@ -158,8 +158,8 @@ struct rtl818x_csr {
 	u8	SLOT;
 	u8	reserved_16[5];
 	u8	CW_CONF;
-#define RTL818X_CW_CONF_PERPACKET_CW_SHIFT	(1 << 0)
-#define RTL818X_CW_CONF_PERPACKET_RETRY_SHIFT	(1 << 1)
+#define RTL818X_CW_CONF_PERPACKET_CW	(1 << 0)
+#define RTL818X_CW_CONF_PERPACKET_RETRY	(1 << 1)
 	u8	CW_VAL;
 	u8	RATE_FALLBACK;
 #define RTL818X_RATE_FALLBACK_ENABLE	(1 << 7)
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/4]  Add error check for pci_map_single return value in rtl8180 RX path
  2014-02-05 21:38 [PATCH 0/4] rtl818x: port fixes from rtl8187se merge process andrea merello
  2014-02-05 21:38 ` [PATCH 1/4] In rtl8180/rtl8187 drivers, few register bit definitions have misleading names. Fix them andrea merello
@ 2014-02-05 21:38 ` andrea merello
  2014-02-06 20:33   ` Larry Finger
  2014-02-05 21:38 ` [PATCH 3/4] Add error check for pci_map_single return value in rtl8180 TX path andrea merello
  2014-02-05 21:38 ` [PATCH 4/4] Write beacon interval register when beacon interval changes andrea merello
  3 siblings, 1 reply; 8+ messages in thread
From: andrea merello @ 2014-02-05 21:38 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Larry.Finger, andrea.merello

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>
---
 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:
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/4] Add error check for pci_map_single return value in rtl8180 TX path
  2014-02-05 21:38 [PATCH 0/4] rtl818x: port fixes from rtl8187se merge process andrea merello
  2014-02-05 21:38 ` [PATCH 1/4] In rtl8180/rtl8187 drivers, few register bit definitions have misleading names. Fix them andrea merello
  2014-02-05 21:38 ` [PATCH 2/4] Add error check for pci_map_single return value in rtl8180 RX path andrea merello
@ 2014-02-05 21:38 ` andrea merello
  2014-02-06 20:35   ` Larry Finger
  2014-02-05 21:38 ` [PATCH 4/4] Write beacon interval register when beacon interval changes andrea merello
  3 siblings, 1 reply; 8+ messages in thread
From: andrea merello @ 2014-02-05 21:38 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Larry.Finger, andrea.merello

From: "andrea.merello" <andrea.merello@gmail.com>

Orignal code will not detect a DMA mapping failure, causing the HW
to attempt a DMA from an invalid address.

This patch add the error check and eventually simply drops the TX
packet if we can't map it for DMA.

Signed-off-by: andrea.merello <andrea.merello@gmail.com>
Signed-off-by: andrea merello <andrea.merello@gmail.com>
---
 drivers/net/wireless/rtl818x/rtl8180/dev.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/wireless/rtl818x/rtl8180/dev.c b/drivers/net/wireless/rtl818x/rtl8180/dev.c
index 1c6ac25..9645ed2 100644
--- a/drivers/net/wireless/rtl818x/rtl8180/dev.c
+++ b/drivers/net/wireless/rtl818x/rtl8180/dev.c
@@ -276,6 +276,13 @@ static void rtl8180_tx(struct ieee80211_hw *dev,
 	mapping = pci_map_single(priv->pdev, skb->data,
 				 skb->len, PCI_DMA_TODEVICE);
 
+	if (pci_dma_mapping_error(priv->pdev, mapping)) {
+		kfree_skb(skb);
+		dev_err(&priv->pdev->dev, "TX DMA mapping error\n");
+		return;
+
+	}
+
 	tx_flags = RTL818X_TX_DESC_FLAG_OWN | RTL818X_TX_DESC_FLAG_FS |
 		   RTL818X_TX_DESC_FLAG_LS |
 		   (ieee80211_get_tx_rate(dev, info)->hw_value << 24) |
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/4] Write beacon interval register when beacon interval changes
  2014-02-05 21:38 [PATCH 0/4] rtl818x: port fixes from rtl8187se merge process andrea merello
                   ` (2 preceding siblings ...)
  2014-02-05 21:38 ` [PATCH 3/4] Add error check for pci_map_single return value in rtl8180 TX path andrea merello
@ 2014-02-05 21:38 ` andrea merello
  3 siblings, 0 replies; 8+ messages in thread
From: andrea merello @ 2014-02-05 21:38 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Larry.Finger, andrea.merello

From: "andrea.merello" <andrea.merello@gmail.com>

HW Specifications says this register has to be written whenever the NIC joins
(or creates) a network (BSS and IBSS).

I suspect that not writing the beacon interval register is probably not a
big issue (probably related to functions not used in the driver right now).
Possibly power-save and HW beaconing.
But I'm not sure it is only needed for that purposes..

On the other hand I'm sure writing it will not hurt anyway.

This patch adds register write on beacon interval changes.

Signed-off-by: andrea.merello <andrea.merello@gmail.com>
Signed-off-by: andrea merello <andrea.merello@gmail.com>
---
 drivers/net/wireless/rtl818x/rtl8180/dev.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/rtl818x/rtl8180/dev.c b/drivers/net/wireless/rtl818x/rtl8180/dev.c
index 9645ed2..88f3afd 100644
--- a/drivers/net/wireless/rtl818x/rtl8180/dev.c
+++ b/drivers/net/wireless/rtl818x/rtl8180/dev.c
@@ -806,6 +806,10 @@ static void rtl8180_bss_info_changed(struct ieee80211_hw *dev,
 
 	vif_priv = (struct rtl8180_vif *)&vif->drv_priv;
 
+	if (changed & BSS_CHANGED_BEACON_INT)
+		rtl818x_iowrite16(priv, &priv->map->BEACON_INTERVAL,
+				  info->beacon_int);
+
 	if (changed & BSS_CHANGED_BSSID) {
 		for (i = 0; i < ETH_ALEN; i++)
 			rtl818x_iowrite8(priv, &priv->map->BSSID[i],
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/4] In rtl8180/rtl8187 drivers, few register bit definitions have misleading names. Fix them
  2014-02-05 21:38 ` [PATCH 1/4] In rtl8180/rtl8187 drivers, few register bit definitions have misleading names. Fix them andrea merello
@ 2014-02-06 20:30   ` Larry Finger
  0 siblings, 0 replies; 8+ messages in thread
From: Larry Finger @ 2014-02-06 20:30 UTC (permalink / raw)
  To: andrea merello, linville; +Cc: linux-wireless

On 02/05/2014 03:38 PM, andrea merello wrote:
> From: "andrea.merello" <andrea.merello@gmail.com>
>
> - names of form FOOBAR_SHIFT, suggesting they should be used as
>    shift offset, for example
>    reg |= (1 << ENABLE_FOO_SHIFT).
>
>    However they are actually defined as (1 << x) and thus they are
>    used (correctly) like
>    reg |= ENABLE_FOO_SHIFT;
>
> - RTL818X_TX_CONF_HW_SEQNUM  that actually means _disable_ hardware
>    sequence number
>
> This patch kills the misleading _SHIFT suffix and changes
> RTL818X_TX_CONF_HW_SEQNUM with RTL818X_TX_CONF_SW_SEQNUM
>
> Signed-off-by: andrea.merello <andrea.merello@gmail.com>
> Signed-off-by: andrea merello <andrea.merello@gmail.com>
> ---

Acked-by: Larry Finger <Larry.Finger@lwfinger.net>

Larry

>   drivers/net/wireless/rtl818x/rtl8180/dev.c | 10 +++++-----
>   drivers/net/wireless/rtl818x/rtl8187/dev.c | 16 ++++++++--------
>   drivers/net/wireless/rtl818x/rtl818x.h     | 12 ++++++------
>   3 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/wireless/rtl818x/rtl8180/dev.c b/drivers/net/wireless/rtl818x/rtl8180/dev.c
> index 8ec17aa..79b9398 100644
> --- a/drivers/net/wireless/rtl818x/rtl8180/dev.c
> +++ b/drivers/net/wireless/rtl818x/rtl8180/dev.c
> @@ -602,13 +602,13 @@ static int rtl8180_start(struct ieee80211_hw *dev)
>
>   	if (priv->r8185) {
>   		reg = rtl818x_ioread8(priv, &priv->map->CW_CONF);
> -		reg &= ~RTL818X_CW_CONF_PERPACKET_CW_SHIFT;
> -		reg |= RTL818X_CW_CONF_PERPACKET_RETRY_SHIFT;
> +		reg &= ~RTL818X_CW_CONF_PERPACKET_CW;
> +		reg |= RTL818X_CW_CONF_PERPACKET_RETRY;
>   		rtl818x_iowrite8(priv, &priv->map->CW_CONF, reg);
>
>   		reg = rtl818x_ioread8(priv, &priv->map->TX_AGC_CTL);
> -		reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT;
> -		reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL_SHIFT;
> +		reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_GAIN;
> +		reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL;
>   		reg |=  RTL818X_TX_AGC_CTL_FEEDBACK_ANT;
>   		rtl818x_iowrite8(priv, &priv->map->TX_AGC_CTL, reg);
>
> @@ -623,7 +623,7 @@ static int rtl8180_start(struct ieee80211_hw *dev)
>   	if (priv->r8185)
>   		reg &= ~RTL818X_TX_CONF_PROBE_DTS;
>   	else
> -		reg &= ~RTL818X_TX_CONF_HW_SEQNUM;
> +		reg &= ~RTL818X_TX_CONF_SW_SEQNUM;
>
>   	/* different meaning, same value on both rtl8185 and rtl8180 */
>   	reg &= ~RTL818X_TX_CONF_SAT_HWPLCP;
> diff --git a/drivers/net/wireless/rtl818x/rtl8187/dev.c b/drivers/net/wireless/rtl818x/rtl8187/dev.c
> index fd78df8..b0b1730 100644
> --- a/drivers/net/wireless/rtl818x/rtl8187/dev.c
> +++ b/drivers/net/wireless/rtl818x/rtl8187/dev.c
> @@ -785,7 +785,7 @@ static int rtl8187b_init_hw(struct ieee80211_hw *dev)
>   	rtl818x_iowrite16(priv, (__le16 *)0xFF34, 0x0FFF);
>
>   	reg = rtl818x_ioread8(priv, &priv->map->CW_CONF);
> -	reg |= RTL818X_CW_CONF_PERPACKET_RETRY_SHIFT;
> +	reg |= RTL818X_CW_CONF_PERPACKET_RETRY;
>   	rtl818x_iowrite8(priv, &priv->map->CW_CONF, reg);
>
>   	/* Auto Rate Fallback Register (ARFR): 1M-54M setting */
> @@ -943,13 +943,13 @@ static int rtl8187_start(struct ieee80211_hw *dev)
>   		rtl818x_iowrite32(priv, &priv->map->RX_CONF, reg);
>
>   		reg = rtl818x_ioread8(priv, &priv->map->TX_AGC_CTL);
> -		reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT;
> -		reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL_SHIFT;
> +		reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_GAIN;
> +		reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL;
>   		reg &= ~RTL818X_TX_AGC_CTL_FEEDBACK_ANT;
>   		rtl818x_iowrite8(priv, &priv->map->TX_AGC_CTL, reg);
>
>   		rtl818x_iowrite32(priv, &priv->map->TX_CONF,
> -				  RTL818X_TX_CONF_HW_SEQNUM |
> +				  RTL818X_TX_CONF_SW_SEQNUM |
>   				  RTL818X_TX_CONF_DISREQQSIZE |
>   				  (RETRY_COUNT << 8  /* short retry limit */) |
>   				  (RETRY_COUNT << 0  /* long retry limit */) |
> @@ -986,13 +986,13 @@ static int rtl8187_start(struct ieee80211_hw *dev)
>   	rtl818x_iowrite32(priv, &priv->map->RX_CONF, reg);
>
>   	reg = rtl818x_ioread8(priv, &priv->map->CW_CONF);
> -	reg &= ~RTL818X_CW_CONF_PERPACKET_CW_SHIFT;
> -	reg |= RTL818X_CW_CONF_PERPACKET_RETRY_SHIFT;
> +	reg &= ~RTL818X_CW_CONF_PERPACKET_CW;
> +	reg |= RTL818X_CW_CONF_PERPACKET_RETRY;
>   	rtl818x_iowrite8(priv, &priv->map->CW_CONF, reg);
>
>   	reg = rtl818x_ioread8(priv, &priv->map->TX_AGC_CTL);
> -	reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT;
> -	reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL_SHIFT;
> +	reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_GAIN;
> +	reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL;
>   	reg &= ~RTL818X_TX_AGC_CTL_FEEDBACK_ANT;
>   	rtl818x_iowrite8(priv, &priv->map->TX_AGC_CTL, reg);
>
> diff --git a/drivers/net/wireless/rtl818x/rtl818x.h b/drivers/net/wireless/rtl818x/rtl818x.h
> index ce23dfd..89fe1d3 100644
> --- a/drivers/net/wireless/rtl818x/rtl818x.h
> +++ b/drivers/net/wireless/rtl818x/rtl818x.h
> @@ -71,7 +71,7 @@ struct rtl818x_csr {
>   #define RTL818X_TX_CONF_HWVER_MASK	(7 << 25)
>   #define RTL818X_TX_CONF_DISREQQSIZE	(1 << 28)
>   #define RTL818X_TX_CONF_PROBE_DTS	(1 << 29)
> -#define RTL818X_TX_CONF_HW_SEQNUM	(1 << 30)
> +#define RTL818X_TX_CONF_SW_SEQNUM	(1 << 30)
>   #define RTL818X_TX_CONF_CW_MIN		(1 << 31)
>   	__le32	RX_CONF;
>   #define RTL818X_RX_CONF_MONITOR		(1 <<  0)
> @@ -144,9 +144,9 @@ struct rtl818x_csr {
>   	__le32	HSSI_PARA;
>   	u8	reserved_13[4];
>   	u8	TX_AGC_CTL;
> -#define RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT		(1 << 0)
> -#define RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL_SHIFT	(1 << 1)
> -#define RTL818X_TX_AGC_CTL_FEEDBACK_ANT			(1 << 2)
> +#define RTL818X_TX_AGC_CTL_PERPACKET_GAIN	(1 << 0)
> +#define RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL	(1 << 1)
> +#define RTL818X_TX_AGC_CTL_FEEDBACK_ANT		(1 << 2)
>   	u8	TX_GAIN_CCK;
>   	u8	TX_GAIN_OFDM;
>   	u8	TX_ANTENNA;
> @@ -158,8 +158,8 @@ struct rtl818x_csr {
>   	u8	SLOT;
>   	u8	reserved_16[5];
>   	u8	CW_CONF;
> -#define RTL818X_CW_CONF_PERPACKET_CW_SHIFT	(1 << 0)
> -#define RTL818X_CW_CONF_PERPACKET_RETRY_SHIFT	(1 << 1)
> +#define RTL818X_CW_CONF_PERPACKET_CW	(1 << 0)
> +#define RTL818X_CW_CONF_PERPACKET_RETRY	(1 << 1)
>   	u8	CW_VAL;
>   	u8	RATE_FALLBACK;
>   #define RTL818X_RATE_FALLBACK_ENABLE	(1 << 7)
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/4]  Add error check for pci_map_single return value in rtl8180 RX path
  2014-02-05 21:38 ` [PATCH 2/4] Add error check for pci_map_single return value in rtl8180 RX path andrea merello
@ 2014-02-06 20:33   ` Larry Finger
  0 siblings, 0 replies; 8+ messages in thread
From: Larry Finger @ 2014-02-06 20:33 UTC (permalink / raw)
  To: andrea merello, linville; +Cc: linux-wireless

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:
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/4] Add error check for pci_map_single return value in rtl8180 TX path
  2014-02-05 21:38 ` [PATCH 3/4] Add error check for pci_map_single return value in rtl8180 TX path andrea merello
@ 2014-02-06 20:35   ` Larry Finger
  0 siblings, 0 replies; 8+ messages in thread
From: Larry Finger @ 2014-02-06 20:35 UTC (permalink / raw)
  To: andrea merello, linville; +Cc: linux-wireless

On 02/05/2014 03:38 PM, andrea merello wrote:
> From: "andrea.merello" <andrea.merello@gmail.com>
>
> Orignal code will not detect a DMA mapping failure, causing the HW
> to attempt a DMA from an invalid address.
>
> This patch add the error check and eventually simply drops the TX
> packet if we can't map it for DMA.
>
> Signed-off-by: andrea.merello <andrea.merello@gmail.com>
> Signed-off-by: andrea merello <andrea.merello@gmail.com>

This code is standard for recovery from an error in pci_map_single().

Acked-by: Larry Finger <Larry.Finger@lwfinger.net>

Larry

> ---
>   drivers/net/wireless/rtl818x/rtl8180/dev.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/wireless/rtl818x/rtl8180/dev.c b/drivers/net/wireless/rtl818x/rtl8180/dev.c
> index 1c6ac25..9645ed2 100644
> --- a/drivers/net/wireless/rtl818x/rtl8180/dev.c
> +++ b/drivers/net/wireless/rtl818x/rtl8180/dev.c
> @@ -276,6 +276,13 @@ static void rtl8180_tx(struct ieee80211_hw *dev,
>   	mapping = pci_map_single(priv->pdev, skb->data,
>   				 skb->len, PCI_DMA_TODEVICE);
>
> +	if (pci_dma_mapping_error(priv->pdev, mapping)) {
> +		kfree_skb(skb);
> +		dev_err(&priv->pdev->dev, "TX DMA mapping error\n");
> +		return;
> +
> +	}
> +
>   	tx_flags = RTL818X_TX_DESC_FLAG_OWN | RTL818X_TX_DESC_FLAG_FS |
>   		   RTL818X_TX_DESC_FLAG_LS |
>   		   (ieee80211_get_tx_rate(dev, info)->hw_value << 24) |
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-02-06 20:35 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-05 21:38 [PATCH 0/4] rtl818x: port fixes from rtl8187se merge process andrea merello
2014-02-05 21:38 ` [PATCH 1/4] In rtl8180/rtl8187 drivers, few register bit definitions have misleading names. Fix them andrea merello
2014-02-06 20:30   ` Larry Finger
2014-02-05 21:38 ` [PATCH 2/4] Add error check for pci_map_single return value in rtl8180 RX path andrea merello
2014-02-06 20:33   ` Larry Finger
2014-02-05 21:38 ` [PATCH 3/4] Add error check for pci_map_single return value in rtl8180 TX path andrea merello
2014-02-06 20:35   ` Larry Finger
2014-02-05 21:38 ` [PATCH 4/4] Write beacon interval register when beacon interval changes andrea merello

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.