Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cpu/esp*: fix linker scripts #15186

Merged
merged 3 commits into from
Oct 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions cpu/esp32/ld/esp32.common.ld
Original file line number Diff line number Diff line change
Expand Up @@ -116,25 +116,25 @@ SECTIONS
*libc.a:*(.literal .text .literal.* .text.*)

/* Xtensa basic functionality written in assembler should be placed in iram */
*xtensa.a:*(.literal .text .literal.* .text.*)
*xtensa/*(.literal .text .literal.* .text.*)
/* ESP-IDF parts that have to run in IRAM */
*esp_idf_heap.a:*(.literal .text .literal.* .text.*)
*esp_idf_spi_flash.a:*(.literal .text .literal.* .text.*)
*esp_idf_heap/*(.literal .text .literal.* .text.*)
*esp_idf_spi_flash/*(.literal .text .literal.* .text.*)
/* parts of RIOT that should to run in IRAM */
*core.a:*(.literal .text .literal.* .text.*)
*littlefs.a:*(.literal .text .literal.* .text.*)
*littlefs2.a:*(.literal .text .literal.* .text.*)
*newlib_syscalls_default.a:*(.literal .text .literal.* .text.*)
*spiffs_fs.a:*(.literal .text .literal.* .text.*)
*spiffs.a:*(.literal .text .literal.* .text.*)
*core/*(.literal .text .literal.* .text.*)
*littlefs/*(.literal .text .literal.* .text.*)
*littlefs2/*(.literal .text .literal.* .text.*)
*newlib_syscalls_default/*(.literal .text .literal.* .text.*)
*spiffs_fs/*(.literal .text .literal.* .text.*)
*spiffs/*(.literal .text .literal.* .text.*)
*syscalls.o(.literal .text .literal.* .text.*)
*vfs.a:*(.literal .text .literal.* .text.*)
*vfs/*(.literal .text .literal.* .text.*)

/* part of the RIOT port that should run in IRAM */
*cpu.a:*(.literal .text .literal.* .text.*)
*esp_common.a:*(.literal .text .literal.* .text.*)
*periph.a:*(.literal .text .literal.* .text.*)
*mtd.a:**(.literal .text .literal.* .text.*)
*cpu/*(.literal .text .literal.* .text.*)
*esp_common/*(.literal .text .literal.* .text.*)
*periph/*(.literal .text .literal.* .text.*)
*mtd/**(.literal .text .literal.* .text.*)

INCLUDE esp32.spiram.rom-functions-iram.ld
_iram_text_end = ABSOLUTE(.);
Expand Down
24 changes: 12 additions & 12 deletions cpu/esp8266/ld/esp8266.riot-os.ld
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ SECTIONS
/* TODO put only necessary .rodata to dram */
/* *(.rodata .rodata.*) */
*libc.a:*.o(.rodata.* .rodata)
*core.a:*(.rodata.* .rodata)
*cpu.a:*(.rodata .rodata.*)
*core/*(.rodata.* .rodata)
*cpu/*(.rodata .rodata.*)
*libpp.a:(.rodata.* .rodata)
*liblog.a:(.rodata.* .rodata)

Expand Down Expand Up @@ -222,24 +222,24 @@ SECTIONS
*(.literal .text)
*core.a:*(.literal .text .literal.* .text.*)
*/
*gdbstub.a:*(.literal .text .literal.* .text.*)
*gdbstub/*(.literal .text .literal.* .text.*)
*(.stub .gnu.warning .gnu.linkonce.literal.* .gnu.linkonce.t.*.literal .gnu.linkonce.t.*)
/* RIOT-OS compiled source files that use the .iram1.* section names for IRAM
functions, etc. */
*(.iram1 .iram1.*)

/* SDK libraries that expect their .text or .data sections to link to iram */
/* TODO *libcore.a:(.bss .data .bss.* .data.* COMMON) */
*esp_idf_spi_flash.a:spi_flash_raw.o(.literal .text .literal.* .text.*)
*esp_idf_esp8266.a:ets_printf.o(.literal .text .literal.* .text.*)
*esp_idf_spi_flash/spi_flash_raw.o(.literal .text .literal.* .text.*)
*esp_idf_esp8266/ets_printf.o(.literal .text .literal.* .text.*)
/*
*cpu.a:*.o(.literal .text .literal.* .text.*)
*/
*core.a:sched.o(.literal .text .literal.* .text.*)
*esp_wifi.a:*(.literal .text .literal.* .text.*)
*freertos.a:*(.literal .text .literal.* .text.*)
*periph.a:*(.literal .text .literal.* .text.*)
*xtimer.a:*(.literal .text .literal.* .text.*)
*core/sched.o(.literal .text .literal.* .text.*)
*esp_wifi/*(.literal .text .literal.* .text.*)
*freertos/*(.literal .text .literal.* .text.*)
*periph/*(.literal .text .literal.* .text.*)
*xtimer/*(.literal .text .literal.* .text.*)

*libhal.a:clock.o(.literal .text .literal.* .text.*)
*libhal.a:int_asm--set_intclear.o(.literal .text .literal.* .text.*)
Expand All @@ -254,7 +254,7 @@ SECTIONS
*libphy.a:phy_sleep.o(.literal .text .literal.* .text.*)

/* Xtensa basic functionality written in assembler should be placed in iram */
*xtensa.a:*(.literal .text .literal.* .text.*)
*xtensa/*(.literal .text .literal.* .text.*)

/* libgcc functions required for debugging have to be in IRAM */
*libgcc.a:unwind-dw2.o(.literal .text .literal.* .text.*)
Expand Down Expand Up @@ -286,7 +286,7 @@ SECTIONS
*libc.a:*fputwc.o(.literal .text .literal.* .text.*)
*/

enc28j60.a:*(.literal .text .literal.* .text.*)
*enc28j60/*(.literal .text .literal.* .text.*)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's with this random driver getting special treatment 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really sure, maybe @gschorcht can clarify?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, yes I can clarify. In section Network Devices of the ESP8266 documentation it is mentioned that ESP8266 is working and has been tested with ENC28J60 and MRF24J40. However, the ENC28J60 works only if the driver code is placed in IRAM.

Since the IRAM of the ESP8266 is very limited and only has a size of 48 kByte, it is not possible to place code segments in the IRAM by default. The IROM of the ESP8266 on the other hand has 448 kByte. The IRAM is of course much faster, while the IROM is much slower and can only be accessed via a cache, which is also sometimes disabled, e.g. by the WiFi module or when writing to the flash.

Therefore, time-critical code as well as code that has to work even when the cache is disabled must be placed in the IRAM. Due to the limited size of the IRAM, fine-tuning is always required as to what must be placed in the IRAM and what can be placed in the IROM.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IROM is much slower and can only be accessed via a cache, which is also sometimes disabled, e.g. by the WiFi module

The WiFi module disabling the cache is surprising! Wouldn't that mean that any code can be hit by the problem if another task gets scheduled while the WiFi driver has the cache disabled? Or are interrupts disabled together with the cache?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benpicco I'm not really sure (I just forgot too much in last 2 years doing other things 😟), but when I remember correctly, I had problems to access IROM when the WiFi module was active. So my guess at that time was (without knowing it definitely, there is no documentation) that it might be that the WiFi module is connected via SPI inside the SoC. That's why esp_wifi is already in IRAM. I could try it again.


*(.fini.literal)
*(.fini)
Expand Down
3 changes: 0 additions & 3 deletions tests/mtd_mapper/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,4 @@ include ../Makefile.tests_common
USEMODULE += mtd_mapper
USEMODULE += embunit

# disabling due to unknown failure. See #15123.
TEST_ON_CI_BLACKLIST += esp32-wroom-32

include $(RIOTBASE)/Makefile.include
3 changes: 0 additions & 3 deletions tests/pkg_littlefs/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,4 @@ include ../Makefile.tests_common
USEMODULE += littlefs
USEMODULE += embunit

# disabling due to unknown failure. See #15123.
TEST_ON_CI_BLACKLIST += esp32-wroom-32

include $(RIOTBASE)/Makefile.include
3 changes: 0 additions & 3 deletions tests/pkg_littlefs2/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,4 @@ include ../Makefile.tests_common
USEPKG += littlefs2
USEMODULE += embunit

# disabling due to unknown failure. See #15123.
TEST_ON_CI_BLACKLIST += esp32-wroom-32

include $(RIOTBASE)/Makefile.include
3 changes: 0 additions & 3 deletions tests/pkg_spiffs/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,4 @@ include ../Makefile.tests_common
USEMODULE += spiffs
USEMODULE += embunit

# disabling due to unknown failure. See #15123.
TEST_ON_CI_BLACKLIST += esp32-wroom-32

include $(RIOTBASE)/Makefile.include