Advansys driver
Subsystems (2)
Since the AdvanSys driver hasn't had a maintainer in 6 years, I don't think anybody's going to seriously object to me taking this on.
I present a git tree of patches:
http://git.kernel.org/?p=linux/kernel/git/willy/advansys.git;a=summary
and the broken-out version of those patches:
http://www.kernel.org/pub/linux/kernel/people/willy/advansys-2007-07-26/
There's no way I'm going to send the Lindenting of that driver to the list; it's 1.1MB. Here's the git-log entry for it:
advansys: Large, uninteresting changes
- Run Lindent
- Move advansys_detect and advansys_release to the end of the file
- Split advansys_board_found out of advansys_detect
- Rename a few variables, such as shp to shost and pci_devp to pdev
- Turn STATIC into static
I'll send the other 21 patches as a reply to this mail.
I'd like to particularly thank Ken Witherow for testing many different versions of these patches. He was indefatigable in the presence of the daft changes I was making when I didn't have a card to test it on. I would thank Dave Jones for sending me his card, but I think he did it to get revenge on me for something :-)
If anyone has an EISA version of this card they want to donate to me, I can give it a good home in an Alpha.
Signature · 3 lines
"Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
PATCH 1/21 advansys: Clean up proc_info implementation
Just use the Scsi_Host passed in, rather than looking through the driver's own array of boards for one that matches it.
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 38 +++++++++++++------------------------- 1 files changed, 13 insertions(+), 25 deletions(-)
drivers/scsi/advansys.c +13 -25
@@ -4036,9 +4036,7 @@ static int advansys_proc_info(struct Scsi_Host *shost, char *buffer, char **start, off_t offset, int length, int inout) { - struct Scsi_Host *shp; asc_board_t *boardp; -int i;char *cp;int cplen;int cnt;
@@ -4063,18 +4061,7 @@ advansys_proc_info(struct Scsi_Host *shost, char *buffer, char **start,* User read of /proc/scsi/advansys/[0...] file. */ -/* Find the specified board. */ -for (i =0 ; i < asc_board_count; i++) { -if (asc_host[i]->host_no == shost->host_no) { -break ; - } - } -if (i == asc_board_count) { -return (-ENOENT ); - } - - shp = asc_host[i]; - boardp =ASC_BOARDP (shp); + boardp =ASC_BOARDP (shost);/* Copy read data starting at the beginning of the buffer. */ *start = buffer;
@@ -4088,7 +4075,7 @@ advansys_proc_info(struct Scsi_Host *shost, char *buffer, char **start,* * advansys_info() returns the board string from its own static buffer. */ - cp = (char *)advansys_info (shp); + cp = (char *)advansys_info (shost);strcat (cp,"\n" ); cplen =strlen (cp);/* Copy board information. */
@@ -4107,7 +4094,7 @@ advansys_proc_info(struct Scsi_Host *shost, char *buffer, char **start,*/ if (ASC_WIDE_BOARD (boardp)) { cp = boardp->prtbuf; - cplen =asc_prt_adv_bios (shp, cp,ASC_PRTBUF_SIZE ); + cplen =asc_prt_adv_bios (shost, cp,ASC_PRTBUF_SIZE );ASC_ASSERT (cplen <ASC_PRTBUF_SIZE ); cnt = asc_proc_copy(advoffset, offset, curbuf, leftlen, cp,
@@ -4126,7 +4113,7 @@ advansys_proc_info(struct Scsi_Host *shost, char *buffer, char **start,* Display driver information for each device attached to the board. */ cp = boardp->prtbuf; - cplen =asc_prt_board_devices (shp, cp,ASC_PRTBUF_SIZE ); + cplen =asc_prt_board_devices (shost, cp,ASC_PRTBUF_SIZE );ASC_ASSERT (cplen <ASC_PRTBUF_SIZE ); cnt =asc_proc_copy (advoffset, offset, curbuf, leftlen, cp, cplen); totcnt += cnt;
@@ -4143,9 +4130,9 @@ advansys_proc_info(struct Scsi_Host *shost, char *buffer, char **start,*/ cp = boardp->prtbuf;if (ASC_NARROW_BOARD (boardp)) { - cplen =asc_prt_asc_board_eeprom (shp, cp,ASC_PRTBUF_SIZE ); + cplen =asc_prt_asc_board_eeprom (shost, cp,ASC_PRTBUF_SIZE ); }else { - cplen =asc_prt_adv_board_eeprom (shp, cp,ASC_PRTBUF_SIZE ); + cplen =asc_prt_adv_board_eeprom (shost, cp,ASC_PRTBUF_SIZE ); }ASC_ASSERT (cplen <ASC_PRTBUF_SIZE ); cnt =asc_proc_copy (advoffset, offset, curbuf, leftlen, cp, cplen);
@@ -4162,7 +4149,7 @@ advansys_proc_info(struct Scsi_Host *shost, char *buffer, char **start,* Display driver configuration and information for the board. */ cp = boardp->prtbuf; - cplen =asc_prt_driver_conf (shp, cp,ASC_PRTBUF_SIZE ); + cplen =asc_prt_driver_conf (shost, cp,ASC_PRTBUF_SIZE );ASC_ASSERT (cplen <ASC_PRTBUF_SIZE ); cnt =asc_proc_copy (advoffset, offset, curbuf, leftlen, cp, cplen); totcnt += cnt;
@@ -4179,7 +4166,7 @@ advansys_proc_info(struct Scsi_Host *shost, char *buffer, char **start,* Display driver statistics for the board. */ cp = boardp->prtbuf; - cplen =asc_prt_board_stats (shp, cp,ASC_PRTBUF_SIZE ); + cplen =asc_prt_board_stats (shost, cp,ASC_PRTBUF_SIZE );ASC_ASSERT (cplen <=ASC_PRTBUF_SIZE ); cnt =asc_proc_copy (advoffset, offset, curbuf, leftlen, cp, cplen); totcnt += cnt;
@@ -4196,7 +4183,8 @@ advansys_proc_info(struct Scsi_Host *shost, char *buffer, char **start,*/ for (tgt_id =0 ; tgt_id <=ADV_MAX_TID ; tgt_id++) { cp = boardp->prtbuf; - cplen =asc_prt_target_stats (shp, tgt_id, cp,ASC_PRTBUF_SIZE ); + cplen =asc_prt_target_stats (shost, tgt_id, cp, +ASC_PRTBUF_SIZE );ASC_ASSERT (cplen <=ASC_PRTBUF_SIZE ); cnt = asc_proc_copy(advoffset, offset, curbuf, leftlen, cp,
@@ -4218,9 +4206,9 @@ advansys_proc_info(struct Scsi_Host *shost, char *buffer, char **start,*/ cp = boardp->prtbuf;if (ASC_NARROW_BOARD (boardp)) { - cplen =asc_prt_asc_board_info (shp, cp,ASC_PRTBUF_SIZE ); + cplen =asc_prt_asc_board_info (shost, cp,ASC_PRTBUF_SIZE ); }else { - cplen =asc_prt_adv_board_info (shp, cp,ASC_PRTBUF_SIZE ); + cplen =asc_prt_adv_board_info (shost, cp,ASC_PRTBUF_SIZE ); }ASC_ASSERT (cplen <ASC_PRTBUF_SIZE ); cnt =asc_proc_copy (advoffset, offset, curbuf, leftlen, cp, cplen);
@@ -4653,7 +4641,7 @@ advansys_biosparam(struct scsi_device *sdev, struct block_device *bdev, }static int __init advansys_detect (struct scsi_host_template *tpnt); -static int advansys_release (struct Scsi_Host *shp); +static int advansys_release (struct Scsi_Host *shost);static struct scsi_host_template driver_template = { .proc_name ="advansys" ,
Signature · 1 line
1.4.4.4
Re: [PATCH 1/21] advansys: Clean up proc_info implementation
drivers/scsi/advansys.c advansys_biosparam(struct scsi_device *sdev, struct block_device *bdev, · line 4647
Quoted 9 lines · Matthew Wilcox
drivers/scsi/advansys.c +1
-1@@ -4653,7 +4641,7 @@ advansys_biosparam(struct scsi_device *sdev, struct block_device *bdev, }static int __init advansys_detect (struct scsi_host_template *tpnt); -static int advansys_release (struct Scsi_Host *shp); +static int advansys_release (struct Scsi_Host *shost);static struct scsi_host_template driver_template = { .proc_name ="advansys" ,
Not a _bug_ per se, but the above useless patch chunk seems to have leaked into this patch.
Jeff
Re: [PATCH 1/21] advansys: Clean up proc_info implementation
Quoted 2 lines · Jeff Garzik
Not a _bug_ per se, but the above useless patch chunk seems to have leaked into this patch.
Weird. Those lines are deleted later in this patch series anyway, so I don't intend to worry about this problem unless I respin the patch series. Thanks for pointing it out though.
Signature · 3 lines
"Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
PATCH 2/21 advansys: version, copyright, etc
In reply to Matthew WilcoxUpdate the version to 3.4
Add my copyright
Add myself to MAINTAINERS
Exercise my right to change the license from dual BSD/GPL to GPL Don't force the definition of CONFIG_ISA on x86
Always include pci.h
Stop including stat.h
--- MAINTAINERS | 6 ++++++ drivers/scsi/advansys.c | 43 +++++++++++++++++++------------------------ 2 files changed, 25 insertions(+), 24 deletions(-)
MAINTAINERS +6 -0
@@ -282,6 +282,12 @@ P: Colin Leroy M: [email protected] S: Maintained +ADVANSYS SCSI DRIVER +P: Matthew Wilcox +M: [email protected] +L: [email protected] +S: Maintained + AEDSP16 DRIVER P: Riccardo Facchetti M: [email protected]
drivers/scsi/advansys.c +19 -24
89 lines not on this page · raw message
Signature · 1 line
1.4.4.4
Re: [PATCH 2/21] advansys: version, copyright, etc
Quoted 1 line · Matthew Wilcox
Exercise my right to change the license from dual BSD/GPL to GPL
What do the other copyright holders think about this?
Jeff
Re: [PATCH 2/21] advansys: version, copyright, etc
Quoted 4 lines · Jeff Garzik
Quoted 1 line · Matthew Wilcox
Exercise my right to change the license from dual BSD/GPL to GPL
What do the other copyright holders think about this?
They're not contactable.
Signature · 3 lines
"Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Re: [PATCH 2/21] advansys: version, copyright, etc
Quoted 6 lines · Matthew Wilcox
Quoted 3 lines · Jeff Garzik
Quoted 1 line · Matthew Wilcox
Exercise my right to change the license from dual BSD/GPL to GPL
What do the other copyright holders think about this?
They're not contactable.
^^ easily
AFAIK that does not eliminate your legal obligation...
Jeff
Re: [PATCH 2/21] advansys: version, copyright, etc
Quoted 10 lines · Jeff Garzik
Quoted 6 lines · Matthew Wilcox
Quoted 3 lines · Jeff Garzik
Quoted 1 line · Matthew Wilcox
Exercise my right to change the license from dual BSD/GPL to GPL
What do the other copyright holders think about this?
They're not contactable.
^^ easily
AFAIK that does not eliminate your legal obligation...
No, really. They don't exist any more. Advanced System Products (first listed copyright holder) changed their name to ConnectCom Solutions (second copyright holder) and were then acquired (in part) by Initio.
In any case, this is not a legal question it's an ethical question. Depending on whether you believe the copyright statment or the MODULE_LICENSE() statement, it's either licensed under the BSD license, or a disjunction of the GPL and BSD licenses. In either case, I am entitled to take the work and redistribute it under the terms of the GPL.
We don't generally do it if the copyright holders are around to object, because we're nice people. But when they're gone, there's no ethical, moral, or legal reason not to.
IANAL.
Signature · 3 lines
"Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Re: [PATCH 2/21] advansys: version, copyright, etc
In reply to Jeff GarzikQuoted 7 lines · On Thu, 26 Jul 2007 15:08:39 -0400 Jeff Garzik <[email protected]> wrote:
Quoted 5 lines · Matthew Wilcox
Quoted 3 lines · Jeff Garzik
Quoted 1 line · Matthew Wilcox
Exercise my right to change the license from dual BSD/GPL to GPL
What do the other copyright holders think about this?
They're not contactable.
AFAIK that does not eliminate your legal obligation...
Isn't the whole point of dual BSD/GPL that it makes it possible to make the driver BSD only/GPL only later on? Pure BSD code (w/o advertising clause) can have additional restrictions (GPL or some proprietary licensing terms) added later on by a vendor, so why can't BSD/GPL code be relicensed the same way?
If it's polite to relicense the code is a completely different question. On the other hand, if Matthew makes major changes and wants those to become GPL, that's up to him.
/Christer
Off topic: I've been told that some Irish radio station has had a competition where you were supposed to send in photos of people holding an egg. Can somebody tell me what radio station is that and where I can find the pictures?
PATCH 3/21 advansys: Convert to pci_register_driver interface
In reply to Matthew Wilcox - Add a pci_driver interface for the PCI advansys devices.
- For ISA/EISA/VLB devices, we still call advansys_detect and advansys_release.
- Many functions are converted from __init to __devinit to allow hotplug PCI to work.
- Remove some now-unused macros and struct definitions
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 371 ++++++++++++++++++----------------------------- 1 files changed, 140 insertions(+), 231 deletions(-)
drivers/scsi/advansys.c +140 -231
@@ -865,12 +865,8 @@ typedef unsigned char uchar;#define AscPCIConfigLatencyTimer 0x000D#define AscPCIIOBaseRegister 0x0010#define AscPCICmdRegBits_IOMemBusMaster 0x0007 -#define ASC_PCI_ID2BUS (id) ((id) & 0xFF) -#define ASC_PCI_ID2DEV (id) (((id) >> 11) & 0x1F)#define ASC_PCI_ID2FUNC (id) (((id) >> 8) & 0x7)#define ASC_PCI_MKID (bus, dev, func) ((((dev) & 0x1F) << 11) | (((func) & 0x7) << 8) | ((bus) & 0xFF)) -#define ASC_PCI_REVISION_3150 0x02 -#define ASC_PCI_REVISION_3050 0x03#define ASC_DVCLIB_CALL_DONE (1)#define ASC_DVCLIB_CALL_FAILED (0)
@@ -3422,7 +3418,7 @@ typedef struct {#define ASC_NUM_BOARD_SUPPORTED 16#define ASC_NUM_IOPORT_PROBE 4 -#define ASC_NUM_BUS 4 +#define ASC_NUM_BUS 3/* Reference Scsi_Host hostdata */ #define ASC_BOARDP (host) ((asc_board_t *) &((host)->hostdata))
@@ -3526,11 +3522,6 @@ typedef struct scsi_cmnd REQ, *REQP;/* Return non-zero, if the queue is empty. */ #define ASC_QUEUE_EMPTY (ascq) ((ascq)->q_tidmask == 0) -#define PCI_MAX_SLOT 0x1F -#define PCI_MAX_BUS 0xFF -#define PCI_IOADDRESS_MASK 0xFFFE -#define ASC_PCI_DEVICE_ID_CNT 6/* PCI Device ID count. */ -#ifndef ADVANSYS_STATS #define ASC_STATS (shost, counter)#define ASC_STATS_ADD (shost, counter, count)
@@ -3856,52 +3847,6 @@ typedef struct asc_board { } asc_board_t;/* -* PCI configuration structures -*/ -typedef struct _PCI_DATA_ { -uchar type; -uchar bus; -uchar slot; -uchar func; -uchar offset; -}PCI_DATA ; - -typedef struct _PCI_DEVICE_ { -ushort vendorID; -ushort deviceID; -ushort slotNumber; -ushort slotFound; -uchar busNumber; -uchar maxBusNumber; -uchar devFunc; -ushort startSlot; -ushort endSlot; -uchar bridge; -uchar type; -}PCI_DEVICE ; - -typedef struct _PCI_CONFIG_SPACE_ { -ushort vendorID; -ushort deviceID; -ushort command; -ushort status; -uchar revision; -uchar classCode[3 ]; -uchar cacheSize; -uchar latencyTimer; -uchar headerType; -uchar bist; -ADV_PADDR baseAddress[6 ]; -ushort reserved[4 ]; -ADV_PADDR optionRomAddr; -ushort reserved2[4 ]; -uchar irqLine; -uchar irqPin; -uchar minGnt; -uchar maxLatency; -}PCI_CONFIG_SPACE ; - -/* * --- Driver Data */
@@ -3920,12 +3865,11 @@ static uchar overrun_buf[ASC_OVERRUN_BSIZE] = { 0 };static ASC_SCSI_Q asc_scsi_q = { {0 } };static ASC_SG_HEAD asc_sg_head = {0 }; -/* List of supported bus types. */ +/* List of bus types probed in advansys_detect. */ static ushort asc_bus[ASC_NUM_BUS ]__initdata = {ASC_IS_ISA ,ASC_IS_VL ,ASC_IS_EISA , -ASC_IS_PCI , };static int asc_iopflag =ASC_FALSE ;
@@ -3936,7 +3880,6 @@ static char *asc_bus_name[ASC_NUM_BUS] = {"ASC_IS_ISA" ,"ASC_IS_VL" , "ASC_IS_EISA", - "ASC_IS_PCI", };static int asc_dbglvl =3 ;
@@ -4635,17 +4578,12 @@ advansys_biosparam(struct scsi_device *sdev, struct block_device *bdev,return 0 ; } -static int __init advansys_detect (struct scsi_host_template *tpnt); -static int advansys_release (struct Scsi_Host *shost); -static struct scsi_host_template driver_template = { .proc_name ="advansys" , #ifdef CONFIG_PROC_FS .proc_info = advansys_proc_info, #endif .name ="advansys" , - .detect = advansys_detect, - .release = advansys_release, .info = advansys_info, .queuecommand = advansys_queuecommand, .eh_bus_reset_handler = advansys_reset,
@@ -4667,8 +4605,6 @@ static struct scsi_host_template driver_template = { .use_clustering = ENABLE_CLUSTERING, }; -#include "scsi_module.c" -/* * --- Miscellaneous Driver Functions */
@@ -4699,6 +4635,8 @@ static irqreturn_t advansys_interrupt(int irq, void *dev_id)*/ for (i =0 ; i < asc_board_count; i++) { shost = asc_host[i]; +if (!shost) +continue ; boardp =ASC_BOARDP (shost);ASC_DBG2 (2 ,"advansys_interrupt: i %d, boardp 0x%lx\n" , i, (ulong )boardp);
@@ -7502,7 +7440,7 @@ DvcGetQinfo(PortAddr iop_base, ushort s_addr, uchar *inbuf, int words)/* * Read a PCI configuration byte. */ -static uchar __init DvcReadPCIConfigByte (ASC_DVC_VAR *asc_dvc,ushort offset) +static uchar __devinit DvcReadPCIConfigByte (ASC_DVC_VAR *asc_dvc,ushort offset) {#ifdef CONFIG_PCI uchar byte_data;
@@ -7516,7 +7454,7 @@ static uchar __init DvcReadPCIConfigByte(ASC_DVC_VAR *asc_dvc, ushort offset)/* * Write a PCI configuration byte. */ -static void __init +static void __devinit DvcWritePCIConfigByte (ASC_DVC_VAR *asc_dvc,ushort offset,uchar byte_data) {#ifdef CONFIG_PCI
@@ -7528,7 +7466,7 @@ DvcWritePCIConfigByte(ASC_DVC_VAR *asc_dvc, ushort offset, uchar byte_data)* Return the BIOS address of the adapter at the specified * I/O port and with the specified bus type. */ -static ushort __init AscGetChipBiosAddress (PortAddr iop_base,ushort bus_type) +static ushort __devinit AscGetChipBiosAddress (PortAddr iop_base,ushort bus_type) {ushort cfg_lsw;ushort bios_addr;
@@ -7599,7 +7537,7 @@ DvcGetPhyAddr(ADV_DVC_VAR *asc_dvc, ADV_SCSI_REQ_Q *scsiq,/* * Read a PCI configuration byte. */ -static uchar __init DvcAdvReadPCIConfigByte (ADV_DVC_VAR *asc_dvc,ushort offset) +static uchar __devinit DvcAdvReadPCIConfigByte (ADV_DVC_VAR *asc_dvc,ushort offset) {#ifdef CONFIG_PCI uchar byte_data;
@@ -7613,7 +7551,7 @@ static uchar __init DvcAdvReadPCIConfigByte(ADV_DVC_VAR *asc_dvc, ushort offset)/* * Write a PCI configuration byte. */ -static void __init +static void __devinit DvcAdvWritePCIConfigByte (ADV_DVC_VAR *asc_dvc,ushort offset,uchar byte_data) {#ifdef CONFIG_PCI
@@ -8209,7 +8147,7 @@ static void asc_prt_hex(char *f, uchar *s, int l)* --- Asc Library Functions */ -static ushort __init AscGetEisaChipCfg (PortAddr iop_base) +static ushort __devinit AscGetEisaChipCfg (PortAddr iop_base) {PortAddr eisa_cfg_iop;
@@ -8218,7 +8156,7 @@ static ushort __init AscGetEisaChipCfg(PortAddr iop_base)return (inpw (eisa_cfg_iop)); } -static uchar __init AscSetChipScsiID (PortAddr iop_base,uchar new_host_id) +static uchar __devinit AscSetChipScsiID (PortAddr iop_base,uchar new_host_id) {ushort cfg_lsw;
@@ -8232,7 +8170,7 @@ static uchar __init AscSetChipScsiID(PortAddr iop_base, uchar new_host_id)return (AscGetChipScsiID (iop_base)); } -static uchar __init AscGetChipScsiCtrl (PortAddr iop_base) +static uchar __devinit AscGetChipScsiCtrl (PortAddr iop_base) {uchar sc;
@@ -8242,7 +8180,7 @@ static uchar __init AscGetChipScsiCtrl(PortAddr iop_base)return (sc); } -static uchar __init AscGetChipVersion (PortAddr iop_base,ushort bus_type) +static uchar __devinit AscGetChipVersion (PortAddr iop_base,ushort bus_type) {if ((bus_type &ASC_IS_EISA ) !=0 ) {PortAddr eisa_iop;
@@ -8255,7 +8193,7 @@ static uchar __init AscGetChipVersion(PortAddr iop_base, ushort bus_type)return (AscGetChipVerNo (iop_base)); } -static ushort __init AscGetChipBusType (PortAddr iop_base) +static ushort __devinit AscGetChipBusType (PortAddr iop_base) {ushort chip_ver;
@@ -8407,14 +8345,14 @@ static void __init AscSetISAPNPWaitForKey(void) }#endif /* CONFIG_ISA */ -static void __init AscToggleIRQAct (PortAddr iop_base) +static void __devinit AscToggleIRQAct (PortAddr iop_base) {AscSetChipStatus (iop_base,CIW_IRQ_ACT );AscSetChipStatus (iop_base,0 );return ; } -static uchar __init AscGetChipIRQ (PortAddr iop_base,ushort bus_type) +static uchar __devinit AscGetChipIRQ (PortAddr iop_base,ushort bus_type) {ushort cfg_lsw;uchar chip_irq;
@@ -8442,7 +8380,7 @@ static uchar __init AscGetChipIRQ(PortAddr iop_base, ushort bus_type)return ((uchar)(chip_irq +ASC_MIN_IRQ_NO )); } -static uchar __init +static uchar __devinit AscSetChipIRQ (PortAddr iop_base,uchar irq_no,ushort bus_type) {ushort cfg_lsw;
@@ -8479,7 +8417,7 @@ AscSetChipIRQ(PortAddr iop_base, uchar irq_no, ushort bus_type) }#ifdef CONFIG_ISA -static void __init AscEnableIsaDma (uchar dma_channel) +static void __devinit AscEnableIsaDma (uchar dma_channel) {if (dma_channel <4 ) {outp (0x000B , (ushort)(0xC0 | dma_channel));
@@ -10561,7 +10499,7 @@ static int AscResetChipAndScsiBus(ASC_DVC_VAR *asc_dvc)return (AscIsChipHalted (iop_base)); } -static ASC_DCNT __init AscGetMaxDmaCount (ushort bus_type) +static ASC_DCNT __devinit AscGetMaxDmaCount (ushort bus_type) {if (bus_type &ASC_IS_ISA )return (ASC_MAX_ISA_DMA_COUNT );
@@ -10571,7 +10509,7 @@ static ASC_DCNT __init AscGetMaxDmaCount(ushort bus_type) }#ifdef CONFIG_ISA -static ushort __init AscGetIsaDmaChannel (PortAddr iop_base) +static ushort __devinit AscGetIsaDmaChannel (PortAddr iop_base) {ushort channel;
@@ -10583,7 +10521,7 @@ static ushort __init AscGetIsaDmaChannel(PortAddr iop_base)return (channel +4 ); } -static ushort __init AscSetIsaDmaChannel (PortAddr iop_base,ushort dma_channel) +static ushort __devinit AscSetIsaDmaChannel (PortAddr iop_base,ushort dma_channel) {ushort cfg_lsw;uchar value;
@@ -10601,7 +10539,7 @@ static ushort __init AscSetIsaDmaChannel(PortAddr iop_base, ushort dma_channel)return (0 ); } -static uchar __init AscSetIsaDmaSpeed (PortAddr iop_base,uchar speed_value) +static uchar __devinit AscSetIsaDmaSpeed (PortAddr iop_base,uchar speed_value) { speed_value &=0x07 ;AscSetBank (iop_base,1 );
@@ -10610,7 +10548,7 @@ static uchar __init AscSetIsaDmaSpeed(PortAddr iop_base, uchar speed_value)return (AscGetIsaDmaSpeed (iop_base)); } -static uchar __init AscGetIsaDmaSpeed (PortAddr iop_base) +static uchar __devinit AscGetIsaDmaSpeed (PortAddr iop_base) {uchar speed_value;
@@ -10622,7 +10560,7 @@ static uchar __init AscGetIsaDmaSpeed(PortAddr iop_base) }#endif /* CONFIG_ISA */ -static ushort __init +static ushort __devinit AscReadPCIConfigWord (ASC_DVC_VAR *asc_dvc,ushort pci_config_offset) {uchar lsb, msb;
@@ -10632,7 +10570,7 @@ AscReadPCIConfigWord(ASC_DVC_VAR *asc_dvc, ushort pci_config_offset)return ((ushort)((msb <<8 ) | lsb)); } -static ushort __init AscInitGetConfig (ASC_DVC_VAR *asc_dvc) +static ushort __devinit AscInitGetConfig (ASC_DVC_VAR *asc_dvc) {ushort warn_code;PortAddr iop_base;
@@ -10717,7 +10655,7 @@ static ushort __init AscInitGetConfig(ASC_DVC_VAR *asc_dvc)return (warn_code); } -static ushort __init AscInitSetConfig (ASC_DVC_VAR *asc_dvc) +static ushort __devinit AscInitSetConfig (ASC_DVC_VAR *asc_dvc) {ushort warn_code =0 ;
@@ -10733,7 +10671,7 @@ static ushort __init AscInitSetConfig(ASC_DVC_VAR *asc_dvc)return (warn_code); } -static ushort __init AscInitFromAscDvcVar (ASC_DVC_VAR *asc_dvc) +static ushort __devinit AscInitFromAscDvcVar (ASC_DVC_VAR *asc_dvc) {PortAddr iop_base;ushort cfg_msw;
@@ -10834,7 +10772,7 @@ static ushort AscInitAsc1000Driver(ASC_DVC_VAR *asc_dvc)return (warn_code); } -static ushort __init AscInitAscDvcVar (ASC_DVC_VAR *asc_dvc) +static ushort __devinit AscInitAscDvcVar (ASC_DVC_VAR *asc_dvc) {int i;PortAddr iop_base;
@@ -10946,7 +10884,7 @@ static ushort __init AscInitAscDvcVar(ASC_DVC_VAR *asc_dvc)return (warn_code); } -static ushort __init AscInitFromEEP (ASC_DVC_VAR *asc_dvc) +static ushort __devinit AscInitFromEEP (ASC_DVC_VAR *asc_dvc) {ASCEEP_CONFIG eep_config_buf;ASCEEP_CONFIG *eep_config;
@@ -11167,7 +11105,7 @@ static ushort AscInitMicroCodeVar(ASC_DVC_VAR *asc_dvc)return (warn_code); } -static int __init AscTestExternalLram (ASC_DVC_VAR *asc_dvc) +static int __devinit AscTestExternalLram (ASC_DVC_VAR *asc_dvc) {PortAddr iop_base;ushort q_addr;
@@ -11189,7 +11127,7 @@ static int __init AscTestExternalLram(ASC_DVC_VAR *asc_dvc)return (sta); } -static int __init AscWriteEEPCmdReg (PortAddr iop_base,uchar cmd_reg) +static int __devinit AscWriteEEPCmdReg (PortAddr iop_base,uchar cmd_reg) {uchar read_back;int retry;
@@ -11208,7 +11146,7 @@ static int __init AscWriteEEPCmdReg(PortAddr iop_base, uchar cmd_reg) } } -static int __init AscWriteEEPDataReg (PortAddr iop_base,ushort data_reg) +static int __devinit AscWriteEEPDataReg (PortAddr iop_base,ushort data_reg) {ushort read_back;int retry;
@@ -11227,19 +11165,19 @@ static int __init AscWriteEEPDataReg(PortAddr iop_base, ushort data_reg) } } -static void __init AscWaitEEPRead (void ) +static void __devinit AscWaitEEPRead (void ) {DvcSleepMilliSecond (1 );return ; } -static void __init AscWaitEEPWrite (void ) +static void __devinit AscWaitEEPWrite (void ) {DvcSleepMilliSecond (20 );return ; } -static ushort __init AscReadEEPWord (PortAddr iop_base,uchar addr) +static ushort __devinit AscReadEEPWord (PortAddr iop_base,uchar addr) {ushort read_wval;uchar cmd_reg;
@@ -11254,7 +11192,7 @@ static ushort __init AscReadEEPWord(PortAddr iop_base, uchar addr)return (read_wval); } -static ushort __init +static ushort __devinit AscWriteEEPWord (PortAddr iop_base,uchar addr,ushort word_val) {ushort read_wval;
@@ -11275,7 +11213,7 @@ AscWriteEEPWord(PortAddr iop_base, uchar addr, ushort word_val)return (read_wval); } -static ushort __init +static ushort __devinit AscGetEEPConfig (PortAddr iop_base,ASCEEP_CONFIG *cfg_buf,ushort bus_type) {ushort wval;
@@ -11322,7 +11260,7 @@ AscGetEEPConfig(PortAddr iop_base, ASCEEP_CONFIG *cfg_buf, ushort bus_type)return (sum); } -static int __init +static int __devinit AscSetEEPConfigOnce (PortAddr iop_base,ASCEEP_CONFIG *cfg_buf,ushort bus_type) {int n_error;
@@ -11418,7 +11356,7 @@ AscSetEEPConfigOnce(PortAddr iop_base, ASCEEP_CONFIG *cfg_buf, ushort bus_type)return (n_error); } -static int __init +static int __devinit AscSetEEPConfig (PortAddr iop_base,ASCEEP_CONFIG *cfg_buf,ushort bus_type) {int retry;
@@ -13830,7 +13768,7 @@ static ADV_DCNT _adv_asc38C1600_chksum = 0x0604EF77UL; /* Expanded little-endian* on big-endian platforms so char fields read as words are actually being * unswapped on big-endian platforms. */ -static ADVEEP_3550_CONFIG Default_3550_EEPROM_Config__initdata = { +static ADVEEP_3550_CONFIG Default_3550_EEPROM_Config__devinitdata = {ADV_EEPROM_BIOS_ENABLE ,/* cfg_lsw */ 0x0000 ,/* cfg_msw */ 0xFFFF ,/* disc_enable */
@@ -13868,7 +13806,7 @@ static ADVEEP_3550_CONFIG Default_3550_EEPROM_Config __initdata = {0 /* num_of_err */ }; -static ADVEEP_3550_CONFIG ADVEEP_3550_Config_Field_IsChar__initdata = { +static ADVEEP_3550_CONFIG ADVEEP_3550_Config_Field_IsChar__devinitdata = {0 ,/* cfg_lsw */ 0 ,/* cfg_msw */ 0 ,/* -disc_enable */
@@ -13906,7 +13844,7 @@ static ADVEEP_3550_CONFIG ADVEEP_3550_Config_Field_IsChar __initdata = {0 /* num_of_err */ }; -static ADVEEP_38C0800_CONFIG Default_38C0800_EEPROM_Config__initdata = { +static ADVEEP_38C0800_CONFIG Default_38C0800_EEPROM_Config__devinitdata = {ADV_EEPROM_BIOS_ENABLE ,/* 00 cfg_lsw */ 0x0000 ,/* 01 cfg_msw */ 0xFFFF ,/* 02 disc_enable */
@@ -13971,7 +13909,7 @@ static ADVEEP_38C0800_CONFIG Default_38C0800_EEPROM_Config __initdata = {0 /* 63 reserved */ }; -static ADVEEP_38C0800_CONFIG ADVEEP_38C0800_Config_Field_IsChar__initdata = { +static ADVEEP_38C0800_CONFIG ADVEEP_38C0800_Config_Field_IsChar__devinitdata = {0 ,/* 00 cfg_lsw */ 0 ,/* 01 cfg_msw */ 0 ,/* 02 disc_enable */
@@ -14036,7 +13974,7 @@ static ADVEEP_38C0800_CONFIG ADVEEP_38C0800_Config_Field_IsChar __initdata = {0 /* 63 reserved */ }; -static ADVEEP_38C1600_CONFIG Default_38C1600_EEPROM_Config__initdata = { +static ADVEEP_38C1600_CONFIG Default_38C1600_EEPROM_Config__devinitdata = {ADV_EEPROM_BIOS_ENABLE ,/* 00 cfg_lsw */ 0x0000 ,/* 01 cfg_msw */ 0xFFFF ,/* 02 disc_enable */
@@ -14101,7 +14039,7 @@ static ADVEEP_38C1600_CONFIG Default_38C1600_EEPROM_Config __initdata = {0 /* 63 reserved */ }; -static ADVEEP_38C1600_CONFIG ADVEEP_38C1600_Config_Field_IsChar__initdata = { +static ADVEEP_38C1600_CONFIG ADVEEP_38C1600_Config_Field_IsChar__devinitdata = {0 ,/* 00 cfg_lsw */ 0 ,/* 01 cfg_msw */ 0 ,/* 02 disc_enable */
@@ -14174,7 +14112,7 @@ static ADVEEP_38C1600_CONFIG ADVEEP_38C1600_Config_Field_IsChar __initdata = {* For a non-fatal error return a warning code. If there are no warnings * then 0 is returned. */ -static int __init AdvInitGetConfig (ADV_DVC_VAR *asc_dvc) +static int __devinit AdvInitGetConfig (ADV_DVC_VAR *asc_dvc) {ushort warn_code;AdvPortAddr iop_base;
@@ -16118,7 +16056,7 @@ static int AdvInitAsc38C1600Driver(ADV_DVC_VAR *asc_dvc)* * Note: Chip is stopped on entry. */ -static int __init AdvInitFrom3550EEP (ADV_DVC_VAR *asc_dvc) +static int __devinit AdvInitFrom3550EEP (ADV_DVC_VAR *asc_dvc) {AdvPortAddr iop_base;ushort warn_code;
@@ -16272,7 +16210,7 @@ static int __init AdvInitFrom3550EEP(ADV_DVC_VAR *asc_dvc)* * Note: Chip is stopped on entry. */ -static int __init AdvInitFrom38C0800EEP (ADV_DVC_VAR *asc_dvc) +static int __devinit AdvInitFrom38C0800EEP (ADV_DVC_VAR *asc_dvc) {AdvPortAddr iop_base;ushort warn_code;
@@ -16475,7 +16413,7 @@ static int __init AdvInitFrom38C0800EEP(ADV_DVC_VAR *asc_dvc)* * Note: Chip is stopped on entry. */ -static int __init AdvInitFrom38C1600EEP (ADV_DVC_VAR *asc_dvc) +static int __devinit AdvInitFrom38C1600EEP (ADV_DVC_VAR *asc_dvc) {AdvPortAddr iop_base;ushort warn_code;
@@ -16716,7 +16654,7 @@ static int __init AdvInitFrom38C1600EEP(ADV_DVC_VAR *asc_dvc)* * Return a checksum based on the EEPROM configuration read. */ -static ushort __init +static ushort __devinit AdvGet3550EEPConfig (AdvPortAddr iop_base,ADVEEP_3550_CONFIG *cfg_buf) {ushort wval, chksum;
@@ -16759,7 +16697,7 @@ AdvGet3550EEPConfig(AdvPortAddr iop_base, ADVEEP_3550_CONFIG *cfg_buf)* * Return a checksum based on the EEPROM configuration read. */ -static ushort __init +static ushort __devinit AdvGet38C0800EEPConfig (AdvPortAddr iop_base,ADVEEP_38C0800_CONFIG *cfg_buf) {ushort wval, chksum;
@@ -16802,7 +16740,7 @@ AdvGet38C0800EEPConfig(AdvPortAddr iop_base, ADVEEP_38C0800_CONFIG *cfg_buf)* * Return a checksum based on the EEPROM configuration read. */ -static ushort __init +static ushort __devinit AdvGet38C1600EEPConfig (AdvPortAddr iop_base,ADVEEP_38C1600_CONFIG *cfg_buf) {ushort wval, chksum;
@@ -16843,7 +16781,7 @@ AdvGet38C1600EEPConfig(AdvPortAddr iop_base, ADVEEP_38C1600_CONFIG *cfg_buf)/* * Read the EEPROM from specified location */ -static ushort __init AdvReadEEPWord (AdvPortAddr iop_base,int eep_word_addr) +static ushort __devinit AdvReadEEPWord (AdvPortAddr iop_base,int eep_word_addr) {AdvWriteWordRegister (iop_base,IOPW_EE_CMD ,ASC_EEP_CMD_READ | eep_word_addr);
@@ -16854,7 +16792,7 @@ static ushort __init AdvReadEEPWord(AdvPortAddr iop_base, int eep_word_addr)/* * Wait for EEPROM command to complete */ -static void __init AdvWaitEEPCmd (AdvPortAddr iop_base) +static void __devinit AdvWaitEEPCmd (AdvPortAddr iop_base) {int eep_delay_ms;
@@ -16875,7 +16813,7 @@ static void __init AdvWaitEEPCmd(AdvPortAddr iop_base)/* * Write the EEPROM from 'cfg_buf'. */ -void __init +void __devinit AdvSet3550EEPConfig (AdvPortAddr iop_base,ADVEEP_3550_CONFIG *cfg_buf) {ushort *wbuf;
@@ -16943,7 +16881,7 @@ AdvSet3550EEPConfig(AdvPortAddr iop_base, ADVEEP_3550_CONFIG *cfg_buf)/* * Write the EEPROM from 'cfg_buf'. */ -void __init +void __devinit AdvSet38C0800EEPConfig (AdvPortAddr iop_base,ADVEEP_38C0800_CONFIG *cfg_buf) {ushort *wbuf;
@@ -17011,7 +16949,7 @@ AdvSet38C0800EEPConfig(AdvPortAddr iop_base, ADVEEP_38C0800_CONFIG *cfg_buf)/* * Write the EEPROM from 'cfg_buf'. */ -void __init +void __devinit AdvSet38C1600EEPConfig (AdvPortAddr iop_base,ADVEEP_38C1600_CONFIG *cfg_buf) {ushort *wbuf;
@@ -18667,29 +18605,12 @@ advansys_board_found(int iop, struct device *dev, int bus_type)* it must not call SCSI mid-level functions including scsi_malloc() * and scsi_free(). */ -static int __init advansys_detect (struct scsi_host_template *tpnt) +static int __init advansys_detect (void ) {static int detect_called =ASC_FALSE ;int iop;int bus;int ioport =0 ; -struct device *dev =NULL ; -#ifdef CONFIG_PCI -int pci_init_search =0 ; -struct pci_dev *pci_devicep[ASC_NUM_BOARD_SUPPORTED ]; -int pci_card_cnt_max =0 ; -int pci_card_cnt =0 ; -struct pci_dev *pdev =NULL ; -int pci_device_id_cnt =0 ; -unsigned int pci_device_id[ASC_PCI_DEVICE_ID_CNT ] = { -PCI_DEVICE_ID_ASP_1200A , -PCI_DEVICE_ID_ASP_ABP940 , -PCI_DEVICE_ID_ASP_ABP940U , -PCI_DEVICE_ID_ASP_ABP940UW , -PCI_DEVICE_ID_38C0800_REV1 , -PCI_DEVICE_ID_38C1600_REV1 - }; -#endif /* CONFIG_PCI */ if (detect_called ==ASC_FALSE ) { detect_called =ASC_TRUE ;
@@ -18834,97 +18755,6 @@ static int __init advansys_detect(struct scsi_host_template *tpnt)#endif /* CONFIG_ISA */ break ; -case ASC_IS_PCI : -#ifdef CONFIG_PCI -if (pci_init_search ==0 ) { -int i, j; - - pci_init_search =1 ; - -/* Find all PCI cards. */ -while (pci_device_id_cnt < -ASC_PCI_DEVICE_ID_CNT ) { -if ((pdev = -pci_find_device - (PCI_VENDOR_ID_ASP , - pci_device_id - [pci_device_id_cnt], - pdev)) ==NULL ) { - pci_device_id_cnt++; - }else { -if (pci_enable_device - (pdev) ==0 ) { - pci_devicep - [pci_card_cnt_max++] - = pdev; - } - } - } - -/* -* Sort PCI cards in ascending order by PCI Bus, Slot, -* and Device Number. -*/ -for (i =0 ; i < pci_card_cnt_max -1 ; - i++) { -for (j = i +1 ; - j < pci_card_cnt_max; - j++) { -if ((pci_devicep[j]-> - bus->number < - pci_devicep[i]-> - bus->number) - || - ((pci_devicep[j]-> - bus->number == - pci_devicep[i]-> - bus->number) - && - (pci_devicep[j]-> - devfn < - pci_devicep[i]-> - devfn))) { - pdev = - pci_devicep - [i]; - pci_devicep[i] = - pci_devicep - [j]; - pci_devicep[j] = - pdev; - } - } - } - - pci_card_cnt =0 ; - }else { - pci_card_cnt++; - } - -if (pci_card_cnt == pci_card_cnt_max) { - iop =0 ; - }else { - pdev = pci_devicep[pci_card_cnt]; - -ASC_DBG2 (2 , -"advansys_detect: devfn %d, bus number %d\n" , - pdev->devfn, - pdev->bus->number); - iop =pci_resource_start (pdev,0 ); -ASC_DBG2 (1 , -"advansys_detect: vendorID %X, deviceID %X\n" , - pdev->vendor, - pdev->device); -ASC_DBG2 (2 , -"advansys_detect: iop %X, irqLine %d\n" , - iop, pdev->irq); - } -if (pdev) - dev = &pdev->dev; - -#endif /* CONFIG_PCI */ -break ; -default :ASC_PRINT1 ("advansys_detect: unknown bus type: %d\n",
@@ -18940,7 +18770,7 @@ static int __init advansys_detect(struct scsi_host_template *tpnt)break ; } -advansys_board_found (iop, dev, asc_bus[bus]); +advansys_board_found (iop,NULL , asc_bus[bus]); } }
@@ -18990,7 +18820,6 @@ static int advansys_release(struct Scsi_Host *shost)return 0 ; } -#ifdef CONFIG_PCI/* PCI Devices supported by this driver */ static struct pci_device_id advansys_pci_tbl[]__devinitdata = { {PCI_VENDOR_ID_ASP ,PCI_DEVICE_ID_ASP_1200A ,
@@ -19009,6 +18838,86 @@ static struct pci_device_id advansys_pci_tbl[] __devinitdata = { };MODULE_DEVICE_TABLE (pci, advansys_pci_tbl); -#endif /* CONFIG_PCI */ + +static int __devinit +advansys_pci_probe (struct pci_dev *pdev,const struct pci_device_id *ent) +{ +int ioport; +struct Scsi_Host *shost; + +if (pci_enable_device (pdev)) + goto fail; + + ioport =pci_resource_start (pdev,0 ); + shost =advansys_board_found (ioport, &pdev->dev,ASC_IS_PCI ); + +if (shost) { +pci_set_drvdata (pdev, shost); +return 0 ; + } + +pci_disable_device (pdev); + fail: +return -ENODEV ; +} + +static void __devexit advansys_pci_remove(struct pci_dev *pdev) +{ +int i; +struct Scsi_Host *shost =pci_get_drvdata (pdev); +scsi_remove_host (shost); +advansys_release (shost); + +for (i =0 ; i < asc_board_count; i++) { +if (asc_host[i] == shost) { + asc_host[i] =NULL ; +break ; + } + } + +pci_disable_device (pdev); +} + +static struct pci_driver advansys_pci_driver = { + .name ="advansys" , + .id_table = advansys_pci_tbl, + .probe = advansys_pci_probe, + .remove =__devexit_p (advansys_pci_remove), +}; + +static int __init advansys_init (void ) +{ +int count, error; + count =advansys_detect (); + error =pci_register_driver (&advansys_pci_driver); + +/* +* If we found some ISA, EISA or VLB devices, we must not fail. +* We may not drive any PCI devices, but it's better to drive +* the cards that we successfully discovered than none at all. +*/ +if (count >0 ) + error =0 ; +return error; +} + +static void __exit advansys_exit(void) +{ +int i; + +pci_unregister_driver (&advansys_pci_driver); + +for (i =0 ; i < asc_board_count; i++) { +struct Scsi_Host *host = asc_host[i]; +if (!host) +continue ; +scsi_remove_host (host); +advansys_release (host); + asc_host[i] =NULL ; + } +} + +module_init (advansys_init); +module_exit (advansys_exit);MODULE_LICENSE ("GPL" );
Signature · 1 line
1.4.4.4
Re: [PATCH 3/21] advansys: Convert to pci_register_driver interface
Reading this more, this commit breaks everything AFAICS.
You still use scsi_register, which populates ->legacy_hosts list, but no longer include scsi_module.c. Thus you alloc a bunch of scsi hosts, and... that's it.
They are never added, nor scanned.
Take a look at my gdth conversion, 'gdth' branch ofgit://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/misc-2.6.git
You want to do the SCSI host model conversion before PCI/EISA/ISA conversions. Otherwise you run into the breakage seen here.
Jeff
Re: [PATCH 3/21] advansys: Convert to pci_register_driver interface
In reply to Matthew WilcoxQuoted 11 lines · Matthew Wilcox
drivers/scsi/advansys.c +11
-0+static int __devinit +advansys_pci_probe (struct pci_dev *pdev,const struct pci_device_id *ent) +{ +int ioport; +struct Scsi_Host *shost; + +if (pci_enable_device (pdev)) + goto fail; + + ioport =pci_resource_start (pdev,0 ); + shost =advansys_board_found (ioport, &pdev->dev,ASC_IS_PCI );
drivers/scsi/advansys.c static struct pci_device_id advansys_pci_tbl[] __devinitdata = { · line 18862
Quoted 10 lines
drivers/scsi/advansys.c +10
-0+ +if (shost) { +pci_set_drvdata (pdev, shost); +return 0 ; + } + +pci_disable_device (pdev); + fail: +return -ENODEV ; +}
1) you should propagate pci_enable_device return value to caller on error
1.1) in general, following my comment #1 will make this function look more like other normal Linux code, i.e.
rc = foo
if (rc)
goto err_out
return 0;
err_out:
return rc;2) you dropped the check for pci_resource_start() returning zero (look for 'iop == 0' in original code)
3) what happened to PCI BAR #1 ? shouldn't you move that call here too? it gets ioremapped (look at 'pci_memory_address' in old code, right
next to pci_resource_start call)
4) it is often wise to add sanity checks that ensure that PCI BAR #0 == IORESOURCE_IO and PCI BAR #1 == IORESOURCE_MEM
Quoted 5 lines
drivers/scsi/advansys.c +5
-0+static void __devexit advansys_pci_remove(struct pci_dev *pdev) +{ +int i; +struct Scsi_Host *shost =pci_get_drvdata (pdev); +scsi_remove_host (shost);
drivers/scsi/advansys.c static struct pci_device_id advansys_pci_tbl[] __devinitdata = { · line 18878
Quoted 10 lines
drivers/scsi/advansys.c +10
-0+advansys_release (shost); + +for (i =0 ; i < asc_board_count; i++) { +if (asc_host[i] == shost) { + asc_host[i] =NULL ; +break ; + } + } + +pci_disable_device (pdev);
5) call pci_set_drvdata(pdev, NULL) as with other PCI drivers
drivers/scsi/advansys.c static struct pci_device_id advansys_pci_tbl[] __devinitdata = { · line 18886
Quoted 6 lines
drivers/scsi/advansys.c +6
-0+static struct pci_driver advansys_pci_driver = { + .name ="advansys" , + .id_table = advansys_pci_tbl, + .probe = advansys_pci_probe, + .remove =__devexit_p (advansys_pci_remove), +};
6) suggestions: tab alignment for struct member values; makes it far easier to read.
Quoted 11 lines
drivers/scsi/advansys.c +11
-0+static int __init advansys_init (void ) +{ +int count, error; + count =advansys_detect (); + error =pci_register_driver (&advansys_pci_driver); + +/* +* If we found some ISA, EISA or VLB devices, we must not fail. +* We may not drive any PCI devices, but it's better to drive +* the cards that we successfully discovered than none at all. +*/
drivers/scsi/advansys.c static struct pci_device_id advansys_pci_tbl[] __devinitdata = { · line 18908
Quoted 10 lines
drivers/scsi/advansys.c +10
-0+if (count >0 ) + error =0 ; +return error; +} + +static void __exit advansys_exit(void) +{ +int i; + +pci_unregister_driver (&advansys_pci_driver);
7) bug: don't unregister, if pci_register_driver() failed during init
drivers/scsi/advansys.c static struct pci_device_id advansys_pci_tbl[] __devinitdata = { · line 18916
Quoted 7 lines
drivers/scsi/advansys.c +7
-0+for (i =0 ; i < asc_board_count; i++) { +struct Scsi_Host *host = asc_host[i]; +if (!host) +continue ; +scsi_remove_host (host); +advansys_release (host); + asc_host[i] =NULL ;
this last line of code is rather pointless, isn't it?
Re: [PATCH 3/21] advansys: Convert to pci_register_driver interface
Quoted 1 line · Jeff Garzik
1) you should propagate
pci_enable_devicereturn value to caller on error
ok. I'll return -ENODEV if I don't get a Scsi_Host back, though. Unless you think it's worth going through the PTR_ERR/IS_ERR/ERR_PTR hooplah. It's not like this is "what's scrogging my filesystem", it's "my driver didn't load".
Quoted 2 lines
2) you dropped the check for
pci_resource_start()returning zero (look for 'iop == 0' in original code)
That's not actually something that could happen through that path.
Quoted 3 lines
3) what happened to PCI BAR #1 ? shouldn't you move that call here too? it gets ioremapped (look at '
pci_memory_address' in old code, right
next topci_resource_startcall)
That'll move later, once I'm on the iomap path. We don't have the structure to stash the result in at this point.
Quoted 2 lines
4) it is often wise to add sanity checks that ensure that PCI BAR #0 ==
IORESOURCE_IOand PCI BAR #1 ==IORESOURCE_MEM
Why is that? There's no danger of advansys producing any more cards ;-)
Quoted 1 line
5) call
pci_set_drvdata(pdev, NULL)as with other PCI drivers
Why?
Quoted 2 lines
6) suggestions: tab alignment for struct member values; makes it far easier to read.
ok
Quoted 1 line
7) bug: don't unregister, if
pci_register_driver()failed during init
I assumed that pci_unregister_driver handled that, for the same reason that kfree(NULL) works. Otherwise I have to keep track of that in the driver somewhere.
Quoted 9 lines
drivers/scsi/advansys.c static struct pci_device_id advansys_pci_tbl[] __devinitdata = { · line 18916
Quoted 7 lines
drivers/scsi/advansys.c +7
-0+for (i =0 ; i < asc_board_count; i++) { +struct Scsi_Host *host = asc_host[i]; +if (!host) +continue ; +scsi_remove_host (host); +advansys_release (host); + asc_host[i] =NULL ;this last line of code is rather pointless, isn't it?
No ... see advansys_interrupt. Yes, that needs to be cleaned up *too*, but I can't fix everything at once. Eventually, the asc_host array will disappear.
Signature · 3 lines
"Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Re: [PATCH 3/21] advansys: Convert to pci_register_driver interface
I'll respond to the other stuff soon, but I wanted to add:
Quoted 7 lines
Quoted 7 lines
drivers/scsi/advansys.c static struct pci_device_id advansys_pci_tbl[] __devinitdata = { · line 18916
Quoted 7 lines
drivers/scsi/advansys.c +7
-0+for (i =0 ; i < asc_board_count; i++) { +struct Scsi_Host *host = asc_host[i]; +if (!host) +continue ; +scsi_remove_host (host); +advansys_release (host); + asc_host[i] =NULL ;
It is wise to have two loops in module exit:
1) For each host, call scsi_remove_host()
2) For each host, {do the rest of the cleanup}
scsi_module.c behaves in this way.
There is no strict requirement to do this, but IMO it is wise because you don't have to worry about the SCSI midlayer passing new traffic to asc_host[x+1] while you are deactivating and freeing asc_host[x].
Obviously this goes away for the new-style PCI/EISA/ISA probe APIs, once asc_host[] goes away.
Jeff
Re: [PATCH 3/21] advansys: Convert to pci_register_driver interface
In reply to Matthew WilcoxQuoted 4 lines · Matthew Wilcox
Quoted 1 line · Jeff Garzik
2) you dropped the check for
pci_resource_start()returning zero (look for 'iop == 0' in original code)That's not actually something that could happen through that path.
Er, huh? It's a value returned from the PCI device. Of course it can happen on that path. That's the first time you look at the PCI BAR values.
Quoted 6 lines
Quoted 3 lines
3) what happened to PCI BAR #1 ? shouldn't you move that call here too? it gets ioremapped (look at '
pci_memory_address' in old code, right
next topci_resource_startcall)That'll move later, once I'm on the iomap path. We don't have the structure to stash the result in at this point.
If you follow other PCI drivers, you should check it in pci_driver::probe().
Quoted 3 lines
Quoted 1 line
4) it is often wise to add sanity checks that ensure that PCI BAR #0 ==
IORESOURCE_IOand PCI BAR #1 ==IORESOURCE_MEMWhy is that? There's no danger of advansys producing any more cards ;-)
Granted it is far less important than checking pci_resource_start()==0 and pci_resource_len()==0 -- which happens often enough to care about it. Firmware or kernel PCI allocation routines or even PCI quirks can affect this.
Generally this is not about new cards but about misconfiguration, or even protection from users trying to see if adding a PCI ID works for them.
But overall, it's sane to check that the resource is memory before passing it to ioremap, so that you catch the error at the correct time.
This is standard stuff that all PCI drivers should do.
Quoted 3 lines
Quoted 1 line
5) call
pci_set_drvdata(pdev, NULL)as with other PCI driversWhy?
So that you don't leave a dangling pointer to an object that has already been destroyed.
Again, this is standard PCI driver practice.
Quoted 5 lines
Quoted 1 line
7) bug: don't unregister, if
pci_register_driver()failed during initI assumed that
pci_unregister_driverhandled that, for the same reason that kfree(NULL) works. Otherwise I have to keep track of that in the driver somewhere.
Well either verify your assumption or keep track of it :)
Quoted 9 lines
Quoted 8 lines
drivers/scsi/advansys.c static struct pci_device_id advansys_pci_tbl[] __devinitdata = { · line 18916
Quoted 7 lines
drivers/scsi/advansys.c +7
-0+for (i =0 ; i < asc_board_count; i++) { +struct Scsi_Host *host = asc_host[i]; +if (!host) +continue ; +scsi_remove_host (host); +advansys_release (host); + asc_host[i] =NULL ;this last line of code is rather pointless, isn't it?
Quoted 3 lines
No ... see
advansys_interrupt. Yes, that needs to be cleaned up *too*, but I can't fix everything at once. Eventually, theasc_hostarray will disappear.
Look at scsi_module.c: it is sane practice to have two loops for code like this: the first loop calls scsi_remove_host() for all hosts, and the second loop does the rest of the cleanup. IMO this is more conservative, as you know the SCSI layer will not be passing anything to any port on your card, when you start shutting it down. Makes it harder to trigger races and bugs.
Of course, when everything is converted to EISA/PCI hotplug API, you cannot do this anymore.
But I find it sane practice for older buses, especially.
Jeff
PATCH 4/21 advansys: restructure error handling in advansys_board_found
In reply to Matthew WilcoxPut all the error cleanup at the end of the function and goto the correct spot.
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 106 ++++++++++++++++------------------------------ 1 files changed, 37 insertions(+), 69 deletions(-)
drivers/scsi/advansys.c +37 -69
@@ -17804,9 +17804,7 @@ advansys_board_found(int iop, struct device *dev, int bus_type)ASC_PRINT3 ("advansys_board_found: board %d: ioremap(%x, %d) returned NULL\n" , boardp->id, pci_memory_address, iolen); -scsi_unregister (shost); - asc_board_count--; -return NULL ; + goto err_shost; } ASC_DBG1(1, "advansys_board_found: ioremap_addr: 0x%lx\n",
@@ -17840,9 +17838,7 @@ advansys_board_found(int iop, struct device *dev, int bus_type)ASC_PRINT3 ("advansys_board_found: board %d: kmalloc(%d, %d) returned NULL\n" , boardp->id,ASC_PRTBUF_SIZE ,GFP_ATOMIC ); -scsi_unregister (shost); - asc_board_count--; - return NULL; + goto err_unmap; }#endif /* CONFIG_PROC_FS */
@@ -17968,14 +17964,8 @@ advansys_board_found(int iop, struct device *dev, int bus_type) } } -if (err_code !=0 ) { -#ifdef CONFIG_PROC_FS -kfree (boardp->prtbuf); -#endif /* CONFIG_PROC_FS */ -scsi_unregister (shost); - asc_board_count--; -return NULL ; - } +if (err_code !=0 ) + goto err_free_proc;/* * Save the EEPROM configuration so that it can be displayed
@@ -18057,12 +18047,7 @@ advansys_board_found(int iop, struct device *dev, int bus_type) ("AscInitSetConfig: board %d error: init_state 0x%x, err_code 0x%x\n" , boardp->id, asc_dvc_varp->init_state, asc_dvc_varp->err_code); -#ifdef CONFIG_PROC_FS -kfree (boardp->prtbuf); -#endif /* CONFIG_PROC_FS */ -scsi_unregister (shost); - asc_board_count--; -return NULL ; + goto err_free_proc; }/*
@@ -18327,12 +18312,7 @@ advansys_board_found(int iop, struct device *dev, int bus_type)ASC_PRINT3 ("advansys_board_found: board %d: request_region() failed, port 0x%lx, len 0x%x\n" , boardp->id, (ulong )shost->io_port, boardp->asc_n_io_port); -#ifdef CONFIG_PROC_FS -kfree (boardp->prtbuf); -#endif /* CONFIG_PROC_FS */ -scsi_unregister (shost); - asc_board_count--; -return NULL ; + goto err_free_proc; }/* Register DMA Channel for Narrow boards. */
@@ -18342,19 +18322,12 @@ advansys_board_found(int iop, struct device *dev, int bus_type)/* Register DMA channel for ISA bus. */ if (asc_dvc_varp->bus_type &ASC_IS_ISA ) { shost->dma_channel = asc_dvc_varp->cfg->isa_dma_channel; -if ((ret = -request_dma (shost->dma_channel,"advansys" )) !=0 ) { + ret =request_dma (shost->dma_channel,"advansys" ); +if (ret) {ASC_PRINT3 ("advansys_board_found: board %d: request_dma() %d failed %d\n" , boardp->id, shost->dma_channel, ret); -release_region (shost->io_port, - boardp->asc_n_io_port); -#ifdef CONFIG_PROC_FS -kfree (boardp->prtbuf); -#endif /* CONFIG_PROC_FS */ -scsi_unregister (shost); - asc_board_count--; -return NULL ; + goto err_free_region; }AscEnableIsaDma (shost->dma_channel); }
@@ -18372,16 +18345,15 @@ advansys_board_found(int iop, struct device *dev, int bus_type)* If IRQF_DISABLED is not set, then interrupts are enabled * before the driver interrupt function is called. */ -if (((ret =request_irq (shost->irq, advansys_interrupt, -IRQF_DISABLED | (share_irq == - TRUE ? -IRQF_SHARED : -0 ),"advansys" , boardp)) !=0 ) - && - ((ret = -request_irq (shost->irq, advansys_interrupt, - (share_irq == TRUE ?IRQF_SHARED :0 ), -"advansys" , boardp)) !=0 )) { + ret =request_irq (shost->irq, advansys_interrupt,IRQF_DISABLED | + (share_irq == TRUE ?IRQF_SHARED :0 ),"advansys" , + boardp); +if (ret) + ret =request_irq (shost->irq, advansys_interrupt, + (share_irq == TRUE ?IRQF_SHARED :0 ), +"advansys" , boardp); + +if (ret) {if (ret == -EBUSY ) {ASC_PRINT2 ("advansys_board_found: board %d: request_irq(): IRQ 0x%x already in use.\n",
@@ -18395,17 +18367,7 @@ advansys_board_found(int iop, struct device *dev, int bus_type) ("advansys_board_found: board %d: request_irq(): IRQ 0x%x failed with %d\n" , boardp->id, shost->irq, ret); } -release_region (shost->io_port, boardp->asc_n_io_port); -iounmap (boardp->ioremap_addr); -if (shost->dma_channel !=NO_ISA_DMA ) { -free_dma (shost->dma_channel); - } -#ifdef CONFIG_PROC_FS -kfree (boardp->prtbuf); -#endif /* CONFIG_PROC_FS */ -scsi_unregister (shost); - asc_board_count--; -return NULL ; + goto err_free_dma; }/*
@@ -18562,9 +18524,7 @@ advansys_board_found(int iop, struct device *dev, int bus_type) }if (err_code !=0 ) { -release_region (shost->io_port, boardp->asc_n_io_port);if (ASC_WIDE_BOARD (boardp)) { -iounmap (boardp->ioremap_addr);kfree (boardp->orig_carrp); boardp->orig_carrp =NULL ;if (boardp->orig_reqp) {
@@ -18576,20 +18536,28 @@ advansys_board_found(int iop, struct device *dev, int bus_type)kfree (sgp); } } -if (shost->dma_channel !=NO_ISA_DMA ) { -free_dma (shost->dma_channel); - } -#ifdef CONFIG_PROC_FS -kfree (boardp->prtbuf); -#endif /* CONFIG_PROC_FS */ -free_irq (shost->irq, boardp); -scsi_unregister (shost); - asc_board_count--; -return NULL ; + goto err_free_irq; }ASC_DBG_PRT_SCSI_HOST (2 , shost);return shost; + + err_free_irq: +free_irq (shost->irq, boardp); + err_free_dma: +if (shost->dma_channel !=NO_ISA_DMA ) +free_dma (shost->dma_channel); + err_free_region: +release_region (shost->io_port, boardp->asc_n_io_port); + err_free_proc: +kfree (boardp->prtbuf); + err_unmap: +if (boardp->ioremap_addr) +iounmap (boardp->ioremap_addr); + err_shost: +scsi_unregister (shost); + asc_board_count--; +return NULL ; }/*
Signature · 1 line
1.4.4.4
PATCH 5/21 advansys: split advansys_init_wide_chip() out of advansys_board_found()
In reply to Matthew WilcoxThe error handling around that part of advansys_board_found() was tricky, so I split out that part into its own function. It's still 120 lines, but it's somewhat easier to see what's going on. Also use GFP_KERNEL when allocating memory during initialisation, not GFP_ATOMIC.
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 285 +++++++++++++++++++++------------------------- 1 files changed, 130 insertions(+), 155 deletions(-)
drivers/scsi/advansys.c +130 -155
@@ -17693,6 +17693,129 @@ static void AdvInquiryHandling(ADV_DVC_VAR *asc_dvc, ADV_SCSI_REQ_Q *scsiq) } } +static int __devinit +advansys_wide_init_chip (asc_board_t *boardp,ADV_DVC_VAR *adv_dvc_varp) +{ +ADV_CARR_T *carrp; +int req_cnt =0 ; +adv_req_t *reqp =NULL ; +int sg_cnt =0 ; +adv_sgblk_t *sgp; +int warn_code, err_code; + +/* +* Allocate buffer carrier structures. The total size +* is about 4 KB, so allocate all at once. +*/ + carrp =kmalloc (ADV_CARRIER_BUFSIZE ,GFP_KERNEL ); +ASC_DBG1 (1 ,"advansys_wide_init_chip: carrp 0x%lx\n" , (ulong ) carrp); + +if (!carrp) + goto kmalloc_failed; + +/* +* Allocate up to 'max_host_qng' request structures for the Wide +* board. The total size is about 16 KB, so allocate all at once. +* If the allocation fails decrement and try again. +*/ +for (req_cnt = adv_dvc_varp->max_host_qng; req_cnt >0 ; req_cnt--) { + reqp =kmalloc (sizeof (adv_req_t) * req_cnt,GFP_KERNEL ); + +ASC_DBG3 (1 ,"advansys_wide_init_chip: reqp 0x%p, req_cnt %d, " +"bytes %lu\n" , reqp, req_cnt, + (ulong )sizeof (adv_req_t) * req_cnt); + +if (reqp) +break ; + } + +if (!reqp) + goto kmalloc_failed; + +/* +* Allocate up to ADV_TOT_SG_BLOCK request structures for +* the Wide board. Each structure is about 136 bytes. +*/ + boardp->adv_sgblkp =NULL ; +for (sg_cnt =0 ; sg_cnt <ADV_TOT_SG_BLOCK ; sg_cnt++) { + sgp =kmalloc (sizeof (adv_sgblk_t),GFP_KERNEL ); + +if (!sgp) +break ; + + sgp->next_sgblkp = boardp->adv_sgblkp; + boardp->adv_sgblkp = sgp; + + } + +ASC_DBG3 (1 ,"advansys_wide_init_chip: sg_cnt %d * %u = %u bytes\n" , + sg_cnt,sizeof (adv_sgblk_t), + (unsigned )(sizeof (adv_sgblk_t) * sg_cnt)); + +if (!boardp->adv_sgblkp) + goto kmalloc_failed; + +/* Save carrier buffer pointer. */ + boardp->orig_carrp = carrp; + +/* +* Save original pointer for kfree() in case the +* driver is built as a module and can be unloaded. +*/ + boardp->orig_reqp = reqp; + + adv_dvc_varp->carrier_buf = carrp; + +/* +* Point 'adv_reqp' to the request structures and +* link them together. +*/ + req_cnt--; + reqp[req_cnt].next_reqp =NULL ; +for (; req_cnt >0 ; req_cnt--) { + reqp[req_cnt -1 ].next_reqp = &reqp[req_cnt]; + } + boardp->adv_reqp = &reqp[0 ]; + +if (adv_dvc_varp->chip_type ==ADV_CHIP_ASC3550 ) { +ASC_DBG (2 ,"advansys_wide_init_chip: AdvInitAsc3550Driver()\n" ); + warn_code =AdvInitAsc3550Driver (adv_dvc_varp); + }else if (adv_dvc_varp->chip_type ==ADV_CHIP_ASC38C0800 ) { +ASC_DBG (2 ,"advansys_wide_init_chip: AdvInitAsc38C0800Driver()" +"\n" ); + warn_code =AdvInitAsc38C0800Driver (adv_dvc_varp); + }else { +ASC_DBG (2 ,"advansys_wide_init_chip: AdvInitAsc38C1600Driver()" +"\n" ); + warn_code =AdvInitAsc38C1600Driver (adv_dvc_varp); + } + err_code = adv_dvc_varp->err_code; + +if (warn_code || err_code) { +ASC_PRINT3 ("advansys_wide_init_chip: board %d error: warn 0x%x," +" error 0x%x\n" , boardp->id, warn_code, err_code); + } + +if (err_code ==0 ) +return 0 ; + +while ((sgp = boardp->adv_sgblkp) !=NULL ) { + boardp->adv_sgblkp = sgp->next_sgblkp; +kfree (sgp); + } + + failed: +kfree (reqp); +kfree (carrp); +return err_code; + + kmalloc_failed: +ASC_PRINT1 ("advansys_wide_init_chip: board %d error: kmalloc() " +"failed\n" , boardp->id); + err_code =ADV_ERROR ; + goto failed; +} +static struct Scsi_Host *__devinit advansys_board_found (int iop,struct device *dev,int bus_type) {
@@ -17701,7 +17824,6 @@ advansys_board_found(int iop, struct device *dev, int bus_type)asc_board_t *boardp;ASC_DVC_VAR *asc_dvc_varp =NULL ;ADV_DVC_VAR *adv_dvc_varp =NULL ; -adv_sgblk_t *sgp =NULL ;int share_irq = FALSE;int iolen =0 ;ADV_PADDR pci_memory_address;
@@ -17834,10 +17956,10 @@ advansys_board_found(int iop, struct device *dev, int bus_type)* Allocate buffer for printing information from * /proc/scsi/advansys/[0...]. */ -if ((boardp->prtbuf =kmalloc (ASC_PRTBUF_SIZE ,GFP_ATOMIC )) ==NULL ) { -ASC_PRINT3 - ("advansys_board_found: board %d: kmalloc(%d, %d) returned NULL\n" , - boardp->id,ASC_PRTBUF_SIZE ,GFP_ATOMIC ); + boardp->prtbuf =kmalloc (ASC_PRTBUF_SIZE ,GFP_KERNEL ); +if (!boardp->prtbuf) { +ASC_PRINT2 ("advansys_board_found: board %d: kmalloc(%d) " +"returned NULL\n" , boardp->id,ASC_PRTBUF_SIZE ); goto err_unmap; }#endif /* CONFIG_PROC_FS */
@@ -18385,159 +18507,12 @@ advansys_board_found(int iop, struct device *dev, int bus_type) asc_dvc_varp->init_state, warn_code, err_code); } }else { -ADV_CARR_T *carrp; -int req_cnt =0 ; -adv_req_t *reqp =NULL ; -int sg_cnt =0 ; - -/* -* Allocate buffer carrier structures. The total size -* is about 4 KB, so allocate all at once. -*/ - carrp = (ADV_CARR_T *)kmalloc (ADV_CARRIER_BUFSIZE ,GFP_ATOMIC ); -ASC_DBG1 (1 ,"advansys_board_found: carrp 0x%lx\n" , (ulong )carrp); - -if (carrp ==NULL ) { - goto kmalloc_error; - } - -/* -* Allocate up to 'max_host_qng' request structures for -* the Wide board. The total size is about 16 KB, so -* allocate all at once. If the allocation fails decrement -* and try again. -*/ -for (req_cnt = adv_dvc_varp->max_host_qng; - req_cnt >0 ; req_cnt--) { - - reqp = (adv_req_t *) -kmalloc (sizeof (adv_req_t) * req_cnt,GFP_ATOMIC ); - -ASC_DBG3 (1 , -"advansys_board_found: reqp 0x%lx, req_cnt %d, bytes %lu\n" , - (ulong )reqp, req_cnt, - (ulong )sizeof (adv_req_t) * req_cnt); - -if (reqp !=NULL ) { -break ; - } - } -if (reqp ==NULL ) { - goto kmalloc_error; - } - -/* -* Allocate up to ADV_TOT_SG_BLOCK request structures for -* the Wide board. Each structure is about 136 bytes. -*/ - boardp->adv_sgblkp =NULL ; -for (sg_cnt =0 ; sg_cnt <ADV_TOT_SG_BLOCK ; sg_cnt++) { - - sgp = (adv_sgblk_t *) -kmalloc (sizeof (adv_sgblk_t),GFP_ATOMIC ); - -if (sgp ==NULL ) { -break ; - } - - sgp->next_sgblkp = boardp->adv_sgblkp; - boardp->adv_sgblkp = sgp; - - } -ASC_DBG3 (1 , -"advansys_board_found: sg_cnt %d * %u = %u bytes\n" , - sg_cnt,sizeof (adv_sgblk_t), - (unsigned )(sizeof (adv_sgblk_t) * sg_cnt)); - -/* -* If no request structures or scatter-gather structures could -* be allocated, then return an error. Otherwise continue with -* initialization. -*/ - kmalloc_error: -if (carrp ==NULL ) { -ASC_PRINT1 - ("advansys_board_found: board %d error: failed to kmalloc() carrier buffer.\n" , - boardp->id); - err_code =ADV_ERROR ; - }else if (reqp ==NULL ) { -kfree (carrp); -ASC_PRINT1 - ("advansys_board_found: board %d error: failed to kmalloc() adv_req_t buffer.\n" , - boardp->id); - err_code =ADV_ERROR ; - }else if (boardp->adv_sgblkp ==NULL ) { -kfree (carrp); -kfree (reqp); -ASC_PRINT1 - ("advansys_board_found: board %d error: failed to kmalloc() adv_sgblk_t buffers.\n" , - boardp->id); - err_code =ADV_ERROR ; - }else { - -/* Save carrier buffer pointer. */ - boardp->orig_carrp = carrp; - -/* -* Save original pointer for kfree() in case the -* driver is built as a module and can be unloaded. -*/ - boardp->orig_reqp = reqp; - - adv_dvc_varp->carrier_buf = carrp; - -/* -* Point 'adv_reqp' to the request structures and -* link them together. -*/ - req_cnt--; - reqp[req_cnt].next_reqp =NULL ; -for (; req_cnt >0 ; req_cnt--) { - reqp[req_cnt -1 ].next_reqp = &reqp[req_cnt]; - } - boardp->adv_reqp = &reqp[0 ]; - -if (adv_dvc_varp->chip_type ==ADV_CHIP_ASC3550 ) { -ASC_DBG (2 , -"advansys_board_found: AdvInitAsc3550Driver()\n" ); - warn_code =AdvInitAsc3550Driver (adv_dvc_varp); - }else if (adv_dvc_varp->chip_type == -ADV_CHIP_ASC38C0800 ) { -ASC_DBG (2 , -"advansys_board_found: AdvInitAsc38C0800Driver()\n" ); - warn_code = -AdvInitAsc38C0800Driver (adv_dvc_varp); - }else { -ASC_DBG (2 , -"advansys_board_found: AdvInitAsc38C1600Driver()\n" ); - warn_code = -AdvInitAsc38C1600Driver (adv_dvc_varp); - } - err_code = adv_dvc_varp->err_code; - -if (warn_code || err_code) { -ASC_PRINT3 - ("advansys_board_found: board %d error: warn 0x%x, error 0x%x\n" , - boardp->id, warn_code, err_code); - } - } + err_code =advansys_wide_init_chip (boardp, adv_dvc_varp); } -if (err_code !=0 ) { -if (ASC_WIDE_BOARD (boardp)) { -kfree (boardp->orig_carrp); - boardp->orig_carrp =NULL ; -if (boardp->orig_reqp) { -kfree (boardp->orig_reqp); - boardp->orig_reqp = boardp->adv_reqp =NULL ; - } -while ((sgp = boardp->adv_sgblkp) !=NULL ) { - boardp->adv_sgblkp = sgp->next_sgblkp; -kfree (sgp); - } - } +if (err_code !=0 ) goto err_free_irq; - } +ASC_DBG_PRT_SCSI_HOST (2 , shost);return shost;
Signature · 1 line
1.4.4.4
PATCH 6/21 advansys: Move to scsi hotplug initialisation model
In reply to Matthew Wilcox - Switch from scsi_register/scsi_unregister to scsi_host_alloc, scsi_add_host, scsi_scan_host and scsi_host_put.
- Rename the scsi_host_template to advansys_template
- Move the memory freeing on error from advansys_wide_init_chip() back into advansys_board_found() as we need to free it if scsi_add_host() returns an error.
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 71 +++++++++++++++++++++------------------------- 1 files changed, 32 insertions(+), 39 deletions(-)
drivers/scsi/advansys.c +32 -39
@@ -3784,7 +3784,7 @@ typedef struct adv_req {/* * Structure allocated for each board. * -* This structure is allocated by scsi_register() at the end +* This structure is allocated by scsi_host_alloc() at the end * of the 'Scsi_Host' structure starting at the 'hostdata' * field. It is guaranteed to be allocated from DMA-able memory. */
@@ -4578,7 +4578,7 @@ advansys_biosparam(struct scsi_device *sdev, struct block_device *bdev,return 0 ; } -static struct scsi_host_template driver_template = { +static struct scsi_host_template advansys_template = { .proc_name ="advansys" , #ifdef CONFIG_PROC_FS .proc_info = advansys_proc_info,
@@ -17696,7 +17696,6 @@ static void AdvInquiryHandling(ADV_DVC_VAR *asc_dvc, ADV_SCSI_REQ_Q *scsiq)static int __devinit advansys_wide_init_chip (asc_board_t *boardp,ADV_DVC_VAR *adv_dvc_varp) { -ADV_CARR_T *carrp;int req_cnt =0 ;adv_req_t *reqp =NULL ;int sg_cnt =0 ;
@@ -17707,10 +17706,11 @@ advansys_wide_init_chip(asc_board_t *boardp, ADV_DVC_VAR *adv_dvc_varp)* Allocate buffer carrier structures. The total size * is about 4 KB, so allocate all at once. */ - carrp =kmalloc (ADV_CARRIER_BUFSIZE ,GFP_KERNEL ); -ASC_DBG1 (1 ,"advansys_wide_init_chip: carrp 0x%lx\n" , (ulong ) carrp); + boardp->orig_carrp =kmalloc (ADV_CARRIER_BUFSIZE ,GFP_KERNEL ); +ASC_DBG1 (1 ,"advansys_wide_init_chip: carrp 0x%p\n" , + boardp->orig_carrp); -if (!carrp) +if (!boardp->orig_carrp) goto kmalloc_failed;/*
@@ -17732,6 +17732,8 @@ advansys_wide_init_chip(asc_board_t *boardp, ADV_DVC_VAR *adv_dvc_varp)if (!reqp) goto kmalloc_failed; + boardp->orig_reqp = reqp; +/* * Allocate up to ADV_TOT_SG_BLOCK request structures for * the Wide board. Each structure is about 136 bytes.
@@ -17755,16 +17757,7 @@ advansys_wide_init_chip(asc_board_t *boardp, ADV_DVC_VAR *adv_dvc_varp)if (!boardp->adv_sgblkp) goto kmalloc_failed; -/* Save carrier buffer pointer. */ - boardp->orig_carrp = carrp; - -/* -* Save original pointer for kfree() in case the -* driver is built as a module and can be unloaded. -*/ - boardp->orig_reqp = reqp; - - adv_dvc_varp->carrier_buf = carrp; + adv_dvc_varp->carrier_buf = boardp->orig_carrp;/* * Point 'adv_reqp' to the request structures and
@@ -17796,24 +17789,14 @@ advansys_wide_init_chip(asc_board_t *boardp, ADV_DVC_VAR *adv_dvc_varp) " error 0x%x\n", boardp->id, warn_code, err_code); } -if (err_code ==0 ) -return 0 ; - -while ((sgp = boardp->adv_sgblkp) !=NULL ) { - boardp->adv_sgblkp = sgp->next_sgblkp; -kfree (sgp); - } - - failed: -kfree (reqp); -kfree (carrp); -return err_code; + goto exit; kmalloc_failed:ASC_PRINT1 ("advansys_wide_init_chip: board %d error: kmalloc() " "failed\n" , boardp->id); err_code =ADV_ERROR ; - goto failed; + exit: +return err_code; } static struct Scsi_Host *__devinit
@@ -17824,6 +17807,7 @@ advansys_board_found(int iop, struct device *dev, int bus_type)asc_board_t *boardp;ASC_DVC_VAR *asc_dvc_varp =NULL ;ADV_DVC_VAR *adv_dvc_varp =NULL ; +adv_sgblk_t *sgp;int share_irq = FALSE;int iolen =0 ;ADV_PADDR pci_memory_address;
@@ -17836,8 +17820,8 @@ advansys_board_found(int iop, struct device *dev, int bus_type)* Register the adapter, get its configuration, and * initialize it. */ -ASC_DBG (2 ,"advansys_board_found: scsi_register()\n" ); - shost =scsi_register (&driver_template,sizeof (asc_board_t)); +ASC_DBG (2 ,"advansys_board_found: scsi_host_alloc()\n" ); + shost =scsi_host_alloc (&advansys_template,sizeof (asc_board_t));if (!shost)return NULL ;
@@ -18373,10 +18357,8 @@ advansys_board_found(int iop, struct device *dev, int bus_type)/* BIOS start address. */ if (ASC_NARROW_BOARD (boardp)) { - shost->base = ((ulong ) -AscGetChipBiosAddress (asc_dvc_varp-> - iop_base, - asc_dvc_varp->bus_type)); + shost->base =AscGetChipBiosAddress (asc_dvc_varp->iop_base, + asc_dvc_varp->bus_type); }else {/* * Fill-in BIOS board variables. The Wide BIOS saves
@@ -18511,13 +18493,24 @@ advansys_board_found(int iop, struct device *dev, int bus_type) }if (err_code !=0 ) - goto err_free_irq; + goto err_free_wide_mem;ASC_DBG_PRT_SCSI_HOST (2 , shost); + ret =scsi_add_host (shost, dev); +if (ret) + goto err_free_wide_mem; + +scsi_scan_host (shost);return shost; - err_free_irq: + err_free_wide_mem: +kfree (boardp->orig_carrp); +kfree (boardp->orig_reqp); +while ((sgp = boardp->adv_sgblkp) !=NULL ) { + boardp->adv_sgblkp = sgp->next_sgblkp; +kfree (sgp); + }free_irq (shost->irq, boardp); err_free_dma:if (shost->dma_channel !=NO_ISA_DMA )
@@ -18530,7 +18523,7 @@ advansys_board_found(int iop, struct device *dev, int bus_type)if (boardp->ioremap_addr)iounmap (boardp->ioremap_addr); err_shost: -scsi_unregister (shost); +scsi_host_put (shost); asc_board_count--;return NULL ; }
@@ -18758,7 +18751,7 @@ static int advansys_release(struct Scsi_Host *shost)ASC_ASSERT (boardp->prtbuf !=NULL );kfree (boardp->prtbuf);#endif /* CONFIG_PROC_FS */ -scsi_unregister (shost); +scsi_host_put (shost);ASC_DBG (1 ,"advansys_release: end\n" );return 0 ; }
Signature · 1 line
1.4.4.4
Re: [PATCH 6/21] advansys: Move to scsi hotplug initialisation model
Quoted 6 lines · Matthew Wilcox
- Switch from scsi_register/scsi_unregister to
scsi_host_alloc,scsi_add_host,scsi_scan_hostandscsi_host_put.
- Rename thescsi_host_templatetoadvansys_template
- Move the memory freeing on error fromadvansys_wide_init_chip()back intoadvansys_board_found()as we need to free it ifscsi_add_host()returns an error.
If you switch to new style allocation please also remove ->detect and ->release. We don't really want half-conversions to the new model.
Re: [PATCH 6/21] advansys: Move to scsi hotplug initialisation model
Quoted 10 lines · Christoph Hellwig
Quoted 6 lines · Matthew Wilcox
- Switch from scsi_register/scsi_unregister to
scsi_host_alloc,scsi_add_host,scsi_scan_hostandscsi_host_put.
- Rename thescsi_host_templatetoadvansys_template
- Move the memory freeing on error fromadvansys_wide_init_chip()back intoadvansys_board_found()as we need to free it ifscsi_add_host()returns an error.If you switch to new style allocation please also remove ->detect and ->release. We don't really want half-conversions to the new model.
That happened in patch 4. The function that looks for ISA and VL is still called advansys_detect, but I can rename it to advansys_probe_legacy if you want.
Signature · 3 lines
"Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Re: [PATCH 6/21] advansys: Move to scsi hotplug initialisation model
Quoted 3 lines · Matthew Wilcox
That happened in patch 4. The function that looks for ISA and VL is still called
advansys_detect, but I can rename it toadvansys_probe_legacyif you want.
Um. That's patch 4/22 ... or patch 3/21. I knew I should have used git-format-patch's --start-number option. This patch, anyway:
http://www.kernel.org/pub/linux/kernel/people/willy/advansys-2007-07-26/0004-advansys-Convert-to-pci_register_driver-interface.txt
Signature · 3 lines
"Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Re: [PATCH 6/21] advansys: Move to scsi hotplug initialisation model
Quoted 7 lines · Matthew Wilcox
Quoted 3 lines · Matthew Wilcox
That happened in patch 4. The function that looks for ISA and VL is still called
advansys_detect, but I can rename it toadvansys_probe_legacyif you want.git-format-patch's --start-number option. This patch, anyway:
http://www.kernel.org/pub/linux/kernel/people/willy/advansys-2007-07-26/0004-advansys-Convert-to-pci_register_driver-interface.txt
In that case, reverse what I said: the above pci_register_driver commit breaks non-PCI probing.
advansys_detect/release populate legacy list, but nothing ever happens from there... your "Move to scsi hotplug initialisation model" patch fixes this bug, I presume.
Jeff
Re: [PATCH 6/21] advansys: Move to scsi hotplug initialisation model
Quoted 2 lines · Jeff Garzik
In that case, reverse what I said: the above
pci_register_drivercommit breaks non-PCI probing.
Ah yes. Fair point.
Quoted 3 lines
advansys_detect/release populate legacy list, but nothing ever happens from there... your "Move to scsi hotplug initialisation model" patch fixes this bug, I presume.
Looks like it. I don't know of any testers with non-PCI cards though.
Anyone out there with a non-PCI advansys card want to do some testing for me?
Signature · 3 lines
"Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Re: [PATCH 6/21] advansys: Move to scsi hotplug initialisation model
In reply to Matthew WilcoxQuoted 8 lines · Matthew Wilcox
Quoted 7 lines · Christoph Hellwig
Quoted 6 lines · Matthew Wilcox
- Switch from scsi_register/scsi_unregister to
scsi_host_alloc,scsi_add_host,scsi_scan_hostandscsi_host_put.
- Rename thescsi_host_templatetoadvansys_template
- Move the memory freeing on error fromadvansys_wide_init_chip()back intoadvansys_board_found()as we need to free it ifscsi_add_host()returns an error.
Quoted 4 lines
Quoted 2 lines
If you switch to new style allocation please also remove ->detect and ->release. We don't really want half-conversions to the new model.
That happened in patch 4.
Incorrect. It happened in "convert to pci_register_driver interface", 42a06cfdd19a436047fef419d9b621c917118e3c
And Christoph is right -- ->detect and ->release should disappear in the same commit that you upgrade to the new scsi host model -- and stop including scsi_module.c -- otherwise your patch is incomplete (and broken WRT git-bisect producing a working driver at each point).
Look at the relevant code in scsi_module.c:
sht->detect(sht);
if (list_empty(&sht->legacy_hosts))
return -ENODEV;If you are not using the old scsi_register() stuff, you are not populating sht->legacy_hosts, and your module load will fail.
Jeff
Re: [PATCH 6/21] advansys: Move to scsi hotplug initialisation model
In reply to Matthew WilcoxQuoted 17 lines · Matthew Wilcox
drivers/scsi/advansys.c +6
-2@@ -18511,13 +18493,24 @@ advansys_board_found(int iop, struct device *dev, int bus_type) }if (err_code !=0 ) - goto err_free_irq; + goto err_free_wide_mem;ASC_DBG_PRT_SCSI_HOST (2 , shost); + ret =scsi_add_host (shost, dev); +if (ret) + goto err_free_wide_mem; + +scsi_scan_host (shost);return shost; - err_free_irq:
drivers/scsi/advansys.c advansys_board_found(int iop, struct device *dev, int bus_type) · line 18516
Quoted 10 lines
drivers/scsi/advansys.c +7
-0+ err_free_wide_mem: +kfree (boardp->orig_carrp); +kfree (boardp->orig_reqp); +while ((sgp = boardp->adv_sgblkp) !=NULL ) { + boardp->adv_sgblkp = sgp->next_sgblkp; +kfree (sgp); + }free_irq (shost->irq, boardp); err_free_dma:if (shost->dma_channel !=NO_ISA_DMA )
When this patch is considered in context of replies to patch #3, it's OK... it un-breaks driver which was broken in patch #3.
But like I noted, this should be done -before- PCI hotplug conversion, for the obvious reasons.
Generally the process involves:
* call foo_detect from module init
* for each scsi host, call foo_release from module exit
* do the scsi conversion as you've presented it here
and then in later patches, rip out the PCI code from foo_detect and foo_release, etc.
Jeff
PATCH 7/21 advansys: Convert to EISA driver model
In reply to Matthew Wilcox - Switch the EISA probing to the driver model.
- Factor out the guts of advansys_pci_remove() so that advansys_eisa_remove() can use it.
- Update the FIXME now that we use the correct driver model probing API
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 188 +++++++++++++++++++++++------------------------ 1 files changed, 93 insertions(+), 95 deletions(-)
drivers/scsi/advansys.c +93 -95
@@ -769,6 +769,7 @@#include <linux/proc_fs.h> #include <linux/init.h> #include <linux/blkdev.h> +#include <linux/eisa.h> #include <linux/pci.h> #include <linux/spinlock.h> #include <linux/dma-mapping.h>
@@ -783,13 +784,9 @@#include <scsi/scsi.h> #include <scsi/scsi_host.h> -/* FIXME: (by [email protected]) This warning is present for two -* reasons: +/* FIXME: (by [email protected]) * -* 1) This driver badly needs converting to the correct driver model -* probing API -* -* 2) Although all of the necessary command mapping places have the +* Although all of the necessary command mapping places have the * appropriate dma_map.. APIs, the driver still processes its internal * queue using bus_to_virt() and virt_to_bus() which are illegal under * the API. The entire queue processing structure will need to be
@@ -1787,16 +1784,10 @@ typedef struct asceep_config {#define ASC_1000_ID0W 0x04C1#define ASC_1000_ID0W_FIX 0x00C1#define ASC_1000_ID1B 0x25 -#define ASC_EISA_BIG_IOP_GAP (0x1C30-0x0C50) -#define ASC_EISA_SMALL_IOP_GAP (0x0020) -#define ASC_EISA_MIN_IOP_ADDR (0x0C30) -#define ASC_EISA_MAX_IOP_ADDR (0xFC50)#define ASC_EISA_REV_IOP_MASK (0x0C83)#define ASC_EISA_PID_IOP_MASK (0x0C80)#define ASC_EISA_CFG_IOP_MASK (0x0C86)#define ASC_GET_EISA_SLOT (iop) (PortAddr)((iop) & 0xF000) -#define ASC_EISA_ID_740 0x01745004UL -#define ASC_EISA_ID_750 0x01755004UL#define INS_HALTINT (ushort)0x6281#define INS_HALT (ushort)0x6280#define INS_SINT (ushort)0x6200
@@ -1943,8 +1934,6 @@ static int AscIsrQDone(ASC_DVC_VAR *);static int AscCompareString (uchar *,uchar *,int );#ifdef CONFIG_ISA static ushort AscGetEisaChipCfg (PortAddr ); -static ASC_DCNT AscGetEisaProductID (PortAddr ); -static PortAddr AscSearchIOPortAddrEISA (PortAddr );static PortAddr AscSearchIOPortAddr11 (PortAddr );static PortAddr AscSearchIOPortAddr (PortAddr ,ushort );static void AscSetISAPNPWaitForKey (void );
@@ -3418,7 +3407,7 @@ typedef struct {#define ASC_NUM_BOARD_SUPPORTED 16#define ASC_NUM_IOPORT_PROBE 4 -#define ASC_NUM_BUS 3 +#define ASC_NUM_BUS 2/* Reference Scsi_Host hostdata */ #define ASC_BOARDP (host) ((asc_board_t *) &((host)->hostdata))
@@ -3869,7 +3858,6 @@ static ASC_SG_HEAD asc_sg_head = { 0 };static ushort asc_bus[ASC_NUM_BUS ]__initdata = {ASC_IS_ISA ,ASC_IS_VL , -ASC_IS_EISA , };static int asc_iopflag =ASC_FALSE ;
@@ -3879,7 +3867,6 @@ static int asc_ioport[ASC_NUM_IOPORT_PROBE] = { 0, 0, 0, 0 };static char *asc_bus_name[ASC_NUM_BUS ] = {"ASC_IS_ISA" ,"ASC_IS_VL" , -"ASC_IS_EISA" , };static int asc_dbglvl =3 ;
@@ -8300,12 +8287,6 @@ static PortAddr __init AscSearchIOPortAddr(PortAddr iop_beg, ushort bus_type) }return (0 ); } -if (bus_type &ASC_IS_EISA ) { -if ((iop_beg =AscSearchIOPortAddrEISA (iop_beg)) !=0 ) { -return (iop_beg); - } -return (0 ); - }return (0 ); }
@@ -10315,57 +10296,6 @@ static void DvcDelayNanoSecond(ASC_DVC_VAR *asc_dvc, ASC_DCNT nano_sec)udelay ((nano_sec +999 ) /1000 ); } -#ifdef CONFIG_ISA -static ASC_DCNT __init AscGetEisaProductID (PortAddr iop_base) -{ -PortAddr eisa_iop; -ushort product_id_high, product_id_low; -ASC_DCNT product_id; - - eisa_iop =ASC_GET_EISA_SLOT (iop_base) |ASC_EISA_PID_IOP_MASK ; - product_id_low =inpw (eisa_iop); - product_id_high =inpw (eisa_iop +2 ); - product_id = ((ASC_DCNT ) product_id_high <<16 ) | - (ASC_DCNT ) product_id_low; -return (product_id); -} - -static PortAddr __init AscSearchIOPortAddrEISA (PortAddr iop_base) -{ -ASC_DCNT eisa_product_id; - -if (iop_base ==0 ) { - iop_base =ASC_EISA_MIN_IOP_ADDR ; - }else { -if (iop_base ==ASC_EISA_MAX_IOP_ADDR ) -return (0 ); -if ((iop_base &0x0050 ) ==0x0050 ) { - iop_base +=ASC_EISA_BIG_IOP_GAP ; - }else { - iop_base +=ASC_EISA_SMALL_IOP_GAP ; - } - } -while (iop_base <=ASC_EISA_MAX_IOP_ADDR ) { - eisa_product_id =AscGetEisaProductID (iop_base); -if ((eisa_product_id ==ASC_EISA_ID_740 ) || - (eisa_product_id ==ASC_EISA_ID_750 )) { -if (AscFindSignature (iop_base)) { -inpw (iop_base +4 ); -return (iop_base); - } - } -if (iop_base ==ASC_EISA_MAX_IOP_ADDR ) -return (0 ); -if ((iop_base &0x0050 ) ==0x0050 ) { - iop_base +=ASC_EISA_BIG_IOP_GAP ; - }else { - iop_base +=ASC_EISA_SMALL_IOP_GAP ; - } - } -return (0 ); -} -#endif /* CONFIG_ISA */ -static int AscStartChip (PortAddr iop_base) {AscSetChipControl (iop_base,0 );
@@ -18685,12 +18615,6 @@ static int __init advansys_detect(void)#endif /* CONFIG_ISA */ break ; -case ASC_IS_EISA : -#ifdef CONFIG_ISA - iop =AscSearchIOPortAddr (iop, asc_bus[bus]); -#endif /* CONFIG_ISA */ -break ; -default :ASC_PRINT1 ("advansys_detect: unknown bus type: %d\n",
@@ -18756,6 +18680,88 @@ static int advansys_release(struct Scsi_Host *shost)return 0 ; } +static void __devexit advansys_remove(struct Scsi_Host *shost) +{ +int i; +scsi_remove_host (shost); +advansys_release (shost); + +for (i =0 ; i < asc_board_count; i++) { +if (asc_host[i] == shost) { + asc_host[i] =NULL ; +break ; + } + } +} + +static struct eisa_device_id advansys_eisa_table[]__devinitdata = { + {"ABP7401" }, + {"ABP7501" }, + {"" } +}; + +MODULE_DEVICE_TABLE (eisa, advansys_eisa_table); + +/* +* EISA is a little more tricky than PCI; each EISA device may have two +* channels, and this driver is written to make each channel its own Scsi_Host +*/ +struct eisa_scsi_data { +struct Scsi_Host *host[2 ]; +}; + +static int __devinit advansys_eisa_probe (struct device *dev) +{ +int i, ioport; +int err = -ENODEV ; +struct eisa_device *edev =to_eisa_device (dev); + +struct eisa_scsi_data *data =kzalloc (sizeof (*data),GFP_KERNEL ); + ioport = edev->base_addr +0xc30 ; + +for (i =0 ; i <2 ; i++, ioport +=0x20 ) { +if (!AscFindSignature (ioport)) +continue ; +inw (ioport +4 ); + data->host[i] =advansys_board_found (ioport, dev,ASC_IS_EISA ); +if (data->host[i]) + err =0 ; + } + +if (err) { +kfree (data); + }else { +dev_set_drvdata (dev, data); + } + +return err; +} + +static __devexit int advansys_eisa_remove(struct device *dev) +{ +int i, ioport; +struct eisa_scsi_data *data =dev_get_drvdata (dev); + +for (i =0 ; i <2 ; i++) { +struct Scsi_Host *shost = data->host[i]; +if (!shost) +continue ; + ioport = shost->io_port; +advansys_remove (data->host[i]); + } + +return 0 ; +} + +static struct eisa_driver advansys_eisa_driver = { + .id_table = advansys_eisa_table, + .driver = { + .name ="advansys" , + .probe = advansys_eisa_probe, + .remove =__devexit_p (advansys_eisa_remove), + } +}; +/* PCI Devices supported by this driver */ static struct pci_device_id advansys_pci_tbl[]__devinitdata = { {PCI_VENDOR_ID_ASP ,PCI_DEVICE_ID_ASP_1200A ,
@@ -18799,18 +18805,7 @@ advansys_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) static void __devexit advansys_pci_remove(struct pci_dev *pdev) { -int i; -struct Scsi_Host *shost =pci_get_drvdata (pdev); -scsi_remove_host (shost); -advansys_release (shost); - -for (i =0 ; i < asc_board_count; i++) { -if (asc_host[i] == shost) { - asc_host[i] =NULL ; -break ; - } - } - +advansys_remove (pci_get_drvdata (pdev));pci_disable_device (pdev); }
@@ -18825,11 +18820,13 @@ static int __init advansys_init(void) {int count, error; count =advansys_detect (); - error =pci_register_driver (&advansys_pci_driver); + error =eisa_driver_register (&advansys_eisa_driver); +if (!error) + error =pci_register_driver (&advansys_pci_driver);/* -* If we found some ISA, EISA or VLB devices, we must not fail. -* We may not drive any PCI devices, but it's better to drive +* If we found some ISA or VLB devices, we must not fail. +* We may not drive any EISA or PCI devices, but it's better to drive * the cards that we successfully discovered than none at all. */ if (count >0 )
@@ -18842,6 +18839,7 @@ static void __exit advansys_exit(void)int i;pci_unregister_driver (&advansys_pci_driver); +eisa_driver_unregister (&advansys_eisa_driver);for (i =0 ; i < asc_board_count; i++) {struct Scsi_Host *host = asc_host[i];
Signature · 1 line
1.4.4.4
Re: [PATCH 7/21] advansys: Convert to EISA driver model
drivers/scsi/advansys.c static int advansys_release(struct Scsi_Host *shost) · line 18695
Quoted 13 lines · Matthew Wilcox
drivers/scsi/advansys.c +13
-0+static void __devexit advansys_remove(struct Scsi_Host *shost) +{ +int i; +scsi_remove_host (shost); +advansys_release (shost); + +for (i =0 ; i < asc_board_count; i++) { +if (asc_host[i] == shost) { + asc_host[i] =NULL ; +break ; + } + } +}
1) IMO this should be in the PCI patch
drivers/scsi/advansys.c static int advansys_release(struct Scsi_Host *shost) · line 18719
Quoted 7 lines
drivers/scsi/advansys.c +7
-0+static int __devinit advansys_eisa_probe (struct device *dev) +{ +int i, ioport; +int err = -ENODEV ; +struct eisa_device *edev =to_eisa_device (dev); + +struct eisa_scsi_data *data =kzalloc (sizeof (*data),GFP_KERNEL );
nits:
2) check for NULL
3) maybe it's just me, but the mixing initializers and allocation code is way too close to C99/C++ code/decl mixing. I would declare 'data', then initialize it on an separate line. But again, maybe that's just me.
drivers/scsi/advansys.c static int advansys_release(struct Scsi_Host *shost) · line 18725
Quoted 6 lines
drivers/scsi/advansys.c +6
-0+ ioport = edev->base_addr +0xc30 ; + +for (i =0 ; i <2 ; i++, ioport +=0x20 ) { +if (!AscFindSignature (ioport)) +continue ; +inw (ioport +4 );
4) would be nice to have a comment noting what this inw() does.
5) I would suggest putting a "remove inpw/outpw pointless wrappers" cleanup patch before patches #2 .. #N.
Quoted 18 lines
drivers/scsi/advansys.c +18
-0+ data->host[i] =advansys_board_found (ioport, dev,ASC_IS_EISA ); +if (data->host[i]) + err =0 ; + } + +if (err) { +kfree (data); + }else { +dev_set_drvdata (dev, data); + } + +return err; +} + +static __devexit int advansys_eisa_remove(struct device *dev) +{ +int i, ioport; +struct eisa_scsi_data *data =dev_get_drvdata (dev);
drivers/scsi/advansys.c static int advansys_release(struct Scsi_Host *shost) · line 18753
Quoted 10 lines
drivers/scsi/advansys.c +10
-0+ +for (i =0 ; i <2 ; i++) { +struct Scsi_Host *shost = data->host[i]; +if (!shost) +continue ; + ioport = shost->io_port; +advansys_remove (data->host[i]); + } + +return 0 ;
6) set drvdata to NULL
drivers/scsi/advansys.c static int advansys_release(struct Scsi_Host *shost) · line 18762
Quoted 7 lines
drivers/scsi/advansys.c +7
-0+static struct eisa_driver advansys_eisa_driver = { + .id_table = advansys_eisa_table, + .driver = { + .name ="advansys" , + .probe = advansys_eisa_probe, + .remove =__devexit_p (advansys_eisa_remove), + }
7) values much more readable when tab-aligned
Re: [PATCH 7/21] advansys: Convert to EISA driver model
Quoted 16 lines · Jeff Garzik
drivers/scsi/advansys.c static int advansys_release(struct Scsi_Host *shost) · line 18695
Quoted 13 lines · Matthew Wilcox
drivers/scsi/advansys.c +13
-0+static void __devexit advansys_remove(struct Scsi_Host *shost) +{ +int i; +scsi_remove_host (shost); +advansys_release (shost); + +for (i =0 ; i < asc_board_count; i++) { +if (asc_host[i] == shost) { + asc_host[i] =NULL ; +break ; + } + } +}1) IMO this should be in the PCI patch
I suppose so.
Quoted 11 lines
drivers/scsi/advansys.c static int advansys_release(struct Scsi_Host *shost) · line 18719
Quoted 7 lines
drivers/scsi/advansys.c +7
-0+static int __devinit advansys_eisa_probe (struct device *dev) +{ +int i, ioport; +int err = -ENODEV ; +struct eisa_device *edev =to_eisa_device (dev); + +struct eisa_scsi_data *data =kzalloc (sizeof (*data),GFP_KERNEL );nits:
2) check for NULL
OK. I don't think an 8-byte allocation can actually fail, but it's good style.
Quoted 8 lines
drivers/scsi/advansys.c static int advansys_release(struct Scsi_Host *shost) · line 18725
Quoted 6 lines
drivers/scsi/advansys.c +6
-0+ ioport = edev->base_addr +0xc30 ; + +for (i =0 ; i <2 ; i++, ioport +=0x20 ) { +if (!AscFindSignature (ioport)) +continue ; +inw (ioport +4 );4) would be nice to have a comment noting what this
inw()does.
Yeah, it would. I just copied it from the previous code:
while (iop_base <= ASC_EISA_MAX_IOP_ADDR) {
eisa_product_id = AscGetEisaProductID(iop_base);
if ((eisa_product_id == ASC_EISA_ID_740) ||
(eisa_product_id == ASC_EISA_ID_750)) {
if (AscFindSignature(iop_base)) {
inpw(iop_base + 4);
return (iop_base);
}
}Anyone got a manual and can tell me why we do this? ;-)
Would it make sense to put in a comment that says "No idea why we do this"?
Quoted 2 lines
5) I would suggest putting a "remove inpw/outpw pointless wrappers" cleanup patch before patches #2 .. #N.
Next thing on my todo list is to convert the driver over to ioread8 and
friends. Currently, only the wide boards get accessed through PCIMEM,
but the narrow boards also have PCIMEM BARs. It would also allow the wide boards to be used on machines where PCIMEM is broken, like the VisWS.
Quoted 8 lines
Quoted 8 lines
drivers/scsi/advansys.c +8
-0+static __devexit int advansys_eisa_remove(struct device *dev) +{ +int i, ioport; +struct eisa_scsi_data *data =dev_get_drvdata (dev); + +for (i =0 ; i <2 ; i++) { +struct Scsi_Host *shost = data->host[i]; +if (!shost)
Quoted 8 lines
drivers/scsi/advansys.c static int advansys_release(struct Scsi_Host *shost) · line 18753
Quoted 6 lines
drivers/scsi/advansys.c +6
-0+continue ; + ioport = shost->io_port; +advansys_remove (data->host[i]); + } + +return 0 ;6) set drvdata to NULL
Any reason why?
Oh, I seem to have forgotten the kfree() here too. I'll fix that.
Quoted 1 line
7) values much more readable when tab-aligned
ok.
Signature · 3 lines
"Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Re: [PATCH 7/21] advansys: Convert to EISA driver model
Quoted 21 lines · Matthew Wilcox
Quoted 7 lines · Jeff Garzik
drivers/scsi/advansys.c static int advansys_release(struct Scsi_Host *shost) · line 18725
Quoted 6 lines
drivers/scsi/advansys.c +6
-0+ ioport = edev->base_addr +0xc30 ; + +for (i =0 ; i <2 ; i++, ioport +=0x20 ) { +if (!AscFindSignature (ioport)) +continue ; +inw (ioport +4 );4) would be nice to have a comment noting what this
inw()does.Yeah, it would. I just copied it from the previous code:
while (iop_base <= ASC_EISA_MAX_IOP_ADDR) { eisa_product_id = AscGetEisaProductID(iop_base); if ((eisa_product_id == ASC_EISA_ID_740) || (eisa_product_id == ASC_EISA_ID_750)) { if (AscFindSignature(iop_base)) { inpw(iop_base + 4); return (iop_base); } }
Quoted 3 lines
Anyone got a manual and can tell me why we do this? ;-)
Would it make sense to put in a comment that says "No idea why we do this"?
IMO, yes. It should be noticeable at the very least.
Quoted 5 lines
Quoted 1 line
5) I would suggest putting a "remove inpw/outpw pointless wrappers" cleanup patch before patches #2 .. #N.
Next thing on my todo list is to convert the driver over to ioread8 and
friends. Currently, only the wide boards get accessed through PCIMEM,
but the narrow boards also have PCIMEM BARs. It would also allow the wide boards to be used on machines where PCIMEM is broken, like the VisWS.
PCIMEM versus PCIMEM... huh?
Quoted 16 lines
Quoted 7 lines
Quoted 7 lines
drivers/scsi/advansys.c +7
-0+static __devexit int advansys_eisa_remove(struct device *dev) +{ +int i, ioport; +struct eisa_scsi_data *data =dev_get_drvdata (dev); + +for (i =0 ; i <2 ; i++) { +struct Scsi_Host *shost = data->host[i];Quoted 8 lines
drivers/scsi/advansys.c static int advansys_release(struct Scsi_Host *shost) · line 18753
Quoted 7 lines
drivers/scsi/advansys.c +7
-0+if (!shost) +continue ; + ioport = shost->io_port; +advansys_remove (data->host[i]); + } + +return 0 ;6) set drvdata to NULL
Quoted 1 line
Any reason why?
See other email. Safe object lifetime practice, plus it ensures a nice oops if there is a bug.
Setting drvdata to NULL in ->remove() is standard for Linux drivers. grep around.
Jeff
Re: [PATCH 7/21] advansys: Convert to EISA driver model
Quoted 3 lines · Jeff Garzik
Quoted 1 line
Would it make sense to put in a comment that says "No idea why we do this"?
IMO, yes. It should be noticeable at the very least.
Added to todo for next spin of these patches.
Quoted 9 lines
Quoted 7 lines
Quoted 2 lines
5) I would suggest putting a "remove inpw/outpw pointless wrappers" cleanup patch before patches #2 .. #N.
Next thing on my todo list is to convert the driver over to ioread8 and friends. Currently, only the wide boards get accessed through PCIMEM, but the narrow boards also have PCIMEM BARs. It would also allow the wide boards to be used on machines where PCIMEM is broken, like the VisWS.
PCIMEM versus PCIMEM... huh?
This driver is really two drivers in one -- the wide boards and the narrow boards. Christoph is working on a new driver for the wide boards only, and once he's finished that, I'll delete the support for the wide boards in this driver.
Right now, only the wide boards get to take advantage of PCIMEM. I want to give the narrow boards (like the one I have) the opportunity to take advantage of PCIMEM, if they have it. There's also a pending todo for wide boards to fall back to ioports if PCIMEM doesn't work.
If that doesn't answer your question, I need something more detailed than "huh?" ;-)
Signature · 3 lines
"Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Re: [PATCH 7/21] advansys: Convert to EISA driver model
Quoted 17 lines · Matthew Wilcox
Quoted 2 lines · Jeff Garzik
Quoted 1 line
Would it make sense to put in a comment that says "No idea why we do this"?
IMO, yes. It should be noticeable at the very least.
Added to todo for next spin of these patches.
Quoted 5 lines
Quoted 4 lines
Quoted 1 line
5) I would suggest putting a "remove inpw/outpw pointless wrappers" cleanup patch before patches #2 .. #N.
Next thing on my todo list is to convert the driver over to ioread8 and
friends. Currently, only the wide boards get accessed through PCIMEM,
but the narrow boards also have PCIMEM BARs. It would also allow the wide boards to be used on machines where PCIMEM is broken, like the VisWS.PCIMEM versus PCIMEM... huh?
This driver is really two drivers in one -- the wide boards and the narrow boards. Christoph is working on a new driver for the wide boards only, and once he's finished that, I'll delete the support for the wide boards in this driver.
Quoted 7 lines
Right now, only the wide boards get to take advantage of PCIMEM. I want to give the narrow boards (like the one I have) the opportunity to take advantage of PCIMEM, if they have it. There's also a pending todo for wide boards to fall back to ioports if PCIMEM doesn't work.
If that doesn't answer your question, I need something more detailed than "huh?" ;-)
Your earlier statement was just confusing. You basically said "if PCIMEM doesn't work, we should fall back to PCIMEM" if I read it correctly.
I think you meant falling back to ioports, but wanted to clarify.
Jeff
PATCH 8/21 advansys: Rewrite resource management
In reply to Matthew WilcoxMake sure the resources are reserved and released by all the callers of advansys_board_found(). This eliminates the check_region-style race. It also allows us to use the pci_request_regions() API.
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 103 +++++++++++++++++++--------------------------- 1 files changed, 43 insertions(+), 60 deletions(-)
drivers/scsi/advansys.c +43 -60
@@ -1935,7 +1935,6 @@ static int AscCompareString(uchar *, uchar *, int);#ifdef CONFIG_ISA static ushort AscGetEisaChipCfg (PortAddr );static PortAddr AscSearchIOPortAddr11 (PortAddr ); -static PortAddr AscSearchIOPortAddr (PortAddr ,ushort );static void AscSetISAPNPWaitForKey (void );#endif /* CONFIG_ISA */ static uchar AscGetChipScsiCtrl (PortAddr );
@@ -8261,7 +8260,7 @@ static PortAddr _asc_def_iop_base[ASC_IOADR_TABLE_MAX_IX] __initdata = { };#ifdef CONFIG_ISA -static uchar _isa_pnp_inited__initdata =0 ; +static unsigned char _isa_pnp_inited__initdata =0 ;static PortAddr __init AscSearchIOPortAddr (PortAddr iop_beg,ushort bus_type) {
@@ -8269,12 +8268,11 @@ static PortAddr __init AscSearchIOPortAddr(PortAddr iop_beg, ushort bus_type)while ((iop_beg =AscSearchIOPortAddr11 (iop_beg)) !=0 ) {if (AscGetChipVersion (iop_beg, bus_type) <=ASC_CHIP_MAX_VER_VL ) { -return (iop_beg); +return iop_beg; } +release_region (iop_beg,ASC_IOADR_GAP ); } -return (0 ); - } -if (bus_type &ASC_IS_ISA ) { + }else if (bus_type &ASC_IS_ISA ) {if (_isa_pnp_inited ==0 ) {AscSetISAPNPWaitForKey (); _isa_pnp_inited++;
@@ -8282,40 +8280,33 @@ static PortAddr __init AscSearchIOPortAddr(PortAddr iop_beg, ushort bus_type)while ((iop_beg =AscSearchIOPortAddr11 (iop_beg)) !=0 ) {if ((AscGetChipVersion (iop_beg, bus_type) &ASC_CHIP_VER_ISA_BIT ) !=0 ) { -return (iop_beg); +return iop_beg; } +release_region (iop_beg,ASC_IOADR_GAP ); } -return (0 ); } -return (0 ); +return 0 ; }static PortAddr __init AscSearchIOPortAddr11 (PortAddr s_addr) {int i; -PortAddr iop_base;for (i =0 ; i <ASC_IOADR_TABLE_MAX_IX ; i++) { -if (_asc_def_iop_base[i] > s_addr) { -break ; - } - } -for (; i <ASC_IOADR_TABLE_MAX_IX ; i++) { - iop_base = _asc_def_iop_base[i]; +PortAddr iop_base = _asc_def_iop_base[i]; +if (iop_base <= s_addr) +continue ;if (!request_region (iop_base,ASC_IOADR_GAP ,"advansys" )) { -ASC_DBG1 (1 , -"AscSearchIOPortAddr11: check_region() failed I/O port 0x%x\n" , - iop_base); +ASC_DBG1 (1 ,"AscSearchIOPortAddr11: I/O port 0x%x " +"in use\n" , iop_base);continue ; }ASC_DBG1 (1 ,"AscSearchIOPortAddr11: probing I/O port 0x%x\n" , iop_base); -release_region (iop_base,ASC_IOADR_GAP ); -if (AscFindSignature (iop_base)) { -return (iop_base); - } +if (AscFindSignature (iop_base)) +return iop_base; } -return (0 ); +return 0 ; }static void __init AscSetISAPNPWaitForKey (void )
@@ -17842,14 +17833,12 @@ advansys_board_found(int iop, struct device *dev, int bus_type) boardp->id, pci_memory_address, iolen); goto err_shost; } -ASC_DBG1 (1 , -"advansys_board_found: ioremap_addr: 0x%lx\n" , +ASC_DBG1 (1 ,"advansys_board_found: ioremap_addr: 0x%lx\n" , (ulong )boardp->ioremap_addr); adv_dvc_varp->iop_base = (AdvPortAddr) (boardp->ioremap_addr + (pci_memory_address - (pci_memory_address &PAGE_MASK ))); -ASC_DBG1 (1 , -"advansys_board_found: iop_base: 0x%lx\n" , +ASC_DBG1 (1 ,"advansys_board_found: iop_base: 0x%lx\n" , adv_dvc_varp->iop_base);#endif /* CONFIG_PCI */
@@ -18330,25 +18319,6 @@ advansys_board_found(int iop, struct device *dev, int bus_type)* Register Board Resources - I/O Port, DMA, IRQ */ -/* -* Register I/O port range. -* -* For Wide boards the I/O ports are not used to access -* the board, but request the region anyway. -* -* 'shost->n_io_port' is not referenced, because it may be truncated. -*/ -ASC_DBG2 (2 , -"advansys_board_found: request_region port 0x%lx, len 0x%x\n" , - (ulong )shost->io_port, boardp->asc_n_io_port); -if (request_region (shost->io_port, boardp->asc_n_io_port, -"advansys" ) ==NULL ) { -ASC_PRINT3 - ("advansys_board_found: board %d: request_region() failed, port 0x%lx, len 0x%x\n" , - boardp->id, (ulong )shost->io_port, boardp->asc_n_io_port); - goto err_free_proc; - } -/* Register DMA Channel for Narrow boards. */ shost->dma_channel =NO_ISA_DMA ;/* Default to no ISA DMA. */ #ifdef CONFIG_ISA
@@ -18361,7 +18331,7 @@ advansys_board_found(int iop, struct device *dev, int bus_type)ASC_PRINT3 ("advansys_board_found: board %d: request_dma() %d failed %d\n" , boardp->id, shost->dma_channel, ret); - goto err_free_region; + goto err_free_proc; }AscEnableIsaDma (shost->dma_channel); }
@@ -18445,8 +18415,6 @@ advansys_board_found(int iop, struct device *dev, int bus_type) err_free_dma:if (shost->dma_channel !=NO_ISA_DMA )free_dma (shost->dma_channel); - err_free_region: -release_region (shost->io_port, boardp->asc_n_io_port); err_free_proc:kfree (boardp->prtbuf); err_unmap:
@@ -18474,9 +18442,8 @@ advansys_board_found(int iop, struct device *dev, int bus_type)static int __init advansys_detect (void ) {static int detect_called =ASC_FALSE ; -int iop; -int bus; -int ioport =0 ; +int iop, bus, ioport =0 ; +struct Scsi_Host *shost;if (detect_called ==ASC_FALSE ) { detect_called =ASC_TRUE ;
@@ -18533,8 +18500,7 @@ static int __init advansys_detect(void)case ASC_IS_VL :#ifdef CONFIG_ISA if (asc_iopflag ==ASC_FALSE ) { - iop = -AscSearchIOPortAddr (iop, + iop =AscSearchIOPortAddr (iop, asc_bus[bus]); }else {/*
@@ -18630,7 +18596,9 @@ static int __init advansys_detect(void)break ; } -advansys_board_found (iop,NULL , asc_bus[bus]); + shost =advansys_board_found (iop,NULL , asc_bus[bus]); +if (!shost) +release_region (iop,ASC_IOADR_GAP ); } }
@@ -18655,7 +18623,6 @@ static int advansys_release(struct Scsi_Host *shost)ASC_DBG (1 ,"advansys_release: free_dma()\n" );free_dma (shost->dma_channel); } -release_region (shost->io_port, boardp->asc_n_io_port);if (ASC_WIDE_BOARD (boardp)) {adv_sgblk_t *sgp =NULL ;
@@ -18720,12 +18687,22 @@ static int __devinit advansys_eisa_probe(struct device *dev) ioport = edev->base_addr +0xc30 ;for (i =0 ; i <2 ; i++, ioport +=0x20 ) { -if (!AscFindSignature (ioport)) +if (!request_region (ioport,ASC_IOADR_GAP ,"advansys" )) { +printk (KERN_WARNING "Region %x-%x busy\n" , ioport, + ioport +ASC_IOADR_GAP -1 ); +continue ; + } +if (!AscFindSignature (ioport)) { +release_region (ioport,ASC_IOADR_GAP );continue ; + }inw (ioport +4 ); data->host[i] =advansys_board_found (ioport, dev,ASC_IS_EISA ); -if (data->host[i]) +if (data->host[i]) { err =0 ; + }else { +release_region (ioport,ASC_IOADR_GAP ); + } }if (err) {
@@ -18748,6 +18725,7 @@ static __devexit int advansys_eisa_remove(struct device *dev)continue ; ioport = shost->io_port;advansys_remove (data->host[i]); +release_region (ioport,ASC_IOADR_GAP ); }return 0 ;
@@ -18787,8 +18765,10 @@ advansys_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)int ioport;struct Scsi_Host *shost; -if (pci_enable_device (pdev)) +if (pci_request_regions (pdev,"advansys" )) goto fail; +if (pci_enable_device (pdev)) + goto release_regions; ioport =pci_resource_start (pdev,0 ); shost =advansys_board_found (ioport, &pdev->dev,ASC_IS_PCI );
@@ -18799,6 +18779,8 @@ advansys_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) }pci_disable_device (pdev); + release_regions: +pci_release_regions (pdev); fail:return -ENODEV ; }
@@ -18807,6 +18789,7 @@ static void __devexit advansys_pci_remove(struct pci_dev *pdev) {advansys_remove (pci_get_drvdata (pdev));pci_disable_device (pdev); +pci_release_regions (pdev); }static struct pci_driver advansys_pci_driver = {
Signature · 1 line
1.4.4.4
Re: [PATCH 8/21] advansys: Rewrite resource management
Quoted 22 lines · Matthew Wilcox
drivers/scsi/advansys.c +5
-1@@ -18787,8 +18765,10 @@ advansys_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)int ioport;struct Scsi_Host *shost; -if (pci_enable_device (pdev)) +if (pci_request_regions (pdev,"advansys" )) goto fail; +if (pci_enable_device (pdev)) + goto release_regions; ioport =pci_resource_start (pdev,0 ); shost =advansys_board_found (ioport, &pdev->dev,ASC_IS_PCI );@@ -18799,6 +18779,8 @@ advansys_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) }pci_disable_device (pdev); + release_regions: +pci_release_regions (pdev); fail:return -ENODEV ; }
drivers/scsi/advansys.c static void __devexit advansys_pci_remove(struct pci_dev *pdev) · line 18792
Quoted 4 lines
drivers/scsi/advansys.c +1
-0@@ -18807,6 +18789,7 @@ static void __devexit advansys_pci_remove(struct pci_dev *pdev) {advansys_remove (pci_get_drvdata (pdev));pci_disable_device (pdev); +pci_release_regions (pdev);
Your ordering here is completely backwards.
You do not have resources to reserve, until you have enabled the device.
pci_enable_device() ALWAYS comes first, and pci_disable_device() always comes next-to-last (pci_set_drvdata(pdev, NULL) is last).
Jeff
PATCH 9/21 advansys: More PCI cleanups
In reply to Matthew Wilcox - Remove wrappers around the PCI configuration space accessors
- Call pci_set_master() instead of poking at config space directly
- Move the latency setting into one function called for both narrow and wide boards.
- Tidy up AdvInitGetConfig() a little.
- Delete a few unused prototypes and definitions.
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 254 +++++++---------------------------------------- 1 files changed, 34 insertions(+), 220 deletions(-)
drivers/scsi/advansys.c +34 -220
@@ -853,15 +853,6 @@ typedef unsigned char uchar;#define ERR (-1)#define UW_ERR (uint)(0xFFFF)#define isodd_word (val) ((((uint)val) & (uint)0x0001) != 0) -#define AscPCIConfigVendorIDRegister 0x0000 -#define AscPCIConfigDeviceIDRegister 0x0002 -#define AscPCIConfigCommandRegister 0x0004 -#define AscPCIConfigStatusRegister 0x0006 -#define AscPCIConfigRevisionIDRegister 0x0008 -#define AscPCIConfigCacheSize 0x000C -#define AscPCIConfigLatencyTimer 0x000D -#define AscPCIIOBaseRegister 0x0010 -#define AscPCICmdRegBits_IOMemBusMaster 0x0007#define ASC_PCI_ID2FUNC (id) (((id) >> 8) & 0x7)#define ASC_PCI_MKID (bus, dev, func) ((((dev) & 0x1F) << 11) | (((func) & 0x7) << 8) | ((bus) & 0xFF))
@@ -1949,17 +1940,11 @@ static uchar AscSetChipIRQ(PortAddr, uchar, ushort);static ushort AscGetChipBiosAddress (PortAddr ,ushort );static inline ulong DvcEnterCritical (void );static inline void DvcLeaveCritical (ulong ); -#ifdef CONFIG_PCI -static uchar DvcReadPCIConfigByte (ASC_DVC_VAR *,ushort ); -static void DvcWritePCIConfigByte (ASC_DVC_VAR *,ushort ,uchar ); -#endif /* CONFIG_PCI */ static ushort AscGetChipBiosAddress (PortAddr ,ushort );static void DvcSleepMilliSecond (ASC_DCNT );static void DvcDelayNanoSecond (ASC_DVC_VAR *,ASC_DCNT );static void DvcPutScsiQ (PortAddr ,ushort ,uchar *,int );static void DvcGetQinfo (PortAddr ,ushort ,uchar *,int ); -static ushort AscInitGetConfig (ASC_DVC_VAR *); -static ushort AscInitSetConfig (ASC_DVC_VAR *);static ushort AscInitAsc1000Driver (ASC_DVC_VAR *);static void AscAsyncFix (ASC_DVC_VAR *,uchar ,ASC_SCSI_INQUIRY *);static int AscTagQueuingSafe (ASC_SCSI_INQUIRY *);
@@ -3090,8 +3075,6 @@ typedef struct adv_scsi_req_q {static inline ulong DvcEnterCritical (void );static inline void DvcLeaveCritical (ulong );static void DvcSleepMilliSecond (ADV_DCNT ); -static uchar DvcAdvReadPCIConfigByte (ADV_DVC_VAR *,ushort ); -static void DvcAdvWritePCIConfigByte (ADV_DVC_VAR *,ushort ,uchar );static ADV_PADDR DvcGetPhyAddr (ADV_DVC_VAR *,ADV_SCSI_REQ_Q *,uchar *,ASC_SDCNT *,int );static void DvcDelayMicroSecond (ADV_DVC_VAR *,ushort );
@@ -3125,12 +3108,6 @@ static void AdvSet38C1600EEPConfig(AdvPortAddr, ADVEEP_38C1600_CONFIG *);static void AdvWaitEEPCmd (AdvPortAddr );static ushort AdvReadEEPWord (AdvPortAddr ,int ); -/* -* PCI Bus Definitions -*/ -#define AscPCICmdRegBits_BusMastering 0x0007 -#define AscPCICmdRegBits_ParErrRespCtrl 0x0040 -/* Read byte from a register. */ #define AdvReadByteRegister (iop_base, reg_off) \ (ADV_MEM_READB((iop_base) + (reg_off)))
@@ -7424,31 +7401,6 @@ DvcGetQinfo(PortAddr iop_base, ushort s_addr, uchar *inbuf, int words) }/* -* Read a PCI configuration byte. -*/ -static uchar __devinit DvcReadPCIConfigByte (ASC_DVC_VAR *asc_dvc,ushort offset) -{ -#ifdef CONFIG_PCI -uchar byte_data; -pci_read_config_byte (to_pci_dev (asc_dvc->cfg->dev), offset, &byte_data); -return byte_data; -#else /* !defined(CONFIG_PCI) */ -return 0 ; -#endif /* !defined(CONFIG_PCI) */ -} - -/* -* Write a PCI configuration byte. -*/ -static void __devinit -DvcWritePCIConfigByte (ASC_DVC_VAR *asc_dvc,ushort offset,uchar byte_data) -{ -#ifdef CONFIG_PCI -pci_write_config_byte (to_pci_dev (asc_dvc->cfg->dev), offset, byte_data); -#endif /* CONFIG_PCI */ -} - -/* * Return the BIOS address of the adapter at the specified * I/O port and with the specified bus type. */
@@ -7521,33 +7473,6 @@ DvcGetPhyAddr(ADV_DVC_VAR *asc_dvc, ADV_SCSI_REQ_Q *scsiq, }/* -* Read a PCI configuration byte. -*/ -static uchar __devinit DvcAdvReadPCIConfigByte (ADV_DVC_VAR *asc_dvc,ushort offset) -{ -#ifdef CONFIG_PCI -uchar byte_data; -pci_read_config_byte (to_pci_dev (asc_dvc->cfg->dev), offset, &byte_data); -return byte_data; -#else /* CONFIG_PCI */ -return 0 ; -#endif /* CONFIG_PCI */ -} - -/* -* Write a PCI configuration byte. -*/ -static void __devinit -DvcAdvWritePCIConfigByte (ADV_DVC_VAR *asc_dvc,ushort offset,uchar byte_data) -{ -#ifdef CONFIG_PCI -pci_write_config_byte (to_pci_dev (asc_dvc->cfg->dev), offset, byte_data); -#else /* CONFIG_PCI */ -return ; -#endif /* CONFIG_PCI */ -} - -/* * --- Tracing and Debugging Functions */
@@ -10481,89 +10406,15 @@ static uchar __devinit AscGetIsaDmaSpeed(PortAddr iop_base) }#endif /* CONFIG_ISA */ -static ushort __devinit -AscReadPCIConfigWord (ASC_DVC_VAR *asc_dvc,ushort pci_config_offset) -{ -uchar lsb, msb; - - lsb =DvcReadPCIConfigByte (asc_dvc, pci_config_offset); - msb =DvcReadPCIConfigByte (asc_dvc, pci_config_offset +1 ); -return ((ushort)((msb <<8 ) | lsb)); -} -static ushort __devinit AscInitGetConfig (ASC_DVC_VAR *asc_dvc) { -ushort warn_code; -PortAddr iop_base; -ushort PCIDeviceID; -ushort PCIVendorID; -uchar PCIRevisionID; -uchar prevCmdRegBits; +unsigned short warn_code =0 ; - warn_code =0 ; - iop_base = asc_dvc->iop_base; asc_dvc->init_state =ASC_INIT_STATE_BEG_GET_CFG ; -if (asc_dvc->err_code !=0 ) { +if (asc_dvc->err_code !=0 )return (UW_ERR ); - } -if (asc_dvc->bus_type ==ASC_IS_PCI ) { - PCIVendorID =AscReadPCIConfigWord (asc_dvc, - AscPCIConfigVendorIDRegister); - PCIDeviceID =AscReadPCIConfigWord (asc_dvc, - AscPCIConfigDeviceIDRegister); - - PCIRevisionID =DvcReadPCIConfigByte (asc_dvc, - AscPCIConfigRevisionIDRegister); - -if (PCIVendorID !=PCI_VENDOR_ID_ASP ) { - warn_code |=ASC_WARN_SET_PCI_CONFIG_SPACE ; - } - prevCmdRegBits =DvcReadPCIConfigByte (asc_dvc, - AscPCIConfigCommandRegister); - -if ((prevCmdRegBits & AscPCICmdRegBits_IOMemBusMaster) != - AscPCICmdRegBits_IOMemBusMaster) { -DvcWritePCIConfigByte (asc_dvc, - AscPCIConfigCommandRegister, - (prevCmdRegBits | - AscPCICmdRegBits_IOMemBusMaster)); - -if ((DvcReadPCIConfigByte (asc_dvc, - AscPCIConfigCommandRegister) - & AscPCICmdRegBits_IOMemBusMaster) - != AscPCICmdRegBits_IOMemBusMaster) { - warn_code |=ASC_WARN_SET_PCI_CONFIG_SPACE ; - } - } -if ((PCIDeviceID ==PCI_DEVICE_ID_ASP_1200A ) || - (PCIDeviceID ==PCI_DEVICE_ID_ASP_ABP940 )) { -DvcWritePCIConfigByte (asc_dvc, - AscPCIConfigLatencyTimer,0x00 ); -if (DvcReadPCIConfigByte - (asc_dvc, AscPCIConfigLatencyTimer) - !=0x00 ) { - warn_code |=ASC_WARN_SET_PCI_CONFIG_SPACE ; - } - }else if (PCIDeviceID ==PCI_DEVICE_ID_ASP_ABP940U ) { -if (DvcReadPCIConfigByte (asc_dvc, - AscPCIConfigLatencyTimer) < -0x20 ) { -DvcWritePCIConfigByte (asc_dvc, - AscPCIConfigLatencyTimer, -0x20 ); - -if (DvcReadPCIConfigByte (asc_dvc, - AscPCIConfigLatencyTimer) - <0x20 ) { - warn_code |= -ASC_WARN_SET_PCI_CONFIG_SPACE ; - } - } - } - } - -if (AscFindSignature (iop_base)) { +if (AscFindSignature (asc_dvc->iop_base)) { warn_code |=AscInitAscDvcVar (asc_dvc); warn_code |=AscInitFromEEP (asc_dvc); asc_dvc->init_state |=ASC_INIT_STATE_END_GET_CFG ;
@@ -10597,13 +10448,8 @@ static ushort __devinit AscInitFromAscDvcVar(ASC_DVC_VAR *asc_dvc)PortAddr iop_base;ushort cfg_msw;ushort warn_code; -ushort pci_device_id =0 ; iop_base = asc_dvc->iop_base; -#ifdef CONFIG_PCI -if (asc_dvc->cfg->dev) - pci_device_id =to_pci_dev (asc_dvc->cfg->dev)->device; -#endif warn_code =0 ; cfg_msw =AscGetChipCfgMsw (iop_base);if ((cfg_msw &ASC_CFG_MSW_CLR_MASK ) !=0 ) {
@@ -10625,19 +10471,23 @@ static ushort __devinit AscInitFromAscDvcVar(ASC_DVC_VAR *asc_dvc) asc_dvc->err_code |=ASC_IERR_SET_IRQ_NO ; } } +#ifdef CONFIG_PCI if (asc_dvc->bus_type &ASC_IS_PCI ) { +struct pci_dev *pdev =to_pci_dev (asc_dvc->cfg->dev); cfg_msw &=0xFFC0 ;AscSetChipCfgMsw (iop_base, cfg_msw);if ((asc_dvc->bus_type &ASC_IS_PCI_ULTRA ) ==ASC_IS_PCI_ULTRA ) { }else { -if ((pci_device_id ==PCI_DEVICE_ID_ASP_1200A ) || - (pci_device_id ==PCI_DEVICE_ID_ASP_ABP940 )) { +if ((pdev->device ==PCI_DEVICE_ID_ASP_1200A ) || + (pdev->device ==PCI_DEVICE_ID_ASP_ABP940 )) { asc_dvc->bug_fix_cntl |=ASC_BUG_FIX_IF_NOT_DWB ; asc_dvc->bug_fix_cntl |=ASC_BUG_FIX_ASYN_USE_SYN ; } } - }else if (asc_dvc->bus_type ==ASC_IS_ISAPNP ) { + } else +#endif /* CONFIG_PCI */ +if (asc_dvc->bus_type ==ASC_IS_ISAPNP ) {if (AscGetChipVersion (iop_base, asc_dvc->bus_type) ==ASC_CHIP_VER_ASYN_BUG ) { asc_dvc->bug_fix_cntl |=ASC_BUG_FIX_ASYN_USE_SYN ;
@@ -14035,55 +13885,13 @@ static ADVEEP_38C1600_CONFIG ADVEEP_38C1600_Config_Field_IsChar __devinitdata =*/ static int __devinit AdvInitGetConfig (ADV_DVC_VAR *asc_dvc) { -ushort warn_code; -AdvPortAddr iop_base; -uchar pci_cmd_reg; +unsigned short warn_code =0 ; +AdvPortAddr iop_base = asc_dvc->iop_base; +struct pci_dev *pdev =to_pci_dev (asc_dvc->cfg->dev); +u16 cmd;int status; - warn_code =0 ; asc_dvc->err_code =0 ; - iop_base = asc_dvc->iop_base; - -/* -* PCI Command Register -* -* Note: AscPCICmdRegBits_BusMastering definition (0x0007) includes -* I/O Space Control, Memory Space Control and Bus Master Control bits. -*/ - -if (((pci_cmd_reg =DvcAdvReadPCIConfigByte (asc_dvc, - AscPCIConfigCommandRegister)) - & AscPCICmdRegBits_BusMastering) - != AscPCICmdRegBits_BusMastering) { - pci_cmd_reg |= AscPCICmdRegBits_BusMastering; - -DvcAdvWritePCIConfigByte (asc_dvc, - AscPCIConfigCommandRegister, - pci_cmd_reg); - -if (((DvcAdvReadPCIConfigByte - (asc_dvc, AscPCIConfigCommandRegister)) - & AscPCICmdRegBits_BusMastering) - != AscPCICmdRegBits_BusMastering) { - warn_code |=ASC_WARN_SET_PCI_CONFIG_SPACE ; - } - } - -/* -* PCI Latency Timer -* -* If the "latency timer" register is 0x20 or above, then we don't need -* to change it. Otherwise, set it to 0x20 (i.e. set it to 0x20 if it -* comes up less than 0x20). -*/ -if (DvcAdvReadPCIConfigByte (asc_dvc, AscPCIConfigLatencyTimer) <0x20 ) { -DvcAdvWritePCIConfigByte (asc_dvc, AscPCIConfigLatencyTimer, -0x20 ); -if (DvcAdvReadPCIConfigByte (asc_dvc, AscPCIConfigLatencyTimer) < -0x20 ) { - warn_code |=ASC_WARN_SET_PCI_CONFIG_SPACE ; - } - }/* * Save the state of the PCI Configuration Command Register
@@ -14092,10 +13900,9 @@ static int __devinit AdvInitGetConfig(ADV_DVC_VAR *asc_dvc)* DMA parity errors. */ asc_dvc->cfg->control_flag =0 ; -if (((DvcAdvReadPCIConfigByte (asc_dvc, AscPCIConfigCommandRegister) - & AscPCICmdRegBits_ParErrRespCtrl)) ==0 ) { +pci_read_config_word (pdev,PCI_COMMAND , &cmd); +if ((cmd &PCI_COMMAND_PARITY ) ==0 ) asc_dvc->cfg->control_flag |=CONTROL_FLAG_IGNORE_PERR ; - } asc_dvc->cfg->lib_version = (ADV_LIB_VERSION_MAJOR <<8 ) |ADV_LIB_VERSION_MINOR ;
@@ -14137,19 +13944,11 @@ static int __devinit AdvInitGetConfig(ADV_DVC_VAR *asc_dvc) ADV_CTRL_REG_CMD_WR_IO_REG);if (asc_dvc->chip_type ==ADV_CHIP_ASC38C1600 ) { -if ((status = -AdvInitFrom38C1600EEP (asc_dvc)) ==ADV_ERROR ) { -return ADV_ERROR ; - } + status =AdvInitFrom38C1600EEP (asc_dvc); }else if (asc_dvc->chip_type ==ADV_CHIP_ASC38C0800 ) { -if ((status = -AdvInitFrom38C0800EEP (asc_dvc)) ==ADV_ERROR ) { -return ADV_ERROR ; - } + status =AdvInitFrom38C0800EEP (asc_dvc); }else { -if ((status =AdvInitFrom3550EEP (asc_dvc)) ==ADV_ERROR ) { -return ADV_ERROR ; - } + status =AdvInitFrom3550EEP (asc_dvc); } warn_code |= status; }
@@ -18759,6 +18558,19 @@ static struct pci_device_id advansys_pci_tbl[] __devinitdata = {MODULE_DEVICE_TABLE (pci, advansys_pci_tbl); +static void __devinit advansys_set_latency (struct pci_dev *pdev) +{ +if ((pdev->device ==PCI_DEVICE_ID_ASP_1200A ) || + (pdev->device ==PCI_DEVICE_ID_ASP_ABP940 )) { +pci_write_config_byte (pdev,PCI_LATENCY_TIMER ,0 ); + }else { +u8 latency; +pci_read_config_byte (pdev,PCI_LATENCY_TIMER , &latency); +if (latency <0x20 ) +pci_write_config_byte (pdev,PCI_LATENCY_TIMER ,0x20 ); + } +} +static int __devinit advansys_pci_probe (struct pci_dev *pdev,const struct pci_device_id *ent) {
@@ -18769,6 +18581,8 @@ advansys_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) goto fail;if (pci_enable_device (pdev)) goto release_regions; +pci_set_master (pdev); +advansys_set_latency (pdev); ioport =pci_resource_start (pdev,0 ); shost =advansys_board_found (ioport, &pdev->dev,ASC_IS_PCI );
Signature · 1 line
1.4.4.4
Re: [PATCH 9/21] advansys: More PCI cleanups
Quoted 5 lines · Matthew Wilcox
drivers/scsi/advansys.c +1
-0@@ -18759,6 +18558,19 @@ static struct pci_device_id advansys_pci_tbl[] __devinitdata = {MODULE_DEVICE_TABLE (pci, advansys_pci_tbl); +static void __devinit advansys_set_latency (struct pci_dev *pdev)
drivers/scsi/advansys.c static struct pci_device_id advansys_pci_tbl[] __devinitdata = { · line 18571
Quoted 10 lines
drivers/scsi/advansys.c +10
-0+{ +if ((pdev->device ==PCI_DEVICE_ID_ASP_1200A ) || + (pdev->device ==PCI_DEVICE_ID_ASP_ABP940 )) { +pci_write_config_byte (pdev,PCI_LATENCY_TIMER ,0 ); + }else { +u8 latency; +pci_read_config_byte (pdev,PCI_LATENCY_TIMER , &latency); +if (latency <0x20 ) +pci_write_config_byte (pdev,PCI_LATENCY_TIMER ,0x20 ); + }
the latter branch is useless -- pci_set_master() ensures PCI_LATENCY_TIMER is adequate
Re: [PATCH 9/21] advansys: More PCI cleanups
Quoted 6 lines · Jeff Garzik
Quoted 6 lines
drivers/scsi/advansys.c +6
-0+static void __devinit advansys_set_latency (struct pci_dev *pdev) +{ +if ((pdev->device ==PCI_DEVICE_ID_ASP_1200A ) || + (pdev->device ==PCI_DEVICE_ID_ASP_ABP940 )) { +pci_write_config_byte (pdev,PCI_LATENCY_TIMER ,0 ); + }else {
Quoted 8 lines
drivers/scsi/advansys.c static struct pci_device_id advansys_pci_tbl[] __devinitdata = { · line 18571
Quoted 5 lines
drivers/scsi/advansys.c +5
-0+u8 latency; +pci_read_config_byte (pdev,PCI_LATENCY_TIMER , &latency); +if (latency <0x20 ) +pci_write_config_byte (pdev,PCI_LATENCY_TIMER ,0x20 ); + }the latter branch is useless --
pci_set_master()ensuresPCI_LATENCY_TIMERis adequate
i386 certainly has a workaround for broken BIOSes, but this looks to me like it's working around an issue the card has with a latency timer below 32. Note that i386's implementation of pcibios_set_master() only ensures that the latency timer is 16 or higher.
Signature · 3 lines
"Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Re: [PATCH 9/21] advansys: More PCI cleanups
Quoted 14 lines · Matthew Wilcox
Quoted 4 lines · Jeff Garzik
Quoted 4 lines
drivers/scsi/advansys.c +4
-0+static void __devinit advansys_set_latency (struct pci_dev *pdev) +{ +if ((pdev->device ==PCI_DEVICE_ID_ASP_1200A ) || + (pdev->device ==PCI_DEVICE_ID_ASP_ABP940 )) {Quoted 8 lines
drivers/scsi/advansys.c static struct pci_device_id advansys_pci_tbl[] __devinitdata = { · line 18571
Quoted 7 lines
drivers/scsi/advansys.c +7
-0+pci_write_config_byte (pdev,PCI_LATENCY_TIMER ,0 ); + }else { +u8 latency; +pci_read_config_byte (pdev,PCI_LATENCY_TIMER , &latency); +if (latency <0x20 ) +pci_write_config_byte (pdev,PCI_LATENCY_TIMER ,0x20 ); + }the latter branch is useless --
pci_set_master()ensuresPCI_LATENCY_TIMERis adequate
Quoted 4 lines
i386 certainly has a workaround for broken BIOSes, but this looks to me like it's working around an issue the card has with a latency timer below 32. Note that i386's implementation of
pcibios_set_master()only ensures that the latency timer is 16 or higher.
Ah, indeed.
Jeff
PATCH 10/21 advansys: remove AscCompareString()
In reply to Matthew WilcoxAscCompareString() is just another name for strncmp
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 23 +++-------------------- 1 files changed, 3 insertions(+), 20 deletions(-)
drivers/scsi/advansys.c +3 -20
@@ -1922,7 +1922,6 @@ static int AscIsrChipHalted(ASC_DVC_VAR *);static uchar _AscCopyLramScsiDoneQ (PortAddr ,ushort ,ASC_QDONE_INFO *,ASC_DCNT );static int AscIsrQDone (ASC_DVC_VAR *); -static int AscCompareString (uchar *,uchar *,int );#ifdef CONFIG_ISA static ushort AscGetEisaChipCfg (PortAddr );static PortAddr AscSearchIOPortAddr11 (PortAddr );
@@ -11158,8 +11157,7 @@ AscAsyncFix(ASC_DVC_VAR *asc_dvc, uchar tid_no, ASC_SCSI_INQUIRY *inq)if (asc_dvc->bug_fix_cntl &ASC_BUG_FIX_ASYN_USE_SYN ) {if (!(asc_dvc->init_sdtr & tid_bits)) {if ((dvc_type ==TYPE_ROM ) && - (AscCompareString ((uchar *)inq->vendor_id, - (uchar *)"HP " ,3 ) ==0 )) { + (strncmp (inq->vendor_id,"HP " ,3 ) ==0 )) { asc_dvc->pci_fix_asyn_xfer_always |= tid_bits; } asc_dvc->pci_fix_asyn_xfer |= tid_bits;
@@ -11182,10 +11180,8 @@ AscAsyncFix(ASC_DVC_VAR *asc_dvc, uchar tid_no, ASC_SCSI_INQUIRY *inq)static int AscTagQueuingSafe (ASC_SCSI_INQUIRY *inq) {if ((inq->add_len >=32 ) && - (AscCompareString ((uchar *)inq->vendor_id, - (uchar *)"QUANTUM XP34301" ,15 ) ==0 ) && - (AscCompareString ((uchar *)inq->product_rev_level, - (uchar *)"1071" ,4 ) ==0 )) { + (strncmp (inq->vendor_id,"QUANTUM XP34301" ,15 ) ==0 ) && + (strncmp (inq->product_rev_level,"1071" ,4 ) ==0 )) {return 0 ; }return 1 ;
@@ -11236,19 +11232,6 @@ AscInquiryHandling(ASC_DVC_VAR *asc_dvc, uchar tid_no, ASC_SCSI_INQUIRY *inq)return ; } -static int AscCompareString (uchar *str1,uchar *str2,int len) -{ -int i; -int diff; - -for (i =0 ; i < len; i++) { - diff = (int )(str1[i] - str2[i]); -if (diff !=0 ) -return (diff); - } -return (0 ); -} -static uchar AscReadLramByte (PortAddr iop_base,ushort addr) {uchar byte_data;
Signature · 1 line
1.4.4.4
PATCH 11/21 Add QUANTUM XP34301 to the blacklist
In reply to Matthew WilcoxAccording to the AdvanSys driver, this device has a problem with tagged queueing.
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/scsi_devinfo.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/scsi/scsi_devinfo.c +1 -0
@@ -211,6 +211,7 @@ static struct { {"PIONEER", "CD-ROM DRM-604X", NULL, BLIST_FORCELUN | BLIST_SINGLELUN}, {"PIONEER", "CD-ROM DRM-624X", NULL, BLIST_FORCELUN | BLIST_SINGLELUN}, {"Promise", "", NULL, BLIST_SPARSELUN}, + {"QUANTUM", "XP34301", "1071", BLIST_NOTQ}, {"REGAL", "CDC-4X", NULL, BLIST_MAX5LUN | BLIST_SINGLELUN}, {"SanDisk", "ImageMate CF-SD1", NULL, BLIST_FORCELUN}, {"SEAGATE", "ST34555N", "0930", BLIST_NOTQ}, /* Chokes on tagged INQUIRY */
Signature · 1 line
1.4.4.4
PATCH 12/21 advansys: remove INQUIRY sniffing
In reply to Matthew WilcoxUse slave_configure() to do all the work that used to be done in AscInquiryHandling and AdvInquiryHandling. Split slave_configure into two functions, one for wide and one for narrow controllers.
Remove some unused definitions, duplicate definitions, unnecessary declarations, and scsireqq, cap_info and inquiry from struct asc_board.
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 637 ++++++++++++++++------------------------------- 1 files changed, 211 insertions(+), 426 deletions(-)
drivers/scsi/advansys.c +211 -426
@@ -943,10 +943,6 @@ typedef unsigned char uchar;#define ASC_MAX_CDB_LEN 12#define ASC_SCSI_RESET_HOLD_TIME_US 60 -#define ADV_INQ_CLOCKING_ST_ONLY 0x0 -#define ADV_INQ_CLOCKING_DT_ONLY 0x1 -#define ADV_INQ_CLOCKING_ST_AND_DT 0x3 -/* * Inquiry SPC-2 SPI Byte 1 EVPD (Enable Vital Product Data) * and CmdDt (Command Support Data) field bit definitions.
@@ -965,57 +961,8 @@ typedef unsigned char uchar;#define ASC_SRB_TID (x) ((uchar)((uchar)(x) & (uchar)0x0F))#define ASC_SRB_LUN (x) ((uchar)((uint)(x) >> 13))#define PUT_CDB1 (x) ((uchar)((uint)(x) >> 8)) -#define MS_CMD_DONE 0x00 -#define MS_EXTEND 0x01#define MS_SDTR_LEN 0x03 -#define MS_SDTR_CODE 0x01#define MS_WDTR_LEN 0x02 -#define MS_WDTR_CODE 0x03 -#define MS_MDP_LEN 0x05 -#define MS_MDP_CODE 0x00 - -/* -* Inquiry data structure and bitfield macros -* -* Only quantities of more than 1 bit are shifted, since the others are -* just tested for true or false. C bitfields aren't portable between big -* and little-endian platforms so they are not used. -*/ - -#define ASC_INQ_DVC_TYPE (inq) ((inq)->periph & 0x1f) -#define ASC_INQ_QUALIFIER (inq) (((inq)->periph & 0xe0) >> 5) -#define ASC_INQ_DVC_TYPE_MOD (inq) ((inq)->devtype & 0x7f) -#define ASC_INQ_REMOVABLE (inq) ((inq)->devtype & 0x80) -#define ASC_INQ_ANSI_VER (inq) ((inq)->ver & 0x07) -#define ASC_INQ_ECMA_VER (inq) (((inq)->ver & 0x38) >> 3) -#define ASC_INQ_ISO_VER (inq) (((inq)->ver & 0xc0) >> 6) -#define ASC_INQ_RESPONSE_FMT (inq) ((inq)->byte3 & 0x0f) -#define ASC_INQ_TERM_IO (inq) ((inq)->byte3 & 0x40) -#define ASC_INQ_ASYNC_NOTIF (inq) ((inq)->byte3 & 0x80) -#define ASC_INQ_SOFT_RESET (inq) ((inq)->flags & 0x01) -#define ASC_INQ_CMD_QUEUE (inq) ((inq)->flags & 0x02) -#define ASC_INQ_LINK_CMD (inq) ((inq)->flags & 0x08) -#define ASC_INQ_SYNC (inq) ((inq)->flags & 0x10) -#define ASC_INQ_WIDE16 (inq) ((inq)->flags & 0x20) -#define ASC_INQ_WIDE32 (inq) ((inq)->flags & 0x40) -#define ASC_INQ_REL_ADDR (inq) ((inq)->flags & 0x80) -#define ASC_INQ_INFO_UNIT (inq) ((inq)->info & 0x01) -#define ASC_INQ_QUICK_ARB (inq) ((inq)->info & 0x02) -#define ASC_INQ_CLOCKING (inq) (((inq)->info & 0x0c) >> 2) - -typedef struct { -uchar periph; -uchar devtype; -uchar ver; -uchar byte3; -uchar add_len; -uchar res1; -uchar res2; -uchar flags; -uchar vendor_id[8 ]; -uchar product_id[16 ]; -uchar product_rev_level[4 ]; -}ASC_SCSI_INQUIRY ;#define ASC_SG_LIST_PER_Q 7#define QS_FREE 0x00
@@ -1945,9 +1892,7 @@ static void DvcDelayNanoSecond(ASC_DVC_VAR *, ASC_DCNT);static void DvcPutScsiQ (PortAddr ,ushort ,uchar *,int );static void DvcGetQinfo (PortAddr ,ushort ,uchar *,int );static ushort AscInitAsc1000Driver (ASC_DVC_VAR *); -static void AscAsyncFix (ASC_DVC_VAR *,uchar ,ASC_SCSI_INQUIRY *); -static int AscTagQueuingSafe (ASC_SCSI_INQUIRY *); -static void AscInquiryHandling (ASC_DVC_VAR *,uchar ,ASC_SCSI_INQUIRY *); +static void AscAsyncFix (ASC_DVC_VAR *,struct scsi_device *);static int AscExeScsiQueue (ASC_DVC_VAR *,ASC_SCSI_Q *);static int AscISR (ASC_DVC_VAR *);static uint AscGetNumOfFreeQueue (ASC_DVC_VAR *,uchar ,uchar );
@@ -3094,7 +3039,6 @@ static int AdvResetSB(ADV_DVC_VAR *asc_dvc);* Internal Adv Library functions. */ static int AdvSendIdleCmd (ADV_DVC_VAR *,ushort ,ADV_DCNT ); -static void AdvInquiryHandling (ADV_DVC_VAR *,ADV_SCSI_REQ_Q *);static int AdvInitFrom3550EEP (ADV_DVC_VAR *);static int AdvInitFrom38C0800EEP (ADV_DVC_VAR *);static int AdvInitFrom38C1600EEP (ADV_DVC_VAR *);
@@ -3309,74 +3253,6 @@ static ADVEEP_38C1600_CONFIG Default_38C1600_EEPROM_Config; ((ADV_MAX_SG_LIST + (NO_OF_SG_PER_BLOCK - 1))/NO_OF_SG_PER_BLOCK))/* -* Inquiry data structure and bitfield macros -* -* Using bitfields to access the subchar data isn't portable across -* endianness, so instead mask and shift. Only quantities of more -* than 1 bit are shifted, since the others are just tested for true -* or false. -*/ - -#define ADV_INQ_DVC_TYPE (inq) ((inq)->periph & 0x1f) -#define ADV_INQ_QUALIFIER (inq) (((inq)->periph & 0xe0) >> 5) -#define ADV_INQ_DVC_TYPE_MOD (inq) ((inq)->devtype & 0x7f) -#define ADV_INQ_REMOVABLE (inq) ((inq)->devtype & 0x80) -#define ADV_INQ_ANSI_VER (inq) ((inq)->ver & 0x07) -#define ADV_INQ_ECMA_VER (inq) (((inq)->ver & 0x38) >> 3) -#define ADV_INQ_ISO_VER (inq) (((inq)->ver & 0xc0) >> 6) -#define ADV_INQ_RESPONSE_FMT (inq) ((inq)->byte3 & 0x0f) -#define ADV_INQ_TERM_IO (inq) ((inq)->byte3 & 0x40) -#define ADV_INQ_ASYNC_NOTIF (inq) ((inq)->byte3 & 0x80) -#define ADV_INQ_SOFT_RESET (inq) ((inq)->flags & 0x01) -#define ADV_INQ_CMD_QUEUE (inq) ((inq)->flags & 0x02) -#define ADV_INQ_LINK_CMD (inq) ((inq)->flags & 0x08) -#define ADV_INQ_SYNC (inq) ((inq)->flags & 0x10) -#define ADV_INQ_WIDE16 (inq) ((inq)->flags & 0x20) -#define ADV_INQ_WIDE32 (inq) ((inq)->flags & 0x40) -#define ADV_INQ_REL_ADDR (inq) ((inq)->flags & 0x80) -#define ADV_INQ_INFO_UNIT (inq) ((inq)->info & 0x01) -#define ADV_INQ_QUICK_ARB (inq) ((inq)->info & 0x02) -#define ADV_INQ_CLOCKING (inq) (((inq)->info & 0x0c) >> 2) - -typedef struct { -uchar periph;/* peripheral device type [0:4] */ -/* peripheral qualifier [5:7] */ -uchar devtype;/* device type modifier (for SCSI I) [0:6] */ -/* RMB - removable medium bit [7] */ -uchar ver;/* ANSI approved version [0:2] */ -/* ECMA version [3:5] */ -/* ISO version [6:7] */ -uchar byte3;/* response data format [0:3] */ -/* 0 SCSI 1 */ -/* 1 CCS */ -/* 2 SCSI-2 */ -/* 3-F reserved */ -/* reserved [4:5] */ -/* terminate I/O process bit (see 5.6.22) [6] */ -/* asynch. event notification (processor) [7] */ -uchar add_len;/* additional length */ -uchar res1;/* reserved */ -uchar res2;/* reserved */ -uchar flags;/* soft reset implemented [0] */ -/* command queuing [1] */ -/* reserved [2] */ -/* linked command for this logical unit [3] */ -/* synchronous data transfer [4] */ -/* wide bus 16 bit data transfer [5] */ -/* wide bus 32 bit data transfer [6] */ -/* relative addressing mode [7] */ -uchar vendor_id[8 ];/* vendor identification */ -uchar product_id[16 ];/* product identification */ -uchar product_rev_level[4 ];/* product revision level */ -uchar vendor_specific[20 ];/* vendor specific */ -uchar info;/* information unit supported [0] */ -/* quick arbitrate supported [1] */ -/* clocking field [2:3] */ -/* reserved [4:7] */ -uchar res3;/* reserved */ -}ADV_SCSI_INQUIRY ;/* 58 bytes */ - -/* * --- Driver Constants and Macros */
@@ -3790,10 +3666,6 @@ typedef struct asc_board {/* * The following fields are used only for Narrow Boards. */ -/* The following three structures must be in DMA-able memory. */ -ASC_SCSI_REQ_Q scsireqq; -ASC_CAP_INFO cap_info; -ASC_SCSI_INQUIRY inquiry;uchar sdtr_data[ASC_MAX_TID +1 ];/* SDTR information */ /* * The following fields are used only for Wide Boards.
@@ -3852,8 +3724,6 @@ static PortAddr _asc_def_iop_base[];/* * --- Driver Function Prototypes -* -* advansys.h contains function prototypes for functions global to Linux. */ static irqreturn_t advansys_interrupt (int ,void *);
@@ -4681,38 +4551,203 @@ static irqreturn_t advansys_interrupt(int irq, void *dev_id)return IRQ_HANDLED ; } +static void +advansys_narrow_slave_configure (struct scsi_device *sdev,ASC_DVC_VAR *asc_dvc) +{ +ASC_SCSI_BIT_ID_TYPE tid_bit =1 << sdev->id; +ASC_SCSI_BIT_ID_TYPE orig_use_tagged_qng = asc_dvc->use_tagged_qng; + +if (sdev->lun ==0 ) { +ASC_SCSI_BIT_ID_TYPE orig_init_sdtr = asc_dvc->init_sdtr; +if ((asc_dvc->cfg->sdtr_enable & tid_bit) && sdev->sdtr) { + asc_dvc->init_sdtr |= tid_bit; + }else { + asc_dvc->init_sdtr &= ~tid_bit; + } + +if (orig_init_sdtr != asc_dvc->init_sdtr) +AscAsyncFix (asc_dvc, sdev); + } + +if (sdev->tagged_supported) { +if (asc_dvc->cfg->cmd_qng_enabled & tid_bit) { +if (sdev->lun ==0 ) { + asc_dvc->cfg->can_tagged_qng |= tid_bit; + asc_dvc->use_tagged_qng |= tid_bit; + } +scsi_adjust_queue_depth (sdev,MSG_ORDERED_TAG , + asc_dvc->max_dvc_qng[sdev->id]); + } + }else { +if (sdev->lun ==0 ) { + asc_dvc->cfg->can_tagged_qng &= ~tid_bit; + asc_dvc->use_tagged_qng &= ~tid_bit; + } +scsi_adjust_queue_depth (sdev,0 , sdev->host->cmd_per_lun); + } + +if ((sdev->lun ==0 ) && + (orig_use_tagged_qng != asc_dvc->use_tagged_qng)) { +AscWriteLramByte (asc_dvc->iop_base,ASCV_DISC_ENABLE_B , + asc_dvc->cfg->disc_enable); +AscWriteLramByte (asc_dvc->iop_base,ASCV_USE_TAGGED_QNG_B , + asc_dvc->use_tagged_qng); +AscWriteLramByte (asc_dvc->iop_base,ASCV_CAN_TAGGED_QNG_B , + asc_dvc->cfg->can_tagged_qng); + + asc_dvc->max_dvc_qng[sdev->id] = + asc_dvc->cfg->max_tag_qng[sdev->id]; +AscWriteLramByte (asc_dvc->iop_base, + (ushort)(ASCV_MAX_DVC_QNG_BEG + sdev->id), + asc_dvc->max_dvc_qng[sdev->id]); + } +} +/* -* Set the number of commands to queue per device for the -* specified host adapter. +* Wide Transfers +* +* If the EEPROM enabled WDTR for the device and the device supports wide +* bus (16 bit) transfers, then turn on the device's 'wdtr_able' bit and +* write the new value to the microcode. */ -static int advansys_slave_configure (struct scsi_device *device) +static void +advansys_wide_enable_wdtr (AdvPortAddr iop_base,unsigned short tidmask) { -asc_board_t *boardp; +unsigned short cfg_word; +AdvReadWordLram (iop_base,ASC_MC_WDTR_ABLE , cfg_word); +if ((cfg_word & tidmask) !=0 ) +return ; + + cfg_word |= tidmask; +AdvWriteWordLram (iop_base,ASC_MC_WDTR_ABLE , cfg_word); - boardp =ASC_BOARDP (device->host); - boardp->flags |=ASC_SELECT_QUEUE_DEPTHS ;/* -* Save a pointer to the device and set its initial/maximum -* queue depth. Only save the pointer for a lun0 dev though. +* Clear the microcode SDTR and WDTR negotiation done indicators for +* the target to cause it to negotiate with the new setting set above. +* WDTR when accepted causes the target to enter asynchronous mode, so +* SDTR must be negotiated. */ -if (device->lun ==0 ) - boardp->device[device->id] = device; -if (device->tagged_supported) { -if (ASC_NARROW_BOARD (boardp)) { -scsi_adjust_queue_depth (device,MSG_ORDERED_TAG , - boardp->dvc_var.asc_dvc_var. - max_dvc_qng[device->id]); - }else { -scsi_adjust_queue_depth (device,MSG_ORDERED_TAG , - boardp->dvc_var.adv_dvc_var. - max_dvc_qng); +AdvReadWordLram (iop_base,ASC_MC_SDTR_DONE , cfg_word); + cfg_word &= ~tidmask; +AdvWriteWordLram (iop_base,ASC_MC_SDTR_DONE , cfg_word); +AdvReadWordLram (iop_base,ASC_MC_WDTR_DONE , cfg_word); + cfg_word &= ~tidmask; +AdvWriteWordLram (iop_base,ASC_MC_WDTR_DONE , cfg_word); +} + +/* +* Synchronous Transfers +* +* If the EEPROM enabled SDTR for the device and the device +* supports synchronous transfers, then turn on the device's +* 'sdtr_able' bit. Write the new value to the microcode. +*/ +static void +advansys_wide_enable_sdtr (AdvPortAddr iop_base,unsigned short tidmask) +{ +unsigned short cfg_word; +AdvReadWordLram (iop_base,ASC_MC_SDTR_ABLE , cfg_word); +if ((cfg_word & tidmask) !=0 ) +return ; + + cfg_word |= tidmask; +AdvWriteWordLram (iop_base,ASC_MC_SDTR_ABLE , cfg_word); + +/* +* Clear the microcode "SDTR negotiation" done indicator for the +* target to cause it to negotiate with the new setting set above. +*/ +AdvReadWordLram (iop_base,ASC_MC_SDTR_DONE , cfg_word); + cfg_word &= ~tidmask; +AdvWriteWordLram (iop_base,ASC_MC_SDTR_DONE , cfg_word); +} + +/* +* PPR (Parallel Protocol Request) Capable +* +* If the device supports DT mode, then it must be PPR capable. +* The PPR message will be used in place of the SDTR and WDTR +* messages to negotiate synchronous speed and offset, transfer +* width, and protocol options. +*/ +static void advansys_wide_enable_ppr (ADV_DVC_VAR *adv_dvc, +AdvPortAddr iop_base,unsigned short tidmask) +{ +AdvReadWordLram (iop_base,ASC_MC_PPR_ABLE , adv_dvc->ppr_able); + adv_dvc->ppr_able |= tidmask; +AdvWriteWordLram (iop_base,ASC_MC_PPR_ABLE , adv_dvc->ppr_able); +} + +static void +advansys_wide_slave_configure (struct scsi_device *sdev,ADV_DVC_VAR *adv_dvc) +{ +AdvPortAddr iop_base = adv_dvc->iop_base; +unsigned short tidmask =1 << sdev->id; + +if (sdev->lun ==0 ) { +/* +* Handle WDTR, SDTR, and Tag Queuing. If the feature +* is enabled in the EEPROM and the device supports the +* feature, then enable it in the microcode. +*/ + +if ((adv_dvc->wdtr_able & tidmask) && sdev->wdtr) +advansys_wide_enable_wdtr (iop_base, tidmask); +if ((adv_dvc->sdtr_able & tidmask) && sdev->sdtr) +advansys_wide_enable_sdtr (iop_base, tidmask); +if (adv_dvc->chip_type ==ADV_CHIP_ASC38C1600 && sdev->ppr) +advansys_wide_enable_ppr (adv_dvc, iop_base, tidmask); + +/* +* Tag Queuing is disabled for the BIOS which runs in polled +* mode and would see no benefit from Tag Queuing. Also by +* disabling Tag Queuing in the BIOS devices with Tag Queuing +* bugs will at least work with the BIOS. +*/ +if ((adv_dvc->tagqng_able & tidmask) && + sdev->tagged_supported) { +unsigned short cfg_word; +AdvReadWordLram (iop_base,ASC_MC_TAGQNG_ABLE , cfg_word); + cfg_word |= tidmask; +AdvWriteWordLram (iop_base,ASC_MC_TAGQNG_ABLE , + cfg_word); +AdvWriteByteLram (iop_base, +ASC_MC_NUMBER_OF_MAX_CMD + sdev->id, + adv_dvc->max_dvc_qng); } + } + +if ((adv_dvc->tagqng_able & tidmask) && sdev->tagged_supported) { +scsi_adjust_queue_depth (sdev,MSG_ORDERED_TAG , + adv_dvc->max_dvc_qng); }else { -scsi_adjust_queue_depth (device,0 , device->host->cmd_per_lun); +scsi_adjust_queue_depth (sdev,0 , sdev->host->cmd_per_lun); } -ASC_DBG4 (1 , -"advansys_slave_configure: device 0x%lx, boardp 0x%lx, id %d, depth %d\n" , - (ulong )device, (ulong )boardp, device->id, device->queue_depth); +} + +/* +* Set the number of commands to queue per device for the +* specified host adapter. +*/ +static int advansys_slave_configure (struct scsi_device *sdev) +{ +asc_board_t *boardp =ASC_BOARDP (sdev->host); + boardp->flags |=ASC_SELECT_QUEUE_DEPTHS ; + +/* +* Save a pointer to the sdev and set its initial/maximum +* queue depth. Only save the pointer for a lun0 dev though. +*/ +if (sdev->lun ==0 ) + boardp->device[sdev->id] = sdev; + +if (ASC_NARROW_BOARD (boardp)) +advansys_narrow_slave_configure (sdev, + &boardp->dvc_var.asc_dvc_var); +else +advansys_wide_slave_configure (sdev, + &boardp->dvc_var.adv_dvc_var); +return 0 ; }
@@ -5482,21 +5517,10 @@ static void asc_isr_callback(ASC_DVC_VAR *asc_dvc_varp, ASC_QDONE_INFO *qdonep) scp->result =0 ;/* -* If an INQUIRY command completed successfully, then call -* the AscInquiryHandling() function to set-up the device. -*/ -if (scp->cmnd[0 ] ==INQUIRY && scp->device->lun ==0 && - (scp->request_bufflen - qdonep->remain_bytes) >=8 ) { -AscInquiryHandling (asc_dvc_varp, scp->device->id &0x7 , - (ASC_SCSI_INQUIRY *)scp-> - request_buffer); - } - -/* * Check for an underrun condition. * * If there was no error and an underrun condition, then -* then return the number of underrun bytes. +* return the number of underrun bytes. */ if (scp->request_bufflen !=0 && qdonep->remain_bytes !=0 && qdonep->remain_bytes <= scp->request_bufflen) {
@@ -8389,8 +8413,8 @@ static int AscIsrChipHalted(ASC_DVC_VAR *asc_dvc) (uchar *)&ext_msg, sizeof(EXT_MSG) >> 1); -if (ext_msg.msg_type ==MS_EXTEND && - ext_msg.msg_req ==MS_SDTR_CODE && +if (ext_msg.msg_type ==EXTENDED_MESSAGE && + ext_msg.msg_req ==EXTENDED_SDTR && ext_msg.msg_len ==MS_SDTR_LEN ) { sdtr_accept = TRUE;if ((ext_msg.req_ack_offset >ASC_SYN_MAX_OFFSET )) {
@@ -8472,8 +8496,8 @@ static int AscIsrChipHalted(ASC_DVC_VAR *asc_dvc) q_cntl);AscWriteLramWord (iop_base,ASCV_HALTCODE_W ,0 );return (0 ); - }else if (ext_msg.msg_type ==MS_EXTEND && - ext_msg.msg_req ==MS_WDTR_CODE && + }else if (ext_msg.msg_type ==EXTENDED_MESSAGE && + ext_msg.msg_req ==EXTENDED_WDTR && ext_msg.msg_len ==MS_WDTR_LEN ) { ext_msg.wdtr_width =0 ;
@@ -8566,9 +8590,9 @@ static int AscIsrChipHalted(ASC_DVC_VAR *asc_dvc) (uchar *)&out_msg, sizeof(EXT_MSG) >> 1); -if ((out_msg.msg_type ==MS_EXTEND ) && +if ((out_msg.msg_type ==EXTENDED_MESSAGE ) && (out_msg.msg_len ==MS_SDTR_LEN ) && - (out_msg.msg_req ==MS_SDTR_CODE )) { + (out_msg.msg_req ==EXTENDED_SDTR )) { asc_dvc->init_sdtr &= ~target_id; asc_dvc->sdtr_done &= ~target_id;
@@ -10061,9 +10085,9 @@ AscMsgOutSDTR(ASC_DVC_VAR *asc_dvc, uchar sdtr_period, uchar sdtr_offset)PortAddr iop_base; iop_base = asc_dvc->iop_base; - sdtr_buf.msg_type =MS_EXTEND ; + sdtr_buf.msg_type =EXTENDED_MESSAGE ; sdtr_buf.msg_len =MS_SDTR_LEN ; - sdtr_buf.msg_req =MS_SDTR_CODE ; + sdtr_buf.msg_req =EXTENDED_SDTR ; sdtr_buf.xfer_period = sdtr_period; sdtr_offset &=ASC_SYN_MAX_OFFSET ; sdtr_buf.req_ack_offset = sdtr_offset;
@@ -11145,91 +11169,31 @@ AscSetEEPConfig(PortAddr iop_base, ASCEEP_CONFIG *cfg_buf, ushort bus_type)return (n_error); } -static void -AscAsyncFix (ASC_DVC_VAR *asc_dvc,uchar tid_no,ASC_SCSI_INQUIRY *inq) +static void AscAsyncFix (ASC_DVC_VAR *asc_dvc,struct scsi_device *sdev) { -uchar dvc_type; -ASC_SCSI_BIT_ID_TYPE tid_bits; - - dvc_type =ASC_INQ_DVC_TYPE (inq); - tid_bits =ASC_TIX_TO_TARGET_ID (tid_no); +char type = sdev->type; +ASC_SCSI_BIT_ID_TYPE tid_bits =1 << sdev->id;if (asc_dvc->bug_fix_cntl &ASC_BUG_FIX_ASYN_USE_SYN ) {if (!(asc_dvc->init_sdtr & tid_bits)) { -if ((dvc_type ==TYPE_ROM ) && - (strncmp (inq->vendor_id,"HP " ,3 ) ==0 )) { +if ((type ==TYPE_ROM ) && + (strncmp (sdev->vendor,"HP " ,3 ) ==0 )) { asc_dvc->pci_fix_asyn_xfer_always |= tid_bits; } asc_dvc->pci_fix_asyn_xfer |= tid_bits; -if ((dvc_type ==TYPE_PROCESSOR ) || - (dvc_type ==TYPE_SCANNER ) || - (dvc_type ==TYPE_ROM ) || (dvc_type ==TYPE_TAPE )) { +if ((type ==TYPE_PROCESSOR ) || + (type ==TYPE_SCANNER ) || (type ==TYPE_ROM ) || + (type ==TYPE_TAPE )) { asc_dvc->pci_fix_asyn_xfer &= ~tid_bits; }if (asc_dvc->pci_fix_asyn_xfer & tid_bits) {AscSetRunChipSynRegAtID (asc_dvc->iop_base, - tid_no, -ASYN_SDTR_DATA_FIX_PCI_REV_AB ); + sdev->id, +ASYN_SDTR_DATA_FIX_PCI_REV_AB ); } } } -return ; -} - -static int AscTagQueuingSafe (ASC_SCSI_INQUIRY *inq) -{ -if ((inq->add_len >=32 ) && - (strncmp (inq->vendor_id,"QUANTUM XP34301" ,15 ) ==0 ) && - (strncmp (inq->product_rev_level,"1071" ,4 ) ==0 )) { -return 0 ; - } -return 1 ; -} - -static void -AscInquiryHandling (ASC_DVC_VAR *asc_dvc,uchar tid_no,ASC_SCSI_INQUIRY *inq) -{ -ASC_SCSI_BIT_ID_TYPE tid_bit =ASC_TIX_TO_TARGET_ID (tid_no); -ASC_SCSI_BIT_ID_TYPE orig_init_sdtr, orig_use_tagged_qng; - - orig_init_sdtr = asc_dvc->init_sdtr; - orig_use_tagged_qng = asc_dvc->use_tagged_qng; - - asc_dvc->init_sdtr &= ~tid_bit; - asc_dvc->cfg->can_tagged_qng &= ~tid_bit; - asc_dvc->use_tagged_qng &= ~tid_bit; - -if (ASC_INQ_RESPONSE_FMT (inq) >=2 ||ASC_INQ_ANSI_VER (inq) >=2 ) { -if ((asc_dvc->cfg->sdtr_enable & tid_bit) &&ASC_INQ_SYNC (inq)) { - asc_dvc->init_sdtr |= tid_bit; - } -if ((asc_dvc->cfg->cmd_qng_enabled & tid_bit) && -ASC_INQ_CMD_QUEUE (inq)) { -if (AscTagQueuingSafe (inq)) { - asc_dvc->use_tagged_qng |= tid_bit; - asc_dvc->cfg->can_tagged_qng |= tid_bit; - } - } - } -if (orig_use_tagged_qng != asc_dvc->use_tagged_qng) { -AscWriteLramByte (asc_dvc->iop_base,ASCV_DISC_ENABLE_B , - asc_dvc->cfg->disc_enable); -AscWriteLramByte (asc_dvc->iop_base,ASCV_USE_TAGGED_QNG_B , - asc_dvc->use_tagged_qng); -AscWriteLramByte (asc_dvc->iop_base,ASCV_CAN_TAGGED_QNG_B , - asc_dvc->cfg->can_tagged_qng); - - asc_dvc->max_dvc_qng[tid_no] = - asc_dvc->cfg->max_tag_qng[tid_no]; -AscWriteLramByte (asc_dvc->iop_base, - (ushort)(ASCV_MAX_DVC_QNG_BEG + tid_no), - asc_dvc->max_dvc_qng[tid_no]); - } -if (orig_init_sdtr != asc_dvc->init_sdtr) { -AscAsyncFix (asc_dvc, tid_no, inq); - } -return ; }static uchar AscReadLramByte (PortAddr iop_base,ushort addr)
@@ -14158,7 +14122,7 @@ static int AdvInitAsc3550Driver(ADV_DVC_VAR *asc_dvc)/* * Microcode operating variables for WDTR, SDTR, and command tag -* queuing will be set in AdvInquiryHandling() based on what a +* queuing will be set in slave_configure() based on what a * device reports it is capable of in Inquiry byte 7. * * If SCSI Bus Resets have been disabled, then directly set
@@ -14809,7 +14773,7 @@ static int AdvInitAsc38C0800Driver(ADV_DVC_VAR *asc_dvc)/* * Microcode operating variables for WDTR, SDTR, and command tag -* queuing will be set in AdvInquiryHandling() based on what a +* queuing will be set in slave_configure() based on what a * device reports it is capable of in Inquiry byte 7. * * If SCSI Bus Resets have been disabled, then directly set
@@ -15429,7 +15393,7 @@ static int AdvInitAsc38C1600Driver(ADV_DVC_VAR *asc_dvc)/* * Microcode operating variables for WDTR, SDTR, and command tag -* queuing will be set in AdvInquiryHandling() based on what a +* queuing will be set in slave_configure() based on what a * device reports it is capable of in Inquiry byte 7. * * If SCSI Bus Resets have been disabled, then directly set
@@ -17113,23 +17077,6 @@ static int AdvISR(ADV_DVC_VAR *asc_dvc) scsiq->cntl =0 ;/* -* If the command that completed was a SCSI INQUIRY and -* LUN 0 was sent the command, then process the INQUIRY -* command information for the device. -* -* Note: If data returned were either VPD or CmdDt data, -* don't process the INQUIRY command information for -* the device, otherwise may erroneously set *_able bits. -*/ -if (scsiq->done_status ==QD_NO_ERROR && - scsiq->cdb[0 ] ==INQUIRY && - scsiq->target_lun ==0 && - (scsiq->cdb[1 ] &ADV_INQ_RTN_VPD_AND_CMDDT ) - ==ADV_INQ_RTN_STD_INQUIRY_DATA ) { -AdvInquiryHandling (asc_dvc, scsiq); - } - -/* * Notify the driver of the completed request by passing * the ADV_SCSI_REQ_Q pointer to its callback function. */
@@ -17234,168 +17181,6 @@ AdvSendIdleCmd(ADV_DVC_VAR *asc_dvc,return ADV_ERROR ; } -/* -* Inquiry Information Byte 7 Handling -* -* Handle SCSI Inquiry Command information for a device by setting -* microcode operating variables that affect WDTR, SDTR, and Tag -* Queuing. -*/ -static void AdvInquiryHandling (ADV_DVC_VAR *asc_dvc,ADV_SCSI_REQ_Q *scsiq) -{ -AdvPortAddr iop_base; -uchar tid; -ADV_SCSI_INQUIRY *inq; -ushort tidmask; -ushort cfg_word; - -/* -* AdvInquiryHandling() requires up to INQUIRY information Byte 7 -* to be available. -* -* If less than 8 bytes of INQUIRY information were requested or less -* than 8 bytes were transferred, then return. cdb[4] is the request -* length and the ADV_SCSI_REQ_Q 'data_cnt' field is set by the -* microcode to the transfer residual count. -*/ - -if (scsiq->cdb[4 ] <8 || - (scsiq->cdb[4 ] -le32_to_cpu (scsiq->data_cnt)) <8 ) { -return ; - } - - iop_base = asc_dvc->iop_base; - tid = scsiq->target_id; - - inq = (ADV_SCSI_INQUIRY *) scsiq->vdata_addr; - -/* -* WDTR, SDTR, and Tag Queuing cannot be enabled for old devices. -*/ -if (ADV_INQ_RESPONSE_FMT (inq) <2 &&ADV_INQ_ANSI_VER (inq) <2 ) { -return ; - }else { -/* -* INQUIRY Byte 7 Handling -* -* Use a device's INQUIRY byte 7 to determine whether it -* supports WDTR, SDTR, and Tag Queuing. If the feature -* is enabled in the EEPROM and the device supports the -* feature, then enable it in the microcode. -*/ - - tidmask =ADV_TID_TO_TIDMASK (tid); - -/* -* Wide Transfers -* -* If the EEPROM enabled WDTR for the device and the device -* supports wide bus (16 bit) transfers, then turn on the -* device's 'wdtr_able' bit and write the new value to the -* microcode. -*/ -if ((asc_dvc->wdtr_able & tidmask) &&ADV_INQ_WIDE16 (inq)) { -AdvReadWordLram (iop_base,ASC_MC_WDTR_ABLE , cfg_word); -if ((cfg_word & tidmask) ==0 ) { - cfg_word |= tidmask; -AdvWriteWordLram (iop_base,ASC_MC_WDTR_ABLE , - cfg_word); - -/* -* Clear the microcode "SDTR negotiation" and "WDTR -* negotiation" done indicators for the target to cause -* it to negotiate with the new setting set above. -* WDTR when accepted causes the target to enter -* asynchronous mode, so SDTR must be negotiated. -*/ -AdvReadWordLram (iop_base,ASC_MC_SDTR_DONE , - cfg_word); - cfg_word &= ~tidmask; -AdvWriteWordLram (iop_base,ASC_MC_SDTR_DONE , - cfg_word); -AdvReadWordLram (iop_base,ASC_MC_WDTR_DONE , - cfg_word); - cfg_word &= ~tidmask; -AdvWriteWordLram (iop_base,ASC_MC_WDTR_DONE , - cfg_word); - } - } - -/* -* Synchronous Transfers -* -* If the EEPROM enabled SDTR for the device and the device -* supports synchronous transfers, then turn on the device's -* 'sdtr_able' bit. Write the new value to the microcode. -*/ -if ((asc_dvc->sdtr_able & tidmask) &&ADV_INQ_SYNC (inq)) { -AdvReadWordLram (iop_base,ASC_MC_SDTR_ABLE , cfg_word); -if ((cfg_word & tidmask) ==0 ) { - cfg_word |= tidmask; -AdvWriteWordLram (iop_base,ASC_MC_SDTR_ABLE , - cfg_word); - -/* -* Clear the microcode "SDTR negotiation" done indicator -* for the target to cause it to negotiate with the new -* setting set above. -*/ -AdvReadWordLram (iop_base,ASC_MC_SDTR_DONE , - cfg_word); - cfg_word &= ~tidmask; -AdvWriteWordLram (iop_base,ASC_MC_SDTR_DONE , - cfg_word); - } - } -/* -* If the Inquiry data included enough space for the SPI-3 -* Clocking field, then check if DT mode is supported. -*/ -if (asc_dvc->chip_type ==ADV_CHIP_ASC38C1600 && - (scsiq->cdb[4 ] >=57 || - (scsiq->cdb[4 ] -le32_to_cpu (scsiq->data_cnt)) >=57 )) { -/* -* PPR (Parallel Protocol Request) Capable -* -* If the device supports DT mode, then it must be PPR capable. -* The PPR message will be used in place of the SDTR and WDTR -* messages to negotiate synchronous speed and offset, transfer -* width, and protocol options. -*/ -if (ADV_INQ_CLOCKING (inq) &ADV_INQ_CLOCKING_DT_ONLY ) { -AdvReadWordLram (iop_base,ASC_MC_PPR_ABLE , - asc_dvc->ppr_able); - asc_dvc->ppr_able |= tidmask; -AdvWriteWordLram (iop_base,ASC_MC_PPR_ABLE , - asc_dvc->ppr_able); - } - } - -/* -* If the EEPROM enabled Tag Queuing for the device and the -* device supports Tag Queueing, then turn on the device's -* 'tagqng_enable' bit in the microcode and set the microcode -* maximum command count to the ADV_DVC_VAR 'max_dvc_qng' -* value. -* -* Tag Queuing is disabled for the BIOS which runs in polled -* mode and would see no benefit from Tag Queuing. Also by -* disabling Tag Queuing in the BIOS devices with Tag Queuing -* bugs will at least work with the BIOS. -*/ -if ((asc_dvc->tagqng_able & tidmask) &&ADV_INQ_CMD_QUEUE (inq)) { -AdvReadWordLram (iop_base,ASC_MC_TAGQNG_ABLE , cfg_word); - cfg_word |= tidmask; -AdvWriteWordLram (iop_base,ASC_MC_TAGQNG_ABLE , - cfg_word); - -AdvWriteByteLram (iop_base, -ASC_MC_NUMBER_OF_MAX_CMD + tid, - asc_dvc->max_dvc_qng); - } - } -} -static int __devinit advansys_wide_init_chip (asc_board_t *boardp,ADV_DVC_VAR *adv_dvc_varp) {
Signature · 1 line
1.4.4.4
PATCH 13/21 advansys: misc reformatting
In reply to Matthew WilcoxRemove some useless forward declarations
Reformat some comments, debug messages, and the occasional piece of real code Removal of unnecessary braces
Don't need to initialise share_irq
Remove duplicate setting of shost->irq
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 276 ++++++++++++++++++++--------------------------- 1 files changed, 117 insertions(+), 159 deletions(-)
drivers/scsi/advansys.c +117 -159
@@ -1826,9 +1826,6 @@ static void AscEnableInterrupt(PortAddr);static void AscSetBank (PortAddr ,uchar );static int AscResetChipAndScsiBus (ASC_DVC_VAR *);#ifdef CONFIG_ISA -static ushort AscGetIsaDmaChannel (PortAddr ); -static ushort AscSetIsaDmaChannel (PortAddr ,ushort ); -static uchar AscSetIsaDmaSpeed (PortAddr ,uchar );static uchar AscGetIsaDmaSpeed (PortAddr );#endif /* CONFIG_ISA */ static uchar AscReadLramByte (PortAddr ,ushort );
@@ -1875,18 +1872,11 @@ static PortAddr AscSearchIOPortAddr11(PortAddr);static void AscSetISAPNPWaitForKey (void );#endif /* CONFIG_ISA */ static uchar AscGetChipScsiCtrl (PortAddr ); -static uchar AscSetChipScsiID (PortAddr ,uchar );static uchar AscGetChipVersion (PortAddr ,ushort ); -static ushort AscGetChipBusType (PortAddr );static ASC_DCNT AscLoadMicroCode (PortAddr ,ushort ,uchar *,ushort ); -static int AscFindSignature (PortAddr );static void AscToggleIRQAct (PortAddr ); -static uchar AscGetChipIRQ (PortAddr ,ushort ); -static uchar AscSetChipIRQ (PortAddr ,uchar ,ushort ); -static ushort AscGetChipBiosAddress (PortAddr ,ushort );static inline ulong DvcEnterCritical (void );static inline void DvcLeaveCritical (ulong ); -static ushort AscGetChipBiosAddress (PortAddr ,ushort );static void DvcSleepMilliSecond (ASC_DCNT );static void DvcDelayNanoSecond (ASC_DVC_VAR *,ASC_DCNT );static void DvcPutScsiQ (PortAddr ,ushort ,uchar *,int );
@@ -1900,7 +1890,6 @@ static int AscSgListToQueue(int);#ifdef CONFIG_ISA static void AscEnableIsaDma (uchar );#endif /* CONFIG_ISA */ -static ASC_DCNT AscGetMaxDmaCount (ushort );static const char *advansys_info (struct Scsi_Host *shost);/*
@@ -3754,10 +3743,6 @@ static int asc_prt_adv_board_info(struct Scsi_Host *, char *, int);static int asc_prt_line (char *,int ,char *fmt, ...);#endif /* CONFIG_PROC_FS */ -/* Declaration for Asc Library internal functions referenced by driver. */ -static int AscFindSignature (PortAddr ); -static ushort AscGetEEPConfig (PortAddr ,ASCEEP_CONFIG *,ushort ); -/* Statistics function prototypes. */ #ifdef ADVANSYS_STATS #ifdef CONFIG_PROC_FS
@@ -3866,8 +3851,7 @@ advansys_proc_info(struct Scsi_Host *shost, char *buffer, char **start, cp = boardp->prtbuf; cplen =asc_prt_adv_bios (shost, cp,ASC_PRTBUF_SIZE );ASC_ASSERT (cplen <ASC_PRTBUF_SIZE ); - cnt = -asc_proc_copy (advoffset, offset, curbuf, leftlen, cp, + cnt =asc_proc_copy (advoffset, offset, curbuf, leftlen, cp, cplen); totcnt += cnt; leftlen -= cnt;
@@ -3954,11 +3938,10 @@ advansys_proc_info(struct Scsi_Host *shost, char *buffer, char **start,for (tgt_id =0 ; tgt_id <=ADV_MAX_TID ; tgt_id++) { cp = boardp->prtbuf; cplen =asc_prt_target_stats (shost, tgt_id, cp, -ASC_PRTBUF_SIZE ); +ASC_PRTBUF_SIZE );ASC_ASSERT (cplen <=ASC_PRTBUF_SIZE ); - cnt = -asc_proc_copy (advoffset, offset, curbuf, leftlen, cp, - cplen); + cnt =asc_proc_copy (advoffset, offset, curbuf, leftlen, cp, + cplen); totcnt += cnt; leftlen -= cnt;if (leftlen ==0 ) {
@@ -4048,15 +4031,14 @@ static const char *advansys_info(struct Scsi_Host *shost) } }else { busname ="?" ; -ASC_PRINT2 - ("advansys_info: board %d: unknown bus type %d\n" , - boardp->id, asc_dvc_varp->bus_type); +ASC_PRINT2 ("advansys_info: board %d: unknown " +"bus type %d\n" , boardp->id, + asc_dvc_varp->bus_type); }/* Don't reference 'shost->n_io_port'; It may be truncated. */ sprintf (info,"AdvanSys SCSI %s: %s: IO 0x%lX-0x%lX, IRQ 0x%X" , -ASC_VERSION , busname, - (ulong )shost->io_port, +ASC_VERSION , busname, (ulong )shost->io_port, (ulong )shost->io_port + boardp->asc_n_io_port -1 , shost->irq); }
@@ -4237,18 +4219,16 @@ static int advansys_reset(struct scsi_cmnd *scp)/* Refer to ASC_IERR_* defintions for meaning of 'err_code'. */ if (asc_dvc_varp->err_code) { -ASC_PRINT2 - ("advansys_reset: board %d: SCSI bus reset error: 0x%x\n" , - boardp->id, asc_dvc_varp->err_code); +ASC_PRINT2 ("advansys_reset: board %d: SCSI bus reset " +"error: 0x%x\n" , boardp->id, + asc_dvc_varp->err_code); ret =FAILED ; }else if (status) { -ASC_PRINT2 - ("advansys_reset: board %d: SCSI bus reset warning: 0x%x\n" , - boardp->id, status); +ASC_PRINT2 ("advansys_reset: board %d: SCSI bus reset " +"warning: 0x%x\n" , boardp->id, status); }else { -ASC_PRINT1 - ("advansys_reset: board %d: SCSI bus reset successful.\n" , - boardp->id); +ASC_PRINT1 ("advansys_reset: board %d: SCSI bus reset " +"successful.\n" , boardp->id); }ASC_DBG (1 ,"advansys_reset: after AscInitAsc1000Driver()\n" );
@@ -4269,15 +4249,13 @@ static int advansys_reset(struct scsi_cmnd *scp)ASC_DBG (1 ,"advansys_reset: before AdvResetChipAndSB()\n" );switch (AdvResetChipAndSB (adv_dvc_varp)) {case ASC_TRUE : -ASC_PRINT1 - ("advansys_reset: board %d: SCSI bus reset successful.\n" , - boardp->id); +ASC_PRINT1 ("advansys_reset: board %d: SCSI bus reset " +"successful.\n" , boardp->id);break ;case ASC_FALSE :default : -ASC_PRINT1 - ("advansys_reset: board %d: SCSI bus reset error.\n" , - boardp->id); +ASC_PRINT1 ("advansys_reset: board %d: SCSI bus reset " +"error.\n" , boardp->id); ret =FAILED ;break ; }
@@ -4298,8 +4276,8 @@ static int advansys_reset(struct scsi_cmnd *scp)* is returned in 'last_scp'. */ if (done_scp ==NULL ) { - done_scp = -asc_dequeue_list (&boardp->active, &last_scp,ASC_TID_ALL ); + done_scp =asc_dequeue_list (&boardp->active, &last_scp, +ASC_TID_ALL );for (tscp = done_scp; tscp; tscp =REQPNEXT (tscp)) { tscp->result =HOST_BYTE (DID_RESET ); }
@@ -4325,8 +4303,8 @@ static int advansys_reset(struct scsi_cmnd *scp)* to DID_RESET. */ if (done_scp ==NULL ) { - done_scp = -asc_dequeue_list (&boardp->waiting, &last_scp,ASC_TID_ALL ); + done_scp =asc_dequeue_list (&boardp->waiting, &last_scp, +ASC_TID_ALL );for (tscp = done_scp; tscp; tscp =REQPNEXT (tscp)) { tscp->result =HOST_BYTE (DID_RESET ); }
@@ -4357,9 +4335,8 @@ static int advansys_reset(struct scsi_cmnd *scp)/* * Complete all the 'done_scp' requests. */ -if (done_scp !=NULL ) { +if (done_scp)asc_scsi_done_list (done_scp); - }ASC_DBG1 (1 ,"advansys_reset: ret %d\n" , ret);
@@ -4500,14 +4477,13 @@ static irqreturn_t advansys_interrupt(int irq, void *dev_id)* handler will complete pending requests after it has completed. */ if ((boardp->flags &ASC_HOST_IN_RESET ) ==0 ) { -ASC_DBG2 (1 , -"advansys_interrupt: done_scp 0x%lx, last_scp 0x%lx\n" , - (ulong )done_scp, (ulong )last_scp); +ASC_DBG2 (1 ,"advansys_interrupt: done_scp 0x%p, " +"last_scp 0x%p\n" , done_scp, last_scp);/* Start any waiting commands for the board. */ if (!ASC_QUEUE_EMPTY (&boardp->waiting)) { -ASC_DBG (1 , -"advansys_interrupt: before asc_execute_queue()\n" ); +ASC_DBG (1 ,"advansys_interrupt: before " +"asc_execute_queue()\n" );asc_execute_queue (&boardp->waiting); }
@@ -4519,9 +4495,8 @@ static irqreturn_t advansys_interrupt(int irq, void *dev_id)* 'done_scp'. */ if (done_scp ==NULL ) { - done_scp = -asc_dequeue_list (&boardp->done, &last_scp, -ASC_TID_ALL ); + done_scp =asc_dequeue_list (&boardp->done, + &last_scp,ASC_TID_ALL ); }else {ASC_ASSERT (last_scp !=NULL ); last_scp->host_scribble =
@@ -4888,33 +4863,33 @@ static int asc_execute_scsi_cmnd(struct scsi_cmnd *scp)case ASC_NOERROR :ASC_STATS (scp->device->host, exe_noerror);/* -* Increment monotonically increasing per device successful -* request counter. Wrapping doesn't matter. +* Increment monotonically increasing per device +* successful request counter. Wrapping doesn't matter. */ boardp->reqcnt[scp->device->id]++;asc_enqueue (&boardp->active, scp,ASC_BACK ); -ASC_DBG (1 , -"asc_execute_scsi_cmnd: AscExeScsiQueue(), ASC_NOERROR\n" ); +ASC_DBG (1 ,"asc_execute_scsi_cmnd: AscExeScsiQueue(), " +"ASC_NOERROR\n" );break ;case ASC_BUSY :/* -* Caller will enqueue request on the target's waiting queue -* and retry later. +* Caller will enqueue request on the target's waiting +* queue and retry later. */ ASC_STATS (scp->device->host, exe_busy);break ;case ASC_ERROR : -ASC_PRINT2 - ("asc_execute_scsi_cmnd: board %d: AscExeScsiQueue() ASC_ERROR, err_code 0x%x\n" , - boardp->id, asc_dvc_varp->err_code); +ASC_PRINT2 ("asc_execute_scsi_cmnd: board %d: " +"AscExeScsiQueue() ASC_ERROR, err_code 0x%x\n" , + boardp->id, asc_dvc_varp->err_code);ASC_STATS (scp->device->host, exe_error); scp->result =HOST_BYTE (DID_ERROR );asc_enqueue (&boardp->done, scp,ASC_BACK );break ;default : -ASC_PRINT2 - ("asc_execute_scsi_cmnd: board %d: AscExeScsiQueue() unknown, err_code 0x%x\n" , - boardp->id, asc_dvc_varp->err_code); +ASC_PRINT2 ("asc_execute_scsi_cmnd: board %d: " +"AscExeScsiQueue() unknown, err_code 0x%x\n" , + boardp->id, asc_dvc_varp->err_code);ASC_STATS (scp->device->host, exe_unknown); scp->result =HOST_BYTE (DID_ERROR );asc_enqueue (&boardp->done, scp,ASC_BACK );
@@ -4934,20 +4909,21 @@ static int asc_execute_scsi_cmnd(struct scsi_cmnd *scp)*/ switch (adv_build_req (boardp, scp, &adv_scsiqp)) {case ASC_NOERROR : -ASC_DBG (3 , -"asc_execute_scsi_cmnd: adv_build_req ASC_NOERROR\n" ); +ASC_DBG (3 ,"asc_execute_scsi_cmnd: adv_build_req " +"ASC_NOERROR\n" );break ;case ASC_BUSY : -ASC_DBG (1 , -"asc_execute_scsi_cmnd: adv_build_req ASC_BUSY\n" ); +ASC_DBG (1 ,"asc_execute_scsi_cmnd: adv_build_req " +"ASC_BUSY\n" );/* -* If busy is returned the request has not been enqueued. -* It will be enqueued by the caller on the target's waiting -* queue and retried later. +* If busy is returned the request has not been +* enqueued. It will be enqueued by the caller on the +* target's waiting queue and retried later. * -* The asc_stats fields 'adv_build_noreq' and 'adv_build_nosg' -* count wide board busy conditions. They are updated in -* adv_build_req and adv_get_sglist, respectively. +* The asc_stats fields 'adv_build_noreq' and +* 'adv_build_nosg' count wide board busy conditions. +* They are updated in adv_build_req and +* adv_get_sglist, respectively. */ return ASC_BUSY ;case ASC_ERROR :
@@ -4957,8 +4933,8 @@ static int asc_execute_scsi_cmnd(struct scsi_cmnd *scp)* by the caller. */ default : -ASC_DBG (1 , -"asc_execute_scsi_cmnd: adv_build_req ASC_ERROR\n" ); +ASC_DBG (1 ,"asc_execute_scsi_cmnd: adv_build_req " +"ASC_ERROR\n" );ASC_STATS (scp->device->host, build_error);return ASC_ERROR ; }
@@ -4971,33 +4947,33 @@ static int asc_execute_scsi_cmnd(struct scsi_cmnd *scp)case ASC_NOERROR :ASC_STATS (scp->device->host, exe_noerror);/* -* Increment monotonically increasing per device successful -* request counter. Wrapping doesn't matter. +* Increment monotonically increasing per device +* successful request counter. Wrapping doesn't matter. */ boardp->reqcnt[scp->device->id]++;asc_enqueue (&boardp->active, scp,ASC_BACK ); -ASC_DBG (1 , -"asc_execute_scsi_cmnd: AdvExeScsiQueue(), ASC_NOERROR\n" ); +ASC_DBG (1 ,"asc_execute_scsi_cmnd: AdvExeScsiQueue(), " +"ASC_NOERROR\n" );break ;case ASC_BUSY :/* -* Caller will enqueue request on the target's waiting queue -* and retry later. +* Caller will enqueue request on the target's waiting +* queue and retry later. */ ASC_STATS (scp->device->host, exe_busy);break ;case ASC_ERROR : -ASC_PRINT2 - ("asc_execute_scsi_cmnd: board %d: AdvExeScsiQueue() ASC_ERROR, err_code 0x%x\n" , - boardp->id, adv_dvc_varp->err_code); +ASC_PRINT2 ("asc_execute_scsi_cmnd: board %d: " +"AdvExeScsiQueue() ASC_ERROR, err_code 0x%x\n" , + boardp->id, adv_dvc_varp->err_code);ASC_STATS (scp->device->host, exe_error); scp->result =HOST_BYTE (DID_ERROR );asc_enqueue (&boardp->done, scp,ASC_BACK );break ;default : -ASC_PRINT2 - ("asc_execute_scsi_cmnd: board %d: AdvExeScsiQueue() unknown, err_code 0x%x\n" , - boardp->id, adv_dvc_varp->err_code); +ASC_PRINT2 ("asc_execute_scsi_cmnd: board %d: " +"AdvExeScsiQueue() unknown, err_code 0x%x\n" , + boardp->id, adv_dvc_varp->err_code);ASC_STATS (scp->device->host, exe_unknown); scp->result =HOST_BYTE (DID_ERROR );asc_enqueue (&boardp->done, scp,ASC_BACK );
@@ -5040,9 +5016,9 @@ static int asc_build_req(asc_board_t *boardp, struct scsi_cmnd *scp)* is supported. */ if (scp->cmd_len >ASC_MAX_CDB_LEN ) { -ASC_PRINT3 - ("asc_build_req: board %d: cmd_len %d > ASC_MAX_CDB_LEN %d\n" , - boardp->id, scp->cmd_len,ASC_MAX_CDB_LEN ); +ASC_PRINT3 ("asc_build_req: board %d: cmd_len %d > " +"ASC_MAX_CDB_LEN %d\n" , boardp->id, scp->cmd_len, +ASC_MAX_CDB_LEN ); scp->result =HOST_BYTE (DID_ERROR );asc_enqueue (&boardp->done, scp,ASC_BACK );return ASC_ERROR ;
@@ -7427,27 +7403,27 @@ DvcGetQinfo(PortAddr iop_base, ushort s_addr, uchar *inbuf, int words)* Return the BIOS address of the adapter at the specified * I/O port and with the specified bus type. */ -static ushort __devinit AscGetChipBiosAddress (PortAddr iop_base,ushort bus_type) +static unsigned short __devinit +AscGetChipBiosAddress (PortAddr iop_base,unsigned short bus_type) { -ushort cfg_lsw; -ushort bios_addr; +unsigned short cfg_lsw; +unsigned short bios_addr;/* * The PCI BIOS is re-located by the motherboard BIOS. Because * of this the driver can not determine where a PCI BIOS is * loaded and executes. */ -if (bus_type &ASC_IS_PCI ) { -return (0 ); - } +if (bus_type &ASC_IS_PCI ) +return 0 ; +#ifdef CONFIG_ISA if ((bus_type &ASC_IS_EISA ) !=0 ) { cfg_lsw =AscGetEisaChipCfg (iop_base); cfg_lsw &=0x000F ; - bios_addr = (ushort)(ASC_BIOS_MIN_ADDR + - (cfg_lsw *ASC_BIOS_BANK_SIZE )); -return (bios_addr); - }/* if */ + bios_addr =ASC_BIOS_MIN_ADDR + cfg_lsw *ASC_BIOS_BANK_SIZE ; +return bios_addr; + }#endif /* CONFIG_ISA */ cfg_lsw =AscGetChipCfgLsw (iop_base);
@@ -7455,13 +7431,10 @@ static ushort __devinit AscGetChipBiosAddress(PortAddr iop_base, ushort bus_type/* * ISA PnP uses the top bit as the 32K BIOS flag */ -if (bus_type ==ASC_IS_ISAPNP ) { +if (bus_type ==ASC_IS_ISAPNP ) cfg_lsw &=0x7FFF ; - } -/* if */ - bios_addr = (ushort)(((cfg_lsw >>12 ) *ASC_BIOS_BANK_SIZE ) + -ASC_BIOS_MIN_ADDR ); -return (bios_addr); + bios_addr =ASC_BIOS_MIN_ADDR + (cfg_lsw >>12 ) *ASC_BIOS_BANK_SIZE ; +return bios_addr; }/*
@@ -7758,10 +7731,8 @@ static void asc_prt_scsi_cmnd(struct scsi_cmnd *s)printk (" timeout_per_command %d\n" , s->timeout_per_command); -printk - (" scsi_done 0x%lx, done 0x%lx, host_scribble 0x%lx, result 0x%x\n" , - (ulong )s->scsi_done, (ulong )s->done, (ulong )s->host_scribble, - s->result); +printk (" scsi_done 0x%p, done 0x%p, host_scribble 0x%p, result 0x%x\n" , + s->scsi_done, s->done, s->host_scribble, s->result);printk (" tag %u, pid %u\n" , (unsigned )s->tag, (unsigned )s->pid); }
@@ -7773,34 +7744,31 @@ static void asc_prt_asc_dvc_var(ASC_DVC_VAR *h) {printk ("ASC_DVC_VAR at addr 0x%lx\n" , (ulong )h); -printk - (" iop_base 0x%x, err_code 0x%x, dvc_cntl 0x%x, bug_fix_cntl %d,\n" , - h->iop_base, h->err_code, h->dvc_cntl, h->bug_fix_cntl); +printk (" iop_base 0x%x, err_code 0x%x, dvc_cntl 0x%x, bug_fix_cntl " +"%d,\n" , h->iop_base, h->err_code, h->dvc_cntl, h->bug_fix_cntl); -printk - (" bus_type %d, isr_callback 0x%lx, exe_callback 0x%lx, init_sdtr 0x%x,\n" , - h->bus_type, (ulong )h->isr_callback, (ulong )h->exe_callback, - (unsigned )h->init_sdtr); +printk (" bus_type %d, isr_callback 0x%p, exe_callback 0x%p, " +"init_sdtr 0x%x,\n" , h->bus_type, h->isr_callback, + h->exe_callback, (unsigned )h->init_sdtr); -printk - (" sdtr_done 0x%x, use_tagged_qng 0x%x, unit_not_ready 0x%x, chip_no 0x%x,\n" , - (unsigned )h->sdtr_done, (unsigned )h->use_tagged_qng, - (unsigned )h->unit_not_ready, (unsigned )h->chip_no); +printk (" sdtr_done 0x%x, use_tagged_qng 0x%x, unit_not_ready 0x%x, " +"chip_no 0x%x,\n" , (unsigned )h->sdtr_done, + (unsigned )h->use_tagged_qng, (unsigned )h->unit_not_ready, + (unsigned )h->chip_no); -printk - (" queue_full_or_busy 0x%x, start_motor 0x%x, scsi_reset_wait %u,\n" , - (unsigned )h->queue_full_or_busy, (unsigned )h->start_motor, - (unsigned )h->scsi_reset_wait); +printk (" queue_full_or_busy 0x%x, start_motor 0x%x, scsi_reset_wait " +"%u,\n" , (unsigned )h->queue_full_or_busy, + (unsigned )h->start_motor, (unsigned )h->scsi_reset_wait); -printk - (" is_in_int %u, max_total_qng %u, cur_total_qng %u, in_critical_cnt %u,\n" , - (unsigned )h->is_in_int, (unsigned )h->max_total_qng, - (unsigned )h->cur_total_qng, (unsigned )h->in_critical_cnt); +printk (" is_in_int %u, max_total_qng %u, cur_total_qng %u, " +"in_critical_cnt %u,\n" , (unsigned )h->is_in_int, + (unsigned )h->max_total_qng, (unsigned )h->cur_total_qng, + (unsigned )h->in_critical_cnt); -printk - (" last_q_shortage %u, init_state 0x%x, no_scam 0x%x, pci_fix_asyn_xfer 0x%x,\n" , - (unsigned )h->last_q_shortage, (unsigned )h->init_state, - (unsigned )h->no_scam, (unsigned )h->pci_fix_asyn_xfer); +printk (" last_q_shortage %u, init_state 0x%x, no_scam 0x%x, " +"pci_fix_asyn_xfer 0x%x,\n" , (unsigned )h->last_q_shortage, + (unsigned )h->init_state, (unsigned )h->no_scam, + (unsigned )h->pci_fix_asyn_xfer);printk (" cfg 0x%lx, irq_no 0x%x\n" , (ulong )h->cfg, (unsigned )h->irq_no); }
@@ -8104,27 +8072,28 @@ static uchar __devinit AscSetChipScsiID(PortAddr iop_base, uchar new_host_id)return (AscGetChipScsiID (iop_base)); } -static uchar __devinit AscGetChipScsiCtrl (PortAddr iop_base) +static unsigned char __devinit AscGetChipScsiCtrl (PortAddr iop_base) { -uchar sc; +unsigned char sc;AscSetBank (iop_base,1 ); sc =inp (iop_base +IOP_REG_SC );AscSetBank (iop_base,0 ); -return (sc); +return sc; } -static uchar __devinit AscGetChipVersion (PortAddr iop_base,ushort bus_type) +static unsigned char __devinit +AscGetChipVersion (PortAddr iop_base,unsigned short bus_type) { -if ((bus_type &ASC_IS_EISA ) !=0 ) { +if (bus_type &ASC_IS_EISA ) {PortAddr eisa_iop; -uchar revision; +unsigned char revision; eisa_iop = (PortAddr )ASC_GET_EISA_SLOT (iop_base) | (PortAddr )ASC_EISA_REV_IOP_MASK ; revision =inp (eisa_iop); -return ((uchar)((ASC_CHIP_MIN_VER_EISA -1 ) + revision)); +return ASC_CHIP_MIN_VER_EISA -1 + revision; } -return (AscGetChipVerNo (iop_base)); +return AscGetChipVerNo (iop_base); }static ushort __devinit AscGetChipBusType (PortAddr iop_base)
@@ -10441,13 +10410,12 @@ static ushort __devinit AscInitGetConfig(ASC_DVC_VAR *asc_dvc) warn_code |=AscInitAscDvcVar (asc_dvc); warn_code |=AscInitFromEEP (asc_dvc); asc_dvc->init_state |=ASC_INIT_STATE_END_GET_CFG ; -if (asc_dvc->scsi_reset_wait >ASC_MAX_SCSI_RESET_WAIT ) { +if (asc_dvc->scsi_reset_wait >ASC_MAX_SCSI_RESET_WAIT ) asc_dvc->scsi_reset_wait =ASC_MAX_SCSI_RESET_WAIT ; - } }else { asc_dvc->err_code =ASC_IERR_BAD_SIGNATURE ; } -return (warn_code); +return warn_code; }static ushort __devinit AscInitSetConfig (ASC_DVC_VAR *asc_dvc)
@@ -17296,21 +17264,18 @@ advansys_board_found(int iop, struct device *dev, int bus_type)ASC_DVC_VAR *asc_dvc_varp =NULL ;ADV_DVC_VAR *adv_dvc_varp =NULL ;adv_sgblk_t *sgp; -int share_irq = FALSE; +int share_irq;int iolen =0 ;ADV_PADDR pci_memory_address;int warn_code, err_code;int ret;/* -* Adapter found. -* * Register the adapter, get its configuration, and * initialize it. */ ASC_DBG (2 ,"advansys_board_found: scsi_host_alloc()\n" ); shost =scsi_host_alloc (&advansys_template,sizeof (asc_board_t)); -if (!shost)return NULL ;
@@ -17321,8 +17286,6 @@ advansys_board_found(int iop, struct device *dev, int bus_type) boardp =ASC_BOARDP (shost);memset (boardp,0 ,sizeof (asc_board_t)); boardp->id = asc_board_count -1 ; - -/* Initialize spinlock. */ spin_lock_init (&boardp->lock);/*
@@ -17742,11 +17705,6 @@ advansys_board_found(int iop, struct device *dev, int bus_type)*/ boardp->init_tidmask |=ADV_TID_TO_TIDMASK (adv_dvc_varp->chip_scsi_id); - -/* -* Finish initializing the 'Scsi_Host' structure. -*/ - shost->irq = adv_dvc_varp->irq_no; }/*
Signature · 1 line
1.4.4.4
PATCH 14/21 advansys: delete AscGetChipBusType
In reply to Matthew WilcoxBy moving a test from AscGetChipBusType into its only caller, we can delete the whole function
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 36 ++++-------------------------------- 1 files changed, 4 insertions(+), 32 deletions(-)
drivers/scsi/advansys.c +4 -32
@@ -8096,34 +8096,6 @@ AscGetChipVersion(PortAddr iop_base, unsigned short bus_type)return AscGetChipVerNo (iop_base); } -static ushort __devinit AscGetChipBusType (PortAddr iop_base) -{ -ushort chip_ver; - - chip_ver =AscGetChipVerNo (iop_base); -if ((chip_ver >=ASC_CHIP_MIN_VER_VL ) - && (chip_ver <=ASC_CHIP_MAX_VER_VL ) - ) { -if (((iop_base &0x0C30 ) ==0x0C30 ) - || ((iop_base &0x0C50 ) ==0x0C50 ) - ) { -return (ASC_IS_EISA ); - } -return (ASC_IS_VL ); - } -if ((chip_ver >=ASC_CHIP_MIN_VER_ISA ) && - (chip_ver <=ASC_CHIP_MAX_VER_ISA )) { -if (chip_ver >=ASC_CHIP_MIN_VER_ISA_PNP ) { -return (ASC_IS_ISAPNP ); - } -return (ASC_IS_ISA ); - }else if ((chip_ver >=ASC_CHIP_MIN_VER_PCI ) && - (chip_ver <=ASC_CHIP_MAX_VER_PCI )) { -return (ASC_IS_PCI ); - } -return (0 ); -} -static ASC_DCNT AscLoadMicroCode (PortAddr iop_base,ushort s_addr,uchar *mcode_buf,ushort mcode_size)
@@ -10626,12 +10598,12 @@ static ushort __devinit AscInitAscDvcVar(ASC_DVC_VAR *asc_dvc) } asc_dvc->cfg->isa_dma_speed =ASC_DEF_ISA_DMA_SPEED ; -if (AscGetChipBusType (iop_base) ==ASC_IS_ISAPNP ) { -AscSetChipIFC (iop_base,IFC_INIT_DEFAULT ); - asc_dvc->bus_type =ASC_IS_ISAPNP ; - }#ifdef CONFIG_ISA if ((asc_dvc->bus_type &ASC_IS_ISA ) !=0 ) { +if (chip_version >=ASC_CHIP_MIN_VER_ISA_PNP ) { +AscSetChipIFC (iop_base,IFC_INIT_DEFAULT ); + asc_dvc->bus_type =ASC_IS_ISAPNP ; + } asc_dvc->cfg->isa_dma_channel = (uchar )AscGetIsaDmaChannel (iop_base); }
Signature · 1 line
1.4.4.4
PATCH 15/21 advansys: ioremap no longer needs page-aligned addresses
In reply to Matthew WilcoxAt some point during Linux 2.1 development, ioremap() gained the ability to handle addresses which weren't page-aligned. Also expand the CONFIG_PCI range to encompass that entire section of wide board initialisation, since all wide boards are PCI.
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 52 ++++++++++------------------------------------ 1 files changed, 12 insertions(+), 40 deletions(-)
drivers/scsi/advansys.c +12 -40
@@ -17237,8 +17237,6 @@ advansys_board_found(int iop, struct device *dev, int bus_type)ADV_DVC_VAR *adv_dvc_varp =NULL ;adv_sgblk_t *sgp;int share_irq; -int iolen =0 ; -ADV_PADDR pci_memory_address;int warn_code, err_code;int ret;
@@ -17286,13 +17284,13 @@ advansys_board_found(int iop, struct device *dev, int bus_type) asc_dvc_varp->iop_base = iop; asc_dvc_varp->isr_callback = asc_isr_callback; }else { +#ifdef CONFIG_PCI ASC_DBG (1 ,"advansys_board_found: wide board\n" ); adv_dvc_varp = &boardp->dvc_var.adv_dvc_var; adv_dvc_varp->drv_ptr = boardp; adv_dvc_varp->cfg = &boardp->dvc_cfg.adv_dvc_cfg; adv_dvc_varp->isr_callback = adv_isr_callback; adv_dvc_varp->async_callback = adv_async_callback; -#ifdef CONFIG_PCI if (pdev->device ==PCI_DEVICE_ID_ASP_ABP940UW ) {ASC_DBG (1 ,"advansys_board_found: ASC-3550\n" ); adv_dvc_varp->chip_type =ADV_CHIP_ASC3550 ;
@@ -17303,46 +17301,20 @@ advansys_board_found(int iop, struct device *dev, int bus_type)ASC_DBG (1 ,"advansys_board_found: ASC-38C1600\n" ); adv_dvc_varp->chip_type =ADV_CHIP_ASC38C1600 ; } -#endif /* CONFIG_PCI */ -/* -* Map the board's registers into virtual memory for -* PCI slave access. Only memory accesses are used to -* access the board's registers. -* -* Note: The PCI register base address is not always -* page aligned, but the address passed to ioremap() -* must be page aligned. It is guaranteed that the -* PCI register base address will not cross a page -* boundary. -*/ -if (adv_dvc_varp->chip_type ==ADV_CHIP_ASC3550 ) { - iolen =ADV_3550_IOLEN ; - }else if (adv_dvc_varp->chip_type ==ADV_CHIP_ASC38C0800 ) { - iolen =ADV_38C0800_IOLEN ; - }else { - iolen =ADV_38C1600_IOLEN ; - } -#ifdef CONFIG_PCI - pci_memory_address =pci_resource_start (pdev,1 ); -ASC_DBG1 (1 , -"advansys_board_found: pci_memory_address: 0x%lx\n" , - (ulong )pci_memory_address); -if ((boardp->ioremap_addr = -ioremap (pci_memory_address &PAGE_MASK ,PAGE_SIZE )) ==0 ) { + boardp->asc_n_io_port =pci_resource_len (pdev,1 ); + boardp->ioremap_addr =ioremap (pci_resource_start (pdev,1 ), + boardp->asc_n_io_port); +if (!boardp->ioremap_addr) {ASC_PRINT3 ("advansys_board_found: board %d: ioremap(%x, %d) returned NULL\n" , - boardp->id, pci_memory_address, iolen); + boardp->id,pci_resource_start (pdev,1 ), + boardp->asc_n_io_port); goto err_shost; } -ASC_DBG1 (1 ,"advansys_board_found: ioremap_addr: 0x%lx\n" , - (ulong )boardp->ioremap_addr); - adv_dvc_varp->iop_base = (AdvPortAddr) - (boardp->ioremap_addr + - (pci_memory_address - (pci_memory_address &PAGE_MASK ))); + adv_dvc_varp->iop_base = (AdvPortAddr)boardp->ioremap_addrASC_DBG1 (1 ,"advansys_board_found: iop_base: 0x%lx\n" , adv_dvc_varp->iop_base); -#endif /* CONFIG_PCI */ /* * Even though it isn't used to access wide boards, other
@@ -17351,9 +17323,10 @@ advansys_board_found(int iop, struct device *dev, int bus_type)*/ boardp->ioport = iop; -ASC_DBG2 (1 , -"advansys_board_found: iopb_chip_id_1 0x%x, iopw_chip_id_0 0x%x\n" , - (ushort )inp (iop +1 ), (ushort )inpw (iop)); +ASC_DBG2 (1 ,"advansys_board_found: iopb_chip_id_1 0x%x, " +"iopw_chip_id_0 0x%x\n" , (ushort )inp (iop +1 ), + (ushort )inpw (iop)); +#endif /* CONFIG_PCI */ }#ifdef CONFIG_PROC_FS
@@ -17706,7 +17679,6 @@ advansys_board_found(int iop, struct device *dev, int bus_type)* PCI Memory Mapped I/O. */ shost->io_port = iop; - boardp->asc_n_io_port = iolen; shost->this_id = adv_dvc_varp->chip_scsi_id;
Signature · 1 line
1.4.4.4
PATCH 16/21 advansys: Stop using n_io_port in Scsi_Host structure
In reply to Matthew Wilcoxn_io_port isn't suitable for advansys because some of the boards have more than 255 bytes of io port space. There's already a driver-private replacement, asc_n_io_port, but for some reason the driver was still setting and occasionally reporting n_io_port.
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 38 ++++++++------------------------------ 1 files changed, 8 insertions(+), 30 deletions(-)
drivers/scsi/advansys.c +8 -30
@@ -2302,10 +2302,8 @@ typedef struct adveep_38C1600_config {#define BIOS_CTRL_AIPP_DIS 0x2000#define ADV_3550_MEMSIZE 0x2000/* 8 KB Internal Memory */ -#define ADV_3550_IOLEN 0x40/* I/O Port Range in bytes */ #define ADV_38C0800_MEMSIZE 0x4000/* 16 KB Internal Memory */ -#define ADV_38C0800_IOLEN 0x100/* I/O Port Range in bytes */ /* * XXX - Since ASC38C1600 Rev.3 has a local RAM failure issue, there is
@@ -2315,8 +2313,6 @@ typedef struct adveep_38C1600_config {* #define ADV_38C1600_MEMSIZE 0x8000L * 32 KB Internal Memory * */ #define ADV_38C1600_MEMSIZE 0x4000/* 16 KB Internal Memory */ -#define ADV_38C1600_IOLEN 0x100/* I/O Port Range 256 bytes */ -#define ADV_38C1600_MEMLEN 0x1000/* Memory Range 4KB bytes */ /* * Byte I/O register address from base of 'iop_base'.
@@ -3996,7 +3992,6 @@ static const char *advansys_info(struct Scsi_Host *shost)ASC_DVC_VAR *asc_dvc_varp;ADV_DVC_VAR *adv_dvc_varp;char *busname; -int iolen;char *widename =NULL ; boardp =ASC_BOARDP (shost);
@@ -4010,13 +4005,12 @@ static const char *advansys_info(struct Scsi_Host *shost) }else { busname ="ISA" ; } -/* Don't reference 'shost->n_io_port'; It may be truncated. */ sprintf (info,"AdvanSys SCSI %s: %s: IO 0x%lX-0x%lX, IRQ 0x%X, DMA 0x%X" ,ASC_VERSION , busname, (ulong )shost->io_port, - (ulong )shost->io_port + boardp->asc_n_io_port - -1 , shost->irq, shost->dma_channel); + (ulong )shost->io_port +ASC_IOADR_GAP -1 , + shost->irq, shost->dma_channel); }else {if (asc_dvc_varp->bus_type &ASC_IS_VL ) { busname ="VL" ;
@@ -4035,12 +4029,11 @@ static const char *advansys_info(struct Scsi_Host *shost)"bus type %d\n" , boardp->id, asc_dvc_varp->bus_type); } -/* Don't reference 'shost->n_io_port'; It may be truncated. */ sprintf (info,"AdvanSys SCSI %s: %s: IO 0x%lX-0x%lX, IRQ 0x%X" ,ASC_VERSION , busname, (ulong )shost->io_port, - (ulong )shost->io_port + boardp->asc_n_io_port - -1 , shost->irq); + (ulong )shost->io_port +ASC_IOADR_GAP -1 , + shost->irq); } }else {/*
@@ -4052,19 +4045,16 @@ static const char *advansys_info(struct Scsi_Host *shost)*/ adv_dvc_varp = &boardp->dvc_var.adv_dvc_var;if (adv_dvc_varp->chip_type ==ADV_CHIP_ASC3550 ) { - iolen =ADV_3550_IOLEN ; widename ="Ultra-Wide" ; }else if (adv_dvc_varp->chip_type ==ADV_CHIP_ASC38C0800 ) { - iolen =ADV_38C0800_IOLEN ; widename ="Ultra2-Wide" ; }else { - iolen =ADV_38C1600_IOLEN ; widename ="Ultra3-Wide" ; }sprintf (info,"AdvanSys SCSI %s: PCI %s: PCIMEM 0x%lX-0x%lX, IRQ 0x%X" ,ASC_VERSION , widename, (ulong )adv_dvc_varp->iop_base, - (ulong )adv_dvc_varp->iop_base + iolen -1 , shost->irq); + (ulong )adv_dvc_varp->iop_base + boardp->asc_n_io_port -1 , shost->irq); }ASC_ASSERT (strlen (info) <ASC_INFO_SIZE );ASC_DBG (1 ,"advansys_info: end\n" );
@@ -6795,10 +6785,7 @@ static int asc_prt_driver_conf(struct Scsi_Host *shost, char *cp, int cplen) boardp->asc_n_io_port);ASC_PRT_NEXT (); -/* 'shost->n_io_port' may be truncated because it is only one byte. */ - len =asc_prt_line (cp, leftlen, -" io_port 0x%x, n_io_port 0x%x\n" , - shost->io_port, shost->n_io_port); + len =asc_prt_line (cp, leftlen," io_port 0x%x\n" , shost->io_port);ASC_PRT_NEXT ();if (ASC_NARROW_BOARD (boardp)) {
@@ -7690,8 +7677,8 @@ static void asc_prt_scsi_host(struct Scsi_Host *s)printk (" host_busy %u, host_no %d, last_reset %d,\n" , s->host_busy, s->host_no, (unsigned )s->last_reset); -printk (" base 0x%lx, io_port 0x%lx, n_io_port %u, irq 0x%x,\n" , - (ulong )s->base, (ulong )s->io_port, s->n_io_port, s->irq); +printk (" base 0x%lx, io_port 0x%lx, irq 0x%x,\n" , + (ulong )s->base, (ulong )s->io_port, s->irq);printk (" dma_channel %d, this_id %d, can_queue %d,\n" , s->dma_channel, s->this_id, s->can_queue);
@@ -17687,15 +17674,6 @@ advansys_board_found(int iop, struct device *dev, int bus_type) }/* -* 'n_io_port' currently is one byte. -* -* Set a value to 'n_io_port', but never referenced it because -* it may be truncated. -*/ - shost->n_io_port = boardp->asc_n_io_port <=255 ? - boardp->asc_n_io_port :255 ; - -/* * Following v1.3.89, 'cmd_per_lun' is no longer needed * and should be set to zero. *
Signature · 1 line
1.4.4.4
PATCH 17/21 advansys: remove detect_called
In reply to Matthew WilcoxApparently it used to be possible to call the detect method twice. That's not possible any more with the new style init routine.
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 9 --------- 1 files changed, 0 insertions(+), 9 deletions(-)
drivers/scsi/advansys.c +0 -9
@@ -17888,18 +17888,9 @@ advansys_board_found(int iop, struct device *dev, int bus_type)*/ static int __init advansys_detect (void ) { -static int detect_called =ASC_FALSE ;int iop, bus, ioport =0 ;struct Scsi_Host *shost; -if (detect_called ==ASC_FALSE ) { - detect_called =ASC_TRUE ; - }else { -printk - ("AdvanSys SCSI: advansys_detect() multiple calls ignored\n" ); -return 0 ; - } -ASC_DBG (1 ,"advansys_detect: begin\n" ); asc_board_count =0 ;
Signature · 1 line
1.4.4.4
Re: [PATCH 17/21] advansys: remove detect_called
Quoted 1 line · Matthew Wilcox
Apparently it used to be possible to call the detect method twice.
Perhaps in pre-history, but not in today's kernel...
The patch is nonetheless correct, of course...
Jeff
PATCH 18/21 advansys: Move struct device out of the cfg structures
In reply to Matthew WilcoxThe cfg structures are supposed to be disposable after initialisation; with the 'dev' used for DMA mapping in there, that's not possible. Move the dev to the board.
Also inline AscInitFromAscDvcVar into its only caller, remove some unnecessary prototypes and sort out a few minor formatting issues.
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 104 ++++++++++++++++++---------------------------- 1 files changed, 41 insertions(+), 63 deletions(-)
drivers/scsi/advansys.c +41 -63
@@ -1391,7 +1391,6 @@ typedef struct asc_dvc_cfg {uchar sdtr_period_offset[ASC_MAX_TID +1 ];ushort pci_slot_info;uchar adapter_info[6 ]; -struct device *dev; }ASC_DVC_CFG ;#define ASC_DEF_DVC_CNTL 0xFFFF
@@ -1842,7 +1841,6 @@ static void AscMemDWordCopyPtrToLram(PortAddr, ushort, uchar *, int);static void AscMemWordCopyPtrFromLram (PortAddr ,ushort ,uchar *,int );static ushort AscInitAscDvcVar (ASC_DVC_VAR *);static ushort AscInitFromEEP (ASC_DVC_VAR *); -static ushort AscInitFromAscDvcVar (ASC_DVC_VAR *);static ushort AscInitMicroCodeVar (ASC_DVC_VAR *);static int AscTestExternalLram (ASC_DVC_VAR *);static uchar AscMsgOutSDTR (ASC_DVC_VAR *,uchar ,uchar );
@@ -2840,7 +2838,6 @@ typedef struct adv_dvc_cfg {ushort serial1;/* EEPROM serial number word 1 */ ushort serial2;/* EEPROM serial number word 2 */ ushort serial3;/* EEPROM serial number word 3 */ -struct device *dev;/* pointer to the pci dev structure for this board */ }ADV_DVC_CFG ;struct adv_dvc_var ;
@@ -3013,7 +3010,6 @@ static void DvcDelayMicroSecond(ADV_DVC_VAR *, ushort);*/ static int AdvExeScsiQueue (ADV_DVC_VAR *,ADV_SCSI_REQ_Q *);static int AdvISR (ADV_DVC_VAR *); -static int AdvInitGetConfig (ADV_DVC_VAR *);static int AdvInitAsc3550Driver (ADV_DVC_VAR *);static int AdvInitAsc38C0800Driver (ADV_DVC_VAR *);static int AdvInitAsc38C1600Driver (ADV_DVC_VAR *);
@@ -3614,6 +3610,7 @@ typedef struct adv_req {* field. It is guaranteed to be allocated from DMA-able memory. */ typedef struct asc_board { + struct device *dev; int id; /* Board Id */ uint flags; /* Board flags */ union {
@@ -4729,7 +4726,6 @@ static void asc_scsi_done_list(struct scsi_cmnd *scp)ASC_DBG (2 ,"asc_scsi_done_list: begin\n" );while (scp !=NULL ) {asc_board_t *boardp; -struct device *dev;ASC_DBG1 (3 ,"asc_scsi_done_list: scp 0x%lx\n" , (ulong )scp); tscp =REQPNEXT (scp);
@@ -4737,17 +4733,12 @@ static void asc_scsi_done_list(struct scsi_cmnd *scp) boardp =ASC_BOARDP (scp->device->host); -if (ASC_NARROW_BOARD (boardp)) - dev = boardp->dvc_cfg.asc_dvc_cfg.dev; -else - dev = boardp->dvc_cfg.adv_dvc_cfg.dev; -if (scp->use_sg) -dma_unmap_sg (dev, +dma_unmap_sg (boardp->dev, (struct scatterlist *)scp->request_buffer, scp->use_sg, scp->sc_data_direction);else if (scp->request_bufflen) -dma_unmap_single (dev, scp->SCp.dma_handle, +dma_unmap_single (boardp->dev, scp->SCp.dma_handle, scp->request_bufflen, scp->sc_data_direction);
@@ -4986,8 +4977,6 @@ static int asc_execute_scsi_cmnd(struct scsi_cmnd *scp)*/ static int asc_build_req (asc_board_t *boardp,struct scsi_cmnd *scp) { -struct device *dev = boardp->dvc_cfg.asc_dvc_cfg.dev; -/* * Mutually exclusive access is required to 'asc_scsi_q' and * 'asc_sg_head' until after the request is started.
@@ -5051,7 +5040,7 @@ static int asc_build_req(asc_board_t *boardp, struct scsi_cmnd *scp)*/ ASC_STATS (scp->device->host, cont_cnt); scp->SCp.dma_handle = scp->request_bufflen ? -dma_map_single (dev, scp->request_buffer, +dma_map_single (boardp->dev, scp->request_buffer, scp->request_bufflen, scp->sc_data_direction) :0 ; asc_scsi_q.q1.data_addr =cpu_to_le32 (scp->SCp.dma_handle);
@@ -5069,15 +5058,14 @@ static int asc_build_req(asc_board_t *boardp, struct scsi_cmnd *scp)struct scatterlist *slp; slp = (struct scatterlist *)scp->request_buffer; - use_sg = -dma_map_sg (dev, slp, scp->use_sg, scp->sc_data_direction); + use_sg =dma_map_sg (boardp->dev, slp, scp->use_sg, + scp->sc_data_direction);if (use_sg > scp->device->host->sg_tablesize) { -ASC_PRINT3 - ("asc_build_req: board %d: use_sg %d > sg_tablesize %d\n" , - boardp->id, use_sg, - scp->device->host->sg_tablesize); -dma_unmap_sg (dev, slp, scp->use_sg, +ASC_PRINT3 ("asc_build_req: board %d: use_sg %d > " +"sg_tablesize %d\n" , boardp->id, use_sg, + scp->device->host->sg_tablesize); +dma_unmap_sg (boardp->dev, slp, scp->use_sg, scp->sc_data_direction); scp->result =HOST_BYTE (DID_ERROR );asc_enqueue (&boardp->done, scp,ASC_BACK );
@@ -5138,7 +5126,6 @@ adv_build_req(asc_board_t *boardp, struct scsi_cmnd *scp,ADV_SCSI_REQ_Q *scsiqp;int i;int ret; -struct device *dev = boardp->dvc_cfg.adv_dvc_cfg.dev;/* * Allocate an adv_req_t structure from the board to execute
@@ -5225,7 +5212,7 @@ adv_build_req(asc_board_t *boardp, struct scsi_cmnd *scp,if (scp->request_bufflen) { scsiqp->vdata_addr = scp->request_buffer; scp->SCp.dma_handle = -dma_map_single (dev, scp->request_buffer, +dma_map_single (boardp->dev, scp->request_buffer, scp->request_bufflen, scp->sc_data_direction); }else {
@@ -5246,22 +5233,21 @@ adv_build_req(asc_board_t *boardp, struct scsi_cmnd *scp,int use_sg; slp = (struct scatterlist *)scp->request_buffer; - use_sg = -dma_map_sg (dev, slp, scp->use_sg, scp->sc_data_direction); + use_sg =dma_map_sg (boardp->dev, slp, scp->use_sg, + scp->sc_data_direction);if (use_sg >ADV_MAX_SG_LIST ) { -ASC_PRINT3 - ("adv_build_req: board %d: use_sg %d > ADV_MAX_SG_LIST %d\n" , - boardp->id, use_sg, - scp->device->host->sg_tablesize); -dma_unmap_sg (dev, slp, scp->use_sg, +ASC_PRINT3 ("adv_build_req: board %d: use_sg %d > " +"ADV_MAX_SG_LIST %d\n" , boardp->id, use_sg, + scp->device->host->sg_tablesize); +dma_unmap_sg (boardp->dev, slp, scp->use_sg, scp->sc_data_direction); scp->result =HOST_BYTE (DID_ERROR );asc_enqueue (&boardp->done, scp,ASC_BACK );/* -* Free the 'adv_req_t' structure by adding it back to the -* board free list. +* Free the 'adv_req_t' structure by adding it back +* to the board free list. */ reqp->next_reqp = boardp->adv_reqp; boardp->adv_reqp = reqp;
@@ -5269,12 +5255,11 @@ adv_build_req(asc_board_t *boardp, struct scsi_cmnd *scp,return ASC_ERROR ; } -if ((ret = -adv_get_sglist (boardp, reqp, scp, - use_sg)) !=ADV_SUCCESS ) { + ret =adv_get_sglist (boardp, reqp, scp, use_sg); +if (ret !=ADV_SUCCESS ) {/* -* Free the adv_req_t structure by adding it back to the -* board free list. +* Free the adv_req_t structure by adding it back to +* the board free list. */ reqp->next_reqp = boardp->adv_reqp; boardp->adv_reqp = reqp;
@@ -10377,30 +10362,21 @@ static ushort __devinit AscInitGetConfig(ASC_DVC_VAR *asc_dvc)return warn_code; } -static ushort __devinit AscInitSetConfig (ASC_DVC_VAR *asc_dvc) +static unsigned short __devinit +AscInitSetConfig (struct pci_dev *pdev,ASC_DVC_VAR *asc_dvc) { -ushort warn_code =0 ; +PortAddr iop_base = asc_dvc->iop_base; +unsigned short cfg_msw; +unsigned short warn_code =0 ; asc_dvc->init_state |=ASC_INIT_STATE_BEG_SET_CFG ;if (asc_dvc->err_code !=0 ) -return (UW_ERR ); -if (AscFindSignature (asc_dvc->iop_base)) { - warn_code |=AscInitFromAscDvcVar (asc_dvc); - asc_dvc->init_state |=ASC_INIT_STATE_END_SET_CFG ; - }else { +return UW_ERR ; +if (!AscFindSignature (asc_dvc->iop_base)) { asc_dvc->err_code =ASC_IERR_BAD_SIGNATURE ; +return 0 ; } -return (warn_code); -} - -static ushort __devinit AscInitFromAscDvcVar (ASC_DVC_VAR *asc_dvc) -{ -PortAddr iop_base; -ushort cfg_msw; -ushort warn_code; - iop_base = asc_dvc->iop_base; - warn_code =0 ; cfg_msw =AscGetChipCfgMsw (iop_base);if ((cfg_msw &ASC_CFG_MSW_CLR_MASK ) !=0 ) { cfg_msw &= (~(ASC_CFG_MSW_CLR_MASK ));
@@ -10423,7 +10399,6 @@ static ushort __devinit AscInitFromAscDvcVar(ASC_DVC_VAR *asc_dvc) }#ifdef CONFIG_PCI if (asc_dvc->bus_type &ASC_IS_PCI ) { -struct pci_dev *pdev =to_pci_dev (asc_dvc->cfg->dev); cfg_msw &=0xFFC0 ;AscSetChipCfgMsw (iop_base, cfg_msw);if ((asc_dvc->bus_type &ASC_IS_PCI_ULTRA ) ==ASC_IS_PCI_ULTRA ) {
@@ -10453,7 +10428,9 @@ static ushort __devinit AscInitFromAscDvcVar(ASC_DVC_VAR *asc_dvc) AscSetIsaDmaSpeed(iop_base, asc_dvc->cfg->isa_dma_speed); }#endif /* CONFIG_ISA */ -return (warn_code); + + asc_dvc->init_state |=ASC_INIT_STATE_END_SET_CFG ; +return warn_code; }static ushort AscInitAsc1000Driver (ASC_DVC_VAR *asc_dvc)
@@ -13757,11 +13734,11 @@ static ADVEEP_38C1600_CONFIG ADVEEP_38C1600_Config_Field_IsChar __devinitdata =* For a non-fatal error return a warning code. If there are no warnings * then 0 is returned. */ -static int __devinit AdvInitGetConfig (ADV_DVC_VAR *asc_dvc) +static int __devinit +AdvInitGetConfig (struct pci_dev *pdev,ADV_DVC_VAR *asc_dvc) {unsigned short warn_code =0 ;AdvPortAddr iop_base = asc_dvc->iop_base; -struct pci_dev *pdev =to_pci_dev (asc_dvc->cfg->dev);u16 cmd;int status;
@@ -17244,6 +17221,7 @@ advansys_board_found(int iop, struct device *dev, int bus_type)memset (boardp,0 ,sizeof (asc_board_t)); boardp->id = asc_board_count -1 ;spin_lock_init (&boardp->lock); + boardp->dev = dev;/* * Handle both narrow and wide boards.
@@ -17330,7 +17308,6 @@ advansys_board_found(int iop, struct device *dev, int bus_type)#endif /* CONFIG_PROC_FS */ if (ASC_NARROW_BOARD (boardp)) { - asc_dvc_varp->cfg->dev = dev;/* * Set the board bus type and PCI IRQ before * calling AscInitGetConfig().
@@ -17370,7 +17347,6 @@ advansys_board_found(int iop, struct device *dev, int bus_type)break ; } }else { - adv_dvc_varp->cfg->dev = dev;/* * For Wide boards set PCI information before calling * AdvInitGetConfig().
@@ -17439,7 +17415,9 @@ advansys_board_found(int iop, struct device *dev, int bus_type) } }else {ASC_DBG (2 ,"advansys_board_found: AdvInitGetConfig()\n" ); -if ((ret =AdvInitGetConfig (adv_dvc_varp)) !=0 ) { + + ret =AdvInitGetConfig (pdev, adv_dvc_varp); +if (ret !=0 ) {ASC_PRINT2 ("AdvInitGetConfig: board %d: warning: 0x%x\n" , boardp->id, ret);
@@ -17495,7 +17473,7 @@ advansys_board_found(int iop, struct device *dev, int bus_type)* Modify board configuration. */ ASC_DBG (2 ,"advansys_board_found: AscInitSetConfig()\n" ); -switch (ret =AscInitSetConfig (asc_dvc_varp)) { +switch (ret =AscInitSetConfig (pdev, asc_dvc_varp)) {case 0 :/* No error. */ break ;case ASC_WARN_IO_PORT_ROTATE :
Signature · 1 line
1.4.4.4
Re: [PATCH 18/21] advansys: Move struct device out of the cfg structures
ancillary TODO list item: check DMA mapping functions for failure
Re: [PATCH 18/21] advansys: Move struct device out of the cfg structures
Quoted 1 line · Jeff Garzik
ancillary TODO list item: check DMA mapping functions for failure
Added to in-driver TODO list
Signature · 3 lines
"Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
PATCH 19/21 advansys: Remove library-style callback routines
In reply to Matthew WilcoxConvert adv_isr_callback, adv_async_callback and asc_isr_callback into direct calls. Remove the unused asc_exe_callback.
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 50 +++++++--------------------------------------- 1 files changed, 8 insertions(+), 42 deletions(-)
drivers/scsi/advansys.c +8 -42
@@ -1415,17 +1415,12 @@ typedef struct asc_dvc_cfg {struct asc_dvc_var ;/* Forward Declaration. */ -typedef void (*ASC_ISR_CALLBACK ) (struct asc_dvc_var *,ASC_QDONE_INFO *); -typedef int (*ASC_EXE_CALLBACK ) (struct asc_dvc_var *,ASC_SCSI_Q *); - typedef struct asc_dvc_var { PortAddr iop_base; ushort err_code; ushort dvc_cntl; ushort bug_fix_cntl; ushort bus_type; - ASC_ISR_CALLBACK isr_callback; - ASC_EXE_CALLBACK exe_callback; ASC_SCSI_BIT_ID_TYPE init_sdtr; ASC_SCSI_BIT_ID_TYPE sdtr_done; ASC_SCSI_BIT_ID_TYPE use_tagged_qng;
@@ -2843,12 +2838,6 @@ typedef struct adv_dvc_cfg {struct adv_dvc_var ;struct adv_scsi_req_q ; -typedef void (*ADV_ISR_CALLBACK ) - (struct adv_dvc_var *,struct adv_scsi_req_q *); - -typedef void (*ADV_ASYNC_CALLBACK ) - (struct adv_dvc_var *,uchar ); -/* * Adapter operation variable structure. *
@@ -2865,8 +2854,6 @@ typedef struct adv_dvc_var {AdvPortAddr iop_base;/* I/O port address */ ushort err_code;/* fatal error code */ ushort bios_ctrl;/* BIOS control word, EEPROM word 12 */ -ADV_ISR_CALLBACK isr_callback; -ADV_ASYNC_CALLBACK async_callback;ushort wdtr_able;/* try WDTR for a device */ ushort sdtr_able;/* try SDTR for a device */ ushort ultra_able;/* try SDTR Ultra speed for a device */
@@ -3715,9 +3702,6 @@ static int asc_execute_scsi_cmnd(struct scsi_cmnd *);static int asc_build_req (asc_board_t *,struct scsi_cmnd *);static int adv_build_req (asc_board_t *,struct scsi_cmnd *,ADV_SCSI_REQ_Q **);static int adv_get_sglist (asc_board_t *,adv_req_t *,struct scsi_cmnd *,int ); -static void asc_isr_callback (ASC_DVC_VAR *,ASC_QDONE_INFO *); -static void adv_isr_callback (ADV_DVC_VAR *,ADV_SCSI_REQ_Q *); -static void adv_async_callback (ADV_DVC_VAR *,uchar );static void asc_enqueue (asc_queue_t *,REQP ,int );static REQP asc_dequeue (asc_queue_t *,int );static REQP asc_dequeue_list (asc_queue_t *,REQP *,int );
@@ -7719,9 +7703,8 @@ static void asc_prt_asc_dvc_var(ASC_DVC_VAR *h)printk (" iop_base 0x%x, err_code 0x%x, dvc_cntl 0x%x, bug_fix_cntl " "%d,\n" , h->iop_base, h->err_code, h->dvc_cntl, h->bug_fix_cntl); -printk (" bus_type %d, isr_callback 0x%p, exe_callback 0x%p, " -"init_sdtr 0x%x,\n" , h->bus_type, h->isr_callback, - h->exe_callback, (unsigned )h->init_sdtr); +printk (" bus_type %d, init_sdtr 0x%x,\n" , h->bus_type, + (unsigned )h->init_sdtr); printk(" sdtr_done 0x%x, use_tagged_qng 0x%x, unit_not_ready 0x%x, " "chip_no 0x%x,\n", (unsigned)h->sdtr_done,
@@ -8789,10 +8772,8 @@ static int AscIsrQDone(ASC_DVC_VAR *asc_dvc)ASC_QDONE_INFO scsiq_buf;ASC_QDONE_INFO *scsiq;int false_overrun; -ASC_ISR_CALLBACK asc_isr_callback; iop_base = asc_dvc->iop_base; - asc_isr_callback = asc_dvc->isr_callback; n_q_used =1 ; scsiq = (ASC_QDONE_INFO *)&scsiq_buf; done_q_tail = (uchar )AscGetVarDoneQTail (iop_base);
@@ -8904,7 +8885,7 @@ static int AscIsrQDone(ASC_DVC_VAR *asc_dvc) } }if ((scsiq->cntl &QC_NO_CALLBACK ) ==0 ) { - (*asc_isr_callback) (asc_dvc, scsiq); +asc_isr_callback (asc_dvc, scsiq); }else { if ((AscReadLramByte(iop_base, (ushort)(q_addr + (ushort)
@@ -8922,7 +8903,7 @@ static int AscIsrQDone(ASC_DVC_VAR *asc_dvc)AscSetLibErrorCode (asc_dvc,ASCQ_ERR_Q_STATUS ); FATAL_ERR_QDONE:if ((scsiq->cntl &QC_NO_CALLBACK ) ==0 ) { - (*asc_isr_callback) (asc_dvc, scsiq); +asc_isr_callback (asc_dvc, scsiq); }return (0x80 ); }
@@ -8948,9 +8929,7 @@ static int AscISR(ASC_DVC_VAR *asc_dvc)return int_pending; } -if (((asc_dvc->init_state &ASC_INIT_STATE_END_LOAD_MC ) ==0 ) - || (asc_dvc->isr_callback ==0 ) - ) { +if ((asc_dvc->init_state &ASC_INIT_STATE_END_LOAD_MC ) ==0 ) {return (ERR ); }if (asc_dvc->in_critical_cnt !=0 ) {
@@ -9349,7 +9328,6 @@ static int AscExeScsiQueue(ASC_DVC_VAR *asc_dvc, ASC_SCSI_Q *scsiq)int disable_syn_offset_one_fix;int i;ASC_PADDR addr; -ASC_EXE_CALLBACK asc_exe_callback;ushort sg_entry_cnt =0 ;ushort sg_entry_cnt_minus_one =0 ;uchar target_ix;
@@ -9363,7 +9341,6 @@ static int AscExeScsiQueue(ASC_DVC_VAR *asc_dvc, ASC_SCSI_Q *scsiq) iop_base = asc_dvc->iop_base; sg_head = scsiq->sg_head; - asc_exe_callback = asc_dvc->exe_callback;if (asc_dvc->err_code !=0 )return (ERR );if (scsiq == (ASC_SCSI_Q *)0L ) {
@@ -9524,9 +9501,6 @@ static int AscExeScsiQueue(ASC_DVC_VAR *asc_dvc, ASC_SCSI_Q *scsiq) AscSendScsiQueue(asc_dvc, scsiq, n_q_required)) == 1) { asc_dvc->in_critical_cnt--; -if (asc_exe_callback !=0 ) { - (*asc_exe_callback) (asc_dvc, scsiq); - }DvcLeaveCritical (last_int_level);return (sta); }
@@ -9572,9 +9546,6 @@ static int AscExeScsiQueue(ASC_DVC_VAR *asc_dvc, ASC_SCSI_Q *scsiq)if ((sta =AscSendScsiQueue (asc_dvc, scsiq, n_q_required)) ==1 ) { asc_dvc->in_critical_cnt--; -if (asc_exe_callback !=0 ) { - (*asc_exe_callback) (asc_dvc, scsiq); - }DvcLeaveCritical (last_int_level);return (sta); }
@@ -16903,7 +16874,7 @@ static int AdvISR(ADV_DVC_VAR *asc_dvc)/* * Notify the driver of an asynchronous microcode condition by -* calling the ADV_DVC_VAR.async_callback function. The function +* calling the adv_async_callback function. The function * is passed the microcode ASC_MC_INTRB_CODE byte value. */ if (int_stat &ADV_INTR_STATUS_INTRB ) {
@@ -16925,9 +16896,7 @@ static int AdvISR(ADV_DVC_VAR *asc_dvc) } } -if (asc_dvc->async_callback !=0 ) { - (*asc_dvc->async_callback) (asc_dvc, intrb_code); - } +adv_async_callback (asc_dvc, intrb_code); }/*
@@ -16985,7 +16954,7 @@ static int AdvISR(ADV_DVC_VAR *asc_dvc)* the ADV_SCSI_REQ_Q pointer to its callback function. */ scsiq->a_flag |=ADV_SCSIQ_DONE ; - (*asc_dvc->isr_callback) (asc_dvc, scsiq); +adv_isr_callback (asc_dvc, scsiq);/* * Note: After the driver callback function is called, 'scsiq' * can no longer be referenced.
@@ -17247,15 +17216,12 @@ advansys_board_found(int iop, struct device *dev, int bus_type) asc_dvc_varp->cfg = &boardp->dvc_cfg.asc_dvc_cfg; asc_dvc_varp->cfg->overrun_buf = &overrun_buf[0 ]; asc_dvc_varp->iop_base = iop; - asc_dvc_varp->isr_callback = asc_isr_callback; }else {#ifdef CONFIG_PCI ASC_DBG (1 ,"advansys_board_found: wide board\n" ); adv_dvc_varp = &boardp->dvc_var.adv_dvc_var; adv_dvc_varp->drv_ptr = boardp; adv_dvc_varp->cfg = &boardp->dvc_cfg.adv_dvc_cfg; - adv_dvc_varp->isr_callback = adv_isr_callback; - adv_dvc_varp->async_callback = adv_async_callback;if (pdev->device ==PCI_DEVICE_ID_ASP_ABP940UW ) {ASC_DBG (1 ,"advansys_board_found: ASC-3550\n" ); adv_dvc_varp->chip_type =ADV_CHIP_ASC3550 ;
Signature · 1 line
1.4.4.4
PATCH 20/21 advansys: Remove pci_slot_info
In reply to Matthew WilcoxThe driver kept a copy of the PCI config address; refer to the pci_dev associated with the card instead.
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 29 +++++++++-------------------- 1 files changed, 9 insertions(+), 20 deletions(-)
drivers/scsi/advansys.c +9 -20
@@ -853,8 +853,6 @@ typedef unsigned char uchar;#define ERR (-1)#define UW_ERR (uint)(0xFFFF)#define isodd_word (val) ((((uint)val) & (uint)0x0001) != 0) -#define ASC_PCI_ID2FUNC (id) (((id) >> 8) & 0x7) -#define ASC_PCI_MKID (bus, dev, func) ((((dev) & 0x1F) << 11) | (((func) & 0x7) << 8) | ((bus) & 0xFF))#define ASC_DVCLIB_CALL_DONE (1)#define ASC_DVCLIB_CALL_FAILED (0)
@@ -1389,7 +1387,6 @@ typedef struct asc_dvc_cfg {uchar max_tag_qng[ASC_MAX_TID +1 ];uchar *overrun_buf;uchar sdtr_period_offset[ASC_MAX_TID +1 ]; -ushort pci_slot_info;uchar adapter_info[6 ]; }ASC_DVC_CFG ;
@@ -2827,9 +2824,6 @@ typedef struct adv_dvc_cfg {ushort control_flag;/* Microcode Control Flag */ ushort mcode_date;/* Microcode date */ ushort mcode_version;/* Microcode version */ -ushort pci_slot_info;/* high byte device/function number */ -/* bits 7-3 device num., bits 2-0 function num. */ -/* low byte bus num. */ ushort serial1;/* EEPROM serial number word 1 */ ushort serial2;/* EEPROM serial number word 2 */ ushort serial3;/* EEPROM serial number word 3 */
@@ -3651,6 +3645,10 @@ typedef struct asc_board {ushort bios_codelen;/* BIOS Code Segment Length. */ } asc_board_t; +#define adv_dvc_to_board (adv_dvc) container_of(adv_dvc, struct asc_board, \ + dvc_var.adv_dvc_var) +#define adv_dvc_to_pdev (adv_dvc) to_pci_dev(adv_dvc_to_board(adv_dvc)->dev) +/* * --- Driver Data */
@@ -7859,8 +7857,7 @@ static void asc_prt_adv_dvc_cfg(ADV_DVC_CFG *h)printk (" mcode_version 0x%x, pci_device_id 0x%x, lib_version %u\n" , h->mcode_version,to_pci_dev (h->dev)->device, h->lib_version); -printk (" control_flag 0x%x, pci_slot_info 0x%x\n" , - h->control_flag, h->pci_slot_info); +printk (" control_flag 0x%x\n" , h->control_flag); }/*
@@ -15359,6 +15356,7 @@ static int AdvInitAsc38C1600Driver(ADV_DVC_VAR *asc_dvc)* ready to be 'ored' into SCSI_CFG1. */ if ((asc_dvc->cfg->termination &TERM_SE ) ==0 ) { +struct pci_dev *pdev =adv_dvc_to_pdev (asc_dvc);/* SE automatic termination control is enabled. */ switch (scsi_cfg1 &C_DET_SE ) {/* TERM_SE_HI: on, TERM_SE_LO: on */
@@ -15369,7 +15367,7 @@ static int AdvInitAsc38C1600Driver(ADV_DVC_VAR *asc_dvc)break ;case 0x0 : -if (ASC_PCI_ID2FUNC (asc_dvc->cfg->pci_slot_info) ==0 ) { +if (PCI_FUNC (pdev->devfn) ==0 ) {/* Function 0 - TERM_SE_HI: off, TERM_SE_LO: off */ }else {/* Function 1 - TERM_SE_HI: on, TERM_SE_LO: off */
@@ -15975,15 +15973,14 @@ static int __devinit AdvInitFrom38C1600EEP(ADV_DVC_VAR *asc_dvc)*/ if (AdvGet38C1600EEPConfig (iop_base, &eep_config) != eep_config.check_sum) { +struct pci_dev *pdev =adv_dvc_to_pdev (asc_dvc); warn_code |=ASC_WARN_EEPROM_CHKSUM ;/* * Set EEPROM default values. */ for (i =0 ; i <sizeof (ADVEEP_38C1600_CONFIG ); i++) { -if (i ==1 - &&ASC_PCI_ID2FUNC (asc_dvc->cfg->pci_slot_info) != -0 ) { +if (i ==1 &&PCI_FUNC (pdev->devfn) !=0 ) {/* * Set Function 1 EEPROM Word 0 MSB *
@@ -17296,10 +17293,6 @@ advansys_board_found(int iop, struct device *dev, int bus_type)#ifdef CONFIG_PCI case ASC_IS_PCI : shost->irq = asc_dvc_varp->irq_no = pdev->irq; - asc_dvc_varp->cfg->pci_slot_info = -ASC_PCI_MKID (pdev->bus->number, -PCI_SLOT (pdev->devfn), -PCI_FUNC (pdev->devfn)); shost->unchecked_isa_dma = FALSE; share_irq = TRUE;break ;
@@ -17319,10 +17312,6 @@ advansys_board_found(int iop, struct device *dev, int bus_type)*/ #ifdef CONFIG_PCI shost->irq = adv_dvc_varp->irq_no = pdev->irq; - adv_dvc_varp->cfg->pci_slot_info = -ASC_PCI_MKID (pdev->bus->number, -PCI_SLOT (pdev->devfn), -PCI_FUNC (pdev->devfn)); shost->unchecked_isa_dma = FALSE; share_irq = TRUE;#endif /* CONFIG_PCI */
Signature · 1 line
1.4.4.4
PATCH 21/21 advansys: use memcpy instead of open-coded loop
In reply to Matthew WilcoxUse memcpy to initialise eep_config instead of a loop. For AdvInitFrom38C1600EEP where we need to modify the default EEPROM configuration, do it after the loop, and do it using the structure definition, not by finding the right byte. I think it was wrong for big-endian machines.
Also delete some non-useful comments and prototypes.
Signed-off-by Matthew Wilcox <[email protected]>
--- drivers/scsi/advansys.c | 119 ++++++++++++++++------------------------------- 1 files changed, 40 insertions(+), 79 deletions(-)
drivers/scsi/advansys.c +40 -79
@@ -3181,13 +3181,6 @@ do { \#define QHSTA_M_SGBACKUP_ERROR 0x47/* Scatter-Gather backup error */ /* -* Default EEPROM Configuration structure defined in a_init.c. -*/ -static ADVEEP_3550_CONFIG Default_3550_EEPROM_Config; -static ADVEEP_38C0800_CONFIG Default_38C0800_EEPROM_Config; -static ADVEEP_38C1600_CONFIG Default_38C1600_EEPROM_Config; - -/* * DvcGetPhyAddr() flag arguments */ #define ADV_IS_SCSIQ_FLAG 0x01/* 'addr' is ASC_SCSI_REQ_Q pointer */
@@ -13341,7 +13334,6 @@ static unsigned char _adv_asc38C1600_buf[] = {static unsigned short _adv_asc38C1600_size =sizeof (_adv_asc38C1600_buf);/* 0x1673 */ static ADV_DCNT _adv_asc38C1600_chksum =0x0604EF77UL ;/* Expanded little-endian checksum. */ -/* a_init.c */ /* * EEPROM Configuration. *
@@ -15601,7 +15593,6 @@ static int __devinit AdvInitFrom3550EEP(ADV_DVC_VAR *asc_dvc)AdvPortAddr iop_base;ushort warn_code;ADVEEP_3550_CONFIG eep_config; -int i; iop_base = asc_dvc->iop_base;
@@ -15618,15 +15609,12 @@ static int __devinit AdvInitFrom3550EEP(ADV_DVC_VAR *asc_dvc)/* * Set EEPROM default values. */ -for (i =0 ; i <sizeof (ADVEEP_3550_CONFIG ); i++) { - *((uchar *)&eep_config + i) = - *((uchar *)&Default_3550_EEPROM_Config + i); - } +memcpy (&eep_config, &Default_3550_EEPROM_Config, +sizeof (ADVEEP_3550_CONFIG ));/* -* Assume the 6 byte board serial number that was read -* from EEPROM is correct even if the EEPROM checksum -* failed. +* Assume the 6 byte board serial number that was read from +* EEPROM is correct even if the EEPROM checksum failed. */ eep_config.serial_number_word3 =AdvReadEEPWord (iop_base,ADV_EEP_DVC_CFG_END -1 );
@@ -15755,7 +15743,6 @@ static int __devinit AdvInitFrom38C0800EEP(ADV_DVC_VAR *asc_dvc)AdvPortAddr iop_base;ushort warn_code;ADVEEP_38C0800_CONFIG eep_config; -int i;uchar tid, termination;ushort sdtr_speed =0 ;
@@ -15775,15 +15762,12 @@ static int __devinit AdvInitFrom38C0800EEP(ADV_DVC_VAR *asc_dvc)/* * Set EEPROM default values. */ -for (i =0 ; i <sizeof (ADVEEP_38C0800_CONFIG ); i++) { - *((uchar *)&eep_config + i) = - *((uchar *)&Default_38C0800_EEPROM_Config + i); - } +memcpy (&eep_config, &Default_38C0800_EEPROM_Config, +sizeof (ADVEEP_38C0800_CONFIG ));/* -* Assume the 6 byte board serial number that was read -* from EEPROM is correct even if the EEPROM checksum -* failed. +* Assume the 6 byte board serial number that was read from +* EEPROM is correct even if the EEPROM checksum failed. */ eep_config.serial_number_word3 =AdvReadEEPWord (iop_base,ADV_EEP_DVC_CFG_END -1 );
@@ -15958,7 +15942,6 @@ static int __devinit AdvInitFrom38C1600EEP(ADV_DVC_VAR *asc_dvc)AdvPortAddr iop_base;ushort warn_code;ADVEEP_38C1600_CONFIG eep_config; -int i;uchar tid, termination;ushort sdtr_speed =0 ;
@@ -15979,68 +15962,46 @@ static int __devinit AdvInitFrom38C1600EEP(ADV_DVC_VAR *asc_dvc)/* * Set EEPROM default values. */ -for (i =0 ; i <sizeof (ADVEEP_38C1600_CONFIG ); i++) { -if (i ==1 &&PCI_FUNC (pdev->devfn) !=0 ) { -/* -* Set Function 1 EEPROM Word 0 MSB -* -* Clear the BIOS_ENABLE (bit 14) and INTAB (bit 11) -* EEPROM bits. -* -* Disable Bit 14 (BIOS_ENABLE) to fix SPARC Ultra 60 and -* old Mac system booting problem. The Expansion ROM must -* be disabled in Function 1 for these systems. -* -*/ - *((uchar *)&eep_config + i) = - ((* - ((uchar *)&Default_38C1600_EEPROM_Config - + - i)) & - (~ - (((ADV_EEPROM_BIOS_ENABLE | -ADV_EEPROM_INTAB ) >>8 ) &0xFF ))); +memcpy (&eep_config, &Default_38C1600_EEPROM_Config, +sizeof (ADVEEP_38C1600_CONFIG )); -/* -* Set the INTAB (bit 11) if the GPIO 0 input indicates -* the Function 1 interrupt line is wired to INTA. -* -* Set/Clear Bit 11 (INTAB) from the GPIO bit 0 input: -* 1 - Function 1 interrupt line wired to INT A. -* 0 - Function 1 interrupt line wired to INT B. -* -* Note: Adapter boards always have Function 0 wired to INTA. -* Put all 5 GPIO bits in input mode and then read -* their input values. -*/ -AdvWriteByteRegister (iop_base,IOPB_GPIO_CNTL , -0 ); -if (AdvReadByteRegister - (iop_base,IOPB_GPIO_DATA ) &0x01 ) { -/* Function 1 interrupt wired to INTA; Set EEPROM bit. */ - *((uchar *)&eep_config + i) |= - ((ADV_EEPROM_INTAB >>8 ) &0xFF ); - } - }else { - *((uchar *)&eep_config + i) = - *((uchar *)&Default_38C1600_EEPROM_Config - + i); - } +if (PCI_FUNC (pdev->devfn) !=0 ) { +u8 ints; +/* +* Disable Bit 14 (BIOS_ENABLE) to fix SPARC Ultra 60 +* and old Mac system booting problem. The Expansion +* ROM must be disabled in Function 1 for these systems +*/ + eep_config.cfg_lsw &= ~ADV_EEPROM_BIOS_ENABLE ; +/* +* Clear the INTAB (bit 11) if the GPIO 0 input +* indicates the Function 1 interrupt line is wired +* to INTB. +* +* Set/Clear Bit 11 (INTAB) from the GPIO bit 0 input: +* 1 - Function 1 interrupt line wired to INT A. +* 0 - Function 1 interrupt line wired to INT B. +* +* Note: Function 0 is always wired to INTA. +* Put all 5 GPIO bits in input mode and then read +* their input values. +*/ +AdvWriteByteRegister (iop_base,IOPB_GPIO_CNTL ,0 ); + ints =AdvReadByteRegister (iop_base,IOPB_GPIO_DATA ); +if ((ints &0x01 ) ==0 ) + eep_config.cfg_lsw &= ~ADV_EEPROM_INTAB ; }/* -* Assume the 6 byte board serial number that was read -* from EEPROM is correct even if the EEPROM checksum -* failed. +* Assume the 6 byte board serial number that was read from +* EEPROM is correct even if the EEPROM checksum failed. */ eep_config.serial_number_word3 = -AdvReadEEPWord (iop_base,ADV_EEP_DVC_CFG_END -1 ); - +AdvReadEEPWord (iop_base,ADV_EEP_DVC_CFG_END -1 ); eep_config.serial_number_word2 = -AdvReadEEPWord (iop_base,ADV_EEP_DVC_CFG_END -2 ); - +AdvReadEEPWord (iop_base,ADV_EEP_DVC_CFG_END -2 ); eep_config.serial_number_word1 = -AdvReadEEPWord (iop_base,ADV_EEP_DVC_CFG_END -3 ); +AdvReadEEPWord (iop_base,ADV_EEP_DVC_CFG_END -3 );AdvSet38C1600EEPConfig (iop_base, &eep_config); }
Signature · 1 line
1.4.4.4
Re: [PATCH 21/21] advansys: use memcpy instead of open-coded loop
Done with my review. Any patches I did not comment on can be considered sane...
Re: [PATCH 21/21] advansys: use memcpy instead of open-coded loop
Quoted 2 lines · Jeff Garzik
Done with my review. Any patches I did not comment on can be considered sane...
Thanks, Jeff, that was a Herculean effort. I've gone through and made most of your suggested changes. I'm making too many mistakes now, so it's time to go to bed, but I'll post another round of patches early tomorrow.
Signature · 3 lines
"Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."