openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* AST2500 u-boot breakage with CONFIG_RAM=y
@ 2022-09-30 21:09 Zev Weiss
  2022-10-01  3:21 ` [Phishing Risk] [External] " Zhang Jian
  0 siblings, 1 reply; 5+ messages in thread
From: Zev Weiss @ 2022-09-30 21:09 UTC (permalink / raw)
  To: openbmc
  Cc: Andrew Jeffery, Dylan Hung, Chia-Wei Wang, Ryan Chen, Joel Stanley

Hello u-boot/Aspeed folks,

I recently set about getting e3c246d4i switched over to 
u-boot-aspeed-sdk from the old u-boot branch, but after building and 
installing it I ran into some odd misbehavior.

It's not entirely consistent -- sometimes the kernel hand-off goes 
alright but then the kernel's boot sequence hangs a few seconds in, 
sometimes it hangs before I get any kernel console output at all, and 
sometimes I end up with the following error message from u-boot as it 
loads the kernel FIT image:

   fdt_find_or_add_subnode: chosen: FDT_ERR_BADSTRUCTURE
   ERROR: /chosen node create failed
    - must RESET the board to recover.
   
   FDT creation failed! hanging...### ERROR ### Please RESET the board ###

Additionally, if I try to boot a FIT image loaded via TFTP I 
consistently get checksum-mismatch errors on the kernel subimage portion 
(though the exact same FIT image loaded from flash passes that check).  

I wasn't able to reproduce any misbehavior in qemu, unfortunately.

I had previously tested the 2019.04 u-boot branch on this platform and 
not hit any problems like this, so I started bisecting between the 
version that had worked before and the current version, which landed me 
on Joel's recent patch adding CONFIG_RAM=y to the evb-ast2500 defconfig 
(commit 0545d7325ff0cb1a77dc6f8007f74f415840fd90).  I confirmed that 
setting CONFIG_RAM=n gets things working normally again.

Looking into the code that CONFIG_RAM=y enables, I added some debug 
prints to arch/arm/mach-aspeed/ast2500/board_common.c and verified that 
the dram_init() routine was setting gd->ram_size to the same value in 
both cases.  However, I noticed there's also one instruction in 
platform.S that's included when CONFIG_RAM is enabled [1].  My 
recollection of ARM assembly is fairly rusty, but I believe that's just 
an early return short-circuiting the rest of the initialization code in 
that routine, perhaps with the intent that that work will get taken care 
of by C code in the Aspeed RAM driver instead?  In any case, I 
experimented with leaving CONFIG_RAM=y but removing just that 
instruction, and it seems to resolve the problems I was seeing -- so if 
my interpretation does match the actual intent, it seems like there's 
some discrepancy between the initialization done in the C code and the 
assembly code, though I'm not sure what it might be.  For what it's 
worth, it did seem like the CONFIG_RAM=n build ran noticeably faster.

Does anyone have any thoughts as to what might be going on, or tips on 
how to go about debugging further?

Thanks,
Zev

[1] https://github.com/openbmc/u-boot/blob/8e834983d0a6b9265cee1674564b016565630883/arch/arm/mach-aspeed/ast2500/platform.S#L663-L665


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Phishing Risk] [External] AST2500 u-boot breakage with CONFIG_RAM=y
  2022-09-30 21:09 AST2500 u-boot breakage with CONFIG_RAM=y Zev Weiss
@ 2022-10-01  3:21 ` Zhang Jian
  2022-10-03  4:09   ` Dylan Hung
  0 siblings, 1 reply; 5+ messages in thread
From: Zhang Jian @ 2022-10-01  3:21 UTC (permalink / raw)
  To: Zev Weiss
  Cc: Ryan Chen, Andrew Jeffery, openbmc, Joel Stanley, Dylan Hung,
	Chia-Wei Wang

Hi Zev,

I had some similar questions about this .

On Sat, Oct 1, 2022 at 5:09 AM Zev Weiss <zev@bewilderbeest.net> wrote:
>
> Hello u-boot/Aspeed folks,
>
> I recently set about getting e3c246d4i switched over to
> u-boot-aspeed-sdk from the old u-boot branch, but after building and
> installing it I ran into some odd misbehavior.
>
> It's not entirely consistent -- sometimes the kernel hand-off goes
> alright but then the kernel's boot sequence hangs a few seconds in,
Load the kernel is slow, will cause my 22's watchdog timeout.
> sometimes it hangs before I get any kernel console output at all, and
I disabled the watchdog, and always hang in the kernel.
[    2.382046] ftgmac100 1e660000.ethernet: Read MAC address 2e:38:9e:24:b0:7c p
[    2.389968] ftgmac100 1e660000.ethernet: Using NCSI interface
[    2.397721] ftgmac100 1e660000.ethernet eth0: irq 20, mapped at (ptrval)
[    2.405224] ftgmac100 1e680000.ethernet: Read MAC address d2:8a:d4:b8:26:da
> sometimes I end up with the following error message from u-boot as it
> loads the kernel FIT image:
>
>    fdt_find_or_add_subnode: chosen: FDT_ERR_BADSTRUCTURE
>    ERROR: /chosen node create failed
>     - must RESET the board to recover.
>
>    FDT creation failed! hanging...### ERROR ### Please RESET the board ###
>
> Additionally, if I try to boot a FIT image loaded via TFTP I
> consistently get checksum-mismatch errors on the kernel subimage portion
> (though the exact same FIT image loaded from flash passes that check).
>
> I wasn't able to reproduce any misbehavior in qemu, unfortunately.
>
> I had previously tested the 2019.04 u-boot branch on this platform and
> not hit any problems like this, so I started bisecting between the
> version that had worked before and the current version, which landed me
> on Joel's recent patch adding CONFIG_RAM=y to the evb-ast2500 defconfig
> (commit 0545d7325ff0cb1a77dc6f8007f74f415840fd90).  I confirmed that
> setting CONFIG_RAM=n gets things working normally again.
>
Me too.

> Looking into the code that CONFIG_RAM=y enables, I added some debug
> prints to arch/arm/mach-aspeed/ast2500/board_common.c and verified that
> the dram_init() routine was setting gd->ram_size to the same value in
> both cases.  However, I noticed there's also one instruction in
> platform.S that's included when CONFIG_RAM is enabled [1].  My
> recollection of ARM assembly is fairly rusty, but I believe that's just
> an early return short-circuiting the rest of the initialization code in
> that routine, perhaps with the intent that that work will get taken care
> of by C code in the Aspeed RAM driver instead?  In any case, I
> experimented with leaving CONFIG_RAM=y but removing just that
> instruction, and it seems to resolve the problems I was seeing -- so if
> my interpretation does match the actual intent, it seems like there's
> some discrepancy between the initialization done in the C code and the
> assembly code, though I'm not sure what it might be.  For what it's
> worth, it did seem like the CONFIG_RAM=n build ran noticeably faster.
>
> Does anyone have any thoughts as to what might be going on, or tips on
> how to go about debugging further?
>
> Thanks,
> Zev
>
> [1] https://github.com/openbmc/u-boot/blob/8e834983d0a6b9265cee1674564b016565630883/arch/arm/mach-aspeed/ast2500/platform.S#L663-L665
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [Phishing Risk] [External] AST2500 u-boot breakage with CONFIG_RAM=y
  2022-10-01  3:21 ` [Phishing Risk] [External] " Zhang Jian
@ 2022-10-03  4:09   ` Dylan Hung
  2022-10-03  7:12     ` Joel Stanley
  0 siblings, 1 reply; 5+ messages in thread
From: Dylan Hung @ 2022-10-03  4:09 UTC (permalink / raw)
  To: Zhang Jian, Zev Weiss
  Cc: Andrew Jeffery, openbmc, Ryan Chen, ChiaWei Wang, Joel Stanley

Hi Zev, Zhang,

Aspeed recommends using "CONFIG_RAM=n" on AST2500 since platform.S is created by Aspeed.

> -----Original Message-----
> From: Zhang Jian [mailto:zhangjian.3032@bytedance.com]
> Sent: Saturday, October 1, 2022 11:21 AM
> To: Zev Weiss <zev@bewilderbeest.net>
> Cc: openbmc@lists.ozlabs.org; Andrew Jeffery <andrew@aj.id.au>; Dylan
> Hung <dylan_hung@aspeedtech.com>; ChiaWei Wang
> <chiawei_wang@aspeedtech.com>; Ryan Chen
> <ryan_chen@aspeedtech.com>; Joel Stanley <joel@jms.id.au>
> Subject: Re: [Phishing Risk] [External] AST2500 u-boot breakage with
> CONFIG_RAM=y
> 
> Hi Zev,
> 
> I had some similar questions about this .
> 
> On Sat, Oct 1, 2022 at 5:09 AM Zev Weiss <zev@bewilderbeest.net> wrote:
> >
> > Hello u-boot/Aspeed folks,
> >
> > I recently set about getting e3c246d4i switched over to
> > u-boot-aspeed-sdk from the old u-boot branch, but after building and
> > installing it I ran into some odd misbehavior.
> >
> > It's not entirely consistent -- sometimes the kernel hand-off goes
> > alright but then the kernel's boot sequence hangs a few seconds in,
> Load the kernel is slow, will cause my 22's watchdog timeout.
> > sometimes it hangs before I get any kernel console output at all, and
> I disabled the watchdog, and always hang in the kernel.
> [    2.382046] ftgmac100 1e660000.ethernet: Read MAC address
> 2e:38:9e:24:b0:7c p
> [    2.389968] ftgmac100 1e660000.ethernet: Using NCSI interface
> [    2.397721] ftgmac100 1e660000.ethernet eth0: irq 20, mapped at
> (ptrval)
> [    2.405224] ftgmac100 1e680000.ethernet: Read MAC address
> d2:8a:d4:b8:26:da
> > sometimes I end up with the following error message from u-boot as it
> > loads the kernel FIT image:
> >
> >    fdt_find_or_add_subnode: chosen: FDT_ERR_BADSTRUCTURE
> >    ERROR: /chosen node create failed
> >     - must RESET the board to recover.
> >
> >    FDT creation failed! hanging...### ERROR ### Please RESET the board
> > ###
> >
> > Additionally, if I try to boot a FIT image loaded via TFTP I
> > consistently get checksum-mismatch errors on the kernel subimage
> > portion (though the exact same FIT image loaded from flash passes that
> check).
> >
> > I wasn't able to reproduce any misbehavior in qemu, unfortunately.
> >
> > I had previously tested the 2019.04 u-boot branch on this platform and
> > not hit any problems like this, so I started bisecting between the
> > version that had worked before and the current version, which landed
> > me on Joel's recent patch adding CONFIG_RAM=y to the evb-ast2500
> > defconfig (commit 0545d7325ff0cb1a77dc6f8007f74f415840fd90).  I
> > confirmed that setting CONFIG_RAM=n gets things working normally again.
> >
> Me too.
> 
> > Looking into the code that CONFIG_RAM=y enables, I added some debug
> > prints to arch/arm/mach-aspeed/ast2500/board_common.c and verified
> > that the dram_init() routine was setting gd->ram_size to the same
> > value in both cases.  However, I noticed there's also one instruction
> > in platform.S that's included when CONFIG_RAM is enabled [1].  My
> > recollection of ARM assembly is fairly rusty, but I believe that's
> > just an early return short-circuiting the rest of the initialization
> > code in that routine, perhaps with the intent that that work will get
> > taken care of by C code in the Aspeed RAM driver instead?  In any

Yes, that instruction is intended to bypass the dram initialization in platform.S

> > case, I experimented with leaving CONFIG_RAM=y but removing just that
> > instruction, and it seems to resolve the problems I was seeing -- so
> > if my interpretation does match the actual intent, it seems like
> > there's some discrepancy between the initialization done in the C code
> > and the assembly code, though I'm not sure what it might be.  For what

The C code will early return if bit6 of scu->vga_handshake[0] is set, which
means the dram initialization is actually done by platform.S, not the C code.

https://github.com/openbmc/u-boot/blob/8e834983d0a6b9265cee1674564b016565630883/drivers/ram/aspeed/sdram_ast2500.c#L422-L430
https://github.com/openbmc/u-boot/blob/8e834983d0a6b9265cee1674564b016565630883/drivers/ram/aspeed/sdram_ast2500.c#L461


> > it's worth, it did seem like the CONFIG_RAM=n build ran noticeably faster.
> >
> > Does anyone have any thoughts as to what might be going on, or tips on
> > how to go about debugging further?
> >
> > Thanks,
> > Zev
> >
> > [1]
> >
> https://github.com/openbmc/u-boot/blob/8e834983d0a6b9265cee1674564
> b016565630883/arch/arm/mach-aspeed/ast2500/platform.S#L663-L665
> >


--
Dylan

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Phishing Risk] [External] AST2500 u-boot breakage with CONFIG_RAM=y
  2022-10-03  4:09   ` Dylan Hung
@ 2022-10-03  7:12     ` Joel Stanley
  2022-10-05  8:42       ` Dylan Hung
  0 siblings, 1 reply; 5+ messages in thread
From: Joel Stanley @ 2022-10-03  7:12 UTC (permalink / raw)
  To: Dylan Hung
  Cc: Ryan Chen, Zev Weiss, Andrew Jeffery, ChiaWei Wang, Zhang Jian, openbmc

On Mon, 3 Oct 2022 at 04:09, Dylan Hung <dylan_hung@aspeedtech.com> wrote:
>
> Hi Zev, Zhang,
>
> Aspeed recommends using "CONFIG_RAM=n" on AST2500 since platform.S is created by Aspeed.

Can you send a fix to your SDK (which I will rebase into the openbmc
tree) that makes it clear that CONFIG_RAM=n means it will use
platform.S?

It would be even better if there was only one way to do the DRAM
training in the tree. Could Aspeed look at improving this?

Cheers,

Joel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [Phishing Risk] [External] AST2500 u-boot breakage with CONFIG_RAM=y
  2022-10-03  7:12     ` Joel Stanley
@ 2022-10-05  8:42       ` Dylan Hung
  0 siblings, 0 replies; 5+ messages in thread
From: Dylan Hung @ 2022-10-05  8:42 UTC (permalink / raw)
  To: Joel Stanley
  Cc: Ryan Chen, Zev Weiss, Andrew Jeffery, ChiaWei Wang, Zhang Jian, openbmc

> -----Original Message-----
> From: Joel Stanley [mailto:joel@jms.id.au]
> Sent: Monday, October 3, 2022 3:13 PM
> To: Dylan Hung <dylan_hung@aspeedtech.com>
> Cc: Zhang Jian <zhangjian.3032@bytedance.com>; Zev Weiss
> <zev@bewilderbeest.net>; openbmc@lists.ozlabs.org; Andrew Jeffery
> <andrew@aj.id.au>; ChiaWei Wang <chiawei_wang@aspeedtech.com>; Ryan
> Chen <ryan_chen@aspeedtech.com>
> Subject: Re: [Phishing Risk] [External] AST2500 u-boot breakage with
> CONFIG_RAM=y
> 
> On Mon, 3 Oct 2022 at 04:09, Dylan Hung <dylan_hung@aspeedtech.com>
> wrote:
> >
> > Hi Zev, Zhang,
> >
> > Aspeed recommends using "CONFIG_RAM=n" on AST2500 since platform.S is
> created by Aspeed.
> 
> Can you send a fix to your SDK (which I will rebase into the openbmc
> tree) that makes it clear that CONFIG_RAM=n means it will use platform.S?
> 
> It would be even better if there was only one way to do the DRAM training in
> the tree. Could Aspeed look at improving this?

How about I force platform.S to initialize the DRAM, and remove the compile
option for C driver?

arch/arm/mach-aspeed/ast2500/platform.S

    ldr   r0, =0x1e6e0000
    ldr   r1, =0xFC600309
    str   r1, [r0]
+ /*
+  * Aspeed recommends using platform.S for DRAM initialization.
#ifdef CONFIG_RAM
	mov   pc, lr
#endif
+ */

drivers/ram/aspeed/Makefile

ifndef CONFIG_SPL_BUILD
- obj-$(CONFIG_ASPEED_AST2500) += sdram_ast2500.o
obj-$(CONFIG_ASPEED_AST2600) += sdram_ast2600.o
endif
else
- obj-$(CONFIG_ASPEED_AST2500) += sdram_ast2500.o
obj-$(CONFIG_ASPEED_AST2600) += sdram_ast2600.o
endif

> 
> Cheers,
> 
> Joel

--
Dylan

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-10-05  8:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-30 21:09 AST2500 u-boot breakage with CONFIG_RAM=y Zev Weiss
2022-10-01  3:21 ` [Phishing Risk] [External] " Zhang Jian
2022-10-03  4:09   ` Dylan Hung
2022-10-03  7:12     ` Joel Stanley
2022-10-05  8:42       ` Dylan Hung

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).