Re: [GIT PULL 1/3] ARM: tegra: rework PCIe regulators

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1810 bytes --]

On Thu, Jul 10, 2014 at 12:15:28PM +0200, Thierry Reding wrote:
> On Mon, Jul 07, 2014 at 09:45:46PM -0700, Olof Johansson wrote:
[...]
> > If you have to stay compatible, then I suggest you try to fill in
> > local driver variables with derivatives of the old properties (and
> > directly from the newer properties where you can). I haven't looked at
> > the specifics here so I don't know how hard it might be.
> > 
> > If you are 100% sure that you don't have to stay compatible, then you
> > can remove the code handling the old bindings. Still, even then I am a
> > little worried about dependencies (and more importantly conflicts)
> > between these dtsi changes and others done by tegra platform code for
> > this release. I suppose that can be resolved by having this as a base
> > of any DT changes for tegra if needed.
> 
> To be honest, I'm very much tempted to just drop this series. Even if
> that means keeping a totally broken DT binding. But frankly I don't have
> any energy left to debate DT stability.

So this kept bugging me and I couldn't leave it alone after all. How
about if I squash in the attached patch. I've verified that that keeps
compatibility with old device trees on TrimSlice and Beaver. I think the
remainder of the series could still remain as-is (the top few commits
that you said shouldn't be there) if I squash this into

	PCI: tegra: Implement accurate power supply scheme

That way the binding will be the new one so that people don't get any
wrong ideas about taking shortcuts while still preserving compatibility
with existing DTBs.

Interestingly, despite my initial disgust for having to keep around old
code (it's in fact new code in this case) for compatibility reasons, it
ended up making the code look more mature.

Thierry

[-- Attachment #1.2: 0001-PCI-tegra-Preserve-DT-backwards-compatibility.patch --]
[-- Type: text/x-diff, Size: 3742 bytes --]

From 569fd6029e77f6b3ea0dc23dfc0ef32239cf55c9 Mon Sep 17 00:00:00 2001
From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Date: Thu, 17 Jul 2014 15:29:46 +0200
Subject: [PATCH] PCI: tegra: Preserve DT backwards-compatibility

Parse the set of power supplies in the deprecated version of the device
tree binding to remain backwards-compatible with old device trees.

Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/pci/host/pci-tegra.c | 78 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 76 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 7df5aaf58921..d697587dbb7c 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -1359,6 +1359,66 @@ static int tegra_pcie_get_xbar_config(struct tegra_pcie *pcie, u32 lanes,
 }
 
 /*
+ * Check whether a given set of supplies is available in a device tree node.
+ * This is used to check whether the new or the legacy device tree bindings
+ * should be used.
+ */
+static bool of_regulator_bulk_available(struct device_node *np,
+					struct regulator_bulk_data *supplies,
+					unsigned int num_supplies)
+{
+	char property[32];
+	unsigned int i;
+
+	for (i = 0; i < num_supplies; i++) {
+		snprintf(property, 32, "%s-supply", supplies[i].supply);
+
+		if (of_find_property(np, property, NULL) == NULL)
+			return false;
+	}
+
+	return true;
+}
+
+/*
+ * Old versions of the device tree binding for this device used a set of power
+ * supplies that didn't match the hardware inputs. This happened to work for a
+ * number of cases but is not future proof. However to preserve backwards-
+ * compatibility with old device trees, this function will try to use the old
+ * set of supplies.
+ */
+static int tegra_pcie_get_legacy_regulators(struct tegra_pcie *pcie)
+{
+	struct device_node *np = pcie->dev->of_node;
+
+	if (of_device_is_compatible(np, "nvidia,tegra30-pcie"))
+		pcie->num_supplies = 3;
+	else if (of_device_is_compatible(np, "nvidia,tegra20-pcie"))
+		pcie->num_supplies = 2;
+
+	if (pcie->num_supplies == 0) {
+		dev_err(pcie->dev, "device %s not supported in legacy mode\n",
+			np->full_name);
+		return -ENODEV;
+	}
+
+	pcie->supplies = devm_kcalloc(pcie->dev, pcie->num_supplies,
+				      sizeof(*pcie->supplies),
+				      GFP_KERNEL);
+	if (!pcie->supplies)
+		return -ENOMEM;
+
+	pcie->supplies[0].supply = "pex-clk";
+	pcie->supplies[1].supply = "vdd";
+
+	if (pcie->num_supplies > 2)
+		pcie->supplies[2].supply = "avdd";
+
+	return devm_regulator_bulk_get(pcie->dev, pcie->num_supplies,
+				       pcie->supplies);
+}
+
+/*
  * Obtains the list of regulators required for a particular generation of the
  * IP block.
  *
@@ -1422,8 +1482,22 @@ static int tegra_pcie_get_regulators(struct tegra_pcie *pcie, u32 lane_mask)
 		pcie->supplies[4].supply = "vddio-pex-clk";
 	}
 
-	return devm_regulator_bulk_get(pcie->dev, pcie->num_supplies,
-				       pcie->supplies);
+	if (of_regulator_bulk_available(pcie->dev->of_node, pcie->supplies,
+					pcie->num_supplies))
+		return devm_regulator_bulk_get(pcie->dev, pcie->num_supplies,
+					       pcie->supplies);
+
+	/*
+	 * If not all regulators are available for this new scheme, assume
+	 * that the device tree complies with an older version of the device
+	 * tree binding.
+	 */
+	dev_info(pcie->dev, "using legacy DT binding for power supplies\n");
+
+	devm_kfree(pcie->dev, pcie->supplies);
+	pcie->num_supplies = 0;
+
+	return tegra_pcie_get_legacy_regulators(pcie);
 }
 
 static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
-- 
2.0.1


[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]