Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 09/10/17 12:03, Kishon Vijay Abraham I wrote: > PCI core access configuration space registers in resume_noirq callbacks. > In the case of dra7xx, PIPE3 PHY connected to PCIe controller has to be > enabled before accessing configuration space registers. Since > PIPE3 PHY is enabled by only configuring control module registers, no > aborts has been observed so far (though during noirq stage, interface > clock of PIPE3 PHY is not enabled). > > With new TRM updates, PIPE3 PHY has to be initialized (PIPE3 PHY > registers has to be accessed) as well which requires the interface > clock of PIPE3 PHY to be enabled. The interface clock of PIPE3 PHY is > derived from OCP2SCP and hence PCIe PHY is modeled as a child of > OCP2SCP. Since pm_runtime is not enabled during noirq stage, > pm_runtime_get_sync done in phy_init doesn't enable > OCP2SCP clocks resulting in abort when PIPE3 PHY registers are > accessed. This could be the case not only with PCIe but even with USB and SATA? Maybe today it doesn't complain because USB/SATA is being resumed (by chance) after PHY? > > Create a function dependency between PCIe and PHY here to make > sure PCIe is suspended before PCIe PHY/OCP2SCP and resumed after > PCIe PHY/OCP2SCP. > > Suggested-by: Grygorii Strashko <grygorii.strashko@xxxxxx> > Signed-off-by: Kishon Vijay Abraham I <kishon@xxxxxx> > Signed-off-by: Sekhar Nori <nsekhar@xxxxxx> > --- > drivers/pci/dwc/pci-dra7xx.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c > index 34427a6a15af..362607f727ee 100644 > --- a/drivers/pci/dwc/pci-dra7xx.c > +++ b/drivers/pci/dwc/pci-dra7xx.c > @@ -11,6 +11,7 @@ > */ > > #include <linux/delay.h> > +#include <linux/device.h> > #include <linux/err.h> > #include <linux/interrupt.h> > #include <linux/irq.h> > @@ -594,6 +595,7 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev) > int i; > int phy_count; > struct phy **phy; > + struct device_link **link; > void __iomem *base; > struct resource *res; > struct dw_pcie *pci; > @@ -649,11 +651,21 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev) > if (!phy) > return -ENOMEM; > > + link = devm_kzalloc(dev, sizeof(*link) * phy_count, GFP_KERNEL); > + if (!link) > + return -ENOMEM; > + > for (i = 0; i < phy_count; i++) { > snprintf(name, sizeof(name), "pcie-phy%d", i); > phy[i] = devm_phy_get(dev, name); > if (IS_ERR(phy[i])) > return PTR_ERR(phy[i]); > + > + link[i] = device_link_add(dev, &phy[i]->dev, DL_FLAG_STATELESS); > + if (!link[i]) { > + ret = -EINVAL; > + goto err_link; > + } > } > > dra7xx->base = base; > @@ -732,6 +744,10 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev) > pm_runtime_disable(dev); > dra7xx_pcie_disable_phy(dra7xx); > > +err_link: > + while (--i >= 0) > + device_link_del(link[i]); > + > return ret; > } > > -- cheers, -roger