From: Gabor Juhos <juhosg@openwrt.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] MIPS: use CONFIG_SYS_TEXT_BASE in relocation calculations
Date: Tue, 12 Nov 2013 15:54:34 +0100 [thread overview]
Message-ID: <5282412A.8030505@openwrt.org> (raw)
In-Reply-To: <CACUy__WNqyYZ4TLTeSqnAuGeOFARkLJh1Px6OVbOrZ8JMitBsg@mail.gmail.com>
2013.11.11. 23:36 keltez?ssel, Daniel Schwierzeck ?rta:
<...>
>>> 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.
I see. Thank you for the explanation.
>>
>> 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.
Ok.
>
>>
>>> 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
Yeah, this definitely looks better than my solution. :)
Do you want me to incorporate this into the 'malta: use unmapped flash base
address' patch, or do you want to apply this separately?
-Gabor
>
next prev parent reply other threads:[~2013-11-12 14:54 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-11 19:02 [U-Boot] [PATCH] MIPS: use CONFIG_SYS_TEXT_BASE in relocation calculations Gabor Juhos
2013-11-11 19:15 ` Daniel Schwierzeck
2013-11-11 19:36 ` Daniel Schwierzeck
2013-11-11 20:31 ` Gabor Juhos
2013-11-11 22:36 ` Daniel Schwierzeck
2013-11-12 14:54 ` Gabor Juhos [this message]
2013-11-12 15:40 ` Daniel Schwierzeck
2013-11-12 15:47 ` Gabor Juhos
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=5282412A.8030505@openwrt.org \
--to=juhosg@openwrt.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox