[U-Boot] [PATCH] MIPS: use CONFIG_SYS_TEXT_BASE in relocation calculations

[Daniel Schwierzeck]

2013/11/11 Gabor Juhos <juhosg at openwrt.org>:
> 2013.11.11. 20:36 keltezéssel, Daniel Schwierzeck írta:
>> 2013/11/11 Gabor Juhos <juhosg at openwrt.org>:
>>> The relocation code uses the CONFIG_SYS_MONITOR_BASE
>>> constant for calculating the reserved memory size and
>>> relocation offset values. Along with these predefined
>>> values the code also uses several other constants which
>>> are computed by the linker from the CONFIG_SYS_TEXT_BASE
>>> value. Due to this, the relocation code works incorreclty
>>> if the CONFIG_SYS_TEXT_BASE and CONFIG_SYS_MONITOR_BASE
>>> values are different.
>>
>> to be consistent with all other architectures, we should keep
>> CONFIG_SYS_MONITOR_BASE. I think it is neither valid nor intentional
>> to use a value different from CONFIG_SYS_TEXT_BASE.
>
> If it is neither valid nor intentional, the CONFIG_SYS_MONITOR_BASE constant
> should not exist at all and CONFIG_SYS_TEXT_BASE should be used everywhere
> instead IMHO.

By now it is redundant. Once CONFIG_SYS_TEXT_BASE was only a make
variable named TEXT_BASE and defined in board-specific config.mk. This
 has been converted with commit
14d0a02a168b36e87665b8d7f42fa3e88263d26d.

BTW the README states:

- CONFIG_SYS_MONITOR_BASE:
Physical start address of boot monitor code (set by
make config files to be same as the text base address
(CONFIG_SYS_TEXT_BASE) used when linking) - same as
CONFIG_SYS_FLASH_BASE when booting from flash.

>
> Additionally, we have this check in arch/mips/lib/board.c:
>
>> #if CONFIG_SYS_MONITOR_BASE == CONFIG_SYS_FLASH_BASE
>>       bd->bi_flashoffset = monitor_flash_len; /* reserved area for U-Boot */
>> #else
>>       bd->bi_flashoffset = 0;
>> #endif
>
> If it is not allowed to use different values for the two constants,
> the condition and the #else branch should be removed.

no that is still needed if a board has NOR flash but boots from NAND
or SPI flash or other media (e.g. SoC evaluation boards). In that case
CONFIG_SYS_TEXT_BASE points usually to an SRAM address.

>
>> Instead we should change include/configs/malta.h:
>>
>> -#define CONFIG_SYS_MONITOR_BASE                CONFIG_SYS_FLASH_BASE
>> +#define CONFIG_SYS_MONITOR_BASE                CONFIG_SYS_TEXT_BASE
>>
>>
>> Comments?
>
> I have tried this already. It is working as well, however with this change the
> flash sectors containing the bootloader are not protected correctly:
>
>> malta # flinfo
>>
>> Bank # 1: CFI conformant flash (32 x 32)  Size: 4 MB in 64 Sectors
>>   Intel Extended command set, Manufacturer ID: 0x00, Device ID: 0x00
>>   Erase timeout: 16384 ms, write timeout: 3 ms
>>   Buffer write timeout: 3 ms, buffer size: 2048 bytes
>>
>>   Sector Start Addresses:
>>   BE000000        BE010000        BE020000        BE030000        BE040000
>>   BE050000        BE060000        BE070000        BE080000        BE090000
>>   BE0A0000        BE0B0000        BE0C0000        BE0D0000        BE0E0000
>>   BE0F0000        BE100000        BE110000        BE120000        BE130000
>>   BE140000        BE150000        BE160000        BE170000        BE180000
>>   BE190000        BE1A0000        BE1B0000        BE1C0000        BE1D0000
>>   BE1E0000        BE1F0000        BE200000        BE210000        BE220000
>>   BE230000        BE240000        BE250000        BE260000        BE270000
>>   BE280000        BE290000        BE2A0000        BE2B0000        BE2C0000
>>   BE2D0000        BE2E0000        BE2F0000        BE300000        BE310000
>>   BE320000        BE330000        BE340000        BE350000        BE360000
>>   BE370000        BE380000        BE390000        BE3A0000        BE3B0000
>>   BE3C0000        BE3D0000        BE3E0000   RO   BE3F0000   RO
>> malta #
>
> For reference, this is the output of flinfo with my change:
>
>> malta # flinfo
>>
>> Bank # 1: CFI conformant flash (32 x 32)  Size: 4 MB in 64 Sectors
>>   Intel Extended command set, Manufacturer ID: 0x00, Device ID: 0x00
>>   Erase timeout: 16384 ms, write timeout: 3 ms
>>   Buffer write timeout: 3 ms, buffer size: 2048 bytes
>>
>>   Sector Start Addresses:
>>   BE000000   RO   BE010000   RO   BE020000   RO   BE030000        BE040000
>>   BE050000        BE060000        BE070000        BE080000        BE090000
>>   BE0A0000        BE0B0000        BE0C0000        BE0D0000        BE0E0000
>>   BE0F0000        BE100000        BE110000        BE120000        BE130000
>>   BE140000        BE150000        BE160000        BE170000        BE180000
>>   BE190000        BE1A0000        BE1B0000        BE1C0000        BE1D0000
>>   BE1E0000        BE1F0000        BE200000        BE210000        BE220000
>>   BE230000        BE240000        BE250000        BE260000        BE270000
>>   BE280000        BE290000        BE2A0000        BE2B0000        BE2C0000
>>   BE2D0000        BE2E0000        BE2F0000        BE300000        BE310000
>>   BE320000        BE330000        BE340000        BE350000        BE360000
>>   BE370000        BE380000        BE390000        BE3A0000        BE3B0000
>>   BE3C0000        BE3D0000        BE3E0000   RO   BE3F0000   RO
>> malta #
>
> Any idea how can we resolve this properly?
>
> -Gabor

following seems to work (in both variants with -bios and -pflash)

#define CONFIG_SYS_TEXT_BASE 0xbe000000
#define CONFIG_SYS_MONITOR_BASE CONFIG_SYS_TEXT_BASE

-- 
- Daniel