[PATCH v2] CONFIG_NR_DRAM_BANKS: Remove unreferenced code as its always defined

[Stefan Roese]

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 <sr at denx.de>
>>>> Cc: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
>>>> Cc: Tom Rini <trini at konsulko.com>
>>>> Cc: Ramon Fried <ramon.fried at gmail.com>
>>>> Cc: Simon Glass <sjg at chromium.org>
>>>> Cc: Michal Simek <michal.simek at xilinx.com>
>>>> Reviewed-by: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
>>>
>>> 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