Patchwork [U-Boot] tegra20: usb: rework set_host_mode

login
register
mail settings
Submitter Lucas Stach
Date Aug. 7, 2012, 6:19 p.m.
Message ID <1344363555-32755-1-git-send-email-dev@lynxeye.de>
Download mbox | patch
Permalink /patch/175764/
State Accepted
Delegated to: Tom Warren
Headers show

Comments

Lucas Stach - Aug. 7, 2012, 6:19 p.m.
This allows for two things:
- VBus GPIO may be used on other ports than the OTG one
- VBus GPIO may be low active if specified by DT

Signed-off-by: Lucas Stach <dev@lynxeye.de>
CC: Stephen Warren <swarren@wwwdotorg.org>
CC: Tom Warren <TWarren@nvidia.com>
---
 arch/arm/cpu/armv7/tegra20/usb.c | 35 +++++++++++++++++++----------------
 1 Datei geändert, 19 Zeilen hinzugefügt(+), 16 Zeilen entfernt(-)
Stephen Warren - Aug. 7, 2012, 8:21 p.m.
On 08/07/2012 12:19 PM, Lucas Stach wrote:
> This allows for two things:
> - VBus GPIO may be used on other ports than the OTG one
> - VBus GPIO may be low active if specified by DT

Hmmm. Why would the board have control over whether VBUS is asserted if
the port isn't intended to operate in OTG mode?

Perhaps power-saving? In that case, I wonder if representing this as a
regulator rather than as a VBUS GPIO would make more sense?

But irrespective of those questions, I'm inclined to think the patch is
probably OK.
Lucas Stach - Aug. 7, 2012, 8:35 p.m.
Am Dienstag, den 07.08.2012, 14:21 -0600 schrieb Stephen Warren:
> On 08/07/2012 12:19 PM, Lucas Stach wrote:
> > This allows for two things:
> > - VBus GPIO may be used on other ports than the OTG one
> > - VBus GPIO may be low active if specified by DT
> 
> Hmmm. Why would the board have control over whether VBUS is asserted if
> the port isn't intended to operate in OTG mode?
> 
> Perhaps power-saving? In that case, I wonder if representing this as a
> regulator rather than as a VBUS GPIO would make more sense?
> 
> But irrespective of those questions, I'm inclined to think the patch is
> probably OK.
> 
On Colibri the second USB Port is powered down by default and only
powered up if we are really going to use it. And the GPIO does exactly
what it tells you from it's name: turn on VBus by triggering the USB
power switch.
Thinking about a regulator for this in U-Boot is a bit of an overkill,
for the Linux kernel this might be another story.
Stephen Warren - Aug. 7, 2012, 10 p.m.
On 08/07/2012 02:35 PM, Lucas Stach wrote:
> Am Dienstag, den 07.08.2012, 14:21 -0600 schrieb Stephen Warren:
>> On 08/07/2012 12:19 PM, Lucas Stach wrote:
>>> This allows for two things:
>>> - VBus GPIO may be used on other ports than the OTG one
>>> - VBus GPIO may be low active if specified by DT
>>
>> Hmmm. Why would the board have control over whether VBUS is asserted if
>> the port isn't intended to operate in OTG mode?
>>
>> Perhaps power-saving? In that case, I wonder if representing this as a
>> regulator rather than as a VBUS GPIO would make more sense?
>>
>> But irrespective of those questions, I'm inclined to think the patch is
>> probably OK.
>>
> On Colibri the second USB Port is powered down by default and only
> powered up if we are really going to use it. And the GPIO does exactly
> what it tells you from it's name: turn on VBus by triggering the USB
> power switch.

It's probably fine to do this; the kernel also unconditionally sets up
the VBUS GPIO irrespective of USB port mode.

> Thinking about a regulator for this in U-Boot is a bit of an overkill,
> for the Linux kernel this might be another story.

Device tree content isn't supposed to be influence by the consumer; it
should be identical irrespective of which bootloader/OS/... is using it.

So, in the future I expect this to be reworked to get rid of the
vbus-gpio property and replace it with a regulator.

Patch

diff --git a/arch/arm/cpu/armv7/tegra20/usb.c b/arch/arm/cpu/armv7/tegra20/usb.c
index 84260e6..77966e5 100644
--- a/arch/arm/cpu/armv7/tegra20/usb.c
+++ b/arch/arm/cpu/armv7/tegra20/usb.c
@@ -137,24 +137,27 @@  static const u8 utmip_elastic_limit = 16;
 /* UTMIP High Speed Sync Start Delay */
 static const u8 utmip_hs_sync_start_delay = 9;
 
-/* Put the port into host mode (this only works for OTG ports) */
+/* Put the port into host mode */
 static void set_host_mode(struct fdt_usb *config)
 {
-	if (config->dr_mode == DR_MODE_OTG) {
-		/* Check whether remote host from USB1 is driving VBus */
-		if (readl(&config->reg->phy_vbus_sensors) & VBUS_VLD_STS)
-			return;
-
-		/*
-		 * If not driving, we set the GPIO to enable VBUS. We assume
-		 * that the pinmux is set up correctly for this.
-		 */
-		if (fdt_gpio_isvalid(&config->vbus_gpio)) {
-			fdtdec_setup_gpio(&config->vbus_gpio);
-			gpio_direction_output(config->vbus_gpio.gpio, 1);
-			debug("set_host_mode: GPIO %d high\n",
-			      config->vbus_gpio.gpio);
-		}
+	/*
+	 * If we are an OTG port, check if remote host is driving VBus and
+	 * bail out in this case.
+	 */
+	if (config->dr_mode == DR_MODE_OTG &&
+		(readl(&config->reg->phy_vbus_sensors) & VBUS_VLD_STS))
+		return;
+
+	/*
+	 * If not driving, we set the GPIO to enable VBUS. We assume
+	 * that the pinmux is set up correctly for this.
+	 */
+	if (fdt_gpio_isvalid(&config->vbus_gpio)) {
+		fdtdec_setup_gpio(&config->vbus_gpio);
+		gpio_direction_output(config->vbus_gpio.gpio,
+				(config->vbus_gpio.flags & FDT_GPIO_ACTIVE_LOW) ? 0 : 1);
+		debug("set_host_mode: GPIO %d %s\n", config->vbus_gpio.gpio,
+		      (config->vbus_gpio.flags & FDT_GPIO_ACTIVE_LOW) ? "low" : "high");
 	}
 }