Bug 60736 - acpiphp and pciehp not working together on Thinkpad X200s
Summary: acpiphp and pciehp not working together on Thinkpad X200s
Status: RESOLVED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: PCI (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: drivers_pci@kernel-bugs.osdl.org
URL: http://lkml.kernel.org.r/520747B6.801...
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-12 17:15 UTC by Bjorn Helgaas
Modified: 2013-09-23 18:22 UTC (History)
2 users (show)

See Also:
Kernel Version: 3.11-rc4
Subsystem:
Regression: Yes
Bisected commit-id:


Attachments
dmesg without acpiphp.disable=1 (non-working case) (55.23 KB, text/plain)
2013-08-12 21:22 UTC, Stefan Seyfried
Details
dmesg with acpiphp.disable=1 (working case) (55.82 KB, text/plain)
2013-08-12 21:23 UTC, Stefan Seyfried
Details
tarball of binary acpidump from X200s (26.80 KB, application/gzip)
2013-08-12 21:24 UTC, Stefan Seyfried
Details
plain text ascii version of acpidump (gzip'ed) (79.93 KB, application/gzip)
2013-08-12 21:25 UTC, Stefan Seyfried
Details
"lspci -vvv" of my x200s (25.24 KB, text/plain)
2013-08-29 04:22 UTC, Stefan Seyfried
Details
patch from Neil Horman (5.11 KB, patch)
2013-08-30 16:56 UTC, Bjorn Helgaas
Details | Diff
dmesg of working minimal boot of 3.12rc1 (54.67 KB, text/plain)
2013-09-23 09:11 UTC, Stefan Seyfried
Details

Description Bjorn Helgaas 2013-08-12 17:15:23 UTC
Stefan Seyfried <stefan.seyfried@googlemail.com> reported:

after openSUSE changed its kernel config to statically build in acpiphp
and pciehp, my ExpressCard slot does not work anymore. The workaround
is to disable acpiphp with acpiphp.disable=1
Comment 1 Stefan Seyfried 2013-08-12 21:22:07 UTC
Created attachment 107192 [details]
dmesg without acpiphp.disable=1 (non-working case)

dmesg minimal boot of 3.11rc4 without acpiphp.disable=1
Comment 2 Stefan Seyfried 2013-08-12 21:23:18 UTC
Created attachment 107193 [details]
dmesg with acpiphp.disable=1 (working case)

dmesg of minimal boot with acpiphp.disable=1. After booting, I inserted my ExpressCard.
Comment 3 Stefan Seyfried 2013-08-12 21:24:23 UTC
Created attachment 107194 [details]
tarball of binary acpidump from X200s

acpidump -b
Comment 4 Stefan Seyfried 2013-08-12 21:25:23 UTC
Created attachment 107195 [details]
plain text ascii version of acpidump (gzip'ed)
Comment 5 Stefan Seyfried 2013-08-12 21:26:57 UTC
For reference: I tried an older SUSE kernel which has acpiphp and pciehp as modules.
It is 3.7.10 (actually it's the openSUSE 12.3 kernel).

Trying to load the acpiphp module there fails with ENODEV, so only pciehp can be loaded on that one.
Comment 6 Stefan Seyfried 2013-08-29 04:22:42 UTC
Created attachment 107351 [details]
"lspci -vvv" of my x200s

As requested by Bjorn, "lspci -vvv", the ExpressCard is not present.
Comment 7 Neil Horman 2013-08-30 11:25:08 UTC
fixed in commit 3dc48af310709b85d07c8b0d3aa8f1ead02829d3
Comment 8 Stefan Seyfried 2013-08-30 12:26:23 UTC
this will appear in 3.12-rc series?
Comment 9 Bjorn Helgaas 2013-08-30 13:35:28 UTC
It should appear in v3.12-rc1, and then be backported to the v3.10 and v3.11 stable kernels soon after that.
Comment 10 Stefan Seyfried 2013-08-30 13:51:14 UTC
Ok, I'll test it early in the 3.12-rc series and report back.
Comment 11 Bjorn Helgaas 2013-08-30 16:56:00 UTC
Created attachment 107369 [details]
patch from Neil Horman

This is the patch referred to in comment #7.  Here's my analysis of the behavior before and after Neil's patch.

Current behavior (before Neil's patch):

  acpi_pci_root_add
    root = kzalloc                      # root->osc_control_set = 0
    acpi_pci_osc_support                # indicate OS support for segments
    root->bus = pci_acpi_scan_root
      pci_create_root_bus
        pcibios_add_bus
          acpi_pci_add_bus
            acpiphp_enumerate_slots
              init_bridge_misc
                acpi_walk_namespace(..., register_slot, ...)
                  register_slot
                    device_is_managed_by_native_pciehp
                      <test root->osc_control_set>      # root->osc_control_set == 0
                    return if OS has PCIe hotplug control
                    acpiphp_register_hotplug_slot
    acpi_pci_osc_support                # indicate OS support for MSI, ASPM, etc
    if (_osc_support() failed)
      pci_no_aspm
        aspm_disabled = 1
    acpi_pci_osc_control_set            # request OS control of hotplug, PME, AER, etc
      root->osc_control_set = XX
    if (_osc_control_set() succeeded) {
      if (FADT NO_ASPM bit)
        pcie_clear_aspm
          list_for_each_entry(..., &bus->devices, ...)
            __pci_disable_link_state
    } else {                            # _osc_control_set() failed
      pcie_no_aspm
        aspm_disabled = 1
    }


After Neil's patch:
  acpi_pci_root_add
    root = kzalloc                      # root->osc_control_set = 0
    acpi_pci_osc_support                # indicate OS support for segments
    if (_osc_support() failed)
      no_aspm = true
    acpi_pci_osc_support                # indicate OS support for MSI, ASPM, etc
    acpi_pci_osc_control_set            # request OS control of hotplug, PME, AER, etc
      root->osc_control_set = XX
    if (_osc_control_set() succeeded) {
      if (FADT NO_ASPM bit)
        clear_aspm = true
    } else {                            # _osc_control_set() failed
      no_aspm = true
    }
    root->bus = pci_acpi_scan_root
      pci_create_root_bus
        pcibios_add_bus
          acpi_pci_add_bus
            acpiphp_enumerate_slots
              acpi_walk_namespace(..., register_slot, ...)
                register_slot
                  device_is_managed_by_native_pciehp
                    <test root->osc_control_set>
                  return if OS has PCIe hotplug control
                  acpiphp_register_hotplug_slot
    if (clear_aspm)
      pcie_clear_aspm(root->bus)
        list_for_each_entry(..., &bus->devices, ...)
          __pci_disable_link_state
    if (no_aspm)
      pcie_no_aspm
        aspm_disabled = 1
Comment 12 Bjorn Helgaas 2013-08-30 16:58:31 UTC
Here are my notes about when this broke and what kernels need fixing.

In v3.8:
    acpi_pci_root_add
      acpi_pci_osc_control_set          # request OS control
      pci_acpi_scan_root                # scan bus
    acpi_pci_root_start
      add_bridge                        # acpi_pci_driver .add method
        ...
          device_is_managed_by_native_pciehp  # OK

In v3.9:
    acpi_pci_root_add
      pci_acpi_scan_root                # scan bus
      acpi_pci_osc_control_set          # request OS control
      add_bridge                        # acpi_pci_driver .add method
        ...
          device_is_managed_by_native_pciehp  # OK

In v3.10:
    acpi_pci_root_add
      pci_acpi_scan_root                # scan bus
        ...
          acpiphp_enumerate_slots
            ...
              device_is_managed_by_native_pciehp  # BROKEN (by 3b63aaa70e)
      acpi_pci_osc_control_set          # request OS control

In v3.12:
    acpi_pci_root_add
      acpi_pci_osc_control_set          # request OS control
      pci_acpi_scan_root                # scan bus
        ...
          acpiphp_enumerate_slots
            ...
              device_is_managed_by_native_pciehp  # FIXED
Comment 13 Bjorn Helgaas 2013-09-20 20:59:32 UTC
v3.12-rc1 contains Neil's patch.  Can you please retest with
v3.12-rc1 or later, attach the complete dmesg log, and note
whether this issue is resolved?
Comment 14 Stefan Seyfried 2013-09-23 09:11:24 UTC
Created attachment 109271 [details]
dmesg of working minimal boot of 3.12rc1

3.12rc1 works.
I booted pretty minimal kernel config (to speed up compilation) into init=/bin/bash without acpiphp.disable=1, then inserted ExpressCard, it was detected and worked (after manually inserting xhci_hcd, no udev running in minimal boot mode).

=> seems fixed, will test more once 3.12rc lands in openSUSE Kernel repositories :-)

Thanks!
Comment 15 Bjorn Helgaas 2013-09-23 18:22:38 UTC
Thanks for testing this, Stefan.  I'm going to mark this as resolved, but please reopen it if further testing shows any issues.

Note You need to log in before you can comment on or make changes to this bug.