On Friday 12 November 2021 14:59:31 Stefan Roese wrote:
> > diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> > index 14cd82db6f..a3364d5a59 100644
> > --- a/drivers/pci/pci_mvebu.c
> > +++ b/drivers/pci/pci_mvebu.c
> > @@ -22,6 +22,7 @@
> > #include <asm/arch/cpu.h>
> > #include <asm/arch/soc.h>
> > #include <linux/bitops.h>
> > +#include <linux/delay.h>
> > #include <linux/errno.h>
> > #include <linux/ioport.h>
> > #include <linux/mbus.h>
> > @@ -70,6 +71,9 @@ DECLARE_GLOBAL_DATA_PTR;
> > #define PCIE_DEBUG_CTRL 0x1a60
> > #define PCIE_DEBUG_SOFT_RESET BIT(20)
> > +#define LINK_WAIT_RETRIES 100
> > +#define LINK_WAIT_TIMEOUT 1000
>
> Wouldn't it be easier read, if this was defines like this:
>
> #define LINK_TIMEOUT_MS 100
> #define LINK_WAIT_TIMEOUT_US 1000
> #define LINK_WAIT_RETRIES ((LINK_TIMEOUT_MS * 1000) /
> LINK_WAIT_TIMEOUT_US)
It looks like a good idea!
> Other than this:
>
> Reviewed-by: Stefan Roese <s...@denx.de>
>
> Thanks,
> Stefan
>
> > +
> > struct mvebu_pcie {
> > struct pci_controller hose;
> > void __iomem *base;
> > @@ -106,6 +110,23 @@ static inline bool mvebu_pcie_link_up(struct
> > mvebu_pcie *pcie)
> > return !(val & PCIE_STAT_LINK_DOWN);
> > }
> > +static void mvebu_pcie_wait_for_link(struct mvebu_pcie *pcie)
> > +{
> > + int retries;
> > +
> > + /* check if the link is up or not */
> > + for (retries = 0; retries < LINK_WAIT_RETRIES; retries++) {
> > + if (mvebu_pcie_link_up(pcie)) {
> > + printf("%s: Link up\n", pcie->name);
> > + return;
> > + }
> > +
> > + udelay(LINK_WAIT_TIMEOUT);
> > + }
> > +
> > + printf("%s: Link down\n", pcie->name);
> > +}
> > +
> > static void mvebu_pcie_set_local_bus_nr(struct mvebu_pcie *pcie, int
> > busno)
> > {
> > u32 stat;
