diff mbox

ARM: tegra: PCIe: Provide 3.3V supply voltage

Message ID 1329493468-7741-1-git-send-email-thierry.reding@avionic-design.de
State Accepted, archived
Headers show

Commit Message

Thierry Reding Feb. 17, 2012, 3:44 p.m. UTC
The PCIe reference clock needs a 3.3V supply voltage to work properly.

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
---
This patch is based on next-20120217. Note that PCIe works without this
patch when the Linux kernel is booted with a combination of QuickBoot
and U-Boot because either apparently sets LD0 to 3.3V. This is not the
case when booting from mainline U-Boot, so it must be set explicitly.

 arch/arm/mach-tegra/board-harmony-pcie.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

Comments

Mark Brown Feb. 17, 2012, 4:39 p.m. UTC | #1
On Fri, Feb 17, 2012 at 04:44:28PM +0100, Thierry Reding wrote:

> +	err = regulator_set_voltage(regulator, 3300000, 3300000);
> +	if (err < 0)
> +		goto err_put;
> +

This is a bit weird, why is this not just set by constraints?
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding Feb. 17, 2012, 7:16 p.m. UTC | #2
* Mark Brown wrote:
> On Fri, Feb 17, 2012 at 04:44:28PM +0100, Thierry Reding wrote:
> 
> > +	err = regulator_set_voltage(regulator, 3300000, 3300000);
> > +	if (err < 0)
> > +		goto err_put;
> > +
> 
> This is a bit weird, why is this not just set by constraints?

I guess because I didn't know any better. I don't have access to the hardware
right now, so it'll have to wait until Monday. I'll follow up with a patch
after I've verified that setting via constraints works.

Thierry
Mark Brown Feb. 17, 2012, 7:25 p.m. UTC | #3
On Fri, Feb 17, 2012 at 08:16:05PM +0100, Thierry Reding wrote:
> * Mark Brown wrote:

> > This is a bit weird, why is this not just set by constraints?

> I guess because I didn't know any better. I don't have access to the hardware
> right now, so it'll have to wait until Monday. I'll follow up with a patch
> after I've verified that setting via constraints works.

OK.  In general the expected idiom is that drivers will only use
set_voltage() if they actually need to change the voltage at runtime, if
they just need a single fixed voltage then the expectation is that
either the board will boot in the correct configuration or the setup
will be done by constraints.  This replaces code with data (which is
generally a good thing) and avoids issues with things like error
handling when working with fixed voltage regulators.
diff mbox

Patch

diff --git a/arch/arm/mach-tegra/board-harmony-pcie.c b/arch/arm/mach-tegra/board-harmony-pcie.c
index 33c4fed..d521cf4 100644
--- a/arch/arm/mach-tegra/board-harmony-pcie.c
+++ b/arch/arm/mach-tegra/board-harmony-pcie.c
@@ -45,6 +45,10 @@  static int __init harmony_pcie_init(void)
 	if (IS_ERR_OR_NULL(regulator))
 		goto err_reg;
 
+	err = regulator_set_voltage(regulator, 3300000, 3300000);
+	if (err < 0)
+		goto err_put;
+
 	regulator_enable(regulator);
 
 	err = tegra_pcie_init(true, true);
@@ -55,6 +59,7 @@  static int __init harmony_pcie_init(void)
 
 err_pcie:
 	regulator_disable(regulator);
+err_put:
 	regulator_put(regulator);
 err_reg:
 	gpio_free(TEGRA_GPIO_EN_VDD_1V05_GPIO);