On Tue, 2009-01-27 at 15:07 -0800, Jing Huang wrote: > > > -----Original Message----- > > From: Christoph Hellwig [mailto:hch@xxxxxxxxxxxxx] > > Sent: Tuesday, January 27, 2009 2:16 PM > > To: Jing Huang > > Cc: James.Bottomley@xxxxxxxxxxxxxxxxxxxxx; Krishna Gudipati; linux- > > kernel@xxxxxxxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx; Ramkumar Vadivelu; > > Vinodh Ravindran > > Subject: Re: [PATCH 2/5] bfa: Brocade BFA FC SCSI driver submission > > > > On Tue, Jan 27, 2009 at 10:54:07AM -0800, Jing Huang wrote: > > > +/** > > > + * Generic HAL callback element. > > > + */ > > > +struct bfa_cb_qe_s { > > > + struct bfa_q_s qe; > > > + bfa_cb_cbfn_t cbfn; > > > + bfa_boolean_t once; > > > + u32 rsvd; > > > + void *cbarg; > > > +}; > > > + > > > +#define bfa_cb_queue(__bfa, __hcb_qe, __cbfn, __cbarg) do { > \ > > > + (__hcb_qe)->cbfn = (__cbfn); > \ > > > + (__hcb_qe)->cbarg = (__cbarg); > \ > > > + bfa_q_enq(&(__bfa)->comp_q, (__hcb_qe)); > \ > > > +} while (0) > > > > Please stop here, weird HALs and own list management routines are not > > things we put into the tree. (except for gregs junk subdirectory..) > > > > Please rewrite this into a proper Linux driver. > > > Thanks Christoph for the code review, > > I will clean up all the HAL references and also replace our own list > management code with standard linux implementation. This might take a while. I can offer to put this driver in the staging tree while you work on it if that would be helpful? The way the staging tree works is that drivers are held in a separate area (drivers/staging) in the linux kernel while they're being worked on. The driver can be installed by users while it's in the staging tree (so made use of) but it will taint the kernel with a warning that it's not of linux kernel quality yet. Once we get an acceptable driver in drivers/staging, I'll move it across to drivers/scsi and remove the taint. James -- To unsubscribe from this list: 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
