public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2] tegra: add Colibri T20 board support
Date: Mon, 01 Oct 2012 10:33:35 -0600	[thread overview]
Message-ID: <5069C5DF.9030600@wwwdotorg.org> (raw)
In-Reply-To: <1348949029-30000-1-git-send-email-dev@lynxeye.de>

On 09/29/2012 02:03 PM, Lucas Stach wrote:
> This adds board support for the Toradex Colibri T20 module.
> 
> Working functions:
> - SD card boot
> - USB boot
> - Network
> - NAND environment

> diff --git a/board/toradex/colibri_t20/Makefile b/board/toradex/colibri_t20/Makefile
...
> +#########################################################################
> \ No newline at end of file

I assume that's a mistake.

> diff --git a/board/toradex/colibri_t20/colibri_t20.c b/board/toradex/colibri_t20/colibri_t20.c

> +#ifdef CONFIG_USB_EHCI_TEGRA
> +void pin_mux_usb(void)
> +{
> +	/* USB 1 aka Tegra USB port 3 */
> +	pinmux_tristate_disable(PINGRP_SPIG);

I don't think that's muxing USB itself, but rather muxing perhaps the
VBUS GPIO?

> +#ifdef CONFIG_TEGRA_MMC
> +int board_mmc_init(bd_t *bd)
> +{
> +	funcmux_select(PERIPH_ID_SDMMC4, FUNCMUX_SDMMC4_ATB_GMA_4_BIT);
> +	pinmux_tristate_disable(PINGRP_GMB);

It might be useful to comment the tristate call like other boards, e.g.:

        /* For power GPIO PI6 */
        pinmux_tristate_disable(PINGRP_ATA);

so it's obvious why the call isn't done inside funcmux_select().

> diff --git a/board/toradex/dts/tegra20-colibri_t20.dts b/board/toradex/dts/tegra20-colibri_t20.dts

> +	usb at c5008000 {
> +		nvidia,vbus-gpio = <&gpio 178 1>; /* PW2 low-active */
> +	};

As an FYI, although the GPIO bindings do specify that the last cell
there is for GPIO flags, I'm not sure that we should rely on it. Not all
GPIO bindings actually have the ability to specify flags there, so if a
given board's GPIO is provided by some device whose GPIO binding doesn't
allow for flags, then it won't be possible to specify an active-low
GPIO, and this won't work.

The kernel certainly doesn't actually do anything with the flags
argument in the EHCI driver, IIRC.

Either we should forcibly change all GPIO bindings in the kernel to
require that they allow flags to be specified (probably very hard), or
remove the flags from the Tegra GPIO binding, and use a separate
property such as nvidia,vbus-gpio-active-low for this purpose.
Certainly, the latter form of approach has been taken in other places
(such as fixed regulator IIRC).

  reply	other threads:[~2012-10-01 16:33 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-29 20:03 [U-Boot] [PATCH v2] tegra: add Colibri T20 board support Lucas Stach
2012-10-01 16:33 ` Stephen Warren [this message]
2012-10-01 16:48   ` Lucas Stach
2012-10-01 16:55     ` Stephen Warren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5069C5DF.9030600@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox