On 31 May 2018 14:26, Kishon Vijay Abraham I wrote:
> On Tuesday 22 May 2018 06:57 PM, Alan Douglas wrote:
> > If PHYs are present, they will be initialized and enabled in driver probe,
> > and disabled in driver shutdown.
> >
> > Signed-off-by: Alan Douglas <adouglas@xxxxxxxxxxx>
> > ---
> > drivers/pci/cadence/pcie-cadence-ep.c | 14 ++++-
> > drivers/pci/cadence/pcie-cadence-host.c | 31 +++++++++++
> > drivers/pci/cadence/pcie-cadence.c | 93 +++++++++++++++++++++++++++++++++
> > drivers/pci/cadence/pcie-cadence.h | 7 +++
> > 4 files changed, 144 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/cadence/pcie-cadence-ep.c b/drivers/pci/cadence/pcie-cadence-ep.c
> > index 3d8283e..2581caf 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,7 +547,7 @@ 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 = {
> > diff --git a/drivers/pci/cadence/pcie-cadence-host.c b/drivers/pci/cadence/pcie-cadence-host.c
> > index a4ebbd3..7536926a 100644
> > --- a/drivers/pci/cadence/pcie-cadence-host.c
> > +++ b/drivers/pci/cadence/pcie-cadence-host.c
> > @@ -58,6 +58,9 @@ static void __iomem *cdns_pci_map_bus(struct pci_bus *bus, unsigned int devfn,
> >
> > return pcie->reg_base + (where & 0xfff);
> > }
> > + /* Check that the link is up */
> > + if (!(cdns_pcie_readl(pcie, CDNS_PCIE_LM_BASE) & 0x1))
> > + return NULL;
> >
> > /* Update Output registers for AXI region 0. */
> > addr0 = CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_NBITS(12) |
> > @@ -239,6 +242,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 +294,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 +333,35 @@ 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);
> > +}
>
> shutdown callback can be added in a separate patch.
>
Will move to a separate patch in v3
> > +
> > +
> spurious blank line.
Will remove in v3
> > static struct platform_driver cdns_pcie_host_driver = {
> > .driver = {
> > .name = "cdns-pcie-host",
> > .of_match_table = cdns_pcie_host_of_match,
> > },
> > .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..681609a 100644
> > --- a/drivers/pci/cadence/pcie-cadence.c
> > +++ b/drivers/pci/cadence/pcie-cadence.c
> > @@ -124,3 +124,96 @@ 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");
>
> It's preferable to have num-lanes property, which can be used to configure the
> number of lanes supported in a platform even if it doesn't populate PHYs property.
There is no suitable register in the core IP to configure num-lanes, this is
configured externally to the IP, and so can be read from an IP register.
> > + 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);
>
> For optional PHYs, devm_phy_optional_get() should be used.
>
I'll prepare a v3 using devm_phy_optional_get()
Thanks,
Alan
