[PATCH 2.6.12.3] PCI/libata INTx cleanup

[PATCH 2.6.12.3] PCI/libata INTx cleanup

[Brett Russ]

Simple cleanup to eliminate X copies of the same function in libata.  
Moved pci_enable_intx() to pci.c, added pci_disable_intx() as well, 
and use them throughout libata and msi.c.

Signed-off-by: Brett Russ <russb@emc.com>

Index: linux-2.6.12.3-mv/drivers/pci/msi.c
===================================================================
--- linux-2.6.12.3-mv.orig/drivers/pci/msi.c
+++ linux-2.6.12.3-mv/drivers/pci/msi.c
@@ -456,10 +456,7 @@ static void enable_msi_mode(struct pci_d
 	}
     	if (pci_find_capability(dev, PCI_CAP_ID_EXP)) {
 		/* PCI Express Endpoint device detected */
-		u16 cmd;
-		pci_read_config_word(dev, PCI_COMMAND, &cmd);
-		cmd |= PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
+		pci_disable_intx(dev);
 	}
 }
 
@@ -478,10 +475,7 @@ static void disable_msi_mode(struct pci_
 	}
     	if (pci_find_capability(dev, PCI_CAP_ID_EXP)) {
 		/* PCI Express Endpoint device detected */
-		u16 cmd;
-		pci_read_config_word(dev, PCI_COMMAND, &cmd);
-		cmd &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
+		pci_enable_intx(dev);
 	}
 }
 
Index: linux-2.6.12.3-mv/drivers/pci/pci.c
===================================================================
--- linux-2.6.12.3-mv.orig/drivers/pci/pci.c
+++ linux-2.6.12.3-mv/drivers/pci/pci.c
@@ -733,6 +733,43 @@ pci_clear_mwi(struct pci_dev *dev)
 	}
 }
 
+
+/**
+ * pci_enable_intx - enables INTx generation in PCI cfg space cmd register
+ * @dev: the PCI device to enable
+ *
+ * Enables PCI INTx generation for device
+ */
+void 
+pci_enable_intx(struct pci_dev *pdev)
+{
+        u16 pci_command;
+
+        pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
+        if (pci_command & PCI_COMMAND_INTX_DISABLE) {
+                pci_command &= ~PCI_COMMAND_INTX_DISABLE;
+                pci_write_config_word(pdev, PCI_COMMAND, pci_command);
+        }
+}
+
+/**
+ * pci_disable_intx - disables INTx generation in PCI cfg space cmd register
+ * @dev: the PCI device to disable
+ *
+ * Disables PCI INTx generation for device
+ */
+void 
+pci_disable_intx(struct pci_dev *pdev)
+{
+        u16 pci_command;
+
+        pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
+        if (!(pci_command & PCI_COMMAND_INTX_DISABLE)) {
+                pci_command |= PCI_COMMAND_INTX_DISABLE;
+                pci_write_config_word(pdev, PCI_COMMAND, pci_command);
+        }
+}
+
 #ifndef HAVE_ARCH_PCI_SET_DMA_MASK
 /*
  * These can be overridden by arch-specific implementations
@@ -809,6 +846,8 @@ EXPORT_SYMBOL(pci_request_region);
 EXPORT_SYMBOL(pci_set_master);
 EXPORT_SYMBOL(pci_set_mwi);
 EXPORT_SYMBOL(pci_clear_mwi);
+EXPORT_SYMBOL(pci_enable_intx);
+EXPORT_SYMBOL(pci_disable_intx);
 EXPORT_SYMBOL(pci_set_dma_mask);
 EXPORT_SYMBOL(pci_set_consistent_dma_mask);
 EXPORT_SYMBOL(pci_assign_resource);
Index: linux-2.6.12.3-mv/drivers/scsi/sata_sis.c
===================================================================
--- linux-2.6.12.3-mv.orig/drivers/scsi/sata_sis.c
+++ linux-2.6.12.3-mv/drivers/scsi/sata_sis.c
@@ -186,18 +186,6 @@ static void sis_scr_write (struct ata_po
 		outl(val, ap->ioaddr.scr_addr + (sc_reg * 4));
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_enable_intx(struct pci_dev *pdev)
-{
-	u16 pci_command;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-	if (pci_command & PCI_COMMAND_INTX_DISABLE) {
-		pci_command &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-	}
-}
-
 static int sis_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct ata_probe_ent *probe_ent = NULL;
Index: linux-2.6.12.3-mv/drivers/scsi/ahci.c
===================================================================
--- linux-2.6.12.3-mv.orig/drivers/scsi/ahci.c
+++ linux-2.6.12.3-mv/drivers/scsi/ahci.c
@@ -878,18 +878,6 @@ static int ahci_host_init(struct ata_pro
 	return 0;
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_enable_intx(struct pci_dev *pdev)
-{
-	u16 pci_command;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-	if (pci_command & PCI_COMMAND_INTX_DISABLE) {
-		pci_command &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-	}
-}
-
 static void ahci_print_info(struct ata_probe_ent *probe_ent)
 {
 	struct ahci_host_priv *hpriv = probe_ent->private_data;
Index: linux-2.6.12.3-mv/drivers/scsi/ata_piix.c
===================================================================
--- linux-2.6.12.3-mv.orig/drivers/scsi/ata_piix.c
+++ linux-2.6.12.3-mv/drivers/scsi/ata_piix.c
@@ -545,18 +545,6 @@ static void piix_set_dmamode (struct ata
 	}
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_enable_intx(struct pci_dev *pdev)
-{
-	u16 pci_command;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-	if (pci_command & PCI_COMMAND_INTX_DISABLE) {
-		pci_command &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-	}
-}
-
 #define AHCI_PCI_BAR 5
 #define AHCI_GLOBAL_CTL 0x04
 #define AHCI_ENABLE (1 << 31)
Index: linux-2.6.12.3-mv/drivers/scsi/sata_uli.c
===================================================================
--- linux-2.6.12.3-mv.orig/drivers/scsi/sata_uli.c
+++ linux-2.6.12.3-mv/drivers/scsi/sata_uli.c
@@ -171,18 +171,6 @@ static void uli_scr_write (struct ata_po
 	uli_scr_cfg_write(ap, sc_reg, val);
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_enable_intx(struct pci_dev *pdev)
-{
-	u16 pci_command;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-	if (pci_command & PCI_COMMAND_INTX_DISABLE) {
-		pci_command &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-	}
-}
-
 static int uli_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct ata_probe_ent *probe_ent;
Index: linux-2.6.12.3-mv/include/linux/pci.h
===================================================================
--- linux-2.6.12.3-mv.orig/include/linux/pci.h
+++ linux-2.6.12.3-mv/include/linux/pci.h
@@ -810,6 +810,8 @@ void pci_set_master(struct pci_dev *dev)
 #define HAVE_PCI_SET_MWI
 int pci_set_mwi(struct pci_dev *dev);
 void pci_clear_mwi(struct pci_dev *dev);
+void pci_enable_intx(struct pci_dev *pdev);
+void pci_disable_intx(struct pci_dev *pdev);
 int pci_set_dma_mask(struct pci_dev *dev, u64 mask);
 int pci_set_consistent_dma_mask(struct pci_dev *dev, u64 mask);
 int pci_assign_resource(struct pci_dev *dev, int i);

Re: [PATCH 2.6.12.3] PCI/libata INTx cleanup

[Jeff Garzik]

Brett Russ wrote:
> Simple cleanup to eliminate X copies of the same function in libata.  
> Moved pci_enable_intx() to pci.c, added pci_disable_intx() as well, 
> and use them throughout libata and msi.c.
> 
> Signed-off-by: Brett Russ <russb@emc.com>

Though there is nothing wrong with this patch, I would prefer a single 
function, pci_intx(), as found in drivers/scsi/ahci.c.

Would you be willing to move that one to the PCI layer, eliminate the 
multiple copies of pci_enable_intx(), and replace the calls to 
pci_enable_intx() with calls to pci_intx() ?

	Jeff

[PATCH 2.6.12.3] PCI/libata INTx cleanup

[Brett Russ]

Jeff Garzik wrote:
> Brett Russ wrote:
> 
>> Simple cleanup to eliminate X copies of the same function in libata.  
>> Moved pci_enable_intx() to pci.c, added pci_disable_intx() as well, 
>> and use them throughout libata and msi.c.
>>
>> Signed-off-by: Brett Russ <russb@emc.com>
> 
> 
> Though there is nothing wrong with this patch, I would prefer a single 
> function, pci_intx(), as found in drivers/scsi/ahci.c.
> 
> Would you be willing to move that one to the PCI layer, eliminate the 
> multiple copies of pci_enable_intx(), and replace the calls to 
> pci_enable_intx() with calls to pci_intx() ?

Sounds like what I did, except for the naming change.  I did away with
pci_disable_intx() and changed the names.  Look ok?

BR


Index: linux-2.6.12.3-mv/drivers/pci/msi.c
===================================================================
--- linux-2.6.12.3-mv.orig/drivers/pci/msi.c
+++ linux-2.6.12.3-mv/drivers/pci/msi.c
@@ -478,10 +478,7 @@ static void disable_msi_mode(struct pci_
 	}
     	if (pci_find_capability(dev, PCI_CAP_ID_EXP)) {
 		/* PCI Express Endpoint device detected */
-		u16 cmd;
-		pci_read_config_word(dev, PCI_COMMAND, &cmd);
-		cmd &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
+		pci_intx(dev);
 	}
 }
 
Index: linux-2.6.12.3-mv/drivers/pci/pci.c
===================================================================
--- linux-2.6.12.3-mv.orig/drivers/pci/pci.c
+++ linux-2.6.12.3-mv/drivers/pci/pci.c
@@ -733,6 +733,25 @@ pci_clear_mwi(struct pci_dev *dev)
 	}
 }
 
+
+/**
+ * pci_intx - enables INTx generation in PCI cfg space cmd register
+ * @dev: the PCI device to enable
+ *
+ * Enables PCI INTx generation for device
+ */
+void 
+pci_intx(struct pci_dev *pdev)
+{
+        u16 pci_command;
+
+        pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
+        if (pci_command & PCI_COMMAND_INTX_DISABLE) {
+                pci_command &= ~PCI_COMMAND_INTX_DISABLE;
+                pci_write_config_word(pdev, PCI_COMMAND, pci_command);
+        }
+}
+
 #ifndef HAVE_ARCH_PCI_SET_DMA_MASK
 /*
  * These can be overridden by arch-specific implementations
@@ -809,6 +828,7 @@ EXPORT_SYMBOL(pci_request_region);
 EXPORT_SYMBOL(pci_set_master);
 EXPORT_SYMBOL(pci_set_mwi);
 EXPORT_SYMBOL(pci_clear_mwi);
+EXPORT_SYMBOL(pci_intx);
 EXPORT_SYMBOL(pci_set_dma_mask);
 EXPORT_SYMBOL(pci_set_consistent_dma_mask);
 EXPORT_SYMBOL(pci_assign_resource);
Index: linux-2.6.12.3-mv/drivers/scsi/sata_sis.c
===================================================================
--- linux-2.6.12.3-mv.orig/drivers/scsi/sata_sis.c
+++ linux-2.6.12.3-mv/drivers/scsi/sata_sis.c
@@ -186,18 +186,6 @@ static void sis_scr_write (struct ata_po
 		outl(val, ap->ioaddr.scr_addr + (sc_reg * 4));
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_enable_intx(struct pci_dev *pdev)
-{
-	u16 pci_command;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-	if (pci_command & PCI_COMMAND_INTX_DISABLE) {
-		pci_command &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-	}
-}
-
 static int sis_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct ata_probe_ent *probe_ent = NULL;
@@ -254,7 +242,7 @@ static int sis_init_one (struct pci_dev 
 	}
 
 	pci_set_master(pdev);
-	pci_enable_intx(pdev);
+	pci_intx(pdev);
 
 	/* FIXME: check ata_device_add return value */
 	ata_device_add(probe_ent);
Index: linux-2.6.12.3-mv/drivers/scsi/ahci.c
===================================================================
--- linux-2.6.12.3-mv.orig/drivers/scsi/ahci.c
+++ linux-2.6.12.3-mv/drivers/scsi/ahci.c
@@ -878,18 +878,6 @@ static int ahci_host_init(struct ata_pro
 	return 0;
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_enable_intx(struct pci_dev *pdev)
-{
-	u16 pci_command;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-	if (pci_command & PCI_COMMAND_INTX_DISABLE) {
-		pci_command &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-	}
-}
-
 static void ahci_print_info(struct ata_probe_ent *probe_ent)
 {
 	struct ahci_host_priv *hpriv = probe_ent->private_data;
@@ -987,7 +975,7 @@ static int ahci_init_one (struct pci_dev
 		goto err_out;
 	}
 
-	pci_enable_intx(pdev);
+	pci_intx(pdev);
 
 	probe_ent = kmalloc(sizeof(*probe_ent), GFP_KERNEL);
 	if (probe_ent == NULL) {
Index: linux-2.6.12.3-mv/drivers/scsi/ata_piix.c
===================================================================
--- linux-2.6.12.3-mv.orig/drivers/scsi/ata_piix.c
+++ linux-2.6.12.3-mv/drivers/scsi/ata_piix.c
@@ -545,18 +545,6 @@ static void piix_set_dmamode (struct ata
 	}
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_enable_intx(struct pci_dev *pdev)
-{
-	u16 pci_command;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-	if (pci_command & PCI_COMMAND_INTX_DISABLE) {
-		pci_command &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-	}
-}
-
 #define AHCI_PCI_BAR 5
 #define AHCI_GLOBAL_CTL 0x04
 #define AHCI_ENABLE (1 << 31)
@@ -651,7 +639,7 @@ static int piix_init_one (struct pci_dev
 	 * message-signalled interrupts currently).
 	 */
 	if (port_info[0]->host_flags & PIIX_FLAG_CHECKINTR)
-		pci_enable_intx(pdev);
+		pci_intx(pdev);
 
 	if (combined) {
 		port_info[sata_chan] = &piix_port_info[ent->driver_data];
Index: linux-2.6.12.3-mv/drivers/scsi/sata_uli.c
===================================================================
--- linux-2.6.12.3-mv.orig/drivers/scsi/sata_uli.c
+++ linux-2.6.12.3-mv/drivers/scsi/sata_uli.c
@@ -171,18 +171,6 @@ static void uli_scr_write (struct ata_po
 	uli_scr_cfg_write(ap, sc_reg, val);
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_enable_intx(struct pci_dev *pdev)
-{
-	u16 pci_command;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-	if (pci_command & PCI_COMMAND_INTX_DISABLE) {
-		pci_command &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-	}
-}
-
 static int uli_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct ata_probe_ent *probe_ent;
@@ -255,7 +243,7 @@ static int uli_init_one (struct pci_dev 
 	}
 
 	pci_set_master(pdev);
-	pci_enable_intx(pdev);
+	pci_intx(pdev);
 
 	/* FIXME: check ata_device_add return value */
 	ata_device_add(probe_ent);
Index: linux-2.6.12.3-mv/include/linux/pci.h
===================================================================
--- linux-2.6.12.3-mv.orig/include/linux/pci.h
+++ linux-2.6.12.3-mv/include/linux/pci.h
@@ -810,6 +810,7 @@ void pci_set_master(struct pci_dev *dev)
 #define HAVE_PCI_SET_MWI
 int pci_set_mwi(struct pci_dev *dev);
 void pci_clear_mwi(struct pci_dev *dev);
+void pci_intx(struct pci_dev *pdev);
 int pci_set_dma_mask(struct pci_dev *dev, u64 mask);
 int pci_set_consistent_dma_mask(struct pci_dev *dev, u64 mask);
 int pci_assign_resource(struct pci_dev *dev, int i);

Re: [PATCH 2.6.12.3] PCI/libata INTx cleanup

[Greg KH]

On Fri, Aug 12, 2005 at 01:10:43PM -0400, Brett Russ wrote:
> Jeff Garzik wrote:
> > Brett Russ wrote:
> > 
> >> Simple cleanup to eliminate X copies of the same function in libata.  
> >> Moved pci_enable_intx() to pci.c, added pci_disable_intx() as well, 
> >> and use them throughout libata and msi.c.
> >>
> >> Signed-off-by: Brett Russ <russb@emc.com>
> > 
> > 
> > Though there is nothing wrong with this patch, I would prefer a single 
> > function, pci_intx(), as found in drivers/scsi/ahci.c.
> > 
> > Would you be willing to move that one to the PCI layer, eliminate the 
> > multiple copies of pci_enable_intx(), and replace the calls to 
> > pci_enable_intx() with calls to pci_intx() ?
> 
> Sounds like what I did, except for the naming change.  I did away with
> pci_disable_intx() and changed the names.  Look ok?

Looks ok to me, care to resend it with a Signed-off-by: and a new
changelog entry so I can apply it?

> +EXPORT_SYMBOL(pci_intx);

Hm, for new pci functions, I prefer to have them marked as
EXPORT_SYMBOL_GPL(), is that ok with you?

thanks,

greg k-h

[PATCH 2.6.12.3] PCI/libata INTx cleanup

[Brett Russ]

Simple cleanup to eliminate X copies of the pci_enable_intx() function
in libata.  Moved pci_enable_intx() to pci.c as pci_intx() and use it
throughout libata and msi.c.

Signed-off-by: Brett Russ <russb@emc.com>


Index: linux-2.6.12.3-mv/drivers/pci/msi.c
===================================================================
--- linux-2.6.12.3-mv.orig/drivers/pci/msi.c
+++ linux-2.6.12.3-mv/drivers/pci/msi.c
@@ -478,10 +478,7 @@ static void disable_msi_mode(struct pci_
 	}
     	if (pci_find_capability(dev, PCI_CAP_ID_EXP)) {
 		/* PCI Express Endpoint device detected */
-		u16 cmd;
-		pci_read_config_word(dev, PCI_COMMAND, &cmd);
-		cmd &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
+		pci_intx(dev);
 	}
 }
 
Index: linux-2.6.12.3-mv/drivers/pci/pci.c
===================================================================
--- linux-2.6.12.3-mv.orig/drivers/pci/pci.c
+++ linux-2.6.12.3-mv/drivers/pci/pci.c
@@ -733,6 +733,25 @@ pci_clear_mwi(struct pci_dev *dev)
 	}
 }
 
+
+/**
+ * pci_intx - enables INTx generation in PCI cfg space cmd register
+ * @dev: the PCI device to enable
+ *
+ * Enables PCI INTx generation for device
+ */
+void 
+pci_intx(struct pci_dev *pdev)
+{
+        u16 pci_command;
+
+        pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
+        if (pci_command & PCI_COMMAND_INTX_DISABLE) {
+                pci_command &= ~PCI_COMMAND_INTX_DISABLE;
+                pci_write_config_word(pdev, PCI_COMMAND, pci_command);
+        }
+}
+
 #ifndef HAVE_ARCH_PCI_SET_DMA_MASK
 /*
  * These can be overridden by arch-specific implementations
@@ -809,6 +828,7 @@ EXPORT_SYMBOL(pci_request_region);
 EXPORT_SYMBOL(pci_set_master);
 EXPORT_SYMBOL(pci_set_mwi);
 EXPORT_SYMBOL(pci_clear_mwi);
+EXPORT_SYMBOL_GPL(pci_intx);
 EXPORT_SYMBOL(pci_set_dma_mask);
 EXPORT_SYMBOL(pci_set_consistent_dma_mask);
 EXPORT_SYMBOL(pci_assign_resource);
Index: linux-2.6.12.3-mv/drivers/scsi/sata_sis.c
===================================================================
--- linux-2.6.12.3-mv.orig/drivers/scsi/sata_sis.c
+++ linux-2.6.12.3-mv/drivers/scsi/sata_sis.c
@@ -186,18 +186,6 @@ static void sis_scr_write (struct ata_po
 		outl(val, ap->ioaddr.scr_addr + (sc_reg * 4));
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_enable_intx(struct pci_dev *pdev)
-{
-	u16 pci_command;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-	if (pci_command & PCI_COMMAND_INTX_DISABLE) {
-		pci_command &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-	}
-}
-
 static int sis_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct ata_probe_ent *probe_ent = NULL;
@@ -254,7 +242,7 @@ static int sis_init_one (struct pci_dev 
 	}
 
 	pci_set_master(pdev);
-	pci_enable_intx(pdev);
+	pci_intx(pdev);
 
 	/* FIXME: check ata_device_add return value */
 	ata_device_add(probe_ent);
Index: linux-2.6.12.3-mv/drivers/scsi/ahci.c
===================================================================
--- linux-2.6.12.3-mv.orig/drivers/scsi/ahci.c
+++ linux-2.6.12.3-mv/drivers/scsi/ahci.c
@@ -878,18 +878,6 @@ static int ahci_host_init(struct ata_pro
 	return 0;
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_enable_intx(struct pci_dev *pdev)
-{
-	u16 pci_command;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-	if (pci_command & PCI_COMMAND_INTX_DISABLE) {
-		pci_command &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-	}
-}
-
 static void ahci_print_info(struct ata_probe_ent *probe_ent)
 {
 	struct ahci_host_priv *hpriv = probe_ent->private_data;
@@ -987,7 +975,7 @@ static int ahci_init_one (struct pci_dev
 		goto err_out;
 	}
 
-	pci_enable_intx(pdev);
+	pci_intx(pdev);
 
 	probe_ent = kmalloc(sizeof(*probe_ent), GFP_KERNEL);
 	if (probe_ent == NULL) {
Index: linux-2.6.12.3-mv/drivers/scsi/ata_piix.c
===================================================================
--- linux-2.6.12.3-mv.orig/drivers/scsi/ata_piix.c
+++ linux-2.6.12.3-mv/drivers/scsi/ata_piix.c
@@ -545,18 +545,6 @@ static void piix_set_dmamode (struct ata
 	}
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_enable_intx(struct pci_dev *pdev)
-{
-	u16 pci_command;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-	if (pci_command & PCI_COMMAND_INTX_DISABLE) {
-		pci_command &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-	}
-}
-
 #define AHCI_PCI_BAR 5
 #define AHCI_GLOBAL_CTL 0x04
 #define AHCI_ENABLE (1 << 31)
@@ -651,7 +639,7 @@ static int piix_init_one (struct pci_dev
 	 * message-signalled interrupts currently).
 	 */
 	if (port_info[0]->host_flags & PIIX_FLAG_CHECKINTR)
-		pci_enable_intx(pdev);
+		pci_intx(pdev);
 
 	if (combined) {
 		port_info[sata_chan] = &piix_port_info[ent->driver_data];
Index: linux-2.6.12.3-mv/drivers/scsi/sata_uli.c
===================================================================
--- linux-2.6.12.3-mv.orig/drivers/scsi/sata_uli.c
+++ linux-2.6.12.3-mv/drivers/scsi/sata_uli.c
@@ -171,18 +171,6 @@ static void uli_scr_write (struct ata_po
 	uli_scr_cfg_write(ap, sc_reg, val);
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_enable_intx(struct pci_dev *pdev)
-{
-	u16 pci_command;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-	if (pci_command & PCI_COMMAND_INTX_DISABLE) {
-		pci_command &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-	}
-}
-
 static int uli_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct ata_probe_ent *probe_ent;
@@ -255,7 +243,7 @@ static int uli_init_one (struct pci_dev 
 	}
 
 	pci_set_master(pdev);
-	pci_enable_intx(pdev);
+	pci_intx(pdev);
 
 	/* FIXME: check ata_device_add return value */
 	ata_device_add(probe_ent);
Index: linux-2.6.12.3-mv/include/linux/pci.h
===================================================================
--- linux-2.6.12.3-mv.orig/include/linux/pci.h
+++ linux-2.6.12.3-mv/include/linux/pci.h
@@ -810,6 +810,7 @@ void pci_set_master(struct pci_dev *dev)
 #define HAVE_PCI_SET_MWI
 int pci_set_mwi(struct pci_dev *dev);
 void pci_clear_mwi(struct pci_dev *dev);
+void pci_intx(struct pci_dev *pdev);
 int pci_set_dma_mask(struct pci_dev *dev, u64 mask);
 int pci_set_consistent_dma_mask(struct pci_dev *dev, u64 mask);
 int pci_assign_resource(struct pci_dev *dev, int i);

Re: [PATCH 2.6.12.3] PCI/libata INTx cleanup

[Jeff Garzik]

Greg KH wrote:
> On Fri, Aug 12, 2005 at 01:10:43PM -0400, Brett Russ wrote:
> 
>>Jeff Garzik wrote:
>>
>>>Brett Russ wrote:
>>>
>>>
>>>>Simple cleanup to eliminate X copies of the same function in libata.  
>>>>Moved pci_enable_intx() to pci.c, added pci_disable_intx() as well, 
>>>>and use them throughout libata and msi.c.
>>>>
>>>>Signed-off-by: Brett Russ <russb@emc.com>
>>>
>>>
>>>Though there is nothing wrong with this patch, I would prefer a single 
>>>function, pci_intx(), as found in drivers/scsi/ahci.c.
>>>
>>>Would you be willing to move that one to the PCI layer, eliminate the 
>>>multiple copies of pci_enable_intx(), and replace the calls to 
>>>pci_enable_intx() with calls to pci_intx() ?
>>
>>Sounds like what I did, except for the naming change.  I did away with
>>pci_disable_intx() and changed the names.  Look ok?
> 
> 
> Looks ok to me, care to resend it with a Signed-off-by: and a new
> changelog entry so I can apply it?
> 
> 
>>+EXPORT_SYMBOL(pci_intx);
> 
> 
> Hm, for new pci functions, I prefer to have them marked as
> EXPORT_SYMBOL_GPL(), is that ok with you?

Don't apply as-is, it needs to look like the current 2.6.13-rcX two-arg 
version from drivers/scsi/ahci.c:

/* move to PCI layer, integrate w/ MSI stuff */
static void pci_intx(struct pci_dev *pdev, int enable)
{
         u16 pci_command, new;

         pci_read_config_word(pdev, PCI_COMMAND, &pci_command);

         if (enable)
                 new = pci_command & ~PCI_COMMAND_INTX_DISABLE;
         else
                 new = pci_command | PCI_COMMAND_INTX_DISABLE;

         if (new != pci_command)
                 pci_write_config_word(pdev, PCI_COMMAND, pci_command);
}

[PATCH 2.6.13-rc6] PCI/libata INTx cleanup

[Brett Russ]

Simple cleanup to eliminate X copies of the pci_enable_intx() function
in libata.  Moved ahci.c's pci_intx() to pci.c and use it throughout
libata and msi.c.

Signed-off-by: Brett Russ <russb@emc.com>


Index: linux-2.6.13-rc6/drivers/scsi/sata_sis.c
===================================================================
--- linux-2.6.13-rc6.orig/drivers/scsi/sata_sis.c
+++ linux-2.6.13-rc6/drivers/scsi/sata_sis.c
@@ -186,18 +186,6 @@ static void sis_scr_write (struct ata_po
 		outl(val, ap->ioaddr.scr_addr + (sc_reg * 4));
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_enable_intx(struct pci_dev *pdev)
-{
-	u16 pci_command;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-	if (pci_command & PCI_COMMAND_INTX_DISABLE) {
-		pci_command &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-	}
-}
-
 static int sis_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct ata_probe_ent *probe_ent = NULL;
@@ -254,7 +242,7 @@ static int sis_init_one (struct pci_dev 
 	}
 
 	pci_set_master(pdev);
-	pci_enable_intx(pdev);
+	pci_intx(pdev, TRUE);
 
 	/* FIXME: check ata_device_add return value */
 	ata_device_add(probe_ent);
Index: linux-2.6.13-rc6/drivers/scsi/ahci.c
===================================================================
--- linux-2.6.13-rc6.orig/drivers/scsi/ahci.c
+++ linux-2.6.13-rc6/drivers/scsi/ahci.c
@@ -862,22 +862,6 @@ static int ahci_host_init(struct ata_pro
 	return 0;
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_intx(struct pci_dev *pdev, int enable)
-{
-	u16 pci_command, new;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-
-	if (enable)
-		new = pci_command & ~PCI_COMMAND_INTX_DISABLE;
-	else
-		new = pci_command | PCI_COMMAND_INTX_DISABLE;
-
-	if (new != pci_command)
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-}
-
 static void ahci_print_info(struct ata_probe_ent *probe_ent)
 {
 	struct ahci_host_priv *hpriv = probe_ent->private_data;
@@ -978,7 +962,7 @@ static int ahci_init_one (struct pci_dev
 	if (pci_enable_msi(pdev) == 0)
 		have_msi = 1;
 	else {
-		pci_intx(pdev, 1);
+		pci_intx(pdev, TRUE);
 		have_msi = 0;
 	}
 
@@ -1044,7 +1028,7 @@ err_out_msi:
 	if (have_msi)
 		pci_disable_msi(pdev);
 	else
-		pci_intx(pdev, 0);
+		pci_intx(pdev, FALSE);
 	pci_release_regions(pdev);
 err_out:
 	if (!pci_dev_busy)
@@ -1083,7 +1067,7 @@ static void ahci_remove_one (struct pci_
 	if (have_msi)
 		pci_disable_msi(pdev);
 	else
-		pci_intx(pdev, 0);
+		pci_intx(pdev, FALSE);
 	pci_release_regions(pdev);
 	pci_disable_device(pdev);
 	dev_set_drvdata(dev, NULL);
Index: linux-2.6.13-rc6/drivers/scsi/ata_piix.c
===================================================================
--- linux-2.6.13-rc6.orig/drivers/scsi/ata_piix.c
+++ linux-2.6.13-rc6/drivers/scsi/ata_piix.c
@@ -548,18 +548,6 @@ static void piix_set_dmamode (struct ata
 	}
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_enable_intx(struct pci_dev *pdev)
-{
-	u16 pci_command;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-	if (pci_command & PCI_COMMAND_INTX_DISABLE) {
-		pci_command &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-	}
-}
-
 #define AHCI_PCI_BAR 5
 #define AHCI_GLOBAL_CTL 0x04
 #define AHCI_ENABLE (1 << 31)
@@ -658,7 +646,7 @@ static int piix_init_one (struct pci_dev
 	 * message-signalled interrupts currently).
 	 */
 	if (port_info[0]->host_flags & PIIX_FLAG_CHECKINTR)
-		pci_enable_intx(pdev);
+		pci_intx(pdev, TRUE);
 
 	if (combined) {
 		port_info[sata_chan] = &piix_port_info[ent->driver_data];
Index: linux-2.6.13-rc6/drivers/pci/msi.c
===================================================================
--- linux-2.6.13-rc6.orig/drivers/pci/msi.c
+++ linux-2.6.13-rc6/drivers/pci/msi.c
@@ -446,10 +446,7 @@ static void enable_msi_mode(struct pci_d
 	}
     	if (pci_find_capability(dev, PCI_CAP_ID_EXP)) {
 		/* PCI Express Endpoint device detected */
-		u16 cmd;
-		pci_read_config_word(dev, PCI_COMMAND, &cmd);
-		cmd |= PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
+		pci_intx(dev, FALSE);  /* disable intx */
 	}
 }
 
@@ -468,10 +465,7 @@ static void disable_msi_mode(struct pci_
 	}
     	if (pci_find_capability(dev, PCI_CAP_ID_EXP)) {
 		/* PCI Express Endpoint device detected */
-		u16 cmd;
-		pci_read_config_word(dev, PCI_COMMAND, &cmd);
-		cmd &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
+		pci_intx(dev, TRUE);  /* enable intx */
 	}
 }
 
Index: linux-2.6.13-rc6/drivers/pci/pci.c
===================================================================
--- linux-2.6.13-rc6.orig/drivers/pci/pci.c
+++ linux-2.6.13-rc6/drivers/pci/pci.c
@@ -793,6 +793,31 @@ pci_clear_mwi(struct pci_dev *dev)
 	}
 }
 
+/**
+ * pci_intx - enables/disables PCI INTx for device dev
+ * @dev: the PCI device to operate on
+ * @enable: boolean
+ *
+ * Enables/disables PCI INTx for device dev
+ */
+void 
+pci_intx(struct pci_dev *pdev, int enable)
+{
+	u16 pci_command, new;
+
+	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
+
+	if (enable) {
+		new = pci_command & ~PCI_COMMAND_INTX_DISABLE;
+	} else {
+		new = pci_command | PCI_COMMAND_INTX_DISABLE;
+	}
+
+	if (new != pci_command) {
+		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
+	}
+}
+
 #ifndef HAVE_ARCH_PCI_SET_DMA_MASK
 /*
  * These can be overridden by arch-specific implementations
@@ -870,6 +895,7 @@ EXPORT_SYMBOL(pci_request_region);
 EXPORT_SYMBOL(pci_set_master);
 EXPORT_SYMBOL(pci_set_mwi);
 EXPORT_SYMBOL(pci_clear_mwi);
+EXPORT_SYMBOL_GPL(pci_intx);
 EXPORT_SYMBOL(pci_set_dma_mask);
 EXPORT_SYMBOL(pci_set_consistent_dma_mask);
 EXPORT_SYMBOL(pci_assign_resource);
Index: linux-2.6.13-rc6/include/linux/pci.h
===================================================================
--- linux-2.6.13-rc6.orig/include/linux/pci.h
+++ linux-2.6.13-rc6/include/linux/pci.h
@@ -815,6 +815,7 @@ void pci_set_master(struct pci_dev *dev)
 #define HAVE_PCI_SET_MWI
 int pci_set_mwi(struct pci_dev *dev);
 void pci_clear_mwi(struct pci_dev *dev);
+void pci_intx(struct pci_dev *dev, int enable);
 int pci_set_dma_mask(struct pci_dev *dev, u64 mask);
 int pci_set_consistent_dma_mask(struct pci_dev *dev, u64 mask);
 void pci_update_resource(struct pci_dev *dev, struct resource *res, int resno);
Index: linux-2.6.13-rc6/drivers/scsi/sata_uli.c
===================================================================
--- linux-2.6.13-rc6.orig/drivers/scsi/sata_uli.c
+++ linux-2.6.13-rc6/drivers/scsi/sata_uli.c
@@ -171,18 +171,6 @@ static void uli_scr_write (struct ata_po
 	uli_scr_cfg_write(ap, sc_reg, val);
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_enable_intx(struct pci_dev *pdev)
-{
-	u16 pci_command;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-	if (pci_command & PCI_COMMAND_INTX_DISABLE) {
-		pci_command &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-	}
-}
-
 static int uli_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct ata_probe_ent *probe_ent;
@@ -255,7 +243,7 @@ static int uli_init_one (struct pci_dev 
 	}
 
 	pci_set_master(pdev);
-	pci_enable_intx(pdev);
+	pci_intx(pdev, TRUE);
 
 	/* FIXME: check ata_device_add return value */
 	ata_device_add(probe_ent);

Re: [PATCH 2.6.13-rc6] PCI/libata INTx cleanup

[Jeff Garzik]

Brett Russ wrote:
> Simple cleanup to eliminate X copies of the pci_enable_intx() function
> in libata.  Moved ahci.c's pci_intx() to pci.c and use it throughout
> libata and msi.c.
> 
> Signed-off-by: Brett Russ <russb@emc.com>

Looks good to me.

Greg, do you want to queue this (since it touches PCI), or should I 
(since it touches SATA drivers)?

	Jeff

Re: [PATCH 2.6.13-rc6] PCI/libata INTx cleanup

[Greg KH]

On Fri, Aug 12, 2005 at 08:29:19PM -0400, Jeff Garzik wrote:
> Brett Russ wrote:
> >Simple cleanup to eliminate X copies of the pci_enable_intx() function
> >in libata.  Moved ahci.c's pci_intx() to pci.c and use it throughout
> >libata and msi.c.
> >
> >Signed-off-by: Brett Russ <russb@emc.com>
> 
> Looks good to me.
> 
> Greg, do you want to queue this (since it touches PCI), or should I 
> (since it touches SATA drivers)?

I'll take it and add it to my trees and send it off to Linus after
2.6.13 comes out.

thanks,

greg k-h

Re: [PATCH 2.6.13-rc6] PCI/libata INTx cleanup

[Greg KH]

On Fri, Aug 12, 2005 at 06:43:03PM -0400, Brett Russ wrote:
> Simple cleanup to eliminate X copies of the pci_enable_intx() function
> in libata.  Moved ahci.c's pci_intx() to pci.c and use it throughout
> libata and msi.c.
> 
> Signed-off-by: Brett Russ <russb@emc.com>

It would have been nice if you had built this code :(

Hint, get rid of all TRUE and FALSE usages in your patch.  Care to try
again?

thanks,

greg k-h

Re: [PATCH 2.6.13-rc6] PCI/libata INTx cleanup

[Brett Russ]

Greg KH wrote:
> On Fri, Aug 12, 2005 at 06:43:03PM -0400, Brett Russ wrote:
> 
>>Simple cleanup to eliminate X copies of the pci_enable_intx() function
>>in libata.  Moved ahci.c's pci_intx() to pci.c and use it throughout
>>libata and msi.c.
>>
>>Signed-off-by: Brett Russ <russb@emc.com>
> 
> 
> It would have been nice if you had built this code :(
> 
> Hint, get rid of all TRUE and FALSE usages in your patch.  Care to try
> again?
> 
> thanks,
> 
> greg k-h


Hmm, I did build it before submitting, saw not even a warning.  So I 
just rebuilt it again to verify, and same thing.  Why would my tree work 
and not yours?  config file?

russb@lns1058:/net/maggie/exports/russb/lk/linux-2.6.13-rc6> q pop
Removing patch patches/pcix
Restoring drivers/pci/msi.c
Restoring drivers/pci/pci.c
Restoring drivers/scsi/ata_piix.c
Restoring drivers/scsi/sata_sis.c
Restoring drivers/scsi/sata_uli.c
Restoring drivers/scsi/ahci.c
Restoring include/linux/pci.h

No patches applied
russb@lns1058:/net/maggie/exports/russb/lk/linux-2.6.13-rc6> q push
Applying patch patches/pcix
patching file drivers/scsi/sata_sis.c
patching file drivers/scsi/ahci.c
patching file drivers/scsi/ata_piix.c
patching file drivers/pci/msi.c
patching file drivers/pci/pci.c
patching file include/linux/pci.h
patching file drivers/scsi/sata_uli.c

Now at patch patches/pcix
russb@lns1058:/net/maggie/exports/russb/lk/linux-2.6.13-rc6> quilt applied
patches/pcix


maggie:/net/maggie/exports/russb/lk/linux-2.6.13-rc6 # make -j5
   CHK     include/linux/version.h
make[1]: `arch/i386/kernel/asm-offsets.s' is up to date.
   CHK     include/asm-i386/asm_offsets.h
   CHK     usr/initramfs_list
   CHK     include/linux/compile.h
   CC      arch/i386/kernel/pci-dma.o
   CC      arch/i386/pci/i386.o
   CC      arch/i386/kernel/quirks.o
   CC      arch/i386/pci/pcbios.o
   CC      drivers/char/agp/backend.o
   CC      arch/i386/kernel/cpu/cyrix.o
   CC      arch/i386/kernel/cpu/mtrr/main.o
   CC      drivers/char/agp/frontend.o
   CC      arch/i386/pci/direct.o
   CC      lib/iomap.o
   CC      drivers/char/agp/generic.o
   LD      lib/built-in.o
   CC      arch/i386/pci/fixup.o
   LD      arch/i386/kernel/cpu/mtrr/built-in.o
   LD      arch/i386/kernel/cpu/built-in.o
   CC      arch/i386/pci/legacy.o
   LD      arch/i386/kernel/built-in.o
   CC      drivers/char/agp/isoch.o
   CC      drivers/char/drm/drm_auth.o
   CC      arch/i386/pci/irq.o
   CC      drivers/char/drm/drm_bufs.o
   CC      arch/i386/pci/common.o
   CC      drivers/char/agp/intel-agp.o
   CC      drivers/char/drm/drm_context.o
   LD      arch/i386/pci/built-in.o
   CC      drivers/char/drm/drm_dma.o
   CC      drivers/ide/pci/piix.o
   LD      drivers/char/agp/agpgart.o
   LD      drivers/char/agp/built-in.o
   CC      drivers/char/drm/drm_drawable.o
   CC      drivers/ieee1394/ieee1394_core.o
   CC      drivers/ide/ide.o
   CC      drivers/ide/ide-io.o
   CC      drivers/char/drm/drm_drv.o
   CC      drivers/ide/pci/rz1000.o
   CC      drivers/char/drm/drm_fops.o
   CC      drivers/char/drm/drm_init.o
   CC      drivers/ieee1394/hosts.o
   CC      drivers/char/drm/drm_ioctl.o
   CC      drivers/ide/pci/generic.o
   CC      drivers/ide/ide-iops.o
   CC      drivers/char/drm/drm_irq.o
   CC      drivers/ieee1394/nodemgr.o
   CC      drivers/ide/ide-lib.o
   LD      drivers/ide/pci/built-in.o
   CC      drivers/net/tg3.o
   CC      drivers/char/drm/drm_lock.o
   CC      drivers/parport/parport_pc.o
   CC      drivers/char/drm/drm_memory.o
   CC      drivers/ide/ide-probe.o
   CC      drivers/ieee1394/dma.o
   CC      drivers/char/drm/drm_proc.o
   CC      drivers/ieee1394/iso.o
   LD      drivers/parport/built-in.o
   CC      drivers/char/drm/drm_stub.o
   CC      drivers/ide/ide-taskfile.o
   CC      drivers/ieee1394/ohci1394.o
   CC      drivers/ieee1394/raw1394.o
   CC      drivers/char/drm/drm_vm.o
   CC      drivers/ide/pci/cmd640.o
   CC      drivers/char/drm/drm_agpsupport.o
   CC      drivers/char/drm/drm_scatter.o
   CC      drivers/ide/setup-pci.o
   CC      drivers/ide/ide-dma.o
   LD      drivers/ieee1394/ieee1394.o
   LD      drivers/ieee1394/built-in.o
   CC      drivers/ide/ide-proc.o
   CC      drivers/char/drm/ati_pcigart.o
   CC      drivers/pci/access.o
   CC      drivers/char/drm/drm_pci.o
   CC      drivers/pnp/resource.o
   CC      drivers/pci/bus.o
   CC      drivers/ide/ide-generic.o
   LD      drivers/char/drm/drm.o
   LD      drivers/char/drm/built-in.o
   LD      drivers/char/built-in.o
   CC      drivers/ide/ide-disk.o
   CC      drivers/pci/probe.o
   LD      drivers/pnp/built-in.o
   CC      drivers/ide/ide-cd.o
   CC      drivers/pci/remove.o
   LD      drivers/net/built-in.o
   LD      drivers/ide/ide-core.o
   CC      drivers/pci/pci.o
   CC      drivers/pci/quirks.o
   CC      drivers/pci/names.o
   CC      drivers/pci/pci-driver.o
   CC      drivers/scsi/libata-core.o
   CC      drivers/serial/8250_pnp.o
   LD      drivers/ide/built-in.o
   CC      drivers/pci/search.o
   CC      drivers/usb/core/message.o
   CC      drivers/serial/8250_pci.o
   CC      drivers/scsi/libata-scsi.o
   CC      drivers/pci/pci-sysfs.o
   CC      drivers/usb/core/hcd-pci.o
   LD      drivers/serial/built-in.o
   CC      drivers/usb/host/ehci-hcd.o
   CC      drivers/pci/rom.o
   CC      drivers/scsi/scsi_lib.o
   CC      drivers/pci/proc.o
   LD      drivers/usb/core/usbcore.o
   LD      drivers/usb/core/built-in.o
   CC      drivers/usb/host/uhci-hcd.o
   CC      drivers/pci/setup-res.o
   CC      drivers/pci/hotplug.o
   LD      drivers/scsi/libata.o
   CC      drivers/scsi/ahci.o
   CC      drivers/pci/setup-bus.o
   CC      drivers/scsi/ata_piix.o
   LD      drivers/scsi/sd_mod.o
   LD      drivers/scsi/scsi_mod.o
   LD      drivers/scsi/built-in.o
   LD      drivers/pci/built-in.o
   LD      drivers/usb/host/built-in.o
   LD      drivers/usb/built-in.o
   LD      drivers/built-in.o
   GEN     .version
   CHK     include/linux/compile.h
   UPD     include/linux/compile.h
   CC      init/version.o
   LD      init/built-in.o
   LD      .tmp_vmlinux1
   KSYM    .tmp_kallsyms1.S
   AS      .tmp_kallsyms1.o
   LD      .tmp_vmlinux2
   KSYM    .tmp_kallsyms2.S
   AS      .tmp_kallsyms2.o
   LD      vmlinux
   SYSMAP  System.map
   SYSMAP  .tmp_System.map
   Building modules, stage 2.
   AS      arch/i386/boot/setup.o
   MODPOST
   LD      arch/i386/boot/setup
   OBJCOPY arch/i386/boot/compressed/vmlinux.bin
   GZIP    arch/i386/boot/compressed/vmlinux.bin.gz
   LD      arch/i386/boot/compressed/piggy.o
   LD      arch/i386/boot/compressed/vmlinux
   OBJCOPY arch/i386/boot/vmlinux.bin
   BUILD   arch/i386/boot/bzImage
Root device is (8, 5)
Boot sector 512 bytes.
Setup is 4602 bytes.
System is 1814 kB
Kernel: arch/i386/boot/bzImage is ready  (#12)

Re: [PATCH 2.6.13-rc6] PCI/libata INTx cleanup

[Greg KH]

On Mon, Aug 15, 2005 at 03:08:47PM -0400, Brett Russ wrote:
> Greg KH wrote:
> >On Fri, Aug 12, 2005 at 06:43:03PM -0400, Brett Russ wrote:
> >
> >>Simple cleanup to eliminate X copies of the pci_enable_intx() function
> >>in libata.  Moved ahci.c's pci_intx() to pci.c and use it throughout
> >>libata and msi.c.
> >>
> >>Signed-off-by: Brett Russ <russb@emc.com>
> >
> >
> >It would have been nice if you had built this code :(
> >
> >Hint, get rid of all TRUE and FALSE usages in your patch.  Care to try
> >again?
> >
> >thanks,
> >
> >greg k-h
> 
> 
> Hmm, I did build it before submitting, saw not even a warning.  So I 
> just rebuilt it again to verify, and same thing.  Why would my tree work 
> and not yours?  config file?

You don't have MSI enabled in yours :(

thanks,

greg k-h

Re: [PATCH 2.6.13-rc6] PCI/libata INTx cleanup

[Brett Russ]

Greg KH wrote:
>>Hmm, I did build it before submitting, saw not even a warning.  So I 
>>just rebuilt it again to verify, and same thing.  Why would my tree work 
>>and not yours?  config file?
> 
> 
> You don't have MSI enabled in yours :(

Yup, you're right.  Sorry about that.  The patch I sent @15:23 today 
should be all set now.

BR

Re: [PATCH 2.6.13-rc6] PCI/libata INTx cleanup

[Jeff Garzik]

Greg KH wrote:
> On Fri, Aug 12, 2005 at 06:43:03PM -0400, Brett Russ wrote:
> 
>>Simple cleanup to eliminate X copies of the pci_enable_intx() function
>>in libata.  Moved ahci.c's pci_intx() to pci.c and use it throughout
>>libata and msi.c.
>>
>>Signed-off-by: Brett Russ <russb@emc.com>
> 
> 
> It would have been nice if you had built this code :(
> 
> Hint, get rid of all TRUE and FALSE usages in your patch.  Care to try
> again?

just manually s/TRUE/1/ and s/FALSE/0/ in the patch...

I could have sworn TRUE and FALSE were in linux/kernel.h, but it looks 
like I was wrong.

	Jeff

[PATCH 2.6.13-rc6] PCI/libata INTx cleanup

[Brett M Russ]

Simple cleanup to eliminate X copies of the pci_enable_intx() function
in libata.  Moved ahci.c's pci_intx() to pci.c and use it throughout
libata and msi.c.

Signed-off-by: Brett Russ <russb@emc.com>


Index: linux-2.6.13-rc6/drivers/scsi/sata_sis.c
===================================================================
--- linux-2.6.13-rc6.orig/drivers/scsi/sata_sis.c
+++ linux-2.6.13-rc6/drivers/scsi/sata_sis.c
@@ -186,18 +186,6 @@ static void sis_scr_write (struct ata_po
 		outl(val, ap->ioaddr.scr_addr + (sc_reg * 4));
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_enable_intx(struct pci_dev *pdev)
-{
-	u16 pci_command;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-	if (pci_command & PCI_COMMAND_INTX_DISABLE) {
-		pci_command &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-	}
-}
-
 static int sis_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct ata_probe_ent *probe_ent = NULL;
@@ -254,7 +242,7 @@ static int sis_init_one (struct pci_dev 
 	}
 
 	pci_set_master(pdev);
-	pci_enable_intx(pdev);
+	pci_intx(pdev, 1);
 
 	/* FIXME: check ata_device_add return value */
 	ata_device_add(probe_ent);
Index: linux-2.6.13-rc6/drivers/scsi/ahci.c
===================================================================
--- linux-2.6.13-rc6.orig/drivers/scsi/ahci.c
+++ linux-2.6.13-rc6/drivers/scsi/ahci.c
@@ -862,22 +862,6 @@ static int ahci_host_init(struct ata_pro
 	return 0;
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_intx(struct pci_dev *pdev, int enable)
-{
-	u16 pci_command, new;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-
-	if (enable)
-		new = pci_command & ~PCI_COMMAND_INTX_DISABLE;
-	else
-		new = pci_command | PCI_COMMAND_INTX_DISABLE;
-
-	if (new != pci_command)
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-}
-
 static void ahci_print_info(struct ata_probe_ent *probe_ent)
 {
 	struct ahci_host_priv *hpriv = probe_ent->private_data;
Index: linux-2.6.13-rc6/drivers/scsi/ata_piix.c
===================================================================
--- linux-2.6.13-rc6.orig/drivers/scsi/ata_piix.c
+++ linux-2.6.13-rc6/drivers/scsi/ata_piix.c
@@ -548,18 +548,6 @@ static void piix_set_dmamode (struct ata
 	}
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_enable_intx(struct pci_dev *pdev)
-{
-	u16 pci_command;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-	if (pci_command & PCI_COMMAND_INTX_DISABLE) {
-		pci_command &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-	}
-}
-
 #define AHCI_PCI_BAR 5
 #define AHCI_GLOBAL_CTL 0x04
 #define AHCI_ENABLE (1 << 31)
@@ -658,7 +646,7 @@ static int piix_init_one (struct pci_dev
 	 * message-signalled interrupts currently).
 	 */
 	if (port_info[0]->host_flags & PIIX_FLAG_CHECKINTR)
-		pci_enable_intx(pdev);
+		pci_intx(pdev, 1);
 
 	if (combined) {
 		port_info[sata_chan] = &piix_port_info[ent->driver_data];
Index: linux-2.6.13-rc6/drivers/pci/msi.c
===================================================================
--- linux-2.6.13-rc6.orig/drivers/pci/msi.c
+++ linux-2.6.13-rc6/drivers/pci/msi.c
@@ -446,10 +446,7 @@ static void enable_msi_mode(struct pci_d
 	}
     	if (pci_find_capability(dev, PCI_CAP_ID_EXP)) {
 		/* PCI Express Endpoint device detected */
-		u16 cmd;
-		pci_read_config_word(dev, PCI_COMMAND, &cmd);
-		cmd |= PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
+		pci_intx(dev, 0);  /* disable intx */
 	}
 }
 
@@ -468,10 +465,7 @@ static void disable_msi_mode(struct pci_
 	}
     	if (pci_find_capability(dev, PCI_CAP_ID_EXP)) {
 		/* PCI Express Endpoint device detected */
-		u16 cmd;
-		pci_read_config_word(dev, PCI_COMMAND, &cmd);
-		cmd &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
+		pci_intx(dev, 1);  /* enable intx */
 	}
 }
 
Index: linux-2.6.13-rc6/drivers/pci/pci.c
===================================================================
--- linux-2.6.13-rc6.orig/drivers/pci/pci.c
+++ linux-2.6.13-rc6/drivers/pci/pci.c
@@ -793,6 +793,31 @@ pci_clear_mwi(struct pci_dev *dev)
 	}
 }
 
+/**
+ * pci_intx - enables/disables PCI INTx for device dev
+ * @dev: the PCI device to operate on
+ * @enable: boolean
+ *
+ * Enables/disables PCI INTx for device dev
+ */
+void 
+pci_intx(struct pci_dev *pdev, int enable)
+{
+	u16 pci_command, new;
+
+	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
+
+	if (enable) {
+		new = pci_command & ~PCI_COMMAND_INTX_DISABLE;
+	} else {
+		new = pci_command | PCI_COMMAND_INTX_DISABLE;
+	}
+
+	if (new != pci_command) {
+		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
+	}
+}
+
 #ifndef HAVE_ARCH_PCI_SET_DMA_MASK
 /*
  * These can be overridden by arch-specific implementations
@@ -870,6 +895,7 @@ EXPORT_SYMBOL(pci_request_region);
 EXPORT_SYMBOL(pci_set_master);
 EXPORT_SYMBOL(pci_set_mwi);
 EXPORT_SYMBOL(pci_clear_mwi);
+EXPORT_SYMBOL_GPL(pci_intx);
 EXPORT_SYMBOL(pci_set_dma_mask);
 EXPORT_SYMBOL(pci_set_consistent_dma_mask);
 EXPORT_SYMBOL(pci_assign_resource);
Index: linux-2.6.13-rc6/include/linux/pci.h
===================================================================
--- linux-2.6.13-rc6.orig/include/linux/pci.h
+++ linux-2.6.13-rc6/include/linux/pci.h
@@ -815,6 +815,7 @@ void pci_set_master(struct pci_dev *dev)
 #define HAVE_PCI_SET_MWI
 int pci_set_mwi(struct pci_dev *dev);
 void pci_clear_mwi(struct pci_dev *dev);
+void pci_intx(struct pci_dev *dev, int enable);
 int pci_set_dma_mask(struct pci_dev *dev, u64 mask);
 int pci_set_consistent_dma_mask(struct pci_dev *dev, u64 mask);
 void pci_update_resource(struct pci_dev *dev, struct resource *res, int resno);
Index: linux-2.6.13-rc6/drivers/scsi/sata_uli.c
===================================================================
--- linux-2.6.13-rc6.orig/drivers/scsi/sata_uli.c
+++ linux-2.6.13-rc6/drivers/scsi/sata_uli.c
@@ -171,18 +171,6 @@ static void uli_scr_write (struct ata_po
 	uli_scr_cfg_write(ap, sc_reg, val);
 }
 
-/* move to PCI layer, integrate w/ MSI stuff */
-static void pci_enable_intx(struct pci_dev *pdev)
-{
-	u16 pci_command;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
-	if (pci_command & PCI_COMMAND_INTX_DISABLE) {
-		pci_command &= ~PCI_COMMAND_INTX_DISABLE;
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
-	}
-}
-
 static int uli_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct ata_probe_ent *probe_ent;
@@ -255,7 +243,7 @@ static int uli_init_one (struct pci_dev 
 	}
 
 	pci_set_master(pdev);
-	pci_enable_intx(pdev);
+	pci_intx(pdev, 1);
 
 	/* FIXME: check ata_device_add return value */
 	ata_device_add(probe_ent);

[PATCH 2.6.13] PCI/libata INTx bug fix

[Brett M Russ]

Previous INTx cleanup patch had a bug that was not caught.  I found
this last night during testing and can confirm that it is now 100%
working.

Signed-off-by: Brett Russ <russb@emc.com>


Index: linux-2.6.13/drivers/pci/pci.c
===================================================================
--- linux-2.6.13.orig/drivers/pci/pci.c
+++ linux-2.6.13/drivers/pci/pci.c
@@ -764,7 +764,7 @@ pci_intx(struct pci_dev *pdev, int enabl
 	}
 
 	if (new != pci_command) {
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
+		pci_write_config_word(pdev, PCI_COMMAND, new);
 	}
 }
 

Re: [PATCH 2.6.13] PCI/libata INTx bug fix

[Jeff Garzik]

Brett M Russ wrote:
> Previous INTx cleanup patch had a bug that was not caught.  I found
> this last night during testing and can confirm that it is now 100%
> working.
> 
> Signed-off-by: Brett Russ <russb@emc.com>
> 
> 
> Index: linux-2.6.13/drivers/pci/pci.c
> ===================================================================
> --- linux-2.6.13.orig/drivers/pci/pci.c
> +++ linux-2.6.13/drivers/pci/pci.c
> @@ -764,7 +764,7 @@ pci_intx(struct pci_dev *pdev, int enabl
>  	}
>  
>  	if (new != pci_command) {
> -		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
> +		pci_write_config_word(pdev, PCI_COMMAND, new);


If GregKH doesn't pick this up, I'll send it to Andrew/Linus ASAP.

libata is the only user currently, and we definitely need this fix.

	Jeff

[PATCH] PCI: PCI/libata INTx bug fix

[Greg KH]

From: Brett M Russ <russb@emc.com>

Previous INTx cleanup patch had a bug that was not caught.  I found
this last night during testing and can confirm that it is now 100%
working.

Signed-off-by: Brett Russ <russb@emc.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -819,7 +819,7 @@ pci_intx(struct pci_dev *pdev, int enabl
 	}
 
 	if (new != pci_command) {
-		pci_write_config_word(pdev, PCI_COMMAND, pci_command);
+		pci_write_config_word(pdev, PCI_COMMAND, new);
 	}
 }
 

Re: [PATCH] PCI: PCI/libata INTx bug fix

[Jeff Garzik]

On Fri, Sep 09, 2005 at 10:02:22AM -0700, Greg KH wrote:
> From: Brett M Russ <russb@emc.com>
> 
> Previous INTx cleanup patch had a bug that was not caught.  I found
> this last night during testing and can confirm that it is now 100%
> working.
> 
> Signed-off-by: Brett Russ <russb@emc.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

Acked-by: Jeff Garzik <jgarzik@pobox.com>

Linus, please apply to fix my drivers :)

Greg and Brett, thanks much.

	Jeff

Re: [PATCH 2.6.12.3] PCI/libata INTx cleanup

[Brian Gerst]

Greg KH wrote:
> On Fri, Aug 12, 2005 at 01:10:43PM -0400, Brett Russ wrote:
> 
>>Jeff Garzik wrote:
>>
>>>Brett Russ wrote:
>>>
>>>
>>>>Simple cleanup to eliminate X copies of the same function in libata.  
>>>>Moved pci_enable_intx() to pci.c, added pci_disable_intx() as well, 
>>>>and use them throughout libata and msi.c.
>>>>
>>>>Signed-off-by: Brett Russ <russb@emc.com>
>>>
>>>
>>>Though there is nothing wrong with this patch, I would prefer a single 
>>>function, pci_intx(), as found in drivers/scsi/ahci.c.
>>>
>>>Would you be willing to move that one to the PCI layer, eliminate the 
>>>multiple copies of pci_enable_intx(), and replace the calls to 
>>>pci_enable_intx() with calls to pci_intx() ?
>>
>>Sounds like what I did, except for the naming change.  I did away with
>>pci_disable_intx() and changed the names.  Look ok?
> 
> 
> Looks ok to me, care to resend it with a Signed-off-by: and a new
> changelog entry so I can apply it?
> 
> 
>>+EXPORT_SYMBOL(pci_intx);
> 
> 
> Hm, for new pci functions, I prefer to have them marked as
> EXPORT_SYMBOL_GPL(), is that ok with you?

It's not really "new".  It's simply a trivial wrapper around other pci 
functions.

--
				Brian Gerst

Re: [PATCH 2.6.12.3] PCI/libata INTx cleanup

[Greg KH]

On Fri, Aug 12, 2005 at 05:44:08PM -0400, Brian Gerst wrote:
> Greg KH wrote:
> >On Fri, Aug 12, 2005 at 01:10:43PM -0400, Brett Russ wrote:
> >
> >>Jeff Garzik wrote:
> >>
> >>>Brett Russ wrote:
> >>>
> >>>
> >>>>Simple cleanup to eliminate X copies of the same function in libata.  
> >>>>Moved pci_enable_intx() to pci.c, added pci_disable_intx() as well, 
> >>>>and use them throughout libata and msi.c.
> >>>>
> >>>>Signed-off-by: Brett Russ <russb@emc.com>
> >>>
> >>>
> >>>Though there is nothing wrong with this patch, I would prefer a single 
> >>>function, pci_intx(), as found in drivers/scsi/ahci.c.
> >>>
> >>>Would you be willing to move that one to the PCI layer, eliminate the 
> >>>multiple copies of pci_enable_intx(), and replace the calls to 
> >>>pci_enable_intx() with calls to pci_intx() ?
> >>
> >>Sounds like what I did, except for the naming change.  I did away with
> >>pci_disable_intx() and changed the names.  Look ok?
> >
> >
> >Looks ok to me, care to resend it with a Signed-off-by: and a new
> >changelog entry so I can apply it?
> >
> >
> >>+EXPORT_SYMBOL(pci_intx);
> >
> >
> >Hm, for new pci functions, I prefer to have them marked as
> >EXPORT_SYMBOL_GPL(), is that ok with you?
> 
> It's not really "new".  It's simply a trivial wrapper around other pci 
> functions.

It's a newly exported function.

thanks,

greg k-h

Re: [PATCH 2.6.12.3] PCI/libata INTx cleanup

[Jeff Garzik]

Brett Russ wrote:
> Jeff Garzik wrote:

>>Though there is nothing wrong with this patch, I would prefer a single 
>>function, pci_intx(), as found in drivers/scsi/ahci.c.

> Sounds like what I did, except for the naming change.  I did away with
> pci_disable_intx() and changed the names.  Look ok?


Nope.

<thinks, and checks something>  Ahhhhh.  You were looking at an older 
kernel.Nope.  Read the implementation I referenced, in ahci.c, from 
2.6.13-rc6.  It takes a second argument:

	static void pci_intx(struct pci_dev *pdev, int enable)

Regards,

	Jeff