* [PATCH 0/2] PCI: Initial imx7d pm support
@ 2018-05-29 19:39 Leonard Crestez
2018-05-29 19:39 ` [PATCH 1/2] reset: imx7: Fix always writing bits as 0 Leonard Crestez
2018-05-29 19:39 ` [PATCH 2/2] PCI: imx: Initial imx7d pm support Leonard Crestez
0 siblings, 2 replies; 11+ messages in thread
From: Leonard Crestez @ 2018-05-29 19:39 UTC (permalink / raw)
To: Andrey Smirnov, Philipp Zabel, Lucas Stach, Richard Zhu,
linux-pci, linux-pm, linux-arm-kernel, linux-kernel
Cc: Joao Pinto, Abel Vesa, Anson Huang, Jingoo Han, Rafael J. Wysocki,
Lorenzo Pieralisi, Bjorn Helgaas
This series adds initial pm support on imx7d so that after
suspend/resume lspci works again. This mostly copies the resume sequence
from the imx tree.
More can be done later to reduce power in suspend as well as adding
support for other socs.
This is motivated mostly by a desire to bring imx PM code closer to
upstream. It is possible that I am missing some things about how PM
should be done for pci host drivers, it would be great if you could
point me the right way.
It also relies on this bugfix for PGC offsets:
https://lkml.org/lkml/2018/5/29/138
Without that patch resume hangs on first PCI read from PCI-PM core. It
is not strictly related to PCI but pci-imx6 is the only user of gpcv2
power domains.
Patch 1 in this series is also technically an unrelated bugfix, however
pci-imx6 is the only user.
Leonard Crestez (2):
reset: imx7: Fix always writing bits as 0
PCI: imx: Initial imx7d pm support
drivers/pci/dwc/pci-imx6.c | 94 ++++++++++++++++++++++++++++++++++++--
drivers/reset/reset-imx7.c | 2 +-
2 files changed, 90 insertions(+), 6 deletions(-)
--
2.17.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/2] reset: imx7: Fix always writing bits as 0 2018-05-29 19:39 [PATCH 0/2] PCI: Initial imx7d pm support Leonard Crestez @ 2018-05-29 19:39 ` Leonard Crestez 2018-06-08 14:23 ` Lucas Stach 2018-07-04 16:35 ` Lorenzo Pieralisi 2018-05-29 19:39 ` [PATCH 2/2] PCI: imx: Initial imx7d pm support Leonard Crestez 1 sibling, 2 replies; 11+ messages in thread From: Leonard Crestez @ 2018-05-29 19:39 UTC (permalink / raw) To: Andrey Smirnov, Philipp Zabel, Lucas Stach, Richard Zhu, linux-pci, linux-pm, linux-arm-kernel, linux-kernel Cc: Joao Pinto, Abel Vesa, Anson Huang, Jingoo Han, Rafael J. Wysocki, Lorenzo Pieralisi, Bjorn Helgaas Right now the only user of reset-imx7 is pci-imx6 and the reset_control_assert and deassert calls on pciephy_reset don't toggle the PCIEPHY_BTN and PCIEPHY_G_RST bits as expected. Fix this by writing 1 or 0 respectively. The reference manual is not very clear regarding SRC_PCIEPHY_RCR but for other registers like MIPIPHY and HSICPHY the bits are explicitly documented as "1 means assert, 0 means deassert". The values are still reversed for IMX7_RESET_PCIE_CTRL_APPS_EN. Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> --- drivers/reset/reset-imx7.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c index 4db177bc89bc..fdeac1946429 100644 --- a/drivers/reset/reset-imx7.c +++ b/drivers/reset/reset-imx7.c @@ -78,11 +78,11 @@ static struct imx7_src *to_imx7_src(struct reset_controller_dev *rcdev) static int imx7_reset_set(struct reset_controller_dev *rcdev, unsigned long id, bool assert) { struct imx7_src *imx7src = to_imx7_src(rcdev); const struct imx7_src_signal *signal = &imx7_src_signals[id]; - unsigned int value = 0; + unsigned int value = assert ? signal->bit : 0; switch (id) { case IMX7_RESET_PCIEPHY: /* * wait for more than 10us to release phy g_rst and -- 2.17.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] reset: imx7: Fix always writing bits as 0 2018-05-29 19:39 ` [PATCH 1/2] reset: imx7: Fix always writing bits as 0 Leonard Crestez @ 2018-06-08 14:23 ` Lucas Stach 2018-07-04 16:35 ` Lorenzo Pieralisi 1 sibling, 0 replies; 11+ messages in thread From: Lucas Stach @ 2018-06-08 14:23 UTC (permalink / raw) To: Leonard Crestez, Andrey Smirnov, Philipp Zabel, Richard Zhu, linux-pci, linux-pm, linux-arm-kernel, linux-kernel Cc: Lorenzo Pieralisi, Abel Vesa, Anson Huang, Jingoo Han, Rafael J. Wysocki, Joao Pinto, Bjorn Helgaas QW0gRGllbnN0YWcsIGRlbiAyOS4wNS4yMDE4LCAyMjozOSArMDMwMCBzY2hyaWViIExlb25hcmQg Q3Jlc3RlejoKPiBSaWdodCBub3cgdGhlIG9ubHkgdXNlciBvZiByZXNldC1pbXg3IGlzIHBjaS1p bXg2IGFuZCB0aGUKPiByZXNldF9jb250cm9sX2Fzc2VydCBhbmQgZGVhc3NlcnQgY2FsbHMgb24g cGNpZXBoeV9yZXNldCBkb24ndCB0b2dnbGUKPiB0aGUgUENJRVBIWV9CVE4gYW5kIFBDSUVQSFlf R19SU1QgYml0cyBhcyBleHBlY3RlZC4gRml4IHRoaXMgYnkgd3JpdGluZwo+IDEgb3IgMCByZXNw ZWN0aXZlbHkuCj4gCj4gVGhlIHJlZmVyZW5jZSBtYW51YWwgaXMgbm90IHZlcnkgY2xlYXIgcmVn YXJkaW5nIFNSQ19QQ0lFUEhZX1JDUiBidXQgZm9yCj4gb3RoZXIgcmVnaXN0ZXJzIGxpa2UgTUlQ SVBIWSBhbmQgSFNJQ1BIWSB0aGUgYml0cyBhcmUgZXhwbGljaXRseQo+IGRvY3VtZW50ZWQgYXMg IjEgbWVhbnMgYXNzZXJ0LCAwIG1lYW5zIGRlYXNzZXJ0Ii4KPiAKPiBUaGUgdmFsdWVzIGFyZSBz dGlsbCByZXZlcnNlZCBmb3IgSU1YN19SRVNFVF9QQ0lFX0NUUkxfQVBQU19FTi4KPiAKPiBTaWdu ZWQtb2ZmLWJ5OiBMZW9uYXJkIENyZXN0ZXogPGxlb25hcmQuY3Jlc3RlekBueHAuY29tPgoKUmV2 aWV3ZWQtYnk6IEx1Y2FzIFN0YWNoIDxsLnN0YWNoQHBlbmd1dHJvbml4LmRlPgoKPiAtLS0KPiDC oGRyaXZlcnMvcmVzZXQvcmVzZXQtaW14Ny5jIHwgMiArLQo+IMKgMSBmaWxlIGNoYW5nZWQsIDEg aW5zZXJ0aW9uKCspLCAxIGRlbGV0aW9uKC0pCj4gCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvcmVz ZXQvcmVzZXQtaW14Ny5jIGIvZHJpdmVycy9yZXNldC9yZXNldC1pbXg3LmMKPiBpbmRleCA0ZGIx NzdiYzg5YmMuLmZkZWFjMTk0NjQyOSAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL3Jlc2V0L3Jlc2V0 LWlteDcuYwo+ICsrKyBiL2RyaXZlcnMvcmVzZXQvcmVzZXQtaW14Ny5jCj4gQEAgLTc4LDExICs3 OCwxMSBAQCBzdGF0aWMgc3RydWN0IGlteDdfc3JjICp0b19pbXg3X3NyYyhzdHJ1Y3QgcmVzZXRf Y29udHJvbGxlcl9kZXYgKnJjZGV2KQo+IMKgc3RhdGljIGludCBpbXg3X3Jlc2V0X3NldChzdHJ1 Y3QgcmVzZXRfY29udHJvbGxlcl9kZXYgKnJjZGV2LAo+ID4gwqAJCQnCoMKgdW5zaWduZWQgbG9u ZyBpZCwgYm9vbCBhc3NlcnQpCj4gwqB7Cj4gPiDCoAlzdHJ1Y3QgaW14N19zcmMgKmlteDdzcmMg PSB0b19pbXg3X3NyYyhyY2Rldik7Cj4gPiDCoAljb25zdCBzdHJ1Y3QgaW14N19zcmNfc2lnbmFs ICpzaWduYWwgPSAmaW14N19zcmNfc2lnbmFsc1tpZF07Cj4gPiAtCXVuc2lnbmVkIGludCB2YWx1 ZSA9IDA7Cj4gPiArCXVuc2lnbmVkIGludCB2YWx1ZSA9IGFzc2VydCA/IHNpZ25hbC0+Yml0IDog MDsKPiDCoAo+ID4gwqAJc3dpdGNoIChpZCkgewo+ID4gwqAJY2FzZSBJTVg3X1JFU0VUX1BDSUVQ SFk6Cj4gPiDCoAkJLyoKPiA+IMKgCQnCoCogd2FpdCBmb3IgbW9yZSB0aGFuIDEwdXMgdG8gcmVs ZWFzZSBwaHkgZ19yc3QgYW5kCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fXwpsaW51eC1hcm0ta2VybmVsIG1haWxpbmcgbGlzdApsaW51eC1hcm0ta2VybmVs QGxpc3RzLmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFpbG1hbi9s aXN0aW5mby9saW51eC1hcm0ta2VybmVsCg== ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] reset: imx7: Fix always writing bits as 0 2018-05-29 19:39 ` [PATCH 1/2] reset: imx7: Fix always writing bits as 0 Leonard Crestez 2018-06-08 14:23 ` Lucas Stach @ 2018-07-04 16:35 ` Lorenzo Pieralisi 1 sibling, 0 replies; 11+ messages in thread From: Lorenzo Pieralisi @ 2018-07-04 16:35 UTC (permalink / raw) To: Leonard Crestez, Philipp Zabel Cc: Joao Pinto, Richard Zhu, Abel Vesa, Anson Huang, linux-pm, Andrey Smirnov, linux-pci, Rafael J. Wysocki, linux-kernel, Jingoo Han, Bjorn Helgaas, linux-arm-kernel, Lucas Stach On Tue, May 29, 2018 at 10:39:16PM +0300, Leonard Crestez wrote: > Right now the only user of reset-imx7 is pci-imx6 and the > reset_control_assert and deassert calls on pciephy_reset don't toggle > the PCIEPHY_BTN and PCIEPHY_G_RST bits as expected. Fix this by writing > 1 or 0 respectively. > > The reference manual is not very clear regarding SRC_PCIEPHY_RCR but for > other registers like MIPIPHY and HSICPHY the bits are explicitly > documented as "1 means assert, 0 means deassert". > > The values are still reversed for IMX7_RESET_PCIE_CTRL_APPS_EN. > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > --- > drivers/reset/reset-imx7.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) I think Philipp will pick it up, so I will drop it from the PCI patchwork, if there is a problem please let me know. Thanks, Lorenzo > diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c > index 4db177bc89bc..fdeac1946429 100644 > --- a/drivers/reset/reset-imx7.c > +++ b/drivers/reset/reset-imx7.c > @@ -78,11 +78,11 @@ static struct imx7_src *to_imx7_src(struct reset_controller_dev *rcdev) > static int imx7_reset_set(struct reset_controller_dev *rcdev, > unsigned long id, bool assert) > { > struct imx7_src *imx7src = to_imx7_src(rcdev); > const struct imx7_src_signal *signal = &imx7_src_signals[id]; > - unsigned int value = 0; > + unsigned int value = assert ? signal->bit : 0; > > switch (id) { > case IMX7_RESET_PCIEPHY: > /* > * wait for more than 10us to release phy g_rst and > -- > 2.17.0 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] PCI: imx: Initial imx7d pm support 2018-05-29 19:39 [PATCH 0/2] PCI: Initial imx7d pm support Leonard Crestez 2018-05-29 19:39 ` [PATCH 1/2] reset: imx7: Fix always writing bits as 0 Leonard Crestez @ 2018-05-29 19:39 ` Leonard Crestez 2018-06-08 14:33 ` Lucas Stach 1 sibling, 1 reply; 11+ messages in thread From: Leonard Crestez @ 2018-05-29 19:39 UTC (permalink / raw) To: Andrey Smirnov, Philipp Zabel, Lucas Stach, Richard Zhu, linux-pci, linux-pm, linux-arm-kernel, linux-kernel Cc: Bjorn Helgaas, Lorenzo Pieralisi, Anson Huang, Jingoo Han, Joao Pinto, Rafael J. Wysocki, Abel Vesa On imx7d the phy is turned off in suspend and must be reset on resume. Right now lspci -v fails after a suspend/resume cycle, fix this by adding minimal suspend/resume code from the nxp vendor tree. This is currently only enabled for imx7 but the same sequence can be applied to other imx pcie variants. Tested on imx7d-sabresd with an Intel 5622ANHMW wireless pcie adapter. The original author is mostly Richard Zhu <hongxing.zhu@nxp.com>, this patch adjusts the code to the upstream imx7d implemention using reset controls and power domains. Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> --- drivers/pci/dwc/pci-imx6.c | 94 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 89 insertions(+), 5 deletions(-) diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c index 4818ef875f8a..ff6077eeb387 100644 --- a/drivers/pci/dwc/pci-imx6.c +++ b/drivers/pci/dwc/pci-imx6.c @@ -540,10 +540,27 @@ 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); + } +} + 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 +575,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; @@ -681,10 +694,80 @@ 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 int imx6_pcie_suspend_noirq(struct device *dev) +{ + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev); + + if (imx6_pcie->variant == IMX7D) { + /* Disable clks */ + clk_disable_unprepare(imx6_pcie->pcie); + clk_disable_unprepare(imx6_pcie->pcie_phy); + clk_disable_unprepare(imx6_pcie->pcie_bus); + /* turn off external osc input */ + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12, + IMX7D_GPR12_PCIE_PHY_REFCLK_SEL, + IMX7D_GPR12_PCIE_PHY_REFCLK_SEL); + } + + return 0; +} + +static void imx6_pcie_ltssm_disable(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, 0); + break; + case IMX7D: + reset_control_assert(imx6_pcie->apps_reset); + break; + } +} + +static int imx6_pcie_resume_noirq(struct device *dev) +{ + int ret = 0; + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev); + struct pcie_port *pp = &imx6_pcie->pci->pp; + + if (imx6_pcie->variant == IMX7D) { + imx6_pcie_ltssm_disable(dev); + + imx6_pcie_assert_core_reset(imx6_pcie); + imx6_pcie_init_phy(imx6_pcie); + imx6_pcie_deassert_core_reset(imx6_pcie); + + /* + * controller maybe turn off, re-configure again + */ + dw_pcie_setup_rc(pp); + + imx6_pcie_ltssm_enable(dev); + + ret = imx6_pcie_wait_for_link(imx6_pcie); + if (ret < 0) + pr_info("pcie link is down after resume.\n"); + } + + return 0; +} + +static const struct dev_pm_ops imx6_pcie_pm_ops = { + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx6_pcie_suspend_noirq, + imx6_pcie_resume_noirq) +}; +#endif + static int imx6_pcie_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct dw_pcie *pci; struct imx6_pcie *imx6_pcie; @@ -847,10 +930,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.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] PCI: imx: Initial imx7d pm support 2018-05-29 19:39 ` [PATCH 2/2] PCI: imx: Initial imx7d pm support Leonard Crestez @ 2018-06-08 14:33 ` Lucas Stach 2018-07-02 17:18 ` Leonard Crestez 0 siblings, 1 reply; 11+ messages in thread From: Lucas Stach @ 2018-06-08 14:33 UTC (permalink / raw) To: Leonard Crestez, Andrey Smirnov, Philipp Zabel, Richard Zhu, linux-pci, linux-pm, linux-arm-kernel, linux-kernel Cc: Joao Pinto, Abel Vesa, Anson Huang, Jingoo Han, Rafael J. Wysocki, Lorenzo Pieralisi, Bjorn Helgaas QW0gRGllbnN0YWcsIGRlbiAyOS4wNS4yMDE4LCAyMjozOSArMDMwMCBzY2hyaWViIExlb25hcmQg Q3Jlc3RlejoKPiBPbiBpbXg3ZCB0aGUgcGh5IGlzIHR1cm5lZCBvZmYgaW4gc3VzcGVuZCBhbmQg bXVzdCBiZSByZXNldCBvbiByZXN1bWUuCj4gUmlnaHQgbm93IGxzcGNpIC12IGZhaWxzIGFmdGVy IGEgc3VzcGVuZC9yZXN1bWUgY3ljbGUsIGZpeCB0aGlzIGJ5Cj4gYWRkaW5nIG1pbmltYWwgc3Vz cGVuZC9yZXN1bWUgY29kZSBmcm9tIHRoZSBueHAgdmVuZG9yIHRyZWUuCj4gCj4gVGhpcyBpcyBj dXJyZW50bHkgb25seSBlbmFibGVkIGZvciBpbXg3IGJ1dCB0aGUgc2FtZSBzZXF1ZW5jZSBjYW4g YmUKPiBhcHBsaWVkIHRvIG90aGVyIGlteCBwY2llIHZhcmlhbnRzLgo+IAo+IFRlc3RlZCBvbiBp bXg3ZC1zYWJyZXNkIHdpdGggYW4gSW50ZWwgNTYyMkFOSE1XIHdpcmVsZXNzIHBjaWUgYWRhcHRl ci4KPiAKPiA+IFRoZSBvcmlnaW5hbCBhdXRob3IgaXMgbW9zdGx5IFJpY2hhcmQgWmh1IDxob25n eGluZy56aHVAbnhwLmNvbT4sIHRoaXMKPiBwYXRjaCBhZGp1c3RzIHRoZSBjb2RlIHRvIHRoZSB1 cHN0cmVhbSBpbXg3ZCBpbXBsZW1lbnRpb24gdXNpbmcgcmVzZXQKPiBjb250cm9scyBhbmQgcG93 ZXIgZG9tYWlucy4KPiAKPiA+IFNpZ25lZC1vZmYtYnk6IExlb25hcmQgQ3Jlc3RleiA8bGVvbmFy ZC5jcmVzdGV6QG54cC5jb20+Cj4gLS0tCj4gwqBkcml2ZXJzL3BjaS9kd2MvcGNpLWlteDYuYyB8 IDk0ICsrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKy0tCj4gwqAxIGZpbGUgY2hh bmdlZCwgODkgaW5zZXJ0aW9ucygrKSwgNSBkZWxldGlvbnMoLSkKPiAKPiBkaWZmIC0tZ2l0IGEv ZHJpdmVycy9wY2kvZHdjL3BjaS1pbXg2LmMgYi9kcml2ZXJzL3BjaS9kd2MvcGNpLWlteDYuYwo+ IGluZGV4IDQ4MThlZjg3NWY4YS4uZmY2MDc3ZWViMzg3IDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMv cGNpL2R3Yy9wY2ktaW14Ni5jCj4gKysrIGIvZHJpdmVycy9wY2kvZHdjL3BjaS1pbXg2LmMKPiBA QCAtNTQwLDEwICs1NDAsMjcgQEAgc3RhdGljIGludCBpbXg2X3BjaWVfd2FpdF9mb3Jfc3BlZWRf Y2hhbmdlKHN0cnVjdCBpbXg2X3BjaWUgKmlteDZfcGNpZSkKPiDCoAo+ID4gwqAJZGV2X2Vycihk ZXYsICJTcGVlZCBjaGFuZ2UgdGltZW91dFxuIik7Cj4gPiDCoAlyZXR1cm4gLUVJTlZBTDsKPiDC oH0KPiDCoAo+ICtzdGF0aWMgdm9pZCBpbXg2X3BjaWVfbHRzc21fZW5hYmxlKHN0cnVjdCBkZXZp Y2UgKmRldikKPiArewo+ID4gKwlzdHJ1Y3QgaW14Nl9wY2llICppbXg2X3BjaWUgPSBkZXZfZ2V0 X2RydmRhdGEoZGV2KTsKPiArCj4gPiArCXN3aXRjaCAoaW14Nl9wY2llLT52YXJpYW50KSB7Cj4g PiArCWNhc2UgSU1YNlE6Cj4gPiArCWNhc2UgSU1YNlNYOgo+ID4gKwljYXNlIElNWDZRUDoKPiA+ ICsJCXJlZ21hcF91cGRhdGVfYml0cyhpbXg2X3BjaWUtPmlvbXV4Y19ncHIsIElPTVVYQ19HUFIx MiwKPiA+ICsJCQkJwqDCoMKgSU1YNlFfR1BSMTJfUENJRV9DVExfMiwKPiA+ICsJCQkJwqDCoMKg SU1YNlFfR1BSMTJfUENJRV9DVExfMik7Cj4gPiArCQlicmVhazsKPiA+ICsJY2FzZSBJTVg3RDoK PiA+ICsJCXJlc2V0X2NvbnRyb2xfZGVhc3NlcnQoaW14Nl9wY2llLT5hcHBzX3Jlc2V0KTsKPiA+ ICsJfQo+ICt9Cj4gKwo+IMKgc3RhdGljIGludCBpbXg2X3BjaWVfZXN0YWJsaXNoX2xpbmsoc3Ry dWN0IGlteDZfcGNpZSAqaW14Nl9wY2llKQo+IMKgewo+ID4gwqAJc3RydWN0IGR3X3BjaWUgKnBj aSA9IGlteDZfcGNpZS0+cGNpOwo+ID4gwqAJc3RydWN0IGRldmljZSAqZGV2ID0gcGNpLT5kZXY7 Cj4gPiDCoAl1MzIgdG1wOwo+IEBAIC01NTgsMTUgKzU3NSwxMSBAQCBzdGF0aWMgaW50IGlteDZf cGNpZV9lc3RhYmxpc2hfbGluayhzdHJ1Y3QgaW14Nl9wY2llICppbXg2X3BjaWUpCj4gPiDCoAl0 bXAgJj0gflBDSUVfUkNfTENSX01BWF9MSU5LX1NQRUVEU19NQVNLOwo+ID4gwqAJdG1wIHw9IFBD SUVfUkNfTENSX01BWF9MSU5LX1NQRUVEU19HRU4xOwo+ID4gwqAJZHdfcGNpZV93cml0ZWxfZGJp KHBjaSwgUENJRV9SQ19MQ1IsIHRtcCk7Cj4gwqAKPiA+IMKgCS8qIFN0YXJ0IExUU1NNLiAqLwo+ ID4gLQlpZiAoaW14Nl9wY2llLT52YXJpYW50ID09IElNWDdEKQo+ID4gLQkJcmVzZXRfY29udHJv bF9kZWFzc2VydChpbXg2X3BjaWUtPmFwcHNfcmVzZXQpOwo+ID4gLQllbHNlCj4gPiAtCQlyZWdt YXBfdXBkYXRlX2JpdHMoaW14Nl9wY2llLT5pb211eGNfZ3ByLCBJT01VWENfR1BSMTIsCj4gPiAt CQkJCcKgwqDCoElNWDZRX0dQUjEyX1BDSUVfQ1RMXzIsIDEgPDwgMTApOwo+ID4gKwlpbXg2X3Bj aWVfbHRzc21fZW5hYmxlKGRldik7Cj4gwqAKPiA+IMKgCXJldCA9IGlteDZfcGNpZV93YWl0X2Zv cl9saW5rKGlteDZfcGNpZSk7Cj4gPiDCoAlpZiAocmV0KQo+ID4gwqAJCWdvdG8gZXJyX3Jlc2V0 X3BoeTsKPiDCoAo+IEBAIC02ODEsMTAgKzY5NCw4MCBAQCBzdGF0aWMgaW50IGlteDZfYWRkX3Bj aWVfcG9ydChzdHJ1Y3QgaW14Nl9wY2llICppbXg2X3BjaWUsCj4gwqAKPiDCoHN0YXRpYyBjb25z dCBzdHJ1Y3QgZHdfcGNpZV9vcHMgZHdfcGNpZV9vcHMgPSB7Cj4gPiDCoAkubGlua191cCA9IGlt eDZfcGNpZV9saW5rX3VwLAo+IMKgfTsKPiDCoAo+ICsjaWZkZWYgQ09ORklHX1BNX1NMRUVQCj4g K3N0YXRpYyBpbnQgaW14Nl9wY2llX3N1c3BlbmRfbm9pcnEoc3RydWN0IGRldmljZSAqZGV2KQo+ ICt7Cj4gPiArCXN0cnVjdCBpbXg2X3BjaWUgKmlteDZfcGNpZSA9IGRldl9nZXRfZHJ2ZGF0YShk ZXYpOwo+ICsKPiArCWlmIChpbXg2X3BjaWUtPnZhcmlhbnQgPT0gSU1YN0QpIHsKCkluc3RlYWQg b2YgdGhpcyBsYXJnZSBpbmRlbnRlZCBibG9jaywgcGxlYXNlIGp1c3QgaGF2ZSBhbiBlYXJseSBy ZXR1cm4KYXQgdGhlIHN0YXJ0IG9mIHRoaXMgZnVuY3Rpb24sIGxpa2U6CgppZiAoaW14Nl9wY2ll LT52YXJpYW50ICE9IElNWDdEKQoJcmV0dXJuIDA7CgpTYW1lIGZvciB0aGUgcmVzdW1lIGZ1bmN0 aW9uLgoKPiArCQkvKiBEaXNhYmxlIGNsa3MgKi8KPiA+ICsJCWNsa19kaXNhYmxlX3VucHJlcGFy ZShpbXg2X3BjaWUtPnBjaWUpOwo+ID4gKwkJY2xrX2Rpc2FibGVfdW5wcmVwYXJlKGlteDZfcGNp ZS0+cGNpZV9waHkpOwo+ID4gKwkJY2xrX2Rpc2FibGVfdW5wcmVwYXJlKGlteDZfcGNpZS0+cGNp ZV9idXMpOwo+ID4gKwkJLyogdHVybiBvZmYgZXh0ZXJuYWwgb3NjIGlucHV0ICovCj4gPiArCQly ZWdtYXBfdXBkYXRlX2JpdHMoaW14Nl9wY2llLT5pb211eGNfZ3ByLCBJT01VWENfR1BSMTIsCj4g PiArCQkJCcKgwqDCoElNWDdEX0dQUjEyX1BDSUVfUEhZX1JFRkNMS19TRUwsCj4gPiArCQkJCcKg wqDCoElNWDdEX0dQUjEyX1BDSUVfUEhZX1JFRkNMS19TRUwpOwo+ID4gKwl9Cj4gKwo+ID4gKwly ZXR1cm4gMDsKPiArfQo+ICsKPiArc3RhdGljIHZvaWQgaW14Nl9wY2llX2x0c3NtX2Rpc2FibGUo c3RydWN0IGRldmljZSAqZGV2KQo+ICt7Cj4gPiArCXN0cnVjdCBpbXg2X3BjaWUgKmlteDZfcGNp ZSA9IGRldl9nZXRfZHJ2ZGF0YShkZXYpOwo+ICsKPiA+ICsJc3dpdGNoIChpbXg2X3BjaWUtPnZh cmlhbnQpIHsKPiA+ICsJY2FzZSBJTVg2UToKPiA+ICsJY2FzZSBJTVg2U1g6Cj4gPiArCWNhc2Ug SU1YNlFQOgo+ID4gKwkJcmVnbWFwX3VwZGF0ZV9iaXRzKGlteDZfcGNpZS0+aW9tdXhjX2dwciwg SU9NVVhDX0dQUjEyLAo+ID4gKwkJCQnCoMKgwqBJTVg2UV9HUFIxMl9QQ0lFX0NUTF8yLCAwKTsK PiA+ICsJCWJyZWFrOwo+ID4gKwljYXNlIElNWDdEOgo+ID4gKwkJcmVzZXRfY29udHJvbF9hc3Nl cnQoaW14Nl9wY2llLT5hcHBzX3Jlc2V0KTsKPiA+ICsJCWJyZWFrOwo+ID4gKwl9Cj4gK30KPiAr Cj4gK3N0YXRpYyBpbnQgaW14Nl9wY2llX3Jlc3VtZV9ub2lycShzdHJ1Y3QgZGV2aWNlICpkZXYp Cj4gK3sKPiA+ICsJaW50IHJldCA9IDA7Cj4gPiArCXN0cnVjdCBpbXg2X3BjaWUgKmlteDZfcGNp ZSA9IGRldl9nZXRfZHJ2ZGF0YShkZXYpOwo+ID4gKwlzdHJ1Y3QgcGNpZV9wb3J0ICpwcCA9ICZp bXg2X3BjaWUtPnBjaS0+cHA7Cj4gKwo+ID4gKwlpZiAoaW14Nl9wY2llLT52YXJpYW50ID09IElN WDdEKSB7Cj4gPiArCQlpbXg2X3BjaWVfbHRzc21fZGlzYWJsZShkZXYpOwoKVGhlIExUU1NNIGRp c2FibGUgc2VlbXMgbWlzcGxhY2VkIGhlcmUuIEkgd291bGQgaGF2ZSBleHBlY3RlZCBpdCB0byBi ZQppbiB0aGUgc3VzcGVuZCBmdW5jdGlvbi4KCj4gKwkJaW14Nl9wY2llX2Fzc2VydF9jb3JlX3Jl c2V0KGlteDZfcGNpZSk7Cj4gPiArCQlpbXg2X3BjaWVfaW5pdF9waHkoaW14Nl9wY2llKTsKPiA+ ICsJCWlteDZfcGNpZV9kZWFzc2VydF9jb3JlX3Jlc2V0KGlteDZfcGNpZSk7Cj4gKwo+ID4gKwkJ LyoKPiA+ICsJCcKgKiBjb250cm9sbGVyIG1heWJlIHR1cm4gb2ZmLCByZS1jb25maWd1cmUgYWdh aW4KPiA+ICsJCcKgKi8KPiA+ICsJCWR3X3BjaWVfc2V0dXBfcmMocHApOwo+ICsKPiA+ICsJCWlt eDZfcGNpZV9sdHNzbV9lbmFibGUoZGV2KTsKPiArCj4gPiArCQlyZXQgPSBpbXg2X3BjaWVfd2Fp dF9mb3JfbGluayhpbXg2X3BjaWUpOwo+ID4gKwkJaWYgKHJldCA8IDApCj4gKwkJCXByX2luZm8o InBjaWUgbGluayBpcyBkb3duIGFmdGVyIHJlc3VtZS5cbiIpOwoKSWYgdGhlIFBIWSBoYXMgYmVl biBwb3dlcmVkIGRvd24gYW5kIExUU1NNIHN0b3BwZWQgSSBndWVzcyB3ZSBuZWVkIHRvCmRvIHRo ZSBmdWxsIGlteDZfcGNpZV9lc3RhYmxpc2hfbGluaygpIGRhbmNlIGFnYWluIGhlcmUgdG8gcmVs aWFibHkgcmUtCmVzdGFibGlzaCB0aGUgbGluay4gVGhlIGFib3ZlIHNlZW1zIHVuc2FmZS4KCj4g Kwl9Cj4gKwo+ID4gKwlyZXR1cm4gMDsKPiArfQo+ICsKPiArc3RhdGljIGNvbnN0IHN0cnVjdCBk ZXZfcG1fb3BzIGlteDZfcGNpZV9wbV9vcHMgPSB7Cj4gPiArCVNFVF9OT0lSUV9TWVNURU1fU0xF RVBfUE1fT1BTKGlteDZfcGNpZV9zdXNwZW5kX25vaXJxLAo+ID4gKwkJCQnCoMKgwqDCoMKgwqBp bXg2X3BjaWVfcmVzdW1lX25vaXJxKQo+ICt9Owo+ICsjZW5kaWYKClRoaXMgc3RydWN0dXJlIG11 c3QgYmUgb3V0c2lkZSBvZiB0aGUgaWZkZWYsIG9yIHlvdSdsbCBicmVhayB0aGUgYnVpbGQKb24g IUNPTkZJR19QTV9TTEVFUC4KCj4gwqBzdGF0aWMgaW50IGlteDZfcGNpZV9wcm9iZShzdHJ1Y3Qg cGxhdGZvcm1fZGV2aWNlICpwZGV2KQo+IMKgewo+ID4gwqAJc3RydWN0IGRldmljZSAqZGV2ID0g JnBkZXYtPmRldjsKPiA+IMKgCXN0cnVjdCBkd19wY2llICpwY2k7Cj4gPiDCoAlzdHJ1Y3QgaW14 Nl9wY2llICppbXg2X3BjaWU7Cj4gQEAgLTg0NywxMCArOTMwLDExIEBAIHN0YXRpYyBjb25zdCBz dHJ1Y3Qgb2ZfZGV2aWNlX2lkIGlteDZfcGNpZV9vZl9tYXRjaFtdID0gewo+IMKgc3RhdGljIHN0 cnVjdCBwbGF0Zm9ybV9kcml2ZXIgaW14Nl9wY2llX2RyaXZlciA9IHsKPiA+IMKgCS5kcml2ZXIg PSB7Cj4gPiA+IMKgCQkubmFtZQk9ICJpbXg2cS1wY2llIiwKPiA+IMKgCQkub2ZfbWF0Y2hfdGFi bGUgPSBpbXg2X3BjaWVfb2ZfbWF0Y2gsCj4gPiDCoAkJLnN1cHByZXNzX2JpbmRfYXR0cnMgPSB0 cnVlLAo+ID4gKwkJLnBtID0gJmlteDZfcGNpZV9wbV9vcHMsCj4gPiDCoAl9LAo+ID4gwqAJLnBy b2JlwqDCoMKgwqA9IGlteDZfcGNpZV9wcm9iZSwKPiA+IMKgCS5zaHV0ZG93biA9IGlteDZfcGNp ZV9zaHV0ZG93biwKPiDCoH07Cj4gwqAKClJlZ2FyZHMsCkx1Y2FzCgpfX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpsaW51eC1hcm0ta2VybmVsIG1haWxpbmcg bGlzdApsaW51eC1hcm0ta2VybmVsQGxpc3RzLmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmlu ZnJhZGVhZC5vcmcvbWFpbG1hbi9saXN0aW5mby9saW51eC1hcm0ta2VybmVsCg== ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] PCI: imx: Initial imx7d pm support 2018-06-08 14:33 ` Lucas Stach @ 2018-07-02 17:18 ` Leonard Crestez 2018-07-03 8:42 ` Lucas Stach 0 siblings, 1 reply; 11+ messages in thread From: Leonard Crestez @ 2018-07-02 17:18 UTC (permalink / raw) To: l.stach@pengutronix.de, andrew.smirnov@gmail.com, Richard Zhu Cc: lorenzo.pieralisi@arm.com, Abel Vesa, Anson Huang, linux-pm@vger.kernel.org, jingoohan1@gmail.com, rjw@rjwysocki.net, linux-kernel@vger.kernel.org, Joao.Pinto@synopsys.com, p.zabel@pengutronix.de, linux-pci@vger.kernel.org, bhelgaas@google.com, linux-arm-kernel@lists.infradead.org On Fri, 2018-06-08 at 16:33 +0200, Lucas Stach wrote: > Am Dienstag, den 29.05.2018, 22:39 +0300 schrieb Leonard Crestez: > > On imx7d the phy is turned off in suspend and must be reset on resume. > > Right now lspci -v fails after a suspend/resume cycle, fix this by > > adding minimal suspend/resume code from the nxp vendor tree. > > > > This is currently only enabled for imx7 but the same sequence can be > > applied to other imx pcie variants. > > +#ifdef CONFIG_PM_SLEEP > > +static int imx6_pcie_suspend_noirq(struct device *dev) > > +{ > > + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev); > > + > > + if (imx6_pcie->variant == IMX7D) { > > Instead of this large indented block, please just have an early return > at the start of this function, like: > > if (imx6_pcie->variant != IMX7D) > return 0; > > Same for the resume function. OK. The resume sequence is mostly the same for all SOCs where it applies. > > +static int imx6_pcie_resume_noirq(struct device *dev) > > +{ > > + int ret = 0; > > + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev); > > + struct pcie_port *pp = &imx6_pcie->pci->pp; > > > > + > > + if (imx6_pcie->variant == IMX7D) { > > + imx6_pcie_ltssm_disable(dev); > > The LTSSM disable seems misplaced here. I would have expected it to be > in the suspend function. This is a requirement for reinitializing the core: LTSSM needs to be turned off during initialization. > > + /* > > + * controller maybe turn off, re-configure again > > + */ > > + dw_pcie_setup_rc(pp); > > + > > + imx6_pcie_ltssm_enable(dev); > > + > > + ret = imx6_pcie_wait_for_link(imx6_pcie); > > + if (ret < 0) > > + pr_info("pcie link is down after resume.\n"); > > If the PHY has been powered down and LTSSM stopped I guess we need to > do the full imx6_pcie_establish_link() dance again here to reliably re- > establish the link. The above seems unsafe. What imx6_pcie_establish_link does additionally is some workaround for link gen detection. I agree that it should be included. This would make resume mostly the same as imx_pcie_host_init except for dw_pcie_msi_init; that needs to be saved/restored separately. Another issue that should be discussed here is that on 6sx and 7d the gpc PCIE power domain is not defined correctly: the PCIE block is in the DISPMIX domain on both SOCs and it is only PCIE_PHY which has a separate power domain. This matters: enabling power-gating for displays will break pcie if this relationship is not taken into account. Here are some options: 1) Make &pd_pcie a child of &pd_disp by hacking into gpc probe functions and calling pm_genpd_add_subdomain. Not very nice. 2) Support nesting PGCs in GPC code? Lots of code and still an incorrect representation of hardware. 3) Set power-domains = <&pd_disp> and enable runtime PM on &pd_pcie? 4) Add separate devices for the pcie-phy. These would be mostly empty but still different, for example on imx6sx the PHY is not even accessible on the bus but only though PCIE registers. Solutions 1-3 seem like workarounds while 4 seems excessive, would appreciate some feedback. -- Regards, Leonard _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] PCI: imx: Initial imx7d pm support 2018-07-02 17:18 ` Leonard Crestez @ 2018-07-03 8:42 ` Lucas Stach 2018-07-04 16:37 ` Lorenzo Pieralisi 2018-07-09 14:59 ` Leonard Crestez 0 siblings, 2 replies; 11+ messages in thread From: Lucas Stach @ 2018-07-03 8:42 UTC (permalink / raw) To: Leonard Crestez, andrew.smirnov@gmail.com, Richard Zhu Cc: lorenzo.pieralisi@arm.com, Abel Vesa, Anson Huang, linux-pm@vger.kernel.org, jingoohan1@gmail.com, rjw@rjwysocki.net, linux-kernel@vger.kernel.org, Joao.Pinto@synopsys.com, p.zabel@pengutronix.de, linux-pci@vger.kernel.org, bhelgaas@google.com, linux-arm-kernel@lists.infradead.org QW0gTW9udGFnLCBkZW4gMDIuMDcuMjAxOCwgMTc6MTggKzAwMDAgc2NocmllYiBMZW9uYXJkIENy ZXN0ZXo6Cj4gT24gRnJpLCAyMDE4LTA2LTA4IGF0IDE2OjMzICswMjAwLCBMdWNhcyBTdGFjaCB3 cm90ZToKPiA+IEFtIERpZW5zdGFnLCBkZW4gMjkuMDUuMjAxOCwgMjI6MzkgKzAzMDAgc2Nocmll YiBMZW9uYXJkIENyZXN0ZXo6Cj4gPiA+IE9uIGlteDdkIHRoZSBwaHkgaXMgdHVybmVkIG9mZiBp biBzdXNwZW5kIGFuZCBtdXN0IGJlIHJlc2V0IG9uIHJlc3VtZS4KPiA+ID4gUmlnaHQgbm93IGxz cGNpIC12IGZhaWxzIGFmdGVyIGEgc3VzcGVuZC9yZXN1bWUgY3ljbGUsIGZpeCB0aGlzIGJ5Cj4g PiA+IGFkZGluZyBtaW5pbWFsIHN1c3BlbmQvcmVzdW1lIGNvZGUgZnJvbSB0aGUgbnhwIHZlbmRv ciB0cmVlLgo+ID4gPiAKPiA+ID4gVGhpcyBpcyBjdXJyZW50bHkgb25seSBlbmFibGVkIGZvciBp bXg3IGJ1dCB0aGUgc2FtZSBzZXF1ZW5jZSBjYW4gYmUKPiA+ID4gYXBwbGllZCB0byBvdGhlciBp bXggcGNpZSB2YXJpYW50cy4KPiA+ID4gKyNpZmRlZiBDT05GSUdfUE1fU0xFRVAKPiA+ID4gK3N0 YXRpYyBpbnQgaW14Nl9wY2llX3N1c3BlbmRfbm9pcnEoc3RydWN0IGRldmljZSAqZGV2KQo+ID4g PiArewo+ID4gPiA+ID4gPiArCXN0cnVjdCBpbXg2X3BjaWUgKmlteDZfcGNpZSA9IGRldl9nZXRf ZHJ2ZGF0YShkZXYpOwo+ID4gPiArCj4gPiA+ICsJaWYgKGlteDZfcGNpZS0+dmFyaWFudCA9PSBJ TVg3RCkgewo+ID4gCj4gPiBJbnN0ZWFkIG9mIHRoaXMgbGFyZ2UgaW5kZW50ZWQgYmxvY2ssIHBs ZWFzZSBqdXN0IGhhdmUgYW4gZWFybHkgcmV0dXJuCj4gPiBhdCB0aGUgc3RhcnQgb2YgdGhpcyBm dW5jdGlvbiwgbGlrZToKPiA+IAo+ID4gaWYgKGlteDZfcGNpZS0+dmFyaWFudCAhPSBJTVg3RCkK PiA+IAlyZXR1cm4gMDsKPiA+IAo+ID4gU2FtZSBmb3IgdGhlIHJlc3VtZSBmdW5jdGlvbi4KPiAK PiBPSy4gVGhlIHJlc3VtZSBzZXF1ZW5jZSBpcyBtb3N0bHkgdGhlIHNhbWUgZm9yIGFsbCBTT0Nz IHdoZXJlIGl0Cj4gYXBwbGllcy4KPiAKPiA+ID4gK3N0YXRpYyBpbnQgaW14Nl9wY2llX3Jlc3Vt ZV9ub2lycShzdHJ1Y3QgZGV2aWNlICpkZXYpCj4gPiA+ICt7Cj4gPiA+ID4gPiA+ICsJaW50IHJl dCA9IDA7Cj4gPiA+ID4gPiA+ICsJc3RydWN0IGlteDZfcGNpZSAqaW14Nl9wY2llID0gZGV2X2dl dF9kcnZkYXRhKGRldik7Cj4gPiA+ID4gPiA+ICsJc3RydWN0IHBjaWVfcG9ydCAqcHAgPSAmaW14 Nl9wY2llLT5wY2ktPnBwOwo+ID4gPiAKPiA+ID4gKwo+ID4gPiA+ID4gPiArCWlmIChpbXg2X3Bj aWUtPnZhcmlhbnQgPT0gSU1YN0QpIHsKPiA+ID4gKwkJaW14Nl9wY2llX2x0c3NtX2Rpc2FibGUo ZGV2KTsKPiA+IAo+ID4gVGhlIExUU1NNIGRpc2FibGUgc2VlbXMgbWlzcGxhY2VkIGhlcmUuIEkg d291bGQgaGF2ZSBleHBlY3RlZCBpdCB0byBiZQo+ID4gaW4gdGhlIHN1c3BlbmQgZnVuY3Rpb24u Cj4gCj4gVGhpcyBpcyBhIHJlcXVpcmVtZW50IGZvciByZWluaXRpYWxpemluZyB0aGUgY29yZTog TFRTU00gbmVlZHMgdG8gYmUKPiB0dXJuZWQgb2ZmIGR1cmluZyBpbml0aWFsaXphdGlvbi4KCklm IHlvdSBkaXNhYmxlIExUU1NNIGR1cmluZyBzdXNwZW5kLCBpdCBzaG91bGQgYmUgb2ZmIHdoZW4g ZW50ZXJpbmcKdGhpcyByZXN1bWUgZnVuY3Rpb24sIG5vPwoKPiA+ID4gKwkJLyoKPiA+ID4gPiA+ ID4gKwkJwqAqIGNvbnRyb2xsZXIgbWF5YmUgdHVybiBvZmYsIHJlLWNvbmZpZ3VyZSBhZ2Fpbgo+ ID4gPiA+ID4gPiArCQnCoCovCj4gPiA+ID4gPiA+ICsJCWR3X3BjaWVfc2V0dXBfcmMocHApOwo+ ID4gPiArCj4gPiA+ID4gPiA+ICsJCWlteDZfcGNpZV9sdHNzbV9lbmFibGUoZGV2KTsKPiA+ID4g Kwo+ID4gPiA+ID4gPiArCQlyZXQgPSBpbXg2X3BjaWVfd2FpdF9mb3JfbGluayhpbXg2X3BjaWUp Owo+ID4gPiA+ID4gPiArCQlpZiAocmV0IDwgMCkKPiA+ID4gKwkJCXByX2luZm8oInBjaWUgbGlu ayBpcyBkb3duIGFmdGVyIHJlc3VtZS5cbiIpOwo+ID4gCj4gPiBJZiB0aGUgUEhZIGhhcyBiZWVu IHBvd2VyZWQgZG93biBhbmQgTFRTU00gc3RvcHBlZCBJIGd1ZXNzIHdlIG5lZWQgdG8KPiA+IGRv IHRoZSBmdWxsIGlteDZfcGNpZV9lc3RhYmxpc2hfbGluaygpIGRhbmNlIGFnYWluIGhlcmUgdG8g cmVsaWFibHkgcmUtCj4gPiBlc3RhYmxpc2ggdGhlIGxpbmsuIFRoZSBhYm92ZSBzZWVtcyB1bnNh ZmUuCj4gCj4gV2hhdCBpbXg2X3BjaWVfZXN0YWJsaXNoX2xpbmsgZG9lcyBhZGRpdGlvbmFsbHkg aXMgc29tZSB3b3JrYXJvdW5kIGZvcgo+IGxpbmsgZ2VuIGRldGVjdGlvbi4gSSBhZ3JlZSB0aGF0 IGl0IHNob3VsZCBiZSBpbmNsdWRlZC4KPiAKPiBUaGlzIHdvdWxkIG1ha2UgcmVzdW1lIG1vc3Rs eSB0aGUgc2FtZSBhcyBpbXhfcGNpZV9ob3N0X2luaXQgZXhjZXB0IGZvcgo+IGR3X3BjaWVfbXNp X2luaXQ7IHRoYXQgbmVlZHMgdG8gYmUgc2F2ZWQvcmVzdG9yZWQgc2VwYXJhdGVseS4KClJpZ2h0 LCBzbyBtYXliZSB3ZSBjYW4gZXZlbiBjdXQgZG93biBvbiBsaW5lcyBvZiBjb2RlIGJ5IHNwbGl0 dGluZyBhbmQKcmV1c2luZyBleGlzdGluZyBmdW5jdGlvbnMuCgo+IAo+IEFub3RoZXIgaXNzdWUg dGhhdCBzaG91bGQgYmUgZGlzY3Vzc2VkIGhlcmUgaXMgdGhhdCBvbiA2c3ggYW5kIDdkIHRoZQo+ IGdwYyBQQ0lFIHBvd2VyIGRvbWFpbiBpcyBub3QgZGVmaW5lZCBjb3JyZWN0bHk6IHRoZSBQQ0lF IGJsb2NrIGlzIGluCj4gdGhlIERJU1BNSVggZG9tYWluIG9uIGJvdGggU09DcyBhbmQgaXQgaXMg b25seSBQQ0lFX1BIWSB3aGljaCBoYXMgYQo+IHNlcGFyYXRlIHBvd2VyIGRvbWFpbi4KPiAKPiBU aGlzIG1hdHRlcnM6IGVuYWJsaW5nIHBvd2VyLWdhdGluZyBmb3IgZGlzcGxheXMgd2lsbCBicmVh ayBwY2llIGlmCj4gdGhpcyByZWxhdGlvbnNoaXAgaXMgbm90IHRha2VuIGludG8gYWNjb3VudC4g SGVyZSBhcmUgc29tZSBvcHRpb25zOgo+IAo+IDEpIE1ha2UgJnBkX3BjaWUgYSBjaGlsZCBvZiAm cGRfZGlzcCBieSBoYWNraW5nIGludG8gZ3BjIHByb2JlCj4gZnVuY3Rpb25zIGFuZCBjYWxsaW5n IHBtX2dlbnBkX2FkZF9zdWJkb21haW4uIE5vdCB2ZXJ5IG5pY2UuCj4gCj4gMikgU3VwcG9ydCBu ZXN0aW5nIFBHQ3MgaW4gR1BDIGNvZGU/IExvdHMgb2YgY29kZSBhbmQgc3RpbGwgYW4KPiBpbmNv cnJlY3QgcmVwcmVzZW50YXRpb24gb2YgaGFyZHdhcmUuCj4gCj4gMykgU2V0IHBvd2VyLWRvbWFp bnMgPSA8JnBkX2Rpc3A+IGFuZCBlbmFibGUgcnVudGltZSBQTSBvbiAmcGRfcGNpZT8KPiAKPiA0 KSBBZGQgc2VwYXJhdGUgZGV2aWNlcyBmb3IgdGhlIHBjaWUtcGh5LiBUaGVzZSB3b3VsZCBiZSBt b3N0bHkgZW1wdHkKPiBidXQgc3RpbGwgZGlmZmVyZW50LCBmb3IgZXhhbXBsZSBvbiBpbXg2c3gg dGhlIFBIWSBpcyBub3QgZXZlbgo+IGFjY2Vzc2libGUgb24gdGhlIGJ1cyBidXQgb25seSB0aG91 Z2ggUENJRSByZWdpc3RlcnMuCgo1KSBUYWtlIGEgbG9vayBhdCB0aGUgbGludXgtcG0gbGlzdC4g OykgVGhlIHBvd2VyIGRvbWFpbiBmcmFtZXdvcmsgaGFzCmp1c3QgZ2FpbmVkIHN1cHBvcnQgZm9y IG11bHRpcGxlIHBvd2VyIGRvbWFpbnMgcGVyIGRldmljZS4gSSB0aGluayB0aGF0CiBpcyB0aGUg cmlnaHQgc29sdXRpb24gZm9yIHRoaXMsIGFzIGxpa2UgeW91IG1lbnRpb25lZCB0aGUgUEhZIGlz bid0CnJlYWxseSBhIHNlcGFyYXRlIGJsb2NrIG9uIGkuTVg2LCBidXQgcGFydCBvZiB0aGUgUENJ ZSBjb250cm9sbGVyCmRldmljZS4KClJlZ2FyZHMsCkx1Y2FzCgpfX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fXwpsaW51eC1hcm0ta2VybmVsIG1haWxpbmcgbGlz dApsaW51eC1hcm0ta2VybmVsQGxpc3RzLmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJh ZGVhZC5vcmcvbWFpbG1hbi9saXN0aW5mby9saW51eC1hcm0ta2VybmVsCg== ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] PCI: imx: Initial imx7d pm support 2018-07-03 8:42 ` Lucas Stach @ 2018-07-04 16:37 ` Lorenzo Pieralisi 2018-07-09 14:59 ` Leonard Crestez 1 sibling, 0 replies; 11+ messages in thread From: Lorenzo Pieralisi @ 2018-07-04 16:37 UTC (permalink / raw) To: Lucas Stach Cc: Joao.Pinto@synopsys.com, Richard Zhu, Abel Vesa, Anson Huang, linux-pm@vger.kernel.org, andrew.smirnov@gmail.com, jingoohan1@gmail.com, rjw@rjwysocki.net, linux-kernel@vger.kernel.org, p.zabel@pengutronix.de, linux-pci@vger.kernel.org, bhelgaas@google.com, Leonard Crestez, linux-arm-kernel@lists.infradead.org On Tue, Jul 03, 2018 at 10:42:08AM +0200, Lucas Stach wrote: > Am Montag, den 02.07.2018, 17:18 +0000 schrieb Leonard Crestez: > > On Fri, 2018-06-08 at 16:33 +0200, Lucas Stach wrote: > > > Am Dienstag, den 29.05.2018, 22:39 +0300 schrieb Leonard Crestez: > > > > On imx7d the phy is turned off in suspend and must be reset on resu= me. > > > > Right now lspci -v fails after a suspend/resume cycle, fix this by > > > > adding minimal suspend/resume code from the nxp vendor tree. > > > > = > > > > This is currently only enabled for imx7 but the same sequence can be > > > > applied to other imx pcie variants. > > > > +#ifdef CONFIG_PM_SLEEP > > > > +static int imx6_pcie_suspend_noirq(struct device *dev) > > > > +{ > > > > > > > + struct imx6_pcie *imx6_pcie =3D dev_get_drvdata(dev); > > > > + > > > > + if (imx6_pcie->variant =3D=3D IMX7D) { > > > = > > > Instead of this large indented block, please just have an early return > > > at the start of this function, like: > > > = > > > if (imx6_pcie->variant !=3D IMX7D) > > > return 0; > > > = > > > Same for the resume function. > > = > > OK. The resume sequence is mostly the same for all SOCs where it > > applies. > > = > > > > +static int imx6_pcie_resume_noirq(struct device *dev) > > > > +{ > > > > > > > + int ret =3D 0; > > > > > > > + struct imx6_pcie *imx6_pcie =3D dev_get_drvdata(dev); > > > > > > > + struct pcie_port *pp =3D &imx6_pcie->pci->pp; > > > > = > > > > + > > > > > > > + if (imx6_pcie->variant =3D=3D IMX7D) { > > > > + imx6_pcie_ltssm_disable(dev); > > > = > > > The LTSSM disable seems misplaced here. I would have expected it to be > > > in the suspend function. > > = > > This is a requirement for reinitializing the core: LTSSM needs to be > > turned off during initialization. > = > If you disable LTSSM during suspend, it should be off when entering > this resume function, no? > = > > > > + /* > > > > > > > + =A0* controller maybe turn off, re-configure again > > > > > > > + =A0*/ > > > > > > > + dw_pcie_setup_rc(pp); > > > > + > > > > > > > + imx6_pcie_ltssm_enable(dev); > > > > + > > > > > > > + ret =3D imx6_pcie_wait_for_link(imx6_pcie); > > > > > > > + if (ret < 0) > > > > + pr_info("pcie link is down after resume.\n"); > > > = > > > If the PHY has been powered down and LTSSM stopped I guess we need to > > > do the full imx6_pcie_establish_link() dance again here to reliably r= e- > > > establish the link. The above seems unsafe. > > = > > What imx6_pcie_establish_link does additionally is some workaround for > > link gen detection. I agree that it should be included. > > = > > This would make resume mostly the same as imx_pcie_host_init except for > > dw_pcie_msi_init; that needs to be saved/restored separately. > = > Right, so maybe we can even cut down on lines of code by splitting and > reusing existing functions. > = > > = > > Another issue that should be discussed here is that on 6sx and 7d the > > gpc PCIE power domain is not defined correctly: the PCIE block is in > > the DISPMIX domain on both SOCs and it is only PCIE_PHY which has a > > separate power domain. > > = > > This matters: enabling power-gating for displays will break pcie if > > this relationship is not taken into account. Here are some options: > > = > > 1) Make &pd_pcie a child of &pd_disp by hacking into gpc probe > > functions and calling pm_genpd_add_subdomain. Not very nice. > > = > > 2) Support nesting PGCs in GPC code? Lots of code and still an > > incorrect representation of hardware. > > = > > 3) Set power-domains =3D <&pd_disp> and enable runtime PM on &pd_pcie? > > = > > 4) Add separate devices for the pcie-phy. These would be mostly empty > > but still different, for example on imx6sx the PHY is not even > > accessible on the bus but only though PCIE registers. > = > 5) Take a look at the linux-pm list. ;) The power domain framework has > just gained support for multiple power domains per device. I think that > is the right solution for this, as like you mentioned the PHY isn't > really a separate block on i.MX6, but part of the PCIe controller > device. >From the discussion I take this as there is going to be a v2 so I will mark this as Changes Requested, please let me know if that's a problem. Thanks, Lorenzo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] PCI: imx: Initial imx7d pm support 2018-07-03 8:42 ` Lucas Stach 2018-07-04 16:37 ` Lorenzo Pieralisi @ 2018-07-09 14:59 ` Leonard Crestez 2018-07-10 10:26 ` Ulf Hansson 1 sibling, 1 reply; 11+ messages in thread From: Leonard Crestez @ 2018-07-09 14:59 UTC (permalink / raw) To: l.stach@pengutronix.de, ulf.hansson@linaro.org, andrew.smirnov@gmail.com, Richard Zhu, viresh.kumar@linaro.org Cc: lorenzo.pieralisi@arm.com, Abel Vesa, Anson Huang, linux-pm@vger.kernel.org, jingoohan1@gmail.com, rjw@rjwysocki.net, linux-kernel@vger.kernel.org, Joao.Pinto@synopsys.com, p.zabel@pengutronix.de, linux-pci@vger.kernel.org, bhelgaas@google.com, linux-arm-kernel@lists.infradead.org On Tue, 2018-07-03 at 10:42 +0200, Lucas Stach wrote: > Am Montag, den 02.07.2018, 17:18 +0000 schrieb Leonard Crestez: > > On Fri, 2018-06-08 at 16:33 +0200, Lucas Stach wrote: > > > Am Dienstag, den 29.05.2018, 22:39 +0300 schrieb Leonard Crestez: > > > > On imx7d the phy is turned off in suspend and must be reset on resume. > > > > Right now lspci -v fails after a suspend/resume cycle, fix this by > > > > adding minimal suspend/resume code from the nxp vendor tree. > > > > > > > > This is currently only enabled for imx7 but the same sequence can be > > > > applied to other imx pcie variants. > > > > +#ifdef CONFIG_PM_SLEEP > > > > Another issue that should be discussed here is that on 6sx and 7d the > > gpc PCIE power domain is not defined correctly: the PCIE block is in > > the DISPMIX domain on both SOCs and it is only PCIE_PHY which has a > > separate power domain. > > > > This matters: enabling power-gating for displays will break pcie if > > this relationship is not taken into account. Here are some options: > > > > 1) Make &pd_pcie a child of &pd_disp by hacking into gpc probe > > functions and calling pm_genpd_add_subdomain. Not very nice. > > > > 2) Support nesting PGCs in GPC code? Lots of code and still an > > incorrect representation of hardware. > > > > 3) Set power-domains = <&pd_disp> and enable runtime PM on &pd_pcie? > > > > 4) Add separate devices for the pcie-phy. These would be mostly empty > > but still different, for example on imx6sx the PHY is not even > > accessible on the bus but only though PCIE registers. > > 5) Take a look at the linux-pm list. ;) The power domain framework has > just gained support for multiple power domains per device. I think that > is the right solution for this, as like you mentioned the PHY isn't > really a separate block on i.MX6, but part of the PCIe controller > device. Yes, I noticed that earlier but not that it was merged recently. It is a better solution. Unfortunately the multiple power-domain code seems to require devices to explicitly fetch references to the power domains and manipulate them. As far as I can tell this means that every device using multiple power domains has to be modified to do so. In this particular case it would be useful to just have all power domains turned on before probe, just like when a single power-domain is specified: power-domains = <&pd_pci>, <&pd_disp>; But this issue is not strictly related to imx7d pci hanging on suspend/resume, it can be dealt with later. -- Regards, Leonard _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] PCI: imx: Initial imx7d pm support 2018-07-09 14:59 ` Leonard Crestez @ 2018-07-10 10:26 ` Ulf Hansson 0 siblings, 0 replies; 11+ messages in thread From: Ulf Hansson @ 2018-07-10 10:26 UTC (permalink / raw) To: Leonard Crestez Cc: lorenzo.pieralisi@arm.com, Richard Zhu, Abel Vesa, Anson Huang, linux-pm@vger.kernel.org, andrew.smirnov@gmail.com, viresh.kumar@linaro.org, rjw@rjwysocki.net, linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, Joao.Pinto@synopsys.com, p.zabel@pengutronix.de, jingoohan1@gmail.com, bhelgaas@google.com, linux-arm-kernel@lists.infradead.org, l.stach@pengutronix.de On 9 July 2018 at 16:59, Leonard Crestez <leonard.crestez@nxp.com> wrote: > On Tue, 2018-07-03 at 10:42 +0200, Lucas Stach wrote: >> Am Montag, den 02.07.2018, 17:18 +0000 schrieb Leonard Crestez: >> > On Fri, 2018-06-08 at 16:33 +0200, Lucas Stach wrote: >> > > Am Dienstag, den 29.05.2018, 22:39 +0300 schrieb Leonard Crestez: >> > > > On imx7d the phy is turned off in suspend and must be reset on resume. >> > > > Right now lspci -v fails after a suspend/resume cycle, fix this by >> > > > adding minimal suspend/resume code from the nxp vendor tree. >> > > > >> > > > This is currently only enabled for imx7 but the same sequence can be >> > > > applied to other imx pcie variants. >> > > > +#ifdef CONFIG_PM_SLEEP >> > >> > Another issue that should be discussed here is that on 6sx and 7d the >> > gpc PCIE power domain is not defined correctly: the PCIE block is in >> > the DISPMIX domain on both SOCs and it is only PCIE_PHY which has a >> > separate power domain. >> > >> > This matters: enabling power-gating for displays will break pcie if >> > this relationship is not taken into account. Here are some options: >> > >> > 1) Make &pd_pcie a child of &pd_disp by hacking into gpc probe >> > functions and calling pm_genpd_add_subdomain. Not very nice. >> > >> > 2) Support nesting PGCs in GPC code? Lots of code and still an >> > incorrect representation of hardware. >> > >> > 3) Set power-domains = <&pd_disp> and enable runtime PM on &pd_pcie? >> > >> > 4) Add separate devices for the pcie-phy. These would be mostly empty >> > but still different, for example on imx6sx the PHY is not even >> > accessible on the bus but only though PCIE registers. >> >> 5) Take a look at the linux-pm list. ;) The power domain framework has >> just gained support for multiple power domains per device. I think that >> is the right solution for this, as like you mentioned the PHY isn't >> really a separate block on i.MX6, but part of the PCIe controller >> device. Yep, it sounds like the PCIe controller device is partitioned across multiple PM domains. > > Yes, I noticed that earlier but not that it was merged recently. It is > a better solution. > > Unfortunately the multiple power-domain code seems to require devices > to explicitly fetch references to the power domains and manipulate > them. Actually, it's the other way around. You need only one device to attach the PM domains. However, in genpd, one device will be created per PM domain and its that device you get back to operate upon. Typically the returned device(s) should be linked with the "master" device, device_link_add(). > As far as I can tell this means that every device using multiple > power domains has to be modified to do so. In this particular case it > would be useful to just have all power domains turned on before probe, > just like when a single power-domain is specified: I you need them to be powered on, just use the corresponding device links flags when you create the link during probe. Something along the lines of this: device_link_add(dev, genpd_dev0, DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE); > > power-domains = <&pd_pci>, <&pd_disp>; > > But this issue is not strictly related to imx7d pci hanging on > suspend/resume, it can be dealt with later. Maybe not, I don't have the complete picture. However, you do get the opportunity to use the genpd infrastructure, which calls the ->power_on|off() callbacks of the PM domain during suspend/resume. And by using the dev_link_add(), you can make sure that the PM domain gets powered on/off in the order as the corresponding supplier device. Kind regards Uffe _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-07-10 10:26 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-05-29 19:39 [PATCH 0/2] PCI: Initial imx7d pm support Leonard Crestez 2018-05-29 19:39 ` [PATCH 1/2] reset: imx7: Fix always writing bits as 0 Leonard Crestez 2018-06-08 14:23 ` Lucas Stach 2018-07-04 16:35 ` Lorenzo Pieralisi 2018-05-29 19:39 ` [PATCH 2/2] PCI: imx: Initial imx7d pm support Leonard Crestez 2018-06-08 14:33 ` Lucas Stach 2018-07-02 17:18 ` Leonard Crestez 2018-07-03 8:42 ` Lucas Stach 2018-07-04 16:37 ` Lorenzo Pieralisi 2018-07-09 14:59 ` Leonard Crestez 2018-07-10 10:26 ` Ulf Hansson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).