Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1333214 > unrolled thread

[PATCH] pci: Wait for up to an additional 1000ms after FLR reset

Back to article view | Back to linux.kernel

Contents

  [PATCH] pci: Wait for up to an additional 1000ms after FLR reset Alex Williamson <alex.williamson@redhat.com> - 2016-02-13 01:00 +0100
    Re: [PATCH] pci: Wait for up to an additional 1000ms after FLR reset Bjorn Helgaas <helgaas@kernel.org> - 2016-02-16 18:00 +0100

#1333214 — [PATCH] pci: Wait for up to an additional 1000ms after FLR reset

Some devices take longer than the spec indicates to return from FLR
reset, a notable case of this is Intel integrated graphics (IGD),
which can often take an additional 300ms powering down an attached
LCD panel as part of the FLR.  Allow devices up to an additional
1000ms, testing every 100ms whether the first dword of config space
is read as -1.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

Copying KVM list as this patch is required for IGD assignment.

 drivers/pci/pci.c |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 602eb42..3b90a42 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3414,6 +3414,25 @@ int pci_wait_for_pending_transaction(struct pci_dev *dev)
 }
 EXPORT_SYMBOL(pci_wait_for_pending_transaction);
 
+static void pci_wait_alive(struct pci_dev *dev)
+{
+	int i;
+	u32 id;
+
+	for (i = 0; i < 10; i++) {
+		pci_read_config_dword(dev, PCI_VENDOR_ID, &id);
+		if (~id != 0) {
+			if (i > 0)
+				dev_info(&dev->dev, "Required additional %d"
+				         "ms to return from reset\n", i * 100);
+			return;
+		}
+		msleep(100);
+	}
+
+	dev_warn(&dev->dev, "Failed to return from reset\n");
+}
+
 static int pcie_flr(struct pci_dev *dev, int probe)
 {
 	u32 cap;
@@ -3430,6 +3449,7 @@ static int pcie_flr(struct pci_dev *dev, int probe)
 
 	pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_BCR_FLR);
 	msleep(100);
+	pci_wait_alive(dev);
 	return 0;
 }
 
@@ -3460,6 +3480,7 @@ static int pci_af_flr(struct pci_dev *dev, int probe)
 
 	pci_write_config_byte(dev, pos + PCI_AF_CTRL, PCI_AF_CTRL_FLR);
 	msleep(100);
+	pci_wait_alive(dev);
 	return 0;
 }
 

[toc] | [next] | [standalone]

#1335636

Hi Alex,

On Fri, Feb 12, 2016 at 04:51:08PM -0700, Alex Williamson wrote:
> Some devices take longer than the spec indicates to return from FLR
> reset, a notable case of this is Intel integrated graphics (IGD),
> which can often take an additional 300ms powering down an attached
> LCD panel as part of the FLR.  Allow devices up to an additional
> 1000ms, testing every 100ms whether the first dword of config space
> is read as -1.

This gives me a little more heartburn about ignoring presence detect
during reset and pretending that the post-reset device is the same as
the pre-reset device.  I suspect this problem wouldn't occur if we
just watched for a new device to show up after the FLR, because we
wouldn't have a timeout at all.

But given that this *is* the way we do it, ...

> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
> 
> Copying KVM list as this patch is required for IGD assignment.
> 
>  drivers/pci/pci.c |   21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 602eb42..3b90a42 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3414,6 +3414,25 @@ int pci_wait_for_pending_transaction(struct pci_dev *dev)
>  }
>  EXPORT_SYMBOL(pci_wait_for_pending_transaction);
>  
> +static void pci_wait_alive(struct pci_dev *dev)
> +{
> +	int i;
> +	u32 id;
> +
> +	for (i = 0; i < 10; i++) {
> +		pci_read_config_dword(dev, PCI_VENDOR_ID, &id);
> +		if (~id != 0) {
> +			if (i > 0)
> +				dev_info(&dev->dev, "Required additional %d"
> +				         "ms to return from reset\n", i * 100);
> +			return;
> +		}
> +		msleep(100);
> +	}
> +
> +	dev_warn(&dev->dev, "Failed to return from reset\n");
> +}
> +
>  static int pcie_flr(struct pci_dev *dev, int probe)
>  {
>  	u32 cap;
> @@ -3430,6 +3449,7 @@ static int pcie_flr(struct pci_dev *dev, int probe)
>  
>  	pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_BCR_FLR);
>  	msleep(100);
> +	pci_wait_alive(dev);

Can we fold the msleep() into pci_wait_alive(), so all the delay
happens in one place?  Maybe you can add a comment to the effect that
the spec says devices should be ready 100ms after FLR, but some
devices take longer.

>  	return 0;
>  }
>  
> @@ -3460,6 +3480,7 @@ static int pci_af_flr(struct pci_dev *dev, int probe)
>  
>  	pci_write_config_byte(dev, pos + PCI_AF_CTRL, PCI_AF_CTRL_FLR);
>  	msleep(100);
> +	pci_wait_alive(dev);
>  	return 0;
>  }
>  
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web