[PATCH v3 0/3] PCI: pciehp: Do not turn off slot if presence comes up after link

[PATCH v3 0/3] PCI: pciehp: Do not turn off slot if presence comes up after link

[Stuart Hayes]

In older PCIe specs, PDS (presence detect) would come up when the
"in-band" presence detect pin connected, and would be up before DLLLA
(link active).

In PCIe 4.0 (as an ECN) and in PCIe 5.0, there is a new bit to show if
in-band presence detection can be disabled for the slot, and another bit
that disables it--and a recommendation that it should be disabled if it
can be. In addition, certain OEMs disable in-band presence detection
without implementing these bits.

This means it is possible to get a "card present" interrupt after the
link is up and the driver is loaded.  This causes an erroneous removal
of the device driver, followed by an immediate re-probing.

This patch set defines these new bits, uses them to disable in-band
presence detection if it can be, waits for PDS to go up if in-band
presence detection is disabled, and adds a DMI table that will let us
know if we should assume in-band presence is disabled on a system.

The first two patches in this set come from a patch set that was
submitted but not accepted many months ago by Alexandru Gagniuc [1].
The first is unmodified, the second has the commit message and timeout 
modified.

[1] https://patchwork.kernel.org/cover/10909167/
    [v3,0/4] PCI: pciehp: Do not turn off slot if presence comes up after link

v2:
- modify loop in pcie_wait_for_presence to do..while

v3:
- remove unused variable declaration
- modify text of warning message

Alexandru Gagniuc (2):
  PCI: pciehp: Add support for disabling in-band presence
  PCI: pciehp: Wait for PDS if in-band presence is disabled

Stuart Hayes (1):
  PCI: pciehp: Add dmi table for in-band presence disabled

 drivers/pci/hotplug/pciehp.h     |  1 +
 drivers/pci/hotplug/pciehp_hpc.c | 45 +++++++++++++++++++++++++++++++-
 include/uapi/linux/pci_regs.h    |  2 ++
 3 files changed, 47 insertions(+), 1 deletion(-)

-- 
2.18.1

[PATCH v3 1/3] PCI: pciehp: Add support for disabling in-band presence

[Stuart Hayes]

From: Alexandru Gagniuc <mr.nuke.me@gmail.com>

The presence detect state (PDS) is normally a logical or of in-band and
out-of-band presence. As of PCIe 4.0, there is the option to disable
in-band presence so that the PDS bit always reflects the state of the
out-of-band presence.

The recommendation of the PCIe spec is to disable in-band presence
whenever supported.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 drivers/pci/hotplug/pciehp.h     | 1 +
 drivers/pci/hotplug/pciehp_hpc.c | 9 ++++++++-
 include/uapi/linux/pci_regs.h    | 2 ++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
index 654c972b8ea0..27e4cd6529b0 100644
--- a/drivers/pci/hotplug/pciehp.h
+++ b/drivers/pci/hotplug/pciehp.h
@@ -83,6 +83,7 @@ struct controller {
 	struct pcie_device *pcie;
 
 	u32 slot_cap;				/* capabilities and quirks */
+	unsigned int inband_presence_disabled:1;
 
 	u16 slot_ctrl;				/* control register access */
 	struct mutex ctrl_lock;
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 1a522c1c4177..dc109d521f30 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -811,7 +811,7 @@ static inline void dbg_ctrl(struct controller *ctrl)
 struct controller *pcie_init(struct pcie_device *dev)
 {
 	struct controller *ctrl;
-	u32 slot_cap, link_cap;
+	u32 slot_cap, slot_cap2, link_cap;
 	u8 poweron;
 	struct pci_dev *pdev = dev->port;
 	struct pci_bus *subordinate = pdev->subordinate;
@@ -869,6 +869,13 @@ struct controller *pcie_init(struct pcie_device *dev)
 		FLAG(link_cap, PCI_EXP_LNKCAP_DLLLARC),
 		pdev->broken_cmd_compl ? " (with Cmd Compl erratum)" : "");
 
+	pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP2, &slot_cap2);
+	if (slot_cap2 & PCI_EXP_SLTCAP2_IBPD) {
+		pcie_write_cmd_nowait(ctrl, PCI_EXP_SLTCTL_IBPD_DISABLE,
+				      PCI_EXP_SLTCTL_IBPD_DISABLE);
+		ctrl->inband_presence_disabled = 1;
+	}
+
 	/*
 	 * If empty slot's power status is on, turn power off.  The IRQ isn't
 	 * requested yet, so avoid triggering a notification with this command.
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 29d6e93fd15e..ea1cf9546e4d 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -604,6 +604,7 @@
 #define  PCI_EXP_SLTCTL_PWR_OFF        0x0400 /* Power Off */
 #define  PCI_EXP_SLTCTL_EIC	0x0800	/* Electromechanical Interlock Control */
 #define  PCI_EXP_SLTCTL_DLLSCE	0x1000	/* Data Link Layer State Changed Enable */
+#define  PCI_EXP_SLTCTL_IBPD_DISABLE	0x4000 /* In-band PD disable */
 #define PCI_EXP_SLTSTA		26	/* Slot Status */
 #define  PCI_EXP_SLTSTA_ABP	0x0001	/* Attention Button Pressed */
 #define  PCI_EXP_SLTSTA_PFD	0x0002	/* Power Fault Detected */
@@ -676,6 +677,7 @@
 #define PCI_EXP_LNKSTA2		50	/* Link Status 2 */
 #define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2	52	/* v2 endpoints with link end here */
 #define PCI_EXP_SLTCAP2		52	/* Slot Capabilities 2 */
+#define  PCI_EXP_SLTCAP2_IBPD	0x0001	/* In-band PD Disable Supported */
 #define PCI_EXP_SLTCTL2		56	/* Slot Control 2 */
 #define PCI_EXP_SLTSTA2		58	/* Slot Status 2 */
 
-- 
2.18.1

[PATCH v3 2/3] PCI: pciehp: Wait for PDS if in-band presence is disabled

[Stuart Hayes]

From: Alexandru Gagniuc <mr.nuke.me@gmail.com>

When inband presence is disabled, PDS may come up at any time, or not
at all. PDS being low may indicate that the card is still mating, and
we could expect contact bounce to bring down the link as well.

It is reasonable to assume that most cards will mate in a hotplug slot
in about a second. Thus, when we know PDS only reflects out-of-band
presence, it's worthwhile to wait the extra second or so to make sure
the card is properly mated before loading the driver, and to prevent
the hotplug code from disabling a device if the presence detect change
goes active after the device is enabled.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
---
v2:
  replace while(true) loop with do...while
v3
  remove unused variable declaration (pds)
  modify text of warning message

 drivers/pci/hotplug/pciehp_hpc.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index dc109d521f30..02eb811a014f 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -242,6 +242,22 @@ static bool pci_bus_check_dev(struct pci_bus *bus, int devfn)
 	return found;
 }
 
+static void pcie_wait_for_presence(struct pci_dev *pdev)
+{
+	int timeout = 1250;
+	u16 slot_status;
+
+	do {
+		pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
+		if (!!(slot_status & PCI_EXP_SLTSTA_PDS))
+			return;
+		msleep(10);
+		timeout -= 10;
+	} while (timeout > 0);
+
+	pci_info(pdev, "Timeout waiting for Presence Detect state to be set\n");
+}
+
 int pciehp_check_link_status(struct controller *ctrl)
 {
 	struct pci_dev *pdev = ctrl_dev(ctrl);
@@ -251,6 +267,9 @@ int pciehp_check_link_status(struct controller *ctrl)
 	if (!pcie_wait_for_link(pdev, true))
 		return -1;
 
+	if (ctrl->inband_presence_disabled)
+		pcie_wait_for_presence(pdev);
+
 	found = pci_bus_check_dev(ctrl->pcie->port->subordinate,
 					PCI_DEVFN(0, 0));
 
-- 
2.18.1

[PATCH v3 3/3] PCI: pciehp: Add dmi table for in-band presence disabled

[Stuart Hayes]

Some systems have in-band presence detection disabled for hot-plug PCI
slots, but do not report this in the slot capabilities 2 (SLTCAP2) register.
On these systems, presence detect can become active well after the link is
reported to be active, which can cause the slots to be disabled after a
device is connected.

Add a dmi table to flag these systems as having in-band presence disabled.

Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
---
 drivers/pci/hotplug/pciehp_hpc.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 02eb811a014f..4d377a2a62ce 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -14,6 +14,7 @@
 
 #define dev_fmt(fmt) "pciehp: " fmt
 
+#include <linux/dmi.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/jiffies.h>
@@ -26,6 +27,16 @@
 #include "../pci.h"
 #include "pciehp.h"
 
+static const struct dmi_system_id inband_presence_disabled_dmi_table[] = {
+	{
+		.ident = "Dell System",
+		.matches = {
+			DMI_MATCH(DMI_OEM_STRING, "Dell System"),
+		},
+	},
+	{}
+};
+
 static inline struct pci_dev *ctrl_dev(struct controller *ctrl)
 {
 	return ctrl->pcie->port;
@@ -895,6 +906,9 @@ struct controller *pcie_init(struct pcie_device *dev)
 		ctrl->inband_presence_disabled = 1;
 	}
 
+	if (dmi_first_match(inband_presence_disabled_dmi_table))
+		ctrl->inband_presence_disabled = 1;
+
 	/*
 	 * If empty slot's power status is on, turn power off.  The IRQ isn't
 	 * requested yet, so avoid triggering a notification with this command.
-- 
2.18.1

Re: [PATCH v3 1/3] PCI: pciehp: Add support for disabling in-band presence

[Mika Westerberg]

On Thu, Oct 17, 2019 at 03:32:54PM -0400, Stuart Hayes wrote:
> From: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> 
> The presence detect state (PDS) is normally a logical or of in-band and
> out-of-band presence. As of PCIe 4.0, there is the option to disable
> in-band presence so that the PDS bit always reflects the state of the
> out-of-band presence.
> 
> The recommendation of the PCIe spec is to disable in-band presence
> whenever supported.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH v3 2/3] PCI: pciehp: Wait for PDS if in-band presence is disabled

[Mika Westerberg]

On Thu, Oct 17, 2019 at 03:32:55PM -0400, Stuart Hayes wrote:
> From: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> 
> When inband presence is disabled, PDS may come up at any time, or not
> at all. PDS being low may indicate that the card is still mating, and
> we could expect contact bounce to bring down the link as well.
> 
> It is reasonable to assume that most cards will mate in a hotplug slot
> in about a second. Thus, when we know PDS only reflects out-of-band
> presence, it's worthwhile to wait the extra second or so to make sure
> the card is properly mated before loading the driver, and to prevent
> the hotplug code from disabling a device if the presence detect change
> goes active after the device is enabled.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

One nit below.

> ---
> v2:
>   replace while(true) loop with do...while
> v3
>   remove unused variable declaration (pds)
>   modify text of warning message
> 
>  drivers/pci/hotplug/pciehp_hpc.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index dc109d521f30..02eb811a014f 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -242,6 +242,22 @@ static bool pci_bus_check_dev(struct pci_bus *bus, int devfn)
>  	return found;
>  }
>  
> +static void pcie_wait_for_presence(struct pci_dev *pdev)
> +{
> +	int timeout = 1250;
> +	u16 slot_status;
> +
> +	do {
> +		pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
> +		if (!!(slot_status & PCI_EXP_SLTSTA_PDS))

It is more readable if you write it like:

		if (slot_status & PCI_EXP_SLTSTA_PDS)

> +			return;
> +		msleep(10);
> +		timeout -= 10;
> +	} while (timeout > 0);
> +
> +	pci_info(pdev, "Timeout waiting for Presence Detect state to be set\n");
> +}
> +
>  int pciehp_check_link_status(struct controller *ctrl)
>  {
>  	struct pci_dev *pdev = ctrl_dev(ctrl);
> @@ -251,6 +267,9 @@ int pciehp_check_link_status(struct controller *ctrl)
>  	if (!pcie_wait_for_link(pdev, true))
>  		return -1;
>  
> +	if (ctrl->inband_presence_disabled)
> +		pcie_wait_for_presence(pdev);
> +
>  	found = pci_bus_check_dev(ctrl->pcie->port->subordinate,
>  					PCI_DEVFN(0, 0));
>  
> -- 
> 2.18.1

Re: [PATCH v3 3/3] PCI: pciehp: Add dmi table for in-band presence disabled

[Mika Westerberg]

On Thu, Oct 17, 2019 at 03:32:56PM -0400, Stuart Hayes wrote:
> Some systems have in-band presence detection disabled for hot-plug PCI
> slots, but do not report this in the slot capabilities 2 (SLTCAP2) register.
> On these systems, presence detect can become active well after the link is
> reported to be active, which can cause the slots to be disabled after a
> device is connected.
> 
> Add a dmi table to flag these systems as having in-band presence disabled.
> 
> Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
> ---
>  drivers/pci/hotplug/pciehp_hpc.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index 02eb811a014f..4d377a2a62ce 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -14,6 +14,7 @@
>  
>  #define dev_fmt(fmt) "pciehp: " fmt
>  
> +#include <linux/dmi.h>
>  #include <linux/kernel.h>
>  #include <linux/types.h>
>  #include <linux/jiffies.h>
> @@ -26,6 +27,16 @@
>  #include "../pci.h"
>  #include "pciehp.h"
>  
> +static const struct dmi_system_id inband_presence_disabled_dmi_table[] = {
> +	{
> +		.ident = "Dell System",
> +		.matches = {
> +			DMI_MATCH(DMI_OEM_STRING, "Dell System"),

Sorry if this has been discussed previously already but isn't this going
to apply on all Dell systems, not just the affected ones? Is this the
intention?

Re: [PATCH v3 2/3] PCI: pciehp: Wait for PDS if in-band presence is disabled

[Stuart Hayes]

On 10/21/19 8:41 AM, Mika Westerberg wrote:
> On Thu, Oct 17, 2019 at 03:32:55PM -0400, Stuart Hayes wrote:
>> From: Alexandru Gagniuc <mr.nuke.me@gmail.com>
>>
>> When inband presence is disabled, PDS may come up at any time, or not
>> at all. PDS being low may indicate that the card is still mating, and
>> we could expect contact bounce to bring down the link as well.
>>
>> It is reasonable to assume that most cards will mate in a hotplug slot
>> in about a second. Thus, when we know PDS only reflects out-of-band
>> presence, it's worthwhile to wait the extra second or so to make sure
>> the card is properly mated before loading the driver, and to prevent
>> the hotplug code from disabling a device if the presence detect change
>> goes active after the device is enabled.
>>
>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
>> Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
> 
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> One nit below.
> 
>> ---
>> v2:
>>   replace while(true) loop with do...while
>> v3
>>   remove unused variable declaration (pds)
>>   modify text of warning message
>>
>>  drivers/pci/hotplug/pciehp_hpc.c | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
>> index dc109d521f30..02eb811a014f 100644
>> --- a/drivers/pci/hotplug/pciehp_hpc.c
>> +++ b/drivers/pci/hotplug/pciehp_hpc.c
>> @@ -242,6 +242,22 @@ static bool pci_bus_check_dev(struct pci_bus *bus, int devfn)
>>  	return found;
>>  }
>>  
>> +static void pcie_wait_for_presence(struct pci_dev *pdev)
>> +{
>> +	int timeout = 1250;
>> +	u16 slot_status;
>> +
>> +	do {
>> +		pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
>> +		if (!!(slot_status & PCI_EXP_SLTSTA_PDS))
> 
> It is more readable if you write it like:
> 
> 		if (slot_status & PCI_EXP_SLTSTA_PDS)
> 

I agree, it is more readable, and the double bang shouldn't be needed for an 
"if" condition.  Thanks.

>> +			return;
>> +		msleep(10);
>> +		timeout -= 10;
>> +	} while (timeout > 0);
>> +
>> +	pci_info(pdev, "Timeout waiting for Presence Detect state to be set\n");
>> +}
>> +
>>  int pciehp_check_link_status(struct controller *ctrl)
>>  {
>>  	struct pci_dev *pdev = ctrl_dev(ctrl);
>> @@ -251,6 +267,9 @@ int pciehp_check_link_status(struct controller *ctrl)
>>  	if (!pcie_wait_for_link(pdev, true))
>>  		return -1;
>>  
>> +	if (ctrl->inband_presence_disabled)
>> +		pcie_wait_for_presence(pdev);
>> +
>>  	found = pci_bus_check_dev(ctrl->pcie->port->subordinate,
>>  					PCI_DEVFN(0, 0));
>>  
>> -- 
>> 2.18.1

Re: [PATCH v3 3/3] PCI: pciehp: Add dmi table for in-band presence disabled

[Stuart Hayes]

On 10/21/19 8:47 AM, Mika Westerberg wrote:
> On Thu, Oct 17, 2019 at 03:32:56PM -0400, Stuart Hayes wrote:
>> Some systems have in-band presence detection disabled for hot-plug PCI
>> slots, but do not report this in the slot capabilities 2 (SLTCAP2) register.
>> On these systems, presence detect can become active well after the link is
>> reported to be active, which can cause the slots to be disabled after a
>> device is connected.
>>
>> Add a dmi table to flag these systems as having in-band presence disabled.
>>
>> Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
>> ---
>>  drivers/pci/hotplug/pciehp_hpc.c | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
>> index 02eb811a014f..4d377a2a62ce 100644
>> --- a/drivers/pci/hotplug/pciehp_hpc.c
>> +++ b/drivers/pci/hotplug/pciehp_hpc.c
>> @@ -14,6 +14,7 @@
>>  
>>  #define dev_fmt(fmt) "pciehp: " fmt
>>  
>> +#include <linux/dmi.h>
>>  #include <linux/kernel.h>
>>  #include <linux/types.h>
>>  #include <linux/jiffies.h>
>> @@ -26,6 +27,16 @@
>>  #include "../pci.h"
>>  #include "pciehp.h"
>>  
>> +static const struct dmi_system_id inband_presence_disabled_dmi_table[] = {
>> +	{
>> +		.ident = "Dell System",
>> +		.matches = {
>> +			DMI_MATCH(DMI_OEM_STRING, "Dell System"),
> 
> Sorry if this has been discussed previously already but isn't this going
> to apply on all Dell systems, not just the affected ones? Is this the
> intention?
> 

Yes, that is the intention. Applying this just makes the hotplug code wait for 
the presence detect bit to be set before proceeding, which ideally wouldn't hurt 
anything--for devices that don't have inband presence detect disabled, presence
detect should already be up when the code in patch 2/3 starts to wait for it.

The only issue should be with broken hotplug implementations that don't ever 
bring presence detect active (these apparently exist)--but even those would still 
work, they would just take an extra second to come up.

On the other hand, a number of Dell systems have (and will have) NVMe 
implementations that have inband presence detect disabled (but they won't have
the new bit implemented to report that), and they don't work correctly without 
this.

Re: [PATCH v3 3/3] PCI: pciehp: Add dmi table for in-band presence disabled

[Alex G.]

On 10/21/19 1:19 PM, Stuart Hayes wrote:
> 
> 
> On 10/21/19 8:47 AM, Mika Westerberg wrote:
>> On Thu, Oct 17, 2019 at 03:32:56PM -0400, Stuart Hayes wrote:
>>> Some systems have in-band presence detection disabled for hot-plug PCI
>>> slots, but do not report this in the slot capabilities 2 (SLTCAP2) register.
>>> On these systems, presence detect can become active well after the link is
>>> reported to be active, which can cause the slots to be disabled after a
>>> device is connected.
>>>
>>> Add a dmi table to flag these systems as having in-band presence disabled.
>>>
>>> Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
>>> ---
>>>   drivers/pci/hotplug/pciehp_hpc.c | 14 ++++++++++++++
>>>   1 file changed, 14 insertions(+)
>>>
>>> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
>>> index 02eb811a014f..4d377a2a62ce 100644
>>> --- a/drivers/pci/hotplug/pciehp_hpc.c
>>> +++ b/drivers/pci/hotplug/pciehp_hpc.c
>>> @@ -14,6 +14,7 @@
>>>   
>>>   #define dev_fmt(fmt) "pciehp: " fmt
>>>   
>>> +#include <linux/dmi.h>
>>>   #include <linux/kernel.h>
>>>   #include <linux/types.h>
>>>   #include <linux/jiffies.h>
>>> @@ -26,6 +27,16 @@
>>>   #include "../pci.h"
>>>   #include "pciehp.h"
>>>   
>>> +static const struct dmi_system_id inband_presence_disabled_dmi_table[] = {
>>> +	{
>>> +		.ident = "Dell System",
>>> +		.matches = {
>>> +			DMI_MATCH(DMI_OEM_STRING, "Dell System"),
>>
>> Sorry if this has been discussed previously already but isn't this going
>> to apply on all Dell systems, not just the affected ones? Is this the
>> intention?
>>
> 
> Yes, that is the intention. Applying this just makes the hotplug code wait for
> the presence detect bit to be set before proceeding, which ideally wouldn't hurt
> anything--for devices that don't have inband presence detect disabled, presence
> detect should already be up when the code in patch 2/3 starts to wait for it.
> 
> The only issue should be with broken hotplug implementations that don't ever
> bring presence detect active (these apparently exist)--but even those would still
> work, they would just take an extra second to come up.
> 
> On the other hand, a number of Dell systems have (and will have) NVMe
> implementations that have inband presence detect disabled (but they won't have
> the new bit implemented to report that), and they don't work correctly without
> this.

I think it's clearer if this is explained in a comment. That it doesn't 
break anything, and we're okay this applies to all hotplug ports, even 
those that are not in front of an NVMe backplane.

Alex

Re: [PATCH v3 3/3] PCI: pciehp: Add dmi table for in-band presence disabled

[Mika Westerberg]

On Mon, Oct 21, 2019 at 07:13:32PM -0500, Alex G. wrote:
> I think it's clearer if this is explained in a comment. That it doesn't
> break anything, and we're okay this applies to all hotplug ports, even those
> that are not in front of an NVMe backplane.

I agree.