Re: [RFT] [PATCH] bcm43xx: ACI fixes

2007-03-13 Thread Larry Finger 
Stefano,

As noted earlier on the list, I have started testing this patch. In setting the 
various interference
modes, I have found some bugs in other places that were not previously tested. 
Thanks for this
contribution.

I have some comments below.

Stefano Brivio wrote:
 Please test the following patch. It syncs ACI (Adjacent Channels
 Interference) code to specs. I can't test it right now. Will port to
 mac80211 branch ASAP.
 
 
 Signed-off-by: Stefano Brivio [EMAIL PROTECTED]
 
 
 diff --git a/drivers/net/wireless/bcm43xx/bcm43xx.h 
 b/drivers/net/wireless/bcm43xx/bcm43xx.h
 index 95ff175..7023327 100644
 --- a/drivers/net/wireless/bcm43xx/bcm43xx.h
 +++ b/drivers/net/wireless/bcm43xx/bcm43xx.h
 @@ -398,6 +398,9 @@
  #define BCM43xx_DEFAULT_SHORT_RETRY_LIMIT7
  #define BCM43xx_DEFAULT_LONG_RETRY_LIMIT 4
  
 +/* Statscounter offsets. */
 +#define BCM43xx_SC_ACI   0x2E
 +
  /* FIXME: the next line is a guess as to what the maximum RSSI value might 
 be */
  #define RX_RSSI_MAX  60
  
 @@ -550,7 +553,11 @@ struct bcm43xx_phyinfo {
   u8 connected:1,
  calibrated:1,
  is_locked:1, /* used in bcm43xx_phy_{un}lock() */
 -dyn_tssi_tbl:1; /* used in bcm43xx_phy_init_tssi2dbm_table() */
 +dyn_tssi_tbl:1, /* used in bcm43xx_phy_init_tssi2dbm_table() */
 +g_interfmode:1; /* bit 4000 in PHY_G_CRS, used in periodic tasks */
 + u8 g_interfmode_timer; /* how much time ago g_interfmode was unset */
 + u16 g_sc_saved; /* used in periodic tasks for ACI */
 +
   /* LO Measurement Data.
* Use bcm43xx_get_lopair() to get a value.
*/
 @@ -629,7 +636,9 @@ struct bcm43xx_radioinfo {
   /* ACI (adjacent channel interference) flags. */
   u8 aci_enable:1,
  aci_wlan_automatic:1,
 -aci_hw_rssi:1;
 +aci_hw_rssi:1,
 +aci_delay:5;
Should this be a bit field? Why not just make it a u8?
 + u8 aci_start;
  };
  
  /* Data structures for DMA transmission, per 80211 core. */
 @@ -699,6 +708,18 @@ struct bcm43xx_noise_calculation {
   s8 samples[8][4];
  };
  
 +/* Statscounter data (currently ACI only). */
 +struct bcm43xx_statscounter_saved {
 + u16 aci;
 +};
 +
 +/* Values for ACI moving average calculation. */
 +struct bcm43xx_aci_saved {
 + u16 value[8];
 + u8 next:3,
 +set:3;
As I will explain later, I don't think 'set' is needed. I also recommend having 
'next' be a plain u8.
 +};
 +
  struct bcm43xx_stats {
   u8 noise;
   struct iw_statistics wstats;
 @@ -812,6 +833,10 @@ struct bcm43xx_private {
   u32 irq_savedstate;
   /* Link Quality calculation context. */
   struct bcm43xx_noise_calculation noisecalc;
 + /* Statscounter tracking. */
 + struct bcm43xx_statscounter_saved sc;
 + /* ACI tracking. */
 + struct bcm43xx_aci_saved aci;
   /* if  0 MAC is suspended. if == 0 MAC is enabled. */
   int mac_suspended;
  
 diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.c 
 b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 index e594af4..27ec519 100644
 --- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 +++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
 @@ -3059,6 +3059,44 @@ static void bcm43xx_pcie_mdio_write(stru
   bcm43xx_write32(bcm, BCM43xx_PCIECORE_MDIO_CTL, 0);
  }
  
 +/* We don't use any statscounter other than ACI for now */
 +static u16 bcm43xx_sc_read(struct bcm43xx_private *bcm)
 +{
 + struct bcm43xx_statscounter_saved *sc = bcm-sc;
 + u16 tmp;
 +
 + tmp = bcm43xx_shm_read16(bcm, BCM43xx_SHM_SHARED, 0x80 + BCM43xx_SC_ACI)
 + - sc-aci;
 + if (tmp)
 + sc-aci += tmp;
 +
 + return tmp;
 +}
 +
 +static u16 bcm43xx_aci_moving_average(struct bcm43xx_private *bcm)
 +{
 + struct bcm43xx_aci_saved *aci = bcm-aci;
 +
 + u8 i;
 + u16 avg;
 + u32 sum = 0;
 +
 + aci-value[aci-next] = bcm43xx_sc_read(bcm);
 + if (unlikely(aci-set  7))
 + aci-set++;
I don't think the variable aci-set is needed as the divisor is alway 8 in the 
calculation of the
average below. Any error in the average shouldn't be important as it will occur 
only in the first
cycle through the value buffer. The for loop below should end at i  8. There 
will be extra
calculations only during the first buffer cycle.
 + for (i = 0; i  aci-set; i++)
 + sum += aci-value[i];
 + avg = sum / (u16)8;
Why the cast here? I didn't get any gcc warnings without it. I expect this ends 
up as a shift right
anyway.
 + if ((sum % 8)  3)
 + avg++;
 + if (aci-next  7)
 + aci-next++;
 + else
 + aci-next = 0;
Why not auto-increment aci-next in the line where bcm43xx_sc_read is called. 
Then you only need to
test for next  7 and reset it to zero then.
 +
 + return avg;
 +}

Larry

___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de

Re: [RFT] [PATCH] bcm43xx: ACI fixes

2007-03-12 Thread Michael Buesch 
On Monday 12 March 2007 21:28, Larry Finger wrote:
 Stefano Brivio wrote:
  On Mon, 12 Mar 2007 00:51:19 -0500
  
  Well, wait. I'd want to see some performance improvements before the patch
  is applied. That's why I didn't send it to John and put RFT in the
  subject. Are there any?
 
 No, I don't see any significant difference in RX or TX throughput with this 
 patch. So far, I have
 only tested on the 4311. Results for the 4306 and 4318 will come later.

Not that interference mitigation can not improve the signal,
if there is no interference disturbing it. ;) It _is_ already
best possible signal, then.

-- 
Greetings Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev

[RFT] [PATCH] bcm43xx: ACI fixes

2007-03-11 Thread Stefano Brivio 
Please test the following patch. It syncs ACI (Adjacent Channels
Interference) code to specs. I can't test it right now. Will port to
mac80211 branch ASAP.


Signed-off-by: Stefano Brivio [EMAIL PROTECTED]


diff --git a/drivers/net/wireless/bcm43xx/bcm43xx.h 
b/drivers/net/wireless/bcm43xx/bcm43xx.h
index 95ff175..7023327 100644
--- a/drivers/net/wireless/bcm43xx/bcm43xx.h
+++ b/drivers/net/wireless/bcm43xx/bcm43xx.h
@@ -398,6 +398,9 @@
 #define BCM43xx_DEFAULT_SHORT_RETRY_LIMIT  7
 #define BCM43xx_DEFAULT_LONG_RETRY_LIMIT   4
 
+/* Statscounter offsets. */
+#define BCM43xx_SC_ACI 0x2E
+
 /* FIXME: the next line is a guess as to what the maximum RSSI value might be 
*/
 #define RX_RSSI_MAX60
 
@@ -550,7 +553,11 @@ struct bcm43xx_phyinfo {
u8 connected:1,
   calibrated:1,
   is_locked:1, /* used in bcm43xx_phy_{un}lock() */
-  dyn_tssi_tbl:1; /* used in bcm43xx_phy_init_tssi2dbm_table() */
+  dyn_tssi_tbl:1, /* used in bcm43xx_phy_init_tssi2dbm_table() */
+  g_interfmode:1; /* bit 4000 in PHY_G_CRS, used in periodic tasks */
+   u8 g_interfmode_timer; /* how much time ago g_interfmode was unset */
+   u16 g_sc_saved; /* used in periodic tasks for ACI */
+
/* LO Measurement Data.
 * Use bcm43xx_get_lopair() to get a value.
 */
@@ -629,7 +636,9 @@ struct bcm43xx_radioinfo {
/* ACI (adjacent channel interference) flags. */
u8 aci_enable:1,
   aci_wlan_automatic:1,
-  aci_hw_rssi:1;
+  aci_hw_rssi:1,
+  aci_delay:5;
+   u8 aci_start;
 };
 
 /* Data structures for DMA transmission, per 80211 core. */
@@ -699,6 +708,18 @@ struct bcm43xx_noise_calculation {
s8 samples[8][4];
 };
 
+/* Statscounter data (currently ACI only). */
+struct bcm43xx_statscounter_saved {
+   u16 aci;
+};
+
+/* Values for ACI moving average calculation. */
+struct bcm43xx_aci_saved {
+   u16 value[8];
+   u8 next:3,
+  set:3;
+};
+
 struct bcm43xx_stats {
u8 noise;
struct iw_statistics wstats;
@@ -812,6 +833,10 @@ struct bcm43xx_private {
u32 irq_savedstate;
/* Link Quality calculation context. */
struct bcm43xx_noise_calculation noisecalc;
+   /* Statscounter tracking. */
+   struct bcm43xx_statscounter_saved sc;
+   /* ACI tracking. */
+   struct bcm43xx_aci_saved aci;
/* if  0 MAC is suspended. if == 0 MAC is enabled. */
int mac_suspended;
 
diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.c 
b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
index e594af4..27ec519 100644
--- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
+++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
@@ -3059,6 +3059,44 @@ static void bcm43xx_pcie_mdio_write(stru
bcm43xx_write32(bcm, BCM43xx_PCIECORE_MDIO_CTL, 0);
 }
 
+/* We don't use any statscounter other than ACI for now */
+static u16 bcm43xx_sc_read(struct bcm43xx_private *bcm)
+{
+   struct bcm43xx_statscounter_saved *sc = bcm-sc;
+   u16 tmp;
+
+   tmp = bcm43xx_shm_read16(bcm, BCM43xx_SHM_SHARED, 0x80 + BCM43xx_SC_ACI)
+   - sc-aci;
+   if (tmp)
+   sc-aci += tmp;
+
+   return tmp;
+}
+
+static u16 bcm43xx_aci_moving_average(struct bcm43xx_private *bcm)
+{
+   struct bcm43xx_aci_saved *aci = bcm-aci;
+
+   u8 i;
+   u16 avg;
+   u32 sum = 0;
+
+   aci-value[aci-next] = bcm43xx_sc_read(bcm);
+   if (unlikely(aci-set  7))
+   aci-set++;
+   for (i = 0; i  aci-set; i++)
+   sum += aci-value[i];
+   avg = sum / (u16)8;
+   if ((sum % 8)  3)
+   avg++;
+   if (aci-next  7)
+   aci-next++;
+   else
+   aci-next = 0;
+
+   return avg;
+}
+
 /* Make an I/O Core usable. core_mask is the bitmask of the cores to enable.
  * To enable core 0, pass a core_mask of 10
  */
@@ -3182,6 +3220,8 @@ static void bcm43xx_periodic_every1sec(s
struct bcm43xx_phyinfo *phy = bcm43xx_current_phy(bcm);
struct bcm43xx_radioinfo *radio = bcm43xx_current_radio(bcm);
int radio_hw_enable;
+   unsigned long phylock_flags;
+   u16 tmp;
 
/* check if radio hardware enabled status changed */
radio_hw_enable = bcm43xx_is_hw_radio_enabled(bcm);
@@ -3191,27 +3231,56 @@ static void bcm43xx_periodic_every1sec(s
   (radio_hw_enable == 0) ? disabled : enabled);
bcm43xx_leds_update(bcm, 0);
}
+
if (phy-type == BCM43xx_PHYTYPE_G) {
-   //TODO: update_aci_moving_average
-   if (radio-aci_enable  radio-aci_wlan_automatic) {
+   
+   if (radio-aci_start  60)
+   radio-aci_start++;
+   if (radio-aci_delay)
+   radio-aci_delay--;
+   if (phy-g_interfmode  phy-g_interfmode_timer  30)
+

Re: [RFT] [PATCH] bcm43xx: ACI fixes

2007-03-11 Thread Larry Finger 
Stefano Brivio wrote:
 Please test the following patch. It syncs ACI (Adjacent Channels
 Interference) code to specs. I can't test it right now. Will port to
 mac80211 branch ASAP.
 
 
 Signed-off-by: Stefano Brivio [EMAIL PROTECTED]
 

NACK - this patch really kills performance on my 4311. I'll see if I can find 
the problem.

Larry

___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev

Re: [RFT] [PATCH] bcm43xx: ACI fixes

2007-03-11 Thread Larry Finger 
Larry Finger wrote:
 Stefano Brivio wrote:
 Please test the following patch. It syncs ACI (Adjacent Channels
 Interference) code to specs. I can't test it right now. Will port to
 mac80211 branch ASAP.


 Signed-off-by: Stefano Brivio [EMAIL PROTECTED]
 
 
 NACK - this patch really kills performance on my 4311. I'll see if I can find 
 the problem.

Forget the above. The performance was killed by my AP, which needed to be 
rebooted. I will send this
patch on to John.

Larry

___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/bcm43xx-dev