* [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs @ 2016-04-08 16:13 Thierry Reding [not found] ` <1460131994-24493-1-git-send-email-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 21+ messages in thread From: Thierry Reding @ 2016-04-08 16:13 UTC (permalink / raw) To: Bjorn Helgaas Cc: Stephen Warren, Alexandre Courbot, linux-tegra, linux-pci, devicetree From: Thierry Reding <treding@nvidia.com> Changes to the pad controller device tree binding have required that each lane be associated with a separate PHY. Update the PCI host bridge device tree binding to allow each root port to define the list of PHYs required to drive the lanes associated with it. Acked-by: Rob Herring <robh@kernel.org> Signed-off-by: Thierry Reding <treding@nvidia.com> --- Changes in v4: - add additional lanes subnode when dereferencing PHYs from the XUSB pad controller to reflect changes in its binding .../devicetree/bindings/pci/nvidia,tegra20-pcie.txt | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt index 75321ae23c08..f5364084b494 100644 --- a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt @@ -60,11 +60,14 @@ Required properties: - afi - pcie_x -Required properties on Tegra124 and later: +Required properties on Tegra124 and later (deprecated): - phys: Must contain an entry for each entry in phy-names. - phy-names: Must include the following entries: - pcie +These properties are deprecated in favour of per-lane PHYs define in each of +the root ports (see below). + Power supplies for Tegra20: - avdd-pex-supply: Power supply for analog PCIe logic. Must supply 1.05 V. - vdd-pex-supply: Power supply for digital PCIe I/O. Must supply 1.05 V. @@ -122,6 +125,13 @@ Required properties: - Root port 0 uses 4 lanes, root port 1 is unused. - Both root ports use 2 lanes. +Required properties for Tegra124 and later: +- phys: Must contain an phandle to a PHY for each entry in phy-names. +- phy-names: Must include an entry for each active lane. Note that the number + of entries does not have to (though usually will) be equal to the specified + number of lanes in the nvidia,num-lanes property. Entries are of the form + "pcie-N": where N ranges from 0 to the value specified in nvidia,num-lanes. + Example: SoC DTSI: @@ -169,6 +179,9 @@ SoC DTSI: ranges; nvidia,num-lanes = <2>; + + phys = <&{/padctl@0,7009f000/pads/pcie/lanes/pcie-4}>; + phy-names = "pcie-0"; }; pci@2,0 { @@ -183,6 +196,9 @@ SoC DTSI: ranges; nvidia,num-lanes = <2>; + + phys = <&{/padctl@0,7009f000/pads/pcie/lanes/pcie-2}>; + phy-names = "pcie-0"; }; }; -- 2.8.0 ^ permalink raw reply related [flat|nested] 21+ messages in thread
[parent not found: <1460131994-24493-1-git-send-email-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* [PATCH v4 2/2] PCI: tegra: Support per-lane PHYs 2016-04-08 16:13 [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs Thierry Reding @ 2016-04-08 16:13 ` Thierry Reding 0 siblings, 0 replies; 21+ messages in thread From: Thierry Reding @ 2016-04-08 16:13 UTC (permalink / raw) To: Bjorn Helgaas Cc: Stephen Warren, Alexandre Courbot, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-pci-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> The current XUSB pad controller bindings are insufficient to describe PHY devices attached to USB controllers. New bindings have been created to overcome these restrictions. As a side-effect each root port now is assigned a set of PHY devices, one for each lane associated with the root port. This has the benefit of allowing fine-grained control of the power management for each lane. Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> --- Changes in v4: - propagate failure from PHY power on - refactor PHY power off sequence Changes in v3: - cache result of check for new PHY bindings usage (Stephen Warren) Changes in v2: - rework commit message to more accurately describe this change drivers/pci/host/pci-tegra.c | 243 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 226 insertions(+), 17 deletions(-) diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c index 68d1f41b3cbf..d50568bf93c5 100644 --- a/drivers/pci/host/pci-tegra.c +++ b/drivers/pci/host/pci-tegra.c @@ -295,6 +295,7 @@ struct tegra_pcie { struct reset_control *afi_rst; struct reset_control *pcie_xrst; + bool legacy_phy; struct phy *phy; struct tegra_msi msi; @@ -311,11 +312,14 @@ struct tegra_pcie { struct tegra_pcie_port { struct tegra_pcie *pcie; + struct device_node *np; struct list_head list; struct resource regs; void __iomem *base; unsigned int index; unsigned int lanes; + + struct phy **phys; }; struct tegra_pcie_bus { @@ -860,6 +864,128 @@ static int tegra_pcie_phy_enable(struct tegra_pcie *pcie) return 0; } +static int tegra_pcie_phy_disable(struct tegra_pcie *pcie) +{ + const struct tegra_pcie_soc_data *soc = pcie->soc_data; + u32 value; + + /* disable TX/RX data */ + value = pads_readl(pcie, PADS_CTL); + value &= ~(PADS_CTL_TX_DATA_EN_1L | PADS_CTL_RX_DATA_EN_1L); + pads_writel(pcie, value, PADS_CTL); + + /* override IDDQ */ + value = pads_readl(pcie, PADS_CTL); + value |= PADS_CTL_IDDQ_1L; + pads_writel(pcie, PADS_CTL, value); + + /* reset PLL */ + value = pads_readl(pcie, soc->pads_pll_ctl); + value &= ~PADS_PLL_CTL_RST_B4SM; + pads_writel(pcie, value, soc->pads_pll_ctl); + + usleep_range(20, 100); + + return 0; +} + +static int tegra_pcie_port_phy_power_on(struct tegra_pcie_port *port) +{ + struct device *dev = port->pcie->dev; + unsigned int i; + int err; + + for (i = 0; i < port->lanes; i++) { + err = phy_power_on(port->phys[i]); + if (err < 0) { + dev_err(dev, "failed to power on PHY#%u: %d\n", i, + err); + return err; + } + } + + return 0; +} + +static int tegra_pcie_port_phy_power_off(struct tegra_pcie_port *port) +{ + struct device *dev = port->pcie->dev; + unsigned int i; + int err; + + for (i = 0; i < port->lanes; i++) { + err = phy_power_off(port->phys[i]); + if (err < 0) { + dev_err(dev, "failed to power off PHY#%u: %d\n", i, + err); + return err; + } + } + + return 0; +} + +static int tegra_pcie_phy_power_on(struct tegra_pcie *pcie) +{ + struct tegra_pcie_port *port; + int err; + + if (pcie->legacy_phy) { + if (pcie->phy) + err = phy_power_on(pcie->phy); + else + err = tegra_pcie_phy_enable(pcie); + + if (err < 0) + dev_err(pcie->dev, "failed to power on PHY: %d\n", err); + + return err; + } + + list_for_each_entry(port, &pcie->ports, list) { + err = tegra_pcie_port_phy_power_on(port); + if (err < 0) { + dev_err(pcie->dev, + "failed to power on PCIe port %u PHY: %d\n", + port->index, err); + return err; + } + } + + return 0; +} + +static int tegra_pcie_phy_power_off(struct tegra_pcie *pcie) +{ + struct tegra_pcie_port *port; + int err; + + if (pcie->legacy_phy) { + if (pcie->phy) + err = phy_power_on(pcie->phy); + else + err = tegra_pcie_phy_disable(pcie); + + if (err < 0) + dev_err(pcie->dev, "failed to power off PHY: %d\n", + err); + + return err; + } + + list_for_each_entry(port, &pcie->ports, list) { + err = tegra_pcie_port_phy_power_off(port); + if (err < 0) { + dev_err(pcie->dev, + "failed to power off PCIe port %u PHY: %d\n", + port->index, err); + return err; + } + } + + return 0; +} + static int tegra_pcie_enable_controller(struct tegra_pcie *pcie) { const struct tegra_pcie_soc_data *soc = pcie->soc_data; @@ -899,13 +1025,9 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie) afi_writel(pcie, value, AFI_FUSE); } - if (!pcie->phy) - err = tegra_pcie_phy_enable(pcie); - else - err = phy_power_on(pcie->phy); - + err = tegra_pcie_phy_power_on(pcie); if (err < 0) { - dev_err(pcie->dev, "failed to power on PHY: %d\n", err); + dev_err(pcie->dev, "failed to power off PHY(s): %d\n", err); return err; } @@ -942,9 +1064,9 @@ static void tegra_pcie_power_off(struct tegra_pcie *pcie) /* TODO: disable and unprepare clocks? */ - err = phy_power_off(pcie->phy); + err = tegra_pcie_phy_power_off(pcie); if (err < 0) - dev_warn(pcie->dev, "failed to power off PHY: %d\n", err); + dev_err(pcie->dev, "failed to power off PHY(s): %d\n", err); reset_control_assert(pcie->pcie_xrst); reset_control_assert(pcie->afi_rst); @@ -1049,6 +1171,99 @@ static int tegra_pcie_resets_get(struct tegra_pcie *pcie) return 0; } +static int tegra_pcie_phys_get_legacy(struct tegra_pcie *pcie) +{ + int err; + + pcie->phy = devm_phy_optional_get(pcie->dev, "pcie"); + if (IS_ERR(pcie->phy)) { + err = PTR_ERR(pcie->phy); + dev_err(pcie->dev, "failed to get PHY: %d\n", err); + return err; + } + + err = phy_init(pcie->phy); + if (err < 0) { + dev_err(pcie->dev, "failed to initialize PHY: %d\n", err); + return err; + } + + pcie->legacy_phy = true; + + return 0; +} + +static struct phy *devm_of_phy_optional_get_index(struct device *dev, + struct device_node *np, + const char *consumer, + unsigned int index) +{ + struct phy *phy; + char *name; + + name = kasprintf(GFP_KERNEL, "%s-%u", consumer, index); + if (!name) + return ERR_PTR(-ENOMEM); + + phy = devm_of_phy_get(dev, np, name); + kfree(name); + + if (IS_ERR(phy) && PTR_ERR(phy) == -ENODEV) + phy = NULL; + + return phy; +} + +static int tegra_pcie_port_get_phys(struct tegra_pcie_port *port) +{ + struct device *dev = port->pcie->dev; + struct phy *phy; + unsigned int i; + int err; + + port->phys = devm_kcalloc(dev, sizeof(phy), port->lanes, GFP_KERNEL); + if (!port->phys) + return -ENOMEM; + + for (i = 0; i < port->lanes; i++) { + phy = devm_of_phy_optional_get_index(dev, port->np, "pcie", i); + if (IS_ERR(phy)) { + dev_err(dev, "failed to get PHY#%u: %ld\n", i, + PTR_ERR(phy)); + return PTR_ERR(phy); + } + + err = phy_init(phy); + if (err < 0) { + dev_err(dev, "failed to initialize PHY#%u: %d\n", i, + err); + return err; + } + + port->phys[i] = phy; + } + + return 0; +} + +static int tegra_pcie_phys_get(struct tegra_pcie *pcie) +{ + struct tegra_pcie_port *port; + int err; + + if (of_get_property(pcie->dev->of_node, "phys", NULL) != NULL) + return tegra_pcie_phys_get_legacy(pcie); + + list_for_each_entry(port, &pcie->ports, list) { + err = tegra_pcie_port_get_phys(port); + if (err < 0) { + return err; + } + } + + return 0; +} + static int tegra_pcie_get_resources(struct tegra_pcie *pcie) { struct platform_device *pdev = to_platform_device(pcie->dev); @@ -1067,16 +1282,9 @@ static int tegra_pcie_get_resources(struct tegra_pcie *pcie) return err; } - pcie->phy = devm_phy_optional_get(pcie->dev, "pcie"); - if (IS_ERR(pcie->phy)) { - err = PTR_ERR(pcie->phy); - dev_err(&pdev->dev, "failed to get PHY: %d\n", err); - return err; - } - - err = phy_init(pcie->phy); + err = tegra_pcie_phys_get(pcie); if (err < 0) { - dev_err(&pdev->dev, "failed to initialize PHY: %d\n", err); + dev_err(&pdev->dev, "failed to get PHYs: %d\n", err); return err; } @@ -1752,6 +1960,7 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie) rp->index = index; rp->lanes = value; rp->pcie = pcie; + rp->np = port; rp->base = devm_ioremap_resource(pcie->dev, &rp->regs); if (IS_ERR(rp->base)) -- 2.8.0 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v4 2/2] PCI: tegra: Support per-lane PHYs @ 2016-04-08 16:13 ` Thierry Reding 0 siblings, 0 replies; 21+ messages in thread From: Thierry Reding @ 2016-04-08 16:13 UTC (permalink / raw) To: Bjorn Helgaas Cc: Stephen Warren, Alexandre Courbot, linux-tegra, linux-pci, devicetree From: Thierry Reding <treding@nvidia.com> The current XUSB pad controller bindings are insufficient to describe PHY devices attached to USB controllers. New bindings have been created to overcome these restrictions. As a side-effect each root port now is assigned a set of PHY devices, one for each lane associated with the root port. This has the benefit of allowing fine-grained control of the power management for each lane. Signed-off-by: Thierry Reding <treding@nvidia.com> --- Changes in v4: - propagate failure from PHY power on - refactor PHY power off sequence Changes in v3: - cache result of check for new PHY bindings usage (Stephen Warren) Changes in v2: - rework commit message to more accurately describe this change drivers/pci/host/pci-tegra.c | 243 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 226 insertions(+), 17 deletions(-) diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c index 68d1f41b3cbf..d50568bf93c5 100644 --- a/drivers/pci/host/pci-tegra.c +++ b/drivers/pci/host/pci-tegra.c @@ -295,6 +295,7 @@ struct tegra_pcie { struct reset_control *afi_rst; struct reset_control *pcie_xrst; + bool legacy_phy; struct phy *phy; struct tegra_msi msi; @@ -311,11 +312,14 @@ struct tegra_pcie { struct tegra_pcie_port { struct tegra_pcie *pcie; + struct device_node *np; struct list_head list; struct resource regs; void __iomem *base; unsigned int index; unsigned int lanes; + + struct phy **phys; }; struct tegra_pcie_bus { @@ -860,6 +864,128 @@ static int tegra_pcie_phy_enable(struct tegra_pcie *pcie) return 0; } +static int tegra_pcie_phy_disable(struct tegra_pcie *pcie) +{ + const struct tegra_pcie_soc_data *soc = pcie->soc_data; + u32 value; + + /* disable TX/RX data */ + value = pads_readl(pcie, PADS_CTL); + value &= ~(PADS_CTL_TX_DATA_EN_1L | PADS_CTL_RX_DATA_EN_1L); + pads_writel(pcie, value, PADS_CTL); + + /* override IDDQ */ + value = pads_readl(pcie, PADS_CTL); + value |= PADS_CTL_IDDQ_1L; + pads_writel(pcie, PADS_CTL, value); + + /* reset PLL */ + value = pads_readl(pcie, soc->pads_pll_ctl); + value &= ~PADS_PLL_CTL_RST_B4SM; + pads_writel(pcie, value, soc->pads_pll_ctl); + + usleep_range(20, 100); + + return 0; +} + +static int tegra_pcie_port_phy_power_on(struct tegra_pcie_port *port) +{ + struct device *dev = port->pcie->dev; + unsigned int i; + int err; + + for (i = 0; i < port->lanes; i++) { + err = phy_power_on(port->phys[i]); + if (err < 0) { + dev_err(dev, "failed to power on PHY#%u: %d\n", i, + err); + return err; + } + } + + return 0; +} + +static int tegra_pcie_port_phy_power_off(struct tegra_pcie_port *port) +{ + struct device *dev = port->pcie->dev; + unsigned int i; + int err; + + for (i = 0; i < port->lanes; i++) { + err = phy_power_off(port->phys[i]); + if (err < 0) { + dev_err(dev, "failed to power off PHY#%u: %d\n", i, + err); + return err; + } + } + + return 0; +} + +static int tegra_pcie_phy_power_on(struct tegra_pcie *pcie) +{ + struct tegra_pcie_port *port; + int err; + + if (pcie->legacy_phy) { + if (pcie->phy) + err = phy_power_on(pcie->phy); + else + err = tegra_pcie_phy_enable(pcie); + + if (err < 0) + dev_err(pcie->dev, "failed to power on PHY: %d\n", err); + + return err; + } + + list_for_each_entry(port, &pcie->ports, list) { + err = tegra_pcie_port_phy_power_on(port); + if (err < 0) { + dev_err(pcie->dev, + "failed to power on PCIe port %u PHY: %d\n", + port->index, err); + return err; + } + } + + return 0; +} + +static int tegra_pcie_phy_power_off(struct tegra_pcie *pcie) +{ + struct tegra_pcie_port *port; + int err; + + if (pcie->legacy_phy) { + if (pcie->phy) + err = phy_power_on(pcie->phy); + else + err = tegra_pcie_phy_disable(pcie); + + if (err < 0) + dev_err(pcie->dev, "failed to power off PHY: %d\n", + err); + + return err; + } + + list_for_each_entry(port, &pcie->ports, list) { + err = tegra_pcie_port_phy_power_off(port); + if (err < 0) { + dev_err(pcie->dev, + "failed to power off PCIe port %u PHY: %d\n", + port->index, err); + return err; + } + } + + return 0; +} + static int tegra_pcie_enable_controller(struct tegra_pcie *pcie) { const struct tegra_pcie_soc_data *soc = pcie->soc_data; @@ -899,13 +1025,9 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie) afi_writel(pcie, value, AFI_FUSE); } - if (!pcie->phy) - err = tegra_pcie_phy_enable(pcie); - else - err = phy_power_on(pcie->phy); - + err = tegra_pcie_phy_power_on(pcie); if (err < 0) { - dev_err(pcie->dev, "failed to power on PHY: %d\n", err); + dev_err(pcie->dev, "failed to power off PHY(s): %d\n", err); return err; } @@ -942,9 +1064,9 @@ static void tegra_pcie_power_off(struct tegra_pcie *pcie) /* TODO: disable and unprepare clocks? */ - err = phy_power_off(pcie->phy); + err = tegra_pcie_phy_power_off(pcie); if (err < 0) - dev_warn(pcie->dev, "failed to power off PHY: %d\n", err); + dev_err(pcie->dev, "failed to power off PHY(s): %d\n", err); reset_control_assert(pcie->pcie_xrst); reset_control_assert(pcie->afi_rst); @@ -1049,6 +1171,99 @@ static int tegra_pcie_resets_get(struct tegra_pcie *pcie) return 0; } +static int tegra_pcie_phys_get_legacy(struct tegra_pcie *pcie) +{ + int err; + + pcie->phy = devm_phy_optional_get(pcie->dev, "pcie"); + if (IS_ERR(pcie->phy)) { + err = PTR_ERR(pcie->phy); + dev_err(pcie->dev, "failed to get PHY: %d\n", err); + return err; + } + + err = phy_init(pcie->phy); + if (err < 0) { + dev_err(pcie->dev, "failed to initialize PHY: %d\n", err); + return err; + } + + pcie->legacy_phy = true; + + return 0; +} + +static struct phy *devm_of_phy_optional_get_index(struct device *dev, + struct device_node *np, + const char *consumer, + unsigned int index) +{ + struct phy *phy; + char *name; + + name = kasprintf(GFP_KERNEL, "%s-%u", consumer, index); + if (!name) + return ERR_PTR(-ENOMEM); + + phy = devm_of_phy_get(dev, np, name); + kfree(name); + + if (IS_ERR(phy) && PTR_ERR(phy) == -ENODEV) + phy = NULL; + + return phy; +} + +static int tegra_pcie_port_get_phys(struct tegra_pcie_port *port) +{ + struct device *dev = port->pcie->dev; + struct phy *phy; + unsigned int i; + int err; + + port->phys = devm_kcalloc(dev, sizeof(phy), port->lanes, GFP_KERNEL); + if (!port->phys) + return -ENOMEM; + + for (i = 0; i < port->lanes; i++) { + phy = devm_of_phy_optional_get_index(dev, port->np, "pcie", i); + if (IS_ERR(phy)) { + dev_err(dev, "failed to get PHY#%u: %ld\n", i, + PTR_ERR(phy)); + return PTR_ERR(phy); + } + + err = phy_init(phy); + if (err < 0) { + dev_err(dev, "failed to initialize PHY#%u: %d\n", i, + err); + return err; + } + + port->phys[i] = phy; + } + + return 0; +} + +static int tegra_pcie_phys_get(struct tegra_pcie *pcie) +{ + struct tegra_pcie_port *port; + int err; + + if (of_get_property(pcie->dev->of_node, "phys", NULL) != NULL) + return tegra_pcie_phys_get_legacy(pcie); + + list_for_each_entry(port, &pcie->ports, list) { + err = tegra_pcie_port_get_phys(port); + if (err < 0) { + return err; + } + } + + return 0; +} + static int tegra_pcie_get_resources(struct tegra_pcie *pcie) { struct platform_device *pdev = to_platform_device(pcie->dev); @@ -1067,16 +1282,9 @@ static int tegra_pcie_get_resources(struct tegra_pcie *pcie) return err; } - pcie->phy = devm_phy_optional_get(pcie->dev, "pcie"); - if (IS_ERR(pcie->phy)) { - err = PTR_ERR(pcie->phy); - dev_err(&pdev->dev, "failed to get PHY: %d\n", err); - return err; - } - - err = phy_init(pcie->phy); + err = tegra_pcie_phys_get(pcie); if (err < 0) { - dev_err(&pdev->dev, "failed to initialize PHY: %d\n", err); + dev_err(&pdev->dev, "failed to get PHYs: %d\n", err); return err; } @@ -1752,6 +1960,7 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie) rp->index = index; rp->lanes = value; rp->pcie = pcie; + rp->np = port; rp->base = devm_ioremap_resource(pcie->dev, &rp->regs); if (IS_ERR(rp->base)) -- 2.8.0 ^ permalink raw reply related [flat|nested] 21+ messages in thread
[parent not found: <1460131994-24493-2-git-send-email-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v4 2/2] PCI: tegra: Support per-lane PHYs 2016-04-08 16:13 ` Thierry Reding @ 2016-04-08 18:05 ` Bjorn Helgaas -1 siblings, 0 replies; 21+ messages in thread From: Bjorn Helgaas @ 2016-04-08 18:05 UTC (permalink / raw) To: Thierry Reding Cc: Bjorn Helgaas, Stephen Warren, Alexandre Courbot, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-pci-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA Hi Thierry, I think there are a couple typos (one in a message and one that actually looks important), and one question below. On Fri, Apr 08, 2016 at 06:13:14PM +0200, Thierry Reding wrote: > From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> > > The current XUSB pad controller bindings are insufficient to describe > PHY devices attached to USB controllers. New bindings have been created > to overcome these restrictions. As a side-effect each root port now is > assigned a set of PHY devices, one for each lane associated with the > root port. This has the benefit of allowing fine-grained control of the > power management for each lane. > > Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> > --- > Changes in v4: > - propagate failure from PHY power on > - refactor PHY power off sequence > > Changes in v3: > - cache result of check for new PHY bindings usage (Stephen Warren) > > Changes in v2: > - rework commit message to more accurately describe this change > > drivers/pci/host/pci-tegra.c | 243 ++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 226 insertions(+), 17 deletions(-) > > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c > index 68d1f41b3cbf..d50568bf93c5 100644 > --- a/drivers/pci/host/pci-tegra.c > +++ b/drivers/pci/host/pci-tegra.c > @@ -295,6 +295,7 @@ struct tegra_pcie { > struct reset_control *afi_rst; > struct reset_control *pcie_xrst; > > + bool legacy_phy; > struct phy *phy; > > struct tegra_msi msi; > @@ -311,11 +312,14 @@ struct tegra_pcie { > > struct tegra_pcie_port { > struct tegra_pcie *pcie; > + struct device_node *np; > struct list_head list; > struct resource regs; > void __iomem *base; > unsigned int index; > unsigned int lanes; > + > + struct phy **phys; > }; > > struct tegra_pcie_bus { > @@ -860,6 +864,128 @@ static int tegra_pcie_phy_enable(struct tegra_pcie *pcie) > return 0; > } > > +static int tegra_pcie_phy_disable(struct tegra_pcie *pcie) > +{ > + const struct tegra_pcie_soc_data *soc = pcie->soc_data; > + u32 value; > + > + /* disable TX/RX data */ > + value = pads_readl(pcie, PADS_CTL); > + value &= ~(PADS_CTL_TX_DATA_EN_1L | PADS_CTL_RX_DATA_EN_1L); > + pads_writel(pcie, value, PADS_CTL); > + > + /* override IDDQ */ > + value = pads_readl(pcie, PADS_CTL); > + value |= PADS_CTL_IDDQ_1L; > + pads_writel(pcie, PADS_CTL, value); > + > + /* reset PLL */ > + value = pads_readl(pcie, soc->pads_pll_ctl); > + value &= ~PADS_PLL_CTL_RST_B4SM; > + pads_writel(pcie, value, soc->pads_pll_ctl); > + > + usleep_range(20, 100); > + > + return 0; > +} > + > +static int tegra_pcie_port_phy_power_on(struct tegra_pcie_port *port) > +{ > + struct device *dev = port->pcie->dev; > + unsigned int i; > + int err; > + > + for (i = 0; i < port->lanes; i++) { > + err = phy_power_on(port->phys[i]); > + if (err < 0) { > + dev_err(dev, "failed to power on PHY#%u: %d\n", i, > + err); > + return err; > + } > + } > + > + return 0; > +} > + > +static int tegra_pcie_port_phy_power_off(struct tegra_pcie_port *port) > +{ > + struct device *dev = port->pcie->dev; > + unsigned int i; > + int err; > + > + for (i = 0; i < port->lanes; i++) { > + err = phy_power_off(port->phys[i]); > + if (err < 0) { > + dev_err(dev, "failed to power off PHY#%u: %d\n", i, > + err); > + return err; > + } > + } > + > + return 0; > +} > + > +static int tegra_pcie_phy_power_on(struct tegra_pcie *pcie) > +{ > + struct tegra_pcie_port *port; > + int err; > + > + if (pcie->legacy_phy) { > + if (pcie->phy) > + err = phy_power_on(pcie->phy); > + else > + err = tegra_pcie_phy_enable(pcie); > + > + if (err < 0) > + dev_err(pcie->dev, "failed to power on PHY: %d\n", err); > + > + return err; > + } > + > + list_for_each_entry(port, &pcie->ports, list) { > + err = tegra_pcie_port_phy_power_on(port); > + if (err < 0) { > + dev_err(pcie->dev, > + "failed to power on PCIe port %u PHY: %d\n", > + port->index, err); > + return err; > + } > + } > + > + return 0; > +} > + > +static int tegra_pcie_phy_power_off(struct tegra_pcie *pcie) > +{ > + struct tegra_pcie_port *port; > + int err; > + > + if (pcie->legacy_phy) { > + if (pcie->phy) > + err = phy_power_on(pcie->phy); s/phy_power_on/phy_power_off/ > + else > + err = tegra_pcie_phy_disable(pcie); > + > + if (err < 0) > + dev_err(pcie->dev, "failed to power off PHY: %d\n", > + err); > + > + return err; > + } > + > + list_for_each_entry(port, &pcie->ports, list) { > + err = tegra_pcie_port_phy_power_off(port); > + if (err < 0) { > + dev_err(pcie->dev, > + "failed to power off PCIe port %u PHY: %d\n", > + port->index, err); > + return err; > + } > + } > + > + return 0; > +} > + > static int tegra_pcie_enable_controller(struct tegra_pcie *pcie) > { > const struct tegra_pcie_soc_data *soc = pcie->soc_data; > @@ -899,13 +1025,9 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie) > afi_writel(pcie, value, AFI_FUSE); > } > > - if (!pcie->phy) > - err = tegra_pcie_phy_enable(pcie); > - else > - err = phy_power_on(pcie->phy); > - > + err = tegra_pcie_phy_power_on(pcie); > if (err < 0) { > - dev_err(pcie->dev, "failed to power on PHY: %d\n", err); > + dev_err(pcie->dev, "failed to power off PHY(s): %d\n", err); s/off/on/ > return err; > } > > @@ -942,9 +1064,9 @@ static void tegra_pcie_power_off(struct tegra_pcie *pcie) > > /* TODO: disable and unprepare clocks? */ > > - err = phy_power_off(pcie->phy); > + err = tegra_pcie_phy_power_off(pcie); > if (err < 0) > - dev_warn(pcie->dev, "failed to power off PHY: %d\n", err); > + dev_err(pcie->dev, "failed to power off PHY(s): %d\n", err); > > reset_control_assert(pcie->pcie_xrst); > reset_control_assert(pcie->afi_rst); > @@ -1049,6 +1171,99 @@ static int tegra_pcie_resets_get(struct tegra_pcie *pcie) > return 0; > } > > +static int tegra_pcie_phys_get_legacy(struct tegra_pcie *pcie) > +{ > + int err; > + > + pcie->phy = devm_phy_optional_get(pcie->dev, "pcie"); > + if (IS_ERR(pcie->phy)) { > + err = PTR_ERR(pcie->phy); > + dev_err(pcie->dev, "failed to get PHY: %d\n", err); > + return err; > + } > + > + err = phy_init(pcie->phy); > + if (err < 0) { > + dev_err(pcie->dev, "failed to initialize PHY: %d\n", err); > + return err; > + } > + > + pcie->legacy_phy = true; > + > + return 0; > +} > + > +static struct phy *devm_of_phy_optional_get_index(struct device *dev, > + struct device_node *np, > + const char *consumer, > + unsigned int index) > +{ > + struct phy *phy; > + char *name; > + > + name = kasprintf(GFP_KERNEL, "%s-%u", consumer, index); > + if (!name) > + return ERR_PTR(-ENOMEM); > + > + phy = devm_of_phy_get(dev, np, name); > + kfree(name); > + > + if (IS_ERR(phy) && PTR_ERR(phy) == -ENODEV) > + phy = NULL; > + > + return phy; > +} > + > +static int tegra_pcie_port_get_phys(struct tegra_pcie_port *port) > +{ > + struct device *dev = port->pcie->dev; > + struct phy *phy; > + unsigned int i; > + int err; > + > + port->phys = devm_kcalloc(dev, sizeof(phy), port->lanes, GFP_KERNEL); > + if (!port->phys) > + return -ENOMEM; > + > + for (i = 0; i < port->lanes; i++) { > + phy = devm_of_phy_optional_get_index(dev, port->np, "pcie", i); > + if (IS_ERR(phy)) { > + dev_err(dev, "failed to get PHY#%u: %ld\n", i, > + PTR_ERR(phy)); > + return PTR_ERR(phy); > + } > + > + err = phy_init(phy); > + if (err < 0) { > + dev_err(dev, "failed to initialize PHY#%u: %d\n", i, > + err); > + return err; > + } > + > + port->phys[i] = phy; > + } > + > + return 0; > +} > + > +static int tegra_pcie_phys_get(struct tegra_pcie *pcie) > +{ > + struct tegra_pcie_port *port; > + int err; > + > + if (of_get_property(pcie->dev->of_node, "phys", NULL) != NULL) > + return tegra_pcie_phys_get_legacy(pcie); > + > + list_for_each_entry(port, &pcie->ports, list) { > + err = tegra_pcie_port_get_phys(port); > + if (err < 0) { > + return err; > + } > + } This seems backwards: if I'm reading this right, you first check for the legacy property ("phys") and use it if you find it. If there is no legacy property, you look for the new per-lane PHYs. The usual pattern would be "look for the new stuff, and if you don't find it, fall back to the old stuff." Is there a configuration that could be described either way, e.g., something with only one lane and only one PHY? I'm not sure whether it matters, but if it *could* use the "new, fall back to old" pattern, that would be nice and would keep people from wondering whether it's safe to do it backwards. > + > + return 0; > +} > + > static int tegra_pcie_get_resources(struct tegra_pcie *pcie) > { > struct platform_device *pdev = to_platform_device(pcie->dev); > @@ -1067,16 +1282,9 @@ static int tegra_pcie_get_resources(struct tegra_pcie *pcie) > return err; > } > > - pcie->phy = devm_phy_optional_get(pcie->dev, "pcie"); > - if (IS_ERR(pcie->phy)) { > - err = PTR_ERR(pcie->phy); > - dev_err(&pdev->dev, "failed to get PHY: %d\n", err); > - return err; > - } > - > - err = phy_init(pcie->phy); > + err = tegra_pcie_phys_get(pcie); > if (err < 0) { > - dev_err(&pdev->dev, "failed to initialize PHY: %d\n", err); > + dev_err(&pdev->dev, "failed to get PHYs: %d\n", err); > return err; > } > > @@ -1752,6 +1960,7 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie) > rp->index = index; > rp->lanes = value; > rp->pcie = pcie; > + rp->np = port; > > rp->base = devm_ioremap_resource(pcie->dev, &rp->regs); > if (IS_ERR(rp->base)) > -- > 2.8.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 2/2] PCI: tegra: Support per-lane PHYs @ 2016-04-08 18:05 ` Bjorn Helgaas 0 siblings, 0 replies; 21+ messages in thread From: Bjorn Helgaas @ 2016-04-08 18:05 UTC (permalink / raw) To: Thierry Reding Cc: Bjorn Helgaas, Stephen Warren, Alexandre Courbot, linux-tegra, linux-pci, devicetree Hi Thierry, I think there are a couple typos (one in a message and one that actually looks important), and one question below. On Fri, Apr 08, 2016 at 06:13:14PM +0200, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > The current XUSB pad controller bindings are insufficient to describe > PHY devices attached to USB controllers. New bindings have been created > to overcome these restrictions. As a side-effect each root port now is > assigned a set of PHY devices, one for each lane associated with the > root port. This has the benefit of allowing fine-grained control of the > power management for each lane. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > Changes in v4: > - propagate failure from PHY power on > - refactor PHY power off sequence > > Changes in v3: > - cache result of check for new PHY bindings usage (Stephen Warren) > > Changes in v2: > - rework commit message to more accurately describe this change > > drivers/pci/host/pci-tegra.c | 243 ++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 226 insertions(+), 17 deletions(-) > > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c > index 68d1f41b3cbf..d50568bf93c5 100644 > --- a/drivers/pci/host/pci-tegra.c > +++ b/drivers/pci/host/pci-tegra.c > @@ -295,6 +295,7 @@ struct tegra_pcie { > struct reset_control *afi_rst; > struct reset_control *pcie_xrst; > > + bool legacy_phy; > struct phy *phy; > > struct tegra_msi msi; > @@ -311,11 +312,14 @@ struct tegra_pcie { > > struct tegra_pcie_port { > struct tegra_pcie *pcie; > + struct device_node *np; > struct list_head list; > struct resource regs; > void __iomem *base; > unsigned int index; > unsigned int lanes; > + > + struct phy **phys; > }; > > struct tegra_pcie_bus { > @@ -860,6 +864,128 @@ static int tegra_pcie_phy_enable(struct tegra_pcie *pcie) > return 0; > } > > +static int tegra_pcie_phy_disable(struct tegra_pcie *pcie) > +{ > + const struct tegra_pcie_soc_data *soc = pcie->soc_data; > + u32 value; > + > + /* disable TX/RX data */ > + value = pads_readl(pcie, PADS_CTL); > + value &= ~(PADS_CTL_TX_DATA_EN_1L | PADS_CTL_RX_DATA_EN_1L); > + pads_writel(pcie, value, PADS_CTL); > + > + /* override IDDQ */ > + value = pads_readl(pcie, PADS_CTL); > + value |= PADS_CTL_IDDQ_1L; > + pads_writel(pcie, PADS_CTL, value); > + > + /* reset PLL */ > + value = pads_readl(pcie, soc->pads_pll_ctl); > + value &= ~PADS_PLL_CTL_RST_B4SM; > + pads_writel(pcie, value, soc->pads_pll_ctl); > + > + usleep_range(20, 100); > + > + return 0; > +} > + > +static int tegra_pcie_port_phy_power_on(struct tegra_pcie_port *port) > +{ > + struct device *dev = port->pcie->dev; > + unsigned int i; > + int err; > + > + for (i = 0; i < port->lanes; i++) { > + err = phy_power_on(port->phys[i]); > + if (err < 0) { > + dev_err(dev, "failed to power on PHY#%u: %d\n", i, > + err); > + return err; > + } > + } > + > + return 0; > +} > + > +static int tegra_pcie_port_phy_power_off(struct tegra_pcie_port *port) > +{ > + struct device *dev = port->pcie->dev; > + unsigned int i; > + int err; > + > + for (i = 0; i < port->lanes; i++) { > + err = phy_power_off(port->phys[i]); > + if (err < 0) { > + dev_err(dev, "failed to power off PHY#%u: %d\n", i, > + err); > + return err; > + } > + } > + > + return 0; > +} > + > +static int tegra_pcie_phy_power_on(struct tegra_pcie *pcie) > +{ > + struct tegra_pcie_port *port; > + int err; > + > + if (pcie->legacy_phy) { > + if (pcie->phy) > + err = phy_power_on(pcie->phy); > + else > + err = tegra_pcie_phy_enable(pcie); > + > + if (err < 0) > + dev_err(pcie->dev, "failed to power on PHY: %d\n", err); > + > + return err; > + } > + > + list_for_each_entry(port, &pcie->ports, list) { > + err = tegra_pcie_port_phy_power_on(port); > + if (err < 0) { > + dev_err(pcie->dev, > + "failed to power on PCIe port %u PHY: %d\n", > + port->index, err); > + return err; > + } > + } > + > + return 0; > +} > + > +static int tegra_pcie_phy_power_off(struct tegra_pcie *pcie) > +{ > + struct tegra_pcie_port *port; > + int err; > + > + if (pcie->legacy_phy) { > + if (pcie->phy) > + err = phy_power_on(pcie->phy); s/phy_power_on/phy_power_off/ > + else > + err = tegra_pcie_phy_disable(pcie); > + > + if (err < 0) > + dev_err(pcie->dev, "failed to power off PHY: %d\n", > + err); > + > + return err; > + } > + > + list_for_each_entry(port, &pcie->ports, list) { > + err = tegra_pcie_port_phy_power_off(port); > + if (err < 0) { > + dev_err(pcie->dev, > + "failed to power off PCIe port %u PHY: %d\n", > + port->index, err); > + return err; > + } > + } > + > + return 0; > +} > + > static int tegra_pcie_enable_controller(struct tegra_pcie *pcie) > { > const struct tegra_pcie_soc_data *soc = pcie->soc_data; > @@ -899,13 +1025,9 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie) > afi_writel(pcie, value, AFI_FUSE); > } > > - if (!pcie->phy) > - err = tegra_pcie_phy_enable(pcie); > - else > - err = phy_power_on(pcie->phy); > - > + err = tegra_pcie_phy_power_on(pcie); > if (err < 0) { > - dev_err(pcie->dev, "failed to power on PHY: %d\n", err); > + dev_err(pcie->dev, "failed to power off PHY(s): %d\n", err); s/off/on/ > return err; > } > > @@ -942,9 +1064,9 @@ static void tegra_pcie_power_off(struct tegra_pcie *pcie) > > /* TODO: disable and unprepare clocks? */ > > - err = phy_power_off(pcie->phy); > + err = tegra_pcie_phy_power_off(pcie); > if (err < 0) > - dev_warn(pcie->dev, "failed to power off PHY: %d\n", err); > + dev_err(pcie->dev, "failed to power off PHY(s): %d\n", err); > > reset_control_assert(pcie->pcie_xrst); > reset_control_assert(pcie->afi_rst); > @@ -1049,6 +1171,99 @@ static int tegra_pcie_resets_get(struct tegra_pcie *pcie) > return 0; > } > > +static int tegra_pcie_phys_get_legacy(struct tegra_pcie *pcie) > +{ > + int err; > + > + pcie->phy = devm_phy_optional_get(pcie->dev, "pcie"); > + if (IS_ERR(pcie->phy)) { > + err = PTR_ERR(pcie->phy); > + dev_err(pcie->dev, "failed to get PHY: %d\n", err); > + return err; > + } > + > + err = phy_init(pcie->phy); > + if (err < 0) { > + dev_err(pcie->dev, "failed to initialize PHY: %d\n", err); > + return err; > + } > + > + pcie->legacy_phy = true; > + > + return 0; > +} > + > +static struct phy *devm_of_phy_optional_get_index(struct device *dev, > + struct device_node *np, > + const char *consumer, > + unsigned int index) > +{ > + struct phy *phy; > + char *name; > + > + name = kasprintf(GFP_KERNEL, "%s-%u", consumer, index); > + if (!name) > + return ERR_PTR(-ENOMEM); > + > + phy = devm_of_phy_get(dev, np, name); > + kfree(name); > + > + if (IS_ERR(phy) && PTR_ERR(phy) == -ENODEV) > + phy = NULL; > + > + return phy; > +} > + > +static int tegra_pcie_port_get_phys(struct tegra_pcie_port *port) > +{ > + struct device *dev = port->pcie->dev; > + struct phy *phy; > + unsigned int i; > + int err; > + > + port->phys = devm_kcalloc(dev, sizeof(phy), port->lanes, GFP_KERNEL); > + if (!port->phys) > + return -ENOMEM; > + > + for (i = 0; i < port->lanes; i++) { > + phy = devm_of_phy_optional_get_index(dev, port->np, "pcie", i); > + if (IS_ERR(phy)) { > + dev_err(dev, "failed to get PHY#%u: %ld\n", i, > + PTR_ERR(phy)); > + return PTR_ERR(phy); > + } > + > + err = phy_init(phy); > + if (err < 0) { > + dev_err(dev, "failed to initialize PHY#%u: %d\n", i, > + err); > + return err; > + } > + > + port->phys[i] = phy; > + } > + > + return 0; > +} > + > +static int tegra_pcie_phys_get(struct tegra_pcie *pcie) > +{ > + struct tegra_pcie_port *port; > + int err; > + > + if (of_get_property(pcie->dev->of_node, "phys", NULL) != NULL) > + return tegra_pcie_phys_get_legacy(pcie); > + > + list_for_each_entry(port, &pcie->ports, list) { > + err = tegra_pcie_port_get_phys(port); > + if (err < 0) { > + return err; > + } > + } This seems backwards: if I'm reading this right, you first check for the legacy property ("phys") and use it if you find it. If there is no legacy property, you look for the new per-lane PHYs. The usual pattern would be "look for the new stuff, and if you don't find it, fall back to the old stuff." Is there a configuration that could be described either way, e.g., something with only one lane and only one PHY? I'm not sure whether it matters, but if it *could* use the "new, fall back to old" pattern, that would be nice and would keep people from wondering whether it's safe to do it backwards. > + > + return 0; > +} > + > static int tegra_pcie_get_resources(struct tegra_pcie *pcie) > { > struct platform_device *pdev = to_platform_device(pcie->dev); > @@ -1067,16 +1282,9 @@ static int tegra_pcie_get_resources(struct tegra_pcie *pcie) > return err; > } > > - pcie->phy = devm_phy_optional_get(pcie->dev, "pcie"); > - if (IS_ERR(pcie->phy)) { > - err = PTR_ERR(pcie->phy); > - dev_err(&pdev->dev, "failed to get PHY: %d\n", err); > - return err; > - } > - > - err = phy_init(pcie->phy); > + err = tegra_pcie_phys_get(pcie); > if (err < 0) { > - dev_err(&pdev->dev, "failed to initialize PHY: %d\n", err); > + dev_err(&pdev->dev, "failed to get PHYs: %d\n", err); > return err; > } > > @@ -1752,6 +1960,7 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie) > rp->index = index; > rp->lanes = value; > rp->pcie = pcie; > + rp->np = port; > > rp->base = devm_ioremap_resource(pcie->dev, &rp->regs); > if (IS_ERR(rp->base)) > -- > 2.8.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 2/2] PCI: tegra: Support per-lane PHYs 2016-04-08 18:05 ` Bjorn Helgaas (?) @ 2016-04-11 11:11 ` Thierry Reding -1 siblings, 0 replies; 21+ messages in thread From: Thierry Reding @ 2016-04-11 11:11 UTC (permalink / raw) To: Bjorn Helgaas Cc: Bjorn Helgaas, Stephen Warren, Alexandre Courbot, linux-tegra, linux-pci, devicetree [-- Attachment #1: Type: text/plain, Size: 2986 bytes --] On Fri, Apr 08, 2016 at 01:05:28PM -0500, Bjorn Helgaas wrote: > Hi Thierry, > > I think there are a couple typos (one in a message and one that > actually looks important), and one question below. > > On Fri, Apr 08, 2016 at 06:13:14PM +0200, Thierry Reding wrote: [...] > > +static int tegra_pcie_phy_power_off(struct tegra_pcie *pcie) > > +{ > > + struct tegra_pcie_port *port; > > + int err; > > + > > + if (pcie->legacy_phy) { > > + if (pcie->phy) > > + err = phy_power_on(pcie->phy); > > s/phy_power_on/phy_power_off/ Good catch. Fixed. > > @@ -899,13 +1025,9 @@ static int tegra_pcie_enable_controller(struct tegra_pcie *pcie) > > afi_writel(pcie, value, AFI_FUSE); > > } > > > > - if (!pcie->phy) > > - err = tegra_pcie_phy_enable(pcie); > > - else > > - err = phy_power_on(pcie->phy); > > - > > + err = tegra_pcie_phy_power_on(pcie); > > if (err < 0) { > > - dev_err(pcie->dev, "failed to power on PHY: %d\n", err); > > + dev_err(pcie->dev, "failed to power off PHY(s): %d\n", err); > > s/off/on/ Fixed. > > +static int tegra_pcie_phys_get(struct tegra_pcie *pcie) > > +{ > > + struct tegra_pcie_port *port; > > + int err; > > + > > + if (of_get_property(pcie->dev->of_node, "phys", NULL) != NULL) > > + return tegra_pcie_phys_get_legacy(pcie); > > + > > + list_for_each_entry(port, &pcie->ports, list) { > > + err = tegra_pcie_port_get_phys(port); > > + if (err < 0) { > > + return err; > > + } > > + } > > This seems backwards: if I'm reading this right, you first check for > the legacy property ("phys") and use it if you find it. If there is > no legacy property, you look for the new per-lane PHYs. > > The usual pattern would be "look for the new stuff, and if you don't > find it, fall back to the old stuff." Is there a configuration that > could be described either way, e.g., something with only one lane and > only one PHY? > > I'm not sure whether it matters, but if it *could* use the "new, fall > back to old" pattern, that would be nice and would keep people from > wondering whether it's safe to do it backwards. The reason why I wrote it this way is to special case the legacy code. The alternative would be to do: if (of_get_property(...) == NULL) { list_for_each_entry(port, &pcie->ports, list) { err = tegra_pcie_port_get_phys(port); if (err < 0) return err; } } return tegra_pcie_phys_get_legacy(pcie); Which is better in the way you describe (fall back to legacy if new binding is not found). But like I said, it makes, from the code flow, the new binding the exception, which looks odd to me as well. Perhaps this could be somewhat mitigated by wrapping the new code into a separate function: if (of_get_property(...) == NULL) return tegra_pcie_phys_get_per_lane(pcie); return tegra_pcie_phys_get_legacy(pcie); I don't feel very strongly in either direction. Do you have a preference? Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 2/2] PCI: tegra: Support per-lane PHYs 2016-04-08 16:13 ` Thierry Reding (?) (?) @ 2016-04-11 17:41 ` Stephen Warren -1 siblings, 0 replies; 21+ messages in thread From: Stephen Warren @ 2016-04-11 17:41 UTC (permalink / raw) To: Thierry Reding Cc: Bjorn Helgaas, Alexandre Courbot, linux-tegra, linux-pci, devicetree On 04/08/2016 10:13 AM, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > The current XUSB pad controller bindings are insufficient to describe > PHY devices attached to USB controllers. New bindings have been created > to overcome these restrictions. As a side-effect each root port now is > assigned a set of PHY devices, one for each lane associated with the > root port. This has the benefit of allowing fine-grained control of the > power management for each lane. Same here. http://www.spinics.net/lists/devicetree/msg118644.html ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs 2016-04-08 16:13 [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs Thierry Reding @ 2016-04-08 16:48 ` Bjorn Helgaas 0 siblings, 0 replies; 21+ messages in thread From: Bjorn Helgaas @ 2016-04-08 16:48 UTC (permalink / raw) To: Thierry Reding Cc: Bjorn Helgaas, Stephen Warren, Alexandre Courbot, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-pci-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA Hi Thierry, On Fri, Apr 08, 2016 at 06:13:13PM +0200, Thierry Reding wrote: > From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> > > Changes to the pad controller device tree binding Is there a commit we can reference for this pad controller binding change? If so, I can fold it in. Is there a benefit to keeping this binding patch separate from the corresponding driver patch? It seems like it'd nice to have a connection between them. (Sorry, I'm sure I've asked this before, but I can't remember the answer.) Trivial typos below that I've already fixed, but you can fold them in if there's any reason to repost this. > have required that > each lane be associated with a separate PHY. Update the PCI host bridge > device tree binding to allow each root port to define the list of PHYs > required to drive the lanes associated with it. > > Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> > --- > Changes in v4: > - add additional lanes subnode when dereferencing PHYs from the XUSB pad > controller to reflect changes in its binding > > .../devicetree/bindings/pci/nvidia,tegra20-pcie.txt | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > index 75321ae23c08..f5364084b494 100644 > --- a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > @@ -60,11 +60,14 @@ Required properties: > - afi > - pcie_x > > -Required properties on Tegra124 and later: > +Required properties on Tegra124 and later (deprecated): > - phys: Must contain an entry for each entry in phy-names. > - phy-names: Must include the following entries: > - pcie > > +These properties are deprecated in favour of per-lane PHYs define in each of s/define/defined/ > +the root ports (see below). > + > Power supplies for Tegra20: > - avdd-pex-supply: Power supply for analog PCIe logic. Must supply 1.05 V. > - vdd-pex-supply: Power supply for digital PCIe I/O. Must supply 1.05 V. > @@ -122,6 +125,13 @@ Required properties: > - Root port 0 uses 4 lanes, root port 1 is unused. > - Both root ports use 2 lanes. > > +Required properties for Tegra124 and later: > +- phys: Must contain an phandle to a PHY for each entry in phy-names. s/an phandle/a phandle/ > +- phy-names: Must include an entry for each active lane. Note that the number > + of entries does not have to (though usually will) be equal to the specified > + number of lanes in the nvidia,num-lanes property. Entries are of the form > + "pcie-N": where N ranges from 0 to the value specified in nvidia,num-lanes. > + > Example: > > SoC DTSI: > @@ -169,6 +179,9 @@ SoC DTSI: > ranges; > > nvidia,num-lanes = <2>; > + > + phys = <&{/padctl@0,7009f000/pads/pcie/lanes/pcie-4}>; > + phy-names = "pcie-0"; > }; > > pci@2,0 { > @@ -183,6 +196,9 @@ SoC DTSI: > ranges; > > nvidia,num-lanes = <2>; > + > + phys = <&{/padctl@0,7009f000/pads/pcie/lanes/pcie-2}>; > + phy-names = "pcie-0"; > }; > }; > > -- > 2.8.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs @ 2016-04-08 16:48 ` Bjorn Helgaas 0 siblings, 0 replies; 21+ messages in thread From: Bjorn Helgaas @ 2016-04-08 16:48 UTC (permalink / raw) To: Thierry Reding Cc: Bjorn Helgaas, Stephen Warren, Alexandre Courbot, linux-tegra, linux-pci, devicetree Hi Thierry, On Fri, Apr 08, 2016 at 06:13:13PM +0200, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > Changes to the pad controller device tree binding Is there a commit we can reference for this pad controller binding change? If so, I can fold it in. Is there a benefit to keeping this binding patch separate from the corresponding driver patch? It seems like it'd nice to have a connection between them. (Sorry, I'm sure I've asked this before, but I can't remember the answer.) Trivial typos below that I've already fixed, but you can fold them in if there's any reason to repost this. > have required that > each lane be associated with a separate PHY. Update the PCI host bridge > device tree binding to allow each root port to define the list of PHYs > required to drive the lanes associated with it. > > Acked-by: Rob Herring <robh@kernel.org> > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > Changes in v4: > - add additional lanes subnode when dereferencing PHYs from the XUSB pad > controller to reflect changes in its binding > > .../devicetree/bindings/pci/nvidia,tegra20-pcie.txt | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > index 75321ae23c08..f5364084b494 100644 > --- a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > @@ -60,11 +60,14 @@ Required properties: > - afi > - pcie_x > > -Required properties on Tegra124 and later: > +Required properties on Tegra124 and later (deprecated): > - phys: Must contain an entry for each entry in phy-names. > - phy-names: Must include the following entries: > - pcie > > +These properties are deprecated in favour of per-lane PHYs define in each of s/define/defined/ > +the root ports (see below). > + > Power supplies for Tegra20: > - avdd-pex-supply: Power supply for analog PCIe logic. Must supply 1.05 V. > - vdd-pex-supply: Power supply for digital PCIe I/O. Must supply 1.05 V. > @@ -122,6 +125,13 @@ Required properties: > - Root port 0 uses 4 lanes, root port 1 is unused. > - Both root ports use 2 lanes. > > +Required properties for Tegra124 and later: > +- phys: Must contain an phandle to a PHY for each entry in phy-names. s/an phandle/a phandle/ > +- phy-names: Must include an entry for each active lane. Note that the number > + of entries does not have to (though usually will) be equal to the specified > + number of lanes in the nvidia,num-lanes property. Entries are of the form > + "pcie-N": where N ranges from 0 to the value specified in nvidia,num-lanes. > + > Example: > > SoC DTSI: > @@ -169,6 +179,9 @@ SoC DTSI: > ranges; > > nvidia,num-lanes = <2>; > + > + phys = <&{/padctl@0,7009f000/pads/pcie/lanes/pcie-4}>; > + phy-names = "pcie-0"; > }; > > pci@2,0 { > @@ -183,6 +196,9 @@ SoC DTSI: > ranges; > > nvidia,num-lanes = <2>; > + > + phys = <&{/padctl@0,7009f000/pads/pcie/lanes/pcie-2}>; > + phy-names = "pcie-0"; > }; > }; > > -- > 2.8.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs 2016-04-08 16:48 ` Bjorn Helgaas @ 2016-04-11 10:23 ` Thierry Reding -1 siblings, 0 replies; 21+ messages in thread From: Thierry Reding @ 2016-04-11 10:23 UTC (permalink / raw) To: Bjorn Helgaas Cc: Bjorn Helgaas, Stephen Warren, Alexandre Courbot, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-pci-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1: Type: text/plain, Size: 1529 bytes --] On Fri, Apr 08, 2016 at 11:48:14AM -0500, Bjorn Helgaas wrote: > Hi Thierry, > > On Fri, Apr 08, 2016 at 06:13:13PM +0200, Thierry Reding wrote: > > From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> > > > > Changes to the pad controller device tree binding > > Is there a commit we can reference for this pad controller binding > change? If so, I can fold it in. It hasn't been merged yet. Technically this patch is a prerequisite for getting the new binding merged. And there's a specific order that needs to be observed for this not to regress (albeit only temporarily). The correct sequence for merging this would be: 1) XUSB pad controller binding & driver updates 2) PCI driver updates 3) DT changes that implement the new binding Because of the above I'd prefer to merge everything through the Tegra tree. If that's okay with you, I can provide stable branches once its all in place for you to merge back into the PCI tree. > Is there a benefit to keeping this binding patch separate from the > corresponding driver patch? It seems like it'd nice to have a > connection between them. (Sorry, I'm sure I've asked this before, > but I can't remember the answer.) I've seen both approaches applied. I think separate patches is (or was?) preferred because eventually the DT bindings were supposed to get moved to a separate repository. Having separate patches would make the transition easier by making it possible to cherry-pick between trees. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs @ 2016-04-11 10:23 ` Thierry Reding 0 siblings, 0 replies; 21+ messages in thread From: Thierry Reding @ 2016-04-11 10:23 UTC (permalink / raw) To: Bjorn Helgaas Cc: Bjorn Helgaas, Stephen Warren, Alexandre Courbot, linux-tegra, linux-pci, devicetree [-- Attachment #1: Type: text/plain, Size: 1500 bytes --] On Fri, Apr 08, 2016 at 11:48:14AM -0500, Bjorn Helgaas wrote: > Hi Thierry, > > On Fri, Apr 08, 2016 at 06:13:13PM +0200, Thierry Reding wrote: > > From: Thierry Reding <treding@nvidia.com> > > > > Changes to the pad controller device tree binding > > Is there a commit we can reference for this pad controller binding > change? If so, I can fold it in. It hasn't been merged yet. Technically this patch is a prerequisite for getting the new binding merged. And there's a specific order that needs to be observed for this not to regress (albeit only temporarily). The correct sequence for merging this would be: 1) XUSB pad controller binding & driver updates 2) PCI driver updates 3) DT changes that implement the new binding Because of the above I'd prefer to merge everything through the Tegra tree. If that's okay with you, I can provide stable branches once its all in place for you to merge back into the PCI tree. > Is there a benefit to keeping this binding patch separate from the > corresponding driver patch? It seems like it'd nice to have a > connection between them. (Sorry, I'm sure I've asked this before, > but I can't remember the answer.) I've seen both approaches applied. I think separate patches is (or was?) preferred because eventually the DT bindings were supposed to get moved to a separate repository. Having separate patches would make the transition easier by making it possible to cherry-pick between trees. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs 2016-04-08 16:13 [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs Thierry Reding @ 2016-04-08 19:11 ` Bjorn Helgaas 0 siblings, 0 replies; 21+ messages in thread From: Bjorn Helgaas @ 2016-04-08 19:11 UTC (permalink / raw) To: Thierry Reding Cc: Bjorn Helgaas, Stephen Warren, Alexandre Courbot, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-pci-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA Hi Thierry, I have a couple more questions, probably just because I'm DT-illiterate. On Fri, Apr 08, 2016 at 06:13:13PM +0200, Thierry Reding wrote: > From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> > > Changes to the pad controller device tree binding have required that > each lane be associated with a separate PHY. Update the PCI host bridge > device tree binding to allow each root port to define the list of PHYs > required to drive the lanes associated with it. > > Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> > --- > Changes in v4: > - add additional lanes subnode when dereferencing PHYs from the XUSB pad > controller to reflect changes in its binding > > .../devicetree/bindings/pci/nvidia,tegra20-pcie.txt | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > index 75321ae23c08..f5364084b494 100644 > --- a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > @@ -60,11 +60,14 @@ Required properties: > - afi > - pcie_x > > -Required properties on Tegra124 and later: > +Required properties on Tegra124 and later (deprecated): > - phys: Must contain an entry for each entry in phy-names. > - phy-names: Must include the following entries: > - pcie > > +These properties are deprecated in favour of per-lane PHYs define in each of > +the root ports (see below). > + > Power supplies for Tegra20: > - avdd-pex-supply: Power supply for analog PCIe logic. Must supply 1.05 V. > - vdd-pex-supply: Power supply for digital PCIe I/O. Must supply 1.05 V. > @@ -122,6 +125,13 @@ Required properties: > - Root port 0 uses 4 lanes, root port 1 is unused. > - Both root ports use 2 lanes. > > +Required properties for Tegra124 and later: I had a little trouble disambiguating this from the "Required properties on Tegra124 and later (deprecated)" line above. It might help if they said: Required PCIe controller properties on Tegra124 and later (deprecated): Required PCIe Root Port properties for Tegra124 and later: I'm not sure how to interpret the "deprecated" part. Assume I'm writing a DTS. What am I supposed to include? - "phys" and "phy-names" under the PCIe controller *and* "phys" and "phy-names" under the Root Port? - "phys" and "phy-names" under the PCIe controller only if I don't supply "phys" and "phy-names" under the Root Port? My guess is that a board with more than one PHY for PCIe should omit "phys" and "phy-names" under the PCIe controller and include them under each Root Port. And a board with only one PHY could conceivably supply these properties either under the controller or the Root Port or both. > +- phys: Must contain an phandle to a PHY for each entry in phy-names. > +- phy-names: Must include an entry for each active lane. Note that the number > + of entries does not have to (though usually will) be equal to the specified > + number of lanes in the nvidia,num-lanes property. Entries are of the form > + "pcie-N": where N ranges from 0 to the value specified in nvidia,num-lanes. > + > Example: > > SoC DTSI: > @@ -169,6 +179,9 @@ SoC DTSI: > ranges; > > nvidia,num-lanes = <2>; > + > + phys = <&{/padctl@0,7009f000/pads/pcie/lanes/pcie-4}>; > + phy-names = "pcie-0"; I'm also a little confused here because it looks like this root port supports two lanes, but there's only one entry in phy-names. I thought you needed one entry for each lane. > }; > > pci@2,0 { > @@ -183,6 +196,9 @@ SoC DTSI: > ranges; > > nvidia,num-lanes = <2>; > + > + phys = <&{/padctl@0,7009f000/pads/pcie/lanes/pcie-2}>; > + phy-names = "pcie-0"; > }; > }; > > -- > 2.8.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs @ 2016-04-08 19:11 ` Bjorn Helgaas 0 siblings, 0 replies; 21+ messages in thread From: Bjorn Helgaas @ 2016-04-08 19:11 UTC (permalink / raw) To: Thierry Reding Cc: Bjorn Helgaas, Stephen Warren, Alexandre Courbot, linux-tegra, linux-pci, devicetree Hi Thierry, I have a couple more questions, probably just because I'm DT-illiterate. On Fri, Apr 08, 2016 at 06:13:13PM +0200, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > Changes to the pad controller device tree binding have required that > each lane be associated with a separate PHY. Update the PCI host bridge > device tree binding to allow each root port to define the list of PHYs > required to drive the lanes associated with it. > > Acked-by: Rob Herring <robh@kernel.org> > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > Changes in v4: > - add additional lanes subnode when dereferencing PHYs from the XUSB pad > controller to reflect changes in its binding > > .../devicetree/bindings/pci/nvidia,tegra20-pcie.txt | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > index 75321ae23c08..f5364084b494 100644 > --- a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > @@ -60,11 +60,14 @@ Required properties: > - afi > - pcie_x > > -Required properties on Tegra124 and later: > +Required properties on Tegra124 and later (deprecated): > - phys: Must contain an entry for each entry in phy-names. > - phy-names: Must include the following entries: > - pcie > > +These properties are deprecated in favour of per-lane PHYs define in each of > +the root ports (see below). > + > Power supplies for Tegra20: > - avdd-pex-supply: Power supply for analog PCIe logic. Must supply 1.05 V. > - vdd-pex-supply: Power supply for digital PCIe I/O. Must supply 1.05 V. > @@ -122,6 +125,13 @@ Required properties: > - Root port 0 uses 4 lanes, root port 1 is unused. > - Both root ports use 2 lanes. > > +Required properties for Tegra124 and later: I had a little trouble disambiguating this from the "Required properties on Tegra124 and later (deprecated)" line above. It might help if they said: Required PCIe controller properties on Tegra124 and later (deprecated): Required PCIe Root Port properties for Tegra124 and later: I'm not sure how to interpret the "deprecated" part. Assume I'm writing a DTS. What am I supposed to include? - "phys" and "phy-names" under the PCIe controller *and* "phys" and "phy-names" under the Root Port? - "phys" and "phy-names" under the PCIe controller only if I don't supply "phys" and "phy-names" under the Root Port? My guess is that a board with more than one PHY for PCIe should omit "phys" and "phy-names" under the PCIe controller and include them under each Root Port. And a board with only one PHY could conceivably supply these properties either under the controller or the Root Port or both. > +- phys: Must contain an phandle to a PHY for each entry in phy-names. > +- phy-names: Must include an entry for each active lane. Note that the number > + of entries does not have to (though usually will) be equal to the specified > + number of lanes in the nvidia,num-lanes property. Entries are of the form > + "pcie-N": where N ranges from 0 to the value specified in nvidia,num-lanes. > + > Example: > > SoC DTSI: > @@ -169,6 +179,9 @@ SoC DTSI: > ranges; > > nvidia,num-lanes = <2>; > + > + phys = <&{/padctl@0,7009f000/pads/pcie/lanes/pcie-4}>; > + phy-names = "pcie-0"; I'm also a little confused here because it looks like this root port supports two lanes, but there's only one entry in phy-names. I thought you needed one entry for each lane. > }; > > pci@2,0 { > @@ -183,6 +196,9 @@ SoC DTSI: > ranges; > > nvidia,num-lanes = <2>; > + > + phys = <&{/padctl@0,7009f000/pads/pcie/lanes/pcie-2}>; > + phy-names = "pcie-0"; > }; > }; > > -- > 2.8.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs 2016-04-08 19:11 ` Bjorn Helgaas @ 2016-04-11 11:26 ` Thierry Reding -1 siblings, 0 replies; 21+ messages in thread From: Thierry Reding @ 2016-04-11 11:26 UTC (permalink / raw) To: Bjorn Helgaas Cc: Bjorn Helgaas, Stephen Warren, Alexandre Courbot, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-pci-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1: Type: text/plain, Size: 6043 bytes --] On Fri, Apr 08, 2016 at 02:11:42PM -0500, Bjorn Helgaas wrote: > Hi Thierry, > > I have a couple more questions, probably just because I'm > DT-illiterate. > > On Fri, Apr 08, 2016 at 06:13:13PM +0200, Thierry Reding wrote: > > From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> > > > > Changes to the pad controller device tree binding have required that > > each lane be associated with a separate PHY. Update the PCI host bridge > > device tree binding to allow each root port to define the list of PHYs > > required to drive the lanes associated with it. > > > > Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > > Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> > > --- > > Changes in v4: > > - add additional lanes subnode when dereferencing PHYs from the XUSB pad > > controller to reflect changes in its binding > > > > .../devicetree/bindings/pci/nvidia,tegra20-pcie.txt | 18 +++++++++++++++++- > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > > index 75321ae23c08..f5364084b494 100644 > > --- a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > > +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > > @@ -60,11 +60,14 @@ Required properties: > > - afi > > - pcie_x > > > > -Required properties on Tegra124 and later: > > +Required properties on Tegra124 and later (deprecated): > > - phys: Must contain an entry for each entry in phy-names. > > - phy-names: Must include the following entries: > > - pcie > > > > +These properties are deprecated in favour of per-lane PHYs define in each of > > +the root ports (see below). > > + > > Power supplies for Tegra20: > > - avdd-pex-supply: Power supply for analog PCIe logic. Must supply 1.05 V. > > - vdd-pex-supply: Power supply for digital PCIe I/O. Must supply 1.05 V. > > @@ -122,6 +125,13 @@ Required properties: > > - Root port 0 uses 4 lanes, root port 1 is unused. > > - Both root ports use 2 lanes. > > > > +Required properties for Tegra124 and later: > > I had a little trouble disambiguating this from the "Required > properties on Tegra124 and later (deprecated)" line above. It might > help if they said: > > Required PCIe controller properties on Tegra124 and later (deprecated): > Required PCIe Root Port properties for Tegra124 and later: That's kind of implied by the structure of the binding. The first is in the section dedicated to the description of the host bridge controller, whereas the second in in the section for root port properties. Granted, the format for device tree bindings isn't very rich, so this isn't very easy to spot. Perhaps something like this could help: NVIDIA Tegra PCIe controller The top-level device tree node describes the host bridge controller and properties listed therein apply to the controller as a whole. Required properties: ... Root Ports: ----------- Root ports are defined as subnodes of the PCIe controller node. Required properties: ... Would that help clarify? > I'm not sure how to interpret the "deprecated" part. Assume I'm > writing a DTS. What am I supposed to include? > > - "phys" and "phy-names" under the PCIe controller *and* > "phys" and "phy-names" under the Root Port? > > - "phys" and "phy-names" under the PCIe controller only if I don't > supply "phys" and "phy-names" under the Root Port? > > My guess is that a board with more than one PHY for PCIe should omit > "phys" and "phy-names" under the PCIe controller and include them > under each Root Port. And a board with only one PHY could conceivably > supply these properties either under the controller or the Root Port > or both. Whatever is marked as deprecated should not be used in new bindings at all. That is, new board files should move to per-lane PHYs, no matter how many lanes they enable. The XUSB pad controller that provides these PHYs can be driven by two different drivers: a pinctrl driver that exposes a single PHY for all PCI lanes (it uses per-lane nvidia,iddq properties to enable or disable each of the lanes) or a PHY driver that exposes one PHY per lane. Each PHY can separately be powered on. Perhaps a better approach would be to simply remove the deprecated properties to avoid any possible confusion. It does have the drawback that old device trees won't have a documentation counterpart. > > +- phys: Must contain an phandle to a PHY for each entry in phy-names. > > +- phy-names: Must include an entry for each active lane. Note that the number > > + of entries does not have to (though usually will) be equal to the specified > > + number of lanes in the nvidia,num-lanes property. Entries are of the form > > + "pcie-N": where N ranges from 0 to the value specified in nvidia,num-lanes. > > + > > Example: > > > > SoC DTSI: > > @@ -169,6 +179,9 @@ SoC DTSI: > > ranges; > > > > nvidia,num-lanes = <2>; > > + > > + phys = <&{/padctl@0,7009f000/pads/pcie/lanes/pcie-4}>; > > + phy-names = "pcie-0"; > > I'm also a little confused here because it looks like this root port > supports two lanes, but there's only one entry in phy-names. I > thought you needed one entry for each lane. Not necessarily. I'm not sure if the wording in the description makes it clear, but there are a limited number of configurations that the root ports support. However the ports may not want (or need) to drive all of the lanes that they support. Consider for example one possible configuration on Tegra124, which is to have one root port operate in x1 mode and the other in x2 mode. The x2 port may have a fixed card wired to it (onboard) that is x1. Enabling the second lane would be wasting power because it is never used. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs @ 2016-04-11 11:26 ` Thierry Reding 0 siblings, 0 replies; 21+ messages in thread From: Thierry Reding @ 2016-04-11 11:26 UTC (permalink / raw) To: Bjorn Helgaas Cc: Bjorn Helgaas, Stephen Warren, Alexandre Courbot, linux-tegra, linux-pci, devicetree [-- Attachment #1: Type: text/plain, Size: 5956 bytes --] On Fri, Apr 08, 2016 at 02:11:42PM -0500, Bjorn Helgaas wrote: > Hi Thierry, > > I have a couple more questions, probably just because I'm > DT-illiterate. > > On Fri, Apr 08, 2016 at 06:13:13PM +0200, Thierry Reding wrote: > > From: Thierry Reding <treding@nvidia.com> > > > > Changes to the pad controller device tree binding have required that > > each lane be associated with a separate PHY. Update the PCI host bridge > > device tree binding to allow each root port to define the list of PHYs > > required to drive the lanes associated with it. > > > > Acked-by: Rob Herring <robh@kernel.org> > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > --- > > Changes in v4: > > - add additional lanes subnode when dereferencing PHYs from the XUSB pad > > controller to reflect changes in its binding > > > > .../devicetree/bindings/pci/nvidia,tegra20-pcie.txt | 18 +++++++++++++++++- > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > > index 75321ae23c08..f5364084b494 100644 > > --- a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > > +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt > > @@ -60,11 +60,14 @@ Required properties: > > - afi > > - pcie_x > > > > -Required properties on Tegra124 and later: > > +Required properties on Tegra124 and later (deprecated): > > - phys: Must contain an entry for each entry in phy-names. > > - phy-names: Must include the following entries: > > - pcie > > > > +These properties are deprecated in favour of per-lane PHYs define in each of > > +the root ports (see below). > > + > > Power supplies for Tegra20: > > - avdd-pex-supply: Power supply for analog PCIe logic. Must supply 1.05 V. > > - vdd-pex-supply: Power supply for digital PCIe I/O. Must supply 1.05 V. > > @@ -122,6 +125,13 @@ Required properties: > > - Root port 0 uses 4 lanes, root port 1 is unused. > > - Both root ports use 2 lanes. > > > > +Required properties for Tegra124 and later: > > I had a little trouble disambiguating this from the "Required > properties on Tegra124 and later (deprecated)" line above. It might > help if they said: > > Required PCIe controller properties on Tegra124 and later (deprecated): > Required PCIe Root Port properties for Tegra124 and later: That's kind of implied by the structure of the binding. The first is in the section dedicated to the description of the host bridge controller, whereas the second in in the section for root port properties. Granted, the format for device tree bindings isn't very rich, so this isn't very easy to spot. Perhaps something like this could help: NVIDIA Tegra PCIe controller The top-level device tree node describes the host bridge controller and properties listed therein apply to the controller as a whole. Required properties: ... Root Ports: ----------- Root ports are defined as subnodes of the PCIe controller node. Required properties: ... Would that help clarify? > I'm not sure how to interpret the "deprecated" part. Assume I'm > writing a DTS. What am I supposed to include? > > - "phys" and "phy-names" under the PCIe controller *and* > "phys" and "phy-names" under the Root Port? > > - "phys" and "phy-names" under the PCIe controller only if I don't > supply "phys" and "phy-names" under the Root Port? > > My guess is that a board with more than one PHY for PCIe should omit > "phys" and "phy-names" under the PCIe controller and include them > under each Root Port. And a board with only one PHY could conceivably > supply these properties either under the controller or the Root Port > or both. Whatever is marked as deprecated should not be used in new bindings at all. That is, new board files should move to per-lane PHYs, no matter how many lanes they enable. The XUSB pad controller that provides these PHYs can be driven by two different drivers: a pinctrl driver that exposes a single PHY for all PCI lanes (it uses per-lane nvidia,iddq properties to enable or disable each of the lanes) or a PHY driver that exposes one PHY per lane. Each PHY can separately be powered on. Perhaps a better approach would be to simply remove the deprecated properties to avoid any possible confusion. It does have the drawback that old device trees won't have a documentation counterpart. > > +- phys: Must contain an phandle to a PHY for each entry in phy-names. > > +- phy-names: Must include an entry for each active lane. Note that the number > > + of entries does not have to (though usually will) be equal to the specified > > + number of lanes in the nvidia,num-lanes property. Entries are of the form > > + "pcie-N": where N ranges from 0 to the value specified in nvidia,num-lanes. > > + > > Example: > > > > SoC DTSI: > > @@ -169,6 +179,9 @@ SoC DTSI: > > ranges; > > > > nvidia,num-lanes = <2>; > > + > > + phys = <&{/padctl@0,7009f000/pads/pcie/lanes/pcie-4}>; > > + phy-names = "pcie-0"; > > I'm also a little confused here because it looks like this root port > supports two lanes, but there's only one entry in phy-names. I > thought you needed one entry for each lane. Not necessarily. I'm not sure if the wording in the description makes it clear, but there are a limited number of configurations that the root ports support. However the ports may not want (or need) to drive all of the lanes that they support. Consider for example one possible configuration on Tegra124, which is to have one root port operate in x1 mode and the other in x2 mode. The x2 port may have a fixed card wired to it (onboard) that is x1. Enabling the second lane would be wasting power because it is never used. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <20160411112654.GC17743-EkSeR96xj6Pcmrwk2tT4+A@public.gmane.org>]
* Re: [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs 2016-04-11 11:26 ` Thierry Reding @ 2016-04-11 16:55 ` Stephen Warren -1 siblings, 0 replies; 21+ messages in thread From: Stephen Warren @ 2016-04-11 16:55 UTC (permalink / raw) To: Thierry Reding, Bjorn Helgaas Cc: Bjorn Helgaas, Alexandre Courbot, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-pci-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA On 04/11/2016 05:26 AM, Thierry Reding wrote: > On Fri, Apr 08, 2016 at 02:11:42PM -0500, Bjorn Helgaas wrote: >> Hi Thierry, >> >> I have a couple more questions, probably just because I'm >> DT-illiterate. >> >> On Fri, Apr 08, 2016 at 06:13:13PM +0200, Thierry Reding wrote: >>> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> >>> >>> Changes to the pad controller device tree binding have required that >>> each lane be associated with a separate PHY. Update the PCI host bridge >>> device tree binding to allow each root port to define the list of PHYs >>> required to drive the lanes associated with it. >>> diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt >>> -Required properties on Tegra124 and later: >>> +Required properties on Tegra124 and later (deprecated): >>> - phys: Must contain an entry for each entry in phy-names. >>> - phy-names: Must include the following entries: >>> - pcie >>> >>> +These properties are deprecated in favour of per-lane PHYs define in each of >>> +the root ports (see below). >>> + >>> Power supplies for Tegra20: >>> - avdd-pex-supply: Power supply for analog PCIe logic. Must supply 1.05 V. >>> - vdd-pex-supply: Power supply for digital PCIe I/O. Must supply 1.05 V. >>> @@ -122,6 +125,13 @@ Required properties: >>> - Root port 0 uses 4 lanes, root port 1 is unused. >>> - Both root ports use 2 lanes. >>> >>> +Required properties for Tegra124 and later: >> >> I had a little trouble disambiguating this from the "Required >> properties on Tegra124 and later (deprecated)" line above. It might >> help if they said: >> >> Required PCIe controller properties on Tegra124 and later (deprecated): >> Required PCIe Root Port properties for Tegra124 and later: ... >> I'm not sure how to interpret the "deprecated" part. Assume I'm >> writing a DTS. What am I supposed to include? >> >> - "phys" and "phy-names" under the PCIe controller *and* >> "phys" and "phy-names" under the Root Port? >> >> - "phys" and "phy-names" under the PCIe controller only if I don't >> supply "phys" and "phy-names" under the Root Port? >> >> My guess is that a board with more than one PHY for PCIe should omit >> "phys" and "phy-names" under the PCIe controller and include them >> under each Root Port. And a board with only one PHY could conceivably >> supply these properties either under the controller or the Root Port >> or both. > > Whatever is marked as deprecated should not be used in new bindings at > all. That is, new board files should move to per-lane PHYs, no matter > how many lanes they enable. > > The XUSB pad controller that provides these PHYs can be driven by two > different drivers: a pinctrl driver that exposes a single PHY for all > PCI lanes (it uses per-lane nvidia,iddq properties to enable or disable > each of the lanes) or a PHY driver that exposes one PHY per lane. Each > PHY can separately be powered on. > > Perhaps a better approach would be to simply remove the deprecated > properties to avoid any possible confusion. It does have the drawback > that old device trees won't have a documentation counterpart. I recall requests during past DT binding reviews that deprecated properties still be documented. That allows legacy bindings and driver support for them to be understood, since they're still documented. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs @ 2016-04-11 16:55 ` Stephen Warren 0 siblings, 0 replies; 21+ messages in thread From: Stephen Warren @ 2016-04-11 16:55 UTC (permalink / raw) To: Thierry Reding, Bjorn Helgaas Cc: Bjorn Helgaas, Alexandre Courbot, linux-tegra, linux-pci, devicetree On 04/11/2016 05:26 AM, Thierry Reding wrote: > On Fri, Apr 08, 2016 at 02:11:42PM -0500, Bjorn Helgaas wrote: >> Hi Thierry, >> >> I have a couple more questions, probably just because I'm >> DT-illiterate. >> >> On Fri, Apr 08, 2016 at 06:13:13PM +0200, Thierry Reding wrote: >>> From: Thierry Reding <treding@nvidia.com> >>> >>> Changes to the pad controller device tree binding have required that >>> each lane be associated with a separate PHY. Update the PCI host bridge >>> device tree binding to allow each root port to define the list of PHYs >>> required to drive the lanes associated with it. >>> diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt >>> -Required properties on Tegra124 and later: >>> +Required properties on Tegra124 and later (deprecated): >>> - phys: Must contain an entry for each entry in phy-names. >>> - phy-names: Must include the following entries: >>> - pcie >>> >>> +These properties are deprecated in favour of per-lane PHYs define in each of >>> +the root ports (see below). >>> + >>> Power supplies for Tegra20: >>> - avdd-pex-supply: Power supply for analog PCIe logic. Must supply 1.05 V. >>> - vdd-pex-supply: Power supply for digital PCIe I/O. Must supply 1.05 V. >>> @@ -122,6 +125,13 @@ Required properties: >>> - Root port 0 uses 4 lanes, root port 1 is unused. >>> - Both root ports use 2 lanes. >>> >>> +Required properties for Tegra124 and later: >> >> I had a little trouble disambiguating this from the "Required >> properties on Tegra124 and later (deprecated)" line above. It might >> help if they said: >> >> Required PCIe controller properties on Tegra124 and later (deprecated): >> Required PCIe Root Port properties for Tegra124 and later: ... >> I'm not sure how to interpret the "deprecated" part. Assume I'm >> writing a DTS. What am I supposed to include? >> >> - "phys" and "phy-names" under the PCIe controller *and* >> "phys" and "phy-names" under the Root Port? >> >> - "phys" and "phy-names" under the PCIe controller only if I don't >> supply "phys" and "phy-names" under the Root Port? >> >> My guess is that a board with more than one PHY for PCIe should omit >> "phys" and "phy-names" under the PCIe controller and include them >> under each Root Port. And a board with only one PHY could conceivably >> supply these properties either under the controller or the Root Port >> or both. > > Whatever is marked as deprecated should not be used in new bindings at > all. That is, new board files should move to per-lane PHYs, no matter > how many lanes they enable. > > The XUSB pad controller that provides these PHYs can be driven by two > different drivers: a pinctrl driver that exposes a single PHY for all > PCI lanes (it uses per-lane nvidia,iddq properties to enable or disable > each of the lanes) or a PHY driver that exposes one PHY per lane. Each > PHY can separately be powered on. > > Perhaps a better approach would be to simply remove the deprecated > properties to avoid any possible confusion. It does have the drawback > that old device trees won't have a documentation counterpart. I recall requests during past DT binding reviews that deprecated properties still be documented. That allows legacy bindings and driver support for them to be understood, since they're still documented. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs 2016-04-08 16:13 [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs Thierry Reding @ 2016-04-11 17:38 ` Stephen Warren 0 siblings, 0 replies; 21+ messages in thread From: Stephen Warren @ 2016-04-11 17:38 UTC (permalink / raw) To: Thierry Reding Cc: Bjorn Helgaas, Alexandre Courbot, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-pci-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA On 04/08/2016 10:13 AM, Thierry Reding wrote: > From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> > > Changes to the pad controller device tree binding have required that > each lane be associated with a separate PHY. Update the PCI host bridge > device tree binding to allow each root port to define the list of PHYs > required to drive the lanes associated with it. I think the feedback I gave on v3 still applies here (I'm talking about the comments on the patch, not the commit description). http://www.spinics.net/lists/linux-pci/msg49718.html ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs @ 2016-04-11 17:38 ` Stephen Warren 0 siblings, 0 replies; 21+ messages in thread From: Stephen Warren @ 2016-04-11 17:38 UTC (permalink / raw) To: Thierry Reding Cc: Bjorn Helgaas, Alexandre Courbot, linux-tegra, linux-pci, devicetree On 04/08/2016 10:13 AM, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > Changes to the pad controller device tree binding have required that > each lane be associated with a separate PHY. Update the PCI host bridge > device tree binding to allow each root port to define the list of PHYs > required to drive the lanes associated with it. I think the feedback I gave on v3 still applies here (I'm talking about the comments on the patch, not the commit description). http://www.spinics.net/lists/linux-pci/msg49718.html ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <570BE129.40907-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]
* Re: [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs 2016-04-11 17:38 ` Stephen Warren @ 2016-04-13 16:23 ` Thierry Reding -1 siblings, 0 replies; 21+ messages in thread From: Thierry Reding @ 2016-04-13 16:23 UTC (permalink / raw) To: Stephen Warren Cc: Bjorn Helgaas, Alexandre Courbot, linux-tegra-u79uwXL29TY76Z2rM5mHXA, linux-pci-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1: Type: text/plain, Size: 962 bytes --] On Mon, Apr 11, 2016 at 11:38:49AM -0600, Stephen Warren wrote: > On 04/08/2016 10:13 AM, Thierry Reding wrote: > > From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> > > > > Changes to the pad controller device tree binding have required that > > each lane be associated with a separate PHY. Update the PCI host bridge > > device tree binding to allow each root port to define the list of PHYs > > required to drive the lanes associated with it. > > I think the feedback I gave on v3 still applies here (I'm talking about the > comments on the patch, not the commit description). > > http://www.spinics.net/lists/linux-pci/msg49718.html Apologies, I had missed those when going over review comments. I think I've addressed all of them now, or provided rationale for why the code and binding are the way they are. I'll be sending out a v5 soon, hopefully that will have addressed any outstanding concerns. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs @ 2016-04-13 16:23 ` Thierry Reding 0 siblings, 0 replies; 21+ messages in thread From: Thierry Reding @ 2016-04-13 16:23 UTC (permalink / raw) To: Stephen Warren Cc: Bjorn Helgaas, Alexandre Courbot, linux-tegra, linux-pci, devicetree [-- Attachment #1: Type: text/plain, Size: 933 bytes --] On Mon, Apr 11, 2016 at 11:38:49AM -0600, Stephen Warren wrote: > On 04/08/2016 10:13 AM, Thierry Reding wrote: > > From: Thierry Reding <treding@nvidia.com> > > > > Changes to the pad controller device tree binding have required that > > each lane be associated with a separate PHY. Update the PCI host bridge > > device tree binding to allow each root port to define the list of PHYs > > required to drive the lanes associated with it. > > I think the feedback I gave on v3 still applies here (I'm talking about the > comments on the patch, not the commit description). > > http://www.spinics.net/lists/linux-pci/msg49718.html Apologies, I had missed those when going over review comments. I think I've addressed all of them now, or provided rationale for why the code and binding are the way they are. I'll be sending out a v5 soon, hopefully that will have addressed any outstanding concerns. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2016-04-13 16:23 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-04-08 16:13 [PATCH v4 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs Thierry Reding [not found] ` <1460131994-24493-1-git-send-email-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2016-04-08 16:13 ` [PATCH v4 2/2] PCI: tegra: Support " Thierry Reding 2016-04-08 16:13 ` Thierry Reding [not found] ` <1460131994-24493-2-git-send-email-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2016-04-08 18:05 ` Bjorn Helgaas 2016-04-08 18:05 ` Bjorn Helgaas 2016-04-11 11:11 ` Thierry Reding 2016-04-11 17:41 ` Stephen Warren 2016-04-08 16:48 ` [PATCH v4 1/2] dt-bindings: pci: tegra: Update for " Bjorn Helgaas 2016-04-08 16:48 ` Bjorn Helgaas 2016-04-11 10:23 ` Thierry Reding 2016-04-11 10:23 ` Thierry Reding 2016-04-08 19:11 ` Bjorn Helgaas 2016-04-08 19:11 ` Bjorn Helgaas 2016-04-11 11:26 ` Thierry Reding 2016-04-11 11:26 ` Thierry Reding [not found] ` <20160411112654.GC17743-EkSeR96xj6Pcmrwk2tT4+A@public.gmane.org> 2016-04-11 16:55 ` Stephen Warren 2016-04-11 16:55 ` Stephen Warren 2016-04-11 17:38 ` Stephen Warren 2016-04-11 17:38 ` Stephen Warren [not found] ` <570BE129.40907-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 2016-04-13 16:23 ` Thierry Reding 2016-04-13 16:23 ` Thierry Reding
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.