[PATCH] sound/oss/pss: set_io_base() always returns success, mark it void

[PATCH] sound/oss/pss: set_io_base() always returns success, mark it void

[Jeff Garzik]

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 sound/oss/pss.c |   25 ++++++-------------------
 1 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/sound/oss/pss.c b/sound/oss/pss.c
index ece428b..e970f75 100644
--- a/sound/oss/pss.c
+++ b/sound/oss/pss.c
@@ -232,14 +232,12 @@ static int set_irq(pss_confdata * devc, int dev, int irq)
 	return 1;
 }
 
-static int set_io_base(pss_confdata * devc, int dev, int base)
+static void set_io_base(pss_confdata * devc, int dev, int base)
 {
 	unsigned short  tmp = inw(REG(dev)) & 0x003f;
 	unsigned short  bits = (base & 0x0ffc) << 4;
 
 	outw(bits | tmp, REG(dev));
-
-	return 1;
 }
 
 static int set_dma(pss_confdata * devc, int dev, int dma)
@@ -677,16 +675,11 @@ static void configure_nonsound_components(void)
 	{
 		printk(KERN_INFO "PSS: CDROM port not enabled.\n");
 	}
-	else if(check_region(pss_cdrom_port, 2))
-	{
+	else if(check_region(pss_cdrom_port, 2)) {
 		printk(KERN_ERR "PSS: CDROM I/O port conflict.\n");
 	}
-	else if(!set_io_base(devc, CONF_CDROM, pss_cdrom_port))
-	{
-		printk(KERN_ERR "PSS: CDROM I/O port could not be set.\n");
-	}
-	else					/* CDROM port successfully configured */
-	{
+	else {
+		set_io_base(devc, CONF_CDROM, pss_cdrom_port);
 		printk(KERN_INFO "PSS: CDROM I/O port set to 0x%x.\n", pss_cdrom_port);
 	}
 }
@@ -758,10 +751,7 @@ static int __init probe_pss_mpu(struct address_info *hw_config)
 		printk(KERN_ERR "PSS: MPU I/O port conflict\n");
 		return 0;
 	}
-	if (!set_io_base(devc, CONF_MIDI, hw_config->io_base)) {
-		printk(KERN_ERR "PSS: MIDI base could not be set.\n");
-		goto fail;
-	}
+	set_io_base(devc, CONF_MIDI, hw_config->io_base);
 	if (!set_irq(devc, CONF_MIDI, hw_config->irq)) {
 		printk(KERN_ERR "PSS: MIDI IRQ allocation error.\n");
 		goto fail;
@@ -1057,10 +1047,7 @@ static int __init probe_pss_mss(struct address_info *hw_config)
 		release_region(hw_config->io_base, 4);
 		return 0;
 	}
-	if (!set_io_base(devc, CONF_WSS, hw_config->io_base)) {
-		printk("PSS: WSS base not settable.\n");
-		goto fail;
-	}
+	set_io_base(devc, CONF_WSS, hw_config->io_base);
 	if (!set_irq(devc, CONF_WSS, hw_config->irq)) {
 		printk("PSS: WSS IRQ allocation error.\n");
 		goto fail;
-- 
1.5.2.4

[PATCH 1/3] [IDE] Add helper __ide_setup_pci_device()

[Jeff Garzik]

Like ide_setup_pci_device(), except with additional array argument
'u8 *idx' that permits low-level driver to become aware of its assigned
hwifs.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/ide/setup-pci.c |   17 ++++++++++++-----
 include/linux/ide.h     |    1 +
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/ide/setup-pci.c b/drivers/ide/setup-pci.c
index 02d14bf..4960b9f 100644
--- a/drivers/ide/setup-pci.c
+++ b/drivers/ide/setup-pci.c
@@ -666,12 +666,10 @@ out:
 	return ret;
 }
 
-int ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d)
+int __ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d,
+			   u8 *idx)
 {
-	u8 idx[4] = { 0xff, 0xff, 0xff, 0xff };
-	int ret;
-
-	ret = do_ide_setup_pci_device(dev, d, &idx[0], 1);
+	int ret = do_ide_setup_pci_device(dev, d, idx, 1);
 
 	if (ret >= 0)
 		ide_device_add(idx);
@@ -679,6 +677,15 @@ int ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d)
 	return ret;
 }
 
+EXPORT_SYMBOL_GPL(__ide_setup_pci_device);
+
+int ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d)
+{
+	u8 idx[4] = { 0xff, 0xff, 0xff, 0xff };
+
+	return __ide_setup_pci_device(dev, d, &idx[0]);
+}
+
 EXPORT_SYMBOL_GPL(ide_setup_pci_device);
 
 int ide_setup_pci_devices(struct pci_dev *dev1, struct pci_dev *dev2,
diff --git a/include/linux/ide.h b/include/linux/ide.h
index 4ed4777..3404fb9 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1244,6 +1244,7 @@ struct ide_port_info {
 	u8			udma_mask;
 };
 
+int __ide_setup_pci_device(struct pci_dev *, const struct ide_port_info *, u8 *);
 int ide_setup_pci_device(struct pci_dev *, const struct ide_port_info *);
 int ide_setup_pci_devices(struct pci_dev *, struct pci_dev *, const struct ide_port_info *);
 
-- 
1.5.2.4

[PATCH 2/3] drivers/ide/pci/sc1200.c: remove pointless hwif lookup loop

[Jeff Garzik]

Store our hwif indices at probe time, in order to eliminate a needless
and ugly loop across all hwifs, searching for our pci device.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/ide/pci/sc1200.c |   76 +++++++++++++++++++++++++++++++---------------
 1 files changed, 51 insertions(+), 25 deletions(-)

diff --git a/drivers/ide/pci/sc1200.c b/drivers/ide/pci/sc1200.c
index d2c8b55..17e58d6 100644
--- a/drivers/ide/pci/sc1200.c
+++ b/drivers/ide/pci/sc1200.c
@@ -41,6 +41,8 @@
 #define PCI_CLK_66	0x02
 #define PCI_CLK_33A	0x03
 
+#define SC1200_IFS	4
+
 static unsigned short sc1200_get_pci_clock (void)
 {
 	unsigned char chip_id, silicon_revision;
@@ -261,31 +263,32 @@ static void sc1200_set_pio_mode(ide_drive_t *drive, const u8 pio)
 }
 
 #ifdef CONFIG_PM
-static ide_hwif_t *lookup_pci_dev (ide_hwif_t *prev, struct pci_dev *dev)
-{
-	int	h;
-
-	for (h = 0; h < MAX_HWIFS; h++) {
-		ide_hwif_t *hwif = &ide_hwifs[h];
-		if (prev) {
-			if (hwif == prev)
-				prev = NULL;	// found previous, now look for next match
-		} else {
-			if (hwif && hwif->pci_dev == dev)
-				return hwif;	// found next match
-		}
-	}
-	return NULL;	// not found
-}
-
 typedef struct sc1200_saved_state_s {
 	__u32		regs[4];
 } sc1200_saved_state_t;
 
+static unsigned int pack_hwif_idx(u8 *idx)
+{
+	return	(((unsigned int) idx[0]) << 0) |
+		(((unsigned int) idx[1]) << 8) |
+		(((unsigned int) idx[2]) << 16) |
+		(((unsigned int) idx[3]) << 24);
+}
+
+static ide_hwif_t *sc1200_hwif(struct pci_dev *pdev, unsigned int iface)
+{
+	unsigned int packed_hwifs, idx;
+
+	packed_hwifs = (unsigned long) pci_get_drvdata(pdev);
+	idx = (packed_hwifs >> (iface * 8)) & 0xff;
+
+	return (idx == 0xff) ? NULL : &ide_hwifs[idx];
+}
 
 static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
 {
-	ide_hwif_t		*hwif = NULL;
+	ide_hwif_t		*hwif;
+	int			i;
 
 	printk("SC1200: suspend(%u)\n", state.event);
 
@@ -295,9 +298,14 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
 		//
 		// Loop over all interfaces that are part of this PCI device:
 		//
-		while ((hwif = lookup_pci_dev(hwif, dev)) != NULL) {
+		for (i = 0; i < SC1200_IFS; i++) {
 			sc1200_saved_state_t	*ss;
 			unsigned int		basereg, r;
+
+			hwif = sc1200_hwif(dev, i);
+			if (!hwif)
+				continue;
+
 			//
 			// allocate a permanent save area, if not already allocated
 			//
@@ -310,7 +318,7 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
 			}
 			ss = (sc1200_saved_state_t *)hwif->config_data;
 			//
-			// Save timing registers:  this may be unnecessary if 
+			// Save timing registers:  this may be unnecessary if
 			// BIOS also does it
 			//
 			basereg = hwif->channel ? 0x50 : 0x40;
@@ -320,7 +328,7 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
 		}
 	}
 
-	/* You don't need to iterate over disks -- sysfs should have done that for you already */ 
+	/* You don't need to iterate over disks -- sysfs should have done that for you already */
 
 	pci_disable_device(dev);
 	pci_set_power_state(dev, pci_choose_state(dev, state));
@@ -330,7 +338,8 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
 
 static int sc1200_resume (struct pci_dev *dev)
 {
-	ide_hwif_t	*hwif = NULL;
+	ide_hwif_t		*hwif;
+	int			i;
 
 	pci_set_power_state(dev, PCI_D0);	// bring chip back from sleep state
 	dev->current_state = PM_EVENT_ON;
@@ -338,9 +347,15 @@ static int sc1200_resume (struct pci_dev *dev)
 	//
 	// loop over all interfaces that are part of this pci device:
 	//
-	while ((hwif = lookup_pci_dev(hwif, dev)) != NULL) {
+	for (i = 0; i < SC1200_IFS; i++) {
 		unsigned int		basereg, r;
-		sc1200_saved_state_t	*ss = (sc1200_saved_state_t *)hwif->config_data;
+		sc1200_saved_state_t	*ss;
+
+		hwif = sc1200_hwif(dev, i);
+		if (!hwif)
+			continue;
+
+		ss = (sc1200_saved_state_t *)hwif->config_data;
 
 		//
 		// Restore timing registers:  this may be unnecessary if BIOS also does it
@@ -386,7 +401,18 @@ static const struct ide_port_info sc1200_chipset __devinitdata = {
 
 static int __devinit sc1200_init_one(struct pci_dev *dev, const struct pci_device_id *id)
 {
-	return ide_setup_pci_device(dev, &sc1200_chipset);
+	unsigned int packed_hwifs;
+	int rc;
+	u8 idx[4] = { 0xff, 0xff, 0xff, 0xff };
+
+	rc = __ide_setup_pci_device(dev, &sc1200_chipset, &idx[0]);
+	if (rc)
+		return rc;
+	
+	packed_hwifs = pack_hwif_idx(&idx[0]);
+
+	pci_set_drvdata(dev, (void *)(unsigned long) packed_hwifs);
+	return 0;
 }
 
 static const struct pci_device_id sc1200_pci_tbl[] = {
-- 
1.5.2.4

[PATCH 3/3] drivers/ide/pci/sc1200.c: fix suspend/resume buglets and warnings

[Jeff Garzik]

* We shouldn't bother with dev->current_state, the PCI API functions we
  call manage this for us (and do a far better job at it too).

* Remove pci_set_power_state(dev, PCI_D0) call in resume, as
  pci_enable_device() does the same thing.

* Check pci_enable_device() return value.  If it failed, fail
  the entire resume and avoid programming timings into the [potentially
  dead/asleep] chip.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/ide/pci/sc1200.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/ide/pci/sc1200.c b/drivers/ide/pci/sc1200.c
index 17e58d6..10c79a5 100644
--- a/drivers/ide/pci/sc1200.c
+++ b/drivers/ide/pci/sc1200.c
@@ -332,7 +332,6 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
 
 	pci_disable_device(dev);
 	pci_set_power_state(dev, pci_choose_state(dev, state));
-	dev->current_state = state.event;
 	return 0;
 }
 
@@ -341,9 +340,10 @@ static int sc1200_resume (struct pci_dev *dev)
 	ide_hwif_t		*hwif;
 	int			i;
 
-	pci_set_power_state(dev, PCI_D0);	// bring chip back from sleep state
-	dev->current_state = PM_EVENT_ON;
-	pci_enable_device(dev);
+	i = pci_enable_device(dev);
+	if (i)
+		return i;
+
 	//
 	// loop over all interfaces that are part of this pci device:
 	//
-- 
1.5.2.4

[PATCH 1/4] [SCSI] ips: remove ips_ha members that duplicate struct pci_dev members

[Jeff Garzik]

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/scsi/ips.c |  178 ++++++++++++++++++++++++----------------------------
 drivers/scsi/ips.h |   20 +++----
 2 files changed, 91 insertions(+), 107 deletions(-)

diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index 5c5a9b2..595a91a 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -707,7 +707,7 @@ ips_release(struct Scsi_Host *sh)
 		release_region(ha->io_addr, ha->io_len);
 
 	/* free IRQ */
-	free_irq(ha->irq, ha);
+	free_irq(ha->pcidev->irq, ha);
 
 	scsi_host_put(sh);
 
@@ -1637,7 +1637,7 @@ ips_make_passthru(ips_ha_t *ha, struct scsi_cmnd *SC, ips_scb_t *scb, int intr)
 				return (IPS_FAILURE);
 			}
 
-			if (ha->device_id == IPS_DEVICEID_COPPERHEAD &&
+			if (ha->pcidev->device == IPS_DEVICEID_COPPERHEAD &&
 			    pt->CoppCP.cmd.flashfw.op_code ==
 			    IPS_CMD_RW_BIOSFW) {
 				ret = ips_flash_copperhead(ha, pt, scb);
@@ -2021,7 +2021,7 @@ ips_cleanup_passthru(ips_ha_t * ha, ips_scb_t * scb)
 	pt->ExtendedStatus = scb->extended_status;
 	pt->AdapterType = ha->ad_type;
 
-	if (ha->device_id == IPS_DEVICEID_COPPERHEAD &&
+	if (ha->pcidev->device == IPS_DEVICEID_COPPERHEAD &&
 	    (scb->cmd.flashfw.op_code == IPS_CMD_DOWNLOAD ||
 	     scb->cmd.flashfw.op_code == IPS_CMD_RW_BIOSFW))
 		ips_free_flash_copperhead(ha);
@@ -2075,7 +2075,7 @@ ips_host_info(ips_ha_t * ha, char *ptr, off_t offset, int len)
 			  ha->mem_ptr);
 	}
 
-	copy_info(&info, "\tIRQ number                        : %d\n", ha->irq);
+	copy_info(&info, "\tIRQ number                        : %d\n", ha->pcidev->irq);
 
     /* For the Next 3 lines Check for Binary 0 at the end and don't include it if it's there. */
     /* That keeps everything happy for "text" operations on the proc file.                    */
@@ -2232,31 +2232,31 @@ ips_identify_controller(ips_ha_t * ha)
 {
 	METHOD_TRACE("ips_identify_controller", 1);
 
-	switch (ha->device_id) {
+	switch (ha->pcidev->device) {
 	case IPS_DEVICEID_COPPERHEAD:
-		if (ha->revision_id <= IPS_REVID_SERVERAID) {
+		if (ha->pcidev->revision <= IPS_REVID_SERVERAID) {
 			ha->ad_type = IPS_ADTYPE_SERVERAID;
-		} else if (ha->revision_id == IPS_REVID_SERVERAID2) {
+		} else if (ha->pcidev->revision == IPS_REVID_SERVERAID2) {
 			ha->ad_type = IPS_ADTYPE_SERVERAID2;
-		} else if (ha->revision_id == IPS_REVID_NAVAJO) {
+		} else if (ha->pcidev->revision == IPS_REVID_NAVAJO) {
 			ha->ad_type = IPS_ADTYPE_NAVAJO;
-		} else if ((ha->revision_id == IPS_REVID_SERVERAID2)
+		} else if ((ha->pcidev->revision == IPS_REVID_SERVERAID2)
 			   && (ha->slot_num == 0)) {
 			ha->ad_type = IPS_ADTYPE_KIOWA;
-		} else if ((ha->revision_id >= IPS_REVID_CLARINETP1) &&
-			   (ha->revision_id <= IPS_REVID_CLARINETP3)) {
+		} else if ((ha->pcidev->revision >= IPS_REVID_CLARINETP1) &&
+			   (ha->pcidev->revision <= IPS_REVID_CLARINETP3)) {
 			if (ha->enq->ucMaxPhysicalDevices == 15)
 				ha->ad_type = IPS_ADTYPE_SERVERAID3L;
 			else
 				ha->ad_type = IPS_ADTYPE_SERVERAID3;
-		} else if ((ha->revision_id >= IPS_REVID_TROMBONE32) &&
-			   (ha->revision_id <= IPS_REVID_TROMBONE64)) {
+		} else if ((ha->pcidev->revision >= IPS_REVID_TROMBONE32) &&
+			   (ha->pcidev->revision <= IPS_REVID_TROMBONE64)) {
 			ha->ad_type = IPS_ADTYPE_SERVERAID4H;
 		}
 		break;
 
 	case IPS_DEVICEID_MORPHEUS:
-		switch (ha->subdevice_id) {
+		switch (ha->pcidev->subsystem_device) {
 		case IPS_SUBDEVICEID_4L:
 			ha->ad_type = IPS_ADTYPE_SERVERAID4L;
 			break;
@@ -2285,7 +2285,7 @@ ips_identify_controller(ips_ha_t * ha)
 		break;
 
 	case IPS_DEVICEID_MARCO:
-		switch (ha->subdevice_id) {
+		switch (ha->pcidev->subsystem_device) {
 		case IPS_SUBDEVICEID_6M:
 			ha->ad_type = IPS_ADTYPE_SERVERAID6M;
 			break;
@@ -2332,20 +2332,20 @@ ips_get_bios_version(ips_ha_t * ha, int intr)
 
 	strncpy(ha->bios_version, "       ?", 8);
 
-	if (ha->device_id == IPS_DEVICEID_COPPERHEAD) {
+	if (ha->pcidev->device == IPS_DEVICEID_COPPERHEAD) {
 		if (IPS_USE_MEMIO(ha)) {
 			/* Memory Mapped I/O */
 
 			/* test 1st byte */
 			writel(0, ha->mem_ptr + IPS_REG_FLAP);
-			if (ha->revision_id == IPS_REVID_TROMBONE64)
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 				udelay(25);	/* 25 us */
 
 			if (readb(ha->mem_ptr + IPS_REG_FLDP) != 0x55)
 				return;
 
 			writel(1, ha->mem_ptr + IPS_REG_FLAP);
-			if (ha->revision_id == IPS_REVID_TROMBONE64)
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 				udelay(25);	/* 25 us */
 
 			if (readb(ha->mem_ptr + IPS_REG_FLDP) != 0xAA)
@@ -2353,20 +2353,20 @@ ips_get_bios_version(ips_ha_t * ha, int intr)
 
 			/* Get Major version */
 			writel(0x1FF, ha->mem_ptr + IPS_REG_FLAP);
-			if (ha->revision_id == IPS_REVID_TROMBONE64)
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 				udelay(25);	/* 25 us */
 
 			major = readb(ha->mem_ptr + IPS_REG_FLDP);
 
 			/* Get Minor version */
 			writel(0x1FE, ha->mem_ptr + IPS_REG_FLAP);
-			if (ha->revision_id == IPS_REVID_TROMBONE64)
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 				udelay(25);	/* 25 us */
 			minor = readb(ha->mem_ptr + IPS_REG_FLDP);
 
 			/* Get SubMinor version */
 			writel(0x1FD, ha->mem_ptr + IPS_REG_FLAP);
-			if (ha->revision_id == IPS_REVID_TROMBONE64)
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 				udelay(25);	/* 25 us */
 			subminor = readb(ha->mem_ptr + IPS_REG_FLDP);
 
@@ -2375,14 +2375,14 @@ ips_get_bios_version(ips_ha_t * ha, int intr)
 
 			/* test 1st byte */
 			outl(0, ha->io_addr + IPS_REG_FLAP);
-			if (ha->revision_id == IPS_REVID_TROMBONE64)
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 				udelay(25);	/* 25 us */
 
 			if (inb(ha->io_addr + IPS_REG_FLDP) != 0x55)
 				return;
 
 			outl(cpu_to_le32(1), ha->io_addr + IPS_REG_FLAP);
-			if (ha->revision_id == IPS_REVID_TROMBONE64)
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 				udelay(25);	/* 25 us */
 
 			if (inb(ha->io_addr + IPS_REG_FLDP) != 0xAA)
@@ -2390,21 +2390,21 @@ ips_get_bios_version(ips_ha_t * ha, int intr)
 
 			/* Get Major version */
 			outl(cpu_to_le32(0x1FF), ha->io_addr + IPS_REG_FLAP);
-			if (ha->revision_id == IPS_REVID_TROMBONE64)
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 				udelay(25);	/* 25 us */
 
 			major = inb(ha->io_addr + IPS_REG_FLDP);
 
 			/* Get Minor version */
 			outl(cpu_to_le32(0x1FE), ha->io_addr + IPS_REG_FLAP);
-			if (ha->revision_id == IPS_REVID_TROMBONE64)
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 				udelay(25);	/* 25 us */
 
 			minor = inb(ha->io_addr + IPS_REG_FLDP);
 
 			/* Get SubMinor version */
 			outl(cpu_to_le32(0x1FD), ha->io_addr + IPS_REG_FLAP);
-			if (ha->revision_id == IPS_REVID_TROMBONE64)
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 				udelay(25);	/* 25 us */
 
 			subminor = inb(ha->io_addr + IPS_REG_FLDP);
@@ -4903,7 +4903,7 @@ ips_init_copperhead(ips_ha_t * ha)
 	/* Enable busmastering */
 	outb(IPS_BIT_EBM, ha->io_addr + IPS_REG_SCPR);
 
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		/* fix for anaconda64 */
 		outl(0, ha->io_addr + IPS_REG_NDAE);
 
@@ -4997,7 +4997,7 @@ ips_init_copperhead_memio(ips_ha_t * ha)
 	/* Enable busmastering */
 	writeb(IPS_BIT_EBM, ha->mem_ptr + IPS_REG_SCPR);
 
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		/* fix for anaconda64 */
 		writel(0, ha->mem_ptr + IPS_REG_NDAE);
 
@@ -5142,7 +5142,7 @@ ips_reset_copperhead(ips_ha_t * ha)
 	METHOD_TRACE("ips_reset_copperhead", 1);
 
 	DEBUG_VAR(1, "(%s%d) ips_reset_copperhead: io addr: %x, irq: %d",
-		  ips_name, ha->host_num, ha->io_addr, ha->irq);
+		  ips_name, ha->host_num, ha->io_addr, ha->pcidev->irq);
 
 	reset_counter = 0;
 
@@ -5187,7 +5187,7 @@ ips_reset_copperhead_memio(ips_ha_t * ha)
 	METHOD_TRACE("ips_reset_copperhead_memio", 1);
 
 	DEBUG_VAR(1, "(%s%d) ips_reset_copperhead_memio: mem addr: %x, irq: %d",
-		  ips_name, ha->host_num, ha->mem_addr, ha->irq);
+		  ips_name, ha->host_num, ha->mem_addr, ha->pcidev->irq);
 
 	reset_counter = 0;
 
@@ -5233,7 +5233,7 @@ ips_reset_morpheus(ips_ha_t * ha)
 	METHOD_TRACE("ips_reset_morpheus", 1);
 
 	DEBUG_VAR(1, "(%s%d) ips_reset_morpheus: mem addr: %x, irq: %d",
-		  ips_name, ha->host_num, ha->mem_addr, ha->irq);
+		  ips_name, ha->host_num, ha->mem_addr, ha->pcidev->irq);
 
 	reset_counter = 0;
 
@@ -6196,32 +6196,32 @@ ips_erase_bios(ips_ha_t * ha)
 
 	/* Clear the status register */
 	outl(0, ha->io_addr + IPS_REG_FLAP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	outb(0x50, ha->io_addr + IPS_REG_FLDP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	/* Erase Setup */
 	outb(0x20, ha->io_addr + IPS_REG_FLDP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	/* Erase Confirm */
 	outb(0xD0, ha->io_addr + IPS_REG_FLDP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	/* Erase Status */
 	outb(0x70, ha->io_addr + IPS_REG_FLDP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	timeout = 80000;	/* 80 seconds */
 
 	while (timeout > 0) {
-		if (ha->revision_id == IPS_REVID_TROMBONE64) {
+		if (ha->pcidev->revision == IPS_REVID_TROMBONE64) {
 			outl(0, ha->io_addr + IPS_REG_FLAP);
 			udelay(25);	/* 25 us */
 		}
@@ -6241,13 +6241,13 @@ ips_erase_bios(ips_ha_t * ha)
 
 		/* try to suspend the erase */
 		outb(0xB0, ha->io_addr + IPS_REG_FLDP);
-		if (ha->revision_id == IPS_REVID_TROMBONE64)
+		if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 			udelay(25);	/* 25 us */
 
 		/* wait for 10 seconds */
 		timeout = 10000;
 		while (timeout > 0) {
-			if (ha->revision_id == IPS_REVID_TROMBONE64) {
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64) {
 				outl(0, ha->io_addr + IPS_REG_FLAP);
 				udelay(25);	/* 25 us */
 			}
@@ -6277,12 +6277,12 @@ ips_erase_bios(ips_ha_t * ha)
 	/* Otherwise, we were successful */
 	/* clear status */
 	outb(0x50, ha->io_addr + IPS_REG_FLDP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	/* enable reads */
 	outb(0xFF, ha->io_addr + IPS_REG_FLDP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	return (0);
@@ -6308,32 +6308,32 @@ ips_erase_bios_memio(ips_ha_t * ha)
 
 	/* Clear the status register */
 	writel(0, ha->mem_ptr + IPS_REG_FLAP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	writeb(0x50, ha->mem_ptr + IPS_REG_FLDP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	/* Erase Setup */
 	writeb(0x20, ha->mem_ptr + IPS_REG_FLDP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	/* Erase Confirm */
 	writeb(0xD0, ha->mem_ptr + IPS_REG_FLDP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	/* Erase Status */
 	writeb(0x70, ha->mem_ptr + IPS_REG_FLDP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	timeout = 80000;	/* 80 seconds */
 
 	while (timeout > 0) {
-		if (ha->revision_id == IPS_REVID_TROMBONE64) {
+		if (ha->pcidev->revision == IPS_REVID_TROMBONE64) {
 			writel(0, ha->mem_ptr + IPS_REG_FLAP);
 			udelay(25);	/* 25 us */
 		}
@@ -6353,13 +6353,13 @@ ips_erase_bios_memio(ips_ha_t * ha)
 
 		/* try to suspend the erase */
 		writeb(0xB0, ha->mem_ptr + IPS_REG_FLDP);
-		if (ha->revision_id == IPS_REVID_TROMBONE64)
+		if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 			udelay(25);	/* 25 us */
 
 		/* wait for 10 seconds */
 		timeout = 10000;
 		while (timeout > 0) {
-			if (ha->revision_id == IPS_REVID_TROMBONE64) {
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64) {
 				writel(0, ha->mem_ptr + IPS_REG_FLAP);
 				udelay(25);	/* 25 us */
 			}
@@ -6389,12 +6389,12 @@ ips_erase_bios_memio(ips_ha_t * ha)
 	/* Otherwise, we were successful */
 	/* clear status */
 	writeb(0x50, ha->mem_ptr + IPS_REG_FLDP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	/* enable reads */
 	writeb(0xFF, ha->mem_ptr + IPS_REG_FLDP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	return (0);
@@ -6423,21 +6423,21 @@ ips_program_bios(ips_ha_t * ha, char *buffer, uint32_t buffersize,
 	for (i = 0; i < buffersize; i++) {
 		/* write a byte */
 		outl(cpu_to_le32(i + offset), ha->io_addr + IPS_REG_FLAP);
-		if (ha->revision_id == IPS_REVID_TROMBONE64)
+		if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 			udelay(25);	/* 25 us */
 
 		outb(0x40, ha->io_addr + IPS_REG_FLDP);
-		if (ha->revision_id == IPS_REVID_TROMBONE64)
+		if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 			udelay(25);	/* 25 us */
 
 		outb(buffer[i], ha->io_addr + IPS_REG_FLDP);
-		if (ha->revision_id == IPS_REVID_TROMBONE64)
+		if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 			udelay(25);	/* 25 us */
 
 		/* wait up to one second */
 		timeout = 1000;
 		while (timeout > 0) {
-			if (ha->revision_id == IPS_REVID_TROMBONE64) {
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64) {
 				outl(0, ha->io_addr + IPS_REG_FLAP);
 				udelay(25);	/* 25 us */
 			}
@@ -6454,11 +6454,11 @@ ips_program_bios(ips_ha_t * ha, char *buffer, uint32_t buffersize,
 		if (timeout == 0) {
 			/* timeout error */
 			outl(0, ha->io_addr + IPS_REG_FLAP);
-			if (ha->revision_id == IPS_REVID_TROMBONE64)
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 				udelay(25);	/* 25 us */
 
 			outb(0xFF, ha->io_addr + IPS_REG_FLDP);
-			if (ha->revision_id == IPS_REVID_TROMBONE64)
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 				udelay(25);	/* 25 us */
 
 			return (1);
@@ -6468,11 +6468,11 @@ ips_program_bios(ips_ha_t * ha, char *buffer, uint32_t buffersize,
 		if (status & 0x18) {
 			/* programming error */
 			outl(0, ha->io_addr + IPS_REG_FLAP);
-			if (ha->revision_id == IPS_REVID_TROMBONE64)
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 				udelay(25);	/* 25 us */
 
 			outb(0xFF, ha->io_addr + IPS_REG_FLDP);
-			if (ha->revision_id == IPS_REVID_TROMBONE64)
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 				udelay(25);	/* 25 us */
 
 			return (1);
@@ -6481,11 +6481,11 @@ ips_program_bios(ips_ha_t * ha, char *buffer, uint32_t buffersize,
 
 	/* Enable reading */
 	outl(0, ha->io_addr + IPS_REG_FLAP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	outb(0xFF, ha->io_addr + IPS_REG_FLDP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	return (0);
@@ -6514,21 +6514,21 @@ ips_program_bios_memio(ips_ha_t * ha, char *buffer, uint32_t buffersize,
 	for (i = 0; i < buffersize; i++) {
 		/* write a byte */
 		writel(i + offset, ha->mem_ptr + IPS_REG_FLAP);
-		if (ha->revision_id == IPS_REVID_TROMBONE64)
+		if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 			udelay(25);	/* 25 us */
 
 		writeb(0x40, ha->mem_ptr + IPS_REG_FLDP);
-		if (ha->revision_id == IPS_REVID_TROMBONE64)
+		if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 			udelay(25);	/* 25 us */
 
 		writeb(buffer[i], ha->mem_ptr + IPS_REG_FLDP);
-		if (ha->revision_id == IPS_REVID_TROMBONE64)
+		if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 			udelay(25);	/* 25 us */
 
 		/* wait up to one second */
 		timeout = 1000;
 		while (timeout > 0) {
-			if (ha->revision_id == IPS_REVID_TROMBONE64) {
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64) {
 				writel(0, ha->mem_ptr + IPS_REG_FLAP);
 				udelay(25);	/* 25 us */
 			}
@@ -6545,11 +6545,11 @@ ips_program_bios_memio(ips_ha_t * ha, char *buffer, uint32_t buffersize,
 		if (timeout == 0) {
 			/* timeout error */
 			writel(0, ha->mem_ptr + IPS_REG_FLAP);
-			if (ha->revision_id == IPS_REVID_TROMBONE64)
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 				udelay(25);	/* 25 us */
 
 			writeb(0xFF, ha->mem_ptr + IPS_REG_FLDP);
-			if (ha->revision_id == IPS_REVID_TROMBONE64)
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 				udelay(25);	/* 25 us */
 
 			return (1);
@@ -6559,11 +6559,11 @@ ips_program_bios_memio(ips_ha_t * ha, char *buffer, uint32_t buffersize,
 		if (status & 0x18) {
 			/* programming error */
 			writel(0, ha->mem_ptr + IPS_REG_FLAP);
-			if (ha->revision_id == IPS_REVID_TROMBONE64)
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 				udelay(25);	/* 25 us */
 
 			writeb(0xFF, ha->mem_ptr + IPS_REG_FLDP);
-			if (ha->revision_id == IPS_REVID_TROMBONE64)
+			if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 				udelay(25);	/* 25 us */
 
 			return (1);
@@ -6572,11 +6572,11 @@ ips_program_bios_memio(ips_ha_t * ha, char *buffer, uint32_t buffersize,
 
 	/* Enable reading */
 	writel(0, ha->mem_ptr + IPS_REG_FLAP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	writeb(0xFF, ha->mem_ptr + IPS_REG_FLDP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	return (0);
@@ -6601,14 +6601,14 @@ ips_verify_bios(ips_ha_t * ha, char *buffer, uint32_t buffersize,
 
 	/* test 1st byte */
 	outl(0, ha->io_addr + IPS_REG_FLAP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	if (inb(ha->io_addr + IPS_REG_FLDP) != 0x55)
 		return (1);
 
 	outl(cpu_to_le32(1), ha->io_addr + IPS_REG_FLAP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 	if (inb(ha->io_addr + IPS_REG_FLDP) != 0xAA)
 		return (1);
@@ -6617,7 +6617,7 @@ ips_verify_bios(ips_ha_t * ha, char *buffer, uint32_t buffersize,
 	for (i = 2; i < buffersize; i++) {
 
 		outl(cpu_to_le32(i + offset), ha->io_addr + IPS_REG_FLAP);
-		if (ha->revision_id == IPS_REVID_TROMBONE64)
+		if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 			udelay(25);	/* 25 us */
 
 		checksum = (uint8_t) checksum + inb(ha->io_addr + IPS_REG_FLDP);
@@ -6650,14 +6650,14 @@ ips_verify_bios_memio(ips_ha_t * ha, char *buffer, uint32_t buffersize,
 
 	/* test 1st byte */
 	writel(0, ha->mem_ptr + IPS_REG_FLAP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 
 	if (readb(ha->mem_ptr + IPS_REG_FLDP) != 0x55)
 		return (1);
 
 	writel(1, ha->mem_ptr + IPS_REG_FLAP);
-	if (ha->revision_id == IPS_REVID_TROMBONE64)
+	if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 		udelay(25);	/* 25 us */
 	if (readb(ha->mem_ptr + IPS_REG_FLDP) != 0xAA)
 		return (1);
@@ -6666,7 +6666,7 @@ ips_verify_bios_memio(ips_ha_t * ha, char *buffer, uint32_t buffersize,
 	for (i = 2; i < buffersize; i++) {
 
 		writel(i + offset, ha->mem_ptr + IPS_REG_FLAP);
-		if (ha->revision_id == IPS_REVID_TROMBONE64)
+		if (ha->pcidev->revision == IPS_REVID_TROMBONE64)
 			udelay(25);	/* 25 us */
 
 		checksum =
@@ -6837,9 +6837,9 @@ ips_register_scsi(int index)
 	}
 	ha = IPS_HA(sh);
 	memcpy(ha, oldha, sizeof (ips_ha_t));
-	free_irq(oldha->irq, oldha);
+	free_irq(oldha->pcidev->irq, oldha);
 	/* Install the interrupt handler with the new ha */
-	if (request_irq(ha->irq, do_ipsintr, IRQF_SHARED, ips_name, ha)) {
+	if (request_irq(ha->pcidev->irq, do_ipsintr, IRQF_SHARED, ips_name, ha)) {
 		IPS_PRINTK(KERN_WARNING, ha->pcidev,
 			   "Unable to install interrupt handler\n");
 		scsi_host_put(sh);
@@ -6851,10 +6851,7 @@ ips_register_scsi(int index)
 	ips_ha[index] = ha;
 
 	/* Store away needed values for later use */
-	sh->io_port = ha->io_addr;
-	sh->n_io_port = ha->io_addr ? 255 : 0;
 	sh->unique_id = (ha->io_addr) ? ha->io_addr : ha->mem_addr;
-	sh->irq = ha->irq;
 	sh->sg_tablesize = sh->hostt->sg_tablesize;
 	sh->can_queue = sh->hostt->can_queue;
 	sh->cmd_per_lun = sh->hostt->cmd_per_lun;
@@ -6992,8 +6989,6 @@ ips_init_phase1(struct pci_dev *pci_dev, int *indexPtr)
 	uint32_t mem_len;
 	uint8_t bus;
 	uint8_t func;
-	uint8_t irq;
-	uint16_t subdevice_id;
 	int j;
 	int index;
 	dma_addr_t dma_address;
@@ -7014,7 +7009,6 @@ ips_init_phase1(struct pci_dev *pci_dev, int *indexPtr)
 		return -1;
 
 	/* stuff that we get in dev */
-	irq = pci_dev->irq;
 	bus = pci_dev->bus->number;
 	func = pci_dev->devfn;
 
@@ -7068,8 +7062,6 @@ ips_init_phase1(struct pci_dev *pci_dev, int *indexPtr)
 		}
 	}
 
-	subdevice_id = pci_dev->subsystem_device;
-
 	/* found a controller */
 	ha = kzalloc(sizeof (ips_ha_t), GFP_KERNEL);
 	if (ha == NULL) {
@@ -7084,7 +7076,6 @@ ips_init_phase1(struct pci_dev *pci_dev, int *indexPtr)
 	ha->active = 1;
 
 	/* Store info in HA structure */
-	ha->irq = irq;
 	ha->io_addr = io_addr;
 	ha->io_len = io_len;
 	ha->mem_addr = mem_addr;
@@ -7092,10 +7083,7 @@ ips_init_phase1(struct pci_dev *pci_dev, int *indexPtr)
 	ha->mem_ptr = mem_ptr;
 	ha->ioremap_ptr = ioremap_ptr;
 	ha->host_num = (uint32_t) index;
-	ha->revision_id = pci_dev->revision;
 	ha->slot_num = PCI_SLOT(pci_dev->devfn);
-	ha->device_id = pci_dev->device;
-	ha->subdevice_id = subdevice_id;
 	ha->pcidev = pci_dev;
 
 	/*
@@ -7240,7 +7228,7 @@ ips_init_phase2(int index)
 	}
 
 	/* Install the interrupt handler */
-	if (request_irq(ha->irq, do_ipsintr, IRQF_SHARED, ips_name, ha)) {
+	if (request_irq(ha->pcidev->irq, do_ipsintr, IRQF_SHARED, ips_name, ha)) {
 		IPS_PRINTK(KERN_WARNING, ha->pcidev,
 			   "Unable to install interrupt handler\n");
 		return ips_abort_init(ha, index);
@@ -7253,14 +7241,14 @@ ips_init_phase2(int index)
 	if (!ips_allocatescbs(ha)) {
 		IPS_PRINTK(KERN_WARNING, ha->pcidev,
 			   "Unable to allocate a CCB\n");
-		free_irq(ha->irq, ha);
+		free_irq(ha->pcidev->irq, ha);
 		return ips_abort_init(ha, index);
 	}
 
 	if (!ips_hainit(ha)) {
 		IPS_PRINTK(KERN_WARNING, ha->pcidev,
 			   "Unable to initialize controller\n");
-		free_irq(ha->irq, ha);
+		free_irq(ha->pcidev->irq, ha);
 		return ips_abort_init(ha, index);
 	}
 	/* Free the temporary SCB */
@@ -7270,7 +7258,7 @@ ips_init_phase2(int index)
 	if (!ips_allocatescbs(ha)) {
 		IPS_PRINTK(KERN_WARNING, ha->pcidev,
 			   "Unable to allocate CCBs\n");
-		free_irq(ha->irq, ha);
+		free_irq(ha->pcidev->irq, ha);
 		return ips_abort_init(ha, index);
 	}
 
diff --git a/drivers/scsi/ips.h b/drivers/scsi/ips.h
index 3bcbd9f..fb4a036 100644
--- a/drivers/scsi/ips.h
+++ b/drivers/scsi/ips.h
@@ -60,14 +60,14 @@
     */
    #define IPS_HA(x)                   ((ips_ha_t *) x->hostdata)
    #define IPS_COMMAND_ID(ha, scb)     (int) (scb - ha->scbs)
-   #define IPS_IS_TROMBONE(ha)         (((ha->device_id == IPS_DEVICEID_COPPERHEAD) && \
-                                         (ha->revision_id >= IPS_REVID_TROMBONE32) && \
-                                         (ha->revision_id <= IPS_REVID_TROMBONE64)) ? 1 : 0)
-   #define IPS_IS_CLARINET(ha)         (((ha->device_id == IPS_DEVICEID_COPPERHEAD) && \
-                                         (ha->revision_id >= IPS_REVID_CLARINETP1) && \
-                                         (ha->revision_id <= IPS_REVID_CLARINETP3)) ? 1 : 0)
-   #define IPS_IS_MORPHEUS(ha)         (ha->device_id == IPS_DEVICEID_MORPHEUS)
-   #define IPS_IS_MARCO(ha)            (ha->device_id == IPS_DEVICEID_MARCO)
+   #define IPS_IS_TROMBONE(ha)         (((ha->pcidev->device == IPS_DEVICEID_COPPERHEAD) && \
+                                         (ha->pcidev->revision >= IPS_REVID_TROMBONE32) && \
+                                         (ha->pcidev->revision <= IPS_REVID_TROMBONE64)) ? 1 : 0)
+   #define IPS_IS_CLARINET(ha)         (((ha->pcidev->device == IPS_DEVICEID_COPPERHEAD) && \
+                                         (ha->pcidev->revision >= IPS_REVID_CLARINETP1) && \
+                                         (ha->pcidev->revision <= IPS_REVID_CLARINETP3)) ? 1 : 0)
+   #define IPS_IS_MORPHEUS(ha)         (ha->pcidev->device == IPS_DEVICEID_MORPHEUS)
+   #define IPS_IS_MARCO(ha)            (ha->pcidev->device == IPS_DEVICEID_MARCO)
    #define IPS_USE_I2O_DELIVER(ha)     ((IPS_IS_MORPHEUS(ha) || \
                                          (IPS_IS_TROMBONE(ha) && \
                                           (ips_force_i2o))) ? 1 : 0)
@@ -1034,7 +1034,6 @@ typedef struct ips_ha {
    uint8_t            ha_id[IPS_MAX_CHANNELS+1];
    uint32_t           dcdb_active[IPS_MAX_CHANNELS];
    uint32_t           io_addr;            /* Base I/O address           */
-   uint8_t            irq;                /* IRQ for adapter            */
    uint8_t            ntargets;           /* Number of targets          */
    uint8_t            nbus;               /* Number of buses            */
    uint8_t            nlun;               /* Number of Luns             */
@@ -1066,10 +1065,7 @@ typedef struct ips_ha {
    int                ioctl_reset;        /* IOCTL Requested Reset Flag */
    uint16_t           reset_count;        /* number of resets           */
    time_t             last_ffdc;          /* last time we sent ffdc info*/
-   uint8_t            revision_id;        /* Revision level             */
-   uint16_t           device_id;          /* PCI device ID              */
    uint8_t            slot_num;           /* PCI Slot Number            */
-   uint16_t           subdevice_id;       /* Subsystem device ID        */
    int                ioctl_len;          /* size of ioctl buffer       */
    dma_addr_t         ioctl_busaddr;      /* dma address of ioctl buffer*/
    uint8_t            bios_version[8];    /* BIOS Revision              */
-- 
1.5.2.4

[PATCH 2/4] [SCSI] ips: trim trailing whitespace

[Jeff Garzik]

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/scsi/ips.c |   44 ++++++++++++++++++++++----------------------
 drivers/scsi/ips.h |   12 ++++++------
 2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index 595a91a..c9f3e1f 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -389,17 +389,17 @@ static struct  pci_device_id  ips_pci_table[] = {
 MODULE_DEVICE_TABLE( pci, ips_pci_table );
 
 static char ips_hot_plug_name[] = "ips";
-   
+
 static int __devinit  ips_insert_device(struct pci_dev *pci_dev, const struct pci_device_id *ent);
 static void __devexit ips_remove_device(struct pci_dev *pci_dev);
-   
+
 static struct pci_driver ips_pci_driver = {
 	.name		= ips_hot_plug_name,
 	.id_table	= ips_pci_table,
 	.probe		= ips_insert_device,
 	.remove		= __devexit_p(ips_remove_device),
 };
-           
+
 
 /*
  * Necessary forward function protoypes
@@ -587,7 +587,7 @@ static void
 ips_setup_funclist(ips_ha_t * ha)
 {
 
-	/*                                
+	/*
 	 * Setup Functions
 	 */
 	if (IPS_IS_MORPHEUS(ha) || IPS_IS_MARCO(ha)) {
@@ -2081,7 +2081,7 @@ ips_host_info(ips_ha_t * ha, char *ptr, off_t offset, int len)
     /* That keeps everything happy for "text" operations on the proc file.                    */
 
 	if (le32_to_cpu(ha->nvram->signature) == IPS_NVRAM_P5_SIG) {
-        if (ha->nvram->bios_low[3] == 0) { 
+        if (ha->nvram->bios_low[3] == 0) {
             copy_info(&info,
 			          "\tBIOS Version                      : %c%c%c%c%c%c%c\n",
 			          ha->nvram->bios_high[0], ha->nvram->bios_high[1],
@@ -2782,8 +2782,8 @@ ips_next(ips_ha_t * ha, int intr)
 
         /* Allow a WRITE BUFFER Command to Have no Data */
         /* This is Used by Tape Flash Utilites          */
-        if ((scb->scsi_cmd->cmnd[0] == WRITE_BUFFER) && (scb->data_len == 0)) 
-            scb->dcdb.cmd_attribute = 0;                  
+        if ((scb->scsi_cmd->cmnd[0] == WRITE_BUFFER) && (scb->data_len == 0))
+            scb->dcdb.cmd_attribute = 0;
 
 		if (!(scb->dcdb.cmd_attribute & 0x3))
 			scb->dcdb.transfer_length = 0;
@@ -3404,7 +3404,7 @@ ips_map_status(ips_ha_t * ha, ips_scb_t * scb, ips_stat_t * sp)
 
 				/* Restrict access to physical DASD */
 				if (scb->scsi_cmd->cmnd[0] == INQUIRY) {
-				    ips_scmd_buf_read(scb->scsi_cmd, 
+				    ips_scmd_buf_read(scb->scsi_cmd,
                                       &inquiryData, sizeof (inquiryData));
  				    if ((inquiryData.DeviceType & 0x1f) == TYPE_DISK) {
 				        errcode = DID_TIME_OUT;
@@ -4090,10 +4090,10 @@ ips_chkstatus(ips_ha_t * ha, IPS_STATUS * pstatus)
 			scb->scsi_cmd->result = errcode << 16;
 		} else {	/* bus == 0 */
 			/* restrict access to physical drives */
-			if (scb->scsi_cmd->cmnd[0] == INQUIRY) { 
-			    ips_scmd_buf_read(scb->scsi_cmd, 
+			if (scb->scsi_cmd->cmnd[0] == INQUIRY) {
+			    ips_scmd_buf_read(scb->scsi_cmd,
                                   &inquiryData, sizeof (inquiryData));
-			    if ((inquiryData.DeviceType & 0x1f) == TYPE_DISK) 
+			    if ((inquiryData.DeviceType & 0x1f) == TYPE_DISK)
 			        scb->scsi_cmd->result = DID_TIME_OUT << 16;
 			}
 		}		/* else */
@@ -4661,8 +4661,8 @@ ips_isinit_morpheus(ips_ha_t * ha)
 	uint32_t bits;
 
 	METHOD_TRACE("ips_is_init_morpheus", 1);
-   
-	if (ips_isintr_morpheus(ha)) 
+
+	if (ips_isintr_morpheus(ha))
 	    ips_flush_and_reset(ha);
 
 	post = readl(ha->mem_ptr + IPS_REG_I960_MSG0);
@@ -4686,7 +4686,7 @@ ips_isinit_morpheus(ips_ha_t * ha)
 /*   state ( was trying to INIT and an interrupt was already pending ) ...  */
 /*                                                                          */
 /****************************************************************************/
-static void 
+static void
 ips_flush_and_reset(ips_ha_t *ha)
 {
 	ips_scb_t *scb;
@@ -4718,9 +4718,9 @@ ips_flush_and_reset(ips_ha_t *ha)
 	    if (ret == IPS_SUCCESS) {
 	        time = 60 * IPS_ONE_SEC;	              /* Max Wait time is 60 seconds */
 	        done = 0;
-	            
+
 	        while ((time > 0) && (!done)) {
-	           done = ips_poll_for_flush_complete(ha); 	   
+	           done = ips_poll_for_flush_complete(ha);
 	           /* This may look evil, but it's only done during extremely rare start-up conditions ! */
 	           udelay(1000);
 	           time--;
@@ -4749,17 +4749,17 @@ static int
 ips_poll_for_flush_complete(ips_ha_t * ha)
 {
 	IPS_STATUS cstatus;
-    
+
 	while (TRUE) {
 	    cstatus.value = (*ha->func.statupd) (ha);
 
 	    if (cstatus.value == 0xffffffff)      /* If No Interrupt to process */
 			break;
-            
+
 	    /* Success is when we see the Flush Command ID */
-	    if (cstatus.fields.command_id == IPS_MAX_CMDS ) 
+	    if (cstatus.fields.command_id == IPS_MAX_CMDS )
 	        return 1;
-	 }	
+	 }
 
 	return 0;
 }
@@ -5920,7 +5920,7 @@ ips_read_config(ips_ha_t * ha, int intr)
 
 		return (0);
 	}
-	
+
 	memcpy(ha->conf, ha->ioctl_data, sizeof(*ha->conf));
 	return (1);
 }
@@ -5959,7 +5959,7 @@ ips_readwrite_page5(ips_ha_t * ha, int write, int intr)
 	scb->cmd.nvram.buffer_addr = ha->ioctl_busaddr;
 	if (write)
 		memcpy(ha->ioctl_data, ha->nvram, sizeof(*ha->nvram));
-	
+
 	/* issue the command */
 	if (((ret =
 	      ips_send_wait(ha, scb, ips_cmd_timeout, intr)) == IPS_FAILURE)
diff --git a/drivers/scsi/ips.h b/drivers/scsi/ips.h
index fb4a036..e0657b6 100644
--- a/drivers/scsi/ips.h
+++ b/drivers/scsi/ips.h
@@ -92,7 +92,7 @@
    #ifndef min
       #define min(x,y) ((x) < (y) ? x : y)
    #endif
-   
+
    #ifndef __iomem       /* For clean compiles in earlier kernels without __iomem annotations */
       #define __iomem
    #endif
@@ -171,7 +171,7 @@
    #define IPS_CMD_DOWNLOAD             0x20
    #define IPS_CMD_RW_BIOSFW            0x22
    #define IPS_CMD_GET_VERSION_INFO     0xC6
-   #define IPS_CMD_RESET_CHANNEL        0x1A  
+   #define IPS_CMD_RESET_CHANNEL        0x1A
 
    /*
     * Adapter Equates
@@ -458,7 +458,7 @@ typedef struct {
    uint32_t reserved3;
    uint32_t buffer_addr;
    uint32_t reserved4;
-} IPS_IOCTL_CMD, *PIPS_IOCTL_CMD; 
+} IPS_IOCTL_CMD, *PIPS_IOCTL_CMD;
 
 typedef struct {
    uint8_t  op_code;
@@ -552,7 +552,7 @@ typedef struct {
    uint32_t cccr;
 } IPS_NVRAM_CMD, *PIPS_NVRAM_CMD;
 
-typedef struct 
+typedef struct
 {
     uint8_t  op_code;
     uint8_t  command_id;
@@ -650,7 +650,7 @@ typedef struct {
    uint8_t   device_address;
    uint8_t   cmd_attribute;
    uint8_t   cdb_length;
-   uint8_t   reserved_for_LUN; 	 
+   uint8_t   reserved_for_LUN;
    uint32_t  transfer_length;
    uint32_t  buffer_pointer;
    uint16_t  sg_count;
@@ -790,7 +790,7 @@ typedef struct {
                                              /* SubSystem Parameter[4]      */
 #define  IPS_GET_VERSION_SUPPORT 0x00018000  /* Mask for Versioning Support */
 
-typedef struct 
+typedef struct
 {
    uint32_t  revision;
    uint8_t   bootBlkVersion[32];
-- 
1.5.2.4

[PATCH 3/4] [SCSI] ips: PCI API cleanups

[Jeff Garzik]

* pass Scsi_Host to ips_remove_device() via pci_set_drvdata(),
  allowing us to eliminate the ips_ha[] search loop and call
  ips_release() directly.

* call pci_{request,release}_regions() and eliminate individual
  request/release_[mem_]region() calls

* call pci_disable_device(), paired with pci_enable_device()

* s/0/NULL/ in a few places

* check ioremap() return value

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/scsi/ips.c |   72 ++++++++++++++++++++++-----------------------------
 1 files changed, 31 insertions(+), 41 deletions(-)

diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index c9f3e1f..fb90b6c 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -702,10 +702,6 @@ ips_release(struct Scsi_Host *sh)
 	/* free extra memory */
 	ips_free(ha);
 
-	/* Free I/O Region */
-	if (ha->io_addr)
-		release_region(ha->io_addr, ha->io_len);
-
 	/* free IRQ */
 	free_irq(ha->pcidev->irq, ha);
 
@@ -4393,8 +4389,6 @@ ips_free(ips_ha_t * ha)
 			ha->mem_ptr = NULL;
 		}
 
-		if (ha->mem_addr)
-			release_mem_region(ha->mem_addr, ha->mem_len);
 		ha->mem_addr = 0;
 
 	}
@@ -6879,20 +6873,14 @@ ips_register_scsi(int index)
 static void __devexit
 ips_remove_device(struct pci_dev *pci_dev)
 {
-	int i;
-	struct Scsi_Host *sh;
-	ips_ha_t *ha;
+	struct Scsi_Host *sh = pci_get_drvdata(pci_dev);
 
-	for (i = 0; i < IPS_MAX_ADAPTERS; i++) {
-		ha = ips_ha[i];
-		if (ha) {
-			if ((pci_dev->bus->number == ha->pcidev->bus->number) &&
-			    (pci_dev->devfn == ha->pcidev->devfn)) {
-				sh = ips_sh[i];
-				ips_release(sh);
-			}
-		}
-	}
+	pci_set_drvdata(pci_dev, NULL);
+
+	ips_release(sh);
+
+	pci_release_regions(pci_dev);
+	pci_disable_device(pci_dev);
 }
 
 /****************************************************************************/
@@ -6946,12 +6934,17 @@ module_exit(ips_module_exit);
 static int __devinit
 ips_insert_device(struct pci_dev *pci_dev, const struct pci_device_id *ent)
 {
-	int uninitialized_var(index);
+	int index = -1;
 	int rc;
 
 	METHOD_TRACE("ips_insert_device", 1);
-	if (pci_enable_device(pci_dev))
-		return -1;
+	rc = pci_enable_device(pci_dev);
+	if (rc)
+		return rc;
+
+	rc = pci_request_regions(pci_dev, "ips");
+	if (rc)
+		goto err_out;
 
 	rc = ips_init_phase1(pci_dev, &index);
 	if (rc == SUCCESS)
@@ -6967,6 +6960,19 @@ ips_insert_device(struct pci_dev *pci_dev, const struct pci_device_id *ent)
 		ips_num_controllers++;
 
 	ips_next_controller = ips_num_controllers;
+
+	if (rc < 0) {
+		rc = -ENODEV;
+		goto err_out_regions;
+	}
+
+	pci_set_drvdata(pci_dev, ips_sh[index]);
+	return 0;
+
+err_out_regions:
+	pci_release_regions(pci_dev);
+err_out:
+	pci_disable_device(pci_dev);
 	return rc;
 }
 
@@ -6999,7 +7005,7 @@ ips_init_phase1(struct pci_dev *pci_dev, int *indexPtr)
 	METHOD_TRACE("ips_init_phase1", 1);
 	index = IPS_MAX_ADAPTERS;
 	for (j = 0; j < IPS_MAX_ADAPTERS; j++) {
-		if (ips_ha[j] == 0) {
+		if (ips_ha[j] == NULL) {
 			index = j;
 			break;
 		}
@@ -7036,32 +7042,17 @@ ips_init_phase1(struct pci_dev *pci_dev, int *indexPtr)
 		uint32_t base;
 		uint32_t offs;
 
-		if (!request_mem_region(mem_addr, mem_len, "ips")) {
-			IPS_PRINTK(KERN_WARNING, pci_dev,
-				   "Couldn't allocate IO Memory space %x len %d.\n",
-				   mem_addr, mem_len);
-			return -1;
-		}
-
 		base = mem_addr & PAGE_MASK;
 		offs = mem_addr - base;
 		ioremap_ptr = ioremap(base, PAGE_SIZE);
+		if (!ioremap_ptr)
+			return -1;
 		mem_ptr = ioremap_ptr + offs;
 	} else {
 		ioremap_ptr = NULL;
 		mem_ptr = NULL;
 	}
 
-	/* setup I/O mapped area (if applicable) */
-	if (io_addr) {
-		if (!request_region(io_addr, io_len, "ips")) {
-			IPS_PRINTK(KERN_WARNING, pci_dev,
-				   "Couldn't allocate IO space %x len %d.\n",
-				   io_addr, io_len);
-			return -1;
-		}
-	}
-
 	/* found a controller */
 	ha = kzalloc(sizeof (ips_ha_t), GFP_KERNEL);
 	if (ha == NULL) {
@@ -7070,7 +7061,6 @@ ips_init_phase1(struct pci_dev *pci_dev, int *indexPtr)
 		return -1;
 	}
 
-
 	ips_sh[index] = NULL;
 	ips_ha[index] = ha;
 	ha->active = 1;
-- 
1.5.2.4

[PATCH 4/4] [SCSI] ips: handle scsi_add_host() failure, and other err cleanups

[Jeff Garzik]

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/scsi/ips.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index fb90b6c..b8e2f5a 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -6836,13 +6836,10 @@ ips_register_scsi(int index)
 	if (request_irq(ha->pcidev->irq, do_ipsintr, IRQF_SHARED, ips_name, ha)) {
 		IPS_PRINTK(KERN_WARNING, ha->pcidev,
 			   "Unable to install interrupt handler\n");
-		scsi_host_put(sh);
-		return -1;
+		goto err_out_sh;
 	}
 
 	kfree(oldha);
-	ips_sh[index] = sh;
-	ips_ha[index] = ha;
 
 	/* Store away needed values for later use */
 	sh->unique_id = (ha->io_addr) ? ha->io_addr : ha->mem_addr;
@@ -6858,10 +6855,21 @@ ips_register_scsi(int index)
 	sh->max_channel = ha->nbus - 1;
 	sh->can_queue = ha->max_cmds - 1;
 
-	scsi_add_host(sh, NULL);
+	if (scsi_add_host(sh, &ha->pcidev->dev))
+		goto err_out;
+
+	ips_sh[index] = sh;
+	ips_ha[index] = ha;
+
 	scsi_scan_host(sh);
 
 	return 0;
+
+err_out:
+	free_irq(ha->pcidev->irq, ha);
+err_out_sh:
+	scsi_host_put(sh);
+	return -1;
 }
 
 /*---------------------------------------------------------------------------*/
-- 
1.5.2.4

[PATCH] [libata] fix 'if(' and similar areas that lack whitespace

[Jeff Garzik]

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/ata/pata_acpi.c         |    4 +-
 drivers/ata/pata_optidma.c      |    2 +-
 drivers/ata/pata_pdc2027x.c     |    2 +-
 drivers/ata/pata_pdc202xx_old.c |    4 +-
 drivers/ata/pata_via.c          |    2 +-
 drivers/ata/pata_winbond.c      |    2 +-
 drivers/ata/sata_nv.c           |   46 +++++++++++++++++++-------------------
 drivers/ata/sata_sx4.c          |    4 +-
 8 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/ata/pata_acpi.c b/drivers/ata/pata_acpi.c
index 0f6f7bc..e4542ab 100644
--- a/drivers/ata/pata_acpi.c
+++ b/drivers/ata/pata_acpi.c
@@ -181,7 +181,7 @@ static void pacpi_set_piomode(struct ata_port *ap, struct ata_device *adev)
 	int unit = adev->devno;
 	struct pata_acpi *acpi = ap->private_data;
 
-	if(!(acpi->gtm.flags & 0x10))
+	if (!(acpi->gtm.flags & 0x10))
 		unit = 0;
 
 	/* Now stuff the nS values into the structure */
@@ -202,7 +202,7 @@ static void pacpi_set_dmamode(struct ata_port *ap, struct ata_device *adev)
 	int unit = adev->devno;
 	struct pata_acpi *acpi = ap->private_data;
 
-	if(!(acpi->gtm.flags & 0x10))
+	if (!(acpi->gtm.flags & 0x10))
 		unit = 0;
 
 	/* Now stuff the nS values into the structure */
diff --git a/drivers/ata/pata_optidma.c b/drivers/ata/pata_optidma.c
index 6b07b5b..f9b485a 100644
--- a/drivers/ata/pata_optidma.c
+++ b/drivers/ata/pata_optidma.c
@@ -449,7 +449,7 @@ static int optiplus_with_udma(struct pci_dev *pdev)
 
 	/* Find function 1 */
 	dev1 = pci_get_device(0x1045, 0xC701, NULL);
-	if(dev1 == NULL)
+	if (dev1 == NULL)
 		return 0;
 
 	/* Rev must be >= 0x10 */
diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c
index 3d3f155..2622577 100644
--- a/drivers/ata/pata_pdc2027x.c
+++ b/drivers/ata/pata_pdc2027x.c
@@ -348,7 +348,7 @@ static unsigned long pdc2027x_mode_filter(struct ata_device *adev, unsigned long
 	ata_id_c_string(pair->id, model_num, ATA_ID_PROD,
 			  ATA_ID_PROD_LEN + 1);
 	/* If the master is a maxtor in UDMA6 then the slave should not use UDMA 6 */
-	if(strstr(model_num, "Maxtor") == 0 && pair->dma_mode == XFER_UDMA_6)
+	if (strstr(model_num, "Maxtor") == 0 && pair->dma_mode == XFER_UDMA_6)
 		mask &= ~ (1 << (6 + ATA_SHIFT_UDMA));
 
 	return ata_pci_default_filter(adev, mask);
diff --git a/drivers/ata/pata_pdc202xx_old.c b/drivers/ata/pata_pdc202xx_old.c
index 65d9516..bc7c2d5 100644
--- a/drivers/ata/pata_pdc202xx_old.c
+++ b/drivers/ata/pata_pdc202xx_old.c
@@ -351,9 +351,9 @@ static int pdc202xx_init_one(struct pci_dev *dev, const struct pci_device_id *id
 		struct pci_dev *bridge = dev->bus->self;
 		/* Don't grab anything behind a Promise I2O RAID */
 		if (bridge && bridge->vendor == PCI_VENDOR_ID_INTEL) {
-			if( bridge->device == PCI_DEVICE_ID_INTEL_I960)
+			if (bridge->device == PCI_DEVICE_ID_INTEL_I960)
 				return -ENODEV;
-			if( bridge->device == PCI_DEVICE_ID_INTEL_I960RM)
+			if (bridge->device == PCI_DEVICE_ID_INTEL_I960RM)
 				return -ENODEV;
 		}
 	}
diff --git a/drivers/ata/pata_via.c b/drivers/ata/pata_via.c
index ea7a9a6..a4175fb 100644
--- a/drivers/ata/pata_via.c
+++ b/drivers/ata/pata_via.c
@@ -176,7 +176,7 @@ static int via_cable_detect(struct ata_port *ap) {
 	if ((config->flags & VIA_UDMA) < VIA_UDMA_66)
 		return ATA_CBL_PATA40;
 	/* UDMA 66 chips have only drive side logic */
-	else if((config->flags & VIA_UDMA) < VIA_UDMA_100)
+	else if ((config->flags & VIA_UDMA) < VIA_UDMA_100)
 		return ATA_CBL_PATA_UNK;
 	/* UDMA 100 or later */
 	pci_read_config_dword(pdev, 0x50, &ata66);
diff --git a/drivers/ata/pata_winbond.c b/drivers/ata/pata_winbond.c
index 549cbbe..311cdb3 100644
--- a/drivers/ata/pata_winbond.c
+++ b/drivers/ata/pata_winbond.c
@@ -279,7 +279,7 @@ static __init int winbond_init(void)
 
 			if (request_region(port, 2, "pata_winbond")) {
 				ret = winbond_init_one(port);
-				if(ret <= 0)
+				if (ret <= 0)
 					release_region(port, 2);
 				else ct+= ret;
 			}
diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c
index 2e0279f..9210bb2 100644
--- a/drivers/ata/sata_nv.c
+++ b/drivers/ata/sata_nv.c
@@ -1012,7 +1012,7 @@ static irqreturn_t nv_adma_interrupt(int irq, void *dev_instance)
 				u32 check_commands;
 				int pos, error = 0;
 
-				if(ata_tag_valid(ap->link.active_tag))
+				if (ata_tag_valid(ap->link.active_tag))
 					check_commands = 1 << ap->link.active_tag;
 				else
 					check_commands = ap->link.sactive;
@@ -1028,7 +1028,7 @@ static irqreturn_t nv_adma_interrupt(int irq, void *dev_instance)
 		}
 	}
 
-	if(notifier_clears[0] || notifier_clears[1]) {
+	if (notifier_clears[0] || notifier_clears[1]) {
 		/* Note: Both notifier clear registers must be written
 		   if either is set, even if one is zero, according to NVIDIA. */
 		struct nv_adma_port_priv *pp = host->ports[0]->private_data;
@@ -1119,7 +1119,7 @@ static void nv_adma_post_internal_cmd(struct ata_queued_cmd *qc)
 {
 	struct nv_adma_port_priv *pp = qc->ap->private_data;
 
-	if(pp->flags & NV_ADMA_PORT_REGISTER_MODE)
+	if (pp->flags & NV_ADMA_PORT_REGISTER_MODE)
 		ata_bmdma_post_internal_cmd(qc);
 }
 
@@ -1194,10 +1194,10 @@ static int nv_adma_port_start(struct ata_port *ap)
 
 	tmp = readw(mmio + NV_ADMA_CTL);
 	writew(tmp | NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
-	readw( mmio + NV_ADMA_CTL );	/* flush posted write */
+	readw(mmio + NV_ADMA_CTL );	/* flush posted write */
 	udelay(1);
 	writew(tmp & ~NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
-	readw( mmio + NV_ADMA_CTL );	/* flush posted write */
+	readw(mmio + NV_ADMA_CTL );	/* flush posted write */
 
 	return 0;
 }
@@ -1255,10 +1255,10 @@ static int nv_adma_port_resume(struct ata_port *ap)
 
 	tmp = readw(mmio + NV_ADMA_CTL);
 	writew(tmp | NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
-	readw( mmio + NV_ADMA_CTL );	/* flush posted write */
+	readw(mmio + NV_ADMA_CTL );	/* flush posted write */
 	udelay(1);
 	writew(tmp & ~NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
-	readw( mmio + NV_ADMA_CTL );	/* flush posted write */
+	readw(mmio + NV_ADMA_CTL );	/* flush posted write */
 
 	return 0;
 }
@@ -1359,12 +1359,12 @@ static int nv_adma_use_reg_mode(struct ata_queued_cmd *qc)
 	/* ADMA engine can only be used for non-ATAPI DMA commands,
 	   or interrupt-driven no-data commands, where a result taskfile
 	   is not required. */
-	if((pp->flags & NV_ADMA_ATAPI_SETUP_COMPLETE) ||
+	if ((pp->flags & NV_ADMA_ATAPI_SETUP_COMPLETE) ||
 	   (qc->tf.flags & ATA_TFLAG_POLLING) ||
 	   (qc->flags & ATA_QCFLAG_RESULT_TF))
 		return 1;
 
-	if((qc->flags & ATA_QCFLAG_DMAMAP) ||
+	if ((qc->flags & ATA_QCFLAG_DMAMAP) ||
 	   (qc->tf.protocol == ATA_PROT_NODATA))
 		return 0;
 
@@ -1401,7 +1401,7 @@ static void nv_adma_qc_prep(struct ata_queued_cmd *qc)
 
 	nv_adma_tf_to_cpb(&qc->tf, cpb->tf);
 
-	if(qc->flags & ATA_QCFLAG_DMAMAP) {
+	if (qc->flags & ATA_QCFLAG_DMAMAP) {
 		nv_adma_fill_sg(qc, cpb);
 		ctl_flags |= NV_CPB_CTL_APRD_VALID;
 	} else
@@ -1435,7 +1435,7 @@ static unsigned int nv_adma_qc_issue(struct ata_queued_cmd *qc)
 	   and (number of cpbs to append -1) in top 8 bits */
 	wmb();
 
-	if(curr_ncq != pp->last_issue_ncq) {
+	if (curr_ncq != pp->last_issue_ncq) {
 	   	/* Seems to need some delay before switching between NCQ and non-NCQ
 		   commands, else we get command timeouts and such. */
 		udelay(20);
@@ -1641,12 +1641,12 @@ static void nv_error_handler(struct ata_port *ap)
 static void nv_adma_error_handler(struct ata_port *ap)
 {
 	struct nv_adma_port_priv *pp = ap->private_data;
-	if(!(pp->flags & NV_ADMA_PORT_REGISTER_MODE)) {
+	if (!(pp->flags & NV_ADMA_PORT_REGISTER_MODE)) {
 		void __iomem *mmio = pp->ctl_block;
 		int i;
 		u16 tmp;
 
-		if(ata_tag_valid(ap->link.active_tag) || ap->link.sactive) {
+		if (ata_tag_valid(ap->link.active_tag) || ap->link.sactive) {
 			u32 notifier = readl(mmio + NV_ADMA_NOTIFIER);
 			u32 notifier_error = readl(mmio + NV_ADMA_NOTIFIER_ERROR);
 			u32 gen_ctl = readl(pp->gen_block + NV_ADMA_GEN_CTL);
@@ -1660,9 +1660,9 @@ static void nv_adma_error_handler(struct ata_port *ap)
 				notifier, notifier_error, gen_ctl, status,
 				cpb_count, next_cpb_idx);
 
-			for( i=0;i<NV_ADMA_MAX_CPBS;i++) {
+			for (i = 0; i < NV_ADMA_MAX_CPBS; i++) {
 				struct nv_adma_cpb *cpb = &pp->cpb[i];
-				if( (ata_tag_valid(ap->link.active_tag) && i == ap->link.active_tag) ||
+				if ((ata_tag_valid(ap->link.active_tag) && i == ap->link.active_tag) ||
 				    ap->link.sactive & (1 << i) )
 					ata_port_printk(ap, KERN_ERR,
 						"CPB %d: ctl_flags 0x%x, resp_flags 0x%x\n",
@@ -1674,7 +1674,7 @@ static void nv_adma_error_handler(struct ata_port *ap)
 		nv_adma_register_mode(ap);
 
 		/* Mark all of the CPBs as invalid to prevent them from being executed */
-		for( i=0;i<NV_ADMA_MAX_CPBS;i++)
+		for (i = 0; i < NV_ADMA_MAX_CPBS; i++)
 			pp->cpb[i].ctl_flags &= ~NV_CPB_CTL_CPB_VALID;
 
 		/* clear CPB fetch count */
@@ -1683,10 +1683,10 @@ static void nv_adma_error_handler(struct ata_port *ap)
 		/* Reset channel */
 		tmp = readw(mmio + NV_ADMA_CTL);
 		writew(tmp | NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
-		readw( mmio + NV_ADMA_CTL );	/* flush posted write */
+		readw(mmio + NV_ADMA_CTL);	/* flush posted write */
 		udelay(1);
 		writew(tmp & ~NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
-		readw( mmio + NV_ADMA_CTL );	/* flush posted write */
+		readw(mmio + NV_ADMA_CTL);	/* flush posted write */
 	}
 
 	ata_bmdma_drive_eh(ap, ata_std_prereset, ata_std_softreset,
@@ -2440,32 +2440,32 @@ static int nv_pci_device_resume(struct pci_dev *pdev)
 	int rc;
 
 	rc = ata_pci_device_do_resume(pdev);
-	if(rc)
+	if (rc)
 		return rc;
 
 	if (pdev->dev.power.power_state.event == PM_EVENT_SUSPEND) {
-		if(hpriv->type >= CK804) {
+		if (hpriv->type >= CK804) {
 			u8 regval;
 
 			pci_read_config_byte(pdev, NV_MCP_SATA_CFG_20, &regval);
 			regval |= NV_MCP_SATA_CFG_20_SATA_SPACE_EN;
 			pci_write_config_byte(pdev, NV_MCP_SATA_CFG_20, regval);
 		}
-		if(hpriv->type == ADMA) {
+		if (hpriv->type == ADMA) {
 			u32 tmp32;
 			struct nv_adma_port_priv *pp;
 			/* enable/disable ADMA on the ports appropriately */
 			pci_read_config_dword(pdev, NV_MCP_SATA_CFG_20, &tmp32);
 
 			pp = host->ports[0]->private_data;
-			if(pp->flags & NV_ADMA_ATAPI_SETUP_COMPLETE)
+			if (pp->flags & NV_ADMA_ATAPI_SETUP_COMPLETE)
 				tmp32 &= ~(NV_MCP_SATA_CFG_20_PORT0_EN |
 				 	   NV_MCP_SATA_CFG_20_PORT0_PWB_EN);
 			else
 				tmp32 |=  (NV_MCP_SATA_CFG_20_PORT0_EN |
 				 	   NV_MCP_SATA_CFG_20_PORT0_PWB_EN);
 			pp = host->ports[1]->private_data;
-			if(pp->flags & NV_ADMA_ATAPI_SETUP_COMPLETE)
+			if (pp->flags & NV_ADMA_ATAPI_SETUP_COMPLETE)
 				tmp32 &= ~(NV_MCP_SATA_CFG_20_PORT1_EN |
 				 	   NV_MCP_SATA_CFG_20_PORT1_PWB_EN);
 			else
diff --git a/drivers/ata/sata_sx4.c b/drivers/ata/sata_sx4.c
index b6026bc..16b3b8a 100644
--- a/drivers/ata/sata_sx4.c
+++ b/drivers/ata/sata_sx4.c
@@ -1091,7 +1091,7 @@ static int pdc20621_detect_dimm(struct ata_host *host)
 		return 0;
 
 	if (pdc20621_i2c_read(host, PDC_DIMM0_SPD_DEV_ADDRESS, 9, &data)) {
-		if(data <= 0x75)
+		if (data <= 0x75)
 			return 133;
    	} else
 		return 0;
@@ -1254,7 +1254,7 @@ static unsigned int pdc20621_dimm_init(struct ata_host *host)
 	   If SX4 is on PCI-X bus, after 3 seconds, the timer counter
 	   register should be >= (0xffffffff - 3x10^8).
 	*/
-	if(tcount >= PCI_X_TCOUNT) {
+	if (tcount >= PCI_X_TCOUNT) {
 		ticks = (time_period - tcount);
 		VPRINTK("Num counters 0x%x (%d)\n", ticks, ticks);
 
-- 
1.5.2.4

Re: [PATCH 1/4] [SCSI] ips: remove ips_ha members that duplicate struct pci_dev members

[Andrew Morton]

On Wed, 24 Oct 2007 19:48:26 -0400 (EDT) Jeff Garzik <jeff@garzik.org> wrote:

>  drivers/scsi/ips.c |  178 ++++++++++++++++++++++++----------------------------

this driver seems a bit of a basket case :(


What's going on here?

		scb->dcdb.cmd_attribute =
		    ips_command_direction[scb->scsi_cmd->cmnd[0]];

        /* Allow a WRITE BUFFER Command to Have no Data */
        /* This is Used by Tape Flash Utilites          */
        if ((scb->scsi_cmd->cmnd[0] == WRITE_BUFFER) && (scb->data_len == 0)) 
            scb->dcdb.cmd_attribute = 0;                  

		if (!(scb->dcdb.cmd_attribute & 0x3))
			scb->dcdb.transfer_length = 0;

		if (scb->data_len >= IPS_MAX_XFER) {

I hope that's just busted indentation and not a missing {} block.

Re: [PATCH 1/4] [SCSI] ips: remove ips_ha members that duplicate struct pci_dev members

[Jeff Garzik]

Andrew Morton wrote:
> On Wed, 24 Oct 2007 19:48:26 -0400 (EDT) Jeff Garzik <jeff@garzik.org> wrote:
> 
>>  drivers/scsi/ips.c |  178 ++++++++++++++++++++++++----------------------------
> 
> this driver seems a bit of a basket case :(
> 
> 
> What's going on here?
> 
> 		scb->dcdb.cmd_attribute =
> 		    ips_command_direction[scb->scsi_cmd->cmnd[0]];
> 
>         /* Allow a WRITE BUFFER Command to Have no Data */
>         /* This is Used by Tape Flash Utilites          */
>         if ((scb->scsi_cmd->cmnd[0] == WRITE_BUFFER) && (scb->data_len == 0)) 
>             scb->dcdb.cmd_attribute = 0;                  
> 
> 		if (!(scb->dcdb.cmd_attribute & 0x3))
> 			scb->dcdb.transfer_length = 0;
> 
> 		if (scb->data_len >= IPS_MAX_XFER) {
> 
> I hope that's just busted indentation and not a missing {} block.

The driver is one of the grotty drivers people are afraid to touch, 
therefore it bitrots even faster, in a vicious cycle.  You don't have to 
look hard at all to find checkpatch pukeage, very real bugs, and 
Pythonesque silliness.

Not having hardware I went only for changes that are provable and 
obvious, really...

	Jeff

Re: [PATCH 3/4] [SCSI] ips: PCI API cleanups

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 1373 bytes --]

Jeff Garzik wrote:
> * pass Scsi_Host to ips_remove_device() via pci_set_drvdata(),
>   allowing us to eliminate the ips_ha[] search loop and call
>   ips_release() directly.
>
> * call pci_{request,release}_regions() and eliminate individual
>   request/release_[mem_]region() calls
>
> * call pci_disable_device(), paired with pci_enable_device()
>
> * s/0/NULL/ in a few places
>
> * check ioremap() return value

> @@ -7036,32 +7042,17 @@ ips_init_phase1(struct pci_dev *pci_dev, int
> *indexPtr) uint32_t base;
>  		uint32_t offs;
>
> -		if (!request_mem_region(mem_addr, mem_len, "ips")) {
> -			IPS_PRINTK(KERN_WARNING, pci_dev,
> -				   "Couldn't allocate IO Memory space %x len %d.\n",
> -				   mem_addr, mem_len);
> -			return -1;
> -		}
> -
>  		base = mem_addr & PAGE_MASK;
>  		offs = mem_addr - base;
>  		ioremap_ptr = ioremap(base, PAGE_SIZE);

This looks odd. What are we actually doing here?

It seems that we want to map that PCI BAR. Since we're playing with PAGE_MASK 
I assume the BAR always has a length <PAGE_SIZE, else we would get page 
aligned memory anyway. If that's true something like

mem_ptr = pci_iomap(pci_dev, bar, 0)

should do the trick. Later we would do

pci_iounmap(pci_dev, mem_ptr)

This whole ioremap_ptr stuff can go away at all. Am I right? Then I'll cook up 
a patch soon.

Eike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 194 bytes --]

RE: [PATCH 1/4] [SCSI] ips: remove ips_ha members that duplicate struct pci_dev members

[Salyzyn, Mark]

ACK. Inspected; Mechanical, precise and no introduction of bugs.

Sincerely -- Mark Salyzyn

> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org 
> [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Jeff Garzik
> Sent: Wednesday, October 24, 2007 7:48 PM
> To: LKML; linux-scsi@vger.kernel.org
> Cc: akpm@linux-foundation.org
> Subject: [PATCH 1/4] [SCSI] ips: remove ips_ha members that 
> duplicate struct pci_dev members
> 
> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> ---
>  drivers/scsi/ips.c |  178 
> ++++++++++++++++++++++++----------------------------
>  drivers/scsi/ips.h |   20 +++----
>  2 files changed, 91 insertions(+), 107 deletions(-)

RE: [PATCH 2/4] [SCSI] ips: trim trailing whitespace

[Salyzyn, Mark]

ACK. Inspected; trivial, clean and no sign of any code changes.

Sincerely -- Mark Salyzyn

> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org 
> [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Jeff Garzik
> Sent: Wednesday, October 24, 2007 7:48 PM
> To: LKML; linux-scsi@vger.kernel.org
> Cc: akpm@linux-foundation.org
> Subject: [PATCH 2/4] [SCSI] ips: trim trailing whitespace
> 
> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> ---
>  drivers/scsi/ips.c |   44 
> ++++++++++++++++++++++----------------------
>  drivers/scsi/ips.h |   12 ++++++------
>  2 files changed, 28 insertions(+), 28 deletions(-)

RE: [PATCH 3/4] [SCSI] ips: PCI API cleanups

[Salyzyn, Mark]

ACK. Inspected only. Looks ok.

Sincerely -- Mark Salyzyn

> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org 
> [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Jeff Garzik
> Sent: Wednesday, October 24, 2007 7:49 PM
> To: LKML; linux-scsi@vger.kernel.org
> Cc: akpm@linux-foundation.org
> Subject: [PATCH 3/4] [SCSI] ips: PCI API cleanups
> 
> * pass Scsi_Host to ips_remove_device() via pci_set_drvdata(),
>   allowing us to eliminate the ips_ha[] search loop and call
>   ips_release() directly.
> 
> * call pci_{request,release}_regions() and eliminate individual
>   request/release_[mem_]region() calls
> 
> * call pci_disable_device(), paired with pci_enable_device()
> 
> * s/0/NULL/ in a few places
> 
> * check ioremap() return value
> 
> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> ---
>  drivers/scsi/ips.c |   72 
> ++++++++++++++++++++++-----------------------------
>  1 files changed, 31 insertions(+), 41 deletions(-)

RE: [PATCH 4/4] [SCSI] ips: handle scsi_add_host() failure, and other err cleanups

[Salyzyn, Mark]

ACK. Inspected. cleanup with zero risk.

Sincerely -- Mark Salyzyn

> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org 
> [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Jeff Garzik
> Sent: Wednesday, October 24, 2007 7:49 PM
> To: LKML; linux-scsi@vger.kernel.org
> Cc: akpm@linux-foundation.org
> Subject: [PATCH 4/4] [SCSI] ips: handle scsi_add_host() 
> failure, and other err cleanups
> 
> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> ---
>  drivers/scsi/ips.c |   18 +++++++++++++-----
>  1 files changed, 13 insertions(+), 5 deletions(-)

Re: [PATCH] [libata] fix 'if(' and similar areas that lack whitespace

[Richard Knutsson]

Jeff Garzik wrote:
> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> ---
>  drivers/ata/pata_acpi.c         |    4 +-
>  drivers/ata/pata_optidma.c      |    2 +-
>  drivers/ata/pata_pdc2027x.c     |    2 +-
>  drivers/ata/pata_pdc202xx_old.c |    4 +-
>  drivers/ata/pata_via.c          |    2 +-
>  drivers/ata/pata_winbond.c      |    2 +-
>  drivers/ata/sata_nv.c           |   46 +++++++++++++++++++-------------------
>  drivers/ata/sata_sx4.c          |    4 +-
>  8 files changed, 33 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/ata/pata_acpi.c b/drivers/ata/pata_acpi.c
> index 0f6f7bc..e4542ab 100644
> --- a/drivers/ata/pata_acpi.c
> +++ b/drivers/ata/pata_acpi.c
> @@ -181,7 +181,7 @@ static void pacpi_set_piomode(struct ata_port *ap, struct ata_device *adev)
>  	int unit = adev->devno;
>  	struct pata_acpi *acpi = ap->private_data;
>  
> -	if(!(acpi->gtm.flags & 0x10))
> +	if (!(acpi->gtm.flags & 0x10))
>  		unit = 0;
>  
>  	/* Now stuff the nS values into the structure */
> @@ -202,7 +202,7 @@ static void pacpi_set_dmamode(struct ata_port *ap, struct ata_device *adev)
>  	int unit = adev->devno;
>  	struct pata_acpi *acpi = ap->private_data;
>  
> -	if(!(acpi->gtm.flags & 0x10))
> +	if (!(acpi->gtm.flags & 0x10))
>  		unit = 0;
>  
>  	/* Now stuff the nS values into the structure */
> diff --git a/drivers/ata/pata_optidma.c b/drivers/ata/pata_optidma.c
> index 6b07b5b..f9b485a 100644
> --- a/drivers/ata/pata_optidma.c
> +++ b/drivers/ata/pata_optidma.c
> @@ -449,7 +449,7 @@ static int optiplus_with_udma(struct pci_dev *pdev)
>  
>  	/* Find function 1 */
>  	dev1 = pci_get_device(0x1045, 0xC701, NULL);
> -	if(dev1 == NULL)
> +	if (dev1 == NULL)
>   
Maybe "if (!dev1)" instead?
>  		return 0;
>  
>  	/* Rev must be >= 0x10 */
>
>   
<snip>
> diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c
> index 2e0279f..9210bb2 100644
> --- a/drivers/ata/sata_nv.c
> +++ b/drivers/ata/sata_nv.c
> @@ -1012,7 +1012,7 @@ static irqreturn_t nv_adma_interrupt(int irq, void *dev_instance)
>  				u32 check_commands;
>  				int pos, error = 0;
>  
> -				if(ata_tag_valid(ap->link.active_tag))
> +				if (ata_tag_valid(ap->link.active_tag))
>  					check_commands = 1 << ap->link.active_tag;
>  				else
>  					check_commands = ap->link.sactive;
> @@ -1028,7 +1028,7 @@ static irqreturn_t nv_adma_interrupt(int irq, void *dev_instance)
>  		}
>  	}
>  
> -	if(notifier_clears[0] || notifier_clears[1]) {
> +	if (notifier_clears[0] || notifier_clears[1]) {
>  		/* Note: Both notifier clear registers must be written
>  		   if either is set, even if one is zero, according to NVIDIA. */
>  		struct nv_adma_port_priv *pp = host->ports[0]->private_data;
> @@ -1119,7 +1119,7 @@ static void nv_adma_post_internal_cmd(struct ata_queued_cmd *qc)
>  {
>  	struct nv_adma_port_priv *pp = qc->ap->private_data;
>  
> -	if(pp->flags & NV_ADMA_PORT_REGISTER_MODE)
> +	if (pp->flags & NV_ADMA_PORT_REGISTER_MODE)
>  		ata_bmdma_post_internal_cmd(qc);
>  }
>  
> @@ -1194,10 +1194,10 @@ static int nv_adma_port_start(struct ata_port *ap)
>  
>  	tmp = readw(mmio + NV_ADMA_CTL);
>  	writew(tmp | NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
> -	readw( mmio + NV_ADMA_CTL );	/* flush posted write */
> +	readw(mmio + NV_ADMA_CTL );	/* flush posted write */
>   
Missed the space before closing parentheses...
>  	udelay(1);
>  	writew(tmp & ~NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
> -	readw( mmio + NV_ADMA_CTL );	/* flush posted write */
> +	readw(mmio + NV_ADMA_CTL );	/* flush posted write */
>   
and here
>  
>  	return 0;
>  }
> @@ -1255,10 +1255,10 @@ static int nv_adma_port_resume(struct ata_port *ap)
>  
>  	tmp = readw(mmio + NV_ADMA_CTL);
>  	writew(tmp | NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
> -	readw( mmio + NV_ADMA_CTL );	/* flush posted write */
> +	readw(mmio + NV_ADMA_CTL );	/* flush posted write */
>   
and here
>  	udelay(1);
>  	writew(tmp & ~NV_ADMA_CTL_CHANNEL_RESET, mmio + NV_ADMA_CTL);
> -	readw( mmio + NV_ADMA_CTL );	/* flush posted write */
> +	readw(mmio + NV_ADMA_CTL );	/* flush posted write */
>   
and here
>  
>  	return 0;
>  }
>   
The rest looks good.

cu
Richard Knutsson

Re: [PATCH 1/3] [IDE] Add helper __ide_setup_pci_device()

[Bartlomiej Zolnierkiewicz]

[ The last few days were extra busy with pushing previously queued
  IDE bits upstream but I'm back to reviewing the new stuff. ]

Thanks for splitting your patch on smaller chunks.

On Thursday 25 October 2007, Jeff Garzik wrote:
> Like ide_setup_pci_device(), except with additional array argument
> 'u8 *idx' that permits low-level driver to become aware of its assigned
> hwifs.

I would prefer to avoid exporting this information to host drivers
if possible (they shouldn't need to know about higher-layer decisions,
plus this change opens the door for various "creative" abuses).

In case of sc1200 host driver fixes (patches #2-3/3) it should be
possible to remove the need for the below patch and at the same time
simplify sc1200 code further (more details in review of patch #2/2).

> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> ---
>  drivers/ide/setup-pci.c |   17 ++++++++++++-----
>  include/linux/ide.h     |    1 +
>  2 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/ide/setup-pci.c b/drivers/ide/setup-pci.c
> index 02d14bf..4960b9f 100644
> --- a/drivers/ide/setup-pci.c
> +++ b/drivers/ide/setup-pci.c
> @@ -666,12 +666,10 @@ out:
>  	return ret;
>  }
>  
> -int ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d)
> +int __ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d,
> +			   u8 *idx)
>  {
> -	u8 idx[4] = { 0xff, 0xff, 0xff, 0xff };
> -	int ret;
> -
> -	ret = do_ide_setup_pci_device(dev, d, &idx[0], 1);
> +	int ret = do_ide_setup_pci_device(dev, d, idx, 1);
>  
>  	if (ret >= 0)
>  		ide_device_add(idx);
> @@ -679,6 +677,15 @@ int ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d)
>  	return ret;
>  }
>  
> +EXPORT_SYMBOL_GPL(__ide_setup_pci_device);
> +
> +int ide_setup_pci_device(struct pci_dev *dev, const struct ide_port_info *d)
> +{
> +	u8 idx[4] = { 0xff, 0xff, 0xff, 0xff };
> +
> +	return __ide_setup_pci_device(dev, d, &idx[0]);
> +}
> +
>  EXPORT_SYMBOL_GPL(ide_setup_pci_device);
>  
>  int ide_setup_pci_devices(struct pci_dev *dev1, struct pci_dev *dev2,
> diff --git a/include/linux/ide.h b/include/linux/ide.h
> index 4ed4777..3404fb9 100644
> --- a/include/linux/ide.h
> +++ b/include/linux/ide.h
> @@ -1244,6 +1244,7 @@ struct ide_port_info {
>  	u8			udma_mask;
>  };
>  
> +int __ide_setup_pci_device(struct pci_dev *, const struct ide_port_info *, u8 *);
>  int ide_setup_pci_device(struct pci_dev *, const struct ide_port_info *);
>  int ide_setup_pci_devices(struct pci_dev *, struct pci_dev *, const struct ide_port_info *);

Re: [PATCH 2/3] drivers/ide/pci/sc1200.c: remove pointless hwif lookup loop

[Bartlomiej Zolnierkiewicz]

On Thursday 25 October 2007, Jeff Garzik wrote:
> Store our hwif indices at probe time, in order to eliminate a needless
> and ugly loop across all hwifs, searching for our pci device.

It seems that we can simplify it even further and remove knowledge about
hwifs altogether from suspend/resume methods.

> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> ---
>  drivers/ide/pci/sc1200.c |   76 +++++++++++++++++++++++++++++++---------------
>  1 files changed, 51 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/ide/pci/sc1200.c b/drivers/ide/pci/sc1200.c
> index d2c8b55..17e58d6 100644
> --- a/drivers/ide/pci/sc1200.c
> +++ b/drivers/ide/pci/sc1200.c
> @@ -41,6 +41,8 @@
>  #define PCI_CLK_66	0x02
>  #define PCI_CLK_33A	0x03
>  
> +#define SC1200_IFS	4
> +
>  static unsigned short sc1200_get_pci_clock (void)
>  {
>  	unsigned char chip_id, silicon_revision;
> @@ -261,31 +263,32 @@ static void sc1200_set_pio_mode(ide_drive_t *drive, const u8 pio)
>  }
>  
>  #ifdef CONFIG_PM
> -static ide_hwif_t *lookup_pci_dev (ide_hwif_t *prev, struct pci_dev *dev)
> -{
> -	int	h;
> -
> -	for (h = 0; h < MAX_HWIFS; h++) {
> -		ide_hwif_t *hwif = &ide_hwifs[h];
> -		if (prev) {
> -			if (hwif == prev)
> -				prev = NULL;	// found previous, now look for next match
> -		} else {
> -			if (hwif && hwif->pci_dev == dev)
> -				return hwif;	// found next match
> -		}
> -	}
> -	return NULL;	// not found
> -}
> -
>  typedef struct sc1200_saved_state_s {
>  	__u32		regs[4];
>  } sc1200_saved_state_t;
>  
> +static unsigned int pack_hwif_idx(u8 *idx)
> +{
> +	return	(((unsigned int) idx[0]) << 0) |
> +		(((unsigned int) idx[1]) << 8) |
> +		(((unsigned int) idx[2]) << 16) |
> +		(((unsigned int) idx[3]) << 24);
> +}
> +
> +static ide_hwif_t *sc1200_hwif(struct pci_dev *pdev, unsigned int iface)
> +{
> +	unsigned int packed_hwifs, idx;
> +
> +	packed_hwifs = (unsigned long) pci_get_drvdata(pdev);
> +	idx = (packed_hwifs >> (iface * 8)) & 0xff;
> +
> +	return (idx == 0xff) ? NULL : &ide_hwifs[idx];
> +}
>  
>  static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
>  {
> -	ide_hwif_t		*hwif = NULL;
> +	ide_hwif_t		*hwif;
> +	int			i;
>  
>  	printk("SC1200: suspend(%u)\n", state.event);
>  
> @@ -295,9 +298,14 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
>  		//
>  		// Loop over all interfaces that are part of this PCI device:
>  		//
> -		while ((hwif = lookup_pci_dev(hwif, dev)) != NULL) {
> +		for (i = 0; i < SC1200_IFS; i++) {
>  			sc1200_saved_state_t	*ss;
>  			unsigned int		basereg, r;
> +
> +			hwif = sc1200_hwif(dev, i);
> +			if (!hwif)
> +				continue;
> +
>  			//
>  			// allocate a permanent save area, if not already allocated
>  			//
> @@ -310,7 +318,7 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
>  			}
>  			ss = (sc1200_saved_state_t *)hwif->config_data;
>  			//
> -			// Save timing registers:  this may be unnecessary if 
> +			// Save timing registers:  this may be unnecessary if
>  			// BIOS also does it
>  			//
>  			basereg = hwif->channel ? 0x50 : 0x40;

Please take a close look at the line above and the next three lines:

for (r = 0; r < 4; ++r) {
	pci_read_config_dword (hwif->pci_dev, basereg + (r<<2), &ss->regs[r]);
}

It is highly obfuscated but the sc1200_suspend() reads 16 bytes from
the offset 0x40 (for the primary port) and puts them in the corresponding
struct sc1200_saved_state_s buffer, then it reads another 16 bytes from the
offset 0x50 (for the secondary port) and puts it in the another buffer.

In summary sc1200_suspend() reads 32 continuous bytes from offset 0x40
and the exactly reverse operation happens in sc1200_resume().

Given that and the fact that struct sc1200_save_state_s buffers are used
_only_ by sc1200_{suspend,resume}() we may safely convert the code to use
one buffer for both ports (the whole PCI device).  We just need to bump
the size of struct sc1200_saved_state_s (from 4 to 8 double-words) and use
pci_{get,set}_drvdata() instead of hwif->config_data.  Then we can remove
looping over interfaces completely from sc1200_{suspend,resume}()! :)

> @@ -320,7 +328,7 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
>  		}
>  	}
>  
> -	/* You don't need to iterate over disks -- sysfs should have done that for you already */ 
> +	/* You don't need to iterate over disks -- sysfs should have done that for you already */
>  
>  	pci_disable_device(dev);
>  	pci_set_power_state(dev, pci_choose_state(dev, state));
> @@ -330,7 +338,8 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
>  
>  static int sc1200_resume (struct pci_dev *dev)
>  {
> -	ide_hwif_t	*hwif = NULL;
> +	ide_hwif_t		*hwif;
> +	int			i;
>  
>  	pci_set_power_state(dev, PCI_D0);	// bring chip back from sleep state
>  	dev->current_state = PM_EVENT_ON;
> @@ -338,9 +347,15 @@ static int sc1200_resume (struct pci_dev *dev)
>  	//
>  	// loop over all interfaces that are part of this pci device:
>  	//
> -	while ((hwif = lookup_pci_dev(hwif, dev)) != NULL) {
> +	for (i = 0; i < SC1200_IFS; i++) {
>  		unsigned int		basereg, r;
> -		sc1200_saved_state_t	*ss = (sc1200_saved_state_t *)hwif->config_data;
> +		sc1200_saved_state_t	*ss;
> +
> +		hwif = sc1200_hwif(dev, i);
> +		if (!hwif)
> +			continue;
> +
> +		ss = (sc1200_saved_state_t *)hwif->config_data;
>  
>  		//
>  		// Restore timing registers:  this may be unnecessary if BIOS also does it
> @@ -386,7 +401,18 @@ static const struct ide_port_info sc1200_chipset __devinitdata = {
>  
>  static int __devinit sc1200_init_one(struct pci_dev *dev, const struct pci_device_id *id)
>  {
> -	return ide_setup_pci_device(dev, &sc1200_chipset);
> +	unsigned int packed_hwifs;
> +	int rc;
> +	u8 idx[4] = { 0xff, 0xff, 0xff, 0xff };
> +
> +	rc = __ide_setup_pci_device(dev, &sc1200_chipset, &idx[0]);
> +	if (rc)
> +		return rc;
> +	
> +	packed_hwifs = pack_hwif_idx(&idx[0]);
> +
> +	pci_set_drvdata(dev, (void *)(unsigned long) packed_hwifs);
> +	return 0;
>  }
>  
>  static const struct pci_device_id sc1200_pci_tbl[] = {

Re: [PATCH 3/3] drivers/ide/pci/sc1200.c: fix suspend/resume buglets and warnings

[Bartlomiej Zolnierkiewicz]

On Thursday 25 October 2007, Jeff Garzik wrote:
> * We shouldn't bother with dev->current_state, the PCI API functions we
>   call manage this for us (and do a far better job at it too).
> 
> * Remove pci_set_power_state(dev, PCI_D0) call in resume, as
>   pci_enable_device() does the same thing.
> 
> * Check pci_enable_device() return value.  If it failed, fail
>   the entire resume and avoid programming timings into the [potentially
>   dead/asleep] chip.
> 
> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>

applied, thanks

Re: [PATCH 1/4] [SCSI] ips: remove ips_ha members that duplicate struct pci_dev members

[Jeff Garzik]

thanks for reviewing these!

Re: [PATCH 2/3] drivers/ide/pci/sc1200.c: remove pointless hwif lookup loop

[Jeff Garzik]

Bartlomiej Zolnierkiewicz wrote:
> On Thursday 25 October 2007, Jeff Garzik wrote:
>> Store our hwif indices at probe time, in order to eliminate a needless
>> and ugly loop across all hwifs, searching for our pci device.
> 
> It seems that we can simplify it even further and remove knowledge about
> hwifs altogether from suspend/resume methods.
> 
>> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
>> ---
>>  drivers/ide/pci/sc1200.c |   76 +++++++++++++++++++++++++++++++---------------
>>  1 files changed, 51 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/ide/pci/sc1200.c b/drivers/ide/pci/sc1200.c
>> index d2c8b55..17e58d6 100644
>> --- a/drivers/ide/pci/sc1200.c
>> +++ b/drivers/ide/pci/sc1200.c
>> @@ -41,6 +41,8 @@
>>  #define PCI_CLK_66	0x02
>>  #define PCI_CLK_33A	0x03
>>  
>> +#define SC1200_IFS	4
>> +
>>  static unsigned short sc1200_get_pci_clock (void)
>>  {
>>  	unsigned char chip_id, silicon_revision;
>> @@ -261,31 +263,32 @@ static void sc1200_set_pio_mode(ide_drive_t *drive, const u8 pio)
>>  }
>>  
>>  #ifdef CONFIG_PM
>> -static ide_hwif_t *lookup_pci_dev (ide_hwif_t *prev, struct pci_dev *dev)
>> -{
>> -	int	h;
>> -
>> -	for (h = 0; h < MAX_HWIFS; h++) {
>> -		ide_hwif_t *hwif = &ide_hwifs[h];
>> -		if (prev) {
>> -			if (hwif == prev)
>> -				prev = NULL;	// found previous, now look for next match
>> -		} else {
>> -			if (hwif && hwif->pci_dev == dev)
>> -				return hwif;	// found next match
>> -		}
>> -	}
>> -	return NULL;	// not found
>> -}
>> -
>>  typedef struct sc1200_saved_state_s {
>>  	__u32		regs[4];
>>  } sc1200_saved_state_t;
>>  
>> +static unsigned int pack_hwif_idx(u8 *idx)
>> +{
>> +	return	(((unsigned int) idx[0]) << 0) |
>> +		(((unsigned int) idx[1]) << 8) |
>> +		(((unsigned int) idx[2]) << 16) |
>> +		(((unsigned int) idx[3]) << 24);
>> +}
>> +
>> +static ide_hwif_t *sc1200_hwif(struct pci_dev *pdev, unsigned int iface)
>> +{
>> +	unsigned int packed_hwifs, idx;
>> +
>> +	packed_hwifs = (unsigned long) pci_get_drvdata(pdev);
>> +	idx = (packed_hwifs >> (iface * 8)) & 0xff;
>> +
>> +	return (idx == 0xff) ? NULL : &ide_hwifs[idx];
>> +}
>>  
>>  static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
>>  {
>> -	ide_hwif_t		*hwif = NULL;
>> +	ide_hwif_t		*hwif;
>> +	int			i;
>>  
>>  	printk("SC1200: suspend(%u)\n", state.event);
>>  
>> @@ -295,9 +298,14 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
>>  		//
>>  		// Loop over all interfaces that are part of this PCI device:
>>  		//
>> -		while ((hwif = lookup_pci_dev(hwif, dev)) != NULL) {
>> +		for (i = 0; i < SC1200_IFS; i++) {
>>  			sc1200_saved_state_t	*ss;
>>  			unsigned int		basereg, r;
>> +
>> +			hwif = sc1200_hwif(dev, i);
>> +			if (!hwif)
>> +				continue;
>> +
>>  			//
>>  			// allocate a permanent save area, if not already allocated
>>  			//
>> @@ -310,7 +318,7 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
>>  			}
>>  			ss = (sc1200_saved_state_t *)hwif->config_data;
>>  			//
>> -			// Save timing registers:  this may be unnecessary if 
>> +			// Save timing registers:  this may be unnecessary if
>>  			// BIOS also does it
>>  			//
>>  			basereg = hwif->channel ? 0x50 : 0x40;
> 
> Please take a close look at the line above and the next three lines:
> 
> for (r = 0; r < 4; ++r) {
> 	pci_read_config_dword (hwif->pci_dev, basereg + (r<<2), &ss->regs[r]);
> }
> 
> It is highly obfuscated but the sc1200_suspend() reads 16 bytes from
> the offset 0x40 (for the primary port) and puts them in the corresponding
> struct sc1200_saved_state_s buffer, then it reads another 16 bytes from the
> offset 0x50 (for the secondary port) and puts it in the another buffer.
> 
> In summary sc1200_suspend() reads 32 continuous bytes from offset 0x40
> and the exactly reverse operation happens in sc1200_resume().
> 
> Given that and the fact that struct sc1200_save_state_s buffers are used
> _only_ by sc1200_{suspend,resume}() we may safely convert the code to use
> one buffer for both ports (the whole PCI device).  We just need to bump
> the size of struct sc1200_saved_state_s (from 4 to 8 double-words) and use
> pci_{get,set}_drvdata() instead of hwif->config_data.  Then we can remove
> looping over interfaces completely from sc1200_{suspend,resume}()! :)

May I assume you'll handle that task?  :)  It sounds like you have a far 
better idea than mine, and my main goal -- fixing bugs and warning -- is 
accomplished anyway with the merging of patch #3.

	Jeff

Re: [PATCH 2/3] drivers/ide/pci/sc1200.c: remove pointless hwif lookup loop

[Bartlomiej Zolnierkiewicz]

On Friday 26 October 2007, Jeff Garzik wrote:
> Bartlomiej Zolnierkiewicz wrote:
> > On Thursday 25 October 2007, Jeff Garzik wrote:
> >> Store our hwif indices at probe time, in order to eliminate a needless
> >> and ugly loop across all hwifs, searching for our pci device.
> > 
> > It seems that we can simplify it even further and remove knowledge about
> > hwifs altogether from suspend/resume methods.
> > 
> >> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> >> ---
> >>  drivers/ide/pci/sc1200.c |   76 +++++++++++++++++++++++++++++++---------------
> >>  1 files changed, 51 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/drivers/ide/pci/sc1200.c b/drivers/ide/pci/sc1200.c
> >> index d2c8b55..17e58d6 100644
> >> --- a/drivers/ide/pci/sc1200.c
> >> +++ b/drivers/ide/pci/sc1200.c
> >> @@ -41,6 +41,8 @@
> >>  #define PCI_CLK_66	0x02
> >>  #define PCI_CLK_33A	0x03
> >>  
> >> +#define SC1200_IFS	4
> >> +
> >>  static unsigned short sc1200_get_pci_clock (void)
> >>  {
> >>  	unsigned char chip_id, silicon_revision;
> >> @@ -261,31 +263,32 @@ static void sc1200_set_pio_mode(ide_drive_t *drive, const u8 pio)
> >>  }
> >>  
> >>  #ifdef CONFIG_PM
> >> -static ide_hwif_t *lookup_pci_dev (ide_hwif_t *prev, struct pci_dev *dev)
> >> -{
> >> -	int	h;
> >> -
> >> -	for (h = 0; h < MAX_HWIFS; h++) {
> >> -		ide_hwif_t *hwif = &ide_hwifs[h];
> >> -		if (prev) {
> >> -			if (hwif == prev)
> >> -				prev = NULL;	// found previous, now look for next match
> >> -		} else {
> >> -			if (hwif && hwif->pci_dev == dev)
> >> -				return hwif;	// found next match
> >> -		}
> >> -	}
> >> -	return NULL;	// not found
> >> -}
> >> -
> >>  typedef struct sc1200_saved_state_s {
> >>  	__u32		regs[4];
> >>  } sc1200_saved_state_t;
> >>  
> >> +static unsigned int pack_hwif_idx(u8 *idx)
> >> +{
> >> +	return	(((unsigned int) idx[0]) << 0) |
> >> +		(((unsigned int) idx[1]) << 8) |
> >> +		(((unsigned int) idx[2]) << 16) |
> >> +		(((unsigned int) idx[3]) << 24);
> >> +}
> >> +
> >> +static ide_hwif_t *sc1200_hwif(struct pci_dev *pdev, unsigned int iface)
> >> +{
> >> +	unsigned int packed_hwifs, idx;
> >> +
> >> +	packed_hwifs = (unsigned long) pci_get_drvdata(pdev);
> >> +	idx = (packed_hwifs >> (iface * 8)) & 0xff;
> >> +
> >> +	return (idx == 0xff) ? NULL : &ide_hwifs[idx];
> >> +}
> >>  
> >>  static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
> >>  {
> >> -	ide_hwif_t		*hwif = NULL;
> >> +	ide_hwif_t		*hwif;
> >> +	int			i;
> >>  
> >>  	printk("SC1200: suspend(%u)\n", state.event);
> >>  
> >> @@ -295,9 +298,14 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
> >>  		//
> >>  		// Loop over all interfaces that are part of this PCI device:
> >>  		//
> >> -		while ((hwif = lookup_pci_dev(hwif, dev)) != NULL) {
> >> +		for (i = 0; i < SC1200_IFS; i++) {
> >>  			sc1200_saved_state_t	*ss;
> >>  			unsigned int		basereg, r;
> >> +
> >> +			hwif = sc1200_hwif(dev, i);
> >> +			if (!hwif)
> >> +				continue;
> >> +
> >>  			//
> >>  			// allocate a permanent save area, if not already allocated
> >>  			//
> >> @@ -310,7 +318,7 @@ static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
> >>  			}
> >>  			ss = (sc1200_saved_state_t *)hwif->config_data;
> >>  			//
> >> -			// Save timing registers:  this may be unnecessary if 
> >> +			// Save timing registers:  this may be unnecessary if
> >>  			// BIOS also does it
> >>  			//
> >>  			basereg = hwif->channel ? 0x50 : 0x40;
> > 
> > Please take a close look at the line above and the next three lines:
> > 
> > for (r = 0; r < 4; ++r) {
> > 	pci_read_config_dword (hwif->pci_dev, basereg + (r<<2), &ss->regs[r]);
> > }
> > 
> > It is highly obfuscated but the sc1200_suspend() reads 16 bytes from
> > the offset 0x40 (for the primary port) and puts them in the corresponding
> > struct sc1200_saved_state_s buffer, then it reads another 16 bytes from the
> > offset 0x50 (for the secondary port) and puts it in the another buffer.
> > 
> > In summary sc1200_suspend() reads 32 continuous bytes from offset 0x40
> > and the exactly reverse operation happens in sc1200_resume().
> > 
> > Given that and the fact that struct sc1200_save_state_s buffers are used
> > _only_ by sc1200_{suspend,resume}() we may safely convert the code to use
> > one buffer for both ports (the whole PCI device).  We just need to bump
> > the size of struct sc1200_saved_state_s (from 4 to 8 double-words) and use
> > pci_{get,set}_drvdata() instead of hwif->config_data.  Then we can remove
> > looping over interfaces completely from sc1200_{suspend,resume}()! :)
> 
> May I assume you'll handle that task?  :)  It sounds like you have a far 
> better idea than mine, and my main goal -- fixing bugs and warning -- is 
> accomplished anyway with the merging of patch #3.

Uh, OK...

[PATCH] sc1200: remove pointless hwif lookup loop

Save PCI regs values for both IDE ports in one buffer, in order to eliminate
a needless and ugly loop across all hwifs, searching for our PCI device.

Partially based on the previous patch by Jeff Garzik.

Cc: Jeff Garzik <jeff@garzik.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/pci/sc1200.c |  106 ++++++++++++++++-------------------------------
 1 file changed, 37 insertions(+), 69 deletions(-)

Index: b/drivers/ide/pci/sc1200.c
===================================================================
--- a/drivers/ide/pci/sc1200.c
+++ b/drivers/ide/pci/sc1200.c
@@ -261,66 +261,39 @@ static void sc1200_set_pio_mode(ide_driv
 }
 
 #ifdef CONFIG_PM
-static ide_hwif_t *lookup_pci_dev (ide_hwif_t *prev, struct pci_dev *dev)
-{
-	int	h;
-
-	for (h = 0; h < MAX_HWIFS; h++) {
-		ide_hwif_t *hwif = &ide_hwifs[h];
-		if (prev) {
-			if (hwif == prev)
-				prev = NULL;	// found previous, now look for next match
-		} else {
-			if (hwif && hwif->pci_dev == dev)
-				return hwif;	// found next match
-		}
-	}
-	return NULL;	// not found
-}
-
-typedef struct sc1200_saved_state_s {
-	__u32		regs[4];
-} sc1200_saved_state_t;
-
+struct sc1200_saved_state {
+	u32 regs[8];
+};
 
 static int sc1200_suspend (struct pci_dev *dev, pm_message_t state)
 {
-	ide_hwif_t		*hwif = NULL;
-
 	printk("SC1200: suspend(%u)\n", state.event);
 
+	/*
+	 * we only save state when going from full power to less
+	 */
 	if (state.event == PM_EVENT_ON) {
-		// we only save state when going from full power to less
+		struct sc1200_saved_state *ss;
+		unsigned int r;
 
-		//
-		// Loop over all interfaces that are part of this PCI device:
-		//
-		while ((hwif = lookup_pci_dev(hwif, dev)) != NULL) {
-			sc1200_saved_state_t	*ss;
-			unsigned int		basereg, r;
-			//
-			// allocate a permanent save area, if not already allocated
-			//
-			ss = (sc1200_saved_state_t *)hwif->config_data;
-			if (ss == NULL) {
-				ss = kmalloc(sizeof(sc1200_saved_state_t), GFP_KERNEL);
-				if (ss == NULL)
-					return -ENOMEM;
-				hwif->config_data = (unsigned long)ss;
-			}
-			ss = (sc1200_saved_state_t *)hwif->config_data;
-			//
-			// Save timing registers:  this may be unnecessary if 
-			// BIOS also does it
-			//
-			basereg = hwif->channel ? 0x50 : 0x40;
-			for (r = 0; r < 4; ++r) {
-				pci_read_config_dword (hwif->pci_dev, basereg + (r<<2), &ss->regs[r]);
-			}
+		/*
+		 * allocate a permanent save area, if not already allocated
+		 */
+		ss = (struct sc1200_saved_state *)pci_get_drvdata(dev);
+		if (ss == NULL) {
+			ss = kmalloc(sizeof(*ss), GFP_KERNEL);
+			if (ss == NULL)
+				return -ENOMEM;
+			pci_set_drvdata(dev, ss);
 		}
-	}
 
-	/* You don't need to iterate over disks -- sysfs should have done that for you already */ 
+		/*
+		 * save timing registers
+		 * (this may be unnecessary if BIOS also does it)
+		 */
+		for (r = 0; r < 8; r++)
+			pci_read_config_dword(dev, 0x40 + r * 4, &ss->regs[r]);
+	}
 
 	pci_disable_device(dev);
 	pci_set_power_state(dev, pci_choose_state(dev, state));
@@ -329,30 +302,25 @@ static int sc1200_suspend (struct pci_de
 
 static int sc1200_resume (struct pci_dev *dev)
 {
-	ide_hwif_t	*hwif = NULL;
-	int		i;
+	struct sc1200_saved_state *ss;
+	unsigned int r;
+	int i;
 
 	i = pci_enable_device(dev);
 	if (i)
 		return i;
 
-	//
-	// loop over all interfaces that are part of this pci device:
-	//
-	while ((hwif = lookup_pci_dev(hwif, dev)) != NULL) {
-		unsigned int		basereg, r;
-		sc1200_saved_state_t	*ss = (sc1200_saved_state_t *)hwif->config_data;
-
-		//
-		// Restore timing registers:  this may be unnecessary if BIOS also does it
-		//
-		basereg = hwif->channel ? 0x50 : 0x40;
-		if (ss != NULL) {
-			for (r = 0; r < 4; ++r) {
-				pci_write_config_dword(hwif->pci_dev, basereg + (r<<2), ss->regs[r]);
-			}
-		}
+	ss = (struct sc1200_saved_state *)pci_get_drvdata(dev);
+
+	/*
+	 * restore timing registers
+	 * (this may be unnecessary if BIOS also does it)
+	 */
+	if (ss) {
+		for (r = 0; r < 8; r++)
+			pci_write_config_dword(dev, 0x40 + r * 4, ss->regs[r]);
 	}
+
 	return 0;
 }
 #endif