Re: [PATCH v2 05/11] PCI: mvebu: Correctly configure x1/x4 mode

From: Rob Herring
Date: Thu Jan 20 2022 - 12:09:21 EST


On Wed, Jan 12, 2022 at 04:18:08PM +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.
>
> Signed-off-by: Pali Rohár <pali@xxxxxxxxxx>
> ---
> drivers/pci/controller/pci-mvebu.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
> index a075ba26cff1..0f2ec0a17874 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;

I would just store the number of lanes.

> 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;

then this is just: lanes << 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);
> @@ -986,6 +999,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;
>
> @@ -998,6 +1012,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;

And this can be:

num_lanes = 1;
of_property_read_u32(child, "num-lanes", &num_lanes);

If you want to validate the DT is only 1 or 4, make the DT schema do
that.


> +
> port->name = devm_kasprintf(dev, GFP_KERNEL, "pcie%d.%d", port->port,
> port->lane);
> if (!port->name) {
> --
> 2.20.1
>
>