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

Don't warn if chip version is greater than CONFIG_ESP32_REV_MIN (IDFGH-1778) #4000

Closed
ecc1 opened this issue Sep 2, 2019 · 4 comments
Closed

Comments

@ecc1
Copy link

ecc1 commented Sep 2, 2019

The whole point of this configuration variable is that it's a minimum revision. There's no need to warn if the actual version is greater, and indeed "suggest to upgrade it" is not only bad English, it's bad software engineering advice.

https://github.com/espressif/esp-idf/blob/master/components/esp32/cpu_start.c#L411-L418

@github-actions github-actions bot changed the title Don't warn if chip version is greater than CONFIG_ESP32_REV_MIN Don't warn if chip version is greater than CONFIG_ESP32_REV_MIN (IDFGH-1778) Sep 2, 2019
@suda-morris
Copy link
Collaborator

Thanks @ecc1 for your suggestion. We will change it to normal information log ASAP.

@jimmo
Copy link
Contributor

jimmo commented Sep 20, 2019

@ecc1 I thought the idea was that by setting a higher minimum version (i.e. to match your chip revision) then fewer workarounds for old revisions need to be enabled. That sounds like good advice, especially if the workarounds cost performance or code size.

@negativekelvin
Copy link
Contributor

negativekelvin commented Sep 20, 2019

It's a good hint, although better to receive this info at compile time like "This build supports ESP32 REV:0+". Also helpful would be if app binary header had this minimum rev and esptool or OTA would not flash an incompatible binary (@projectgus). Ideally workarounds would be selectively available by compile time config but dynamically enabled at runtime based on detected chip version. Some do have runtime checks, I think.

@igrr
Copy link
Member

igrr commented Sep 20, 2019

Also helpful would be if app binary header had this minimum rev and esptool or OTA would not flash an incompatible binary

You seem to have gained access to our internal repository somehow :) This has been merged recently, should be on Github soon.

@igrr igrr closed this as completed in dd248ff Sep 30, 2019
igrr pushed a commit that referenced this issue Oct 18, 2019
Check chip id and chip revision before boot app image

Closes #4000
igrr pushed a commit that referenced this issue Oct 27, 2019
Check chip id and chip revision before boot app image

Closes #4000
bencbyers pushed a commit to ifit/esp-idf that referenced this issue Apr 28, 2021
Check chip id and chip revision before boot app image

Closes espressif#4000
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

No branches or pull requests

5 participants