[PATCH 0/4] rt2x00: fix initialization after legacy driver

[PATCH 0/4] rt2x00: fix initialization after legacy driver

[Jakub Kicinski]

Hi,

I've been testing these patches for a few weeks and I think it's about 
the time to post them.

The main purpose of this set is to make it possible to switch between
rt2800pci and legacy driver w/o rebooting or suspending whole machine.
Today rt2800pci will fail to bring the interface up if it was earlier
operated by legacy drv. I'm sure it happens with rt3090, rt2860 seems
to work fine.

First two patches are just random fixes, third and fourth actually 
fix the issue.

Jakub Kicinski (4):
  rt2x00: broaden PCIE L1-state wakeup configuration
  rt2x00: move disabling of DMA before loading firmware
  rt2x00: initialize queues before giving up due to DMA error
  rt2x00: zero registers of unused TX rings

 drivers/net/wireless/rt2x00/rt2800lib.c |   23 ++++++++++-------------
 drivers/net/wireless/rt2x00/rt2800pci.c |   21 +++++++++++++++------
 2 files changed, 25 insertions(+), 19 deletions(-)

-- 
1.7.7.6

[PATCH 1/4] rt2x00: broaden PCIE L1-state wakeup configuration

[Jakub Kicinski]

Broaden wakeup configuration to all PCIE from rt3xxx on.

Signed-off-by: Jakub Kicinski <kubakici@wp.pl>
---
2011_0406_RT5390_RT5392_Linux_STA_V2.5.0.3_DPO
rtmp_init_inf.c:171

Legacy driver applies this configuration to the following devices:
3071 3090 3572 3592 3562 3062 3390 3593 5390 5370 5392 5372 5360 5362
if they are working on PCIE bus. AFAIK these are all post 2xxx PCIE
devices.
---
 drivers/net/wireless/rt2x00/rt2800lib.c |    4 +---
 drivers/net/wireless/rt2x00/rt2800pci.c |    5 +----
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index 6c0a12e..4722030 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -401,9 +401,7 @@ int rt2800_load_firmware(struct rt2x00_dev *rt2x00dev,
 		return -EBUSY;
 
 	if (rt2x00_is_pci(rt2x00dev)) {
-		if (rt2x00_rt(rt2x00dev, RT3572) ||
-		    rt2x00_rt(rt2x00dev, RT5390) ||
-		    rt2x00_rt(rt2x00dev, RT5392)) {
+		if (rt2x00_is_pcie(rt2x00dev) && rt2x00dev->chip.rt > 0x3000) {
 			rt2800_register_read(rt2x00dev, AUX_CTRL, &reg);
 			rt2x00_set_field32(&reg, AUX_CTRL_FORCE_PCIE_CLK, 1);
 			rt2x00_set_field32(&reg, AUX_CTRL_WAKE_PCIE_EN, 1);
diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index 0397bbf..5a2f68f 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -478,10 +478,7 @@ static int rt2800pci_init_registers(struct rt2x00_dev *rt2x00dev)
 	rt2x00pci_register_write(rt2x00dev, PBF_SYS_CTRL, 0x00000e1f);
 	rt2x00pci_register_write(rt2x00dev, PBF_SYS_CTRL, 0x00000e00);
 
-	if (rt2x00_is_pcie(rt2x00dev) &&
-	    (rt2x00_rt(rt2x00dev, RT3572) ||
-	     rt2x00_rt(rt2x00dev, RT5390) ||
-	     rt2x00_rt(rt2x00dev, RT5392))) {
+	if (rt2x00_is_pcie(rt2x00dev) && rt2x00dev->chip.rt > 0x3000) {
 		rt2x00pci_register_read(rt2x00dev, AUX_CTRL, &reg);
 		rt2x00_set_field32(&reg, AUX_CTRL_FORCE_PCIE_CLK, 1);
 		rt2x00_set_field32(&reg, AUX_CTRL_WAKE_PCIE_EN, 1);
-- 
1.7.7.6

[PATCH 2/4] rt2x00: move disabling of DMA before loading firmware

[Jakub Kicinski]

Legacy driver disables DMA before not after loading firmware.

Signed-off-by: Jakub Kicinski <kubakici@wp.pl>
---
2011_0406_RT5390_RT5392_Linux_STA_V2.5.0.3_DPO
rtmp_init_inf.c:186
---
 drivers/net/wireless/rt2x00/rt2800lib.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index 4722030..c872ba6 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -411,6 +411,14 @@ int rt2800_load_firmware(struct rt2x00_dev *rt2x00dev,
 	}
 
 	/*
+	 * Disable DMA, will be reenabled later when enabling the radio.
+	 */
+	rt2800_register_read(rt2x00dev, WPDMA_GLO_CFG, &reg);
+	rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_TX_DMA, 0);
+	rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_RX_DMA, 0);
+	rt2800_register_write(rt2x00dev, WPDMA_GLO_CFG, reg);
+
+	/*
 	 * Write firmware to the device.
 	 */
 	rt2800_drv_write_firmware(rt2x00dev, data, len);
@@ -431,15 +439,6 @@ int rt2800_load_firmware(struct rt2x00_dev *rt2x00dev,
 	}
 
 	/*
-	 * Disable DMA, will be reenabled later when enabling
-	 * the radio.
-	 */
-	rt2800_register_read(rt2x00dev, WPDMA_GLO_CFG, &reg);
-	rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_TX_DMA, 0);
-	rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_RX_DMA, 0);
-	rt2800_register_write(rt2x00dev, WPDMA_GLO_CFG, reg);
-
-	/*
 	 * Initialize firmware.
 	 */
 	rt2800_register_write(rt2x00dev, H2M_BBP_AGENT, 0);
-- 
1.7.7.6

[PATCH 3/4] rt2x00: initialize queues before giving up due to DMA error

[Jakub Kicinski]

Don't immediately abort .start if DMA is busy before we
initialize the queues. Some drivers do not deinitialize
queues properly and we would fail to take over after them.

This behaviour is consistent with legacy driver.

Signed-off-by: Jakub Kicinski <kubakici@wp.pl>
---
2011_0406_RT5390_RT5392_Linux_STA_V2.5.0.3_DPO
common/rtmp_init.c:2007
---
 drivers/net/wireless/rt2x00/rt2800lib.c |    2 +-
 drivers/net/wireless/rt2x00/rt2800pci.c |    6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index c872ba6..de975ef 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -290,7 +290,7 @@ int rt2800_wait_wpdma_ready(struct rt2x00_dev *rt2x00dev)
 		msleep(10);
 	}
 
-	ERROR(rt2x00dev, "WPDMA TX/RX busy, aborting.\n");
+	ERROR(rt2x00dev, "WPDMA TX/RX busy [0x%08x].\n", reg);
 	return -EACCES;
 }
 EXPORT_SYMBOL_GPL(rt2800_wait_wpdma_ready);
diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index 5a2f68f..f39587b 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -501,8 +501,10 @@ static int rt2800pci_enable_radio(struct rt2x00_dev *rt2x00dev)
 {
 	int retval;
 
-	if (unlikely(rt2800_wait_wpdma_ready(rt2x00dev) ||
-		     rt2800pci_init_queues(rt2x00dev)))
+	/* Wait for DMA, ignore error until we initialize queues. */
+	rt2800_wait_wpdma_ready(rt2x00dev);
+
+	if (unlikely(rt2800pci_init_queues(rt2x00dev)))
 		return -EIO;
 
 	retval = rt2800_enable_radio(rt2x00dev);
-- 
1.7.7.6

[PATCH 4/4] rt2x00: zero registers of unused TX rings

[Jakub Kicinski]

This is needed if we take over after drivers which use those.

Signed-off-by: Jakub Kicinski <kubakici@wp.pl>
---
 drivers/net/wireless/rt2x00/rt2800pci.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index f39587b..1d24124 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -394,6 +394,16 @@ static int rt2800pci_init_queues(struct rt2x00_dev *rt2x00dev)
 	rt2x00pci_register_write(rt2x00dev, TX_CTX_IDX3, 0);
 	rt2x00pci_register_write(rt2x00dev, TX_DTX_IDX3, 0);
 
+	rt2x00pci_register_write(rt2x00dev, TX_BASE_PTR4, 0);
+	rt2x00pci_register_write(rt2x00dev, TX_MAX_CNT4, 0);
+	rt2x00pci_register_write(rt2x00dev, TX_CTX_IDX4, 0);
+	rt2x00pci_register_write(rt2x00dev, TX_DTX_IDX4, 0);
+
+	rt2x00pci_register_write(rt2x00dev, TX_BASE_PTR5, 0);
+	rt2x00pci_register_write(rt2x00dev, TX_MAX_CNT5, 0);
+	rt2x00pci_register_write(rt2x00dev, TX_CTX_IDX5, 0);
+	rt2x00pci_register_write(rt2x00dev, TX_DTX_IDX5, 0);
+
 	entry_priv = rt2x00dev->rx->entries[0].priv_data;
 	rt2x00pci_register_write(rt2x00dev, RX_BASE_PTR, entry_priv->desc_dma);
 	rt2x00pci_register_write(rt2x00dev, RX_MAX_CNT,
-- 
1.7.7.6

Re: [PATCH 2/4] rt2x00: move disabling of DMA before loading firmware

[Gertjan van Wingerde]

Hi Jakub,

On 03/27/12 16:51, Jakub Kicinski wrote:
> Legacy driver disables DMA before not after loading firmware.
> 
> Signed-off-by: Jakub Kicinski <kubakici@wp.pl>
> ---
> 2011_0406_RT5390_RT5392_Linux_STA_V2.5.0.3_DPO
> rtmp_init_inf.c:186
> ---
>  drivers/net/wireless/rt2x00/rt2800lib.c |   17 ++++++++---------
>  1 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> index 4722030..c872ba6 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> @@ -411,6 +411,14 @@ int rt2800_load_firmware(struct rt2x00_dev *rt2x00dev,
>  	}
>  
>  	/*
> +	 * Disable DMA, will be reenabled later when enabling the radio.
> +	 */
> +	rt2800_register_read(rt2x00dev, WPDMA_GLO_CFG, &reg);
> +	rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_TX_DMA, 0);
> +	rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_RX_DMA, 0);
> +	rt2800_register_write(rt2x00dev, WPDMA_GLO_CFG, reg);
> +
> +	/*
>  	 * Write firmware to the device.
>  	 */
>  	rt2800_drv_write_firmware(rt2x00dev, data, len);
> @@ -431,15 +439,6 @@ int rt2800_load_firmware(struct rt2x00_dev *rt2x00dev,
>  	}
>  
>  	/*
> -	 * Disable DMA, will be reenabled later when enabling
> -	 * the radio.
> -	 */
> -	rt2800_register_read(rt2x00dev, WPDMA_GLO_CFG, &reg);
> -	rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_TX_DMA, 0);
> -	rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_RX_DMA, 0);
> -	rt2800_register_write(rt2x00dev, WPDMA_GLO_CFG, reg);
> -
> -	/*
>  	 * Initialize firmware.
>  	 */
>  	rt2800_register_write(rt2x00dev, H2M_BBP_AGENT, 0);

This basically seems to be a revert of commit
4ed1dd2a7ec880b301beb61cbc1e08811ec340e4 in the wireless-testing tree.
That commit was submitted by Stanislaw, which basically does the reverse
of what you are doing here.

I leave it up to you and Stanislaw to decide on what the state is that
we need to be in.

---
Gertjan

Re: [PATCH 4/4] rt2x00: zero registers of unused TX rings

[Gertjan van Wingerde]

On 03/27/12 16:51, Jakub Kicinski wrote:
> This is needed if we take over after drivers which use those.
> 
> Signed-off-by: Jakub Kicinski <kubakici@wp.pl>

Acked-by: Gertjan van Wingerde <gwingerde@gmail.com>

> ---
>  drivers/net/wireless/rt2x00/rt2800pci.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
> index f39587b..1d24124 100644
> --- a/drivers/net/wireless/rt2x00/rt2800pci.c
> +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
> @@ -394,6 +394,16 @@ static int rt2800pci_init_queues(struct rt2x00_dev *rt2x00dev)
>  	rt2x00pci_register_write(rt2x00dev, TX_CTX_IDX3, 0);
>  	rt2x00pci_register_write(rt2x00dev, TX_DTX_IDX3, 0);
>  
> +	rt2x00pci_register_write(rt2x00dev, TX_BASE_PTR4, 0);
> +	rt2x00pci_register_write(rt2x00dev, TX_MAX_CNT4, 0);
> +	rt2x00pci_register_write(rt2x00dev, TX_CTX_IDX4, 0);
> +	rt2x00pci_register_write(rt2x00dev, TX_DTX_IDX4, 0);
> +
> +	rt2x00pci_register_write(rt2x00dev, TX_BASE_PTR5, 0);
> +	rt2x00pci_register_write(rt2x00dev, TX_MAX_CNT5, 0);
> +	rt2x00pci_register_write(rt2x00dev, TX_CTX_IDX5, 0);
> +	rt2x00pci_register_write(rt2x00dev, TX_DTX_IDX5, 0);
> +
>  	entry_priv = rt2x00dev->rx->entries[0].priv_data;
>  	rt2x00pci_register_write(rt2x00dev, RX_BASE_PTR, entry_priv->desc_dma);
>  	rt2x00pci_register_write(rt2x00dev, RX_MAX_CNT,


-- 
---
Gertjan

Re: [PATCH 3/4] rt2x00: initialize queues before giving up due to DMA error

[Gertjan van Wingerde]

On 03/27/12 16:51, Jakub Kicinski wrote:
> Don't immediately abort .start if DMA is busy before we
> initialize the queues. Some drivers do not deinitialize
> queues properly and we would fail to take over after them.
> 
> This behaviour is consistent with legacy driver.
> 
> Signed-off-by: Jakub Kicinski <kubakici@wp.pl>

Acked-by: Gertjan van Wingerde <gwingerde@gmail.com>

> ---
> 2011_0406_RT5390_RT5392_Linux_STA_V2.5.0.3_DPO
> common/rtmp_init.c:2007
> ---
>  drivers/net/wireless/rt2x00/rt2800lib.c |    2 +-
>  drivers/net/wireless/rt2x00/rt2800pci.c |    6 ++++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> index c872ba6..de975ef 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> @@ -290,7 +290,7 @@ int rt2800_wait_wpdma_ready(struct rt2x00_dev *rt2x00dev)
>  		msleep(10);
>  	}
>  
> -	ERROR(rt2x00dev, "WPDMA TX/RX busy, aborting.\n");
> +	ERROR(rt2x00dev, "WPDMA TX/RX busy [0x%08x].\n", reg);
>  	return -EACCES;
>  }
>  EXPORT_SYMBOL_GPL(rt2800_wait_wpdma_ready);
> diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
> index 5a2f68f..f39587b 100644
> --- a/drivers/net/wireless/rt2x00/rt2800pci.c
> +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
> @@ -501,8 +501,10 @@ static int rt2800pci_enable_radio(struct rt2x00_dev *rt2x00dev)
>  {
>  	int retval;
>  
> -	if (unlikely(rt2800_wait_wpdma_ready(rt2x00dev) ||
> -		     rt2800pci_init_queues(rt2x00dev)))
> +	/* Wait for DMA, ignore error until we initialize queues. */
> +	rt2800_wait_wpdma_ready(rt2x00dev);
> +
> +	if (unlikely(rt2800pci_init_queues(rt2x00dev)))
>  		return -EIO;
>  
>  	retval = rt2800_enable_radio(rt2x00dev);


-- 
---
Gertjan

Re: [PATCH 1/4] rt2x00: broaden PCIE L1-state wakeup configuration

[Gertjan van Wingerde]

Hi Jakub,

On 03/27/12 16:51, Jakub Kicinski wrote:
> Broaden wakeup configuration to all PCIE from rt3xxx on.
> 
> Signed-off-by: Jakub Kicinski <kubakici@wp.pl>
> ---
> 2011_0406_RT5390_RT5392_Linux_STA_V2.5.0.3_DPO
> rtmp_init_inf.c:171
> 
> Legacy driver applies this configuration to the following devices:
> 3071 3090 3572 3592 3562 3062 3390 3593 5390 5370 5392 5372 5360 5362
> if they are working on PCIE bus. AFAIK these are all post 2xxx PCIE
> devices.
> ---
>  drivers/net/wireless/rt2x00/rt2800lib.c |    4 +---
>  drivers/net/wireless/rt2x00/rt2800pci.c |    5 +----
>  2 files changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> index 6c0a12e..4722030 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> @@ -401,9 +401,7 @@ int rt2800_load_firmware(struct rt2x00_dev *rt2x00dev,
>  		return -EBUSY;
>  
>  	if (rt2x00_is_pci(rt2x00dev)) {
> -		if (rt2x00_rt(rt2x00dev, RT3572) ||
> -		    rt2x00_rt(rt2x00dev, RT5390) ||
> -		    rt2x00_rt(rt2x00dev, RT5392)) {
> +		if (rt2x00_is_pcie(rt2x00dev) && rt2x00dev->chip.rt > 0x3000) {
>  			rt2800_register_read(rt2x00dev, AUX_CTRL, &reg);
>  			rt2x00_set_field32(&reg, AUX_CTRL_FORCE_PCIE_CLK, 1);
>  			rt2x00_set_field32(&reg, AUX_CTRL_WAKE_PCIE_EN, 1);

I don't like the comparison against 0x3000. Maybe you should invert the
conditions and test for the chipsets for which this does not have to be
applied (e.g. !rt2x00_rt(rt2x00dev, RT2860) && !rt2x00_rt(rt2x00dev,
RT2872).
Also, the flow looks a bit strange here. The old code applied to all PCI
and PCIe devices, now you narrow it down to just PCIe devices.
Is that right?
Can anything be done to the strange flow now, with first checking for
PCI and PCIe devices and then immediately only for PCIe devices?

> diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
> index 0397bbf..5a2f68f 100644
> --- a/drivers/net/wireless/rt2x00/rt2800pci.c
> +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
> @@ -478,10 +478,7 @@ static int rt2800pci_init_registers(struct rt2x00_dev *rt2x00dev)
>  	rt2x00pci_register_write(rt2x00dev, PBF_SYS_CTRL, 0x00000e1f);
>  	rt2x00pci_register_write(rt2x00dev, PBF_SYS_CTRL, 0x00000e00);
>  
> -	if (rt2x00_is_pcie(rt2x00dev) &&
> -	    (rt2x00_rt(rt2x00dev, RT3572) ||
> -	     rt2x00_rt(rt2x00dev, RT5390) ||
> -	     rt2x00_rt(rt2x00dev, RT5392))) {
> +	if (rt2x00_is_pcie(rt2x00dev) && rt2x00dev->chip.rt > 0x3000) {
>  		rt2x00pci_register_read(rt2x00dev, AUX_CTRL, &reg);
>  		rt2x00_set_field32(&reg, AUX_CTRL_FORCE_PCIE_CLK, 1);
>  		rt2x00_set_field32(&reg, AUX_CTRL_WAKE_PCIE_EN, 1);

---
Gertjan

Re: [rt2x00-users] [PATCH 1/4] rt2x00: broaden PCIE L1-state wakeup configuration

[Jakub Kicinski]

Hi,

On Tue, 27 Mar 2012 20:19:00 +0200 Gertjan van Wingerde <gwingerde@gmail.com> wrote:
> On 03/27/12 16:51, Jakub Kicinski wrote:
> > Broaden wakeup configuration to all PCIE from rt3xxx on.
> > 
> > Signed-off-by: Jakub Kicinski <kubakici@wp.pl>
> > ---
> > 2011_0406_RT5390_RT5392_Linux_STA_V2.5.0.3_DPO
> > rtmp_init_inf.c:171
> > 
> > Legacy driver applies this configuration to the following devices:
> > 3071 3090 3572 3592 3562 3062 3390 3593 5390 5370 5392 5372 5360 5362
> > if they are working on PCIE bus. AFAIK these are all post 2xxx PCIE
> > devices.
> > ---
> >  drivers/net/wireless/rt2x00/rt2800lib.c |    4 +---
> >  drivers/net/wireless/rt2x00/rt2800pci.c |    5 +----
> >  2 files changed, 2 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> > index 6c0a12e..4722030 100644
> > --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> > +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> > @@ -401,9 +401,7 @@ int rt2800_load_firmware(struct rt2x00_dev *rt2x00dev,
> >  		return -EBUSY;
> >  
> >  	if (rt2x00_is_pci(rt2x00dev)) {
> > -		if (rt2x00_rt(rt2x00dev, RT3572) ||
> > -		    rt2x00_rt(rt2x00dev, RT5390) ||
> > -		    rt2x00_rt(rt2x00dev, RT5392)) {
> > +		if (rt2x00_is_pcie(rt2x00dev) && rt2x00dev->chip.rt > 0x3000) {
> >  			rt2800_register_read(rt2x00dev, AUX_CTRL, &reg);
> >  			rt2x00_set_field32(&reg, AUX_CTRL_FORCE_PCIE_CLK, 1);
> >  			rt2x00_set_field32(&reg, AUX_CTRL_WAKE_PCIE_EN, 1);
> 
> I don't like the comparison against 0x3000. Maybe you should invert the
> conditions and test for the chipsets for which this does not have to be
> applied (e.g. !rt2x00_rt(rt2x00dev, RT2860) && !rt2x00_rt(rt2x00dev,
> RT2872).
Not sure about that. Excluding old chips would be opposite to what
legacy driver is doing.

Legacy driver uses a long list of almost all > 0x3000 devices here. It
leaves out only RT3070, RT3352 and RT5350 but those chips are not used
on PCIe. The same list is later used throughout PCIe power management
code. I can only guess that from 0x3000 on PCIe devices gained some
kind of new power management capabilities.

It might be worth adding a macro which will check for all those devices
(rt2800pci_has_pcie_ps or sth) later on, when implementing support for
that PCIe PS, but for now I thought check against 0x3000 will be
equally good.

> Also, the flow looks a bit strange here. The old code applied to all PCI
> and PCIe devices, now you narrow it down to just PCIe devices.
> Is that right?
Yes, legacy driver applies this to PCIe only.

> Can anything be done to the strange flow now, with first checking for
> PCI and PCIe devices and then immediately only for PCIe devices?
Well, there are pros and cons for leaving it as it is, but I will move
PCIe checks out of PCI block if you like ;-)

  -- Kuba

Re: [PATCH 2/4] rt2x00: move disabling of DMA before loading firmware

[Stanislaw Gruszka]

Hello,

On Tue, Mar 27, 2012 at 08:11:07PM +0200, Gertjan van Wingerde wrote:
> >  	/*
> > -	 * Disable DMA, will be reenabled later when enabling
> > -	 * the radio.
> > -	 */
> > -	rt2800_register_read(rt2x00dev, WPDMA_GLO_CFG, &reg);
> > -	rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_TX_DMA, 0);
> > -	rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_RX_DMA, 0);
> > -	rt2800_register_write(rt2x00dev, WPDMA_GLO_CFG, reg);
> > -
> > -	/*
> >  	 * Initialize firmware.
> >  	 */
> >  	rt2800_register_write(rt2x00dev, H2M_BBP_AGENT, 0);
> 
> This basically seems to be a revert of commit
> 4ed1dd2a7ec880b301beb61cbc1e08811ec340e4 in the wireless-testing tree.
> That commit was submitted by Stanislaw, which basically does the reverse
> of what you are doing here.
> 
> I leave it up to you and Stanislaw to decide on what the state is that
> we need to be in.

When DMA was disabled before FW load, I observed frame receives before
->enable_radio, so looks like WPDMA_GLO_CFG register is overwritten by
F/W load. Although received frames did not make any damage, I prefer
not to allow hw to send data to driver, before driver will be really
ready to receive them.

Stanislaw

Re: [rt2x00-users] [PATCH 1/4] rt2x00: broaden PCIE L1-state wakeup configuration

[Stanislaw Gruszka]

On Wed, Mar 28, 2012 at 12:52:44AM +0200, Jakub Kicinski wrote:
> > 
> > I don't like the comparison against 0x3000. Maybe you should invert the
> > conditions and test for the chipsets for which this does not have to be
> > applied (e.g. !rt2x00_rt(rt2x00dev, RT2860) && !rt2x00_rt(rt2x00dev,
> > RT2872).
> Not sure about that. Excluding old chips would be opposite to what
> legacy driver is doing.
> 
> Legacy driver uses a long list of almost all > 0x3000 devices here. It
> leaves out only RT3070, RT3352 and RT5350 but those chips are not used
> on PCIe. The same list is later used throughout PCIe power management
> code. I can only guess that from 0x3000 on PCIe devices gained some
> kind of new power management capabilities.
> 
> It might be worth adding a macro which will check for all those devices
> (rt2800pci_has_pcie_ps or sth) later on, when implementing support for
> that PCIe PS, but for now I thought check against 0x3000 will be
> equally good.

Leaving > 0x3000 check, but wrap it in a macro with a descriptive name,
which could tell what this check actually mean, would be most
appreciated change IMHO.

BTW: if you plan to add ASPM quirks from vendor driver to rt2x00, don't
do it. ASPM should be configured by pci core.

Stanislaw

Re: [rt2x00-users] [PATCH 2/4] rt2x00: move disabling of DMA before loading firmware

[Jakub Kicinski]

On Wed, 28 Mar 2012 14:14:29 +0200
Stanislaw Gruszka <sgruszka@redhat.com> wrote:

> Hello,
> 
> On Tue, Mar 27, 2012 at 08:11:07PM +0200, Gertjan van Wingerde wrote:
> > >  	/*
> > > -	 * Disable DMA, will be reenabled later when enabling
> > > -	 * the radio.
> > > -	 */
> > > -	rt2800_register_read(rt2x00dev, WPDMA_GLO_CFG, &reg);
> > > -	rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_TX_DMA, 0);
> > > -	rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_RX_DMA, 0);
> > > -	rt2800_register_write(rt2x00dev, WPDMA_GLO_CFG, reg);
> > > -
> > > -	/*
> > >  	 * Initialize firmware.
> > >  	 */
> > >  	rt2800_register_write(rt2x00dev, H2M_BBP_AGENT, 0);
> > 
> > This basically seems to be a revert of commit
> > 4ed1dd2a7ec880b301beb61cbc1e08811ec340e4 in the wireless-testing tree.
> > That commit was submitted by Stanislaw, which basically does the reverse
> > of what you are doing here.
> > 
> > I leave it up to you and Stanislaw to decide on what the state is that
> > we need to be in.
> 
> When DMA was disabled before FW load, I observed frame receives before
> ->enable_radio, so looks like WPDMA_GLO_CFG register is overwritten by
> F/W load. Although received frames did not make any damage, I prefer
> not to allow hw to send data to driver, before driver will be really
> ready to receive them.

Hm. I obviously did check whether FW load does overwrite WPDMA_GLO_CFG
and it didn't. Maybe it's device dependent? What bothers me is that now
we have a (very) short window between FW load and disabling DMA.

  -- Kuba

Re: [rt2x00-users] [PATCH 2/4] rt2x00: move disabling of DMA before loading firmware

[Julian Calaby]

Hi all,

On Wed, Mar 28, 2012 at 23:53, Jakub Kicinski <kubakici@wp.pl> wrote:
> On Wed, 28 Mar 2012 14:14:29 +0200
> Stanislaw Gruszka <sgruszka@redhat.com> wrote:
>
>> Hello,
>>
>> On Tue, Mar 27, 2012 at 08:11:07PM +0200, Gertjan van Wingerde wrote:
>> > >   /*
>> > > -  * Disable DMA, will be reenabled later when enabling
>> > > -  * the radio.
>> > > -  */
>> > > - rt2800_register_read(rt2x00dev, WPDMA_GLO_CFG, &reg);
>> > > - rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_TX_DMA, 0);
>> > > - rt2x00_set_field32(&reg, WPDMA_GLO_CFG_ENABLE_RX_DMA, 0);
>> > > - rt2800_register_write(rt2x00dev, WPDMA_GLO_CFG, reg);
>> > > -
>> > > - /*
>> > >    * Initialize firmware.
>> > >    */
>> > >   rt2800_register_write(rt2x00dev, H2M_BBP_AGENT, 0);
>> >
>> > This basically seems to be a revert of commit
>> > 4ed1dd2a7ec880b301beb61cbc1e08811ec340e4 in the wireless-testing tree.
>> > That commit was submitted by Stanislaw, which basically does the reverse
>> > of what you are doing here.
>> >
>> > I leave it up to you and Stanislaw to decide on what the state is that
>> > we need to be in.
>>
>> When DMA was disabled before FW load, I observed frame receives before
>> ->enable_radio, so looks like WPDMA_GLO_CFG register is overwritten by
>> F/W load. Although received frames did not make any damage, I prefer
>> not to allow hw to send data to driver, before driver will be really
>> ready to receive them.
>
> Hm. I obviously did check whether FW load does overwrite WPDMA_GLO_CFG
> and it didn't. Maybe it's device dependent? What bothers me is that now
> we have a (very) short window between FW load and disabling DMA.

Stupid question: why not disable DMA before the FW load, then disable
it afterwards just to make sure the hardware's in a known state after
the FW load? - this will catch any errant firmwares out there and
(mostly) eliminate the window between FW load and DMA disabling.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

Re: [rt2x00-users] [PATCH 2/4] rt2x00: move disabling of DMA before loading firmware

[Stanislaw Gruszka]

Hi

On Wed, Mar 28, 2012 at 11:58:45PM +1100, Julian Calaby wrote:
> > Hm. I obviously did check whether FW load does overwrite WPDMA_GLO_CFG
> > and it didn't. Maybe it's device dependent? What bothers me is that now
> > we have a (very) short window between FW load and disabling DMA.
> 
> Stupid question: why not disable DMA before the FW load, then disable
> it afterwards just to make sure the hardware's in a known state after
> the FW load? - this will catch any errant firmwares out there and
> (mostly) eliminate the window between FW load and DMA disabling.

Yep, that seems to be the best approach.

Stanislaw

Re: [rt2x00-users] [PATCH 0/4] rt2x00: fix initialization after legacy driver

[Andreas Hartmann]

Hi Jakub,

Jakub Kicinski wrote:
> Hi,
> 
> I've been testing these patches for a few weeks and I think it's about 
> the time to post them.
> 
> The main purpose of this set is to make it possible to switch between
> rt2800pci and legacy driver w/o rebooting or suspending whole machine.
> Today rt2800pci will fail to bring the interface up if it was earlier
> operated by legacy drv. I'm sure it happens with rt3090, rt2860 seems
> to work fine.

I'm running these patches since a few weeks now. I didn't had the
problem you tried to fix (because I never used the legacy driver), but I
had another one, which seems to be fixed with this patchset, too:

Before this set, the device often wasn't usable at all after loading the
modules and starting of hostapd. No beacon could be seen, although the
log files didn't show any problem.

With your patchset applied, I face this problem really seldom now, about
once a week (instead of 3 or 4 times a day).


Thanks!
Regards,
Andreas