lkml.org 
[lkml]   [2004]   [Mar]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    SubjectRe: [PATCH 2.6] Intersil Prism54 wireless driver
    From
    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

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 14:01    [W:0.028 / U:1.576 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site