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
Gen1 speeds and once the link is up then change the speed back.
The U-Boot driver claims to set the link-speed to Gen1 to get the probe
to work (and U-Boot does print link up at Gen1) 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 <[email protected]>
--
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.
---
drivers/pci/controller/dwc/pcie-fu740.c | 51 ++++++++++++++++++++++++-
1 file changed, 50 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-fu740.c b/drivers/pci/controller/dwc/pcie-fu740.c
index 842b7202b96e..16ad52f53490 100644
--- a/drivers/pci/controller/dwc/pcie-fu740.c
+++ b/drivers/pci/controller/dwc/pcie-fu740.c
@@ -181,10 +181,59 @@ 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 Gen1 when starting link, due to some devices not
+ * probing at higher speeds. This happens with the PCIe switch
+ * on the Unmatched board. The fix in U-Boot is to force Gen1
+ * and hope later resets will clear this capaility.
+ */
+
+ 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:
+ // todo - if we do have an unliekly error, what do we do here?
+ dw_pcie_dbi_ro_wr_dis(pci);
+ return ret;
}
static int fu740_pcie_host_init(struct pcie_port *pp)
--
2.34.1
On 28/02/2022 23:22, 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
> Gen1 speeds and once the link is up then change the speed back.
>
> The U-Boot driver claims to set the link-speed to Gen1 to get the probe
> to work (and U-Boot does print link up at Gen1) 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 <[email protected]>
> --
> 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.
So the pci-imx6.c driver does wait after the setting of
PCIE_LINK_WIDTH_SPEED_CONTROL for the PCIE_LINK_WIDTH_SPEED_CONTROL
to show PORT_LOGIC_SPEED_CHANGE. Not sure if this is needed in this
case.
I have put this driver in to our CI and it so far seems to be working
in our case. I'm not sure if there's anything we could do better to
detect already initialised values from U-Boot and avoid this. Also I
am questioning if we need some sort of hardware property to control
the behaviour.
Does anyone at SiFive have any more information about whether this fix
is now correct? It is very annoying for us as we network boot and thus
the PCIe does not come up without this fix in.
Note, it is possible the pci-imx6.c driver does not build, could not
find a field called linl in the struct dw_pcie. I will try and see if
this driver is being built or not.
> ---
> drivers/pci/controller/dwc/pcie-fu740.c | 51 ++++++++++++++++++++++++-
> 1 file changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-fu740.c b/drivers/pci/controller/dwc/pcie-fu740.c
> index 842b7202b96e..16ad52f53490 100644
> --- a/drivers/pci/controller/dwc/pcie-fu740.c
> +++ b/drivers/pci/controller/dwc/pcie-fu740.c
> @@ -181,10 +181,59 @@ 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 Gen1 when starting link, due to some devices not
> + * probing at higher speeds. This happens with the PCIe switch
> + * on the Unmatched board. The fix in U-Boot is to force Gen1
> + * and hope later resets will clear this capaility.
> + */
> +
> + 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:
> + // todo - if we do have an unliekly error, what do we do here?
> + dw_pcie_dbi_ro_wr_dis(pci);
> + return ret;
> }
>
> static int fu740_pcie_host_init(struct pcie_port *pp)
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
On 28/02/2022 23:22, 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
> Gen1 speeds and once the link is up then change the speed back.
>
> The U-Boot driver claims to set the link-speed to Gen1 to get the probe
> to work (and U-Boot does print link up at Gen1) 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 <[email protected]>
> --
> 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.
The internal feedback is this version is passing on our CI.
If there are no comments on this soon, I will post this as either the
v5 of the original or as a new patch.
> ---
> drivers/pci/controller/dwc/pcie-fu740.c | 51 ++++++++++++++++++++++++-
> 1 file changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-fu740.c b/drivers/pci/controller/dwc/pcie-fu740.c
> index 842b7202b96e..16ad52f53490 100644
> --- a/drivers/pci/controller/dwc/pcie-fu740.c
> +++ b/drivers/pci/controller/dwc/pcie-fu740.c
> @@ -181,10 +181,59 @@ 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 Gen1 when starting link, due to some devices not
> + * probing at higher speeds. This happens with the PCIe switch
> + * on the Unmatched board. The fix in U-Boot is to force Gen1
> + * and hope later resets will clear this capaility.
> + */
> +
> + 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:
> + // todo - if we do have an unliekly error, what do we do here?
> + dw_pcie_dbi_ro_wr_dis(pci);
> + return ret;
> }
>
> static int fu740_pcie_host_init(struct pcie_port *pp)
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
On Tue, Mar 08, 2022 at 09:45:36AM +0000, Ben Dooks wrote:
> On 28/02/2022 23:22, 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
> > Gen1 speeds and once the link is up then change the speed back.
> >
> > The U-Boot driver claims to set the link-speed to Gen1 to get the probe
> > to work (and U-Boot does print link up at Gen1) 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 <[email protected]>
> > --
> > 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.
>
> The internal feedback is this version is passing on our CI.
>
> If there are no comments on this soon, I will post this as either the
> v5 of the original or as a new patch.
Seems like this isn't quite baked yet. Lorenzo has the v4 of this on
his pci/fu740 branch, but I'm going to drop that for now because (a)
this one is better and (b) it'd be nice to have an ack from a FU740
maintainer (Paul or Greentime).
I'm also not clear on whether this works around a general FU740 defect
or something specific to the Unmatched board or the ASMedia ASM2824
switch. This patch currently limits to 2.5GT/s on *all* FU740
devices.
I'd prefer to use "2.5GT/s" instead of "Gen1" in the subject, commit
log, and comments because it's more specific and matches the
PCI_EXP_LNKCAP_SLS_2_5GB in the code.
> > ---
> > drivers/pci/controller/dwc/pcie-fu740.c | 51 ++++++++++++++++++++++++-
> > 1 file changed, 50 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-fu740.c b/drivers/pci/controller/dwc/pcie-fu740.c
> > index 842b7202b96e..16ad52f53490 100644
> > --- a/drivers/pci/controller/dwc/pcie-fu740.c
> > +++ b/drivers/pci/controller/dwc/pcie-fu740.c
> > @@ -181,10 +181,59 @@ 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 Gen1 when starting link, due to some devices not
> > + * probing at higher speeds. This happens with the PCIe switch
> > + * on the Unmatched board. The fix in U-Boot is to force Gen1
> > + * and hope later resets will clear this capaility.
s/capaility/capability/
But the sentence still doesn't quite make sense. Are you saying that
if we bring the link up at 2.5GT/s, it will stay there?
And that a future reset may clear Link Capabilities? Actually, I
guess you don't want it *cleared*, you would just want it to
accurately reflect the real max link speed, which would not be 0000b
in the register (since that's not even a defined encoding).
And the reset would also cause link retrain that would then use the
real max link speed?
> > + 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:
> > + // todo - if we do have an unliekly error, what do we do here?
Wrong comment style (use /* */, not //), and s/unliekly/unlikely/
> > + dw_pcie_dbi_ro_wr_dis(pci);
> > + return ret;
> > }
On 10/03/2022 00:15, Bjorn Helgaas wrote:
> On Tue, Mar 08, 2022 at 09:45:36AM +0000, Ben Dooks wrote:
>> On 28/02/2022 23:22, 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
>>> Gen1 speeds and once the link is up then change the speed back.
>>>
>>> The U-Boot driver claims to set the link-speed to Gen1 to get the probe
>>> to work (and U-Boot does print link up at Gen1) 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 <[email protected]>
>>> --
>>> 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.
>>
>> The internal feedback is this version is passing on our CI.
>>
>> If there are no comments on this soon, I will post this as either the
>> v5 of the original or as a new patch.
>
> Seems like this isn't quite baked yet. Lorenzo has the v4 of this on
> his pci/fu740 branch, but I'm going to drop that for now because (a)
> this one is better and (b) it'd be nice to have an ack from a FU740
> maintainer (Paul or Greentime).
Yes. I'll fix the comments up and try and get this out later in the
week. I hope the GPIO patch is easier and can be merged on its own.
It would be great if someone at SiFive could comment on this, I don't
really have a lot of info other than this doesn't work for any of our
boards. I just assume that everyone else boots from NVME and it just
works for them.
> I'm also not clear on whether this works around a general FU740 defect
> or something specific to the Unmatched board or the ASMedia ASM2824
> switch. This patch currently limits to 2.5GT/s on *all* FU740
> devices.
I am not sure on this either, all I have is a pair of Unmatched boards
and neither work without this fix in. Has the FU740 been used by anyone
other than this board?
I have not verified with a speed test any of this yet. It should do
2.5GT/s as initial probe and then attempt to get the link back to
the maximum speed once the device has probed. It seems once the
ASM2824 has done the first initialisation it will then continue
back at the higher speed.
Once the ASM2824 is working it seems the rest will follow.
> I'd prefer to use "2.5GT/s" instead of "Gen1" in the subject, commit
> log, and comments because it's more specific and matches the
> PCI_EXP_LNKCAP_SLS_2_5GB in the code.
Ok, will do.
>>> ---
>>> drivers/pci/controller/dwc/pcie-fu740.c | 51 ++++++++++++++++++++++++-
>>> 1 file changed, 50 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/pci/controller/dwc/pcie-fu740.c b/drivers/pci/controller/dwc/pcie-fu740.c
>>> index 842b7202b96e..16ad52f53490 100644
>>> --- a/drivers/pci/controller/dwc/pcie-fu740.c
>>> +++ b/drivers/pci/controller/dwc/pcie-fu740.c
>>> @@ -181,10 +181,59 @@ 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 Gen1 when starting link, due to some devices not
>>> + * probing at higher speeds. This happens with the PCIe switch
>>> + * on the Unmatched board. The fix in U-Boot is to force Gen1
>>> + * and hope later resets will clear this capaility.
>
> s/capaility/capability/
>
> But the sentence still doesn't quite make sense. Are you saying that
> if we bring the link up at 2.5GT/s, it will stay there?
>
> And that a future reset may clear Link Capabilities? Actually, I
> guess you don't want it *cleared*, you would just want it to
> accurately reflect the real max link speed, which would not be 0000b
> in the register (since that's not even a defined encoding).
So what I've seen is if U-boot does the initial probe at 2.5GT/s and
then Linux comes along and does the reset itself, the LINKCAP gets
set back to the original full speed then the device probe works under
Linux. I've not verified with any NVME speed test yet.
> And the reset would also cause link retrain that would then use the
> real max link speed?
I think so, the only verification I have done is lspci to check what
the link is reporting.
>>> + 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:
>>> + // todo - if we do have an unliekly error, what do we do here?
>
> Wrong comment style (use /* */, not //), and s/unliekly/unlikely/
Ok, if no-one has a better idea I am just going to return the error
code for now.
>
>>> + dw_pcie_dbi_ro_wr_dis(pci);
>>> + return ret;
>>> }
>
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
On Thu, Mar 10, 2022 at 09:06:08AM +0000, Ben Dooks wrote:
> On 10/03/2022 00:15, Bjorn Helgaas wrote:
> > Seems like this isn't quite baked yet. Lorenzo has the v4 of this on
> > his pci/fu740 branch, but I'm going to drop that for now because (a)
> > this one is better and (b) it'd be nice to have an ack from a FU740
> > maintainer (Paul or Greentime).
>
> Yes. I'll fix the comments up and try and get this out later in the
> week. I hope the GPIO patch is easier and can be merged on its own.
Yes, it's in -next now.