Re: [V3] PCI: fu740: Drop to 2.5GT/s to fix initial device probing on some boards

[Ben Dooks]

On 18/03/2022 23:03, Palmer Dabbelt wrote:
> On Fri, 18 Mar 2022 08:24:30 PDT (-0700), ben.dooks@xxxxxxxxxxxxxxx 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@xxxxxxxxxxxxxxx>
> > --
>
> 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@xxxxxxxxxxxx>
>
> 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