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

feat(esp32s3) add support for esp32s3 #72

Merged
merged 5 commits into from
Apr 21, 2024

Conversation

kerms
Copy link
Contributor

@kerms kerms commented Apr 19, 2024

Tested on Windows Keil uVision 5.38.0.

ESP32S3-WROOM-1-N16R8
Target: stm32f103c8t8
Port used: SWD

@windowsair
Copy link
Owner

Thanks for your contribution! I will review your code later :)

#elif defined CONFIG_IDF_TARGET_ESP32S3
__STATIC_INLINE void PORT_JTAG_SETUP(void)
{
// set TCK, TMS pin
Copy link
Owner

Choose a reason for hiding this comment

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

not use tab

README_CN.md Outdated
Copy link
Owner

Choose a reason for hiding this comment

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

encoding or crlf diff ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mine is lf, and it was crlf

@@ -30,6 +30,8 @@
#define DAP_SPI SPI2
#elif defined CONFIG_IDF_TARGET_ESP32C3
#define DAP_SPI GPSPI2
#elif defined CONFIG_IDF_TARGET_ESP32S3
#define DAP_SPI GPSPI2
Copy link
Owner

Choose a reason for hiding this comment

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

not use tab

@@ -18,6 +18,7 @@
#include "components/DAP/include/cmsis_compiler.h"
#include "components/DAP/include/spi_switch.h"
#include "components/DAP/include/gpio_common.h"
#include "DAP_config.h"
Copy link
Owner

Choose a reason for hiding this comment

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

Is it necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used the definitions like PIN_SWDIO_MOSI from DAP_config.h, you are right that IOMUX pins are fixed anyway, it will cause error if user change the pin from DAP_config.h. I will change that.

#elif defined CONFIG_IDF_TARGET_ESP32S3
void DAP_SPI_Init()
{
periph_ll_enable_clk_clear_rst(PERIPH_SPI2_MODULE);
Copy link
Owner

Choose a reason for hiding this comment

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

not use tab

@@ -23,6 +23,8 @@
#define PIN_LED_WIFI_STATUS 27
#elif defined CONFIG_IDF_TARGET_ESP32C3
#define PIN_LED_WIFI_STATUS 10
#elif defined CONFIG_IDF_TARGET_ESP32S3
#define PIN_LED_WIFI_STATUS 4
Copy link
Owner

Choose a reason for hiding this comment

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

not use tab

@windowsair
Copy link
Owner

Looks really good.

I have a few comments related to coding style. These are hopefully minor things that should be relatively trivial to change. You can try to change them. If you don't feel like it, I might see if I can hijack this branch and make these changes for you.

Thanks a lot for your contributions to this project.

@kerms
Copy link
Contributor Author

kerms commented Apr 20, 2024

Looks really good.

I have a few comments related to coding style. These are hopefully minor things that should be relatively trivial to change. You can try to change them. If you don't feel like it, I might see if I can hijack this branch and make these changes for you.

Thanks a lot for your contributions to this project.

People who uses spaces are 👿. 😛

@windowsair
Copy link
Owner

It seems that there are no major changes to the esp32s3 spi controller, that's good.

I'll merge this. Thanks again for your contribution to this project.

@windowsair windowsair merged commit 46fa164 into windowsair:master Apr 21, 2024
2 checks passed
@kerms kerms deleted the pr_windowsair branch April 21, 2024 06:57
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.

2 participants