[U-Boot] [PATCH v2 5/8] drivers/pci/pci_mvebu: Fix for boards with X4 lanes

Stefan Roese
2015-12-22T08:24:21Z

On 22.12.2015 00:25, Phil Sutter wrote:
> Armada XP has support for X4 lanes, boards specify this in their
> serdes_cfg. During PEX init in high_speed_env_lib.c, the configuration
> is stored in GEN_PURP_RES_2_REG.
>
> When enumerating PEX, subsequent interfaces of an X4 lane must be
> skipped. Otherwise the enumeration hangs up the board.
>
> The way this is implemented here is not exactly beautiful, but it mimics
> how Marvell's BSP does it. Alternatively we could get the information
> using board_serdes_cfg_get(), but that won't lead to clean code, either.
> Especially since the ugly includes will have to be done anyway.
>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> drivers/pci/pci_mvebu.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> index fd2744d..aab53f4 100644
> --- a/drivers/pci/pci_mvebu.c
> +++ b/drivers/pci/pci_mvebu.c
> @@ -18,6 +18,11 @@
> #include <asm/arch/soc.h>
> #include <linux/mbus.h>
>
> +#if defined(CONFIG_ARMADA_XP)
> +#include "../ddr/marvell/axp/ddr3_init.h"
> +#include "../../arch/arm/mach-mvebu/serdes/axp/board_env_spec.h"
> +#endif

Yes, this is ugly. And AFAICT, you only need it to include the
GEN_PURP_RES_2_REG define from these headers.

I would prefer to add this macro either here (again), or in soc.h.
And use the name from the Marvell manual, so something like

#define COMPHY_REFCLK_ALIGNMENT (MVEBU_REGISTER(0x182f8))

> +
> DECLARE_GLOBAL_DATA_PTR;
>
> /* PCIe unit register offsets */
> @@ -155,6 +160,16 @@ static void mvebu_get_port_lane(struct mvebu_pcie *pcie, int pex_idx,
> }
> #endif
>
> +#if defined(CONFIG_ARMADA_XP)
> +static int mvebu_pex_unit_is_x4(int pex_idx)
> +{
> + int pex_unit = pex_idx < 9 ? pex_idx >> 2 : 3;
> + u32 mask = (0x0f << (pex_unit * 8));
> +
> + return (reg_read(GEN_PURP_RES_2_REG) & mask) == mask;

readl() with the new define please.

> +}
> +#endif

Please drop the #ifdef here. This can go in unconditionally.

> +
> static inline bool mvebu_pcie_link_up(struct mvebu_pcie *pcie)
> {
> u32 val;
> @@ -419,5 +434,11 @@ void pci_init_board(void)
> writel(0, pcie->base + PCIE_BAR_HI_OFF(0));
>
> bus = hose->last_busno + 1;
> +
> +#if defined(CONFIG_ARMADA_XP)
> + /* need to skip more for X4 links, otherwise scan will hang */
> + if (mvebu_pex_unit_is_x4(i))
> + i += 3;
> +#endif

And please use this version here:

/* need to skip more for X4 links, otherwise scan will hang */
if (mvebu_soc_family() == MVEBU_SOC_AXP) {
if (mvebu_pex_unit_is_x4(i))
i += 3;
}

Thanks,
Stefan