Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

PCI: pciehp: Drop pointless ACPI-based "slot detection" check

Jarod Wilson reports that ExpressCard hotplug doesn't work on HP ZBook G2.
The problem turns out to be the ACPI-based "slot detection" code called
from pciehp_probe() which uses questionable heuristics based on what ACPI
objects are present for the PCIe port device to figure out whether to
register a hotplug slot for that port.

That code is used if there is at least one PCIe port having an ACPI device
configuration object related to hotplug (such as _EJ0 or _RMV), and the
Thunderbolt port on the ZBook has _RMV. Of course, Thunderbolt and PCIe
native hotplug need not be mutually exclusive (as they aren't on the
ZBook), so that rule is simply incorrect.

Moreover, the ACPI-based "slot detection" check does not add any value if
pciehp_probe() is called at all and the service type of the device object
it has been called for is PCIE_PORT_SERVICE_HP, because PCIe hotplug
services are only registered if the _OSC handshake in acpi_pci_root_add()
allows the kernel to control the PCIe native hotplug feature. No more
checks need to be carried out to decide whether or not to register a native
PCIe hotlug slot in that case.

For the above reasons, make pciehp_probe() check if it has been called for
the right service type and drop the pointless ACPI-based "slot detection"
check from it. Also remove the entire code whose only user is that check
(the entire pciehp_acpi.c file goes away as a result) and drop function
headers related to it from the internal pciehp header file.

Link: http://lkml.kernel.org/r/1431632038-39917-1-git-send-email-jarod@redhat.com
Link: https://bugzilla.kernel.org/show_bug.cgi?id=98581
Reported-by: Jarod Wilson <jarod@redhat.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Jarod Wilson <jarod@redhat.com>
Tested-by: Jarod Wilson <jarod@redhat.com>

authored by

Rafael J. Wysocki and committed by
Bjorn Helgaas
e705c295 5ebe6afa

+3 -164
-3
drivers/pci/hotplug/Makefile
··· 61 61 pciehp_ctrl.o \ 62 62 pciehp_pci.o \ 63 63 pciehp_hpc.o 64 - ifdef CONFIG_ACPI 65 - pciehp-objs += pciehp_acpi.o 66 - endif 67 64 68 65 shpchp-objs := shpchp_core.o \ 69 66 shpchp_ctrl.o \
-17
drivers/pci/hotplug/pciehp.h
··· 167 167 return hotplug_slot_name(slot->hotplug_slot); 168 168 } 169 169 170 - #ifdef CONFIG_ACPI 171 - #include <linux/pci-acpi.h> 172 - 173 - void __init pciehp_acpi_slot_detection_init(void); 174 - int pciehp_acpi_slot_detection_check(struct pci_dev *dev); 175 - 176 - static inline void pciehp_firmware_init(void) 177 - { 178 - pciehp_acpi_slot_detection_init(); 179 - } 180 - #else 181 - #define pciehp_firmware_init() do {} while (0) 182 - static inline int pciehp_acpi_slot_detection_check(struct pci_dev *dev) 183 - { 184 - return 0; 185 - } 186 - #endif /* CONFIG_ACPI */ 187 170 #endif /* _PCIEHP_H */
-137
drivers/pci/hotplug/pciehp_acpi.c
··· 1 - /* 2 - * ACPI related functions for PCI Express Hot Plug driver. 3 - * 4 - * Copyright (C) 2008 Kenji Kaneshige 5 - * Copyright (C) 2008 Fujitsu Limited. 6 - * 7 - * All rights reserved. 8 - * 9 - * This program is free software; you can redistribute it and/or modify 10 - * it under the terms of the GNU General Public License as published by 11 - * the Free Software Foundation; either version 2 of the License, or (at 12 - * your option) any later version. 13 - * 14 - * This program is distributed in the hope that it will be useful, but 15 - * WITHOUT ANY WARRANTY; without even the implied warranty of 16 - * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or 17 - * NON INFRINGEMENT. See the GNU General Public License for more 18 - * details. 19 - * 20 - * You should have received a copy of the GNU General Public License 21 - * along with this program; if not, write to the Free Software 22 - * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. 23 - * 24 - */ 25 - 26 - #include <linux/acpi.h> 27 - #include <linux/pci.h> 28 - #include <linux/pci_hotplug.h> 29 - #include <linux/slab.h> 30 - #include <linux/module.h> 31 - #include "pciehp.h" 32 - 33 - #define PCIEHP_DETECT_PCIE (0) 34 - #define PCIEHP_DETECT_ACPI (1) 35 - #define PCIEHP_DETECT_AUTO (2) 36 - #define PCIEHP_DETECT_DEFAULT PCIEHP_DETECT_AUTO 37 - 38 - struct dummy_slot { 39 - u32 number; 40 - struct list_head list; 41 - }; 42 - 43 - static int slot_detection_mode; 44 - static char *pciehp_detect_mode; 45 - module_param(pciehp_detect_mode, charp, 0444); 46 - MODULE_PARM_DESC(pciehp_detect_mode, 47 - "Slot detection mode: pcie, acpi, auto\n" 48 - " pcie - Use PCIe based slot detection\n" 49 - " acpi - Use ACPI for slot detection\n" 50 - " auto(default) - Auto select mode. Use acpi option if duplicate\n" 51 - " slot ids are found. Otherwise, use pcie option\n"); 52 - 53 - int pciehp_acpi_slot_detection_check(struct pci_dev *dev) 54 - { 55 - if (slot_detection_mode != PCIEHP_DETECT_ACPI) 56 - return 0; 57 - if (acpi_pci_detect_ejectable(ACPI_HANDLE(&dev->dev))) 58 - return 0; 59 - return -ENODEV; 60 - } 61 - 62 - static int __init parse_detect_mode(void) 63 - { 64 - if (!pciehp_detect_mode) 65 - return PCIEHP_DETECT_DEFAULT; 66 - if (!strcmp(pciehp_detect_mode, "pcie")) 67 - return PCIEHP_DETECT_PCIE; 68 - if (!strcmp(pciehp_detect_mode, "acpi")) 69 - return PCIEHP_DETECT_ACPI; 70 - if (!strcmp(pciehp_detect_mode, "auto")) 71 - return PCIEHP_DETECT_AUTO; 72 - warn("bad specifier '%s' for pciehp_detect_mode. Use default\n", 73 - pciehp_detect_mode); 74 - return PCIEHP_DETECT_DEFAULT; 75 - } 76 - 77 - static int __initdata dup_slot_id; 78 - static int __initdata acpi_slot_detected; 79 - static struct list_head __initdata dummy_slots = LIST_HEAD_INIT(dummy_slots); 80 - 81 - /* Dummy driver for duplicate name detection */ 82 - static int __init dummy_probe(struct pcie_device *dev) 83 - { 84 - u32 slot_cap; 85 - acpi_handle handle; 86 - struct dummy_slot *slot, *tmp; 87 - struct pci_dev *pdev = dev->port; 88 - 89 - pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP, &slot_cap); 90 - slot = kzalloc(sizeof(*slot), GFP_KERNEL); 91 - if (!slot) 92 - return -ENOMEM; 93 - slot->number = (slot_cap & PCI_EXP_SLTCAP_PSN) >> 19; 94 - list_for_each_entry(tmp, &dummy_slots, list) { 95 - if (tmp->number == slot->number) 96 - dup_slot_id++; 97 - } 98 - list_add_tail(&slot->list, &dummy_slots); 99 - handle = ACPI_HANDLE(&pdev->dev); 100 - if (!acpi_slot_detected && acpi_pci_detect_ejectable(handle)) 101 - acpi_slot_detected = 1; 102 - return -ENODEV; /* dummy driver always returns error */ 103 - } 104 - 105 - static struct pcie_port_service_driver __initdata dummy_driver = { 106 - .name = "pciehp_dummy", 107 - .port_type = PCIE_ANY_PORT, 108 - .service = PCIE_PORT_SERVICE_HP, 109 - .probe = dummy_probe, 110 - }; 111 - 112 - static int __init select_detection_mode(void) 113 - { 114 - struct dummy_slot *slot, *tmp; 115 - 116 - if (pcie_port_service_register(&dummy_driver)) 117 - return PCIEHP_DETECT_ACPI; 118 - pcie_port_service_unregister(&dummy_driver); 119 - list_for_each_entry_safe(slot, tmp, &dummy_slots, list) { 120 - list_del(&slot->list); 121 - kfree(slot); 122 - } 123 - if (acpi_slot_detected && dup_slot_id) 124 - return PCIEHP_DETECT_ACPI; 125 - return PCIEHP_DETECT_PCIE; 126 - } 127 - 128 - void __init pciehp_acpi_slot_detection_init(void) 129 - { 130 - slot_detection_mode = parse_detect_mode(); 131 - if (slot_detection_mode != PCIEHP_DETECT_AUTO) 132 - goto out; 133 - slot_detection_mode = select_detection_mode(); 134 - out: 135 - if (slot_detection_mode == PCIEHP_DETECT_ACPI) 136 - info("Using ACPI for slot detection.\n"); 137 - }
+3 -7
drivers/pci/hotplug/pciehp_core.c
··· 248 248 struct slot *slot; 249 249 u8 occupied, poweron; 250 250 251 - if (pciehp_force) 252 - dev_info(&dev->device, 253 - "Bypassing BIOS check for pciehp use on %s\n", 254 - pci_name(dev->port)); 255 - else if (pciehp_acpi_slot_detection_check(dev->port)) 256 - goto err_out_none; 251 + /* If this is not a "hotplug" service, we have no business here. */ 252 + if (dev->service != PCIE_PORT_SERVICE_HP) 253 + return -ENODEV; 257 254 258 255 if (!dev->port->subordinate) { 259 256 /* Can happen if we run out of bus numbers during probe */ ··· 363 366 { 364 367 int retval = 0; 365 368 366 - pciehp_firmware_init(); 367 369 retval = pcie_port_service_register(&hpdriver_portdrv); 368 370 dbg("pcie_port_service_register = %d\n", retval); 369 371 info(DRIVER_DESC " version: " DRIVER_VERSION "\n");