All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Roese <sr@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3
Date: Thu, 20 Jun 2013 19:01:22 +0200	[thread overview]
Message-ID: <51C33562.60707@denx.de> (raw)
In-Reply-To: <20130620184209.43ed9fad@lilith>

Hi Albert,

On 20.06.2013 18:42, Albert ARIBAUD wrote:
>> diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
>> index a9657d1..b05f66a 100644
>> --- a/arch/arm/lib/crt0.S
>> +++ b/arch/arm/lib/crt0.S
>> @@ -85,7 +85,13 @@ ENTRY(_main)
>>  	bic	sp, sp, #7	/* 8-byte alignment for ABI compliance */
>>  	sub	sp, #GD_SIZE	/* allocate one GD above SP */
>>  	bic	sp, sp, #7	/* 8-byte alignment for ABI compliance */
>> +#if !defined(CONFIG_SPL_BUILD)
>> +/*
>> + * SPL already has GD set to the correct location (in s_init), we mustn't
>> + * move it around now since some data (clocks etc) is already present.
>> + */
>>  	mov	r8, sp		/* GD is above SP */
>> +#endif
>>  	mov	r0, #0
>>  	bl	board_init_f
>>  
> 
> NAK in this form. I don't want gd to be set "somewhere in the code"
> depending on the actual target; I want it set in crt0.S, period.
> 
> I see there are several locations in ARM architecture or board code
> which set up GD themselves in the same manner as OMAP does. Luckily all
> these locations set it to the same value, the address of gdata.
> 
> The correct fix (read: the one I won't NAK) is thus to add a #else
> clause in the code above, in which r8 will be set to =gdata, and to
> remove the corresponding assignments in the various places where they
> reside.

Here's the problem. Setting r8 in _main is too late. As it has already
been used (in the current implementation) to store some data (e.g.
clocks for baudrate generation etc). Here the code from
arch/arm/cpu/armv7/omap3/board.c:s_init():

#ifdef CONFIG_SPL_BUILD
	gd = &gdata;

	preloader_console_init();

	timer_init();
#endif

Note that this is done *before* _main() is called (we are talking about
SPL for OMAP here). And it did cost me quite some time to find this
problem, that r8 was re-configured in _main() and all the already set
values disappeared again (no serial output etc).

Yes, this needs some cleanup/fixup. Unfortunately I won't find the time
to look into such a cleanup in the next days. Perhaps somebody else
might jump in...

Thanks,
Stefan

  reply	other threads:[~2013-06-20 17:01 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-14  8:54 [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3 Stefan Roese
2013-06-14  8:55 ` [U-Boot] [PATCH 2/3] arm: omap3: spl: Fix problem with 8bit NAND devices Stefan Roese
2013-07-30 13:25   ` [U-Boot] [U-Boot, " Tom Rini
2013-06-14  8:55 ` [U-Boot] [PATCH 3/3] arm: omap3: Add SPL support to cm_t35 Stefan Roese
2013-06-17 11:53   ` Igor Grinberg
2013-06-17 12:38     ` Tom Rini
2013-06-17 13:38       ` Igor Grinberg
2013-06-17 13:52         ` Stefan Roese
2013-06-17 14:03   ` [U-Boot] [PATCH v2 " Stefan Roese
2013-06-18  6:14     ` Nikita Kiryanov
2013-07-30 10:52   ` [U-Boot] [PATCH v3 " Stefan Roese
2013-07-30 12:10     ` Albert ARIBAUD
2013-07-30 12:14       ` Stefan Roese
2013-11-15  7:51   ` [U-Boot] [PATCH v4] " Stefan Roese
     [not found]     ` <5288BBAC.2020307@compulab.co.il>
     [not found]       ` <5295BF6C.20902@compulab.co.il>
     [not found]         ` <5295C3F8.50101@denx.de>
     [not found]           ` <529E01B7.4070402@compulab.co.il>
2013-12-04 11:38             ` [U-Boot] Fwd: " Stefan Roese
2013-12-04 11:57               ` Tom Rini
2013-12-04 12:02                 ` Stefan Roese
2013-12-04 12:15               ` Gupta, Pekon
2013-12-04 12:38                 ` Stefan Roese
2013-06-20 16:42 ` [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3 Albert ARIBAUD
2013-06-20 17:01   ` Stefan Roese [this message]
2013-06-20 17:51     ` Albert ARIBAUD
2013-06-20 18:28       ` Stefan Roese
2013-06-20 19:18         ` Albert ARIBAUD
2013-06-21  2:13 ` [U-Boot] [PATCH v2 " Stefan Roese
2013-06-21  8:57   ` Albert ARIBAUD
2013-06-21  9:10 ` [U-Boot] [PATCH v3 " Stefan Roese
2013-06-21 10:30   ` Albert ARIBAUD
2013-06-21 10:39     ` Stefan Roese
2013-06-21 10:42 ` [U-Boot] [PATCH v4 " Stefan Roese
2013-06-21 11:02   ` Albert ARIBAUD
2013-06-25  7:14 ` [U-Boot] [PATCH v5 " Stefan Roese
2013-06-27  8:27   ` Albert ARIBAUD
2013-07-03 19:47     ` Tom Rini
2013-07-04 11:58       ` Albert ARIBAUD
2013-07-15 14:33 ` [U-Boot] [PATCH " Tom Rini
2013-07-16  6:24   ` Stefan Roese
2013-07-16 14:36     ` Tom Rini

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=51C33562.60707@denx.de \
    --to=sr@denx.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.