On Tue, Mar 3, 2020 at 2:30 PM Stanislav Spassov <stanspas@xxxxxxxxxx> wrote:
>
> From: Stanislav Spassov <stanspas@xxxxxxxxx>
>
> ...and replace several "magic numbers" scattered throughout the code.
What "magic numbers" exactly do you mean? The SRIOV and FLR delays?
Why not to be more specific here if so?
> Signed-off-by: Stanislav Spassov <stanspas@xxxxxxxxx>
> ---
> drivers/pci/controller/pci-aardvark.c | 2 +-
> drivers/pci/controller/pci-mvebu.c | 2 +-
> drivers/pci/iov.c | 4 +-
> drivers/pci/pci-acpi.c | 4 +-
> drivers/pci/pci.c | 21 ++------
> drivers/pci/pci.h | 72 +++++++++++++++++++++++++--
> 6 files changed, 80 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index 2a20b649f40c..2ece24abf751 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -344,7 +344,7 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
> * fundamental reset. As required by PCI Express spec a delay for at
> * least 100ms after such a reset before link training is needed.
> */
> - msleep(PCI_PM_D3COLD_WAIT);
> + msleep(PCI_RESET_DELAY);
>
> /* Start link training */
> reg = advk_readl(pcie, PCIE_CORE_LINK_CTRL_STAT_REG);
> diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
> index 153a64676bc9..c1cd1b7cf7ee 100644
> --- a/drivers/pci/controller/pci-mvebu.c
> +++ b/drivers/pci/controller/pci-mvebu.c
> @@ -927,7 +927,7 @@ static int mvebu_pcie_powerup(struct mvebu_pcie_port *port)
> return ret;
>
> if (port->reset_gpio) {
> - u32 reset_udelay = PCI_PM_D3COLD_WAIT * 1000;
> + u32 reset_udelay = PCI_RESET_DELAY * 1000;
>
> of_property_read_u32(port->dn, "reset-delay-us",
> &reset_udelay);
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 4d1f392b05f9..d4e4a0c0a97f 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -524,7 +524,7 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
> iov->ctrl |= PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE;
> pci_cfg_access_lock(dev);
> pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl);
> - msleep(100);
> + msleep(PCI_VF_ENABLE_DELAY);
> pci_cfg_access_unlock(dev);
>
> rc = sriov_add_vfs(dev, initial);
> @@ -735,7 +735,7 @@ static void sriov_restore_state(struct pci_dev *dev)
> pci_iov_set_numvfs(dev, iov->num_VFs);
> pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl);
> if (iov->ctrl & PCI_SRIOV_CTRL_VFE)
> - msleep(100);
> + msleep(PCI_VF_ENABLE_DELAY);
> }
>
> /**
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index b147b61c6668..0ece144ac9c5 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -1242,7 +1242,7 @@ static void pci_acpi_optimize_delay(struct pci_dev *pdev,
> value = (int)value_us / 1000;
> if ((int)value_us % 1000 > 0)
> value++;
> - if (value < PCI_PM_D3COLD_WAIT)
> + if (value < PCI_RESET_DELAY)
> pdev->d3cold_delay = value;
> }
> if (elements[3].type == ACPI_TYPE_INTEGER) {
> @@ -1250,7 +1250,7 @@ static void pci_acpi_optimize_delay(struct pci_dev *pdev,
> value = (int)value_us / 1000;
> if ((int)value_us % 1000 > 0)
> value++;
> - if (value < PCI_PM_D3_WAIT)
> + if (value < PCI_PM_D3HOT_DELAY)
> pdev->d3_delay = value;
> }
> }
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 4899b12b5a38..aaef00578487 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2844,8 +2844,8 @@ void pci_pm_init(struct pci_dev *dev)
>
> dev->pm_cap = pm;
> dev->ignore_reset_delay_on_sx_resume = 0;
> - dev->d3_delay = PCI_PM_D3_WAIT;
> - dev->d3cold_delay = PCI_PM_D3COLD_WAIT;
> + dev->d3_delay = PCI_PM_D3HOT_DELAY;
> + dev->d3cold_delay = PCI_RESET_DELAY;
> dev->bridge_d3 = pci_bridge_d3_possible(dev);
> dev->d3cold_allowed = true;
>
> @@ -4500,12 +4500,7 @@ int pcie_flr(struct pci_dev *dev)
> if (dev->imm_ready)
> return 0;
>
> - /*
> - * Per PCIe r4.0, sec 6.6.2, a device must complete an FLR within
> - * 100ms, but may silently discard requests while the FLR is in
> - * progress. Wait 100ms before trying to access the device.
> - */
> - msleep(100);
> + msleep(PCI_FLR_DELAY);
>
> return pci_dev_wait(dev, "FLR", PCIE_RESET_READY_POLL_MS);
> }
> @@ -4544,13 +4539,7 @@ static int pci_af_flr(struct pci_dev *dev, int probe)
> if (dev->imm_ready)
> return 0;
>
> - /*
> - * Per Advanced Capabilities for Conventional PCI ECN, 13 April 2006,
> - * updated 27 July 2006; a device must complete an FLR within
> - * 100ms, but may silently discard requests while the FLR is in
> - * progress. Wait 100ms before trying to access the device.
> - */
> - msleep(100);
> + msleep(PCI_FLR_DELAY);
>
> return pci_dev_wait(dev, "AF_FLR", PCIE_RESET_READY_POLL_MS);
> }
> @@ -4590,7 +4579,7 @@ static int pci_pm_reset(struct pci_dev *dev, int probe)
> csr &= ~PCI_PM_CTRL_STATE_MASK;
> csr |= PCI_D3hot;
> pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, csr);
> - msleep(PCI_PM_D3_WAIT);
> + msleep(PCI_PM_D3HOT_DELAY);
>
> csr &= ~PCI_PM_CTRL_STATE_MASK;
> csr |= PCI_D0;
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index c4c3ba926f45..9b5dd6ea2f52 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -43,9 +43,75 @@ int pci_probe_reset_function(struct pci_dev *dev);
> int pci_bridge_secondary_bus_reset(struct pci_dev *dev);
> int pci_bus_error_reset(struct pci_dev *dev);
>
> -#define PCI_PM_D2_DELAY 200
> -#define PCI_PM_D3_WAIT 10
> -#define PCI_PM_D3COLD_WAIT 100
> +/*
> + * These constants represent the minimum amounts of time mandated by the
> + * PCI Express Base specification that software needs to wait after
> + * various PCI device events involving (re-)initialization. Only after
> + * the appropriate delay has elapsed, is software permitted to issue
> + * Configuration Requests targeting the affected device.
> + *
> + * Relevant sections in PCI Express Base Specification r5.0 (May 22, 2019):
> + * - 6.6.1 "Conventional Reset" for PCI_RESET_DELAY and PCI_DL_UP_DELAY
> + * - 6.6.2 "Function Level Reset" for PCI_FLR_DELAY
> + * - 5.9 "State Transition Recovery Time Requirements" for PCI_PM_D3HOT_DELAY
> + * and PCI_PM_D2_DELAY
> + * - 9.3.3.3.1 "VF Enable" for PCI_VF_ENABLE_DELAY
> + *
> + * There are mechanisms to reduce some of the delay values for specific devices:
> + * - a device may expose the Readiness Time Reporting Extended Capability from:
> + * PCI Express Base Specification r4.0 (September 27, 2017), sec 7.9.17
> + * (This is currently not supported by the kernel.)
> + * - system firmware may provide overrides using an ACPI _DSM Function 9:
> + * PCI Firmware Specification r3.2 (January 26, 2015), sec 4.6.9
> + * (see pci_acpi_optimize_delay)
> + *
> + * Unless overridden by _DSM Function 9, other mechanisms may be used to reduce
> + * or completely avoid some of the delays:
> + * - Readiness Notifications (DRS and FRS)
> + * - the Immediate Readiness bit of the Status Register in the PCI header
> + * - the Immediate_Readiness_on_Return_to_D0 in the Power Management
> + * Capabilities Register in the PCI Power Management Capability
> + * (None of these are currently supported by the kernel.)
> + *
> + * Note: While devices are required to be responsive to Configuration
> + * Requests after these delays, they may not respond with Successful
> + * Completion status until they complete potentially lengthy internal
> + * initialization sequences. Instead, devices respond with Configuration
> + * Request Retry Status (CRS) Completions. Therefore, additional waiting
> + * is necessary as handled by pci_dev_wait().
> + */
> +/*
> + * Conventional (non-FLR) reset delay, including D3cold->D0 transitions,
> + * Secondary Bus Reset, and any platform-specific means of triggering
> + * a Conventional Reset.
> + *
> + * According to PCI Firmware spec r3.2, sec 4.6.9, for devices beneath
> + * downstream ports supporting the Data Link Layer Active Reporting
> + * capability, this delay should not be used (see PCI_DL_UP_DELAY).
> + */
> +#define PCI_RESET_DELAY 100
> +/*
> + * Post-DL_Up (Data Link Layer Active) delay applicable for devices immediately
> + * under a Downstream Port that is capable of reporting Data Link Layer Ready.
> + * Not to be confused with how much time it takes for the link itself to become
> + * active (see pcie_wait_for_link_delay).
> + */
> +#define PCI_DL_UP_DELAY 100
> +/*
> + * Post-FLR delay
> + * Also applies to legacy devices supporting AF_FLR per Advanced Capabilities
> + * for Conventional PCI ECN, 13 April 2006, updated 27 July 2006)
> + */
> +#define PCI_FLR_DELAY 100
> +/*
> + * D0/D1/D2->D3hot and D3hot->D0 delay
> + * The specifications do *not* mention overridability of the ->D3hot direction
> + */
> +#define PCI_PM_D3HOT_DELAY 10
> +/* Post-VF_Enable delay */
> +#define PCI_VF_ENABLE_DELAY 100
> +/* D0/D1->D2 and D2->D0 delay */
> +#define PCI_PM_D2_DELAY 200
>
> /**
> * struct pci_platform_pm_ops - Firmware PM callbacks
> --
> 2.25.1
>
>
>
>
> Amazon Development Center Germany GmbH
> Krausenstr. 38
> 10117 Berlin
> Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
> Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
> Sitz: Berlin
> Ust-ID: DE 289 237 879
>
>
>
