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(-)
--
2.17.1
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(-)
diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index 7cbc2ffa4b3a..7234e8330a57 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -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>,
--
2.17.1
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(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 4a9a673b4777..65b6d1015723 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -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,
};
--
2.17.1
Am Dienstag, den 24.07.2018, 19:14 +0300 schrieb 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]>
Acked-by: Lucas Stach <[email protected]>
> ---
> arch/arm/boot/dts/imx7d.dtsi | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
> index 7cbc2ffa4b3a..7234e8330a57 100644
> --- a/arch/arm/boot/dts/imx7d.dtsi
> +++ b/arch/arm/boot/dts/imx7d.dtsi
> @@ -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>,
Am Dienstag, den 24.07.2018, 19:14 +0300 schrieb 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]>
Reviewed-by: Lucas Stach <[email protected]>
> ---
> drivers/pci/controller/dwc/pci-imx6.c | 97 +++++++++++++++++++++++++--
> 1 file changed, 92 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 4a9a673b4777..65b6d1015723 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -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,
> };
>
On Tue, 2018-07-24 at 19:14 +0300, Leonard Crestez wrote:
> 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
This is a gentle reminder that this series was reviewed by Lucas two
weeks ago but not yet included.
--
Regards,
Leonard
On Wed, Aug 08, 2018 at 10:53:52AM +0000, Leonard Crestez wrote:
> On Tue, 2018-07-24 at 19:14 +0300, Leonard Crestez wrote:
> > 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
>
> 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
On Wed, 2018-08-08 at 12:14 +0100, Lorenzo Pieralisi wrote:
> On Wed, Aug 08, 2018 at 10:53:52AM +0000, Leonard Crestez wrote:
> > On Tue, 2018-07-24 at 19:14 +0300, Leonard Crestez wrote:
> > > 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://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2018%2F7%2F20%2F472&data=02%7C01%7Cleonard.crestez%40nxp.com%7Cc1e98512a50246c457a208d5fd2021dc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636693236811248777&sdata=gxs1%2BfevIaBmQCVcJLUK41ml8CK2zLg%2FFKGGV%2F%2FHSLQ%3D&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.
On Wed, Aug 08, 2018 at 11:37:14AM +0000, Leonard Crestez wrote:
> On Wed, 2018-08-08 at 12:14 +0100, Lorenzo Pieralisi wrote:
> > On Wed, Aug 08, 2018 at 10:53:52AM +0000, Leonard Crestez wrote:
> > > On Tue, 2018-07-24 at 19:14 +0300, Leonard Crestez wrote:
> > > > 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://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2018%2F7%2F20%2F472&data=02%7C01%7Cleonard.crestez%40nxp.com%7Cc1e98512a50246c457a208d5fd2021dc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636693236811248777&sdata=gxs1%2BfevIaBmQCVcJLUK41ml8CK2zLg%2FFKGGV%2F%2FHSLQ%3D&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.
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
On Wed, 2018-08-08 at 15:19 +0100, Lorenzo Pieralisi wrote:
> On Wed, Aug 08, 2018 at 11:37:14AM +0000, Leonard Crestez wrote:
> > On Wed, 2018-08-08 at 12:14 +0100, Lorenzo Pieralisi wrote:
> > > On Wed, Aug 08, 2018 at 10:53:52AM +0000, Leonard Crestez wrote:
> > > > On Tue, 2018-07-24 at 19:14 +0300, Leonard Crestez wrote:
> > > > > 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.
> > > >
> > > > 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.
>
> 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.
On Wed, Aug 08, 2018 at 02:58:15PM +0000, Leonard Crestez wrote:
> On Wed, 2018-08-08 at 15:19 +0100, Lorenzo Pieralisi wrote:
> > On Wed, Aug 08, 2018 at 11:37:14AM +0000, Leonard Crestez wrote:
> > > On Wed, 2018-08-08 at 12:14 +0100, Lorenzo Pieralisi wrote:
> > > > On Wed, Aug 08, 2018 at 10:53:52AM +0000, Leonard Crestez wrote:
> > > > > On Tue, 2018-07-24 at 19:14 +0300, Leonard Crestez wrote:
> > > > > > 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.
> > > > >
> > > > > 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.
> >
> > 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.
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.
> Pulling pci features into the reset tree doesn't make much sense to me.
I was just giving you an option.
> 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
On Wed, 2018-08-08 at 16:27 +0100, Lorenzo Pieralisi wrote:
> On Wed, Aug 08, 2018 at 02:58:15PM +0000, Leonard Crestez wrote:
> > On Wed, 2018-08-08 at 15:19 +0100, Lorenzo Pieralisi wrote:
> > > On Wed, Aug 08, 2018 at 11:37:14AM +0000, Leonard Crestez wrote:
> > > > On Wed, 2018-08-08 at 12:14 +0100, Lorenzo Pieralisi wrote:
> > > > > On Wed, Aug 08, 2018 at 10:53:52AM +0000, Leonard Crestez wrote:
> > > > > > On Tue, 2018-07-24 at 19:14 +0300, Leonard Crestez wrote:
> > > > > > > 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.
> > > > > >
> > > > > > 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?
On Fri, Aug 10, 2018 at 11:11:55AM +0000, Leonard Crestez wrote:
[...]
> 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
On Fri, Aug 10, 2018 at 04:13:41PM +0100, Lorenzo Pieralisi wrote:
> On Fri, Aug 10, 2018 at 11:11:55AM +0000, Leonard Crestez wrote:
>
> [...]
>
> > 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.
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
On Tue, Jul 24, 2018 at 07:14:19PM +0300, Leonard Crestez wrote:
> 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.