| Message ID | 1527822270-30280-1-git-send-email-honghui.zhang@mediatek.com |
|---|---|
| State | Superseded |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | [v3] PCI: mediatek: Add system pm support for MT2712 | expand |
On Fri, Jun 1, 2018 at 6:04 AM, <honghui.zhang@mediatek.com> wrote: > From: Honghui Zhang <honghui.zhang@mediatek.com> > +#ifdef CONFIG_PM_SLEEP > +static int mtk_pcie_suspend_noirq(struct device *dev) __maybe_unused
On Fri, 2018-06-01 at 13:17 +0300, Andy Shevchenko wrote: > On Fri, Jun 1, 2018 at 6:04 AM, <honghui.zhang@mediatek.com> wrote: > > From: Honghui Zhang <honghui.zhang@mediatek.com> > > > +#ifdef CONFIG_PM_SLEEP > > +static int mtk_pcie_suspend_noirq(struct device *dev) > > __maybe_unused > Hi, Andy, thanks for your review. Bjorn had point this out that at: https://www.spinics.net/lists/arm-kernel/msg656774.html So __maybe_unused is not really needed. thanks
On Fri, Jun 1, 2018 at 1:49 PM, Honghui Zhang <honghui.zhang@mediatek.com> wrote: > On Fri, 2018-06-01 at 13:17 +0300, Andy Shevchenko wrote: >> On Fri, Jun 1, 2018 at 6:04 AM, <honghui.zhang@mediatek.com> wrote: >> > From: Honghui Zhang <honghui.zhang@mediatek.com> >> >> > +#ifdef CONFIG_PM_SLEEP >> > +static int mtk_pcie_suspend_noirq(struct device *dev) >> >> __maybe_unused >> > > Hi, Andy, thanks for your review. > Bjorn had point this out that at: > https://www.spinics.net/lists/arm-kernel/msg656774.html Nice, one more maintainer with strong opinion here. Arnd, that's what I mentioned as a split in opinions earlier. Any new developer or even existing contributor would be now really confusing since maintainers asked for two different approaches on the same matter.
On Fri, 2018-06-01 at 13:52 +0300, Andy Shevchenko wrote: > On Fri, Jun 1, 2018 at 1:49 PM, Honghui Zhang > <honghui.zhang@mediatek.com> wrote: > > On Fri, 2018-06-01 at 13:17 +0300, Andy Shevchenko wrote: > >> On Fri, Jun 1, 2018 at 6:04 AM, <honghui.zhang@mediatek.com> wrote: > >> > From: Honghui Zhang <honghui.zhang@mediatek.com> > >> > >> > +#ifdef CONFIG_PM_SLEEP > >> > +static int mtk_pcie_suspend_noirq(struct device *dev) > >> > >> __maybe_unused > >> > > > > Hi, Andy, thanks for your review. > > Bjorn had point this out that at: > > https://www.spinics.net/lists/arm-kernel/msg656774.html > > Nice, one more maintainer with strong opinion here. > > Arnd, that's what I mentioned as a split in opinions earlier. > Any new developer or even existing contributor would be now really > confusing since maintainers asked for two different approaches on the > same matter. > Hi, Arnd, Do you have any comments about this patch? Sorry I missed the previous discussion about which should be used for this approach. thanks.
diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c index dabf1086..5363cc7 100644 --- a/drivers/pci/host/pcie-mediatek.c +++ b/drivers/pci/host/pcie-mediatek.c @@ -132,12 +132,14 @@ struct mtk_pcie_port; /** * struct mtk_pcie_soc - differentiate between host generations * @need_fix_class_id: whether this host's class ID needed to be fixed or not + * @pm_support: whether the host's MTCMOS will be off when suspend * @ops: pointer to configuration access functions * @startup: pointer to controller setting functions * @setup_irq: pointer to initialize IRQ functions */ struct mtk_pcie_soc { bool need_fix_class_id; + bool pm_support; struct pci_ops *ops; int (*startup)(struct mtk_pcie_port *port); int (*setup_irq)(struct mtk_pcie_port *port, struct device_node *node); @@ -1179,12 +1181,69 @@ static int mtk_pcie_probe(struct platform_device *pdev) return err; } +#ifdef CONFIG_PM_SLEEP +static int mtk_pcie_suspend_noirq(struct device *dev) +{ + struct mtk_pcie *pcie = dev_get_drvdata(dev); + const struct mtk_pcie_soc *soc = pcie->soc; + struct mtk_pcie_port *port; + + if (!soc->pm_support) + return 0; + + list_for_each_entry(port, &pcie->ports, list) { + clk_disable_unprepare(port->ahb_ck); + clk_disable_unprepare(port->sys_ck); + phy_power_off(port->phy); + } + + return 0; +} + +static int mtk_pcie_resume_noirq(struct device *dev) +{ + struct mtk_pcie *pcie = dev_get_drvdata(dev); + const struct mtk_pcie_soc *soc = pcie->soc; + struct mtk_pcie_port *port; + int ret; + + if (!soc->pm_support) + return 0; + + list_for_each_entry(port, &pcie->ports, list) { + phy_power_on(port->phy); + clk_prepare_enable(port->sys_ck); + clk_prepare_enable(port->ahb_ck); + + ret = soc->startup(port); + if (ret) { + dev_err(dev, "Port%d link down\n", port->slot); + phy_power_off(port->phy); + clk_disable_unprepare(port->sys_ck); + clk_disable_unprepare(port->ahb_ck); + return ret; + } + + if (IS_ENABLED(CONFIG_PCI_MSI)) + mtk_pcie_enable_msi(port); + } + + return 0; +} +#endif + +static const struct dev_pm_ops mtk_pcie_pm_ops = { + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(mtk_pcie_suspend_noirq, + mtk_pcie_resume_noirq) +}; + static const struct mtk_pcie_soc mtk_pcie_soc_v1 = { .ops = &mtk_pcie_ops, .startup = mtk_pcie_startup_port, }; static const struct mtk_pcie_soc mtk_pcie_soc_mt2712 = { + .pm_support = true, .ops = &mtk_pcie_ops_v2, .startup = mtk_pcie_startup_port_v2, .setup_irq = mtk_pcie_setup_irq, @@ -1211,6 +1270,7 @@ static struct platform_driver mtk_pcie_driver = { .name = "mtk-pcie", .of_match_table = mtk_pcie_ids, .suppress_bind_attrs = true, + .pm = &mtk_pcie_pm_ops, }, }; builtin_platform_driver(mtk_pcie_driver);