| Message ID | 20220222155030.988-6-pali@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | PCI: mvebu: subsystem ids, AER and INTx | expand |
On Tue, Feb 22, 2022 at 04:50:23PM +0100, Pali Rohár wrote: > If x1/x4 mode is not set correctly then link with endpoint card is not > established. > > Use DTS property 'num-lanes' to deteriminate x1/x4 mode. I know this is already merged, but if tweaking for any other reason, s/deteriminate/determine/ > + * 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. True, everything in Link Capability is RO or HwInit, but that's for the architected access via config space. I think a device-specific mechanism like this is fair game as long as you do it before anybody can read it via config space. > + */ > + lnkcap = mvebu_readl(port, PCIE_CAP_PCIEXP + PCI_EXP_LNKCAP); > + lnkcap &= ~PCI_EXP_LNKCAP_MLW; > + lnkcap |= (port->is_x4 ? 4 : 1) << 4; > + mvebu_writel(port, lnkcap, PCIE_CAP_PCIEXP + PCI_EXP_LNKCAP);
On Thursday 24 February 2022 18:08:00 Bjorn Helgaas wrote: > On Tue, Feb 22, 2022 at 04:50:23PM +0100, Pali Rohár wrote: > > If x1/x4 mode is not set correctly then link with endpoint card is not > > established. > > > > Use DTS property 'num-lanes' to deteriminate x1/x4 mode. > > I know this is already merged, but if tweaking for any other reason, > s/deteriminate/determine/ > > > + * 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. > > True, everything in Link Capability is RO or HwInit, but that's for > the architected access via config space. I think a device-specific > mechanism like this is fair game as long as you do it before anybody > can read it via config space. Maybe I was not clear and explicit in above comment, but this register sets number of PCIe lanes which HW will use. Armada PCIe controllers supports only x1 and x4. Sometimes default HW value is 4 for x1 HW and sometimes default value for x4 HW is 1. First case cause that link never comes up (HW is trying to setup 4 lanes but in reality there is only one, so link training never finish) and second case cause degraded performance (x4 link is established only in x1 mode as HW is via this register instructed to ignores other 3 lanes). So basically HW designers misused this Link Capability register for configuring PCIe Link of PCIe Root Port. > > + */ > > + lnkcap = mvebu_readl(port, PCIE_CAP_PCIEXP + PCI_EXP_LNKCAP); > > + lnkcap &= ~PCI_EXP_LNKCAP_MLW; > > + lnkcap |= (port->is_x4 ? 4 : 1) << 4; > > + mvebu_writel(port, lnkcap, PCIE_CAP_PCIEXP + PCI_EXP_LNKCAP);
diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c index 357f0f41f68e..d0a75c3b78c3 100644 --- a/drivers/pci/controller/pci-mvebu.c +++ b/drivers/pci/controller/pci-mvebu.c @@ -93,6 +93,7 @@ struct mvebu_pcie_port { void __iomem *base; u32 port; u32 lane; + bool is_x4; int devfn; unsigned int mem_target; unsigned int mem_attr; @@ -233,13 +234,25 @@ static void mvebu_pcie_setup_wins(struct mvebu_pcie_port *port) static void mvebu_pcie_setup_hw(struct mvebu_pcie_port *port) { - u32 ctrl, cmd, dev_rev, mask; + u32 ctrl, lnkcap, cmd, dev_rev, mask; /* Setup PCIe controller to Root Complex mode. */ ctrl = mvebu_readl(port, PCIE_CTRL_OFF); ctrl |= PCIE_CTRL_RC_MODE; mvebu_writel(port, ctrl, 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. + */ + lnkcap = mvebu_readl(port, PCIE_CAP_PCIEXP + PCI_EXP_LNKCAP); + lnkcap &= ~PCI_EXP_LNKCAP_MLW; + lnkcap |= (port->is_x4 ? 4 : 1) << 4; + mvebu_writel(port, lnkcap, PCIE_CAP_PCIEXP + PCI_EXP_LNKCAP); + /* Disable Root Bridge I/O space, memory space and bus mastering. */ cmd = mvebu_readl(port, PCIE_CMD_OFF); cmd &= ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER); @@ -982,6 +995,7 @@ static int mvebu_pcie_parse_port(struct mvebu_pcie *pcie, struct device *dev = &pcie->pdev->dev; enum of_gpio_flags flags; int reset_gpio, ret; + u32 num_lanes; port->pcie = pcie; @@ -994,6 +1008,9 @@ static int mvebu_pcie_parse_port(struct mvebu_pcie *pcie, if (of_property_read_u32(child, "marvell,pcie-lane", &port->lane)) port->lane = 0; + if (!of_property_read_u32(child, "num-lanes", &num_lanes) && num_lanes == 4) + port->is_x4 = true; + port->name = devm_kasprintf(dev, GFP_KERNEL, "pcie%d.%d", port->port, port->lane); if (!port->name) {
If x1/x4 mode is not set correctly then link with endpoint card is not established. Use DTS property 'num-lanes' to deteriminate x1/x4 mode. Signed-off-by: Pali Rohár <pali@kernel.org> --- drivers/pci/controller/pci-mvebu.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)