mbed-ce / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
79 stars 15 forks source link

Re-add CMSIS MCU description file and a utility to manage it #282

Closed multiplemonomials closed 3 months ago

multiplemonomials commented 4 months ago

Summary of changes

This is the first PR of a series that will restore support for loading memory bank information of targets based on CMSIS pack descriptions. This support has never existed in Mbed CLI 2, meaning that build flags are wrong or missing in certain cases. Not sure how we got away with it for this long, but this really needs to be fixed.

Now, the target memory bank information is theoretically available for download via the cmsis-pack-manager Python package. However, there are 4 issues with using this package as-is:

  1. It takes a long time to download the cache, and the cache must be updated occasionally. That's bad for user experience.
  2. Sometimes certain manufacturer servers are offline, so randomly one MCU or another just won't show up in the cache at all!
  3. Some MCUs don't exist in this index, like RP2040 or some Renesas MCUs
  4. Some of the data in the CMSIS pack index is either wrong (e.g listing ROM on the MIMXRT, which does not have any programmable ROM), or vague, like not using memory bank names that match the datasheet.

So, I feel like we need a system that lets us take advantage of the data available via CMSIS pack manager, without forcing ourselves to only use it. For this reason, I'd like to maintain a local file inside Mbed that's used to source the memory bank information. However, we still need some tools to maintain this file as targets are added & removed from Mbed.

So, what I did is:

Then, I wrote a small subcommand for mbed_tools that allows us to manage this file. It just has 3 commands:

For this first PR, all I'm adding is this tool plus the JSON file with the descriptions. Actually using this file as part of the build is the next step.

Impact of changes

Had to remove the apt requirements files (already legacy) as this method stops working now that we depend on cmsis-pack-manager.

Migration actions required

Documentation

Removed mentions of the legacy apt method of installing python packages from the wiki.


Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[X] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Have not added any tests for the new script, since it's a maintenance tool only and the output will always be able to be checked by humans. Hopefully that's OK.


Reviewers


VictorWTang commented 3 months ago

I noticed that when I removed a description from cmsis_mcu_descriptions.json5 (I removed ADuCM3029), then ran fetch-missing, then added the output from the command, the descriptions included differences:

diff --git a/targets/cmsis_mcu_descriptions.json5 b/targets/cmsis_mcu_descriptions.json5
index b90bd6e029..aa121a020e 100644
--- a/targets/cmsis_mcu_descriptions.json5
+++ b/targets/cmsis_mcu_descriptions.json5
@@ -7,7 +7,8 @@
                 "ram_size": null,
                 "ram_start": null,
                 "size": 262144,
-                "start": 0
+                "start": 0,
+                "style": "Keil"
             }
         ],
         "family": "ADuCM302x Series",
@@ -15,7 +16,7 @@
             "pack": "ADuCM302x_DFP",
             "url": "http://download.analog.com/tools/EZBoards/CM302x/Releases/",
             "vendor": "AnalogDevices",
-            "version": "3.2.0"
+            "version": "3.2.1"
         },
         "memories": {
             "IRAM1": {
@@ -29,6 +30,7 @@
                     "write": true
                 },
                 "default": true,
+                "p_name": null,
                 "size": 16384,
                 "start": 536870912,
                 "startup": false
@@ -44,6 +46,7 @@
                     "write": true
                 },
                 "default": true,
+                "p_name": null,
                 "size": 16384,
                 "start": 537133056,
                 "startup": false
@@ -59,25 +62,27 @@
                     "write": false
                 },
                 "default": true,
+                "p_name": null,
                 "size": 262144,
                 "start": 0,
                 "startup": true
             }
         },
         "name": "ADuCM3029",
-        "processor": {
-            "Symmetric": {
+        "processors": [
+            {
+                "address": null,
+                "ap": 0,
+                "apid": null,
                 "core": "CortexM3",
+                "default_reset_sequence": null,
+                "dp": 0,
                 "fpu": "None",
                 "mpu": "NotPresent",
-                "units": 1
+                "name": null,
+                "svd": "SVD/ADuCM302x.svd",
+                "unit": 0
             }
-        },
-        "sectors": [
-            [
-                0,
-                2048
-            ]
         ],
         "sub_family": null,
         "vendor": "Analog Devices:1"

Are these differences important? If so, would it be useful to do a "deep" check and not just check for MCU names?

multiplemonomials commented 3 months ago

Ah yeah... that will be because the existing file has relatively old data -- most of it was imported when these MCUs were added and then not touched since then. So, for many MCUs, the CMSIS data has in changed and Mbed is in need of updates. However, some of the data is also wrong in the CMSIS database (e.g. the entry for MIMXRT1062, the MCU we know and love, is flagrantly wrong). So, we cannot just do a bulk update without the risk of breaking useful things.

In the case of this specific MCU, the differences are not important, because none of this info is used by Mbed. We basically just care about the memory bank info at this point. Maaaybe the SVD file could be useful someday (used by the VS Code debugger to provide extra information) but we aren't using that yet. Unfortunately I think updates like you're doing will have to be done on a case by case basis, with knowledge of what changes have been made verses the database information.