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

boards/esp32: changes the approach for peripheral configurations in ESP32 board definitions #10644

Closed
wants to merge 20 commits into from

Conversation

gschorcht
Copy link
Contributor

Contribution description

This PR changes the approach of peripheral configurations for ADC, DAC, I2C, SPI and UART in board definitions to the usual RIOT approach. With these changes, peripheral configurations use static const arrays in the boards/esp32*/periph_conf.h files and define the *_NUMOF macros using the size of these static array.

The static configuration arrays contain only definitions that can be changed by the board definition or the application. They do not contain any MCU implementation detail. The board definitions use preprocessor defines as before to fill these static configuration arrays. This makes it possible to override all configurations either with the make command or application specific configuration files.

Testing procedure

Compilation and test with the most common ESP32 board should be executed

make BOARD=esp32-wroom-32 -C tests/periph_adc flash test
make BOARD=esp32-wroom-32 -C tests/periph_dac flash test
make BOARD=esp32-wroom-32 -C tests/periph_i2c flash test
make BOARD=esp32-wroom-32 -C tests/periph_pwm flash test
make BOARD=esp32-wroom-32 -C tests/periph_spi flash test
make BOARD=esp32-wroom-32 -C tests/periph_uart flash test

In addition to the tests above, the PR has been already tested with tests/driver_lis3dh and SPI as well as tests/driver_lis3mdl and I2C.

Issues/PRs references

This PR contains also the changes already provided by PRs #10643 and #10641.

@gschorcht gschorcht changed the title boards/esp32: changes the approach for peripheral configuration in ESP32 board definitions boards/esp32: changes the approach for peripheral configurations in ESP32 board definitions Dec 20, 2018
@gschorcht gschorcht added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: boards Area: Board ports Area: cpu Area: CPU/MCU ports labels Dec 20, 2018
@gschorcht gschorcht force-pushed the esp32_boards_periph_conf branch 2 times, most recently from b15a206 to 99934ea Compare March 25, 2019 09:10
@gschorcht
Copy link
Contributor Author

I have rebased the PR to current master. Since the master changed a lot since I opened this PR 3 months ago, I had to spent some effort to fix merge conflicts. It would be great, if we could make some progress with this PR before I start to change the ESP32 code again a lot for code deduplication with the new implementation of ESP8266 in PR #11108.

@leandrolanzieri
Copy link
Contributor

@gschorcht Do you think it could be possible to split this into separate PRs for every peripheral, I think that would make it easier to test and review.

@gschorcht
Copy link
Contributor Author

gschorcht commented Mar 27, 2019

@leandrolanzieri

Do you think it could be possible to split this into separate PRs for every peripheral, I think that would make it easier to test and review.

Sure, could be possible. I placed all of them in one PR since they are related and to be sure that they are merged all together.

Default definition of gpio_t has to be overwritten since gpio_t is required in this header file to be able to define ADC, DAC, I2C, PWM and SPI configurations.
ADC and DAC pins are now configured using static arrays in header files instead of static variables in implementation to be able to define ADC_NUMOF and DAC_NUM using the size of these arrays instead of a variable.
ADC and DAC pins are now configured using static arrays in header files instead of static variables in implementation to be able to define ADC_NUMOF and DAC_NUM using the size of these arrays instead of a variable.
I2C devices are now configured using static array in header files instead of static variables in implementation to be able to define I2C_NUMOF using the size of the array instead of a variable.
I2C devices are now configured using static array in header files instead of static variables in implementation to be able to define I2C_NUMOF using the size of the array instead of a variable.
SPI devices are now configured using static array in header files instead of static variables in implementation to be able to define SPI_NUMOF using the size of the array instead of a variable.
SPI devices are now configured using static array in header files instead of static variables in implementation to be able to define SPI_NUMOF using the size of the array instead of a variable.
UART devices are now configured using static array in header files instead of static variables in implementation to be able to define UART_NUMOF using the size of the array instead of a variable.
UART devices are now configured using static array in header files instead of static variables in implementation to be able to define UART_NUMOF using the size of the array instead of a variable.
PWM channels are now configured using static array in header files instead of static variables in implementation.
PWM channels are now configured using static array in header files instead of static variables in implementation.
Static configuration array is used in hardware implementation for pins instead of having additional members in the hardware configuration structure. Hardware configuration structure is now const. In the software implementation, the configuration structure has to be used for printing the configuration.
Parameter bus is renamed to dev to
@gschorcht gschorcht force-pushed the esp32_boards_periph_conf branch from 99934ea to 76e0357 Compare March 27, 2019 07:24
@leandrolanzieri
Copy link
Contributor

@leandrolanzieri

Do you think it could be possible to split this into separate PRs for every peripheral, I think that would make it easier to test and review.

Sure, could be possible. I placed all of them in one PR since they are related and to be sure that they are merged all together.

Great!

@gschorcht
Copy link
Contributor Author

The PR is now splitted into PRs #11289 #11290 #11291 #11292 #11293 #11294. Since they are all releated, they should be merged all together to avoid that there are inconsistent peripheral configuration approaches.

@gschorcht
Copy link
Contributor Author

The PR is replaced by splitted PRs #11289 #11290 #11291 #11292 #11293 #11294.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: cpu Area: CPU/MCU ports Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants