> > 2016-01-27 2:46 GMT-05:00 Grumbach, Emmanuel > <emmanuel.grumb...@intel.com>: > >> Hi > >> > >> 2016-01-26 3:28 GMT-05:00 Grumbach, Emmanuel > >> <emmanuel.grumb...@intel.com>: > >> > > >> > > >> > On 01/26/2016 12:20 AM, Nikolay Martynov wrote: > >> >> It looks like sometimes firmware returns zero for chain noise and > >> >> signal during calibration period. This seems to be a known problem > >> >> and current implementation accounts for this by ignoring invalid > >> >> data when all chains return zero signal and noise. > >> >> > >> >> The problem is that sometimes firmware returns zero for only one > >> >> chain for some (not all) beacons used for calibration. This leads > >> >> to perfectly valid chains be disabled and may cause invalid gain > settings. > >> >> For example this is calibration data taken on laptop with Intel > >> >> 6300 card with all three antennas attached: > >> >> > >> >> active_chains: 3 > >> >> chain_noise_a: 312 > >> >> chain_noise_b: 297 > >> >> chain_noise_c: 0 > >> >> chain_signal_a: 549 > >> >> chain_signal_b: 513 > >> >> chain_signal_c: 0 > >> >> beacon_count: 16 > >> >> disconn_array: 0 0 1 > >> >> delta_gain_code: 4 0 0 > >> >> radio_write: 1 > >> >> state: 3 > >> >> > >> >> This patch changes statistics gathering to make sure that zero > >> >> noise results are ignored for valid rx chains. The rationale being > >> >> that even if anntenna is not connected we should be able to see > >> >> non zero noise if rx chain is present. > >> > > >> > But then the firmware will continue to send zero for signal and > >> > this will impact lots of flows like roaming. If the driver allows > >> > the firmware to use that antenna, the firmware may use this antenna > >> > for scanning and roaming will be broken. > >> > This seems to be a bug in the firmware, but there isn't much I can > >> > do about it. > >> > Sorry, I have to NACK this patch. > >> > >> Could you please elaborate on how this patch would affect roaming > >> or other things. As far as I can see this patch doesn't change much > >> behavior apart from ignoring invalid values from firmware. > >> Disconnected antennas still get disabled (as before) connected > >> antennas still work (more often than before). So I'm not sure I can > >> see how this patch would change what firmware does at all. I really > >> hope you could find a moment and explain this. > >> > > > > What you are saying here is that there is a bug in the firmware which > > makes it report wrong values for one of the antennas. But when you > > will have this antenna enabled (with your patch), the firmware will > > keep sending bad signal / noise values for it. If the driver allows > > the firmware to use this antenna (after your patch), the firmware will > > choose this antenna to receive beacons or to scan. Then, the driver will > look at the beacons' rssi (which will be wrong) and it will think that an AP > which is very close is in fact far away. > > > No. That is not correct, I think. What I'm saying is that sometimes (not > always) firmware is sending 0 (exactly 0) for signal and noise for some (or > all) > chains. > The case when all chains get 0 seem to be a known problem: it is worked > around in iwl_find_disconn_antenna. The case when only one chain gets > zero is not currently handled. > And just to clarify - all chains are affected by this problem, it's not like > one > specific chain is broken in some way and gets zero. So both of the cards I > have may be running with 3 chains or with 2 chains depending on how lucky > I'm during initial scan. > > It's just firmware that has a bug that sometimes returns zero for chain 1, > sometimes for chain 2, and sometimes for all of them. > So currently driver is already enabling chains for which we may get zero later > for rssi (presumably this is true) if it gets non zero during scan for first > 16 > beacons. > Moreover, if it gets non-zero for 15 out of 16 beacons the chain is not > disabled but gain values are wrong because of this - and one chain would be > amplifying things more than it should - this is currently happening to the > best > of my understanding. > > So my patch filters out results that we know are bad to account for this > firmware bug. > With this patch all chains with antenna attached get signal and noise reading > - > suggesting that firmware actually returns zero only some times and after > several retries we get reasonable statistics. It looks like there are some > 'transitioning' processes in firmware and if we out-wait them we get good > statistics. > > I'm not sure I see how this patch makes anything more worse than they > currently already are. > Currently it is already (presumably) possible to get wrong rssi reading > because chain that may have been enabled during first scan may get zeros > later. All my patch does is to enable all equivalently good (or bad) antennas, > instead of two equivalently good (or bad) as current code does. > > Does this explanation make any sense? Is it flawed in some way? > If patch in it's current state seems too controversial would patch that > enables > this check if some module parameter is set (and it is not set my default) be > more acceptable? >
Are you sure that the antenna that reported 0 "recovers" and reports good values later?
