From patchwork Wed May 14 05:03:11 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tim Harvey X-Patchwork-Id: 348598 X-Patchwork-Delegate: sbabic@denx.de Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id DB4C1140079 for ; Wed, 14 May 2014 15:03:27 +1000 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id A15D34B90D; Wed, 14 May 2014 07:03:24 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 1iyE4XUXCxjV; Wed, 14 May 2014 07:03:24 +0200 (CEST) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 071614B90E; Wed, 14 May 2014 07:03:23 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 6A7654B90E for ; Wed, 14 May 2014 07:03:18 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Y6lN6Wxffk4B for ; Wed, 14 May 2014 07:03:16 +0200 (CEST) X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from mail-wi0-f170.google.com (mail-wi0-f170.google.com [209.85.212.170]) by theia.denx.de (Postfix) with ESMTPS id 1C70B4B90D for ; Wed, 14 May 2014 07:03:11 +0200 (CEST) Received: by mail-wi0-f170.google.com with SMTP id bs8so8105879wib.5 for ; Tue, 13 May 2014 22:03:11 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=2goS9tZ8u0mpfT0UYr+HfI71HWcz2M3tBFh0QDPDC5A=; b=dkIidjvWttBhPNtDCN3WQJmA+/7QIWstO+J8HMS6kNUM2JZKVyaaN/RFjVmQT+bqJb PiZ1gy9NhZu+4U8zzVIZ+Z0g8w5rhu2eqkB1PIu+kOdjp5b56ZCsfs2AqeQojPn78S3o uotX4PdfEorH1FR+jN/8z/E/ItM0s/BZMeHtssyg77GaShv+tBHlSzb8euvMlrRAt7l5 9uD+C4h5ohOC3E0d9Q/U9smL2Ra/lzYRs/xOT6gMePGPRCKNbtuSilGjwN0U4Sp/CQnc zthWU92prs/ctdvfiQz1kB22o3qi8LHKWNTNBhIzSjJvQdjJFjDyJeLSDrl0Hf8EXfXP bCxg== X-Gm-Message-State: ALoCoQnfjrs9k0GVatDPU6d4Pse+Yaan4E+G/yb7V5qTVCNWBAzYkCYI4nUiIJA2rDQvDEn/HLuf MIME-Version: 1.0 X-Received: by 10.180.149.240 with SMTP id ud16mr1568293wib.3.1400043791084; Tue, 13 May 2014 22:03:11 -0700 (PDT) Received: by 10.194.22.9 with HTTP; Tue, 13 May 2014 22:03:11 -0700 (PDT) In-Reply-To: References: <1398716258-8420-1-git-send-email-tharvey@gateworks.com> <1398716258-8420-13-git-send-email-tharvey@gateworks.com> <5369FCEE.5090502@denx.de> Date: Tue, 13 May 2014 22:03:11 -0700 Message-ID: From: Tim Harvey To: Stefano Babic , York Sun Cc: Otavio Salvador , u-boot , Tom Rini , Stefan Roese Subject: Re: [U-Boot] [PATCH 12/12] imx: ventana: switch to SPL X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.11 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: u-boot-bounces@lists.denx.de Errors-To: u-boot-bounces@lists.denx.de On Tue, May 13, 2014 at 9:58 PM, Tim Harvey wrote: > On Wed, May 7, 2014 at 2:29 AM, Stefano Babic wrote: >> Hi Tim, >> >> On 06/05/2014 20:18, Tim Harvey wrote: >>> >>> Stefano / York, >>> >>> While preparing for a v3 patch series of my IMX6 SPL bootloader, I >>> find that commit dec1861be90c948ea9fb771927d3d26a994d2e20 [1] breaks >>> the above code because gd is now needed within setup_i2c. >>> >>> I've always been a bit fuzzy on the order of the above calls so I dug >>> through the code and I think I understand things better. Please >>> correct any wrong assumptions I'm making below: >>> - assignment to gd should 'always' be first (before anything needs >>> it, so why not do it first) >>> - arch_cpu_init() should go next as this sets up very low level >>> CPU/SoC resources (in this case AIPS config and watchdog disable) >>> - board_early_init_f() should be next as that sets up any board-specific iomux >>> - any additional iomux necessary for SPL should go next (I take care >>> of i2c iomux and setup here) >>> - timer_init() next as you need a timer for UART and mxc i2c (for >>> delays and busy checks) >>> - preloader_console_init() next as we are now able to send something >>> over the UART (this gives me early debug for sdram config now too!) >>> - sdram setup goes next >>> - after sdram is setup, the bss can be cleared >>> - board_init_r - pass over to generic SPL code which will load/call >>> an image based on boot device >> >> I think your analyses is correct. >> >>> >>> So, if the above is correct, I should rework the above function as follows: >>> >>> void board_init_f(ulong dummy) >>> { >>> struct ventana_board_info ventana_info; >>> int board_model; >>> >>> /* Set global data pointer. */ >>> gd = &gdata; >>> >>> /* setup AIPS and disable watchdog */ >>> arch_cpu_init(); >>> >>> /* iomux and setup of i2c */ >>> board_early_init_f(); >>> i2c_setup_iomux(); >>> >>> /* setup GP timer */ >>> timer_init(); >>> >>> /* UART clocks enabled and gd valid - init serial console */ >>> preloader_console_init(); >>> >>> /* read/validate EEPROM info to determine board model and SDRAM cfg */ >>> board_model = read_eeprom(I2C_GSC, &ventana_info); >>> >>> /* provide some some default: 32bit 128MB */ >>> if (GW_UNKNOWN == board_model) { >>> ventana_info.sdram_width = 2; >>> ventana_info.sdram_size = 3; >>> } >>> >>> /* configure MMDC for SDRAM width/size and per-model calibration */ >>> spl_dram_init(8 << ventana_info.sdram_width, >>> 16 << ventana_info.sdram_size, >>> board_model); >>> >>> /* Clear the BSS. */ >>> memset(__bss_start, 0, __bss_end - __bss_start); >>> >>> /* load/boot image from boot device */ >>> board_init_r(NULL, 0); >>> } >> >> It seems reasonable, go on this way. >> >> Regards, >> Stefano >> > > Stefano, > > I've just found that one of my boards fails with the above re-org. > Strangely a board which has the same mem layout, mem width/size, CPU > and nand does not fail. > > If I make the following change: (sorry, accidentally fat fingered and sent instead of pasting). I'll continue: Things start to work properly. I took a look at arch_cpu_init() and the call that really needs to be moved (or just duplicated here) is mxs_dma_init() to start APBH DMA. The failure mode of the one board is that apparently it hangs while loading u-boot.img from NAND (which of course uses mxs_nand and thus APBH DMA). Anyone know what's going on here and why mxs_dma_init() needs to be called after MMDC setup, and before clearing the BSS? I don't like having magic placement of functions without understanding why. Thanks, Tim --- a/board/gateworks/gw_ventana/gw_ventana_spl.c +++ b/board/gateworks/gw_ventana/gw_ventana_spl.c @@ -380,9 +380,6 @@ void board_init_f(ulong dummy) */ memset((void *)gd, 0, sizeof(struct global_data)); - /* setup AIPS and disable watchdog */ - arch_cpu_init(); - /* iomux and setup of i2c */ board_early_init_f(); i2c_setup_iomux(); @@ -407,6 +404,9 @@ void board_init_f(ulong dummy) 16 << ventana_info.sdram_size, board_model); + /* setup AIPS and disable watchdog */ + arch_cpu_init(); + /* Clear the BSS. */ memset(__bss_start, 0, __bss_end - __bss_start);