Hi Pali,

On 11.08.20 10:00, Pali Rohár wrote:
Hello!

On Tuesday 11 August 2020 08:37:46 Stefan Roese wrote:
Hi Tom,
Hi Pali,

(added Pali because of the Nokia RX51 issue)

Could you please send me a link to "problematic" patch? As you have not
inlined it in this email.

https://gitlab.denx.de/u-boot/custodians/u-boot-marvell/-/commit/7196447f2e708a83c653f39c97f2439960482ad6

Its a rebase of the original patch.

On 07.08.20 21:21, Tom Rini wrote:
On Thu, Jul 30, 2020 at 12:51:10PM +0200, Stefan Roese wrote:

Since commit 86cf1c82850f ("configs: Migrate CONFIG_NR_DRAM_BANKS") &
commit 999a772d9f24 ("Kconfig: Migrate CONFIG_NR_DRAM_BANKS"),
CONFIG_NR_DRAM_BANKS is always defined with a value (4 is default).
It makes no sense to still carry code that is guarded with
"#ifndef CONFIG_NR_DRAM_BANKS" (and similar). This patch removes
all these unreferenced code paths.

Also complete remove bi_memstart & bi_memsize from the board-info
struct. As now bi_dram[] is always enabled and should be used instead.
This removes the redundant varriables.

Signed-off-by: Stefan Roese <[email protected]>
Cc: Daniel Schwierzeck <[email protected]>
Cc: Tom Rini <[email protected]>
Cc: Ramon Fried <[email protected]>
Cc: Simon Glass <[email protected]>
Cc: Michal Simek <[email protected]>
Reviewed-by: Daniel Schwierzeck <[email protected]>

I don't see quite how, but this is breaking running
test/nokia_rx51_test.sh (or, my fixup of this to apply to current master
was wrong, and is what's breaking the test).  Please rebase and confirm
that test passes as well, thanks!

I've checked the issue with nokia_rx51_test.sh and could not find any
issues in the patch. My assumption now is, that the very old Linux
kernel (2.6.28) that is used in this Nokia test, still uses the bd_info
struct in Linux to pass the memory information (via bd_memstart &
bi_memsize), as was also done in the very old PowerPC days. With this
patch now and the removal of these fields from bd_info, this might
explain why this kernel does not boot any more (no output on the serial
console at all).

Pali, could you please check if my assumption is correct here? And if
yes, could please switch the test to using a newer kernel version? Or
remove the Linux kernel booting from the test?

Yes, Nokia N900 uses "old" ATAGs. But we cannot remove it. New kernel
version does not contain Maemo patches which are required for Maemo
system which is still widely used. And yes, people are using it with
U-Boot.

Second reason why we cannot remove support for ATAGs is that Nokia's
signed first stage bootloader pass other setup data via ATAGs for kernel
and U-Boot N900 board code parses it, reuse it and pass to kernel.

And replacing first stage bootloader is not possible because it is
signed and signing keys are secret (now probably lost).

Okay. But this patch is not removing ATAG support, but removes 2
member from the bd_info struct (bi_memstart & bi_memsize), which are
replaced (superseeded) by the bi_dram[].start/.size values. And AFAICT,
the generation of the memory ATAG is already done based on these
values:

setup_memory_tags() in arch/arm/lib/bootm.c

So I still fail to understand, why booting of this old kernel using
ATAGs does not work with this patch applied. Perhaps you could give
it a quick test? That would be really helpful. Here again the gitlab
branch that you can use for some tests:

https://gitlab.denx.de/u-boot/custodians/u-boot-marvell/-/commits/remove-config-nr-dram-banks-v3-2020-08-11

Thanks,
Stefan

Reply via email to