From 05196e258a617ee14dd3255c480cbc3f0caff5ce Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Mon, 30 Oct 2017 13:26:32 +0100 Subject: PCI: ibmphp: Use common error handling code in unconfigure_boot_device() Combine two error paths that emit the same message and return the same error code. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring [bhelgaas: changelog] Signed-off-by: Bjorn Helgaas --- drivers/pci/hotplug/ibmphp_pci.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/hotplug/ibmphp_pci.c b/drivers/pci/hotplug/ibmphp_pci.c index dc1876feb06f..25edd0b18b75 100644 --- a/drivers/pci/hotplug/ibmphp_pci.c +++ b/drivers/pci/hotplug/ibmphp_pci.c @@ -1267,20 +1267,19 @@ static int unconfigure_boot_device(u8 busno, u8 device, u8 function) size = size & 0xFFFFFFFC; size = ~size + 1; end_address = start_address + size - 1; - if (ibmphp_find_resource(bus, start_address, &io, IO) < 0) { - err("cannot find corresponding IO resource to remove\n"); - return -EIO; - } + if (ibmphp_find_resource(bus, start_address, &io, IO)) + goto report_search_failure; + debug("io->start = %x\n", io->start); temp_end = io->end; start_address = io->end + 1; ibmphp_remove_resource(io); /* This is needed b/c of the old I/O restrictions in the BIOS */ while (temp_end < end_address) { - if (ibmphp_find_resource(bus, start_address, &io, IO) < 0) { - err("cannot find corresponding IO resource to remove\n"); - return -EIO; - } + if (ibmphp_find_resource(bus, start_address, + &io, IO)) + goto report_search_failure; + debug("io->start = %x\n", io->start); temp_end = io->end; start_address = io->end + 1; @@ -1327,6 +1326,10 @@ static int unconfigure_boot_device(u8 busno, u8 device, u8 function) } /* end of for */ return 0; + +report_search_failure: + err("cannot find corresponding IO resource to remove\n"); + return -EIO; } static int unconfigure_boot_bridge(u8 busno, u8 device, u8 function) -- cgit v1.2.3 From c4459a086748848b8a498282780f708c235a06a5 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 4 Oct 2017 17:53:48 -0700 Subject: PCI: pciehp: Convert timers to use timer_setup() In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly. This fixes what appears to be a bug in passing the wrong pointer to the timer handler (address of ctrl pointer instead of ctrl pointer). Signed-off-by: Kees Cook Signed-off-by: Bjorn Helgaas Cc: Mika Westerberg Cc: Mayurkumar Patel Cc: Keith Busch Cc: Thomas Gleixner --- drivers/pci/hotplug/pciehp_hpc.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index e5d5ce9e3010..ba5055c5115c 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -50,14 +50,13 @@ static irqreturn_t pcie_isr(int irq, void *dev_id); static void start_int_poll_timer(struct controller *ctrl, int sec); /* This is the interrupt polling timeout function. */ -static void int_poll_timeout(unsigned long data) +static void int_poll_timeout(struct timer_list *t) { - struct controller *ctrl = (struct controller *)data; + struct controller *ctrl = from_timer(ctrl, t, poll_timer); /* Poll for interrupt events. regs == NULL => polling */ pcie_isr(0, ctrl); - init_timer(&ctrl->poll_timer); if (!pciehp_poll_time) pciehp_poll_time = 2; /* default polling interval is 2 sec */ @@ -71,8 +70,6 @@ static void start_int_poll_timer(struct controller *ctrl, int sec) if ((sec <= 0) || (sec > 60)) sec = 2; - ctrl->poll_timer.function = &int_poll_timeout; - ctrl->poll_timer.data = (unsigned long)ctrl; ctrl->poll_timer.expires = jiffies + sec * HZ; add_timer(&ctrl->poll_timer); } @@ -83,7 +80,7 @@ static inline int pciehp_request_irq(struct controller *ctrl) /* Install interrupt polling timer. Start with 10 sec delay */ if (pciehp_poll_mode) { - init_timer(&ctrl->poll_timer); + timer_setup(&ctrl->poll_timer, int_poll_timeout, 0); start_int_poll_timer(ctrl, 10); return 0; } @@ -764,8 +761,7 @@ int pciehp_reset_slot(struct slot *slot, int probe) ctrl_dbg(ctrl, "%s: SLOTCTRL %x write cmd %x\n", __func__, pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, ctrl_mask); if (pciehp_poll_mode) - int_poll_timeout(ctrl->poll_timer.data); - + int_poll_timeout(&ctrl->poll_timer); return 0; } -- cgit v1.2.3 From 34d773f6ca5b4b5b1961836b557d34314226aa23 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 16 Oct 2017 16:18:02 -0700 Subject: PCI: cpqphp: Convert timers to use timer_setup() In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly. This has the result of fixing pushbutton_helper_thread(), which was truncating the event pointer to 32 bits. Signed-off-by: Kees Cook Signed-off-by: Bjorn Helgaas Cc: Ingo Molnar Cc: Arvind Yadav Cc: Quentin Lambert Cc: Aleksandr Bezzubikov Cc: "Michael S. Tsirkin" Cc: Marcel Apfelbaum --- drivers/pci/hotplug/cpqphp.h | 2 +- drivers/pci/hotplug/cpqphp_core.c | 3 +-- drivers/pci/hotplug/cpqphp_ctrl.c | 19 ++++++++++--------- 3 files changed, 12 insertions(+), 12 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/hotplug/cpqphp.h b/drivers/pci/hotplug/cpqphp.h index 48c8a066a6b7..c2bbe6b65d06 100644 --- a/drivers/pci/hotplug/cpqphp.h +++ b/drivers/pci/hotplug/cpqphp.h @@ -410,7 +410,7 @@ void cpqhp_create_debugfs_files(struct controller *ctrl); void cpqhp_remove_debugfs_files(struct controller *ctrl); /* controller functions */ -void cpqhp_pushbutton_thread(unsigned long event_pointer); +void cpqhp_pushbutton_thread(struct timer_list *t); irqreturn_t cpqhp_ctrl_intr(int IRQ, void *data); int cpqhp_find_available_resources(struct controller *ctrl, void __iomem *rom_start); diff --git a/drivers/pci/hotplug/cpqphp_core.c b/drivers/pci/hotplug/cpqphp_core.c index 4d06b8461255..70967ac75265 100644 --- a/drivers/pci/hotplug/cpqphp_core.c +++ b/drivers/pci/hotplug/cpqphp_core.c @@ -661,9 +661,8 @@ static int ctrl_slot_setup(struct controller *ctrl, slot->p_sm_slot = slot_entry; - init_timer(&slot->task_event); + timer_setup(&slot->task_event, cpqhp_pushbutton_thread, 0); slot->task_event.expires = jiffies + 5 * HZ; - slot->task_event.function = cpqhp_pushbutton_thread; /*FIXME: these capabilities aren't used but if they are * they need to be correctly implemented diff --git a/drivers/pci/hotplug/cpqphp_ctrl.c b/drivers/pci/hotplug/cpqphp_ctrl.c index a55653b54eed..a93069e739cb 100644 --- a/drivers/pci/hotplug/cpqphp_ctrl.c +++ b/drivers/pci/hotplug/cpqphp_ctrl.c @@ -47,7 +47,7 @@ static void interrupt_event_handler(struct controller *ctrl); static struct task_struct *cpqhp_event_thread; -static unsigned long pushbutton_pending; /* = 0 */ +static struct timer_list *pushbutton_pending; /* = NULL */ /* delay is in jiffies to wait for */ static void long_delay(int delay) @@ -1732,9 +1732,10 @@ static u32 remove_board(struct pci_func *func, u32 replace_flag, struct controll return 0; } -static void pushbutton_helper_thread(unsigned long data) +static void pushbutton_helper_thread(struct timer_list *t) { - pushbutton_pending = data; + pushbutton_pending = t; + wake_up_process(cpqhp_event_thread); } @@ -1883,13 +1884,13 @@ static void interrupt_event_handler(struct controller *ctrl) wait_for_ctrl_irq(ctrl); mutex_unlock(&ctrl->crit_sect); - init_timer(&p_slot->task_event); + timer_setup(&p_slot->task_event, + pushbutton_helper_thread, + 0); p_slot->hp_slot = hp_slot; p_slot->ctrl = ctrl; /* p_slot->physical_slot = physical_slot; */ p_slot->task_event.expires = jiffies + 5 * HZ; /* 5 second delay */ - p_slot->task_event.function = pushbutton_helper_thread; - p_slot->task_event.data = (u32) p_slot; dbg("add_timer p_slot = %p\n", p_slot); add_timer(&p_slot->task_event); @@ -1920,15 +1921,15 @@ static void interrupt_event_handler(struct controller *ctrl) * Scheduled procedure to handle blocking stuff for the pushbuttons. * Handles all pending events and exits. */ -void cpqhp_pushbutton_thread(unsigned long slot) +void cpqhp_pushbutton_thread(struct timer_list *t) { u8 hp_slot; u8 device; struct pci_func *func; - struct slot *p_slot = (struct slot *) slot; + struct slot *p_slot = from_timer(p_slot, t, task_event); struct controller *ctrl = (struct controller *) p_slot->ctrl; - pushbutton_pending = 0; + pushbutton_pending = NULL; hp_slot = p_slot->hp_slot; device = p_slot->device; -- cgit v1.2.3 From 3691314a905fd716de46735d944da3f374246550 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 20 Oct 2017 15:11:42 -0500 Subject: PCI: shpchp: Convert timers to use timer_setup() In preparation for unconditionally passing the struct timer_list pointer to all timer callbacks, switch to using the new timer_setup() and from_timer() to pass the timer pointer explicitly. Signed-off-by: Kees Cook Signed-off-by: Bjorn Helgaas Cc: Ingo Molnar Cc: Arvind Yadav Cc: Quentin Lambert Cc: Aleksandr Bezzubikov Cc: "Michael S. Tsirkin" Cc: Marcel Apfelbaum --- drivers/pci/hotplug/shpchp_hpc.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/hotplug/shpchp_hpc.c b/drivers/pci/hotplug/shpchp_hpc.c index e5824c7b7b6b..4810e9626d9f 100644 --- a/drivers/pci/hotplug/shpchp_hpc.c +++ b/drivers/pci/hotplug/shpchp_hpc.c @@ -229,14 +229,13 @@ static inline int shpc_indirect_read(struct controller *ctrl, int index, /* * This is the interrupt polling timeout function. */ -static void int_poll_timeout(unsigned long data) +static void int_poll_timeout(struct timer_list *t) { - struct controller *ctrl = (struct controller *)data; + struct controller *ctrl = from_timer(ctrl, t, poll_timer); /* Poll for interrupt events. regs == NULL => polling */ shpc_isr(0, ctrl); - init_timer(&ctrl->poll_timer); if (!shpchp_poll_time) shpchp_poll_time = 2; /* default polling interval is 2 sec */ @@ -252,8 +251,6 @@ static void start_int_poll_timer(struct controller *ctrl, int sec) if ((sec <= 0) || (sec > 60)) sec = 2; - ctrl->poll_timer.function = &int_poll_timeout; - ctrl->poll_timer.data = (unsigned long)ctrl; ctrl->poll_timer.expires = jiffies + sec * HZ; add_timer(&ctrl->poll_timer); } @@ -1054,7 +1051,7 @@ int shpc_init(struct controller *ctrl, struct pci_dev *pdev) if (shpchp_poll_mode) { /* Install interrupt polling timer. Start with 10 sec delay */ - init_timer(&ctrl->poll_timer); + timer_setup(&ctrl->poll_timer, int_poll_timeout, 0); start_int_poll_timer(ctrl, 10); } else { /* Installs the interrupt handler */ -- cgit v1.2.3 From 24a0c654d7d6063301c51361f911369264342b3c Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Fri, 20 Oct 2017 15:38:54 -0500 Subject: PCI: Add for_each_pci_bridge() helper The following pattern is often used: list_for_each_entry(dev, &bus->devices, bus_list) { if (pci_is_bridge(dev)) { ... } } Add a for_each_pci_bridge() helper to make that code easier to write and read by reducing indentation level. It also saves one or few lines of code in each occurrence. Convert PCI core parts here at the same time. Signed-off-by: Andy Shevchenko [bhelgaas: fold in http://lkml.kernel.org/r/20171013165352.25550-1-andriy.shevchenko@linux.intel.com] Signed-off-by: Bjorn Helgaas --- drivers/pci/hotplug/acpiphp_glue.c | 15 ++++++--------- drivers/pci/hotplug/cpci_hotplug_pci.c | 7 ++----- drivers/pci/hotplug/pciehp_pci.c | 5 ++--- drivers/pci/hotplug/shpchp_pci.c | 6 ++---- drivers/pci/probe.c | 6 ++---- drivers/pci/setup-bus.c | 7 +++---- 6 files changed, 17 insertions(+), 29 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c index 5ed2dcaa8e27..5db6f1839dad 100644 --- a/drivers/pci/hotplug/acpiphp_glue.c +++ b/drivers/pci/hotplug/acpiphp_glue.c @@ -462,18 +462,15 @@ static void enable_slot(struct acpiphp_slot *slot) acpiphp_rescan_slot(slot); max = acpiphp_max_busnr(bus); for (pass = 0; pass < 2; pass++) { - list_for_each_entry(dev, &bus->devices, bus_list) { + for_each_pci_bridge(dev, bus) { if (PCI_SLOT(dev->devfn) != slot->device) continue; - if (pci_is_bridge(dev)) { - max = pci_scan_bridge(bus, dev, max, pass); - if (pass && dev->subordinate) { - check_hotplug_bridge(slot, dev); - pcibios_resource_survey_bus(dev->subordinate); - __pci_bus_size_bridges(dev->subordinate, - &add_list); - } + max = pci_scan_bridge(bus, dev, max, pass); + if (pass && dev->subordinate) { + check_hotplug_bridge(slot, dev); + pcibios_resource_survey_bus(dev->subordinate); + __pci_bus_size_bridges(dev->subordinate, &add_list); } } } diff --git a/drivers/pci/hotplug/cpci_hotplug_pci.c b/drivers/pci/hotplug/cpci_hotplug_pci.c index 80c80017197d..f616358fa938 100644 --- a/drivers/pci/hotplug/cpci_hotplug_pci.c +++ b/drivers/pci/hotplug/cpci_hotplug_pci.c @@ -286,14 +286,11 @@ int cpci_configure_slot(struct slot *slot) } parent = slot->dev->bus; - list_for_each_entry(dev, &parent->devices, bus_list) { - if (PCI_SLOT(dev->devfn) != PCI_SLOT(slot->devfn)) - continue; - if (pci_is_bridge(dev)) + for_each_pci_bridge(dev, parent) { + if (PCI_SLOT(dev->devfn) == PCI_SLOT(slot->devfn)) pci_hp_add_bridge(dev); } - pci_assign_unassigned_bridge_resources(parent->self); pci_bus_add_devices(parent); diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c index 19f30a9f461d..c3af027ee1a6 100644 --- a/drivers/pci/hotplug/pciehp_pci.c +++ b/drivers/pci/hotplug/pciehp_pci.c @@ -60,9 +60,8 @@ int pciehp_configure_device(struct slot *p_slot) goto out; } - list_for_each_entry(dev, &parent->devices, bus_list) - if (pci_is_bridge(dev)) - pci_hp_add_bridge(dev); + for_each_pci_bridge(dev, parent) + pci_hp_add_bridge(dev); pci_assign_unassigned_bridge_resources(bridge); pcie_bus_configure_settings(parent); diff --git a/drivers/pci/hotplug/shpchp_pci.c b/drivers/pci/hotplug/shpchp_pci.c index f8cd3a27e351..ea63db58b4b1 100644 --- a/drivers/pci/hotplug/shpchp_pci.c +++ b/drivers/pci/hotplug/shpchp_pci.c @@ -61,10 +61,8 @@ int shpchp_configure_device(struct slot *p_slot) goto out; } - list_for_each_entry(dev, &parent->devices, bus_list) { - if (PCI_SLOT(dev->devfn) != p_slot->device) - continue; - if (pci_is_bridge(dev)) + for_each_pci_bridge(dev, parent) { + if (PCI_SLOT(dev->devfn) == p_slot->device) pci_hp_add_bridge(dev); } diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index ff94b69738a8..cdc2f83c11c5 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -2421,10 +2421,8 @@ unsigned int pci_scan_child_bus(struct pci_bus *bus) } for (pass = 0; pass < 2; pass++) - list_for_each_entry(dev, &bus->devices, bus_list) { - if (pci_is_bridge(dev)) - max = pci_scan_bridge(bus, dev, max, pass); - } + for_each_pci_bridge(dev, bus) + max = pci_scan_bridge(bus, dev, max, pass); /* * Make sure a hotplug bridge has at least the minimum requested diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 958da7db9033..7ca03407404c 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -1921,10 +1921,9 @@ void pci_assign_unassigned_bus_resources(struct pci_bus *bus) want additional resources */ down_read(&pci_bus_sem); - list_for_each_entry(dev, &bus->devices, bus_list) - if (pci_is_bridge(dev) && pci_has_subordinate(dev)) - __pci_bus_size_bridges(dev->subordinate, - &add_list); + for_each_pci_bridge(dev, bus) + if (pci_has_subordinate(dev)) + __pci_bus_size_bridges(dev->subordinate, &add_list); up_read(&pci_bus_sem); __pci_bus_assign_resources(bus, &add_list, NULL); BUG_ON(!list_empty(&add_list)); -- cgit v1.2.3 From 95e3ba9772331502cc33f1e1d4a96f3310e2f31e Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Fri, 13 Oct 2017 21:35:41 +0300 Subject: PCI: Move pci_hp_add_bridge() to drivers/pci/probe.c There is not much point of having a file with a single function in it. Instead we can just move pci_hp_add_bridge() to drivers/pci/probe.c and make it available always when PCI core is enabled. Signed-off-by: Mika Westerberg [bhelgaas: convert printk to dev_err()] Signed-off-by: Bjorn Helgaas --- drivers/pci/Makefile | 3 --- drivers/pci/hotplug-pci.c | 29 ----------------------------- drivers/pci/probe.c | 23 +++++++++++++++++++++++ 3 files changed, 23 insertions(+), 32 deletions(-) delete mode 100644 drivers/pci/hotplug-pci.c (limited to 'drivers/pci') diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 66a21acad952..fa56267fa2c0 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -16,9 +16,6 @@ obj-$(CONFIG_PCIEPORTBUS) += pcie/ # Build the PCI Hotplug drivers if we were asked to obj-$(CONFIG_HOTPLUG_PCI) += hotplug/ -ifdef CONFIG_HOTPLUG_PCI -obj-y += hotplug-pci.o -endif # Build the PCI MSI interrupt support obj-$(CONFIG_PCI_MSI) += msi.o diff --git a/drivers/pci/hotplug-pci.c b/drivers/pci/hotplug-pci.c deleted file mode 100644 index c68366cee6b7..000000000000 --- a/drivers/pci/hotplug-pci.c +++ /dev/null @@ -1,29 +0,0 @@ -/* Core PCI functionality used only by PCI hotplug */ - -#include -#include -#include "pci.h" - -int pci_hp_add_bridge(struct pci_dev *dev) -{ - struct pci_bus *parent = dev->bus; - int pass, busnr, start = parent->busn_res.start; - int end = parent->busn_res.end; - - for (busnr = start; busnr <= end; busnr++) { - if (!pci_find_bus(pci_domain_nr(parent), busnr)) - break; - } - if (busnr-- > end) { - printk(KERN_ERR "No bus number available for hot-added bridge %s\n", - pci_name(dev)); - return -1; - } - for (pass = 0; pass < 2; pass++) - busnr = pci_scan_bridge(parent, dev, busnr, pass); - if (!dev->subordinate) - return -1; - - return 0; -} -EXPORT_SYMBOL_GPL(pci_hp_add_bridge); diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index cdc2f83c11c5..62d45a76f663 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -2735,3 +2735,26 @@ void __init pci_sort_breadthfirst(void) { bus_sort_breadthfirst(&pci_bus_type, &pci_sort_bf_cmp); } + +int pci_hp_add_bridge(struct pci_dev *dev) +{ + struct pci_bus *parent = dev->bus; + int pass, busnr, start = parent->busn_res.start; + int end = parent->busn_res.end; + + for (busnr = start; busnr <= end; busnr++) { + if (!pci_find_bus(pci_domain_nr(parent), busnr)) + break; + } + if (busnr-- > end) { + dev_err(&dev->dev, "No bus number available for hot-added bridge\n"); + return -1; + } + for (pass = 0; pass < 2; pass++) + busnr = pci_scan_bridge(parent, dev, busnr, pass); + if (!dev->subordinate) + return -1; + + return 0; +} +EXPORT_SYMBOL_GPL(pci_hp_add_bridge); -- cgit v1.2.3 From 4147c2fd9b12ae1e0bdbb2dbb9a9163c94a10a22 Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Fri, 13 Oct 2017 21:35:42 +0300 Subject: PCI: Open-code the two pass loop when scanning bridges The current scanning code is really hard to understand because it calls the same function in a loop where pass value is changed without any comments explaining it: for (pass = 0; pass < 2; pass++) for_each_pci_bridge(dev, bus) max = pci_scan_bridge(bus, dev, max, pass); Unfamiliar reader cannot tell easily what is the purpose of this loop without looking at internals of pci_scan_bridge(). In order to make this bit easier to understand, open-code the loop in pci_scan_child_bus() and pci_hp_add_bridge() with added comments. No functional changes intended. Signed-off-by: Mika Westerberg Signed-off-by: Bjorn Helgaas --- drivers/pci/probe.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 62d45a76f663..61813938d186 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -2398,7 +2398,7 @@ void __weak pcibios_fixup_bus(struct pci_bus *bus) unsigned int pci_scan_child_bus(struct pci_bus *bus) { - unsigned int devfn, pass, max = bus->busn_res.start; + unsigned int devfn, max = bus->busn_res.start; struct pci_dev *dev; dev_dbg(&bus->dev, "scanning bus\n"); @@ -2420,9 +2420,17 @@ unsigned int pci_scan_child_bus(struct pci_bus *bus) bus->is_added = 1; } - for (pass = 0; pass < 2; pass++) - for_each_pci_bridge(dev, bus) - max = pci_scan_bridge(bus, dev, max, pass); + /* + * Scan bridges that are already configured. We don't touch them + * unless they are misconfigured (which will be done in the second + * scan below). + */ + for_each_pci_bridge(dev, bus) + max = pci_scan_bridge(bus, dev, max, 0); + + /* Scan bridges that need to be reconfigured */ + for_each_pci_bridge(dev, bus) + max = pci_scan_bridge(bus, dev, max, 1); /* * Make sure a hotplug bridge has at least the minimum requested @@ -2739,7 +2747,7 @@ void __init pci_sort_breadthfirst(void) int pci_hp_add_bridge(struct pci_dev *dev) { struct pci_bus *parent = dev->bus; - int pass, busnr, start = parent->busn_res.start; + int busnr, start = parent->busn_res.start; int end = parent->busn_res.end; for (busnr = start; busnr <= end; busnr++) { @@ -2750,8 +2758,13 @@ int pci_hp_add_bridge(struct pci_dev *dev) dev_err(&dev->dev, "No bus number available for hot-added bridge\n"); return -1; } - for (pass = 0; pass < 2; pass++) - busnr = pci_scan_bridge(parent, dev, busnr, pass); + + /* Scan bridges that are already configured */ + busnr = pci_scan_bridge(parent, dev, busnr, 0); + + /* Scan bridges that need to be reconfigured */ + pci_scan_bridge(parent, dev, busnr, 1); + if (!dev->subordinate) return -1; -- cgit v1.2.3 From a20c7f36bd3d20d245616ae223bb9d05dfb6f050 Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Fri, 13 Oct 2017 21:35:43 +0300 Subject: PCI: Do not allocate more buses than available in parent One can ask more buses to be reserved for hotplug bridges by passing pci=hpbussize=N in the kernel command line. If the parent bus does not have enough bus space available we incorrectly create child bus with the requested number of subordinate buses. In the example below hpbussize is set to one more than we have available buses in the root port: pci 0000:07:00.0: [8086:1578] type 01 class 0x060400 pci 0000:07:00.0: scanning [bus 00-00] behind bridge, pass 0 pci 0000:07:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring pci 0000:07:00.0: scanning [bus 00-00] behind bridge, pass 1 pci_bus 0000:08: busn_res: can not insert [bus 08-ff] under [bus 07-3f] (conflicts with (null) [bus 07-3f]) pci_bus 0000:08: scanning bus ... pci_bus 0000:0a: bus scan returning with max=40 pci_bus 0000:0a: busn_res: [bus 0a-ff] end is updated to 40 pci_bus 0000:0a: [bus 0a-40] partially hidden behind bridge 0000:07 [bus 07-3f] pci_bus 0000:08: bus scan returning with max=40 pci_bus 0000:08: busn_res: [bus 08-ff] end is updated to 40 Instead of allowing this, limit the subordinate number to be less than or equal the maximum subordinate number allocated for the parent bus (if it has any). Signed-off-by: Mika Westerberg [bhelgaas: remove irrelevant dmesg messages] Signed-off-by: Bjorn Helgaas --- drivers/pci/probe.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'drivers/pci') diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 61813938d186..1f82f49c0bb3 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1076,7 +1076,8 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) child = pci_add_new_bus(bus, dev, max+1); if (!child) goto out; - pci_bus_insert_busn_res(child, max+1, 0xff); + pci_bus_insert_busn_res(child, max+1, + bus->busn_res.end); } max++; buses = (buses & 0xff000000) @@ -2439,6 +2440,10 @@ unsigned int pci_scan_child_bus(struct pci_bus *bus) if (bus->self && bus->self->is_hotplug_bridge && pci_hotplug_bus_size) { if (max - bus->busn_res.start < pci_hotplug_bus_size - 1) max = bus->busn_res.start + pci_hotplug_bus_size - 1; + + /* Do not allocate more buses than we have room left */ + if (max > bus->busn_res.end) + max = bus->busn_res.end; } /* -- cgit v1.2.3 From 1c02ea81006548a5cd55f7ac6df77cf8d7b08e00 Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Fri, 13 Oct 2017 21:35:44 +0300 Subject: PCI: Distribute available buses to hotplug-capable bridges System BIOS sometimes allocates extra bus space for hotplug-capable PCIe root/downstream ports. This space is needed if the device plugged to the port will have more hotplug-capable downstream ports. A good example of this is Thunderbolt. Each Thunderbolt device contains a PCIe switch and one or more hotplug-capable PCIe downstream ports where the daisy chain can be extended. Currently Linux only allocates minimal bus space to make sure all the enumerated devices barely fit there. The BIOS reserved extra space is not taken into consideration at all. Because of this we run out of bus space pretty quickly when more PCIe devices are attached to hotplug downstream ports in order to extend the chain. Modify the PCI core so we distribute the available BIOS allocated bus space equally between hotplug-capable bridges to make sure there is enough bus space for extending the hierarchy later on. Update kernel docs of the affected functions. Signed-off-by: Mika Westerberg Signed-off-by: Bjorn Helgaas --- drivers/pci/probe.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 138 insertions(+), 18 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 1f82f49c0bb3..14e0ea1ff38b 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -959,7 +959,21 @@ static void pci_enable_crs(struct pci_dev *pdev) PCI_EXP_RTCTL_CRSSVE); } +static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus, + unsigned int available_buses); + /* + * pci_scan_bridge_extend() - Scan buses behind a bridge + * @bus: Parent bus the bridge is on + * @dev: Bridge itself + * @max: Starting subordinate number of buses behind this bridge + * @available_buses: Total number of buses available for this bridge and + * the devices below. After the minimal bus space has + * been allocated the remaining buses will be + * distributed equally between hotplug-capable bridges. + * @pass: Either %0 (scan already configured bridges) or %1 (scan bridges + * that need to be reconfigured. + * * If it's a bridge, configure it and scan the bus behind it. * For CardBus bridges, we don't scan behind as the devices will * be handled by the bridge driver itself. @@ -969,7 +983,9 @@ static void pci_enable_crs(struct pci_dev *pdev) * them, we proceed to assigning numbers to the remaining buses in * order to avoid overlaps between old and new bus numbers. */ -int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) +static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev, + int max, unsigned int available_buses, + int pass) { struct pci_bus *child; int is_cardbus = (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS); @@ -1080,6 +1096,9 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) bus->busn_res.end); } max++; + if (available_buses) + available_buses--; + buses = (buses & 0xff000000) | ((unsigned int)(child->primary) << 0) | ((unsigned int)(child->busn_res.start) << 8) @@ -1101,7 +1120,7 @@ int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) if (!is_cardbus) { child->bridge_ctl = bctl; - max = pci_scan_child_bus(child); + max = pci_scan_child_bus_extend(child, available_buses); } else { /* * For CardBus bridges, we leave 4 bus numbers @@ -1169,6 +1188,28 @@ out: return max; } + +/* + * pci_scan_bridge() - Scan buses behind a bridge + * @bus: Parent bus the bridge is on + * @dev: Bridge itself + * @max: Starting subordinate number of buses behind this bridge + * @pass: Either %0 (scan already configured bridges) or %1 (scan bridges + * that need to be reconfigured. + * + * If it's a bridge, configure it and scan the bus behind it. + * For CardBus bridges, we don't scan behind as the devices will + * be handled by the bridge driver itself. + * + * We need to process bridges in two passes -- first we scan those + * already configured by the BIOS and after we are done with all of + * them, we proceed to assigning numbers to the remaining buses in + * order to avoid overlaps between old and new bus numbers. + */ +int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass) +{ + return pci_scan_bridge_extend(bus, dev, max, 0, pass); +} EXPORT_SYMBOL(pci_scan_bridge); /* @@ -2397,9 +2438,24 @@ void __weak pcibios_fixup_bus(struct pci_bus *bus) /* nothing to do, expected to be removed in the future */ } -unsigned int pci_scan_child_bus(struct pci_bus *bus) +/** + * pci_scan_child_bus_extend() - Scan devices below a bus + * @bus: Bus to scan for devices + * @available_buses: Total number of buses available (%0 does not try to + * extend beyond the minimal) + * + * Scans devices below @bus including subordinate buses. Returns new + * subordinate number including all the found devices. Passing + * @available_buses causes the remaining bus space to be distributed + * equally between hotplug-capable bridges to allow future extension of the + * hierarchy. + */ +static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus, + unsigned int available_buses) { - unsigned int devfn, max = bus->busn_res.start; + unsigned int used_buses, normal_bridges = 0, hotplug_bridges = 0; + unsigned int start = bus->busn_res.start; + unsigned int devfn, cmax, max = start; struct pci_dev *dev; dev_dbg(&bus->dev, "scanning bus\n"); @@ -2409,7 +2465,8 @@ unsigned int pci_scan_child_bus(struct pci_bus *bus) pci_scan_slot(bus, devfn); /* Reserve buses for SR-IOV capability. */ - max += pci_iov_bus_range(bus); + used_buses = pci_iov_bus_range(bus); + max += used_buses; /* * After performing arch-dependent fixup of the bus, look behind @@ -2421,29 +2478,73 @@ unsigned int pci_scan_child_bus(struct pci_bus *bus) bus->is_added = 1; } + /* + * Calculate how many hotplug bridges and normal bridges there + * are on this bus. We will distribute the additional available + * buses between hotplug bridges. + */ + for_each_pci_bridge(dev, bus) { + if (dev->is_hotplug_bridge) + hotplug_bridges++; + else + normal_bridges++; + } + /* * Scan bridges that are already configured. We don't touch them * unless they are misconfigured (which will be done in the second * scan below). */ - for_each_pci_bridge(dev, bus) - max = pci_scan_bridge(bus, dev, max, 0); + for_each_pci_bridge(dev, bus) { + cmax = max; + max = pci_scan_bridge_extend(bus, dev, max, 0, 0); + used_buses += cmax - max; + } /* Scan bridges that need to be reconfigured */ - for_each_pci_bridge(dev, bus) - max = pci_scan_bridge(bus, dev, max, 1); + for_each_pci_bridge(dev, bus) { + unsigned int buses = 0; + + if (!hotplug_bridges && normal_bridges == 1) { + /* + * There is only one bridge on the bus (upstream + * port) so it gets all available buses which it + * can then distribute to the possible hotplug + * bridges below. + */ + buses = available_buses; + } else if (dev->is_hotplug_bridge) { + /* + * Distribute the extra buses between hotplug + * bridges if any. + */ + buses = available_buses / hotplug_bridges; + buses = min(buses, available_buses - used_buses); + } + + cmax = max; + max = pci_scan_bridge_extend(bus, dev, cmax, buses, 1); + used_buses += max - cmax; + } /* * Make sure a hotplug bridge has at least the minimum requested - * number of buses. + * number of buses but allow it to grow up to the maximum available + * bus number of there is room. */ - if (bus->self && bus->self->is_hotplug_bridge && pci_hotplug_bus_size) { - if (max - bus->busn_res.start < pci_hotplug_bus_size - 1) - max = bus->busn_res.start + pci_hotplug_bus_size - 1; - - /* Do not allocate more buses than we have room left */ - if (max > bus->busn_res.end) - max = bus->busn_res.end; + if (bus->self && bus->self->is_hotplug_bridge) { + used_buses = max_t(unsigned int, available_buses, + pci_hotplug_bus_size - 1); + if (max - start < used_buses) { + max = start + used_buses; + + /* Do not allocate more buses than we have room left */ + if (max > bus->busn_res.end) + max = bus->busn_res.end; + + dev_dbg(&bus->dev, "%pR extended by %#02x\n", + &bus->busn_res, max - start); + } } /* @@ -2456,6 +2557,18 @@ unsigned int pci_scan_child_bus(struct pci_bus *bus) dev_dbg(&bus->dev, "bus scan returning with max=%02x\n", max); return max; } + +/** + * pci_scan_child_bus() - Scan devices below a bus + * @bus: Bus to scan for devices + * + * Scans devices below @bus including subordinate buses. Returns new + * subordinate number including all the found devices. + */ +unsigned int pci_scan_child_bus(struct pci_bus *bus) +{ + return pci_scan_child_bus_extend(bus, 0); +} EXPORT_SYMBOL_GPL(pci_scan_child_bus); /** @@ -2753,6 +2866,7 @@ int pci_hp_add_bridge(struct pci_dev *dev) { struct pci_bus *parent = dev->bus; int busnr, start = parent->busn_res.start; + unsigned int available_buses = 0; int end = parent->busn_res.end; for (busnr = start; busnr <= end; busnr++) { @@ -2767,8 +2881,14 @@ int pci_hp_add_bridge(struct pci_dev *dev) /* Scan bridges that are already configured */ busnr = pci_scan_bridge(parent, dev, busnr, 0); + /* + * Distribute the available bus numbers between hotplug-capable + * bridges to make extending the chain later possible. + */ + available_buses = end - busnr; + /* Scan bridges that need to be reconfigured */ - pci_scan_bridge(parent, dev, busnr, 1); + pci_scan_bridge_extend(parent, dev, busnr, available_buses, 1); if (!dev->subordinate) return -1; -- cgit v1.2.3 From 1a5767725cecedd80a541799c83c0c97b8b5b624 Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Fri, 13 Oct 2017 21:35:45 +0300 Subject: PCI: Distribute available resources to hotplug-capable bridges The same problem that we have with bus space applies to other resources as well. Linux only allocates the minimal amount of resources so that the devices currently present barely fit there. This prevents extending the chain later on because the resource windows allocated for hotplug downstream ports are too small. Follow what we already did for bus number and assign all available extra resources to hotplug-capable bridges. This makes it possible to extend the hierarchy later. Signed-off-by: Mika Westerberg Signed-off-by: Bjorn Helgaas --- drivers/pci/setup-bus.c | 177 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 177 insertions(+) (limited to 'drivers/pci') diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 7ca03407404c..5e547e0dc47b 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -1853,6 +1853,175 @@ void __init pci_assign_unassigned_resources(void) } } +static void extend_bridge_window(struct pci_dev *bridge, struct resource *res, + struct list_head *add_list, resource_size_t available) +{ + struct pci_dev_resource *dev_res; + + if (res->parent) + return; + + if (resource_size(res) >= available) + return; + + dev_res = res_to_dev_res(add_list, res); + if (!dev_res) + return; + + /* Is there room to extend the window? */ + if (available - resource_size(res) <= dev_res->add_size) + return; + + dev_res->add_size = available - resource_size(res); + dev_dbg(&bridge->dev, "bridge window %pR extended by %pa\n", res, + &dev_res->add_size); +} + +static void pci_bus_distribute_available_resources(struct pci_bus *bus, + struct list_head *add_list, resource_size_t available_io, + resource_size_t available_mmio, resource_size_t available_mmio_pref) +{ + resource_size_t remaining_io, remaining_mmio, remaining_mmio_pref; + unsigned int normal_bridges = 0, hotplug_bridges = 0; + struct resource *io_res, *mmio_res, *mmio_pref_res; + struct pci_dev *dev, *bridge = bus->self; + + io_res = &bridge->resource[PCI_BRIDGE_RESOURCES + 0]; + mmio_res = &bridge->resource[PCI_BRIDGE_RESOURCES + 1]; + mmio_pref_res = &bridge->resource[PCI_BRIDGE_RESOURCES + 2]; + + /* + * Update additional resource list (add_list) to fill all the + * extra resource space available for this port except the space + * calculated in __pci_bus_size_bridges() which covers all the + * devices currently connected to the port and below. + */ + extend_bridge_window(bridge, io_res, add_list, available_io); + extend_bridge_window(bridge, mmio_res, add_list, available_mmio); + extend_bridge_window(bridge, mmio_pref_res, add_list, + available_mmio_pref); + + /* + * Calculate the total amount of extra resource space we can + * pass to bridges below this one. This is basically the + * extra space reduced by the minimal required space for the + * non-hotplug bridges. + */ + remaining_io = available_io; + remaining_mmio = available_mmio; + remaining_mmio_pref = available_mmio_pref; + + /* + * Calculate how many hotplug bridges and normal bridges there + * are on this bus. We will distribute the additional available + * resources between hotplug bridges. + */ + for_each_pci_bridge(dev, bus) { + if (dev->is_hotplug_bridge) + hotplug_bridges++; + else + normal_bridges++; + } + + for_each_pci_bridge(dev, bus) { + const struct resource *res; + + if (dev->is_hotplug_bridge) + continue; + + /* + * Reduce the available resource space by what the + * bridge and devices below it occupy. + */ + res = &dev->resource[PCI_BRIDGE_RESOURCES + 0]; + if (!res->parent && available_io > resource_size(res)) + remaining_io -= resource_size(res); + + res = &dev->resource[PCI_BRIDGE_RESOURCES + 1]; + if (!res->parent && available_mmio > resource_size(res)) + remaining_mmio -= resource_size(res); + + res = &dev->resource[PCI_BRIDGE_RESOURCES + 2]; + if (!res->parent && available_mmio_pref > resource_size(res)) + remaining_mmio_pref -= resource_size(res); + } + + /* + * Go over devices on this bus and distribute the remaining + * resource space between hotplug bridges. + */ + for_each_pci_bridge(dev, bus) { + struct pci_bus *b; + + b = dev->subordinate; + if (!b) + continue; + + if (!hotplug_bridges && normal_bridges == 1) { + /* + * There is only one bridge on the bus (upstream + * port) so it gets all available resources + * which it can then distribute to the possible + * hotplug bridges below. + */ + pci_bus_distribute_available_resources(b, add_list, + available_io, available_mmio, + available_mmio_pref); + } else if (dev->is_hotplug_bridge) { + resource_size_t align, io, mmio, mmio_pref; + + /* + * Distribute available extra resources equally + * between hotplug-capable downstream ports + * taking alignment into account. + * + * Here hotplug_bridges is always != 0. + */ + align = pci_resource_alignment(bridge, io_res); + io = div64_ul(available_io, hotplug_bridges); + io = min(ALIGN(io, align), remaining_io); + remaining_io -= io; + + align = pci_resource_alignment(bridge, mmio_res); + mmio = div64_ul(available_mmio, hotplug_bridges); + mmio = min(ALIGN(mmio, align), remaining_mmio); + remaining_mmio -= mmio; + + align = pci_resource_alignment(bridge, mmio_pref_res); + mmio_pref = div64_ul(available_mmio_pref, + hotplug_bridges); + mmio_pref = min(ALIGN(mmio_pref, align), + remaining_mmio_pref); + remaining_mmio_pref -= mmio_pref; + + pci_bus_distribute_available_resources(b, add_list, io, + mmio, mmio_pref); + } + } +} + +static void +pci_bridge_distribute_available_resources(struct pci_dev *bridge, + struct list_head *add_list) +{ + resource_size_t available_io, available_mmio, available_mmio_pref; + const struct resource *res; + + if (!bridge->is_hotplug_bridge) + return; + + /* Take the initial extra resources from the hotplug port */ + res = &bridge->resource[PCI_BRIDGE_RESOURCES + 0]; + available_io = resource_size(res); + res = &bridge->resource[PCI_BRIDGE_RESOURCES + 1]; + available_mmio = resource_size(res); + res = &bridge->resource[PCI_BRIDGE_RESOURCES + 2]; + available_mmio_pref = resource_size(res); + + pci_bus_distribute_available_resources(bridge->subordinate, + add_list, available_io, available_mmio, available_mmio_pref); +} + void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge) { struct pci_bus *parent = bridge->subordinate; @@ -1867,6 +2036,14 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge) again: __pci_bus_size_bridges(parent, &add_list); + + /* + * Distribute remaining resources (if any) equally between + * hotplug bridges below. This makes it possible to extend the + * hierarchy later without running out of resources. + */ + pci_bridge_distribute_available_resources(bridge, &add_list); + __pci_bridge_assign_resources(bridge, &add_list, &fail_head); BUG_ON(!list_empty(&add_list)); tried_times++; -- cgit v1.2.3 From 499022396a381d1b681105b74ac366017ce051e9 Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Fri, 13 Oct 2017 21:35:46 +0300 Subject: PCI: pciehp: Fix race condition handling surprise link down A surprise link down may retrain very quickly causing the same slot generate a link up event before handling the link down event completes. Since the link is active, the power off work queued from the first link down will cause a second down event when power is disabled. However, the link up event sets the slot state to POWERON_STATE before the event to handle this is enqueued, making the second down event believe it needs to do something. This creates constant link up and down event cycle. To prevent this it is better to handle each event at the time in order it occurred, so change the driver to use ordered workqueue instead. A normal device hotplug triggers two events (presense detect and link up) that are already handled properly in the driver but we currently log an error if we find an existing device in the slot. Since this is not an error change the log level to be debug instead to avoid scaring users. This is based on the original work by Ashok Raj. Link: https://patchwork.kernel.org/patch/9469023 Suggested-by: Bjorn Helgaas Signed-off-by: Mika Westerberg Signed-off-by: Bjorn Helgaas --- drivers/pci/hotplug/pciehp_ctrl.c | 7 ++++--- drivers/pci/hotplug/pciehp_hpc.c | 2 +- drivers/pci/hotplug/pciehp_pci.c | 6 +++++- 3 files changed, 10 insertions(+), 5 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c index ec0b4c11ccd9..83f3d4af3677 100644 --- a/drivers/pci/hotplug/pciehp_ctrl.c +++ b/drivers/pci/hotplug/pciehp_ctrl.c @@ -113,10 +113,11 @@ static int board_added(struct slot *p_slot) retval = pciehp_configure_device(p_slot); if (retval) { - ctrl_err(ctrl, "Cannot add device at %04x:%02x:00\n", - pci_domain_nr(parent), parent->number); - if (retval != -EEXIST) + if (retval != -EEXIST) { + ctrl_err(ctrl, "Cannot add device at %04x:%02x:00\n", + pci_domain_nr(parent), parent->number); goto err_exit; + } } pciehp_green_led_on(p_slot); diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index ba5055c5115c..fd0877e92b05 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -791,7 +791,7 @@ static int pcie_init_slot(struct controller *ctrl) if (!slot) return -ENOMEM; - slot->wq = alloc_workqueue("pciehp-%u", 0, 0, PSN(ctrl)); + slot->wq = alloc_ordered_workqueue("pciehp-%u", 0, PSN(ctrl)); if (!slot->wq) goto abort; diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c index c3af027ee1a6..2a1ca020cf5a 100644 --- a/drivers/pci/hotplug/pciehp_pci.c +++ b/drivers/pci/hotplug/pciehp_pci.c @@ -46,7 +46,11 @@ int pciehp_configure_device(struct slot *p_slot) dev = pci_get_slot(parent, PCI_DEVFN(0, 0)); if (dev) { - ctrl_err(ctrl, "Device %s already exists at %04x:%02x:00, cannot hot-add\n", + /* + * The device is already there. Either configured by the + * boot firmware or a previous hotplug event. + */ + ctrl_dbg(ctrl, "Device %s already exists at %04x:%02x:00, skipping hot-add\n", pci_name(dev), pci_domain_nr(parent), parent->number); pci_dev_put(dev); ret = -EEXIST; -- cgit v1.2.3 From db63d40017a523d127ec78328fed643918c7c54c Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Fri, 13 Oct 2017 21:35:47 +0300 Subject: PCI: pciehp: Do not clear Presence Detect Changed during initialization It is possible that the hotplug event has already happened before the driver is attached to a PCIe hotplug downstream port. If we just clear the status we never get the hotplug interrupt and thus the event will be missed. To make sure that does not happen, we leave Presence Detect Changed bit untouched during initialization. Then once the event is unmasked we get an interrupt and handle the hotplug event properly. Signed-off-by: Mika Westerberg Signed-off-by: Bjorn Helgaas --- drivers/pci/hotplug/pciehp_hpc.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index fd0877e92b05..7bab0606f1a9 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -858,11 +858,16 @@ struct controller *pcie_init(struct pcie_device *dev) if (link_cap & PCI_EXP_LNKCAP_DLLLARC) ctrl->link_active_reporting = 1; - /* Clear all remaining event bits in Slot Status register */ + /* + * Clear all remaining event bits in Slot Status register except + * Presence Detect Changed. We want to make sure possible + * hotplug event is triggered when the interrupt is unmasked so + * that we don't lose that event. + */ pcie_capability_write_word(pdev, PCI_EXP_SLTSTA, PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD | - PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC | - PCI_EXP_SLTSTA_CC | PCI_EXP_SLTSTA_DLLSC); + PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_CC | + PCI_EXP_SLTSTA_DLLSC); ctrl_info(ctrl, "Slot #%d AttnBtn%c PwrCtrl%c MRL%c AttnInd%c PwrInd%c HotPlug%c Surprise%c Interlock%c NoCompl%c LLActRep%c\n", (slot_cap & PCI_EXP_SLTCAP_PSN) >> 19, -- cgit v1.2.3