esp32: make ESP32_IGNORE_CHIP_REVISION_CHECK a kconfig option

[yamt]

Summary

my esp32-devkitc seems to be the old version, which is rejected by this version check. i don't want to patch the code whenever i test nuttx on the board.

Impact

Testing

[tmedicci]

Hi @yamt , you don't need to patch the firmware in the previous version, you just need to set it in the EXTRAFLAGS argument:

make EXTRAFLAGS="-DESP32_IGNORE_CHIP_REVISION_CHECK"

Particularly, I don't think adding this flag as part of the Kconfig is necessary: versions before ECO3 are not officially supported by Espressif on NuttX. It contains hardware-level limitations that you can check here.

[yamt]

Hi @yamt , you don't need to patch the firmware in the previous version, you just need to set it in the EXTRAFLAGS argument:

make EXTRAFLAGS="-DESP32_IGNORE_CHIP_REVISION_CHECK"

i don't see any reason to prefer it than a kconfig.

Particularly, I don't agree with adding this flag as part of the Kconfig: versions before ECO3 are not officially supported by Espressif on NuttX. It contains hardware-level limitations that you can check here.

which of those limitations are relevant here?

[yamt]

If insisted, this option should at least depends on EXPERIMENTAL config being selected as revisions before v3.0 are not supported in NuttX.

as far as i know, EXPERIMENTAL doesn't mean "not officially supported by a vendor".

[xiaoxiang781216]

If insisted, this option should at least depends on EXPERIMENTAL config being selected as revisions before v3.0 are not supported in NuttX.

as far as i know, EXPERIMENTAL doesn't mean "not officially supported by a vendor".

@yamt , you can do the modification privately, but it is better to follow the best practice for public repo from the maintainer.

[tmedicci]

i don't see any reason to prefer it than a config.

Using an older chip revision must be done by advanced users aware of the associated limitations. There is no problem turning it a config, but it needs to be very clear of the drawbacks.

which of those limitations are relevant here?

Specially:

• When the CPU accesses the external SRAM in a certain sequence, read & write errors may occur;
• When each CPU reads certain different address spaces simultaneously, a read error can occur;
• Due to the flash start-up time, a spurious watchdog reset occurs when ESP32 is powered up or wakes up from Deep-sleep;

as far as i know, EXPERIMENTAL doesn't mean "not officially supported by a vendor".

You are right, it means "not ready to use in production". We would use it to let users know that they are doing such an experiment at their own risk as the software workaround (when possible) are not implemented on NuttX for old chip revisions.

[yamt]

which of those limitations are relevant here?

Specially:

* When the CPU accesses the external SRAM in a certain
  sequence, read & write errors may occur;

* When each CPU reads certain different address spaces simultaneously, a read
  error can occur;

* Due to the flash start-up time, a spurious watchdog reset occurs when ESP32 is
  powered up or wakes up from Deep-sleep;

thank you. then, how about rejecting a boot only when relevant configs are enabled? eg. ESP32_SPIRAM, SMP, WATCHDOG. (i happened to use none of them.)

[tmedicci]

thank you. then, how about rejecting a boot only when relevant configs are enabled? eg. ESP32_SPIRAM, SMP, WATCHDOG. (i happened to use none of them.)

That would require additional efforts and testing to make sure these problems would not interfere with user experience. After all, this would not change the fact that old revisions are not supported in NuttX. Not supported is different from WIP (work-in-progress) and there are no plans to provide these workarounds or the suggested software that enables using old versions. In this case, I recommend keeping with the EXPERIMENTAL dependency.

[yamt]

thank you. then, how about rejecting a boot only when relevant configs are enabled? eg. ESP32_SPIRAM, SMP, WATCHDOG. (i happened to use none of them.)

That would require additional efforts and testing to make sure these problems would not interfere with user experience. After all, this would not change the fact that old revisions are not supported in NuttX. Not supported is different from WIP (work-in-progress) and there are no plans to provide these workarounds or the suggested software that enables using old versions. In this case, I recommend keeping with the EXPERIMENTAL dependency.

well, if you care user experience, let me tell you my honest impression as a user. it was a horrible user experience to see nuttx suddenly started rejecting my board. i suspect it was not only me.

of course, it's fine for a company (espressif in this case) to decide what it supports. but i feel it's a bit wrong for a company to decide what nuttx as a community supports.

as you know, our INVIOLABLES.md says "We should seek to expand the NuttX user base, not to limit it for reasons of preference or priority."

[xiaoxiang781216]

@yamt @tmedicci doesn't reject your patch but suggest guarding by EXPERIMENTAL. It's not bad to give the end user more info.

[yamt]

@yamt @tmedicci doesn't reject your patch but suggest guarding by EXPERIMENTAL. It's not bad to give the end user more info.

i added a help text. i'm not convinced about EXPERIMENTAL.

[tmedicci]

@yamt @tmedicci doesn't reject your patch but suggest guarding by EXPERIMENTAL. It's not bad to give the end user more info.

i added a help text. i'm not convinced about EXPERIMENTAL.

As @xiaoxiang781216 said, I didn't reject your PR, just recommended putting it under EXPERIMENTAL, which defines its actual status: experimental.

When Espressif started supporting ESP32-family on NuttX in 2020, this chip revision was already not recommended for new designs (NRND). We are not preventing the community from implementing the workarounds and the limitations that were implemented on IDF to enable using chip revisions older than v3.0: when these are done (and the limitations properly treated), we can remove the EXPERIMENTAL dependency.

[xiaoxiang781216]

@tmedicci do you think it's fine not depend on EXPERIMENTAL in this patch?

[tmedicci]

@tmedicci do you think it's fine not depend on EXPERIMENTAL in this patch?

Hi @xiaoxiang781216 , from my perspective, it is highly dependent. This chip's status has been NRND for more than 4 years and software workarounds are necessary to make it usable for most of the product applications. That being said, it could only be used for experimentation. That being said, I think it is highly advisable to depend on EXPERIMENTAL to avoid unintended bug reports...

[yamt]

i probably will take a look at what esp-idf does when/if i find a good combination of time and motivation. @tmedicci, are ESP32_REV_xxx configs there the appropriate places to look at?

[tmedicci]

i probably will take a look at what esp-idf does when/if i find a good combination of time and motivation. @tmedicci, are ESP32_REV_xxx configs there the appropriate places to look at?

Yes, exactly!