diff mbox series

[6/7] pci: Add DW PCIe controller support for iMX8MP SoC

Message ID 20240220131056.2962331-7-sumit.garg@linaro.org
State New
Headers show
Series imx8mp: Enable PCIe/NVMe support | expand

Commit Message

Sumit Garg Feb. 20, 2024, 1:10 p.m. UTC 
pcie_imx doesn't seem to share any useful code for iMX8 SoC and it is
tied to quite old port of pcie_designware driver from Linux which
suffices only iMX6 specific needs.

But currently we have the common DWC specific bits which alligns pretty
well with DW PCIe controller on iMX8MP SoC. So lets reuse those common
bits instead as a new driver for iMX8 SoCs. It should be fairly easy to
add support for other iMX8 variants to this driver.

iMX8MP SoC also comes up with standalone PCIe PHY support, so hence we
can reuse the generic PHY infrastructure to power on PCIe PHY.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 drivers/pci/Kconfig        |   8 +
 drivers/pci/Makefile       |   1 +
 drivers/pci/pcie_dw_imx8.c | 348 +++++++++++++++++++++++++++++++++++++
 3 files changed, 357 insertions(+)
 create mode 100644 drivers/pci/pcie_dw_imx8.c

Comments

Marek Vasut Feb. 20, 2024, 3:22 p.m. UTC | #1 
On 2/20/24 14:10, Sumit Garg wrote:
> pcie_imx doesn't seem to share any useful code for iMX8 SoC and it is
> tied to quite old port of pcie_designware driver from Linux which
> suffices only iMX6 specific needs.
> 
> But currently we have the common DWC specific bits which alligns pretty
> well with DW PCIe controller on iMX8MP SoC. So lets reuse those common
> bits instead as a new driver for iMX8 SoCs. It should be fairly easy to
> add support for other iMX8 variants to this driver.
> 
> iMX8MP SoC also comes up with standalone PCIe PHY support, so hence we
> can reuse the generic PHY infrastructure to power on PCIe PHY.
> 
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> ---
>   drivers/pci/Kconfig        |   8 +
>   drivers/pci/Makefile       |   1 +
>   drivers/pci/pcie_dw_imx8.c | 348 +++++++++++++++++++++++++++++++++++++
>   3 files changed, 357 insertions(+)
>   create mode 100644 drivers/pci/pcie_dw_imx8.c
> 
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 463ec47eb92..b7c7922b091 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -413,4 +413,12 @@ config PCIE_STARFIVE_JH7110
>   	  Say Y here if you want to enable PLDA XpressRich PCIe controller
>   	  support on StarFive JH7110 SoC.
>   
> +config PCIE_DW_IMX8
> +	bool "i.MX8 PCIe support"
> +	depends on ARCH_IMX8M
> +	select PCIE_DW_COMMON
> +	help
> +	  Say Y here if you want to enable DW PCIe controller support on
> +	  iMX8 SoCs.
> +
>   endif
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index 72ef8b4bc77..cddbb902095 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -53,3 +53,4 @@ obj-$(CONFIG_PCIE_UNIPHIER) += pcie_uniphier.o
>   obj-$(CONFIG_PCIE_XILINX_NWL) += pcie-xilinx-nwl.o
>   obj-$(CONFIG_PCIE_PLDA_COMMON) += pcie_plda_common.o
>   obj-$(CONFIG_PCIE_STARFIVE_JH7110) += pcie_starfive_jh7110.o
> +obj-$(CONFIG_PCIE_DW_IMX8) += pcie_dw_imx8.o
> diff --git a/drivers/pci/pcie_dw_imx8.c b/drivers/pci/pcie_dw_imx8.c
> new file mode 100644
> index 00000000000..b9921644765
> --- /dev/null
> +++ b/drivers/pci/pcie_dw_imx8.c
> @@ -0,0 +1,348 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2024 Linaro Ltd.
> + *
> + * Author: Sumit Garg <sumit.garg@linaro.org>
> + */
> +
> +#include <asm/io.h>
> +#include <asm-generic/gpio.h>
> +#include <clk.h>
> +#include <dm.h>
> +#include <dm/device_compat.h>
> +#include <generic-phy.h>
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <log.h>
> +#include <pci.h>
> +#include <regmap.h>
> +#include <reset.h>
> +#include <syscon.h>
> +#include <time.h>
> +
> +#include "pcie_dw_common.h"
> +
> +#define PCIE_LINK_CAPABILITY		0x7c
> +#define TARGET_LINK_SPEED_MASK		0xf
> +#define LINK_SPEED_GEN_1		0x1
> +#define LINK_SPEED_GEN_2		0x2
> +#define LINK_SPEED_GEN_3		0x3
> +
> +#define PCIE_MISC_CONTROL_1_OFF		0x8bc
> +#define PCIE_DBI_RO_WR_EN		BIT(0)
> +
> +#define PCIE_PORT_DEBUG0			0x728
> +#define PCIE_PORT_DEBUG1			0x72c
> +#define PCIE_PORT_DEBUG1_LINK_UP		BIT(4)
> +#define PCIE_PORT_DEBUG1_LINK_IN_TRAINING	BIT(29)
> +
> +#define PCIE_LINK_UP_TIMEOUT_MS		100
> +
> +#define IOMUXC_GPR14_OFFSET			0x38
> +#define IMX8M_GPR_PCIE_CLK_REQ_OVERRIDE_EN	BIT(10)
> +#define IMX8M_GPR_PCIE_CLK_REQ_OVERRIDE		BIT(11)
> +
> +struct pcie_dw_imx8 {
> +	/* Must be first member of the struct */
> +	struct pcie_dw			dw;
> +	struct regmap			*iomuxc_gpr;
> +	struct clk			pcie;
> +	struct clk			pcie_bus;
> +	struct clk			pcie_aux;
> +	struct gpio_desc		reset_gpio;
> +	struct reset_ctl		apps_reset;
> +	struct phy			phy;
> +};
> +
> +static void pcie_dw_configure(struct pcie_dw_imx8 *priv, u32 cap_speed)
> +{
> +	u32 val;
> +
> +	dw_pcie_dbi_write_enable(&priv->dw, true);
> +
> +	val = readl(priv->dw.dbi_base + PCIE_LINK_CAPABILITY);
> +	val &= ~TARGET_LINK_SPEED_MASK;
> +	val |= cap_speed;
> +	writel(val, priv->dw.dbi_base + PCIE_LINK_CAPABILITY);

clrsetbits_le32()

> +
> +	dw_pcie_dbi_write_enable(&priv->dw, false);
> +}
> +
> +static void imx8_pcie_ltssm_enable(struct pcie_dw_imx8 *priv)
> +{
> +	reset_deassert(&priv->apps_reset);
> +}
> +
> +static void imx8_pcie_ltssm_disable(struct pcie_dw_imx8 *priv)
> +{
> +	reset_assert(&priv->apps_reset);
> +}
> +
> +static int is_link_up(struct pcie_dw_imx8 *priv)
> +{
> +	u32 val;
> +
> +	val = readl(priv->dw.dbi_base + PCIE_PORT_DEBUG1);
> +
> +	return ((val & PCIE_PORT_DEBUG1_LINK_UP) &&
> +		(!(val & PCIE_PORT_DEBUG1_LINK_IN_TRAINING)));
> +}
> +
> +static int wait_link_up(struct pcie_dw_imx8 *priv)
> +{
> +	unsigned long timeout;
> +
> +	timeout = get_timer(0) + PCIE_LINK_UP_TIMEOUT_MS;

wait_for_bit() or read_poll_timeout()

> +	while (!is_link_up(priv)) {
> +		if (get_timer(0) > timeout)
> +			return 0;
> +		mdelay(10);
> +	};
> +
> +	return 1;

return -ETIMEDOUT ?

> +}
> +
> +static int pcie_link_up(struct pcie_dw_imx8 *priv, u32 cap_speed)
> +{
> +	if (is_link_up(priv)) {
> +		printf("PCI Link already up before configuration!\n");
> +		return 1;
> +	}
> +
> +	/* DW pre link configurations */
> +	pcie_dw_configure(priv, cap_speed);
> +
> +	/* Initiate link training */
> +	imx8_pcie_ltssm_enable(priv);
> +
> +	/* Check that link was established */
> +	if (!wait_link_up(priv)) {
> +		imx8_pcie_ltssm_disable(priv);
> +		return 0;
> +	}
> +
> +	return 1;
> +}
> +
> +static int imx8_pcie_assert_core_reset(struct pcie_dw_imx8 *priv)
> +{
> +	if (dm_gpio_is_valid(&priv->reset_gpio)) {
> +		dm_gpio_set_value(&priv->reset_gpio, 1);
> +		mdelay(20);
> +	}
> +
> +	return reset_assert(&priv->apps_reset);
> +}
> +
> +static int imx8_pcie_clk_enable(struct pcie_dw_imx8 *priv)
> +{
> +	int ret;
> +
> +	ret = clk_enable(&priv->pcie);

Can you use clk_bulk functions here ?

[...]

> +static int imx8_pcie_deassert_core_reset(struct pcie_dw_imx8 *priv)
> +{
> +	if (dm_gpio_is_valid(&priv->reset_gpio)) {

Invert the match here and return early to reduce indent.

> +		mdelay(100);
> +		dm_gpio_set_value(&priv->reset_gpio, 0);
> +		/* Wait for 100ms after PERST# deassertion (PCIe r5.0, 6.6.1) */
> +		mdelay(100);
> +	}
> +
> +	return 0;
> +}
> +
> +static int pcie_dw_imx8_probe(struct udevice *dev)
> +{
> +	struct pcie_dw_imx8 *priv = dev_get_priv(dev);
> +	struct udevice *ctlr = pci_get_controller(dev);
> +	struct pci_controller *hose = dev_get_uclass_priv(ctlr);
> +	int ret;
> +
> +	ret = imx8_pcie_assert_core_reset(priv);
> +	if (ret) {
> +		dev_err(dev, "failed to assert core reset\n");
> +		return ret;
> +	}
> +
> +	ret = imx8_pcie_clk_enable(priv);
> +	if (ret) {
> +		dev_err(dev, "failed to enable clocks\n");

This needs a fail path, else reset remains released on failure here .

> +		return ret;
> +	}
> +
> +	generic_phy_init(&priv->phy);
> +	generic_phy_power_on(&priv->phy);
> +
> +	ret = imx8_pcie_deassert_core_reset(priv);
> +	if (ret) {
> +		dev_err(dev, "failed to assert core reset\n");
> +		return ret;
> +	}
> +
> +	priv->dw.first_busno = dev_seq(dev);
> +	priv->dw.dev = dev;
> +
> +	pcie_dw_setup_host(&priv->dw);
> +
> +	if (!pcie_link_up(priv, LINK_SPEED_GEN_1)) {
> +		printf("PCIE-%d: Link down\n", dev_seq(dev));
> +		return -ENODEV;
> +	}
> +
> +	printf("PCIE-%d: Link up (Gen%d-x%d, Bus%d)\n", dev_seq(dev),
> +	       pcie_dw_get_link_speed(&priv->dw),
> +	       pcie_dw_get_link_width(&priv->dw),
> +	       hose->first_busno);
> +
> +	pcie_dw_prog_outbound_atu_unroll(&priv->dw, PCIE_ATU_REGION_INDEX0,
> +					 PCIE_ATU_TYPE_MEM,
> +					 priv->dw.mem.phys_start,
> +					 priv->dw.mem.bus_start, priv->dw.mem.size);
> +
> +	return 0;
> +}
> +
> +static int pcie_dw_imx8_remove(struct udevice *dev)
> +{
> +	struct pcie_dw_imx8 *priv = dev_get_priv(dev);
> +
> +	imx8_pcie_assert_core_reset(priv);
> +
> +	return 0;
> +}

[...]
Sumit Garg Feb. 21, 2024, 6:25 a.m. UTC | #2 
On Tue, 20 Feb 2024 at 21:02, Marek Vasut <marex@denx.de> wrote:
>
> On 2/20/24 14:10, Sumit Garg wrote:
> > pcie_imx doesn't seem to share any useful code for iMX8 SoC and it is
> > tied to quite old port of pcie_designware driver from Linux which
> > suffices only iMX6 specific needs.
> >
> > But currently we have the common DWC specific bits which alligns pretty
> > well with DW PCIe controller on iMX8MP SoC. So lets reuse those common
> > bits instead as a new driver for iMX8 SoCs. It should be fairly easy to
> > add support for other iMX8 variants to this driver.
> >
> > iMX8MP SoC also comes up with standalone PCIe PHY support, so hence we
> > can reuse the generic PHY infrastructure to power on PCIe PHY.
> >
> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > ---
> >   drivers/pci/Kconfig        |   8 +
> >   drivers/pci/Makefile       |   1 +
> >   drivers/pci/pcie_dw_imx8.c | 348 +++++++++++++++++++++++++++++++++++++
> >   3 files changed, 357 insertions(+)
> >   create mode 100644 drivers/pci/pcie_dw_imx8.c
> >
> > diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> > index 463ec47eb92..b7c7922b091 100644
> > --- a/drivers/pci/Kconfig
> > +++ b/drivers/pci/Kconfig
> > @@ -413,4 +413,12 @@ config PCIE_STARFIVE_JH7110
> >         Say Y here if you want to enable PLDA XpressRich PCIe controller
> >         support on StarFive JH7110 SoC.
> >
> > +config PCIE_DW_IMX8
> > +     bool "i.MX8 PCIe support"
> > +     depends on ARCH_IMX8M
> > +     select PCIE_DW_COMMON
> > +     help
> > +       Say Y here if you want to enable DW PCIe controller support on
> > +       iMX8 SoCs.
> > +
> >   endif
> > diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> > index 72ef8b4bc77..cddbb902095 100644
> > --- a/drivers/pci/Makefile
> > +++ b/drivers/pci/Makefile
> > @@ -53,3 +53,4 @@ obj-$(CONFIG_PCIE_UNIPHIER) += pcie_uniphier.o
> >   obj-$(CONFIG_PCIE_XILINX_NWL) += pcie-xilinx-nwl.o
> >   obj-$(CONFIG_PCIE_PLDA_COMMON) += pcie_plda_common.o
> >   obj-$(CONFIG_PCIE_STARFIVE_JH7110) += pcie_starfive_jh7110.o
> > +obj-$(CONFIG_PCIE_DW_IMX8) += pcie_dw_imx8.o
> > diff --git a/drivers/pci/pcie_dw_imx8.c b/drivers/pci/pcie_dw_imx8.c
> > new file mode 100644
> > index 00000000000..b9921644765
> > --- /dev/null
> > +++ b/drivers/pci/pcie_dw_imx8.c
> > @@ -0,0 +1,348 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2024 Linaro Ltd.
> > + *
> > + * Author: Sumit Garg <sumit.garg@linaro.org>
> > + */
> > +
> > +#include <asm/io.h>
> > +#include <asm-generic/gpio.h>
> > +#include <clk.h>
> > +#include <dm.h>
> > +#include <dm/device_compat.h>
> > +#include <generic-phy.h>
> > +#include <linux/bitops.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <log.h>
> > +#include <pci.h>
> > +#include <regmap.h>
> > +#include <reset.h>
> > +#include <syscon.h>
> > +#include <time.h>
> > +
> > +#include "pcie_dw_common.h"
> > +
> > +#define PCIE_LINK_CAPABILITY         0x7c
> > +#define TARGET_LINK_SPEED_MASK               0xf
> > +#define LINK_SPEED_GEN_1             0x1
> > +#define LINK_SPEED_GEN_2             0x2
> > +#define LINK_SPEED_GEN_3             0x3
> > +
> > +#define PCIE_MISC_CONTROL_1_OFF              0x8bc
> > +#define PCIE_DBI_RO_WR_EN            BIT(0)
> > +
> > +#define PCIE_PORT_DEBUG0                     0x728
> > +#define PCIE_PORT_DEBUG1                     0x72c
> > +#define PCIE_PORT_DEBUG1_LINK_UP             BIT(4)
> > +#define PCIE_PORT_DEBUG1_LINK_IN_TRAINING    BIT(29)
> > +
> > +#define PCIE_LINK_UP_TIMEOUT_MS              100
> > +
> > +#define IOMUXC_GPR14_OFFSET                  0x38
> > +#define IMX8M_GPR_PCIE_CLK_REQ_OVERRIDE_EN   BIT(10)
> > +#define IMX8M_GPR_PCIE_CLK_REQ_OVERRIDE              BIT(11)
> > +
> > +struct pcie_dw_imx8 {
> > +     /* Must be first member of the struct */
> > +     struct pcie_dw                  dw;
> > +     struct regmap                   *iomuxc_gpr;
> > +     struct clk                      pcie;
> > +     struct clk                      pcie_bus;
> > +     struct clk                      pcie_aux;
> > +     struct gpio_desc                reset_gpio;
> > +     struct reset_ctl                apps_reset;
> > +     struct phy                      phy;
> > +};
> > +
> > +static void pcie_dw_configure(struct pcie_dw_imx8 *priv, u32 cap_speed)
> > +{
> > +     u32 val;
> > +
> > +     dw_pcie_dbi_write_enable(&priv->dw, true);
> > +
> > +     val = readl(priv->dw.dbi_base + PCIE_LINK_CAPABILITY);
> > +     val &= ~TARGET_LINK_SPEED_MASK;
> > +     val |= cap_speed;
> > +     writel(val, priv->dw.dbi_base + PCIE_LINK_CAPABILITY);
>
> clrsetbits_le32()

Ack.

>
> > +
> > +     dw_pcie_dbi_write_enable(&priv->dw, false);
> > +}
> > +
> > +static void imx8_pcie_ltssm_enable(struct pcie_dw_imx8 *priv)
> > +{
> > +     reset_deassert(&priv->apps_reset);
> > +}
> > +
> > +static void imx8_pcie_ltssm_disable(struct pcie_dw_imx8 *priv)
> > +{
> > +     reset_assert(&priv->apps_reset);
> > +}
> > +
> > +static int is_link_up(struct pcie_dw_imx8 *priv)
> > +{
> > +     u32 val;
> > +
> > +     val = readl(priv->dw.dbi_base + PCIE_PORT_DEBUG1);
> > +
> > +     return ((val & PCIE_PORT_DEBUG1_LINK_UP) &&
> > +             (!(val & PCIE_PORT_DEBUG1_LINK_IN_TRAINING)));
> > +}
> > +
> > +static int wait_link_up(struct pcie_dw_imx8 *priv)
> > +{
> > +     unsigned long timeout;
> > +
> > +     timeout = get_timer(0) + PCIE_LINK_UP_TIMEOUT_MS;
>
> wait_for_bit() or read_poll_timeout()

They won't appropriately fit here as I would like to add delay in
between consecutive polls too...

>
> > +     while (!is_link_up(priv)) {
> > +             if (get_timer(0) > timeout)
> > +                     return 0;
> > +             mdelay(10);

...here.

> > +     };
> > +
> > +     return 1;
>
> return -ETIMEDOUT ?

Here 0 represents timeout and 1 represents success condition.

>
> > +}
> > +
> > +static int pcie_link_up(struct pcie_dw_imx8 *priv, u32 cap_speed)
> > +{
> > +     if (is_link_up(priv)) {
> > +             printf("PCI Link already up before configuration!\n");
> > +             return 1;
> > +     }
> > +
> > +     /* DW pre link configurations */
> > +     pcie_dw_configure(priv, cap_speed);
> > +
> > +     /* Initiate link training */
> > +     imx8_pcie_ltssm_enable(priv);
> > +
> > +     /* Check that link was established */
> > +     if (!wait_link_up(priv)) {
> > +             imx8_pcie_ltssm_disable(priv);
> > +             return 0;
> > +     }
> > +
> > +     return 1;
> > +}
> > +
> > +static int imx8_pcie_assert_core_reset(struct pcie_dw_imx8 *priv)
> > +{
> > +     if (dm_gpio_is_valid(&priv->reset_gpio)) {
> > +             dm_gpio_set_value(&priv->reset_gpio, 1);
> > +             mdelay(20);
> > +     }
> > +
> > +     return reset_assert(&priv->apps_reset);
> > +}
> > +
> > +static int imx8_pcie_clk_enable(struct pcie_dw_imx8 *priv)
> > +{
> > +     int ret;
> > +
> > +     ret = clk_enable(&priv->pcie);
>
> Can you use clk_bulk functions here ?
>

Sure, I can try to use them.

> [...]
>
> > +static int imx8_pcie_deassert_core_reset(struct pcie_dw_imx8 *priv)
> > +{
> > +     if (dm_gpio_is_valid(&priv->reset_gpio)) {
>
> Invert the match here and return early to reduce indent.

Ack.

>
> > +             mdelay(100);
> > +             dm_gpio_set_value(&priv->reset_gpio, 0);
> > +             /* Wait for 100ms after PERST# deassertion (PCIe r5.0, 6.6.1) */
> > +             mdelay(100);
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int pcie_dw_imx8_probe(struct udevice *dev)
> > +{
> > +     struct pcie_dw_imx8 *priv = dev_get_priv(dev);
> > +     struct udevice *ctlr = pci_get_controller(dev);
> > +     struct pci_controller *hose = dev_get_uclass_priv(ctlr);
> > +     int ret;
> > +
> > +     ret = imx8_pcie_assert_core_reset(priv);
> > +     if (ret) {
> > +             dev_err(dev, "failed to assert core reset\n");
> > +             return ret;
> > +     }
> > +
> > +     ret = imx8_pcie_clk_enable(priv);
> > +     if (ret) {
> > +             dev_err(dev, "failed to enable clocks\n");
>
> This needs a fail path, else reset remains released on failure here .
>

Ack, I will add that.

-Sumit

> > +             return ret;
> > +     }
> > +
> > +     generic_phy_init(&priv->phy);
> > +     generic_phy_power_on(&priv->phy);
> > +
> > +     ret = imx8_pcie_deassert_core_reset(priv);
> > +     if (ret) {
> > +             dev_err(dev, "failed to assert core reset\n");
> > +             return ret;
> > +     }
> > +
> > +     priv->dw.first_busno = dev_seq(dev);
> > +     priv->dw.dev = dev;
> > +
> > +     pcie_dw_setup_host(&priv->dw);
> > +
> > +     if (!pcie_link_up(priv, LINK_SPEED_GEN_1)) {
> > +             printf("PCIE-%d: Link down\n", dev_seq(dev));
> > +             return -ENODEV;
> > +     }
> > +
> > +     printf("PCIE-%d: Link up (Gen%d-x%d, Bus%d)\n", dev_seq(dev),
> > +            pcie_dw_get_link_speed(&priv->dw),
> > +            pcie_dw_get_link_width(&priv->dw),
> > +            hose->first_busno);
> > +
> > +     pcie_dw_prog_outbound_atu_unroll(&priv->dw, PCIE_ATU_REGION_INDEX0,
> > +                                      PCIE_ATU_TYPE_MEM,
> > +                                      priv->dw.mem.phys_start,
> > +                                      priv->dw.mem.bus_start, priv->dw.mem.size);
> > +
> > +     return 0;
> > +}
> > +
> > +static int pcie_dw_imx8_remove(struct udevice *dev)
> > +{
> > +     struct pcie_dw_imx8 *priv = dev_get_priv(dev);
> > +
> > +     imx8_pcie_assert_core_reset(priv);
> > +
> > +     return 0;
> > +}
>
> [...]
Marek Vasut Feb. 21, 2024, 9:44 a.m. UTC | #3 
On 2/21/24 07:25, Sumit Garg wrote:

[...]

>>> +static int wait_link_up(struct pcie_dw_imx8 *priv)
>>> +{
>>> +     unsigned long timeout;
>>> +
>>> +     timeout = get_timer(0) + PCIE_LINK_UP_TIMEOUT_MS;
>>
>> wait_for_bit() or read_poll_timeout()
> 
> They won't appropriately fit here as I would like to add delay in
> between consecutive polls too...
> 
>>
>>> +     while (!is_link_up(priv)) {
>>> +             if (get_timer(0) > timeout)
>>> +                     return 0;
>>> +             mdelay(10);
> 
> ...here.

include/linux/iopoll.h:#define read_poll_timeout(op, val, cond, 
sleep_us, timeout_us, args...)  \

That's the "sleep_us" , is it not ?

>>> +     };
>>> +
>>> +     return 1;
>>
>> return -ETIMEDOUT ?
> 
> Here 0 represents timeout and 1 represents success condition.

0 generally represents success, non-zero error, please fix.

[...]
diff mbox series

Patch

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 463ec47eb92..b7c7922b091 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -413,4 +413,12 @@  config PCIE_STARFIVE_JH7110
 	  Say Y here if you want to enable PLDA XpressRich PCIe controller
 	  support on StarFive JH7110 SoC.
 
+config PCIE_DW_IMX8
+	bool "i.MX8 PCIe support"
+	depends on ARCH_IMX8M
+	select PCIE_DW_COMMON
+	help
+	  Say Y here if you want to enable DW PCIe controller support on
+	  iMX8 SoCs.
+
 endif
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index 72ef8b4bc77..cddbb902095 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -53,3 +53,4 @@  obj-$(CONFIG_PCIE_UNIPHIER) += pcie_uniphier.o
 obj-$(CONFIG_PCIE_XILINX_NWL) += pcie-xilinx-nwl.o
 obj-$(CONFIG_PCIE_PLDA_COMMON) += pcie_plda_common.o
 obj-$(CONFIG_PCIE_STARFIVE_JH7110) += pcie_starfive_jh7110.o
+obj-$(CONFIG_PCIE_DW_IMX8) += pcie_dw_imx8.o
diff --git a/drivers/pci/pcie_dw_imx8.c b/drivers/pci/pcie_dw_imx8.c
new file mode 100644
index 00000000000..b9921644765
--- /dev/null
+++ b/drivers/pci/pcie_dw_imx8.c
@@ -0,0 +1,348 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2024 Linaro Ltd.
+ *
+ * Author: Sumit Garg <sumit.garg@linaro.org>
+ */
+
+#include <asm/io.h>
+#include <asm-generic/gpio.h>
+#include <clk.h>
+#include <dm.h>
+#include <dm/device_compat.h>
+#include <generic-phy.h>
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <log.h>
+#include <pci.h>
+#include <regmap.h>
+#include <reset.h>
+#include <syscon.h>
+#include <time.h>
+
+#include "pcie_dw_common.h"
+
+#define PCIE_LINK_CAPABILITY		0x7c
+#define TARGET_LINK_SPEED_MASK		0xf
+#define LINK_SPEED_GEN_1		0x1
+#define LINK_SPEED_GEN_2		0x2
+#define LINK_SPEED_GEN_3		0x3
+
+#define PCIE_MISC_CONTROL_1_OFF		0x8bc
+#define PCIE_DBI_RO_WR_EN		BIT(0)
+
+#define PCIE_PORT_DEBUG0			0x728
+#define PCIE_PORT_DEBUG1			0x72c
+#define PCIE_PORT_DEBUG1_LINK_UP		BIT(4)
+#define PCIE_PORT_DEBUG1_LINK_IN_TRAINING	BIT(29)
+
+#define PCIE_LINK_UP_TIMEOUT_MS		100
+
+#define IOMUXC_GPR14_OFFSET			0x38
+#define IMX8M_GPR_PCIE_CLK_REQ_OVERRIDE_EN	BIT(10)
+#define IMX8M_GPR_PCIE_CLK_REQ_OVERRIDE		BIT(11)
+
+struct pcie_dw_imx8 {
+	/* Must be first member of the struct */
+	struct pcie_dw			dw;
+	struct regmap			*iomuxc_gpr;
+	struct clk			pcie;
+	struct clk			pcie_bus;
+	struct clk			pcie_aux;
+	struct gpio_desc		reset_gpio;
+	struct reset_ctl		apps_reset;
+	struct phy			phy;
+};
+
+static void pcie_dw_configure(struct pcie_dw_imx8 *priv, u32 cap_speed)
+{
+	u32 val;
+
+	dw_pcie_dbi_write_enable(&priv->dw, true);
+
+	val = readl(priv->dw.dbi_base + PCIE_LINK_CAPABILITY);
+	val &= ~TARGET_LINK_SPEED_MASK;
+	val |= cap_speed;
+	writel(val, priv->dw.dbi_base + PCIE_LINK_CAPABILITY);
+
+	dw_pcie_dbi_write_enable(&priv->dw, false);
+}
+
+static void imx8_pcie_ltssm_enable(struct pcie_dw_imx8 *priv)
+{
+	reset_deassert(&priv->apps_reset);
+}
+
+static void imx8_pcie_ltssm_disable(struct pcie_dw_imx8 *priv)
+{
+	reset_assert(&priv->apps_reset);
+}
+
+static int is_link_up(struct pcie_dw_imx8 *priv)
+{
+	u32 val;
+
+	val = readl(priv->dw.dbi_base + PCIE_PORT_DEBUG1);
+
+	return ((val & PCIE_PORT_DEBUG1_LINK_UP) &&
+		(!(val & PCIE_PORT_DEBUG1_LINK_IN_TRAINING)));
+}
+
+static int wait_link_up(struct pcie_dw_imx8 *priv)
+{
+	unsigned long timeout;
+
+	timeout = get_timer(0) + PCIE_LINK_UP_TIMEOUT_MS;
+	while (!is_link_up(priv)) {
+		if (get_timer(0) > timeout)
+			return 0;
+		mdelay(10);
+	};
+
+	return 1;
+}
+
+static int pcie_link_up(struct pcie_dw_imx8 *priv, u32 cap_speed)
+{
+	if (is_link_up(priv)) {
+		printf("PCI Link already up before configuration!\n");
+		return 1;
+	}
+
+	/* DW pre link configurations */
+	pcie_dw_configure(priv, cap_speed);
+
+	/* Initiate link training */
+	imx8_pcie_ltssm_enable(priv);
+
+	/* Check that link was established */
+	if (!wait_link_up(priv)) {
+		imx8_pcie_ltssm_disable(priv);
+		return 0;
+	}
+
+	return 1;
+}
+
+static int imx8_pcie_assert_core_reset(struct pcie_dw_imx8 *priv)
+{
+	if (dm_gpio_is_valid(&priv->reset_gpio)) {
+		dm_gpio_set_value(&priv->reset_gpio, 1);
+		mdelay(20);
+	}
+
+	return reset_assert(&priv->apps_reset);
+}
+
+static int imx8_pcie_clk_enable(struct pcie_dw_imx8 *priv)
+{
+	int ret;
+
+	ret = clk_enable(&priv->pcie);
+	if (ret) {
+		printf("unable to enable pcie clock\n");
+		return ret;
+	}
+
+	ret = clk_enable(&priv->pcie_bus);
+	if (ret) {
+		printf("unable to enable pcie_bus clock\n");
+		goto err_pcie;
+	}
+
+	ret = clk_enable(&priv->pcie_aux);
+	if (ret) {
+		printf("unable to enable pcie_aux clock\n");
+		goto err_pcie_bus;
+	}
+
+	/*
+	 * Set the over ride low and enabled make sure that
+	 * REF_CLK is turned on.
+	 */
+	regmap_update_bits(priv->iomuxc_gpr, IOMUXC_GPR14_OFFSET,
+			   IMX8M_GPR_PCIE_CLK_REQ_OVERRIDE, 0);
+	regmap_update_bits(priv->iomuxc_gpr, IOMUXC_GPR14_OFFSET,
+			   IMX8M_GPR_PCIE_CLK_REQ_OVERRIDE_EN,
+			   IMX8M_GPR_PCIE_CLK_REQ_OVERRIDE_EN);
+
+	/* allow the clocks to stabilize */
+	udelay(500);
+
+	return 0;
+
+err_pcie_bus:
+	clk_disable(&priv->pcie_bus);
+err_pcie:
+	clk_disable(&priv->pcie);
+
+	return ret;
+}
+
+static int imx8_pcie_deassert_core_reset(struct pcie_dw_imx8 *priv)
+{
+	if (dm_gpio_is_valid(&priv->reset_gpio)) {
+		mdelay(100);
+		dm_gpio_set_value(&priv->reset_gpio, 0);
+		/* Wait for 100ms after PERST# deassertion (PCIe r5.0, 6.6.1) */
+		mdelay(100);
+	}
+
+	return 0;
+}
+
+static int pcie_dw_imx8_probe(struct udevice *dev)
+{
+	struct pcie_dw_imx8 *priv = dev_get_priv(dev);
+	struct udevice *ctlr = pci_get_controller(dev);
+	struct pci_controller *hose = dev_get_uclass_priv(ctlr);
+	int ret;
+
+	ret = imx8_pcie_assert_core_reset(priv);
+	if (ret) {
+		dev_err(dev, "failed to assert core reset\n");
+		return ret;
+	}
+
+	ret = imx8_pcie_clk_enable(priv);
+	if (ret) {
+		dev_err(dev, "failed to enable clocks\n");
+		return ret;
+	}
+
+	generic_phy_init(&priv->phy);
+	generic_phy_power_on(&priv->phy);
+
+	ret = imx8_pcie_deassert_core_reset(priv);
+	if (ret) {
+		dev_err(dev, "failed to assert core reset\n");
+		return ret;
+	}
+
+	priv->dw.first_busno = dev_seq(dev);
+	priv->dw.dev = dev;
+
+	pcie_dw_setup_host(&priv->dw);
+
+	if (!pcie_link_up(priv, LINK_SPEED_GEN_1)) {
+		printf("PCIE-%d: Link down\n", dev_seq(dev));
+		return -ENODEV;
+	}
+
+	printf("PCIE-%d: Link up (Gen%d-x%d, Bus%d)\n", dev_seq(dev),
+	       pcie_dw_get_link_speed(&priv->dw),
+	       pcie_dw_get_link_width(&priv->dw),
+	       hose->first_busno);
+
+	pcie_dw_prog_outbound_atu_unroll(&priv->dw, PCIE_ATU_REGION_INDEX0,
+					 PCIE_ATU_TYPE_MEM,
+					 priv->dw.mem.phys_start,
+					 priv->dw.mem.bus_start, priv->dw.mem.size);
+
+	return 0;
+}
+
+static int pcie_dw_imx8_remove(struct udevice *dev)
+{
+	struct pcie_dw_imx8 *priv = dev_get_priv(dev);
+
+	imx8_pcie_assert_core_reset(priv);
+
+	return 0;
+}
+
+static int pcie_dw_imx8_of_to_plat(struct udevice *dev)
+{
+	struct pcie_dw_imx8 *priv = dev_get_priv(dev);
+	ofnode gpr;
+	int ret;
+
+	/* Get the controller base address */
+	priv->dw.dbi_base = (void *)dev_read_addr_name(dev, "dbi");
+	if ((fdt_addr_t)priv->dw.dbi_base == FDT_ADDR_T_NONE) {
+		dev_err(dev, "failed to get dbi_base address\n");
+		return -EINVAL;
+	}
+
+	/* Get the config space base address and size */
+	priv->dw.cfg_base = (void *)dev_read_addr_size_name(dev, "config",
+							    &priv->dw.cfg_size);
+	if ((fdt_addr_t)priv->dw.cfg_base == FDT_ADDR_T_NONE) {
+		dev_err(dev, "failed to get cfg_base address\n");
+		return -EINVAL;
+	}
+
+	ret = clk_get_by_name(dev, "pcie_bus", &priv->pcie_bus);
+	if (ret) {
+		dev_err(dev, "failed to get pcie_bus clk\n");
+		return ret;
+	}
+
+	ret = clk_get_by_name(dev, "pcie", &priv->pcie);
+	if (ret) {
+		dev_err(dev, "failed to get pcie clk\n");
+		return ret;
+	}
+
+	ret = clk_get_by_name(dev, "pcie", &priv->pcie_aux);
+	if (ret) {
+		dev_err(dev, "failed to get pcie clk\n");
+		return ret;
+	}
+
+	ret = reset_get_by_name(dev, "apps", &priv->apps_reset);
+	if (ret) {
+		dev_err(dev,
+			"Failed to get PCIe apps reset control\n");
+		return ret;
+	}
+
+	ret = gpio_request_by_name(dev, "reset-gpio", 0, &priv->reset_gpio,
+				   (GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE));
+	if (ret) {
+		dev_err(dev, "unable to get reset-gpio\n");
+		return ret;
+	}
+
+	ret = generic_phy_get_by_name(dev, "pcie-phy", &priv->phy);
+	if (ret) {
+		dev_err(dev, "failed to get pcie phy\n");
+		return ret;
+	}
+
+	gpr = ofnode_by_compatible(ofnode_null(), "fsl,imx8mp-iomuxc-gpr");
+	if (ofnode_equal(gpr, ofnode_null())) {
+		dev_err(dev, "unable to find GPR node\n");
+		return -ENODEV;
+	}
+
+	priv->iomuxc_gpr = syscon_node_to_regmap(gpr);
+	if (IS_ERR(priv->iomuxc_gpr)) {
+		dev_err(dev, "unable to find iomuxc registers\n");
+		return PTR_ERR(priv->iomuxc_gpr);
+	}
+
+	return 0;
+}
+
+static const struct dm_pci_ops pcie_dw_imx8_ops = {
+	.read_config	= pcie_dw_read_config,
+	.write_config	= pcie_dw_write_config,
+};
+
+static const struct udevice_id pcie_dw_imx8_ids[] = {
+	{ .compatible = "fsl,imx8mp-pcie" },
+	{ }
+};
+
+U_BOOT_DRIVER(pcie_dw_imx8) = {
+	.name		= "pcie_dw_imx8",
+	.id		= UCLASS_PCI,
+	.of_match	= pcie_dw_imx8_ids,
+	.ops		= &pcie_dw_imx8_ops,
+	.of_to_plat	= pcie_dw_imx8_of_to_plat,
+	.probe		= pcie_dw_imx8_probe,
+	.remove		= pcie_dw_imx8_remove,
+	.priv_auto	= sizeof(struct pcie_dw_imx8),
+};