On Tue, 14 Jun 2005, Christoph Hellwig wrote: > > +#ifndef PCI_DEVICE_ID_QLOGIC_ISP2512 > > +#define PCI_DEVICE_ID_QLOGIC_ISP2512 0x2512 > > +#endif > > + > > +#ifndef PCI_DEVICE_ID_QLOGIC_ISP2522 > > +#define PCI_DEVICE_ID_QLOGIC_ISP2522 0x2522 > > +#endif > > It's only an historic accident that the older ids are in here, care > to remove all and move them to pci_ids.h instead? > > > + int (*start_scsi)(srb_t *); > > umm, that's most of ->queuecommand, isn't it? Shouldn't you define > a separate scsi_host_template for 24xx/25xx? > > > +//ISP24xx > > please try to avoid C++-style comments > > > + volatile uint16_t mailbox0; > > + volatile uint16_t mailbox1; > > + volatile uint16_t mailbox2; > > never use volatile, always use proper kernel primitives to access > hardware or atomic_t for atomic in-kernel variables. No problem. I'll have another patchset which hopefully addresses the concerns and with a bit of additional testing in the next week or so. Thanks, Andrew - : 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