On Thu, 2008-09-25 at 15:34 +0800, Zhu Yi wrote: > On Wed, 2008-09-24 at 02:44 -0600, Johannes Berg wrote: > > On Wed, 2008-09-24 at 13:57 +0800, Zhu Yi wrote: > > > From: Abhijeet Kolekar <abhijeet.kolekar@xxxxxxxxx> > > > > > > The patch adds channels sysfs entry for iwl3945. Make it consistent > > > with iwlagn. > > > > I advocate just make it consistent by removing it entirely. > > We need a way to show the channel info the card EEPROM advocated, not > the final one intersected with the regdomain. Are you suggesting a > debugfs entry? Not that I understand why you need that since iw shows all channels you are registering (albeit with possibly a few more flags like "disabled"), but yeah, if anything all this stuff should be in debugfs. There's also this little fact that this channel list shouldn't ever get used, you should be (and afaik are since I fixed it) using what the regdomain is enforcing, and you're already printing the channel list when enough debugging flags are set on driver load. Therefore, I don't understand why you need to clutter the driver with this code. Malicious gossip has it you're trying to obfuscate the code to a point where everybody else refuses to work on it. Anyhow, I see none of the sysfs files you're exporting as being userspace API that is actually needed in that form. Also, looking into more details, your sysfs files seem to fall into three categories: 1) things that mac80211 provides but you haven't hooked up 2) things that are generally useful, but you've added to your driver instead of helping build the generic framework 3) things that are truly only useful for your internal debugging 1) notably TX power; 2) it looks like retry_rate should be settable through a generic rate control API if it is truly useful (otherwise category 3), power_level also belongs here, sometimes one can't avoid the impression that you're adding things like that to avoid having to help define and implement the generic functionality; 3) Neither flags nor filter flags seem useful (and filter flags you can actually influence via nl80211) to set outside test environments, statistics falls here too, same probably with antenna, status, error and events logs and "measurement". Similar issues surround the module parameters, I won't go into them other than saying that they constitute API as well and at least having different versions for 4965 and 5000 hw seems entirely pointless. Many of them seem rather pointless in general, and something not only you have been doing and that I've asked for before is that I'd like to see a generic option to disable hardware crypto in _mac80211_ rather than each driver because this is actually genuinely useful if you want to monitor frames at the same time. johannes
Attachment:
signature.asc
Description: This is a digitally signed message part