[PATCH u-boot-marvell 02/10] arm: mvebu: a38x: serdes: Move non-serdes PCIe code to pci_mvebu.c
Pali Rohár
pali at kernel.org
Thu Nov 18 19:01:03 CET 2021
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 <pali at 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 at kernel.org>
> > Signed-off-by: Marek Behún <marek.behun at 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: sr at denx.de
More information about the U-Boot
mailing list