Re: [PATCH 03/15] [media] i2c: make device_type const
On Sat, 19 Aug 2017, Bhumika Goyal wrote: > Make this const as it is only stored in the type field of a device > structure, which is const. > Done using Coccinelle. > > Signed-off-by: Bhumika Goyal <bhumi...@gmail.com> Acked-by: Guennadi Liakhovetski <g.liakhovet...@gmx.de> Thanks Guennadi > --- > drivers/media/i2c/soc_camera/mt9t031.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/soc_camera/mt9t031.c > b/drivers/media/i2c/soc_camera/mt9t031.c > index 714fb35..4802d30 100644 > --- a/drivers/media/i2c/soc_camera/mt9t031.c > +++ b/drivers/media/i2c/soc_camera/mt9t031.c > @@ -592,7 +592,7 @@ static int mt9t031_runtime_resume(struct device *dev) > .runtime_resume = mt9t031_runtime_resume, > }; > > -static struct device_type mt9t031_dev_type = { > +static const struct device_type mt9t031_dev_type = { > .name = "MT9T031", > .pm = _dev_pm_ops, > }; > -- > 1.9.1 >
Re: [PATCHv2 00/12] Re-implement am53c974 driver
Hi Christoph, On Mon, 24 Nov 2014, Christoph Hellwig wrote: I'm tempted to merge this into drivers-for-3.19 ASAP, any argument against that? Guennadi, do you have any additional hardware to Hannes one real and one virtual adapter to test this new driver on before? I've got a similar / identical Tekram dc390 SCSI PCI card and a PC with an on-board am53c974 chip, but that is either a rather old (2 x PII @ 400MHz :)) or a very old (P @ 166MHz) PC of mine, don't think testing on any of those would be viable, even if I dig them out from the basement :) More interesting would be testing with different devices like tape streamers, scanners... But I'm really unsure anyone still uses those, even when I was fixing them, I was pretty much the only affected user :) Thanks Guennadi -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 12/12] Replace tmscsim by am53c974
On Fri, 21 Nov 2014, Hannes Reinecke wrote: The am53c974 is a re-implementation of the tmscsim driver, and provides the same functionality. So remove the tmscsim driver and make am53c974 an alias to tmscsim. Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de Signed-off-by: Hannes Reinecke h...@suse.de --- MAINTAINERS |7 +- drivers/scsi/Kconfig| 16 - drivers/scsi/Makefile |1 - drivers/scsi/am53c974.c |1 + drivers/scsi/tmscsim.c | 2626 --- drivers/scsi/tmscsim.h | 549 -- 6 files changed, 4 insertions(+), 3196 deletions(-) delete mode 100644 drivers/scsi/tmscsim.c delete mode 100644 drivers/scsi/tmscsim.h diff --git a/MAINTAINERS b/MAINTAINERS index d206f37..d780e46 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2862,11 +2862,10 @@ F:Documentation/networking/dmfe.txt F: drivers/net/ethernet/dec/tulip/dmfe.c DC390/AM53C974 SCSI driver -M: Kurt Garloff garl...@suse.de -W: http://www.garloff.de/kurt/linux/dc390/ -M: Guennadi Liakhovetski g.liakhovet...@gmx.de +M: Hannes Reinecke h...@suse.de +L: linux-scsi@vger.kernel.org S: Maintained -F: drivers/scsi/tmscsim.* +F: drivers/scsi/am53c974.c DC395x SCSI driver M: Oliver Neukum oli...@neukum.org Acked-by: Guennadi Liakhovetski g.liakhovet...@gmx.de Thanks Guennadi diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig index 519c3ef..f871a80 100644 --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -1341,22 +1341,6 @@ config SCSI_DC395x To compile this driver as a module, choose M here: the module will be called dc395x. -config SCSI_DC390T - tristate Tekram DC390(T) and Am53/79C974 SCSI support - depends on PCI SCSI - ---help--- - This driver supports PCI SCSI host adapters based on the Am53C974A - chip, e.g. Tekram DC390(T), DawiControl 2974 and some onboard - PCscsi/PCnet (Am53/79C974) solutions. - - Documentation can be found in file:Documentation/scsi/tmscsim.txt. - - Note that this driver does NOT support Tekram DC390W/U/F, which are - based on NCR/Symbios chips. Use NCR53C8XX SCSI support for those. - - To compile this driver as a module, choose M here: the - module will be called tmscsim. - config SCSI_AM53C974 tristate Tekram DC390(T) and Am53/79C974 SCSI support (new driver) depends on PCI SCSI diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile index 79a6571..a9f3fa8 100644 --- a/drivers/scsi/Makefile +++ b/drivers/scsi/Makefile @@ -100,7 +100,6 @@ obj-$(CONFIG_SCSI_EATA_PIO) += eata_pio.o obj-$(CONFIG_SCSI_7000FASST) += wd7000.o obj-$(CONFIG_SCSI_EATA) += eata.o obj-$(CONFIG_SCSI_DC395x)+= dc395x.o -obj-$(CONFIG_SCSI_DC390T)+= tmscsim.o obj-$(CONFIG_SCSI_AM53C974) += esp_scsi.o am53c974.o obj-$(CONFIG_MEGARAID_LEGACY)+= megaraid.o obj-$(CONFIG_MEGARAID_NEWGEN)+= megaraid/ diff --git a/drivers/scsi/am53c974.c b/drivers/scsi/am53c974.c index 55622eb..5ce2bb2 100644 --- a/drivers/scsi/am53c974.c +++ b/drivers/scsi/am53c974.c @@ -572,6 +572,7 @@ MODULE_DESCRIPTION(AM53C974 SCSI driver); MODULE_AUTHOR(Hannes Reinecke h...@suse.de); MODULE_LICENSE(GPL); MODULE_VERSION(DRV_MODULE_VERSION); +MODULE_ALIAS(tmscsim); module_param(am53c974_fenab, bool, 0444); MODULE_PARM_DESC(am53c974_fenab, Enable 24-bit DMA transfer sizes); diff --git a/drivers/scsi/tmscsim.c b/drivers/scsi/tmscsim.c deleted file mode 100644 index 844c9a0..000 --- a/drivers/scsi/tmscsim.c +++ /dev/null @@ -1,2626 +0,0 @@ -/ - * FILE NAME : TMSCSIM.C * - *BY : C.L. Huang, ch...@tekram.com.tw* - * Description: Device Driver for Tekram DC-390(T) PCI SCSI* - *Bus Master Host Adapter* - * (C)Copyright 1995-1996 Tekram Technology Co., Ltd. * - - * (C) Copyright: put under GNU GPL in 10/96 * - * (see Documentation/scsi/tmscsim.txt)* - - * $Id: tmscsim.c,v 2.60.2.30 2000/12/20 01:07:12 garloff Exp $ * - * Enhancements and bugfixes by* - * Kurt Garloff k...@garloff.de garl...@suse.de * - - * HISTORY:* - * * - * REV#DATENAMEDESCRIPTION * - * 1.00 96/04/24 CLH First
Re: [PATCH 00/10] Re-implement am53c974 driver
Hi, On Fri, 21 Nov 2014, Hannes Reinecke wrote: On 11/21/2014 11:26 AM, Christoph Hellwig wrote: On Fri, Nov 21, 2014 at 10:27:47AM +0100, Hannes Reinecke wrote: Hi all, having found some issues with the current tmscsim driver I looked at the code and found is really awkward to clean up. Seeing that the am53c974 chip is actually based on the 'ESP' chip design I've re-implemented it based on the common esp_scsi routines. This new driver (cunningly named 'am53c974') provides all features found on the tmscsim driver, with the goal of obsoleting the old tmscsim driver. Tested with Tekram DC390 and qemu esp-scsi. This looks very nice! Three high level comments: - please Cc Dave for changes to esp_scsi That's what I thought, too, but 'get_maintainers.pl' doesn't know about him. Maybe we should fix this up ... - please Cc Guennadi as the tmscsim maintainer - this probably should deprecated and/or remove the tmscsim driver That was the general idea. I just thought to be conservative and add a new driver. But yeah, the idea is to sent another patch to remove tmscsim altogether. Sure, the driver is rather old and... well... its style is somewhat foreign to the modern Linux kernel coding style :) I actually also thought, that the hardware is really legacy and hardly anyone is still using it, so the tmscsim driver was in a bug-fix only mode, but if you want to replace it with a better written one - certainly go for it! I've hardly had any work with this my maintainer task in the last few years, except for a recent SCSI Tag rehash, but that was committed before I had enough time to sufficiently understand them :) And I'm not using that hardware actively any more these days... So, I'd have nothing against removal of tmscsim and, respectively, from its entry in MAINTAINERS :) Thanks Guennadi -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tmscim: Remove unused SCSI_IRQ_NONE macro definition
On Fri, 3 Oct 2014, Finn Thain wrote: Signed-off-by: Finn Thain fth...@telegraphics.com.au Acked-by: Guennadi Liakhovetski g.liakhovet...@gmx.de Thanks Guennadi --- This macro is only used in the NCR5380 drivers and they don't include this header. --- drivers/scsi/tmscsim.h |2 -- 1 file changed, 2 deletions(-) Index: linux/drivers/scsi/tmscsim.h === --- linux.orig/drivers/scsi/tmscsim.h 2014-09-29 10:55:44.0 +1000 +++ linux/drivers/scsi/tmscsim.h 2014-10-01 16:33:25.0 +1000 @@ -10,8 +10,6 @@ #include linux/types.h -#define SCSI_IRQ_NONE 255 - #define MAX_ADAPTER_NUM 4 #define MAX_SG_LIST_BUF 16 /* Not used */ #define MAX_SCSI_ID 8 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: dc395x.c: Fix for possible null pointer dereference
On Sun, 18 May 2014, Rickard Strandqvist wrote: There is otherwise a risk of a possible null pointer dereference. Was largely found by using a static code analysis program called cppcheck. Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se --- drivers/scsi/dc395x.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c index 83d9bf6..0d86bc7 100644 --- a/drivers/scsi/dc395x.c +++ b/drivers/scsi/dc395x.c @@ -2637,7 +2637,7 @@ static struct ScsiReqBlk *msgin_qtag(struct AdapterCtlBlk *acb, struct ScsiReqBlk *srb = NULL; struct ScsiReqBlk *i; dprintkdbg(DBG_0, msgin_qtag: (0x%p) tag=%i srb=%p\n, -srb-cmd, tag, srb); +srb ? srb-cmd : 0, tag, srb); There's not just a risk, it is a NULL-pointer dereference, so, just remove it, e.g. like - dprintkdbg(DBG_0, msgin_qtag: (0x%p) tag=%i srb=%p\n, -srb-cmd, tag, srb); + dprintkdbg(DBG_0, msgin_qtag: tag=%i\n, tag); Thanks Guennadi if (!(dcb-tag_mask (1 tag))) dprintkl(KERN_DEBUG, -- 1.7.10.4 --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 20/20] [SCSI] tmscsim: remove unnecessary pci_set_drvdata()
On Mon, 23 Sep 2013, Jingoo Han wrote: The driver core clears the driver data to NULL after device_release or on probe failure. Thus, it is not needed to manually clear the device driver data to NULL. Signed-off-by: Jingoo Han jg1@samsung.com Acked-by: Guennadi Liakhovetski g.liakhovet...@gmx.de Thanks Guennadi --- drivers/scsi/tmscsim.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/tmscsim.c b/drivers/scsi/tmscsim.c index 9327f5f..b06a1de 100644 --- a/drivers/scsi/tmscsim.c +++ b/drivers/scsi/tmscsim.c @@ -2553,7 +2553,6 @@ static void dc390_remove_one(struct pci_dev *dev) pci_disable_device(dev); scsi_host_put(scsi_host); - pci_set_drvdata(dev, NULL); } static struct pci_device_id tmscsim_pci_tbl[] = { -- 1.7.10.4 --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 319/493] scsi: remove use of __devinitdata
On Mon, 19 Nov 2012, Bill Pemberton wrote: drivers/scsi/tmscsim.c | 2 +- Acked-by: Guennadi Liakhovetski g.liakhovet...@gmx.de Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 097/493] scsi: remove use of __devexit_p
On Mon, 19 Nov 2012, Bill Pemberton wrote: CONFIG_HOTPLUG is going away as an option so __devexit_p is no longer needed. drivers/scsi/tmscsim.c| 2 +- Acked-by: Guennadi Liakhovetski g.liakhovet...@gmx.de Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 425/493] scsi: remove use of __devexit
On Mon, 19 Nov 2012, Bill Pemberton wrote: CONFIG_HOTPLUG is going away as an option so __devexit is no longer needed. drivers/scsi/tmscsim.c| 2 +- Acked-by: Guennadi Liakhovetski g.liakhovet...@gmx.de Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 192/493] scsi: remove use of __devinit
On Mon, 19 Nov 2012, Bill Pemberton wrote: drivers/scsi/tmscsim.c| 16 +++--- Acked-by: Guennadi Liakhovetski g.liakhovet...@gmx.de Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] tmscsim: spin_unlock_irq in interrupt handler fix
On Tue, 31 Jul 2012, Denis Efremov wrote: The replacement of spin_lock_irq/spin_unlock_irq pairs which can be called from interrupt handler by irqsave/irqrestore versions. Found by Linux Driver Verification project (linuxtesting.org). Signed-off-by: Denis Efremov yefremov.de...@gmail.com Acked-by: Guennadi Liakhovetski g.liakhovet...@gmx.de Note: I didn't test this. I still have a dc390 card in my PC, so, I could test it, but I haven't yet got time for this and I'll be away the next week. The patch looks correct and safe. If it does break anything, well, we'll get to know about that sooner or later... Thanks Guennadi --- drivers/scsi/tmscsim.c | 12 ++-- drivers/scsi/tmscsim.h |1 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/scsi/tmscsim.c b/drivers/scsi/tmscsim.c index a1baccc..e9b7148 100644 --- a/drivers/scsi/tmscsim.c +++ b/drivers/scsi/tmscsim.c @@ -665,7 +665,7 @@ DC390_Interrupt(void *dev_id) //dstatus = DC390_read8 (DMA_Status); //DC390_write32 (DMA_ScsiBusCtrl, EN_INT_ON_PCI_ABORT); -spin_lock_irq(pACB-pScsiHost-host_lock); +spin_lock_irqsave(pACB-pScsiHost-host_lock, pACB-hlock_flags); istate = DC390_read8 (Intern_State); istatus = DC390_read8 (INT_Status); /* This clears Scsi_Status, Intern_State and INT_Status ! */ @@ -736,7 +736,7 @@ DC390_Interrupt(void *dev_id) } unlock: -spin_unlock_irq(pACB-pScsiHost-host_lock); +spin_unlock_irqrestore(pACB-pScsiHost-host_lock, pACB-hlock_flags); return IRQ_HANDLED; } @@ -771,9 +771,9 @@ dc390_DataOut_0(struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 *psstatus) /* Function called from the ISR with the host_lock held and interrupts disabled */ if (pSRB-SGToBeXferLen) while (time_before(jiffies, timeout) !((dstate = DC390_read8 (DMA_Status)) DMA_XFER_DONE)) { - spin_unlock_irq(pACB-pScsiHost-host_lock); + spin_unlock_irqrestore(pACB-pScsiHost-host_lock, pACB-hlock_flags); udelay(50); - spin_lock_irq(pACB-pScsiHost-host_lock); + spin_lock_irqsave(pACB-pScsiHost-host_lock, pACB-hlock_flags); } if (!time_before(jiffies, timeout)) printk (KERN_CRIT DC390: Deadlock in DataOut_0: DMA aborted unfinished: %06x bytes remain!!\n, @@ -830,9 +830,9 @@ dc390_DataIn_0(struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 *psstatus) /* Function called from the ISR with the host_lock held and interrupts disabled */ if (pSRB-SGToBeXferLen) while (time_before(jiffies, timeout) !((dstate = DC390_read8 (DMA_Status)) DMA_XFER_DONE)) { - spin_unlock_irq(pACB-pScsiHost-host_lock); + spin_unlock_irqrestore(pACB-pScsiHost-host_lock, pACB-hlock_flags); udelay(50); - spin_lock_irq(pACB-pScsiHost-host_lock); + spin_lock_irqsave(pACB-pScsiHost-host_lock, pACB-hlock_flags); } if (!time_before(jiffies, timeout)) { printk (KERN_CRIT DC390: Deadlock in DataIn_0: DMA aborted unfinished: %06x bytes remain!!\n, diff --git a/drivers/scsi/tmscsim.h b/drivers/scsi/tmscsim.h index 77adc54..3f9ea2b 100644 --- a/drivers/scsi/tmscsim.h +++ b/drivers/scsi/tmscsim.h @@ -107,6 +107,7 @@ u8SyncOffset; /*;for reg. and nego.(low nibble) */ struct dc390_acb { struct Scsi_Host *pScsiHost; +unsigned long hlock_flags; u16 IOPortBase; u8 IRQLevel; u8 status; -- 1.7.7 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tmscsim: spin_unlock_irq in interrupt handler fix
Wow, been ages since the last patch to this driver has hit my mail:-) On Sat, 21 Jul 2012, Denis Efremov wrote: The replacement of spin_lock_irq/spin_unlock_irq pair in interrupt handler by spin_lock_irqsave/spin_lock_irqrestore pair. Found by Linux Driver Verification project (linuxtesting.org). Signed-off-by: Denis Efremov yefremov.de...@gmail.com Right, unconditionally enabling local IRQs in an ISR seems rude. BUT: unfortunately, it's not as easy as this. From DC390_Interrupt() dc390_DataOut_0() and dc390_DataIn_0() can be called, which also use spin_lock_irq() / spin_unlock_irq()... So, maybe adding a flags field to struct dc390_acb and using it on all 3 occasions would be the easiest and safest fix... Thanks Guennadi --- drivers/scsi/tmscsim.c |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/tmscsim.c b/drivers/scsi/tmscsim.c index a1baccc..0b9d68a 100644 --- a/drivers/scsi/tmscsim.c +++ b/drivers/scsi/tmscsim.c @@ -654,6 +654,7 @@ DC390_Interrupt(void *dev_id) u8 phase; void (*stateV)( struct dc390_acb*, struct dc390_srb*, u8 *); u8 istate, istatus; +unsigned long flags; sstatus = DC390_read8 (Scsi_Status); if( !(sstatus INTERRUPT) ) @@ -665,7 +666,7 @@ DC390_Interrupt(void *dev_id) //dstatus = DC390_read8 (DMA_Status); //DC390_write32 (DMA_ScsiBusCtrl, EN_INT_ON_PCI_ABORT); -spin_lock_irq(pACB-pScsiHost-host_lock); +spin_lock_irqsave(pACB-pScsiHost-host_lock, flags); istate = DC390_read8 (Intern_State); istatus = DC390_read8 (INT_Status); /* This clears Scsi_Status, Intern_State and INT_Status ! */ @@ -736,7 +737,7 @@ DC390_Interrupt(void *dev_id) } unlock: -spin_unlock_irq(pACB-pScsiHost-host_lock); +spin_unlock_irqrestore(pACB-pScsiHost-host_lock, flags); return IRQ_HANDLED; } -- 1.7.7 --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: rootfs on sda async scan?
On Sat, 16 Jun 2007, Matthew Wilcox wrote: On Sun, Jun 17, 2007 at 12:42:44AM +0200, Guennadi Liakhovetski wrote: sd* as long as the respective ldd is linked into the kernel. Now, this just didn't work for me under 2.6.22-rc3-... (scsi-misc-2.6) with sym53c8xx_2. I'd expected it would let scan run asynchronously, but wait as long as root on an sd* is requested. Do I understand it wrong or is this a bug somewhere. I also have 2 more ldd's compiled as modules. Did you have http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-rc-fixes-2.6.git;a=commit;h=f2f027c6e9912840020be8b78f037d5c8ac665e0 in your tree? It's supposed to fix this bug. No, it is still not in scsi-misc-2.6. Thanks for the pointer Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] merge scsiiom.c into tmscsim.c
Hi Christoph, On Sun, 3 Oct 2004, Christoph Hellwig wrote: might be a good time to get rid of that clumsy include .c file into another one thing. (Also please bk rm scsiiom.c afterwards) You certainly will have no problem at all remembering this patch, will you?:-) It's only less than 3 years ago... Sure, you'll also have no problem answering a couple of my simple questions to this patch... Specifically, this place in it: --- 1.56/drivers/scsi/tmscsim.c 2004-10-03 15:43:04 +02:00 +++ edited/drivers/scsi/tmscsim.c 2004-10-03 15:48:31 +02:00 @@ -252,7 +252,7 @@ [snip] +static int __inline__ +dc390_RequestSense(struct dc390_acb* pACB, struct dc390_dcb* pDCB, struct dc390_srb* pSRB) +{ + struct scsi_cmnd *pcmd; + + pcmd = pSRB-pcmd; + + REMOVABLEDEBUG(printk(KERN_INFO DC390: RequestSense(Cmd %02x, Id %02x, LUN %02x)\n,\ + pcmd-cmnd[0], pDCB-TargetID, pDCB-TargetLUN)); + + pSRB-SRBFlag |= AUTO_REQSENSE; + pSRB-SavedSGCount = pcmd-use_sg; + pSRB-SavedTotXLen = pSRB-TotalXferredLen; + pSRB-AdaptStatus = 0; + pSRB-TargetStatus = 0; /* CHECK_CONDITION1; */ + + /* We are called from SRBdone, original PCI mapping has been removed + * already, new one is set up from StartSCSI */ + pSRB-SGIndex = 0; + + pSRB-TotalXferredLen = 0; + pSRB-SGToBeXferLen = 0; + return dc390_StartSCSI(pACB, pDCB, pSRB); +} + + +static void +dc390_SRBdone( struct dc390_acb* pACB, struct dc390_dcb* pDCB, struct dc390_srb* pSRB ) +{ +u8 bval, status; +struct scsi_cmnd *pcmd; + +pcmd = pSRB-pcmd; +/* KG: Moved pci_unmap here */ +dc390_pci_unmap(pSRB); + +status = pSRB-TargetStatus; + +DEBUG0(printk ( SRBdone (%02x,%08x), SRB %p, pid %li\n, status, pcmd-result,\ + pSRB, pcmd-pid)); +if(pSRB-SRBFlag AUTO_REQSENSE) +{/* Last command was a Request Sense */ Here we come in after Request Sense above. + pSRB-SRBFlag = ~AUTO_REQSENSE; + pSRB-AdaptStatus = 0; + pSRB-TargetStatus = CHECK_CONDITION 1; + + //pcmd-result = MK_RES(DRIVER_SENSE,DID_OK,0,status); + if (status == (CHECK_CONDITION 1)) + pcmd-result = MK_RES_LNX(0, DID_BAD_TARGET, 0, /*CHECK_CONDITION*/0); + else /* Retry */ + { + if( pSRB-pcmd-cmnd[0] == TEST_UNIT_READY /* || pSRB-pcmd-cmnd[0] == START_STOP */) + { + /* Don't retry on TEST_UNIT_READY */ + pcmd-result = MK_RES_LNX(DRIVER_SENSE,DID_OK,0,CHECK_CONDITION); + REMOVABLEDEBUG(printk(KERN_INFO Cmd=%02x, Result=%08x, XferL=%08x\n,pSRB-pcmd-cmnd[0],\ +(u32) pcmd-result, (u32) pSRB-TotalXferredLen)); + } else { + SET_RES_DRV(pcmd-result, DRIVER_SENSE); + pcmd-use_sg = pSRB-SavedSGCount; + //pSRB-ScsiCmdLen = (u8) (pSRB-Segment1[0] 8); + DEBUG0 (printk (DC390: RETRY pid %li (%02x), target %02i-%02i\n, pcmd-pid, pcmd-cmnd[0], pcmd-device-id, pcmd-device-lun)); + pSRB-TotalXferredLen = 0; + SET_RES_DID(pcmd-result, DID_SOFT_ERROR); + } + } + goto cmd_done; And here we jump out. +} +if( status ) +{ Therefore here we come only if we didn't request sense before. + if( status_byte(status) == CHECK_CONDITION ) + { + if (dc390_RequestSense(pACB, pDCB, pSRB)) { + SET_RES_DID(pcmd-result, DID_ERROR); + goto cmd_done; + } + return; + } + else if( status_byte(status) == QUEUE_FULL ) + { + bval = (u8) pDCB-GoingSRBCnt; + bval--; + pDCB-MaxCommand = bval; + pcmd-use_sg = pSRB-SavedSGCount; Now, this I don't understand. If we didn't request sense, SavedSGCount is not defined, is it?... Thanks and sorry for taking you on a time-travel Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] merge scsiiom.c into tmscsim.c
On Sun, 17 Jun 2007, Boaz Harrosh wrote: I think the all thing is no longer relevant. It is leftovers from the time scsi_error.c specifically scsi_send_eh_cmnd (used by scsi_request_sense) was stepping all over the scsi_cmnd fields and things needed to be saved, if they were needed for a retry. But now (since a later cleanup made by Christoph) scsi_send_eh_cmnd() does not do that any more. Ok, what I really was trying to understand is the auto request sense path in tmscsim, and, I think, I have a better idea now how it is supposed to work. In fact, I don't know how really necessary this path is. I guess, requesting sense would anyway be handled by the ml, so, doing it in the lld is just a (micro-) optimization, right? So, though superfluous and making the driver less maintainable, we might keep it for now. In any way pcmd-use_sg is never touched anywhere in the code and any code called from this driver. So why save it? Or am I missing some bigger picture Here? Looks like you're right. SavedSGCount seems indeed unneeded. I'll ack your patch. Thanks Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Farther clean-up of tmscsim driver
On Thu, 14 Jun 2007, Boaz Harrosh wrote: From 4a7ac954dcc11531a09fa07d6a6365d98c67b216 Mon Sep 17 00:00:00 2001 From: Boaz Harrosh [EMAIL PROTECTED](none) Date: Thu, 14 Jun 2007 19:04:17 +0300 Subject: [PATCH] Farther clean-up of tmscsim driver - this is a followup of commit 85289f2efa108d1586a86d0c426ffc9d641bbdc2 [SCSI] tmscsim: convert to use the data buffer accessors. The saved sg_count was a leftover from the time the driver was doing dma mapping by himself. But now that scsi-ml is called for the mapping it is not the drivers responsibility. Apart from being line-wrapped and missing a Signed-off-by tag, this patch looks ok to me. Please resend properly formatted and then you can add my Signed-off-by: G. Liakhovetski [EMAIL PROTECTED] Thanks Guennadi --- drivers/scsi/tmscsim.c |3 --- drivers/scsi/tmscsim.h |1 - 2 files changed, 0 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/tmscsim.c b/drivers/scsi/tmscsim.c index 73c5ca0..5758f20 100644 --- a/drivers/scsi/tmscsim.c +++ b/drivers/scsi/tmscsim.c @@ -1680,7 +1680,6 @@ dc390_RequestSense(struct dc390_acb* pACB, struct dc390_dcb* pDCB, struct dc390_ pcmd-cmnd[0], pDCB-TargetID, pDCB-TargetLUN)); pSRB-SRBFlag |= AUTO_REQSENSE; - pSRB-SavedSGCount = scsi_sg_count(pcmd); pSRB-SavedTotXLen = pSRB-TotalXferredLen; pSRB-AdaptStatus = 0; pSRB-TargetStatus = 0; /* CHECK_CONDITION1; */ @@ -1728,7 +1727,6 @@ dc390_SRBdone( struct dc390_acb* pACB, struct dc390_dcb* pDCB, struct dc390_srb* (u32) pcmd-result, (u32) pSRB-TotalXferredLen)); } else { SET_RES_DRV(pcmd-result, DRIVER_SENSE); - scsi_sg_count(pcmd) = pSRB-SavedSGCount; //pSRB-ScsiCmdLen = (u8) (pSRB-Segment1[0] 8); DEBUG0 (printk (DC390: RETRY pid %li (%02x), target %02i-%02i\n, pcmd-pid, pcmd-cmnd[0], pcmd-device-id, pcmd-device-lun)); pSRB-TotalXferredLen = 0; @@ -1750,7 +1748,6 @@ dc390_SRBdone( struct dc390_acb* pACB, struct dc390_dcb* pDCB, struct dc390_srb* else if( status_byte(status) == QUEUE_FULL ) { scsi_track_queue_full(pcmd-device, pDCB-GoingSRBCnt - 1); - scsi_sg_count(pcmd) = pSRB-SavedSGCount; DEBUG0 (printk (DC390: RETRY pid %li (%02x), target %02i-%02i\n, pcmd-pid, pcmd-cmnd[0], pcmd-device-id, pcmd-device-lun)); pSRB-TotalXferredLen = 0; SET_RES_DID(pcmd-result, DID_SOFT_ERROR); diff --git a/drivers/scsi/tmscsim.h b/drivers/scsi/tmscsim.h index c3d8c80..fbe7a3e 100644 --- a/drivers/scsi/tmscsim.h +++ b/drivers/scsi/tmscsim.h @@ -57,7 +57,6 @@ u8 SGcount; u8 MsgCnt; u8 EndMessage; -u8 SavedSGCount; u8 MsgInBuf[6]; u8 MsgOutBuf[6]; -- 1.5.0.4.402.g8035 --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] tmscsim: clean-up status codes
*/ #define RES_DRV0xFF00 /* DRIVER_ codes */ #define MK_RES(drv,did,msg,tgt) ((int)(drv)24 | (int)(did)16 | (int)(msg)8 | (int)(tgt)) -#define MK_RES_LNX(drv,did,msg,tgt) ((int)(drv)24 | (int)(did)16 | (int)(msg)8 | (int)(tgt)1) +#define MK_RES_LNX(drv,did,msg,tgt) ((int)(drv)24 | (int)(did)16 | (int)(msg)8 | (int)(tgt)) #define SET_RES_TARGET(who, tgt) do { who = ~RES_TARGET; who |= (int)(tgt); } while (0) #define SET_RES_TARGET_LNX(who, tgt) do { who = ~RES_TARGET_LNX; who |= (int)(tgt) 1; } while (0) --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: scsi_cmnd accessors issues
Hi On Tue, 12 Jun 2007, Boaz Harrosh wrote: I get compilation errors. Attached solution is to add scsi_set_ accessors to these members. But it would be best to STOP these drivers from doing that ugly hack of modifying sg_count and bufflen. (CC sign-off-by maintainers of the drivers in question. Files are drivers/scsi/tmscsim.c drivers/scsi/stex.c ) Thanks for the patch. I think, getting rid of the SavedSGCount field is a bit more complicated (would be better if you had sent the patch inline). I'm currently working on a solution, please, give me some time. Thanks Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
rootfs on sda async scan?
Hi This is a basic confusion: if I read kernel config help to SCSI_SCAN_ASYNC correctly If you build your SCSI drivers into the kernel, then everything will work fine if you say Y here. means I can have async on and root on sd* as long as the respective ldd is linked into the kernel. Now, this just didn't work for me under 2.6.22-rc3-... (scsi-misc-2.6) with sym53c8xx_2. I'd expected it would let scan run asynchronously, but wait as long as root on an sd* is requested. Do I understand it wrong or is this a bug somewhere. I also have 2 more ldd's compiled as modules. Thanks Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] tmscsim: remove bogus endianness conversions
On Sat, 5 May 2007, Christoph Hellwig wrote: On Fri, May 04, 2007 at 10:59:40PM +0200, Guennadi Liakhovetski wrote: cpu_to_le32 endianness conversions in tmscsim.c, followed by arithmetic operations don't look correct. Besides, {in,out}[wl] already perform the necessary conversions. Further, bus addresses of request buffers are guaranteed to be (mapped) under 4G by current scsi- and block-layer defaults. This could be explicitly enforced by using blk_queue_bounce_limit(), which, however, doesn't seem to be the common practice among SCSI drivers. Looks good to me. If there are structures in dma memory in the driver it would help to give the __le annotation to get the whole endianess handling completely right. Yeah, but these variables are not like that. Actually, I don't think there are any variables in tmscsim that get passed to the controller over DMA. Thanks Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] tmscsim: Remove the last bus_to_virt()
On Sat, 5 May 2007, Christoph Hellwig wrote: On Fri, May 04, 2007 at 11:00:49PM +0200, Guennadi Liakhovetski wrote: Dynamically map the buffer for PIO for the residue byte. This look okay. Btw, is there a chance you could run tmscsim.c through scripts/Lindent before your next round of bigger changes? Sure, with great pleasure:-) And thanks for reviewing these patches! Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] remove long dead DMA_INT
DMA_INT code is disabled since 1998, remove it to prepare for further cleanup. Signed-off-by: G. Liakhovetski [EMAIL PROTECTED] diff --git a/drivers/scsi/tmscsim.c b/drivers/scsi/tmscsim.c index a583e89..d0e171c 100644 --- a/drivers/scsi/tmscsim.c +++ b/drivers/scsi/tmscsim.c @@ -625,70 +625,6 @@ dc390_StartSCSI( struct dc390_acb* pACB, struct dc390_dcb* pDCB, struct dc390_sr return 0; } -//#define DMA_INT EN_DMA_INT /*| EN_PAGE_INT*/ -#define DMA_INT 0 - -#if DMA_INT -/* This is similar to AM53C974.c ... */ -static u8 -dc390_dma_intr (struct dc390_acb* pACB) -{ - struct dc390_srb* pSRB; - u8 dstate; - DEBUG0(u16 pstate; struct pci_dev *pdev = pACB-pdev); - - DEBUG0(pci_read_config_word(pdev, PCI_STATUS, pstate)); - DEBUG0(if (pstate (PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_DETECTED_PARITY))\ - { printk(KERN_WARNING DC390: PCI state = %04x!\n, pstate); \ - pci_write_config_word(pdev, PCI_STATUS, (PCI_STATUS_SIG_SYSTEM_ERROR | PCI_STATUS_DETECTED_PARITY));}); - - dstate = DC390_read8 (DMA_Status); - - if (! pACB-pActiveDCB || ! pACB-pActiveDCB-pActiveSRB) return dstate; - else pSRB = pACB-pActiveDCB-pActiveSRB; - - if (dstate (DMA_XFER_ABORT | DMA_XFER_ERROR | POWER_DOWN | PCI_MS_ABORT)) -{ - printk (KERN_ERR DC390: DMA error (%02x)!\n, dstate); - return dstate; -} - if (dstate DMA_XFER_DONE) -{ - u32 residual, xferCnt; int ctr = 600; - if (! (DC390_read8 (DMA_Cmd) READ_DIRECTION)) - { - do - { - DEBUG1(printk (KERN_DEBUG DC390: read residual bytes ... \n)); - dstate = DC390_read8 (DMA_Status); - residual = DC390_read8 (CtcReg_Low) | DC390_read8 (CtcReg_Mid) 8 | - DC390_read8 (CtcReg_High) 16; - residual += DC390_read8 (Current_Fifo) 0x1f; - } while (residual ! (dstate SCSI_INTERRUPT) --ctr); - if (!ctr) printk (KERN_CRIT DC390: dma_intr: DMA aborted unfinished: %06x bytes remain!!\n, DC390_read32 (DMA_Wk_ByteCntr)); - /* residual = ... */ - } - else - residual = 0; - - /* ??? */ - - xferCnt = pSRB-SGToBeXferLen - residual; - pSRB-SGBusAddr += xferCnt; - pSRB-TotalXferredLen += xferCnt; - pSRB-SGToBeXferLen = residual; -# ifdef DC390_DEBUG0 - printk (KERN_INFO DC390: DMA: residual = %i, xfer = %i\n, - (unsigned int)residual, (unsigned int)xferCnt); -# endif - - DC390_write8 (DMA_Cmd, DMA_IDLE_CMD); -} - dc390_laststatus = ~0xff00; dc390_laststatus |= dstate 24; - return dstate; -} -#endif - static void __inline__ dc390_InvalidCmd(struct dc390_acb* pACB) @@ -708,9 +644,6 @@ DC390_Interrupt(void *dev_id) u8 phase; void (*stateV)( struct dc390_acb*, struct dc390_srb*, u8 *); u8 istate, istatus; -#if DMA_INT -u8 dstatus; -#endif sstatus = DC390_read8 (Scsi_Status); if( !(sstatus INTERRUPT) ) @@ -718,22 +651,9 @@ DC390_Interrupt(void *dev_id) DEBUG1(printk (KERN_DEBUG sstatus=%02x,, sstatus)); -#if DMA_INT -spin_lock_irq(pACB-pScsiHost-host_lock); -dstatus = dc390_dma_intr (pACB); -spin_unlock_irq(pACB-pScsiHost-host_lock); - -DEBUG1(printk (KERN_DEBUG dstatus=%02x,, dstatus)); -if (! (dstatus SCSI_INTERRUPT)) - { - DEBUG0(printk (KERN_WARNING DC390 Int w/o SCSI actions (only DMA?)\n)); - return IRQ_NONE; - } -#else //DC390_write32 (DMA_ScsiBusCtrl, WRT_ERASE_DMA_STAT | EN_INT_ON_PCI_ABORT); //dstatus = DC390_read8 (DMA_Status); //DC390_write32 (DMA_ScsiBusCtrl, EN_INT_ON_PCI_ABORT); -#endif spin_lock_irq(pACB-pScsiHost-host_lock); @@ -879,7 +799,7 @@ dc390_DataOut_0( struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 *psstatus) } if ((*psstatus 7) != SCSI_DATA_OUT) { - DC390_write8 (DMA_Cmd, WRITE_DIRECTION+DMA_IDLE_CMD); /* | DMA_INT */ + DC390_write8 (DMA_Cmd, WRITE_DIRECTION+DMA_IDLE_CMD); DC390_write8 (ScsiCmd, CLEAR_FIFO_CMD); } } @@ -924,7 +844,7 @@ dc390_DataIn_0( struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 *psstatus) + ((unsigned long) DC390_read8 (CtcReg_Low))); DEBUG1(printk (KERN_DEBUG Count_2_Zero (ResidCnt=%i,ToBeXfer=%li),, ResidCnt, pSRB-SGToBeXferLen)); - DC390_write8 (DMA_Cmd, READ_DIRECTION+DMA_IDLE_CMD); /* | DMA_INT */ + DC390_write8 (DMA_Cmd, READ_DIRECTION+DMA_IDLE_CMD); pSRB-TotalXferredLen += pSRB-SGToBeXferLen; pSRB-SGIndex++; @@ -973,7 +893,7 @@ din_1: } /* It seems a DMA Blast abort isn't that bad ... */ if (!i) printk (KERN_ERR DC390: DMA Blast aborted unfinished!\n); - //DC390_write8 (DMA_Cmd, READ_DIRECTION+DMA_IDLE_CMD); /* | DMA_INT */ + //DC390_write8 (DMA_Cmd, READ_DIRECTION+DMA_IDLE_CMD); dc390_laststatus =
[PATCH 2/3] tmscsim: remove bogus endianness conversions
cpu_to_le32 endianness conversions in tmscsim.c, followed by arithmetic operations don't look correct. Besides, {in,out}[wl] already perform the necessary conversions. Further, bus addresses of request buffers are guaranteed to be (mapped) under 4G by current scsi- and block-layer defaults. This could be explicitly enforced by using blk_queue_bounce_limit(), which, however, doesn't seem to be the common practice among SCSI drivers. Signed-off-by: G. Liakhovetski [EMAIL PROTECTED] diff --git a/drivers/scsi/tmscsim.c b/drivers/scsi/tmscsim.c index d0e171c..be869d0 100644 --- a/drivers/scsi/tmscsim.c +++ b/drivers/scsi/tmscsim.c @@ -778,8 +778,8 @@ dc390_DataOut_0( struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 *psstatus) pSRB-pSegmentList++; psgl = pSRB-pSegmentList; - pSRB-SGBusAddr = cpu_to_le32(pci_dma_lo32(sg_dma_address(psgl))); - pSRB-SGToBeXferLen = cpu_to_le32(sg_dma_len(psgl)); + pSRB-SGBusAddr = sg_dma_address(psgl); + pSRB-SGToBeXferLen = sg_dma_len(psgl); } else pSRB-SGToBeXferLen = 0; @@ -842,7 +842,7 @@ dc390_DataIn_0( struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 *psstatus) DEBUG1(ResidCnt = ((unsigned long) DC390_read8 (CtcReg_High) 16) \ + ((unsigned long) DC390_read8 (CtcReg_Mid) 8) \ + ((unsigned long) DC390_read8 (CtcReg_Low))); - DEBUG1(printk (KERN_DEBUG Count_2_Zero (ResidCnt=%i,ToBeXfer=%li),, ResidCnt, pSRB-SGToBeXferLen)); + DEBUG1(printk (KERN_DEBUG Count_2_Zero (ResidCnt=%u,ToBeXfer=%lu),, ResidCnt, pSRB-SGToBeXferLen)); DC390_write8 (DMA_Cmd, READ_DIRECTION+DMA_IDLE_CMD); @@ -853,8 +853,8 @@ dc390_DataIn_0( struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 *psstatus) pSRB-pSegmentList++; psgl = pSRB-pSegmentList; - pSRB-SGBusAddr = cpu_to_le32(pci_dma_lo32(sg_dma_address(psgl))); - pSRB-SGToBeXferLen = cpu_to_le32(sg_dma_len(psgl)); + pSRB-SGBusAddr = sg_dma_address(psgl); + pSRB-SGToBeXferLen = sg_dma_len(psgl); } else pSRB-SGToBeXferLen = 0; @@ -921,11 +921,12 @@ din_1: ptr = (u8 *) bus_to_virt( pSRB-SGBusAddr ); *ptr = bval; - pSRB-SGBusAddr++; xferCnt++; + pSRB-SGBusAddr++; + xferCnt++; pSRB-TotalXferredLen++; pSRB-SGToBeXferLen--; } - DEBUG1(printk (KERN_DEBUG Xfered: %li, Total: %li, Remaining: %li\n, xferCnt,\ + DEBUG1(printk (KERN_DEBUG Xfered: %lu, Total: %lu, Remaining: %lu\n, xferCnt,\ pSRB-TotalXferredLen, pSRB-SGToBeXferLen)); } @@ -1157,14 +1158,14 @@ dc390_restore_ptr (struct dc390_acb* pACB, struct dc390_srb* pSRB) { pSRB-pSegmentList++; psgl = pSRB-pSegmentList; - pSRB-SGBusAddr = cpu_to_le32(pci_dma_lo32(sg_dma_address(psgl))); - pSRB-SGToBeXferLen = cpu_to_le32(sg_dma_len(psgl)); + pSRB-SGBusAddr = sg_dma_address(psgl); + pSRB-SGToBeXferLen = sg_dma_len(psgl); } else pSRB-SGToBeXferLen = 0; } - pSRB-SGToBeXferLen -= (pSRB-Saved_Ptr - pSRB-TotalXferredLen); - pSRB-SGBusAddr += (pSRB-Saved_Ptr - pSRB-TotalXferredLen); + pSRB-SGToBeXferLen -= pSRB-Saved_Ptr - pSRB-TotalXferredLen; + pSRB-SGBusAddr += pSRB-Saved_Ptr - pSRB-TotalXferredLen; printk (KERN_INFO DC390: Pointer restored. Segment %i, Total %li, Bus %08lx\n, pSRB-SGIndex, pSRB-Saved_Ptr, pSRB-SGBusAddr); @@ -1315,8 +1316,8 @@ dc390_DataIO_Comm( struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 ioDir) if( !pSRB-SGToBeXferLen ) { psgl = pSRB-pSegmentList; - pSRB-SGBusAddr = cpu_to_le32(pci_dma_lo32(sg_dma_address(psgl))); - pSRB-SGToBeXferLen = cpu_to_le32(sg_dma_len(psgl)); + pSRB-SGBusAddr = sg_dma_address(psgl); + pSRB-SGToBeXferLen = sg_dma_len(psgl); DEBUG1(printk (KERN_DEBUG DC390: Next SG segment.)); } lval = pSRB-SGToBeXferLen; diff --git a/drivers/scsi/tmscsim.h b/drivers/scsi/tmscsim.h index 9b66fa8..c3d8c80 100644 --- a/drivers/scsi/tmscsim.h +++ b/drivers/scsi/tmscsim.h @@ -19,14 +19,6 @@ #define SEL_TIMEOUT153 /* 250 ms selection timeout (@ 40 MHz) */ -#define pci_dma_lo32(a)(a 0x) - -typedef u8 UCHAR; /* 8 bits */ -typedef u16USHORT; /* 16 bits */ -typedef u32UINT; /* 32 bits */ -typedef unsigned long ULONG; /* 32/64 bits */ - - /* ;--- ; SCSI Request Block @@ -43,7 +35,9 @@ struct
[PATCH 3/3] tmscsim: Remove the last bus_to_virt()
Dynamically map the buffer for PIO for the residue byte. Signed-off-by: G. Liakhovetski [EMAIL PROTECTED] diff --git a/drivers/scsi/tmscsim.c b/drivers/scsi/tmscsim.c index be869d0..aec63ef 100644 --- a/drivers/scsi/tmscsim.c +++ b/drivers/scsi/tmscsim.c @@ -351,6 +351,27 @@ static u8 dc390_clock_speed[] = {100,80,67,57,50, 40, 31, 20}; * (DCBs, SRBs, Queueing) * **/ +static void inline dc390_start_segment(struct dc390_srb* pSRB) +{ + struct scatterlist *psgl = pSRB-pSegmentList; + + /* start new sg segment */ + pSRB-SGBusAddr = sg_dma_address(psgl); + pSRB-SGToBeXferLen = sg_dma_len(psgl); +} + +static unsigned long inline dc390_advance_segment(struct dc390_srb* pSRB, u32 residue) +{ + unsigned long xfer = pSRB-SGToBeXferLen - residue; + + /* xfer more bytes transferred */ + pSRB-SGBusAddr += xfer; + pSRB-TotalXferredLen += xfer; + pSRB-SGToBeXferLen = residue; + + return xfer; +} + static struct dc390_dcb __inline__ *dc390_findDCB ( struct dc390_acb* pACB, u8 id, u8 lun) { struct dc390_dcb* pDCB = pACB-pLinkDCB; if (!pDCB) return NULL; @@ -741,11 +762,10 @@ static irqreturn_t do_DC390_Interrupt(int irq, void *dev_id) } static void -dc390_DataOut_0( struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 *psstatus) +dc390_DataOut_0(struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 *psstatus) { u8 sstatus; -struct scatterlist *psgl; -u32ResidCnt, xferCnt; +u32 ResidCnt; u8 dstate = 0; sstatus = *psstatus; @@ -776,25 +796,20 @@ dc390_DataOut_0( struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 *psstatus) if( pSRB-SGIndex pSRB-SGcount ) { pSRB-pSegmentList++; - psgl = pSRB-pSegmentList; - pSRB-SGBusAddr = sg_dma_address(psgl); - pSRB-SGToBeXferLen = sg_dma_len(psgl); + dc390_start_segment(pSRB); } else pSRB-SGToBeXferLen = 0; } else { - ResidCnt = (u32) DC390_read8 (Current_Fifo) 0x1f; - ResidCnt |= (u32) DC390_read8 (CtcReg_High) 16; - ResidCnt |= (u32) DC390_read8 (CtcReg_Mid) 8; - ResidCnt += (u32) DC390_read8 (CtcReg_Low); - - xferCnt = pSRB-SGToBeXferLen - ResidCnt; - pSRB-SGBusAddr += xferCnt; - pSRB-TotalXferredLen += xferCnt; - pSRB-SGToBeXferLen = ResidCnt; + ResidCnt = ((u32) DC390_read8 (Current_Fifo) 0x1f) + + (((u32) DC390_read8 (CtcReg_High) 16) | +((u32) DC390_read8 (CtcReg_Mid) 8) | +(u32) DC390_read8 (CtcReg_Low)); + + dc390_advance_segment(pSRB, ResidCnt); } } if ((*psstatus 7) != SCSI_DATA_OUT) @@ -805,13 +820,11 @@ dc390_DataOut_0( struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 *psstatus) } static void -dc390_DataIn_0( struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 *psstatus) +dc390_DataIn_0(struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 *psstatus) { u8 sstatus, residual, bval; -struct scatterlist *psgl; -u32ResidCnt, i; +u32 ResidCnt, i; unsigned long xferCnt; -u8 *ptr; sstatus = *psstatus; @@ -851,10 +864,8 @@ dc390_DataIn_0( struct dc390_acb* pACB, struct dc390_srb* pSRB, u8 *psstatus) if( pSRB-SGIndex pSRB-SGcount ) { pSRB-pSegmentList++; - psgl = pSRB-pSegmentList; - pSRB-SGBusAddr = sg_dma_address(psgl); - pSRB-SGToBeXferLen = sg_dma_len(psgl); + dc390_start_segment(pSRB); } else pSRB-SGToBeXferLen = 0; @@ -894,41 +905,38 @@ din_1: /* It seems a DMA Blast abort isn't that bad ... */ if (!i) printk (KERN_ERR DC390: DMA Blast aborted unfinished!\n); //DC390_write8 (DMA_Cmd, READ_DIRECTION+DMA_IDLE_CMD); - dc390_laststatus = ~0xff00; dc390_laststatus |= bval 24; + dc390_laststatus = ~0xff00; + dc390_laststatus |= bval 24; DEBUG1(printk (KERN_DEBUG Blast: Read %i times DMA_Status %02x, 0xa000-i, bval)); - ResidCnt = (u32) DC390_read8 (CtcReg_High); - ResidCnt = 8; - ResidCnt |= (u32) DC390_read8 (CtcReg_Mid); - ResidCnt = 8; - ResidCnt |= (u32) DC390_read8 (CtcReg_Low); - - xferCnt = pSRB-SGToBeXferLen - ResidCnt; - pSRB-SGBusAddr += xferCnt; - pSRB-TotalXferredLen += xferCnt; - pSRB-SGToBeXferLen = ResidCnt; - - if( residual ) - { - static int feedback_requested; + ResidCnt = (((u32) DC390_read8 (CtcReg_High) 16) | + ((u32) DC390_read8 (CtcReg_Mid) 8)) | + (u32) DC390_read8 (CtcReg_Low); + +
Re: SAVE_DATA / RESTORE_POINTERS message confusion
On Sun, 29 Apr 2007, David Miller wrote: From: James Bottomley [EMAIL PROTECTED] Date: Sun, 29 Apr 2007 16:18:19 -0500 On Sun, 2007-04-29 at 22:56 +0200, Guennadi Liakhovetski wrote: 2. are they ever sent apart from when accompanying the DISCONNECT / RESELECT? I've never seen an occasion, no. I have. 3. if yes to 2 - can the RESTORE be used to rewind the pointer to a value smaller than the current? Theoretically, yes. In practice, no device I've ever heard of has done this. I've seen a device do this, and ironically for an inquiry :-) Ok, looking through t10 docs there seem to be quite a few functions for the SAVE DATA POINTERS / MODIFY DATA POINTER / RESTORE POINTERS (btw, even POINTER vs. POINTERS use was inconsistent between the spi5r06, s2-r10l, ana a random book I have) also apart from the usual DISCONNECT / RESELECT, like re-requesting command / data / status. Is there a SCSI driver that does all this pointer management correctly? And that's easy enough to understand and use as an example? For example, tmscsim just would reject MODIFY DP. And for every RESTORE POINTERS with sg there would be a KERN_INFO printk... Thanks Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
SAVE_DATA / RESTORE_POINTERS message confusion
Hi all, looking at tmscsim (again) I cannot persuade myself that the handling of the RESTORE_POINTERS command is correct... Therefore a couple of questions: 1. do any devices at all use this message-pair? 2. are they ever sent apart from when accompanying the DISCONNECT / RESELECT? 3. if yes to 2 - can the RESTORE be used to rewind the pointer to a value smaller than the current? 4. Some LLDs just pretty much ignore the message. Thereby, AFAIU, they just assume that this is just a part of the RESELECT procedure and that they already have the proper context. Is it sufficient? From which I'm trying to derive whether the handling in tmscsim is correct, and, if not, how to correct it... Thanks Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] make scsi_wait_scan always modular
Hi On Sun, 11 Mar 2007, James Bottomley wrote: Currently scsi_wait_scan is only built modular if SCSI is modular. However, it's perfectly possible for a built in SCSI still to have modular drivers and thus need scsi_wait_scan as a module. Therefore, scsi_wait_scan should always be built as a module (unless the kernel doesn't support modules). James diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig index 4cd280e..f3bc0f4 100644 --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -241,6 +241,12 @@ config SCSI_SCAN_ASYNC You can override this choice by specifying scsi_mod.scan=sync or async on the kernel's command line. +config SCSI_WAIT_SCAN + tristate + default m + depends on SCSI + depends on MODULES + menu SCSI Transports depends on SCSI diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile index 79ecf4e..41c7883 100644 --- a/drivers/scsi/Makefile +++ b/drivers/scsi/Makefile @@ -145,7 +145,7 @@ obj-$(CONFIG_CHR_DEV_SCH) += ch.o # This goes last, so that real scsi devices probe earlier obj-$(CONFIG_SCSI_DEBUG) += scsi_debug.o -obj-$(CONFIG_SCSI) += scsi_wait_scan.o +obj-$(CONFIG_SCSI_WAIT_SCAN) += scsi_wait_scan.o Which means, with this change you cannot build a kernel without module support but with scsi_wait_scan, which was possible before. Don't know if anybody would ever want to do this though... Thanks Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] make scsi_wait_scan always modular
On Sun, 11 Mar 2007, James Bottomley wrote: On Sun, 2007-03-11 at 21:56 +0100, Guennadi Liakhovetski wrote: Which means, with this change you cannot build a kernel without module support but with scsi_wait_scan, which was possible before. Don't know if anybody would ever want to do this though... I'll do something about that if you can tell me how one might use scsi_wait_scan without module support in the kernel ... I won't:-) Just from the comment at the top of the patch I didn't get the impression that this indeed was its intended result. Building statically with scsi_wait_scan would mean that you might allow async scan, but then some time at late_initcall() time you want a sync point. Could this ever be desired? If not, just ignore my comment. Thanks Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] make scsi_wait_scan always modular
On Sun, 11 Mar 2007, James Bottomley wrote: scsi_wait_scan is designed to be a module that is loaded *after* you've loaded all HBA modules that doesn't return from module_init until the scans previously launched by the HBA module insertions are complete. sure In a monolithic kernel, this is done by the async scan infrastructure but in a modular kernel, you have to have some signal that you've finished loading the SCSI modules (which is what scsi_wait_scan is). So, if now (before this patch) you build a monolithic kernel, scsi_wait_scan will be built in, scsi_complete_async_scans() will be called at late_initcall time, but it will be redundant because In a monolithic kernel, this is done by the async scan infrastructure so, it will just have no effect, right? If so, then, of course it only makes sense as a module. Thanks Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dc395x
On Thu, 15 Feb 2007, Aldo Barreto wrote: I'am trying to install Mandriva 2007 with kernel 2.6.17 in my system. It's a Pentium IV P4P 800 ASSUS system with 1 Gb of memory and a scsi board Tekram dc395. System doesn't end startup and syslog sends me a messagge: kernel: drivers/scsi/dc395x.c: Please, contact (your address). This occurs booting from a ide disk, when I tried to install all in a scsi disk system gives me a message :QUEUE_FULL. If it's possible I want to work only with scsi disk. With Mandrake 10.0 system works fine. Can you help me? Please, try a newer kernel, preferrably 2.6.20. I think, by now Mandriva should have something newer than 2.6.17? If it still fails, please, write again. Thanks Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: atomic_kmap for PIO (was Re: Fw: Kernel panic with dc395x in 2.6.12.2)
Hi all I've been buggering the list with this ridiculous problem for quite some time now:-) I understand, it is too risky to apply an untested patch to mainline, but could we either 1) get it into -mm or 2) try to find some beta-testers by applying something like the below: Thanks Guennadi --- Guennadi Liakhovetski Signed-off-by: Guennadi Liakhovetski [EMAIL PROTECTED] Index: linux-2_6/drivers/scsi/dc395x.c === RCS file: /usr/src/cvs/linux-2_6/drivers/scsi/dc395x.c,v retrieving revision 1.1.1.8 diff -u -p -r1.1.1.8 dc395x.c --- linux-2_6/drivers/scsi/dc395x.c 2 Sep 2005 19:16:29 - 1.1.1.8 +++ linux-2_6/drivers/scsi/dc395x.c 5 Sep 2005 20:38:23 - @@ -976,6 +976,17 @@ static void send_srb(struct AdapterCtlBl } } +static inline void *__page_address(struct page *page) +{ + static int feedback_requested; + + if (!feedback_requested) { + feedback_requested = 1; + printk(KERN_WARNING %s: Please, contact linux-scsi@vger.kernel.org + to help improve support for your system.\n, __FILE__); + } + return page_address(page); +} /* Prepare SRB for being sent to Device DCB w/ command *cmd */ static void build_srb(struct scsi_cmnd *cmd, struct DeviceCtlBlk *dcb, @@ -1021,7 +1032,7 @@ static void build_srb(struct scsi_cmnd * reqlen, cmd-request_buffer, cmd-use_sg, srb-sg_count); - srb-virt_addr = page_address(sl-page); + srb-virt_addr = __page_address(sl-page); for (i = 0; i srb-sg_count; i++) { u32 busaddr = (u32)sg_dma_address(sl[i]); u32 seglen = (u32)sl[i].length; @@ -2018,7 +2029,7 @@ static void sg_update_list(struct ScsiRe unsigned long mask = ~((unsigned long)sg-length - 1) PAGE_MASK; if ((sg_dma_address(sg) mask) == (psge-address mask)) { - srb-virt_addr = (page_address(sg-page) + srb-virt_addr = (__page_address(sg-page) + psge-address - (psge-address PAGE_MASK)); return; @@ -3322,7 +,7 @@ static void srb_done(struct AdapterCtlBl if (cmd-use_sg) { struct scatterlist* sg = (struct scatterlist *)cmd-request_buffer; - ptr = (struct ScsiInqData *)(page_address(sg-page) + sg-offset); + ptr = (struct ScsiInqData *)(__page_address(sg-page) + sg-offset); } else { ptr = (struct ScsiInqData *)(cmd-request_buffer); } Index: linux-2_6/drivers/scsi/tmscsim.c === RCS file: /usr/src/cvs/linux-2_6/drivers/scsi/tmscsim.c,v retrieving revision 1.1.1.9 diff -u -p -r1.1.1.9 tmscsim.c --- linux-2_6/drivers/scsi/tmscsim.c2 Sep 2005 19:16:29 - 1.1.1.9 +++ linux-2_6/drivers/scsi/tmscsim.c5 Sep 2005 20:38:31 - @@ -989,6 +989,14 @@ din_1: if( residual ) { bval = DC390_read8 (ScsiFifo); /* get one residual byte */ + static int feedback_requested; + + if (!feedback_requested) { + feedback_requested = 1; + printk(KERN_WARNING %s: Please, contact linux-scsi@vger.kernel.org + to help improve support for your system.\n, __FILE__); + } + ptr = (u8 *) bus_to_virt( pSRB-SGBusAddr ); *ptr = bval; pSRB-SGBusAddr++; xferCnt++; - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PATCH] scsi bug fixes for 2.6.13
On Sun, 14 Aug 2005, James Bottomley wrote: OK, why don't we do this. Instead of having me trawl through the trees looking for the correct patch to reverse, why don't you attach it in an email and I'll try to get it in to 2.6.13? Looks like just reverting that patch is not enough. More in about 12 hours. Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PATCH] scsi bug fixes for 2.6.13
On Mon, 15 Aug 2005, Guennadi Liakhovetski wrote: On Sun, 14 Aug 2005, James Bottomley wrote: OK, why don't we do this. Instead of having me trawl through the trees looking for the correct patch to reverse, why don't you attach it in an email and I'll try to get it in to 2.6.13? Looks like just reverting that patch is not enough. More in about 12 hours. Sorry, forget it, just tested here with (pseudo-)highmem on SMP . just reverting the patch does indeed fix the driver. Thanks Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
internal tape + external zip on TRM-S1040 (dc395x)
Hi It might be more an electrical than software question, sorry if somewhat OT. If I connect an external 100MB zip and an internal HP DDS-1 tape drive to the dc390 adapter, I can tar zip to the tape. If I connect them to the TRM-S1040 (dc315) it aborts with tar: /dev/st0: Wrote only 0 of 10240 bytes tar: Error is not recoverable: exiting now and in dmesg: dc395x: eh_abort: (pid#37555) target=06-0 cmd=c5c16b40 dc395x: eh_abort: Command in progress6dc395x: eh_abort: (pid#37556) target=06-0 cmd=c5f6ed00 dc395x: eh_abort: Command was waiting dc395x: eh_abort: (pid#37556) target=06-0 cmd=c5f6ed00 dc395x: eh_abort: Command was waiting dc395x: eh_abort: (pid#37557) target=06-0 cmd=c5c16040 dc395x: eh_abort: Command was waiting dc395x: eh_abort: (pid#37557) target=06-0 cmd=c5c16040 dc395x: eh_abort: Command was waiting dc395x: eh_bus_reset: (pid#37555) target=06-0 cmd=c5c16b40 dc395x: doing_srb_done: pids G:37555(06-0) dc395x: Target 05: Sync: 100ns Offset 15 (10.0 MB/s) st0: Error with sense data: 6st0: Current: sense key=0x6 ASC=0x29 ASCQ=0x0 Separately accessing the zip and the tape works on TRM-S1040 too. The problem appears also if I just access the zip and the tape simultaneously from 2 processes not transferring the data between them. On loading the driver it reports: dc395x: Tekram DC395(U/UW/F), DC315(U) - ASIC TRM-S1040 v2.05, 2004/03/08 dc395x: Used settings: AdapterID=07, Speed=0(20.0MHz), dev_mode=0x77 dc395x:AdaptMode=0x0f, Tags=4(16), DelayReset=1s dc395x: Connectors: int50 Termination: Auto Low High dc395x: Performing initial SCSI bus reset scsi3 : Tekram DC395(U/UW/F), DC315(U) - ASIC TRM-S1040 v2.05, 2004/03/08 dc395x: Target 05: Sync: 100ns Offset 15 (10.0 MB/s) Vendor: HPModel: C1533ARev: A708 Type: Sequential-Access ANSI SCSI revision: 02 Attached scsi tape st0 at scsi3, channel 0, id 5, lun 0 st0: try direct i/o: yes (alignment 512 B), max page reachable by HBA 1048575 Attached scsi generic sg2 at scsi3, channel 0, id 5, lun 0, type 1 Vendor: IOMEGAModel: ZIP 100 Rev: D.13 Type: Direct-Access ANSI SCSI revision: 02 SCSI device sdc: 196608 512-byte hdwr sectors (101 MB) sdc: Write Protect is off sdc: Mode Sense: 25 00 00 08 sdc: cache data unavailable sdc: assuming drive cache: write through SCSI device sdc: 196608 512-byte hdwr sectors (101 MB) sdc: Write Protect is off sdc: Mode Sense: 25 00 00 08 sdc: cache data unavailable sdc: assuming drive cache: write through sdc: sdc4 Attached scsi removable disk sdc at scsi3, channel 0, id 6, lun 0 Attached scsi generic sg3 at scsi3, channel 0, id 6, lun 0, type 0 Termination turned on on the zip, at the tape end there's a passive terminator. The cable to the tape is pretty bad, I had to replace it in another computer, but the tape IS SLOW. And it works on dc390. Just wanted to make sure it's not a software bug? Thanks Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PATCH] scsi bug fixes for 2.6.13
Hi On Thu, 4 Aug 2005, James Bottomley wrote: This is my (hopefully final) collection of safe driver updates and bug fixes for 2.6.13. Just to make sure everyone agrees on this - there's currently a know bug in dc395x with highmem reported by Pierre Ossman in thread Kernel panic with dc395x in 2.6.12.2 on linux-scsi. It is also trivial to reproduce on non-highmem machines. It was there also in 2.6.12. It only was hit by one person because not many use dc395x controlled cards in highmem machines today. The fix is known - at least revert my patch to dc395x commited to 2.6.12-rc5. This will fix this bug, leaving another one, which is much harder to hit. There was only one person who hit it, but he cannot test any more patches - the hardware is not there any more. I think I have a fix for that one too, but that's another story. So, I would say it would be worth it reverting that patch before 2.6.13, but, that's because I feel personal responsibility for that bug:-) Thanks Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PATCH] scsi bug fixes for 2.6.13
On Sun, 14 Aug 2005, James Bottomley wrote: On Sun, 2005-08-14 at 21:33 +0200, Guennadi Liakhovetski wrote: Just to make sure everyone agrees on this - there's currently a know bug in dc395x with highmem reported by Pierre Ossman in thread Kernel panic with dc395x in 2.6.12.2 on linux-scsi. It is also trivial to reproduce on non-highmem machines. It was there also in 2.6.12. It only was hit by one person because not many use dc395x controlled cards in highmem machines today. The fix is known - at least revert my patch to dc395x commited to 2.6.12-rc5. This will fix this bug, leaving another one, which is much harder to hit. There was only one person who hit it, but he cannot test any more patches - the hardware is not there any more. I think I have a fix for that one too, but that's another story. So, I would say it would be worth it reverting that patch before 2.6.13, but, that's because I feel personal responsibility for that bug:-) OK, why don't we do this. Instead of having me trawl through the trees looking for the correct patch to reverse, why don't you attach it in an email and I'll try to get it in to 2.6.13? Ok. Below. Sorry again for the trouble Guennadi --- Guennadi Liakhovetski Signed-off-by: Guennadi Liakhovetski [EMAIL PROTECTED] diff -u a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c --- a/drivers/scsi/dc395x.c 17 Nov 2004 21:04:51 +++ b/drivers/scsi/dc395x.c 22 Jan 2005 22:55:45 @@ -182,7 +182,7 @@ * cross a page boundy. */ #define SEGMENTX_LEN (sizeof(struct SGentry)*DC395x_MAX_SG_LISTENTRY) - +#define VIRTX_LEN (sizeof(void *) * DC395x_MAX_SG_LISTENTRY) struct SGentry { u32 address;/* bus! address */ @@ -234,6 +234,7 @@ u8 sg_count;/* No of HW sg entries for this request */ u8 sg_index;/* Index of HW sg entry for this request */ u32 total_xfer_length; /* Total number of bytes remaining to be transfered */ + void **virt_map; unsigned char *virt_addr; /* Virtual address of current transfer position */ /* @@ -1020,14 +1021,14 @@ reqlen, cmd-request_buffer, cmd-use_sg, srb-sg_count); - srb-virt_addr = page_address(sl-page); for (i = 0; i srb-sg_count; i++) { - u32 busaddr = (u32)sg_dma_address(sl[i]); - u32 seglen = (u32)sl[i].length; - sgp[i].address = busaddr; + u32 seglen = (u32)sg_dma_len(sl + i); + sgp[i].address = (u32)sg_dma_address(sl + i); sgp[i].length = seglen; srb-total_xfer_length += seglen; + srb-virt_map[i] = kmap(sl[i].page); } + srb-virt_addr = srb-virt_map[0]; sgp += srb-sg_count - 1; /* @@ -1964,6 +1965,7 @@ int segment = cmd-use_sg; u32 xferred = srb-total_xfer_length - left; /* bytes transfered */ struct SGentry *psge = srb-segment_x + srb-sg_index; + void **virt = srb-virt_map; dprintkdbg(DBG_0, sg_update_list: Transfered %i of %i bytes, %i remain\n, @@ -2003,16 +2005,16 @@ /* We have to walk the scatterlist to find it */ sg = (struct scatterlist *)cmd-request_buffer; + idx = 0; while (segment--) { unsigned long mask = ~((unsigned long)sg-length - 1) PAGE_MASK; if ((sg_dma_address(sg) mask) == (psge-address mask)) { - srb-virt_addr = (page_address(sg-page) - + psge-address - - (psge-address PAGE_MASK)); + srb-virt_addr = virt[idx] + (psge-address ~PAGE_MASK); return; } ++sg; + ++idx; } dprintkl(KERN_ERR, sg_update_list: sg_to_virt failed\n); @@ -2138,7 +2140,7 @@ DC395x_read32(acb, TRM_S1040_DMA_CXCNT)); } /* -* calculate all the residue data that not yet tranfered +* calculate all the residue data that not yet transfered * SCSI transfer counter + left in SCSI FIFO data * * .TRM_S1040_SCSI_COUNTER (24bits) @@ -3256,6 +3258,7 @@ struct scsi_cmnd *cmd = srb-cmd; enum dma_data_direction dir = cmd-sc_data_direction; if (cmd-use_sg dir != PCI_DMA_NONE) { + int i; /* unmap DC395x SG list */ dprintkdbg(DBG_SG, pci_unmap_srb: list=%08x(%05x)\n, srb-sg_bus_addr, SEGMENTX_LEN); @@ -3265,6 +3268,8 @@ dprintkdbg(DBG_SG, pci_unmap_srb: segs=%i buffer=%p\n
atomic_kmap for PIO (was Re: Fw: Kernel panic with dc395x in 2.6.12.2)
Hi all A problem has first been reported in Date: Thu, 6 Jan 2005 05:18:25 -0500 From: Andrew Schulman [EMAIL PROTECTED] Subject: dc395x: can't write to tape whereby a bug has become apparent in driver's virtual address calculation. I wrote a patch, which propagated into the mainline, before Jens A. (added to CC:) noticed, that it was buggy. Now Pierre actually hit that bug. Yesterday in http://marc.theaimsgroup.com/?l=linux-scsim=112336777508938w=2 I sent him and to the SCSI list a patch, which made his system again usable. Which doesn't mean, however, that the patch is correct. The patch uses the same 2 functions that I used earlier in a proposed in http://marc.theaimsgroup.com/?l=linux-scsim=111435313619444w=2 patch to tmscsim: static void *dc395x_kmap_atomic_sg(struct scatterlist *sg, int sg_count, size_t *offset, size_t *len) and static void dc395x_kunmap_atomic_sg(void *virt) to atomically map and unmap a byte sequence from an sg. This has also been discussed around April this year in a thread with subject: Re: [PATCH] dc395x: Fix support for highmem. That discussion didn't come to a definite conclusion. As well as my patch to tmscsim didn't produce any reaction, and, I think, wasn't included in any official tree. Now, it would be best, I think, if somebody could review the patches to dc395x and tmscsim. In case the 2 functions are correct, they could be included in a central file (scsi_lib?) for all drivers, needing PIO (I think IDE and libata won't be able to use them in this form). The rest of the 2 patches could then be applied to respective drivers. Thanks Guennadi On Sun, 7 Aug 2005, Pierre Ossman wrote: Guennadi Liakhovetski wrote: So, now that we have somebody to test alternative patches, let's try it... Please, first revert the first patch (apply with -R), and then apply the second one. There, probably, will be offsets, don't worry about them. Warning: untested. Please, try some read-only test first, if you get so far at all... I prepared this patch a long time ago, and didn't revisit it since then, hope, it's still fresh enough:-) Ok, I've tried the included patches now and everything seems to work fine. Note though that I only have a CD writer hooked up to the controller, no disks. Anything special I should try to make sure the patch does what it's supposed to? Rgds Pierre --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: atomic_kmap for PIO (was Re: Fw: Kernel panic with dc395x in 2.6.12.2)
On Sun, 7 Aug 2005, Mike Christie wrote: Guennadi Liakhovetski wrote: Now, it would be best, I think, if somebody could review the patches to dc395x and tmscsim. In case the 2 functions are correct, they could be included in a central file (scsi_lib?) for all drivers, needing PIO (I would it be possible to modify the scatterwalk code so we can share something with other subsystems? Do you have something specific in mind or you just mean drivers, that I mentioned: IDE and libata? If the latter - don't know. I seem to remember as somebody described their requirements in the April discussion, they seemed different enough to me. I am pretty sure IDE was mentioned as also doing PIO sometimes, not even sure if libata really was also discussed. If you mean something specific, like iscsi, - what exactly? Thanks Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fw: Kernel panic with dc395x in 2.6.12.2
Hi On Fri, 5 Aug 2005, Pierre Ossman wrote: Ok, I've now figured out where the bug appears. 2.6.12-rc4 runs fine while -rc5 does not. O... Ok, before you discover it yourself - the fault is mine, to be more precise, my patch, that somehow got applied... The long interesting discussion that followed the review by Jens Axboe, who revealed its bogus nature, didn't produce a replacement patch, mainly because there wasn't anybody with dc395x and highmem, until you came...:-) I just forgot / didn't realise it actually made its way into the mainline... So, now that we have somebody to test alternative patches, let's try it... Please, first revert the first patch (apply with -R), and then apply the second one. There, probably, will be offsets, don't worry about them. Warning: untested. Please, try some read-only test first, if you get so far at all... I prepared this patch a long time ago, and didn't revisit it since then, hope, it's still fresh enough:-) Including as attachments for easier separation. Good luck Guennadi --- Guennadi LiakhovetskiSigned-off-by: Guennadi Liakhovetski [EMAIL PROTECTED] diff -u a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c --- a/drivers/scsi/dc395x.c 17 Nov 2004 21:04:51 +++ b/drivers/scsi/dc395x.c 22 Jan 2005 22:55:45 @@ -182,7 +182,7 @@ * cross a page boundy. */ #define SEGMENTX_LEN (sizeof(struct SGentry)*DC395x_MAX_SG_LISTENTRY) - +#define VIRTX_LEN (sizeof(void *) * DC395x_MAX_SG_LISTENTRY) struct SGentry { u32 address;/* bus! address */ @@ -234,6 +234,7 @@ u8 sg_count;/* No of HW sg entries for this request */ u8 sg_index;/* Index of HW sg entry for this request */ u32 total_xfer_length; /* Total number of bytes remaining to be transfered */ + void **virt_map; unsigned char *virt_addr; /* Virtual address of current transfer position */ /* @@ -1020,14 +1021,14 @@ reqlen, cmd-request_buffer, cmd-use_sg, srb-sg_count); - srb-virt_addr = page_address(sl-page); for (i = 0; i srb-sg_count; i++) { - u32 busaddr = (u32)sg_dma_address(sl[i]); - u32 seglen = (u32)sl[i].length; - sgp[i].address = busaddr; + u32 seglen = (u32)sg_dma_len(sl + i); + sgp[i].address = (u32)sg_dma_address(sl + i); sgp[i].length = seglen; srb-total_xfer_length += seglen; + srb-virt_map[i] = kmap(sl[i].page); } + srb-virt_addr = srb-virt_map[0]; sgp += srb-sg_count - 1; /* @@ -1964,6 +1965,7 @@ int segment = cmd-use_sg; u32 xferred = srb-total_xfer_length - left; /* bytes transfered */ struct SGentry *psge = srb-segment_x + srb-sg_index; + void **virt = srb-virt_map; dprintkdbg(DBG_0, sg_update_list: Transfered %i of %i bytes, %i remain\n, @@ -2003,16 +2005,16 @@ /* We have to walk the scatterlist to find it */ sg = (struct scatterlist *)cmd-request_buffer; + idx = 0; while (segment--) { unsigned long mask = ~((unsigned long)sg-length - 1) PAGE_MASK; if ((sg_dma_address(sg) mask) == (psge-address mask)) { - srb-virt_addr = (page_address(sg-page) - + psge-address - - (psge-address PAGE_MASK)); + srb-virt_addr = virt[idx] + (psge-address ~PAGE_MASK); return; } ++sg; + ++idx; } dprintkl(KERN_ERR, sg_update_list: sg_to_virt failed\n); @@ -2138,7 +2140,7 @@ DC395x_read32(acb, TRM_S1040_DMA_CXCNT)); } /* -* calculate all the residue data that not yet tranfered +* calculate all the residue data that not yet transfered * SCSI transfer counter + left in SCSI FIFO data * * .TRM_S1040_SCSI_COUNTER (24bits) @@ -3256,6 +3258,7 @@ struct scsi_cmnd *cmd = srb-cmd; enum dma_data_direction dir = cmd-sc_data_direction; if (cmd-use_sg dir != PCI_DMA_NONE) { + int i; /* unmap DC395x SG list */ dprintkdbg(DBG_SG, pci_unmap_srb: list=%08x(%05x)\n, srb-sg_bus_addr, SEGMENTX_LEN); @@ -3265,6 +3268,8 @@ dprintkdbg(DBG_SG, pci_unmap_srb: segs=%i buffer=%p\n, cmd-use_sg, cmd-request_buffer); /* unmap the sg segments */ + for (i = 0; i srb-sg_count; i++) + kunmap
Re: Fw: Kernel panic with dc395x in 2.6.12.2
Hi On Sun, 10 Jul 2005, Pierre Ossman wrote: randy_dunlap wrote: Are you using netcat on the receive side? I've been using tcpdump and not a single packet eminates from the machine when printk:s show up. Have you managed to get netconsole running? Do you have no chance at all to use a serial console? You are, perhaps, the only / last user of dc395x on a high-mem machine, the driver is known to have a problem there, although from your dump I cannot see yet if that is indeed the problem. As someone also already suggested, you could also just try to reduce your terminal font size to fit the entire Oops on the monitor. I've tried to make a patche for the high-mem problem of dc395x, but we didn't have any testers. So, would be nice if you could help. Thanks Guennadi Matt Mackall says that a common problem is running kernels with a log low level: Kernel command line: ro root=LABEL=/ rhgb quiet acpi=ht so remove the quiet option there, or change it to debug. Tried with 'debug' and nothing at all. No effect. I also messed around with the 'printk' setting in /proc/sys/kernel. Netconsole simply doesn't like me. :( Perhaps I'm misunderstanding the scope of netconsole. Is it supposed to send out everything I can see with 'dmesg' or just panics? Rgds Pierre - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dc395x: Fix support for highmem
On Thu, 31 Mar 2005 [EMAIL PROTECTED] wrote: On Wed, 30 Mar 2005, James Bottomley wrote: On Wed, 2005-03-30 at 23:22 +0200, Guennadi Liakhovetski wrote: What is going to happen to this stuff? The current Linus' tree contains a broken (by me:-() dc395x, other drivers (including tmscsim) would benefit from a generic API. I understand, the drivers are not top importance, but still. I'd propose fixing the driver first and then using that fix as the basis for a generic API. Is the patch you sent on the 17th of March what you'd consider to be your final fix for this? Well, it could be. But I'd use Jens' suggestion and actually test it for highmem. And, since we are at it, I'd try to organise this code in separate API, so, that we just have to replace dc395x_* with scsi_* in function names. Makes sense? Ok, here's what I've come up with so far. Too bad - I cannot actually test it. Not only for high-, but also for lowmem. In my tests those PIO paths are not entered. More precisely - they are entered, e.g., when reading the disk's partition table, or doing sginfo -a, but only without scatter-gather... I also cannot test the wide part. I've only got a narrow (dc315, I think) card and (easy) I can only connect an external 100MB ZIP and an internal tape drives. Anybody has an idea when those paths are actually entered? Or anybody could test the patch? Another bad thing - diffstat: dc395x.c | 200 +-- 1 files changed, 132 insertions(+), 68 deletions(-) One of the reasons the number of +s is so large, is that on the one hand, not knowing the hardware and having no datasheet at hand, I tried not to modify the flow essentially, and on the other hand, I tried to make it rather generic, e.g., I am pretty sure, that those PIO bytes always belong to only one sg-segment, but I did it more generic, perhaps, an overkill. Anyway, please, comment. Not asking for inclusion - yet. Thanks Guennadi --- Guennadi Liakhovetski diff -u a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c --- a/drivers/scsi/dc395x.c 4 Mar 2005 22:42:35 +++ b/drivers/scsi/dc395x.c 9 Apr 2005 22:10:23 @@ -229,12 +229,8 @@ struct scsi_cmnd *cmd; struct SGentry *segment_x; /* Linear array of hw sg entries (up to 64 entries) */ - u32 sg_bus_addr;/* Bus address of sg list (ie, of segment_x) */ - - u8 sg_count;/* No of HW sg entries for this request */ - u8 sg_index;/* Index of HW sg entry for this request */ - u32 total_xfer_length; /* Total number of bytes remaining to be transfered */ - unsigned char *virt_addr; /* Virtual address of current transfer position */ + size_t total_xfer_length; /* Total number of bytes remaining to be transfered */ + size_t request_length; /* Total number of bytes in this request */ /* * The sense buffer handling function, request_sense, uses @@ -245,7 +241,12 @@ * total_xfer_length in xferred. These values are restored in * pci_unmap_srb_sense. This is the only place xferred is used. */ - u32 xferred;/* Saved copy of total_xfer_length */ + size_t xferred; /* Saved copy of total_xfer_length */ + + u32 sg_bus_addr;/* Bus address of sg list (ie, of segment_x) */ + + u8 sg_count;/* No of HW sg entries for this request */ + u8 sg_index;/* Index of HW sg entry for this request */ u16 state; @@ -989,7 +990,6 @@ srb-sg_count = 0; srb-total_xfer_length = 0; srb-sg_bus_addr = 0; - srb-virt_addr = NULL; srb-sg_index = 0; srb-adapter_status = 0; srb-target_status = 0; @@ -1020,7 +1020,6 @@ reqlen, cmd-request_buffer, cmd-use_sg, srb-sg_count); - srb-virt_addr = page_address(sl-page); for (i = 0; i srb-sg_count; i++) { u32 busaddr = (u32)sg_dma_address(sl[i]); u32 seglen = (u32)sl[i].length; @@ -1065,12 +1064,14 @@ srb-total_xfer_length++; srb-segment_x[0].length = srb-total_xfer_length; - srb-virt_addr = cmd-request_buffer; + dprintkdbg(DBG_0, build_srb: [1] len=%d buf=%p use_sg=%d map=%08x\n, srb-total_xfer_length, cmd-request_buffer, cmd-use_sg, srb-segment_x[0].address); } + + srb-request_length = srb-total_xfer_length; } @@ -1954,14 +1955,12 @@ /* * Compute the next Scatter Gather list index and adjust its length - * and address if necessary; also compute virt_addr + * and address if necessary */ static void sg_update_list(struct ScsiReqBlk
Re: [PATCH] dc395x: Fix support for highmem
On Sun, 20 Mar 2005, Christoph Hellwig wrote: On Wed, Mar 16, 2005 at 06:04:17PM +0100, Jens Axboe wrote: On Wed, Mar 16 2005, Christoph Hellwig wrote: On Wed, Mar 16, 2005 at 05:53:39PM +0100, Jens Axboe wrote: The list doesn't really need dma mapping at that point, the problem here is that the driver needs to punt to pio mode because of foo. So calling pci/dma_map_* is pointless, since the CPU will have to do the transfer anyways. What the driver is really looking for at this point, is a way to map the pages in the sglist to a virtual address. Given that there's quite a few cases of this problem it would be nice to have common helpers for it. Especially as it's really difficult when we allow merging of sg list entries I thought about that when writing the above, but is there really more than one case for SCSI drivers? If there is, sure lets add the helpers. But I would consider it a quite rare occurence, I've never seen it before. There's lots of pio only drivers, aswell as raid drivers that need to look into the non I/O-path command and things like iscsi. Well, how about something like char *kmap_atomic_sg(struct scatterlist *sg, unsigned int offset, int *mapped); void kunmap_atomic_sg(struct scatterlist *sg, int mapped); The latter would just call the kunmap_atomic with the respective KM_ type. By merging of sg list entries above is meant, that pci_map_sg may return a number smaller than the number of elements in the original sg list because some adjacent elements were merged during the mapping? Thanks Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dc395x: Fix support for highmem
Hi Here's a fixed patch - thanks to everybody for reviewing and commenting! However, if it is decided to implement helper functions, as suggested by Christoph and others, feel free to drop it. There, probably, was only 1 user of this card with highmem and he retired his card too:-) Thanks Guennadi --- Guennadi Liakhovetski Signed-off-by: Guennadi Liakhovetski [EMAIL PROTECTED] diff -u a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c --- a/drivers/scsi/dc395x.c 17 Nov 2004 21:04:51 +++ b/drivers/scsi/dc395x.c 17 Mar 2005 20:18:14 @@ -234,7 +234,8 @@ u8 sg_count;/* No of HW sg entries for this request */ u8 sg_index;/* Index of HW sg entry for this request */ u32 total_xfer_length; /* Total number of bytes remaining to be transfered */ - unsigned char *virt_addr; /* Virtual address of current transfer position */ + struct page *page; + unsigned int offset; /* * The sense buffer handling function, request_sense, uses @@ -989,7 +990,8 @@ srb-sg_count = 0; srb-total_xfer_length = 0; srb-sg_bus_addr = 0; - srb-virt_addr = NULL; + srb-page = NULL; + srb-offset = 0; srb-sg_index = 0; srb-adapter_status = 0; srb-target_status = 0; @@ -1020,7 +1022,8 @@ reqlen, cmd-request_buffer, cmd-use_sg, srb-sg_count); - srb-virt_addr = page_address(sl-page); + srb-page = sl-page; + srb-offset = sl-offset; for (i = 0; i srb-sg_count; i++) { u32 busaddr = (u32)sg_dma_address(sl[i]); u32 seglen = (u32)sl[i].length; @@ -1065,7 +1068,8 @@ srb-total_xfer_length++; srb-segment_x[0].length = srb-total_xfer_length; - srb-virt_addr = cmd-request_buffer; + srb-page = virt_to_page(cmd-request_buffer); + srb-offset = (unsigned long)cmd-request_buffer ~PAGE_MASK; dprintkdbg(DBG_0, build_srb: [1] len=%d buf=%p use_sg=%d map=%08x\n, srb-total_xfer_length, cmd-request_buffer, @@ -1984,8 +1988,7 @@ psge-length -= xferred; psge-address += xferred; srb-sg_index = idx; - pci_dma_sync_single_for_device(srb-dcb- - acb-dev, + pci_dma_sync_single_for_device(srb-dcb-acb-dev, srb-sg_bus_addr, SEGMENTX_LEN, PCI_DMA_TODEVICE); @@ -1995,28 +1998,20 @@ } sg_verify_length(srb); - /* we need the corresponding virtual address */ - if (!segment) { - srb-virt_addr += xferred; - return; - } - /* We have to walk the scatterlist to find it */ sg = (struct scatterlist *)cmd-request_buffer; while (segment--) { unsigned long mask = ~((unsigned long)sg-length - 1) PAGE_MASK; if ((sg_dma_address(sg) mask) == (psge-address mask)) { - srb-virt_addr = (page_address(sg-page) - + psge-address - - (psge-address PAGE_MASK)); + srb-page = sg-page; + srb-offset = psge-address ~PAGE_MASK; return; } ++sg; } dprintkl(KERN_ERR, sg_update_list: sg_to_virt failed\n); - srb-virt_addr = NULL; } @@ -2138,7 +2133,7 @@ DC395x_read32(acb, TRM_S1040_DMA_CXCNT)); } /* -* calculate all the residue data that not yet tranfered +* calculate all the residue data that not yet transfered * SCSI transfer counter + left in SCSI FIFO data * * .TRM_S1040_SCSI_COUNTER (24bits) @@ -2184,15 +2179,10 @@ (dcb-sync_period WIDE_SYNC) ? 2 : 1; sg_update_list(srb, d_left_counter); /* KG: Most ugly hack! Apparently, this works around a chip bug */ - if ((srb-segment_x[srb-sg_index].length == -diff srb-cmd-use_sg) - || ((oldxferred ~PAGE_MASK) == - (PAGE_SIZE - diff)) - ) { - dprintkl(KERN_INFO, data_out_phase0: - Work around chip bug (%i)?\n, diff); - d_left_counter = - srb
Re: [PATCH] dc395x: Fix support for highmem
Hi Thanks for reviewing this patch! On Wed, 16 Mar 2005, Jens Axboe wrote: On Wed, Mar 16 2005, James Bottomley wrote: ... I agree the kmap is inefficient. The efficient alternative is to do dma_map_sg() and use kmap_atomic() in the interrupt routine where we do the PIO cleanup---I'm afraid I just passed on explaining how to do this ... unless you care to do the honours ? In fact, the first version of my patch (attached below) did exactly this - only when the driver recognises, that it needs to do PIO in the interrupt, I would call kmap_atomic(), do PIO, then kunmap_atomic(). The main reason, why I didn't submit that patch, was that I got confused about various KM_ macros, and I thought, since it is a valuable limited resource, only very special drivers are allowed to use them / are allocated one of them. But, I guess now, you can just do local_irq_save(flags); kmap_atomic(); ... kunmap_atomic(); local_irq_restore(flags); Please, have a look. Or should we indeed go the generic helper functions way? The kmap() isn't just inefficient, it's a problem to iterate over the sg list and kmap all the pages. That is illegal. Hm, what do you mean illegal? Could you explain why? But it's not so tricky to get right, if the punting just happens in the isr. Basically just iterate over every sg entry left ala: for (i = start; i sg_entries; i++) { unsigned long flags; char *ptr; local_irq_save(flags); ptr = kmap_atomic(sg-page, KM_BIO_SRC_IRQ); /* transfer to/from ptr + sg-offset, sg-length bytes */ kunmap_atomic(ptr, KM_BIO_SRC_IRQ); local_irq_restore(flags); } I _think_ the sg-length field is universally called so, you should not use sg-dma_length/sg_dma_len() or sg_dma_address(), as we are outside the work of the iommu at this point. One more fragment in the driver I wasn't sure about is this: unsigned long mask = ~((unsigned long)sg-length - 1) PAGE_MASK; if ((sg_dma_address(sg) mask) == (psge-address mask)) { Is sg-length guaranteed to be something like a power of 2 or smaller than page? I thought about replacing the above with + if (sg_dma_address(sg) = psge-address sg_dma_address(sg) + psge-length psge-address) { but wasn't sure. Thanks Guennadi --- Guennadi Liakhovetski Index: drivers/scsi/dc395x.c === RCS file: /usr/src/cvs/linux-2_6/drivers/scsi/dc395x.c,v retrieving revision 1.1.1.4 diff -u -r1.1.1.4 dc395x.c --- drivers/scsi/dc395x.c 17 Nov 2004 21:04:51 - 1.1.1.4 +++ drivers/scsi/dc395x.c 16 Mar 2005 20:12:07 - @@ -185,7 +185,7 @@ struct SGentry { - u32 address;/* bus! address */ + dma_addr_t address; /* bus! address */ u32 length; }; @@ -234,7 +234,8 @@ u8 sg_count;/* No of HW sg entries for this request */ u8 sg_index;/* Index of HW sg entry for this request */ u32 total_xfer_length; /* Total number of bytes remaining to be transfered */ - unsigned char *virt_addr; /* Virtual address of current transfer position */ + struct page *page; + unsigned int offset; /* * The sense buffer handling function, request_sense, uses @@ -989,7 +990,8 @@ srb-sg_count = 0; srb-total_xfer_length = 0; srb-sg_bus_addr = 0; - srb-virt_addr = NULL; + srb-page = NULL; + srb-offset = 0; srb-sg_index = 0; srb-adapter_status = 0; srb-target_status = 0; @@ -1020,11 +1022,11 @@ reqlen, cmd-request_buffer, cmd-use_sg, srb-sg_count); - srb-virt_addr = page_address(sl-page); + srb-page = sl-page; + srb-offset = sl-offset; for (i = 0; i srb-sg_count; i++) { - u32 busaddr = (u32)sg_dma_address(sl[i]); - u32 seglen = (u32)sl[i].length; - sgp[i].address = busaddr; + u32 seglen = (u32)sg_dma_len(sl + i); + sgp[i].address = cpu_to_le32(0x sg_dma_address(sl +i)); sgp[i].length = seglen; srb-total_xfer_length += seglen; } @@ -1065,7 +1067,8 @@ srb-total_xfer_length++; srb-segment_x[0].length = srb-total_xfer_length; - srb-virt_addr = cmd-request_buffer; + srb-page = virt_to_page(cmd-request_buffer); + srb-offset = (unsigned long)cmd-request_buffer ~PAGE_MASK; dprintkdbg(DBG_0, build_srb: [1] len=%d buf=%p use_sg=%d map
Re: [PATCH] dc395x fix memory mapping (was Re: dc395x: can't write to tape)
On Sun, 6 Feb 2005, Guennadi Liakhovetski wrote: Two weeks since I posted this patch - ping... ...and another two weeks since you, Jamie, replied to me privately. Just wanted to say, it would be good to get this bug fixed for the post-2.6.11 SCSI patch. Regards Guennadi On Sun, 23 Jan 2005, Guennadi Liakhovetski wrote: On Sun, 9 Jan 2005, Jamie Lenehan wrote: On Sun, Jan 09, 2005 at 12:42:26PM +0100, Guennadi Liakhovetski wrote: [...] I could try to improve the highmem situation / sg-handling. Or do you plan to do it, Jamie? It's on my list of things to do, but I doubt I'll have any time to do anything about in the next few months. So if you have the time and desire please go ahead! I'll can test any changes you make (with a CD-R/W, tape-drive and HDD), although it may take me a week or two to get around to it. Ok, less than 2 weeks and here comes the first attempt. It removes page_to_virt and maps sg lists dynamically. Please, review, test. Slightly tested here - without highmem. Thanks Guennadi --- Guennadi Liakhovetski Signed-off-by: Guennadi Liakhovetski [EMAIL PROTECTED] diff -u a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c --- a/drivers/scsi/dc395x.c 17 Nov 2004 21:04:51 +++ b/drivers/scsi/dc395x.c 22 Jan 2005 22:55:45 @@ -182,7 +182,7 @@ * cross a page boundy. */ #define SEGMENTX_LEN (sizeof(struct SGentry)*DC395x_MAX_SG_LISTENTRY) - +#define VIRTX_LEN (sizeof(void *) * DC395x_MAX_SG_LISTENTRY) struct SGentry { u32 address;/* bus! address */ @@ -234,6 +234,7 @@ u8 sg_count;/* No of HW sg entries for this request */ u8 sg_index;/* Index of HW sg entry for this request */ u32 total_xfer_length; /* Total number of bytes remaining to be transfered */ + void **virt_map; unsigned char *virt_addr; /* Virtual address of current transfer position */ /* @@ -1020,14 +1021,14 @@ reqlen, cmd-request_buffer, cmd-use_sg, srb-sg_count); - srb-virt_addr = page_address(sl-page); for (i = 0; i srb-sg_count; i++) { - u32 busaddr = (u32)sg_dma_address(sl[i]); - u32 seglen = (u32)sl[i].length; - sgp[i].address = busaddr; + u32 seglen = (u32)sg_dma_len(sl + i); + sgp[i].address = (u32)sg_dma_address(sl + i); sgp[i].length = seglen; srb-total_xfer_length += seglen; + srb-virt_map[i] = kmap(sl[i].page); } + srb-virt_addr = srb-virt_map[0]; sgp += srb-sg_count - 1; /* @@ -1964,6 +1965,7 @@ int segment = cmd-use_sg; u32 xferred = srb-total_xfer_length - left; /* bytes transfered */ struct SGentry *psge = srb-segment_x + srb-sg_index; + void **virt = srb-virt_map; dprintkdbg(DBG_0, sg_update_list: Transfered %i of %i bytes, %i remain\n, @@ -2003,16 +2005,16 @@ /* We have to walk the scatterlist to find it */ sg = (struct scatterlist *)cmd-request_buffer; + idx = 0; while (segment--) { unsigned long mask = ~((unsigned long)sg-length - 1) PAGE_MASK; if ((sg_dma_address(sg) mask) == (psge-address mask)) { - srb-virt_addr = (page_address(sg-page) - + psge-address - - (psge-address PAGE_MASK)); + srb-virt_addr = virt[idx] + (psge-address ~PAGE_MASK); return; } ++sg; + ++idx; } dprintkl(KERN_ERR, sg_update_list: sg_to_virt failed\n); @@ -2138,7 +2140,7 @@ DC395x_read32(acb, TRM_S1040_DMA_CXCNT)); } /* -* calculate all the residue data that not yet tranfered +* calculate all the residue data that not yet transfered * SCSI transfer counter + left in SCSI FIFO data * * .TRM_S1040_SCSI_COUNTER (24bits) @@ -3256,6 +3258,7 @@ struct scsi_cmnd *cmd = srb-cmd; enum dma_data_direction dir = cmd-sc_data_direction; if (cmd-use_sg dir != PCI_DMA_NONE) { + int i; /* unmap DC395x SG list */ dprintkdbg(DBG_SG, pci_unmap_srb: list=%08x(%05x)\n, srb-sg_bus_addr, SEGMENTX_LEN); @@ -3265,6 +3268,8 @@ dprintkdbg(DBG_SG, pci_unmap_srb: segs=%i buffer=%p\n, cmd-use_sg, cmd-request_buffer); /* unmap the sg segments */ + for (i = 0; i srb-sg_count; i++) + kunmap(virt_to_page(srb-virt_map[i
dc395x patch: failure notice (fwd)
Jamie, mail to you bounced: -- Forwarded message -- Date: 21 Feb 2005 22:28:46 - From: [EMAIL PROTECTED] To: [EMAIL PROTECTED] Subject: failure notice Hi. This is the qmail-send program at mail.gmx.net. I'm afraid I wasn't able to deliver your message to the following addresses. This is a permanent error; I've given up. Sorry it didn't work out. [EMAIL PROTECTED]: 202.173.155.197_does_not_like_recipient./Remote_host_said:_554_pop.gmx.de[213.165.64.20]:_Client_host_rejected:_See_http://twibble.org/Policy/excessivespam/Giving_up_on_202.173.155.197./ --- Below this line is a copy of the message. Return-Path: [EMAIL PROTECTED] Received: (qmail invoked by alias); 21 Feb 2005 22:28:39 - Received: from dialin-212-144-033-049.arcor-ip.net (EHLO poirot.grange) (212.144.33.49) by mail.gmx.net (mp014) with SMTP; 21 Feb 2005 23:28:39 +0100 X-Authenticated: #20450766 Received: from lyakh (helo=localhost) by poirot.grange with local-esmtp (Exim 3.35 #1 (Debian)) id 1D3LcN-00021y-00; Mon, 21 Feb 2005 23:01:27 +0100 Date: Mon, 21 Feb 2005 23:01:27 +0100 (CET) From: Guennadi Liakhovetski [EMAIL PROTECTED] To: Jamie Lenehan [EMAIL PROTECTED] cc: linux-scsi@vger.kernel.org Subject: Re: [PATCH] dc395x fix memory mapping (was Re: dc395x: can't write to tape) In-Reply-To: [EMAIL PROTECTED] Message-ID: [EMAIL PROTECTED] References: [EMAIL PROTECTED] [EMAIL PROTECTED] [EMAIL PROTECTED] [EMAIL PROTECTED] [EMAIL PROTECTED] [EMAIL PROTECTED] [EMAIL PROTECTED] MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Y-GMX-Trusted: 0 X-GMX-Antivirus: 0 (no virus found) Regards Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dc395x: can't write to tape
On Wed, 26 Jan 2005, Andrew Schulman wrote: I could try to improve the highmem situation / sg-handling. Or do you plan to do it, Jamie? It's on my list of things to do, but I doubt I'll have any time to do anything about in the next few months. So if you have the time and desire please go ahead! I'll can test any changes you make (with a CD-R/W, tape-drive and HDD), although it may take me a week or two to get around to it. Ok, less than 2 weeks and here comes the first attempt. It removes page_to_virt and maps sg lists dynamically. Please, review, test. Slightly tested here - without highmem. Guennadi and Jamie, thanks for your help on this. However, I'm afraid I have to bail out at this point. My tape drive was the only reason I was keeping my old DC395UW controller going, and it has now gone bad. So I've had to yank both the card and tape drive and install a different backup solution. So, I can't test any patches. Thanks for your help. Guennadi --- Guennadi Liakhovetski - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dc395x: can't write to tape
On Sun, 9 Jan 2005, Jamie Lenehan wrote: On Sun, Jan 09, 2005 at 12:42:26PM +0100, Guennadi Liakhovetski wrote: [...] I could try to improve the highmem situation / sg-handling. Or do you plan to do it, Jamie? It's on my list of things to do, but I doubt I'll have any time to do anything about in the next few months. So if you have the time and desire please go ahead! I'll can test any changes you make (with a CD-R/W, tape-drive and HDD), although it may take me a week or two to get around to it. Ok, less than 2 weeks and here comes the first attempt. It removes page_to_virt and maps sg lists dynamically. Please, review, test. Slightly tested here - without highmem. Thanks Guennadi --- Guennadi Liakhovetski Signed-off-by: Guennadi Liakhovetski [EMAIL PROTECTED] diff -u a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c --- a/drivers/scsi/dc395x.c 17 Nov 2004 21:04:51 +++ b/drivers/scsi/dc395x.c 22 Jan 2005 22:55:45 @@ -182,7 +182,7 @@ * cross a page boundy. */ #define SEGMENTX_LEN (sizeof(struct SGentry)*DC395x_MAX_SG_LISTENTRY) - +#define VIRTX_LEN (sizeof(void *) * DC395x_MAX_SG_LISTENTRY) struct SGentry { u32 address;/* bus! address */ @@ -234,6 +234,7 @@ u8 sg_count;/* No of HW sg entries for this request */ u8 sg_index;/* Index of HW sg entry for this request */ u32 total_xfer_length; /* Total number of bytes remaining to be transfered */ + void **virt_map; unsigned char *virt_addr; /* Virtual address of current transfer position */ /* @@ -1020,14 +1021,14 @@ reqlen, cmd-request_buffer, cmd-use_sg, srb-sg_count); - srb-virt_addr = page_address(sl-page); for (i = 0; i srb-sg_count; i++) { - u32 busaddr = (u32)sg_dma_address(sl[i]); - u32 seglen = (u32)sl[i].length; - sgp[i].address = busaddr; + u32 seglen = (u32)sg_dma_len(sl + i); + sgp[i].address = (u32)sg_dma_address(sl + i); sgp[i].length = seglen; srb-total_xfer_length += seglen; + srb-virt_map[i] = kmap(sl[i].page); } + srb-virt_addr = srb-virt_map[0]; sgp += srb-sg_count - 1; /* @@ -1964,6 +1965,7 @@ int segment = cmd-use_sg; u32 xferred = srb-total_xfer_length - left; /* bytes transfered */ struct SGentry *psge = srb-segment_x + srb-sg_index; + void **virt = srb-virt_map; dprintkdbg(DBG_0, sg_update_list: Transfered %i of %i bytes, %i remain\n, @@ -2003,16 +2005,16 @@ /* We have to walk the scatterlist to find it */ sg = (struct scatterlist *)cmd-request_buffer; + idx = 0; while (segment--) { unsigned long mask = ~((unsigned long)sg-length - 1) PAGE_MASK; if ((sg_dma_address(sg) mask) == (psge-address mask)) { - srb-virt_addr = (page_address(sg-page) - + psge-address - - (psge-address PAGE_MASK)); + srb-virt_addr = virt[idx] + (psge-address ~PAGE_MASK); return; } ++sg; + ++idx; } dprintkl(KERN_ERR, sg_update_list: sg_to_virt failed\n); @@ -2138,7 +2140,7 @@ DC395x_read32(acb, TRM_S1040_DMA_CXCNT)); } /* -* calculate all the residue data that not yet tranfered +* calculate all the residue data that not yet transfered * SCSI transfer counter + left in SCSI FIFO data * * .TRM_S1040_SCSI_COUNTER (24bits) @@ -3256,6 +3258,7 @@ struct scsi_cmnd *cmd = srb-cmd; enum dma_data_direction dir = cmd-sc_data_direction; if (cmd-use_sg dir != PCI_DMA_NONE) { + int i; /* unmap DC395x SG list */ dprintkdbg(DBG_SG, pci_unmap_srb: list=%08x(%05x)\n, srb-sg_bus_addr, SEGMENTX_LEN); @@ -3265,6 +3268,8 @@ dprintkdbg(DBG_SG, pci_unmap_srb: segs=%i buffer=%p\n, cmd-use_sg, cmd-request_buffer); /* unmap the sg segments */ + for (i = 0; i srb-sg_count; i++) + kunmap(virt_to_page(srb-virt_map[i])); pci_unmap_sg(acb-dev, (struct scatterlist *)cmd-request_buffer, cmd-use_sg, dir); @@ -3311,7 +3316,7 @@ if (cmd-use_sg) { struct scatterlist* sg = (struct scatterlist *)cmd-request_buffer