-
Notifications
You must be signed in to change notification settings - Fork 62
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
Initial Infineon I2C TPM support for Espressif ESP32 #351
Conversation
IDE/Espressif/include.am
Outdated
EXTRA_DIST+= IDE\Espressif\main\main.c | ||
EXTRA_DIST+= IDE\Espressif\main\wrap_test.c | ||
EXTRA_DIST+= IDE\Espressif\main\include\main.h | ||
EXTRA_DIST+= IDE\Espressif\main\include\wrap_test.h |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing from repo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch, but not intended to be included: wrap_test
was an early demo that I abandoned in favor of native test
.
I've removed those lines from include.am
.
IDE/Espressif/include.am
Outdated
# The example application. | ||
EXTRA_DIST+= IDE\Espressif\main\CMakeLists.txt | ||
EXTRA_DIST+= IDE\Espressif\main\main.c | ||
EXTRA_DIST+= IDE\Espressif\main\wrap_test.c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing from repo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as prior one. good catch, but not intended to be included: wrap_test
was an early demo that I abandoned in favor of native_test
.
I've removed those lines from include.am.
EXTRA_DIST+= IDE\Espressif\components\wolftpm\CMakeLists.txt | ||
EXTRA_DIST+= IDE\Espressif\components\wolftpm\include\options.h | ||
|
||
# The example application. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to add IDE/Espressif/main/Kconfig.projbuild here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, thanks for that one.
IDE/Espressif/include.am
Outdated
|
||
# wolfSSL source code is not included here and must be available in separate directory. | ||
EXTRA_DIST+= IDE\Espressif\components\wolfssl\CMakeLists.txt | ||
EXTRA_DIST+= IDE\Espressif\components\wolfssl\include\config.h |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing from repo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, good catch. this one was a bit more tricky as config.h
files are excluded in .gitignore
EXTRA_DIST+= IDE\Espressif\partitions_singleapp_large.csv | ||
EXTRA_DIST+= IDE\Espressif\README.md | ||
|
||
# wolfSSL source code is not included here and must be available in separate directory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to add IDE/Espressif/components/wolftpm/CMakeLists.txt and IDE/Espressif/components/wolftpm/include/README.md?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup. also missing. thank you.
@@ -0,0 +1,23 @@ | |||
# vim:ft=automake | |||
# included from Top Level Makefile.am | |||
# All paths should be given relative to the root |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to add IDE/Espressif/VisualGDB/wolfssl_IDF_v5.2_ESP32.vgdbproj ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, that was missing. too.
root cause of the pesky git actions error was an off-by-one keypress. I inadvertently had a $
for a comment in the new include.am
instead of the adjacent key #
.
hal/tpm_io_espressif.c
Outdated
|
||
/* I2C master i2c port number, | ||
* the number of i2c peripheral interfaces available will depend on the chip */ | ||
#define I2C_MASTER_NUM 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be overridable at build time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's a nice feature to add. I also added some range checking.
I have plans for Kconfig
in a future PR.
wolftpm/tpm2_types.h
Outdated
@@ -452,6 +452,8 @@ typedef int64_t INT64; | |||
#ifdef WIN32 | |||
#include <windows.h> | |||
#define XSLEEP_MS(ms) Sleep(ms) | |||
#elif defined(WOLFSSL_ESPIDF) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would prefer #elif defined(FREERTOS)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed using FREERTOS
here is better. Updated.
This is the initial I2C TPM support for the Infineon Optiga 9673 TPM2 on the Espressif ESP32.
Note there's a newer I2C implementation noted in the latest Espressif documentation. The PR does not use that library yet, but instead utilizes the older library used in the Espressif v5.2 I2C examples which is compatible with a wider range of ESP-IDF versions.
The included example directly references the /examples/native/native_test.c to demonstrate using the TPM on the ESP32.
Support for the newer I2C library as well as SPI will be submitted in separate future pull requests.
This library is expected to work with all flavors of the ESP32-[C2/C3/C6/S2S6, etc.], but has only been tested on the basic Xtensa ESP32 at this time.
Edit
This PR as-is appears to be working reliably. For future reference:
After I submitted this PR, I bought a few LetsTrust TPM for Raspberry Pi devices. The folks at https://letstrust.de/ reached out to me today with an interesting question & comment:
I did not explicitly enable any clock stretching.
See the api-reference/peripherals/i2c docs and espressif/esp-idf#4173 - in particular the comment from
costaud
:and this one from
stickbreaker
:The ESP32 for this PR is the
master
I2C device, talking to the Infineon 9673.Although I did not implement any clock stretching in this PR, there are some adjustable delays between write-then-read, and after retries:
Adding some details on my
sdkconfig.h
... all the defaults resulting in these settings that may be of interest:See also I2C clock is not generate reliably after clock stretch