On Wed, Jan 18, 2006 at 10:07:16AM -0700, Matthew Wilcox was heard to remark: > On Wed, Jan 18, 2006 at 10:53:45AM -0600, linas wrote: > > Hi James, > > Actually, I'm the sym2 maintainer. Ah, sorry, then. > > +static void sym_eeh_timeout(u_long p) > > Please don't propagate more uses of this u_long crap. I haven't got > rid of all of them, but there's no need to add more. I was trying to follow the coding style as currently found in that file; the other routines of this type use u_long. Should I use u_int instead? or unsigned long? > > @@ -1630,6 +1682,8 @@ static struct Scsi_Host * __devinit sym_ > > np->maxoffs = dev->chip.offset_max; > > np->maxburst = dev->chip.burst_max; > > np->myaddr = dev->host_id; > > + np->s.io_state = pci_channel_io_normal; > > + np->s.io_reset_wait = NULL; > > Is there a good reason to put them in the 's'? It's really a dirty > hack. The struct sym_shcb is defined in a file "sym_glue.h" which seems to contain linux-kernel specific code. The comments before this struct state that its a "System specific host data structure." By contrast, struct sym_hcb is defined in "sym_hipd.h", which appears to have no Linux-specific structs in it. Thus, I assumed that it should go into the glue struct rather than the main struct. I can of course add it to sym_hcb if that is a better location. > I really hate "if (constant == variable)". Sorry, I'll fix it. I find it easier on the eyes to read that way. Old habit. (It has the marginal benefit of flagging as a compiler error any accidental drop of an =, such as if (variable = constant) which is usually not what the coder intended). I'll fix the other bits you mentioned as well. --linas - : send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
