[PATCH v4 1/2] PCI: xilinx: Wait for link-up status during initialization

[PATCH v4 1/2] PCI: xilinx: Wait for link-up status during initialization

[Mike Looijmans]

When the driver loads, the transceiver and endpoint may still be setting
up a link. Wait for that to complete before continuing. This fixes that
the PCIe core does not work when loading the PL bitstream from
userspace. Existing reference designs worked because the endpoint and
PL were initialized by a bootloader. If the endpoint power and/or reset
is supplied by the kernel, or if the PL is programmed from within the
kernel, the link won't be up yet and the driver just has to wait for
link training to finish.

As the PCIe spec allows up to 100 ms time to establish a link, we'll
allow up to 200ms before giving up.

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---

Changes in v4:
Adapt patch description to "PCI: xilinx:" format
Explain the initialization order issue better
Use PCIE_T_RRS_READY_MS instead of 100ms

Changes in v2:
Split into "reset GPIO" and "wait for link" patches
Add timeout explanation

 drivers/pci/controller/pcie-xilinx.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
index e36aa874bae9..e8a07535539e 100644
--- a/drivers/pci/controller/pcie-xilinx.c
+++ b/drivers/pci/controller/pcie-xilinx.c
@@ -15,6 +15,7 @@
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
+#include <linux/iopoll.h>
 #include <linux/msi.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
@@ -126,6 +127,19 @@ static inline bool xilinx_pcie_link_up(struct xilinx_pcie *pcie)
 		XILINX_PCIE_REG_PSCR_LNKUP) ? 1 : 0;
 }
 
+static int xilinx_pci_wait_link_up(struct xilinx_pcie *pcie)
+{
+	u32 val;
+
+	/*
+	 * PCIe r6.0, sec 6.6.1 provides 100ms timeout. Since this is FPGA
+	 * fabric, we're more lenient and allow 200 ms for link training.
+	 */
+	return readl_poll_timeout(pcie->reg_base + XILINX_PCIE_REG_PSCR, val,
+			(val & XILINX_PCIE_REG_PSCR_LNKUP), 2 * USEC_PER_MSEC,
+			2 * PCIE_T_RRS_READY_MS * USEC_PER_MSEC);
+}
+
 /**
  * xilinx_pcie_clear_err_interrupts - Clear Error Interrupts
  * @pcie: PCIe port information
@@ -492,7 +506,7 @@ static void xilinx_pcie_init_port(struct xilinx_pcie *pcie)
 {
 	struct device *dev = pcie->dev;
 
-	if (xilinx_pcie_link_up(pcie))
+	if (!xilinx_pci_wait_link_up(pcie))
 		dev_info(dev, "PCIe Link is UP\n");
 	else
 		dev_info(dev, "PCIe Link is DOWN\n");
-- 
2.43.0

base-commit: f09079bd04a924c72d555cd97942d5f8d7eca98c
branch: linux-master-pci-reset

Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl

Please consider the environment before printing this e-mail

[PATCH v4 2/2] PCI: xilinx: Support reset GPIO for PERST#

[Mike Looijmans]

Support providing the PERST# reset signal through a devicetree binding.
Thus the system no longer relies on external components to perform the
bus reset.

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---

Changes in v4:
Adapt patch description to "PCI: xilinx:" format

Changes in v3:
Include linux/gpio/consumer.h
Message "Failed to request reset GPIO"
Use PCIE_T_PVPERL_MS and PCIE_T_RRS_READY_MS
Dropped dt-bindings patch, not needed

Changes in v2:
Split into "reset GPIO" and "wait for link" patches
Handle GPIO defer and/or errors

 drivers/pci/controller/pcie-xilinx.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
index e8a07535539e..92aa4f3528aa 100644
--- a/drivers/pci/controller/pcie-xilinx.c
+++ b/drivers/pci/controller/pcie-xilinx.c
@@ -10,6 +10,7 @@
  * ARM PCI Host generic driver.
  */
 
+#include <linux/gpio/consumer.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
@@ -576,11 +577,17 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct xilinx_pcie *pcie;
 	struct pci_host_bridge *bridge;
+	struct gpio_desc *perst_gpio;
 	int err;
 
 	if (!dev->of_node)
 		return -ENODEV;
 
+	perst_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(perst_gpio))
+		return dev_err_probe(dev, PTR_ERR(perst_gpio),
+				     "Failed to request reset GPIO\n");
+
 	bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));
 	if (!bridge)
 		return -ENODEV;
@@ -595,6 +602,13 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
 		return err;
 	}
 
+	if (perst_gpio) {
+		msleep(PCIE_T_PVPERL_MS); /* Minimum assertion time */
+		gpiod_set_value_cansleep(perst_gpio, 0);
+		/* Initial delay to provide endpoint time to initialize */
+		msleep(PCIE_T_RRS_READY_MS);
+	}
+
 	xilinx_pcie_init_port(pcie);
 
 	err = xilinx_pcie_init_irq_domain(pcie);
-- 
2.43.0

base-commit: f09079bd04a924c72d555cd97942d5f8d7eca98c
branch: linux-master-pci-reset

Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl

Please consider the environment before printing this e-mail

Re: [PATCH v4 2/2] PCI: xilinx: Support reset GPIO for PERST#

[Bjorn Helgaas]

On Tue, Jun 10, 2025 at 04:39:04PM +0200, Mike Looijmans wrote:
> Support providing the PERST# reset signal through a devicetree binding.
> Thus the system no longer relies on external components to perform the
> bus reset.

> @@ -576,11 +577,17 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct xilinx_pcie *pcie;
>  	struct pci_host_bridge *bridge;
> +	struct gpio_desc *perst_gpio;
>  	int err;
>  
>  	if (!dev->of_node)
>  		return -ENODEV;
>  
> +	perst_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(perst_gpio))
> +		return dev_err_probe(dev, PTR_ERR(perst_gpio),
> +				     "Failed to request reset GPIO\n");
> +
>  	bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));
>  	if (!bridge)
>  		return -ENODEV;
> @@ -595,6 +602,13 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
>  		return err;
>  	}
>  
> +	if (perst_gpio) {
> +		msleep(PCIE_T_PVPERL_MS); /* Minimum assertion time */
> +		gpiod_set_value_cansleep(perst_gpio, 0);

Are we assured that PERST# was already asserted when we entered
xilinx_pcie_probe()?

> +		/* Initial delay to provide endpoint time to initialize */
> +		msleep(PCIE_T_RRS_READY_MS);

I don't think this is the right spot for PCIE_T_RRS_READY_MS, details
in https://lore.kernel.org/r/20250610185734.GA819344@bhelgaas

I guess the spec assumes that for ports that don't support speeds
greater than 5.0 GT/s, 100ms is enough for the link to come up *and*
the endpoint to initialize.  But since you're going to wait for the
link to come up immediately *after* this PCIE_T_RRS_READY_MS sleep, I
would think you could extend the timeout in xilinx_pci_wait_link_up()
and then do the PCIE_T_RRS_READY_MS sleep.

Re: [PATCH v4 2/2] PCI: xilinx: Support reset GPIO for PERST#

[Mike Looijmans]

On 10-06-2025 21:07, Bjorn Helgaas wrote:
> On Tue, Jun 10, 2025 at 04:39:04PM +0200, Mike Looijmans wrote:
>> Support providing the PERST# reset signal through a devicetree binding.
>> Thus the system no longer relies on external components to perform the
>> bus reset.
>> @@ -576,11 +577,17 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
>>   	struct device *dev = &pdev->dev;
>>   	struct xilinx_pcie *pcie;
>>   	struct pci_host_bridge *bridge;
>> +	struct gpio_desc *perst_gpio;
>>   	int err;
>>   
>>   	if (!dev->of_node)
>>   		return -ENODEV;
>>   
>> +	perst_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
>> +	if (IS_ERR(perst_gpio))
>> +		return dev_err_probe(dev, PTR_ERR(perst_gpio),
>> +				     "Failed to request reset GPIO\n");
>> +
>>   	bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));
>>   	if (!bridge)
>>   		return -ENODEV;
>> @@ -595,6 +602,13 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
>>   		return err;
>>   	}
>>   
>> +	if (perst_gpio) {
>> +		msleep(PCIE_T_PVPERL_MS); /* Minimum assertion time */
>> +		gpiod_set_value_cansleep(perst_gpio, 0);
> Are we assured that PERST# was already asserted when we entered
> xilinx_pcie_probe()?

Yes, because of the GPIOD_OUT_HIGH a few lines up, the reset GPIO is asserted 
when we arrive here.


>> +		/* Initial delay to provide endpoint time to initialize */
>> +		msleep(PCIE_T_RRS_READY_MS);
> I don't think this is the right spot for PCIE_T_RRS_READY_MS, details
> in https://lore.kernel.org/r/20250610185734.GA819344@bhelgaas
>
> I guess the spec assumes that for ports that don't support speeds
> greater than 5.0 GT/s, 100ms is enough for the link to come up *and*
> the endpoint to initialize.  But since you're going to wait for the
> link to come up immediately *after* this PCIE_T_RRS_READY_MS sleep, I
> would think you could extend the timeout in xilinx_pci_wait_link_up()
> and then do the PCIE_T_RRS_READY_MS sleep.

That would change the behavior of the original driver though, which never did 
any sleep during init. But it appears to match the spec better. Note that the 
hardware is limited to 5GT/s.

M.

Re: [PATCH v4 1/2] PCI: xilinx: Wait for link-up status during initialization

[Bjorn Helgaas]

On Tue, Jun 10, 2025 at 04:39:03PM +0200, Mike Looijmans wrote:
> When the driver loads, the transceiver and endpoint may still be setting
> up a link. Wait for that to complete before continuing. This fixes that
> the PCIe core does not work when loading the PL bitstream from
> userspace. Existing reference designs worked because the endpoint and
> PL were initialized by a bootloader. If the endpoint power and/or reset
> is supplied by the kernel, or if the PL is programmed from within the
> kernel, the link won't be up yet and the driver just has to wait for
> link training to finish.
> 
> As the PCIe spec allows up to 100 ms time to establish a link, we'll
> allow up to 200ms before giving up.

> +static int xilinx_pci_wait_link_up(struct xilinx_pcie *pcie)
> +{
> +	u32 val;
> +
> +	/*
> +	 * PCIe r6.0, sec 6.6.1 provides 100ms timeout. Since this is FPGA
> +	 * fabric, we're more lenient and allow 200 ms for link training.
> +	 */
> +	return readl_poll_timeout(pcie->reg_base + XILINX_PCIE_REG_PSCR, val,
> +			(val & XILINX_PCIE_REG_PSCR_LNKUP), 2 * USEC_PER_MSEC,
> +			2 * PCIE_T_RRS_READY_MS * USEC_PER_MSEC);
> +}

I don't think this is what PCIE_T_RRS_READY_MS is for.  Sec 6.6.1
requires 100ms *after* the link is up before sending config requests:

  For cases where system software cannot determine that DRS is
  supported by the attached device, or by the Downstream Port above
  the attached device:

    ◦ With a Downstream Port that does not support Link speeds greater
      than 5.0 GT/s, software must wait a minimum of 100 ms following
      exit from a Conventional Reset before sending a Configuration
      Request to the device immediately below that Port.

    ◦ With a Downstream Port that supports Link speeds greater than
      5.0 GT/s, software must wait a minimum of 100 ms after Link
      training completes before sending a Configuration Request to the
      device immediately below that Port. Software can determine when
      Link training completes by polling the Data Link Layer Link
      Active bit or by setting up an associated interrupt (see §
      Section 6.7.3.3).  It is strongly recommended for software to
      use this mechanism whenever the Downstream Port supports it.

Bjorn

Re: [PATCH v4 1/2] PCI: xilinx: Wait for link-up status during initialization

[Mike Looijmans]

Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl

Please consider the environment before printing this e-mail
On 10-06-2025 20:57, Bjorn Helgaas wrote:
> On Tue, Jun 10, 2025 at 04:39:03PM +0200, Mike Looijmans wrote:
>> When the driver loads, the transceiver and endpoint may still be setting
>> up a link. Wait for that to complete before continuing. This fixes that
>> the PCIe core does not work when loading the PL bitstream from
>> userspace. Existing reference designs worked because the endpoint and
>> PL were initialized by a bootloader. If the endpoint power and/or reset
>> is supplied by the kernel, or if the PL is programmed from within the
>> kernel, the link won't be up yet and the driver just has to wait for
>> link training to finish.
>>
>> As the PCIe spec allows up to 100 ms time to establish a link, we'll
>> allow up to 200ms before giving up.
>> +static int xilinx_pci_wait_link_up(struct xilinx_pcie *pcie)
>> +{
>> +	u32 val;
>> +
>> +	/*
>> +	 * PCIe r6.0, sec 6.6.1 provides 100ms timeout. Since this is FPGA
>> +	 * fabric, we're more lenient and allow 200 ms for link training.
>> +	 */
>> +	return readl_poll_timeout(pcie->reg_base + XILINX_PCIE_REG_PSCR, val,
>> +			(val & XILINX_PCIE_REG_PSCR_LNKUP), 2 * USEC_PER_MSEC,
>> +			2 * PCIE_T_RRS_READY_MS * USEC_PER_MSEC);
>> +}
> I don't think this is what PCIE_T_RRS_READY_MS is for.  Sec 6.6.1
> requires 100ms *after* the link is up before sending config requests:
>
>    For cases where system software cannot determine that DRS is
>    supported by the attached device, or by the Downstream Port above
>    the attached device:
>
>      ◦ With a Downstream Port that does not support Link speeds greater
>        than 5.0 GT/s, software must wait a minimum of 100 ms following
>        exit from a Conventional Reset before sending a Configuration
>        Request to the device immediately below that Port.
>
>      ◦ With a Downstream Port that supports Link speeds greater than
>        5.0 GT/s, software must wait a minimum of 100 ms after Link
>        training completes before sending a Configuration Request to the
>        device immediately below that Port. Software can determine when
>        Link training completes by polling the Data Link Layer Link
>        Active bit or by setting up an associated interrupt (see §
>        Section 6.7.3.3).  It is strongly recommended for software to
>        use this mechanism whenever the Downstream Port supports it.
>
Yeah, true, I misread the comment in pci.h. I cannot find any #define to match 
the "how long to wait for link training". Each driver appears to use its own 
timeout. So I should just create my own?

Re: [PATCH v4 1/2] PCI: xilinx: Wait for link-up status during initialization

[Manivannan Sadhasivam]

On Wed, Jun 11, 2025 at 08:48:51AM +0200, Mike Looijmans wrote:
> 
> Met vriendelijke groet / kind regards,
> 
> Mike Looijmans
> System Expert
> 
> 
> TOPIC Embedded Products B.V.
> Materiaalweg 4, 5681 RJ Best
> The Netherlands
> 
> T: +31 (0) 499 33 69 69
> E: mike.looijmans@topic.nl
> W: www.topic.nl
> 
> Please consider the environment before printing this e-mail
> On 10-06-2025 20:57, Bjorn Helgaas wrote:
> > On Tue, Jun 10, 2025 at 04:39:03PM +0200, Mike Looijmans wrote:
> > > When the driver loads, the transceiver and endpoint may still be setting
> > > up a link. Wait for that to complete before continuing. This fixes that
> > > the PCIe core does not work when loading the PL bitstream from
> > > userspace. Existing reference designs worked because the endpoint and
> > > PL were initialized by a bootloader. If the endpoint power and/or reset
> > > is supplied by the kernel, or if the PL is programmed from within the
> > > kernel, the link won't be up yet and the driver just has to wait for
> > > link training to finish.
> > > 
> > > As the PCIe spec allows up to 100 ms time to establish a link, we'll
> > > allow up to 200ms before giving up.
> > > +static int xilinx_pci_wait_link_up(struct xilinx_pcie *pcie)
> > > +{
> > > +	u32 val;
> > > +
> > > +	/*
> > > +	 * PCIe r6.0, sec 6.6.1 provides 100ms timeout. Since this is FPGA
> > > +	 * fabric, we're more lenient and allow 200 ms for link training.
> > > +	 */
> > > +	return readl_poll_timeout(pcie->reg_base + XILINX_PCIE_REG_PSCR, val,
> > > +			(val & XILINX_PCIE_REG_PSCR_LNKUP), 2 * USEC_PER_MSEC,
> > > +			2 * PCIE_T_RRS_READY_MS * USEC_PER_MSEC);
> > > +}
> > I don't think this is what PCIE_T_RRS_READY_MS is for.  Sec 6.6.1
> > requires 100ms *after* the link is up before sending config requests:
> > 
> >    For cases where system software cannot determine that DRS is
> >    supported by the attached device, or by the Downstream Port above
> >    the attached device:
> > 
> >      ◦ With a Downstream Port that does not support Link speeds greater
> >        than 5.0 GT/s, software must wait a minimum of 100 ms following
> >        exit from a Conventional Reset before sending a Configuration
> >        Request to the device immediately below that Port.
> > 
> >      ◦ With a Downstream Port that supports Link speeds greater than
> >        5.0 GT/s, software must wait a minimum of 100 ms after Link
> >        training completes before sending a Configuration Request to the
> >        device immediately below that Port. Software can determine when
> >        Link training completes by polling the Data Link Layer Link
> >        Active bit or by setting up an associated interrupt (see §
> >        Section 6.7.3.3).  It is strongly recommended for software to
> >        use this mechanism whenever the Downstream Port supports it.
> > 
> Yeah, true, I misread the comment in pci.h. I cannot find any #define to
> match the "how long to wait for link training". Each driver appears to use
> its own timeout. So I should just create my own?
> 

We recently merged a series that moved the timing definitions to
drivers/pci/pci.h:
https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?h=controller/linkup-fix&id=470f10f18b482b3d46429c9e6723ff0f7854d049

So if you base your series on top this branch, you could reuse the definitions.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v4 1/2] PCI: xilinx: Wait for link-up status during initialization

[Manivannan Sadhasivam]

On Wed, Jun 25, 2025 at 03:46:56PM -0600, Manivannan Sadhasivam wrote:
> On Wed, Jun 11, 2025 at 08:48:51AM +0200, Mike Looijmans wrote:
> > 
> > Met vriendelijke groet / kind regards,
> > 
> > Mike Looijmans
> > System Expert
> > 
> > 
> > TOPIC Embedded Products B.V.
> > Materiaalweg 4, 5681 RJ Best
> > The Netherlands
> > 
> > T: +31 (0) 499 33 69 69
> > E: mike.looijmans@topic.nl
> > W: www.topic.nl
> > 
> > Please consider the environment before printing this e-mail
> > On 10-06-2025 20:57, Bjorn Helgaas wrote:
> > > On Tue, Jun 10, 2025 at 04:39:03PM +0200, Mike Looijmans wrote:
> > > > When the driver loads, the transceiver and endpoint may still be setting
> > > > up a link. Wait for that to complete before continuing. This fixes that
> > > > the PCIe core does not work when loading the PL bitstream from
> > > > userspace. Existing reference designs worked because the endpoint and
> > > > PL were initialized by a bootloader. If the endpoint power and/or reset
> > > > is supplied by the kernel, or if the PL is programmed from within the
> > > > kernel, the link won't be up yet and the driver just has to wait for
> > > > link training to finish.
> > > > 
> > > > As the PCIe spec allows up to 100 ms time to establish a link, we'll
> > > > allow up to 200ms before giving up.
> > > > +static int xilinx_pci_wait_link_up(struct xilinx_pcie *pcie)
> > > > +{
> > > > +	u32 val;
> > > > +
> > > > +	/*
> > > > +	 * PCIe r6.0, sec 6.6.1 provides 100ms timeout. Since this is FPGA
> > > > +	 * fabric, we're more lenient and allow 200 ms for link training.
> > > > +	 */
> > > > +	return readl_poll_timeout(pcie->reg_base + XILINX_PCIE_REG_PSCR, val,
> > > > +			(val & XILINX_PCIE_REG_PSCR_LNKUP), 2 * USEC_PER_MSEC,
> > > > +			2 * PCIE_T_RRS_READY_MS * USEC_PER_MSEC);
> > > > +}
> > > I don't think this is what PCIE_T_RRS_READY_MS is for.  Sec 6.6.1
> > > requires 100ms *after* the link is up before sending config requests:
> > > 
> > >    For cases where system software cannot determine that DRS is
> > >    supported by the attached device, or by the Downstream Port above
> > >    the attached device:
> > > 
> > >      ◦ With a Downstream Port that does not support Link speeds greater
> > >        than 5.0 GT/s, software must wait a minimum of 100 ms following
> > >        exit from a Conventional Reset before sending a Configuration
> > >        Request to the device immediately below that Port.
> > > 
> > >      ◦ With a Downstream Port that supports Link speeds greater than
> > >        5.0 GT/s, software must wait a minimum of 100 ms after Link
> > >        training completes before sending a Configuration Request to the
> > >        device immediately below that Port. Software can determine when
> > >        Link training completes by polling the Data Link Layer Link
> > >        Active bit or by setting up an associated interrupt (see §
> > >        Section 6.7.3.3).  It is strongly recommended for software to
> > >        use this mechanism whenever the Downstream Port supports it.
> > > 
> > Yeah, true, I misread the comment in pci.h. I cannot find any #define to
> > match the "how long to wait for link training". Each driver appears to use
> > its own timeout. So I should just create my own?
> > 
> 
> We recently merged a series that moved the timing definitions to
> drivers/pci/pci.h:
> https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?h=controller/linkup-fix&id=470f10f18b482b3d46429c9e6723ff0f7854d049
> 
> So if you base your series on top this branch, you could reuse the definitions.
> 

You could also introduce a new macro if all you need is 1s:

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9d20f0222fb1..f03bb882bf2e 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -64,6 +64,7 @@ struct pcie_tlp_log;
  */
 #define PCIE_LINK_WAIT_MAX_RETRIES     100
 #define PCIE_LINK_WAIT_SLEEP_MS                10
+#define PCIE_LINK_WAIT_MS              PCIE_LINK_WAIT_MAX_RETRIES * PCIE_LINK_WAIT_SLEEP_MS
 
 /* Message Routing (r[2:0]); PCIe r6.0, sec 2.2.8 */
 #define PCIE_MSG_TYPE_R_RC     0

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v4 1/2] PCI: xilinx: Wait for link-up status during initialization

[Bjorn Helgaas]

On Tue, Jun 10, 2025 at 04:39:03PM +0200, Mike Looijmans wrote:
> When the driver loads, the transceiver and endpoint may still be setting
> up a link. Wait for that to complete before continuing. This fixes that
> the PCIe core does not work when loading the PL bitstream from
> userspace. Existing reference designs worked because the endpoint and
> PL were initialized by a bootloader. If the endpoint power and/or reset
> is supplied by the kernel, or if the PL is programmed from within the
> kernel, the link won't be up yet and the driver just has to wait for
> link training to finish.

> +static int xilinx_pci_wait_link_up(struct xilinx_pcie *pcie)
> +{
> +	u32 val;
> +
> +	/*
> +	 * PCIe r6.0, sec 6.6.1 provides 100ms timeout. Since this is FPGA
> +	 * fabric, we're more lenient and allow 200 ms for link training.

Does this FPGA fabric refer to the Root Port or to the Endpoint?  We
should know whether this issue is common to all xilinx Root Ports or
specific to certain Endpoints.

I assume that even if we wait for the link to come up and then wait
PCIE_T_RRS_READY_MS before sending config requests, this Endpoint is
still not ready to return an RRS response?  I'm looking at this text
from sec 6.6.1:

  Unless Readiness Notifications mechanisms are used, the Root Complex
  and/or system software must allow at least 1.0 s following exit from
  a Conventional Reset of a device, before determining that the device
  is broken if it fails to return a Successful Completion status for a
  valid Configuration Request. This period is independent of how
  quickly Link training completes.

  Note: This delay is analogous to the Trhfa parameter specified for
  PCI/PCI-X, and is intended to allow an adequate amount of time for
  devices which require self initialization.

It seems like the PCI core RRS handling should already account for
this 1.0 s period.

> +	 */
> +	return readl_poll_timeout(pcie->reg_base + XILINX_PCIE_REG_PSCR, val,
> +			(val & XILINX_PCIE_REG_PSCR_LNKUP), 2 * USEC_PER_MSEC,
> +			2 * PCIE_T_RRS_READY_MS * USEC_PER_MSEC);
> +}

Re: [PATCH v4 1/2] PCI: xilinx: Wait for link-up status during initialization

[Mike Looijmans]

Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@topic.nl
W: www.topic.nl

Please consider the environment before printing this e-mail
On 10-06-2025 21:12, Bjorn Helgaas wrote:
> On Tue, Jun 10, 2025 at 04:39:03PM +0200, Mike Looijmans wrote:
>> When the driver loads, the transceiver and endpoint may still be setting
>> up a link. Wait for that to complete before continuing. This fixes that
>> the PCIe core does not work when loading the PL bitstream from
>> userspace. Existing reference designs worked because the endpoint and
>> PL were initialized by a bootloader. If the endpoint power and/or reset
>> is supplied by the kernel, or if the PL is programmed from within the
>> kernel, the link won't be up yet and the driver just has to wait for
>> link training to finish.
>> +static int xilinx_pci_wait_link_up(struct xilinx_pcie *pcie)
>> +{
>> +	u32 val;
>> +
>> +	/*
>> +	 * PCIe r6.0, sec 6.6.1 provides 100ms timeout. Since this is FPGA
>> +	 * fabric, we're more lenient and allow 200 ms for link training.
> Does this FPGA fabric refer to the Root Port or to the Endpoint?  We
> should know whether this issue is common to all xilinx Root Ports or
> specific to certain Endpoints.

The FPGA is root point. The endpoint is usually some generic PCIe device like 
an NVME or Wifi card.


> I assume that even if we wait for the link to come up and then wait
> PCIE_T_RRS_READY_MS before sending config requests, this Endpoint is
> still not ready to return an RRS response?  I'm looking at this text
> from sec 6.6.1:

My initial finding was that usually the endpoint would be ready well within 100ms.

The issue at hand here is that Xilinx assumed that their proprietary 
bootloader would have taken care of power, reset and clock signals and 
programming the FPGA. Thus, when this driver probes, seconds later, it would 
already be in a "link up" state.

In our system, reset, clock and power are under kernel control, so the 
endpoint (e.g. NVME) has just been powered-up, and the root complex (in the 
FPGA) also got powered up just a millisecond ago. So it would always report a 
"link down" at startup and give up.

Analysis showed that the PCIe root was just still training the link, and all 
that's required to make the system work is to wait for the link to be established.


>    Unless Readiness Notifications mechanisms are used, the Root Complex
>    and/or system software must allow at least 1.0 s following exit from
>    a Conventional Reset of a device, before determining that the device
>    is broken if it fails to return a Successful Completion status for a
>    valid Configuration Request. This period is independent of how
>    quickly Link training completes.
>
>    Note: This delay is analogous to the Trhfa parameter specified for
>    PCI/PCI-X, and is intended to allow an adequate amount of time for
>    devices which require self initialization.
>
> It seems like the PCI core RRS handling should already account for
> this 1.0 s period.
>
>> +	 */
>> +	return readl_poll_timeout(pcie->reg_base + XILINX_PCIE_REG_PSCR, val,
>> +			(val & XILINX_PCIE_REG_PSCR_LNKUP), 2 * USEC_PER_MSEC,
>> +			2 * PCIE_T_RRS_READY_MS * USEC_PER_MSEC);
>> +}