> I have done the same thing myself but I think my patch is a little
> cleaner, though it currently does not support setting the dma speed..
>
> <snip>
> > +/* These are used with insmod parameters */
> > +static int aha1542[NUMPARMS] = {
> > + 0x0, /* base address */
> > + 0, /* buson */
> > + 0, /* busoff */
> > + 0, /* dmaspeed */
> > +};
>
> This is IMHO ugly, we can name them now, why don't we?
The main reason I did this was to maintain a certain consistency
with drivers I've seen.. I originally implemented it in a very similar
fashion to the way you did in your attached patch, but decided it would be
better just to have one parameter called aha1542.
> > +
> > +static int base_address = 0;
> > +
> > +static int allowed_base_address[] = {0x130,0x134,0x230,0x234,0x330,0x334};
> > +static int buson = 0;
> > +static int allowed_buson[] = {2,3,4,5,6,7,8,9,10,11,12,13,14,15};
> > +static int busoff = 0;
> > +static int allowed_busoff[] = {1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,
> > + 17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,
> > + 33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,
> > + 49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64};
> > +static int dmaspeed = 0;
> > +static int allowed_dmaspeed[] = {5,6,7,8,10};
>
> LOTS of checking which is not done if compiled into the kernel, perhaps
> it should be added if its really needed, however I don't see the point,
> and there are much cleaner ways then this..
Yeah, this checking really isn't necessary, but I just felt like
adding it. There are some cleaner ways I can think of, but my method
seems to most clear to me. I could have checked the ranges for buson,
busoff, and even dmaspeed, but I'd basically have to check every value for
base_address, and I wanted to keep it consistent throughout the code. I
do suppose I could remove the checking altogether though, since it isn't
really necessary.
> <snip>
> > /* Eventually this will go into an include file, but this will be later */
> > Scsi_Host_Template driver_template = AHA1542;
> >
> > -#include "scsi_module.c"
> > -#endif
> > +int init_module(void) {
>
> Why not just copy the code from above where the boot prams are handled?
Stupidity on my part, perhaps? For some reason, it didn't even
occur to me to do that.
> Done..
Thanks. I may go through what I've done and see if I can clean it
up a bit.
Eddie
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/