2009-07-04 05:07:52

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 1/3] pci: determine CLS more intelligently

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 <[email protected]>
Acked-by: David Miller <[email protected]>
Acked-by: Greg KH <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Tony Luck <[email protected]>
---
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);


2009-07-04 05:08:21

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 2/3] pci,sparc64: drop PCI_CACHE_LINE_BYTES

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 <[email protected]>
Signed-off-by: Tejun Heo <[email protected]>
---
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;

/**

2009-07-04 05:08:57

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 3/3] pccard: configure CLS on attach

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 <[email protected]>
Cc: Daniel Ritz <[email protected]>
Cc: Dominik Brodowski <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Kenji Kaneshige <[email protected]>
Cc: Axel Birndt <[email protected]>
---
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);

2009-07-04 09:30:41

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 1/3] pci: determine CLS more intelligently


* Tejun Heo <[email protected]> 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 <[email protected]>
> Acked-by: David Miller <[email protected]>
> Acked-by: Greg KH <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Tony Luck <[email protected]>

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 <[email protected]>

Ingo

2009-09-17 17:31:39

by Jesse Barnes

[permalink] [raw]
Subject: Re: [PATCH 1/3] pci: determine CLS more intelligently

On Sat, 04 Jul 2009 14:08:57 +0900
Tejun Heo <[email protected]> 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

2009-09-22 08:34:20

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 1/3 pci#linux-next] pci: determine CLS more intelligently

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 <[email protected]>
Acked-by: David Miller <[email protected]>
Acked-by: Greg KH <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Tony Luck <[email protected]>
---
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;

2009-09-22 08:34:54

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 2/3 pci#linux-next] pci,sparc64: drop PCI_CACHE_LINE_BYTES

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 <[email protected]>
Signed-off-by: Tejun Heo <[email protected]>
---
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;

/**

2009-09-22 08:35:26

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 3/3 pci#linux-next] pccard: configure CLS on attach

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 <[email protected]>
Cc: Daniel Ritz <[email protected]>
Cc: Dominik Brodowski <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Kenji Kaneshige <[email protected]>
Cc: Axel Birndt <[email protected]>
---
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);

2009-09-22 10:02:13

by Axel Birndt

[permalink] [raw]
Subject: Re: [PATCH 3/3 pci#linux-next] pccard: configure CLS on attach



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

2009-09-23 01:20:17

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH 3/3 pci#linux-next] pccard: configure CLS on attach

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

2009-09-23 05:26:11

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [PATCH 1/3 pci#linux-next] pci: determine CLS more intelligently

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 <[email protected]>
---

> Signed-off-by: Tejun Heo <[email protected]>
> Acked-by: David Miller <[email protected]>
> Acked-by: Greg KH <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Tony Luck <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2009-09-23 05:30:24

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [PATCH 3/3 pci#linux-next] pccard: configure CLS on attach

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 <[email protected]>

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 <[email protected]>
> Cc: Daniel Ritz <[email protected]>
> Cc: Dominik Brodowski <[email protected]>
> Cc: Greg KH <[email protected]>
> Cc: Kenji Kaneshige <[email protected]>
> Cc: Axel Birndt <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2009-09-28 17:52:46

by Jesse Barnes

[permalink] [raw]
Subject: Re: [PATCH 1/3] pci: determine CLS more intelligently

On Sat, 04 Jul 2009 14:08:57 +0900
Tejun Heo <[email protected]> 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 <[email protected]>
> Acked-by: David Miller <[email protected]>
> Acked-by: Greg KH <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Tony Luck <[email protected]>
> ---

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

Jesse

2009-10-06 16:53:53

by Jesse Barnes

[permalink] [raw]
Subject: Re: [PATCH 1/3 pci#linux-next] pci: determine CLS more intelligently

On Tue, 22 Sep 2009 17:33:38 +0900
Tejun Heo <[email protected]> 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 <[email protected]>
> Acked-by: David Miller <[email protected]>
> Acked-by: Greg KH <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Tony Luck <[email protected]>

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

--
Jesse Barnes, Intel Open Source Technology Center