> -----Original Message----- > From: Lucas Stach [mailto:l.stach@xxxxxxxxxxxxxx] > Sent: Thursday, October 16, 2014 6:54 PM > To: Richard Zhu > Cc: linux-pci@xxxxxxxxxxxxxxx; Guo Shawn-R65073; festevam@xxxxxxxxx; > tharvey@xxxxxxxxxxxxx; Zhu Richard-R65037 > Subject: Re: [PATCH v6 09/13] PCI: imx6: Add imx6sx pcie support > > Am Donnerstag, den 16.10.2014, 15:52 +0800 schrieb Richard Zhu: > > From: Richard Zhu <r65037@xxxxxxxxxxxxx> > > > > - imx6sx pcie has its own standalone pcie power supply. > > In order to turn on the imx6sx pcie power during initialization. Add > > the pcie regulator and the gpc regmap into the imx6sx pcie structure. > > - imx6sx pcie has the new added reset mechanism, add the reset > > operations into the initialization. > > - register one PM call-back, enter/exit L2 state during system > > suspend/resume. > > use noirq pm_ops instead of the general pm_ops in dev_pm_ops, since > > cfg read/write may occurs after suspend and before resume. > > do msi store/re-store in suspend/resume callbacks, since controller > > maybe turned off, and these msi cfg maybe lost in suspend. > > - disp_axi clock is required by pcie inbound axi port actually. > > Add one more clock named pcie_inbound_axi for imx6sx pcie. > > - host init maybe failed, return negative value when there is a > > failure in the host init. > > > > Signed-off-by: Richard Zhu <richard.zhu@xxxxxxxxxxxxx> > > --- > > drivers/pci/host/pci-imx6.c | 204 > > +++++++++++++++++++++++++++++++++++++++----- > > 1 file changed, 183 insertions(+), 21 deletions(-) > > > [...] > > > > +#ifdef CONFIG_PM_SLEEP > > +static int pci_imx_suspend_noirq(struct device *dev) { > > + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev); > > + struct pcie_port *pp = &imx6_pcie->pp; > > + > > + if (is_imx6sx_pcie(imx6_pcie)) { > > + if (IS_ENABLED(CONFIG_PCI_MSI)) > > + dw_pcie_msi_cfg_store(pp); > > + > > + /* PM_TURN_OFF */ > > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12, > > + IMX6SX_GPR12_PCIE_PM_TURN_OFF, > > + IMX6SX_GPR12_PCIE_PM_TURN_OFF); > > + udelay(10); > > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12, > > + IMX6SX_GPR12_PCIE_PM_TURN_OFF, 0); > > + clk_disable_unprepare(imx6_pcie->pcie); > > + clk_disable_unprepare(imx6_pcie->pcie_bus); > > + clk_disable_unprepare(imx6_pcie->pcie_phy); > > + clk_disable_unprepare(imx6_pcie->pcie_inbound_axi); > > + } > > + > > + return 0; > > +} > > + > > +static int pci_imx_resume_noirq(struct device *dev) { > > + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev); > > + struct pcie_port *pp = &imx6_pcie->pp; > > + > > + if (is_imx6sx_pcie(imx6_pcie)) { > > + clk_prepare_enable(imx6_pcie->pcie_inbound_axi); > > + clk_prepare_enable(imx6_pcie->pcie_bus); > > + clk_prepare_enable(imx6_pcie->pcie_phy); > > + clk_prepare_enable(imx6_pcie->pcie); > > + > > + /* Reset iMX6SX PCIe */ > > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5, > > + IMX6SX_GPR5_PCIE_PERST, IMX6SX_GPR5_PCIE_PERST); > > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5, > > + IMX6SX_GPR5_PCIE_PERST, 0); > > + /* > > + * controller maybe turn off, re-configure again > > + */ > > + dw_pcie_setup_rc(pp); > > + > > + if (IS_ENABLED(CONFIG_PCI_MSI)) > > + dw_pcie_msi_cfg_restore(pp); > > + > > + /* RESET EP */ > > + gpio_set_value(imx6_pcie->reset_gpio, 0); > > + udelay(10); > > You are violating the spec here: "When PERST# is asserted it must remain > active for a minimum of 100 us (Tperst)." Also you probably want to put the EP > in reset at suspend and only release the reset in the resume path. > [Richard] Oops, sorry. Would be changed immediately. Assert the PERST# in suspend, and de-assert it in resume. Thanks a lot for your detailed review. > > + gpio_set_value(imx6_pcie->reset_gpio, 1); > > + } > > + > > + return 0; > > +} > > + > > +static const struct dev_pm_ops pci_imx_pm_ops = { > > + .suspend_noirq = pci_imx_suspend_noirq, > > + .resume_noirq = pci_imx_resume_noirq, > > + .freeze_noirq = pci_imx_suspend_noirq, > > + .thaw_noirq = pci_imx_resume_noirq, > > + .poweroff_noirq = pci_imx_suspend_noirq, > > + .restore_noirq = pci_imx_resume_noirq, }; #endif > > + > > static int __init imx6_pcie_probe(struct platform_device *pdev) { > > struct imx6_pcie *imx6_pcie; > > @@ -610,9 +751,28 @@ static int __init imx6_pcie_probe(struct > platform_device *pdev) > > return PTR_ERR(imx6_pcie->pcie); > > } > > > > - /* Grab GPR config register range */ > > - imx6_pcie->iomuxc_gpr = > > - syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr"); > > + if (is_imx6sx_pcie(imx6_pcie)) { > > + imx6_pcie->pcie_inbound_axi = devm_clk_get(&pdev->dev, > > + "pcie_inbound_axi"); > > + if (IS_ERR(imx6_pcie->pcie_inbound_axi)) { > > + dev_err(&pdev->dev, > > + "pcie clock source missing or invalid\n"); > > + return PTR_ERR(imx6_pcie->pcie_inbound_axi); > > + } > > + > > + imx6_pcie->pcie_phy_regulator = devm_regulator_get(pp->dev, > > + "pcie-phy"); > > + > > + imx6_pcie->iomuxc_gpr = > > + syscon_regmap_lookup_by_compatible > > + ("fsl,imx6sx-iomuxc-gpr"); > > + imx6_pcie->gpc_ips_reg = > > + syscon_regmap_lookup_by_compatible("fsl,imx6sx-gpc"); > > + } else { > > + imx6_pcie->iomuxc_gpr = > > + syscon_regmap_lookup_by_compatible > > + ("fsl,imx6q-iomuxc-gpr"); > > + } > > if (IS_ERR(imx6_pcie->iomuxc_gpr)) { > > dev_err(&pdev->dev, "unable to find iomuxc registers\n"); > > return PTR_ERR(imx6_pcie->iomuxc_gpr); @@ -636,6 +796,7 @@ static > > void imx6_pcie_shutdown(struct platform_device *pdev) > > > > static const struct of_device_id imx6_pcie_of_match[] = { > > { .compatible = "fsl,imx6q-pcie", }, > > + { .compatible = "fsl,imx6sx-pcie", }, > > {}, > > }; > > MODULE_DEVICE_TABLE(of, imx6_pcie_of_match); @@ -645,6 +806,7 @@ > > static struct platform_driver imx6_pcie_driver = { > > .name = "imx6q-pcie", > > .owner = THIS_MODULE, > > .of_match_table = imx6_pcie_of_match, > > + .pm = &pci_imx_pm_ops, > > }, > > .shutdown = imx6_pcie_shutdown, > > }; > Best Regards Richard Zhu ��.n��������+%������w��{.n�����{���"�)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
