[PATCH 0/6] PCI-X/PCI-Express read control interfaces

[PATCH 0/6] PCI-X/PCI-Express read control interfaces

[Stephen Hemminger]

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.

-- 

[PATCH 1/6] PCI-X Max Read Byte Count interface

[Stephen Hemminger]

[-- 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);

-- 

[PATCH 1/6] PCI-X Max Read Byte Count interface (v2)

[Stephen Hemminger]

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);

[PATCH 2/6] e1000: use pcix_set_mmrbc

[Stephen Hemminger]

[-- 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);

-- 

Re: [PATCH 2/6] e1000: use pcix_set_mmrbc

[Roland Dreier]

 > -        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.

Re: [PATCH 2/6] e1000: use pcix_set_mmrbc

[Stephen Hemminger]

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>

Re: [PATCH 2/6] e1000: use pcix_set_mmrbc

[Auke Kok]

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
> 
> 

Re: [PATCH 2/6] e1000: use pcix_set_mmrbc

[Jeff Kirsher]

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

[PATCH 3/6] PCI Express get/set read request size

[Stephen Hemminger]

[-- 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);

-- 

[PATCH 4/6] MTHCA driver (infiniband) use new pci interfaces

[Stephen Hemminger]

[-- 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;
 }

-- 

Re: [PATCH 4/6] MTHCA driver (infiniband) use new pci interfaces

[Roland Dreier]

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

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

Re: [PATCH 4/6] MTHCA driver (infiniband) use new pci interfaces

[Benjamin Herrenschmidt]

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.

Re: [PATCH 4/6] MTHCA driver (infiniband) use new pci interfaces

[Grant Grundler]

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

Re: [PATCH 4/6] MTHCA driver (infiniband) use new pci interfaces

[Roland Dreier]

 > 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.

Re: [PATCH 4/6] MTHCA driver (infiniband) use new pci interfaces

[Benjamin Herrenschmidt]

> 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.

[PATCH 5/6] QLA2 use pci read tuning interface

[Stephen Hemminger]

[-- 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);

-- 

[PATCH 6/6] PCI-X relaxed ordering interface

[Stephen Hemminger]

[-- 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);

-- 

Re: [PATCH 0/6] PCI-X/PCI-Express read control interfaces

[Benjamin Herrenschmidt]

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.

Re: [PATCH 0/6] PCI-X/PCI-Express read control interfaces

[Stephen Hemminger]

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>

Re: [PATCH 0/6] PCI-X/PCI-Express read control interfaces

[Benjamin Herrenschmidt]

> 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.