linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] PCI-X/PCI-Express read control interfaces
@ 2006-12-08 18:22 Stephen Hemminger
  2006-12-08 18:22 ` [PATCH 1/6] PCI-X Max Read Byte Count interface Stephen Hemminger
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Stephen Hemminger @ 2006-12-08 18:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-pci, linux-kernel

This patch set adds hooks to set PCI-X max read request size
and PCI-Express read request size. It is important that this be a PCI
subsystem function rather than a per device hack. That way, the PCI
system quirks can be used if needed.

-- 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/6] PCI-X Max Read Byte Count interface
  2006-12-08 18:22 [PATCH 0/6] PCI-X/PCI-Express read control interfaces Stephen Hemminger
@ 2006-12-08 18:22 ` Stephen Hemminger
  2006-12-08 22:56   ` [PATCH 1/6] PCI-X Max Read Byte Count interface (v2) Stephen Hemminger
  2006-12-08 18:22 ` [PATCH 2/6] e1000: use pcix_set_mmrbc Stephen Hemminger
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Stephen Hemminger @ 2006-12-08 18:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-pci, linux-kernel

[-- Attachment #1: pci-x-mmrbc --]
[-- Type: text/plain, Size: 4745 bytes --]

Add interface to allow setting PCI-X Max Memory Read Byte Count
The get interface is commented out because there are no drivers
that need it.

The AMD 8131 chipset has a number of errata's related to PCI-X
configuration. The BIOS sets initial value correctly, but if a
device changes MMRBC it could cause data corruption. There are
some more settings of MMRBC that could be allowed but 
determining the safe values is more effort than it is worth.


Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
---
 drivers/pci/pci.c    |   80 +++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/quirks.c |   19 +++++++++++-
 include/linux/pci.h  |    5 ++-
 3 files changed, 102 insertions(+), 2 deletions(-)

--- pci-x.orig/drivers/pci/pci.c
+++ pci-x/drivers/pci/pci.c
@@ -1059,6 +1059,86 @@ pci_set_consistent_dma_mask(struct pci_d
 	return 0;
 }
 #endif
+
+
+#if 0
+/**
+ * pcix_get_mmrbc - get PCI-X maximum memory read byte count
+ * @dev: PCI device to query
+ *
+ * Returns mmrbc: maximum memory read count in bytes
+ *    or appropriate error value.
+ */
+int pcix_get_mmrbc(struct pci_dev *dev)
+{
+	int ret, cap;
+	u32 cmd;
+
+	cap = pci_find_capability(dev, PCI_CAP_ID_PCIX);
+	if (!cap)
+		return -EINVAL;
+
+	ret = pci_read_config_dword(dev, cap + PCI_X_CMD, &cmd);
+	if (!ret)
+		ret = 512 << ((cmd & PCI_X_CMD_MAX_READ) >> 2);
+
+	return ret;
+}
+EXPORT_SYMBOL(pcix_get_mmrbc);
+#endif
+
+/**
+ * pcix_set_mmrbc - set PCI-X maximum memory read byte count
+ * @dev: PCI device to query
+ * @mmrbc: maximum memory read count in bytes
+ *    valid values are 512, 1024, 2048, 4096
+ *
+ * If possible sets maximum memory read byte count, some bridges have erratas
+ * that prevent this.
+ */
+int
+pcix_set_mmrbc(struct pci_dev *dev, int mmrbc)
+{
+	int cap, err = -EINVAL;
+	u32 stat, cmd, v, o;
+
+	if (mmrbc < 512 || mmrbc > 4096 || (mmrbc & (mmrbc-1)))
+		goto out;
+
+	v = ffs(mmrbc) - 10;
+
+	cap = pci_find_capability(dev, PCI_CAP_ID_PCIX);
+	if (!cap)
+		goto out;	/* not PCI-X */
+
+	err = pci_read_config_dword(dev, cap + PCI_X_STATUS, &stat);
+	if (err)
+		goto out;
+
+	if (v > (stat & PCI_X_STATUS_MAX_READ) >> 21)
+		return -E2BIG;
+
+	err = pci_read_config_dword(dev, cap + PCI_X_CMD, &cmd);
+	if (err)
+		goto out;
+
+	o = (cmd & PCI_X_CMD_MAX_READ) >> 2;
+	if (o != v) {
+		/* Increasing can cause problems o some broken bridges */
+		if (v > o && dev->bus &&
+		    (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_MMRBC))
+			return -EIO;
+
+		cmd &= ~PCI_X_CMD_MAX_READ;
+		cmd |= v << 2;
+		err = pci_write_config_dword(dev, cap + PCI_X_CMD, cmd);
+	}
+
+out:
+	return err;
+}
+EXPORT_SYMBOL(pcix_set_mmrbc);
+
      
 static int __devinit pci_init(void)
 {
--- pci-x.orig/drivers/pci/quirks.c
+++ pci-x/drivers/pci/quirks.c
@@ -615,9 +615,26 @@ static void __init quirk_amd_8131_ioapic
                 pci_write_config_byte( dev, AMD8131_MISC, tmp);
         }
 } 
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_8131_BRIDGE, quirk_amd_8131_ioapic);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_8131_BRIDGE, quirk_amd_8131o_ioapic);
 #endif /* CONFIG_X86_IO_APIC */
 
+/*
+ * Some settings of MMRBC can lead to data corruption so block changes.
+ * See AMD 8131 HyperTransport PCI-X Tunnel Revision Guide
+ */
+static void __init quirk_amd_8131_mmrbc(struct pci_dev *dev)
+{
+	unsigned char revid;
+
+        pci_read_config_byte(dev, PCI_REVISION_ID, &revid);
+        if (dev->subordinate && revid <= 0x12) {
+                printk(KERN_INFO "AMD8131 rev %x detected, disabling PCI-X MMRBC\n",
+		       revid);
+		dev->subordinate->bus_flags |= PCI_BUS_FLAGS_NO_MMRBC;
+        }
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_8131_BRIDGE, quirk_amd_8131_mmrbc);
+
 
 /*
  * FIXME: it is questionable that quirk_via_acpi
--- pci-x.orig/include/linux/pci.h
+++ pci-x/include/linux/pci.h
@@ -98,7 +98,8 @@ enum pci_channel_state {
 
 typedef unsigned short __bitwise pci_bus_flags_t;
 enum pci_bus_flags {
-	PCI_BUS_FLAGS_NO_MSI = (__force pci_bus_flags_t) 1,
+	PCI_BUS_FLAGS_NO_MSI   = (__force pci_bus_flags_t) 1,
+	PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2,
 };
 
 struct pci_cap_saved_state {
@@ -511,6 +512,8 @@ 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);
+int pcix_get_mmrbc(struct pci_dev *dev);
+int pcix_set_mmrbc(struct pci_dev *dev, int mmrbc);
 void pci_update_resource(struct pci_dev *dev, struct resource *res, int resno);
 int __must_check pci_assign_resource(struct pci_dev *dev, int i);
 int __must_check pci_assign_resource_fixed(struct pci_dev *dev, int i);

-- 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/6] e1000: use pcix_set_mmrbc
  2006-12-08 18:22 [PATCH 0/6] PCI-X/PCI-Express read control interfaces Stephen Hemminger
  2006-12-08 18:22 ` [PATCH 1/6] PCI-X Max Read Byte Count interface Stephen Hemminger
@ 2006-12-08 18:22 ` Stephen Hemminger
  2006-12-08 21:45   ` Roland Dreier
  2006-12-08 18:22 ` [PATCH 3/6] PCI Express get/set read request size Stephen Hemminger
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Stephen Hemminger @ 2006-12-08 18:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-pci, linux-kernel

[-- Attachment #1: e1000-mmrbc --]
[-- Type: text/plain, Size: 3008 bytes --]

Use new pcix_set_mmrbc interface, this prevents possible data
corruption on broken PCI-X chipsets.

The E1000 PCI hardware wrappers are a nuisance.
Untested on real hardware.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

---
 drivers/net/e1000/e1000_hw.c   |   23 ++---------------------
 drivers/net/e1000/e1000_hw.h   |    1 +
 drivers/net/e1000/e1000_main.c |    8 ++++++++
 3 files changed, 11 insertions(+), 21 deletions(-)

--- pci-x.orig/drivers/net/e1000/e1000_hw.c
+++ pci-x/drivers/net/e1000/e1000_hw.c
@@ -846,10 +846,6 @@ e1000_init_hw(struct e1000_hw *hw)
     uint32_t ctrl;
     uint32_t i;
     int32_t ret_val;
-    uint16_t pcix_cmd_word;
-    uint16_t pcix_stat_hi_word;
-    uint16_t cmd_mmrbc;
-    uint16_t stat_mmrbc;
     uint32_t mta_size;
     uint32_t reg_data;
     uint32_t ctrl_ext;
@@ -939,23 +935,8 @@ e1000_init_hw(struct e1000_hw *hw)
         break;
     default:
         /* Workaround for PCI-X problem when BIOS sets MMRBC incorrectly. */
-        if (hw->bus_type == e1000_bus_type_pcix) {
-            e1000_read_pci_cfg(hw, PCIX_COMMAND_REGISTER, &pcix_cmd_word);
-            e1000_read_pci_cfg(hw, PCIX_STATUS_REGISTER_HI,
-                &pcix_stat_hi_word);
-            cmd_mmrbc = (pcix_cmd_word & PCIX_COMMAND_MMRBC_MASK) >>
-                PCIX_COMMAND_MMRBC_SHIFT;
-            stat_mmrbc = (pcix_stat_hi_word & PCIX_STATUS_HI_MMRBC_MASK) >>
-                PCIX_STATUS_HI_MMRBC_SHIFT;
-            if (stat_mmrbc == PCIX_STATUS_HI_MMRBC_4K)
-                stat_mmrbc = PCIX_STATUS_HI_MMRBC_2K;
-            if (cmd_mmrbc > stat_mmrbc) {
-                pcix_cmd_word &= ~PCIX_COMMAND_MMRBC_MASK;
-                pcix_cmd_word |= stat_mmrbc << PCIX_COMMAND_MMRBC_SHIFT;
-                e1000_write_pci_cfg(hw, PCIX_COMMAND_REGISTER,
-                    &pcix_cmd_word);
-            }
-        }
+        if (hw->bus_type == e1000_bus_type_pcix)
+		e1000_pcix_set_mmrbc(hw, 2048);
         break;
     }
 
--- pci-x.orig/drivers/net/e1000/e1000_main.c
+++ pci-x/drivers/net/e1000/e1000_main.c
@@ -4770,6 +4770,14 @@ e1000_read_pci_cfg(struct e1000_hw *hw, 
 }
 
 void
+e1000_pcix_set_mmrbc(struct e1000_hw *hw, int mmrbc)
+{
+	struct e1000_adapter *adapter = hw->back;
+
+	pcix_set_mmrbc(adapter->pdev, mmrbc);
+}
+
+void
 e1000_write_pci_cfg(struct e1000_hw *hw, uint32_t reg, uint16_t *value)
 {
 	struct e1000_adapter *adapter = hw->back;
--- pci-x.orig/drivers/net/e1000/e1000_hw.h
+++ pci-x/drivers/net/e1000/e1000_hw.h
@@ -421,6 +421,7 @@ void e1000_tbi_adjust_stats(struct e1000
 void e1000_get_bus_info(struct e1000_hw *hw);
 void e1000_pci_set_mwi(struct e1000_hw *hw);
 void e1000_pci_clear_mwi(struct e1000_hw *hw);
+void e1000_pcix_set_mmrbc(struct e1000_hw *hw, int mmbrc);
 void e1000_read_pci_cfg(struct e1000_hw *hw, uint32_t reg, uint16_t * value);
 void e1000_write_pci_cfg(struct e1000_hw *hw, uint32_t reg, uint16_t * value);
 int32_t e1000_read_pcie_cap_reg(struct e1000_hw *hw, uint32_t reg, uint16_t *value);

-- 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 3/6] PCI Express get/set read request size
  2006-12-08 18:22 [PATCH 0/6] PCI-X/PCI-Express read control interfaces Stephen Hemminger
  2006-12-08 18:22 ` [PATCH 1/6] PCI-X Max Read Byte Count interface Stephen Hemminger
  2006-12-08 18:22 ` [PATCH 2/6] e1000: use pcix_set_mmrbc Stephen Hemminger
@ 2006-12-08 18:22 ` Stephen Hemminger
  2006-12-08 18:22 ` [PATCH 4/6] MTHCA driver (infiniband) use new pci interfaces Stephen Hemminger
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Stephen Hemminger @ 2006-12-08 18:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-pci, linux-kernel

[-- Attachment #1: pcie-rrbc.patch --]
[-- Type: text/plain, Size: 2607 bytes --]

Add PCI infrastructure for setting read request size.
The pcie_get_readrq is commented out because there are no drivers
that need it.


---
 drivers/pci/pci.c   |   65 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/pci.h |    4 ++-
 2 files changed, 67 insertions(+), 2 deletions(-)

--- pci-x.orig/drivers/pci/pci.c
+++ pci-x/drivers/pci/pci.c
@@ -1139,7 +1139,70 @@ out:
 }
 EXPORT_SYMBOL(pcix_set_mmrbc);
 
-     
+#if 0
+/**
+ * pcie_get_readrq - get PCI Express read request size
+ * @dev: PCI device to query
+ *
+ * Returns maximum memory read request in bytes
+ *    or appropriate error value.
+ */
+int pcie_get_readrq(struct pci_dev *dev)
+{
+	int ret, cap;
+	u16 ctl;
+
+	cap = pci_find_capability(dev, PCI_CAP_ID_EXP);
+	if (!cap)
+		return -EINVAL;
+
+	ret = pci_read_config_word(dev, cap + PCI_EXP_DEVCTL, &ctl);
+	if (!ret)
+		ret = 128 << ((ctl & PCI_EXP_DEVCTL_READRQ) >> 12);
+
+	return ret;
+}
+EXPORT_SYMBOL(pcie_get_readrq);
+#endif
+
+/**
+ * pcie_set_readrq - set PCI Express maximum memory read request
+ * @dev: PCI device to query
+ * @count: maximum memory read count in bytes
+ *    valid values are 128, 256, 512, 1024, 2048, 4096
+ *
+ * If possible sets maximum read byte count
+ */
+int
+pcie_set_readrq(struct pci_dev *dev, int count)
+{
+	int cap, err = -EINVAL;
+	u16 ctl, v;
+
+	if (count < 128 || count > 4096 || (count & (count-1)))
+		goto out;
+
+	v = (ffs(count) - 8) << 12;
+
+	cap = pci_find_capability(dev, PCI_CAP_ID_EXP);
+	if (!cap)
+		goto out;
+
+	err = pci_read_config_word(dev, cap + PCI_EXP_DEVCTL, &ctl);
+	if (err)
+		goto out;
+
+	if ((ctl & PCI_EXP_DEVCTL_READRQ) != v) {
+		ctl &= ~PCI_EXP_DEVCTL_READRQ;
+		ctl |= v;
+		err = pci_write_config_dword(dev, cap + PCI_EXP_DEVCTL, ctl);
+	}
+
+out:
+	return err;
+}
+EXPORT_SYMBOL(pcie_set_readrq);
+
 static int __devinit pci_init(void)
 {
 	struct pci_dev *dev = NULL;
--- pci-x.orig/include/linux/pci.h
+++ pci-x/include/linux/pci.h
@@ -513,7 +513,9 @@ void pci_intx(struct pci_dev *dev, int e
 int pci_set_dma_mask(struct pci_dev *dev, u64 mask);
 int pci_set_consistent_dma_mask(struct pci_dev *dev, u64 mask);
 int pcix_get_mmrbc(struct pci_dev *dev);
-int pcix_set_mmrbc(struct pci_dev *dev, int mmrbc);
+int pcix_set_mmrbc(struct pci_dev *dev, int rq);
+int pcie_get_readrq(struct pci_dev *dev);
+int pcie_set_readrq(struct pci_dev *dev, int rq);
 void pci_update_resource(struct pci_dev *dev, struct resource *res, int resno);
 int __must_check pci_assign_resource(struct pci_dev *dev, int i);
 int __must_check pci_assign_resource_fixed(struct pci_dev *dev, int i);

-- 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 4/6] MTHCA driver (infiniband) use new pci interfaces
  2006-12-08 18:22 [PATCH 0/6] PCI-X/PCI-Express read control interfaces Stephen Hemminger
                   ` (2 preceding siblings ...)
  2006-12-08 18:22 ` [PATCH 3/6] PCI Express get/set read request size Stephen Hemminger
@ 2006-12-08 18:22 ` Stephen Hemminger
  2006-12-08 21:46   ` Roland Dreier
  2006-12-11  3:55   ` Benjamin Herrenschmidt
  2006-12-08 18:22 ` [PATCH 5/6] QLA2 use pci read tuning interface Stephen Hemminger
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 20+ messages in thread
From: Stephen Hemminger @ 2006-12-08 18:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-pci, linux-kernel

[-- Attachment #1: mthca-rbc.patch --]
[-- Type: text/plain, Size: 2241 bytes --]

Use new pci interfaces to set read request tuning values
Untested because of lack of hardware.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

---
 drivers/infiniband/hw/mthca/mthca_main.c |   38 +++++++------------------------
 1 file changed, 9 insertions(+), 29 deletions(-)

--- pci-x.orig/drivers/infiniband/hw/mthca/mthca_main.c
+++ pci-x/drivers/infiniband/hw/mthca/mthca_main.c
@@ -100,45 +100,25 @@ static struct mthca_profile default_prof
 
 static int mthca_tune_pci(struct mthca_dev *mdev)
 {
-	int cap;
-	u16 val;
-
 	if (!tune_pci)
 		return 0;
 
 	/* First try to max out Read Byte Count */
-	cap = pci_find_capability(mdev->pdev, PCI_CAP_ID_PCIX);
-	if (cap) {
-		if (pci_read_config_word(mdev->pdev, cap + PCI_X_CMD, &val)) {
-			mthca_err(mdev, "Couldn't read PCI-X command register, "
+	if (pci_find_capability(mdev->pdev, PCI_CAP_ID_PCIX)) {
+		if (pcix_set_mmrbc(mdev->pdev, 4096)) {
+			mthca_err(mdev, "Couldn't set PCI-X max read count, "
 				  "aborting.\n");
 			return -ENODEV;
 		}
-		val = (val & ~PCI_X_CMD_MAX_READ) | (3 << 2);
-		if (pci_write_config_word(mdev->pdev, cap + PCI_X_CMD, val)) {
-			mthca_err(mdev, "Couldn't write PCI-X command register, "
-				  "aborting.\n");
-			return -ENODEV;
-		}
-	} else if (!(mdev->mthca_flags & MTHCA_FLAG_PCIE))
-		mthca_info(mdev, "No PCI-X capability, not setting RBC.\n");
+	}
 
-	cap = pci_find_capability(mdev->pdev, PCI_CAP_ID_EXP);
-	if (cap) {
-		if (pci_read_config_word(mdev->pdev, cap + PCI_EXP_DEVCTL, &val)) {
-			mthca_err(mdev, "Couldn't read PCI Express device control "
-				  "register, aborting.\n");
-			return -ENODEV;
-		}
-		val = (val & ~PCI_EXP_DEVCTL_READRQ) | (5 << 12);
-		if (pci_write_config_word(mdev->pdev, cap + PCI_EXP_DEVCTL, val)) {
-			mthca_err(mdev, "Couldn't write PCI Express device control "
-				  "register, aborting.\n");
+	if (pci_find_capability(mdev->pdev, PCI_CAP_ID_EXP)) {
+		if (pcie_set_readrq(mdev->pdev, 4096)) {
+			mthca_err(mdev, "Couldn't write PCI Express read request, "
+				  "aborting.\n");
 			return -ENODEV;
 		}
-	} else if (mdev->mthca_flags & MTHCA_FLAG_PCIE)
-		mthca_info(mdev, "No PCI Express capability, "
-			   "not setting Max Read Request Size.\n");
+	}
 
 	return 0;
 }

-- 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 5/6] QLA2 use pci read tuning interface
  2006-12-08 18:22 [PATCH 0/6] PCI-X/PCI-Express read control interfaces Stephen Hemminger
                   ` (3 preceding siblings ...)
  2006-12-08 18:22 ` [PATCH 4/6] MTHCA driver (infiniband) use new pci interfaces Stephen Hemminger
@ 2006-12-08 18:22 ` Stephen Hemminger
  2006-12-08 18:22 ` [PATCH 6/6] PCI-X relaxed ordering interface Stephen Hemminger
  2006-12-11  3:48 ` [PATCH 0/6] PCI-X/PCI-Express read control interfaces Benjamin Herrenschmidt
  6 siblings, 0 replies; 20+ messages in thread
From: Stephen Hemminger @ 2006-12-08 18:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-pci, linux-kernel

[-- Attachment #1: qla-mmrbc --]
[-- Type: text/plain, Size: 1693 bytes --]

(Resend of earlier patch, driver name conflicts with porn filters)

Use new PCI read tuning interface. This makes sure driver doesn't
run into PCI chipset errata problems now or in future.
Untested on real hardware.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

--- pci-x.orig/drivers/scsi/qla2xxx/qla_init.c
+++ pci-x/drivers/scsi/qla2xxx/qla_init.c
@@ -250,7 +250,6 @@ qla24xx_pci_config(scsi_qla_host_t *ha)
 	uint32_t d;
 	unsigned long flags = 0;
 	struct device_reg_24xx __iomem *reg = &ha->iobase->isp24;
-	int pcix_cmd_reg, pcie_dctl_reg;
 
 	pci_set_master(ha->pdev);
 	mwi = 0;
@@ -265,28 +264,10 @@ qla24xx_pci_config(scsi_qla_host_t *ha)
 	pci_write_config_byte(ha->pdev, PCI_LATENCY_TIMER, 0x80);
 
 	/* PCI-X -- adjust Maximum Memory Read Byte Count (2048). */
-	pcix_cmd_reg = pci_find_capability(ha->pdev, PCI_CAP_ID_PCIX);
-	if (pcix_cmd_reg) {
-		uint16_t pcix_cmd;
-
-		pcix_cmd_reg += PCI_X_CMD;
-		pci_read_config_word(ha->pdev, pcix_cmd_reg, &pcix_cmd);
-		pcix_cmd &= ~PCI_X_CMD_MAX_READ;
-		pcix_cmd |= 0x0008;
-		pci_write_config_word(ha->pdev, pcix_cmd_reg, pcix_cmd);
-	}
+	pcix_set_mmrbc(ha->pdev, 2048);
 
 	/* PCIe -- adjust Maximum Read Request Size (2048). */
-	pcie_dctl_reg = pci_find_capability(ha->pdev, PCI_CAP_ID_EXP);
-	if (pcie_dctl_reg) {
-		uint16_t pcie_dctl;
-
-		pcie_dctl_reg += PCI_EXP_DEVCTL;
-		pci_read_config_word(ha->pdev, pcie_dctl_reg, &pcie_dctl);
-		pcie_dctl &= ~PCI_EXP_DEVCTL_READRQ;
-		pcie_dctl |= 0x4000;
-		pci_write_config_word(ha->pdev, pcie_dctl_reg, pcie_dctl);
-	}
+	pcie_set_readrq(ha->pdev, 2048);
 
 	/* Reset expansion ROM address decode enable */
 	pci_read_config_dword(ha->pdev, PCI_ROM_ADDRESS, &d);

-- 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 6/6] PCI-X relaxed ordering interface
  2006-12-08 18:22 [PATCH 0/6] PCI-X/PCI-Express read control interfaces Stephen Hemminger
                   ` (4 preceding siblings ...)
  2006-12-08 18:22 ` [PATCH 5/6] QLA2 use pci read tuning interface Stephen Hemminger
@ 2006-12-08 18:22 ` Stephen Hemminger
  2006-12-11  3:48 ` [PATCH 0/6] PCI-X/PCI-Express read control interfaces Benjamin Herrenschmidt
  6 siblings, 0 replies; 20+ messages in thread
From: Stephen Hemminger @ 2006-12-08 18:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-pci, linux-kernel

[-- Attachment #1: pcix-ero --]
[-- Type: text/plain, Size: 3216 bytes --]

Add an interface for set/clearing relaxed ordering

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
---
 drivers/net/bnx2.c  |   10 +-------
 drivers/net/tg3.c   |    4 ---
 drivers/pci/pci.c   |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h |    2 +
 4 files changed, 63 insertions(+), 11 deletions(-)

--- pci-x.orig/drivers/net/bnx2.c
+++ pci-x/drivers/net/bnx2.c
@@ -3384,14 +3384,8 @@ bnx2_init_chip(struct bnx2 *bp)
 		REG_WR(bp, BNX2_TDMA_CONFIG, val);
 	}
 
-	if (bp->flags & PCIX_FLAG) {
-		u16 val16;
-
-		pci_read_config_word(bp->pdev, bp->pcix_cap + PCI_X_CMD,
-				     &val16);
-		pci_write_config_word(bp->pdev, bp->pcix_cap + PCI_X_CMD,
-				      val16 & ~PCI_X_CMD_ERO);
-	}
+	if (bp->flags & PCIX_FLAG)
+		pcix_clear_ero(bp->pdev);
 
 	REG_WR(bp, BNX2_MISC_ENABLE_SET_BITS,
 	       BNX2_MISC_ENABLE_SET_BITS_HOST_COALESCE_ENABLE |
--- pci-x.orig/drivers/net/tg3.c
+++ pci-x/drivers/net/tg3.c
@@ -4884,9 +4884,7 @@ static int tg3_chip_reset(struct tg3 *tp
 	pci_restore_state(tp->pdev);
 
 	/* Make sure PCI-X relaxed ordering bit is clear. */
-	pci_read_config_dword(tp->pdev, TG3PCI_X_CAPS, &val);
-	val &= ~PCIX_CAPS_RELAXED_ORDERING;
-	pci_write_config_dword(tp->pdev, TG3PCI_X_CAPS, val);
+	pcix_clear_ero(tp->pdev);
 
 	if (tp->tg3_flags2 & TG3_FLG2_5780_CLASS) {
 		u32 val;
--- pci-x.orig/drivers/pci/pci.c
+++ pci-x/drivers/pci/pci.c
@@ -1203,6 +1203,64 @@ out:
 }
 EXPORT_SYMBOL(pcie_set_readrq);
 
+/**
+ * pcix_set_ero - enable relaxed operations
+ * @dev: PCI device to enable
+ *
+ * Enables PCI-X Relaxed Ordering
+ */
+int
+pcix_set_ero(struct pci_dev *dev)
+{
+	int cap, err;
+	u16 val;
+
+	cap = pci_find_capability(dev, PCI_CAP_ID_PCIX);
+	if (!cap)
+		return -EINVAL;
+
+	err = pci_read_config_word(dev, cap + PCI_X_CMD, &val);
+	if (err)
+		goto out;
+
+	if (!(val & PCI_X_CMD_ERO)) {
+		val |= PCI_X_CMD_ERO;
+		err = pci_write_config_dword(dev, cap + PCI_X_CMD, val);
+	}
+out:
+	return err;
+}
+EXPORT_SYMBOL(pcix_set_ero);
+
+/**
+ * pcix_clear_ero - enable relaxed operations
+ * @dev: PCI device to disable reordering
+ *
+ * Disables PCI-X Relaxed Ordering
+ */
+int
+pcix_clear_ero(struct pci_dev *dev)
+{
+	int cap, err;
+	u16 val;
+
+	cap = pci_find_capability(dev, PCI_CAP_ID_PCIX);
+	if (!cap)
+		return -EINVAL;
+
+	err = pci_read_config_word(dev, cap + PCI_X_CMD, &val);
+	if (err)
+		goto out;
+
+	if (val & PCI_X_CMD_ERO) {
+		val &= ~PCI_X_CMD_ERO;
+		err = pci_write_config_dword(dev, cap + PCI_X_CMD, val);
+	}
+out:
+	return err;
+}
+EXPORT_SYMBOL(pcix_clear_ero);
+
 static int __devinit pci_init(void)
 {
 	struct pci_dev *dev = NULL;
--- pci-x.orig/include/linux/pci.h
+++ pci-x/include/linux/pci.h
@@ -516,6 +516,8 @@ int pcix_get_mmrbc(struct pci_dev *dev);
 int pcix_set_mmrbc(struct pci_dev *dev, int rq);
 int pcie_get_readrq(struct pci_dev *dev);
 int pcie_set_readrq(struct pci_dev *dev, int rq);
+int pcix_set_ero(struct pci_dev *dev);
+int pcix_clear_ero(struct pci_dev *dev);
 void pci_update_resource(struct pci_dev *dev, struct resource *res, int resno);
 int __must_check pci_assign_resource(struct pci_dev *dev, int i);
 int __must_check pci_assign_resource_fixed(struct pci_dev *dev, int i);

-- 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/6] e1000: use pcix_set_mmrbc
  2006-12-08 18:22 ` [PATCH 2/6] e1000: use pcix_set_mmrbc Stephen Hemminger
@ 2006-12-08 21:45   ` Roland Dreier
  2006-12-08 22:43     ` Stephen Hemminger
  0 siblings, 1 reply; 20+ messages in thread
From: Roland Dreier @ 2006-12-08 21:45 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Greg Kroah-Hartman, linux-pci, linux-kernel

 > -        if (hw->bus_type == e1000_bus_type_pcix) {
 > -            e1000_read_pci_cfg(hw, PCIX_COMMAND_REGISTER, &pcix_cmd_word);
 > -            e1000_read_pci_cfg(hw, PCIX_STATUS_REGISTER_HI,
 > -                &pcix_stat_hi_word);
 > -            cmd_mmrbc = (pcix_cmd_word & PCIX_COMMAND_MMRBC_MASK) >>
 > -                PCIX_COMMAND_MMRBC_SHIFT;
 > -            stat_mmrbc = (pcix_stat_hi_word & PCIX_STATUS_HI_MMRBC_MASK) >>
 > -                PCIX_STATUS_HI_MMRBC_SHIFT;
 > -            if (stat_mmrbc == PCIX_STATUS_HI_MMRBC_4K)
 > -                stat_mmrbc = PCIX_STATUS_HI_MMRBC_2K;
 > -            if (cmd_mmrbc > stat_mmrbc) {
 > -                pcix_cmd_word &= ~PCIX_COMMAND_MMRBC_MASK;
 > -                pcix_cmd_word |= stat_mmrbc << PCIX_COMMAND_MMRBC_SHIFT;
 > -                e1000_write_pci_cfg(hw, PCIX_COMMAND_REGISTER,
 > -                    &pcix_cmd_word);
 > -            }
 > -        }
 > +        if (hw->bus_type == e1000_bus_type_pcix)
 > +		e1000_pcix_set_mmrbc(hw, 2048);

This changes the behavior of the driver.  The existing driver only
sets MMRBC if it's bigger than min(2048, value in the status register).
You're setting MMRBC to 2048 even if it starts out at a smaller value.

 - R.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/6] MTHCA driver (infiniband) use new pci interfaces
  2006-12-08 18:22 ` [PATCH 4/6] MTHCA driver (infiniband) use new pci interfaces Stephen Hemminger
@ 2006-12-08 21:46   ` Roland Dreier
  2006-12-11  3:55   ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 20+ messages in thread
From: Roland Dreier @ 2006-12-08 21:46 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Greg Kroah-Hartman, linux-pci, linux-kernel

Looks fine (assuming the PCI core interfaces it uses go in).

Acked-by: Roland Dreier <rolandd@cisco.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/6] e1000: use pcix_set_mmrbc
  2006-12-08 21:45   ` Roland Dreier
@ 2006-12-08 22:43     ` Stephen Hemminger
  2006-12-08 22:58       ` Auke Kok
  0 siblings, 1 reply; 20+ messages in thread
From: Stephen Hemminger @ 2006-12-08 22:43 UTC (permalink / raw)
  To: Roland Dreier; +Cc: Greg Kroah-Hartman, linux-pci, linux-kernel

On Fri, 08 Dec 2006 13:45:05 -0800
Roland Dreier <rdreier@cisco.com> wrote:

>  > -        if (hw->bus_type == e1000_bus_type_pcix) {
>  > -            e1000_read_pci_cfg(hw, PCIX_COMMAND_REGISTER, &pcix_cmd_word);
>  > -            e1000_read_pci_cfg(hw, PCIX_STATUS_REGISTER_HI,
>  > -                &pcix_stat_hi_word);
>  > -            cmd_mmrbc = (pcix_cmd_word & PCIX_COMMAND_MMRBC_MASK) >>
>  > -                PCIX_COMMAND_MMRBC_SHIFT;
>  > -            stat_mmrbc = (pcix_stat_hi_word & PCIX_STATUS_HI_MMRBC_MASK) >>
>  > -                PCIX_STATUS_HI_MMRBC_SHIFT;
>  > -            if (stat_mmrbc == PCIX_STATUS_HI_MMRBC_4K)
>  > -                stat_mmrbc = PCIX_STATUS_HI_MMRBC_2K;
>  > -            if (cmd_mmrbc > stat_mmrbc) {
>  > -                pcix_cmd_word &= ~PCIX_COMMAND_MMRBC_MASK;
>  > -                pcix_cmd_word |= stat_mmrbc << PCIX_COMMAND_MMRBC_SHIFT;
>  > -                e1000_write_pci_cfg(hw, PCIX_COMMAND_REGISTER,
>  > -                    &pcix_cmd_word);
>  > -            }
>  > -        }
>  > +        if (hw->bus_type == e1000_bus_type_pcix)
>  > +		e1000_pcix_set_mmrbc(hw, 2048);
> 
> This changes the behavior of the driver.  The existing driver only
> sets MMRBC if it's bigger than min(2048, value in the status register).
> You're setting MMRBC to 2048 even if it starts out at a smaller value.
> 
>  - R.

Hmm.. looks like all that code should really be moved off to PCI bus
quirk/setup.  None of it is E1000 specific.  Something like this (untested):

---
 drivers/pci/quirks.c |   30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

--- pci-x.orig/drivers/pci/quirks.c
+++ pci-x/drivers/pci/quirks.c
@@ -1719,6 +1719,36 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NV
 #endif /* CONFIG_PCI_MSI */
 
 EXPORT_SYMBOL(pcie_mch_quirk);
+
+/* Check that BIOS has not set PCI-X MMRBC to value bigger than board allows */
+static void __devinit quirk_pcix_mmrbc(struct pci_dev *dev)
+{
+	int cap;
+	u32 stat;
+	u16 cmd, m, c;
+
+	cap = pci_find_capability(dev, PCI_CAP_ID_PCIX);
+	if (cap <= 0)
+		return;
+
+	if (pci_read_config_dword(dev, cap + PCI_X_STATUS, &stat) ||
+	    pci_read_config_word(dev, cap + PCI_X_CMD, &cmd))
+		return;
+
+	m = (stat & PCI_X_STATUS_MAX_READ) >> 21;	/* max possible MRRBC*/
+	c = (cmd & PCI_X_CMD_MAX_READ) >> 2;		/* current MMRBC */
+	if (c > m) {
+		printk(KERN_INFO "PCIX: %s resetting MMRBC to %d\n",
+		       pci_name(dev), 512 << m);
+
+		cmd &= ~PCI_X_CMD_MAX_READ;
+		cmd |= m << 2;
+		pci_write_config_dword(dev, cap + PCI_X_CMD, cmd);
+	}
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, quirk_pcix_mmrbc);
+
+
 #ifdef CONFIG_HOTPLUG
 EXPORT_SYMBOL(pci_fixup_device);
 #endif


-- 
Stephen Hemminger <shemminger@osdl.org>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/6] PCI-X Max Read Byte Count interface (v2)
  2006-12-08 18:22 ` [PATCH 1/6] PCI-X Max Read Byte Count interface Stephen Hemminger
@ 2006-12-08 22:56   ` Stephen Hemminger
  0 siblings, 0 replies; 20+ messages in thread
From: Stephen Hemminger @ 2006-12-08 22:56 UTC (permalink / raw)
  To: linux-kernel

Add interface to allow setting PCI-X Max Memory Read Byte Count
and handle quirks.

The AMD 8131 chipset has a number of errata's related to PCI-X
configuration. The BIOS sets initial value correctly, but if a
device changes MMRBC it could cause data corruption. There are
some more settings of MMRBC that could be allowed but 
determining the safe values is more effort than it is worth.


Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
---
 drivers/pci/pci.c    |   79 +++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/quirks.c |   17 ++++++++++
 include/linux/pci.h  |    5 ++-
 3 files changed, 100 insertions(+), 1 deletion(-)

--- pci-x.orig/drivers/pci/pci.c
+++ pci-x/drivers/pci/pci.c
@@ -1059,6 +1059,85 @@ pci_set_consistent_dma_mask(struct pci_d
 	return 0;
 }
 #endif
+
+
+/**
+ * pcix_get_mmrbc - get PCI-X maximum memory read byte count
+ * @dev: PCI device to query
+ *
+ * Returns mmrbc: maximum memory read count in bytes
+ *    or appropriate error value.
+ */
+int pcix_get_mmrbc(struct pci_dev *dev)
+{
+	int ret, cap;
+	u16 cmd;
+
+	cap = pci_find_capability(dev, PCI_CAP_ID_PCIX);
+	if (!cap)
+		return -EINVAL;
+
+	ret = pci_read_config_word(dev, cap + PCI_X_CMD, &cmd);
+	if (!ret)
+		ret = 512 << ((cmd & PCI_X_CMD_MAX_READ) >> 2);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pcix_get_mmrbc);
+
+/**
+ * pcix_set_mmrbc - set PCI-X maximum memory read byte count
+ * @dev: PCI device to query
+ * @mmrbc: maximum memory read count in bytes
+ *    valid values are 512, 1024, 2048, 4096
+ *
+ * If possible sets maximum memory read byte count, some bridges have erratas
+ * that prevent this.
+ */
+int
+pcix_set_mmrbc(struct pci_dev *dev, int mmrbc)
+{
+	int cap, err = -EINVAL;
+	u32 stat;
+	u16 cmd, v, o;
+
+	if (mmrbc < 512 || mmrbc > 4096 || (mmrbc & (mmrbc-1)))
+		goto out;
+
+	v = ffs(mmrbc) - 10;
+
+	cap = pci_find_capability(dev, PCI_CAP_ID_PCIX);
+	if (!cap)
+		goto out;	/* not PCI-X */
+
+	err = pci_read_config_dword(dev, cap + PCI_X_STATUS, &stat);
+	if (err)
+		goto out;
+
+	if (v > (stat & PCI_X_STATUS_MAX_READ) >> 21)
+		return -E2BIG;
+
+	err = pci_read_config_word(dev, cap + PCI_X_CMD, &cmd);
+	if (err)
+		goto out;
+
+	o = (cmd & PCI_X_CMD_MAX_READ) >> 2;
+	if (o != v) {
+		/* Increasing can cause problems on some broken bridges */
+		if (v > o && dev->bus &&
+		    (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_MMRBC))
+			return -EIO;
+
+		cmd &= ~PCI_X_CMD_MAX_READ;
+		cmd |= v << 2;
+		err = pci_write_config_word(dev, cap + PCI_X_CMD, cmd);
+	}
+
+out:
+	return err;
+}
+EXPORT_SYMBOL_GPL(pcix_set_mmrbc);
+
      
 static int __devinit pci_init(void)
 {
--- pci-x.orig/drivers/pci/quirks.c
+++ pci-x/drivers/pci/quirks.c
@@ -618,6 +618,23 @@ static void __init quirk_amd_8131_ioapic
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_8131_BRIDGE, quirk_amd_8131_ioapic);
 #endif /* CONFIG_X86_IO_APIC */
 
+/*
+ * Some settings of MMRBC can lead to data corruption so block changes.
+ * See AMD 8131 HyperTransport PCI-X Tunnel Revision Guide
+ */
+static void __init quirk_amd_8131_mmrbc(struct pci_dev *dev)
+{
+	unsigned char revid;
+
+        pci_read_config_byte(dev, PCI_REVISION_ID, &revid);
+        if (dev->subordinate && revid <= 0x12) {
+                printk(KERN_INFO "AMD8131 rev %x detected, disabling PCI-X MMRBC\n",
+		       revid);
+		dev->subordinate->bus_flags |= PCI_BUS_FLAGS_NO_MMRBC;
+        }
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_8131_BRIDGE, quirk_amd_8131_mmrbc);
+
 
 /*
  * FIXME: it is questionable that quirk_via_acpi
--- pci-x.orig/include/linux/pci.h
+++ pci-x/include/linux/pci.h
@@ -98,7 +98,8 @@ enum pci_channel_state {
 
 typedef unsigned short __bitwise pci_bus_flags_t;
 enum pci_bus_flags {
-	PCI_BUS_FLAGS_NO_MSI = (__force pci_bus_flags_t) 1,
+	PCI_BUS_FLAGS_NO_MSI   = (__force pci_bus_flags_t) 1,
+	PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2,
 };
 
 struct pci_cap_saved_state {
@@ -511,6 +512,8 @@ 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);
+int pcix_get_mmrbc(struct pci_dev *dev);
+int pcix_set_mmrbc(struct pci_dev *dev, int mmrbc);
 void pci_update_resource(struct pci_dev *dev, struct resource *res, int resno);
 int __must_check pci_assign_resource(struct pci_dev *dev, int i);
 int __must_check pci_assign_resource_fixed(struct pci_dev *dev, int i);

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/6] e1000: use pcix_set_mmrbc
  2006-12-08 22:43     ` Stephen Hemminger
@ 2006-12-08 22:58       ` Auke Kok
  2006-12-08 23:38         ` Jeff Kirsher
  0 siblings, 1 reply; 20+ messages in thread
From: Auke Kok @ 2006-12-08 22:58 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Roland Dreier, Greg Kroah-Hartman, linux-pci, linux-kernel,
	Cramer, Jeb J

Stephen Hemminger wrote:
> On Fri, 08 Dec 2006 13:45:05 -0800
> Roland Dreier <rdreier@cisco.com> wrote:
> 
>>  > -        if (hw->bus_type == e1000_bus_type_pcix) {
>>  > -            e1000_read_pci_cfg(hw, PCIX_COMMAND_REGISTER, &pcix_cmd_word);
>>  > -            e1000_read_pci_cfg(hw, PCIX_STATUS_REGISTER_HI,
>>  > -                &pcix_stat_hi_word);
>>  > -            cmd_mmrbc = (pcix_cmd_word & PCIX_COMMAND_MMRBC_MASK) >>
>>  > -                PCIX_COMMAND_MMRBC_SHIFT;
>>  > -            stat_mmrbc = (pcix_stat_hi_word & PCIX_STATUS_HI_MMRBC_MASK) >>
>>  > -                PCIX_STATUS_HI_MMRBC_SHIFT;
>>  > -            if (stat_mmrbc == PCIX_STATUS_HI_MMRBC_4K)
>>  > -                stat_mmrbc = PCIX_STATUS_HI_MMRBC_2K;
>>  > -            if (cmd_mmrbc > stat_mmrbc) {
>>  > -                pcix_cmd_word &= ~PCIX_COMMAND_MMRBC_MASK;
>>  > -                pcix_cmd_word |= stat_mmrbc << PCIX_COMMAND_MMRBC_SHIFT;
>>  > -                e1000_write_pci_cfg(hw, PCIX_COMMAND_REGISTER,
>>  > -                    &pcix_cmd_word);
>>  > -            }
>>  > -        }
>>  > +        if (hw->bus_type == e1000_bus_type_pcix)
>>  > +		e1000_pcix_set_mmrbc(hw, 2048);
>>
>> This changes the behavior of the driver.  The existing driver only
>> sets MMRBC if it's bigger than min(2048, value in the status register).
>> You're setting MMRBC to 2048 even if it starts out at a smaller value.
>>
>>  - R.
> 
> Hmm.. looks like all that code should really be moved off to PCI bus
> quirk/setup.  None of it is E1000 specific.  Something like this (untested):

This is not true, and I have to NAK the original patch. Part of the code Stephan is 
removing fixes a BUG in one of our *e1000 parts* that has the wrong size.

It would be nice to fix generix pci-x issues qith quirks for platforms but the 
adjustment needs to stay for this specific e1000 case.

Perhaps we can accomodate that specific case so that it is apparent from our code, as is 
not the case right now.

Auke

PS Thanks to Jeb for fishing this out ;)


> 
> ---
>  drivers/pci/quirks.c |   30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> --- pci-x.orig/drivers/pci/quirks.c
> +++ pci-x/drivers/pci/quirks.c
> @@ -1719,6 +1719,36 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NV
>  #endif /* CONFIG_PCI_MSI */
>  
>  EXPORT_SYMBOL(pcie_mch_quirk);
> +
> +/* Check that BIOS has not set PCI-X MMRBC to value bigger than board allows */
> +static void __devinit quirk_pcix_mmrbc(struct pci_dev *dev)
> +{
> +	int cap;
> +	u32 stat;
> +	u16 cmd, m, c;
> +
> +	cap = pci_find_capability(dev, PCI_CAP_ID_PCIX);
> +	if (cap <= 0)
> +		return;
> +
> +	if (pci_read_config_dword(dev, cap + PCI_X_STATUS, &stat) ||
> +	    pci_read_config_word(dev, cap + PCI_X_CMD, &cmd))
> +		return;
> +
> +	m = (stat & PCI_X_STATUS_MAX_READ) >> 21;	/* max possible MRRBC*/
> +	c = (cmd & PCI_X_CMD_MAX_READ) >> 2;		/* current MMRBC */
> +	if (c > m) {
> +		printk(KERN_INFO "PCIX: %s resetting MMRBC to %d\n",
> +		       pci_name(dev), 512 << m);
> +
> +		cmd &= ~PCI_X_CMD_MAX_READ;
> +		cmd |= m << 2;
> +		pci_write_config_dword(dev, cap + PCI_X_CMD, cmd);
> +	}
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, quirk_pcix_mmrbc);
> +
> +
>  #ifdef CONFIG_HOTPLUG
>  EXPORT_SYMBOL(pci_fixup_device);
>  #endif
> 
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/6] e1000: use pcix_set_mmrbc
  2006-12-08 22:58       ` Auke Kok
@ 2006-12-08 23:38         ` Jeff Kirsher
  0 siblings, 0 replies; 20+ messages in thread
From: Jeff Kirsher @ 2006-12-08 23:38 UTC (permalink / raw)
  To: Auke Kok
  Cc: Stephen Hemminger, Roland Dreier, Greg Kroah-Hartman, linux-pci,
	linux-kernel, Cramer, Jeb J

On 12/8/06, Auke Kok <auke-jan.h.kok@intel.com> wrote:
> Stephen Hemminger wrote:
> > On Fri, 08 Dec 2006 13:45:05 -0800
> >
> > Hmm.. looks like all that code should really be moved off to PCI bus
> > quirk/setup.  None of it is E1000 specific.  Something like this (untested):
>
> This is not true, and I have to NAK the original patch. Part of the code Stephan is
> removing fixes a BUG in one of our *e1000 parts* that has the wrong size.
>
> It would be nice to fix generix pci-x issues qith quirks for platforms but the
> adjustment needs to stay for this specific e1000 case.
>
> Perhaps we can accomodate that specific case so that it is apparent from our code, as is
> not the case right now.
>
> Auke
>
> PS Thanks to Jeb for fishing this out ;)
>

Actually there are two issues that are being resolved with this function:
1- BIOS reports incorrect maximum memory read byte count (mmrbc).
This was seen on some older systems > 5 years ago.

2- EEPROM is reporting an incorrect mmrbc.

This function corrects both of these issues, Stephen second suggestion
of moving the BIOS fix to quirks.c is fine with me.  Even with the
code added to quirks.c, we still need this workaround as is to correct
for EEPROM's reporting 4k for a mmrbc.  So I am fine with Stephen's
second suggestion NAK the suggested change to e1000_hw.c

-- 
Cheers,
Jeff

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/6] PCI-X/PCI-Express read control interfaces
  2006-12-08 18:22 [PATCH 0/6] PCI-X/PCI-Express read control interfaces Stephen Hemminger
                   ` (5 preceding siblings ...)
  2006-12-08 18:22 ` [PATCH 6/6] PCI-X relaxed ordering interface Stephen Hemminger
@ 2006-12-11  3:48 ` Benjamin Herrenschmidt
  2006-12-14  0:17   ` Stephen Hemminger
  6 siblings, 1 reply; 20+ messages in thread
From: Benjamin Herrenschmidt @ 2006-12-11  3:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Greg Kroah-Hartman, linux-pci, linux-kernel

On Fri, 2006-12-08 at 10:22 -0800, Stephen Hemminger wrote:
> This patch set adds hooks to set PCI-X max read request size
> and PCI-Express read request size. It is important that this be a PCI
> subsystem function rather than a per device hack. That way, the PCI
> system quirks can be used if needed.

Excellent, I've been needing that to work around bogus firmwares...

Ben.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/6] MTHCA driver (infiniband) use new pci interfaces
  2006-12-08 18:22 ` [PATCH 4/6] MTHCA driver (infiniband) use new pci interfaces Stephen Hemminger
  2006-12-08 21:46   ` Roland Dreier
@ 2006-12-11  3:55   ` Benjamin Herrenschmidt
  2006-12-11  5:56     ` Grant Grundler
  2006-12-12  1:38     ` Roland Dreier
  1 sibling, 2 replies; 20+ messages in thread
From: Benjamin Herrenschmidt @ 2006-12-11  3:55 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Greg Kroah-Hartman, linux-pci, linux-kernel

On Fri, 2006-12-08 at 10:22 -0800, Stephen Hemminger wrote:
> plain text document attachment (mthca-rbc.patch)
> Use new pci interfaces to set read request tuning values
> Untested because of lack of hardware.

I'm worried by this... At no point do you check the host bridge
capabilities, and thus will happily set the max read req size to some
value larger than the max the host bridge can cope...

I've been having exactly that problem on a number of setups, for
example, the sky2 cards tend to start with a value of 512 while the G5's
host bridge can't cope with more than 256 (iirc). The firmware fixes
that up properly on the G5 at least (but not on all machines), but if
you allow drivers to go tweak the value without a way to go check what
are the host bridge capabilities, you are toast.

Of course, on PCI-X, this is moot, there is no clear definition on how
to get to a host bridge config space (if any), but on PCI-E, we should
be more careful.

So for PCI-X, if we want tat, we need a pcibios hook for the platform
to validate the size requested. For PCI-E, we can use standard code to
look for the root complex (and bridges on the path to it) and get the
proper max value.

Ben.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/6] MTHCA driver (infiniband) use new pci interfaces
  2006-12-11  3:55   ` Benjamin Herrenschmidt
@ 2006-12-11  5:56     ` Grant Grundler
  2006-12-12  1:38     ` Roland Dreier
  1 sibling, 0 replies; 20+ messages in thread
From: Grant Grundler @ 2006-12-11  5:56 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Stephen Hemminger, Greg Kroah-Hartman, linux-pci, linux-kernel

On Mon, Dec 11, 2006 at 02:55:39PM +1100, Benjamin Herrenschmidt wrote:
> On Fri, 2006-12-08 at 10:22 -0800, Stephen Hemminger wrote:
> > plain text document attachment (mthca-rbc.patch)
> > Use new pci interfaces to set read request tuning values
> > Untested because of lack of hardware.

Sorry...I missed that. I have mthca HW on publicly available IA64 machines.
I'll contact Steve off list to check if he is interested/time.

> I'm worried by this... At no point do you check the host bridge
> capabilities, and thus will happily set the max read req size to some
> value larger than the max the host bridge can cope...
> 
> I've been having exactly that problem on a number of setups, for
> example, the sky2 cards tend to start with a value of 512 while the G5's
> host bridge can't cope with more than 256 (iirc). The firmware fixes
> that up properly on the G5 at least (but not on all machines), but if
> you allow drivers to go tweak the value without a way to go check what
> are the host bridge capabilities, you are toast.
> 
> Of course, on PCI-X, this is moot, there is no clear definition on how
> to get to a host bridge config space (if any)
...
> So for PCI-X, if we want tat, we need a pcibios hook for the platform
> to validate the size requested.

Yes, agreed. 

grant

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/6] MTHCA driver (infiniband) use new pci interfaces
  2006-12-11  3:55   ` Benjamin Herrenschmidt
  2006-12-11  5:56     ` Grant Grundler
@ 2006-12-12  1:38     ` Roland Dreier
  2006-12-12  1:59       ` Benjamin Herrenschmidt
  1 sibling, 1 reply; 20+ messages in thread
From: Roland Dreier @ 2006-12-12  1:38 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Stephen Hemminger, Greg Kroah-Hartman, linux-pci, linux-kernel

 > I'm worried by this... At no point do you check the host bridge
 > capabilities, and thus will happily set the max read req size to some
 > value larger than the max the host bridge can cope...

Well, it's disabled by default... the option is there as a quick way
to fix "why is my bandwidth so low" when a broken BIOS sets these to
minimum values.  Maybe we should just strip out that code and point
people who want to tweak this at setpci instead.

 > So for PCI-X, if we want tat, we need a pcibios hook for the platform
 > to validate the size requested. For PCI-E, we can use standard code to
 > look for the root complex (and bridges on the path to it) and get the
 > proper max value.

Actually even PCIe might not be that easy.  For example with current
kernels on PowerPC 440SPe (SoC with PCIe), I just get:

    # lspci
    00:01.0 InfiniBand: Mellanox Technology: Unknown device 6274 (rev a0)

ie no host bridge / root complex.

 - R.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/6] MTHCA driver (infiniband) use new pci interfaces
  2006-12-12  1:38     ` Roland Dreier
@ 2006-12-12  1:59       ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 20+ messages in thread
From: Benjamin Herrenschmidt @ 2006-12-12  1:59 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Stephen Hemminger, Greg Kroah-Hartman, linux-pci, linux-kernel


> Actually even PCIe might not be that easy.  For example with current
> kernels on PowerPC 440SPe (SoC with PCIe), I just get:
> 
>     # lspci
>     00:01.0 InfiniBand: Mellanox Technology: Unknown device 6274 (rev a0)
> 
> ie no host bridge / root complex.

Did somebody used the spec as toilet paper again ? Or is it just the
kernel that isn't properly showing the root complex ? 
 
Ben.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/6] PCI-X/PCI-Express read control interfaces
  2006-12-11  3:48 ` [PATCH 0/6] PCI-X/PCI-Express read control interfaces Benjamin Herrenschmidt
@ 2006-12-14  0:17   ` Stephen Hemminger
  2006-12-14  0:34     ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 20+ messages in thread
From: Stephen Hemminger @ 2006-12-14  0:17 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Greg Kroah-Hartman, linux-pci, linux-kernel

On Mon, 11 Dec 2006 14:48:32 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Fri, 2006-12-08 at 10:22 -0800, Stephen Hemminger wrote:
> > This patch set adds hooks to set PCI-X max read request size
> > and PCI-Express read request size. It is important that this be a PCI
> > subsystem function rather than a per device hack. That way, the PCI
> > system quirks can be used if needed.
> 
> Excellent, I've been needing that to work around bogus firmwares...
> 
> Ben.
> 
> 

I am thinking in the next revision of these of masking the distinction
between pci-x and pci express and just have:

pci_get_read_count
pci_get_max_read_count
pci_set_read_count



-- 
Stephen Hemminger <shemminger@osdl.org>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/6] PCI-X/PCI-Express read control interfaces
  2006-12-14  0:17   ` Stephen Hemminger
@ 2006-12-14  0:34     ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 20+ messages in thread
From: Benjamin Herrenschmidt @ 2006-12-14  0:34 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Greg Kroah-Hartman, linux-pci, linux-kernel


> I am thinking in the next revision of these of masking the distinction
> between pci-x and pci express and just have:
> 
> pci_get_read_count
> pci_get_max_read_count
> pci_set_read_count

We absolutely need an arch hook though to limit the size. We need to
make sure we don't go over the capabilities of parent bridges,
especially the host bridge. P2P bridges and most PCIe host bridges can
be handled by generic code but pretty much no PCI-X host bridge, they'll
need an arch hook.

Ben.



^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2006-12-14  0:34 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-12-08 18:22 [PATCH 0/6] PCI-X/PCI-Express read control interfaces Stephen Hemminger
2006-12-08 18:22 ` [PATCH 1/6] PCI-X Max Read Byte Count interface Stephen Hemminger
2006-12-08 22:56   ` [PATCH 1/6] PCI-X Max Read Byte Count interface (v2) Stephen Hemminger
2006-12-08 18:22 ` [PATCH 2/6] e1000: use pcix_set_mmrbc Stephen Hemminger
2006-12-08 21:45   ` Roland Dreier
2006-12-08 22:43     ` Stephen Hemminger
2006-12-08 22:58       ` Auke Kok
2006-12-08 23:38         ` Jeff Kirsher
2006-12-08 18:22 ` [PATCH 3/6] PCI Express get/set read request size Stephen Hemminger
2006-12-08 18:22 ` [PATCH 4/6] MTHCA driver (infiniband) use new pci interfaces Stephen Hemminger
2006-12-08 21:46   ` Roland Dreier
2006-12-11  3:55   ` Benjamin Herrenschmidt
2006-12-11  5:56     ` Grant Grundler
2006-12-12  1:38     ` Roland Dreier
2006-12-12  1:59       ` Benjamin Herrenschmidt
2006-12-08 18:22 ` [PATCH 5/6] QLA2 use pci read tuning interface Stephen Hemminger
2006-12-08 18:22 ` [PATCH 6/6] PCI-X relaxed ordering interface Stephen Hemminger
2006-12-11  3:48 ` [PATCH 0/6] PCI-X/PCI-Express read control interfaces Benjamin Herrenschmidt
2006-12-14  0:17   ` Stephen Hemminger
2006-12-14  0:34     ` Benjamin Herrenschmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).