Breno Leitao wrote:
Hello, I found that qlge is broken on PPC, and it got broken after commit 06a1cbafb253c4c60d6a54a994887f5fbceabcc0. It happens because dev->pcie is not set on PPC, because the function set_pcie_port_type(), who sets
dev->pcie, is not being called on PPC PCI code.

Hi Breno,

I'm sorry for the regression. I didn't realize that PPC uses open-coded
PCI scan.

I guess the problem is caused by dev->pcie_cap, right? But I don't
understand what is happening clearly. Could you send me the detailed
information about the problem?


So, I have two ideas to fix it, the first one is to call set_pcie_port_type() on pci_device_add() instead of pci_setup_device(). Since that PPC device flow calls pci_device_add(), it fixes the problem without duplicating the caller for this function.

The set_pcie_port_type() needs to be called before pci_fixup_device() in
pci_setup_device() because fixup code might refer dev->pcie_cap. So I
don't think the first one is a good idea.

OTOH, it's also possible to add set_pcie_port_type() on pci.h and call
it inside the PPC PCI files, specifically on of_create_pci_dev().

I tested both ideas and they work perfect, so I'd like to figure out which one is the most correct one. Both patches are attached here.

I think the second one is better. But to prevent similar problems, I
think it would be better to remove open-coded PCI scan in PPC in a
long term, though I don't know the background about that.

Thanks,
Kenji Kaneshige



Thanks, Breno

----- First idea -----

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 98ffb2d..328c3ab 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -746,7 +746,6 @@ int pci_setup_device(struct pci_dev *dev)
        dev->hdr_type = hdr_type & 0x7f;
        dev->multifunction = !!(hdr_type & 0x80);
        dev->error_state = pci_channel_io_normal;
-       set_pcie_port_type(dev);
        set_pci_aer_firmware_first(dev);
list_for_each_entry(slot, &dev->bus->slots, list)
@@ -1052,6 +1051,7 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus 
*bus)
        /* Initialize various capabilities */
        pci_init_capabilities(dev);
+ set_pcie_port_type(dev);
        /*
         * Add the device to our list of discovered devices
         * and the bus list for fixup functions, etc.

----- Second idea -----

diff --git a/arch/powerpc/kernel/pci_of_scan.c 
b/arch/powerpc/kernel/pci_of_scan.c
index 7311fdf..f8820e8 100644
--- a/arch/powerpc/kernel/pci_of_scan.c
+++ b/arch/powerpc/kernel/pci_of_scan.c
@@ -160,6 +160,7 @@ struct pci_dev *of_create_pci_dev(struct device_node *node,
        dev->error_state = pci_channel_io_normal;
        dev->dma_mask = 0xffffffff;
+ set_pcie_port_type(dev);
        if (!strcmp(type, "pci") || !strcmp(type, "pciex")) {
                /* a PCI-PCI bridge */
                dev->hdr_type = PCI_HEADER_TYPE_BRIDGE;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 98ffb2d..f787eea 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -681,7 +681,7 @@ static void pci_read_irq(struct pci_dev *dev)
        dev->irq = irq;
 }
-static void set_pcie_port_type(struct pci_dev *pdev)
+void set_pcie_port_type(struct pci_dev *pdev)
 {
        int pos;
        u16 reg16;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 174e539..765095b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1269,6 +1269,7 @@ int pcibios_add_platform_entries(struct pci_dev *dev);
 void pcibios_disable_device(struct pci_dev *dev);
 int pcibios_set_pcie_reset_state(struct pci_dev *dev,
                                 enum pcie_reset_state state);
+extern void set_pcie_port_type(struct pci_dev *pdev);
#ifdef CONFIG_PCI_MMCONFIG
 extern void __init pci_mmcfg_early_init(void);




_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to