On 2012-06-17 3:06 AM, Sujith Manoharan wrote: > Felix Fietkau wrote: >> It was found to be buggy on a variety of chipsets from AR913x to AR928x. >> The new version (which was introduced along with AR93xx support) is more >> reliable in preventing connectivity dropouts and also fixes MIB interrupt >> storm issues. > > What bugs exactly ? Can you elaborate which cases show improvement ? > OFDM weak signal detection ? > CCK weak signal detection ? > Noise immunity ? > Firstep_Level ? I don't know which of those parameters are relevant, I don't have a reproducible test case myself. I don't have links to all relevant bug reports anymore, here are a few that I could find: https://dev.openwrt.org/ticket/10166 https://dev.openwrt.org/ticket/11442 Some of the discussion is missing in those tickets, as I had some email exchanges with some of the bug reporters off-list. What is not very visible in the ticket discussion is that I gave some test patches to a few users with various parts of Raj's commit reverted to try to pinpoint the source of the regression, but that didn't work out, there may be other changes that we haven't found yet that break old-ANI as well. > MIB interrupt storms will not be seen because they aren't used at all. Right, the old implementation depends on MIB interrupt support, the new one doesn't. > False positives are an issue with ANI and has this been studied with the > move to the new algorithm ? What kind of false positives? > The new ANI algorithm is designed to take advantage of the HW improvements > in AR300 and above. Has it been verified that the older HW generation can actually > conform to the requirements of the new algorithm ? I did review pretty much all of the code, and the knobs that the ANI code tweaks really haven't changed much compared to the old implementation. It's split up into different functions, which makes it a bit harder to see the similarities. The main implementation differences that I can find is that the new implementation does not tweak the AGC parameters anymore (commit log mentions something about this messing up RIFS Rx) and that the old implementation tries to tune all available ANI control parameters individually, whereas the new implementation contains a table that describes how the parameters are raised/lowered at once (split into OFDM and CCK). This simplifies the state machine quite a bit and makes it easier to make sure that it does not get stuck somewhere. As for hw specific improvements, old hw does not tweak MRC enable/disable for CCK, this is dealt with properly in the code. > If there are bugs or regressions in the existing code in ath9k, we can surely > fix them. But IMHO, saying 'general stability is improved' is not really a > justification for code extermination. Why can't we opt for a more evolutionary > approach ? Having two similar implementations of the same thing in the code, partially overlapping, makes the code hard to follow and hard to fix. Also, as I pointed out, the issue of regressions creeping in because of complex code is not theoretical, it has already happened, and I believe it is likely to happen again unless the root cause (spaghetti code) is fixed. The main state machine of the new implementation is much easier to understand and follow than the old one, and I'm confident that it'll be much easier for us to fix bugs there. - Felix -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html