public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Lucas Stach <dev@lynxeye.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/2] tegra: convert gpio_config_uart to weak symbol
Date: Sun, 19 Aug 2012 18:56:34 +0200	[thread overview]
Message-ID: <1345395394.12175.15.camel@selen> (raw)
In-Reply-To: <50214BDC.40604@wwwdotorg.org>

Hi Stephen,

Am Dienstag, den 07.08.2012, 11:09 -0600 schrieb Stephen Warren:
> On 08/06/2012 07:18 PM, Lucas Stach wrote:
> > Most boards don't need this fixup hook. To avoid a lot of empty
> > implementations in board files convert it to a weak symbol.
> 
> This seems OK on the surface, but I think there may be more opportunity
> for cleanup here.
> 
I looked a bit deeper into this and it seems that in fact the logic of
all those calls are correct and none them could be removed.

To clarify what's going on:
if we are building without CONFIG_SPI_UART_SWITCH the board file has to
provide a function gpio_config_uart() to ensure correct GPIO setting and
it's correct to call this in early_init.

Now, if we build with CONFIG_SPI_UART_SWITCH set, the board file stops
providing the gpio_config_uart() and the UART switch provides a
gpio_early_init_uart() function which does essentially the same, so we
call this instead in early init.

The confusing part is that the UART switch now provides a function also
named gpio_config_uart(), which redoes the GPIO setting after relocation
and does some internal state setting.

Now while explaining all this in this email I see how we should make
this more clear with some reasonable renaming, but we won't get around
the weak symbol if we want to keep the possibility for boards to build
without CONFIG_SPI_UART_SWITCH. I'll spin a new patch to untangle this
mess a bit.

Thanks,
Lucas

> In board/nvidia/common/board.c, I see both of the following:
> 
> board_init:
> 
> #ifdef CONFIG_SPI_UART_SWITCH
>         gpio_config_uart();
> #endif
> 
> 
> board_early_init_f:
> 
>         /* Initialize periph GPIOs */
>         gpio_early_init();
> #ifdef CONFIG_SPI_UART_SWITCH
>         gpio_early_init_uart();
> #else
>         gpio_config_uart();
> #endif
> 
> and in arch/arm/cpu/arm720t/tegra20/spl.c:
> 
> board_init_f:
> 
> #ifdef CONFIG_SPI_UART_SWITCH
>         gpio_early_init_uart();
> #else
>         gpio_config_uart();
> #endif
> 
> It sure seems like we don't need to call those two init function all
> those times. Perhaps we can clarify which of the functions are actually
> needed at all, and when they should be called.
> 

  reply	other threads:[~2012-08-19 16:56 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-07  1:18 [U-Boot] [PATCH 1/2] tegra: init MMC from common board init Lucas Stach
2012-08-07  1:18 ` [U-Boot] [PATCH 2/2] tegra: convert gpio_config_uart to weak symbol Lucas Stach
2012-08-07 17:09   ` Stephen Warren
2012-08-19 16:56     ` Lucas Stach [this message]
2012-08-07 17:04 ` [U-Boot] [PATCH 1/2] tegra: init MMC from common board init Stephen Warren
2012-08-07 17:35   ` Lucas Stach

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=1345395394.12175.15.camel@selen \
    --to=dev@lynxeye.de \
    --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