Re: ipw(4) driver

["Damien Bergamini" <damien.bergamini@xxxxxxx>]

| Hm. This is bad. As I read the LICENSE file of the ipw-firmware,
| the only problem is EXHIBIT "A", point 2. If they would remove that 
one,
| I think we could live with the rest.

The license permits the redistribution of the firmware through packages
or ports: http://ipw2100.sourceforge.net/firmware_faq.php
So there is no problem with the current ioctl-based implementation. 
That's
what it was initially designed for.

| Yes, I know. I read the driver and to be precise I don't like that 
part
| at all. IMO iwi_init does too much work which belongs either into
| iwi_attach or iwi_resume.

iwi_init() upload the firmware to the adapter, initializes the tx and
rx rings and configures the adapter. iwi_stop() "kills" the firmware and
free the tx and rx rings.
You don't want to upload the firmware in attach(). First because you 
don't
have access to the firmware in attach(). Second because of power saving.
Yes, the adapter consumes more when the firmware is loaded. That's bad 
if
you don't use the it.
Initializing tx and rx rings in iwi_init() instead of attach() reduces
kernel memory footprint when the adapter is not used.

| BTW, there's a race in the attach code. If the interrupt assigned to
| iwi(4) is really shared, iwi_intr can actually be called. This could 
lead
| to corruption and other bad things.

Interrupts are shared. I don't see any race condition in attach().
Shared interrupts are dropped immediately in iwi_intr() without 
accessing
non-initialized data structures. The adapter itself can't raise any
interrupt (they are explicitly disabled at the time we install the 
interrupt
handler).

| One reason to not like the ioctl interface is that it's adding even
| more driver specific configuration programs.

That's where a common framework for loading firmwares makes sense.
Linux has such a feature (called firmware hotplug AFAIK).

| wicontrol is now general program for the many 802.11 devices, it has
| grown over it's initial use.

Yes. But that's bad IMO. wicontrol should have stayed bound to wi(4).
NetBSD has wlanctl(8) for common IEEE802.11 settings (like displaying 
the
list of available APs). The mix of WI_* and IEEE80211_* ioctls in
ieee80211_ioctl.c is a real mess.
ioctls like WI_RID_PRISM2 and WI_RID_SYMBOL_DIVERSITY have nothing to do
in a generic 802.11 layer.

| E.g. the SIOCGTABLE0 and SIOCGRADIO can be better handled with a 
sysctl
| handler, I'll start writing some support code for it soon.

Ok for SIOCGRADIO. For SIOCGTABLE0: I don't like printf's in the kernel
(prevent localization and such). I prefer the driver to output raw data.

--
Damien Bergamini
damien@xxxxxxxxxxx