Discussion:
[PATCH v6]PCI: imx6: enable pcie on imx6sx sdb and imx6qdl sabreauto.
Richard Zhu
2014-10-16 07:52:30 UTC
Permalink
Main changes since the v5:
1.use noirq pm_ops instead of the syscore pm_ops
2.in order to not break compilation, move gpr bits modifications, before
imx6 pcie changes.
3.host init maybe failed, return negative value when there is a failure
in the host init. In responding, the host_init func type had been change from
void to init.

[PATCH v6 01/13] PCI: designware: Refine setup_rc and add msi data
[PATCH v6 02/13] PCI: designware: Set func type of host init to int
[PATCH v6 03/13] PCI: dra7xx: Change the func type of host init
[PATCH v6 04/13] PCI: exynos: Change the func type of host init
[PATCH v6 05/13] PCI: spear: Change the func type of host init
[PATCH v6 06/13] PCI: designware: Fix one potential assignment error
[PATCH v6 07/13] ARM: imx6sx: Add imx6sx pcie related gpr bits
[PATCH v6 08/13] PCI: imx6: Wait the clocks to stabilize after ref_en
[PATCH v6 09/13] PCI: imx6: Add imx6sx pcie support
[PATCH v6 10/13] ARM: imx6qdl: Enable pcie on imx6qdl sabreauto
[PATCH v6 11/13] ARM: imx6: Update dts and binding for imx6sx pcie
[PATCH v6 12/13] ARM: imx6sx: Add syscon into gpc dts
[PATCH v6 13/13] ARM: imx6sx: Enable pcie on imx6sx sdb board
Richard Zhu
2014-10-16 07:52:34 UTC
Permalink
In order to avoid compilation warning, change
the func type of host init from void to int.

Signed-off-by: Richard Zhu <***@freescale.com>
---
drivers/pci/host/pci-exynos.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
index c5d0ca3..606d0a9 100644
--- a/drivers/pci/host/pci-exynos.c
+++ b/drivers/pci/host/pci-exynos.c
@@ -494,10 +494,12 @@ static int exynos_pcie_link_up(struct pcie_port *pp)
return 0;
}

-static void exynos_pcie_host_init(struct pcie_port *pp)
+static int exynos_pcie_host_init(struct pcie_port *pp)
{
exynos_pcie_establish_link(pp);
exynos_pcie_enable_interrupts(pp);
+
+ return 0;
}

static struct pcie_host_ops exynos_pcie_host_ops = {
--
1.9.1
Richard Zhu
2014-10-16 07:52:31 UTC
Permalink
From: Richard Zhu <***@freescale.com>

- move "program correct class for RC" from dw_pcie_host_init()
to dw_pcie_setup_rc(). since this is RC setup, it's
better to contained in dw_pcie_setup_rc function.
Then, RC can be re-setup really by dw_pcie_setup_rc().
- add one store/re-store msi cfg functions. Because that
pcie controller maybe powered off during system suspend,
and the msi data configuration would be lost.
these functions can be used to store/restore the msi data
and msi_enable during the suspend/resume callback.

Signed-off-by: Richard Zhu <***@freescale.com>
---
drivers/pci/host/pcie-designware.c | 21 ++++++++++++++++++---
drivers/pci/host/pcie-designware.h | 3 +++
2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 538bbf3..b1f82ff 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -194,6 +194,19 @@ void dw_pcie_msi_init(struct pcie_port *pp)
dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_HI, 4, 0);
}

+void dw_pcie_msi_cfg_store(struct pcie_port *pp)
+{
+ dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE, 4, &pp->msi_enable);
+}
+
+void dw_pcie_msi_cfg_restore(struct pcie_port *pp)
+{
+ dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4,
+ virt_to_phys((void *)pp->msi_data));
+ dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_HI, 4, 0);
+ dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE, 4, pp->msi_enable);
+}
+
static int find_valid_pos0(struct pcie_port *pp, int msgvec, int pos, int *pos0)
{
int flag = 1;
@@ -570,9 +583,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp)

dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0);

- /* program correct class for RC */
- dw_pcie_wr_own_conf(pp, PCI_CLASS_DEVICE, 2, PCI_CLASS_BRIDGE_PCI);
-
dw_pcie_rd_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, &val);
val |= PORT_LOGIC_SPEED_CHANGE;
dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
@@ -917,6 +927,11 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
val = memlimit | membase;
dw_pcie_writel_rc(pp, val, PCI_MEMORY_BASE);

+ /* program correct class for RC */
+ dw_pcie_readl_rc(pp, PCI_CLASS_REVISION, &val);
+ val |= PCI_CLASS_BRIDGE_PCI << 16;
+ dw_pcie_writel_rc(pp, val, PCI_CLASS_REVISION);
+
/* setup command register */
dw_pcie_readl_rc(pp, PCI_COMMAND, &val);
val &= 0xffff0000;
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index a476e60..b0bfed0 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -56,6 +56,7 @@ struct pcie_port {
int msi_irq;
struct irq_domain *irq_domain;
unsigned long msi_data;
+ unsigned int msi_enable;
DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
};

@@ -83,6 +84,8 @@ int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val);
int dw_pcie_cfg_write(void __iomem *addr, int where, int size, u32 val);
irqreturn_t dw_handle_msi_irq(struct pcie_port *pp);
void dw_pcie_msi_init(struct pcie_port *pp);
+void dw_pcie_msi_cfg_store(struct pcie_port *pp);
+void dw_pcie_msi_cfg_restore(struct pcie_port *pp);
int dw_pcie_link_up(struct pcie_port *pp);
void dw_pcie_setup_rc(struct pcie_port *pp);
int dw_pcie_host_init(struct pcie_port *pp);
--
1.9.1
Lucas Stach
2014-10-16 10:36:57 UTC
Permalink
Post by Richard Zhu
- move "program correct class for RC" from dw_pcie_host_init()
to dw_pcie_setup_rc(). since this is RC setup, it's
better to contained in dw_pcie_setup_rc function.
Then, RC can be re-setup really by dw_pcie_setup_rc().
- add one store/re-store msi cfg functions. Because that
pcie controller maybe powered off during system suspend,
and the msi data configuration would be lost.
these functions can be used to store/restore the msi data
and msi_enable during the suspend/resume callback.
NAK for the reasons below.
Post by Richard Zhu
---
drivers/pci/host/pcie-designware.c | 21 ++++++++++++++++++---
drivers/pci/host/pcie-designware.h | 3 +++
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 538bbf3..b1f82ff 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -194,6 +194,19 @@ void dw_pcie_msi_init(struct pcie_port *pp)
dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_HI, 4, 0);
}
+void dw_pcie_msi_cfg_store(struct pcie_port *pp)
+{
+ dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE, 4, &pp->msi_enable);
You are only saving and restoring this one register. While this might
work for the current implementation that uses only a max of 32 vectors
this is not true in general. At least the imx6 implementation supports
up to 128 vectors, so actually 4 registers might be used here.
Post by Richard Zhu
+}
+
+void dw_pcie_msi_cfg_restore(struct pcie_port *pp)
+{
+ dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4,
+ virt_to_phys((void *)pp->msi_data));
+ dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_HI, 4, 0);
+ dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE, 4, pp->msi_enable);
Murali asked you to take into account the newly added requirements for
older designware cores. You did not.
Post by Richard Zhu
+}
+
static int find_valid_pos0(struct pcie_port *pp, int msgvec, int pos, int *pos0)
{
int flag = 1;
@@ -570,9 +583,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0);
- /* program correct class for RC */
- dw_pcie_wr_own_conf(pp, PCI_CLASS_DEVICE, 2, PCI_CLASS_BRIDGE_PCI);
-
dw_pcie_rd_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, &val);
val |= PORT_LOGIC_SPEED_CHANGE;
dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
@@ -917,6 +927,11 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
val = memlimit | membase;
dw_pcie_writel_rc(pp, val, PCI_MEMORY_BASE);
+ /* program correct class for RC */
+ dw_pcie_readl_rc(pp, PCI_CLASS_REVISION, &val);
+ val |= PCI_CLASS_BRIDGE_PCI << 16;
+ dw_pcie_writel_rc(pp, val, PCI_CLASS_REVISION);
+
This is getting ridiculous. How many time did I ask you to change this
now. This hunk should just read:

+ /* program correct class for RC */
+ dw_pcie_wr_own_conf(pp, PCI_CLASS_DEVICE, 2, PCI_CLASS_BRIDGE_PCI);
+
Post by Richard Zhu
/* setup command register */
dw_pcie_readl_rc(pp, PCI_COMMAND, &val);
val &= 0xffff0000;
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index a476e60..b0bfed0 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -56,6 +56,7 @@ struct pcie_port {
int msi_irq;
struct irq_domain *irq_domain;
unsigned long msi_data;
+ unsigned int msi_enable;
DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
};
@@ -83,6 +84,8 @@ int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val);
int dw_pcie_cfg_write(void __iomem *addr, int where, int size, u32 val);
irqreturn_t dw_handle_msi_irq(struct pcie_port *pp);
void dw_pcie_msi_init(struct pcie_port *pp);
+void dw_pcie_msi_cfg_store(struct pcie_port *pp);
+void dw_pcie_msi_cfg_restore(struct pcie_port *pp);
int dw_pcie_link_up(struct pcie_port *pp);
void dw_pcie_setup_rc(struct pcie_port *pp);
int dw_pcie_host_init(struct pcie_port *pp);
H***@freescale.com
2014-10-20 05:23:52 UTC
Permalink
-----Original Message-----
Sent: Thursday, October 16, 2014 6:37 PM
To: Richard Zhu
Subject: Re: [PATCH v6 01/13] PCI: designware: Refine setup_rc and add msi
data restore
- move "program correct class for RC" from dw_pcie_host_init() to
dw_pcie_setup_rc(). since this is RC setup, it's better to contained
in dw_pcie_setup_rc function.
Then, RC can be re-setup really by dw_pcie_setup_rc().
- add one store/re-store msi cfg functions. Because that pcie
controller maybe powered off during system suspend, and the msi data
configuration would be lost.
these functions can be used to store/restore the msi data and
msi_enable during the suspend/resume callback.
NAK for the reasons below.
---
drivers/pci/host/pcie-designware.c | 21 ++++++++++++++++++---
drivers/pci/host/pcie-designware.h | 3 +++
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/host/pcie-designware.c
b/drivers/pci/host/pcie-designware.c
index 538bbf3..b1f82ff 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -194,6 +194,19 @@ void dw_pcie_msi_init(struct pcie_port *pp)
dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_HI, 4, 0); }
+void dw_pcie_msi_cfg_store(struct pcie_port *pp) {
+ dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE, 4, &pp->msi_enable);
You are only saving and restoring this one register. While this might work for
the current implementation that uses only a max of 32 vectors this is not true
in general. At least the imx6 implementation supports up to 128 vectors, so
actually 4 registers might be used here.
[Richard] Ok, all the four registers would be stored/re-stored.
+}
+
+void dw_pcie_msi_cfg_restore(struct pcie_port *pp) {
+ dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_LO, 4,
+ virt_to_phys((void *)pp->msi_data));
+ dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_HI, 4, 0);
+ dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE, 4, pp->msi_enable);
Murali asked you to take into account the newly added requirements for older
designware cores. You did not.
[Richard] Sorry, Murali's comments is missed before, would be added later.
+}
+
static int find_valid_pos0(struct pcie_port *pp, int msgvec, int pos,
int *pos0) {
int flag = 1;
@@ -570,9 +583,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0);
- /* program correct class for RC */
- dw_pcie_wr_own_conf(pp, PCI_CLASS_DEVICE, 2, PCI_CLASS_BRIDGE_PCI);
-
dw_pcie_rd_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, &val);
val |= PORT_LOGIC_SPEED_CHANGE;
val = memlimit | membase;
dw_pcie_writel_rc(pp, val, PCI_MEMORY_BASE);
+ /* program correct class for RC */
+ dw_pcie_readl_rc(pp, PCI_CLASS_REVISION, &val);
+ val |= PCI_CLASS_BRIDGE_PCI << 16;
+ dw_pcie_writel_rc(pp, val, PCI_CLASS_REVISION);
+
This is getting ridiculous. How many time did I ask you to change this now.
+ /* program correct class for RC */
+ dw_pcie_wr_own_conf(pp, PCI_CLASS_DEVICE, 2, PCI_CLASS_BRIDGE_PCI);
+
[Richard] Ok, would be kept this way. Thanks.
/* setup command register */
dw_pcie_readl_rc(pp, PCI_COMMAND, &val);
val &= 0xffff0000;
diff --git a/drivers/pci/host/pcie-designware.h
b/drivers/pci/host/pcie-designware.h
index a476e60..b0bfed0 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -56,6 +56,7 @@ struct pcie_port {
int msi_irq;
struct irq_domain *irq_domain;
unsigned long msi_data;
+ unsigned int msi_enable;
DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS); };
@@ -83,6 +84,8 @@ int dw_pcie_cfg_read(void __iomem *addr, int where,
int size, u32 *val); int dw_pcie_cfg_write(void __iomem *addr, int
where, int size, u32 val); irqreturn_t dw_handle_msi_irq(struct
pcie_port *pp); void dw_pcie_msi_init(struct pcie_port *pp);
+void dw_pcie_msi_cfg_store(struct pcie_port *pp); void
+dw_pcie_msi_cfg_restore(struct pcie_port *pp);
int dw_pcie_link_up(struct pcie_port *pp); void
dw_pcie_setup_rc(struct pcie_port *pp); int dw_pcie_host_init(struct
pcie_port *pp);
Best Regard

Richard Zhu
2014-10-16 07:52:43 UTC
Permalink
From: Richard Zhu <***@freescale.com>

Signed-off-by: Richard Zhu <***@freescale.com>
---
arch/arm/boot/dts/imx6sx-sdb.dts | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/imx6sx-sdb.dts b/arch/arm/boot/dts/imx6sx-sdb.dts
index a3980d9..e28214a 100644
--- a/arch/arm/boot/dts/imx6sx-sdb.dts
+++ b/arch/arm/boot/dts/imx6sx-sdb.dts
@@ -90,6 +90,19 @@
regulator-min-microvolt = <5000000>;
regulator-max-microvolt = <5000000>;
};
+
+ reg_pcie: ***@4 {
+ compatible = "regulator-fixed";
+ reg = <3>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pcie_reg>;
+ regulator-name = "MPCIE_3V3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ gpio = <&gpio2 1 0>;
+ regulator-always-on;
+ enable-active-high;
+ };
};

sound {
@@ -251,6 +264,13 @@
};
};

+&pcie {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pcie>;
+ reset-gpio = <&gpio2 0 0>;
+ status = "okay";
+};
+
&ssi2 {
status = "okay";
};
@@ -365,6 +385,18 @@
;
};

+ pinctrl_pcie: pciegrp {
+ fsl,pins = <
+ MX6SX_PAD_ENET1_COL__GPIO2_IO_0 0x10b0
+ >;
+ };
+
+ pinctrl_pcie_reg: pciereggrp {
+ fsl,pins = <
+ MX6SX_PAD_ENET1_CRS__GPIO2_IO_1 0x10b0
+ >;
+ };
+
pinctrl_vcc_sd3: vccsd3grp {
fsl,pins = <
MX6SX_PAD_KEY_COL1__GPIO2_IO_11 0x17059
--
1.9.1
Richard Zhu
2014-10-16 07:52:35 UTC
Permalink
In order to avoid compilation warning, change
the func type of host init from void to int.

Signed-off-by: Richard Zhu <***@freescale.com>
---
drivers/pci/host/pcie-spear13xx.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
index 6dea9e4..b8fd76b 100644
--- a/drivers/pci/host/pcie-spear13xx.c
+++ b/drivers/pci/host/pcie-spear13xx.c
@@ -258,10 +258,12 @@ static int spear13xx_pcie_link_up(struct pcie_port *pp)
return 0;
}

-static void spear13xx_pcie_host_init(struct pcie_port *pp)
+static int spear13xx_pcie_host_init(struct pcie_port *pp)
{
spear13xx_pcie_establish_link(pp);
spear13xx_pcie_enable_interrupts(pp);
+
+ return 0;
}

static struct pcie_host_ops spear13xx_pcie_host_ops = {
--
1.9.1
Richard Zhu
2014-10-16 07:52:33 UTC
Permalink
In order to avoid compilation warning, change
the func type of host init from void to int.

Signed-off-by: Richard Zhu <***@freescale.com>
---
drivers/pci/host/pci-dra7xx.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
index 52b34fe..7b11968 100644
--- a/drivers/pci/host/pci-dra7xx.c
+++ b/drivers/pci/host/pci-dra7xx.c
@@ -141,13 +141,15 @@ static void dra7xx_pcie_enable_interrupts(struct pcie_port *pp)
LEG_EP_INTERRUPTS);
}

-static void dra7xx_pcie_host_init(struct pcie_port *pp)
+static int dra7xx_pcie_host_init(struct pcie_port *pp)
{
dw_pcie_setup_rc(pp);
dra7xx_pcie_establish_link(pp);
if (IS_ENABLED(CONFIG_PCI_MSI))
dw_pcie_msi_init(pp);
dra7xx_pcie_enable_interrupts(pp);
+
+ return 0;
}

static struct pcie_host_ops dra7xx_pcie_host_ops = {
--
1.9.1
Richard Zhu
2014-10-16 07:52:37 UTC
Permalink
From: Richard Zhu <***@freescale.com>

Signed-off-by: Richard Zhu <***@freescale.com>
---
include/linux/mfd/syscon/imx6q-iomuxc-gpr.h | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
index ff44374..3273b87 100644
--- a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
+++ b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
@@ -301,6 +301,7 @@
#define IMX6Q_GPR12_DEVICE_TYPE (0xf << 12)
#define IMX6Q_GPR12_PCIE_CTL_2 BIT(10)
#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 +396,12 @@
#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_PERST BIT(18)
+
+#define IMX6SX_GPR12_PCIE_PM_TURN_OFF BIT(16)
+#define IMX6SX_GPR12_PCIE_TEST_PD BIT(30)
+#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
Richard Zhu
2014-10-16 07:52:40 UTC
Permalink
From: Richard Zhu <***@freescale.com>

- enable pcie on imx6qdl sabreauto boards.

Signed-off-by: Richard Zhu <***@freescale.com>
Reviewed-by: Lucas Stach <***@pengutronix.de>
---
arch/arm/boot/dts/imx6qdl-sabreauto.dtsi | 4 ++++
1 file changed, 4 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>;
--
1.9.1
Richard Zhu
2014-10-16 07:52:42 UTC
Permalink
From: Richard Zhu <***@freescale.com>

In order to manipulate gpc bits for imx6sx
pcie in driver, add syscon into gpc dts

Signed-off-by: Richard Zhu <***@freescale.com>
---
arch/arm/boot/dts/imx6sx.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 0dfeade..88d7fd7 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -689,7 +689,8 @@
};

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>;
};
--
1.9.1
Richard Zhu
2014-10-16 07:52:36 UTC
Permalink
From: Richard Zhu <***@freescale.com>

if va_cfg0_base/va_cfg1_base are initialized by
designware core, the pp->cfg.start is not initialized
properly, when IORESOURCE_MEM "config" is represented
as cfg space resource.
solution: assign cfg_res->start to pp->cfg.start.

Signed-off-by: Richard Zhu <***@freescale.com>
---
drivers/pci/host/pcie-designware.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 1a2b477..3edd94d 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -450,6 +450,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
if (cfg_res) {
pp->config.cfg0_size = resource_size(cfg_res)/2;
pp->config.cfg1_size = resource_size(cfg_res)/2;
+ pp->cfg.start = cfg_res->start;
pp->cfg0_base = cfg_res->start;
pp->cfg1_base = cfg_res->start + pp->config.cfg0_size;
--
1.9.1
Richard Zhu
2014-10-16 07:52:32 UTC
Permalink
host init maybe failed, change the func type of host_init
defined in struct pci_host_ops from void to int.

Signed-off-by: Richard Zhu <***@freescale.com>
---
drivers/pci/host/pcie-designware.c | 7 +++++--
drivers/pci/host/pcie-designware.h | 2 +-
2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index b1f82ff..1a2b477 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -578,8 +578,11 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
}
}

- if (pp->ops->host_init)
- pp->ops->host_init(pp);
+ if (pp->ops->host_init) {
+ ret = pp->ops->host_init(pp);
+ if (ret < 0)
+ return ret;
+ }

dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0);

diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index b0bfed0..57ab11d 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -72,7 +72,7 @@ struct pcie_host_ops {
int (*wr_other_conf)(struct pcie_port *pp, struct pci_bus *bus,
unsigned int devfn, int where, int size, u32 val);
int (*link_up)(struct pcie_port *pp);
- void (*host_init)(struct pcie_port *pp);
+ int (*host_init)(struct pcie_port *pp);
void (*msi_set_irq)(struct pcie_port *pp, int irq);
void (*msi_clear_irq)(struct pcie_port *pp, int irq);
u32 (*get_msi_data)(struct pcie_port *pp);
--
1.9.1
Lucas Stach
2014-10-16 10:39:05 UTC
Permalink
Post by Richard Zhu
host init maybe failed, change the func type of host_init
defined in struct pci_host_ops from void to int.
NAK. You are breaking compilation within the series here. If you are
going to change the prototype the dependant changes need to be in the
same patch. So squash in at least patch 3-5 plus the imx6 change.
Post by Richard Zhu
---
drivers/pci/host/pcie-designware.c | 7 +++++--
drivers/pci/host/pcie-designware.h | 2 +-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index b1f82ff..1a2b477 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -578,8 +578,11 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
}
}
- if (pp->ops->host_init)
- pp->ops->host_init(pp);
+ if (pp->ops->host_init) {
+ ret = pp->ops->host_init(pp);
+ if (ret < 0)
+ return ret;
+ }
dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0);
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index b0bfed0..57ab11d 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -72,7 +72,7 @@ struct pcie_host_ops {
int (*wr_other_conf)(struct pcie_port *pp, struct pci_bus *bus,
unsigned int devfn, int where, int size, u32 val);
int (*link_up)(struct pcie_port *pp);
- void (*host_init)(struct pcie_port *pp);
+ int (*host_init)(struct pcie_port *pp);
void (*msi_set_irq)(struct pcie_port *pp, int irq);
void (*msi_clear_irq)(struct pcie_port *pp, int irq);
u32 (*get_msi_data)(struct pcie_port *pp);
H***@freescale.com
2014-10-20 05:21:10 UTC
Permalink
-----Original Message-----
Sent: Thursday, October 16, 2014 6:39 PM
To: Richard Zhu
Subject: Re: [PATCH v6 02/13] PCI: designware: Set func type of host init to
int
host init maybe failed, change the func type of host_init defined in
struct pci_host_ops from void to int.
NAK. You are breaking compilation within the series here. If you are going to
change the prototype the dependant changes need to be in the same patch. So
squash in at least patch 3-5 plus the imx6 change.
[Richard] So, this patch should be squashed with patch3-5, and the "PCI: imx6: Add imx6sx pcie support".
Thanks.
---
drivers/pci/host/pcie-designware.c | 7 +++++--
drivers/pci/host/pcie-designware.h | 2 +-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/host/pcie-designware.c
b/drivers/pci/host/pcie-designware.c
index b1f82ff..1a2b477 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -578,8 +578,11 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
}
}
- if (pp->ops->host_init)
- pp->ops->host_init(pp);
+ if (pp->ops->host_init) {
+ ret = pp->ops->host_init(pp);
+ if (ret < 0)
+ return ret;
+ }
dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0);
diff --git a/drivers/pci/host/pcie-designware.h
b/drivers/pci/host/pcie-designware.h
index b0bfed0..57ab11d 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -72,7 +72,7 @@ struct pcie_host_ops {
int (*wr_other_conf)(struct pcie_port *pp, struct pci_bus *bus,
unsigned int devfn, int where, int size, u32 val);
int (*link_up)(struct pcie_port *pp);
- void (*host_init)(struct pcie_port *pp);
+ int (*host_init)(struct pcie_port *pp);
void (*msi_set_irq)(struct pcie_port *pp, int irq);
void (*msi_clear_irq)(struct pcie_port *pp, int irq);
u32 (*get_msi_data)(struct pcie_port *pp);
Best Reg
Richard Zhu
2014-10-16 07:52:38 UTC
Permalink
From: Richard Zhu <***@freescale.com>

For boards without a reset gpio we skip the delay between enabling
the pcie_ref_clk and touching the RC registers for configuration.
System would be hangs when the clocks are not yet settled in the DW
PCIe core. So we need to make sure that there is always an
appropriate delay between those two actions.

Signed-off-by: Richard Zhu <***@freescale.com>
Tested-by: Tim Harvey <***@gateworks.com>
---
drivers/pci/host/pci-imx6.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 233fe8a..eac96fb 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -275,15 +275,22 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
goto err_pcie;
}

- /* 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);
+ /*
+ * 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);

+ /* allow the clocks to stabilize */
+ usleep_range(200, 500);
+
/* Some boards don't have PCIe reset GPIO. */
if (gpio_is_valid(imx6_pcie->reset_gpio)) {
gpio_set_value(imx6_pcie->reset_gpio, 0);
--
1.9.1
Richard Zhu
2014-10-16 07:52:39 UTC
Permalink
From: Richard Zhu <***@freescale.com>

- 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.
- register one PM call-back, enter/exit L2 state during
system suspend/resume.
use noirq pm_ops instead of the general pm_ops in dev_pm_ops,
since cfg read/write may occurs after suspend and before resume.
do msi store/re-store in suspend/resume callbacks, since
controller maybe turned off, and these msi cfg maybe lost
in suspend.
- disp_axi clock is required by pcie inbound axi port actually.
Add one more clock named pcie_inbound_axi for imx6sx pcie.
- host init maybe failed, return negative value when
there is a failure in the host init.

Signed-off-by: Richard Zhu <***@freescale.com>
---
drivers/pci/host/pci-imx6.c | 204 +++++++++++++++++++++++++++++++++++++++-----
1 file changed, 183 insertions(+), 21 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index eac96fb..dfe2334 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -22,6 +22,7 @@
#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/types.h>
@@ -35,9 +36,12 @@ struct imx6_pcie {
int reset_gpio;
struct clk *pcie_bus;
struct clk *pcie_phy;
+ struct clk *pcie_inbound_axi;
struct clk *pcie;
struct pcie_port pp;
struct regmap *iomuxc_gpr;
+ struct regmap *gpc_ips_reg;
+ struct regulator *pcie_phy_regulator;
void __iomem *mem_base;
};

@@ -77,6 +81,18 @@ 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)

+/* GPC PCIE PHY bit definitions */
+#define GPC_CNTR 0
+#define GPC_CNTR_PCIE_PHY_PUP_REQ BIT(7)
+
+static inline bool is_imx6sx_pcie(struct imx6_pcie *imx6_pcie)
+{
+ struct pcie_port *pp = &imx6_pcie->pp;
+ struct device_node *np = pp->dev->of_node;
+
+ return of_device_is_compatible(np, "fsl,imx6sx-pcie");
+}
+
static int pcie_phy_poll_ack(void __iomem *dbi_base, int exp_val)
{
u32 val;
@@ -275,18 +291,30 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
goto err_pcie;
}

- /* 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);
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ ret = clk_prepare_enable(imx6_pcie->pcie_inbound_axi);
+ if (ret) {
+ dev_err(pp->dev, "unable to enable pcie clock\n");
+ goto err_inbound_axi;
+ }
+
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ IMX6SX_GPR12_PCIE_TEST_PD, 0);
+ } else {
+ /* power up core phy and enable ref clock */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
+ IMX6Q_GPR1_PCIE_TEST_PD, 0);
+ /*
+ * 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,
+ IMX6Q_GPR1_PCIE_REF_CLK_EN);
+ }

/* allow the clocks to stabilize */
usleep_range(200, 500);
@@ -297,8 +325,19 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
msleep(100);
gpio_set_value(imx6_pcie->reset_gpio, 1);
}
+
+ /*
+ * Release the PCIe PHY reset here, that we have set in
+ * imx6_pcie_init_phy() now
+ */
+ if (is_imx6sx_pcie(imx6_pcie))
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
+ IMX6SX_GPR5_PCIE_BTNRST, 0);
+
return 0;

+err_inbound_axi:
+ clk_disable_unprepare(imx6_pcie->pcie);
err_pcie:
clk_disable_unprepare(imx6_pcie->pcie_bus);
err_pcie_bus:
@@ -308,9 +347,31 @@ err_pcie_phy:

}

-static void imx6_pcie_init_phy(struct pcie_port *pp)
+static int imx6_pcie_init_phy(struct pcie_port *pp)
{
struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
+ int ret;
+
+ /* Power up the separate domain available on i.MX6SX */
+ 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, GPC_CNTR,
+ GPC_CNTR_PCIE_PHY_PUP_REQ,
+ GPC_CNTR_PCIE_PHY_PUP_REQ);
+ regulator_set_voltage(imx6_pcie->pcie_phy_regulator,
+ 1100000, 1100000);
+ ret = regulator_enable(imx6_pcie->pcie_phy_regulator);
+ if (ret) {
+ dev_err(pp->dev, "failed to enable pcie regulator.\n");
+ return ret;
+ }
+ 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);
@@ -319,7 +380,7 @@ static void imx6_pcie_init_phy(struct pcie_port *pp)
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);
@@ -331,6 +392,8 @@ static void imx6_pcie_init_phy(struct pcie_port *pp)
IMX6Q_GPR8_TX_SWING_FULL, 127 << 18);
regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
IMX6Q_GPR8_TX_SWING_LOW, 127 << 25);
+
+ return 0;
}

static int imx6_pcie_wait_for_link(struct pcie_port *pp)
@@ -377,7 +440,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)
@@ -422,13 +486,19 @@ static int imx6_pcie_start_link(struct pcie_port *pp)
return ret;
}

-static void imx6_pcie_host_init(struct pcie_port *pp)
+static int imx6_pcie_host_init(struct pcie_port *pp)
{
+ int ret;
+
imx6_pcie_assert_core_reset(pp);

- imx6_pcie_init_phy(pp);
+ ret = imx6_pcie_init_phy(pp);
+ if (ret < 0)
+ return ret;

- imx6_pcie_deassert_core_reset(pp);
+ ret = imx6_pcie_deassert_core_reset(pp);
+ if (ret < 0)
+ return ret;

dw_pcie_setup_rc(pp);

@@ -436,6 +506,8 @@ static void imx6_pcie_host_init(struct pcie_port *pp)

if (IS_ENABLED(CONFIG_PCI_MSI))
dw_pcie_msi_init(pp);
+
+ return 0;
}

static void imx6_pcie_reset_phy(struct pcie_port *pp)
@@ -553,6 +625,75 @@ static int __init imx6_add_pcie_port(struct pcie_port *pp,
return 0;
}

+#ifdef CONFIG_PM_SLEEP
+static int pci_imx_suspend_noirq(struct device *dev)
+{
+ struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+ struct pcie_port *pp = &imx6_pcie->pp;
+
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ if (IS_ENABLED(CONFIG_PCI_MSI))
+ dw_pcie_msi_cfg_store(pp);
+
+ /* PM_TURN_OFF */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ IMX6SX_GPR12_PCIE_PM_TURN_OFF,
+ IMX6SX_GPR12_PCIE_PM_TURN_OFF);
+ udelay(10);
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ IMX6SX_GPR12_PCIE_PM_TURN_OFF, 0);
+ clk_disable_unprepare(imx6_pcie->pcie);
+ clk_disable_unprepare(imx6_pcie->pcie_bus);
+ clk_disable_unprepare(imx6_pcie->pcie_phy);
+ clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
+ }
+
+ return 0;
+}
+
+static int pci_imx_resume_noirq(struct device *dev)
+{
+ struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+ struct pcie_port *pp = &imx6_pcie->pp;
+
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ clk_prepare_enable(imx6_pcie->pcie_inbound_axi);
+ clk_prepare_enable(imx6_pcie->pcie_bus);
+ clk_prepare_enable(imx6_pcie->pcie_phy);
+ clk_prepare_enable(imx6_pcie->pcie);
+
+ /* Reset iMX6SX PCIe */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
+ IMX6SX_GPR5_PCIE_PERST, IMX6SX_GPR5_PCIE_PERST);
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
+ IMX6SX_GPR5_PCIE_PERST, 0);
+ /*
+ * controller maybe turn off, re-configure again
+ */
+ dw_pcie_setup_rc(pp);
+
+ if (IS_ENABLED(CONFIG_PCI_MSI))
+ dw_pcie_msi_cfg_restore(pp);
+
+ /* RESET EP */
+ gpio_set_value(imx6_pcie->reset_gpio, 0);
+ udelay(10);
+ gpio_set_value(imx6_pcie->reset_gpio, 1);
+ }
+
+ return 0;
+}
+
+static const struct dev_pm_ops pci_imx_pm_ops = {
+ .suspend_noirq = pci_imx_suspend_noirq,
+ .resume_noirq = pci_imx_resume_noirq,
+ .freeze_noirq = pci_imx_suspend_noirq,
+ .thaw_noirq = pci_imx_resume_noirq,
+ .poweroff_noirq = pci_imx_suspend_noirq,
+ .restore_noirq = pci_imx_resume_noirq,
+};
+#endif
+
static int __init imx6_pcie_probe(struct platform_device *pdev)
{
struct imx6_pcie *imx6_pcie;
@@ -610,9 +751,28 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
return PTR_ERR(imx6_pcie->pcie);
}

- /* Grab GPR config register range */
- imx6_pcie->iomuxc_gpr =
- syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ imx6_pcie->pcie_inbound_axi = devm_clk_get(&pdev->dev,
+ "pcie_inbound_axi");
+ if (IS_ERR(imx6_pcie->pcie_inbound_axi)) {
+ dev_err(&pdev->dev,
+ "pcie clock source missing or invalid\n");
+ return PTR_ERR(imx6_pcie->pcie_inbound_axi);
+ }
+
+ imx6_pcie->pcie_phy_regulator = devm_regulator_get(pp->dev,
+ "pcie-phy");
+
+ imx6_pcie->iomuxc_gpr =
+ syscon_regmap_lookup_by_compatible
+ ("fsl,imx6sx-iomuxc-gpr");
+ 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);
@@ -636,6 +796,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", },
{},
};
MODULE_DEVICE_TABLE(of, imx6_pcie_of_match);
@@ -645,6 +806,7 @@ static struct platform_driver imx6_pcie_driver = {
.name = "imx6q-pcie",
.owner = THIS_MODULE,
.of_match_table = imx6_pcie_of_match,
+ .pm = &pci_imx_pm_ops,
},
.shutdown = imx6_pcie_shutdown,
};
--
1.9.1
Lucas Stach
2014-10-16 10:54:09 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.
- register one PM call-back, enter/exit L2 state during
system suspend/resume.
use noirq pm_ops instead of the general pm_ops in dev_pm_ops,
since cfg read/write may occurs after suspend and before resume.
do msi store/re-store in suspend/resume callbacks, since
controller maybe turned off, and these msi cfg maybe lost
in suspend.
- disp_axi clock is required by pcie inbound axi port actually.
Add one more clock named pcie_inbound_axi for imx6sx pcie.
- host init maybe failed, return negative value when
there is a failure in the host init.
---
drivers/pci/host/pci-imx6.c | 204 +++++++++++++++++++++++++++++++++++++++-----
1 file changed, 183 insertions(+), 21 deletions(-)
[...]
Post by Richard Zhu
+#ifdef CONFIG_PM_SLEEP
+static int pci_imx_suspend_noirq(struct device *dev)
+{
+ struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+ struct pcie_port *pp = &imx6_pcie->pp;
+
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ if (IS_ENABLED(CONFIG_PCI_MSI))
+ dw_pcie_msi_cfg_store(pp);
+
+ /* PM_TURN_OFF */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ IMX6SX_GPR12_PCIE_PM_TURN_OFF,
+ IMX6SX_GPR12_PCIE_PM_TURN_OFF);
+ udelay(10);
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ IMX6SX_GPR12_PCIE_PM_TURN_OFF, 0);
+ clk_disable_unprepare(imx6_pcie->pcie);
+ clk_disable_unprepare(imx6_pcie->pcie_bus);
+ clk_disable_unprepare(imx6_pcie->pcie_phy);
+ clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
+ }
+
+ return 0;
+}
+
+static int pci_imx_resume_noirq(struct device *dev)
+{
+ struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+ struct pcie_port *pp = &imx6_pcie->pp;
+
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ clk_prepare_enable(imx6_pcie->pcie_inbound_axi);
+ clk_prepare_enable(imx6_pcie->pcie_bus);
+ clk_prepare_enable(imx6_pcie->pcie_phy);
+ clk_prepare_enable(imx6_pcie->pcie);
+
+ /* Reset iMX6SX PCIe */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
+ IMX6SX_GPR5_PCIE_PERST, IMX6SX_GPR5_PCIE_PERST);
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
+ IMX6SX_GPR5_PCIE_PERST, 0);
+ /*
+ * controller maybe turn off, re-configure again
+ */
+ dw_pcie_setup_rc(pp);
+
+ if (IS_ENABLED(CONFIG_PCI_MSI))
+ dw_pcie_msi_cfg_restore(pp);
+
+ /* RESET EP */
+ gpio_set_value(imx6_pcie->reset_gpio, 0);
+ udelay(10);
You are violating the spec here: "When PERST# is asserted it must remain
active for a minimum of 100 us (Tperst)." Also you probably want to put
the EP in reset at suspend and only release the reset in the resume
path.
Post by Richard Zhu
+ gpio_set_value(imx6_pcie->reset_gpio, 1);
+ }
+
+ return 0;
+}
+
+static const struct dev_pm_ops pci_imx_pm_ops = {
+ .suspend_noirq = pci_imx_suspend_noirq,
+ .resume_noirq = pci_imx_resume_noirq,
+ .freeze_noirq = pci_imx_suspend_noirq,
+ .thaw_noirq = pci_imx_resume_noirq,
+ .poweroff_noirq = pci_imx_suspend_noirq,
+ .restore_noirq = pci_imx_resume_noirq,
+};
+#endif
+
static int __init imx6_pcie_probe(struct platform_device *pdev)
{
struct imx6_pcie *imx6_pcie;
@@ -610,9 +751,28 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
return PTR_ERR(imx6_pcie->pcie);
}
- /* Grab GPR config register range */
- imx6_pcie->iomuxc_gpr =
- syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ imx6_pcie->pcie_inbound_axi = devm_clk_get(&pdev->dev,
+ "pcie_inbound_axi");
+ if (IS_ERR(imx6_pcie->pcie_inbound_axi)) {
+ dev_err(&pdev->dev,
+ "pcie clock source missing or invalid\n");
+ return PTR_ERR(imx6_pcie->pcie_inbound_axi);
+ }
+
+ imx6_pcie->pcie_phy_regulator = devm_regulator_get(pp->dev,
+ "pcie-phy");
+
+ imx6_pcie->iomuxc_gpr =
+ syscon_regmap_lookup_by_compatible
+ ("fsl,imx6sx-iomuxc-gpr");
+ 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);
@@ -636,6 +796,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", },
{},
};
MODULE_DEVICE_TABLE(of, imx6_pcie_of_match);
@@ -645,6 +806,7 @@ static struct platform_driver imx6_pcie_driver = {
.name = "imx6q-pcie",
.owner = THIS_MODULE,
.of_match_table = imx6_pcie_of_match,
+ .pm = &pci_imx_pm_ops,
},
.shutdown = imx6_pcie_shutdown,
};
H***@freescale.com
2014-10-17 02:11:15 UTC
Permalink
-----Original Message-----
Sent: Thursday, October 16, 2014 6:54 PM
To: Richard Zhu
Subject: Re: [PATCH v6 09/13] 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.
- register one PM call-back, enter/exit L2 state during system
suspend/resume.
use noirq pm_ops instead of the general pm_ops in dev_pm_ops, since
cfg read/write may occurs after suspend and before resume.
do msi store/re-store in suspend/resume callbacks, since controller
maybe turned off, and these msi cfg maybe lost in suspend.
- disp_axi clock is required by pcie inbound axi port actually.
Add one more clock named pcie_inbound_axi for imx6sx pcie.
- host init maybe failed, return negative value when there is a
failure in the host init.
---
drivers/pci/host/pci-imx6.c | 204
+++++++++++++++++++++++++++++++++++++++-----
1 file changed, 183 insertions(+), 21 deletions(-)
[...]
Post by Richard Zhu
+#ifdef CONFIG_PM_SLEEP
+static int pci_imx_suspend_noirq(struct device *dev) {
+ struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+ struct pcie_port *pp = &imx6_pcie->pp;
+
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ if (IS_ENABLED(CONFIG_PCI_MSI))
+ dw_pcie_msi_cfg_store(pp);
+
+ /* PM_TURN_OFF */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ IMX6SX_GPR12_PCIE_PM_TURN_OFF,
+ IMX6SX_GPR12_PCIE_PM_TURN_OFF);
+ udelay(10);
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ IMX6SX_GPR12_PCIE_PM_TURN_OFF, 0);
+ clk_disable_unprepare(imx6_pcie->pcie);
+ clk_disable_unprepare(imx6_pcie->pcie_bus);
+ clk_disable_unprepare(imx6_pcie->pcie_phy);
+ clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
+ }
+
+ return 0;
+}
+
+static int pci_imx_resume_noirq(struct device *dev) {
+ struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+ struct pcie_port *pp = &imx6_pcie->pp;
+
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ clk_prepare_enable(imx6_pcie->pcie_inbound_axi);
+ clk_prepare_enable(imx6_pcie->pcie_bus);
+ clk_prepare_enable(imx6_pcie->pcie_phy);
+ clk_prepare_enable(imx6_pcie->pcie);
+
+ /* Reset iMX6SX PCIe */
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
+ IMX6SX_GPR5_PCIE_PERST, IMX6SX_GPR5_PCIE_PERST);
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
+ IMX6SX_GPR5_PCIE_PERST, 0);
+ /*
+ * controller maybe turn off, re-configure again
+ */
+ dw_pcie_setup_rc(pp);
+
+ if (IS_ENABLED(CONFIG_PCI_MSI))
+ dw_pcie_msi_cfg_restore(pp);
+
+ /* RESET EP */
+ gpio_set_value(imx6_pcie->reset_gpio, 0);
+ udelay(10);
You are violating the spec here: "When PERST# is asserted it must remain
active for a minimum of 100 us (Tperst)." Also you probably want to put the EP
in reset at suspend and only release the reset in the resume path.
[Richard] Oops, sorry. Would be changed immediately.
Assert the PERST# in suspend, and de-assert it in resume.
Thanks a lot for your detailed review.
Post by Richard Zhu
+ gpio_set_value(imx6_pcie->reset_gpio, 1);
+ }
+
+ return 0;
+}
+
+static const struct dev_pm_ops pci_imx_pm_ops = {
+ .suspend_noirq = pci_imx_suspend_noirq,
+ .resume_noirq = pci_imx_resume_noirq,
+ .freeze_noirq = pci_imx_suspend_noirq,
+ .thaw_noirq = pci_imx_resume_noirq,
+ .poweroff_noirq = pci_imx_suspend_noirq,
+ .restore_noirq = pci_imx_resume_noirq, }; #endif
+
static int __init imx6_pcie_probe(struct platform_device *pdev) {
struct imx6_pcie *imx6_pcie;
@@ -610,9 +751,28 @@ static int __init imx6_pcie_probe(struct
platform_device *pdev)
Post by Richard Zhu
return PTR_ERR(imx6_pcie->pcie);
}
- /* Grab GPR config register range */
- imx6_pcie->iomuxc_gpr =
- syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+ if (is_imx6sx_pcie(imx6_pcie)) {
+ imx6_pcie->pcie_inbound_axi = devm_clk_get(&pdev->dev,
+ "pcie_inbound_axi");
+ if (IS_ERR(imx6_pcie->pcie_inbound_axi)) {
+ dev_err(&pdev->dev,
+ "pcie clock source missing or invalid\n");
+ return PTR_ERR(imx6_pcie->pcie_inbound_axi);
+ }
+
+ imx6_pcie->pcie_phy_regulator = devm_regulator_get(pp->dev,
+ "pcie-phy");
+
+ imx6_pcie->iomuxc_gpr =
+ syscon_regmap_lookup_by_compatible
+ ("fsl,imx6sx-iomuxc-gpr");
+ 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");
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", },
{},
};
static struct platform_driver imx6_pcie_driver = {
.name = "imx6q-pcie",
.owner = THIS_MODULE,
.of_match_table = imx6_pcie_of_match,
+ .pm = &pci_imx_pm_ops,
},
.shutdown = imx6_pcie_shu
Richard Zhu
2014-10-16 07:52:41 UTC
Permalink
From: Richard Zhu <***@freescale.com>

- imx6sx pcie phy has its own power regulator. Add the
pcie phy power suppy into im6sx pcie dts and binding.
- in order to align with imx6qdl's pcie dts, re-format
imx6sx pcie dts.
- in order to align with imx6qdl pcie dts format and
keep clean of imx6 pcie driver, keep the pcie phy clock
in imx6sx pcie dts, although it's the parent clk of the
pcie bus clock now, and would be enabled automatically
when pcie bus clock is enabled. secondly, it's
possible that the external osc maybe used as source
of the pcie_bus clk in board design in future.
- disp_axi clock is required by pcie inbound axi port.
Add one more clock named pcie_inbound_axi for imx6sx pcie.

Signed-off-by: Richard Zhu <***@freescale.com>
---
.../devicetree/bindings/pci/fsl,imx6q-pcie.txt | 8 +++++-
arch/arm/boot/dts/imx6sx.dtsi | 32 ++++++++++++----------
2 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
index 9455fd0..ad81179 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"
+- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie"
- reg: base addresse 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.
@@ -13,6 +13,12 @@ Required properties:
- clock-names: Must include the following additional entries:
- "pcie_phy"

+Additional required properties for imx6sx-pcie:
+- clock names: Must include the following additional entries:
+ - "pcie_inbound_axi"
+- power supplies:
+ - pcie-phy-supply: regulator used to power the PCIe PHY
+
Example:

***@0x01000000 {
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index f4b9da6..0dfeade 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -599,9 +599,9 @@
anatop-max-voltage = <1450000>;
};

- reg_pcie: regulator-***@140 {
+ reg_pcie_phy: regulator-vddpcie-***@140 {
compatible = "fsl,anatop-regulator";
- regulator-name = "vddpcie";
+ regulator-name = "vddpcie-phy";
regulator-min-microvolt = <725000>;
regulator-max-microvolt = <1450000>;
anatop-reg-offset = <0x140>;
@@ -1184,24 +1184,28 @@

pcie: ***@0x08000000 {
compatible = "fsl,imx6sx-pcie", "snps,dw-pcie";
- reg = <0x08ffc000 0x4000>; /* DBI */
+ reg = <0x08ffc000 0x4000>, <0x08f00000 0x80000>;
+ reg-names = "dbi", "config";
#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 = <0x81000000 0 0 0x08f80000 0 0x00010000 /* downstream I/O */
+ 0x82000000 0 0x08000000 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>,
+ 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_LVDS1_OUT>,
+ <&clks IMX6SX_CLK_PCIE_REF_125M>,
<&clks IMX6SX_CLK_DISPLAY_AXI>;
- clock-names = "pcie_ref_125m", "pcie_axi",
- "lvds_gate", "display_axi";
+ clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_inbound_axi";
+ pcie-phy-supply = <&reg_pcie_phy>;
status = "disabled";
};
};
--
1.9.1
Loading...