From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Roese <sr@denx.de>
Date: Tue, 22 Dec 2015 09:24:21 +0100
Subject: [U-Boot] [PATCH v2 5/8] drivers/pci/pci_mvebu: Fix for boards
 with X4 lanes
In-Reply-To: <20151221232441.D035661201@mail.nwl.cc>
References: <1450740358-5014-1-git-send-email-phil@nwl.cc>
 <20151221232441.D035661201@mail.nwl.cc>
Message-ID: <567908B5.3030708@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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