PCI patch breaks boot on rp2470 (2xPA8700)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Kyle,

I spent the weekend bisecting to determine which patch broke booting
from scsi on my rp2470.

This patch is the culprit:
~~~
ac1aa47b131416a6ff37eb1005a0a1d2541aad6c is the first bad commit
commit ac1aa47b131416a6ff37eb1005a0a1d2541aad6c
Author: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
Date:   Mon Oct 26 13:20:44 2009 -0700

    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 <tj@xxxxxxxxxx>
    Acked-by: David Miller <davem@xxxxxxxxxxxxx>
    Acked-by: Greg KH <gregkh@xxxxxxx>
    Cc: Ingo Molnar <mingo@xxxxxxx>
    Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
    Cc: Tony Luck <tony.luck@xxxxxxxxx>
    Signed-off-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>

:040000 040000 8b20ad60ed3e273b74bfb588dbea6948547e8de1
92498585770ca360c2716c0d3c55d5f4a37356a1 M     arch
:040000 040000 56d1abd61286dd303bb37c2002b699e526988f85
3f20bba2d1e107a80a738e3561fc0fa92e0c4024 M     drivers
:040000 040000 26d85393248c542ca2cea0e3ac4ceabd0ea659aa
326cbb98321cd4e490d888f48bc584b3662c8f06 M     include
~~~

Removing this patch allows my system to boot again.

The patch does something which is wrong IMO. It changes *when*
pci_cache_line_size is set from (a) initialized data to (b) a call to
pci_apply_final_quirks(...). There are several drivers which call
pci_try_set_mwi(...) and pci_set_mwi(...) which indirectly use of the
value of pci_cache_line_size, and the author of the patch doesn't make
any mention of having verified that the drivers don't call
pci_*_set_mwi(...) *before* pci_apply_final_quirks(...) is called.

Case in point the sym53c8xx driver does this:
~~~ drivers/scsi/sym53c8xx_2/sym_glue.c
static int __devinit sym_set_workarounds(struct sym_device *device)
        ...
        /* If the chip can do Memory Write Invalidate, enable it */
        if (chip->features & FE_WRIE) {
                if (pci_set_mwi(pdev))
                        return -ENODEV;
        }
        ...
~~~ drivers/pci/pci.c
int
pci_set_mwi(struct pci_dev *dev)
{
        int rc;
        u16 cmd;

        rc = pci_set_cacheline_size(dev);
        if (rc)
                return rc;
...
~~~ driver/pci/pci.c
int pci_set_cacheline_size(struct pci_dev *dev)
{
        u8 cacheline_size;

        if (!pci_cache_line_size)
                return -EINVAL;
...
~~~
Which means that if the driver calls pci_set_mwi() *before*
pci_apply_final_quirks() sets pci_cache_line_size, that it would
appear as if there are no devices.

To verify this I instrumented the PCI subsystem but my assumption
turns out not to be true:
~~~
PCI: pcibios_init set CLS.
...
PCI: CLS 64 bytes
...
pci_set_mwi begin
pci_set_cacheline_size: PCI_CLS is 16
pci_set_cacheline_size: Read CLS is 16
pci_set_mwi end
...
~~~
The ordering is correct.

The attached patch allows me to boot again by setting the PCI CLS to
64 early-on in pcibios_init().

I don't know *why* it works, what a shame, can anyone see something I've missed?

Cheers,
Carlos.
Subject: [PATCH] hppa: Set PCI CLS early in boot.
From: Carlos O'Donell <carlos@xxxxxxxxxxxxxxxx>

Set the PCI CLS early in the boot process to prevent
device failures. In pcibios_set_master use the new 
pci_cache_line_size instead of a hard-coded value.

Signed-off-by: Carlos O'Donell <carlos@xxxxxxxxxxxxxxxx>

---
 arch/parisc/kernel/pci.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/parisc/kernel/pci.c b/arch/parisc/kernel/pci.c
index f7064ab..4f84991 100644
--- a/arch/parisc/kernel/pci.c
+++ b/arch/parisc/kernel/pci.c
@@ -18,7 +18,6 @@
 
 #include <asm/io.h>
 #include <asm/system.h>
-#include <asm/cache.h>		/* for L1_CACHE_BYTES */
 #include <asm/superio.h>
 
 #define DEBUG_RESOURCES 0
@@ -123,6 +122,10 @@ static int __init pcibios_init(void)
 	} else {
 		printk(KERN_WARNING "pci_bios != NULL but init() is!\n");
 	}
+
+	/* Set the CLS for PCI as early as possible. */
+	pci_cache_line_size = pci_dfl_cache_line_size;
+ 
 	return 0;
 }
 
@@ -170,8 +173,8 @@ void pcibios_set_master(struct pci_dev *dev)
 	** HP generally has fewer devices on the bus than other architectures.
 	** upper byte is PCI_LATENCY_TIMER.
 	*/
-	pci_write_config_word(dev, PCI_CACHE_LINE_SIZE,
-				(0x80 << 8) | (L1_CACHE_BYTES / sizeof(u32)));
+	pci_write_config_word(dev, PCI_CACHE_LINE_SIZE, 
+			      (0x80 << 8) | pci_cache_line_size);
 }
 
 

[Index of Archives]     [Linux SoC]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux