* [V3] PCI: fu740: Drop to 2.5GT/s to fix initial device probing on some boards
@ 2022-03-18 15:24 Ben Dooks
2022-03-18 23:03 ` Palmer Dabbelt
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Ben Dooks @ 2022-03-18 15:24 UTC (permalink / raw)
To: linux-pci
Cc: linux-kernel, linux-riscv, Bjorn Helgaas, Palmer Dabbelt,
Rob Herring, Lorenzo Pieralisi, Greentime Hu, Paul Walmsley,
Ben Dooks
The fu740 PCIe core does not probe any devices on the SiFive Unmatched
board without this fix (or having U-Boot explicitly start the PCIe via
either boot-script or user command). The fix is to start the link at
2.5GT/s speeds and once the link is up then change the maximum speed back
to the default.
The U-Boot driver claims to set the link-speed to 2.5GT/s to get the probe
to work (and U-Boot does print link up at 2.5GT/s) in the following code:
https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/pci/pcie_dw_sifive.c?id=v2022.01#L271
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
--
Note, this patch has had significant re-work since the previous 4
sets, including trying to fix style, message, reliance on the U-Boot
fix and the comments about usage of LINK_CAP and reserved fields.
v2:
- fix issues with Gen1/2.5GTs
- updated comment on the initial probe
- run tests with both uninitialised and initialsed pcie from uboot
---
drivers/pci/controller/dwc/pcie-fu740.c | 52 ++++++++++++++++++++++++-
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-fu740.c b/drivers/pci/controller/dwc/pcie-fu740.c
index 842b7202b96e..ecac0364178a 100644
--- a/drivers/pci/controller/dwc/pcie-fu740.c
+++ b/drivers/pci/controller/dwc/pcie-fu740.c
@@ -181,10 +181,60 @@ static int fu740_pcie_start_link(struct dw_pcie *pci)
{
struct device *dev = pci->dev;
struct fu740_pcie *afp = dev_get_drvdata(dev);
+ u8 cap_exp = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+ int ret;
+ u32 orig, tmp;
+
+ /*
+ * Force 2.5GT/s when starting the link, due to some devices not
+ * probing at higher speeds. This happens with the PCIe switch
+ * on the Unmatched board when U-Boot has not initialised the PCIe.
+ * The fix in U-Boot is to force 2.5GT/s, which then gets cleared
+ * by the soft reset does by this driver.
+ */
+
+ dev_dbg(dev, "cap_exp at %x\n", cap_exp);
+ dw_pcie_dbi_ro_wr_en(pci);
+
+ tmp = dw_pcie_readl_dbi(pci, cap_exp + PCI_EXP_LNKCAP);
+ orig = tmp & PCI_EXP_LNKCAP_SLS;
+ tmp &= ~PCI_EXP_LNKCAP_SLS;
+ tmp |= PCI_EXP_LNKCAP_SLS_2_5GB;
+ dw_pcie_writel_dbi(pci, cap_exp + PCI_EXP_LNKCAP, tmp);
/* Enable LTSSM */
writel_relaxed(0x1, afp->mgmt_base + PCIEX8MGMT_APP_LTSSM_ENABLE);
- return 0;
+
+ ret = dw_pcie_wait_for_link(pci);
+ if (ret) {
+ dev_err(dev, "error: link did not start\n");
+ goto err;
+ }
+
+ tmp = dw_pcie_readl_dbi(pci, cap_exp + PCI_EXP_LNKCAP);
+ if ((tmp & PCI_EXP_LNKCAP_SLS) != orig) {
+ dev_dbg(dev, "changing speed back to original\n");
+
+ tmp &= ~PCI_EXP_LNKCAP_SLS;
+ tmp |= orig;
+ dw_pcie_writel_dbi(pci, cap_exp + PCI_EXP_LNKCAP, tmp);
+
+ tmp = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
+ tmp |= PORT_LOGIC_SPEED_CHANGE;
+ dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, tmp);
+
+ ret = dw_pcie_wait_for_link(pci);
+ if (ret) {
+ dev_err(dev, "error: link did not start at new speed\n");
+ goto err;
+ }
+ }
+
+ ret = 0;
+err:
+ WARN_ON(ret); /* we assume that errors will be very rare */
+ dw_pcie_dbi_ro_wr_dis(pci);
+ return ret;
}
static int fu740_pcie_host_init(struct pcie_port *pp)
--
2.35.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [V3] PCI: fu740: Drop to 2.5GT/s to fix initial device probing on some boards
2022-03-18 15:24 [V3] PCI: fu740: Drop to 2.5GT/s to fix initial device probing on some boards Ben Dooks
@ 2022-03-18 23:03 ` Palmer Dabbelt
2022-03-19 12:40 ` Ben Dooks
2022-03-21 13:49 ` Alexandre Ghiti
2022-03-21 20:29 ` Bjorn Helgaas
2 siblings, 1 reply; 11+ messages in thread
From: Palmer Dabbelt @ 2022-03-18 23:03 UTC (permalink / raw)
To: ben.dooks
Cc: linux-pci, linux-kernel, linux-riscv, bhelgaas, robh,
lorenzo.pieralisi, greentime.hu, Paul Walmsley, ben.dooks
On Fri, 18 Mar 2022 08:24:30 PDT (-0700), ben.dooks@codethink.co.uk wrote:
> The fu740 PCIe core does not probe any devices on the SiFive Unmatched
> board without this fix (or having U-Boot explicitly start the PCIe via
> either boot-script or user command). The fix is to start the link at
> 2.5GT/s speeds and once the link is up then change the maximum speed back
> to the default.
>
> The U-Boot driver claims to set the link-speed to 2.5GT/s to get the probe
> to work (and U-Boot does print link up at 2.5GT/s) in the following code:
> https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/pci/pcie_dw_sifive.c?id=v2022.01#L271
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> --
A "--" triggers some mail handles to think the rest of this is a
signature, git folks usually use a "---" to indicate a comment that
shouldn't be part of what's eventually merged (like this changelog
stuff).
> Note, this patch has had significant re-work since the previous 4
> sets, including trying to fix style, message, reliance on the U-Boot
> fix and the comments about usage of LINK_CAP and reserved fields.
>
> v2:
> - fix issues with Gen1/2.5GTs
> - updated comment on the initial probe
> - run tests with both uninitialised and initialsed pcie from uboot
> ---
> drivers/pci/controller/dwc/pcie-fu740.c | 52 ++++++++++++++++++++++++-
> 1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-fu740.c b/drivers/pci/controller/dwc/pcie-fu740.c
> index 842b7202b96e..ecac0364178a 100644
> --- a/drivers/pci/controller/dwc/pcie-fu740.c
> +++ b/drivers/pci/controller/dwc/pcie-fu740.c
> @@ -181,10 +181,60 @@ static int fu740_pcie_start_link(struct dw_pcie *pci)
Is there an errata? IIUC this will trigger the workaround on all
FU740s, but from the description it seems like more of a board bug than
a chip bug. The distinction doesn't really matter, as there's only one
board for this chip (and I'm assuming that'll always be the case), but
if there's an errata (or any way this is documented) it might make
things a bit easier to sort out if we end up with another similar
chip/board.
Either way
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
I'm assuming you, or someone else, has tested this on the board? I'm
pretty sure I've got one lying around somewhere, but I don't regularly
use it. I can dust it off if nobody else has tried this, but happy to
avoid the need to do so.
Thanks!
> {
> struct device *dev = pci->dev;
> struct fu740_pcie *afp = dev_get_drvdata(dev);
> + u8 cap_exp = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> + int ret;
> + u32 orig, tmp;
> +
> + /*
> + * Force 2.5GT/s when starting the link, due to some devices not
> + * probing at higher speeds. This happens with the PCIe switch
> + * on the Unmatched board when U-Boot has not initialised the PCIe.
> + * The fix in U-Boot is to force 2.5GT/s, which then gets cleared
> + * by the soft reset does by this driver.
> + */
> +
> + dev_dbg(dev, "cap_exp at %x\n", cap_exp);
> + dw_pcie_dbi_ro_wr_en(pci);
> +
> + tmp = dw_pcie_readl_dbi(pci, cap_exp + PCI_EXP_LNKCAP);
> + orig = tmp & PCI_EXP_LNKCAP_SLS;
> + tmp &= ~PCI_EXP_LNKCAP_SLS;
> + tmp |= PCI_EXP_LNKCAP_SLS_2_5GB;
> + dw_pcie_writel_dbi(pci, cap_exp + PCI_EXP_LNKCAP, tmp);
>
> /* Enable LTSSM */
> writel_relaxed(0x1, afp->mgmt_base + PCIEX8MGMT_APP_LTSSM_ENABLE);
> - return 0;
> +
> + ret = dw_pcie_wait_for_link(pci);
> + if (ret) {
> + dev_err(dev, "error: link did not start\n");
> + goto err;
> + }
> +
> + tmp = dw_pcie_readl_dbi(pci, cap_exp + PCI_EXP_LNKCAP);
> + if ((tmp & PCI_EXP_LNKCAP_SLS) != orig) {
> + dev_dbg(dev, "changing speed back to original\n");
> +
> + tmp &= ~PCI_EXP_LNKCAP_SLS;
> + tmp |= orig;
> + dw_pcie_writel_dbi(pci, cap_exp + PCI_EXP_LNKCAP, tmp);
> +
> + tmp = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
> + tmp |= PORT_LOGIC_SPEED_CHANGE;
> + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, tmp);
> +
> + ret = dw_pcie_wait_for_link(pci);
> + if (ret) {
> + dev_err(dev, "error: link did not start at new speed\n");
> + goto err;
> + }
> + }
> +
> + ret = 0;
> +err:
> + WARN_ON(ret); /* we assume that errors will be very rare */
Maybe a message is better than a WARN_ON there? Something users might
be able to understand.
> + dw_pcie_dbi_ro_wr_dis(pci);
> + return ret;
> }
>
> static int fu740_pcie_host_init(struct pcie_port *pp)
> --
> 2.35.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [V3] PCI: fu740: Drop to 2.5GT/s to fix initial device probing on some boards
2022-03-18 23:03 ` Palmer Dabbelt
@ 2022-03-19 12:40 ` Ben Dooks
0 siblings, 0 replies; 11+ messages in thread
From: Ben Dooks @ 2022-03-19 12:40 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: linux-pci, linux-kernel, linux-riscv, bhelgaas, robh,
lorenzo.pieralisi, greentime.hu, Paul Walmsley
On 18/03/2022 23:03, Palmer Dabbelt wrote:
> On Fri, 18 Mar 2022 08:24:30 PDT (-0700), ben.dooks@codethink.co.uk wrote:
>> The fu740 PCIe core does not probe any devices on the SiFive Unmatched
>> board without this fix (or having U-Boot explicitly start the PCIe via
>> either boot-script or user command). The fix is to start the link at
>> 2.5GT/s speeds and once the link is up then change the maximum speed back
>> to the default.
>>
>> The U-Boot driver claims to set the link-speed to 2.5GT/s to get the
>> probe
>> to work (and U-Boot does print link up at 2.5GT/s) in the following code:
>> https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/pci/pcie_dw_sifive.c?id=v2022.01#L271
>>
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> --
>
> A "--" triggers some mail handles to think the rest of this is a
> signature, git folks usually use a "---" to indicate a comment that
> shouldn't be part of what's eventually merged (like this changelog stuff).
>
>> Note, this patch has had significant re-work since the previous 4
>> sets, including trying to fix style, message, reliance on the U-Boot
>> fix and the comments about usage of LINK_CAP and reserved fields.
>>
>> v2:
>> - fix issues with Gen1/2.5GTs
>> - updated comment on the initial probe
>> - run tests with both uninitialised and initialsed pcie from uboot
>> ---
>> drivers/pci/controller/dwc/pcie-fu740.c | 52 ++++++++++++++++++++++++-
>> 1 file changed, 51 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-fu740.c
>> b/drivers/pci/controller/dwc/pcie-fu740.c
>> index 842b7202b96e..ecac0364178a 100644
>> --- a/drivers/pci/controller/dwc/pcie-fu740.c
>> +++ b/drivers/pci/controller/dwc/pcie-fu740.c
>> @@ -181,10 +181,60 @@ static int fu740_pcie_start_link(struct dw_pcie
>> *pci)
>
> Is there an errata? IIUC this will trigger the workaround on all
> FU740s, but from the description it seems like more of a board bug than
> a chip bug. The distinction doesn't really matter, as there's only one
> board for this chip (and I'm assuming that'll always be the case), but
> if there's an errata (or any way this is documented) it might make
> things a bit easier to sort out if we end up with another similar
> chip/board.
>
> Either way
>
> Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
>
> I'm assuming you, or someone else, has tested this on the board? I'm
> pretty sure I've got one lying around somewhere, but I don't regularly
> use it. I can dust it off if nobody else has tried this, but happy to
> avoid the need to do so.
I've been trying this on my own Unmatched board, where I often use
network boot. We have also tested with a new-ish U-Boot (2022.01 I
think) with both "pci enum" and not. It has been on our test board
for over a week without an issue.
I will have a look for v4 about changing the WARN_ON() to something
more useful.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [V3] PCI: fu740: Drop to 2.5GT/s to fix initial device probing on some boards
2022-03-18 15:24 [V3] PCI: fu740: Drop to 2.5GT/s to fix initial device probing on some boards Ben Dooks
2022-03-18 23:03 ` Palmer Dabbelt
@ 2022-03-21 13:49 ` Alexandre Ghiti
2022-03-23 10:36 ` Alexandre Ghiti
2022-03-21 20:29 ` Bjorn Helgaas
2 siblings, 1 reply; 11+ messages in thread
From: Alexandre Ghiti @ 2022-03-21 13:49 UTC (permalink / raw)
To: Ben Dooks, linux-pci
Cc: linux-kernel, linux-riscv, Bjorn Helgaas, Palmer Dabbelt,
Rob Herring, Lorenzo Pieralisi, Greentime Hu, Paul Walmsley,
Maciej W. Rozycki, David Abdurachmanov
Hi Ben,
On 3/18/22 16:24, Ben Dooks wrote:
> The fu740 PCIe core does not probe any devices on the SiFive Unmatched
> board without this fix (or having U-Boot explicitly start the PCIe via
> either boot-script or user command). The fix is to start the link at
> 2.5GT/s speeds and once the link is up then change the maximum speed back
> to the default.
>
> The U-Boot driver claims to set the link-speed to 2.5GT/s to get the probe
> to work (and U-Boot does print link up at 2.5GT/s) in the following code:
> https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/pci/pcie_dw_sifive.c?id=v2022.01#L271
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> --
> Note, this patch has had significant re-work since the previous 4
> sets, including trying to fix style, message, reliance on the U-Boot
> fix and the comments about usage of LINK_CAP and reserved fields.
>
> v2:
> - fix issues with Gen1/2.5GTs
> - updated comment on the initial probe
> - run tests with both uninitialised and initialsed pcie from uboot
> ---
> drivers/pci/controller/dwc/pcie-fu740.c | 52 ++++++++++++++++++++++++-
> 1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-fu740.c b/drivers/pci/controller/dwc/pcie-fu740.c
> index 842b7202b96e..ecac0364178a 100644
> --- a/drivers/pci/controller/dwc/pcie-fu740.c
> +++ b/drivers/pci/controller/dwc/pcie-fu740.c
> @@ -181,10 +181,60 @@ static int fu740_pcie_start_link(struct dw_pcie *pci)
> {
> struct device *dev = pci->dev;
> struct fu740_pcie *afp = dev_get_drvdata(dev);
> + u8 cap_exp = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> + int ret;
> + u32 orig, tmp;
> +
> + /*
> + * Force 2.5GT/s when starting the link, due to some devices not
> + * probing at higher speeds. This happens with the PCIe switch
> + * on the Unmatched board when U-Boot has not initialised the PCIe.
> + * The fix in U-Boot is to force 2.5GT/s, which then gets cleared
> + * by the soft reset does by this driver.
> + */
> +
> + dev_dbg(dev, "cap_exp at %x\n", cap_exp);
> + dw_pcie_dbi_ro_wr_en(pci);
> +
> + tmp = dw_pcie_readl_dbi(pci, cap_exp + PCI_EXP_LNKCAP);
> + orig = tmp & PCI_EXP_LNKCAP_SLS;
> + tmp &= ~PCI_EXP_LNKCAP_SLS;
> + tmp |= PCI_EXP_LNKCAP_SLS_2_5GB;
> + dw_pcie_writel_dbi(pci, cap_exp + PCI_EXP_LNKCAP, tmp);
>
> /* Enable LTSSM */
> writel_relaxed(0x1, afp->mgmt_base + PCIEX8MGMT_APP_LTSSM_ENABLE);
> - return 0;
> +
> + ret = dw_pcie_wait_for_link(pci);
> + if (ret) {
> + dev_err(dev, "error: link did not start\n");
> + goto err;
> + }
> +
> + tmp = dw_pcie_readl_dbi(pci, cap_exp + PCI_EXP_LNKCAP);
> + if ((tmp & PCI_EXP_LNKCAP_SLS) != orig) {
> + dev_dbg(dev, "changing speed back to original\n");
> +
> + tmp &= ~PCI_EXP_LNKCAP_SLS;
> + tmp |= orig;
> + dw_pcie_writel_dbi(pci, cap_exp + PCI_EXP_LNKCAP, tmp);
> +
> + tmp = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
> + tmp |= PORT_LOGIC_SPEED_CHANGE;
> + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, tmp);
> +
> + ret = dw_pcie_wait_for_link(pci);
> + if (ret) {
> + dev_err(dev, "error: link did not start at new speed\n");
> + goto err;
> + }
> + }
> +
> + ret = 0;
> +err:
> + WARN_ON(ret); /* we assume that errors will be very rare */
> + dw_pcie_dbi_ro_wr_dis(pci);
> + return ret;
> }
>
> static int fu740_pcie_host_init(struct pcie_port *pp)
+cc Maciej and David as there is this other fix that seems to do the
same but differently, it's been under review for some time now:
https://lore.kernel.org/all/20220302000043.GA662523@bhelgaas/t/
I fell onto this issue recently, I'll give your patch and the above a
try soon.
Thanks
Alex
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [V3] PCI: fu740: Drop to 2.5GT/s to fix initial device probing on some boards
2022-03-18 15:24 [V3] PCI: fu740: Drop to 2.5GT/s to fix initial device probing on some boards Ben Dooks
2022-03-18 23:03 ` Palmer Dabbelt
2022-03-21 13:49 ` Alexandre Ghiti
@ 2022-03-21 20:29 ` Bjorn Helgaas
2 siblings, 0 replies; 11+ messages in thread
From: Bjorn Helgaas @ 2022-03-21 20:29 UTC (permalink / raw)
To: Ben Dooks
Cc: linux-pci, linux-kernel, linux-riscv, Bjorn Helgaas,
Palmer Dabbelt, Rob Herring, Lorenzo Pieralisi, Greentime Hu,
Paul Walmsley
On Fri, Mar 18, 2022 at 03:24:30PM +0000, Ben Dooks wrote:
> The fu740 PCIe core does not probe any devices on the SiFive Unmatched
> board without this fix (or having U-Boot explicitly start the PCIe via
> either boot-script or user command). The fix is to start the link at
> 2.5GT/s speeds and once the link is up then change the maximum speed back
> to the default.
>
> The U-Boot driver claims to set the link-speed to 2.5GT/s to get the probe
> to work (and U-Boot does print link up at 2.5GT/s) in the following code:
> https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/pci/pcie_dw_sifive.c?id=v2022.01#L271
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Lorenzo has an old version of this patch on his pci/fu740 branch.
Since we're now in the merge window, I applied this V3 patch to my
pci/host/fu740 branch so we can try to get this in for v5.18.
> --
> Note, this patch has had significant re-work since the previous 4
> sets, including trying to fix style, message, reliance on the U-Boot
> fix and the comments about usage of LINK_CAP and reserved fields.
>
> v2:
> - fix issues with Gen1/2.5GTs
> - updated comment on the initial probe
> - run tests with both uninitialised and initialsed pcie from uboot
> ---
> drivers/pci/controller/dwc/pcie-fu740.c | 52 ++++++++++++++++++++++++-
> 1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-fu740.c b/drivers/pci/controller/dwc/pcie-fu740.c
> index 842b7202b96e..ecac0364178a 100644
> --- a/drivers/pci/controller/dwc/pcie-fu740.c
> +++ b/drivers/pci/controller/dwc/pcie-fu740.c
> @@ -181,10 +181,60 @@ static int fu740_pcie_start_link(struct dw_pcie *pci)
> {
> struct device *dev = pci->dev;
> struct fu740_pcie *afp = dev_get_drvdata(dev);
> + u8 cap_exp = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> + int ret;
> + u32 orig, tmp;
> +
> + /*
> + * Force 2.5GT/s when starting the link, due to some devices not
> + * probing at higher speeds. This happens with the PCIe switch
> + * on the Unmatched board when U-Boot has not initialised the PCIe.
> + * The fix in U-Boot is to force 2.5GT/s, which then gets cleared
> + * by the soft reset does by this driver.
> + */
> +
> + dev_dbg(dev, "cap_exp at %x\n", cap_exp);
> + dw_pcie_dbi_ro_wr_en(pci);
> +
> + tmp = dw_pcie_readl_dbi(pci, cap_exp + PCI_EXP_LNKCAP);
> + orig = tmp & PCI_EXP_LNKCAP_SLS;
> + tmp &= ~PCI_EXP_LNKCAP_SLS;
> + tmp |= PCI_EXP_LNKCAP_SLS_2_5GB;
> + dw_pcie_writel_dbi(pci, cap_exp + PCI_EXP_LNKCAP, tmp);
>
> /* Enable LTSSM */
> writel_relaxed(0x1, afp->mgmt_base + PCIEX8MGMT_APP_LTSSM_ENABLE);
> - return 0;
> +
> + ret = dw_pcie_wait_for_link(pci);
> + if (ret) {
> + dev_err(dev, "error: link did not start\n");
> + goto err;
> + }
> +
> + tmp = dw_pcie_readl_dbi(pci, cap_exp + PCI_EXP_LNKCAP);
> + if ((tmp & PCI_EXP_LNKCAP_SLS) != orig) {
> + dev_dbg(dev, "changing speed back to original\n");
> +
> + tmp &= ~PCI_EXP_LNKCAP_SLS;
> + tmp |= orig;
> + dw_pcie_writel_dbi(pci, cap_exp + PCI_EXP_LNKCAP, tmp);
> +
> + tmp = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
> + tmp |= PORT_LOGIC_SPEED_CHANGE;
> + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, tmp);
> +
> + ret = dw_pcie_wait_for_link(pci);
> + if (ret) {
> + dev_err(dev, "error: link did not start at new speed\n");
> + goto err;
> + }
> + }
> +
> + ret = 0;
> +err:
> + WARN_ON(ret); /* we assume that errors will be very rare */
> + dw_pcie_dbi_ro_wr_dis(pci);
> + return ret;
> }
>
> static int fu740_pcie_host_init(struct pcie_port *pp)
> --
> 2.35.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [V3] PCI: fu740: Drop to 2.5GT/s to fix initial device probing on some boards
2022-03-21 13:49 ` Alexandre Ghiti
@ 2022-03-23 10:36 ` Alexandre Ghiti
2022-03-23 10:40 ` Ben Dooks
0 siblings, 1 reply; 11+ messages in thread
From: Alexandre Ghiti @ 2022-03-23 10:36 UTC (permalink / raw)
To: Ben Dooks, linux-pci
Cc: linux-kernel, linux-riscv, Bjorn Helgaas, Palmer Dabbelt,
Rob Herring, Lorenzo Pieralisi, Greentime Hu, Paul Walmsley,
Maciej W. Rozycki, David Abdurachmanov
On 3/21/22 14:49, Alexandre Ghiti wrote:
> Hi Ben,
>
> On 3/18/22 16:24, Ben Dooks wrote:
>> The fu740 PCIe core does not probe any devices on the SiFive Unmatched
>> board without this fix (or having U-Boot explicitly start the PCIe via
>> either boot-script or user command). The fix is to start the link at
>> 2.5GT/s speeds and once the link is up then change the maximum speed
>> back
>> to the default.
>>
>> The U-Boot driver claims to set the link-speed to 2.5GT/s to get the
>> probe
>> to work (and U-Boot does print link up at 2.5GT/s) in the following
>> code:
>> https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/pci/pcie_dw_sifive.c?id=v2022.01#L271
>>
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> --
>> Note, this patch has had significant re-work since the previous 4
>> sets, including trying to fix style, message, reliance on the U-Boot
>> fix and the comments about usage of LINK_CAP and reserved fields.
>>
>> v2:
>> - fix issues with Gen1/2.5GTs
>> - updated comment on the initial probe
>> - run tests with both uninitialised and initialsed pcie from uboot
>> ---
>> drivers/pci/controller/dwc/pcie-fu740.c | 52 ++++++++++++++++++++++++-
>> 1 file changed, 51 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-fu740.c
>> b/drivers/pci/controller/dwc/pcie-fu740.c
>> index 842b7202b96e..ecac0364178a 100644
>> --- a/drivers/pci/controller/dwc/pcie-fu740.c
>> +++ b/drivers/pci/controller/dwc/pcie-fu740.c
>> @@ -181,10 +181,60 @@ static int fu740_pcie_start_link(struct dw_pcie
>> *pci)
>> {
>> struct device *dev = pci->dev;
>> struct fu740_pcie *afp = dev_get_drvdata(dev);
>> + u8 cap_exp = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
>> + int ret;
>> + u32 orig, tmp;
>> +
>> + /*
>> + * Force 2.5GT/s when starting the link, due to some devices not
>> + * probing at higher speeds. This happens with the PCIe switch
>> + * on the Unmatched board when U-Boot has not initialised the PCIe.
>> + * The fix in U-Boot is to force 2.5GT/s, which then gets cleared
>> + * by the soft reset does by this driver.
>> + */
>> +
>> + dev_dbg(dev, "cap_exp at %x\n", cap_exp);
>> + dw_pcie_dbi_ro_wr_en(pci);
>> +
>> + tmp = dw_pcie_readl_dbi(pci, cap_exp + PCI_EXP_LNKCAP);
>> + orig = tmp & PCI_EXP_LNKCAP_SLS;
>> + tmp &= ~PCI_EXP_LNKCAP_SLS;
>> + tmp |= PCI_EXP_LNKCAP_SLS_2_5GB;
>> + dw_pcie_writel_dbi(pci, cap_exp + PCI_EXP_LNKCAP, tmp);
>> /* Enable LTSSM */
>> writel_relaxed(0x1, afp->mgmt_base + PCIEX8MGMT_APP_LTSSM_ENABLE);
>> - return 0;
>> +
>> + ret = dw_pcie_wait_for_link(pci);
>> + if (ret) {
>> + dev_err(dev, "error: link did not start\n");
>> + goto err;
>> + }
>> +
>> + tmp = dw_pcie_readl_dbi(pci, cap_exp + PCI_EXP_LNKCAP);
>> + if ((tmp & PCI_EXP_LNKCAP_SLS) != orig) {
>> + dev_dbg(dev, "changing speed back to original\n");
>> +
>> + tmp &= ~PCI_EXP_LNKCAP_SLS;
>> + tmp |= orig;
>> + dw_pcie_writel_dbi(pci, cap_exp + PCI_EXP_LNKCAP, tmp);
>> +
>> + tmp = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
>> + tmp |= PORT_LOGIC_SPEED_CHANGE;
>> + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, tmp);
>> +
>> + ret = dw_pcie_wait_for_link(pci);
>> + if (ret) {
>> + dev_err(dev, "error: link did not start at new speed\n");
>> + goto err;
>> + }
>> + }
>> +
>> + ret = 0;
>> +err:
>> + WARN_ON(ret); /* we assume that errors will be very rare */
>> + dw_pcie_dbi_ro_wr_dis(pci);
>> + return ret;
>> }
>> static int fu740_pcie_host_init(struct pcie_port *pp)
>
> +cc Maciej and David as there is this other fix that seems to do the
> same but differently, it's been under review for some time now:
> https://lore.kernel.org/all/20220302000043.GA662523@bhelgaas/t/
>
> I fell onto this issue recently, I'll give your patch and the above a
> try soon.
FWIW, I have tested this and it solved my issue with nvme not being
probed, so:
Tested-by: Alexandre Ghiti <alexandre.ghiti@canonical.com>
Thanks,
Alex
>
> Thanks
>
> Alex
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [V3] PCI: fu740: Drop to 2.5GT/s to fix initial device probing on some boards
2022-03-23 10:36 ` Alexandre Ghiti
@ 2022-03-23 10:40 ` Ben Dooks
2022-04-14 0:10 ` Maciej W. Rozycki
0 siblings, 1 reply; 11+ messages in thread
From: Ben Dooks @ 2022-03-23 10:40 UTC (permalink / raw)
To: Alexandre Ghiti, linux-pci
Cc: linux-kernel, linux-riscv, Bjorn Helgaas, Palmer Dabbelt,
Rob Herring, Lorenzo Pieralisi, Greentime Hu, Paul Walmsley,
Maciej W. Rozycki, David Abdurachmanov, Neill Whillans
On 23/03/2022 10:36, Alexandre Ghiti wrote:
> On 3/21/22 14:49, Alexandre Ghiti wrote:
>> Hi Ben,
>>
>> On 3/18/22 16:24, Ben Dooks wrote:
>>> The fu740 PCIe core does not probe any devices on the SiFive Unmatched
>>> board without this fix (or having U-Boot explicitly start the PCIe via
>>> either boot-script or user command). The fix is to start the link at
>>> 2.5GT/s speeds and once the link is up then change the maximum speed
>>> back
>>> to the default.
>>>
>>> The U-Boot driver claims to set the link-speed to 2.5GT/s to get the
>>> probe
>>> to work (and U-Boot does print link up at 2.5GT/s) in the following
>>> code:
>>> https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/pci/pcie_dw_sifive.c?id=v2022.01#L271
>>>
>>>
>>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
[snip]
>>> static int fu740_pcie_host_init(struct pcie_port *pp)
>>
>> +cc Maciej and David as there is this other fix that seems to do the
>> same but differently, it's been under review for some time now:
>> https://lore.kernel.org/all/20220302000043.GA662523@bhelgaas/t/
>>
I did have a quick look, but I think because we don't get any PCIe
probing at-all we don't even have a device to attach to.
>> I fell onto this issue recently, I'll give your patch and the above a
>> try soon.
>
>
> FWIW, I have tested this and it solved my issue with nvme not being
> probed, so:
>
> Tested-by: Alexandre Ghiti <alexandre.ghiti@canonical.com>
Ok, great. Our test rig seems to be still working with this.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [V3] PCI: fu740: Drop to 2.5GT/s to fix initial device probing on some boards
2022-03-23 10:40 ` Ben Dooks
@ 2022-04-14 0:10 ` Maciej W. Rozycki
2022-04-14 7:11 ` Ben Dooks
0 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2022-04-14 0:10 UTC (permalink / raw)
To: Ben Dooks
Cc: Alexandre Ghiti, linux-pci, linux-kernel, linux-riscv,
Bjorn Helgaas, Palmer Dabbelt, Rob Herring, Lorenzo Pieralisi,
Greentime Hu, Paul Walmsley, David Abdurachmanov, Neill Whillans
On Wed, 23 Mar 2022, Ben Dooks wrote:
> > FWIW, I have tested this and it solved my issue with nvme not being probed,
> > so:
> >
> > Tested-by: Alexandre Ghiti <alexandre.ghiti@canonical.com>
>
> Ok, great. Our test rig seems to be still working with this.
I ran simple verification of your change by interrupting U-Boot after a
power-up and issuing:
=> setenv boot_pci_enum true
at the command prompt before booting from the uSD card and curiously
enough the root port comes up with the Link Capabilities Register
reporting the lack of Link Bandwidth Notification Capability in this
scenario, while it reports its presence if booted undisturbed, i.e.:
LnkCap: Port #0, Speed 8GT/s, Width x8, ASPM L0s L1, Exit Latency L0s <4us, L1 <4us
ClockPM- Surprise- LLActRep+ BwNot- ASPMOptComp+
vs:
LnkCap: Port #0, Speed 8GT/s, Width x8, ASPM L0s L1, Exit Latency L0s <4us, L1 <4us
ClockPM- Surprise- LLActRep+ BwNot+ ASPMOptComp+
It is fully reproducible.
Any idea what might be causing it? I can't see it being explicitly set
or cleared anywhere, be it in U-Boot or Linux, so it must be done by the
device itself depending on something. And the lack of this capability
seems to me like non-compliance for a multiple-lane, multiple-speed
device.
Maciej
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [V3] PCI: fu740: Drop to 2.5GT/s to fix initial device probing on some boards
2022-04-14 0:10 ` Maciej W. Rozycki
@ 2022-04-14 7:11 ` Ben Dooks
2022-05-02 11:46 ` Maciej W. Rozycki
0 siblings, 1 reply; 11+ messages in thread
From: Ben Dooks @ 2022-04-14 7:11 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: Alexandre Ghiti, linux-pci, linux-kernel, linux-riscv,
Bjorn Helgaas, Palmer Dabbelt, Rob Herring, Lorenzo Pieralisi,
Greentime Hu, Paul Walmsley, David Abdurachmanov, Neill Whillans
On 14/04/2022 01:10, Maciej W. Rozycki wrote:
> On Wed, 23 Mar 2022, Ben Dooks wrote:
>
>>> FWIW, I have tested this and it solved my issue with nvme not being probed,
>>> so:
>>>
>>> Tested-by: Alexandre Ghiti <alexandre.ghiti@canonical.com>
>>
>> Ok, great. Our test rig seems to be still working with this.
>
> I ran simple verification of your change by interrupting U-Boot after a
> power-up and issuing:
>
> => setenv boot_pci_enum true
>
> at the command prompt before booting from the uSD card and curiously
> enough the root port comes up with the Link Capabilities Register
> reporting the lack of Link Bandwidth Notification Capability in this
> scenario, while it reports its presence if booted undisturbed, i.e.:
>
> LnkCap: Port #0, Speed 8GT/s, Width x8, ASPM L0s L1, Exit Latency L0s <4us, L1 <4us
> ClockPM- Surprise- LLActRep+ BwNot- ASPMOptComp+
>
> vs:
>
> LnkCap: Port #0, Speed 8GT/s, Width x8, ASPM L0s L1, Exit Latency L0s <4us, L1 <4us
> ClockPM- Surprise- LLActRep+ BwNot+ ASPMOptComp+
>
> It is fully reproducible.
>
> Any idea what might be causing it? I can't see it being explicitly set
> or cleared anywhere, be it in U-Boot or Linux, so it must be done by the
> device itself depending on something. And the lack of this capability
> seems to me like non-compliance for a multiple-lane, multiple-speed
> device.
I'll see if we can reproduce this
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [V3] PCI: fu740: Drop to 2.5GT/s to fix initial device probing on some boards
2022-04-14 7:11 ` Ben Dooks
@ 2022-05-02 11:46 ` Maciej W. Rozycki
2022-05-29 15:56 ` Ben Dooks
0 siblings, 1 reply; 11+ messages in thread
From: Maciej W. Rozycki @ 2022-05-02 11:46 UTC (permalink / raw)
To: Ben Dooks
Cc: Alexandre Ghiti, linux-pci, linux-kernel, linux-riscv,
Bjorn Helgaas, Palmer Dabbelt, Rob Herring, Lorenzo Pieralisi,
Greentime Hu, Paul Walmsley, David Abdurachmanov, Neill Whillans
Hi Ben,
> > Any idea what might be causing it? I can't see it being explicitly set
> > or cleared anywhere, be it in U-Boot or Linux, so it must be done by the
> > device itself depending on something. And the lack of this capability
> > seems to me like non-compliance for a multiple-lane, multiple-speed
> > device.
>
> I'll see if we can reproduce this
Have you been able to look into it?
Maciej
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [V3] PCI: fu740: Drop to 2.5GT/s to fix initial device probing on some boards
2022-05-02 11:46 ` Maciej W. Rozycki
@ 2022-05-29 15:56 ` Ben Dooks
0 siblings, 0 replies; 11+ messages in thread
From: Ben Dooks @ 2022-05-29 15:56 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: Alexandre Ghiti, linux-pci, linux-kernel, linux-riscv,
Bjorn Helgaas, Palmer Dabbelt, Rob Herring, Lorenzo Pieralisi,
Greentime Hu, Paul Walmsley, David Abdurachmanov, Neill Whillans
On 02/05/2022 12:46, Maciej W. Rozycki wrote:
> Hi Ben,
>
>>> Any idea what might be causing it? I can't see it being explicitly set
>>> or cleared anywhere, be it in U-Boot or Linux, so it must be done by the
>>> device itself depending on something. And the lack of this capability
>>> seems to me like non-compliance for a multiple-lane, multiple-speed
>>> device.
>>
>> I'll see if we can reproduce this
>
> Have you been able to look into it?
>
I can't see anything obvious from the code, however my SD card image
does not have lspci on it and I have not had time to make a new image
to test.
I wonder if anyone at SiFive can comment on this?
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-05-29 16:15 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-18 15:24 [V3] PCI: fu740: Drop to 2.5GT/s to fix initial device probing on some boards Ben Dooks
2022-03-18 23:03 ` Palmer Dabbelt
2022-03-19 12:40 ` Ben Dooks
2022-03-21 13:49 ` Alexandre Ghiti
2022-03-23 10:36 ` Alexandre Ghiti
2022-03-23 10:40 ` Ben Dooks
2022-04-14 0:10 ` Maciej W. Rozycki
2022-04-14 7:11 ` Ben Dooks
2022-05-02 11:46 ` Maciej W. Rozycki
2022-05-29 15:56 ` Ben Dooks
2022-03-21 20:29 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).