diff mbox

[10/10] ARM: tegra: Enable USB on Venice2

Message ID 1387469182-14398-11-git-send-email-treding@nvidia.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thierry Reding Dec. 19, 2013, 4:06 p.m. UTC
USB1 and USB3 are routed to two external connectors, while USB2 is used
for the integrated webcam.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Note: connecting a USB mouse to USB3 works properly and lsusb also detects
the integrated webcam, though I haven't tested whether the camera actually
works.

USB1 doesn't seem to work, but I suspect that might just be because it's
used for recovery mode and we can't switch it around at runtime yet.
---
 arch/arm/boot/dts/tegra124-venice2.dts | 47 +++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

Comments

Stephen Warren Dec. 19, 2013, 9:37 p.m. UTC | #1
On 12/19/2013 09:06 AM, Thierry Reding wrote:
> USB1 and USB3 are routed to two external connectors, while USB2 is used
> for the integrated webcam.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Note: connecting a USB mouse to USB3 works properly and lsusb also detects
> the integrated webcam, though I haven't tested whether the camera actually
> works.
> 
> USB1 doesn't seem to work, but I suspect that might just be because it's
> used for recovery mode and we can't switch it around at runtime yet.

The two regulators you added don't actually hook up to any GPIOs for
control of VBUS, so perhaps this isn't surprising. The regulators added
for VBUS control by Laxman's patch (which I already applied) *do* have
some gpio properties. However, testing indicates:

* With Laxman's patch applied, and your patch 1/10 not applied since
it's a duplicate, this patch doesn't work, i.e. I see no USB devices.

* With Laxman's patch reverted and your patch 1/10 replacing it, I do
see USB devices, and they work. However, then we're not actually
controlling VBUS, so USB1 doesn't work.

Can you please rebase this whole series on the latest Tegra for-3.14/dt
and sort out the issues? Thanks.
Jim Lin Dec. 20, 2013, 2:59 a.m. UTC | #2
About USB1 issue, for chips after Tegra30 (including Tegra30), BIASPD is
controlled by USB1 controller.
In ehci-tegra.c probe phase, we assert reset and deassert reset on
controller clock.
If initializing sequence follows USB1 and then USB2, we are safe.
If initializing sequence follows USB2 and then USB1, then in
phy-tegra-usb.c a counter utmip_pad_count which has been increased for
USB2 will prevent UTMIP_BIASPD from being cleared while initializing for
USB1 controller (UTMIP_BIASPD will be set again after asserting and
deasserting reset on USB1 controller clock in ehci-tegra.c probe for
USB1 controller).

There are several ways to fix.
The local workaround I use is as following in phy-tegra-usb.c.

static void utmip_pad_power_on(struct tegra_usb_phy *phy)
{
	unsigned long val, flags;
	void __iomem *base = phy->pad_regs;
	struct tegra_utmip_config *config = phy->config;

	clk_prepare_enable(phy->pad_clk);

	spin_lock_irqsave(&utmip_pad_lock, flags);

	val = readl(base + UTMIP_BIAS_CFG0);
	if ((utmip_pad_count++ == 0) || (val & UTMIP_BIASPD)) {
		val &= ~(UTMIP_OTGPD | UTMIP_BIASPD)

On Fri, 2013-12-20 at 05:37 +0800, Stephen Warren wrote:
> On 12/19/2013 09:06 AM, Thierry Reding wrote:
> > USB1 and USB3 are routed to two external connectors, while USB2 is used
> > for the integrated webcam.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > Note: connecting a USB mouse to USB3 works properly and lsusb also detects
> > the integrated webcam, though I haven't tested whether the camera actually
> > works.
> > 
> > USB1 doesn't seem to work, but I suspect that might just be because it's
> > used for recovery mode and we can't switch it around at runtime yet.
> 
> The two regulators you added don't actually hook up to any GPIOs for
> control of VBUS, so perhaps this isn't surprising. The regulators added
> for VBUS control by Laxman's patch (which I already applied) *do* have
> some gpio properties. However, testing indicates:
> 
> * With Laxman's patch applied, and your patch 1/10 not applied since
> it's a duplicate, this patch doesn't work, i.e. I see no USB devices.
> 
> * With Laxman's patch reverted and your patch 1/10 replacing it, I do
> see USB devices, and they work. However, then we're not actually
> controlling VBUS, so USB1 doesn't work.
> 
> Can you please rebase this whole series on the latest Tegra for-3.14/dt
> and sort out the issues? Thanks.
> --
> 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

--nvpublic
Thierry Reding Dec. 20, 2013, 11:53 a.m. UTC | #3
On Thu, Dec 19, 2013 at 02:37:36PM -0700, Stephen Warren wrote:
> On 12/19/2013 09:06 AM, Thierry Reding wrote:
> > USB1 and USB3 are routed to two external connectors, while USB2 is used
> > for the integrated webcam.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > Note: connecting a USB mouse to USB3 works properly and lsusb also detects
> > the integrated webcam, though I haven't tested whether the camera actually
> > works.
> > 
> > USB1 doesn't seem to work, but I suspect that might just be because it's
> > used for recovery mode and we can't switch it around at runtime yet.
> 
> The two regulators you added don't actually hook up to any GPIOs for
> control of VBUS, so perhaps this isn't surprising. The regulators added
> for VBUS control by Laxman's patch (which I already applied) *do* have
> some gpio properties. However, testing indicates:
> 
> * With Laxman's patch applied, and your patch 1/10 not applied since
> it's a duplicate, this patch doesn't work, i.e. I see no USB devices.
> 
> * With Laxman's patch reverted and your patch 1/10 replacing it, I do
> see USB devices, and they work. However, then we're not actually
> controlling VBUS, so USB1 doesn't work.
> 
> Can you please rebase this whole series on the latest Tegra for-3.14/dt
> and sort out the issues? Thanks.

I've looked into this a bit, but I don't see why it isn't working. The
PN4 and PN5 are the correct GPIOs for USB1 and USB3 VBUS, but hooking
them up indeed breaks things. For some reason adding those even causes
the camera on USB2 to not work properly.

Thierry
diff mbox

Patch

diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts
index b770e903fabb..efdf93e6f5cc 100644
--- a/arch/arm/boot/dts/tegra124-venice2.dts
+++ b/arch/arm/boot/dts/tegra124-venice2.dts
@@ -534,7 +534,7 @@ 
 					ams,enable-tracking;
 				};
 
-				ldo4 {
+				cam_2v8_reg: ldo4 {
 					regulator-name = "avdd_cam";
 					regulator-min-microvolt = <2800000>;
 					regulator-max-microvolt = <2800000>;
@@ -715,6 +715,33 @@ 
 		};
 	};
 
+	usb@7d000000 {
+		status = "okay";
+	};
+
+	usb-phy@7d000000 {
+		status = "okay";
+		vbus-supply = <&usb1_vbus_reg>;
+	};
+
+	usb@7d004000 {
+		status = "okay";
+	};
+
+	usb-phy@7d004000 {
+		status = "okay";
+		vbus-supply = <&cam_2v8_reg>;
+	};
+
+	usb@7d008000 {
+		status = "okay";
+	};
+
+	usb-phy@7d008000 {
+		status = "okay";
+		vbus-supply = <&usb3_vbus_reg>;
+	};
+
 	backlight: backlight {
 		compatible = "pwm-backlight";
 
@@ -788,6 +815,24 @@ 
 			gpio = <&gpio TEGRA_GPIO(P, 2) 0 GPIO_ACTIVE_HIGH>;
 			enable-active-high;
 		};
+
+		usb1_vbus_reg: regulator@3 {
+			compatible = "regulator-fixed";
+			reg = <3>;
+			regulator-name = "usb1_vbus";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			regulator-boot-on;
+		};
+
+		usb3_vbus_reg: regulator@4 {
+			compatible = "regulator-fixed";
+			reg = <4>;
+			regulator-name = "usb3_vbus";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			regulator-boot-on;
+		};
 	};
 
 	sound {