search

FreeRTOS / FreeRTOS-Kernel

FreeRTOS kernel files only, submoduled into https://github.com/FreeRTOS/FreeRTOS and various other repos.
https://www.FreeRTOS.org
MIT License
2.71k stars 1.11k forks source link

Add a check for configENABLE_MVE to M23, M33 ports #968

Closed aggarg closed 8 months ago

aggarg commented 8 months ago

Description

configENABLE_MVE is only applicable to Cortex-M55 and Cortex-M85 ports. It must not be defined to 1 for other ARMv8_m ports.

Test Steps

Built M23 and M33 example projects locally.

Checklist:

Related Issue

https://forums.freertos.org/t/status-of-the-gcc-arm-cm55-and-ntz-ports/19042

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

codecov[bot] commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (5a2237a) 93.64% compared to head (e0a7b0f) 93.64%.

:exclamation: Current head e0a7b0f differs from pull request most recent head 746a3f3. Consider uploading reports for the commit 746a3f3 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #968 +/- ## ======================================= Coverage 93.64% 93.64% ======================================= Files 6 6 Lines 3194 3194 Branches 885 885 ======================================= Hits 2991 2991 Misses 91 91 Partials 112 112 ``` | [Flag](https://app.codecov.io/gh/FreeRTOS/FreeRTOS-Kernel/pull/968/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FreeRTOS) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/FreeRTOS/FreeRTOS-Kernel/pull/968/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FreeRTOS) | `93.64% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FreeRTOS#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sonarcloud[bot] commented 8 months ago

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Skptak commented 8 months ago

I feel this change is inconsistent with https://github.com/FreeRTOS/FreeRTOS-Kernel/pull/966

If we removed the check for the configTOTAL_MPU_REGIONS from the CM3_MPU port because it is not possible to configure it, why would we add a check to ensure that configENABLE_MVE is disabled in a port that does not support the MVE extension? Shouldn't we do the same thing we do in the ARM_CM3_MPU port then and allow users to set this, even though it then does nothing?

aggarg commented 8 months ago

Shouldn't we do the same thing we do in the ARM_CM3_MPU port then and allow users to set this, even though it then does nothing?

The reason I removed that config from the M3 port is because we are checking a config and that config is used nowhere in the code. That is not the case here as configENABLE_MVE is used in the port.c (which is common for all ARMv8M ports).

Skptak commented 8 months ago

The reason I removed that config from the M3 port is because we are checking a config and that config is used nowhere in the code. That is not the case here as configENABLE_MVE is used in the port.c (which is common for all ARMv8M ports).

Thanks for the clarification! That makes sense