[PATCH v2 1/3] PCI: imx6: Use enum instead of bool for variant indicator

[PATCH v2 1/3] PCI: imx6: Use enum instead of bool for variant indicator

[Andrey Smirnov]

Use enumerated type instead of a boolean flag to specify the variant of
the PCIe IP block (6Q, 6SX, etc). This patch has zero functional impact,
however it makes the code easier to extend for the case of more than 2
possible variants of an IP block (of which there are).

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---

Changes since v1:

	- Patchset is rebased against
          https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-imx6

	- DTS files changes moved into a separate patch


 drivers/pci/host/pci-imx6.c | 126 +++++++++++++++++++++++++-------------------
 1 file changed, 71 insertions(+), 55 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 0f6d630..c570bbb 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -31,6 +31,11 @@
 
 #define to_imx6_pcie(x)	container_of(x, struct imx6_pcie, pp)
 
+enum imx6_pcie_variants {
+	IMX6Q,
+	IMX6SX
+};
+
 struct imx6_pcie {
 	struct gpio_desc	*reset_gpio;
 	struct clk		*pcie_bus;
@@ -39,7 +44,7 @@ struct imx6_pcie {
 	struct clk		*pcie;
 	struct pcie_port	pp;
 	struct regmap		*iomuxc_gpr;
-	bool			is_imx6sx;
+	enum imx6_pcie_variants variant;
 	void __iomem		*mem_base;
 	u32			tx_deemph_gen1;
 	u32			tx_deemph_gen2_3p5db;
@@ -238,7 +243,8 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp)
 	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
 	u32 val, gpr1, gpr12;
 
-	if (imx6_pcie->is_imx6sx) {
+	switch (imx6_pcie->variant) {
+	case IMX6SX:
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
 				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN);
@@ -246,72 +252,80 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp)
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
 				   IMX6SX_GPR5_PCIE_BTNRST_RESET,
 				   IMX6SX_GPR5_PCIE_BTNRST_RESET);
-		return 0;
-	}
-
-	/*
-	 * If the bootloader already enabled the link we need some special
-	 * handling to get the core back into a state where it is safe to
-	 * touch it for configuration.  As there is no dedicated reset signal
-	 * wired up for MX6QDL, we need to manually force LTSSM into "detect"
-	 * state before completely disabling LTSSM, which is a prerequisite
-	 * for core configuration.
-	 *
-	 * If both LTSSM_ENABLE and REF_SSP_ENABLE are active we have a strong
-	 * indication that the bootloader activated the link.
-	 */
-	regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, &gpr1);
-	regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12, &gpr12);
-
-	if ((gpr1 & IMX6Q_GPR1_PCIE_REF_CLK_EN) &&
-	    (gpr12 & IMX6Q_GPR12_PCIE_CTL_2)) {
-		val = readl(pp->dbi_base + PCIE_PL_PFLR);
-		val &= ~PCIE_PL_PFLR_LINK_STATE_MASK;
-		val |= PCIE_PL_PFLR_FORCE_LINK;
-		writel(val, pp->dbi_base + PCIE_PL_PFLR);
+		break;
+	case IMX6Q:
+		/*
+		 * If the bootloader already enabled the link we need some special
+		 * handling to get the core back into a state where it is safe to
+		 * touch it for configuration.  As there is no dedicated reset signal
+		 * wired up for MX6QDL, we need to manually force LTSSM into "detect"
+		 * state before completely disabling LTSSM, which is a prerequisite
+		 * for core configuration.
+		 *
+		 * If both LTSSM_ENABLE and REF_SSP_ENABLE are active we have a strong
+		 * indication that the bootloader activated the link.
+		 */
+		regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, &gpr1);
+		regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12, &gpr12);
+
+		if ((gpr1 & IMX6Q_GPR1_PCIE_REF_CLK_EN) &&
+		    (gpr12 & IMX6Q_GPR12_PCIE_CTL_2)) {
+			val = readl(pp->dbi_base + PCIE_PL_PFLR);
+			val &= ~PCIE_PL_PFLR_LINK_STATE_MASK;
+			val |= PCIE_PL_PFLR_FORCE_LINK;
+			writel(val, pp->dbi_base + PCIE_PL_PFLR);
+
+			regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+					   IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
+		}
 
-		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-				IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
+				   IMX6Q_GPR1_PCIE_TEST_PD, 1 << 18);
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
+				   IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
+		break;
+	default:
+		BUG();
 	}
 
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
-			IMX6Q_GPR1_PCIE_TEST_PD, 1 << 18);
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
-			IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
-
 	return 0;
 }
 
 static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
 {
 	struct pcie_port *pp = &imx6_pcie->pp;
-	int ret;
+	int ret = 0;
 
-	if (imx6_pcie->is_imx6sx) {
+	switch (imx6_pcie->variant) {
+	case IMX6SX:
 		ret = clk_prepare_enable(imx6_pcie->pcie_inbound_axi);
 		if (ret) {
 			dev_err(pp->dev, "unable to enable pcie_axi clock\n");
-			return ret;
+			break;
 		}
 
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN, 0);
-		return ret;
+		break;
+	case IMX6Q:
+		/* power up core phy and enable ref clock */
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
+				   IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18);
+		/*
+		 * the async reset input need ref clock to sync internally,
+		 * when the ref clock comes after reset, internal synced
+		 * reset time is too short, cannot meet the requirement.
+		 * add one ~10us delay here.
+		 */
+		udelay(10);
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
+				   IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
+		break;
+	default:
+		BUG();
 	}
 
-	/* power up core phy and enable ref clock */
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
-			IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18);
-	/*
-	 * the async reset input need ref clock to sync internally,
-	 * when the ref clock comes after reset, internal synced
-	 * reset time is too short, cannot meet the requirement.
-	 * add one ~10us delay here.
-	 */
-	udelay(10);
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
-			IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
-	return 0;
+	return ret;
 }
 
 static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
@@ -353,7 +367,7 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
 		gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1);
 	}
 
-	if (imx6_pcie->is_imx6sx)
+	if (imx6_pcie->variant == IMX6SX)
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
 				   IMX6SX_GPR5_PCIE_BTNRST_RESET, 0);
 
@@ -374,11 +388,10 @@ static void imx6_pcie_init_phy(struct pcie_port *pp)
 {
 	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
 
-	if (imx6_pcie->is_imx6sx) {
+	if (imx6_pcie->variant == IMX6SX)
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 				   IMX6SX_GPR12_PCIE_RX_EQ_MASK,
 				   IMX6SX_GPR12_PCIE_RX_EQ_2);
-	}
 
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 			IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
@@ -585,8 +598,11 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 	pp = &imx6_pcie->pp;
 	pp->dev = &pdev->dev;
 
-	imx6_pcie->is_imx6sx = of_device_is_compatible(pp->dev->of_node,
-						       "fsl,imx6sx-pcie");
+	if (of_device_is_compatible(pp->dev->of_node,
+				    "fsl,imx6sx-pcie"))
+		imx6_pcie->variant = IMX6SX;
+	else
+		imx6_pcie->variant = IMX6Q;
 
 	/* Added for PCI abort handling */
 	hook_fault_code(16 + 6, imx6q_pcie_abort_handler, SIGBUS, 0,
@@ -623,7 +639,7 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(imx6_pcie->pcie);
 	}
 
-	if (imx6_pcie->is_imx6sx) {
+	if (imx6_pcie->variant == IMX6SX) {
 		imx6_pcie->pcie_inbound_axi = devm_clk_get(&pdev->dev,
 							   "pcie_inbound_axi");
 		if (IS_ERR(imx6_pcie->pcie_inbound_axi)) {
-- 
2.5.5

[PATCH v2 2/3] PCI: imx6: Implement reset sequence for i.MX6+

[Andrey Smirnov]

I.MX6+ has a dedicated bit for reseting PCIe core, which should be used
instead of a regular reset sequence since using the latter will hang the
SoC.

This commit is based on c34068d48273e24d392d9a49a38be807954420ed from
http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---

Changes since v1:

	- Patchset is rebased against
          https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-imx6

	- DTS files changes moved into a separate patch

 drivers/pci/host/pci-imx6.c                 | 28 ++++++++++++++++++++++++++--
 include/linux/mfd/syscon/imx6q-iomuxc-gpr.h |  1 +
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index c570bbb..834c5b8 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -33,7 +33,8 @@
 
 enum imx6_pcie_variants {
 	IMX6Q,
-	IMX6SX
+	IMX6SX,
+	IMX6QP,
 };
 
 struct imx6_pcie {
@@ -253,6 +254,11 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp)
 				   IMX6SX_GPR5_PCIE_BTNRST_RESET,
 				   IMX6SX_GPR5_PCIE_BTNRST_RESET);
 		break;
+	case IMX6QP:
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
+				   IMX6Q_GPR1_PCIE_SW_RST,
+				   IMX6Q_GPR1_PCIE_SW_RST);
+		break;
 	case IMX6Q:
 		/*
 		 * If the bootloader already enabled the link we need some special
@@ -307,6 +313,7 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN, 0);
 		break;
+	case IMX6QP: 		/* FALLTHROUGH */
 	case IMX6Q:
 		/* power up core phy and enable ref clock */
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
@@ -367,9 +374,22 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
 		gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1);
 	}
 
-	if (imx6_pcie->variant == IMX6SX)
+	switch (imx6_pcie->variant) {
+	case IMX6SX:
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
 				   IMX6SX_GPR5_PCIE_BTNRST_RESET, 0);
+		break;
+	case IMX6QP:
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
+				   IMX6Q_GPR1_PCIE_SW_RST, 0);
+
+		usleep_range(200, 500);
+		break;
+	case IMX6Q:		/* Nothing to do */
+		break;
+	default:
+		BUG();
+	}
 
 	return 0;
 
@@ -601,6 +621,9 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 	if (of_device_is_compatible(pp->dev->of_node,
 				    "fsl,imx6sx-pcie"))
 		imx6_pcie->variant = IMX6SX;
+	else if (of_device_is_compatible(pp->dev->of_node,
+					 "fsl,imx6qp-pcie"))
+		imx6_pcie->variant = IMX6QP;
 	else
 		imx6_pcie->variant = IMX6Q;
 
@@ -697,6 +720,7 @@ static void imx6_pcie_shutdown(struct platform_device *pdev)
 static const struct of_device_id imx6_pcie_of_match[] = {
 	{ .compatible = "fsl,imx6q-pcie", },
 	{ .compatible = "fsl,imx6sx-pcie", },
+	{ .compatible = "fsl,imx6qp-pcie", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, imx6_pcie_of_match);
diff --git a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
index 238c8db..5b08e3c 100644
--- a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
+++ b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
@@ -95,6 +95,7 @@
 #define IMX6Q_GPR0_DMAREQ_MUX_SEL0_IOMUX	BIT(0)
 
 #define IMX6Q_GPR1_PCIE_REQ_MASK		(0x3 << 30)
+#define IMX6Q_GPR1_PCIE_SW_RST			BIT(29)
 #define IMX6Q_GPR1_PCIE_EXIT_L1			BIT(28)
 #define IMX6Q_GPR1_PCIE_RDY_L23			BIT(27)
 #define IMX6Q_GPR1_PCIE_ENTER_L1		BIT(26)
-- 
2.5.5

[PATCH v2 3/3] ARM: dts: imx6qp: Specify imx6qp version of PCIe core

[Andrey Smirnov]

I.MX6Quad Plus has a slightly different version of PCIe core than
reqular i.MX6Quad.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---

Changes since v1:

	- Patchset is rebased against
          https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-imx6

	- DTS files changes moved into a separate patch

 arch/arm/boot/dts/imx6qp.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qp.dtsi b/arch/arm/boot/dts/imx6qp.dtsi
index 1ada714..886dbf2 100644
--- a/arch/arm/boot/dts/imx6qp.dtsi
+++ b/arch/arm/boot/dts/imx6qp.dtsi
@@ -82,5 +82,8 @@
 				      "ldb_di0", "ldb_di1", "prg";
 		};
 
+		pcie: pcie@0x01000000 {
+			compatible = "fsl,imx6qp-pcie", "snps,dw-pcie";
+		};
 	};
 };
-- 
2.5.5

Re: [PATCH v2 1/3] PCI: imx6: Use enum instead of bool for variant indicator

[Lucas Stach]

Am Sonntag, den 17.04.2016, 22:51 -0700 schrieb Andrey Smirnov:
> Use enumerated type instead of a boolean flag to specify the variant of
> the PCIe IP block (6Q, 6SX, etc). This patch has zero functional impact,
> however it makes the code easier to extend for the case of more than 2
> possible variants of an IP block (of which there are).
> 
This is the right thing to do, but I'm not yet satisfied with the
implementation.

> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
> 
> Changes since v1:
> 
> 	- Patchset is rebased against
>           https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-imx6
> 
> 	- DTS files changes moved into a separate patch
> 
> 
>  drivers/pci/host/pci-imx6.c | 126 +++++++++++++++++++++++++-------------------
>  1 file changed, 71 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index 0f6d630..c570bbb 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -31,6 +31,11 @@
>  
>  #define to_imx6_pcie(x)	container_of(x, struct imx6_pcie, pp)
>  
> +enum imx6_pcie_variants {
> +	IMX6Q,
> +	IMX6SX
> +};
> +
>  struct imx6_pcie {
>  	struct gpio_desc	*reset_gpio;
>  	struct clk		*pcie_bus;
> @@ -39,7 +44,7 @@ struct imx6_pcie {
>  	struct clk		*pcie;
>  	struct pcie_port	pp;
>  	struct regmap		*iomuxc_gpr;
> -	bool			is_imx6sx;
> +	enum imx6_pcie_variants variant;
>  	void __iomem		*mem_base;
>  	u32			tx_deemph_gen1;
>  	u32			tx_deemph_gen2_3p5db;
> @@ -238,7 +243,8 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp)
>  	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
>  	u32 val, gpr1, gpr12;
>  
> -	if (imx6_pcie->is_imx6sx) {
> +	switch (imx6_pcie->variant) {
> +	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
>  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN);
> @@ -246,72 +252,80 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp)
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
>  				   IMX6SX_GPR5_PCIE_BTNRST_RESET,
>  				   IMX6SX_GPR5_PCIE_BTNRST_RESET);
> -		return 0;
> -	}
> -
> -	/*
> -	 * If the bootloader already enabled the link we need some special
> -	 * handling to get the core back into a state where it is safe to
> -	 * touch it for configuration.  As there is no dedicated reset signal
> -	 * wired up for MX6QDL, we need to manually force LTSSM into "detect"
> -	 * state before completely disabling LTSSM, which is a prerequisite
> -	 * for core configuration.
> -	 *
> -	 * If both LTSSM_ENABLE and REF_SSP_ENABLE are active we have a strong
> -	 * indication that the bootloader activated the link.
> -	 */
> -	regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, &gpr1);
> -	regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12, &gpr12);
> -
> -	if ((gpr1 & IMX6Q_GPR1_PCIE_REF_CLK_EN) &&
> -	    (gpr12 & IMX6Q_GPR12_PCIE_CTL_2)) {
> -		val = readl(pp->dbi_base + PCIE_PL_PFLR);
> -		val &= ~PCIE_PL_PFLR_LINK_STATE_MASK;
> -		val |= PCIE_PL_PFLR_FORCE_LINK;
> -		writel(val, pp->dbi_base + PCIE_PL_PFLR);
> +		break;
> +	case IMX6Q:
> +		/*
> +		 * If the bootloader already enabled the link we need some special
> +		 * handling to get the core back into a state where it is safe to
> +		 * touch it for configuration.  As there is no dedicated reset signal
> +		 * wired up for MX6QDL, we need to manually force LTSSM into "detect"
> +		 * state before completely disabling LTSSM, which is a prerequisite
> +		 * for core configuration.
> +		 *
> +		 * If both LTSSM_ENABLE and REF_SSP_ENABLE are active we have a strong
> +		 * indication that the bootloader activated the link.
> +		 */
> +		regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1, &gpr1);
> +		regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12, &gpr12);
> +
> +		if ((gpr1 & IMX6Q_GPR1_PCIE_REF_CLK_EN) &&
> +		    (gpr12 & IMX6Q_GPR12_PCIE_CTL_2)) {
> +			val = readl(pp->dbi_base + PCIE_PL_PFLR);
> +			val &= ~PCIE_PL_PFLR_LINK_STATE_MASK;
> +			val |= PCIE_PL_PFLR_FORCE_LINK;
> +			writel(val, pp->dbi_base + PCIE_PL_PFLR);
> +
> +			regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +					   IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
> +		}
>  
> -		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -				IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> +				   IMX6Q_GPR1_PCIE_TEST_PD, 1 << 18);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> +				   IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
> +		break;
> +	default:
> +		BUG();

Don't do this. BUG() is always a potential kernel DoS and especially if
it depends on external data like the devicetree.

Please remove the default case, without it the compiler will warn if we
ever forget to update one of those switch statements for another
variant.

>  	}
>  
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> -			IMX6Q_GPR1_PCIE_TEST_PD, 1 << 18);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> -			IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
> -
>  	return 0;
>  }
>  
>  static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
>  {
>  	struct pcie_port *pp = &imx6_pcie->pp;
> -	int ret;
> +	int ret = 0;
>  
> -	if (imx6_pcie->is_imx6sx) {
> +	switch (imx6_pcie->variant) {
> +	case IMX6SX:
>  		ret = clk_prepare_enable(imx6_pcie->pcie_inbound_axi);
>  		if (ret) {
>  			dev_err(pp->dev, "unable to enable pcie_axi clock\n");
> -			return ret;
> +			break;
>  		}
>  
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN, 0);
> -		return ret;
> +		break;
> +	case IMX6Q:
> +		/* power up core phy and enable ref clock */
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> +				   IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18);
> +		/*
> +		 * the async reset input need ref clock to sync internally,
> +		 * when the ref clock comes after reset, internal synced
> +		 * reset time is too short, cannot meet the requirement.
> +		 * add one ~10us delay here.
> +		 */
> +		udelay(10);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> +				   IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
> +		break;
> +	default:
> +		BUG();

Same comment as above.

>  	}
>  
> -	/* power up core phy and enable ref clock */
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> -			IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18);
> -	/*
> -	 * the async reset input need ref clock to sync internally,
> -	 * when the ref clock comes after reset, internal synced
> -	 * reset time is too short, cannot meet the requirement.
> -	 * add one ~10us delay here.
> -	 */
> -	udelay(10);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> -			IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
> -	return 0;
> +	return ret;
>  }
>  
>  static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
> @@ -353,7 +367,7 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
>  		gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1);
>  	}
>  
> -	if (imx6_pcie->is_imx6sx)
> +	if (imx6_pcie->variant == IMX6SX)
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
>  				   IMX6SX_GPR5_PCIE_BTNRST_RESET, 0);
>  
> @@ -374,11 +388,10 @@ static void imx6_pcie_init_phy(struct pcie_port *pp)
>  {
>  	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
>  
> -	if (imx6_pcie->is_imx6sx) {
> +	if (imx6_pcie->variant == IMX6SX)
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_RX_EQ_MASK,
>  				   IMX6SX_GPR12_PCIE_RX_EQ_2);
> -	}
>  
>  	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  			IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
> @@ -585,8 +598,11 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
>  	pp = &imx6_pcie->pp;
>  	pp->dev = &pdev->dev;
>  
> -	imx6_pcie->is_imx6sx = of_device_is_compatible(pp->dev->of_node,
> -						       "fsl,imx6sx-pcie");
> +	if (of_device_is_compatible(pp->dev->of_node,
> +				    "fsl,imx6sx-pcie"))
> +		imx6_pcie->variant = IMX6SX;
> +	else
> +		imx6_pcie->variant = IMX6Q;

Please use of_match_data for this.

>  
>  	/* Added for PCI abort handling */
>  	hook_fault_code(16 + 6, imx6q_pcie_abort_handler, SIGBUS, 0,
> @@ -623,7 +639,7 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
>  		return PTR_ERR(imx6_pcie->pcie);
>  	}
>  
> -	if (imx6_pcie->is_imx6sx) {
> +	if (imx6_pcie->variant == IMX6SX) {
>  		imx6_pcie->pcie_inbound_axi = devm_clk_get(&pdev->dev,
>  							   "pcie_inbound_axi");
>  		if (IS_ERR(imx6_pcie->pcie_inbound_axi)) {

Re: [PATCH v2 2/3] PCI: imx6: Implement reset sequence for i.MX6+

[Lucas Stach]

Am Sonntag, den 17.04.2016, 22:51 -0700 schrieb Andrey Smirnov:
> I.MX6+ has a dedicated bit for reseting PCIe core, which should be used
> instead of a regular reset sequence since using the latter will hang the
> SoC.
> 
> This commit is based on c34068d48273e24d392d9a49a38be807954420ed from
> http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
> 
> Changes since v1:
> 
> 	- Patchset is rebased against
>           https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-imx6
> 
> 	- DTS files changes moved into a separate patch
> 
>  drivers/pci/host/pci-imx6.c                 | 28 ++++++++++++++++++++++++++--
>  include/linux/mfd/syscon/imx6q-iomuxc-gpr.h |  1 +
>  2 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index c570bbb..834c5b8 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -33,7 +33,8 @@
>  
>  enum imx6_pcie_variants {
>  	IMX6Q,
> -	IMX6SX
> +	IMX6SX,
> +	IMX6QP,
>  };
>  
>  struct imx6_pcie {
> @@ -253,6 +254,11 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp)
>  				   IMX6SX_GPR5_PCIE_BTNRST_RESET,
>  				   IMX6SX_GPR5_PCIE_BTNRST_RESET);
>  		break;
> +	case IMX6QP:
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> +				   IMX6Q_GPR1_PCIE_SW_RST,
> +				   IMX6Q_GPR1_PCIE_SW_RST);
> +		break;
>  	case IMX6Q:
>  		/*
>  		 * If the bootloader already enabled the link we need some special
> @@ -307,6 +313,7 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN, 0);
>  		break;
> +	case IMX6QP: 		/* FALLTHROUGH */
>  	case IMX6Q:
>  		/* power up core phy and enable ref clock */
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> @@ -367,9 +374,22 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
>  		gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1);
>  	}
>  
> -	if (imx6_pcie->variant == IMX6SX)
> +	switch (imx6_pcie->variant) {
> +	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
>  				   IMX6SX_GPR5_PCIE_BTNRST_RESET, 0);
> +		break;
> +	case IMX6QP:
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> +				   IMX6Q_GPR1_PCIE_SW_RST, 0);
> +
> +		usleep_range(200, 500);
> +		break;
> +	case IMX6Q:		/* Nothing to do */
> +		break;
> +	default:
> +		BUG();
> +	}
>  
>  	return 0;
>  
> @@ -601,6 +621,9 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
>  	if (of_device_is_compatible(pp->dev->of_node,
>  				    "fsl,imx6sx-pcie"))
>  		imx6_pcie->variant = IMX6SX;
> +	else if (of_device_is_compatible(pp->dev->of_node,
> +					 "fsl,imx6qp-pcie"))
> +		imx6_pcie->variant = IMX6QP;
>  	else
>  		imx6_pcie->variant = IMX6Q;
>  
> @@ -697,6 +720,7 @@ static void imx6_pcie_shutdown(struct platform_device *pdev)
>  static const struct of_device_id imx6_pcie_of_match[] = {
>  	{ .compatible = "fsl,imx6q-pcie", },
>  	{ .compatible = "fsl,imx6sx-pcie", },
> +	{ .compatible = "fsl,imx6qp-pcie", },

New DT compatibles must be documented in
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt.

Otherwise same comments as on the first patch.

>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, imx6_pcie_of_match);
> diff --git a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
> index 238c8db..5b08e3c 100644
> --- a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
> +++ b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
> @@ -95,6 +95,7 @@
>  #define IMX6Q_GPR0_DMAREQ_MUX_SEL0_IOMUX	BIT(0)
>  
>  #define IMX6Q_GPR1_PCIE_REQ_MASK		(0x3 << 30)
> +#define IMX6Q_GPR1_PCIE_SW_RST			BIT(29)
>  #define IMX6Q_GPR1_PCIE_EXIT_L1			BIT(28)
>  #define IMX6Q_GPR1_PCIE_RDY_L23			BIT(27)
>  #define IMX6Q_GPR1_PCIE_ENTER_L1		BIT(26)

Re: [PATCH v2 3/3] ARM: dts: imx6qp: Specify imx6qp version of PCIe core

[Lucas Stach]

Am Sonntag, den 17.04.2016, 22:51 -0700 schrieb Andrey Smirnov:
> I.MX6Quad Plus has a slightly different version of PCIe core than
> reqular i.MX6Quad.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

Under the assumption that the missing documentation for this compatible
is added in patch 2/3 in the next revision of this patchset:

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

> ---
> 
> Changes since v1:
> 
> 	- Patchset is rebased against
>           https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-imx6
> 
> 	- DTS files changes moved into a separate patch
> 
>  arch/arm/boot/dts/imx6qp.dtsi | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6qp.dtsi b/arch/arm/boot/dts/imx6qp.dtsi
> index 1ada714..886dbf2 100644
> --- a/arch/arm/boot/dts/imx6qp.dtsi
> +++ b/arch/arm/boot/dts/imx6qp.dtsi
> @@ -82,5 +82,8 @@
>  				      "ldb_di0", "ldb_di1", "prg";
>  		};
>  
> +		pcie: pcie@0x01000000 {
> +			compatible = "fsl,imx6qp-pcie", "snps,dw-pcie";
> +		};
>  	};
>  };