* AMD 53c974 SCSI driver in 2.6 @ 2003-10-25 21:42 Guennadi Liakhovetski 2003-10-26 19:49 ` Guennadi Liakhovetski 0 siblings, 1 reply; 16+ messages in thread From: Guennadi Liakhovetski @ 2003-10-25 21:42 UTC (permalink / raw) To: linux-kernel Hello The above (AM53C974) driver depends on BROKEN in 2.6 and doesn't compile. Is anybody working on / planning to fix it? Thanks Guennadi --- Guennadi Liakhovetski ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: AMD 53c974 SCSI driver in 2.6 2003-10-25 21:42 AMD 53c974 SCSI driver in 2.6 Guennadi Liakhovetski @ 2003-10-26 19:49 ` Guennadi Liakhovetski 2003-10-30 22:12 ` [PATCH] " Guennadi Liakhovetski 0 siblings, 1 reply; 16+ messages in thread From: Guennadi Liakhovetski @ 2003-10-26 19:49 UTC (permalink / raw) To: linux-kernel; +Cc: Kurt Garloff, linux-scsi On Sat, 25 Oct 2003, Guennadi Liakhovetski wrote: > The above (AM53C974) driver depends on BROKEN in 2.6 and doesn't compile. > Is anybody working on / planning to fix it? In a private email Matthias Andree suggested to try the tmscsim driver, which is [retty badly broken too. So, the question: what is the future of these drivers? Are they slowly dying because the hardware is outdated, or are they going to be fixed? If the latter - what is the status of this work (if any), and - which of the 2 (or both) drivers should survive? Thanks Guennadi --- Guennadi Liakhovetski ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] Re: AMD 53c974 SCSI driver in 2.6 2003-10-26 19:49 ` Guennadi Liakhovetski @ 2003-10-30 22:12 ` Guennadi Liakhovetski 2003-10-30 23:52 ` Kurt Garloff 2003-10-31 9:46 ` Christoph Hellwig 0 siblings, 2 replies; 16+ messages in thread From: Guennadi Liakhovetski @ 2003-10-30 22:12 UTC (permalink / raw) To: linux-kernel; +Cc: Kurt Garloff, Matthias Andree On Sun, 26 Oct 2003, Guennadi Liakhovetski wrote: > > The above (AM53C974) driver depends on BROKEN in 2.6 and doesn't compile. > > Is anybody working on / planning to fix it? Ok, I fixed it, well, at least, it works for me. What now? The fix is, probably, not perfect. E.g. it doesn't support multiple cards now, but it looks like the driver didn't support them even when it worked in its latest version (sorry, if I am wrong). Patch attached. Comments / improvement suggestions mostly welcome. Guennadi --- Guennadi Liakhovetski diff -ur linux-2.6.0-test7.arm/drivers/scsi/AM53C974.c linux-2.6.0-test7/drivers/scsi/AM53C974.c --- linux-2.6.0-test7.arm/drivers/scsi/AM53C974.c Wed Oct 8 23:26:43 2003 +++ linux-2.6.0-test7/drivers/scsi/AM53C974.c Thu Oct 30 23:00:40 2003 @@ -1,6 +1,5 @@ -#error Please convert me to Documentation/DMA-mapping.txt - #include <linux/module.h> +#include <linux/version.h> #include <linux/delay.h> #include <linux/signal.h> #include <linux/sched.h> @@ -10,6 +9,7 @@ #include <linux/blkdev.h> #include <linux/init.h> #include <linux/spinlock.h> +#include <linux/interrupt.h> #include <asm/io.h> #include <asm/system.h> @@ -361,8 +361,8 @@ static __inline__ void initialize_SCp(Scsi_Cmnd * cmd); static __inline__ void run_main(void); static void AM53C974_main(void); -static void AM53C974_intr(int irq, void *dev_id, struct pt_regs *regs); -static void do_AM53C974_intr(int irq, void *dev_id, struct pt_regs *regs); +static irqreturn_t AM53C974_intr(int irq, void *dev_id, struct pt_regs *regs); +static irqreturn_t do_AM53C974_intr(int irq, void *dev_id, struct pt_regs *regs); static void AM53C974_intr_disconnect(struct Scsi_Host *instance); static int AM53C974_sync_neg(struct Scsi_Host *instance, int target, unsigned char *msg); static __inline__ void AM53C974_set_async(struct Scsi_Host *instance, int target); @@ -382,7 +382,11 @@ static struct Scsi_Host *first_instance; static Scsi_Host_Template *the_template; +#undef AM53C974_MULTIPLE_CARD +#ifdef AM53C974_MULTIPLE_CARD +#error "FIXME! Multiple card support is broken. Looks like it never really worked. Might have to be fixed." static struct Scsi_Host *first_host; /* Head of list of AMD boards */ +#endif static volatile int main_running; static int commandline_current; override_t overrides[7] = @@ -457,8 +461,7 @@ printk("AM53C974: coroutine is%s running.\n", main_running ? "" : "n't"); - save_flags(flags); - cli(); + local_irq_save(flags); if (!hostdata->connected) { printk("scsi%d: no currently connected command\n", instance->host_no); @@ -489,7 +492,7 @@ print_Scsi_Cmnd(ptr); } - restore_flags(flags); + local_irq_restore(flags); } #endif /* AM53C974_DEBUG */ @@ -504,14 +507,17 @@ static void AM53C974_print(struct Scsi_Host *instance) { AM53C974_local_declare(); +#if 0 /* Called only from error-handling paths with sufficient protection? */ unsigned long flags; +#endif unsigned long ctcreg, dmastc, dmaspa, dmawbc, dmawac; unsigned char cmdreg, statreg, isreg, cfireg, cntlreg[4], dmacmd, dmastatus; AM53C974_setio(instance); - save_flags(flags); - cli(); +#if 0 /* Called only from error-handling paths with sufficient protection? */ + local_irq_save(flags); +#endif ctcreg = AM53C974_read_8(CTCHREG) << 16; ctcreg |= AM53C974_read_8(CTCMREG) << 8; ctcreg |= AM53C974_read_8(CTCLREG); @@ -529,7 +535,9 @@ dmawbc = AM53C974_read_32(DMAWBC); dmawac = AM53C974_read_32(DMAWAC); dmastatus = AM53C974_read_8(DMASTATUS); - restore_flags(flags); +#if 0 /* Called only from error-handling paths with sufficient protection? */ + local_irq_restore(flags); +#endif printk("AM53C974 register dump:\n"); printk("IO base: 0x%04lx; CTCREG: 0x%04lx; CMDREG: 0x%02x; STATREG: 0x%02x; ISREG: 0x%02x\n", @@ -559,15 +567,14 @@ return; #endif - save_flags(flags); - cli(); + local_irq_save(flags); while ((inb_p(0x64) & 0x01) != 0x01); #ifdef AM53C974_DEBUG key = inb(0x60); if (key == 0x93) deb_stop = 0; /* don't stop if 'r' was pressed */ #endif - restore_flags(flags); + local_irq_restore(flags); } #ifndef MODULE @@ -667,7 +674,10 @@ { AM53C974_local_declare(); int i, j; - struct Scsi_Host *instance, *search; + struct Scsi_Host *instance; +#ifdef AM53C974_MULTIPLE_CARD + struct Scsi_Host *search; +#endif struct AM53C974_hostdata *hostdata; #ifdef AM53C974_OPTION_DEBUG_PROBE_ONLY @@ -739,21 +749,24 @@ return 0; } /* Set up an interrupt handler if we aren't already sharing an IRQ with another board */ +#ifdef AM53C974_MULTIPLE_CARD for (search = first_host; search && (((the_template != NULL) && (search->hostt != the_template)) || (search->irq != instance->irq) || (search == instance)); search = search->next); if (!search) { +#endif if (request_irq(instance->irq, do_AM53C974_intr, SA_SHIRQ, "AM53C974", instance)) { printk("scsi%d: IRQ%d not free, detaching\n", instance->host_no, instance->irq); scsi_unregister(instance); return 0; } +#ifdef AM53C974_MULTIPLE_CARD } else { printk("scsi%d: using interrupt handler previously installed for scsi%d\n", instance->host_no, search->host_no); } - +#endif if (!the_template) { the_template = instance->hostt; first_instance = instance; @@ -832,7 +845,7 @@ if (cmd->use_sg) { cmd->SCp.buffer = (struct scatterlist *) cmd->buffer; cmd->SCp.buffers_residual = cmd->use_sg - 1; - cmd->SCp.ptr = (char *) cmd->SCp.buffer->address; + cmd->SCp.ptr = (char *) page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset; cmd->SCp.this_residual = cmd->SCp.buffer->length; } else { cmd->SCp.buffer = NULL; @@ -858,15 +871,14 @@ static __inline__ void run_main(void) { unsigned long flags; - save_flags(flags); - cli(); + local_irq_save(flags); if (!main_running) { /* main_running is cleared in AM53C974_main once it can't do more work, and AM53C974_main exits with interrupts disabled. */ main_running = 1; AM53C974_main(); } - restore_flags(flags); + local_irq_restore(flags); } /************************************************************************** @@ -891,8 +903,7 @@ struct AM53C974_hostdata *hostdata = (struct AM53C974_hostdata *) instance->hostdata; Scsi_Cmnd *tmp; - save_flags(flags); - cli(); + local_irq_save(flags); DEB_QUEUE(printk(SEPARATOR_LINE)); DEB_QUEUE(printk("scsi%d: AM53C974_queue_command called\n", instance->host_no)); DEB_QUEUE(printk("cmd=%02x target=%02x lun=%02x bufflen=%d use_sg = %02x\n", @@ -924,7 +935,7 @@ /* Run the coroutine if it isn't already running. */ run_main(); - restore_flags(flags); + local_irq_restore(flags); return 0; } @@ -952,12 +963,14 @@ * the host adapters have anything that can be done, at which point * we set main_running to 0 and exit. */ - save_flags(flags); - cli(); /* Freeze request queues */ + local_irq_save(flags); /* Freeze request queues */ do { done = 1; +#ifdef AM53C974_MULTIPLE_CARD for (instance = first_instance; instance && instance->hostt == the_template; instance = instance->next) { +#endif + instance = first_instance; hostdata = (struct AM53C974_hostdata *) instance->hostdata; AM53C974_setio(instance); /* start to select target if we are not connected and not in the @@ -993,10 +1006,12 @@ DEB(printk("main: connected; cmd = 0x%lx, sel_cmd = 0x%lx\n", (long) hostdata->connected, (long) hostdata->sel_cmd)); } +#ifdef AM53C974_MULTIPLE_CARD } /* for instance */ +#endif } while (!done); main_running = 0; - restore_flags(flags); + local_irq_restore(flags); } /************************************************************************ @@ -1008,14 +1023,16 @@ * * * Returns : nothing * ************************************************************************/ -static void do_AM53C974_intr(int irq, void *dev_id, struct pt_regs *regs) +static irqreturn_t do_AM53C974_intr(int irq, void *dev_id, struct pt_regs *regs) { unsigned long flags; struct Scsi_Host *dev = dev_id; - + irqreturn_t ret; + spin_lock_irqsave(dev->host_lock, flags); - AM53C974_intr(irq, dev_id, regs); + ret = AM53C974_intr(irq, dev_id, regs); spin_unlock_irqrestore(dev->host_lock, flags); + return ret; } /************************************************************************ @@ -1027,7 +1044,7 @@ * * * Returns : nothing * ************************************************************************/ -static void AM53C974_intr(int irq, void *dev_id, struct pt_regs *regs) +static irqreturn_t AM53C974_intr(int irq, void *dev_id, struct pt_regs *regs) { AM53C974_local_declare(); struct Scsi_Host *instance; @@ -1035,10 +1052,13 @@ unsigned char cmdreg, dmastatus, statreg, isreg, instreg, cfifo; /* find AM53C974 hostadapter responsible for this interrupt */ +#ifdef AM53C974_MULTIPLE_CARD for (instance = first_instance; instance; instance = instance->next) +#endif + instance = first_instance; if ((instance->irq == irq) && (instance->hostt == the_template)) goto FOUND; - return; + return IRQ_NONE; /* found; now decode and process */ FOUND: @@ -1065,8 +1085,7 @@ /* DMA transfer done */ unsigned long residual; unsigned long flags; - save_flags(flags); - cli(); + local_irq_save(flags); if (!(AM53C974_read_8(DMACMD) & DMACMD_DIR)) { do { dmastatus = AM53C974_read_8(DMASTATUS); @@ -1095,10 +1114,10 @@ AM53C974_information_transfer(instance, statreg, isreg, instreg, cfifo, dmastatus); } - restore_flags(flags); + local_irq_restore(flags); } if (!(dmastatus & DMASTATUS_SCSIINT)) { - return; + return IRQ_HANDLED; } /*** SCSI related interrupts ***/ cmdreg = AM53C974_read_8(CMDREG); @@ -1138,8 +1157,7 @@ #endif DEB(printk("Bus reset interrupt received\n")); AM53C974_intr_bus_reset(instance); - save_flags(flags); - cli(); + local_irq_save(flags); if (hostdata->connected) { hostdata->connected->result = DID_RESET << 16; hostdata->connected->scsi_done((Scsi_Cmnd *) hostdata->connected); @@ -1151,11 +1169,11 @@ hostdata->sel_cmd = NULL; } } - restore_flags(flags); + local_irq_restore(flags); if (hostdata->in_reset == 1) goto EXIT; else - return; + return IRQ_HANDLED; } if (instreg & INSTREG_ICMD) { /* INVALID COMMAND INTERRUPT */ @@ -1172,20 +1190,18 @@ unsigned long flags; /* DISCONNECT INTERRUPT */ DEB_INTR(printk("Disconnect interrupt received; ")); - save_flags(flags); - cli(); + local_irq_save(flags); AM53C974_intr_disconnect(instance); - restore_flags(flags); + local_irq_restore(flags); goto EXIT; } if (instreg & INSTREG_RESEL) { unsigned long flags; /* RESELECTION INTERRUPT */ DEB_INTR(printk("Reselection interrupt received\n")); - save_flags(flags); - cli(); + local_irq_save(flags); AM53C974_intr_reselect(instance, statreg); - restore_flags(flags); + local_irq_restore(flags); goto EXIT; } if (instreg & INSTREG_SO) { @@ -1193,15 +1209,14 @@ if (hostdata->selecting) { unsigned long flags; DEB_INTR(printk("DSR completed, starting select\n")); - save_flags(flags); - cli(); + local_irq_save(flags); AM53C974_select(instance, (Scsi_Cmnd *) hostdata->sel_cmd, (hostdata->sel_cmd->cmnd[0] == REQUEST_SENSE) ? TAG_NONE : TAG_NEXT); hostdata->selecting = 0; AM53C974_set_sync(instance, hostdata->sel_cmd->device->id); - restore_flags(flags); - return; + local_irq_restore(flags); + return IRQ_HANDLED; } if (hostdata->sel_cmd != NULL) { if (((isreg & ISREG_IS) != ISREG_OK_NO_STOP) && @@ -1209,22 +1224,20 @@ unsigned long flags; /* UNSUCCESSFUL SELECTION */ DEB_INTR(printk("unsuccessful selection\n")); - save_flags(flags); - cli(); + local_irq_save(flags); hostdata->dma_busy = 0; LIST(hostdata->sel_cmd, hostdata->issue_queue); hostdata->sel_cmd->host_scribble = (unsigned char *) hostdata->issue_queue; hostdata->issue_queue = hostdata->sel_cmd; hostdata->sel_cmd = NULL; hostdata->selecting = 0; - restore_flags(flags); + local_irq_restore(flags); goto EXIT; } else { unsigned long flags; /* SUCCESSFUL SELECTION */ DEB(printk("successful selection; cmd=0x%02lx\n", (long) hostdata->sel_cmd)); - save_flags(flags); - cli(); + local_irq_save(flags); hostdata->dma_busy = 0; hostdata->disconnecting = 0; hostdata->connected = hostdata->sel_cmd; @@ -1232,7 +1245,7 @@ hostdata->selecting = 0; #ifdef SCSI2 if (!hostdata->conneted->device->simple_tags) -#else +#endif hostdata->busy[hostdata->connected->device->id] |= (1 << hostdata->connected->device->lun); /* very strange -- use_sg is sometimes nonzero for request sense commands !! */ if ((hostdata->connected->cmnd[0] == REQUEST_SENSE) && hostdata->connected->use_sg) { @@ -1243,16 +1256,15 @@ initialize_SCp((Scsi_Cmnd *) hostdata->connected); hostdata->connected->SCp.phase = PHASE_CMDOUT; AM53C974_information_transfer(instance, statreg, isreg, instreg, cfifo, dmastatus); - restore_flags(flags); - return; + local_irq_restore(flags); + return IRQ_HANDLED; } } else { unsigned long flags; - save_flags(flags); - cli(); + local_irq_save(flags); AM53C974_information_transfer(instance, statreg, isreg, instreg, cfifo, dmastatus); - restore_flags(flags); - return; + local_irq_restore(flags); + return IRQ_HANDLED; } } if (instreg & INSTREG_SR) { @@ -1260,20 +1272,20 @@ if (hostdata->connected) { unsigned long flags; DEB_INTR(printk("calling information_transfer\n")); - save_flags(flags); - cli(); + local_irq_save(flags); AM53C974_information_transfer(instance, statreg, isreg, instreg, cfifo, dmastatus); - restore_flags(flags); + local_irq_restore(flags); } else { printk("scsi%d: weird: service request when no command connected\n", instance->host_no); AM53C974_write_8(CMDREG, CMDREG_CFIFO); } /* clear FIFO */ - return; + return IRQ_HANDLED; } EXIT: DEB_INTR(printk("intr: starting main\n")); run_main(); DEB_INTR(printk("end of intr\n")); + return IRQ_HANDLED; } /************************************************************************** @@ -1546,7 +1558,7 @@ if ((!cmd->SCp.this_residual) && cmd->SCp.buffers_residual) { cmd->SCp.buffer++; cmd->SCp.buffers_residual--; - cmd->SCp.ptr = (unsigned char *) cmd->SCp.buffer->address; + cmd->SCp.ptr = (unsigned char *) page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset; cmd->SCp.this_residual = cmd->SCp.buffer->length; } if (cmd->SCp.this_residual) { @@ -2268,7 +2280,6 @@ static int AM53C974_abort(Scsi_Cmnd * cmd) { AM53C974_local_declare(); - unsigned long flags; struct Scsi_Host *instance = cmd->device->host; struct AM53C974_hostdata *hostdata = (struct AM53C974_hostdata *) instance->hostdata; Scsi_Cmnd *tmp, **prev; @@ -2276,8 +2287,6 @@ #ifdef AM53C974_DEBUG deb_stop = 1; #endif - save_flags(flags); - cli(); AM53C974_setio(instance); DEB_ABORT(printk(SEPARATOR_LINE)); @@ -2292,7 +2301,6 @@ DEB_ABORT(printk("scsi%d: aborting connected command\n", instance->host_no)); hostdata->aborted = 1; hostdata->msgout[0] = ABORT; - restore_flags(flags); return (SCSI_ABORT_PENDING); } /* Case 2 : If the command hasn't been issued yet, @@ -2307,7 +2315,6 @@ (*prev) = (Scsi_Cmnd *) tmp->host_scribble; tmp->host_scribble = NULL; tmp->result = DID_ABORT << 16; - restore_flags(flags); tmp->done(tmp); return (SCSI_ABORT_SUCCESS); } @@ -2329,7 +2336,6 @@ * we fail. */ if (hostdata->connected || hostdata->sel_cmd) { DEB_ABORT(printk("scsi%d : abort failed, other command connected.\n", instance->host_no)); - restore_flags(flags); return (SCSI_ABORT_NOT_RUNNING); } /* Case 4: If the command is currently disconnected from the bus, and @@ -2345,7 +2351,6 @@ hostdata->selecting = 1; hostdata->sel_cmd = tmp; AM53C974_write_8(CMDREG, CMDREG_DSR); - restore_flags(flags); return (SCSI_ABORT_PENDING); } } @@ -2358,7 +2363,6 @@ * so we won't panic, but we will notify the user in case something really * broke. */ DEB_ABORT(printk("scsi%d : abort failed, command not found.\n", instance->host_no)); - restore_flags(flags); return (SCSI_ABORT_NOT_RUNNING); } @@ -2370,20 +2374,15 @@ * Inputs : cmd -- which command within the command block was responsible for the reset * * Returns : status (SCSI_ABORT_SUCCESS) -* -* FIXME(eric) the reset_flags are ignored. **************************************************************************/ -static int AM53C974_reset(Scsi_Cmnd * cmd, unsigned int reset_flags) +static int AM53C974_reset(Scsi_Cmnd * cmd) { AM53C974_local_declare(); - unsigned long flags; int i; struct Scsi_Host *instance = cmd->device->host; struct AM53C974_hostdata *hostdata = (struct AM53C974_hostdata *) instance->hostdata; AM53C974_setio(instance); - save_flags(flags); - cli(); DEB(printk("AM53C974_reset called; ")); printk("AM53C974_reset called\n"); @@ -2417,10 +2416,9 @@ udelay(40); AM53C974_config_after_reset(instance); - restore_flags(flags); cmd->result = DID_RESET << 16; cmd->scsi_done(cmd); - return SCSI_ABORT_SUCCESS; + return SCSI_RESET_SUCCESS; } @@ -2451,8 +2449,8 @@ .release = AM53C974_release, .info = AM53C974_info, .queuecommand = AM53C974_queue_command, - .abort = AM53C974_abort, - .reset = AM53C974_reset, + .eh_abort_handler = AM53C974_abort, + .eh_bus_reset_handler = AM53C974_reset, .can_queue = 12, .this_id = -1, .sg_tablesize = SG_ALL, diff -ur linux-2.6.0-test7.arm/drivers/scsi/AM53C974.h linux-2.6.0-test7/drivers/scsi/AM53C974.h --- linux-2.6.0-test7.arm/drivers/scsi/AM53C974.h Sat Aug 9 06:40:41 2003 +++ linux-2.6.0-test7/drivers/scsi/AM53C974.h Thu Oct 30 23:05:41 2003 @@ -53,9 +53,7 @@ static int AM53C974_pci_detect(Scsi_Host_Template * tpnt); static int AM53C974_release(struct Scsi_Host *shp); static const char *AM53C974_info(struct Scsi_Host *); -static int AM53C974_command(Scsi_Cmnd * SCpnt); static int AM53C974_queue_command(Scsi_Cmnd * cmd, void (*done) (Scsi_Cmnd *)); static int AM53C974_abort(Scsi_Cmnd * cmd); -static int AM53C974_reset(Scsi_Cmnd * cmd, unsigned int); - +static int AM53C974_reset(Scsi_Cmnd * cmd); #endif /* AM53C974_H */ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6 2003-10-30 22:12 ` [PATCH] " Guennadi Liakhovetski @ 2003-10-30 23:52 ` Kurt Garloff 2003-10-31 9:47 ` Christoph Hellwig 2003-10-31 9:46 ` Christoph Hellwig 1 sibling, 1 reply; 16+ messages in thread From: Kurt Garloff @ 2003-10-30 23:52 UTC (permalink / raw) To: Guennadi Liakhovetski; +Cc: linux-kernel, Matthias Andree [-- Attachment #1: Type: text/plain, Size: 858 bytes --] Guennadi, Great! On Thu, Oct 30, 2003 at 11:12:57PM +0100, Guennadi Liakhovetski wrote: > Ok, I fixed it, well, at least, it works for me. What now? The fix is, > probably, not perfect. E.g. it doesn't support multiple cards now, but it > looks like the driver didn't support them even when it worked in its > latest version (sorry, if I am wrong). > > Patch attached. Comments / improvement suggestions mostly welcome. I think it should just be merged; Christoph will certainly find places to criticize ... but that's the way the stuff gets better. Interested in taking tmscsim as well? I promised to port it, and I will, but it won't happen during the next ten days :-( Regards, -- Kurt Garloff <garloff@suse.de> Cologne, DE SUSE LINUX AG, Nuernberg, DE SUSE Labs (Head) [-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6 2003-10-30 23:52 ` Kurt Garloff @ 2003-10-31 9:47 ` Christoph Hellwig 2003-10-31 9:59 ` Kurt Garloff 0 siblings, 1 reply; 16+ messages in thread From: Christoph Hellwig @ 2003-10-31 9:47 UTC (permalink / raw) To: Kurt Garloff, Guennadi Liakhovetski, linux-kernel, Matthias Andree On Fri, Oct 31, 2003 at 12:52:04AM +0100, Kurt Garloff wrote: > Interested in taking tmscsim as well? > I promised to port it, and I will, but it won't happen during the next > ten days :-( Any reason why we'd keep both drivers? Given that there's not much ressources for fixing drivers for older hardware I'd rather see us not keeping multiple drivers for the same hardware. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6 2003-10-31 9:47 ` Christoph Hellwig @ 2003-10-31 9:59 ` Kurt Garloff 2003-10-31 10:06 ` Christoph Hellwig 0 siblings, 1 reply; 16+ messages in thread From: Kurt Garloff @ 2003-10-31 9:59 UTC (permalink / raw) To: Christoph Hellwig, Guennadi Liakhovetski, linux-kernel, Matthias Andree [-- Attachment #1: Type: text/plain, Size: 535 bytes --] Christoph, On Fri, Oct 31, 2003 at 09:47:42AM +0000, Christoph Hellwig wrote: > Any reason why we'd keep both drivers? Given that there's not much ressources > for fixing drivers for older hardware I'd rather see us not keeping multiple > drivers for the same hardware. As long as there are people willing to do the work, there's no reason to drop any of them. Regards, -- Kurt Garloff <garloff@suse.de> Cologne, DE SUSE LINUX AG, Nuernberg, DE SUSE Labs (Head) [-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6 2003-10-31 9:59 ` Kurt Garloff @ 2003-10-31 10:06 ` Christoph Hellwig 0 siblings, 0 replies; 16+ messages in thread From: Christoph Hellwig @ 2003-10-31 10:06 UTC (permalink / raw) To: Kurt Garloff, Guennadi Liakhovetski, linux-kernel, Matthias Andree On Fri, Oct 31, 2003 at 10:59:10AM +0100, Kurt Garloff wrote: > On Fri, Oct 31, 2003 at 09:47:42AM +0000, Christoph Hellwig wrote: > > Any reason why we'd keep both drivers? Given that there's not much ressources > > for fixing drivers for older hardware I'd rather see us not keeping multiple > > drivers for the same hardware. > > As long as there are people willing to do the work, there's no reason to > drop any of them. Well, yes. If we had two properly maintained drivers there would be no reason to drop one. But we currently have two broken drivers, one of them just resurrected to compiling and mostly working on UP state.. -- Christoph Hellwig <hch@lst.de> - Freelance Hacker Contact me for driver hacking and kernel development consulting ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6 2003-10-30 22:12 ` [PATCH] " Guennadi Liakhovetski 2003-10-30 23:52 ` Kurt Garloff @ 2003-10-31 9:46 ` Christoph Hellwig 2003-10-31 11:25 ` Guennadi Liakhovetski 1 sibling, 1 reply; 16+ messages in thread From: Christoph Hellwig @ 2003-10-31 9:46 UTC (permalink / raw) To: Guennadi Liakhovetski; +Cc: linux-kernel, Kurt Garloff, Matthias Andree Any reason you fix this driver? The tmcsim one for the same hardware looks like much better structured (though a bit obsufacted :))? > +#include <linux/version.h> What do you need version.h for? > +#undef AM53C974_MULTIPLE_CARD > +#ifdef AM53C974_MULTIPLE_CARD > +#error "FIXME! Multiple card support is broken. Looks like it never really worked. Might have to be fixed." > static struct Scsi_Host *first_host; /* Head of list of AMD boards */ > +#endif Why do you need the undef? It looks like you need to kill a #define for this symbol somewhere else :) > - save_flags(flags); > - cli(); > + local_irq_save(flags); That's not safe on SMP, you must mark the driver BROKEN_ON_SMP or better fix this. > static void AM53C974_print(struct Scsi_Host *instance) > { > AM53C974_local_declare(); > +#if 0 /* Called only from error-handling paths with sufficient protection? */ > unsigned long flags; > +#endif So don't if 0 it but completely kill it. > /* Set up an interrupt handler if we aren't already sharing an IRQ with another board */ > +#ifdef AM53C974_MULTIPLE_CARD > for (search = first_host; > search && (((the_template != NULL) && (search->hostt != the_template)) || > (search->irq != instance->irq) || (search == instance)); > search = search->next); > if (!search) { > +#endif Not sure whether you're interested in fixing this, but the proper way to fix that would be to call request_irq for each card, mark the irq's sharable. The irq handler then can use the void * argument to find the right host and you can kill all this ugly host list walking. That shold get multiple host support working again in theory (ok, except that the driver has a totally broken single state machine..) > if (cmd->use_sg) { > cmd->SCp.buffer = (struct scatterlist *) cmd->buffer; > cmd->SCp.buffers_residual = cmd->use_sg - 1; > - cmd->SCp.ptr = (char *) cmd->SCp.buffer->address; > + cmd->SCp.ptr = (char *) page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset; This means you need a dma_mask < highmem to work. I don't think we want such crude hacks merged, could you please convert it to the proper dma API? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6 2003-10-31 9:46 ` Christoph Hellwig @ 2003-10-31 11:25 ` Guennadi Liakhovetski 2003-10-31 11:46 ` Christoph Hellwig 0 siblings, 1 reply; 16+ messages in thread From: Guennadi Liakhovetski @ 2003-10-31 11:25 UTC (permalink / raw) To: Christoph Hellwig; +Cc: linux-kernel, garloff, matthias.andree, gl (Replying fromo the web-interface, hope, the email will not be scrued up completely). > Any reason you fix this driver? The tmcsim one for the same hardware > looks like much better structured (though a bit obsufacted :))? This is the easiest to asnwer: Kurt Garloff wrote to me, saying that he would fix the tmcsim driver, so, I didn't want to duplicate the work. Also, I was considering this mostly as a fun-excersice, not really hoping / aiming at merging it inthe kernel (also given the current freeze). But now, if Kurt doesn't mind, I can try fixing the other driver, and, hopefully, with your help, I'll try to do it properly. > > +#include <linux/version.h> > > What do you need version.h for? Yep, I had my changes encapsulated in #if KERNEL_VERSION... first, but then I removed them, but forgot to remove this include. > > +#undef AM53C974_MULTIPLE_CARD > > +#ifdef AM53C974_MULTIPLE_CARD > > +#error "FIXME! Multiple card support is broken. Looks like it never > really worked. Might have to be fixed." > > static struct Scsi_Host *first_host; /* Head of list of AMD boards */ > > +#endif > > Why do you need the undef? It looks like you need to kill a #define for > this symbol somewhere else :) So that, when it is fixed, somebody can easily switch it on / off:-) > > - save_flags(flags); > > - cli(); > > + local_irq_save(flags); > > That's not safe on SMP, you must mark the driver BROKEN_ON_SMP or better > fix this. Yes. Again - the fix was pretty much mechanical. I didn't understand why it was considered SMP-safe to just disable local interrupts, so, just preferred to go the "minimal modifications" path. > > static void AM53C974_print(struct Scsi_Host *instance) > > { > > AM53C974_local_declare(); > > +#if 0 /* Called only from error-handling paths with sufficient > protection? */ > > unsigned long flags; > > +#endif > > So don't if 0 it but completely kill it. There's a "?" there:-) Which means, I wasn't quite sure, so, is my assumption correct? I just saw, that other drivers do not implement any locking in these functions. > > /* Set up an interrupt handler if we aren't already sharing an IRQ with > another board */ > > +#ifdef AM53C974_MULTIPLE_CARD > > for (search = first_host; > > search && (((the_template != NULL) && (search->hostt != > the_template)) || > > (search->irq != instance->irq) || (search == instance)); > > search = search->next); > > if (!search) { > > +#endif > > Not sure whether you're interested in fixing this, but the proper way > to fix that would be to call request_irq for each card, mark the irq's > sharable. The irq handler then can use the void * argument to find the > right host and you can kill all this ugly host list walking. That shold > get multiple host support working again in theory (ok, except that the > driver has a totally broken single state machine..) Sure - in irq-handler. But are these lists needed anywhere else, where a function is called in an "abstract" context without a dev-pointer? Probably, not. > > if (cmd->use_sg) { > > cmd->SCp.buffer = (struct scatterlist *) cmd->buffer; > > cmd->SCp.buffers_residual = cmd->use_sg - 1; > > - cmd->SCp.ptr = (char *) cmd->SCp.buffer->address; > > + cmd->SCp.ptr = (char *) page_address(cmd->SCp.buffer->page) + > cmd->SCp.buffer->offset; > > This means you need a dma_mask < highmem to work. I don't think we > want such crude hacks merged, could you please convert it to the proper > dma API? Found in a "working" driver (which has to be fixed too, then). Will try to find a proper fix (for the new driver). So, thanks a lot for commenting on the patch, and, if nobody minds, I'll try to fix the tmcsim driver, will try to do it better this time:-) Thanks Guennadi --- Guennadi Liakhovetski ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6 2003-10-31 11:25 ` Guennadi Liakhovetski @ 2003-10-31 11:46 ` Christoph Hellwig 2003-10-31 12:19 ` Guennadi Liakhovetski 2003-11-02 19:22 ` Guennadi Liakhovetski 0 siblings, 2 replies; 16+ messages in thread From: Christoph Hellwig @ 2003-10-31 11:46 UTC (permalink / raw) To: Guennadi Liakhovetski; +Cc: linux-kernel, garloff, matthias.andree, gl On Fri, Oct 31, 2003 at 12:25:08PM +0100, Guennadi Liakhovetski wrote: > (Replying fromo the web-interface, hope, the email will not be scrued up > completely). Seems fine. > > Any reason you fix this driver? The tmcsim one for the same hardware > > looks like much better structured (though a bit obsufacted :))? > > This is the easiest to asnwer: Kurt Garloff wrote to me, saying that he > would fix the tmcsim driver, so, I didn't want to duplicate the work. Also, > I was considering this mostly as a fun-excersice, not really hoping / aiming Ah, okay. > > > +#ifdef AM53C974_MULTIPLE_CARD > > > +#error "FIXME! Multiple card support is broken. Looks like it never > > really worked. Might have to be fixed." > > > static struct Scsi_Host *first_host; /* Head of list of AMD boards */ > > > +#endif > > > > Why do you need the undef? It looks like you need to kill a #define for > > this symbol somewhere else :) > > So that, when it is fixed, somebody can easily switch it on / off:-) Then just comment ou the #define. Rule of the thumb: #undef always means you screwed up elsewhere :) > > > > - save_flags(flags); > > > - cli(); > > > + local_irq_save(flags); > > > > That's not safe on SMP, you must mark the driver BROKEN_ON_SMP or better > > fix this. > > Yes. Again - the fix was pretty much mechanical. I didn't understand why it > was considered SMP-safe to just disable local interrupts, so, just preferred > to go the "minimal modifications" path. cli() disabled all intereupts in 2.4 which was safe, you only disable local interrupts which is ok on UP but doesn't work on SMP. The right fix would be to introduce spinlocks. But given the driver design this might be everything but easy - that's why I think killing the driver in favour of tmcsim might be the better idea. > > Not sure whether you're interested in fixing this, but the proper way > > to fix that would be to call request_irq for each card, mark the irq's > > sharable. The irq handler then can use the void * argument to find the > > right host and you can kill all this ugly host list walking. That shold > > get multiple host support working again in theory (ok, except that the > > driver has a totally broken single state machine..) > > Sure - in irq-handler. But are these lists needed anywhere else, where a > function is called in an "abstract" context without a dev-pointer? Probably, > not. A poper driver shouldn't do that, but we already know this one isn't.. > > > if (cmd->use_sg) { > > > cmd->SCp.buffer = (struct scatterlist *) cmd->buffer; > > > cmd->SCp.buffers_residual = cmd->use_sg - 1; > > > - cmd->SCp.ptr = (char *) cmd->SCp.buffer->address; > > > + cmd->SCp.ptr = (char *) page_address(cmd->SCp.buffer->page) + > > cmd->SCp.buffer->offset; > > > > This means you need a dma_mask < highmem to work. I don't think we > > want such crude hacks merged, could you please convert it to the proper > > dma API? > > Found in a "working" driver (which has to be fixed too, then). Will try to > find a proper fix (for the new driver). Oh. What driver did you find this construct in? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6 2003-10-31 11:46 ` Christoph Hellwig @ 2003-10-31 12:19 ` Guennadi Liakhovetski 2003-10-31 13:09 ` Russell King 2003-11-02 19:22 ` Guennadi Liakhovetski 1 sibling, 1 reply; 16+ messages in thread From: Guennadi Liakhovetski @ 2003-10-31 12:19 UTC (permalink / raw) To: Christoph Hellwig Cc: Guennadi Liakhovetski, linux-kernel, garloff, matthias.andree On Fri, 31 Oct 2003, Christoph Hellwig wrote: > On Fri, Oct 31, 2003 at 12:25:08PM +0100, Guennadi Liakhovetski wrote: > > > > +#ifdef AM53C974_MULTIPLE_CARD > > > > +#error "FIXME! Multiple card support is broken. Looks like it never > > > really worked. Might have to be fixed." > > > > static struct Scsi_Host *first_host; /* Head of list of AMD boards */ > > > > +#endif > > > > > > Why do you need the undef? It looks like you need to kill a #define for > > > this symbol somewhere else :) > > > > So that, when it is fixed, somebody can easily switch it on / off:-) > > Then just comment ou the #define. Rule of the thumb: #undef always means > you screwed up elsewhere :) Grrrr... Commented out code... > cli() disabled all intereupts in 2.4 which was safe, you only disable local Auch, ok, forgot, that cli() was global in 2.4, thanks for reminding, I don't work with SMPs due to the lack thereof:-( > Oh. What driver did you find this construct in? grep... (maybe not all of them are real hits, but, most of them do certainly look very much like what I've done). BTW, I also saw arrays of cards in some drvers, which also doesn't correspond to your suggestions: drivers/scsi/arm/scsi.h-41- SCp->ptr = (char *) drivers/scsi/arm/scsi.h:42: (page_address(SCp->buffer->page) + drivers/scsi/arm/scsi.h-43- SCp->buffer->offset); -- drivers/scsi/arm/scsi.h-79- SCpnt->SCp.ptr = (char *) drivers/scsi/arm/scsi.h:80: (page_address(SCpnt->SCp.buffer->page) + drivers/scsi/arm/scsi.h-81- SCpnt->SCp.buffer->offset); -- drivers/scsi/sg.c-1071- return (sclp && sclp->page) ? drivers/scsi/sg.c:1072: (unsigned char *) page_address(sclp->page) + sclp->offset : NULL; -- drivers/scsi/st.c:3517: res = copy_from_user(page_address(st_bp->frp[i].page) + offset, ubp, cnt); -- drivers/scsi/st.c:3550: res = copy_to_user(ubp, page_address(st_bp->frp[i].page) + offset, cnt); -- drivers/scsi/st.c:3593: memmove(page_address(st_bp->frp[dst_seg].page) + dst_offset, drivers/scsi/st.c:3594: page_address(st_bp->frp[src_seg].page) + src_offset, count); -- drivers/scsi/scsi_debug.c:260: return (unsigned char *)page_address(sclp->page) + drivers/scsi/scsi_debug.c-261- sclp->offset; -- drivers/scsi/in2000.c:377: cmd->SCp.ptr = (char *) page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset; -- drivers/scsi/in2000.c:769: cmd->SCp.ptr = page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset; -- drivers/scsi/ide-scsi.c:149: buf = page_address(pc->sg->page) + pc->sg->offset; -- drivers/scsi/ide-scsi.c:171: buf = page_address(pc->sg->page) + pc->sg->offset; -- drivers/scsi/NCR53C9x.c:924: (char *) virt_to_phys((page_address(sp->SCp.buffer->page) + sp->SCp.buffer->offset)); -- drivers/scsi/NCR53C9x.c:1758: sp->SCp.ptr = (char *) virt_to_phys((page_address(sp->SCp.buffer->page) + sp->SCp.buffer->offset)); -- drivers/scsi/imm.c:840: cmd->SCp.ptr = page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset; -- drivers/scsi/imm.c:981: cmd->SCp.ptr = page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset; -- drivers/scsi/ips.c:215:#define IPS_SG_ADDRESS(sg) (page_address((sg)->page) ? \ drivers/scsi/ips.c:216: page_address((sg)->page)+(sg)->offset : 0) -- drivers/scsi/ppa.c:752: cmd->SCp.ptr = page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset; -- drivers/scsi/ppa.c:903: cmd->SCp.ptr = page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset; -- drivers/scsi/qlogicfas.c:426: buf = page_address(sglist->page) + sglist->offset; -- drivers/scsi/sun3x_esp.c:349: sg[sz].dvma_address = dvma_map((unsigned long)page_address(sg[sz].page) + drivers/scsi/sun3x_esp.c-350- sg[sz].offset, sg[sz].length); -- drivers/scsi/aha1542.c:72: page_address(sgpnt[badseg].page) + sgpnt[badseg].offset, -- drivers/scsi/aha1542.c:719: (page_address(sgpnt[i].page) + drivers/scsi/aha1542.c-720- sgpnt[i].offset), -- drivers/scsi/aha152x.c:584:#define SG_ADDRESS(buffer) ((char *) (page_address((buffer)->page)+(buffer)->offset)) -- drivers/scsi/oktagon_esp.c:559: sp->SCp.ptr = page_address(sp->SCp.buffer->page)+ drivers/scsi/oktagon_esp.c-560- sp->SCp.buffer->offset; -- drivers/scsi/oktagon_esp.c:573: sp->SCp.ptr = page_address(sp->SCp.buffer->page)+ drivers/scsi/oktagon_esp.c-574- sp->SCp.buffer->offset; -- drivers/scsi/megaraid.c:2040: page_address((&sgl[0])->page) + drivers/scsi/megaraid.c-2041- (&sgl[0])->offset; -- drivers/scsi/fd_mcs.c:1024: current_SC->SCp.ptr = page_address(current_SC->SCp.buffer->page) + current_SC->SCp.buffer->offset; -- drivers/scsi/fd_mcs.c:1057: current_SC->SCp.ptr = page_address(current_SC->SCp.buffer->page) + current_SC->SCp.buffer->offset; -- drivers/scsi/fd_mcs.c:1160: current_SC->SCp.ptr = page_address(current_SC->SCp.buffer->page) + current_SC->SCp.buffer->offset; -- drivers/scsi/dc395x.c:238:#define CPU_ADDR(sg) (page_address((sg).page)+(sg).offset) -- drivers/scsi/sun3_NCR5380.c:273:#define SGADDR(buffer) (void *)(((unsigned long)page_address((buffer)->page)) + \ drivers/scsi/sun3_NCR5380.c-274- (buffer)->offset) -- drivers/scsi/seagate.c:989: page_address(buffer[i].page) + buffer[i].offset, -- drivers/scsi/seagate.c:996: data = page_address(buffer->page) + buffer->offset; -- drivers/scsi/seagate.c:1243: data = page_address(buffer->page) + buffer->offset; -- drivers/scsi/seagate.c:1417: data = page_address(buffer->page) + buffer->offset; -- drivers/scsi/fdomain.c:1294: current_SC->SCp.ptr = page_address(current_SC->SCp.buffer->page) + current_SC->SCp.buffer->offset; -- drivers/scsi/fdomain.c:1327: current_SC->SCp.ptr = page_address(current_SC->SCp.buffer->page) + current_SC->SCp.buffer->offset; -- drivers/scsi/fdomain.c:1413: current_SC->SCp.ptr = page_address(current_SC->SCp.buffer->page) + current_SC->SCp.buffer->offset; -- drivers/scsi/osst.c:4280: STp->buffer->aux = (os_aux_t *) (page_address(STp->buffer->sg[i].page) + OS_DATA_SIZE - b_size); -- drivers/scsi/osst.c:5146: res = copy_from_user(page_address(st_bp->sg[i].page) + offset, ubp, cnt); -- drivers/scsi/osst.c:5179: res = copy_to_user(ubp, page_address(st_bp->sg[i].page) + offset, cnt); -- drivers/scsi/osst.c:5212: memset(page_address(st_bp->sg[i].page) + offset, 0, cnt); -- drivers/scsi/NCR53c406a.c:884: NCR53c406a_pio_write(page_address(sglist->page) + sglist->offset, sglist->length); -- drivers/scsi/NCR53c406a.c:911: NCR53c406a_pio_read(page_address(sglist->page) + sglist->offset, sglist->length); -- drivers/scsi/atari_NCR5380.c:472: virt_to_phys(page_address(cmd->SCp.buffer[1].page)+ drivers/scsi/atari_NCR5380.c-473- cmd->SCp.buffer[1].offset) == endaddr; ) { -- drivers/scsi/atari_NCR5380.c:510: cmd->SCp.ptr = (char *)page_address(cmd->SCp.buffer->page)+ drivers/scsi/atari_NCR5380.c-511- cmd->SCp.buffer->offset; -- drivers/scsi/atari_NCR5380.c:2044: cmd->SCp.ptr = page_address(cmd->SCp.buffer->page)+ drivers/scsi/atari_NCR5380.c-2045- cmd->SCp.buffer->offset; -- drivers/scsi/sym53c416.c:200:#define SG_ADDRESS(buffer) ((char *) (page_address((buffer)->page)+(buffer)->offset)) -- drivers/scsi/53c7xx.c:3453: ? (u32)page_address(((struct scatterlist *)cmd->buffer)[i].page)+ drivers/scsi/53c7xx.c-3454- ((struct scatterlist *)cmd->buffer)[i].offset -- drivers/scsi/53c7xx.c:5420: buffers && !((found = ((ptr >= (char *)page_address(segment->page)+segment->offset) && drivers/scsi/53c7xx.c:5421: (ptr < ((char *)page_address(segment->page)+segment->offset+segment->length))))); -- drivers/scsi/53c7xx.c:5425: cmd->device->host->host_no, saved, page_address(segment->page+segment->offset); -- drivers/scsi/53c7xx.c:5429: offset += ptr - ((char *)page_address(segment->page)+segment->offset); -- drivers/scsi/eata_pio.c:184: SCp->ptr = page_address(SCp->buffer->page) + SCp->buffer->offset; -- drivers/scsi/eata_pio.c:417: cmd->SCp.ptr = page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset; -- drivers/scsi/NCR5380.c:337: cmd->SCp.ptr = page_address(cmd->SCp.buffer->page)+ drivers/scsi/NCR5380.c-338- cmd->SCp.buffer->offset; -- drivers/scsi/NCR5380.c:2338: cmd->SCp.ptr = page_address(cmd->SCp.buffer->page)+ drivers/scsi/NCR5380.c-2339- cmd->SCp.buffer->offset; -- drivers/scsi/wd33c93.c:388: cmd->SCp.ptr = page_address(cmd->SCp.buffer->page) + drivers/scsi/wd33c93.c-389- cmd->SCp.buffer->offset; -- drivers/scsi/wd33c93.c:721: cmd->SCp.ptr = page_address(cmd->SCp.buffer->page) + drivers/scsi/wd33c93.c-722- cmd->SCp.buffer->offset; -- drivers/scsi/advansys.c:6728: (unsigned char *)page_address(slp->page) + slp->offset)); -- drivers/scsi/advansys.c:6987: (unsigned char *)page_address(slp->page) + slp->offset)); Thanks Guennadi --------------------------------- Guennadi Liakhovetski, Ph.D. DSA Daten- und Systemtechnik GmbH Pascalstr. 28 D-52076 Aachen Germany ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6 2003-10-31 12:19 ` Guennadi Liakhovetski @ 2003-10-31 13:09 ` Russell King 0 siblings, 0 replies; 16+ messages in thread From: Russell King @ 2003-10-31 13:09 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: Christoph Hellwig, Guennadi Liakhovetski, linux-kernel, garloff, matthias.andree On Fri, Oct 31, 2003 at 01:19:56PM +0100, Guennadi Liakhovetski wrote: > > Oh. What driver did you find this construct in? > > grep... (maybe not all of them are real hits, but, most of them do > certainly look very much like what I've done). BTW, I also saw arrays of > cards in some drvers, which also doesn't correspond to your suggestions: > > drivers/scsi/arm/scsi.h-41- SCp->ptr = (char *) > drivers/scsi/arm/scsi.h:42: (page_address(SCp->buffer->page) + > drivers/scsi/arm/scsi.h-43- SCp->buffer->offset); > -- > drivers/scsi/arm/scsi.h-79- SCpnt->SCp.ptr = (char *) > drivers/scsi/arm/scsi.h:80: (page_address(SCpnt->SCp.buffer->page) + > drivers/scsi/arm/scsi.h-81- SCpnt->SCp.buffer->offset); The drivers which use this will never be used on highmem-capable machines. -- Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/ 2.6 Serial core ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6 2003-10-31 11:46 ` Christoph Hellwig 2003-10-31 12:19 ` Guennadi Liakhovetski @ 2003-11-02 19:22 ` Guennadi Liakhovetski 1 sibling, 0 replies; 16+ messages in thread From: Guennadi Liakhovetski @ 2003-11-02 19:22 UTC (permalink / raw) To: Christoph Hellwig; +Cc: linux-scsi, linux-kernel, garloff, gl > > > Any reason you fix this driver? The tmcsim one for the same hardware > > > looks like much better structured (though a bit obsufacted :))? Ok, started looking at the tmscsim. A couple of questions: 1) After the "next" element has disappeared from the struct scsi_cmnd, what is the "correct" / preferred way to queue scsi commands in drivers? I saw aic7xxx (new) casting a part of struct scsi_pointer SCp in scsi_cmnd, starting from Status to a list_head (or an anology thereof), which doesn't seem very nice. Anyway, I didn't find any "standard" way for doing this. Should host_scribble be used? 2) Actually, which scsi driver (or, better, several drivers) can be considered well-written and can be taken as examples? I tried looking at aic7xxx, as it is a pretty new one, but I am not sure if it is really a good example to follow and it is pretty big too. Thanks Guennadi P.S. Should this thread be taken to linux-scsi or is it better to continue it on lkml? --- Guennadi Liakhovetski ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] Re: AMD 53c974 SCSI driver in 2.6 @ 2003-12-11 9:01 Tomas Martisius 2003-12-11 11:12 ` Christoph Hellwig 0 siblings, 1 reply; 16+ messages in thread From: Tomas Martisius @ 2003-12-11 9:01 UTC (permalink / raw) To: linux-kernel [-- Attachment #1: Type: text/plain, Size: 690 bytes --] -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hello, On Sun, 26 Oct 2003, Guennadi Liakhovetski posted patch for AM53c974 driver. May be it is not perfect, but it is still not applied to kernel source. I think it is better to have at least not broken driver compared with existing. I have Compaq deskpro XL 590 PC with integrated such controler, and this driver affter applinig Guennadi Liakhovetski pach to 2.6.0-test11 source works for me. Best regards Tomas Martisius VMU network administrator -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.1 (GNU/Linux) iD8DBQE/2DJcAMslIG6CXk4RAtb9AJ98s5l+T7IqACGrp1l/Mv7PBxuFtQCg5g+0 rwpS2VetZGl8rS49hXE2aPM= =EYze -----END PGP SIGNATURE----- [-- Attachment #2: AM53C974.diff.gz --] [-- Type: application/x-tar, Size: 4888 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6 2003-12-11 9:01 Tomas Martisius @ 2003-12-11 11:12 ` Christoph Hellwig 2003-12-11 20:10 ` Guennadi Liakhovetski 0 siblings, 1 reply; 16+ messages in thread From: Christoph Hellwig @ 2003-12-11 11:12 UTC (permalink / raw) To: Tomas Martisius; +Cc: linux-kernel On Thu, Dec 11, 2003 at 11:01:16AM +0200, Tomas Martisius wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Hello, > > On Sun, 26 Oct 2003, Guennadi Liakhovetski posted patch for > AM53c974 driver. May be it is not perfect, but it is still not applied > to kernel source. I think it is better to have at least not broken > driver compared with existing. > I have Compaq deskpro XL 590 PC with integrated such controler, and this > driver affter applinig Guennadi Liakhovetski pach to 2.6.0-test11 source > works for me. The patch is broken but he has a better patch for the other driver for that hardware, that will hopefully go into 2.6.1. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6 2003-12-11 11:12 ` Christoph Hellwig @ 2003-12-11 20:10 ` Guennadi Liakhovetski 0 siblings, 0 replies; 16+ messages in thread From: Guennadi Liakhovetski @ 2003-12-11 20:10 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Tomas Martisius, linux-kernel On Thu, 11 Dec 2003, Christoph Hellwig wrote: > On Thu, Dec 11, 2003 at 11:01:16AM +0200, Tomas Martisius wrote: > > On Sun, 26 Oct 2003, Guennadi Liakhovetski posted patch for > > AM53c974 driver. May be it is not perfect, but it is still not applied > > to kernel source. I think it is better to have at least not broken > > driver compared with existing. > > I have Compaq deskpro XL 590 PC with integrated such controler, and this > > driver affter applinig Guennadi Liakhovetski pach to 2.6.0-test11 source > > works for me. > > The patch is broken but he has a better patch for the other driver for > that hardware, that will hopefully go into 2.6.1. Right, I think, you can safely use the patch for AM53C974 for now, although it is broken, it should work for you (I have a slightly newer Deskpro XL 5133), wait for 2.6.1 and just enable tmscsim:-) Guennadi --- Guennadi Liakhovetski ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2003-12-11 20:38 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2003-10-25 21:42 AMD 53c974 SCSI driver in 2.6 Guennadi Liakhovetski 2003-10-26 19:49 ` Guennadi Liakhovetski 2003-10-30 22:12 ` [PATCH] " Guennadi Liakhovetski 2003-10-30 23:52 ` Kurt Garloff 2003-10-31 9:47 ` Christoph Hellwig 2003-10-31 9:59 ` Kurt Garloff 2003-10-31 10:06 ` Christoph Hellwig 2003-10-31 9:46 ` Christoph Hellwig 2003-10-31 11:25 ` Guennadi Liakhovetski 2003-10-31 11:46 ` Christoph Hellwig 2003-10-31 12:19 ` Guennadi Liakhovetski 2003-10-31 13:09 ` Russell King 2003-11-02 19:22 ` Guennadi Liakhovetski 2003-12-11 9:01 Tomas Martisius 2003-12-11 11:12 ` Christoph Hellwig 2003-12-11 20:10 ` Guennadi Liakhovetski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).