Re: [PATCH u-boot-marvell 02/10] arm: mvebu: a38x: serdes: Move non-serdes PCIe code to pci_mvebu.c

[Stefan Roese <sr@denx.de>]

On 11/11/21 16:35, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
> 
> As explained in commit 3bedbcc3aa18 ("arm: mvebu: a38x: serdes: Don't
> overwrite read-only SAR PCIe registers") it is required to set Maximum Link
> Width bits of PCIe Root Port Link Capabilities Register depending of number
> of used serdes lanes. As this register is part of PCIe address space and
> not serdes address space, move it into pci_mvebu.c driver.
> 
> Read number of PCIe lanes from DT propery "num-lanes" which is used also by
> other PCIe controller drivers in Linux kernel. If this property is absent.
> default to 1. This property needs to be set to 4 for every mvebu board
> which use PEX_ROOT_COMPLEX_X4. Currently in U-Boot there is no such board.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> ---
>   arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h     |  4 ----
>   .../serdes/a38x/high_speed_env_spec.c          | 15 ---------------
>   drivers/pci/pci_mvebu.c                        | 18 ++++++++++++++++++
>   3 files changed, 18 insertions(+), 19 deletions(-)

I'm wondering now, if and how this works on Armada XP, which uses the
same PCIe driver but a different serdes/axp/*. Did you take this into
account?

Thanks,
Stefan

> diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h
> index 64193d5288..0df898c625 100644
> --- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h
> +++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h
> @@ -6,12 +6,8 @@
>   #ifndef _CTRL_PEX_H
>   #define _CTRL_PEX_H
>   
> -#include <pci.h>
>   #include "high_speed_env_spec.h"
>   
> -/* Direct access to PEX0 Root Port's PCIe Capability structure */
> -#define PEX0_RP_PCIE_CFG_OFFSET		(0x00080000 + 0x60)
> -
>   /* SOC_CONTROL_REG1 fields */
>   #define PCIE0_ENABLE_OFFS		0
>   #define PCIE0_ENABLE_MASK		(0x1 << PCIE0_ENABLE_OFFS)
> diff --git a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c
> index d2bc3ab25c..ef4b89c96a 100644
> --- a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c
> +++ b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c
> @@ -1720,21 +1720,6 @@ int serdes_power_up_ctrl(u32 serdes_num, int serdes_power_up,
>   				else
>   					reg_data &= ~0x4000;
>   				reg_write(SOC_CONTROL_REG1, reg_data);
> -
> -				/*
> -				 * Set Maximum Link Width to X1 or X4 in Root
> -				 * Port's PCIe Link Capability register.
> -				 * This register is read-only but if is not set
> -				 * correctly then access to PCI config space of
> -				 * endpoint card behind this Root Port does not
> -				 * work.
> -				 */
> -				reg_data = reg_read(PEX0_RP_PCIE_CFG_OFFSET +
> -						    PCI_EXP_LNKCAP);
> -				reg_data &= ~PCI_EXP_LNKCAP_MLW;
> -				reg_data |= (is_pex_by1 ? 1 : 4) << 4;
> -				reg_write(PEX0_RP_PCIE_CFG_OFFSET +
> -					  PCI_EXP_LNKCAP, reg_data);
>   			}
>   
>   			CHECK_STATUS(mv_seq_exec(serdes_num, PEX_POWER_UP_SEQ));
> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> index a3364d5a59..278dc2756f 100644
> --- a/drivers/pci/pci_mvebu.c
> +++ b/drivers/pci/pci_mvebu.c
> @@ -83,6 +83,7 @@ struct mvebu_pcie {
>   	struct resource io;
>   	u32 port;
>   	u32 lane;
> +	bool is_x4;
>   	int devfn;
>   	u32 lane_mask;
>   	int first_busno;
> @@ -399,6 +400,18 @@ static int mvebu_pcie_probe(struct udevice *dev)
>   	reg |= PCIE_CTRL_RC_MODE;
>   	writel(reg, pcie->base + PCIE_CTRL_OFF);
>   
> +	/*
> +	 * Set Maximum Link Width to X1 or X4 in Root Port's PCIe Link
> +	 * Capability register. This register is defined by PCIe specification
> +	 * as read-only but this mvebu controller has it as read-write and must
> +	 * be set to number of SerDes PCIe lanes (1 or 4). If this register is
> +	 * not set correctly then link with endpoint card is not established.
> +	 */
> +	reg = readl(pcie->base + PCIE_CAPAB_OFF + PCI_EXP_LNKCAP);
> +	reg &= ~PCI_EXP_LNKCAP_MLW;
> +	reg |= (pcie->is_x4 ? 4 : 1) << 4;
> +	writel(reg, pcie->base + PCIE_CAPAB_OFF + PCI_EXP_LNKCAP);
> +
>   	/*
>   	 * Change Class Code of PCI Bridge device to PCI Bridge (0x600400)
>   	 * because default value is Memory controller (0x508000) which
> @@ -582,6 +595,7 @@ static int mvebu_get_tgt_attr(ofnode node, int devfn,
>   static int mvebu_pcie_of_to_plat(struct udevice *dev)
>   {
>   	struct mvebu_pcie *pcie = dev_get_plat(dev);
> +	u32 num_lanes = 1;
>   	int ret = 0;
>   
>   	/* Get port number, lane number and memory target / attr */
> @@ -594,6 +608,10 @@ static int mvebu_pcie_of_to_plat(struct udevice *dev)
>   	if (ofnode_read_u32(dev_ofnode(dev), "marvell,pcie-lane", &pcie->lane))
>   		pcie->lane = 0;
>   
> +	if (!ofnode_read_u32(dev_ofnode(dev), "num-lanes", &num_lanes) &&
> +	    num_lanes == 4)
> +		pcie->is_x4 = true;
> +
>   	sprintf(pcie->name, "pcie%d.%d", pcie->port, pcie->lane);
>   
>   	/* pci_get_devfn() returns devfn in bits 15..8, see PCI_DEV usage */
> 

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de