Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 27 Apr 1999 09:19:21 -0400 (EDT) | From | Eddie Mansu <> | Subject | Re: [PATCH]: aha1542.c, support for insmod parameters |
| |
On Tue, 27 Apr 1999 kernel@whitestar.soark.net wrote:
> 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/
| ![\](/images/icornerr.gif) |