LKML Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/3] PCI: pciehp: Do not turn off slot if presence comes up after link
@ 2019-10-09 20:05 Stuart Hayes
  2019-10-09 20:05 ` [PATCH 1/3] PCI: pciehp: Add support for disabling in-band presence Stuart Hayes
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Stuart Hayes @ 2019-10-09 20:05 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Austin Bolen, keith.busch, Alexandru Gagniuc, Rafael J . Wysocki,
	Mika Westerberg, Andy Shevchenko, Gustavo A . R . Silva,
	Sinan Kaya, Oza Pawandeep, linux-pci, linux-kernel, lukas,
	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

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/3] PCI: pciehp: Add support for disabling in-band presence
  2019-10-09 20:05 [PATCH 0/3] PCI: pciehp: Do not turn off slot if presence comes up after link Stuart Hayes
@ 2019-10-09 20:05 ` Stuart Hayes
  2019-10-09 20:05 ` [PATCH 2/3] PCI: pciehp: Wait for PDS if in-band presence is disabled Stuart Hayes
  2019-10-09 20:05 ` [PATCH 3/3] PCI: pciehp: Add dmi table for in-band presence disabled Stuart Hayes
  2 siblings, 0 replies; 15+ messages in thread
From: Stuart Hayes @ 2019-10-09 20:05 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Austin Bolen, keith.busch, Alexandru Gagniuc, Rafael J . Wysocki,
	Mika Westerberg, Andy Shevchenko, Gustavo A . R . Silva,
	Sinan Kaya, Oza Pawandeep, linux-pci, linux-kernel, lukas

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/3] PCI: pciehp: Wait for PDS if in-band presence is disabled
  2019-10-09 20:05 [PATCH 0/3] PCI: pciehp: Do not turn off slot if presence comes up after link Stuart Hayes
  2019-10-09 20:05 ` [PATCH 1/3] PCI: pciehp: Add support for disabling in-band presence Stuart Hayes
@ 2019-10-09 20:05 ` Stuart Hayes
  2019-10-10  5:37   ` Andy Shevchenko
  2019-10-09 20:05 ` [PATCH 3/3] PCI: pciehp: Add dmi table for in-band presence disabled Stuart Hayes
  2 siblings, 1 reply; 15+ messages in thread
From: Stuart Hayes @ 2019-10-09 20:05 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Austin Bolen, keith.busch, Alexandru Gagniuc, Rafael J . Wysocki,
	Mika Westerberg, Andy Shevchenko, Gustavo A . R . Silva,
	Sinan Kaya, Oza Pawandeep, linux-pci, linux-kernel, lukas,
	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>
---
 drivers/pci/hotplug/pciehp_hpc.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index dc109d521f30..1282641c6458 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -242,6 +242,25 @@ 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;
+	bool pds;
+	u16 slot_status;
+
+	while (true) {
+		pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
+		pds = !!(slot_status & PCI_EXP_SLTSTA_PDS);
+		if (pds || timeout <= 0)
+			break;
+		msleep(10);
+		timeout -= 10;
+	}
+
+	if (!pds)
+		pci_info(pdev, "Presence Detect state not set in 1250 msec\n");
+}
+
 int pciehp_check_link_status(struct controller *ctrl)
 {
 	struct pci_dev *pdev = ctrl_dev(ctrl);
@@ -251,6 +270,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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 3/3] PCI: pciehp: Add dmi table for in-band presence disabled
  2019-10-09 20:05 [PATCH 0/3] PCI: pciehp: Do not turn off slot if presence comes up after link Stuart Hayes
  2019-10-09 20:05 ` [PATCH 1/3] PCI: pciehp: Add support for disabling in-band presence Stuart Hayes
  2019-10-09 20:05 ` [PATCH 2/3] PCI: pciehp: Wait for PDS if in-band presence is disabled Stuart Hayes
@ 2019-10-09 20:05 ` Stuart Hayes
  2 siblings, 0 replies; 15+ messages in thread
From: Stuart Hayes @ 2019-10-09 20:05 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Austin Bolen, keith.busch, Alexandru Gagniuc, Rafael J . Wysocki,
	Mika Westerberg, Andy Shevchenko, Gustavo A . R . Silva,
	Sinan Kaya, Oza Pawandeep, linux-pci, linux-kernel, lukas,
	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 1282641c6458..cabd745b844e 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;
@@ -898,6 +909,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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] PCI: pciehp: Wait for PDS if in-band presence is disabled
  2019-10-09 20:05 ` [PATCH 2/3] PCI: pciehp: Wait for PDS if in-band presence is disabled Stuart Hayes
@ 2019-10-10  5:37   ` Andy Shevchenko
  2019-10-10  5:40     ` Andy Shevchenko
  0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2019-10-10  5:37 UTC (permalink / raw)
  To: Stuart Hayes
  Cc: Bjorn Helgaas, Austin Bolen, Keith Busch, Alexandru Gagniuc,
	Rafael J . Wysocki, Mika Westerberg, Gustavo A . R . Silva,
	Sinan Kaya, Oza Pawandeep, linux-pci, Linux Kernel Mailing List,
	Lukas Wunner

On Wed, Oct 9, 2019 at 11:05 PM Stuart Hayes <stuart.w.hayes@gmail.com> 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.

> +static void pcie_wait_for_presence(struct pci_dev *pdev)
> +{
> +       int timeout = 1250;
> +       bool pds;
> +       u16 slot_status;
> +
> +       while (true) {
> +               pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
> +               pds = !!(slot_status & PCI_EXP_SLTSTA_PDS);
> +               if (pds || timeout <= 0)
> +                       break;
> +               msleep(10);
> +               timeout -= 10;
> +       }

Can we avoid infinite loops? They are hard to parse (in most cases,
and especially when it's a timeout loop)

unsigned int retries = 125; // 1250 ms

do {
 ...
} while (--retries);

> +
> +       if (!pds)
> +               pci_info(pdev, "Presence Detect state not set in 1250 msec\n");
> +}

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] PCI: pciehp: Wait for PDS if in-band presence is disabled
  2019-10-10  5:37   ` Andy Shevchenko
@ 2019-10-10  5:40     ` Andy Shevchenko
  2019-10-10 20:37       ` Stuart Hayes
  0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2019-10-10  5:40 UTC (permalink / raw)
  To: Stuart Hayes
  Cc: Bjorn Helgaas, Austin Bolen, Keith Busch, Alexandru Gagniuc,
	Rafael J . Wysocki, Mika Westerberg, Gustavo A . R . Silva,
	Sinan Kaya, Oza Pawandeep, linux-pci, Linux Kernel Mailing List,
	Lukas Wunner

On Thu, Oct 10, 2019 at 8:37 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Oct 9, 2019 at 11:05 PM Stuart Hayes <stuart.w.hayes@gmail.com> wrote:

> > +static void pcie_wait_for_presence(struct pci_dev *pdev)
> > +{
> > +       int timeout = 1250;

> > +       bool pds;

Also this is redundant. Just use the following outside the loop

 if (!retries)
   pc_info(...);

.

> > +       u16 slot_status;
> > +
> > +       while (true) {
> > +               pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
> > +               pds = !!(slot_status & PCI_EXP_SLTSTA_PDS);
> > +               if (pds || timeout <= 0)
> > +                       break;
> > +               msleep(10);
> > +               timeout -= 10;
> > +       }
>
> Can we avoid infinite loops? They are hard to parse (in most cases,
> and especially when it's a timeout loop)
>
> unsigned int retries = 125; // 1250 ms
>
> do {
>  ...
> } while (--retries);
>
> > +
> > +       if (!pds)
> > +               pci_info(pdev, "Presence Detect state not set in 1250 msec\n");
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] PCI: pciehp: Wait for PDS if in-band presence is disabled
  2019-10-10  5:40     ` Andy Shevchenko
@ 2019-10-10 20:37       ` Stuart Hayes
  2019-10-11  6:49         ` Andy Shevchenko
  0 siblings, 1 reply; 15+ messages in thread
From: Stuart Hayes @ 2019-10-10 20:37 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Bjorn Helgaas, Austin Bolen, Keith Busch, Alexandru Gagniuc,
	Rafael J . Wysocki, Mika Westerberg, Gustavo A . R . Silva,
	Sinan Kaya, Oza Pawandeep, linux-pci, Linux Kernel Mailing List,
	Lukas Wunner

On Thu, Oct 10, 2019 at 12:40 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Thu, Oct 10, 2019 at 8:37 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Wed, Oct 9, 2019 at 11:05 PM Stuart Hayes <stuart.w.hayes@gmail.com> wrote:
>
> > > +static void pcie_wait_for_presence(struct pci_dev *pdev)
> > > +{
> > > +       int timeout = 1250;
>
> > > +       bool pds;
>
> Also this is redundant. Just use the following outside the loop
>
>  if (!retries)
>    pc_info(...);
>
> .
>
> > > +       u16 slot_status;
> > > +
> > > +       while (true) {
> > > +               pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
> > > +               pds = !!(slot_status & PCI_EXP_SLTSTA_PDS);
> > > +               if (pds || timeout <= 0)
> > > +                       break;
> > > +               msleep(10);
> > > +               timeout -= 10;
> > > +       }
> >
> > Can we avoid infinite loops? They are hard to parse (in most cases,
> > and especially when it's a timeout loop)
> >
> > unsigned int retries = 125; // 1250 ms
> >
> > do {
> >  ...
> > } while (--retries);
> >
> > > +
> > > +       if (!pds)
> > > +               pci_info(pdev, "Presence Detect state not set in 1250 msec\n");
> > > +}
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
>
>
>
> --
> With Best Regards,
> Andy Shevchenko

Thank you for the feedback!  An infinite loop is used several other places in
this driver--this keeps the style similar.  I can change it as you suggest,
though, if that would be preferable to consistency.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] PCI: pciehp: Wait for PDS if in-band presence is disabled
  2019-10-10 20:37       ` Stuart Hayes
@ 2019-10-11  6:49         ` Andy Shevchenko
  2019-10-11  8:48           ` Andy Shevchenko
  0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2019-10-11  6:49 UTC (permalink / raw)
  To: Stuart Hayes
  Cc: Bjorn Helgaas, Austin Bolen, Keith Busch, Alexandru Gagniuc,
	Rafael J . Wysocki, Mika Westerberg, Gustavo A . R . Silva,
	Sinan Kaya, Oza Pawandeep, linux-pci, Linux Kernel Mailing List,
	Lukas Wunner

On Thu, Oct 10, 2019 at 11:37 PM Stuart Hayes <stuart.w.hayes@gmail.com> wrote:

> Thank you for the feedback!  An infinite loop is used several other places in
> this driver--this keeps the style similar.  I can change it as you suggest,
> though, if that would be preferable to consistency.

Better to start the change now. I'll look into the file and see how we
can improve the rest.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] PCI: pciehp: Wait for PDS if in-band presence is disabled
  2019-10-11  6:49         ` Andy Shevchenko
@ 2019-10-11  8:48           ` Andy Shevchenko
  2019-10-11  9:05             ` Andy Shevchenko
  0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2019-10-11  8:48 UTC (permalink / raw)
  To: Stuart Hayes
  Cc: Bjorn Helgaas, Austin Bolen, Keith Busch, Alexandru Gagniuc,
	Rafael J . Wysocki, Mika Westerberg, Gustavo A . R . Silva,
	Sinan Kaya, Oza Pawandeep, linux-pci, Linux Kernel Mailing List,
	Lukas Wunner

On Fri, Oct 11, 2019 at 9:49 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Thu, Oct 10, 2019 at 11:37 PM Stuart Hayes <stuart.w.hayes@gmail.com> wrote:
>
> > Thank you for the feedback!  An infinite loop is used several other places in
> > this driver--this keeps the style similar.  I can change it as you suggest,
> > though, if that would be preferable to consistency.
>
> Better to start the change now. I'll look into the file and see how we
> can improve the rest.

I found only one infinite loop there, the other timeout loop is done
as do {} while.
I'll send a patch to refactor the infinite one.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] PCI: pciehp: Wait for PDS if in-band presence is disabled
  2019-10-11  8:48           ` Andy Shevchenko
@ 2019-10-11  9:05             ` Andy Shevchenko
  0 siblings, 0 replies; 15+ messages in thread
From: Andy Shevchenko @ 2019-10-11  9:05 UTC (permalink / raw)
  To: Stuart Hayes
  Cc: Bjorn Helgaas, Austin Bolen, Keith Busch, Alexandru Gagniuc,
	Rafael J . Wysocki, Mika Westerberg, Gustavo A . R . Silva,
	Sinan Kaya, Oza Pawandeep, linux-pci, Linux Kernel Mailing List,
	Lukas Wunner

On Fri, Oct 11, 2019 at 11:48 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Fri, Oct 11, 2019 at 9:49 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Thu, Oct 10, 2019 at 11:37 PM Stuart Hayes <stuart.w.hayes@gmail.com> wrote:
> >
> > > Thank you for the feedback!  An infinite loop is used several other places in
> > > this driver--this keeps the style similar.  I can change it as you suggest,
> > > though, if that would be preferable to consistency.
> >
> > Better to start the change now. I'll look into the file and see how we
> > can improve the rest.
>
> I found only one infinite loop there, the other timeout loop is done
> as do {} while.
> I'll send a patch to refactor the infinite one.

https://lore.kernel.org/linux-pci/20191011090230.81080-1-andriy.shevchenko@linux.intel.com/T/#u

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/3] PCI: pciehp: Do not turn off slot if presence comes up after link
  2019-10-02 22:13   ` Alex G.
@ 2019-10-03  3:33     ` Lukas Wunner
  0 siblings, 0 replies; 15+ messages in thread
From: Lukas Wunner @ 2019-10-03  3:33 UTC (permalink / raw)
  To: Alex G.
  Cc: Stuart Hayes, Bjorn Helgaas, Austin Bolen, keith.busch,
	Rafael J . Wysocki, Mika Westerberg, Andy Shevchenko,
	Gustavo A . R . Silva, Sinan Kaya, Oza Pawandeep, linux-pci,
	linux-kernel

On Wed, Oct 02, 2019 at 05:13:58PM -0500, Alex G. wrote:
> On 10/1/19 11:13 PM, Lukas Wunner wrote:
> > On Tue, Oct 01, 2019 at 05:14:16PM -0400, Stuart Hayes wrote:
> > > This patch set is based on a patch set [1] submitted many months ago by
> > > Alexandru Gagniuc, who is no longer working on it.
> > 
> > I'm not sure if it's appropriate to change the author and
> > omit Alex' Signed-off-by.
> 
> Legally Dell owns the patches. I have no objection on my end.

From a kernel community POV, I don't think it matters (in this case)
who legally owns the copyright to the contributed code.  It's just that
we go to great lengths to provide proper attribution even for small
contributions (e.g. Tested-by).

The benefit to the community is that we know who to cc if that portion
of the code is changed again and someone knowledgable should take a look.

The benefit to contributors is that they can change jobs and their past
contributions are still visible in the git history and attributed to
their names.  By contrast, if you've worked on closed source code and
changed jobs, that work isn't visible to future employers or even yourself,
and it may happen that someone else takes credit for your past work
without you even knowing about it or being able to stop that.
(I've seen it before.)

In this case, there should be a S-o-b line for Alex preceding that
for Stuart, and the author of the commit should be Alex unless a
significant portion of the patch was changed.

Thanks,

Lukas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/3] PCI: pciehp: Do not turn off slot if presence comes up after link
  2019-10-02  4:13 ` Lukas Wunner
  2019-10-02  4:31   ` Stuart Hayes
@ 2019-10-02 22:13   ` Alex G.
  2019-10-03  3:33     ` Lukas Wunner
  1 sibling, 1 reply; 15+ messages in thread
From: Alex G. @ 2019-10-02 22:13 UTC (permalink / raw)
  To: Lukas Wunner, Stuart Hayes
  Cc: Bjorn Helgaas, Austin Bolen, keith.busch, Rafael J . Wysocki,
	Mika Westerberg, Andy Shevchenko, Gustavo A . R . Silva,
	Sinan Kaya, Oza Pawandeep, linux-pci, linux-kernel

On 10/1/19 11:13 PM, Lukas Wunner wrote:
> On Tue, Oct 01, 2019 at 05:14:16PM -0400, Stuart Hayes wrote:
>> This patch set is based on a patch set [1] submitted many months ago by
>> Alexandru Gagniuc, who is no longer working on it.
>>
>> [1] https://patchwork.kernel.org/cover/10909167/
>>      [v3,0/4] PCI: pciehp: Do not turn off slot if presence comes up after link
> 
> If I'm not mistaken, these two are identical to Alex' patches, right?
> 
>    PCI: pciehp: Add support for disabling in-band presence
>    PCI: pciehp: Wait for PDS when in-band presence is disabled
> 
> I'm not sure if it's appropriate to change the author and
> omit Alex' Signed-off-by.

Legally Dell owns the patches. I have no objection on my end.

Alex

> Otherwise I have no objections against this series.
> 
> Thanks,
> 
> Lukas
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/3] PCI: pciehp: Do not turn off slot if presence comes up after link
  2019-10-02  4:13 ` Lukas Wunner
@ 2019-10-02  4:31   ` Stuart Hayes
  2019-10-02 22:13   ` Alex G.
  1 sibling, 0 replies; 15+ messages in thread
From: Stuart Hayes @ 2019-10-02  4:31 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Bjorn Helgaas, Austin Bolen, keith.busch, Alexandru Gagniuc,
	Rafael J . Wysocki, Mika Westerberg, Andy Shevchenko,
	Gustavo A . R . Silva, Sinan Kaya, Oza Pawandeep, linux-pci,
	Linux Kernel Mailing List

On Tue, Oct 1, 2019 at 11:13 PM Lukas Wunner <lukas@wunner.de> wrote:
>
> On Tue, Oct 01, 2019 at 05:14:16PM -0400, Stuart Hayes wrote:
> > This patch set is based on a patch set [1] submitted many months ago by
> > Alexandru Gagniuc, who is no longer working on it.
> >
> > [1] https://patchwork.kernel.org/cover/10909167/
> >     [v3,0/4] PCI: pciehp: Do not turn off slot if presence comes up after link
>
> If I'm not mistaken, these two are identical to Alex' patches, right?
>
>   PCI: pciehp: Add support for disabling in-band presence
>   PCI: pciehp: Wait for PDS when in-band presence is disabled
>
> I'm not sure if it's appropriate to change the author and
> omit Alex' Signed-off-by.
>
> Otherwise I have no objections against this series.
>
> Thanks,
>
> Lukas

Thanks!  The first patch is identical to the one Alex submitted, and
the second is nearly so... they both basically his work.  I wasn't
sure what proper etiquette was--I was thinking the signed-off-by was
taking responsibility that the patch was ok (functional, not
copyrighted by someone else, etc) rather than giving credit, but he
definitely deserves credit for them.  I'm happy to add a signed-off-by
for Alex on the first two and resubmit if he doesn't object.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/3] PCI: pciehp: Do not turn off slot if presence comes up after link
  2019-10-01 21:14 [PATCH 0/3] PCI: pciehp: Do not turn off slot if presence comes up after link Stuart Hayes
@ 2019-10-02  4:13 ` Lukas Wunner
  2019-10-02  4:31   ` Stuart Hayes
  2019-10-02 22:13   ` Alex G.
  0 siblings, 2 replies; 15+ messages in thread
From: Lukas Wunner @ 2019-10-02  4:13 UTC (permalink / raw)
  To: Stuart Hayes
  Cc: Bjorn Helgaas, Austin Bolen, keith.busch, Alexandru Gagniuc,
	Rafael J . Wysocki, Mika Westerberg, Andy Shevchenko,
	Gustavo A . R . Silva, Sinan Kaya, Oza Pawandeep, linux-pci,
	linux-kernel

On Tue, Oct 01, 2019 at 05:14:16PM -0400, Stuart Hayes wrote:
> This patch set is based on a patch set [1] submitted many months ago by
> Alexandru Gagniuc, who is no longer working on it.
> 
> [1] https://patchwork.kernel.org/cover/10909167/
>     [v3,0/4] PCI: pciehp: Do not turn off slot if presence comes up after link

If I'm not mistaken, these two are identical to Alex' patches, right?

  PCI: pciehp: Add support for disabling in-band presence
  PCI: pciehp: Wait for PDS when in-band presence is disabled

I'm not sure if it's appropriate to change the author and
omit Alex' Signed-off-by.

Otherwise I have no objections against this series.

Thanks,

Lukas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 0/3] PCI: pciehp: Do not turn off slot if presence comes up after link
@ 2019-10-01 21:14 Stuart Hayes
  2019-10-02  4:13 ` Lukas Wunner
  0 siblings, 1 reply; 15+ messages in thread
From: Stuart Hayes @ 2019-10-01 21:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Austin Bolen, keith.busch, Alexandru Gagniuc, Rafael J . Wysocki,
	Mika Westerberg, Andy Shevchenko, Gustavo A . R . Silva,
	Sinan Kaya, Oza Pawandeep, linux-pci, linux-kernel, lukas,
	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.

This patch set is based on a patch set [1] submitted many months ago by
Alexandru Gagniuc, who is no longer working on it.

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

Stuart Hayes (3):
  PCI: pciehp: Add support for disabling in-band presence
  PCI: pciehp: Wait for PDS when in-band presence is disabled
  PCI: pciehp: Add dmi table for systems with 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


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, back to index

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-09 20:05 [PATCH 0/3] PCI: pciehp: Do not turn off slot if presence comes up after link Stuart Hayes
2019-10-09 20:05 ` [PATCH 1/3] PCI: pciehp: Add support for disabling in-band presence Stuart Hayes
2019-10-09 20:05 ` [PATCH 2/3] PCI: pciehp: Wait for PDS if in-band presence is disabled Stuart Hayes
2019-10-10  5:37   ` Andy Shevchenko
2019-10-10  5:40     ` Andy Shevchenko
2019-10-10 20:37       ` Stuart Hayes
2019-10-11  6:49         ` Andy Shevchenko
2019-10-11  8:48           ` Andy Shevchenko
2019-10-11  9:05             ` Andy Shevchenko
2019-10-09 20:05 ` [PATCH 3/3] PCI: pciehp: Add dmi table for in-band presence disabled Stuart Hayes
  -- strict thread matches above, loose matches on Subject: below --
2019-10-01 21:14 [PATCH 0/3] PCI: pciehp: Do not turn off slot if presence comes up after link Stuart Hayes
2019-10-02  4:13 ` Lukas Wunner
2019-10-02  4:31   ` Stuart Hayes
2019-10-02 22:13   ` Alex G.
2019-10-03  3:33     ` Lukas Wunner

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox