Disable Cortex-A in tooling for better error messages #4377
Conversation
…ted in CMSIS5/RTX2" This partially reverts commit e535493.
tools/options.py
Outdated
| @@ -121,3 +121,12 @@ def extract_profile(parser, options, toolchain, fallback="develop"): | |||
| " supported by profile {}").format(toolchain, | |||
| filename)) | |||
| return profile | |||
|
|
|||
| def not_disabled(parser, mcu): | |||
There was a problem hiding this comment.
this looks like generic function but semes like only for cortex-a, should it be an arugment and related to MCU (mcu in the name?) . Why is it negative by default - not_disabled?
should it be is_core_disabled(parser, core, mcu) ? However this would make more work to get the url link work with the release one. If no, at least is_cortex_a_disabled or something ?
There was a problem hiding this comment.
It's named that way so that I can use assert in front of it. Even if asserting "not_disabled" is not actually what happens, it documents the intent well. The intent being Fail now, hard or keep going.
There was a problem hiding this comment.
Also, is_cortex_a_disabled implies that I'm checking if all cortex-A CPUs are disabled, not this particular CPU.
There was a problem hiding this comment.
I would vote for is_disabled, usually logic is positive is_active, is_disabled, is_whatever. No strong preference, I care more about the name than logic being negative.
Also, is_cortex_a_disabled implies that I'm checking if all cortex-A CPUs are disabled, not this particular CPU.
The function should have be more specific than generic name, either provide MCU parameter or something more to make it generic or change the function name specify what the function does is_rza1h_disabled() (the file for the implementation is options.py that is for anything).
these are minor things though
There was a problem hiding this comment.
I would vote for is_disabled,
The function has to either pass or fail an entire build. I'm willing to go for is_enabled
is_rza1h_disabled()
The function does not test for this. the function test for all Cortex-A support. As it stands, the RZ_A1H is the only cortex-A part we have. Further, it is generic: it tests any mcu you give it to verify that the MCU is enabled in the current release. If we disabled cortex-M7, we would extend this function to disable all cortex-M7 builds as well.
I propose: mcu_is_enabled
|
Thank you for this PR. I can not confirm from the error message whether there is Cortex-A(RZ/A1H etc) in the future release. RZ_A1H example:
|
|
@TomoYamanaka Thanks for the text update. I'll update the commit to use that text. |
theotherjimmy
force-pushed
the
disable-cortex-A
branch
2 times, most recently
from
22df496 to
67ca2c4
Compare
|
Updated. |
|
Thanks! |
|
/morph test |
Result: FAILUREYour command has finished executing! Here's what you wrote!
OutputBuild failed! |
|
The build above seems to be OK , no errors. /morph test |
Result: FAILUREYour command has finished executing! Here's what you wrote!
OutputBuild failed! |
|
@theotherjimmy please look at failures. looks like test tries to execute anyway for these cortex-a targets? |
|
Yeah, I'm going to have to add some more code! |
theotherjimmy
force-pushed
the
disable-cortex-A
branch
from
b929d32 to
5a2e928
Compare
|
I squashed the two commits, so that we can revert all of the tools changes as one commit for 5.6 |
Disable Cortex-A in compile supported matrix Disable Cortex-A in export supported matrix
theotherjimmy
force-pushed
the
disable-cortex-A
branch
from
5a2e928 to
f5859b3
Compare
|
/morph test |
|
/morph export-build |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
Outputmbed Build Number: 46 All exports and builds passed! |
Description
This PR will disable building of Cortex-A targets for the mean time. The tools
bits that disable Cortex-A are (currently) in a single commit that can be
reverted to re-enable Cortex-A support.
Tests