[PATCH RFC 1/2] PCI: imx6: enable pcie on imx6qdl sabresd and sabreauto

Discussion:

Lucas Stach

2014-09-22 09:36:16 UTC

- enable pcie support on imx6qdl sabresd and asbreauto boards.
- sabresd board has the pcie power on and reset gpios, but
sabreauto doesn't have these two gpios.
---
arch/arm/boot/dts/imx6qdl-sabreauto.dtsi | 4 ++++
arch/arm/boot/dts/imx6qdl-sabresd.dtsi | 2 ++
2 files changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index 009abd6..d6040a5 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
@@ -410,6 +410,10 @@
};
};
+&pcie {
+ status = "okay";
+};
+
&pwm3 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pwm3>;
diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index ec43dde..c2d3224 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -396,6 +396,7 @@
pinctrl_pcie: pciegrp {
fsl,pins = <
+ MX6QDL_PAD_EIM_D19__GPIO3_IO19 0x80000000
MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x80000000

;

};
@@ -502,6 +503,7 @@
&pcie {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pcie>;
+ power-on-gpio = <&gpio3 19 0>;
reset-gpio = <&gpio7 12 0>;
status = "okay";
};

This hunk is wrong. There is no "power-on-gpio" in the binding anymore.
Also there is already a change in Shawns tree to model this as a
always-on regulator. If we really want to control pci bus power this
needs to be done through this regulator, not some arbitrary gpio hack.

Also I don't see why this would be stable material.

Regards,
Lucas

--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |

Richard Zhu

2014-09-22 09:01:38 UTC

Permalink

- imx6sx pcie has its own standalone pcie power supply.
In order to turn on the imx6sx pcie power during
initialization. Add the pcie regulator and the gpc regmap
into the imx6sx pcie structure.
- imx6sx pcie has the new added reset mechanism, add the
reset operations into the initialization.
- another dis_axi clk is mandatory required by imx6sx pcie.
Add one new clk named pcie_sec into imx6_pcie structure.
- pcie_ref_125m is not used as pcie_phy clk anymore on imx6sx.
The parent clk (pcie_ref) of the pcie_bus(lvds1_gate)
is used as pcie_phy clk.
- Register one PM call-back, enter/exit L2 state of the ASPM
during system suspend/resume.
- wait the clocks to stabilize after the pcie_ref_en
(IMX6Q_GPR1_PCIE_REF_CLK_EN) is set.

Signed-off-by: Richard Zhu <***@freescale.com>
---
arch/arm/boot/dts/imx6sx-sdb.dts | 15 ++
arch/arm/boot/dts/imx6sx.dtsi | 33 ++--
arch/arm/mach-imx/Kconfig | 1 +
drivers/pci/host/pci-imx6.c | 228 ++++++++++++++++++++++++----
include/linux/mfd/syscon/imx6q-iomuxc-gpr.h | 14 ++
5 files changed, 249 insertions(+), 42 deletions(-)

diff --git a/arch/arm/boot/dts/imx6sx-sdb.dts b/arch/arm/boot/dts/imx6sx-sdb.dts
index a3980d9..83d0892 100644
--- a/arch/arm/boot/dts/imx6sx-sdb.dts
+++ b/arch/arm/boot/dts/imx6sx-sdb.dts
@@ -251,6 +251,14 @@
};
};

+&pcie {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pcie>;
+ power-on-gpio = <&gpio2 1 0>;
+ reset-gpio = <&gpio2 0 0>;
+ status = "okay";
+};
+
&ssi2 {
status = "okay";
};
@@ -365,6 +373,13 @@

;

};

+ pinctrl_pcie: pciegrp {
+ fsl,pins = <
+ MX6SX_PAD_ENET1_COL__GPIO2_IO_0 0x17059
+ MX6SX_PAD_ENET1_CRS__GPIO2_IO_1 0x17059
+ >;
+ };
+
pinctrl_vcc_sd3: vccsd3grp {
fsl,pins = <
MX6SX_PAD_KEY_COL1__GPIO2_IO_11 0x17059
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index f4b9da6..ec34698 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -689,9 +689,11 @@
};

gpc: ***@020dc000 {
- compatible = "fsl,imx6sx-gpc", "fsl,imx6q-gpc";
+ compatible = "fsl,imx6sx-gpc",
+ "fsl,imx6q-gpc", "syscon";
reg = <0x020dc000 0x4000>;
interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
+ pcie-supply = <&reg_pcie>;
};

iomuxc: ***@020e0000 {
@@ -1188,20 +1190,23 @@
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
- /* configuration space */
- ranges = <0x00000800 0 0x08f00000 0x08f00000 0 0x00080000
- /* downstream I/O */
- 0x81000000 0 0 0x08f80000 0 0x00010000
- /* non-prefetchable memory */
- 0x82000000 0 0x08000000 0x08000000 0 0x00f00000>;
+ ranges = <0x00000800 0 0x01f00000 0x08f00000 0 0x00080000 /* configuration space */
+ 0x81000000 0 0 0x08f80000 0 0x00010000 /* downstream I/O */
+ 0x82000000 0 0x01000000 0x08000000 0 0x00f00000>; /* non-prefetchable memory */
num-lanes = <1>;
- interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&clks IMX6SX_CLK_PCIE_REF_125M>,
- <&clks IMX6SX_CLK_PCIE_AXI>,
- <&clks IMX6SX_CLK_LVDS1_OUT>,
- <&clks IMX6SX_CLK_DISPLAY_AXI>;
- clock-names = "pcie_ref_125m", "pcie_axi",
- "lvds_gate", "display_axi";
+ interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "msi";
+ #interrupt-cells = <1>;
+ interrupt-map-mask = <0 0 0 0x7>;
+ interrupt-map = <0 0 0 1 &intc GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 2 &intc GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 3 &intc GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 4 &intc GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks IMX6SX_CLK_PCIE_AXI>,
+ <&clks IMX6SX_CLK_DISPLAY_AXI>,
+ <&clks IMX6SX_CLK_LVDS1_OUT>;
+ clock-names = "pcie", "pcie_sec", "pcie_bus";
+ pcie-supply = <&reg_pcie>;
status = "disabled";
};
};
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index be9a51a..0a055f0 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -718,6 +718,7 @@ config SOC_IMX6SL

config SOC_IMX6SX
bool "i.MX6 SoloX support"
+ select PCI_DOMAINS if PCI
select PINCTRL_IMX6SX
select SOC_IMX6

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 233fe8a..c9b2f69 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -18,12 +18,16 @@
#include <linux/mfd/syscon.h>
#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
#include <linux/of_gpio.h>
#include <linux/pci.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
#include <linux/resource.h>
#include <linux/signal.h>
+#include <linux/syscore_ops.h>
#include <linux/types.h>
#include <linux/interrupt.h>

@@ -31,15 +35,32 @@

#define to_imx6_pcie(x) container_of(x, struct imx6_pcie, pp)

+/* The pcie who have standalone power domain */
+#define PCIE_PHY_HAS_PWR_DOMAIN BIT(0)
+
+struct imx_pcie_data {
+ unsigned int flags;
+};
+
+static const struct imx_pcie_data imx6sx_pcie_data = {
+ .flags = PCIE_PHY_HAS_PWR_DOMAIN,
+};
+
struct imx6_pcie {
int reset_gpio;
+ int power_on_gpio;
+ const struct imx_pcie_data *data;
struct clk *pcie_bus;
struct clk *pcie_phy;
+ struct clk *pcie_sec;
struct clk *pcie;
struct pcie_port pp;
struct regmap *iomuxc_gpr;
+ struct regmap *gpc_ips_reg;
+ struct regulator *pcie_regulator;
void __iomem *mem_base;
};
+static struct imx6_pcie *imx6_pcie;

/* PCIe Root Complex registers (memory-mapped) */
#define PCIE_RC_LCR 0x7c
@@ -77,6 +98,11 @@ struct imx6_pcie {
#define PHY_RX_OVRD_IN_LO_RX_DATA_EN (1 << 5)
#define PHY_RX_OVRD_IN_LO_RX_PLL_EN (1 << 3)

+static inline bool is_imx6sx_pcie(struct imx6_pcie *imx6_pcie)
+{
+ return imx6_pcie->data == &imx6sx_pcie_data;
+}
+
static int pcie_phy_poll_ack(void __iomem *dbi_base, int exp_val)
{
u32 val;
@@ -257,10 +283,21 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
int ret;

- ret = clk_prepare_enable(imx6_pcie->pcie_phy);
- if (ret) {
- dev_err(pp->dev, "unable to enable pcie_phy clock\n");
- goto err_pcie_phy;
+ if (gpio_is_valid(imx6_pcie->power_on_gpio))
+ gpio_set_value(imx6_pcie->power_on_gpio, 1);
+
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ ret = clk_prepare_enable(imx6_pcie->pcie_sec);
+ if (ret) {
+ dev_err(pp->dev, "unable to enable pcie_sec clk.\n");
+ goto err_pcie_sec;
+ }
+ } else {
+ ret = clk_prepare_enable(imx6_pcie->pcie_phy);
+ if (ret) {
+ dev_err(pp->dev, "unable to enable pcie_phy clock\n");
+ goto err_pcie_phy;
+ }
}

ret = clk_prepare_enable(imx6_pcie->pcie_bus);
@@ -275,28 +312,50 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
goto err_pcie;
}

+ if (is_imx6sx_pcie(imx6_pcie)) {
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ IMX6SX_GPR12_PCIE_TEST_PD,
+ IMX6SX_GPR12_PCIE_TEST_PD_CLR);
+ } else {
+ /* power up core phy and enable ref clock */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
+ IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18);
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
+ IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
+ }
+
/* allow the clocks to stabilize */
usleep_range(200, 500);

- /* power up core phy and enable ref clock */
- regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
- IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18);
- regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
- IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
-
/* Some boards don't have PCIe reset GPIO. */
if (gpio_is_valid(imx6_pcie->reset_gpio)) {
gpio_set_value(imx6_pcie->reset_gpio, 0);
msleep(100);
gpio_set_value(imx6_pcie->reset_gpio, 1);
}
+
+ /*
+ * iMX6SX PCIe has the stand-alone power domain.
+ * refer to the initialization for iMX6SX PCIe,
+ * release the PCIe PHY reset here,
+ * before LTSSM enable is set.
+ */
+ if (is_imx6sx_pcie(imx6_pcie))
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
+ IMX6SX_GPR5_PCIE_BTNRST,
+ IMX6SX_GPR5_PCIE_BTNRST_CLR);
+
return 0;

err_pcie:
clk_disable_unprepare(imx6_pcie->pcie_bus);
err_pcie_bus:
- clk_disable_unprepare(imx6_pcie->pcie_phy);
+ if (!is_imx6sx_pcie(imx6_pcie))
+ clk_disable_unprepare(imx6_pcie->pcie_phy);
err_pcie_phy:
+ if (is_imx6sx_pcie(imx6_pcie))
+ clk_disable_unprepare(imx6_pcie->pcie_sec);
+err_pcie_sec:
return ret;

}
@@ -304,15 +363,38 @@ err_pcie_phy:
static void imx6_pcie_init_phy(struct pcie_port *pp)
{
struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
+ int ret;
+
+ /*
+ * iMX6SX PCIe has the stand-alone power domain
+ * add the initialization here for iMX6SX PCIe.
+ */
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ /* Force PCIe PHY reset */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
+ IMX6SX_GPR5_PCIE_BTNRST,
+ IMX6SX_GPR5_PCIE_BTNRST);
+
+ regmap_update_bits(imx6_pcie->gpc_ips_reg, 0, 1 << 7, 1 << 7);
+ /* Power up PCIe PHY, ANATOP_REG_CORE offset 0x140, bit13-9 */
+ regulator_set_voltage(imx6_pcie->pcie_regulator,
+ 1100000, 1100000);
+ ret = regulator_enable(imx6_pcie->pcie_regulator);
+ if (ret)
+ dev_info(pp->dev, "failed to enable pcie regulator.\n");
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ IMX6SX_GPR12_RX_EQ_MASK, IMX6SX_GPR12_RX_EQ_2);
+ }

regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
- IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
+ IMX6Q_GPR12_PCIE_CTL_2,
+ IMX6Q_GPR12_PCIE_CTL_2_CLR);

/* configure constant input signal to the pcie ctrl and phy */
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
IMX6Q_GPR12_DEVICE_TYPE, PCI_EXP_TYPE_ROOT_PORT << 12);
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
- IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
+ IMX6Q_GPR12_LOS_LEVEL, IMX6Q_GPR12_LOS_LEVEL_9);

regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
IMX6Q_GPR8_TX_DEEMPH_GEN1, 0 << 0);
@@ -370,7 +452,8 @@ static int imx6_pcie_start_link(struct pcie_port *pp)

/* Start LTSSM. */
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
- IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
+ IMX6Q_GPR12_PCIE_CTL_2,
+ IMX6Q_GPR12_PCIE_CTL_2);

ret = imx6_pcie_wait_for_link(pp);
if (ret)
@@ -546,10 +629,73 @@ static int __init imx6_add_pcie_port(struct pcie_port *pp,
return 0;
}

+static const struct of_device_id imx6_pcie_of_match[] = {
+ { .compatible = "fsl,imx6q-pcie", },
+ { .compatible = "fsl,imx6sx-pcie", .data = &imx6sx_pcie_data},
+ {},
+};
+MODULE_DEVICE_TABLE(of, imx6_pcie_of_match);
+
+#ifdef CONFIG_PM_SLEEP
+static int pci_imx_suspend(void)
+{
+ int rc = 0;
+
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ /* PM_TURN_OFF */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ BIT(16), 1 << 16);
+ udelay(10);
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ BIT(16), 0 << 16);
+ }
+
+ return rc;
+}
+
+static void pci_imx_resume(void)
+{
+ struct pcie_port *pp = &imx6_pcie->pp;
+
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ /* reset iMX6SX PCIe */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr,
+ IOMUXC_GPR5, BIT(18), 1 << 18);
+
+ regmap_update_bits(imx6_pcie->iomuxc_gpr,
+ IOMUXC_GPR5, BIT(18), 0 << 18);
+
+ /*
+ * controller maybe turn off, re-configure again
+ * Set the CLASS_REV of RC CFG header to
+ * PCI_CLASS_BRIDGE_PCI
+ */
+ writel(readl(pp->dbi_base + PCI_CLASS_REVISION)
+ | (PCI_CLASS_BRIDGE_PCI << 16),
+ pp->dbi_base + PCI_CLASS_REVISION);
+
+ dw_pcie_setup_rc(pp);
+
+ /* reset iMX6SX PCIe */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr,
+ IOMUXC_GPR5, BIT(18), 1 << 18);
+
+ regmap_update_bits(imx6_pcie->iomuxc_gpr,
+ IOMUXC_GPR5, BIT(18), 0 << 18);
+ }
+}
+
+static struct syscore_ops pci_imx_syscore_ops = {
+ .suspend = pci_imx_suspend,
+ .resume = pci_imx_resume,
+};
+#endif
+
static int __init imx6_pcie_probe(struct platform_device *pdev)
{
- struct imx6_pcie *imx6_pcie;
struct pcie_port *pp;
+ const struct of_device_id *of_id =
+ of_match_device(imx6_pcie_of_match, &pdev->dev);
struct device_node *np = pdev->dev.of_node;
struct resource *dbi_base;
int ret;
@@ -560,6 +706,7 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)

pp = &imx6_pcie->pp;
pp->dev = &pdev->dev;
+ imx6_pcie->data = of_id->data;

/* Added for PCI abort handling */
hook_fault_code(16 + 6, imx6q_pcie_abort_handler, SIGBUS, 0,
@@ -581,12 +728,26 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
}
}

+ imx6_pcie->power_on_gpio = of_get_named_gpio(np, "power-on-gpio", 0);
+ if (gpio_is_valid(imx6_pcie->power_on_gpio)) {
+ ret = devm_gpio_request_one(&pdev->dev,
+ imx6_pcie->power_on_gpio,
+ GPIOF_OUT_INIT_LOW,
+ "PCIe power enable");
+ if (ret) {
+ dev_err(&pdev->dev, "unable to get power-on gpio\n");
+ return ret;
+ }
+ }
+
/* Fetch clocks */
- imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");
- if (IS_ERR(imx6_pcie->pcie_phy)) {
- dev_err(&pdev->dev,
- "pcie_phy clock source missing or invalid\n");
- return PTR_ERR(imx6_pcie->pcie_phy);
+ if (!is_imx6sx_pcie(imx6_pcie)) {
+ imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");
+ if (IS_ERR(imx6_pcie->pcie_phy)) {
+ dev_err(&pdev->dev,
+ "pcie_phy clock source missing or invalid\n");
+ return PTR_ERR(imx6_pcie->pcie_phy);
+ }
}

imx6_pcie->pcie_bus = devm_clk_get(&pdev->dev, "pcie_bus");
@@ -604,8 +765,22 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
}

/* Grab GPR config register range */
- imx6_pcie->iomuxc_gpr =
- syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ /* Get pcie regulator */
+ imx6_pcie->pcie_regulator = devm_regulator_get(pp->dev, "pcie");
+
+ /* Grab GPR config register range */
+ imx6_pcie->iomuxc_gpr =
+ syscon_regmap_lookup_by_compatible
+ ("fsl,imx6sx-iomuxc-gpr");
+ /* Grab GPC IPS register range */
+ imx6_pcie->gpc_ips_reg =
+ syscon_regmap_lookup_by_compatible("fsl,imx6sx-gpc");
+ } else {
+ imx6_pcie->iomuxc_gpr =
+ syscon_regmap_lookup_by_compatible
+ ("fsl,imx6q-iomuxc-gpr");
+ }
if (IS_ERR(imx6_pcie->iomuxc_gpr)) {
dev_err(&pdev->dev, "unable to find iomuxc registers\n");
return PTR_ERR(imx6_pcie->iomuxc_gpr);
@@ -616,6 +791,9 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
return ret;

platform_set_drvdata(pdev, imx6_pcie);
+#ifdef CONFIG_PM_SLEEP
+ register_syscore_ops(&pci_imx_syscore_ops);
+#endif
return 0;
}

@@ -627,12 +805,6 @@ static void imx6_pcie_shutdown(struct platform_device *pdev)
imx6_pcie_assert_core_reset(&imx6_pcie->pp);
}

-static const struct of_device_id imx6_pcie_of_match[] = {
- { .compatible = "fsl,imx6q-pcie", },
- {},
-};
-MODULE_DEVICE_TABLE(of, imx6_pcie_of_match);
-
static struct platform_driver imx6_pcie_driver = {
.driver = {
.name = "imx6q-pcie",
diff --git a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
index ff44374..f02875e 100644
--- a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
+++ b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
@@ -113,10 +113,12 @@
#define IMX6Q_GPR1_MIPI_IPU1_MUX_GASKET 0x0
#define IMX6Q_GPR1_MIPI_IPU1_MUX_IOMUX BIT(19)
#define IMX6Q_GPR1_PCIE_TEST_PD BIT(18)
+#define IMX6Q_GPR1_PCIE_TEST_PD_CLR 0x0
#define IMX6Q_GPR1_IPU_VPU_MUX_MASK BIT(17)
#define IMX6Q_GPR1_IPU_VPU_MUX_IPU1 0x0
#define IMX6Q_GPR1_IPU_VPU_MUX_IPU2 BIT(17)
#define IMX6Q_GPR1_PCIE_REF_CLK_EN BIT(16)
+#define IMX6Q_GPR1_PCIE_REF_CLK_CLR 0x0
#define IMX6Q_GPR1_USB_EXP_MODE BIT(15)
#define IMX6Q_GPR1_PCIE_INT BIT(14)
#define IMX6Q_GPR1_USB_OTG_ID_SEL_MASK BIT(13)
@@ -300,7 +302,9 @@
#define IMX6Q_GPR12_ARMP_APB_CLK_EN BIT(24)
#define IMX6Q_GPR12_DEVICE_TYPE (0xf << 12)
#define IMX6Q_GPR12_PCIE_CTL_2 BIT(10)
+#define IMX6Q_GPR12_PCIE_CTL_2_CLR 0x0
#define IMX6Q_GPR12_LOS_LEVEL (0x1f << 4)
+#define IMX6Q_GPR12_LOS_LEVEL_9 (0x9 << 4)

#define IMX6Q_GPR13_SDMA_STOP_REQ BIT(30)
#define IMX6Q_GPR13_CAN2_STOP_REQ BIT(29)
@@ -395,4 +399,14 @@
#define IMX6SL_GPR1_FEC_CLOCK_MUX1_SEL_MASK (0x3 << 17)
#define IMX6SL_GPR1_FEC_CLOCK_MUX2_SEL_MASK (0x1 << 14)

+/* For imx6sx iomux gpr register field define */
+#define IMX6SX_GPR5_PCIE_BTNRST BIT(19)
+#define IMX6SX_GPR5_PCIE_BTNRST_CLR 0x0
+#define IMX6SX_GPR5_PCIE_PERST BIT(18)
+#define IMX6SX_GPR5_PCIE_PERST_CLR 0x0
+
+#define IMX6SX_GPR12_PCIE_TEST_PD BIT(30)
+#define IMX6SX_GPR12_PCIE_TEST_PD_CLR 0x0
+#define IMX6SX_GPR12_RX_EQ_MASK (0x7 << 0)
+#define IMX6SX_GPR12_RX_EQ_2 (0x2 << 0)
#endif /* __LINUX_IMX6Q_IOMUXC_GPR_H */

--
1.9.1

Lucas Stach

2014-09-22 10:02:16 UTC

Permalink

Hi Richard,

This commit smashes lots of things together. It would be much easier to
review this if the changes were broken out appropriately.

Also why are you sending RFC patches to stable?

A few quick comments below, this isn't a full review yet.

You remove this clock from the binding. I don't see a reason why, just
fill in pcie_ref for both pcie_bus and pcie_phy. No need to implement
different code paths for this.

Post by Richard Zhu
- Register one PM call-back, enter/exit L2 state of the ASPM
during system suspend/resume.
- wait the clocks to stabilize after the pcie_ref_en
(IMX6Q_GPR1_PCIE_REF_CLK_EN) is set.
---
arch/arm/boot/dts/imx6sx-sdb.dts | 15 ++
arch/arm/boot/dts/imx6sx.dtsi | 33 ++--
arch/arm/mach-imx/Kconfig | 1 +
drivers/pci/host/pci-imx6.c | 228 ++++++++++++++++++++++++----
include/linux/mfd/syscon/imx6q-iomuxc-gpr.h | 14 ++
5 files changed, 249 insertions(+), 42 deletions(-)
diff --git a/arch/arm/boot/dts/imx6sx-sdb.dts b/arch/arm/boot/dts/imx6sx-sdb.dts
index a3980d9..83d0892 100644
--- a/arch/arm/boot/dts/imx6sx-sdb.dts
+++ b/arch/arm/boot/dts/imx6sx-sdb.dts
@@ -251,6 +251,14 @@
};
};
+&pcie {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pcie>;
+ power-on-gpio = <&gpio2 1 0>;

No such GPIO in the binding.

Post by Richard Zhu
+ reset-gpio = <&gpio2 0 0>;
+ status = "okay";
+};
+
&ssi2 {
status = "okay";
};
@@ -365,6 +373,13 @@

;

I won't allow this to creep in again. This needs to be a proper
regulator, not some kind of gpio.

Post by Richard Zhu
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ ret = clk_prepare_enable(imx6_pcie->pcie_sec);
+ if (ret) {
+ dev_err(pp->dev, "unable to enable pcie_sec clk.\n");
+ goto err_pcie_sec;
+ }
+ } else {
+ ret = clk_prepare_enable(imx6_pcie->pcie_phy);
+ if (ret) {
+ dev_err(pp->dev, "unable to enable pcie_phy clock\n");
+ goto err_pcie_phy;
+ }
}
ret = clk_prepare_enable(imx6_pcie->pcie_bus);
@@ -275,28 +312,50 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
goto err_pcie;
}
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ IMX6SX_GPR12_PCIE_TEST_PD,
+ IMX6SX_GPR12_PCIE_TEST_PD_CLR);
+ } else {
+ /* power up core phy and enable ref clock */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
+ IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18);
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
+ IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
+ }
+
/* allow the clocks to stabilize */
usleep_range(200, 500);
- /* power up core phy and enable ref clock */
- regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
- IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18);
- regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
- IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
-
/* Some boards don't have PCIe reset GPIO. */
if (gpio_is_valid(imx6_pcie->reset_gpio)) {
gpio_set_value(imx6_pcie->reset_gpio, 0);
msleep(100);
gpio_set_value(imx6_pcie->reset_gpio, 1);
}
+
+ /*
+ * iMX6SX PCIe has the stand-alone power domain.
+ * refer to the initialization for iMX6SX PCIe,
+ * release the PCIe PHY reset here,
+ * before LTSSM enable is set.
+ */
+ if (is_imx6sx_pcie(imx6_pcie))
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
+ IMX6SX_GPR5_PCIE_BTNRST,
+ IMX6SX_GPR5_PCIE_BTNRST_CLR);
+
return 0;
clk_disable_unprepare(imx6_pcie->pcie_bus);
- clk_disable_unprepare(imx6_pcie->pcie_phy);
+ if (!is_imx6sx_pcie(imx6_pcie))
+ clk_disable_unprepare(imx6_pcie->pcie_phy);
+ if (is_imx6sx_pcie(imx6_pcie))
+ clk_disable_unprepare(imx6_pcie->pcie_sec);
return ret;
}
static void imx6_pcie_init_phy(struct pcie_port *pp)
{
struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
+ int ret;
+
+ /*
+ * iMX6SX PCIe has the stand-alone power domain
+ * add the initialization here for iMX6SX PCIe.
+ */
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ /* Force PCIe PHY reset */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
+ IMX6SX_GPR5_PCIE_BTNRST,
+ IMX6SX_GPR5_PCIE_BTNRST);
+
+ regmap_update_bits(imx6_pcie->gpc_ips_reg, 0, 1 << 7, 1 << 7);

What is this? Is this a regulator? If so, why isn't it abstracted as a
proper regulator?

Post by Richard Zhu
+ /* Power up PCIe PHY, ANATOP_REG_CORE offset 0x140, bit13-9 */
+ regulator_set_voltage(imx6_pcie->pcie_regulator,
+ 1100000, 1100000);
+ ret = regulator_enable(imx6_pcie->pcie_regulator);
+ if (ret)
+ dev_info(pp->dev, "failed to enable pcie regulator.\n");

This regulator gets enabled here, but I don't see any path were we would
disable it. Is this a always-on regulator or are you simply missing the
calls to disable it in the runtime-pm hooks?

Post by Richard Zhu
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ IMX6SX_GPR12_RX_EQ_MASK, IMX6SX_GPR12_RX_EQ_2);
+ }
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
- IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
+ IMX6Q_GPR12_PCIE_CTL_2,
+ IMX6Q_GPR12_PCIE_CTL_2_CLR);
/* configure constant input signal to the pcie ctrl and phy */
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
IMX6Q_GPR12_DEVICE_TYPE, PCI_EXP_TYPE_ROOT_PORT << 12);
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
- IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
+ IMX6Q_GPR12_LOS_LEVEL, IMX6Q_GPR12_LOS_LEVEL_9);
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
IMX6Q_GPR8_TX_DEEMPH_GEN1, 0 << 0);
@@ -370,7 +452,8 @@ static int imx6_pcie_start_link(struct pcie_port *pp)
/* Start LTSSM. */
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
- IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
+ IMX6Q_GPR12_PCIE_CTL_2,
+ IMX6Q_GPR12_PCIE_CTL_2);
ret = imx6_pcie_wait_for_link(pp);
if (ret)
@@ -546,10 +629,73 @@ static int __init imx6_add_pcie_port(struct pcie_port *pp,
return 0;
}
+static const struct of_device_id imx6_pcie_of_match[] = {
+ { .compatible = "fsl,imx6q-pcie", },
+ { .compatible = "fsl,imx6sx-pcie", .data = &imx6sx_pcie_data},
+ {},
+};
+MODULE_DEVICE_TABLE(of, imx6_pcie_of_match);
+
+#ifdef CONFIG_PM_SLEEP
+static int pci_imx_suspend(void)
+{
+ int rc = 0;
+
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ /* PM_TURN_OFF */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ BIT(16), 1 << 16);
+ udelay(10);
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ BIT(16), 0 << 16);

Why are there no defines for those bits? I can't really tell what's
going on here. Is this some kind of power gating the SoC partition? In
that case it should really be implemented as a power-domain and not as
some kind of ad-hoc GPR register bashing.

Post by Richard Zhu
+ }
+
+ return rc;
+}
+
+static void pci_imx_resume(void)
+{
+ struct pcie_port *pp = &imx6_pcie->pp;
+
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ /* reset iMX6SX PCIe */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr,
+ IOMUXC_GPR5, BIT(18), 1 << 18);
+
+ regmap_update_bits(imx6_pcie->iomuxc_gpr,
+ IOMUXC_GPR5, BIT(18), 0 << 18);
+
+ /*
+ * controller maybe turn off, re-configure again
+ * Set the CLASS_REV of RC CFG header to
+ * PCI_CLASS_BRIDGE_PCI
+ */
+ writel(readl(pp->dbi_base + PCI_CLASS_REVISION)
+ | (PCI_CLASS_BRIDGE_PCI << 16),
+ pp->dbi_base + PCI_CLASS_REVISION);
+
+ dw_pcie_setup_rc(pp);
+
+ /* reset iMX6SX PCIe */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr,
+ IOMUXC_GPR5, BIT(18), 1 << 18);
+
+ regmap_update_bits(imx6_pcie->iomuxc_gpr,
+ IOMUXC_GPR5, BIT(18), 0 << 18);

Why the double reset?

Post by Richard Zhu
+ }
+}
+
+static struct syscore_ops pci_imx_syscore_ops = {
+ .suspend = pci_imx_suspend,
+ .resume = pci_imx_resume,
+};
+#endif
+
static int __init imx6_pcie_probe(struct platform_device *pdev)
{
- struct imx6_pcie *imx6_pcie;
struct pcie_port *pp;
+ const struct of_device_id *of_id =
+ of_match_device(imx6_pcie_of_match, &pdev->dev);
struct device_node *np = pdev->dev.of_node;
struct resource *dbi_base;
int ret;
@@ -560,6 +706,7 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
pp = &imx6_pcie->pp;
pp->dev = &pdev->dev;
+ imx6_pcie->data = of_id->data;
/* Added for PCI abort handling */
hook_fault_code(16 + 6, imx6q_pcie_abort_handler, SIGBUS, 0,
@@ -581,12 +728,26 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
}
}
+ imx6_pcie->power_on_gpio = of_get_named_gpio(np, "power-on-gpio", 0);
+ if (gpio_is_valid(imx6_pcie->power_on_gpio)) {
+ ret = devm_gpio_request_one(&pdev->dev,
+ imx6_pcie->power_on_gpio,
+ GPIOF_OUT_INIT_LOW,
+ "PCIe power enable");
+ if (ret) {
+ dev_err(&pdev->dev, "unable to get power-on gpio\n");
+ return ret;
+ }
+ }
+
/* Fetch clocks */
- imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");
- if (IS_ERR(imx6_pcie->pcie_phy)) {
- dev_err(&pdev->dev,
- "pcie_phy clock source missing or invalid\n");
- return PTR_ERR(imx6_pcie->pcie_phy);
+ if (!is_imx6sx_pcie(imx6_pcie)) {
+ imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");
+ if (IS_ERR(imx6_pcie->pcie_phy)) {
+ dev_err(&pdev->dev,
+ "pcie_phy clock source missing or invalid\n");
+ return PTR_ERR(imx6_pcie->pcie_phy);
+ }
}

Missing binding update for the changed clock handling. Also what is this
clock pcie_sec? Is it really a second AXI clock? I highly doubt that the
register interface of the core is clocked by two different clocks. What
blocks of the dw_pci core does this clock feed?

Post by Richard Zhu
imx6_pcie->pcie_bus = devm_clk_get(&pdev->dev, "pcie_bus");
@@ -604,8 +765,22 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
}
/* Grab GPR config register range */
- imx6_pcie->iomuxc_gpr =
- syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ /* Get pcie regulator */
+ imx6_pcie->pcie_regulator = devm_regulator_get(pp->dev, "pcie");

Uh, this has nothing to do with the GPR range, so the comment above is
confusing. Also missing binding update for this regulator.

Post by Richard Zhu
+
+ /* Grab GPR config register range */
+ imx6_pcie->iomuxc_gpr =
+ syscon_regmap_lookup_by_compatible
+ ("fsl,imx6sx-iomuxc-gpr");
+ /* Grab GPC IPS register range */
+ imx6_pcie->gpc_ips_reg =
+ syscon_regmap_lookup_by_compatible("fsl,imx6sx-gpc");
+ } else {
+ imx6_pcie->iomuxc_gpr =
+ syscon_regmap_lookup_by_compatible
+ ("fsl,imx6q-iomuxc-gpr");
+ }
if (IS_ERR(imx6_pcie->iomuxc_gpr)) {
dev_err(&pdev->dev, "unable to find iomuxc registers\n");
return PTR_ERR(imx6_pcie->iomuxc_gpr);
@@ -616,6 +791,9 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
return ret;
platform_set_drvdata(pdev, imx6_pcie);
+#ifdef CONFIG_PM_SLEEP
+ register_syscore_ops(&pci_imx_syscore_ops);
+#endif
return 0;
}
@@ -627,12 +805,6 @@ static void imx6_pcie_shutdown(struct platform_device *pdev)
imx6_pcie_assert_core_reset(&imx6_pcie->pp);
}
-static const struct of_device_id imx6_pcie_of_match[] = {
- { .compatible = "fsl,imx6q-pcie", },
- {},
-};
-MODULE_DEVICE_TABLE(of, imx6_pcie_of_match);
-
static struct platform_driver imx6_pcie_driver = {
.driver = {
.name = "imx6q-pcie",
diff --git a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
index ff44374..f02875e 100644
--- a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
+++ b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
@@ -113,10 +113,12 @@
#define IMX6Q_GPR1_MIPI_IPU1_MUX_GASKET 0x0
#define IMX6Q_GPR1_MIPI_IPU1_MUX_IOMUX BIT(19)
#define IMX6Q_GPR1_PCIE_TEST_PD BIT(18)
+#define IMX6Q_GPR1_PCIE_TEST_PD_CLR 0x0
#define IMX6Q_GPR1_IPU_VPU_MUX_MASK BIT(17)
#define IMX6Q_GPR1_IPU_VPU_MUX_IPU1 0x0
#define IMX6Q_GPR1_IPU_VPU_MUX_IPU2 BIT(17)
#define IMX6Q_GPR1_PCIE_REF_CLK_EN BIT(16)
+#define IMX6Q_GPR1_PCIE_REF_CLK_CLR 0x0
#define IMX6Q_GPR1_USB_EXP_MODE BIT(15)
#define IMX6Q_GPR1_PCIE_INT BIT(14)
#define IMX6Q_GPR1_USB_OTG_ID_SEL_MASK BIT(13)
@@ -300,7 +302,9 @@
#define IMX6Q_GPR12_ARMP_APB_CLK_EN BIT(24)
#define IMX6Q_GPR12_DEVICE_TYPE (0xf << 12)
#define IMX6Q_GPR12_PCIE_CTL_2 BIT(10)
+#define IMX6Q_GPR12_PCIE_CTL_2_CLR 0x0
#define IMX6Q_GPR12_LOS_LEVEL (0x1f << 4)
+#define IMX6Q_GPR12_LOS_LEVEL_9 (0x9 << 4)
#define IMX6Q_GPR13_SDMA_STOP_REQ BIT(30)
#define IMX6Q_GPR13_CAN2_STOP_REQ BIT(29)
@@ -395,4 +399,14 @@
#define IMX6SL_GPR1_FEC_CLOCK_MUX1_SEL_MASK (0x3 << 17)
#define IMX6SL_GPR1_FEC_CLOCK_MUX2_SEL_MASK (0x1 << 14)
+/* For imx6sx iomux gpr register field define */
+#define IMX6SX_GPR5_PCIE_BTNRST BIT(19)
+#define IMX6SX_GPR5_PCIE_BTNRST_CLR 0x0
+#define IMX6SX_GPR5_PCIE_PERST BIT(18)
+#define IMX6SX_GPR5_PCIE_PERST_CLR 0x0
+
+#define IMX6SX_GPR12_PCIE_TEST_PD BIT(30)
+#define IMX6SX_GPR12_PCIE_TEST_PD_CLR 0x0
+#define IMX6SX_GPR12_RX_EQ_MASK (0x7 << 0)
+#define IMX6SX_GPR12_RX_EQ_2 (0x2 << 0)
#endif /* __LINUX_IMX6Q_IOMUXC_GPR_H */

--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |

H***@freescale.com

2014-09-22 10:48:30 UTC

Permalink

-----Original Message-----
Sent: Monday, September 22, 2014 6:02 PM
To: Zhu Richard-R65037
Subject: Re: [PATCH RFC 2/2] PCI: imx6: add imx6sx pcie support
Hi Richard,
This commit smashes lots of things together. It would be much easier to review
this if the changes were broken out appropriately.
Also why are you sending RFC patches to stable?
A few quick comments below, this isn't a full review yet.

[Richard] This comment is missing in last reply.
Ok, commit would be separated in the next review-around.
Stable is one mistake when I copy/past the --cc mail list. It would be removed later.

Best Regards
Richard Zhu

You remove this clock from the binding. I don't see a reason why, just fill in
pcie_ref for both pcie_bus and pcie_phy. No need to implement different code
paths for this.

---

Post by Richard Zhu
include/linux/mfd/syscon/imx6q-iomuxc-gpr.h | 14 ++
5 files changed, 249 insertions(+), 42 deletions(-)
diff --git a/arch/arm/boot/dts/imx6sx-sdb.dts
b/arch/arm/boot/dts/imx6sx-sdb.dts
index a3980d9..83d0892 100644
--- a/arch/arm/boot/dts/imx6sx-sdb.dts
+++ b/arch/arm/boot/dts/imx6sx-sdb.dts
@@ -251,6 +251,14 @@
};
};
+&pcie {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pcie>;
+ power-on-gpio = <&gpio2 1 0>;

No such GPIO in the binding.

Post by Richard Zhu
+ reset-gpio = <&gpio2 0 0>;
+ status = "okay";
+};
+
&ssi2 {
status = "okay";
};
@@ -365,6 +373,13 @@

;

configuration space */

Post by Richard Zhu
+ 0x81000000 0 0 0x08f80000 0 0x00010000 /*

downstream I/O */

Post by Richard Zhu
+ 0x82000000 0 0x01000000 0x08000000 0 0x00f00000>; /*
+non-prefetchable memory */
num-lanes = <1>;
- interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&clks IMX6SX_CLK_PCIE_REF_125M>,
- <&clks IMX6SX_CLK_PCIE_AXI>,
- <&clks IMX6SX_CLK_LVDS1_OUT>,
- <&clks IMX6SX_CLK_DISPLAY_AXI>;
- clock-names = "pcie_ref_125m", "pcie_axi",
- "lvds_gate", "display_axi";
+ interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "msi";
+ #interrupt-cells = <1>;
+ interrupt-map-mask = <0 0 0 0x7>;
+ interrupt-map = <0 0 0 1 &intc GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 2 &intc GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 3 &intc GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 4 &intc GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks IMX6SX_CLK_PCIE_AXI>,
+ <&clks IMX6SX_CLK_DISPLAY_AXI>,
+ <&clks IMX6SX_CLK_LVDS1_OUT>;
+ clock-names = "pcie", "pcie_sec", "pcie_bus";
+ pcie-supply = <&reg_pcie>;
status = "disabled";
};
};
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index be9a51a..0a055f0 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -718,6 +718,7 @@ config SOC_IMX6SL
config SOC_IMX6SX
bool "i.MX6 SoloX support"
+ select PCI_DOMAINS if PCI
select PINCTRL_IMX6SX
select SOC_IMX6
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 233fe8a..c9b2f69 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -18,12 +18,16 @@
#include <linux/mfd/syscon.h>
#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
#include <linux/of_gpio.h>
#include <linux/pci.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
#include <linux/resource.h>
#include <linux/signal.h>
+#include <linux/syscore_ops.h>
#include <linux/types.h>
#include <linux/interrupt.h>
@@ -31,15 +35,32 @@
#define to_imx6_pcie(x) container_of(x, struct imx6_pcie, pp)
+/* The pcie who have standalone power domain */
+#define PCIE_PHY_HAS_PWR_DOMAIN BIT(0)
+
+struct imx_pcie_data {
+ unsigned int flags;
+};
+
+static const struct imx_pcie_data imx6sx_pcie_data = {
+ .flags = PCIE_PHY_HAS_PWR_DOMAIN,
+};
+
struct imx6_pcie {
int reset_gpio;
+ int power_on_gpio;
+ const struct imx_pcie_data *data;
struct clk *pcie_bus;
struct clk *pcie_phy;
+ struct clk *pcie_sec;
struct clk *pcie;
struct pcie_port pp;
struct regmap *iomuxc_gpr;
+ struct regmap *gpc_ips_reg;
+ struct regulator *pcie_regulator;
void __iomem *mem_base;
};
+static struct imx6_pcie *imx6_pcie;
/* PCIe Root Complex registers (memory-mapped) */
#define PCIE_RC_LCR 0x7c
@@ -77,6 +98,11 @@ struct imx6_pcie {
#define PHY_RX_OVRD_IN_LO_RX_DATA_EN (1 << 5) #define
PHY_RX_OVRD_IN_LO_RX_PLL_EN (1 << 3)
+static inline bool is_imx6sx_pcie(struct imx6_pcie *imx6_pcie) {
+ return imx6_pcie->data == &imx6sx_pcie_data; }
+
static int pcie_phy_poll_ack(void __iomem *dbi_base, int exp_val) {
u32 val;
@@ -257,10 +283,21 @@ static int imx6_pcie_deassert_core_reset(struct

pcie_port *pp)

Post by Richard Zhu
struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
int ret;
- ret = clk_prepare_enable(imx6_pcie->pcie_phy);
- if (ret) {
- dev_err(pp->dev, "unable to enable pcie_phy clock\n");
- goto err_pcie_phy;
+ if (gpio_is_valid(imx6_pcie->power_on_gpio))
+ gpio_set_value(imx6_pcie->power_on_gpio, 1);
+

I won't allow this to creep in again. This needs to be a proper regulator, not
some kind of gpio.

pcie_port *pp)

Post by Richard Zhu
goto err_pcie;
}
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ IMX6SX_GPR12_PCIE_TEST_PD,
+ IMX6SX_GPR12_PCIE_TEST_PD_CLR);
+ } else {
+ /* power up core phy and enable ref clock */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
+ IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18);
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
+ IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
+ }
+
/* allow the clocks to stabilize */
usleep_range(200, 500);
- /* power up core phy and enable ref clock */
- regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
- IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18);
- regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
- IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
-
/* Some boards don't have PCIe reset GPIO. */
if (gpio_is_valid(imx6_pcie->reset_gpio)) {
gpio_set_value(imx6_pcie->reset_gpio, 0);
msleep(100);
gpio_set_value(imx6_pcie->reset_gpio, 1);
}
+
+ /*
+ * iMX6SX PCIe has the stand-alone power domain.
+ * refer to the initialization for iMX6SX PCIe,
+ * release the PCIe PHY reset here,
+ * before LTSSM enable is set.
+ */
+ if (is_imx6sx_pcie(imx6_pcie))
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
+ IMX6SX_GPR5_PCIE_BTNRST,
+ IMX6SX_GPR5_PCIE_BTNRST_CLR);
+
return 0;
clk_disable_unprepare(imx6_pcie->pcie_bus);
- clk_disable_unprepare(imx6_pcie->pcie_phy);
+ if (!is_imx6sx_pcie(imx6_pcie))
+ clk_disable_unprepare(imx6_pcie->pcie_phy);
+ if (is_imx6sx_pcie(imx6_pcie))
+ clk_disable_unprepare(imx6_pcie->pcie_sec);
return ret;
}
static void imx6_pcie_init_phy(struct pcie_port *pp) {
struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
+ int ret;
+
+ /*
+ * iMX6SX PCIe has the stand-alone power domain
+ * add the initialization here for iMX6SX PCIe.
+ */
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ /* Force PCIe PHY reset */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
+ IMX6SX_GPR5_PCIE_BTNRST,
+ IMX6SX_GPR5_PCIE_BTNRST);
+
+ regmap_update_bits(imx6_pcie->gpc_ips_reg, 0, 1 << 7, 1 << 7);

What is this? Is this a regulator? If so, why isn't it abstracted as a proper
regulator?

This regulator gets enabled here, but I don't see any path were we would
disable it. Is this a always-on regulator or are you simply missing the calls
to disable it in the runtime-pm hooks?

int

Post by Richard Zhu
imx6_pcie_start_link(struct pcie_port *pp)
/* Start LTSSM. */
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
- IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
+ IMX6Q_GPR12_PCIE_CTL_2,
+ IMX6Q_GPR12_PCIE_CTL_2);
ret = imx6_pcie_wait_for_link(pp);
if (ret)
@@ -546,10 +629,73 @@ static int __init imx6_add_pcie_port(struct pcie_port

*pp,

Post by Richard Zhu
return 0;
}
+static const struct of_device_id imx6_pcie_of_match[] = {
+ { .compatible = "fsl,imx6q-pcie", },
+ { .compatible = "fsl,imx6sx-pcie", .data = &imx6sx_pcie_data},
+ {},
+};
+MODULE_DEVICE_TABLE(of, imx6_pcie_of_match);
+
+#ifdef CONFIG_PM_SLEEP
+static int pci_imx_suspend(void)
+{
+ int rc = 0;
+
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ /* PM_TURN_OFF */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ BIT(16), 1 << 16);
+ udelay(10);
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ BIT(16), 0 << 16);

Why are there no defines for those bits? I can't really tell what's going on
here. Is this some kind of power gating the SoC partition? In that case it
should really be implemented as a power-domain and not as some kind of ad-hoc
GPR register bashing.

Why the double reset?

*pdev)

Post by Richard Zhu
}
}
+ imx6_pcie->power_on_gpio = of_get_named_gpio(np, "power-on-gpio", 0);
+ if (gpio_is_valid(imx6_pcie->power_on_gpio)) {
+ ret = devm_gpio_request_one(&pdev->dev,
+ imx6_pcie->power_on_gpio,
+ GPIOF_OUT_INIT_LOW,
+ "PCIe power enable");
+ if (ret) {
+ dev_err(&pdev->dev, "unable to get power-on gpio\n");
+ return ret;
+ }
+ }
+
/* Fetch clocks */
- imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");
- if (IS_ERR(imx6_pcie->pcie_phy)) {
- dev_err(&pdev->dev,
- "pcie_phy clock source missing or invalid\n");
- return PTR_ERR(imx6_pcie->pcie_phy);
+ if (!is_imx6sx_pcie(imx6_pcie)) {
+ imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");
+ if (IS_ERR(imx6_pcie->pcie_phy)) {
+ dev_err(&pdev->dev,
+ "pcie_phy clock source missing or invalid\n");
+ return PTR_ERR(imx6_pcie->pcie_phy);
+ }
}

Missing binding update for the changed clock handling. Also what is this clock
pcie_sec? Is it really a second AXI clock? I highly doubt that the register
interface of the core is clocked by two different clocks. What blocks of the
dw_pci core does this clock feed?
*pdev)

Post by Richard Zhu
}
/* Grab GPR config register range */
- imx6_pcie->iomuxc_gpr =
- syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ /* Get pcie regulator */
+ imx6_pcie->pcie_regulator = devm_regulator_get(pp->dev, "pcie");

Uh, this has nothing to do with the GPR range, so the comment above is
confusing. Also missing binding update for this regulator.

*pdev)

Post by Richard Zhu
imx6_pcie_assert_core_reset(&imx6_pcie->pp);
}
-static const struct of_device_id imx6_pcie_of_match[] = {
- { .compatible = "fsl,imx6q-pcie", },
- {},
-};
-MODULE_DEVICE_TABLE(of, imx6_pcie_of_match);
-
static struct platform_driver imx6_pcie_driver = {
.driver = {
.name = "imx6q-pcie",
diff --git a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
index ff44374..f02875e 100644
--- a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
+++ b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
@@ -113,10 +113,12 @@
#define IMX6Q_GPR1_MIPI_IPU1_MUX_GASKET 0x0
#define IMX6Q_GPR1_MIPI_IPU1_MUX_IOMUX BIT(19)
#define IMX6Q_GPR1_PCIE_TEST_PD BIT(18)
+#define IMX6Q_GPR1_PCIE_TEST_PD_CLR 0x0
#define IMX6Q_GPR1_IPU_VPU_MUX_MASK BIT(17)
#define IMX6Q_GPR1_IPU_VPU_MUX_IPU1 0x0
#define IMX6Q_GPR1_IPU_VPU_MUX_IPU2 BIT(17)
#define IMX6Q_GPR1_PCIE_REF_CLK_EN BIT(16)
+#define IMX6Q_GPR1_PCIE_REF_CLK_CLR 0x0
#define IMX6Q_GPR1_USB_EXP_MODE BIT(15)
#define IMX6Q_GPR1_PCIE_INT BIT(14)
#define IMX6Q_GPR1_USB_OTG_ID_SEL_MASK BIT(13)
@@ -300,7 +302,9 @@
#define IMX6Q_GPR12_ARMP_APB_CLK_EN BIT(24)
#define IMX6Q_GPR12_DEVICE_TYPE (0xf << 12)
#define IMX6Q_GPR12_PCIE_CTL_2 BIT(10)
+#define IMX6Q_GPR12_PCIE_CTL_2_CLR 0x0
#define IMX6Q_GPR12_LOS_LEVEL (0x1f << 4)
+#define IMX6Q_GPR12_LOS_LEVEL_9 (0x9 << 4)
#define IMX6Q_GPR13_SDMA_STOP_REQ BIT(30)
#define IMX6Q_GPR13_CAN2_STOP_REQ BIT(29)
@@ -395,4 +399,14 @@
#define IMX6SL_GPR1_FEC_CLOCK_MUX1_SEL_MASK (0x3 << 17)
#define IMX6SL_GPR1_FEC_CLOCK_MUX2_SEL_MASK (0x1 << 14)
+/* For imx6sx iomux gpr register field define */
+#define IMX6SX_GPR5_PCIE_BTNRST BIT(19)
+#define IMX6SX_GPR5_PCIE_BTNRST_CLR 0x0
+#define IMX6SX_GPR5_PCIE_PERST BIT(18)
+#define IMX6SX_GPR5_PCIE_PERST_CLR 0x0
+
+#define IMX6SX_GPR12_PCIE_TEST_PD BIT(30)
+#define IMX6SX_GPR12_PCIE_TEST_PD_CLR 0x0
+#define IMX6SX_GPR12_RX_EQ_MASK (0x7 << 0)
+#define IMX6SX_GPR12_RX_EQ_2 (0x2 << 0)
#endif /* __LINUX_IMX6Q_IOMUXC_GPR_H */

--
Pengutronix e.K. | Lucas Stach |
Industrial Linux

H***@freescale.com

2014-09-22 10:31:41 UTC

Permalink

Hi Lucas:
Thanks for your quick review comments.

You remove this clock from the binding. I don't see a reason why, just fill in
pcie_ref for both pcie_bus and pcie_phy. No need to implement different code
paths for this.

[Richard] pcie_ref is not only used as pcie_phy, but also as the parent of the pcie_bus.
Pcie_phy would be enabled automatically, when pcie_bus is enabled on imx6sx.

---

No such GPIO in the binding.

[Richard] power-on-gpio would be removed.

Post by Richard Zhu
+ reset-gpio = <&gpio2 0 0>;
+ status = "okay";
+};
+
&ssi2 {
status = "okay";
};
@@ -365,6 +373,13 @@

;

configuration space */

Post by Richard Zhu
+ 0x81000000 0 0 0x08f80000 0 0x00010000 /*

downstream I/O */

pcie_port *pp)

I won't allow this to creep in again. This needs to be a proper regulator, not
some kind of gpio.

[Richard] would be removed. Thanks.

pcie_port *pp)

What is this? Is this a regulator? If so, why isn't it abstracted as a proper
regulator?

[Richard]No, it's a pre-condition before enable the pcie regulator.

This regulator gets enabled here, but I don't see any path were we would
disable it. Is this a always-on regulator or are you simply missing the calls
to disable it in the runtime-pm hooks?

[Richard]This regulator is enabled-once during pcie initialization.
imx pcie has to be re-initialized completely if this regulator is disabled once.
It's a always on regulator after the imx pcie is initialized.

int

*pp,

Why are there no defines for those bits? I can't really tell what's going on
here. Is this some kind of power gating the SoC partition? In that case it
should really be implemented as a power-domain and not as some kind of ad-hoc
GPR register bashing.

[Richard] Not a power-domain. This bit is just used as RC to kick-off the PM_TURN_OFF message to EP.
It's is used to indicated to EP that the RC is ready to ENTER into L2 state of ASPM.

Why the double reset?

[Richard] The second one would be removed. Thanks.

*pdev)

Missing binding update for the changed clock handling. Also what is this clock
pcie_sec? Is it really a second AXI clock? I highly doubt that the register
interface of the core is clocked by two different clocks. What blocks of the
dw_pci core does this clock feed?

[Richard] As I know that imx6sx pcie axi clock is contained in the dis_axi domain.
Dis_axi should be turned-on, if pcie is enabled.
How about use the pcie_bus binding dis_axi on imx6sx?
Since the pcie_ref is not only used as pcie_ref but also as the parent of the pcie_phy on imx6sx.
In this case, we can keep alignment the clocks binding with other imx6 pcie.

*pdev)

Uh, this has nothing to do with the GPR range, so the comment above is
confusing. Also missing binding update for this regulator.

[Richard] The comment would be removed. Binding update for this regulator would be added later.

*pdev)

--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |

Best Regards
Richard Zhu

Greg KH

2014-09-22 13:15:39 UTC

Permalink

Post by Richard Zhu
- imx6sx pcie has its own standalone pcie power supply.
In order to turn on the imx6sx pcie power during
initialization. Add the pcie regulator and the gpc regmap
into the imx6sx pcie structure.
- imx6sx pcie has the new added reset mechanism, add the
reset operations into the initialization.
- another dis_axi clk is mandatory required by imx6sx pcie.
Add one new clk named pcie_sec into imx6_pcie structure.
- pcie_ref_125m is not used as pcie_phy clk anymore on imx6sx.
The parent clk (pcie_ref) of the pcie_bus(lvds1_gate)
is used as pcie_phy clk.
- Register one PM call-back, enter/exit L2 state of the ASPM
during system suspend/resume.
- wait the clocks to stabilize after the pcie_ref_en
(IMX6Q_GPR1_PCIE_REF_CLK_EN) is set.
---
arch/arm/boot/dts/imx6sx-sdb.dts | 15 ++
arch/arm/boot/dts/imx6sx.dtsi | 33 ++--
arch/arm/mach-imx/Kconfig | 1 +
drivers/pci/host/pci-imx6.c | 228 ++++++++++++++++++++++++----
include/linux/mfd/syscon/imx6q-iomuxc-gpr.h | 14 ++
5 files changed, 249 insertions(+), 42 deletions(-)

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

</formletter>

H***@freescale.com

2014-09-23 03:11:51 UTC

Permalink

-----Original Message-----
Sent: Monday, September 22, 2014 9:16 PM
To: Zhu Richard-R65037
Subject: Re: [PATCH RFC 2/2] PCI: imx6: add imx6sx pcie support

Post by Richard Zhu
- imx6sx pcie has its own standalone pcie power supply.
In order to turn on the imx6sx pcie power during initialization. Add
the pcie regulator and the gpc regmap into the imx6sx pcie structure.
- imx6sx pcie has the new added reset mechanism, add the reset
operations into the initialization.
- another dis_axi clk is mandatory required by imx6sx pcie.
Add one new clk named pcie_sec into imx6_pcie structure.
- pcie_ref_125m is not used as pcie_phy clk anymore on imx6sx.
The parent clk (pcie_ref) of the pcie_bus(lvds1_gate) is used as
pcie_phy clk.
- Register one PM call-back, enter/exit L2 state of the ASPM during
system suspend/resume.
- wait the clocks to stabilize after the pcie_ref_en
(IMX6Q_GPR1_PCIE_REF_CLK_EN) is set.
---
arch/arm/boot/dts/imx6sx-sdb.dts | 15 ++
arch/arm/boot/dts/imx6sx.dtsi | 33 ++--
arch/arm/mach-imx/Kconfig | 1 +
drivers/pci/host/pci-imx6.c | 228 ++++++++++++++++++++++++-

---

Post by Richard Zhu
include/linux/mfd/syscon/imx6q-iomuxc-gpr.h | 14 ++
5 files changed, 249 insertions(+), 42 deletions(-)

<formletter>
This is not the correct way to submit patches for inclusion in the stable
kernel tree. Please read Documentation/stable_kernel_rules.txt
for how to do this properly.
</formletter>

[Richard] Roger that, thanks for your reminder.
It's my fault to make a mistake to send the patch review to table kernel mail-list. Sorry about that.

Best Regards
Richard Zhu

Richard Zhu

2014-09-22 09:01:37 UTC

Permalink

- enable pcie support on imx6qdl sabresd and asbreauto boards.
- sabresd board has the pcie power on and reset gpios, but
sabreauto doesn't have these two gpios.

Signed-off-by: Richard Zhu <***@freescale.com>
---
arch/arm/boot/dts/imx6qdl-sabreauto.dtsi | 4 ++++
arch/arm/boot/dts/imx6qdl-sabresd.dtsi | 2 ++
2 files changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index 009abd6..d6040a5 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
@@ -410,6 +410,10 @@
};
};

+&pcie {
+ status = "okay";
+};
+
&pwm3 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pwm3>;
diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index ec43dde..c2d3224 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -396,6 +396,7 @@

pinctrl_pcie: pciegrp {
fsl,pins = <
+ MX6QDL_PAD_EIM_D19__GPIO3_IO19 0x80000000
MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x80000000

;

};
@@ -502,6 +503,7 @@
&pcie {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pcie>;
+ power-on-gpio = <&gpio3 19 0>;
reset-gpio = <&gpio7 12 0>;
status = "okay";
};

--
1.9.1

Greg KH

2014-09-22 13:15:19 UTC

Permalink

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

</formletter>

H***@freescale.com

2014-09-23 03:11:06 UTC

Permalink

-----Original Message-----
Sent: Monday, September 22, 2014 9:15 PM
To: Zhu Richard-R65037
Subject: Re: [PATCH RFC 1/2] PCI: imx6: enable pcie on imx6qdl sabresd and
sabreauto

<formletter>
This is not the correct way to submit patches for inclusion in the stable
kernel tree. Please read Documentation/stable_kernel_rules.txt
for how to do this properly.
</formletter>

[Richard] Roger that, thanks for your reminder.
It's my fault to make a mistake to send the patch review to table kernel mail-list. Sorry about that.

Best Regards
Richard Zhu

H***@freescale.com

2014-09-22 09:55:31 UTC

Permalink

Hi Lucas:
Thanks for your quickly review-comments.

-----Original Message-----
Sent: Monday, September 22, 2014 5:36 PM
To: Zhu Richard-R65037
Subject: Re: [PATCH RFC 1/2] PCI: imx6: enable pcie on imx6qdl sabresd and
sabreauto

- enable pcie support on imx6qdl sabresd and asbreauto boards.
- sabresd board has the pcie power on and reset gpios, but sabreauto
doesn't have these two gpios.
---
arch/arm/boot/dts/imx6qdl-sabreauto.dtsi | 4 ++++
arch/arm/boot/dts/imx6qdl-sabresd.dtsi | 2 ++
2 files changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index 009abd6..d6040a5 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
@@ -410,6 +410,10 @@
};
};
+&pcie {
+ status = "okay";
+};
+
&pwm3 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pwm3>;
diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index ec43dde..c2d3224 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -396,6 +396,7 @@
pinctrl_pcie: pciegrp {
fsl,pins = <
+ MX6QDL_PAD_EIM_D19__GPIO3_IO19 0x80000000
MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x80000000

;

};
@@ -502,6 +503,7 @@
&pcie {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pcie>;
+ power-on-gpio = <&gpio3 19 0>;
reset-gpio = <&gpio7 12 0>;
status = "okay";
};

This hunk is wrong. There is no "power-on-gpio" in the binding anymore.
Also there is already a change in Shawns tree to model this as a always-on
regulator. If we really want to control pci bus power this needs to be done
through this regulator, not some arbitrary gpio hack.
Also I don't see why this would be stable material.

[Richard] Yes, it is. The "power-on-gpio" is not required anymore, would be removed.
Stable kernel mail-list would be removed too.

Regards,
Lucas
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solution