2016-04-20 15:45:52

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH v3 1/3] PCI: imx6: Use enum instead of bool for variant indicator

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 <[email protected]>
---

Changes since v2:
- Removed default clause in all introduced switch statements
- Switched to using of_device_get_match_data instead of
explicit of_device_is_compatible checks
- Added a mention of the new DT compatibility string in the documentation

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 | 124 ++++++++++++++++++++++++--------------------
1 file changed, 67 insertions(+), 57 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 0f6d630..a77e7fd 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -19,6 +19,7 @@
#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
#include <linux/module.h>
#include <linux/of_gpio.h>
+#include <linux/of_device.h>
#include <linux/pci.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
@@ -31,6 +32,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 +45,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 +244,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 +253,76 @@ 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;
}

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

- /* 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 +364,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 +385,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 +595,8 @@ 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");
+ imx6_pcie->variant =
+ (enum imx6_pcie_variants)of_device_get_match_data(&pdev->dev);

/* Added for PCI abort handling */
hook_fault_code(16 + 6, imx6q_pcie_abort_handler, SIGBUS, 0,
@@ -623,7 +633,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)) {
@@ -679,8 +689,8 @@ 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,imx6q-pcie", .data = (void *)IMX6Q, },
+ { .compatible = "fsl,imx6sx-pcie", .data = (void *)IMX6SX, },
{},
};
MODULE_DEVICE_TABLE(of, imx6_pcie_of_match);
--
2.5.5


2016-04-20 15:46:00

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH v3 3/3] ARM: dts: imx6qp: Specify imx6qp version of PCIe core

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

Signed-off-by: Andrey Smirnov <[email protected]>
---


Changes since v2:
- Removed default clause in all introduced switch statements
- Switched to using of_device_get_match_data instead of
explicit of_device_is_compatible checks
- Added a mention of the new DT compatibility string in the documentation

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

2016-04-20 15:45:55

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH v3 2/3] PCI: imx6: Implement reset sequence for i.MX6+

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 <[email protected]>
---

Changes since v2:
- Removed default clause in all introduced switch statements
- Switched to using of_device_get_match_data instead of
explicit of_device_is_compatible checks
- Added a mention of the new DT compatibility string in the documentation

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

.../devicetree/bindings/pci/fsl,imx6q-pcie.txt | 2 +-
drivers/pci/host/pci-imx6.c | 23 ++++++++++++++++++++--
include/linux/mfd/syscon/imx6q-iomuxc-gpr.h | 1 +
3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
index 0561e88..4210fe1 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
@@ -4,7 +4,7 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
and thus inherits all the common properties defined in designware-pcie.txt.

Required properties:
-- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie"
+- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
- reg: base address and length of the PCIe controller
- interrupts: A list of interrupt outputs of the controller. Must contain an
entry for each entry in the interrupt-names property.
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index a77e7fd..a47d993 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -34,7 +34,8 @@

enum imx6_pcie_variants {
IMX6Q,
- IMX6SX
+ IMX6SX,
+ IMX6QP,
};

struct imx6_pcie {
@@ -254,6 +255,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
@@ -306,6 +312,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,
@@ -364,9 +371,20 @@ 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;
+ }

return 0;

@@ -691,6 +709,7 @@ static void imx6_pcie_shutdown(struct platform_device *pdev)
static const struct of_device_id imx6_pcie_of_match[] = {
{ .compatible = "fsl,imx6q-pcie", .data = (void *)IMX6Q, },
{ .compatible = "fsl,imx6sx-pcie", .data = (void *)IMX6SX, },
+ { .compatible = "fsl,imx6qp-pcie", .data = (void *)IMX6QP, },
{},
};
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

2016-04-29 01:46:20

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] PCI: imx6: Use enum instead of bool for variant indicator

On Wed, Apr 20, 2016 at 12:45 PM, Andrey Smirnov
<[email protected]> wrote:
> 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 <[email protected]>

Reviewed-by: Fabio Estevam <[email protected]>

2016-04-29 01:47:20

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] PCI: imx6: Implement reset sequence for i.MX6+

On Wed, Apr 20, 2016 at 12:45 PM, Andrey Smirnov
<[email protected]> wrote:
> 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 <[email protected]>

Reviewed-by: Fabio Estevam <[email protected]>

2016-04-29 01:48:06

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] ARM: dts: imx6qp: Specify imx6qp version of PCIe core

On Wed, Apr 20, 2016 at 12:45 PM, Andrey Smirnov
<[email protected]> wrote:
> I.MX6Quad Plus has a slightly different version of PCIe core than
> reqular i.MX6Quad.
>
> Signed-off-by: Andrey Smirnov <[email protected]>

Reviewed-by: Fabio Estevam <[email protected]>

2016-04-29 12:52:37

by Gary Bisson

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] PCI: imx6: Implement reset sequence for i.MX6+

Hi all,

On Fri, Apr 29, 2016 at 3:47 AM, Fabio Estevam <[email protected]> wrote:
> On Wed, Apr 20, 2016 at 12:45 PM, Andrey Smirnov
> <[email protected]> wrote:
>> 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 <[email protected]>
>
> Reviewed-by: Fabio Estevam <[email protected]>

Tested on a Nitrogen6QP_MAX with an Intel WiFi Link 5300 PCIe card.
Everything is working properly (Gen1 speed).
# lspci
00:00.0 Class 0604: 16c3:abcd
01:00.0 Class 0280: 8086:4235

Tested-by: Gary Bisson <[email protected]>

Thanks,
Gary

2016-04-29 12:52:54

by Gary Bisson

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] ARM: dts: imx6qp: Specify imx6qp version of PCIe core

Hi all,

On Fri, Apr 29, 2016 at 3:48 AM, Fabio Estevam <[email protected]> wrote:
> On Wed, Apr 20, 2016 at 12:45 PM, Andrey Smirnov
> <[email protected]> wrote:
>> I.MX6Quad Plus has a slightly different version of PCIe core than
>> reqular i.MX6Quad.
>>
>> Signed-off-by: Andrey Smirnov <[email protected]>
>
> Reviewed-by: Fabio Estevam <[email protected]>

Tested on a Nitrogen6QP_MAX with an Intel WiFi Link 5300 PCIe card.
Everything is working properly (Gen1 speed).
# lspci
00:00.0 Class 0604: 16c3:abcd
01:00.0 Class 0280: 8086:4235

Tested-by: Gary Bisson <[email protected]>

Thanks,
Gary