All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] pci: determine CLS more intelligently
@ 2009-07-04  5:08 Tejun Heo
  2009-07-04  5:09 ` [PATCH 2/3] pci,sparc64: drop PCI_CACHE_LINE_BYTES Tejun Heo
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Tejun Heo @ 2009-07-04  5:08 UTC (permalink / raw)
  To: Greg KH, Robert Hancock, Alan Cox, linux-pci, Linux Kernel,
	Daniel Ritz, Dominik Brodowski, Kenji Kaneshige, Axel Birndt,
	Benjamin Herrenschmidt, Ingo Molnar, Thomas Gleixner, Tony Luck,
	David Miller, Ingo Molnar

Till now, CLS has been determined either by arch code or as
L1_CACHE_BYTES.  Only x86 and ia64 set CLS explicitly and x86 doesn't
always get it right.  On most configurations, the chance is that
firmware configures the correct value during boot.

This patch makes pci_init() determine CLS by looking at what firmware
has configured.  It scans all devices and if all non-zero values
agree, the value is used.  If none is configured or there is a
disagreement, pci_dfl_cache_line_size is used.  arch can set the dfl
value (via PCI_CACHE_LINE_BYTES or pci_dfl_cache_line_size) or
override the actual one.

ia64, x86 and sparc64 updated to set the default cls instead of the
actual one.

While at it, declare pci_cache_line_size and pci_dfl_cache_line_size
in pci.h and drop private declarations from arch code.

Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: David Miller <davem@davemloft.net>
Acked-by: Greg KH <gregkh@suse.de>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tony Luck <tony.luck@intel.com>
---
Private declarations dropped.  Greg, please schedule for the next
merge window.  Thanks.

 arch/ia64/pci/pci.c   |    9 +++------
 arch/x86/pci/common.c |    8 +++-----
 drivers/pci/pci.c     |   49 +++++++++++++++++++++++++++++++++++++++++--------
 include/linux/pci.h   |    2 ++
 4 files changed, 49 insertions(+), 19 deletions(-)

Index: work/arch/ia64/pci/pci.c
===================================================================
--- work.orig/arch/ia64/pci/pci.c
+++ work/arch/ia64/pci/pci.c
@@ -715,9 +715,6 @@ int ia64_pci_legacy_write(struct pci_bus
 	return ret;
 }
 
-/* It's defined in drivers/pci/pci.c */
-extern u8 pci_cache_line_size;
-
 /**
  * set_pci_cacheline_size - determine cacheline size for PCI devices
  *
@@ -726,7 +723,7 @@ extern u8 pci_cache_line_size;
  *
  * Code mostly taken from arch/ia64/kernel/palinfo.c:cache_info().
  */
-static void __init set_pci_cacheline_size(void)
+static void __init set_pci_dfl_cacheline_size(void)
 {
 	unsigned long levels, unique_caches;
 	long status;
@@ -746,7 +743,7 @@ static void __init set_pci_cacheline_siz
 			"(status=%ld)\n", __func__, status);
 		return;
 	}
-	pci_cache_line_size = (1 << cci.pcci_line_size) / 4;
+	pci_dfl_cache_line_size = (1 << cci.pcci_line_size) / 4;
 }
 
 u64 ia64_dma_get_required_mask(struct device *dev)
@@ -777,7 +774,7 @@ EXPORT_SYMBOL_GPL(dma_get_required_mask)
 
 static int __init pcibios_init(void)
 {
-	set_pci_cacheline_size();
+	set_pci_dfl_cacheline_size();
 	return 0;
 }
 
Index: work/arch/x86/pci/common.c
===================================================================
--- work.orig/arch/x86/pci/common.c
+++ work/arch/x86/pci/common.c
@@ -410,8 +410,6 @@ struct pci_bus * __devinit pcibios_scan_
 	return bus;
 }
 
-extern u8 pci_cache_line_size;
-
 int __init pcibios_init(void)
 {
 	struct cpuinfo_x86 *c = &boot_cpu_data;
@@ -426,11 +424,11 @@ int __init pcibios_init(void)
 	 * and P4. It's also good for 386/486s (which actually have 16)
 	 * as quite a few PCI devices do not support smaller values.
 	 */
-	pci_cache_line_size = 32 >> 2;
+	pci_dfl_cache_line_size = 32 >> 2;
 	if (c->x86 >= 6 && c->x86_vendor == X86_VENDOR_AMD)
-		pci_cache_line_size = 64 >> 2;	/* K7 & K8 */
+		pci_dfl_cache_line_size = 64 >> 2;	/* K7 & K8 */
 	else if (c->x86 > 6 && c->x86_vendor == X86_VENDOR_INTEL)
-		pci_cache_line_size = 128 >> 2;	/* P4 */
+		pci_dfl_cache_line_size = 128 >> 2;	/* P4 */
 
 	pcibios_resource_survey();
 
Index: work/drivers/pci/pci.c
===================================================================
--- work.orig/drivers/pci/pci.c
+++ work/drivers/pci/pci.c
@@ -41,6 +41,19 @@ int pci_domains_supported = 1;
 unsigned long pci_cardbus_io_size = DEFAULT_CARDBUS_IO_SIZE;
 unsigned long pci_cardbus_mem_size = DEFAULT_CARDBUS_MEM_SIZE;
 
+#ifndef PCI_CACHE_LINE_BYTES
+#define PCI_CACHE_LINE_BYTES L1_CACHE_BYTES
+#endif
+
+/*
+ * The default CLS is used if arch didn't set CLS explicitly and not
+ * all pci devices agree on the same value.  Arch can override either
+ * the dfl or actual value as it sees fit.  Don't forget this is
+ * measured in 32-bit words, not bytes.
+ */
+u8 pci_dfl_cache_line_size __initdata = PCI_CACHE_LINE_BYTES >> 2;
+u8 pci_cache_line_size;
+
 /**
  * pci_bus_max_busnr - returns maximum PCI bus number of given bus' children
  * @bus: pointer to PCI bus structure to search
@@ -1848,14 +1861,6 @@ void pci_clear_mwi(struct pci_dev *dev)
 
 #else
 
-#ifndef PCI_CACHE_LINE_BYTES
-#define PCI_CACHE_LINE_BYTES L1_CACHE_BYTES
-#endif
-
-/* This can be overridden by arch code. */
-/* Don't forget this is measured in 32-bit words, not bytes */
-u8 pci_cache_line_size = PCI_CACHE_LINE_BYTES / 4;
-
 /**
  * pci_set_cacheline_size - ensure the CACHE_LINE_SIZE register is programmed
  * @dev: the PCI device for which MWI is to be enabled
@@ -2631,9 +2636,37 @@ int __attribute__ ((weak)) pci_ext_cfg_a
 static int __devinit pci_init(void)
 {
 	struct pci_dev *dev = NULL;
+	u8 cls = 0;
+	u8 tmp;
+
+	if (pci_cache_line_size)
+		printk(KERN_DEBUG "PCI: CLS %u bytes\n",
+		       pci_cache_line_size << 2);
 
 	while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
 		pci_fixup_device(pci_fixup_final, dev);
+		/*
+		 * If arch hasn't set it explicitly yet, use the CLS
+		 * value shared by all PCI devices.  If there's a
+		 * mismatch, fall back to the default value.
+		 */
+		if (!pci_cache_line_size) {
+			pci_read_config_byte(dev, PCI_CACHE_LINE_SIZE, &tmp);
+			if (!cls)
+				cls = tmp;
+			if (!tmp || cls == tmp)
+				continue;
+
+			printk(KERN_DEBUG "PCI: CLS mismatch (%u != %u), "
+			       "using %u bytes\n", cls << 2, tmp << 2,
+			       pci_dfl_cache_line_size << 2);
+			pci_cache_line_size = pci_dfl_cache_line_size;
+		}
+	}
+	if (!pci_cache_line_size) {
+		printk(KERN_DEBUG "PCI: CLS %u bytes, default %u\n",
+		       cls << 2, pci_dfl_cache_line_size << 2);
+		pci_cache_line_size = cls;
 	}
 
 	return 0;
Index: work/include/linux/pci.h
===================================================================
--- work.orig/include/linux/pci.h
+++ work/include/linux/pci.h
@@ -1235,6 +1235,8 @@ extern int pci_pci_problems;
 
 extern unsigned long pci_cardbus_io_size;
 extern unsigned long pci_cardbus_mem_size;
+extern u8 pci_dfl_cache_line_size;
+extern u8 pci_cache_line_size;
 
 int pcibios_add_platform_entries(struct pci_dev *dev);
 void pcibios_disable_device(struct pci_dev *dev);

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

* [PATCH 2/3] pci,sparc64: drop PCI_CACHE_LINE_BYTES
  2009-07-04  5:08 [PATCH 1/3] pci: determine CLS more intelligently Tejun Heo
@ 2009-07-04  5:09 ` Tejun Heo
  2009-07-04  5:10   ` [PATCH 3/3] pccard: configure CLS on attach Tejun Heo
  2009-07-04  9:29 ` [PATCH 1/3] pci: determine CLS more intelligently Ingo Molnar
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Tejun Heo @ 2009-07-04  5:09 UTC (permalink / raw)
  To: Greg KH, Robert Hancock, Alan Cox, linux-pci, Linux Kernel,
	Daniel Ritz, Dominik Brodowski, Kenji Kaneshige, Axel Birndt,
	Benjamin Herrenschmidt, Ingo Molnar, Thomas Gleixner, Tony Luck,
	David Miller, Ingo Molnar

sparc64 is now the only user of PCI_CACHE_LINE_BYTES.  Drop it and set
pci_dfl_cache_line_size from pcibios_init() instead and drop
PCI_CACHE_LINE_BYTES handling from generic pci code.

Orignally-From: David Miller <davem@davemloft.net>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
 arch/sparc/include/asm/pci_64.h |    2 --
 arch/sparc/kernel/pci.c         |    7 +++++++
 drivers/pci/pci.c               |    6 +-----
 3 files changed, 8 insertions(+), 7 deletions(-)

Index: work/arch/sparc/kernel/pci.c
===================================================================
--- work.orig/arch/sparc/kernel/pci.c
+++ work/arch/sparc/kernel/pci.c
@@ -1081,3 +1081,10 @@ void pci_resource_to_user(const struct p
 	*start = rp->start - offset;
 	*end = rp->end - offset;
 }
+
+static int __init pcibios_init(void)
+{
+	pci_dfl_cache_line_size = 64 >> 2;
+	return 0;
+}
+subsys_initcall(pcibios_init);
Index: work/arch/sparc/include/asm/pci_64.h
===================================================================
--- work.orig/arch/sparc/include/asm/pci_64.h
+++ work/arch/sparc/include/asm/pci_64.h
@@ -17,8 +17,6 @@
 
 #define PCI_IRQ_NONE		0xffffffff
 
-#define PCI_CACHE_LINE_BYTES	64
-
 static inline void pcibios_set_master(struct pci_dev *dev)
 {
 	/* No special bus mastering setup handling */
Index: work/drivers/pci/pci.c
===================================================================
--- work.orig/drivers/pci/pci.c
+++ work/drivers/pci/pci.c
@@ -41,17 +41,13 @@ int pci_domains_supported = 1;
 unsigned long pci_cardbus_io_size = DEFAULT_CARDBUS_IO_SIZE;
 unsigned long pci_cardbus_mem_size = DEFAULT_CARDBUS_MEM_SIZE;
 
-#ifndef PCI_CACHE_LINE_BYTES
-#define PCI_CACHE_LINE_BYTES L1_CACHE_BYTES
-#endif
-
 /*
  * The default CLS is used if arch didn't set CLS explicitly and not
  * all pci devices agree on the same value.  Arch can override either
  * the dfl or actual value as it sees fit.  Don't forget this is
  * measured in 32-bit words, not bytes.
  */
-u8 pci_dfl_cache_line_size __initdata = PCI_CACHE_LINE_BYTES >> 2;
+u8 pci_dfl_cache_line_size __initdata = L1_CACHE_BYTES >> 2;
 u8 pci_cache_line_size;
 
 /**

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

* [PATCH 3/3] pccard: configure CLS on attach
  2009-07-04  5:09 ` [PATCH 2/3] pci,sparc64: drop PCI_CACHE_LINE_BYTES Tejun Heo
@ 2009-07-04  5:10   ` Tejun Heo
  0 siblings, 0 replies; 14+ messages in thread
From: Tejun Heo @ 2009-07-04  5:10 UTC (permalink / raw)
  To: Greg KH, Robert Hancock, Alan Cox, linux-pci, Linux Kernel,
	Daniel Ritz, Dominik Brodowski, Kenji Kaneshige, Axel Birndt,
	Benjamin Herrenschmidt, Ingo Molnar, Thomas Gleixner, Tony Luck,
	David Miller, Ingo Molnar

For non hotplug PCI devices, the system firmware usually configures
CLS correctly.  For pccard devices system firmware can't do it and
Linux PCI layer doesn't do it either.  Unfortunately this leads to
poor performance for certain devices (sata_sil).  Unless MWI, which
requires separate configuration, is to be used, CLS doesn't affect
correctness, so the configuration should be harmless.

This patch makes pci_set_cacheline_size() always built and export it
and make pccard call it during attach.

Please note that some other PCI hotplug drivers (shpchp and pciehp)
also configure CLS on hotplug.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Daniel Ritz <daniel.ritz@gmx.ch>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Cc: Greg KH <greg@kroah.com>
Cc: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
Cc: Axel Birndt <towerlexa@gmx.de>
---
 drivers/pci/pci.c        |   39 +++++++++++++++++++--------------------
 drivers/pcmcia/cardbus.c |   23 +++++++++++++++--------
 include/linux/pci.h      |    1 +
 3 files changed, 35 insertions(+), 28 deletions(-)

Index: work/drivers/pcmcia/cardbus.c
===================================================================
--- work.orig/drivers/pcmcia/cardbus.c
+++ work/drivers/pcmcia/cardbus.c
@@ -184,26 +184,33 @@ fail:
     
 =====================================================================*/
 
-/*
- * Since there is only one interrupt available to CardBus
- * devices, all devices downstream of this device must
- * be using this IRQ.
- */
-static void cardbus_assign_irqs(struct pci_bus *bus, int irq)
+static void cardbus_config_irq_and_cls(struct pci_bus *bus, int irq)
 {
 	struct pci_dev *dev;
 
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		u8 irq_pin;
 
+		/*
+		 * Since there is only one interrupt available to
+		 * CardBus devices, all devices downstream of this
+		 * device must be using this IRQ.
+		 */
 		pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &irq_pin);
 		if (irq_pin) {
 			dev->irq = irq;
 			pci_write_config_byte(dev, PCI_INTERRUPT_LINE, dev->irq);
 		}
 
+		/*
+		 * Some controllers transfer very slowly with 0 CLS.
+		 * Configure it.  This may fail as CLS configuration
+		 * is mandatory only for MWI.
+		 */
+		pci_set_cacheline_size(dev);
+
 		if (dev->subordinate)
-			cardbus_assign_irqs(dev->subordinate, irq);
+			cardbus_config_irq_and_cls(dev->subordinate, irq);
 	}
 }
 
@@ -228,7 +235,7 @@ int __ref cb_alloc(struct pcmcia_socket
 	 */
 	pci_bus_size_bridges(bus);
 	pci_bus_assign_resources(bus);
-	cardbus_assign_irqs(bus, s->pci_irq);
+	cardbus_config_irq_and_cls(bus, s->pci_irq);
 
 	/* socket specific tune function */
 	if (s->tune_bridge)
Index: work/drivers/pci/pci.c
===================================================================
--- work.orig/drivers/pci/pci.c
+++ work/drivers/pci/pci.c
@@ -1840,23 +1840,6 @@ void pci_clear_master(struct pci_dev *de
 	__pci_set_master(dev, false);
 }
 
-#ifdef PCI_DISABLE_MWI
-int pci_set_mwi(struct pci_dev *dev)
-{
-	return 0;
-}
-
-int pci_try_set_mwi(struct pci_dev *dev)
-{
-	return 0;
-}
-
-void pci_clear_mwi(struct pci_dev *dev)
-{
-}
-
-#else
-
 /**
  * pci_set_cacheline_size - ensure the CACHE_LINE_SIZE register is programmed
  * @dev: the PCI device for which MWI is to be enabled
@@ -1867,13 +1850,12 @@ void pci_clear_mwi(struct pci_dev *dev)
  *
  * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
  */
-static int
-pci_set_cacheline_size(struct pci_dev *dev)
+int pci_set_cacheline_size(struct pci_dev *dev)
 {
 	u8 cacheline_size;
 
 	if (!pci_cache_line_size)
-		return -EINVAL;		/* The system doesn't support MWI. */
+		return -EINVAL;
 
 	/* Validate current setting: the PCI_CACHE_LINE_SIZE must be
 	   equal to or multiple of the right value. */
@@ -1895,6 +1877,23 @@ pci_set_cacheline_size(struct pci_dev *d
 	return -EINVAL;
 }
 
+#ifdef PCI_DISABLE_MWI
+int pci_set_mwi(struct pci_dev *dev)
+{
+	return 0;
+}
+
+int pci_try_set_mwi(struct pci_dev *dev)
+{
+	return 0;
+}
+
+void pci_clear_mwi(struct pci_dev *dev)
+{
+}
+
+#else
+
 /**
  * pci_set_mwi - enables memory-write-invalidate PCI transaction
  * @dev: the PCI device for which MWI is enabled
Index: work/include/linux/pci.h
===================================================================
--- work.orig/include/linux/pci.h
+++ work/include/linux/pci.h
@@ -697,6 +697,7 @@ void pci_disable_device(struct pci_dev *
 void pci_set_master(struct pci_dev *dev);
 void pci_clear_master(struct pci_dev *dev);
 int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state);
+int pci_set_cacheline_size(struct pci_dev *dev);
 #define HAVE_PCI_SET_MWI
 int __must_check pci_set_mwi(struct pci_dev *dev);
 int pci_try_set_mwi(struct pci_dev *dev);

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

* Re: [PATCH 1/3] pci: determine CLS more intelligently
  2009-07-04  5:08 [PATCH 1/3] pci: determine CLS more intelligently Tejun Heo
  2009-07-04  5:09 ` [PATCH 2/3] pci,sparc64: drop PCI_CACHE_LINE_BYTES Tejun Heo
@ 2009-07-04  9:29 ` Ingo Molnar
  2009-09-17 17:31 ` Jesse Barnes
  2009-09-28 17:52 ` [PATCH 1/3] pci: determine CLS more intelligently Jesse Barnes
  3 siblings, 0 replies; 14+ messages in thread
From: Ingo Molnar @ 2009-07-04  9:29 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Greg KH, Robert Hancock, Alan Cox, linux-pci, Linux Kernel,
	Daniel Ritz, Dominik Brodowski, Kenji Kaneshige, Axel Birndt,
	Benjamin Herrenschmidt, Thomas Gleixner, Tony Luck, David Miller


* Tejun Heo <tj@kernel.org> wrote:

> Till now, CLS has been determined either by arch code or as 
> L1_CACHE_BYTES.  Only x86 and ia64 set CLS explicitly and x86 
> doesn't always get it right.  On most configurations, the chance 
> is that firmware configures the correct value during boot.
> 
> This patch makes pci_init() determine CLS by looking at what 
> firmware has configured.  It scans all devices and if all non-zero 
> values agree, the value is used.  If none is configured or there 
> is a disagreement, pci_dfl_cache_line_size is used.  arch can set 
> the dfl value (via PCI_CACHE_LINE_BYTES or 
> pci_dfl_cache_line_size) or override the actual one.
> 
> ia64, x86 and sparc64 updated to set the default cls instead of 
> the actual one.
> 
> While at it, declare pci_cache_line_size and 
> pci_dfl_cache_line_size in pci.h and drop private declarations 
> from arch code.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Acked-by: David Miller <davem@davemloft.net>
> Acked-by: Greg KH <gregkh@suse.de>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Tony Luck <tony.luck@intel.com>

The principle looks good to me. Regressions could be expected though 
- these details are fragile and affect the way how we talk to 
hardware.

Acked-by: Ingo Molnar <mingo@elte.hu>

	Ingo

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

* Re: [PATCH 1/3] pci: determine CLS more intelligently
  2009-07-04  5:08 [PATCH 1/3] pci: determine CLS more intelligently Tejun Heo
  2009-07-04  5:09 ` [PATCH 2/3] pci,sparc64: drop PCI_CACHE_LINE_BYTES Tejun Heo
  2009-07-04  9:29 ` [PATCH 1/3] pci: determine CLS more intelligently Ingo Molnar
@ 2009-09-17 17:31 ` Jesse Barnes
  2009-09-22  8:33   ` [PATCH 1/3 pci#linux-next] " Tejun Heo
                     ` (2 more replies)
  2009-09-28 17:52 ` [PATCH 1/3] pci: determine CLS more intelligently Jesse Barnes
  3 siblings, 3 replies; 14+ messages in thread
From: Jesse Barnes @ 2009-09-17 17:31 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Greg KH, Robert Hancock, Alan Cox, linux-pci, Linux Kernel,
	Daniel Ritz, Dominik Brodowski, Kenji Kaneshige, Axel Birndt,
	Benjamin Herrenschmidt, Ingo Molnar, Thomas Gleixner, Tony Luck,
	David Miller

On Sat, 04 Jul 2009 14:08:57 +0900
Tejun Heo <tj@kernel.org> wrote:

> Till now, CLS has been determined either by arch code or as
> L1_CACHE_BYTES.  Only x86 and ia64 set CLS explicitly and x86 doesn't
> always get it right.  On most configurations, the chance is that
> firmware configures the correct value during boot.
> 
> This patch makes pci_init() determine CLS by looking at what firmware
> has configured.  It scans all devices and if all non-zero values
> agree, the value is used.  If none is configured or there is a
> disagreement, pci_dfl_cache_line_size is used.  arch can set the dfl
> value (via PCI_CACHE_LINE_BYTES or pci_dfl_cache_line_size) or
> override the actual one.
> 
> ia64, x86 and sparc64 updated to set the default cls instead of the
> actual one.
> 
> While at it, declare pci_cache_line_size and pci_dfl_cache_line_size
> in pci.h and drop private declarations from arch code.

This sounds like a good improvement (though I share Ingo's concerns
about platform breakage here).  Can you respin the patchset against my
linux-next current tree?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

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

* [PATCH 1/3 pci#linux-next] pci: determine CLS more intelligently
  2009-09-17 17:31 ` Jesse Barnes
@ 2009-09-22  8:33   ` Tejun Heo
  2009-09-23  5:24     ` Benjamin Herrenschmidt
  2009-10-06 16:53     ` Jesse Barnes
  2009-09-22  8:34   ` [PATCH 2/3 pci#linux-next] pci,sparc64: drop PCI_CACHE_LINE_BYTES Tejun Heo
  2009-09-22  8:34   ` [PATCH 3/3 pci#linux-next] pccard: configure CLS on attach Tejun Heo
  2 siblings, 2 replies; 14+ messages in thread
From: Tejun Heo @ 2009-09-22  8:33 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: Greg KH, Robert Hancock, Alan Cox, linux-pci, Linux Kernel,
	Daniel Ritz, Dominik Brodowski, Kenji Kaneshige, Axel Birndt,
	Benjamin Herrenschmidt, Ingo Molnar, Thomas Gleixner, Tony Luck,
	David Miller

Till now, CLS has been determined either by arch code or as
L1_CACHE_BYTES.  Only x86 and ia64 set CLS explicitly and x86 doesn't
always get it right.  On most configurations, the chance is that
firmware configures the correct value during boot.

This patch makes pci_init() determine CLS by looking at what firmware
has configured.  It scans all devices and if all non-zero values
agree, the value is used.  If none is configured or there is a
disagreement, pci_dfl_cache_line_size is used.  arch can set the dfl
value (via PCI_CACHE_LINE_BYTES or pci_dfl_cache_line_size) or
override the actual one.

ia64, x86 and sparc64 updated to set the default cls instead of the
actual one.

While at it, declare pci_cache_line_size and pci_dfl_cache_line_size
in pci.h and drop private declarations from arch code.

Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: David Miller <davem@davemloft.net>
Acked-by: Greg KH <gregkh@suse.de>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tony Luck <tony.luck@intel.com>
---
Sorry about the delay.  Here are the regenerated patches.  They are
against linux-next branch of pci-2.6.git tree as of Sept 22nd
(76baeebf7df493703eeb4428eac015bdb7fabda6).  Thanks.

 arch/ia64/pci/pci.c   |    9 +++------
 arch/x86/pci/common.c |    8 +++-----
 drivers/pci/pci.c     |   49 +++++++++++++++++++++++++++++++++++++++++--------
 include/linux/pci.h   |    2 ++
 4 files changed, 49 insertions(+), 19 deletions(-)

Index: work/arch/ia64/pci/pci.c
===================================================================
--- work.orig/arch/ia64/pci/pci.c
+++ work/arch/ia64/pci/pci.c
@@ -715,9 +715,6 @@ int ia64_pci_legacy_write(struct pci_bus
 	return ret;
 }

-/* It's defined in drivers/pci/pci.c */
-extern u8 pci_cache_line_size;
-
 /**
  * set_pci_cacheline_size - determine cacheline size for PCI devices
  *
@@ -726,7 +723,7 @@ extern u8 pci_cache_line_size;
  *
  * Code mostly taken from arch/ia64/kernel/palinfo.c:cache_info().
  */
-static void __init set_pci_cacheline_size(void)
+static void __init set_pci_dfl_cacheline_size(void)
 {
 	unsigned long levels, unique_caches;
 	long status;
@@ -746,7 +743,7 @@ static void __init set_pci_cacheline_siz
 			"(status=%ld)\n", __func__, status);
 		return;
 	}
-	pci_cache_line_size = (1 << cci.pcci_line_size) / 4;
+	pci_dfl_cache_line_size = (1 << cci.pcci_line_size) / 4;
 }

 u64 ia64_dma_get_required_mask(struct device *dev)
@@ -777,7 +774,7 @@ EXPORT_SYMBOL_GPL(dma_get_required_mask)

 static int __init pcibios_init(void)
 {
-	set_pci_cacheline_size();
+	set_pci_dfl_cacheline_size();
 	return 0;
 }

Index: work/arch/x86/pci/common.c
===================================================================
--- work.orig/arch/x86/pci/common.c
+++ work/arch/x86/pci/common.c
@@ -410,8 +410,6 @@ struct pci_bus * __devinit pcibios_scan_
 	return bus;
 }

-extern u8 pci_cache_line_size;
-
 int __init pcibios_init(void)
 {
 	struct cpuinfo_x86 *c = &boot_cpu_data;
@@ -426,11 +424,11 @@ int __init pcibios_init(void)
 	 * and P4. It's also good for 386/486s (which actually have 16)
 	 * as quite a few PCI devices do not support smaller values.
 	 */
-	pci_cache_line_size = 32 >> 2;
+	pci_dfl_cache_line_size = 32 >> 2;
 	if (c->x86 >= 6 && c->x86_vendor == X86_VENDOR_AMD)
-		pci_cache_line_size = 64 >> 2;	/* K7 & K8 */
+		pci_dfl_cache_line_size = 64 >> 2;	/* K7 & K8 */
 	else if (c->x86 > 6 && c->x86_vendor == X86_VENDOR_INTEL)
-		pci_cache_line_size = 128 >> 2;	/* P4 */
+		pci_dfl_cache_line_size = 128 >> 2;	/* P4 */

 	pcibios_resource_survey();

Index: work/drivers/pci/pci.c
===================================================================
--- work.orig/drivers/pci/pci.c
+++ work/drivers/pci/pci.c
@@ -47,6 +47,19 @@ unsigned long pci_cardbus_mem_size = DEF
 unsigned long pci_hotplug_io_size  = DEFAULT_HOTPLUG_IO_SIZE;
 unsigned long pci_hotplug_mem_size = DEFAULT_HOTPLUG_MEM_SIZE;

+#ifndef PCI_CACHE_LINE_BYTES
+#define PCI_CACHE_LINE_BYTES L1_CACHE_BYTES
+#endif
+
+/*
+ * The default CLS is used if arch didn't set CLS explicitly and not
+ * all pci devices agree on the same value.  Arch can override either
+ * the dfl or actual value as it sees fit.  Don't forget this is
+ * measured in 32-bit words, not bytes.
+ */
+u8 pci_dfl_cache_line_size __initdata = PCI_CACHE_LINE_BYTES >> 2;
+u8 pci_cache_line_size;
+
 /**
  * pci_bus_max_busnr - returns maximum PCI bus number of given bus' children
  * @bus: pointer to PCI bus structure to search
@@ -1879,14 +1892,6 @@ void pci_clear_mwi(struct pci_dev *dev)

 #else

-#ifndef PCI_CACHE_LINE_BYTES
-#define PCI_CACHE_LINE_BYTES L1_CACHE_BYTES
-#endif
-
-/* This can be overridden by arch code. */
-/* Don't forget this is measured in 32-bit words, not bytes */
-u8 pci_cache_line_size = PCI_CACHE_LINE_BYTES / 4;
-
 /**
  * pci_set_cacheline_size - ensure the CACHE_LINE_SIZE register is programmed
  * @dev: the PCI device for which MWI is to be enabled
@@ -2722,9 +2727,37 @@ int __attribute__ ((weak)) pci_ext_cfg_a
 static int __devinit pci_init(void)
 {
 	struct pci_dev *dev = NULL;
+	u8 cls = 0;
+	u8 tmp;
+
+	if (pci_cache_line_size)
+		printk(KERN_DEBUG "PCI: CLS %u bytes\n",
+		       pci_cache_line_size << 2);

 	while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
 		pci_fixup_device(pci_fixup_final, dev);
+		/*
+		 * If arch hasn't set it explicitly yet, use the CLS
+		 * value shared by all PCI devices.  If there's a
+		 * mismatch, fall back to the default value.
+		 */
+		if (!pci_cache_line_size) {
+			pci_read_config_byte(dev, PCI_CACHE_LINE_SIZE, &tmp);
+			if (!cls)
+				cls = tmp;
+			if (!tmp || cls == tmp)
+				continue;
+
+			printk(KERN_DEBUG "PCI: CLS mismatch (%u != %u), "
+			       "using %u bytes\n", cls << 2, tmp << 2,
+			       pci_dfl_cache_line_size << 2);
+			pci_cache_line_size = pci_dfl_cache_line_size;
+		}
+	}
+	if (!pci_cache_line_size) {
+		printk(KERN_DEBUG "PCI: CLS %u bytes, default %u\n",
+		       cls << 2, pci_dfl_cache_line_size << 2);
+		pci_cache_line_size = cls;
 	}

 	return 0;
Index: work/include/linux/pci.h
===================================================================
--- work.orig/include/linux/pci.h
+++ work/include/linux/pci.h
@@ -1246,6 +1246,8 @@ extern int pci_pci_problems;

 extern unsigned long pci_cardbus_io_size;
 extern unsigned long pci_cardbus_mem_size;
+extern u8 pci_dfl_cache_line_size;
+extern u8 pci_cache_line_size;

 extern unsigned long pci_hotplug_io_size;
 extern unsigned long pci_hotplug_mem_size;

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

* [PATCH 2/3 pci#linux-next] pci,sparc64: drop PCI_CACHE_LINE_BYTES
  2009-09-17 17:31 ` Jesse Barnes
  2009-09-22  8:33   ` [PATCH 1/3 pci#linux-next] " Tejun Heo
@ 2009-09-22  8:34   ` Tejun Heo
  2009-09-22  8:34   ` [PATCH 3/3 pci#linux-next] pccard: configure CLS on attach Tejun Heo
  2 siblings, 0 replies; 14+ messages in thread
From: Tejun Heo @ 2009-09-22  8:34 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: Greg KH, Robert Hancock, Alan Cox, linux-pci, Linux Kernel,
	Daniel Ritz, Dominik Brodowski, Kenji Kaneshige, Axel Birndt,
	Benjamin Herrenschmidt, Ingo Molnar, Thomas Gleixner, Tony Luck,
	David Miller

sparc64 is now the only user of PCI_CACHE_LINE_BYTES.  Drop it and set
pci_dfl_cache_line_size from pcibios_init() instead and drop
PCI_CACHE_LINE_BYTES handling from generic pci code.

Orignally-From: David Miller <davem@davemloft.net>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
 arch/sparc/include/asm/pci_64.h |    2 --
 arch/sparc/kernel/pci.c         |    7 +++++++
 drivers/pci/pci.c               |    6 +-----
 3 files changed, 8 insertions(+), 7 deletions(-)

Index: work/arch/sparc/kernel/pci.c
===================================================================
--- work.orig/arch/sparc/kernel/pci.c
+++ work/arch/sparc/kernel/pci.c
@@ -1081,3 +1081,10 @@ void pci_resource_to_user(const struct p
 	*start = rp->start - offset;
 	*end = rp->end - offset;
 }
+
+static int __init pcibios_init(void)
+{
+	pci_dfl_cache_line_size = 64 >> 2;
+	return 0;
+}
+subsys_initcall(pcibios_init);
Index: work/arch/sparc/include/asm/pci_64.h
===================================================================
--- work.orig/arch/sparc/include/asm/pci_64.h
+++ work/arch/sparc/include/asm/pci_64.h
@@ -16,8 +16,6 @@

 #define PCI_IRQ_NONE		0xffffffff

-#define PCI_CACHE_LINE_BYTES	64
-
 static inline void pcibios_set_master(struct pci_dev *dev)
 {
 	/* No special bus mastering setup handling */
Index: work/drivers/pci/pci.c
===================================================================
--- work.orig/drivers/pci/pci.c
+++ work/drivers/pci/pci.c
@@ -47,17 +47,13 @@ unsigned long pci_cardbus_mem_size = DEF
 unsigned long pci_hotplug_io_size  = DEFAULT_HOTPLUG_IO_SIZE;
 unsigned long pci_hotplug_mem_size = DEFAULT_HOTPLUG_MEM_SIZE;

-#ifndef PCI_CACHE_LINE_BYTES
-#define PCI_CACHE_LINE_BYTES L1_CACHE_BYTES
-#endif
-
 /*
  * The default CLS is used if arch didn't set CLS explicitly and not
  * all pci devices agree on the same value.  Arch can override either
  * the dfl or actual value as it sees fit.  Don't forget this is
  * measured in 32-bit words, not bytes.
  */
-u8 pci_dfl_cache_line_size __initdata = PCI_CACHE_LINE_BYTES >> 2;
+u8 pci_dfl_cache_line_size __initdata = L1_CACHE_BYTES >> 2;
 u8 pci_cache_line_size;

 /**

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

* [PATCH 3/3 pci#linux-next] pccard: configure CLS on attach
  2009-09-17 17:31 ` Jesse Barnes
  2009-09-22  8:33   ` [PATCH 1/3 pci#linux-next] " Tejun Heo
  2009-09-22  8:34   ` [PATCH 2/3 pci#linux-next] pci,sparc64: drop PCI_CACHE_LINE_BYTES Tejun Heo
@ 2009-09-22  8:34   ` Tejun Heo
  2009-09-22 10:02     ` Axel Birndt
  2009-09-23  5:28     ` Benjamin Herrenschmidt
  2 siblings, 2 replies; 14+ messages in thread
From: Tejun Heo @ 2009-09-22  8:34 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: Greg KH, Robert Hancock, Alan Cox, linux-pci, Linux Kernel,
	Daniel Ritz, Dominik Brodowski, Kenji Kaneshige, Axel Birndt,
	Benjamin Herrenschmidt, Ingo Molnar, Thomas Gleixner, Tony Luck,
	David Miller

For non hotplug PCI devices, the system firmware usually configures
CLS correctly.  For pccard devices system firmware can't do it and
Linux PCI layer doesn't do it either.  Unfortunately this leads to
poor performance for certain devices (sata_sil).  Unless MWI, which
requires separate configuration, is to be used, CLS doesn't affect
correctness, so the configuration should be harmless.

This patch makes pci_set_cacheline_size() always built and export it
and make pccard call it during attach.

Please note that some other PCI hotplug drivers (shpchp and pciehp)
also configure CLS on hotplug.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Daniel Ritz <daniel.ritz@gmx.ch>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Cc: Greg KH <greg@kroah.com>
Cc: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
Cc: Axel Birndt <towerlexa@gmx.de>
---
 drivers/pci/pci.c        |   39 +++++++++++++++++++--------------------
 drivers/pcmcia/cardbus.c |   23 +++++++++++++++--------
 include/linux/pci.h      |    1 +
 3 files changed, 35 insertions(+), 28 deletions(-)

Index: work/drivers/pcmcia/cardbus.c
===================================================================
--- work.orig/drivers/pcmcia/cardbus.c
+++ work/drivers/pcmcia/cardbus.c
@@ -184,26 +184,33 @@ fail:

 =====================================================================*/

-/*
- * Since there is only one interrupt available to CardBus
- * devices, all devices downstream of this device must
- * be using this IRQ.
- */
-static void cardbus_assign_irqs(struct pci_bus *bus, int irq)
+static void cardbus_config_irq_and_cls(struct pci_bus *bus, int irq)
 {
 	struct pci_dev *dev;

 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		u8 irq_pin;

+		/*
+		 * Since there is only one interrupt available to
+		 * CardBus devices, all devices downstream of this
+		 * device must be using this IRQ.
+		 */
 		pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &irq_pin);
 		if (irq_pin) {
 			dev->irq = irq;
 			pci_write_config_byte(dev, PCI_INTERRUPT_LINE, dev->irq);
 		}

+		/*
+		 * Some controllers transfer very slowly with 0 CLS.
+		 * Configure it.  This may fail as CLS configuration
+		 * is mandatory only for MWI.
+		 */
+		pci_set_cacheline_size(dev);
+
 		if (dev->subordinate)
-			cardbus_assign_irqs(dev->subordinate, irq);
+			cardbus_config_irq_and_cls(dev->subordinate, irq);
 	}
 }

@@ -228,7 +235,7 @@ int __ref cb_alloc(struct pcmcia_socket
 	 */
 	pci_bus_size_bridges(bus);
 	pci_bus_assign_resources(bus);
-	cardbus_assign_irqs(bus, s->pci_irq);
+	cardbus_config_irq_and_cls(bus, s->pci_irq);

 	/* socket specific tune function */
 	if (s->tune_bridge)
Index: work/drivers/pci/pci.c
===================================================================
--- work.orig/drivers/pci/pci.c
+++ work/drivers/pci/pci.c
@@ -1871,23 +1871,6 @@ void pci_clear_master(struct pci_dev *de
 	__pci_set_master(dev, false);
 }

-#ifdef PCI_DISABLE_MWI
-int pci_set_mwi(struct pci_dev *dev)
-{
-	return 0;
-}
-
-int pci_try_set_mwi(struct pci_dev *dev)
-{
-	return 0;
-}
-
-void pci_clear_mwi(struct pci_dev *dev)
-{
-}
-
-#else
-
 /**
  * pci_set_cacheline_size - ensure the CACHE_LINE_SIZE register is programmed
  * @dev: the PCI device for which MWI is to be enabled
@@ -1898,13 +1881,12 @@ void pci_clear_mwi(struct pci_dev *dev)
  *
  * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
  */
-static int
-pci_set_cacheline_size(struct pci_dev *dev)
+int pci_set_cacheline_size(struct pci_dev *dev)
 {
 	u8 cacheline_size;

 	if (!pci_cache_line_size)
-		return -EINVAL;		/* The system doesn't support MWI. */
+		return -EINVAL;

 	/* Validate current setting: the PCI_CACHE_LINE_SIZE must be
 	   equal to or multiple of the right value. */
@@ -1926,6 +1908,23 @@ pci_set_cacheline_size(struct pci_dev *d
 	return -EINVAL;
 }

+#ifdef PCI_DISABLE_MWI
+int pci_set_mwi(struct pci_dev *dev)
+{
+	return 0;
+}
+
+int pci_try_set_mwi(struct pci_dev *dev)
+{
+	return 0;
+}
+
+void pci_clear_mwi(struct pci_dev *dev)
+{
+}
+
+#else
+
 /**
  * pci_set_mwi - enables memory-write-invalidate PCI transaction
  * @dev: the PCI device for which MWI is enabled
Index: work/include/linux/pci.h
===================================================================
--- work.orig/include/linux/pci.h
+++ work/include/linux/pci.h
@@ -701,6 +701,7 @@ void pci_disable_device(struct pci_dev *
 void pci_set_master(struct pci_dev *dev);
 void pci_clear_master(struct pci_dev *dev);
 int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state);
+int pci_set_cacheline_size(struct pci_dev *dev);
 #define HAVE_PCI_SET_MWI
 int __must_check pci_set_mwi(struct pci_dev *dev);
 int pci_try_set_mwi(struct pci_dev *dev);

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

* Re: [PATCH 3/3 pci#linux-next] pccard: configure CLS on attach
  2009-09-22  8:34   ` [PATCH 3/3 pci#linux-next] pccard: configure CLS on attach Tejun Heo
@ 2009-09-22 10:02     ` Axel Birndt
  2009-09-23  1:18       ` Tejun Heo
  2009-09-23  5:28     ` Benjamin Herrenschmidt
  1 sibling, 1 reply; 14+ messages in thread
From: Axel Birndt @ 2009-09-22 10:02 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Jesse Barnes, Greg KH, Robert Hancock, Alan Cox, linux-pci,
	Linux Kernel, Daniel Ritz, Dominik Brodowski, Kenji Kaneshige,
	Benjamin Herrenschmidt, Ingo Molnar, Thomas Gleixner, Tony Luck,
	David Miller



Tejun Heo schrieb:
> For non hotplug PCI devices, the system firmware usually configures
> CLS correctly.  For pccard devices system firmware can't do it and
> Linux PCI layer doesn't do it either.  Unfortunately this leads to
> poor performance for certain devices (sata_sil).  Unless MWI, which
> requires separate configuration, is to be used, CLS doesn't affect
> correctness, so the configuration should be harmless.
> 
> This patch makes pci_set_cacheline_size() always built and export it
> and make pccard call it during attach.
> 
> Please note that some other PCI hotplug drivers (shpchp and pciehp)
> also configure CLS on hotplug.

Hello Tejun,

i would like to ask, when this patch is usable for normal guy's, like me.
could you tell me in which kernel release or in which bugfix this will
be implemented?

currently i'am using Kubuntu 8.04 with all current released bugfixes and
kernel updates...

thanks and regards

-- 

Regards Axel

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

* Re: [PATCH 3/3 pci#linux-next] pccard: configure CLS on attach
  2009-09-22 10:02     ` Axel Birndt
@ 2009-09-23  1:18       ` Tejun Heo
  0 siblings, 0 replies; 14+ messages in thread
From: Tejun Heo @ 2009-09-23  1:18 UTC (permalink / raw)
  To: Axel Birndt
  Cc: Jesse Barnes, Greg KH, Robert Hancock, Alan Cox, linux-pci,
	Linux Kernel, Daniel Ritz, Dominik Brodowski, Kenji Kaneshige,
	Benjamin Herrenschmidt, Ingo Molnar, Thomas Gleixner, Tony Luck,
	David Miller

Hello, Axel.

Axel Birndt wrote:
> Tejun Heo schrieb:
>> For non hotplug PCI devices, the system firmware usually configures
>> CLS correctly.  For pccard devices system firmware can't do it and
>> Linux PCI layer doesn't do it either.  Unfortunately this leads to
>> poor performance for certain devices (sata_sil).  Unless MWI, which
>> requires separate configuration, is to be used, CLS doesn't affect
>> correctness, so the configuration should be harmless.
>>
>> This patch makes pci_set_cacheline_size() always built and export it
>> and make pccard call it during attach.
>>
>> Please note that some other PCI hotplug drivers (shpchp and pciehp)
>> also configure CLS on hotplug.
> 
> i would like to ask, when this patch is usable for normal guy's, like me.
> could you tell me in which kernel release or in which bugfix this will
> be implemented?
> 
> currently i'am using Kubuntu 8.04 with all current released bugfixes and
> kernel updates...

This patch is rather dangerous and should sit out for a while in
linux-next and then -rc.  In mainline, I think it will appear in
2.6.33.  Distros may choose to backport them but I don't think they'll
do it before the change at least lands on -rc which is at least 3+
months away.  It's also possible for distros to backport it but enable
it only if certain kernel parameter is set but you'll have to ask each
distro whether they're willing to do such thing.

Thanks.

-- 
tejun

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

* Re: [PATCH 1/3 pci#linux-next] pci: determine CLS more intelligently
  2009-09-22  8:33   ` [PATCH 1/3 pci#linux-next] " Tejun Heo
@ 2009-09-23  5:24     ` Benjamin Herrenschmidt
  2009-10-06 16:53     ` Jesse Barnes
  1 sibling, 0 replies; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2009-09-23  5:24 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Jesse Barnes, Greg KH, Robert Hancock, Alan Cox, linux-pci,
	Linux Kernel, Daniel Ritz, Dominik Brodowski, Kenji Kaneshige,
	Axel Birndt, Ingo Molnar, Thomas Gleixner, Tony Luck,
	David Miller

On Tue, 2009-09-22 at 17:33 +0900, Tejun Heo wrote:
> Till now, CLS has been determined either by arch code or as
> L1_CACHE_BYTES.  Only x86 and ia64 set CLS explicitly and x86 doesn't
> always get it right.  On most configurations, the chance is that
> firmware configures the correct value during boot.
> 
> This patch makes pci_init() determine CLS by looking at what firmware
> has configured.  It scans all devices and if all non-zero values
> agree, the value is used.  If none is configured or there is a
> disagreement, pci_dfl_cache_line_size is used.  arch can set the dfl
> value (via PCI_CACHE_LINE_BYTES or pci_dfl_cache_line_size) or
> override the actual one.
> 
> ia64, x86 and sparc64 updated to set the default cls instead of the
> actual one.
> 
> While at it, declare pci_cache_line_size and pci_dfl_cache_line_size
> in pci.h and drop private declarations from arch code.

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

> Signed-off-by: Tejun Heo <tj@kernel.org>
> Acked-by: David Miller <davem@davemloft.net>
> Acked-by: Greg KH <gregkh@suse.de>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Tony Luck <tony.luck@intel.com>
> ---
> Sorry about the delay.  Here are the regenerated patches.  They are
> against linux-next branch of pci-2.6.git tree as of Sept 22nd
> (76baeebf7df493703eeb4428eac015bdb7fabda6).  Thanks.
> 
>  arch/ia64/pci/pci.c   |    9 +++------
>  arch/x86/pci/common.c |    8 +++-----
>  drivers/pci/pci.c     |   49 +++++++++++++++++++++++++++++++++++++++++--------
>  include/linux/pci.h   |    2 ++
>  4 files changed, 49 insertions(+), 19 deletions(-)
> 
> Index: work/arch/ia64/pci/pci.c
> ===================================================================
> --- work.orig/arch/ia64/pci/pci.c
> +++ work/arch/ia64/pci/pci.c
> @@ -715,9 +715,6 @@ int ia64_pci_legacy_write(struct pci_bus
>  	return ret;
>  }
> 
> -/* It's defined in drivers/pci/pci.c */
> -extern u8 pci_cache_line_size;
> -
>  /**
>   * set_pci_cacheline_size - determine cacheline size for PCI devices
>   *
> @@ -726,7 +723,7 @@ extern u8 pci_cache_line_size;
>   *
>   * Code mostly taken from arch/ia64/kernel/palinfo.c:cache_info().
>   */
> -static void __init set_pci_cacheline_size(void)
> +static void __init set_pci_dfl_cacheline_size(void)
>  {
>  	unsigned long levels, unique_caches;
>  	long status;
> @@ -746,7 +743,7 @@ static void __init set_pci_cacheline_siz
>  			"(status=%ld)\n", __func__, status);
>  		return;
>  	}
> -	pci_cache_line_size = (1 << cci.pcci_line_size) / 4;
> +	pci_dfl_cache_line_size = (1 << cci.pcci_line_size) / 4;
>  }
> 
>  u64 ia64_dma_get_required_mask(struct device *dev)
> @@ -777,7 +774,7 @@ EXPORT_SYMBOL_GPL(dma_get_required_mask)
> 
>  static int __init pcibios_init(void)
>  {
> -	set_pci_cacheline_size();
> +	set_pci_dfl_cacheline_size();
>  	return 0;
>  }
> 
> Index: work/arch/x86/pci/common.c
> ===================================================================
> --- work.orig/arch/x86/pci/common.c
> +++ work/arch/x86/pci/common.c
> @@ -410,8 +410,6 @@ struct pci_bus * __devinit pcibios_scan_
>  	return bus;
>  }
> 
> -extern u8 pci_cache_line_size;
> -
>  int __init pcibios_init(void)
>  {
>  	struct cpuinfo_x86 *c = &boot_cpu_data;
> @@ -426,11 +424,11 @@ int __init pcibios_init(void)
>  	 * and P4. It's also good for 386/486s (which actually have 16)
>  	 * as quite a few PCI devices do not support smaller values.
>  	 */
> -	pci_cache_line_size = 32 >> 2;
> +	pci_dfl_cache_line_size = 32 >> 2;
>  	if (c->x86 >= 6 && c->x86_vendor == X86_VENDOR_AMD)
> -		pci_cache_line_size = 64 >> 2;	/* K7 & K8 */
> +		pci_dfl_cache_line_size = 64 >> 2;	/* K7 & K8 */
>  	else if (c->x86 > 6 && c->x86_vendor == X86_VENDOR_INTEL)
> -		pci_cache_line_size = 128 >> 2;	/* P4 */
> +		pci_dfl_cache_line_size = 128 >> 2;	/* P4 */
> 
>  	pcibios_resource_survey();
> 
> Index: work/drivers/pci/pci.c
> ===================================================================
> --- work.orig/drivers/pci/pci.c
> +++ work/drivers/pci/pci.c
> @@ -47,6 +47,19 @@ unsigned long pci_cardbus_mem_size = DEF
>  unsigned long pci_hotplug_io_size  = DEFAULT_HOTPLUG_IO_SIZE;
>  unsigned long pci_hotplug_mem_size = DEFAULT_HOTPLUG_MEM_SIZE;
> 
> +#ifndef PCI_CACHE_LINE_BYTES
> +#define PCI_CACHE_LINE_BYTES L1_CACHE_BYTES
> +#endif
> +
> +/*
> + * The default CLS is used if arch didn't set CLS explicitly and not
> + * all pci devices agree on the same value.  Arch can override either
> + * the dfl or actual value as it sees fit.  Don't forget this is
> + * measured in 32-bit words, not bytes.
> + */
> +u8 pci_dfl_cache_line_size __initdata = PCI_CACHE_LINE_BYTES >> 2;
> +u8 pci_cache_line_size;
> +
>  /**
>   * pci_bus_max_busnr - returns maximum PCI bus number of given bus' children
>   * @bus: pointer to PCI bus structure to search
> @@ -1879,14 +1892,6 @@ void pci_clear_mwi(struct pci_dev *dev)
> 
>  #else
> 
> -#ifndef PCI_CACHE_LINE_BYTES
> -#define PCI_CACHE_LINE_BYTES L1_CACHE_BYTES
> -#endif
> -
> -/* This can be overridden by arch code. */
> -/* Don't forget this is measured in 32-bit words, not bytes */
> -u8 pci_cache_line_size = PCI_CACHE_LINE_BYTES / 4;
> -
>  /**
>   * pci_set_cacheline_size - ensure the CACHE_LINE_SIZE register is programmed
>   * @dev: the PCI device for which MWI is to be enabled
> @@ -2722,9 +2727,37 @@ int __attribute__ ((weak)) pci_ext_cfg_a
>  static int __devinit pci_init(void)
>  {
>  	struct pci_dev *dev = NULL;
> +	u8 cls = 0;
> +	u8 tmp;
> +
> +	if (pci_cache_line_size)
> +		printk(KERN_DEBUG "PCI: CLS %u bytes\n",
> +		       pci_cache_line_size << 2);
> 
>  	while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
>  		pci_fixup_device(pci_fixup_final, dev);
> +		/*
> +		 * If arch hasn't set it explicitly yet, use the CLS
> +		 * value shared by all PCI devices.  If there's a
> +		 * mismatch, fall back to the default value.
> +		 */
> +		if (!pci_cache_line_size) {
> +			pci_read_config_byte(dev, PCI_CACHE_LINE_SIZE, &tmp);
> +			if (!cls)
> +				cls = tmp;
> +			if (!tmp || cls == tmp)
> +				continue;
> +
> +			printk(KERN_DEBUG "PCI: CLS mismatch (%u != %u), "
> +			       "using %u bytes\n", cls << 2, tmp << 2,
> +			       pci_dfl_cache_line_size << 2);
> +			pci_cache_line_size = pci_dfl_cache_line_size;
> +		}
> +	}
> +	if (!pci_cache_line_size) {
> +		printk(KERN_DEBUG "PCI: CLS %u bytes, default %u\n",
> +		       cls << 2, pci_dfl_cache_line_size << 2);
> +		pci_cache_line_size = cls;
>  	}
> 
>  	return 0;
> Index: work/include/linux/pci.h
> ===================================================================
> --- work.orig/include/linux/pci.h
> +++ work/include/linux/pci.h
> @@ -1246,6 +1246,8 @@ extern int pci_pci_problems;
> 
>  extern unsigned long pci_cardbus_io_size;
>  extern unsigned long pci_cardbus_mem_size;
> +extern u8 pci_dfl_cache_line_size;
> +extern u8 pci_cache_line_size;
> 
>  extern unsigned long pci_hotplug_io_size;
>  extern unsigned long pci_hotplug_mem_size;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

* Re: [PATCH 3/3 pci#linux-next] pccard: configure CLS on attach
  2009-09-22  8:34   ` [PATCH 3/3 pci#linux-next] pccard: configure CLS on attach Tejun Heo
  2009-09-22 10:02     ` Axel Birndt
@ 2009-09-23  5:28     ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 14+ messages in thread
From: Benjamin Herrenschmidt @ 2009-09-23  5:28 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Jesse Barnes, Greg KH, Robert Hancock, Alan Cox, linux-pci,
	Linux Kernel, Daniel Ritz, Dominik Brodowski, Kenji Kaneshige,
	Axel Birndt, Ingo Molnar, Thomas Gleixner, Tony Luck,
	David Miller

On Tue, 2009-09-22 at 17:34 +0900, Tejun Heo wrote:
> For non hotplug PCI devices, the system firmware usually configures
> CLS correctly.  For pccard devices system firmware can't do it and
> Linux PCI layer doesn't do it either.  Unfortunately this leads to
> poor performance for certain devices (sata_sil).  Unless MWI, which
> requires separate configuration, is to be used, CLS doesn't affect
> correctness, so the configuration should be harmless.

That isn't entirely true. We had some errata from some AMCC SoCs
telling us to filter out CLS because devices would use Memory
Read Multiple unless it's set to 0 and they have a bug with it...

(I find it weird too, MRM shouldn't relate to CLS ... I strongly suspect
they were thinking about Memory Read Line).

No objection to the patch though, I don't see a problem in setting
it properly on Cardbus, and for those bogus SoCs we actually filter
out the write at the low level config space access level afaik.

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Ben.

> This patch makes pci_set_cacheline_size() always built and export it
> and make pccard call it during attach.
> 
> Please note that some other PCI hotplug drivers (shpchp and pciehp)
> also configure CLS on hotplug.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Daniel Ritz <daniel.ritz@gmx.ch>
> Cc: Dominik Brodowski <linux@dominikbrodowski.net>
> Cc: Greg KH <greg@kroah.com>
> Cc: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
> Cc: Axel Birndt <towerlexa@gmx.de>
> ---
>  drivers/pci/pci.c        |   39 +++++++++++++++++++--------------------
>  drivers/pcmcia/cardbus.c |   23 +++++++++++++++--------
>  include/linux/pci.h      |    1 +
>  3 files changed, 35 insertions(+), 28 deletions(-)
> 
> Index: work/drivers/pcmcia/cardbus.c
> ===================================================================
> --- work.orig/drivers/pcmcia/cardbus.c
> +++ work/drivers/pcmcia/cardbus.c
> @@ -184,26 +184,33 @@ fail:
> 
>  =====================================================================*/
> 
> -/*
> - * Since there is only one interrupt available to CardBus
> - * devices, all devices downstream of this device must
> - * be using this IRQ.
> - */
> -static void cardbus_assign_irqs(struct pci_bus *bus, int irq)
> +static void cardbus_config_irq_and_cls(struct pci_bus *bus, int irq)
>  {
>  	struct pci_dev *dev;
> 
>  	list_for_each_entry(dev, &bus->devices, bus_list) {
>  		u8 irq_pin;
> 
> +		/*
> +		 * Since there is only one interrupt available to
> +		 * CardBus devices, all devices downstream of this
> +		 * device must be using this IRQ.
> +		 */
>  		pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &irq_pin);
>  		if (irq_pin) {
>  			dev->irq = irq;
>  			pci_write_config_byte(dev, PCI_INTERRUPT_LINE, dev->irq);
>  		}
> 
> +		/*
> +		 * Some controllers transfer very slowly with 0 CLS.
> +		 * Configure it.  This may fail as CLS configuration
> +		 * is mandatory only for MWI.
> +		 */
> +		pci_set_cacheline_size(dev);
> +
>  		if (dev->subordinate)
> -			cardbus_assign_irqs(dev->subordinate, irq);
> +			cardbus_config_irq_and_cls(dev->subordinate, irq);
>  	}
>  }
> 
> @@ -228,7 +235,7 @@ int __ref cb_alloc(struct pcmcia_socket
>  	 */
>  	pci_bus_size_bridges(bus);
>  	pci_bus_assign_resources(bus);
> -	cardbus_assign_irqs(bus, s->pci_irq);
> +	cardbus_config_irq_and_cls(bus, s->pci_irq);
> 
>  	/* socket specific tune function */
>  	if (s->tune_bridge)
> Index: work/drivers/pci/pci.c
> ===================================================================
> --- work.orig/drivers/pci/pci.c
> +++ work/drivers/pci/pci.c
> @@ -1871,23 +1871,6 @@ void pci_clear_master(struct pci_dev *de
>  	__pci_set_master(dev, false);
>  }
> 
> -#ifdef PCI_DISABLE_MWI
> -int pci_set_mwi(struct pci_dev *dev)
> -{
> -	return 0;
> -}
> -
> -int pci_try_set_mwi(struct pci_dev *dev)
> -{
> -	return 0;
> -}
> -
> -void pci_clear_mwi(struct pci_dev *dev)
> -{
> -}
> -
> -#else
> -
>  /**
>   * pci_set_cacheline_size - ensure the CACHE_LINE_SIZE register is programmed
>   * @dev: the PCI device for which MWI is to be enabled
> @@ -1898,13 +1881,12 @@ void pci_clear_mwi(struct pci_dev *dev)
>   *
>   * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
>   */
> -static int
> -pci_set_cacheline_size(struct pci_dev *dev)
> +int pci_set_cacheline_size(struct pci_dev *dev)
>  {
>  	u8 cacheline_size;
> 
>  	if (!pci_cache_line_size)
> -		return -EINVAL;		/* The system doesn't support MWI. */
> +		return -EINVAL;
> 
>  	/* Validate current setting: the PCI_CACHE_LINE_SIZE must be
>  	   equal to or multiple of the right value. */
> @@ -1926,6 +1908,23 @@ pci_set_cacheline_size(struct pci_dev *d
>  	return -EINVAL;
>  }
> 
> +#ifdef PCI_DISABLE_MWI
> +int pci_set_mwi(struct pci_dev *dev)
> +{
> +	return 0;
> +}
> +
> +int pci_try_set_mwi(struct pci_dev *dev)
> +{
> +	return 0;
> +}
> +
> +void pci_clear_mwi(struct pci_dev *dev)
> +{
> +}
> +
> +#else
> +
>  /**
>   * pci_set_mwi - enables memory-write-invalidate PCI transaction
>   * @dev: the PCI device for which MWI is enabled
> Index: work/include/linux/pci.h
> ===================================================================
> --- work.orig/include/linux/pci.h
> +++ work/include/linux/pci.h
> @@ -701,6 +701,7 @@ void pci_disable_device(struct pci_dev *
>  void pci_set_master(struct pci_dev *dev);
>  void pci_clear_master(struct pci_dev *dev);
>  int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state);
> +int pci_set_cacheline_size(struct pci_dev *dev);
>  #define HAVE_PCI_SET_MWI
>  int __must_check pci_set_mwi(struct pci_dev *dev);
>  int pci_try_set_mwi(struct pci_dev *dev);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

* Re: [PATCH 1/3] pci: determine CLS more intelligently
  2009-07-04  5:08 [PATCH 1/3] pci: determine CLS more intelligently Tejun Heo
                   ` (2 preceding siblings ...)
  2009-09-17 17:31 ` Jesse Barnes
@ 2009-09-28 17:52 ` Jesse Barnes
  3 siblings, 0 replies; 14+ messages in thread
From: Jesse Barnes @ 2009-09-28 17:52 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Greg KH, Robert Hancock, Alan Cox, linux-pci, Linux Kernel,
	Daniel Ritz, Dominik Brodowski, Kenji Kaneshige, Axel Birndt,
	Benjamin Herrenschmidt, Ingo Molnar, Thomas Gleixner, Tony Luck,
	David Miller

On Sat, 04 Jul 2009 14:08:57 +0900
Tejun Heo <tj@kernel.org> wrote:

> Till now, CLS has been determined either by arch code or as
> L1_CACHE_BYTES.  Only x86 and ia64 set CLS explicitly and x86 doesn't
> always get it right.  On most configurations, the chance is that
> firmware configures the correct value during boot.
> 
> This patch makes pci_init() determine CLS by looking at what firmware
> has configured.  It scans all devices and if all non-zero values
> agree, the value is used.  If none is configured or there is a
> disagreement, pci_dfl_cache_line_size is used.  arch can set the dfl
> value (via PCI_CACHE_LINE_BYTES or pci_dfl_cache_line_size) or
> override the actual one.
> 
> ia64, x86 and sparc64 updated to set the default cls instead of the
> actual one.
> 
> While at it, declare pci_cache_line_size and pci_dfl_cache_line_size
> in pci.h and drop private declarations from arch code.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Acked-by: David Miller <davem@davemloft.net>
> Acked-by: Greg KH <gregkh@suse.de>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Tony Luck <tony.luck@intel.com>
> ---

Thanks Tejun, I'll pull these in later this week when I get back from
Portland.

Jesse

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

* Re: [PATCH 1/3 pci#linux-next] pci: determine CLS more intelligently
  2009-09-22  8:33   ` [PATCH 1/3 pci#linux-next] " Tejun Heo
  2009-09-23  5:24     ` Benjamin Herrenschmidt
@ 2009-10-06 16:53     ` Jesse Barnes
  1 sibling, 0 replies; 14+ messages in thread
From: Jesse Barnes @ 2009-10-06 16:53 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Greg KH, Robert Hancock, Alan Cox, linux-pci, Linux Kernel,
	Daniel Ritz, Dominik Brodowski, Kenji Kaneshige, Axel Birndt,
	Benjamin Herrenschmidt, Ingo Molnar, Thomas Gleixner, Tony Luck,
	David Miller

On Tue, 22 Sep 2009 17:33:38 +0900
Tejun Heo <tj@kernel.org> wrote:

> Till now, CLS has been determined either by arch code or as
> L1_CACHE_BYTES.  Only x86 and ia64 set CLS explicitly and x86 doesn't
> always get it right.  On most configurations, the chance is that
> firmware configures the correct value during boot.
> 
> This patch makes pci_init() determine CLS by looking at what firmware
> has configured.  It scans all devices and if all non-zero values
> agree, the value is used.  If none is configured or there is a
> disagreement, pci_dfl_cache_line_size is used.  arch can set the dfl
> value (via PCI_CACHE_LINE_BYTES or pci_dfl_cache_line_size) or
> override the actual one.
> 
> ia64, x86 and sparc64 updated to set the default cls instead of the
> actual one.
> 
> While at it, declare pci_cache_line_size and pci_dfl_cache_line_size
> in pci.h and drop private declarations from arch code.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Acked-by: David Miller <davem@davemloft.net>
> Acked-by: Greg KH <gregkh@suse.de>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Tony Luck <tony.luck@intel.com>

Applied this series to my linux-next branch, thanks.

-- 
Jesse Barnes, Intel Open Source Technology Center

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

end of thread, other threads:[~2009-10-06 16:53 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-07-04  5:08 [PATCH 1/3] pci: determine CLS more intelligently Tejun Heo
2009-07-04  5:09 ` [PATCH 2/3] pci,sparc64: drop PCI_CACHE_LINE_BYTES Tejun Heo
2009-07-04  5:10   ` [PATCH 3/3] pccard: configure CLS on attach Tejun Heo
2009-07-04  9:29 ` [PATCH 1/3] pci: determine CLS more intelligently Ingo Molnar
2009-09-17 17:31 ` Jesse Barnes
2009-09-22  8:33   ` [PATCH 1/3 pci#linux-next] " Tejun Heo
2009-09-23  5:24     ` Benjamin Herrenschmidt
2009-10-06 16:53     ` Jesse Barnes
2009-09-22  8:34   ` [PATCH 2/3 pci#linux-next] pci,sparc64: drop PCI_CACHE_LINE_BYTES Tejun Heo
2009-09-22  8:34   ` [PATCH 3/3 pci#linux-next] pccard: configure CLS on attach Tejun Heo
2009-09-22 10:02     ` Axel Birndt
2009-09-23  1:18       ` Tejun Heo
2009-09-23  5:28     ` Benjamin Herrenschmidt
2009-09-28 17:52 ` [PATCH 1/3] pci: determine CLS more intelligently Jesse Barnes

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.