2004-03-04 02:35:57

by Jean Tourrilhes

[permalink] [raw]
Subject: [PATCH 2.6] Intersil Prism54 wireless driver

Hi Dave & Jeff,

The attached .bz2 file is a patch for 2.6.3 adding the
Intersil Prism54 wireless driver. Sorry for the attachement, the file
is rather big, if you want inline+plaintext, I'll send that personal
to you.
I've been using this driver with great success on 2.6.3 and
2.6.4-rc1 (SMP). This driver support various popular CardBus and PCI
802.11g cards (54 Mb/s) based on the Intersil PrismGT/PrismDuette
chipset.
I would like this driver to go into 2.6.X. However, I
understand that it's lot's of code to review.

Have fun...

Jean


Attachments:
(No filename) (549.00 B)
patch-2.6.3-prism54-cvs20040304.bz2 (42.50 kB)
Download all attachments

2004-03-04 02:38:15

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

Jean Tourrilhes wrote:
> Hi Dave & Jeff,
>
> The attached .bz2 file is a patch for 2.6.3 adding the
> Intersil Prism54 wireless driver. Sorry for the attachement, the file
> is rather big, if you want inline+plaintext, I'll send that personal
> to you.
> I've been using this driver with great success on 2.6.3 and
> 2.6.4-rc1 (SMP). This driver support various popular CardBus and PCI
> 802.11g cards (54 Mb/s) based on the Intersil PrismGT/PrismDuette
> chipset.
> I would like this driver to go into 2.6.X. However, I
> understand that it's lot's of code to review.


I would like it to go into 2.6 too :) I'm glad somebody submitted it.

I'll review it this weekend... and hopefully some other netdev denizens
will review as well and post their comments.

Jeff



2004-03-04 02:49:36

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

On Wed, Mar 03, 2004 at 09:37:55PM -0500, Jeff Garzik wrote:
> Jean Tourrilhes wrote:
> > Hi Dave & Jeff,
> >
> > The attached .bz2 file is a patch for 2.6.3 adding the
> >Intersil Prism54 wireless driver. Sorry for the attachement, the file
> >is rather big, if you want inline+plaintext, I'll send that personal
> >to you.
> > I've been using this driver with great success on 2.6.3 and
> >2.6.4-rc1 (SMP). This driver support various popular CardBus and PCI
> >802.11g cards (54 Mb/s) based on the Intersil PrismGT/PrismDuette
> >chipset.
> > I would like this driver to go into 2.6.X. However, I
> >understand that it's lot's of code to review.
>
>
> I would like it to go into 2.6 too :) I'm glad somebody submitted it.
>
> I'll review it this weekend... and hopefully some other netdev denizens
> will review as well and post their comments.
>
> Jeff

By the way, I should note that during my test I had
difficulties to associate with Aironet cards, but it looked like it
was a firmware related issue (firmware timing out). The rest work fine
(managed to Lucent, Ad-Hoc to another Prism54, scanning,
WE). Actually, this is the driver I used to develop ifrename.
Have fun...

Jean

2004-03-10 03:24:59

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

On Wed, Mar 03, 2004 at 09:37:55PM -0500, Jeff Garzik wrote:
> Jean Tourrilhes wrote:
> > Hi Dave & Jeff,
> >
> > The attached .bz2 file is a patch for 2.6.3 adding the
> >Intersil Prism54 wireless driver. Sorry for the attachement, the file
> >is rather big, if you want inline+plaintext, I'll send that personal
> >to you.
> > I've been using this driver with great success on 2.6.3 and
> >2.6.4-rc1 (SMP). This driver support various popular CardBus and PCI
> >802.11g cards (54 Mb/s) based on the Intersil PrismGT/PrismDuette
> >chipset.
> > I would like this driver to go into 2.6.X. However, I
> >understand that it's lot's of code to review.
>
>
> I would like it to go into 2.6 too :) I'm glad somebody submitted it.
>
> I'll review it this weekend... and hopefully some other netdev denizens
> will review as well and post their comments.
>
> Jeff

Any news, Jeff ?
Thanks in advance...

Jean

2004-03-10 07:12:37

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

Jean Tourrilhes wrote:
> On Wed, Mar 03, 2004 at 09:37:55PM -0500, Jeff Garzik wrote:
>
>>Jean Tourrilhes wrote:
>>
>>> Hi Dave & Jeff,
>>>
>>> The attached .bz2 file is a patch for 2.6.3 adding the
>>>Intersil Prism54 wireless driver. Sorry for the attachement, the file
>>>is rather big, if you want inline+plaintext, I'll send that personal
>>>to you.
>>> I've been using this driver with great success on 2.6.3 and
>>>2.6.4-rc1 (SMP). This driver support various popular CardBus and PCI
>>>802.11g cards (54 Mb/s) based on the Intersil PrismGT/PrismDuette
>>>chipset.
>>> I would like this driver to go into 2.6.X. However, I
>>>understand that it's lot's of code to review.
>>
>>
>>I would like it to go into 2.6 too :) I'm glad somebody submitted it.
>>
>>I'll review it this weekend... and hopefully some other netdev denizens
>>will review as well and post their comments.
>>
>> Jeff
>
>
> Any news, Jeff ?
> Thanks in advance...


Looks pretty decent to me. I'm basically waiting on 2.6.4 release to
merge it. I might remove some of the more egregious #ifdefs.

Jeff



2004-03-10 16:56:30

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

On Wed, Mar 03, 2004 at 06:35:24PM -0800, Jean Tourrilhes wrote:
> Hi Dave & Jeff,
>
> The attached .bz2 file is a patch for 2.6.3 adding the
> Intersil Prism54 wireless driver. Sorry for the attachement, the file
> is rather big, if you want inline+plaintext, I'll send that personal
> to you.
> I've been using this driver with great success on 2.6.3 and
> 2.6.4-rc1 (SMP). This driver support various popular CardBus and PCI
> 802.11g cards (54 Mb/s) based on the Intersil PrismGT/PrismDuette
> chipset.
> I would like this driver to go into 2.6.X. However, I
> understand that it's lot's of code to review.

Here's a few things I found. It's not exactly a full review, there's
too much new snow to spend lots of time in front of a computer here :)

diff -Naur -X /home/mcgrof/lib/dontdiff linux-2.6.3/drivers/net/wireless/prism54/Makefile linux-2.6.3-prism54/drivers/net/wireless/prism54/Makefile
--- linux-2.6.3/drivers/net/wireless/prism54/Makefile Thu Jan 1 00:00:00 1970
+++ linux-2.6.3-prism54/drivers/net/wireless/prism54/Makefile Thu Mar 4 02:00:01 2004
@@ -0,0 +1,10 @@
+# $Id: Makefile.k26,v 1.7 2004/01/30 16:24:00 ajfa Exp $
+
+prism54-objs := islpci_eth.o islpci_mgt.o \
+ isl_38xx.o isl_ioctl.o islpci_dev.o \
+ islpci_hotplug.o isl_wds.o oid_mgt.o

please use foo-y for new drivers.

+
+obj-$(CONFIG_PRISM54) += prism54.o
+
+EXTRA_CFLAGS = -I$(PWD) #-DCONFIG_PRISM54_WDS

This is bogus, especially with srcdir != objdir.
please fixup the includes instead

+#define __KERNEL_SYSCALLS__

this shouldn't be used anymore.

+
+#include <linux/version.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/delay.h>
+
+#include "isl_38xx.h"
+#include <linux/firmware.h>
+
+#include <asm/uaccess.h>
+#include <asm/io.h>

Please include headers in the following order <linux/*.h>,
<asm/*.h>, driver-specific.

+#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,5,75))
+#include <linux/device.h>
+# define _REQ_FW_DEV_T struct device *
+#else
+# define _REQ_FW_DEV_T char *
+#endif

Eeek, why don't you simply pass the pci_dev down?


+typedef struct isl38xx_cb isl38xx_control_block;

No useless typedefs please.

+MODULE_PARM(init_mode, "i");
+MODULE_PARM_DESC(init_mode,
+ "Set card mode:\n0: Auto\n1: Ad-Hoc\n2: Managed Client (Default)\n3: Master / Access Point\n4: Repeater (Not supported yet)\n5: Secondary (Not supported yet)\n6: Monitor");

Please use module_param

diff -Naur -X /home/mcgrof/lib/dontdiff linux-2.6.3/drivers/net/wireless/prism54/isl_wds.c linux-2.6.3-prism54/drivers/net/wireless/prism54/isl_wds.c
--- linux-2.6.3/drivers/net/wireless/prism54/isl_wds.c Thu Jan 1 00:00:00 1970
+++ linux-2.6.3-prism54/drivers/net/wireless/prism54/isl_wds.c Thu Mar 4 02:00:01 2004

WDS doesn't belong into a driver but in higher-level code.

2004-03-10 17:22:18

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

On Wed, Mar 10, 2004 at 02:12:22AM -0500, Jeff Garzik wrote:
>
> Looks pretty decent to me. I'm basically waiting on 2.6.4 release to
> merge it. I might remove some of the more egregious #ifdefs.
>
> Jeff

Wonderful ! Thanks a lot !

Jean

2004-03-10 17:21:23

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

On Wed, Mar 10, 2004 at 04:55:48PM +0000, Christoph Hellwig wrote:
> On Wed, Mar 03, 2004 at 06:35:24PM -0800, Jean Tourrilhes wrote:
> > Hi Dave & Jeff,
> >
> > The attached .bz2 file is a patch for 2.6.3 adding the
> > Intersil Prism54 wireless driver. Sorry for the attachement, the file
> > is rather big, if you want inline+plaintext, I'll send that personal
> > to you.
> > I've been using this driver with great success on 2.6.3 and
> > 2.6.4-rc1 (SMP). This driver support various popular CardBus and PCI
> > 802.11g cards (54 Mb/s) based on the Intersil PrismGT/PrismDuette
> > chipset.
> > I would like this driver to go into 2.6.X. However, I
> > understand that it's lot's of code to review.
>
> Here's a few things I found.

I'm forwarding to prism54-devel where the real developpers can
answer your questions.

> It's not exactly a full review, there's
> too much new snow to spend lots of time in front of a computer here :)

Grrr... This year, no snow for me.

> diff -Naur -X /home/mcgrof/lib/dontdiff linux-2.6.3/drivers/net/wireless/prism54/Makefile linux-2.6.3-prism54/drivers/net/wireless/prism54/Makefile
> --- linux-2.6.3/drivers/net/wireless/prism54/Makefile Thu Jan 1 00:00:00 1970
> +++ linux-2.6.3-prism54/drivers/net/wireless/prism54/Makefile Thu Mar 4 02:00:01 2004
> @@ -0,0 +1,10 @@
> +# $Id: Makefile.k26,v 1.7 2004/01/30 16:24:00 ajfa Exp $
> +
> +prism54-objs := islpci_eth.o islpci_mgt.o \
> + isl_38xx.o isl_ioctl.o islpci_dev.o \
> + islpci_hotplug.o isl_wds.o oid_mgt.o
>
> please use foo-y for new drivers.
>
> +
> +obj-$(CONFIG_PRISM54) += prism54.o
> +
> +EXTRA_CFLAGS = -I$(PWD) #-DCONFIG_PRISM54_WDS
>
> This is bogus, especially with srcdir != objdir.
> please fixup the includes instead
>
> +#define __KERNEL_SYSCALLS__
>
> this shouldn't be used anymore.
>
> +
> +#include <linux/version.h>
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/delay.h>
> +
> +#include "isl_38xx.h"
> +#include <linux/firmware.h>
> +
> +#include <asm/uaccess.h>
> +#include <asm/io.h>
>
> Please include headers in the following order <linux/*.h>,
> <asm/*.h>, driver-specific.
>
> +#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,5,75))
> +#include <linux/device.h>
> +# define _REQ_FW_DEV_T struct device *
> +#else
> +# define _REQ_FW_DEV_T char *
> +#endif
>
> Eeek, why don't you simply pass the pci_dev down?
>
>
> +typedef struct isl38xx_cb isl38xx_control_block;
>
> No useless typedefs please.
>
> +MODULE_PARM(init_mode, "i");
> +MODULE_PARM_DESC(init_mode,
> + "Set card mode:\n0: Auto\n1: Ad-Hoc\n2: Managed Client (Default)\n3: Master / Access Point\n4: Repeater (Not supported yet)\n5: Secondary (Not supported yet)\n6: Monitor");
>
> Please use module_param

I would even say that this is useless because the driver
support WE, and WE scripts set the mode before the card is up.

> diff -Naur -X /home/mcgrof/lib/dontdiff linux-2.6.3/drivers/net/wireless/prism54/isl_wds.c linux-2.6.3-prism54/drivers/net/wireless/prism54/isl_wds.c
> --- linux-2.6.3/drivers/net/wireless/prism54/isl_wds.c Thu Jan 1 00:00:00 1970
> +++ linux-2.6.3-prism54/drivers/net/wireless/prism54/isl_wds.c Thu Mar 4 02:00:01 2004
>
> WDS doesn't belong into a driver but in higher-level code.

The big 802.11 reorg can only happen when HostAP is in the
kernel.

Regards,

Jean

2004-03-10 17:29:32

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

On Wed, Mar 10, 2004 at 09:21:14AM -0800, Jean Tourrilhes wrote:
> > --- linux-2.6.3/drivers/net/wireless/prism54/isl_wds.c Thu Jan 1 00:00:00 1970
> > +++ linux-2.6.3-prism54/drivers/net/wireless/prism54/isl_wds.c Thu Mar 4 02:00:01 2004
> >
> > WDS doesn't belong into a driver but in higher-level code.
>
> The big 802.11 reorg can only happen when HostAP is in the
> kernel.

I don't quite understand that comment. This feature doesn't belong into
a driver. Whether it's some day implemented in a hypothetic 802.11 midlayer
that can only be done with another driver merged is a different question,
isn't it?

>
> Regards,
>
> Jean
>
---end quoted text---

2004-03-10 17:30:27

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

Jean Tourrilhes wrote:
> On Wed, Mar 10, 2004 at 04:55:48PM +0000, Christoph Hellwig wrote:
>>+MODULE_PARM(init_mode, "i");
>>+MODULE_PARM_DESC(init_mode,
>>+ "Set card mode:\n0: Auto\n1: Ad-Hoc\n2: Managed Client (Default)\n3: Master / Access Point\n4: Repeater (Not supported yet)\n5: Secondary (Not supported yet)\n6: Monitor");
>>
>> Please use module_param
>
>
> I would even say that this is useless because the driver
> support WE, and WE scripts set the mode before the card is up.

module_param() is a type-safe interface roughly identical to
MODULE_PARM(). Therefore, if MODULE_PARM() works, module_param() works
also.


>>diff -Naur -X /home/mcgrof/lib/dontdiff linux-2.6.3/drivers/net/wireless/prism54/isl_wds.c linux-2.6.3-prism54/drivers/net/wireless/prism54/isl_wds.c
>>--- linux-2.6.3/drivers/net/wireless/prism54/isl_wds.c Thu Jan 1 00:00:00 1970
>>+++ linux-2.6.3-prism54/drivers/net/wireless/prism54/isl_wds.c Thu Mar 4 02:00:01 2004
>>
>> WDS doesn't belong into a driver but in higher-level code.
>
>
> The big 802.11 reorg can only happen when HostAP is in the
> kernel.

ISTR it needed some cleaning up before it could go in.

Further, in Linux, there is _never_ a requirement that "this driver be
included before we can clean up." You can start the re-org any time you
wish. Out-of-tree maintainers can follow the re-org, sometimes more easily.

Jeff



P.S. I still need to look at your netlink thing. Seems like a decent
direction.

2004-03-10 17:52:13

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

On Wed, Mar 10, 2004 at 12:29:59PM -0500, Jeff Garzik wrote:
> Jean Tourrilhes wrote:
> >On Wed, Mar 10, 2004 at 04:55:48PM +0000, Christoph Hellwig wrote:
> >>+MODULE_PARM(init_mode, "i");
> >>+MODULE_PARM_DESC(init_mode,
> >>+ "Set card mode:\n0: Auto\n1: Ad-Hoc\n2: Managed Client
> >>(Default)\n3: Master / Access Point\n4: Repeater (Not supported yet)\n5:
> >>Secondary (Not supported yet)\n6: Monitor");
> >>
> >> Please use module_param
> >
> >
> > I would even say that this is useless because the driver
> >support WE, and WE scripts set the mode before the card is up.
>
> module_param() is a type-safe interface roughly identical to
> MODULE_PARM(). Therefore, if MODULE_PARM() works, module_param() works
> also.

Yes, I know, I've been doing that for IrDA. What I meant was
that this specific module parameter could be remove entirely because
redundant.

> >>diff -Naur -X /home/mcgrof/lib/dontdiff
> >>linux-2.6.3/drivers/net/wireless/prism54/isl_wds.c
> >>linux-2.6.3-prism54/drivers/net/wireless/prism54/isl_wds.c
> >>--- linux-2.6.3/drivers/net/wireless/prism54/isl_wds.c Thu Jan 1
> >>00:00:00 1970
> >>+++ linux-2.6.3-prism54/drivers/net/wireless/prism54/isl_wds.c Thu
> >>Mar 4 02:00:01 2004
> >>
> >> WDS doesn't belong into a driver but in higher-level code.
> >
> >
> > The big 802.11 reorg can only happen when HostAP is in the
> >kernel.
>
> ISTR it needed some cleaning up before it could go in.

I think it would be nice to give some more explicit feedback
to Jouni.

> Further, in Linux, there is _never_ a requirement that "this driver be
> included before we can clean up." You can start the re-org any time you
> wish. Out-of-tree maintainers can follow the re-org, sometimes more easily.

You misunderstood. The HostAP driver has a pretty much
complete generic 802.11 stack. However, other driver can't depend on
that code until it's in the kernel.
By "big 802.11 reorg", I meant "make the other driver depend
on HostAP 802.11 code".
Of course, I'm quite partial to the HostAP code because I'm
more familiar with it and I believe it's the most advanced (host WEP,
802.1x, WPA, AP...). Other candidated are linux-wlan-ng or the *BSD
stack (by the way of the MadWifi driver).

> Jeff
>
>
>
> P.S. I still need to look at your netlink thing. Seems like a decent
> direction.

Thanks ;-) I would need to make sure that there is no popular
driver still using the old driver API (orinoco_cs is converted in the
CVS).

Jean

2004-03-10 17:58:43

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

Jean Tourrilhes wrote:
> You misunderstood. The HostAP driver has a pretty much
> complete generic 802.11 stack. However, other driver can't depend on
> that code until it's in the kernel.
> By "big 802.11 reorg", I meant "make the other driver depend
> on HostAP 802.11 code".
> Of course, I'm quite partial to the HostAP code because I'm
> more familiar with it and I believe it's the most advanced (host WEP,
> 802.1x, WPA, AP...). Other candidated are linux-wlan-ng or the *BSD
> stack (by the way of the MadWifi driver).

Ah! I did indeed misunderstand.

Yes, it would be good to end the cycle of re-implementing 802.11 over
and over again ;-)

So here is my suggested plan:
* I merge prism54 upstream
* I create wireless-2.6 queue
* somebody (you, Jouni(sp?)) submits HostAP to me
* I merge HostAP
* submit rtnetlink patches to me
* start working on generic 802.11 stack in wireless-2.6 queue
* at the same time, migrate away from iw_handler to "wireless_ops" (i.e.
something like ethtool_ops)
* once things have stabilized again, merge upstream

I'm not sure about the order of the last few steps, you know better than
I what dependencies exist.

Jeff



2004-03-10 18:10:51

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

Jean Tourrilhes wrote:
> On Wed, Mar 10, 2004 at 04:55:48PM +0000, Christoph Hellwig wrote:
>
>>On Wed, Mar 03, 2004 at 06:35:24PM -0800, Jean Tourrilhes wrote:
>>
>>> Hi Dave & Jeff,
>>>
>>> The attached .bz2 file is a patch for 2.6.3 adding the
>>>Intersil Prism54 wireless driver. Sorry for the attachement, the file
>>>is rather big, if you want inline+plaintext, I'll send that personal
>>>to you.
>>> I've been using this driver with great success on 2.6.3 and
>>>2.6.4-rc1 (SMP). This driver support various popular CardBus and PCI
>>>802.11g cards (54 Mb/s) based on the Intersil PrismGT/PrismDuette
>>>chipset.
>>> I would like this driver to go into 2.6.X. However, I
>>>understand that it's lot's of code to review.
>>
>>Here's a few things I found.
>
>
> I'm forwarding to prism54-devel where the real developpers can
> answer your questions.

since I'm not on the this list, I am getting a bunch of "your message
for prism54-devel awaits moderator approval" responses.

Maybe the prism54 and hostAP developers could join us on netdev? It
would be nice to have everybody in one place, if we're all gonna be
collaborating on a generic 802.11 stack.

Jeff



2004-03-10 22:17:24

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [Prism54-devel] Re: [PATCH 2.6] Intersil Prism54 wireless driver

On Wed, Mar 10, 2004 at 09:21:14AM -0800, Jean Tourrilhes wrote:
> On Wed, Mar 10, 2004 at 04:55:48PM +0000, Christoph Hellwig wrote:
> > On Wed, Mar 03, 2004 at 06:35:24PM -0800, Jean Tourrilhes wrote:
> > > Hi Dave & Jeff,
> > >
> > > The attached .bz2 file is a patch for 2.6.3 adding the
> > > Intersil Prism54 wireless driver. Sorry for the attachement, the file
> > > is rather big, if you want inline+plaintext, I'll send that personal
> > > to you.
> > > I've been using this driver with great success on 2.6.3 and
> > > 2.6.4-rc1 (SMP). This driver support various popular CardBus and PCI
> > > 802.11g cards (54 Mb/s) based on the Intersil PrismGT/PrismDuette
> > > chipset.
> > > I would like this driver to go into 2.6.X. However, I
> > > understand that it's lot's of code to review.
> >
> > Here's a few things I found.
>
> I'm forwarding to prism54-devel where the real developpers can
> answer your questions.
>
> > It's not exactly a full review, there's
> > too much new snow to spend lots of time in front of a computer here :)
>
> Grrr... This year, no snow for me.
>
> > diff -Naur -X /home/mcgrof/lib/dontdiff linux-2.6.3/drivers/net/wireless/prism54/Makefile linux-2.6.3-prism54/drivers/net/wireless/prism54/Makefile
> > --- linux-2.6.3/drivers/net/wireless/prism54/Makefile Thu Jan 1 00:00:00 1970
> > +++ linux-2.6.3-prism54/drivers/net/wireless/prism54/Makefile Thu Mar 4 02:00:01 2004
> > @@ -0,0 +1,10 @@
> > +# $Id: Makefile.k26,v 1.7 2004/01/30 16:24:00 ajfa Exp $
> > +
> > +prism54-objs := islpci_eth.o islpci_mgt.o \
> > + isl_38xx.o isl_ioctl.o islpci_dev.o \
> > + islpci_hotplug.o isl_wds.o oid_mgt.o
> >
> > please use foo-y for new drivers.

TODO

> >
> > +
> > +obj-$(CONFIG_PRISM54) += prism54.o
> > +
> > +EXTRA_CFLAGS = -I$(PWD) #-DCONFIG_PRISM54_WDS
> >
> > This is bogus, especially with srcdir != objdir.
> > please fixup the includes instead
> >
> > +#define __KERNEL_SYSCALLS__
> >
> > this shouldn't be used anymore.

Done

> >
> > +
> > +#include <linux/version.h>
> > +#include <linux/module.h>
> > +#include <linux/types.h>
> > +#include <linux/delay.h>
> > +
> > +#include "isl_38xx.h"
> > +#include <linux/firmware.h>
> > +
> > +#include <asm/uaccess.h>
> > +#include <asm/io.h>
> >
> > Please include headers in the following order <linux/*.h>,
> > <asm/*.h>, driver-specific.

Done

> >
> > +#if (LINUX_VERSION_CODE > KERNEL_VERSION(2,5,75))
> > +#include <linux/device.h>
> > +# define _REQ_FW_DEV_T struct device *
> > +#else
> > +# define _REQ_FW_DEV_T char *
> > +#endif
> >
> > Eeek, why don't you simply pass the pci_dev down?

TODO

> >
> >
> > +typedef struct isl38xx_cb isl38xx_control_block;
> >
> > No useless typedefs please.
> >
> > +MODULE_PARM(init_mode, "i");
> > +MODULE_PARM_DESC(init_mode,
> > + "Set card mode:\n0: Auto\n1: Ad-Hoc\n2: Managed Client (Default)\n3: Master / Access Point\n4: Repeater (Not supported yet)\n5: Secondary (Not supported yet)\n6: Monitor");
> >
> > Please use module_param

Done

>
> I would even say that this is useless because the driver
> support WE, and WE scripts set the mode before the card is up.

True, we can just remove the param for iw_mode.

>
> > diff -Naur -X /home/mcgrof/lib/dontdiff linux-2.6.3/drivers/net/wireless/prism54/isl_wds.c linux-2.6.3-prism54/drivers/net/wireless/prism54/isl_wds.c
> > --- linux-2.6.3/drivers/net/wireless/prism54/isl_wds.c Thu Jan 1 00:00:00 1970
> > +++ linux-2.6.3-prism54/drivers/net/wireless/prism54/isl_wds.c Thu Mar 4 02:00:01 2004
> >
> > WDS doesn't belong into a driver but in higher-level code.

The driver features some firmware-specific WDS functionality, such as
adding/removing WDS links. We haven't looked much into it yet though since
it's not high priority. Where exactly should this code go to then?

Elements noted as DONE were just committed into our CVS repository. You
can always find our latest 2.6 kernel patch at:

http://prism54.org/pub/linux/snapshot/kernel/v2.6/patch-2.6-prism54-cvs-latest.bz2

FWIW, the same driver code base supports 2.4:
http://prism54.org/pub/linux/snapshot/kernel/v2.4/patch-2.4-prism54-cvs-latest.bz2

Luis

--
GnuPG Key fingerprint = 113F B290 C6D2 0251 4D84 A34A 6ADD 4937 E20A 525E


Attachments:
(No filename) (4.15 kB)
(No filename) (189.00 B)
Download all attachments

2004-03-10 23:38:22

by James Ketrenos

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

Jeff Garzik wrote:

> Yes, it would be good to end the cycle of re-implementing 802.11 over
> and over again ;-)

< snip >

> * start working on generic 802.11 stack in wireless-2.6 queue

As we're currently walking the path of implementing the same thing for the
IPW2100 driver, being able to re-use this code would be _very nice_.

I'd like to get WEP into IPW2100 as soon as possible, and would like to do so in
a way that would make transitioning to a common 802.11 layer seamless (or at
least reasonably isolated). Any suggestions on how to best do this, or where we
might be able to help, would be much appreciated.

I'm very interested in these discussions, so if they move to or are being
discussed on another list (besides netdev), please let me know where.

Thanks,
James

2004-03-11 02:25:29

by Jouni Malinen

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

On Wed, Mar 10, 2004 at 01:07:17PM -0500, Jeff Garzik wrote:

> Maybe the prism54 and hostAP developers could join us on netdev? It
> would be nice to have everybody in one place, if we're all gonna be
> collaborating on a generic 802.11 stack.

I am (and have been for quite a while) on netdev, so as far as anything
related to IEEE 802.11 stack or Host AP driver is concerned, I certainly
do follow netdev list, too.

--
Jouni Malinen PGP id EFC895FA

2004-03-11 02:34:30

by Jouni Malinen

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

On Wed, Mar 10, 2004 at 05:37:16PM -0600, James Ketrenos wrote:

> I'd like to get WEP into IPW2100 as soon as possible, and would like to do
> so in a way that would make transitioning to a common 802.11 layer seamless
> (or at least reasonably isolated). Any suggestions on how to best do this,
> or where we might be able to help, would be much appreciated.

Host AP driver (http://hostap.epitest.fi/) has generic (i.e., hardware
independent) implementation of IEEE 802.11 encryption for WEP, TKIP, and
CCMP. These functions take in skb's with IEEE 802.11 headers and
encrypt/decrypt the frames. I haven't yet taken a look at your IPW2100
driver, but if you are including IEEE 802.11 headers in the skb's at
some point, I would assume that the WEP implementation from Host AP
driver would work fine with that driver, too.

The current implementation has hardware independent module (hostap.ko)
that exports the crypto functions for IEEE 802.11 skb's. Both client
station and AP is supported. (To be honest, there is probably still
couple of small Prism2-specific parts in hostap.ko, but not in the
crypto parts and I'm in the process of getting rid of the remaining
wlan hardware dependent parts).

I'm in the process of replacing the algorithm parts (RC4, Michael MIC,
AES-CCM) with crypto API versions so that only the IEEE 802.11 specific
parts (like format of the IV/packet number, replay protection,
pseudo-header for authentication) remain in the implementation and
low-level crypto algorithms can share code with other uses. Once this is
done, I would hope to get the code merged into the kernel tree either
with full Host AP driver or separately. One option would be to first add
Host AP driver in its current structure (i.e., everything in
drivers/net/wireless) and then create a new directory (net/ieee80211 ?)
for generic IEEE 802.11 functionality and start moving things like the
IEEE 802.11 encryption into the new location.

--
Jouni Malinen PGP id EFC895FA

2004-03-11 02:43:23

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

Jouni Malinen wrote:
> done, I would hope to get the code merged into the kernel tree either
> with full Host AP driver or separately. One option would be to first add
> Host AP driver in its current structure (i.e., everything in
> drivers/net/wireless) and then create a new directory (net/ieee80211 ?)
> for generic IEEE 802.11 functionality and start moving things like the
> IEEE 802.11 encryption into the new location.

Given the discussion today, I think my preference is to merge all of
HostAP into the wireless-2.6 tree I just created, then submit patches to
that which create and populate net/802_11. Once the work on that is
mostly done, it can get merged back into the main upstream tree.

Jeff



2004-03-11 02:51:08

by Jouni Malinen

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

On Wed, Mar 10, 2004 at 12:58:28PM -0500, Jeff Garzik wrote:

> So here is my suggested plan:
> * I merge prism54 upstream
> * I create wireless-2.6 queue
> * somebody (you, Jouni(sp?)) submits HostAP to me
> * I merge HostAP

Sounds good to me. I have the Kconfig/Makefile(etc.) patches ready and
the current CVS snapshot of Host AP driver supports 2.6.x kernel
versions, so in theory it is ready to be submitted.

When this topic came up some time ago, I got one concrete comment about
needed changes before the merge (I think it was from you) and that was
to replace the internal encryption algorithms with crypto API ones. I'm
currently in the process of doing this and submitting needed changes for
crypto API. WEP and TKIP have the needed parts as crypto API components
(RC4 is already in kernel tree, Michael MIC patch is pending). CCMP
requires some work (new encryption mode, counter with CBC-MAC, but AES
is already in crypto API).

What would be the preferred order for the HostAP submission? I'm
currently doing the crypto changes in the Host AP CVS repository, but I
can do this also in another repository since you mentioned a new
non-mainline queue for wireless-2.6. I have also some other cleanup
things in my to do list (like getting rid of 2.4 and old wireless
extensions compatibility code, because this would not be needed in the
kernel tree anymore). Again, this is currently proceeding in my CVS
repository, but it can also be done elsewhere, if that is desired.

I'm going to be at the IEEE 802.11 meeting for the next week which is
probably going to take more or less all of my time, but I should be able
to allocate more time after that. If people are interested in reviewing
the current Host AP code from the viewpoint of what would need to happen
before it can be merged into the kernel tree, the latest version is
available as a snapshot from my CVS tree (pserver or tarball) at
http://hostap.epitest.fi/. The current version is almost 20k lines, so
there is certainly quite a bit of code to review. I hope to get this to
about 15k lines, though, with the crypto API and backwards
compatibility cleanup.

--
Jouni Malinen PGP id EFC895FA

2004-03-11 03:03:11

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

Jouni Malinen wrote:
> I'm going to be at the IEEE 802.11 meeting for the next week which is
> probably going to take more or less all of my time, but I should be able
> to allocate more time after that. If people are interested in reviewing
> the current Host AP code from the viewpoint of what would need to happen
> before it can be merged into the kernel tree, the latest version is
> available as a snapshot from my CVS tree (pserver or tarball) at
> http://hostap.epitest.fi/. The current version is almost 20k lines, so
> there is certainly quite a bit of code to review. I hope to get this to
> about 15k lines, though, with the crypto API and backwards
> compatibility cleanup.

How about submitting a patch, when the CryptoAPI and backcompat cleanups
are complete? I will apply to the wireless-2.6 tree, and we can start
working on the kernel's overall 802.11 support from there. I don't
pretend to be an 802.11 expert, so I'll let you and Jean and other
developers drive that end of things. I'm mainly interested in keeping
the API simple and clean, and maintainable.

Jeff



2004-03-11 03:19:50

by Jouni Malinen

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

On Wed, Mar 10, 2004 at 10:02:20PM -0500, Jeff Garzik wrote:

> How about submitting a patch, when the CryptoAPI and backcompat cleanups
> are complete? I will apply to the wireless-2.6 tree, and we can start
> working on the kernel's overall 802.11 support from there.

OK, I'll do the cleanups in the Host AP CVS and submit the patch after
that. I'll make a new branch for this cleanup, so the changes will be
available there if people are interested in commenting the code before
the cleanup is complete.

--
Jouni Malinen PGP id EFC895FA

2004-03-15 22:43:25

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

On St 10-03-04 21:43:08, Jeff Garzik wrote:
> Jouni Malinen wrote:
> >done, I would hope to get the code merged into the kernel tree either
> >with full Host AP driver or separately. One option would be to first add
> >Host AP driver in its current structure (i.e., everything in
> >drivers/net/wireless) and then create a new directory (net/ieee80211 ?)
> >for generic IEEE 802.11 functionality and start moving things like the
> >IEEE 802.11 encryption into the new location.
>
> Given the discussion today, I think my preference is to merge all of
> HostAP into the wireless-2.6 tree I just created, then submit patches to
> that which create and populate net/802_11. Once the work on that is
> mostly done, it can get merged back into the main upstream tree.

Can we have net/wifi? 802_11 looks ugly. 802.11 would be better, but
wifi seems best.
Pavel
--
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

2004-03-15 22:50:13

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

Pavel Machek wrote:
> Can we have net/wifi? 802_11 looks ugly. 802.11 would be better, but
> wifi seems best.

That's fine, I'm not picky about it...

Jeff



2004-03-15 22:57:54

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [PATCH 2.6] Intersil Prism54 wireless driver

On Mon, Mar 15, 2004 at 11:18:26PM +0100, Pavel Machek wrote:
> On St 10-03-04 21:43:08, Jeff Garzik wrote:
> > Jouni Malinen wrote:
> > >done, I would hope to get the code merged into the kernel tree either
> > >with full Host AP driver or separately. One option would be to first add
> > >Host AP driver in its current structure (i.e., everything in
> > >drivers/net/wireless) and then create a new directory (net/ieee80211 ?)
> > >for generic IEEE 802.11 functionality and start moving things like the
> > >IEEE 802.11 encryption into the new location.
> >
> > Given the discussion today, I think my preference is to merge all of
> > HostAP into the wireless-2.6 tree I just created, then submit patches to
> > that which create and populate net/802_11. Once the work on that is
> > mostly done, it can get merged back into the main upstream tree.
>
> Can we have net/wifi? 802_11 looks ugly. 802.11 would be better, but
> wifi seems best.
> Pavel

IEEE 802.11 is a technical standard. WiFi is a marketing
certification. There are 802.11 products which are not WiFi certified
(actually, there is one such driver already in the kernel).
By the way, I don't really care about the final name, I'm hust
being pedantic ;-)
Have fun...

Jean