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

Refactor ESP32xx folder structure #58454

Merged

Conversation

marekmatej
Copy link

This PR aims to refactor Espressif soc folders for all architectures, in order to match current Zephyrs standards and to better reflect ESP32 product line SoC and SiP.

@zephyrbot
Copy link
Collaborator

zephyrbot commented Jun 6, 2023

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
hal_espressif zephyrproject-rtos/hal_espressif@322f638 zephyrproject-rtos/hal_espressif@ae1bd64 (zephyr) zephyrproject-rtos/hal_espressif@322f6389..ae1bd648

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@marekmatej marekmatej force-pushed the feature/espressif_soc_rework branch 5 times, most recently from 6fe0819 to ab1d07c Compare June 11, 2023 21:57
@marekmatej marekmatej force-pushed the feature/espressif_soc_rework branch 4 times, most recently from e7f8903 to 0b91b0b Compare June 12, 2023 12:36
@zephyrbot zephyrbot added the DNM This PR should not be merged (Do Not Merge) label Jun 12, 2023
@marekmatej marekmatej force-pushed the feature/espressif_soc_rework branch 7 times, most recently from ba8dd91 to f968d6d Compare June 13, 2023 16:02
@marekmatej marekmatej force-pushed the feature/espressif_soc_rework branch 7 times, most recently from 511a03e to 227a1b6 Compare June 21, 2023 12:00
@marekmatej marekmatej force-pushed the feature/espressif_soc_rework branch from 7c8ad41 to 10617f2 Compare July 25, 2023 13:47
@MaureenHelm
Copy link
Member

Ok, so those CONFIG_SOC_PART_NUMBER strings were added for downstream users so they can be used to identify the underlying hw. There is no other meaningful usage, except maybe creating the sample to code to use those strings as @carlescufi noted, but that would be very artificial justification. So I'll remove those strings from the PR. Thank you all for your clarification.

I introduced CONFIG_SOC_PART_NUMBER many years ago to help integrate the Kinetis SDK, now called MCUXpresso SDK, into Zephyr. See commit 2815687. The pattern has since been copy-pasted to other SoC families.

@marekmatej
Copy link
Author

@mniestroj, at the end I think we were mixing two different things - SOC_PART_NUMBER strings and bool flags. I was advocating for both! From your messages, I got the impression you wanted to remove both strings and flags. However, I must admit that having those strings for future use is not vindicated enough, so I apologize for the misunderstanding from my side.

Marek Matej added 7 commits July 25, 2023 16:14
Introduce dtsi files representing the
current portfolio of chips and modules
based on the followint criteria:

- flash size
- psram size
- gpio count
- certification status

Update the boards dts files according
to which SOC/SIP they are using.

Signed-off-by: Marek Matej <marek.matej@espressif.com>
Introduce dtsi files representing the
current portfolio of chips and modules
based on the:

- flash size
- psram size
- gpio count
- certification status

Update the boards dts files according
to which SOC/SIP they are using.

Signed-off-by: Marek Matej <marek.matej@espressif.com>
Remove virtual esp32 board and replace it with the
real word boards:

- esp32_devkitc_wroom
- esp32_devkitc_wrover (with PSRAM option)

Signed-off-by: Marek Matej <marek.matej@espressif.com>
Mark the 'esp32' board as deprecated after removing
it and replaced by the two real boards.

Signed-off-by: Marek Matej <marek.matej@espressif.com>
Refactor the ESP32 target SOCs together with
all related boards. Most braking changes includes:

- changing the CONFIG_SOC_ESP32* to refer to
  the actual soc line (esp32,esp32s2,esp32s3,esp32c3)
- replacing CONFIG_SOC with the CONFIG_SOC_SERIES
- creating CONFIG_SOC_FAMILY_ESP32 to embrace all
  the ESP32 across all used architectures
- introducing CONFIG_SOC_PART_NUMBER_* to
  provide a SOC model config
- introducing the 'common' folder to hide all
  commonly used configs and files.
- updating west.yml to reflect previous changes in hal

Signed-off-by: Marek Matej <marek.matej@espressif.com>
Fix minor typo in esp32s3-gpio-sigmap

Signed-off-by: Marek Matej <marek.matej@espressif.com>
Change the line length to comply with the rules.

Signed-off-by: Marek Matej <marek.matej@espressif.com>
@marekmatej marekmatej force-pushed the feature/espressif_soc_rework branch from 10617f2 to 8571418 Compare July 25, 2023 14:15
@mniestroj
Copy link
Member

@mniestroj, at the end I think we were mixing two different things - SOC_PART_NUMBER strings and bool flags. I was advocating for both! From your messages, I got the impression you wanted to remove both strings and flags. However, I must admit that having those strings for future use is not vindicated enough, so I apologize for the misunderstanding from my side.

No need for apologies! Actually it is true that I wanted both to be removed. I wanted to prevent blind copy-pasting of what @MaureenHelm mentioned in #58454 (comment), so I didn't take "this is already used in other platforms" as a good reason why it should be introduced for Espressif platform. This platform was fine without using those symbols up to now. If it will be useful, then fine. But the commit introducing it (and the discussions that we were having above @marekmatej) didn't provide a good reason for it.

This PR looks fine to me. I just have issues with SOC_PART_NUMBER (not critical though). Since others are good with it (based on the comments above) and overall this PR is bringing lots of net profit (thanks @marekmatej), then I will not stay in the way.

@mniestroj mniestroj dismissed their stale review July 25, 2023 15:10

Don't block

@carlescufi
Copy link
Member

This PR looks fine to me. I just have issues with SOC_PART_NUMBER (not critical though). Since others are good with it (based on the comments above) and overall this PR is bringing lots of net profit (thanks @marekmatej), then I will not stay in the way.

Just to be clear @mniestroj, SOC_PART_NUMBER has now been removed from this PR.

@mniestroj
Copy link
Member

This PR looks fine to me. I just have issues with SOC_PART_NUMBER (not critical though). Since others are good with it (based on the comments above) and overall this PR is bringing lots of net profit (thanks @marekmatej), then I will not stay in the way.

Just to be clear @mniestroj, SOC_PART_NUMBER has now been removed from this PR.

Now we got confused... in this context I meant choice SOC_PART_NUMBER with bool SOC_PART_NUMBER_*.

string SOC_PART_NUMBER was removed, indeed. @carlescufi

@carlescufi
Copy link
Member

This PR looks fine to me. I just have issues with SOC_PART_NUMBER (not critical though). Since others are good with it (based on the comments above) and overall this PR is bringing lots of net profit (thanks @marekmatej), then I will not stay in the way.

Just to be clear @mniestroj, SOC_PART_NUMBER has now been removed from this PR.

Now we got confused... in this context I meant choice SOC_PART_NUMBER with bool SOC_PART_NUMBER_*.

string SOC_PART_NUMBER was removed, indeed. @carlescufi

Aah, of course. The choice vs the string Kconfig :) But as you say others chimed in in favor of keeping the choice, so thanks for unblocking. Merging this now.

@carlescufi carlescufi merged commit 938732c into zephyrproject-rtos:main Jul 25, 2023
38 checks passed
mniestroj added a commit to mniestroj/mcuboot that referenced this pull request Jul 27, 2023
This is a follow-up on upstream Zephyr split from `esp32` to distinct
esp32_devkitc_wroom and esp32_devkitc_wrover (see [1] and [2]).

[1] zephyrproject-rtos/zephyr#58454
[2] zephyrproject-rtos/zephyr@3776402

Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
almir-okato pushed a commit to mcu-tools/mcuboot that referenced this pull request Jul 29, 2023
This is a follow-up on upstream Zephyr split from `esp32` to distinct
esp32_devkitc_wroom and esp32_devkitc_wrover (see [1] and [2]).

[1] zephyrproject-rtos/zephyr#58454
[2] zephyrproject-rtos/zephyr@3776402

Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
kartben added a commit to kartben/zephyr that referenced this pull request Sep 7, 2023
Following zephyrproject-rtos#58454, fixed a few remaining references to old "virtual"
esp32 board.

Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
carlescufi pushed a commit that referenced this pull request Sep 7, 2023
Following #58454, fixed a few remaining references to old "virtual"
esp32 board.

Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
Thalley pushed a commit to Thalley/zephyr that referenced this pull request Sep 8, 2023
Following zephyrproject-rtos#58454, fixed a few remaining references to old "virtual"
esp32 board.

Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
kartben added a commit to kartben/zephyr that referenced this pull request Sep 11, 2023
Following zephyrproject-rtos#58454, fixed a few remaining references to old "virtual"
esp32 board.

Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
coran21 pushed a commit to coran21/zephyr that referenced this pull request Sep 21, 2023
Following zephyrproject-rtos#58454, fixed a few remaining references to old "virtual"
esp32 board.

Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wifi: esp32: build error due to WIFI_SCAN_TYPE_{ACTIVE,PASSIVE} redeclaration