search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.87k stars 6.62k forks source link

hwmv2: Mismatch between SOC_SERIES name and value #69317

Open nordicjm opened 8 months ago

nordicjm commented 8 months ago

For hwmv2, there should be a match between the names and values of SoCs, this should take the form of:

config SOC_SERIES
        default "nrf51" if SOC_SERIES_NRF51
        default "nrf52" if SOC_SERIES_NRF52

As it stands today without any exceptions, the following would be wrong:

config SOC_SERIES
        default "nrf51" if SOC_SERIES_NRF51X
        default "nrf52" if SOC_SERIES_NRF52X

The reason for needing alignment in SOC_SERIES and SOC names/values is so that CI checks can be added for spotting typos, as there currently is with boards in hwmv2 now. An example hwmv2 Kconfig.<board> file:

config BOARD_V2M_BEETLE
        select SOC_BEETLE_R0

and corresponding board.yml file:

board:
  name: v2m_beetle
  vendor: ARM
  socs:
  - name: beetle_r0

If there is a typo in a board name, the following illustrates how a mismatch is now detected in hwmv2 which would not be caught had the bool prompt be left:

config BOARD_V2M_BEETLE
        bool
        select SOC_BEETLE_R0

config BOARD_V8M_BEETLE
        bool
        select SOC_BEETLE_R0

There is similar checking which has been added for SOC_PART_NUMBER whereby issues have been found and corrected.

This checking can be expanded to SOC_SERIES and SOC, as of right now the aim is for the values of these to match the name without any rules or exceptions but this is not currently adhered to by all converted hwmv2 SoCs.

@manuargue has helpfully put together a script which lists the current mismatches for SOC_SERIES:

Therefore, the possible options that need to be decided upon:

nordicjm commented 8 months ago

@fabiobaltieri @jvasanth1 I think yours is the exception here and the name is incorrect

erwango commented 8 months ago

@nordicjm This is doable but my initial question is what is the use of Kconfig symbol SOC_SERIES ?

nordicjm commented 8 months ago

@nordicjm This is doable but my initial question is what is the use of Kconfig symbol SOC_SERIES ?

It should match up with what is in soc.yml. Can think of it as how boards no longer have bool in their Kconfig.<board> files but the build system generates it automatically, that too should be possible for SOC fields in future e.g. SOC, SOC_FAMILY, SOC_SERIES.

Actually here is a good example, take this soc.yml file https://github.com/zephyrproject-rtos/zephyr/blob/collab-hwm/soc/aspeed/soc.yml:

series:
- name: ast10x0
  socs:
  - name: ast1030

and Kconfig.soc files https://github.com/zephyrproject-rtos/zephyr/blob/collab-hwm/soc/aspeed/Kconfig.soc and https://github.com/zephyrproject-rtos/zephyr/blob/collab-hwm/soc/aspeed/ast10x0/Kconfig.soc:

config SOC_FAMILY_ASPEED
    bool

config SOC_FAMILY
    default "aspeed" if SOC_FAMILY_ASPEED

config SOC_SERIES_AST10X0
    bool
    select SOC_FAMILY_ASPEED
    help
      Enable support for ASPEED AST10X0 series

config SOC_AST1030
    bool
    select SOC_SERIES_AST10X0
    help
      AST1030

config SOC_SERIES
    default "ast10x0" if SOC_SERIES_AST10X0

config SOC
    default "ast1030" if SOC_AST1030

The soc.yml file here is wrong because it defines a soc series and soc but does not define a soc family, so if the build system parses all soc.yml files and generated all the SOC_FAMILY_<X> symbols automatically instead of Kconfig.soc files doing it then this would fail for this board because the soc.yml file does not define it when it should, indicating an error

tejlmand commented 8 months ago

Please see this PR which introduces SoC name checking in existing compliance check: https://github.com/zephyrproject-rtos/zephyr/pull/69309

tejlmand commented 8 months ago

Can think of it as how boards no longer have bool in their Kconfig. files but the build system generates it automatically, that too should be possible for SOC fields in future e.g. SOC, SOC_FAMILY, SOC_SERIES.

See reason here why this haven't been implemented so far: https://github.com/zephyrproject-rtos/zephyr/pull/69309#issuecomment-1958907445

golowanow commented 8 months ago

soc/intel/intel_adsp/ace/Kconfig.soc default string != option name ('intel_ace15_mtpm', 'INTEL_ACE')

@nordicjm this one is for toolchain selection

https://github.com/zephyrproject-rtos/zephyr/blob/d6cb9c967227b05e59a3f8e5a619fa695c174594/soc/intel/intel_adsp/ace/Kconfig.soc#L13-L14

nordicjm commented 8 months ago

soc/intel/intel_adsp/ace/Kconfig.soc default string != option name ('intel_ace15_mtpm', 'INTEL_ACE')

@nordicjm this one is for toolchain selection

https://github.com/zephyrproject-rtos/zephyr/blob/d6cb9c967227b05e59a3f8e5a619fa695c174594/soc/intel/intel_adsp/ace/Kconfig.soc#L13-L14

Removed from list

fabiobaltieri commented 8 months ago

@fabiobaltieri @jvasanth1 I think yours is the exception here and the name is incorrect

yeah that one gave me some head scratching, initially tried to rename the directory to match and then bumped into a dependency with the HAL modules/hal/microchip/mec/CMakeLists.txt and figure I'd leave it alone, @nordicjm do you think it's worth fixing it now a the cost of having a dependency on a hal change?

erwango commented 8 months ago

@nordicjm STM32 https://github.com/zephyrproject-rtos/zephyr/pull/69325

nordicjm commented 8 months ago

@fabiobaltieri @jvasanth1 I think yours is the exception here and the name is incorrect

yeah that one gave me some head scratching, initially tried to rename the directory to match and then bumped into a dependency with the HAL modules/hal/microchip/mec/CMakeLists.txt and figure I'd leave it alone, @nordicjm do you think it's worth fixing it now a the cost of having a dependency on a hal change?

HAL update is fine, think we have about 5 or 6 repos that have changes for hwmv2 that will need merging

fabiobaltieri commented 8 months ago

HAL update is fine, think we have about 5 or 6 repos that have changes for hwmv2 that will need merging

Cool, @jvasanth1 @albertofloyd is this something you could look into? I'm OOO at the moment.

manuargue commented 8 months ago

for NXP S32 addressed in https://github.com/zephyrproject-rtos/zephyr/pull/69292

fabiobaltieri commented 8 months ago

https://github.com/zephyrproject-rtos/zephyr/pull/69528 for the MEC stuff (needs https://github.com/zephyrproject-rtos/hal_microchip/pull/16)