Skip to contentSkip to conversation

PCI: imx: Initial imx7d pm support

PATCH Applied Leonard Crestez 4 participants 15 messages Jul 24, 2018 – Aug 21, 2018

Subsystems (5)
Device tree bindings Arm i.MX and Layerscape PCI PCI native host bridge and endpoint drivers +1 more subsystems

v3 of 4 Applied Latest is v4 →

lore
Share & tools
lore

Leonard Crestez

author
#0 lore raw

v3 cover PCI: imx: Initial imx7d pm support

Changes since v2:
* Print with dev_info if link fails on resume (Lucas)
* Add a comment on imx7d pci irq mappings (Andrey)
* Make imx6_pcie_ltssm_disable print an error on IMX6Q (Lucas)

The ltssm_disable does not return an error because it can't be usefully handled, reversing partial suspend is a nightmare and unlikely to work.

* Drop "reset: imx7: Fix always writing bits as 0 (accepted by Philipp)

Series is against linux-next tag next-20180724 where the reset patch was already accepted. The imx7d.dtsi patch is also useful standalone.

Link to v2: https://lkml.org/lkml/2018/7/20/472

Leonard Crestez (2):
Revert "ARM: dts: imx7d: Invert legacy PCI irq mapping"
PCI: imx: Initial imx7d pm support

 arch/arm/boot/dts/imx7d.dtsi          | 12 ++--
 drivers/pci/controller/dwc/pci-imx6.c | 97 +++++++++++++++++++++++++--
 2 files changed, 100 insertions(+), 9 deletions(-)
Signature · 1 line
2.17.1
author
#1 lore raw

PATCH v3 1/2 Revert "ARM: dts: imx7d: Invert legacy PCI irq mapping"

This reverts commit 1c86c9dd82f859b474474a7fee0d5195da2c9c1d.

That commit followed the reference manual but unfortunately the imx7d manual is incorrect.

Tested with ath9k pcie card and confirmed internally.

Signed-off-by Leonard Crestez <[email protected]>
---
 arch/arm/boot/dts/imx7d.dtsi | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
arch/arm/boot/dts/imx7d.dtsi +8 -4
index 7cbc2ffa4b3a..7234e8330a57 100644
@@ -124,14 +124,18 @@
 		num-lanes = <1>;
 		interrupts = <GIC_SPI 122 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 122 IRQ_TYPE_LEVEL_HIGH>,
-				<0 0 0 2 &intc GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
-				<0 0 0 3 &intc GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
-				<0 0 0 4 &intc GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
+		/*
+		 * Reference manual lists pci irqs incorrectly
+		 * Real hardware ordering is same as imx6: D+MSI, C, B, A
+		 */
+		interrupt-map = <0 0 0 1 &intc GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
+				<0 0 0 2 &intc GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
+				<0 0 0 3 &intc GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
+				<0 0 0 4 &intc GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&clks IMX7D_PCIE_CTRL_ROOT_CLK>,
 			 <&clks IMX7D_PLL_ENET_MAIN_100M_CLK>,
 			 <&clks IMX7D_PCIE_PHY_ROOT_CLK>;
 		clock-names = "pcie", "pcie_bus", "pcie_phy";
 		assigned-clocks = <&clks IMX7D_PCIE_CTRL_ROOT_SRC>,
Signature · 1 line
2.17.1
maintainer Acked-by on v3 1/2
#2 lore raw

Re: [PATCH v3 1/2] Revert "ARM: dts: imx7d: Invert legacy PCI irq mapping"

Am Dienstag, den 24.07.2018, 19:14 +0300 schrieb Leonard Crestez:

Quoted 8 lines

This reverts commit 1c86c9dd82f859b474474a7fee0d5195da2c9c1d.

That commit followed the reference manual but unfortunately the imx7d manual is incorrect.

Tested with ath9k pcie card and confirmed internally.

Signed-off-by Leonard Crestez <[email protected]>
Acked-by Lucas Stach <[email protected]>
Quoted 31 lines

---
?arch/arm/boot/dts/imx7d.dtsi | 12 ++++++++----
?1 file changed, 8 insertions(+), 4 deletions(-)

arch/arm/boot/dts/imx7d.dtsi +0 -0
index 7cbc2ffa4b3a..7234e8330a57 100644
@@ -124,14 +124,18 @@
 > ?		num-lanes = <1>;
 > ?		interrupts = <GIC_SPI 122 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 122 IRQ_TYPE_LEVEL_HIGH>,
 > -				<0 0 0 2 &intc GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
 > -				<0 0 0 3 &intc GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
 > -				<0 0 0 4 &intc GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
Quoted 13 lines
arch/arm/boot/dts/imx7d.dtsi +8 -0
+		/*
+		?* Reference manual lists pci irqs incorrectly
+		?* Real hardware ordering is same as imx6: D+MSI, C, B, A
+		?*/
+		interrupt-map = <0 0 0 1 &intc GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
+				<0 0 0 2 &intc GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
+				<0 0 0 3 &intc GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
+				<0 0 0 4 &intc GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>;

? clocks = <&clks IMX7D_PCIE_CTRL_ROOT_CLK>,
? ?<&clks IMX7D_PLL_ENET_MAIN_100M_CLK>,
? ?<&clks IMX7D_PCIE_PHY_ROOT_CLK>;
? clock-names = "pcie", "pcie_bus", "pcie_phy";
? assigned-clocks = <&clks IMX7D_PCIE_CTRL_ROOT_SRC>,

Applied
#3 lore raw

Re: [PATCH v3 1/2] Revert "ARM: dts: imx7d: Invert legacy PCI irq mapping"

In reply to Leonard Crestez
Quoted 8 lines · Leonard Crestez

This reverts commit 1c86c9dd82f859b474474a7fee0d5195da2c9c1d.

That commit followed the reference manual but unfortunately the imx7d manual is incorrect.

Tested with ath9k pcie card and confirmed internally.

Signed-off-by Leonard Crestez <[email protected]>

Applied, thanks.

author
#4 lore raw

PATCH v3 2/2 PCI: imx: Initial imx7d pm support

In reply to Leonard Crestez

On imx7d the pcie-phy power domain is turned off in suspend and this can make the system hang after resume when attempting any read from PCI.

Fix this by adding minimal suspend/resume code from the nxp internal tree. This will prepare for powering down on suspend and reset the block on resume.

Code is only for imx7d but a very similar sequence can be used for other socs.

The original author is mostly Richard Zhu <[email protected]>, this patch adjusts the code to the upstream imx7d implemention using reset controls and power domains.

Signed-off-by Leonard Crestez <[email protected]>
---
 drivers/pci/controller/dwc/pci-imx6.c | 97 +++++++++++++++++++++++++--
 1 file changed, 92 insertions(+), 5 deletions(-)
drivers/pci/controller/dwc/pci-imx6.c +92 -5
index 4a9a673b4777..65b6d1015723 100644
@@ -540,10 +540,28 @@ static int imx6_pcie_wait_for_speed_change(struct imx6_pcie *imx6_pcie)
 
 	dev_err(dev, "Speed change timeout\n");
 	return -EINVAL;
 }
 
+static void imx6_pcie_ltssm_enable(struct device *dev)
+{
+	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+
+	switch (imx6_pcie->variant) {
+	case IMX6Q:
+	case IMX6SX:
+	case IMX6QP:
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+				   IMX6Q_GPR12_PCIE_CTL_2,
+				   IMX6Q_GPR12_PCIE_CTL_2);
+		break;
+	case IMX7D:
+		reset_control_deassert(imx6_pcie->apps_reset);
+		break;
+	}
+}
+
 static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
 {
 	struct dw_pcie *pci = imx6_pcie->pci;
 	struct device *dev = pci->dev;
 	u32 tmp;

@@ -558,15 +576,11 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
 	tmp &= ~PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK;
 	tmp |= PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1;
 	dw_pcie_writel_dbi(pci, PCIE_RC_LCR, tmp);
 
 	/* Start LTSSM. */
-	if (imx6_pcie->variant == IMX7D)
-		reset_control_deassert(imx6_pcie->apps_reset);
-	else
-		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-				   IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
+	imx6_pcie_ltssm_enable(dev);
 
 	ret = imx6_pcie_wait_for_link(imx6_pcie);
 	if (ret)
 		goto err_reset_phy;
 

@@ -680,10 +694,82 @@ static int imx6_add_pcie_port(struct imx6_pcie *imx6_pcie,
 
 static const struct dw_pcie_ops dw_pcie_ops = {
 	.link_up = imx6_pcie_link_up,
 };
 
+#ifdef CONFIG_PM_SLEEP
+static void imx6_pcie_ltssm_disable(struct device *dev)
+{
+	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+
+	switch (imx6_pcie->variant) {
+	case IMX6SX:
+	case IMX6QP:
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+				   IMX6Q_GPR12_PCIE_CTL_2, 0);
+		break;
+	case IMX7D:
+		reset_control_assert(imx6_pcie->apps_reset);
+		break;
+	default:
+		dev_err(dev, "ltssm_disable not supported\n");
+	}
+}
+
+static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
+{
+	clk_disable_unprepare(imx6_pcie->pcie);
+	clk_disable_unprepare(imx6_pcie->pcie_phy);
+	clk_disable_unprepare(imx6_pcie->pcie_bus);
+
+	if (imx6_pcie->variant == IMX7D) {
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
+				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
+	}
+}
+
+static int imx6_pcie_suspend_noirq(struct device *dev)
+{
+	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+
+	if (imx6_pcie->variant != IMX7D)
+		return 0;
+
+	imx6_pcie_clk_disable(imx6_pcie);
+	imx6_pcie_ltssm_disable(dev);
+
+	return 0;
+}
+
+static int imx6_pcie_resume_noirq(struct device *dev)
+{
+	int ret;
+	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+	struct pcie_port *pp = &imx6_pcie->pci->pp;
+
+	if (imx6_pcie->variant != IMX7D)
+		return 0;
+
+	imx6_pcie_assert_core_reset(imx6_pcie);
+	imx6_pcie_init_phy(imx6_pcie);
+	imx6_pcie_deassert_core_reset(imx6_pcie);
+	dw_pcie_setup_rc(pp);
+
+	ret = imx6_pcie_establish_link(imx6_pcie);
+	if (ret < 0)
+		dev_info(dev, "pcie link is down after resume.\n");
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops imx6_pcie_pm_ops = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx6_pcie_suspend_noirq,
+				      imx6_pcie_resume_noirq)
+};
+
 static int imx6_pcie_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct dw_pcie *pci;
 	struct imx6_pcie *imx6_pcie;

@@ -846,10 +932,11 @@ static const struct of_device_id imx6_pcie_of_match[] = {
 static struct platform_driver imx6_pcie_driver = {
 	.driver = {
 		.name	= "imx6q-pcie",
 		.of_match_table = imx6_pcie_of_match,
 		.suppress_bind_attrs = true,
+		.pm = &imx6_pcie_pm_ops,
 	},
 	.probe    = imx6_pcie_probe,
 	.shutdown = imx6_pcie_shutdown,
 };
Signature · 1 line
2.17.1
maintainer Reviewed-by on v3 2/2
#5 lore raw

Am Dienstag, den 24.07.2018, 19:14 +0300 schrieb Leonard Crestez:

Quoted 7 lines

On imx7d the pcie-phy power domain is turned off in suspend and this can make the system hang after resume when attempting any read from PCI.

Fix this by adding minimal suspend/resume code from the nxp internal tree. This will prepare for powering down on suspend and reset the block on resume.

Quoted 8 lines

Code is only for imx7d but a very similar sequence can be used for other socs.

Quoted 1 line

The original author is mostly Richard Zhu <[email protected]>, this

patch adjusts the code to the upstream imx7d implemention using reset controls and power domains.

Signed-off-by Leonard Crestez <[email protected]>
Reviewed-by Lucas Stach <[email protected]>
Quoted 149 lines

---
?drivers/pci/controller/dwc/pci-imx6.c | 97 +++++++++++++++++++++++++-- ?1 file changed, 92 insertions(+), 5 deletions(-)

drivers/pci/controller/dwc/pci-imx6.c +0 -0
index 4a9a673b4777..65b6d1015723 100644
@@ -540,10 +540,28 @@ static int imx6_pcie_wait_for_speed_change(struct imx6_pcie *imx6_pcie)

?

Quoted 2 lines

? dev_err(dev, "Speed change timeout\n");
? return -EINVAL;

?}
?

drivers/pci/controller/dwc/pci-imx6.c +2 -0
+static void imx6_pcie_ltssm_enable(struct device *dev)
+{

Review

Quoted 1 line
(quoted diff) +1 -0
+	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);

+

Quoted 12 lines
drivers/pci/controller/dwc/pci-imx6.c +12 -0
+	switch (imx6_pcie->variant) {
+	case IMX6Q:
+	case IMX6SX:
+	case IMX6QP:
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+				???IMX6Q_GPR12_PCIE_CTL_2,
+				???IMX6Q_GPR12_PCIE_CTL_2);
+		break;
+	case IMX7D:
+		reset_control_deassert(imx6_pcie->apps_reset);
+		break;
+	}
drivers/pci/controller/dwc/pci-imx6.c +2 -0
+}
+

?static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie) ?{

Quoted 3 lines

? struct dw_pcie *pci = imx6_pcie->pci;
? struct device *dev = pci->dev;
? u32 tmp;

drivers/pci/controller/dwc/pci-imx6.c +0 -0
@@ -558,15 +576,11 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
 > ?	tmp &= ~PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK;
 > ?	tmp |= PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1;
 > ?	dw_pcie_writel_dbi(pci, PCIE_RC_LCR, tmp);

?

drivers/pci/controller/dwc/pci-imx6.c static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie) · line 581

Quoted 7 lines

? /* Start LTSSM. */

drivers/pci/controller/dwc/pci-imx6.c +1 -5
-	if (imx6_pcie->variant == IMX7D)
-		reset_control_deassert(imx6_pcie->apps_reset);
-	else
-		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-				???IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
+	imx6_pcie_ltssm_enable(dev);

?

Quoted 3 lines

? ret = imx6_pcie_wait_for_link(imx6_pcie);
? if (ret)
? goto err_reset_phy;

?

drivers/pci/controller/dwc/pci-imx6.c +0 -0
@@ -680,10 +694,82 @@ static int imx6_add_pcie_port(struct imx6_pcie *imx6_pcie,

?
?static const struct dw_pcie_ops dw_pcie_ops = {

Quoted 1 line

? .link_up = imx6_pcie_link_up,

?};
?

drivers/pci/controller/dwc/pci-imx6.c +3 -0
+#ifdef CONFIG_PM_SLEEP
+static void imx6_pcie_ltssm_disable(struct device *dev)
+{

Review

Quoted 1 line
(quoted diff) +1 -0
+	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);

+

Quoted 12 lines
drivers/pci/controller/dwc/pci-imx6.c +12 -0
+	switch (imx6_pcie->variant) {
+	case IMX6SX:
+	case IMX6QP:
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+				???IMX6Q_GPR12_PCIE_CTL_2, 0);
+		break;
+	case IMX7D:
+		reset_control_assert(imx6_pcie->apps_reset);
+		break;
+	default:
+		dev_err(dev, "ltssm_disable not supported\n");
+	}
drivers/pci/controller/dwc/pci-imx6.c +4 -0
+}
+
+static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
+{

drivers/pci/controller/dwc/pci-imx6.c static int imx6_add_pcie_port(struct imx6_pcie *imx6_pcie, · line 722

Quoted 3 lines
drivers/pci/controller/dwc/pci-imx6.c +3 -0
+	clk_disable_unprepare(imx6_pcie->pcie);
+	clk_disable_unprepare(imx6_pcie->pcie_phy);
+	clk_disable_unprepare(imx6_pcie->pcie_bus);

+

Quoted 5 lines
drivers/pci/controller/dwc/pci-imx6.c +5 -0
+	if (imx6_pcie->variant == IMX7D) {
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+				???IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
+				???IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
+	}
drivers/pci/controller/dwc/pci-imx6.c +4 -0
+}
+
+static int imx6_pcie_suspend_noirq(struct device *dev)
+{

Review

Quoted 1 line
(quoted diff) +1 -0
+	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);

+

Review

Quoted 2 lines
(quoted diff) +2 -0
+	if (imx6_pcie->variant != IMX7D)
+		return 0;

+

drivers/pci/controller/dwc/pci-imx6.c static int imx6_add_pcie_port(struct imx6_pcie *imx6_pcie, · line 739

Quoted 2 lines
drivers/pci/controller/dwc/pci-imx6.c +2 -0
+	imx6_pcie_clk_disable(imx6_pcie);
+	imx6_pcie_ltssm_disable(dev);

+

Quoted 1 line
(quoted diff) +1 -0
+	return 0;
drivers/pci/controller/dwc/pci-imx6.c +4 -0
+}
+
+static int imx6_pcie_resume_noirq(struct device *dev)
+{

drivers/pci/controller/dwc/pci-imx6.c static int imx6_add_pcie_port(struct imx6_pcie *imx6_pcie, · line 748

Quoted 3 lines
drivers/pci/controller/dwc/pci-imx6.c +3 -0
+	int ret;
+	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+	struct pcie_port *pp = &imx6_pcie->pci->pp;

+

Review

Quoted 2 lines
(quoted diff) +2 -0
+	if (imx6_pcie->variant != IMX7D)
+		return 0;

+

drivers/pci/controller/dwc/pci-imx6.c static int imx6_add_pcie_port(struct imx6_pcie *imx6_pcie, · line 756

Quoted 4 lines
drivers/pci/controller/dwc/pci-imx6.c +4 -0
+	imx6_pcie_assert_core_reset(imx6_pcie);
+	imx6_pcie_init_phy(imx6_pcie);
+	imx6_pcie_deassert_core_reset(imx6_pcie);
+	dw_pcie_setup_rc(pp);

+

drivers/pci/controller/dwc/pci-imx6.c static int imx6_add_pcie_port(struct imx6_pcie *imx6_pcie, · line 760

Quoted 3 lines
drivers/pci/controller/dwc/pci-imx6.c +3 -0
+	ret = imx6_pcie_establish_link(imx6_pcie);
+	if (ret < 0)
+		dev_info(dev, "pcie link is down after resume.\n");

+

Quoted 1 line
(quoted diff) +1 -0
+	return 0;
drivers/pci/controller/dwc/pci-imx6.c +4 -0
+}
+#endif
+
+static const struct dev_pm_ops imx6_pcie_pm_ops = {
Quoted 2 lines
(quoted diff) +2 -0
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx6_pcie_suspend_noirq,
+				??????imx6_pcie_resume_noirq)
drivers/pci/controller/dwc/pci-imx6.c +2 -0
+};
+

?static int imx6_pcie_probe(struct platform_device *pdev) ?{

Quoted 3 lines

? struct device *dev = &pdev->dev;
? struct dw_pcie *pci;
? struct imx6_pcie *imx6_pcie;

drivers/pci/controller/dwc/pci-imx6.c +0 -0
@@ -846,10 +932,11 @@ static const struct of_device_id imx6_pcie_of_match[] = {
 ?static struct platform_driver imx6_pcie_driver = {
 > ?	.driver = {
 > > ?		.name	= "imx6q-pcie",
 > ?		.of_match_table = imx6_pcie_of_match,
 > ?		.suppress_bind_attrs = true,
 > +		.pm = &imx6_pcie_pm_ops,
 > ?	},
 > ?	.probe????= imx6_pcie_probe,
 > ?	.shutdown = imx6_pcie_shutdown,
 ?};

?

author
#6 lore raw
In reply to Leonard Crestez
Quoted 8 lines · Leonard Crestez

Changes since v2:
* Print with dev_info if link fails on resume (Lucas)
* Add a comment on imx7d pci irq mappings (Andrey)
* Make imx6_pcie_ltssm_disable print an error on IMX6Q (Lucas)

The ltssm_disable does not return an error because it can't be usefully handled, reversing partial suspend is a nightmare and unlikely to work.

Quoted 6 lines

* Drop "reset: imx7: Fix always writing bits as 0 (accepted by Philipp)

Series is against linux-next tag next-20180724 where the reset patch was already accepted. The imx7d.dtsi patch is also useful standalone.

Link to v2: https://lkml.org/lkml/2018/7/20/472

This is a gentle reminder that this series was reviewed by Lucas two weeks ago but not yet included.

--
Regards,
Leonard

#7 lore raw
Quoted 11 lines · Leonard Crestez
Quoted 10 lines · Leonard Crestez

Changes since v2:
* Print with dev_info if link fails on resume (Lucas)
* Add a comment on imx7d pci irq mappings (Andrey)
* Make imx6_pcie_ltssm_disable print an error on IMX6Q (Lucas)

The ltssm_disable does not return an error because it can't be usefully handled, reversing partial suspend is a nightmare and unlikely to work.

* Drop "reset: imx7: Fix always writing bits as 0 (accepted by Philipp)

Quoted 7 lines
Quoted 4 lines

Series is against linux-next tag next-20180724 where the reset patch was already accepted. The imx7d.dtsi patch is also useful standalone.

Link to v2: https://lkml.org/lkml/2018/7/20/472

This is a gentle reminder that this series was reviewed by Lucas two weeks ago but not yet included.

Does this series have a functional dependency on the reset fix ? If yes we can have a bisection proplem depending on which tree gets merged first.

Please let me know.

Thanks,
Lorenzo

author
#8 lore raw
Quoted 15 lines · Lorenzo Pieralisi
Quoted 14 lines · Leonard Crestez
Quoted 13 lines · Leonard Crestez

Changes since v2:
* Print with dev_info if link fails on resume (Lucas)
* Add a comment on imx7d pci irq mappings (Andrey)
* Make imx6_pcie_ltssm_disable print an error on IMX6Q (Lucas)

The ltssm_disable does not return an error because it can't be usefully handled, reversing partial suspend is a nightmare and unlikely to work.

* Drop "reset: imx7: Fix always writing bits as 0 (accepted by Philipp)

Series is against linux-next tag next-20180724 where the reset patch was already accepted. The imx7d.dtsi patch is also useful standalone.

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

Link to v2: https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2018%2F7%2F20%2F472&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7Cc1e98512a50246c457a208d5fd2021dc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636693236811248777&amp;sdata=gxs1%2BfevIaBmQCVcJLUK41ml8CK2zLg%2FFKGGV%2F%2FHSLQ%3D&amp;reserved=0

This is a gentle reminder that this series was reviewed by Lucas two weeks ago but not yet included.

Does this series have a functional dependency on the reset fix ? If yes we can have a bisection proplem depending on which tree gets merged first.

Yes, without the reset fix I expect hangs.

Maybe the reset fix should be pulled in the pci tree? I don't know how these issues are sorted out.

#9 lore raw
Quoted 21 lines · Leonard Crestez
Quoted 20 lines · Lorenzo Pieralisi
Quoted 18 lines · Leonard Crestez
Quoted 8 lines · Leonard Crestez

Changes since v2:
* Print with dev_info if link fails on resume (Lucas)
* Add a comment on imx7d pci irq mappings (Andrey)
* Make imx6_pcie_ltssm_disable print an error on IMX6Q (Lucas)

The ltssm_disable does not return an error because it can't be usefully handled, reversing partial suspend is a nightmare and unlikely to work.

Quoted 6 lines

* Drop "reset: imx7: Fix always writing bits as 0 (accepted by Philipp)

Series is against linux-next tag next-20180724 where the reset patch was already accepted. The imx7d.dtsi patch is also useful standalone.

Link to v2: https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2018%2F7%2F20%2F472&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7Cc1e98512a50246c457a208d5fd2021dc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636693236811248777&amp;sdata=gxs1%2BfevIaBmQCVcJLUK41ml8CK2zLg%2FFKGGV%2F%2FHSLQ%3D&amp;reserved=0

This is a gentle reminder that this series was reviewed by Lucas two weeks ago but not yet included.

Quoted 8 lines
Quoted 3 lines

Does this series have a functional dependency on the reset fix ? If yes we can have a bisection proplem depending on which tree gets merged first.

Yes, without the reset fix I expect hangs.

Maybe the reset fix should be pulled in the pci tree? I don't know how these issues are sorted out.

Well either I pull that fix into the PCI tree (but it is already -rc8 and I can't guarantee v4.19 inclusion - I shall try) or I can ack these patches and Philippe will send them upstream on my behalf, atop the reset fix above.

When will the reset fix will be sent to Linus ? Please let me know how you want to proceed.

Thanks,
Lorenzo

author
#10 lore raw
Quoted 29 lines · Lorenzo Pieralisi
Quoted 19 lines · Leonard Crestez
Quoted 18 lines · Lorenzo Pieralisi
Quoted 16 lines · Leonard Crestez
Quoted 5 lines · Leonard Crestez

Changes since v2:
* Print with dev_info if link fails on resume (Lucas)
* Add a comment on imx7d pci irq mappings (Andrey)
* Make imx6_pcie_ltssm_disable print an error on IMX6Q (Lucas)

Quoted 7 lines

The ltssm_disable does not return an error because it can't be usefully handled, reversing partial suspend is a nightmare and unlikely to work.

* Drop "reset: imx7: Fix always writing bits as 0 (accepted by Philipp)

Series is against linux-next tag next-20180724 where the reset patch was already accepted. The imx7d.dtsi patch is also useful standalone.

This is a gentle reminder that this series was reviewed by Lucas two weeks ago but not yet included.

Quoted 8 lines
Quoted 3 lines

Does this series have a functional dependency on the reset fix ? If yes we can have a bisection proplem depending on which tree gets merged first.

Yes, without the reset fix I expect hangs.

Maybe the reset fix should be pulled in the pci tree? I don't know how these issues are sorted out.

Quoted 7 lines

Well either I pull that fix into the PCI tree (but it is already -rc8 and I can't guarantee v4.19 inclusion - I shall try) or I can ack these patches and Philippe will send them upstream on my behalf, atop the reset fix above.

When will the reset fix will be sent to Linus ? Please let me know how you want to proceed.

I have some other unsent changes for imx pci+dts+reset standing by. Since these changes qualify more as "pm features" than fixes I don't really mind skipping 4.19.

Pulling pci features into the reset tree doesn't make much sense to me.

Maybe this kind of stuff could go through Shawn's tree? There shouldn't be any interaction with non-imx stuff.

#11 lore raw
Quoted 35 lines · Leonard Crestez
Quoted 34 lines · Lorenzo Pieralisi
Quoted 19 lines · Leonard Crestez
Quoted 18 lines · Lorenzo Pieralisi
Quoted 16 lines · Leonard Crestez
Quoted 5 lines · Leonard Crestez

Changes since v2:
* Print with dev_info if link fails on resume (Lucas)
* Add a comment on imx7d pci irq mappings (Andrey)
* Make imx6_pcie_ltssm_disable print an error on IMX6Q (Lucas)

Quoted 7 lines

The ltssm_disable does not return an error because it can't be usefully handled, reversing partial suspend is a nightmare and unlikely to work.

* Drop "reset: imx7: Fix always writing bits as 0 (accepted by Philipp)

Series is against linux-next tag next-20180724 where the reset patch was already accepted. The imx7d.dtsi patch is also useful standalone.

This is a gentle reminder that this series was reviewed by Lucas two weeks ago but not yet included.

Quoted 8 lines
Quoted 3 lines

Does this series have a functional dependency on the reset fix ? If yes we can have a bisection proplem depending on which tree gets merged first.

Yes, without the reset fix I expect hangs.

Maybe the reset fix should be pulled in the pci tree? I don't know how these issues are sorted out.

Well either I pull that fix into the PCI tree (but it is already -rc8 and I can't guarantee v4.19 inclusion - I shall try) or I can ack these patches and Philippe will send them upstream on my behalf, atop the reset fix above.

Quoted 6 lines
Quoted 2 lines

When will the reset fix will be sent to Linus ? Please let me know how you want to proceed.

I have some other unsent changes for imx pci+dts+reset standing by. Since these changes qualify more as "pm features" than fixes I don't really mind skipping 4.19.

OK that's fine by me, resend this series and the new patches at v4.19-rc1 and I will pull the changes into the pci tree then.

Quoted 1 line

Pulling pci features into the reset tree doesn't make much sense to me.

I was just giving you an option.

Quoted 2 lines

Maybe this kind of stuff could go through Shawn's tree? There shouldn't be any interaction with non-imx stuff.

As I said, I can ack the changes for Shawn to pick them up but if Shawn's tree goes to Linus via eg arm-soc we are back to square one, it is just too late.

I have no problem taking these changes in the PCI tree for v4.20 as long as we plan the dependencies in advance.

Lorenzo

author
#12 lore raw
Quoted 19 lines · Lorenzo Pieralisi
Quoted 18 lines · Leonard Crestez
Quoted 17 lines · Lorenzo Pieralisi
Quoted 16 lines · Leonard Crestez
Quoted 15 lines · Lorenzo Pieralisi
Quoted 14 lines · Leonard Crestez
Quoted 12 lines · Leonard Crestez

Changes since v2:
* Print with dev_info if link fails on resume (Lucas)
* Add a comment on imx7d pci irq mappings (Andrey)
* Make imx6_pcie_ltssm_disable print an error on IMX6Q (Lucas)

The ltssm_disable does not return an error because it can't be usefully handled, reversing partial suspend is a nightmare and unlikely to work.

* Drop "reset: imx7: Fix always writing bits as 0 (accepted by Philipp)

Series is against linux-next tag next-20180724 where the reset patch was already accepted. The imx7d.dtsi patch is also useful standalone.

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

This is a gentle reminder that this series was reviewed by Lucas two weeks ago but not yet included.

Does this series have a functional dependency on the reset fix ? If yes we can have a bisection proplem depending on which tree gets merged first.

Yes, without the reset fix I expect hangs.

This needs further clarification: without the reset patch this will hang on imx7 suspend/resume but this is the current behavior anyway.

Both patches are required for imx7 pci suspend and including them out of order shouldn't break unrelated functionality.

So it should actually be fine to just include the pci changes now, right?

#13 lore raw

On Fri, Aug 10, 2018 at 11:11:55AM +0000, Leonard Crestez wrote:

[...]

Quoted 8 lines

This needs further clarification: without the reset patch this will hang on imx7 suspend/resume but this is the current behavior anyway.

Both patches are required for imx7 pci suspend and including them out of order shouldn't break unrelated functionality.

So it should actually be fine to just include the pci changes now, right?

It is a bit late for v4.19 but before considering that, this series is actually a set of fixes, correct ? If that's the case I think Shawn can send them upstream for a v4.19-rc*, I can ACK them if that sounds reasonable.

Patch (2) subject must make clear you are actually fixing a problem if we choose this course of action.

Lorenzo

#14 lore raw
Quoted 13 lines · Lorenzo Pieralisi

On Fri, Aug 10, 2018 at 11:11:55AM +0000, Leonard Crestez wrote:

[...]

Quoted 8 lines

This needs further clarification: without the reset patch this will hang on imx7 suspend/resume but this is the current behavior anyway.

Both patches are required for imx7 pci suspend and including them out of order shouldn't break unrelated functionality.

So it should actually be fine to just include the pci changes now, right?

Quoted 7 lines

It is a bit late for v4.19 but before considering that, this series is actually a set of fixes, correct ? If that's the case I think Shawn can send them upstream for a v4.19-rc*, I can ACK them if that sounds reasonable.

Patch (2) subject must make clear you are actually fixing a problem if we choose this course of action.

This is something never worked before, so it's a new feature to me. That said, I'm not in the position to send the PCI patch for 4.19-rc as a fix though IMX (arm-soc) tree.

I will send the DTS patch for 4.19-rc inclusion though, since that's indeed a fix.

Shawn