On Friday 12 November 2021 15:01:57 Stefan Roese wrote: > On 11/11/21 16:35, Marek Behún wrote: > > From: Pali Rohár <p...@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 <p...@kernel.org> > > Signed-off-by: Marek Behún <marek.be...@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?
It looks like that axp serdes code also touches register of PCIe Root Ports, e.g. in section /* 6.2 PCI Express Link Capabilities */. So it is something which could be improved and cleaned. But it should not cause any issue if registers are configures two times, once in serdes code and once in pci-mvebu.c code. Moreover registers in pci-mvebu space, including config space of PCIe Root Ports are reconfigured by kernel, so I think that it should not cause any issue. But what could cause issues is X1 vs X4 configuration in pci-mvebu.c if "num-lanes" property is not properly set. As is mentioned in commit message there is no A38x board in U-Boot which uses X4. Now with your comment for Armada XP I checked that serdes code and find out that AXP uses macro 'PEX_BUS_MODE_X4' for X4 mode (A38x serdes uses PEX_ROOT_COMPLEX_X4). And in U-Boot there are two boards which sets this macro: board/Synology/ds414/ds414.c and board/theadorable/theadorable.c So it would be needed to adjust this patch for these two boards. Please hold this one patch for now. I will try to prepare new fixed version. Other patches should be OK and independent of this one. > 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: s...@denx.de