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

Init phy data to default if invalid in flash partition to avoid bootloops (IDFGH-4812) #6610

Closed

Conversation

0xFEEDC0DE64
Copy link
Contributor

@0xFEEDC0DE64 0xFEEDC0DE64 commented Feb 25, 2021

I just had a case of endless bootloop crashes because I enabled the menuconfig option to store PHY init data in a dedicated partition instead of NVS.

This happens because esp_phy_load_cal_and_init() calls abort() when esp_phy_get_init_data() returns a nullptr. I added a menuconfig option to reset invalid PHY init data to default when it cannot be verified successfully to avoid such bootloops.

Also I think I fixed a few memory leaks on the way be deleting the pointer when it was returning NULL;

I (1553) phy_init: phy_version 4660,0162888,Dec 23 2020
E (1563) phy_init: failed to validate PHY data partition
E (1563) phy_init: failed to obtain PHY init data

abort() was called at PC 0x4014029f on core 0
0x4014029f: esp_phy_load_cal_and_init at <project-home>/build_hw4/../esp-idf/components/esp_wifi/src/phy_init.c:546 (discriminator 3)

Backtrace:0x40087ac6:0x3ffd4780 0x4008841d:0x3ffd47a0 0x400901d2:0x3ffd47c0 0x4014029f:0x3ffd4830 0x40140411:0x3ffd4860 0x4018e463:0x3ffd4880 0x4018e78b:0x3ffd48a0 0x4018b4fd:0x3ffd48c0 0x40091d4a:0x3ffd48e0
0x40087ac6: panic_abort at <project-home>/build_hw4/../esp-idf/components/esp_system/panic.c:367
0x4008841d: esp_system_abort at <project-home>/build_hw4/../esp-idf/components/esp_system/system_api.c:112
0x400901d2: abort at <project-home>/build_hw4/../esp-idf/components/newlib/abort.c:46
0x4014029f: esp_phy_load_cal_and_init at <project-home>/build_hw4/../esp-idf/components/esp_wifi/src/phy_init.c:546 (discriminator 3)
0x40140411: esp_phy_enable at <project-home>/build_hw4/../esp-idf/components/esp_wifi/src/phy_init.c:215
0x4018e463: wifi_hw_start at ??:?
ELF file SHA256: 4366c305673513cc

I (1400) esp_core_dump_flash: Save core dump to flash...
I (1406) esp_core_dump_flash: Erase flash 20480 bytes @ 0x961000
I (1681) esp_core_dump_flash: Write end offset 0x4900, check sum length 32
I (1681) esp_core_dump_flash: Core dump has been saved to flash.
Rebooting...

@github-actions github-actions bot changed the title Init phy data to default if invalid in flash partition to avoid bootloops Init phy data to default if invalid in flash partition to avoid bootloops (IDFGH-4812) Feb 25, 2021
@Alvin1Zhang
Copy link
Collaborator

Thanks for your contribution.

@HarveyRong-Esp
Copy link
Contributor

Hi @0xFEEDC0DE64, What is your esp-idf commit ID?

@0xFEEDC0DE64
Copy link
Contributor Author

Hi @HarveyRong-Esp, I don't get the question? the git hash of the branch? i branched off the 4.4 release tag

@igrr
Copy link
Member

igrr commented Mar 1, 2021

@0xFEEDC0DE64 FWIW, there is no v4.4 release tag yet, as we have just started working on this version. The latest release is v4.2 and latest pre-release is v4.3-beta1. There is a v4.4-dev tag in the repository which indicates the point when 4.4 development starts, but it is not related to a release.

(this likely doesn't affect the issue you are seeing, clarifying this just in case)

@igrr
Copy link
Member

igrr commented Mar 1, 2021

On the topic of your issue, didn't idf.py flash flash the PHY init data partition after you have enabled that option?

@0xFEEDC0DE64
Copy link
Contributor Author

I enabled secure boot and flash encryption on development mode, maybe the phy data didn't flash automatically when using encrypted-flash ? If just flashing the phy was missing in my case maybe my change is not needed at all :)

@0xFEEDC0DE64
Copy link
Contributor Author

Do you think it is ok to have a crash if the phy partition ever corrupts somehow? I still think it should at least ignore invalid data or reset it to default somehow.

@espressif-bot espressif-bot added the Status: Done Issue is done internally label Mar 31, 2021
espressif-bot pushed a commit that referenced this pull request Apr 1, 2021
…oops

Signed-off-by: ronghulin <ronghulin@espressif.com>

Merges #6610
@Alvin1Zhang
Copy link
Collaborator

Changes merged with 63a7a84, thanks for contribution again.

@Alvin1Zhang Alvin1Zhang closed this Apr 2, 2021
espressif-bot pushed a commit that referenced this pull request Feb 14, 2022
…oops

Signed-off-by: ronghulin <ronghulin@espressif.com>

Merges #6610
espressif-bot pushed a commit that referenced this pull request Aug 1, 2022
…oops

Signed-off-by: ronghulin <ronghulin@espressif.com>

Merges #6610
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Done Issue is done internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants