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

Reimplemented esp_init_data_default. #1526

Closed
wants to merge 1 commit into from

Conversation

jmattsson
Copy link
Member

@jmattsson jmattsson commented Oct 4, 2016

Fixes #1500.

  • This PR is for the dev branch rather than for master.
  • This PR is compliant with the other contributing guidelines as well (if not, please describe why).
  • I have thoroughly tested my contribution.
  • The code changes are reflected in the documentation at docs/en/*.
Reimplemented esp_init_data_default to work around the pesky "rf_cal[0] !=0x05" hang when booting on a chip which doesn't have esp_init_data written to it.

It is no longer possible to do the writing of the esp_init_data_default
from within nodemcu_init(), as the SDK now hangs long before it gets
there. As such, I've had to reimplement this in our user_start_trampoline
and get it all done before the SDK has a chance to look for the init data.
It's unfortunate that we have to spend IRAM on this, but I see no better
alternative at this point.

With this patch, NodeMCU should finally be back to "it just works" when flashed onto a fresh(ly erased) ESP, and the previously documented approach should be correct once more.

I'd really appreciate it if a few more people could try this PR before considering merging - while it's working fine for me, I'm always a bit hesitant messing with the early boot stuff.

To work around the pesky "rf_cal[0] !=0x05" hang when booting on a chip
which doesn't have esp_init_data written to it.

It is no longer possible to do the writing of the esp_init_data_default
from within nodemcu_init(), as the SDK now hangs long before it gets
there.  As such, I've had to reimplement this in our user_start_trampoline
and get it all done before the SDK has a chance to look for the init data.
It's unfortunate that we have to spend IRAM on this, but I see no better
alternative at this point.
@jimparis
Copy link
Contributor

jimparis commented Oct 4, 2016

Cool, I didn't realize we were able to run code before the SDK error. Definitely a good solution!

I tested 5 cases:

  1. Erase, flash as 2MB chip and flash init data to 0x1fc000 -- works
  2. Erase, flash as 4MB chip and flash init data to 0x3fc000 -- works
  3. Erase, flash as 2MB chip, don't flash init data -- works
  4. Erase, flash as 4MB chip, don't flash init data -- works
  5. No erase after test 4, Flash as 2MB chip, don't flash init data -- works

👍

// Note: ideally we should generate the actual init data from the
// SDK's bin/esp_init_data_default.bin during the build, and #include it
// straight into this array. E.g.:
// xxd -i esp_init_data_default | tail -n13 | head -n11 > eidd.h
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason that you didn't do this?

Copy link
Member Author

Choose a reason for hiding this comment

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

@pjsg Time.

Copy link
Contributor

Choose a reason for hiding this comment

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

See #1527

@pjsg
Copy link
Member

pjsg commented Oct 4, 2016

I tried this, and it worked in my case.....

// SDK's bin/esp_init_data_default.bin during the build, and #include it
// straight into this array. E.g.:
// xxd -i esp_init_data_default | tail -n13 | head -n11 > eidd.h
static uint8_t flash_init_data[128] __attribute__((aligned(4))) =
Copy link
Member

Choose a reason for hiding this comment

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

If this is indeed what I think it is then it's the same I complained about in #1500. The init data bytes shouldn't be hardcoded (will soon be outdated) but read from the SDK which is downloaded during the build. @vowstar fixed this in his PR with 7f3e4b8.

Copy link
Contributor

Choose a reason for hiding this comment

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

See #1527

@marcelstoer
Copy link
Member

I'm currently on the road and can't test but did one of you verify if the "old firmware" (0.9.x, 1.4, etc.) -> 1.5.4.1 upgrade runs smoothly? IIRC there were some fundamental init data changes between 1.5.1 and 1.5.4.1.

@jimparis
Copy link
Contributor

jimparis commented Oct 4, 2016

Upgrading from old firmware would still require an erase first. This code only covers the case where the beginning of the init data is blank. I don't think you can do anything more generic without running the risk of overwriting a user's custom init data.

@jmattsson
Copy link
Member Author

This PR is superseded by @jimparis' #1527, closing.

@jmattsson jmattsson closed this Oct 5, 2016
@jmattsson
Copy link
Member Author

@marcelstoer Upgrade from 1.5.1 to 1.5.4.1 worked fine here.

Tested:

  1. Full chip erase.
  2. Flashed NodeMCU 1.5.1
  3. Let it boot.
  4. Flashed this branch.
  5. Booted fine, it just had to reformat the filesystem.

@marcelstoer
Copy link
Member

running the risk of overwriting a user's custom init data

Can we reliably determine the size (i.e. length) of the init data block on the device? If so then we can compare it against the SDKs init block size and still overwrite if they don't match.

@jmattsson
Copy link
Member Author

No, the init data block is always 128 bytes, but there's no (sane) way to differentiate between a custom init block and an old SDK one.

If we had heaps of IRAM we could keep copies of all previous SDK defaults and compare against each, but we don't, so i think #1527 is as good as we're going to get at this point.

@marcelstoer
Copy link
Member

That would indeed be a waste of space if all we care about is whether the defaults were changed or not. So, if we could guard all functions that alter init data we'd only need a single bit of IRAM.

@jmattsson
Copy link
Member Author

To my knowledge there are no functions which alter the init data (other than our initial installation thereof). Custom init data typically gets flashed at the same time (or soon after) the NodeMCU image is flashed to the chip, and there is no way for us to detect that short of a full comparison. And even then, if the user explicitly wants/needs the values from e.g. SDK 1.4.0, we wouldn't be able to tell that intent apart from someone who just upgraded from a 1.4.0 build and would like to have the init data updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants