On Wed, May 16, 2018 at 01:06:05PM +0000, Alan Douglas wrote: > From: Alan Douglas <adouglas@xxxxxxxxxxx> > > This patch is based on next branch in Bjorn Helgaas' linux-pci git repository. This sentence does not belong in a commit log and you should not be basing patches on top of Bjorn's next branch, use v4.17-rc1, that is an immutable commit. > Allow optional list of generic PHYs to be provided via DTS for cadence > RP and EP drivers. Added power management ops which will > enable/disable these PHYs. Corrected parameters for cdns_pcie_writel > function, value to be written had too small width. Too many things at once. Each patch must solve one logical problem and one only - it is easier to understand why if you think that a patch may need to be reverted - if many changes are lumped together this becomes complicated, that's one of the reasons. So, please split this patch and send a v2, thank you. Lorenzo > Signed-off-by: Alan Douglas <adouglas@xxxxxxxxxxx> > --- > .../devicetree/bindings/pci/cdns,cdns-pcie-ep.txt | 4 + > .../bindings/pci/cdns,cdns-pcie-host.txt | 5 + > drivers/pci/cadence/pcie-cadence-ep.c | 15 +++- > drivers/pci/cadence/pcie-cadence-host.c | 36 ++++++- > drivers/pci/cadence/pcie-cadence.c | 123 ++++++++++++++++++++ > drivers/pci/cadence/pcie-cadence.h | 13 ++- > 6 files changed, 193 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.txt b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.txt > index 9a30523..e40c635 100644 > --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.txt > +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.txt > @@ -9,6 +9,8 @@ Required properties: > > Optional properties: > - max-functions: Maximum number of functions that can be configured (default 1). > +- phys: From PHY bindings: List of Generic PHY phandles. > +- phy-names: List of names to identify the PHY. > > Example: > > @@ -19,4 +21,6 @@ pcie@fc000000 { > reg-names = "reg", "mem"; > cdns,max-outbound-regions = <16>; > max-functions = /bits/ 8 <8>; > + phys = <&ep_phy0 &ep_phy1>; > + phy-names = "pcie-lane0","pcie-lane1"; > }; > diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt > index 20a33f3..13be218 100644 > --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt > +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.txt > @@ -24,6 +24,8 @@ Optional properties: > translations (default 32) > - vendor-id: The PCI vendor ID (16 bits, default is design dependent) > - device-id: The PCI device ID (16 bits, default is design dependent) > +- phys: From PHY bindings: List of Generic PHY phandles. > +- phy-names: List of names to identify the PHY. > > Example: > > @@ -57,4 +59,7 @@ pcie@fb000000 { > interrupt-map-mask = <0x0 0x0 0x0 0x7>; > > msi-parent = <&its_pci>; > + > + phys = <&pcie_phy0>; > + phy-names = "pcie-phy"; > }; > diff --git a/drivers/pci/cadence/pcie-cadence-ep.c b/drivers/pci/cadence/pcie-cadence-ep.c > index 3d8283e..e74b8a4 100644 > --- a/drivers/pci/cadence/pcie-cadence-ep.c > +++ b/drivers/pci/cadence/pcie-cadence-ep.c > @@ -439,6 +439,7 @@ static int cdns_pcie_ep_probe(struct platform_device *pdev) > struct pci_epc *epc; > struct resource *res; > int ret; > + int phy_count; > > ep = devm_kzalloc(dev, sizeof(*ep), GFP_KERNEL); > if (!ep) > @@ -472,6 +473,12 @@ static int cdns_pcie_ep_probe(struct platform_device *pdev) > if (!ep->ob_addr) > return -ENOMEM; > > + ret = cdns_pcie_init_phy(dev, pcie); > + if (ret) { > + dev_err(dev, "failed to init phy\n"); > + return ret; > + } > + platform_set_drvdata(pdev, pcie); > pm_runtime_enable(dev); > ret = pm_runtime_get_sync(dev); > if (ret < 0) { > @@ -520,6 +527,10 @@ static int cdns_pcie_ep_probe(struct platform_device *pdev) > > err_get_sync: > pm_runtime_disable(dev); > + cdns_pcie_disable_phy(pcie); > + phy_count = pcie->phy_count; > + while (phy_count--) > + device_link_del(pcie->link[phy_count]); > > return ret; > } > @@ -527,6 +538,7 @@ static int cdns_pcie_ep_probe(struct platform_device *pdev) > static void cdns_pcie_ep_shutdown(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > + struct cdns_pcie *pcie = dev_get_drvdata(dev); > int ret; > > ret = pm_runtime_put_sync(dev); > @@ -535,13 +547,14 @@ static void cdns_pcie_ep_shutdown(struct platform_device *pdev) > > pm_runtime_disable(dev); > > - /* The PCIe controller can't be disabled. */ > + cdns_pcie_disable_phy(pcie); > } > > static struct platform_driver cdns_pcie_ep_driver = { > .driver = { > .name = "cdns-pcie-ep", > .of_match_table = cdns_pcie_ep_of_match, > + .pm = &cdns_pcie_pm_ops, > }, > .probe = cdns_pcie_ep_probe, > .shutdown = cdns_pcie_ep_shutdown, > diff --git a/drivers/pci/cadence/pcie-cadence-host.c b/drivers/pci/cadence/pcie-cadence-host.c > index a4ebbd3..992ebe2 100644 > --- a/drivers/pci/cadence/pcie-cadence-host.c > +++ b/drivers/pci/cadence/pcie-cadence-host.c > @@ -37,7 +37,6 @@ struct cdns_pcie_rc { > u16 vendor_id; > u16 device_id; > }; > - > static void __iomem *cdns_pci_map_bus(struct pci_bus *bus, unsigned int devfn, > int where) > { > @@ -46,6 +45,7 @@ struct cdns_pcie_rc { > struct cdns_pcie *pcie = &rc->pcie; > unsigned int busn = bus->number; > u32 addr0, desc0; > + u32 link_status; > > if (busn == rc->bus_range->start) { > /* > @@ -58,6 +58,11 @@ struct cdns_pcie_rc { > > return pcie->reg_base + (where & 0xfff); > } > + /* Check that the link is up. Clear AXI link-down status */ > + link_status = cdns_pcie_readl(pcie, CDNS_PCIE_LM_BASE); > + if (!(link_status & 0x1)) > + return NULL; > + cdns_pcie_writel(pcie, CDNS_PCIE_AT_LINKDOWN, 0x0); > > /* Update Output registers for AXI region 0. */ > addr0 = CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_NBITS(12) | > @@ -239,6 +244,7 @@ static int cdns_pcie_host_probe(struct platform_device *pdev) > struct cdns_pcie *pcie; > struct resource *res; > int ret; > + int phy_count; > > bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc)); > if (!bridge) > @@ -290,6 +296,13 @@ static int cdns_pcie_host_probe(struct platform_device *pdev) > } > pcie->mem_res = res; > > + ret = cdns_pcie_init_phy(dev, pcie); > + if (ret) { > + dev_err(dev, "failed to init phy\n"); > + return ret; > + } > + platform_set_drvdata(pdev, pcie); > + > pm_runtime_enable(dev); > ret = pm_runtime_get_sync(dev); > if (ret < 0) { > @@ -322,15 +335,36 @@ static int cdns_pcie_host_probe(struct platform_device *pdev) > > err_get_sync: > pm_runtime_disable(dev); > + cdns_pcie_disable_phy(pcie); > + phy_count = pcie->phy_count; > + while (phy_count--) > + device_link_del(pcie->link[phy_count]); > > return ret; > } > > +static void cdns_pcie_shutdown(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct cdns_pcie *pcie = dev_get_drvdata(dev); > + int ret; > + > + ret = pm_runtime_put_sync(dev); > + if (ret < 0) > + dev_dbg(dev, "pm_runtime_put_sync failed\n"); > + > + pm_runtime_disable(dev); > + cdns_pcie_disable_phy(pcie); > +} > + > + > static struct platform_driver cdns_pcie_host_driver = { > .driver = { > .name = "cdns-pcie-host", > .of_match_table = cdns_pcie_host_of_match, > + .pm = &cdns_pcie_pm_ops, > }, > .probe = cdns_pcie_host_probe, > + .shutdown = cdns_pcie_shutdown, > }; > builtin_platform_driver(cdns_pcie_host_driver); > diff --git a/drivers/pci/cadence/pcie-cadence.c b/drivers/pci/cadence/pcie-cadence.c > index 138d113..7a34780 100644 > --- a/drivers/pci/cadence/pcie-cadence.c > +++ b/drivers/pci/cadence/pcie-cadence.c > @@ -124,3 +124,126 @@ void cdns_pcie_reset_outbound_region(struct cdns_pcie *pcie, u32 r) > cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR0(r), 0); > cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR1(r), 0); > } > + > +void cdns_pcie_disable_phy(struct cdns_pcie *pcie) > +{ > + int i = pcie->phy_count; > + > + while (i--) { > + phy_power_off(pcie->phy[i]); > + phy_exit(pcie->phy[i]); > + } > +} > + > +int cdns_pcie_enable_phy(struct cdns_pcie *pcie) > +{ > + int ret; > + int i; > + > + for (i = 0; i < pcie->phy_count; i++) { > + ret = phy_init(pcie->phy[i]); > + if (ret < 0) > + goto err_phy; > + > + ret = phy_power_on(pcie->phy[i]); > + if (ret < 0) { > + phy_exit(pcie->phy[i]); > + goto err_phy; > + } > + } > + > + return 0; > + > +err_phy: > + while (--i >= 0) { > + phy_power_off(pcie->phy[i]); > + phy_exit(pcie->phy[i]); > + } > + > + return ret; > +} > + > +int cdns_pcie_init_phy(struct device *dev, struct cdns_pcie *pcie) > +{ > + struct device_node *np = dev->of_node; > + int phy_count; > + struct phy **phy; > + struct device_link **link; > + int i; > + int ret; > + const char *name; > + > + phy_count = of_property_count_strings(np, "phy-names"); > + if (phy_count < 1) { > + dev_err(dev, "no phy-names. PHY will not be initialized\n"); > + pcie->phy_count = 0; > + return 0; > + } > + > + phy = devm_kzalloc(dev, sizeof(*phy) * phy_count, GFP_KERNEL); > + 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++) { > + of_property_read_string_index(np, "phy-names", i, &name); > + phy[i] = devm_phy_get(dev, name); > + if (IS_ERR(phy)) > + return PTR_ERR(phy); > + > + link[i] = device_link_add(dev, &phy[i]->dev, DL_FLAG_STATELESS); > + if (!link[i]) { > + ret = -EINVAL; > + goto err_link; > + } > + } > + > + pcie->phy_count = phy_count; > + pcie->phy = phy; > + pcie->link = link; > + > + ret = cdns_pcie_enable_phy(pcie); > + if (ret) > + goto err_link; > + > + return 0; > + > +err_link: > + while (--i >= 0) > + device_link_del(link[i]); > + > + return ret; > +} > + > +#ifdef CONFIG_PM_SLEEP > +static int cdns_pcie_suspend_noirq(struct device *dev) > +{ > + struct cdns_pcie *pcie = dev_get_drvdata(dev); > + > + cdns_pcie_disable_phy(pcie); > + > + return 0; > +} > + > +static int cdns_pcie_resume_noirq(struct device *dev) > +{ > + struct cdns_pcie *pcie = dev_get_drvdata(dev); > + int ret; > + > + ret = cdns_pcie_enable_phy(pcie); > + if (ret) { > + dev_err(dev, "failed to enable phy\n"); > + return ret; > + } > + > + return 0; > +} > +#endif > + > +const struct dev_pm_ops cdns_pcie_pm_ops = { > + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(cdns_pcie_suspend_noirq, > + cdns_pcie_resume_noirq) > +}; > diff --git a/drivers/pci/cadence/pcie-cadence.h b/drivers/pci/cadence/pcie-cadence.h > index 4bb2733..ae6bf2a 100644 > --- a/drivers/pci/cadence/pcie-cadence.h > +++ b/drivers/pci/cadence/pcie-cadence.h > @@ -8,6 +8,7 @@ > > #include <linux/kernel.h> > #include <linux/pci.h> > +#include <linux/phy/phy.h> > > /* > * Local Management Registers > @@ -165,6 +166,9 @@ > #define CDNS_PCIE_AT_IB_RP_BAR_ADDR1(bar) \ > (CDNS_PCIE_AT_BASE + 0x0804 + (bar) * 0x0008) > > +/* AXI link down register */ > +#define CDNS_PCIE_AT_LINKDOWN (CDNS_PCIE_AT_BASE + 0x0824) > + > enum cdns_pcie_rp_bar { > RP_BAR0, > RP_BAR1, > @@ -229,6 +233,9 @@ struct cdns_pcie { > struct resource *mem_res; > bool is_rc; > u8 bus; > + int phy_count; > + struct phy **phy; > + struct device_link **link; > }; > > /* Register access */ > @@ -279,7 +286,7 @@ static inline void cdns_pcie_ep_fn_writew(struct cdns_pcie *pcie, u8 fn, > } > > static inline void cdns_pcie_ep_fn_writel(struct cdns_pcie *pcie, u8 fn, > - u32 reg, u16 value) > + u32 reg, u32 value) > { > writel(value, pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg); > } > @@ -307,5 +314,9 @@ void cdns_pcie_set_outbound_region_for_normal_msg(struct cdns_pcie *pcie, u8 fn, > u32 r, u64 cpu_addr); > > void cdns_pcie_reset_outbound_region(struct cdns_pcie *pcie, u32 r); > +void cdns_pcie_disable_phy(struct cdns_pcie *pcie); > +int cdns_pcie_enable_phy(struct cdns_pcie *pcie); > +int cdns_pcie_init_phy(struct device *dev, struct cdns_pcie *pcie); > +extern const struct dev_pm_ops cdns_pcie_pm_ops; > > #endif /* _PCIE_CADENCE_H */ > -- > 1.7.1 >