diff mbox

pci/hotplug: work-around for missing _RMV on HP ZBook G2

Message ID 1431632038-39917-1-git-send-email-jarod@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jarod Wilson May 14, 2015, 7:33 p.m. UTC
The HP ZBook 15 and 17 Mobile Workstations, generation 2, up to and
including at least BIOS revision 01.07, do not have an ACPI _RMV object
associated with their expresscard slots, so acpi-based hotplug-capable
slot detection fails. If we fall back to pcie-based detection, the systems
work just fine, so this uses dmi matching to do that. With luck, a future
BIOS will remedy this (I've let someone at HP know about the problem),
but for now, just use this for all existing versions.

Note: they *do* have a proper _RMV object for what I believe is their
thunderbolt ports.

Tested successfully on an HP ZBook 17 G2 and HP ZBook 15 G2.

CC: Len Brown <lenb@kernel.org>
CC: "Rafael J. Wysocki" <rjw@sisk.pl>
CC: Bjorn Helgaas <bhelgaas@google.com>
CC: linux-acpi@vger.kernel.org
CC: linux-pci@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/pci/hotplug/pciehp_acpi.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

Comments

Bjorn Helgaas May 16, 2015, 2:37 p.m. UTC | #1
Hi Jarod,

On Thu, May 14, 2015 at 03:33:58PM -0400, Jarod Wilson wrote:
> The HP ZBook 15 and 17 Mobile Workstations, generation 2, up to and
> including at least BIOS revision 01.07, do not have an ACPI _RMV object
> associated with their expresscard slots, so acpi-based hotplug-capable
> slot detection fails. If we fall back to pcie-based detection, the systems
> work just fine, so this uses dmi matching to do that. With luck, a future
> BIOS will remedy this (I've let someone at HP know about the problem),
> but for now, just use this for all existing versions.
> 
> Note: they *do* have a proper _RMV object for what I believe is their
> thunderbolt ports.
> 
> Tested successfully on an HP ZBook 17 G2 and HP ZBook 15 G2.
> 
> CC: Len Brown <lenb@kernel.org>
> CC: "Rafael J. Wysocki" <rjw@sisk.pl>
> CC: Bjorn Helgaas <bhelgaas@google.com>
> CC: linux-acpi@vger.kernel.org
> CC: linux-pci@vger.kernel.org
> Signed-off-by: Jarod Wilson <jarod@redhat.com>
> ---
>  drivers/pci/hotplug/pciehp_acpi.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/pci/hotplug/pciehp_acpi.c b/drivers/pci/hotplug/pciehp_acpi.c
> index 93cc926..db38fb5 100644
> --- a/drivers/pci/hotplug/pciehp_acpi.c
> +++ b/drivers/pci/hotplug/pciehp_acpi.c
> @@ -28,6 +28,7 @@
>  #include <linux/pci_hotplug.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
> +#include <linux/dmi.h>
>  #include "pciehp.h"
>  
>  #define PCIEHP_DETECT_PCIE	(0)
> @@ -109,10 +110,40 @@ static struct pcie_port_service_driver __initdata dummy_driver = {
>  	.probe		= dummy_probe,
>  };
>  
> +static int __init set_slot_detection_mode_pcie(const struct dmi_system_id *d)
> +{
> +	info("%s lacks ACPI _RMV object for expresscard\n", d->ident);
> +	return 1;
> +}
> +
> +static struct dmi_system_id __initdata missing_acpi_rmv[] = {
> +	/* ZBook 17 through at least bios v01.07 */
> +	{
> +	 .callback = set_slot_detection_mode_pcie,
> +	 .ident = "HP ZBook 17 G2 Mobile Workstation",
> +	 .matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> +		DMI_MATCH(DMI_PRODUCT_NAME, "HP ZBook 17 G2"),
> +		},
> +	},
> +	/* ZBook 15 through at least bios v01.07 */
> +	{
> +	 .callback = set_slot_detection_mode_pcie,
> +	 .ident = "HP ZBook 15 G2 Mobile Workstation",
> +	 .matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> +		DMI_MATCH(DMI_PRODUCT_NAME, "HP ZBook 15 G2"),
> +		},
> +	},
> +	{ .ident = NULL }
> +};
> +
>  static int __init select_detection_mode(void)
>  {
>  	struct dummy_slot *slot, *tmp;
>  
> +	if (dmi_check_system(missing_acpi_rmv))
> +		return PCIEHP_DETECT_PCIE;

Oh, my goodness.  I forgot how terrible this path is.  Can anyone write a
simple explanation of how we choose to use acpiphp or pciehp?  Module
parameters?  A dummy driver that looks for duplicate slot numbers?  Looking
for _ADR, _EJ0, _RMV?  This is just nuts.

I can't really believe that we're doing this correctly.

If I understand correctly, the ZBooks don't have _RMV, but we try to use
acpiphp anyway, and acpiphp doesn't work?  That sounds more like a problem
with our acpiphp/pciehp selection "algorithm" than a BIOS bug.

Jarod, can you open a report at http://bugzilla.kernel.org and attach a
complete dmesg log, "lspci -vv" output, and an acpidump?  I'm particularly
interested in whether the BIOS granted us control over PCIe native hotplug.
If it did, I wonder why we would even attempt to use acpiphp.

Bjorn

>  	if (pcie_port_service_register(&dummy_driver))
>  		return PCIEHP_DETECT_ACPI;
>  	pcie_port_service_unregister(&dummy_driver);
> @@ -134,4 +165,6 @@ void __init pciehp_acpi_slot_detection_init(void)
>  out:
>  	if (slot_detection_mode == PCIEHP_DETECT_ACPI)
>  		info("Using ACPI for slot detection.\n");
> +	else if (slot_detection_mode == PCIEHP_DETECT_PCIE)
> +		info("Using PCIE-based slot detection.\n");
>  }
> -- 
> 1.8.3.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas May 16, 2015, 2:41 p.m. UTC | #2
[fix Rafael's email address]

On Sat, May 16, 2015 at 09:37:50AM -0500, Bjorn Helgaas wrote:
> Hi Jarod,
> 
> On Thu, May 14, 2015 at 03:33:58PM -0400, Jarod Wilson wrote:
> > The HP ZBook 15 and 17 Mobile Workstations, generation 2, up to and
> > including at least BIOS revision 01.07, do not have an ACPI _RMV object
> > associated with their expresscard slots, so acpi-based hotplug-capable
> > slot detection fails. If we fall back to pcie-based detection, the systems
> > work just fine, so this uses dmi matching to do that. With luck, a future
> > BIOS will remedy this (I've let someone at HP know about the problem),
> > but for now, just use this for all existing versions.
> > 
> > Note: they *do* have a proper _RMV object for what I believe is their
> > thunderbolt ports.
> > 
> > Tested successfully on an HP ZBook 17 G2 and HP ZBook 15 G2.
> > 
> > CC: Len Brown <lenb@kernel.org>
> > CC: "Rafael J. Wysocki" <rjw@sisk.pl>
> > CC: Bjorn Helgaas <bhelgaas@google.com>
> > CC: linux-acpi@vger.kernel.org
> > CC: linux-pci@vger.kernel.org
> > Signed-off-by: Jarod Wilson <jarod@redhat.com>
> > ---
> >  drivers/pci/hotplug/pciehp_acpi.c | 33 +++++++++++++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> > 
> > diff --git a/drivers/pci/hotplug/pciehp_acpi.c b/drivers/pci/hotplug/pciehp_acpi.c
> > index 93cc926..db38fb5 100644
> > --- a/drivers/pci/hotplug/pciehp_acpi.c
> > +++ b/drivers/pci/hotplug/pciehp_acpi.c
> > @@ -28,6 +28,7 @@
> >  #include <linux/pci_hotplug.h>
> >  #include <linux/slab.h>
> >  #include <linux/module.h>
> > +#include <linux/dmi.h>
> >  #include "pciehp.h"
> >  
> >  #define PCIEHP_DETECT_PCIE	(0)
> > @@ -109,10 +110,40 @@ static struct pcie_port_service_driver __initdata dummy_driver = {
> >  	.probe		= dummy_probe,
> >  };
> >  
> > +static int __init set_slot_detection_mode_pcie(const struct dmi_system_id *d)
> > +{
> > +	info("%s lacks ACPI _RMV object for expresscard\n", d->ident);
> > +	return 1;
> > +}
> > +
> > +static struct dmi_system_id __initdata missing_acpi_rmv[] = {
> > +	/* ZBook 17 through at least bios v01.07 */
> > +	{
> > +	 .callback = set_slot_detection_mode_pcie,
> > +	 .ident = "HP ZBook 17 G2 Mobile Workstation",
> > +	 .matches = {
> > +		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> > +		DMI_MATCH(DMI_PRODUCT_NAME, "HP ZBook 17 G2"),
> > +		},
> > +	},
> > +	/* ZBook 15 through at least bios v01.07 */
> > +	{
> > +	 .callback = set_slot_detection_mode_pcie,
> > +	 .ident = "HP ZBook 15 G2 Mobile Workstation",
> > +	 .matches = {
> > +		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> > +		DMI_MATCH(DMI_PRODUCT_NAME, "HP ZBook 15 G2"),
> > +		},
> > +	},
> > +	{ .ident = NULL }
> > +};
> > +
> >  static int __init select_detection_mode(void)
> >  {
> >  	struct dummy_slot *slot, *tmp;
> >  
> > +	if (dmi_check_system(missing_acpi_rmv))
> > +		return PCIEHP_DETECT_PCIE;
> 
> Oh, my goodness.  I forgot how terrible this path is.  Can anyone write a
> simple explanation of how we choose to use acpiphp or pciehp?  Module
> parameters?  A dummy driver that looks for duplicate slot numbers?  Looking
> for _ADR, _EJ0, _RMV?  This is just nuts.
> 
> I can't really believe that we're doing this correctly.
> 
> If I understand correctly, the ZBooks don't have _RMV, but we try to use
> acpiphp anyway, and acpiphp doesn't work?  That sounds more like a problem
> with our acpiphp/pciehp selection "algorithm" than a BIOS bug.
> 
> Jarod, can you open a report at http://bugzilla.kernel.org and attach a
> complete dmesg log, "lspci -vv" output, and an acpidump?  I'm particularly
> interested in whether the BIOS granted us control over PCIe native hotplug.
> If it did, I wonder why we would even attempt to use acpiphp.
> 
> Bjorn
> 
> >  	if (pcie_port_service_register(&dummy_driver))
> >  		return PCIEHP_DETECT_ACPI;
> >  	pcie_port_service_unregister(&dummy_driver);
> > @@ -134,4 +165,6 @@ void __init pciehp_acpi_slot_detection_init(void)
> >  out:
> >  	if (slot_detection_mode == PCIEHP_DETECT_ACPI)
> >  		info("Using ACPI for slot detection.\n");
> > +	else if (slot_detection_mode == PCIEHP_DETECT_PCIE)
> > +		info("Using PCIE-based slot detection.\n");
> >  }
> > -- 
> > 1.8.3.1
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki May 18, 2015, 12:26 a.m. UTC | #3
On Saturday, May 16, 2015 09:41:55 AM Bjorn Helgaas wrote:
> [fix Rafael's email address]

Thanks. :-)

> On Sat, May 16, 2015 at 09:37:50AM -0500, Bjorn Helgaas wrote:
> > Hi Jarod,
> > 
> > On Thu, May 14, 2015 at 03:33:58PM -0400, Jarod Wilson wrote:
> > > The HP ZBook 15 and 17 Mobile Workstations, generation 2, up to and
> > > including at least BIOS revision 01.07, do not have an ACPI _RMV object
> > > associated with their expresscard slots, so acpi-based hotplug-capable
> > > slot detection fails. If we fall back to pcie-based detection, the systems
> > > work just fine, so this uses dmi matching to do that. With luck, a future
> > > BIOS will remedy this (I've let someone at HP know about the problem),
> > > but for now, just use this for all existing versions.
> > > 
> > > Note: they *do* have a proper _RMV object for what I believe is their
> > > thunderbolt ports.
> > > 
> > > Tested successfully on an HP ZBook 17 G2 and HP ZBook 15 G2.
> > > 
> > > CC: Len Brown <lenb@kernel.org>
> > > CC: "Rafael J. Wysocki" <rjw@sisk.pl>
> > > CC: Bjorn Helgaas <bhelgaas@google.com>
> > > CC: linux-acpi@vger.kernel.org
> > > CC: linux-pci@vger.kernel.org
> > > Signed-off-by: Jarod Wilson <jarod@redhat.com>
> > > ---
> > >  drivers/pci/hotplug/pciehp_acpi.c | 33 +++++++++++++++++++++++++++++++++
> > >  1 file changed, 33 insertions(+)
> > > 
> > > diff --git a/drivers/pci/hotplug/pciehp_acpi.c b/drivers/pci/hotplug/pciehp_acpi.c
> > > index 93cc926..db38fb5 100644
> > > --- a/drivers/pci/hotplug/pciehp_acpi.c
> > > +++ b/drivers/pci/hotplug/pciehp_acpi.c
> > > @@ -28,6 +28,7 @@
> > >  #include <linux/pci_hotplug.h>
> > >  #include <linux/slab.h>
> > >  #include <linux/module.h>
> > > +#include <linux/dmi.h>
> > >  #include "pciehp.h"
> > >  
> > >  #define PCIEHP_DETECT_PCIE	(0)
> > > @@ -109,10 +110,40 @@ static struct pcie_port_service_driver __initdata dummy_driver = {
> > >  	.probe		= dummy_probe,
> > >  };
> > >  
> > > +static int __init set_slot_detection_mode_pcie(const struct dmi_system_id *d)
> > > +{
> > > +	info("%s lacks ACPI _RMV object for expresscard\n", d->ident);
> > > +	return 1;
> > > +}
> > > +
> > > +static struct dmi_system_id __initdata missing_acpi_rmv[] = {
> > > +	/* ZBook 17 through at least bios v01.07 */
> > > +	{
> > > +	 .callback = set_slot_detection_mode_pcie,
> > > +	 .ident = "HP ZBook 17 G2 Mobile Workstation",
> > > +	 .matches = {
> > > +		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> > > +		DMI_MATCH(DMI_PRODUCT_NAME, "HP ZBook 17 G2"),
> > > +		},
> > > +	},
> > > +	/* ZBook 15 through at least bios v01.07 */
> > > +	{
> > > +	 .callback = set_slot_detection_mode_pcie,
> > > +	 .ident = "HP ZBook 15 G2 Mobile Workstation",
> > > +	 .matches = {
> > > +		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> > > +		DMI_MATCH(DMI_PRODUCT_NAME, "HP ZBook 15 G2"),
> > > +		},
> > > +	},
> > > +	{ .ident = NULL }
> > > +};
> > > +
> > >  static int __init select_detection_mode(void)
> > >  {
> > >  	struct dummy_slot *slot, *tmp;
> > >  
> > > +	if (dmi_check_system(missing_acpi_rmv))
> > > +		return PCIEHP_DETECT_PCIE;
> > 
> > Oh, my goodness.  I forgot how terrible this path is.  Can anyone write a
> > simple explanation of how we choose to use acpiphp or pciehp?

In theory, that should depend on the _OSC handshake in acpi_pci_root_add().

If the firmware doesn't give us control of the PCIe features, we'll not use
pciehp (or at least that's the idea).

acpiphp is used if pciehp doesn't claim the device, AFAICS.

Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jarod Wilson May 18, 2015, 2:30 p.m. UTC | #4
On 5/16/2015 10:41 AM, Bjorn Helgaas wrote:
> [fix Rafael's email address]
>
> On Sat, May 16, 2015 at 09:37:50AM -0500, Bjorn Helgaas wrote:
>> Hi Jarod,
>>
>> On Thu, May 14, 2015 at 03:33:58PM -0400, Jarod Wilson wrote:
>>> The HP ZBook 15 and 17 Mobile Workstations, generation 2, up to and
>>> including at least BIOS revision 01.07, do not have an ACPI _RMV object
>>> associated with their expresscard slots, so acpi-based hotplug-capable
>>> slot detection fails. If we fall back to pcie-based detection, the systems
>>> work just fine, so this uses dmi matching to do that. With luck, a future
>>> BIOS will remedy this (I've let someone at HP know about the problem),
>>> but for now, just use this for all existing versions.
>>>
>>> Note: they *do* have a proper _RMV object for what I believe is their
>>> thunderbolt ports.
>>>
>>> Tested successfully on an HP ZBook 17 G2 and HP ZBook 15 G2.
>>>
>>> CC: Len Brown <lenb@kernel.org>
>>> CC: "Rafael J. Wysocki" <rjw@sisk.pl>
>>> CC: Bjorn Helgaas <bhelgaas@google.com>
>>> CC: linux-acpi@vger.kernel.org
>>> CC: linux-pci@vger.kernel.org
>>> Signed-off-by: Jarod Wilson <jarod@redhat.com>
>>> ---
>>>   drivers/pci/hotplug/pciehp_acpi.c | 33 +++++++++++++++++++++++++++++++++
>>>   1 file changed, 33 insertions(+)
>>>
>>> diff --git a/drivers/pci/hotplug/pciehp_acpi.c b/drivers/pci/hotplug/pciehp_acpi.c
>>> index 93cc926..db38fb5 100644
>>> --- a/drivers/pci/hotplug/pciehp_acpi.c
>>> +++ b/drivers/pci/hotplug/pciehp_acpi.c
>>> @@ -28,6 +28,7 @@
>>>   #include <linux/pci_hotplug.h>
>>>   #include <linux/slab.h>
>>>   #include <linux/module.h>
>>> +#include <linux/dmi.h>
>>>   #include "pciehp.h"
>>>
>>>   #define PCIEHP_DETECT_PCIE	(0)
>>> @@ -109,10 +110,40 @@ static struct pcie_port_service_driver __initdata dummy_driver = {
>>>   	.probe		= dummy_probe,
>>>   };
>>>
>>> +static int __init set_slot_detection_mode_pcie(const struct dmi_system_id *d)
>>> +{
>>> +	info("%s lacks ACPI _RMV object for expresscard\n", d->ident);
>>> +	return 1;
>>> +}
>>> +
>>> +static struct dmi_system_id __initdata missing_acpi_rmv[] = {
>>> +	/* ZBook 17 through at least bios v01.07 */
>>> +	{
>>> +	 .callback = set_slot_detection_mode_pcie,
>>> +	 .ident = "HP ZBook 17 G2 Mobile Workstation",
>>> +	 .matches = {
>>> +		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
>>> +		DMI_MATCH(DMI_PRODUCT_NAME, "HP ZBook 17 G2"),
>>> +		},
>>> +	},
>>> +	/* ZBook 15 through at least bios v01.07 */
>>> +	{
>>> +	 .callback = set_slot_detection_mode_pcie,
>>> +	 .ident = "HP ZBook 15 G2 Mobile Workstation",
>>> +	 .matches = {
>>> +		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
>>> +		DMI_MATCH(DMI_PRODUCT_NAME, "HP ZBook 15 G2"),
>>> +		},
>>> +	},
>>> +	{ .ident = NULL }
>>> +};
>>> +
>>>   static int __init select_detection_mode(void)
>>>   {
>>>   	struct dummy_slot *slot, *tmp;
>>>
>>> +	if (dmi_check_system(missing_acpi_rmv))
>>> +		return PCIEHP_DETECT_PCIE;
>>
>> Oh, my goodness.  I forgot how terrible this path is.  Can anyone write a
>> simple explanation of how we choose to use acpiphp or pciehp?  Module
>> parameters?  A dummy driver that looks for duplicate slot numbers?  Looking
>> for _ADR, _EJ0, _RMV?  This is just nuts.
>>
>> I can't really believe that we're doing this correctly.
>>
>> If I understand correctly, the ZBooks don't have _RMV, but we try to use
>> acpiphp anyway, and acpiphp doesn't work?

They do have an _RMV entry for another device, whatever is on 
0000:00:1c.0, which appears to be the thunderbolt port, but I have yet 
to verify that (no thunderbolt devices to play with yet). The 
expresscard slot is 0000:3c:02.0.

>> That sounds more like a problem
>> with our acpiphp/pciehp selection "algorithm" than a BIOS bug.
>>
>> Jarod, can you open a report at http://bugzilla.kernel.org and attach a
>> complete dmesg log, "lspci -vv" output, and an acpidump?  I'm particularly
>> interested in whether the BIOS granted us control over PCIe native hotplug.
>> If it did, I wonder why we would even attempt to use acpiphp.

Done:

https://bugzilla.kernel.org/show_bug.cgi?id=98581
Jarod Wilson May 18, 2015, 2:33 p.m. UTC | #5
On 5/17/2015 8:26 PM, Rafael J. Wysocki wrote:
> On Saturday, May 16, 2015 09:41:55 AM Bjorn Helgaas wrote:
>> On Sat, May 16, 2015 at 09:37:50AM -0500, Bjorn Helgaas wrote:
>>> Hi Jarod,
>>>
>>> On Thu, May 14, 2015 at 03:33:58PM -0400, Jarod Wilson wrote:
>>>> The HP ZBook 15 and 17 Mobile Workstations, generation 2, up to and
>>>> including at least BIOS revision 01.07, do not have an ACPI _RMV object
>>>> associated with their expresscard slots, so acpi-based hotplug-capable
>>>> slot detection fails. If we fall back to pcie-based detection, the systems
>>>> work just fine, so this uses dmi matching to do that. With luck, a future
>>>> BIOS will remedy this (I've let someone at HP know about the problem),
>>>> but for now, just use this for all existing versions.
...
>>> Oh, my goodness.  I forgot how terrible this path is.  Can anyone write a
>>> simple explanation of how we choose to use acpiphp or pciehp?
>
> In theory, that should depend on the _OSC handshake in acpi_pci_root_add().
>
> If the firmware doesn't give us control of the PCIe features, we'll not use
> pciehp (or at least that's the idea).
>
> acpiphp is used if pciehp doesn't claim the device, AFAICS.

[    4.013326] acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM 
ClockPM Segments MSI]
[    4.015860] acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug PME 
AER PCIeCapability]

So at a glance, it would appear that pciehp *should* be claiming it, 
right? Something I noted in the bug I filed is that the device ID 
reported there is PNP0A08, and the root_device_id table that associates 
with acpi_pci_root_add() only includes PNP0A03 in it. Is that correct, 
or should 08 also be in there, which might remedy this? (I can test this 
out easily enough).
Jarod Wilson May 18, 2015, 4:17 p.m. UTC | #6
On 5/18/2015 10:33 AM, Jarod Wilson wrote:
> On 5/17/2015 8:26 PM, Rafael J. Wysocki wrote:
>> On Saturday, May 16, 2015 09:41:55 AM Bjorn Helgaas wrote:
>>> On Sat, May 16, 2015 at 09:37:50AM -0500, Bjorn Helgaas wrote:
>>>> Hi Jarod,
>>>>
>>>> On Thu, May 14, 2015 at 03:33:58PM -0400, Jarod Wilson wrote:
>>>>> The HP ZBook 15 and 17 Mobile Workstations, generation 2, up to and
>>>>> including at least BIOS revision 01.07, do not have an ACPI _RMV
>>>>> object
>>>>> associated with their expresscard slots, so acpi-based hotplug-capable
>>>>> slot detection fails. If we fall back to pcie-based detection, the
>>>>> systems
>>>>> work just fine, so this uses dmi matching to do that. With luck, a
>>>>> future
>>>>> BIOS will remedy this (I've let someone at HP know about the problem),
>>>>> but for now, just use this for all existing versions.
> ...
>>>> Oh, my goodness.  I forgot how terrible this path is.  Can anyone
>>>> write a
>>>> simple explanation of how we choose to use acpiphp or pciehp?
>>
>> In theory, that should depend on the _OSC handshake in
>> acpi_pci_root_add().
>>
>> If the firmware doesn't give us control of the PCIe features, we'll
>> not use
>> pciehp (or at least that's the idea).
>>
>> acpiphp is used if pciehp doesn't claim the device, AFAICS.
>
> [    4.013326] acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM
> ClockPM Segments MSI]
> [    4.015860] acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug PME
> AER PCIeCapability]
>
> So at a glance, it would appear that pciehp *should* be claiming it,
> right? Something I noted in the bug I filed is that the device ID
> reported there is PNP0A08, and the root_device_id table that associates
> with acpi_pci_root_add() only includes PNP0A03 in it. Is that correct,
> or should 08 also be in there, which might remedy this? (I can test this
> out easily enough).

Nope, makes no difference, seems those are just two different references 
to the same bus, based on a peek at the extracted dsdt:

Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */)  // _HID: Hardware ID
Name (_CID, EisaId ("PNP0A03") /* PCI Bus */)  // _CID: Compatible ID
Jarod Wilson May 18, 2015, 8:45 p.m. UTC | #7
On 5/18/2015 12:17 PM, Jarod Wilson wrote:
> On 5/18/2015 10:33 AM, Jarod Wilson wrote:
>> On 5/17/2015 8:26 PM, Rafael J. Wysocki wrote:
>>> On Saturday, May 16, 2015 09:41:55 AM Bjorn Helgaas wrote:
>>>> On Sat, May 16, 2015 at 09:37:50AM -0500, Bjorn Helgaas wrote:
>>>>> Hi Jarod,
>>>>>
>>>>> On Thu, May 14, 2015 at 03:33:58PM -0400, Jarod Wilson wrote:
>>>>>> The HP ZBook 15 and 17 Mobile Workstations, generation 2, up to and
>>>>>> including at least BIOS revision 01.07, do not have an ACPI _RMV
>>>>>> object
>>>>>> associated with their expresscard slots, so acpi-based
>>>>>> hotplug-capable
>>>>>> slot detection fails. If we fall back to pcie-based detection, the
>>>>>> systems
>>>>>> work just fine, so this uses dmi matching to do that. With luck, a
>>>>>> future
>>>>>> BIOS will remedy this (I've let someone at HP know about the
>>>>>> problem),
>>>>>> but for now, just use this for all existing versions.
>> ...
>>>>> Oh, my goodness.  I forgot how terrible this path is.  Can anyone
>>>>> write a
>>>>> simple explanation of how we choose to use acpiphp or pciehp?
>>>
>>> In theory, that should depend on the _OSC handshake in
>>> acpi_pci_root_add().
>>>
>>> If the firmware doesn't give us control of the PCIe features, we'll
>>> not use
>>> pciehp (or at least that's the idea).
>>>
>>> acpiphp is used if pciehp doesn't claim the device, AFAICS.
>>
>> [    4.013326] acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM
>> ClockPM Segments MSI]
>> [    4.015860] acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug PME
>> AER PCIeCapability]
>>
>> So at a glance, it would appear that pciehp *should* be claiming it,
>> right? Something I noted in the bug I filed is that the device ID
>> reported there is PNP0A08, and the root_device_id table that associates
>> with acpi_pci_root_add() only includes PNP0A03 in it. Is that correct,
>> or should 08 also be in there, which might remedy this? (I can test this
>> out easily enough).
>
> Nope, makes no difference, seems those are just two different references
> to the same bus, based on a peek at the extracted dsdt:
>
> Name (_HID, EisaId ("PNP0A08") /* PCI Express Bus */)  // _HID: Hardware ID
> Name (_CID, EisaId ("PNP0A03") /* PCI Bus */)  // _CID: Compatible ID

Ah, I forgot some additional details. pciehp_probe() in 
drivers/pci/hotplug/pciehp_core.c fails on the 
pciehp_acpi_slot_detection_check() call for the expresscard slot, which 
is why the base pciehp doesn't bind. DEVICE_ACPI_HANDLE(&dev->dev) in 
the slot detection check is winding up with a NULL acpi device.
Rafael J. Wysocki May 18, 2015, 9:57 p.m. UTC | #8
On Monday, May 18, 2015 10:33:08 AM Jarod Wilson wrote:
> On 5/17/2015 8:26 PM, Rafael J. Wysocki wrote:
> > On Saturday, May 16, 2015 09:41:55 AM Bjorn Helgaas wrote:
> >> On Sat, May 16, 2015 at 09:37:50AM -0500, Bjorn Helgaas wrote:
> >>> Hi Jarod,
> >>>
> >>> On Thu, May 14, 2015 at 03:33:58PM -0400, Jarod Wilson wrote:
> >>>> The HP ZBook 15 and 17 Mobile Workstations, generation 2, up to and
> >>>> including at least BIOS revision 01.07, do not have an ACPI _RMV object
> >>>> associated with their expresscard slots, so acpi-based hotplug-capable
> >>>> slot detection fails. If we fall back to pcie-based detection, the systems
> >>>> work just fine, so this uses dmi matching to do that. With luck, a future
> >>>> BIOS will remedy this (I've let someone at HP know about the problem),
> >>>> but for now, just use this for all existing versions.
> ...
> >>> Oh, my goodness.  I forgot how terrible this path is.  Can anyone write a
> >>> simple explanation of how we choose to use acpiphp or pciehp?
> >
> > In theory, that should depend on the _OSC handshake in acpi_pci_root_add().
> >
> > If the firmware doesn't give us control of the PCIe features, we'll not use
> > pciehp (or at least that's the idea).
> >
> > acpiphp is used if pciehp doesn't claim the device, AFAICS.

That wasn't precise enough, sorry.  ACPIPHP will handle device check and bus
check notifications from ACPI even for devices that have been claimed by
pciehp.  It won't call pci_hp_register() for them, though.

> [    4.013326] acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM 
> ClockPM Segments MSI]
> [    4.015860] acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug PME 
> AER PCIeCapability]
> 
> So at a glance, it would appear that pciehp *should* be claiming it, 
> right?

Yes, it should.

> Something I noted in the bug I filed is that the device ID 
> reported there is PNP0A08, and the root_device_id table that associates 
> with acpi_pci_root_add() only includes PNP0A03 in it. Is that correct, 
> or should 08 also be in there, which might remedy this? (I can test this 
> out easily enough).

It is correct (or at least not obviously incorrect).  The _OSC is for the host
bridge and determines the behavior for all devices below root ports.
diff mbox

Patch

diff --git a/drivers/pci/hotplug/pciehp_acpi.c b/drivers/pci/hotplug/pciehp_acpi.c
index 93cc926..db38fb5 100644
--- a/drivers/pci/hotplug/pciehp_acpi.c
+++ b/drivers/pci/hotplug/pciehp_acpi.c
@@ -28,6 +28,7 @@ 
 #include <linux/pci_hotplug.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/dmi.h>
 #include "pciehp.h"
 
 #define PCIEHP_DETECT_PCIE	(0)
@@ -109,10 +110,40 @@  static struct pcie_port_service_driver __initdata dummy_driver = {
 	.probe		= dummy_probe,
 };
 
+static int __init set_slot_detection_mode_pcie(const struct dmi_system_id *d)
+{
+	info("%s lacks ACPI _RMV object for expresscard\n", d->ident);
+	return 1;
+}
+
+static struct dmi_system_id __initdata missing_acpi_rmv[] = {
+	/* ZBook 17 through at least bios v01.07 */
+	{
+	 .callback = set_slot_detection_mode_pcie,
+	 .ident = "HP ZBook 17 G2 Mobile Workstation",
+	 .matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
+		DMI_MATCH(DMI_PRODUCT_NAME, "HP ZBook 17 G2"),
+		},
+	},
+	/* ZBook 15 through at least bios v01.07 */
+	{
+	 .callback = set_slot_detection_mode_pcie,
+	 .ident = "HP ZBook 15 G2 Mobile Workstation",
+	 .matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
+		DMI_MATCH(DMI_PRODUCT_NAME, "HP ZBook 15 G2"),
+		},
+	},
+	{ .ident = NULL }
+};
+
 static int __init select_detection_mode(void)
 {
 	struct dummy_slot *slot, *tmp;
 
+	if (dmi_check_system(missing_acpi_rmv))
+		return PCIEHP_DETECT_PCIE;
 	if (pcie_port_service_register(&dummy_driver))
 		return PCIEHP_DETECT_ACPI;
 	pcie_port_service_unregister(&dummy_driver);
@@ -134,4 +165,6 @@  void __init pciehp_acpi_slot_detection_init(void)
 out:
 	if (slot_detection_mode == PCIEHP_DETECT_ACPI)
 		info("Using ACPI for slot detection.\n");
+	else if (slot_detection_mode == PCIEHP_DETECT_PCIE)
+		info("Using PCIE-based slot detection.\n");
 }